🤖 refactor: persist cancellation ownership under history locks - #4146
Conversation
This comment has been minimized.
This comment has been minimized.
|
@codex review Please review this inactive real-storage adapter separately from its H1 core parent. Focus on exact nonce/frontier authority, receipt visibility at publication, malformed repair privacy floors, and the injected accepted-replacement witness requirement. Final integration passed 164 tests and full static checks; independent review approved. Runtime activation remains a later layer. Generated with |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e5bf60bf3
ℹ️ 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".
5e5bf60 to
0ffba8a
Compare
|
Addressed finding 3959558175: the held history lease is now rechecked at actual cancellation, generation, repair, provenance, and cleanup mutations. Eleven deterministic displaced-token regressions reproduce the original failure and now reject the stale writer without issuing a receipt. Integrated H1's committed-record receipt contract: publish, narrow, exact confirmation, and retire report synchronously before cleanup or lock release. Four real-filesystem pause regressions verify exact receipt contents and timing, including inherited retention. The final combined tree passes 221 affected tests and full static checks; independent review approved both changes. This remains the inactive adapter and fixed top of phase #4140 → #4146. No runtime activation was added. Both phase members will be queued together once ready. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ffba8a2f3
ℹ️ 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".
0ffba8a to
07250d1
Compare
|
Addressed all four findings:
All 242 affected tests (including 71 core cases) and full static checks pass after integrating #4140. Independent review approved. This round adds 61 changed production lines, using existing helpers and lock boundaries. No activation or new parser/storage framework was added. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07250d1f40
ℹ️ 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".
Implement exact-nonce storage under the existing history locks, preserving retry frontiers before failing awaits. Repair malformed cancellation only after fencing and neutralizing authoritative recovery intent while preserving privacy floors. Keep witness authority injected and runtime consumers inactive. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: Ib4f43dab3679f4103f90b81737311a50e8e739e3
Exclude Promise-returning repair and generation observers so local receipt state is visible at the durable publication boundary. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: I3a295533c5ae76bd9209032753833b908955760f
Propagate the lock ownership guard through cancellation and recovery writes, including provenance publication and cleanup, so a displaced lease cannot publish after takeover. Preserve synchronous commit receipts and unchanged callers without optional guards. Eleven displaced-lease regressions fail against original production and pass with the guard. All 205 affected tests and full static checks pass. Correct the existing atomic-rename test fixture to compare canonical paths on macOS. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: If930b019e6c3fb231ed3635f6a5223bbf73d3e7f
Implement the parent core's synchronous receipt contract at publication, narrowing, exact confirmation, and retirement. Report the exact inherited record or null before cleanup or lock release while retaining all lease-ownership guards. Four real-filesystem pause regressions fail on the unadapted adapter and pass with the receipt. All 221 affected tests and full static checks pass; independent review approved. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: I3151aceff7c20bf5adca0bfb88f9026b37fa4adc
Reject duplicate keys, refuse unsupported and oversized records, canonicalize serialized receipts, and sync the directory before successful mutation settlement. Checkpoint for integration with the reviewed cancellation core. Final combined validation follows parent integration. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: I08c7db03e1575f230cb6241b416543c32992be56
Keep the receipt expectation within the cancellation record union after parent integration. All 242 affected tests and full static checks pass. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: Icb096881c55a7688e95426dd557a1d30ccf8688c
Read at most the existing size limit plus one byte through a single descriptor, handle short reads, and always close it. Preserve oversized data and existing error classification. All 57 storage tests and independent review pass. Final combined validation follows parent integration. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: Ib8cdc5fefa7733283b41767ebf9360130026007e
07250d1 to
603677f
Compare
|
Addressed 3960866846: cancellation reads allocate one buffer capped at the existing limit plus one byte and read through a single descriptor. Legal short reads continue until EOF or the cap; the descriptor closes on every path. Oversized bytes remain untouched, and missing files, ordinary I/O errors, malformed records, and explicit refusal remain distinct. Four real-filesystem regressions fail against the previous whole-file reader and pass with the fix, checking buffer/read bounds, short reads, descriptor closure, and preserved bytes. All 252 combined tests (77 core and 57 storage cases included) and full static checks pass after parent integration. Independent review approved. The adapter change is 15 production lines with no new helper framework or interface changes. @codex review Generated with |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This comment has been minimized.
This comment has been minimized.
1 participant
Add the filesystem adapter for #4140's inactive cancellation core. Compare exact nonces and generation frontiers under local and shared history locks. Recheck lease ownership before every mutation so a displaced writer cannot publish, remove state, or repair history after takeover.
The adapter reports the exact JSON-normalized committed record, including inherited retention, synchronously before cleanup or lock release. Publication, narrowing, idempotent confirmation, and retirement all implement the parent's receipt contract. Directory sync completes before successful storage settlement; failure retains the visible receipt and persistence debt for retry. A later cleanup failure cannot undo that receipt or overwrite a newer accepted read.
Malformed cancellation repair fences publication and neutralizes authoritative recovery intent while preserving raw privacy floors. Duplicate-key detection reuses the existing scanner. Reads allocate and consume at most the existing size limit plus one byte through a descriptor that always closes. Future-version and oversized records explicitly refuse repair and replacement, preserving their bytes. Ordinary I/O failures remain visible. Retained cancellation can be retired only through an injected verifier for a durably accepted replacement; the adapter has no default witness authority.
This is the fixed top of phase stack #4140 → #4146. The adapter remains inactive, and both PRs will be queued together after current Codex approval, resolved findings, and passing CI.
Validation: 252 affected tests and full static checks pass. Eleven deterministic displaced-lease cases and four paused cleanup/lock-release cases fail against the respective unpatched implementations and pass with the fixes. Real HistoryService cases also cover short reads and bounded allocation, duplicate fields, future-version/oversize preservation, canonical receipts, and directory-sync failure/retry; all 77 parent core cases are included. Independent review approved the combined fixes. Nix formatting was skipped because Nix is unavailable.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable