From 28b922dee3383ea3ac45aaa2155c1e2558993d89 Mon Sep 17 00:00:00 2001 From: JongHyeok Park Date: Sun, 26 Jul 2026 16:01:57 +0900 Subject: [PATCH] fix(hooks): preserve Stop output through lifecycle wrappers (#2493) Preserve complete Stop-hook stdout through lifecycle wrappers, wait for queued output to flush before exiting, bound child output with a larger explicit buffer, and add end-to-end regressions for large, multibyte, dry-run, and failure cases. --- hooks/hooks.json | 12 +-- scripts/hooks/run-with-flags.js | 29 ++++-- tests/hooks/stop-hooks-stdout.test.js | 137 +++++++++++++++++++++++++- tests/lib/dry-run.test.js | 28 ++++++ 4 files changed, 188 insertions(+), 18 deletions(-) diff --git a/hooks/hooks.json b/hooks/hooks.json index ad13859a2..00ab4d0aa 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -179,7 +179,7 @@ "hooks": [ { "type": "command", - "command": "node -e \"const fs=require('fs');const path=require('path');const {spawnSync}=require('child_process');const raw=fs.readFileSync(0,'utf8');const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i{let pending=1;const done=()=>{pending-=1;if(pending===0)process.exit(code);};if(out){pending+=1;process.stdout.write(out,done);}if(err){pending+=1;process.stderr.write(err,done);}process.nextTick(done);};const rel=path.join('scripts','hooks','run-with-flags.js');const root=(function(){var p=require('path'),f=require('fs'),o=require('os');var e=process.env.CLAUDE_PLUGIN_ROOT;if(e&&e.trim())return e.trim();var d=p.join(o.homedir(),'.claude');function L(x){try{return require(p.join(x,'scripts','lib','resolve-ecc-root')).resolveEccRoot()}catch(_){return null}}var r=L(d);if(r)return r;var s=['ecc','ecc@ecc','marketplaces/ecc','everything-claude-code','everything-claude-code@everything-claude-code','marketplaces/everything-claude-code'];for(var i=0;i { + pendingWrites -= 1; + if (pendingWrites === 0) { + process.exit(exitCode); + } + }; + + if (typeof text === 'string' && text.length > 0) { + pendingWrites += 1; + process.stdout.write(text, exitWhenFlushed); } - process.stdout.write(text, () => process.exit(exitCode)); + process.stderr.write('', exitWhenFlushed); } function resolveHookResult(raw, output) { @@ -169,8 +178,8 @@ async function main() { if (isDryRun()) { const preview = buildDryRunPreview(hookId, relScriptPath, profilesCsv, raw); process.stderr.write(preview); - process.stdout.write(raw); - process.exit(0); + exitWithStdout(sanitizeEcho(raw), 0); + return; } const pluginRoot = getPluginRoot(); diff --git a/tests/hooks/stop-hooks-stdout.test.js b/tests/hooks/stop-hooks-stdout.test.js index 08b79e66b..04ef84acd 100644 --- a/tests/hooks/stop-hooks-stdout.test.js +++ b/tests/hooks/stop-hooks-stdout.test.js @@ -24,6 +24,9 @@ const { spawnSync } = require('child_process'); const repoRoot = path.join(__dirname, '..', '..'); const runner = path.join(repoRoot, 'scripts', 'hooks', 'run-with-flags.js'); +const hooksConfig = JSON.parse( + fs.readFileSync(path.join(repoRoot, 'hooks', 'hooks.json'), 'utf8') +); const MAX_STDIN = 1024 * 1024; @@ -42,14 +45,14 @@ function test(name, fn) { } } -function stopPayload(messageBytes) { +function stopPayload(messageCharacters, character = 'm') { return JSON.stringify({ session_id: `stop-stdout-test-${process.pid}`, transcript_path: path.join(workDir, 'missing-transcript.jsonl'), cwd: workDir, hook_event_name: 'Stop', stop_hook_active: false, - last_assistant_message: 'm'.repeat(messageBytes) + last_assistant_message: character.repeat(messageCharacters) }); } @@ -62,6 +65,7 @@ function hookEnv() { }; delete env.ECC_GATEGUARD; delete env.ECC_DISABLED_HOOKS; + delete env.ECC_DRY_RUN; return env; } @@ -89,6 +93,26 @@ function runDirect(script, input) { }); } +function runRegisteredStopHook(entry, input, envOverrides = {}) { + const env = { + ...hookEnv(), + CLAUDE_PLUGIN_ROOT: repoRoot, + ECC_DISABLED_HOOKS: entry.id, + ...envOverrides + }; + + return spawnSync(entry.hooks[0].command, { + input, + encoding: 'utf8', + cwd: workDir, + env, + shell: true, + timeout: 60000, + maxBuffer: 16 * 1024 * 1024, + stdio: ['pipe', 'pipe', 'pipe'] + }); +} + function assertStdoutContract(result, label) { assert.strictEqual(result.status, 0, `${label}: expected exit 0, got ${result.status}: ${result.stderr}`); if (result.stdout.length > 0) { @@ -130,6 +154,78 @@ let failed = 0; // runner path, making the harness report "JSON validation failed". const realisticPayload = stopPayload(100 * 1024); +// Exercise the command users actually run from hooks.json. The runner already +// flushes large stdout before exiting, but the outer lifecycle wrapper used to +// call process.exit() immediately after forwarding it, cutting the JSON at the +// OS pipe buffer and reintroducing #2222 above the tested runner layer. +for (const entry of hooksConfig.hooks.Stop) { + if ( + test(`${entry.id} registered wrapper flushes a 100KB Stop payload`, () => { + const result = runRegisteredStopHook(entry, realisticPayload); + assert.strictEqual( + result.status, + 0, + `${entry.id}: expected exit 0, got ${result.status}: ${result.stderr}` + ); + assert.ok( + result.stdout === realisticPayload, + `${entry.id}: registered wrapper must echo ${realisticPayload.length} characters uncut (got ${result.stdout.length})` + ); + JSON.parse(result.stdout); + }) + ) + passed++; + else failed++; +} + +const representativeStopEntry = hooksConfig.hooks.Stop.find( + entry => entry.id === 'stop:cost-tracker' +); + +if ( + test('registered Stop wrapper flushes a 100KB dry-run payload', () => { + const result = runRegisteredStopHook(representativeStopEntry, realisticPayload, { + ECC_DISABLED_HOOKS: '', + ECC_DRY_RUN: '1' + }); + assert.strictEqual(result.status, 0, `expected exit 0, got ${result.status}: ${result.stderr}`); + assert.ok( + result.stdout === realisticPayload, + `dry-run wrapper must echo ${realisticPayload.length} characters uncut (got ${result.stdout.length})` + ); + JSON.parse(result.stdout); + }) +) + passed++; +else failed++; + +// spawnSync limits captured output by bytes while the runner's stdin cap is +// counted after UTF-8 decoding. A payload can therefore be below MAX_STDIN in +// characters but above Node's default 1MB child-process buffer in bytes. +const multibytePayload = stopPayload(400 * 1024, '한'); +assert.ok(multibytePayload.length < MAX_STDIN, 'fixture must stay below the runner character cap'); +assert.ok(Buffer.byteLength(multibytePayload) > MAX_STDIN, 'fixture must exceed the default byte buffer'); + +for (const entry of hooksConfig.hooks.Stop) { + if ( + test(`${entry.id} registered wrapper preserves a multibyte sub-cap payload`, () => { + const result = runRegisteredStopHook(entry, multibytePayload); + assert.strictEqual( + result.status, + 0, + `${entry.id}: expected exit 0, got ${result.status}: ${result.stderr}` + ); + assert.ok( + result.stdout === multibytePayload, + `${entry.id}: registered wrapper must echo ${Buffer.byteLength(multibytePayload)} bytes uncut (got ${Buffer.byteLength(result.stdout)})` + ); + JSON.parse(result.stdout); + }) + ) + passed++; + else failed++; +} + for (const [hookId, script] of STOP_HOOKS) { if ( test(`${hookId} via runner keeps stdout valid for a 100KB Stop payload`, () => { @@ -146,6 +242,43 @@ for (const [hookId, script] of STOP_HOOKS) { const oversizedPayload = stopPayload(MAX_STDIN + 64 * 1024); +if ( + test('registered Stop wrapper suppresses a >1MB dry-run payload', () => { + const result = runRegisteredStopHook(representativeStopEntry, oversizedPayload, { + ECC_DISABLED_HOOKS: '', + ECC_DRY_RUN: '1' + }); + assert.strictEqual(result.status, 0, `expected exit 0, got ${result.status}: ${result.stderr}`); + assert.strictEqual( + result.stdout.length, + 0, + `dry-run wrapper must preserve oversized-input suppression (got ${result.stdout.length} characters)` + ); + }) +) + passed++; +else failed++; + +for (const entry of hooksConfig.hooks.Stop) { + if ( + test(`${entry.id} registered wrapper suppresses a >1MB Stop payload`, () => { + const result = runRegisteredStopHook(entry, oversizedPayload); + assert.strictEqual( + result.status, + 0, + `${entry.id}: expected exit 0, got ${result.status}: ${result.stderr}` + ); + assert.strictEqual( + result.stdout.length, + 0, + `${entry.id}: wrapper must preserve oversized-input suppression (got ${result.stdout.length} characters)` + ); + }) + ) + passed++; + else failed++; +} + for (const [hookId, script] of [...STOP_HOOKS, ['stop:desktop-notify', 'scripts/hooks/desktop-notify.js']]) { if ( test(`${hookId} via runner fails open on a >1MB Stop payload`, () => { diff --git a/tests/lib/dry-run.test.js b/tests/lib/dry-run.test.js index 18190ea9c..ffe7f9ed8 100644 --- a/tests/lib/dry-run.test.js +++ b/tests/lib/dry-run.test.js @@ -95,6 +95,34 @@ function runTests() { assert.strictEqual(result.stdout, input, 'Expected stdin to be passed through unchanged'); })) passed++; else failed++; + if (test('flushes a large dry-run preview when oversized stdout is suppressed', () => { + const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); + const hookScript = 'scripts/hooks/block-no-verify.js'; + const command = 'x'.repeat(900 * 1024); + const document = JSON.stringify({ tool: 'Bash', tool_input: { command } }); + const input = document.padEnd(1024 * 1024 + 1024, ' '); + + const result = spawnSync(process.execPath, [ + runWithFlags, + 'pre:bash:block-no-verify', + hookScript, + 'standard,strict', + ], { + input, + encoding: 'utf8', + env: { ...process.env, ECC_DRY_RUN: '1' }, + cwd: path.resolve(__dirname, '..', '..'), + maxBuffer: 4 * 1024 * 1024, + }); + + assert.strictEqual(result.status, 0, `Expected exit 0, got ${result.status}`); + assert.strictEqual(result.stdout, '', 'Oversized dry-run input must keep stdout suppressed'); + assert.ok( + result.stderr.endsWith(`command=${command}\n`), + `Expected the complete dry-run preview on stderr, got ${result.stderr.length} characters` + ); + })) passed++; else failed++; + if (test('dry-run preview includes command for bash hooks', () => { const runWithFlags = path.resolve(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js'); const hookScript = 'scripts/hooks/block-no-verify.js';