Skip to content

5-category notification preferences for mobile, web, and backend - #4692

Merged
iscekic merged 11 commits into
mainfrom
feat/mobile-notification-categories
Jul 23, 2026
Merged

5-category notification preferences for mobile, web, and backend#4692
iscekic merged 11 commits into
mainfrom
feat/mobile-notification-categories

Conversation

@iscekic

@iscekic iscekic commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Replaces the single global agent_push_enabled boolean with a 5-category per-user push-preference model, enforced fail-closed at every notification RPC and surfaced on a dedicated mobile Notifications screen.

Changes

  • DB: additive migration with 4 new columns (chat_messages_enabled, agent_attention_enabled, session_status_enabled, kiloclaw_activity_enabled); agent_push_enabled retained as category 3 "Agent updates"
  • Notifications contract: optional category discriminator ('attention' | 'status') on cloud_agent_session pushes; per-RPC suppression reasons widened
  • Notifications enforcement: per-category fail-closed gates across chat, cloud-agent-session, session-ready, instance-lifecycle, and scheduled-action RPCs
  • tRPC: additive 5-key preference surface with backward-compatible agentPushEnabled for shipped clients
  • cloud-agent-next: terminal push producers set category:'status'; attention classifier sets category:'attention'
  • session-ingest: needs_input→attention, completed→status discrimination
  • Mobile: new dedicated Notifications screen with master OS+token gate, 5 category toggles with optimistic updates, and legacy snapshot rollback

Backward compatibility

  • tRPC responses include the legacy agentPushEnabled field alongside the new per-category keys
  • setNotificationPreferences accepts both agentUpdates and agentPushEnabled input
  • Category field is optional on RPC inputs with rolling-deploy default 'status'

Verification

  • Mobile: format + typecheck + lint + check:unused + 1604 tests pass
  • Notifications service: 161 tests pass
  • cloud-agent-next: 2343 tests pass
  • session-ingest: 573 tests pass
  • Web router: typecheck + 9274 tests pass

On-device E2E (iOS simulator, feature-state matrix)

Run on a worktree-owned iOS 26.5 simulator against the local dev stack, driven with Maestro + simctl, with the Postgres row inspected directly as ground truth.

  • Happy — persistence: PASS (end-to-end). From Profile → Notifications, the master gate renders ON and all 5 category toggles render and are interactive. Flipping Agent needs you wrote through to the DB (agent_attention_enabled false → true) and only that column changed (chat/session-status/kiloclaw untouched), confirming the per-key partial setNotificationPreferences update. The new value survived cold app relaunches and read back correctly (on-screen toggles matched the persisted user_notification_preferences row exactly: chat ON, agent-attention ON, agent-updates OFF, session-status ON, kiloclaw OFF).
  • Empty — verified by unit tests; not on-device. Reaching notificationsEnabled = false on-device requires revoking OS notification permission, which is not permitted via simctl on this runtime (returns Operation not permitted) and would otherwise need a destructive simulator erase + re-login. The empty state (all 5 switches disabled + the "Enable notifications" CTA) is covered by the mobile unit tests.
  • Happy — gating (push suppression/delivery): backend-verified; not reproducible as real on-device delivery locally. Per-category fail-closed suppression (incl. the attention-vs-status branch on category) is covered by the notifications-service tests and the cloud-agent-next integration test that asserts the category field on the push payload. Real end-to-end push delivery/tap-through cannot be exercised on the local stack (no EXPO_ACCESS_TOKEN; no deterministic producer trigger available without disrupting the shared dev stack), so it is reported here rather than self-accepted.
  • Unhappy, retryable: verified by unit tests; not reproduced on-device. The onError rollback + toast.error + reconcile-on-refetch path is covered by unit tests. Forcing a deterministic toggle-mutation failure on-device would require stopping the shared web/tRPC worker, which is unsafe on the shared dev stack, so it is reported rather than reproduced.
  • Unhappy, non-retryable: N/A (structural). A preference toggle has no terminal failure mode — every failure is a transient, retryable mutation.

Note: an earlier automated pass reported "category rows not visible"; this was a test-navigation artifact (the ambiguous Notifications selector matched the section header instead of the row) and not a product defect — the category rows render and persist correctly on-device.

@iscekic iscekic self-assigned this Jul 22, 2026
Comment thread apps/mobile/src/components/notifications-screen.tsx Outdated
Comment thread apps/mobile/src/components/notifications-screen.tsx
@kilo-code-bot

kilo-code-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

This increment reworks apps/mobile/src/components/notifications-screen.tsx to fix all 3 previously-flagged races (shared pending-category scalar, silent permission-error catch, and premature master-toggle re-enablement); no new issues found.

Files Reviewed (1 file)
  • apps/mobile/src/components/notifications-screen.tsx
Previous Review Summaries (5 snapshots, latest commit e9d825f)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit e9d825f)

Status: No Issues Found | Recommendation: Merge

Executive Summary

This increment only adds category: 'status' to expected push-notification payloads in a test file, matching already-implemented production behavior in the terminal push producer; no new logic, security, or behavioral issues found.

Files Reviewed (1 file)
  • services/cloud-agent-next/test/integration/session/push-notifications.test.ts

Previous review (commit 9cfaae2)

Status: No Issues Found | Recommendation: Merge

Executive Summary

This increment only reformats a multi-line object literal in a test call; no logic, assertion, or behavior change, so no new issues were found.

Files Reviewed (1 file)
  • services/session-ingest/src/remote-session-notifications.test.ts

Previous review (commit 4c90ad6)

Status: No Issues Found | Recommendation: Merge

Executive Summary

This increment only adds a pinned @types/pg devDependency and a missing createdOnPlatform test field, matching the stated typecheck fix; no new correctness or security issues found.

Files Reviewed (2 files)
  • apps/web/package.json
  • services/session-ingest/src/remote-session-notifications.test.ts

Previous review (commit fe5961d)

Status: 3 Issues Found | Recommendation: Address before merge

Executive Summary

The mobile Notifications screen has overlapping-mutation and busy-flag races (shared pendingCategory scalar cleared by the wrong mutation, and the master toggle becoming interactive mid in-flight enable) plus a silent catch that can hide native permission-request errors.

Overview

Severity Count
CRITICAL 0
WARNING 3
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/components/notifications-screen.tsx 304 Shared pendingCategory scalar can be cleared by the wrong mutation's onSettled during overlapping category toggles
apps/mobile/src/components/notifications-screen.tsx 337 Master toggle becomes re-interactive before the enable flow (permission request → token registration) finishes, allowing a re-entrant tap
apps/mobile/src/components/notifications-screen.tsx 350 Silent catch can swallow native requestPermissionsAsync errors with no toast feedback
Files Reviewed (32 files)
  • apps/mobile/src/app/(app)/(tabs)/(3_profile)/notifications.tsx
  • apps/mobile/src/components/notifications-card.tsx
  • apps/mobile/src/components/notifications-screen.tsx - 3 issues
  • apps/mobile/src/components/profile-screen.tsx
  • apps/mobile/src/lib/hooks/agent-push-preference.test.ts
  • apps/mobile/src/lib/hooks/agent-push-preference.ts
  • apps/web/src/routers/user-router.test.ts
  • apps/web/src/routers/user-router.ts
  • packages/db/src/migrations/0196_add_per_category_notification_preference_columns.sql
  • packages/db/src/migrations/meta/0196_snapshot.json
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/src/schema.ts
  • packages/notifications/src/push-data.ts
  • packages/notifications/src/rpc-schemas.ts
  • services/cloud-agent-next/src/session/message-settlement-outbox.test.ts
  • services/cloud-agent-next/src/session/message-settlement-outbox.ts
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.test.ts
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.ts
  • services/notifications/src/__tests__/send-push-for-conversation.test.ts
  • services/notifications/src/index.ts
  • services/notifications/src/lib/agent-session-notification-push.test.ts
  • services/notifications/src/lib/agent-session-notification-push.ts
  • services/notifications/src/lib/cloud-agent-session-push.ts
  • services/notifications/src/lib/instance-lifecycle-push.ts
  • services/notifications/src/lib/notifications-service-cloud-agent.test.ts
  • services/notifications/src/lib/notifications-service.test.ts
  • services/notifications/src/lib/scheduled-action-push.test.ts
  • services/notifications/src/lib/scheduled-action-push.ts
  • services/session-ingest/src/ingest/direct-ingest.test.ts
  • services/session-ingest/src/queue-consumer.test.ts
  • services/session-ingest/src/remote-session-notifications.test.ts
  • services/session-ingest/src/remote-session-notifications.ts

Fix these issues in Kilo Cloud

Previous review (commit fc9fec8)

Status: 2 Issues Found | Recommendation: Address before merge

Executive Summary

The mobile Notifications screen shares a single pendingCategory scalar across five independent per-category mutations, letting one mutation's onSettled clear another's busy state during overlapping in-flight toggles.

Overview

Severity Count
CRITICAL 0
WARNING 2
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
apps/mobile/src/components/notifications-screen.tsx 304 Shared pendingCategory scalar can be cleared by the wrong mutation's onSettled during overlapping category toggles
apps/mobile/src/components/notifications-screen.tsx 350 Silent catch can swallow native requestPermissionsAsync errors with no toast feedback
Files Reviewed (30 files)
  • apps/mobile/src/app/(app)/(tabs)/(3_profile)/notifications.tsx
  • apps/mobile/src/components/notifications-card.tsx
  • apps/mobile/src/components/notifications-screen.tsx - 2 issues
  • apps/mobile/src/components/profile-screen.tsx
  • apps/mobile/src/lib/hooks/agent-push-preference.test.ts
  • apps/mobile/src/lib/hooks/agent-push-preference.ts
  • apps/web/src/routers/user-router.test.ts
  • apps/web/src/routers/user-router.ts
  • packages/db/src/migrations/0196_add_per_category_notification_preference_columns.sql
  • packages/db/src/migrations/meta/0196_snapshot.json
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/src/schema.ts
  • packages/notifications/src/push-data.ts
  • packages/notifications/src/rpc-schemas.ts
  • services/cloud-agent-next/src/session/message-settlement-outbox.test.ts
  • services/cloud-agent-next/src/session/message-settlement-outbox.ts
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.test.ts
  • services/cloud-agent-next/src/websocket/ingest-attention-classifier.ts
  • services/notifications/src/__tests__/send-push-for-conversation.test.ts
  • services/notifications/src/index.ts
  • services/notifications/src/lib/agent-session-notification-push.test.ts
  • services/notifications/src/lib/agent-session-notification-push.ts
  • services/notifications/src/lib/cloud-agent-session-push.ts
  • services/notifications/src/lib/instance-lifecycle-push.ts
  • services/notifications/src/lib/notifications-service-cloud-agent.test.ts
  • services/notifications/src/lib/notifications-service.test.ts
  • services/notifications/src/lib/scheduled-action-push.test.ts
  • services/notifications/src/lib/scheduled-action-push.ts
  • services/session-ingest/src/ingest/direct-ingest.test.ts
  • services/session-ingest/src/queue-consumer.test.ts
  • services/session-ingest/src/remote-session-notifications.test.ts
  • services/session-ingest/src/remote-session-notifications.ts

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 20 · Output: 2.6K · Cached: 364.2K

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the feat/mobile-notification-categories branch from fc9fec8 to fe5961d Compare July 22, 2026 23:20
Comment thread apps/mobile/src/components/notifications-screen.tsx Outdated
iscekic added 4 commits July 23, 2026 01:33
…dOnPlatform

- Add @types/pg@8.18.0 to web devDependencies to resolve version conflict with
  @opentelemetry/instrumentation-pg which pulls in @types/pg@8.15.6
- Add missing createdOnPlatform: null to needs_input test params
… category field

The S2c change adds 'category' to cloud_agent_session push payload. Update
test expectations to include category: 'status' for:
- completed message pushes
- failed message pushes
- interrupted message pushes
- coalesced batch representative pushes
@iscekic
iscekic merged commit 46d887e into main Jul 23, 2026
68 checks passed
@iscekic
iscekic deleted the feat/mobile-notification-categories branch July 23, 2026 10:36
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.

2 participants