Skip to content

fix(outcome): diagnose and repair completed-ledger digest chain breaks - #645

Merged
solomonneas merged 2 commits into
mainfrom
fix/639-outcome-ledger-repair
Aug 1, 2026
Merged

fix(outcome): diagnose and repair completed-ledger digest chain breaks#645
solomonneas merged 2 commits into
mainfrom
fix/639-outcome-ledger-repair

Conversation

@solomonneas

Copy link
Copy Markdown
Collaborator

Summary

  • brigade outcome doctor now validates the completed ledger chain and reports the first break with line number, expected/actual digests, and suspected cause (including duplicate-writer).
  • Adds sanctioned brigade outcome repair --operator-confirm: quarantine write-once under .brigade/outcome/repairs/, preserve the invalid segment, keep the valid prefix byte-identical, append a neutral ledger-repair record, and re-verify.
  • Capture/record degrade to a bounded actionable error (ledger corrupt at line N … run outcome repair) instead of a raw traceback, and write nothing while the chain is broken.

Closes #639.

Verify receipt

20260801-034309-work-verify-12d572

Command:

brigade work verify run --target . --command "./scripts/verify" --capture brigade-work

Acceptance criteria → tests

Criterion Test
Doctor reports completed-ledger chain break with line + digests + cause test_doctor_reports_completed_ledger_chain_break
Duplicate-record break diagnosed as duplicate-writer test_diagnose_duplicate_record_chain_break
Truncated-line break diagnosed test_diagnose_truncated_line_break
Repair requires operator confirmation test_repair_requires_operator_confirmation
Repair quarantines, re-chains, appends repair record, re-verifies test_repair_quarantines_rechains_and_reverify
Capture degrades with bounded error and writes nothing test_capture_degrades_with_bounded_error_and_writes_nothing
Post-repair capture succeeds test_post_repair_capture_succeeds
Truncated trailing recovery stays healthy without rewrite test_repair_of_truncated_line_recovers_via_lock_path_and_stays_healthy

Operator note

Do not run repair against the live corrupted ledger from this PR worktree. After merge, repair the live checkout manually:

brigade outcome doctor --target ~/repos/brigade
brigade outcome repair --target ~/repos/brigade --operator-confirm

Test plan

  • brigade work verify run --target . --command "./scripts/verify" --capture brigade-work
  • Focused tests/test_outcome_repair.py green
  • External grader review before merge

Made with Cursor

A broken completed outcome ledger used to hard-block every capture with a
raw traceback and no recovery path. Doctor now surfaces the first chain
break, capture degrades to a bounded repair hint, and outcome repair
quarantines the original ledger before re-chaining from the valid prefix.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are limited based on label configuration.

🏷️ Required labels (at least one) (1)
  • coderabbit-review

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: escoffier-labs/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 10b8017e-a075-4f2f-aca3-11fed8a934ee

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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

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

@solomonneas solomonneas self-assigned this Aug 1, 2026
@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader review: APPROVE-WITH-CHANGES - and one finding is a hard stop before this ever runs on the live ledger. The mechanics survive attack (healthy-ledger no-op under the append lock, O_EXCL quarantine keyed on content, correct crash ordering, byte-identical ledger after failed capture). Required changes:

  1. HARD STOP - silent tail data loss on the live shape. Diagnosis stops at the FIRST break and quarantines raw[offset:] to EOF (outcome_repair.py:97-98). On the live ledger (break at line 999 of 1,631) repair drops ~633 records - 39% of all scoring signal - into invalid-segment.jsonl with no warning and no count. Every test places the break on the LAST line, exactly not the live shape. Minimum: print the post-break record count and range and require confirmation before mutating. Better: re-chain the internally consistent tail (re-signed, disclosed in the repair record). Add tests: valid records after the break, first-line break, multiple breaks.
  2. Quarantine-first is violated for incomplete_trailing: _recover_interrupted_append runs before diagnosis or quarantine (outcome_repair.py:372) and truncates the partial line in place, destroying the bytes outcome: completed-ledger digest chain break has no diagnosis or recovery path and blocks all capture #639 requires preserved. Quarantine the original before recovery in the repair path.
  3. Windows byte-fidelity: _publish_exclusive_bytes routes through write_text_exclusive without newline='' (localio.py:124), so quarantines are not byte-identical on Windows and second-run idempotency comparison fails. Write bytes with O_EXCL unconditionally.
  4. repo-metadata CI: regenerate docs/command-inventory.md for the new 'outcome repair' command.

Non-blocking: format_ledger_corrupt_error is dead code; add a directory fsync for the quarantine dir before the ledger rewrite (run_redaction does this); doctor rc=0 on corrupt prevents automation gating; the duplicate-writer heuristic requires payload equality minus digests so the live near-identical rows (differing ts) will report plain discontinuity; verify_run ignores capture's rc so degradation is stderr-only. Ping for re-grade; the live repair happens only after re-grade, operator-run, with the dropped-record count on screen.

Co-authored-by: Codex <codex@openai.com>
@solomonneas

solomonneas commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

Implemented in 4a8110f (fix(outcome): retain valid records after ledger breaks).

  1. Tail preservation: re-sign self-consistent records after the first break, retain the raw segment, and disclose salvaged count and line ranges in the repair record, audit file, JSON output, and text output.
    Tests: test_repair_rechains_valid_records_after_break, test_repair_rechains_from_a_first_line_break, test_repair_rechains_across_multiple_chain_breaks, test_repair_skips_binary_bytes_and_rechains_later_valid_records.
  2. Incomplete trailing bytes: quarantine original bytes before recovery, and do not recover a partial tail when an earlier chain break is authoritative.
    Tests: test_repair_quarantines_incomplete_trailing_bytes_before_recovery, test_repair_keeps_the_original_tail_range_when_chain_break_precedes_partial_bytes.
  3. Byte-fidelity and idempotency: quarantine publication now uses binary O_EXCL writes with file fsync; the repair directory is fsynced before the ledger rewrite.
    Test: test_publish_exclusive_bytes_preserves_non_utf8_bytes.
  4. Regenerated docs/command-inventory.md with brigade outcome repair; removed dead format_ledger_corrupt_error.

Verification passed: brigade work verify run --target . --command './scripts/verify' --capture brigade-work - 5570 passed, 3 skipped, coverage 82.96%. Receipt: 20260801-202819-work-verify-19fcc2.

Ready for re-review.

@solomonneas

Copy link
Copy Markdown
Collaborator Author

Grader: APPROVE, merging. The hard-stop finding is properly fixed: rather than the minimum (warn before mutating), the repair now re-signs self-consistent records after the first break and discloses salvaged counts and line ranges in the repair record, audit file, and both output modes. The four new tests cover exactly the shapes that were untested and that the live ledger actually exhibits: records after the break, a first-line break, multiple breaks, and binary bytes. Quarantine-first is restored for incomplete trailing bytes, and quarantine publication is now binary O_EXCL with directory fsync before the ledger rewrite, closing the Windows byte-fidelity gap. Receipt 20260801-202819-work-verify-19fcc2, CI green.

Note for the live repair: this now preserves the ~633 post-break records on our corrupted ledger instead of discarding them. Run it operator-supervised with the salvaged count on screen.

@solomonneas
solomonneas merged commit ed1f489 into main Aug 1, 2026
28 checks passed
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.

outcome: completed-ledger digest chain break has no diagnosis or recovery path and blocks all capture

1 participant