diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index 40f420208d..1d86ac356d 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -24,6 +24,27 @@ interface ToolMeta { hasOutput: boolean; } +type ToolCallWithNestedContent = Agents.ToolCall & { + subagent_content?: TMessageContentParts[]; +}; + +function hasPendingApprovalInPart(part: TMessageContentParts): boolean { + if (part.type !== ContentTypes.TOOL_CALL) { + return false; + } + const toolCall = part[ContentTypes.TOOL_CALL] as ToolCallWithNestedContent | undefined; + if (!toolCall) { + return false; + } + if (toolCall.approval != null && (toolCall.output?.length ?? 0) === 0) { + return true; + } + return ( + Array.isArray(toolCall.subagent_content) && + toolCall.subagent_content.some(hasPendingApprovalInPart) + ); +} + function getToolMeta(part: TMessageContentParts): ToolMeta | null { if (part.type !== ContentTypes.TOOL_CALL) { return null; @@ -107,18 +128,12 @@ export default function ToolCallGroup({ const mcpIconMap = useMCPIconMap(); const rootRef = useRef(null); const cancelLayoutReconcileRef = useRef<(() => void) | null>(null); + const retainedForPendingApprovalRef = useRef(false); const count = parts.length; const toolMetadata = useMemo(() => parts.map((p) => getToolMeta(p.part)), [parts]); const hasPendingApproval = useMemo( - () => - parts.some(({ part }) => { - if (part.type !== ContentTypes.TOOL_CALL) { - return false; - } - const toolCall = part[ContentTypes.TOOL_CALL] as Agents.ToolCall | undefined; - return toolCall?.approval != null && (toolCall.output?.length ?? 0) === 0; - }), + () => parts.some(({ part }) => hasPendingApprovalInPart(part)), [parts], ); const allCompleted = useMemo( @@ -234,18 +249,37 @@ export default function ToolCallGroup({ if (event.target !== event.currentTarget) { return; } - if (isExpanded || hasPendingApproval) { + if (isExpanded) { return; } - // Approval controls own unsent local form state. Keep unresolved cards - // mounted (the collapsed panel is inert/hidden) so collapsing a batch - // cannot erase decisions the reviewer already made. + if (hasPendingApproval) { + // Approval controls own unsent local form state. Keep unresolved cards + // mounted (the collapsed panel is inert/hidden) so collapsing a batch + // cannot erase decisions the reviewer already made. + retainedForPendingApprovalRef.current = true; + return; + } + retainedForPendingApprovalRef.current = false; setShouldRenderBody(false); notifyLayoutChange(); }, [hasPendingApproval, isExpanded, notifyLayoutChange], ); + useEffect(() => { + if (isExpanded) { + retainedForPendingApprovalRef.current = false; + return; + } + if (!hasPendingApproval && retainedForPendingApprovalRef.current) { + // A completed collapse transition retained this body only to preserve + // approval form state. Release it once the last approval resolves. + retainedForPendingApprovalRef.current = false; + setShouldRenderBody(false); + notifyLayoutChange(); + } + }, [hasPendingApproval, isExpanded, notifyLayoutChange]); + /** Category-aware header verb: subagents and questions read as their own * category (with tense), everything else is the generic "Used N tools". */ const resolveGroupLabel = (): string => { diff --git a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx index 14c66b0f73..10d61e4482 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ToolCallGroup.test.tsx @@ -83,14 +83,14 @@ const makePart = ( }, }) as unknown as TMessageContentParts; -const makePendingApprovalPart = (id: string): TMessageContentParts => +const makeApprovalPart = (id: string, output = ''): TMessageContentParts => ({ type: ContentTypes.TOOL_CALL, [ContentTypes.TOOL_CALL]: { id, name: 'approval_probe', args: {}, - output: '', + output, approval: { actionId: 'action-1', allowed_decisions: ['approve', 'reject'], @@ -98,6 +98,21 @@ const makePendingApprovalPart = (id: string): TMessageContentParts => }, }) as unknown as TMessageContentParts; +const makeSubagentPart = ( + id: string, + subagentContent: TMessageContentParts[], +): TMessageContentParts => + ({ + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { + id, + name: Constants.SUBAGENT, + args: {}, + output: '', + subagent_content: subagentContent, + }, + }) as unknown as TMessageContentParts; + const imageAttachment: TAttachment = { filename: 'foo.png', filepath: '/files/foo.png', @@ -228,8 +243,8 @@ describe('ToolCallGroup image hoisting', () => { it('keeps unresolved approval bodies mounted while the group is collapsed', () => { const approvalParts = [ - { part: makePendingApprovalPart('t1'), idx: 0 }, - { part: makePendingApprovalPart('t2'), idx: 1 }, + { part: makeApprovalPart('t1'), idx: 0 }, + { part: makeApprovalPart('t2'), idx: 1 }, ]; renderGroup({ ...baseProps, @@ -253,6 +268,142 @@ describe('ToolCallGroup image hoisting', () => { expect(screen.getByTestId('approval-1')).toBeInTheDocument(); }); + it('keeps deeply nested unresolved approval bodies mounted while the group is collapsed', () => { + const nestedApprovalParts = [ + { + part: makeSubagentPart('parent', [ + makeSubagentPart('child', [makeApprovalPart('grandchild')]), + ]), + idx: 0, + }, + { part: makePart('sibling'), idx: 1 }, + ]; + renderGroup({ + ...baseProps, + parts: nestedApprovalParts, + renderPart: (_p: TMessageContentParts, idx: number) => ( +
+ {'nested'} +
+ ), + }); + + const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const collapsible = button.nextElementSibling as HTMLElement; + fireEvent.click(button); + fireEvent.transitionEnd(collapsible); + + expect(button).toHaveAttribute('aria-expanded', 'false'); + expect(screen.getByTestId('nested-0')).toBeInTheDocument(); + expect(screen.getByTestId('nested-1')).toBeInTheDocument(); + }); + + it('does not retain a collapsed group for an already resolved nested approval', () => { + const nestedApprovalParts = [ + { + part: makeSubagentPart('parent', [ + makeSubagentPart('child', [makeApprovalPart('grandchild', 'done')]), + ]), + idx: 0, + }, + { part: makePart('sibling'), idx: 1 }, + ]; + renderGroup({ + ...baseProps, + parts: nestedApprovalParts, + renderPart: (_p: TMessageContentParts, idx: number) => ( +
+ {'nested'} +
+ ), + }); + + const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const collapsible = button.nextElementSibling as HTMLElement; + fireEvent.click(button); + fireEvent.transitionEnd(collapsible); + + expect(screen.queryByTestId('resolved-nested-0')).not.toBeInTheDocument(); + expect(screen.queryByTestId('resolved-nested-1')).not.toBeInTheDocument(); + }); + + it('unmounts retained approval bodies after every approval in a collapsed group resolves', async () => { + const renderPart = (_p: TMessageContentParts, idx: number) => ( +
+ {'approval'} +
+ ); + const propsFor = ( + firstOutput = '', + secondOutput = '', + ): React.ComponentProps => ({ + ...baseProps, + parts: [ + { part: makeApprovalPart('t1', firstOutput), idx: 0 }, + { part: makeApprovalPart('t2', secondOutput), idx: 1 }, + ], + renderPart, + }); + const { rerender } = renderGroup(propsFor()); + + const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const collapsible = button.nextElementSibling as HTMLElement; + fireEvent.click(button); + fireEvent.transitionEnd(collapsible); + expect(screen.getByTestId('retained-0')).toBeInTheDocument(); + + rerender( + + + , + ); + expect(screen.getByTestId('retained-0')).toBeInTheDocument(); + expect(screen.getByTestId('retained-1')).toBeInTheDocument(); + + rerender( + + + , + ); + await waitFor(() => { + expect(screen.queryByTestId('retained-0')).not.toBeInTheDocument(); + expect(screen.queryByTestId('retained-1')).not.toBeInTheDocument(); + }); + }); + + it('waits for an active collapse transition before unmounting resolved approval bodies', () => { + const renderPart = (_p: TMessageContentParts, idx: number) => ( +
+ {'approval'} +
+ ); + const propsFor = (output = ''): React.ComponentProps => ({ + ...baseProps, + parts: [ + { part: makeApprovalPart('t1', output), idx: 0 }, + { part: makeApprovalPart('t2', output), idx: 1 }, + ], + renderPart, + }); + const { rerender } = renderGroup(propsFor()); + + const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const collapsible = button.nextElementSibling as HTMLElement; + fireEvent.click(button); + + rerender( + + + , + ); + expect(screen.getByTestId('transitioning-0')).toBeInTheDocument(); + expect(screen.getByTestId('transitioning-1')).toBeInTheDocument(); + + fireEvent.transitionEnd(collapsible); + expect(screen.queryByTestId('transitioning-0')).not.toBeInTheDocument(); + expect(screen.queryByTestId('transitioning-1')).not.toBeInTheDocument(); + }); + it('reconciles layout after the group collapses from an expanded state', async () => { renderGroup(baseProps);