fix(models): Persist selected model into new sessions

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/base-acp-agent.ts

@@ -158,6 +158,17 @@ export abstract class BaseAcpAgent implements Agent {
 
     if (!options.some((opt) => opt.value === currentModelId)) {
       if (!isAnthropicModelId(currentModelId)) {
+        // A non-Anthropic model id reached the Claude adapter, which means the
+        // adapter and model desynced upstream (e.g. a Codex model paired with
+        // the Claude adapter). Log it instead of silently masquerading as a
+        // deliberate Opus session.
+        this.logger.warn(
+          "Non-Anthropic model requested on Claude adapter; falling back to default model",
+          {
+            requestedModel: currentModelId,
+            fallbackModel: DEFAULT_GATEWAY_MODEL,
+          },
+        );
         currentModelId = DEFAULT_GATEWAY_MODEL;
       }
     }

packages/agent/src/adapters/claude/claude-agent.resume-model.test.ts

@@ -3,6 +3,7 @@ import * as os from "node:os";
 import * as path from "node:path";
 import type { AgentSideConnection } from "@agentclientprotocol/sdk";
 import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
+import { DEFAULT_GATEWAY_MODEL } from "../../gateway-models";
 
 type SdkQueryHandle = {
   interrupt: ReturnType<typeof vi.fn>;
@@ -153,18 +154,43 @@ describe("ClaudeAcpAgent session model on resume", () => {
     },
   );
 
-  it("does not call setModel for a new session on the default model", async () => {
+  // New sessions pass the model to the SDK at spawn, never via setModel. The
+  // Codex-model row guards the desync that surfaced as "picked gpt-5.5, session
+  // ran Opus": a non-Anthropic id on the Claude adapter must fall back to the
+  // default AND warn rather than silently masquerade as a deliberate Opus run.
+  it.each([
+    {
+      name: "uses the gateway default and never calls setModel without a requested model",
+      model: undefined,
+      expectsWarn: false,
+    },
+    {
+      name: "warns and falls back to the default when a Codex model reaches the Claude adapter",
+      model: "gpt-5.5",
+      expectsWarn: true,
+    },
+  ])("newSession $name", async ({ model, expectsWarn }) => {
     const agent = makeAgent();
+    const warnSpy = vi.spyOn(agent.logger, "warn");
 
-    await agent.newSession({
+    const response = await agent.newSession({
       cwd,
       mcpServers: [],
-      _meta: { taskRunId: "run-3" },
+      _meta: { taskRunId: "run-new", ...(model ? { model } : {}) },
     });
 
-    // New sessions already pass options.model to the SDK at spawn.
-    expect(createdQueries).toHaveLength(1);
     expect(createdQueries[0].setModel).not.toHaveBeenCalled();
+    expect(getModelConfigOption(response)?.currentValue).toBe(
+      DEFAULT_GATEWAY_MODEL,
+    );
+    if (expectsWarn) {
+      expect(warnSpy).toHaveBeenCalledWith(
+        expect.stringContaining("Non-Anthropic model"),
+        expect.objectContaining({ requestedModel: model }),
+      );
+    } else {
+      expect(warnSpy).not.toHaveBeenCalled();
+    }
   });
 
   // The timeout *message* (RequestError "... timed out after ...") is covered

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/agent/src/adapters/codex/settings.test.ts

@@ -0,0 +1,54 @@
+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.each([
+    {
+      name: "collapses a nested sub-table to its parent server and dedupes",
+      toml: [
+        "[mcp_servers.node_repl]",
+        'command = "node"',
+        "[mcp_servers.node_repl.env]",
+        'FOO = "bar"',
+        "[mcp_servers.other]",
+        'command = "x"',
+      ],
+      expected: ["node_repl", "other"],
+    },
+    {
+      name: "collapses a deeply nested sub-table to its parent server",
+      toml: ["[mcp_servers.foo.bar.baz]", 'k = "v"'],
+      expected: ["foo"],
+    },
+    {
+      name: "keeps the inner name for a double-quoted dotted server key",
+      toml: ['[mcp_servers."my.server"]', 'command = "x"'],
+      expected: ["my.server"],
+    },
+    {
+      name: "keeps the inner name for a single-quoted dotted server key",
+      toml: ["[mcp_servers.'my.server']", 'command = "x"'],
+      expected: ["my.server"],
+    },
+    {
+      name: "returns no servers when none are declared",
+      toml: ['model = "gpt-5.5"'],
+      expected: [],
+    },
+  ])("$name", ({ toml, expected }) => {
+    expect(serverNamesFor(toml.join("\n"))).toEqual(expected);
+  });
+});

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.test.ts

@@ -0,0 +1,40 @@
+import { describe, expect, it, vi } from "vitest";
+import { Logger } from "../../utils/logger";
+
+const spawnMock = vi.hoisted(() => vi.fn());
+vi.mock("node:child_process", () => ({ spawn: spawnMock }));
+
+const { spawnCodexProcess } = await import("./spawn");
+
+function makeFakeChild() {
+  return {
+    stdin: { destroy: vi.fn() },
+    stdout: { on: vi.fn(), destroy: vi.fn() },
+    stderr: { on: vi.fn(), destroy: vi.fn() },
+    on: vi.fn(),
+    kill: vi.fn(),
+    pid: 1234,
+  };
+}
+
+describe("spawnCodexProcess MCP disable args", () => {
+  it("disables bare-key servers but skips names codex's -c parser cannot express", () => {
+    spawnMock.mockReturnValue(makeFakeChild());
+
+    spawnCodexProcess({
+      logger: new Logger({ debug: false }),
+      settings: {
+        mcpServerNames: ["simple", "with-dash", "my.server", "weird name"],
+      },
+    });
+
+    const args: string[] = spawnMock.mock.calls[0][1];
+    expect(args).toContain("mcp_servers.simple.enabled=false");
+    expect(args).toContain("mcp_servers.with-dash.enabled=false");
+    // A dotted or otherwise non-bare name would emit an override codex rejects,
+    // which crashes the whole session, so it is skipped (the server stays
+    // enabled, which is harmless).
+    expect(args.some((arg) => arg.includes("my.server"))).toBe(false);
+    expect(args.some((arg) => arg.includes("weird name"))).toBe(false);
+  });
+});

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`);
   }
 

packages/ui/src/features/task-detail/components/TaskInput.tsx

@@ -808,7 +808,9 @@ export function TaskInput({
               isLoading={isCreatingTask}
               autoFocus
               clearOnSubmit={false}
-              submitDisabledExternal={!canSubmit || isCreatingTask || !isOnline}
+              submitDisabledExternal={
+                !canSubmit || isCreatingTask || !isOnline || isPreviewLoading
+              }
               tourTarget="task-input"
               repoPath={selectedDirectory}
               modeOption={modeOption}

packages/ui/src/features/task-detail/hooks/usePreviewConfig.ts

@@ -50,15 +50,27 @@ export function usePreviewConfig(
   const [configOptions, setConfigOptions] = useState<SessionConfigOption[]>([]);
   const [isLoading, setIsLoading] = useState(true);
   const abortRef = useRef<AbortController | null>(null);
+  const hasHydrated = useSettingsStore((state) => state._hasHydrated);
 
   useEffect(() => {
     if (!apiHost) return;
 
+    // Wait for the settings store to finish its async hydration before
+    // resolving the model. Otherwise lastUsedModel and lastUsedAdapter read as
+    // their pre-hydration defaults, the restore below is skipped, and the
+    // selector silently falls back to the server default (Opus for Claude).
+    // isLoading initializes to true, so the picker stays loading until hydration
+    // lands and the fetch below resolves.
+    if (!hasHydrated) return;
+
     abortRef.current?.abort();
     const abort = new AbortController();
     abortRef.current = abort;
 
     setIsLoading(true);
+    // Drop the previous adapter's options so a stale model id can never be sent
+    // as the current selection while the new adapter's config is loading.
+    setConfigOptions([]);
 
     hostClient.agent.getPreviewConfigOptions
       .query({ apiHost, adapter }, { signal: abort.signal })
@@ -122,7 +134,7 @@ export function usePreviewConfig(
     return () => {
       abort.abort();
     };
-  }, [adapter, apiHost, hostClient]);
+  }, [adapter, apiHost, hostClient, hasHydrated]);
 
   const setConfigOption = useCallback(
     (configId: string, value: string) => {

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", () => {
@@ -273,7 +278,7 @@ describe("AgentService", () => {
       );
     });
 
-    it("passes identical MCP servers regardless of adapter", async () => {
+    it("passes identical MCP servers to both adapters when all servers are reachable", async () => {
       await service.startSession({
         ...baseSessionParams,
         taskRunId: "run-claude",
@@ -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,