fix codex crash from nested mcp_servers tables

Author: charlesvien

packages/agent/src/adapters/codex/settings.test.ts

@@ -0,0 +1,42 @@
+import { describe, expect, it, vi } from "vitest";
+
+const readFileSync = vi.hoisted(() => vi.fn());
+vi.mock("node:fs", () => ({ readFileSync }));
+
+const { CodexSettingsManager } = await import("./settings");
+
+function serverNamesFor(toml: string): string[] {
+  readFileSync.mockReturnValue(toml);
+  return new CodexSettingsManager("/repo").getSettings().mcpServerNames.sort();
+}
+
+describe("CodexSettingsManager MCP server names", () => {
+  // Regression: a `[mcp_servers.<name>.env]` table was treated as its own
+  // server, so the spawn emitted `mcp_servers.<name>.env.enabled=false`, which
+  // sets a boolean on codex's string-typed env map. codex-acp then rejected the
+  // whole config, crashed the session, and the host silently ran Claude/Opus.
+  it("collapses nested mcp_servers sub-tables to the parent server name", () => {
+    expect(
+      serverNamesFor(
+        [
+          "[mcp_servers.node_repl]",
+          'command = "node"',
+          "[mcp_servers.node_repl.env]",
+          'FOO = "bar"',
+          "[mcp_servers.other]",
+          'command = "x"',
+        ].join("\n"),
+      ),
+    ).toEqual(["node_repl", "other"]);
+  });
+
+  it("keeps the inner name for a quoted dotted server key", () => {
+    expect(
+      serverNamesFor(['[mcp_servers."my.server"]', 'command = "x"'].join("\n")),
+    ).toEqual(["my.server"]);
+  });
+
+  it("returns no servers when none are declared", () => {
+    expect(serverNamesFor('model = "gpt-5.5"')).toEqual([]);
+  });
+});

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

@@ -71,6 +71,24 @@ export class CodexSettingsManager {
   }
 }
 
+/**
+ * Extracts the server name from a `mcp_servers.<name>...` section path, taking
+ * only the first key segment. A nested table like `mcp_servers.foo.env`
+ * describes the `env` field of server `foo`, not a separate server, so it must
+ * collapse to `foo`. Treating it as its own server emits
+ * `mcp_servers.foo.env.enabled=false`, which sets a boolean on the string-typed
+ * env map and makes codex-acp reject the whole config (it then crashes and the
+ * host silently falls back to Claude). Quoted segments (`"a.b"`) keep their dots.
+ */
+function firstMcpServerName(sectionPath: string): string | null {
+  const trimmed = sectionPath.trim();
+  if (!trimmed) return null;
+  const quoted = trimmed.match(/^(["'])(.*?)\1/);
+  if (quoted) return quoted[2] ?? null;
+  const dotIndex = trimmed.indexOf(".");
+  return dotIndex === -1 ? trimmed : trimmed.slice(0, dotIndex);
+}
+
 /**
  * Minimal TOML parser for codex config.toml.
  * Handles flat key=value pairs and [projects."path"] sections.
@@ -90,7 +108,10 @@ function parseCodexToml(content: string, cwd: string): CodexSettings {
     if (sectionMatch) {
       currentSection = sectionMatch[1] ?? "";
       if (currentSection.startsWith("mcp_servers.")) {
-        mcpServerNames.add(currentSection.slice("mcp_servers.".length));
+        const serverName = firstMcpServerName(
+          currentSection.slice("mcp_servers.".length),
+        );
+        if (serverName) mcpServerNames.add(serverName);
       }
       continue;
     }

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

@@ -36,7 +36,14 @@ function buildConfigArgs(options: CodexProcessOptions): string[] {
   // Disable the user's local MCPs one-by-one so Codex only uses the MCPs we
   // provide via ACP. We can't use `-c mcp_servers={}` because that makes Codex
   // ignore MCPs entirely, including the ones we inject later.
+  //
+  // Only bare-key names are emitted: codex's `-c` parser rejects quoted key
+  // segments, so a name with a dot or other special character cannot be
+  // expressed as `mcp_servers.<name>.enabled=false` without producing an
+  // override that fails to load and crashes the whole codex session. Skipping
+  // such a name leaves that server enabled (harmless) instead of killing codex.
   for (const name of options.settings?.mcpServerNames ?? []) {
+    if (!/^[A-Za-z0-9_-]+$/.test(name)) continue;
     args.push("-c", `mcp_servers.${name}.enabled=false`);
   }