diff --git a/packages/coding-agent/src/registry/agent-lifecycle.ts b/packages/coding-agent/src/registry/agent-lifecycle.ts index 1cdbf51c5..0f5606990 100644 --- a/packages/coding-agent/src/registry/agent-lifecycle.ts +++ b/packages/coding-agent/src/registry/agent-lifecycle.ts @@ -176,21 +176,35 @@ export class AgentLifecycleManager { /** * Reclaim a provably-dead parked corpse so a fresh spawn can reuse its id. - * A ref qualifies only when it still resolves to `expected`, is `parked` - * with no live session, this manager does not own it (no in-memory reviver - * adoption), and no park/revive is in flight. Such a ref cannot be revived - * — {@link ensureLive} throws for it — yet {@link AgentRegistry.registerIfAvailable} - * refuses to overwrite it, so one construction failure or isolated-run park - * would otherwise poison the id for the whole process (#8490). + * Refuses live, adopted, in-flight, or cold-revivable refs. For a parked ref + * restored from disk, the persisted factory is consulted before removal + * because cold revivers are created lazily by {@link ensureLive}. * * Only refs in the registry this manager owns are touched; the transcript * stays readable at `history://`. Returns true when the corpse was * unregistered. */ - reclaimDeadCorpse(id: string, expected: AgentRef): boolean { + async reclaimDeadCorpse(id: string, expected: AgentRef): Promise { const ref = this.#registry.get(id); if (ref !== expected || ref.status !== "parked" || ref.session) return false; if (this.#adopted.has(id) || this.#parks.has(id) || this.#revivals.has(id)) return false; + + const persistedFactory = ref.sessionFile ? this.#persistedReviverFactory : undefined; + if (persistedFactory) { + try { + if (await persistedFactory(ref)) return false; + } catch (error) { + logger.warn("AgentLifecycleManager.reclaimDeadCorpse: persisted reviver probe failed", { + id, + error: error instanceof Error ? error.message : String(error), + }); + return false; + } + // The factory awaited I/O; another lifecycle operation may now own or + // have replaced this ref. Revalidate every reclaim invariant. + if (this.#registry.get(id) !== ref || ref.status !== "parked" || ref.session) return false; + if (this.#adopted.has(id) || this.#parks.has(id) || this.#revivals.has(id)) return false; + } return this.#registry.unregister(id, ref); } diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index c5e410c39..0df643414 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -3044,7 +3044,7 @@ async function createAgentSessionScoped(options: CreateAgentSessionOptions): Pro // registry it manages; the corpse's transcript stays at history://. const stale = agentRegistry.get(resolvedAgentId); const lifecycle = AgentLifecycleManager.global(); - if (stale && lifecycle.manages(agentRegistry) && lifecycle.reclaimDeadCorpse(resolvedAgentId, stale)) { + if (stale && lifecycle.manages(agentRegistry) && (await lifecycle.reclaimDeadCorpse(resolvedAgentId, stale))) { registeredAgentRef = agentRegistry.registerIfAvailable(registrationInput, null); } } diff --git a/packages/coding-agent/test/registry/agent-lifecycle.test.ts b/packages/coding-agent/test/registry/agent-lifecycle.test.ts index 95fe8b8c8..0f5d12619 100644 --- a/packages/coding-agent/test/registry/agent-lifecycle.test.ts +++ b/packages/coding-agent/test/registry/agent-lifecycle.test.ts @@ -169,7 +169,7 @@ describe("AgentLifecycleManager", () => { session: live.session, status: "running", }); - expect(lifecycle.reclaimDeadCorpse("Live-Sub", registry.get("Live-Sub")!)).toBe(false); + expect(await lifecycle.reclaimDeadCorpse("Live-Sub", registry.get("Live-Sub")!)).toBe(false); expect(registry.get("Live-Sub")?.session).toBe(live.session); // An adopted (revivable) parked agent is never reclaimed. @@ -182,14 +182,14 @@ describe("AgentLifecycleManager", () => { status: "parked", }); lifecycle.adopt("Adopted-Sub", { idleTtlMs: 0, revive: async () => makeSessionStub().session }, adopted); - expect(lifecycle.reclaimDeadCorpse("Adopted-Sub", adopted)).toBe(false); + expect(await lifecycle.reclaimDeadCorpse("Adopted-Sub", adopted)).toBe(false); expect(registry.get("Adopted-Sub")).toBe(adopted); // A stale expected ref (points at a different agent) is never reclaimed. - expect(lifecycle.reclaimDeadCorpse("Corpse-Sub", adopted)).toBe(false); + expect(await lifecycle.reclaimDeadCorpse("Corpse-Sub", adopted)).toBe(false); // The corpse is reclaimed, and its id becomes registerable again. - expect(lifecycle.reclaimDeadCorpse("Corpse-Sub", corpse)).toBe(true); + expect(await lifecycle.reclaimDeadCorpse("Corpse-Sub", corpse)).toBe(true); expect(registry.get("Corpse-Sub")).toBeUndefined(); const respawn = registry.registerIfAvailable( { id: "Corpse-Sub", displayName: "task", kind: "sub", session: null, status: "running" }, @@ -199,6 +199,32 @@ describe("AgentLifecycleManager", () => { expect(registry.get("Corpse-Sub")).toBe(respawn); }); + it("reclaimDeadCorpse preserves an unadopted parked ref when its persisted session can cold-revive", async () => { + const revived = makeSessionStub(); + const cold = registry.register({ + id: "Cold-Sub", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Cold-Sub.jsonl", + status: "parked", + }); + let factoryCalls = 0; + lifecycle.setPersistedSubagentReviverFactory(async ref => { + factoryCalls++; + expect(ref).toBe(cold); + return async () => revived.session; + }, 0); + + expect(await lifecycle.reclaimDeadCorpse("Cold-Sub", cold)).toBe(false); + expect(registry.get("Cold-Sub")).toBe(cold); + expect(factoryCalls).toBe(1); + + // The preserved ref remains messageable through the normal cold-revive path. + expect(await lifecycle.ensureLive("Cold-Sub")).toBe(revived.session); + expect(registry.get("Cold-Sub")?.session).toBe(revived.session); + }); + it("concurrent ensureLive calls during a slow revive coalesce into one reviver run", async () => { const gate = deferred(); const revived = makeSessionStub();