diff --git a/CHANGELOG.md b/CHANGELOG.md index 01c3a8e0a..1051f7a97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ breaking changes may land in a minor release. ### Added +- Add a free-form `effort` key to `[adapter]` and every `[adapter.]` table, + inherited like `model`; `opencode-http` sends it as the per-prompt `variant` on + every turn, and `validate` warns (`policy.effort-unsupported`) when a tmux stage + sets it (#643). - Journal a session's idle stretches (#680). The tmux adapter stats the live transcript on the heartbeat cadence, stamps `transcript_idle_s` on `heartbeat.json`, and — with the engine's journal attached (`CodingCLIAdapter.journal`) — writes one `session-idle` @@ -27,6 +31,12 @@ breaking changes may land in a minor release. dialog, a login, a dead-on-arrival window); `decide_dev` pauses ahead of the budget as an environment fault does, `dev-decision` and `session-end` carry the flag, and re-arm resets the attempt. +- Preserve inherited `model`, `effort` and `extra_args` when a stage names an alias + of the base client (`opencode` / `opencode-http`, `claude-code-tmux` / `claude`) + instead of treating it as a client switch. +- Key the `run --dry-run` launch preview on the adapter kind, not `profile.hookless`: + an `opencode-http` profile with a hook dialect shows the server/prompt_async line, + a hookless profile of another kind shows the argv line. ## [0.12.0] — 2026-09-20 diff --git a/README.md b/README.md index 09c3ed96b..926f144ef 100644 --- a/README.md +++ b/README.md @@ -462,14 +462,17 @@ skill = "bmad-dev-auto" # the only supported value — the generic upstream d [adapter] name = "claude" # CLI profile: claude | codex | gemini | copilot | antigravity | opencode-http (alias: opencode) | custom model = "" # empty = CLI default (opencode-http wants "provider/model") +effort = "" # reasoning effort, free-form (e.g. "high", "max"); empty = provider default. + # Sent by opencode-http as the per-prompt `variant`; the tmux CLIs have no + # channel for it and ignore it (`bmad-loop validate` warns) 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 bypass flags when set: # extra_args = ["--permission-mode", "bypassPermissions"] # Optional per-stage overrides — run the review pass on a different CLI/model # than the dev pass. Unset keys inherit from [adapter] when the stage runs the -# same client; switching client falls back to that profile's defaults (model -# and extra_args are client-specific). +# same client; switching client falls back to that profile's defaults (model, +# effort and extra_args are client-specific). # [adapter.dev] # model = "opus" # [adapter.review] @@ -477,6 +480,12 @@ cleanup_session_on_finish = true # kill the run's tmux session when it finishes # model = "gpt-5-codex" # [adapter.triage] # sweep triage stage # model = "opus" +# With an opencode-http base, effort tunes reasoning per stage (opencode-http +# only — a tmux CLI ignores it and `bmad-loop validate` warns). An unrecognized +# name is not rejected: the session silently runs at the provider default, so +# spell it exactly as the model's variant list names it. +# [adapter.review] +# effort = "max" # e.g. a deeper review pass than dev [sweep] auto = "never" # never | per-epic | run-end (auto sweeps never prompt) diff --git a/docs/FEATURES.md b/docs/FEATURES.md index dffc976b8..8912cddcb 100644 --- a/docs/FEATURES.md +++ b/docs/FEATURES.md @@ -698,6 +698,7 @@ persisted artifacts. - 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]`). +- Reasoning effort: a free-form `effort` string on `[adapter]` and every `[adapter.]` table (values are provider/model-specific — `high`, `max`, … — so nothing is validated against a catalog), inherited exactly like `model` (a stage that switches client falls back to `""` = provider default). Only the `opencode-http` adapter carries it: it is sent as the per-call `variant` on every `prompt_async` body the session issues (initial prompt and every nudge) and omitted entirely when empty, never through `OPENCODE_CONFIG_CONTENT` (whose only effort key, `agent..variant`, is inert unless that agent also pins a model). An unrecognized name is not rejected — OpenCode accepts the prompt and the session silently runs at the provider default — so spell it exactly as the model's variant list names it. The tmux CLIs have no channel for it and ignore it; `bmad-loop validate` reports `policy.effort-unsupported` (a warning, exit code unchanged) when a stage on that family sets it. The value never reaches argv, so `config_digest` is unaffected; `run --dry-run` shows it as `effort=` beside the model on the hookless launch line. - 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). A CLI that needs its own adapter _class_ still needs Python — but not a core edit: the profile's `adapter` field names a kind resolved against the registry, which a co-installed package extends. - `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/docs/adapter-authoring-guide.md b/docs/adapter-authoring-guide.md index 3cbae97e6..becfee66d 100644 --- a/docs/adapter-authoring-guide.md +++ b/docs/adapter-authoring-guide.md @@ -566,7 +566,11 @@ Three frozen dataclasses cross the seam: - **`SessionSpec`** (engine → adapter) — `task_id`, `role` (`"dev"` / `"review"` / `"retro"`), `prompt`, `cwd`, `env`, `model` (empty = CLI default), - `timeout_s`. + `timeout_s`, and `effort` (empty = provider default; a free-form reasoning-effort + name resolved per stage from `[adapter] effort`). Only `opencode-http` carries + `effort` — as the per-prompt `variant` — and the generic tmux adapter ignores it, + because no profile key maps it onto a CLI flag; `bmad-loop validate` warns when a + stage on that family sets it. An out-of-tree adapter class may read it or not. - **`SessionHandle`** (returned by `start_session`) — `task_id`, `native_id` (tmux window id, HTTP session id, …), `launched_ns` (wall-clock ns just before launch; the floor for hook events). @@ -657,7 +661,10 @@ decisions worth stealing: Permissions, the model, and a hermetic skills path are injected via the `OPENCODE_CONFIG_CONTENT` env var (zero worktree pollution), and each server gets its own `OPENCODE_SERVER_PASSWORD` so a foreign process on a recycled - port can never impersonate it. + port can never impersonate it. Reasoning effort (`SessionSpec.effort`) is the + one knob that does NOT go through the config: it is a per-call `variant` on + every `prompt_async` body instead, because the config has no top-level + `variant` and its `agent..variant` is inert unless that agent pins a model. - **Map the transport onto the hook-signal semantics** instead of inventing new ones: the SSE `session.idle` event ≙ the Stop hook, server-process death ≙ window death (`crashed`, landed artifact honored), and a poll fallback diff --git a/docs/setup-guide.md b/docs/setup-guide.md index f30138f1f..53b9f34c1 100644 --- a/docs/setup-guide.md +++ b/docs/setup-guide.md @@ -298,7 +298,9 @@ them to whoever owns the machine: - **opencode** — install the HTTP client extra (`pip install 'bmad-loop[opencode]'`) and authenticate once, **globally**, with `opencode auth login` (not per-project — there is no workspace-trust dialog to answer). Requires OpenCode ≥ 1.18. Set the model as - `provider/model` (e.g. `[adapter] model = "anthropic/claude-haiku-4-5"`). No hooks are + `provider/model` (e.g. `[adapter] model = "anthropic/claude-haiku-4-5"`). A reasoning + `effort` (e.g. `[adapter.review] effort = "max"`) is sent as the per-prompt variant and is + opencode-only — the tmux CLIs ignore it. No hooks are registered — the adapter drives a headless `opencode serve` over HTTP/SSE, so there is no tmux window to attach to; watch a session via its `logs/.log` — a curated transcript of the agent's prose, tool calls, file edits and permission decisions — or the diff --git a/docs/tui-guide.md b/docs/tui-guide.md index 670ae7d7f..01a4c8f8b 100644 --- a/docs/tui-guide.md +++ b/docs/tui-guide.md @@ -674,9 +674,10 @@ behavior. | `review.trigger` | select | `recommended` | `recommended` (run only when the dev pass flags `followup_review_recommended`) / `always`; bounded by `limits.max_review_cycles` | | `adapter.name` | text | `claude` | CLI profile: `claude` / `codex` / `gemini` / custom | | `adapter.model` | text | (CLI default) | model override | +| `adapter.effort` | text | (provider default) | reasoning effort, free-form (`high`, `max`, …); sent by `opencode-http` as the per-prompt `variant`, ignored by the tmux CLIs (validate warns) | | `adapter.extra_args` | override switch + args | profile defaults | see below | | `adapter.cleanup_session_on_finish` | switch | on | kill the run's tmux session on finish; off keeps it | -| `adapter.dev` / `.review` / `.triage` | text ×2 + args | inherit | per-stage `name` / `model` / `extra_args` overrides | +| `adapter.dev` / `.review` / `.triage` | text ×3 + args | inherit | per-stage `name` / `model` / `effort` / `extra_args` overrides | | `sweep.auto` | select | `never` | `never` / `per-epic` / `run-end` | | `sweep.max_bundles` | int ≥ 1 | 5 | bundles per sweep; triage excess truncated | | `sweep.max_triage_attempts` | int ≥ 1 | 2 | triage validation retries | diff --git a/src/bmad_loop/adapters/base.py b/src/bmad_loop/adapters/base.py index d5b261616..4ac75c737 100644 --- a/src/bmad_loop/adapters/base.py +++ b/src/bmad_loop/adapters/base.py @@ -204,6 +204,14 @@ class SessionSpec: # resumed run is protected too — always an absolute path by the time it lands # here. Kept LAST alongside spec_snapshot so positional constructions stay valid. expected_spec: str | None = None + # Reasoning effort (#643), free-form because the legal names are provider- and + # model-specific; "" = provider default. Resolved per stage by + # `AdapterPolicy.resolved()` with the same client-specific inheritance as + # `model`. Only the opencode-http adapter has a channel for it — it rides every + # `prompt_async` body as `variant` — and the tmux generic family ignores it + # (`bmad-loop validate` warns). Never reaches argv, so `config_digest` is + # untouched. Kept LAST so positional SessionSpec constructions stay valid. + effort: str = "" @dataclass(frozen=True) diff --git a/src/bmad_loop/adapters/opencode_http.py b/src/bmad_loop/adapters/opencode_http.py index f3c0e3ed9..5acafcef4 100644 --- a/src/bmad_loop/adapters/opencode_http.py +++ b/src/bmad_loop/adapters/opencode_http.py @@ -103,9 +103,16 @@ ``opencode.json``): a blanket permission allow (the bypass-flags analogue), the hermetic-skills recipe above (project ``.claude/skills`` only — without it every session sees the operator's personal skills), and - the policy model when set. A per-session ``OPENCODE_SERVER_PASSWORD`` makes - the health poll self-discriminating against a foreign server on a reused - port and keeps other local processes from driving an allow-all server. + the policy model when set. Reasoning effort (``SessionSpec.effort``, #643) + deliberately does NOT ride the config: the config schema has no top-level + ``variant``, and its only effort key (``agent..variant``) applies solely + when that agent table also pins its own ``model`` — inert otherwise, which is + the measured negative result in #643. It is sent instead as the per-call + ``variant`` in every ``prompt_async`` body (``_prompt``), initial prompt and + nudges alike, and omitted entirely when empty. A per-session + ``OPENCODE_SERVER_PASSWORD`` makes the health poll self-discriminating against + a foreign server on a reused port and keeps other local processes from + driving an allow-all server. - **SSE ``session.idle`` ≙ the Stop hook**, filtered to this session's id — child/subagent sessions share the stream and emit their own idles. SSE is lossy upstream, so a silent or reconnecting stream degrades to an HTTP poll @@ -370,6 +377,12 @@ class _ServerSession: msg_roles: dict = field(default_factory=dict) client: Any = None # control httpx.Client — main thread only session_id: str = "" + # `SessionSpec.effort`, stashed once at session construction and sent as the + # per-call `variant` on EVERY prompt_async body this session issues (initial + # prompt and nudges — a nudge dropping back to the provider default mid-session + # would be silent drift). "" = omit the key, so the body is byte-identical to + # an effort-less session's. + variant: str = "" events: queue.Queue = field(default_factory=queue.Queue) sse_thread: threading.Thread | None = None sse_stop: threading.Event = field(default_factory=threading.Event) @@ -686,6 +699,7 @@ def start_session(self, spec: SessionSpec) -> SessionHandle: launched_ns = time.time_ns() sess = self._spawn_server(spec) + sess.variant = spec.effort # Registered before the API handshake so the atexit sweep (and kill()) # covers a crash mid-setup; run()'s finally-kill only exists once # start_session has returned a handle. @@ -722,10 +736,12 @@ def _prompt(self, sess: _ServerSession, text: str) -> None: starts no new turn, and consuming the floor for it would discard still-valid completion evidence of the previous turn.""" sent_ms = _now_ms() # sampled before the POST: it precedes the new turn - resp = sess.client.post( - f"/session/{sess.session_id}/prompt_async", - json={"parts": [{"type": "text", "text": text}]}, - ) + body: dict[str, Any] = {"parts": [{"type": "text", "text": text}]} + # Reasoning effort is a per-call PromptInput key (#643); the key is + # omitted, not sent empty, so an effort-less session's body is unchanged. + if sess.variant: + body["variant"] = sess.variant + resp = sess.client.post(f"/session/{sess.session_id}/prompt_async", json=body) if resp.status_code != 204: raise OpencodeServerError(f"prompt_async failed: {resp.status_code} {resp.text[:200]}") sess.floor_ms = max(sess.floor_ms, sent_ms) diff --git a/src/bmad_loop/adapters/profile.py b/src/bmad_loop/adapters/profile.py index 1e3eb6ddf..98ad754a1 100644 --- a/src/bmad_loop/adapters/profile.py +++ b/src/bmad_loop/adapters/profile.py @@ -47,6 +47,7 @@ names_tree_root, names_win32_alias, ) +from ..policy import PROFILE_ALIASES from .entrypoints import record_load_error USAGE_PARSERS = {"claude-jsonl", "codex-rollout", "gemini-chat", "copilot-events", "none"} @@ -63,8 +64,10 @@ CANONICAL_EVENTS = {"SessionStart", "Stop", "SessionEnd", "PreCompact"} USER_PROFILES_REL = Path(".bmad-loop") / "profiles" -# legacy adapter names from older policy.toml files, plus friendly short names -ALIASES = {"claude-code-tmux": "claude", "opencode": "opencode-http"} +# Legacy adapter names from older policy.toml files, plus friendly short names. +# The table itself lives in `policy` (see `PROFILE_ALIASES` there for why) and +# is re-exported here under the name `get_profile` and `install` always used. +ALIASES = PROFILE_ALIASES class ProfileError(Exception): diff --git a/src/bmad_loop/checks.py b/src/bmad_loop/checks.py index e04168cc8..e44932bd3 100644 --- a/src/bmad_loop/checks.py +++ b/src/bmad_loop/checks.py @@ -47,6 +47,7 @@ "bmad-config", "policy", "policy.model-qualified", + "policy.effort-unsupported", "policy.isolation-repo-root", "adapter.profile", "adapter.binary", diff --git a/src/bmad_loop/cli.py b/src/bmad_loop/cli.py index ffcadc7fa..7cb3b94b6 100644 --- a/src/bmad_loop/cli.py +++ b/src/bmad_loop/cli.py @@ -890,6 +890,22 @@ def cmd_validate(args: argparse.Namespace) -> int: f"{prof.name} expects e.g. 'anthropic/claude-haiku-4-5'", {"role": role, "model": cfg.model, "profile": prof.name}, ) + # Reasoning effort (#643) has exactly one carrier: the opencode-http + # kind sends it as the per-prompt `variant`. The tmux generic family + # has no channel for it — no profile flag, no hook field — so a stage + # that sets it there runs at the provider default with nothing to show + # for it. Keyed on the bundled GENERIC kind, like the two checks above, + # because "cannot carry effort" is a fact about that family; an + # out-of-tree kind's capability is not knowable here, so it stays + # silent rather than assert one. Advisory: severity `problem` is + # validate's exit code, and an ignored knob does not make a run unrunnable. + if prof is not None and prof.adapter == adapter_registry.GENERIC and cfg.effort: + report.warn( + "policy.effort-unsupported", + f"{role} effort {cfg.effort!r} is ignored by {prof.name}: " + f"only the opencode-http adapter carries a reasoning-effort value", + {"role": role, "effort": cfg.effort, "profile": prof.name}, + ) base_findings = install.missing_base_skills(project, dev_trees) # gated on PROBLEMS, not on any finding: an advisory review layer (a `when` @@ -2105,19 +2121,27 @@ def cmd_run(args: argparse.Namespace) -> int: def _render_invocation(pol, project: Path, role: str, prompt: str) -> str: + from .adapters import registry as adapter_registry from .adapters.profile import get_profile cfg = pol.adapter.resolved(role) profile = get_profile(cfg.name, project) - if profile.hookless: + # Keyed on the adapter KIND, not on `hookless`: the registry decoupled the two + # axes, so an `opencode-http` profile carrying a hook dialect still launches + # the HTTP adapter (and sends effort), while a hookless profile of another + # kind never does. The preview must follow the adapter `make_adapters` builds. + if profile.adapter == adapter_registry.OPENCODE_HTTP: # 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 "" + # effort rides the prompt_async body as `variant` (#643); shown under the + # policy's own key so the preview distinguishes the configurations. + effort = f" effort={cfg.effort}" if cfg.effort else "" return ( f"{profile.binary} serve --hostname 127.0.0.1 --port " f'(cwd=) → POST /session → prompt_async "{profile.render_prompt(prompt)}"' - f"{model}" + f"{model}{effort}" ) extra = cfg.extra_args if cfg.extra_args is not None else profile.bypass_args argv = [ @@ -3470,7 +3494,9 @@ def cmd_resolve(args: argparse.Namespace) -> int: if (rc := _reject_isolation_conflict(pre_session_paths, pol)) is not None: return rc adapters = _make_adapters(project, run_dir, pol) - model = pol.adapter.resolved("dev").model + dev_cfg = pol.adapter.resolved("dev") + model = dev_cfg.model + effort = dev_cfg.effort _ctx_path, withheld, unreadable = resolve.build_context( state, run_dir, @@ -3499,6 +3525,7 @@ def cmd_resolve(args: argparse.Namespace) -> int: # this `task` object reads the same either way. generation=task.generation, model=model, + effort=effort, ) except NotImplementedError: print( diff --git a/src/bmad_loop/data/settings/core.toml b/src/bmad_loop/data/settings/core.toml index b5d44e056..23363de67 100644 --- a/src/bmad_loop/data/settings/core.toml +++ b/src/bmad_loop/data/settings/core.toml @@ -234,6 +234,11 @@ key = "model" kind = "str" placeholder = "CLI default model" [[section.field]] +key = "effort" +kind = "str" +placeholder = "provider default" +description = "reasoning effort, free-form (provider/model-specific, e.g. high, max); sent by opencode-http as the per-prompt variant, ignored by the tmux CLIs" +[[section.field]] key = "extra_args" kind = "args" [[section.field]] @@ -270,6 +275,10 @@ key = "model" kind = "str" placeholder = "inherit / client default" [[section.field]] +key = "effort" +kind = "str" +placeholder = "inherit / provider default" +[[section.field]] key = "extra_args" kind = "args" [[section.field]] diff --git a/src/bmad_loop/data/skills/bmad-loop-setup/SKILL.md b/src/bmad_loop/data/skills/bmad-loop-setup/SKILL.md index 00bf7c887..5e3f1ee27 100644 --- a/src/bmad_loop/data/skills/bmad-loop-setup/SKILL.md +++ b/src/bmad_loop/data/skills/bmad-loop-setup/SKILL.md @@ -123,7 +123,7 @@ Unless the user explicitly asked to skip it (e.g. `skills only` / `--no-tool`), `validate` exits non-zero when the project isn't fully ready (e.g. no `sprint-status.yaml` yet, or `bmad-sprint-planning` hasn't run). On a fresh project that is **expected** — report its findings to the user as a readiness checklist, not as an install failure. -5. **Point the user at per-role adapter config.** `--cli` in step 3 only registers _hooks_ for each CLI. Which CLI actually **runs** each stage is governed by `{project-root}/.bmad-loop/policy.toml`, written from a template by `init`. The `[adapter] name` (default `claude`) applies to every stage; optional `[adapter.dev]`, `[adapter.review]`, and `[adapter.triage]` tables override individual stages (each takes its own `name` and `extra_args`). So a mixed setup — e.g. `claude` for dev, `codex` for review — needs both the hooks registered (step 3) **and** the role pointed at that CLI in `policy.toml`: +5. **Point the user at per-role adapter config.** `--cli` in step 3 only registers _hooks_ for each CLI. Which CLI actually **runs** each stage is governed by `{project-root}/.bmad-loop/policy.toml`, written from a template by `init`. The `[adapter] name` (default `claude`) applies to every stage; optional `[adapter.dev]`, `[adapter.review]`, and `[adapter.triage]` tables override individual stages (each takes its own `name`, `model`, `effort` and `extra_args`). So a mixed setup — e.g. `claude` for dev, `codex` for review — needs both the hooks registered (step 3) **and** the role pointed at that CLI in `policy.toml`: ```toml [adapter] diff --git a/src/bmad_loop/engine.py b/src/bmad_loop/engine.py index aac63ccdf..1db102252 100644 --- a/src/bmad_loop/engine.py +++ b/src/bmad_loop/engine.py @@ -6698,6 +6698,7 @@ def _run_session( cwd=self.workspace.root, env=env, model=cfg.model, + effort=cfg.effort, timeout_s=self._session_timeout_s(self.policy.limits.session_timeout_min * 60), stall_nudges_cap=( self.policy.limits.workflow_stall_nudges_cap diff --git a/src/bmad_loop/policy.py b/src/bmad_loop/policy.py index 6cf0228bf..319af2467 100644 --- a/src/bmad_loop/policy.py +++ b/src/bmad_loop/policy.py @@ -363,12 +363,29 @@ class CleanupPolicy: clean_tmp: bool = True # let engine plugins clean their /tmp scratch (e.g. Unity MCP zips) +# Legacy adapter names from older policy.toml files, plus friendly short names, +# mapped to the canonical profile name. Owned here — not in `adapters.profile`, +# which re-exports it as `ALIASES` for `get_profile` — because `[adapter] name` +# semantics are a policy fact: `AdapterPolicy.resolved()` must see "opencode" and +# "opencode-http" as the SAME client, or a stage naming the other spelling would +# be treated as a client switch and lose the inherited model/effort/extra_args. +PROFILE_ALIASES: dict[str, str] = {"claude-code-tmux": "claude", "opencode": "opencode-http"} + + +def canonical_profile_name(name: str) -> str: + """The profile name `get_profile` resolves `name` to — aliases collapsed.""" + return PROFILE_ALIASES.get(name, name) + + @dataclass(frozen=True) class StageAdapterPolicy: """Per-stage overrides; None = inherit from [adapter].""" name: str | None = None model: str | None = None + # Reasoning effort, free-form (provider- and model-specific names); None = + # inherit from [adapter] under the same client-specific rule as `model`. + effort: str | None = None extra_args: tuple[str, ...] | None = None # None = inherit from [adapter] (which itself falls back to the CLI profile) usage_grace_s: float | None = None @@ -385,12 +402,20 @@ class ResolvedAdapter: # limits.stop_without_result_nudges respectively usage_grace_s: float | None = None stop_without_result_nudges: int | None = None + # Reasoning effort; "" = provider default. Only the opencode-http adapter + # carries it (as the per-prompt `variant`); the tmux generic family has no + # channel for it and ignores it (`bmad-loop validate` warns). Appended AFTER + # the older fields because `resolved()` constructs this positionally. + effort: str = "" @dataclass(frozen=True) class AdapterPolicy: name: str = "claude" # CLI profile name; "claude-code-tmux" kept as legacy alias model: str = "" + # Reasoning effort for every stage that runs this client; free-form because + # the legal names are provider- and model-specific ("" = provider default). + effort: 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 @@ -413,12 +438,15 @@ def resolved(self, role: str) -> ResolvedAdapter: self.extra_args, self.usage_grace_s, self.stop_without_result_nudges, + effort=self.effort, ) 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). - same_client = name == self.name + # model, effort 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, provider default effort, + # profile bypass flags). Compared by CANONICAL name: `opencode` and + # `opencode-http` are one profile, not a switch. + same_client = canonical_profile_name(name) == canonical_profile_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 ?? # base inheritance is safe regardless of a client switch. @@ -438,6 +466,9 @@ def resolved(self, role: str) -> ResolvedAdapter: if stage.stop_without_result_nudges is not None else self.stop_without_result_nudges ), + effort=( + stage.effort if stage.effort is not None else (self.effort if same_client else "") + ), ) @@ -464,9 +495,11 @@ def _stage_from_snapshot(raw: Any) -> StageAdapterPolicy: return StageAdapterPolicy() name = raw.get("name") model = raw.get("model") + effort = raw.get("effort") return StageAdapterPolicy( name=None if name is None else str(name), model=None if model is None else str(model), + effort=None if effort is None else str(effort), extra_args=_snapshot_extra_args(raw.get("extra_args")), usage_grace_s=raw.get("usage_grace_s"), stop_without_result_nudges=raw.get("stop_without_result_nudges"), @@ -501,6 +534,7 @@ def adapter_policy_from_snapshot(snapshot: dict[str, Any] | None) -> AdapterPoli return AdapterPolicy( name=name, model=str(adapter_d.get("model", AdapterPolicy.model)), + effort=str(adapter_d.get("effort", AdapterPolicy.effort)), extra_args=_snapshot_extra_args(adapter_d.get("extra_args")), cleanup_session_on_finish=bool( adapter_d.get("cleanup_session_on_finish", AdapterPolicy.cleanup_session_on_finish) @@ -685,6 +719,7 @@ def _stage_adapter(adapter_d: dict[str, Any], key: str) -> StageAdapterPolicy: return StageAdapterPolicy( name=_opt_typed_str(raw, f"adapter.{key}", "name"), model=_opt_typed_str(raw, f"adapter.{key}", "model"), + effort=_opt_typed_str(raw, f"adapter.{key}", "effort"), extra_args=_typed_str_tuple(raw, f"adapter.{key}", "extra_args"), usage_grace_s=_opt_grace(raw, f"adapter.{key}"), stop_without_result_nudges=_opt_nudges(raw, f"adapter.{key}"), @@ -1044,6 +1079,7 @@ def loads(text: str, plugin_schemas: dict[str, Any] | None = None) -> Policy: adapter = AdapterPolicy( name=_typed_str(adapter_d, "adapter", "name", AdapterPolicy.name), model=_typed_str(adapter_d, "adapter", "model", AdapterPolicy.model), + effort=_typed_str(adapter_d, "adapter", "effort", AdapterPolicy.effort), extra_args=_typed_str_tuple(adapter_d, "adapter", "extra_args"), cleanup_session_on_finish=_typed_bool( adapter_d, @@ -1379,6 +1415,8 @@ 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") +effort = "" # reasoning effort, free-form (e.g. "high", "max"); empty = provider default. + # Sent by opencode-http as the per-prompt `variant`; the tmux CLIs ignore it 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"] @@ -1390,9 +1428,9 @@ 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, effort +# and extra_args are client-specific). Stage tables must come after the +# [adapter] keys above. # [adapter.dev] # model = "opus" # [adapter.review] @@ -1401,6 +1439,10 @@ def _fold_deprecated_engine( # stop_without_result_nudges = 5 # e.g. a multi-turn review needs more nudges than dev # [adapter.triage] # model = "opus" +# With an opencode-http base, effort tunes reasoning per stage (opencode-http +# only — a tmux CLI ignores it and `bmad-loop validate` warns): +# [adapter.review] +# effort = "max" # e.g. a deeper review pass than dev [sweep] # Deferred-work sweep: triage + execute open deferred-work.md entries. diff --git a/src/bmad_loop/resolve.py b/src/bmad_loop/resolve.py index bb90a4bf1..8ea8f2c20 100644 --- a/src/bmad_loop/resolve.py +++ b/src/bmad_loop/resolve.py @@ -517,6 +517,7 @@ def run_session( *, generation: int, model: str = "", + effort: str = "", ) -> bool: """Launch the interactive resolve agent attached to the caller's terminal. @@ -555,6 +556,7 @@ def run_session( "BMAD_LOOP_RESOLVE_CONTEXT": str(context_path(run_dir, story_key)), }, model=model, + effort=effort, ) # Drop any marker from a previous resolve of this story: otherwise the agent # sees it and reports "already resolved", and a session that records nothing diff --git a/tests/test_adapter_registry.py b/tests/test_adapter_registry.py index 3b9e707ff..73fc994ca 100644 --- a/tests/test_adapter_registry.py +++ b/tests/test_adapter_registry.py @@ -1141,6 +1141,27 @@ def test_validate_model_format_check_keys_on_the_adapter_kind_not_hooklessness( assert any(f["check"] == "adapter.hookless" for f in findings) +def test_validate_effort_silent_on_an_out_of_tree_kind(fresh_adapter_registry, project, capsys): + """`policy.effort-unsupported` is a fact about the bundled tmux GENERIC family + ("no channel for effort"); whether an out-of-tree kind can carry it is not + knowable here, so the check must stay silent for one rather than assert a + capability it cannot see. + + The `adapter.kind == "ok"` assert is the control: the profile loaded and its + kind resolved, so the absent warning is the predicate, not a failed load. + + ABLATION: flip the predicate to `prof.adapter != adapter_registry.OPENCODE_HTTP` + and this reddens.""" + fresh_adapter_registry.register_adapter("hermes", needs_mux=False, load=lambda: _stub_builder()) + install_bmad_config(project) + _write_profile(project.project, "hermes", adapter="hermes") + _write_policy(project.project, '[adapter]\nname = "hermes"\neffort = "max"\n') + + findings = _validate_findings(project.project, capsys) + assert not any(f["check"] == "policy.effort-unsupported" for f in findings) + assert [f["severity"] for f in findings if f["check"] == "adapter.kind"] == ["ok"] + + def test_validate_model_format_warns_on_an_opencode_kind_carrying_a_hook_dialect( fresh_adapter_registry, project, capsys ): @@ -1165,6 +1186,49 @@ def test_validate_model_format_warns_on_an_opencode_kind_carrying_a_hook_dialect assert all("haiku" in f["message"] for f in findings) +def _dry_run_dev_line(project, capsys) -> str: + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + args = argparse.Namespace(epic=None, story=None, max_stories=None) + assert cli._dry_run(project, pol, args) == 0 + out = capsys.readouterr().out + return next(line for line in out.splitlines() if "dev:" in line) + + +def test_dry_run_previews_the_http_sequence_for_an_opencode_kind_with_a_hook_dialect( + fresh_adapter_registry, project, capsys +): + """`_render_invocation` follows the adapter KIND, as `make_adapters` does. An + `opencode-http` profile carrying a hook dialect is a legal profile that still + launches the HTTP adapter — and sends `effort` as `variant` — so its preview + is the server/prompt_async line with the effort, not a tmux argv. + + ABLATION: key the branch on `profile.hookless` and this reddens (argv render, + no effort).""" + _write_profile(project.project, "ochooked", adapter="opencode-http", hookless=False) + _write_policy( + project.project, + '[adapter]\nname = "ochooked"\nmodel = "anthropic/claude-x"\neffort = "max"\n', + ) + line = _dry_run_dev_line(project, capsys) + assert "ochooked serve --hostname 127.0.0.1 --port " in line + assert line.endswith("model=anthropic/claude-x effort=max") + + +def test_dry_run_previews_argv_for_a_hookless_profile_of_another_kind( + fresh_adapter_registry, project, capsys +): + """The other direction: a hookless profile owned by an out-of-tree kind must + not draw the OpenCode server line (its transport is unknown here, so the + fallback is the profile's own binary/argv shape, as for every non-HTTP kind).""" + fresh_adapter_registry.register_adapter("hermes", needs_mux=False, load=lambda: _stub_builder()) + _write_profile(project.project, "hermes", adapter="hermes") # hookless=True + _write_policy(project.project, '[adapter]\nname = "hermes"\nmodel = "m1"\n') + line = _dry_run_dev_line(project, capsys) + assert "serve --hostname" not in line and "prompt_async" not in line + assert "hermes" in line and "--model m1" in line + + def test_validate_flags_an_unregistered_adapter_kind(fresh_adapter_registry, project, capsys): """`adapter.kind` is resolved against the live registry, so a profile naming a kind no installed package provides is a FAIL that names the known set.""" diff --git a/tests/test_cli.py b/tests/test_cli.py index 52699389c..f040c624f 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -5011,7 +5011,7 @@ def test_resolve_passes_the_tasks_own_generation_to_the_session(tmp_path, monkey save_state(run_dir, state) seen: list[int] = [] - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): seen.append(generation) marker = resolve.resolution_path(rd, story_key) marker.parent.mkdir(parents=True, exist_ok=True) @@ -5028,6 +5028,35 @@ def fake_session(adapter, project, rd, story_key, *, generation, model=""): assert load_state(run_dir).tasks["s1"].generation == 3 # the re-arm bumped past it +def test_resolve_hands_the_dev_stage_model_and_effort_to_the_session(tmp_path, monkeypatch): + """`cmd_resolve` launches the resolve agent as the DEV stage's client, so it + passes that stage's resolved `model` and (#643) `effort` — a stage override, + not the base value, proving the read goes through `resolved("dev")`.""" + from bmad_loop import resolve + + _escalated_run(tmp_path, "r1") + _write_policy( + tmp_path, + '[adapter]\nname = "claude"\nmodel = "opus"\neffort = "low"\n' + '[adapter.dev]\nmodel = "sonnet"\neffort = "max"\n', + ) + seen: dict[str, str] = {} + + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): + seen.update(model=model, effort=effort) + marker = resolve.resolution_path(rd, story_key) + marker.parent.mkdir(parents=True, exist_ok=True) + marker.write_text("{}", encoding="utf-8") + return True + + monkeypatch.setattr(cli, "_make_adapters", lambda *a, **k: {"dev": object()}) + monkeypatch.setattr(resolve, "build_context", lambda *a, **k: (None, 0, 0)) + monkeypatch.setattr(resolve, "run_session", fake_session) + assert cli.main(["resolve", "--project", str(tmp_path), "r1", "--no-resume"]) == 0 + + assert seen == {"model": "sonnet", "effort": "max"} + + def test_resolve_interactive_unsupported_adapter(tmp_path, monkeypatch, capsys): from bmad_loop import resolve @@ -5207,7 +5236,7 @@ def _redrive_escalates(run_dir, detail): def _marker_writing_session(run_dir_marker=True): from bmad_loop import resolve - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): marker = resolve.resolution_path(rd, story_key) marker.parent.mkdir(parents=True, exist_ok=True) if run_dir_marker: @@ -5708,7 +5737,7 @@ def test_resolve_restore_patch_unresolvable_from_resolution_json_rejected( run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) ran: list = [] - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): # the resolve agent records a restore_patch in its output marker ran.append(story_key) marker = resolve.resolution_path(rd, story_key) @@ -5893,7 +5922,7 @@ def test_resolve_interactive_restore_patch_from_resolution_json(tmp_path, monkey patch.write_text("diff", encoding="utf-8") run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): # the resolve agent records a restore_patch in its output marker marker = resolve.resolution_path(rd, story_key) marker.parent.mkdir(parents=True, exist_ok=True) @@ -5949,7 +5978,7 @@ def test_resolve_rereads_isolation_after_the_agent_session( _write_policy(tmp_path, '[scm]\nisolation = "none"\n') _escalated_run(tmp_path, "r1", spec_file=str(spec)) - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): # the human and the agent conclude the story needs isolation, and the operator # edits policy.toml from another terminal while the session is still open if flipped_mid_session: @@ -6000,7 +6029,7 @@ def test_resolve_corrupt_resolution_json_aborts_loudly(tmp_path, monkeypatch, ca spec.write_text("---\nstatus: blocked\n---\n", encoding="utf-8") run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): marker = resolve.resolution_path(rd, story_key) marker.parent.mkdir(parents=True, exist_ok=True) marker.write_text('{"restore_patch": "artifacts/attempt.patch",}', encoding="utf-8") @@ -6031,7 +6060,7 @@ def test_resolve_empty_restore_patch_field_aborts_loudly(tmp_path, monkeypatch, spec.write_text("---\nstatus: blocked\n---\n", encoding="utf-8") run_dir = _escalated_run(tmp_path, "r1", spec_file=str(spec)) - def fake_session(adapter, project, rd, story_key, *, generation, model=""): + def fake_session(adapter, project, rd, story_key, *, generation, model="", effort=""): marker = resolve.resolution_path(rd, story_key) marker.parent.mkdir(parents=True, exist_ok=True) marker.write_text(json.dumps({"restore_patch": ""}), encoding="utf-8") @@ -9505,6 +9534,27 @@ def test_dry_run_renders_hookless_http_line(project, capsys): # the profile's codex-style template is rendered into the prompt_async body assert "Use the bmad-dev-auto skill now:" in dev_line assert "model=anthropic/claude-haiku-4-5" in dev_line + assert "effort=" not in dev_line # unset → absent, like model + + +def test_dry_run_hookless_line_shows_the_stage_effort(project, capsys): + """#643: the real session sends the stage's effort as `variant`, so the launch + plan names it (under the policy key) beside the model — a preview that read + the same with and without it could not confirm the per-stage configuration.""" + write_sprint(project, {"epic-1": "backlog", "1-1-a": "ready-for-dev"}) + _write_policy( + project.project, + OPENCODE_QUALIFIED_POLICY + 'effort = "low"\n[adapter.dev]\neffort = "max"\n', + ) + pol = policy_mod.load(project.project / ".bmad-loop" / "policy.toml") + args = argparse.Namespace(epic=None, story=None, max_stories=None) + + assert cli._dry_run(project, pol, args) == 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) + assert dev_line.endswith("model=anthropic/claude-haiku-4-5 effort=max") + assert review_line.endswith("model=anthropic/claude-haiku-4-5 effort=low") def test_validate_warns_on_bare_opencode_model(project, capsys): @@ -9540,6 +9590,63 @@ def test_validate_model_warning_ignores_tmux_profiles(project, capsys): assert "is not 'provider/model'" not in _validate_output(capsys) +def test_validate_warns_when_effort_is_set_on_a_tmux_profile(project, capsys): + """#643: only the opencode-http adapter carries a reasoning-effort value. A + stage that sets `effort` on the tmux generic family runs at the provider + default with nothing to show for it, so validate says so — advisory, naming + the role, profile and value.""" + install_bmad_config(project) + _write_policy(project.project, '[adapter]\nname = "claude"\n[adapter.dev]\neffort = "high"\n') + write_sprint(project, {"epic-1": "backlog"}) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys, rc=1) + findings = [f for f in doc["findings"] if f["check"] == "policy.effort-unsupported"] + assert [f["severity"] for f in findings] == ["warning"] # dev only: review/triage unset + assert findings[0]["detail"] == {"role": "dev", "effort": "high", "profile": "claude"} + assert "dev effort 'high' is ignored by claude" in findings[0]["message"] + + +def test_validate_effort_warning_does_not_change_the_exit_code(project, capsys, monkeypatch): + """The warning is advisory: an otherwise-clean project with effort on a tmux + stage still exits 0 (the same rc as `test_validate_json_clean_project_is_a_ + pure_document_at_rc_0`, whose fixture this shares minus the key).""" + _make_validate_pass( + project, monkeypatch, capsys, policy=CLAUDE_ONLY_POLICY + 'effort = "high"\n' + ) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys) + assert doc["ok"] is True + warned = [f for f in doc["findings"] if f["check"] == "policy.effort-unsupported"] + # base effort inherits into every stage that keeps the client, so all three warn + assert sorted(f["detail"]["role"] for f in warned) == ["dev", "review", "triage"] + assert {f["severity"] for f in warned} == {"warning"} + + +def test_validate_effort_silent_on_the_opencode_kind(project, capsys): + """The carrier: effort on an opencode-http stage draws no warning. + + ABLATION: drop the `prof.adapter == GENERIC` predicate and this reddens (the + check would fire for the one family that actually sends the value).""" + install_bmad_config(project) + _write_policy(project.project, OPENCODE_QUALIFIED_POLICY + 'effort = "max"\n') + write_sprint(project, {"epic-1": "backlog"}) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys, rc=1) + assert not any(f["check"] == "policy.effort-unsupported" for f in doc["findings"]) + # control: the policy loaded with the value in it (not silent for lack of a key) + assert policy_mod.load(project.project / ".bmad-loop" / "policy.toml").adapter.effort == "max" + + +def test_validate_effort_silent_when_unset(project, capsys): + """No effort anywhere → no finding, on the very profile that would warn.""" + install_bmad_config(project) + _write_policy(project.project) # DUAL_CLIENT_POLICY: claude + codex, no effort + write_sprint(project, {"epic-1": "backlog"}) + + doc = machine_json(["validate", "--project", str(project.project), "--json"], capsys, rc=1) + assert not any(f["check"] == "policy.effort-unsupported" for f in doc["findings"]) + + 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_engine.py b/tests/test_engine.py index d95091380..d321328e7 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -7536,7 +7536,8 @@ def test_per_stage_adapter_and_model_dispatch(project): adapter=AdapterPolicy( name="claude", model="opus", - review=StageAdapterPolicy(name="codex", model="gpt-5-codex"), + effort="high", + review=StageAdapterPolicy(name="codex", model="gpt-5-codex", effort="max"), ), ) engine = Engine( @@ -7555,6 +7556,9 @@ def test_per_stage_adapter_and_model_dispatch(project): assert [s.role for s in review_mock.sessions] == ["review"] assert dev_mock.sessions[0].model == "opus" assert review_mock.sessions[0].model == "gpt-5-codex" + # #643: the resolved per-stage effort rides the SessionSpec the same way + assert dev_mock.sessions[0].effort == "high" + assert review_mock.sessions[0].effort == "max" def test_review_loop_converges_within_budget(project): diff --git a/tests/test_generic_tmux.py b/tests/test_generic_tmux.py index 65fb66a55..9781657be 100644 --- a/tests/test_generic_tmux.py +++ b/tests/test_generic_tmux.py @@ -184,6 +184,21 @@ def test_build_command_gemini_uses_interactive_flag(tmp_path): assert cmd.endswith("--model sonnet") +@pytest.mark.parametrize("profile_name", ["claude", "codex", "gemini"]) +def test_effort_never_reaches_the_generic_argv_or_env(tmp_path, profile_name): + """#643: the tmux generic family has no channel for a reasoning-effort value, + so `SessionSpec.effort` is ignored — argv and env are byte-identical to an + effort-less spec's (and `config_digest` therefore stays untouched). The + carrier is the opencode-http adapter; validate warns about this family.""" + adapter = make_adapter(tmp_path, profile_name=profile_name) + plain = make_spec(tmp_path) + with_effort = dataclasses.replace(plain, effort="max") + assert with_effort.effort == "max" # the field landed (kept LAST on SessionSpec) + assert adapter.interactive_argv(with_effort) == adapter.interactive_argv(plain) + assert adapter.interactive_env(with_effort) == adapter.interactive_env(plain) + assert "max" not in adapter.build_command(with_effort) + + def test_extra_args_replace_profile_bypass(tmp_path): adapter = make_adapter(tmp_path, extra_args=("--custom-flag",)) cmd = adapter.build_command(make_spec(tmp_path)) diff --git a/tests/test_opencode_http.py b/tests/test_opencode_http.py index ba73cbcb7..1b11dfc95 100644 --- a/tests/test_opencode_http.py +++ b/tests/test_opencode_http.py @@ -457,6 +457,21 @@ def test_config_content_shapes(tmp_path): config = json.loads(adapter._config_content(spec_model)) assert config["model"] == "anthropic/claude-x" + # Reasoning effort never lands in the config blob (#643): the config schema + # has no top-level `variant`, and `agent..variant` is inert unless that + # agent also pins a model — it rides the prompt_async body instead. + spec_effort = SessionSpec( + task_id="t", + role="triage", + prompt="p", + cwd=tmp_path, + model="anthropic/claude-x", + effort="max", + ) + config = json.loads(adapter._config_content(spec_effort)) + assert config == json.loads(adapter._config_content(spec_model)) + assert "variant" not in json.dumps(config) and "effort" not in json.dumps(config) + def test_session_env_carries_contract(tmp_path): adapter = make_adapter(tmp_path) @@ -2144,6 +2159,49 @@ def test_e2e_result_less_stop_nudges_then_completes(tmp_path, fake_opencode): assert_server_gone(rec) +def test_e2e_effort_rides_every_prompt_body_as_variant(tmp_path, fake_opencode): + """#643: `SessionSpec.effort` is sent as the per-call `variant` on EVERY + prompt_async body — the initial prompt AND the wake-up nudge. A nudge that + dropped back to the provider default would be silent mid-session drift, so + the value is stashed once on the server session and emitted by the single + `_prompt` primitive both paths share.""" + launcher, rec = fake_opencode + adapter = make_adapter(tmp_path, binary=str(launcher)) + spec = make_spec(tmp_path, rec, "nudge-then-complete", effort="max") + + result = adapter.run(spec) + + assert result.status == "completed" + bodies = read_jsonl(rec / "prompts.jsonl") + assert len(bodies) == 2 # initial prompt + one nudge + assert bodies[1]["parts"][0]["text"] == NUDGE_TEXT + assert [b["variant"] for b in bodies] == ["max", "max"] + assert_server_gone(rec) + + +def test_e2e_effort_unset_omits_variant_from_every_prompt_body(tmp_path, fake_opencode): + """The inverse: with no effort the key is OMITTED, not sent empty, so the body + of an effort-less session is byte-identical to the pre-#643 shape + (`{"parts": [...]}` and nothing else) on the initial prompt and the nudge. + + ABLATION: drop the `if sess.variant` guard in `_prompt` (always send the key) + and this reddens.""" + launcher, rec = fake_opencode + adapter = make_adapter(tmp_path, binary=str(launcher)) + spec = make_spec(tmp_path, rec, "nudge-then-complete") + assert spec.effort == "" + + result = adapter.run(spec) + + assert result.status == "completed" + bodies = read_jsonl(rec / "prompts.jsonl") + assert len(bodies) == 2 + for body in bodies: + assert "variant" not in body + assert set(body) == {"parts"} + assert_server_gone(rec) + + def test_e2e_stall_after_nudge_budget(tmp_path, fake_opencode): launcher, rec = fake_opencode adapter = make_adapter(tmp_path, binary=str(launcher)) diff --git a/tests/test_policy.py b/tests/test_policy.py index e1bcbb56c..1cadd597c 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -175,6 +175,123 @@ def test_stage_client_switch_drops_base_model_and_extra_args(tmp_path): assert review == policy.ResolvedAdapter("codex", "", None) +def test_stage_client_switch_drops_base_effort(tmp_path): + """Effort is client-specific like `model`: its legal names belong to one + provider's models, so a stage that switches client must NOT inherit the base + value and falls back to "" (provider default). + + ABLATION: replace the `same_client` fallback for effort in `resolved()` with a + plain `self.effort` and the review assert reddens.""" + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +name = "opencode-http" +effort = "max" +[adapter.review] +name = "claude" +""") + pol = policy.load(p) + assert pol.adapter.resolved("review").effort == "" + # controls: the stages that keep the client inherit the base value + assert pol.adapter.resolved("dev").effort == "max" + assert pol.adapter.resolved("triage").effort == "max" + + +@pytest.mark.parametrize( + ("base", "stage"), + [("opencode", "opencode-http"), ("opencode-http", "opencode"), ("claude-code-tmux", "claude")], +) +def test_stage_naming_an_alias_of_the_base_client_is_not_a_switch(tmp_path, base, stage): + """`get_profile` resolves an alias and its canonical name to ONE profile, so a + stage spelling the base client the other way runs the same client and must + inherit the client-specific keys (model, effort, extra_args) rather than + falling back to that profile's defaults. + + ABLATION: compare raw names in `resolved()`'s `same_client` and every row + reddens on all three keys.""" + p = tmp_path / "policy.toml" + p.write_text(f""" +[adapter] +name = "{base}" +model = "anthropic/claude-x" +effort = "max" +extra_args = ["--foo"] +[adapter.review] +name = "{stage}" +""") + review = policy.load(p).adapter.resolved("review") + assert review.name == stage # the stage's own spelling is kept for get_profile + assert review.model == "anthropic/claude-x" + assert review.effort == "max" + assert review.extra_args == ("--foo",) + + +def test_profile_aliases_are_the_table_get_profile_uses(): + """`resolved()`'s same-client test and `get_profile`'s lookup must collapse + the same aliases: one table, re-exported, never two copies that can drift.""" + from bmad_loop.adapters import profile as profile_mod + + assert profile_mod.ALIASES is policy.PROFILE_ALIASES + for alias, canonical in policy.PROFILE_ALIASES.items(): + assert profile_mod.get_profile(alias).name == canonical + assert policy.canonical_profile_name(alias) == canonical + assert policy.canonical_profile_name("claude") == "claude" # canonical is a fixed point + + +def test_base_effort_inherits_into_every_stage(tmp_path): + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +name = "opencode-http" +effort = "high" +""") + pol = policy.load(p) + assert pol.adapter.effort == "high" + for role in ("dev", "review", "triage"): + assert pol.adapter.resolved(role).effort == "high" + # the base-only (unknown role) branch constructs ResolvedAdapter positionally + # and must carry effort too + assert pol.adapter.resolved("retro").effort == "high" + + +def test_stage_effort_overrides_base(tmp_path): + p = tmp_path / "policy.toml" + p.write_text(""" +[adapter] +effort = "low" +[adapter.review] +effort = "max" +""") + pol = policy.load(p) + assert pol.adapter.resolved("review").effort == "max" + assert pol.adapter.resolved("dev").effort == "low" + assert pol.adapter.resolved("triage").effort == "low" + + +def test_effort_defaults_empty_everywhere(tmp_path): + pol = policy.load(None) + assert pol.adapter.effort == "" + for role in ("dev", "review", "triage", "retro"): + assert pol.adapter.resolved(role).effort == "" + + +@pytest.mark.parametrize( + ("body", "match"), + [ + ("[adapter]\neffort = 3\n", r"adapter\.effort must be a string"), + ("[adapter.dev]\neffort = 3\n", r"adapter\.dev\.effort must be a string"), + ("[adapter.review]\neffort = true\n", r"adapter\.review\.effort must be a string"), + ], +) +def test_effort_wrong_type_rejected(tmp_path, body, match): + """Effort is free-form (no catalog validation) but it IS typed: a non-string is + loud at policy load, like every other `[adapter]` string key.""" + p = tmp_path / "policy.toml" + p.write_text(body) + with pytest.raises(policy.PolicyError, match=match): + policy.load(p) + + def test_stage_same_client_inherits_and_overrides(tmp_path): p = tmp_path / "policy.toml" p.write_text(""" @@ -233,6 +350,12 @@ 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) base effort inherited by every stage + '[adapter]\nname = "opencode-http"\neffort = "high"\n', + # (e) a stage effort override beside a base one + '[adapter]\nname = "opencode-http"\neffort = "low"\n[adapter.review]\neffort = "max"\n', + # (f) a stage name override — the client switch resets effort to "" + '[adapter]\nname = "opencode-http"\neffort = "max"\n[adapter.review]\nname = "claude"\n', ], ) def test_adapter_policy_from_snapshot_roundtrips_resolved(body): diff --git a/tests/test_resolve.py b/tests/test_resolve.py index 0378a2e2e..6e078aa6f 100644 --- a/tests/test_resolve.py +++ b/tests/test_resolve.py @@ -3684,6 +3684,28 @@ def _minted_id(tmp_path, monkeypatch, story_key, generation) -> str: return adapter.specs[0].task_id +def test_run_session_threads_model_and_effort_onto_the_spec(tmp_path, monkeypatch): + """`cmd_resolve` reads both from `pol.adapter.resolved("dev")`; `run_session` + must hand both to the adapter on the spec (#643 for effort), and default both + to "" so every existing caller stays byte-identical.""" + monkeypatch.setattr(resolve.subprocess, "run", lambda *a, **k: None) + adapter = _SpecCapture() + resolve.run_session( + adapter, + tmp_path, + tmp_path / "run", + "6-4-cli-list-command", + generation=0, + model="anthropic/claude-x", + effort="max", + ) + assert adapter.specs[0].model == "anthropic/claude-x" + assert adapter.specs[0].effort == "max" + plain = _SpecCapture() + resolve.run_session(plain, tmp_path, tmp_path / "run", "6-4-cli-list-command", generation=0) + assert plain.specs[0].model == "" and plain.specs[0].effort == "" + + def test_run_session_id_is_byte_identical_to_the_hand_mint_at_generation_zero( tmp_path, monkeypatch ):