From 230a571a2e1cd207838750b091ae73024df49fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAlberto?= Date: Thu, 30 Jul 2026 18:12:43 +0200 Subject: [PATCH 1/5] feat(adapter): override the CLI binary a session spawns (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A profile's binary was fixed, so running the same CLI under a second name — a work account launched as `cc` beside a personal `claude`, or two aliases splitting usage limits — meant copying the whole packaged profile into .bmad-loop/profiles/ to change one field. That copy then freezes bypass_args, [env], seed_files and env_fault_patterns against upstream forever: the cost of one field is permanent drift on eight others. Add `binary` to [adapter] and [adapter.]. Empty (the default) keeps the profile's own binary, so existing projects are untouched. Both adapter families already accepted a `binary` argument and resolved it as `binary or profile.binary`; policy is what was never plumbed into it, so make_adapters passes the resolved value and the seam does the rest. Inheritance follows the model/extra_args rule: client-specific, inherited from the base only when the stage runs the same client, so a `cc` alias of claude can never become argv[0] for a stage that switched to codex. Two read paths had to follow the effective binary rather than the profile's: validate's PATH probe (it would report "claude found" for a run that spawns a typo'd `cc`) and the --dry-run invocation render. The override is for an alias or wrapper of the same CLI — the profile still supplies the hook dialect, config path and transcript layout. --- CHANGELOG.md | 10 ++++ docs/FEATURES.md | 1 + src/bmad_loop/cli.py | 28 ++++++++-- src/bmad_loop/data/settings/core.toml | 10 ++++ src/bmad_loop/policy.py | 72 +++++++++++++++++++++--- src/bmad_loop/runsetup.py | 4 ++ tests/test_cli.py | 54 ++++++++++++++++++ tests/test_policy.py | 79 +++++++++++++++++++++++++++ 8 files changed, 245 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97fbb2d9..87ca5127 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,16 @@ breaking changes may land in a minor release. ### Added +- **`[adapter] binary` spawns a CLI under a custom name (#395).** Set `binary = "cc"` (base or + per-stage) to drive a second subscription or a work/personal account that lives beside the + default install, without forking the whole profile to change one field — a copy freezes its + `bypass_args`, `[env]`, `seed_files` and `env_fault_patterns` against upstream. Empty (the + default) keeps the profile's own binary, so nothing changes for existing projects. The override + is for an alias or wrapper of the _same_ CLI: the profile still supplies the hook dialect, config + path and transcript layout. Client-specific like `model`/`extra_args` — a stage that also switches + `name` falls back to that profile's binary. `validate` probes the effective executable (a missing + `cc` reports as `cc`, not as a missing `claude`) and `--dry-run` prints it. + - **A park travels with its story's commit, so `bmad-loop confirm` works from any clone (#356).** Each parked story now writes one committed JSON record to `.bmad-loop/operator/.json`, inside the story's own commit window, so the record rides the park's commit — through the worktree diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 1e0e7596..ebdfd381 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -141,6 +141,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - Supported, E2E-verified over HTTP/SSE (no tmux window): `opencode` (OpenCode ≥ 1.18, profile `opencode-http`, alias `opencode`) — one headless `opencode serve` per session, SSE `session.idle` completion with an HTTP poll fallback, per-session server password, token usage read back over the API. Hookless (`[hooks] dialect = "none"`, no hook registration). With no pane to replay, the run logs split three ways: a curated readable transcript in `logs/.log` (agent/user prose, tool calls, slash commands, file edits, permission asks/replies, errors), the server's own stdout in `.server.out`, and a structured SSE trace in `.sse.jsonl`. Install the extra (`pip install 'bmad-loop[opencode]'`), auth once globally (`opencode auth login`), and set `model` as `provider/model`; the Unity plugin's window guards don't apply (there is no window). - Experimental, `isolation = "none"` only: `antigravity` (Google's `agy` ≥ 1.1.3) — `-i` interactive launch, `Stop` turn-end hook (flat handler in `.agents/hooks.json`, no SessionStart/SessionEnd), `--dangerously-skip-permissions` for unattended runs; `usage_parser = "none"` permanently — agy's transcript exposes no usage data (tokens live only in an internal SQLite/protobuf store). `agy` gates each workspace on an exact-path `trustedWorkspaces` entry and blocks on an interactive trust dialog, which `--dangerously-skip-permissions` does not bypass — so worktree isolation hangs ([#169](https://github.com/bmad-code-org/bmad-loop/issues/169)). Verify against your `agy` build with `probe-adapter antigravity`. - Per-stage CLI/model overrides: run dev on one CLI/model, review on another (`[adapter.dev]`, `[adapter.review]`, `[adapter.triage]`). +- Custom executable per project (`[adapter] binary`, per-stage too): spawn the same CLI under another name — a second subscription or a work/personal account reachable as `cc` beside `claude`, or two aliases used to split usage limits — without forking the profile (a copied profile freezes its `bypass_args`, `[env]`, `seed_files` and `env_fault_patterns` against upstream). It must be an alias or wrapper of the **same** CLI: the profile still supplies the hook dialect, config path and transcript layout. Empty = the profile's own binary. `validate` probes the overridden name, and `--dry-run` prints it. `binary` is client-specific, so a stage that also switches `name` falls back to that profile's binary instead of inheriting. - Add a CLI without touching Python: drop a TOML profile in `.bmad-loop/profiles/.toml` (binary, prompt template, bypass flags, hook dialect, native→canonical event map). - `bmad-loop probe-adapter` collects + sanitizes the data needed to finalize/add a profile (hook payload shape, transcript location/format, token schema): a zero-launch scan by default, opt-in `--probe` for live capture. See the [adapter authoring guide](adapter-authoring-guide.md). diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 1afd313f..f470eb63 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -209,6 +209,11 @@ def cmd_validate(args: argparse.Namespace) -> int: profiles = [] profile_by_name: dict[str, CLIProfile] = {} + # Effective executable -> was it an [adapter] binary override. Keyed by the + # binary actually spawned, not by profile: a `binary` override means the + # profile's own name is NOT what runs, and probing it would report "claude + # found" for a run that will spawn `cc`. + binaries: dict[str, bool] = {} pol = None try: pol = policy_mod.load(_policy_path(project)) @@ -227,6 +232,12 @@ def cmd_validate(args: argparse.Namespace) -> int: profile_by_name[name] = profile except ProfileError as e: report.fail("adapter.profile", str(e), {"profile": name}) + for role in ROLES: + cfg = pol.adapter.resolved(role) + profile = profile_by_name.get(cfg.name) + if profile is not None: + tool = cfg.binary or profile.binary + binaries[tool] = binaries.get(tool, False) or bool(cfg.binary) except policy_mod.PolicyError as e: report.fail("policy", str(e)) @@ -291,11 +302,15 @@ def cmd_validate(args: argparse.Namespace) -> int: {"platform": sys.platform}, ) - for tool in dict.fromkeys(p.binary for p in profiles): + for tool, overridden in binaries.items(): + # Name the override in the message: a missing `cc` is a typo'd alias, not + # a missing Claude Code install, and the fix differs. + note = " (binary override)" if overridden else "" + detail = {"binary": tool, "override": overridden} if shutil.which(tool): - report.ok("adapter.binary", f"{tool} found", {"binary": tool}) + report.ok("adapter.binary", f"{tool} found{note}", detail) else: - report.fail("adapter.binary", f"{tool} not found on PATH", {"binary": tool}) + report.fail("adapter.binary", f"{tool} not found on PATH{note}", detail) for profile in profiles: if profile.hookless: @@ -944,19 +959,22 @@ def _render_invocation(pol, project: Path, role: str, prompt: str) -> str: cfg = pol.adapter.resolved(role) profile = get_profile(cfg.name, project) + # The dry-run plan must name the executable the run would really spawn, so a + # binary override is visible before any session starts. + binary = cfg.binary or profile.binary if profile.hookless: # HTTP/SSE transport — there is no shell invocation to print. Render # the real sequence (per-session server spawn + API prompt) instead of # a fake argv that run would never execute. model = f" model={cfg.model}" if cfg.model else "" return ( - f"{profile.binary} serve --hostname 127.0.0.1 --port " + f"{binary} serve --hostname 127.0.0.1 --port " f'(cwd=) → POST /session → prompt_async "{profile.render_prompt(prompt)}"' f"{model}" ) extra = cfg.extra_args if cfg.extra_args is not None else profile.bypass_args argv = [ - profile.binary, + binary, *profile.launch_args, f'"{profile.render_prompt(prompt)}"', *extra, diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index 124c8d18..44252348 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -213,6 +213,11 @@ key = "model" kind = "str" placeholder = "CLI default model" [[section.field]] +key = "binary" +kind = "str" +placeholder = "the profile's own binary" +description = "executable to spawn — an alias or wrapper of the same CLI (e.g. `cc` for a second Claude account); the profile still supplies the hook dialect and config path" +[[section.field]] key = "extra_args" kind = "args" [[section.field]] @@ -249,6 +254,11 @@ key = "model" kind = "str" placeholder = "inherit / client default" [[section.field]] +key = "binary" +kind = "str" +placeholder = "inherit / profile binary" +description = "executable for this stage only — inherits from [adapter] when the stage runs the same client" +[[section.field]] key = "extra_args" kind = "args" [[section.field]] diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index edb5aed4..87cb9695 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -329,6 +329,9 @@ class StageAdapterPolicy: name: str | None = None model: str | None = None extra_args: tuple[str, ...] | None = None + # None = inherit from [adapter]; "" explicitly clears a base override back to + # the CLI profile's own binary (same empty-means-default rule as model) + binary: str | None = None # None = inherit from [adapter] (which itself falls back to the CLI profile) usage_grace_s: float | None = None stop_without_result_nudges: int | None = None @@ -340,6 +343,11 @@ class ResolvedAdapter: model: str # None = use the profile's default bypass flags; a list replaces them extra_args: tuple[str, ...] | None + # "" = spawn the CLI profile's own binary; a value replaces it (an alias or + # wrapper of the SAME CLI — the profile's hook dialect, config_path and + # transcript layout still apply). Trails extra_args so the historical + # positional ResolvedAdapter(name, model, extra_args) construction still holds. + binary: str = "" # None = fall back to the CLI profile's default (usage_grace_s) / the global # limits.stop_without_result_nudges respectively usage_grace_s: float | None = None @@ -350,6 +358,14 @@ class ResolvedAdapter: class AdapterPolicy: name: str = "claude" # CLI profile name; "claude-code-tmux" kept as legacy alias model: str = "" + # "" = spawn the CLI profile's shipped binary. A value replaces it so a + # second subscription/account reachable under another name (`cc` beside + # `claude`) needs no forked profile — copying a packaged profile to change + # this one field would freeze its bypass_args/[env]/seed_files/ + # env_fault_patterns against upstream forever. The override assumes an alias + # or wrapper of the SAME CLI: the profile still supplies the hook dialect, + # hooks.config_path and transcript layout. + binary: str = "" # None = use the profile's default bypass flags; a list replaces them extra_args: tuple[str, ...] | None = None # kill the run's bmad-loop- tmux session when it finishes (False keeps @@ -370,13 +386,16 @@ def resolved(self, role: str) -> ResolvedAdapter: self.name, self.model, self.extra_args, - self.usage_grace_s, - self.stop_without_result_nudges, + binary=self.binary, + usage_grace_s=self.usage_grace_s, + stop_without_result_nudges=self.stop_without_result_nudges, ) name = stage.name if stage.name is not None else self.name - # model and extra_args are client-specific: inherit from the base only - # when the stage runs the same client; a client switch falls back to - # that profile's defaults (CLI default model, profile bypass flags). + # model, extra_args and binary are client-specific: inherit from the base + # only when the stage runs the same client; a client switch falls back to + # that profile's defaults (CLI default model, profile bypass flags, the + # profile's own binary — a `cc` alias of claude must never be spawned as + # the executable for a stage that switched to codex). same_client = name == self.name # usage_grace_s / stop_without_result_nudges are benign timing knobs that # mean "fall back to the profile default" when None, so plain stage ?? @@ -389,6 +408,9 @@ def resolved(self, role: str) -> ResolvedAdapter: if stage.extra_args is not None else (self.extra_args if same_client else None) ), + binary=( + stage.binary if stage.binary is not None else (self.binary if same_client else "") + ), usage_grace_s=( stage.usage_grace_s if stage.usage_grace_s is not None else self.usage_grace_s ), @@ -416,10 +438,12 @@ def _stage_from_snapshot(raw: Any) -> StageAdapterPolicy: return StageAdapterPolicy() name = raw.get("name") model = raw.get("model") + binary = raw.get("binary") return StageAdapterPolicy( name=None if name is None else str(name), model=None if model is None else str(model), extra_args=_snapshot_extra_args(raw.get("extra_args")), + binary=None if binary is None else str(binary), usage_grace_s=raw.get("usage_grace_s"), stop_without_result_nudges=raw.get("stop_without_result_nudges"), ) @@ -454,6 +478,7 @@ def adapter_policy_from_snapshot(snapshot: dict[str, Any] | None) -> AdapterPoli name=name, model=str(adapter_d.get("model", AdapterPolicy.model)), extra_args=_snapshot_extra_args(adapter_d.get("extra_args")), + binary=str(adapter_d.get("binary", AdapterPolicy.binary)), cleanup_session_on_finish=bool( adapter_d.get("cleanup_session_on_finish", AdapterPolicy.cleanup_session_on_finish) ), @@ -608,6 +633,28 @@ def _opt_nudges(d: dict[str, Any], where: str) -> int | None: return value +def _adapter_binary(d: dict[str, Any], where: str) -> str | None: + """An [adapter]/[adapter.] ``binary`` override. Missing -> None (the + caller turns that into "" for the base table and "inherit" for a stage). + + Strict about the value because it becomes argv[0] of the spawned session: a + non-string would coerce to something unspawnable, and a blank-but-present + value (" ") reads as "I set this" while behaving as unset. Both raise + rather than degrade. Surrounding whitespace is trimmed; interior whitespace + is allowed so a path like "/Applications/My Tools/cc" works — the value is a + single executable, not a command line (session flags belong in extra_args). + """ + if "binary" not in d: + return None + raw = d["binary"] + if not isinstance(raw, str): + raise PolicyError(f"{where}.binary must be a string: got {raw!r}") + value = raw.strip() + if raw and not value: + raise PolicyError(f"{where}.binary must not be blank — omit it to use the profile's binary") + return value + + def _tui_dim(d: dict[str, Any], key: str) -> int: """A persisted TUI pane dimension (cells). 0 = unset; negatives are rejected. Strict like scm.max_parallel: a TOML bool or float would coerce silently and @@ -629,6 +676,7 @@ def _stage_adapter(adapter_d: dict[str, Any], key: str) -> StageAdapterPolicy: name=None if raw.get("name") is None else str(raw["name"]), model=None if raw.get("model") is None else str(raw["model"]), extra_args=None if raw_extra is None else tuple(str(a) for a in raw_extra), + binary=_adapter_binary(raw, f"adapter.{key}"), usage_grace_s=_opt_grace(raw, f"adapter.{key}"), stop_without_result_nudges=_opt_nudges(raw, f"adapter.{key}"), ) @@ -871,6 +919,7 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: name=str(adapter_d.get("name", AdapterPolicy.name)), model=str(adapter_d.get("model", AdapterPolicy.model)), extra_args=None if raw_extra is None else tuple(str(a) for a in raw_extra), + binary=_adapter_binary(adapter_d, "adapter") or AdapterPolicy.binary, cleanup_session_on_finish=bool( adapter_d.get("cleanup_session_on_finish", AdapterPolicy.cleanup_session_on_finish) ), @@ -1126,6 +1175,12 @@ def _fold_deprecated_engine( [adapter] name = "claude" # claude | codex | gemini | copilot | antigravity | opencode-http (alias: opencode) | model = "" # empty = CLI default model (opencode-http wants "provider/model") +# Executable to spawn. Empty = the profile's own binary. Set it to run the same +# CLI under another name — a second subscription or a work/personal account +# reachable as `cc` beside `claude` — without forking the whole profile. It must +# be an alias or wrapper of the SAME CLI: the profile still supplies the hook +# dialect, config path and transcript layout. +binary = "" cleanup_session_on_finish = true # kill the run's tmux session when it finishes (false keeps it for inspection) # extra_args replaces the profile's default permission-bypass flags when set: # extra_args = ["--permission-mode", "bypassPermissions"] @@ -1137,11 +1192,12 @@ def _fold_deprecated_engine( # Per-stage overrides for the dev, review and sweep-triage passes. Unset keys # inherit from [adapter] when the stage runs the same client; a stage that -# switches client falls back to that profile's defaults instead (model and -# extra_args are client-specific). Stage tables must come after the [adapter] -# keys above. +# switches client falls back to that profile's defaults instead (model, +# extra_args and binary are client-specific). Stage tables must come after the +# [adapter] keys above. # [adapter.dev] # model = "opus" +# binary = "cc" # e.g. bill dev sessions to a second account # [adapter.review] # name = "codex" # model = "gpt-5-codex" diff --git a/src/bmad_loop/runsetup.py b/src/bmad_loop/runsetup.py index b79ffaf1..38bf4368 100644 --- a/src/bmad_loop/runsetup.py +++ b/src/bmad_loop/runsetup.py @@ -100,6 +100,7 @@ def make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIAd run_dir=run_dir, policy=policy, profile=profile, + binary=cfg.binary or None, extra_args=cfg.extra_args, usage_grace_s=cfg.usage_grace_s, stop_without_result_nudges=cfg.stop_without_result_nudges, @@ -129,10 +130,13 @@ def make_adapters(project: Path, run_dir: Path, policy) -> dict[str, CodingCLIAd "missing, the version is unsupported, or a required helper is " "absent (psmux needs `pwsh` on PATH); see `bmad-loop diagnose`" ) + # binary: "" means "the profile's own", which is exactly what the + # adapters' `binary or profile.binary` already does with None. common = dict( run_dir=run_dir, policy=policy, profile=profile, + binary=cfg.binary or None, extra_args=cfg.extra_args, usage_grace_s=cfg.usage_grace_s, stop_without_result_nudges=cfg.stop_without_result_nudges, diff --git a/tests/test_cli.py b/tests/test_cli.py index c1d2a3e4..ccfeee15 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1510,6 +1510,34 @@ def test_make_adapters_review_synthesizes_from_spec(project, monkeypatch): assert not isinstance(adapters["triage"], GenericDevAdapter) +def test_make_adapters_wires_the_policy_binary_override(project, monkeypatch): + """[adapter] binary reaches the adapter that spawns the session, per stage. + The adapters have always accepted a `binary` argument; policy is what was + never plumbed into it, so this is the seam the feature actually turns on.""" + monkeypatch.setattr(mux_mod, "_usable", lambda mux: True) + install_bmad_config(project) + _write_policy(project.project, '[adapter]\nbinary = "cc"\n[adapter.triage]\nbinary = ""\n') + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + adapters = cli._make_adapters( + project.project, project.project / ".bmad-loop" / "runs" / "r", pol + ) + assert adapters["dev"].binary == "cc" + assert adapters["review"].binary == "cc" + # an explicit empty stage value clears the override back to the profile binary + assert adapters["triage"].binary == "claude" + # differing binaries must not collapse onto one shared adapter + assert adapters["triage"] is not adapters["dev"] + + +def test_make_adapters_without_an_override_keeps_the_profile_binary(project, monkeypatch): + monkeypatch.setattr(mux_mod, "_usable", lambda mux: True) + install_bmad_config(project) + adapters = cli._make_adapters( + project.project, project.project / ".bmad-loop" / "runs" / "r", policy_mod.load(None) + ) + assert all(adapters[role].binary == "claude" for role in cli.ROLES) + + def test_make_adapters_hookless_synthesizing_roles_get_dev_adapter(project, monkeypatch): """Hookless dev/review (bmad-dev-auto roles) dispatch to OpencodeDevAdapter — the _DevSynthesisMixin composed over the HTTP transport — sharing one @@ -3742,6 +3770,32 @@ def test_validate_model_warning_ignores_tmux_profiles(project, capsys): assert "is not 'provider/model'" not in _validate_output(capsys) +def test_validate_probes_the_overridden_binary_not_the_profile_one(project, capsys, monkeypatch): + """The PATH probe must follow the binary the run will really spawn. Probing + the profile's own name would report "claude found" for a run that spawns a + typo'd `cc` alias and then dies at session start.""" + install_bmad_config(project) + _write_policy(project.project, '[adapter]\nbinary = "cc-not-installed"\n') + monkeypatch.setattr(cli.shutil, "which", lambda tool: None if "cc" in tool else "/usr/bin/x") + args = argparse.Namespace(project=str(project.project), spec=None) + + cli.cmd_validate(args) + text = _validate_output(capsys) + assert "cc-not-installed not found on path (binary override)" in text + assert "claude not found" not in text # the profile binary is never probed + + +def test_validate_probes_the_profile_binary_without_an_override(project, capsys, monkeypatch): + install_bmad_config(project) + monkeypatch.setattr(cli.shutil, "which", lambda tool: None) + args = argparse.Namespace(project=str(project.project), spec=None) + + cli.cmd_validate(args) + text = _validate_output(capsys) + assert "claude not found on path" in text + assert "binary override" not in text + + def test_validate_stories_mode_skips_sprint_gate(project, capsys): """Item 8: a stories-mode project (no sprint-status.yaml) validates its stories.yaml manifest instead of failing on the missing sprint gate.""" diff --git a/tests/test_policy.py b/tests/test_policy.py index c2b0805b..d5e500bd 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -214,6 +214,83 @@ def test_adapter_timing_knobs_base_and_per_stage(tmp_path): assert dev.stop_without_result_nudges is None +def test_adapter_binary_defaults_to_the_profile_binary(): + # unset means "" everywhere, which make_adapters turns into None so the + # adapters keep spawning CLIProfile.binary — the pre-feature behavior. + pol = policy.load(None) + assert pol.adapter.binary == "" + assert all(pol.adapter.resolved(role).binary == "" for role in ("dev", "review", "triage")) + + +def test_adapter_binary_base_reaches_every_stage(tmp_path): + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +binary = "cc" +""") + pol = policy.load(p) + assert pol.adapter.binary == "cc" + assert all(pol.adapter.resolved(role).binary == "cc" for role in ("dev", "review", "triage")) + + +def test_adapter_binary_per_stage_override(tmp_path): + # the work account drives dev; review stays on the default `claude` install + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +binary = "cc" +[adapter.review] +binary = "" +""") + pol = policy.load(p) + assert pol.adapter.resolved("dev").binary == "cc" + # an explicit empty stage value clears the base override (the model rule) + assert pol.adapter.resolved("review").binary == "" + + +def test_stage_client_switch_drops_base_binary(tmp_path): + """binary is client-specific: a `cc` alias of claude must never become argv[0] + for a stage that switched to codex, which would spawn the wrong CLI entirely.""" + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +name = "claude" +binary = "cc" +[adapter.review] +name = "codex" +""") + review = policy.load(p).adapter.resolved("review") + assert review == policy.ResolvedAdapter("codex", "", None, binary="") + + +def test_adapter_binary_is_trimmed(tmp_path): + p = tmp_path / "policy.toml" + p.write_text('[adapter]\nbinary = " cc "\n') + assert policy.load(p).adapter.binary == "cc" + + +@pytest.mark.parametrize( + "body", + [ + '[adapter]\nbinary = " "\n', # present but blank: reads as set, behaves as unset + '[adapter.dev]\nbinary = "\\t"\n', + ], +) +def test_blank_adapter_binary_is_rejected(body): + with pytest.raises(policy.PolicyError, match="must not be blank"): + policy.loads(body) + + +@pytest.mark.parametrize( + "body", + ["[adapter]\nbinary = 7\n", "[adapter.review]\nbinary = true\n"], +) +def test_non_string_adapter_binary_is_rejected(body): + # it becomes argv[0]; a coerced int or bool would be unspawnable + with pytest.raises(policy.PolicyError, match="must be a string"): + policy.loads(body) + + def _roundtrip_snapshot(pol): # RunState.policy_snapshot is the json-round-tripped asdict(Policy). return json.loads(json.dumps(pol.to_dict())) @@ -229,6 +306,8 @@ def _roundtrip_snapshot(pol): # (c) a stage name override — the client switch resets model to "" '[adapter]\nname = "claude"\nmodel = "opus"\n' 'extra_args = ["--permission-mode", "plan"]\n[adapter.review]\nname = "codex"\n', + # (d) a binary override, base + an explicit stage clear + '[adapter]\nname = "claude"\nbinary = "cc"\n[adapter.review]\nbinary = ""\n', ], ) def test_adapter_policy_from_snapshot_roundtrips_resolved(body): From 348aa3618628a2c8ccc0a68dea2f11a8bd343baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAlberto?= Date: Thu, 30 Jul 2026 18:26:01 +0200 Subject: [PATCH 2/5] feat(cli): add --cli and --cli-binary to run and sweep (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picking the client or the account for a single run meant editing policy.toml, which is project-scoped state — awkward when alternating work and personal accounts on one repo, or when one account hits its usage limit mid-sprint. Both flags fold into the policy before the run composes, not into the engine, so the RunState snapshot records what actually ran: resume, the --json adapter projection, per-task stamping and --dry-run all follow with no further wiring. A flag is a whole-run choice, so it beats the per-stage tables rather than only the base. Clearing mirrors AdapterPolicy.resolved's own client-switch rule: a stage whose effective client the flag changes drops the model/extra_args/binary it chose for the old one and keeps its timing knobs; a stage already on the forced client is left alone. An unknown --cli fails before anything spawns — the dry-run render resolves the profile too, so it would otherwise surface as a traceback. Naming follows probe-adapter's vocabulary: `cli` is the profile, `binary` the executable. --- CHANGELOG.md | 9 ++++ docs/FEATURES.md | 1 + src/bmad_loop/cli.py | 83 ++++++++++++++++++++++++++++++++++- src/bmad_loop/policy.py | 2 +- tests/test_cli.py | 97 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 189 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87ca5127..2bb1677a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ breaking changes may land in a minor release. ### Added +- **`run`/`sweep` take `--cli` and `--cli-binary` (#395).** Pick the coding CLI or the executable + for one run without editing `policy.toml` — `bmad-loop run --cli-binary cc` bills a run to a + second account, `--cli codex` moves it to another client. A flag is a whole-run choice, so it + beats the per-stage `[adapter.]` tables as well as the base; a stage the flag moves to a + different client drops the model and flags it had chosen for the old one, and keeps its timing + knobs. The override is folded into the policy before the run is stamped, so `resume`, + `status --json` and `--dry-run` all report what actually ran. An unknown `--cli` fails before + anything spawns. + - **`[adapter] binary` spawns a CLI under a custom name (#395).** Set `binary = "cc"` (base or per-stage) to drive a second subscription or a work/personal account that lives beside the default install, without forking the whole profile to change one field — a copy freezes its diff --git a/docs/FEATURES.md b/docs/FEATURES.md index ebdfd381..50ae887f 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -192,6 +192,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - `bmad-loop mux` — list registered terminal-multiplexer backends (platform · availability · version · which is selected and why); `mux set ` persists a machine-scoped choice into policy.toml (`--clear` reverts to auto, `--force` allows a name only registered on the target machine). Bundled backend: `tmux`; external backends (e.g. the herdr adapter) register via the `bmad_loop.mux_backends` entry-point group — see [Terminal multiplexer backends](multiplexer-backends.md). - `bmad-loop run` — drive the dev → review → verify → commit loop. - `bmad-loop sweep` — triage + execute open deferred-work entries. +- `run` and `sweep` both take `--cli ` and `--cli-binary ` — the per-run form of `[adapter] name` / `[adapter] binary`, for picking a client or an account without editing project config (`bmad-loop run --cli-binary cc`). A flag is a whole-run choice, so it beats the per-stage tables too; it is folded into the policy before the run is stamped, so `resume`, `status --json` and `--dry-run` all report what actually ran. - `bmad-loop resume ` — continue a paused/interrupted run. - `bmad-loop resolve ` — resolve a CRITICAL escalation, then re-arm + resume (`--story`, `--no-interactive`, `--restore-patch ` for intent-gap patch-restore, `--resume`/`--no-resume`). - `bmad-loop decisions` — answer deferred-work decisions past sweeps left unanswered (`--list` to just show them). `--json` instead emits a stable machine-readable document (schema-versioned; per decision the id, question, context, recommendation and every option's key/label/effect/intent/resolution/bundle-name plus a derived `recommended` flag) per the [contract below](#machine-readable-output---json); it implies the listing and never prompts, and nothing pending yields a valid empty document. diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index f470eb63..96bb7abd 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -885,12 +885,89 @@ def _warn_unknown_keys(ss: sprintstatus.SprintStatus) -> None: ) +def _add_cli_override_flags(parser: argparse.ArgumentParser) -> None: + """Per-run coding-CLI selection, shared by `run` and `sweep`. Named after + `probe-adapter`'s existing vocabulary: `cli` is the profile, `binary` the + executable, so one word means one thing across the CLI surface.""" + parser.add_argument( + "--cli", metavar="PROFILE", help="override [adapter] name for this run (e.g. codex)" + ) + parser.add_argument( + "--cli-binary", + metavar="EXECUTABLE", + help="override [adapter] binary for this run: spawn the same CLI under " + "another name, e.g. a second account reachable as `cc`", + ) + + +def _apply_cli_overrides(pol, args, project: Path): + """Fold ``--cli`` / ``--cli-binary`` into the policy, returning a new Policy. + + Applied to the *policy object* before the run composes, not threaded into the + engine, so the RunState snapshot records what actually ran: `resume`, the + ``--json`` adapter projection, per-task adapter stamping and the dry-run + render all follow with no further wiring. + + A flag is a whole-run choice, so it wins over the per-stage tables rather + than only the base: forcing a client or a binary and then silently keeping a + stage pinned to another one would defeat the point of passing it. Clearing is + surgical, and mirrors ``AdapterPolicy.resolved``'s own client-switch rule — a + stage whose effective client the flag changes drops its client-specific + fields (model, extra_args, binary) and keeps its timing knobs; a stage that + already ran the forced client keeps everything but its now-redundant name. + """ + from dataclasses import replace + + from .adapters.profile import ProfileError, get_profile + + name, binary = getattr(args, "cli", None), getattr(args, "cli_binary", None) + if not name and not binary: + return pol + if name: + # Fail here rather than at session spawn: the dry-run render resolves the + # profile too, and an unknown name should not surface as a traceback. + try: + get_profile(name, project) + except ProfileError as e: + raise SystemExit(f"error: {e}") from e + adapter = pol.adapter + stages = {} + for role in ROLES: + stage = getattr(adapter, role) + if name and (stage.name or adapter.name) != name: + # the client changed under this stage: its model/extra_args/binary + # were chosen for the old one and must not be carried over + stages[role] = policy_mod.StageAdapterPolicy( + usage_grace_s=stage.usage_grace_s, + stop_without_result_nudges=stage.stop_without_result_nudges, + ) + continue + stages[role] = replace( + stage, + name=None if name else stage.name, + binary=None if binary else stage.binary, + ) + return replace( + pol, + adapter=replace( + adapter, + name=name or adapter.name, + binary=binary or adapter.binary, + # a forced client falls back to that CLI's own default model/flags, + # the same reset a stage client switch performs + model="" if name and name != adapter.name else adapter.model, + extra_args=None if name and name != adapter.name else adapter.extra_args, + **stages, + ), + ) + + def cmd_run(args: argparse.Namespace) -> int: if (rc := _reject_bad_run_id(args.run_id)) is not None: return rc project = _project(args) paths = bmadconfig.load_paths(project) - pol = policy_mod.load(_policy_path(project)) + pol = _apply_cli_overrides(policy_mod.load(_policy_path(project)), args, project) stories_on, spec_folder = _stories_mode(args, pol) if stories_on and args.epic is not None: @@ -1150,7 +1227,7 @@ def cmd_sweep(args: argparse.Namespace) -> int: return rc project = _project(args) paths = bmadconfig.load_paths(project) - pol = policy_mod.load(_policy_path(project)) + pol = _apply_cli_overrides(policy_mod.load(_policy_path(project)), args, project) if args.dry_run: return _sweep_dry_run(paths, pol) @@ -2770,6 +2847,7 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: "or full key (sprint mode); a story id (stories mode)", ) run_p.add_argument("--max-stories", type=int, help="stop after N stories") + _add_cli_override_flags(run_p) run_p.add_argument("--dry-run", action="store_true", help="print the plan, spawn nothing") run_p.add_argument("--run-id", help=argparse.SUPPRESS) # pre-assigned id (used by the TUI) @@ -2785,6 +2863,7 @@ def add(name: str, func, help: str, *, aliases=()) -> argparse.ArgumentParser: help="triage + answer decisions + record them; run no bundles", ) sweep_p.add_argument("--max-bundles", type=int, help="override [sweep] max_bundles") + _add_cli_override_flags(sweep_p) sweep_p.add_argument( "--repeat", action=argparse.BooleanOptionalAction, diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 87cb9695..bdc6c698 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -1179,7 +1179,7 @@ def _fold_deprecated_engine( # CLI under another name — a second subscription or a work/personal account # reachable as `cc` beside `claude` — without forking the whole profile. It must # be an alias or wrapper of the SAME CLI: the profile still supplies the hook -# dialect, config path and transcript layout. +# dialect, config path and transcript layout. Per run: `--cli-binary cc`. binary = "" cleanup_session_on_finish = true # kill the run's tmux session when it finishes (false keeps it for inspection) # extra_args replaces the profile's default permission-bypass flags when set: diff --git a/tests/test_cli.py b/tests/test_cli.py index ccfeee15..7dcbb245 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -103,6 +103,103 @@ def test_dry_run_renders_per_stage_commands(project, capsys): assert "--model gpt-5-codex" in review_line +def _cli_flags(cli_name=None, cli_binary=None): + return argparse.Namespace(cli=cli_name, cli_binary=cli_binary) + + +def test_cli_overrides_absent_leave_the_policy_untouched(tmp_path): + pol = policy_mod.loads('[adapter]\nname = "claude"\nmodel = "opus"\n') + assert cli._apply_cli_overrides(pol, _cli_flags(), tmp_path) is pol + + +def test_cli_binary_flag_reaches_every_stage(tmp_path): + """A flag is a whole-run choice, so it beats a per-stage binary rather than + only the base — forcing an account and silently leaving one stage on another + would defeat the point of passing it.""" + pol = policy_mod.loads('[adapter]\nbinary = "old"\n[adapter.dev]\nbinary = "older"\n') + out = cli._apply_cli_overrides(pol, _cli_flags(cli_binary="cc"), tmp_path) + assert all(out.adapter.resolved(role).binary == "cc" for role in cli.ROLES) + + +def test_cli_binary_flag_spares_a_stage_on_another_client(tmp_path): + """binary stays client-specific under a flag: a `cc` alias of claude must not + become argv[0] for a review stage pinned to codex.""" + pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.review]\nname = "codex"\n') + out = cli._apply_cli_overrides(pol, _cli_flags(cli_binary="cc"), tmp_path) + assert out.adapter.resolved("dev").binary == "cc" + assert out.adapter.resolved("review") == policy_mod.ResolvedAdapter("codex", "", None) + + +def test_cli_flag_forces_the_client_and_drops_stale_stage_settings(tmp_path): + """--cli moves a stage to another client, so that stage's model/extra_args — + chosen for the client it is leaving — must not ride along. A stage already on + the forced client keeps its own settings.""" + pol = policy_mod.loads( + '[adapter]\nname = "claude"\nmodel = "opus"\nextra_args = ["--x"]\n' + '[adapter.review]\nname = "codex"\nmodel = "gpt-5-codex"\n' + ) + out = cli._apply_cli_overrides(pol, _cli_flags(cli_name="codex"), tmp_path) + dev = out.adapter.resolved("dev") + assert dev.name == "codex" + assert dev.model == "" and dev.extra_args is None # claude's opus/--x dropped + review = out.adapter.resolved("review") + assert review.name == "codex" and review.model == "gpt-5-codex" # already codex + + +def test_cli_flag_keeps_stage_timing_knobs_across_a_client_switch(tmp_path): + # usage_grace_s / stop_without_result_nudges mean "fall back to the profile + # default" — they are not client-specific, so a switch must not clear them + pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.dev]\nusage_grace_s = 3.5\n') + out = cli._apply_cli_overrides(pol, _cli_flags(cli_name="codex"), tmp_path) + assert out.adapter.resolved("dev").usage_grace_s == 3.5 + + +def test_unknown_cli_flag_fails_before_anything_spawns(tmp_path): + pol = policy_mod.loads("[adapter]\n") + with pytest.raises(SystemExit, match="unknown CLI profile: 'nope'"): + cli._apply_cli_overrides(pol, _cli_flags(cli_name="nope"), tmp_path) + + +def test_run_dry_run_renders_the_flag_overrides(project, capsys): + """End-to-end through cmd_run: the plan names the executable and client the + run would really spawn, so the flags are verifiable before any session.""" + install_bmad_config(project) + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + _write_policy(project.project) # DUAL_CLIENT_POLICY: claude dev, codex review + argv = ["run", "--project", str(project.project), "--dry-run"] + assert cli.main([*argv, "--cli", "claude", "--cli-binary", "cc"]) == 0 + out = capsys.readouterr().out + dev_line = next(line for line in out.splitlines() if "dev:" in line) + review_line = next(line for line in out.splitlines() if "review:" in line) + # --cli claude pulls the codex review stage onto claude, and --cli-binary + # then applies to it too + assert dev_line.split("dev:")[1].strip().startswith("cc ") + assert review_line.split("review:")[1].strip().startswith("cc ") + assert "gpt-5-codex" not in out # codex-specific model dropped with the client + + +def test_sweep_applies_the_same_flags(project, monkeypatch): + """Both run types take the override, so an unattended sweep can be billed to + the same account as the run that deferred the work.""" + install_bmad_config(project) + seen = {} + + def fake_start(project, paths, pol, **kw): + seen["pol"] = pol + return 0 + + monkeypatch.setattr(cli, "_start_sweep", fake_start) + monkeypatch.setattr(cli, "_require_base_skills", lambda *a, **k: True) + monkeypatch.setattr(cli, "_reconcile_stale", lambda *a, **k: None) + monkeypatch.setattr(cli.verify, "worktree_clean", lambda repo: True) + assert ( + cli.main(["sweep", "--project", str(project.project), "--cli-binary", "cc", "--no-prompt"]) + == 0 + ) + assert seen["pol"].adapter.binary == "cc" + assert all(seen["pol"].adapter.resolved(role).binary == "cc" for role in cli.ROLES) + + @pytest.mark.parametrize( "epic,story", [(None, "3-1"), (None, "3.1"), (3, "1"), (None, "user-auth"), (None, "3-1-user-auth")], From 88a344a416ed24fb96cdbf54c06e232667540ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAlberto?= Date: Thu, 30 Jul 2026 18:38:08 +0200 Subject: [PATCH 3/5] fix(cli): reject a blank --cli/--cli-binary and trim both (#395) The flag path skipped the normalization the TOML path gets, so `--cli-binary " "` silently read as unset and `--cli-binary " cc "` became a whitespace-padded argv[0] that dies at spawn with an unrecognizable error. Route both flags through the same trim-and-refuse rule policy.toml already applies. --- src/bmad_loop/cli.py | 7 +++++++ tests/test_cli.py | 27 ++++++++++++++++++++++----- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index 96bb7abd..b88a1264 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -921,6 +921,13 @@ def _apply_cli_overrides(pol, args, project: Path): from .adapters.profile import ProfileError, get_profile name, binary = getattr(args, "cli", None), getattr(args, "cli_binary", None) + # The same normalization policy.toml gets: trim, and refuse a present-but- + # blank value rather than letting it read as unset (or, worse, become a + # whitespace argv[0] that fails at spawn with an unrecognizable error). + for flag, raw in (("--cli", name), ("--cli-binary", binary)): + if raw is not None and not raw.strip(): + raise SystemExit(f"error: {flag} must not be blank") + name, binary = (name or "").strip(), (binary or "").strip() if not name and not binary: return pol if name: diff --git a/tests/test_cli.py b/tests/test_cli.py index 7dcbb245..aac7b041 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -103,8 +103,8 @@ def test_dry_run_renders_per_stage_commands(project, capsys): assert "--model gpt-5-codex" in review_line -def _cli_flags(cli_name=None, cli_binary=None): - return argparse.Namespace(cli=cli_name, cli_binary=cli_binary) +def _cli_flags(cli=None, cli_binary=None): + return argparse.Namespace(cli=cli, cli_binary=cli_binary) def test_cli_overrides_absent_leave_the_policy_untouched(tmp_path): @@ -138,7 +138,7 @@ def test_cli_flag_forces_the_client_and_drops_stale_stage_settings(tmp_path): '[adapter]\nname = "claude"\nmodel = "opus"\nextra_args = ["--x"]\n' '[adapter.review]\nname = "codex"\nmodel = "gpt-5-codex"\n' ) - out = cli._apply_cli_overrides(pol, _cli_flags(cli_name="codex"), tmp_path) + out = cli._apply_cli_overrides(pol, _cli_flags(cli="codex"), tmp_path) dev = out.adapter.resolved("dev") assert dev.name == "codex" assert dev.model == "" and dev.extra_args is None # claude's opus/--x dropped @@ -150,14 +150,31 @@ def test_cli_flag_keeps_stage_timing_knobs_across_a_client_switch(tmp_path): # usage_grace_s / stop_without_result_nudges mean "fall back to the profile # default" — they are not client-specific, so a switch must not clear them pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.dev]\nusage_grace_s = 3.5\n') - out = cli._apply_cli_overrides(pol, _cli_flags(cli_name="codex"), tmp_path) + out = cli._apply_cli_overrides(pol, _cli_flags(cli="codex"), tmp_path) assert out.adapter.resolved("dev").usage_grace_s == 3.5 +@pytest.mark.parametrize("flag", ["cli", "cli_binary"]) +def test_blank_cli_flag_is_rejected(tmp_path, flag): + """The flags get policy.toml's own blank rule. Without it `--cli-binary " "` + read as unset while `--cli-binary " cc "` became a whitespace-padded argv[0] + that dies at spawn with an unrecognizable error.""" + pol = policy_mod.loads("[adapter]\n") + with pytest.raises(SystemExit, match="must not be blank"): + cli._apply_cli_overrides(pol, _cli_flags(**{flag: " "}), tmp_path) + + +def test_cli_flags_are_trimmed(tmp_path): + pol = policy_mod.loads("[adapter]\n") + out = cli._apply_cli_overrides(pol, _cli_flags(" codex ", " cc "), tmp_path) + assert out.adapter.name == "codex" + assert out.adapter.binary == "cc" + + def test_unknown_cli_flag_fails_before_anything_spawns(tmp_path): pol = policy_mod.loads("[adapter]\n") with pytest.raises(SystemExit, match="unknown CLI profile: 'nope'"): - cli._apply_cli_overrides(pol, _cli_flags(cli_name="nope"), tmp_path) + cli._apply_cli_overrides(pol, _cli_flags(cli="nope"), tmp_path) def test_run_dry_run_renders_the_flag_overrides(project, capsys): From 43a6bd725f1184e62771d62f916a8e984b28ac4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAlberto?= Date: Thu, 30 Jul 2026 18:48:02 +0200 Subject: [PATCH 4/5] fix(cli): drop a base binary belonging to the client --cli leaves (#395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base table reset model and extra_args on a --cli client switch but kept binary, so a `cc` alias of claude survived `--cli codex` and the run previewed as `cc-work "Use the $bmad-dev-auto skill…" --dangerously-bypass- approvals-and-sandbox` — claude's executable driven with codex's prompt template and bypass flags. binary is client-specific like the other two; give the base the same reset the stage loop already performed. An explicit --cli-binary still wins, and forcing the client already in use is not a switch, so a configured alias survives that. --- src/bmad_loop/cli.py | 14 +++++++++----- tests/test_cli.py | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index b88a1264..cb9a2cbd 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -938,6 +938,12 @@ def _apply_cli_overrides(pol, args, project: Path): except ProfileError as e: raise SystemExit(f"error: {e}") from e adapter = pol.adapter + # A forced client resets the base's client-specific fields to that CLI's own + # defaults, the same reset the stage loop below performs — binary included: a + # `cc` alias of claude left standing under `--cli codex` would spawn claude + # with codex's prompt template and bypass flags. An explicit --cli-binary + # still wins over the reset. + switched = bool(name) and name != adapter.name stages = {} for role in ROLES: stage = getattr(adapter, role) @@ -959,11 +965,9 @@ def _apply_cli_overrides(pol, args, project: Path): adapter=replace( adapter, name=name or adapter.name, - binary=binary or adapter.binary, - # a forced client falls back to that CLI's own default model/flags, - # the same reset a stage client switch performs - model="" if name and name != adapter.name else adapter.model, - extra_args=None if name and name != adapter.name else adapter.extra_args, + binary=binary or ("" if switched else adapter.binary), + model="" if switched else adapter.model, + extra_args=None if switched else adapter.extra_args, **stages, ), ) diff --git a/tests/test_cli.py b/tests/test_cli.py index aac7b041..07f55341 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -146,6 +146,30 @@ def test_cli_flag_forces_the_client_and_drops_stale_stage_settings(tmp_path): assert review.name == "codex" and review.model == "gpt-5-codex" # already codex +def test_cli_flag_drops_a_base_binary_belonging_to_the_old_client(tmp_path): + """A `cc` alias of claude left standing under `--cli codex` would spawn claude + with codex's prompt template and bypass flags. The base table gets the same + client-switch reset as the stages.""" + pol = policy_mod.loads('[adapter]\nname = "claude"\nbinary = "cc-work"\n') + out = cli._apply_cli_overrides(pol, _cli_flags(cli="codex"), tmp_path) + assert out.adapter.binary == "" + assert all(out.adapter.resolved(role).binary == "" for role in cli.ROLES) + + +def test_cli_binary_flag_survives_a_client_switch(tmp_path): + # the reset is a fallback, not a veto: an explicit binary still wins + pol = policy_mod.loads('[adapter]\nname = "claude"\nbinary = "cc-work"\n') + out = cli._apply_cli_overrides(pol, _cli_flags("codex", "/opt/work/codex"), tmp_path) + assert all(out.adapter.resolved(role).binary == "/opt/work/codex" for role in cli.ROLES) + + +def test_cli_flag_keeps_a_base_binary_when_the_client_is_unchanged(tmp_path): + # forcing the client you already run is not a switch — the alias stays + pol = policy_mod.loads('[adapter]\nname = "claude"\nbinary = "cc-work"\n') + out = cli._apply_cli_overrides(pol, _cli_flags(cli="claude"), tmp_path) + assert out.adapter.resolved("dev").binary == "cc-work" + + def test_cli_flag_keeps_stage_timing_knobs_across_a_client_switch(tmp_path): # usage_grace_s / stop_without_result_nudges mean "fall back to the profile # default" — they are not client-specific, so a switch must not clear them From 1d2199cbc17ea5aa81ecdf263cd800546bf2021c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9CAlberto?= Date: Thu, 30 Jul 2026 19:36:56 +0200 Subject: [PATCH 5/5] fix(cli): say when --cli-binary cannot reach a stage (#395) A binary belongs to one client: the profile decides the prompt dialect, hook contract and bypass flags, so spawning a `cc` alias of claude for a codex-pinned stage hands claude `--dangerously-bypass-approvals-and- sandbox` and kills the session at startup. The flag therefore stops at a stage running another client. Doing that silently was the mistake. An operator who passes a whole-run flag had to read a --dry-run to notice it applied to only some stages. Name the stage, the client it runs, and the way out (--cli forces one client for the whole run). Nothing changes when the flag reaches everything, which is the homogeneous-config default. --- CHANGELOG.md | 4 +++- docs/FEATURES.md | 2 +- src/bmad_loop/cli.py | 32 +++++++++++++++++++++++++++++++- tests/test_cli.py | 26 ++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bb1677a..b2639c67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,9 @@ breaking changes may land in a minor release. different client drops the model and flags it had chosen for the old one, and keeps its timing knobs. The override is folded into the policy before the run is stamped, so `resume`, `status --json` and `--dry-run` all report what actually ran. An unknown `--cli` fails before - anything spawns. + anything spawns. `--cli-binary` stops at a stage pinned to another client — an executable + belongs to one CLI — and prints a note naming that stage rather than applying partly in + silence. - **`[adapter] binary` spawns a CLI under a custom name (#395).** Set `binary = "cc"` (base or per-stage) to drive a second subscription or a work/personal account that lives beside the diff --git a/docs/FEATURES.md b/docs/FEATURES.md index 50ae887f..76c9af95 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -192,7 +192,7 @@ See [README.md](../README.md) for the narrative overview and [setup-guide.md](se - `bmad-loop mux` — list registered terminal-multiplexer backends (platform · availability · version · which is selected and why); `mux set ` persists a machine-scoped choice into policy.toml (`--clear` reverts to auto, `--force` allows a name only registered on the target machine). Bundled backend: `tmux`; external backends (e.g. the herdr adapter) register via the `bmad_loop.mux_backends` entry-point group — see [Terminal multiplexer backends](multiplexer-backends.md). - `bmad-loop run` — drive the dev → review → verify → commit loop. - `bmad-loop sweep` — triage + execute open deferred-work entries. -- `run` and `sweep` both take `--cli ` and `--cli-binary ` — the per-run form of `[adapter] name` / `[adapter] binary`, for picking a client or an account without editing project config (`bmad-loop run --cli-binary cc`). A flag is a whole-run choice, so it beats the per-stage tables too; it is folded into the policy before the run is stamped, so `resume`, `status --json` and `--dry-run` all report what actually ran. +- `run` and `sweep` both take `--cli ` and `--cli-binary ` — the per-run form of `[adapter] name` / `[adapter] binary`, for picking a client or an account without editing project config (`bmad-loop run --cli-binary cc`). A flag is a whole-run choice, so it beats the per-stage tables too; it is folded into the policy before the run is stamped, so `resume`, `status --json` and `--dry-run` all report what actually ran. `--cli-binary` stops at a stage pinned to another client — an executable belongs to one CLI, and handing a `cc` alias of claude to a codex-pinned stage would give claude codex's flags and kill the session at startup — and says so on stderr, naming the stage and pointing at `--cli` to force one client for the whole run. - `bmad-loop resume ` — continue a paused/interrupted run. - `bmad-loop resolve ` — resolve a CRITICAL escalation, then re-arm + resume (`--story`, `--no-interactive`, `--restore-patch ` for intent-gap patch-restore, `--resume`/`--no-resume`). - `bmad-loop decisions` — answer deferred-work decisions past sweeps left unanswered (`--list` to just show them). `--json` instead emits a stable machine-readable document (schema-versioned; per decision the id, question, context, recommendation and every option's key/label/effect/intent/resolution/bundle-name plus a derived `recommended` flag) per the [contract below](#machine-readable-output---json); it implies the listing and never prompts, and nothing pending yields a valid empty document. diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index cb9a2cbd..312ccf0a 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -960,7 +960,7 @@ def _apply_cli_overrides(pol, args, project: Path): name=None if name else stage.name, binary=None if binary else stage.binary, ) - return replace( + out = replace( pol, adapter=replace( adapter, @@ -971,6 +971,36 @@ def _apply_cli_overrides(pol, args, project: Path): **stages, ), ) + _warn_unapplied_binary(out, binary) + return out + + +def _warn_unapplied_binary(pol, binary: str) -> None: + """Name every stage a ``--cli-binary`` did not reach, and why. + + A binary only means something relative to one client — the profile decides + the prompt dialect, hook contract and bypass flags, so spawning a `cc` alias + of claude for a codex-pinned stage would hand claude codex's flags and kill + the session at startup. The flag therefore stops at a stage running another + client. That is defensible; doing it *silently* is not, since the operator + passed a whole-run flag and would otherwise have to read a `--dry-run` to + notice it was partly ignored.""" + if not binary: + return + skipped = [ + (role, resolved.name) + for role in ROLES + if (resolved := pol.adapter.resolved(role)).binary != binary + ] + if not skipped: + return + where = ", ".join(f"{role} (runs {client})" for role, client in skipped) + print( + f"note: --cli-binary {binary} not applied to {where} — a binary belongs to one " + f"client, and {pol.adapter.name} is not what these stages run; " + f"pass --cli to force one client for the whole run", + file=sys.stderr, + ) def cmd_run(args: argparse.Namespace) -> int: diff --git a/tests/test_cli.py b/tests/test_cli.py index 07f55341..23966cc7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -130,6 +130,32 @@ def test_cli_binary_flag_spares_a_stage_on_another_client(tmp_path): assert out.adapter.resolved("review") == policy_mod.ResolvedAdapter("codex", "", None) +def test_cli_binary_flag_warns_about_a_stage_it_could_not_reach(tmp_path, capsys): + """A whole-run flag that silently applies to only some stages is the worst of + both worlds: the operator has to read a --dry-run to notice. Name the stage + and the way out.""" + pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.review]\nname = "codex"\n') + cli._apply_cli_overrides(pol, _cli_flags(cli_binary="cc"), tmp_path) + err = capsys.readouterr().err + assert "--cli-binary cc not applied to review (runs codex)" in err + assert "pass --cli to force one client" in err + + +def test_cli_binary_flag_silent_when_it_reaches_every_stage(tmp_path, capsys): + pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.dev]\nmodel = "opus"\n') + cli._apply_cli_overrides(pol, _cli_flags(cli_binary="cc"), tmp_path) + assert capsys.readouterr().err == "" + + +def test_cli_binary_flag_silent_when_cli_forces_one_client(tmp_path, capsys): + # --cli collapses the heterogeneous config, so the binary now reaches + # everything and there is nothing to warn about + pol = policy_mod.loads('[adapter]\nname = "claude"\n[adapter.review]\nname = "codex"\n') + out = cli._apply_cli_overrides(pol, _cli_flags("codex", "/opt/work/codex"), tmp_path) + assert capsys.readouterr().err == "" + assert all(out.adapter.resolved(role).binary == "/opt/work/codex" for role in cli.ROLES) + + def test_cli_flag_forces_the_client_and_drops_stale_stage_settings(tmp_path): """--cli moves a stage to another client, so that stage's model/extra_args — chosen for the client it is leaving — must not ride along. A stage already on