fix(subagent): contain a structuredClone failure on the detached subagent/end path
Review noted the deep-clone of the child output runs inside `onFulfilled`, OUTSIDE emitLifecycle's per-listener containment, and the settle `.then` is `void`ed — so an uncloneable output (a future non-serializable content-block type, or a contract-violating result) would throw and become an UNHANDLED rejection, contradicting the "any throw is contained" guarantee the comment claims. Wrap the clone in try/catch: on failure, log via ctx.logger.warn and emit subagent/end WITHOUT lastAssistantMessage (preserving stopReason/agentType) rather than dropping the event or crashing. Regression proves the unfixed code produces an unhandled rejection.
This commit is contained in:
@@ -207,8 +207,20 @@ export class SubagentService extends Service {
|
||||
// the SAME array reference the caller consumes would let a mutating
|
||||
// `subagent/end` listener corrupt the caller's SubagentResult.output —
|
||||
// breaking the observe-only contract. A snapshot makes the event a
|
||||
// read-only view, not a shared handle.
|
||||
this.emitLifecycle('subagent/end', { provider: name, id: run.id, ...agentType, stopReason: result.stopReason, lastAssistantMessage: structuredClone(result.output) })
|
||||
// read-only view, not a shared handle. The clone is wrapped: it runs
|
||||
// inside `onFulfilled`, OUTSIDE emitLifecycle's per-listener containment,
|
||||
// so an uncloneable value (a future non-serializable content-block type,
|
||||
// or a contract-violating result with no `output`) would otherwise become
|
||||
// an unhandled rejection on this detached `.then`. On clone failure, log
|
||||
// and emit the event WITHOUT lastAssistantMessage rather than dropping the
|
||||
// whole `subagent/end`.
|
||||
let lastAssistantMessage: SubagentResult['output'] | undefined
|
||||
try {
|
||||
lastAssistantMessage = structuredClone(result.output)
|
||||
} catch (error: unknown) {
|
||||
this.ctx.logger.warn(`subagent: could not clone ${name} output for subagent/end: ${String(error)}`)
|
||||
}
|
||||
this.emitLifecycle('subagent/end', { provider: name, id: run.id, ...agentType, stopReason: result.stopReason, ...lastAssistantMessage !== undefined ? { lastAssistantMessage } : {} })
|
||||
},
|
||||
() => { this.emitLifecycle('subagent/end', { provider: name, id: run.id, ...agentType, stopReason: 'error' }) },
|
||||
)
|
||||
|
||||
@@ -277,6 +277,41 @@ describe('SubagentService', () => {
|
||||
expect('lastAssistantMessage' in endInfo).toBe(false) // but no output exists
|
||||
})
|
||||
|
||||
it('contains a structuredClone failure: emits subagent/end without lastAssistantMessage (no unhandled rejection)', async () => {
|
||||
// The clone runs inside onFulfilled, OUTSIDE emitLifecycle's per-listener
|
||||
// containment. An uncloneable output (here a content block carrying a
|
||||
// function) would otherwise throw and become an unhandled rejection on the
|
||||
// detached `.then`. The handler must instead log and emit the event WITHOUT
|
||||
// lastAssistantMessage, still carrying the real stopReason/agentType.
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SubagentService)
|
||||
const warn = vi.fn(); ctx.logger.warn = warn as never
|
||||
// An output value structuredClone cannot handle (a function is uncloneable).
|
||||
const uncloneable = [{ type: 'text', text: 'x', evil: () => 0 }] as unknown as SubagentResult['output']
|
||||
ctx.subagents.registerProvider({
|
||||
name: 'unclone',
|
||||
capabilities: NO_CAPS,
|
||||
start: () => ({
|
||||
id: AgentId('unclone-child'),
|
||||
result: Promise.resolve({ output: uncloneable, stopReason: 'completed' } as SubagentResult),
|
||||
cancel() {},
|
||||
dispose: async () => {},
|
||||
}),
|
||||
})
|
||||
|
||||
const ended = vi.fn()
|
||||
ctx.on('subagent/end', ended)
|
||||
const run = ctx.subagents.start('unclone', baseRequest({ agentType: 'researcher' }))
|
||||
await run.result
|
||||
await Promise.resolve()
|
||||
|
||||
const endInfo = ended.mock.calls[0]![0] as Record<string, unknown>
|
||||
expect(endInfo.stopReason).toBe('completed') // the real outcome is preserved
|
||||
expect(endInfo.agentType).toBe('researcher')
|
||||
expect('lastAssistantMessage' in endInfo).toBe(false) // clone failed → omitted, not crashed
|
||||
expect(warn).toHaveBeenCalledWith(expect.stringContaining('could not clone'))
|
||||
})
|
||||
|
||||
it('emits subagent/end with stopReason "error" when the run result promise rejects', async () => {
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SubagentService)
|
||||
|
||||
Reference in New Issue
Block a user