feat(cli): add --cli and --cli-binary to run and sweep - #398
Draft
albi-bon wants to merge 5 commits into
Draft
Conversation
…g#395) 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.<stage>]. 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.
…#395) 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.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…org#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.
…ad-code-org#395) 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
runandsweeptake--cli <profile>and--cli-binary <executable>: the per-run form of[adapter] nameand[adapter] binary.Why
Fixes #395 (part 2 of 3).
#397 makes the account choice expressible, but only as project-scoped state: switching means editing
policy.toml. That is awkward when alternating work and personal accounts on the same repo, and worse when one account hits its usage limit mid-sprint and you want the next run elsewhere.How
_add_cli_override_flagson both subparsers, sorunandsweepcannot drift. Naming followsprobe-adapter's existing vocabulary —cliis the profile,binarythe executable — so one word means one thing across the CLI surface._apply_cli_overridesfolds the flags into the policy right after load, before the run composes. That is the whole trick: the policy is what gets stamped intoRunState, soresume, the--jsonadapter projection, per-task adapter stamping and the--dry-runrender all follow with no further threading.[adapter.<stage>]tables as well as the base. Clearing is surgical and mirrorsAdapterPolicy.resolved's own client-switch rule: a stage whose effective client the flag changes drops the model/extra_args/binary it had chosen for the old client, and keeps its timing knobs (those mean "fall back to the profile default" and aren't client-specific). A stage already running the forced client keeps everything.--cli-binarystops at a stage pinned to another client, and says so on stderr. An executable belongs to one CLI — the profile decides the prompt dialect, hook contract and bypass flags, so handing accalias of claude to a codex-pinned stage would give claude--dangerously-bypass-approvals-and-sandboxand kill the session at startup. The note names the stage, the client it runs, and the way out (--cliforces one client for the whole run). Silent when the flag reaches every stage, which is the homogeneous-config default.policy.tomldoes: trimmed, and a present-but-blank value refused rather than read as unset.--cliraises before anything spawns, with the sameunknown CLI profile … (available: …)messagemake_adaptersgives for a bad policy value — needed because the dry-run render resolves the profile too, so it would otherwise surface as a traceback.Baseline with codex configured for review and custom binary configured for
devCustom binary specified per-run, warning that it does not get applied to the custom
reviewagentCustom goes back to harness-default when
--cliarg is usedTwo calls I'd flag for you
resumetakes no flags. It rebuilds from the stamped snapshot, so a resumed run already inherits whatever it was launched with. Adding flags there would mean deciding whether a resume may change client mid-run, which seems like its own conversation.--cliexitsFAILURE(1), notUSAGE(2), to match what the identical unknown-profile error already does when it comes frompolicy.toml. Say the word if you'd rather have 2 and I'll split the paths.Testing
uv run pytest -q— 3709 passed, with the same two pre-existing macOS-only failures noted in #397 (invalid-UTF-8 filenames APFS rejects), which reproduce on a clean checkout.uv run pyrightreports the same 2 pre-existingos.setxattrerrors before and after.ruff checkclean; formatting verified with theblackandprettierversions pinned in.trunk/trunk.yaml.Coverage in
tests/test_cli.py: the no-flag identity case;--cli-binaryreaching every stage; the client-specific carve-out (accalias not leaking into a codex-pinned stage) and the note that reports it; silence when the flag reaches everything and when--clicollapses the config first;--clidropping stale stage settings while keeping timing knobs; the blank-value and unknown-profile refusals; plus end-to-endrun --dry-runandsweepassertions. Per the AGENTS.md ablation rule I removed the_apply_cli_overridescall from both call sites, and separately the warning call, and confirmed the corresponding tests FAIL first.Also exercised by hand against a scratch BMAD project — the screenshots above are that session.