Closes #173

packages/engine/src/claude-cli-engine.ts

@@ -506,19 +506,38 @@ export class ClaudeCliEngine implements TranslationEngine {
       // stdin may already be closed.
     }
     current.kill();
-    // Bank the retired session's cost so the reported cumulative can't regress
-    // when the fresh session's total_cost_usd restarts near 0 (#136/#24).
+    // Snapshot the pre-rollover identity + cost so a FAILED fresh spawn rolls the
+    // WHOLE transition back atomically (#173). Committing the new id + banked cost
+    // before the spawn is confirmed would otherwise strand recovery: sessionId/
+    // resumeId would point at a conversation that never existed (the next respawn
+    // would `--resume` that phantom id for the rest of the meeting), and the
+    // retired session's cost would stay banked — double-counting it if recovery
+    // then resumed that session.
+    const prevSessionId = this.sessionId;
+    const prevResumeId = this.resumeId;
+    const prevRolledOverCostUsd = this.rolledOverCostUsd;
+    const prevSessionCumulativeCostUsd = this.sessionCumulativeCostUsd;
+    // Provisionally commit: bank the retired session's cost so the reported
+    // cumulative can't regress when the fresh session's total_cost_usd restarts
+    // near 0 (#136/#24), and mint the fresh conversation id (history resets;
+    // resumeId follows so a later crash resumes THIS session). spawnSession spawns
+    // with `--session-id this.sessionId`, so the fresh id must be live before it.
     this.rolledOverCostUsd += this.sessionCumulativeCostUsd;
     this.sessionCumulativeCostUsd = 0;
-    // Fresh conversation id — history resets. resumeId follows it so a later
-    // crash resumes THIS session, not the abandoned one.
     this.sessionId = randomUUID();
     this.resumeId = this.sessionId;
     let child: ChildProcessWithoutNullStreams;
     try {
       child = await this.spawnSession(undefined); // fresh — no --resume
     } catch {
+      // The fresh session never spawned: roll the transition back so the next
+      // runTurn's respawn resumes the still-valid retired session (--resume the
+      // prior id) with consistent, non-duplicated accounting — not a phantom id.
       // spawnSession recorded the error status/detail; caller throws not-started.
+      this.sessionId = prevSessionId;
+      this.resumeId = prevResumeId;
+      this.rolledOverCostUsd = prevRolledOverCostUsd;
+      this.sessionCumulativeCostUsd = prevSessionCumulativeCostUsd;
       return null;
     }
     this.emitHealthEvent({ kind: "rolledOver" });

packages/engine/test/claude-cli-rollover.e2e.test.ts

@@ -6,7 +6,10 @@
 //
 // Real spawn/stdio via fake-cli echo mode; LIVECAP_FAKE_CACHE_READ drives the
 // per-turn cache-read the threshold reacts to.
+import { randomUUID } from "node:crypto";
+import { readFileSync } from "node:fs";
 import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { describe, it, expect } from "vitest";
 
@@ -103,4 +106,87 @@ describe("ClaudeCliEngine — session rollover (#136)", () => {
       await engine.stop();
     }
   });
+
+  // #173: performRollover mints a fresh id + banks the retired session's cost
+  // BEFORE the fresh `claude -p` is confirmed. If that spawn fails (ENOENT /
+  // resource exhaustion — plausible exactly under the long-session load that
+  // triggers rollover), the old code left sessionId/resumeId pointing at a
+  // conversation that never existed, so every later recovery `--resume`d that
+  // phantom id forever, AND the retired session's cost stayed banked (double-
+  // counted if recovery resumed it). The fix rolls the WHOLE transition back.
+  it("rolls a failed-spawn rollover back so recovery resumes the prior session with non-duplicated cost (#173)", async () => {
+    const ORIG = "00000000-0000-4000-8000-0000000000aa";
+    const argvOut = join(tmpdir(), `livecap-argv-${randomUUID()}.json`);
+    const engine = new ClaudeCliEngine({
+      bin: FAKE_CLI,
+      cwd: tmpdir(),
+      env: {
+        ...process.env,
+        LIVECAP_FAKE_ECHO: "1",
+        LIVECAP_FAKE_CACHE_READ: "500000", // >= threshold → arms a rollover each turn
+        LIVECAP_FAKE_ARGV_OUT: argvOut,
+      },
+      includePartialMessages: false,
+      rolloverAfterCacheReadTokens: 100_000,
+      sessionId: ORIG,
+      continuitySeed: () => "The Fed held rates.",
+    });
+
+    // Inject a one-shot FRESH-spawn failure to model a rollover whose `claude -p`
+    // cannot spawn — the one failure mode a real fake-cli binary can't produce (a
+    // successfully-started process can never make the parent's spawn() emit
+    // 'error'). Only the rollover's fresh spawn (resume === undefined, once armed)
+    // is failed; start() and the recovery respawn take the real path.
+    interface SpawnSeam {
+      spawnSession(resume: string | undefined): Promise<unknown>;
+    }
+    const seam = engine as unknown as SpawnSeam;
+    const realSpawn = seam.spawnSession.bind(engine);
+    let failFreshSpawn = false;
+    seam.spawnSession = (resume) => {
+      if (failFreshSpawn && resume === undefined) {
+        failFreshSpawn = false;
+        return Promise.reject(new Error("simulated spawn failure (ENOENT)"));
+      }
+      return realSpawn(resume);
+    };
+
+    const usages: Usage[] = [];
+    engine.onUsage((u) => usages.push(u));
+    await engine.start();
+    try {
+      // Turn 1: runs on the ORIGINAL session and arms a rollover for turn 2.
+      await drain(engine);
+
+      // Turn 2: the armed rollover's fresh spawn fails → performRollover rolls the
+      // whole transition back → runTurn throws not-started for this turn.
+      failFreshSpawn = true;
+      await expect(drain(engine)).rejects.toThrow();
+
+      // Turn 3: recovery. The child is null, so runTurn respawns — and it must
+      // resume the PRIOR session id, never a phantom rollover id.
+      const t3 = await drain(engine);
+      expect(t3.at(-1)?.done).toBe(true);
+      expect(t3.at(-1)?.text.length).toBeGreaterThan(0); // translation recovers
+
+      // (a) Viable identity: the recovery respawn resumed the ORIGINAL session.
+      const argv = JSON.parse(readFileSync(argvOut, "utf8")) as string[];
+      const ri = argv.indexOf("--resume");
+      expect(ri).toBeGreaterThan(-1);
+      expect(argv[ri + 1]).toBe(ORIG); // the prior id — NOT a stranded phantom
+      expect(argv).not.toContain("--session-id"); // resumes, doesn't mint fresh
+
+      // (b) Cost monotonic AND non-duplicated: the retired session's 0.001 was
+      // banked then UN-banked by the rollback, and its cumulative restored, so the
+      // resumed session's repeat of total_cost_usd=0.001 nets a 0 delta — final
+      // cumulative is one session's 0.001, not the 0.002 a restore-ids-only fix
+      // (leaving the cost double-banked) would report.
+      for (let i = 1; i < usages.length; i += 1) {
+        expect(usages[i]!.cumulativeCostUsd).toBeGreaterThanOrEqual(usages[i - 1]!.cumulativeCostUsd);
+      }
+      expect(usages.at(-1)!.cumulativeCostUsd).toBeCloseTo(0.001, 6);
+    } finally {
+      await engine.stop();
+    }
+  });
 });