Handle noisy shell output in executable lookup

packages/server/src/server/agent/provider-launch-config.test.ts

@@ -1,11 +1,43 @@
-import { describe, expect, test, vi } from "vitest";
+import { beforeEach, describe, expect, test, vi } from "vitest";
+
+const { execFileSyncMock, execSyncMock, existsSyncMock, platformMock } = vi.hoisted(
+  () => ({
+    execFileSyncMock: vi.fn(),
+    execSyncMock: vi.fn(),
+    existsSyncMock: vi.fn(),
+    platformMock: vi.fn(() => "darwin"),
+  })
+);
+
+vi.mock("node:child_process", () => ({
+  execFileSync: execFileSyncMock,
+  execSync: execSyncMock,
+}));
+
+vi.mock("node:fs", () => ({
+  existsSync: existsSyncMock,
+}));
+
+vi.mock("node:os", () => ({
+  platform: platformMock,
+}));
 
 import {
+  findExecutable,
   resolveProviderCommandPrefix,
   applyProviderEnv,
   type ProviderRuntimeSettings,
 } from "./provider-launch-config.js";
 
+beforeEach(() => {
+  execFileSyncMock.mockReset();
+  execSyncMock.mockReset();
+  existsSyncMock.mockReset();
+  platformMock.mockReset();
+  platformMock.mockReturnValue("darwin");
+  delete process.env["SHELL"];
+});
+
 describe("resolveProviderCommandPrefix", () => {
   test("uses resolved default command in default mode", () => {
     const resolveDefault = vi.fn(() => "/usr/local/bin/claude");
@@ -75,3 +107,33 @@ describe("applyProviderEnv", () => {
     });
   });
 });
+
+describe("findExecutable", () => {
+  test("uses the last line from login-shell which output", () => {
+    process.env["SHELL"] = "/bin/zsh";
+    execSyncMock.mockReturnValue("echo from profile\n/usr/local/bin/codex\n");
+
+    expect(findExecutable("codex")).toBe("/usr/local/bin/codex");
+    expect(execSyncMock).toHaveBeenCalledOnce();
+    expect(execFileSyncMock).not.toHaveBeenCalled();
+  });
+
+  test("warns and returns null when the final which line is not an absolute path", () => {
+    process.env["SHELL"] = "/bin/zsh";
+    execSyncMock.mockReturnValue("profile noise\ncodex\n");
+    execFileSyncMock.mockReturnValue("codex\n");
+    const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {});
+
+    expect(findExecutable("codex")).toBeNull();
+    expect(warnSpy).toHaveBeenCalledTimes(2);
+
+    warnSpy.mockRestore();
+  });
+
+  test("returns direct paths when they exist", () => {
+    existsSyncMock.mockReturnValue(true);
+
+    expect(findExecutable("/usr/local/bin/codex")).toBe("/usr/local/bin/codex");
+    expect(existsSyncMock).toHaveBeenCalledWith("/usr/local/bin/codex");
+  });
+});

packages/server/src/server/agent/provider-launch-config.ts

@@ -55,6 +55,31 @@ export type ProviderCommandPrefix = {
   args: string[];
 };
 
+function resolveExecutableFromWhichOutput(
+  name: string,
+  output: string,
+  source: "login-shell" | "which"
+): string | null {
+  const lines = output
+    .split(/\r?\n/)
+    .map((line) => line.trim())
+    .filter((line) => line.length > 0);
+  const candidate = lines.at(-1);
+
+  if (!candidate) {
+    return null;
+  }
+
+  if (!candidate.startsWith("/")) {
+    console.warn(
+      `[findExecutable] Ignoring non-absolute ${source} output for '${name}': ${JSON.stringify(candidate)}`
+    );
+    return null;
+  }
+
+  return candidate;
+}
+
 export function resolveProviderCommandPrefix(
   commandConfig: ProviderCommand | undefined,
   resolveDefaultCommand: () => string
@@ -127,8 +152,9 @@ export function findExecutable(name: string): string | null {
         encoding: "utf8",
         timeout: 5000,
       }).trim();
-      if (out) {
-        return out;
+      const resolved = resolveExecutableFromWhichOutput(trimmed, out, "login-shell");
+      if (resolved) {
+        return resolved;
       }
     } catch {
       // Login shell failed (broken rc, etc.) — fall through to plain which.
@@ -136,7 +162,11 @@ export function findExecutable(name: string): string | null {
   }
 
   try {
-    return execFileSync("which", [trimmed], { encoding: "utf8" }).trim() || null;
+    return resolveExecutableFromWhichOutput(
+      trimmed,
+      execFileSync("which", [trimmed], { encoding: "utf8" }).trim(),
+      "which"
+    );
   } catch {
     return null;
   }