Repository navigation
fix(core): complete a prepared binding change only under the exclusive admission - #1035
Merged
Merged
Conversation
…sion (#445) A phase change ends the window in which one phase admitted what the next one forbids, so it cannot be published while an ordinary operation may be in flight. Until now only the conversion gate held that by type; the public checkpoint and binding advance accepted a phase change from any caller. commit_phase_transition takes the installation's ExclusiveAdmissionGuard by mutable borrow, so a shared guard does not compile, and commits the root below that admission. An admission that does not guard the root of its layout is refused as NotAdmitted before anything is read. The progress checkpoint, the binding advance and the journal-level checkpoint publish now refuse a successor that changes the phase (PhaseChangeNotACheckpoint), and the transition refuses one that keeps it (NotAPhaseChange), each before a journal byte is written. The root classifies the advance with a new BindingStep::PhaseTransition, so a generation promoted directly becomes authoritative only through the transition. Capture, renewal and takeover keep their own relations. The conversion driver's two phase changes use the held form of the transition; its behaviour is unchanged. Existing tests that moved the phase through the checkpoint are migrated: the root-binding fixture picks the path by the committed phase, and the binding-advance scenario keeps the phase of its bootstrap revision. This adds no producer of a binding, opens no PREPARE/DONE window and leaves the capture writers, the writer journal locator and E3 closed.
…lusive event type (#445) The admitted root event now carries the mode of the admission it was taken from (AdmittedRootCommitGuard<'_, Mode>). The held form of the transition takes the exclusive event, so a shared admission or a bare root lock cannot reach it; a compile_fail doctest on try_root_commit pairs with one that compiles. Both routes to the held form, the public commit and the conversion driver, hand it the event of their exclusive admission. commit_phase_transition resolves the root to its canonical path once, when the admission is proven, and uses that path for every read and write of the commit. An alias that is retargeted after the proof can no longer redirect the commit to another root: a test points the alias at a second root during the call and asserts the admitted root advanced and the other one is untouched, and a second test refuses an alias that already leads to another root before anything is written. Both are Unix-only because they need a symbolic link. The test helper that bundled a checkpoint takes the journal source as one argument instead of five.
…attempt (#445) CodeScene counts the receiver, so the helper that runs a transition over a chosen file system under a chosen spelling of the root had five arguments. The two now travel as one value; the helper takes four and the tests are unchanged.
…wn module (#445) CodeScene reports Low Cohesion for the test file once it holds the fixture, the manifest builders, the transition cases and a hook file system with its alias cases. The alias cases are a separate responsibility, so they move to a module file included like the submodules of gate2_identity_test. No test changed.
The raw commit_root publishes whatever live-migration binding it is given. The relation of a changed binding to the committed one (phase, owner, inventory, lease) is proven only by the authority commits, so a caller with a bare root lock could still name a directly promoted phase change and bypass the exclusive transition. commit_root is now exported only with the test-only test-support feature, like load_catalog and list_records. The production publications (the authority commits and write_key_epoch) call the crate-private commit_root_primitive, so nothing changes for them and for bootstrap and recovery. Tests keep the raw commit as scaffolding for states that have no production producer yet, such as the binding that a later finalization will clear. A first attempt to refuse a changed binding at run time broke exactly such a test and would still be bypassed by clearing and rebinding, so the restriction is the visibility of the primitive.
…ecord the open recovery point (#445) publish_phase_transition_fenced without a committed predecessor reached the journal write for the bootstrap revision, because the successor relation was skipped when nothing was committed. A bootstrap is no phase change: the publisher now refuses it as NotAPhaseChange before any journal I/O, and the regular publisher keeps the bootstrap. A test asserts the refusal leaves the journal directory empty and that the regular publisher still writes generation 0 afterwards. The contract, the evidence and the rustdoc no longer say that the transition is the only way a phase change becomes authoritative. It is the only way to start one in the production crate. A transition that a crash interrupts after the anchor was prepared is completed forward by recover_root, which asks for no exclusive admission; that is recorded as open instead of claimed closed.
…e admission (#445) A phase transition that a crash stops after the anchor was prepared left a prepared root that recover_root completed under whatever lock its caller held, including the shared admission of an ordinary operation. The exclusive admission that began the transition was long gone, so the exclusivity the transition claims did not extend to its completion. recover_root now completes a prepared root only when it keeps the binding of the committed root (including a root that binds nothing). A prepared root that changes the binding is refused as BindingChangeNeedsExclusiveRecovery before any directory sync, pointer write or anchor write, and stays prepared. The classification compares the committed root with the prepared target, both authenticated against the anchor's digests; it is the binding and not the phase, because the phase would need the journal locator, which is not admitted. recover_root_exclusive takes the root event of the exclusive admission by type and completes any authenticated target. The shared operation path maps the refusal to RecoveryPending and writes nothing. begin_conversion, the only exclusive entry, resolves a pending preparation through the exclusive recovery before it reads the journal; it now takes the provider by mutable reference, so the tests that call it pass &mut. The tests pin the reproduction from the defect record (red without the change), the crash points (before and after the anchor prepare, a slot that was never written, a pointer that never moved, an ambiguous commit), the classification of none/some/other/cleared bindings, the exclusion by a held shared admission, the ordinary operation refused with RecoveryPending without a write, and the conversion gate resolving the root a crash left prepared in enter_convert. No journal locator, no E3 writer, no PREPARE/DONE window, no change of the C-F commit protocol.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
🤖 CodeAnt AI — Review Status
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideThis PR closes the prepared-root recovery gap by making authenticated binding changes complete only through a typed exclusive-admission path, while shared recovery refuses without writing, ordinary operations surface RecoveryPending, and the conversion gate resolves pending preparations before journal access; extensive crash and classification tests document the resulting protocol. Sequence diagram for exclusive recovery of a prepared binding changesequenceDiagram
participant Conversion as begin_conversion
participant Admission as ExclusiveAdmissionGuard
participant Recovery as recover_root_exclusive
participant RootStore as RootStore
participant Journal as CommittedJournal
Conversion->>Admission: try_root_commit()
Admission-->>Conversion: AdmittedRootCommitGuard<ExclusiveAdmission>
Conversion->>Recovery: recover_root_exclusive(fs, provider, layout, event)
Recovery->>RootStore: authenticate committed root and prepared target
RootStore-->>Recovery: binding change accepted
Recovery->>RootStore: sync directory and publish pointer/root
Recovery-->>Conversion: recovery complete
Conversion->>Journal: read_committed_journal(fs, provider, layout, source)
Sequence diagram for shared recovery refusalsequenceDiagram
participant Operation as OrdinaryOperation
participant Recovery as recover_root
participant RootStore as RootStore
participant Admission as SharedAdmission
Operation->>Admission: root_guard()
Admission-->>Operation: RootCommitGuard
Operation->>Recovery: recover_root(fs, provider, layout, held)
Recovery->>RootStore: authenticate committed root and prepared target
RootStore-->>Recovery: BindingChangeNeedsExclusiveRecovery
Recovery-->>Operation: refusal before sync or write
Operation-->>Operation: RecoveryPending
Flow diagram for prepared-root recovery admissionflowchart TD
A[Prepared root detected] --> B{Prepared binding matches committed binding?}
B -->|Yes| C[recover_root completes recovery]
B -->|No, shared path| D[recover_root returns BindingChangeNeedsExclusiveRecovery]
D --> E[Ordinary operation returns RecoveryPending]
B -->|No, exclusive path| F[recover_root_exclusive completes recovery]
F --> G[begin_conversion reads committed journal]
E --> H[Preparation remains pending without writes]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
codeant-ai
Bot
added
the
size:XL
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
CodeAnt Nitpicks1 code suggestion1. If the root directory is renamed during recovery, path-based reads and writes can use its replacement and commit anchor state before the lost identity is noticed.Race condition · |
CodeAnt PR Risk: Low Risk
Assessed commit: |
![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: 81a7c353f6
ℹ️ 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".
…nd share the refusal check (#445) recover_root_exclusive took the event of the exclusive admission but passed the caller's spelling of the root to every later read, sync and write of the recovery. An alias retargeted after the event was taken could then send them to another root while the anchor of the admitted installation is committed or aborted. It now resolves the root to its canonical path once, as commit_phase_transition does, and the lock is checked against that path. A Unix test points the alias at a second root during the recovery and asserts that the admitted root is completed and the other tree is untouched; it fails without the change. The two tests that assert "refused under a shared recovery, completed under the exclusive one" share one helper instead of repeating the sequence.
qnbs
changed the base branch from
feat/445-gate4e-e2b-phase-transition-admission
to
main
October 13, 2026 18:34
qnbs
commented
Oct 13, 2026
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
[check-pr-size] PR size is over the hard tier (normal profile): 21 files, 2232 meaningful lines, 9 commits — limit ≤20 files / ≤1200 lines / ≤10 commits. Consider splitting into smaller, independently reviewable PRs. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Owner
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
codeant-ai
Bot
added
size:XXL
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! 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 was referenced Oct 13, 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
1 participant
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.
User description
What
Closes the defect recorded in GH #359 comment 6151082206 (Codex P2-B on #1033): a phase transition that a crash stops after the anchor was prepared was completed by
recover_rootunder whatever lock its caller held, including the shared admission of an ordinary operation.recover_rootcompletes a prepared root only when it keeps the binding of the committed root (including none to none). A prepared root that changes the binding is refused asBindingChangeNeedsExclusiveRecoverybefore any directory sync, pointer write or anchor write, and stays prepared.recover_root_exclusivetakes the root event of the exclusive admission by type (AdmittedRootCommitGuard<'_, ExclusiveAdmission>) and completes any authenticated target. Acompile_faildoctest (shared event) pairs with one that compiles.OperationError::RecoveryPendingand writes nothing.begin_conversion, the only exclusive entry, resolves a pending preparation through the exclusive recovery before it reads the journal. It now takes the provider as&mut P; the test call sites changed accordingly.Scope as admitted: GH #359 comment 6151692274.
Evidence
cargo clippy --all-targets -D warnings,cargo fmt --check, and the full test suite of the crate (all passing), including the new tests below.PreparationPendingwithout the gate's resolution.gate4e_phase_transition_test/crash_recovery.rs,faulty_fs.rs): the reproduction with a snapshot of root files, journal names and anchor before and after a refused shared recovery; the crash points (before and after the anchor prepare, a slot that was never written, a pointer that never moved, an ambiguous commit); the classification (none to a binding, one binding to another, a binding to none refused; an unchanged binding and none to none complete); a held shared admission stopping the exclusive recovery; an ordinary operation refused withRecoveryPendingwithout a write; and, ingate4d_conversion_entry_test.rs, the gate resolving the root a crash left prepared inenter_convert.gate3c,gate4a,gate4band conversion tests are unchanged except for the&mutprovider argument ofbegin_conversion.bff6e34b:recover_root_exclusiveresolves the root to its canonical path once (Codex P1, with a Unix test that is red without it), and the duplicated test sequence is shared (CodeScene).Not in scope
Journal locator, E3 and capture writers,
PREPARE/DONEwindows, a phase-precise classification, any change to the C to F commit protocol, Gate 5 and later, release.Part of #359. Owner: #922 / QNB-168.
Summary by CodeRabbit
New Features
Bug Fixes
CodeAnt-AI Description
Require exclusive access for migration phase changes and recovery
What Changed
Impact
✅ Phase changes wait until ordinary operations finish✅ Interrupted phase changes recover under exclusive access✅ Ordinary operations stop safely while recovery is pending💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.