Respect OPENCODE_CONFIG in open-cursor CLI#91
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesOPENCODE_CONFIG env var for config path resolution
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
🧹 Nitpick comments (1)
tests/unit/cli/opencode-cursor.test.ts (1)
132-165: ⚡ Quick winConsider adding a test for the default path fallback.
The current tests verify env-var usage and flag precedence. Adding a test that verifies the default path (when neither
--confignorOPENCODE_CONFIGis set) would make the suite more complete and serve as a regression guard.✅ Suggested test case
+ it("falls back to default config path when neither --config nor OPENCODE_CONFIG is set", () => { + const originalConfig = process.env.OPENCODE_CONFIG; + delete process.env.OPENCODE_CONFIG; + + try { + const { configPath } = resolvePaths({}); + const configHome = process.env.XDG_CONFIG_HOME && process.env.XDG_CONFIG_HOME.length > 0 + ? process.env.XDG_CONFIG_HOME + : join(require("os").homedir(), ".config"); + const expectedDefault = resolve(join(configHome, "opencode", "opencode.json")); + expect(configPath).toBe(expectedDefault); + } finally { + if (originalConfig === undefined) { + delete process.env.OPENCODE_CONFIG; + } else { + process.env.OPENCODE_CONFIG = originalConfig; + } + } + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/cli/opencode-cursor.test.ts` around lines 132 - 165, Add a new test case in the "cli/opencode-cursor path resolution" describe block to verify the default path fallback behavior. The test should clear the OPENCODE_CONFIG environment variable (if set) and call resolvePaths with no arguments, then assert that the returned configPath matches the expected default path. Follow the same pattern as the existing tests in the describe block by saving the original OPENCODE_CONFIG value, clearing it in the test setup, verifying the default path behavior, and properly restoring the original value in the finally block. This test should be added alongside the existing "uses OPENCODE_CONFIG when --config is not provided" and "prefers --config over OPENCODE_CONFIG" test cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/cli/opencode-cursor.test.ts`:
- Around line 132-165: Add a new test case in the "cli/opencode-cursor path
resolution" describe block to verify the default path fallback behavior. The
test should clear the OPENCODE_CONFIG environment variable (if set) and call
resolvePaths with no arguments, then assert that the returned configPath matches
the expected default path. Follow the same pattern as the existing tests in the
describe block by saving the original OPENCODE_CONFIG value, clearing it in the
test setup, verifying the default path behavior, and properly restoring the
original value in the finally block. This test should be added alongside the
existing "uses OPENCODE_CONFIG when --config is not provided" and "prefers
--config over OPENCODE_CONFIG" test cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fab1e727-d769-4a24-b0d9-3ddab57c9959
📒 Files selected for processing (2)
src/cli/opencode-cursor.tstests/unit/cli/opencode-cursor.test.ts
|
Thanks Akshay, good catch here. I merged this one. The change makes sense and lines the CLI up with how the plugin/runtime already resolve I also pushed a tiny follow-up cleanup after merge to reuse the shared config path resolver, just so this logic does not drift in two places. cheers, RAMA |
2 participants
Summary
Tests
Note: bun test tests/unit/models/sync.test.ts currently exits 1 locally without diagnostic output; this test file is outside the CLI resolver change.