From 6a2ac1c510222897952dde854320cb258ec75530 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 21 Sep 2026 20:52:57 -0700 Subject: [PATCH 1/6] feat: register installed hook relay commands --- CHANGELOG.md | 4 + README.md | 3 +- docs/FEATURES.md | 6 +- docs/porting-to-a-new-os.md | 19 +-- docs/setup-guide.md | 14 +- src/bmad_loop/cli.py | 113 ++++---------- src/bmad_loop/codex_trust.py | 13 +- src/bmad_loop/install.py | 224 +++++++++++++++++---------- src/bmad_loop/probe.py | 2 +- src/bmad_loop/process_host.py | 11 +- src/bmad_loop/worktree_flow.py | 37 +++-- tests/test_cli.py | 247 ++++++++++++------------------ tests/test_codex_trust.py | 56 ++++++- tests/test_hook_script.py | 26 +++- tests/test_install.py | 271 +++++++++++++++++++++++++++++++-- tests/test_process_host.py | 27 ++-- 16 files changed, 676 insertions(+), 397 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c32ce67fe..4a0d4fa0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ breaking changes may land in a minor release. ### Changed +- Register hooks through the installed `bmad-loop relay ` command. Upgrading + invalidates Codex hook trust: Codex re-prompts at the next launch, and hooks silently + do not fire until the new commands are accepted. Re-run `bmad-loop init` to migrate + managed registrations. - Document the live-session removal guard's measured ceiling (#732): `delete`, `archive` and `clean` still remove a run directory when a listing omits a live session. Behavior unchanged; the psmux half is reported upstream (psmux/psmux#622), its retirement tracked in #754. ### Fixed diff --git a/README.md b/README.md index 926f144ef..d0fbfb4f9 100644 --- a/README.md +++ b/README.md @@ -105,8 +105,7 @@ partial — the tail after the last `-` (e.g. `a1b2`), shortened to any prefix t > One subcommand is deliberately left out of the table: `bmad-loop relay ` writes a single > session event file from a coding-CLI hook payload on stdin. Its own help calls it "a hook target > for machines, not a command to run by hand" — it takes no `--project`, and `bmad-loop init` -> currently registers the copied workspace relay (`.bmad-loop/bmad_loop_hook.py`) instead, so no -> installed hook reaches the console script today. Never invoke it yourself. +> registers this installed command with an absolute path. Never invoke it yourself. ## The TUI diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 199b42a11..6e8dd4569 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -209,7 +209,7 @@ Result` section. Every other spec keeps warn-and-continue, and the record says w - Dirt in your MAIN checkout only blocks a unit merge when the merge — **or the run's own post-merge bookkeeping** — could commit it (#460, #618). Two questions, asked per path over the strays that lie outside the branch's incoming set. What the MERGE can commit is what git has **staged**, so the axis is the index column rather than trackedness: an untracked stray and a tracked stray edited in the working tree only are both inert — measured on git 2.55.0 across both topologies and both strategies, rc 0, the edit survives uncommitted, and it is absent from the resulting commit — so both are left exactly where they are and the guard journals `merge-target-tolerated` naming them. Before this, one unrelated `notes.txt` (untracked, #460) or one saved-but-unstaged edit (#618) in the main checkout escalated the first story and paused an unattended run. A **staged** stray still escalates, because `merge --no-ff` refuses it outright and a fast-forwardable `merge --squash` folds it into the story's commit — against a diverged target `--squash` refuses too, with the same error as `--no-ff`, so the fold is a fast-path artifact rather than a squash property. What the RUN can commit is the second question: the post-merge carries stage the sprint board and the deferred-work ledger **by pathspec** (`git add -- :(literal)`), which takes whatever the working tree holds no matter who wrote it, so **any** dirt on one of those two paths escalates whatever its index column says — otherwise an operator's private reopen of a story row rides out under a `chore(sprint-status): carry …` message with the tree left clean and nothing to read the substitution back from. That protection covers artifacts git already **tracks**: an untracked one has no baseline to diverge from, the orchestrator has been reading that exact file as its own all along, and committing it whole is how a non-ignored board first reaches git (#350) — protecting it would halt the first story of every project that has yet to commit its board. Either escalation names the paths and asks you to commit, stash or revert them (it never cleans them for you), and each names its own remedy, since staged work has to be committed or unstaged while dirt on a carried path has to leave the path entirely. `merge-target-tolerated` records what the **guard** decided and is emitted before the merge, so a stray it waved through by path can still clash with the incoming commit by **shape** — a file where the merge needs a directory, or the reverse — and git then refuses at pre-flight over the very path the event called harmless; that run journals a corrective `merge-preflight-refused` beside it, naming the same paths and carrying git's raw text (#623). A `per_worktree` engine Editor leaking _this branch's own_ files into the main checkout is still auto-cleaned and journaled `merge-target-cleaned`, since those are duplicates of content the branch already committed. - A merge-back that fails escalates in one of **five** typed shapes rather than one, plus an honest fallback for anything unclassified (#619). Git declining at **pre-flight** — an untracked file the merge would overwrite, a staged change on an incoming path, a file/directory shape clash, a `merge_strategy = "ff"` target that cannot fast-forward — is not a conflict: nothing was merged, the target checkout is exactly as it was, and there are no markers to find. That escalation now says so and lets git's appended text name the cause and the paths, instead of sending you to resolve a conflict that does not exist; a genuine content conflict keeps the resolve-by-hand wording, and both keep the unit's branch and worktree mounted for manual recovery. The third shape is a `--no-ff` that merged cleanly and was then refused at the **commit** — a `pre-merge-commit` or `commit-msg` hook exiting non-zero, or a `commit.gpgsign` that cannot sign. Nothing conflicted, so it leaves no unmerged stages, but it does leave `MERGE_HEAD`: an index-only reading calls that started merge a pre-flight refusal and sends you to clear a clash that does not exist. bmad-loop aborts it, restores the checkout, and points you at the policy that declined instead. Telling the three apart takes **both** probes — `git ls-files -u` leads and answers content, and `MERGE_HEAD`, read before the abort that erases it, parts a merge that never started from one that started and could not be sealed. Neither alone is enough: a conflicted `merge --squash` writes three unmerged stages and conflict markers while creating no `MERGE_HEAD` at all. The `squash` leg reaches the third shape through a different door: `--squash` itself stops before committing by design, so no hook or signature can refuse the merge invocation — but the leg then seals the staged result with its own plain `git commit`, where commit hooks and `commit.gpgsign` run like anywhere else, and a refusal there is the same third shape. That one is rolled back with `git reset --hard HEAD`, gated on the pre-merge reading having found the tree clean — a checkout already carrying uncommitted work of yours is never reset, and the reset is deliberately whole-tree, since it undoes a merge that SUCCEEDED, whose staged result spans the entire incoming set (the stated ceiling: an edit landing after that clean reading rides the reset); when the rollback is withheld the squash result is left **staged**, and the escalation says so and names clearing it as your first step. Neither the exit code nor the message text can stand in, the same refusal being rc 1 or rc 2 depending on topology, rc 1 also being what a conflict returns, and the message being fully translated. The fourth shape is the one **all three** strategies reach and no index reading can see: git dying part-way through the CHECKOUT. It materializes the incoming files in index order, so a failure partway — measured under a **required** clean/smudge filter that cannot run, on all three strategies — stops with HEAD where it was and the tree already partly rewritten. `--ff-only` is not exempt: it declines the _topology_ question before touching anything, but once the fast-forward is possible it checks the incoming tree out like any other merge, so the "it never starts a merge" premise that used to excuse this leg from checking was simply wrong. That leaves no unmerged stages, no `MERGE_HEAD`, and a tree that reads clean against HEAD (an untracked file is in neither HEAD nor the index), so every probe above calls it a pre-flight refusal and tells you the checkout is untouched — while the residue refuses the NEXT merge as an untracked-overwrite, identically on every resume, over paths nothing named. Residue probes answer it: the untracked set and the dirty-tracked set are each sampled **before** the merge, differenced after, and the deltas intersected with the branch's incoming set — the only paths a merge can write — so the answer is "git wrote this", not "this is here" and not "something changed while the merge ran". The intersection closes the seventh mislabeled state: with attribution read off a repo-wide "the tree is dirty now" boolean, an edit YOU made to an unrelated tracked file while the merge was failing was attributed to git — a genuine pre-flight refusal classified as "failed part-way through checkout", and the repo-wide `reset --hard HEAD` riding on that attribution destroyed your edit (measured on all three strategies). The residue has two axes and they get different answers. An incoming path the target did not already track lands **untracked**, and nothing reaches it — neither `git merge --abort` (which exits 128 here anyway, there being no merge to abort) nor `git reset --hard` touches untracked files — so it is named for you to clear rather than cleaned, since the delta proves git wrote a path, not that the bytes now there are yours or git's. An incoming path the target **did** track is rewritten in place, which a path-scoped `git checkout HEAD -- ` over exactly the attributed paths does undo, so that half is undone for you on every leg — never by a repo-wide reset, whose blast radius would take your own uncommitted work on paths the attribution deliberately left alone — and the escalation asks only for whichever residue actually survived, rather than reciting both; a failed restore names the exact paths and prescribes the same path-scoped command. Two ceilings worth knowing, both per path rather than per tree: a path that was _already_ dirty before the merge stays unattributable — its delta cannot say which bytes are whose — and an edit landing **during** the merge window on a path **inside** the incoming set is indistinguishable from git's own write, so it is restored with it; "cannot tell" and "git did nothing" fail to the same side on purpose, the alternative being a restore over your uncommitted work. The differencing is load-bearing and not tidiness: an absolute reading would reclassify every genuine pre-flight refusal that happens to have an untracked stray in the checkout, which is exactly the stray the guard above deliberately tolerates. Relatedly, a `squash` merge git refuses no longer discards your uncommitted work: `--squash` has no `--abort`, so the recovery is that same path-scoped restore over whatever the deltas attribute to git — your pre-existing dirt is never touched, and a conflict landing on top of it has the conflicted incoming paths restored while your edit stays put. The fifth shape is one of the classifier's own post-merge readings failing after a failed merge — the residue deltas or the incoming set that attributes them, the unmerged-stages reading, or the `MERGE_HEAD` reading: what rested on that reading is then **unverified**, and the escalation says exactly that — every verdict still standing on its own live measurement keeps its class, cleanup not gated on the dead reading still runs, and an unread `MERGE_HEAD` skips the abort it gates and says so rather than repairing on uncertainty — and you are sent to the one reading the run could not take, your own `git status`, instead of a probe failure impersonating a verdict. A reading can die on the far side of success too: a `merge --squash` that succeeded still needs one index reading to tell a no-op replay from a result to commit, and with that reading dead the same unverified escalation fires — nothing committed, nothing reset, any staged result left in place — rather than the doomed `git commit` that used to dress the failure as a commit refusal and roll the tree back on the fiction. The conflict itself is typed from its own measurement too (the unmerged stages), so the final catch-all no longer equates "anything unrecognized" with "content conflict": a failure none of the probes classified escalates saying just that — the run cannot say what state the checkout is in; `git status` and git's own appended text are the sources — rather than prescribing conflict resolution for a state nothing measured. - Failed-unit forensics: a deferred/escalated unit's worktree + branch stay mounted (`keep_failed`, default on) and its full diff is preserved to `run_dir/failed//changes.patch`; `failed_diff_max_mb` caps per-file untracked-file size (oversized skipped with a marker), `failed_diff_unlimited` lifts the cap. -- Config seeding: a worktree checks out _tracked_ files only, so a project's gitignored MCP/CLI configs (`.mcp.json`, `.claude/settings.json`, `.codex/config.toml`, `.gemini/settings.json`) would be missing — an isolated session couldn't reach its MCP server. With `seed_adapter_defaults` (default on) each loaded adapter's own `seed_files` are copied in from the main repo before the session launches, together with every non-hookless adapter's resolved hook `config_path` — which is gitignored for codex, so without it the project's own hook configuration never reached the worktree and the session ran against a file holding bmad-loop's relay registrations alone (#471). Seeding does not decide whether that relay hook registers — the hook-merge writes it either way, with one refusal: a seeded config that is present but **will not parse** stops provisioning outright (`GitError` → CRITICAL escalation, run paused) instead of being read as an empty document. JSON has no partial read, so an unparseable config is evidence of an earlier fault rather than a blank slate — and merging the relay into `{}` would publish a hooks-only file over the operator's allowlist, `env` and MCP entries, erasing that evidence along with them. The bytes are left exactly as found, and the refusal names whichever source actually supplies them rather than the copy it read: an escalated story re-enters the run only through a re-arm, which discards the worktree and provisions a fresh one, so a repair applied to the disposable copy is undone before the next drive. Which source that is comes from the seed bookkeeping rather than from a counterpart's existence — seeding is copy-when-absent, so a config the project TRACKS is skipped as an occupied destination and arrives with the branch checkout, and that lane is told to commit the repair on the target branch while a genuinely seeded one is sent to the main checkout. That bookkeeping is recorded per path actually written rather than per seed entry: a directory entry seeds child by child, so its presence proves that some child landed and never that this config did, and inferring from it would advise committing a gitignored settings file the checkout supplied. The remedy names the re-arm (`bmad-loop resolve --no-interactive`) because `ESCALATED` is terminal: repairing the file alone leaves the story out of the run (#592). Invalid UTF-8 takes the same lane, which before this crashed the engine instead of escalating. `worktree_seed` adds extra paths. Every configured seed path is refused at **config load** if any component names a Windows device or ends in a period or space (#480) — `worktree_seed`, an adapter's `seed_files`/`seed_globs`, its hook `config_path` and `skill_tree`, a plugin's `[python] module`, and the Unity seeder's guard dir all take the same refusal, on every platform. Such a component names a _different_ path on Windows than the one it spells: `NUL` and `aux.json` are devices rather than files, and `.claude/skills.` is created as `.claude/skills` because Win32 strips a trailing run of periods and spaces before the path reaches the filesystem — so the copy lands somewhere the operator did not configure, and the exclude pattern the shield later renders from the authored spelling names a path that does not exist. Refusing at load is what keeps the render site below from ever seeing one. It is a compatibility break on config that previously loaded, and a hard refusal rather than a warning for the reason the value is dangerous at all — it means one thing here and another there, and a run that quietly seeds a different file than the one configured is the failure the guard exists to prevent. Copy-when-absent at file granularity — a directory entry whose destination already exists (a worktree checkout carries its tracked children) still seeds the children that are missing — seeded before the hook-merge (a seeded `settings.json` keeps its content, but its relay entry is replaced rather than kept — the seeded copy names the main repo's `$CLAUDE_PROJECT_DIR`-relative relay, which resolves to the worktree, where no relay exists; left in place it would read as already registered and the session would stall silently, #352; a _tracked_ config gets the same rewrite pinned `skip-worktree` in the worktree's own index, so the machine-specific command never rides `git add -A` into a story commit — while pinned the config is orchestrator-owned, so a story's own edit to it stays session-local), and shielded from the unit's `git add -A` — in a private exclude scoped to that worktree alone (see below), never repo-wide. +- Config seeding: a worktree checks out _tracked_ files only, so a project's gitignored MCP/CLI configs (`.mcp.json`, `.claude/settings.json`, `.codex/config.toml`, `.gemini/settings.json`) would be missing — an isolated session couldn't reach its MCP server. With `seed_adapter_defaults` (default on) each loaded adapter's own `seed_files` are copied in from the main repo before the session launches, together with every non-hookless adapter's resolved hook `config_path` — which is gitignored for codex, so without it the project's own hook configuration never reached the worktree and the session ran against a file holding bmad-loop's relay registrations alone (#471). Seeding does not decide whether that relay hook registers — the hook-merge writes it either way, with one refusal: a seeded config that is present but **will not parse** stops provisioning outright (`GitError` → CRITICAL escalation, run paused) instead of being read as an empty document. JSON has no partial read, so an unparseable config is evidence of an earlier fault rather than a blank slate — and merging the relay into `{}` would publish a hooks-only file over the operator's allowlist, `env` and MCP entries, erasing that evidence along with them. The bytes are left exactly as found, and the refusal names whichever source actually supplies them rather than the copy it read: an escalated story re-enters the run only through a re-arm, which discards the worktree and provisions a fresh one, so a repair applied to the disposable copy is undone before the next drive. Which source that is comes from the seed bookkeeping rather than from a counterpart's existence — seeding is copy-when-absent, so a config the project TRACKS is skipped as an occupied destination and arrives with the branch checkout, and that lane is told to commit the repair on the target branch while a genuinely seeded one is sent to the main checkout. That bookkeeping is recorded per path actually written rather than per seed entry: a directory entry seeds child by child, so its presence proves that some child landed and never that this config did, and inferring from it would advise committing a gitignored settings file the checkout supplied. The remedy names the re-arm (`bmad-loop resolve --no-interactive`) because `ESCALATED` is terminal: repairing the file alone leaves the story out of the run (#592). Invalid UTF-8 takes the same lane, which before this crashed the engine instead of escalating. `worktree_seed` adds extra paths. Every configured seed path is refused at **config load** if any component names a Windows device or ends in a period or space (#480) — `worktree_seed`, an adapter's `seed_files`/`seed_globs`, its hook `config_path` and `skill_tree`, a plugin's `[python] module`, and the Unity seeder's guard dir all take the same refusal, on every platform. Such a component names a _different_ path on Windows than the one it spells: `NUL` and `aux.json` are devices rather than files, and `.claude/skills.` is created as `.claude/skills` because Win32 strips a trailing run of periods and spaces before the path reaches the filesystem — so the copy lands somewhere the operator did not configure, and the exclude pattern the shield later renders from the authored spelling names a path that does not exist. Refusing at load is what keeps the render site below from ever seeing one. It is a compatibility break on config that previously loaded, and a hard refusal rather than a warning for the reason the value is dangerous at all — it means one thing here and another there, and a run that quietly seeds a different file than the one configured is the failure the guard exists to prevent. Copy-when-absent at file granularity — a directory entry whose destination already exists (a worktree checkout carries its tracked children) still seeds the children that are missing — seeded before the hook-merge (a seeded `settings.json` keeps its content, but its relay entry is replaced rather than kept — an older seeded copy can name a workspace relay missing in the worktree; left in place it would read as already registered and the session would stall silently, #352; a _tracked_ config that needs the same rewrite is pinned `skip-worktree` in the worktree's own index, so the local command never rides `git add -A` into a story commit — while pinned the config is orchestrator-owned, so a story's own edit to it stays session-local), and shielded from the unit's `git add -A` — in a private exclude scoped to that worktree alone (see below), never repo-wide. - Seeding is verified **result-side**, by re-probing the worktree on disk rather than trusting the copier's bookkeeping — so a user-authored `worktree_seed` rel can neither forge nor mask a report: `worktree-seed-skipped` (a seed entry that copied nothing), `worktree-seed-dropped` (a repo-carried seed that never arrived), and `worktree-module-skills-dropped` (#464 — a wheel-bundled `bmad-loop-*` skill whose content came up short, e.g. a checkout file squatting the skill's directory, which the per-file no-clobber refuses to replace) are all **informational**, journaled and never a pause. Presence is the whole contract; content is never compared, since no-clobber is per _file_ and a checkout's own divergent fork of a bundled skill is a healthy shape. Only the upstream dev/review skill and renderer completeness probes escalate, because those have a deterministic in-worktree consumer — the session dispatches them inside the worktree and stalls having written nothing — while the bundled operator/triage skills are dispatched at the **main checkout** (`/bmad-loop-sweep` from sweep triage, `bmad-loop resolve`; `bmad-loop-setup` has no session consumer at all), so their absence in a worktree stalls nothing. - A fresh mount that supersedes an **accepted-but-uncommitted spec** says so (`accepted-spec-write-unreachable`, DW-101). The `pause_after_spec` gate hands you a spec that is uncommitted by construction, and a re-drive's worktree is a checkout of a commit — so for a **tracked** artifacts dir `git worktree add` delivers the pre-approval bytes, seeding skips an occupied destination, and the delivery probe above passes on existence alone while the session reads text your corrections were meant to replace. The mount's copy is byte-compared against the main checkout's with line endings normalized (CRLF ≡ LF — the one translation a checkout under `core.autocrlf` applies on its own, and one git folds away again on commit; any other byte still counts), and a difference — or a read that fails either side, recorded as `compared: false`, since a probe that cannot read cannot prove delivery — is journaled naming the **main-checkout** spec path (the file to commit) and the **target branch** to commit it on. **Informational**, journaled and never a pause, exactly like the seeding records above: it does not overwrite the mount's copy — a dirty tracked file there is not covered by the worktree-scoped exclude, so the unit's `git add -A` would fold your in-progress edits into the story commit — and it does not refuse the mount, which would hard-fail every isolated unit in a project that tracks its artifacts dir. The escalation path's counterpart is `rearm-spec-write-unreachable`, whose remedy is the same sentence. - A mount that cannot be shown to carry the accepted spec **at all** says so too (`accepted-spec-delivery-unreachable`, DW-104/DW-115). Two silences end here, both on a spec the task already spells **project-relative** — the spelling a resume persists, and the one leg with no escalating guard over it. Either the spec's mounted parent escapes the worktree (a real directory in your checkout, a committed **outward symlink** in the commit the worktree was cut from), so seeding refuses it on containment; or the spec could not be located at all — a filesystem fault while resolving it was swallowed, or the path no longer resolves because the file is gone. Both used to produce the same nothing: the unit dispatched against a mount lacking your spec, fell back to the bare story key, and no journal named why. Now the refused rel is also nominated as a seed — nothing is copied outside the mount, because the copier re-checks containment itself, but `worktree-seed-dropped` names it — and the record is journaled naming the **main-checkout** spec path and the **target branch**. `located` says which silence it was, and the two have different remedies: `true` means the spec was found in your checkout but the mount could not be shown to carry it, so commit it on the named branch (or unpick the outward symlink that makes the artifacts dir unseedable) and re-drive. A file probe fault also produces `true`, even if the bytes are present: check filesystem access and I/O health when the file is already delivered. `false` means it could not be resolved in your checkout at all, so the named path is the one to restore or re-point before re-driving. **Informational**, journaled and never a pause, exactly like the seeding records above. The **relocated** leg — an absolute spec normalized for the mount — gets no record here, because it already escalates on the same loss a few lines earlier; it may still carry a `worktree-seed-dropped` entry for the refused rel, which is the seeding report rather than a second advisory. @@ -248,7 +248,7 @@ Result` section. Every other spec keeps warn-and-continue, and the record says w ### Hook-based transport (no pane-scraping) - Coding-agent hooks (`Stop` / `SessionStart` / `SessionEnd` / `PreCompact`) write structured event files the orchestrator watches; skills write a machine-readable `result.json`. -- The event channel lives **outside the project tree** (#494), at `///events/` — a branch switch, a worktree mount or a rollback must not be able to take a live run's control plane away. Each session is told where to write via `BMAD_LOOP_EVENTS_DIR`; the state root itself resolves per `BMAD_LOOP_STATE_DIR` (see the env-var table in the README). The relay falls back to the legacy in-tree `/events` when that variable is absent, and the orchestrator keeps polling that location too — the hook script is copied into the project by `init`, so an upgraded orchestrator regularly drives sessions whose relay predates the move, and without both halves every such session would stall to `session_timeout_min`. +- The event channel lives **outside the project tree** (#494), at `///events/` — a branch switch, a worktree mount or a rollback must not be able to take a live run's control plane away. Each session is told where to write via `BMAD_LOOP_EVENTS_DIR`; the state root itself resolves per `BMAD_LOOP_STATE_DIR` (see the env-var table in the README). The relay falls back to the legacy in-tree `/events` when that variable is absent, and the orchestrator keeps polling that location too — older sessions may still carry a copied relay, and without both halves those sessions would stall to `session_timeout_min`. Fresh `init` registrations call the installed `bmad-loop relay ` executable by absolute path and require no workspace relay copy. - What the move is and is not worth: it eliminates the whole in-tree redirect class — nothing an agent writes inside the project can any longer point the completion channel somewhere else — and it is a hard boundary for a sandboxed session, which cannot reach outside the tree at all. It is **not** a boundary against a session running with permissions bypassed: that session is told the directory by `BMAD_LOOP_EVENTS_DIR`, so it can reach it by construction. This is why the relay keeps its own `_is_link_like` refusal on the write path (#493) as belt-and-braces rather than retiring it as redundant. ### Deferred-work sweeps @@ -752,7 +752,7 @@ verdict unverifiable rather than certifying a different launch configuration. ### Setup & install -- `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), the hook relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`. +- `bmad-loop init` installs the three `bmad-loop-*` skills (`bmad-loop-setup`, `bmad-loop-resolve`, `bmad-loop-sweep`, into `.claude/skills/` and/or `.agents/skills/`), an absolute hook registration for the installed relay, `.bmad-loop/policy.toml`, and a gitignore covering the runs dir, plugin caches, and policy.toml itself (per-machine config). Flags: `--cli` (repeatable), `--no-skills`, `--force-skills`. - `bmad-loop validate` preflights every prerequisite: BMAD config, sprint-status, git (including its version — a host below the **2.34** support floor gets a `git.version` **problem** and exit 1, so validate's verdict cannot disagree with run/sweep/resume's outright refusal), the selected terminal-multiplexer backend (listing all detected when more than one is registered), CLI binary (**probed, not just resolved**: a name that is on `PATH` but fails `--version`, typically a dead WSL/npm shim, adds an `adapter.binary-unrunnable` finding at warning severity — `adapter.binary` itself still reports ok, and validate's exit code is unchanged — [#294](https://github.com/bmad-code-org/bmad-loop/issues/294); only **packaged** profiles are probed — a project overlay's binary is resolved but never launched, so a clone cannot choose which binary this diagnostic launches; resolution still goes through your `PATH`, so what a probed name resolves to is whatever the session launch would itself run), hook registration, and the review skills the installed dev primitive actually invokes (reporting which name it resolved) — derived from its `customize.toml` review layers (or from `step-04-review.md` on releases that name reviewers inline), so both the merged `bmad-review` topology and the standalone-hunter one validate, and configured layers naming an uninstalled skill are caught — plus its `customize.toml`. - The preflight also **names the multiplexer selection reason wherever selection resolves** (`mux.selection`, e.g. `platform default for win32`), not only when a `BMAD_LOOP_MUX_BACKEND`/`[mux] backend` choice forced it. A `fallback` selection is reported as a warning (its own label says no available backend matches this platform); a selection that outright failed is carried by `mux.preflight`, and a detection that failed by `mux.backends-detected` at warning — so a missing `mux.selection` line is normally explained by another finding (the historical unregistered-tmux fallback is the one silent exception; see the `--json` contract note in `documents.py`). On top of that, `host.win32-on-wsl-path` warns when a **native-Windows interpreter is working on a `\\wsl.localhost\...` project** ([#332](https://github.com/bmad-code-org/bmad-loop/issues/332) — see [multiplexer-backends.md](multiplexer-backends.md) for why WSL can hand a bash prompt the Windows build). Both are diagnostics only: neither changes which backend is selected (psmux _is_ correct for a `win32` interpreter) and neither flips validate's exit code. `bmad-loop diagnose` carries the same two facts in its Environment block as `sys.platform` and `win32 on WSL distro path` (`yes`/`no`). - Non-invasive: drives the upstream dev primitive unmodified — there is no fork to keep in sync — and review is just a re-invocation of it on the `done` spec. Your standard BMAD install is never modified. diff --git a/docs/porting-to-a-new-os.md b/docs/porting-to-a-new-os.md index 2fb9720d4..a11bb9b74 100644 --- a/docs/porting-to-a-new-os.md +++ b/docs/porting-to-a-new-os.md @@ -303,17 +303,12 @@ register_process_host("windows", lambda platform: platform == "win32", WindowsPr ## Seam 3 — hook interpreter -`ProcessHost.hook_interpreter()` is the command prefix that `install` / `probe` -interpolate into the hook registrations they write (the script path and canonical -event are appended by the caller). It exists so hook registration never branches -on `sys.platform` at the call site: - -- POSIX returns `"python3"` (the interpreter on PATH). -- `WindowsProcessHost` returns `"uv run --no-project python"` — Windows ships no - `python3` launcher, and `--no-project` resolves an interpreter without activating - a project venv (hooks fire detached). - -A new OS overrides this on its `ProcessHost`; nothing else changes. +`ProcessHost.hook_interpreter()` supplies an absolute, host-quoted +`sys.executable` path for temporary Python hook scripts, such as the live probe +capture hook. Both POSIX and Windows use that interpreter directly. `init` and +worktree provisioning register the installed `bmad-loop relay ` console +script by absolute path instead of invoking a Python script in the workspace. +A new OS overrides the quoting behavior on its `ProcessHost` as needed. --- @@ -466,7 +461,7 @@ Concretely, a native-Windows port is: its `register_multiplexer("psmux", …)`. 2. `WindowsProcessHost` — **already shipped** — needs only its registration, which is **already present** in `_load_builtin_hosts`. Its `hook_interpreter()` - (`uv run --no-project python`) is in place too. + (an absolute, host-quoted interpreter path) is in place too. 3. A CI runner on Windows to exercise the above. No edits to the adapters, `runs.py`, `tui/launch.py`, `probe.py`, `tui/data.py`, diff --git a/docs/setup-guide.md b/docs/setup-guide.md index 53b9f34c1..e2d5a109d 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -360,8 +360,8 @@ what each command touches. Make sure no run is still live (Editor open, session ### 2. Remove the orchestrator state -Delete the `.bmad-loop/` directory. This removes the hook relay script -(`.bmad-loop/bmad_loop_hook.py`), the `policy.toml` template, and all per-run state +Delete the `.bmad-loop/` directory. This removes the `policy.toml` template, +legacy relay copies from older installs, and all per-run state (`runs/`, `cache/`, `archive/`) in one step: ```bash @@ -400,14 +400,18 @@ that also cleans up a pre-0.7.0 install.) `init` **merged** its Stop-hook registration into each CLI's existing hook config, so these files must be **edited, not deleted** (they hold your own settings too). In each config below, -remove the hook entry whose `command` contains `bmad_loop_hook.py`: +remove the hook entry whose `command` ends in `bmad-loop relay ` (or +contains `bmad_loop_hook.py` on older installs): - **claude** — `.claude/settings.json` - **codex** — `.codex/hooks.json` - **gemini** — `.gemini/settings.json` +- **copilot** — `.github/copilot/settings.json` +- **antigravity** — `.agents/hooks.json` (the `bmad-loop` hook group) -Edit only the registered CLIs. The `bmad_loop_hook.py` substring uniquely identifies the -entries to strip; leave every other hook in place. +Edit only the registered CLIs. Match the full relay command and event; leave every +other hook in place. The installed `bmad-loop` executable is shared with other projects, +so remove it only when uninstalling the package itself. ### 5. Drop the gitignore lines diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 262649d4b..b9850d98e 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -679,7 +679,7 @@ def cmd_validate(args: argparse.Namespace) -> int: {"binary": tool, "path": resolved, "returncode": rc}, ) - any_hooks_registered = False + registered_relay_paths: set[Path] = set() for profile in profiles: # Keyed on the adapter KIND, not on `hookless`. httpx is the bundled # opencode family's optional extra — a fact about one adapter class, which @@ -714,12 +714,27 @@ def cmd_validate(args: argparse.Namespace) -> int: continue hook_config = project / profile.hooks.config_path hooks_ok = False + parsed: dict = {} if hook_config.is_file(): try: parsed = json.loads(hook_config.read_text(encoding="utf-8")) hooks_ok = isinstance(parsed, dict) and relay_registered( parsed, profile.hooks.dialect, profile.hooks.events ) + if isinstance(parsed, dict): + container = install.hook_event_container(parsed, profile.hooks.dialect) + malformed = [ + event + for event in profile.hooks.events + if event in container and not isinstance(container[event], list) + ] + if malformed: + hooks_ok = False + report.fail( + "hooks.config-parse", + f"{hook_config} has malformed handlers for {', '.join(malformed)}", + {"profile": profile.name, "config_path": str(hook_config)}, + ) except json.JSONDecodeError: report.fail( "hooks.config-parse", @@ -727,7 +742,11 @@ def cmd_validate(args: argparse.Namespace) -> int: {"profile": profile.name, "config_path": str(hook_config)}, ) if hooks_ok: - any_hooks_registered = True + registered_relay_paths.update( + install.registered_relay_paths( + parsed, profile.hooks.dialect, profile.hooks.events, project + ) + ) report.ok( "hooks.registered", f"bmad-loop hooks registered for {profile.name}", @@ -784,91 +803,29 @@ def cmd_validate(args: argparse.Namespace) -> int: {"profile": profile.name, "project": str(project), "binary": profile.binary}, ) - # #461: `hooks.registered` above is a substring match on the config JSON — it - # never touches the artifact the registered command points AT. A branch switch - # (or a deleted .bmad-loop/) leaves the registration green while every hook - # event is a silent no-op and the run stalls to session_timeout_min, so stat - # the relay itself. Outside the per-profile loop on purpose: the relay is one - # shared artifact, and per-profile reporting would print the same line N times. - # A distinct id, not a repurposed `hooks.registered` — the two answer different - # questions and an operator needs to see which one failed. - # - # COUPLING (#461 Phase 2): Phase 2 moves the relay to the installed console - # script — `bmad-loop relay ` (cmd_relay / events.py), NOT the - # ` -m bmad_loop.hookrelay` spelling this once anticipated — and - # retires HOOK_SCRIPT_REL. It must RETARGET this check to stat what the - # registration actually points at (the resolved `bmad-loop` executable), not - # drop it — the stall it guards against survives the move: an entry point that - # is gone or unreadable strands every hook event exactly like a missing script. - if any_hooks_registered: - relay = project / install.HOOK_SCRIPT_REL - # Existence is not enough: `is_file()` stays True for a mode-000 file, and - # the registered command is ` `, which has to - # READ the script — an unreadable relay exits 2 ("can't open file") and the - # run stalls exactly as if the relay were gone, which is the blind spot - # this whole check exists to remove. `os.access` uses the REAL uid/gid, - # which is what the operator's own `bmad-loop` invocation runs as, and it - # stays correct under root (who can read a 000 file) where a mode-bit test - # would false-fail. On Windows `chmod` can only toggle the read-only flag, - # so this arm is POSIX-effective and never makes the Windows path stricter. + # Inspect the executable each managed registration actually names. A new + # installation in this process cannot repair an older path in a hook config. + for relay in sorted(registered_relay_paths): if not relay.is_file(): report.fail( "hooks.relay-present", - f"hooks are registered but the relay script {relay} is missing — " - f"run `bmad-loop init`", + f"registered hook executable {relay} is missing — re-run `bmad-loop init`", {"path": str(relay)}, ) - elif not os.access(relay, os.R_OK): - # Deliberately NOT "run `bmad-loop init`": install_into writes this path - # with write_text(), which needs write access to the same file, so init - # raises PermissionError instead of repairing it. Sending the operator - # to a command that also fails is worse than saying nothing. + elif not os.access( + relay, os.R_OK if relay.name == "bmad_loop_hook.py" else os.R_OK | os.X_OK + ): report.fail( "hooks.relay-present", - f"hooks are registered but the relay script {relay} is not readable — " - f"the registered hook command cannot run it, so every hook event " - f"no-ops. Restore read permission (`chmod u+r`) or delete it and " - f"re-run `bmad-loop init`", + f"registered hook executable {relay} is not usable — repair its permissions or re-run `bmad-loop init`", {"path": str(relay)}, ) else: report.ok( "hooks.relay-present", - f"hook relay script present: {relay}", - {"path": str(relay)}, - ) - - # #494 Phase 4: present-and-readable is not current. The relay is COPIED - # into the project by `init`, so an upgraded orchestrator routinely drives - # sessions through a relay written by an older wheel — and the #494 move - # is exactly the kind of change that skew hides: a pre-move relay writes - # its events to the in-tree `/events` while the operator believes - # the channel left the project tree, so a branch switch can still take the - # control plane away mid-run. - # - # A WARNING, never a problem, and validate's exit code must not move: - # Phase 3's fallback pair keeps a stale relay FUNCTIONAL (it writes the - # legacy directory, which SignalWatcher still polls), so the run completes - # — the operator is losing the property, not the loop. `passed` counts - # only problems, so `warn` is what says "degraded but working". - stale = install.hook_script_current(project) - if stale is False: - report.warn( - "hooks.relay-stale", - f"the installed hook relay {relay} differs from this bmad-loop's " - f"— it is from another version, or was edited. Events may still be " - f"written inside the project tree; run `bmad-loop init` to refresh it", - {"path": str(relay)}, - ) - elif stale is True: - report.ok( - "hooks.relay-stale", - f"hook relay script up to date: {relay}", + f"registered hook executable available: {relay}", {"path": str(relay)}, ) - # `None` (unreadable/undecodable on either side) reports nothing: the - # relay-present block above already spoke for the cases an operator can - # act on, and "I could not compare" is not a finding about their project. # Adapter-kind validity is enforced against the LIVE registry, never a # hardcoded set: a profile.adapter naming no registered kind is a config error @@ -5312,14 +5269,8 @@ def cmd_init(args: argparse.Namespace) -> int: def cmd_relay(args: argparse.Namespace) -> int: """``bmad-loop relay `` — the hook relay as an installed console script. - **Nothing points at it yet.** ``init`` still registers the copied workspace - relay (``install._hook_command`` emits `` /.bmad-loop/ - bmad_loop_hook.py ``), so no installed hook reaches this handler today; - it is the target #461 Phase 2 retargets those registrations to, and that move - carries its own obligation — see the COUPLING note on ``hooks.relay-present``, - which must be retargeted rather than dropped in the same change. Said here - because a console script that exists and is documented reads as the live path, - and an operator debugging a lost Stop needs to know which relay actually ran. + ``init`` registers the absolute entry point belonging to this installation. + ``hooks.relay-present`` checks the path each registration actually names. Total by contract, unlike every other handler: a coding CLI runs this INSIDE the session whose completion it reports, and several of them surface a diff --git a/src/bmad_loop/codex_trust.py b/src/bmad_loop/codex_trust.py index 13141a3e0..f521a3425 100644 --- a/src/bmad_loop/codex_trust.py +++ b/src/bmad_loop/codex_trust.py @@ -17,12 +17,12 @@ from dataclasses import dataclass from pathlib import Path -from .adapters.profile import CLIProfile -from .install import _hook_command +from .adapters.profile import CLIProfile, ProfileError +from .install import _hook_command, _relay_command from .process_host import ProcessHostError, get_process_host _EVENTS = {"SessionStart": "sessionStart", "Stop": "stop"} -_RELAY_MARKER = "bmad_loop_hook.py" +_RELAY_MARKER = "bmad-loop" _PROBE_MARKER = "bmad_loop_probe_hook.py" _TIMEOUT_S = 5.0 _SAFE_BYPASS_ARG = "--dangerously-bypass-approvals-and-sandbox" @@ -74,7 +74,10 @@ def _commands( if not isinstance(hook, dict): raise ValueError("malformed Codex hook entry") command = hook.get("command") - if isinstance(command, str) and marker in command: + if not isinstance(command, str): + continue + is_relay = _relay_command(command) if marker == _RELAY_MARKER else marker in command + if is_relay: # A SessionStart matcher can exclude startup even when # Codex reports the command trusted and enabled. The # installed relay has none; refuse customized matchers. @@ -200,6 +203,8 @@ def project_hook_trust( return TrustResult("unverifiable", "hook trust config is unreadable") try: commands = _commands(config, profile, project, marker) + except ProfileError as e: + return TrustResult("unverifiable", f"hook trust installed relay unavailable: {e}") except ValueError: return TrustResult("unverifiable", "hook trust config has malformed fields") if commands is None: diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index ea15a14bd..8673d03c6 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -1,6 +1,6 @@ """`bmad-loop init`: make a target project orchestratable. -- copies the hook relay script to /.bmad-loop/bmad_loop_hook.py +- registers the installed bmad-loop relay console script by absolute path - idempotently merges hook registrations into each selected CLI's hook config (dialect + native->canonical event map come from the CLI profile) - installs the bundled bmad-loop-* skills into each selected CLI's skill tree @@ -9,8 +9,8 @@ - gitignores generated dirs: .bmad-loop/runs/ (per-run state) and .bmad-loop/cache/ (engine plugins' rebuildable caches, e.g. the Unity Library) -Every dialect registers the same relay script under the CLI's native event -names while passing the canonical event name as the script argument, so the +Every dialect registers the same installed relay under the CLI's native event +names while passing the canonical event name as an argument, so the orchestrator's signal watcher is CLI-agnostic. """ @@ -20,13 +20,15 @@ import json import os import re +import shlex import shutil +import sys import tomllib from collections.abc import Iterable, Iterator, Sequence from contextlib import ExitStack from importlib import resources from importlib.resources.abc import Traversable -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any, NamedTuple from .adapters.profile import ALIASES, CLIProfile, ProfileError, load_profiles @@ -36,17 +38,9 @@ from .process_host import get_process_host from .verify import GitError, git_below_floor, git_bytes, git_floor_text, git_version_at_least -HOOK_SCRIPT_REL = ".bmad-loop/bmad_loop_hook.py" -# Markers for bmad-loop-managed hook commands. RELAY_MARKER is shared by -# merge_hooks' dedup and validate/probe detection (via relay_registered) so init -# and the preflight can never disagree about whether the relay is installed. It -# matches the relay script name specifically: a hook command whose path merely -# contains "bmad_loop" can't read as a registration — or suppress one. -RELAY_MARKER = "bmad_loop_hook" # The probe-adapter capture hook participates in merge_hooks' dedup only (a # probe re-merge must stay idempotent) and never counts as a relay -# registration. Disjoint from RELAY_MARKER: "bmad_loop_probe_hook" does not -# contain the substring "bmad_loop_hook". +# registration. Probe capture has a separate command shape. PROBE_MARKER = "bmad_loop_probe_hook" GEMINI_HOOK_TIMEOUT_MS = 60_000 COPILOT_HOOK_TIMEOUT_SEC = 60 @@ -1016,52 +1010,79 @@ def _review_findings(project: Path, tree: str) -> list[Finding]: return findings -def hook_script_current(project: Path) -> bool | None: - """Does the project's installed relay match the one this wheel would write? +def _hook_command(project: Path, profile: CLIProfile, canonical_event: str) -> str: + """Command for this installation's console entry point, independent of PATH.""" + del project, profile + name = "bmad-loop.exe" if os.name == "nt" else "bmad-loop" + invoked = Path(sys.argv[0]).absolute() + executable = invoked if invoked.name == name else Path(sys.executable).absolute().parent / name + if not executable.is_file() or not os.access(executable, os.R_OK | os.X_OK): + raise ProfileError(f"installed bmad-loop command is unavailable: {executable}") + return f"{get_process_host().shell_quote(str(executable))} relay {canonical_event}" + + +def relay_executable(command: str) -> Path | None: + """Return the absolute executable named by an installed relay registration.""" + try: + parts = shlex.split(command, posix=os.name != "nt") + except ValueError: + return None + if ( + len(parts) != 3 + or parts[1] != "relay" + or parts[2] not in {"SessionStart", "Stop", "SessionEnd", "Notification", "PreCompact"} + ): + return None + executable = Path(parts[0].strip('"')) + if executable.name not in {"bmad-loop", "bmad-loop.exe"} or not executable.is_absolute(): + return None + return executable - ``True`` yes, ``False`` stale (or otherwise divergent), ``None`` unknowable — - the installed copy or the packaged source could not be read as text. The - unknown arm is a third state and not a coerced ``False`` on purpose: the sole - caller (``cmd_validate``'s ``hooks.relay-stale``) reports what it knows, and - "I could not look" is not "your relay is out of date". - Lives here, beside :func:`install_into`'s write of the same two paths, so the - reader and the writer of the relay stay in one module and one reviewer's view - — a comparison that resolved the source differently from the writer would - answer a different question. +def _legacy_relay_script(command: str) -> str | None: + """Recognize only the Python command that old init actually registered.""" + for posix in (True, False): + try: + parts = shlex.split(command, posix=posix) + except ValueError: + continue + if len(parts) < 3 or not parts[-1].isidentifier(): + continue + script = parts[-2].strip('"') + prefix = parts[:-2] + # Non-POSIX shlex splits the historical Claude variable from its suffix. + if script.startswith("/.bmad-loop/") and prefix[-1:] == ['"$CLAUDE_PROJECT_DIR"']: + script = "$CLAUDE_PROJECT_DIR" + script + prefix = prefix[:-1] + if not prefix: + continue + normalized = script.replace("\\", "/") + if PurePosixPath(normalized).parts[-2:] != (".bmad-loop", "bmad_loop_hook.py"): + continue + interpreter = PurePosixPath(prefix[0].strip('"').replace("\\", "/")).name.lower() + if interpreter.startswith("python") and len(prefix) == 1: + return script + if interpreter in {"uv", "uv.exe"} and prefix[1:] == ["run", "--no-project", "python"]: + return script + return None + - Compared as TEXT read with universal newlines, not as raw bytes. That is - precisely the round trip ``install_into`` performs (``read_text`` then - ``write_text``), and ``write_text`` translates ``\\n`` to ``os.linesep`` — so - on Windows every freshly-installed relay differs from the packaged source - byte-for-byte while being exactly what ``init`` writes. A byte compare would - call those installs permanently stale. - """ - try: - installed = (project / HOOK_SCRIPT_REL).read_text(encoding="utf-8") - packaged = ( - resources.files("bmad_loop.data") - .joinpath("bmad_loop_hook.py") - .read_text(encoding="utf-8") - ) - except (OSError, ValueError): - # ValueError covers UnicodeDecodeError (a relay overwritten with non-UTF-8 - # bytes); OSError covers missing/unreadable on either side. Observation - # degrades — the missing and unreadable cases already have their own - # finding (`hooks.relay-present`), and a packaged source this process - # cannot read is a broken wheel, not a stale project. - return None - return installed == packaged +def _relay_command(command: object) -> bool: + return isinstance(command, str) and ( + relay_executable(command) is not None or _legacy_relay_script(command) is not None + ) -def _hook_command(project: Path, profile: CLIProfile, canonical_event: str) -> str: - host = get_process_host() - interp = host.hook_interpreter() - if profile.hooks.dialect == "claude-settings-json": - return f'{interp} "$CLAUDE_PROJECT_DIR"/{HOOK_SCRIPT_REL} {canonical_event}' - # Codex/Gemini expose no $CLAUDE_PROJECT_DIR equivalent to hook commands; - # bake the absolute path at init time. - return f"{interp} {host.shell_quote(str(project / HOOK_SCRIPT_REL))} {canonical_event}" +def _commands_in_handler(handler: object) -> Iterator[str]: + if not isinstance(handler, dict): + return + nested = handler.get("hooks") + if isinstance(nested, list): + for item in nested: + if isinstance(item, dict) and isinstance(item.get("command"), str): + yield item["command"] + elif isinstance(handler.get("command"), str): + yield handler["command"] def _hook_entry(dialect: str, command: str) -> dict: @@ -1099,27 +1120,24 @@ def hook_event_container(config: dict, dialect: str) -> dict: def _relay_in_handlers(handlers) -> bool: """True if any handler in a native-event list carries the relay command.""" - return RELAY_MARKER in json.dumps(handlers) + return isinstance(handlers, list) and any( + _relay_command(command) for handler in handlers for command in _commands_in_handler(handler) + ) def _managed_hook_in_handlers(handlers) -> bool: """merge_hooks' dedup: a relay OR probe-capture command is already present.""" - dumped = json.dumps(handlers) - return RELAY_MARKER in dumped or PROBE_MARKER in dumped + return _relay_in_handlers(handlers) or PROBE_MARKER in json.dumps(handlers) def strip_relay_hooks(config: dict, dialect: str) -> bool: """Drop every relay registration from a parsed hook config. True if any went. - The inverse of :func:`merge_hooks`, for the one caller that needs its own - registration to be authoritative rather than additive: a worktree seeded with - the main repo's hook config (``provision_worktree``). That config already - carries a relay command written for the main repo — `$CLAUDE_PROJECT_DIR`-relative - for the claude dialect, which resolves inside the worktree, where no relay - exists. `merge_hooks` will not replace it, since `_managed_hook_in_handlers` - reports the event as already registered, so the stale command has to go first. + Worktree provisioning uses this on its first encounter with a config file. + It removes both legacy copied-script commands and installed relay commands + while preserving user handlers and temporary probe handlers. - Only RELAY_MARKER commands are removed, at command granularity: a matcher + Only managed relay commands are removed, at command granularity: a matcher entry whose nested list holds a project command beside the relay keeps the entry and loses only the relay command. A probe-capture hook is a deliberate, temporary registration that no worktree seeding produces, and is left alone. @@ -1134,7 +1152,7 @@ def strip_relay_hooks(config: dict, dialect: str) -> bool: continue kept = [] for handler in handlers: - if RELAY_MARKER not in json.dumps(handler): + if not any(_relay_command(c) for c in _commands_in_handler(handler)): kept.append(handler) continue # claude/codex/gemini wrap commands in a nested "hooks" list, and a @@ -1144,7 +1162,11 @@ def strip_relay_hooks(config: dict, dialect: str) -> bool: # match means the entry IS the relay and it drops whole. nested = handler.get("hooks") if isinstance(handler, dict) else None if isinstance(nested, list): - surviving = [c for c in nested if RELAY_MARKER not in json.dumps(c)] + surviving = [ + c + for c in nested + if not (isinstance(c, dict) and _relay_command(c.get("command"))) + ] if surviving: if len(surviving) != len(nested): handler["hooks"] = surviving @@ -1166,6 +1188,28 @@ def relay_registered(config: dict, dialect: str, events: Iterable[str]) -> bool: return any(_relay_in_handlers(container.get(event, [])) for event in events) +def registered_relay_paths( + config: dict, dialect: str, events: Iterable[str], project: Path +) -> list[Path]: + """Paths invoked by the actual managed commands in a hook config.""" + container = hook_event_container(config, dialect) + paths: list[Path] = [] + for event in events: + handlers = container.get(event) + if not isinstance(handlers, list): + continue + for handler in handlers: + for command in _commands_in_handler(handler): + executable = relay_executable(command) + if executable is not None: + paths.append(executable) + else: + script = _legacy_relay_script(command) + if script is not None: + paths.append(Path(script.replace("$CLAUDE_PROJECT_DIR", str(project)))) + return paths + + def merge_hooks(config: dict, registrations: dict[str, str], dialect: str) -> tuple[dict, bool]: """Add relay registrations (native event -> command) to a hook config dict.""" changed = False @@ -1186,6 +1230,18 @@ def merge_hooks(config: dict, registrations: dict[str, str], dialect: str) -> tu f"hook event {native_event!r} under {ANTIGRAVITY_HOOK_GROUP!r} " "is not a list; fix the hooks file before re-running init" ) + if any( + _relay_command(existing) and command != existing + for h in handlers + for existing in _commands_in_handler(h) + ): + scoped = {ANTIGRAVITY_HOOK_GROUP: {native_event: handlers}} + changed |= strip_relay_hooks(scoped, dialect) + if native_event in scoped[ANTIGRAVITY_HOOK_GROUP]: + group[native_event] = scoped[ANTIGRAVITY_HOOK_GROUP][native_event] + else: + del group[native_event] + handlers = group.setdefault(native_event, []) if not _managed_hook_in_handlers(handlers): handlers.append(_hook_entry(dialect, command)) changed = True @@ -1195,6 +1251,20 @@ def merge_hooks(config: dict, registrations: dict[str, str], dialect: str) -> tu hooks = config.setdefault("hooks", {}) for native_event, command in registrations.items(): matchers = hooks.setdefault(native_event, []) + if any( + _relay_command(existing) and command != existing + for h in matchers + for existing in _commands_in_handler(h) + ): + # Scope the strip to this event. Profiles may share a config file; + # their disjoint events must survive later registrations. + scoped = {"hooks": {native_event: matchers}} + changed |= strip_relay_hooks(scoped, dialect) + if native_event in scoped["hooks"]: + hooks[native_event] = scoped["hooks"][native_event] + else: + del hooks[native_event] + matchers = hooks.setdefault(native_event, []) # claude/codex/gemini nest handlers under "hooks"; copilot stores the # handler dict directly in the event list — the serialized scan covers # both shapes so a re-run stays idempotent for every dialect. @@ -1246,10 +1316,14 @@ def _register_hooks(project: Path, profile: CLIProfile) -> int: except json.JSONDecodeError: print(f"FAIL: {config_path} is not valid JSON; fix it and re-run init") return 1 - registrations = { - native: _hook_command(project, profile, canonical) - for native, canonical in profile.hooks.events.items() - } + try: + registrations = { + native: _hook_command(project, profile, canonical) + for native, canonical in profile.hooks.events.items() + } + except ProfileError as e: + print(f"FAIL: {e}") + return 1 config, changed = merge_hooks(config, registrations, profile.hooks.dialect) if changed: # atomic_write_text, never write_text (#379), the same rule @@ -2852,13 +2926,7 @@ def install_into( bmad_loop_dir = project / ".bmad-loop" bmad_loop_dir.mkdir(parents=True, exist_ok=True) - # 1. hook relay script (shared by all CLIs) - script_target = project / HOOK_SCRIPT_REL - script_source = resources.files("bmad_loop.data").joinpath("bmad_loop_hook.py") - script_target.write_text(script_source.read_text(encoding="utf-8"), encoding="utf-8") - print(f" hook script: {script_target}") - - # 2. per-CLI hook registration + # 1. per-CLI hook registration for profile in profiles: if _register_hooks(project, profile) != 0: return 1 diff --git a/src/bmad_loop/probe.py b/src/bmad_loop/probe.py index e232fb170..0a4b9df84 100644 --- a/src/bmad_loop/probe.py +++ b/src/bmad_loop/probe.py @@ -481,7 +481,7 @@ def _check_hook_trust( return from .codex_trust import project_hook_trust - marker = PROBE_HOOK_NAME if live else "bmad_loop_hook.py" + marker = PROBE_HOOK_NAME if live else "bmad-loop" trust = project_hook_trust(project, profile, binary=binary, marker=marker) finding.hook_trust = trust.status if trust.status != "trusted": diff --git a/src/bmad_loop/process_host.py b/src/bmad_loop/process_host.py index 1140315c6..aecdb34f0 100644 --- a/src/bmad_loop/process_host.py +++ b/src/bmad_loop/process_host.py @@ -67,9 +67,8 @@ def identity(self, pid: int) -> float | None: def hook_interpreter(self) -> str: """The command prefix that runs a bmad-loop python hook script on this host, interpolated into the hook registrations `install`/`probe` write - (the script path + canonical event are appended by the caller). POSIX runs - the ``python3`` on PATH; a Windows host overrides it (no ``python3`` there) - so hook registration never branches on ``sys.platform`` at the call site.""" + (the script path + canonical event are appended by the caller). The + prefix is an absolute interpreter path, quoted for the host shell.""" def alive_and_ours(self, pid: int, identity: float | None) -> bool: """Identity-aware liveness: True only when ``pid`` is alive **and** still the @@ -184,7 +183,7 @@ def descendants(self, pid: int) -> dict[int, float | None]: return super().descendants(pid) # macOS: psutil, guarded by the seam's never-raise def hook_interpreter(self) -> str: - return "python3" + return self.shell_quote(str(Path(sys.executable).absolute())) class WindowsProcessHost(ProcessHost): @@ -221,9 +220,7 @@ def identity(self, pid: int) -> float | None: return None def hook_interpreter(self) -> str: - # Windows ships no `python3` launcher; `uv run --no-project python` resolves - # an interpreter without activating a project venv (hooks fire detached). - return "uv run --no-project python" + return self.shell_quote(str(Path(sys.executable).absolute())) def shell_quote(self, arg: str) -> str: # POSIX single-quoting breaks Windows paths; list2cmdline is the stdlib's diff --git a/src/bmad_loop/worktree_flow.py b/src/bmad_loop/worktree_flow.py index 17c328c32..0790322c4 100644 --- a/src/bmad_loop/worktree_flow.py +++ b/src/bmad_loop/worktree_flow.py @@ -30,6 +30,7 @@ from typing import TYPE_CHECKING, Any, Callable, NoReturn from . import artifact_publication, gates, verify +from .adapters.profile import ProfileError from .install import ( _REVIEW_LAYER_SKILLS, BASE_SKILLS, @@ -39,7 +40,6 @@ CENTRAL_CONFIG_REL, DEV_PRIMITIVE_MARKERS, DEV_PRIMITIVE_ROLES, - HOOK_SCRIPT_REL, MERGED_REVIEW_SKILL, MODULE_SKILLS, RENDER_DIR_REL, @@ -48,6 +48,7 @@ RENDERER_SEED_SENTINELS, _absent_renderer_sources, _copy_traversable, + _hook_command, _is_dir, _is_file, _occupied, @@ -63,7 +64,6 @@ ) from .model import Phase from .platform_util import atomic_write_text -from .process_host import get_process_host from .workspace import ( UnitWorkspace, Workspace, @@ -793,9 +793,9 @@ def provision_worktree( seed_files are copied BEFORE the hook step so a seeded settings file that is also a hook config_path (.claude/settings.json, .gemini/settings.json) keeps its real content rather than being created empty. Its relay entry is replaced, not - kept: the seeded copy carries the main repo's $CLAUDE_PROJECT_DIR-relative relay - command, which resolves to the worktree, so the hook step strips it and registers - its own absolute command in its place (#352). A config that is already there but + kept: a seeded copy can carry a legacy workspace relay or stale installed + command, so the hook step replaces it with this installation's absolute + command. A config that is already there but cannot be parsed refuses provisioning outright — `verify.GitError`, which the caller escalates as CRITICAL and pauses the run — rather than being replaced by a hooks-only file: an unparseable config is evidence of an earlier fault, and the @@ -825,7 +825,6 @@ def provision_worktree( "cannot resolve worktree provisioning roots safely " f"(worktree={unresolved_worktree}, repo_root={unresolved_repo_root}): {e}" ) from e - relay = repo_root / HOOK_SCRIPT_REL skills_root = resources.files("bmad_loop.data").joinpath("skills") # project gitignored MCP/CLI configs: copy from the main repo when absent. @@ -1142,18 +1141,17 @@ def provision_worktree( "terminal, so repairing the file alone does not put the story " "back in the run (#592)" ) from e - host = get_process_host() - interp = host.hook_interpreter() - registrations = { - native: f"{interp} {host.shell_quote(str(relay))} {canonical}" - for native, canonical in profile.hooks.events.items() - } + try: + registrations = { + native: _hook_command(repo_root, profile, canonical) + for native, canonical in profile.hooks.events.items() + } + except ProfileError as e: + raise verify.GitError(f"cannot register worktree relay: {e}") from e # A seeded config_path (.claude/settings.json is both a seeded file and the - # hook config) arrives carrying the MAIN repo's relay command, which for the - # claude dialect is $CLAUDE_PROJECT_DIR-relative and resolves to a path that - # does not exist inside the worktree. merge_hooks will not replace an - # already-registered relay, so strip it first and let this registration — - # baked to the main repo's relay, absolute — be authoritative. Strip only on + # hook config) may carry a legacy workspace relay or a stale installed + # command. Strip it first and register this installation's entry point. + # Strip only on # FIRST encounter per config file: profiles can share a config_path # (user-overlay aliases of one CLI), and a later profile's pass must not # tear out the relay events an earlier one just registered — merge_hooks @@ -1164,9 +1162,8 @@ def provision_worktree( stripped_paths.add(config_path) config, _ = merge_hooks(config, registrations, profile.hooks.dialect) # Write — and pin — only when the strip+merge actually changed the parsed - # config. Non-claude dialects bake the absolute main-repo relay at init - # (_hook_command), so a tracked codex/gemini config often arrives already - # carrying exactly the command registered here: strip-then-merge nets to + # config. A tracked config may already carry exactly the installed command, + # so strip-then-merge nets to # zero, and a pin would claim orchestrator ownership of a file this run # never modified, hiding a story's own edit to it for no benefit. if config != baseline_config: diff --git a/tests/test_cli.py b/tests/test_cli.py index 50bd093c4..675a3148a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9674,190 +9674,131 @@ def test_validate_stories_mode_reports_missing_manifest(project, capsys): assert "sprint status" not in text -def test_validate_flags_registered_hooks_with_missing_relay_script(project, capsys): - """#461 papercut: `hooks.registered` is a substring match on the hook config — - it stays green after the relay script itself is gone (branch switch, deleted - `.bmad-loop/`), while every hook event silently no-ops and the run stalls to - session_timeout_min. A distinct finding stats the artifact. - - Ablation guard: deleting the `hooks.relay-present` block in cmd_validate makes - this FAIL on the first assertion.""" - from bmad_loop import install as install_mod +def _validate_json(project_dir, capsys): + rc = cli.main(["validate", "--project", str(project_dir), "--json"]) + out, err = capsys.readouterr() + assert err == "" + return rc, json.loads(out) + + +def test_validate_flags_registered_hooks_with_missing_relay_executable(project, capsys): + from bmad_loop.install import install_into install_bmad_config(project) _write_policy(project.project) - assert cli.main(["init", "--project", str(project.project), "--no-skills"]) == 0 - args = argparse.Namespace(project=str(project.project), spec=None) + assert install_into(project.project, clis=("claude",), skills=False) == 0 + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + missing = project.project / "missing" / "bmad-loop" + data["hooks"]["Stop"][0]["hooks"][0]["command"] = f"{missing} relay Stop" + config.write_text(json.dumps(data)) + capsys.readouterr() - cli.cmd_validate(args) - assert "hook relay script present" in _validate_output(capsys) + _rc, doc = _validate_json(project.project, capsys) + findings = [f for f in doc["findings"] if f["check"] == "hooks.relay-present"] + assert any( + f["severity"] == "problem" and str(missing) in f["message"] and "is missing" in f["message"] + for f in findings + ) + assert any(f["check"] == "hooks.registered" and f["severity"] == "ok" for f in doc["findings"]) - (project.project / install_mod.HOOK_SCRIPT_REL).unlink() - cli.cmd_validate(args) - text = _validate_output(capsys) - # The registration check is untouched — it still reads green, which is exactly - # the blind spot the new finding covers. - assert "hooks registered" in text - assert "relay script" in text and "missing" in text - assert "bmad-loop init" in text +def test_validate_json_reports_null_hook_handlers_without_crashing(project, capsys): + from bmad_loop.install import install_into + + install_bmad_config(project) + _write_policy(project.project) + assert install_into(project.project, clis=("claude",), skills=False) == 0 + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + data["hooks"]["Stop"] = None + config.write_text(json.dumps(data)) + capsys.readouterr() + + _rc, doc = _validate_json(project.project, capsys) + assert any( + finding["check"] == "hooks.config-parse" + and finding["severity"] == "problem" + and "Stop" in finding["message"] + for finding in doc["findings"] + ) @pytest.mark.skipif(os.name == "nt", reason="Windows chmod only toggles the read-only flag") @pytest.mark.skipif( - os.geteuid() == 0 if hasattr(os, "geteuid") else False, reason="root reads a 000 file" + os.geteuid() == 0 if hasattr(os, "geteuid") else False, reason="root executes mode-000 files" ) -def test_validate_flags_registered_hooks_with_an_unreadable_relay_script(project, capsys): - """A relay that exists but cannot be READ is the same stall as a missing one: - the registered command is ` `, which exits 2 - ("can't open file") and never writes the event, so the run waits out - session_timeout_min. `Path.is_file()` is True for a mode-000 file, so - existence alone left that green — the blind spot this check exists to remove. - - The remediation differs from the missing case and is asserted here: `init` - writes the relay with `write_text()`, which needs write access to the same - file, so it raises PermissionError rather than repairing it. - - Ablation guard: dropping the `os.access` branch makes this FAIL — validate - reports the relay present.""" - from bmad_loop import install as install_mod +def test_validate_flags_registered_hooks_with_unusable_relay_executable(project, capsys): + from bmad_loop.install import install_into install_bmad_config(project) _write_policy(project.project) - assert cli.main(["init", "--project", str(project.project), "--no-skills"]) == 0 - args = argparse.Namespace(project=str(project.project), spec=None) - - relay = project.project / install_mod.HOOK_SCRIPT_REL - cli.cmd_validate(args) - assert "hook relay script present" in _validate_output(capsys) - - relay.chmod(0o000) - # The premise, asserted rather than assumed: existence still reads True, which - # is the whole reason readability needs its own check. - assert relay.is_file() and not os.access(relay, os.R_OK) - - cli.cmd_validate(args) - text = _validate_output(capsys) - assert "hooks registered" in text # registration is untouched, still green - assert "relay script" in text and "not readable" in text - # It must NOT tell the operator to run a command that also fails. - assert "chmod" in text - - relay.chmod(0o644) # so the sandbox tears down cleanly - + assert install_into(project.project, clis=("claude",), skills=False) == 0 + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + unusable = project.project / "bmad-loop" + unusable.write_text("#!/bin/sh\nexit 0\n") + unusable.chmod(0o000) + data["hooks"]["Stop"][0]["hooks"][0]["command"] = f"{unusable} relay Stop" + config.write_text(json.dumps(data)) + capsys.readouterr() -def _validate_json(project_dir, capsys): - """`validate --json` as (rc, document). Not `machine_json`: that helper takes - the expected rc as an input, and these tests are ABOUT the rc — the sandbox's - baseline verdict comes from findings that have nothing to do with the relay, - so it must be observed, never asserted.""" - rc = cli.main(["validate", "--project", str(project_dir), "--json"]) - out, err = capsys.readouterr() - assert err == "" # the machine.py purity contract still holds - return rc, json.loads(out) + _rc, doc = _validate_json(project.project, capsys) + assert any( + f["check"] == "hooks.relay-present" + and f["severity"] == "problem" + and str(unusable) in f["message"] + for f in doc["findings"] + ) + unusable.chmod(0o644) -def test_validate_reports_a_fresh_relay_as_up_to_date(project, capsys): - """#494 Phase 4: a relay `init` just wrote matches the packaged source. +def test_validate_inspects_old_registered_script_before_migration(project, capsys): + from bmad_loop.install import install_into - Pinned separately from the stale case because the two arms fail for opposite - reasons — a comparison wrong in the *other* direction (a raw byte compare - against a `write_text`-translated file, which on Windows calls every fresh - install stale) reddens here and nowhere else.""" install_bmad_config(project) _write_policy(project.project) - assert cli.main(["init", "--project", str(project.project), "--no-skills"]) == 0 - args = argparse.Namespace(project=str(project.project), spec=None) - - cli.cmd_validate(args) - text = _validate_output(capsys) - assert "hook relay script up to date" in text - assert "differs from this bmad-loop" not in text - - -def test_validate_warns_when_the_installed_relay_is_stale(project, capsys): - """#494 Phase 4: the relay is COPIED into the project by `init`, so an - upgraded orchestrator drives sessions through whatever version the project - happens to hold — and a pre-#494 relay still writes its events INSIDE the - project tree. `hooks.relay-present` cannot see that: the file is there and - readable, so it reads green. + assert install_into(project.project, clis=("claude",), skills=False) == 0 + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + data["hooks"]["Stop"][0]["hooks"][0][ + "command" + ] = 'python3 "$CLAUDE_PROJECT_DIR"/.bmad-loop/bmad_loop_hook.py Stop' + config.write_text(json.dumps(data)) + capsys.readouterr() - The verdict must not move — the dual-poll fallback keeps a stale relay - working, so this reports a lost property, not a broken run. Asserted as - "unchanged from the same project one edit earlier" rather than a literal 0: - the sandbox's baseline comes from unrelated findings, and hardcoding it would - pin this test to those instead of to the relay. + _rc, doc = _validate_json(project.project, capsys) + legacy = str(project.project / ".bmad-loop/bmad_loop_hook.py") + assert any( + f["check"] == "hooks.relay-present" + and f["severity"] == "problem" + and legacy in f["message"] + for f in doc["findings"] + ) + legacy_path = Path(legacy) + legacy_path.write_text("# readable legacy script\n") + _rc, doc = _validate_json(project.project, capsys) + assert any( + f["check"] == "hooks.relay-present" and f["severity"] == "ok" and legacy in f["message"] + for f in doc["findings"] + ) - Ablation guard: deleting the `hook_script_current` block in cmd_validate (or - forcing it to `True`) makes this FAIL on the `hooks.relay-stale` lookup.""" - from bmad_loop import install as install_mod - install_bmad_config(project) - _write_policy(project.project) - assert cli.main(["init", "--project", str(project.project), "--no-skills"]) == 0 - capsys.readouterr() # drop init's chatter; _validate_json parses the WHOLE stream - - baseline_rc, baseline = _validate_json(project.project, capsys) - - relay = project.project / install_mod.HOOK_SCRIPT_REL - relay.write_text( - relay.read_text(encoding="utf-8") + "\n# an older wheel wrote this\n", encoding="utf-8" - ) - rc, doc = _validate_json(project.project, capsys) - - # The id is the matchable identity (checks.py) — match on it, not the prose. - stale = [f for f in doc["findings"] if f["check"] == "hooks.relay-stale"] - assert len(stale) == 1 - assert stale[0]["severity"] == "warning" - assert stale[0]["detail"]["path"] == str(relay) - assert "bmad-loop init" in stale[0]["message"] - - # The verdict and exit code are untouched by a warning. - assert (rc, doc["ok"]) == (baseline_rc, baseline["ok"]) - # And the two checks it must not be confused with still read exactly as before. - by_id = {f["check"]: f["severity"] for f in doc["findings"]} - assert by_id["hooks.registered"] == "ok" - assert by_id["hooks.relay-present"] == "ok" - - -def test_validate_is_silent_about_relay_staleness_when_hooks_are_unregistered(project, capsys): - """The check hangs off `any_hooks_registered`, like `hooks.relay-present`: a - project not routing events through any relay is not owed a note about which - version of one it holds — it is owed the registration FAIL, and nothing on - top of it. - - The relay is left INSTALLED and STALE on purpose. A test that simply skips - `init` proves nothing: with no relay on disk the comparison degrades to - "unknowable" and emits no finding whether it is gated or not, so the gate - could be deleted outright and such a test would stay green (it was, and it - did). This shape is the one that separates them. - - Ablation guard: hoisting the `hooks.relay-stale` block out of the - `if any_hooks_registered:` block makes this FAIL.""" - from bmad_loop import install as install_mod - from bmad_loop.adapters.profile import load_profiles +def test_validate_ignores_unused_legacy_copy(project, capsys): + from bmad_loop.install import install_into install_bmad_config(project) _write_policy(project.project) - assert cli.main(["init", "--project", str(project.project), "--no-skills"]) == 0 - - # Unregister: keep the relay, drop every registration that points at it. - for profile in load_profiles(project.project).values(): - config = project.project / profile.hooks.config_path - if config.is_file(): - config.write_text("{}\n", encoding="utf-8") - relay = project.project / install_mod.HOOK_SCRIPT_REL - relay.write_text(relay.read_text(encoding="utf-8") + "\n# stale\n", encoding="utf-8") + assert install_into(project.project, clis=("claude",), skills=False) == 0 + legacy = project.project / ".bmad-loop/bmad_loop_hook.py" + legacy.write_text("stale") capsys.readouterr() _rc, doc = _validate_json(project.project, capsys) - # The premise, asserted rather than assumed — a still-registered project would - # make the absence below prove nothing. + assert not [f for f in doc["findings"] if f["check"] == "hooks.relay-stale"] assert any( - f["check"] == "hooks.registered" and f["severity"] == "problem" for f in doc["findings"] + f["check"] == "hooks.relay-present" and f["severity"] == "ok" for f in doc["findings"] ) - assert relay.is_file() # and the stale artifact really is still there - assert not [f for f in doc["findings"] if f["check"] == "hooks.relay-stale"] def test_validate_sprint_mode_still_gates_on_sprint_status(project, capsys): diff --git a/tests/test_codex_trust.py b/tests/test_codex_trust.py index 75ea594da..eb390bb6b 100644 --- a/tests/test_codex_trust.py +++ b/tests/test_codex_trust.py @@ -10,7 +10,7 @@ from conftest import install_bmad_config, write_script_launcher from bmad_loop import cli, codex_trust, probe -from bmad_loop.adapters.profile import get_profile +from bmad_loop.adapters.profile import ProfileError, get_profile from bmad_loop.install import _hook_command, merge_hooks @@ -42,6 +42,53 @@ def _rpc(root: Path, data: dict, status: str = "trusted") -> dict: return {"data": [{"cwd": str(root.resolve()), "errors": [], "warnings": [], "hooks": hooks}]} +@pytest.mark.parametrize("event", ["SessionStart", "Stop"]) +def test_trust_rejects_old_script_command_for_each_required_event(tmp_path, monkeypatch, event): + data = _config(tmp_path) + data["hooks"][event][0]["hooks"][0][ + "command" + ] = f"python3 {tmp_path / '.bmad-loop/bmad_loop_hook.py'} {event}" + (tmp_path / ".codex/hooks.json").write_text(json.dumps(data)) + monkeypatch.setattr( + codex_trust, + "_hooks_list", + lambda *_: pytest.fail("an old command must not be queried as current trust"), + ) + result = codex_trust.project_hook_trust(tmp_path, get_profile("codex")) + assert result.status == "untrusted" and "relay" in result.reason + + +def test_trust_ignores_unrelated_command_containing_bmad_loop(tmp_path, monkeypatch): + data = _config(tmp_path) + data["hooks"]["Stop"][0]["hooks"].append( + {"type": "command", "command": "echo /tools/bmad-loop/notice"} + ) + (tmp_path / ".codex/hooks.json").write_text(json.dumps(data)) + monkeypatch.setattr(codex_trust, "resolved_codex_binary", lambda *_: "codex-stub") + monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: _rpc(tmp_path, data)) + assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "trusted" + + +def test_trust_rejects_installed_relay_at_wrong_path(tmp_path, monkeypatch): + data = _config(tmp_path) + data["hooks"]["Stop"][0]["hooks"][0]["command"] = "/old/bin/bmad-loop relay Stop" + (tmp_path / ".codex/hooks.json").write_text(json.dumps(data)) + monkeypatch.setattr(codex_trust, "_hooks_list", lambda *_: pytest.fail("wrong relay path")) + assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "untrusted" + + +def test_trust_degrades_when_installed_relay_disappears(tmp_path, monkeypatch): + _config(tmp_path) + + def missing(*_args): + raise ProfileError("installed bmad-loop command is unavailable") + + monkeypatch.setattr(codex_trust, "_hook_command", missing) + result = codex_trust.project_hook_trust(tmp_path, get_profile("codex")) + assert result.status == "unverifiable" + assert "installed relay unavailable" in result.reason + + def test_scripted_app_server_executes_request_sequence_and_reads_environment(tmp_path): """An actual zero-token child parses initialize and hooks/list, not a mocked RPC.""" data = _config(tmp_path) @@ -145,9 +192,9 @@ def test_trust_refuses_relay_for_old_checkout_and_startup_excluding_matcher(tmp_ assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "untrusted" data["hooks"]["SessionStart"][0].pop("matcher") - data["hooks"]["Stop"][0]["hooks"][0]["command"] = _hook_command( - tmp_path / "old-checkout", get_profile("codex"), "Stop" - ) + data["hooks"]["Stop"][0]["hooks"][0][ + "command" + ] = f"python3 {tmp_path / 'old-checkout/.bmad-loop/bmad_loop_hook.py'} Stop" (tmp_path / ".codex/hooks.json").write_text(json.dumps(data), encoding="utf-8") assert codex_trust.project_hook_trust(tmp_path, get_profile("codex")).status == "untrusted" @@ -312,6 +359,7 @@ def trust(path, _profile, *, binary=None, marker=None): cli="codex", profile=profile, project=tmp_path, hints=probe.Hints(binary="chosen") ) assert scanned.hook_trust == "untrusted" and calls[-1][:2] == (tmp_path, "chosen") + assert calls[-1][2] == "bmad-loop" class Mux: def available(self): diff --git a/tests/test_hook_script.py b/tests/test_hook_script.py index 3c81351a3..4b6be8ddc 100644 --- a/tests/test_hook_script.py +++ b/tests/test_hook_script.py @@ -410,9 +410,27 @@ def test_a_symlinked_env_directed_events_dir_writes_nothing_and_exits_zero(tmp_p assert not (run_dir / "events").exists() -def test_installed_copy_matches_source(tmp_path): +def test_installed_command_writes_without_workspace_script(tmp_path): from bmad_loop.install import install_into - install_into(tmp_path) - installed = (tmp_path / ".bmad-loop" / "bmad_loop_hook.py").read_text() - assert installed == SCRIPT.read_text() + assert install_into(tmp_path, skills=False) == 0 + assert not (tmp_path / ".bmad-loop" / "bmad_loop_hook.py").exists() + config = json.loads((tmp_path / ".claude/settings.json").read_text()) + command = config["hooks"]["Stop"][0]["hooks"][0]["command"] + proc = subprocess.run( + command, + shell=True, + cwd=tmp_path, + env={ + "PATH": "/usr/bin:/bin", + "BMAD_LOOP_RUN_DIR": str(tmp_path), + "BMAD_LOOP_TASK_ID": "t1", + }, + input=json.dumps({"session_id": "s1"}), + text=True, + capture_output=True, + timeout=10, + ) + assert proc.returncode == 0 and proc.stdout == "" + event = json.loads(next((tmp_path / "events").glob("*.json")).read_text()) + assert event["event"] == "Stop" and event["task_id"] == "t1" diff --git a/tests/test_install.py b/tests/test_install.py index 832962e08..23131535e 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -56,6 +56,7 @@ missing_base_skills, missing_stories_support, provision_worktree, + relay_executable, renderer_stub_resolved, resolve_dev_primitive, resolve_review_layers, @@ -173,6 +174,182 @@ def test_merge_hooks_preserves_existing(): assert any("bmad_loop_hook" in c for c in commands) +def test_init_migrates_legacy_relay_and_preserves_user_hook(tmp_path): + profile = get_profile("claude") + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + config.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + { + "type": "command", + "command": "python3 /old/.bmad-loop/bmad_loop_hook.py Stop", + }, + {"type": "command", "command": "make lint"}, + ] + } + ] + } + } + ) + ) + assert install_into(tmp_path, skills=False) == 0 + migrated = config.read_bytes() + commands = [ + hook["command"] + for group in json.loads(migrated)["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert commands.count("make lint") == 1 + assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + assert all("bmad_loop_hook.py" not in command for command in commands) + assert install_into(tmp_path, skills=False) == 0 + assert config.read_bytes() == migrated + + +def test_init_does_not_treat_mention_of_old_script_as_managed(tmp_path): + profile = get_profile("claude") + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + user_command = "echo bmad_loop_hook.py Stop" + config.write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "command", "command": user_command}]}]}}) + ) + assert install_into(tmp_path, skills=False) == 0 + commands = [ + hook["command"] + for group in json.loads(config.read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert user_command in commands + assert any(command.endswith("bmad-loop relay Stop") for command in commands) + + +@pytest.mark.parametrize( + "old_command", + [ + 'uv run --no-project python "$CLAUDE_PROJECT_DIR"/.bmad-loop/bmad_loop_hook.py Stop', + r"uv run --no-project python C:\work\.bmad-loop\bmad_loop_hook.py Stop", + ], +) +def test_init_migrates_windows_legacy_relay_on_posix(tmp_path, old_command): + profile = get_profile("claude") + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + config.write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "command", "command": old_command}]}]}}) + ) + assert install_into(tmp_path, skills=False) == 0 + commands = [ + hook["command"] + for group in json.loads(config.read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert old_command not in commands + assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + + +def test_init_preserves_different_script_with_same_basename(tmp_path): + profile = get_profile("claude") + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + user_command = "python3 /user/tools/bmad_loop_hook.py Stop" + config.write_text( + json.dumps({"hooks": {"Stop": [{"hooks": [{"type": "command", "command": user_command}]}]}}) + ) + assert install_into(tmp_path, skills=False) == 0 + commands = [ + hook["command"] + for group in json.loads(config.read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert user_command in commands + assert any(command.endswith("bmad-loop relay Stop") for command in commands) + + +@pytest.mark.parametrize("name,container", [("copilot", "hooks"), ("antigravity", "bmad-loop")]) +def test_init_migrates_flat_legacy_hook_and_preserves_user(tmp_path, name, container): + profile = get_profile(name) + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + user = {"type": "command", "command": "echo mine"} + old = {"type": "command", "command": "python3 /old/.bmad-loop/bmad_loop_hook.py Stop"} + native_stop = next(native for native, event in profile.hooks.events.items() if event == "Stop") + config.write_text(json.dumps({container: {native_stop: [old, user]}})) + assert install_into(tmp_path, clis=(name,), skills=False) == 0 + current = json.loads(config.read_text())[container][native_stop] + commands = [item["command"] for item in current] + assert commands.count("echo mine") == 1 + assert old["command"] not in commands + assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + + +def test_init_removes_stale_relay_beside_current_relay(tmp_path): + profile = get_profile("claude") + assert install_into(tmp_path, skills=False) == 0 + config = tmp_path / profile.hooks.config_path + data = json.loads(config.read_text()) + data["hooks"]["Stop"].append( + { + "hooks": [ + {"type": "command", "command": "python3 /old/.bmad-loop/bmad_loop_hook.py Stop"} + ] + } + ) + config.write_text(json.dumps(data)) + assert install_into(tmp_path, skills=False) == 0 + commands = [ + hook["command"] + for group in json.loads(config.read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert len(commands) == 1 and commands[0].endswith("bmad-loop relay Stop") + + +def test_init_refuses_missing_installed_command(tmp_path, monkeypatch, capsys): + monkeypatch.setattr(install_mod.sys, "executable", str(tmp_path / "missing" / "python")) + assert install_into(tmp_path, skills=False) == 1 + assert "installed bmad-loop command is unavailable" in capsys.readouterr().out + assert not (tmp_path / ".claude/settings.json").exists() + + +def test_hook_command_uses_invoked_non_sibling_launcher(tmp_path, monkeypatch): + launcher = tmp_path / "user bin" / ("bmad-loop.exe" if os.name == "nt" else "bmad-loop") + launcher.parent.mkdir() + launcher.write_text("#!/bin/sh\n") + launcher.chmod(0o755) + monkeypatch.setattr(install_mod.sys, "argv", [str(launcher)]) + command = install_mod._hook_command(tmp_path, get_profile("claude"), "Stop") + assert shlex.split(command)[0] == str(launcher) + + +def test_hook_command_refuses_unreadable_executable(tmp_path, monkeypatch): + launcher = tmp_path / ("bmad-loop.exe" if os.name == "nt" else "bmad-loop") + launcher.write_text("#!/bin/sh\n") + launcher.chmod(0o111) + monkeypatch.setattr(install_mod.sys, "argv", [str(launcher)]) + real_access = install_mod.os.access + + def access(path, mode): + if Path(path) == launcher and mode == os.R_OK | os.X_OK: + return False + return real_access(path, mode) + + monkeypatch.setattr(install_mod.os, "access", access) + with pytest.raises(ProfileError, match="installed bmad-loop command is unavailable"): + install_mod._hook_command(tmp_path, get_profile("claude"), "Stop") + + +def test_provision_worktree_refuses_missing_installed_command(tmp_path, monkeypatch): + monkeypatch.setattr(install_mod.sys, "executable", str(tmp_path / "missing" / "python")) + with pytest.raises(verify.GitError, match="cannot register worktree relay"): + provision_worktree(tmp_path / "wt", [get_profile("claude")], tmp_path / "repo") + + def test_merge_hooks_gemini_entry_shape(): profile = get_profile("gemini") settings, _ = merge_hooks({}, _registrations(profile), profile.hooks.dialect) @@ -344,7 +521,7 @@ def test_install_into_copilot(tmp_path): assert set(settings["hooks"]) == {"agentStop", "sessionStart", "sessionEnd"} cmd = settings["hooks"]["agentStop"][0]["command"] # absolute path baked in (no $CLAUDE_PROJECT_DIR equivalent in copilot) - assert str(tmp_path.resolve()) in cmd and cmd.endswith(" Stop") + assert "bmad-loop relay Stop" in cmd and str(tmp_path.resolve()) not in cmd # skills land in the shared .agents/skills tree for skill in MODULE_SKILLS: assert (tmp_path / ".agents" / "skills" / skill / "SKILL.md").is_file() @@ -357,12 +534,13 @@ def test_install_into_copilot(tmp_path): def test_install_into_full(tmp_path): assert install_into(tmp_path) == 0 - assert (tmp_path / ".bmad-loop" / "bmad_loop_hook.py").is_file() + assert not (tmp_path / ".bmad-loop" / "bmad_loop_hook.py").exists() assert (tmp_path / ".bmad-loop" / "policy.toml").is_file() settings = json.loads((tmp_path / ".claude" / "settings.json").read_text()) assert "Stop" in settings["hooks"] gitignore = (tmp_path / ".gitignore").read_text() assert ".bmad-loop/runs/" in gitignore + assert ".bmad-loop/cache/" in gitignore # engine plugins' rebuildable caches assert ".bmad-loop/policy.toml" in gitignore # per-machine config ([mux] backend) assert f"{RENDER_DIR_REL}/" in gitignore # regenerated, checkout-absolute renderer output @@ -384,6 +562,28 @@ def test_install_into_full(tmp_path): assert final_gitignore.count(f"{RENDER_DIR_REL}/") == 1 +@pytest.mark.parametrize("name", ["claude", "codex", "gemini", "copilot", "antigravity"]) +def test_fresh_init_registers_installed_command_for_each_dialect(tmp_path, name): + profile = get_profile(name) + assert install_into(tmp_path, clis=(name,), skills=False) == 0 + data = json.loads((tmp_path / profile.hooks.config_path).read_text()) + from bmad_loop.install import hook_event_container + + container = hook_event_container(data, profile.hooks.dialect) + for native, canonical in profile.hooks.events.items(): + handlers = container[native] + command = ( + handlers[0]["command"] + if profile.hooks.dialect in {"copilot-settings-json", "antigravity-hooks-json"} + else handlers[0]["hooks"][0]["command"] + ) + assert command.endswith(f"bmad-loop relay {canonical}") + executable = relay_executable(command) + assert executable is not None and executable.is_absolute() + assert str(tmp_path) not in command + assert not (tmp_path / ".bmad-loop/bmad_loop_hook.py").exists() + + def test_install_into_warns_when_policy_is_tracked(tmp_path, capsys): """A .gitignore entry doesn't untrack an already-committed policy.toml: upgrading repos get the one-time `git rm --cached` hint.""" @@ -452,7 +652,7 @@ def test_hook_command_uses_selected_process_host(tmp_path, monkeypatch): assert install_into(tmp_path) == 0 settings = json.loads((tmp_path / ".claude" / "settings.json").read_text()) cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] - assert cmd.startswith("uv run --no-project python ") + assert cmd.endswith("bmad-loop relay Stop") finally: monkeypatch.delenv("BMAD_LOOP_PROCESS_HOST", raising=False) get_process_host.cache_clear() @@ -465,7 +665,7 @@ def test_install_into_multiple_clis(tmp_path): assert set(codex_hooks["hooks"]) == {"SessionStart", "Stop"} cmd = codex_hooks["hooks"]["Stop"][0]["hooks"][0]["command"] # absolute path (no $CLAUDE_PROJECT_DIR equivalent in codex/gemini) - assert str(tmp_path.resolve()) in cmd and cmd.endswith(" Stop") + assert "bmad-loop relay Stop" in cmd and str(tmp_path.resolve()) not in cmd gemini_settings = json.loads((tmp_path / ".gemini" / "settings.json").read_text()) assert set(gemini_settings["hooks"]) == {"SessionStart", "AfterAgent", "SessionEnd"} @@ -536,8 +736,20 @@ def test_provision_worktree_lays_down_skills_and_hook(tmp_path): settings = json.loads((wt / claude.hooks.config_path).read_text()) assert set(claude.hooks.events) <= set(settings["hooks"]) cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] - assert str((repo / ".bmad-loop" / "bmad_loop_hook.py")) in cmd + assert cmd.endswith("bmad-loop relay Stop") assert not (wt / ".bmad-loop").exists() + delivered = subprocess.run( + cmd, + shell=True, + cwd=wt, + env={"PATH": "/usr/bin:/bin", "BMAD_LOOP_RUN_DIR": str(wt), "BMAD_LOOP_TASK_ID": "t1"}, + input=json.dumps({"session_id": "worktree-1"}), + text=True, + capture_output=True, + timeout=10, + ) + assert delivered.returncode == 0 and delivered.stdout == "" + assert json.loads(next((wt / "events").glob("*.json")).read_text())["event"] == "Stop" def test_provision_worktree_rewrites_seeded_relative_hook_to_absolute(tmp_path): @@ -550,6 +762,13 @@ def test_provision_worktree_rewrites_seeded_relative_hook_to_absolute(tmp_path): repo.mkdir() claude = get_profile("claude") assert _register_hooks(repo, claude) == 0 + settings_path = repo / claude.hooks.config_path + settings = json.loads(settings_path.read_text()) + for event, groups in settings["hooks"].items(): + groups[0]["hooks"][0][ + "command" + ] = f'python3 "$CLAUDE_PROJECT_DIR"/.bmad-loop/bmad_loop_hook.py {event}' + settings_path.write_text(json.dumps(settings)) main_settings = json.loads((repo / claude.hooks.config_path).read_text()) assert "$CLAUDE_PROJECT_DIR" in main_settings["hooks"]["Stop"][0]["hooks"][0]["command"] @@ -558,7 +777,7 @@ def test_provision_worktree_rewrites_seeded_relative_hook_to_absolute(tmp_path): stop = json.loads((wt / claude.hooks.config_path).read_text())["hooks"]["Stop"] assert len(stop) == 1 # replaced, not appended alongside cmd = stop[0]["hooks"][0]["command"] - assert str(repo / ".bmad-loop" / "bmad_loop_hook.py") in cmd + assert cmd.endswith("bmad-loop relay Stop") assert "$CLAUDE_PROJECT_DIR" not in cmd @@ -575,7 +794,12 @@ def test_provision_worktree_tracked_config_rewrite_stays_out_of_commits(project, claude = get_profile("claude") hook_rel = claude.hooks.config_path assert _register_hooks(repo, claude) == 0 - assert "$CLAUDE_PROJECT_DIR" in (repo / hook_rel).read_text(encoding="utf-8") + settings = json.loads((repo / hook_rel).read_text()) + for event, groups in settings["hooks"].items(): + groups[0]["hooks"][0][ + "command" + ] = f'python3 "$CLAUDE_PROJECT_DIR"/.bmad-loop/bmad_loop_hook.py {event}' + (repo / hook_rel).write_text(json.dumps(settings)) git(repo, "add", "-A") git(repo, "commit", "-q", "-m", "track the hook config") wt = tmp_path / "wt" @@ -586,7 +810,7 @@ def test_provision_worktree_tracked_config_rewrite_stays_out_of_commits(project, cmd = json.loads((wt / hook_rel).read_text(encoding="utf-8"))["hooks"]["Stop"][0]["hooks"][0][ "command" ] - assert str(repo / ".bmad-loop" / "bmad_loop_hook.py") in cmd + assert cmd.endswith("bmad-loop relay Stop") assert "$CLAUDE_PROJECT_DIR" not in cmd git(wt, "add", "-A") assert hook_rel not in git(wt, "diff", "--cached", "--name-only").splitlines() @@ -641,7 +865,7 @@ def test_provision_worktree_shared_config_path_keeps_first_profiles_events(tmp_p hooks = json.loads((wt / claude.hooks.config_path).read_text(encoding="utf-8"))["hooks"] assert "Stop" in hooks # claude's completion event survived the alias pass assert "Notification" in hooks # and the alias still merged its own event in - relay = str(repo / ".bmad-loop" / "bmad_loop_hook.py") + relay = "bmad-loop relay" assert relay in hooks["Stop"][0]["hooks"][0]["command"] assert relay in hooks["Notification"][0]["hooks"][0]["command"] @@ -655,6 +879,12 @@ def test_provision_worktree_tracked_pin_failure_raises(project, tmp_path): repo = project.project claude = get_profile("claude") assert _register_hooks(repo, claude) == 0 + config = repo / claude.hooks.config_path + settings = json.loads(config.read_text()) + settings["hooks"]["Stop"][0]["hooks"][0][ + "command" + ] = 'python3 "$CLAUDE_PROJECT_DIR"/.bmad-loop/bmad_loop_hook.py Stop' + config.write_text(json.dumps(settings)) git(repo, "add", "-A") git(repo, "commit", "-q", "-m", "track the hook config") wt = tmp_path / "wt" @@ -681,14 +911,22 @@ def test_strip_relay_hooks_leaves_foreign_handlers(tmp_path): "Stop": [ { "matcher": "", - "hooks": [{"type": "command", "command": "python bmad_loop_hook.py Stop"}], + "hooks": [ + { + "type": "command", + "command": "python /proj/.bmad-loop/bmad_loop_hook.py Stop", + } + ], }, {"matcher": "", "hooks": [{"type": "command", "command": "make lint"}]}, probe, { "matcher": "", "hooks": [ - {"type": "command", "command": "python bmad_loop_hook.py Stop"}, + { + "type": "command", + "command": "python /proj/.bmad-loop/bmad_loop_hook.py Stop", + }, {"type": "command", "command": "make fmt"}, ], }, @@ -696,7 +934,12 @@ def test_strip_relay_hooks_leaves_foreign_handlers(tmp_path): "SessionStart": [ { "matcher": "", - "hooks": [{"type": "command", "command": "python bmad_loop_hook.py start"}], + "hooks": [ + { + "type": "command", + "command": "python /proj/.bmad-loop/bmad_loop_hook.py start", + } + ], } ], } @@ -886,7 +1129,7 @@ def test_register_hooks_merge_preserves_the_operators_own_settings(tmp_path): assert set(claude.hooks.events) <= set(hooks) # names the relay marker the write-failure row asserts ABSENT, so that row's # negative is graded against a string this path provably produces - assert "bmad_loop_hook" in hooks["Stop"][0]["hooks"][0]["command"] + assert "bmad-loop relay Stop" in hooks["Stop"][0]["hooks"][0]["command"] def test_a_truncated_hook_config_makes_the_next_init_refuse(tmp_path, capsys): @@ -1106,7 +1349,7 @@ def test_provision_worktree_merge_preserves_the_operators_own_settings(tmp_path) assert _operator_keys(config) == before hooks = json.loads(config.read_text(encoding="utf-8"))["hooks"] assert set(claude.hooks.events) <= set(hooks) - assert str(repo / ".bmad-loop" / "bmad_loop_hook.py") in hooks["Stop"][0]["hooks"][0]["command"] + assert "bmad-loop relay Stop" in hooks["Stop"][0]["hooks"][0]["command"] def test_provision_worktree_write_failure_raises_and_leaves_the_config_entire( diff --git a/tests/test_process_host.py b/tests/test_process_host.py index abebe030b..0536e5683 100644 --- a/tests/test_process_host.py +++ b/tests/test_process_host.py @@ -7,6 +7,7 @@ import subprocess import sys import time +from pathlib import Path import pytest @@ -364,16 +365,20 @@ def test_shell_quote_windows_uses_list2cmdline(): assert quoted == '"C:\\a b\\c.py"' -def test_hook_interpreter_is_python3_on_posix(host): - # Hook registrations (install/probe) interpolate this prefix; POSIX keeps the - # historical `python3` byte-for-byte so existing configs stay valid. - assert PosixProcessHost().hook_interpreter() == "python3" +def test_hook_interpreter_is_absolute_on_posix(host): + import sys + assert PosixProcessHost().hook_interpreter() == PosixProcessHost().shell_quote( + str(Path(sys.executable).absolute()) + ) -def test_hook_interpreter_windows_resolves_without_project_venv(): - # Windows has no `python3` launcher — `uv run` resolves an interpreter, and - # `--no-project` keeps it from activating a project venv for a detached hook. - assert WindowsProcessHost().hook_interpreter() == "uv run --no-project python" + +def test_hook_interpreter_windows_uses_absolute_path(): + import sys + + assert WindowsProcessHost().hook_interpreter() == WindowsProcessHost().shell_quote( + str(Path(sys.executable).absolute()) + ) def test_hook_interpreter_routed_through_selected_host(monkeypatch): @@ -381,4 +386,8 @@ def test_hook_interpreter_routed_through_selected_host(monkeypatch): # registered hook command with no `sys.platform` branch at the call site. monkeypatch.setenv("BMAD_LOOP_PROCESS_HOST", "windows") get_process_host.cache_clear() - assert get_process_host().hook_interpreter() == "uv run --no-project python" + import sys + + assert get_process_host().hook_interpreter() == WindowsProcessHost().shell_quote( + str(Path(sys.executable).absolute()) + ) From dcb880f1afa12c3d1e5d56762d5a759ec1ab5df3 Mon Sep 17 00:00:00 2001 From: t Date: Mon, 21 Sep 2026 21:35:05 -0700 Subject: [PATCH 2/6] Validate relay paths even with malformed hook events --- src/bmad_loop/cli.py | 3 ++- tests/test_cli.py | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index b9850d98e..37d473aa6 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -741,12 +741,13 @@ def cmd_validate(args: argparse.Namespace) -> int: f"{hook_config} is not valid JSON", {"profile": profile.name, "config_path": str(hook_config)}, ) - if hooks_ok: + if isinstance(parsed, dict): registered_relay_paths.update( install.registered_relay_paths( parsed, profile.hooks.dialect, profile.hooks.events, project ) ) + if hooks_ok: report.ok( "hooks.registered", f"bmad-loop hooks registered for {profile.name}", diff --git a/tests/test_cli.py b/tests/test_cli.py index 675a3148a..9ea6d0bcc 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9724,6 +9724,33 @@ def test_validate_json_reports_null_hook_handlers_without_crashing(project, caps ) +def test_validate_checks_valid_relay_when_another_event_is_malformed(project, capsys): + from bmad_loop.install import install_into + + install_bmad_config(project) + _write_policy(project.project) + assert install_into(project.project, clis=("claude",), skills=False) == 0 + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + missing = project.project / "missing" / "bmad-loop" + data["hooks"]["Stop"][0]["hooks"][0]["command"] = f"{missing} relay Stop" + data["hooks"]["SessionStart"] = None + config.write_text(json.dumps(data)) + capsys.readouterr() + + _rc, doc = _validate_json(project.project, capsys) + assert any( + finding["check"] == "hooks.config-parse" and finding["severity"] == "problem" + for finding in doc["findings"] + ) + assert any( + finding["check"] == "hooks.relay-present" + and finding["severity"] == "problem" + and str(missing) in finding["message"] + for finding in doc["findings"] + ) + + @pytest.mark.skipif(os.name == "nt", reason="Windows chmod only toggles the read-only flag") @pytest.mark.skipif( os.geteuid() == 0 if hasattr(os, "geteuid") else False, reason="root executes mode-000 files" From c2cf029c7a77bead7f5a61d74752f003ffdb704c Mon Sep 17 00:00:00 2001 From: t Date: Mon, 21 Sep 2026 22:20:35 -0700 Subject: [PATCH 3/6] Fix Windows installed relay assertions --- tests/test_install.py | 54 ++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/tests/test_install.py b/tests/test_install.py index 23131535e..e85875e33 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -135,6 +135,12 @@ def _is_unset(args): return any(a.startswith("--unset") for a in args) +def _installed_relay_suffix(event: str) -> str: + """Expected installed entry point and canonical relay event on this host.""" + name = "bmad-loop.exe" if os.name == "nt" else "bmad-loop" + return f"{name} relay {event}" + + def _registrations(profile, command="python3 /x/.bmad-loop/bmad_loop_hook.py {event}"): return { native: command.format(event=canonical) @@ -205,7 +211,7 @@ def test_init_migrates_legacy_relay_and_preserves_user_hook(tmp_path): for hook in group["hooks"] ] assert commands.count("make lint") == 1 - assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 assert all("bmad_loop_hook.py" not in command for command in commands) assert install_into(tmp_path, skills=False) == 0 assert config.read_bytes() == migrated @@ -226,7 +232,7 @@ def test_init_does_not_treat_mention_of_old_script_as_managed(tmp_path): for hook in group["hooks"] ] assert user_command in commands - assert any(command.endswith("bmad-loop relay Stop") for command in commands) + assert any(command.endswith(_installed_relay_suffix("Stop")) for command in commands) @pytest.mark.parametrize( @@ -250,7 +256,7 @@ def test_init_migrates_windows_legacy_relay_on_posix(tmp_path, old_command): for hook in group["hooks"] ] assert old_command not in commands - assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 def test_init_preserves_different_script_with_same_basename(tmp_path): @@ -268,7 +274,7 @@ def test_init_preserves_different_script_with_same_basename(tmp_path): for hook in group["hooks"] ] assert user_command in commands - assert any(command.endswith("bmad-loop relay Stop") for command in commands) + assert any(command.endswith(_installed_relay_suffix("Stop")) for command in commands) @pytest.mark.parametrize("name,container", [("copilot", "hooks"), ("antigravity", "bmad-loop")]) @@ -285,7 +291,7 @@ def test_init_migrates_flat_legacy_hook_and_preserves_user(tmp_path, name, conta commands = [item["command"] for item in current] assert commands.count("echo mine") == 1 assert old["command"] not in commands - assert sum(command.endswith("bmad-loop relay Stop") for command in commands) == 1 + assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 def test_init_removes_stale_relay_beside_current_relay(tmp_path): @@ -307,7 +313,7 @@ def test_init_removes_stale_relay_beside_current_relay(tmp_path): for group in json.loads(config.read_text())["hooks"]["Stop"] for hook in group["hooks"] ] - assert len(commands) == 1 and commands[0].endswith("bmad-loop relay Stop") + assert len(commands) == 1 and commands[0].endswith(_installed_relay_suffix("Stop")) def test_init_refuses_missing_installed_command(tmp_path, monkeypatch, capsys): @@ -521,7 +527,7 @@ def test_install_into_copilot(tmp_path): assert set(settings["hooks"]) == {"agentStop", "sessionStart", "sessionEnd"} cmd = settings["hooks"]["agentStop"][0]["command"] # absolute path baked in (no $CLAUDE_PROJECT_DIR equivalent in copilot) - assert "bmad-loop relay Stop" in cmd and str(tmp_path.resolve()) not in cmd + assert _installed_relay_suffix("Stop") in cmd and str(tmp_path.resolve()) not in cmd # skills land in the shared .agents/skills tree for skill in MODULE_SKILLS: assert (tmp_path / ".agents" / "skills" / skill / "SKILL.md").is_file() @@ -577,7 +583,7 @@ def test_fresh_init_registers_installed_command_for_each_dialect(tmp_path, name) if profile.hooks.dialect in {"copilot-settings-json", "antigravity-hooks-json"} else handlers[0]["hooks"][0]["command"] ) - assert command.endswith(f"bmad-loop relay {canonical}") + assert command.endswith(_installed_relay_suffix(canonical)) executable = relay_executable(command) assert executable is not None and executable.is_absolute() assert str(tmp_path) not in command @@ -652,7 +658,7 @@ def test_hook_command_uses_selected_process_host(tmp_path, monkeypatch): assert install_into(tmp_path) == 0 settings = json.loads((tmp_path / ".claude" / "settings.json").read_text()) cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] - assert cmd.endswith("bmad-loop relay Stop") + assert cmd.endswith(_installed_relay_suffix("Stop")) finally: monkeypatch.delenv("BMAD_LOOP_PROCESS_HOST", raising=False) get_process_host.cache_clear() @@ -665,7 +671,7 @@ def test_install_into_multiple_clis(tmp_path): assert set(codex_hooks["hooks"]) == {"SessionStart", "Stop"} cmd = codex_hooks["hooks"]["Stop"][0]["hooks"][0]["command"] # absolute path (no $CLAUDE_PROJECT_DIR equivalent in codex/gemini) - assert "bmad-loop relay Stop" in cmd and str(tmp_path.resolve()) not in cmd + assert _installed_relay_suffix("Stop") in cmd and str(tmp_path.resolve()) not in cmd gemini_settings = json.loads((tmp_path / ".gemini" / "settings.json").read_text()) assert set(gemini_settings["hooks"]) == {"SessionStart", "AfterAgent", "SessionEnd"} @@ -736,13 +742,22 @@ def test_provision_worktree_lays_down_skills_and_hook(tmp_path): settings = json.loads((wt / claude.hooks.config_path).read_text()) assert set(claude.hooks.events) <= set(settings["hooks"]) cmd = settings["hooks"]["Stop"][0]["hooks"][0]["command"] - assert cmd.endswith("bmad-loop relay Stop") + assert cmd.endswith(_installed_relay_suffix("Stop")) assert not (wt / ".bmad-loop").exists() + env = {"PATH": "", "BMAD_LOOP_RUN_DIR": str(wt), "BMAD_LOOP_TASK_ID": "t1"} + if os.name == "nt": + env.update( + { + key: os.environ[key] + for key in ("SystemRoot", "WINDIR", "COMSPEC") + if key in os.environ + } + ) delivered = subprocess.run( cmd, shell=True, cwd=wt, - env={"PATH": "/usr/bin:/bin", "BMAD_LOOP_RUN_DIR": str(wt), "BMAD_LOOP_TASK_ID": "t1"}, + env=env, input=json.dumps({"session_id": "worktree-1"}), text=True, capture_output=True, @@ -777,7 +792,7 @@ def test_provision_worktree_rewrites_seeded_relative_hook_to_absolute(tmp_path): stop = json.loads((wt / claude.hooks.config_path).read_text())["hooks"]["Stop"] assert len(stop) == 1 # replaced, not appended alongside cmd = stop[0]["hooks"][0]["command"] - assert cmd.endswith("bmad-loop relay Stop") + assert cmd.endswith(_installed_relay_suffix("Stop")) assert "$CLAUDE_PROJECT_DIR" not in cmd @@ -810,7 +825,7 @@ def test_provision_worktree_tracked_config_rewrite_stays_out_of_commits(project, cmd = json.loads((wt / hook_rel).read_text(encoding="utf-8"))["hooks"]["Stop"][0]["hooks"][0][ "command" ] - assert cmd.endswith("bmad-loop relay Stop") + assert cmd.endswith(_installed_relay_suffix("Stop")) assert "$CLAUDE_PROJECT_DIR" not in cmd git(wt, "add", "-A") assert hook_rel not in git(wt, "diff", "--cached", "--name-only").splitlines() @@ -865,9 +880,10 @@ def test_provision_worktree_shared_config_path_keeps_first_profiles_events(tmp_p hooks = json.loads((wt / claude.hooks.config_path).read_text(encoding="utf-8"))["hooks"] assert "Stop" in hooks # claude's completion event survived the alias pass assert "Notification" in hooks # and the alias still merged its own event in - relay = "bmad-loop relay" - assert relay in hooks["Stop"][0]["hooks"][0]["command"] - assert relay in hooks["Notification"][0]["hooks"][0]["command"] + assert hooks["Stop"][0]["hooks"][0]["command"].endswith(_installed_relay_suffix("Stop")) + assert hooks["Notification"][0]["hooks"][0]["command"].endswith( + _installed_relay_suffix("Notification") + ) def test_provision_worktree_tracked_pin_failure_raises(project, tmp_path): @@ -1129,7 +1145,7 @@ def test_register_hooks_merge_preserves_the_operators_own_settings(tmp_path): assert set(claude.hooks.events) <= set(hooks) # names the relay marker the write-failure row asserts ABSENT, so that row's # negative is graded against a string this path provably produces - assert "bmad-loop relay Stop" in hooks["Stop"][0]["hooks"][0]["command"] + assert _installed_relay_suffix("Stop") in hooks["Stop"][0]["hooks"][0]["command"] def test_a_truncated_hook_config_makes_the_next_init_refuse(tmp_path, capsys): @@ -1349,7 +1365,7 @@ def test_provision_worktree_merge_preserves_the_operators_own_settings(tmp_path) assert _operator_keys(config) == before hooks = json.loads(config.read_text(encoding="utf-8"))["hooks"] assert set(claude.hooks.events) <= set(hooks) - assert "bmad-loop relay Stop" in hooks["Stop"][0]["hooks"][0]["command"] + assert _installed_relay_suffix("Stop") in hooks["Stop"][0]["hooks"][0]["command"] def test_provision_worktree_write_failure_raises_and_leaves_the_config_entire( From ac1873f4ab71af231ea0a21bc7e862ab42f8a8fd Mon Sep 17 00:00:00 2001 From: t Date: Mon, 21 Sep 2026 23:06:59 -0700 Subject: [PATCH 4/6] Recognize installed relay hooks across operating systems --- CHANGELOG.md | 2 + src/bmad_loop/install.py | 39 ++++++----- tests/test_install.py | 137 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0d4fa0d..15189e7e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ breaking changes may land in a minor release. ### Fixed +- 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` before a live probe launches; check both relay events against Codex's read-only hook discovery for the operation's directory and executable (#461). diff --git a/src/bmad_loop/install.py b/src/bmad_loop/install.py index 8673d03c6..68a86ca12 100644 --- a/src/bmad_loop/install.py +++ b/src/bmad_loop/install.py @@ -28,7 +28,7 @@ from contextlib import ExitStack from importlib import resources from importlib.resources.abc import Traversable -from pathlib import Path, PurePosixPath +from pathlib import Path, PurePosixPath, PureWindowsPath from typing import Any, NamedTuple from .adapters.profile import ALIASES, CLIProfile, ProfileError, load_profiles @@ -1022,21 +1022,28 @@ def _hook_command(project: Path, profile: CLIProfile, canonical_event: str) -> s def relay_executable(command: str) -> Path | None: - """Return the absolute executable named by an installed relay registration.""" - try: - parts = shlex.split(command, posix=os.name != "nt") - except ValueError: - return None - if ( - len(parts) != 3 - or parts[1] != "relay" - or parts[2] not in {"SessionStart", "Stop", "SessionEnd", "Notification", "PreCompact"} - ): - return None - executable = Path(parts[0].strip('"')) - if executable.name not in {"bmad-loop", "bmad-loop.exe"} or not executable.is_absolute(): - return None - return executable + """Return the executable in an absolute POSIX or Windows relay command. + + A foreign path remains unusable on this host, but must still be recognized + as managed so init and worktree provisioning replace its stale hook. + """ + for posix in (os.name != "nt", os.name == "nt"): + try: + parts = shlex.split(command, posix=posix) + except ValueError: + continue + if ( + len(parts) != 3 + or parts[1] != "relay" + or parts[2] not in {"SessionStart", "Stop", "SessionEnd", "Notification", "PreCompact"} + ): + continue + raw = parts[0].strip('"') + for flavor in (PurePosixPath, PureWindowsPath): + executable = flavor(raw) + if executable.name in {"bmad-loop", "bmad-loop.exe"} and executable.is_absolute(): + return Path(raw) + return None def _legacy_relay_script(command: str) -> str | None: diff --git a/tests/test_install.py b/tests/test_install.py index e85875e33..b9ba4ff2c 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -56,6 +56,7 @@ missing_base_skills, missing_stories_support, provision_worktree, + registered_relay_paths, relay_executable, renderer_stub_resolved, resolve_dev_primitive, @@ -259,6 +260,72 @@ def test_init_migrates_windows_legacy_relay_on_posix(tmp_path, old_command): assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 +@pytest.mark.parametrize( + "stale_command", + [ + r"C:\old\bin\bmad-loop.exe relay Stop", + "/old/bin/bmad-loop relay Stop", + r'"C:\Program Files\bmad-loop\bmad-loop.exe" relay Stop', + "'/opt/bmad loop/bin/bmad-loop' relay Stop", + ], + ids=["windows", "posix", "windows-quoted", "posix-quoted"], +) +def test_init_replaces_installed_relay_from_either_os(tmp_path, stale_command): + profile = get_profile("claude") + config = tmp_path / profile.hooks.config_path + config.parent.mkdir(parents=True) + config.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": stale_command}, + {"type": "command", "command": "make lint"}, + ] + } + ] + } + } + ) + ) + + assert install_into(tmp_path, skills=False) == 0 + commands = [ + hook["command"] + for group in json.loads(config.read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert commands.count("make lint") == 1 + assert stale_command not in commands + assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 + assert len(commands) == 2 + + +@pytest.mark.parametrize( + "stale_command,expected_path", + [ + (r"C:\old\bin\bmad-loop.exe relay Stop", r"C:\old\bin\bmad-loop.exe"), + ("/old/bin/bmad-loop relay Stop", "/old/bin/bmad-loop"), + ], + ids=["windows", "posix"], +) +def test_registered_relay_paths_reads_installed_relay_from_either_os( + tmp_path, stale_command, expected_path +): + config = { + "hooks": { + "Stop": [ + {"hooks": [{"type": "command", "command": stale_command}]}, + {"hooks": [{"type": "command", "command": "make lint"}]}, + ] + } + } + paths = registered_relay_paths(config, "claude-settings-json", ["Stop"], tmp_path) + assert [str(path).replace("\\", "/") for path in paths] == [expected_path.replace("\\", "/")] + + def test_init_preserves_different_script_with_same_basename(tmp_path): profile = get_profile("claude") config = tmp_path / profile.hooks.config_path @@ -796,6 +863,50 @@ def test_provision_worktree_rewrites_seeded_relative_hook_to_absolute(tmp_path): assert "$CLAUDE_PROJECT_DIR" not in cmd +@pytest.mark.parametrize( + "stale_command", + [ + r"C:\old\bin\bmad-loop.exe relay Stop", + "/old/bin/bmad-loop relay Stop", + ], + ids=["windows", "posix"], +) +def test_provision_worktree_replaces_seeded_relay_from_either_os(tmp_path, stale_command): + wt, repo = tmp_path / "wt", tmp_path / "repo" + repo.mkdir() + profile = get_profile("claude") + config = repo / profile.hooks.config_path + config.parent.mkdir(parents=True) + config.write_text( + json.dumps( + { + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": stale_command}, + {"type": "command", "command": "make lint"}, + ] + } + ] + } + } + ) + ) + + provision_worktree(wt, [profile], repo, seed_files=[profile.hooks.config_path]) + + commands = [ + hook["command"] + for group in json.loads((wt / profile.hooks.config_path).read_text())["hooks"]["Stop"] + for hook in group["hooks"] + ] + assert commands.count("make lint") == 1 + assert stale_command not in commands + assert sum(command.endswith(_installed_relay_suffix("Stop")) for command in commands) == 1 + assert len(commands) == 2 + + def test_provision_worktree_tracked_config_rewrite_stays_out_of_commits(project, tmp_path): """A project that TRACKS its hook config still gets the relay rewrite — the checkout carries the same stale $CLAUDE_PROJECT_DIR command a seeded copy @@ -971,6 +1082,32 @@ def test_strip_relay_hooks_leaves_foreign_handlers(tmp_path): assert strip_relay_hooks(config, "claude-settings-json") is False +@pytest.mark.parametrize( + "stale_command", + [ + r"C:\old\bin\bmad-loop.exe relay Stop", + "/old/bin/bmad-loop relay Stop", + ], + ids=["windows", "posix"], +) +def test_strip_relay_hooks_removes_installed_relay_from_either_os(stale_command): + config = { + "hooks": { + "Stop": [ + { + "hooks": [ + {"type": "command", "command": stale_command}, + {"type": "command", "command": "make lint"}, + ] + } + ] + } + } + + assert strip_relay_hooks(config, "claude-settings-json") is True + assert config["hooks"]["Stop"] == [{"hooks": [{"type": "command", "command": "make lint"}]}] + + def test_provision_worktree_covers_multiple_profiles(tmp_path): """Dev=claude + review=codex provisions both skill trees (.claude/skills and .agents/skills) and both hook configs.""" From f75acdb42468252aea9040888a1a8358adc27468 Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 08:39:11 -0700 Subject: [PATCH 5/6] Warn when registered relay points to another installation --- CHANGELOG.md | 2 +- src/bmad_loop/cli.py | 21 ++++++++++++++++++ tests/test_cli.py | 51 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15189e7e1..b999ba5fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ breaking changes may land in a minor release. - Register hooks through the installed `bmad-loop relay ` command. Upgrading invalidates Codex hook trust: Codex re-prompts at the next launch, and hooks silently do not fire until the new commands are accepted. Re-run `bmad-loop init` to migrate - managed registrations. + managed registrations. `validate` warns when a hook still points to another installation. - Document the live-session removal guard's measured ceiling (#732): `delete`, `archive` and `clean` still remove a run directory when a listing omits a live session. Behavior unchanged; the psmux half is reported upstream (psmux/psmux#622), its retirement tracked in #754. ### Fixed diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 37d473aa6..a324630de 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -806,6 +806,19 @@ def cmd_validate(args: argparse.Namespace) -> int: # Inspect the executable each managed registration actually names. A new # installation in this process cannot repair an older path in a hook config. + # Compare with the command init would write now: an old executable can remain + # usable after switching installations, while still running an outdated relay. + expected_relay = None + if registered_relay_paths: + hook_profile = next(profile for profile in profiles if not profile.hookless) + try: + expected_relay = install.relay_executable( + install._hook_command(project, hook_profile, "Stop") + ) + except ProfileError: + # No current executable to compare. The registered path still gets + # its own presence check below; do not call it stale by inference. + pass for relay in sorted(registered_relay_paths): if not relay.is_file(): report.fail( @@ -827,6 +840,14 @@ def cmd_validate(args: argparse.Namespace) -> int: f"registered hook executable available: {relay}", {"path": str(relay)}, ) + if expected_relay is not None and relay != expected_relay: + report.warn( + "hooks.relay-stale", + f"registered hook executable {relay} differs from this " + f"installation's {expected_relay} — re-run `bmad-loop init` " + "to update the hook registration", + {"path": str(relay), "expected_path": str(expected_relay)}, + ) # Adapter-kind validity is enforced against the LIVE registry, never a # hardcoded set: a profile.adapter naming no registered kind is a config error diff --git a/tests/test_cli.py b/tests/test_cli.py index 9ea6d0bcc..ed7070552 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -9703,6 +9703,51 @@ def test_validate_flags_registered_hooks_with_missing_relay_executable(project, assert any(f["check"] == "hooks.registered" and f["severity"] == "ok" for f in doc["findings"]) +def test_validate_warns_when_registered_relay_uses_another_installation( + project, capsys, monkeypatch +): + """An old but runnable console script must not make validation look current.""" + from bmad_loop import install as install_mod + + _make_validate_pass(project, monkeypatch, capsys) + config = project.project / ".claude/settings.json" + data = json.loads(config.read_text()) + current_command = data["hooks"]["Stop"][0]["hooks"][0]["command"] + current = install_mod.relay_executable(current_command) + assert current is not None + + previous = project.project / "previous-install" / "bmad-loop" + previous.parent.mkdir() + previous.write_text("#!/bin/sh\nexit 0\n", encoding="utf-8") + previous.chmod(0o755) + data["hooks"]["Stop"][0]["hooks"][0]["command"] = f"{previous} relay Stop" + config.write_text(json.dumps(data), encoding="utf-8") + git(project.project, "add", "-A") + git(project.project, "commit", "-q", "-m", "keep earlier hook registration") + + rc, doc = _validate_json(project.project, capsys) + assert rc == 0 # stale registration is advisory while the relay remains usable + findings = [f for f in doc["findings"] if f["check"] == "hooks.relay-stale"] + assert findings == [ + { + "check": "hooks.relay-stale", + "severity": "warning", + "message": ( + f"registered hook executable {previous} differs from this " + f"installation's {current} — re-run `bmad-loop init` " + "to update the hook registration" + ), + "detail": {"path": str(previous), "expected_path": str(current)}, + } + ] + assert any( + f["check"] == "hooks.relay-present" + and f["severity"] == "ok" + and f["detail"]["path"] == str(previous) + for f in doc["findings"] + ) + + def test_validate_json_reports_null_hook_handlers_without_crashing(project, capsys): from bmad_loop.install import install_into @@ -9809,6 +9854,12 @@ def test_validate_inspects_old_registered_script_before_migration(project, capsy f["check"] == "hooks.relay-present" and f["severity"] == "ok" and legacy in f["message"] for f in doc["findings"] ) + assert any( + f["check"] == "hooks.relay-stale" + and f["severity"] == "warning" + and f["detail"]["path"] == legacy + for f in doc["findings"] + ) def test_validate_ignores_unused_legacy_copy(project, capsys): From f61f9d616e810c5e7aac6510665bcef54aa2020d Mon Sep 17 00:00:00 2001 From: t Date: Tue, 22 Sep 2026 09:06:51 -0700 Subject: [PATCH 6/6] docs: narrow legacy relay removal guidance --- docs/setup-guide.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/setup-guide.md b/docs/setup-guide.md index e2d5a109d..71c515e76 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -400,8 +400,10 @@ that also cleans up a pre-0.7.0 install.) `init` **merged** its Stop-hook registration into each CLI's existing hook config, so these files must be **edited, not deleted** (they hold your own settings too). In each config below, -remove the hook entry whose `command` ends in `bmad-loop relay ` (or -contains `bmad_loop_hook.py` on older installs): +remove the hook entry whose `command` ends in `bmad-loop relay ` (or, on +older installs, is a Python or `uv run --no-project python` command whose script +path ends in `.bmad-loop/bmad_loop_hook.py` and whose event argument matches +that hook event): - **claude** — `.claude/settings.json` - **codex** — `.codex/hooks.json`