fix(agent): close the window-2 early-whenIdle race + sync cancellation RFC docs (review)
A reviewer found that window 2 (a cancel from a synchronous agent/status('running')
listener) had the same early-whenIdle() race that window 1 already guards: it
unconditionally `setStatus('idle')` + continue, which settles `whenIdle()`
waiters — so if the running listener cancels AND queues replacement work, the
waiter resolves while the replacement is still queued-and-unrun (the next
iteration runs it later, but the caller already observed quiescence).
Mirror window 1: after clearing the marker, only `setStatus('idle')` when
nothing new is queued; otherwise fall through to run the queued replacement
(status is already `running`), so `whenIdle()` resolves on that turn's
running→idle. Regression test reproduces the reviewer's interleaving (running
listener cancels A, sends B; whenIdle() resolves only after B ran).
Also syncs the cancellation contract in the two ACP RFCs that describe the live
behavior: `session/cancel` is the queue-aware `agent.cancel()` (drops an
about-to-start turn), not the old best-effort `agent.abort()` pre-step
limitation.
This commit is contained in:
@@ -201,14 +201,22 @@ export async function runLoop(ctx: Context, agent: ReactLoopAgent, handle: LoopH
|
||||
|
||||
// Pre-step cancel (window 2): `setStatus('running')` emits `agent/status`
|
||||
// SYNCHRONOUSLY, so a `running` listener can `cancel()` in the gap between the
|
||||
// check above and `runTurn`. `cancel()` already cleared the queued FIFO, so
|
||||
// drop the turn before it starts (runTurn would otherwise throw on an empty
|
||||
// queue) and transition back to idle — `running` was already emitted, so a
|
||||
// real `idle` transition (which also settles waiters) balances the status.
|
||||
// check above and `runTurn`. Mirror window 1: clear the marker, then
|
||||
// - if NOTHING new is queued, drop the about-to-run turn and transition
|
||||
// back to `idle` (`running` was already emitted, so a real idle
|
||||
// transition balances the status AND settles `whenIdle()` waiters);
|
||||
// - if a NEW prompt was queued AFTER the cancel (a `running` listener that
|
||||
// cancels then sends), the marker was for the cancelled work only — fall
|
||||
// through and run the new prompt's turn (status is already `running`), so
|
||||
// a `whenIdle()` waiter resolves on THAT turn's running→idle, not before
|
||||
// it runs. Settling here would resolve quiescence while the replacement
|
||||
// is still queued and unrun (the same early-resolve race window 1 fixes).
|
||||
if (handle.isCancelled()) {
|
||||
handle.clearCancel()
|
||||
handle.setStatus('idle')
|
||||
continue
|
||||
if (!agent.inbox.hasQueued) {
|
||||
handle.setStatus('idle')
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
// Re-derive the turn number from the log each iteration (do NOT keep a local
|
||||
|
||||
@@ -252,6 +252,36 @@ describe('Agent.cancel()', () => {
|
||||
expect(agent.session.events.some(e => e.type === 'turn/start')).toBe(false)
|
||||
})
|
||||
|
||||
it('window 2: whenIdle() does NOT resolve early when a running listener cancels then queues replacement work', async () => {
|
||||
// The window-1 early-resolve race has a window-2 twin: a synchronous
|
||||
// agent/status('running') listener cancels the about-to-run turn AND queues a
|
||||
// replacement. window 2 must NOT settle waiters (via setStatus('idle')) while
|
||||
// the replacement is still queued-and-unrun — it must fall through and run it,
|
||||
// so whenIdle() resolves on the replacement turn's running→idle, not before.
|
||||
const adapter = new MockAdapter([textResponse('A reply'), textResponse('B reply')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create('a1', { model: 'mock' })
|
||||
|
||||
let replaced = false
|
||||
const dispose = ctx.on('agent/status', (subject, status) => {
|
||||
if (subject !== agent || status !== 'running' || replaced) return
|
||||
replaced = true
|
||||
agent.cancel('drop A')
|
||||
send(agent, 'B')
|
||||
})
|
||||
|
||||
send(agent, 'A')
|
||||
const idle = agent.whenIdle()
|
||||
await idle
|
||||
dispose()
|
||||
|
||||
// whenIdle() resolved only AFTER B's turn ran: B's user message + a turn/end
|
||||
// are in the log, and A was dropped.
|
||||
expect(userTexts(agent)).toContain('B')
|
||||
expect(userTexts(agent)).not.toContain('A')
|
||||
expect(agent.session.events.some(e => e.type === 'turn/end')).toBe(true)
|
||||
})
|
||||
|
||||
it('whenIdle() does NOT resolve early when a new prompt is queued during a pre-step cancel', async () => {
|
||||
// The subtle race: a whenIdle() waiter is registered for prompt A; cancel()
|
||||
// clears A; prompt B is queued BEFORE the loop resumes from the idle wait.
|
||||
|
||||
Reference in New Issue
Block a user