fix(pi): prevent recursive compiled OMP hook execution

Forward-port #2911 for #2909 and exercise the real adapter lifecycle with a recorded process boundary, including unavailable Node and invalid overrides.

Co-authored-by: DavidHLP <lysf15520112973@163.com>

.pi/README.md

@@ -90,8 +90,9 @@ The `extensions/index.ts` file handles:
 4. **Context injection** — Parses `hookSpecificOutput.additionalContext` from the SessionStart
    hook and appends it to the system prompt on the next `before_agent_start`, wrapped in an
    `<ecc-session-context>` block. Non-JSON hook output is tolerated, not treated as an error
-5. **Hook isolation** — Failing, missing, or slow hooks degrade to a warning and never
-   terminate the Pi session. Hook execution is bounded by a timeout and an output limit
+5. **Hook isolation** — Failing, missing, slow, or misconfigured hooks degrade to
+   a warning and never terminate the Pi session. Hook execution is bounded by a
+   timeout and an output limit
 6. **Package resolution** — Resolves hook scripts from the installed package via `__dirname`,
    never from `process.cwd()`, so a global install works from any project directory. Hooks
    still *run* in the user's project directory, so project detection stays correct
@@ -99,6 +100,13 @@ The `extensions/index.ts` file handles:
 All hook execution is non-shell (`execFile` without shell interpretation), so paths containing
 spaces, tabs, or shell metacharacters are safe.
 
+Hook runtime selection uses the host `process.execPath` only under Node.
+Without an override, compiled OMP/Bun falls back to `node` instead of
+recursively launching the OMP binary as a hook runner. Set `ECC_HOOK_NODE` to
+an explicit absolute Node executable path when `node` is not available on
+`PATH`.
+Relative values are rejected when the hook runs and surfaced as a warning.
+
 ## Scope
 
 Intentionally **out of scope** for this first adapter (to be added independently):

.pi/extensions/hook-runtime.js

@@ -0,0 +1,35 @@
+const path = require("node:path")
+
+/**
+ * Select a real Node executable for hook scripts.
+ *
+ * Compiled OMP may report `process.release.name` as `node` even though its
+ * `process.execPath` points to the OMP launcher. Bun is detected separately via
+ * `process.versions.bun`; both fall back to `node` unless `ECC_HOOK_NODE`
+ * supplies an explicit absolute path.
+ *
+ * @param options - Runtime metadata and an optional absolute Node override.
+ * @returns The executable path to use for hook scripts.
+ * @throws {Error} If the hook runtime override is non-empty and relative.
+ */
+function resolveHookRuntime({
+  execPath = process.execPath,
+  releaseName = process.release?.name,
+  bunVersion = process.versions?.bun,
+  override = process.env.ECC_HOOK_NODE,
+} = {}) {
+  const isNodeRuntime =
+    releaseName === "node" &&
+    !bunVersion &&
+    /^(?:node|nodejs)(?:\.exe)?$/i.test(path.basename(execPath))
+  const overridePath = override?.trim()
+  if (overridePath) {
+    if (!path.isAbsolute(overridePath)) {
+      throw new Error("ECC_HOOK_NODE must be an absolute path: " + overridePath)
+    }
+    return overridePath
+  }
+  return isNodeRuntime ? execPath : "node"
+}
+
+module.exports = { resolveHookRuntime }

.pi/extensions/index.ts

@@ -15,16 +15,20 @@
  * Design constraints (see .pi/README.md):
  *   - Hooks resolve relative to THIS file, never `process.cwd()`, so a global
  *     `pi install` works from any project directory.
- *   - Hooks execute via `execFile(process.execPath, [...])` with no shell, so
- *     paths containing spaces or shell metacharacters are safe.
- *   - Hook failures are isolated: a broken, missing, or slow hook degrades to a
- *     warning and never terminates the Pi session.
+ *   - Hooks execute via `execFile(hookRuntime, [...])` with no shell, so paths
+ *     containing spaces or shell metacharacters are safe. The hook runtime is
+ *     selected separately because compiled OMP may report `process.release.name`
+ *     as `node` while `process.execPath` points back to `omp`; Bun is detected
+ *     separately via `process.versions.bun`.
+ *   - Hook failures are isolated: a broken, missing, slow, or misconfigured hook
+ *     degrades to a warning and never terminates the Pi session.
  */
 
 import { execFile } from "node:child_process"
 import * as fs from "node:fs"
 import * as os from "node:os"
 import * as path from "node:path"
