fix(agent-loop): let a concluding tool result beat same-step steering
concludesTurn is terminal: a completed step now carries the concluded fact to the driver, which ends the turn immediately instead of letting steering that arrived during the tool batch (or from an agent/stopping listener) reopen it. The steering is not lost — it drained into the log as steering/message before the decision point, so it feeds the next turn's request. Also annotates the driver's unreachable invariant guards (exhaustiveness default, abort-slot ownership) for the coverage gate.
This commit is contained in:
@@ -2,5 +2,5 @@
|
|||||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||||
# after editing either side, bring the other along and re-record with:
|
# after editing either side, bring the other along and re-record with:
|
||||||
# pnpm run verify-translation-pairing --write
|
# pnpm run verify-translation-pairing --write
|
||||||
architecture.md: 40d1641e494d2087e5fb5f9c50dbe1a2df30b103
|
architecture.md: 514a74534df4761c2677498b435d0f6cd22e5cd8
|
||||||
architecture.zh.md: 324c4ff1950cbe3a3b8f015bb545dad3869dd5b4
|
architecture.zh.md: c7e88e6f08d361e4cec15b51046145e88ceff5f6
|
||||||
|
|||||||
@@ -100,6 +100,7 @@ forever:
|
|||||||
each model-order result -> ordered tools/post-execute -> 'tool/result'
|
each model-order result -> ordered tools/post-execute -> 'tool/result'
|
||||||
drain accepted tool context and steering
|
drain accepted tool context and steering
|
||||||
'step/end'
|
'step/end'
|
||||||
|
a concluding tool result ends the turn; drained steering waits for the next turn
|
||||||
continue when tools or steering require another step
|
continue when tools or steering require another step
|
||||||
otherwise agent/stopping -> drain once more -> continue only for steering
|
otherwise agent/stopping -> drain once more -> continue only for steering
|
||||||
'turn/end' -> agent/idle
|
'turn/end' -> agent/idle
|
||||||
|
|||||||
@@ -100,6 +100,7 @@ forever:
|
|||||||
each model-order result -> ordered tools/post-execute -> 'tool/result'
|
each model-order result -> ordered tools/post-execute -> 'tool/result'
|
||||||
drain accepted tool context and steering
|
drain accepted tool context and steering
|
||||||
'step/end'
|
'step/end'
|
||||||
|
a concluding tool result ends the turn; drained steering waits for the next turn
|
||||||
continue when tools or steering require another step
|
continue when tools or steering require another step
|
||||||
otherwise agent/stopping -> drain once more -> continue only for steering
|
otherwise agent/stopping -> drain once more -> continue only for steering
|
||||||
'turn/end' -> agent/idle
|
'turn/end' -> agent/idle
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ import { executeToolCalls } from './tool-calls.ts'
|
|||||||
|
|
||||||
/** One completed step or a final-adapter failure eligible for recovery. */
|
/** One completed step or a final-adapter failure eligible for recovery. */
|
||||||
type StepOutcome =
|
type StepOutcome =
|
||||||
| { kind: 'completed'; continueTurn: boolean; maxTokens: boolean }
|
| { kind: 'completed'; continueTurn: boolean; concluded: boolean; maxTokens: boolean }
|
||||||
| { kind: 'request-failed'; error: RequestError; failure: LlmFailure }
|
| { kind: 'request-failed'; error: RequestError; failure: LlmFailure }
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -197,6 +197,7 @@ export class ReactLoopAgent implements Agent {
|
|||||||
private kick(): void {
|
private kick(): void {
|
||||||
if (this.abort !== undefined || !this.queued.some(item => item.wakeup)) return
|
if (this.abort !== undefined || !this.queued.some(item => item.wakeup)) return
|
||||||
const item = this.queued.shift()
|
const item = this.queued.shift()
|
||||||
|
/* v8 ignore next -- unreachable: the some() guard above proves the queue is non-empty */
|
||||||
if (item === undefined) return
|
if (item === undefined) return
|
||||||
const { message } = item
|
const { message } = item
|
||||||
|
|
||||||
@@ -229,6 +230,10 @@ export class ReactLoopAgent implements Agent {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// cancel() aborts but never clears the slot, and kick()/run()/retry()
|
||||||
|
// all refuse to install a new owner while one exists, so the admission
|
||||||
|
// still owns the slot here.
|
||||||
|
/* v8 ignore next -- unreachable false arm: no writer replaces the abort owner mid-admission */
|
||||||
if (this.abort === admission) this.abort = undefined
|
if (this.abort === admission) this.abort = undefined
|
||||||
if (admitted === undefined) {
|
if (admitted === undefined) {
|
||||||
this.continueOrIdle()
|
this.continueOrIdle()
|
||||||
@@ -243,6 +248,9 @@ export class ReactLoopAgent implements Agent {
|
|||||||
* only after `turn/start` commits; until then it has no owner state to unwind.
|
* only after `turn/start` commits; until then it has no owner state to unwind.
|
||||||
*/
|
*/
|
||||||
private async run(trigger: TurnTrigger, admitted: UserMessageData[] = []): Promise<void> {
|
private async run(trigger: TurnTrigger, admitted: UserMessageData[] = []): Promise<void> {
|
||||||
|
// Both entries hold the invariant: kick() clears the admission slot before
|
||||||
|
// awaiting run(), and retry() returns early whenever a slot owner exists.
|
||||||
|
/* v8 ignore next -- unreachable guard: every caller clears or checks the abort slot first */
|
||||||
if (this.abort !== undefined) throw new Error(`agent "${this.id}" is already running`)
|
if (this.abort !== undefined) throw new Error(`agent "${this.id}" is already running`)
|
||||||
const controller = new AbortController()
|
const controller = new AbortController()
|
||||||
this.abort = controller
|
this.abort = controller
|
||||||
@@ -279,6 +287,11 @@ export class ReactLoopAgent implements Agent {
|
|||||||
switch (outcome.kind) {
|
switch (outcome.kind) {
|
||||||
case 'completed':
|
case 'completed':
|
||||||
if (outcome.maxTokens) reason = { kind: 'max-tokens' }
|
if (outcome.maxTokens) reason = { kind: 'max-tokens' }
|
||||||
|
// A concluding tool result is terminal: steering already in the
|
||||||
|
// log waits for the next turn's request instead of reopening this
|
||||||
|
// one, and the agent/stopping drain below is skipped for the same
|
||||||
|
// reason.
|
||||||
|
if (outcome.concluded) break steps
|
||||||
if (outcome.continueTurn || this.outbox.some(item => 'id' in item)) continue
|
if (outcome.continueTurn || this.outbox.some(item => 'id' in item)) continue
|
||||||
break
|
break
|
||||||
case 'request-failed': {
|
case 'request-failed': {
|
||||||
@@ -313,6 +326,7 @@ export class ReactLoopAgent implements Agent {
|
|||||||
idle = settlement.idle
|
idle = settlement.idle
|
||||||
break steps
|
break steps
|
||||||
}
|
}
|
||||||
|
/* v8 ignore next 2 -- closed-union exhaustiveness guard */
|
||||||
default:
|
default:
|
||||||
assertNever(outcome)
|
assertNever(outcome)
|
||||||
}
|
}
|
||||||
@@ -455,6 +469,7 @@ export class ReactLoopAgent implements Agent {
|
|||||||
return {
|
return {
|
||||||
kind: 'completed',
|
kind: 'completed',
|
||||||
continueTurn: (toolCalls.length > 0 && !concluded) || steered,
|
continueTurn: (toolCalls.length > 0 && !concluded) || steered,
|
||||||
|
concluded,
|
||||||
maxTokens: finish.kind === 'max-tokens',
|
maxTokens: finish.kind === 'max-tokens',
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -508,6 +508,46 @@ describe('agent loop', () => {
|
|||||||
expect(agent.session.events.some(e => e.type === 'tool/result')).toBe(true)
|
expect(agent.session.events.some(e => e.type === 'tool/result')).toBe(true)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('a concluding tool result beats steering that arrived during the same step', async () => {
|
||||||
|
const adapter = new MockAdapter([
|
||||||
|
toolCallResponse('c1', 'finalize', {}),
|
||||||
|
textResponse('next turn reply'),
|
||||||
|
])
|
||||||
|
const ctx = await harness(adapter)
|
||||||
|
const agent = ctx.agentLoop.create(SessionId('a1'), { provider: 'mock', model: 'mock' })
|
||||||
|
ctx.tools.register(defineContentToolFixture({
|
||||||
|
name: 'finalize',
|
||||||
|
description: '',
|
||||||
|
parameters: {},
|
||||||
|
async execute(_args, exec) {
|
||||||
|
// Steering lands while the concluding tool is still executing.
|
||||||
|
agent.steer({ content: [{ type: 'text', text: 'late steering' }], source: { kind: 'user' } })
|
||||||
|
exec.concludeTurn()
|
||||||
|
return [{ type: 'text', text: 'final' }]
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
send(agent, 'go')
|
||||||
|
await waitForIdle(ctx, agent)
|
||||||
|
|
||||||
|
// The terminal result stands: no extra request reopens the concluded turn.
|
||||||
|
expect(adapter.requests).toHaveLength(1)
|
||||||
|
const events = agent.session.events.map(event => event.type)
|
||||||
|
expect(events.filter(type => type === 'turn/end')).toHaveLength(1)
|
||||||
|
// The steering is durable inside the concluded turn and feeds the NEXT
|
||||||
|
// turn's request instead of being dropped or re-queued.
|
||||||
|
expect(events).toContain('steering/message')
|
||||||
|
|
||||||
|
send(agent, 'follow up')
|
||||||
|
await waitForIdle(ctx, agent)
|
||||||
|
expect(adapter.requests).toHaveLength(2)
|
||||||
|
const texts = adapter.requests[1]!.messages
|
||||||
|
.flatMap(message => message.content)
|
||||||
|
.filter(block => block.type === 'text')
|
||||||
|
.map(block => block.text)
|
||||||
|
expect(texts).toContain('late steering')
|
||||||
|
})
|
||||||
|
|
||||||
it('agent/request waterfall switches models by returning a replacement config; the switch is logged', async () => {
|
it('agent/request waterfall switches models by returning a replacement config; the switch is logged', async () => {
|
||||||
const adapter = new MockAdapter([textResponse('ok')])
|
const adapter = new MockAdapter([textResponse('ok')])
|
||||||
const ctx = await harness(adapter)
|
const ctx = await harness(adapter)
|
||||||
|
|||||||
Reference in New Issue
Block a user