Repository navigation
fix(opencode): preserve global config discovery - #21854
Conversation
57f3db7 to
2c6ca89
Compare
📝 WalkthroughWalkthroughThe change resolves OpenCode’s canonical configuration directory from Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to An existing plugin symlink can cause user-file loss, while stale Orca environment state can prevent OpenCode from discovering the newly installed plugin and global configuration. Both should be fixed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description clearly explains the problem, change, and validation, but it omits most required template sections, including ELI5, What Changed, Why, formal Linked Issue, Visual Proof or N/A, Testing checkboxes, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist. Resolution Complete the required template sections. Add the ELI5, What Changed, and Why sections; provide a formal Linked Issue entry such as Fixes
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: b6a6271f-a7c6-42f4-b61a-770e814ea809
📒 Files selected for processing (9)
src/main/opencode/hook-service.test.tssrc/main/opencode/hook-service.tssrc/relay/omp-status-extension.tssrc/relay/opencode-canonical-config.tssrc/relay/plugin-overlay.test.tssrc/relay/plugin-overlay.tssrc/relay/relay-agent-hook-runtime.tssrc/shared/opencode-config-directory.tstests/e2e/ssh-localhost.spec.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| const managedConfigDir = this.getSharedConfigDir() | ||
| if (!existingConfigDir || existingConfigDir === managedConfigDir) { | ||
| try { | ||
| this.writePluginToConfigDir(resolveOpenCodeConfigDirectory()) | ||
| return {} | ||
| } catch { | ||
| return {} | ||
| } | ||
| return { OPENCODE_CONFIG_DIR: configDir } | ||
| } | ||
|
|
||
| // Why: don't mkdir the user's (possibly typoed) path — that's the config-replacement failure mode in docs/opencode-config-dir-collision.md; let OpenCode surface it. | ||
| if (!existsSync(existingConfigDir)) { | ||
| return { OPENCODE_CONFIG_DIR: existingConfigDir } | ||
| } | ||
|
|
||
| const overlayDir = this.getSourceOverlayDir(existingConfigDir) | ||
|
|
||
| try { | ||
| mkdirSync(overlayDir, { recursive: true }) | ||
| this.mirrorUserConfig(existingConfigDir, overlayDir) | ||
| this.writePluginIntoOverlay(overlayDir) | ||
| return { OPENCODE_CONFIG_DIR: overlayDir } | ||
| } catch { | ||
| // Why: best-effort — symlink creation needs Windows developer mode (else EPERM) and userData may be read-only; preserve the user's config over dropping their auth/models/keymap. | ||
| return { OPENCODE_CONFIG_DIR: existingConfigDir } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '35,115p' src/main/ipc/pty/host-env/assembly.ts
rg -n 'delete .*OPENCODE_CONFIG_DIR|OPENCODE_CONFIG_DIR.*undefined|unset|Object.assign\(baseEnv' src/main src/relay
sed -n '115,165p' src/main/opencode/hook-service.tsRepository: stablyai/orca
Length of output: 42003
🏁 Script executed:
sed -n '80,175p' src/main/ipc/pty/host-env/assembly.ts
sed -n '1,130p' src/main/ipc/pty-spawn-env-agent-overlays.test.ts
sed -n '200,250p' src/main/ipc/pty-spawn-env-agent-overlays.test.ts
sed -n '1,100p' src/main/ipc/pty/pty-ipc-process-env-scope.ts 2>/dev/null || true
rg -n -C 4 'delete baseEnv\.OPENCODE_CONFIG_DIR|delete baseEnv\.ORCA_OPENCODE_CONFIG_DIR|OPENCODE_CONFIG_DIR: undefined|buildPtyEnv\(' src/main/ipc src/main/opencode src/main --glob '*.ts' --glob '!**/__fixtures__/**'Repository: stablyai/orca
Length of output: 41906
🏁 Script executed:
sed -n '1,90p' src/main/ipc/pty/host-env/assembly.ts
rg -n -C 12 'function restoreOrStripOverlayEnv|const restoreOrStripOverlayEnv|restoreOrStripOverlayEnv|resolveOpenCodeSourceConfigDir' src/main/ipc/pty/host-env src/mainRepository: stablyai/orca
Length of output: 31003
🏁 Script executed:
sed -n '145,205p' src/main/ipc/pty/host-env/pi-agent.tsRepository: stablyai/orca
Length of output: 2383
Strip inherited OpenCode overlay state before resolving the source directory. When OPENCODE_CONFIG_DIR equals ORCA_OPENCODE_CONFIG_DIR and no source marker exists, resolveOpenCodeSourceConfigDir() returns undefined. buildPtyEnv() then returns {}, but the managed value remains in baseEnv and can hide the plugin installed in the canonical directory. Call restoreOrStripOverlayEnv() for the OpenCode variables before resolving preexistingOpenCodeConfigDir in the enabled-hooks path. This deletes only Orca-owned values and preserves a recorded user source. Do not set OPENCODE_CONFIG_DIR to the canonical directory; ordinary launches must leave it unset for OpenCode global discovery.
| } | ||
| return configDir | ||
| mkdirSync(pluginsDir, { recursive: true }) | ||
| writeFileSync(join(pluginsDir, this.pluginFileName), this.pluginSource()) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '100,175p' src/main/opencode/hook-service.ts
sed -n '235,285p' src/main/opencode/hook-service.ts
rg -n 'orca-opencode-status|pluginFileName|writePluginIntoOverlay|unlinkSync|clearPty' src/main/opencode src/relayRepository: stablyai/orca
Length of output: 13888
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- hook-service imports and lifecycle ---'
sed -n '1,155p' src/main/opencode/hook-service.ts
printf '%s\n' '--- hook-service mirror/write section ---'
sed -n '155,275p' src/main/opencode/hook-service.ts
printf '%s\n' '--- hook-service tests: canonical lifecycle and overlay ownership ---'
sed -n '235,365p' src/main/opencode/hook-service.test.ts
sed -n '365,490p' src/main/opencode/hook-service.test.ts
sed -n '490,575p' src/main/opencode/hook-service.test.ts
printf '%s\n' '--- canonical writer and overlay implementation ---'
cat -n src/relay/opencode-canonical-config.ts
sed -n '150,275p' src/relay/plugin-overlay.ts
printf '%s\n' '--- config directory bindings ---'
rg -n -A8 -B8 'resolveOpenCodeConfigDirectory|OPENCODE_CONFIG_DIR|OPENCODE_SHARED_CONFIG_DIR|ORCA_OPENCODE_PLUGIN_FILE' src/main src/relayRepository: stablyai/orca
Length of output: 42714
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/architecture
Length of output: 36813
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- relay canonical helper ---'
cat -n src/relay/opencode-canonical-config.ts
printf '%s\n' '--- canonical helper callers ---'
rg -n -A10 -B10 'install.*OpenCode|installOpenCode|opencode-canonical-config|writePluginToConfigDir|resolveOpenCodeConfigDirectory\\(' src
printf '%s\n' '--- config resolver ---'
cat -n src/shared/opencode-config-directory.ts
printf '%s\n' '--- target change summary ---'
git diff --stat
git diff -- src/main/opencode/hook-service.ts src/main/opencode/hook-service.test.ts src/relay/opencode-canonical-config.tsRepository: stablyai/orca
Length of output: 2215
🏁 Script executed:
#!/bin/bash
printf '%s\n' '--- relay canonical helper references ---'
rg -n -F -A8 -B8 'installOpenCodePluginInCanonicalConfig' src || true
printf '%s\n' '--- service writer references ---'
rg -n -F -A8 -B8 'writePluginToConfigDir' src || true
printf '%s\n' '--- config resolver ---'
cat -n src/shared/opencode-config-directory.ts
printf '%s\n' '--- relevant diff summary ---'
git diff --stat -- src/main/opencode/hook-service.ts src/main/opencode/hook-service.test.ts src/relay/opencode-canonical-config.tsRepository: stablyai/orca
Length of output: 6931
Unlink the canonical plugin path before writing.
During an ordinary launch, writePluginToConfigDir writes the fixed plugin path. If that path is an existing symlink, writeFileSync follows it and replaces its writable target. Unlink the path first, as writePluginIntoOverlay and the relay canonical installer already do.
Proposed fix
private writePluginToConfigDir(configDir: string): void {
const pluginsDir = join(configDir, 'plugins')
mkdirSync(pluginsDir, { recursive: true })
- writeFileSync(join(pluginsDir, this.pluginFileName), this.pluginSource())
+ const pluginPath = join(pluginsDir, this.pluginFileName)
+ try {
+ unlinkSync(pluginPath)
+ } catch {
+ // The plugin file may not exist on first install.
+ }
+ writeFileSync(pluginPath, this.pluginSource())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| writeFileSync(join(pluginsDir, this.pluginFileName), this.pluginSource()) | |
| const pluginPath = join(pluginsDir, this.pluginFileName) | |
| try { | |
| unlinkSync(pluginPath) | |
| } catch { | |
| // The plugin file may not exist on first install. | |
| } | |
| writeFileSync(pluginPath, this.pluginSource()) |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two paths still resolve OpenCode's global config to an Orca-owned directory: WSL launches keep setting OPENCODE_CONFIG_DIR to a guest overlay, and the new canonical-install branches don't clear an inherited OPENCODE_CONFIG_DIR/ORCA_OPENCODE_CONFIG_DIR, so a nested Orca terminal re-exports the Orca dir after the user's rc files. Details below and inline.
Reviewed changes
- Local plugin install moves to canonical config —
OpenCodeHookService.buildPtyEnvwritesorca-opencode-status.jsintoresolveOpenCodeConfigDirectory()and returns no env override for the no-user-dir and old-shared-dir cases, instead of pointingOPENCODE_CONFIG_DIRatuserData/opencode-hooks/shared. - Shared directory resolver — new
src/shared/opencode-config-directory.tsmirroring OpenCode'sxdgConfig + "opencode". - SSH relay canonical install — new
opencode-canonical-config.ts(installOpenCodePluginInCanonicalConfig,isRelayOpenCodeOverlayPath);RelayAgentHookRuntime.buildPluginEnvironmentinstalls into the canonical root unless an explicit, non-Orca source dir must be mirrored. - OMP status extension extracted —
writeRelayOmpStatusExtensionmoved out ofPluginOverlayManager; behavior unchanged. - Tests — local assertions moved from the userData shared dir to the canonical dir; relay canonical-install test added.
⚠️ WSL launches still override OPENCODE_CONFIG_DIR with an Orca-owned overlay
The PR's stated goal is that relay launches install into the remote canonical config root and leave discovery untouched. The WSL guest path is unchanged: the host still points OPENCODE_CONFIG_DIR at ~/.orca-relay/opencode-overlays/<hash> inside the distro, so the same hijack issue #19708 reports persists there.
Technical details
# WSL still hijacks OpenCode's global config
## Affected sites
- `src/main/ipc/pty/host-env/assembly.ts:144-155` — for `opts.isWsl`, sets `baseEnv.OPENCODE_CONFIG_DIR` and `baseEnv.ORCA_OPENCODE_CONFIG_DIR` to `wslHookRelayManager.getOpenCodeOverlayDir(distro, openCodeAgent)`. This runs *after* the newly-fixed local `buildPtyEnv` call at `assembly.ts:94`, so it overwrites the `{}` the PR now returns.
- `src/relay/wsl-install-plugins-handler.ts:86` — the guest handler still materializes a plugin-only overlay via `materializeOpenCode`; it never installs into the guest's canonical `$XDG_CONFIG_HOME/opencode` root. The only canonical installer (`installOpenCodePluginInCanonicalConfig`) has exactly one caller, the SSH runtime at `relay-agent-hook-runtime.ts:105`.
- `src/relay/wsl-install-plugins-handler.ts:29-33` — the header comment asserts OpenCode "APPENDS `OPENCODE_CONFIG_DIR` ... the XDG default is always read too". OpenCode's current source (`packages/core/src/global.ts`) resolves `config: Flag.OPENCODE_CONFIG_DIR ?? Path.config`, i.e. it REPLACES the global config dir. That premise is why the XDG default is deliberately not mirrored, and it is the premise issue #19708 falsifies.
## Required outcome
- WSL launches must stop setting `OPENCODE_CONFIG_DIR` to an Orca overlay. The guest handler should install the plugin into the guest's canonical config root and the host branch at `assembly.ts:144-155` should stop injecting the overlay (or the overlay mechanism must be retained only where genuinely needed).
- Correct the stale comment at `wsl-install-plugins-handler.ts:29-33` so the next reader doesn't re-derive the overlay from it.
## Open questions for the human
- Is WSL intentionally out of scope for this fix, with a separate follow-up? If so, note it on #19708 so WSL users aren't told the hijack is resolved.ℹ️ The canonical plugin file is user-global, shared across Orca builds, and never removed
~/.config/opencode/plugins/orca-opencode-status.js is now written by every Orca build into the user's config and is never cleaned up — clearPty is a deliberate no-op and nothing else deletes it. Stable and dev builds will overwrite each other's plugin, and the file outlives an Orca uninstall while still being loaded by every OpenCode session.
Technical details
# Shared canonical plugin path
## Affected sites
- `src/main/opencode/hook-service.ts:259-263` (`writePluginToConfigDir`) and `src/relay/opencode-canonical-config.ts:9-34` — both write to a single user-global filename.
- `src/main/opencode/hook-service.ts:111-113` — `clearPty` intentionally does not remove it.
## Context
The repo deliberately scopes the Codex home per-PTY "so dev/prod Orcas don't share hooks through `~/.codex`" (`src/main/ipc/pty/host-env/assembly.ts:225`); the old OpenCode design was userData-scoped for the same reason. Because the plugin resolves hook coordinates at runtime from `ORCA_AGENT_HOOK_ENDPOINT`/the endpoint file, a stale or foreign-build plugin is not fatal today — but the source version can silently mismatch the running Orca.
## Open questions for the human
- Is a user-global, unremoved plugin acceptable, or should there be an uninstall/version story? If two builds must coexist, what wins?ℹ️ Nitpicks
writeRelayOmpStatusExtension(src/relay/omp-status-extension.ts:5-8) re-declares.orca-relay,omp-managed-status-extension,orca-agent-status.ts, and the@orca-managed-pi-extensionmarker that already exist inplugin-overlay.ts; likewiseopencode-canonical-config.ts:17-18re-declares the plugin filenames. Two sources of truth for the same paths.- The new relay branch selection (canonical vs overlay,
relay-agent-hook-runtime.ts:94-106) has no test; only the leafinstallOpenCodePluginis covered inplugin-overlay.test.ts.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| if (!existingConfigDir || existingConfigDir === managedConfigDir) { | ||
| try { | ||
| this.writePluginToConfigDir(resolveOpenCodeConfigDirectory()) | ||
| return {} |
There was a problem hiding this comment.
Returning {} here is not enough to stop using an inherited Orca dir. assembly.ts:95-104 only assigns ORCA_OPENCODE_CONFIG_DIR from a truthy baseEnv.OPENCODE_CONFIG_DIR and never deletes either key, and the zsh/bash wrapper (zsh-startup-wrapper-builder.ts:73) re-exports OPENCODE_CONFIG_DIR="${ORCA_OPENCODE_CONFIG_DIR}" after the user's rc files. A nested Orca launched from a pane whose OPENCODE_CONFIG_DIR is an Orca overlay therefore still runs OpenCode against that overlay — the exact hijack this PR is fixing. Clearing the inherited keys (as restoreOrStripOverlayEnv does on the hooks-disabled path) is needed here.
Technical details
# Canonical branch does not clear an inherited override
## Affected sites
- `src/main/opencode/hook-service.ts:122-125` — returns `{}` with no instruction to drop `OPENCODE_CONFIG_DIR`.
- `src/main/ipc/pty/host-env/assembly.ts:95-104` — merges `{}` (no-op), then re-sets `ORCA_OPENCODE_CONFIG_DIR` from the inherited `OPENCODE_CONFIG_DIR` and only deletes `ORCA_OPENCODE_SOURCE_CONFIG_DIR`; no `else` deletes either key.
- `src/main/ipc/pty/host-env/pi-agent.ts:146-163` — `restoreOrStripOverlayEnv` (the only helper that deletes both) runs only when `agentStatusHooksEnabled` is false (`assembly.ts:117-128`), not on this branch.
- `src/main/zsh-startup-wrapper-builder.ts:73` — wrapper re-exports `OPENCODE_CONFIG_DIR` from `ORCA_OPENCODE_CONFIG_DIR`.
## Reachable sequence
1. A pane with a user-set `OPENCODE_CONFIG_DIR` gets an Orca overlay and `ORCA_OPENCODE_CONFIG_DIR` set to it (`hook-service.ts:138`, `assembly.ts:97-100`).
2. Orca is launched from that pane, so its `process.env` carries both keys, equal.
3. The child Orca spawns an OpenCode pane: `resolveOpenCodeSourceConfigDir` early-returns `undefined` (`pi-agent.ts:197-199`), so `buildPtyEnv` takes this branch.
4. The inherited overlay survives into the spawn env and the wrapper re-exports it.
## Required outcome
- When the canonical install is chosen, the spawned env must not carry an Orca-owned `OPENCODE_CONFIG_DIR`/`ORCA_OPENCODE_CONFIG_DIR`. `src/main/ipc/pty-spawn-env-agent-overlays.test.ts:92-124` mocks `buildPtyEnv` to a non-empty object, so this combination is untested.| } else { | ||
| this.pluginOverlay.installOpenCodePlugin(opencodeAgent, context.env) | ||
| } |
There was a problem hiding this comment.
This branch installs the plugin into the canonical root but leaves an inherited OPENCODE_CONFIG_DIR/ORCA_OPENCODE_CONFIG_DIR in context.env. Relay augmenters can only add or overwrite keys (pty-handler.ts:809-843; only the spawn request's envToDelete deletes), and pty-shell-overlay-wrappers.ts:72 re-exports OPENCODE_CONFIG_DIR from ORCA_OPENCODE_CONFIG_DIR. When sourceDir was itself the inherited relay overlay, the child still inherits it and the overlay wins over the canonical root. Same class as the local comment on hook-service.ts.
Technical details
# Relay canonical branch leaves inherited override
## Affected sites
- `src/relay/relay-agent-hook-runtime.ts:104-106` — `installOpenCodePlugin` only writes a file; it returns/modifies no env keys.
- `src/relay/plugin-overlay-env.ts:23-32` — `resolveOpenCodeSourceConfigDir` ignores `ORCA_OPENCODE_CONFIG_DIR`, so an inherited `ORCA_OPENCODE_CONFIG_DIR` alone yields `sourceDir === undefined` and takes this branch while the key survives.
- `src/relay/pty-handler.ts:809-843` — augmenter output is `Object.assign`-merged; no augmenter-side deletion exists.
- `src/relay/pty-shell-overlay-wrappers.ts:72` — wrapper re-exports the surviving value.
## Required outcome
- If the resolved source dir is an Orca relay overlay, the spawned env must not carry `OPENCODE_CONFIG_DIR`/`ORCA_OPENCODE_CONFIG_DIR`.
There was a problem hiding this comment.
ℹ️ No new issues in this delta. The only change since the prior review is the SSH e2e plugin assertion, which now matches the relay's canonical install. The findings from the prior review remain open and unaddressed.
Reviewed changes
- SSH e2e plugin assertion follows canonical discovery —
tests/e2e/ssh-localhost.spec.tsnow derives the OpenCode config root as${OPENCODE_CONFIG_DIR:-${XDG_CONFIG_HOME:-$HOME/.config}/opencode}and checksplugins/orca-opencode-status.jsthere, dropping the previous hard requirement thatOPENCODE_CONFIG_DIRbe set. This is consistent withinstallOpenCodePluginInCanonicalConfigwriting into the canonical root when no explicit source dir is present, and the marker assertion still fails the test when the plugin is missing.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Problem
Orca injected a relay config directory through
OPENCODE_CONFIG_DIR, which made OpenCode skip the user’s canonical global providers, models, plugins, and auth. This affected OpenCode 1 and OpenCode 2 launches.Change
Install Orca status plugins in OpenCode’s canonical XDG config directory and leave config discovery untouched for ordinary launches. Explicit user-configured
OPENCODE_CONFIG_DIRvalues still use the existing mirrored overlay so user files remain available. Relay launches ignore inherited Orca overlay paths and install into the remote canonical config root.Validation
pnpm tc:nodepnpm run check:code-quality:changedFollow-up
Addresses the follow-up to #21418 and issue #19708.
@mcxinyu Follow-up to your #19708 comment: this removes the inherited
OPENCODE_CONFIG_DIRoverride for ordinary OpenCode 1/2 launches, while preserving explicit user-configured overlays. Full local, relay, SSH, Docker SSH, packaging, typecheck, and E2E CI checks pass.