mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-04-01 14:43:28 +08:00
perf(hooks): batch format+typecheck at Stop instead of per Edit (#746)
* perf(hooks): batch format+typecheck at Stop instead of per Edit Fixes #735. The per-edit post:edit:format and post:edit:typecheck hooks ran synchronously after every Edit call, adding 15-30s of latency per file — up to 7.5 minutes for a 10-file refactor. New approach: - post-edit-accumulator.js (PostToolUse/Edit): lightweight hook that records each edited JS/TS path to a session-scoped temp file in os.tmpdir(). No formatters, no tsc — exits in microseconds. - stop-format-typecheck.js (Stop): reads the accumulator once per response, groups files by project root and runs the formatter in one batched invocation per root, then groups .ts/.tsx files by tsconfig dir and runs tsc once per tsconfig. Clears the accumulator immediately on read so repeated Stop calls don't double-process. For a 10-file refactor: was 10 × (15s + 30s) = 7.5 min overhead, now 1 × (batch format + batch tsc) = ~5-30s total. * fix(hooks): address race condition, spawn timeout, and Windows path guard Three issues raised in code review: 1. Race condition: switched accumulator from non-atomic JSON read-modify-write to appendFileSync (one path per line). Concurrent Edit hook processes each append independently without clobbering each other. Deduplication moved to the Stop hook at read time. 2. Effective timeout: added run() export to stop-format-typecheck.js so run-with-flags.js uses the direct require() path instead of falling through to spawnSync (which has a hardcoded 30s cap). The 120s timeout in hooks.json now governs the full batch as intended. 3. Windows path guard: added spaces and parentheses to UNSAFE_PATH_CHARS so paths like "C:\Users\John Doe\project\file.ts" are caught before being passed to cmd.exe with shell: true. * fix(hooks): fix session fallback, stale comment, trim verbose comments - Replace 'default' session ID fallback with a cwd-based sha1 hash so concurrent sessions in different projects don't share the same accumulator file when CLAUDE_SESSION_ID is unset - Remove stale "JSON file" reference in accumulator header (format is now newline-delimited plain text) - Remove redundant/verbose inline comments throughout both files * fix(hooks): sanitize session ID, fix Windows tsc, proportional timeouts - Sanitize CLAUDE_SESSION_ID with /[^a-zA-Z0-9_-]/g before embedding in the temp filename so crafted separators or '..' sequences cannot escape os.tmpdir() (cubic P1) - Fix typecheckBatch on Windows: npx.cmd requires shell:true like formatBatch already does; use spawnSync and extract stdout/stderr from the result object (coderabbit P1) - Proportional per-batch timeouts: divide 270s budget across all format and typecheck batches so sequential runs in monorepos stay within the Stop hook wall-clock limit (greptile P2) - Raise Stop hook timeout from 120s to 300s to give large monorepos adequate headroom (cubic P2) * fix(hooks): extend accumulator to Write|MultiEdit, fix tests - Extend matcher from Edit to Edit|Write|MultiEdit so files created with Write and all files in a MultiEdit batch are included in the Stop-time format+typecheck pass (cubic P1) - Handle tool_input.edits[] array in accumulator for MultiEdit support - Rename misleading 'concurrent writes' test to clarify it tests append preservation, not true concurrency (cubic P2) - Add Stop hook dedup test: writes duplicate paths to accumulator and verifies the hook clears it cleanly (cubic P2) - Add Write and MultiEdit accumulation tests * fix(hooks): move timeout to command level, add dedup unit tests - Move timeout: 300 from the matcher object to the hook command object where it is actually enforced; the previous position was a no-op (cubic P2) - Extract parseAccumulator() and export it so tests can assert dedup behavior directly without relying only on side effects (cubic P2) - Add two unit tests for parseAccumulator: deduplication and blank-line handling; rename the integration test to match its scope * fix(hooks): replace removed format/typecheck hooks with accumulator in cursor adapter
This commit is contained in:
Yuval Dinodia
248
tests/hooks/stop-format-typecheck.test.js
Normal file
248
tests/hooks/stop-format-typecheck.test.js
Normal file
@@ -0,0 +1,248 @@
|
||||
/**
|
||||
* Tests for scripts/hooks/post-edit-accumulator.js and
|
||||
* scripts/hooks/stop-format-typecheck.js
|
||||
*
|
||||
* Run with: node tests/hooks/stop-format-typecheck.test.js
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const os = require('os');
|
||||
const path = require('path');
|
||||
|
||||
const accumulator = require('../../scripts/hooks/post-edit-accumulator');
|
||||
const { parseAccumulator } = require('../../scripts/hooks/stop-format-typecheck');
|
||||
|
||||
function test(name, fn) {
|
||||
try {
|
||||
fn();
|
||||
console.log(` ✓ ${name}`);
|
||||
return true;
|
||||
} catch (err) {
|
||||
console.log(` ✗ ${name}`);
|
||||
console.log(` Error: ${err.message}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
|
||||
// Use a unique session ID for tests so we don't pollute real sessions
|
||||
const TEST_SESSION_ID = `test-${Date.now()}`;
|
||||
const origSessionId = process.env.CLAUDE_SESSION_ID;
|
||||
process.env.CLAUDE_SESSION_ID = TEST_SESSION_ID;
|
||||
|
||||
function getAccumFile() {
|
||||
return path.join(os.tmpdir(), `ecc-edited-${TEST_SESSION_ID}.txt`);
|
||||
}
|
||||
|
||||
function cleanAccumFile() {
|
||||
try { fs.unlinkSync(getAccumFile()); } catch { /* doesn't exist */ }
|
||||
}
|
||||
|
||||
// ── post-edit-accumulator.js ─────────────────────────────────────
|
||||
|
||||
console.log('\npost-edit-accumulator: pass-through behavior');
|
||||
console.log('=============================================\n');
|
||||
|
||||
if (test('returns original input unchanged', () => {
|
||||
cleanAccumFile();
|
||||
const input = JSON.stringify({ tool_input: { file_path: '/tmp/x.ts' } });
|
||||
const result = accumulator.run(input);
|
||||
assert.strictEqual(result, input);
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('returns original input for invalid JSON', () => {
|
||||
cleanAccumFile();
|
||||
const input = 'not json';
|
||||
const result = accumulator.run(input);
|
||||
assert.strictEqual(result, input);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('returns original input when no file_path', () => {
|
||||
cleanAccumFile();
|
||||
const input = JSON.stringify({ tool_input: { command: 'ls' } });
|
||||
const result = accumulator.run(input);
|
||||
assert.strictEqual(result, input);
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
console.log('\npost-edit-accumulator: file accumulation');
|
||||
console.log('=========================================\n');
|
||||
|
||||
if (test('creates accumulator file for a .ts file', () => {
|
||||
cleanAccumFile();
|
||||
const input = JSON.stringify({ tool_input: { file_path: '/tmp/foo.ts' } });
|
||||
accumulator.run(input);
|
||||
const accumFile = getAccumFile();
|
||||
assert.ok(fs.existsSync(accumFile), 'accumulator file should exist');
|
||||
const lines = fs.readFileSync(accumFile, 'utf8').split('\n').filter(Boolean);
|
||||
assert.ok(lines.includes('/tmp/foo.ts'));
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('accumulates multiple files across calls', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/a.ts' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/b.tsx' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/c.js' } }));
|
||||
const lines = fs.readFileSync(getAccumFile(), 'utf8').split('\n').filter(Boolean);
|
||||
assert.deepStrictEqual(lines, ['/tmp/a.ts', '/tmp/b.tsx', '/tmp/c.js']);
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('all appended paths are preserved including duplicates (dedup is Stop hook responsibility)', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/a.ts' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/b.ts' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/a.ts' } })); // duplicate
|
||||
const lines = fs.readFileSync(getAccumFile(), 'utf8').split('\n').filter(Boolean);
|
||||
assert.strictEqual(lines.length, 3); // all three appends land
|
||||
assert.strictEqual(new Set(lines).size, 2); // two unique paths
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('accumulates Write tool file_path', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/new-file.ts' } }));
|
||||
const lines = fs.readFileSync(getAccumFile(), 'utf8').split('\n').filter(Boolean);
|
||||
assert.ok(lines.includes('/tmp/new-file.ts'));
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('accumulates MultiEdit edits array paths', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({
|
||||
tool_input: {
|
||||
edits: [
|
||||
{ file_path: '/tmp/multi-a.ts', old_string: 'a', new_string: 'b' },
|
||||
{ file_path: '/tmp/multi-b.tsx', old_string: 'c', new_string: 'd' },
|
||||
{ file_path: '/tmp/skip.md', old_string: 'e', new_string: 'f' }
|
||||
]
|
||||
}
|
||||
}));
|
||||
const lines = fs.readFileSync(getAccumFile(), 'utf8').split('\n').filter(Boolean);
|
||||
assert.ok(lines.includes('/tmp/multi-a.ts'));
|
||||
assert.ok(lines.includes('/tmp/multi-b.tsx'));
|
||||
assert.ok(!lines.includes('/tmp/skip.md'), 'non-JS/TS should be excluded');
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('does not create accumulator file for non-JS/TS files', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/README.md' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/styles.css' } }));
|
||||
assert.ok(!fs.existsSync(getAccumFile()), 'no accumulator for non-JS/TS files');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('handles .tsx and .jsx extensions', () => {
|
||||
cleanAccumFile();
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/comp.tsx' } }));
|
||||
accumulator.run(JSON.stringify({ tool_input: { file_path: '/tmp/comp.jsx' } }));
|
||||
const lines = fs.readFileSync(getAccumFile(), 'utf8').split('\n').filter(Boolean);
|
||||
assert.ok(lines.includes('/tmp/comp.tsx'));
|
||||
assert.ok(lines.includes('/tmp/comp.jsx'));
|
||||
cleanAccumFile();
|
||||
})) passed++; else failed++;
|
||||
|
||||
// ── stop-format-typecheck: accumulator teardown ──────────────────
|
||||
|
||||
console.log('\nstop-format-typecheck: accumulator cleanup');
|
||||
console.log('==========================================\n');
|
||||
|
||||
if (test('stop hook removes accumulator file after reading it', () => {
|
||||
cleanAccumFile();
|
||||
// Write a fake accumulator with a non-existent file so no real formatter runs
|
||||
fs.writeFileSync(getAccumFile(), '/nonexistent/file.ts\n', 'utf8');
|
||||
assert.ok(fs.existsSync(getAccumFile()), 'accumulator should exist before stop hook');
|
||||
|
||||
// Require the stop hook and invoke main() directly via its stdin entry.
|
||||
// We simulate the stdin+stdout flow by spawning node and feeding empty stdin.
|
||||
const { execFileSync } = require('child_process');
|
||||
const stopScript = path.resolve(__dirname, '../../scripts/hooks/stop-format-typecheck.js');
|
||||
try {
|
||||
execFileSync('node', [stopScript], {
|
||||
input: '{}',
|
||||
env: { ...process.env, CLAUDE_SESSION_ID: TEST_SESSION_ID },
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
timeout: 10000
|
||||
});
|
||||
} catch {
|
||||
// tsc/formatter may fail for the nonexistent file — that's OK
|
||||
}
|
||||
|
||||
assert.ok(!fs.existsSync(getAccumFile()), 'accumulator file should be deleted by stop hook');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('stop hook is a no-op when no accumulator exists', () => {
|
||||
cleanAccumFile();
|
||||
const { execFileSync } = require('child_process');
|
||||
const stopScript = path.resolve(__dirname, '../../scripts/hooks/stop-format-typecheck.js');
|
||||
// Should exit cleanly with no errors
|
||||
execFileSync('node', [stopScript], {
|
||||
input: '{}',
|
||||
env: { ...process.env, CLAUDE_SESSION_ID: TEST_SESSION_ID },
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
timeout: 10000
|
||||
});
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('parseAccumulator deduplicates repeated paths', () => {
|
||||
const raw = '/tmp/a.ts\n/tmp/b.ts\n/tmp/a.ts\n/tmp/a.ts\n/tmp/c.js\n';
|
||||
const result = parseAccumulator(raw);
|
||||
assert.deepStrictEqual(result, ['/tmp/a.ts', '/tmp/b.ts', '/tmp/c.js']);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('parseAccumulator ignores blank lines and trims whitespace', () => {
|
||||
const raw = ' /tmp/a.ts \n\n/tmp/b.ts\n\n';
|
||||
const result = parseAccumulator(raw);
|
||||
assert.deepStrictEqual(result, ['/tmp/a.ts', '/tmp/b.ts']);
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('stop hook clears accumulator after processing duplicates', () => {
|
||||
cleanAccumFile();
|
||||
fs.writeFileSync(getAccumFile(), '/nonexistent/x.ts\n/nonexistent/x.ts\n/nonexistent/y.ts\n', 'utf8');
|
||||
const { execFileSync } = require('child_process');
|
||||
const stopScript = path.resolve(__dirname, '../../scripts/hooks/stop-format-typecheck.js');
|
||||
try {
|
||||
execFileSync('node', [stopScript], {
|
||||
input: '{}',
|
||||
env: { ...process.env, CLAUDE_SESSION_ID: TEST_SESSION_ID },
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
timeout: 10000
|
||||
});
|
||||
} catch { /* formatter/tsc may fail for nonexistent files */ }
|
||||
assert.ok(!fs.existsSync(getAccumFile()), 'accumulator cleared after stop hook');
|
||||
})) passed++; else failed++;
|
||||
|
||||
if (test('stop hook passes stdin through unchanged', () => {
|
||||
cleanAccumFile();
|
||||
const { execFileSync } = require('child_process');
|
||||
const stopScript = path.resolve(__dirname, '../../scripts/hooks/stop-format-typecheck.js');
|
||||
const input = '{"stop_reason":"end_turn"}';
|
||||
const result = execFileSync('node', [stopScript], {
|
||||
input,
|
||||
env: { ...process.env, CLAUDE_SESSION_ID: TEST_SESSION_ID },
|
||||
stdio: ['pipe', 'pipe', 'pipe'],
|
||||
timeout: 10000
|
||||
});
|
||||
assert.strictEqual(result.toString(), input);
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Restore env
|
||||
if (origSessionId === undefined) {
|
||||
delete process.env.CLAUDE_SESSION_ID;
|
||||
} else {
|
||||
process.env.CLAUDE_SESSION_ID = origSessionId;
|
||||
}
|
||||
|
||||
console.log(`\n=== Test Results ===`);
|
||||
console.log(`Passed: ${passed}`);
|
||||
console.log(`Failed: ${failed}`);
|
||||
console.log(`Total: ${passed + failed}`);
|
||||
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
Reference in New Issue
Block a user