Преглед на файлове

fix(agent-loop): one quiescence boundary across owner unload and handle.dispose

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.
Tianyi Cui преди 3 месеца
родител
ревизия
513ba2716d
променени са 2 файла, в които са добавени 35 реда и са изтрити 1 реда
  1. 14 1
      packages/core/agent-loop/src/index.ts
  2. 21 0
      packages/core/agent-loop/tests/scope-lifecycle.spec.ts

+ 14 - 1
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<void> } {
     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<void>()
     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 } }
   }
 
   /**

+ 21 - 0
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<typeof ctx.agents.create>
+    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
+  })
 })