From 49e5b6424fe31d0529773cce380569148edfae64 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 14:48:34 -0700 Subject: [PATCH 1/2] fix(sweep): scope automation-mode validation rules to the triage universe (#824) The open_ids and partition bullets assumed every status: open entry, but a --only or --min-severity sweep validates against the selected set only, which reaches the session as --only . An agent following the stale bullets listed every open entry and burned a retry. --- CHANGELOG.md | 4 ++++ .../skills/bmad-loop-sweep/automation-mode.md | 12 ++++++++---- tests/test_sweep_skill_contract.py | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff43560f2..3c283fbbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,10 @@ breaking changes may land in a minor release. - Parse plugin manifests in `validate` (`plugins.manifests`) without importing plugin code; a malformed `plugin.toml` fails validate instead of engine start (#765). +- Scope the sweep skill's `open_ids` and partition validation rules to the session's + triage universe, so a `--only` or `--min-severity` triage no longer lists every open + entry and burns a retry (#824). + - Replace stale installed relay hooks when a project moves between Windows and POSIX. - Report stale or unverifiable Codex hook trust in `validate` and `probe-adapter` diff --git a/src/bmad_loop/data/skills/bmad-loop-sweep/automation-mode.md b/src/bmad_loop/data/skills/bmad-loop-sweep/automation-mode.md index 3d990eedc..7c9feab70 100644 --- a/src/bmad_loop/data/skills/bmad-loop-sweep/automation-mode.md +++ b/src/bmad_loop/data/skills/bmad-loop-sweep/automation-mode.md @@ -53,10 +53,14 @@ field-by-field, and will kill this session after your final turn. - Validation rules the orchestrator enforces (a violation fails the whole result and burns a retry): - - `open_ids` must list exactly the ledger's `status: open` entries — the - orchestrator parses the ledger itself and compares. - - Every open id appears in exactly ONE of already_resolved / bundles / - blocked / skip / decisions. No misses, no duplicates, no invented ids. + - `open_ids` must list exactly this session's triage universe: every + `status: open` entry in the ledger or, when the invocation carries + `--only DW-1,DW-2,...`, exactly those named ids — the orchestrator + parses the ledger itself, applies the same selection, and compares. + - Every id in the triage universe appears in exactly ONE of + already_resolved / bundles / blocked / skip / decisions. No misses, no + duplicates, no invented ids — an open entry outside the `--only` + selection counts as invented. - Bundle names: `^[a-z0-9][a-z0-9-]{1,39}\Z`, unique, non-empty `dw_ids`, non-empty `intent`. An otherwise-valid overlong bundle name or decision option `bundle_name` is truncated to 40 characters and journaled before diff --git a/tests/test_sweep_skill_contract.py b/tests/test_sweep_skill_contract.py index 9502bb225..98a0a9169 100644 --- a/tests/test_sweep_skill_contract.py +++ b/tests/test_sweep_skill_contract.py @@ -26,3 +26,22 @@ def test_sweep_skill_states_bundle_name_contract(skill_root): "to 40 characters and journaled before validation" ) in automation_md assert "post-truncation name collisions still fail validation" in automation_md + + +def test_sweep_automation_mode_scopes_validation_to_the_triage_universe(skill_root): + """ABLATION (#824): restore the old unconditional `open_ids` bullet and this fails + its first assertion.""" + automation_md = " ".join( + skill_root.joinpath("automation-mode.md").read_text(encoding="utf-8").split() + ) + rules = automation_md.split("Validation rules the orchestrator enforces", 1)[1].split( + "Write `already_resolved[].evidence`", 1 + )[0] + + assert "`open_ids` must list exactly this session's triage universe" in rules + assert "when the invocation carries `--only DW-1,DW-2,...`, exactly those named ids" in rules + assert "applies the same selection, and compares" in rules + assert "Every id in the triage universe appears in exactly ONE of" in rules + assert "an open entry outside the `--only` selection counts as invented" in rules + assert "`open_ids` must list exactly the ledger's `status: open` entries" not in automation_md + assert "Every open id appears in exactly ONE of" not in automation_md From 9e7dd14914410bd3534ce775c98e83970e193236 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 14:54:20 -0700 Subject: [PATCH 2/2] fix(status): report a sweep run's effective options (#815) sweep.json holds a sweep's nullable launch overrides and policy_snapshot holds [sweep] from policy.toml; the engine enforces override ?? snapshot, so the snapshot alone misreported an overridden cap. Text status now prints a `sweep options:` line with each of max_bundles/repeat/max_cycles at its effective value, labelled override or policy, plus any selector. sweep.json is read through the resume path's bounded, version- and digest-checked loader; a refusal degrades to "unverifiable" with the reason, and a legacy run without options says so. The override resolution moves into sweep.resolve_sweep_override, shared with SweepEngine.__init__ (no behavior change). --json is unchanged. --- CHANGELOG.md | 4 + docs/FEATURES.md | 2 +- src/bmad_loop/cli.py | 55 ++++++++++ src/bmad_loop/sweep.py | 24 ++++- tests/test_cli.py | 221 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 300 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c283fbbe..b54603a43 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,10 @@ breaking changes may land in a minor release. triage universe, so a `--only` or `--min-severity` triage no longer lists every open entry and burns a retry (#824). +- Show a sweep run's effective `max_bundles`, `repeat` and `max_cycles` in text `status`, + labelled launch override or policy snapshot, plus its selector; unreadable or + tampered `sweep.json` reports `unverifiable` (#815). + - Replace stale installed relay hooks when a project moves between Windows and POSIX. - Report stale or unverifiable Codex hook trust in `validate` and `probe-adapter` diff --git a/docs/FEATURES.md b/docs/FEATURES.md index a1f263f95..f34129209 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -259,7 +259,7 @@ Result` section. Every other spec keeps warn-and-continue, and the record says w - Frontmatter harvest bridge (BMAD-METHOD#2640/#2651; shipped 0.9.1, hardened #433): since BMAD-METHOD 6.10.1-next.33 the unattended primitive records defer-triaged review findings in its spec's frontmatter `deferred:` list (summary/evidence, optional location/severity) and writes nothing to the ledger. The orchestrator harvests them itself — post-session but _above_ the artifact gate, so before verification and before the attempt is accepted — into canonical `### DW-` entries, so `deferred-work.md` stays the sweep's sole read surface. Entries therefore appear even when the attempt goes on to fail verification: a fixable retry deliberately keeps them (the attribution reference moves onto the kept tree), and `_harvest_gate_exclude` stops the engine's own append from counting as the session's proof of work. Dedupe key is the fingerprinted `origin: spec-deferred ` plus `source_spec:`, scanned across entries of _every_ status, so a replay neither doubles an entry nor re-opens a closed one. Era-agnostic (the gate is the field's presence, never the skill name resolved on disk) and bounded to sessions bmad-loop drove to a success status — `in-review` with the follow-up review enabled, else `done`, plus an operator park; a plan-halt checkpoint keeps its notes for the implementation pass. A spec outside the orchestrator-owned roots is refused (`spec-deferrals-skipped-out-of-tree`) and an unreadable one retries the session rather than accepting it with findings silently dropped; `deferred:` items that will not parse are journaled (`spec-deferrals-malformed`) and filed as one low-severity entry naming the spec. A ledger whose bytes do not decode is routed at every one of the engine's own four `read_for_write` sites (DW-231) according to what the read was about to do: the observation reads — the proof-of-work digest, the pre-harvest snapshot, the defer snapshot and the two restores' compare-and-set probes — degrade to a typed answer nothing can write back or anchor a write on (the digest hashes the raw bytes, so "did the ledger change" stays exact; the snapshots stay unarmed; a restore skips and journals) and journal `ledger-read-degraded` naming the site; the two reads that precede a publish — this harvest's append and the isolated unit's carry into the main ledger — normally journal `ledger-read-refused`, raise an `ACTION REQUIRED` notice naming the ledger, and pause the run at `escalation`; a sweep's terminal post-merge harvest carry instead journals `sweep-bundle-close-refused` and pauses at `story-gate`, while its direct pre-terminal defer carry retains the engine route. Both routes leave the task's phase untouched, so `bmad-loop resume` after the hand repair retries the write — resume recovery replays the recorded session result where one exists (the dev and review legs) and otherwise re-drives the leg (the unlatched review-timeout salvage and fix legs) — rather than, as `_escalate` would, demanding a `bmad-loop resolve` session and a clean rebuild over a fault that is not the story's. Bare, the first of those reads ended a story run as `run-crash` with the completed session's work on disk. The route also covers the window INSIDE each write, for decode faults (DW-259) and OS metadata/text-read faults (DW-279): every `deferredwork` mutator takes its own locked `read_for_write` — after the routed pre-read at the harvest and the harvest carry, after an observation snapshot at the commit-boundary close, and with no pre-read at all at the review-timeout salvage refile (`deferredwork.append_entry`) and the isolated close carry — so a `LedgerReadError` raised from the mutator call itself — the harvest's seen-again mark and append, the commit-boundary `closes_deferred:` close, the salvage refile, the isolated unit's harvest carry and close carry — pauses through its owning repair route under a site name ending in `-locked` (`spec-deferrals-harvest-mark-locked`, `spec-deferrals-harvest-append-locked`, `story-close-locked`, `review-timeout-salvage-refile-locked`, `harvest-carry-append-locked`, `story-close-carry-locked`); the locked read fires ahead of every write, so a pause there proves the mutator wrote nothing, which is what lets the commit-boundary close disarm its rollback first rather than journal a `deferred-close-rollback-failed` against bytes it cannot read. The notice names both kinds of write (findings to file, a declared close to record); `bmad-loop resume` re-drives the close through the COMMITTING arm, and a pending salvage refile through its persisted retry latch (DW-278): resume verifies the preserved product again, refiles the outstanding follow-up, and commits without new dev/review sessions or additional attempt/cycle charges under either rollback policy; ordinary commit gates still run, including any configured `pre_commit_gate` workflow sessions. An unrepaired ledger pauses again with the latch retained; a failed salvage verification follows the usual retry/exhaust routing. Successful refile records the publication while retaining recovery authority through notification and commit gates — the latch is set at every salvage's handoff save, the first fault-free one included, not only after a repair pause, so a host lost between that save and the commit replays the salvage rather than restarting it; the durable COMMITTING transition clears the latch. Legacy and unlatched timeouts keep the baseline restart or manual recovery behavior governed by `scm.rollback_on_failure`. The replay is the story engine's: a sweep bundle's recovery (`_recover_inflight_bundle`) has no session-replay arm at all, so a latched bundle restarts as every other post-session bundle does and its restart clears the latch, so the abandoned product's salvage cannot force a review on the replacement attempt. `LedgerReadFault(LedgerReadError)` wraps OS metadata/text-read failures with the original `OSError` as `__cause__`, so the locked-read catches cover them without catching lock/write failures. Pre-lock presence probes, lock acquisition and writes keep their raw `OSError` behavior. A read the OS refuses (EACCES, EIO, a symlink cycle) is routed the same way at the same four sites (DW-258): the observation reads degrade to a typed answer that carries NO digest — nothing is read from a ledger the OS refused — so the proof-of-work digest becomes an `` sentinel and the attribution compare treats a sentinel on either side as UNKNOWN, which is never credited: the ledger path stays excluded from proof of work, so the engine's own harvest append after the hand repair cannot pass a session that wrote nothing (a session whose only work was a ledger edit over a refused baseline is rolled back and retried over a readable one); the publish reads pause with the same `ACTION REQUIRED` notice, whose repair sentence now names both repairs (valid UTF-8; the path's permissions or storage). Where no publish read is reached — a spec with no findings — the story completes over the refused ledger, and a declared `closes_deferred:` then journals `deferred-close-ledger-unavailable` and sends a best-effort notice through the configured notification channels (including ATTENTION when file notifications are enabled), naming the story, every unapplied declared ID, and the fault (DW-277). The snapshot outage writes nothing to the ledger and does not pause or crash the story. Pre-rename primitives and the attended `bmad-build` still append flat `- source_spec:` blocks directly, which `sweep --migrate` normalizes. - Story-declared closure (`closes_deferred: [DW-5, DW-6]`, human-authored on a `stories.yaml` entry or in a story spec's frontmatter — the two are unioned): when the story commits, each declared entry flips to `status: done ` + `resolution: resolved by story ` — the annotation a sweep bundle writes — so the ledger stops being one-way. Written at the commit boundary, so an in-repo ledger carries the annotation in the story's own commit and a story that fails, is rejected by review, or escalates closes nothing. Declared, never inferred from a diff; re-read at the commit so a declaration edited after implementation still counts; idempotent across a resume; an unknown id, an unreadable entry status or a non-list declaration in a story spec is journaled, never fatal, and `bmad-loop validate` warns about all of them before the run starts. (A non-list `closes_deferred` in `stories.yaml` is different: the manifest is a schema the parser owns, so it is refused outright, before the run.) An artifact dir outside the repo cannot be committed — the annotation is written anyway and journaled (`deferred-close-external-ledger`). If the advisory ledger snapshot cannot be read, the declared closes remain unapplied and open entries stay open; the outage is journaled and notified as above. Restore ledger readability (valid UTF-8 and accessible permissions or storage), then run `bmad-loop sweep`: the next sweep re-triages those IDs against the actual code and can close verified resolutions as `already_resolved`, with the completed story's commit serving as evidence. - Hard gates (`gate: 3-2, 3-3` on an entry): until the entry lands, `bmad-loop validate` fails (`deferred.hard-gate`) for every actionable story a token matches and `run` pauses (`story-gate`) rather than dispatch one — a token gates a key it equals or prefixes at a key boundary (`-`, or a split-story suffix), so `3-2` covers `3-2-invite-link`, the stories-mode id `3-2` and both halves of a `3-2a`/`3-2b` split, but never `3-20-later`. Only an explicit `status: done` retires a gate; an unreadable status (`opne`, or no status line) still gates. The dispatch pause precedes the story's own run record, so closing the entry and resuming runs it; sweeps are exempt, since a sweep is what closes the gating entry. The only deferred check that gates rather than advises; cleared by closing the entry or dropping the token. A warning (`deferred.hard-gate-unstructured`) covers the four gates nothing can enforce: a token that cannot name a story key (`3-2 3-3`, or an unmatchable `3.2` — `.`/`_` are legal inside a sprint slug), an empty `gate:` line, a `gate:` not lowercase at the start of a line, and prose declaring `HARD GATE:` (matched mid-line, since `reason:` prose wraps — but not straight after a quote, so a citation stays silent) on an entry with no `gate:` line. Migration is checked rather than merely instructed: a `sweep --migrate` rewrite that drops a `gate:` token a pre-existing entry declared is refused, restoring the original ledger (#519); an added token is accepted, since over-blocking fails loudly and in the safe direction. A ledger already carrying duplicate `DW-` ids is refused before any rewrite is dispatched, pausing with the ids to renumber: no rewrite of it can both preserve the entries and produce a valid ledger, so the only one that would pass collapses the pair and drops a twin's gate. -- `bmad-loop sweep` triages every selected open entry against the actual code (all open entries by default; ledger statuses treated as unreliable) → partition: already-resolved (auto-closed with evidence) / bundles / blocked / skip / decisions. `--only DW-1,DW-3` requires every named id to exist and be open on the initial post-migration ledger; repeats and resumes intersect that fixed scope with entries still open. `--min-severity low|medium|high|critical` is re-evaluated each cycle and includes the named level and higher; missing or unknown severity is excluded and explicitly reported. Selection precedes triage, bundle formation, and `--max-bundles`; dry-run uses the same rules; both exclusion classes are journaled; selectors persist in `sweep.json`, are mutually exclusive, and cannot combine with archive mode. On a mixed legacy ledger, named dry-run selection projects provisional ids by continuing from the highest canonical id in manifest/file order, labels selected and excluded projections, and rejects projected done/unknown ids without spawning migration; semantic duplicate merging may compact the final mapping, so the real run revalidates against the actual post-migration ids. Before the first-attempt validation, an otherwise-valid lowercase ASCII/kebab bundle name or decision-option `bundle_name` over 40 characters is truncated to 40 and journaled; malformed shapes and post-truncation collisions still fail through the existing retry path. +- `bmad-loop sweep` triages every selected open entry against the actual code (all open entries by default; ledger statuses treated as unreliable) → partition: already-resolved (auto-closed with evidence) / bundles / blocked / skip / decisions. `--only DW-1,DW-3` requires every named id to exist and be open on the initial post-migration ledger; repeats and resumes intersect that fixed scope with entries still open. `--min-severity low|medium|high|critical` is re-evaluated each cycle and includes the named level and higher; missing or unknown severity is excluded and explicitly reported. Selection precedes triage, bundle formation, and `--max-bundles`; dry-run uses the same rules; both exclusion classes are journaled; selectors persist in `sweep.json`, are mutually exclusive, and cannot combine with archive mode. On a mixed legacy ledger, named dry-run selection projects provisional ids by continuing from the highest canonical id in manifest/file order, labels selected and excluded projections, and rejects projected done/unknown ids without spawning migration; semantic duplicate merging may compact the final mapping, so the real run revalidates against the actual post-migration ids. Before the first-attempt validation, an otherwise-valid lowercase ASCII/kebab bundle name or decision-option `bundle_name` over 40 characters is truncated to 40 and journaled; malformed shapes and post-truncation collisions still fail through the existing retry path. Text `bmad-loop status` on a sweep run prints a `sweep options:` line giving `max_bundles`, `repeat` and `max_cycles` at their effective values — the run's `sweep.json` override, else its `policy_snapshot` (never live policy.toml) — labelled `override` or `policy`, plus any selector; options that fail the resume loader's bound or digest checks read `unverifiable` with the reason, and `--json` is unchanged (#815). - Bundles run the full pipeline (dev `--dw-bundle` → review → verify → commit); the review gate checks every bundle entry is `status: done`. - Artifact-only bundles (DW-273): a bundle whose only permitted deliverable lives under a gitignored `implementation_artifacts` dir (a spec-only erratum) can never satisfy the ordinary dev proof-of-work probe, which measures tracked and untracked-not-ignored paths only — and a bundle has no `awaiting-operator` park to fall back on. The bundle dev session may instead assert it by appending an `Artifact only: true` line inside its `## Auto Run Result` marker, beside `Status:`. Accepted spellings: `Artifact only`, `artifact_only`, `Artifact-only` — a run of at least one space, underscore or hyphen between the two words (the fused `Artifactonly` is no spelling and asserts nothing), case-insensitive, in the same bulleted or bolded label/value shapes as `Status:` (`**Artifact only:** **true**` and `- **Artifact only: true**` both assert) — with the value `true` alone on the line (anchored to end of line: `Artifact only: true for the ledger, false for code` is prose, not an assertion) and, like `Status:`, on the SAME line as its label (`Artifact only:` with `true` on the next line is a bare label, not an assertion; any horizontal whitespace, NBSP included, is fine); a line inside a fenced block within the marker is documentation and mints nothing. The synthesized result carries `artifact_only: true` only when that line sits in the current session's last genuine, non-fenced marker with no orchestrator repair note — the same four-part shape and the same session-authorship proof (`park_marker_session_authored`) that mint `park_asserted`; frontmatter never mints it, and a loose `"true"`/`1` is no assertion. Gate rule: on the bundle path only (`verify_dev_bundle` — a story result asserting it still owes the ordinary diff), after the ordinary probe has positively answered "nothing changed", a strict `artifact_only: true` makes the gate run `git status --ignored --porcelain --untracked-files=all` scoped to the configured `implementation_artifacts` dir and count only its IGNORED (`!!`) records — the tracked and untracked-not-ignored ones are what the ordinary probe already measured, so under the `bmad-loop init` default layout (where `_bmad-output/` is not gitignored) a bundle's own spec flip or newly written spec is no receipt — and keep only the entries THIS ATTEMPT created or changed: ignored paths carry no git baseline, so the sweep engine fingerprints (`lstat` mtime and size) every ignored entry under the dir at each genuinely new attempt's start (`StoryTask.baseline_artifacts`, stamped beside `baseline_commit` and persisted in state.json; a fixable repair keeps the chain's snapshot, a rolled-back retry takes a fresh one since rollback never removes ignored files), and an entry absent from that snapshot or carrying a different fingerprint now is the attempt's, while one unmeasurable at either end is not. A positive owned count is accepted as proof of work; residue that all predates the attempt (`lists N ignored entries, none created or changed by this attempt`), an empty listing, a task with no snapshot (a pre-upgrade run, or a capture that degraded to `bundle-artifact-baseline-unavailable` on a git fault — the attempt is still driven, only the relaxation is withheld), an artifacts dir outside the code tree (or equal to it), or a git refusal keep the ordinary retry with the refusal's cause appended to the verbatim `no changes in worktree since baseline commit` reason, and a git fault at the gate escalates like the ordinary probe's. An accepted receipt is journaled as `bundle-artifact-only-accepted` (`story_key`, `attempt`, `dw_ids`, `count` — the number of owned entries). Under `scm.isolation = "worktree"` an in-tree artifacts dir is rebased into the unit worktree, so the receipt measures the worktree's copy and the success teardown removes that worktree — an artifact-only unit lands no tracked change, and the ledger carry re-applies the close but copies no file. What survives is what publication carries (DW-283, below): the accepted ignored spec and the exact files its `artifact_deliverables` list names are published to the main checkout ahead of the teardown; undeclared ignored residue is not copied, and the bundle prompt says so (DW-284). An artifacts dir OUTSIDE the code tree is not rebased and survives the teardown, but the receipt is refused for it regardless of isolation, since git lists nothing outside the repo (the "outside the code tree" refusal above). The limit: ownership is read off fingerprints, not content, so a rewrite that lands byte-identical with a preserved mtime is invisible to it, as it is to the ordinary probe; the assertion selects the receipt, the snapshot is what makes it proof. Nothing else relaxes — workflow tag, expected status, baseline match, dw_ids cross-check, your `[verify]` commands and the review gate's every-id-`done` check all still apply. - Artifact-only bundle guidance (DW-284): initial, restored-review, and repair bundle prompts advertise this conditional receipt: assert it only for the current session's actual qualifying deliverables, never for ordinary changes, other nonqualifying deliverables, or old artifacts alone. Sweep triage describes qualifying deliverables and their location in the existing bundle `intent` field; the executing session owns the assertion, with no added triage receipt field. Successful isolated integration publishes explicit ignored deliverables through the separate publication contract in Git worktree isolation; accepting the receipt alone does not publish files. diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index a5cec190d..d753fae3b 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -98,9 +98,11 @@ from .sweep import ( DW_ID_RE, SEVERITY_ORDER, + SWEEP_OVERRIDE_KEYS, SweepEngine, decimal_digits_key, increment_decimal_digits, + resolve_sweep_override, select_entries, ) @@ -4375,6 +4377,57 @@ def cmd_decisions(args: argparse.Namespace) -> int: return 0 +def _sweep_options_line(run_dir: Path, state: RunState) -> str: + """The text-status line naming a sweep run's effective options (#815). + + `policy_snapshot` alone reads as the enforced cap, but a launch override in + `sweep.json` wins over it (`resolve_sweep_override`, as `SweepEngine.__init__` + applies it). The policy half comes from the run's snapshot, never live + policy.toml: the engine loads policy once, and resume re-stamps the snapshot + to the policy it reloads. `sweep.json` is read the way resume reads it — + bounded, version-checked and digest-bound — but status only observes, so a + refusal degrades to "unverifiable" rather than failing the command.""" + version = state.sweep_options_version + try: + runsetup.validate_sweep_options_version(version) + current = version == runsetup.SWEEP_OPTIONS_VERSION + options = runsetup.load_sweep_resume_options( + run_dir, + required=version >= runsetup.SWEEP_OPTIONS_VERSION, + expected_digest=state.sweep_options_digest if current else None, + ) + runsetup.validate_sweep_options_binding(version, state.sweep_options_digest, options) + except runsetup.SweepOptionsError as exc: + return f"sweep options: unverifiable — {exc}" + if options.digest is None: + # The legacy loader's tolerant empty shape: no readable sweep.json, so the + # launch overrides were never recorded (resume would run on policy alone). + return "sweep options: unknown — legacy run with no readable sweep.json" + raw_policy = state.policy_snapshot.get("sweep") + snapshot: dict[str, Any] = raw_policy if isinstance(raw_policy, dict) else {} + parts: list[str] = [] + for key in SWEEP_OVERRIDE_KEYS: + override = options.values.get(key) + from_policy = snapshot.get(key) + if override is None and from_policy is None: + parts.append(f"{key} unknown (no override; not in the policy snapshot)") + continue + value = json.dumps(resolve_sweep_override(override, from_policy)) + if override is None: + source = "policy" + elif from_policy is None: + source = "override" + else: + source = f"override; policy {json.dumps(from_policy)}" + parts.append(f"{key} {value} ({source})") + if options.only_ids is not None: + parts.append(f"only {','.join(options.only_ids)}") + if options.min_severity is not None: + parts.append(f"min_severity {options.min_severity}") + legacy = " [legacy options format]" if version == 0 else "" + return f"sweep options: {', '.join(parts)}{legacy}" + + def cmd_status(args: argparse.Namespace) -> int: project = _project(args) if args.run_id: @@ -4414,6 +4467,8 @@ def cmd_status(args: argparse.Namespace) -> int: print("status: in progress — graceful stop pending (will stop after the current item)") else: print("status: in progress (or interrupted)") + if state.run_type == "sweep": + print(_sweep_options_line(run_dir, state)) if state.sweeps_refused: detail = ", ".join(f"{trigger} ({why})" for trigger, why in state.sweeps_refused.items()) print(f"auto-sweep not run: {detail} — deferred work is untouched") diff --git a/src/bmad_loop/sweep.py b/src/bmad_loop/sweep.py index e88b5609d..493d76dbd 100644 --- a/src/bmad_loop/sweep.py +++ b/src/bmad_loop/sweep.py @@ -18,7 +18,7 @@ import unicodedata from dataclasses import dataclass, replace from pathlib import Path -from typing import Any, Callable, Iterable, Literal, NoReturn, assert_never +from typing import Any, Callable, Iterable, Literal, NoReturn, TypeVar, assert_never from . import deferredwork, gates, verify from .engine import ( @@ -1300,6 +1300,22 @@ def _rearm_generation(task: StoryTask) -> None: task.generation += 1 +# The launch overrides a sweep run resolves against `[sweep]` policy. `sweep.json` +# persists each as nullable (None = "not given"); `SweepEngine.__init__` and +# `bmad-loop status` both resolve through `resolve_sweep_override`. +SWEEP_OVERRIDE_KEYS = ("max_bundles", "repeat", "max_cycles") + +_T = TypeVar("_T") + + +def resolve_sweep_override(override: _T | None, policy_value: _T) -> _T: + """The value a sweep run enforces for one of `SWEEP_OVERRIDE_KEYS`: its own + override when one was given, else the policy's. Tested with `is not None`, + never truthiness — an explicit `repeat=False` or `max_bundles=0` is an + override, not an absence.""" + return override if override is not None else policy_value + + class SweepEngine(Engine): """Engine variant whose loop processes the deferred-work ledger instead of sprint-status. Bundles reuse the inherited story pipeline through the @@ -1329,9 +1345,9 @@ def __init__( self.adapters["triage"].journal = self.journal self.prompting = prompting self.decisions_only = decisions_only - self.max_bundles = max_bundles if max_bundles is not None else self.policy.sweep.max_bundles - self.repeat = repeat if repeat is not None else self.policy.sweep.repeat - self.max_cycles = max_cycles if max_cycles is not None else self.policy.sweep.max_cycles + self.max_bundles = resolve_sweep_override(max_bundles, self.policy.sweep.max_bundles) + self.repeat = resolve_sweep_override(repeat, self.policy.sweep.repeat) + self.max_cycles = resolve_sweep_override(max_cycles, self.policy.sweep.max_cycles) self.only_ids = only_ids self.min_severity = min_severity self._selection_started = self.state.sweep_cycle > 1 or any( diff --git a/tests/test_cli.py b/tests/test_cli.py index e5422c0d5..164ba3758 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -47,7 +47,7 @@ write_sprint, ) -from bmad_loop import cli, deferredwork, envvars, platform_util +from bmad_loop import bmadconfig, cli, deferredwork, envvars, platform_util from bmad_loop import policy as policy_mod from bmad_loop import probe as probe_mod from bmad_loop import runs, runsetup, verify @@ -1700,6 +1700,225 @@ def test_status_stories_mode_bad_manifest_is_soft(project, capsys): assert "no stories.yaml found" in capsys.readouterr().out +# ------------------------------------------- status: a sweep's effective options + +# `sweep.json` holds a sweep run's nullable launch overrides; `policy_snapshot` +# holds `[sweep]` from policy.toml. The engine enforces `override ?? snapshot`, so +# the snapshot alone misreports an overridden cap (#815). These rows compose the +# run through the same `runsetup.compose_sweep` `cmd_sweep` uses. + +SWEEP_STATUS_POLICY = "[sweep]\nmax_bundles = 5\nrepeat = true\nmax_cycles = 3\n" + + +class _ComposedSweepEngine: + def __init__(self, *args, **kwargs): + pass + + +def _compose_sweep_run(project, policy_text=SWEEP_STATUS_POLICY, **overrides): + """A sweep run composed from the sandbox's real policy.toml. The pid file + `compose_sweep` publishes names this test process, so it is removed: the run + reads as interrupted, which is what `status` and `resume` expect of it.""" + _write_policy(project.project, policy_text) + options = {"max_bundles": None, "repeat": None, "max_cycles": None} | overrides + composed = runsetup.compose_sweep( + project=project.project, + paths=bmadconfig.ProjectPaths( + project=project.project, + implementation_artifacts=project.project / "impl", + planning_artifacts=project.project / "plan", + ), + policy=policy_mod.load(cli._policy_path(project.project)), + run_id="20260101-000000-sw01", + prompting=False, + decisions_only=False, + trigger="cli", + make_adapters=lambda *a, **k: {role: None for role in runsetup.ROLES}, + sweep_engine_cls=_ComposedSweepEngine, + trusted_config_digest="deadbeef", + **options, + ) + (composed.run_dir / runs.PID_FILE).unlink() + return composed.run_dir + + +def _status_sweep_options(project, capsys) -> str: + assert cli.main(["status", "--project", str(project.project)]) == 0 + out, err = capsys.readouterr() + assert "Traceback" not in out + err + (line,) = [ln for ln in out.splitlines() if ln.startswith("sweep options:")] + return line + + +def test_status_sweep_shows_an_override_as_effective(project, capsys): + _compose_sweep_run(project, max_bundles=15) + line = _status_sweep_options(project, capsys) + assert "max_bundles 15 (override; policy 5)" in line + + +def test_status_sweep_omitted_override_shows_the_snapshot_value(project, capsys): + _compose_sweep_run(project, max_bundles=15) + line = _status_sweep_options(project, capsys) + assert "repeat true (policy)" in line + assert "max_cycles 3 (policy)" in line + + +def test_status_sweep_explicit_false_override_is_an_override(project, capsys): + """ABLATION: resolve the override by truthiness and this fails — `repeat=False` + against policy `repeat = true` would read as the policy's `true`.""" + _compose_sweep_run(project, repeat=False, max_cycles=0) + line = _status_sweep_options(project, capsys) + assert "repeat false (override; policy true)" in line + assert "max_cycles 0 (override; policy 3)" in line + + +def test_status_sweep_reads_the_snapshot_not_live_policy(project, capsys): + """The engine loaded policy once, at launch; a later policy.toml edit does not + reach it, so it must not reach status either. + + ABLATION: resolve against live policy.toml instead of `policy_snapshot` and + this fails on the edited `9`.""" + _compose_sweep_run(project) + _write_policy(project.project, "[sweep]\nmax_bundles = 9\nrepeat = false\nmax_cycles = 3\n") + line = _status_sweep_options(project, capsys) + assert "max_bundles 5 (policy)" in line + assert "repeat true (policy)" in line + + +def test_status_sweep_after_resume_reports_the_restamped_policy(project, monkeypatch, capsys): + """Resume reloads policy.toml, hands it to the rebuilt SweepEngine and re-stamps + `policy_snapshot` to match (#189) — while `sweep.json` is left byte-identical, + so the launch override survives and its digest still binds.""" + from conftest import install_base_skills + + from bmad_loop.journal import load_state, save_state + + install_bmad_config(project) + install_base_skills(project) + write_sprint(project, {}) + run_dir = _compose_sweep_run(project, max_bundles=15) + state = load_state(run_dir) + state.paused_reason, state.paused_stage = "escalation", "escalation" + save_state(run_dir, state) + _write_policy(project.project, "[sweep]\nmax_bundles = 6\nrepeat = true\nmax_cycles = 4\n") + monkeypatch.setattr(runs, "kill_session", lambda rid: None) + monkeypatch.setattr(runs, "write_pid", lambda _run_dir: None) + monkeypatch.setattr(cli, "_make_adapters", lambda *a, **k: {r: None for r in cli.ROLES}) + monkeypatch.setattr(cli, "SweepEngine", _StubEngine) + + assert cli._resume_paused_run(project.project, run_dir) == 0 + capsys.readouterr() + + line = _status_sweep_options(project, capsys) + assert "max_bundles 15 (override; policy 6)" in line + assert "max_cycles 4 (policy)" in line + + +def test_status_sweep_shows_the_selector(project, capsys): + _compose_sweep_run(project, only_ids=("DW-3", "DW-1")) + assert _status_sweep_options(project, capsys).endswith(", only DW-3,DW-1") + + +def test_status_sweep_shows_the_severity_selector(project, capsys): + _compose_sweep_run(project, min_severity="high") + assert _status_sweep_options(project, capsys).endswith(", min_severity high") + + +def _mark_legacy(run_dir) -> None: + from bmad_loop.journal import load_state, save_state + + state = load_state(run_dir) + state.sweep_options_version, state.sweep_options_digest = 0, "" + save_state(run_dir, state) + + +def test_status_sweep_legacy_run_reports_its_recorded_options(project, capsys): + """A pre-marker run resumes on its sweep.json limits but never its selectors + (`load_sweep_resume_options(required=False)`), so status reports the same.""" + run_dir = _compose_sweep_run(project, max_bundles=15, only_ids=("DW-1",)) + _mark_legacy(run_dir) + line = _status_sweep_options(project, capsys) + assert "max_bundles 15 (override; policy 5)" in line + assert "only" not in line + assert line.endswith("[legacy options format]") + + +def test_status_sweep_legacy_run_without_options_says_so(project, capsys): + run_dir = _compose_sweep_run(project, max_bundles=15) + _mark_legacy(run_dir) + (run_dir / "sweep.json").unlink() + line = _status_sweep_options(project, capsys) + assert line == "sweep options: unknown — legacy run with no readable sweep.json" + + +@pytest.mark.parametrize("fault", ["corrupt", "digest-mismatch", "missing"]) +def test_status_sweep_unverifiable_options_degrade(project, capsys, fault): + """Status observes; resume would refuse this run, status reports why and still + exits 0 — and prints no policy value, which would read as the enforced one.""" + from bmad_loop.journal import load_state, save_state + + run_dir = _compose_sweep_run(project, max_bundles=15) + options = run_dir / "sweep.json" + if fault == "corrupt": + options.write_bytes(b"{not json") + # Re-bind the digest so the parse, not the binding, is what refuses. + state = load_state(run_dir) + state.sweep_options_digest = hashlib.sha256(options.read_bytes()).hexdigest() + save_state(run_dir, state) + reason = "not valid JSON" + elif fault == "digest-mismatch": + # The widening swap the digest exists to catch: valid, but not launch's. + options.write_text(json.dumps({"only": None, "min_severity": None}), encoding="utf-8") + reason = "no longer matches the options bound at launch" + else: + options.unlink() + reason = "sweep.json is missing" + + line = _status_sweep_options(project, capsys) + assert line.startswith("sweep options: unverifiable — ") + assert reason in line + assert "policy" not in line + + +def test_status_json_for_a_sweep_run_is_unchanged(project, capsys): + """#815 is text-only: `status --json` stays the pure `status_document` projection + of state.json, with its keys exactly as before — no sweep-options key.""" + from bmad_loop.documents import status_document + from bmad_loop.journal import load_state + + run_dir = _compose_sweep_run(project, max_bundles=15, only_ids=("DW-1",)) + assert cli.main(["status", "--json", "--project", str(project.project)]) == 0 + document = json.loads(capsys.readouterr().out) + assert document == status_document(load_state(run_dir)) + assert set(document) == { + "schema_version", + "run_id", + "run_type", + "source", + "started_at", + "status", + "finished", + "stopped", + "graceful_stop_pending", + "crashed", + "crash_error", + "paused_stage", + "paused_reason", + "paused_story_key", + "cache_read_weight", + "tokens", + "adapters", + "sweeps_refused", + "tasks", + } + + +def test_status_story_run_prints_no_sweep_options(project, capsys): + _make_run_with_tokens(project, {}, weight=0.1) + assert cli.main(["status", "--project", str(project.project)]) == 0 + assert "sweep options" not in capsys.readouterr().out + + def test_emit_document_verifies_without_altering_the_bytes(capsys): """The helper half the --json commands write through: it must validate, and it must emit the ORIGINAL string — diagnose's leak self-check verified those