Skip to content

Notifications - Add cloud agent push notifications#2954

Open
eshurakov wants to merge 9 commits intomainfrom
eshurakov/difficult-butter
Open

Notifications - Add cloud agent push notifications#2954
eshurakov wants to merge 9 commits intomainfrom
eshurakov/difficult-butter

Conversation

@eshurakov
Copy link
Copy Markdown
Contributor

@eshurakov eshurakov commented Apr 30, 2026

Summary

Terminal cloud-agent session events (completed / failed / interrupted) now produce push notifications on the user's registered mobile devices.

Verification

  • local testing

Comment thread dev/local/services.ts Outdated
Comment thread services/notifications/src/lib/channel-push.ts
@kilo-code-bot
Copy link
Copy Markdown
Contributor

kilo-code-bot Bot commented Apr 30, 2026

Code Review Summary

Status: 3 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 0

Fix these issues in Kilo Cloud

Issue Details (click to expand)

WARNING

File Line Issue
services/notifications/src/routes/webhooks.ts 19 Webhook request bodies are fully buffered before signature verification or any size cap, reopening unauthenticated oversized-body memory/CPU DoS risk.
services/notifications/src/queue-consumer.ts 19 Malformed receipt queue messages can throw in the catch logging path before msg.retry() or msg.ack() runs, causing poison-message batch failures.
services/cloud-agent-next/vitest.workers.config.ts 28 Miniflare queue consumer override uses the producer binding name instead of queue name, so the faster test batch timeout does not apply.

Resolved Since Previous Review

File Line Issue
N/A N/A No previously active findings were changed by this incremental diff.
Other Observations (not in diff)

Issues found in unchanged code that cannot receive inline comments:

File Line Issue
services/notifications/src/dos/NotificationChannelDO.ts 109 alarm() no longer schedules a future cleanup after marking messages as notified, so notified msg:* and dedup:* records can remain in DO storage indefinitely if no later webhook resets the alarm.
Files Reviewed (4 files)
  • services/kiloclaw/src/durable-objects/kiloclaw-instance.test.ts - 0 new issues
  • services/kiloclaw/src/durable-objects/kiloclaw-instance/lifecycle-push.ts - 0 new issues
  • services/kiloclaw/src/test-utils.ts - 0 new issues
  • services/kiloclaw/src/types.ts - 0 new issues

Reviewed by gpt-5.5-20260423 · 1,369,062 tokens

@eshurakov eshurakov force-pushed the eshurakov/difficult-butter branch from 42acc4f to f39952a Compare April 30, 2026 14:11
@eshurakov eshurakov force-pushed the eshurakov/difficult-butter branch from f39952a to 571a8fe Compare April 30, 2026 14:13
Comment thread services/cloud-agent-next/test/env.d.ts Outdated
Comment thread services/cloud-agent-next/wrangler.jsonc
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.
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.
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