Honor an already-aborted request signal in the subagent driver (Codex review round 2)
A request signal aborted BEFORE the run starts never fires an `abort` event (`addEventListener` only fires on the transition), so the backend-level bridge missed it and ran the child to `completed`. The driver now checks `request.signal?.aborted` at the top of the result path and settles `aborted` without running the child. Regression test proven red on the pre-fix code. Also refresh two stale RFC prose blocks the round-1 fix left behind: the subagent RFC's Problem statement (cited the removed `TODO(sub-agents)` markers and claimed nothing existed yet) and the unify-id RFC's fork/spawn risk bullet (described the seam as "explicitly deferred" via `AgentLoop.create`'s old TODO), now pointing at the realized seam.
This commit is contained in:
@@ -143,6 +143,11 @@ export function startInProcessRun(
|
||||
|
||||
const result: Promise<SubagentResult> = (async () => {
|
||||
try {
|
||||
// A signal already aborted BEFORE the run starts never fires an `abort`
|
||||
// event (`addEventListener` only fires on the transition), so the listener
|
||||
// above won't catch it — settle `aborted` without running the child rather
|
||||
// than completing an already-cancelled request.
|
||||
if (request.signal?.aborted) return { output: [], stopReason: 'aborted' }
|
||||
child.send(request.prompt)
|
||||
await child.whenIdle()
|
||||
return readResult(child, seedLength, cancelled)
|
||||
|
||||
@@ -128,6 +128,22 @@ describe('dsh-subagent-spawn', () => {
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('settles aborted (without running the child) when the request signal is ALREADY aborted', async () => {
|
||||
// Regression: a signal aborted BEFORE the run starts never fires an `abort`
|
||||
// event, so the listener can't catch it. The driver must check the
|
||||
// already-aborted case up front and settle `aborted` without running the
|
||||
// child — otherwise an already-cancelled request runs to `completed`. The
|
||||
// empty script proves the child's model is never called.
|
||||
const controller = new AbortController()
|
||||
controller.abort()
|
||||
const { ctx, parent } = await setup([])
|
||||
const run = ctx.subagents.start('spawn', { prompt: [{ type: 'text', text: 'p' }], parent, signal: controller.signal })
|
||||
const result = await run.result
|
||||
expect(result.stopReason).toBe('aborted')
|
||||
expect(result.output).toEqual([])
|
||||
await run.dispose()
|
||||
})
|
||||
|
||||
it('cancelling BEFORE the child turn starts settles aborted, not error', async () => {
|
||||
// Regression: a cancel landing in the pre-turn window clears the queued
|
||||
// prompt before any `turn/end` is logged. Deriving the stop reason from
|
||||
|
||||
Reference in New Issue
Block a user