Skip to content

feat(spend-alerts): add spend alerts on web and mobile - #6291

Open
iscekic wants to merge 1 commit into
mainfrom
kwf/owner-spend-alerts-0876
Open

iscekic wants to merge 1 commit into
mainfrom
kwf/owner-spend-alerts-0876

Conversation

@iscekic

@iscekic iscekic commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Changelog for users

  • Owners can turn spend alerts on or off in their spend view, on web and in the mobile app.
  • In the spend view they choose alert kinds, limits, windows, and channels.
  • An alert fires when rolling spend crosses a configured limit, or when an hour runs far above the owner's usual rate.
  • Only the owner's authorized billing contacts receive the alert by email or mobile push, and only that owner's own spend figures appear in it.
  • Every alert carries one call to action that opens the owner's spend view.
  • A crossed condition alerts once, then stays quiet until it clears and crosses again.
  • Spend alerts are a mobile notification category beside the existing ones; turning it off stops push and matches the push choice in the spend view.
  • A retired Kilo-exclusive OpenAI alias is no longer offered as available to a signed-in ChatGPT connection.
  • Returning to the spend view refreshes the alert settings it shows.
  • The mobile Retry button is disabled while the form cannot be saved.
  • An edit made while a spend-alert save is in flight is kept when the save finishes.
  • Usage-analytics summary cards keep their height once data loads, so the content below them stays put.
  • A spend-alert push that fails for a recipient is retried.

Changelog for maintainers

  • Migration 0245 adds spend_alert_settings, spend_alert_rules, spend_alert_rule_state, spend_alert_hourly, and spend_alert_deliveries, and adds user_notification_preferences.spend_alerts_enabled (default true). Scope keys are user:<id> or org:<id>. A check constraint allows exactly one scope foreign key.
  • A five-minute Vercel cron re-derives the last three hours of per-scope hourly buckets from microdollar_usage in one idempotent SET upsert, then decides only the scopes that upsert touched plus rules already firing. An idle fleet costs one empty range scan and one empty claim, and a replayed window cannot double count.
  • The one-alert guarantee is spend_alert_rule_state.firing latching the crossing, a 95% hysteresis band for clearing, and a unique dedupe_key per scope, kind, channel, observed hour, and previous episode. Push adds an idempotency key on scope, kind, and threshold.
  • Delivery drains the outbox with FOR UPDATE SKIP LOCKED, leases rows by advancing next_attempt_at, fences completion on attempt_count, and retries transport failures with backoff. Senders are the new single-CTA email template and the existing notifications-worker push dispatch.
  • Push dispatches through a new spend_alert variant of the internal dispatch RPC; the device payload carries only scope and organization id, and each recipient is gated on their own spendAlerts category. A save that changes a rule's push channel writes the caller's category column in the same transaction; recipients are billing-role organization members or the owner, and account anonymization removes personal-scope settings, counters, and deliveries.
  • Review first the rollup/decision split in the sweep, the delivery claim fence, and the panel's reserved-height slot class. The web panel infers a saved settings row from non-null rule values, so a never-configured scope looks saved off; the spendAlerts router is registered for web and mobile, and usage-analytics cards always reserve their loaded height.
  • isOpenAiChatGptModel now also rejects a disabled Kilo-exclusive alias; the existing live-exclusive lookup ignores disabled entries, so retired aliases reached the served-model lookup, a delegated provider, and BYOK tagging. Regression tests cover public, hidden, and disabled statuses across eligibility, BYOK tagging, and token resolution.
  • The mobile passkey classifier test now covers the native reason arriving in name, message, or code and asserts the existing catalog key; the casing guard allowlists the passkey client's non-display identifier folding. No catalog, migration, or configuration change.
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx:121 — accepted. The flat ['spendAlerts'] prefix compared a string against the stored key's [['spendAlerts','get'], …] array, so the foreground refresh never matched. It now passes [[['spendAlerts']]], and a mounted test pins the argument.
  • packages/db/package.json:35 — accepted. The package had no test script, so the DB-less workspace matrix skipped its suites. Added a test script that runs jest; jest.config.mjs ignores the two database-backed suites; the migration-journal guard moved to its own DB-free suite. The package's typecheck and lint stand as the check and both passed.
  • apps/web/src/lib/notifications-worker-client.ts:63 — accepted. The worker answers 200 with per-recipient outcomes, including failed. Dispatch now parses that body, counts the failed recipients, captures them, and returns false so the outbox retries; an unparseable body keeps the accepted reading.
  • apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx:198 — accepted. The shared save callback replaced a draft edited while the request was open. It now captures the submitted draft and adopts the response only while the current draft is unchanged (draftAfterSave).
  • apps/web/src/components/usage-analytics/MetricCard.tsx:73 — accepted. A card without a subtext dropped the reserved row when loading finished, moving the summary row. The row now always renders with its reserved height.
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx:388 — accepted. Retry stayed tappable after an edit left the draft unsubmittable. It now carries the same disabled={!canSave} gate as Save.
  • Test-only: the @kilocode/db client suite now calls jest.mock on the global binding so it hoists above the imports and the pg mock applies; the push-data schema assertions moved to their own mobile suite; the casing guard's duplicate allowlist entry and an import order were cleaned up.
  • The @kilocode/db vitest check is excluded: this package has no vitest, so that check is skipped and pending owner verification. The package's typecheck and lint stand as its check and both passed.
  • The earlier e1 scenario narrative was garbled by a plan defect and is withdrawn. The focused apps/mobile vitest suites, the apps/web jest suites, and the package typechecks are the proof, and all four uploaded captures remain. The web setup smoke is advisory and did not pass, and no web must-run scenario covers this change.

