diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 28720e06d..105e9c218 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -106,6 +106,7 @@ - Fixed the CLI crashing at startup with a raw uncaught `AuthBrokerError` when a configured auth broker (`auth.broker.url` / `OMP_AUTH_BROKER_URL`) is unreachable and no fresh cached snapshot exists. Startup auth discovery now fails with an actionable message naming the broker URL and the recovery options (`omp auth-broker serve`, or resetting `auth.broker.url` / `auth.broker.token`) and exits non-zero, instead of dumping a stack trace ([#8096](https://github.com/can1357/oh-my-pi/issues/8096)). - Fixed OpenCode discovery ignoring `opencode.jsonc` files and rejecting comments in `opencode.json`, which omitted imported settings and MCP servers. ([#8104](https://github.com/can1357/oh-my-pi/issues/8104)) - Fixed the launch broker staying alive indefinitely after its last persistent daemon exited with no clients connected: the idle-shutdown timer that fired while the daemon was still live returned without rearming, and terminal settlement never scheduled another idle check, so the broker process, endpoint, timers, and record maps leaked. Terminal settlement now rearms idle shutdown, which re-checks clients, remaining live persistent daemons, and detached project presence before exiting ([#8110](https://github.com/can1357/oh-my-pi/issues/8110)). +- Fixed a cold persisted-agent revival racing with lifecycle teardown: a revive whose reviver factory or session resolved after `AgentLifecycleManager.dispose()` could attach a live session and arm a TTL on the disposed manager, leaking a session graph and timers past teardown. Late revivals now reject deterministically and dispose any session they built ([#8114](https://github.com/can1357/oh-my-pi/issues/8114)). ## [17.2.12] - 2026-08-08 diff --git a/packages/coding-agent/src/registry/agent-lifecycle.ts b/packages/coding-agent/src/registry/agent-lifecycle.ts index ae8dd13e6..cdc0116a5 100644 --- a/packages/coding-agent/src/registry/agent-lifecycle.ts +++ b/packages/coding-agent/src/registry/agent-lifecycle.ts @@ -127,6 +127,8 @@ export class AgentLifecycleManager { #persistedReviverFactory: PersistedSubagentReviverFactory | undefined; /** TTL applied when a cold-revived ref is adopted on demand. */ #persistedReviveTtlMs = 0; + /** Set once {@link dispose} runs; blocks late revivals from adopting into a torn-down manager. */ + #disposed = false; constructor(registry: AgentRegistry = AgentRegistry.global()) { this.#registry = registry; @@ -332,6 +334,14 @@ export class AgentLifecycleManager { let coldAdopted = false; if (!revive && ref.status === "parked" && ref.sessionFile && this.#persistedReviverFactory) { revive = await this.#persistedReviverFactory(ref); + // Teardown can complete during the factory await. A late cold revive must + // not cold-adopt (and later attach a live session + TTL) into a disposed + // manager — reject deterministically before creating any session. + if (this.#disposed) { + throw new Error( + `Agent "${id}" revival aborted: its lifecycle was disposed while its persisted session was being prepared.`, + ); + } if (revive) { adoption = { ref, idleTtlMs: this.#persistedReviveTtlMs, revive }; this.#adopted.set(id, adoption); @@ -411,9 +421,10 @@ export class AgentLifecycleManager { return true; } - /** Teardown everything (process exit / main session dispose). */ + /** Teardown everything; disposing the global manager makes its next owner a fresh instance. */ async dispose(deadlineAt: number = Date.now() + AGENT_RELEASE_GRACE_MS): Promise { this.#unsubscribe?.(); + this.#disposed = true; this.#unsubscribe = undefined; const ids = [...new Set([...this.#adopted.keys(), ...this.#parks.keys()])]; await Promise.all( @@ -435,10 +446,19 @@ export class AgentLifecycleManager { this.#revivals.clear(); this.#parks.clear(); this.#persistedReviverFactory = undefined; + if (AgentLifecycleManager.#global === this) AgentLifecycleManager.#global = undefined; } async #revive(id: string, revive: AgentReviver, ref: AgentRef, adopted: AdoptedAgent): Promise { const session = await revive(ref); + if (this.#disposed) { + // The owning lifecycle tore down while the reviver was in flight; dispose + // the freshly built session instead of attaching it, and fail the waiter. + await session.dispose(); + throw new Error( + `Agent "${id}" revival aborted: its lifecycle was disposed while its persisted session was reviving.`, + ); + } let liveRef = this.#registry.get(id); if (liveRef === ref && ref.status === "parked" && !ref.session) { // A simple reviver returned a session without claiming the parked ref; diff --git a/packages/coding-agent/test/registry/agent-lifecycle.test.ts b/packages/coding-agent/test/registry/agent-lifecycle.test.ts index 9f61c6de4..611e79de4 100644 --- a/packages/coding-agent/test/registry/agent-lifecycle.test.ts +++ b/packages/coding-agent/test/registry/agent-lifecycle.test.ts @@ -621,4 +621,96 @@ describe("AgentLifecycleManager", () => { await registerPersistedSubagents(restoredRegistry, rootSessionFile); expect(restoredRegistry.get(workerId)?.status).toBe("aborted"); }); + + it("a cold revive whose factory resolves after dispose rejects without adopting or arming a TTL", async () => { + vi.useFakeTimers(); + const gate = deferred(); + const revived = makeSessionStub(); + let reviverRuns = 0; + // Restored-from-disk parked ref: never adopted, so dispose() does not track it. + registry.register({ + id: "Cold-DisposeRace", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Cold-DisposeRace.jsonl", + status: "parked", + }); + lifecycle.setPersistedSubagentReviverFactory(async () => { + await gate.promise; + return async () => { + reviverRuns++; + return revived.session; + }; + }, TTL); + + const revival = lifecycle.ensureLive("Cold-DisposeRace"); + await flushAsync(); // reach the factory await + await lifecycle.dispose(Date.now()); // teardown while the factory is in flight + gate.resolve(); // factory completes for a superseded owner + + await expect(revival).rejects.toThrow(/disposed/); + // Rejected before the reviver ran: no session was ever created. + expect(reviverRuns).toBe(0); + expect(revived.disposeCalls()).toBe(0); + // No adoption, no live session, no armed TTL that could fire a late park. + expect(lifecycle.has("Cold-DisposeRace")).toBe(false); + expect(registry.get("Cold-DisposeRace")?.session ?? null).toBeNull(); + expect(registry.get("Cold-DisposeRace")?.status).not.toBe("idle"); + vi.advanceTimersByTime(TTL * 10); + await flushAsync(); + expect(revived.disposeCalls()).toBe(0); + }); + + it("a cold revive whose session resolves after dispose disposes that session and rejects", async () => { + const gate = deferred(); + const revived = makeSessionStub(); + registry.register({ + id: "Cold-SessionRace", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Cold-SessionRace.jsonl", + status: "parked", + }); + // Factory resolves immediately (cold-adopts), but the reviver — which builds + // the live session — is held open across dispose(). + lifecycle.setPersistedSubagentReviverFactory( + async () => async () => { + await gate.promise; + return revived.session; + }, + TTL, + ); + + const revival = lifecycle.ensureLive("Cold-SessionRace"); + await flushAsync(); // reach the reviver await + await lifecycle.dispose(Date.now()); // teardown while the reviver is in flight + gate.resolve(); // reviver hands back a live session for a disposed owner + + await expect(revival).rejects.toThrow(/disposed/); + expect(revived.disposeCalls()).toBe(1); + expect(lifecycle.has("Cold-SessionRace")).toBe(false); + expect(registry.get("Cold-SessionRace")?.session ?? null).toBeNull(); + }); + + it("a new top-level owner can cold-revive after the previous global lifecycle was disposed", async () => { + await lifecycle.dispose(Date.now()); + const revived = makeSessionStub(); + registry.register({ + id: "Next-Owner", + displayName: "task", + kind: "sub", + session: null, + sessionFile: "/tmp/Next-Owner.jsonl", + status: "parked", + }); + + const nextLifecycle = AgentLifecycleManager.global(); + nextLifecycle.setPersistedSubagentReviverFactory(async () => async () => revived.session, 0); + + await expect(nextLifecycle.ensureLive("Next-Owner")).resolves.toBe(revived.session); + expect(registry.get("Next-Owner")).toMatchObject({ status: "idle", session: revived.session }); + expect(revived.disposeCalls()).toBe(0); + }); });