Docs: state the post-merge version-two status across the documentation - #98
Docs: state the post-merge version-two status across the documentation#98flyingrobots wants to merge 1 commit into
Conversation
PR #78 landed one-way migration, version-two reopen, and forward retention publication with the review hardening on top, but several pages still spoke from before it: the version-two overview said production retention publication did not exist, the recovery page said version two was unavailable as production, the closure page called publication integration planned, the reconstruction contract said retention remained planned, and the README said version one was the only store admitted for production. Every page now states the same boundary. Implemented: migration, reopen with root-identity binding and pinned directories, publication binding this store's catalog head and selected catalog, and typed refusals for every namespace, capacity, stage, and substitution violation. Planned: retention publication recovery and KEEP-CRASH-036..052, partial-prefix migration recovery and KEEP-CRASH-053..073, the reader fence, model-based transition evidence (#19), garbage collection (#21), and the restart-stable root identity coordinate (#97). The README says plainly that an interrupted version-two publication waits for a human until #19 lands, routes to the split recovery and publication pages, and adds #97 to its gap table. The requirements ledger's RETENTION-007 and MIGRATION-003 evidence cells name the new laws. No claim was added that a test does not back. Refs #19 #21 #97
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (2)
🧰 Additional context used🪛 LanguageTooldocs/formats/segment-store-v2/requirements.md[style] ~33-~33: The words ‘observation’ and ‘observed’ are quite similar. Consider replacing ‘observed’ with a different word. (VERB_NOUN_SENT_LEVEL_REP) 🔇 Additional comments (9)
Summary by CodeRabbit
WalkthroughThe documentation updates align Version-two status statements across the README, format specifications, recovery notes, requirements evidence, reconstruction contract, and changelog. They distinguish implemented behavior from planned recovery, fencing, collection, and identity work. ChangesVersion-two documentation status
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to The documentation now distinguishes implemented Version 2 retention, migration, and closure behavior from remaining recovery and fencing limitations. No merge-blocking risk is currently identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the problem, approach, implemented boundaries, planned work, evidence, and issue references, but it does not follow the required template closely. It omits explicit sections for the affected invariant, rejected alternatives, benchmark impact, format and API compatibility, security implications, and the checklist. Resolution Revise the description to include every template section. State the affected Keep invariant, rejected alternatives and reasons, malformed/corrupt/interrupted/resource failure modes, benchmark impact, durable-format/identity/migration/public-API compatibility, recovery behavior, security implications, and checklist completion. Use the required section headings, or explicitly state that a section has no impact.
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. Version two speaks with a clearer voice Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![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: 46ead1f445
ℹ️ 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".
| that residue, and readers have no fence, so **an interrupted version-2 | ||
| publication waits for a human until #19 lands.** A version-1 store stays | ||
| admitted until its owner migrates it; migrate only if you accept that wait. |
There was a problem hiding this comment.
This migration advice mentions only the wait caused by an interrupted publication, but a completely successful migration can also become unavailable after an ordinary reboot, unmount/remount, or restore changes statx.stx_mnt_id. The intent persists that mount ID (docs/formats/segment-store-v2/recovery.md:121-123), and FilesystemVersionTwoAdmission::require_root_identity compares it exactly and returns RootIdentityChanged (src/adapters/filesystem_version_two_admission.rs:120-134). Until #97 supplies a restart-stable coordinate, operators need this consequence stated explicitly before being advised to migrate.
AGENTS.md reference: AGENTS.md:L152-L156
Useful? React with 👍 / 👎.
| it selects, and refuses retained stages, superseded candidates, substituted | ||
| files, replaced protocol directories, and every namespace or capacity | ||
| violation before it writes anything. Each refusal is a typed value, not a |
There was a problem hiding this comment.
When the candidate namespace changes after verify_current, this promise is false: execute_root writes and synchronizes root.next before calling admit_root_namespace (src/adapters/retention/publication_execution.rs:55-65). The regression at filesystem_retention_attempt_tests.rs:78-102 demonstrates exactly this ordering by creating the conflicting namespace after the stage write and then receiving NamespaceExpectationViolated; the retained stage subsequently requires recovery. Limit the claim to violations observed during preflight, or document that a later namespace refusal can leave durable stage evidence.
AGENTS.md reference: AGENTS.md:L84-L89
Useful? React with 👍 / 👎.
| | `KEEP-RETENTION-005` | Closure derivation is deterministic, bounded, cycle-safe, fail-closed, and verifies complete blob reconstruction | exact accounting, reconstruction, adversarial-catalog, and exhaustive model laws in `tests/retention_closure.rs`; corrupt members refuse through the inherited segment-record admission laws and seeded `segment_format` fuzz target routed by `closure-corruption.md` | Implemented | | ||
| | `KEEP-RETENTION-006` | Publication follows the exact ordered durability protocol, including new namespace-directory admission and retention of fixed-stage evidence until head commit, and returns only after cleanup synchronization | typed vocabulary and blocking port in `tests/retention_publication_phase.rs` and `tests/retention_publication_storage.rs`; ordered execution, conditional namespace sync, and all 17 exact storage-fault boundaries in `tests/retention_publication_execution.rs`; production 17-phase forward filesystem execution, exclusive staging, byte-equal inode-substitution refusal, and retained-stage recovery refusal in `filesystem_retention_storage_tests`; orphan namespace directories count against the 4,096 ceiling and refuse a new namespace before any stage is written in `filesystem_retention_capacity_tests`; crash injection remains | In progress in #19 | | ||
| | `KEEP-RETENTION-007` | Restart resolves every fixed-stage crash prefix to one documented lawful state or typed ambiguity | recovery-required refusals before any mutation in `filesystem_retention_expectation_tests`: an absent head over populated pools, a non-initial head prepared against an absent head, an orphan directory for a namespace expected absent, and an absent directory for a namespace expected current; debug and release crash matrix remains | In progress in #19 | | ||
| | `KEEP-RETENTION-007` | Restart resolves every fixed-stage crash prefix to one documented lawful state or typed ambiguity | recovery-required refusals before any mutation in `filesystem_retention_expectation_tests`: an absent head over populated pools, a non-initial head prepared against an absent head, an orphan directory for a namespace expected absent, and an absent directory for a namespace expected current; debug and release crash matrix remains; replaced protocol directories, an absent or changed head-selected catalog, an over-full census, zero-generation pool names, and a stage retained by a failed write refuse in `filesystem_retention_*_tests` | In progress in #19 | |
There was a problem hiding this comment.
The new evidence entry says that a changed head-selected catalog refuses in filesystem_retention_*_tests, but a repo-wide search for CatalogChanged finds only the production variant and branch (filesystem_retention_catalog.rs:70) plus prose; filesystem_retention_catalog_tests.rs:17-125 tests foreign, corrupt-head, and absent-catalog cases but never constructs a changed selected catalog or asserts CatalogChanged. The authoritative ledger therefore presents an untested case as evidence; add an exact regression for that variant or leave the case explicitly outstanding.
AGENTS.md reference: AGENTS.md:L123-L129
Useful? React with 👍 / 👎.
| violation before it writes anything. Each refusal is a typed value, not a | ||
| string. |
There was a problem hiding this comment.
For a byte-equal file substitution during publication, the specific refusal is still only a string: filesystem_retention_stage::retention_error maps ExactRecordRefusal to io::Error::new(..., &'static str) (src/adapters/retention/filesystem_retention_stage.rs:94-112), and execute_retention_publication merely wraps that in the generic RetentionPublicationError::Storage phase variant. Consequently callers cannot downcast or match the listed substituted-file condition as a typed refusal—the corresponding test at filesystem_retention_storage_tests.rs:84-102 can assert only InvalidData. Narrow this statement to the current-state refusals that actually carry RetentionCurrentStateRefusal, or preserve the exact typed source.
AGENTS.md reference: AGENTS.md:L71-L74
Useful? React with 👍 / 👎.
1 participant
Why
PR #78 changed what Keep does, and several pages still described the state before it: the version-two overview said production retention publication did not exist, the recovery page said version two was unavailable as production, the closure page called publication integration planned, the reconstruction contract said retention remained planned, and the README said version one was the only store admitted for production.
What
Every page now states one boundary.
Implemented, with the tests that prove it: one-way migration (refusing a version-one store that still holds a retained stage), version-two reopen with root-identity binding and pinned retention directories, forward retention publication that binds this store's catalog head and the catalog it selects, and typed refusals for every namespace, capacity, stage, substitution, and replaced-directory violation.
Planned: retention publication recovery and
KEEP-CRASH-036..052, partial-prefix migration recovery andKEEP-CRASH-053..073, the reader fence, model-based transition evidence (#19); garbage collection (#21); the restart-stable root identity coordinate (#97).Files:
README.md(guarantees bullet, the "not yet" paragraph, gap table row for #97, routing rows for the split pages),docs/formats/segment-store-v2/README.md(Status rewritten),recovery.md,closure.md,docs/invariants/authenticated-reconstruction/README.md,requirements.md(RETENTION-007 and MIGRATION-003 evidence),CHANGELOG.md.Evidence
Full gate chain green locally (fmt, clippy both feature sets, both test suites, doctests,
cargo doc, documentation refusal and integrity, source structure, conformance, golden worldline). The README contract tests and the version-two protocol contract pass; no claim was added that a test does not back.Refs #19 #21 #97