fix: address codex review round 3
- Strict steer additionally requires an OPEN STEP: between steps the loop may be awaiting its continuation/turn-stop checkpoints, where pending steering was already folded and a terminal stop discards a later arrival. A message accepted during an open step is drained and recorded at that step's settlement before any terminal decision, so the acknowledged-then-discarded window is closed. New keyless test holds agent/turn-stop open and pins the rejection. - tool-subagent-control README: distinguish synchronous not-delivered errors from started-Task failures (unknown/foreign/descriptor-less ids settle the started Task as failed), and drop the claim that the completion notice carries the child's response.
This commit is contained in:
@@ -30,7 +30,7 @@ The required request signal covers both startup and the live run. Before publica
|
||||
|
||||
After fulfillment, the caller owns the run. Provider-plugin unload does not revoke it. `dispose()` removes the live abort listener, records cancellation, and delegates to the returned `AgentHandle.dispose()`, whose memoized quiescence transaction stops the loop, removes the agent and session, and unwinds scoped registrations. Cancellation owns every non-completed in-flight outcome and reports `aborted`; an already-completed turn remains completed.
|
||||
|
||||
Runs expose the strict `steer` capability: the synchronous checks and the `Agent.steer()` call share one frame, so delivery joins the observed turn or throws. Delivery requires `AgentStatus.running`, an open turn in the child log (status stays `running` through a closed turn's durability flush, where the loop would strand the message), and no committed structured capture (whose terminal stop makes the loop discard late steering). The Agent-level idle fallback (queue and start a new turn) is deliberately not reachable through the run — that would start an untracked turn after the run's result was read.
|
||||
Runs expose the strict `steer` capability: the synchronous checks and the `Agent.steer()` call share one frame, so delivery joins the observed turn or throws. Delivery requires `AgentStatus.running`, an open turn in the child log (status stays `running` through a closed turn's durability flush, where the loop would strand the message), an open step (between steps the loop may sit at its continuation/turn-stop checkpoints, where steering was already folded and a terminal stop discards a later arrival; a message accepted during an open step is recorded at that step's settlement before any terminal decision), and no committed structured capture (whose terminal stop makes the loop discard late steering). The Agent-level idle fallback (queue and start a new turn) is deliberately not reachable through the run — that would start an untracked turn after the run's result was read.
|
||||
|
||||
## Spawn and fork inputs
|
||||
|
||||
|
||||
@@ -276,6 +276,19 @@ function driveTurn(
|
||||
if (lastBoundary?.type !== 'turn/start') {
|
||||
throw new Error(`subagent child "${childId}" turn has already closed; the message was not delivered`)
|
||||
}
|
||||
// Terminal turn-stops only run between steps: with no step open, the
|
||||
// loop may be awaiting its continuation/turn-stop checkpoints, where
|
||||
// pending steering was already folded and a terminal decision discards
|
||||
// a later arrival. A message accepted during an OPEN step is instead
|
||||
// drained and recorded at that step's settlement checkpoint before any
|
||||
// terminal decision (cancellation remains the documented shared-outcome
|
||||
// race).
|
||||
const lastStep = child.session.events.findLast(
|
||||
event => event.type === 'step/start' || event.type === 'step/end',
|
||||
)
|
||||
if (lastStep?.type !== 'step/start') {
|
||||
throw new Error(`subagent child "${childId}" is between steps; the message was not delivered`)
|
||||
}
|
||||
// A committed structured capture makes the pending `agent/turn-stop`
|
||||
// checkpoint terminal, and the loop then discards late steering. The
|
||||
// capture is synchronously observable, so reject rather than
|
||||
|
||||
@@ -258,6 +258,35 @@ describe('startInProcessRun', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('strict steer rejects the between-steps window where a terminal turn-stop discards steering', async () => {
|
||||
// Hold `agent/turn-stop` open: the step has closed, pending steering was
|
||||
// already folded into the continuation decision, and a terminal stop
|
||||
// would discard a message arriving now — the exact window an
|
||||
// acknowledged delivery would be a lie.
|
||||
const { ctx, parent } = await setup([textResponse('quick')])
|
||||
let releaseStop: (() => void) | undefined
|
||||
ctx.on('agent/turn-stop', (agent) => {
|
||||
if (agent.session.header.parentSession === undefined || releaseStop !== undefined) return undefined
|
||||
return new Promise((resolve) => {
|
||||
releaseStop = () => { resolve(undefined) }
|
||||
})
|
||||
})
|
||||
const run = await startInProcessRun(request(parent), {})
|
||||
const child = ctx.agents.get(run.id)!
|
||||
await new Promise<void>((resolve) => {
|
||||
const timer = setInterval(() => {
|
||||
if (releaseStop !== undefined) { clearInterval(timer); resolve() }
|
||||
}, 5)
|
||||
})
|
||||
expect(child.status).toBe('running')
|
||||
expect(() => { run.steer!([{ type: 'text', text: 'too late for this turn' }]) })
|
||||
.toThrow(/between steps; the message was not delivered/)
|
||||
releaseStop!()
|
||||
await run.result
|
||||
expect(child.session.events.some(event => event.type === 'steering/message')).toBe(false)
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('strict steer rejects the closed-turn flush window where the loop discards steering', async () => {
|
||||
// Hold the turn-end durability flush open: the turn has closed in the log
|
||||
// and status is still `running`, exactly the window where the loop would
|
||||
|
||||
@@ -24,11 +24,11 @@ Prefix-stable; the schema does not change at runtime.
|
||||
|
||||
#### What the model sees
|
||||
|
||||
`message delivered to running task <taskId>` when the message joined the running activation, or `message started task <taskId> continuing subagent <subagent_id>` when it cold-resumed the child. Failures are errored results whose message states the message was not delivered (unknown or foreign child, ownership conflict, settlement race, no live-delivery capability).
|
||||
`message delivered to running task <taskId>` when the message joined the running activation, or `message started task <taskId> continuing subagent <subagent_id>` when it started a cold-resume activation. Synchronous routing failures — an ownership conflict, a lost steering race, no live-delivery capability — are errored results whose message states the message was not delivered. An absent activation always reports `started`: lookup runs inside that Task, so an unknown, foreign, or descriptor-less child surfaces as the started Task settling `failed` (read through `task_output`), not as an errored `send_message` result.
|
||||
|
||||
#### Token effect
|
||||
|
||||
One short acknowledgement per call; the child's response enters parent history only when collected through `task_output` or injected by the task completion notice.
|
||||
One short acknowledgement per call; the child's response enters parent history only when collected through `task_output` (the completion notice is a status line, never the response).
|
||||
|
||||
#### KV Cache effect
|
||||
|
||||
|
||||
Reference in New Issue
Block a user