feat(agents): replace yolo mode with 3-way permission mode#1155
Conversation
Add permissionMode: 'full' | 'standard' | 'readonly' to replace the binary yolo-mode/read-only toggle. 'full' preserves current bypass behavior (renamed from yolo), 'standard' is new and runs agents with no bypass flags so they use their default permission model, 'readonly' is the existing plan/exploration mode, unchanged. - Move each agent's bypass flags to fullAccessArgs (falls back to legacy yoloModeArgs); opencode's default env now denies the question tool without granting blanket permission, with the old full-allow config moved to fullAccessEnvOverrides - agent-args builder resolves isFullAccess/isReadOnly from permissionMode with legacy yoloMode/readOnlyMode booleans as fallback - Thread permissionMode through the IPC spawn config, preload bridge, and renderer spawn helpers - Toolbar toggle button now cycles full -> standard -> readonly instead of a binary switch; label/tooltip/color driven by getPermissionModeLabel/getPermissionModeTooltip, which preserve the existing per-agent Plan-Mode vs Read-Only wording for readonly state - Cue keeps permissionMode: 'full' (always full access, like Auto Run); context grooming keeps permissionMode: 'standard' (never needs bypass) - Update wizard badge and welcome copy from YOLO to Full Access Co-Authored-By: anthropic/claude-sonnet-4-6 <noreply@opencode.ai>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds ChangesPermission Mode Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 replaces the binary yolo/read-only toggle with a 3-way
Confidence Score: 3/5Merging as-is would silently remove bypass flags from all agent spawns in the default Full Access mode — users would encounter unexpected permission prompts from their agents. The agent definitions correctly move bypass flags to src/renderer/hooks/input/useInputProcessing.ts, src/renderer/stores/agentStore.ts, src/renderer/hooks/remote/useRemoteHandlers.ts — all three spawn calls need Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User clicks toolbar button] --> B[ToolbarControls cycles permissionMode on activeTab]
B --> C{tab.permissionMode}
subgraph spawn["Spawn paths (missing permissionMode forwarding)"]
D[useInputProcessing] -->|readOnlyMode only| IPC
E[agentStore queue] -->|readOnlyMode only| IPC
F[useRemoteHandlers] -->|readOnlyMode only| IPC
end
subgraph correct["Correctly updated paths"]
G[cue-spawn-builder] -->|permissionMode: 'full'| IPC
H[context-groomer] -->|permissionMode: 'standard'| IPC
end
C -->|'full' or undefined| D
C -->|'standard'| D
C -->|'readonly'| D
IPC[IPC process handler] --> J[buildAgentArgs]
J --> K{isFullAccess?}
K -->|permissionMode=undefined, yoloMode=undefined → false| L[No fullAccessArgs added]
K -->|permissionMode='full' → true| M[fullAccessArgs added]
style L fill:#ffcccc
style spawn fill:#fff3cc
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[User clicks toolbar button] --> B[ToolbarControls cycles permissionMode on activeTab]
B --> C{tab.permissionMode}
subgraph spawn["Spawn paths (missing permissionMode forwarding)"]
D[useInputProcessing] -->|readOnlyMode only| IPC
E[agentStore queue] -->|readOnlyMode only| IPC
F[useRemoteHandlers] -->|readOnlyMode only| IPC
end
subgraph correct["Correctly updated paths"]
G[cue-spawn-builder] -->|permissionMode: 'full'| IPC
H[context-groomer] -->|permissionMode: 'standard'| IPC
end
C -->|'full' or undefined| D
C -->|'standard'| D
C -->|'readonly'| D
IPC[IPC process handler] --> J[buildAgentArgs]
J --> K{isFullAccess?}
K -->|permissionMode=undefined, yoloMode=undefined → false| L[No fullAccessArgs added]
K -->|permissionMode='full' → true| M[fullAccessArgs added]
style L fill:#ffcccc
style spawn fill:#fff3cc
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }; | ||
| } | ||
| // In full-access mode, apply agent-specific env var overrides for full permissions | ||
| const isFullAccess = config.permissionMode === 'full' || config.yoloMode === true; |
There was a problem hiding this comment.
isFullAccess guard for env-var overrides vs. arg injection
buildAgentArgs only treats yoloMode: true as full-access when permissionMode is undefined — an explicit permissionMode: 'standard' suppresses it (see the test "permissionMode standard suppresses bypass args even when yoloMode: true is also passed"). But here config.yoloMode === true always wins regardless of permissionMode. Passing permissionMode: 'standard' + yoloMode: true therefore skips bypass flags (args path, correct) yet still applies fullAccessEnvOverrides (env-var path, wrong). The guard should be config.permissionMode === 'full' || (config.permissionMode === undefined && config.yoloMode === true) to stay consistent.
| theme: Theme; | ||
| isTerminalMode: boolean; | ||
| isReadOnlyMode: boolean; | ||
| isReadOnlyMode?: boolean; |
There was a problem hiding this comment.
isReadOnlyMode prop is declared but never consumed
The prop was removed from the destructure list but is still present in ToolbarControlsProps. Callers (e.g. MainPanelContent) still pass it, but the component now derives the display state from activeTab?.permissionMode directly. Leaving the dead prop in the interface creates confusion for future maintainers about where the control's source of truth lives.
| isReadOnlyMode?: boolean; | |
| // isReadOnlyMode removed — component now reads permissionMode directly from activeTab |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/ipc/handlers/process.ts (1)
461-477: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMatch env override precedence to
buildAgentArgs.
permissionModeis authoritative for args, but env overrides still read legacy booleans directly. A staleyoloMode: truecan apply full-access env overrides under explicitstandard/readonly, and explicitreadonlycan missreadOnlyEnvOverrides.🐛 Proposed fix
// In read-only mode, apply agent-specific env var overrides to strip // blanket permission grants (e.g., OpenCode's "*":"allow" YOLO config) let effectiveCustomEnvVars = configResolution.effectiveCustomEnvVars; - if (config.readOnlyMode && agent?.readOnlyEnvOverrides) { + const hasExplicitPermissionMode = config.permissionMode !== undefined; + const isReadOnly = + config.permissionMode === 'readonly' || + (!hasExplicitPermissionMode && config.readOnlyMode === true); + const isFullAccess = + config.permissionMode === 'full' || + (!hasExplicitPermissionMode && config.yoloMode === true); + + if (isReadOnly && agent?.readOnlyEnvOverrides) { effectiveCustomEnvVars = { ...(effectiveCustomEnvVars || {}), ...agent.readOnlyEnvOverrides, }; } // In full-access mode, apply agent-specific env var overrides for full permissions - const isFullAccess = config.permissionMode === 'full' || config.yoloMode === true; - if (isFullAccess && (agent as any)?.fullAccessEnvOverrides) { + if (isFullAccess && agent?.fullAccessEnvOverrides) { effectiveCustomEnvVars = { ...(effectiveCustomEnvVars || {}), - ...(agent as any).fullAccessEnvOverrides, + ...agent.fullAccessEnvOverrides, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/handlers/process.ts` around lines 461 - 477, Match the env override precedence in process IPC handling to the same authority rules used by buildAgentArgs. In the env merge logic around effectiveCustomEnvVars, stop using legacy yoloMode/readOnlyMode booleans directly and instead derive behavior from permissionMode first, so explicit standard/readonly/full-access settings win consistently. Update the branches that apply agent.readOnlyEnvOverrides and agent.fullAccessEnvOverrides to use the same permission-mode checks and fallback ordering as buildAgentArgs, preventing stale flags from overriding the current permission mode.src/main/cue/cue-spawn-builder.ts (1)
99-104: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
permissionMode: 'full'is incomplete here.This only upgrades the argv path. Cue still sanitizes
configResolution.effectiveCustomEnvVarswithout mergingagentDef.fullAccessEnvOverrides, so env-driven agents like OpenCode keep their standard permission config during Cue runs.process:spawnoverlays those full-access env overrides, so this path now diverges from the advertised “same pipeline” behavior.Suggested fix
const configResolution = applyAgentConfigOverrides(agentConfig, finalArgs, { agentConfigValues: (agentConfigValues ?? {}) as Record<string, any>, sessionCustomModel: customModel, sessionCustomEffort: customEffort, sessionCustomArgs: customArgs, sessionCustomEnvVars: customEnvVars, }); finalArgs = configResolution.args; + let envVarsForSpawn = configResolution.effectiveCustomEnvVars; + if (agentDef.fullAccessEnvOverrides) { + envVarsForSpawn = { + ...(envVarsForSpawn || {}), + ...agentDef.fullAccessEnvOverrides, + }; + } // Sanitize custom env vars BEFORE they reach the spawn environment. This // drops blocklisted names (PATH, HOME, USER, SHELL, LD_PRELOAD, // DYLD_INSERT_LIBRARIES, NODE_OPTIONS) and any name that does not match the // POSIX identifier regex. Keeping this in the spawn-builder means SSH // wrapping below inherits the sanitized map automatically via // `sshWrapConfig.customEnvVars`. const sanitizedResult = sanitizeCustomEnvVars( - configResolution.effectiveCustomEnvVars, + envVarsForSpawn, config.onLog );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/cue/cue-spawn-builder.ts` around lines 99 - 104, The Cue spawn path in cue-spawn-builder.ts is only passing permissionMode: 'full' through buildAgentArgs, but it still drops agentDef.fullAccessEnvOverrides when preparing env vars. Update the Cue pipeline to merge those full-access env overrides into configResolution.effectiveCustomEnvVars the same way process:spawn does, so agents like OpenCode get the same permissions in Cue runs; use the existing buildAgentArgs and agentDef.fullAccessEnvOverrides symbols to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/agents/definitions.ts`:
- Around line 376-378: Cue’s full-access spawn path is not applying the new
full-access environment overrides, so `permissionMode: 'full'` still uses the
standard env. Update the Cue builder in `cue-spawn-builder.ts` to merge
`fullAccessEnvOverrides` from the definitions into the spawned env when full
access is requested, rather than only sanitizing
`configResolution.effectiveCustomEnvVars`. Use the existing
`fullAccessEnvOverrides` symbol from `definitions.ts` and the Cue spawn builder
logic that sets `permissionMode: 'full'` to ensure the full-access env is
actually used.
In `@src/renderer/components/InputArea/components/ToolbarControls.tsx`:
- Around line 81-83: The derived pill state in ToolbarControls is missing the
legacy yoloMode fallback, so restored tabs with unset permissionMode can resolve
to the wrong mode. Update the currentPermissionMode calculation to account for
the legacy session state alongside activeTab.readOnlyMode, using the existing
activeTab/session values so the pill resolves to standard when yoloMode is false
and readOnlyMode is false. Keep the logic localized to ToolbarControls and
preserve the current fallback order for permissionMode first, then legacy state.
In `@src/renderer/hooks/input/useInputProcessing.ts`:
- Around line 1058-1061: The spawn path in useInputProcessing.ts is only passing
readOnlyMode into window.maestro.process.spawn(...), so permissionMode is not
preserved for batch sends. Update the spawn payload in the same flow that
computes isReadOnly to also include freshActiveTab?.permissionMode (or an
equivalent explicit flag) and wire it through the spawn call so Full Access tabs
can reach the new full-access arg/env branch instead of collapsing into the
standard path.
In `@src/renderer/hooks/remote/useRemoteHandlers.ts`:
- Around line 415-416: The remote spawn path in useRemoteHandlers is still
collapsing targetTab.permissionMode into isReadOnly and only forwarding
readOnlyMode, so full vs standard tabs lose their distinction. Update the
spawn/config assembly in this handler to include permissionMode explicitly
alongside readOnlyMode, and make sure the remote agent launch logic uses the
permissionMode value when dispatching web/mobile commands rather than deriving
everything from the boolean.
In `@src/renderer/stores/agentStore.ts`:
- Around line 372-375: Queued sends are still dropping the non-readonly
permission state because the enqueue/dequeue path only persists readOnlyMode.
Update the agentStore flow around isReadOnly and the subsequent spawn payload so
it carries targetTab.permissionMode as well, not just a boolean readonly flag.
Then, when replaying queued work, use the stored permissionMode to restore
full-access behavior for tabs that were originally full.
---
Outside diff comments:
In `@src/main/cue/cue-spawn-builder.ts`:
- Around line 99-104: The Cue spawn path in cue-spawn-builder.ts is only passing
permissionMode: 'full' through buildAgentArgs, but it still drops
agentDef.fullAccessEnvOverrides when preparing env vars. Update the Cue pipeline
to merge those full-access env overrides into
configResolution.effectiveCustomEnvVars the same way process:spawn does, so
agents like OpenCode get the same permissions in Cue runs; use the existing
buildAgentArgs and agentDef.fullAccessEnvOverrides symbols to locate the change.
In `@src/main/ipc/handlers/process.ts`:
- Around line 461-477: Match the env override precedence in process IPC handling
to the same authority rules used by buildAgentArgs. In the env merge logic
around effectiveCustomEnvVars, stop using legacy yoloMode/readOnlyMode booleans
directly and instead derive behavior from permissionMode first, so explicit
standard/readonly/full-access settings win consistently. Update the branches
that apply agent.readOnlyEnvOverrides and agent.fullAccessEnvOverrides to use
the same permission-mode checks and fallback ordering as buildAgentArgs,
preventing stale flags from overriding the current permission mode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e52ec763-bae3-4732-8cd3-7a2366fe61b0
📒 Files selected for processing (27)
src/__tests__/main/agents/definitions.test.tssrc/__tests__/main/agents/detector.test.tssrc/__tests__/main/cue/cue-spawn-builder.test.tssrc/__tests__/main/utils/agent-args.test.tssrc/__tests__/main/utils/context-groomer.test.tssrc/__tests__/renderer/components/InputArea.test.tsxsrc/__tests__/renderer/components/InputArea/components/ToolbarControls.test.tsxsrc/__tests__/shared/agentMetadata.test.tssrc/main/agents/definitions.tssrc/main/cue/cue-spawn-builder.tssrc/main/ipc/handlers/process.tssrc/main/preload/process.tssrc/main/utils/agent-args.tssrc/main/utils/context-groomer.tssrc/renderer/components/InputArea/components/ToolbarControls.tsxsrc/renderer/components/MainPanel/MainPanelContent.tsxsrc/renderer/components/WelcomeContent.tsxsrc/renderer/components/Wizard/screens/DirectorySelectionScreen/components/DirectorySelectionHeader.tsxsrc/renderer/global.d.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/remote/useRemoteHandlers.tssrc/renderer/stores/agentStore.tssrc/renderer/types/index.tssrc/renderer/utils/agentArgs.tssrc/renderer/utils/sessionHelpers.tssrc/shared/agentMetadata.tssrc/shared/types.ts
| const activeTab = session.aiTabs?.find((t) => t.id === session.activeTabId); | ||
| const currentPermissionMode: 'full' | 'standard' | 'readonly' = | ||
| activeTab?.permissionMode ?? (activeTab?.readOnlyMode ? 'readonly' : 'full'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include the legacy yoloMode fallback in the derived pill state.
This only falls back to readOnlyMode. For restored tabs where permissionMode is still unset and legacy state is { yoloMode: false, readOnlyMode: false }, the toolbar will render Full Access even though the PR contract says that state should resolve to standard. That leaves the pill out of sync and makes the next click cycle from the wrong mode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/InputArea/components/ToolbarControls.tsx` around
lines 81 - 83, The derived pill state in ToolbarControls is missing the legacy
yoloMode fallback, so restored tabs with unset permissionMode can resolve to the
wrong mode. Update the currentPermissionMode calculation to account for the
legacy session state alongside activeTab.readOnlyMode, using the existing
activeTab/session values so the pill resolves to standard when yoloMode is false
and readOnlyMode is false. Keep the logic localized to ToolbarControls and
preserve the current fallback order for permissionMode first, then legacy state.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeec799dac
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'stream-json', | ||
| '--dangerously-skip-permissions', | ||
| ], | ||
| args: ['--print', '--verbose', '--output-format', 'stream-json'], |
There was a problem hiding this comment.
This removes Claude's bypass flag from the default args, but several non-mode-aware spawn paths still do not pass permissionMode or yoloMode. For example Auto Run still sends only readOnlyMode: false in src/renderer/hooks/agent/useAgentExecution.ts:572, and the normal input path only forwards readOnlyMode at src/renderer/hooks/input/useInputProcessing.ts:1151, so buildAgentArgs() never appends fullAccessArgs. Default Full Access Claude runs now start without --dangerously-skip-permissions, which can stall or deny tool calls in noninteractive runs.
Useful? React with 👍 / 👎.
| if (!isReadOnly) { | ||
| finalArgs = [...finalArgs, ...agent.batchModeArgs]; |
There was a problem hiding this comment.
When a caller explicitly selects permissionMode: 'standard', isReadOnly is false here, so the agent's batchModeArgs are still appended. Several definitions put the bypass itself in batchModeArgs (--dangerously-bypass-approvals-and-sandbox, --skip-permissions-unsafe, --allow-all), so Standard mode for Codex, Factory Droid, and Copilot-CLI still grants full permissions instead of using the provider's default model.
Useful? React with 👍 / 👎.
| readOnlyMode, | ||
| modelId: undefined, | ||
| yoloMode: false, | ||
| permissionMode: 'standard' as const, |
There was a problem hiding this comment.
Passing an explicit permissionMode: 'standard' makes buildAgentArgs() ignore the readOnlyMode boolean above, because permissionMode takes precedence. Callers such as group chat and Director's Notes summaries pass readOnlyMode: true for read-only grooming, but after this change they no longer get readOnlyArgs like Claude plan mode or Codex read-only sandbox.
Useful? React with 👍 / 👎.
| yoloMode: true, // Cue runs always use YOLO mode like Auto Run | ||
| permissionMode: 'full' as const, |
There was a problem hiding this comment.
Cue marks its runs as full access here, but the builder only carries configResolution.effectiveCustomEnvVars into the spawn and never layers agentDef.fullAccessEnvOverrides. After OpenCode's default env was changed to the Standard config, Cue OpenCode runs no longer receive the full-access OPENCODE_CONFIG_CONTENT, so unattended Cue tasks can be denied or hang on permissions despite the explicit full mode.
Useful? React with 👍 / 👎.
| permissionMode: nextMode, | ||
| readOnlyMode: nextMode === 'readonly', |
There was a problem hiding this comment.
Once the toolbar writes permissionMode, the other read-only entry points still only flip readOnlyMode (keyboard shortcut, Quick Actions, Prompt Composer, and tab store). If a user cycles the toolbar to Readonly and then uses one of those controls to turn it off, this field stays readonly, so later sends still run as read-only and the toolbar continues to show Readonly despite readOnlyMode being false.
Useful? React with 👍 / 👎.
| fullAccessEnvOverrides: { | ||
| OPENCODE_CONFIG_CONTENT: | ||
| '{"permission":{"*":"allow","external_directory":"allow","question":"deny"},"tools":{"question":false}}', |
There was a problem hiding this comment.
This new full-access override is only applied in the desktop process:spawn handler, while maestro-cli send still layers defaultEnvVars, batchModeEnvVars, user vars, and read-only overrides only. Since OpenCode's default env is now the Standard config, default write-mode CLI sends with OpenCode no longer get the permission:{"*":"allow","external_directory":"allow"} config and can be denied or hang in unattended runs.
Useful? React with 👍 / 👎.
| readOnlyMode: config.readOnlyMode, | ||
| modelId: config.modelId, | ||
| yoloMode: config.yoloMode, | ||
| permissionMode: config.permissionMode, |
There was a problem hiding this comment.
Forwarding permissionMode here only changes argument selection; the env override block below still checks the legacy config.readOnlyMode boolean. A caller using the new API with permissionMode: 'readonly' and no legacy flag gets read-only CLI args but misses readOnlyEnvOverrides, so OpenCode can keep the Standard OPENCODE_CONFIG_CONTENT instead of the no-prompt read-only config.
Useful? React with 👍 / 👎.
| effectiveCustomEnvVars = { | ||
| ...(effectiveCustomEnvVars || {}), | ||
| ...(agent as any).fullAccessEnvOverrides, |
There was a problem hiding this comment.
These full-access overrides are layered after effectiveCustomEnvVars, which already contains session and agent custom env vars. In Full Access OpenCode, a user-provided OPENCODE_CONFIG_CONTENT is overwritten by Maestro's blanket config, so users cannot customize or narrow the permission config even though custom env vars are supposed to override agent defaults.
Useful? React with 👍 / 👎.
|
Thanks for this, @athal7 - the 3-way permission model is a genuinely nice cleanup over the binary yolo/read-only toggle, and the Before we merge, there's one blocking regression that both Greptile and CodeRabbit flagged and that I've confirmed against the code:
This PR moves the bypass flags (
So inside the IPC handler both The fix is to forward permissionMode: freshActiveTab?.permissionMode ?? (freshActiveTab?.readOnlyMode ? 'readonly' : 'full'),A few smaller items worth addressing while you're in here (non-blocking, your call):
No merge conflicts on this branch, so once the spawn forwarding is in place this should be good to go. Thanks again for the contribution! |
Align the full-access env-override guard in the process IPC handler with the args-injection guard in agent-args.ts, so an explicit permissionMode: 'standard' correctly suppresses fullAccessEnvOverrides even when a legacy yoloMode: true is also passed. Also drop the now- dead isReadOnlyMode/onToggleTabReadOnlyMode props from ToolbarControlsProps and their call site now that the toggle reads permissionMode directly from the active tab. Co-Authored-By: anthropic/claude-sonnet-4-6 <noreply@opencode.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c469ad993
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| defaultEnvVars: { | ||
| OPENCODE_CONFIG_CONTENT: '{"permission":{"question":"deny"},"tools":{"question":false}}', |
There was a problem hiding this comment.
For desktop sends where the user leaves the new toolbar at its default Full Access state, the renderer never sends permissionMode: 'full' or yoloMode to process:spawn (checked normal input at src/renderer/hooks/input/useInputProcessing.ts:1151, queued sends at src/renderer/stores/agentStore.ts:433, and remote sends at src/renderer/hooks/remote/useRemoteHandlers.ts:519). After this default was changed to the Standard OpenCode config, those spawns only receive {"permission":{"question":"deny"}}, so fullAccessEnvOverrides is not layered and OpenCode can deny or block write/external-directory actions despite the UI saying Full Access. Please either keep the full-access env as the default until callers pass the mode, or thread the tab permission mode through these spawn calls.
Useful? React with 👍 / 👎.
| // The question tool waits for stdin input which hangs batch mode. | ||
| // Users can override by setting customEnvVars in agent config. | ||
| defaultEnvVars: { | ||
| OPENCODE_CONFIG_CONTENT: '{"permission":{"question":"deny"},"tools":{"question":false}}', |
There was a problem hiding this comment.
After this line makes OpenCode's default env the Standard config, direct read-only spawns that bypass process:spawn no longer receive readOnlyEnvOverrides. For example, tab naming builds read-only args in src/main/ipc/handlers/tabNaming.ts:155-160 but then passes only configResolution.effectiveCustomEnvVars at src/main/ipc/handlers/tabNaming.ts:195-198, so OpenCode tab naming runs with --agent plan plus {"permission":{"question":"deny"}} instead of the blanket allow config that the read-only override comments say is needed to avoid permission prompts. This can make automatic naming fail or hang for OpenCode tabs.
Useful? React with 👍 / 👎.
Align env-override precedence in process.ts with the same permissionMode-first, legacy-boolean-fallback derivation already used by the arg builder, so a stale yoloMode/readOnlyMode can no longer win over an explicit permissionMode. Merge agentDef.fullAccessEnvOverrides into Cue's spawn env so env-driven full-access agents (e.g. OpenCode) actually get full permissions during Cue runs, not just agents whose bypass is argv-flag-based. Thread permissionMode through the four remaining renderer spawn call sites (AutoRun/batch send, remote/web command dispatch, and both agentStore spawn paths) that previously only sent readOnlyMode, so full-access tabs no longer silently regress to standard mode when dispatched through these paths. Co-Authored-By: anthropic/claude-sonnet-4-6 <noreply@opencode.ai>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/hooks/input/useInputProcessing.ts (1)
1058-1061: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winForward the effective permission mode when read-only is forced.
Line 1058 correctly makes Auto Run/no-worktree and legacy read-only tabs force
isReadOnly, but Line 1152 still sends the tab’s rawpermissionMode. If that raw value isfullorstandard, the main process can treat the spawn as writable despitereadOnlyMode: true. Derive the payload mode fromisReadOnly.🐛 Proposed fix
const isReadOnly = isAutoRunReadOnly || freshActiveTab?.readOnlyMode === true || freshActiveTab?.permissionMode === 'readonly'; +const effectivePermissionMode = isReadOnly ? 'readonly' : freshActiveTab?.permissionMode;readOnlyMode: isReadOnly, -permissionMode: freshActiveTab?.permissionMode, +permissionMode: effectivePermissionMode,Also applies to: 1151-1152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/input/useInputProcessing.ts` around lines 1058 - 1061, The spawn payload in useInputProcessing should forward the effective permission mode, not the tab’s raw permissionMode, because read-only can be forced by isAutoRunReadOnly or readOnlyMode. Update the logic around the isReadOnly calculation and the payload construction so the mode sent to the main process is derived from isReadOnly (read-only when true, otherwise the existing tab mode). Use the existing freshActiveTab, isReadOnly, and permissionMode handling in useInputProcessing to locate the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/renderer/hooks/input/useInputProcessing.ts`:
- Around line 1058-1061: The spawn payload in useInputProcessing should forward
the effective permission mode, not the tab’s raw permissionMode, because
read-only can be forced by isAutoRunReadOnly or readOnlyMode. Update the logic
around the isReadOnly calculation and the payload construction so the mode sent
to the main process is derived from isReadOnly (read-only when true, otherwise
the existing tab mode). Use the existing freshActiveTab, isReadOnly, and
permissionMode handling in useInputProcessing to locate the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4f33eb28-0a8f-49dc-8d0a-ede59696f698
📒 Files selected for processing (6)
src/__tests__/main/cue/cue-spawn-builder.test.tssrc/main/cue/cue-spawn-builder.tssrc/main/ipc/handlers/process.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/remote/useRemoteHandlers.tssrc/renderer/stores/agentStore.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/hooks/remote/useRemoteHandlers.ts
- src/renderer/stores/agentStore.ts
- src/main/ipc/handlers/process.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c84acb853
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| appendSystemPrompt, | ||
| agentSessionId: tabAgentSessionId ?? undefined, | ||
| readOnlyMode: isReadOnly, | ||
| permissionMode: targetTab.permissionMode, |
There was a problem hiding this comment.
When processing queued messages, this reads targetTab.permissionMode at dispatch time, but QueuedItem only snapshots the legacy readOnlyMode when the item is enqueued. If a user queues a prompt while the tab is in Standard mode and then switches the tab back to Full Access before the current run exits, the queued prompt is spawned with full bypass instead of the Standard mode selected for that queued work. Please store permissionMode on queued items and pass that here, falling back to the tab only for legacy queue entries; the slash-command spawn path below has the same issue.
Useful? React with 👍 / 👎.
The renderer spawn call sites sent the tab's raw permissionMode alongside readOnlyMode, but readOnlyMode can be forced to true by conditions independent of the tab's own permissionMode field (Auto Run without a worktree, a queued item's own readOnlyMode flag). That produced an internally inconsistent spawn payload (readOnlyMode: true, permissionMode: 'full') where the main process could honor the raw permissionMode and treat the spawn as writable despite the forced read-only intent. Derive effectivePermissionMode as 'readonly' whenever the forced isReadOnly is true, otherwise fall back to the tab's own permissionMode, at all four renderer spawn call sites (AutoRun/batch send, remote/web command dispatch, and both agentStore spawn paths). Co-Authored-By: anthropic/claude-sonnet-4-6 <noreply@opencode.ai>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/__tests__/renderer/hooks/useInputProcessing.test.ts (1)
1412-1444: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a complementary "full access preserved" test.
This test only covers the forced-readonly override path. Given the PR's core regression was
permissionModenot being forwarded at all (losing Full Access bypass), a sibling test assertingspawnCall.permissionMode === 'full'when Auto Run is not forcing readonly would directly guard against regressing the original bug.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/renderer/hooks/useInputProcessing.test.ts` around lines 1412 - 1444, Add a sibling test in useInputProcessing.test that covers the non-forced path for useInputProcessing/processInput. The new test should create a tab/session where Auto Run does not force readonly, then verify window.maestro.process.spawn is called with permissionMode preserved as "full". Reuse the existing helpers like createMockTab, createMockSession, createDeps, and processInput so the test directly guards the forwarding behavior that regressed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/__tests__/renderer/hooks/useInputProcessing.test.ts`:
- Around line 1412-1444: Add a sibling test in useInputProcessing.test that
covers the non-forced path for useInputProcessing/processInput. The new test
should create a tab/session where Auto Run does not force readonly, then verify
window.maestro.process.spawn is called with permissionMode preserved as "full".
Reuse the existing helpers like createMockTab, createMockSession, createDeps,
and processInput so the test directly guards the forwarding behavior that
regressed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3e6c228-08c6-4c31-87e9-b6b74c21c378
📒 Files selected for processing (6)
src/__tests__/renderer/hooks/useInputProcessing.test.tssrc/__tests__/renderer/hooks/useRemoteHandlers.test.tssrc/__tests__/renderer/stores/agentStore.test.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/remote/useRemoteHandlers.tssrc/renderer/stores/agentStore.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/renderer/stores/agentStore.ts
- src/renderer/hooks/remote/useRemoteHandlers.ts
- src/renderer/hooks/input/useInputProcessing.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac3adcb28f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // env-only full-access agents silently run at standard permissions in Cue. | ||
| let envVarsForSpawn = configResolution.effectiveCustomEnvVars; | ||
| if (agentDef.fullAccessEnvOverrides) { | ||
| envVarsForSpawn = { ...(envVarsForSpawn || {}), ...agentDef.fullAccessEnvOverrides }; |
There was a problem hiding this comment.
When a Cue run supplies customEnvVars with the same key as fullAccessEnvOverrides (for example an OpenCode OPENCODE_CONFIG_CONTENT that narrows or customizes permissions), configResolution.effectiveCustomEnvVars already contains the user value, but this spread overwrites it with Maestro's blanket full-access config before sanitizing and spawning. That makes Cue ignore the user's per-run env override; layer the full-access defaults before user/session env instead.
Useful? React with 👍 / 👎.
Add a sibling test to the forced-readonly effectivePermissionMode test: asserts a tab with permissionMode 'full' sends permissionMode: 'full' to the spawn config when Auto Run is not forcing read-only. Directly guards against regressing the original bug this PR fixes (permissionMode not forwarded at all, silently losing Full Access bypass). Co-Authored-By: anthropic/claude-sonnet-4-6 <noreply@opencode.ai>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7852d02bb7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // The question tool waits for stdin input which hangs batch mode. | ||
| // Users can override by setting customEnvVars in agent config. | ||
| defaultEnvVars: { | ||
| OPENCODE_CONFIG_CONTENT: '{"permission":{"question":"deny"},"tools":{"question":false}}', |
There was a problem hiding this comment.
When an OpenCode group chat participant is run in write mode (readOnly is false), group-chat-router.ts builds args with only the legacy readOnlyMode flag and spawnGroupChatAgent.ts passes configResolution.effectiveCustomEnvVars directly, so fullAccessEnvOverrides is never layered. Since this default is now the Standard config rather than the allow-all config, OpenCode participants and recovery spawns run without the full read/write permissions promised by the group chat prompt and can deny or block file edits.
Useful? React with 👍 / 👎.
|
Hey @athal7 I'm going to have @pedramamini or @reachrazamair take a look at this one. If memory recalls correctly the reason why we use yolo mode is most clis don't expose a nice way to permission handle when spawning programatically instead of via TUI. That leaves us in a spot where if you hit something that's blocked and you want the agent to do it there's not a way to approve that. Please correct me if I'm wrong here guys |
|
this pr needs more work, putting back in draft while i work on it |
3 participants
Replaces the binary yolo-mode/read-only toggle with
permissionMode: 'full' | 'standard' | 'readonly'.fullpreserves current bypass behavior (renamed from yolo),standardis new and runs agents with no bypass flags so they use their own default permission model,readonlyis the existing plan/exploration mode, unchanged.Summary by CodeRabbit
permissionModeoverride.