[kentcdodds]

Summary

• make UserMeter authoritative for non-email write-lease acquire, held checks, release, list, and drain counting
• retain D1 users.deleting_at as permanent cross-surface deletion projection
• keep email on unchanged D1 lease path during mailbox transition
• temporarily mirror same-token DO leases into D1 so old deletion markers see every writer
• add audit-first, idempotent prepare/finalize repair for DO leases
• clean partial D1 mirror commits before failing acquisition
• preserve public errors, ALS nesting, holder, lease-lost, and waitUntil behavior

Validation

• PR validation — passed against Mailbox parity main
• post-merge validation — passed
• production deployment — passed
• repeated independent high-risk reviews — no code blockers

Deployment notes

High-risk rolling cutover is deployed. The temporary same-token D1 lease mirror remains until the old-isolate drain gate below passes. D1 deleting_at, the repair audit table, and email's D1 lease path remain.

System recap — extends User meter into deletion-fence authority (high risk)

Mode: recap · Base: main @ e76b8c9a · Head: 7e888008

Classification: extends — changes authoritative coordination for non-email account write leases and deletion drains while preserving Mailbox dual-write/parity behavior.

Primitives touched

Primitive	Group	Impact
user-meter	Storage & infrastructure	extends — schema-v7 authoritative leases and repair protocol
app-ui	Entry points	extends — non-email mutation leases use UserMeter authority
rbac	Identity & auth	extends — union list and audit-safe DO repair
account-export	User-facing assistant primitives	composes — sanitized deletion inventory remains additive
mailbox	Storage & infrastructure	composes — existing email D1 lease path and parity soak remain unchanged

System map

Non-email writes acquire in UserMeter and temporarily mirror the same token into D1; email remains on D1; deletion mark counts the deduped union.

Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).

flowchart LR
	writers["app-ui<br/>Non-email mutation boundaries"]:::extended
	mailbox["mailbox<br/>Per-user mailbox"]:::touched
	meter["user-meter<br/>User meter"]:::extended
	d1["d1-app-db<br/>D1 app database"]:::extended
	admin["rbac<br/>Admin repair"]:::extended
	writers -->|"authoritative acquire / held / release"| meter
	writers -->|"temporary same-token rollout mirror"| d1
	mailbox -->|"transitional email lease path"| d1
	d1 -->|"deleting_at gate + legacy/email snapshot"| meter
	admin -->|"prepare → audit → finalize → mirror clear"| meter
	admin -->|"durable repair audit"| d1
	classDef touched fill:#1a7f37,color:#fff
	classDef extended fill:#9a6700,color:#fff
	classDef added fill:#cf222e,color:#fff
	classDef untouched fill:#57606a,color:#fff

Loading

Invariants

• old and new markers see every writer during rollout
• failed/partial mirror acquisition is cleaned before write starts
• D1 mirror is never cleared while a DO lease remains held
• missing UserMeter fails closed
• email no-env path remains exact D1 behavior
• purge preserves tombstone
• repair is audit-first/idempotent across partial failures
• Mailbox dual-write/parity behavior remains unchanged

Conductor report

• STATUS: done
• What shipped: non-email write-lease authority in UserMeter with rolling-version D1 mirrors, partial-mirror cleanup, mixed D1+DO drain counting, and audit-safe DO repair.
• Risk: high — merged under Kent's explicit one-PR authority after current-main validation and independent security review.
• Merged/deployed: yes / yes; PR #1125, post-merge validation, and production deploy succeeded.
• Sibling-track spill: Mailbox dual-write/parity-soak changes were merged and validated; feat(account): move write-lease authority into UserMeter #1125 added no email/** diff.
• Turnkey follow-up gates:
  1. Deletion D1 mirror removal: after an old-isolate drain window covering the longest Worker/waitUntil lease, compare active D1 tokens with UserMeter DO-authority tokens for sampled active users, allowing only known email D1-only leases. Require zero unexplained mismatches plus successful mark/list/repair smoke tests. Then remove same-token D1 acquire/release mirrors only; retain D1 deleting_at, email fallback, and repair audit.
  2. Storage authority flip: mailbox inbound/outbound storage reservations must pass env.USER_METER; cron reconciliation already passes env. Run a complete reconciliation rotation and compare users.d1_storage_bytes with storageBytesShadow; require zero unexplained mismatches and cold-bootstrap/deny-at-limit smoke tests.
  3. Package-service authority flip: after the full 24h stale window from feat(services): shadow liveness in UserMeter #1119 deploy, compare D1 fresh-running counts (including excludeService) with UserMeter counts across active users; require zero unexplained mismatches and cold-bootstrap/restart smoke tests.

  

Summary by CodeRabbit

• Improvements

  • Account deletion write protection now remains reliable during rollouts and service interruptions.
  • Lease acquisition, release, repair, and retries fail safely while preventing duplicate or stale records.
  • Administrative lease listings provide deduplicated results across active systems.
  • Deletion safeguards remain effective after account purge operations.
  • Account exports continue to provide sanitized lease and deletion-fence information.

