From b4c17dfb02ab376b6aa24f6a51ece711e980ade5 Mon Sep 17 00:00:00 2001 From: Jack McIntyre Date: Mon, 24 Aug 2026 20:36:26 +1000 Subject: [PATCH 01/16] feat(sweep): archive closed deferred-work entries (#706) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add `bmad-loop sweep --archive` moving closed (`status: done `) entries to a sibling deferred-work-archive.md, leaving minimal stubs that preserve the DW- id for grep and closes_deferred cross-references. Supports --before DATE to archive only entries closed before a cutoff, and --dry-run to preview. Deterministic Python — no LLM involvement. --- CHANGELOG.md | 8 ++ src/bmad_loop/cli.py | 63 ++++++++- src/bmad_loop/deferredwork.py | 134 ++++++++++++++++++ tests/test_cli.py | 50 +++++-- tests/test_deferredwork.py | 258 ++++++++++++++++++++++++++++++++++ 5 files changed, 498 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4241bc85..72d87f9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ breaking changes may land in a minor release. ## [Unreleased] +### Added + +- **`bmad-loop sweep --archive`** moves closed (`status: done`) deferred-work entries to a sibling + `deferred-work-archive.md`, replacing each with a minimal stub that preserves the DW- id for + grep and `closes_deferred` cross-references. The live ledger stays proportional to open work + rather than to all history. Supports `--before DATE` to archive only entries closed before a + cutoff, and `--dry-run` to preview. Pure deterministic Python — no LLM involvement. + ## [0.11.1] — 2026-08-23 ### Added diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 915595d1..58936bbc 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -482,8 +482,7 @@ def cmd_validate(args: argparse.Namespace) -> int: channel = ( "the ATTENTION file in the run directory is the only alert channel left" if pol.notify.file - else "notify.file is also off, so no alert channel is configured — " - "enable notify.file" + else "notify.file is also off, so no alert channel is configured — enable notify.file" ) report.warn( "notify.desktop-unavailable", @@ -2211,6 +2210,26 @@ def cmd_sweep(args: argparse.Namespace) -> int: return rc project = _project(args) paths = bmadconfig.load_paths(project) + + if args.before and not args.archive: + print("--before requires --archive", file=sys.stderr) + return ExitCode.FAILURE + + if args.archive: + if ( + args.decisions_only + or args.repeat is not None + or args.max_bundles is not None + or args.max_cycles is not None + ): + print( + "--archive cannot combine with --decisions-only, --repeat, " + "--max-bundles, or --max-cycles", + file=sys.stderr, + ) + return ExitCode.FAILURE + return _sweep_archive(paths, args) + pol = policy_mod.load(_policy_path(project)) if args.dry_run: @@ -2245,6 +2264,33 @@ def cmd_sweep(args: argparse.Namespace) -> int: ) +def _sweep_archive(paths: bmadconfig.ProjectPaths, args: argparse.Namespace) -> int: + """`bmad-loop sweep --archive`: move closed deferred-work entries to a + sibling archive file. A self-contained sub-mode — no worktree, no + preflight, no LLM.""" + try: + archived = deferredwork.archive_closed( + paths.deferred_work, + before=args.before, + dry_run=args.dry_run, + ) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return ExitCode.FAILURE + archive_path = paths.deferred_work.parent / deferredwork.ARCHIVE_REL + if not archived: + print("no closed entries to archive") + elif args.dry_run: + print(f"would archive {len(archived)} entries:") + for dw_id in archived: + print(f" {dw_id}") + else: + print(f"archived {len(archived)} entries to {archive_path}:") + for dw_id in archived: + print(f" {dw_id}") + return 0 + + def _sweep_dry_run(paths: bmadconfig.ProjectPaths, pol) -> int: # Before the no-ledger early return below: a broken install is worth saying so # about whether or not there is anything to sweep. @@ -4221,6 +4267,19 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: sweep_p.add_argument( "--dry-run", action="store_true", help="list open ledger entries, spawn nothing" ) + sweep_p.add_argument( + "--archive", + action="store_true", + help="move closed (status: done) deferred-work entries to a sibling " + "deferred-work-archive.md, leaving a minimal stub in the live ledger; " + "use --before DATE to archive only entries closed before that date, " + "and --dry-run to preview", + ) + sweep_p.add_argument( + "--before", + metavar="DATE", + help="with --archive: archive only entries closed before this ISO date", + ) sweep_p.add_argument("--run-id", help=argparse.SUPPRESS) # pre-assigned id (used by the TUI) resume_p = add("resume", cmd_resume, "resume a paused run") diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index bb973d90..4c1bde25 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -1044,6 +1044,140 @@ def append_entry( return dw_id +ARCHIVE_REL = "deferred-work-archive.md" +# A stub left by a prior archive_closed run carries this field. The next run +# reads it to skip entries whose body has already been moved — without it, +# every run would re-archive the stub (a heading + status line) and the +# archive would accumulate duplicates. +_ARCHIVED_FIELD_RE = re.compile(r"^archived:", re.MULTILINE) + + +def _is_archived(entry: DWEntry) -> bool: + """Whether the entry carries a live ``archived:`` field line (not a quoted + example), marking it as a stub from a prior :func:`archive_closed` run. + + Reads through :func:`_quoted` for the same reason every gate scan does: + an entry documenting the archive field in a fenced example carries the + line in column 0, right where the anchor looks, and without the fence + check a quoted ``archived:`` would be mistaken for the real thing. + """ + return any(not _quoted(entry, m.start()) for m in _ARCHIVED_FIELD_RE.finditer(entry.body)) + + +def _close_date(entry: DWEntry) -> str | None: + """The ISO close date from a ``done `` status, or None when the + entry is not done, is done without a date suffix, or carries a date + that does not match the ISO ``YYYY-MM-DD`` shape. + + Entries closed with a bare ``status: done`` (no date) or a hand-edited + non-ISO date are skipped by :func:`archive_closed`: there is no close + date to compare against a ``--before`` cutoff, and the stub the function + leaves in the ledger needs one to stay readable as done. + """ + if not entry.done: + return None + parts = entry.status.split() + if len(parts) < 2: + return None + date_str = parts[1] + return date_str if _ISO_DATE_RE.fullmatch(date_str) else None + + +def archive_closed( + path: Path, + *, + before: str | None = None, + archive_date: str | None = None, + dry_run: bool = False, +) -> list[str]: + """Move closed (``status: done ``) ledger entries to a sibling + archive file (:data:`ARCHIVE_REL`), replacing each with a minimal stub + that preserves the DW- id for grep and ``closes_deferred`` + cross-references. + + Returns the list of archived ids, in ledger order. ``dry_run=True`` + returns the ids that *would* be archived without writing anything. + + Each archived entry's body is preserved verbatim in the archive file, + with an ``archived: `` field line appended after the entry's status + line. The stub left in the live ledger keeps the heading, a ``status: + done `` line (so :func:`parse_ledger` reads it as done and + :func:`open_ids` drops it), and an ``archived: `` line (so a + subsequent run skips it rather than re-archiving the stub). + + ``before`` (ISO ``YYYY-MM-DD``) archives only entries closed strictly + *before* that date. Entries with ``status: done`` (no date) are always + skipped — there is no close date to compare against a cutoff or to stamp + the stub with. Open and legacy entries are never touched. + + Dates are validated with :func:`_require_iso_date` (same validation as + the existing close-path writers), ahead of the ``is_file`` short-circuit + so a programmer bug fails the same way whether or not a ledger exists. + Both writes — the trimmed ledger and the appended archive — go through + :func:`atomic_write_text`, the same primitive every ledger writer uses. + The archive file accumulates on repeat runs: new entries are appended to + the existing file, never overwritten, and stubs from a prior run are + skipped by their ``archived:`` line. + """ + if before is not None: + _require_iso_date(before) + if archive_date is not None: + _require_iso_date(archive_date) + if not path.is_file(): + return [] + text = path.read_text(encoding="utf-8") + to_archive: list[tuple[DWEntry, str]] = [] + for entry in parse_ledger(text): + close_date = _close_date(entry) + if close_date is None: + continue # not done, or done without a date + if before is not None and close_date >= before: + continue # closed on or after the cutoff + if _is_archived(entry): + continue # stub from a prior archive_closed run + to_archive.append((entry, close_date)) + if not to_archive: + return [] + archived_ids = [e.id for e, _ in to_archive] + if dry_run: + return archived_ids + stamp = archive_date or calendar_date.today().isoformat() + archive_path = path.parent / ARCHIVE_REL + existing = archive_path.read_text(encoding="utf-8") if archive_path.is_file() else "" + # Append an `archived:` line after each entry's status line. The status + # span is body-relative, so the insertion works within the body slice — + # same offset math as `_insert_after_status`, applied to the body. + archive_blocks: list[str] = [] + for entry, _ in to_archive: + body = entry.body + assert entry.status_span is not None # done with a date implies a status line + pos = entry.status_span[1] + body = body[:pos] + f"\narchived: {stamp}" + body[pos:] + archive_blocks.append(body) + if existing == "" or existing.endswith("\n\n"): + sep = "" + elif existing.endswith("\n"): + sep = "\n" + else: + sep = "\n\n" + archive_content = existing + sep + "".join(archive_blocks) + # Replace each archived entry's span with a stub, working backwards so + # earlier spans are unaffected by later replacements — the same + # text-surgery pattern as `_apply_done`, applied to multiple entries. + for entry, close_date in reversed(to_archive): + stub = f"### {entry.id}: {entry.title}\n\nstatus: done {close_date}\narchived: {stamp}\n\n" + start, end = entry.span + text = text[:start] + stub + text[end:] + # Write the archive BEFORE the ledger: a crash between writes leaves the + # archive with extra content (harmless — the archive is append-only) and + # the ledger unchanged (safe — the bodies are still in the live file). + # Writing the ledger first would leave stubs in the ledger with no bodies + # in the archive — content lost. + atomic_write_text(archive_path, archive_content) + atomic_write_text(path, text) + return archived_ids + + # ------------------------------------------------------------------- legacy # # Ledgers written before the DW format (older BMAD-method projects) are diff --git a/tests/test_cli.py b/tests/test_cli.py index 9ea43d12..19e00a0e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1583,10 +1583,12 @@ def test_attach_records_return_pane_inside_tmux(project, monkeypatch): monkeypatch.setattr( launch, "attach_plan", - lambda proj, rid: planned.append((proj, rid)) - or ( - ["tmux", "switch-client", "-t", "=bmad-loop-ctl"], - "=bmad-loop-ctl:sweep-RID", + lambda proj, rid: ( + planned.append((proj, rid)) + or ( + ["tmux", "switch-client", "-t", "=bmad-loop-ctl"], + "=bmad-loop-ctl:sweep-RID", + ) ), ) monkeypatch.setenv("TMUX", "/tmp/tmux-1000/default,1,0") @@ -8600,9 +8602,9 @@ def run(self): # the gate saw the honest bytes and passed factory("epic-boundary", started=lambda: signalled.append("started")) - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" assert signalled == ["started"] # #501: the child composed, so the parent may latch run_id = captured["state"].run_id @@ -8679,9 +8681,9 @@ def run(self): assert cli.main(["run", "--project", str(project.project)]) == 0 - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" run_id = captured["state"].run_id assert runs.read_trusted_config_digest(project.project, run_id) == pin @@ -8739,9 +8741,9 @@ def __init__(self, **kwargs): assert cli._resume_paused_run(project.project, run_dir) == 0 - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" assert runs.read_trusted_config_digest(project.project, run_dir.name) == pin @@ -9135,3 +9137,25 @@ def test_dry_run_banner_names_an_under_floor_git(project, capsys, monkeypatch): err = capsys.readouterr().err assert "NOT runnable" in err assert "2.25.1" in err and verify.git_floor_text() in err + + +def test_sweep_archive_rejects_bad_before_date(project, capsys): + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + rc = cli.main(["sweep", "--archive", "--before", "bad-date", "--project", str(project.project)]) + assert rc == 1 + assert "date must be YYYY-MM-DD" in capsys.readouterr().err + + +def test_sweep_archive_dry_run(project, capsys): + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + rc = cli.main(["sweep", "--archive", "--dry-run", "--project", str(project.project)]) + assert rc == 0 + out = capsys.readouterr().out + assert "would archive 1 entries" in out + assert "DW-2" in out diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 36674625..63712710 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -7,10 +7,12 @@ from bmad_loop import deferredwork, fences from bmad_loop.deferredwork import ( _ISO_DATE_RE, + ARCHIVE_REL, LINE_BREAK_RE, SEVERITY_ALIASES, append_decision, append_entry, + archive_closed, classify, field_line_present, field_severity, @@ -2350,3 +2352,259 @@ def test_an_unclosed_fence_swallows_no_prose_gate(): """Parity with `gates()`: `unclosed_hides_rest=False`, so one stray ``` cannot silence every declaration below it.""" assert _prose_gated("```", "an example nobody closed", "HARD GATE: for real") is True + + +# ------------------------------------------------------- archive_closed (#706) +# +# Closed entries are moved verbatim to a sibling archive file; a minimal +# stub (heading + status + archived line) replaces each in the live ledger +# so parse_ledger reads it as done, open_ids drops it, and a subsequent run +# skips it by the `archived:` line rather than re-archiving the stub. + + +def test_archive_all_done(tmp_path): + """Done entries are moved verbatim to the archive file and replaced with + minimal stubs that parse as done; open entries are untouched.""" + path = write_ledger(tmp_path) + archived = archive_closed(path, archive_date="2026-08-24") + assert archived == ["DW-2"] + + text = path.read_text(encoding="utf-8") + entries = {e.id: e for e in parse_ledger(text)} + # The stub still parses as done, with the original close date + assert entries["DW-2"].done + assert entries["DW-2"].status == "done 2026-05-25" + assert "origin: code review" not in entries["DW-2"].body # body was moved + assert entries["DW-2"].title == "Old closed item" # heading preserved + assert "archived: 2026-08-24" in entries["DW-2"].body # stub marker + # Open entries untouched + assert entries["DW-1"].open + assert "origin: quick-dev" in entries["DW-1"].body + assert entries["DW-3"].open + assert "seen-again" in entries["DW-3"].body + + # The archive file has the full body with an `archived:` line + archive_path = path.parent / ARCHIVE_REL + assert archive_path.is_file() + archive_text = archive_path.read_text(encoding="utf-8") + assert "### DW-2: Old closed item" in archive_text + assert "origin: code review of spec-1-1.md" in archive_text + assert "status: done 2026-05-25" in archive_text + assert "archived: 2026-08-24" in archive_text + + +def test_archive_before_cutoff(tmp_path): + """--before archives only entries closed strictly before that date.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: May item\n\norigin: a\nstatus: done 2026-05-15\n\n" + "### DW-2: july item\n\norigin: b\nstatus: done 2026-07-01\n\n" + "### DW-3: still open\n\norigin: c\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + archived = archive_closed(path, before="2026-06-01", archive_date="2026-08-24") + assert archived == ["DW-1"] + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert entries["DW-1"].done # stub + assert "origin: a" not in entries["DW-1"].body + # July entry untouched — not before the cutoff + assert entries["DW-2"].status == "done 2026-07-01" + assert "origin: b" in entries["DW-2"].body + assert entries["DW-3"].open + + +def test_archive_dry_run(tmp_path): + """Dry run returns the ids that would be archived but writes nothing.""" + path = write_ledger(tmp_path) + before = path.read_text(encoding="utf-8") + archived = archive_closed(path, dry_run=True) + assert archived == ["DW-2"] + assert path.read_text(encoding="utf-8") == before + assert not (path.parent / ARCHIVE_REL).exists() + + +def test_archive_no_done_entries(tmp_path): + """A ledger with only open entries returns an empty list and writes nothing.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: open one\n\norigin: a\nstatus: open\n\n" + "### DW-2: open two\n\norigin: b\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + before = path.read_text(encoding="utf-8") + assert archive_closed(path) == [] + assert path.read_text(encoding="utf-8") == before + assert not (path.parent / ARCHIVE_REL).exists() + + +def test_archive_no_ledger(tmp_path): + """A missing ledger file returns an empty list.""" + path = tmp_path / "deferred-work.md" + assert archive_closed(path) == [] + assert not (path.parent / ARCHIVE_REL).exists() + + +def test_archive_done_without_date_skipped(tmp_path): + """An entry with `status: done` (no date) is skipped — there is no close + date to compare against a cutoff or to stamp the stub with.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: closed without date\n\norigin: a\nstatus: done\n\n" + "### DW-2: closed with date\n\norigin: b\nstatus: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, text) + archived = archive_closed(path, archive_date="2026-08-24") + assert archived == ["DW-2"] + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert entries["DW-1"].status == "done" # untouched + assert "origin: a" in entries["DW-1"].body # body still there + assert entries["DW-2"].done # stub + + +def test_archive_rerun_appends_to_existing(tmp_path): + """A second run appends new entries to the existing archive without + overwriting, and skips stubs left by the first run.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: first done\n\norigin: a\nstatus: done 2026-05-15\n\n" + "### DW-2: second done\n\norigin: b\nstatus: done 2026-06-01\n\n" + "### DW-3: open\n\norigin: c\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + # First run: archive only the May entry + archive_closed(path, before="2026-06-01", archive_date="2026-08-24") + archive_path = path.parent / ARCHIVE_REL + first = archive_path.read_text(encoding="utf-8") + assert "DW-1" in first + assert "DW-2" not in first + + # Close DW-3 and archive the rest — DW-1's stub is skipped + mark_done(path, "DW-3", "2026-07-01", "resolved") + archived = archive_closed(path, archive_date="2026-08-25") + assert set(archived) == {"DW-2", "DW-3"} + + second = archive_path.read_text(encoding="utf-8") + # Both old and new entries are in the archive + assert "DW-1" in second and "DW-2" in second and "DW-3" in second + # The first run's stamp is preserved (not overwritten) + assert "archived: 2026-08-24" in second + assert "archived: 2026-08-25" in second + + +def test_archive_stub_preserves_id_and_parses_as_done(tmp_path): + """The stub's heading and status line let parse_ledger read it as done + and open_ids exclude it, while the DW- id stays findable for grep.""" + path = write_ledger(tmp_path) + archive_closed(path, archive_date="2026-08-24") + + text = path.read_text(encoding="utf-8") + entries = {e.id: e for e in parse_ledger(text)} + assert entries["DW-2"].id == "DW-2" + assert entries["DW-2"].done + assert not entries["DW-2"].open + assert "DW-2" not in open_ids(text) + + +def test_archive_open_entries_untouched(tmp_path): + """Open entries are never modified — their bodies stay byte-identical.""" + path = write_ledger(tmp_path) + before_open = {e.id: e.body for e in parse_ledger(path.read_text(encoding="utf-8")) if e.open} + archive_closed(path, archive_date="2026-08-24") + after = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + for dw_id, body in before_open.items(): + assert after[dw_id].body == body + + +def test_archive_validates_before_date(tmp_path): + """An invalid --before date raises ValueError without writing anything.""" + path = write_ledger(tmp_path) + before = path.read_text(encoding="utf-8") + with pytest.raises(ValueError, match="date must be YYYY-MM-DD"): + archive_closed(path, before="not-a-date") + assert path.read_text(encoding="utf-8") == before + assert not (path.parent / ARCHIVE_REL).exists() + + +def test_archive_validates_archive_date(tmp_path): + """An invalid archive_date raises ValueError without writing anything.""" + path = write_ledger(tmp_path) + before = path.read_text(encoding="utf-8") + with pytest.raises(ValueError, match="date must be YYYY-MM-DD"): + archive_closed(path, archive_date="2026-13-01") + assert path.read_text(encoding="utf-8") == before + + +def test_archive_bad_date_raises_even_with_no_ledger(tmp_path): + """Validated at function entry, ahead of the is_file short-circuit.""" + path = tmp_path / "nope.md" + with pytest.raises(ValueError, match="date must be YYYY-MM-DD"): + archive_closed(path, before="nope") + with pytest.raises(ValueError, match="date must be YYYY-MM-DD"): + archive_closed(path, archive_date="nope") + + +def test_archive_skips_already_archived_stubs(tmp_path): + """A second run with no new closures finds only stubs (which carry an + `archived:` line) and archives nothing.""" + path = write_ledger(tmp_path) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + assert archive_closed(path, archive_date="2026-08-25") == [] + archive_path = path.parent / ARCHIVE_REL + assert "archived: 2026-08-25" not in archive_path.read_text(encoding="utf-8") + + +def test_archive_default_archive_date_is_today(tmp_path): + """When archive_date is not supplied, the stub and archive carry today's date.""" + from datetime import date as calendar_date + + path = write_ledger(tmp_path) + archive_closed(path) + today = calendar_date.today().isoformat() + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert f"archived: {today}" in entries["DW-2"].body + archive_path = path.parent / ARCHIVE_REL + assert f"archived: {today}" in archive_path.read_text(encoding="utf-8") + + +def test_archive_multi_entry_reparse(tmp_path): + """Archiving 2+ done entries in one call leaves stubs that re-parse with + correct id, title, status, and archived line; open entries are byte-identical.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: first done\n\norigin: a\nstatus: done 2026-05-15\n\n" + "### DW-2: open one\n\norigin: b\nstatus: open\n\n" + "### DW-3: second done\n\norigin: c\nstatus: done 2026-06-01\n\n" + "### DW-4: open two\n\norigin: d\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + before_open = {e.id: e.body for e in parse_ledger(path.read_text(encoding="utf-8")) if e.open} + archived = archive_closed(path, archive_date="2026-08-24") + assert set(archived) == {"DW-1", "DW-3"} + + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + for dw_id, title, close_date in [ + ("DW-1", "first done", "2026-05-15"), + ("DW-3", "second done", "2026-06-01"), + ]: + assert entries[dw_id].id == dw_id + assert entries[dw_id].title == title + assert entries[dw_id].done + assert entries[dw_id].status == f"done {close_date}" + assert "archived: 2026-08-24" in entries[dw_id].body + for dw_id, body in before_open.items(): + assert entries[dw_id].body == body + + +def test_archive_before_boundary_excludes_cutoff_date(tmp_path): + """An entry closed exactly on the cutoff date is NOT archived (strict <).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: on the boundary\n\norigin: a\nstatus: done 2026-06-01\n\n" + "### DW-2: before the boundary\n\norigin: b\nstatus: done 2026-05-31\n" + ) + path = write_ledger(tmp_path, text) + archived = archive_closed(path, before="2026-06-01", archive_date="2026-08-24") + assert archived == ["DW-2"] # DW-1 is ON the cutoff, excluded by strict < From f2a98770dab1a25c64c55c567751f38f5d4a4f57 Mon Sep 17 00:00:00 2001 From: Jack McIntyre Date: Mon, 24 Aug 2026 21:20:16 +1000 Subject: [PATCH 02/16] fix(sweep): harden archive close-date parsing and crash recovery (#706 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - _close_date now requires exactly `done YYYY-MM-DD` (extra tokens skipped) and validates the calendar day with fromisoformat — a well-shaped impossible date (2026-02-30) no longer passes selection. - Crash between the archive write and the ledger write: a retry completes the stubbing but skips bodies already present in the archive, so the append-only archive cannot accumulate duplicates. - Restore black formatting in tests/test_cli.py (ruff format had restyled pre-existing assert lines; trunk pins black@26.5.1 — CI lint failure). --- src/bmad_loop/deferredwork.py | 37 +++++++++++++++----- tests/test_cli.py | 18 +++++----- tests/test_deferredwork.py | 63 +++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 17 deletions(-) diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index 4c1bde25..455a836b 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -1077,10 +1077,18 @@ def _close_date(entry: DWEntry) -> str | None: if not entry.done: return None parts = entry.status.split() - if len(parts) < 2: + if len(parts) != 2: # exactly `done YYYY-MM-DD` — extra tokens are not a close date return None date_str = parts[1] - return date_str if _ISO_DATE_RE.fullmatch(date_str) else None + if not _ISO_DATE_RE.fullmatch(date_str): + return None + try: + # The regex alone admits well-shaped impossible days (2026-02-30) that + # no calendar carries — the same half of `_require_iso_date`. + calendar_date.fromisoformat(date_str) + except ValueError: + return None + return date_str def archive_closed( @@ -1147,20 +1155,33 @@ def archive_closed( # Append an `archived:` line after each entry's status line. The status # span is body-relative, so the insertion works within the body slice — # same offset math as `_insert_after_status`, applied to the body. + # + # Crash recovery: the archive is written BEFORE the ledger (see below), so + # a crash between the two writes leaves the ledger with full entries whose + # bodies are already in the archive. A retry must still stub those ledger + # entries (completing the interrupted operation) but must NOT append their + # bodies again — an append-only archive accumulating duplicates. Entries + # whose heading is already present in the existing archive text are + # therefore skipped here and only replaced with stubs below. archive_blocks: list[str] = [] for entry, _ in to_archive: + if f"### {entry.id}:" in existing: + continue # body already archived by a crashed prior run body = entry.body assert entry.status_span is not None # done with a date implies a status line pos = entry.status_span[1] body = body[:pos] + f"\narchived: {stamp}" + body[pos:] archive_blocks.append(body) - if existing == "" or existing.endswith("\n\n"): - sep = "" - elif existing.endswith("\n"): - sep = "\n" + if archive_blocks: + if existing == "" or existing.endswith("\n\n"): + sep = "" + elif existing.endswith("\n"): + sep = "\n" + else: + sep = "\n\n" + archive_content = existing + sep + "".join(archive_blocks) else: - sep = "\n\n" - archive_content = existing + sep + "".join(archive_blocks) + archive_content = existing # pure crash-recovery pass: only stub the ledger # Replace each archived entry's span with a stub, working backwards so # earlier spans are unaffected by later replacements — the same # text-surgery pattern as `_apply_done`, applied to multiple entries. diff --git a/tests/test_cli.py b/tests/test_cli.py index 19e00a0e..40862bea 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -8602,9 +8602,9 @@ def run(self): # the gate saw the honest bytes and passed factory("epic-boundary", started=lambda: signalled.append("started")) - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" assert signalled == ["started"] # #501: the child composed, so the parent may latch run_id = captured["state"].run_id @@ -8681,9 +8681,9 @@ def run(self): assert cli.main(["run", "--project", str(project.project)]) == 0 - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" run_id = captured["state"].run_id assert runs.read_trusted_config_digest(project.project, run_id) == pin @@ -8741,9 +8741,9 @@ def __init__(self, **kwargs): assert cli._resume_paused_run(project.project, run_dir) == 0 - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" assert runs.read_trusted_config_digest(project.project, run_dir.name) == pin diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 63712710..7e622abf 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2608,3 +2608,66 @@ def test_archive_before_boundary_excludes_cutoff_date(tmp_path): path = write_ledger(tmp_path, text) archived = archive_closed(path, before="2026-06-01", archive_date="2026-08-24") assert archived == ["DW-2"] # DW-1 is ON the cutoff, excluded by strict < + + +def test_archive_rejects_status_with_extra_tokens(tmp_path): + """A status like `done 2026-05-25 junk` is not a close date — exactly two + tokens are required, so the entry is skipped rather than archived on a + garbage date (#706 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: extra tokens\n\norigin: a\nstatus: done 2026-05-25 junk\n\n" + "### DW-2: clean close\n\norigin: b\nstatus: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert "origin: a" in entries["DW-1"].body # untouched + + +def test_archive_rejects_impossible_calendar_date(tmp_path): + """A well-shaped impossible day (2026-02-30) passes the ISO regex but no + calendar carries it — skipped, not archived (#706 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: feb 30\n\norigin: a\nstatus: done 2026-02-30\n\n" + "### DW-2: real date\n\norigin: b\nstatus: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert "origin: a" in entries["DW-1"].body # untouched + + +def test_archive_crash_recovery_no_duplicate_bodies(tmp_path): + """Crash between the archive write and the ledger write leaves full entries + in the ledger with bodies already archived. A retry must stub the ledger + entries (completing the operation) without appending duplicate bodies + (#706 review).""" + path = write_ledger(tmp_path) + # Simulate the crashed first run: the body landed in the archive, but the + # ledger was never trimmed (still holds the full DW-2 entry). + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text( + "### DW-2: Old closed item\n\n" + "origin: code review of spec-1-1.md, 2026-05-20\n" + "location: src/foo.py:10\n" + "reason: pre-existing.\n" + "status: done 2026-05-25\n" + "archived: 2026-08-24\n", + encoding="utf-8", + ) + archived = archive_closed(path, archive_date="2026-08-25") + assert archived == ["DW-2"] # the operation completes: stub written + + # The ledger now holds the stub... + entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} + assert entries["DW-2"].done + assert "origin: code review" not in entries["DW-2"].body + + # ...and the archive carries the body exactly once, with the FIRST run's + # stamp preserved — no duplicate append, no re-stamp. + archive_text = archive_path.read_text(encoding="utf-8") + assert archive_text.count("### DW-2:") == 1 + assert "archived: 2026-08-24" in archive_text + assert "archived: 2026-08-25" not in archive_text From acc107111067c6a8d0439d9f074b8a944d1f7370 Mon Sep 17 00:00:00 2001 From: Jack McIntyre Date: Mon, 24 Aug 2026 21:47:24 +1000 Subject: [PATCH 03/16] fix(sweep): fence-aware crash-recovery skip in archive (#706 review) The duplicate-body skip matched `### DW-:` as a raw substring of the archive text, so a fenced worked example quoting the heading would falsely suppress archiving a live entry. The skip now parses the archive with the fence-aware ledger parser and counts only entries carrying a live (non-fenced) archived: field. --- src/bmad_loop/deferredwork.py | 9 ++++++--- tests/test_deferredwork.py | 27 +++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index 455a836b..adeb3694 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -1161,11 +1161,14 @@ def archive_closed( # bodies are already in the archive. A retry must still stub those ledger # entries (completing the interrupted operation) but must NOT append their # bodies again — an append-only archive accumulating duplicates. Entries - # whose heading is already present in the existing archive text are - # therefore skipped here and only replaced with stubs below. + # whose parsed archive twin carries a live (non-fenced) ``archived:`` + # field are therefore skipped here and only replaced with stubs below. archive_blocks: list[str] = [] + already_archived = { + e.id for e in parse_ledger(existing) if _is_archived(e) + } # fence-aware: a quoted example in the archive is not a real body for entry, _ in to_archive: - if f"### {entry.id}:" in existing: + if entry.id in already_archived: continue # body already archived by a crashed prior run body = entry.body assert entry.status_span is not None # done with a date implies a status line diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 7e622abf..8457092b 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2671,3 +2671,30 @@ def test_archive_crash_recovery_no_duplicate_bodies(tmp_path): assert archive_text.count("### DW-2:") == 1 assert "archived: 2026-08-24" in archive_text assert "archived: 2026-08-25" not in archive_text + + +def test_archive_crash_recovery_fenced_example_does_not_suppress(tmp_path): + """A fenced worked example in the archive quoting `### DW-2:` is not a + real archived body — the crash-recovery skip must be fence-aware, so the + live DW-2 is still archived rather than silently kept (#706 review).""" + path = write_ledger(tmp_path) + # An archive whose only DW-2 mention is inside a fenced example entry: + # the example carries no live `archived:` field, so it must not count. + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text( + "# Archived Deferred Work\n\n" + "```markdown\n" + "### DW-2: Old closed item\n\n" + "origin: quoted example, not a real body\n" + "status: done 2026-05-25\n" + "```\n", + encoding="utf-8", + ) + archived = archive_closed(path, archive_date="2026-08-24") + assert archived == ["DW-2"] # not suppressed by the fenced heading + + # The real body was appended; the fenced example survives verbatim above it + archive_text = archive_path.read_text(encoding="utf-8") + assert archive_text.count("### DW-2:") == 2 # quoted + real + assert "origin: code review of spec-1-1.md" in archive_text + assert "quoted example" in archive_text From 32f5d369c8cbf9b6260d0b378a4768c12cfa55c8 Mon Sep 17 00:00:00 2001 From: Jack McIntyre Date: Mon, 24 Aug 2026 22:11:54 +1000 Subject: [PATCH 04/16] fix(sweep): harden ledger archiving (#706 review, pass 2) Stubs keep load-bearing field lines (gate/origin/source_spec/undo tail); crash-recovery skip keys on id + close date; CLI refuses while a run is live, names a missing ledger, guards --before ""/--no-prompt/--run-id; docs and ablation-proof tests added. --- CHANGELOG.md | 12 +- README.md | 2 +- docs/FEATURES.md | 1 + src/bmad_loop/cli.py | 59 ++++-- .../bmad-loop-sweep/deferred-work-format.md | 31 ++- src/bmad_loop/deferredwork.py | 133 ++++++++++--- tests/test_cli.py | 105 +++++++++- tests/test_deferredwork.py | 188 +++++++++++++++++- 8 files changed, 459 insertions(+), 72 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d87f9b..18dd01d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,13 @@ breaking changes may land in a minor release. ### Added -- **`bmad-loop sweep --archive`** moves closed (`status: done`) deferred-work entries to a sibling - `deferred-work-archive.md`, replacing each with a minimal stub that preserves the DW- id for - grep and `closes_deferred` cross-references. The live ledger stays proportional to open work - rather than to all history. Supports `--before DATE` to archive only entries closed before a - cutoff, and `--dry-run` to preview. Pure deterministic Python — no LLM involvement. +- **`bmad-loop sweep --archive`** moves closed (`status: done `) deferred-work entries to + a sibling `deferred-work-archive.md`, replacing each with a stub that preserves the DW- id for + grep and `closes_deferred` cross-references plus the load-bearing field lines (`gate:`, + `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger stays proportional to + open work rather than to all history. Supports `--before DATE` to archive only entries closed + before a cutoff, and `--dry-run` to preview. Refuses while any engine run is live. Pure + deterministic Python — no LLM involvement. ## [0.11.1] — 2026-08-23 diff --git a/README.md b/README.md index 436d94b5..48752450 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ bmad-loop tui # …or drive everything from the dashboard | `bmad-loop mux` | List registered terminal-multiplexer backends — platform match, availability, version, and which one is selected (and why). `mux set ` persists a machine-scoped choice into `.bmad-loop/policy.toml` (`--clear` reverts to auto-select, `--force` allows a name that only registers on the target machine); the `BMAD_LOOP_MUX_BACKEND` env var outranks it. | | `bmad-loop adapters` | List registered coding-CLI adapter **kinds** — name, builtin/external, whether the family drives a multiplexer, and which profiles select each — the CLI axis's counterpart to `mux`. Unlike `mux` there is no global choice to persist: a kind is selected per profile by its `adapter` field. A profile naming an unregistered kind, and any out-of-tree adapter/profile package that failed to load, get a `warning:` on stderr. | | `bmad-loop run` | Drive the dev → review → verify → commit loop. `--epic N`, `--story KEY`, `--max-stories N`, `--dry-run`. `--spec ` forces **stories mode** (folder+id dispatch off `/stories.yaml`), overriding `[stories].source`; `--story` then filters by story id. | -| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. | +| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. `--archive [--before DATE]` instead moves closed ledger entries to `deferred-work-archive.md`, leaving id-preserving stubs. | | `bmad-loop resume ` | Continue a run paused at a gate, escalation, or interruption. | | `bmad-loop resolve ` | Resolve a CRITICAL escalation: open an interactive resolve agent to fix the frozen spec, then re-arm the story and resume. On an _intent gap_ the re-drive can resume review on the attempted change instead of re-implementing it. `--story KEY`, `--no-interactive`, `--restore-patch ` (intent-gap patch-restore), `--resume` / `--no-resume`, `--force` (proceed when engine liveness is unverifiable; a provably-live engine still blocks). | | `bmad-loop decisions` | Answer deferred-work decisions earlier sweeps left unanswered (skipped by `--no-prompt`, or an abandoned interactive sweep). Recorded so the next sweep acts on them without re-asking. `--list` shows them without answering; `--json` emits them as a stable machine-readable document — id, question, context, recommendation, and every option's key/label/effect/intent/resolution/bundle-name with a derived `recommended` flag. It implies the listing and never prompts, so a script can select an option by policy instead of scraping the text. | diff --git a/docs/FEATURES.md b/docs/FEATURES.md index daf9d6ec..b2f31060 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,6 +133,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. +- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done `) entries verbatim to sibling `deferred-work-archive.md`, leaving an id-preserving stub (`status: done ` + `archived: `) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive ` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume `). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve --restore-patch ` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 58936bbc..302cd777 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2211,7 +2211,7 @@ def cmd_sweep(args: argparse.Namespace) -> int: project = _project(args) paths = bmadconfig.load_paths(project) - if args.before and not args.archive: + if args.before is not None and not args.archive: print("--before requires --archive", file=sys.stderr) return ExitCode.FAILURE @@ -2221,14 +2221,16 @@ def cmd_sweep(args: argparse.Namespace) -> int: or args.repeat is not None or args.max_bundles is not None or args.max_cycles is not None + or args.no_prompt + or args.run_id is not None ): print( "--archive cannot combine with --decisions-only, --repeat, " - "--max-bundles, or --max-cycles", + "--max-bundles, --max-cycles, --no-prompt, or --run-id", file=sys.stderr, ) return ExitCode.FAILURE - return _sweep_archive(paths, args) + return _sweep_archive(project, paths, args) pol = policy_mod.load(_policy_path(project)) @@ -2264,31 +2266,47 @@ def cmd_sweep(args: argparse.Namespace) -> int: ) -def _sweep_archive(paths: bmadconfig.ProjectPaths, args: argparse.Namespace) -> int: +def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse.Namespace) -> int: """`bmad-loop sweep --archive`: move closed deferred-work entries to a sibling archive file. A self-contained sub-mode — no worktree, no - preflight, no LLM.""" + preflight, no LLM. Refuses while any engine run is live: this is the one + out-of-band ledger writer, and a concurrent close or harvest landing + between its read and its writes would be silently clobbered.""" + for run_dir in runs.list_run_dirs(project): + if runs.engine_liveness(run_dir) == "alive": + print( + f"run {run_dir.name} is live — stop it before archiving ledger entries", + file=sys.stderr, + ) + return ExitCode.FAILURE + ledger = paths.deferred_work + if not ledger.is_file(): + print(f"no deferred-work ledger at {ledger}") + return ExitCode.OK try: archived = deferredwork.archive_closed( - paths.deferred_work, + ledger, before=args.before, dry_run=args.dry_run, ) except ValueError as exc: print(f"error: {exc}", file=sys.stderr) return ExitCode.FAILURE - archive_path = paths.deferred_work.parent / deferredwork.ARCHIVE_REL + archive_path = ledger.parent / deferredwork.ARCHIVE_REL if not archived: print("no closed entries to archive") - elif args.dry_run: - print(f"would archive {len(archived)} entries:") - for dw_id in archived: - print(f" {dw_id}") - else: - print(f"archived {len(archived)} entries to {archive_path}:") + return ExitCode.OK + noun = "entry" if len(archived) == 1 else "entries" + if args.dry_run: + print(f"would archive {len(archived)} {noun}:") for dw_id in archived: print(f" {dw_id}") - return 0 + return ExitCode.OK + print(f"archived {len(archived)} {noun} to {archive_path}:") + for dw_id in archived: + print(f" {dw_id}") + print("note: the ledger and archive are tracked files — commit them to make the move durable") + return ExitCode.OK def _sweep_dry_run(paths: bmadconfig.ProjectPaths, pol) -> int: @@ -4265,15 +4283,18 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: ) sweep_p.add_argument("--max-cycles", type=int, help="override [sweep] max_cycles") sweep_p.add_argument( - "--dry-run", action="store_true", help="list open ledger entries, spawn nothing" + "--dry-run", + action="store_true", + help="list open ledger entries, spawn nothing; with --archive: list the " + "entries that would move, write nothing", ) sweep_p.add_argument( "--archive", action="store_true", - help="move closed (status: done) deferred-work entries to a sibling " - "deferred-work-archive.md, leaving a minimal stub in the live ledger; " - "use --before DATE to archive only entries closed before that date, " - "and --dry-run to preview", + help="move closed (status: done ) deferred-work entries to a " + "sibling deferred-work-archive.md, leaving a minimal stub in the live " + "ledger; use --before DATE to archive only entries closed before that " + "date, and --dry-run to preview", ) sweep_p.add_argument( "--before", diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index 03cb9b56..7e2dbcee 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -20,7 +20,36 @@ splits are a legacy/attended source only — the current unattended primitive do not split a multi-goal spec, it records a `multiple-goals` warning in the spec's `warnings:` and proceeds. -The file is append-only — never rewrite or delete existing entries. +The file is append-only — never rewrite or delete existing entries. The one +sanctioned rewrite is operator-run archiving (below): closed entries move to +`deferred-work-archive.md` verbatim, leaving a stub behind. + +## Archiving (`bmad-loop sweep --archive`) + +The operator may periodically move closed entries +(`status: done `) to the sibling `deferred-work-archive.md`, leaving +a stub in this file: + +```markdown +### DW-7: Old closed item + +status: done 2026-05-25 +archived: 2026-08-24 +``` + +Rules for sessions reading the ledger: + +- The stub's `status: done` line means what it always meant — the entry is + closed and not open work. +- An `archived:` line marks content that lives in `deferred-work-archive.md`. + The full body (evidence, resolution, dates) is there, keyed by the same + DW- id — read the archive file for anything beyond the stub. +- Stubs keep load-bearing field lines (`gate:`, `origin:`/`source_spec:`, + resolution undo markers) — treat them exactly as if the entry were still + whole. Never edit or drop them when touching a stub. +- When deduping against existing entries, a stub still counts: match on the + id and preserved `origin:`/`source_spec:` lines, and check the archive for + the full substance before appending. ## Before appending: dedupe check diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index adeb3694..0fe81049 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -3,7 +3,9 @@ The ledger (`{implementation_artifacts}/deferred-work.md`) is append-only markdown in the canonical form documented at bmad-loop-sweep/deferred-work-format.md: `### DW-: ` headings with -`origin:`/`location:`/`reason:`/`status:` field lines. Pre-#2651 dev primitives +`origin:`/`location:`/`reason:`/`status:` field lines. The one sanctioned +rewrite is :func:`archive_closed`, which moves closed entries verbatim to a +sibling archive file and leaves id-preserving stubs. Pre-#2651 dev primitives and the attended `bmad-build` append flatter entries here directly, which the orchestrator normalizes on sweep; the current unattended primitive records its findings in the spec's frontmatter instead and the engine harvests them into @@ -675,6 +677,26 @@ def _one_line(value: str) -> str: return LINE_BREAK_RE.sub(" ", value).strip() +def _iso_date_or_none(value: str) -> str | None: + """`value` when it is a strict ISO ``YYYY-MM-DD`` calendar date, else None. + + The shared shape of the ledger's two date checks, so a skip-not-raise caller + (:func:`_close_date`) and a raise caller (:func:`_require_iso_date`) cannot + drift apart on what counts as a close date. The regex is not redundant with + ``date.fromisoformat``: since 3.11 that also accepts ``20260611`` and ISO + week dates, neither of which the ledger's own readers recognize, and it is + the regex — via ``[0-9]`` — that pins the digits to ASCII. ``fromisoformat`` + in turn rejects the well-shaped impossible day (``2026-02-30``) that no + pattern can catch.""" + if not _ISO_DATE_RE.fullmatch(value): + return None + try: + calendar_date.fromisoformat(value) + except ValueError: + return None + return value + + def _require_iso_date(value: str) -> None: """Raise unless `value` is a strict ISO `YYYY-MM-DD` calendar date. @@ -682,19 +704,9 @@ def _require_iso_date(value: str) -> None: (`Engine._today()`), never model-authored, so a bad value is a programmer bug. Letting it through writes a `status:` line that reads as neither open nor done, which `classify` reports as malformed and `open_ids` drops — the - entry silently leaves the sweep's world. - - The regex is not redundant with `date.fromisoformat`: since 3.11 that also - accepts `20260611` and ISO week dates, neither of which the ledger's own - readers recognize, and it is the regex — via `[0-9]` — that pins the digits - to ASCII. `fromisoformat` in turn rejects the well-shaped impossible day - (`2026-02-30`) that no pattern can catch.""" - if not _ISO_DATE_RE.fullmatch(value): + entry silently leaves the sweep's world.""" + if _iso_date_or_none(value) is None: raise ValueError(f"date must be YYYY-MM-DD: {value!r}") - try: - calendar_date.fromisoformat(value) - except ValueError as exc: - raise ValueError(f"date must be YYYY-MM-DD: {value!r}") from exc def _require_canonical_status(status: str) -> None: @@ -1054,7 +1066,8 @@ def append_entry( def _is_archived(entry: DWEntry) -> bool: """Whether the entry carries a live ``archived:`` field line (not a quoted - example), marking it as a stub from a prior :func:`archive_closed` run. + example), marking it as touched by :func:`archive_closed` — a stub in the + live ledger, or an archived body in the archive file. Reads through :func:`_quoted` for the same reason every gate scan does: an entry documenting the archive field in a fenced example carries the @@ -1064,6 +1077,57 @@ def _is_archived(entry: DWEntry) -> bool: return any(not _quoted(entry, m.start()) for m in _ARCHIVED_FIELD_RE.finditer(entry.body)) +# Field lines a stub must carry when the archived body had them, because +# downstream readers key on them regardless of status: `gate:` (validate's +# closed-entry gate report deliberately keeps speaking), `origin:` + +# `source_spec:` (the engine's status-agnostic harvest-replay dedupe), and the +# reopenable-close undo tail (`mark_open`'s adjacency requirement). +_PRESERVED_FIELD_RE = re.compile(r"^(gate:.*|origin:.*|source_spec:.*)$", re.MULTILINE) + +# The exact stub shape :func:`archive_closed` leaves in the live ledger. +# A done entry that merely carries a hand-written `archived:` line does NOT +# match — it is a real entry, not a stub, and must still be archived. +_STUB_BODY_RE = re.compile( + r"### .*: .*\n\n" + r"status: done [0-9]{4}-[0-9]{2}-[0-9]{2}\n" + r"(?:resolution: [^\n]*\nresolution-undo: [0-9a-f]{64} [^\n]*\n)?" + r"(?:(?:gate:|origin:|source_spec:)[^\n]*\n)*" + r"archived: [^\n]*\n" + r"\n?" +) + + +def _is_stub(entry: DWEntry) -> bool: + """Whether the entry is a stub left by a prior :func:`archive_closed` run. + + Shape-based rather than `archived:`-line-based: a done entry a human + annotated with a stray unfenced ``archived:`` line is a real entry whose + body still belongs in the live ledger — skipping it forever on the strength + of one line would silently exclude it from every future archive. + """ + return entry.done and _STUB_BODY_RE.fullmatch(entry.body.rstrip("\n") + "\n") is not None + + +def _preserved_stub_lines(entry: DWEntry) -> list[str]: + """The load-bearing field lines a stub must keep from the archived body. + + Scanned fence-aware like every field read in this module: a fenced example + documenting `origin:` is not a declaration. The undo tail is read with the + same adjacency regex :func:`mark_open` will later use against the stub, so + what qualifies here is exactly what remains undoable there. + """ + lines = [ + entry.body[m.start() : m.end()] + for m in _PRESERVED_FIELD_RE.finditer(entry.body) + if not _quoted(entry, m.start()) + ] + if entry.status_span is not None: + tail = _MARK_DONE_TAIL_RE.match(entry.body, entry.status_span[1]) + if tail is not None: + lines = [tail.group(0).lstrip("\n")] + lines + return lines + + def _close_date(entry: DWEntry) -> str | None: """The ISO close date from a ``done <date>`` status, or None when the entry is not done, is done without a date suffix, or carries a date @@ -1079,16 +1143,9 @@ def _close_date(entry: DWEntry) -> str | None: parts = entry.status.split() if len(parts) != 2: # exactly `done YYYY-MM-DD` — extra tokens are not a close date return None - date_str = parts[1] - if not _ISO_DATE_RE.fullmatch(date_str): - return None - try: - # The regex alone admits well-shaped impossible days (2026-02-30) that - # no calendar carries — the same half of `_require_iso_date`. - calendar_date.fromisoformat(date_str) - except ValueError: - return None - return date_str + # Same shape check as `_require_iso_date` (well-formed regex AND a real + # calendar day), skip-not-raise: a hand-edited close is data, not a bug. + return _iso_date_or_none(parts[1]) def archive_closed( @@ -1110,8 +1167,12 @@ def archive_closed( with an ``archived: <date>`` field line appended after the entry's status line. The stub left in the live ledger keeps the heading, a ``status: done <date>`` line (so :func:`parse_ledger` reads it as done and - :func:`open_ids` drops it), and an ``archived: <date>`` line (so a - subsequent run skips it rather than re-archiving the stub). + :func:`open_ids` drops it), an ``archived: <date>`` line (so a subsequent + run skips it rather than re-archiving the stub), and the entry's + load-bearing field lines — ``gate:``, ``origin:``/``source_spec:``, and + the reopenable-close undo tail — because downstream readers key on those + regardless of status (validate's closed-gate report, the engine's + harvest-replay dedupe, and sweep bundle rollback respectively). ``before`` (ISO ``YYYY-MM-DD``) archives only entries closed strictly *before* that date. Entries with ``status: done`` (no date) are always @@ -1125,7 +1186,7 @@ def archive_closed( :func:`atomic_write_text`, the same primitive every ledger writer uses. The archive file accumulates on repeat runs: new entries are appended to the existing file, never overwritten, and stubs from a prior run are - skipped by their ``archived:`` line. + skipped by their exact stub shape. """ if before is not None: _require_iso_date(before) @@ -1141,7 +1202,7 @@ def archive_closed( continue # not done, or done without a date if before is not None and close_date >= before: continue # closed on or after the cutoff - if _is_archived(entry): + if _is_stub(entry): continue # stub from a prior archive_closed run to_archive.append((entry, close_date)) if not to_archive: @@ -1165,11 +1226,11 @@ def archive_closed( # field are therefore skipped here and only replaced with stubs below. archive_blocks: list[str] = [] already_archived = { - e.id for e in parse_ledger(existing) if _is_archived(e) + e.id: _close_date(e) for e in parse_ledger(existing) if _is_archived(e) } # fence-aware: a quoted example in the archive is not a real body - for entry, _ in to_archive: - if entry.id in already_archived: - continue # body already archived by a crashed prior run + for entry, close_date in to_archive: + if already_archived.get(entry.id) == close_date: + continue # this closure's body is already archived (crashed prior run) body = entry.body assert entry.status_span is not None # done with a date implies a status line pos = entry.status_span[1] @@ -1189,7 +1250,13 @@ def archive_closed( # earlier spans are unaffected by later replacements — the same # text-surgery pattern as `_apply_done`, applied to multiple entries. for entry, close_date in reversed(to_archive): - stub = f"### {entry.id}: {entry.title}\n\nstatus: done {close_date}\narchived: {stamp}\n\n" + preserved = "".join(f"{line}\n" for line in _preserved_stub_lines(entry)) + stub = ( + f"### {entry.id}: {entry.title}\n\n" + f"status: done {close_date}\n" + f"{preserved}" + f"archived: {stamp}\n\n" + ) start, end = entry.span text = text[:start] + stub + text[end:] # Write the archive BEFORE the ledger: a crash between writes leaves the diff --git a/tests/test_cli.py b/tests/test_cli.py index 40862bea..9766f8fb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -8602,9 +8602,9 @@ def run(self): # the gate saw the honest bytes and passed factory("epic-boundary", started=lambda: signalled.append("started")) - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" assert signalled == ["started"] # #501: the child composed, so the parent may latch run_id = captured["state"].run_id @@ -8681,9 +8681,9 @@ def run(self): assert cli.main(["run", "--project", str(project.project)]) == 0 - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" run_id = captured["state"].run_id assert runs.read_trusted_config_digest(project.project, run_id) == pin @@ -8741,9 +8741,9 @@ def __init__(self, **kwargs): assert cli._resume_paused_run(project.project, run_dir) == 0 - assert ( - profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" - ), "the swap must actually have landed on disk, or this test proves nothing" + assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( + "the swap must actually have landed on disk, or this test proves nothing" + ) assert captured["adapter"].profile.binary == "mycli" assert runs.read_trusted_config_digest(project.project, run_dir.name) == pin @@ -9157,5 +9157,90 @@ def test_sweep_archive_dry_run(project, capsys): rc = cli.main(["sweep", "--archive", "--dry-run", "--project", str(project.project)]) assert rc == 0 out = capsys.readouterr().out - assert "would archive 1 entries" in out + assert "would archive 1 entry" in out assert "DW-2" in out + + +def test_sweep_archive_writes_ledger_and_archive(project, capsys): + """The writing path is observed on disk: bodies move, stubs land, open + entries stay (#706 pass 2 — a plumbing regression must not ship green).""" + from bmad_loop import deferredwork + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 0 + out = capsys.readouterr().out + assert "archived 1 entry" in out + assert "commit them to make the move durable" in out + ledger_path = project.deferred_work + archive_path = ledger_path.parent / "deferred-work-archive.md" + text = ledger_path.read_text(encoding="utf-8") + entries = {e.id: e for e in deferredwork.parse_ledger(text)} + assert entries["DW-2"].done # stub parses as done + assert entries["DW-1"].open # open entry untouched + assert "DW-2" in text # grep-resolvable + assert archive_path.is_file() + assert "### DW-2:" in archive_path.read_text(encoding="utf-8") + + +def test_sweep_archive_before_without_archive_refused(project, capsys): + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open"}, commit=False) + rc = cli.main(["sweep", "--before", "2026-06-01", "--project", str(project.project)]) + assert rc == 1 + assert "--before requires --archive" in capsys.readouterr().err + + +def test_sweep_archive_conflicting_flags_refused(project, capsys): + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open"}, commit=False) + rc = cli.main(["sweep", "--archive", "--no-prompt", "--project", str(project.project)]) + assert rc == 1 + err = capsys.readouterr().err + assert "--archive cannot combine with" in err + assert "--no-prompt" in err + + +def test_sweep_archive_empty_before_without_archive_refused(project, capsys): + """`--before ""` is a provided value, not an absent one — the guard must + not test truthiness (#706 pass 2).""" + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open"}, commit=False) + rc = cli.main(["sweep", "--before", "", "--project", str(project.project)]) + assert rc == 1 + assert "--before requires --archive" in capsys.readouterr().err + + +def test_sweep_archive_missing_ledger_named(project, capsys): + """A missing ledger is named, not reported as a clean nothing-to-archive.""" + install_bmad_config(project) + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 0 + out = capsys.readouterr().out + assert "no deferred-work ledger at" in out + assert "no closed entries" not in out + + +def test_sweep_archive_refuses_while_run_live(project, monkeypatch, capsys): + """The out-of-band ledger writer refuses when any engine run is live — + a concurrent close between its read and writes would be lost (#706 pass 2).""" + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" + run_dir.mkdir(parents=True) + (run_dir / "state.json").write_text("{}", encoding="utf-8") + monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "alive") + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 1 + assert "is live" in capsys.readouterr().err + assert "DW-2" in project.deferred_work.read_text(encoding="utf-8") # nothing written diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 8457092b..1d67833b 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -16,6 +16,7 @@ classify, field_line_present, field_severity, + gates, has_legacy, mark_done, mark_done_many, @@ -2374,7 +2375,10 @@ def test_archive_all_done(tmp_path): # The stub still parses as done, with the original close date assert entries["DW-2"].done assert entries["DW-2"].status == "done 2026-05-25" - assert "origin: code review" not in entries["DW-2"].body # body was moved + assert "reason: pre-existing." not in entries["DW-2"].body # body was moved + assert "location: src/foo.py:10" not in entries["DW-2"].body + # Load-bearing field lines survive in the stub (engine replay dedupe) + assert "origin: code review of spec-1-1.md" in entries["DW-2"].body assert entries["DW-2"].title == "Old closed item" # heading preserved assert "archived: 2026-08-24" in entries["DW-2"].body # stub marker # Open entries untouched @@ -2407,7 +2411,8 @@ def test_archive_before_cutoff(tmp_path): entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} assert entries["DW-1"].done # stub - assert "origin: a" not in entries["DW-1"].body + assert "archived: 2026-08-24" in entries["DW-1"].body + assert "origin: a" in entries["DW-1"].body # preserved field line # July entry untouched — not before the cutoff assert entries["DW-2"].status == "done 2026-07-01" assert "origin: b" in entries["DW-2"].body @@ -2663,7 +2668,7 @@ def test_archive_crash_recovery_no_duplicate_bodies(tmp_path): # The ledger now holds the stub... entries = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))} assert entries["DW-2"].done - assert "origin: code review" not in entries["DW-2"].body + assert "reason: pre-existing." not in entries["DW-2"].body # body was moved, stub left # ...and the archive carries the body exactly once, with the FIRST run's # stamp preserved — no duplicate append, no re-stamp. @@ -2698,3 +2703,180 @@ def test_archive_crash_recovery_fenced_example_does_not_suppress(tmp_path): assert archive_text.count("### DW-2:") == 2 # quoted + real assert "origin: code review of spec-1-1.md" in archive_text assert "quoted example" in archive_text + + +# ------------------------------------------- archive follow-up review (#706, pass 2) + + +def test_archive_stub_preserves_gate_origin_source_spec(tmp_path): + """The stub keeps gate:/origin:/source_spec: lines — validate's closed-gate + report and the engine's status-agnostic replay dedupe both key on them + regardless of status.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: gated close\n\n" + "origin: spec-harvest fingerprint-abc\n" + "source_spec: specs/spec-1-1.md\n" + "gate: 1-2\n" + "location: src/foo.py\n" + "status: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert "gate: 1-2" in stub.body + assert "origin: spec-harvest fingerprint-abc" in stub.body + assert "source_spec: specs/spec-1-1.md" in stub.body + assert "location: src/foo.py" not in stub.body # the rest moved + assert gates(stub).tokens == ("1-2",) # still speaks for validate + + +def test_archive_stub_preserves_reopenable_undo_tail(tmp_path): + """A reopenable close's resolution/resolution-undo tail survives into the + stub, so a later sweep-bundle rollback can still undo the close.""" + text = "# Deferred Work\n\n### DW-1: bundle close\n\norigin: a\nlocation: b\nstatus: open\n" + path = write_ledger(tmp_path, text) + mark_done_many_reopenable(path, ["DW-1"], "2026-05-25", "sweep bundle", "op-1") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + # The stub still reads done... + stub_ledger = path.read_text(encoding="utf-8") + assert "DW-1" not in open_ids(stub_ledger) + # ...and mark_open can still undo it (the tail is intact and adjacent) + assert mark_open(path, "DW-1", "sweep bundle", "op-1") is True + assert "DW-1" in open_ids(path.read_text(encoding="utf-8")) + + +def test_archive_hand_written_archived_line_still_archives(tmp_path): + """A done entry carrying a stray unfenced `archived:` line but a real body + is NOT mistaken for a stub — shape, not one line, decides (#706 pass 2).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: real entry, stray field\n\n" + "origin: a\nlocation: src/x.py\nreason: still real work\n" + "status: done 2026-05-25\narchived: 2026-01-01\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + assert "reason: still real work" in (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + + +def test_archive_is_archived_fence_filter_pinned(tmp_path): + """A done entry documenting `archived:` only inside a fenced example is + not treated as already-archived — the `_quoted` filter is load-bearing.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: documents the field\n\n" + "origin: a\nlocation: b\n" + "```\n" + "archived: 2026-01-01\n" + "```\n" + "status: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert "archived: 2026-08-24" in stub.body # real stamp, not the quoted one + + +def test_archive_legacy_entries_untouched(tmp_path): + """Legacy (flat/pre-DW-format) content is never modified by archiving.""" + text = ( + "# Deferred Work\n\n" + "### DW-1: done canonical\n\norigin: a\nstatus: done 2026-05-25\n\n" + "- source_spec: specs/spec-2-1.md — legacy flat finding, RESOLVED 2026-04-01\n\n" + "## Deferred from: review of spec-2-1.md\n\n" + "Some legacy freeform prose that predates the DW format.\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + after = path.read_text(encoding="utf-8") + assert "- source_spec: specs/spec-2-1.md — legacy flat finding, RESOLVED 2026-04-01" in after + assert "## Deferred from: review of spec-2-1.md" in after + assert "legacy freeform prose" in after + + +def test_archive_stub_is_grep_resolvable_and_classifies_clean(tmp_path): + """The stub keeps the heading so `grep DW-2` finds it, and a post-archive + ledger classifies with no malformed entries.""" + path = write_ledger(tmp_path) + archive_closed(path, archive_date="2026-08-24") + text = path.read_text(encoding="utf-8") + assert "DW-2" in text # grep-resolvable + declared = classify(text, ids=["DW-2"]) + assert declared.malformed == () # stub reads done, not malformed + assert "DW-2" in declared.already_done + + +def test_archive_reclose_after_archive_appends_new_body(tmp_path): + """An entry reopened and re-closed after its first body was archived gets + its second body appended — id equivalence alone must not suppress it.""" + text = "# Deferred Work\n\n### DW-1: closes twice\n\norigin: a\nstatus: done 2026-05-25\n" + path = write_ledger(tmp_path, text) + archive_closed(path, archive_date="2026-06-01") + # Reopen (undo the stub) and re-close with a different date and body + reopened = text.replace("status: done 2026-05-25", "status: open\nreason: reopened") + reopened = reopened.replace("archived: 2026-06-01\n", "") + path.write_text(reopened, encoding="utf-8") + mark_done(path, "DW-1", "2026-07-01", "second close") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + assert archive_text.count("### DW-1:") == 2 # both closures preserved + assert "status: done 2026-07-01" in archive_text + + +def test_archive_crash_recovery_edited_entry_keeps_both_bodies(tmp_path): + """Crash-recovery skip keys on id AND close date: an entry edited between + the crash and the retry is re-archived, not silently dropped.""" + path = write_ledger(tmp_path) + # First (crashed) run archived the original body + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text( + "### DW-2: Old closed item\n\n" + "origin: code review of spec-1-1.md, 2026-05-20\n" + "location: src/foo.py:10\n" + "reason: pre-existing.\n" + "status: done 2026-05-25\n" + "archived: 2026-08-24\n", + encoding="utf-8", + ) + # The ledger entry was then re-closed later (different close date) + edited = LEDGER.replace("status: done 2026-05-25", "status: done 2026-06-10") + path.write_text(edited, encoding="utf-8") + assert archive_closed(path, archive_date="2026-08-25") == ["DW-2"] + archive_text = archive_path.read_text(encoding="utf-8") + assert archive_text.count("### DW-2:") == 2 + assert "status: done 2026-06-10" in archive_text + + +def test_archive_default_date_flake_fixed(tmp_path): + """`today` sampled BEFORE the call, so a midnight rollover mid-call cannot + fail the assertion (docs/testing.md flake policy).""" + from datetime import date as calendar_date + + path = write_ledger(tmp_path) + today = calendar_date.today().isoformat() + archive_closed(path) + assert f"archived: {today}" in (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + + +def test_archive_fenced_heading_in_archive_does_not_suppress_reclose(tmp_path): + """A `### DW-2:` mention inside an archived body's fenced example must not + read as that id's archived twin — the false-positive direction of the + crash-recovery membership check.""" + path = write_ledger(tmp_path) + # An archive holding a fenced worked example that quotes DW-2's heading + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text( + "### DW-9: documents the format\n\n" + "```\n" + "### DW-2: quoted example\n" + "status: done 2026-05-25\n" + "archived: 2026-01-01\n" + "```\n" + "status: done 2026-01-02\n" + "archived: 2026-01-03\n", + encoding="utf-8", + ) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + archive_text = archive_path.read_text(encoding="utf-8") + assert "origin: code review of spec-1-1.md" in archive_text # real body landed From 92817ec8e2684593e9aee3c2e63037e4a3be4fad Mon Sep 17 00:00:00 2001 From: Jack McIntyre <jack@jackmcintyre.net> Date: Mon, 24 Aug 2026 22:27:17 +1000 Subject: [PATCH 05/16] fix(sweep): archive refuses unverifiable run liveness; deterministic date test; docs wording (#711 review) --- docs/FEATURES.md | 2 +- src/bmad_loop/cli.py | 12 +++++----- .../bmad-loop-sweep/deferred-work-format.md | 3 ++- tests/test_cli.py | 19 +++++++++++++++- tests/test_deferredwork.py | 22 +++++++++++++------ 5 files changed, 43 insertions(+), 15 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index b2f31060..1dc5430a 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,7 +133,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. -- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries verbatim to sibling `deferred-work-archive.md`, leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). +- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume <id>`). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve <id> --restore-patch <path>` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 302cd777..931f6953 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2269,13 +2269,15 @@ def cmd_sweep(args: argparse.Namespace) -> int: def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse.Namespace) -> int: """`bmad-loop sweep --archive`: move closed deferred-work entries to a sibling archive file. A self-contained sub-mode — no worktree, no - preflight, no LLM. Refuses while any engine run is live: this is the one - out-of-band ledger writer, and a concurrent close or harvest landing - between its read and its writes would be silently clobbered.""" + preflight, no LLM. Refuses while any engine run is live or unverifiably + so: this is the one out-of-band ledger writer, and a concurrent close or + harvest landing between its read and its writes would be silently + clobbered. An unverifiable pid is treated as live — a write op takes the + conservative side, unlike the cleanup guards which only warn.""" for run_dir in runs.list_run_dirs(project): - if runs.engine_liveness(run_dir) == "alive": + if runs.engine_liveness(run_dir) != "dead": print( - f"run {run_dir.name} is live — stop it before archiving ledger entries", + f"run {run_dir.name} may still be live — stop it before archiving ledger entries", file=sys.stderr, ) return ExitCode.FAILURE diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index 7e2dbcee..dcacd11f 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -22,7 +22,8 @@ not split a multi-goal spec, it records a `multiple-goals` warning in the spec's The file is append-only — never rewrite or delete existing entries. The one sanctioned rewrite is operator-run archiving (below): closed entries move to -`deferred-work-archive.md` verbatim, leaving a stub behind. +`deferred-work-archive.md` — body preserved, with an `archived: <date>` marker +appended after the status line — leaving a stub behind. ## Archiving (`bmad-loop sweep --archive`) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9766f8fb..63ab6db2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9242,5 +9242,22 @@ def test_sweep_archive_refuses_while_run_live(project, monkeypatch, capsys): monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "alive") rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) assert rc == 1 - assert "is live" in capsys.readouterr().err + assert "may still be live" in capsys.readouterr().err assert "DW-2" in project.deferred_work.read_text(encoding="utf-8") # nothing written + + +def test_sweep_archive_refuses_on_unknown_liveness(project, monkeypatch, capsys): + """An unverifiable pid is treated as live for this write op — the archive + must not run over a run it cannot prove is dead (#711 review).""" + from conftest import write_ledger + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" + run_dir.mkdir(parents=True) + (run_dir / "state.json").write_text("{}", encoding="utf-8") + monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "unknown") + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 1 + assert "may still be live" in capsys.readouterr().err + assert "DW-2" in project.deferred_work.read_text(encoding="utf-8") # ledger unchanged diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 1d67833b..4890b1c8 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2848,15 +2848,23 @@ def test_archive_crash_recovery_edited_entry_keeps_both_bodies(tmp_path): assert "status: done 2026-06-10" in archive_text -def test_archive_default_date_flake_fixed(tmp_path): - """`today` sampled BEFORE the call, so a midnight rollover mid-call cannot - fail the assertion (docs/testing.md flake policy).""" - from datetime import date as calendar_date - +def test_archive_default_date_flake_fixed(tmp_path, monkeypatch): + """`calendar_date` is patched to a fixed clock (both `today()` and + `fromisoformat()`), so a midnight rollover mid-call cannot fail the + assertion (docs/testing.md flake policy).""" + from datetime import date as real_date + + class FixedDate(real_date): + @classmethod + def today(cls): + return cls(2026, 8, 24) + + monkeypatch.setattr(deferredwork, "calendar_date", FixedDate) path = write_ledger(tmp_path) - today = calendar_date.today().isoformat() archive_closed(path) - assert f"archived: {today}" in (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + assert f"archived: {FixedDate.today().isoformat()}" in (path.parent / ARCHIVE_REL).read_text( + encoding="utf-8" + ) def test_archive_fenced_heading_in_archive_does_not_suppress_reclose(tmp_path): From e207623608861e43ee3d11cff5894bfd702a8634 Mon Sep 17 00:00:00 2001 From: Jack McIntyre <jack@jackmcintyre.net> Date: Mon, 24 Aug 2026 22:35:16 +1000 Subject: [PATCH 06/16] style: repad README sweep table row (prettier) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 48752450..23042ae8 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ bmad-loop tui # …or drive everything from the dashboard | `bmad-loop mux` | List registered terminal-multiplexer backends — platform match, availability, version, and which one is selected (and why). `mux set <name>` persists a machine-scoped choice into `.bmad-loop/policy.toml` (`--clear` reverts to auto-select, `--force` allows a name that only registers on the target machine); the `BMAD_LOOP_MUX_BACKEND` env var outranks it. | | `bmad-loop adapters` | List registered coding-CLI adapter **kinds** — name, builtin/external, whether the family drives a multiplexer, and which profiles select each — the CLI axis's counterpart to `mux`. Unlike `mux` there is no global choice to persist: a kind is selected per profile by its `adapter` field. A profile naming an unregistered kind, and any out-of-tree adapter/profile package that failed to load, get a `warning:` on stderr. | | `bmad-loop run` | Drive the dev → review → verify → commit loop. `--epic N`, `--story KEY`, `--max-stories N`, `--dry-run`. `--spec <folder>` forces **stories mode** (folder+id dispatch off `<folder>/stories.yaml`), overriding `[stories].source`; `--story` then filters by story id. | -| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. `--archive [--before DATE]` instead moves closed ledger entries to `deferred-work-archive.md`, leaving id-preserving stubs. | +| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. `--archive [--before DATE]` instead moves closed ledger entries to `deferred-work-archive.md`, leaving id-preserving stubs. | | `bmad-loop resume <run-id>` | Continue a run paused at a gate, escalation, or interruption. | | `bmad-loop resolve <run-id>` | Resolve a CRITICAL escalation: open an interactive resolve agent to fix the frozen spec, then re-arm the story and resume. On an _intent gap_ the re-drive can resume review on the attempted change instead of re-implementing it. `--story KEY`, `--no-interactive`, `--restore-patch <path>` (intent-gap patch-restore), `--resume` / `--no-resume`, `--force` (proceed when engine liveness is unverifiable; a provably-live engine still blocks). | | `bmad-loop decisions` | Answer deferred-work decisions earlier sweeps left unanswered (skipped by `--no-prompt`, or an abandoned interactive sweep). Recorded so the next sweep acts on them without re-asking. `--list` shows them without answering; `--json` emits them as a stable machine-readable document — id, question, context, recommendation, and every option's key/label/effect/intent/resolution/bundle-name with a derived `recommended` flag. It implies the listing and never prompts, so a script can select an option by policy instead of scraping the text. | From 531ac2680432aedf9412f0846e49e89b6b9e222e Mon Sep 17 00:00:00 2001 From: Jack McIntyre <jack@jackmcintyre.net> Date: Mon, 24 Aug 2026 22:39:30 +1000 Subject: [PATCH 07/16] wip: bisect lint --- CHANGELOG.md | 12 +++---- README.md | 2 +- docs/FEATURES.md | 1 - .../bmad-loop-sweep/deferred-work-format.md | 32 +------------------ 4 files changed, 7 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18dd01d9..72d87f9b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,13 +9,11 @@ breaking changes may land in a minor release. ### Added -- **`bmad-loop sweep --archive`** moves closed (`status: done <ISO date>`) deferred-work entries to - a sibling `deferred-work-archive.md`, replacing each with a stub that preserves the DW- id for - grep and `closes_deferred` cross-references plus the load-bearing field lines (`gate:`, - `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger stays proportional to - open work rather than to all history. Supports `--before DATE` to archive only entries closed - before a cutoff, and `--dry-run` to preview. Refuses while any engine run is live. Pure - deterministic Python — no LLM involvement. +- **`bmad-loop sweep --archive`** moves closed (`status: done`) deferred-work entries to a sibling + `deferred-work-archive.md`, replacing each with a minimal stub that preserves the DW- id for + grep and `closes_deferred` cross-references. The live ledger stays proportional to open work + rather than to all history. Supports `--before DATE` to archive only entries closed before a + cutoff, and `--dry-run` to preview. Pure deterministic Python — no LLM involvement. ## [0.11.1] — 2026-08-23 diff --git a/README.md b/README.md index 23042ae8..436d94b5 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ bmad-loop tui # …or drive everything from the dashboard | `bmad-loop mux` | List registered terminal-multiplexer backends — platform match, availability, version, and which one is selected (and why). `mux set <name>` persists a machine-scoped choice into `.bmad-loop/policy.toml` (`--clear` reverts to auto-select, `--force` allows a name that only registers on the target machine); the `BMAD_LOOP_MUX_BACKEND` env var outranks it. | | `bmad-loop adapters` | List registered coding-CLI adapter **kinds** — name, builtin/external, whether the family drives a multiplexer, and which profiles select each — the CLI axis's counterpart to `mux`. Unlike `mux` there is no global choice to persist: a kind is selected per profile by its `adapter` field. A profile naming an unregistered kind, and any out-of-tree adapter/profile package that failed to load, get a `warning:` on stderr. | | `bmad-loop run` | Drive the dev → review → verify → commit loop. `--epic N`, `--story KEY`, `--max-stories N`, `--dry-run`. `--spec <folder>` forces **stories mode** (folder+id dispatch off `<folder>/stories.yaml`), overriding `[stories].source`; `--story` then filters by story id. | -| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. `--archive [--before DATE]` instead moves closed ledger entries to `deferred-work-archive.md`, leaving id-preserving stubs. | +| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. | | `bmad-loop resume <run-id>` | Continue a run paused at a gate, escalation, or interruption. | | `bmad-loop resolve <run-id>` | Resolve a CRITICAL escalation: open an interactive resolve agent to fix the frozen spec, then re-arm the story and resume. On an _intent gap_ the re-drive can resume review on the attempted change instead of re-implementing it. `--story KEY`, `--no-interactive`, `--restore-patch <path>` (intent-gap patch-restore), `--resume` / `--no-resume`, `--force` (proceed when engine liveness is unverifiable; a provably-live engine still blocks). | | `bmad-loop decisions` | Answer deferred-work decisions earlier sweeps left unanswered (skipped by `--no-prompt`, or an abandoned interactive sweep). Recorded so the next sweep acts on them without re-asking. `--list` shows them without answering; `--json` emits them as a stable machine-readable document — id, question, context, recommendation, and every option's key/label/effect/intent/resolution/bundle-name with a derived `recommended` flag. It implies the listing and never prompts, so a script can select an option by policy instead of scraping the text. | diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 1dc5430a..daf9d6ec 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,7 +133,6 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. -- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume <id>`). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve <id> --restore-patch <path>` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index dcacd11f..03cb9b56 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -20,37 +20,7 @@ splits are a legacy/attended source only — the current unattended primitive do not split a multi-goal spec, it records a `multiple-goals` warning in the spec's `warnings:` and proceeds. -The file is append-only — never rewrite or delete existing entries. The one -sanctioned rewrite is operator-run archiving (below): closed entries move to -`deferred-work-archive.md` — body preserved, with an `archived: <date>` marker -appended after the status line — leaving a stub behind. - -## Archiving (`bmad-loop sweep --archive`) - -The operator may periodically move closed entries -(`status: done <ISO date>`) to the sibling `deferred-work-archive.md`, leaving -a stub in this file: - -```markdown -### DW-7: Old closed item - -status: done 2026-05-25 -archived: 2026-08-24 -``` - -Rules for sessions reading the ledger: - -- The stub's `status: done` line means what it always meant — the entry is - closed and not open work. -- An `archived:` line marks content that lives in `deferred-work-archive.md`. - The full body (evidence, resolution, dates) is there, keyed by the same - DW- id — read the archive file for anything beyond the stub. -- Stubs keep load-bearing field lines (`gate:`, `origin:`/`source_spec:`, - resolution undo markers) — treat them exactly as if the entry were still - whole. Never edit or drop them when touching a stub. -- When deduping against existing entries, a stub still counts: match on the - id and preserved `origin:`/`source_spec:` lines, and check the archive for - the full substance before appending. +The file is append-only — never rewrite or delete existing entries. ## Before appending: dedupe check From 29ab3015df34cfb2e9b5be467a06fabda9d16a7d Mon Sep 17 00:00:00 2001 From: Jack McIntyre <jack@jackmcintyre.net> Date: Mon, 24 Aug 2026 22:44:30 +1000 Subject: [PATCH 08/16] fix: restore black-style asserts trunk enforces; reinstate doc updates --- CHANGELOG.md | 12 ++++--- README.md | 2 +- docs/FEATURES.md | 1 + .../bmad-loop-sweep/deferred-work-format.md | 32 ++++++++++++++++++- tests/test_cli.py | 21 ++++++------ 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72d87f9b..18dd01d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,11 +9,13 @@ breaking changes may land in a minor release. ### Added -- **`bmad-loop sweep --archive`** moves closed (`status: done`) deferred-work entries to a sibling - `deferred-work-archive.md`, replacing each with a minimal stub that preserves the DW- id for - grep and `closes_deferred` cross-references. The live ledger stays proportional to open work - rather than to all history. Supports `--before DATE` to archive only entries closed before a - cutoff, and `--dry-run` to preview. Pure deterministic Python — no LLM involvement. +- **`bmad-loop sweep --archive`** moves closed (`status: done <ISO date>`) deferred-work entries to + a sibling `deferred-work-archive.md`, replacing each with a stub that preserves the DW- id for + grep and `closes_deferred` cross-references plus the load-bearing field lines (`gate:`, + `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger stays proportional to + open work rather than to all history. Supports `--before DATE` to archive only entries closed + before a cutoff, and `--dry-run` to preview. Refuses while any engine run is live. Pure + deterministic Python — no LLM involvement. ## [0.11.1] — 2026-08-23 diff --git a/README.md b/README.md index 436d94b5..23042ae8 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ bmad-loop tui # …or drive everything from the dashboard | `bmad-loop mux` | List registered terminal-multiplexer backends — platform match, availability, version, and which one is selected (and why). `mux set <name>` persists a machine-scoped choice into `.bmad-loop/policy.toml` (`--clear` reverts to auto-select, `--force` allows a name that only registers on the target machine); the `BMAD_LOOP_MUX_BACKEND` env var outranks it. | | `bmad-loop adapters` | List registered coding-CLI adapter **kinds** — name, builtin/external, whether the family drives a multiplexer, and which profiles select each — the CLI axis's counterpart to `mux`. Unlike `mux` there is no global choice to persist: a kind is selected per profile by its `adapter` field. A profile naming an unregistered kind, and any out-of-tree adapter/profile package that failed to load, get a `warning:` on stderr. | | `bmad-loop run` | Drive the dev → review → verify → commit loop. `--epic N`, `--story KEY`, `--max-stories N`, `--dry-run`. `--spec <folder>` forces **stories mode** (folder+id dispatch off `<folder>/stories.yaml`), overriding `[stories].source`; `--story` then filters by story id. | -| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. | +| `bmad-loop sweep` | Triage + execute open `deferred-work.md` entries. `--no-prompt`, `--decisions-only`, `--max-bundles N`, `--repeat`, `--max-cycles N`, `--dry-run`. `--archive [--before DATE]` instead moves closed ledger entries to `deferred-work-archive.md`, leaving id-preserving stubs. | | `bmad-loop resume <run-id>` | Continue a run paused at a gate, escalation, or interruption. | | `bmad-loop resolve <run-id>` | Resolve a CRITICAL escalation: open an interactive resolve agent to fix the frozen spec, then re-arm the story and resume. On an _intent gap_ the re-drive can resume review on the attempted change instead of re-implementing it. `--story KEY`, `--no-interactive`, `--restore-patch <path>` (intent-gap patch-restore), `--resume` / `--no-resume`, `--force` (proceed when engine liveness is unverifiable; a provably-live engine still blocks). | | `bmad-loop decisions` | Answer deferred-work decisions earlier sweeps left unanswered (skipped by `--no-prompt`, or an abandoned interactive sweep). Recorded so the next sweep acts on them without re-asking. `--list` shows them without answering; `--json` emits them as a stable machine-readable document — id, question, context, recommendation, and every option's key/label/effect/intent/resolution/bundle-name with a derived `recommended` flag. It implies the listing and never prompts, so a script can select an option by policy instead of scraping the text. | diff --git a/docs/FEATURES.md b/docs/FEATURES.md index daf9d6ec..1dc5430a 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,6 +133,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. +- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume <id>`). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve <id> --restore-patch <path>` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index 03cb9b56..dcacd11f 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -20,7 +20,37 @@ splits are a legacy/attended source only — the current unattended primitive do not split a multi-goal spec, it records a `multiple-goals` warning in the spec's `warnings:` and proceeds. -The file is append-only — never rewrite or delete existing entries. +The file is append-only — never rewrite or delete existing entries. The one +sanctioned rewrite is operator-run archiving (below): closed entries move to +`deferred-work-archive.md` — body preserved, with an `archived: <date>` marker +appended after the status line — leaving a stub behind. + +## Archiving (`bmad-loop sweep --archive`) + +The operator may periodically move closed entries +(`status: done <ISO date>`) to the sibling `deferred-work-archive.md`, leaving +a stub in this file: + +```markdown +### DW-7: Old closed item + +status: done 2026-05-25 +archived: 2026-08-24 +``` + +Rules for sessions reading the ledger: + +- The stub's `status: done` line means what it always meant — the entry is + closed and not open work. +- An `archived:` line marks content that lives in `deferred-work-archive.md`. + The full body (evidence, resolution, dates) is there, keyed by the same + DW- id — read the archive file for anything beyond the stub. +- Stubs keep load-bearing field lines (`gate:`, `origin:`/`source_spec:`, + resolution undo markers) — treat them exactly as if the entry were still + whole. Never edit or drop them when touching a stub. +- When deduping against existing entries, a stub still counts: match on the + id and preserved `origin:`/`source_spec:` lines, and check the archive for + the full substance before appending. ## Before appending: dedupe check diff --git a/tests/test_cli.py b/tests/test_cli.py index 63ab6db2..8402b185 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -8602,9 +8602,9 @@ def run(self): # the gate saw the honest bytes and passed factory("epic-boundary", started=lambda: signalled.append("started")) - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" assert signalled == ["started"] # #501: the child composed, so the parent may latch run_id = captured["state"].run_id @@ -8681,9 +8681,9 @@ def run(self): assert cli.main(["run", "--project", str(project.project)]) == 0 - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" run_id = captured["state"].run_id assert runs.read_trusted_config_digest(project.project, run_id) == pin @@ -8741,9 +8741,9 @@ def __init__(self, **kwargs): assert cli._resume_paused_run(project.project, run_dir) == 0 - assert profile_mod.get_profile("mycli", project.project).binary == "rogue-cli", ( - "the swap must actually have landed on disk, or this test proves nothing" - ) + assert ( + profile_mod.get_profile("mycli", project.project).binary == "rogue-cli" + ), "the swap must actually have landed on disk, or this test proves nothing" assert captured["adapter"].profile.binary == "mycli" assert runs.read_trusted_config_digest(project.project, run_dir.name) == pin @@ -9164,9 +9164,10 @@ def test_sweep_archive_dry_run(project, capsys): def test_sweep_archive_writes_ledger_and_archive(project, capsys): """The writing path is observed on disk: bodies move, stubs land, open entries stay (#706 pass 2 — a plumbing regression must not ship green).""" - from bmad_loop import deferredwork from conftest import write_ledger + from bmad_loop import deferredwork + install_bmad_config(project) write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) From f1f08b0f1b9eba0c16ccee0118c8d8f4bc539d9a Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 16:04:21 -0700 Subject: [PATCH 09/16] fix(sweep): preserve divergent re-closures and reopen-cycle convergence in archive_closed (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DW id outlives any one closure, so `archive_closed`'s crash-recovery skip could not key on id + close date alone: reopened and re-closed the same day with a new resolution, or annotated with `append_decision` after its body was archived, an entry was stubbed over its own content while the id was reported as archived — the body reached neither file. The skip now compares the twin's body, modulo live `archived:` stamps, and appends on divergence. Two smaller reopen-cycle defects fall out of the same family: `_STUB_BODY_RE` demanded literal spaces in the resolution tail that `_MARK_DONE_TAIL_RE` copies into the stub with tabs intact, so such a stub read as a live entry and was restamped on every run forever; and `mark_open` left the `archived:` line behind, letting the next reopenable close rebuild the exact stub shape and strand the entry outside every future archive. The fence filter that decides which `archived:` lines are real now lives in one helper, `_archived_line_spans`, shared by all three readers of the field. Every new negative and idempotence assertion is ablation-proven. --- src/bmad_loop/deferredwork.py | 90 +++++++++++++++++++++--- tests/test_deferredwork.py | 125 ++++++++++++++++++++++++++++++++++ 2 files changed, 206 insertions(+), 9 deletions(-) diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index 0fe81049..ba65b465 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -908,7 +908,23 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: return False start = entry.span[0] + entry.status_span[0] end = entry.span[0] + res_m.end() - atomic_write_text(path, text[:start] + previous_status_line + text[end:]) + # Drop the entry's live `archived:` stamps along with the close they + # describe. A stub's stamp says "this body lives in the archive file"; once + # the close is undone the body is here and the line is a lie, and leaving it + # is not merely untidy — status + undo tail + stamp is the exact + # `_STUB_BODY_RE` shape, so the next reopenable close reconstitutes a stub + # `archive_closed` skips forever, stranding the entry outside every future + # archive (#711). Cuts are disjoint (an `^archived:` line cannot start + # inside the status line or its adjacent tail) and applied back-to-front so + # earlier offsets stay valid. + cuts = [(start, end, previous_status_line)] + cuts += [ + (entry.span[0] + cut_start, entry.span[0] + cut_end, "") + for cut_start, cut_end in _archived_line_spans(entry) + ] + for cut_start, cut_end, replacement in sorted(cuts, reverse=True): + text = text[:cut_start] + replacement + text[cut_end:] + atomic_write_text(path, text) return True @@ -1064,17 +1080,55 @@ def append_entry( _ARCHIVED_FIELD_RE = re.compile(r"^archived:", re.MULTILINE) +def _archived_line_spans(entry: DWEntry) -> list[tuple[int, int]]: + """Body-relative spans of the entry's live ``archived:`` field lines, each + covering the whole line including its terminating newline. + + Reads through :func:`_quoted` for the same reason every gate scan does: + an entry documenting the archive field in a fenced example carries the + line in column 0, right where the anchor looks, and without the fence + check a quoted ``archived:`` would be mistaken for the real thing. The one + place that rule is written, so the three questions asked about the field — + is this entry archived, what does its body say apart from the stamp, and + which bytes must a reopen drop — cannot answer it differently. + + Whole lines rather than match starts because both cutting callers remove + the line, and a span ending at the anchor would leave the stamp's value + behind as orphaned text. + """ + spans: list[tuple[int, int]] = [] + for m in _ARCHIVED_FIELD_RE.finditer(entry.body): + if _quoted(entry, m.start()): + continue + line_end = entry.body.find("\n", m.end()) + spans.append((m.start(), len(entry.body) if line_end == -1 else line_end + 1)) + return spans + + def _is_archived(entry: DWEntry) -> bool: """Whether the entry carries a live ``archived:`` field line (not a quoted example), marking it as touched by :func:`archive_closed` — a stub in the live ledger, or an archived body in the archive file. + """ + return bool(_archived_line_spans(entry)) - Reads through :func:`_quoted` for the same reason every gate scan does: - an entry documenting the archive field in a fenced example carries the - line in column 0, right where the anchor looks, and without the fence - check a quoted ``archived:`` would be mistaken for the real thing. + +def _body_without_archived(entry: DWEntry) -> str: + """The entry's body with its live ``archived:`` stamps and trailing blank + lines removed — the comparison key for :func:`archive_closed`'s + crash-recovery skip. + + An archived twin is its ledger entry plus exactly one ``archived:`` line, + so the two are the same content only once that line is discounted; trailing + newlines go with it because they record where the entry sat in its file, + not what it says. Everything else is compared verbatim, deliberately: the + cheap wrong answer is archiving a body twice, and the expensive one is + deciding a divergent re-closure was already saved and dropping it (#711). """ - return any(not _quoted(entry, m.start()) for m in _ARCHIVED_FIELD_RE.finditer(entry.body)) + body = entry.body + for start, end in reversed(_archived_line_spans(entry)): + body = body[:start] + body[end:] + return body.rstrip("\n") # Field lines a stub must carry when the archived body had them, because @@ -1090,7 +1144,11 @@ def _is_archived(entry: DWEntry) -> bool: _STUB_BODY_RE = re.compile( r"### .*: .*\n\n" r"status: done [0-9]{4}-[0-9]{2}-[0-9]{2}\n" - r"(?:resolution: [^\n]*\nresolution-undo: [0-9a-f]{64} [^\n]*\n)?" + # Separators mirror `_MARK_DONE_TAIL_RE`, which tolerates tabs: that regex + # decides what `_preserved_stub_lines` copies into the stub verbatim, so a + # stricter shape here reads a stub this module just wrote as a live entry + # and re-archives it on every run, forever, appending nothing (#711). + r"(?:resolution:[ \t]*[^\n]*\nresolution-undo:[ \t]*[0-9a-f]{64}[ \t]+[^\n]*\n)?" r"(?:(?:gate:|origin:|source_spec:)[^\n]*\n)*" r"archived: [^\n]*\n" r"\n?" @@ -1224,12 +1282,26 @@ def archive_closed( # bodies again — an append-only archive accumulating duplicates. Entries # whose parsed archive twin carries a live (non-fenced) ``archived:`` # field are therefore skipped here and only replaced with stubs below. + # + # The twin must match in BODY, not merely in id and close date. A DW id is + # reusable across closures (`mark_open` reopens, a re-close follows) and a + # closed entry still accepts writes (`append_decision` does not read + # status), so id + date names a *closure slot*, not its content: reopened + # and re-closed the same day with a new resolution, or annotated with a + # decision after its body was archived, the ledger entry and its twin + # differ. Skipping on the slot alone stubbed that entry over its own + # content while reporting the id as archived — the body reached neither + # file (#711). A body that differs is appended instead; the archive holds + # several blocks per id by design, and over-archiving is recoverable where + # a silent drop is not. archive_blocks: list[str] = [] already_archived = { - e.id: _close_date(e) for e in parse_ledger(existing) if _is_archived(e) + e.id: (_close_date(e), _body_without_archived(e)) + for e in parse_ledger(existing) + if _is_archived(e) } # fence-aware: a quoted example in the archive is not a real body for entry, close_date in to_archive: - if already_archived.get(entry.id) == close_date: + if already_archived.get(entry.id) == (close_date, _body_without_archived(entry)): continue # this closure's body is already archived (crashed prior run) body = entry.body assert entry.status_span is not None # done with a date implies a status line diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 4890b1c8..5d842678 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2888,3 +2888,128 @@ def test_archive_fenced_heading_in_archive_does_not_suppress_reclose(tmp_path): assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] archive_text = archive_path.read_text(encoding="utf-8") assert "origin: code review of spec-1-1.md" in archive_text # real body landed + + +# --------------------------------------------- archive reopen cycle (#711 review) +# +# A DW id outlives any one closure: `mark_open` reopens, a re-close follows, and +# `append_decision` writes to a closed entry without reading its status. The +# crash-recovery skip therefore cannot key on id + close date alone, the stub +# shape must survive the spacing `_MARK_DONE_TAIL_RE` tolerates, and a reopen +# must drop the `archived:` stamp that no longer describes the entry. + + +def test_archive_same_date_reclose_preserves_new_body(tmp_path): + """Reopened and re-closed on the SAME date with a new resolution, the entry + is archived again rather than stubbed over its own content — id + close date + names a closure slot, not the body that filled it (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: closes twice on one day\n\n" + "origin: a\nlocation: src/x.py:1\nreason: first pass\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + close_reopenable(path, "DW-1", "first close") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + assert mark_open(path, "DW-1", "first close", OPERATION_ID) is True + assert mark_done(path, "DW-1", "2026-06-11", "second close, a different note") is True + + assert archive_closed(path, archive_date="2026-08-25") == ["DW-1"] + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + # The returned id is truthful: the second body really reached the archive. + assert "second close, a different note" in archive_text + assert archive_text.count("### DW-1:") == 2 + # ...and it is not hiding in the ledger either — the stub carries no note. + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert "second close, a different note" not in stub.body + + +def test_archive_decision_on_stub_preserved(tmp_path): + """`append_decision` writes to a closed entry without reading its status, so + a decision can land on a stub; the next archive run must carry it across + instead of overwriting the stub with a fresh one (#711 review).""" + path = write_ledger(tmp_path) # DW-2 is done 2026-05-25 + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + assert append_decision(path, "DW-2", "2026-08-25", "keep", "still relevant") is True + + assert archive_closed(path, archive_date="2026-08-26") == ["DW-2"] + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + assert "decision: 2026-08-25 keep — still relevant" in archive_text + assert archive_text.count("### DW-2:") == 2 + + +def test_mark_open_strips_archived_line(tmp_path): + """Reopening drops the entry's live `archived:` stamp — the body is back in + the ledger, so the line is a lie — while a fenced example of the field is + left alone (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: documents the field it also carries\n\n" + "origin: a\n" + "```\n" + "archived: 2026-01-01\n" + "```\n" + "status: open\n" + ) + path = write_ledger(tmp_path, text) + close_reopenable(path, "DW-1", "bundle close") + # Stamp it the way a stub is stamped: after the close's undo tail. + closed = path.read_text(encoding="utf-8") + path.write_text(closed.rstrip("\n") + "\narchived: 2026-08-24\n", encoding="utf-8") + + assert mark_open(path, "DW-1", "bundle close", OPERATION_ID) is True + entry = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert entry.open + assert "archived: 2026-08-24" not in entry.body # the live stamp is gone + assert "archived: 2026-01-01" in entry.body # the fenced example is not a stamp + assert "origin: a" in entry.body # nothing else was cut + + +def test_archive_reopened_stub_recloses_and_archives(tmp_path): + """Full cycle: archive, reopen, re-close reopenably at a later date. Without + the reopen-side strip the re-close rebuilds the exact stub shape and + `_is_stub` traps the entry outside every future archive (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: full reopen cycle\n\n" + "origin: a\nlocation: src/x.py:1\nreason: first pass\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + close_reopenable(path, "DW-1", "first close") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + assert mark_open(path, "DW-1", "first close", OPERATION_ID) is True + close_reopenable(path, "DW-1", "second close", date="2026-07-01") + + entry = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert not deferredwork._is_stub(entry) # the reopen cycle broke the stub shape + assert archive_closed(path, archive_date="2026-08-25") == ["DW-1"] + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + assert archive_text.count("### DW-1:") == 2 # both closures preserved + assert "status: done 2026-07-01" in archive_text + assert "resolution: second close" in archive_text + + +def test_archive_tab_resolution_stub_converges(tmp_path): + """A tab-separated undo tail is copied into the stub verbatim, so the stub + shape must tolerate the spacing `_MARK_DONE_TAIL_RE` accepts. Otherwise the + stub reads as a live entry and every run re-archives it (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: tab-separated tail\n\n" + "origin: a\n" + "location: src/x.py:1\n" + "reason: prose that does not survive into the stub\n" + "status: done 2026-05-25\n" + "resolution:\ttabbed note\n" + "resolution-undo:\t" + "a" * 64 + "\t2026-05-25\t7374617475733a206f70656e\n" + ) + path = write_ledger(tmp_path, text) + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + # The stub kept the tail verbatim, tabs and all... + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert "resolution:\ttabbed note" in stub.body + # ...and a second run recognizes it as a stub and settles. + assert archive_closed(path, archive_date="2026-08-25") == [] + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + assert archive_text.count("### DW-1:") == 1 + assert "archived: 2026-08-25" not in archive_text From 746d27daa8846b86fe05784a6e8f07fcda09402b Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 16:15:37 -0700 Subject: [PATCH 10/16] fix(sweep): probe liveness on raw run-dir names in --archive; close archive test gaps (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sweep --archive` refused while any run was live, but asked `runs.list_run_dirs`, which is `state.json`-gated. A run whose state file was removed still owns its `engine.pid` and still writes this ledger, so the gated view reported it as no run at all and the archive proceeded — contradicting the guard's own conservative doctrine and the hazard `_run_dir_names` already documents. Liveness is now probed on raw run-dir names via a public counterpart, `runs.all_run_dirs`, which preserves that helper's missing-vs-unreadable distinction: an unreadable runs root learns nothing about liveness, so it refuses too rather than reading as "no runs". Three test gaps this feature shipped with are closed alongside it. The two refusal tests asserted "nothing written" as `"DW-2" in text`, which archiving preserves by design — they now assert an absent archive file and byte equality of the ledger. Nothing covered the allow direction, so a gate that refused unconditionally passed every one of them. And the `_quoted` filter in `_is_archived` had no failing coverage at all: the test named for it exercises `_is_stub`'s path, never the archive-file parse where the filter is read. The new fence-filter test gives the archive twin a body byte-identical to the ledger entry's, so body equivalence — the skip's other half — holds in both directions and the fenced `archived:` line is the only thing deciding. Every new negative assertion is ablation-proven. --- src/bmad_loop/cli.py | 18 ++++++- src/bmad_loop/runs.py | 22 ++++++++ tests/test_cli.py | 105 +++++++++++++++++++++++++++++++++++-- tests/test_deferredwork.py | 38 ++++++++++++++ tests/test_runs.py | 27 ++++++++++ 5 files changed, 205 insertions(+), 5 deletions(-) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 6a088f8d..5f6d4728 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2274,8 +2274,22 @@ def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse so: this is the one out-of-band ledger writer, and a concurrent close or harvest landing between its read and its writes would be silently clobbered. An unverifiable pid is treated as live — a write op takes the - conservative side, unlike the cleanup guards which only warn.""" - for run_dir in runs.list_run_dirs(project): + conservative side, unlike the cleanup guards which only warn. + + Run dirs are enumerated raw (:func:`runs.all_run_dirs`) rather than through + the ``state.json``-gated :func:`runs.list_run_dirs`: a run whose state file + was removed still owns its ``engine.pid`` and still writes this ledger, and + the gated view would report it as no run at all. An unreadable runs root + answers nothing, so it refuses too — same conservative side.""" + run_dirs = runs.all_run_dirs(project) + if run_dirs is None: + print( + f"cannot list runs under {project / runs.RUNS_DIR} — " + "refusing to archive ledger entries", + file=sys.stderr, + ) + return ExitCode.FAILURE + for run_dir in run_dirs: if runs.engine_liveness(run_dir) != "dead": print( f"run {run_dir.name} may still be live — stop it before archiving ledger entries", diff --git a/src/bmad_loop/runs.py b/src/bmad_loop/runs.py index be6819ae..448ab252 100644 --- a/src/bmad_loop/runs.py +++ b/src/bmad_loop/runs.py @@ -143,6 +143,28 @@ def list_run_dirs(project: Path) -> list[Path]: return sorted(d for d in runs.iterdir() if (d / "state.json").is_file()) +def all_run_dirs(project: Path) -> list[Path] | None: + """Every run dir under the runs root — ``state.json`` or not — oldest first, + or ``None`` when the listing could not be taken. + + The ungated counterpart to :func:`list_run_dirs`, and the one to ask when the + question is "does a run still own its control plane" rather than "which runs + can I read". A run whose state.json was removed or corrupted still holds a + live ``engine.pid``, so the gated view walks straight past exactly the run an + operator is mid-recovery on — the hazard :func:`_run_dir_names` documents, + whose set this wraps rather than re-listing. + + ``None`` is an unreadable runs root and means *nothing was learned*, which is + not the same answer as the empty list a missing root gives. Callers that act + on "no live runs" have to tell those apart; see :func:`_run_dir_names`. + """ + names = _run_dir_names(project) + if names is None: + return None + root = project / RUNS_DIR + return sorted(root / name for name in names) + + def latest_run_dir(project: Path) -> Path | None: candidates = list_run_dirs(project) return candidates[-1] if candidates else None diff --git a/tests/test_cli.py b/tests/test_cli.py index 8e5049ad..5916f90c 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9234,19 +9234,28 @@ def test_sweep_archive_missing_ledger_named(project, capsys): def test_sweep_archive_refuses_while_run_live(project, monkeypatch, capsys): """The out-of-band ledger writer refuses when any engine run is live — - a concurrent close between its read and writes would be lost (#706 pass 2).""" + a concurrent close between its read and writes would be lost (#706 pass 2). + + "Nothing written" is asserted as byte equality plus an absent archive, not + as `"DW-2" in text`: archiving DW-2 *keeps* its heading in the ledger by + design, so the id-presence form passes just as well when the refusal never + happened (#711 review).""" from conftest import write_ledger + from bmad_loop import deferredwork + install_bmad_config(project) write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" run_dir.mkdir(parents=True) (run_dir / "state.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "alive") + before = project.deferred_work.read_text(encoding="utf-8") rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) assert rc == 1 assert "may still be live" in capsys.readouterr().err - assert "DW-2" in project.deferred_work.read_text(encoding="utf-8") # nothing written + assert not (project.deferred_work.parent / deferredwork.ARCHIVE_REL).exists() + assert project.deferred_work.read_text(encoding="utf-8") == before def test_sweep_archive_refuses_on_unknown_liveness(project, monkeypatch, capsys): @@ -9254,13 +9263,103 @@ def test_sweep_archive_refuses_on_unknown_liveness(project, monkeypatch, capsys) must not run over a run it cannot prove is dead (#711 review).""" from conftest import write_ledger + from bmad_loop import deferredwork + install_bmad_config(project) write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" run_dir.mkdir(parents=True) (run_dir / "state.json").write_text("{}", encoding="utf-8") monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "unknown") + before = project.deferred_work.read_text(encoding="utf-8") rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) assert rc == 1 assert "may still be live" in capsys.readouterr().err - assert "DW-2" in project.deferred_work.read_text(encoding="utf-8") # ledger unchanged + assert not (project.deferred_work.parent / deferredwork.ARCHIVE_REL).exists() + assert project.deferred_work.read_text(encoding="utf-8") == before # ledger unchanged + + +def test_sweep_archive_refuses_run_dir_without_state_json(project, monkeypatch, capsys): + """A run whose state.json was removed still owns its `engine.pid` — and its + ledger writes. `runs.list_run_dirs` is state.json-gated (it answers "which + runs can I resume"), so a guard built on it walks past exactly the run an + operator is mid-recovery on and archives out from under it (#711 review). + + Ablation: restore `runs.list_run_dirs` in `_sweep_archive` and this fails — + the archive proceeds and returns 0 with a live engine beside it.""" + from conftest import write_ledger + + from bmad_loop import deferredwork + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" + run_dir.mkdir(parents=True) + (run_dir / "engine.pid").write_text("4242", encoding="utf-8") # live, but no state.json + monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "alive") + before = project.deferred_work.read_text(encoding="utf-8") + + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 1 + assert "may still be live" in capsys.readouterr().err + assert not (project.deferred_work.parent / deferredwork.ARCHIVE_REL).exists() + assert project.deferred_work.read_text(encoding="utf-8") == before + + +def test_sweep_archive_proceeds_when_runs_are_dead(project, monkeypatch, capsys): + """The allow direction of the liveness gate: a run dir whose engine is dead + is not a reason to refuse, and the archive runs to completion. + + Without this the three refusal tests are satisfied by a gate that refuses + unconditionally — every one of them would still pass (#711 review).""" + from conftest import write_ledger + + from bmad_loop import deferredwork + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + run_dir = project.project / ".bmad-loop" / "runs" / "20260824-100000-aaaa" + run_dir.mkdir(parents=True) + (run_dir / "state.json").write_text("{}", encoding="utf-8") + monkeypatch.setattr(cli.runs, "engine_liveness", lambda _dir: "dead") + + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 0 + assert "archived 1 entry" in capsys.readouterr().out + archive = project.deferred_work.parent / deferredwork.ARCHIVE_REL + assert archive.is_file() + assert "### DW-2:" in archive.read_text(encoding="utf-8") + + +def test_sweep_archive_refuses_when_runs_dir_unreadable(project, capsys): + """An unreadable runs root answers nothing about liveness, and this write op + takes the conservative side — the same doctrine as an unverifiable pid. + + A plain file where the runs dir belongs is the portable way to make the + listing fail: `os.scandir` raises `NotADirectoryError` (an `OSError`, not a + `FileNotFoundError`), which is the "learned nothing" arm of `_run_dir_names` + rather than its "no runs" arm. + + Two ablation axes, and they redden different lines. Drop the `run_dirs is + None` arm and the message assert fails: iterating `None` raises, the CLI's + top-level handler still returns 1, and the operator is told `'NoneType' + object is not iterable` instead of what refused. Restore the + `state.json`-gated `runs.list_run_dirs` and the rc assert fails: a file + where the runs dir belongs is simply not a dir, so it lists nothing and the + archive proceeds.""" + from conftest import write_ledger + + from bmad_loop import deferredwork + + install_bmad_config(project) + write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + runs_root = project.project / ".bmad-loop" / "runs" + runs_root.parent.mkdir(parents=True, exist_ok=True) + runs_root.write_text("not a directory", encoding="utf-8") + before = project.deferred_work.read_text(encoding="utf-8") + + rc = cli.main(["sweep", "--archive", "--project", str(project.project)]) + assert rc == 1 + assert "cannot list runs under" in capsys.readouterr().err + assert not (project.deferred_work.parent / deferredwork.ARCHIVE_REL).exists() + assert project.deferred_work.read_text(encoding="utf-8") == before diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 5d842678..99d2500d 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -3013,3 +3013,41 @@ def test_archive_tab_resolution_stub_converges(tmp_path): archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") assert archive_text.count("### DW-1:") == 1 assert "archived: 2026-08-25" not in archive_text + + +def test_archive_fenced_archived_line_in_twin_does_not_suppress(tmp_path): + """An archive entry whose ONLY `archived:` line sits inside a fenced example + is not an archived twin — the crash-recovery skip reads the field through the + fence filter, so the live entry is archived rather than stubbed over its own + body (#711 review, finding 5). + + The twin's body is byte-identical to the ledger entry's, which is what makes + this test decide the fence filter and nothing else. Body equivalence is the + other half of the skip, and it is satisfied here in BOTH directions: with the + filter ablated the same fenced line is stripped from both sides, so the + bodies still compare equal and only `_is_archived` changes its answer. + + Ablation: drop the `_quoted` guard from `_archived_line_spans` and the count + below reads 1 — the fenced example reads as a real stamp and the body never + reaches the archive.""" + entry = ( + "### DW-2: documents the archive field\n\n" + "origin: code review of spec-1-1.md, 2026-05-20\n" + "location: src/foo.py:10\n" + "reason: pre-existing.\n" + "```markdown\n" + "archived: 2026-01-01\n" + "```\n" + "status: done 2026-05-25\n" + ) + path = write_ledger(tmp_path, "# Deferred Work\n\n" + entry) + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text("# Archived Deferred Work\n\n" + entry, encoding="utf-8") + + assert archive_closed(path, archive_date="2026-08-24") == ["DW-2"] + archive_text = archive_path.read_text(encoding="utf-8") + assert archive_text.count("### DW-2:") == 2 # the quoted stamp suppressed nothing + assert archive_text.count("archived: 2026-08-24") == 1 # the real stamp, once + # ...and the body left the ledger for the archive rather than being dropped. + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-2"] + assert "reason: pre-existing." not in stub.body diff --git a/tests/test_runs.py b/tests/test_runs.py index 2768c6e7..93f15c05 100644 --- a/tests/test_runs.py +++ b/tests/test_runs.py @@ -104,6 +104,33 @@ def test_list_run_dirs_missing(tmp_path): assert runs.latest_run_dir(tmp_path) is None +def test_all_run_dirs_includes_state_json_less_dirs(tmp_path): + """The ungated counterpart sees the run `list_run_dirs` filters out — a run + whose state.json is gone still owns its engine.pid, and a liveness guard + built on the gated view would archive out from under it (#711 review).""" + _make_run(tmp_path, "20260611-120000-bbbb") + _make_run(tmp_path, "20260610-090000-aaaa") + _make_run(tmp_path, "20260612-080000-cccc", with_state=False) + listed = runs.all_run_dirs(tmp_path) + assert listed is not None + assert [d.name for d in listed] == [ + "20260610-090000-aaaa", + "20260611-120000-bbbb", + "20260612-080000-cccc", + ] + assert all(d.parent == tmp_path / runs.RUNS_DIR for d in listed) + + +def test_all_run_dirs_distinguishes_missing_from_unreadable(tmp_path): + """A missing runs root is a real answer (no runs); an unreadable one is no + answer at all. Callers that refuse on "cannot tell" need them apart, so the + empty list and `None` must not collapse into each other.""" + assert runs.all_run_dirs(tmp_path) == [] + (tmp_path / runs.RUNS_DIR).parent.mkdir(parents=True, exist_ok=True) + (tmp_path / runs.RUNS_DIR).write_text("not a directory", encoding="utf-8") + assert runs.all_run_dirs(tmp_path) is None + + def test_latest_run_dir(tmp_path): _make_run(tmp_path, "20260610-090000-aaaa") newest = _make_run(tmp_path, "20260611-120000-bbbb") From 3900379a687ae9f75c00feb621ed12038e821b4c Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 16:23:57 -0700 Subject: [PATCH 11/16] docs(sweep): condition the archive durable-note on trackedness; point triage at the archive file (#711 review) --- docs/FEATURES.md | 2 +- src/bmad_loop/cli.py | 9 +++++++-- src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md | 4 ++++ .../data/skills/bmad-loop-sweep/deferred-work-format.md | 3 +++ tests/test_cli.py | 2 +- 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 02b1f98b..1867fd88 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,7 +133,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. -- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer — and both touched files are tracked, so the move is durable only once committed. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). +- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer. When the ledger is tracked, the move is durable only once both files are committed; a gitignored ledger — the default shape — or an artifact dir outside the repo has nothing to commit. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume <id>`). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve <id> --restore-patch <path>` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 5f6d4728..d7352fce 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2322,7 +2322,7 @@ def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse print(f"archived {len(archived)} {noun} to {archive_path}:") for dw_id in archived: print(f" {dw_id}") - print("note: the ledger and archive are tracked files — commit them to make the move durable") + print("note: if the ledger is tracked, commit both files to make the move durable") return ExitCode.OK @@ -4452,7 +4452,12 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: "--force", action="store_true", help="stop the run first if it is still live" ) - archive_p = add("archive", cmd_archive, "compress a run into .bmad-loop/archive and remove it") + archive_p = add( + "archive", + cmd_archive, + "compress a run into .bmad-loop/archive and remove it; " + "for ledger archiving see `sweep --archive`", + ) archive_p.add_argument("run_id") archive_p.add_argument( "--force", action="store_true", help="stop the run first if it is still live" diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md b/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md index d103a914..e78265cb 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md @@ -35,6 +35,10 @@ are `### DW-<n>:` blocks whose `status:` line is `open`. If the ledger is missing or unreadable, escalate `CRITICAL` (`type: missing-ledger`) per automation-mode.md and end your turn. +An entry carrying an `archived:` line keeps only a stub here — its full body +lives in the sibling `deferred-work-archive.md`, keyed by the same DW- id; read +it there before classifying that entry. + If the invocation carries `--feedback <path>`, read that file FIRST — it lists the deterministic validation errors your previous attempt's result.json failed on. Fix exactly those defects in this attempt's output. diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index dcacd11f..a15da081 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -45,6 +45,9 @@ Rules for sessions reading the ledger: - An `archived:` line marks content that lives in `deferred-work-archive.md`. The full body (evidence, resolution, dates) is there, keyed by the same DW- id — read the archive file for anything beyond the stub. +- The archive may be absent even when stubs reference it: only the ledger is + seeded into an isolated unit worktree. That is not an error — the fields the + stub preserves are sufficient for dedupe. - Stubs keep load-bearing field lines (`gate:`, `origin:`/`source_spec:`, resolution undo markers) — treat them exactly as if the entry were still whole. Never edit or drop them when touching a stub. diff --git a/tests/test_cli.py b/tests/test_cli.py index 5916f90c..32f6ecee 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9176,7 +9176,7 @@ def test_sweep_archive_writes_ledger_and_archive(project, capsys): assert rc == 0 out = capsys.readouterr().out assert "archived 1 entry" in out - assert "commit them to make the move durable" in out + assert "commit" in out # durable-note printed; its wording is not a contract ledger_path = project.deferred_work archive_path = ledger_path.parent / "deferred-work-archive.md" text = ledger_path.read_text(encoding="utf-8") From 9168c0f36dae2142a70e823fc36ce0cab26107ec Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 16:36:22 -0700 Subject: [PATCH 12/16] test(sweep): de-vacuous the --archive dry-run CLI assert; correct the CHANGELOG ledger claim (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit test_sweep_archive_dry_run asserted only the preview message, and that message branches on args.dry_run rather than on whether anything was written: dropping `dry_run=` from the archive_closed call rewrites the ledger and writes the archive while the test stays green. Ablation-proven — under that ablation both message asserts still passed and the new archive-absent assert reddened. Now pins an absent archive file and a byte-identical ledger, matching the refusal tests strengthened in the liveness phase. CHANGELOG: the Added bullet claimed the live ledger "stays proportional to open work rather than to all history", but archiving leaves a stub per closed entry, so the file keeps a term linear in closed count. Reworded to what it actually does. --- CHANGELOG.md | 8 ++++---- tests/test_cli.py | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f311c62..f88ce40b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,10 +18,10 @@ breaking changes may land in a minor release. - **`bmad-loop sweep --archive`** moves closed (`status: done <ISO date>`) deferred-work entries to a sibling `deferred-work-archive.md`, replacing each with a stub that preserves the DW- id for grep and `closes_deferred` cross-references plus the load-bearing field lines (`gate:`, - `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger stays proportional to - open work rather than to all history. Supports `--before DATE` to archive only entries closed - before a cutoff, and `--dry-run` to preview. Refuses while any engine run is live. Pure - deterministic Python — no LLM involvement. + `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger then carries open + entries in full and archived ones as compact stubs, rather than every closed body forever. + Supports `--before DATE` to archive only entries closed before a cutoff, and `--dry-run` to + preview. Refuses while any engine run is live. Pure deterministic Python — no LLM involvement. ### Changed diff --git a/tests/test_cli.py b/tests/test_cli.py index 32f6ecee..aa18981a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9152,15 +9152,28 @@ def test_sweep_archive_rejects_bad_before_date(project, capsys): def test_sweep_archive_dry_run(project, capsys): + """--dry-run previews and writes nothing. + + The "would archive" wording branches on ``args.dry_run``, not on whether + anything was written, so the message alone is a vacuous oracle: drop + ``dry_run=`` from the ``archive_closed`` call and the preview still reads + the same while the ledger is rewritten under it. The file asserts are what + make this test decide the wiring (#711 review). + """ from conftest import write_ledger + from bmad_loop import deferredwork + install_bmad_config(project) write_ledger(project, {"DW-1": "open", "DW-2": "done 2026-06-01"}, commit=False) + before = project.deferred_work.read_text(encoding="utf-8") rc = cli.main(["sweep", "--archive", "--dry-run", "--project", str(project.project)]) assert rc == 0 out = capsys.readouterr().out assert "would archive 1 entry" in out assert "DW-2" in out + assert not (project.deferred_work.parent / deferredwork.ARCHIVE_REL).exists() + assert project.deferred_work.read_text(encoding="utf-8") == before def test_sweep_archive_writes_ledger_and_archive(project, capsys): From f898f5ebd3182a1a1ea01ef970b4e16c1a376675 Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 18:13:39 -0700 Subject: [PATCH 13/16] fix(sweep): keep a reopened archive stub resolvable to its archived body (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mark_open dropped the entry's live `archived:` stamps, and the sweep triage rule keys on exactly that line — so a reopened stub, the one case the strip changed, carried no pointer at all. A stub preserves neither `location:` nor `reason:` (_PRESERVED_FIELD_RE), so the reopened entry reached triage with a heading and nothing to triage. Two individually-correct fixes, one joint hole. The stamp is now demoted rather than deleted: `archived:` becomes `archived-body:`, value and spacing verbatim. That keeps both properties the strip was protecting and the pointer it was destroying — the renamed line matches neither _ARCHIVED_FIELD_RE nor _STUB_BODY_RE, so the entry still reads as live and a later reopenable close still re-archives normally instead of reconstituting a stub _is_stub skips forever, while the value still names the archive block (each carries its own `archived:` date, and an id owns several once a divergent re-closure is archived too). Rehydrating the body during reopen was the alternative and is worse: several blocks per id is by design, so mark_open — called from a rollback — would have to guess which one, and a wrong guess overwrites live content with a stale body. test_mark_open_leaves_a_pointer_to_the_archived_body walks the pointer the way a triage session must: live entry -> `archived-body:` date -> the archive block stamped with it -> its `location:`/`reason:`. Ablation-proven with a cp backup — replacing the demotion with the old deletion reddens it on `assert pointers == ["archived-body: 2026-08-24"]` reading `[]`, while test_archive_reopened_stub_recloses_and_archives stays green under the same ablation, so the two guarantees sit on disjoint axes. Docs: the sweep SKILL.md triage step and deferred-work-format.md now describe `archived-body:` alongside `archived:`; FEATURES.md and the existing [Unreleased] bullet gained the reopen clause. --- CHANGELOG.md | 4 +- docs/FEATURES.md | 2 +- .../data/skills/bmad-loop-sweep/SKILL.md | 5 +- .../bmad-loop-sweep/deferred-work-format.md | 14 +++- src/bmad_loop/deferredwork.py | 64 +++++++++++++++---- tests/test_deferredwork.py | 45 ++++++++++++- 6 files changed, 111 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f88ce40b..f7014b50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,9 @@ breaking changes may land in a minor release. `origin:`/`source_spec:`, reopenable-close undo markers). The live ledger then carries open entries in full and archived ones as compact stubs, rather than every closed body forever. Supports `--before DATE` to archive only entries closed before a cutoff, and `--dry-run` to - preview. Refuses while any engine run is live. Pure deterministic Python — no LLM involvement. + preview. Reopening an archived stub leaves an `archived-body:` line pointing at the archive + block that holds its body. Refuses while any engine run is live. Pure deterministic Python — + no LLM involvement. ### Changed diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 1867fd88..9ee0a2a7 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -133,7 +133,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Auto-sweep at epic boundaries or run-end (`[sweep] auto`); a failed or paused child sweep is journaled + notified and leaves the parent running — including the `SystemExit` an unusable multiplexer or an unresolvable profile raises, which used to end the parent at exit 1 with an orphaned session (#600). A stop (`bmad-loop stop`) or a Ctrl-C delivered through the child is the deliberate exception and propagates, so a parent stays stoppable while a child sweep is mid-flight (#601). - A trigger is spent only once its child has actually started, so a refusal no longer consumes it (#501) — but nothing re-asks it either, since both call sites close their boundary within a few statements of the refusal. What the run keeps instead is a record: `sweeps_refused` (trigger → `not-started` / `failed` / `dirty`), surfaced by the end-of-run summary, `bmad-loop status`, `status --json` and `bmad-loop diagnose`, naming `bmad-loop sweep` (which needs a clean worktree) as the human-present follow-up. - Repeat mode (`--repeat` / `[sweep] repeat`): re-triages after each cycle to absorb newly generated deferred work, stopping when a cycle does nothing addressable or hits `max_cycles`. -- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer. When the ledger is tracked, the move is durable only once both files are committed; a gitignored ledger — the default shape — or an artifact dir outside the repo has nothing to commit. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). +- Ledger archiving (`sweep --archive`, optionally `--before DATE` to bound by close date; `--dry-run` previews): moves closed (`status: done <ISO date>`) entries to sibling `deferred-work-archive.md` (body preserved, an `archived: <date>` marker appended), leaving an id-preserving stub (`status: done <date>` + `archived: <date>`) so grep and `closes_deferred` cross-references keep resolving and the live ledger stays proportional to open work rather than all history. The stub keeps load-bearing field lines — `gate:` (validate's closed-gate report keeps speaking), `origin:`/`source_spec:` (the harvest-replay dedupe stays status-agnostic), and the reopenable-close undo tail (a paused sweep's bundle rollback still works). Reopening an archived stub — what that rollback does — demotes its `archived:` stamp to `archived-body:` rather than dropping it: the entry is live work again, so the stamp would be a lie and its shape would strand the entry outside every future archive, but the body its close moved out is still in the archive file and that line is what a later triage follows to it. Deterministic, no LLM, atomic writes with crash-safe ordering (archive before ledger; a retry keyed on id + close date completes the move without duplicate bodies). Refuses while any engine run is live or its liveness is unverifiable — it is the one out-of-band ledger writer. When the ledger is tracked, the move is durable only once both files are committed; a gitignored ledger — the default shape — or an artifact dir outside the repo has nothing to commit. Unrelated to `bmad-loop archive <run-id>` (run-tarball archiving). - Sweeps are their own resumable runs (`bmad-loop resume <id>`). An escalated bundle resolves like a story escalation, including intent-gap patch-restore: `bmad-loop resolve <id> --restore-patch <path>` re-arms the bundle spec to `in-review` and the re-driven bundle session resumes review on the re-applied patch instead of re-implementing. ### Stories mode (folder+id dispatch) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md b/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md index e78265cb..eef2b40b 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/SKILL.md @@ -37,7 +37,10 @@ automation-mode.md and end your turn. An entry carrying an `archived:` line keeps only a stub here — its full body lives in the sibling `deferred-work-archive.md`, keyed by the same DW- id; read -it there before classifying that entry. +it there before classifying that entry. An `archived-body:` line says the same +of an entry that was archived and later reopened: it is live work again, but the +body it carried before that close is still in the archive file, in the block +stamped with the date the line carries. If the invocation carries `--feedback <path>`, read that file FIRST — it lists the deterministic validation errors your previous attempt's result.json failed diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index a15da081..e2317099 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -45,9 +45,17 @@ Rules for sessions reading the ledger: - An `archived:` line marks content that lives in `deferred-work-archive.md`. The full body (evidence, resolution, dates) is there, keyed by the same DW- id — read the archive file for anything beyond the stub. -- The archive may be absent even when stubs reference it: only the ledger is - seeded into an isolated unit worktree. That is not an error — the fields the - stub preserves are sufficient for dedupe. +- An `archived-body:` line is that same pointer carried by an entry that was + archived and then **reopened** — the orchestrator writes it in place of the + `archived:` stamp, which would otherwise claim a live entry's body is + elsewhere. The entry is open work again and its `status:` says so, but the + body it carried before that close is still in the archive file, in the block + stamped with the date this line carries. Reopening does not bring the body + back, and a stub keeps neither `location:` nor `reason:`, so read that block + before triaging the entry. Never edit or drop the line. +- The archive may be absent even when a stub or an `archived-body:` line + references it: only the ledger is seeded into an isolated unit worktree. That + is not an error — the fields the stub preserves are sufficient for dedupe. - Stubs keep load-bearing field lines (`gate:`, `origin:`/`source_spec:`, resolution undo markers) — treat them exactly as if the entry were still whole. Never edit or drop them when touching a stub. diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index ba65b465..1e44c31b 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -869,6 +869,11 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: The entry must still carry the operation's adjacent resolution and undo-marker lines. A standard or earlier close has no matching marker and cannot be reopened merely because it reused the same human-readable note. + + A live ``archived:`` stamp is demoted to :data:`_ARCHIVED_BODY_FIELD` rather + than dropped: the reopened entry is no longer archived, but the body its + close moved out still is, and that line is the only thing a later triage has + to find it with. """ undo_owner = _operation_digest(operation_id) if not path.is_file(): @@ -908,20 +913,42 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: return False start = entry.span[0] + entry.status_span[0] end = entry.span[0] + res_m.end() - # Drop the entry's live `archived:` stamps along with the close they - # describe. A stub's stamp says "this body lives in the archive file"; once - # the close is undone the body is here and the line is a lie, and leaving it - # is not merely untidy — status + undo tail + stamp is the exact - # `_STUB_BODY_RE` shape, so the next reopenable close reconstitutes a stub - # `archive_closed` skips forever, stranding the entry outside every future - # archive (#711). Cuts are disjoint (an `^archived:` line cannot start - # inside the status line or its adjacent tail) and applied back-to-front so - # earlier offsets stay valid. + # Demote the entry's live `archived:` stamps along with the close they + # describe, rather than deleting them. A stub's stamp says "this body lives + # in the archive file"; once the close is undone the body is here and the + # line is a lie, and leaving it standing is not merely untidy — status + + # undo tail + stamp is the exact `_STUB_BODY_RE` shape, so the next + # reopenable close reconstitutes a stub `archive_closed` skips forever, + # stranding the entry outside every future archive (#711). + # + # Cutting the line outright strands the entry a second way: a stub keeps + # neither `location:` nor `reason:` (`_PRESERVED_FIELD_RE`), so the stamp is + # the reopened entry's ONLY route back to the body, and triage arrives with + # a heading and nothing to triage (#711 review). Renaming the field keeps + # both properties — the value still names the archive block, since an id may + # own several and each carries its own `archived:` date, while the renamed + # line matches neither `_ARCHIVED_FIELD_RE` nor `_STUB_BODY_RE`, so the + # entry reads as live and re-archives normally. Rehydrating the body here + # instead was the alternative and is worse: several blocks per id is by + # design, so a rollback's reopen would have to guess which one, and a wrong + # guess overwrites live content with a stale body. + # + # Cuts are disjoint (an `^archived:` line cannot start inside the status + # line or its adjacent tail) and applied back-to-front so earlier offsets + # stay valid. cuts = [(start, end, previous_status_line)] - cuts += [ - (entry.span[0] + cut_start, entry.span[0] + cut_end, "") - for cut_start, cut_end in _archived_line_spans(entry) - ] + for cut_start, cut_end in _archived_line_spans(entry): + # Everything after the field name — value, spacing and the terminating + # newline — carries over verbatim; the span starts at the anchor, so + # the first colon is the field's own. + stamp = entry.body[cut_start:cut_end].split(":", 1)[1] + cuts.append( + ( + entry.span[0] + cut_start, + entry.span[0] + cut_end, + f"{_ARCHIVED_BODY_FIELD}{stamp}", + ) + ) for cut_start, cut_end, replacement in sorted(cuts, reverse=True): text = text[:cut_start] + replacement + text[cut_end:] atomic_write_text(path, text) @@ -1079,6 +1106,15 @@ def append_entry( # archive would accumulate duplicates. _ARCHIVED_FIELD_RE = re.compile(r"^archived:", re.MULTILINE) +# What :func:`mark_open` leaves where that stamp was. A reopened entry is not +# archived — its body is back in the ledger — but the body the undone close +# moved out still is, and this line is what a triage session follows to it. +# Deliberately a different field name: `archived:` means "the body is +# elsewhere", which a reopened entry must not claim, and a line matching +# `_ARCHIVED_FIELD_RE` here would rebuild the exact `_STUB_BODY_RE` shape on +# the next reopenable close. +_ARCHIVED_BODY_FIELD = "archived-body:" + def _archived_line_spans(entry: DWEntry) -> list[tuple[int, int]]: """Body-relative spans of the entry's live ``archived:`` field lines, each @@ -1090,7 +1126,7 @@ def _archived_line_spans(entry: DWEntry) -> list[tuple[int, int]]: check a quoted ``archived:`` would be mistaken for the real thing. The one place that rule is written, so the three questions asked about the field — is this entry archived, what does its body say apart from the stamp, and - which bytes must a reopen drop — cannot answer it differently. + which bytes must a reopen rename — cannot answer it differently. Whole lines rather than match starts because both cutting callers remove the line, and a span ending at the anchor would leave the stamp's value diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index 99d2500d..b959001d 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2896,7 +2896,8 @@ def test_archive_fenced_heading_in_archive_does_not_suppress_reclose(tmp_path): # `append_decision` writes to a closed entry without reading its status. The # crash-recovery skip therefore cannot key on id + close date alone, the stub # shape must survive the spacing `_MARK_DONE_TAIL_RE` tolerates, and a reopen -# must drop the `archived:` stamp that no longer describes the entry. +# must demote the `archived:` stamp that no longer describes the entry without +# severing the reopened entry from the body that stamp was pointing at. def test_archive_same_date_reclose_preserves_new_body(tmp_path): @@ -2940,8 +2941,8 @@ def test_archive_decision_on_stub_preserved(tmp_path): def test_mark_open_strips_archived_line(tmp_path): """Reopening drops the entry's live `archived:` stamp — the body is back in - the ledger, so the line is a lie — while a fenced example of the field is - left alone (#711 review).""" + the ledger, so the line is a lie — demoting it to `archived-body:`, while a + fenced example of the field is left alone (#711 review).""" text = ( "# Deferred Work\n\n" "### DW-1: documents the field it also carries\n\n" @@ -2961,10 +2962,48 @@ def test_mark_open_strips_archived_line(tmp_path): entry = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] assert entry.open assert "archived: 2026-08-24" not in entry.body # the live stamp is gone + assert "archived-body: 2026-08-24" in entry.body # demoted, not deleted assert "archived: 2026-01-01" in entry.body # the fenced example is not a stamp + assert "archived-body: 2026-01-01" not in entry.body # ...so it was not demoted assert "origin: a" in entry.body # nothing else was cut +def test_mark_open_leaves_a_pointer_to_the_archived_body(tmp_path): + """A reopened stub stays triage-resolvable. Its `location:`/`reason:` are in + the archive file — the stub preserves neither — so the demoted + `archived-body:` line has to name the block holding them. Deleting the + stamp outright left triage a heading and nothing to triage (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: reopened after archiving\n\n" + "origin: a\n" + "location: src/x.py:1\n" + "reason: waiting on the codec seam\n" + "status: open\n" + ) + path = write_ledger(tmp_path, text) + close_reopenable(path, "DW-1", "bundle close") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + assert mark_open(path, "DW-1", "bundle close", OPERATION_ID) is True + + entry = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + assert entry.open + assert "reason: waiting on the codec seam" not in entry.body # the body did move out + pointers = [ln for ln in entry.body.splitlines() if ln.startswith("archived-body:")] + assert pointers == ["archived-body: 2026-08-24"] # and this is what says where to + + # Walk the pointer the way a triage session must: its date picks the block, + # since one id owns several once a divergent re-closure is archived too. + stamp = pointers[0].split(":", 1)[1].strip() + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + blocks = [ + e for e in parse_ledger(archive_text) if e.id == "DW-1" and f"archived: {stamp}" in e.body + ] + assert len(blocks) == 1 + assert "location: src/x.py:1" in blocks[0].body + assert "reason: waiting on the codec seam" in blocks[0].body + + def test_archive_reopened_stub_recloses_and_archives(tmp_path): """Full cycle: archive, reopen, re-close reopenably at a later date. Without the reopen-side strip the re-close rebuilds the exact stub shape and From a4c9ba485e39c635248947ebfba7409564967966 Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 18:30:01 -0700 Subject: [PATCH 14/16] fix(sweep): validate --archive --before ahead of the missing-ledger return (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sweep --archive --before not-a-date` exited 0 on a project with no ledger and 1 on one that has it: the CLI's missing-ledger short-circuit ran before `archive_closed`, which is where the cutoff is validated. The same invocation was graded by optional project data rather than by its own shape. archive_closed already orders `_require_iso_date` ahead of its own `is_file` short-circuit, and its docstring says why — "so a programmer bug fails the same way whether or not a ledger exists". The CLI put that back by checking first. Now the primitive is called first and the missing ledger is reported from its empty result; the call is safe on an absent file, which is the property that ordering already relied on. Behavior is otherwise unchanged: a missing ledger still prints "no deferred-work ledger at <path>" and returns OK, and a malformed date still reports through the same ValueError path at rc 1. test_sweep_archive_rejects_bad_before_date_without_a_ledger is ablation-proven with a cp backup — restoring the old order reddens it on `assert rc == 1` reading `ExitCode.OK`, with the missing-ledger line on stdout; the other twelve sweep-archive CLI tests stay green under that ablation. --- src/bmad_loop/cli.py | 15 ++++++++++++--- tests/test_cli.py | 13 +++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index d7352fce..27999463 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -2297,9 +2297,15 @@ def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse ) return ExitCode.FAILURE ledger = paths.deferred_work - if not ledger.is_file(): - print(f"no deferred-work ledger at {ledger}") - return ExitCode.OK + # Call the primitive BEFORE reporting a missing ledger, and report the + # missing ledger from its empty result. `archive_closed` validates `before` + # ahead of its own `is_file` short-circuit precisely so a malformed date + # fails the same way whether or not a ledger exists; short-circuiting here + # first put that back, and `--before not-a-date` then exited 0 on a project + # that happens to have no ledger today and 1 on one that does — the same + # invocation graded by optional project data rather than by its own shape + # (#711 review). The call is safe on a missing file: it short-circuits to + # an empty list without writing. try: archived = deferredwork.archive_closed( ledger, @@ -2309,6 +2315,9 @@ def _sweep_archive(project: Path, paths: bmadconfig.ProjectPaths, args: argparse except ValueError as exc: print(f"error: {exc}", file=sys.stderr) return ExitCode.FAILURE + if not ledger.is_file(): + print(f"no deferred-work ledger at {ledger}") + return ExitCode.OK archive_path = ledger.parent / deferredwork.ARCHIVE_REL if not archived: print("no closed entries to archive") diff --git a/tests/test_cli.py b/tests/test_cli.py index aa18981a..9e27cff3 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9151,6 +9151,19 @@ def test_sweep_archive_rejects_bad_before_date(project, capsys): assert "date must be YYYY-MM-DD" in capsys.readouterr().err +def test_sweep_archive_rejects_bad_before_date_without_a_ledger(project, capsys): + """The same malformed `--before` is refused whether or not the project has + a ledger. `archive_closed` validates dates ahead of its own `is_file` + short-circuit for that reason; a missing-ledger early return in the CLI + graded the invocation by optional project data instead (#711 review).""" + install_bmad_config(project) # no ledger written + rc = cli.main(["sweep", "--archive", "--before", "bad-date", "--project", str(project.project)]) + assert rc == 1 + captured = capsys.readouterr() + assert "date must be YYYY-MM-DD" in captured.err + assert "no deferred-work ledger at" not in captured.out # not reported as a clean run + + def test_sweep_archive_dry_run(project, capsys): """--dry-run previews and writes nothing. From 8b4e5fc0b816f74e99d022d17ceb149bf423a8ab Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 18:42:01 -0700 Subject: [PATCH 15/16] fix(sweep): stamp a crash-recovered stub with its archived body's own date (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A retry after a crash between the archive write and the ledger write stamped the recovered stub with the RETRY's date while its body in the archive kept the crashed run's. The two diverge whenever the retry lands on a later day. That was cosmetic while the stamp was only a "there is a body elsewhere" flag. It is not any more: the archive holds several blocks per id by design, so the stamp is what picks one — for a reader following the stub, and for the `archived-body:` pointer mark_open demotes that stamp into, which is a reopened entry's only route back to its body. A stub naming a date no block carries resolves to nothing. The crash-recovery skip now carries the twin's own stamp (new `_archived_stamp` accessor, read through the shared `_archived_line_spans` fence filter like every other question about the field) and the stub loop uses it in place of this run's date. Scoped to entries the skip actually fired for — an entry archived normally is stamped with this run's date on both sides. test_archive_crash_recovery_stub_keeps_the_archived_body_stamp resolves the stub the way a reader must: its stamp must name exactly one block, and that block must hold the body. Ablation-proven with a cp backup — reverting to the run's stamp reddens it on `assert "archived: 2026-08-24" in stub.body` with the stub reading `archived: 2026-08-25`, while test_archive_fresh_stub_keeps_this_runs_stamp (the scoping half) stays green. deferred-work-format.md now says the stamp picks the block, so the rule is the same for `archived:` and `archived-body:`. --- .../bmad-loop-sweep/deferred-work-format.md | 4 +- src/bmad_loop/deferredwork.py | 40 ++++++++++++++--- tests/test_deferredwork.py | 44 +++++++++++++++++++ 3 files changed, 82 insertions(+), 6 deletions(-) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index e2317099..46c3de30 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -44,7 +44,9 @@ Rules for sessions reading the ledger: closed and not open work. - An `archived:` line marks content that lives in `deferred-work-archive.md`. The full body (evidence, resolution, dates) is there, keyed by the same - DW- id — read the archive file for anything beyond the stub. + DW- id — read the archive file for anything beyond the stub. An id may own + more than one block there once a reopened entry has been archived twice; the + date on this line is what picks the block this stub's body is in. - An `archived-body:` line is that same pointer carried by an entry that was archived and then **reopened** — the orchestrator writes it in place of the `archived:` stamp, which would otherwise claim a live entry's body is diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index 1e44c31b..1e054a29 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -1167,6 +1167,22 @@ def _body_without_archived(entry: DWEntry) -> str: return body.rstrip("\n") +def _archived_stamp(entry: DWEntry) -> str | None: + """The value of the entry's first live ``archived:`` field line, or None + when it carries none. + + Read from an *archive* twin, this is that block's identity: the archive + holds several blocks per id by design, and their stamps are what tells them + apart — for a stub pointing at one, and for the `archived-body:` pointer + :func:`mark_open` demotes that stub's stamp into. + """ + spans = _archived_line_spans(entry) + if not spans: + return None + start, end = spans[0] + return entry.body[start:end].split(":", 1)[1].strip() + + # Field lines a stub must carry when the archived body had them, because # downstream readers key on them regardless of status: `gate:` (validate's # closed-entry gate report deliberately keeps speaking), `origin:` + @@ -1280,7 +1296,9 @@ def archive_closed( :func:`atomic_write_text`, the same primitive every ledger writer uses. The archive file accumulates on repeat runs: new entries are appended to the existing file, never overwritten, and stubs from a prior run are - skipped by their exact stub shape. + skipped by their exact stub shape. A stub's ``archived:`` date names the + archive block holding its body, so an entry recovered from a crashed run + is stamped with the date already on that block rather than with this run's. """ if before is not None: _require_iso_date(before) @@ -1332,13 +1350,25 @@ def archive_closed( # a silent drop is not. archive_blocks: list[str] = [] already_archived = { - e.id: (_close_date(e), _body_without_archived(e)) + e.id: ((_close_date(e), _body_without_archived(e)), _archived_stamp(e)) for e in parse_ledger(existing) if _is_archived(e) } # fence-aware: a quoted example in the archive is not a real body + # A recovered entry's stub is stamped with the date already on its archived + # body, not with this run's. The two diverge whenever the retry lands on a + # later day than the crashed run, and the stamp is not decoration: it is + # what picks one of an id's several archive blocks — for a reader following + # the stub, and for the `archived-body:` pointer `mark_open` demotes that + # stamp into, which is a reopened entry's only route back to its body + # (#711 review). A stub naming a date no block carries resolves to nothing. + recovered_stamps: dict[str, str] = {} for entry, close_date in to_archive: - if already_archived.get(entry.id) == (close_date, _body_without_archived(entry)): - continue # this closure's body is already archived (crashed prior run) + twin = already_archived.get(entry.id) + if twin is not None and twin[0] == (close_date, _body_without_archived(entry)): + # this closure's body is already archived (crashed prior run) + if twin[1] is not None: + recovered_stamps[entry.id] = twin[1] + continue body = entry.body assert entry.status_span is not None # done with a date implies a status line pos = entry.status_span[1] @@ -1363,7 +1393,7 @@ def archive_closed( f"### {entry.id}: {entry.title}\n\n" f"status: done {close_date}\n" f"{preserved}" - f"archived: {stamp}\n\n" + f"archived: {recovered_stamps.get(entry.id, stamp)}\n\n" ) start, end = entry.span text = text[:start] + stub + text[end:] diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index b959001d..d78c5cfd 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -2678,6 +2678,50 @@ def test_archive_crash_recovery_no_duplicate_bodies(tmp_path): assert "archived: 2026-08-25" not in archive_text +def test_archive_crash_recovery_stub_keeps_the_archived_body_stamp(tmp_path): + """A stub recovered from a crashed run is stamped with the date already on + its archived body, not with the retry's date. The stamp is what picks one + of an id's several archive blocks — including once `mark_open` demotes it + into the `archived-body:` pointer — so a stub naming a date no block + carries resolves to nothing (#711 review).""" + path = write_ledger(tmp_path) + archive_path = path.parent / ARCHIVE_REL + archive_path.write_text( + "### DW-2: Old closed item\n\n" + "origin: code review of spec-1-1.md, 2026-05-20\n" + "location: src/foo.py:10\n" + "reason: pre-existing.\n" + "status: done 2026-05-25\n" + "archived: 2026-08-24\n", + encoding="utf-8", + ) + assert archive_closed(path, archive_date="2026-08-25") == ["DW-2"] + + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-2"] + assert "archived: 2026-08-24" in stub.body # the body's own stamp + assert "archived: 2026-08-25" not in stub.body # not the retry's + + # Resolve the stub the way a reader must: its stamp names exactly one block. + stamp = deferredwork._archived_stamp(stub) + archive_text = archive_path.read_text(encoding="utf-8") + blocks = [ + e for e in parse_ledger(archive_text) if e.id == "DW-2" and f"archived: {stamp}" in e.body + ] + assert len(blocks) == 1 + assert "reason: pre-existing." in blocks[0].body + + +def test_archive_fresh_stub_keeps_this_runs_stamp(tmp_path): + """The recovered-stamp carry-over is scoped to entries the crash-recovery + skip fired for: an entry archived normally is stamped with this run's date + on both sides (#711 review).""" + path = write_ledger(tmp_path) + assert archive_closed(path, archive_date="2026-08-25") == ["DW-2"] + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-2"] + assert "archived: 2026-08-25" in stub.body + assert "archived: 2026-08-25" in (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + + def test_archive_crash_recovery_fenced_example_does_not_suppress(tmp_path): """A fenced worked example in the archive quoting `### DW-2:` is not a real archived body — the crash-recovery skip must be fence-aware, so the From 9498b517419b968f8d5674b8106cd2dbe59fe4ec Mon Sep 17 00:00:00 2001 From: t <t@t> Date: Mon, 24 Aug 2026 18:54:53 -0700 Subject: [PATCH 16/16] docs(sweep): make the archive-block resolution rule exact, and pin its tie-break (#711 review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two closures of one id archived on the SAME day share a stamp, so a stub's date narrows to its archive block rather than identifying it. The prior round's doc said "the date picks the block", which over-claims in exactly that case. Rejected the reviewer's remedy — a time or sequence discriminator inside the stamp — as disproportionate: `archived: <date>` is documented in the format skill, FEATURES.md and the CHANGELOG, `archive_date` is an ISO-date seam validated by `_require_iso_date`, and a dozen tests read the field as a date. Widening it would change a published field shape for a case that already has a deterministic answer. That answer is the archive's append order: blocks are appended, never prepended, so for one id a later block is a later closure. Documented as the tie-break for both `archived:` and `archived-body:`, and stated at the append itself, so a later reduction cannot quietly invert it. test_archive_same_day_reclosures_resolve_by_append_order drives the full cycle — close, archive, reopen, re-close, archive again on the same date — and pins both halves: the two blocks really do share a stamp, and file order is closure order. Ablation-proven with a cp backup: prepending instead of appending reddens it on `assert "resolution: first close" in blocks[0].body`, which then holds the second closure. --- .../bmad-loop-sweep/deferred-work-format.md | 13 +++++--- src/bmad_loop/deferredwork.py | 21 +++++++----- tests/test_deferredwork.py | 33 +++++++++++++++++++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md index 46c3de30..e197e138 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/deferred-work-format.md @@ -45,15 +45,18 @@ Rules for sessions reading the ledger: - An `archived:` line marks content that lives in `deferred-work-archive.md`. The full body (evidence, resolution, dates) is there, keyed by the same DW- id — read the archive file for anything beyond the stub. An id may own - more than one block there once a reopened entry has been archived twice; the - date on this line is what picks the block this stub's body is in. + more than one block there once a reopened entry has been archived twice. + Narrow by the date on this line; when several blocks share it — one entry + closed, archived, reopened and archived again inside a single day — take the + **last** of them. The archive file is append-only, so for one id a later + block is a later closure. - An `archived-body:` line is that same pointer carried by an entry that was archived and then **reopened** — the orchestrator writes it in place of the `archived:` stamp, which would otherwise claim a live entry's body is elsewhere. The entry is open work again and its `status:` says so, but the - body it carried before that close is still in the archive file, in the block - stamped with the date this line carries. Reopening does not bring the body - back, and a stub keeps neither `location:` nor `reason:`, so read that block + body it carried before that close is still in the archive file — resolve + this line exactly as an `archived:` stamp above: narrow by its date, and take + the last of the blocks that share it. Reopening does not bring the body back, and a stub keeps neither `location:` nor `reason:`, so read that block before triaging the entry. Never edit or drop the line. - The archive may be absent even when a stub or an `archived-body:` line references it: only the ledger is seeded into an isolated unit worktree. That diff --git a/src/bmad_loop/deferredwork.py b/src/bmad_loop/deferredwork.py index 1e054a29..0d6e8c05 100644 --- a/src/bmad_loop/deferredwork.py +++ b/src/bmad_loop/deferredwork.py @@ -925,10 +925,10 @@ def mark_open(path: Path, dw_id: str, note: str, operation_id: str) -> bool: # neither `location:` nor `reason:` (`_PRESERVED_FIELD_RE`), so the stamp is # the reopened entry's ONLY route back to the body, and triage arrives with # a heading and nothing to triage (#711 review). Renaming the field keeps - # both properties — the value still names the archive block, since an id may - # own several and each carries its own `archived:` date, while the renamed - # line matches neither `_ARCHIVED_FIELD_RE` nor `_STUB_BODY_RE`, so the - # entry reads as live and re-archives normally. Rehydrating the body here + # both properties — the value still narrows to the archive block, an id + # owning several once a re-closure is archived too, while the renamed line + # matches neither `_ARCHIVED_FIELD_RE` nor `_STUB_BODY_RE`, so the entry + # reads as live and re-archives normally. Rehydrating the body here # instead was the alternative and is worse: several blocks per id is by # design, so a rollback's reopen would have to guess which one, and a wrong # guess overwrites live content with a stale body. @@ -1171,10 +1171,12 @@ def _archived_stamp(entry: DWEntry) -> str | None: """The value of the entry's first live ``archived:`` field line, or None when it carries none. - Read from an *archive* twin, this is that block's identity: the archive - holds several blocks per id by design, and their stamps are what tells them - apart — for a stub pointing at one, and for the `archived-body:` pointer - :func:`mark_open` demotes that stub's stamp into. + Read from an *archive* twin, this is what a stub pointing at that block + must carry — and what :func:`mark_open` demotes into an `archived-body:` + pointer. The archive holds several blocks per id by design, so the stamp + narrows rather than identifies: two closures archived on one day share it, + and the append-only file's order is the tie-break (later block, later + closure). """ spans = _archived_line_spans(entry) if not spans: @@ -1374,6 +1376,9 @@ def archive_closed( pos = entry.status_span[1] body = body[:pos] + f"\narchived: {stamp}" + body[pos:] archive_blocks.append(body) + # Appended, never prepended: for one id the file's order is closure order, + # which is the documented tie-break when two closures were archived on the + # same day and so carry the same stamp (#711 review). if archive_blocks: if existing == "" or existing.endswith("\n\n"): sep = "" diff --git a/tests/test_deferredwork.py b/tests/test_deferredwork.py index d78c5cfd..fc578f5e 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -3072,6 +3072,39 @@ def test_archive_reopened_stub_recloses_and_archives(tmp_path): assert "resolution: second close" in archive_text +def test_archive_same_day_reclosures_resolve_by_append_order(tmp_path): + """Two closures of one id archived on the SAME day share a stamp, so the + stamp narrows rather than identifies. The tie-break the format documents is + the archive's append order — later block, later closure — and that is a + property of how `archive_closed` writes, not a convention a reader can only + hope for (#711 review).""" + text = ( + "# Deferred Work\n\n" + "### DW-1: closed twice in one day\n\n" + "origin: a\nlocation: src/x.py:1\nreason: first pass\nstatus: open\n" + ) + path = write_ledger(tmp_path, text) + close_reopenable(path, "DW-1", "first close", date="2026-06-11") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + assert mark_open(path, "DW-1", "first close", OPERATION_ID) is True + close_reopenable(path, "DW-1", "second close", date="2026-06-12") + assert archive_closed(path, archive_date="2026-08-24") == ["DW-1"] + + stub = {e.id: e for e in parse_ledger(path.read_text(encoding="utf-8"))}["DW-1"] + stamp = deferredwork._archived_stamp(stub) + archive_text = (path.parent / ARCHIVE_REL).read_text(encoding="utf-8") + blocks = [ + e for e in parse_ledger(archive_text) if e.id == "DW-1" and f"archived: {stamp}" in e.body + ] + # Both closures carry the same stamp — the ambiguity the tie-break exists for + assert len(blocks) == 2 + # ...and file order is closure order, so the LAST one is what the stub points at + assert "resolution: first close" in blocks[0].body + assert "resolution: second close" in blocks[-1].body + assert "status: done 2026-06-11" in blocks[0].body + assert "status: done 2026-06-12" in blocks[-1].body + + def test_archive_tab_resolution_stub_converges(tmp_path): """A tab-separated undo tail is copied into the stub verbatim, so the stub shape must tolerate the spacing `_MARK_DONE_TAIL_RE` accepts. Otherwise the