Resolve the resumed run's turnsUsed and mcpServers once at the resume boundary

Reading resumedState?.turnsUsed and resumedState?.mcpServers separately
at each of the three sites that needed them meant the empty/zero default
was decided three times instead of once. resolveResumeSeed folds a
picked session's run.json into a single concrete seed right where it is
loaded, so createRunSink, connectedMcpServers, and the immediate
post-resume saveState all read a trusted value with no further
defaulting.

Documents why runner.ts's finalized flag still earns its place now that
saveState serializes writes per session: the write chain only orders
writes that are already issued, it has no way to know a stale
post-finalize snapshot shouldn't be issued at all.

Author: TheGreatAxios

src/tui/resume-seed.test.ts

@@ -0,0 +1,41 @@
+import { describe, test, expect } from "bun:test";
+import { resolveResumeSeed } from "./runner.js";
+import type { RunState } from "../session/state.js";
+
+function pickedState(overrides: Partial<RunState>): RunState {
+  return {
+    status: "running",
+    turnsUsed: 0,
+    task: "task",
+    startedAt: 1,
+    ...overrides,
+  };
+}
+
+describe("resolveResumeSeed", () => {
+  test("a fresh (non-resumed) run seeds zero turns and no servers", () => {
+    expect(resolveResumeSeed(null)).toEqual({ turnsUsed: 0, mcpServers: [] });
+  });
+
+  test("carries forward a resumed session's non-zero turnsUsed and non-empty mcpServers", () => {
+    const seed = resolveResumeSeed(
+      pickedState({
+        turnsUsed: 12,
+        mcpServers: [{ name: "filesystem", toolCount: 5 }, { name: "search", toolCount: 2 }],
+      }),
+    );
+
+    expect(seed.turnsUsed).toBe(12);
+    expect(seed.mcpServers).toEqual([
+      { name: "filesystem", toolCount: 5 },
+      { name: "search", toolCount: 2 },
+    ]);
+  });
+
+  test("defaults mcpServers to empty when a resumed record predates that field", () => {
+    const seed = resolveResumeSeed(pickedState({ turnsUsed: 3 }));
+
+    expect(seed.turnsUsed).toBe(3);
+    expect(seed.mcpServers).toEqual([]);
+  });
+});

src/tui/runner.ts

@@ -211,6 +211,29 @@ export function resumeTranscriptLoadErrorBlock(err: unknown): {
   return { type: "error", message: `Could not load prior session transcript: ${message}` };
 }
 
+export type ResumeSeed = {
+  turnsUsed: number;
+  mcpServers: ConnectedMcpServer[];
+};
+
+const FRESH_RESUME_SEED: ResumeSeed = { turnsUsed: 0, mcpServers: [] };
+
+/**
+ * Fold a resumed session's run.json into a concrete seed once, at the
+ * resume boundary, so every downstream reader (the run sink, the
+ * connected-servers list, the immediate post-resume saveState) trusts a
+ * fully-populated value instead of each repeating its own `?? 0` / `?? []`
+ * default. A fresh (non-resumed) run gets the same shape via
+ * FRESH_RESUME_SEED, so callers never branch on "was this a resume."
+ */
+export function resolveResumeSeed(pickedState: RunState | null): ResumeSeed {
+  if (pickedState === null) return FRESH_RESUME_SEED;
+  return {
+    turnsUsed: pickedState.turnsUsed,
+    mcpServers: pickedState.mcpServers ?? [],
+  };
+}
+
 const GRANT_SCOPE_LABEL: Record<GrantScope, string> = {
   session: "This session",
   project: "This project",
@@ -406,17 +429,17 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   let resumeSkipInitialTask = config.skipInitialTask === true;
   let startedAt = Date.now();
   let runTaskTitle = config.task;
-  // Carries the resumed session's prior run.json forward so turnsUsed and
-  // mcpServers can be seeded instead of silently reset to zero/empty.
-  let resumedState: RunState | null = null;
+  // Resolved once at the resume boundary so turnsUsed/mcpServers reads
+  // downstream never repeat their own omission-handling default.
+  let resumeSeed: ResumeSeed = FRESH_RESUME_SEED;
 
   if (config.resumePicker) {
     const picked = await pickSession(config.cwd, { includeCompleted: config.force });
     if (picked === null) return 0;
     sessionId = picked.sessionId;
     resumeSkipInitialTask = true;
     const pickedState = await loadState(config.cwd, sessionId);
-    resumedState = pickedState;
+    resumeSeed = resolveResumeSeed(pickedState);
     if (pickedState !== null) {
       startedAt = pickedState.startedAt;
       runTaskTitle = pickedState.task;
@@ -439,20 +462,28 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   // run.json at all.
   await saveState(config.cwd, sessionId, {
     status: "running",
-    turnsUsed: resumedState?.turnsUsed ?? 0,
+    turnsUsed: resumeSeed.turnsUsed,
     task: runTaskTitle.trim().length > 0 ? runTaskTitle.trim() : "(conversation)",
     startedAt,
     model: `${config.providerName}:${config.model}`,
-    mcpServers: resumedState?.mcpServers ?? [],
+    mcpServers: resumeSeed.mcpServers,
   });
 
   // Crash guard: if anything from setup onward throws all the way out of
   // runTUI instead of reaching the normal finalize block, this still closes
   // out run.json so status and finishedAt never disagree. Declared before the
   // try so every fallible step after the minimal write above is covered.
   // `finalized` is set by the normal finalize path so this never double-writes
-  // on a clean exit; it also gates straggler snapshot writes (see
-  // persistRunSnapshot) from resurrecting a closed record.
+  // on a clean exit. It also gates persistRunSnapshot (below) from *issuing*
+  // a straggler write at all once the run is closed — a different job from
+  // saveState's per-session write ordering in state.ts. That ordering only
+  // decides which already-issued write lands last; it has no way to know a
+  // "running" snapshot fired after finalize is stale and should never be
+  // written in the first place. Without this flag such a snapshot would
+  // still queue behind the terminal write and legitimately "win" the
+  // ordering, resurrecting a closed run.json. Two different constraints
+  // (don't issue a stale write vs. order the writes you do issue), each
+  // owned by its own layer — not a duplicate check.
   let finalized = false;
   // Bound after the cycle recorder exists (it needs the session workdir); the
   // crash guard is declared first so it covers every fallible step below.
@@ -1309,7 +1340,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
   const runSink = createRunSink({
     emitter,
     hookManager,
-    ...(resumedState !== null ? { initialTurnCount: resumedState.turnsUsed } : {}),
+    initialTurnCount: resumeSeed.turnsUsed,
     onTurnComplete: (ctx) => {
       // provider_id is the canonical provider kind, never ctx.source.sourceId:
       // sourceId is the user-typed label from onboarding/settings, and free
@@ -1329,7 +1360,7 @@ export async function runTUI(initialConfig: Config): Promise<number> {
 
   // MCP servers connected so far, keyed by name so a reconnect after a failure
   // replaces rather than duplicates the entry.
-  let connectedMcpServers: ConnectedMcpServer[] = resumedState?.mcpServers ?? [];
+  let connectedMcpServers: ConnectedMcpServer[] = resumeSeed.mcpServers;
   // Every configured server's latest state, for the /mcp surface. Unlike
   // `connectedMcpServers` (persisted run metadata) this keeps the ones that
   // failed or are still waiting on authorization.