review fix: restore idle after between-turn cancellation
This commit is contained in:
@@ -652,8 +652,8 @@ export const EVENT_API: readonly EventApiEntry[] = [
|
||||
name: 'agent/prompt-submit',
|
||||
mode: 'waterfall',
|
||||
signature: '\'agent/prompt-submit\'(this: Scoped<Agent>, agent: Agent, content: ContentBlock[], source: MessageSource, next: () => Promise<PromptDecision>): Promise<PromptDecision>',
|
||||
jsDoc: '/**\n * Allow, rewrite, or block one drained prompt before it becomes a user\n * message. Call `next()` for the unchanged default.\n * @param agent - the agent draining its inbox.\n * @param content - the drained message\'s blocks, as queued.\n * @param source - the message\'s resolved source.\n * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): agent-scoped listeners receive only that agent.\n * @mode waterfall\n */',
|
||||
summary: 'Allow, rewrite, or block one drained prompt before it becomes a user message.',
|
||||
jsDoc: '/**\n * Allow, rewrite, or block one claimed prompt before it becomes a user\n * message. Call `next()` for the unchanged default.\n * @param agent - the agent whose turn claimed the message.\n * @param content - the claimed message\'s blocks, as queued.\n * @param source - the message\'s resolved source.\n * Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): agent-scoped listeners receive only that agent.\n * @mode waterfall\n */',
|
||||
summary: 'Allow, rewrite, or block one claimed prompt before it becomes a user message.',
|
||||
},
|
||||
{
|
||||
name: 'agent/queued',
|
||||
|
||||
@@ -398,7 +398,7 @@ export class ReactLoopAgent implements Agent {
|
||||
cancelReason: () => this.cancelReason,
|
||||
clearCancel: () => { this.cancelRequested = false },
|
||||
withToolBatch: run => this.withToolBatch(run),
|
||||
// Pre-step cancellation re-parks without emitting a status transition.
|
||||
// Already-idle pre-start cancellation still must settle queued-work waiters.
|
||||
settleIdle: () => { this.settleIdleWaiters() },
|
||||
}))
|
||||
}
|
||||
|
||||
@@ -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 skips a turn, without emitting `agent/status`. */
|
||||
/** Settle idle waiters when pre-running cancellation finds the status already 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>
|
||||
@@ -126,6 +126,9 @@ export async function runLoop(ctx: Context, handle: LoopHandle): Promise<void> {
|
||||
if (handle.isCancelled()) {
|
||||
handle.clearCancel()
|
||||
if (!handle.inbox.hasQueued) {
|
||||
// setStatus settles running→idle; the explicit settle covers the
|
||||
// already-idle pre-start path where that transition is deduplicated.
|
||||
handle.setStatus('idle')
|
||||
handle.settleIdle()
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -140,6 +140,60 @@ describe('Agent.cancel()', () => {
|
||||
expect(agent.status).toBe('idle')
|
||||
})
|
||||
|
||||
it('cancel() between consecutive turns restores idle and leaves idle steer usable', async () => {
|
||||
const adapter = new MockAdapter([textResponse('first reply'), textResponse('steer reply')])
|
||||
const ctx = await harness(adapter)
|
||||
const agent = ctx.agentLoop.create(SessionId('between-turn-cancel'), { 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')
|
||||
})
|
||||
|
||||
const cancelled = Promise.withResolvers<undefined>()
|
||||
ctx.on('agent/error', (subject, _turn, _step, error) => {
|
||||
if (subject !== agent || error.message !== 'first flush failed') return
|
||||
// The first hop runs before runLoop resumes from runTurn; the second lands
|
||||
// before its resolved waitForQueued continuation checks cancellation.
|
||||
queueMicrotask(() => {
|
||||
queueMicrotask(() => {
|
||||
agent.cancel('between turns')
|
||||
cancelled.resolve(undefined)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
const statuses: string[] = []
|
||||
ctx.on('agent/status', (subject, status) => {
|
||||
if (subject === agent) statuses.push(status)
|
||||
})
|
||||
|
||||
send(agent, 'first')
|
||||
send(agent, 'queued tail')
|
||||
await cancelled.promise
|
||||
|
||||
expect(agent.status).toBe('idle')
|
||||
expect(statuses).toEqual(['running', 'idle'])
|
||||
expect(adapter.requests).toHaveLength(1)
|
||||
expect(agent.session.events.filter(event => event.type === 'turn/start')).toHaveLength(1)
|
||||
expect(userTexts(agent)).toEqual(['first'])
|
||||
|
||||
let idleResolved = false
|
||||
void agent.whenIdle().then(() => { idleResolved = true })
|
||||
await Promise.resolve()
|
||||
expect(idleResolved).toBe(true)
|
||||
|
||||
const idle = waitForIdle(ctx, agent)
|
||||
agent.steer([{ type: 'text', text: 'idle steer' }])
|
||||
await idle
|
||||
|
||||
expect(statuses).toEqual(['running', 'idle', 'running', 'idle'])
|
||||
expect(adapter.requests).toHaveLength(2)
|
||||
expect(userTexts(agent)).toEqual(['first', 'idle steer'])
|
||||
})
|
||||
|
||||
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)
|
||||
|
||||
@@ -207,10 +207,10 @@ declare module 'cordis' {
|
||||
*/
|
||||
'agent/pre-step'(this: Scoped<Agent>, agent: Agent, turn: number, step: number, signal: AbortSignal): Promise<void> | void
|
||||
/**
|
||||
* Allow, rewrite, or block one drained prompt before it becomes a user
|
||||
* Allow, rewrite, or block one claimed prompt before it becomes a user
|
||||
* message. Call `next()` for the unchanged default.
|
||||
* @param agent - the agent draining its inbox.
|
||||
* @param content - the drained message's blocks, as queued.
|
||||
* @param agent - the agent whose turn claimed the message.
|
||||
* @param content - the claimed message's blocks, as queued.
|
||||
* @param source - the message's resolved source.
|
||||
* Scope-filtered dispatch (`@deepseek-ai/dsh-scope`): agent-scoped listeners receive only that agent.
|
||||
* @mode waterfall
|
||||
|
||||
@@ -196,7 +196,7 @@ export interface SessionEventMap {
|
||||
'step/start': { turn: number; step: number }
|
||||
/** Closes step `step` of turn `turn`. */
|
||||
'step/end': { turn: number; step: number }
|
||||
/** A user-visible prompt (queued message drained at turn start). */
|
||||
/** A user-visible prompt (the queued message claimed for this turn). */
|
||||
'user/message': { content: ContentBlock[]; source: MessageSource }
|
||||
/**
|
||||
* Durable record of a prompt veto and its reason. It is log-only: the blocked
|
||||
|
||||
@@ -830,7 +830,8 @@ export function apply(ctx: Context, config: AcpConfig): void {
|
||||
// not-yet-started prompt never runs, while a prompt accepted afterward
|
||||
// remains a separate queued turn. Scoped to THIS session's
|
||||
// agent — a cancel in one session never touches another's stream or
|
||||
// pending prompt (RFC 011 isolation). We ALSO settle the in-flight prompt
|
||||
// pending prompt (multi-session isolation).
|
||||
// We ALSO settle the in-flight prompt
|
||||
// as cancelled directly here: do NOT rely on the resulting turn/end to
|
||||
// settle it, because cancel() may drop the turn before any turn/end is
|
||||
// emitted, and removing this direct settle would move the RPC's
|
||||
|
||||
@@ -223,7 +223,8 @@ describe('acp bridge — disposal & HMR safety', () => {
|
||||
|
||||
it('per-session AgentHandle dispose leaves sibling agents untouched', async () => {
|
||||
// The factory returns a per-agent AgentHandle whose dispose() tears down
|
||||
// EXACTLY that agent + its session — RFC 011 isolation. Create two agents
|
||||
// EXACTLY that agent + its session — the registry's per-handle isolation
|
||||
// contract. Create two agents
|
||||
// directly through the registry factory (the same path the ACP bridge uses),
|
||||
// dispose one handle, and assert the other survives, registered and
|
||||
// queryable, with its session still in the store.
|
||||
|
||||
@@ -14,7 +14,7 @@ function messageTextFor(updates: { sessionId?: string; update: CapturedUpdate }[
|
||||
.join('')
|
||||
}
|
||||
|
||||
describe('acp bridge — RFC 011 multi-session isolation', () => {
|
||||
describe('acp bridge — multi-session isolation', () => {
|
||||
let storageDir: string
|
||||
let harness: BridgeHarness | undefined
|
||||
|
||||
|
||||
Reference in New Issue
Block a user