feat(session-ingest): push notifications for remote session attention signals#4445
feat(session-ingest): push notifications for remote session attention signals#4445eshurakov wants to merge 1 commit into
Conversation
Code Review SummaryStatus: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe latest changes replace web-view push suppression with a presence-based check and add local-CLI-activity suppression, but the previously flagged eligibility gap (local terminal CLI sessions still qualify for pushes, not just VS Code/Agent Manager sessions) remains unresolved since Overview
Issue Details (click to expand)WARNING
Files Reviewed (16 files, incremental)
Fix these issues in Kilo Cloud Previous Review Summaries (2 snapshots, latest commit e9f3fd2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit e9f3fd2)Status: 1 Issue Found | Recommendation: Address before merge Executive SummaryThe incremental changes replace the platform-based push eligibility check with an active-CLI-heartbeat check that no longer restricts remote-session attention pushes to VS Code/Agent Manager sessions, broadening pushes to local terminal CLI sessions as well. Overview
Issue Details (click to expand)WARNING
Files Reviewed (8 files, incremental)
Fix these issues in Kilo Cloud Previous review (commit 552691c)Status: No Issues Found | Recommendation: Merge Executive SummaryReviewed the new remote-session attention-push pipeline (SessionIngestDO signal detection, queue-consumer dispatch/partial-flush-on-failure, remote-session-notifications eligibility/body-building, and the UserConnectionDO subscriber check) with high confidence and found no security, correctness, or breaking-change issues in the changed code. Files Reviewed (14 files)
Generated files ( Reviewed by claude-sonnet-5 · Input: 58 · Output: 13.1K · Cached: 1.6M Review guidance: REVIEW.md from base branch |
eshurakov
force-pushed
the
likeable-dreadnought
branch
from
552691c to
e9f3fd2
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| }; | ||
|
|
||
| /** Only root sessions (no parent) can be eligible for attention pushes. */ | ||
| export function isEligibleForRemoteSessionAttention(session: RemoteSessionInfo): boolean { |
There was a problem hiding this comment.
WARNING: Eligibility check no longer restricts pushes to remote (VS Code / Agent Manager) sessions
isEligibleForRemoteSessionAttention dropped the createdOnPlatform check that previously limited eligibility to vscode/agent-manager root sessions. The new hasActiveCliSession gate added in dispatchRemoteSessionAttentionSignal (below) doesn't restore that restriction: UserConnectionDO.hasActiveCliSession returns true for any connected CLI reporting the session in its heartbeat, including a plain local cli session run directly in a user's own terminal. As a result, users actively working in a local terminal session will now also receive mobile push notifications on every idle/needs-input transition, which contradicts this PR's stated goal of closing the loop only for sessions started from VS Code or the Agent Manager.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
… signals Detect completed assistant turns and needs-input status transitions during ingest and dispatch a mobile push to the session owner for remote (vscode, agent-manager) root sessions. Pushes are suppressed when the user already has the session open in the web app (UserConnectionDO.hasSessionSubscribers). - SessionIngestDO emits AttentionSignal(s) on idle/question/permission transitions, pairing completed turns with the just-finished assistant message excerpt. First-ever status writes are skipped to avoid pushing about an old turn on full-history backfill. - queue-consumer collects signals across chunks and dispatches them via a NOTIFICATIONS service binding; partial flush on failure preserves signals from already-committed chunks. - Adds @kilocode/notifications dependency and wrangler NOTIFICATIONS binding.
eshurakov
force-pushed
the
likeable-dreadnought
branch
from
e9f3fd2 to
2362a7a
Compare
1 participant
Summary
The session-ingest service now sends a mobile push to the session owner when a remote root session needs attention — either the assistant just finished a turn (status → idle) or the session is waiting on user input (status → question/permission). This closes the loop for users running sessions from VS Code or the Agent Manager, who otherwise have no signal that their session is ready for them.
Architectural changes:
SessionIngestDO.ingest()now returnsattentionSignalsalongsidechanges. On an idle transition (with a prior status already stored), it pairs the transition with the most recent completed assistant message and builds a push-ready text excerpt from that turn's ingested text parts. Needs-input transitions emit a signal without an excerpt.session-ingest-attention.tsowns status classification, completed-assistant-message detection (validated via zod), and excerpt assembly with whitespace collapse + 100-char truncation to fit push payloads.remote-session-notifications.tsowns eligibility (root sessions fromvscode/agent-manageronly), subscriber suppression, push body construction, and the dispatch coroutine. The push reuses thecompletedstatus enum for needs-input signals because the notifications service ignoresstatusand extending the enum would break a notifications worker deployed with the old schema.UserConnectionDOexposeshasSessionSubscribers(sessionId)so pushes are suppressed when the user already has the session open in the web app.queue-consumercollects signals across ingest chunks and dispatches them viactx.waitUntilafter committing. On the failure path it flushes partial metadata changes AND dispatches signals from already-committed chunks, because a committed status transition will not re-emit on retry.@kilocode/notificationsworkspace dependency and aNOTIFICATIONSservice binding (entrypointNotificationsService) wired inwrangler.jsoncand declared innotifications-binding.ts.Verification
Visual Changes
N/A
Reviewer Notes
hadPriorStatus). This prevents a full-history backfill of an already-idle session from pushing about an old turn.ctx.waitUntiland swallows/logs errors; a push failure never blocks ack'ing or retrying the queue message.findLastCompletedAssistantMessageIdscans the 50 newest message rows; R2-offloaded message rows (inline'{}') are skipped.hasSessionSubscribersis called per-signal againstUserConnectionDO; the stub is created viagetUserConnectionDO(env, { kiloUserId }).