fix(installer): always use .config/opencode for CLI on Windows (#2502)#2546
Merged
acamq merged 3 commits intocode-yeongyu:devfrom Mar 15, 2026
Merged
fix(installer): always use .config/opencode for CLI on Windows (#2502)#2546acamq merged 3 commits intocode-yeongyu:devfrom
acamq merged 3 commits intocode-yeongyu:devfrom
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 files
Confidence score: 4/5
- This PR is likely safe to merge, with minimal risk centered on test behavior rather than runtime production code.
- The main issue is in
src/shared/opencode-config-dir-windows-appdata-bug.test.ts: directly mutatingfs.existsSyncmay not mock ESM named imports in Bun, which can cause flaky or ineffective test interception. - Given the moderate severity (5/10) and high confidence, the impact is mainly reduced test reliability and potential false confidence in coverage for this Windows AppData path behavior.
- Pay close attention to
src/shared/opencode-config-dir-windows-appdata-bug.test.ts- switch tomock.module("node:fs", ...)at file scope so Bun reliably interceptsexistsSync.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/opencode-config-dir-windows-appdata-bug.test.ts">
<violation number="1" location="src/shared/opencode-config-dir-windows-appdata-bug.test.ts:37">
P2: Mutating `fs.existsSync` directly fails to intercept ESM named imports in Bun. Use `mock.module("node:fs", () => ({ existsSync: ... }))` from `bun:test` at the file level instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
acamq
force-pushed
the
fix/installer-paths
branch
from
c49902e to
6b2da3c
Compare
acamq
changed the title
Collaborator
Author
|
@cubic-dev-ai review |
@acamq I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 3/5
- There is a concrete compatibility regression risk in
src/shared/opencode-config-dir.ts: hardcoding~/.config/opencodeon Windows can ignoreXDG_CONFIG_HOME, changing where config is discovered for users who rely on that env var. - Given the issue’s high severity/confidence (7/10, 10/10) and user-visible behavior impact, this carries some merge risk even though the change scope appears small.
- Pay close attention to
src/shared/opencode-config-dir.ts- restoreXDG_CONFIG_HOME-aware resolution on Windows to avoid breaking existing OpenCode CLI config lookup behavior.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/opencode-config-dir.ts">
<violation number="1" location="src/shared/opencode-config-dir.ts:52">
P1: Custom agent: **Opencode Compatibility**
Hardcoding `~/.config/opencode` for Windows drops support for the `XDG_CONFIG_HOME` environment variable, which the OpenCode CLI naturally supports via `xdg-basedir` on all OS platforms. This discrepancy means the installer will check the wrong directory for Windows users who override `XDG_CONFIG_HOME`. Remove the entire `win32` platform check and let Windows fall through to the XDG fallback below it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Collaborator
Author
|
@cubic-dev-ai review |
@acamq I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Removing the Win32-specific block causes the installer to respect XDG_CONFIG_HOME on Windows, which may diverge from the CLI's behavior if it strictly uses ~/.config/opencode.
There was a problem hiding this comment.
1 issue found across 2 files
Confidence score: 4/5
- This PR is likely safe to merge, with only a small test-stability risk rather than a production behavior change.
- In
src/shared/opencode-config-dir.test.ts, not clearingprocess.env.XDG_CONFIG_HOMEcan make the test fail depending on the runner’s environment, which may cause intermittent CI noise. - The issue is moderate-low severity (4/10) and highly certain, so it’s worth fixing soon but does not look merge-blocking for runtime functionality.
- Pay close attention to
src/shared/opencode-config-dir.test.ts- ensureprocess.env.XDG_CONFIG_HOMEis deleted/reset to keep the test deterministic.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/opencode-config-dir.test.ts">
<violation number="1" location="src/shared/opencode-config-dir.test.ts:200">
P2: Delete `process.env.XDG_CONFIG_HOME` to prevent environment variables from causing this test to fail.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Windows CLI tests were not deleting XDG_CONFIG_HOME, making them fragile in environments where this variable is set. getCliConfigDir() reads XDG_CONFIG_HOME on all platforms, not just Linux.
Collaborator
Author
|
@cubic-dev-ai review |
@acamq I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Auto-approved: Aligns Windows config directory with CLI behavior by removing incorrect AppData fallback and simplifying the shared logic. Includes regression tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2502 - On Windows, the oh-my-opencode installer was incorrectly checking for existing configs in AppData and potentially targeting that directory. OpenCode CLI always reads from
~/.config/opencode, so the installer should always write there.Changes
src/shared/opencode-config-dir.ts: SimplifiedgetCliConfigDir()Windows block to always return~/.config/opencode, removing the AppData fallback logicsrc/shared/opencode-config-dir.test.ts: Added regression test verifying Windows CLI ignores APPDATA env varTest Plan