Repository navigation
fix(util): warn when OPENCODE_CONFIG_DIR replaces the user-level config - #62383
CodeWithAdarsh007 wants to merge 1 commit into
Conversation
When OPENCODE_CONFIG_DIR is set to a path different from the user-level XDG config directory, Global.layer uses it as the sole global config root. Providers, permissions, instructions, and commands defined in ~/.config/opencode silently stop applying — no log, no error. This adds one Effect.logWarning at layer init, naming the override path and the skipped user-level path, and pointing at the two supported workarounds: OPENCODE_CONFIG (additive overlay) or unset the variable. No semantic change — replacement remains current behavior; this only makes it visible. Refs anomalyco#32825. Implements the minimal ask from the issue discussion (warning rather than semantic change). Tests: pure function `configDirOverrideWarning(override, base)` with 4 cases (unset / equal / differs / empty).
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
Thanks for picking this up. As it stands, the warning never fires for users, so the silent replacement in #32825 stays silent.
The warning is in Global's default layer. The CLI (packages/cli/src/index.ts), the server process (packages/cli/src/server-process.ts) and the server routes (packages/server/src/routes.ts) all replace that layer with Global.layerWith(...), so the new code is skipped. I ran opencode serve from this branch with OPENCODE_CONFIG_DIR set to a separate directory. The server started and logged normally, but no warning appeared in the log file or on the console. To have an effect, the warning has to run where OPENCODE_CONFIG_DIR is actually read: those replacement sites, or one shared helper they all call. A test should then check that path, not only the string helper.
The four unit tests cover only the message helper. They pass, but they can't catch that the warning is never logged.
This also overlaps with #62315, which tries to fix #32825 by making the variable add to the user-level config. If that one lands, this message ("replaces… will not apply") becomes wrong, so maintainers will likely want only one of the two.
Public API: no change, apart from the new exported Global.configDirOverrideWarning helper in packages/util. The behaviour of OPENCODE_CONFIG_DIR itself is unchanged.
Related: #32825
| Service, | ||
| Effect.suspend(() => acquire({ config: process.env.OPENCODE_CONFIG_DIR ?? Path.config })), | ||
| Effect.gen(function* () { | ||
| const override = process.env.OPENCODE_CONFIG_DIR |
There was a problem hiding this comment.
This default layer isn't what the CLI or server use. packages/cli/src/index.ts, packages/cli/src/server-process.ts and packages/server/src/routes.ts all replace Global.node with Global.layerWith(...), so this logWarning never runs on those paths. I checked with opencode serve and OPENCODE_CONFIG_DIR set: normal log lines, no warning. The check needs to run where those call sites read OPENCODE_CONFIG_DIR, for example in one shared helper they all call.
Issue for this PR
Fixes #32825
Type of change
What does this PR do?
Adds a single
Effect.logWarningwhenOPENCODE_CONFIG_DIRis set to a path that differs from the user-level XDG config directory, so the silent replacement is at least visible. This is the warning-only minimal fix requested by tslator on the issue; additive semantics remain unfixed and would be a separate PR.With
OPENCODE_CONFIG_DIRset,Global.layer(packages/util/src/global.ts) uses that path as the sole global config root, so providers, permissions, instructions, and commands from~/.config/opencodestop applying with no log or error (#32825).Reproduction on v2.1.3:
opencode modelsreturned 131 models without the variable set and 2 models with it set. Note: observe with--standalone(or a server restart) —OPENCODE_CONFIG_DIRis read once at server startup, so an already-running shared daemon keeps the original config and hides the drop. Full reproduction: #32825 (comment)tslator's per-resource-type analysis traces which resources silently drop and asks for the minimal fix: one warning line naming the skipped file. niebloomj's +1 describes the managed-vs-personal config setup, which is exactly the scenario this warning helps diagnose.
What changed:
configDirOverrideWarning(override, base): string | undefinedinpackages/util/src/global.ts— exported for testability, takes strings only.layerdeclaration is rewritten fromEffect.suspend(() => acquire(...))toEffect.gen(function* () { ... yield* Effect.logWarning(warning) ... return yield* acquire(...) }). Behavior identical: same env read timing, same acquire input.packages/util/test/global.test.ts.What this does NOT change:
OPENCODE_CONFIG_DIRadditive.ConfigDiscoveryacross ~8 files; that is a separate, larger PR and out of scope here.OPENCODE_CONFIGfor an additive overlay, or unset the variable).How did you verify your code works?
cd packages/util && bun test test/global.test.ts-> 7 pass, 0 fail; full package -> 35 pass, 0 fail;bun run typecheck-> 0 errors in 68 files. The pre-push lint hook panics locally on Windows (oxlint Rust allocator bug), but CI runs the same checks on Linux and passes.Visual
N/A - non-UI change.
Screenshots / recordings
N/A - non-UI change.
Checklist
Added by the opencode bot (AI-generated).
Related: #32825