fix(doctor): build the gemini probe from the dispatch argv builder

.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "multi-review",
   "description": "Human-gated star review: Claude (the neutral primary) fans a spec/plan/PR out to N independent secondary reviewers (in-harness fable by default — disable with MULTI_REVIEW_FABLE=off — plus cross-vendor codex/gemini), adjudicates their findings, and converges — always ending at a human approval gate.",
-  "version": "1.24.0",
+  "version": "1.24.1",
   "author": {
     "name": "Agrology"
   },

README.md

@@ -133,6 +133,8 @@ refuses to run until you remove it, and `--check-reviewers`/`doctor` will flag t
 1. **`export GEMINI_CLI_TRUST_WORKSPACE=true`** (or trust the folder once). An untrusted workspace
    makes the CLI skip `.env` (so it can't authenticate — the error misleadingly says "set an Auth
    method") *and* disables file edits, so the reviewer can't write the doc.
+   `doctor`'s probe runs the same command line as a real dispatch, including
+   `--approval-mode auto_edit`, so its verdict reflects what dispatch will actually do.
 2. An API key — `export GEMINI_API_KEY=…`, or drop `GEMINI_API_KEY=…` in `~/.gemini/.env` (or your
    repo's `.env`). **It auto-loads once the workspace is trusted.**
 3. `.gemini/settings.json` → `{"context":{"fileFiltering":{"respectGitIgnore":false}}}`. Needed
@@ -156,7 +158,7 @@ Run **`/multi-review --check-reviewers`** to verify every reviewer's setup at a
 | `MULTI_REVIEW_MAX_ROUNDS` | `5` | round **ceiling** (each round costs N dispatches; convergence is adaptive) |
 | `MULTI_REVIEW_REVIEWER_MODEL` | *(provider default)* | pin a provider's model (`codex`→`gpt-5.6-terra`, `fable`→`fable`, `gemini`→`gemini-pro-latest`) |
 | `MULTI_REVIEW_DOC_DIRS` | `docs/specs docs/plans docs/superpowers/specs docs/superpowers/plans` | where bare-name local docs are resolved. Covers the plain and `superpowers` layouts out of the box. Bare-name resolution **warns** when a newer dated doc sits in a directory it did not search — the egress guard cannot catch that, since the doc it picked is legitimately inside the configured dirs. |
-| `MULTI_REVIEW_GEMINI_AUTOTRUST` | *(off)* | `=1` scopes `GEMINI_CLI_TRUST_WORKSPACE=true` to the gemini dispatch (no profile edit needed). **Security:** trusting a workspace lets gemini honor its `.env`/settings and auto-edit — enable only for repos you trust, never a freshly-cloned one. |
+| `MULTI_REVIEW_GEMINI_AUTOTRUST` | *(off)* | `=1` scopes `GEMINI_CLI_TRUST_WORKSPACE=true` to the gemini dispatch **and to `doctor`'s live probe, which runs the same argv** (no profile edit needed). **Security:** trusting a workspace lets gemini honor its `.env`/settings and auto-edit — enable only for repos you trust, never a freshly-cloned one. |
 
 The last explicitly-named reviewer combo is remembered per repo in **`.multi-review/reviewers.pref`**
 (gitignored). It is written when reviewers are named (flag or prose) — **except** when

scripts/multi-review-mutation-check.sh

@@ -1141,6 +1141,27 @@ mutations() {
     'a multi-line-block-commented respectGitIgnore read as a live opt-out' 'multi-review-reviewer.test.sh' \
     '        if (inblock) {' \
     '        if (0) {'
+
+  # ---- G4: doctor's probe must RUN the dispatch argv -------------------------------------------
+
+  # Reverting the probe to its own literal restores the two-builder split, where doctor and
+  # dispatch could disagree in both directions. T3 is the assertion that encodes this guard
+  # directly (argv equality); T1 also fails, on the dropped `--approval-mode`, but that is one
+  # consequence rather than the guard itself. Expecting T3 is safe because this suite is NOT
+  # fail-fast — `bad()` increments a counter and returns, so every assertion runs and T3's FAIL
+  # line is emitted even though T1 fails first.
+  #
+  # The replacement restores the old BEHAVIOUR on one parseable line, not merely the old text.
+  # Substituting the literal into a multi-line `while … \` + `< <(…)` construct would leave either
+  # unparseable bash (which the runner rejects as "failing for the wrong reason") or an empty
+  # `argv` — the bash-3.2 `set -u` hazard the probe relies on being unreachable. Either way the
+  # entry would "catch" something while proving nothing about T3. That is why the probe writes
+  # the read loop on a single line.
+  mutate 'reviewer/gemini-probe-uses-dispatch-argv' 'scripts/multi-review-reviewer.sh' replace \
+    'probe and dispatch argv have drifted' 'multi-review-reviewer.test.sh' \
+    '  while IFS= read -r -d '"'"''"'"' a; do argv+=("$a"); done < <(gemini_argv "$model" "reply with OK")' \
+    '  argv=(gemini -m "$model" -p "reply with OK")'
+
   # ---- convergence integrity ------------------------------------------------------------------
 
   # THE SELF-RESPONSE GUARD. This is what makes the review a review rather than a self-review: it

scripts/multi-review-reviewer.sh

@@ -550,6 +550,33 @@ cmd_prompt() { # <doc> --reviewer <id>
   emit_prompt "$(abs_path "$doc")" "$has_skill" "$kind"
 }
 
+# The gemini dispatch argv, built in ONE place. `cmd_command` emits it for the dispatcher; the
+# doctor's live probe RUNS it with a trivial prompt. Two independent builders let doctor and
+# dispatch disagree in BOTH directions — a probe missing `--approval-mode` passes on a CLI that
+# rejects the dispatch, and one missing the autotrust prefix fails on a workspace the dispatch
+# trusts. The prompt is the ONLY deliberate difference between the two callers, so a flag added
+# here reaches both without a second edit.
+#
+# `model` always carries a value (registry default or the env override), so the model is
+# always explicit — we never fall through to the CLI's own default tier.
+#
+# `--approval-mode auto_edit` is the analogue of the codex route's `--write`. Without it
+# `gemini -p` runs at approval mode `default` ("prompt for approval") with nobody there
+# to prompt, so file-modification tools are disabled: observed live, the reviewer emitted
+# its findings as prose and never touched the doc, leaving the marker unflipped. Chosen
+# over `yolo` deliberately — `auto_edit` approves edit tools only, never shell.
+# Opt-in auto-trust (MULTI_REVIEW_GEMINI_AUTOTRUST=1) scopes GEMINI_CLI_TRUST_WORKSPACE=true to
+# THIS dispatch via an `env` prefix, so users needn't set it in their profile. Default
+# (unset/≠1) is byte-identical to before. SECURITY: trusting a workspace lets the CLI honor its
+# .env / settings and auto-edit — opt-in only, never a cloned/untrusted repo (see README).
+gemini_argv() { # <model> <prompt> -> NUL-delimited argv
+  if [[ "${MULTI_REVIEW_GEMINI_AUTOTRUST:-}" == "1" ]]; then
+    printf '%s\0' "env" "GEMINI_CLI_TRUST_WORKSPACE=true" "gemini" "-m" "$1" "--approval-mode" "auto_edit" "-p" "$2"
+  else
+    printf '%s\0' "gemini" "-m" "$1" "--approval-mode" "auto_edit" "-p" "$2"
+  fi
+}
+
 cmd_command() { # <doc> --reviewer <id> -> NUL-delimited argv
   local doc="${1:-}"
   [[ -n "$doc" ]] || die "usage: multi-review-reviewer.sh command <doc-path> --reviewer <id>" 2
@@ -570,24 +597,7 @@ cmd_command() { # <doc> --reviewer <id> -> NUL-delimited argv
   # never re-parse this through a shell. Consumer idiom (bash 3.2 safe, no mapfile):
   #   argv=(); while IFS= read -r -d '' a; do argv+=("$a"); done < <(… command "$doc")
   case "$id" in
-    gemini)
-      # `model` always carries a value (registry default or the env override), so the model is
-      # always explicit — we never fall through to the CLI's own default tier.
-      #
-      # `--approval-mode auto_edit` is the analogue of the codex route's `--write`. Without it
-      # `gemini -p` runs at approval mode `default` ("prompt for approval") with nobody there
-      # to prompt, so file-modification tools are disabled: observed live, the reviewer emitted
-      # its findings as prose and never touched the doc, leaving the marker unflipped. Chosen
-      # over `yolo` deliberately — `auto_edit` approves edit tools only, never shell.
-      # Opt-in auto-trust (MULTI_REVIEW_GEMINI_AUTOTRUST=1) scopes GEMINI_CLI_TRUST_WORKSPACE=true to
-      # THIS dispatch via an `env` prefix, so users needn't set it in their profile. Default
-      # (unset/≠1) is byte-identical to before. SECURITY: trusting a workspace lets the CLI honor its
-      # .env / settings and auto-edit — opt-in only, never a cloned/untrusted repo (see README).
-      if [[ "${MULTI_REVIEW_GEMINI_AUTOTRUST:-}" == "1" ]]; then
-        printf '%s\0' "env" "GEMINI_CLI_TRUST_WORKSPACE=true" "gemini" "-m" "$model" "--approval-mode" "auto_edit" "-p" "$prompt"
-      else
-        printf '%s\0' "gemini" "-m" "$model" "--approval-mode" "auto_edit" "-p" "$prompt"
-      fi ;;
+    gemini) gemini_argv "$model" "$prompt" ;;
     *)
       die "no shell command defined for reviewer provider '${id}'" 2 ;;
   esac
@@ -794,11 +804,17 @@ cmd_doctor() {
 # can't hang) and reap direct children.
 GEMINI_PROBE_MSG=""
 gemini_live_probe() {
-  local model out pid waited bound
+  local model out pid waited bound a
   model="$(provider_row gemini | cut -d'|' -f4)"
   bound="${MULTI_REVIEW_PROBE_TIMEOUT:-30}"
   out="$(mktemp)"
-  ( gemini -m "$model" -p "reply with OK" >"$out" 2>&1 ) & pid=$!
+  # The probe RUNS the dispatch argv (G4), so doctor certifies the command dispatch actually
+  # uses rather than a second one that can drift from it. The read loop is deliberately ONE
+  # line: the mutation entry replaces it, and `replace` operates on a single line that has to
+  # parse on its own.
+  local argv=()
+  while IFS= read -r -d '' a; do argv+=("$a"); done < <(gemini_argv "$model" "reply with OK")
+  ( "${argv[@]}" >"$out" 2>&1 ) & pid=$!
   waited=0
   while kill -0 "$pid" 2>/dev/null && (( waited < bound )); do sleep 1; waited=$((waited+1)); done
   if kill -0 "$pid" 2>/dev/null; then

scripts/multi-review-reviewer.test.sh

@@ -1466,6 +1466,112 @@ HOME="${CD_}/home" PATH="${CD_}/bin:$PATH" bash "$SUT" check --reviewer codex --
 [[ $? -eq 2 ]] && ok "check: --doc with no value exits 2" \
   || bad "--doc with no value did not exit 2"
 
+# --- G4: doctor's live probe must run the DISPATCH argv, not a second one ---------------------
+# The probe and cmd_command built their argv independently, so they could disagree in BOTH
+# directions: a probe without `--approval-mode auto_edit` passes on a CLI that rejects the
+# dispatch, and one without the autotrust `env` prefix fails on a workspace the dispatch trusts.
+# Same "green signal, broken dispatch" class as #73/#80.
+GA="${WORK}/gargv"; mkdir -p "$GA/bin" "$GA/repo/home"
+( cd "$GA/repo" && git init -q )
+# The stub records the argv it was invoked with (NUL-delimited) plus whether the trust variable
+# reached its environment. `$@` never contains `$0`, so the recorded stream begins at `-m`.
+cat > "$GA/bin/gemini" <<'STUB'
+#!/usr/bin/env bash
+: > "$GA_ARGV"
+for a in "$@"; do printf '%s\0' "$a" >> "$GA_ARGV"; done
+printf '%s' "${GEMINI_CLI_TRUST_WORKSPACE-<unset>}" > "$GA_TRUST"
+echo OK
+STUB
+chmod +x "$GA/bin/gemini"
+# Pre-create both sinks: if the probe never launches, the assertions must fail on their own
+# message rather than on a redirect against a missing file.
+: > "$GA/argv.bin"; : > "$GA/trust.txt"
+
+# bash 3.2 has no mapfile. Populates the global array `nulargv`.
+read_nul() { nulargv=(); local a; while IFS= read -r -d '' a; do nulargv+=("$a"); done < "$1"; }
+
+# Run doctor in the fixture with the stub on PATH. Extra assignments come from the caller and
+# MUST go through `env`: a bare `VAR=x "$@" bash …` would not treat an assignment arriving via
+# "$@" as an assignment at all — the parser fixes the assignment prefixes before "$@" expands,
+# so bash would look for a command literally named `MULTI_REVIEW_GEMINI_AUTOTRUST=1`.
+ga_doctor() {
+  ( cd "$GA/repo" && env HOME="$GA/repo/home" PATH="${GA}/bin:$PATH" \
+      GA_ARGV="$GA/argv.bin" GA_TRUST="$GA/trust.txt" MULTI_REVIEW_PROBE_TIMEOUT=5 \
+      "$@" bash "$SUT" doctor >/dev/null 2>&1 )
+}
+
+# T1 — the probe carries the dispatch's `--approval-mode auto_edit`.
+unset GEMINI_CLI_TRUST_WORKSPACE MULTI_REVIEW_GEMINI_AUTOTRUST
+ga_doctor
+read_nul "$GA/argv.bin"
+probe_argv=(); (( ${#nulargv[@]} )) && probe_argv=("${nulargv[@]}")
+# Arithmetic for-loop, NOT `for i in $(seq 0 $(( len - 1 )))`. BSD seq counts DOWN when the end
+# is below the start, so an empty array yields `seq 0 -1` -> "0 -1" and the body runs against an
+# unbound index: under `set -u` on /bin/bash 3.2 that aborts the whole test FILE with exit 127,
+# masking every assertion after it. GNU seq prints nothing there, so the Ubuntu leg would pass
+# straight through and only the macOS leg would break. The `i + 1 < len` bound also keeps both
+# indices in range, so neither needs a `:-` default.
+t1_appr=0
+for (( i = 0; i + 1 < ${#probe_argv[@]}; i++ )); do
+  [[ "${probe_argv[$i]}" == "--approval-mode" && "${probe_argv[$((i+1))]}" == "auto_edit" ]] && t1_appr=1
+done
+(( t1_appr )) \
+  && ok "G4/T1: doctor's probe carries --approval-mode auto_edit, like dispatch" \
+  || bad "G4/T1: probe argv lacks --approval-mode auto_edit: ${probe_argv[*]:-<none>}"
+
+# T1b — with autotrust OFF (this run's setup), the probe carries no trust prefix at all.
+[[ "$(cat "$GA/trust.txt")" == "<unset>" ]] \
+  && ok "G4/T1b: without autotrust the probe carries no trust prefix" \
+  || bad "G4/T1b: probe saw GEMINI_CLI_TRUST_WORKSPACE='$(cat "$GA/trust.txt")' (want unset)"
+
+# T2 — under autotrust the probe gets the `env GEMINI_CLI_TRUST_WORKSPACE=true` prefix.
+# The unset is PART OF THE TEST, not an assumption about the shell: this suite clears only
+# MULTI_REVIEW_REVIEWER_MODEL globally, and an ambient export would make this pass against a probe
+# that carries no prefix at all — green for a reason unrelated to the fix.
+unset GEMINI_CLI_TRUST_WORKSPACE
+ga_doctor MULTI_REVIEW_GEMINI_AUTOTRUST=1
+[[ "$(cat "$GA/trust.txt")" == "true" ]] \
+  && ok "G4/T2: autotrust reaches the probe's environment, like dispatch" \
+  || bad "G4/T2: probe saw GEMINI_CLI_TRUST_WORKSPACE='$(cat "$GA/trust.txt")' (want true)"
+
+# T3 (anti-drift) — the probe argv IS the dispatch argv, modulo the prompt. Autotrust OFF, so
+# neither stream carries the `env` prefix; but `env` execs the binary in place, so ANY leading
+# wrapper — not just this one — is invisible to `$@` and this comparison's fixed offset of one.
+# T1b (absence when autotrust is off) and T2 (presence when it's on) actually pin the prefix.
+unset GEMINI_CLI_TRUST_WORKSPACE MULTI_REVIEW_GEMINI_AUTOTRUST
+: > "$GA/argv.bin"
+ga_doctor
+read_nul "$GA/argv.bin"
+probe_argv=(); (( ${#nulargv[@]} )) && probe_argv=("${nulargv[@]}")
+# `command` needs no marker in the doc (cmd_command checks only that the file exists, then builds
+# the prompt from the path) and the path is absolute, so this is cwd-independent — a failure here
+# is a real one, not fixture drift.
+printf '# t\n' > "$GA/repo/d.md"
+bash "$SUT" command "$GA/repo/d.md" --reviewer gemini > "$GA/cmd.bin" 2>/dev/null
+read_nul "$GA/cmd.bin"
+cmd_argv=(); (( ${#nulargv[@]} )) && cmd_argv=("${nulargv[@]}")
+
+# The stub records `$@`, which never contains `$0`, while the builder stream starts at `gemini`.
+# Drop that element before comparing, and assert separately that it IS `gemini`.
+[[ "${cmd_argv[0]:-}" == "gemini" ]] \
+  && ok "G4/T3a: dispatch argv element 0 is the gemini binary" \
+  || bad "G4/T3a: dispatch argv element 0 is '${cmd_argv[0]:-<none>}' (want gemini)"
+
+drift=""
+if (( ${#probe_argv[@]} != ${#cmd_argv[@]} - 1 )); then
+  drift="length: probe ${#probe_argv[@]} vs dispatch $(( ${#cmd_argv[@]} - 1 ))"
+else
+  # Arithmetic for-loop for the same reason as T1's, and bounded at len-1 so the trailing -p
+  # payload — the one deliberate difference — is simply never compared.
+  for (( i = 0; i < ${#probe_argv[@]} - 1; i++ )); do
+    [[ "${probe_argv[$i]}" == "${cmd_argv[$((i+1))]}" ]] \
+      || drift="element $i: probe '${probe_argv[$i]}' vs dispatch '${cmd_argv[$((i+1))]}'"
+  done
+fi
+[[ -z "$drift" ]] \
+  && ok "G4/T3: doctor's probe argv is the dispatch argv, modulo the prompt" \
+  || bad "G4/T3: probe and dispatch argv have drifted — $drift"
+
 echo
 if (( fails > 0 )); then echo "FAILED: $fails"; exit 1; fi
 echo "all passed"