From 8e57b93a731bb46c20723e1b67de0213d95493a3 Mon Sep 17 00:00:00 2001 From: Tom Hale Date: Wed, 22 Apr 2026 23:41:10 +0700 Subject: [PATCH] refactor(setup): migrate all config I/O to mergeJsoncFile, eliminate readJsonFile/writeJsonFile roundtrip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three remaining readJsonFile→JSON.stringify writeback sites now use jsonc-parser's modify()+applyEdits() instead, which preserves comments, formatting, and indentation in user config files: - setupCursor: mergeJsoncFile(mcpPath, ['mcpServers','gitnexus'], ...) - setupClaudeCode: same pattern for ~/.claude.json - installClaudeCodeHooks: new mergeHooksJsonc() chains multiple modify() calls to append to hooks.PreToolUse/PostToolUse arrays without destroying comments in settings.json - mergeJsoncFile empty-file path: replaced writeJsonFile(config) with modify('{}', keyPath, value)+applyEdits for consistent formatting Dead code removed: readJsonFile, mergeMcpConfig, writeJsonFile. Future-proofing: Cursor mcp.json and Claude Code settings.json don't support JSONC comments today, but both have open feature requests (Cursor forum bug #92529, Claude Code #17968 with 58 👍). When they add JSONC support, this code is already safe — it won't strip comments on roundtrip. The migrate-before-they-support approach avoids a silent data-loss regression the day those parsers gain comment awareness. --- gitnexus/src/cli/setup.ts | 237 +++++++++++++------- gitnexus/test/unit/setup-jsonc.test.ts | 297 +++++++++++++++++++++++++ gitnexus/test/unit/setup.test.ts | 14 +- 3 files changed, 454 insertions(+), 94 deletions(-) diff --git a/gitnexus/src/cli/setup.ts b/gitnexus/src/cli/setup.ts index e6f20d3f9..b565d51b5 100644 --- a/gitnexus/src/cli/setup.ts +++ b/gitnexus/src/cli/setup.ts @@ -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 { - 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 { - 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 { 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 { // 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,88 @@ async function installClaudeCodeSkills(result: SetupResult): Promise { } } +/** + * 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 { + 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) { + const hooksNode = tree.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 { const claudeDir = path.join(os.homedir(), '.claude'); @@ -276,8 +320,6 @@ async function installClaudeCodeHooks(result: SetupResult): Promise { 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 +335,67 @@ async function installClaudeCodeHooks(result: SetupResult): Promise { 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 JSON.parse(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 (!hasGitnexusHook(parsed?.hooks, 'PostToolUse')) { + hookEntries.push({ + eventName: 'PostToolUse', + value: { + matcher: 'Bash', + hooks: [ + { + type: 'command', + command: hookCmd, + timeout: 10, + statusMessage: 'Checking GitNexus index freshness...', + }, + ], + }, + }); + } + + if (hookEntries.length === 0) { + result.configured.push('Claude Code hooks (already configured)'); + return; + } + + 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', ); - if (!hasHook) { - existing.hooks[eventName].push({ - matcher, - hooks: [{ type: 'command', command: hookCmd, timeout, statusMessage }], - }); - } } - - ensureHookEntry('PreToolUse', 'Grep|Glob|Bash', 10, 'Enriching with GitNexus graph context...'); - ensureHookEntry('PostToolUse', 'Bash', 10, 'Checking GitNexus index freshness...'); - - await writeJsonFile(settingsPath, existing); - result.configured.push('Claude Code hooks (PreToolUse, PostToolUse)'); } catch (err: any) { result.errors.push(`Claude Code hooks: ${err.message}`); } diff --git a/gitnexus/test/unit/setup-jsonc.test.ts b/gitnexus/test/unit/setup-jsonc.test.ts index 187f49d95..c0773f036 100644 --- a/gitnexus/test/unit/setup-jsonc.test.ts +++ b/gitnexus/test/unit/setup-jsonc.test.ts @@ -271,3 +271,300 @@ describe('setupOpenCode — JSONC preservation', () => { await expect(fs.access(opencodeJsonPath())).rejects.toThrow(); }); }); + +describe('setupCursor — JSONC preservation', () => { + let tempHome: string; + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + let platformDescriptor: PropertyDescriptor | undefined; + + const setPlatform = (value: NodeJS.Platform) => { + Object.defineProperty(process, 'platform', { + value, + configurable: true, + }); + }; + + const cursorDir = () => path.join(tempHome, '.cursor'); + const mcpPath = () => path.join(cursorDir(), 'mcp.json'); + + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + + originalHome = process.env.HOME; + originalUserProfile = process.env.USERPROFILE; + tempHome = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-cursor-jsonc-')); + process.env.HOME = tempHome; + process.env.USERPROFILE = tempHome; + + await fs.mkdir(cursorDir(), { recursive: true }); + + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + setPlatform('linux'); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor); + } + + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + await fs.rm(tempHome, { recursive: true, force: true }); + }); + + it('creates fresh mcp.json when missing', async () => { + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + const config = JSON.parse(raw); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('preserves existing mcpServers and comments', async () => { + const jsonc = `{ + // my cursor config + "mcpServers": { + "other": { "command": "keep" } + } +}`; + await fs.writeFile(mcpPath(), jsonc, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + expect(raw).toContain('my cursor config'); + const config = parseJsonc(raw); + expect(config.mcpServers.other).toEqual({ command: 'keep' }); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('does not wipe corrupt file', async () => { + const corrupt = '{ "mcpServers": broken'; + await fs.writeFile(mcpPath(), corrupt, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + expect(raw).toBe(corrupt); + }); +}); + +describe('setupClaudeCode — JSONC preservation', () => { + let tempHome: string; + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + let platformDescriptor: PropertyDescriptor | undefined; + + const setPlatform = (value: NodeJS.Platform) => { + Object.defineProperty(process, 'platform', { + value, + configurable: true, + }); + }; + + const claudeDir = () => path.join(tempHome, '.claude'); + const mcpPath = () => path.join(tempHome, '.claude.json'); + + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + + originalHome = process.env.HOME; + originalUserProfile = process.env.USERPROFILE; + tempHome = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-claude-jsonc-')); + process.env.HOME = tempHome; + process.env.USERPROFILE = tempHome; + + await fs.mkdir(claudeDir(), { recursive: true }); + + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + setPlatform('linux'); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor); + } + + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + await fs.rm(tempHome, { recursive: true, force: true }); + }); + + it('creates fresh .claude.json when missing', async () => { + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + const config = JSON.parse(raw); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('preserves existing keys and comments', async () => { + const jsonc = `{ + // my claude config + "permissions": ["read"], + "mcpServers": { + "other": { "command": "keep" } + } +}`; + await fs.writeFile(mcpPath(), jsonc, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + expect(raw).toContain('my claude config'); + const config = parseJsonc(raw); + expect(config.permissions).toEqual(['read']); + expect(config.mcpServers.other).toEqual({ command: 'keep' }); + expect(config.mcpServers.gitnexus).toBeDefined(); + }); + + it('does not wipe corrupt file', async () => { + const corrupt = '{ "permissions": broken'; + await fs.writeFile(mcpPath(), corrupt, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(mcpPath(), 'utf-8'); + expect(raw).toBe(corrupt); + }); +}); + +describe('installClaudeCodeHooks — JSONC preservation', () => { + let tempHome: string; + let originalHome: string | undefined; + let originalUserProfile: string | undefined; + let platformDescriptor: PropertyDescriptor | undefined; + + const setPlatform = (value: NodeJS.Platform) => { + Object.defineProperty(process, 'platform', { + value, + configurable: true, + }); + }; + + const claudeDir = () => path.join(tempHome, '.claude'); + const settingsPath = () => path.join(claudeDir(), 'settings.json'); + + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + + originalHome = process.env.HOME; + originalUserProfile = process.env.USERPROFILE; + tempHome = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-hooks-jsonc-')); + process.env.HOME = tempHome; + process.env.USERPROFILE = tempHome; + + await fs.mkdir(claudeDir(), { recursive: true }); + + platformDescriptor = Object.getOwnPropertyDescriptor(process, 'platform'); + setPlatform('linux'); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + + if (platformDescriptor) { + Object.defineProperty(process, 'platform', platformDescriptor); + } + + process.env.HOME = originalHome; + process.env.USERPROFILE = originalUserProfile; + await fs.rm(tempHome, { recursive: true, force: true }); + }); + + it('creates fresh settings.json with hooks when missing', async () => { + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(settingsPath(), 'utf-8'); + const config = JSON.parse(raw); + expect(config.hooks.PreToolUse).toBeDefined(); + expect(config.hooks.PostToolUse).toBeDefined(); + expect(config.hooks.PreToolUse[0].matcher).toBe('Grep|Glob|Bash'); + }); + + it('appends hooks to existing settings preserving comments', async () => { + const jsonc = `{ + // my settings + "permissions": ["read"], + "hooks": { + "PreToolUse": [ + { "matcher": "Write", "hooks": [{ "type": "command", "command": "other-hook" }] } + ] + } +}`; + await fs.writeFile(settingsPath(), jsonc, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(settingsPath(), 'utf-8'); + expect(raw).toContain('my settings'); + const config = parseJsonc(raw); + expect(config.permissions).toEqual(['read']); + expect(config.hooks.PreToolUse.length).toBe(2); + expect(config.hooks.PreToolUse[0].matcher).toBe('Write'); + expect(config.hooks.PreToolUse[1].hooks[0].command).toContain('gitnexus-hook'); + expect(config.hooks.PostToolUse).toBeDefined(); + }); + + it('does not add duplicate gitnexus-hook entries', async () => { + const jsonc = JSON.stringify( + { + hooks: { + PreToolUse: [ + { + matcher: 'Grep|Glob|Bash', + hooks: [{ type: 'command', command: 'node "gitnexus-hook.cjs"', timeout: 10 }], + }, + ], + PostToolUse: [ + { + matcher: 'Bash', + hooks: [{ type: 'command', command: 'node "gitnexus-hook.cjs"', timeout: 10 }], + }, + ], + }, + }, + null, + 2, + ); + await fs.writeFile(settingsPath(), jsonc, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(settingsPath(), 'utf-8'); + const config = JSON.parse(raw); + expect(config.hooks.PreToolUse.length).toBe(1); + expect(config.hooks.PostToolUse.length).toBe(1); + }); + + it('does not wipe corrupt file', async () => { + const corrupt = '{ "hooks": broken'; + await fs.writeFile(settingsPath(), corrupt, 'utf-8'); + + const { setupCommand } = await import('../../src/cli/setup.js'); + await setupCommand(); + + const raw = await fs.readFile(settingsPath(), 'utf-8'); + expect(raw).toBe(corrupt); + }); +}); diff --git a/gitnexus/test/unit/setup.test.ts b/gitnexus/test/unit/setup.test.ts index a8e05fa79..4544ddb02 100644 --- a/gitnexus/test/unit/setup.test.ts +++ b/gitnexus/test/unit/setup.test.ts @@ -141,21 +141,15 @@ describe('setupClaudeCode', () => { it('handles corrupt JSON gracefully', async () => { setPlatform('linux'); - await fs.writeFile( - path.join(tempHome, '.claude.json'), - '{ this is not valid json !!!', - 'utf-8', - ); + const corrupt = '{ this is not valid json !!!'; + await fs.writeFile(path.join(tempHome, '.claude.json'), corrupt, 'utf-8'); const { setupCommand } = await import('../../src/cli/setup.js'); await setupCommand(); - // readJsonFile returns null on invalid JSON, so mergeMcpConfig - // creates a fresh config — the file should now be valid. + // mergeJsoncFile leaves corrupt files untouched (safer than overwriting) const raw = await fs.readFile(path.join(tempHome, '.claude.json'), 'utf-8'); - const config = JSON.parse(raw); - - expect(config.mcpServers.gitnexus).toBeDefined(); + expect(raw).toBe(corrupt); }); it('uses global binary path when gitnexus is on PATH', async () => {