diff --git a/CHANGELOG.md b/CHANGELOG.md index 97fbb2d9..b2639c67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,27 @@ 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. `--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 + 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..76c9af95 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). @@ -191,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. `--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 1afd313f..312ccf0a 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: @@ -870,12 +885,130 @@ 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) + # 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: + # 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 + # 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) + 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, + ) + out = replace( + pol, + adapter=replace( + adapter, + name=name or adapter.name, + binary=binary or ("" if switched else adapter.binary), + model="" if switched else adapter.model, + extra_args=None if switched else adapter.extra_args, + **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: 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: @@ -944,19 +1077,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, @@ -1132,7 +1268,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) @@ -2752,6 +2888,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) @@ -2767,6 +2904,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/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..bdc6c698 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. 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: # 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..23966cc7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -103,6 +103,170 @@ def test_dry_run_renders_per_stage_commands(project, capsys): assert "--model gpt-5-codex" in review_line +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): + 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_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 + 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="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_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 + 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="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="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")], @@ -1510,6 +1674,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 +3934,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):