fix(install): respect OPENCODE_CONFIG_DIR env var for global config path#573
fix(install): respect OPENCODE_CONFIG_DIR env var for global config path#573cavanaug wants to merge 4 commits intoEveryInc:mainfrom
Conversation
Honour OPENCODE_CONFIG_DIR when resolving the default OpenCode install target, both in the install command and in the sync registry. Falls back to ~/.config/opencode when the variable is unset, preserving existing behaviour.
There was a problem hiding this comment.
Pull request overview
Updates the install + sync flows to respect OpenCode’s OPENCODE_CONFIG_DIR environment override when determining the global OpenCode config directory, falling back to ~/.config/opencode when unset.
Changes:
- Update OpenCode sync target detection/output root to prefer
OPENCODE_CONFIG_DIR. - Update
installdefault output root resolution to preferOPENCODE_CONFIG_DIR. - Adjust inline comments to document the new precedence behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/sync/registry.ts |
Prefer OPENCODE_CONFIG_DIR for OpenCode detect paths and sync output root. |
src/commands/install.ts |
Prefer OPENCODE_CONFIG_DIR when defaulting the install output root for OpenCode. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| process.env.OPENCODE_CONFIG_DIR | ||
| ? path.resolve(process.env.OPENCODE_CONFIG_DIR) | ||
| : path.join(home, ".config", "opencode"), | ||
| path.join(cwd, ".opencode"), | ||
| ], | ||
| resolveOutputRoot: (home) => path.join(home, ".config", "opencode"), | ||
| resolveOutputRoot: (home) => | ||
| process.env.OPENCODE_CONFIG_DIR | ||
| ? path.resolve(process.env.OPENCODE_CONFIG_DIR) | ||
| : path.join(home, ".config", "opencode"), |
There was a problem hiding this comment.
OPENCODE_CONFIG_DIR is used without trimming or ~ expansion. If a user sets it to whitespace (e.g. " /some/path ") or a tilde path (e.g. ~/opencode), path.resolve will either resolve to an unintended directory (relative to CWD) or treat ~ literally. Consider normalizing with trim() + expandHome() (or a shared helper) before resolving, and only honoring the env var when the trimmed value is non-empty.
| if (process.env.OPENCODE_CONFIG_DIR) { | ||
| return path.resolve(process.env.OPENCODE_CONFIG_DIR) |
There was a problem hiding this comment.
process.env.OPENCODE_CONFIG_DIR is checked for truthiness but not trimmed/expanded. A value like " ~/cfg " (or any leading/trailing whitespace) will be treated as set and path.resolve won’t expand ~, potentially writing to the wrong location. Suggest using the existing resolveTargetHome/expandHome normalization (and only honoring the env var when the trimmed value is non-empty).
| if (process.env.OPENCODE_CONFIG_DIR) { | |
| return path.resolve(process.env.OPENCODE_CONFIG_DIR) | |
| const configDir = process.env.OPENCODE_CONFIG_DIR?.trim() | |
| if (configDir) { | |
| return resolveTargetHome(configDir) |
| // OpenCode global config: respect OPENCODE_CONFIG_DIR if set, otherwise | ||
| // fall back to ~/.config/opencode per XDG spec. | ||
| // See: https://opencode.ai/docs/config/ | ||
| if (process.env.OPENCODE_CONFIG_DIR) { | ||
| return path.resolve(process.env.OPENCODE_CONFIG_DIR) | ||
| } | ||
| return path.join(os.homedir(), ".config", "opencode") |
There was a problem hiding this comment.
This change introduces new behavior (default install output root depends on OPENCODE_CONFIG_DIR) but there’s no test covering it. There are already CLI tests asserting the default ~/.config/opencode behavior; please add a companion test that sets OPENCODE_CONFIG_DIR and asserts output is written under that directory.
| { | ||
| name: "opencode", | ||
| detectPaths: (home, cwd) => [ | ||
| path.join(home, ".config", "opencode"), | ||
| process.env.OPENCODE_CONFIG_DIR | ||
| ? path.resolve(process.env.OPENCODE_CONFIG_DIR) | ||
| : path.join(home, ".config", "opencode"), | ||
| path.join(cwd, ".opencode"), | ||
| ], | ||
| resolveOutputRoot: (home) => path.join(home, ".config", "opencode"), | ||
| resolveOutputRoot: (home) => | ||
| process.env.OPENCODE_CONFIG_DIR | ||
| ? path.resolve(process.env.OPENCODE_CONFIG_DIR) | ||
| : path.join(home, ".config", "opencode"), | ||
| sync: syncToOpenCode, |
There was a problem hiding this comment.
detectPaths / resolveOutputRoot now depend on OPENCODE_CONFIG_DIR, but the tool detection/sync path logic isn’t covered by a test for this env override. The repo already has detectInstalledTools tests; consider adding a case that sets OPENCODE_CONFIG_DIR to a temp dir, creates that dir, and asserts OpenCode is detected (and not detected when only ~/.config/opencode exists and the env var points elsewhere).
Apply trim() and expandHome() to OPENCODE_CONFIG_DIR before path.resolve in both src/sync/registry.ts and src/commands/install.ts, preventing tilde paths (~/opencode) and whitespace-padded values from resolving to wrong locations. Refactor registry.ts to use a shared resolveOpenCodeConfigDir() helper so the normalization is defined once and used in both detectPaths and resolveOutputRoot. Add tests covering the env var override for both install (cli.test.ts) and tool detection (detect-tools.test.ts).
|
Pushed a follow-up commit (eab704e) addressing all four Copilot review comments: Normalization fixes (registry.ts + install.ts) Both files now apply Tests
All 662 existing tests still pass; the one pre-existing failure ( |
![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: eab704eb7b
ℹ️ 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".
| const envDir = process.env.OPENCODE_CONFIG_DIR?.trim() | ||
| if (envDir) { | ||
| return path.resolve(expandHome(envDir)) | ||
| } |
There was a problem hiding this comment.
Returning OPENCODE_CONFIG_DIR directly as outputRoot causes a layout regression when the directory name is not opencode/.opencode: writeOpenCodeBundle() infers global-vs-workspace structure from path.basename(outputRoot) (src/targets/opencode.ts, resolveOpenCodePaths), so installs go to <envDir>/.opencode/... instead of <envDir>/agents|skills|commands. In that scenario, users who set a custom config dir (for example /tmp/custom-config) will get files written to a path OpenCode does not treat as its global config tree.
Useful? React with 👍 / 👎.
…regression writeOpenCodeBundle infers global vs workspace layout from path.basename, so a custom OPENCODE_CONFIG_DIR value like /tmp/my-config would fall into the workspace branch and write agents to <dir>/.opencode/agents/ instead of the flat <dir>/agents/ layout OpenCode expects for its config root. Fix: thread isGlobalOpenCodeConfig from resolveOutputRoot through install and pass scope='global' to writeOpenCodeBundle when the output root is the global config dir (default ~/.config/opencode or OPENCODE_CONFIG_DIR). Update resolveOpenCodePaths to treat scope='global' as global layout, independent of basename. Also update the OPENCODE_CONFIG_DIR install test to assert flat layout.
|
The Codex bot's concern is valid — pushed a second follow-up (411c420) to fix it. Root cause confirmed
Fix (411c420)
Existing behaviour for |
This was a valuable insight that we missed — thank you. The basename heuristic in The approach we took (411c420) was to thread an explicit That said, we're open to alternative approaches if there's a cleaner way to handle this. A few options we considered but didn't pursue:
If there's a preferred approach or an issue with the current one, happy to revise. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 411c4204f8
ℹ️ 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".
| // For OpenCode, if the output root is the global config dir (default or OPENCODE_CONFIG_DIR), | ||
| // use "global" scope so writeOpenCodeBundle writes the flat layout regardless of basename. | ||
| const effectiveScope = | ||
| targetName === "opencode" && isGlobalOpenCodeConfig && resolvedScope === undefined | ||
| ? "global" |
There was a problem hiding this comment.
--to all path
Fresh evidence from this commit: isGlobalOpenCodeConfig and effectiveScope were added, but effectiveScope is only computed in the single-target branch after the targetName === "all" early return. If OPENCODE_CONFIG_DIR is set to a directory whose basename is not opencode/.opencode, install --to all still calls writeOpenCodeBundle without global scope, so OpenCode files are written under <envDir>/.opencode/... instead of the global <envDir>/{agents,skills,commands} layout.
Useful? React with 👍 / 👎.
The effectiveScope/isGlobalOpenCodeConfig logic added in 411c420 was only applied in the single-target branch. The --to all path called handler.write(root, bundle) without a scope, so OPENCODE_CONFIG_DIR with a non-standard basename still produced the nested workspace layout. Pass scope='global' for the opencode target in the --to all loop when isGlobalOpenCodeConfig is true, consistent with the single-target path.
Correct — another genuine miss. The Fixed in c1475a1: the |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1475a1456
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const envDir = process.env.OPENCODE_CONFIG_DIR?.trim() | ||
| if (envDir) { | ||
| return { root: path.resolve(expandHome(envDir)), isGlobalOpenCodeConfig: true } |
There was a problem hiding this comment.
resolveOutputRoot now treats OPENCODE_CONFIG_DIR as the primary global output root, so install --to opencode writes opencode.json there and skips ~/.config/opencode/opencode.json whenever the env var exists. In this repo’s OpenCode spec (docs/specs/opencode.md), custom directory sources are additive in precedence rather than a replacement for global config files, so MCP/permission updates can be silently missed for users who set OPENCODE_CONFIG_DIR for overlays. The config file target should remain the global config path (or explicit OPENCODE_CONFIG) instead of being redirected by OPENCODE_CONFIG_DIR.
Useful? React with 👍 / 👎.
| detectPaths: (home, cwd) => [ | ||
| path.join(home, ".config", "opencode"), | ||
| resolveOpenCodeConfigDir(home), | ||
| path.join(cwd, ".opencode"), |
There was a problem hiding this comment.
With this change, OpenCode detection uses resolveOpenCodeConfigDir(home) first and no longer checks ~/.config/opencode when OPENCODE_CONFIG_DIR is defined. If that env var points to a missing or transient directory, detectInstalledTools reports OpenCode as not installed even when the global install exists, which can cause install --to all and sync all to skip OpenCode unexpectedly. Detection should consider both the custom directory and the standard global directory rather than treating the env path as exclusive.
Useful? React with 👍 / 👎.
Note: I used an AI agent (upstream-skill) to help draft this, but I (cavanaug) reviewed the write-up and the code. If the tone/format isn't a good fit here, please let me know and I'll adjust the skill.
I noticed the install command and sync registry always resolve the OpenCode global config path to
~/.config/opencode, ignoring the OPENCODE_CONFIG_DIR environment variable that OpenCode itself honours.This causes problems when users have a non-standard config location (e.g. on NixOS, in Docker, or with XDG_CONFIG_HOME set), because
bun x compound-engineering installorbun x compound-engineering syncwrites to the wrong place while OpenCode reads from wherever OPENCODE_CONFIG_DIR points.The fix is small: both resolveOutputRoot in
src/commands/install.tsand the opencode entry insrc/sync/registry.tsnow check OPENCODE_CONFIG_DIR first and fall back to~/.config/opencodewhen it's unset. No behaviour change for users without the variable set.