fix(config): tolerate JSONC when reading agent config files - #174
Conversation
…gent config files json_at()/merge_json()/merge_opencode_mcp() (components.rs) and wire_opencode() (init.rs) all read existing agent config files with plain serde_json::from_str before merging in agentflare's own entries. opencode.jsonc (and potentially other editor config dialects) commonly contains // and /* */ comments and trailing commas, both invalid in strict JSON — a real user's opencode.jsonc with comments silently fails to parse, and every read site was treating that failure as "no config yet", so agentflare init/apply would overwrite the file with just its own entry and everything else (existing mcp servers, model settings, instructions) got silently dropped. Adds src/jsonc.rs (parse_jsonc: strip comments, strip trailing commas, then serde_json::from_str; string contents preserved verbatim, multi-byte UTF-8 never split) and wires it into all four read sites. Regression tests added for both the components.rs merge path and init.rs's wire_opencode path, each constructing a real jsonc file with comments + an existing entry that must survive.
📝 WalkthroughWalkthroughThis change adds JSONC parsing with support for comments and trailing commas, then uses it when merging component configurations and wiring OpenCode instructions while preserving existing configuration entries. ChangesJSONC configuration support
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/jsonc.rs (1)
17-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate string-skipping logic in
strip_json_commentsandstrip_trailing_commas.The byte-level loop that skips over a
"..."string (including escape handling) is copy-pasted verbatim in both functions. Any future fix to string/escape handling (e.g. a corner case) would need to be applied twice.♻️ Proposed refactor: extract shared helper
/// Advances past a `"..."` string starting at `bytes[i] == b'"'`, honoring /// backslash escapes. Returns the index just past the closing quote (or /// `bytes.len()` if unterminated). fn skip_string(bytes: &[u8], mut i: usize) -> usize { let len = bytes.len(); i += 1; while i < len { let c = bytes[i]; i += 1; if c == b'\\' && i < len { i += 1; } else if c == b'"' { break; } } i }Then in both
strip_json_commentsandstrip_trailing_commas:if b == b'"' { - i += 1; - while i < len { - let c = bytes[i]; - i += 1; - if c == b'\\' && i < len { - i += 1; - } else if c == b'"' { - break; - } - } + i = skip_string(bytes, i); continue; }Also applies to: 78-100
🤖 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 `@src/jsonc.rs` around lines 17 - 39, Extract the duplicated quoted-string scanning logic into a shared skip_string helper that accepts the byte slice and starting quote index, preserves escape handling, and returns the index after the closing quote or the input length if unterminated. Replace the inline loops in both strip_json_comments and strip_trailing_commas with this helper.src/components.rs (1)
50-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a shared
read_jsonc(path, default)helper. The same read-file →parse_jsonc→ fallback-on-failure boilerplate is duplicated across four call sites. A single helper centralizes the JSONC-read contract and avoids a future config-reading site forgetting to useparse_jsonc(the exact class of bug this PR fixes).
src/components.rs#L50-L55: replace thejson_atbody with a call to the shared helper (defaultValue::Null).src/components.rs#L83-L104: replace the read+parse+fallback lines inmerge_jsonwith the shared helper (defaultjson!({})).src/components.rs#L106-L151: replace the read+parse+fallback lines inmerge_opencode_mcpwith the shared helper (defaultjson!({})).src/init.rs#L503-L575: replace the read+parse+fallback lines inwire_opencodewith the shared helper (defaultjson!({})).♻️ Proposed helper
// src/jsonc.rs pub fn read_jsonc(path: &std::path::Path, default: impl FnOnce() -> Value) -> Value { std::fs::read_to_string(path) .ok() .and_then(|s| parse_jsonc(&s).ok()) .unwrap_or_else(default) }🤖 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 `@src/components.rs` around lines 50 - 55, Introduce a shared read_jsonc helper in src/jsonc.rs that reads a path, parses it with parse_jsonc, and invokes a lazy default on any failure. Update json_at in src/components.rs (lines 50-55) to use it with Value::Null; update merge_json (lines 83-104) and merge_opencode_mcp (lines 106-151) to use it with json!({}); and update wire_opencode in src/init.rs (lines 503-575) with the same object default.
🤖 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 `@src/components.rs`:
- Around line 50-55: Introduce a shared read_jsonc helper in src/jsonc.rs that
reads a path, parses it with parse_jsonc, and invokes a lazy default on any
failure. Update json_at in src/components.rs (lines 50-55) to use it with
Value::Null; update merge_json (lines 83-104) and merge_opencode_mcp (lines
106-151) to use it with json!({}); and update wire_opencode in src/init.rs
(lines 503-575) with the same object default.
In `@src/jsonc.rs`:
- Around line 17-39: Extract the duplicated quoted-string scanning logic into a
shared skip_string helper that accepts the byte slice and starting quote index,
preserves escape handling, and returns the index after the closing quote or the
input length if unterminated. Replace the inline loops in both
strip_json_comments and strip_trailing_commas with this helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 036ce7f3-475c-4923-8d99-1c63b6989b52
📒 Files selected for processing (4)
src/components.rssrc/init.rssrc/jsonc.rssrc/main.rs
* chore: shrink release binary via panic=abort + zip feature trim Drops shipped agentflare.exe from 17.94 MiB to 12.35 MiB (-31%). panic=\abort\ strips unwind tables across the whole dependency graph (no catch_unwind anywhere in the workspace, so no functional risk). zip now only pulls the deflate codec it actually uses (src/update.rs reads plain zip archives for self-update; bzip2/lzma/zstd/zopfli/aes were unused default features). * refactor(config): dedupe JSONC helpers per CodeRabbit nitpicks on #174 Extract skip_string() out of strip_json_comments/strip_trailing_commas (same byte-level string-skip loop was copy-pasted in both) and add jsonc::read_jsonc(path, default) to replace the read -> parse_jsonc -> fallback boilerplate repeated across json_at, merge_json, merge_opencode_mcp (components.rs) and wire_opencode (init.rs). * revert(release): drop panic=abort, keep zip feature trim rmcp spawns every MCP tool call as its own tokio task; panic=abort removes that per-task isolation, turning a single panicking tool call into a whole-session process abort instead of just a failed request. The zip default-features trim is still safe and kept (binary drops ~250KiB from that alone; the earlier 31%/5.6MiB figure was mostly panic=abort, which this reverts).
Summary
json_at()/merge_json()/merge_opencode_mcp()(src/components.rs) andwire_opencode()(src/init.rs) all read an existing agent config file with plainserde_json::from_strbefore merging in agentflare's own entries.opencode.jsonc(and the JSONC dialect several editors use forsettings.json/mcp.json) commonly contains////* */comments and trailing commas — both invalid strict JSON. A real user'sopencode.jsoncwith comments silently fails to parse, and every one of these call sites was treating that parse failure as "no config yet" — soagentflare init/component-apply would overwrite the file with just agentflare's own entry, silently dropping the user's existing mcp servers, model settings, and instructions.Fix
Adds
src/jsonc.rs::parse_jsonc(strip////* */comments, strip trailing commas before}/], thenserde_json::from_str; string contents preserved verbatim, multi-byte UTF-8 never split — ported and adapted from lean-ctx'score/jsonc.rs) and wires it into all four read sites. A jsonc-tolerant parser is a strict superset of JSON, so this is safe for the non-jsonc config files that also route throughjson_at/merge_json(cursor/windsurf/vscode-copilot/clinemcp.json).Regression tests added for both the
components.rsmerge path andinit.rs'swire_opencodepath, each constructing a real jsonc file (comments + trailing comma + an existing entry) and asserting the existing entry survives the write.Testing
cargo test --workspace— 392 passed, 0 failedcargo clippy --workspace --all-targets -- -D warnings -A unsafe_code -A clippy::pedantic— cleancargo fmt --check— cleanSummary by CodeRabbit