feat(agent): add queue-aware Agent.cancel() primitive
abort() only kills the in-flight step, so a queued-but-not-yet-started prompt
ran to completion after a cancel and a prompt accepted right after could be
batched into the cancelled turn (the loop merges queued messages into one turn).
This closes TODO(rfc010-cancel-prestep) with a distinct cancel() verb.
cancel() clears the queued + steering FIFOs, aborts the in-flight step, and
drives a turn-scoped marker on the LoopHandle that the driver checks at EVERY
point a turn could start or continue:
- right after the idle wait (window 1): drop the about-to-run turn and settle
whenIdle() waiters directly (no running→idle transition fires, and no
agent/status is emitted, so an ACP listener can't see a spurious idle that
resolves a freshly-queued prompt as cancelled);
- after the synchronous setStatus('running') emit (window 2): a running listener
can cancel in the gap before runTurn;
- in the step-start window (before runStep, after setAbort): a synchronous
turn-start/step-start listener can cancel before any AbortController exists;
- at the continuation gate: a cancel during the continuation waterfall (the
finished step's controller already cleared) ends the turn aborted.
The marker is ARMED only when there is something to cancel (running, an
in-flight step, or queued/steering work) — an idle no-op cancel cannot leave it
set to drop a later prompt — and RESET unconditionally once per loop iteration,
so it governs exactly one turn and never leaks onto the next prompt (even when a
send() lands in the cancelled turn's flush window).
ACP session/cancel now maps to agent.cancel() (keeping the synchronous
settlePrompt). Teardown/disconnect still use abort('disposed') until PR D, so
the ACP README narrows the remaining best-effort window to teardown only.
Tests (agent-loop/cancel.spec.ts) cover every window unit-level (the F1 hang
guard: a whenIdle() waiter registered before a pre-step cancel resolves; the F2
leak guard: idle cancel then a prompt runs; mid-step, continuation, both
pre-step windows, turn-start-listener, steering-cleared, marker-reset). ACP
turns.spec.ts adds the through-bridge tests with NO intervening whenIdle (idle
cancel→prompt runs; mid-stream cancel→immediate next prompt runs) and updates
the stale pre-step test to the queue-aware guarantee. The existing cancel
snapshot golden is byte-identical (it drives the new cancel() path end-to-end
through the real subprocess), so no new golden is needed. 100% coverage.
This commit is contained in:
@@ -107,6 +107,26 @@ export interface LoopHandle {
|
||||
/** Resolves when the agent is disposed — unblocks the idle wait. */
|
||||
disposed: Promise<void>
|
||||
isDisposed(): boolean
|
||||
/**
|
||||
* Whether a `cancel()` is pending for the current turn. The driver checks this
|
||||
* at every decision point where a turn could start or continue (right after
|
||||
* the idle wait, after the `running` flip, before each step, and at the
|
||||
* continuation gate) and drops the about-to-run / continuing turn. Reset once
|
||||
* per loop iteration via {@link clearCancel} after the turn returns, so the
|
||||
* marker governs exactly one cancellation and never leaks to a later prompt.
|
||||
*/
|
||||
isCancelled(): boolean
|
||||
/** Clear the cancel marker (called once per iteration after the turn returns). */
|
||||
clearCancel(): void
|
||||
/**
|
||||
* Settle pending `whenIdle()` waiters WITHOUT a status transition. Used by the
|
||||
* pre-step cancel-skip path: it drops the about-to-run turn and re-parks at the
|
||||
* idle wait, so no `running→idle` transition fires to settle a `whenIdle()`
|
||||
* waiter that was registered in the pre-step window — this settles it directly
|
||||
* (it emits no `agent/status`, so an ACP `agent/status` listener never sees a
|
||||
* spurious idle that would resolve a freshly-queued prompt as cancelled).
|
||||
*/
|
||||
settleIdle(): void
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -148,7 +168,33 @@ export async function runLoop(ctx: Context, agent: ReactLoopAgent, handle: LoopH
|
||||
await agent.inbox.waitForQueued(handle.disposed)
|
||||
if (handle.isDisposed()) break
|
||||
|
||||
// Pre-step cancel (window 1): a `cancel()` landed after a `send()` woke the
|
||||
// idle wait but before we flip to `running`. Drop the about-to-run turn: the
|
||||
// queued/steering work is already cleared by `cancel()`, and we settle any
|
||||
// `whenIdle()` waiter DIRECTLY (no status transition fires here, so the
|
||||
// running→idle settle never runs) WITHOUT emitting `agent/status` (an ACP
|
||||
// listener must not see a spurious idle that resolves a freshly-queued prompt
|
||||
// as cancelled). Clear the marker and re-park.
|
||||
if (handle.isCancelled()) {
|
||||
handle.clearCancel()
|
||||
handle.settleIdle()
|
||||
continue
|
||||
}
|
||||
|
||||
handle.setStatus('running')
|
||||
|
||||
// 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.
|
||||
if (handle.isCancelled()) {
|
||||
handle.clearCancel()
|
||||
handle.setStatus('idle')
|
||||
continue
|
||||
}
|
||||
|
||||
// Re-derive the turn number from the log each iteration (do NOT keep a local
|
||||
// counter): an idle `agent.inject()` can append its own one-shot turn while
|
||||
// the loop waits above, so the next real turn must continue from whatever
|
||||
@@ -170,8 +216,18 @@ export async function runLoop(ctx: Context, agent: ReactLoopAgent, handle: LoopH
|
||||
} catch { /* contained: a throwing agent/error listener must not kill the driver */ }
|
||||
}
|
||||
|
||||
// Reset the cancel marker UNCONDITIONALLY here, after the turn returns and
|
||||
// before the next iteration's idle wait. NOT gated on the idle transition
|
||||
// below: a `send()` that lands during the cancelled turn's flush window makes
|
||||
// `hasQueued` true at the `setStatus('idle')` guard, so an idle-gated reset
|
||||
// would never fire and the stale marker would wrongly drop that next prompt's
|
||||
// turn. Resetting per iteration scopes the marker to exactly the turn that was
|
||||
// cancelled.
|
||||
handle.clearCancel()
|
||||
|
||||
// Steering that arrived too late to join this turn (turn-end listeners,
|
||||
// flush) becomes a queued message — it must never be stranded.
|
||||
// flush) becomes a queued message — it must never be stranded. (A cancelled
|
||||
// turn already cleared its steering, so there is nothing to re-enqueue.)
|
||||
for (const message of agent.inbox.drainSteering()) {
|
||||
agent.inbox.enqueue(message)
|
||||
}
|
||||
@@ -323,6 +379,20 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle,
|
||||
const abort = new AbortController()
|
||||
handle.setAbort(abort)
|
||||
|
||||
// Cancel landing in the step-start window: a synchronous `agent/turn-start`
|
||||
// or `agent/step-start` listener (both fire before this point) can have
|
||||
// called `cancel()`, and `runStep` would otherwise run a full extra step
|
||||
// with no AbortController having observed it. Check the marker AFTER
|
||||
// setAbort (so the next-iteration drain sees a clean controller) and before
|
||||
// `runStep`: drop the step, end the turn `aborted`. closeStep balances the
|
||||
// already-appended step/start.
|
||||
if (handle.isCancelled()) {
|
||||
handle.setAbort(undefined)
|
||||
reason = { kind: 'aborted', reason: 'cancelled' }
|
||||
closeStep()
|
||||
break
|
||||
}
|
||||
|
||||
let stepOutcome: { hadToolCalls: boolean; finish: FinishReason } | { error: Error }
|
||||
try {
|
||||
stepOutcome = await runStep(ctx, agent, turn, step, abort.signal)
|
||||
@@ -382,6 +452,16 @@ async function runTurn(ctx: Context, agent: ReactLoopAgent, handle: LoopHandle,
|
||||
// next iteration's drain records it.
|
||||
if (!shouldContinue && agent.inbox.hasSteering) shouldContinue = true
|
||||
|
||||
// A cancel that landed during the continuation window — after the step's
|
||||
// AbortController was cleared (setAbort(undefined)) but before the next
|
||||
// step starts — has no controller to observe it, so the turn-scoped marker
|
||||
// ends the turn here. cancel() also cleared the steering FIFO, so the
|
||||
// override above did not re-arm continuation.
|
||||
if (handle.isCancelled()) {
|
||||
reason = { kind: 'aborted', reason: 'cancelled' }
|
||||
break
|
||||
}
|
||||
|
||||
if (!shouldContinue || handle.isDisposed()) {
|
||||
/* v8 ignore next -- disposal during continuation-decision window is a narrow race; error-path disposal is covered elsewhere */
|
||||
if (handle.isDisposed()) reason = { kind: 'disposed' }
|
||||
|
||||
Reference in New Issue
Block a user