mirror of
https://github.com/Jeuners/ECC.git
synced 2026-09-09 15:02:30 +02:00
refactor(hooks): consolidate PostToolUse hooks into sync/async dispatchers (#2494)
* refactor(hooks): consolidate PostToolUse hooks into sync/async dispatchers Replace 10 individual PostToolUse entries in hooks.json with two consolidated dispatcher entries (post:dispatcher:sync / post:dispatcher:async). The dispatcher's internal registry preserves every hook ID, matcher, and profile, so ECC_DISABLED_HOOKS and ECC_HOOK_PROFILE gating behave exactly as before. Performance (Edit event, actual hooks.json commands spawned in parallel like the harness does, median of 7 runs): - Blocking hook latency: 81ms -> 49ms (~40% faster; 7 blocking processes -> 1 sync dispatcher) - Node processes per tool call: 10 -> 2 (7 blocking + 3 async -> 1 sync + 1 async) - observe-runner now runs in-process (~370ms) inside the async dispatcher, which stays backgrounded (async: true, timeout 45s), so it adds no user-facing latency. Also: - dashboard-web lists dispatcher-managed child hooks so the hook inventory stays complete - post-edit-console-warn refactored to export run() for in-process dispatch while keeping standalone stdin behavior - dispatcher stdin reading is multi-byte safe (StringDecoder) and child hook exit codes propagate to the dispatcher exit code * test(hooks): replace emoji literal with unicode escape for CI unicode safety check * fix(hooks): adopt explicit cli() entrypoint and merge multi-hook stdout Address Greptile review on #2494: - Replace the non-standard 'require.main === undefined' guard with an explicit exported cli(). The hooks.json bootstraps now call require(s).cli(), so merely requiring the module (dashboard-web, test runners, Jest, worker threads) can never trigger dispatch, attach stdin listeners, or set process.exitCode. - Replace last-writer-wins stdout with mergeHookStdout(): when several hooks emit additionalContext envelopes they merge into a single PostToolUse envelope; non-mergeable raw stdout keeps the last hook's output and emits a stderr warning naming the dropped hook IDs, so nothing is lost silently. Also includes local formatter reformatting of the dispatcher and its test file (no behavioral changes beyond the above). * fix(hooks): keep post:bash:dispatcher phase reachable in minimal profile The Greptile P1 premise was partially incorrect: sub-hooks without explicit profiles default to standard,strict via parseProfiles() (scripts/lib/hook-flags.js), so audit/cost logs never ran under the minimal profile on main either — there is no user-visible regression. However, main did spawn the bash dispatcher phase unconditionally and let each sub-hook gate itself. Restore that semantic by opening the outer registry gate to minimal,standard,strict so a future sub-hook that opts into minimal is not silently blocked at the phase level. Adds the previously missing minimal-profile async dry-run test. * test(hooks): assert failing hook exit code propagates to real process status Spawns the actual dispatcher subprocess with an injected failing hook and asserts the OS-level exit status, stderr diagnostic, and suppressed pass-through — closing the E2E gap CodeRabbit flagged on #2494. * chore: retrigger CI (flaky windows powershell bootstrap test)
This commit is contained in:
JongHyeok Park
GitHub
parent
754b8dd76c
commit
0071fa5c3c
10 changed files with 900 additions and 161 deletions
|
|
@ -80,10 +80,41 @@ function loadMcps(_root) {
|
|||
if (fs.existsSync(dir)) { for (const f of fs.readdirSync(dir).filter(f => f.endsWith('.json'))) { try { const d = JSON.parse(fs.readFileSync(path.join(dir, f), 'utf8')); r.push({ f, s: Object.entries(d.mcpServers || {}).map(([k, v]) => ({ n: k, cmd: typeof v === 'object' ? (v.command || v.url || '') : String(v), args: v.args || [], env: v.env ? Object.keys(v.env).reduce((a,k)=>{a[k]='••••••'; return a;}, {}) : {}, type: v.type || 'stdio' })) }); } catch (e) { console.error('[ECC] Failed to parse mcp-configs/' + f + ':', e.message); } } }
|
||||
return r;
|
||||
}
|
||||
function loadPostToolUseChildren(root) {
|
||||
if (path.resolve(root) !== ROOT) return [];
|
||||
try {
|
||||
const dispatcher = require(path.join(root, 'scripts', 'hooks', 'posttooluse-dispatcher.js'));
|
||||
return [
|
||||
...dispatcher.SYNC_HOOKS.map(hook => ({ ...hook, mode: 'sync' })),
|
||||
...dispatcher.ASYNC_HOOKS.map(hook => ({ ...hook, mode: 'async' })),
|
||||
].map(hook => ({
|
||||
ev: 'PostToolUse',
|
||||
m: hook.matcher,
|
||||
id: hook.id,
|
||||
d: `Managed by the consolidated PostToolUse ${hook.mode} dispatcher`,
|
||||
}));
|
||||
} catch (error) {
|
||||
console.error('[ECC] Failed to load PostToolUse dispatcher registry:', error.message);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
function loadHooks(_root) {
|
||||
const root = _root || ROOT;
|
||||
const p = path.join(root, 'hooks', 'hooks.json'); if (!fs.existsSync(p)) return [];
|
||||
try { const d = JSON.parse(fs.readFileSync(p, 'utf8')); const h = []; for (const [ev, es] of Object.entries(d.hooks || {})) for (const e of es || []) h.push({ ev, m: e.matcher || '*', id: e.id || '', d: e.description || '' }); return h; } catch (e) { console.error('[ECC] Failed to parse hooks/hooks.json:', e.message); return []; }
|
||||
const hooksPath = path.join(root, 'hooks', 'hooks.json');
|
||||
if (!fs.existsSync(hooksPath)) return [];
|
||||
try {
|
||||
const data = JSON.parse(fs.readFileSync(hooksPath, 'utf8'));
|
||||
const hooks = [];
|
||||
for (const [eventName, entries] of Object.entries(data.hooks || {})) {
|
||||
for (const entry of entries || []) {
|
||||
hooks.push({ ev: eventName, m: entry.matcher || '*', id: entry.id || '', d: entry.description || '' });
|
||||
}
|
||||
}
|
||||
return [...hooks, ...loadPostToolUseChildren(root)];
|
||||
} catch (error) {
|
||||
console.error('[ECC] Failed to parse hooks/hooks.json:', error.message);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
const LANG = {
|
||||
|
|
|
|||
|
|
@ -12,43 +12,54 @@
|
|||
const { readFile } = require('../lib/utils');
|
||||
|
||||
const MAX_STDIN = 1024 * 1024; // 1MB limit
|
||||
let data = '';
|
||||
process.stdin.setEncoding('utf8');
|
||||
|
||||
process.stdin.on('data', chunk => {
|
||||
if (data.length < MAX_STDIN) {
|
||||
const remaining = MAX_STDIN - data.length;
|
||||
data += chunk.substring(0, remaining);
|
||||
}
|
||||
});
|
||||
|
||||
process.stdin.on('end', () => {
|
||||
function run(data) {
|
||||
const warnings = [];
|
||||
try {
|
||||
const input = JSON.parse(data);
|
||||
const filePath = input.tool_input?.file_path;
|
||||
|
||||
if (filePath && /\.(ts|tsx|js|jsx)$/.test(filePath)) {
|
||||
const content = readFile(filePath);
|
||||
if (!content) { process.stdout.write(data); process.exit(0); }
|
||||
const lines = content.split('\n');
|
||||
const matches = [];
|
||||
if (content) {
|
||||
const matches = content
|
||||
.split('\n')
|
||||
.map((line, index) => ({ line, index }))
|
||||
.filter(item => /console\.log/.test(item.line))
|
||||
.map(item => `${item.index + 1}: ${item.line.trim()}`);
|
||||
|
||||
lines.forEach((line, idx) => {
|
||||
if (/console\.log/.test(line)) {
|
||||
matches.push((idx + 1) + ': ' + line.trim());
|
||||
if (matches.length > 0) {
|
||||
warnings.push(`[Hook] WARNING: console.log found in ${filePath}`);
|
||||
warnings.push(...matches.slice(0, 5));
|
||||
warnings.push('[Hook] Remove console.log before committing');
|
||||
}
|
||||
});
|
||||
|
||||
if (matches.length > 0) {
|
||||
console.error('[Hook] WARNING: console.log found in ' + filePath);
|
||||
matches.slice(0, 5).forEach(m => console.error(m));
|
||||
console.error('[Hook] Remove console.log before committing');
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Invalid input — pass through
|
||||
}
|
||||
|
||||
process.stdout.write(data);
|
||||
process.exit(0);
|
||||
});
|
||||
return {
|
||||
stdout: data,
|
||||
stderr: warnings.join('\n'),
|
||||
exitCode: 0,
|
||||
};
|
||||
}
|
||||
|
||||
if (require.main === module) {
|
||||
let data = '';
|
||||
process.stdin.setEncoding('utf8');
|
||||
process.stdin.on('data', chunk => {
|
||||
if (data.length < MAX_STDIN) {
|
||||
const remaining = MAX_STDIN - data.length;
|
||||
data += chunk.substring(0, remaining);
|
||||
}
|
||||
});
|
||||
process.stdin.on('end', () => {
|
||||
const result = run(data);
|
||||
if (result.stderr) process.stderr.write(`${result.stderr}\n`);
|
||||
process.stdout.write(result.stdout);
|
||||
process.exitCode = result.exitCode;
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = { run };
|
||||
|
|
|
|||
294
scripts/hooks/posttooluse-dispatcher.js
Normal file
294
scripts/hooks/posttooluse-dispatcher.js
Normal file
|
|
@ -0,0 +1,294 @@
|
|||
#!/usr/bin/env node
|
||||
/**
|
||||
* Consolidates PostToolUse hooks into one synchronous and one asynchronous
|
||||
* entrypoint while preserving each hook's ID, matcher, profile, and output.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const path = require('path');
|
||||
const { StringDecoder } = require('string_decoder');
|
||||
const { VALID_PROFILES, normalizeId, parseProfiles } = require('../lib/hook-flags');
|
||||
const { runPostBash } = require('./bash-hook-dispatcher');
|
||||
const { run: runQualityGate } = require('./quality-gate');
|
||||
const { run: runDesignQualityCheck } = require('./design-quality-check');
|
||||
const { run: runPostEditAccumulator } = require('./post-edit-accumulator');
|
||||
const { run: runConsoleWarn } = require('./post-edit-console-warn');
|
||||
const { run: runGovernanceCapture } = require('./governance-capture');
|
||||
const { run: runSessionActivityTracker } = require('./session-activity-tracker');
|
||||
const { run: runObserve } = require('./observe-runner');
|
||||
const { run: runMetricsBridge } = require('./ecc-metrics-bridge');
|
||||
const { run: runContextMonitor } = require('./ecc-context-monitor');
|
||||
|
||||
const MAX_STDIN = 1024 * 1024;
|
||||
|
||||
const SYNC_HOOKS = [
|
||||
{ id: 'post:edit:design-quality-check', matcher: 'Edit|Write|MultiEdit', profiles: 'standard,strict', script: 'scripts/hooks/design-quality-check.js', run: runDesignQualityCheck },
|
||||
{ id: 'post:edit:accumulator', matcher: 'Edit|Write|MultiEdit', profiles: 'standard,strict', script: 'scripts/hooks/post-edit-accumulator.js', run: runPostEditAccumulator },
|
||||
{ id: 'post:edit:console-warn', matcher: 'Edit', profiles: 'standard,strict', script: 'scripts/hooks/post-edit-console-warn.js', run: runConsoleWarn },
|
||||
{ id: 'post:governance-capture', matcher: 'Bash|Write|Edit|MultiEdit', profiles: 'standard,strict', script: 'scripts/hooks/governance-capture.js', run: runGovernanceCapture },
|
||||
{ id: 'post:session-activity-tracker', matcher: '*', profiles: 'standard,strict', script: 'scripts/hooks/session-activity-tracker.js', run: runSessionActivityTracker },
|
||||
{ id: 'post:ecc-metrics-bridge', matcher: '*', profiles: 'minimal,standard,strict', script: 'scripts/hooks/ecc-metrics-bridge.js', run: runMetricsBridge },
|
||||
{ id: 'post:ecc-context-monitor', matcher: '*', profiles: 'standard,strict', script: 'scripts/hooks/ecc-context-monitor.js', run: runContextMonitor }
|
||||
];
|
||||
|
||||
const ASYNC_HOOKS = [
|
||||
{
|
||||
id: 'post:bash:dispatcher',
|
||||
matcher: 'Bash',
|
||||
// main ran this phase unconditionally; sub-hooks gate themselves internally
|
||||
profiles: 'minimal,standard,strict',
|
||||
script: 'scripts/hooks/post-bash-dispatcher.js',
|
||||
run(raw) {
|
||||
const result = runPostBash(raw);
|
||||
return { stdout: result.output, stderr: result.stderr, exitCode: result.exitCode };
|
||||
}
|
||||
},
|
||||
{ id: 'post:quality-gate', matcher: 'Edit|Write|MultiEdit', profiles: 'standard,strict', script: 'scripts/hooks/quality-gate.js', run: runQualityGate },
|
||||
{ id: 'post:observe:continuous-learning', matcher: '*', profiles: 'standard,strict', script: 'scripts/hooks/observe-runner.js', run: runObserve }
|
||||
];
|
||||
|
||||
function getPluginRoot(env = process.env) {
|
||||
return env.CLAUDE_PLUGIN_ROOT || env.ECC_PLUGIN_ROOT || path.resolve(__dirname, '..', '..');
|
||||
}
|
||||
|
||||
function matchesTool(matcher, toolName) {
|
||||
return (
|
||||
matcher === '*' ||
|
||||
String(matcher || '')
|
||||
.split('|')
|
||||
.map(value => value.trim())
|
||||
.filter(Boolean)
|
||||
.includes(String(toolName || ''))
|
||||
);
|
||||
}
|
||||
|
||||
function isEnabled(hook, env) {
|
||||
const disabled = new Set(
|
||||
String(env.ECC_DISABLED_HOOKS || '')
|
||||
.split(',')
|
||||
.map(normalizeId)
|
||||
.filter(Boolean)
|
||||
);
|
||||
const requestedProfile = String(env.ECC_HOOK_PROFILE || 'standard')
|
||||
.trim()
|
||||
.toLowerCase();
|
||||
const profile = VALID_PROFILES.has(requestedProfile) ? requestedProfile : 'standard';
|
||||
return !disabled.has(normalizeId(hook.id)) && parseProfiles(hook.profiles).includes(profile);
|
||||
}
|
||||
|
||||
function extractToolName(raw) {
|
||||
try {
|
||||
return String(JSON.parse(raw)?.tool_name || '');
|
||||
} catch {
|
||||
return '';
|
||||
}
|
||||
}
|
||||
|
||||
function buildDryRunPreview(hook, raw) {
|
||||
let target = '';
|
||||
try {
|
||||
const input = JSON.parse(raw)?.tool_input || {};
|
||||
target = String(input.file_path || input.path || input.command || '');
|
||||
} catch {
|
||||
target = '';
|
||||
}
|
||||
const suffix = target ? ` target=${target}` : '';
|
||||
return `[DryRun] Hook "${hook.id}" would execute: ${hook.script} (enabled=true, profiles=${hook.profiles})${suffix}\n`;
|
||||
}
|
||||
|
||||
function normalizeResult(raw, output) {
|
||||
if (typeof output === 'string' || Buffer.isBuffer(output)) {
|
||||
const stdout = String(output);
|
||||
return { stdout: stdout !== raw ? stdout : '', stderr: '', exitCode: 0 };
|
||||
}
|
||||
if (!output || typeof output !== 'object') {
|
||||
return { stdout: '', stderr: '', exitCode: 0 };
|
||||
}
|
||||
|
||||
let stdout = '';
|
||||
if (Object.prototype.hasOwnProperty.call(output, 'stdout')) {
|
||||
stdout = String(output.stdout ?? '');
|
||||
} else if (Object.prototype.hasOwnProperty.call(output, 'output')) {
|
||||
stdout = String(output.output ?? '');
|
||||
} else if (Object.prototype.hasOwnProperty.call(output, 'additionalContext')) {
|
||||
stdout = JSON.stringify({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PostToolUse',
|
||||
additionalContext: String(output.additionalContext ?? '')
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
stdout: stdout !== raw ? stdout : '',
|
||||
stderr: typeof output.stderr === 'string' ? output.stderr : '',
|
||||
exitCode: Number.isInteger(output.exitCode) ? output.exitCode : 0
|
||||
};
|
||||
}
|
||||
|
||||
function appendLine(current, next) {
|
||||
if (!next) return current;
|
||||
return current + (String(next).endsWith('\n') ? String(next) : `${next}\n`);
|
||||
}
|
||||
|
||||
function parseAdditionalContext(stdout) {
|
||||
try {
|
||||
const parsed = JSON.parse(stdout);
|
||||
const output = parsed?.hookSpecificOutput;
|
||||
if (output?.hookEventName !== 'PostToolUse') return null;
|
||||
return typeof output.additionalContext === 'string' ? output.additionalContext : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function mergeHookStdout(outputs) {
|
||||
if (outputs.length === 0) return { stdout: '', warning: '' };
|
||||
if (outputs.length === 1) return { stdout: outputs[0].stdout, warning: '' };
|
||||
|
||||
const contexts = outputs.map(output => parseAdditionalContext(output.stdout));
|
||||
if (contexts.every(context => context !== null)) {
|
||||
return {
|
||||
stdout: JSON.stringify({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PostToolUse',
|
||||
additionalContext: contexts.join('\n')
|
||||
}
|
||||
}),
|
||||
warning: ''
|
||||
};
|
||||
}
|
||||
|
||||
const kept = outputs[outputs.length - 1];
|
||||
const dropped = outputs
|
||||
.slice(0, -1)
|
||||
.map(output => output.id)
|
||||
.join(', ');
|
||||
return {
|
||||
stdout: kept.stdout,
|
||||
warning: `[Hook] stdout from ${dropped} dropped in favor of ${kept.id}; raw stdout cannot be merged`
|
||||
};
|
||||
}
|
||||
|
||||
function runHooks(raw, hooks, options = {}) {
|
||||
const env = options.env || process.env;
|
||||
const toolName = options.toolName ?? extractToolName(raw);
|
||||
const pluginRoot = getPluginRoot(env);
|
||||
const outputs = [];
|
||||
let stderr = '';
|
||||
let exitCode = 0;
|
||||
|
||||
for (const hook of hooks) {
|
||||
if (!matchesTool(hook.matcher, toolName) || !isEnabled(hook, env)) continue;
|
||||
if (env.ECC_DRY_RUN === '1') {
|
||||
stderr += buildDryRunPreview(hook, raw);
|
||||
continue;
|
||||
}
|
||||
|
||||
try {
|
||||
const result = normalizeResult(
|
||||
raw,
|
||||
hook.run(raw, {
|
||||
hookId: hook.id,
|
||||
pluginRoot,
|
||||
scriptPath: path.join(pluginRoot, hook.script || ''),
|
||||
truncated: options.truncated === true,
|
||||
maxStdin: MAX_STDIN
|
||||
})
|
||||
);
|
||||
if (result.stdout) outputs.push({ id: hook.id, stdout: result.stdout });
|
||||
stderr = appendLine(stderr, result.stderr);
|
||||
if (result.exitCode !== 0) {
|
||||
if (exitCode === 0) exitCode = result.exitCode;
|
||||
stderr = appendLine(stderr, `[Hook] ${hook.id} exited with code ${result.exitCode}; continuing`);
|
||||
}
|
||||
} catch (error) {
|
||||
stderr = appendLine(stderr, `[Hook] ${hook.id} failed: ${error.message}`);
|
||||
}
|
||||
}
|
||||
|
||||
const merged = mergeHookStdout(outputs);
|
||||
if (merged.warning) stderr = appendLine(stderr, merged.warning);
|
||||
return { stdout: merged.stdout, stderr, exitCode };
|
||||
}
|
||||
|
||||
function readStdinRaw() {
|
||||
return new Promise(resolve => {
|
||||
const decoder = new StringDecoder('utf8');
|
||||
let raw = '';
|
||||
let bytesRead = 0;
|
||||
let truncated = false;
|
||||
let settled = false;
|
||||
process.stdin.on('data', chunk => {
|
||||
const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk);
|
||||
const remaining = Math.max(0, MAX_STDIN - bytesRead);
|
||||
const accepted = buffer.subarray(0, remaining);
|
||||
if (accepted.length > 0) {
|
||||
raw += decoder.write(accepted);
|
||||
bytesRead += accepted.length;
|
||||
}
|
||||
if (buffer.length > accepted.length) truncated = true;
|
||||
});
|
||||
const finish = () => {
|
||||
if (settled) return;
|
||||
settled = true;
|
||||
if (!truncated) raw += decoder.end();
|
||||
resolve({ raw, truncated });
|
||||
};
|
||||
process.stdin.once('end', finish);
|
||||
process.stdin.once('error', finish);
|
||||
});
|
||||
}
|
||||
|
||||
function resolveMainStdout(raw, result, options = {}) {
|
||||
if (result.stdout) return result.stdout;
|
||||
if (options.truncated || result.exitCode !== 0 || !options.passthrough) return '';
|
||||
return raw;
|
||||
}
|
||||
|
||||
async function main() {
|
||||
const mode = process.argv[2] === 'async' ? 'async' : 'sync';
|
||||
const { raw, truncated } = await readStdinRaw();
|
||||
const dispatcherId = `post:dispatcher:${mode}`;
|
||||
const dispatcherEnabled = isEnabled(
|
||||
{
|
||||
id: dispatcherId,
|
||||
profiles: 'minimal,standard,strict'
|
||||
},
|
||||
process.env
|
||||
);
|
||||
const hooks = dispatcherEnabled ? (mode === 'async' ? ASYNC_HOOKS : SYNC_HOOKS) : [];
|
||||
const result = runHooks(raw, hooks, { truncated });
|
||||
if (truncated) {
|
||||
process.stderr.write(`[Hook] stdin exceeded ${MAX_STDIN} bytes for PostToolUse ${mode}; suppressing pass-through\n`);
|
||||
}
|
||||
if (result.stderr) process.stderr.write(result.stderr);
|
||||
const stdout = resolveMainStdout(raw, result, {
|
||||
passthrough: process.env.ECC_POSTTOOLUSE_PASSTHROUGH === '1',
|
||||
truncated
|
||||
});
|
||||
if (stdout) process.stdout.write(stdout);
|
||||
process.exitCode = result.exitCode;
|
||||
}
|
||||
|
||||
function cli() {
|
||||
main().catch(error => {
|
||||
process.stderr.write(`[Hook] PostToolUse dispatcher failed: ${error.message}\n`);
|
||||
process.exitCode = 0;
|
||||
});
|
||||
}
|
||||
|
||||
if (require.main === module) cli();
|
||||
|
||||
module.exports = {
|
||||
ASYNC_HOOKS,
|
||||
SYNC_HOOKS,
|
||||
cli,
|
||||
matchesTool,
|
||||
main,
|
||||
mergeHookStdout,
|
||||
normalizeResult,
|
||||
resolveMainStdout,
|
||||
runHooks
|
||||
};
|
||||
Loading…
Add table
Add a link
Reference in a new issue