test(hooks): close bootstrap review gaps

scripts/hooks/plugin-hook-bootstrap.js

@@ -7,6 +7,7 @@ const { spawnSync } = require('child_process');
 const { ensureAgentDataHomeEnv } = require('../lib/agent-data-home');
 
 const SHELL_PROBE_TIMEOUT_MS = 2000;
+const STDOUT_PIPE_CAP_BYTES = 64 * 1024;
 
 function readStdinRaw() {
   try {
@@ -22,6 +23,18 @@ function writeStderr(stderr) {
   }
 }
 
+function withComparisonInput(result, comparisonInput) {
+  return { ...result, comparisonInput };
+}
+
+function isRawPassthrough(raw, stdout) {
+  if (!raw || !stdout) return false;
+  return (
+    stdout === raw ||
+    (Buffer.byteLength(stdout, 'utf8') === STDOUT_PIPE_CAP_BYTES && raw.startsWith(stdout))
+  );
+}
+
 function passthrough(result) {
   const stdout = typeof result?.stdout === 'string' ? result.stdout : '';
   if (stdout) {
@@ -39,11 +52,8 @@ function passthrough(result) {
     // ~64 KB Node.js pipe buffer and get truncated -- stdout is then exactly
     // 65536 bytes and a strict prefix of raw. So we also detect that
     // truncation sentinel.
-    const raw = typeof result?.__rawInput === 'string' ? result.__rawInput : '';
-    const STDOUT_PIPE_CAP = 64 * 1024;
-    const looksLikePassthrough =
-      (stdout.length === STDOUT_PIPE_CAP && raw.startsWith(stdout)) ||
-      (raw.length > 0 && stdout === raw);
+    const raw = typeof result?.comparisonInput === 'string' ? result.comparisonInput : '';
+    const looksLikePassthrough = isRawPassthrough(raw, stdout);
     if (looksLikePassthrough) {
       writeStderr(
         '[Hook] bootstrap: hook returned raw input as stdout; emitting empty to avoid transcript bloat\n'
@@ -179,11 +189,7 @@ function spawnNode(rootDir, relPath, raw, args) {
     timeout: 30000,
     windowsHide: true,
   });
-  // Tag result with the raw input so passthrough() can detect the
-  // "hook returned raw input as stdout" pattern and suppress it
-  // (the dominant source of session-transcript bloat).
-  result.__rawInput = raw;
-  return result;
+  return withComparisonInput(result, raw);
 }
 
 // spawnShell is not used by any hook in the shipped hooks.json configuration
@@ -228,8 +234,7 @@ function spawnShell(rootDir, relPath, raw, args) {
       timeout: 30000,
       windowsHide: true,
     });
-    bashResult.__rawInput = raw;
-    return bashResult;
+    return withComparisonInput(bashResult, raw);
   }
 
   const shellArgs = isPs
@@ -246,8 +251,7 @@ function spawnShell(rootDir, relPath, raw, args) {
     timeout: 30000,
     windowsHide: true,
   });
-  result.__rawInput = raw;
-  return result;
+  return withComparisonInput(result, raw);
 }
 
 function main() {
@@ -307,6 +311,8 @@ if (require.main === module || require.main === undefined) {
 }
 
 module.exports = {
+  isRawPassthrough,
   main,
   normalizePluginRootForPlatform,
-};
+  withComparisonInput,
+};

tests/hooks/plugin-hook-bootstrap-no-echo.test.js

@@ -25,25 +25,30 @@
  *   - #2239 / #2240 — fixed the same bug in bash-hook-dispatcher.js
  *   - #1575 — "token limit so fast" (symptom caused in part by this)
  *
