From 2cf689301c337c53739e4cebcc37929c06e325ff Mon Sep 17 00:00:00 2001 From: pku-xht Date: Fri, 17 Jul 2026 17:27:53 +0800 Subject: [PATCH] review fix: close one-turn lifecycle gaps --- packages/core/agent-loop/src/loop.ts | 1 + packages/core/agent-loop/tests/cancel.spec.ts | 30 +++++++++ .../agent-loop/tests/interception.spec.ts | 11 ++++ packages/core/agent-loop/tests/loop.spec.ts | 65 +++++++++++++++++-- 4 files changed, 102 insertions(+), 5 deletions(-) diff --git a/packages/core/agent-loop/src/loop.ts b/packages/core/agent-loop/src/loop.ts index b26d09e5c1..80b6b27eaf 100644 --- a/packages/core/agent-loop/src/loop.ts +++ b/packages/core/agent-loop/src/loop.ts @@ -117,6 +117,7 @@ export async function runLoop(ctx: Context, agent: ReactLoopAgent, handle: LoopH } handle.setStatus('running') + if (handle.isDisposed()) break // A synchronous `running` listener can cancel before `runTurn`; balance the // status only when no replacement prompt was queued by that listener. diff --git a/packages/core/agent-loop/tests/cancel.spec.ts b/packages/core/agent-loop/tests/cancel.spec.ts index 5ed15f1b0b..dd02afdec7 100644 --- a/packages/core/agent-loop/tests/cancel.spec.ts +++ b/packages/core/agent-loop/tests/cancel.spec.ts @@ -88,6 +88,36 @@ describe('Agent.cancel()', () => { expect(agent.status).toBe('idle') }) + it('disposal from the running notification drops queued work before turn start', async () => { + const adapter = new MockAdapter([textResponse('should not run')]) + const ctx = await harness(adapter) + const handle = await ctx.agents.create({ + agentId: AgentId('a-dispose-running'), + sessionId: SessionId('dispose-running-session'), + agentOptions: { model: 'mock' }, + }) + const agent = handle.agent as ReactLoopAgent + + const running = Promise.withResolvers() + let disposalDone: Promise | undefined + ctx.on('agent/status', (subject, status) => { + if (subject !== agent || status !== 'running') return + disposalDone = handle.dispose() + running.resolve(undefined) + }) + + send(agent, 'drop before claim') + await running.promise + if (disposalDone === undefined) throw new Error('running listener did not start disposal') + await disposalDone + await agent.done + + expect(agent.status).toBe('disposed') + expect(agent.session.events.some(event => event.type === 'turn/start')).toBe(false) + expect(userTexts(agent)).toEqual([]) + expect(adapter.requests).toHaveLength(0) + }) + it('a whenIdle() waiter registered BEFORE a pre-step cancel resolves (F1 hang guard)', async () => { const adapter = new MockAdapter([textResponse('x')]) const ctx = await harness(adapter) diff --git a/packages/core/agent-loop/tests/interception.spec.ts b/packages/core/agent-loop/tests/interception.spec.ts index 57b773de8a..8663875c6b 100644 --- a/packages/core/agent-loop/tests/interception.spec.ts +++ b/packages/core/agent-loop/tests/interception.spec.ts @@ -239,7 +239,13 @@ describe('agent/prompt-submit', () => { return { kind: 'allow' as const } }) const errors: Error[] = [] + const reasons: TurnEndReason[] = [] + const statuses: string[] = [] ctx.on('agent/error', (_a, _t, _s, error) => void errors.push(error)) + ctx.on('agent/status', (subject, status) => { if (subject === agent) statuses.push(status) }) + ctx.on('session/event', (session, event) => { + if (session === agent.session && event.type === 'turn/end') reasons.push(event.data.reason) + }) const idle = waitForIdle(ctx, agent) send(agent, 'first') @@ -251,6 +257,11 @@ describe('agent/prompt-submit', () => { const log = events(agent) expect(log.filter(e => e.type === 'turn/start')).toHaveLength(2) expect(log.filter(e => e.type === 'turn/end')).toHaveLength(2) + expect(reasons).toEqual([ + { kind: 'error', step: 0, message: 'prompt hook broke' }, + { kind: 'completed' }, + ]) + expect(statuses).toEqual(['running', 'idle']) expect(adapter.requests).toHaveLength(1) expect(JSON.stringify(adapter.requests[0]!.messages)).toContain('second') }) diff --git a/packages/core/agent-loop/tests/loop.spec.ts b/packages/core/agent-loop/tests/loop.spec.ts index aff13ddd5c..0e223563b7 100644 --- a/packages/core/agent-loop/tests/loop.spec.ts +++ b/packages/core/agent-loop/tests/loop.spec.ts @@ -349,14 +349,24 @@ describe('agent loop', () => { expect(flat).toContain('change of plans') }) - it('steering while idle behaves like send (starts a turn)', async () => { - const adapter = new MockAdapter([textResponse('ok')]) + it('same-tick idle steering inherits one-send-one-turn FIFO behavior', async () => { + const adapter = new MockAdapter([textResponse('first'), textResponse('second')]) const ctx = await harness(adapter) const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) - agent.steer([{ type: 'text', text: 'hello' }]) - await waitForIdle(ctx, agent) - expect(agent.session.events.some(e => e.type === 'user/message')).toBe(true) + const idle = waitForIdle(ctx, agent) + agent.steer([{ type: 'text', text: 'first idle steer' }]) + agent.steer([{ type: 'text', text: 'second idle steer' }]) + await idle + + expect(agent.session.events.filter(event => event.type === 'turn/start')).toHaveLength(2) + expect(agent.session.events + .filter(event => event.type === 'user/message') + .map(event => event.data.content)).toEqual([ + [{ type: 'text', text: 'first idle steer' }], + [{ type: 'text', text: 'second idle steer' }], + ]) + expect(adapter.requests).toHaveLength(2) }) it('inject() while idle wraps context in a one-shot turn, visible to the next request', async () => { @@ -893,6 +903,51 @@ describe('agent loop', () => { expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('second message') }) + it('holds a turn-end listener send behind the closing turn checkpoint', async () => { + const adapter = new MockAdapter([textResponse('first answer'), textResponse('second answer')]) + const ctx = await harness(adapter) + const agent = ctx.agentLoop.create(AgentId('a1'), { model: 'mock' }) + + const firstFlush = Promise.withResolvers() + const releaseFirstFlush = Promise.withResolvers() + let flushes = 0 + ctx.on('session/flush', async (session) => { + if (session !== agent.session) return + flushes += 1 + if (flushes === 1) { + firstFlush.resolve(undefined) + await releaseFirstFlush.promise + } + }) + + const turns: number[] = [] + const statuses: string[] = [] + ctx.on('agent/status', (subject, status) => { + if (subject === agent) statuses.push(status) + }) + ctx.on('session/event', (session, event) => { + if (session !== agent.session) return + if (event.type === 'turn/start') turns.push(event.data.turn) + if (event.type === 'turn/end' && event.data.turn === 1) send(agent, 'turn-end listener message') + }) + + const idle = waitForIdle(ctx, agent) + send(agent, 'first message') + await firstFlush.promise + + expect(turns).toEqual([1]) + expect(adapter.requests).toHaveLength(1) + + releaseFirstFlush.resolve(undefined) + await idle + + expect(turns).toEqual([1, 2]) + expect(statuses).toEqual(['running', 'idle']) + expect(adapter.requests).toHaveLength(2) + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('first answer') + expect(JSON.stringify(adapter.requests[1]!.messages)).toContain('turn-end listener message') + }) + it('keeps a reentrant agent/queued send as the next independent turn', async () => { const adapter = new MockAdapter([textResponse('first'), textResponse('second')]) const ctx = await harness(adapter)