Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryThe 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)
Previous Review Summaries (13 snapshots)Current summary above is authoritative. Previous snapshots are kept for context only. Previous reviewStatus: No Issues Found | Recommendation: Merge Executive SummaryThe 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)
Previous review (commit cf51735)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe 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 Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (22 code files plus generated/locale assets)
Fix these issues in Kilo Cloud Previous review (commit 0779b1c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe 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)
Previous review (commit 40629ab)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe incremental change since Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (1 file)
Fix these issues in Kilo Cloud Previous review (commit 94a15cc)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryThe incremental change since Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit b5b17aa)Status: 6 Issues Found | Recommendation: Address before merge Executive SummaryA stale migration Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Files Reviewed (35 files)
Fix these issues in Kilo Cloud Previous review (commit 0ba1cc1)Status: No Issues Found | Recommendation: Merge Executive SummaryThe only change since the previous review at Files Reviewed (3 files)
Previous review (commit e5a836e)Status: No Issues Found | Recommendation: Merge Executive SummaryThe branch was rebased from Files Reviewed (3 files)
Previous review (commit 0d3763c)Status: No Issues Found | Recommendation: Merge Executive SummaryThe branch head Files Reviewed (0 changed files)
Previous review (commit d728964)Status: No Issues Found | Recommendation: Merge Executive SummaryThe 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)
Previous review (commit f2380fc)Status: No Issues Found | Recommendation: Merge Executive SummaryThe 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)
Previous review (commit 48fd15f)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (17 files)
Fix these issues in Kilo Cloud Previous review (commit 7d9ad8b)Status: 9 Issues Found | Recommendation: Address before merge Executive SummaryThe highest-risk issue is in the spend-alert delivery drain: failed push and email sends resolve successfully, so outbox rows are marked Fix these issues in Kilo Cloud Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (68 non-generated files)
Reviewed by efficient · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
a9ce548 to
587eae8
Compare
|
bot: Rejected, no code change (kwf 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 |
1761fac to
48fd15f
Compare
48fd15f to
f2380fc
Compare
d728964 to
0d3763c
Compare
0d3763c to
e5a836e
Compare
e5a836e to
0ba1cc1
Compare
77a59c3 to
b5b17aa
Compare
b5b17aa to
94a15cc
Compare
006f641 to
40629ab
Compare
0779b1c to
cf51735
Compare
cf51735 to
22f1231
Compare
22f1231 to
89d37f5
Compare
Changelog for users
Changelog for maintainers
0245addsspend_alert_settings,spend_alert_rules,spend_alert_rule_state,spend_alert_hourly, andspend_alert_deliveries, and addsuser_notification_preferences.spend_alerts_enabled(default true). Scope keys areuser:<id>ororg:<id>. A check constraint allows exactly one scope foreign key.microdollar_usagein one idempotentSETupsert, 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.spend_alert_rule_state.firinglatching the crossing, a 95% hysteresis band for clearing, and a uniquededupe_keyper scope, kind, channel, observed hour, and previous episode. Push adds an idempotency key on scope, kind, and threshold.FOR UPDATE SKIP LOCKED, leases rows by advancingnext_attempt_at, fences completion onattempt_count, and retries transport failures with backoff. Senders are the new single-CTA email template and the existing notifications-worker push dispatch.spend_alertvariant of the internal dispatch RPC; the device payload carries only scope and organization id, and each recipient is gated on their ownspendAlertscategory. 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.spendAlertsrouter is registered for web and mobile, and usage-analytics cards always reserve their loaded height.isOpenAiChatGptModelnow 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 coverpublic,hidden, anddisabledstatuses across eligibility, BYOK tagging, and token resolution.name,message, orcodeand 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 notestscript, so the DB-less workspace matrix skipped its suites. Added atestscript that runsjest;jest.config.mjsignores the two database-backed suites; the migration-journal guard moved to its own DB-free suite. The package'stypecheckandlintstand as the check and both passed.apps/web/src/lib/notifications-worker-client.ts:63— accepted. The worker answers 200 with per-recipient outcomes, includingfailed. 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 samedisabled={!canSave}gate as Save.@kilocode/dbclientsuite now callsjest.mockon the global binding so it hoists above the imports and thepgmock 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.@kilocode/dbvitest check is excluded: this package has no vitest, so that check is skipped and pending owner verification. The package'stypecheckandlintstand as its check and both passed.apps/mobilevitest suites, theapps/webjest 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
E2E proof — log excerpts