- * Fixtures live under `/tmp/ecc-pr2380-fixtures/` (per reviewer feedback
- * on #2380 — keep temp fixture files out of the live scripts/hooks/ tree).
+ * Fixtures live under a unique os.tmpdir() directory (per reviewer feedback
+ * on #2380 — keep temp fixture files out of the live scripts/hooks/ tree and
+ * avoid collisions across parallel/cross-platform test runs).
  */
 
 'use strict';
 
 const assert = require('assert');
 const fs = require('fs');
+const os = require('os');
 const path = require('path');
 const { spawnSync } = require('child_process');
 
 const repoRoot = path.join(__dirname, '..', '..');
 const bootstrap = path.join(repoRoot, 'scripts', 'hooks', 'plugin-hook-bootstrap.js');
-const FIXTURE_DIR = '/tmp/ecc-pr2380-fixtures';
+const { isRawPassthrough } = require(bootstrap);
+const FIXTURE_DIR = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-pr2380-fixtures-'));
 
-function ensureFixtureDir() {
-  fs.mkdirSync(FIXTURE_DIR, { recursive: true });
+function cleanupFixtureDir() {
+  fs.rmSync(FIXTURE_DIR, { recursive: true, force: true });
 }
 
+process.once('exit', cleanupFixtureDir);
+
 function test(name, fn) {
   try {
     fn();
@@ -68,6 +73,19 @@ function runBootstrap(args, input, env) {
   });
 }
 
+function runHookEntry(args, input, env) {
+  const loader = `const s=${JSON.stringify(bootstrap)};process.argv.splice(1,0,s);require(s)`;
+  return spawnSync(process.execPath, ['-e', loader, ...args], {
+    input,
+    encoding: 'utf8',
+    cwd: repoRoot,
+    env: { ...process.env, ...(env || {}) },
+    timeout: 30000,
+    maxBuffer: 16 * 1024 * 1024,
+    stdio: ['pipe', 'pipe', 'pipe']
+  });
+}
+
 function realisticPostToolUseEditPayload() {
   return JSON.stringify({
     session_id: 'test-session',
@@ -88,8 +106,6 @@ function realisticPostToolUseEditPayload() {
 
 console.log('\nplugin-hook-bootstrap raw-echo (no bloat) tests:');
 
-ensureFixtureDir();
-
 let passed = 0;
 let failed = 0;
 
@@ -129,13 +145,13 @@ else failed++;
 if (
   test('fallthrough 3: silent hook does NOT echo raw input (the core bug)', () => {
     const payload = realisticPostToolUseEditPayload();
-    // A no-op node hook that reads stdin and exits silently. Lives in
-    // /tmp/ecc-pr2380-fixtures/ — NOT in the live scripts/hooks/ tree.
+    // A no-op node hook that reads stdin and exits silently. It lives in the
+    // unique temporary fixture root, not in the live scripts/hooks/ tree.
     const noopHookPath = path.join(FIXTURE_DIR, 'noop-hook-fixture.js');
     fs.writeFileSync(noopHookPath, "process.stdin.resume(); process.stdin.on('end', () => process.exit(0));");
     try {
-      const result = runBootstrap(['node', noopHookPath], payload, {
-        CLAUDE_PLUGIN_ROOT: repoRoot
+      const result = runBootstrap(['node', path.basename(noopHookPath)], payload, {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
       });
       assert.strictEqual(result.status, 0);
       assert.strictEqual(result.stdout, '', 'silent hook must NOT echo raw input (was ' + result.stdout.length + ' bytes)');
@@ -181,8 +197,8 @@ if (
     );
     try {
       const payload = realisticPostToolUseEditPayload();
-      const result = runBootstrap(['node', fixturePath], payload, {
-        CLAUDE_PLUGIN_ROOT: repoRoot
+      const result = runBootstrap(['node', path.basename(fixturePath)], payload, {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
       });
       assert.strictEqual(result.status, 0);
       assert.ok(result.stdout.length > 0, 'hook that produced output should have non-empty stdout');
@@ -211,8 +227,8 @@ if (
     );
     try {
       const payload = realisticPostToolUseEditPayload();
-      const result = runBootstrap(['node', fixturePath], payload, {
-        CLAUDE_PLUGIN_ROOT: repoRoot
+      const result = runBootstrap(['node', path.basename(fixturePath)], payload, {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
       });
       assert.strictEqual(result.status, 0);
       assert.strictEqual(result.stdout, '', 'hook that returned raw input as stdout must be suppressed (was ' + result.stdout.length + ' bytes)');
@@ -225,6 +241,98 @@ if (
   passed++;
 else failed++;
 
+if (
+  test('64 KiB passthrough sentinel is measured in UTF-8 bytes', () => {
+    const fixturePath = path.join(FIXTURE_DIR, 'multibyte-prefix-fixture.js');
+    fs.writeFileSync(
+      fixturePath,
+      "let d=''; process.stdin.setEncoding('utf8'); process.stdin.on('data', c => d += c); process.stdin.on('end', () => process.stdout.write(d.slice(0, 32768)));"
+    );
+    try {
+      const payload = `${'é'.repeat(32768)}tail`;
+      const result = runBootstrap(['node', path.basename(fixturePath)], payload, {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
+      });
+      assert.strictEqual(Buffer.byteLength(payload.slice(0, 32768), 'utf8'), 64 * 1024);
+      assert.strictEqual(result.status, 0, result.stderr);
+      assert.strictEqual(result.stdout, '', 'a 64 KiB UTF-8 prefix of raw input must be suppressed');
+      assert.match(result.stderr, /returned raw input as stdout/);
+    } finally {
+      fs.unlinkSync(fixturePath);
+    }
+  })
+)
+  passed++;
+else failed++;
+
+if (
+  test('byte boundary does not misclassify 64K multibyte characters', () => {
+    const byteBoundaryPrefix = 'é'.repeat(32768);
+    const characterBoundaryPrefix = 'é'.repeat(65536);
+
+    assert.strictEqual(Buffer.byteLength(byteBoundaryPrefix, 'utf8'), 64 * 1024);
+    assert.strictEqual(Buffer.byteLength(characterBoundaryPrefix, 'utf8'), 128 * 1024);
+    assert.strictEqual(isRawPassthrough(`${byteBoundaryPrefix}tail`, byteBoundaryPrefix), true);
+    assert.strictEqual(
+      isRawPassthrough(`${characterBoundaryPrefix}tail`, characterBoundaryPrefix),
+      false,
+      '64K JavaScript characters must not be treated as a 64 KiB byte boundary'
+    );
+  })
+)
+  passed++;
+else failed++;
+
+if (process.platform !== 'win32') {
+  if (
+    test('shell branch suppresses raw stdin echoed by the child', () => {
+      const fixturePath = path.join(FIXTURE_DIR, 'passthrough-fixture.sh');
+      fs.writeFileSync(fixturePath, 'cat\n');
+      try {
+        const payload = realisticPostToolUseEditPayload();
+        const result = runBootstrap(['shell', path.basename(fixturePath)], payload, {
+          CLAUDE_PLUGIN_ROOT: FIXTURE_DIR,
+          BASH: fs.existsSync('/bin/sh') ? '/bin/sh' : 'sh'
+        });
+        assert.strictEqual(result.status, 0, result.stderr);
+        assert.strictEqual(result.stdout, '', 'shell raw-input passthrough must be suppressed');
+        assert.match(result.stderr, /returned raw input as stdout/);
+      } finally {
+        fs.unlinkSync(fixturePath);
+      }
+    })
+  )
+    passed++;
+  else failed++;
+}
+
+if (
+  test('eval hook-entry preserves the original tool result when bootstrap stdout is empty', () => {
+    const fixturePath = path.join(FIXTURE_DIR, 'entry-silent-fixture.js');
+    fs.writeFileSync(fixturePath, "process.stdin.resume(); process.stdin.on('end', () => process.exit(0));");
+    try {
+      const payload = JSON.parse(realisticPostToolUseEditPayload());
+      const originalToolResult = structuredClone(payload.tool_response);
+      const result = runHookEntry(['node', path.basename(fixturePath)], JSON.stringify(payload), {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
+      });
+      assert.strictEqual(result.status, 0, result.stderr);
+      assert.strictEqual(result.stdout, '', 'no-op hook entry must express no replacement result');
+
+      // Claude's hook-entry contract treats empty stdout as no hook override;
+      // the tool result already present in the event remains authoritative.
+      const effectiveToolResult = result.stdout === ''
+        ? payload.tool_response
+        : JSON.parse(result.stdout).tool_response;
+      assert.deepStrictEqual(effectiveToolResult, originalToolResult);
+    } finally {
+      fs.unlinkSync(fixturePath);
+    }
+  })
+)
+  passed++;
+else failed++;
+
 // --- Regression guard: hook with its OWN non-raw output (not equal to raw)
 //     must still pass through unchanged. ---
 if (
@@ -237,8 +345,8 @@ if (
     );
     try {
       const payload = realisticPostToolUseEditPayload();
-      const result = runBootstrap(['node', fixturePath], payload, {
-        CLAUDE_PLUGIN_ROOT: repoRoot
+      const result = runBootstrap(['node', path.basename(fixturePath)], payload, {
+        CLAUDE_PLUGIN_ROOT: FIXTURE_DIR
       });
       assert.strictEqual(result.status, 0);
       // Should contain the hook's own output, not the raw input
@@ -253,4 +361,4 @@ if (
 else failed++;
 
 console.log('\n  ' + passed + ' passed, ' + failed + ' failed\n');
-process.exit(failed > 0 ? 1 : 0);
+process.exit(failed > 0 ? 1 : 0);

tests/hooks/plugin-hook-bootstrap.test.js

@@ -11,7 +11,7 @@ const path = require('path');
 const { spawnSync } = require('child_process');
 
 const SCRIPT = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'plugin-hook-bootstrap.js');
-const { normalizePluginRootForPlatform } = require(SCRIPT);
+const { normalizePluginRootForPlatform, withComparisonInput } = require(SCRIPT);
 
 function createTempDir() {
   return fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-hook-bootstrap-'));
@@ -71,6 +71,16 @@ function runTests() {
     assert.ok(result.stderr.includes('missing required args'));
   })) passed++; else failed++;
 
+  if (test('wraps spawn results without mutating the original object', () => {
+    const original = Object.freeze({ status: 0, stdout: 'ok', stderr: '' });
+    const wrapped = withComparisonInput(original, 'raw-input');
+
+    assert.notStrictEqual(wrapped, original);
+    assert.deepStrictEqual(original, { status: 0, stdout: 'ok', stderr: '' });
+    assert.strictEqual(wrapped.comparisonInput, 'raw-input');
+    assert.strictEqual(wrapped.stdout, 'ok');
+  })) passed++; else failed++;
+
   if (test('normalizes Windows Git Bash POSIX drive roots', () => {
     assert.strictEqual(
       normalizePluginRootForPlatform('/c/Users/x/.claude/plugins/ecc', 'win32'),
@@ -306,16 +316,12 @@ process.exit(7);
   // Windows-only: PowerShell preference and .sh fallback behaviour.
   if (process.platform === 'win32') {
     if (test('shell mode selects PowerShell when BASH is unset on Windows', () => {
-      // Skip if no PowerShell is available.
       const psProbe = spawnSync('pwsh.exe', ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { stdio: 'ignore', timeout: 5000 });
       const ps = psProbe.error
         ? spawnSync('powershell.exe', ['-NoProfile', '-NonInteractive', '-Command', 'exit 0'], { stdio: 'ignore', timeout: 5000 }).error
           ? null : 'powershell.exe'
         : 'pwsh.exe';
-      if (!ps) {
-        console.log('    SKIP: no PowerShell found');
-        return;
-      }
+      assert.ok(ps, 'Windows shell-path coverage requires PowerShell');
 
       const root = createTempDir();
       try {
@@ -340,13 +346,33 @@ process.exit(7);
       }
     })) passed++; else failed++;
 
-    if (test('shell mode falls back to bash for .sh scripts when PowerShell is the resolved shell', () => {
-      // Skip if no bash is available (headless CI without Git for Windows).
-      const bashProbe = spawnSync('bash.exe', ['-c', ':'], { stdio: 'ignore', timeout: 5000 });
-      if (bashProbe.error) {
-        console.log('    SKIP: bash.exe not found');
-        return;
+    if (test('PowerShell branch suppresses raw stdin echoed by the child', () => {
+      const root = createTempDir();
+      try {
+        writeFile(root, path.join('scripts', 'passthrough.ps1'), [
+          '[Console]::OutputEncoding = [System.Text.Encoding]::UTF8',
+          '$OutputEncoding = [System.Text.Encoding]::UTF8',
+          '$input_data = [Console]::In.ReadToEnd()',
+          '[Console]::Out.Write($input_data)',
+        ].join('\n'));
+
+        const result = run(['shell', path.join('scripts', 'passthrough.ps1')], {
+          root,
+          input: 'raw-input',
+          env: { BASH: '' },
+        });
+
+        assert.strictEqual(result.status, 0, result.stderr);
+        assert.strictEqual(result.stdout, '');
+        assert.ok(result.stderr.includes('returned raw input as stdout'));
+      } finally {
+        cleanup(root);
       }
+    })) passed++; else failed++;
+
+    if (test('shell mode falls back to bash for .sh scripts when PowerShell is the resolved shell', () => {
+      const bashProbe = spawnSync('bash.exe', ['-c', ':'], { stdio: 'ignore', timeout: 5000 });
+      assert.ok(!bashProbe.error && bashProbe.status === 0, 'Windows .sh fallback coverage requires bash.exe');
 
       const root = createTempDir();
       try {
@@ -370,6 +396,25 @@ process.exit(7);
       }
     })) passed++; else failed++;
 
+    if (test('PowerShell .sh fallback branch suppresses raw stdin echoed by bash', () => {
+      const root = createTempDir();
+      try {
+        writeFile(root, path.join('scripts', 'passthrough.sh'), 'cat\n');
+
+        const result = run(['shell', path.join('scripts', 'passthrough.sh')], {
+          root,
+          input: 'raw-input',
+          env: { BASH: '' },
+        });
+
+        assert.strictEqual(result.status, 0, result.stderr);
+        assert.strictEqual(result.stdout, '');
+        assert.ok(result.stderr.includes('returned raw input as stdout'));
+      } finally {
+        cleanup(root);
+      }
+    })) passed++; else failed++;
+
     if (test('shell mode emits skip warning for .sh script when no bash found on Windows', () => {
       const root = createTempDir();
       try {
@@ -402,4 +447,4 @@ process.exit(7);
   process.exit(failed > 0 ? 1 : 0);
 }
 
-runTests();
+runTests();