fix: stop injecting managed hooks into claude settings

This commit is contained in:
Affaan Mustafa
2026-04-12 22:39:48 -07:00
parent 28edd197c2
commit 2ece2cfc90
5 changed files with 47 additions and 196 deletions

View File

@@ -331,7 +331,7 @@ function runTests() {
assert.ok(result.stderr.includes('Unknown install module: ghost-module'));
})) passed++; else failed++;
if (test('merges hooks into settings.json for claude target install', () => {
if (test('installs claude hooks without generating settings.json', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
@@ -341,15 +341,7 @@ function runTests() {
const claudeRoot = path.join(homeDir, '.claude');
assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should be copied');
const settingsPath = path.join(claudeRoot, 'settings.json');
assert.ok(fs.existsSync(settingsPath), 'settings.json should exist after install');
const settings = readJson(settingsPath);
assert.ok(settings.hooks, 'settings.json should contain hooks key');
assert.ok(settings.hooks.PreToolUse, 'hooks should include PreToolUse');
assert.ok(Array.isArray(settings.hooks.PreToolUse), 'PreToolUse should be an array');
assert.ok(settings.hooks.PreToolUse.length > 0, 'PreToolUse should have entries');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'settings.json')), 'settings.json should not be created just to install managed hooks');
} finally {
cleanup(homeDir);
cleanup(projectDir);
@@ -365,23 +357,11 @@ function runTests() {
assert.strictEqual(result.code, 0, result.stderr);
const claudeRoot = path.join(homeDir, '.claude');
const settings = readJson(path.join(claudeRoot, 'settings.json'));
const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json'));
const normSep = (s) => s.replace(/\\/g, '/');
const expectedFragment = normSep(path.join(claudeRoot, 'scripts', 'hooks', 'auto-tmux-dev.js'));
const autoTmuxEntry = settings.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev');
assert.ok(autoTmuxEntry, 'settings.json should include the auto tmux hook');
assert.ok(
normSep(autoTmuxEntry.hooks[0].command).includes(expectedFragment),
'settings.json should use the installed Claude root for hook commands'
);
assert.ok(
!autoTmuxEntry.hooks[0].command.includes('${CLAUDE_PLUGIN_ROOT}'),
'settings.json should not retain CLAUDE_PLUGIN_ROOT placeholders after install'
);
const installedAutoTmuxEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:auto-tmux-dev');
assert.ok(installedAutoTmuxEntry, 'hooks/hooks.json should include the auto tmux hook');
assert.ok(
@@ -398,7 +378,7 @@ function runTests() {
}
})) passed++; else failed++;
if (test('preserves existing settings fields and hook entries when merging hooks', () => {
if (test('preserves existing settings.json without mutating it during claude install', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
@@ -423,17 +403,15 @@ function runTests() {
const settings = readJson(path.join(claudeRoot, 'settings.json'));
assert.strictEqual(settings.effortLevel, 'high', 'existing effortLevel should be preserved');
assert.deepStrictEqual(settings.env, { MY_VAR: '1' }, 'existing env should be preserved');
assert.ok(settings.hooks, 'hooks should be merged in');
assert.ok(settings.hooks.PreToolUse, 'PreToolUse hooks should exist');
assert.ok(
settings.hooks.PreToolUse.some(entry => JSON.stringify(entry).includes('echo custom-pretool')),
'existing PreToolUse entries should be preserved'
);
assert.ok(settings.hooks.PreToolUse.length > 1, 'ECC PreToolUse hooks should be appended');
assert.deepStrictEqual(
settings.hooks.UserPromptSubmit,
[{ matcher: '*', hooks: [{ type: 'command', command: 'echo custom-submit' }] }],
'user-defined hook event types should be preserved'
'existing hooks should be left untouched'
);
assert.deepStrictEqual(
settings.hooks.PreToolUse,
[{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo custom-pretool' }] }],
'managed Claude hooks should not be injected into settings.json'
);
} finally {
cleanup(homeDir);
@@ -515,7 +493,7 @@ function runTests() {
}
})) passed++; else failed++;
if (test('reinstall does not duplicate managed hook entries', () => {
if (test('reinstall does not create settings.json when only managed hooks are installed', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
@@ -523,69 +501,43 @@ function runTests() {
const firstInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(firstInstall.code, 0, firstInstall.stderr);
const settingsPath = path.join(homeDir, '.claude', 'settings.json');
const afterFirstInstall = readJson(settingsPath);
const preToolUseLength = afterFirstInstall.hooks.PreToolUse.length;
const secondInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(secondInstall.code, 0, secondInstall.stderr);
const afterSecondInstall = readJson(settingsPath);
assert.strictEqual(
afterSecondInstall.hooks.PreToolUse.length,
preToolUseLength,
'managed hook entries should not duplicate on reinstall'
);
assert.ok(!fs.existsSync(path.join(homeDir, '.claude', 'settings.json')));
} finally {
cleanup(homeDir);
cleanup(projectDir);
}
})) passed++; else failed++;
if (test('reinstall deduplicates legacy hooks without ids against new managed ids', () => {
if (test('reinstall leaves pre-existing hook-based settings.json untouched', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
try {
const firstInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(firstInstall.code, 0, firstInstall.stderr);
const settingsPath = path.join(homeDir, '.claude', 'settings.json');
const afterFirstInstall = readJson(settingsPath);
const legacySettings = JSON.parse(JSON.stringify(afterFirstInstall));
for (const entries of Object.values(legacySettings.hooks)) {
if (!Array.isArray(entries)) {
continue;
}
for (const entry of entries) {
delete entry.id;
}
}
const claudeRoot = path.join(homeDir, '.claude');
fs.mkdirSync(claudeRoot, { recursive: true });
const settingsPath = path.join(claudeRoot, 'settings.json');
const legacySettings = {
hooks: {
PreToolUse: [{ matcher: 'Write', hooks: [{ type: 'command', command: 'echo legacy-pretool' }] }],
},
};
fs.writeFileSync(settingsPath, JSON.stringify(legacySettings, null, 2));
const legacyPreToolUseLength = legacySettings.hooks.PreToolUse.length;
const secondInstall = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(secondInstall.code, 0, secondInstall.stderr);
const afterSecondInstall = readJson(settingsPath);
assert.strictEqual(
afterSecondInstall.hooks.PreToolUse.length,
legacyPreToolUseLength,
'legacy hook installs should not duplicate when ids are introduced'
);
assert.ok(
afterSecondInstall.hooks.PreToolUse.every(entry => entry && typeof entry === 'object'),
'merged hook entries should remain valid objects'
);
assert.deepStrictEqual(afterSecondInstall, legacySettings);
} finally {
cleanup(homeDir);
cleanup(projectDir);
}
})) passed++; else failed++;
if (test('fails when existing settings.json is malformed', () => {
if (test('ignores malformed existing settings.json during claude install', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
@@ -596,18 +548,17 @@ function runTests() {
fs.writeFileSync(settingsPath, '{ invalid json\n');
const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('Failed to parse existing settings at'));
assert.strictEqual(result.code, 0, result.stderr);
assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '{ invalid json\n');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied');
assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written');
} finally {
cleanup(homeDir);
cleanup(projectDir);
}
})) passed++; else failed++;
if (test('fails when existing settings.json root is not an object', () => {
if (test('ignores non-object existing settings.json during claude install', () => {
const homeDir = createTempDir('install-apply-home-');
const projectDir = createTempDir('install-apply-project-');
@@ -618,12 +569,10 @@ function runTests() {
fs.writeFileSync(settingsPath, '[]\n');
const result = run(['--profile', 'core'], { cwd: projectDir, homeDir });
assert.strictEqual(result.code, 1);
assert.ok(result.stderr.includes('Invalid existing settings at'));
assert.ok(result.stderr.includes('expected a JSON object'));
assert.strictEqual(result.code, 0, result.stderr);
assert.strictEqual(fs.readFileSync(settingsPath, 'utf8'), '[]\n');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should not be copied on validation failure');
assert.ok(!fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should not be written on validation failure');
assert.ok(fs.existsSync(path.join(claudeRoot, 'hooks', 'hooks.json')), 'hooks.json should still be copied');
assert.ok(fs.existsSync(path.join(claudeRoot, 'ecc', 'install-state.json')), 'install state should still be written');
} finally {
cleanup(homeDir);
cleanup(projectDir);