[#254] Wire provider selection into New Story flow

app/routes/terminal-sessions.test.ts

@@ -1,5 +1,5 @@
 import { describe, it, expect } from "vitest";
-import { resumeIdFrom, isSessionRecord, type StoredValue } from "./terminal";
+import { resumeIdFrom, isSessionRecord, carrySessionAcrossRename, type StoredValue } from "./terminal";
 
 /**
  * Session-store back-compat: terminal-sessions.json may hold legacy bare-string
@@ -68,3 +68,51 @@ describe("session store map mutation (no wholesale migration)", () => {
     expect(resumeIdFrom(roundTripped["c"])).toBeNull();
   });
 });
+
+describe("carrySessionAcrossRename (rename preserves stored shape)", () => {
+  it("preserves a Codex provider-aware record (does not flatten to the fallback UUID)", () => {
+    // Regression for PR #260: a fresh cartoon _new_* Codex session stores
+    // {provider:"codex", sessionId:null}; renaming must keep that record so a
+    // later resume builds `codex resume --last`, not `codex resume <fallback-uuid>`.
+    const map: Record<string, StoredValue> = {
+      "_new_123": { provider: "codex", sessionId: null, lastStartedAt: 111 },
+    };
+    carrySessionAcrossRename(map, "_new_123", "my-toon", "fallback-pty-uuid");
+    expect(map["_new_123"]).toBeUndefined();
+    expect(map["my-toon"]).toEqual({ provider: "codex", sessionId: null, lastStartedAt: 111 });
+    expect(map["my-toon"]).not.toBe("fallback-pty-uuid");
+    expect(resumeIdFrom(map["my-toon"])).toBe(null);
+  });
+
+  it("preserves a Codex record with a real session id", () => {
+    const map: Record<string, StoredValue> = {
+      "_new_9": { provider: "codex", sessionId: "cdx-real", lastStartedAt: 5 },
+    };
+    carrySessionAcrossRename(map, "_new_9", "toon2", "fallback");
+    expect(map["toon2"]).toEqual({ provider: "codex", sessionId: "cdx-real", lastStartedAt: 5 });
+    expect(resumeIdFrom(map["toon2"])).toBe("cdx-real");
+  });
+
+  it("preserves a legacy Claude bare-string entry", () => {
+    const map: Record<string, StoredValue> = { "_new_7": "claude-uuid" };
+    carrySessionAcrossRename(map, "_new_7", "novel", "fallback");
+    expect(map["_new_7"]).toBeUndefined();
+    expect(map["novel"]).toBe("claude-uuid");
+  });
+
+  it("falls back to the live PTY session id only when no stored entry exists", () => {
+    const map: Record<string, StoredValue> = {};
+    carrySessionAcrossRename(map, "_new_x", "story", "live-uuid");
+    expect(map["story"]).toBe("live-uuid");
+  });
+
+  it("leaves unrelated entries untouched", () => {
+    const map: Record<string, StoredValue> = {
+      "_new_1": { provider: "codex", sessionId: null },
+      "other": "keep-me",
+    };
+    carrySessionAcrossRename(map, "_new_1", "renamed", "fb");
+    expect(map["other"]).toBe("keep-me");
+    expect(map["renamed"]).toEqual({ provider: "codex", sessionId: null });
+  });
+});

app/routes/terminal.test.ts

@@ -7,6 +7,7 @@ import {
   buildClaudeCommand,
   isTerminalSocketOpen,
   resolveBypass,
+  resolveProvider,
   resolveAgentCommandForSession,
   shellQuote,
 } from "./terminal";
@@ -71,6 +72,68 @@ describe("resolveBypass", () => {
   });
 });
 
+describe("resolveProvider", () => {
+  it("new story honors explicit optProvider=codex", () => {
+    expect(resolveProvider({ isNewStory: true, optProvider: "codex" })).toBe("codex");
+  });
+
+  it("new story defaults to claude without explicit flag", () => {
+    expect(resolveProvider({ isNewStory: true })).toBe("claude");
+  });
+
+  it("new story falls back to session provider when no optProvider", () => {
+    expect(resolveProvider({ isNewStory: true, sessionProvider: "codex" })).toBe("codex");
+  });
+
+  it("existing story IGNORES client optProvider (security)", () => {
+    // Malicious WS sends provider=codex, but stored metadata is claude.
+    expect(resolveProvider({ isNewStory: false, optProvider: "codex", storedProvider: "claude" })).toBe("claude");
+  });
+
+  it("existing story derives provider from stored .story.json", () => {
+    expect(resolveProvider({ isNewStory: false, storedProvider: "codex" })).toBe("codex");
+  });
+
+  it("existing story prefers in-memory session provider over stored", () => {
+    expect(resolveProvider({ isNewStory: false, optProvider: "claude", sessionProvider: "codex", storedProvider: "claude" })).toBe("codex");
+  });
+
+  // End-to-end regression for PR #260: a brand-new cartoon (_new_) session must
+  // invoke codex, not claude. Compose the two pure functions exactly as spawnPty
+  // does for a fresh _new_ spawn: resolve provider from the client flag, then
+  // render the concrete CLI command for that provider.
+  it("end-to-end: cartoon _new_ spawn (provider=codex) yields a codex command", () => {
+    const provider = resolveProvider({ isNewStory: true, optProvider: "codex" });
+    expect(provider).toBe("codex");
+    const cmd = resolveAgentCommandForSession({
+      provider,
+      mode: "normal",
+      resumeRequested: false,
+      stored: undefined,
+      newSessionId: "fresh-uuid",
+      storyDir: "/stories/_new_123",
+    });
+    expect(cmd.command).toBe("codex");
+    expect(cmd.command).not.toBe("claude");
+  });
+
+  // Byte-identical guarantee: a fiction _new_ spawn with NO provider flag still
+  // resolves to claude and renders the unchanged claude fresh-session command.
+  it("end-to-end: fiction _new_ spawn (no flag) yields the unchanged claude command", () => {
+    const provider = resolveProvider({ isNewStory: true });
+    expect(provider).toBe("claude");
+    const cmd = resolveAgentCommandForSession({
+      provider,
+      mode: "normal",
+      resumeRequested: false,
+      stored: undefined,
+      newSessionId: "fresh-uuid",
+      storyDir: "/stories/_new_123",
+    });
+    expect(cmd).toEqual({ command: "claude", args: ["--session-id", "fresh-uuid"] });
+  });
+});
+
 describe("shellQuote (command-injection safety)", () => {
   it("wraps plain values in single quotes", () => {
     expect(shellQuote("abc-123")).toBe("'abc-123'");

app/routes/terminal.ts

@@ -169,6 +169,11 @@ export function shellQuote(s: string): string {
 // reconnects before a story directory / .story.json exists).
 const agentModeBySession = new Map<string, "normal" | "bypass">();
 
+// In-memory agent provider per active session name (covers _new_ sessions and
+// reconnects before a story directory / .story.json exists). Mirrors
+// agentModeBySession exactly.
+const agentProviderBySession = new Map<string, "claude" | "codex">();
+
 /**
  * Resolve effective permissions-bypass for a spawn.
  *
@@ -192,7 +197,47 @@ export function resolveBypass(args: {
   return args.storedMode === "bypass";
 }
 
-function spawnPty(storyName: string, opts?: { sessionId?: string; resume?: boolean; bypass?: boolean }) {
+/**
+ * Resolve the effective agent provider for a spawn.
+ *
+ * Mirrors resolveBypass's trust model: the client-supplied provider flag is only
+ * trusted for a brand-new (_new_) story's first spawn. For existing stories the
+ * provider derives strictly from server-side state (already-spawned session
+ * provider, then stored .story.json), so a direct WS URL cannot force a provider
+ * on a story whose metadata says otherwise.
+ */
+export function resolveProvider(args: {
+  isNewStory: boolean;
+  optProvider?: "claude" | "codex";
+  sessionProvider?: "claude" | "codex";
+  storedProvider?: "claude" | "codex";
+}): "claude" | "codex" {
+  if (args.isNewStory) return args.optProvider ?? args.sessionProvider ?? "claude";
+  if (args.sessionProvider !== undefined) return args.sessionProvider;
+  return args.storedProvider ?? "claude";
+}
+
+/**
+ * Move a persisted session entry from one key to another, PRESERVING its stored
+ * shape. A `_new_*` Codex session is stored as a provider-aware record
+ * (`{provider:"codex", sessionId, lastStartedAt}`); a Claude session as a bare
+ * string. Renaming must keep that shape so Codex resume metadata survives. Only
+ * when there is no stored entry do we fall back to the live PTY session id (a
+ * bare string, matching legacy Claude behavior). Mutates and returns `map`.
+ */
+export function carrySessionAcrossRename(
+  map: Record<string, StoredValue>,
+  oldName: string,
+  newName: string,
+  fallbackSessionId: string,
+): Record<string, StoredValue> {
+  const existing = map[oldName];
+  delete map[oldName];
+  map[newName] = existing ?? fallbackSessionId;
+  return map;
+}
+
+function spawnPty(storyName: string, opts?: { sessionId?: string; resume?: boolean; bypass?: boolean; provider?: "claude" | "codex" }) {
   // New story sessions spawn in the stories root so Claude can create any folder
   const isNewStory = storyName.startsWith("_new_");
   const storyDir = isNewStory ? STORIES_DIR : path.join(STORIES_DIR, storyName);
@@ -212,10 +257,16 @@ function spawnPty(storyName: string, opts?: { sessionId?: string; resume?: boole
   });
   agentModeBySession.set(storyName, bypass ? "bypass" : "normal");
 
-  // Resolve provider from stored .story.json. Absent ⇒ Claude (no migration).
-  const provider: AgentProvider = isNewStory
-    ? "claude"
-    : readStoryMeta(storyDir).agentProvider ?? "claude";
+  // Resolve effective provider (see resolveProvider for the trust model). For a
+  // brand-new _new_ session the client flag is trusted; existing stories ignore
+  // it and read from session state then stored .story.json (no migration).
+  const provider: AgentProvider = resolveProvider({
+    isNewStory,
+    optProvider: opts?.provider,
+    sessionProvider: agentProviderBySession.get(storyName),
+    storedProvider: isNewStory ? undefined : readStoryMeta(storyDir).agentProvider,
+  });
+  agentProviderBySession.set(storyName, provider);
 
   // Determine resume id (accepts both legacy-string and record shapes).
   const sessionMap = loadSessionMap();
@@ -307,9 +358,10 @@ function spawnPty(storyName: string, opts?: { sessionId?: string; resume?: boole
 
 /** POST /api/terminal/spawn — spawn Claude CLI for a story */
 terminal.post("/spawn", async (c) => {
-  const body = await c.req.json<{ storyName?: string; resume?: boolean }>().catch(() => ({}));
+  const body = await c.req.json<{ storyName?: string; resume?: boolean; provider?: "claude" | "codex" }>().catch(() => ({}));
   const storyName = safeName(body.storyName || "default");
   if (!storyName) return c.json({ error: "Invalid story name" }, 400);
+  const optProvider = body.provider === "claude" || body.provider === "codex" ? body.provider : undefined;
 
   const existing = ptySessions.get(storyName);
   if (existing?.term && existing.state === "running") {
@@ -323,7 +375,7 @@ terminal.post("/spawn", async (c) => {
   }
 
   try {
-    const session = spawnPty(storyName, { resume: body.resume });
+    const session = spawnPty(storyName, { resume: body.resume, provider: optProvider });
     return c.json({ ok: true, pid: session.term.pid, storyName, sessionId: session.sessionId });
   } catch (err: unknown) {
     const message = err instanceof Error ? err.message : "Failed to spawn PTY";
@@ -412,10 +464,21 @@ terminal.post("/rename", async (c) => {
     agentModeBySession.delete(oldName);
   }
 
-  // Update persisted session map: remove old key, store under new key
+  // Carry the in-memory agent provider across the rename too (mirrors mode).
+  const oldProvider = agentProviderBySession.get(oldName);
+  if (oldProvider) {
+    agentProviderBySession.set(newName, oldProvider);
+    agentProviderBySession.delete(oldName);
+  }
+
+  // Update persisted session map: carry the stored value across the rename so a
+  // provider-aware Codex record (`{provider,sessionId,...}`) or a legacy Claude
+  // string is PRESERVED, not flattened to the live PTY's fallback UUID. Writing
+  // session.sessionId here corrupted Codex metadata (the fresh-spawn fallback
+  // UUID is not a real Codex session id, so later resume built `codex resume
+  // <uuid>` instead of `codex resume --last`).
   const sessionMap = loadSessionMap();
-  delete sessionMap[oldName];
-  sessionMap[newName] = session.sessionId;
+  carrySessionAcrossRename(sessionMap, oldName, newName, session.sessionId);
   saveSessionMap(sessionMap);
 
   return c.json({ ok: true, sessionId: session.sessionId });
@@ -449,7 +512,7 @@ terminal.get("/status", (c) => {
  * Attach a raw WebSocket to a story's PTY session.
  * Called from server.ts WebSocket upgrade handler.
  */
-export function attachTerminalWs(ws: WebSocket, storyName?: string, resume?: boolean, bypass?: boolean) {
+export function attachTerminalWs(ws: WebSocket, storyName?: string, resume?: boolean, bypass?: boolean, provider?: "claude" | "codex") {
   const name = storyName && safeName(storyName) ? storyName : "default";
   let session = ptySessions.get(name);
 
@@ -463,7 +526,7 @@ export function attachTerminalWs(ws: WebSocket, storyName?: string, resume?: boo
     }
 
     try {
-      session = spawnPty(name, { resume, bypass });
+      session = spawnPty(name, { resume, bypass, provider });
     } catch (err) {
       console.error("PTY spawn failed:", err);
       ws.close(1011, "pty-spawn-failed");

app/server.ts

@@ -160,8 +160,15 @@ async function start() {
         const story = url.searchParams.get("story") || undefined;
         const resume = url.searchParams.get("resume") === "true";
         const bypass = url.searchParams.get("bypass") === "true";
+        const provider = url.searchParams.get("provider");
         wss.handleUpgrade(req, socket, head, (ws) => {
-          attachTerminalWs(ws as unknown as WebSocket, story, resume, bypass);
+          attachTerminalWs(
+            ws as unknown as WebSocket,
+            story,
+            resume,
+            bypass,
+            provider === "claude" || provider === "codex" ? provider : undefined,
+          );
         });
       }).catch(() => socket.destroy());
     }