Skip to content

Fix: refuse corrupt partial segment seals during recovery (#171) - #172

Merged
flyingrobots merged 4 commits into
mainfrom
fix/171-partial-seal-corruption
Oct 3, 2026
Merged

flyingrobots merged 4 commits into
mainfrom
fix/171-partial-seal-corruption

Conversation

@flyingrobots

@flyingrobots flyingrobots commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Problem and result

Closes #171 under audit #131. Version-one recovery previously treated a recognized but incomplete seal as discardable truncation even when its available version bytes contradicted the format. The classifier now checks available fixed framing and preserves precise typed seal refusals through fingerprint-bound assessment, before any discard request can be constructed.

Change kind: bug fix, with a seed-test oracle correction. Current candidate: 07e6cc4875c05592b71bb1f8b9c80631a31bcda5. Independent branch from origin/main 6051abb25a9fd33ae7ee0de5614514b709a4d82a; no unmerged feature dependency.

Invariant and approach

KEEP-RECOVERY-010 requires demonstrated corruption to remain a refusal. A bounded, allocation-free seal-prefix validator checks version, flags, seal length, reserved fields and algorithms using existing format constants. RecoverySegmentStageError::Seal preserves the original seal cause. Complete-seal decoding and the separate v2 incomplete-stage disposition contract remain unchanged.

Missing bytes are used only in temporary fixed-field comparisons, never admitted as evidence. This does not claim future completion feasibility of every variable coordinate. Rejected alternatives: unconditional discard after magic recognition, relabeling every partial stage corruption, and expanding this repair into a new storage format or disposition scheme.

Evidence and remaining acceptance

The two public regressions are committed RED separately at 1b6da984ece52e94777fe1b52dcf1bb6b82c3960, and pass with the fix. The fixed-framing family sweep checks precise refusals across short boundaries and retains typed truncation when the mutated byte is absent. New and adjacent classification/assessment/discard laws pass debug/release in copied Docker trees. Formatting, source structure, all-feature Clippy and Markdown checks pass.

Checked-in evidence and actual receipts distinguish runtime classification from static immutable-borrow preservation; no actual unlink or power-loss experiment is claimed.

The parser fuzz/corpus extension is delivered in f94e4188a715a5962d55343538efa648acefaadf: a retained reduced counterexample is replayed by ordinary tests and the registered segment target; the independent semantic oracle fails on main and passes with the fix, followed by a seeded bounded ASan campaign. Full local validation, all four hosted CI jobs and independent review pass on 07e6cc4. The PR is ready; human merge authorization remains separate. T-13.2's remaining audit obligations are not declared complete.

Compatibility and operational implications

No persistent format, identity, dependency, namespace or write protocol changes. The public error enum gains a typed incomplete-seal variant; exhaustive consumers may need to handle it. Previously admitted contradictory stages now block discard, preserving corrupt evidence. Canonical incomplete stages retain their previous truncation behavior. No performance improvement is claimed; the new checks examine a fixed bounded frame and perform no allocation or I/O. Security impact is refusal of malformed retained input before discard authorization.

Review remediation

The independent review of f94e418 found two acceptance blockers, both addressed in 07e6cc4: line-end whitespace in raw fuzz receipts and missing direct error/coordinate assertion calibration. Checked-in mutation patches and runtime RED/GREEN receipts now calibrate diagnostic values, source preservation and canonical-truncation coordinates. The full suite also exposed frozen seed-count assertions; these are replaced with actual emitted-counterexample replay through Keep and exact typed refusal, while deterministic materialization remains checked. Its emitted-input assertions have their own bounded calibrations. The full local chain passed on 07e6cc4, including debug/release workspace tests, both feature Clippy profiles, conformance/worldline checks, doctests/docs and both debug/optimized process-death campaigns. Independent delta review approves this exact head with its complete checklist. All four hosted jobs pass on exact-head run 37097418652. No earlier CI is transferred.

@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 01c9e0aa-27f9-4bb5-b62b-564952bd87f1
📥 Commits

Reviewing files that changed from the base of the PR and between 6051abb and 07e6cc4.

📒 Files selected for processing (36)
  • CHANGELOG.md
  • docs/formats/segment-store-v1/recovery.md
  • docs/formats/segment-store-v1/requirements.md
  • docs/testing-evidence/partial-seal-corruption.md
  • docs/testing-evidence/partial-seal-corruption/calibration-green.txt
  • docs/testing-evidence/partial-seal-corruption/dropped-source-red.txt
  • docs/testing-evidence/partial-seal-corruption/dropped-source.patch
  • docs/testing-evidence/partial-seal-corruption/framing-green.txt
  • docs/testing-evidence/partial-seal-corruption/fuzz-green.txt
  • docs/testing-evidence/partial-seal-corruption/fuzz-parent-red.txt
  • docs/testing-evidence/partial-seal-corruption/materialized-absent-red.txt
  • docs/testing-evidence/partial-seal-corruption/materialized-absent.patch
  • docs/testing-evidence/partial-seal-corruption/materialized-diagnostic-red.txt
  • docs/testing-evidence/partial-seal-corruption/materialized-green.txt
  • docs/testing-evidence/partial-seal-corruption/materialized-selector-red.txt
  • docs/testing-evidence/partial-seal-corruption/materialized-selector.patch
  • docs/testing-evidence/partial-seal-corruption/parent-red.txt
  • docs/testing-evidence/partial-seal-corruption/recovery-green.txt
  • docs/testing-evidence/partial-seal-corruption/reduced-parent-runtime-red.txt
  • docs/testing-evidence/partial-seal-corruption/wrong-coordinate-red.txt
  • docs/testing-evidence/partial-seal-corruption/wrong-coordinate.patch
  • docs/testing-evidence/partial-seal-corruption/wrong-diagnostic-red.txt
  • docs/testing-evidence/partial-seal-corruption/wrong-diagnostic.patch
  • fuzz/README.md
  • fuzz/fuzz_targets/segment_format.rs
  • src/adapters/recovery.rs
  • src/adapters/recovery/rationale.md
  • src/adapters/recovery/recovery_segment_classifier.rs
  • src/adapters/recovery/recovery_segment_seal_framing.rs
  • src/adapters/recovery/recovery_segment_stage_error.rs
  • tests/fixtures/recovery/README.md
  • tests/fixtures/recovery/unsupported-partial-seal-version.hex
  • tests/recovery_partial_seal.rs
  • tests/recovery_partial_seal/framing_laws.rs
  • xtask/src/fuzz_seed_corpus/segment_seeds.rs
  • xtask/src/fuzz_seed_corpus/tests/materialization.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-07-29T05:54:58.524Z
Learnt from: flyingrobots
Repo: flyingrobots/keep PR: 63
File: xtask/src/golden_file_worldline/b3sum_oracle.rs:15-21
Timestamp: 2026-07-29T05:54:58.524Z
Learning: In the flyingrobots/keep Rust codebase, prefer fallible conversions using `TryFrom`/`try_from` (e.g., `u64::try_from(payload.len())`) instead of potentially lossy `as` casts. If the chosen target architecture makes conversion failure logically unreachable, still keep the `TryFrom`-based conversion per repository policy, and do not require fabricated negative-test cases solely to cover an unreachable defensive failure path.

Applied to files:

  • tests/recovery_partial_seal.rs
🪛 LanguageTool
docs/testing-evidence/partial-seal-corruption.md

[grammar] ~15-~15: Use a hyphen to join words.
Context: ...ment remain repository gaps, not claimed implemented controls. Receipt normaliza...

(QB_NEW_EN_HYPHEN)

🔇 Additional comments (9)
src/adapters/recovery/recovery_segment_stage_error.rs (1)

7-7: LGTM!

Also applies to: 32-36, 66-68, 78-78

src/adapters/recovery.rs (1)

46-46: LGTM!

src/adapters/recovery/recovery_segment_seal_framing.rs (1)

9-29: LGTM!

Also applies to: 32-52, 55-67

src/adapters/recovery/recovery_segment_classifier.rs (1)

71-72: LGTM!

tests/recovery_partial_seal.rs (1)

18-30: LGTM!

Also applies to: 36-52, 55-70

tests/recovery_partial_seal/framing_laws.rs (1)

16-57: LGTM!

Also applies to: 61-202

fuzz/fuzz_targets/segment_format.rs (1)

9-9: LGTM!

Also applies to: 21-21, 70-90, 92-111

xtask/src/fuzz_seed_corpus/segment_seeds.rs (1)

44-45: LGTM!

Also applies to: 57-63

xtask/src/fuzz_seed_corpus/tests/materialization.rs (1)

20-20: LGTM!

Also applies to: 23-23, 52-52, 142-164


Summary by CodeRabbit

  • Bug Fixes
    • Incomplete segment seals with invalid or contradictory framing are now rejected instead of being treated as discardable truncation. Error details identify the specific seal issue.
  • Documentation
    • Clarified how incomplete seals are validated during recovery and documented the regression scenario and its test evidence.

Walkthrough

The recovery classifier now validates available fixed fields in incomplete version-one segment seals. It returns typed seal errors for observed framing contradictions instead of classifying those inputs as truncation. Tests and fuzzing cover the refusal and retain a version-2 counterexample.

Changes

Partial-seal recovery

Layer / File(s) Summary
Validate partial-seal framing
src/adapters/recovery/*, src/adapters/recovery/rationale.md, docs/formats/segment-store-v1/recovery.md, CHANGELOG.md
The classifier checks available fixed fields before returning a truncated-seal result. RecoverySegmentStageError::Seal carries the specific SegmentSealError through its error source. The recovery documentation describes this boundary.
Test refusal and boundary behavior
tests/recovery_partial_seal*, tests/fixtures/recovery/*, docs/testing-evidence/partial-seal-corruption*
Regression tests check version-2 refusal during classification and assessment. A mutation sweep checks fixed-field contradictions across seal-prefix lengths. The requirements evidence now lists these tests.
Replay the counterexample in fuzzing
fuzz/*, xtask/src/fuzz_seed_corpus/*, docs/testing-evidence/partial-seal-corruption/*
The recovery fuzz path checks truncated-seal coordinates and available fixed fields. Seed materialization verifies that the retained counterexample reaches the recovery classifier and returns the expected unsupported-version error. The documentation records test and fuzz run results and evidence limits.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 07e6c

The change appears ready to merge after normal checks. It refuses observed corruption in incomplete seals without changing canonical truncation behavior.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 07e6c

The change prevents damaged data from being treated as safely discardable. The demonstrated behavior is narrowly scoped and strengthens protection against deletion, but full failure and concurrent-operation behavior has not been established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated security-relevant scope is caller-supplied version-one segment-stage bytes and authorization to discard the corresponding evidence-matched fixed stage. The change narrows that authorization for contradictory partial seals. Actual deployment, tenant and environment exposure is not established.

Trust Boundaries and Controls

  • observed — Admission verifies stage identity, exact length and a recomputed fingerprint before semantic assessment. Matching evidence does not override a seal refusal. Discard requests retain that evidence, and the concrete removal path compares fresh evidence and verifies namespace and observed-entry identity before removal.

Resilience and Maintainability Implications

  • observed — The existing discard executor synchronizes the selected parent after removal or admitted absence and returns no receipt on either failure. A repeated request against an absent stage still requires synchronization. This ordering remains unchanged; it does not establish atomic unlink durability or safety against every concurrent namespace mutation.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files. (27 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: refusing corrupt partial segment seals during recovery.
Description check ✅ Passed The description covers the problem, invariant, approach, change kind, rejected alternatives, compatibility, recovery and security implications, tests, and testing evidence. It does not follow every te…
Linked Issues check ✅ Passed #171 requires rejection of observed fixed-framing contradictions in incomplete version-one seals before discard authorization. The classifier validates available framing fields and preserves the typed…
Out of Scope Changes check ✅ Passed The reported source changes implement #171’s incomplete-seal classification and error propagation. Regression tests, fuzz coverage, corpus materialization checks, and format/evidence documentation sup…
Full details: Docstring Coverage

Explanation

Docstring coverage is 19.05% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 9 files. (27 skipped: 27 unsupported.)

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A short seal shows its fields in view
Each fixed byte is checked as due
A mismatch keeps its typed error
A seed returns to test the parser
Prefixes pass, or fail with proof

Comment @coderabbitai help to get the list of available commands.

@flyingrobots

Copy link
Copy Markdown
Owner Author

Verified acceptance blockers at f94e418

Severity Location Finding and evidence Acceptance
P2 docs/testing-evidence/partial-seal-corruption/fuzz-green.txt:6 and fuzz-parent-red.txt:42 Six libFuzzer INFO lines contain trailing spaces, failing the required documentation job 111128480162. Remove line-end whitespace, disclose normalization, pass the actual empty-tree diff check.
P2 tests/recovery_partial_seal.rs and tests/recovery_partial_seal/framing_laws.rs Parent RED reaches the missing-refusal checks before exact diagnostic/source assertions; no supplied mutation yet demonstrates those assertions or the unobserved-prefix coordinate assertion failing. Independent reviewer confirmed this evidence gap. Compile and observe bounded wrong-diagnostic, dropped-source and wrong-coordinate mutations failing their intended assertions; restore and verify.

These are static/evidence blockers, not newly demonstrated product defects. @codex

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

@flyingrobots

Copy link
Copy Markdown
Owner Author

Independent adversarial review — PR #172

Reviewed in an isolated checkout at exact pushed head f94e4188a715a5962d55343538efa648acefaadf, branch fix/171-partial-seal-corruption, against main 6051abb25a9fd33ae7ee0de5614514b709a4d82a. This is the authorized independent Codex fallback using the complete agy-review protocol. The review covers issue #171's version-one fixed-seal-framing correction, not version-two retention disposition or universal future-prefix feasibility.

Findings

P2 — Exact diagnostics/source preservation and canonical-truncation assertions lack direct falsification evidence. tests/recovery_partial_seal.rs:23 and :46 assert the exact nested SegmentSealError, and tests/recovery_partial_seal/framing_laws.rs:41 asserts the independently specified precise cause; framing_laws.rs:47 asserts exact truncation coordinates when the mutated byte is absent. The supplied parent REDs stop earlier at the missing-refusal ok_or checks. The reduced sweep stops at offset=16, observed=17: corrupt seal admitted before its diagnostic comparison. The fuzz parent RED reaches the fixed-byte contradiction assertion, but does not establish these diagnostic/source or truncation-coordinate assertions can fail. Thus the known product regression is reproducible, while the separate mandatory Testing Standards rule 4 evidence for these load-bearing assertions is missing. The author confirmed no additional calibration receipts exist. Add bounded producer mutations preserving refusal while changing its diagnostic value or dropping the Seal error source, and changing a returned truncation coordinate while retaining truncation. Record the intended assertions executing and failing, then restore and validate; a coherent campaign is sufficient without per-byte implementation cycles. This is an evidence blocker, not a demonstrated defect in the current source mapping.

P2 — Trailing whitespace in fuzz receipts fails required documentation CI. docs/testing-evidence/partial-seal-corruption/fuzz-green.txt:6, :7, :20, :21, and fuzz-parent-red.txt:42, :43 retain trailing spaces in libFuzzer INFO lines. Read-only empty-tree-to-HEAD git diff --check reproduces the exact warnings. Hosted final-head Documentation and workflow integrity job 111128480162, run 37096872175, fails that step with those diagnostics and exit 2. Strip only line-end whitespace, disclose that normalization alongside prefix/EOF normalization, and rerun the required gate. Preserve raw output separately and do not weaken the check.

No additional concrete production-correctness violation was found within the approved scope.

Verification Checklist

  • Exact identity, scope and merge integration: Local and live GitHub head/base agree; checkout is clean. Inspected all 22 changed files and the three commits: test-only parent 1b6da984ece52e94777fe1b52dcf1bb6b82c3960, correction 43820f5, and reduced fixture/fuzz extension f94e418. No merge commits occur. The public error-enum addition is disclosed as potentially affecting exhaustive consumers; no persistent format, namespace, dependency or write protocol changes. No dependency on unmerged Calibrate writer authority against lock-entry replacement during acquisition #169/Replace catalog scan source-text assertion with runtime cost evidence #168 or expansion into Retention recovery, crash-matrix evidence, reader fence, and model-based transitions (item 6) #99 is asserted.
  • Direct production classification: src/adapters/recovery/recovery_segment_classifier.rs:24 first bounds metadata and admits the segment header; :49 iterates verified records with checked record counts/offset arithmetic. At :69, recognized seal magic plus fewer than 128 bytes selects the new validator at :71, whose failure becomes RecoverySegmentStageError::Seal before the old truncation constructor at :73. Empty tail remains reusable at :66; complete-looking seals still use full AdmittedSegment::decode at :81; partial record/header admission remains at :85 and cursor errors at :116. The new check cannot bypass prior header/record admission or admit a complete segment from padded bytes.
  • Seal discriminator and parallel full decoding: recovery_segment_classifier.rs:150 recognizes exact seal magic, with the existing fixed-width invalid-magic fallback for complete seals. Before full magic is present, recovery_segment_fixed_framing.rs:19 preserves canonical seal-magic prefixes or validates record framing. Full segment_seal_decoder.rs:26 and segment_seal_admission.rs:27 retain strict length, field, coordinate, digest and checksum validation. The new partial validator intentionally covers fixed known framing, not variable coordinates/digest feasibility; complete admission remains the stronger path. This difference is documented and matches issue scope.
  • Fixed fields and constants: recovery_segment_seal_framing.rs:9 validates version at offset 16, flags 18, encoded seal length 20, reserved u16 22, reserved u32 28, checksum algorithm 56, digest algorithm 57 and six reserved bytes 58–63. Compared each with segment_seal.rs:5, the full decoder's sequential field layout, full admission, and docs/formats/segment-store-v1/segment.md:144 field table. Expected values 1, 0, 128 and algorithm 1 agree. recovery_fixed_field_prefix.rs:3 overlays available bytes on a small canonical comparison array; missing bytes produce no contradiction and are not admitted as evidence. Reads are bounded slice operations; temporary arrays are fixed-size, with no new allocation, I/O or externally influenced unchecked arithmetic.
  • Assessment, continuation and discard: recovery_stage_assessor.rs:18 dispatches the exact fingerprint-bound segment bytes to this classifier at :26, preserving the typed error through RecoveryStageAssessmentError::Segment; its Error source remains intact. recovery_stage_discard_planner.rs:15 accepts a segment only through its Truncated arm at :21; the new refusal therefore prevents this ordinary assessment-to-discard pipeline from producing a request. recovery_segment_resume_executor.rs:31 re-admits evidence and reassesses at :34 before allowing a reusable writable stage. Catalog and next-head dispatch are unchanged and do not share the segment grammar. Production-crash tooling also calls these same boundaries at xtask/src/durability_crash_matrix/production_protocol/recovery.rs:111 and restart/semantic.rs:59; these are tools, not separate product evidence.
  • Typed errors and evidence preservation: recovery_segment_stage_error.rs:32 adds the dedicated Seal cause, its Display at :66 identifies incomplete-seal refusal, and Error::source at :78 returns the actual SegmentSealError. The classifier performs read-only borrowed-slice validation, and assessment does not mutate input. No actual unlink or crash-state transition is claimed by the regression. Error source/diagnostic code is statically correct as inspected; its missing direct calibration is reported separately above.
  • Permanent reported counterexample: tests/recovery_partial_seal.rs:18 reaches the public classifier; :37 fingerprints the same bytes, admits them and reaches the public assessor. Both parent runtime REDs compiled and failed at the intended missing-refusal outcomes. tests/fixtures/recovery/unsupported-partial-seal-version.hex contains exactly 82 decoded bytes; independently checked it equals the first 82 bytes of the canonical empty-segment vector with byte 81 changed to version 2. Fixture README states its corrupt-regression status and deletion criterion. The reduced parent receipt preserves both public failures. Earlier compiler/setup failures are retained in scratch and excluded from runtime RED.
  • Finite family sweep: tests/recovery_partial_seal/framing_laws.rs:25 applies a one-bit contradiction at each of 20 independently listed fixed-field byte offsets, then visits every observed seal length 16 through 127. Each table value agrees with big-endian canonical completion (including 257, 384, 16,777,216 and the six reserved-byte positions). This produces 2,240 finite cases on a successful full sweep; it is not arbitrary-value coverage. Observed contradictions require exact causes; absent mutations require typed exact truncation. Checked offset arithmetic and bounded fixture access prevent the test from creating an accidental out-of-range oracle. Parent RED does not execute the complete family after its first failure, as the evidence finding explains.
  • Fuzz boundary and semantic oracle: fuzz/fuzz_targets/segment_format.rs:21 adds selector 5; :70 calls the actual recovery classifier and, for returned seal truncations, checks addressable/in-bounds offset, observed length, required 128, shortness and independent fixed-byte expectations at :92. The byte table duplicates specification literals rather than importing the production validator. xtask/src/fuzz_seed_corpus/segment_seeds.rs:44 adds the canonical complete-empty recovery seed and :57 reads the permanent reduced counterexample with a bounded canonical hex decoder and selector prefix. Existing selectors remain unchanged except that selector 5 now selects recovery explicitly; ordinary complete-segment seeds retain selector 4. The target remains registered at fuzz/Cargo.toml:142; CI prepares corpus and runs registered smoke targets at .github/workflows/ci.yml:150/:156.
  • Fuzz execution evidence and limits: Inspected the actual parent assertion discardable seal contradicts fixed field at 16, observed 2 versus expected 1, after successful instrumented compilation. Corrected fixed-input replay completes, followed by seed 17101 with max_total_time=15, timeout 5, RSS 1024 MiB and max input 1,048,576 bytes. The receipt ends after 3,110,375 runs in 16 seconds; configured time is a fuzzing bound, not a measured hard 15-second completion promise. Fixed-input replay's own generated seed is distinct from the explicit campaign seed. Instrumented build/Clippy success is retained in fuzz-compiled.log; earlier temporary compile/lint failures are not credited as semantic RED. The finite smoke is not exhaustive parser proof, universal memory isolation, allocation measurement or physical-power-loss evidence.
  • Raw receipt integrity and restored checks: Compared all six checked-in receipts (parent-red, reduced-parent-runtime-red, framing-green, recovery-green, fuzz-parent-red, fuzz-green) with raw scratch logs under the documented build-prefix and EOF normalization; all match. Recovery GREEN covers the three new laws plus 15 adjacent classification, six assessment and seven discard laws in debug/release. Final framing GREEN preserves debug/release new-law and all-feature Clippy completion. The full final validation is a separate running process, not inferred green from these focused results. Static formatter/structure/Markdown success is author-supplied execution evidence, not a reviewer test run.
  • Documentation and numerical claims: Reviewed CHANGELOG, recovery/requirements changes, both new rationale/fixture README files, the full evidence page and fuzz README. The fixed-framing guarantee is scoped accurately and the future-completion limit explicit. The reduced 82-byte claim, 128-byte frame, version 2 corruption, field values and replay budgets match source/artifacts. No new performance, throughput, recovery-time or resource-ceiling measurement is claimed. Public error compatibility is disclosed in the PR. The PR body still describes the fuzz extension as pending despite its presence; refresh that status when publishing the next result rather than treating stale text as completed evidence.
  • Repository standards: Applicable AGENTS, binding Testing Standards and enforcement profile were applied. New tests name small in-memory size, specified oracle and deletion criteria. The validator owns a narrow format-boundary concept with a colocated rationale, concrete typed errors and inward-compatible dependencies. No forbidden generic helper module, unsafe code, unchecked external arithmetic, async, global mutable state, sleep or public boolean argument is added. New parser reachability has a registered live fuzz path and retained counterexample. Per-test resource/egress gaps remain explicit. The two findings concern actual missing mandatory evidence and a failing required gate, not speculative unrelated hardening.
  • Hosted feedback and pagination: Queried exact PR/issue bodies, review bodies, global comments, inline threads and pageInfo. All connections ended with hasNextPage: false; there were no review bodies or inline threads. The sole global comment states CodeRabbit skipped the draft; its status is not an approval. Later feedback still requires reconciliation.

Execution and remaining gates

Executed only read-only source/Git/GitHub inspection, raw-log comparisons, fixture-byte verification and the whitespace check. No host Rust/Python, product tests, mutation replay, configuration changes, external comments, source edits or delegation. The report is the only written artifact. The active copied final source/target tree was not modified or used for tests.

Final-head dependency policy passed; documentation/workflow integrity failed as above; Rust quality and runtime fuzz smoke remained in progress at inspection. The local final-f94e418-validation.log was still running. No previous CI result transfers to this head. Human merge authorization remains a separate gate after correction, validation and fresh exact-head review.

REQUEST CHANGES — f94e4188a715a5962d55343538efa648acefaadf

@flyingrobots

Copy link
Copy Markdown
Owner Author

Independent adversarial delta review — PR #172

Reviewed in an isolated checkout at exact pushed head 07e6cc4875c05592b71bb1f8b9c80631a31bcda5, branch fix/171-partial-seal-corruption, against main 6051abb25a9fd33ae7ee0de5614514b709a4d82a. This authorized independent Codex fallback follows the agy-review protocol. The review covers the complete remediation delta from f94e418 and retains the prior full production-path inspection where source is identical.

Findings

Both prior P2 findings are closed. The new mutation receipts directly reach the exact diagnostic/source and canonical-truncation assertions; receipt whitespace now passes the exact required check. The additional seed-materialization law verifies the emitted counterexample through Keep rather than asserting corpus cardinalities, and its named-input, selector and diagnostic outcomes have direct runtime calibration. No verified remaining source or scoped acceptance blocker was found.

Verification Checklist

  • Exact head, scope and merges: Local and live GitHub head/base match the full SHAs above; checkout is clean. Inspected all 17 delta files in commit 07e6cc4. No production src or fuzz-target source changes occurred after the reviewed correction; the only Rust delta is xtask/src/fuzz_seed_corpus/tests/materialization.rs. No merge commits, dependencies, format/API changes or unrelated feature work were introduced. The PR body accurately declares bug fix plus seed-test oracle correction and names the resulting candidate.
  • Direct classification path retained: src/adapters/recovery/recovery_segment_classifier.rs:24 admits metadata/header; :49 walks checked records; :69 selects recognized partial seals; :71 validates fixed framing and wraps the cause before :73 returns truncation. Reusable :66, complete decoder :81, partial-record handling :85 and checked cursor logic remain byte-identical to the full review. recovery_segment_seal_framing.rs:9, :32 and :55 still implement the correct fixed-field comparisons using existing constants and bounded temporary arrays. No unavailable byte becomes admitted evidence.
  • Parallel decoding, assessment and discard: Full decoding remains segment_seal_decoder.rs:26 → segment_seal_admission.rs:27; shorter-than-magic handling remains recovery_segment_fixed_framing.rs:19. Fingerprint-bound recovery_stage_assessor.rs:18 dispatches through the same classifier at :26; recovery_stage_discard_planner.rs:15 accepts only the permitted truncation variants. recovery_segment_resume_executor.rs:31 re-admits evidence and reassesses at :34 before writable continuation. Catalog/next-head paths remain distinct and unchanged. Tool callers at xtask/src/durability_crash_matrix/production_protocol/recovery.rs:111 and restart/semantic.rs:59 also reuse the classifier; tool execution is not substituted for product evidence.
  • Errors and state: recovery_segment_stage_error.rs:32, :66 and :78 preserve the typed Seal variant, display boundary and original source. Assessment retains the original cause. Read-only borrowed classification refuses before an ordinary assessment-to-discard request can be produced. No new I/O, mutation, lock, cancellation, restart, shutdown or durability behavior exists in this delta. Variable-coordinate future completion and Retention recovery, crash-matrix evidence, reader fence, and model-based transitions (item 6) #99 version-two retention policy remain outside the bounded fix.
  • Diagnostic calibration closed: wrong-diagnostic.patch changes only the reported observed u16 value while retaining refusal. Its runtime RED compiles and fails the exact comparisons in both public laws (tests/recovery_partial_seal.rs:23 and the assessment assertion) and the family sweep (framing_laws.rs:41). The observed incorrect value 1 is distinguished from the independent expected 2/257. This now reaches the checks that the parent missing-refusal RED could not reach.
  • Source calibration closed: dropped-source.patch changes RecoverySegmentStageError::Seal source reporting to None without changing the refusal. Its compiled runtime RED reaches those same exact source assertions with None versus the required typed cause. The public classifier and fingerprint-bound assessment witnesses both execute, directly establishing preservation across both layers.
  • Truncation calibration closed: wrong-coordinate.patch retains seal truncation but changes observed length to zero. The two corrupt-input laws remain green while the sweep fails at framing_laws.rs:47, offset 16/observed 16, where the mutated byte is absent. Thus failure is the intended exact canonical-truncation assertion, not a setup problem or missing refusal. calibration-green.txt then records restored three-law debug/release success and workspace Clippy completion.
  • Seed-materialization repair: xtask/src/fuzz_seed_corpus/tests/materialization.rs:49 invokes real prepare, reads actual emitted bytes and verifies the named counterexample at :52. The helper at :143 requires the emitted filename, nonempty input, selector 5, real Keep classifier refusal, exact Seal boundary and UnsupportedVersion { expected: 1, observed: 2 }. It does not regenerate its expected bytes through the producer. The original complete emitted-byte idempotence check remains at :54, using deterministic BTreeMap collection at :167. Owned filesystem scratch, copied source fixtures and existing preparation semantics remain intact.
  • Deletion criterion and first failure: The previous full run's actual failure at the frozen total was inspected: 49 observed versus 47 expected, with 303 other xtask tests passing. Removing frozen total/per-target counts does not claim every possible corpus membership property is now proved. The documented correction changes the protected oracle to an explicit, nonvacuous emitted recovery artifact plus unchanged whole-corpus byte idempotence. It adds no production seed-tool behavior and does not require unrelated seed-tool remediation. The original failed run remains preserved.
  • Materialized-input calibration: Inspected all three producer mutations and runtime RED outputs. Renaming the emitted counterexample causes the explicit missing-input failure; selector 4 instead of 5 reaches the selector assertion; the production wrong-diagnostic mutation reaches the exact runtime cause assertion with 1 versus 2. Each admitted receipt executes exactly the named law, not zero selected tests. Restored materialized-green.txt records one actual named test passing debug/release and workspace Clippy. Earlier zero-selection attempts are explicitly excluded, not credited as GREEN or RED.
  • Portable replay and raw evidence: All five checked-in mutation patches pass read-only git apply --check against this head. Compared all eight newly supplied output files with raw scratch logs after the disclosed build-prefix/line-end/EOF normalization; all match. The calibration scripts retain originals/mutants, restore each producer and touch source timestamps after restoration. Commands and intended failures are recorded in the evidence page. No mutation remains in tracked executable source.
  • Fuzz and existing regression evidence retained: The unchanged 82-byte reduced counterexample, public classifier/assessment parent RED, 20-offset by 112-boundary fixed-framing sweep, selector-5 semantic fuzz oracle at fuzz/fuzz_targets/segment_format.rs:70, seed production at xtask/src/fuzz_seed_corpus/segment_seeds.rs:44/:57, and registered target/CI route remain as verified in the full review. The earlier fixed replay and seed-17101 bounded campaign remain historical evidence; their diagnostic output was changed only by disclosed whitespace normalization. No full-campaign rerun on this head is inferred.
  • Constants and documentation: Checked the unchanged v1 field values/offsets against the full review and all new numeric/error claims against raw results. The 49/47 failure, three diagnostic/source/coordinate mutations, three emitted-input mutations, and one selected restored materialization law are supported. The five patch files intentionally share the diagnostic patch between two campaigns. No new performance, timeout, memory, buffer or numeric budget is introduced. Existing fuzz limits and observed historical durations are not elevated to universal guarantees. The updated evidence distinguishes actual product behavior, tool-to-runtime replay, calibration, static validation and execution limits.
  • Required whitespace gate closed: Executed empty-tree-to-HEAD git diff --check; it passes. Reviewed the exact six line-end corrections and the evidence-page disclosure. Current-head hosted Documentation and workflow integrity has also passed, independently confirming closure of the previous failed gate.
  • Repository standards: Applicable AGENTS, Testing Standards and enforcement profile remain binding. The replacement tool law declares medium size, runtime oracle and deletion criterion; it uses the existing owned scratch fixture. Production allocation/checked-arithmetic/inward-boundary properties from the full review are unchanged. Per-test ceilings and denied-egress gaps remain disclosed. No sleeps, global hooks, unsafe code, format changes or speculative hardening are added. The new evidence meets the bounded assertion-calibration obligations identified in the previous review.
  • Review surfaces and pagination: Refreshed live PR body, review bodies, global comments, inline threads and pageInfo. Every connection reports hasNextPage: false; there are no inline threads/reviews with new findings. Global discussion contains the earlier blockers, the full review and tooling status. CodeRabbit skipped the draft, and hosted Codex reported unavailable repository environment; neither is treated as an approval. Subsequent feedback still requires reconciliation.

Execution and remaining gates

Executed read-only Git/source/receipt inspection, patch applicability, exact whitespace check and live GitHub queries. No host tests, mutation execution, Python, source changes, configuration changes, external comments, delegation or merge. Only this report was written. The running final copied source/build tree was not touched.

Inspected restored debug/release and Clippy receipts for both the original public laws and the replacement materialization law. Structure/format execution remains author-supplied evidence rather than a reviewer rerun. Final 07e6cc4 local validation was still running. On exact-head run 37097418652, documentation/workflow integrity and dependency policy passed; Rust quality and runtime fuzz smoke remained in progress. Earlier CI green is not transferred.

This source/evidence approval does not establish universal future-prefix feasibility, all recovery fault schedules, actual deletion preservation, physical power loss, or complete ambient isolation. Required final checks, later review feedback and human merge authorization remain separate gates.

APPROVE — 07e6cc4875c05592b71bb1f8b9c80631a31bcda5

@flyingrobots

Copy link
Copy Markdown
Owner Author

Activity Summary — ready candidate 07e6cc4

Obligation Evidence / disposition
#171 corrupt partial seal framing can authorize discard Fixed in 43820f5; separately committed parent RED 1b6da98; precise public classifier and assessment refusals, canonical-prefix boundary sweep.
Recovery parser/corpus coverage f94e418 adds the retained reduced input and semantic fuzz oracle; actual parent assertion RED and corrected replay/seeded ASan GREEN are checked in.
Review P2 exact diagnostic/source and coordinate calibration 07e6cc4; three independently violated production outcomes reach their exact assertions, then restored debug/release GREEN. Closed.
Review P2 receipt whitespace 07e6cc4; disclosed line-end normalization, empty-tree diff check and final documentation CI pass. Closed.
Full-suite seed-count failure 07e6cc4 replaces frozen cardinalities with actual emitted-input replay through Keep and exact refusal, retains repeated-run byte equality, and calibrates the emitted-input checks. Closed.

Independent Codex review using the agy-review protocol approves this exact head with a complete Verification Checklist: #172 (comment).

All four binding hosted jobs pass on this head: https://github.com/flyingrobots/keep/actions/runs/37097418652. The copied-Docker full local chain also passes: formatting, structure, conformance/worldline, both feature Clippy profiles, debug/release workspace tests, doctests/docs, and debug/optimized production process-death campaigns. Earlier failures remain recorded; they are not retried into evidence of unchanged correctness.

The final review queue has no inline threads, active changes-requested reviews or unresolved verified findings. CodeRabbit skipped the draft and is not counted as approval; marking ready may trigger additional review, which will be reconciled. Human merge authorization remains separate. No merge performed.

Separate audit issue #173 addresses an existing duplicate-identity invariant bypass across incomplete tails. It is outside this fixed-framing correction and is not presented as solved by this PR.

@flyingrobots
flyingrobots marked this pull request as ready for review October 3, 2026 17:08
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@flyingrobots

Copy link
Copy Markdown
Owner Author

Independent Code Lawyer landing review — PR #172

Repository: flyingrobots/keep. Branch: fix/171-partial-seal-corruption. Reviewed head: 07e6cc4875c05592b71bb1f8b9c80631a31bcda5. Target: main, base 6051abb25a9fd33ae7ee0de5614514b709a4d82a. Candidate tree: af00023bb50da6b5f390fc8f2e4b070bf15ceb52.

This is a fresh, independent, read-only Code Lawyer review using the complete agy-review protocol, performed by the user's authorized Codex substitute. It reviews the entire 36-file base-to-head change. Previous approvals were treated as claims, not transferred to this verdict.

Findings

No verified remaining P0–P5 defect or PR-scoped acceptance blocker was found. Both previous P2 findings are closed by independently inspected evidence: exact error/source and canonical-truncation assertions now have direct runtime calibration, and the required repository whitespace check passes.

The existing duplicate-identity/incomplete-tail defect tracked by #173 remains outside this correction. This review does not represent it as fixed or certify the entire recovery subsystem. The changed validator establishes the promised available-fixed-framing refusal; it does not establish canonical future completion for variable coordinates, digests, or checksums.

Verification Checklist

Exact source, history, and merge audit

  • Local HEAD and live GitHub head/base match the full SHAs above. The checkout was clean on entry and remained clean after inspection. Live GitHub reports MERGEABLE / CLEAN. Fetch was omitted to honor the read-only checkout constraint; live remote coordinates were verified directly.
  • Audited all four first-parent commits: 1b6da984ece52e94777fe1b52dcf1bb6b82c3960 introduces the separately committed regression; 43820f50583635746ea75d083a523b2c21ad9428 fixes classification/error propagation; f94e4188a715a5962d55343538efa648acefaadf adds reduced input and semantic fuzzing; 07e6cc4875c05592b71bb1f8b9c80631a31bcda5 adds calibration receipts and the emitted-input oracle correction. There are no merge commits inside the PR range and no conflict-resolution or rerouted-merge delta to overlook.
  • The incoming Retention recovery, crash-matrix evidence, reader fence, and model-based transitions (item 6) #99 integration is already in the base: 6051abb25a9fd33ae7ee0de5614514b709a4d82a, parents 379b24141bbcc4b883bd169e746ed76c77f7699c and 29067f7d20f078553a0e40a1f2f237a60ade5781. Compared the merge against both parents: it imports the retention feature against the first parent and has no tree difference from the second parent. This PR changes no retention source. The relevant imported invariants were traced below; this is not a fresh certification of every unchanged Retention recovery, crash-matrix evidence, reader fence, and model-based transitions (item 6) #99 file.

Every runtime path and boundary

  • Direct public classification: src/lib.rs:129 and src/adapters/recovery.rs:132 expose recovery_segment_classifier.rs:24. Metadata and complete header are admitted at :28–:46. The cursor uses checked record-count/offset arithmetic at :178–:212. Recognized seals at :69 pass the new validator at :71 before Truncated::Seal at :73. A framing contradiction returns RecoverySegmentStageError::Seal before a lawful assessment is produced.
  • New validator: src/adapters/recovery.rs:46 registers recovery_segment_seal_framing; that module's :9, :32, and :55 compare only available fixed fields. recovery_fixed_field_prefix.rs:3 uses bounded arrays and slice access; absent bytes retain canonical comparison values without being admitted or persisted. There is no input-sized allocation, I/O, global state, unchecked input arithmetic, or mutation in this path.
  • Canonical short prefixes and complete seals: recovery_segment_classifier.rs:150 recognizes the full seal discriminator. Prefixes shorter than the discriminator retain the existing recovery_segment_fixed_framing.rs:18 path. Recognized canonical prefixes of lengths 16–127 retain exact offset/required/observed truncation coordinates. Complete seals use recovery_segment_classifier.rs:81 → AdmittedSegment::decode, retaining full immutable-segment admission. The parallel seal API remains segment_seal.rs:44 → segment_seal_decoder.rs:25 → segment_seal_admission.rs:27. Compared fixed field offsets, values, and error variants on both paths. The complete path additionally validates counts, derived lengths, digest, and checksum; that intentional distinction is documented and does not weaken complete admission.
  • Header/record/reusable paths: recovery_segment_classifier.rs:34, :66, :85, and :103 retain existing header refusal, reusable identity-index admission, record-prefix validation, and policy limits. The changed seal branch does not change those paths. The pre-existing Preserve duplicate-record refusal across incomplete recovery tails #173 limitation is retained explicitly.
  • Fingerprint-bound assessment: recovery_stage_byte_admission.rs:17 checks stage, length, metadata, and recomputed fingerprint before admitting bytes at :55. recovery_stage_assessor.rs:19 dispatches segment bytes to the same classifier at :26; catalog and next-head dispatch at :30/:33 remain separate. recovery_stage_assessment_error.rs:44 preserves the underlying classifier error. Fingerprint matching does not sanitize a framing contradiction.
  • Error chain: recovery_segment_stage_error.rs:32 exposes the documented typed Seal boundary, :66 renders that boundary, and :78 returns the original SegmentSealError as source. The assessment layer retains that error once, rather than replacing it with a string or swallowing it. Both public tests traverse the actual error chain. The public enum addition can require exhaustive consumers to update; the PR explicitly discloses that compatibility effect.
  • Discard planning and execution: recovery_stage_discard_planner.rs:16 only derives a request from permitted truncation variants at :20–:45. The new refusal prevents the ordinary classification → assessment → discard-plan flow for contradictory framing. Existing recovery_stage_discard_executor.rs:17 removes exact matching evidence, synchronizes the selected parent, and only then returns a receipt. filesystem_recovery_stage_discarder.rs:78 retains writer authority; filesystem_recovery_stage_discard_storage.rs:89 revalidates namespaces/evidence/entry before unlink, while :29 synchronizes the parent. These mutation paths are unchanged; the new regression tests do not claim an actual unlink experiment.
  • Resume and completion: recovery_segment_resume_executor.rs:32 re-admits materialized bytes and reassesses at :34 before returning writable continuation; only Reusable is allowed at :36. recovery_stage_completion_planner.rs:19 requires Complete at :24. Contradictory short seals do not acquire either authority through the ordinary flow.
  • Repository crash/restart consumers: xtask/src/durability_crash_matrix/production_protocol/recovery.rs:109 → :111 → :113 admits, assesses, and plans through the same production APIs. xtask/src/durability_crash_matrix/restart/semantic.rs:59 directly uses the public classifier. There is no separate seal-classification implementation in these callers. Tool execution is not substituted for the public runtime regression.
  • Retention recovery, crash-matrix evidence, reader fence, and model-based transitions (item 6) #99 v2 retention separation: src/adapters/retention/recovery_stage_assessment.rs:52, :70, and :91 preserve their own decoder/prefix-admission paths. recovery_planner.rs:32 preserves known corruption and :45, :57, and :69 refuse incomplete stages with IncompleteStageRequiresDisposition before mutation. recovery_execution.rs:98 stops at the first error, preserves completed-step history and original source, and exposes reported effects through :47. None of these paths, cooperating-writer authority, or post-effect uncertainty reporting is changed or routed into v1 discard by this PR.

Regression, generated family, fuzz, and tool evidence

  • tests/recovery_partial_seal.rs:18 and :36 test real public classifier and fingerprint-bound assessment refusals. The retained input is consumed at :55, and typed causes are inspected at :61. The independently specified outcome is UnsupportedVersion { expected: 1, observed: 2 }.
  • tests/recovery_partial_seal/framing_laws.rs:16 sweeps all 20 listed fixed-byte contradiction positions across the 112 recognized short lengths 16–127: 2,240 cases on a successful sweep. Every observed mutation requires its specified exact error at :41; every absent mutation requires exact typed truncation at :47. Checked arithmetic and bounded fixture access retain replay coordinates. The parent sweep fails at its first witnessed defect; it is not evidence that all 2,240 parent cases executed.
  • tests/fixtures/recovery/unsupported-partial-seal-version.hex:1 is exactly 82 bytes. Independently checked its first 80 bytes against the canonical empty segment's header plus magic and its final bytes as [0, 2]. tests/fixtures/recovery/README.md:3 correctly scopes it as a reduced corrupt counterexample rather than a canonical vector.
  • fuzz/fuzz_targets/segment_format.rs:21 selects recovery using selector 5; :70 invokes the real classifier and checks truncation coordinates/bounds before :92 applies an independently literal fixed-byte table. Existing 0–3 selectors and ordinary selector-4 complete-segment seeds remain valid. fuzz/Cargo.toml retains the registered target, and the CI fuzz job prepares, builds, and exercises every registered target.
  • xtask/src/fuzz_seed_corpus/segment_seeds.rs:44 emits canonical recovery input; :57 reads the retained corrupt input through bounded hex admission and adds selector 5. xtask/src/fuzz_seed_corpus/tests/materialization.rs:49 executes the real producer, :52 checks its actual emitted artifact, and :143 verifies filename, nonempty input, selector, exact runtime error boundary, and expected/observed cause. Whole-corpus emitted-byte idempotence remains at :54; BTreeMap collection at :167 prevents enumeration order from changing comparison behavior.
  • Frozen corpus counts were removed as an explicitly justified oracle correction, not silently rebased. Inspected the preserved earlier full-run failure, 49 observed versus 47 expected. The replacement adds a protected tool-to-runtime outcome, while no claim is made that it proves every possible corpus-membership property. partial-seal-corruption.md:47 records the deletion criterion and retained idempotence check.

Raw receipts, calibration, and every changed numerical claim

All 14 checked-in .txt receipts were compared with their supplied raw logs. Every comparison matched after only the disclosed isolated-build-prefix, line-end, and EOF normalization. The five checked-in replay patches all pass read-only git apply --check against this exact head. They are evidence files, not active executable mutations.

Receipt(s) under docs/testing-evidence/partial-seal-corruption/ Raw evidence and verified outcome
parent-red.txt parent-red.log: successful compilation, then both original public regressions fail because unsupported framing receives truncation/assessment. Separately committed RED source is 1b6da984ece52e94777fe1b52dcf1bb6b82c3960, with unchanged base production.
reduced-parent-runtime-red.txt Same-named raw log: the reduced public witnesses and family law execute and fail on unfixed production. Setup/compiler failures are not credited.
recovery-green.txt, framing-green.txt recovery-green.log, final-framing-green.log: restored new and adjacent recovery laws, debug/release, and relevant Clippy completion. Counts agree with executed tests.
fuzz-parent-red.txt, fuzz-green.txt Same-named raw logs: parent semantic assertion observes 2 versus expected 1 at fixed field 16; corrected fixed-input replay completes, followed by seed-17101 bounded exploration.
wrong-diagnostic-red.txt, dropped-source-red.txt, wrong-coordinate-red.txt Respective mutation-directory red.log: false diagnostic values and removed typed sources reach exact assertions; zero observed-length mutation reaches the canonical-truncation assertion while the two corrupt-input laws remain green.
calibration-green.txt calibration-green.log: restored three-law debug/release success and workspace Clippy.
materialized-selector-red.txt, materialized-absent-red.txt, materialized-diagnostic-red.txt Respective mutation-directory red.log: selector 4 versus 5, absent named input, and observed diagnostic 1 versus 2 each fail the actual named emitted-input law after compilation. Exactly one test executes per receipt.
materialized-green.txt materialization-final-green.log: restored named tool-to-runtime law executes successfully in debug/release and Clippy completes. Zero-selection setup attempts are excluded explicitly.
  • All introduced production framing literals: Compared version 1, flags 0, seal length 128, reserved zeros, algorithms 1, field offsets 16/18/20/22/28/56/57/58, and widths 2/4/1/6 against docs/formats/segment-store-v1/segment.md:142–:164, segment_seal.rs:5–:10, and segment_seal_decoder.rs:35–:49. Existing protocol constants are reused. The independent test/fuzz tables agree. Big-endian error values 257, 384, 256, 129, 16,777,216, 65,536, 1, and the six reserved-array mutations match each one-bit counterexample and temporary canonical comparison completion.
  • Fuzz budgets and versions: Evidence commands' nightly 2026-07-24, cargo-fuzz 0.13.2, input timeout 5 seconds, RSS 1,024 MiB, maximum input 1,048,576 bytes, and 15-second exploration setting agree with fuzz/campaign.env and raw commands. Historical fixed replays have their own emitted seeds, distinct from explicit campaign seed 17101. Raw GREEN records 10 initial files and 3,110,375 executions finishing in 16 seconds. A configured 15-second exploration setting is not a hard total-completion guarantee. No changed runtime timeout, buffer, throughput, allocation, or recovery-time threshold was introduced.
  • All changed prose/numeric claims: Reviewed CHANGELOG.md:11, docs/formats/segment-store-v1/recovery.md:55, requirements.md:127, all 51 lines of docs/testing-evidence/partial-seal-corruption.md, src/adapters/recovery/rationale.md, tests/fixtures/recovery/README.md, and fuzz/README.md:82. Their issue/contract/source coordinates, field descriptions, 82-byte input, selector 5, direct calibration counts, replay versions/budgets, and debug/release claims agree with source and receipts. Receipt timing, run counts, addresses, corpus statistics, and generated dictionary frequencies remain pinned historical outputs, not current performance promises. No README performance figure or measured resource bound changed.

Repository rules, complete feedback, and landing gates

  • Applied repository AGENTS.md, docs/Testing Standards.md, and docs/testing/enforcement.md. New direct/family laws declare small in-memory size, independent oracle, and deletion criteria; the emitted-input law declares medium size and owned scratch. Explicit evidence distinguishes product, tool-to-runtime, calibration, and static/API claims. Colocated src/adapters/recovery/rationale.md records the recovery decision and exclusions. The added narrow module and functions respect hard structural limits and introduce no unsafe code, unchecked indexing/casts, public boolean parameters, sleeps, async, forbidden generic filename, or dependency.
  • Per-test time/memory enforcement and denied-egress controls remain disclosed repository gaps. This review does not call those controls implemented or grant a general policy waiver. No new concurrency, persistence, codec identity, encryption, or performance protocol was added; the applicable evidence is fixed-frame corruption/refusal, public API regression, finite family, reduced input, and live semantic fuzzing. Existing process-death validation is additional evidence, not physical power-loss proof.
  • Refreshed live GraphQL reviews, global comments, and inline threads. Every top-level connection ended with hasNextPage: false; there are no inline threads, so no nested thread-comment pagination remained. Seven global comments were reconciled separately from resolvable threads. Previous findings were independently checked as above. CodeRabbit's sole effective review approves this exact head; there are no active changes-requested reviews.
  • CodeRabbit's optional docstring-coverage warning reports 19.05% against its 80% threshold over 21 functions. That provider metric is not a repository merge requirement; the repository requires public-item documentation, and this review found no newly undocumented public item. It is not converted into a storage acceptance percentage.
  • Independently checked exact-head CI run 37097418652: all four jobs are completed and successful. Inspected individual step conclusions: debug/optimized process-death matrices, conformance/worldline, source structure, formatting, both feature checks/Clippy profiles, debug/release workspace tests, doctests/docs, MSRV, fuzz-target checks, documentation/refusal/whitespace, dependency/audit checks, and registered fuzz build/exercise all succeeded. The conditional failure-artifact upload was appropriately skipped; no required validation step was silently skipped.

Executed versus inspected scope

Executed by this reviewer: read-only source/Git/GitHub inspection, raw-to-checked-in receipt comparisons, fixture-byte verification, patch applicability checks, base-to-head and empty-tree-to-HEAD whitespace checks, merge-parent comparisons, and exact-head/CI/feedback queries. Only this scratch report was written. No source change, commit, push, configuration change, external comment, merge, or delegation occurred.

Inspected rather than independently rerun: supplied copied-Docker RED/GREEN/calibration/fuzz logs and final-07e6cc4-validation.log. Its exact-head new-law debug/release results appear at lines 695–704 and 2724–2733; its final documentation build completes at line 4056. Also inspected fresh landing 172-focused-corrected.log: three partial-seal laws, 15 adjacent classification laws, six assessment laws, seven discard laws, and the emitted-input law pass in debug/release. The parent reports its copied source tree matches the reviewed tree hash; the candidate tree hash was independently verified locally. The first landing command's nonexistent-target setup failure remains preserved and is not counted as runtime RED or GREEN.

No host Rust tests or benchmarks were run. This reviewer did not rerun mutations or launch another container campaign. Static inspection and inspected author/landing execution receipts are distinguished throughout. Unrun physical power-loss, actual corrupt-stage unlink, allocation benchmarks, universal future-prefix feasibility, all recovery schedules, and a complete subsystem audit are neither claimed nor required to substantiate this bounded read-only framing correction.

The reviewed source/evidence gate is satisfied for the exact head above. Merge authorization and immediately pre-merge head/protection verification remain the owning agent's responsibility; this report does not authorize bypassing any protection or attest a merge that has not occurred.

APPROVE

@flyingrobots

Copy link
Copy Markdown
Owner Author

Code Lawyer landing pass — candidate 07e6cc4

Target reviewed: main 6051abb25a9fd33ae7ee0de5614514b709a4d82a. The maintainer now explicitly authorizes normal merges after fresh Code Lawyer review and clean, green validation. No repository rule bypass or history rewrite is used.

Item Source / severity Disposition and evidence
Contradictory partial seal framing could authorize discard #171 / P2 Fixed in 43820f5; parent runtime RED 1b6da98, reduced fixture and semantic fuzz RED/GREEN f94e418. Current classifier validates available fixed framing before constructing truncation; assessor uses the same path and discard planner cannot receive its failed assessment.
Exact diagnostic/source/truncation oracle calibration Earlier independent review / P2 Closed in 07e6cc4; reviewed producer mutations and intended assertion failures, restored debug/release results. No failed setup or zero-test run credited.
Trailing receipt whitespace Earlier independent review / P2 Closed in 07e6cc4; disclosed normalization, clean diff and successful required documentation job.
Frozen seed counts Full-suite failure / evidence correction 07e6cc4 replaces cardinality with actual emitted input replay through Keep and precise refusal; deterministic full emitted-byte equality remains. Calibrated selector, missing input and diagnostic outcomes.
CodeRabbit global docstring percentage warning Optional provider heuristic Not a required repository gate. The added public Seal variant and cause are documented; private helpers remain private. No documentation gap justified by the percentage alone. Required documentation job passes.
Duplicate records followed by incomplete tails Existing #173 / separate scope Remains open. This PR changes fixed seal framing, not the unchanged complete-record identity-index bypass. No claim that #173 or the entire recovery audit is complete.

Fresh landing inspection covers the complete diff, surrounding full seal decoder/admission, partial-field comparator, classifier, assessment, resume and discard planning paths, typed sources, fixtures, fuzz route and seed materialization. No merge commits occur in the four-commit PR history. No write protocol, persistent format, variable-field completion proof or #99 v2 disposition change is introduced.

All global conversation comments, review bodies and inline threads were fetched with pagination exhausted. Existing independent findings are accounted for above. Current CodeRabbit review is APPROVED on this exact head; there are no changes-requested reviews or inline threads. Hosted Codex's usage-limit message is not review evidence.

The fresh copied-Docker tree is clean and has tree af00023bb50da6b5f390fc8f2e4b070bf15ceb52, identical to this candidate. Public partial-seal/classification/assessment/discard suites and the emitted-counterexample runtime law pass debug/release. The initial command named a nonexistent target and ran no tests; it is a recorded setup failure, not runtime RED. Corrected execution returns zero and executes each intended suite.

The prior complete copied-Docker validation and all four successful current-head hosted jobs remain valid for this unchanged source: CI run 37097418652. The fresh focused run supplements them without claiming another broad crash/fuzz campaign. Current required signature and branch-history rules were inspected; normal signed GitHub merge is the authorized route. Independent fresh review is posted separately and must approve this exact head before merge.

@flyingrobots
flyingrobots merged commit d0cff10 into main Oct 3, 2026
5 checks passed
@flyingrobots
flyingrobots deleted the fix/171-partial-seal-corruption branch October 3, 2026 18:57
@flyingrobots

Copy link
Copy Markdown
Owner Author

Mainline integration verified

Merged normally as d0cff10d7c911d33d615c3aa2246ae2b4497432a, with parents 6051abb25a9fd33ae7ee0de5614514b709a4d82a and reviewed candidate 07e6cc4875c05592b71bb1f8b9c80631a31bcda5. GitHub verifies the merge signature. Its tree af00023bb50da6b5f390fc8f2e4b070bf15ceb52 exactly equals the reviewed candidate tree.

The actual mainline CI run 37146130023 completed successfully: Rust quality gates, Documentation and workflow integrity, Runtime fuzz smoke, and Dependency policy all pass. This is post-merge evidence for the integration SHA, not a transfer of the earlier candidate's green status.

Issue #171's scoped correction is delivered. Separate #173 and the remaining completed-roadmap audit remain open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refuse corrupt partial segment seals before recovery discard

1 participant