From ae33b999ae9b1e5f5f5108e2b7c62631d24f59dc Mon Sep 17 00:00:00 2001 From: dch0202 Date: Sat, 8 Aug 2026 00:14:40 +0900 Subject: [PATCH 1/2] =?UTF-8?q?feat(orchestrate):=20per-role=20model=20sel?= =?UTF-8?q?ection=20=E2=80=94=20cheap=20worker,=20strong=20planner/auditor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dev-loop launched every worker as a bare `claude --permission-mode `, so the implementer, the planner, and the test-quality auditor all ran on whatever single model the user had configured. That couples three jobs with opposite cost/quality profiles: the implementer is the token-heavy one and the plan and the audit are where a weak model does the most damage. Split them along the seams that already existed: - Worker model is now settable via `--model` / `DEV_LOOP_WORKER_MODEL` in orca-worker-start.sh, orca-spawn.sh, and launch-session.sh. Unset omits the flag entirely, so existing deployments are byte-identical on upgrade. The value is whitelisted (alphanumerics, . _ - and the [1m] suffix) and single-quoted, matching how `--perm` and `--agent` are already guarded. Inert on the non-escalation path, where Orca composes the agent command itself — documented in the script header alongside the same caveat for `--permission-mode`. - test-quality-auditor is pinned (`model: opus`) instead of `inherit`. As the self-grading guard it must not follow the worker down: an inheriting auditor grades a cheap worker at the worker's own tier, so writer and grader share blind spots — the exact failure the agent exists to prevent. - Planning moves to the coordinator. wiki-plan now runs in the coordinator session (Phase 3 step 2a) and the worker ADOPTS the resulting plan via loop-implement's existing "a plan already exists" entry mode, reporting a gap rather than re-planning. The phase sequence is untouched: the worker still signals plan_ready, so watch-status, the ready-set scheduler, and the loop-gate allowlist all see what they saw before. A plan is where an unmade decision becomes the implementer's guess, so it must come from the strongest model in the run rather than the tier that happens to be executing. Tests: 436 pass, 0 fail. New coverage on all three launch scripts for the normal, error (shell-metacharacter injection rejected, nothing created), and boundary (unset adds no flag; `opus[1m]` accepted) cases. The Orca prompt-set checksum in send-prompt.bats is bumped in this commit, as that test requires. Co-Authored-By: Claude Opus 5 (1M context) --- agents/test-quality-auditor.md | 9 +++- skills/loop-implement/SKILL.md | 5 +- skills/orchestrate/SKILL.md | 16 ++++++ skills/orchestrate/scripts/launch-session.sh | 13 +++++ skills/orchestrate/scripts/orca-spawn.sh | 16 +++++- .../orchestrate/scripts/orca-worker-start.sh | 32 ++++++++++-- skills/orchestrate/templates/brief.md | 9 +++- .../orchestrate/templates/session-prompt.md | 18 ++++--- tests/launch-session.bats | 36 +++++++++++++ tests/orca-spawn.bats | 25 ++++++++++ tests/orca-worker-start.bats | 50 +++++++++++++++++++ tests/send-prompt.bats | 5 +- 12 files changed, 217 insertions(+), 17 deletions(-) diff --git a/agents/test-quality-auditor.md b/agents/test-quality-auditor.md index b442859..9496873 100644 --- a/agents/test-quality-auditor.md +++ b/agents/test-quality-auditor.md @@ -2,13 +2,20 @@ name: test-quality-auditor description: Read-only verifier that audits one task's diff and tests for quality. Invoked between self-review and done so the session that wrote the code does not grade its own tests (self-grading guard). Returns a fixed VERDICT and REASONS. tools: Read, Grep, Glob, Bash -model: inherit +model: opus --- You are an independent test-quality auditor for loop-orchestrator. You DO NOT modify code or tests — you are read-only. Your only job is to judge whether the tests genuinely verify the change. +> This agent's model is **pinned** rather than `inherit`. It is the self-grading +> guard, so it must not follow the worker down: when a worker is pinned to a +> cheaper tier (`DEV_LOOP_WORKER_MODEL`, see `skills/orchestrate/scripts/`), an +> inheriting auditor would grade that worker at the worker's own tier — writer +> and grader sharing blind spots is the exact failure this agent exists to +> prevent. Raise the pin, never lower it. + Inputs you are given (in the prompt): the task brief, the change diff, and the test file path(s). If any are missing, ask for them rather than guessing. diff --git a/skills/loop-implement/SKILL.md b/skills/loop-implement/SKILL.md index 777b317..49f854a 100644 --- a/skills/loop-implement/SKILL.md +++ b/skills/loop-implement/SKILL.md @@ -17,8 +17,9 @@ tasks in the plan's order, loading exactly the wiki pages each task names, and citing them — so every change traces back to a verified wiki page. It works the same whether it runs standalone (you produce the plan here in step 2) -or as an orchestrated worker (the orchestrator hands you a task brief and you plan -+ implement it here). Either path, one loop. +or as an orchestrated worker (the orchestrator hands you a task brief *and* the +plan it already produced, and you adopt and implement it here). Either path, one +loop. ## When to use - Use: logic changes, new features, bug fixes, behavior-changing refactors. diff --git a/skills/orchestrate/SKILL.md b/skills/orchestrate/SKILL.md index 35d5109..5defc09 100644 --- a/skills/orchestrate/SKILL.md +++ b/skills/orchestrate/SKILL.md @@ -336,6 +336,22 @@ reasoning-effort flags) that `worker-start` cannot express. tools (the plan step is fixed to `wiki-plan`, not a configurable role), and for a UI-facing task fill `` with the `design` role's pulled spec (Phase 2) — then + + **2a. Plan it yourself, here, before launching.** Invoke the bundled `wiki-plan` + skill for this task and write the result to `plans/.md`. Planning runs in + THIS coordinator session on purpose: a worker can be pinned to a cheaper tier + (`DEV_LOOP_WORKER_MODEL`), and a plan is where an unmade decision becomes the + implementer's guess — so the plan must come from the strongest model in the run, + not from whatever tier is executing. Every design decision must be made and + grounded in a `wiki/` page (record the decision->page map); leave nothing "as + appropriate". The worker then ADOPTS this plan (session-prompt §1 / O1) instead + of authoring one, and still signals `plan_ready` — so the phase sequence, the + `plan_ready` watch, and the ready-set scheduler are all unchanged. + + A worker that reports the plan is contradictory or under-decided is telling you + the planning pass was wrong: fix `plans/.md` here and re-send §1. Do not + let the worker re-plan — that silently moves planning back onto the worker tier, + which is the thing this step exists to prevent. Then `LO_STATUS_DIR= LO_TASK_ID= scripts/launch-session.sh lo- bypassPermissions ""` (plan prompt = templates/session-prompt.md §1 — the tmux set — with the diff --git a/skills/orchestrate/scripts/launch-session.sh b/skills/orchestrate/scripts/launch-session.sh index 639271a..bfb419c 100755 --- a/skills/orchestrate/scripts/launch-session.sh +++ b/skills/orchestrate/scripts/launch-session.sh @@ -36,6 +36,8 @@ # LO_DRY_RUN print the resolved session name and exit # LO_TMUX tmux binary (default: the one on PATH) # LO_CLAUDE claude binary (default: the search below) +# DEV_LOOP_WORKER_MODEL model the WORKER runs (e.g. claude-sonnet-5). Unset = +# omit --model, so the worker inherits the configured model. # LO_READY_TIMEOUT / LO_READY_INTERVAL REPL-ready budget (default 60 / 2); # attempts = floor(timeout/interval), minimum 1 # LO_READY_EXTRA / LO_TRUST_EXTRA extra screen-match substrings @@ -73,6 +75,16 @@ case "$perm" in *) echo "launch-session: invalid permission mode '$perm'" >&2; exit 2 ;; esac +# Same guard for the worker model: ids/aliases are alphanumerics plus . _ - and +# the [1m] context suffix (e.g. opus[1m]). Unset = omit --model entirely, so the +# worker inherits the user's configured model. +model="${DEV_LOOP_WORKER_MODEL:-}" +if [ -n "$model" ]; then + case "$model" in + *[!A-Za-z0-9._\[\]-]*) echo "launch-session: invalid model '$model'" >&2; exit 2 ;; + esac +fi + # Resolve-only mode: print the effective session name and exit before touching # tmux/claude. Lets the orchestrator (and tests) learn the exact name. if [ -n "${LO_DRY_RUN:-}" ]; then echo "session=$session"; exit 0; fi @@ -125,6 +137,7 @@ if [ -n "$esc" ]; then launchcmd="$launchcmd && export GROUNDWORK_ESCALATION_DIR='$esc' && export GROUNDWORK_TASK_ID='$session'" fi launchcmd="$launchcmd && \"$CLAUDE\" --permission-mode $perm" +[ -n "$model" ] && launchcmd="$launchcmd --model '$model'" "$TMUX_BIN" send-keys -t "$session" "$launchcmd" Enter # Pass trust screen + permission warning, wait for REPL ready (~60s). diff --git a/skills/orchestrate/scripts/orca-spawn.sh b/skills/orchestrate/scripts/orca-spawn.sh index 87ad327..e938d12 100755 --- a/skills/orchestrate/scripts/orca-spawn.sh +++ b/skills/orchestrate/scripts/orca-spawn.sh @@ -13,6 +13,9 @@ # ORCA_BIN orca executable (default: orca) # GROUNDWORK_ESCALATION_DIR exported into the worker so an `ask` escalates # GROUNDWORK_TASK_ID worker task label +# DEV_LOOP_WORKER_MODEL model the WORKER runs (e.g. claude-sonnet-5). +# Unset = omit --model, so the worker inherits the +# user's configured model (unchanged behavior). # LO_READY_TIMEOUT seconds to wait for TUI readiness (default 60) # ORCA_SPAWN_DRYRUN=1 print the orca commands instead of running them # ORCA_SPAWN_CREATE_JSON canned `terminal create --json` (tests) @@ -32,6 +35,15 @@ case "$perm" in *) echo "orca-spawn: invalid permission mode '$perm'" >&2; exit 2 ;; esac +# Same for the model: ids/aliases are alphanumerics plus . _ - and the [1m] +# context suffix. Reject anything else rather than sanitizing it. +model="${DEV_LOOP_WORKER_MODEL:-}" +if [ -n "$model" ]; then + case "$model" in + *[!A-Za-z0-9._\[\]-]*) echo "orca-spawn: invalid model '$model'" >&2; exit 2 ;; + esac +fi + rt="${LO_READY_TIMEOUT:-60}"; [ "$rt" -ge 1 ] 2>/dev/null || rt=60 timeout_ms=$(( rt * 1000 )) @@ -44,7 +56,9 @@ env_prefix="" if [ -n "${GROUNDWORK_ESCALATION_DIR:-}" ]; then env_prefix="export GROUNDWORK_ESCALATION_DIR='$(esc_sq "$GROUNDWORK_ESCALATION_DIR")' && export GROUNDWORK_TASK_ID='$(esc_sq "${GROUNDWORK_TASK_ID:-}")' && " fi -worker_cmd="${env_prefix}claude --permission-mode ${perm}" +model_arg="" +[ -n "$model" ] && model_arg=" --model '$(esc_sq "$model")'" +worker_cmd="${env_prefix}claude --permission-mode ${perm}${model_arg}" print_cmd() { printf 'orca'; for a in "$@"; do printf ' [%s]' "$a"; done; printf '\n'; } orca_run() { # $1 = fatal flag (1 = return non-zero on failure); rest = orca args diff --git a/skills/orchestrate/scripts/orca-worker-start.sh b/skills/orchestrate/scripts/orca-worker-start.sh index e348766..fe3f0f7 100755 --- a/skills/orchestrate/scripts/orca-worker-start.sh +++ b/skills/orchestrate/scripts/orca-worker-start.sh @@ -32,17 +32,26 @@ # # When GROUNDWORK_ESCALATION_DIR is NOT set, worker mode falls back to Orca's # composed agent-first `worker-start --agent`, which also accepts new-child / -# new-top-level and adds no fallback shell. +# new-top-level and adds no fallback shell. `--model` is inert on that path for +# the same reason `--permission-mode` is: Orca builds the agent command itself, +# so there is nothing of ours to append to. Set the escalation dir (which +# orchestrate always does) to get a model-pinned worker. # # usage: # orca-worker-start.sh --task --worktree --agent -# [--name ] [--perm ] +# [--name ] [--perm ] [--model ] # orca-worker-start.sh --task --terminal # # env (also test hooks): # GROUNDWORK_ESCALATION_DIR exported into the worker so a guardrails `ask` # escalates instead of blocking (activates worker mode) # GROUNDWORK_TASK_ID worker task label +# DEV_LOOP_WORKER_MODEL default for --model — the model the WORKER runs +# (e.g. claude-sonnet-5). Unset = omit the flag, so +# the worker inherits the user's configured model. +# Lets the implementer run a cheaper tier than the +# coordinator; the auditor is pinned separately in +# agents/test-quality-auditor.md. # LO_READY_TIMEOUT seconds to wait for TUI readiness (default 60) # ORCA_BIN orca executable (default: orca) # ORCA_WORKER_START_DRYRUN print the orca commands instead of running them @@ -63,11 +72,12 @@ ORCA="${ORCA_BIN:-orca}" JQ=$(command -v jq) || { echo "orca-worker-start: jq not found" >&2; exit 127; } usage() { - echo "usage: orca-worker-start.sh --task (--worktree --agent [--name ] [--perm ] | --terminal )" >&2 + echo "usage: orca-worker-start.sh --task (--worktree --agent [--name ] [--perm ] [--model ] | --terminal )" >&2 exit 1 } task=""; wt=""; agent=""; name=""; term=""; perm="bypassPermissions" +model="${DEV_LOOP_WORKER_MODEL:-}" while [ $# -gt 0 ]; do case "$1" in --task) task="${2:-}"; shift 2 || usage ;; @@ -76,6 +86,7 @@ while [ $# -gt 0 ]; do --name) name="${2:-}"; shift 2 || usage ;; --terminal) term="${2:-}"; shift 2 || usage ;; --perm) perm="${2:-}"; shift 2 || usage ;; + --model) model="${2:-}"; shift 2 || usage ;; *) echo "orca-worker-start: unknown argument '$1'" >&2; usage ;; esac done @@ -98,6 +109,17 @@ if [ -n "$agent" ]; then *) echo "orca-worker-start: unsupported agent '$agent'" >&2; exit 2 ;; esac fi + +# The model is interpolated into the same command line. Model ids and aliases are +# alphanumerics plus . _ - and the [1m] context suffix (e.g. opus[1m]); anything +# else — spaces, quotes, $, ;, backticks — is a command-injection vector, so +# reject the whole value rather than trying to sanitize it. +if [ -n "$model" ]; then + case "$model" in + *[!A-Za-z0-9._\[\]-]*) + echo "orca-worker-start: invalid model '$model'" >&2; exit 2 ;; + esac +fi case "$perm" in bypassPermissions|acceptEdits|plan|default) : ;; *) echo "orca-worker-start: invalid permission mode '$perm'" >&2; exit 2 ;; @@ -213,7 +235,9 @@ if [ "$worker_mode" = 1 ] && [ -z "$reused" ]; then # Single-quote the values with embedded quotes escaped (`'\''`) so a path with # any metacharacter — including a quote — cannot break out of the command. esc_sq() { printf '%s' "$1" | sed "s/'/'\\\\''/g"; } - worker_cmd="export GROUNDWORK_ESCALATION_DIR='$(esc_sq "$esc_dir")' && export GROUNDWORK_TASK_ID='$(esc_sq "${GROUNDWORK_TASK_ID:-}")' && claude --permission-mode ${perm}" + model_arg="" + [ -n "$model" ] && model_arg=" --model '$(esc_sq "$model")'" + worker_cmd="export GROUNDWORK_ESCALATION_DIR='$(esc_sq "$esc_dir")' && export GROUNDWORK_TASK_ID='$(esc_sq "${GROUNDWORK_TASK_ID:-}")' && claude --permission-mode ${perm}${model_arg}" set -- terminal create --worktree "$wt" --command "$worker_cmd" --json [ -n "$name" ] && set -- "$@" --title "$name" diff --git a/skills/orchestrate/templates/brief.md b/skills/orchestrate/templates/brief.md index 1be6125..24c798c 100644 --- a/skills/orchestrate/templates/brief.md +++ b/skills/orchestrate/templates/brief.md @@ -58,9 +58,14 @@ specific tags below as authority. complexity={simple|medium|complex}; loop-implement max 3 retries; stop exploring once DoD is met - + + .orchestration/plans/{TASK}.md — written by the coordinator; adopt, verify against this brief, do not re-author + + - plan -> .orchestration/plans/{TASK}.md signal -> STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} worktree=$PWD diff --git a/skills/orchestrate/templates/session-prompt.md b/skills/orchestrate/templates/session-prompt.md index 83136c7..64aa5a1 100644 --- a/skills/orchestrate/templates/session-prompt.md +++ b/skills/orchestrate/templates/session-prompt.md @@ -68,7 +68,7 @@ block — to every §1–§4 prompt, flattened into the single sent line. ## (1) Plan — injected at session launch -You are the session for {TASK}. Treat .orchestration/briefs/{TASK}.md `` as authority — especially ``, ``, and ``. Use the loop-implement skill but STOP after planning: run its step 2 with the bundled `wiki-plan` skill (make every design decision grounded in a `wiki/` page — record the decision->page map; leave nothing "as appropriate"), write the resulting implementation plan to .orchestration/plans/{TASK}.md, then run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} plan_ready worktree=$PWD` and wait for an approval message. Do NOT write implementation code yet. +You are the session for {TASK}. Treat .orchestration/briefs/{TASK}.md `` as authority — especially ``, ``, and ``. Use the loop-implement skill but STOP after planning. The coordinator has ALREADY run `wiki-plan` and written the plan to .orchestration/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" path: ADOPT that plan, do not re-plan it. Check it against the brief — every decision actually made (nothing left "as appropriate"), each with its decision->page map entry, and no contradiction with ``, ``, or ``. If it fails any of those, do NOT quietly rewrite it: report the specific gap as a failure and stop, so the coordinator re-plans on the planning model. Otherwise run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} plan_ready worktree=$PWD` and wait for an approval message. Do NOT write implementation code yet. ## (2) Implement — injected after plan approval @@ -104,13 +104,19 @@ prompt here says "wait" — the worker reports and ends its turn. You are the worker session for {TASK}. Treat .orchestration/briefs/{TASK}.md `` as authority — especially ``, ``, and -``. Use the loop-implement skill but STOP after planning: run its -step 2 with the bundled `wiki-plan` skill (make every design decision grounded in a -`wiki/` page — record the decision->page map; leave nothing "as appropriate"), write the -resulting implementation plan to .orchestration/plans/{TASK}.md, then run +``. Use the loop-implement skill but STOP after planning. The +coordinator has ALREADY run `wiki-plan` and written the plan to +.orchestration/plans/{TASK}.md, so take loop-implement step 2's "a plan already exists" +path: ADOPT that plan, do not re-plan it. Read it against the brief and check it is +executable — every design decision actually made (nothing left "as appropriate"), each +one carrying its decision->page map entry, and no contradiction with +``, ``, or ``. If it fails any of +those, do NOT quietly rewrite it: report the specific gap as a failure and stop, so the +coordinator re-plans on the planning model. Otherwise run `STATUS_DIR={STATUS_DIR} sh {SKILL}/scripts/status-update.sh {TASK} plan_ready worktree=$PWD` and report exactly once: -`orca orchestration send --type worker_done --subject "plan_ready: {TASK}" --body "" --task-id {ORCA_TASK_ID} --dispatch-id {ORCA_DISPATCH_ID} --outcome succeeded --files-modified ".orchestration/plans/{TASK}.md" --json` +`orca orchestration send --type worker_done --subject "plan_ready: {TASK}" --body "" --task-id {ORCA_TASK_ID} --dispatch-id {ORCA_DISPATCH_ID} --outcome succeeded --json` +(no `--files-modified`: you adopted the coordinator's plan and wrote nothing.) (a failure is `--outcome failed`, never failure encoded only in prose). Then END YOUR TURN. Do NOT write implementation code yet. diff --git a/tests/launch-session.bats b/tests/launch-session.bats index 740b773..0f7c11d 100644 --- a/tests/launch-session.bats +++ b/tests/launch-session.bats @@ -349,3 +349,39 @@ pane_not_ready() { # matches none of the ready/trust patterns [ "$(jq -r .session "$st/t3.json")" = "lo-1" ] [ "$(jq -r .worktree "$st/t3.json")" = "$BATS_TEST_TMPDIR/wt" ] } + +# --- DEV_LOOP_WORKER_MODEL -------------------------------------------------- +# The worker may run a cheaper tier than the coordinator. Unset MUST stay +# byte-identical to the previous behavior (no --model at all), or every existing +# deployment silently changes model on upgrade. + +@test "MODEL: an invalid model is rejected before anything launches (injection guard)" { + run env LO_DRY_RUN=1 DEV_LOOP_WORKER_MODEL='bad; rm -rf ~' \ + bash "$LS" lo-1 "${BATS_TEST_TMPDIR}/wt" bypassPermissions "prompt" + [ "$status" -eq 2 ] + [[ "$output" == *"invalid model"* ]] +} + +@test "MODEL: unset adds no --model flag (boundary — unchanged behavior)" { + sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd" + # capture #1 is the readiness check, #2 the submission confirm + pane_ready_submitted > "$sd/pane-1" + pane_ready_submitted > "$sd/pane-2" + run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ + LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=4 LO_SUBMIT_INTERVAL=1 \ + sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD p" + [ "$status" -eq 0 ] + ! grep -q -- '--model' "$sd/keys" +} + +@test "MODEL: a set model reaches the launched claude command" { + sd="$BATS_TEST_TMPDIR/sd"; mkdir -p "$sd" + pane_ready_submitted > "$sd/pane-1" + pane_ready_submitted > "$sd/pane-2" + run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ + DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ + LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=4 LO_SUBMIT_INTERVAL=1 \ + sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD p" + [ "$status" -eq 0 ] + grep -q -- "--model 'claude-sonnet-5'" "$sd/keys" +} diff --git a/tests/orca-spawn.bats b/tests/orca-spawn.bats index e0974c3..f758471 100644 --- a/tests/orca-spawn.bats +++ b/tests/orca-spawn.bats @@ -69,3 +69,28 @@ setup() { # the POSIX single-quote escape '\'' must appear — proves the value was escaped [[ "$output" == *"/p'\\''q"* ]] } + +# --- worker model pin (DEV_LOOP_WORKER_MODEL) --------------------------------- +# orca-spawn takes positional args only, so the model arrives by env — the same +# variable orca-worker-start.sh and launch-session.sh read. + +@test "model: DEV_LOOP_WORKER_MODEL reaches the claude command" { + run env ORCA_SPAWN_DRYRUN=1 DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ + bash "$OS" "r::/wt" bypassPermissions "p" + [ "$status" -eq 0 ] + [[ "$output" == *"claude --permission-mode bypassPermissions --model 'claude-sonnet-5'"* ]] +} + +@test "model: unset adds no --model flag (boundary — unchanged behavior)" { + run env ORCA_SPAWN_DRYRUN=1 bash "$OS" "r::/wt" bypassPermissions "p" + [ "$status" -eq 0 ] + [[ "$output" != *"--model"* ]] +} + +@test "model: a shell-metacharacter model is rejected, nothing is created" { + run env ORCA_SPAWN_DRYRUN=1 DEV_LOOP_WORKER_MODEL='x; rm -rf ~' \ + bash "$OS" "r::/wt" bypassPermissions "p" + [ "$status" -eq 2 ] + [[ "$output" == *"invalid model"* ]] + [[ "$output" != *"[terminal] [create]"* ]] +} diff --git a/tests/orca-worker-start.bats b/tests/orca-worker-start.bats index 0afacdd..8a7bfbd 100644 --- a/tests/orca-worker-start.bats +++ b/tests/orca-worker-start.bats @@ -385,3 +385,53 @@ setup() { --task task_1 --worktree "id:r::/wt" --agent claude --perm "bad; rm -rf ~" [ "$status" -eq 2 ] } + +# --- worker model pin (DEV_LOOP_WORKER_MODEL / --model) ----------------------- +# Lets the implementer run a cheaper tier than the coordinator. Unset MUST stay +# byte-identical to the previous command, or an upgrade silently switches model. + +@test "model: --model reaches the created terminal's claude command" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude --model claude-sonnet-5 + [ "$status" -eq 0 ] + [[ "$output" == *"claude --permission-mode bypassPermissions --model 'claude-sonnet-5'"* ]] +} + +@test "model: DEV_LOOP_WORKER_MODEL is the default when --model is absent" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" == *"--model 'claude-sonnet-5'"* ]] +} + +@test "model: --model overrides the env default" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude --model opus + [ "$status" -eq 0 ] + [[ "$output" == *"--model 'opus'"* ]] + [[ "$output" != *"claude-sonnet-5"* ]] +} + +@test "model: the [1m] context suffix is accepted (boundary)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude --model 'opus[1m]' + [ "$status" -eq 0 ] + [[ "$output" == *"--model 'opus[1m]'"* ]] +} + +@test "model: unset adds no --model flag (boundary — unchanged behavior)" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude + [ "$status" -eq 0 ] + [[ "$output" != *"--model"* ]] +} + +@test "model: a shell-metacharacter model is rejected, nothing is created" { + run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude --model 'x; rm -rf ~' + [ "$status" -eq 2 ] + [[ "$output" == *"invalid model"* ]] + [[ "$output" != *"[terminal] [create]"* ]] +} diff --git a/tests/send-prompt.bats b/tests/send-prompt.bats index 747b44e..d74facb 100644 --- a/tests/send-prompt.bats +++ b/tests/send-prompt.bats @@ -423,9 +423,12 @@ tpl_sections_single_line() { # Bumped from 1714004932/4937 when rule [4] gained the ask-timeout contract: # a timeout leaves the question pending, so the worker resumes it instead of # deciding it. Update in the SAME commit as any intentional edit, as above. + # Bumped from 3932390147/5667 when O1 became adopt-the-coordinator's-plan + # instead of author-your-own: planning moved to the coordinator so it runs on + # the planning model, not on whatever tier the worker is pinned to. run sh -c "sed -n '/^\*\*Orca substrate\.\*\*/,/^## Subagent usage protocol/p' '$TPL' | cksum" [ "$status" -eq 0 ] - [ "$output" = "3932390147 5667" ] + [ "$output" = "4060540920 6077" ] } @test "template: the Orca ask rule forbids deciding a timed-out question" { From 43245a2bdfff933b2687e62045fe3e909688a702 Mon Sep 17 00:00:00 2001 From: dch0202 Date: Sat, 8 Aug 2026 00:27:13 +0900 Subject: [PATCH 2/2] feat(orchestrate): pin the auditor to Fable; document the planning-model lever MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Fable 5 access is confirmed on this account, so the self-grading guard runs on the strongest available tier instead of Opus. Verified rather than assumed: `--output-format json` reported modelUsage.canonicalModel "claude-fable-5", provider firstParty, 1M context — not a silent fallback. Also spells out what step 2a implies but never said: because planning now runs in the coordinator session, the planning model IS the coordinator's model. There is no third setting — start the coordinator with `claude --model ` and leave DEV_LOOP_WORKER_MODEL on the cheaper implementer tier. Test-isolation fix in the same pass: the three "unset adds no --model" boundary tests read the ambient environment, so once DEV_LOOP_WORKER_MODEL was set as a real user setting they failed — they were asserting the developer's shell, not the code. They now pass `env -u DEV_LOOP_WORKER_MODEL` explicitly. Suite is green (436/0) *with* the variable exported, which is what makes the fix real. Co-Authored-By: Claude Opus 5 (1M context) --- agents/test-quality-auditor.md | 2 +- skills/orchestrate/SKILL.md | 6 ++++++ tests/launch-session.bats | 5 ++++- tests/orca-spawn.bats | 5 ++++- tests/orca-worker-start.bats | 5 ++++- 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/agents/test-quality-auditor.md b/agents/test-quality-auditor.md index 9496873..d2e91f0 100644 --- a/agents/test-quality-auditor.md +++ b/agents/test-quality-auditor.md @@ -2,7 +2,7 @@ name: test-quality-auditor description: Read-only verifier that audits one task's diff and tests for quality. Invoked between self-review and done so the session that wrote the code does not grade its own tests (self-grading guard). Returns a fixed VERDICT and REASONS. tools: Read, Grep, Glob, Bash -model: opus +model: fable --- You are an independent test-quality auditor for loop-orchestrator. You DO NOT diff --git a/skills/orchestrate/SKILL.md b/skills/orchestrate/SKILL.md index 5defc09..d6f9c07 100644 --- a/skills/orchestrate/SKILL.md +++ b/skills/orchestrate/SKILL.md @@ -348,6 +348,12 @@ reasoning-effort flags) that `worker-start` cannot express. of authoring one, and still signals `plan_ready` — so the phase sequence, the `plan_ready` watch, and the ready-set scheduler are all unchanged. + Because planning happens here, **the planning model is whatever model this + coordinator session is running**. There is no separate setting to turn: to plan + on a stronger tier than you implement on, start the coordinator on that tier + (`claude --model `) and leave `DEV_LOOP_WORKER_MODEL` pointed at + the cheaper implementer tier. + A worker that reports the plan is contradictory or under-decided is telling you the planning pass was wrong: fix `plans/.md` here and re-send §1. Do not let the worker re-plan — that silently moves planning back onto the worker tier, diff --git a/tests/launch-session.bats b/tests/launch-session.bats index 0f7c11d..f682766 100644 --- a/tests/launch-session.bats +++ b/tests/launch-session.bats @@ -354,6 +354,9 @@ pane_not_ready() { # matches none of the ready/trust patterns # The worker may run a cheaper tier than the coordinator. Unset MUST stay # byte-identical to the previous behavior (no --model at all), or every existing # deployment silently changes model on upgrade. +# The unset case uses `env -u`: DEV_LOOP_WORKER_MODEL is a real user setting, +# so a developer with it exported would otherwise see this test pass vacuously +# (or fail) depending on their shell rather than on the code. @test "MODEL: an invalid model is rejected before anything launches (injection guard)" { run env LO_DRY_RUN=1 DEV_LOOP_WORKER_MODEL='bad; rm -rf ~' \ @@ -367,7 +370,7 @@ pane_not_ready() { # matches none of the ready/trust patterns # capture #1 is the readiness check, #2 the submission confirm pane_ready_submitted > "$sd/pane-1" pane_ready_submitted > "$sd/pane-2" - run env STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ + run env -u DEV_LOOP_WORKER_MODEL STUB_DIR="$sd" LO_TMUX="$(mk_tmux_stub)" LO_CLAUDE=/bin/echo \ LO_READY_TIMEOUT=2 LO_READY_INTERVAL=1 LO_SUBMIT_TIMEOUT=4 LO_SUBMIT_INTERVAL=1 \ sh "$LS" lo-1 "$BATS_TEST_TMPDIR" bypassPermissions "ZZPROMPTHEAD p" [ "$status" -eq 0 ] diff --git a/tests/orca-spawn.bats b/tests/orca-spawn.bats index f758471..bd9770a 100644 --- a/tests/orca-spawn.bats +++ b/tests/orca-spawn.bats @@ -73,6 +73,9 @@ setup() { # --- worker model pin (DEV_LOOP_WORKER_MODEL) --------------------------------- # orca-spawn takes positional args only, so the model arrives by env — the same # variable orca-worker-start.sh and launch-session.sh read. +# The unset case uses `env -u`: DEV_LOOP_WORKER_MODEL is a real user setting, +# so a developer with it exported would otherwise see this test pass vacuously +# (or fail) depending on their shell rather than on the code. @test "model: DEV_LOOP_WORKER_MODEL reaches the claude command" { run env ORCA_SPAWN_DRYRUN=1 DEV_LOOP_WORKER_MODEL=claude-sonnet-5 \ @@ -82,7 +85,7 @@ setup() { } @test "model: unset adds no --model flag (boundary — unchanged behavior)" { - run env ORCA_SPAWN_DRYRUN=1 bash "$OS" "r::/wt" bypassPermissions "p" + run env -u DEV_LOOP_WORKER_MODEL ORCA_SPAWN_DRYRUN=1 bash "$OS" "r::/wt" bypassPermissions "p" [ "$status" -eq 0 ] [[ "$output" != *"--model"* ]] } diff --git a/tests/orca-worker-start.bats b/tests/orca-worker-start.bats index 8a7bfbd..59f2f99 100644 --- a/tests/orca-worker-start.bats +++ b/tests/orca-worker-start.bats @@ -389,6 +389,9 @@ setup() { # --- worker model pin (DEV_LOOP_WORKER_MODEL / --model) ----------------------- # Lets the implementer run a cheaper tier than the coordinator. Unset MUST stay # byte-identical to the previous command, or an upgrade silently switches model. +# The unset case uses `env -u`: DEV_LOOP_WORKER_MODEL is a real user setting, +# so a developer with it exported would otherwise see this test pass vacuously +# (or fail) depending on their shell rather than on the code. @test "model: --model reaches the created terminal's claude command" { run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ @@ -422,7 +425,7 @@ setup() { } @test "model: unset adds no --model flag (boundary — unchanged behavior)" { - run env ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ + run env -u DEV_LOOP_WORKER_MODEL ORCA_WORKER_START_DRYRUN=1 GROUNDWORK_ESCALATION_DIR=/e \ bash "$OWS" --task task_1 --worktree "id:r::/wt" --agent claude [ "$status" -eq 0 ] [[ "$output" != *"--model"* ]]