Notifications - Add cloud agent push notifications#2954
Open
Notifications - Add cloud agent push notifications#2954
Conversation
Contributor
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Resolved Since Previous Review
Other Observations (not in diff)Issues found in unchanged code that cannot receive inline comments:
Files Reviewed (4 files)
Reviewed by gpt-5.5-20260423 · 1,369,062 tokens |
42acc4f to
f39952a
Compare
f39952a to
571a8fe
Compare
Revert drive-by refactors and webhook/queue hardening that landed alongside the cloud-agent push feature in 571a8fe. The remaining diff keeps only the path: cloud-agent-next dispatches via the NOTIFICATIONS service binding, the notifications worker's sendCloudAgentSessionNotification RPC resolves the session and delegates to the shared sendChannelPush helper, and the mobile app handles the new cloud_agent_session payload.
…ore ExecutionStatus - Drop optional `NOTIFICATIONS?` from KiloClawEnv, CloudAgentEnv, and PersistenceEnv; the binding is configured in every wrangler environment, so the runtime guards were dead code. - Restore `ExecutionStatus` (from core/execution.ts) at the execution lifecycle / ingest boundaries; `CloudAgentPushStatus` now only lives in the push-notification modules and downstream of `isTerminalStatus`. - Stop re-exporting NotificationData from apps/mobile; it's only used inside the file and was tripping `check:unused` in CI.
Restores the helper removed in 571a8fe as a standalone `getConnectedStreamClientCount(state)` export so CloudAgentSession doesn't have to know the internal 'stream' tag. Both the `getConnectedClientCount` RPC and the push-skip check in `dispatchPushNotification` now go through it.
- Remove stale CloudAgentPushJob import and NOTIFICATIONS_QUEUE declaration from cloud-agent-next test env types; the producer now uses the NOTIFICATIONS service binding instead of a queue. - Add notifications as a local dependency of cloud-agent-next so the dev orchestrator starts the worker that the new service binding targets.
Trace entry/exit for each hop of the terminal-status push flow so local testing can verify where a notification goes missing: - cloud-agent-next CloudAgentSession.dispatchPushNotification logs skip reasons (active stream, missing metadata) and RPC dispatch/result. - notifications NotificationsService.sendCloudAgentSessionNotification logs entry, missing_user/missing_session skips, and fanout hand-off. - channel-push.sendChannelPush logs entry, badge+token state, the no-token skip, and the Expo ticket/stale-token summary.
The notifications worker is depended on by both kiloclaw and cloud-agent, so surface it as a top-level group in dev:start rather than nesting it under kiloclaw. Both dependent groups now pull it in via groupDependsOn.
…path" This reverts commit c46d943.
Restore the optional NOTIFICATIONS binding on KiloClawEnv and the matching runtime guards in lifecycle-push, along with the "no-ops when NOTIFICATIONS binding is unavailable" test. Only the kiloclaw/src changes from 6ddbc67 are reverted; the cloud-agent-next ExecutionStatus restore and apps/mobile NotificationData change from that commit are kept.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Terminal cloud-agent session events (completed / failed / interrupted) now produce push notifications on the user's registered mobile devices.
Verification