Skip to content

Notify and surface when a remote/cloud-agent session needs input#4605

Open
iscekic wants to merge 9 commits into
mainfrom
feature/mobile-agent-attention-notifications
Open

Notify and surface when a remote/cloud-agent session needs input#4605
iscekic wants to merge 9 commits into
mainfrom
feature/mobile-agent-attention-notifications

Conversation

@iscekic

@iscekic iscekic commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Notifies and surfaces when a Kilo session (remote CLI or cloud agent) needs user input — a push notification plus an in-app indicator in the mobile Agents session list.

Backend push triggers

  • session-ingest: flips REMOTE_SESSION_ATTENTION_PUSH_ENABLED to true, enabling the existing remote-session attention push path for active CLI sessions.
  • cloud-agent-next: adds a raise-only attention classifier (question.asked / permission.asked) and dispatches a push notification when such an event occurs on a cloud-agent-web session, gated by presence (suppressed if a client is actively viewing the session) and session/child-session identity checks.

Both slices are additive and raise-only: there is no resolve/clear push, no outbox, scheduler, alarm, or migration. Push payload status remains 'completed' for needs-input notifications, matching existing deployed-schema expectations (#4445).

In-app session-list indicator (mobile)

When a session's status is question or permission, its row in the mobile Agents session list shows a soft-pulsing amber (warn-tone) StatusDot plus a NEEDS INPUT meta label (and "needs input" in the row's accessibilityLabel), replacing the green live dot / timestamp meta. The indicator clears when the status leaves needs-input or when the user opens the session.

  • Reads the status/status_updated_at already carried by cliSessionsV2.list/search/get and activeSessions.listno backend, tRPC, or DB changes for this surface. Web's SessionStatusIndicator is the production precedent.
  • session-attention.ts: a small predicate plus an in-memory, per-raise ack store (single writer = the session detail screen mount). Acks are intentionally not persisted across app restarts, so a genuinely unanswered question honestly re-raises. The store is anchored on globalThis so the detail-screen writer and the session-list readers share one instance across Expo Router route bundles.
  • StatusDot gains an opt-in pulse prop (soft opacity breathe, static under useReducedMotion); default off leaves all existing call sites unchanged.
  • The Agents list remounts on attention-revision change after refocus so the freezeOnBlur tab reliably reflects the clear-on-open ack on return.

Not included on this surface: Home-screen wiring, ack persistence, and any indicator inside the session detail screen.

Testing

  • services/session-ingest: full suite green (449 passed, 5 skipped), typecheck, lint clean.
  • services/cloud-agent-next: full suite green (2276 passed, 3 skipped), typecheck, lint clean.
  • apps/mobile: format, typecheck, lint, check:unused, and unit tests all green (1037 passed), including the session-attention predicate/ack-store state machine.
  • Fresh independent code reviews on both the push work and the in-app indicator: no actionable findings.
  • E2E on iOS simulator:
    • Push work: verified push wiring, presence-suppression gates, idempotent single-push-per-signal behavior, and simulated APNs delivery (xcrun simctl push) for both payload shapes.
    • In-app indicator: verified a stored question/permission row shows the amber pulsing dot + NEEDS INPUT label and "needs input" accessibility label; a non-attention control row shows neither; the indicator clears on status change and on opening the session (and re-shows on a new raise); the remote/active-only row shows and clears via a live user-connection heartbeat fixture; and the dot renders statically under Reduce Motion with no hard blink.
    • Real APNs-to-physical-device delivery and live LLM-triggered question events are not verifiable in the local dev environment (missing EXPO_ACCESS_TOKEN, no deterministic interactive-prompt harness) and are documented as accepted limitations, consistent with prior related work (feat(mobile): notify when agent sessions need input #4545).

@iscekic iscekic self-assigned this Jul 16, 2026
iscekic added 5 commits July 17, 2026 22:19
Anchor the attention ack store on globalThis so the single-writer detail
screen and the session-list readers share one instance across Expo Router
route bundles, and remount the Agents list on attention-revision change
after refocus so the freezeOnBlur'd list re-reads the ack store on return.
@iscekic iscekic changed the title Push notification when a remote/cloud-agent session needs input Notify and surface when a remote/cloud-agent session needs input Jul 18, 2026
Comment thread services/cloud-agent-next/src/persistence/CloudAgentSession.ts Outdated
Comment thread apps/mobile/src/lib/session-attention.ts
Comment thread apps/mobile/src/lib/session-attention.ts Outdated
Comment thread apps/mobile/src/components/agents/session-list-content.tsx Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Only a trivial formatting change (wrapping an arrow-function body in braces in a test file) was made since the last review; no logic changed and the previously-fixed issues remain fixed.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/lib/session-attention.ts 26 Ack-store entries Map has no eviction path for sessions that are abandoned before their status leaves the attention state (author has declined to change, by design)
Files Reviewed (1 file changed)
  • apps/mobile/src/lib/session-attention.test.ts - 0 issues (style-only change, no logic difference)

Fix these issues in Kilo Cloud

Previous Review Summaries (2 snapshots, latest commit dc86ab9)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit dc86ab9)

Status: 1 Issue Found | Recommendation: Address before merge

Executive Summary

