feat(notifications): agent-callable push notification tool (cloud + mobile)#4627
Open
iscekic wants to merge 7 commits into
Open
feat(notifications): agent-callable push notification tool (cloud + mobile)#4627iscekic wants to merge 7 commits into
iscekic wants to merge 7 commits into
Conversation
Add a user_notification_preferences table (user_id PK/FK, agent_push_enabled default true) with migration 0190, plus getNotificationPreferences and setNotificationPreferences tRPC procedures scoped per user. Clean up the preference row in the soft-deletion path since soft delete has no FK cascade.
Add sendAgentSessionNotification RPC that resolves the session and title server-side, reads the per-user agent push preference (fail closed), and dispatches with presence context and a per-session sliding-window rate limit. NotificationChannelDO gains the rate-limit gate (prune-on-read, suppressed_rate_limit outcome, rl: GC via the cleanup alarm) and a default-off local push sink that records a token-redacted payload without calling Expo for local E2E. The existing sendCloudAgentSessionNotification contract is unchanged.
Add an Agent notifications row to the notifications card, bound to the per-user preference via getNotificationPreferences with an optimistic setNotificationPreferences mutation. The switch is editable whenever the preference query resolves, independent of this device's OS permission or push-token registration, since the preference is per-user and cross-device. Toggle cache and derived-state logic is extracted into a unit-tested hook.
Accept the new agent_notification ingest item on both the queue and direct paths, turn it into an AttentionSignal carrying the notification id and full message, and dispatch it via the new sendAgentSessionNotification RPC without the live-CLI gate (the user explicitly asked for the ping) while keeping root-session eligibility. A durable per-identity dispatch marker makes delivery replay-safe and loss-safe: the signal re-emits while pending and is marked dispatched only on a terminal local decision, so a thrown RPC or a failed session lookup leaves it pending rather than dropping or double-sending.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryAll 6 previously reported issues (rate-limit double-consumption, hardcoded sink Files Reviewed (5 files)
Previous Review Summary (commit 98a5358)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 98a5358)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe highest-risk issue is a rate-limit accounting bug in Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (37 files)
Reviewed by claude-sonnet-5 · Input: 26 · Output: 4.6K · Cached: 561.2K Review guidance: REVIEW.md from base branch |
- NotificationChannelDO: skip rate-limit consumption on retries and write a pending idempotency marker for rate-limited no-badge dispatches so a replay of the same notification does not re-consume a rate-limit slot; report the sink tokenCount as tokens.length - mobile: guard getNotificationPreferences with enabled: isAuthenticated so a logged-out user does not hit a spurious error row - session-ingest: correct the agent_notification identity separator in a doc comment - mobile tests: make the preference-hook cases assert distinct real behaviors
…otifications' into feat/agent-push-tool # Conflicts: # packages/db/src/migrations/meta/0190_snapshot.json # packages/db/src/migrations/meta/_journal.json # services/session-ingest/drizzle/meta/0003_snapshot.json # services/session-ingest/drizzle/meta/_journal.json # services/session-ingest/drizzle/migrations.js # services/session-ingest/src/db/sqlite-schema.ts # services/session-ingest/src/ingest/direct-ingest.ts # services/session-ingest/src/remote-session-notifications.test.ts # services/session-ingest/src/remote-session-notifications.ts
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
1 participant
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
Adds an agent-callable push notification path so a
notify_usertool in the kilo CLI can send a push to the user's phone (Kilo mobile app) for explicitly requested pings and significant mid-run milestones. The platform already auto-notifies on needs-input and completion; this path is deliberately separate and gated so it cannot duplicate or bypass those.This is the cloud half. The CLI tool itself is in the companion kilocode PR (see Cross-repo dependency).
What's here
packages/db,apps/web): newuser_notification_preferencestable (per-user,agent_push_enableddefault true), migration0190,getNotificationPreferences/setNotificationPreferencesprocedures (per-user scoped), and cleanup of the row in the soft-deletion path.packages/notifications,services/notifications): newsendAgentSessionNotificationRPC that resolves the session + title server-side, reads the preference (fail-closed), and dispatches with presence context and a per-session sliding-window rate limit (5 / 600s) inNotificationChannelDO(suppressed_rate_limitoutcome, prune-on-read,rl:GC via the cleanup alarm). A default-off local push sink (PUSH_SINK_MODE, absent fromwrangler.jsonc) records a token-redacted payload without calling Expo, for local E2E. The existingsendCloudAgentSessionNotificationcontract is unchanged.services/session-ingest): accepts the newagent_notificationitem on both ingest paths, turns it into an AttentionSignal (carrying the notification id + full message), and dispatches via the new RPC without the live-CLI gate (the user explicitly asked for the ping) while keeping root-session eligibility. A durable per-identity dispatch marker makes delivery replay-safe and loss-safe.apps/mobile): an "Agent notifications" row in the notifications card bound to the preference with an optimistic mutation; the switch stays editable independent of this device's OS permission / push-token registration (per-user, cross-device).Verification
delivered; preference OFF →suppressed_preference/ ON →delivered(effective immediately); presence foreground →suppressed_presence/ background →delivered; per-session rate limit 5 dispatched, 6thsuppressed_rate_limit; tool-in-disconnected-session returns friendly failure text; the "Agent notifications" row is locatable by its accessibility label and its layout stays intact at accessibility text sizes.datareusestype: 'cloud_agent_session'so tap-routing to/(app)/agent-chat/<cliSessionId>uses the existing, unchanged deep-link map. Real-device banner delivery should be confirmed before production release.Cross-repo dependency
The
notify_userCLI tool lives in the kilocode PR (feat/agent-push-tool). Contracts (ingest item shape, RPC shape, tool policy text) are pinned across both PRs.Deploy ordering (§9.1)
DB migration → notifications worker → session-ingest worker → web (tRPC) / mobile app → then the kilocode CLI release. The kilocode CLI must not be released before this cloud deploy. session-ingest degrades gracefully if the RPC is not yet deployed (the dispatch error is caught only at the outer boundary; ingest never fails). Old servers handle the new item per existing unknown-item behavior; app versions without the toggle simply never write a preference row (default ON).
This PR is stacked on #4605; GitHub will retarget it to
mainwhen #4605 merges.