review fix: preserve idle waiter quiescence

This commit is contained in:
pku-xht
2026-07-20 13:15:11 +08:00
parent d9a9c223e4
commit da70e47048
8 changed files with 107 additions and 20 deletions

View File

@@ -398,7 +398,7 @@ export class ReactLoopAgent implements Agent {
cancelReason: () => this.cancelReason,
clearCancel: () => { this.cancelRequested = false },
withToolBatch: run => this.withToolBatch(run),
// Already-idle pre-start cancellation still must settle queued-work waiters.
// Pre-start cancellation settles queued-work waiters before publishing idle.
settleIdle: () => { this.settleIdleWaiters() },
}))
}

View File

@@ -91,7 +91,7 @@ export interface LoopHandle {
cancelReason(): string
/** Clear the cancel marker (called once per iteration after the turn returns). */
clearCancel(): void
/** Settle idle waiters when pre-running cancellation finds the status already idle. */
/** Settle idle waiters before pre-running cancellation publishes idle. */
settleIdle(): void
/** Run an active tool-call batch, accepting post-tool context into the FIFO drained before settlement. */
readonly withToolBatch: <T>(run: (acceptContext: (context: HookContext) => void) => Promise<T>) => Promise<T>
@@ -118,6 +118,17 @@ export async function runLoop(ctx: Context, handle: LoopHandle): Promise<void> {
const events = agentEvents(ctx, agent)
while (!handle.isDisposed()) {
// An idle listener can enqueue and cancel replacement work before the next
// wait is installed. Consume that empty marker before parking the driver.
if (handle.isCancelled()) {
handle.clearCancel()
if (!handle.inbox.hasQueued) {
handle.settleIdle()
handle.setStatus('idle')
continue
}
}
await handle.inbox.waitForQueued(handle.disposed)
if (handle.isDisposed()) break
@@ -126,10 +137,10 @@ export async function runLoop(ctx: Context, handle: LoopHandle): Promise<void> {
if (handle.isCancelled()) {
handle.clearCancel()
if (!handle.inbox.hasQueued) {
// setStatus settles runningidle; the explicit settle covers the
// already-idle pre-start path where that transition is deduplicated.
handle.setStatus('idle')
// Settle before publishing idle: the already-idle path has no status
// transition, while an idle listener can register waiters for new work.
handle.settleIdle()
handle.setStatus('idle')
continue
}
}

View File

@@ -194,6 +194,82 @@ describe('Agent.cancel()', () => {
expect(userTexts(agent)).toEqual(['first', 'idle steer'])
})
it('an idle-listener replacement keeps whenIdle pending until the replacement turn finishes', async () => {
const adapter = new MockAdapter([textResponse('first reply'), textResponse('replacement reply')])
const ctx = await harness(adapter)
const agent = ctx.agentLoop.create(SessionId('between-turn-idle-listener'), { provider: 'mock', model: 'mock' })
let rejectFirstFlush = true
ctx.on('session/flush', (session) => {
if (session !== agent.session || !rejectFirstFlush) return
rejectFirstFlush = false
throw new Error('first flush failed')
})
ctx.on('agent/error', (subject, _turn, _step, error) => {
if (subject !== agent || error.message !== 'first flush failed') return
queueMicrotask(() => {
queueMicrotask(() => { agent.cancel('between turns') })
})
})
const replacementRegistered = Promise.withResolvers<undefined>()
let replacementObservation: Promise<{ status: string; requests: number; turns: number }> | undefined
ctx.on('agent/status', (subject, status) => {
if (subject !== agent || status !== 'idle' || replacementObservation !== undefined) return
send(agent, 'replacement')
replacementObservation = agent.whenIdle().then(() => ({
status: agent.status,
requests: adapter.requests.length,
turns: agent.session.events.filter(event => event.type === 'turn/start').length,
}))
replacementRegistered.resolve(undefined)
})
send(agent, 'first')
send(agent, 'cancelled tail')
await replacementRegistered.promise
if (replacementObservation === undefined) throw new Error('idle listener did not register replacement work')
await expect(replacementObservation).resolves.toEqual({ status: 'idle', requests: 2, turns: 2 })
expect(userTexts(agent)).toEqual(['first', 'replacement'])
})
it('idle-listener cancellation settles its waiter without cancelling later work', async () => {
const adapter = new MockAdapter([textResponse('first reply'), textResponse('later reply')])
const ctx = await harness(adapter)
const agent = ctx.agentLoop.create(SessionId('idle-listener-cancel'), { provider: 'mock', model: 'mock' })
const replacementRegistered = Promise.withResolvers<undefined>()
let replacementObservation: Promise<{ status: string; requests: number; turns: number }> | undefined
ctx.on('agent/status', (subject, status) => {
if (subject !== agent || status !== 'idle' || replacementObservation !== undefined) return
send(agent, 'cancelled replacement')
replacementObservation = agent.whenIdle().then(() => ({
status: agent.status,
requests: adapter.requests.length,
turns: agent.session.events.filter(event => event.type === 'turn/start').length,
}))
agent.cancel('idle listener')
replacementRegistered.resolve(undefined)
})
send(agent, 'first')
await replacementRegistered.promise
if (replacementObservation === undefined) throw new Error('idle listener did not register replacement work')
await expect(Promise.race([
replacementObservation,
new Promise((_resolve, reject) => setTimeout(() => { reject(new Error('whenIdle hung after idle-listener cancel')) }, 1000)),
])).resolves.toEqual({ status: 'idle', requests: 1, turns: 1 })
const idle = waitForIdle(ctx, agent)
send(agent, 'later')
await idle
expect(adapter.requests).toHaveLength(2)
expect(userTexts(agent)).toEqual(['first', 'later'])
})
it('cancel() mid-step aborts the active turn and drops every queued tail item', async () => {
const adapter = new MockAdapter(['hang'])
const ctx = await harness(adapter)

View File

@@ -241,7 +241,7 @@ describe('agent/prompt-submit', () => {
send(agent, 'second')
await idle
expect(errors.map(e => e.message)).toEqual(['prompt hook broke'])
// The failed prompt owns one balanced error turn; the adjacent prompt owns
// The failed prompt forms one balanced error turn; the adjacent prompt forms
// the following normal turn without an intermediate idle transition.
const log = events(agent)
expect(log.filter(e => e.type === 'turn/start')).toHaveLength(2)

View File

@@ -120,7 +120,7 @@ describe('agent loop scheduling properties', () => {
// No message lost: every send appears as a user/message, in order.
expect(userMessageTexts(agent)).toEqual(texts)
// This failure-free fixture claims every item into an independent turn.
// This failure-free fixture maps every item to an independent turn.
expect(turnNumbers(agent)).toEqual(texts.map((_, i) => i + 1))
expect(turnEndNumbers(agent)).toEqual(texts.map((_, i) => i + 1))
expect(userMessageCountsByTurn(agent)).toEqual(texts.map(() => 1))
@@ -178,7 +178,7 @@ describe('agent loop scheduling properties', () => {
// No message is lost or reordered, regardless of driver timing.
expect(userMessageTexts(agent)).toEqual(steps.map(s => s.text))
// Every item is claimed and therefore owns one FIFO-ordered turn.
// Every item forms one FIFO-ordered turn containing only that message.
const turns = turnNumbers(agent)
expect(turns).toEqual(steps.map((_, i) => i + 1))
expect(turnEndNumbers(agent)).toEqual(turns)