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
This commit is contained in:
@@ -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://<id>`. Returns true when the corpse was
|
||||
* unregistered.
|
||||
*/
|
||||
reclaimDeadCorpse(id: string, expected: AgentRef): boolean {
|
||||
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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in New Issue
Block a user