From 513ba2716d0f8fa539a68adc04d890328d0f5b97 Mon Sep 17 00:00:00 2001 From: Tianyi Cui <53024+tianyicui@users.noreply.github.com> Date: Thu, 9 Jul 2026 03:58:15 +0800 Subject: [PATCH] fix(agent-loop): one quiescence boundary across owner unload and handle.dispose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cordis effect disposers are single-shot but not await-idempotent: when the owning fiber's unload invokes the raw wrapper first, a concurrent handle.dispose() got an immediate undefined and resolved before teardown finished — violating the driver's stated one-boundary contract (Codex implementation-review finding). The teardown chain's FIRST-yielded (so disposed-last) disposer now resolves a shared completion promise; the handle path awaits it after the wrapper, so tool-finally, parent-teardown, and owner-unload all observe the same fully-torn-down state. Regression test: owner unload begins first, concurrent handle.dispose still awaits unregistration + session detach. --- packages/core/agent-loop/src/index.ts | 15 ++++++++++++- .../agent-loop/tests/scope-lifecycle.spec.ts | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/core/agent-loop/src/index.ts b/packages/core/agent-loop/src/index.ts index 306f8707e2..47b2607881 100644 --- a/packages/core/agent-loop/src/index.ts +++ b/packages/core/agent-loop/src/index.ts @@ -303,7 +303,20 @@ export class AgentLoop extends Service implements AgentFactory { setup?: (agentCtx: Context) => void, ): { agent: ReactLoopAgent; disposeAgent: () => Promise } { const agent = new ReactLoopAgent(this.ctx, id, options, session) + // The ONE quiescence boundary every disposal path observes. Cordis effect + // disposers are single-shot but not await-idempotent: when the OWNING + // fiber's unload invokes the raw wrapper first, a concurrent + // `handle.dispose()` calling the same wrapper gets an immediate undefined + // (epoch already cleared) — so the handle path must await THIS promise, + // resolved by the teardown chain's final disposer, not the wrapper's + // return. Every disposer in the chain is deliberately infallible (stop() + // is infallible by contract, unregister/detach contain their listeners, + // the scope unwind is cordis-contained), so the final disposer always + // runs — a throwing link would skip the rest of a cordis dispose chain. + const { promise: torndown, resolve: markTorndown } = Promise.withResolvers() const dispose = this.ctx.effect(function* (this: AgentLoop) { + // First-yielded ⇒ disposed LAST: marks true teardown completion. + yield () => { markTorndown() } // Mint the agent's scope (key = the agent) and wire the two-phase // reference: the scope context tags registrations + filters dispatch; // the extend adds the `ctx.agent` DX own-property on top. The raw @@ -349,7 +362,7 @@ export class AgentLoop extends Service implements AgentFactory { // disposed later) is still attached. yield async () => { stop(); await agent.done } }.bind(this), 'agentLoop.start()') - return { agent, disposeAgent: async () => { await dispose() } } + return { agent, disposeAgent: async () => { await dispose(); await torndown } } } /** diff --git a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts index 28d1da192d..628339f1e5 100644 --- a/packages/core/agent-loop/tests/scope-lifecycle.spec.ts +++ b/packages/core/agent-loop/tests/scope-lifecycle.spec.ts @@ -169,4 +169,25 @@ describe('agent scope lifecycle', () => { agentEvents(ctx, agent).emit('agent/error', 2, 0, new Error('for a1')) expect(heard).toEqual(['a1:2']) }) + + it('handle.dispose() during owner unload still awaits true quiescence (shared boundary)', async () => { + const ctx = await harness() + let handle!: ReturnType + const owner = await ctx.plugin(Object.assign((inner: Context) => { + handle = inner.agents.create({ agentId: AgentId('h1'), sessionId: SessionId('h1-s'), agentOptions: { model: 'mock' } }) + }, { inject: ['agents'] })) + + const teardownDone: string[] = [] + ctx.on('agent/disposed', () => void teardownDone.push('unregistered')) + + // Owner unload begins FIRST (invokes the raw cordis wrapper)… + const unload = owner.dispose() + // …and a concurrent handle.dispose() must not resolve before the chain + // actually finished (the raw wrapper returns undefined on a repeat call). + await handle.dispose() + expect(teardownDone).toContain('unregistered') + expect(ctx.agents.get(AgentId('h1'))).toBeUndefined() + expect(ctx.sessions.get(SessionId('h1-s'))).toBeUndefined() + await unload + }) })