Notify and surface when a remote/cloud-agent session needs input#4605
Notify and surface when a remote/cloud-agent session needs input#4605iscekic wants to merge 9 commits into
Conversation
…attention-notifications
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.
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryOnly 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
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 file changed)
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 SummaryThree 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
Issue Details (click to expand)SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 1602719)Status: 4 Issues Found | Recommendation: Address before merge Executive SummaryThe highest-risk issue is in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (14 files)
Reviewed by claude-sonnet-5 · Input: 18 · Output: 3.6K · Cached: 358.1K Review guidance: REVIEW.md from base branch |
- 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.
|
(bot) Thanks — addressed in
|
oxlint no-confusing-void-expression forbids returning the void expression from the arrow shorthand passed to expect().
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
REMOTE_SESSION_ATTENTION_PUSH_ENABLEDtotrue, enabling the existing remote-session attention push path for active CLI sessions.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
statusremains'completed'for needs-input notifications, matching existing deployed-schema expectations (#4445).In-app session-list indicator (mobile)
When a session's
statusisquestionorpermission, its row in the mobile Agents session list shows a soft-pulsing amber (warn-tone)StatusDotplus aNEEDS INPUTmeta label (and "needs input" in the row'saccessibilityLabel), replacing the green live dot / timestamp meta. The indicator clears when the status leaves needs-input or when the user opens the session.status/status_updated_atalready carried bycliSessionsV2.list/search/getandactiveSessions.list— no backend, tRPC, or DB changes for this surface. Web'sSessionStatusIndicatoris 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 onglobalThisso the detail-screen writer and the session-list readers share one instance across Expo Router route bundles.StatusDotgains an opt-inpulseprop (soft opacity breathe, static underuseReducedMotion); default off leaves all existing call sites unchanged.freezeOnBlurtab 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 thesession-attentionpredicate/ack-store state machine.xcrun simctl push) for both payload shapes.question/permissionrow shows the amber pulsing dot +NEEDS INPUTlabel 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.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).