diff --git a/crates/codegraph-cli/src/installer/shared.rs b/crates/codegraph-cli/src/installer/shared.rs index 2742744..28eccd3 100644 --- a/crates/codegraph-cli/src/installer/shared.rs +++ b/crates/codegraph-cli/src/installer/shared.rs @@ -54,23 +54,156 @@ pub fn codegraph_permissions() -> Vec<&'static str> { ] } -/// Read a JSON file, returning `{}` when missing or unparseable. Ports -/// `readJsonFile` (shared.ts:57): an unparseable file is backed up to -/// `.backup` before returning empty. -pub fn read_json_file(path: &Path) -> Map { +/// Data-safety invariant: a missing file is safe to treat as empty (fresh +/// install), but an existing-yet-unparseable file must NEVER become `{}` — that +/// empty map, written back, destroys the user's real config. Write paths abort +/// on `Unparseable`. +pub enum ConfigRead { + Missing, + Parsed(Map), + Unparseable, +} + +/// Strip `//` line comments and `/* */` block comments from JSONC, preserving +/// anything inside string literals (so `"a//b"` and `"a/*b*/c"` survive). Also +/// drops a single trailing comma before `}`/`]`. Pure string transform — no I/O. +fn strip_jsonc(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let bytes = text.as_bytes(); + let mut i = 0; + let mut in_string = false; + let mut escaped = false; + while i < bytes.len() { + let b = bytes[i]; + if in_string { + out.push(b as char); + if escaped { + escaped = false; + } else if b == b'\\' { + escaped = true; + } else if b == b'"' { + in_string = false; + } + i += 1; + continue; + } + if b == b'"' { + in_string = true; + out.push('"'); + i += 1; + continue; + } + if b == b'/' && i + 1 < bytes.len() && bytes[i + 1] == b'/' { + i += 2; + while i < bytes.len() && bytes[i] != b'\n' { + i += 1; + } + continue; + } + if b == b'/' && i + 1 < bytes.len() && bytes[i + 1] == b'*' { + i += 2; + while i + 1 < bytes.len() && !(bytes[i] == b'*' && bytes[i + 1] == b'/') { + i += 1; + } + i += 2; + continue; + } + out.push(b as char); + i += 1; + } + drop_trailing_commas(&out) +} + +/// Remove `,` that is immediately followed (ignoring whitespace) by `}` or `]`. +/// Runs on comment-stripped text, so it never sees commas inside comments; +/// commas inside strings are guarded by tracking string state. +fn drop_trailing_commas(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let bytes = text.as_bytes(); + let mut in_string = false; + let mut escaped = false; + let mut i = 0; + while i < bytes.len() { + let b = bytes[i]; + if in_string { + out.push(b as char); + if escaped { + escaped = false; + } else if b == b'\\' { + escaped = true; + } else if b == b'"' { + in_string = false; + } + i += 1; + continue; + } + if b == b'"' { + in_string = true; + out.push('"'); + i += 1; + continue; + } + if b == b',' { + let mut j = i + 1; + while j < bytes.len() && bytes[j].is_ascii_whitespace() { + j += 1; + } + if j < bytes.len() && (bytes[j] == b'}' || bytes[j] == b']') { + i += 1; + continue; + } + } + out.push(b as char); + i += 1; + } + out +} + +/// Parse JSON, then JSONC (comments / trailing commas) as a fallback. Returns the +/// top-level object map, or `None` if neither parse yields a JSON object. +pub fn parse_json_object(text: &str) -> Option> { + if text.trim().is_empty() { + return Some(Map::new()); + } + if let Ok(Value::Object(map)) = serde_json::from_str::(text) { + return Some(map); + } + match serde_json::from_str::(&strip_jsonc(text)) { + Ok(Value::Object(map)) => Some(map), + _ => None, + } +} + +/// Read an agent config file, distinguishing missing / parsed / unparseable. +/// +/// Tolerates JSONC (comments + trailing commas). A present-but-unparseable file +/// is backed up to `.backup` and reported as [`ConfigRead::Unparseable`] +/// WITHOUT being modified, so the caller can skip writing instead of clobbering +/// the user's config. +pub fn read_config_file(path: &Path) -> ConfigRead { let Ok(text) = fs::read_to_string(path) else { - return Map::new(); + return ConfigRead::Missing; }; - match serde_json::from_str::(&text) { - Ok(Value::Object(map)) => map, - Ok(_) => Map::new(), - Err(_) => { + match parse_json_object(&text) { + Some(map) => ConfigRead::Parsed(map), + None => { let _ = fs::copy(path, path.with_extension("backup")); - Map::new() + ConfigRead::Unparseable } } } +/// Read a JSON/JSONC file into a map. Backward-compatible helper that maps +/// [`ConfigRead::Missing`] to `{}`. Callers on the WRITE path must NOT use this +/// (it cannot signal the unparseable case); use [`read_config_file`] and abort +/// on [`ConfigRead::Unparseable`] there. +pub fn read_json_file(path: &Path) -> Map { + match read_config_file(path) { + ConfigRead::Parsed(map) => map, + ConfigRead::Missing | ConfigRead::Unparseable => Map::new(), + } +} + /// Write a file atomically: write to `.tmp.`, then rename. /// Ports `atomicWriteFileSync` (shared.ts:80). pub fn atomic_write_file(path: &Path, content: &str) -> std::io::Result<()> { @@ -315,3 +448,73 @@ fn find_next_table_header(content: &str, from: usize) -> usize { } content.len() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parses_jsonc_with_line_and_block_comments() { + let text = r#"{ + // a line comment + "a": 1, /* inline */ + "b": "keep // and /* */ inside string" + }"#; + let map = parse_json_object(text).expect("JSONC must parse"); + assert_eq!(map.get("a"), Some(&serde_json::json!(1))); + assert_eq!( + map.get("b"), + Some(&serde_json::json!("keep // and /* */ inside string")) + ); + } + + #[test] + fn parses_jsonc_with_trailing_commas() { + let map = parse_json_object("{ \"a\": [1, 2,], \"b\": 3, }").expect("trailing commas ok"); + assert_eq!(map.get("a"), Some(&serde_json::json!([1, 2]))); + assert_eq!(map.get("b"), Some(&serde_json::json!(3))); + } + + #[test] + fn truly_corrupt_text_is_unparseable() { + assert!(parse_json_object("{ this is not json").is_none()); + } + + #[test] + fn read_outcome_missing_parsed_unparseable() { + let dir = std::env::temp_dir().join(format!( + "cg-shared-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&dir).unwrap(); + + let missing = dir.join("missing.json"); + assert!(matches!(read_config_file(&missing), ConfigRead::Missing)); + + let jsonc = dir.join("config.jsonc"); + fs::write(&jsonc, "{\n // c\n \"x\": 1,\n}\n").unwrap(); + match read_config_file(&jsonc) { + ConfigRead::Parsed(map) => assert_eq!(map.get("x"), Some(&serde_json::json!(1))), + other => panic!("expected Parsed, got {:?}", std::mem::discriminant(&other)), + } + + let corrupt = dir.join("corrupt.json"); + let raw = "{ broken not json"; + fs::write(&corrupt, raw).unwrap(); + assert!(matches!( + read_config_file(&corrupt), + ConfigRead::Unparseable + )); + assert_eq!( + fs::read_to_string(&corrupt).unwrap(), + raw, + "unparseable file must be left byte-for-byte unchanged" + ); + + let _ = fs::remove_dir_all(&dir); + } +} diff --git a/crates/codegraph-cli/src/installer/targets/antigravity.rs b/crates/codegraph-cli/src/installer/targets/antigravity.rs index 0c094f8..8ead1b1 100644 --- a/crates/codegraph-cli/src/installer/targets/antigravity.rs +++ b/crates/codegraph-cli/src/installer/targets/antigravity.rs @@ -11,10 +11,11 @@ use std::fs; use std::path::{Path, PathBuf}; -use serde_json::{json, Value}; +use serde_json::{json, Map, Value}; use super::super::shared::{ - read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, write_json_file, + read_config_file, read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, + write_json_file, ConfigRead, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -172,7 +173,16 @@ fn write_mcp_entry(ctx: &InstallContext) -> FileWrite { if let Some(dir) = file.parent() { let _ = fs::create_dir_all(dir); } - let mut existing = read_json_file(&file); + let mut existing = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = existing.get("mcpServers").and_then(|s| s.get("codegraph")); let after = build_antigravity_entry(); if before == Some(&after) { diff --git a/crates/codegraph-cli/src/installer/targets/claude.rs b/crates/codegraph-cli/src/installer/targets/claude.rs index 3a9e475..54ade08 100644 --- a/crates/codegraph-cli/src/installer/targets/claude.rs +++ b/crates/codegraph-cli/src/installer/targets/claude.rs @@ -7,12 +7,12 @@ use std::fs; use std::path::PathBuf; -use serde_json::{json, Value}; +use serde_json::{json, Map, Value}; use super::super::shared::{ - self, codegraph_permissions, mcp_server_config, read_json_file, + self, codegraph_permissions, mcp_server_config, read_config_file, read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, upsert_instructions_entry, - write_json_file, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, + write_json_file, ConfigRead, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -133,7 +133,16 @@ impl AgentTarget for ClaudeCodeTarget { // Ports writeMcpEntry (claude.ts:214). fn write_mcp_entry(ctx: &InstallContext, loc: Location) -> FileWrite { let file = mcp_json_path(ctx, loc); - let mut existing = read_json_file(&file); + let mut existing = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = existing.get("mcpServers").and_then(|s| s.get("codegraph")); let after = mcp_server_config(); diff --git a/crates/codegraph-cli/src/installer/targets/cursor.rs b/crates/codegraph-cli/src/installer/targets/cursor.rs index 9a832dc..5f30ac7 100644 --- a/crates/codegraph-cli/src/installer/targets/cursor.rs +++ b/crates/codegraph-cli/src/installer/targets/cursor.rs @@ -10,11 +10,11 @@ use std::fs; use std::path::PathBuf; -use serde_json::{json, Value}; +use serde_json::{json, Map, Value}; use super::super::shared::{ - mcp_server_config, read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, - write_json_file, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, + mcp_server_config, read_config_file, read_json_file, remove_codegraph_from_mcp_servers, + to_upstream_json, write_json_file, ConfigRead, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -133,7 +133,16 @@ impl AgentTarget for CursorTarget { // Ports writeMcpEntry (cursor.ts:177). fn write_mcp_entry(ctx: &InstallContext, loc: Location) -> FileWrite { let file = mcp_json_path(ctx, loc); - let mut existing = read_json_file(&file); + let mut existing = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = existing.get("mcpServers").and_then(|s| s.get("codegraph")); let after = build_cursor_mcp_config(ctx, loc); if before == Some(&after) { diff --git a/crates/codegraph-cli/src/installer/targets/gemini.rs b/crates/codegraph-cli/src/installer/targets/gemini.rs index fef871c..88f2ab5 100644 --- a/crates/codegraph-cli/src/installer/targets/gemini.rs +++ b/crates/codegraph-cli/src/installer/targets/gemini.rs @@ -7,11 +7,12 @@ use std::path::PathBuf; -use serde_json::json; +use serde_json::{json, Map}; use super::super::shared::{ - self, mcp_server_config, read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, - upsert_instructions_entry, write_json_file, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, + self, mcp_server_config, read_config_file, read_json_file, remove_codegraph_from_mcp_servers, + to_upstream_json, upsert_instructions_entry, write_json_file, ConfigRead, + CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -113,7 +114,16 @@ fn write_mcp_entry(ctx: &InstallContext, loc: Location) -> FileWrite { if let Some(dir) = file.parent() { let _ = std::fs::create_dir_all(dir); } - let mut existing = read_json_file(&file); + let mut existing = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = existing.get("mcpServers").and_then(|s| s.get("codegraph")); let after = mcp_server_config(); if before == Some(&after) { diff --git a/crates/codegraph-cli/src/installer/targets/kiro.rs b/crates/codegraph-cli/src/installer/targets/kiro.rs index eb6e317..0599fd1 100644 --- a/crates/codegraph-cli/src/installer/targets/kiro.rs +++ b/crates/codegraph-cli/src/installer/targets/kiro.rs @@ -8,11 +8,11 @@ use std::fs; use std::path::PathBuf; -use serde_json::json; +use serde_json::{json, Map}; use super::super::shared::{ - mcp_server_config, read_json_file, remove_codegraph_from_mcp_servers, to_upstream_json, - write_json_file, + mcp_server_config, read_config_file, read_json_file, remove_codegraph_from_mcp_servers, + to_upstream_json, write_json_file, ConfigRead, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -116,7 +116,16 @@ fn write_mcp_entry(ctx: &InstallContext, loc: Location) -> FileWrite { if let Some(dir) = file.parent() { let _ = fs::create_dir_all(dir); } - let mut existing = read_json_file(&file); + let mut existing = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = existing.get("mcpServers").and_then(|s| s.get("codegraph")); let after = mcp_server_config(); if before == Some(&after) { diff --git a/crates/codegraph-cli/src/installer/targets/opencode.rs b/crates/codegraph-cli/src/installer/targets/opencode.rs index d1d04da..add79f2 100644 --- a/crates/codegraph-cli/src/installer/targets/opencode.rs +++ b/crates/codegraph-cli/src/installer/targets/opencode.rs @@ -18,8 +18,8 @@ use std::path::{Path, PathBuf}; use serde_json::{json, Map, Value}; use super::super::shared::{ - self, to_upstream_json, upsert_instructions_entry, write_json_file, CODEGRAPH_SECTION_END, - CODEGRAPH_SECTION_START, + self, parse_json_object, read_config_file, to_upstream_json, upsert_instructions_entry, + write_json_file, ConfigRead, CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, }; use super::super::types::{ AgentTarget, DetectionResult, FileAction, FileWrite, InstallContext, InstallOptions, Location, @@ -90,13 +90,7 @@ fn opencode_server_entry() -> Value { } fn parse_config(text: &str) -> Map { - if text.trim().is_empty() { - return Map::new(); - } - match serde_json::from_str::(text) { - Ok(Value::Object(map)) => map, - _ => Map::new(), - } + parse_json_object(text).unwrap_or_default() } impl AgentTarget for OpencodeTarget { @@ -169,8 +163,16 @@ impl AgentTarget for OpencodeTarget { fn write_mcp_entry(ctx: &InstallContext, loc: Location) -> FileWrite { let file = config_path(ctx, loc); let existed = file.exists(); - let text = fs::read_to_string(&file).unwrap_or_default(); - let mut config = parse_config(&text); + let mut config = match read_config_file(&file) { + ConfigRead::Missing => Map::new(), + ConfigRead::Parsed(map) => map, + ConfigRead::Unparseable => { + return FileWrite { + path: file, + action: FileAction::Skipped, + }; + } + }; let before = config.get("mcp").and_then(|m| m.get("codegraph")); let after = opencode_server_entry(); if before == Some(&after) { diff --git a/crates/codegraph-cli/src/installer/types.rs b/crates/codegraph-cli/src/installer/types.rs index 4f2e3aa..73142b6 100644 --- a/crates/codegraph-cli/src/installer/types.rs +++ b/crates/codegraph-cli/src/installer/types.rs @@ -61,6 +61,7 @@ pub enum FileAction { Removed, NotFound, Kept, + Skipped, } impl FileAction { @@ -72,6 +73,7 @@ impl FileAction { FileAction::Removed => "Removed", FileAction::NotFound => "Not found", FileAction::Kept => "Kept", + FileAction::Skipped => "Skipped (unparseable — left unchanged)", } } }