fix: enable opencode auto approve#2149
Conversation
Greptile SummaryThis PR fixes auto-approve for the OpenCode provider, which previously had no effect because the provider registry defines no
Confidence Score: 4/5Safe to merge; the change is small, well-targeted, and tested for the main cases. The auto-approve env injection is applied after the user's custom provider env loop, meaning a user-defined OPENCODE_PERMISSION value would be silently overwritten when autoApprove is enabled. For most users this is harmless, but the ordering could surprise anyone who has both configured. Both call sites are updated symmetrically and the logic is otherwise straightforward. src/main/core/conversations/impl/provider-env.ts — the ordering of the custom-env loop relative to the auto-approve injection determines which value wins for OPENCODE_PERMISSION.
|
| Filename | Overview |
|---|---|
| src/main/core/conversations/impl/provider-env.ts | Core change: adds optional options parameter so resolveProviderEnv can inject OPENCODE_PERMISSION={"*":"allow"} when auto-approve is requested for the opencode provider; also refactors the early-return guard to allow building a non-empty env even when providerConfig is undefined. |
| src/main/core/conversations/impl/provider-env.test.ts | Adds two new test cases: opencode with auto-approve sets the env var, and non-opencode providers with auto-approve do not. |
| src/main/core/conversations/impl/local-conversation.ts | Passes providerId and autoApprove from the conversation object into resolveProviderEnv; straightforward call-site update. |
| src/main/core/conversations/impl/ssh-conversation.ts | Mirrors the same call-site change as local-conversation.ts for the SSH conversation provider. |
Sequence Diagram
sequenceDiagram
participant Caller as LocalConversation / SshConversation
participant Fn as resolveProviderEnv()
participant Env as Process Environment
Caller->>Fn: "resolveProviderEnv(providerConfig, { providerId, autoApprove })"
Fn->>Fn: iterate providerConfig.env, filter valid names
alt "providerId === 'opencode' && autoApprove === true"
Fn->>Fn: "env.OPENCODE_PERMISSION = '{"*":"allow"}'"
end
Fn-->>Caller: "Record<string, string> | undefined"
Caller->>Env: spawn opencode with merged env vars
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/main/core/conversations/impl/provider-env.ts:13-19
If a user has set a custom `OPENCODE_PERMISSION` value in their provider env config (e.g. a partially-restrictive policy), enabling `autoApprove` will silently overwrite it because the auto-approve assignment runs after the loop. The user's hand-crafted permission object is discarded without any indication. Merging the user config in first and only writing the allow-all key when it isn't already present would preserve explicit user intent, or at least applying the override before the loop would make it easy to detect the conflict.
```suggestion
if (options.providerId === 'opencode' && options.autoApprove) {
env.OPENCODE_PERMISSION = OPENCODE_ALLOW_ALL_PERMISSIONS;
}
for (const [key, value] of Object.entries(providerConfig?.env ?? {})) {
if (ENV_NAME_PATTERN.test(key)) env[key] = value;
}
```
### Issue 2 of 2
src/main/core/conversations/impl/provider-env.test.ts:26-35
**Missing edge-case test coverage**
Two scenarios that fall out of the new logic aren't covered: (1) `providerId: 'opencode'` with `autoApprove: false` — should not set `OPENCODE_PERMISSION`; (2) opencode with `autoApprove: true` AND a custom `OPENCODE_PERMISSION` value already in `providerConfig.env` — whichever wins the override should be intentional and verifiable.
Reviews (1): Last reviewed commit: "fix: enable opencode auto approve env" | Re-trigger Greptile
arnestrickmann
left a comment
There was a problem hiding this comment.
thanks for the fix!
summary
before auto approve did nothing for opencode
https://opencode.ai/docs/permissions/