• Documentation

  • Updated architecture, repair procedures, deletion behavior, rollout compatibility, and account-export details.

[coderabbitai]

📝 Walkthrough

Walkthrough

UserMeter schema version 7 now manages non-email account write leases. D1 stores temporary mirrors. Account deletion, lease repair, listing, exports, documentation, and test environments now use the split-authority model.

Changes

Account write-lease authority

Layer / File(s)	Summary
UserMeter authority and schema packages/worker/src/entitlements/user-meter-do.ts, packages/worker/src/test-support/user-meter.ts, docs/contributing/architecture/*	UserMeter adds schema-v7 lease authority, lease lifecycle methods, staged repair, authority-filtered listing, tombstone preservation, and legacy lease reconciliation.
Deletion-state lease orchestration packages/worker/src/account/deletion-state.ts, packages/worker/src/account/export.ts, packages/worker/src/account/user-owned-surfaces.ts, packages/worker/src/mcp/capabilities/admin/*, docs/contributing/account-write-lease-repair.md	Account writes use DO leases with temporary D1 mirrors. Listing merges authorities by token. Repair verifies audits, finalizes DO leases, and clears mirrors after successful completion.
Lease protocol validation packages/worker/src/account/deletion-state.node.test.ts, packages/worker/src/entitlements/user-meter.workers.test.ts	Tests cover authority separation, fail-closed behavior, tombstones, release ordering, repair audits, and lost-finalize retries.
Runtime test environment migration packages/worker/src/**/*.test.ts, packages/worker/src/test-support/incidental-runtime-warnings.ts	Tests add UserMeter bindings, lease-aware D1 mocks, shared environment factories, and deferred-work draining.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant withAccountWriteLease
  participant UserMeter
  participant D1
  participant ProtectedWrite
  Caller->>withAccountWriteLease: request account write
  withAccountWriteLease->>UserMeter: acquire DO-authoritative lease
  withAccountWriteLease->>D1: create temporary mirror
  withAccountWriteLease->>ProtectedWrite: execute write callback
  withAccountWriteLease->>UserMeter: verify lease is held
  withAccountWriteLease->>UserMeter: release lease
  withAccountWriteLease->>D1: clear mirror

Loading

Possibly related PRs

• kentcdodds/kody#890: Related account deletion and write-lease coordination changes.
• kentcdodds/kody#1123: Preceding UserMeter lease-shadow implementation replaced by DO authority here.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the primary change: moving write-lease authority into UserMeter.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/meter-do-38c8

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cursor]

@coderabbitai review

[github-actions]

🔎 Preview deployed: https://kody-pr-1125.kody-a99.workers.dev

Worker: kody-pr-1125
D1: kody-pr-1125-db
KV: kody-pr-1125-oauth-kv

Mocks:

• cloudflare: https://kody-pr-1125-mock-cloudflare.kody-a99.workers.dev/__mocks (kody-pr-1125-mock-cloudflare)

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

packages/worker/src/entitlements/user-meter-do.ts (1)

699-716: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider running the v7 column adds whenever the stored version is not 7.

The guard version != null && version < 7 skips the ALTER TABLE statements when the meta row is absent. In that case CREATE TABLE IF NOT EXISTS also does not add the new columns if the table already exists, so every later SELECT ... authority, pending_repair_id would fail. The guard is safe today because initializeSchema always writes the meta row. Dropping the null check makes the migration robust if that invariant changes.

♻️ Proposed defensive change

