Skip to content

Commit 87f14aa

Browse files
authored
[codex] Hide empty subagent spawn errors (#713)
1 parent 702a406 commit 87f14aa

2 files changed

Lines changed: 163 additions & 35 deletions

File tree

cli/src/utils/__tests__/sdk-event-handlers.test.ts

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,104 @@ describe('sdk-event-handlers', () => {
369369
expect(getStreamingAgents().size).toBe(0)
370370
})
371371

372+
test('hides spawn_agents error placeholders with no user-facing output', () => {
373+
const { ctx, getMessages, getStreamingAgents } = createTestContext()
374+
ctx.message.updater.addBlock(
375+
createAgentBlock({
376+
agentId: 'tool-1-0',
377+
agentType: 'basher',
378+
spawnToolCallId: 'tool-1',
379+
spawnIndex: 0,
380+
}),
381+
)
382+
ctx.streaming.setStreamingAgents(() => new Set(['tool-1-0']))
383+
384+
const handleEvent = createEventHandler(ctx)
385+
const toolResultEvent: ToolResultEvent = {
386+
type: 'tool_result',
387+
toolCallId: 'tool-1',
388+
toolName: 'spawn_agents',
389+
output: [
390+
{
391+
type: 'json',
392+
value: [
393+
{
394+
agentName: 'basher',
395+
value: {
396+
errorMessage:
397+
'Error spawning agent: Invalid params for agent basher',
398+
},
399+
},
400+
],
401+
},
402+
],
403+
}
404+
handleEvent(toolResultEvent)
405+
406+
expect(getMessages()[0].blocks).toEqual([])
407+
expect(getStreamingAgents().size).toBe(0)
408+
})
409+
410+
test('renders spawn_agents error content when agent already streamed output', () => {
411+
const { ctx, getMessages, getStreamingAgents } = createTestContext()
412+
ctx.message.updater.updateAiMessageBlocks(() => [
413+
{
414+
type: 'agent',
415+
agentId: 'tool-1-0',
416+
agentName: 'Basher',
417+
agentType: 'basher',
418+
content: '',
419+
status: 'running',
420+
blocks: [
421+
{
422+
type: 'text',
423+
content: 'Checking files...',
424+
textType: 'text',
425+
},
426+
],
427+
initialPrompt: '',
428+
spawnToolCallId: 'tool-1',
429+
spawnIndex: 0,
430+
} as any,
431+
])
432+
ctx.streaming.setStreamingAgents(() => new Set(['tool-1-0']))
433+
434+
const handleEvent = createEventHandler(ctx)
435+
const toolResultEvent: ToolResultEvent = {
436+
type: 'tool_result',
437+
toolCallId: 'tool-1',
438+
toolName: 'spawn_agents',
439+
output: [
440+
{
441+
type: 'json',
442+
value: [
443+
{
444+
agentName: 'basher',
445+
value: {
446+
errorMessage:
447+
'Error spawning agent: Invalid params for agent basher',
448+
},
449+
},
450+
],
451+
},
452+
],
453+
}
454+
handleEvent(toolResultEvent)
455+
456+
const agentBlock = (getMessages()[0].blocks ?? [])[0] as AgentContentBlock
457+
expect(agentBlock.status).toBe('complete')
458+
expect(agentBlock.blocks).toHaveLength(2)
459+
expect(agentBlock.blocks?.[0]).toMatchObject({
460+
type: 'text',
461+
content: 'Checking files...',
462+
})
463+
expect(agentBlock.blocks?.[1]).toMatchObject({
464+
type: 'text',
465+
content: 'Error spawning agent: Invalid params for agent basher',
466+
})
467+
expect(getStreamingAgents().size).toBe(0)
468+
})
469+
372470
test('handles spawn_agents tool results for agents with tool blocks (lastMessage mode)', () => {
373471
const { ctx, getMessages, getStreamingAgents } = createTestContext()
374472

cli/src/utils/sdk-event-handlers.ts

Lines changed: 65 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -358,50 +358,79 @@ const handleToolCall = (state: EventHandlerState, event: PrintModeToolCall) => {
358358
/**
359359
* Recursively finds and updates agent blocks that match a spawn_agents tool call.
360360
*/
361-
const updateSpawnAgentBlocks = (
362-
blocks: ContentBlock[],
361+
const updateSpawnAgentBlock = (
362+
block: ContentBlock,
363363
toolCallId: string,
364364
results: any[],
365-
): ContentBlock[] => {
366-
return blocks.map((block) => {
367-
if (block.type !== 'agent') {
368-
return block
369-
}
365+
): ContentBlock | null => {
366+
if (block.type !== 'agent') {
367+
return block
368+
}
370369

371-
if (block.spawnToolCallId === toolCallId && block.spawnIndex !== undefined && block.blocks) {
372-
const result = results[block.spawnIndex]
373-
374-
if (result?.value) {
375-
const { content, hasError } = extractSpawnAgentResultContent(result.value)
376-
// Check if the agent already streamed text content (e.g., basher).
377-
// Agents like thinker return all output at the end via lastMessage,
378-
// so we should add final content even if they have tool blocks.
379-
const hasStreamedTextContent = block.blocks.some(
380-
(b) => b.type === 'text' && b.textType === 'text'
381-
)
382-
const finalBlocks = content && !hasStreamedTextContent
383-
? [...block.blocks, { type: 'text', content } as ContentBlock]
384-
: block.blocks
385-
if (hasError || finalBlocks.length > 0) {
386-
return {
387-
...block,
388-
blocks: finalBlocks,
389-
status: hasError ? ('failed' as const) : ('complete' as const),
390-
}
370+
const spawnIndex = block.spawnIndex
371+
const childBlocks = block.blocks
372+
const isSpawnResultTarget =
373+
block.spawnToolCallId === toolCallId &&
374+
spawnIndex !== undefined &&
375+
childBlocks
376+
377+
if (isSpawnResultTarget) {
378+
const result = results[spawnIndex]
379+
if (result?.value) {
380+
const { content, hasError } = extractSpawnAgentResultContent(result.value)
381+
382+
if (hasError) {
383+
if (childBlocks.length === 0) {
384+
return null
385+
}
386+
387+
return {
388+
...block,
389+
blocks: content
390+
? [...childBlocks, { type: 'text', content } as ContentBlock]
391+
: childBlocks,
392+
status: 'complete' as const,
391393
}
392394
}
393-
}
394395

395-
// Recursively process nested agent blocks
396-
if (block.blocks?.length) {
397-
const updatedNestedBlocks = updateSpawnAgentBlocks(block.blocks, toolCallId, results)
398-
if (updatedNestedBlocks !== block.blocks) {
399-
return { ...block, blocks: updatedNestedBlocks }
396+
// Agents like thinker return all output at the end via lastMessage,
397+
// while agents like basher may have already streamed their text.
398+
const hasStreamedTextContent = childBlocks.some(
399+
(b) => b.type === 'text' && b.textType === 'text',
400+
)
401+
const finalBlocks =
402+
content && !hasStreamedTextContent
403+
? [...childBlocks, { type: 'text', content } as ContentBlock]
404+
: childBlocks
405+
406+
if (finalBlocks.length > 0) {
407+
return {
408+
...block,
409+
blocks: finalBlocks,
410+
status: 'complete' as const,
411+
}
400412
}
401413
}
414+
}
402415

416+
if (!childBlocks?.length) {
403417
return block
404-
})
418+
}
419+
420+
return {
421+
...block,
422+
blocks: updateSpawnAgentBlocks(childBlocks, toolCallId, results),
423+
}
424+
}
425+
426+
const updateSpawnAgentBlocks = (
427+
blocks: ContentBlock[],
428+
toolCallId: string,
429+
results: any[],
430+
): ContentBlock[] => {
431+
return blocks
432+
.map((block) => updateSpawnAgentBlock(block, toolCallId, results))
433+
.filter((block): block is ContentBlock => block !== null)
405434
}
406435

407436
const handleSpawnAgentsResult = (
@@ -433,7 +462,8 @@ const handleToolResult = (
433462
)
434463

435464
const firstOutput = event.output?.[0]
436-
const firstOutputValue = firstOutput && 'value' in firstOutput ? firstOutput.value : undefined
465+
const firstOutputValue =
466+
firstOutput && 'value' in firstOutput ? firstOutput.value : undefined
437467
const isSpawnAgentsResult =
438468
Array.isArray(firstOutputValue) &&
439469
firstOutputValue.some((v: any) => v?.agentName || v?.agentType)

0 commit comments

Comments
 (0)