From ee5deae58fd10bd6f5f7da4b9b677e9b204bfb78 Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Fri, 4 Sep 2026 14:42:32 -0700 Subject: [PATCH 1/6] fix(harness): attach per-session MCP servers to Codex --- .changeset/codex-session-mcp.md | 5 + packages/harness/README.md | 24 ++ .../adapters/codex-mcp.integration.test.ts | 294 +++++++++++++++++ .../src/core/adapters/codex-mcp.test.ts | 298 ++++++++++++++++++ .../harness/src/core/adapters/codex-mcp.ts | 170 ++++++++++ .../harness/src/core/adapters/codex.test.ts | 10 +- packages/harness/src/core/adapters/codex.ts | 43 +-- .../harness/src/core/inject/mcp-config.ts | 5 +- .../src/server/auth-mcp-wiring.test.ts | 94 +++++- 9 files changed, 905 insertions(+), 38 deletions(-) create mode 100644 .changeset/codex-session-mcp.md create mode 100644 packages/harness/src/core/adapters/codex-mcp.integration.test.ts create mode 100644 packages/harness/src/core/adapters/codex-mcp.test.ts create mode 100644 packages/harness/src/core/adapters/codex-mcp.ts diff --git a/.changeset/codex-session-mcp.md b/.changeset/codex-session-mcp.md new file mode 100644 index 000000000..52098a5c8 --- /dev/null +++ b/.changeset/codex-session-mcp.md @@ -0,0 +1,5 @@ +--- +"@sapiom/harness": patch +--- + +Attach generated Sapiom MCP configuration to Codex sessions on launch and resume, using session-specific server names and environment-based credentials while preserving existing Codex settings. diff --git a/packages/harness/README.md b/packages/harness/README.md index 41b401f33..2582389cc 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -34,6 +34,16 @@ system prompt, in whatever project directory you choose. Uninstall: `rm -rf ~/.sapiom/harness` (all harness-owned state lives there). +Codex receives the generated remote Sapiom, local `sapiom-dev`, and optional +Agent Map MCP configuration on every session launch and resume. Studio uses session-specific +server names such as `sapiom-dev-` and identifies them in the +agent's instructions. This keeps existing Codex MCP registrations intact and +avoids inheriting old credentials or conflicting transports from a server with +the same name. Credentials are passed through the child environment, not command +arguments; Studio does not write to your Codex `config.toml`. If a generated MCP +file cannot be read or parsed, the session reports an error so you can start a +new session to regenerate it. Codex background tasks remain unsupported. + ## Telemetry With explicit opt-in, Agent Studio collects usage events (prompts, tool calls, @@ -86,6 +96,20 @@ 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, first build the workspace dependencies, then run: + +```bash +RUN_CODEX_MCP_INTEGRATION=1 pnpm --filter @sapiom/harness exec vitest run src/core/adapters/codex-mcp.integration.test.ts +``` + +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. + ### 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..e87e64cf8 --- /dev/null +++ b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts @@ -0,0 +1,294 @@ +/** + * 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. + * Run after building workspace dependencies with RUN_CODEX_MCP_INTEGRATION=1. + * CODEX_TEST_BINARY can select an installed CLI version. 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; +} + +async function discover(spec: SpawnSpec, home: string): Promise { + 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", + // 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, + { + resolve: (result: unknown) => void; + reject: (error: Error) => void; + } + >(); + let id = 0; + const fail = (): void => { + for (const request of pending.values()) { + request.reject( + new Error("Codex MCP probe failed; check CLI compatibility."), + ); + } + pending.clear(); + }; + // Drain diagnostics, but never include subprocess output/config in failures. + child.stderr.resume(); + child.on("error", fail); + child.on("exit", fail); + lines.on("line", (line) => { + let response: { id?: number; error?: unknown; 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) request.reject(new Error("Codex MCP RPC failed.")); + else request.resolve(response.result); + }); + const request = (method: string, params: unknown): Promise => { + const requestId = ++id; + return new Promise((resolve, reject) => { + pending.set(requestId, { resolve, reject }); + child.stdin.write( + JSON.stringify({ id: requestId, method, params }) + "\n", + ); + }); + }; + const timeout = setTimeout(() => { + fail(); + child.kill("SIGKILL"); + }, 25_000); + 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[]; + }; + return result.data; + } 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) => { + 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); + }); + 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", + ...(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); + const mcpConfigFile = await generateMcpConfig("integration-session", { + generatedRoot: join(root, "generated"), + apiKey: fixtureKey, + environment: "integration", + devServer: { + command: process.execPath, + args: [ + fileURLToPath( + new URL("../../../../mcp/dist/index.js", import.meta.url), + ), + ], + env: { + HOME: root, + SAPIOM_TELEMETRY_DISABLED: "1", + DO_NOT_TRACK: "1", + }, + }, + }); + 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 = await discover(spec, root); + const authoring = servers.find( + (server) => server.serverInfo?.name === "sapiom-dev", + ); + 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/core/adapters/codex-mcp.test.ts b/packages/harness/src/core/adapters/codex-mcp.test.ts new file mode 100644 index 000000000..0ab4bd651 --- /dev/null +++ b/packages/harness/src/core/adapters/codex-mcp.test.ts @@ -0,0 +1,298 @@ +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { CodexAdapter } from "./codex.js"; +import type { SpawnSpec } from "../../shared/types.js"; + +function serverArg(spec: SpawnSpec, name: string): string | undefined { + return spec.args.find((arg) => + new RegExp(`^mcp_servers\\.${name}-[a-f0-9]{12}=`).test(arg), + ); +} + +function serverConfig(spec: SpawnSpec, name: string): string | undefined { + const arg = serverArg(spec, name); + return arg?.slice(arg.indexOf("=") + 1); +} + +describe("Codex per-session MCP configuration", () => { + let dir: string; + let mcpConfigFile: string; + const adapter = new CodexAdapter({ binary: "fake-codex" }); + + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "harness-codex-mcp-")); + mcpConfigFile = join(dir, "mcp-config.json"); + }); + + afterEach(async () => { + await rm(dir, { recursive: true, force: true }); + }); + + const options = () => ({ + harnessSessionId: "session-1", + cwd: dir, + mcpConfigFile, + }); + const writeConfig = (mcpServers: unknown) => + writeFile(mcpConfigFile, JSON.stringify({ mcpServers })); + + it("attaches remote, local, and Agent Map MCP servers on launch and resume without putting credentials in argv", async () => { + await writeConfig({ + sapiom: { + type: "http", + url: "https://api.sapiom.ai/v1/mcp", + headers: { "x-api-key": "private-sapiom-api-key" }, + }, + "sapiom-dev": { + command: "/Applications/Agent Studio.app/Contents/MacOS/Agent Studio", + args: ["/Applications/Agent Studio.app/Contents/Resources/mcp.js"], + env: { + ELECTRON_RUN_AS_NODE: "1", + SAPIOM_ENVIRONMENT: "staging", + SAPIOM_HARNESS_VERSION: "0.14.0", + SAPIOM_API_KEY: "private-stdio-api-key", + }, + }, + "agent-map": { + type: "http", + url: "http://127.0.0.1:4312/mcp/agent-map", + headers: { Authorization: "Bearer private-map-token" }, + }, + }); + const before = await readFile(mcpConfigFile, "utf8"); + const parentApiKey = process.env.SAPIOM_API_KEY; + + for (const spec of [ + adapter.launch(options()), + adapter.resume("rollout-1", options()), + ]) { + const remote = serverArg(spec, "sapiom"); + expect(remote).toContain('"url" = "https://api.sapiom.ai/v1/mcp"'); + expect(remote).toContain( + '"env_http_headers" = { "x-api-key" = "SAPIOM_CODEX_MCP_0_HEADER_0" }', + ); + expect(spec.env.SAPIOM_CODEX_MCP_0_HEADER_0).toBe( + "private-sapiom-api-key", + ); + + const local = serverArg(spec, "sapiom-dev"); + expect(local).toContain( + '"command" = "/Applications/Agent Studio.app/Contents/MacOS/Agent Studio"', + ); + expect(local).toContain( + '"args" = ["/Applications/Agent Studio.app/Contents/Resources/mcp.js"]', + ); + expect(local).toContain( + '"env_vars" = ["ELECTRON_RUN_AS_NODE", "SAPIOM_ENVIRONMENT", "SAPIOM_HARNESS_VERSION", "SAPIOM_API_KEY"]', + ); + expect(spec.env).toMatchObject({ + ELECTRON_RUN_AS_NODE: "1", + SAPIOM_ENVIRONMENT: "staging", + SAPIOM_HARNESS_VERSION: "0.14.0", + SAPIOM_API_KEY: "private-stdio-api-key", + }); + + const agentMap = serverArg(spec, "agent-map"); + expect(agentMap).toContain( + '"env_http_headers" = { "Authorization" = "SAPIOM_CODEX_MCP_2_HEADER_0" }', + ); + expect(spec.env.SAPIOM_CODEX_MCP_2_HEADER_0).toBe( + "Bearer private-map-token", + ); + expect(spec.args.join(" ")).not.toContain("private-"); + expect(spec.args.some((arg) => arg.startsWith("mcp_servers="))).toBe( + false, + ); + } + expect(await readFile(mcpConfigFile, "utf8")).toBe(before); + expect(process.env.SAPIOM_API_KEY).toBe(parentApiKey); + }); + + it("supports signed-out sessions and the default npx launcher", async () => { + await writeConfig({ + sapiom: { type: "http", url: "https://api.sapiom.ai/v1/mcp" }, + "sapiom-dev": { command: "npx", args: ["-y", "@sapiom/mcp@latest"] }, + }); + const spec = adapter.launch(options()); + expect(serverConfig(spec, "sapiom")).toBe( + '{ "url" = "https://api.sapiom.ai/v1/mcp" }', + ); + expect(serverConfig(spec, "sapiom-dev")).toBe( + '{ "command" = "npx", "args" = ["-y", "@sapiom/mcp@latest"] }', + ); + expect(spec.env).toEqual({}); + }); + + it("keeps the profile prompt and identifies stable MCP aliases unique to each Studio session", async () => { + await writeConfig({ "sapiom-dev": { command: "node", args: ["mcp.js"] } }); + const systemPromptFile = join(dir, "system-prompt.txt"); + await writeFile( + systemPromptFile, + "Build useful agents.\nKeep the user's constraints.", + ); + const opts = { ...options(), systemPromptFile }; + const launched = adapter.launch(opts); + const resumed = adapter.resume("rollout", opts); + const other = adapter.launch({ ...opts, harnessSessionId: "session-2" }); + const alias = serverArg(launched, "sapiom-dev")! + .split("=")[0] + .slice("mcp_servers.".length); + expect(serverArg(resumed, "sapiom-dev")).toBe( + serverArg(launched, "sapiom-dev"), + ); + expect(serverArg(other, "sapiom-dev")).not.toBe( + serverArg(launched, "sapiom-dev"), + ); + for (const spec of [launched, resumed]) { + const overrides = spec.args.filter((arg) => + arg.startsWith("developer_instructions="), + ); + expect(overrides).toHaveLength(1); + const prompt: string = JSON.parse( + overrides[0].slice("developer_instructions=".length), + ); + expect(prompt).toContain( + "Build useful agents.\nKeep the user's constraints.", + ); + expect(prompt).toContain(alias); + } + }); + + it("reads regenerated credentials on resume instead of reusing launch credentials", async () => { + const servers = (key: string) => ({ + sapiom: { + type: "http", + url: "https://api.sapiom.ai/v1/mcp", + headers: { "x-api-key": key }, + }, + }); + await writeConfig(servers("old-session-key")); + const launched = adapter.launch(options()); + await writeConfig(servers("new-session-key")); + const resumed = adapter.resume("rollout-1", options()); + expect(launched.env.SAPIOM_CODEX_MCP_0_HEADER_0).toBe("old-session-key"); + expect(resumed.env.SAPIOM_CODEX_MCP_0_HEADER_0).toBe("new-session-key"); + expect(resumed.args.slice(0, 2)).toEqual(["resume", "rollout-1"]); + expect(serverArg(resumed, "sapiom")).toBe(serverArg(launched, "sapiom")); + }); + + it("uses a newly issued Agent Map capability over the copy in the generated file", async () => { + await writeConfig({ + "agent-map": { + type: "http", + url: "http://127.0.0.1:1/mcp", + headers: { Authorization: "Bearer old-token" }, + }, + }); + const spec = adapter.launch({ + ...options(), + agentMapMcp: { + url: "http://127.0.0.1:2/mcp", + bearerToken: "fresh-token", + }, + }); + expect( + spec.args.filter((arg) => arg.startsWith("mcp_servers.agent-map-")), + ).toHaveLength(1); + expect(spec.args.join(" ")).toContain('"url" = "http://127.0.0.1:2/mcp"'); + expect(spec.args.join(" ")).not.toMatch(/old-token|fresh-token/); + expect(spec.env).toEqual({ SAPIOM_AGENT_MAP_CAPABILITY: "fresh-token" }); + }); + + it("escapes Windows paths, quotes, newlines, and TOML control characters", async () => { + await writeConfig({ + "sapiom-dev": { + command: 'C:\\Program Files\\Agent "Studio"\\node.exe', + args: ["line\nnext\u007f", "C:\\mcp\\index.js"], + }, + }); + const spec = adapter.launch(options()); + expect(serverConfig(spec, "sapiom-dev")).toBe( + '{ "command" = "C:\\\\Program Files\\\\Agent \\"Studio\\"\\\\node.exe", "args" = ["line\\nnext\\u007f", "C:\\\\mcp\\\\index.js"] }', + ); + }); + + it.each([ + ["missing server map", {}], + ["array server map", { mcpServers: [] }], + [ + "dotted server name", + { mcpServers: { "sapiom.injected": { command: "node" } } }, + ], + [ + "wrong header type", + { + mcpServers: { + sapiom: { + type: "http", + url: "https://api.sapiom.ai", + headers: { "x-api-key": 42 }, + }, + }, + }, + ], + [ + "unknown transport", + { mcpServers: { sapiom: { type: "sse", url: "https://api.sapiom.ai" } } }, + ], + [ + "mixed transports", + { + mcpServers: { + sapiom: { + type: "http", + url: "https://api.sapiom.ai", + command: "node", + }, + }, + }, + ], + [ + "non-string argument", + { mcpServers: { "sapiom-dev": { command: "node", args: [42] } } }, + ], + [ + "invalid env name", + { + mcpServers: { + "sapiom-dev": { command: "node", env: { "bad=name": "private-key" } }, + }, + }, + ], + ])( + "rejects %s visibly instead of silently dropping MCP wiring", + async (_label, config) => { + await writeFile(mcpConfigFile, JSON.stringify(config)); + for (const launch of [ + () => adapter.launch(options()), + () => adapter.resume("rollout", options()), + ]) { + expect(launch).toThrow( + "Could not load the generated Codex MCP configuration. Start a new session to regenerate it.", + ); + } + }, + ); + + it("does not expose JSON parser snippets or file paths when a credential-bearing file is malformed", async () => { + await writeFile(mcpConfigFile, 'private-api-key: "broken JSON"'); + expect(() => adapter.launch(options())).toThrow( + /^Could not load the generated Codex MCP configuration\./, + ); + try { + adapter.launch(options()); + } catch (error) { + expect(String(error)).not.toContain("private-api-key"); + expect(String(error)).not.toContain(mcpConfigFile); + } + }); + + it("fails visibly when the generated MCP file is unavailable", () => { + expect(() => adapter.launch(options())).toThrow( + "Could not load the generated Codex MCP configuration", + ); + }); +}); diff --git a/packages/harness/src/core/adapters/codex-mcp.ts b/packages/harness/src/core/adapters/codex-mcp.ts new file mode 100644 index 000000000..b9f91c4e9 --- /dev/null +++ b/packages/harness/src/core/adapters/codex-mcp.ts @@ -0,0 +1,170 @@ +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; + +import type { LaunchOpts, SpawnSpec } from "../../shared/types.js"; + +type TomlValue = string | string[] | { [key: string]: TomlValue }; + +/** JSON string escaping also works for TOML basic strings, except that TOML + * requires DEL to be escaped as well. Inline-table keys are always quoted. */ +function toml(value: TomlValue): string { + if (typeof value === "string") + return JSON.stringify(value).replace(/\u007f/g, "\\u007f"); + if (Array.isArray(value)) return `[${value.map(toml).join(", ")}]`; + return `{ ${Object.entries(value) + .map(([key, item]) => `${toml(key)} = ${toml(item)}`) + .join(", ")} }`; +} + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +function stringRecord(value: unknown): Record { + if ( + !isRecord(value) || + Object.values(value).some((item) => typeof item !== "string") + ) { + throw new Error("Invalid string map"); + } + return value as Record; +} + +/** + * Adapt Studio's generated Claude-shaped MCP file to Codex's per-process + * config overrides. Codex deep-merges even whole-table overrides, so stable + * session aliases prevent existing same-name transport/auth fields from + * bleeding into Studio's servers. All user servers remain unchanged and no + * config.toml is written. The prompt identifies Studio's aliases explicitly. + * + * Header/stdio environment values stay in the child environment, never argv. + * `env_http_headers` and stdio `env_vars` are Codex config keys; neither needs + * persistent `codex mcp add` registration. + */ +export function buildCodexMcpConfig( + opts: Pick, +): Pick & { instructions?: string } { + const args: string[] = []; + const aliases: string[] = []; + const suffix = createHash("sha256") + .update(opts.harnessSessionId) + .digest("hex") + .slice(0, 12); + const env: Record = {}; + const bindEnv = (name: string, value: string): void => { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name) || name === "__proto__") { + throw new Error("Invalid environment name"); + } + if ( + Object.prototype.hasOwnProperty.call(env, name) && + env[name] !== value + ) { + throw new Error("Conflicting environment values"); + } + env[name] = value; + }; + const addServer = (name: string, config: Record): void => { + // Codex -c paths split on dots; restricting generated server names also + // prevents a name from addressing another part of the user's config. + if (!/^[A-Za-z0-9_-]+$/.test(name)) throw new Error("Invalid server name"); + const alias = `${name}-${suffix}`; + aliases.push(alias); + args.push("-c", `mcp_servers.${alias}=${toml(config)}`); + }; + + try { + if (opts.mcpConfigFile) { + const parsed: unknown = JSON.parse( + readFileSync(opts.mcpConfigFile, "utf8"), + ); + if (!isRecord(parsed) || !isRecord(parsed.mcpServers)) { + throw new Error("Invalid MCP configuration"); + } + for (const [index, [name, server]] of Object.entries( + parsed.mcpServers, + ).entries()) { + // A freshly issued capability supplied separately wins over a copy + // in the generated file. Keep compatibility with callers using only + // agentMapMcp as well. + if (name === "agent-map" && opts.agentMapMcp) continue; + if (!isRecord(server)) throw new Error("Invalid server"); + if (server.type === "http") { + if ( + typeof server.url !== "string" || + !/^https?:\/\//.test(server.url) + ) { + throw new Error("Invalid HTTP URL"); + } + if ( + Object.keys(server).some( + (key) => !["type", "url", "headers"].includes(key), + ) + ) { + throw new Error("Unsupported HTTP configuration"); + } + const config: Record = { url: server.url }; + if (server.headers !== undefined) { + const headers = stringRecord(server.headers); + config.env_http_headers = Object.fromEntries( + Object.entries(headers).map(([header, value], headerIndex) => { + const variable = `SAPIOM_CODEX_MCP_${index}_HEADER_${headerIndex}`; + bindEnv(variable, value); + return [header, variable]; + }), + ); + } + addServer(name, config); + } else { + if ( + (server.type !== undefined && server.type !== "stdio") || + typeof server.command !== "string" || + !server.command || + (server.args !== undefined && + (!Array.isArray(server.args) || + server.args.some((arg) => typeof arg !== "string"))) || + Object.keys(server).some( + (key) => !["type", "command", "args", "env"].includes(key), + ) + ) { + throw new Error("Invalid stdio configuration"); + } + const config: Record = { + command: server.command, + ...(server.args !== undefined + ? { args: server.args as string[] } + : {}), + }; + if (server.env !== undefined) { + const values = stringRecord(server.env); + for (const [variable, value] of Object.entries(values)) + bindEnv(variable, value); + config.env_vars = Object.keys(values); + } + addServer(name, config); + } + } + } + if (opts.agentMapMcp) { + bindEnv("SAPIOM_AGENT_MAP_CAPABILITY", opts.agentMapMcp.bearerToken); + addServer("agent-map", { + url: opts.agentMapMcp.url, + bearer_token_env_var: "SAPIOM_AGENT_MAP_CAPABILITY", + }); + } + } catch { + // JSON parser errors can include fragments of the file, which carries + // credentials. Fail the launch visibly without exposing raw contents. + throw new Error( + "Could not load the generated Codex MCP configuration. Start a new session to regenerate it.", + ); + } + return { + args, + env, + ...(aliases.length > 0 + ? { + instructions: `Studio's per-session Sapiom MCP servers are: ${aliases.join(", ")}. Use these session-specific servers for Sapiom tools; they carry this session's configuration and credentials.`, + } + : {}), + }; +} diff --git a/packages/harness/src/core/adapters/codex.test.ts b/packages/harness/src/core/adapters/codex.test.ts index c452b6ab2..77df51ca8 100644 --- a/packages/harness/src/core/adapters/codex.test.ts +++ b/packages/harness/src/core/adapters/codex.test.ts @@ -127,12 +127,11 @@ describe("CodexAdapter", () => { expect(spec.env).toEqual({}); }); - it("ignores mcpConfigFile/settingsFile — Codex has no per-session injection point for either", () => { + it("ignores the Claude-only settingsFile", () => { const adapter = new CodexAdapter({ binary: "fake-codex" }); const spec = adapter.launch({ harnessSessionId: "h1", cwd: "/tmp/proj", - mcpConfigFile: "/tmp/proj/.sapiom/mcp.json", settingsFile: "/tmp/proj/.sapiom/settings.json", }); expect(spec.args).toEqual([ @@ -155,11 +154,8 @@ describe("CodexAdapter", () => { adapter.launch({ harnessSessionId: "h1", cwd: "/tmp/proj", agentMapMcp }), adapter.resume("rollout", { harnessSessionId: "h1", cwd: "/tmp/proj", agentMapMcp }), ]) { - expect(spec.args).toContain( - `mcp_servers.agent-map.url=${JSON.stringify(agentMapMcp.url)}`, - ); - expect(spec.args).toContain( - 'mcp_servers.agent-map.bearer_token_env_var="SAPIOM_AGENT_MAP_CAPABILITY"', + expect(spec.args.find((arg) => arg.startsWith("mcp_servers.agent-map-"))).toContain( + `{ "url" = ${JSON.stringify(agentMapMcp.url)}, "bearer_token_env_var" = "SAPIOM_AGENT_MAP_CAPABILITY" }`, ); expect(spec.args.join(" ")).not.toContain(agentMapMcp.bearerToken); expect(spec.env).toEqual({ diff --git a/packages/harness/src/core/adapters/codex.ts b/packages/harness/src/core/adapters/codex.ts index bab63fe7e..2e72e4446 100644 --- a/packages/harness/src/core/adapters/codex.ts +++ b/packages/harness/src/core/adapters/codex.ts @@ -35,6 +35,7 @@ import type { SpawnSpec, } from "../../shared/types.js"; import { stripAnsi } from "../strip-ansi.js"; +import { buildCodexMcpConfig } from "./codex-mcp.js"; const execFileAsync = promisify(execFile); @@ -321,27 +322,23 @@ export class CodexAdapter implements HarnessAdapter { } launch(opts: LaunchOpts): SpawnSpec { - const args = buildConfigArgs(opts); + const mcp = buildCodexMcpConfig(opts); + const args = [...buildConfigArgs(opts, mcp.instructions), ...mcp.args]; if (opts.initialPrompt) args.push("--", opts.initialPrompt); return { command: this.binary, args: [...this.binaryArgs, ...args], - // Codex has no analog to Claude's CLAUDECODE nested-agent guard; no env - // overrides are needed for a fresh launch. - env: { ...this.binaryEnv, ...(opts.agentMapMcp - ? { SAPIOM_AGENT_MAP_CAPABILITY: opts.agentMapMcp.bearerToken } - : {}) }, + env: { ...this.binaryEnv, ...mcp.env }, cwd: opts.cwd, }; } resume(agentSessionId: string, opts: LaunchOpts): SpawnSpec { + const mcp = buildCodexMcpConfig(opts); return { command: this.binary, - args: [...this.binaryArgs, "resume", agentSessionId, ...buildConfigArgs(opts)], - env: { ...this.binaryEnv, ...(opts.agentMapMcp - ? { SAPIOM_AGENT_MAP_CAPABILITY: opts.agentMapMcp.bearerToken } - : {}) }, + args: [...this.binaryArgs, "resume", agentSessionId, ...buildConfigArgs(opts, mcp.instructions), ...mcp.args], + env: { ...this.binaryEnv, ...mcp.env }, cwd: opts.cwd, }; } @@ -443,12 +440,9 @@ export class CodexAdapter implements HarnessAdapter { } /** - * Codex has no single-flag equivalent to Claude's `--append-system-prompt` / - * `--mcp-config` — MCP servers are registered globally via `codex mcp add` - * (a persistent config.toml mutation, which the harness's "zero config - * mutation" design deliberately avoids), so `opts.mcpConfigFile` / - * `opts.settingsFile` are intentionally unused here. System-prompt injection - * uses the generic `-c key=value` override mechanism instead. + * System-prompt injection uses Codex's generic `-c key=value` overrides. + * The MCP overrides are built separately by buildCodexMcpConfig; the + * Claude-only hook settings in opts.settingsFile remain unused. * * Confirmed against a locally installed codex-cli 0.134.0: `-c * model_instructions_file=` is a real, recognized key — but if that @@ -469,7 +463,7 @@ export class CodexAdapter implements HarnessAdapter { * one rather than passing a broken reference that's guaranteed to kill the * process on startup. */ -function buildConfigArgs(opts: LaunchOpts): string[] { +function buildConfigArgs(opts: LaunchOpts, mcpInstructions?: string): string[] { const args = [ "-c", "check_for_update_on_startup=false", @@ -489,24 +483,21 @@ function buildConfigArgs(opts: LaunchOpts): string[] { "-c", 'sandbox_mode="workspace-write"', ]; - if (opts.agentMapMcp) { - args.push( - "-c", - `mcp_servers.agent-map.url=${JSON.stringify(opts.agentMapMcp.url)}`, - "-c", - 'mcp_servers.agent-map.bearer_token_env_var="SAPIOM_AGENT_MAP_CAPABILITY"', - ); - } + const instructions: string[] = []; if (opts.systemPromptFile) { try { const prompt = readFileSync(opts.systemPromptFile, "utf8"); - args.push("-c", `developer_instructions=${JSON.stringify(prompt)}`); + instructions.push(prompt); } catch (err) { console.error( `[codex adapter] could not read systemPromptFile "${opts.systemPromptFile}" — launching without an injected system prompt: ${(err as Error).message}`, ); } } + if (mcpInstructions) instructions.push(mcpInstructions); + if (instructions.length > 0) { + args.push("-c", `developer_instructions=${JSON.stringify(instructions.join("\n\n"))}`); + } return args; } diff --git a/packages/harness/src/core/inject/mcp-config.ts b/packages/harness/src/core/inject/mcp-config.ts index 9110b8e0c..504c2c055 100644 --- a/packages/harness/src/core/inject/mcp-config.ts +++ b/packages/harness/src/core/inject/mcp-config.ts @@ -56,11 +56,12 @@ export interface McpConfigOptions { } /** - * Writes the `--mcp-config` file for a harness session: the remote HTTP + * Writes the MCP config file for a harness session: the remote HTTP * capability MCP (`sapiom`) and the local stdio authoring MCP (`sapiom-dev`). * Written under `HARNESS_PATHS.generated//` so concurrent * sessions never share (or race on) a config file. Returns the file's - * absolute path for the adapter to pass as `--mcp-config `. + * absolute path. Claude passes it as `--mcp-config `; Codex translates + * it to per-process config overrides and environment variables. */ export async function generateMcpConfig( harnessSessionId: string, diff --git a/packages/harness/src/server/auth-mcp-wiring.test.ts b/packages/harness/src/server/auth-mcp-wiring.test.ts index 0b7a7d631..2f272967b 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 { join } from "node:path"; @@ -88,6 +88,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"; @@ -98,6 +99,35 @@ 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(); + 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 { @@ -198,7 +228,7 @@ describe("Agent Studio MCP authentication wiring", () => { async function boot( options: Pick< Parameters[0], - "identity" | "authMode" + "identity" | "authMode" | "adapters" | "codexHomeDir" > = {}, ): Promise { server = await startServer({ @@ -243,6 +273,64 @@ describe("Agent Studio MCP authentication wiring", () => { expect(injectedKey(captures[0])).toBe("browser-key"); }); + 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 () => { + const response = await post("/api/sessions", { cwd: projectRoot, 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).toBe("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 vi.waitFor(() => expect(writeCredentials).toHaveBeenCalledOnce()); + const signedIn = await create(); + 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 = credential("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(); + expectWiring("rotated-key"); + expect((await post("/api/auth/disconnect")).status).toBe(200); + await create(); + 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"); From 8ae33df53064644a9c777068451808b418e8790f Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Fri, 4 Sep 2026 17:47:43 -0700 Subject: [PATCH 2/6] fix(harness): isolate Codex MCP settings from shell commands --- .changeset/codex-session-mcp.md | 2 +- packages/harness/README.md | 7 +- .../adapters/codex-mcp.integration.test.ts | 48 ++++++++++-- .../src/core/adapters/codex-mcp.test.ts | 30 +++++--- .../harness/src/core/adapters/codex-mcp.ts | 76 ++++++++++++++----- .../src/server/auth-mcp-wiring.test.ts | 3 +- 6 files changed, 125 insertions(+), 41 deletions(-) diff --git a/.changeset/codex-session-mcp.md b/.changeset/codex-session-mcp.md index 52098a5c8..a94065cff 100644 --- a/.changeset/codex-session-mcp.md +++ b/.changeset/codex-session-mcp.md @@ -2,4 +2,4 @@ "@sapiom/harness": patch --- -Attach generated Sapiom MCP configuration to Codex sessions on launch and resume, using session-specific server names and environment-based credentials while preserving existing Codex settings. +Attach generated Sapiom MCP configuration to Codex sessions on launch and resume, using session-specific server names and environment-based credentials while preserving existing Codex settings. Invalid or unreadable generated configuration now reports a launch error instead of silently starting without MCP servers. diff --git a/packages/harness/README.md b/packages/harness/README.md index 2582389cc..e2424231a 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -39,8 +39,9 @@ Agent Map MCP configuration on every session launch and resume. Studio uses sess server names such as `sapiom-dev-` and identifies them in the agent's instructions. This keeps existing Codex MCP registrations intact and avoids inheriting old credentials or conflicting transports from a server with -the same name. Credentials are passed through the child environment, not command -arguments; Studio does not write to your Codex `config.toml`. If a generated MCP +the same name. Credentials are passed through Codex's environment and cleared from +shell-tool environments; they never appear in command arguments. Authoring-process +settings stay on the MCP server. Studio does not write to your Codex `config.toml`. If a generated MCP file cannot be read or parsed, the session reports an error so you can start a new session to regenerate it. Codex background tasks remain unsupported. @@ -109,6 +110,8 @@ RUN_CODEX_MCP_INTEGRATION=1 pnpm --filter @sapiom/harness exec vitest run src/co 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 diff --git a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts index e87e64cf8..603d7bb34 100644 --- a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts +++ b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts @@ -25,7 +25,13 @@ interface McpStatus { tools: Record; } -async function discover(spec: SpawnSpec, home: string): Promise { +async function discover( + spec: SpawnSpec, + home: string, +): Promise<{ + servers: McpStatus[]; + commandEnv: Record; +}> { const env: NodeJS.ProcessEnv = { PATH: process.env.PATH, HOME: home, @@ -33,6 +39,7 @@ async function discover(spec: SpawnSpec, home: string): Promise { 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", @@ -108,7 +115,24 @@ async function discover(spec: SpawnSpec, home: string): Promise { })) as { data: McpStatus[]; }; - return result.data; + 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 failed."); + return { servers: result.data, commandEnv: JSON.parse(command.stdout) }; } finally { clearTimeout(timeout); lines.close(); @@ -214,6 +238,10 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( '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 @@ -242,9 +270,8 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( ), ], env: { - HOME: root, - SAPIOM_TELEMETRY_DISABLED: "1", - DO_NOT_TRACK: "1", + ELECTRON_RUN_AS_NODE: "1", + SAPIOM_REVIEW_STDIO_SECRET: "synthetic-stdio-secret", }, }, }); @@ -257,7 +284,12 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( const spec = adapter.launch(opts); authenticated = false; expect(spec.args.join(" ").includes(fixtureKey)).toBe(false); - const servers = await discover(spec, root); + const { servers, commandEnv } = await discover(spec, root); + 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", ); @@ -278,8 +310,8 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( ); expect( Object.keys( - servers.find((server) => server.name === "user-server") - ?.tools ?? {}, + servers.find((server) => server.name === "user-server")?.tools ?? + {}, ), ).toContain("local_capability_probe"); } diff --git a/packages/harness/src/core/adapters/codex-mcp.test.ts b/packages/harness/src/core/adapters/codex-mcp.test.ts index 0ab4bd651..23f08cbb8 100644 --- a/packages/harness/src/core/adapters/codex-mcp.test.ts +++ b/packages/harness/src/core/adapters/codex-mcp.test.ts @@ -1,7 +1,7 @@ import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { CodexAdapter } from "./codex.js"; import type { SpawnSpec } from "../../shared/types.js"; @@ -85,15 +85,19 @@ describe("Codex per-session MCP configuration", () => { expect(local).toContain( '"args" = ["/Applications/Agent Studio.app/Contents/Resources/mcp.js"]', ); + expect(local).toContain('"env_vars" = ["SAPIOM_API_KEY"]'); expect(local).toContain( - '"env_vars" = ["ELECTRON_RUN_AS_NODE", "SAPIOM_ENVIRONMENT", "SAPIOM_HARNESS_VERSION", "SAPIOM_API_KEY"]', + '"env" = { "ELECTRON_RUN_AS_NODE" = "1", "SAPIOM_ENVIRONMENT" = "staging", "SAPIOM_HARNESS_VERSION" = "0.14.0" }', ); - expect(spec.env).toMatchObject({ - ELECTRON_RUN_AS_NODE: "1", - SAPIOM_ENVIRONMENT: "staging", - SAPIOM_HARNESS_VERSION: "0.14.0", - SAPIOM_API_KEY: "private-stdio-api-key", - }); + expect(spec.env.ELECTRON_RUN_AS_NODE).toBeUndefined(); + expect(spec.env.SAPIOM_ENVIRONMENT).toBeUndefined(); + expect(spec.env.SAPIOM_HARNESS_VERSION).toBeUndefined(); + expect(spec.env.SAPIOM_API_KEY).toBe("private-stdio-api-key"); + for (const variable of Object.keys(spec.env)) { + expect(spec.args).toContain( + `shell_environment_policy.set.${variable}=""`, + ); + } const agentMap = serverArg(spec, "agent-map"); expect(agentMap).toContain( @@ -157,7 +161,8 @@ describe("Codex per-session MCP configuration", () => { expect(prompt).toContain( "Build useful agents.\nKeep the user's constraints.", ); - expect(prompt).toContain(alias); + expect(prompt).toContain(`sapiom-dev is registered as ${alias}`); + expect(prompt).toContain("References to the original server names"); } }); @@ -278,6 +283,7 @@ describe("Codex per-session MCP configuration", () => { ); it("does not expose JSON parser snippets or file paths when a credential-bearing file is malformed", async () => { + const diagnostic = vi.spyOn(console, "error").mockImplementation(() => {}); await writeFile(mcpConfigFile, 'private-api-key: "broken JSON"'); expect(() => adapter.launch(options())).toThrow( /^Could not load the generated Codex MCP configuration\./, @@ -288,6 +294,12 @@ describe("Codex per-session MCP configuration", () => { expect(String(error)).not.toContain("private-api-key"); expect(String(error)).not.toContain(mcpConfigFile); } + expect(diagnostic.mock.calls.flat().join(" ")).toContain("Invalid JSON"); + expect(diagnostic.mock.calls.flat().join(" ")).not.toContain( + "private-api-key", + ); + expect(diagnostic.mock.calls.flat().join(" ")).not.toContain(mcpConfigFile); + diagnostic.mockRestore(); }); it("fails visibly when the generated MCP file is unavailable", () => { diff --git a/packages/harness/src/core/adapters/codex-mcp.ts b/packages/harness/src/core/adapters/codex-mcp.ts index b9f91c4e9..73c072b24 100644 --- a/packages/harness/src/core/adapters/codex-mcp.ts +++ b/packages/harness/src/core/adapters/codex-mcp.ts @@ -5,6 +5,16 @@ import type { LaunchOpts, SpawnSpec } from "../../shared/types.js"; type TomlValue = string | string[] | { [key: string]: TomlValue }; +class InvalidMcpConfigError extends Error {} + +// Studio emits these non-secret values to configure the authoring process. +// They belong on that server, especially Electron's process-mode flag. +const STDIO_SETTINGS = new Set([ + "ELECTRON_RUN_AS_NODE", + "SAPIOM_ENVIRONMENT", + "SAPIOM_HARNESS_VERSION", +]); + /** JSON string escaping also works for TOML basic strings, except that TOML * requires DEL to be escaped as well. Inline-table keys are always quoted. */ function toml(value: TomlValue): string { @@ -25,7 +35,7 @@ function stringRecord(value: unknown): Record { !isRecord(value) || Object.values(value).some((item) => typeof item !== "string") ) { - throw new Error("Invalid string map"); + throw new InvalidMcpConfigError("Invalid string map"); } return value as Record; } @@ -37,9 +47,10 @@ function stringRecord(value: unknown): Record { * bleeding into Studio's servers. All user servers remain unchanged and no * config.toml is written. The prompt identifies Studio's aliases explicitly. * - * Header/stdio environment values stay in the child environment, never argv. - * `env_http_headers` and stdio `env_vars` are Codex config keys; neither needs - * persistent `codex mcp add` registration. + * Known stdio settings stay on the MCP server. Credentials and other stdio + * values reach Codex through its environment, never argv, and are cleared + * from shell-tool environments. Per-variable overrides preserve the user's + * other shell settings. No persistent `codex mcp add` registration is needed. */ export function buildCodexMcpConfig( opts: Pick, @@ -53,22 +64,23 @@ export function buildCodexMcpConfig( const env: Record = {}; const bindEnv = (name: string, value: string): void => { if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(name) || name === "__proto__") { - throw new Error("Invalid environment name"); + throw new InvalidMcpConfigError("Invalid environment name"); } if ( Object.prototype.hasOwnProperty.call(env, name) && env[name] !== value ) { - throw new Error("Conflicting environment values"); + throw new InvalidMcpConfigError("Conflicting environment values"); } env[name] = value; }; const addServer = (name: string, config: Record): void => { // Codex -c paths split on dots; restricting generated server names also // prevents a name from addressing another part of the user's config. - if (!/^[A-Za-z0-9_-]+$/.test(name)) throw new Error("Invalid server name"); + if (!/^[A-Za-z0-9_-]+$/.test(name)) + throw new InvalidMcpConfigError("Invalid server name"); const alias = `${name}-${suffix}`; - aliases.push(alias); + aliases.push(`${name} is registered as ${alias}`); args.push("-c", `mcp_servers.${alias}=${toml(config)}`); }; @@ -78,7 +90,7 @@ export function buildCodexMcpConfig( readFileSync(opts.mcpConfigFile, "utf8"), ); if (!isRecord(parsed) || !isRecord(parsed.mcpServers)) { - throw new Error("Invalid MCP configuration"); + throw new InvalidMcpConfigError("Invalid MCP configuration"); } for (const [index, [name, server]] of Object.entries( parsed.mcpServers, @@ -87,20 +99,21 @@ export function buildCodexMcpConfig( // in the generated file. Keep compatibility with callers using only // agentMapMcp as well. if (name === "agent-map" && opts.agentMapMcp) continue; - if (!isRecord(server)) throw new Error("Invalid server"); + if (!isRecord(server)) + throw new InvalidMcpConfigError("Invalid server"); if (server.type === "http") { if ( typeof server.url !== "string" || !/^https?:\/\//.test(server.url) ) { - throw new Error("Invalid HTTP URL"); + throw new InvalidMcpConfigError("Invalid HTTP URL"); } if ( Object.keys(server).some( (key) => !["type", "url", "headers"].includes(key), ) ) { - throw new Error("Unsupported HTTP configuration"); + throw new InvalidMcpConfigError("Unsupported HTTP configuration"); } const config: Record = { url: server.url }; if (server.headers !== undefined) { @@ -126,7 +139,7 @@ export function buildCodexMcpConfig( (key) => !["type", "command", "args", "env"].includes(key), ) ) { - throw new Error("Invalid stdio configuration"); + throw new InvalidMcpConfigError("Invalid stdio configuration"); } const config: Record = { command: server.command, @@ -136,9 +149,17 @@ export function buildCodexMcpConfig( }; if (server.env !== undefined) { const values = stringRecord(server.env); - for (const [variable, value] of Object.entries(values)) - bindEnv(variable, value); - config.env_vars = Object.keys(values); + const local: Record = {}; + const forwarded: string[] = []; + for (const [variable, value] of Object.entries(values)) { + if (STDIO_SETTINGS.has(variable)) local[variable] = value; + else { + bindEnv(variable, value); + forwarded.push(variable); + } + } + if (Object.keys(local).length > 0) config.env = local; + if (forwarded.length > 0) config.env_vars = forwarded; } addServer(name, config); } @@ -151,19 +172,34 @@ export function buildCodexMcpConfig( bearer_token_env_var: "SAPIOM_AGENT_MAP_CAPABILITY", }); } - } catch { - // JSON parser errors can include fragments of the file, which carries - // credentials. Fail the launch visibly without exposing raw contents. + } catch (error) { + // Parser and filesystem messages can contain credentials or private paths. + const reason = + error instanceof InvalidMcpConfigError + ? error.message + : error instanceof SyntaxError + ? "Invalid JSON" + : isRecord(error) && + ["ENOENT", "EACCES", "EPERM"].includes(String(error.code)) + ? String(error.code) + : "Read failure"; + console.error(`[codex adapter] generated MCP configuration: ${reason}`); throw new Error( "Could not load the generated Codex MCP configuration. Start a new session to regenerate it.", ); } + // Codex's MCP clients read its process env; shell tools use a separate + // policy. Blank only our forwarded values there, without replacing any + // existing exclusions or unrelated user-provided environment settings. + for (const variable of Object.keys(env)) { + args.push("-c", `shell_environment_policy.set.${variable}=""`); + } return { args, env, ...(aliases.length > 0 ? { - instructions: `Studio's per-session Sapiom MCP servers are: ${aliases.join(", ")}. Use these session-specific servers for Sapiom tools; they carry this session's configuration and credentials.`, + instructions: `Studio MCP server names for this session: ${aliases.join("; ")}. References to the original server names in your other instructions mean these session-specific registrations. Use their Sapiom tools; they carry this session's configuration and credentials.`, } : {}), }; diff --git a/packages/harness/src/server/auth-mcp-wiring.test.ts b/packages/harness/src/server/auth-mcp-wiring.test.ts index 2f272967b..95b9ba604 100644 --- a/packages/harness/src/server/auth-mcp-wiring.test.ts +++ b/packages/harness/src/server/auth-mcp-wiring.test.ts @@ -286,7 +286,8 @@ describe("Agent Studio MCP authentication wiring", () => { 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).toBe("staging"); + 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/); }; From 1ce75c201c58b777b0b0565e70acad23c4e2d7c5 Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Tue, 8 Sep 2026 10:35:04 -0700 Subject: [PATCH 3/6] test(harness): verify Codex MCP child environment delivery --- .../adapters/codex-mcp.integration.test.ts | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts index 603d7bb34..4a1d4003f 100644 --- a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts +++ b/packages/harness/src/core/adapters/codex-mcp.integration.test.ts @@ -258,20 +258,41 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( : []), ].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", - SAPIOM_REVIEW_STDIO_SECRET: "synthetic-stdio-secret", + ...(existingRegistrations + ? { SAPIOM_REVIEW_STDIO_SECRET: "synthetic-stdio-secret" } + : {}), }, }, }); @@ -285,6 +306,14 @@ describe.skipIf(process.env.RUN_CODEX_MCP_INTEGRATION !== "1")( 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(""); From 4a293780615078f7039d45ea923341b5d661b27c Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Tue, 8 Sep 2026 10:50:38 -0700 Subject: [PATCH 4/6] test(harness): cover managed Codex launch and principal-bound workspaces --- .../src/core/adapters/codex-mcp.test.ts | 30 +++++++++++++++++++ .../src/server/auth-mcp-wiring.test.ts | 22 ++++++++++---- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/packages/harness/src/core/adapters/codex-mcp.test.ts b/packages/harness/src/core/adapters/codex-mcp.test.ts index 23f08cbb8..b0195bc87 100644 --- a/packages/harness/src/core/adapters/codex-mcp.test.ts +++ b/packages/harness/src/core/adapters/codex-mcp.test.ts @@ -115,6 +115,36 @@ describe("Codex per-session MCP configuration", () => { expect(process.env.SAPIOM_API_KEY).toBe(parentApiKey); }); + it("keeps managed runtime arguments and places MCP options before the initial prompt", async () => { + await writeConfig({ + sapiom: { + type: "http", + url: "https://api.sapiom.ai/v1/mcp", + headers: { "x-api-key": "private-managed-key" }, + }, + }); + const managed = new CodexAdapter({ + binary: "/Applications/Agent Studio.app/Contents/MacOS/Agent Studio", + binaryArgs: ["/managed/codex/launcher.cjs"], + binaryEnv: { ELECTRON_RUN_AS_NODE: "1" }, + }); + const opts = { ...options(), initialPrompt: "Create an agent" }; + const launch = managed.launch(opts); + const resume = managed.resume("rollout-1", opts); + for (const spec of [launch, resume]) { + expect(spec.args[0]).toBe("/managed/codex/launcher.cjs"); + expect(spec.env.ELECTRON_RUN_AS_NODE).toBe("1"); + expect(spec.env.SAPIOM_CODEX_MCP_0_HEADER_0).toBe("private-managed-key"); + expect(serverArg(spec, "sapiom")).toBeDefined(); + } + expect(launch.args.slice(-2)).toEqual(["--", opts.initialPrompt]); + expect(launch.args.indexOf(serverArg(launch, "sapiom")!)).toBeLessThan( + launch.args.indexOf("--"), + ); + expect(resume.args.slice(1, 3)).toEqual(["resume", "rollout-1"]); + expect(resume.args).not.toContain(opts.initialPrompt); + }); + it("supports signed-out sessions and the default npx launcher", async () => { await writeConfig({ sapiom: { type: "http", url: "https://api.sapiom.ai/v1/mcp" }, diff --git a/packages/harness/src/server/auth-mcp-wiring.test.ts b/packages/harness/src/server/auth-mcp-wiring.test.ts index 95b9ba604..40e668b91 100644 --- a/packages/harness/src/server/auth-mcp-wiring.test.ts +++ b/packages/harness/src/server/auth-mcp-wiring.test.ts @@ -187,6 +187,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-")); @@ -204,6 +205,7 @@ describe("Agent Studio MCP authentication wiring", () => { authFixture.credential = null; authFixture.readError = null; captures = []; + identityWorkspaces = []; vi.clearAllMocks(); }); @@ -212,6 +214,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, @@ -276,8 +279,9 @@ describe("Agent Studio MCP authentication wiring", () => { 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 () => { - const response = await post("/api/sessions", { cwd: projectRoot, harness: "codex" }); + 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 }>; }; @@ -296,13 +300,17 @@ describe("Agent Studio MCP authentication wiring", () => { expectWiring(); expect((await post("/api/auth/start")).status).toBe(200); await vi.waitFor(() => expect(writeCredentials).toHaveBeenCalledOnce()); - const signedIn = await create(); + // 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 = credential("rotated-key"); + 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"); @@ -310,10 +318,12 @@ describe("Agent Studio MCP authentication wiring", () => { expect(captures.at(-1)!.spec!.args.filter((arg) => arg.startsWith("mcp_servers."))) .toEqual(launchArgs.filter((arg) => arg.startsWith("mcp_servers."))); - await create(); + await create(signedInRoot); expectWiring("rotated-key"); expect((await post("/api/auth/disconnect")).status).toBe(200); - await create(); + const signedOutRoot = await mkdtemp(join(tmpdir(), "harness-auth-signed-out-")); + identityWorkspaces.push(signedOutRoot); + await create(signedOutRoot); expectWiring(); expect(clearCredentials).toHaveBeenCalled(); }, 20_000); From cdf463643b2f728a3943ed48ab375931b81d55f7 Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Tue, 8 Sep 2026 11:21:24 -0700 Subject: [PATCH 5/6] docs(harness): remove stale Codex task limitation --- packages/harness/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/harness/README.md b/packages/harness/README.md index e2424231a..3e9df1b7f 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -43,7 +43,7 @@ the same name. Credentials are passed through Codex's environment and cleared fr shell-tool environments; they never appear in command arguments. Authoring-process settings stay on the MCP server. Studio does not write to your Codex `config.toml`. If a generated MCP file cannot be read or parsed, the session reports an error so you can start a -new session to regenerate it. Codex background tasks remain unsupported. +new session to regenerate it. ## Telemetry From 40994e256c35b81ef42548f86ef58d13d59af465 Mon Sep 17 00:00:00 2001 From: Brett Wallace Date: Tue, 8 Sep 2026 11:33:48 -0700 Subject: [PATCH 6/6] refactor(harness): separate MCP integration validation --- packages/harness/README.md | 16 - .../adapters/codex-mcp.integration.test.ts | 355 ------------------ .../src/server/auth-mcp-wiring.test.ts | 105 +----- 3 files changed, 3 insertions(+), 473 deletions(-) delete mode 100644 packages/harness/src/core/adapters/codex-mcp.integration.test.ts diff --git a/packages/harness/README.md b/packages/harness/README.md index 3e9df1b7f..d8b830be2 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -97,22 +97,6 @@ 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, first build the workspace dependencies, then run: - -```bash -RUN_CODEX_MCP_INTEGRATION=1 pnpm --filter @sapiom/harness exec vitest run src/core/adapters/codex-mcp.integration.test.ts -``` - -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 deleted file mode 100644 index 4a1d4003f..000000000 --- a/packages/harness/src/core/adapters/codex-mcp.integration.test.ts +++ /dev/null @@ -1,355 +0,0 @@ -/** - * 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. - * Run after building workspace dependencies with RUN_CODEX_MCP_INTEGRATION=1. - * CODEX_TEST_BINARY can select an installed CLI version. 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; -} - -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, - { - resolve: (result: unknown) => void; - reject: (error: Error) => void; - } - >(); - let id = 0; - const fail = (): void => { - for (const request of pending.values()) { - request.reject( - new Error("Codex MCP probe failed; check CLI compatibility."), - ); - } - pending.clear(); - }; - // Drain diagnostics, but never include subprocess output/config in failures. - child.stderr.resume(); - child.on("error", fail); - child.on("exit", fail); - lines.on("line", (line) => { - let response: { id?: number; error?: unknown; 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) request.reject(new Error("Codex MCP RPC failed.")); - else request.resolve(response.result); - }); - const request = (method: string, params: unknown): Promise => { - const requestId = ++id; - return new Promise((resolve, reject) => { - pending.set(requestId, { resolve, reject }); - child.stdin.write( - JSON.stringify({ id: requestId, method, params }) + "\n", - ); - }); - }; - const timeout = setTimeout(() => { - fail(); - child.kill("SIGKILL"); - }, 25_000); - 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 failed."); - 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) => { - 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); - }); - 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", - ); - 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 40e668b91..0b7a7d631 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 and SAP-3122. + * Lifecycle-level regression coverage for SAP-3114. * * 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, writeFileSync } from "node:fs"; +import { readFileSync } from "node:fs"; import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -88,7 +88,6 @@ 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"; @@ -99,35 +98,6 @@ 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(); - 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 { @@ -187,7 +157,6 @@ 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-")); @@ -205,7 +174,6 @@ describe("Agent Studio MCP authentication wiring", () => { authFixture.credential = null; authFixture.readError = null; captures = []; - identityWorkspaces = []; vi.clearAllMocks(); }); @@ -214,7 +182,6 @@ 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, @@ -231,7 +198,7 @@ describe("Agent Studio MCP authentication wiring", () => { async function boot( options: Pick< Parameters[0], - "identity" | "authMode" | "adapters" | "codexHomeDir" + "identity" | "authMode" > = {}, ): Promise { server = await startServer({ @@ -276,72 +243,6 @@ describe("Agent Studio MCP authentication wiring", () => { expect(injectedKey(captures[0])).toBe("browser-key"); }); - 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 vi.waitFor(() => expect(writeCredentials).toHaveBeenCalledOnce()); - // 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");