diff --git a/CHANGELOG.md b/CHANGELOG.md index b6079cba..6b87c548 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -322,6 +322,24 @@ whose seams had diverged enough that several ports needed a different fix, and t ### Fixed +- **A failed ledger write can no longer empty the deferred-work ledger (#328).** `Path.write_text` + truncates the file and only then encodes, so any failure in that window — an unencodable value, + `ENOSPC`, `EIO` — left a zero-byte ledger with every entry gone. `append_decision`, + `append_entry`, the post-rollback and migration-failure restores, and the bundle intent write now + go through `atomic_write_text` like their `mark_done_many` sibling: the replacement is built + beside the target, so a failure raises with the original untouched. A ledger `append_entry` + creates from nothing now lands `0600` (the helper's private temp mode) rather than the umask + default. + +- **A lone surrogate in triage text no longer crashes the sweep (#329).** A cached triage + `result.json` stores `\ud800` as an escape; the reload's `json.loads` revives the real code point + into an entry's evidence and on into the ledger note. It is not a line break, so the sanitizer + passed it through untouched, and it has no UTF-8 encoding at all — the strict encode raised + `UnicodeEncodeError` from a close path that calls the writers bare. Every free-text ledger field + is now neutralized at that same chokepoint, and the bundle `intent.md` gets the same pass over the + whole document (its line breaks are legitimate markdown, so only surrogates are touched). Each + becomes U+FFFD `�`, so the text stays visible rather than vanishing. + - **A `#` inside a quoted sprint-status value is no longer rewritten into a comment (#366).** The board writer split value from inline comment with one fused pattern, so `3-2-x: "a # b"` advanced to `3-2-x: done # b"` — scalar text promoted into a comment the board never had. The split is now diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index be8fa870..5a6f429e 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -24,7 +24,7 @@ from . import sprintstatus from .fences import fenced_spans -from .platform_util import atomic_write_text +from .platform_util import atomic_write_text, neutralize_surrogates HEADING_RE = re.compile(r"^### (DW-\d+): (.+?)\s*$", re.MULTILINE) ANY_HEADING_RE = re.compile(r"^#{1,6} ", re.MULTILINE) @@ -618,11 +618,24 @@ def _one_line(value: str) -> str: line anyway — so this is the fix, and the skill docs are guidance that reduces occurrences without gating on them. - A value with no break is returned **untouched**, so an existing ledger is - never reformatted and a clean write is byte-identical to before the guard. - The trailing `.strip()` removes all surrounding whitespace, not merely the - space a leading or trailing break left behind — which is why it must stay on - the far side of that fast path. + That contract covers one hazard more than the break collapse alone, which is + what the `neutralize_surrogates` pass in front of it buys (#329). A lone + surrogate is not a line break, so it sailed through untouched — but it has + no UTF-8 encoding, and `atomic_write_text`'s strict encode raises + `UnicodeEncodeError` (a `ValueError` subclass) on it, from inside those same + bare close-path calls. It arrives the way the break did: a triage + `result.json` is cached with `json.dumps`, whose `ensure_ascii` keeps the + code point a harmless `\\ud800` escape, and the reload's `json.loads` revives + the real thing into `ResolvedEntry.evidence` and on into the `mark_done` + note. Refusing it upstream would only move the stoppage again — same + doctrine, same answer. + + A value with neither a break nor a surrogate is returned **untouched**, so an + existing ledger is never reformatted and a clean write is byte-identical to + before the guard; each pass keeps its own fast path, so the common value is + scanned twice and copied never. The trailing `.strip()` removes all + surrounding whitespace, not merely the space a leading or trailing break left + behind — which is why it must stay on the far side of that fast path. A break-only value therefore sanitizes to `""`. Keeping it non-empty *here* could only yield bare whitespace, which trades an unfindable entry for an @@ -634,7 +647,13 @@ def _one_line(value: str) -> str: empty detail rather than promising one that is not there. Its `label` needs neither: every member of :data:`LINE_BREAK_RE` is `str.isspace()`, and `validate_triage` builds each `DecisionOption` with `.strip() or key`, so a - break-only label has already become the option key before it arrives.""" + break-only label has already become the option key before it arrives. + + A surrogate-only value, by contrast, sanitizes to a truthy `"�"`, so neither + caller's empty-handling fires for it. That is the point of replacing rather + than stripping: a title reading `�` still says *something unencodable was + here*, where a vanished one would silently become `(untitled DW-)`.""" + value = neutralize_surrogates(value) if not LINE_BREAK_RE.search(value): return value return LINE_BREAK_RE.sub(" ", value).strip() @@ -875,7 +894,13 @@ def append_decision(path: Path, dw_id: str, date: str, label: str, detail: str) Precondition: `date` is ISO `YYYY-MM-DD`; anything else raises `ValueError`, checked before the ``is_file`` short-circuit so an absent ledger cannot hide - the bug.""" + the bug. + + The write goes through :func:`~bmad_loop.platform_util.atomic_write_text` for + the reasons documented on :func:`mark_done_many`, plus one this sibling shares + with it: a bare ``Path.write_text`` truncates *before* it encodes, so any + failure between the two — an unencodable value, ``ENOSPC``, ``EIO`` — leaves a + zero-byte ledger where every entry used to be (#328).""" _require_iso_date(date) if not path.is_file(): return False @@ -890,7 +915,7 @@ def append_decision(path: Path, dw_id: str, date: str, label: str, detail: str) detail = _one_line(detail) detail_part = f" — {detail}" if detail else "" text = _insert_after_status(text, entry, f"decision: {date} {label}{detail_part}") - path.write_text(text, encoding="utf-8") + atomic_write_text(path, text) return True @@ -938,7 +963,13 @@ def append_entry( :func:`field_line_present`: sanitizing afterwards would compare a raw value against a sanitized line, so every replay of the same multiline defer would miss its own entry and append another. `status` and `severity` are - orchestrator-owned enumerations and raise instead.""" + orchestrator-owned enumerations and raise instead. + + The write goes through :func:`~bmad_loop.platform_util.atomic_write_text` for + the reasons documented on :func:`mark_done_many`, plus one this sibling shares + with it: a bare ``Path.write_text`` truncates *before* it encodes, so any + failure between the two — an unencodable value, ``ENOSPC``, ``EIO`` — leaves a + zero-byte ledger where every entry used to be (#328).""" _require_canonical_status(status) # The whitelist is derived from the legacy parser's alias table (defined # below; resolved at call time) so what this writer emits and what @@ -993,7 +1024,7 @@ def append_entry( else: sep = "\n\n" path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(text + sep + block, encoding="utf-8") + atomic_write_text(path, text + sep + block) return dw_id diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index 7834eec5..c6048328 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -5152,7 +5152,7 @@ def _defer(self, task: StoryTask, reason: str) -> None: ) if current != snapshot: deferred_work.parent.mkdir(parents=True, exist_ok=True) - deferred_work.write_text(snapshot, encoding="utf-8") + atomic_write_text(deferred_work, snapshot) # The restore deliberately keeps review-found ledger knowledge, but # it also replays this bundle's accepted close after the code was # discarded. Let the mode undo only the close it can identify as its diff --git a/src/bmad_loop/platform_util.py b/src/bmad_loop/platform_util.py index ec51059b..ec155d6b 100644 --- a/src/bmad_loop/platform_util.py +++ b/src/bmad_loop/platform_util.py @@ -325,6 +325,38 @@ def _copy_xattrs(src: Path, dst: Path) -> None: continue +# Matched as a code-point range rather than by an encode round trip, the same way +# ``engine._TITLE_CONTROL_RE`` reaches these: a Python ``str`` holds code points, +# so an astral character like U+1D11E is one code point *outside* this range and +# is never touched. Only genuinely lone surrogates match. +_SURROGATES_RE = re.compile(r"[\ud800-\udfff]") + + +def neutralize_surrogates(text: str) -> str: + """Replace every lone surrogate in ``text`` with U+FFFD (``�``). + + A surrogate is a legal ``str`` code point with **no UTF-8 encoding at all**, + so any strict encode — :func:`atomic_write_text`'s included — raises + ``UnicodeEncodeError`` on one. That is a ``ValueError`` subclass, which is + how a single unpaired code point reaches a caller as a crash rather than as + mangled text. They arrive from anywhere a decoder is allowed to mint them: + ``json.loads`` reviving a ``\\ud800`` escape, a double-quoted YAML scalar, a + ``surrogateescape`` decode of undecodable filesystem bytes. + + Replace, not strip and not refuse. U+FFFD keeps the value **visible** — the + field still says *something unencodable was here* — where dropping the code + point would let it vanish silently and refusing would only move the stoppage + upstream. It is also why this is a substitution rather than the shorter + ``text.encode("utf-8", "replace").decode("utf-8")``: that spelling yields + ``"?"``, indistinguishable from a question mark the author actually typed. + + Text with no surrogate is returned untouched — the identical object, so a + clean write stays byte-identical.""" + if not _SURROGATES_RE.search(text): + return text + return _SURROGATES_RE.sub("�", text) + + def atomic_write_text(path: Path, text: str, *, follow_symlinks: bool = True) -> None: """Replace ``path``'s contents with ``text`` atomically, preserving what the replacement would otherwise silently discard. diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index 0fd04f66..b61a6982 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -21,7 +21,7 @@ from .engine import Engine from .escalation import critical_escalations, env_fault_pause_reason, session_failure_reason from .model import Phase, StoryTask -from .platform_util import atomic_replace +from .platform_util import atomic_replace, atomic_write_text, neutralize_surrogates from .statemachine import advance from .workspace import discard_worktree @@ -793,7 +793,7 @@ def _ensure_migration(self, text: str) -> None: # ledger that `git reset` cannot restore self._safe_reset(task) ledger.parent.mkdir(parents=True, exist_ok=True) - ledger.write_text(text, encoding="utf-8") + atomic_write_text(ledger, text) if task.attempt >= self.policy.sweep.max_migration_attempts: self._escalate( task, "migration failed deterministic validation: " + "; ".join(errors) @@ -1152,7 +1152,13 @@ def _write_intent(self, bundle: Bundle, dirname: str) -> Path: lines += ["", "## Ledger entries (verbatim)", "", "\n\n".join(blocks), ""] path = self.run_dir / "bundles" / dirname / "intent.md" path.parent.mkdir(parents=True, exist_ok=True) - path.write_text("\n".join(lines), encoding="utf-8") + # Surrogates are neutralized over the whole document, not per field: the + # triage-authored `intent`/`decision_note` are the ones that can revive one + # (#329), but a document-wide pass covers whatever prose is added here + # later. Line breaks are deliberately *kept* — this file is markdown, so + # `_one_line`'s collapse would be damage, and the ledger blocks are read + # back from a strict-UTF-8 file and so pass through byte-unchanged. + atomic_write_text(path, neutralize_surrogates("\n".join(lines))) return path def _ensure_bundle_intent(self, task: StoryTask) -> None: diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 06a162c5..863f4851 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -467,6 +467,23 @@ def test_append_decision_missing_file(tmp_path): assert not mark_done(tmp_path / "nope.md", "DW-1", "2026-06-11", "x") +def test_append_decision_write_failure_raises_and_keeps_the_ledger(tmp_path, monkeypatch): + """#328. `Path.write_text` opens `'w'` (truncate) and only THEN encodes, so a + failure anywhere in that window left the whole ledger at zero bytes. The + atomic helper builds the replacement beside the target, so a raise leaves the + original exactly as it was.""" + path = write_ledger(tmp_path) + before = path.read_bytes() + + def boom(path, text): + raise OSError("disk full") + + monkeypatch.setattr(deferredwork, "atomic_write_text", boom) + with pytest.raises(OSError, match="disk full"): + append_decision(path, "DW-3", "2026-06-11", "Keep cap", "frozen intent stands") + assert path.read_bytes() == before + + # ------------------------------------------------- line-break injection (#305) # # The ledger is line-oriented and every mutator interpolates its arguments, so a @@ -1259,26 +1276,83 @@ def test_append_entry_leaves_an_already_empty_title_as_it_was(tmp_path): assert p.read_text(encoding="utf-8").startswith("### DW-1: \n") -def test_append_entry_idempotence_survives_sanitizing(tmp_path): +@pytest.mark.parametrize( + ("origin", "source_spec"), + [ + ("review-budget-followup", "spec-foo.md\nstatus: open"), + ("review\ud800followup", "spec-foo.md"), + ("review-budget-followup", "spec-\udfff-foo.md"), + ], + ids=["line-break", "surrogate-origin", "surrogate-source-spec"], +) +def test_append_entry_idempotence_survives_sanitizing(tmp_path, origin, source_spec): """Sanitizing must happen BEFORE the idempotence scan: that scan compares the caller's value against the stored line via `field_line_present`, so sanitizing afterwards would compare raw against sanitized and append a fresh - entry on every replay of the same defer.""" + entry on every replay of the same defer. + + Both sanitizing passes are in front of that scan, so both are covered here. + The surrogate rows would otherwise regress the same way the break row does — + a replayed defer whose origin carries a `\\ud800` matches nothing on disk + (the ledger stores the U+FFFD) and burns a fresh id every run.""" p = tmp_path / "deferred-work.md" - dirty = "spec-foo.md\nstatus: open" - first = append_entry( - p, title="t", origin="review-budget-followup", source_spec=dirty, reason="r" - ) - again = append_entry( - p, title="t2", origin="review-budget-followup", source_spec=dirty, reason="r2" - ) + first = append_entry(p, title="t", origin=origin, source_spec=source_spec, reason="r") + again = append_entry(p, title="t2", origin=origin, source_spec=source_spec, reason="r2") assert first == "DW-1" assert again is None assert len(parse_ledger(p.read_text(encoding="utf-8"))) == 1 +# ------------------------- surrogate neutralization at the sanitizer chokepoint (#329) +# Every row here calls a real writer with no monkeypatching: the value has to +# survive `_one_line`, `atomic_write_text`'s strict UTF-8 encode, and the strict +# read back. Delete the `neutralize_surrogates` call in `_one_line` and each one +# fails with `UnicodeEncodeError` — the crash #329 filed, arriving from a close +# path that calls these writers bare. + + +def test_append_entry_writes_a_finding_carrying_a_lone_surrogate(tmp_path): + """A lone surrogate is not a line break, so it sailed past the break collapse + untouched and detonated in the encode. Note the title stays truthy — `�` is a + visible replacement, so `append_entry`'s `(untitled DW-)` substitution + deliberately does NOT fire for it.""" + p = tmp_path / "deferred-work.md" + + dw_id = append_entry(p, title="\ud800", origin="o", source_spec="s.md", reason="x\udfffy") + + assert dw_id == "DW-1" + text = p.read_text(encoding="utf-8") # strict read: unencodable text never got here + (entry,) = parse_ledger(text) + assert entry.title == "�" # replaced, not vanished into `(untitled DW-1)` + assert "reason: x�y" in entry.body + + +def test_append_decision_writes_a_label_and_detail_carrying_a_lone_surrogate(tmp_path): + """`decisions.apply_pre_answer` calls this bare, so the raise would end the + sweep. Both interpolated fields go through the chokepoint.""" + path = write_ledger(tmp_path) + + assert append_decision(path, "DW-3", "2026-06-11", "\ud800", "a\ud800b") + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert "decision: 2026-06-11 � — a�b" in entries["DW-3"].body + + +def test_mark_done_writes_a_note_carrying_a_lone_surrogate(tmp_path): + """The exact string `sweep._close_resolved` builds — `f"already resolved: + {entry.evidence}"` — where `evidence` is the field the cached triage JSON + revives a surrogate into.""" + path = write_ledger(tmp_path) + + assert mark_done(path, "DW-1", "2026-06-11", "already resolved: \ud800evidence") + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert entries["DW-1"].status == "done 2026-06-11" + assert "resolution: already resolved: �evidence" in entries["DW-1"].body + + @pytest.mark.parametrize("status", ["", "done", "closed", "open\n### DW-99: injected", "OPEN"]) def test_append_entry_raises_on_a_noncanonical_status_without_writing(tmp_path, status): p = tmp_path / "deferred-work.md" @@ -1354,6 +1428,55 @@ def test_append_entry_leaves_a_clean_value_byte_identical(tmp_path): ) +def test_append_entry_write_failure_raises_and_keeps_the_ledger(tmp_path, monkeypatch): + """#328, the `append_entry` half — see the `append_decision` twin above.""" + path = write_ledger(tmp_path) + before = path.read_bytes() + + def boom(path, text): + raise OSError("disk full") + + monkeypatch.setattr(deferredwork, "atomic_write_text", boom) + with pytest.raises(OSError, match="disk full"): + append_entry( + path, + title="new finding", + origin="review of spec-foo.md", + source_spec="spec-foo.md", + reason="out of scope", + ) + assert path.read_bytes() == before + + +def test_append_entry_encode_failure_cannot_truncate_the_ledger(tmp_path, monkeypatch): + """#328's worst case, reached without any injected OSError: an unencodable + value raises from inside the encode step itself. Under a bare `write_text` + the file is already truncated by then, so the raise and the data loss arrive + together — the exact compounding #329 describes.""" + path = write_ledger(tmp_path) + before = path.read_bytes() + # Patching the sanitizer to the identity is now LOAD-BEARING: since #329, + # `_one_line` neutralizes surrogates, so without this patch the value would + # reach the write already encodable and nothing would raise. Keeping it pins + # the WRITE layer's defense independently of the sanitizer's — without it + # this row would quietly stop testing #328 and become a second test of + # #329's fix. Do not remove it. + monkeypatch.setattr(deferredwork, "_one_line", lambda v: v) + + with pytest.raises(UnicodeEncodeError): + append_entry( + path, + title="\ud800", + origin="review of spec-foo.md", + source_spec="spec-foo.md", + reason="out of scope", + ) + + # The raise is NOT the invariant under test — a bare `write_text` raises here + # too. The bytes are: this is the assertion that reddens without the fix. + assert path.read_bytes() == before + + def test_field_line_present_matches_field_not_substring(): body = ( "### DW-1: x\norigin: review-budget-followup\n" diff --git a/tests/test_engine.py b/tests/test_engine.py index 94519ec8..342abd25 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -9704,6 +9704,41 @@ def test_dev_defer_keeps_harvest_and_disarms_snapshot(project): assert (persisted.pre_harvest_ledger_captured, persisted.pre_harvest_ledger) == (False, None) +def test_defer_ledger_restore_write_failure_propagates_and_keeps_the_ledger(project, monkeypatch): + """#328. The post-rollback ledger restore inside `_defer` is a repair write: + it must raise rather than degrade, and a failed attempt must never be the + thing that empties the ledger it exists to put back. Under a bare + `Path.write_text` the truncate lands before the failure does, so the run + crashed AND the ledger it was restoring went to zero bytes. + + The patch is module-wide but lands on exactly one call — probed on this + harness, `_defer`'s restore is the only `engine.atomic_write_text` this path + reaches; the pre-harvest restore and the deferred-close rollback both need + state this scenario never builds. Committing the ledger is what makes the + restore fire at all: `git reset` reverts the harvest's append to a *tracked* + file, so the snapshot and the bytes on disk then differ.""" + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + write_ledger(project, {"DW-1": "open"}) + engine, _ = make_engine( + project, + [_baseline_liar_effect(project, deferred=[HARVEST_A])], + policy=_harvest_policy(attempts=1), + ) + + def boom(path, text): + raise OSError("disk full") + + monkeypatch.setattr("bmad_loop.engine.atomic_write_text", boom) + + summary = engine.run() + + assert summary.crashed and "disk full" in str(summary.crash_error) + # the restore never landed, but the ledger is the committed one `git reset` + # put back — not a zero-byte file the failed write truncated on its way out + assert project.deferred_work.read_bytes() + assert _ledger_entries(project)["DW-1"].open + + def test_dev_escalation_keeps_harvest_and_disarms_snapshot(project): inner = dev_effect(project, "1-1-a", followup_review=False, deferred=[HARVEST_A]) diff --git a/tests/test_platform_util.py b/tests/test_platform_util.py index dd6747d7..4163cd81 100644 --- a/tests/test_platform_util.py +++ b/tests/test_platform_util.py @@ -334,6 +334,67 @@ def denied(src, dst): assert sleeps == [] # zero backoff — a real POSIX error surfaces at once +# -------------------------------------------------------- neutralize_surrogates + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ("plain text", "plain text"), + ("", ""), + ("\ud800", "\ufffd"), # the lone surrogate json.loads revives (#329) + ("a\ud800b", "a\ufffdb"), + ("\udfff", "\ufffd"), # the far end of the range + ("x\udfffy", "x\ufffdy"), + ("\ud800\ud801\udfff", "\ufffd\ufffd\ufffd"), # a run: one per code point + ("\U0001d11e", "\U0001d11e"), # astral, ONE code point — never a pair here + ("\u00e9\U0001d11e\u6f22", "\u00e9\U0001d11e\u6f22"), + ("\ud7ff\ue000", "\ud7ff\ue000"), # the code points either side of the range + ], + ids=[ + "clean", + "empty", + "lone-d800", + "surrounded", + "lone-dfff", + "surrounded-dfff", + "run-per-code-point", + "astral-untouched", + "mixed-non-ascii", + "range-boundaries", + ], +) +def test_neutralize_surrogates_replaces_only_lone_surrogates(value, expected): + """A surrogate has no UTF-8 encoding; everything else must survive intact. + The astral row is the one that would break under a naive UTF-16 mental model: + Python holds U+1D11E as a single code point, not a D834/DD1E pair, so it is + outside the range and must come back byte-identical.""" + result = platform_util.neutralize_surrogates(value) + + assert result == expected + result.encode("utf-8") # the whole point: the strict encode now succeeds + + +def test_neutralize_surrogates_returns_clean_text_untouched(): + """The fast path hands back the identical object, so a clean ledger write + stays byte-identical to one taken before the guard existed.""" + value = "origin: review of spec-foo.md" + + assert platform_util.neutralize_surrogates(value) is value + + +def test_neutralize_surrogates_makes_atomic_write_text_survive_a_surrogate(tmp_path): + """The pairing this helper exists for: the same value crashes the strict + encode without it and round-trips through a strict read with it.""" + target = tmp_path / "ledger.md" + + with pytest.raises(UnicodeEncodeError): + platform_util.atomic_write_text(target, "note: \ud800") + + platform_util.atomic_write_text(target, platform_util.neutralize_surrogates("note: \ud800")) + assert target.read_text(encoding="utf-8") == "note: �" + + # ------------------------------------------------------------- atomic_write_text diff --git a/tests/test_sweep.py b/tests/test_sweep.py index be1e7c7a..6b4be41a 100644 --- a/tests/test_sweep.py +++ b/tests/test_sweep.py @@ -43,6 +43,7 @@ VerifyPolicy, ) from bmad_loop.sweep import ( + Bundle, Decision, DecisionOption, DecisionPrompter, @@ -994,6 +995,29 @@ def test_sweep_happy_path(project): assert "fix both" in intent and "DW-2" in intent and "### DW-3" in intent +def test_write_intent_neutralizes_surrogates_and_keeps_the_markdown(project): + """intent.md is written with `atomic_write_text` too, so a surrogate carried + by the triage session's authored prose crashes the strict encode exactly as + it does in the ledger — same revival chain, different file. What differs is + the remedy: line breaks are LEGITIMATE markdown here, so `_one_line`'s + collapse would be damage and only the surrogates are neutralized.""" + engine, _ = make_sweep(project, []) + bundle = Bundle( + name="fix-things", + dw_ids=(), + intent="keep\ud800this\n\nsecond para", + decision_note="note\ud800", + ) + + path = engine._write_intent(bundle, "fix-things") + + assert path.is_file() + text = path.read_text(encoding="utf-8") # strict read; the write did not raise + assert "note�" in text + # the surrogate is gone AND the paragraph break either side of it is verbatim + assert "keep�this\n\nsecond para" in text + + def test_sweep_is_exempt_from_the_dispatch_hard_gate(project): """The sweep must never be gated by the ledger it exists to drain. @@ -2832,6 +2856,43 @@ def test_migration_validation_failure_restores_ledger_then_escalates(project): assert "still parse as legacy" in feedback and "not mapped" in feedback +def test_migration_restore_write_failure_propagates_and_keeps_the_ledger(project, monkeypatch): + """#328. The restore after a failed migration is a repair write: it raises + rather than degrades, and it must never be the thing that empties the ledger + it exists to put back. Under a bare `Path.write_text` the truncate landed + before the failure did — the run crashed with the ledger at zero bytes, and + the `_safe_reset` that would have restored it was already spent. + + The patch is module-wide but reaches exactly one call — probed on this + harness, `_ensure_migration`'s restore is the only `sweep.atomic_write_text` + a failed migration reaches; the bundle-intent write needs a triage plan this + run never gets to.""" + write_legacy_ledger(project, LEGACY_LEDGER) + manifest = legacy_manifest() + half = ( + "# Deferred Work\n\n" + "### DW-1: Old fixed thing\n\norigin: migrated, 2026-06-12\nlocation: n/a\n" + "reason: repaired.\nstatus: done 2026-04-06\n\n" + "## Deferred from: epic 1 review (2026-04-06)\n\n" + "- **Open legacy thing here** — `src.txt` mishandles em-dashes\n" + ) + bad = migrate_effect(project, half, [{"key": manifest[0]["key"], "dw_id": "DW-1"}]) + engine, _ = make_sweep(project, [bad, bad]) + + def boom(path, text): + raise OSError("disk full") + + monkeypatch.setattr("bmad_loop.sweep.atomic_write_text", boom) + + summary = engine.run() + + assert summary.crashed and "disk full" in str(summary.crash_error) + # compared as TEXT, not bytes: the fixture reached disk through `write_text`, + # so a byte assertion would read CRLF on Windows and redden there only + assert project.deferred_work.read_text(encoding="utf-8") == LEGACY_LEDGER + assert project.deferred_work.read_bytes() # and emphatically not zero bytes + + def test_migration_escalation_resume_retries(project): write_legacy_ledger(project, LEGACY_LEDGER) manifest = legacy_manifest()