diff --git a/.ai/allowlist.json b/.ai/allowlist.json index 62d401ba..ea672e43 100644 --- a/.ai/allowlist.json +++ b/.ai/allowlist.json @@ -25,22 +25,33 @@ "codex_reasoning_effort": "medium", "models": { "planner": "gpt-6-astra", - "plan-reviewer-a": "claude-opus-5", + "plan-reviewer-a": "claude-opus-5-5", "plan-reviewer-b": "gpt-6-astra", - "arbiter": "claude-opus-5", - "builder": "gpt-6-astra", - "code-reviewer-a": "claude-opus-5", + "arbiter": "claude-opus-5-5", + "builder": "gpt-5.6-sol", + "code-reviewer-a": "claude-opus-5-5", "code-reviewer-b": "gpt-6-astra", - "review-arbiter": "claude-opus-5", + "review-arbiter": "claude-opus-5-5", "fixer": "gpt-6-astra" }, + "effort": { + "planner": "medium", + "plan-reviewer-a": "medium", + "plan-reviewer-b": "medium", + "arbiter": "medium", + "builder": "medium", + "code-reviewer-a": "medium", + "code-reviewer-b": "medium", + "review-arbiter": "medium", + "fixer": "medium" + }, "quest_startup": { "branch_mode": "branch", "branch_prefix": "quest/", "worktree_root": ".worktrees/quest" }, "quest_id_format": "slug-first", - "review_mode": "full", + "review_mode": "auto", "fast_review_thresholds": { "max_files": 5, "max_loc": 300 @@ -253,7 +264,7 @@ "require_approval_before_commit": true, "require_approval_before_push": true, "require_approval_before_delete": true, - "max_plan_iterations": 4, + "max_plan_iterations": 3, "max_fix_iterations": 3 } } diff --git a/.ai/quest.md b/.ai/quest.md index 8d907a69..91e9d774 100644 --- a/.ai/quest.md +++ b/.ai/quest.md @@ -146,6 +146,7 @@ When a role invocation fails (missing/unparsable handoff), Quest uses a three-ti The Creator controls quest permissions via `.ai/allowlist.json`: - `auto_approve_phases` — which phases need human approval - `models` — default role model map (`planner`, `plan-reviewer-a`, `plan-reviewer-b`, `builder`, `code-reviewer-a`, `code-reviewer-b`, `arbiter`, `fixer`) +- `effort` — reasoning effort per role, applied on whichever runtime the role's model selects. New quests save these values in `orchestration.json`; runner dispatch reads the saved map. In the Quest source repository, run `scripts/quest_sync_model_defaults.py` after editing `models` or `effort` to update shipped fallbacks and static agent files. The generator requires complete role maps. Installed repositories may use partial overrides for runner-dispatched roles without regenerating shipped defaults. For Claude-led Claude roles, the native effort guard additionally requires the actual subagent file's `effort:` frontmatter to match the saved value; a mismatch blocks `Task(...)` dispatch. Configure matching frontmatter before starting a native quest, and do not regenerate shared agent files to bypass a mismatch on an in-flight quest. - `review_mode` — `auto` (default), `fast`, or `full` for Codex reviews - `fast_review_thresholds` — file/LOC thresholds for auto fast mode diff --git a/.ai/schemas/allowlist.schema.json b/.ai/schemas/allowlist.schema.json index 614897b6..256d93c2 100644 --- a/.ai/schemas/allowlist.schema.json +++ b/.ai/schemas/allowlist.schema.json @@ -45,7 +45,18 @@ "codex_reasoning_effort": { "type": "string", "enum": ["low", "medium", "high", "xhigh", "max", "ultra"], - "description": "Reasoning effort pinned for Codex roles in new quests. Must be supported by the selected model and dispatch surface." + "description": "Legacy fallback: reasoning effort for Codex roles absent from the per-role `effort` map. New quests should use `effort`." + }, + "effort": { + "type": "object", + "propertyNames": { + "enum": ["planner", "plan-reviewer-a", "plan-reviewer-b", "arbiter", "builder", "code-reviewer-a", "code-reviewer-b", "review-arbiter", "fixer"] + }, + "additionalProperties": { + "type": "string", + "enum": ["low", "medium", "high", "xhigh", "max", "ultra"] + }, + "description": "Reasoning effort per role, applied on whichever runtime the role's model selects. Partial overrides are supported at quest startup; the source-repository defaults generator requires every canonical role. `ultra` is Codex-only and blocks a Claude-backed role at sync or dispatch." }, "models": { "type": "object", diff --git a/.claude/agents/arbiter.md b/.claude/agents/arbiter.md index 929b2cd8..2957ac51 100644 --- a/.claude/agents/arbiter.md +++ b/.claude/agents/arbiter.md @@ -3,6 +3,7 @@ name: arbiter description: Gatekeeper for plan quality. Synthesizes reviews, filters noise, decides if plan is ready for implementation. tools: Read, Glob, Grep, Write model: inherit +effort: medium --- You are the Arbiter Agent in a quest orchestration system. diff --git a/.claude/agents/builder.md b/.claude/agents/builder.md index c7fe5fba..8730f3e0 100644 --- a/.claude/agents/builder.md +++ b/.claude/agents/builder.md @@ -3,6 +3,7 @@ name: builder description: Implements approved plans. Writes code, runs tests, produces PR description. tools: Read, Glob, Grep, Write, Edit, Bash model: inherit +effort: medium --- You are the Builder Agent in a quest orchestration system. diff --git a/.claude/agents/code-reviewer.md b/.claude/agents/code-reviewer.md index c055f50a..a98f730b 100644 --- a/.claude/agents/code-reviewer.md +++ b/.claude/agents/code-reviewer.md @@ -3,6 +3,7 @@ name: code-reviewer description: Reviews code changes for correctness, quality, security, and adherence to project patterns. tools: Read, Glob, Grep, Write model: inherit +effort: medium --- You are the Code Review Agent in a quest orchestration system. diff --git a/.claude/agents/fixer.md b/.claude/agents/fixer.md index 77c2d119..5970dfc1 100644 --- a/.claude/agents/fixer.md +++ b/.claude/agents/fixer.md @@ -3,6 +3,7 @@ name: fixer description: Fixes issues identified by code review. Applies targeted fixes and re-runs tests. tools: Read, Glob, Grep, Write, Edit, Bash model: inherit +effort: medium --- You are the Fixer Agent in a quest orchestration system. diff --git a/.claude/agents/plan-reviewer.md b/.claude/agents/plan-reviewer.md index 949b7109..f22dfda6 100644 --- a/.claude/agents/plan-reviewer.md +++ b/.claude/agents/plan-reviewer.md @@ -3,6 +3,7 @@ name: plan-reviewer description: Reviews implementation plans for feasibility, completeness, and alignment with acceptance criteria. tools: Read, Glob, Grep, Write model: inherit +effort: medium --- You are a Plan Review Agent in a quest orchestration system. diff --git a/.claude/agents/planner.md b/.claude/agents/planner.md index c91620ae..3fe797a8 100644 --- a/.claude/agents/planner.md +++ b/.claude/agents/planner.md @@ -3,6 +3,7 @@ name: planner description: Creates and refines implementation plans from quest briefs. Writes plans to .quest/ folder. tools: Read, Glob, Grep, Write, Edit, Bash model: inherit +effort: medium --- You are the Planner Agent in a quest orchestration system. diff --git a/.claude/agents/review-arbiter.md b/.claude/agents/review-arbiter.md index 5eebf0c0..3a75efb0 100644 --- a/.claude/agents/review-arbiter.md +++ b/.claude/agents/review-arbiter.md @@ -3,6 +3,7 @@ name: review-arbiter description: Impartial judge for the code-review phase. Adjudicates the two code-reviewer slot findings against the diff and emits the canonical review_findings.json. tools: Read, Glob, Grep, Write model: inherit +effort: medium --- You are the Review Arbiter Agent in a quest orchestration system. diff --git a/.opencode/opencode.json b/.opencode/opencode.json index 8a976ff4..0b119b21 100644 --- a/.opencode/opencode.json +++ b/.opencode/opencode.json @@ -41,7 +41,7 @@ "plan-reviewer-a": { "description": "Reviews implementation plans (Reviewer A - local subagent)", "mode": "subagent", - "model": "opencode/claude-opus-5", + "model": "opencode/claude-opus-5-5", "prompt": "{file:agents/plan-reviewer.md}", "permission": { "edit": { @@ -67,7 +67,7 @@ "arbiter": { "description": "Synthesizes dual reviews, renders APPROVE or ITERATE verdict", "mode": "subagent", - "model": "opencode/claude-opus-5", + "model": "opencode/claude-opus-5-5", "prompt": "{file:agents/arbiter.md}", "permission": { "edit": { @@ -80,7 +80,7 @@ "builder": { "description": "Implements approved plans (local subagent)", "mode": "subagent", - "model": "opencode/gpt-6-astra", + "model": "opencode/gpt-5.6-sol", "prompt": "{file:agents/builder.md}", "permission": { "edit": { @@ -105,7 +105,7 @@ "code-reviewer-a": { "description": "Reviews code changes (Reviewer A - local subagent)", "mode": "subagent", - "model": "opencode/claude-opus-5", + "model": "opencode/claude-opus-5-5", "prompt": "{file:agents/code-reviewer.md}", "permission": { "edit": { @@ -141,7 +141,7 @@ "review-arbiter": { "description": "Impartial judge for the code-review phase; adjudicates A/B findings against the diff and emits canonical review_findings.json", "mode": "subagent", - "model": "opencode/claude-opus-5", + "model": "opencode/claude-opus-5-5", "prompt": "{file:agents/review-arbiter.md}", "permission": { "edit": { diff --git a/.skills/quest/SKILL.md b/.skills/quest/SKILL.md index 8a1fc189..a0e32dd3 100644 --- a/.skills/quest/SKILL.md +++ b/.skills/quest/SKILL.md @@ -236,7 +236,8 @@ Before creating the quest folder, present the routing classification to the user - `claude_transport_resolved`: the `transport` field from the Step 2b preflight result (Codex-led sessions; `null` otherwise) - `claude_transport_downgraded`: compatibility field; write `false` for new runs (Codex-led preflight also emits `false`) - `codex_auth_mode`: persist the resolved Step 2b `checks.codex_auth_mode`, not a later allowlist re-read; pass it as `codex_auth_mode` to `write_default_from_allowlist`. In Codex-led startup retain the explicitly resolved mode for any later Claude-led resume; native Codex dispatch continues using the host identity. - - `codex_reasoning_effort`: copy the optional allowlist value verbatim and pass it to `write_default_from_allowlist`; validate with `validate_codex_reasoning_effort`. Omit it for older allowlists with no setting. Show the selected effort beside the model table before accepting defaults. + - `effort`: `.ai/allowlist.json` `.effort` passed verbatim to `write_default_from_allowlist`, which fills omitted roles from the shipped `DEFAULT_EFFORT`. Show the per-role effort beside the model table before accepting defaults. + - `codex_reasoning_effort`: legacy fallback for allowlists with no `.effort` map — copy the optional value verbatim and pass it through; validate with `validate_codex_reasoning_effort`. Omit it when unset. - `source: "default"` - `overridden_roles: []` - `preflight_validated_at: ` @@ -271,7 +272,7 @@ Before creating the quest folder, present the routing classification to the user - `models`: merged block (all 9 keys present; unused-in-mode roles still carry the default value) - `claude_role_transport` / `claude_transport_resolved`: same sourcing as the Y path above; `claude_transport_downgraded: false` for compatibility - `codex_auth_mode`: same resolved Step 2b mode as the Y path; pass it to `write_orchestration_json`. - - `codex_reasoning_effort`: same allowlist sourcing as the Y path; pass it to `write_orchestration_json`. Model-only overrides do not change effort; validate model support before dispatch. + - `effort` / `codex_reasoning_effort`: same allowlist sourcing as the Y path; resolve the new quest map with `build_default_effort(allowlist_effort)` before passing it and the scalar to `write_orchestration_json`. The low-level writer preserves partial maps for resume. Model-only overrides do not change effort; validate model support before dispatch. - `source: "overridden"` - `overridden_roles`: list of role names that were actually overridden - `preflight_validated_at: ` diff --git a/.skills/quest/delegation/workflow.md b/.skills/quest/delegation/workflow.md index c61f138d..d84f5212 100644 --- a/.skills/quest/delegation/workflow.md +++ b/.skills/quest/delegation/workflow.md @@ -38,14 +38,28 @@ Quest dispatch separates **runtime** from **entrypoint**: | Orchestrator | Selected role runtime | Entrypoint | Rule | |--------------|-----------------------|------------|------| | Codex-led | Codex | local Codex subagent (the `spawn_agent` tool family — versioned namespace such as `multi_agent_v2` varies by Codex CLI release — or repo-supported equivalent) | Use the saved model and Codex effort according to the explicit-selection contract below. Do not use Codex MCP. | -| Codex-led | Claude | `python3 scripts/quest_claude_runner.py` when `claude_transport_available` is true | The runner owns the transport underneath: background-agent (`scripts/quest_claude_bg_run.py`, `claude --bg`, subscription billing) when preflight proved it, or the bridge (`scripts/quest_claude_bridge.py`, `claude --print`) only when bridge was explicitly configured/selected. Pass `--model from .quest//orchestration.json>` and `--transport `. The exact `claude` model sentinel means use the Claude CLI/account default and must not be sent to the CLI as `--model claude`; concrete configured model strings pass through unchanged. Block with transport guidance if unavailable and no explicit Codex fallback exists. | +| Codex-led | Claude | `python3 scripts/quest_claude_runner.py` when `claude_transport_available` is true | The runner owns the transport underneath: background-agent (`scripts/quest_claude_bg_run.py`, `claude --bg`, subscription billing) when preflight proved it, or the bridge (`scripts/quest_claude_bridge.py`, `claude --print`) only when bridge was explicitly configured/selected. Pass `--model from .quest//orchestration.json>` and `--transport `. The runner resolves `effort.` from the same saved file and passes `claude --effort`; do not pass `--effort` yourself unless deliberately overriding the saved config. The exact `claude` model sentinel means use the Claude CLI/account default and must not be sent to the CLI as `--model claude`; concrete configured model strings pass through unchanged. Block with transport guidance if unavailable and no explicit Codex fallback exists. | | Claude-led | Codex | `python3 /scripts/quest_codex_runner.py role` when `codex_available` is true | The runner launches installed `codex exec`, reads saved model, effort and auth, and validates current-attempt artifacts. | -| Claude-led | Claude | native `Task(...)` | Use the orchestrator's native Claude task path. | +| Claude-led | Claude | native `Task(...)` | Use the orchestrator's native Claude task path after running the native Claude effort guard below. Native `Task(...)` has no effort argument to apply the saved `orchestration.json` value, so do not try to pass effort here; the only available control is the subagent's `effort:` frontmatter, generated from the allowlist by `scripts/quest_sync_model_defaults.py`. **Unverified:** on Claude Code 2.1.280 that key could not be shown to change behavior (an invalid value raises no warning, and low-vs-max showed no token separation), unlike `claude --effort`, which measurably does. Treat effort on this path as best-effort. Guaranteeing runtime effort requires a separate dispatch decision. | | Either orchestrator | Antigravity | `python3 scripts/quest_antigravity_runner.py` when `antigravity_available` is true | Selected by Gemini-family model IDs. Antigravity is never an orchestrator, only ever a dispatched runtime, so one runner serves both session types — there is no MCP path and no transport choice. Pass `--model from .quest//orchestration.json>`; the exact `gemini` sentinel means use the agy default model and must not be sent to the CLI as `--model gemini`. **Always pass `--add-dir` covering the quest directory** — see the Antigravity containment rule below. Block with the preflight `warning` lines if unavailable. | -**Explicit model and effort selection:** Before every Codex role dispatch, read `models.` and optional `codex_reasoning_effort` from the active quest's `orchestration.json`. Do not substitute defaults from this skill, the GPT skill, or the current allowlist. For local subagents, pass the exact `model` and, when set, `reasoning_effort` through the tool's exposed controls. This repository configuration authorizes explicit selection. When the tool requires a fresh or bounded context fork for overrides, use that mode and include the role instructions and artifact paths in the prompt. If the controls are unavailable, inherit only after verifying the parent matches the saved model and any pinned effort; otherwise stop and report the mismatch. Never substitute MCP or nested `codex exec` for Codex-led dispatch. +**Native Claude effort guard:** Before every Claude-led Claude `Task(...)`, validate the saved role against the actual subagent file that Task will load. Run the following with absolute paths and the selected role. A nonzero exit blocks dispatch. Do not rewrite saved settings, regenerate global agent files, or switch dispatch paths to bypass a mismatch. A role the quest does not pin passes the guard: the agent file simply carries the repo default, so there is no disagreement to report. Matching frontmatter remains best-effort, not proof of effective runtime effort. -For Claude-led Codex calls, use the installed runner's `role` mode. It reads `models.`, optional `codex_reasoning_effort` and `codex_auth_mode` (legacy default `cached`) from the saved orchestration file. Do not pass model, effort or auth overrides in role mode. Unsupported settings block dispatch. Log requested settings separately from effective settings; effective values require runtime metadata, otherwise record `unknown`. +```bash +PYTHONPATH="/scripts" python3 - "/orchestration.json" "" "" <<'PY' +import json +import sys +from pathlib import Path +from quest_runtime.orchestration import validate_native_claude_effort +validate_native_claude_effort( + json.loads(Path(sys.argv[1]).read_text()), sys.argv[2], Path(sys.argv[3]).read_text() +) +PY +``` + +**Explicit model and effort selection:** Before every Codex role dispatch, read `models.` and `effort.` from the active quest's `orchestration.json` (quests written before the per-role map fall back to the legacy scalar `codex_reasoning_effort`). Do not substitute defaults from this skill, the GPT skill, or the current allowlist. For local subagents, pass the exact `model` and, when set, `reasoning_effort` through the tool's exposed controls. This repository configuration authorizes explicit selection. When the tool requires a fresh or bounded context fork for overrides, use that mode and include the role instructions and artifact paths in the prompt. If the controls are unavailable, inherit only after verifying the parent matches the saved model and any pinned effort; otherwise stop and report the mismatch. Never substitute MCP or nested `codex exec` for Codex-led dispatch. + +For Claude-led Codex calls, use the installed runner's `role` mode. It reads `models.`, `effort.` (falling back to the legacy `codex_reasoning_effort`) and `codex_auth_mode` (legacy default `cached`) from the saved orchestration file. Do not pass model, effort or auth overrides in role mode. Unsupported settings block dispatch. Log requested settings separately from effective settings; effective values require runtime metadata, otherwise record `unknown`. **Orchestration violation:** A Codex-led attempt to dispatch another Codex role through MCP or nested `codex exec` is an entrypoint violation. Use local Codex subagents with the saved model and effort. Missing native controls or mismatched parent settings block, with no CLI/MCP substitution or saved-setting rewrite. diff --git a/scripts/quest_claude_bridge.py b/scripts/quest_claude_bridge.py index 6fadb64a..75fc9dcc 100755 --- a/scripts/quest_claude_bridge.py +++ b/scripts/quest_claude_bridge.py @@ -77,6 +77,12 @@ def parse_args(argv: list[str] | None = None) -> argparse.Namespace: default="", help="Optional Claude model override (passed through if provided)", ) + parser.add_argument( + "--effort", + default="", + choices=["", "low", "medium", "high", "xhigh", "max"], + help="Optional reasoning effort; empty uses the CLI/session default", + ) parser.add_argument( "--system-prompt", default="", @@ -160,6 +166,7 @@ def run_claude( add_dirs: list[str], allowed_tools: str, disallowed_tools: str, + effort: str = "", ) -> dict[str, Any]: cmd = ["claude", "--print", prompt, "--output-format", output_format] # `claude` is the runtime-family sentinel meaning account-default model — @@ -167,6 +174,8 @@ def run_claude( # quest layer normalizes it away, but direct callers reach here unfiltered). if model and model != "claude": cmd.extend(["--model", model]) + if effort: + cmd.extend(["--effort", effort]) if system_prompt: cmd.extend(["--system-prompt", system_prompt]) if append_system_prompt: @@ -240,6 +249,7 @@ def main(argv: list[str] | None = None) -> int: output_format=args.output_format, timeout=args.timeout, model=args.model, + effort=args.effort, system_prompt=args.system_prompt, append_system_prompt=args.append_system_prompt, permission_mode=args.permission_mode, diff --git a/scripts/quest_claude_runner.py b/scripts/quest_claude_runner.py index 9a183540..6b2084ed 100644 --- a/scripts/quest_claude_runner.py +++ b/scripts/quest_claude_runner.py @@ -20,6 +20,7 @@ from pathlib import Path from quest_runtime.artifacts import expected_artifacts_for_role +from quest_runtime.orchestration import effort_for_role from quest_runtime.claude_runner import ( DEFAULT_BG_RUNNER_SCRIPT, DEFAULT_BRIDGE_SCRIPT, @@ -79,6 +80,14 @@ def parse_args() -> argparse.Namespace: default=1800.0, help="Command timeout seconds (default: 1800)", ) + parser.add_argument( + "--effort", + choices=["low", "medium", "high", "xhigh", "max"], + help=( + "Reasoning effort. Omit to resolve effort. from the quest's " + "orchestration.json (the normal path); pass only to override it." + ), + ) parser.add_argument("--permission-mode", default="bypassPermissions") parser.add_argument( "--transport", @@ -127,6 +136,31 @@ def parse_args() -> argparse.Namespace: return args +def _resolve_effort(quest_dir: str, agent: str, override: str | None) -> str | None: + """Effort for this dispatch: explicit flag, else the saved per-role map. + + Resolving from orchestration.json here (rather than trusting the + orchestrator to pass it) mirrors Codex role mode and keeps the saved quest + config authoritative. A quest with no orchestration.json yet resolves to + None, which leaves the CLI default in place. + """ + if override: + return override + try: + saved = json.loads( + (Path(quest_dir) / "orchestration.json").read_text(encoding="utf-8") + ) + except FileNotFoundError: + return None + except (OSError, json.JSONDecodeError) as exc: + raise ValueError("Cannot read saved orchestration.json") from exc + if not isinstance(saved, dict): + raise ValueError("orchestration.json must be an object") + if not isinstance(saved.get("models", {}), dict): + raise ValueError("orchestration.json models must be an object") + return effort_for_role(saved, agent) + + def main() -> int: args = parse_args() transport = resolve_claude_transport(args.transport) @@ -149,6 +183,9 @@ def main() -> int: agent=args.agent, artifact_subset=args.artifact_subset, ) + effort = _resolve_effort( + args.quest_dir, args.agent, getattr(args, "effort", None) + ) except ValueError as exc: payload = { "exit_code": 1, @@ -184,6 +221,7 @@ def main() -> int: handoff_file=args.handoff_file, bridge_script=resolve_path(args.cwd, args.bridge_script), model=args.model, + effort=effort, timeout=args.timeout, permission_mode=args.permission_mode, artifact_paths=artifact_paths, diff --git a/scripts/quest_runtime/claude_runner.py b/scripts/quest_runtime/claude_runner.py index 5ba3c896..8829af66 100644 --- a/scripts/quest_runtime/claude_runner.py +++ b/scripts/quest_runtime/claude_runner.py @@ -24,7 +24,7 @@ check_artifact_paths, prepare_artifact_files, ) -from quest_runtime.orchestration import runtime_for_model +from quest_runtime.orchestration import CLAUDE_EFFORT_LEVELS, runtime_for_model from quest_runtime.plan_iterations import PlanIterationError from quest_runtime.state import StateError, utc_now_iso @@ -123,6 +123,26 @@ def normalize_claude_cli_model(model: str) -> str | None: return normalized +def normalize_claude_cli_effort(effort: str | None) -> str | None: + """Validate an effort level for `claude --effort`; None omits the flag. + + `ultra` is a Codex-only level. It is valid in the shared per-role `effort` + map but unreachable here, so a Claude role pinned to it fails loudly rather + than silently running at the CLI default. + """ + if effort is None: + return None + normalized = effort.strip() + if not normalized: + return None + if normalized not in CLAUDE_EFFORT_LEVELS: + raise ValueError( + f"Claude effort must be one of {'|'.join(CLAUDE_EFFORT_LEVELS)} " + f"(got {effort!r})" + ) + return normalized + + def _effective_permission_mode( permission_mode: str, permission_escalation: bool ) -> str: @@ -313,8 +333,10 @@ def build_bridge_cmd( timeout: float, permission_mode: str, add_dirs: Iterable[str | Path] | None = None, + effort: str | None = None, ) -> list[str]: cli_model = normalize_claude_cli_model(model) + cli_effort = normalize_claude_cli_effort(effort) cmd = [ sys.executable, str(bridge_script), @@ -329,6 +351,8 @@ def build_bridge_cmd( ] if cli_model is not None: cmd.extend(["--model", cli_model]) + if cli_effort is not None: + cmd.extend(["--effort", cli_effort]) if add_dirs: for directory in unique_dirs(add_dirs): cmd.extend(["--add-dir", directory]) @@ -350,6 +374,7 @@ def build_bg_cmd( teardown_on_needs_human: bool = False, resume: str | None = None, answer_file: str | Path | None = None, + effort: str | None = None, ) -> list[str]: """argv for the background-agent transport (scripts/quest_claude_bg_run.py). @@ -359,6 +384,7 @@ def build_bg_cmd( Direct callers with no relay can opt into --teardown-on-needs-human. """ cli_model = normalize_claude_cli_model(model) + cli_effort = normalize_claude_cli_effort(effort) cmd = [ sys.executable, str(bg_runner_script), @@ -382,6 +408,8 @@ def build_bg_cmd( cmd.extend(["--prompt-file", str(prompt_file)]) if cli_model is not None: cmd.extend(["--model", cli_model]) + if cli_effort is not None: + cmd.extend(["--effort", cli_effort]) if teardown_on_needs_human: cmd.append("--teardown-on-needs-human") for path in wait_for: @@ -709,7 +737,22 @@ def run_claude_role( teardown_on_needs_human: bool = False, resume: str | None = None, answer_file: str | Path | None = None, + effort: str | None = None, ) -> RunResult: + # Validate before preparing artifacts, so a rejected level cannot truncate + # an existing artifact on its way to failing — but keep the structured + # contract: callers parse a JSON envelope and must never get a traceback. + try: + normalize_claude_cli_effort(effort) + except ValueError as exc: + return RunResult( + exit_code=1, + handoff_state="missing", + result_kind="invocation_error", + source=None, + stdout="", + stderr=str(exc), + ) if transport not in {"bridge", "background-agent"}: raise ValueError( f"transport must be 'bridge' or 'background-agent' (got {transport!r})" @@ -796,6 +839,7 @@ def run_claude_role( teardown_on_needs_human=teardown_on_needs_human, resume=resume, answer_file=answer_file, + effort=effort, ) combined_stderr = retry_note if retry_result.stderr: @@ -836,6 +880,7 @@ def run_claude_role( teardown_on_needs_human=teardown_on_needs_human, resume=resume, answer_file=resolve_path(cwd, answer_file) if answer_file else None, + effort=effort, ) else: cmd = build_bridge_cmd( @@ -848,9 +893,11 @@ def run_claude_role( permission_mode, permission_escalation ), add_dirs=default_add_dirs, + effort=effort, ) except ValueError as exc: - # e.g. an empty models. value reaching normalize_claude_cli_model: + # e.g. an empty models. value, or a Codex-only `ultra` effort, + # reaching the normalizers: # library callers get a structured invocation_error, never a traceback. return RunResult( exit_code=1, @@ -1118,6 +1165,7 @@ def run_claude_role( teardown_on_needs_human=teardown_on_needs_human, resume=resume, answer_file=answer_file, + effort=effort, ) combined_stderr = retry_note if retry_result.stderr: diff --git a/scripts/quest_runtime/codex_runner.py b/scripts/quest_runtime/codex_runner.py index 344b2f0d..a5ee1f71 100644 --- a/scripts/quest_runtime/codex_runner.py +++ b/scripts/quest_runtime/codex_runner.py @@ -31,7 +31,11 @@ classify_handoff_file, read_handoff_status, ) -from .orchestration import runtime_for_model, validate_codex_reasoning_effort +from .orchestration import ( + effort_for_role, + runtime_for_model, + validate_codex_reasoning_effort, +) from .plan_iterations import PlanIterationError, verify_refinement from .review_intelligence import validate_findings from .state import StateError, load_state @@ -742,7 +746,7 @@ def run_codex_role( try: saved = json.loads((quest / "orchestration.json").read_text(encoding="utf-8")) model = saved["models"][agent] - effort = saved.get("codex_reasoning_effort") + effort = effort_for_role(saved, agent) auth = saved.get("codex_auth_mode", "cached") if ( not isinstance(model, str) diff --git a/scripts/quest_runtime/orchestration.py b/scripts/quest_runtime/orchestration.py index faf6853f..426a1940 100644 --- a/scripts/quest_runtime/orchestration.py +++ b/scripts/quest_runtime/orchestration.py @@ -14,6 +14,7 @@ from __future__ import annotations import json +import re from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -37,15 +38,26 @@ # BEGIN GENERATED MODEL DEFAULTS (edit .ai/allowlist.json, then run scripts/quest_sync_model_defaults.py) DEFAULT_MODELS: dict[str, str] = { "planner": "gpt-6-astra", - "plan-reviewer-a": "claude-opus-5", + "plan-reviewer-a": "claude-opus-5-5", "plan-reviewer-b": "gpt-6-astra", - "arbiter": "claude-opus-5", - "builder": "gpt-6-astra", - "code-reviewer-a": "claude-opus-5", + "arbiter": "claude-opus-5-5", + "builder": "gpt-5.6-sol", + "code-reviewer-a": "claude-opus-5-5", "code-reviewer-b": "gpt-6-astra", - "review-arbiter": "claude-opus-5", + "review-arbiter": "claude-opus-5-5", "fixer": "gpt-6-astra", } +DEFAULT_EFFORT: dict[str, str] = { + "planner": "medium", + "plan-reviewer-a": "medium", + "plan-reviewer-b": "medium", + "arbiter": "medium", + "builder": "medium", + "code-reviewer-a": "medium", + "code-reviewer-b": "medium", + "review-arbiter": "medium", + "fixer": "medium", +} CODEX_NATIVE_FALLBACK_MODEL = "gpt-6-astra" # END GENERATED MODEL DEFAULTS @@ -60,6 +72,12 @@ ORCHESTRATION_VERSION = 1 +# Reasoning-effort levels (.ai/allowlist.json effort). The union is the syntax +# vocabulary for the per-role map; `ultra` is Codex-only, so the Claude CLI +# subset excludes it and the Claude runner rejects it at dispatch. +EFFORT_LEVELS: tuple[str, ...] = ("low", "medium", "high", "xhigh", "max", "ultra") +CLAUDE_EFFORT_LEVELS: tuple[str, ...] = ("low", "medium", "high", "xhigh", "max") + # Transport for Codex-led Claude roles (.ai/allowlist.json claude_role_transport). # "auto" resolves to background-agent when the preflight bg probe succeeds. # If it fails, Quest asks the user; bridge is an explicit API-metered opt-in. @@ -464,19 +482,136 @@ def apply_overrides( def validate_codex_reasoning_effort(effort: str) -> None: """Validate syntax; the dispatch surface must also support the chosen level.""" - if effort not in ( - "low", - "medium", - "high", - "xhigh", - "max", - "ultra", - ): + if effort not in EFFORT_LEVELS: raise ValueError( "codex_reasoning_effort must be low|medium|high|xhigh|max|ultra" ) +def validate_effort(role: str, effort: object) -> None: + """Validate one `effort.` entry against the shared level vocabulary. + + Syntax only. Whether a level is reachable depends on the runtime the role's + model selects: `ultra` is Codex-only, so the Claude runner rejects it at + dispatch rather than here. One role map serves every runtime. + """ + if role not in CANONICAL_ROLES: + raise ValueError(f"effort has unknown role {role!r}") + if not isinstance(effort, str) or effort not in EFFORT_LEVELS: + raise ValueError( + f"effort for {role} must be one of {'|'.join(EFFORT_LEVELS)} " + f"(got {effort!r})" + ) + + +def validate_effort_map(effort: object) -> dict[str, str]: + """Validate a whole `effort` map and return a copy of it.""" + if not isinstance(effort, dict): + raise ValueError("effort must be an object keyed by role") + for role, level in effort.items(): + validate_effort(role, level) + return dict(effort) + + +def build_default_effort(allowlist_effort: object) -> dict[str, str]: + """Resolve the per-role effort map, filling gaps from shipped defaults.""" + merged = dict(DEFAULT_EFFORT) + if allowlist_effort is not None: + merged.update(validate_effort_map(allowlist_effort)) + return merged + + +def effort_for_role(saved: dict, role: str) -> str | None: + """Effort for one role from a saved orchestration.json. + + Resolution order: the per-role `effort` map, then the legacy scalar + `codex_reasoning_effort`, then None (runtime default). + + Two rules keep legacy quests behaving exactly as they did: + + - A present `effort` map is validated, never silently ignored. A malformed + map is a configuration error, not a reason to fall back. + - The legacy scalar is **Codex-scoped**, as its name says. Before the map + existed, Claude roles ran unpinned and the Claude runner never read this + key; applying it to them now would both re-tier legacy Claude roles and + hard-fail any quest that pinned the Codex-only `ultra`. + """ + effort = saved.get("effort") + if "effort" in saved: + level = validate_effort_map(effort).get(role) + if level: + return level + legacy = saved.get("codex_reasoning_effort") + if not isinstance(legacy, str) or not legacy: + return None + model = (saved.get("models") or {}).get(role) + if not isinstance(model, str) or not model.strip(): + return None + return legacy if runtime_for_model(model) == "codex" else None + + +_EFFORT_KEY_ALIAS = re.compile(r"""^\s*(?:"effort"|'effort')\s*:""") +_ANY_EFFORT_KEY = re.compile(r"""^\s*(?:"effort"|'effort'|effort)\s*:""") + + +def validate_native_claude_effort(saved: dict, role: str, agent_text: str) -> None: + """Guard native dispatch using the generator's flat frontmatter format. + + Reject other YAML forms rather than guessing at aliases or quoted keys. + Matching declarations do not prove Claude honors the effort setting. + """ + model = saved.get("models", {}).get(role) + if not isinstance(model, str) or runtime_for_model(model) != "claude": + raise ValueError(f"{role} is not an active Claude role") + requested = effort_for_role(saved, role) + if requested is None: + # The quest pins nothing for this role, so there is no disagreement to + # find: the agent file simply carries the repo-wide default. Blocking + # here would wall off every legacy quest on resume, since the generated + # frontmatter always names a level and an unpinned role never can. + return + if requested not in CLAUDE_EFFORT_LEVELS: + raise ValueError(f"Unsupported native Claude effort: {requested!r}") + lines = agent_text.splitlines() + if not lines or lines[0] != "---" or "---" not in lines[1:]: + raise ValueError("Agent file has no YAML frontmatter") + fields: dict[str, str] = {} + for line in lines[1 : lines.index("---", 1)]: + if not line.strip() or line.lstrip().startswith("#"): + continue + if line[:1].isspace() or line.lstrip().startswith("- "): + # Part of the previous key's nested block. `hooks:` and + # `mcpServers:` are valid subagent keys; refusing to dispatch + # because one is present would be a worse failure than the + # ambiguity it guards against. An `effort:` nested somewhere + # unexpected is the one thing we will not skip past. + if _ANY_EFFORT_KEY.match(line): + raise ValueError( + f"Native effort guard found an ambiguous `effort` key: " + f"{line.strip()!r}" + ) + continue + match = re.fullmatch(r"([a-zA-Z][a-zA-Z0-9_-]*):[ \t]*(.*)", line) + if match is None: + # A quoted or otherwise unusual key spelling. Only `effort` matters + # here, and an ambiguous one must not be read as agreement. + if _EFFORT_KEY_ALIAS.match(line): + raise ValueError( + "Native effort guard cannot read an ambiguous `effort` key: " + f"{line.strip()!r}" + ) + continue + if match[1] in fields: + raise ValueError(f"Duplicate frontmatter key {match[1]!r} in agent file") + fields[match[1]] = match[2].strip() + actual = fields.get("effort") + if actual != requested: + raise ValueError( + f"Native Claude effort mismatch for {role}: saved={requested!r}, " + f"frontmatter={actual!r}. Stop before Task dispatch." + ) + + def validate_codex_auth_mode(mode: str) -> None: """Validate an explicit billing choice; absence defaults to cached.""" if mode not in ("cached", "api-key"): @@ -493,6 +628,7 @@ def write_orchestration_json( claude_role_transport: str = DEFAULT_CLAUDE_ROLE_TRANSPORT, claude_transport_resolved: str | None = None, codex_reasoning_effort: str | None = None, + effort: dict[str, str] | None = None, codex_auth_mode: str = "cached", ) -> None: """Write the orchestration.json artifact with canonical key order.""" @@ -505,11 +641,16 @@ def write_orchestration_json( ) if codex_reasoning_effort is not None: validate_codex_reasoning_effort(codex_reasoning_effort) + # `None` means "this caller has no effort policy" — write no key at all, so + # a migrated legacy quest keeps resolving through whatever it already had. + # Only the new-quest writers below synthesize DEFAULT_EFFORT. + resolved_effort = None if effort is None else validate_effort_map(effort) validate_codex_auth_mode(codex_auth_mode) payload = { "version": ORCHESTRATION_VERSION, "codex_auth_mode": codex_auth_mode, "models": {role: models.get(role) for role in CANONICAL_ROLES}, + **({} if resolved_effort is None else {"effort": resolved_effort}), "claude_role_transport": claude_role_transport, "claude_transport_resolved": claude_transport_resolved, # Compatibility field for consumers created during the downgrade era. @@ -541,6 +682,7 @@ def write_default_from_allowlist( claude_role_transport: str = DEFAULT_CLAUDE_ROLE_TRANSPORT, claude_transport_resolved: str | None = None, codex_reasoning_effort: str | None = None, + effort: dict[str, str] | None = None, codex_auth_mode: str = "cached", ) -> None: """Default-path writer: copy allowlist models into orchestration.json. @@ -569,6 +711,10 @@ def write_default_from_allowlist( claude_role_transport=claude_role_transport, claude_transport_resolved=claude_transport_resolved, codex_reasoning_effort=codex_reasoning_effort, + # New quests always get a complete map: an allowlist without an `effort` + # block still means "use the shipped defaults", not "pin nothing". + # Migration is the opposite case and passes None through deliberately. + effort=build_default_effort(effort), codex_auth_mode=codex_auth_mode, ) @@ -598,6 +744,11 @@ def migrate_from_snapshot( return False if "codex_reasoning_effort" in existing: validate_codex_reasoning_effort(existing["codex_reasoning_effort"]) + # A saved effort map is validated but never backfilled: a quest written + # before the map existed keeps resolving through the legacy scalar, so + # resume cannot silently re-tier an in-flight quest. + if "effort" in existing: + validate_effort_map(existing["effort"]) validate_codex_auth_mode(existing.get("codex_auth_mode", "cached")) merged_models, backfilled = _backfill_legacy_compatible_roles(existing_models) # Transport keys were introduced after early quests; backfill in place @@ -628,12 +779,24 @@ def migrate_from_snapshot( # Fail closed BEFORE writing: a role missing from the merged models # would be written as null and rejected by the very validation this # migration feeds — never persist a file we know is invalid. + state_path = quest_dir / "state.json" + state = json.loads(state_path.read_text()) if state_path.exists() else {} + active_roles = active_roles_for_mode(state.get("quest_mode", "workflow")) missing_roles = [ - role for role in CANONICAL_ROLES if not merged_models.get(role) + role + for role in CANONICAL_ROLES + if role not in merged_models + or ( + role in active_roles + and ( + not isinstance(merged_models[role], str) + or not merged_models[role].strip() + ) + ) ] if missing_roles: raise ValueError( - "orchestration.json migration would write null model(s) for " + "orchestration.json migration would write invalid model(s) for " f"role(s) {missing_roles}; the existing file is malformed — " "fix models. entries before resuming." ) @@ -663,10 +826,13 @@ def migrate_from_snapshot( ) if "codex_reasoning_effort" in snapshot: validate_codex_reasoning_effort(snapshot["codex_reasoning_effort"]) + if "effort" in snapshot: + validate_effort_map(snapshot["effort"]) write_orchestration_json( orch_path, models=build_snapshot_models(models), codex_reasoning_effort=snapshot.get("codex_reasoning_effort"), + effort=snapshot.get("effort"), codex_auth_mode=snapshot.get("codex_auth_mode", "cached"), source="default", overridden_roles=[], diff --git a/scripts/quest_sync_model_defaults.py b/scripts/quest_sync_model_defaults.py index e4f6218f..32f54a65 100644 --- a/scripts/quest_sync_model_defaults.py +++ b/scripts/quest_sync_model_defaults.py @@ -8,13 +8,73 @@ import argparse import json +import re from pathlib import Path -from quest_runtime.orchestration import CANONICAL_ROLES, runtime_for_model +from quest_runtime.orchestration import ( + CANONICAL_ROLES, + CLAUDE_EFFORT_LEVELS, + runtime_for_model, + validate_effort, +) BEGIN = "# BEGIN GENERATED MODEL DEFAULTS" END = "# END GENERATED MODEL DEFAULTS" +# Claude-led Claude roles dispatch through native `Task(...)`, which reads +# effort from subagent frontmatter rather than from orchestration.json. Mapping +# each agent file to the role it serves lets the allowlist stay authoritative +# for that path too. The reviewer files serve the Claude-side slot (A); slot B +# is the Codex slot and is configured through the runner. +AGENT_FILE_ROLES: dict[str, str] = { + "planner.md": "planner", + "plan-reviewer.md": "plan-reviewer-a", + "arbiter.md": "arbiter", + "builder.md": "builder", + "code-reviewer.md": "code-reviewer-a", + "review-arbiter.md": "review-arbiter", + "fixer.md": "fixer", +} + + +_EFFORT_KEY = re.compile(r"""^\s*(?:"effort"|'effort'|effort)\s*:""") + + +def _with_effort(text: str, level: str) -> str: + """Set `effort:` in a subagent file's YAML frontmatter, preserving the rest.""" + if not text.startswith("---\n"): + raise ValueError("subagent file has no YAML frontmatter") + end = text.index("\n---\n", 3) + lines = text[4:end].split("\n") + # Drop every spelling of the key, not just the one we emit: a quoted + # `"effort": low` left behind would win under YAML's last-key-wins and + # silently pin the opposite level. + kept = [] + nested_content = False + for line in lines: + if not line.strip() or line.lstrip().startswith("#"): + kept.append(line) + continue + if line[:1].isspace() and nested_content: + kept.append(line) + continue + if not line[:1].isspace(): + # A block scalar or nested mapping belongs to the preceding field. + # Its indented text is not a top-level dispatch setting. + _, separator, value = line.partition(":") + value = value.split(" #", 1)[0].strip() + nested_content = bool(separator) and ( + not value or value.startswith(("|", ">")) + ) + if not _EFFORT_KEY.match(line): + kept.append(line) + # Keep it adjacent to `model:`, the other generated dispatch control. + anchor = next( + (i for i, line in enumerate(kept) if line.startswith("model:")), len(kept) - 1 + ) + kept.insert(anchor + 1, f"effort: {level}") + return "---\n" + "\n".join(kept) + text[end:] + def sync(root: Path, *, check: bool) -> bool: allowlist = json.loads((root / ".ai/allowlist.json").read_text()) @@ -30,6 +90,16 @@ def sync(root: Path, *, check: bool) -> bool: raise ValueError( "allowlist models must contain all canonical roles with nonempty, trimmed model strings" ) + effort = allowlist["effort"] + if not isinstance(effort, dict) or set(effort) != set(CANONICAL_ROLES): + raise ValueError("allowlist effort must contain all canonical roles") + for role, level in effort.items(): + validate_effort(role, level) + if ( + runtime_for_model(models[role]) == "claude" + and level not in CLAUDE_EFFORT_LEVELS + ): + raise ValueError(f"Unsupported Claude effort for {role}: {level!r}") fallback = allowlist["codex_fallback_model"] if ( not isinstance(fallback, str) @@ -53,6 +123,13 @@ def sync(root: Path, *, check: bool) -> bool: ) + "}" + "\n" + + "DEFAULT_EFFORT: dict[str, str] = " + + "{\n" + + "".join( + f" {json.dumps(role)}: {json.dumps(effort[role])},\n" for role in models + ) + + "}" + + "\n" + "CODEX_NATIVE_FALLBACK_MODEL = " + json.dumps(fallback) + "\n" @@ -67,6 +144,16 @@ def sync(root: Path, *, check: bool) -> bool: for role, model in models.items(): config["agent"][role]["model"] = "opencode/" + model updates[opencode] = json.dumps(config, indent=2) + "\n" + # Claude subagent frontmatter: the only effort control on the + # Claude-led -> Claude path, since native Task() reads the agent file. + # Documented but unverified on Claude Code 2.1.280 — an invalid value there + # raises no warning and low-vs-max showed no token separation, unlike the + # `claude --effort` flag. Emitted anyway: it is free and forward-compatible. + for filename, role in AGENT_FILE_ROLES.items(): + agent_file = root / ".claude/agents" / filename + if not agent_file.exists(): + continue + updates[agent_file] = _with_effort(agent_file.read_text(), effort[role]) clean = True for path, content in updates.items(): if path.read_text() == content: diff --git a/scripts/quest_validate-quest-state.sh b/scripts/quest_validate-quest-state.sh index 2265f1b3..76344f60 100755 --- a/scripts/quest_validate-quest-state.sh +++ b/scripts/quest_validate-quest-state.sh @@ -226,6 +226,21 @@ validate_orchestration_json() { return fi + # Per-role effort map. Absent on quests written before it existed (those + # resolve through the scalar above); when present every value must be valid. + if ! jq -e 'if has("effort") then + (.effort | type == "object") and + (.effort | to_entries | all( + (.key as $role | ["planner", "plan-reviewer-a", "plan-reviewer-b", + "arbiter", "builder", "code-reviewer-a", "code-reviewer-b", + "review-arbiter", "fixer"] | index($role) != null) and + (.value as $level | ["low", "medium", "high", "xhigh", "max", "ultra"] + | index($level) != null))) + else true end' "$orch_file" >/dev/null 2>&1; then + fail "orchestration.json effort is invalid" + return + fi + # Required roles depend on quest_mode. # Keep this list in sync with workflow.md dispatch sites # (planner, plan-reviewer-a, plan-reviewer-b, arbiter, builder, diff --git a/tests/test-quest-orchestration.sh b/tests/test-quest-orchestration.sh index 3c7e6828..b3a1417d 100755 --- a/tests/test-quest-orchestration.sh +++ b/tests/test-quest-orchestration.sh @@ -216,10 +216,10 @@ from quest_runtime.orchestration import build_default_models result = build_default_models({"planner": "gpt-5.5", "builder": "claude"}) assert result["planner"] == "gpt-5.5", result assert result["builder"] == "claude", result -assert result["plan-reviewer-a"] == "claude-opus-5", result +assert result["plan-reviewer-a"] == "claude-opus-5-5", result assert result["plan-reviewer-b"] == "gpt-6-astra", result -assert result["arbiter"] == "claude-opus-5", result -assert result["code-reviewer-a"] == "claude-opus-5", result +assert result["arbiter"] == "claude-opus-5-5", result +assert result["code-reviewer-a"] == "claude-opus-5-5", result assert result["code-reviewer-b"] == "gpt-6-astra", result assert result["fixer"] == "gpt-6-astra", result PY @@ -234,13 +234,13 @@ from quest_runtime.orchestration import CODEX_NATIVE_FALLBACK_MODEL, DEFAULT_MOD expected = { "planner": "gpt-6-astra", - "plan-reviewer-a": "claude-opus-5", + "plan-reviewer-a": "claude-opus-5-5", "plan-reviewer-b": "gpt-6-astra", - "arbiter": "claude-opus-5", - "builder": "gpt-6-astra", - "code-reviewer-a": "claude-opus-5", + "arbiter": "claude-opus-5-5", + "builder": "gpt-5.6-sol", + "code-reviewer-a": "claude-opus-5-5", "code-reviewer-b": "gpt-6-astra", - "review-arbiter": "claude-opus-5", + "review-arbiter": "claude-opus-5-5", "fixer": "gpt-6-astra", } allowlist = json.loads(Path(".ai/allowlist.json").read_text()) @@ -791,7 +791,7 @@ from quest_runtime.orchestration import migrate_from_snapshot written = migrate_from_snapshot(Path(sys.argv[1])) assert written is True, "expected legacy role backfill write" orch = json.loads((Path(sys.argv[1]) / "orchestration.json").read_text()) -assert orch["models"]["review-arbiter"] == "claude-opus-5", orch["models"] +assert orch["models"]["review-arbiter"] == "claude-opus-5-5", orch["models"] PY rm -f "$tmpdir/orchestration.json" @@ -854,7 +854,7 @@ quest_dir = Path(sys.argv[1]) written = migrate_from_snapshot(quest_dir) assert written is True, "expected legacy backfill write" orch = json.loads((quest_dir / "orchestration.json").read_text()) -assert orch["models"]["review-arbiter"] == "claude-opus-5", orch["models"] +assert orch["models"]["review-arbiter"] == "claude-opus-5-5", orch["models"] assert orch["source"] == "overridden", orch assert orch["overridden_roles"] == ["planner", "builder"], orch PY diff --git a/tests/unit/test_canonical_role_lockstep.py b/tests/unit/test_canonical_role_lockstep.py index 1753b2b1..ed2ecc89 100644 --- a/tests/unit/test_canonical_role_lockstep.py +++ b/tests/unit/test_canonical_role_lockstep.py @@ -268,6 +268,7 @@ def test_generator_rejects_invalid_policy_before_writing(tmp_path: Path) -> None policies.append( {**original, "models": {**original["models"], "builder": " codex-fake-model "}} ) + policies.append({**original, "effort": {**original["effort"], "arbiter": "ultra"}}) for policy in policies: allowlist_path.write_text(json.dumps(policy)) with pytest.raises(ValueError): diff --git a/tests/unit/test_codex_runner.py b/tests/unit/test_codex_runner.py index 557765a6..ba051cf6 100644 --- a/tests/unit/test_codex_runner.py +++ b/tests/unit/test_codex_runner.py @@ -911,3 +911,38 @@ def test_reviewer_findings_repair_preserves_prose_and_requires_new_outputs( ) assert result["result_kind"] == "artifact_missing" assert prose.read_bytes() == original + + +@pytest.mark.parametrize("effort", [{}, {"arbiter": "low"}, {"builder": "max"}, None]) +def test_role_mode_resolves_partial_effort_and_rejects_null(cli, monkeypatch, effort): + quest, _, _ = role_fixture(cli, monkeypatch) + path = quest / "orchestration.json" + saved = json.loads(path.read_text()) + saved["effort"] = effort + path.write_text(json.dumps(saved)) + result = run_role(cli, quest) + if effort is None: + assert result["result_kind"] == "precondition_failed" + assert not (cli / "CAPTURE").exists() + else: + assert result["result_kind"] == "complete" + expected = effort.get("builder", "high") + assert result["requested_effort"] == expected + assert ( + f'model_reasoning_effort="{expected}"' + in json.loads((cli / "CAPTURE").read_text())["args"] + ) + + +@pytest.mark.parametrize("model", [None, "", " ", "opencode/claude-opus-5-5"]) +def test_role_mode_rejects_missing_or_claude_model_before_dispatch( + cli, monkeypatch, model +): + quest, _, _ = role_fixture(cli, monkeypatch) + path = quest / "orchestration.json" + saved = json.loads(path.read_text()) + saved["models"]["builder"] = model + path.write_text(json.dumps(saved)) + result = run_role(cli, quest) + assert result["result_kind"] == "precondition_failed" + assert not (cli / "CAPTURE").exists() diff --git a/tests/unit/test_quest_claude_bridge.py b/tests/unit/test_quest_claude_bridge.py index 56bf846f..6d36a601 100644 --- a/tests/unit/test_quest_claude_bridge.py +++ b/tests/unit/test_quest_claude_bridge.py @@ -54,7 +54,8 @@ def fake_run_claude(*_args, **_kwargs): assert "Prompt is empty" in capsys.readouterr().err -def test_run_claude_places_prompt_unchanged_in_argv(monkeypatch) -> None: +@pytest.mark.parametrize("effort", [None, "medium"]) +def test_run_claude_places_prompt_and_effort_in_argv(monkeypatch, effort) -> None: captured: list[str] = [] def fake_run(cmd, **_kwargs): @@ -65,6 +66,7 @@ def fake_run(cmd, **_kwargs): result = bridge.run_claude( prompt=EXACT_PROMPT, + effort=effort, output_format="json", timeout=10.0, model="", @@ -79,6 +81,10 @@ def fake_run(cmd, **_kwargs): assert result["status"] == "ok" assert captured[2] == EXACT_PROMPT + if effort: + assert captured[captured.index("--effort") + 1] == effort + else: + assert "--effort" not in captured @pytest.mark.parametrize( diff --git a/tests/unit/test_quest_effort.py b/tests/unit/test_quest_effort.py new file mode 100644 index 00000000..7d069870 --- /dev/null +++ b/tests/unit/test_quest_effort.py @@ -0,0 +1,111 @@ +"""Per-role reasoning effort across the four dispatch paths. + +Effort reaches a role by a different mechanism per path: subagent frontmatter +(Claude-led -> Claude), `claude --effort` via the runner (Codex-led -> Claude), +and `model_reasoning_effort` (both Codex paths). These cover the shared +resolution rule and the two Claude-side plumbing points. +""" + +import json +from pathlib import Path + +import pytest + +from quest_runtime import claude_runner +from quest_runtime.orchestration import ( + CANONICAL_ROLES, + DEFAULT_EFFORT, + effort_for_role, + validate_effort_map, + write_default_from_allowlist, +) + +BRIDGE_ARGS = { + "cwd": ".", + "bridge_script": "scripts/quest_claude_bridge.py", + "prompt_file": "p.txt", + "model": "claude-opus-5-5", + "timeout": 60.0, + "permission_mode": "bypassPermissions", +} + + +def test_bridge_cmd_passes_effort_flag(): + cmd = claude_runner.build_bridge_cmd(**BRIDGE_ARGS, effort="medium") + assert cmd[cmd.index("--effort") + 1] == "medium" + + +def test_bg_cmd_passes_effort_flag(): + cmd = claude_runner.build_bg_cmd( + cwd=".", + bg_runner_script="scripts/quest_claude_bg_run.py", + prompt_file="p.txt", + name="quest-x-arbiter-i1", + model="claude-opus-5-5", + timeout=60.0, + permission_mode="bypassPermissions", + handoff_file="h.json", + wait_for=[], + effort="high", + ) + assert cmd[cmd.index("--effort") + 1] == "high" + + +def test_effort_flag_omitted_when_unset(): + # No pinned effort must leave the CLI default untouched, not guess a level. + assert "--effort" not in claude_runner.build_bridge_cmd(**BRIDGE_ARGS) + + +def test_claude_runtime_rejects_codex_only_ultra(): + # `ultra` is valid in the shared role map but unreachable on the Claude CLI, + # so it must fail loudly rather than silently run at the default. + with pytest.raises(ValueError, match="ultra"): + claude_runner.build_bridge_cmd(**BRIDGE_ARGS, effort="ultra") + + +def test_effort_map_wins_over_legacy_scalar(): + saved = {"effort": {"arbiter": "high"}, "codex_reasoning_effort": "low"} + assert effort_for_role(saved, "arbiter") == "high" + + +def test_legacy_quests_resolve_through_the_scalar(): + # A quest written before the per-role map keeps its saved effort, so resume + # never silently re-tiers work that is already in flight. The scalar is + # Codex-scoped, so resolving it needs the role's model — see + # test_quest_effort_regressions for the full contract. + legacy = { + "models": {"builder": "gpt-6-astra"}, + "codex_reasoning_effort": "medium", + } + assert effort_for_role(legacy, "builder") == "medium" + assert effort_for_role({}, "builder") is None + + +def test_invalid_level_is_rejected(): + with pytest.raises(ValueError, match="builder"): + validate_effort_map({"builder": "turbo"}) + with pytest.raises(ValueError, match="unknown role"): + validate_effort_map({"nobody": "high"}) + + +def test_orchestration_json_persists_a_complete_effort_map(tmp_path): + path = tmp_path / "orchestration.json" + write_default_from_allowlist( + path, + {role: "claude-opus-5-5" for role in CANONICAL_ROLES}, + effort={"builder": "max"}, + ) + saved = json.loads(path.read_text()) + assert set(saved["effort"]) == set(CANONICAL_ROLES) + assert saved["effort"]["builder"] == "max" + # Roles the caller did not pin fall back to the shipped default. + assert saved["effort"]["fixer"] == DEFAULT_EFFORT["fixer"] + + +def test_allowlist_and_generated_defaults_agree(): + allowlist = json.loads( + (Path(__file__).resolve().parents[2] / ".ai/allowlist.json").read_text( + encoding="utf-8" + ) + ) + assert allowlist["effort"] == DEFAULT_EFFORT diff --git a/tests/unit/test_quest_effort_regressions.py b/tests/unit/test_quest_effort_regressions.py new file mode 100644 index 00000000..a8c6765e --- /dev/null +++ b/tests/unit/test_quest_effort_regressions.py @@ -0,0 +1,391 @@ +"""Regressions for defects found reviewing the per-role effort change. + +Each test pins a case where effort was silently wrong rather than loudly +wrong, which is the failure mode that makes an effort setting untrustworthy. +""" + +import json + +import pytest + +from quest_runtime import claude_runner +from quest_runtime.orchestration import ( + CANONICAL_ROLES, + effort_for_role, + migrate_from_snapshot, + write_orchestration_json, +) + + +def _snapshot(tmp_path, **extra): + quest = tmp_path / "qid" + (quest / "logs").mkdir(parents=True) + (quest / "logs" / "allowlist_snapshot.json").write_text( + json.dumps({"models": {r: "gpt-6-astra" for r in CANONICAL_ROLES}, **extra}) + ) + return quest + + +@pytest.mark.parametrize("existing", [False, True]) +def test_snapshot_migration_keeps_legacy_scalar(tmp_path, existing): + # Filling DEFAULT_EFFORT here would override the scalar the quest was + # started with, re-tiering work already in flight. + quest = _snapshot(tmp_path, codex_reasoning_effort="high") + if existing: + (quest / "orchestration.json").write_text( + (quest / "logs/allowlist_snapshot.json").read_text() + ) + migrate_from_snapshot(quest) + saved = json.loads((quest / "orchestration.json").read_text()) + assert "effort" not in saved + assert effort_for_role(saved, "builder") == "high" + + +def test_snapshot_migration_without_any_effort_stays_unpinned(tmp_path): + quest = _snapshot(tmp_path) + migrate_from_snapshot(quest) + saved = json.loads((quest / "orchestration.json").read_text()) + assert "effort" not in saved + assert effort_for_role(saved, "builder") is None + + +def test_legacy_scalar_is_codex_scoped(): + # The key is named codex_*; before the map existed the Claude runner never + # read it, so Claude roles must stay unpinned. + saved = { + "models": {"builder": "gpt-6-astra", "arbiter": "claude-opus-5-5"}, + "codex_reasoning_effort": "high", + } + assert effort_for_role(saved, "builder") == "high" + assert effort_for_role(saved, "arbiter") is None + + +def test_legacy_codex_only_ultra_does_not_break_claude_roles(): + # `ultra` is valid for Codex. Leaking it onto a Claude role would turn a + # previously-working legacy quest into a hard dispatch failure. + saved = { + "models": {"arbiter": "claude-opus-5-5"}, + "codex_reasoning_effort": "ultra", + } + level = effort_for_role(saved, "arbiter") + assert level is None + assert claude_runner.normalize_claude_cli_effort(level) is None + + +@pytest.mark.parametrize("bad", [{"builder": 5}, {"builder": ""}, [], "high"]) +def test_malformed_effort_map_is_loud(bad): + # A malformed map is a config error. Falling back to the legacy scalar + # would hide it behind a plausible-looking level. + with pytest.raises(ValueError): + effort_for_role({"effort": bad, "codex_reasoning_effort": "high"}, "builder") + + +def test_unknown_role_key_is_rejected(): + with pytest.raises(ValueError, match="unknown role"): + effort_for_role({"effort": {"buidler": "high"}}, "builder") + + +def test_new_quest_writer_still_persists_a_full_map(tmp_path): + # The migration fix must not stop new quests from getting defaults. + from quest_runtime.orchestration import write_default_from_allowlist + + path = tmp_path / "orchestration.json" + write_default_from_allowlist( + path, {r: "gpt-6-astra" for r in CANONICAL_ROLES}, effort=None + ) + saved = json.loads(path.read_text()) + assert set(saved["effort"]) == set(CANONICAL_ROLES) + + +def test_write_orchestration_json_omits_effort_when_unset(tmp_path): + path = tmp_path / "orchestration.json" + write_orchestration_json( + path, + models={r: "gpt-6-astra" for r in CANONICAL_ROLES}, + source="default", + overridden_roles=[], + ) + assert "effort" not in json.loads(path.read_text()) + + +@pytest.mark.parametrize("preparation_failure", [False, True]) +def test_permission_retry_preserves_effort(tmp_path, monkeypatch, preparation_failure): + # A retry that silently drops to the default effort makes the pinned value + # a lie on exactly the runs that were already going badly. Drives the real + # Tier B permission retry and inspects both dispatched argvs. + prompt_file = tmp_path / "prompt.txt" + prompt_file.write_text("prompt", encoding="utf-8") + (tmp_path / "state.json").write_text( + json.dumps({"phase": "plan", "plan_iteration": 1}), encoding="utf-8" + ) + handoff_file = tmp_path / "handoff.json" + artifact = tmp_path / "artifact.md" + + class FakeProcess: + returncode = 1 + + def __init__(self, stderr="", on_communicate=None): + self._stderr = stderr + self._on_communicate = on_communicate + + def communicate(self, timeout=None): + if self._on_communicate: + self._on_communicate() + return "", self._stderr + + def poll(self): + return self.returncode + + def terminate(self): + return None + + def kill(self): + return None + + argvs: list[list[str]] = [] + + def succeed(): + artifact.write_text("ok", encoding="utf-8") + handoff_file.write_text( + '{"status":"complete","artifacts":["artifact.md"],' + '"next":null,"summary":"ok"}', + encoding="utf-8", + ) + + def fake_popen(cmd, *args, **kwargs): + argvs.append(list(cmd)) + if len(argvs) == 1 and not preparation_failure: + return FakeProcess(stderr="Error: Permission denied writing artifact") + return FakeProcess(on_communicate=succeed) + + if preparation_failure: + + def deny_preparation(*args, **kwargs): + raise PermissionError("Permission denied writing artifact") + + monkeypatch.setattr(claude_runner, "prepare_artifact_files", deny_preparation) + monkeypatch.setattr(claude_runner.subprocess, "Popen", fake_popen) + + claude_runner.run_claude_role( + cwd=tmp_path, + quest_dir=tmp_path, + phase="plan", + agent="planner", + iteration=1, + prompt_file=prompt_file, + handoff_file=handoff_file, + bridge_script=tmp_path / "bridge.py", + model="claude-opus-5-5", + effort="high", + timeout=1.0, + permission_mode="bypassPermissions", + artifact_paths=[artifact], + poll_interval=0.01, + exit_grace_seconds=0.01, + ) + + assert len(argvs) == (1 if preparation_failure else 2) + for attempt, argv in enumerate(argvs, start=1): + assert "--effort" in argv, f"attempt {attempt} dropped --effort" + assert argv[argv.index("--effort") + 1] == "high" + + +def test_generator_replaces_quoted_effort_key(): + from quest_sync_model_defaults import _with_effort + + out = _with_effort( + '---\nname: x\nmodel: inherit\n"effort": low\n---\nbody\n', "high" + ) + assert '"effort"' not in out + assert "effort: high" in out + + +@pytest.mark.parametrize("effort", [{}, {"arbiter": "low"}]) +@pytest.mark.parametrize("existing", [False, True]) +def test_migration_preserves_partial_effort(tmp_path, effort, existing): + quest = _snapshot(tmp_path, effort=effort, codex_reasoning_effort="high") + if existing: + (quest / "orchestration.json").write_text( + (quest / "logs/allowlist_snapshot.json").read_text() + ) + migrate_from_snapshot(quest) + saved = json.loads((quest / "orchestration.json").read_text()) + assert saved["effort"] == effort + assert effort_for_role(saved, "builder") == "high" + + +@pytest.mark.parametrize("existing", [False, True]) +def test_migration_rejects_null_effort(tmp_path, existing): + quest = _snapshot(tmp_path, effort=None) + if existing: + (quest / "orchestration.json").write_text( + (quest / "logs/allowlist_snapshot.json").read_text() + ) + with pytest.raises(ValueError, match="effort"): + migrate_from_snapshot(quest) + + +def test_null_effort_is_not_absent(): + with pytest.raises(ValueError, match="effort"): + effort_for_role({"effort": None}, "builder") + + +@pytest.mark.parametrize("contents", ["{", "[]", "null"]) +def test_claude_loader_rejects_corrupt_saved_config(tmp_path, contents): + from quest_claude_runner import _resolve_effort + + (tmp_path / "orchestration.json").write_text(contents) + with pytest.raises(ValueError): + _resolve_effort(str(tmp_path), "arbiter", None) + + +def test_existing_solo_migration_preserves_null_unused_models(tmp_path): + from quest_runtime.orchestration import SOLO_UNUSED_ROLES + + models = { + r: None if r in SOLO_UNUSED_ROLES else "gpt-6-astra" for r in CANONICAL_ROLES + } + quest = _snapshot(tmp_path, models=models, effort={"builder": "high"}) + (quest / "orchestration.json").write_text( + (quest / "logs/allowlist_snapshot.json").read_text() + ) + (quest / "state.json").write_text('{"quest_mode":"solo"}') + migrate_from_snapshot(quest) + saved = json.loads((quest / "orchestration.json").read_text()) + assert saved["models"] == models + assert saved["effort"] == {"builder": "high"} + + +@pytest.mark.parametrize("model", [None, "", " ", "opencode/claude-opus-5-5"]) +def test_legacy_scalar_does_not_leak_to_missing_or_claude_models(model): + assert ( + effort_for_role( + {"models": {"arbiter": model}, "codex_reasoning_effort": "ultra"}, "arbiter" + ) + is None + ) + + +@pytest.mark.parametrize("prefix", ["\t", " ", ""]) +def test_generator_strips_indented_quoted_key(prefix): + from quest_sync_model_defaults import _with_effort + + out = _with_effort( + "---\nmodel: inherit\n" + prefix + '"effort": low\n---\nbody\n', "high" + ) + assert '"effort"' not in out + assert out.count("effort:") == 1 + + +def test_generator_crlf_file_read_and_invalid_opening(tmp_path): + from quest_sync_model_defaults import _with_effort + + path = tmp_path / "agent.md" + path.write_bytes(b'---\r\nmodel: inherit\r\n"effort": low\r\n---\r\nbody\r\n') + assert "effort: high" in _with_effort(path.read_text(), "high") + with pytest.raises(ValueError): + _with_effort(path.read_bytes().decode(), "high") + with pytest.raises(ValueError): + _with_effort('name: x\n---\n"effort": low\n---\n', "high") + + +def test_invalid_claude_effort_does_not_truncate_artifacts(tmp_path): + artifact = tmp_path / "plan.md" + artifact.write_text("preserve") + (tmp_path / "state.json").write_text('{"phase":"plan","plan_iteration":1}') + result = claude_runner.run_claude_role( + cwd=tmp_path, + quest_dir=tmp_path, + phase="plan", + agent="planner", + iteration=1, + prompt_file=tmp_path / "prompt", + handoff_file=tmp_path / "handoff", + bridge_script=tmp_path / "bridge", + model="claude", + effort="ultra", + timeout=1, + permission_mode="bypassPermissions", + artifact_paths=[artifact], + ) + # Validating early must not cost the JSON envelope: orchestrators parse + # this result, and a traceback reaches them as an unreadable failure. + assert result.result_kind == "invocation_error" + assert "ultra" in result.stderr + assert artifact.read_text() == "preserve" + + +@pytest.mark.parametrize( + "level,frontmatter,accepted", + [ + ("high", "effort: high", True), + ("high", "effort: medium", False), + ("ultra", "effort: ultra", False), + # An unpinned role must NOT block: the generated frontmatter always + # names a level, so blocking here would wall off every legacy quest. + (None, "effort: medium", True), + (None, "name: arbiter", True), + # A valid nested subagent key must not be mistaken for ambiguity. + ("high", "effort: high\nhooks:\n PreToolUse:\n - matcher: Bash", True), + ("high", 'effort: high\n"effort": low', False), + ("high", "effort: high\n\teffort: low", False), + ], +) +def test_native_claude_effort_guard(level, frontmatter, accepted): + from quest_runtime.orchestration import validate_native_claude_effort + + saved = { + "models": {"arbiter": "opencode/claude-opus-5-5"}, + "codex_reasoning_effort": "ultra", + } + if level is not None: + saved["effort"] = {"arbiter": level} + text = "---\n" + frontmatter + "\n---\nbody" + if accepted: + validate_native_claude_effort(saved, "arbiter", text) + else: + with pytest.raises(ValueError): + validate_native_claude_effort(saved, "arbiter", text) + + +def test_claude_loader_rejects_unreadable_saved_config(tmp_path): + from quest_claude_runner import _resolve_effort + + (tmp_path / "orchestration.json").mkdir() + with pytest.raises(ValueError, match="Cannot read"): + _resolve_effort(str(tmp_path), "arbiter", None) + + +@pytest.mark.parametrize("field", ["description: |", "description: >-", "metadata:"]) +def test_generator_preserves_nested_effort_content(field): + from quest_sync_model_defaults import _with_effort + + nested = field + "\n effort: low\n other: text" + text = "---\nmodel: inherit\n" + nested + "\neffort: low\n---\nbody\n" + result = _with_effort(text, "medium") + assert nested in result + assert "\neffort: medium\n" in result + + +@pytest.mark.parametrize("models", [123, "invalid", ["arbiter"], []]) +def test_claude_loader_rejects_non_object_models(tmp_path, models): + from quest_claude_runner import _resolve_effort + + (tmp_path / "orchestration.json").write_text( + json.dumps({"models": models, "codex_reasoning_effort": "medium"}) + ) + with pytest.raises(ValueError, match="models"): + _resolve_effort(str(tmp_path), "arbiter", None) + + +@pytest.mark.parametrize("model", [123, ["gpt-6-astra"], True, " "]) +def test_migration_rejects_invalid_active_model_without_writing(tmp_path, model): + models = {r: "gpt-6-astra" for r in CANONICAL_ROLES} + models["builder"] = model + quest = _snapshot(tmp_path, models=models) + path = quest / "orchestration.json" + path.write_text((quest / "logs/allowlist_snapshot.json").read_text()) + before = path.read_bytes() + with pytest.raises(ValueError, match="model"): + migrate_from_snapshot(quest) + assert path.read_bytes() == before