From 9f2d2781da6168ef2d91e6c5f13231e6be448b4a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 31 Jul 2026 03:57:16 -0700 Subject: [PATCH] fix(agents): preserve resumed turns after stale terminal events (#116777) --- config/env-var-count-budget.txt | 2 +- src/agents/main-session-recovery-lifecycle.ts | 12 ++++++ ...ain-session-recovery-run-ownership.test.ts | 32 +++++++++++++-- src/gateway/session-lifecycle-state.test.ts | 39 +++++++++++++++++++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/config/env-var-count-budget.txt b/config/env-var-count-budget.txt index e2c3614de3ce..509777186d26 100644 --- a/config/env-var-count-budget.txt +++ b/config/env-var-count-budget.txt @@ -1,3 +1,3 @@ # Distinct OPENCLAW_* names in production source under src, packages, and extensions. # Ratchet: lower this number when cleanup removes names; never raise it. -518 +517 diff --git a/src/agents/main-session-recovery-lifecycle.ts b/src/agents/main-session-recovery-lifecycle.ts index 5fda42d1e9d3..cd3fd3ddf70a 100644 --- a/src/agents/main-session-recovery-lifecycle.ts +++ b/src/agents/main-session-recovery-lifecycle.ts @@ -113,6 +113,18 @@ export function projectMainSessionRecoveryLifecycle(params: { ) : runs; if (settlesRecovery) { + if ( + matchesFence && + lifecycleGeneration !== params.currentLifecycleGeneration && + remaining?.some( + (run) => + run.runId === runId && run.lifecycleGeneration === params.currentLifecycleGeneration, + ) + ) { + // Older generations share the live owner's run id. Consume only their + // fence; recording that id as terminal would also tombstone its replacement. + return { action: "apply", patch: { restartRecoveryRuns: remaining } }; + } const foregroundClaims = params.entry?.mainRestartRecovery?.foregroundClaims; const foregroundOwnerClaimId = runId && diff --git a/src/agents/main-session-recovery-run-ownership.test.ts b/src/agents/main-session-recovery-run-ownership.test.ts index e41f6d77adf7..b4a2f947f2a5 100644 --- a/src/agents/main-session-recovery-run-ownership.test.ts +++ b/src/agents/main-session-recovery-run-ownership.test.ts @@ -2,12 +2,16 @@ import { describe, expect, it } from "vitest"; import type { InternalSessionEntry as SessionEntry } from "../config/sessions.js"; import { projectMainSessionRecoveryLifecycle } from "./main-session-recovery-lifecycle.js"; -function recoveryEntry(params?: { hasCurrentOwner?: boolean }): SessionEntry { +function recoveryEntry(params?: { + hasCurrentOwner?: boolean; + ownsDelivery?: boolean; +}): SessionEntry { return { sessionId: "session-1", updatedAt: 100, status: "running", abortedLastRun: false, + ...(params?.ownsDelivery ? { restartRecoveryDeliveryRunId: "recovery" } : {}), restartRecoveryRuns: [ { runId: "recovery", lifecycleGeneration: "generation-old" }, { runId: "recovery", lifecycleGeneration: "generation-current" }, @@ -52,11 +56,14 @@ describe("main-session recovery run ownership", () => { }); }); - it("does not let an older same-id terminal settle its replacement generation", () => { + it.each([ + { name: "admitted delivery owner", params: { ownsDelivery: true } }, + { name: "foreground owner", params: { hasCurrentOwner: true, ownsDelivery: true } }, + ])("does not let an older same-id terminal settle or tombstone its $name", ({ params }) => { expect( projectMainSessionRecoveryLifecycle({ currentLifecycleGeneration: "generation-current", - entry: recoveryEntry({ hasCurrentOwner: true }), + entry: recoveryEntry(params), event: { runId: "recovery", lifecycleGeneration: "generation-old", @@ -68,8 +75,25 @@ describe("main-session recovery run ownership", () => { action: "apply", patch: { restartRecoveryRuns: [{ runId: "recovery", lifecycleGeneration: "generation-current" }], - restartRecoveryTerminalRunIds: ["recovery"], }, }); }); + + it("suppresses a duplicate older terminal after its fence has been consumed", () => { + const entry = recoveryEntry({ ownsDelivery: true }); + entry.restartRecoveryRuns = [{ runId: "recovery", lifecycleGeneration: "generation-current" }]; + + expect( + projectMainSessionRecoveryLifecycle({ + currentLifecycleGeneration: "generation-current", + entry, + event: { + runId: "recovery", + lifecycleGeneration: "generation-old", + data: { phase: "end" }, + }, + snapshotPatch: { status: "done", abortedLastRun: false }, + }), + ).toEqual({ action: "suppress" }); + }); }); diff --git a/src/gateway/session-lifecycle-state.test.ts b/src/gateway/session-lifecycle-state.test.ts index 8c40dfbb691f..c47958d24738 100644 --- a/src/gateway/session-lifecycle-state.test.ts +++ b/src/gateway/session-lifecycle-state.test.ts @@ -434,6 +434,45 @@ describe("session lifecycle state", () => { expect(persisted.mainRestartRecovery).toBeUndefined(); }); + it("keeps an active recovery when an older same-run terminal arrives", async () => { + const lifecycleGeneration = getAgentEventLifecycleGeneration(); + const persisted = await persistLifecycle( + { + sessionId: "session-id", + updatedAt: 1_000, + startedAt: 1_050, + status: "running", + abortedLastRun: false, + restartRecoveryDeliveryRunId: "recovery-run", + restartRecoveryRuns: [ + { runId: "recovery-run", lifecycleGeneration: "pre-restart" }, + { runId: "recovery-run", lifecycleGeneration }, + ], + mainRestartRecovery: { + cycleId: "cycle-1", + revision: 5, + chargedAttempts: 2, + }, + }, + { + ts: 2_000, + sessionId: "session-id", + runId: "recovery-run", + lifecycleGeneration: "pre-restart", + data: { phase: "end", endedAt: 1_800 }, + }, + ); + + expect(persisted).toMatchObject({ + status: "running", + abortedLastRun: false, + restartRecoveryDeliveryRunId: "recovery-run", + restartRecoveryRuns: [{ runId: "recovery-run", lifecycleGeneration }], + mainRestartRecovery: { cycleId: "cycle-1" }, + }); + expect(persisted.restartRecoveryTerminalRunIds).toBeUndefined(); + }); + it("does not settle a foreground owner from a stale lifecycle generation", async () => { const persisted = await persistLifecycle( {