refactor(account): retire D1 write lease mirror - #1151
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # packages/worker/src/email/inbound.ts # packages/worker/src/email/outbound.ts # packages/worker/worker-configuration.d.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md # packages/worker/src/account/export.node.test.ts # packages/worker/src/account/export.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/data-storage.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # packages/worker/src/mcp/capabilities/admin/domain.ts # packages/worker/src/mcp/capabilities/registry.node.test.ts Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
# Conflicts: # docs/contributing/architecture/entitlements.md Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR retires temporary D1 lease mirroring for UserMeter and Durable Object authority. It preserves legacy D1 leases and audited repairs, updates parity rules, and revises related tests and documentation. ChangesLease mirror retirement
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🔎 Preview deployed: https://kody-pr-1151.kody-a99.workers.dev Worker: Mocks:
|
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/src/account/deletion-state.node.test.ts (1)
1266-1283: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared helper for seeding a stale pre-retirement D1 lease row. Three tests duplicate the same raw-SQL sequence (insert into
account_write_leases, then incrementactive_write_count), differing only in theholdervalue. One shared helper removes this duplication and keeps the seeding logic in one place if the schema changes.
packages/worker/src/account/deletion-state.node.test.ts#L1266-L1283: replace the manualINSERT INTO account_write_leases/UPDATE users SET active_write_countblock with a call to a new helper, e.g.insertStaleD1LeaseRow(sqlite, { token: held.token, userId: 'user-a', holder: 'test:repair-finalize-stale', acquiredAt: held.acquiredAt }).packages/worker/src/account/deletion-state.node.test.ts#L1344-L1358: replace the equivalent block with the same helper, passingholder: 'test:stale-mirror-after-finalize'.packages/worker/src/account/deletion-state.node.test.ts#L1438-L1452: replace the equivalent block with the same helper, passingholder: 'test:finalize-fail-closed'.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/worker/src/account/deletion-state.node.test.ts` around lines 1266 - 1283, Extract an insertStaleD1LeaseRow helper in packages/worker/src/account/deletion-state.node.test.ts that performs the account_write_leases insert and active_write_count increment, then replace the duplicated SQL at lines 1266-1283, 1344-1358, and 1438-1452 with helper calls using each test’s existing token, userId, acquiredAt, and holder values: test:repair-finalize-stale, test:stale-mirror-after-finalize, and test:finalize-fail-closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/worker/src/account/deletion-state.node.test.ts`:
- Around line 1266-1283: Extract an insertStaleD1LeaseRow helper in
packages/worker/src/account/deletion-state.node.test.ts that performs the
account_write_leases insert and active_write_count increment, then replace the
duplicated SQL at lines 1266-1283, 1344-1358, and 1438-1452 with helper calls
using each test’s existing token, userId, acquiredAt, and holder values:
test:repair-finalize-stale, test:stale-mirror-after-finalize, and
test:finalize-fail-closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f527296-c0df-4b49-ab9a-97b9916f3c51
📒 Files selected for processing (10)
docs/contributing/account-write-lease-repair.mddocs/contributing/architecture/entitlements.mdpackages/worker/src/account/deletion-state.node.test.tspackages/worker/src/account/deletion-state.tspackages/worker/src/account/user-owned-surfaces.tspackages/worker/src/admin/user-meter-parity.node.test.tspackages/worker/src/admin/user-meter-parity.tspackages/worker/src/community/community-icon.node.test.tspackages/worker/src/mcp/capabilities/admin/admin-user-meter-parity.tspackages/worker/src/mcp/memory/service.node.test.ts
Summary
deleting_atpoint gate and exact legacy email D1 lease pathValidation
npm run validate— passed (1,893 tests)System recap
No D1 table/column retirement in this PR.
Conductor report
waitUntil; failed/dropped cleanup left D1-only rows. At 20:47Z: 162 unspecified (all >10m), 20 MCP (19 >1h), 1 web from Jul 24. Growth after feat(account): move write-lease authority into UserMeter #1125 confirmed the leak.0d2896a8at 2026-08-02 03:07Z. No MCP/web D1 rows were acquired after that deploy. One 03:08Z unspecified row remains because it still matches a live DO-authority lease; it was intentionally not repaired.f777653e-24c3-4808-8e3c-e3432c061360returned repaired=true; follow-up list confirmed absent. Mark/list/repair behavior is covered by the authoritative test gate; no production deletion mark was run against a real account.admin_account_write_lease_repair; 190 succeeded, 0 failed, no raw deletes. Eight affected-user parity reports: d1Only=0, legacyWithoutD1=0, gate failures=0; doOnly=2 active and expected.temporaryMirrorRetired=true;doOnlyexpected,legacyWithoutD1remains a gate;d1Onlyis diagnostic.1533313823166173364).Summary by CodeRabbit