+import { resolveHookRuntime } from "./hook-runtime.js"
 
 /**
  * Minimal structural types mirroring `@earendil-works/pi-coding-agent`.
@@ -175,8 +179,9 @@ interface HookResult {
 /**
  * Run an ECC hook through ECC's own runner.
  *
- * Never rejects: a missing runner, a non-zero exit, a timeout, or a spawn error
- * all resolve to a `failure` string that the caller surfaces as a warning.
+ * Never rejects: an invalid runtime override, a missing runner, a non-zero exit,
+ * a timeout, or a spawn error all resolve to a `failure` string that the caller
+ * surfaces as a warning.
  */
 function runEccHook(
   spec: HookSpec,
@@ -189,9 +194,19 @@ function runEccHook(
       resolve({ stdout: "", failure: `hook runner not found at ${HOOK_RUNNER}` })
       return
     }
+    let hookRuntime: string
+    try {
+      hookRuntime = resolveHookRuntime()
+    } catch (error) {
+      resolve({
+        stdout: "",
+        failure: `${spec.id}: ${(error as Error).message}`,
+      })
+      return
+    }
 
     const child = execFile(
-      process.execPath,
+      hookRuntime,
       [HOOK_RUNNER, spec.id, spec.script, spec.profiles],
       {
         // Hooks inspect the user's project, so they run there. Only the script

tests/pi/pi-extension-adapter.test.js

@@ -45,7 +45,11 @@ const fs = require("fs")
 const os = require("os")
 const path = require("path")
 const { spawnSync, execFile } = require("child_process")
+const { resolveHookRuntime } = require(
+  path.join(__dirname, "..", "..", ".pi", "extensions", "hook-runtime.js")
+)
 
+/** Run a single adapter test and report the result. */
 async function runTest(name, fn) {
   try {
     await fn()
@@ -70,9 +74,9 @@ function stripComments(source) {
 }
 
 /**
- * Mirrors the adapter's own hook invocation (`runEccHook` in
- * .pi/extensions/index.ts): same binary (`process.execPath`), same argv
- * shape, same stdin-JSON payload, same env keys. No shell is used anywhere.
+ * Invokes ECC's hook runner like the adapter (`runEccHook` in
+ * .pi/extensions/index.ts): it uses the test host's Node executable with the
+ * same argv shape and JSON payload on stdin. No shell is used.
  */
 function runHookRunner(eccRoot, hookId, relScript, profiles, payload, extraEnv, cwd) {
   const runner = path.join(eccRoot, "scripts", "hooks", "run-with-flags.js")
@@ -283,6 +287,7 @@ function isDisabledByEnvMirror(value) {
   return typeof value === "string" && DISABLED_VALUES_MIRROR.has(value.trim().toLowerCase())
 }
 
+/** Run the Pi adapter regression suite. */
 async function main() {
   console.log("\n=== Testing .pi/extensions/index.ts (Pi thin adapter) ===\n")
 
@@ -320,8 +325,8 @@ async function main() {
         "expected the adapter to invoke hooks via child_process.execFile(...)"
       )
       assert.ok(
-        extensionSource.includes("process.execPath"),
-        "expected hooks to be spawned with process.execPath, not a hardcoded 'node' string"
+        extensionSource.includes("resolveHookRuntime"),
+        "expected the adapter to select a hook runtime before execFile(...)"
       )
 
       const shellExecPattern = /(?<!execFile)\bexec\s*\(/
@@ -343,6 +348,95 @@ async function main() {
           "the path-with-spaces / injection risk execFile(...) with no shell was meant to avoid"
       )
     }],
+    ["selects a real Node runtime instead of compiled OMP's masquerading process.execPath", () => {
+      const runtimeSource = fs.readFileSync(
+        path.join(repoRoot, ".pi", "extensions", "hook-runtime.js"),
+        "utf8"
+      )
+      const runHookStart = extensionSource.indexOf("function runEccHook")
+      const runHookEnd = extensionSource.indexOf("function resolveHookCwd")
+      const runHookSource = extensionSource.slice(runHookStart, runHookEnd)
+      const beforeRunHookSource = extensionSource.slice(0, runHookStart)
+      assert.ok(
+        extensionSource.includes('from "./hook-runtime.js"'),
+        "expected the adapter to import the shared hook runtime selector"
+      )
+      assert.ok(
+        !beforeRunHookSource.includes("resolveHookRuntime()") &&
+          /try\s*\{\s*hookRuntime = resolveHookRuntime\(\)\s*\}\s*catch/.test(runHookSource) &&
+          /execFile\(\s*hookRuntime,/.test(runHookSource),
+        "expected runEccHook to resolve its runtime inside the guarded hook path rather than " +
+          "during module initialization"
+      )
+      assert.ok(
+        runtimeSource.includes("process.versions?.bun") &&
+          runtimeSource.includes("path.basename(execPath)") &&
+          runtimeSource.includes("path.isAbsolute(overridePath)") &&
+          runtimeSource.includes(
+            'throw new Error("ECC_HOOK_NODE must be an absolute path: " + overridePath)'
+          ),
+        "expected the selector to reject Bun/OMP runtimes, require absolute overrides, and " +
+          "fall back to PATH node"
+      )
+    }],
+
+    ["resolves hook runtimes across Node, compiled OMP, and explicit override cases", () => {
+      assert.strictEqual(
+        resolveHookRuntime({ execPath: "/usr/bin/node", override: "" }),
+        "/usr/bin/node"
+      )
+      assert.strictEqual(
+        resolveHookRuntime({ execPath: "/usr/bin/nodejs", override: "" }),
+        "/usr/bin/nodejs"
+      )
+      assert.strictEqual(
+        resolveHookRuntime({
+          execPath: "/usr/bin/node",
+          bunVersion: "1.4.0",
+          override: "",
+        }),
+        "node"
+      )
+      assert.strictEqual(
+        resolveHookRuntime({
+          execPath: "/usr/bin/node",
+          releaseName: "bun",
+          override: "",
+        }),
+        "node"
+      )
+      assert.strictEqual(
+        resolveHookRuntime({
+          execPath: "/home/user/.omp/bin/omp",
+          releaseName: "node",
+          override: "",
+        }),
+        "node"
+      )
+      assert.throws(
+        () =>
+          resolveHookRuntime({
+            execPath: "/usr/bin/node",
+            override: "./node",
+          }),
+        /ECC_HOOK_NODE must be an absolute path: \.\/node/
+      )
+      assert.strictEqual(
+        resolveHookRuntime({
+          execPath: "/usr/bin/node",
+          override: " /opt/node/bin/node ",
+        }),
+        "/opt/node/bin/node"
+      )
+      assert.strictEqual(
+        resolveHookRuntime({
+          execPath: "/home/user/.omp/bin/omp",
+          bunVersion: "1.4.0",
+          override: " /opt/node/bin/node ",
+        }),
+        "/opt/node/bin/node"
+      )
+    }],
 
     ["registers Pi's documented pi.on(...) lifecycle, not the undocumented app.events bus", () => {
       assert.ok(

tests/pi/pi-hook-runtime.test.js

@@ -0,0 +1,120 @@
+#!/usr/bin/env node
+'use strict';
+
+// Run the real adapter against a recording process boundary. A simulated OMP
+// executable is never launched, so a regression cannot create a process storm.
+const assert = require('assert');
+const fs = require('fs');
+const path = require('path');
+const vm = require('vm');
+const { EventEmitter } = require('events');
+const ts = require('typescript');
+
+const extensionDir = path.resolve(__dirname, '../../.pi/extensions');
+const extensionSource = fs.readFileSync(path.join(extensionDir, 'index.ts'), 'utf8');
+const compiled = ts.transpileModule(extensionSource, {
+  compilerOptions: { module: ts.ModuleKind.CommonJS, target: ts.ScriptTarget.ES2020 }
+}).outputText;
+
+/** Load the adapter and its runtime selector with the same host metadata. */
+function loadAdapter(host, spawnError) {
+  const launches = [];
+  const handlers = new Map();
+  const warnings = [];
+  const runtimeModule = { exports: {} };
+  const simulatedProcess = { env: {}, release: { name: 'node' }, versions: {}, ...host };
+  vm.runInNewContext(fs.readFileSync(path.join(extensionDir, 'hook-runtime.js'), 'utf8'), {
+    module: runtimeModule, require, process: simulatedProcess
+  });
+  const adapterModule = { exports: {} };
+  const recordExecFile = (file, args, options, callback) => {
+    const call = { file, args, options };
+    launches.push(call);
+    const child = new EventEmitter();
+    child.stdin = new EventEmitter();
+    child.stdin.end = input => {
+      call.input = JSON.parse(input);
+      callback(spawnError || null, '');
+    };
+    return child;
+  };
+  vm.runInNewContext(compiled, {
+    module: adapterModule,
+    exports: adapterModule.exports,
+    __dirname: extensionDir,
+    process: simulatedProcess,
+    require: name => {
+      if (name === 'node:child_process') return { execFile: recordExecFile };
+      if (name === './hook-runtime.js') return runtimeModule.exports;
+      return require(name);
+    }
+  });
+  adapterModule.exports.default({
+    on: (name, handler) => handlers.set(name, handler),
+    registerCommand: () => {}
+  });
+  const context = {
+    cwd: path.resolve(__dirname, '../..'),
+    sessionManager: { getSessionId: () => 'runtime-regression' },
+    ui: { notify: message => warnings.push(message) }
+  };
+  return { launches, warnings, run: () => handlers.get('session_start')({ reason: 'resume' }, context) };
+}
+
+/** Exercise the actual lifecycle entrypoint without spawning host executables. */
+async function main() {
+  let passed = 0;
+  let failed = 0;
+  const explicitNode = path.resolve('test node runtime', 'node');
+  const cases = [
+    ['normal Node', { execPath: process.execPath }, process.execPath],
+    ['compiled OMP reporting Node', { execPath: '/fake/omp' }, 'node'],
+    ['compiled OMP on Bun', { execPath: '/fake/omp', versions: { bun: '1.4.0' } }, 'node'],
+    ['Bun with a Node basename', { execPath: '/fake/node', versions: { bun: '1.4.0' } }, 'node'],
+    ['explicit absolute Node path with spaces', {
+      execPath: '/fake/omp', env: { ECC_HOOK_NODE: explicitNode }
+    }, explicitNode]
+  ];
+  for (const [name, host, expected] of cases) {
+    try {
+      const adapter = loadAdapter(host);
+      await adapter.run();
+      assert.strictEqual(adapter.launches.length, 1);
+      const launch = adapter.launches[0];
+      assert.strictEqual(launch.file, expected);
+      assert.strictEqual(launch.args[1], 'session:start');
+      assert.strictEqual(launch.input.source, 'resume');
+      assert.strictEqual(launch.input.session_id, 'runtime-regression');
+      assert.ok(launch.options.timeout > 0 && launch.options.timeout <= 30000);
+      assert.ok(launch.options.maxBuffer > 0 && launch.options.maxBuffer <= 16 * 1024 * 1024);
+      assert.ok(!launch.options.shell);
+      assert.strictEqual(adapter.warnings.length, 0);
+      console.log(`  ✓ ${name} launches exactly one bounded Node hook`);
+      passed++;
+    } catch (error) {
+      console.error(`  ✗ ${name}: ${error.message}`);
+      failed++;
+    }
+  }
+  for (const [name, host, spawnError, expectedLaunches] of [
+    ['invalid override', { execPath: '/fake/omp', env: { ECC_HOOK_NODE: './omp' } }, null, 0],
+    ['missing PATH node', { execPath: '/fake/omp' }, new Error('spawn node ENOENT'), 1]
+  ]) {
+    try {
+      const adapter = loadAdapter(host, spawnError);
+      await adapter.run();
+      assert.strictEqual(adapter.launches.length, expectedLaunches);
+      assert.strictEqual(adapter.warnings.length, 1);
+      assert.match(adapter.warnings[0], /hook skipped/);
+      console.log(`  ✓ ${name} warns without retrying the host executable`);
+      passed++;
+    } catch (error) {
+      console.error(`  ✗ ${name}: ${error.message}`);
+      failed++;
+    }
+  }
+  console.log(`\nPassed: ${passed}\nFailed: ${failed}`);
+  process.exitCode = failed ? 1 : 0;
+}
+
+main().catch(error => { console.error(error); process.exitCode = 1; });