mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-17 23:52:36 +00:00
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.
This commit is contained in:
parent
8e57b93a73
commit
e7f74e6143
2 changed files with 26 additions and 3 deletions
|
|
@ -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<void> {
|
|||
const parsed = await (async () => {
|
||||
try {
|
||||
const r = await fs.readFile(settingsPath, 'utf-8');
|
||||
return JSON.parse(r);
|
||||
return parseJsonc(r);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue