fix(gemini): drop removed --skip-trust flag and recover banner-prefixed stream#19
fix(gemini): drop removed --skip-trust flag and recover banner-prefixed stream#19nedjamez wants to merge 1 commit into
Conversation
…ed stream
Two breakages surfaced running the runner against Gemini CLI >= 0.35:
1. `_builder.py` passed `--skip-trust`, which Gemini CLI removed in 0.35.
The CLI now hard-errors ("Unknown arguments: skip-trust, skipTrust") so
every gemini sub-agent failed before doing any work. The permission
mapping already passes `--approval-mode`, which is the current trust
signal, so `--skip-trust` is simply dropped.
2. `_stream.py` dropped all assistant text when Gemini prepended a non-JSON
banner ("MCP issues detected. Run /mcp list for status.") onto the `init`
event line. The glued line failed `json.loads`, so `is_gemini` was never
set, every assistant `message` was ignored, and the result came back
empty with status "success". The parser now recovers JSON from the first
brace on a line, so a leading banner no longer eats the stream.
Tests updated to match (1) and a regression test added for (2). Canonical
skill + the synced runner plugin copy both updated; full suite passes.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
shinpr
left a comment
There was a problem hiding this comment.
Requesting changes on the Gemini trust handling.
The stream recovery change looks good.
I confirmed that --skip-trust is rejected by Gemini CLI 0.35.1 during normal -p execution, so that compatibility issue is real. But in Gemini CLI 0.41.2, --skip-trust exists again, and removing it causes untrusted workspaces to override --approval-mode back to default.
So --approval-mode does not appear to replace the workspace trust behavior.
Instead of just removing --skip-trust, I think we should set GEMINI_CLI_TRUST_WORKSPACE=true for Gemini invocations. That works with the 0.35.x behavior and the current 0.41.x behavior, and it fits the existing env override path used for GEMINI_SYSTEM_MD.
I’d keep the stream parser fix, but change the Gemini command/env handling before merging.
Running the runner against Gemini CLI ≥ 0.35 surfaced two breakages that together make every
geminisub-agent fail or return empty. Both are fixed here, with tests.1.
--skip-trustwas removed from Gemini CLI (hard failure)build_command()passed--skip-trust, which Gemini CLI removed in 0.35. The CLI now exits non-zero with:so the sub-agent dies before doing any work. The permission mapping already passes
--approval-mode(plan/auto_edit/yolo), which is the current trust/approval signal in headless-pmode, so--skip-trustis simply dropped.2. A stdout banner glued onto
initsilently emptied the resultGemini prepends a non-JSON banner onto the first event line when an MCP server is unreachable, e.g.:
json.loadson that glued line throws, sois_geminiis never set, every assistantmessageis ignored, and the run returns{"result": "", "status": "success"}— a silent empty result.StreamProcessor.process_linenow recovers JSON from the first{on a line, so a leading banner no longer eats the stream.Changes
skills/sub-agents/scripts/_builder.py— drop--skip-trust(comment explains why).skills/sub-agents/scripts/_stream.py— recover JSON after a non-JSON line prefix.scripts/sync_plugin.py(drift check passes).tests/test_builder.py— updated expected gemini args; asserts--skip-trustabsent.tests/test_stream.py— added a regression test for the banner-prefixedinitline.Verification
python3 scripts/sync_plugin.py --check→ in syncgemini0.35.1 +gemini-3-flash-preview: pre-fix the run errored on--skip-trust, then returned empty; post-fix it returns the model's text.