fix: consolidate bash hooks without fork storms

This commit is contained in:
Affaan Mustafa
2026-04-14 21:23:57 -07:00
parent 7eb7c598fb
commit 1fabf4d2cf
15 changed files with 605 additions and 279 deletions

View File

@@ -0,0 +1,114 @@
/**
* Tests for consolidated Bash hook dispatchers.
*/
const assert = require('assert');
const fs = require('fs');
const os = require('os');
const path = require('path');
const { spawnSync } = require('child_process');
const preDispatcher = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'pre-bash-dispatcher.js');
const postDispatcher = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'post-bash-dispatcher.js');
function test(name, fn) {
try {
fn();
console.log(`${name}`);
return true;
} catch (error) {
console.log(`${name}`);
console.log(` Error: ${error.message}`);
return false;
}
}
function runScript(scriptPath, input, env = {}) {
return spawnSync('node', [scriptPath], {
input: typeof input === 'string' ? input : JSON.stringify(input),
encoding: 'utf8',
env: {
...process.env,
...env,
},
timeout: 10000,
});
}
function runTests() {
console.log('\n=== Testing Bash hook dispatchers ===\n');
let passed = 0;
let failed = 0;
if (test('pre dispatcher blocks --no-verify before other Bash checks', () => {
const input = { tool_input: { command: 'git commit --no-verify -m "x"' } };
const result = runScript(preDispatcher, input, { ECC_HOOK_PROFILE: 'strict' });
assert.strictEqual(result.status, 2, 'Expected dispatcher to block git hook bypass');
assert.ok(result.stderr.includes('--no-verify'), 'Expected block-no-verify reason in stderr');
assert.strictEqual(result.stdout, '', 'Blocking hook should not pass through stdout');
})) passed++; else failed++;
if (test('pre dispatcher still honors per-hook disable flags', () => {
const input = { tool_input: { command: 'git push origin main' } };
const enabled = runScript(preDispatcher, input, { ECC_HOOK_PROFILE: 'strict' });
assert.strictEqual(enabled.status, 0);
assert.ok(enabled.stderr.includes('Review changes before push'), 'Expected git push reminder when enabled');
const disabled = runScript(preDispatcher, input, {
ECC_HOOK_PROFILE: 'strict',
ECC_DISABLED_HOOKS: 'pre:bash:git-push-reminder',
});
assert.strictEqual(disabled.status, 0);
assert.ok(!disabled.stderr.includes('Review changes before push'), 'Disabled hook should not emit reminder');
})) passed++; else failed++;
if (test('pre dispatcher respects hook profiles inside the consolidated path', () => {
const input = { tool_input: { command: 'git push origin main' } };
const result = runScript(preDispatcher, input, { ECC_HOOK_PROFILE: 'minimal' });
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stderr, '', 'Strict-only reminders should stay disabled in minimal profile');
assert.strictEqual(result.stdout, JSON.stringify(input));
})) passed++; else failed++;
if (test('post dispatcher writes both bash audit and cost logs in one pass', () => {
const homeDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-bash-dispatcher-'));
const payload = { tool_input: { command: 'npm publish --token=$PUBLISH_TOKEN' } };
try {
const result = runScript(postDispatcher, payload, {
HOME: homeDir,
USERPROFILE: homeDir,
});
assert.strictEqual(result.status, 0);
assert.strictEqual(result.stdout, JSON.stringify(payload));
const auditLog = fs.readFileSync(path.join(homeDir, '.claude', 'bash-commands.log'), 'utf8');
const costLog = fs.readFileSync(path.join(homeDir, '.claude', 'cost-tracker.log'), 'utf8');
assert.ok(auditLog.includes('--token=<REDACTED>'));
assert.ok(costLog.includes('tool=Bash command=npm publish --token=<REDACTED>'));
assert.ok(!auditLog.includes('$PUBLISH_TOKEN'));
assert.ok(!costLog.includes('$PUBLISH_TOKEN'));
} finally {
fs.rmSync(homeDir, { recursive: true, force: true });
}
})) passed++; else failed++;
if (test('post dispatcher preserves PR-created hints after consolidated execution', () => {
const payload = {
tool_input: { command: 'gh pr create --title "Fix bug" --body "desc"' },
tool_output: { output: 'https://github.com/owner/repo/pull/42\n' },
};
const result = runScript(postDispatcher, payload);
assert.strictEqual(result.status, 0);
assert.ok(result.stderr.includes('PR created: https://github.com/owner/repo/pull/42'));
assert.ok(result.stderr.includes('gh pr review 42 --repo owner/repo'));
})) passed++; else failed++;
console.log(`\nResults: Passed: ${passed}, Failed: ${failed}`);
process.exit(failed > 0 ? 1 : 0);
}
runTests();

