Add crash-tested segment-store recovery - #67
Conversation
|
Important Review skippedToo many files! This PR contains 242 files, which is 142 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (242)
You can disable this status message by setting the 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. Comment |
Code Lawyer activity summary
Final local gate: debug and optimized workspace suites, all-feature and minimal-feature Clippy, repository verification, documentation integrity, formatting, whole-history whitespace, and pure-Rust source checks are GREEN. Graft reports zero breaking changes. @codex review please. Confirm the remediation and flag any remaining correctness, recovery, determinism, API, documentation, or maintainability defect. |
Activity summary — hosted Linux regression
The existing platform documentation already specifies process-scoped exclusive advisory locking on both the pinned root and writer file; this repair restores the implementation to that contract without changing format, API, or recovery semantics. @codex review please |
![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: c47a6f6b45
ℹ️ 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".
Activity summary — Linux root descriptor correction
@codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8514705470
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Lawyer activity summary — review closure
All six unresolved inline review threads were resolved only after the corresponding commits were pushed. Local evidence is green: full debug and optimized workspace suites; debug and optimized 105-case process-death matrices; all-feature and minimal-feature checks and strict Clippy; doctests and MSRV; Golden File Worldline, protocol conformance, pure-Rust source structure, pinned documentation/refusal checks, whole-history whitespace, root/fuzz deny and audit; locked fuzz build/Clippy; and the complete 9-target bounded runtime fuzz smoke with no crash, timeout, swallowed failure, or RSS-limit violation. CodeRabbit skipped because the 244-path PR exceeds its 100-file limit and review credits are unavailable. @codex review please — confirm the current head and flag any remaining correctness, recovery, determinism, API, documentation, or maintainability defect. |
Activity Summary — Hosted Linux Clippy repair
The helper now borrows the result it inspects; production behavior and public contracts are unchanged. @codex review please |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c8937d3fb
ℹ️ 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".
| match self.verify_current(request)? { | ||
| RecoveryNextHeadFinalizationReadiness::Ready => self | ||
| .root() | ||
| .rename(NEXT_HEAD, self.root(), HEAD) |
There was a problem hiding this comment.
If an external filesystem actor replaces head.next after verify_current returns but before this call, rename resolves the name again and publishes the unverified replacement as HEAD; execution then synchronizes the root and returns a receipt without revalidating the published bytes. Bind the atomic replacement to the candidate inode whose evidence was verified so a name-replacement race is refused.
AGENTS.md reference: AGENTS.md:L80-L81
Useful? React with 👍 / 👎.
| pool_directory: &Dir, | ||
| pool_name: &str, | ||
| ) -> Result<RecoveryStagePoolOutcome, RecoveryStageCompletionStorageError> { | ||
| match stage_directory.hard_link(stage_name, pool_directory, pool_name) { |
There was a problem hiding this comment.
If current.seg or current.cat is replaced after link_or_admit fingerprints it but before this name-based hard link, the replacement inode is installed under the digest-derived immutable pool name. The subsequent pool verification detects the mismatch but leaves that canonical coordinate poisoned, so retries cannot link the valid stage without manual intervention. Link the verified opened inode, or otherwise prevent an evidence mismatch from creating the pool entry.
AGENTS.md reference: AGENTS.md:L80-L81
Useful? React with 👍 / 👎.
| let inventory = FilesystemRecoveryInventoryReader::from_root(inventory_root) | ||
| .map_err(|source| FilesystemRecoveryStageDiscardOpenError::Namespace { source })?; |
There was a problem hiding this comment.
When the store contains an unrecoverably ambiguous inventory—for example both current.seg and current.cat, which classify_recovery_names rejects as conflicting stages—this constructor still grants mutation authority because from_root only pins the three directories and never reads or classifies their entries. A caller can therefore discard or complete one stage and destroy evidence while another incompatible history remains; require and bind an ambiguity-free inventory under the writer lock before any filesystem recovery executor can mutate state.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
| state: RecoveryCatalogStage::Complete(catalog), | ||
| .. | ||
| } => RecoveryStageCompletionTarget::Catalog { |
There was a problem hiding this comment.
For a checksum-valid catalog whose entries reference a missing segment or disagree with the named record offsets, classification produces only a ChecksummedCatalog, yet this branch authorizes immutable-pool completion from it. ChecksummedCatalog::decode verifies local framing and integrity but only admit binds entries to admitted segment bytes, so execution can remove current.cat and return a receipt claiming a valid orphan for a catalog that no reader can admit. Require pool-backed transitive catalog admission before producing the completion request.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
1 participant
Closes #17.
Problem and invariant
Keep lacked an executable recovery boundary proving that every named durable transition either reconstructs the exact bytes selected by the current catalog or refuses ambiguous state.
Approach
Adds typed initialization, bounded inventory and classification, explicit discard/completion/head-finalization/resume plans, writer-authorized filesystem adapters, and a deterministic 105-case process-death matrix. CI exercises the matrix in debug and optimized profiles, and the living recovery contract records the evidence and host-power-loss limit.
Heuristic newest-artifact selection, silent truncation or repair, serializer-owned state, and a Python harness were rejected.
Failure modes and recovery
Corruption, resource excess, symlinks, namespace replacement, identity drift, conflicting pools, stale generations, and incomplete evidence produce typed refusals. Recovery transitions are explicit, ordered, retry-safe, and receipt-bearing. Host power loss and torn media remain out of scope.
Validation
cargo test --workspace --all-features --lockedcargo test --workspace --all-features --release --locked-D warningscargo xtask verifycargo xtask documentation-integrity-checkCompatibility, performance, and security
The format bytes remain unchanged; public recovery APIs and
KEEP-CRASH-001throughKEEP-CRASH-035are additive. No benchmark baseline changes or performance claims are included; inventory and fingerprinting remain bounded and streaming. Filesystem adapters use no-follow capability-relative handles, stable identity checks, exclusive writer authority, and the admitted writable non-casefolded Linux ext4 profile.