refactor(setup): migrate all config I/O to mergeJsoncFile (#1031)

Author: HaleTom

gitnexus/src/cli/setup.ts

@@ -13,7 +13,7 @@ import { execFile, execFileSync } from 'child_process';
 import { promisify } from 'util';
 import { fileURLToPath } from 'url';
 import { glob } from 'glob';
-import { parseTree, modify, applyEdits, ParseError } from 'jsonc-parser';
+import { parseTree, modify, applyEdits, ParseError, parse as parseJsonc } from 'jsonc-parser';
 import { getGlobalDir } from '../storage/repo-manager.js';
 
 const __filename = fileURLToPath(import.meta.url);
@@ -93,41 +93,6 @@ function getOpenCodeMcpEntry() {
   return { type: 'local', command: ['npx', '-y', 'gitnexus@latest', 'mcp'] };
 }
 
-/**
- * Merge gitnexus entry into an existing MCP config JSON object.
- * Returns the updated config.
- */
-function mergeMcpConfig(existing: any): any {
-  if (!existing || typeof existing !== 'object') {
-    existing = {};
-  }
-  if (!existing.mcpServers || typeof existing.mcpServers !== 'object') {
-    existing.mcpServers = {};
-  }
-  existing.mcpServers.gitnexus = getMcpEntry();
-  return existing;
-}
-
-/**
- * Try to read a JSON file, returning null if it doesn't exist or is invalid.
- */
-async function readJsonFile(filePath: string): Promise<any | null> {
-  try {
-    const raw = await fs.readFile(filePath, 'utf-8');
-    return JSON.parse(raw);
-  } catch {
-    return null;
-  }
-}
-
-/**
- * Write JSON to a file, creating parent directories if needed.
- */
-async function writeJsonFile(filePath: string, data: any): Promise<void> {
-  await fs.mkdir(path.dirname(filePath), { recursive: true });
-  await fs.writeFile(filePath, JSON.stringify(data, null, 2) + '\n', 'utf-8');
-}
-
 /**
  * Detect indentation style from file content.
  * Returns formatting options matching the file's existing style.
@@ -156,17 +121,11 @@ async function mergeJsoncFile(
   }
 
   if (raw.trim().length === 0) {
-    const config: any = {};
-    let parent: any = config;
-    for (let i = 0; i < keyPath.length; i++) {
-      if (i === keyPath.length - 1) {
-        parent[keyPath[i]] = value;
-      } else {
-        parent[keyPath[i]] = {};
-        parent = parent[keyPath[i]];
-      }
-    }
-    await writeJsonFile(filePath, config);
+    await fs.mkdir(path.dirname(filePath), { recursive: true });
+    const formattingOptions = { tabSize: 2, insertSpaces: true };
+    const edits = modify('{}', keyPath, value, { formattingOptions });
+    const result = applyEdits('{}', edits);
+    await fs.writeFile(filePath, result, 'utf-8');
     return true;
   }
 
@@ -207,10 +166,12 @@ async function setupCursor(result: SetupResult): Promise<void> {
 
   const mcpPath = path.join(cursorDir, 'mcp.json');
   try {
-    const existing = await readJsonFile(mcpPath);
-    const updated = mergeMcpConfig(existing);
-    await writeJsonFile(mcpPath, updated);
-    result.configured.push('Cursor');
+    const ok = await mergeJsoncFile(mcpPath, ['mcpServers', 'gitnexus'], getMcpEntry());
+    if (ok) {
+      result.configured.push('Cursor');
+    } else {
+      result.errors.push('Cursor: mcp.json is corrupt — skipping to preserve existing content');
+    }
   } catch (err: any) {
     result.errors.push(`Cursor: ${err.message}`);
   }
@@ -226,10 +187,14 @@ async function setupClaudeCode(result: SetupResult): Promise<void> {
   // Claude Code stores MCP config in ~/.claude.json
   const mcpPath = path.join(os.homedir(), '.claude.json');
   try {
-    const existing = await readJsonFile(mcpPath);
-    const updated = mergeMcpConfig(existing);
-    await writeJsonFile(mcpPath, updated);
-    result.configured.push('Claude Code');
+    const ok = await mergeJsoncFile(mcpPath, ['mcpServers', 'gitnexus'], getMcpEntry());
+    if (ok) {
+      result.configured.push('Claude Code');
+    } else {
+      result.errors.push(
+        'Claude Code: .claude.json is corrupt — skipping to preserve existing content',
+      );
+    }
   } catch (err: any) {
     result.errors.push(`Claude Code: ${err.message}`);
   }
@@ -253,9 +218,90 @@ async function installClaudeCodeSkills(result: SetupResult): Promise<void> {
   }
 }
 
+/**
+ * Check whether an event array already contains a gitnexus-hook entry.
+ */
+function hasGitnexusHook(hooksObj: any, eventName: string): boolean {
+  const entries = hooksObj?.[eventName];
+  if (!Array.isArray(entries)) return false;
+  return entries.some(
+    (h: any) =>
+      Array.isArray(h.hooks) &&
+      h.hooks.some(
+        (hh: any) => typeof hh.command === 'string' && hh.command.includes('gitnexus-hook'),
+      ),
+  );
+}
+
+/**
+ * Merge hook entries into a JSONC settings file, preserving comments and formatting.
+ * Uses chained modify()+applyEdits() calls to append to arrays without a full
+ * JSON.stringify roundtrip that would strip comments.
+ */
+async function mergeHooksJsonc(
+  filePath: string,
+  entries: Array<{ eventName: string; value: unknown }>,
+): Promise<boolean> {
+  let raw: string;
+  try {
+    raw = await fs.readFile(filePath, 'utf-8');
+  } catch {
+    raw = '';
+  }
+
+  if (raw.trim().length === 0) {
+    await fs.mkdir(path.dirname(filePath), { recursive: true });
+    const hooks: any = {};
+    for (const { eventName, value } of entries) {
+      hooks[eventName] = [value];
+    }
+    const formattingOptions = { tabSize: 2, insertSpaces: true };
+    const edits = modify('{}', ['hooks'], hooks, { formattingOptions });
+    await fs.writeFile(filePath, applyEdits('{}', edits), 'utf-8');
+    return true;
+  }
+
+  const parseErrors: ParseError[] = [];
+  const tree = parseTree(raw, parseErrors);
+
+  if (!tree || tree.type !== 'object' || parseErrors.length > 0) {
+    return false;
+  }
+
+  const formattingOptions = detectIndentation(raw);
+  let current = raw;
+
+  for (const { eventName, value } of entries) {
+    // Re-parse after each edit to get a fresh insertion index.
+    const currentTree = parseTree(current, []);
+    const hooksNode = currentTree?.children?.find(
+      (c) => c.type === 'property' && c.children?.[0]?.value === 'hooks',
+    );
+    const eventNode = hooksNode?.children?.[1]?.children?.find(
+      (c: any) => c.type === 'property' && c.children?.[0]?.value === eventName,
+    );
+
+    let insertIndex: number;
+    if (eventNode?.children?.[1] && Array.isArray(eventNode.children[1].children)) {
+      insertIndex = eventNode.children[1].children.length;
+    } else {
+      insertIndex = 0;
+    }
+
+    const edits = modify(current, ['hooks', eventName, insertIndex], value, {
+      formattingOptions,
+    });
+    current = applyEdits(current, edits);
+  }
+
+  await fs.writeFile(filePath, current, 'utf-8');
+  return true;
+}
+
 /**
  * Install GitNexus hooks to ~/.claude/settings.json for Claude Code.
- * Merges hook config without overwriting existing hooks.
+ * Merges hook config without overwriting existing hooks, preserving
+ * comments and formatting in the JSONC file.
  */
 async function installClaudeCodeHooks(result: SetupResult): Promise<void> {
   const claudeDir = path.join(os.homedir(), '.claude');
@@ -276,8 +322,6 @@ async function installClaudeCodeHooks(result: SetupResult): Promise<void> {
     const dest = path.join(destHooksDir, 'gitnexus-hook.cjs');
     try {
       let content = await fs.readFile(src, 'utf-8');
-      // Inject resolved CLI path so the copied hook can find the CLI
-      // even when it's no longer inside the npm package tree
       const resolvedCli = path.join(__dirname, '..', 'cli', 'index.js');
       const normalizedCli = path.resolve(resolvedCli).replace(/\\/g, '/');
       const jsonCli = JSON.stringify(normalizedCli);
@@ -293,40 +337,67 @@ async function installClaudeCodeHooks(result: SetupResult): Promise<void> {
     const hookPath = path.join(destHooksDir, 'gitnexus-hook.cjs').replace(/\\/g, '/');
     const hookCmd = `node "${hookPath.replace(/"/g, '\\"')}"`;
 
-    // Merge hook config into ~/.claude/settings.json
-    const existing = (await readJsonFile(settingsPath)) || {};
-    if (!existing.hooks) existing.hooks = {};
+    // Check which hook events need entries (idempotent: skip if already registered)
+    const parsed = await (async () => {
+      try {
+        const r = await fs.readFile(settingsPath, 'utf-8');
+        return parseJsonc(r);
+      } catch {
+        return null;
+      }
+    })();
+
+    const hookEntries: Array<{ eventName: string; value: unknown }> = [];
 
     // NOTE: SessionStart hooks are broken on Windows (Claude Code bug #23576).
     // Session context is delivered via CLAUDE.md / skills instead.
 
-    // Helper: add a hook entry if one with 'gitnexus-hook' isn't already registered
-    interface HookEntry {
-      hooks?: Array<{ command?: string }>;
+    if (!hasGitnexusHook(parsed?.hooks, 'PreToolUse')) {
+      hookEntries.push({
+        eventName: 'PreToolUse',
+        value: {
+          matcher: 'Grep|Glob|Bash',
+          hooks: [
+            {
+              type: 'command',
+              command: hookCmd,
+              timeout: 10,
+              statusMessage: 'Enriching with GitNexus graph context...',
+            },
+          ],
+        },
+      });
     }
-    function ensureHookEntry(
-      eventName: string,
-      matcher: string,
-      timeout: number,
-      statusMessage: string,
-    ) {
-      if (!existing.hooks[eventName]) existing.hooks[eventName] = [];
-      const hasHook = existing.hooks[eventName].some((h: HookEntry) =>
-        h.hooks?.some((hh) => hh.command?.includes('gitnexus-hook')),
-      );
-      if (!hasHook) {
-        existing.hooks[eventName].push({
-          matcher,
-          hooks: [{ type: 'command', command: hookCmd, timeout, statusMessage }],
-        });
-      }
+    if (!hasGitnexusHook(parsed?.hooks, 'PostToolUse')) {
+      hookEntries.push({
+        eventName: 'PostToolUse',
+        value: {
+          matcher: 'Bash',
+          hooks: [
+            {
+              type: 'command',
+              command: hookCmd,
+              timeout: 10,
+              statusMessage: 'Checking GitNexus index freshness...',
+            },
+          ],
+        },
+      });
     }
 
-    ensureHookEntry('PreToolUse', 'Grep|Glob|Bash', 10, 'Enriching with GitNexus graph context...');
-    ensureHookEntry('PostToolUse', 'Bash', 10, 'Checking GitNexus index freshness...');
+    if (hookEntries.length === 0) {
+      result.configured.push('Claude Code hooks (already configured)');
+      return;
+    }
 
-    await writeJsonFile(settingsPath, existing);
-    result.configured.push('Claude Code hooks (PreToolUse, PostToolUse)');
+    const ok = await mergeHooksJsonc(settingsPath, hookEntries);
+    if (ok) {
+      result.configured.push('Claude Code hooks (PreToolUse, PostToolUse)');
+    } else {
+      result.errors.push(
+        'Claude Code hooks: settings.json is corrupt — skipping to preserve existing content',
+      );
+    }
   } catch (err: any) {
     result.errors.push(`Claude Code hooks: ${err.message}`);
   }