diff --git a/packages/harness/README.md b/packages/harness/README.md index 34ca20bd9..274a16a8a 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -103,6 +103,27 @@ Architecture: a single Node process (Express + ws + node-pty) serves the built SPA, a small REST API, terminal WebSocket streams, and the local telemetry ingest endpoint. The interface contract lives in `src/shared/types.ts`. +### Codex MCP validation + +The ordinary unit and server tests cover launch/resume conversion, login and +credential refresh, logout, and error reporting. To verify actual tool discovery +with an installed Codex CLI, build the harness's workspace dependencies, then +run the opt-in test: + +```bash +pnpm --filter "@sapiom/harness^..." build +RUN_CODEX_MCP_INTEGRATION=1 pnpm --filter @sapiom/harness exec vitest run src/core/adapters/codex-mcp.integration.test.ts +``` + +The test runs the `codex` binary found on `PATH`. Set `CODEX_TEST_BINARY` to the +path of a different installed version to test that one instead. + +This test uses a temporary Codex home, the built `sapiom-dev` server, and local +HTTP fixtures. It requires no Codex login or model request and checks both a +fresh home and an existing configuration with conflicting server registrations. +It also checks that command environments exclude MCP credentials and the Electron +launch flag while preserving unrelated user shell settings. + ### Project sessions and Agent Map bootstrap Every session whose working directory resolves to a Studio project is an diff --git a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts new file mode 100644 index 000000000..2f99c4444 --- /dev/null +++ b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts @@ -0,0 +1,403 @@ +/** + * Opt-in vendor compatibility test: a real Codex process discovers the built + * sapiom-dev server through Studio's generated config, without a model or login. + * See "Codex MCP validation" in the harness README for how to run it. All auth + * is synthetic, all HTTP endpoints are loopback, and both homes are temporary. + */ +import { spawn } from "node:child_process"; +import { createServer, type Server } from "node:http"; +import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { createInterface } from "node:readline"; +import { fileURLToPath } from "node:url"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; +import type { SpawnSpec } from "../../shared/types.js"; +import { generateMcpConfig } from "../inject/mcp-config.js"; +import { CodexAdapter } from "./codex.js"; + +interface McpStatus { + name: string; + serverInfo?: { name: string } | null; + tools: Record; + toolsError?: string | null; +} + +/** Discover MCP tools and inspect shell settings through an isolated Codex + * process. Always stop the child, including after a failed request. */ +async function discover( + spec: SpawnSpec, + home: string, +): Promise<{ + servers: McpStatus[]; + commandEnv: Record; +}> { + const env: NodeJS.ProcessEnv = { + PATH: process.env.PATH, + HOME: home, + CODEX_HOME: join(home, ".codex"), + TMPDIR: process.env.TMPDIR, + SAPIOM_TELEMETRY_DISABLED: "1", + DO_NOT_TRACK: "1", + SAPIOM_REVIEW_EXCLUDED: "must-not-reach-commands", + // No vendor network is needed. Prevent optional Codex metadata fetches. + HTTP_PROXY: "http://127.0.0.1:9", + HTTPS_PROXY: "http://127.0.0.1:9", + ALL_PROXY: "http://127.0.0.1:9", + NO_PROXY: "127.0.0.1,localhost", + }; + for (const [key, value] of Object.entries(spec.env)) { + if (value !== null) env[key] = value; + } + // Launch/resume parity is covered by the adapter and lifecycle tests. + // This probe verifies the real CLI's parsing and MCP discovery. + const child = spawn( + process.env.CODEX_TEST_BINARY ?? "codex", + ["app-server", ...spec.args], + { cwd: spec.cwd, env, stdio: ["pipe", "pipe", "pipe"] }, + ); + const lines = createInterface({ input: child.stdout }); + const pending = new Map< + number, + { + method: string; + resolve: (result: unknown) => void; + reject: (error: Error) => void; + } + >(); + let id = 0; + // Settles once with the first failure. Every request races it, so a dead + // CLI rejects immediately instead of stalling until the timeout. Failures + // name their cause and the CLI's JSON-RPC error, never raw stderr or + // credentials, which reach Codex only through the environment. + let terminate: (error: Error) => void = () => {}; + const closed = new Promise((_, reject) => { + terminate = reject; + }); + // Insurance only: the first request subscribes before anything can reject. + void closed.catch(() => {}); + const fail = (cause: string): void => + terminate(new Error(`Codex MCP probe failed: ${cause}.`)); + // Drain stderr so the CLI never blocks on it; it is never quoted, so nothing + // is captured. + child.stderr.resume(); + child.stdin.on("error", () => { + // A failed write means the CLI is gone; "close" reports why. + }); + child.once("error", (error) => + fail(`could not start the CLI (${error.message})`), + ); + child.once("close", (code, signal) => + fail(`the CLI exited with ${signal ?? `code ${code}`}`), + ); + lines.on("line", (line) => { + let response: { + id?: number; + error?: { code?: number; message?: string }; + result?: unknown; + }; + try { + response = JSON.parse(line); + } catch { + return; + } + if (response.id === undefined) return; + const request = pending.get(response.id); + if (!request) return; + pending.delete(response.id); + if (response.error) { + const { code, message } = response.error; + request.reject( + new Error(`Codex ${request.method} failed (${code}): ${message}`), + ); + return; + } + request.resolve(response.result); + }); + const request = (method: string, params: unknown): Promise => + Promise.race([ + closed, + new Promise((resolve, reject) => { + const requestId = ++id; + pending.set(requestId, { method, resolve, reject }); + child.stdin.write( + JSON.stringify({ id: requestId, method, params }) + "\n", + ); + }), + ]); + const timeoutMs = 25_000; + const timeout = setTimeout(() => { + const inFlight = [...pending.values()].map((r) => r.method).join(", "); + fail( + `no response to ${inFlight || "any request"} within ${timeoutMs / 1000}s`, + ); + child.kill("SIGKILL"); + }, timeoutMs); + try { + await request("initialize", { + clientInfo: { name: "sapiom_mcp_test", version: "0.0.0" }, + capabilities: { experimentalApi: true }, + }); + child.stdin.write(JSON.stringify({ method: "initialized" }) + "\n"); + const result = (await request("mcpServerStatus/list", { + detail: "full", + })) as { + data: McpStatus[]; + }; + const names = [ + ...Object.keys(spec.env), + "ELECTRON_RUN_AS_NODE", + "SAPIOM_REVIEW_USER_SETTING", + "SAPIOM_REVIEW_EXCLUDED", + ]; + const command = (await request("command/exec", { + command: [ + process.execPath, + "-e", + `process.stdout.write(JSON.stringify(Object.fromEntries(${JSON.stringify(names)}.map(name => [name, process.env[name] ?? null]))))`, + ], + cwd: home, + timeoutMs: 5_000, + })) as { exitCode: number; stdout: string }; + if (command.exitCode !== 0) + throw new Error( + `Codex command environment probe exited with code ${command.exitCode}.`, + ); + return { servers: result.data, commandEnv: JSON.parse(command.stdout) }; + } finally { + clearTimeout(timeout); + lines.close(); + child.stdin.end(); + child.kill(); + await new Promise((resolve) => { + if (child.exitCode !== null || child.signalCode !== null) + return resolve(); + const killTimeout = setTimeout(() => child.kill("SIGKILL"), 2_000); + child.once("exit", () => { + clearTimeout(killTimeout); + resolve(); + }); + }); + } +} + +describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( + "real Codex Studio MCP discovery", + () => { + let root: string; + let http: Server | undefined; + const mcpConnections: McpServer[] = []; + + afterEach(async () => { + vi.unstubAllEnvs(); + await Promise.all( + mcpConnections.splice(0).map((server) => server.close()), + ); + http?.closeAllConnections(); + await new Promise((resolve) => + http ? http.close(() => resolve()) : resolve(), + ); + http = undefined; + if (root) await rm(root, { recursive: true, force: true, maxRetries: 5 }); + }); + + it.each([false, true])( + "discovers authoring tools with existing global registrations=%s", + async (existingRegistrations) => { + root = await mkdtemp(join(tmpdir(), "studio-codex-mcp-")); + const codexHome = join(root, ".codex"); + await mkdir(codexHome); + await mkdir(join(root, ".sapiom")); + const fixtureKey = "synthetic-integration-credential"; + let authenticated = false; + http = createServer(async (req, res) => { + try { + if (req.url === "/v1/mcp/instructions") { + res.end("Local authoring integration test."); + return; + } + if (req.url !== "/v1/mcp" && req.url !== "/user-mcp") { + res.writeHead(404).end(); + return; + } + if (req.url === "/v1/mcp") { + if (req.headers["x-api-key"] !== fixtureKey) { + res.writeHead(401).end(); + return; + } + authenticated = true; + } + const mcp = new McpServer({ + name: "loopback-capabilities", + version: "1.0.0", + }); + mcp.registerTool( + "local_capability_probe", + { inputSchema: {} }, + async () => ({ + content: [{ type: "text", text: "local" }], + }), + ); + const transport = new StreamableHTTPServerTransport({ + sessionIdGenerator: undefined, + enableJsonResponse: true, + }); + mcpConnections.push(mcp); + await mcp.connect(transport); + await transport.handleRequest(req, res); + } catch { + // Node does not await this callback. End failed requests without + // exposing credentials through an unhandled rejection or response. + if (!res.headersSent) res.writeHead(500); + res.end(); + } + }); + await new Promise((resolve) => + http!.listen(0, "127.0.0.1", resolve), + ); + const address = http.address(); + if (!address || typeof address === "string") + throw new Error("Missing test port"); + const apiURL = `http://127.0.0.1:${address.port}`; + await writeFile( + join(root, ".sapiom", "credentials.json"), + JSON.stringify({ + currentEnvironment: "integration", + environments: { integration: { apiURL, appURL: apiURL } }, + }), + ); + vi.stubEnv("HOME", root); + vi.stubEnv("SAPIOM_ENVIRONMENT", "integration"); + const globalConfig = [ + 'model_provider = "integration"', + 'model = "integration"', + "[model_providers.integration]", + 'name = "Integration"', + 'base_url = "http://127.0.0.1:9/v1"', + 'wire_api = "responses"', + "requires_openai_auth = false", + "[shell_environment_policy]", + 'exclude = ["SAPIOM_REVIEW_EXCLUDED"]', + "[shell_environment_policy.set]", + 'SAPIOM_REVIEW_USER_SETTING = "preserved"', + ...(existingRegistrations + ? [ + // Opposite transport and stale auth must not contaminate Studio's + // entries. Unrelated user servers remain present and unchanged. + "[mcp_servers.sapiom]", + 'command = "unused-global-command"', + "enabled = false", + "[mcp_servers.sapiom-dev]", + 'url = "http://127.0.0.1:9/stale"', + "enabled = false", + "[mcp_servers.user-server]", + `url = "${apiURL}/user-mcp"`, + ] + : []), + ].join("\n"); + await writeFile(join(codexHome, "config.toml"), globalConfig); + // Observe the actual authoring process before its normal entrypoint. + // Fresh config has only local settings; the conflict case also forwards + // a synthetic secret. Both must augment the inherited child environment. + const stdioEnvFile = join(root, "stdio-env.json"); + const observer = join(root, "observe-stdio.cjs"); + await writeFile( + observer, + `require("node:fs").writeFileSync(${JSON.stringify(stdioEnvFile)}, JSON.stringify({ + electron: process.env.ELECTRON_RUN_AS_NODE, + environment: process.env.SAPIOM_ENVIRONMENT, + version: process.env.SAPIOM_HARNESS_VERSION, + forwarded: process.env.SAPIOM_REVIEW_STDIO_SECRET ?? null, + home: process.env.HOME, + hasPath: Boolean(process.env.PATH), + }), { mode: 0o600 });`, + ); + const mcpConfigFile = await generateMcpConfig("integration-session", { + generatedRoot: join(root, "generated"), + apiKey: fixtureKey, + environment: "integration", + harnessVersion: "integration-version", + devServer: { + command: process.execPath, + args: [ + "--require", + observer, + fileURLToPath( + new URL("../../../../mcp/dist/index.js", import.meta.url), + ), + ], + env: { + ELECTRON_RUN_AS_NODE: "1", + ...(existingRegistrations + ? { SAPIOM_REVIEW_STDIO_SECRET: "synthetic-stdio-secret" } + : {}), + }, + }, + }); + const adapter = new CodexAdapter(); + const opts = { + cwd: root, + harnessSessionId: "integration-session", + mcpConfigFile, + }; + const spec = adapter.launch(opts); + authenticated = false; + expect(spec.args.join(" ").includes(fixtureKey)).toBe(false); + const { servers, commandEnv } = await discover(spec, root); + expect(JSON.parse(await readFile(stdioEnvFile, "utf8"))).toEqual({ + electron: "1", + environment: "integration", + version: "integration-version", + forwarded: existingRegistrations ? "synthetic-stdio-secret" : null, + home: root, + hasPath: true, + }); + expect(commandEnv.ELECTRON_RUN_AS_NODE).toBeNull(); + for (const name of Object.keys(spec.env)) + expect(commandEnv[name]).toBe(""); + expect(commandEnv.SAPIOM_REVIEW_USER_SETTING).toBe("preserved"); + expect(commandEnv.SAPIOM_REVIEW_EXCLUDED).toBeNull(); + const authoring = servers.find( + (server) => server.serverInfo?.name === "sapiom-dev", + ); + // A server that failed to start is listed without serverInfo. Surface + // Codex's reason, usually an unbuilt workspace dependency. + if (!authoring) + throw new Error( + `sapiom-dev did not start: ${servers + .map((server) => `${server.name}: ${server.toolsError ?? "ok"}`) + .join("; ")}`, + ); + expect(Object.keys(authoring.tools ?? {})).toContain( + "sapiom_dev_agents_check", + ); + const remote = servers.find( + (server) => + server.serverInfo?.name === "loopback-capabilities" && + server.name !== "user-server", + ); + expect(Object.keys(remote?.tools ?? {})).toContain( + "local_capability_probe", + ); + if (existingRegistrations) { + expect(servers.map((server) => server.name)).toEqual( + expect.arrayContaining(["sapiom", "sapiom-dev", "user-server"]), + ); + expect( + Object.keys( + servers.find((server) => server.name === "user-server")?.tools ?? + {}, + ), + ).toContain("local_capability_probe"); + } + expect(await readFile(join(codexHome, "config.toml"), "utf8")).toBe( + globalConfig, + ); + expect(authenticated).toBe(true); + }, + 60_000, + ); + }, +); diff --git a/packages/harness/src/server/auth-mcp-wiring.test.ts b/packages/harness/src/server/auth-mcp-wiring.test.ts index 423fd256c..d4be43e26 100644 --- a/packages/harness/src/server/auth-mcp-wiring.test.ts +++ b/packages/harness/src/server/auth-mcp-wiring.test.ts @@ -1,5 +1,5 @@ /** - * Lifecycle-level regression coverage for SAP-3114. + * Lifecycle-level regression coverage for SAP-3114 and SAP-3122. * * These tests boot the real server and exercise the shared launch builder used * by interactive create/resume and headless background tasks. OAuth and the @@ -7,7 +7,7 @@ * synchronously before launching local throwaway processes. Nothing opens a * browser or contacts a Sapiom environment. */ -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -92,6 +92,7 @@ import { } from "@sapiom/mcp/auth"; import { startServer, type HarnessServer } from "./index.js"; import type { HarnessAdapter, LaunchOpts, SpawnSpec } from "../shared/types.js"; +import { CodexAdapter } from "../core/adapters/codex.js"; type LaunchKind = "create" | "resume" | "background"; @@ -102,6 +103,36 @@ interface CapturedLaunch { url: string; headers?: Record; }; + spec?: SpawnSpec; +} + +/** Exercise the real Codex conversion, substituting only the final process. + * CLI parsing and actual MCP discovery are covered by the opt-in live test. */ +function capturingCodexAdapter( + captures: CapturedLaunch[], + beforeLaunch?: (opts: LaunchOpts) => void, +): HarnessAdapter { + const adapter = new CodexAdapter(); + /** Capture the real Codex arguments and config, then launch local Bash. */ + const interactiveSpec = (kind: "create" | "resume", opts: LaunchOpts, rolloutId?: string): SpawnSpec => { + beforeLaunch?.(opts); + const spec = kind === "resume" ? adapter.resume(rolloutId!, opts) : adapter.launch(opts); + const config = JSON.parse(readFileSync(opts.mcpConfigFile!, "utf8")) as { + mcpServers: { sapiom: CapturedLaunch["remote"] }; + }; + captures.push({ kind, remote: config.mcpServers.sapiom, spec }); + return { ...spec, command: "bash", args: [] }; + }; + return { + id: "codex", + eventSource: "transcript-tail", + systemPromptDelivery: "launch-flag", + doctor: async () => [], + launch: (opts) => interactiveSpec("create", opts), + resume: (rolloutId, opts) => interactiveSpec("resume", opts, rolloutId), + listPastSessions: async () => [], + canResume: async () => true, + }; } function capturingClaudeAdapter(captures: CapturedLaunch[]): HarnessAdapter { @@ -169,6 +200,7 @@ describe("Agent Studio MCP authentication wiring", () => { let projectRoot: string; let server: HarnessServer | undefined; let captures: CapturedLaunch[]; + let identityWorkspaces: string[]; beforeEach(async () => { root = await mkdtemp(join(tmpdir(), "harness-auth-mcp-wiring-")); @@ -193,6 +225,7 @@ describe("Agent Studio MCP authentication wiring", () => { apiKeyId: "browser-key-id", }; captures = []; + identityWorkspaces = []; vi.clearAllMocks(); }); @@ -201,6 +234,7 @@ describe("Agent Studio MCP authentication wiring", () => { await server?.close(); await server?.sessionManager.flush(); server = undefined; + for (const cwd of identityWorkspaces) await rm(cwd, { recursive: true, force: true }); await rm(root, { recursive: true, force: true, @@ -214,10 +248,11 @@ describe("Agent Studio MCP authentication wiring", () => { } }); + /** Start Studio with isolated state and the test's chosen adapters and identity. */ async function boot( options: Pick< Parameters[0], - "identity" | "authMode" + "identity" | "authMode" | "adapters" | "codexHomeDir" > = {}, ): Promise { server = await startServer({ @@ -467,6 +502,72 @@ describe("Agent Studio MCP authentication wiring", () => { expect(server!.sessionManager.get(newerSession.id)?.status).toBe("exited"); }, 20_000); + it("wires fresh Codex sessions after UI login, refreshes resume/create credentials, and clears auth after logout", async () => { + process.env.SAPIOM_ENVIRONMENT = "staging"; + await boot({ adapters: { codex: capturingCodexAdapter(captures) }, codexHomeDir: root }); + const create = async (cwd = projectRoot) => { + await mkdir(cwd, { recursive: true }); + const response = await post("/api/sessions", { cwd, harness: "codex" }); + expect(response.status).toBe(201); + return response.json() as Promise<{ id: string; harness: string }>; + }; + const expectWiring = (key?: string) => { + const { spec, remote } = captures.at(-1)!; + expect(remote.url).toBe("https://api.staging.example.test/v1/mcp"); + expect(spec!.args.join(" ")).toContain('"url" = "https://api.staging.example.test/v1/mcp"'); + expect(spec!.args.join(" ")).toMatch(/mcp_servers\.sapiom-dev-[a-f0-9]{12}=/); + expect(spec!.env.SAPIOM_ENVIRONMENT).toBeUndefined(); + expect(spec!.args.join(" ")).toContain('"env" = { "SAPIOM_ENVIRONMENT" = "staging"'); + expect(spec!.env.SAPIOM_CODEX_MCP_0_HEADER_0).toBe(key); + expect(spec!.args.join(" ")).not.toMatch(/browser-key|rotated-key/); + }; + + await create(); + expectWiring(); + expect((await post("/api/auth/start")).status).toBe(200); + await waitForAuthenticated(); + // Project bootstrap belongs to the principal that discovered it. Use a + // fresh workspace after identity changes, as a new Studio user would. + const signedInRoot = await mkdtemp(join(tmpdir(), "harness-auth-signed-in-")); + identityWorkspaces.push(signedInRoot); + const signedIn = await create(signedInRoot); + expect(signedIn.harness).toBe("codex"); + expectWiring("browser-key"); + const launchArgs = captures.at(-1)!.spec!.args; + await server!.sessionManager.setAgentSessionId(signedIn.id, "codex-rollout-fixture"); + + authFixture.credential = { ...authFixture.browserResult, apiKey: "rotated-key" }; + await server!.sessionManager.kill(signedIn.id); + expect((await post(`/api/sessions/${signedIn.id}/resume`)).status).toBe(200); + expect(captures.at(-1)!.kind).toBe("resume"); + expectWiring("rotated-key"); + expect(captures.at(-1)!.spec!.args.filter((arg) => arg.startsWith("mcp_servers."))) + .toEqual(launchArgs.filter((arg) => arg.startsWith("mcp_servers."))); + + await create(signedInRoot); + expectWiring("rotated-key"); + expect((await post("/api/auth/disconnect")).status).toBe(200); + const signedOutRoot = await mkdtemp(join(tmpdir(), "harness-auth-signed-out-")); + identityWorkspaces.push(signedOutRoot); + await create(signedOutRoot); + expectWiring(); + expect(clearCredentials).toHaveBeenCalled(); + }, 20_000); + + it("returns a credential-safe error to the UI if Codex's generated MCP file cannot be parsed", async () => { + const adapter = capturingCodexAdapter(captures, (opts) => { + writeFileSync(opts.mcpConfigFile!, 'private-api-key: "broken JSON"'); + }); + await boot({ adapters: { codex: adapter }, codexHomeDir: root }); + const response = await post("/api/sessions", { cwd: projectRoot, harness: "codex" }); + expect(response.status).toBe(500); + const body = await response.text(); + expect(body).toContain("Could not load the generated Codex MCP configuration"); + expect(body).not.toContain("private-api-key"); + expect(body).not.toContain(root); + expect(captures).toHaveLength(0); + }); + it("adopts a credential written externally after boot", async () => { await boot(); authFixture.credential = credential("external-key");