fix(doctor): build the gemini probe from the dispatch argv builder - #87
Open
kevin-agrology wants to merge 4 commits into
Open
fix(doctor): build the gemini probe from the dispatch argv builder#87kevin-agrology wants to merge 4 commits into
kevin-agrology wants to merge 4 commits into
Conversation
T3's fixed offset-of-one absorbs any leading exec-wrapper on the probe side, not just $0, so a future unconditional env prefix would pass T1, T2, and T3 alike. Add G4/T1b to assert the probe carries no trust prefix when autotrust is off, and reword T3's comment to say what it actually proves. Reset argv.bin before T3's own doctor run so a stale file from T2 can't pass on data T3 never produced. Document that doctor's probe now runs --approval-mode auto_edit, same as dispatch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR:
multi-review-reviewer.sh doctorwas testing gemini with a different command than the one it actually dispatches, so it could report "ready" for a setup that fails on every real review — and report a failure for a setup that would have worked fine. Both now come from a single builder, so they cannot drift apart again.The problem
doctor's live probe and the real dispatch each built their own argv:gemini -m <model> -p "reply with OK"[env GEMINI_CLI_TRUST_WORKSPACE=true] gemini -m <model> --approval-mode auto_edit -p <prompt>Two independent divergences, one in each direction:
--approval-mode auto_edit— a CLI that rejects that flag probes green while every dispatch is rejected.envprefix — a workspace that only becomes trusted underMULTI_REVIEW_GEMINI_AUTOTRUST=1probes red while dispatch would have worked.Same "green signal, broken dispatch" class as #73 and #80.
The fix
A single
gemini_argv <model> <prompt>builder.cmd_commandemits its NUL stream to the dispatcher;gemini_live_probereads the same stream into an array and runs it. The prompt is the only deliberate difference left, so a flag added later reaches both with no second edit.No empty-array guard, deliberately (§1.2):
gemini_argvstructurally always emits ≥7 elements, and that guarantee — not a runtime check — is what keeps the bash-3.2set -uexpansion hazard unreachable.Tests
Five assertions in
multi-review-reviewer.test.sh, all run against a stubbedgeminionPATH:--approval-mode auto_edit.gemini.T1b exists because review caught a hole in T3:
envexecs the binary in place, so T3's offset-of-one comparison is blind to any leading wrapper. An unconditional trust prefix on the probe would have passed T1, T2 and T3 alike — the exact drift direction (2) above. T1b was verified to go red before being accepted.Security-relevant (§3)
doctor's probe now runs at--approval-mode auto_edit, and underMULTI_REVIEW_GEMINI_AUTOTRUST=1with the workspace trusted. A diagnostic gains edit approval it previously did not have. Bounded by: a fixed benign prompt (reply with OK) with no document attached,auto_editapproving edit tools only and never shell, a 30s kill escalation, and discarded output. In an untrusted workspace the flag is inert, since edits are disabled there regardless. Making the diagnostic match the thing it diagnoses is the point of the change, but it is a real widening and wants focused review. Documented in the README's gemini prereqs.Verification
bash5 and/bin/bash3.2.57.shellcheck --severity=warningclean onscripts/multi-review-*.shand.githooks/pre-push.multi-review-version-check.sh: 1.24.0 → 1.24.1.reviewer/gemini-probe-uses-dispatch-argv; zero SURVIVED/MISCREDITED/STALE.Verification limit:
geminiis not installed on the development machine, so every assertion runs against a stub. The argv agreement is proven; that the real CLI accepts that argv is not, and this PR does not claim it.Process
Spec and plan each went through this repo's own multi-review (codex + fable) before implementation; the branch was then executed task-by-task with a review after each and a whole-branch review at the end.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Nm3hNXA4WHbgKjFbxX7VUE