feat(agent): prefer a detected real node runtime in the PATH shim

The node shim now probes PATH for a real Node >= 20 at session start and
execs it directly when found, keeping the ELECTRON_RUN_AS_NODE fallback
only for when that node later disappears. Candidates identifying as
Electron (process.versions.electron) are rejected; our own shim dirs are
skipped; POSTHOG_CODE_NODE_PATH overrides detection. The codex PATH
strip recognizes both wrapper variants via a shared isNodeShimScript
predicate instead of exact-content equality.

Generated-By: PostHog Code
Task-Id: ced8de81-260a-443e-823e-eb8828b89d07

Author: gantoine

packages/agent/src/utils/spawn-env.test.ts

@@ -11,6 +11,7 @@ function makeDir(
   kind:
     | "symlink-shim"
     | "wrapper-shim"
+    | "real-node-wrapper-shim"
     | "foreign-wrapper"
     | "other-symlink"
     | "real-node"
@@ -21,6 +22,8 @@ function makeDir(
   if (kind === "symlink-shim") symlinkSync(EXEC_PATH, node);
   if (kind === "wrapper-shim")
     writeFileSync(node, buildNodeShimScript(EXEC_PATH));
+  if (kind === "real-node-wrapper-shim")
+    writeFileSync(node, buildNodeShimScript(EXEC_PATH, "/usr/local/bin/node"));
   if (kind === "foreign-wrapper")
     writeFileSync(node, buildNodeShimScript("/some/other/app"));
   if (kind === "other-symlink") symlinkSync("/usr/bin/true", node);
@@ -32,13 +35,20 @@ describe("stripElectronNodeShimFromPath", () => {
   it("removes only dirs whose node aliases the executable", () => {
     const symlinkShim = makeDir("symlink-shim");
     const wrapperShim = makeDir("wrapper-shim");
+    const realNodeWrapperShim = makeDir("real-node-wrapper-shim");
     const foreign = makeDir("foreign-wrapper");
     const other = makeDir("other-symlink");
     const real = makeDir("real-node");
     const empty = makeDir("empty");
-    const input = [symlinkShim, wrapperShim, foreign, other, real, empty].join(
-      delimiter,
-    );
+    const input = [
+      symlinkShim,
+      wrapperShim,
+      realNodeWrapperShim,
+      foreign,
+      other,
+      real,
+      empty,
+    ].join(delimiter);
     expect(stripElectronNodeShimFromPath(input, EXEC_PATH)).toBe(
       [foreign, other, real, empty].join(delimiter),
     );

packages/agent/src/utils/spawn-env.ts

@@ -1,11 +1,14 @@
 import { lstatSync, readFileSync, readlinkSync } from "node:fs";
 import { delimiter, join } from "node:path";
-import { buildNodeShimScript } from "@posthog/shared/node-shim";
+import { isNodeShimScript } from "@posthog/shared/node-shim";
 
 // Removes PATH entries that alias `node` to the current executable, whether a
-// legacy symlink or the wrapper script written by ensureNodeShim. Codex
-// children must not resolve `node` to Electron's bundled runtime: native
-// modules they install target the real node ABI.
+// legacy symlink or either wrapper-script variant written by ensureNodeShim.
+// Codex children must not resolve `node` through the shim: with the Electron
+// fallback engaged, native modules they install target the wrong ABI. A
+// real-node-backed shim would be harmless, but stripping it too keeps codex
+// on the user's own PATH resolution (detection only ever picks a node that is
+// independently reachable there).
 export function stripElectronNodeShimFromPath(
   pathValue: string | undefined,
   execPath: string = process.execPath,
@@ -23,7 +26,7 @@ function isElectronNodeShimDir(dir: string, execPath: string): boolean {
     if (lstatSync(shimPath).isSymbolicLink()) {
       return readlinkSync(shimPath) === execPath;
     }
-    return readFileSync(shimPath, "utf-8") === buildNodeShimScript(execPath);
+    return isNodeShimScript(readFileSync(shimPath, "utf-8"), execPath);
   } catch {
     return false;
   }

packages/shared/src/node-shim.test.ts

@@ -1,21 +1,61 @@
 import { describe, expect, it } from "vitest";
-import { buildNodeShimScript } from "./node-shim";
+import { buildNodeShimScript, isNodeShimScript } from "./node-shim";
+
+const EXEC_PATH = "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code";
 
 describe("buildNodeShimScript", () => {
   it("sets ELECTRON_RUN_AS_NODE and execs the binary", () => {
-    const script = buildNodeShimScript(
-      "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code",
-    );
+    const script = buildNodeShimScript(EXEC_PATH);
     expect(script).toContain("#!/bin/sh");
     expect(script).toContain("export ELECTRON_RUN_AS_NODE=1");
-    expect(script).toContain(
-      'exec "/Applications/PostHog Code.app/Contents/MacOS/PostHog Code" "$@"',
-    );
+    expect(script).toContain(`exec "${EXEC_PATH}" "$@"`);
   });
 
-  it("escapes shell-special characters in the binary path", () => {
+  it("prefers the real node when given one, keeping the run-as-node fallback", () => {
+    const script = buildNodeShimScript(EXEC_PATH, "/usr/local/bin/node");
+    const lines = script.split("\n");
+    expect(lines[0]).toBe("#!/bin/sh");
+    expect(lines[1]).toBe('if [ -x "/usr/local/bin/node" ]; then');
+    expect(lines[2]).toBe('  exec "/usr/local/bin/node" "$@"');
+    expect(lines[3]).toBe("fi");
+    expect(script).toContain("export ELECTRON_RUN_AS_NODE=1");
+    expect(script).toContain(`exec "${EXEC_PATH}" "$@"`);
+  });
+
+  it("escapes shell-special characters in both paths", () => {
     expect(buildNodeShimScript('/odd/pa"th/$app`bin\\x')).toContain(
       'exec "/odd/pa\\"th/\\$app\\`bin\\\\x" "$@"',
     );
+    expect(buildNodeShimScript(EXEC_PATH, '/odd/no"de/$v')).toContain(
+      'exec "/odd/no\\"de/\\$v" "$@"',
+    );
+  });
+});
+
+describe("isNodeShimScript", () => {
+  it("recognizes both variants written for the same binary", () => {
+    expect(isNodeShimScript(buildNodeShimScript(EXEC_PATH), EXEC_PATH)).toBe(
+      true,
+    );
+    expect(
+      isNodeShimScript(
+        buildNodeShimScript(EXEC_PATH, "/usr/local/bin/node"),
+        EXEC_PATH,
+      ),
+    ).toBe(true);
+  });
+
+  it("rejects shims written for a different binary and non-shim content", () => {
+    expect(
+      isNodeShimScript(buildNodeShimScript("/some/other/app"), EXEC_PATH),
+    ).toBe(false);
+    expect(
+      isNodeShimScript(
+        buildNodeShimScript("/some/other/app", "/usr/local/bin/node"),
+        EXEC_PATH,
+      ),
+    ).toBe(false);
+    expect(isNodeShimScript("not a shim", EXEC_PATH)).toBe(false);
+    expect(isNodeShimScript("", EXEC_PATH)).toBe(false);
   });
 });

packages/shared/src/node-shim.ts

@@ -2,13 +2,44 @@ function escapeForDoubleQuotes(path: string): string {
   return path.replace(/([$`"\\])/g, "\\$1");
 }
 
-// Single source of truth for the PATH `node` shim format: workspace-server
-// writes it and the codex spawn path detects it by exact content.
-export function buildNodeShimScript(execPath: string): string {
+function fallbackBlock(execPath: string): string {
   return [
-    "#!/bin/sh",
     "export ELECTRON_RUN_AS_NODE=1",
     `exec "${escapeForDoubleQuotes(execPath)}" "$@"`,
     "",
   ].join("\n");
 }
+
+// Single source of truth for the PATH `node` shim format: workspace-server
+// writes it and the codex spawn path detects it via isNodeShimScript.
+//
+// With a realNodePath the shim prefers that binary and only falls back to
+// running the app binary as node when it has gone missing — e.g. an nvm
+// prune deleted the version between sessions.
+export function buildNodeShimScript(
+  execPath: string,
+  realNodePath?: string,
+): string {
+  if (!realNodePath) {
+    return `#!/bin/sh\n${fallbackBlock(execPath)}`;
+  }
+  const real = escapeForDoubleQuotes(realNodePath);
+  return [
+    "#!/bin/sh",
+    `if [ -x "${real}" ]; then`,
+    `  exec "${real}" "$@"`,
+    "fi",
+    fallbackBlock(execPath),
+  ].join("\n");
+}
+
+// True for any shim variant written for the given app binary, with or
+// without an embedded preferred real node — detectors (the codex PATH strip,
+// real-node discovery) can't know which real node path a shim embeds, so
+// they match on the fallback block every variant ends with.
+export function isNodeShimScript(content: string, execPath: string): boolean {
+  return (
+    content.startsWith("#!/bin/sh\n") &&
+    content.endsWith(fallbackBlock(execPath))
+  );
+}

packages/workspace-server/src/services/agent/agent.test.ts

@@ -104,6 +104,14 @@ const mockNodeShim = vi.hoisted(() => ({
 
 vi.mock("./node-shim", () => mockNodeShim);
 
+const mockRealNode = vi.hoisted(() => ({
+  findRealNode: vi.fn(
+    async (): Promise<{ path: string; version: string } | null> => null,
+  ),
+}));
+
+vi.mock("./real-node", () => mockRealNode);
+
 // --- Import after mocks ---
 import type { RegisteredFolder } from "../folders/schemas";
 import { AgentService, buildAutoApproveOutcome } from "./agent";
@@ -375,6 +383,21 @@ describe("AgentService", () => {
 
       expect(mockNodeShim.ensureNodeShim).toHaveBeenCalledTimes(2);
     });
+
+    it("passes the detected real node through to the shim writer", async () => {
+      mockRealNode.findRealNode.mockResolvedValueOnce({
+        path: "/usr/local/bin/node",
+        version: "v22.1.0",
+      });
+
+      await service.startSession({ ...baseSessionParams, adapter: "codex" });
+
+      expect(mockNodeShim.ensureNodeShim).toHaveBeenCalledWith(
+        expect.any(String),
+        process.execPath,
+        { realNodePath: "/usr/local/bin/node" },
+      );
+    });
   });
 
   describe("idle timeout", () => {

packages/workspace-server/src/services/agent/agent.ts

@@ -100,6 +100,7 @@ import type {
   AgentScopedLogger,
   AgentSleepCoordinator,
 } from "./ports";
+import { findRealNode } from "./real-node";
 import {
   AgentServiceEvent,
   type AgentServiceEvents,
@@ -370,6 +371,7 @@ export class AgentService extends TypedEventEmitter<AgentServiceEvents> {
   private sessions = new Map<string, ManagedSession>();
   private pendingPermissions = new Map<string, PendingPermission>();
   private mockNodeReady = false;
+  private nodeShimSetup: Promise<void> | null = null;
   private idleTimeouts = new Map<
     string,
     { handle: ReturnType<typeof setTimeout>; deadline: number }
@@ -761,7 +763,7 @@ If a repository IS genuinely required, attach one in this priority order:
     }
 
     const channel = `agent-event:${taskRunId}`;
-    const mockNodeDir = this.setupMockNodeEnvironment();
+    const mockNodeDir = await this.setupMockNodeEnvironment();
     const proxyUrl = await this.agentAuthAdapter.ensureGatewayProxy(
       credentials.apiHost,
     );
@@ -1596,19 +1598,42 @@ For git operations while detached:
     this.log.info("All agent sessions cleaned up");
   }
 
-  private setupMockNodeEnvironment(): string {
+  private async setupMockNodeEnvironment(): Promise<string> {
     const mockNodeDir = getMockNodeDir();
     if (!this.mockNodeReady) {
-      try {
-        ensureNodeShim(mockNodeDir, process.execPath);
-        this.mockNodeReady = true;
-      } catch (err) {
-        this.log.warn("Failed to setup mock node environment", err);
-      }
+      // Concurrent session starts share one setup; a failed run clears the
+      // memo so the next session retries.
+      this.nodeShimSetup ??= this.runNodeShimSetup(mockNodeDir);
+      await this.nodeShimSetup;
+      this.nodeShimSetup = null;
     }
     return mockNodeDir;
   }
 
+  private async runNodeShimSetup(mockNodeDir: string): Promise<void> {
+    // findRealNode absorbs its own failures (null → Electron fallback), so
+    // only the shim write can throw, leaving mockNodeReady false to retry.
+    const realNode = await findRealNode({
+      warn: (message, data) => this.log.warn(message, data),
+    });
+    try {
+      ensureNodeShim(mockNodeDir, process.execPath, {
+        realNodePath: realNode?.path,
+      });
+      this.mockNodeReady = true;
+      if (realNode) {
+        this.log.info("node shim prefers detected real node", {
+          path: realNode.path,
+          version: realNode.version,
+        });
+      } else {
+        this.log.info("node shim using Electron run-as-node fallback");
+      }
+    } catch (err) {
+      this.log.warn("Failed to setup mock node environment", err);
+    }
+  }
+
   private cancelInFlightMcpToolCalls(session: ManagedSession): void {
     for (const [toolCallId, toolKey] of session.inFlightMcpToolCalls) {
       this.mcpAppsService.notifyToolCancelled(toolKey, toolCallId);