Merge PR #8492: fix(coding-agent): reclaim dead parked agent corpses on respawn (@roboomp)
This commit is contained in:
@@ -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://<id>` ([#8490](https://github.com/can1357/oh-my-pi/issues/8490)).
|
||||
|
||||
## [17.3.2] - 2026-08-13
|
||||
|
||||
|
||||
@@ -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://<id>`. Returns true when the corpse was
|
||||
* unregistered.
|
||||
*/
|
||||
async reclaimDeadCorpse(id: string, expected: AgentRef): Promise<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;
|
||||
|
||||
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
|
||||
|
||||
@@ -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.`);
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user