diff --git a/CHANGELOG.md b/CHANGELOG.md index 7125f70c..f7014b50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,16 @@ breaking changes may land in a minor release. compatibility contract. The probe opens non-blocking, so a planted reader-less FIFO cannot park the orchestrator. +- **`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 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. 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 - **A published run archive now lands at mode `0600`** instead of a umask-derived mode (#591). 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 ` 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 121d1f73..9ee0a2a7 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 to sibling `deferred-work-archive.md` (body preserved, an `archived: ` marker appended), 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). 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-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 9e09c4ce..27999463 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", @@ -2212,6 +2211,28 @@ def cmd_sweep(args: argparse.Namespace) -> int: return rc project = _project(args) paths = bmadconfig.load_paths(project) + + if args.before is not None 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 + or args.no_prompt + or args.run_id is not None + ): + print( + "--archive cannot combine with --decisions-only, --repeat, " + "--max-bundles, --max-cycles, --no-prompt, or --run-id", + file=sys.stderr, + ) + return ExitCode.FAILURE + return _sweep_archive(project, paths, args) + pol = policy_mod.load(_policy_path(project)) if args.dry_run: @@ -2246,6 +2267,74 @@ 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 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. + + 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", + file=sys.stderr, + ) + return ExitCode.FAILURE + ledger = paths.deferred_work + # 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, + before=args.before, + dry_run=args.dry_run, + ) + 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") + 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 ExitCode.OK + print(f"archived {len(archived)} {noun} to {archive_path}:") + for dw_id in archived: + print(f" {dw_id}") + print("note: if the ledger is tracked, commit both files to make the move durable") + return ExitCode.OK + + 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. @@ -4222,7 +4311,23 @@ 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", + ) + 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) @@ -4356,7 +4461,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..eef2b40b 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,13 @@ are `### DW-:` 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. 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 `, 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 03cb9b56..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 @@ -20,7 +20,53 @@ 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: ` marker +appended after the status line — 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. An id may own + 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 — 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 + 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. +- 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 bb973d90..0d6e8c05 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: @@ -857,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(): @@ -896,7 +913,45 @@ 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:]) + # 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 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. + # + # 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)] + 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) return True @@ -1044,6 +1099,319 @@ 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) + +# 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 + 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 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 + 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)) + + +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). + """ + body = entry.body + for start, end in reversed(_archived_line_spans(entry)): + body = body[:start] + body[end:] + 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 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: + 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:` + +# `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" + # 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?" +) + + +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 + 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: # exactly `done YYYY-MM-DD` — extra tokens are not a close date + return None + # 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( + path: Path, + *, + before: str | None = None, + archive_date: str | None = None, + dry_run: bool = False, +) -> list[str]: + """Move closed (``status: done <date>``) 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: <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), 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 + 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 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) + 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_stub(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. + # + # 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 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), _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: + 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] + 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 = "" + elif existing.endswith("\n"): + sep = "\n" + else: + sep = "\n\n" + archive_content = existing + sep + "".join(archive_blocks) + else: + 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. + for entry, close_date in reversed(to_archive): + 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: {recovered_stamps.get(entry.id, 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/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 8efa997a..9e27cff3 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") @@ -9137,3 +9139,253 @@ 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_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. + + 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): + """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 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)]) + assert rc == 0 + out = capsys.readouterr().out + assert "archived 1 entry" 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") + 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). + + "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 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): + """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 + + 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 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 36674625..fc578f5e 100644 --- a/tests/test_deferredwork.py +++ b/tests/test_deferredwork.py @@ -7,13 +7,16 @@ 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, + gates, has_legacy, mark_done, mark_done_many, @@ -2350,3 +2353,817 @@ 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 "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 + 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 "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 + 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 < + + +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 "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. + 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 + + +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 + 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 + + +# ------------------------------------------- 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, 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) + archive_closed(path) + 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): + """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 + + +# --------------------------------------------- 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 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): + """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 — 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" + "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-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 + `_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_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 + 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 + + +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")