From afd872ebb01c82c186dae410db3c59e3df150aa0 Mon Sep 17 00:00:00 2001 From: Juan Cruz Fortunatti Date: Sat, 12 Sep 2026 17:41:44 +0200 Subject: [PATCH] fix: require the per-wake Codex MCP connection --- docs/engines.md | 5 +++ src/pi/cliEngineSpawn.test.ts | 22 ++++++++---- src/pi/cliEngineSpawn.ts | 11 ++++-- src/pi/cliSessionRequiredMcp.test.ts | 50 ++++++++++++++++++++++++++++ 4 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 src/pi/cliSessionRequiredMcp.test.ts diff --git a/docs/engines.md b/docs/engines.md index 313ef4f..7c1cdb9 100644 --- a/docs/engines.md +++ b/docs/engines.md @@ -57,6 +57,11 @@ Codex uses a private `.codex/auth.json` under each agent runtime home. Optional Codex config fields are `model`, `reasoningEffort`, and the fixed no-network workspace sandbox policy. +Every Codex wake enables and requires Daimon's per-wake MCP server, including +standalone and strict sandbox launches. If the server cannot initialize, Codex +fails the wake before model work instead of continuing with only built-in tools. +The startup error follows the existing bounded, redacted engine failure path. + The production Grok path uses an external Daimon engine broker with one durable subscription credential authority. Agent workers receive scoped capabilities; the broker owns refresh and stale-credential recovery. The runtime checks broker diff --git a/src/pi/cliEngineSpawn.test.ts b/src/pi/cliEngineSpawn.test.ts index 9dfe702..b60f768 100644 --- a/src/pi/cliEngineSpawn.test.ts +++ b/src/pi/cliEngineSpawn.test.ts @@ -45,11 +45,19 @@ test("Codex output, sandbox, config, and cwd boundaries reject caller overrides" assert.equal(args.includes("--json"), true); }); -test("codex argv is byte-identical to before model selection existed when model/reasoningEffort are absent", () => { - const before = ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-"]; - const after = renderCodexArgs({ commandArgs: [] }, "/workspace", "http://127.0.0.1:1/mcp"); - assert.deepEqual(after, before); +for (const mode of ["standalone", "strict"] as const) { + test(`Codex ${mode} launch requires the per-wake MCP server`, () => { + const args = renderCodexArgs(mode === "standalone" ? {} : { + codexSandbox: { mode: "workspace-write", networkAccess: false, webSearch: "disabled" } + }, "/workspace", "http://127.0.0.1:1/mcp"); + assert.deepEqual(args.slice(-5), ["-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); + }); +} + +test("codex argv preserves model defaults while requiring the per-wake MCP server", () => { + const args = renderCodexArgs({ commandArgs: [] }, "/workspace", "http://127.0.0.1:1/mcp"); + assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--color", "never", "--json", "-C", "/workspace", + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex strict policy is rendered as per-turn Daimon-owned CLI config", () => { @@ -125,13 +133,13 @@ test("Codex permission profile lets protected denies override readable paths", ( test("codex argv renders -m for a pinned model and leaves everything else untouched", () => { const args = renderCodexArgs({ commandArgs: [], model: "gpt-5-codex" }, "/workspace", "http://127.0.0.1:1/mcp"); assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--model=gpt-5-codex", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-"]); + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex argv renders both model and reasoningEffort together in stable order", () => { const args = renderCodexArgs({ commandArgs: [], model: "gpt-5-codex", reasoningEffort: "xhigh" }, "/workspace", "http://127.0.0.1:1/mcp"); assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--model=gpt-5-codex", "-c", "model_reasoning_effort=xhigh", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-"]); + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex argv renders reasoningEffort alone without a model flag", () => { diff --git a/src/pi/cliEngineSpawn.ts b/src/pi/cliEngineSpawn.ts index f6b9b07..eedae24 100644 --- a/src/pi/cliEngineSpawn.ts +++ b/src/pi/cliEngineSpawn.ts @@ -22,8 +22,11 @@ export const renderGrokSandboxArgs = ( * `--flag=value` argv token (never a bare flag followed by a separate value * token), so a value that itself looks like a flag (e.g. `--sandbox`) can * never be parsed as a second, independent argument — the same shape already - * used below for `mcp_servers.daimon.url=`. Omitting both fields renders the - * exact argv Daimon produced before model selection existed. + * used below for `mcp_servers.daimon.url=`. Omitting both fields leaves Codex's + * model defaults unchanged. + * + * The per-wake MCP server is always enabled and required. Otherwise Codex can + * continue a normal turn with only built-in tools after MCP startup fails. */ export const renderCodexArgs = ( options: Pick, @@ -51,7 +54,9 @@ export const renderCodexArgs = ( ...(options.model === undefined ? [] : [`--model=${options.model}`]), ...(options.reasoningEffort === undefined ? [] : ["-c", `model_reasoning_effort=${options.reasoningEffort}`]), "--color", "never", "--json", "-C", cwd, - "-c", `mcp_servers.daimon.url=${endpoint}`, "-"]; + "-c", `mcp_servers.daimon.url=${endpoint}`, + "-c", "mcp_servers.daimon.enabled=true", + "-c", "mcp_servers.daimon.required=true", "-"]; }; export const renderCodexPermissionProfile = ( diff --git a/src/pi/cliSessionRequiredMcp.test.ts b/src/pi/cliSessionRequiredMcp.test.ts new file mode 100644 index 0000000..e0e67fe --- /dev/null +++ b/src/pi/cliSessionRequiredMcp.test.ts @@ -0,0 +1,50 @@ +import assert from "node:assert/strict"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; + +import { createCliSessionFactory } from "./cliSession.js"; + +for (const mode of ["standalone", "strict"] as const) { + test(`Codex ${mode} MCP startup failure rejects the wake without publishing or inventing usage`, async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "daimon-required-mcp-")); + const engine = path.join(root, "codex-stub.mjs"); + const secret = "required-mcp-diagnostic-secret"; + // Simulate Codex's optional-server continuation and required-server exit. + // The generated launch config must select the failure before a normal turn. + await writeFile(engine, [ + "for await (const chunk of process.stdin) {}", + "if (process.argv.includes('mcp_servers.daimon.enabled=true') && process.argv.includes('mcp_servers.daimon.required=true')) {", + ` process.stderr.write(${JSON.stringify(`required MCP server daimon failed startup: ${secret}`)});`, + " process.exit(1);", + "}", + "process.stdout.write(JSON.stringify({ type: 'item.completed', item: { type: 'agent_message', text: 'continued without tools' } }) + '\\n');", + "process.stdout.write(JSON.stringify({ type: 'turn.completed', usage: { input_tokens: 10, cached_input_tokens: 0, output_tokens: 1 } }) + '\\n');" + ].join("\n")); + const published: unknown[] = []; + const usage: unknown[] = []; + try { + const { session } = await createCliSessionFactory({ + command: process.execPath, + commandArgs: [engine], + engine: "codex", + timeoutMs: 10_000, + credentialSecretValues: async () => [secret], + onTurnUsage: async (value) => { usage.push(value); }, + ...(mode === "strict" ? { codexSandbox: { mode: "workspace-write", networkAccess: false, webSearch: "disabled" } as const } : {}) + })({ cwd: root }); + session.subscribe((event) => { published.push(event); }); + try { + await assert.rejects(session.prompt("wake"), (error: unknown) => { + assert.ok(error instanceof Error); + assert.match(error.message, /CLI engine exited 1: required MCP server daimon failed startup:/u); + assert.equal(error.message.includes(secret), false); + return true; + }); + assert.deepEqual(published, []); + assert.deepEqual(usage, []); + } finally { await session.disposeAsync?.(); } + } finally { await rm(root, { recursive: true, force: true }); } + }); +}