Three of the four previously flagged issues (metadata-fetch outside try, throwing listener aborting notification, and over-broad list remount) are fixed in the latest commits; one lower-severity suggestion remains unaddressed.

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
apps/mobile/src/lib/session-attention.ts 26 Ack-store entries Map has no eviction path for sessions that are abandoned before their status leaves the attention state
Files Reviewed (4 files)
  • apps/mobile/src/components/agents/session-list-content.tsx - 0 issues (fixed: list now remounts only on focus-regain, not on every unrelated attention change)
  • apps/mobile/src/lib/session-attention.test.ts - 0 issues
  • apps/mobile/src/lib/session-attention.ts - 1 issue (fixed: throwing listener now isolated with try/catch)
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts - 0 issues (fixed: getMetadata() now called inside the try block)

Fix these issues in Kilo Cloud

Previous review (commit 1602719)

Status: 4 Issues Found | Recommendation: Address before merge

Executive Summary

The highest-risk issue is in CloudAgentSession.ts, where the new attention-push handler fetches session metadata outside its try block, so a metadata-parse failure would break the documented "log and swallow" guarantee for this best-effort push path.

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 2
Issue Details (click to expand)

WARNING

File Line Issue
services/cloud-agent-next/src/persistence/CloudAgentSession.ts 683 getMetadata() called outside the try block, breaking the documented swallow-and-log guarantee on failure
apps/mobile/src/components/agents/session-list-content.tsx 213 Global attention-revision counter used as the SectionList key causes a full remount/scroll reset on any unrelated session's attention change

SUGGESTION

File Line Issue
apps/mobile/src/lib/session-attention.ts 26 Ack-store entries Map has no eviction path for sessions that are abandoned before their status leaves the attention state
apps/mobile/src/lib/session-attention.ts 41 A throwing listener in bumpRevision's notify loop aborts notification to remaining subscribers
Files Reviewed (14 files)
  • apps/mobile/src/app/(app)/agent-chat/[session-id].tsx - 0 issues
  • apps/mobile/src/components/agents/session-list-content.tsx - 1 issue
  • apps/mobile/src/components/agents/session-row.tsx - 0 issues
  • apps/mobile/src/components/ui/session-row.tsx - 0 issues
  • apps/mobile/src/components/ui/status-dot.tsx - 0 issues
  • apps/mobile/src/lib/session-attention.test.ts - 0 issues
  • apps/mobile/src/lib/session-attention.ts - 2 issues
  • services/cloud-agent-next/src/persistence/CloudAgentSession.ts - 1 issue
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.test.ts - 0 issues
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.ts - 0 issues
  • services/cloud-agent-next/src/websocket/ingest.test.ts - 0 issues
  • services/cloud-agent-next/src/websocket/ingest.ts - 0 issues
  • services/session-ingest/src/remote-session-notifications.test.ts - 0 issues
  • services/session-ingest/src/remote-session-notifications.ts - 0 issues

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 18 · Output: 3.6K · Cached: 358.1K

Review guidance: REVIEW.md from base branch main

- cloud-agent-next: fetch session metadata inside the attention-push
  try/catch so a metadata failure is logged and swallowed as documented,
  never breaking the best-effort ingest path.
- mobile: key the Agents list on an attention revision snapshotted at tab
  focus, so it remounts to re-read the ack store only on return from a
  just-opened session, not on any unrelated session's attention change
  during browsing (preserving scroll).
- mobile: isolate attention-store subscribers so one throwing listener
  cannot prevent the rest from being notified.
@iscekic

iscekic commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

(bot) Thanks — addressed in dc86ab994:

  • WARNING — CloudAgentSession.ts getMetadata() outside try: Fixed. Moved the await this.getMetadata() call inside the existing try block so a metadata failure is logged and swallowed, preserving the documented best-effort contract for this ctx.waitUntil push path.
  • WARNING — session-list-content.tsx global revision as SectionList key: Fixed. The list is now keyed on the attention revision snapshotted only at tab focus (useFocusEffectgetRevisionSnapshot()), so it remounts to re-read the ack store exactly when an ack/reconcile happened while the list was away (returning from a just-opened session). A revision bump for an unrelated session during active browsing no longer changes the snapshot, so scroll is preserved. The freeze-driven remount is required because freezeOnBlur + react-freeze reveal stale cached cells on return without re-running them.
  • SUGGESTION — throwing listener aborts bumpRevision notify loop: Fixed. Each subscriber call is wrapped in try/catch so one throwing listener cannot prevent the rest from being notified; added a unit test asserting a later subscriber still runs once and the revision increments exactly once.
  • SUGGESTION — ack-store entries eviction: Not changed, by design. Entries are created only for sessions the user opens and are deleted on the next observed non-attention status (reconcileSessionAttention); each entry is a tiny { raiseId } keyed by session id, the store is per-app-run in-memory (never persisted), and it is cleared on restart. The only residue is an opened, still-attention session that leaves the list before a non-attention status is observed — bounded by distinct sessions opened per run and negligible in size. Adding an eviction policy would add complexity without a meaningful benefit.

oxlint no-confusing-void-expression forbids returning the void expression
from the arrow shorthand passed to expect().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant