Push notifications for low-balance and security-finding alerts

[iscekic]

Summary

Two alerts that today are email-only now also dispatch mobile push notifications, reusing the existing per-user Durable Object push pipeline (idempotency, presence, rate limiting) and the per-category preference model:

1. Organization low-balance alert — when an org's balance crosses below settings.minimum_balance and the alert email is scheduled, every configured recipient email that maps to a Kilo user who is an active member of the alerting org (and has the category enabled) receives exactly one push per crossing. Tapping lands on that org's credit-activity screen with the org reconciled from the deep link.
2. Security-agent finding notifications — when a security_finding_notifications row's email is sent, the recipient also receives a push (new finding / SLA warning / SLA breach copy). Tapping opens the finding detail screen.

Changes

• packages/db: two new user_notification_preferences columns (balance_alerts_enabled, security_findings_enabled, both default ON like every category) + generated migration. Email delivery is never affected by these toggles.
• packages/notifications: two new pushDataSchema members (low_balance, security_finding) + internalDispatchRequestSchema for the internal endpoint.
• services/notifications: new POST /internal/v1/dispatch endpoint (X-Internal-Secret auth with timing-safe compare against the INTERNAL_API_SECRET secrets-store binding, config-only addition). The worker owns copy/payload construction and gates each recipient on the new preference categories before the DO call (fail-closed), then dispatches through NotificationChannelDO.dispatchPush (presenceContext: null, badge: null). Idempotency keys: low-balance:<orgId> (a second crossing of the same org within the DO's 1h TTL is intentionally push-suppressed as anti-spam; email unaffected), security-finding:<notificationId> (covers sweep retry/crash windows).
• apps/web: fire-safe notifications-worker-client.ts (never throws into the email path; log-and-skip when unconfigured). Low-balance push shares the email schedule predicate; recipients are resolved email→user and filtered to active members of the alerting org (the tap target requires membership; unmatched/non-member emails stay email-only). Security-finding push dispatches only after sendEmail succeeded and can never change the route's response (no email double-send via sweep retries). tRPC getNotificationPreferences/setNotificationPreferences expose balanceAlerts/securityFindings.
• apps/mobile: both payload types route via notificationPathForData; credit-activity screen honors a ?org= deep-link param with an explicit reconcile algorithm (param-only sourcing, no query keyed on the pre-tap org, persist selection only for validated memberships after SecureStore hydration, foreign/stale param → "Organization unavailable" without mutating the selection); two new preference toggle rows on the Notifications screen.

iOS App Store rule intact: push copy is informational only, no purchase CTA; no landing-screen changes that add external-purchase CTAs.

Deploy-time env addition (action required)

NOTIFICATIONS_WORKER_URL must be added to the web app's prod and preview env (wherever SESSION_INGEST_WORKER_URL is managed):

• prod: https://notifications.kiloapps.io
• local dev: generated automatically by pnpm dev:env (@url notifications)

Missing URL is safe by design: dispatch logs and skips, the email path is unaffected.

Test plan

• Worker unit tests: category gating, per-kind copy/payload, idempotency keys, fail-closed preference reads (internal-dispatch-push.test.ts); pnpm test in services/notifications — 173/173.
• Web tests: recipient email→user resolution incl. unmatched/non-member, org-name miss, zero-recipient short-circuit, push scheduled iff the email predicate holds; security route dispatches only after email success and swallows push errors; preference keys round-trip (organization-usage.test.ts, route.test.ts, user-router.test.ts) — 111/111.
• Mobile tests: path mappings, deep-link reconcile branches incl. foreign-org → boundary, category keys (org-deep-link.test.ts, notification-path.test.ts, agent-push-preference.test.ts) — 1885/1885.
• Live-DB schema↔migration consistency test passes.
• On-device E2E (iOS simulator, local stack, push sink): low-balance happy path via real org-billed LLM call, opt-out via toggle on a second org, security-finding dispatch + DO dedup on repeated POST, and cold/warm push-tap routing incl. the foreign-org boundary — results tracked below.

[iscekic]

(bot) E2E evidence (iOS simulator, local stack, push sink):

Low-balance happy path (full product path: org-billed kilo-auto/efficient call → gateway → schedule → worker): org balance 10.00 == threshold, one message → microdollars_used 0→4883, exactly one agent_push_sink_payload with idempotencyKey low-balance:, dataType low_balance. Opt-out on second org: balanceAlerts=false → usage crossed (4872) with zero sink lines. Security finding: POST → sent + one security-finding: sink line; repeat POST → sent, no second line (DO idempotency).

Push tap routing (simctl push, Expo body wrapper): warm tap on low_balance → Org A credit-activity with selection persisted; foreign org id → Organization unavailable with selection untouched; security_finding → finding detail; both new toggle rows render and persist.

Cold-start tap (app terminated → Notification Center cell activation) is tooling-blocked on this host (Maestro cannot activate SpringBoard NC cells for a terminated app; two documented rounds). Compensating evidence: delivery without tap correctly does NOT navigate (icon launch → Home); the cold mechanism (getLastNotificationResponse → pending link) shares parseNotificationData/notificationPathForData with the warm path; reconcile/path logic is unit-tested.

[iscekic]

(bot) Standin review of head b6bc8ea2c84805da3a9bd27f46b32c11efad2055. The automated Kilo Code Review status check on this PR runs green but the automated reviewer posts no findings — it never comments — so the requester abandoned it. This review was produced instead by a dedicated review agent over the full git diff origin/main...HEAD and is the review of record for this head. It is not a Kilobot review.

Findings

High

1. Security-finding route awaits push dispatch, exceeding the sweep's 10s timeout → duplicate user-facing emails — apps/web/src/app/api/internal/security-agent/notifications/route.ts:285-300

The route awaits dispatchSecurityFindingPush before returning sent. The push client waits up to 30s (AbortSignal.timeout(30_000), apps/web/src/lib/notifications-worker-client.ts:39), but the only caller — the security-sync sweep — aborts at 10s (DISPATCH_TIMEOUT_MS, services/security-sync/src/notifications/sweep.ts:37,877). When the notifications worker is slow or unreachable, the email has already been sent but the sweep records endpoint_timeout and reschedules; the retry re-enters the route and re-sends the email. The push is deduped by the security-finding:<notificationId> idempotency key; the email is not — contradicting the PR's stated "no email double-send via sweep retries". Required outcome: decouple push dispatch from the route response (e.g. schedule with after() as organization-usage.ts already does, preserving dispatch-only-after-sent ordering) so response latency stays within the caller's budget.

Low

2. No route-level coverage for the new POST /internal/v1/dispatch endpoint or its auth — services/notifications/src/index.ts:117-164. hasValidInternalSecret (timing-safe compare, fail-closed) and the 401/400 branches are new security-sensitive code with no test; route-level precedent exists (src/__tests__/routes-badges.test.ts). Required outcome: coverage for missing/wrong-secret 401, malformed-body 400, and a valid dispatch reaching the core.

3. Security-finding test fixture uses a non-producer-realistic scope — services/notifications/src/lib/internal-dispatch-push.test.ts:44 (assertion at :227). Fixture sends org:org-1; the real producer sends a bare org id or 'personal' (route.ts:291), and scope flows verbatim into push.data.scope and mobile deep-link construction, so the tests would not catch producer/mobile scope-format drift. Required outcome: producer-realistic fixture values.

4. Stale eslint-disable justification — apps/mobile/src/components/notifications-screen.tsx:1-4: comment says "5 per-category toggles"; the screen now renders 7.

5. Unrelated lockfile churn — pnpm-lock.yaml second hunk flips @types/node@25.5.2 → @types/node@24.12.4 in the @cloudflare/vitest-pool-workers snapshot, unrelated to this feature. Required outcome: revert that hunk (the @kilocode/notifications workspace-link hunk stays).

Repairs for all five are being dispatched now; a follow-up comment will record what changed, and a fresh standin review will cover the new head.

[iscekic]

(bot) Repair pushed: head is now b2253fdf9 (commit fix(mobile-push): address PR #4771 standin-review findings). All five findings from the standin review above were validated and repaired; a fresh independent reviewer verified each repair, and its two follow-up coverage gaps in the new test file were also fixed.

What changed

1. High — duplicate-email hazard: route.ts no longer awaits dispatchSecurityFindingPush. Dispatch is scheduled via after() from next/server (same pattern as organization-usage.ts), only when result.sent, with errors swallowed inside the callback. The route response ({ outcome: 'sent' }) no longer waits on push, so the security-sync sweep's 10s budget cannot be blown by push latency — no endpoint_timeout reschedule, no re-sent email. route.test.ts now mocks after with drainable callbacks and asserts scheduling on send, non-scheduling on failure/cancel, and error swallow.
2. Low — dispatch endpoint coverage: new services/notifications/src/__tests__/routes-dispatch.test.ts (7 tests) covers missing secret → 401, empty secret-binding → 401 (fail-closed), wrong secret at equal length (value compare) and different length (self-compare) → 401, malformed JSON → 400, schema-invalid body → 400, and a valid dispatch reaching the core with parsed input.
3. Low — fixture realism: security-finding fixture scope is now producer-realistic ('org-1').
4. Low — stale comment: notifications-screen.tsx eslint-disable justification now says 7 per-category toggles.
5. Low — lockfile: the unrelated @types/node flip in pnpm-lock.yaml is reverted; only the @kilocode/notifications workspace-link hunk remains.

Checks on the repair: worker dispatch suites 7/7 and 12/12, web security-agent route tests 14/14, typecheck/lint/format clean in all touched packages, git diff --check clean. The full services/notifications suite shows 25 pre-existing local-only failures in dispatch-push.test.ts caused by this dev worktree's gitignored .dev.vars (PUSH_SINK_MODE=log); the affected files were not touched by the repair and CI (no .dev.vars) was green on the same suite at the previous head.

A fresh standin review of the new head b2253fdf9 follows.

[iscekic]

(bot) E2E rerun evidence for the repair head b2253fdf9 (local stack for this worktree: nextjs + notifications worker + event-service, push sink in PUSH_SINK_MODE=log; no simulator needed — the repair is server-side only). Seeded a user with a push token, an enabled security_scan agent config, an open critical finding, and security_finding_notifications rows in sending:

1. Worker-down latency (the exact high-finding scenario): SIGSTOPped this worktree's notifications workerd to simulate an unresponsive worker, then POSTed the security-agent notifications route. Response: {"outcome":"sent"} in 93ms — the push dispatch no longer extends route latency (pre-fix this hung up to the client's 30s timeout, blowing the sweep's 10s budget and re-sending the email on retry). Email was sent; after resuming the worker, the deferred after() dispatch delivered (POST /internal/v1/dispatch 200 OK in the worker pane).
2. Full dispatch chain: fresh notification POST → sent → worker → preference gate → DO → exactly one sink payload: idempotencyKey: 'security-finding:<notificationId>', dataType: 'security_finding', dataKeys: ['findingId','scope','type'], priority: 'high'.
3. DO dedup: repeat POST → sent again, still exactly one sink line for the notification (idempotency key suppressed the duplicate).

Stack stopped, fixtures deleted, device slot released.