From e7f74e61435ea55f82ba8d6abeff59d215a9ee62 Mon Sep 17 00:00:00 2001 From: Tom Hale Date: Thu, 23 Apr 2026 00:14:59 +0700 Subject: [PATCH] fix(setup): use parseJsonc for idempotency check, re-parse tree after each edit Two fixes from Copilot review on #1031: 1. installClaudeCodeHooks used JSON.parse() for the idempotency check on settings.json. If the file contained JSONC comments, parse would fail, parsed became null, hasGitnexusHook returned false, and duplicate hook entries could be appended. Now uses jsonc-parser's parse() instead. 2. mergeHooksJsonc computed insertIndex from the original tree and never updated it after applying edits. If two entries targeted the same eventName, the second iteration used a stale index and overwrote the previously-inserted element. Now re-parses the tree inside the loop so each iteration gets a fresh insertion index. --- gitnexus/src/cli/setup.ts | 8 +++++--- gitnexus/test/unit/setup-jsonc.test.ts | 21 +++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/gitnexus/src/cli/setup.ts b/gitnexus/src/cli/setup.ts index b565d51b5..e4408552b 100644 --- a/gitnexus/src/cli/setup.ts +++ b/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); @@ -272,7 +272,9 @@ async function mergeHooksJsonc( let current = raw; for (const { eventName, value } of entries) { - const hooksNode = tree.children?.find( + // 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( @@ -339,7 +341,7 @@ async function installClaudeCodeHooks(result: SetupResult): Promise { const parsed = await (async () => { try { const r = await fs.readFile(settingsPath, 'utf-8'); - return JSON.parse(r); + return parseJsonc(r); } catch { return null; } diff --git a/gitnexus/test/unit/setup-jsonc.test.ts b/gitnexus/test/unit/setup-jsonc.test.ts index c0773f036..3993b1218 100644 --- a/gitnexus/test/unit/setup-jsonc.test.ts +++ b/gitnexus/test/unit/setup-jsonc.test.ts @@ -567,4 +567,25 @@ describe('installClaudeCodeHooks — JSONC preservation', () => { const raw = await fs.readFile(settingsPath(), 'utf-8'); expect(raw).toBe(corrupt); }); + + it('handles idempotency check with JSONC comments in settings', async () => { + const jsonc = `{ + // settings comment + "hooks": { + "PreToolUse": [ + { "matcher": "Grep|Glob|Bash", "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('settings comment'); + const config = parseJsonc(raw); + expect(config.hooks.PreToolUse.length).toBe(2); + expect(config.hooks.PostToolUse.length).toBe(1); + }); });