E2E proof

e1-02-enabled

e1-05-category-off

e1-02-enabled

e1-04-anomaly-off

E2E proof — log excerpts


![[e1] web spend alerts panel (plan scenario text garbled: '</details> ```

@iscekic
iscekic marked this pull request as draft September 17, 2026 23:50
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
Comment thread apps/web/src/lib/spend-alerts/delivery.ts
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
Comment thread apps/web/src/lib/user/index.ts
Comment thread apps/mobile/src/components/organization/spend-alerts-screen.tsx
Comment thread apps/mobile/src/components/organization/spend-alert-validators.ts Outdated
Comment thread apps/web/src/components/usage-analytics/BreakdownBarChart.tsx Outdated
Comment thread apps/web/src/lib/spend-alerts/sweep.ts
@kilo-code-bot

kilo-code-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change since the previously reviewed commit is test-only — new passkey-classifier cases (name/message/code) and a reworded casing-guard allowlist entry — and both match the current implementation; every previously accepted review fix was also re-verified present at current HEAD.

Files Reviewed (23 files)
  • apps/mobile/src/lib/auth/passkey-client.test.ts - new native-reason cases verified against classifyPasskeyError and existing catalog keys
  • apps/mobile/src/lib/case-guard.test.ts - allowlist description reword only
  • Targeted verification of previously accepted fixes at HEAD: apps/mobile/src/components/organization/spend-alerts-screen.tsx (nested foreground-refresh key, hidden-field canSubmit, gated Retry, kind-qualified a11y labels), apps/mobile/src/components/organization/spend-alert-validators.ts (microdollar floor), apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx + spendAlertsPanelState.ts (draftAfterSave), apps/web/src/components/usage-analytics/MetricCard.tsx (always-rendered reserved row), apps/web/src/components/usage-analytics/BreakdownBarChart.tsx (420px reserved body), apps/web/src/lib/spend-alerts/delivery.ts (per-row lease, undeliverable terminal errors, claim ordering, narrowed reschedule), apps/web/src/lib/spend-alerts/sweep.ts (bounded keyset read), apps/web/src/lib/spend-alerts/settings.ts + apps/web/src/routers/spend-alert-router.ts (server-side threshold floor), apps/web/src/lib/notifications-worker-client.ts (per-recipient failure parsing), apps/web/src/lib/user/index.ts (org-scope recipient scrubbing on anonymization), packages/db/package.json (test script), packages/db/src/migrations/meta/_journal.json (0254_charming_iron_patriot newest entry), services/notifications/src/lib/internal-dispatch-push.ts (episode-scoped push idempotency key)
  • Locale deltas between the last reviewed commit and HEAD are main-branch updates (glanceable/live-activity keys), not spend-alert copy
Previous Review Summaries (13 snapshots)

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

Previous review

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commit resolves all six previously reported findings and adds regression tests; no new issues were found in the changed code.

Files Reviewed (18 files)
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx - foreground-refresh key now uses the tRPC-nested form; Retry now gated on canSave
  • apps/mobile/src/components/organization/spend-alerts-screen.mounted.test.tsx - new tests
  • apps/mobile/src/lib/case-guard.test.ts - duplicate allowlist key removed
  • apps/mobile/src/lib/hooks/agent-push-preference.test.ts - import order
  • apps/mobile/src/lib/notification-path-schema.test.ts - moved schema tests
  • apps/mobile/src/lib/notification-path.test.ts - moved schema tests
  • apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx - per-call save response adoption
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.ts - new draftAfterSave
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.test.ts - new tests
  • apps/web/src/components/usage-analytics/MetricCard.tsx - subtext row always rendered
  • apps/web/src/components/usage-analytics/MetricCard.test.ts - new tests
  • apps/web/src/lib/notifications-worker-client.ts - per-recipient failure now reported as refused dispatch
  • apps/web/src/lib/notifications-worker-client.test.ts - new tests
  • packages/db/package.json - test script added
  • packages/db/jest.config.mjs - DB-backed suites excluded from DB-less CI
  • packages/db/src/client.test.ts - pg mock hoisting fix
  • packages/db/src/migration-journal.test.ts - journal guard extracted to a DB-less suite
  • packages/db/src/schema.test.ts - journal guard moved out

Previous review (commit cf51735)

Status: 6 Issues Found | Recommendation: Address before merge

Executive Summary

The spend-alert feature is functionally sound after the earlier fixes, but three new gaps remain: the mobile spend screen never refreshes in the foreground, the packages/db tests this PR adds are excluded from CI, and a per-recipient push failure is still recorded as delivered because the worker always returns HTTP 200.

Overview

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

WARNING

File Line Issue
apps/mobile/src/components/organization/spend-alerts-screen.tsx 121 useRouteForegroundRefresh([['spendAlerts']]) uses the flat key form, which never matches a tRPC-shaped key; focus/foreground refresh is a silent no-op.
packages/db/package.json 35 jest/@swc/jest and jest.config.mjs are added with no test script, so changed-workspaces.sh excludes @kilocode/db and the new schema/migration-journal tests never run in CI.
apps/web/src/lib/notifications-worker-client.ts 63 /internal/v1/dispatch returns 200 with per-recipient failed outcomes, so dispatchSpendAlertPush reports success and the outbox marks a lost spend-alert push sent with no retry.

SUGGESTION

File Line Issue
apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx 198 saveMutation.reset() does not cancel the in-flight mutation, so edits made during a save can be overwritten by the response's setDraft, and a second concurrent save is allowed.
apps/web/src/components/usage-analytics/MetricCard.tsx 73 The reserved subtext row disappears once loading finishes for cards without subtext, shifting the summary grid by 20px instead of preventing movement.
apps/mobile/src/components/organization/spend-alerts-screen.tsx 388 Retry has no canSave gate; with an invalid draft after a failure it stays tappable and silently no-ops.
Files Reviewed (22 code files plus generated/locale assets)
  • apps/web/src/lib/spend-alerts/delivery.ts
  • apps/web/src/lib/spend-alerts/sweep.ts
  • apps/web/src/lib/spend-alerts/settings.ts
  • apps/web/src/routers/spend-alert-router.ts
  • apps/web/src/lib/email.ts
  • apps/web/src/lib/notifications-worker-client.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/src/app/api/cron/dispatch-spend-alerts/route.ts
  • apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.ts
  • apps/web/src/components/usage-analytics/{BreakdownBarChart,MetricCard,UsageAnalyticsDashboard}.tsx
  • apps/web/src/emails/spendAlert.html
  • packages/notifications/src/{rpc-schemas,push-data,push-presentation}.ts
  • services/notifications/src/{index.ts,lib/internal-dispatch-push.ts,lib/cloud-agent-session-push.ts}
  • packages/db/src/{schema.ts,spend-alerts-schema.test.ts,migrations/0254_charming_iron_patriot.sql,migrations/meta/_journal.json}
  • apps/mobile/src/components/organization/{spend-alerts-screen.tsx,spend-alert-validators.ts}
  • apps/mobile/src/{components/notifications-screen.tsx,lib/hooks/agent-push-preference.ts,lib/notification-path.ts}
  • apps/web/src/lib/ai-gateway/openai-chatgpt/routing.ts
  • apps/web/src/routers/{user-router.ts,root-router.ts,admin/email-testing-router.ts}
  • services/kilo-mcp/catalog.json (generated)

Fix these issues in Kilo Cloud

Previous review (commit 0779b1c)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental commits (migration renumber and timestamp, permanent-delivery error handling, threshold lower bound, claim ordering, push idempotency episode key, and distinct mobile switch labels) are correct at HEAD and introduce no new issues in changed code.

Files Reviewed (18 files)
  • apps/web/src/lib/spend-alerts/delivery.ts
  • apps/web/src/lib/spend-alerts/settings.ts
  • apps/web/src/routers/spend-alert-router.ts
  • packages/notifications/src/rpc-schemas.ts
  • services/notifications/src/lib/internal-dispatch-push.ts
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx
  • packages/db/src/schema.test.ts
  • packages/db/src/migrations/0254_charming_iron_patriot.sql (renamed from 0245_friendly_johnny_blaze.sql)
  • packages/db/src/migrations/meta/_journal.json (generated)
  • packages/db/src/migrations/meta/0254_snapshot.json (generated)
  • services/kilo-mcp/catalog.json (generated)

Previous review (commit 40629ab)

Status: 6 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental change since 94a15cc (the ChatGPT routing test now stubs a synthetic Kilo-exclusive alias instead of a real, since-disabled one, and adds an ordinary-model tagging regression case) introduces no new issues; the six findings below remain unresolved at HEAD, led by the stale migration journal when that can skip the spend-alerts schema migration on existing databases.

Overview

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

CRITICAL

File Line Issue
packages/db/src/migrations/meta/_journal.json 1786 New entry's when predates the previous entry, so drizzle skips the migration on any DB that applied idx 253

WARNING

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 358 Permanent row-shape errors are plain Error, so they are rescheduled forever
apps/web/src/routers/spend-alert-router.ts 34 Server threshold schema has no lower bound; sub-microdollar values round to a zero threshold that never clears

SUGGESTION

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 185 Claim ORDER BY does not match the pending index
services/notifications/src/lib/internal-dispatch-push.ts 92 Push idempotency key omits the firing episode, dropping a re-cross push within 1h
apps/mobile/src/components/organization/spend-alerts-screen.tsx 593 Both rule cards use identical channel-switch accessibility labels
Files Reviewed (1 file)
  • apps/web/src/lib/ai-gateway/openai-chatgpt/routing.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 94a15cc)

Status: 6 Issues Found | Recommendation: Address before merge

Executive Summary

The incremental change since b5b17aa (a retired Kilo-exclusive alias guard in ChatGPT routing plus passkey and case-guard tests) introduces no new issues; the six findings below remain unresolved at HEAD, led by the stale migration journal when that can skip the spend-alerts schema migration on existing databases.

Overview

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

CRITICAL

File Line Issue
packages/db/src/migrations/meta/_journal.json 1786 New entry's when predates the previous entry, so drizzle skips the migration on any DB that applied idx 253

WARNING

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 358 Permanent row-shape errors are plain Error, so they are rescheduled forever
apps/web/src/routers/spend-alert-router.ts 34 Server threshold schema has no lower bound; sub-microdollar values round to a zero threshold that never clears

SUGGESTION

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 185 Claim ORDER BY does not match the pending index
services/notifications/src/lib/internal-dispatch-push.ts 92 Push idempotency key omits the firing episode, dropping a re-cross push within 1h
apps/mobile/src/components/organization/spend-alerts-screen.tsx 593 Both rule cards use identical channel-switch accessibility labels
Files Reviewed (4 files)
  • apps/web/src/lib/ai-gateway/openai-chatgpt/routing.ts
  • apps/web/src/lib/ai-gateway/openai-chatgpt/routing.test.ts
  • apps/mobile/src/lib/auth/passkey-client.test.ts
  • apps/mobile/src/lib/case-guard.test.ts

Fix these issues in Kilo Cloud

Previous review (commit b5b17aa)

Status: 6 Issues Found | Recommendation: Address before merge

Executive Summary

A stale migration when in the journal can cause the entire spend-alerts schema migration to be skipped on existing databases; the rest are server-side validation and delivery/bookkeeping gaps.

Overview

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

CRITICAL

File Line Issue
packages/db/src/migrations/meta/_journal.json 1786 New entry's when predates the previous entry, so drizzle skips the migration on any DB that applied idx 253

WARNING

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 358 Permanent row-shape errors are plain Error, so they are rescheduled forever
apps/web/src/routers/spend-alert-router.ts 34 Server threshold schema has no lower bound; sub-microdollar values round to a zero threshold that never clears

SUGGESTION

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 185 Claim ORDER BY does not match the pending index
services/notifications/src/lib/internal-dispatch-push.ts 92 Push idempotency key omits the firing episode, dropping a re-cross push within 1h
apps/mobile/src/components/organization/spend-alerts-screen.tsx 593 Both rule cards use identical channel-switch accessibility labels
Files Reviewed (35 files)
  • packages/db/src/schema.ts
  • packages/db/src/migrations/0245_friendly_johnny_blaze.sql
  • packages/db/src/migrations/meta/_journal.json
  • packages/db/jest.config.mjs
  • packages/db/package.json
  • packages/notifications/src/rpc-schemas.ts
  • packages/notifications/src/push-data.ts
  • packages/notifications/src/push-presentation.ts
  • packages/trpc/src/mobile.ts
  • services/notifications/src/lib/internal-dispatch-push.ts
  • services/notifications/src/index.ts
  • services/kilo-mcp/catalog.json
  • apps/web/src/lib/spend-alerts/delivery.ts
  • apps/web/src/lib/spend-alerts/sweep.ts
  • apps/web/src/lib/spend-alerts/settings.ts
  • apps/web/src/routers/spend-alert-router.ts
  • apps/web/src/routers/user-router.ts
  • apps/web/src/routers/root-router.ts
  • apps/web/src/routers/admin/email-testing-router.ts
  • apps/web/src/app/api/cron/dispatch-spend-alerts/route.ts
  • apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.ts
  • apps/web/src/components/usage-analytics/BreakdownBarChart.tsx
  • apps/web/src/components/usage-analytics/MetricCard.tsx
  • apps/web/src/components/usage-analytics/UsageAnalyticsDashboard.tsx
  • apps/web/src/lib/email.ts
  • apps/web/src/lib/notifications-worker-client.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/vercel.json
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx
  • apps/mobile/src/components/organization/spend-alert-validators.ts
  • apps/mobile/src/components/notifications-screen.tsx
  • apps/mobile/src/components/preferences-screen.tsx
  • apps/mobile/src/lib/notification-path.ts
  • apps/mobile/src/lib/hooks/agent-push-preference.ts

Fix these issues in Kilo Cloud

Previous review (commit 0ba1cc1)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only change since the previous review at e5a836eef is regenerated migration metadata from the rebase onto current main (the 0253 snapshot replacing 0251, and the _journal.json index moving to 253); the spend-alerts source is byte-identical, so there are no new changed-code findings.

Files Reviewed (3 files)
  • packages/db/src/migrations/0245_friendly_johnny_blaze.sql - unchanged by the rebase
  • packages/db/src/migrations/meta/0253_snapshot.json - regenerated snapshot, renamed from 0251, prevId chain intact
  • packages/db/src/migrations/meta/_journal.json - feature migration re-appended at idx 253

Previous review (commit e5a836e)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The branch was rebased from 0d3763ca2 to e5a836eef; the only incremental change is regenerated migration metadata, so there are no new source-code findings in the changed lines.

Files Reviewed (3 files)
  • packages/db/src/migrations/0245_friendly_johnny_blaze.sql - feature migration, unchanged by the rebase
  • packages/db/src/migrations/meta/0251_snapshot.json - regenerated snapshot, renamed from 0250 with the prevId chain intact
  • packages/db/src/migrations/meta/_journal.json - feature migration re-appended at idx 251

Previous review (commit 0d3763c)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The branch head 0d3763ca2 has the same tree as the previously reviewed commit d728964a4 (force-push/squash with no content change), so the incremental diff is empty and there are no new findings; the prior inline findings remain addressed in the current code.

Files Reviewed (0 changed files)
  • No files changed in the incremental diff (d728964a4..0d3763ca2 is empty). Spot-checked the previously flagged delivery claim/lease and sweep keyset-read fixes, which are present at current HEAD.

Previous review (commit d728964)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The only change since the last review is a CI refresh of the generated MCP catalog that adds the spend-alert query/mutation entries; no new issues were found, and all prior findings remain addressed at current HEAD.

Files Reviewed (1 file)
  • services/kilo-mcp/catalog.json (generated)

Previous review (commit f2380fc)

Status: No Issues Found | Recommendation: Merge

Executive Summary

The incremental change folds recipient narrowing into the reschedule write with bounded retries and a terminal fallback, resolving the previously reported duplicate-send risk; no new issues were found in the two changed files.

Files Reviewed (2 files)
  • apps/web/src/lib/spend-alerts/delivery.ts
  • apps/web/src/lib/spend-alerts/delivery.test.ts

Previous review (commit 48fd15f)

Status: 1 Issues Found | Recommendation: Address before merge

Overview

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

SUGGESTION

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 476 A failed recipient-narrowing write can re-send the alert to recipients this attempt already reached
Files Reviewed (17 files)
  • apps/web/src/lib/spend-alerts/delivery.ts - 1 issue
  • apps/web/src/lib/spend-alerts/delivery.test.ts
  • apps/web/src/lib/spend-alerts/sweep.ts
  • apps/web/src/lib/spend-alerts/sweep.test.ts
  • apps/web/src/lib/user/index.ts
  • apps/web/src/lib/user/index.test.ts
  • apps/web/src/lib/email.ts
  • apps/web/src/lib/email.test.ts
  • apps/web/src/lib/notifications-worker-client.ts
  • apps/web/src/lib/notifications-worker-client.test.ts
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.ts
  • apps/web/src/components/spend-alerts/spendAlertsPanelState.test.ts
  • apps/web/src/components/usage-analytics/BreakdownBarChart.tsx
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx
  • apps/mobile/src/components/organization/spend-alerts-screen.mounted.test.tsx
  • apps/mobile/src/components/organization/spend-alert-validators.ts
  • apps/mobile/src/components/organization/spend-alert-validators.test.ts

Fix these issues in Kilo Cloud

Previous review (commit 7d9ad8b)

Status: 9 Issues Found | Recommendation: Address before merge

Executive Summary

The highest-risk issue is in the spend-alert delivery drain: failed push and email sends resolve successfully, so outbox rows are marked sent and the retry/backoff path never runs; a batch-level claim lease also lets overlapping crons re-send the same alert.

Fix these issues in Kilo Cloud

Overview

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

WARNING

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 308 Failed pushes/emails resolve normally, so the row is marked sent and never retried
apps/web/src/lib/spend-alerts/delivery.ts 171 5-minute lease covers up to 50 sequentially-sent rows; overlapping crons can re-claim and duplicate
apps/web/src/lib/user/index.ts 1553 Anonymization leaves org-scope delivery recipients (deleted user's email) in place
apps/mobile/src/components/organization/spend-alerts-screen.tsx 513 Master switch off with an invalid hidden rule field leaves Save permanently disabled

SUGGESTION

File Line Issue
apps/web/src/lib/spend-alerts/delivery.ts 312 A failed mark after a successful send re-queues the row (duplicate send)
apps/web/src/lib/spend-alerts/delivery.ts 256 Empty recipient set is marked delivered, silently dropping the alert
apps/web/src/lib/spend-alerts/sweep.ts 601 Firing-scope read is unbounded despite the bounded-read claim
apps/mobile/src/components/organization/spend-alert-validators.ts 37 Sub-cent limits round to a zero threshold and never clear
apps/web/src/components/usage-analytics/BreakdownBarChart.tsx 43 Fixed 180px body compresses tall breakdowns (previously up to 420px)
Files Reviewed (68 non-generated files)
  • apps/web/src/lib/spend-alerts/settings.ts - clean
  • apps/web/src/lib/spend-alerts/sweep.ts - 1 suggestion
  • apps/web/src/lib/spend-alerts/delivery.ts - 4 issues
  • apps/web/src/app/api/cron/dispatch-spend-alerts/route.ts - clean
  • apps/web/src/routers/spend-alert-router.ts - clean (scope-bound authz verified)
  • apps/web/src/routers/root-router.ts, user-router.ts, admin/email-testing-router.ts - clean
  • apps/web/src/lib/email.ts, notifications-worker-client.ts - 1 issue folded into delivery
  • apps/web/src/lib/user/index.ts - 1 warning
  • apps/web/src/emails/spendAlert.html, AGENTS.md - clean (values escaped, no <img> in markdown)
  • apps/web/src/components/spend-alerts/* - clean
  • apps/web/src/components/usage-analytics/* - 1 suggestion
  • packages/db/src/schema.ts, migrations/0245_friendly_johnny_blaze.sql - clean (new tables; only additive ADD COLUMN ... DEFAULT true NOT NULL on a populated table)
  • packages/db/src/spend-alerts-schema.test.ts, jest.config.mjs, package.json - clean
  • packages/notifications/src/{push-data,push-presentation,rpc-schemas}.ts - clean
  • packages/trpc/src/mobile.ts - clean
  • services/notifications/src/index.ts, lib/{internal-dispatch-push,cloud-agent-session-push}.ts - clean
  • apps/mobile/src/components/organization/spend-alerts-screen.tsx, spend-alert-validators.ts, hub-screen.tsx - 2 issues
  • apps/mobile/src/components/notifications-screen.tsx, preferences-screen.tsx, lib/notification-path.ts, lib/hooks/agent-push-preference.ts - clean
  • apps/web/vercel.json - clean (schedule/path match route)
  • Test files and i18n locale catalogs reviewed for consistency

Reviewed by efficient · Input: 0 · Output: 0 · Cached: 0

Review guidance: REVIEW.md from base branch main

@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from a9ce548 to 587eae8 Compare September 18, 2026 02:43
@iscekic

iscekic commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator Author

bot: Rejected, no code change (kwf kwf-fix-platform-b63d).

Why: (already implemented, verified live: no change needed: The three cited lines import cross-platform expo-router/expo-haptics; the PR patch contains no Platform.OS/select branch and one SpendAlertsScreen serves iOS and Android, so the platform fork the finding claims does not exist and no code was changed. No change: the finding is refut

@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 1761fac to 48fd15f Compare September 18, 2026 03:34
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 03:55
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
@iscekic
iscekic marked this pull request as draft September 18, 2026 04:08
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 48fd15f to f2380fc Compare September 18, 2026 04:58
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 05:38
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from d728964 to 0d3763c Compare September 18, 2026 17:33
@iscekic
iscekic marked this pull request as draft September 18, 2026 19:12
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 0d3763c to e5a836e Compare September 18, 2026 19:52
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 20:17
@iscekic
iscekic marked this pull request as draft September 18, 2026 21:18
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from e5a836e to 0ba1cc1 Compare September 18, 2026 22:36
@iscekic
iscekic marked this pull request as ready for review September 18, 2026 22:57
@iscekic iscekic added the human-ready The PR is ready for human review. label Sep 18, 2026
@iscekic iscekic self-assigned this Sep 18, 2026
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 77a59c3 to b5b17aa Compare September 19, 2026 13:38
Comment thread packages/db/src/migrations/meta/_journal.json Outdated
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
Comment thread apps/web/src/lib/spend-alerts/delivery.ts Outdated
Comment thread services/notifications/src/lib/internal-dispatch-push.ts Outdated
Comment thread apps/web/src/routers/spend-alert-router.ts Outdated
Comment thread apps/mobile/src/components/organization/spend-alerts-screen.tsx Outdated
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from b5b17aa to 94a15cc Compare September 19, 2026 15:44
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 006f641 to 40629ab Compare September 19, 2026 17:00
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 0779b1c to cf51735 Compare September 19, 2026 23:20
Comment thread apps/mobile/src/components/organization/spend-alerts-screen.tsx Outdated
Comment thread packages/db/package.json
Comment thread apps/web/src/lib/notifications-worker-client.ts
Comment thread apps/web/src/components/spend-alerts/SpendAlertsPanel.tsx
Comment thread apps/web/src/components/usage-analytics/MetricCard.tsx Outdated
Comment thread apps/mobile/src/components/organization/spend-alerts-screen.tsx
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from cf51735 to 22f1231 Compare September 20, 2026 01:43
@iscekic
iscekic force-pushed the kwf/owner-spend-alerts-0876 branch from 22f1231 to 89d37f5 Compare September 20, 2026 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

human-ready The PR is ready for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant