diff --git a/packages/engine/src/claude-cli-engine.ts b/packages/engine/src/claude-cli-engine.ts index 4f04c4f..ec54cbd 100644 --- a/packages/engine/src/claude-cli-engine.ts +++ b/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" }); diff --git a/packages/engine/test/claude-cli-rollover.e2e.test.ts b/packages/engine/test/claude-cli-rollover.e2e.test.ts index a65f7b5..e517db9 100644 --- a/packages/engine/test/claude-cli-rollover.e2e.test.ts +++ b/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; + } + 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(); + } + }); });