From 5421add83b2cca97e400ee02ffbb2e1c76191a5a Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 14 Aug 2026 01:33:06 +0000 Subject: [PATCH] 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();