feat: add persistence v0.1 durable state boundary - #21
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fe00da920e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const claims = await Promise.allSettled([ | ||
| persistence.claimTask({ | ||
| workspaceId: 'ws-a', | ||
| taskId: 'task-a', | ||
| sessionId: 'session-a', |
There was a problem hiding this comment.
Exercise claim races through independent coordinators
Both competing calls use the same persistence instance and therefore the same WorkspaceDurableObjectCoordinator; Promise.allSettled only exercises that coordinator's in-memory serialization. An implementation with no database-level compare-and-set or uniqueness guard could pass this test yet grant two active leases when independent coordinator instances or deployments race against the same durable store. Run the claims through independently constructed persistence/coordinator instances sharing the database so this test verifies the durable final guard.
Useful? React with 👍 / 👎.
| await persistence.appendPermissionDecision({ | ||
| decision: finalDecision, | ||
| expectedPreviousDecisionId: initialDecision.id, | ||
| }); |
There was a problem hiding this comment.
Reject stale permission heads in the contract test
This exercises only the successful case where expectedPreviousDecisionId matches the current head. An implementation that ignores this argument and unconditionally inserts the decision and replaces the head would pass, allowing two human decisions based on the same policy decision to both commit and fork the supposedly single ordered chain. Add a second append with a stale previous-decision ID and assert that it is rejected without changing either history or the head.
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary
Implements issue #16 on
feature/persistence-v0-1from common base87fbfcf2ed738071f648a3b94bfff1df1f235dba.This PR is being developed test-first. The initial head contains only persistence contract tests so the permanent
Qualityworkflow can demonstrate the RED phase before production persistence code is added.Intended persistence model
src/persistence/;(workspaceId, commandId)immutable command receipts;Verification status
Qualityrun on this tests-only head.Closes #16