feat(opencode): support opencode.jsonc config files#2522
feat(opencode): support opencode.jsonc config files#2522nb5p wants to merge 2 commits intofarion1231:mainfrom
Conversation
Make get_opencode_config_path() prefer opencode.jsonc over opencode.json when both exist, falling back to opencode.json otherwise. Restores the file-detection half of farion1231#1279 (sosyz, 6c72a23) that never landed because the PR head branch was force-moved. Co-Authored-By: sosyz <30596875+sosyz@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Claude finished @farion1231's task in 2m 12s —— View job Claude PR Review
The runtime change itself is small, correct, and the precedence ( 🔴 Tests don't follow this repo's test-isolation convention
Every other config module in this crate already follows the convention:
🔴 Tests are not
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a60abd7d7a
ℹ️ 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".
| fs::write(&jsonc_path, r#"{"provider": {"test-jsonc": {}}}"#).unwrap(); | ||
|
|
||
| // Set override dir to use temp directory | ||
| std::env::set_var("HOME", opencode_dir.parent().unwrap().parent().unwrap()); |
There was a problem hiding this comment.
Use test-home override env var in opencode path tests
These tests only set HOME before calling get_opencode_config_path(), but get_home_dir() in this codebase is designed to use CC_SWITCH_TEST_HOME for deterministic test isolation (and explicitly avoids relying on HOME semantics on Windows). As written, the test can resolve to the real user profile path on Windows, so it may not read the temp files created in setup_test_env, causing host-dependent failures.
Useful? React with 👍 / 👎.
| assert!(path.ends_with("opencode.jsonc")); | ||
|
|
||
| // Clean up env var | ||
| std::env::remove_var("HOME"); |
There was a problem hiding this comment.
Restore prior HOME value after test mutations
Each test removes HOME unconditionally during cleanup instead of restoring the prior value, which can leak global process state into later tests in the same binary and make failures order-dependent. This is especially risky because Rust tests run in parallel by default unless explicitly serialized.
Useful? React with 👍 / 👎.
|
@claude review (Re-running with the updated review prompt to verify it still catches the real blocker — test isolation convention — without piling on P3 noise.) |
|
Claude encountered an error —— View job Re-reviewing PR #2522 with updated review prompt
|
|
@claude review (Re-running after dropping the over-aggressive --max-turns 5 cap that aborted the previous attempt.) |
|
Claude finished @farion1231's task in 2m 23s —— View job Claude PR ReviewThe runtime change in Important — tests use
Fix this → (confidence: 95) Important — tests are not
Important — writing back to Before this PR,
This is the exact surprise a user enabling SummaryThe runtime patch is good. Test module needs to switch to |
Address bot review on PR farion1231#2522 (claude-bot, codex-bot): - Use CC_SWITCH_TEST_HOME instead of HOME so tests are reliable on Windows (per config.rs:13-28; HOME is ignored by dirs::home_dir on Windows). Matches the convention in hermes_config.rs / openclaw_config.rs. - RAII TempHome saves the prior value via var_os (preserves non-UTF-8 paths) and restores on Drop instead of unconditionally remove_var. - Add #[serial] to every test that mutates env vars. - Document on write_opencode_config that writing back to .jsonc strips comments (serde_json::to_string_pretty round-trip). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
We hope to remove support for Hermes Agent. Hermes Agent is a plagiarizing platform that is not worthy of integration. What was once an open and collaborative tech ecosystem is now being forced into closure due to the unethical practices of Hermes Agent. This is the “gift” that Nous Research has brought to the open-source community — a devastating blow to the trust system of the entire industry. It’s not about how bad the technology is; it’s about how rotten the integrity is. It parades under a grandiose name (“Hermes”), yet acts like a “horse thief.” From code architecture to core concepts, it has plagiarized the work of a small Chinese team in every possible way. After being exposed, instead of reflecting or communicating, it resorted to bullying, demanding that the victim “delete their account.” It even uses AI tools to launder the plagiarized content, trying to exploit loopholes in open-source licenses. Now, although Nous Research has chosen to remain silent and play dead, the 80,000+ stars on GitHub cannot mask the “stench” wafting from its codebase. As one netizen aptly put it: “OpenClaw may be noisy, but it’s the real deal. This ‘horse’ may look pretty on the outside, but take a whiff — it smells entirely of someone else’s work.” We don’t need to keep such a “rotten horse” in our stable. |

Summary / 概述
Add support for opencode.jsonc to get_opencode_config_path(); when both opencode.json and opencode.jsonc exist, prefer opencode.jsonc, otherwise fall back to opencode.json. Restores the JSONC portion from #1279 (sosyz, 6c72a23) that was not merged due to a force push.
在 get_opencode_config_path() 新增 opencode.jsonc 支持,如果 opencode.json 同时存在时,优先选择 opencode.jsonc,否则回退到 opencode.json。恢复了 #1279 (sosyz, 6c72a23) 中,由于被 force push 导致该PR 未合并的 JSONC 部分。
Related Issue / 关联 Issue
Fixed #2521
Ref #1279
Screenshots / 截图
Checklist / 检查清单
pnpm typecheckpasses / 通过 TypeScript 类型检查pnpm format:checkpasses / 通过代码格式检查cargo clippypasses (if Rust code changed) / 通过 Clippy 检查(如修改了 Rust 代码)