stop codex falling back to opus on mcp failure

Author: charlesvien

packages/agent/src/adapters/acp-connection.ts

@@ -206,6 +206,7 @@ function createCodexConnection(config: AcpConnectionConfig): AcpConnection {
       processCallbacks: config.processCallbacks,
       posthogApiConfig: resolveEnricherApiConfig(config),
       onStructuredOutput: config.onStructuredOutput,
+      logger: config.logger?.child("CodexAcpAgent"),
     });
     return agent;
   }, agentStream);

packages/agent/src/adapters/codex/codex-agent.ts

@@ -128,6 +128,13 @@ export interface CodexAcpAgentOptions {
   processCallbacks?: ProcessSpawnedCallback;
   posthogApiConfig?: PostHogAPIConfig;
   onStructuredOutput?: (output: Record<string, unknown>) => Promise<void>;
+  /**
+   * Logger wired to the host log sink. Without it the codex-acp subprocess
+   * stderr, spawn failures and exit codes are written to a throwaway logger and
+   * never reach the exported logs, so a crash surfaces only as a generic
+   * "ACP connection closed" with no cause.
+   */
+  logger?: Logger;
 }
 
 type CodexSession = BaseSession & {
@@ -320,7 +327,8 @@ export class CodexAcpAgent extends BaseAcpAgent {
 
   constructor(client: AgentSideConnection, options: CodexAcpAgentOptions) {
     super(client);
-    this.logger = new Logger({ debug: true, prefix: "[CodexAcpAgent]" });
+    this.logger =
+      options.logger ?? new Logger({ debug: true, prefix: "[CodexAcpAgent]" });
 
     // Load user codex settings before spawning so spawnCodexProcess can
     // filter out any [mcp_servers.*] entries from ~/.codex/config.toml.

packages/workspace-server/src/services/agent/agent.test.ts

@@ -199,6 +199,10 @@ describe("AgentService", () => {
   beforeEach(() => {
     vi.clearAllMocks();
 
+    // The Codex MCP reachability probe hits the network; default it to "reachable"
+    // so unrelated session tests stay deterministic and offline-safe.
+    vi.stubGlobal("fetch", vi.fn().mockResolvedValue({ body: null }));
+
     deps = createMockDependencies();
     service = new AgentService(
       deps.processTracking as never,
@@ -219,6 +223,7 @@ describe("AgentService", () => {
 
   afterEach(() => {
     vi.restoreAllMocks();
+    vi.unstubAllGlobals();
   });
 
   describe("MCP servers", () => {
@@ -291,6 +296,29 @@ describe("AgentService", () => {
       expect(codexMcp).toEqual(claudeMcp);
     });
 
+    it("drops unreachable MCP servers for codex but keeps them for claude", async () => {
+      vi.stubGlobal(
+        "fetch",
+        vi.fn().mockRejectedValue(new Error("ECONNREFUSED")),
+      );
+
+      await service.startSession({
+        ...baseSessionParams,
+        taskRunId: "run-claude",
+        adapter: "claude",
+      });
+      await service.startSession({
+        ...baseSessionParams,
+        taskRunId: "run-codex",
+        adapter: "codex",
+      });
+
+      // Claude connects to MCP lazily, so an unreachable server is harmless.
+      expect(mockNewSession.mock.calls[0][0].mcpServers).toHaveLength(1);
+      // codex-acp dies on an unreachable server, so it must be pruned.
+      expect(mockNewSession.mock.calls[1][0].mcpServers).toHaveLength(0);
+    });
+
     it("passes reasoning effort to local Codex startup options", async () => {
       await service.startSession({
         ...baseSessionParams,

packages/workspace-server/src/services/agent/agent.ts

@@ -762,6 +762,16 @@ When creating pull requests, add the following footer at the end of the PR descr
         })),
       );
 
+      // codex-acp connects to every MCP server eagerly during session creation
+      // and treats an unreachable one as fatal, which kills the session
+      // ("ACP connection closed") and makes the host silently fall back to a
+      // Claude/Opus session. Claude connects lazily and is unaffected, so only
+      // the Codex server list is pruned to the reachable ones.
+      const sessionMcpServers =
+        adapter === "codex"
+          ? await this.filterReachableMcpServers(mcpServers, taskRunId)
+          : mcpServers;
+
       let externalPlugins: Awaited<ReturnType<typeof discoverExternalPlugins>> =
         [];
       try {
@@ -833,7 +843,7 @@ When creating pull requests, add the following footer at the end of the PR descr
         const resumeResponse = await connection.resumeSession({
           sessionId: existingSessionId,
           cwd: repoPath,
-          mcpServers,
+          mcpServers: sessionMcpServers,
           _meta: {
             ...(logUrl && {
               persistence: { taskId, runId: taskRunId, logUrl },
@@ -862,7 +872,7 @@ When creating pull requests, add the following footer at the end of the PR descr
         }
         const newSessionResponse = await connection.newSession({
           cwd: repoPath,
-          mcpServers,
+          mcpServers: sessionMcpServers,
           _meta: {
             taskRunId,
             environment: "local",
@@ -952,6 +962,75 @@ When creating pull requests, add the following footer at the end of the PR descr
     }
   }
 
+  private async filterReachableMcpServers<
+    T extends {
+      name: string;
+      url: string;
+      headers: Array<{ name: string; value: string }>;
+    },
+  >(servers: T[], taskRunId: string): Promise<T[]> {
+    const probed = await Promise.all(
+      servers.map(async (server) => ({
+        server,
+        reachable: await this.isMcpServerReachable(server),
+      })),
+    );
+    const reachable: T[] = [];
+    for (const { server, reachable: ok } of probed) {
+      if (ok) {
+        reachable.push(server);
+      } else {
+        this.log.warn(
+          "Dropping unreachable MCP server from Codex session; codex-acp treats an unreachable server as a fatal startup error",
+          { taskRunId, server: server.name, url: server.url },
+        );
+      }
+    }
+    return reachable;
+  }
+
+  private async isMcpServerReachable(server: {
+    url: string;
+    headers: Array<{ name: string; value: string }>;
+  }): Promise<boolean> {
+    const PROBE_TIMEOUT_MS = 4_000;
+    try {
+      const headers: Record<string, string> = {
+        "content-type": "application/json",
+        accept: "application/json, text/event-stream",
+      };
+      for (const header of server.headers) {
+        headers[header.name] = header.value;
+      }
+      const response = await fetch(server.url, {
+        method: "POST",
+        headers,
+        body: JSON.stringify({
+          jsonrpc: "2.0",
+          id: 0,
+          method: "initialize",
+          params: {
+            protocolVersion: "2025-06-18",
+            capabilities: {},
+            clientInfo: { name: "posthog-code", version: "1.0.0" },
+          },
+        }),
+        signal: AbortSignal.timeout(PROBE_TIMEOUT_MS),
+      });
+      await response.body?.cancel();
+      // Any HTTP response means the endpoint is reachable. codex-acp only treats
+      // transport failures (connection refused, DNS, timeout) as fatal; HTTP or
+      // JSON-RPC error responses are handled gracefully.
+      return true;
+    } catch (err) {
+      this.log.debug("MCP server reachability probe failed", {
+        url: server.url,
+        error: err instanceof Error ? err.message : String(err),
+      });
+      return false;
+    }
+  }
+
   async prompt(
     sessionId: string,
     prompt: ContentBlock[],