Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,34 @@ whose seams had diverged enough that several ports needed a different fix, and t

### Fixed

- **`resume`'s config-change baseline moves out of the agent-writable tree (#498).** The host-exec
baseline `resume` compares against — verify commands, launch binary/args/env, plugin allowlist
— round-tripped through `state.json`, inside the very tree the digest exists to police, so the
session that rewrote `policy.toml` could blank the field in the same breath and the warning
never fired. It now lives beside the events channel in the run's out-of-tree state dir
(`<state root>/<project>/<run-id>/config-digest`, #494), and is collected by the same
`delete`/`archive`/`clean` lifecycle. The auto-sweep gate is unchanged — it always compared
against an in-memory baseline no session can reach.

`state.json` keeps a second copy, and `resume` consults it **only** when the run has no file
out of tree — so rewriting it silences nothing while that file is in reach. Two cases need it,
and in both the file is honestly absent rather than tampered away: a run paused under an older
version, whose baseline is there and nowhere else (this resume migrates it); and a run whose
project has been moved or renamed. The state root is keyed by the project's _resolved path_, so
a rename keys the run somewhere new and orphans the subtree holding its digest — the copy in
the run directory travels with the run, and the warning survives the move. A project that
later returns to a path it ran under before is the one case this does not get right: the old
subtree is still there, its pin is preferred, and one resume answers off it before re-stamping
and healing (#572).

**What this does and does not buy.** It closes the _incidental_ path — nothing a session does
in the ordinary course of rewriting project files can blank the pin any more, because the pin
is no longer a project file. It is **not** a boundary against a deliberate session: sessions
launch with permission bypass by default (that is what an unattended loop is), and are handed
`BMAD_LOOP_EVENTS_DIR`, whose parent is the state dir — so a session that goes looking can
still delete or truncate the baseline and silence the warning. Closing that needs privilege
separation on the state dir, not a better hiding place; tracked in #571.

- **Worktree runs no longer stall when a seeded hook config carries the main repo's relay
(#352).** `.claude/settings.json` is both a seeded file and the hook `config_path`, so it
arrived in the worktree still naming the main repo's `$CLAUDE_PROJECT_DIR`-relative relay
Expand Down
2 changes: 1 addition & 1 deletion docs/FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se
### Disk reclamation (`[cleanup]`)

- `bmad-loop clean` reclaims **disk** (distinct from `cleanup`, which is only tmux). It tears down git worktrees a mid-flight stop left mounted — the main accumulation source: each carries a real Unity `Library/` (incl. the MCP-server build), which `git worktree remove` cannot reach once the engine was killed before teardown. It then trims the heavy `worktrees/` tree from runs kept for history (the run still lists in the dashboard — discovery reads `state.json`, not the worktree), and archives or deletes runs past the retention window.
- It also collects the **out-of-tree** half of a run. Removing a run dir no longer removes everything the run owns (#494), so `delete`/`archive`/`clean` remove the run's control-plane dir under the state root too, and `clean` additionally sweeps this project's orphans there — subtrees whose run dir is gone, from a hand-removed run or a delete that predates this. The sweep keys on the run directory _existing_, not on its `state.json` parsing, so a corrupt run an operator is trying to recover keeps its control plane; a trimmed run keeps its own for the same reason (it is still resumable). Their bytes are not in the reclaim estimate — a state dir holds consumed event files and little else. Known limit: the state root is keyed by the project's resolved path, so a project that is deleted, moved or renamed leaves its old subtree unsweepable — after a move the project keys somewhere new, and no project can name the old key.
- It also collects the **out-of-tree** half of a run. Removing a run dir no longer removes everything the run owns (#494), so `delete`/`archive`/`clean` remove the run's control-plane dir under the state root too, and `clean` additionally sweeps this project's orphans there — subtrees whose run dir is gone, from a hand-removed run or a delete that predates this. The sweep keys on the run directory _existing_, not on its `state.json` parsing, so a corrupt run an operator is trying to recover keeps its control plane; a trimmed run keeps its own for the same reason (it is still resumable). Their bytes are not in the reclaim estimate — a state dir holds consumed event files, the run's `config-digest` (#498), and little else. Known limit: the state root is keyed by the project's resolved path, so a project that is deleted, moved or renamed leaves its old subtree unsweepable — after a move the project keys somewhere new, and no project can name the old key. A move does not cost the run its config-change baseline, though: `state.json` carries a second copy that travels with the run directory, and `resume` falls back to it exactly when the out-of-tree file is out of reach (#498).
- Safe by construction: only **finished or stopped** runs are touched; running, unknown-host, paused and interrupted (resumable) runs are never reclaimed. `--keep <run-id>` protects a specific run (e.g. a finished one whose Editor is still live), `--dry-run` previews, `--retain N`/`--hard` tune the window and archive-vs-delete.
- `--json` emits a stable machine-readable document per the [contract below](#machine-readable-output---json) (schema-versioned; the effective retention policy, `freed_bytes` as a raw integer, and the paths and run ids under `worktrees`/`trimmed`/`archived`/`deleted`/`protected`, and `state_dirs_swept` as a count) instead of the text. Plan and outcome share one schema, with `dry_run` saying which one you are holding, so a script can pre-flight a reclaim and compare it against what happened — though values are each invocation's own sample, not a promise the two agree. It names every item the text only counts or renders, and the unverifiable-pid warning text mode writes to stderr becomes `unverifiable_pid` in the document, leaving stderr empty.
- Prevention is automatic: every `run`/`sweep` start reconciles worktrees leaked by a prior **finished** run (`[cleanup] auto_clean_on_finish`), and the Unity plugin's `post_run` hook removes the IvanMurzak MCP server's downloaded `/tmp/<company>/<product>/*.zip` and truncates its unbounded editor log (`[cleanup] clean_tmp`). For recurring housekeeping of stopped runs, schedule `bmad-loop clean`.
Expand Down
49 changes: 38 additions & 11 deletions src/bmad_loop/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -2127,18 +2127,28 @@ def _resume_paused_run(project: Path, run_dir: Path) -> int:
# Deliberately no --allow-config-change flag: a refuse-by-default gate here
# would break resume-to-fix-a-setting, which is a supported workflow.
#
# ADVISORY, not a control, and the asymmetry with the gate is deliberate. The
# ADVISORY, not a control, and the asymmetry with the gate is deliberate: the
# auto-sweep refusal in `_sweep_factory` baselines off a digest threaded in
# MEMORY from launch (see cmd_run/the compose_resume call below), which a
# session cannot reach. This comparison baselines off `state.trusted_config_digest`,
# which round-trips through <project>/.bmad-loop/runs/<id>/state.json — inside
# the same agent-writable tree the digest exists to police. A session that
# rewrites policy.toml can blank or re-stamp that field and silence the warning
# (blank suffices: the bool() guard reads "" as no prior pin). It catches the
# incidental change, not a deliberate one. Tracked in #498.
security_config_changed = bool(state.trusted_config_digest) and (
new_digest != state.trusted_config_digest
)
# MEMORY from launch (see cmd_run/the compose_resume call below), which nothing
# on disk can reach at all. This one has to survive the pause, so it is read
# back — and since #498 from the run's out-of-tree state dir rather than from
# <project>/.bmad-loop/runs/<id>/state.json, which is inside the very tree the
# digest exists to police. It used to be readable there: a session that
# rewrote policy.toml could blank the field in the same breath and silence this
# warning (blank sufficed — the bool() guard reads "" as no prior pin), so the
# advisory caught the incidental change and never the deliberate one.
#
# The legacy fallback, for one release: a run PAUSED under the old code has no
# file in the state root, and its baseline is in state.json and nowhere else.
# `None` from the reader means exactly that "no file" — distinct from an empty
# file, which is a real answer of "no baseline" and must NOT reopen the tree —
# so the fallback fires only for those runs, and the re-stamp below migrates
# them on this very resume. Empty either way keeps its meaning: no prior pin,
# no warning.
pinned = runs.read_trusted_config_digest(project, run_dir.name)
if pinned is None:
pinned = state.trusted_config_digest
Comment on lines +2148 to +2150

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject stale pins when a project returns to an old path

If a paused project is moved from path A to B, resumed there, and later moved back to A, the old state-root subtree for A still exists (the changed documentation explicitly says it is unsweepable). This lookup therefore prefers A's stale digest over the newer secondary digest re-stamped during the resume at B. An unchanged configuration then produces a false warning; worse, changing the configuration back to A's stale value suppresses the warning entirely. Associate the external pin with its current generation/location or retire stale state-root copies when re-keying instead of treating any file at the current path key as authoritative.

Useful? React with 👍 / 👎.

security_config_changed = bool(pinned) and new_digest != pinned
fields: dict[str, object] = {
# Scalars only, per the note above: a bool records THAT the pinned surface
# moved without journaling a command, a binary path or a plugin name.
Expand Down Expand Up @@ -2187,6 +2197,23 @@ def _resume_paused_run(project: Path, run_dir: Path) -> int:
# this process is about to arm re-reads it from there. Leaving the launch
# digest would make every auto-sweep after a legitimate resume-to-fix-a-setting
# refuse — and would make the warning above fire forever, on every later resume.
#
# This is also the migration for a run paused under the old code: it read its
# baseline out of state.json above, and from here on it has a file in the state
# root, so the in-tree copy stops deciding anything for it.
runs.write_trusted_config_digest(project, run_dir.name, new_digest)
# ...and the in-tree secondary is re-stamped in the same breath, for the same
# reason it is written at launch: the state root is keyed by the project's
# RESOLVED PATH (`runs.project_tag`), so moving or renaming the project — a
# documented operation, FEATURES.md states the GC half of it — keys the run
# somewhere new and the file above becomes unreachable. Leaving only that copy
# made a move silently retire the pin, and the advisory this whole guard exists
# to raise never fired again. Writing both keeps the warning alive across a move
# without making this copy authoritative: the reader prefers the out-of-tree
# file whenever it exists, so a session that rewrites this one is still ignored
# (which is #498, and its test still holds). Same for a changed
# BMAD_LOOP_STATE_DIR. Tampering that removes the out-of-tree file is a
# different problem with no fix at equal privilege — #571.
state.trusted_config_digest = new_digest
state.clear_pause()
# A resume is fresh user intent: discard any graceful-stop request left over from
Expand Down
34 changes: 27 additions & 7 deletions src/bmad_loop/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,13 +467,33 @@ class RunState:
project: str
started_at: str
policy_snapshot: dict[str, Any] = field(default_factory=dict)
# runsetup.config_digest over the agent-writable config that reaches HOST code
# execution — verify commands, the resolved launch binary/args/env, the plugin
# allowlist (#461 point 4). Stamped at launch and re-stamped on resume, beside
# policy_snapshot. The auto-sweep gate compares against its own closure
# baseline, not this; the field is the audit record plus the baseline `resume`
# warns off. Empty on runs persisted before the field existed, which is why the
# resume compare is guarded on it being non-empty — no prior pin, no warning.
# SECONDARY copy of the host-exec baseline (#498) — runsetup.config_digest over
# the agent-writable config that reaches HOST code execution: verify commands,
# the resolved launch binary/args/env, the plugin allowlist (#461 point 4).
#
# The one resume TRUSTS is out of the tree (`runs.write_trusted_config_digest`),
# because a baseline whose whole job is to police the agent-writable tree cannot
# live in it — a session that rewrote policy.toml could blank this field in the
# same breath and silence the warning `resume` owes the operator. So this copy is
# never preferred: `_resume_paused_run` consults it ONLY when the state root
# holds no file for the run, which is what keeps rewriting it pointless (the
# #498 attack test asserts exactly that).
#
# It is still written, and must be, for the two cases where the out-of-tree file
# is honestly absent rather than tampered away — in both, this copy is the run's
# only surviving pin:
# * the state root is keyed by the project's RESOLVED PATH (`runs.project_tag`),
# so moving or renaming the project keys the run somewhere new and orphans
# its state subtree (FEATURES.md documents the GC half of this). state.json
# lives in the run dir and travels with it. Same for a BMAD_LOOP_STATE_DIR
# that changes between launch and resume.
# * a run PAUSED before #498 has its baseline here and nowhere else; the first
# resume under this code reads it and mints the out-of-tree file.
# Dropping it would turn "this run has a pin" into "this run has none" in all of
# them. Empty means what it always did — no prior pin, hence no warning — which
# is why the resume compare is guarded on non-emptiness. The auto-sweep gate has
# never read this field: it compares against its own in-memory closure baseline,
# which no session can reach.
trusted_config_digest: str = ""
current_epic: int | None = None
# the run's story scope + cap, as passed on the launching CLI (`--epic`,
Expand Down
Loading