Repository navigation
Retire the D1 package_invocations table (#1061) - #1064
Merged
Merged
Conversation
The RunLog DO ledger is now the only store for keyed package-invocation idempotency (issue #1061; Kent explicitly waived the dual-read window — pre-migration keys stop replaying and execute fresh on redelivery): - remove the dual-read D1 fallback from the keyed invoke path and the legacy row schema/lookup from repo.ts (no D1 ledger reads or writes remain anywhere on the invoke path; the node suite's fake D1 now throws on ANY package_invocations statement) - remove the package_invocations retention sweep, its policy entry, and its account export/deletion coverage (dispositions + data targets) - remove the table from the storage_bytes entitlement sum (the ledger is DO-resident, excluded like run records; the query would break after the drop) - add 0112-drop-package-invocations.sql (0029 stays in history) and register it in the migration ledger - rewrite the generic retention batch/chunk/round-robin tests onto workflow_runs and drop dead test-schema fixtures for the table
📝 WalkthroughWalkthroughThe PR drops the legacy ChangesPackage invocation ledger retirement
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
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 |
Contributor
|
🔎 Preview deployed: https://kody-pr-1064.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/worker/migrations/0112-drop-package-invocations.sql (1)
1-8: 🧹 Nitpick | 🔵 TrivialConfirm rollout ordering for the DROP TABLE.
No remaining
package_invocationsreferences remain in the tracked code, but D1 migrations and Worker code deploys are separate steps. Run this migration only after every Worker instance is running the code version that no longer readspackage_invocations; otherwise a rolling deploy can leave old instances querying the missing table.🤖 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/migrations/0112-drop-package-invocations.sql` around lines 1 - 8, Document and enforce rollout ordering for migration 0112: deploy the Worker version that no longer reads package_invocations to every instance before applying the DROP TABLE migration. Update the migration or associated deployment guidance to make this prerequisite explicit, while preserving the unrelated package_invocation_tokens table.
🤖 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/migrations/0112-drop-package-invocations.sql`:
- Around line 1-8: Document and enforce rollout ordering for migration 0112:
deploy the Worker version that no longer reads package_invocations to every
instance before applying the DROP TABLE migration. Update the migration or
associated deployment guidance to make this prerequisite explicit, while
preserving the unrelated package_invocation_tokens table.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 052a0f7f-2d6a-4330-b752-ccf7d468331d
📒 Files selected for processing (21)
docs/contributing/architecture/data-storage.mddocs/contributing/architecture/invocation-overhead-guardrails.mddocs/contributing/architecture/run-records.mddocs/contributing/disaster-recovery.mdpackages/worker/migrations/0112-drop-package-invocations.sqlpackages/worker/src/account/data-targets.tspackages/worker/src/app/account-deletion.node.test.tspackages/worker/src/app/account-retention-dispositions.tspackages/worker/src/app/retention.node.test.tspackages/worker/src/app/retention.tspackages/worker/src/email/inbound.workers.test.tspackages/worker/src/email/system-email-subscriptions.workers.test.tspackages/worker/src/entitlements/entitlements.node.test.tspackages/worker/src/entitlements/service.tspackages/worker/src/package-invocations/idempotency.tspackages/worker/src/package-invocations/idempotent-module-invocation.tspackages/worker/src/package-invocations/repo.tspackages/worker/src/package-invocations/service.node.test.tspackages/worker/src/package-registry/service.node.test.tspackages/worker/src/test-support/workers-seed.tstools/migration-ledger.json
💤 Files with no reviewable changes (9)
- packages/worker/src/test-support/workers-seed.ts
- packages/worker/src/account/data-targets.ts
- packages/worker/src/app/account-retention-dispositions.ts
- packages/worker/src/package-registry/service.node.test.ts
- packages/worker/src/app/account-deletion.node.test.ts
- packages/worker/src/email/inbound.workers.test.ts
- packages/worker/src/entitlements/entitlements.node.test.ts
- packages/worker/src/app/retention.ts
- packages/worker/src/email/system-email-subscriptions.workers.test.ts
5 tasks done
This was referenced Jul 30, 2026
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Executes #1061. #1053 moved the keyed package-invocation idempotency ledger into the per-user RunLog Durable Object with a dual-read window for pre-migration D1 rows. Kent explicitly waived that window today: keys claimed before ~14:00 UTC 2026-07-30 stop replaying and may re-execute on redelivery — accepted. The RunLog DO becomes the only ledger store; no D1 ledger reads or writes remain anywhere on the invoke path.
What changed
idempotent-module-invocation.ts): the dual-read fallback (resolveLegacyFallback, legacy poll, poll-disappearance legacy lookup) is gone. A pre-migration-style key is indistinguishable from a new key: it claims in the DO and executes fresh. All DO-ledger semantics are untouched (mismatch 409, 100 ms/1 s polling, 15-minute stale reclaim, bounded replay).repo.ts):getPackageInvocationByKey, the row schema,mapRow, andPackageInvocationRecordremoved. The token helpers and the shared response-bounding (boundedResponseJson/parseStoredResponse, now DO-only) stay.app/retention.ts): thepackage_invocationssweep, its policy entry, andpackageInvocationRetentionDaysremoved; the DO's own 90-day sweep (shipped in Migrate the keyed package-invocation idempotency ledger from D1 into the per-user RunLog DO #1053) is the only retention. The generic batch/chunk/round-robin retention tests that used the table as their fixture now run againstworkflow_runs— same coverage, different fixture.package_invocationsremoved fromaccount-retention-dispositions.tsandaccount/data-targets.ts(the deletion/export guards derive requirements from the migrated schema, so keeping either entry after the drop would fail the stale-coverage checks). DO ledger rows remain covered via the RunLog surface (exportrun_recordssection, deletionclearAll) from Migrate the keyed package-invocation idempotency ledger from D1 into the per-user RunLog DO #1053.entitlements/service.ts): thestorage_bytessum no longer queriespackage_invocations— beyond the issue's bullet list, but required: the query would throw at runtime once the table is dropped. The DO-resident ledger is excluded from the storage quota exactly like run records (self-expiring operational state, not user content).0112-drop-package-invocations.sql(DROP TABLE IF EXISTS package_invocations; indexes from0055drop with it).0029stays in history untouched; the migration ledger is updated;migrations:checkgreen (117 files, next free prefix 0113). The0105restore-safe-row-sizes migration test applies only migrations before0105, so it remains valid history coverage.package_invocationsstatement (read or write) — every keyed test passing against it is the proof for the done-definition. The four dual-read tests are replaced by one explicit test: a pre-migration-style key misses cleanly and executes fresh instead of erroring. DeadCREATE TABLE package_invocationsfixtures removed fromworkers-seed.tsand the two email dispatch workers suites.*_jsonscope columns, which live onpackage_invocation_tokens), invocation-overhead guardrails, and the disaster-recovery ledger-risk note updated to the single-store reality.Deploy-window note
There is a brief mixed window during deploy where previous-version isolates (which still carry the fallback read) can race the applied drop migration; their legacy lookup then fails and the keyed path returns a retriable 500 (
idempotency_lookup_failed) for fresh keys until the new version takes over. Providers retry; this is within the waived-window acceptance. New-version isolates never touch D1.Sibling awareness
Stays clear of the #1048 narrow-phase territory: no changes to
runtime-tool-factories.ts,invoke-check.ts,runtime-helper-manifest.ts,package-runtime/**, orrepo/checks.ts. (Branch note: this work is a fresh history offmain@44b86473; it reuses this run's designated branch name because Cursor Cloud's PR mechanism is pinned to it.)Program report
MERGE-READY
Status: green and mergeable at head
048ecd48(mergeStateStatus CLEAN againstmain).npm run validateexit 0 on the head commit (format, lint, typecheck, node + workers unit, Playwright E2E, MCP E2E, backup:build, primitives/migrations checks).package_invocationsstatement); drop migration in place with0029kept in history;migrations:checkgreen;npm run validategreen.System recap — extends the keyed invocation path and D1 schema (medium risk)
Mode: recap · Base:
main@44b86473· Head:048ecd48Classification: extends — removes the legacy D1 half of the invocation ledger contract (fallback read, sweep, account coverage, entitlement sum) and drops the table. No new primitive; the RunLog DO ledger (shipped in #1053) is unchanged.
Primitives touched
d1-app-db0112-drop-package-invocations.sqldrops the legacy ledger tablescheduled-cronpackage_invocationssweep removed from the retention cronaccount-exportentitlementsstorage_bytesno longer sums the dropped tablerun-recordsUnmapped path:
packages/worker/src/package-invocations/(keyedpackages.invokesurface) — extends: dual-read fallback and legacy repo helpers deleted.System map
The keyed invoke path now talks only to the RunLog DO; every D1 edge to the legacy table is deleted and the table itself is dropped.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Before / after
package_invocationspresent (write-orphaned)0112;0029kept in historyInvariants
no-per-event-shared-writesto completion for this surface: the shared D1 writer now sees zero invocation-ledger traffic, reads included.Summary by CodeRabbit
Data & Reliability
Account Management
Maintenance