diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index e059aed75..7c8372e4f 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -111,6 +111,9 @@ - Automatically continued Gemini turns that stopped after thinking without final output, using a bounded final-answer reminder instead of exhausting generic retries. - Retried Gemini `MALFORMED_FUNCTION_CALL` failures when every emitted tool call was proven unexecuted, while preserving real tool-result and visible-output replay guards. - Kept current terminal retry errors in one pinned banner with attempt context while surfacing local continuation failures instead of stale provider errors. +### 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 diff --git a/packages/coding-agent/src/registry/agent-lifecycle.ts b/packages/coding-agent/src/registry/agent-lifecycle.ts index cdc0116a5..0f5606990 100644 --- a/packages/coding-agent/src/registry/agent-lifecycle.ts +++ b/packages/coding-agent/src/registry/agent-lifecycle.ts @@ -174,6 +174,40 @@ export class AgentLifecycleManager { ); } + /** + * Reclaim a provably-dead parked corpse so a fresh spawn can reuse its id. + * 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. + */ + 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); + } + /** * 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 7ad294893..6dcd83283 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -3037,6 +3037,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) && (await 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..0f5d12619 100644 --- a/packages/coding-agent/test/registry/agent-lifecycle.test.ts +++ b/packages/coding-agent/test/registry/agent-lifecycle.test.ts @@ -145,6 +145,86 @@ 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(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. + 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(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(await lifecycle.reclaimDeadCorpse("Corpse-Sub", adopted)).toBe(false); + + // The corpse is reclaimed, and its id becomes registerable again. + 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" }, + null, + ); + expect(respawn?.status).toBe("running"); + 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();