fix(hooks): reawaken jobs without replacing user hooks

This commit is contained in:
lior
2026-07-28 23:16:37 +03:00
parent 77bcbc9b94
commit 3c903b36ca
5 changed files with 467 additions and 56 deletions
+51 -10
View File
@@ -1,10 +1,10 @@
/**
* COD-91 — `refreshStaleHookSecret` self-heal.
* COD-91 — `refreshStaleCodemanHooks` self-heal.
*
* Making the hook-event secret unconditionally required (PR #127) would silently 401 the
* hook curls baked into cases created before the secret header existed (COD-54). Those
* curls live in `.claude/settings.local.json` and `writeHooksConfig` only runs at case
* CREATION, so existing cases never refresh. `refreshStaleHookSecret` regenerates the
* CREATION, so existing cases never refresh. `refreshStaleCodemanHooks` regenerates the
* hooks block on session spawn — but ONLY when the case already holds Codeman's own
* pre-secret hook curls, never clobbering a user's customizations.
*
@@ -15,7 +15,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { refreshStaleHookSecret } from '../src/hooks-config.js';
import { refreshStaleCodemanHooks } from '../src/hooks-config.js';
const SECRET_HEADER = 'X-Codeman-Hook-Secret';
@@ -41,7 +41,7 @@ function staleCodemanHooks() {
};
}
describe('refreshStaleHookSecret', () => {
describe('refreshStaleCodemanHooks', () => {
let dir: string;
let settingsPath: string;
@@ -60,7 +60,7 @@ describe('refreshStaleHookSecret', () => {
settingsPath,
JSON.stringify({ env: { CLAUDE_CODE_FOO: '1' }, model: 'opus', hooks: staleCodemanHooks() }, null, 2)
);
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
const after = JSON.parse(readFileSync(settingsPath, 'utf-8'));
expect(JSON.stringify(after.hooks)).toContain(SECRET_HEADER);
@@ -73,11 +73,11 @@ describe('refreshStaleHookSecret', () => {
it('leaves a hooks block that already carries the secret unchanged', async () => {
// Seed with a current block by healing a stale one first, then re-heal: second pass must no-op.
writeFileSync(settingsPath, JSON.stringify({ hooks: staleCodemanHooks() }, null, 2));
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
const healed = readFileSync(settingsPath, 'utf-8');
expect(healed).toContain(SECRET_HEADER);
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
expect(readFileSync(settingsPath, 'utf-8')).toBe(healed); // byte-identical: no rewrite
});
@@ -88,19 +88,60 @@ describe('refreshStaleHookSecret', () => {
2
);
writeFileSync(settingsPath, foreign);
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
expect(readFileSync(settingsPath, 'utf-8')).toBe(foreign);
});
it('preserves user handlers and events in a mixed stale configuration', async () => {
const hooks = staleCodemanHooks();
hooks.Stop[0].hooks.push({
type: 'command',
command: './notify-user.sh',
timeout: 10,
});
const customPostToolUse = {
matcher: 'Write',
hooks: [{ type: 'command', command: './format.sh' }],
};
const customEvent = [
{
hooks: [{ type: 'command', command: './audit.sh' }],
},
];
writeFileSync(
settingsPath,
JSON.stringify(
{
hooks: {
...hooks,
PostToolUse: [customPostToolUse],
CustomEvent: customEvent,
},
},
null,
2
)
);
await refreshStaleCodemanHooks(dir);
const after = JSON.parse(readFileSync(settingsPath, 'utf-8'));
expect(JSON.stringify(after.hooks)).toContain(SECRET_HEADER);
expect(JSON.stringify(after.hooks)).toContain('CODEMAN_BACKGROUND_REWAKE_V1');
expect(JSON.stringify(after.hooks.Stop)).toContain('./notify-user.sh');
expect(after.hooks.PostToolUse).toEqual(expect.arrayContaining([customPostToolUse]));
expect(after.hooks.CustomEvent).toEqual(customEvent);
});
it('is a no-op when settings.local.json is absent (does not create one)', async () => {
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
expect(existsSync(settingsPath)).toBe(false);
});
it('leaves a malformed settings file untouched', async () => {
const garbage = '{ not valid json';
writeFileSync(settingsPath, garbage);
await refreshStaleHookSecret(dir);
await refreshStaleCodemanHooks(dir);
expect(readFileSync(settingsPath, 'utf-8')).toBe(garbage);
});
});
+144 -3
View File
@@ -9,7 +9,13 @@ import { describe, it, expect, beforeAll, beforeEach, afterAll, afterEach } from
import { existsSync, readFileSync, writeFileSync, mkdirSync, rmSync } from 'node:fs';
import { join } from 'node:path';
import { tmpdir } from 'node:os';
import { generateHooksConfig, writeHooksConfig } from '../src/hooks-config.js';
import { spawn } from 'node:child_process';
import {
generateBackgroundWakeScript,
generateHooksConfig,
refreshStaleCodemanHooks,
writeHooksConfig,
} from '../src/hooks-config.js';
describe('generateHooksConfig', () => {
it('should return an object with hooks key', () => {
@@ -29,6 +35,30 @@ describe('generateHooksConfig', () => {
expect(config.hooks.Stop).toHaveLength(1);
});
it('should configure a self-contained Bash background-task rewake hook', () => {
const config = generateHooksConfig();
const postToolHooks = config.hooks.PostToolUse as Array<{
matcher: string;
hooks: Array<{
type: string;
command: string;
args: string[];
asyncRewake: boolean;
timeout: number;
}>;
}>;
expect(postToolHooks).toHaveLength(1);
expect(postToolHooks[0].matcher).toBe('Bash');
expect(postToolHooks[0].hooks[0]).toMatchObject({
type: 'command',
command: 'node',
asyncRewake: true,
});
expect(postToolHooks[0].hooks[0].args).toEqual(['-e', generateBackgroundWakeScript()]);
expect(postToolHooks[0].hooks[0].timeout).toBeGreaterThanOrEqual(3600);
});
it('should configure idle_prompt matcher', () => {
const config = generateHooksConfig();
const notifHooks = config.hooks.Notification as Array<{ matcher?: string }>;
@@ -157,7 +187,42 @@ describe('writeHooksConfig', () => {
expect(parsed.hooks).toBeDefined();
});
it('should overwrite existing hooks key', async () => {
it('should upgrade Codeman-owned hooks that predate background rewake', async () => {
const claudeDir = join(testDir, '.claude');
const settingsPath = join(claudeDir, 'settings.local.json');
mkdirSync(claudeDir, { recursive: true });
const oldHooks = generateHooksConfig().hooks;
delete oldHooks.PostToolUse;
writeFileSync(settingsPath, JSON.stringify({ hooks: oldHooks }, null, 2));
await refreshStaleCodemanHooks(testDir);
const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8'));
expect(parsed.hooks.PostToolUse).toHaveLength(1);
expect(JSON.stringify(parsed.hooks.PostToolUse)).toContain('CODEMAN_BACKGROUND_REWAKE_V1');
});
it('should not add rewake hooks to a user-owned hook configuration', async () => {
const claudeDir = join(testDir, '.claude');
const settingsPath = join(claudeDir, 'settings.local.json');
mkdirSync(claudeDir, { recursive: true });
const userHooks = {
PostToolUse: [
{
matcher: 'Write',
hooks: [{ type: 'command', command: './format.sh' }],
},
],
};
writeFileSync(settingsPath, JSON.stringify({ hooks: userHooks }, null, 2));
await refreshStaleCodemanHooks(testDir);
const parsed = JSON.parse(readFileSync(settingsPath, 'utf-8'));
expect(parsed.hooks).toEqual(userHooks);
});
it('should preserve user hook events while installing Codeman hooks', async () => {
const claudeDir = join(testDir, '.claude');
mkdirSync(claudeDir, { recursive: true });
writeFileSync(join(claudeDir, 'settings.local.json'), JSON.stringify({ hooks: { oldHook: [] } }, null, 2));
@@ -165,7 +230,7 @@ describe('writeHooksConfig', () => {
await writeHooksConfig(testDir);
const parsed = JSON.parse(readFileSync(join(claudeDir, 'settings.local.json'), 'utf-8'));
expect(parsed.hooks.oldHook).toBeUndefined();
expect(parsed.hooks.oldHook).toEqual([]);
expect(parsed.hooks.Notification).toBeDefined();
});
@@ -187,6 +252,82 @@ describe('writeHooksConfig', () => {
});
});
describe('background task rewake helper', () => {
const testDir = join(tmpdir(), 'codeman-background-rewake-test-' + Date.now());
beforeEach(() => {
mkdirSync(testDir, { recursive: true });
});
afterEach(() => {
rmSync(testDir, { recursive: true, force: true });
});
function runHelper(input: Record<string, unknown>): Promise<{ code: number | null; stderr: string }> {
return new Promise((resolve, reject) => {
const child = spawn(process.execPath, ['-e', generateBackgroundWakeScript()], {
stdio: ['pipe', 'ignore', 'pipe'],
});
let stderr = '';
const timeout = setTimeout(() => {
child.kill();
reject(new Error('background rewake helper timed out'));
}, 5000);
child.stderr.setEncoding('utf8');
child.stderr.on('data', (chunk) => {
stderr += chunk;
});
child.on('error', reject);
child.on('close', (code) => {
clearTimeout(timeout);
resolve({ code, stderr });
});
child.stdin.end(JSON.stringify(input));
});
}
it('exits without waiting for an ordinary Bash result', async () => {
const result = await runHelper({
transcript_path: join(testDir, 'transcript.jsonl'),
tool_response: { stdout: 'ordinary command completed' },
});
expect(result.code).toBe(0);
expect(result.stderr).toBe('');
});
it('exits 2 when the matching background command completes', async () => {
const transcriptPath = join(testDir, 'transcript.jsonl');
writeFileSync(transcriptPath, '');
const resultPromise = runHelper({
transcript_path: transcriptPath,
tool_response: {
stdout: 'Command running in background with ID: bg-test-1. Output is being written to: /tmp/bg-test-1.output.',
},
});
await new Promise((resolve) => setTimeout(resolve, 100));
writeFileSync(
transcriptPath,
JSON.stringify({
type: 'queue-operation',
operation: 'enqueue',
content:
'<task-notification>\n<task-id>bg-test-1</task-id>\n<status>completed</status>\n' +
'<output-file>/tmp/bg-test-1.output</output-file>\n</task-notification>',
}) + '\n'
);
const result = await resultPromise;
expect(result.code).toBe(2);
expect(result.stderr).toContain('bg-test-1');
expect(result.stderr).toContain('completed');
expect(result.stderr).toContain('/tmp/bg-test-1.output');
});
});
// ========== Hook Event API Integration Tests ==========
// Port 3130 reserved for hooks integration tests