View File

@@ -1888,6 +1888,33 @@ async function runTests() {
passed++;
else failed++;
if (
test('hooks.json consolidates Bash hooks into one pre and one post dispatcher', () => {
const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json');
const hooks = JSON.parse(fs.readFileSync(hooksPath, 'utf8'));
const preBash = hooks.hooks.PreToolUse.filter(entry => entry.matcher === 'Bash');
const postBash = hooks.hooks.PostToolUse.filter(entry => entry.matcher === 'Bash');
assert.strictEqual(preBash.length, 1, 'Should have exactly one PreToolUse Bash dispatcher');
assert.strictEqual(postBash.length, 1, 'Should have exactly one PostToolUse Bash dispatcher');
assert.strictEqual(preBash[0].id, 'pre:bash:dispatcher');
assert.strictEqual(postBash[0].id, 'post:bash:dispatcher');
const preCommand = Array.isArray(preBash[0].hooks[0].command)
? preBash[0].hooks[0].command.join(' ')
: preBash[0].hooks[0].command;
const postCommand = Array.isArray(postBash[0].hooks[0].command)
? postBash[0].hooks[0].command.join(' ')
: postBash[0].hooks[0].command;
assert.ok(preCommand.includes('pre-bash-dispatcher.js'), 'PreToolUse Bash hook should use the pre dispatcher');
assert.ok(postCommand.includes('post-bash-dispatcher.js'), 'PostToolUse Bash hook should use the post dispatcher');
})
)
passed++;
else failed++;
if (
test('SessionEnd marker hook is async and cleanup-safe', () => {
const hooksPath = path.join(__dirname, '..', '..', 'hooks', 'hooks.json');

View File

@@ -256,6 +256,14 @@ function getHookCommandByDescription(hooks, lifecycle, descriptionText) {
return hookGroup.hooks[0].command;
}
function getHookCommandById(hooks, lifecycle, hookId) {
const hookGroup = hooks.hooks[lifecycle]?.find(entry => entry.id === hookId);
assert.ok(hookGroup, `Expected ${lifecycle} hook with id "${hookId}"`);
assert.ok(hookGroup.hooks?.[0]?.command, `Expected ${lifecycle} hook command for id "${hookId}"`);
return hookGroup.hooks[0].command;
}
// Test suite
async function runTests() {
console.log('\n=== Hook Integration Tests ===\n');
@@ -340,12 +348,7 @@ async function runTests() {
})) passed++; else failed++;
if (await asyncTest('dev server hook transforms command to tmux session', async () => {
// Test the auto-tmux dev hook — transforms dev commands to run in tmux
const hookCommand = getHookCommandByDescription(
hooks,
'PreToolUse',
'Auto-start dev servers in tmux'
);
const hookCommand = getHookCommandById(hooks, 'PreToolUse', 'pre:bash:dispatcher');
const result = await runHookCommand(hookCommand, {
tool_input: { command: 'npm run dev' }
});
@@ -526,12 +529,7 @@ async function runTests() {
})) passed++; else failed++;
if (await asyncTest('dev server hook transforms yarn dev to tmux session', async () => {
// The auto-tmux dev hook transforms dev commands (yarn dev, npm run dev, etc.)
const hookCommand = getHookCommandByDescription(
hooks,
'PreToolUse',
'Auto-start dev servers in tmux'
);
const hookCommand = getHookCommandById(hooks, 'PreToolUse', 'pre:bash:dispatcher');
const result = await runHookCommand(hookCommand, {
tool_input: { command: 'yarn dev' }
});
@@ -663,14 +661,8 @@ async function runTests() {
})) passed++; else failed++;
if (await asyncTest('PostToolUse PR hook extracts PR URL', async () => {
// Find the PR logging hook
const prHook = hooks.hooks.PostToolUse.find(h =>
h.description && h.description.includes('PR URL')
);
assert.ok(prHook, 'PR hook should exist');
const result = await runHookCommand(prHook.hooks[0].command, {
const hookCommand = getHookCommandById(hooks, 'PostToolUse', 'post:bash:dispatcher');
const result = await runHookCommand(hookCommand, {
tool_input: { command: 'gh pr create --title "Test"' },
tool_output: { output: 'Creating pull request...\nhttps://github.com/owner/repo/pull/123' }
});

View File

@@ -361,23 +361,27 @@ function runTests() {
const claudeRoot = path.join(homeDir, '.claude');
const installedHooks = readJson(path.join(claudeRoot, 'hooks', 'hooks.json'));
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(Array.isArray(installedAutoTmuxEntry.hooks[0].command), 'hooks/hooks.json should install argv-form commands for cross-platform safety');
const installedBashDispatcherEntry = installedHooks.hooks.PreToolUse.find(entry => entry.id === 'pre:bash:dispatcher');
assert.ok(installedBashDispatcherEntry, 'hooks/hooks.json should include the consolidated Bash dispatcher hook');
assert.ok(Array.isArray(installedBashDispatcherEntry.hooks[0].command), 'hooks/hooks.json should install argv-form commands for cross-platform safety');
assert.ok(
installedAutoTmuxEntry.hooks[0].command[0] === 'node' && installedAutoTmuxEntry.hooks[0].command[1] === '-e',
installedBashDispatcherEntry.hooks[0].command[0] === 'node' && installedBashDispatcherEntry.hooks[0].command[1] === '-e',
'hooks/hooks.json should use the inline node bootstrap contract'
);
assert.ok(
installedAutoTmuxEntry.hooks[0].command.some(part => String(part).includes('plugin-hook-bootstrap.js')),
installedBashDispatcherEntry.hooks[0].command.some(part => String(part).includes('plugin-hook-bootstrap.js')),
'hooks/hooks.json should route plugin-managed hooks through the shared bootstrap'
);
assert.ok(
installedAutoTmuxEntry.hooks[0].command.some(part => String(part).includes('CLAUDE_PLUGIN_ROOT')),
installedBashDispatcherEntry.hooks[0].command.some(part => String(part).includes('CLAUDE_PLUGIN_ROOT')),
'hooks/hooks.json should still consult CLAUDE_PLUGIN_ROOT for runtime resolution'
);
assert.ok(
!installedAutoTmuxEntry.hooks[0].command.some(part => String(part).includes('${CLAUDE_PLUGIN_ROOT}')),
installedBashDispatcherEntry.hooks[0].command.some(part => String(part).includes('pre-bash-dispatcher.js')),
'hooks/hooks.json should point the Bash preflight contract at the consolidated dispatcher'
);
assert.ok(
!installedBashDispatcherEntry.hooks[0].command.some(part => String(part).includes('${CLAUDE_PLUGIN_ROOT}')),
'hooks/hooks.json should not retain raw CLAUDE_PLUGIN_ROOT shell placeholders after install'
);
} finally {