Repository navigation
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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. |
🤖 CodeAnt AI — Review Status
|
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 makes phase changes mechanically exclusive: ordinary checkpoints, binding advances, and journal publishers can only preserve the phase, while Sequence diagram for exclusively admitted phase transitionsequenceDiagram
participant Caller
participant ExclusiveAdmissionGuard
participant Authority
participant Journal
participant Root
Caller->>Authority: commit_phase_transition(admission, fs, provider, layout, checkpoint)
Authority->>Authority: check_operation_id(operation_id)
Authority->>ExclusiveAdmissionGuard: guards(scope)
ExclusiveAdmissionGuard-->>Authority: admission confirmed
Authority->>ExclusiveAdmissionGuard: try_root_commit()
ExclusiveAdmissionGuard-->>Authority: RootCommitGuard
Authority->>Journal: publish_phase_transition_fenced(manifest, fence, committed)
Journal->>Journal: assert_phase_transition_successor(prev, next)
Journal-->>Authority: PublishedManifest
Authority->>Root: commit_planned(BindingStep::PhaseTransition)
Root-->>Caller: RootCommitted
Flow diagram for phase-specific publication pathsflowchart TD
M[Successor manifest] --> C{Phase changes?}
C -->|No| P[publish_manifest_fenced]
P --> PC[assert_checkpoint_successor]
PC --> B[BindingStep::Checkpoint]
C -->|Yes| T[publish_phase_transition_fenced]
T --> PT[assert_phase_transition_successor]
PT --> A[BindingStep::PhaseTransition]
A --> E[Requires mutable ExclusiveAdmissionGuard]
B --> R[Root becomes authoritative]
E --> R
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
|
[check-pr-size] PR size is over the hard tier (normal profile): 16 files, 1368 meaningful lines, 6 commits — limit ≤20 files / ≤1200 lines / ≤10 commits. Consider splitting into smaller, independently reviewable PRs. |
![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.
|
Warning Review limit reachedEnable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Next included review available in 23 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
CodeAnt PR Risk: Low Risk
Assessed commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![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: 32636c1bfc
ℹ️ 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".
…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.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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.
codeant-ai
Bot
added
size:XXL
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fec4ba1dac
ℹ️ 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".
| let event = admission | ||
| .try_root_commit() | ||
| .map_err(AuthorityError::Admission)? | ||
| .ok_or(AuthorityError::RootBusy)?; | ||
| commit_phase_transition_held(fs, provider, layout, checkpoint, &event) |
There was a problem hiding this comment.
If commit_phase_transition fails after preparing the anchor and writing the matching target slot but before step F commits the anchor, its exclusive guard is dropped while the prepared phase-changing root remains recoverable. Although the raw commit is now private, recover_root can still complete that target using only a bare RootCommitGuard; ProtectedStorage::recover_prepared_root_admitted demonstrably calls it while holding a shared admission (operations.rs:250-254). Thus the phase change can become authoritative while an ordinary operation holds shared admission, so recovery of a prepared transition must retain or reacquire exclusive admission (or otherwise distinguish and refuse phase-changing targets).
AGENTS.md reference: AGENTS.md:L31-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid and not dismissed; this thread stays open on purpose.
Verified on the code: a transition interrupted after the anchor was prepared is completed forward by recover_root with only a RootCommitGuard, and its one production caller runs under a shared admission. A phase-precise fix is not possible at the root layer (the manifest needs the journal locator, which is not admitted); the authenticated class is "the prepared target changes the binding", which covers every journal-owner commit. A correct repair therefore changes the recovery protocol (refusal in the shared path, an exclusive recovery entry, and the conversion entry resolving a pending preparation), which goes beyond the admitted slice.
Options A/B/C with a recommendation are on #359: #359 (comment). Until the owner decides, #1033 states the point as open in the contract, the evidence and the rustdoc instead of claiming it closed.
There was a problem hiding this comment.
Owner decision recorded: option B (successor PR), confirmed 2026-10-13. This finding stays VALID / DEFERRED / MUST-FIX and this thread stays open; it is not dismissed and not accepted as safe. #1033 claims exclusivity only for the start of a phase change, and merging it needs a separate final merge admission. Reproduction, call chain, impact, acceptance criteria of the successor slice (proposed, not admitted) and the blocked steps: #359 (comment)
…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.
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.
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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". |
…sive admission (#445) (#1035) Gate 4E E2b and E2c as one protected squash (owner decision S2): a journal phase change is published only by commit_phase_transition under the typed exclusive admission, and a prepared root that changes the live-migration binding is completed only by recover_root_exclusive. Includes the six commits of PR #1033, which is closed unmerged and superseded by this PR. Issue #359.
|
Closed as superseded by #1035 — not merged as its own PR. All six commits of this branch (head The Codex P2-B thread on this PR (a prepared binding change completed under a shared admission after a crash) was valid when raised and stays here unresolved as historical review evidence. It is repaired by the E2c commits The branch is kept ( |
1 participant
User description
What
A journal phase change is now published only by
commit_phase_transition, which takes the installation'sExclusiveAdmissionGuardby mutable borrow. The ordinary progress checkpoint no longer carries a phase change.commit_phase_transitioncommits the root below that admission; an admission that does not guard the root of its layout is refused asNotAdmittedbefore anything is read. A shared guard, and a shared reference to the exclusive guard, do not compile (twocompile_faildoctests paired with one that compiles).PhaseChangeNotACheckpoint); the transition refuses one that keeps it (NotAPhaseChange). Both refusals precede the first journal byte.BindingStep::PhaseTransition, so a generation promoted directly becomes authoritative only through the transition. Capture, renewal and takeover keep their own relations.enter_convert,finish_convert) use the held form of the transition; its behaviour is unchanged.Why
The owner confirmed D1(a), D2(a) and D3 on #359 for exactly one bounded Gate 4E E2 follow-up slice: typed exclusive protection of real phase changes, separated from ordinary progress checkpoints. Contract §10.3 named this as the step that makes the exclusive-admission requirement of a phase change mechanical.
Evidence
cargo clippy --all-targets -D warningsfor the crate,cargo fmt --check, the affected test binaries (the new file, root binding, journal durable, journal route, conversion entry) and the threecommit_phase_transitiondoctests were run, one job at a time.tests/gate4e_phase_transition_test.rs: success under exclusive admission, both refusals writing nothing, an admission for another root, the directly promoted phase change (refused by the advance and the checkpoint, adopted without a journal write only by the transition), the journal-level publishers, and shared/exclusive admission excluding each other.E0308on the guard parameter, not on an unrelated error.Not in scope
No producer of a binding, no
PREPARE/DONEwindow, no change to the capture, takeover or renewal writers, no writer journal locator, no E3, no Gate 5 or later. The E1 barrier is unchanged.Part of #359. Owner: #922 / QNB-168.
Summary by Sourcery
Require exclusive admission for journal phase changes while keeping ordinary progress checkpoints phase-preserving.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by cubic
Publishes a journal phase change only under the installation's exclusive admission: the new
commit_phase_transitiontakesExclusiveAdmissionGuardby mutable borrow, and the progress checkpoint, binding advance, and journal-level checkpoint publish refuse a successor that changes the phase.AdmittedRootCommitGuard<'_, Mode>), so the held transition takes the exclusive event and a shared guard or bare root lock cannot reach it.commit_phase_transitionresolves the root's canonical path once at admission proof and uses it for all reads and writes, so a retargeted alias cannot redirect the commit; an admission for another root is refused asNotAdmitted.PhaseChangeNotACheckpoint,NotAPhaseChange) precede the first journal byte; the transition adopts an identical directly-promoted candidate without a journal write, and the phase-transition publisher refuses a bootstrap, which stays with the ordinary publisher.commit_root, which publishes whatever binding it is given, is now exported only with the test-onlytest-supportfeature, so a bare root lock can no longer make a directly promoted phase change authoritative through it.enter_convertandfinish_convertuse the held transition form with unchanged behavior; capture, takeover, and renewal writers are untouched.gate4e_phase_transition_test.rscovers success, both refusals, foreign admission, direct promotion, the journal-level publishers, shared/exclusive exclusion, and Unix-only alias redirection, the alias cases in their own module.recover_root, which asks for no exclusive admission; that point stays open (Desktop fs-data key-rotation migration is not crash-resumable (mixed-key state possible) #359).Written for commit 8017776. Summary will update on new commits.
Summary by CodeRabbit
CodeAnt-AI Description
Require exclusive admission before changing a journal phase
What Changed
Impact
✅ Phase changes cannot overlap ordinary operations✅ Invalid phase updates write no journal data✅ Wrong-root admissions cannot commit a transition💡 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.