diff --git a/src/cli/commands/wrap.ts b/src/cli/commands/wrap.ts index 35ca177..29e0952 100644 --- a/src/cli/commands/wrap.ts +++ b/src/cli/commands/wrap.ts @@ -58,7 +58,8 @@ export function registerWrapCommand( fs: container.fs, providerName: adapter.name, isolated, - argv: ["node", "poe-code", ...forwarded] + argv: ["node", "poe-code", ...forwarded], + commandRunner: container.commandRunner }); }); } diff --git a/src/cli/isolated-env-runner.ts b/src/cli/isolated-env-runner.ts index 4de55ff..af33bee 100644 --- a/src/cli/isolated-env-runner.ts +++ b/src/cli/isolated-env-runner.ts @@ -3,6 +3,8 @@ import type { ProviderIsolatedEnv } from "./service-registry.js"; import type { CliEnvironment } from "./environment.js"; import { resolveIsolatedEnvDetails } from "./isolated-env.js"; import type { FileSystem } from "../utils/file-system.js"; +import type { CommandRunner } from "../utils/command-checks.js"; +import { checkBinaryExists } from "../utils/command-checks.js"; export async function isolatedEnvRunner(input: { env: CliEnvironment; @@ -10,6 +12,7 @@ export async function isolatedEnvRunner(input: { isolated: ProviderIsolatedEnv; argv: string[]; fs?: FileSystem; + commandRunner?: CommandRunner; }): Promise { const details = resolveIsolatedEnvDetails( input.env, @@ -29,6 +32,16 @@ export async function isolatedEnvRunner(input: { ); } + if (input.commandRunner) { + try { + await checkBinaryExists(details.agentBinary, input.commandRunner); + } catch { + throw new Error( + `${input.providerName} binary "${details.agentBinary}" not found. Please ensure it is installed and available on PATH.` + ); + } + } + const child = spawn(details.agentBinary, args, { stdio: "inherit", env: { diff --git a/src/utils/command-checks.ts b/src/utils/command-checks.ts index 0d816a7..e648aed 100644 --- a/src/utils/command-checks.ts +++ b/src/utils/command-checks.ts @@ -161,42 +161,57 @@ export function createBinaryExistsCheck( id, description, async run({ runCommand }) { - const detectors: Array<{ - command: string; - args: string[]; - validate: (result: CommandRunnerResult) => boolean; - }> = [ - { - command: "which", - args: [binaryName], - validate: (result) => result.exitCode === 0 - }, - { - command: "where", - args: [binaryName], - validate: (result) => - result.exitCode === 0 && result.stdout.trim().length > 0 - }, - { - command: "test", - args: ["-f", `/usr/local/bin/${binaryName}`], - validate: (result) => result.exitCode === 0 - }, - { - command: "ls", - args: [`/usr/local/bin/${binaryName}`], - validate: (result) => result.exitCode === 0 - } - ]; - - for (const detector of detectors) { - const result = await runCommand(detector.command, detector.args); - if (detector.validate(result)) { - return; - } - } - - throw new Error(`${binaryName} CLI binary not found on PATH.`); + await checkBinaryExists(binaryName, runCommand); } }; } + +/** + * Checks if a binary exists on the system using multiple detection methods. + * Throws an error if the binary is not found. + * + * @param binaryName - The name of the binary to check for (e.g., "claude", "codex") + * @param runCommand - Function to execute shell commands + * @throws Error if the binary is not found on PATH + */ +export async function checkBinaryExists( + binaryName: string, + runCommand: CommandRunner +): Promise { + const detectors: Array<{ + command: string; + args: string[]; + validate: (result: CommandRunnerResult) => boolean; + }> = [ + { + command: "which", + args: [binaryName], + validate: (result) => result.exitCode === 0 + }, + { + command: "where", + args: [binaryName], + validate: (result) => + result.exitCode === 0 && result.stdout.trim().length > 0 + }, + { + command: "test", + args: ["-f", `/usr/local/bin/${binaryName}`], + validate: (result) => result.exitCode === 0 + }, + { + command: "ls", + args: [`/usr/local/bin/${binaryName}`], + validate: (result) => result.exitCode === 0 + } + ]; + + for (const detector of detectors) { + const result = await runCommand(detector.command, detector.args); + if (detector.validate(result)) { + return; + } + } + + throw new Error(`${binaryName} CLI binary not found on PATH.`); +} diff --git a/tests/isolated-env-runner.test.ts b/tests/isolated-env-runner.test.ts new file mode 100644 index 0000000..8601823 --- /dev/null +++ b/tests/isolated-env-runner.test.ts @@ -0,0 +1,217 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Volume, createFsFromVolume } from "memfs"; +import type { FileSystem } from "../src/utils/file-system.js"; +import type { CommandRunnerResult } from "../src/utils/command-checks.js"; +import { createCliEnvironment } from "../src/cli/environment.js"; +import type { ProviderIsolatedEnv } from "../src/cli/service-registry.js"; +import { EventEmitter } from "events"; + +function createMemFs(): FileSystem { + const vol = new Volume(); + vol.mkdirSync("/home/test/.poe-code/test-provider", { recursive: true }); + vol.writeFileSync("/home/test/.poe-code/test-provider/config.json", "{}"); + return createFsFromVolume(vol).promises as unknown as FileSystem; +} + +const mockSpawn = vi.fn(); + +vi.mock("node:child_process", () => ({ + spawn: mockSpawn +})); + +describe("isolatedEnvRunner", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSpawn.mockReset(); + }); + + it("checks binary exists before spawning", async () => { + const { isolatedEnvRunner } = await import("../src/cli/isolated-env-runner.js"); + + const fs = createMemFs(); + const env = createCliEnvironment({ + cwd: "/repo", + homeDir: "/home/test" + }); + + const mockCommandRunner = vi.fn(async (command: string): Promise => { + if (command === "which") { + return { stdout: "", stderr: "", exitCode: 1 }; + } + if (command === "where") { + return { stdout: "", stderr: "", exitCode: 1 }; + } + if (command === "test") { + return { stdout: "", stderr: "", exitCode: 1 }; + } + if (command === "ls") { + return { stdout: "", stderr: "", exitCode: 1 }; + } + return { stdout: "", stderr: "", exitCode: 0 }; + }); + + const isolated: ProviderIsolatedEnv = { + agentBinary: "nonexistent-binary", + configProbe: { + kind: "isolatedDir" + }, + env: {} + }; + + await expect( + isolatedEnvRunner({ + env, + providerName: "test-provider", + isolated, + argv: ["node", "poe-code", "--version"], + fs, + commandRunner: mockCommandRunner + }) + ).rejects.toThrow( + 'test-provider binary "nonexistent-binary" not found. Please ensure it is installed and available on PATH.' + ); + + expect(mockCommandRunner).toHaveBeenCalled(); + expect(mockSpawn).not.toHaveBeenCalled(); + }); + + it("skips binary check when commandRunner is not provided", async () => { + const { isolatedEnvRunner } = await import("../src/cli/isolated-env-runner.js"); + + const fs = createMemFs(); + const env = createCliEnvironment({ + cwd: "/repo", + homeDir: "/home/test" + }); + + // Mock spawn to simulate ENOENT error + mockSpawn.mockImplementation(() => { + const emitter = new EventEmitter(); + setImmediate(() => { + const error: NodeJS.ErrnoException = new Error("spawn test-binary ENOENT"); + error.code = "ENOENT"; + emitter.emit("error", error); + }); + return emitter; + }); + + const isolated: ProviderIsolatedEnv = { + agentBinary: "test-binary", + configProbe: { + kind: "isolatedDir" + }, + env: {} + }; + + const promise = isolatedEnvRunner({ + env, + providerName: "test-provider", + isolated, + argv: ["node", "poe-code", "--version"], + fs + }); + + // The function will attempt to spawn, which we expect to fail with ENOENT + // but it won't fail with our custom error message (since commandRunner is not provided) + await expect(promise).rejects.toThrow("spawn test-binary ENOENT"); + expect(mockSpawn).toHaveBeenCalledWith("test-binary", ["--version"], expect.any(Object)); + }); + + it("throws error when config does not exist", async () => { + const { isolatedEnvRunner } = await import("../src/cli/isolated-env-runner.js"); + + const vol = new Volume(); + vol.mkdirSync("/home/test", { recursive: true }); + const fs = createFsFromVolume(vol).promises as unknown as FileSystem; + + const env = createCliEnvironment({ + cwd: "/repo", + homeDir: "/home/test" + }); + + const mockCommandRunner = vi.fn(async (): Promise => ({ + stdout: "", + stderr: "", + exitCode: 0 + })); + + const isolated: ProviderIsolatedEnv = { + agentBinary: "test-binary", + configProbe: { + kind: "isolatedDir" + }, + env: {} + }; + + await expect( + isolatedEnvRunner({ + env, + providerName: "test-provider", + isolated, + argv: ["node", "poe-code", "--version"], + fs, + commandRunner: mockCommandRunner + }) + ).rejects.toThrow( + "test-provider is not configured. Run 'poe-code login' or 'poe-code configure test-provider'." + ); + + // Binary check should not be called if config doesn't exist + expect(mockCommandRunner).not.toHaveBeenCalled(); + expect(mockSpawn).not.toHaveBeenCalled(); + }); + + it("proceeds to spawn when binary exists", async () => { + const { isolatedEnvRunner } = await import("../src/cli/isolated-env-runner.js"); + + const fs = createMemFs(); + const env = createCliEnvironment({ + cwd: "/repo", + homeDir: "/home/test" + }); + + const mockCommandRunner = vi.fn(async (command: string): Promise => { + if (command === "which") { + return { stdout: "/usr/local/bin/test-binary", stderr: "", exitCode: 0 }; + } + return { stdout: "", stderr: "", exitCode: 1 }; + }); + + // Mock spawn to simulate spawn error (since binary doesn't really exist) + mockSpawn.mockImplementation(() => { + const emitter = new EventEmitter(); + setImmediate(() => { + const error: NodeJS.ErrnoException = new Error("spawn test-binary ENOENT"); + error.code = "ENOENT"; + emitter.emit("error", error); + }); + return emitter; + }); + + const isolated: ProviderIsolatedEnv = { + agentBinary: "test-binary", + configProbe: { + kind: "isolatedDir" + }, + env: {} + }; + + const promise = isolatedEnvRunner({ + env, + providerName: "test-provider", + isolated, + argv: ["node", "poe-code", "--version"], + fs, + commandRunner: mockCommandRunner + }); + + // The spawn will fail because the binary doesn't actually exist, + // but we should verify that the binary check passed + await expect(promise).rejects.toThrow("spawn test-binary ENOENT"); + + // Verify that the binary check was attempted and passed + expect(mockCommandRunner).toHaveBeenCalledWith("which", ["test-binary"]); + // Verify that spawn was called (meaning binary check passed) + expect(mockSpawn).toHaveBeenCalledWith("test-binary", ["--version"], expect.any(Object)); + }); +}); diff --git a/tests/wrap-command.test.ts b/tests/wrap-command.test.ts index c916691..e18b876 100644 --- a/tests/wrap-command.test.ts +++ b/tests/wrap-command.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; import { Volume, createFsFromVolume } from "memfs"; import type { FileSystem } from "../src/utils/file-system.js"; +import type { CommandRunnerResult } from "../src/utils/command-checks.js"; function createMemFs(): FileSystem { const vol = new Volume(); @@ -77,4 +78,35 @@ describe("wrap command", () => { }) ); }); + + it("passes commandRunner to isolatedEnvRunner", async () => { + const { createProgram } = await import("../src/cli/program.js"); + const runner = await import("../src/cli/isolated-env-runner.js"); + + const fs = createMemFs(); + const mockCommandRunner = vi.fn(async (): Promise => ({ + stdout: "", + stderr: "", + exitCode: 0 + })); + + const program = createProgram({ + fs, + prompts: vi.fn().mockResolvedValue({}), + env: { cwd: "/repo", homeDir: "/home/test" }, + logger: () => {}, + commandRunner: mockCommandRunner + }); + + await expect( + program.parseAsync(["node", "cli", "wrap", "codex", "--version"]) + ).rejects.toThrow("STOP_WRAP"); + + expect(runner.isolatedEnvRunner).toHaveBeenCalledWith( + expect.objectContaining({ + commandRunner: expect.any(Function), + providerName: "codex" + }) + ); + }); });