From 5421add83b2cca97e400ee02ffbb2e1c76191a5a Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:33:06 +0000 Subject: [PATCH 1/2] fix(coding-agent): reclaim dead parked agent corpses on respawn A parked, session-less agent-registry entry with no reviver permanently poisoned its agent id for the process lifetime. `registerIfAvailable(input, null)` refuses any existing entry, so a fresh subagent spawn reusing the id died post-registration with "already owned by another session generation", and messages to it failed with "is parked and cannot be revived". Such corpses are left by an isolated run's park or an interrupted construction, and there was no reclaim or GC path. Add `AgentLifecycleManager.reclaimDeadCorpse`, which unregisters a ref only when it is parked, holds no live session, is not owned by the manager (no in-memory reviver) and has no park/revive in flight. The fresh-spawn path in `createAgentSession` calls it on a collision and retries registration, so the id becomes reusable. The registry's strict CAS contract is untouched; the corpse's transcript stays readable at history://. Fixes #8490 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/registry/agent-lifecycle.ts | 20 +++++++ packages/coding-agent/src/sdk.ts | 14 +++++ .../test/registry/agent-lifecycle.test.ts | 54 +++++++++++++++++++ 4 files changed, 92 insertions(+) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 38f921d82..ff87470c4 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed a parked, session-less agent-registry entry with no reviver permanently poisoning its agent id for the process lifetime: a fresh subagent spawn reusing that id died post-registration with `already owned by another session generation`, and messages to it failed with `is parked and cannot be revived`. Such dead corpses (left by an isolated run's park or an interrupted construction) are now reclaimed on a fresh-spawn collision so the id becomes reusable; the corpse's transcript remains readable at `history://` ([#8490](https://github.com/can1357/oh-my-pi/issues/8490)). + ## [17.3.2] - 2026-08-13 ### Fixed diff --git a/packages/coding-agent/src/registry/agent-lifecycle.ts b/packages/coding-agent/src/registry/agent-lifecycle.ts index cdc0116a5..1cdbf51c5 100644 --- a/packages/coding-agent/src/registry/agent-lifecycle.ts +++ b/packages/coding-agent/src/registry/agent-lifecycle.ts @@ -174,6 +174,26 @@ 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). + * + * 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 { + 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; + return this.#registry.unregister(id, ref); + } + /** * True when this manager owns `registry` — i.e. its adopt/park/revive state * describes that registry's refs. Lets a caller holding a specific registry diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index 65f3f5acd..c5e410c39 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -3034,6 +3034,20 @@ async function createAgentSessionScoped(options: CreateAgentSessionOptions): Pro options.expectedAgentRef === undefined ? agentRegistry.register(registrationInput) : agentRegistry.registerIfAvailable(registrationInput, options.expectedAgentRef); + if (!registeredAgentRef && options.expectedAgentRef === null) { + // A fresh spawn collided with an existing id. If that id is held by a + // provably-dead parked corpse — no live session, no reviver — reclaim it + // so this new generation can take the id instead of failing forever at + // construction. Without this, one such corpse (isolated-run park, + // interrupted construction) poisons the id for the whole process (#8490). + // The reclaim is gated by the lifecycle owner and only touches the + // 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)) { + registeredAgentRef = agentRegistry.registerIfAvailable(registrationInput, null); + } + } if (!registeredAgentRef) { throw new Error(`Agent "${resolvedAgentId}" is already owned by another session generation.`); } diff --git a/packages/coding-agent/test/registry/agent-lifecycle.test.ts b/packages/coding-agent/test/registry/agent-lifecycle.test.ts index 611e79de4..95fe8b8c8 100644 --- a/packages/coding-agent/test/registry/agent-lifecycle.test.ts +++ b/packages/coding-agent/test/registry/agent-lifecycle.test.ts @@ -145,6 +145,60 @@ describe("AgentLifecycleManager", () => { expect(ref?.sessionFile).toBe("/tmp/3-Sub.jsonl"); }); + it("reclaimDeadCorpse frees a parked, session-less, unadopted id and refuses live/adopted refs (#8490)", async () => { + // A corpse: registered running, then parked with no session and no adoption + // (the isolated-run finalize / interrupted-construction outcome). It cannot + // be revived and would otherwise poison its id for the process lifetime. + const corpse = registry.register({ + id: "Corpse-Sub", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Corpse-Sub.jsonl", + status: "running", + }); + registry.setStatus("Corpse-Sub", "parked", corpse); + await expect(lifecycle.ensureLive("Corpse-Sub")).rejects.toThrow(/parked and cannot be revived/); + + // A live ref is never reclaimed. + const live = makeSessionStub(); + registry.register({ + id: "Live-Sub", + displayName: "task", + kind: "sub", + session: live.session, + status: "running", + }); + expect(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. + const adopted = registry.register({ + id: "Adopted-Sub", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Adopted-Sub.jsonl", + status: "parked", + }); + lifecycle.adopt("Adopted-Sub", { idleTtlMs: 0, revive: async () => makeSessionStub().session }, adopted); + expect(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); + + // The corpse is reclaimed, and its id becomes registerable again. + expect(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" }, + null, + ); + expect(respawn?.status).toBe("running"); + expect(registry.get("Corpse-Sub")).toBe(respawn); + }); + it("concurrent ensureLive calls during a slow revive coalesce into one reviver run", async () => { const gate = deferred(); const revived = makeSessionStub(); From 2929670a252cef65fb6cbfacb793429d574eeec6 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:40:37 +0000 Subject: [PATCH 2/2] fix(coding-agent): preserved cold-revivable parked agents Consulted the persisted subagent reviver factory before reclaiming an unadopted parked ref. A valid cold reviver now preserves the existing agent; factory failures also fail closed rather than deleting a potentially recoverable generation. Reclaim invariants are rechecked after the async probe to close lifecycle races. Added regression coverage proving an unadopted disk-restored ref remains registered and messageable through ensureLive. Fixes #8490 --- .../src/registry/agent-lifecycle.ts | 28 +++++++++++---- packages/coding-agent/src/sdk.ts | 2 +- .../test/registry/agent-lifecycle.test.ts | 34 ++++++++++++++++--- 3 files changed, 52 insertions(+), 12 deletions(-) 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();