fix: enforce read-only mode for JSON line agents and pass customModel in CLI#563
fix: enforce read-only mode for JSON line agents and pass customModel in CLI#563chr1syy wants to merge 3 commits intoRunMaestro:0.16.0-RCfrom
Conversation
spawnAgent was dropping the readOnly flag when calling spawnJsonLineAgent, and spawnJsonLineAgent ignored it entirely. Now readOnlyArgs and readOnlyEnvOverrides from agent definitions are applied for all JSON line agents (Codex, OpenCode, Factory Droid) in read-only/plan mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… in CLI Three fixes for CLI batch/send agent spawning: 1. Pass readOnly flag to spawnJsonLineAgent (was silently dropped) 2. Skip YOLO/bypass args in read-only mode for all agents — they were overriding read-only flags (--dangerously-skip-permissions for Claude, --dangerously-bypass-approvals-and-sandbox for Codex, -y for Gemini) 3. Read customModel from agent session config and pass it via modelArgs so CLI-spawned agents use the model configured in the desktop UI Also fixes OpenCode read-only env overrides to keep blanket permission grants (prevents stdin hangs in batch mode) since --agent plan handles read-only enforcement at the CLI level. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes three independent bugs in Maestro's CLI agent-spawning pipeline: (1) YOLO/permission-bypass args were always applied even in read-only mode, negating Key observations:
Confidence Score: 3/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[spawnAgent called] --> B{toolType?}
B -->|claude-code| C[spawnClaudeAgent\ncustomModel silently dropped]
B -->|usesJsonLineOutput| D[spawnJsonLineAgent\nreadOnlyMode + customModel passed]
C --> E{readOnlyMode?}
E -->|yes| F[Add readOnlyArgs\nfrom definitions\nskip CLAUDE_YOLO_ARGS]
E -->|no| G[Add CLAUDE_YOLO_ARGS\n--dangerously-skip-permissions]
D --> H{readOnlyMode?}
H -->|yes| I[Apply readOnlyEnvOverrides\nFilter yoloModeArgs from batchModeArgs\nAdd readOnlyArgs]
H -->|no| J[Apply all batchModeArgs\nincluding yoloModeArgs]
I --> K{customModel?}
J --> K
K -->|yes| L[Append modelArgs]
K -->|no| M[Skip modelArgs]
L --> N[Spawn process]
M --> N
F --> N
G --> N
style C fill:#ffe0b2
style I fill:#c8e6c9
style F fill:#c8e6c9
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // Permission bypass arg for Claude — skipped in read-only mode | ||
| const CLAUDE_YOLO_ARGS = ['--dangerously-skip-permissions']; |
There was a problem hiding this comment.
Existing test asserts YOLO flag is still present in read-only mode
The removal of --dangerously-skip-permissions from CLAUDE_ARGS and its relegation to CLAUDE_YOLO_ARGS (only added in non-read-only mode) is the correct core change of this PR. However, the existing test at src/__tests__/cli/services/agent-spawner.test.ts:1179 explicitly asserts that --dangerously-skip-permissions is still present when readOnlyMode: true:
// From agent-spawner.test.ts, line ~1178-1179
// Should still have base args
expect(args).toContain('--print');
expect(args).toContain('--dangerously-skip-permissions'); // ← contradicts this PR's intentThis assertion was written against the old behaviour where --dangerously-skip-permissions lived in CLAUDE_ARGS and was therefore always included. After this PR it will no longer be present in read-only mode, so the test will fail. The assertion should be inverted:
expect(args).not.toContain('--dangerously-skip-permissions');Additionally, a new assertion checking it is present in normal (non-read-only) mode would close the gap in coverage.
| // In read-only mode, filter out YOLO/bypass args from batchModeArgs | ||
| // (they override read-only flags). In normal mode, apply all batchModeArgs. | ||
| if (def?.batchModeArgs) { | ||
| if (readOnlyMode && def.yoloModeArgs?.length) { | ||
| const yoloSet = new Set(def.yoloModeArgs); | ||
| args.push(...def.batchModeArgs.filter((a) => !yoloSet.has(a))); | ||
| } else { | ||
| args.push(...def.batchModeArgs); | ||
| } | ||
| } |
There was a problem hiding this comment.
Gemini CLI may hang in read-only batch mode after -y is stripped
For the Gemini CLI agent, batchModeArgs: ['-y'] and yoloModeArgs: ['-y'] are the same flag. In read-only mode, the new filtering logic strips -y from batchModeArgs:
if (readOnlyMode && def.yoloModeArgs?.length) {
const yoloSet = new Set(def.yoloModeArgs);
args.push(...def.batchModeArgs.filter((a) => !yoloSet.has(a)));
}-y is Gemini's auto-confirm flag; without it Gemini may prompt for interactive confirmation on any tool use or file access. Since stdin is closed immediately after spawning (child.stdin?.end()), this would cause Gemini to receive EOF on its prompt input and either hang or exit with an error.
OpenCode faced the same problem and was fixed by keeping "*":"allow" in its readOnlyEnvOverrides. For Gemini there is no readOnlyEnvOverrides defined, and readOnlyArgs is intentionally empty (readOnlyCliEnforced: false).
One option is to add a Gemini-specific readOnlyEnvOverrides that sets GEMINI_YOLO=1 (or equivalent) to prevent interactive prompts while still relying on the system prompt for read-only enforcement. Alternatively, keep -y for Gemini even in read-only mode (similar to the OpenCode fix) and rely solely on the system prompt to prevent writes.
- Invert stale test asserting --dangerously-skip-permissions is present in read-only mode; add coverage for its presence in normal mode - Skip yolo-arg filtering for agents without CLI-level read-only enforcement (Gemini CLI needs -y to avoid interactive prompt hang with closed stdin) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
spawnAgentnever passed thereadOnlyflag tospawnJsonLineAgent, and the function ignored it anyway--dangerously-skip-permissions(Claude),--dangerously-bypass-approvals-and-sandbox(Codex),-y(Gemini) were always applied in batch mode, negating read-only enforcementcustomModelfrom the desktop UI agent config was never read or passed, causing agents to use their default model instead of the user-configured onereadOnlyEnvOverridesstripped blanket permission grants, causing OpenCode to prompt on stdin (which is closed in batch mode). Fixed to keep"*":"allow"since--agent planhandles read-only enforcementChanges
src/cli/services/agent-spawner.tsreadOnlyandcustomModeltospawnJsonLineAgent; skip YOLO args in read-only mode for both Claude and JSON line agents; applyreadOnlyArgs,readOnlyEnvOverrides, andmodelArgssrc/cli/commands/send.tsagent.customModeltospawnAgentsrc/cli/services/batch-processor.tssession.customModeltospawnAgentsrc/main/agents/definitions.tsreadOnlyEnvOverridesto keep blanket permission grantssrc/shared/types.tscustomModeltoSessionInfointerfaceTest plan
-r) prevents file modifications🤖 Generated with Claude Code