perf(hooks): use direct require() instead of spawning child process

Invoke hook scripts directly via require() when they export a run(rawInput) function, eliminating one Node.js process spawn per hook invocation (~50-100ms). Includes path traversal guard, timeouts, error logging, PR review feedback, legacy hooks guard, normalized filePath, and restored findProjectRoot config detection with package manager support.
2026-03-30 13:43:26 +08:00 · 2026-03-08 16:53:20 +09:00
parent e5d02000c3
commit 66498ae9ac
6 changed files with 228 additions and 63 deletions
--- a/scripts/hooks/post-edit-format.js
+++ b/scripts/hooks/post-edit-format.js
@@ -20,11 +20,7 @@
 const { execFileSync } = require('child_process');
 const path = require('path');

-const {
-  findProjectRoot,
-  detectFormatter,
-  resolveFormatterBin,
-} = require('../lib/resolve-formatter');
+const { findProjectRoot, detectFormatter, resolveFormatterBin } = require('../lib/resolve-formatter');

 const MAX_STDIN = 1024 * 1024; // 1MB limit

@@ -42,23 +38,22 @@ function run(rawInput) {

    if (filePath && /\.(ts|tsx|js|jsx)$/.test(filePath)) {
      try {
-        const projectRoot = findProjectRoot(path.dirname(path.resolve(filePath)));
+        const resolvedFilePath = path.resolve(filePath);
+        const projectRoot = findProjectRoot(path.dirname(resolvedFilePath));
        const formatter = detectFormatter(projectRoot);
        if (!formatter) return rawInput;

        const resolved = resolveFormatterBin(projectRoot, formatter);
+        if (!resolved) return rawInput;

        // Biome: `check --write` = format + lint in one pass
        // Prettier: `--write` = format only
-        const args =
-          formatter === 'biome'
-            ? [...resolved.prefix, 'check', '--write', filePath]
-            : [...resolved.prefix, '--write', filePath];
+        const args = formatter === 'biome' ? [...resolved.prefix, 'check', '--write', resolvedFilePath] : [...resolved.prefix, '--write', resolvedFilePath];

        execFileSync(resolved.bin, args, {
          cwd: projectRoot,
          stdio: ['pipe', 'pipe', 'pipe'],
-          timeout: 15000,
+          timeout: 15000
        });
      } catch {
        // Formatter not installed, file missing, or failed — non-blocking
@@ -76,7 +71,7 @@ if (require.main === module) {
  let data = '';
  process.stdin.setEncoding('utf8');

-  process.stdin.on('data', (chunk) => {
+  process.stdin.on('data', chunk => {
    if (data.length < MAX_STDIN) {
      const remaining = MAX_STDIN - data.length;
      data += chunk.substring(0, remaining);
@@ -84,8 +79,8 @@ if (require.main === module) {
  });

  process.stdin.on('end', () => {
-    const result = run(data);
-    process.stdout.write(result);
+    data = run(data);
+    process.stdout.write(data);
    process.exit(0);
  });
 }
--- a/scripts/hooks/quality-gate.js
+++ b/scripts/hooks/quality-gate.js
@@ -18,31 +18,50 @@ const fs = require('fs');
 const path = require('path');
 const { spawnSync } = require('child_process');

-const {
-  findProjectRoot,
-  detectFormatter,
-  resolveFormatterBin,
-} = require('../lib/resolve-formatter');
+const { findProjectRoot, detectFormatter, resolveFormatterBin } = require('../lib/resolve-formatter');

 const MAX_STDIN = 1024 * 1024;

+/**
+ * Execute a command synchronously, returning the spawnSync result.
+ *
+ * @param {string} command - Executable path or name
+ * @param {string[]} args - Arguments to pass
+ * @param {string} [cwd] - Working directory (defaults to process.cwd())
+ * @returns {import('child_process').SpawnSyncReturns<string>}
+ */
 function exec(command, args, cwd = process.cwd()) {
  return spawnSync(command, args, {
    cwd,
    encoding: 'utf8',
    env: process.env,
+    timeout: 15000
  });
 }

+/**
+ * Write a message to stderr for logging.
+ *
+ * @param {string} msg - Message to log
+ */
 function log(msg) {
  process.stderr.write(`${msg}\n`);
 }

+/**
+ * Run quality-gate checks for a single file based on its extension.
+ * Skips JS/TS files when Biome is configured (handled by post-edit-format).
+ *
+ * @param {string} filePath - Path to the edited file
+ */
 function maybeRunQualityGate(filePath) {
  if (!filePath || !fs.existsSync(filePath)) {
    return;
  }

+  // Resolve to absolute path so projectRoot-relative comparisons work
+  filePath = path.resolve(filePath);
+
  const ext = path.extname(filePath).toLowerCase();
  const fix = String(process.env.ECC_QUALITY_GATE_FIX || '').toLowerCase() === 'true';
  const strict = String(process.env.ECC_QUALITY_GATE_STRICT || '').toLowerCase() === 'true';
@@ -59,6 +78,7 @@ function maybeRunQualityGate(filePath) {

      // .json / .md — still need quality gate
      const resolved = resolveFormatterBin(projectRoot, 'biome');
+      if (!resolved) return;
      const args = [...resolved.prefix, 'check', filePath];
      if (fix) args.push('--write');
      const result = exec(resolved.bin, args, projectRoot);
@@ -70,6 +90,7 @@ function maybeRunQualityGate(filePath) {

    if (formatter === 'prettier') {
      const resolved = resolveFormatterBin(projectRoot, 'prettier');
+      if (!resolved) return;
      const args = [...resolved.prefix, fix ? '--write' : '--check', filePath];
      const result = exec(resolved.bin, args, projectRoot);
      if (result.status !== 0 && strict) {
@@ -82,8 +103,20 @@ function maybeRunQualityGate(filePath) {
    return;
  }

-  if (ext === '.go' && fix) {
-    exec('gofmt', ['-w', filePath]);
+  if (ext === '.go') {
+    if (fix) {
+      const r = exec('gofmt', ['-w', filePath]);
+      if (r.status !== 0 && strict) {
+        log(`[QualityGate] gofmt failed for ${filePath}`);
+      }
+    } else if (strict) {
+      const r = exec('gofmt', ['-l', filePath]);
+      if (r.status !== 0) {
+        log(`[QualityGate] gofmt failed for ${filePath}`);
+      } else if (r.stdout && r.stdout.trim()) {
+        log(`[QualityGate] gofmt check failed for ${filePath}`);
+      }
+    }
    return;
  }

@@ -119,7 +152,7 @@ function run(rawInput) {
 if (require.main === module) {
  let raw = '';
  process.stdin.setEncoding('utf8');
-  process.stdin.on('data', (chunk) => {
+  process.stdin.on('data', chunk => {
    if (raw.length < MAX_STDIN) {
      const remaining = MAX_STDIN - raw.length;
      raw += chunk.substring(0, remaining);
--- a/scripts/hooks/run-with-flags.js
+++ b/scripts/hooks/run-with-flags.js
@@ -52,7 +52,15 @@ async function main() {
  }

  const pluginRoot = getPluginRoot();
-  const scriptPath = path.join(pluginRoot, relScriptPath);
+  const resolvedRoot = path.resolve(pluginRoot);
+  const scriptPath = path.resolve(pluginRoot, relScriptPath);
+
+  // Prevent path traversal outside the plugin root
+  if (!scriptPath.startsWith(resolvedRoot + path.sep)) {
+    process.stderr.write(`[Hook] Path traversal rejected for ${hookId}: ${scriptPath}\n`);
+    process.stdout.write(raw);
+    process.exit(0);
+  }

  if (!fs.existsSync(scriptPath)) {
    process.stderr.write(`[Hook] Script not found for ${hookId}: ${scriptPath}\n`);
@@ -60,11 +68,43 @@ async function main() {
    process.exit(0);
  }

+  // Prefer direct require() when the hook exports a run(rawInput) function.
+  // This eliminates one Node.js process spawn (~50-100ms savings per hook).
+  //
+  // SAFETY: Only require() hooks that export run(). Legacy hooks execute
+  // side effects at module scope (stdin listeners, process.exit, main() calls)
+  // which would interfere with the parent process or cause double execution.
+  let hookModule;
+  const src = fs.readFileSync(scriptPath, 'utf8');
+  const hasRunExport = /\bmodule\.exports\b/.test(src) && /\brun\b/.test(src);
+
+  if (hasRunExport) {
+    try {
+      hookModule = require(scriptPath);
+    } catch (requireErr) {
+      process.stderr.write(`[Hook] require() failed for ${hookId}: ${requireErr.message}\n`);
+      // Fall through to legacy spawnSync path
+    }
+  }
+
+  if (hookModule && typeof hookModule.run === 'function') {
+    try {
+      const output = hookModule.run(raw);
+      if (output != null) process.stdout.write(output);
+    } catch (runErr) {
+      process.stderr.write(`[Hook] run() error for ${hookId}: ${runErr.message}\n`);
+      process.stdout.write(raw);
+    }
+    process.exit(0);
+  }
+
+  // Legacy path: spawn a child Node process for hooks without run() export
  const result = spawnSync('node', [scriptPath], {
    input: raw,
    encoding: 'utf8',
    env: process.env,
    cwd: process.cwd(),
+    timeout: 30000
  });

  if (result.stdout) process.stdout.write(result.stdout);
--- a/scripts/lib/resolve-formatter.js
+++ b/scripts/lib/resolve-formatter.js
@@ -18,9 +18,29 @@ const binCache = new Map();

 // ── Public helpers ──────────────────────────────────────────────────

+// Markers that indicate a project root (formatter configs included so
+// repos without package.json are still detected correctly).
+const PROJECT_ROOT_MARKERS = [
+  'package.json',
+  'biome.json',
+  'biome.jsonc',
+  '.prettierrc',
+  '.prettierrc.json',
+  '.prettierrc.js',
+  '.prettierrc.cjs',
+  '.prettierrc.mjs',
+  '.prettierrc.yml',
+  '.prettierrc.yaml',
+  '.prettierrc.toml',
+  'prettier.config.js',
+  'prettier.config.cjs',
+  'prettier.config.mjs'
+];
+
 /**
- * Walk up from `startDir` until a directory containing package.json is found.
- * Returns `startDir` as fallback when no package.json exists above it.
+ * Walk up from `startDir` until a directory containing a known project
+ * root marker (package.json or formatter config) is found.
+ * Returns `startDir` as fallback when no marker exists above it.
 *
 * @param {string} startDir - Absolute directory path to start from
 * @returns {string} Absolute path to the project root
@@ -30,9 +50,11 @@ function findProjectRoot(startDir) {

  let dir = startDir;
  while (dir !== path.dirname(dir)) {
-    if (fs.existsSync(path.join(dir, 'package.json'))) {
-      projectRootCache.set(startDir, dir);
-      return dir;
+    for (const marker of PROJECT_ROOT_MARKERS) {
+      if (fs.existsSync(path.join(dir, marker))) {
+        projectRootCache.set(startDir, dir);
+        return dir;
+      }
    }
    dir = path.dirname(dir);
  }
@@ -59,6 +81,20 @@ function detectFormatter(projectRoot) {
    }
  }

+  // Check package.json "prettier" key before config files
+  try {
+    const pkgPath = path.join(projectRoot, 'package.json');
+    if (fs.existsSync(pkgPath)) {
+      const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'));
+      if (pkg.prettier != null) {
+        formatterCache.set(projectRoot, 'prettier');
+        return 'prettier';
+      }
+    }
+  } catch {
+    // Malformed package.json — continue to file-based detection
+  }
+
  const prettierConfigs = [
    '.prettierrc',
    '.prettierrc.json',
@@ -70,7 +106,7 @@ function detectFormatter(projectRoot) {
    '.prettierrc.toml',
    'prettier.config.js',
    'prettier.config.cjs',
-    'prettier.config.mjs',
+    'prettier.config.mjs'
  ];
  for (const cfg of prettierConfigs) {
    if (fs.existsSync(path.join(projectRoot, cfg))) {
@@ -83,14 +119,35 @@ function detectFormatter(projectRoot) {
  return null;
 }

+/**
+ * Resolve the runner binary and prefix args for the configured package
+ * manager (respects CLAUDE_PACKAGE_MANAGER env and project config).
+ *
+ * @param {string} projectRoot - Absolute path to the project root
+ * @returns {{ bin: string, prefix: string[] }}
+ */
+// Windows .cmd shim mapping for cross-platform safety
+const WIN_CMD_SHIMS = { npx: 'npx.cmd', pnpm: 'pnpm.cmd', yarn: 'yarn.cmd', bunx: 'bunx.cmd' };
+
+function getRunnerFromPackageManager(projectRoot) {
+  const isWin = process.platform === 'win32';
+  const { getPackageManager } = require('./package-manager');
+  const pm = getPackageManager({ projectDir: projectRoot });
+  const execCmd = pm?.config?.execCmd || 'npx';
+  const [rawBin = 'npx', ...prefix] = execCmd.split(/\s+/).filter(Boolean);
+  const bin = isWin ? WIN_CMD_SHIMS[rawBin] || rawBin : rawBin;
+  return { bin, prefix };
+}
+
 /**
 * Resolve the formatter binary, preferring the local node_modules/.bin
- * installation over npx to avoid package-resolution overhead.
+ * installation over the package manager exec command to avoid
+ * package-resolution overhead.
 *
 * @param {string} projectRoot - Absolute path to the project root
 * @param {'biome' | 'prettier'} formatter - Detected formatter name
- * @returns {{ bin: string, prefix: string[] }}
- *   `bin`    – executable path (absolute local path or npx/npx.cmd)
+ * @returns {{ bin: string, prefix: string[] } | null}
+ *   `bin`    – executable path (absolute local path or runner binary)
 *   `prefix` – extra args to prepend (e.g. ['@biomejs/biome'] when using npx)
 */
 function resolveFormatterBin(projectRoot, formatter) {
@@ -98,45 +155,36 @@ function resolveFormatterBin(projectRoot, formatter) {
  if (binCache.has(cacheKey)) return binCache.get(cacheKey);

  const isWin = process.platform === 'win32';
-  const npxBin = isWin ? 'npx.cmd' : 'npx';

  if (formatter === 'biome') {
-    const localBin = path.join(
-      projectRoot,
-      'node_modules',
-      '.bin',
-      isWin ? 'biome.cmd' : 'biome',
-    );
+    const localBin = path.join(projectRoot, 'node_modules', '.bin', isWin ? 'biome.cmd' : 'biome');
    if (fs.existsSync(localBin)) {
      const result = { bin: localBin, prefix: [] };
      binCache.set(cacheKey, result);
      return result;
    }
-    const result = { bin: npxBin, prefix: ['@biomejs/biome'] };
+    const runner = getRunnerFromPackageManager(projectRoot);
+    const result = { bin: runner.bin, prefix: [...runner.prefix, '@biomejs/biome'] };
    binCache.set(cacheKey, result);
    return result;
  }

  if (formatter === 'prettier') {
-    const localBin = path.join(
-      projectRoot,
-      'node_modules',
-      '.bin',
-      isWin ? 'prettier.cmd' : 'prettier',
-    );
+    const localBin = path.join(projectRoot, 'node_modules', '.bin', isWin ? 'prettier.cmd' : 'prettier');
    if (fs.existsSync(localBin)) {
      const result = { bin: localBin, prefix: [] };
      binCache.set(cacheKey, result);
      return result;
    }
-    const result = { bin: npxBin, prefix: ['prettier'] };
+    const runner = getRunnerFromPackageManager(projectRoot);
+    const result = { bin: runner.bin, prefix: [...runner.prefix, 'prettier'] };
    binCache.set(cacheKey, result);
    return result;
  }

-  const result = { bin: npxBin, prefix: [] };
-  binCache.set(cacheKey, result);
-  return result;
+  // Unknown formatter — return null so callers can handle gracefully
+  binCache.set(cacheKey, null);
+  return null;
 }

 /**
@@ -152,5 +200,5 @@ module.exports = {
  findProjectRoot,
  detectFormatter,
  resolveFormatterBin,
-  clearCaches,
-};
+  clearCaches
+};
--- a/tests/lib/resolve-formatter.test.js
+++ b/tests/lib/resolve-formatter.test.js
@@ -9,14 +9,15 @@ const path = require('path');
 const fs = require('fs');
 const os = require('os');

-const {
-  findProjectRoot,
-  detectFormatter,
-  resolveFormatterBin,
-  clearCaches,
-} = require('../../scripts/lib/resolve-formatter');
+const { findProjectRoot, detectFormatter, resolveFormatterBin, clearCaches } = require('../../scripts/lib/resolve-formatter');

-// Test helper
+/**
+ * Run a single test case, printing pass/fail.
+ *
+ * @param {string} name - Test description
+ * @param {() => void} fn - Test body (throws on failure)
+ * @returns {boolean} Whether the test passed
+ */
 function test(name, fn) {
  try {
    fn();
@@ -29,8 +30,32 @@ function test(name, fn) {
  }
 }

+/** Track all created tmp dirs for cleanup */
+const tmpDirs = [];
+
+/**
+ * Create a temporary directory and track it for cleanup.
+ *
+ * @returns {string} Absolute path to the new temp directory
+ */
 function makeTmpDir() {
-  return fs.mkdtempSync(path.join(os.tmpdir(), 'resolve-fmt-'));
+  const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'resolve-fmt-'));
+  tmpDirs.push(dir);
+  return dir;
+}
+
+/**
+ * Remove all tracked temporary directories.
+ */
+function cleanupTmpDirs() {
+  for (const dir of tmpDirs) {
+    try {
+      fs.rmSync(dir, { recursive: true, force: true });
+    } catch {
+      // Best-effort cleanup
+    }
+  }
+  tmpDirs.length = 0;
 }

 function runTests() {
@@ -103,6 +128,18 @@ function runTests() {
    assert.strictEqual(detectFormatter(root), 'prettier');
  });

+  run('detectFormatter: detects prettier key in package.json', () => {
+    const root = makeTmpDir();
+    fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'test', prettier: { singleQuote: true } }));
+    assert.strictEqual(detectFormatter(root), 'prettier');
+  });
+
+  run('detectFormatter: ignores package.json without prettier key', () => {
+    const root = makeTmpDir();
+    fs.writeFileSync(path.join(root, 'package.json'), JSON.stringify({ name: 'test' }));
+    assert.strictEqual(detectFormatter(root), null);
+  });
+
  run('detectFormatter: biome takes priority over prettier', () => {
    const root = makeTmpDir();
    fs.writeFileSync(path.join(root, 'biome.json'), '{}');
@@ -157,6 +194,12 @@ function runTests() {
    assert.deepStrictEqual(result.prefix, ['prettier']);
  });

+  run('resolveFormatterBin: returns null for unknown formatter', () => {
+    const root = makeTmpDir();
+    const result = resolveFormatterBin(root, 'unknown');
+    assert.strictEqual(result, null);
+  });
+
  run('resolveFormatterBin: caches resolved binary', () => {
    const root = makeTmpDir();
    const binDir = path.join(root, 'node_modules', '.bin');
@@ -189,9 +232,14 @@ function runTests() {
    assert.strictEqual(detectFormatter(root), null);
  });

-  // ── Summary ───────────────────────────────────────────────────
+  // ── Summary & Cleanup ─────────────────────────────────────────

-  console.log(`\n  ${passed} passed, ${failed} failed\n`);
+  cleanupTmpDirs();
+
+  console.log('\n=== Test Results ===');
+  console.log(`Passed: ${passed}`);
+  console.log(`Failed: ${failed}`);
+  console.log(`Total:  ${passed + failed}`);
  process.exit(failed > 0 ? 1 : 0);
 }

--- a/tests/run-all.js
+++ b/tests/run-all.js
@@ -16,6 +16,7 @@ const testFiles = [
  'lib/session-manager.test.js',
  'lib/session-aliases.test.js',
  'lib/project-detect.test.js',
+  'lib/resolve-formatter.test.js',
  'hooks/hooks.test.js',
  'hooks/evaluate-session.test.js',
  'hooks/suggest-compact.test.js',
@@ -27,7 +28,7 @@ const testFiles = [
 ];

 const BOX_W = 58; // inner width between ║ delimiters
-const boxLine = (s) => `║${s.padEnd(BOX_W)}║`;
+const boxLine = s => `║${s.padEnd(BOX_W)}║`;

 console.log('╔' + '═'.repeat(BOX_W) + '╗');
 console.log(boxLine('           Everything Claude Code - Test Suite'));