-		if (version != null && version < 7) {
+		if (version == null || version < 7) {

🤖 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/entitlements/user-meter-do.ts` around lines 699 - 716,
Update the migration guard around the v7 column additions to run whenever the
stored version is not 7, including when version is null; preserve the existing
ALTER TABLE statements and idempotent error handling in the surrounding schema
initialization flow.

packages/worker/src/test-support/user-meter.ts (1)

704-712: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Align invalid-authority handling with the Durable Object.

UserMeterBase.bootstrapDeletionState calls assertWriteLeaseAuthority, which throws for an unrecognized authority string. This fixture instead falls back to 'legacy'. A node test that passes an invalid authority would pass here and fail in production. Throw the same error to keep the fixture faithful.

♻️ Proposed change

-					const authority =
-						typeof lease.authority === 'string' &&
-						isUserMeterWriteLeaseAuthority(lease.authority)
-							? lease.authority
-							: 'legacy'
+					let authority: UserMeterWriteLeaseAuthority = 'legacy'
+					if (typeof lease.authority === 'string') {
+						if (!isUserMeterWriteLeaseAuthority(lease.authority)) {
+							throw new Error(
+								`UserMeter write lease authority must be one of do, legacy; got ${JSON.stringify(lease.authority)}.`,
+							)
+						}
+						authority = lease.authority
+					}

🤖 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/test-support/user-meter.ts` around lines 704 - 712,
Update the authority handling in the lease acquisition flow around
acquireWriteLease and shadowAcquireWriteLease to call the same
assertWriteLeaseAuthority validation used by
UserMeterBase.bootstrapDeletionState. Remove the fallback to 'legacy' for
unrecognized string authorities, while preserving the existing handling for
valid authorities and the legacy absence case.

🤖 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.

Inline comments:
In `@packages/worker/src/account/deletion-state.ts`:
- Around line 535-546: Update the catch block around acquireD1WriteLeaseMirror
to call releaseD1WriteLeaseMirror for the same lease before releasing the DO
write lease and rethrowing the original error. Ensure this cleanup runs for both
a false acquisition result and exceptions after a committed insert, relying on
the release method’s idempotency.

---

Nitpick comments:
In `@packages/worker/src/entitlements/user-meter-do.ts`:
- Around line 699-716: Update the migration guard around the v7 column additions
to run whenever the stored version is not 7, including when version is null;
preserve the existing ALTER TABLE statements and idempotent error handling in
the surrounding schema initialization flow.

In `@packages/worker/src/test-support/user-meter.ts`:
- Around line 704-712: Update the authority handling in the lease acquisition
flow around acquireWriteLease and shadowAcquireWriteLease to call the same
assertWriteLeaseAuthority validation used by
UserMeterBase.bootstrapDeletionState. Remove the fallback to 'legacy' for
unrecognized string authorities, while preserving the existing handling for
valid authorities and the legacy absence case.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5f60e21f-bb4f-4aa9-8e77-56d3a0e9bf09

📥 Commits

Reviewing files that changed from the base of the PR and between 0684b78 and 29dc257.

📒 Files selected for processing (22)

• docs/contributing/account-write-lease-repair.md
• docs/contributing/architecture/data-storage.md
• docs/contributing/architecture/entitlements.md
• packages/worker/src/account/deletion-state.node.test.ts
• packages/worker/src/account/deletion-state.ts
• packages/worker/src/account/export.ts
• packages/worker/src/account/user-owned-surfaces.ts
• packages/worker/src/app/account-deletion.node.test.ts
• packages/worker/src/app/account-write-lease-middleware.node.test.ts
• packages/worker/src/community/avatar.node.test.ts
• packages/worker/src/community/community-icon.node.test.ts
• packages/worker/src/entitlements/user-meter-do.ts
• packages/worker/src/entitlements/user-meter.workers.test.ts
• packages/worker/src/jobs/service.node.test.ts
• packages/worker/src/mcp-auth.workers.test.ts
• packages/worker/src/mcp/capabilities/admin/admin-account-write-lease-list.ts
• packages/worker/src/mcp/memory/service.node.test.ts
• packages/worker/src/mcp/observability.workers.test.ts
• packages/worker/src/mcp/run-kody-registry.node.test.ts
• packages/worker/src/package-registry/service.node.test.ts
• packages/worker/src/test-support/incidental-runtime-warnings.ts
• packages/worker/src/test-support/user-meter.ts

[coderabbitai]

🧹 Nitpick comments (1)

packages/worker/src/account/deletion-state.node.test.ts (1)

1147-1248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared D1 mirror-failure stub.

Both tests build the same db.batch and db.prepare stubs. Only the run() behavior differs. Extract a helper that installs the stubs and returns a restore function. Call the restore function in a finally block so a failed assertion does not leave the stubs installed.

♻️ Proposed helper

+function stubPartialMirror(
+	db: D1Database,
+	run: () => Promise<{ meta: { changes: number } }>,
+) {
+	const originalBatch = db.batch.bind(db)
+	const originalPrepare = db.prepare.bind(db)
+	db.batch = (async () => {
+		throw new Error('simulated lost D1 batch response')
+	}) as D1Database['batch']
+	db.prepare = ((query: string) => {
+		const statement = originalPrepare(query)
+		if (!query.includes('active_write_count + 1')) return statement
+		return {
+			bind(...params: Array<unknown>) {
+				const bound = statement.bind(...params)
+				return { run, first: bound.first.bind(bound), all: bound.all.bind(bound) }
+			},
+		}
+	}) as D1Database['prepare']
+	return () => {
+		db.batch = originalBatch
+		db.prepare = originalPrepare
+	}
+}

🤖 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 1147 -
1248, Extract the duplicated db.batch and db.prepare mocking setup from the two
tests into a shared helper that accepts the differing run behavior and returns a
restore function. Update both tests to use this helper, and invoke the returned
restore function in finally blocks so the original database methods are restored
even when assertions fail.

🤖 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 1147-1248: Extract the duplicated db.batch and db.prepare mocking
setup from the two tests into a shared helper that accepts the differing run
behavior and returns a restore function. Update both tests to use this helper,
and invoke the returned restore function in finally blocks so the original
database methods are restored even when assertions fail.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 749bbae3-da0e-49b3-bc19-e9e9c0830497

📥 Commits

Reviewing files that changed from the base of the PR and between 29dc257 and 11814af.

📒 Files selected for processing (2)

• packages/worker/src/account/deletion-state.node.test.ts
• packages/worker/src/account/deletion-state.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• packages/worker/src/account/deletion-state.ts