From 2fb33ad0b4806f0a0e21a878923beeffe4b92a8a Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 31 Jul 2026 02:24:57 +0200 Subject: [PATCH] feat(client): outcome-based tool group summaries Group headers now describe the activity performed: homogeneous search groups read "Searched the web" or "Searched web and files", mixed groups read "Ran N actions" with per-label repeat counts (Create File x2) and a failure tally in the detail line. Generic tool rows resolve their label through getToolDisplayLabel, the OAuth trust warning moved into a shared ToolAuthWarning that a group renders once instead of per call, and the group body is a flex column so row margins stop collapsing unevenly between rows. --- .../Chat/Messages/Content/ToolCall.tsx | 32 ++- .../Chat/Messages/Content/ToolCallGroup.tsx | 244 +++++++++++++----- .../ContentParts.integration.test.tsx | 23 +- .../Content/__tests__/ToolCall.test.tsx | 55 ++++ .../Content/__tests__/ToolCallGroup.test.tsx | 242 +++++++++++++++-- .../components/Chat/Messages/Content/auth.tsx | 22 ++ 6 files changed, 504 insertions(+), 114 deletions(-) create mode 100644 client/src/components/Chat/Messages/Content/auth.tsx diff --git a/client/src/components/Chat/Messages/Content/ToolCall.tsx b/client/src/components/Chat/Messages/Content/ToolCall.tsx index 3ca18c3771..e031699651 100644 --- a/client/src/components/Chat/Messages/Content/ToolCall.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCall.tsx @@ -1,7 +1,6 @@ import { useMemo, useState, useEffect, useCallback } from 'react'; import { useRecoilValue } from 'recoil'; import { Button } from '@librechat/client'; -import { TriangleAlert } from 'lucide-react'; import { Constants, dataService, @@ -12,6 +11,8 @@ import { import type { TAttachment, PartMetadata } from 'librechat-data-provider'; import { useLocalize, useProgress, useExpandCollapse, useLazyCollapseBody } from '~/hooks'; import { ToolIcon, getToolIconType, isError } from './ToolOutput'; +import { cn, getToolDisplayLabel, logger } from '~/utils'; +import { toolPanelSpacingClassName } from './disclosure'; import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { resolveToolCallPhase } from '~/utils/toolCallPhase'; import { useToolCallIntent } from './Parts/intent'; @@ -20,6 +21,7 @@ import ToolCallInfo from './ToolCallInfo'; import ProgressText from './ProgressText'; import { TOOL_ROW_CLASSES } from './rows'; import { logger } from '~/utils'; +import { ToolAuthWarning } from './auth'; import store from '~/store'; export default function ToolCall({ @@ -116,6 +118,10 @@ export default function ToolCall({ }, [name, parsedAuthUrl, mcpServerNames]); const toolIconType = useMemo(() => getToolIconType(name), [name]); + const displayFunctionName = useMemo( + () => getToolDisplayLabel(function_name, localize, mcpServerNames), + [function_name, localize, mcpServerNames], + ); const mcpIconMap = useMCPIconMap(); const mcpIconUrl = isMCPToolCall ? mcpIconMap.get(mcpServerName) : undefined; @@ -247,12 +253,12 @@ export default function ToolCall({ return intent; } if (isMCPToolCall === true) { - return localize('com_assistants_completed_function', { 0: function_name }); + return localize('com_assistants_completed_function', { 0: displayFunctionName }); } if (domain != null && domain && domain.length !== Constants.ENCODED_DOMAIN_LENGTH) { return localize('com_assistants_completed_action', { 0: domain }); } - return localize('com_assistants_completed_function', { 0: function_name }); + return localize('com_assistants_completed_function', { 0: displayFunctionName }); }; if (!isLast && (!function_name || function_name.length === 0) && !output) { @@ -268,8 +274,8 @@ export default function ToolCall({ {(() => { if (phase === 'running') { - return function_name - ? localize('com_assistants_running_var', { 0: function_name }) + return displayFunctionName + ? localize('com_assistants_running_var', { 0: displayFunctionName }) : localize('com_assistants_running_action'); } return getFinishedText(); @@ -281,8 +287,8 @@ export default function ToolCall({ onClick={handleToggleInfo} inProgressText={ intent ?? - (function_name - ? localize('com_assistants_running_var', { 0: function_name }) + (displayFunctionName + ? localize('com_assistants_running_var', { 0: displayFunctionName }) : localize('com_assistants_running_action')) } authText={ @@ -307,7 +313,12 @@ export default function ToolCall({ >
{hasInfo && shouldRenderBody && ( -
+
)} @@ -325,10 +336,7 @@ export default function ToolCall({ {localize('com_ui_sign_in_to_domain', { 0: authDomain })}
-

-

+
)} {!hideAttachments && attachments && attachments.length > 0 && ( diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index af6c78925f..d4c661677f 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -18,11 +18,12 @@ import { getActivityLabelText, } from '~/utils'; import { useLocalize, useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks'; -import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; +import { ToolAuthWarning, ToolAuthWarningContext } from './auth'; +import { AttachmentGroup, ReasoningCompact } from './Parts'; +import { isError, StackedToolIcons } from './ToolOutput'; import { isBashProgrammaticToolCall } from './routing'; import { ASK_USER_QUESTION } from '~/utils/approval'; -import { StackedToolIcons } from './ToolOutput'; -import { AttachmentGroup, ReasoningCompact } from './Parts'; +import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { ROW_GLYPH_SLOT } from './rows'; import store from '~/store'; @@ -30,6 +31,30 @@ interface ToolMeta { name: string; iconName: string; hasOutput: boolean; + failed: boolean; +} + +function hasFailedOutput(output: unknown): boolean { + return typeof output === 'string' && isError(output); +} + +function hasPendingAuth(part: TMessageContentParts, isSubmitting: boolean): boolean { + if (part.type !== ContentTypes.TOOL_CALL) { + return false; + } + const toolCall = part[ContentTypes.TOOL_CALL]; + if (!toolCall || !('args' in toolCall)) { + return false; + } + const standardToolCall = toolCall as Agents.ToolCall & { progress?: number }; + const progress = standardToolCall.progress ?? 0.1; + + return ( + typeof standardToolCall.auth === 'string' && + standardToolCall.auth.length > 0 && + progress < 1 && + (isSubmitting || hasFailedOutput(standardToolCall.output)) + ); } function getToolMeta(part: TMessageContentParts): ToolMeta | null { @@ -52,7 +77,7 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { const completed = !!tc.output || tc.progress === 1; const name = tc.name ?? ''; const iconName = isBashProgrammaticToolCall(name, tc.args) ? Tools.bash_tool : name; - return { name, iconName, hasOutput: completed }; + return { name, iconName, hasOutput: completed, failed: hasFailedOutput(tc.output) }; } if (toolCall.type === ToolCallTypes.CODE_INTERPRETER) { @@ -61,20 +86,28 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { name: 'code_interpreter', iconName: 'code_interpreter', hasOutput: (ci?.outputs?.length ?? 0) > 0, + failed: false, }; } if (toolCall.type === ToolCallTypes.RETRIEVAL || toolCall.type === ToolCallTypes.FILE_SEARCH) { + const output = (toolCall as { output?: string }).output; return { name: 'file_search', iconName: 'file_search', - hasOutput: !!(toolCall as { output?: string }).output, + hasOutput: !!output, + failed: hasFailedOutput(output), }; } if (toolCall.type === ToolCallTypes.FUNCTION && ToolCallTypes.FUNCTION in toolCall) { const fn = (toolCall as FunctionToolCall).function; - return { name: fn.name, iconName: fn.name, hasOutput: !!fn.output }; + return { + name: fn.name, + iconName: fn.name, + hasOutput: !!fn.output, + failed: hasFailedOutput(fn.output), + }; } return null; @@ -131,10 +164,9 @@ export default function ToolCallGroup({ const cancelLayoutReconcileRef = useRef<(() => void) | null>(null); const retainedForPendingApprovalRef = useRef(false); - /** Labeled activity blocks also contain THINK parts, which yield null - * metadata. Narrow to tool entries once: they alone drive the count, the - * completion check, and the icon strip — passing a null-derived empty - * name to StackedToolIcons would render a phantom generic tool icon. */ + /** `parts` may include interleaved reasoning ("Thoughts") parts that render + * inside the body but are not actions — count and summarize only the real + * tool calls so the header and stacked icons stay accurate. */ const toolMetadata = useMemo( () => parts.map((p) => getToolMeta(p.part)).filter((m): m is ToolMeta => m != null), [parts], @@ -161,9 +193,57 @@ export default function ToolCallGroup({ () => labelSettled || toolMetadata.every((m) => m.hasOutput === true), [toolMetadata, labelSettled], ); - const toolNames = useMemo(() => toolMetadata.map((m) => m.name), [toolMetadata]); const iconToolNames = useMemo(() => toolMetadata.map((m) => m.iconName), [toolMetadata]); + /** Turn raw tool ids into an activity summary. Repeated calls retain their + * count (`Web Search ×3`) instead of disappearing behind de-duplication, + * while the primary label can use an outcome verb such as "Searched". */ + const activitySummary = useMemo(() => { + const labelCounts = new Map(); + let webSearchCount = 0; + let fileSearchCount = 0; + let subagentCount = 0; + let askQuestionCount = 0; + let failedCount = 0; + + for (const tool of toolMetadata) { + if (tool.name === Tools.web_search) { + webSearchCount++; + } else if (tool.name === 'file_search' || tool.name === 'retrieval') { + fileSearchCount++; + } else if (tool.name === Constants.SUBAGENT) { + subagentCount++; + } else if (tool.name === ASK_USER_QUESTION) { + askQuestionCount++; + } + if (tool.failed) { + failedCount++; + } + if (!tool.name) { + continue; + } + const label = getToolDisplayLabel(tool.name, localize, mcpServerNames); + labelCounts.set(label, (labelCounts.get(label) ?? 0) + 1); + } + + const labels = Array.from(labelCounts, ([label, labelCount]) => + labelCount > 1 ? `${label} ×${labelCount}` : label, + ); + const toolNameSummary = + labels.length <= 3 + ? labels.join(', ') + : `${labels.slice(0, 3).join(', ')}, +${labels.length - 3}`; + + return { + webSearchCount, + fileSearchCount, + subagentCount, + askQuestionCount, + failedCount, + toolNameSummary, + }; + }, [toolMetadata, localize, mcpServerNames]); + /** Reasoning interleaved with the tool calls renders inside the body but is * hidden while collapsed — note it in the header's accessible label so screen * readers know the group also contains thoughts. */ @@ -175,11 +255,7 @@ export default function ToolCallGroup({ /** Subagent tool calls get their own label verb ("Running/Ran N agents") * since "Used N tools" reads oddly when the "tools" are actually child * agents. `subagentCount === count` ⇒ the group is 100% subagents. */ - const subagentCount = useMemo( - () => toolNames.filter((n) => n === Constants.SUBAGENT).length, - [toolNames], - ); - const allSubagents = subagentCount > 0 && subagentCount === count; + const allSubagents = activitySummary.subagentCount > 0 && activitySummary.subagentCount === count; /** Past-tense label once the parent stream is no longer live OR every * child has a terminal signal (output / progress === 1). Without the * `!isSubmitting` branch, a cancelled or errored subagent that never @@ -192,39 +268,20 @@ export default function ToolCallGroup({ /** `ask_user_question` calls form their own category, mirroring subagents: * a homogeneous group reads "Asking/Asked N questions" (never "Used N * tools — ask_user_question") with a question glyph. */ - const askQuestionCount = useMemo( - () => toolNames.filter((n) => n === ASK_USER_QUESTION).length, - [toolNames], - ); - const allAskQuestions = askQuestionCount > 0 && askQuestionCount === count; + const allAskQuestions = + activitySummary.askQuestionCount > 0 && activitySummary.askQuestionCount === count; /** Past tense once the turn is settled — matches the Asking/Asked record * card. While a multi-question turn streams, the still-open question's * tool_call part has no output yet, so keep the present tense. */ const askQuestionsDone = allAskQuestions && (allCompleted || !isSubmitting); - const toolNameSummary = useMemo(() => { - const seen = new Set(); - const labels: string[] = []; - for (const rawName of toolNames) { - if (!rawName) continue; - const label = getToolDisplayLabel(rawName, localize, mcpServerNames); - if (!seen.has(label)) { - seen.add(label); - labels.push(label); - } - } - if (labels.length <= 3) { - return labels.join(', '); - } - return `${labels.slice(0, 3).join(', ')}, +${labels.length - 3}`; - }, [toolNames, localize, mcpServerNames]); /** For a single-tool group, lead with the tool's own (capitalized) label * instead of the generic "Used 1 tool — name", which reads awkwardly. */ const singleToolLabel = useMemo(() => { - const raw = getToolDisplayLabel(toolNames[0] ?? '', localize); + const raw = getToolDisplayLabel(toolMetadata[0]?.name ?? '', localize); return raw ? raw.charAt(0).toUpperCase() + raw.slice(1) : ''; - }, [toolNames, localize]); + }, [toolMetadata, localize]); const autoExpand = useRecoilValue(store.autoExpandTools); /** A labeled activity block is summarized by its header, so it collapses @@ -322,8 +379,13 @@ export default function ToolCallGroup({ } }, [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 searchCount = activitySummary.webSearchCount + activitySummary.fileSearchCount; + const searchesOnly = count > 0 && searchCount === count; + const groupDone = allCompleted || !isSubmitting; + + /** Outcome-first header verb. Homogeneous searches, subagents, and questions + * read as the activity the assistant performed. Mixed implementation-level + * tool calls fall back to the user-facing concept of "actions". */ const resolveGroupLabel = (): string => { if (allSubagents) { if (count === 1) { @@ -343,15 +405,46 @@ export default function ToolCallGroup({ ? localize('com_ui_asked_n_questions', { 0: String(count) }) : localize('com_ui_asking_n_questions', { 0: String(count) }); } + if (activitySummary.webSearchCount === count) { + return localize(groupDone ? 'com_ui_web_searched' : 'com_ui_web_searching'); + } + if (activitySummary.fileSearchCount === count) { + return localize(groupDone ? 'com_ui_retrieved_files' : 'com_ui_searching_files'); + } + if (searchesOnly) { + return localize( + groupDone ? 'com_ui_searched_web_and_files' : 'com_ui_searching_web_and_files', + ); + } if (count === 1) { return singleToolLabel || localize('com_ui_used_one_tool'); } - return localize('com_ui_used_n_tools', { 0: String(count) }); + return localize(groupDone ? 'com_ui_ran_n_actions' : 'com_ui_running_n_actions', { + 0: String(count), + }); }; /** The generated line wins over the generic category verb — but only once * it exists. An unfilled label part leaves the block rendering exactly as * it would without the feature. */ const groupLabel = activityLabelText.length > 0 ? activityLabelText : resolveGroupLabel(); + const groupDetailParts: string[] = []; + if (searchesOnly && count > 1) { + groupDetailParts.push(localize('com_ui_n_searches', { 0: String(count) })); + } else if (!allSubagents && !allAskQuestions && count > 1) { + groupDetailParts.push(activitySummary.toolNameSummary); + } + if (activitySummary.failedCount > 0) { + groupDetailParts.push( + localize( + activitySummary.failedCount === 1 ? 'com_ui_one_action_failed' : 'com_ui_n_actions_failed', + { 0: String(activitySummary.failedCount) }, + ), + ); + } + const groupDetail = groupDetailParts.filter(Boolean).join(' · '); + const groupAriaLabel = [groupLabel, groupDetail, hasReasoning ? localize('com_ui_thoughts') : ''] + .filter(Boolean) + .join(', '); /** Single category glyph for homogeneous groups (else StackedToolIcons). */ const CategoryIcon = allSubagents ? Users : MessageCircleQuestion; @@ -359,6 +452,10 @@ export default function ToolCallGroup({ () => isSubmitting && toolMetadata.some((m) => m && !m.hasOutput), [toolMetadata, isSubmitting], ); + const hasPendingAuthRequest = useMemo( + () => parts.some(({ part }) => hasPendingAuth(part, isSubmitting)), + [parts, isSubmitting], + ); useEffect(() => { if (hasActiveToolCall && !userOverride && !suppressAutoExpand) { @@ -375,7 +472,7 @@ export default function ToolCallGroup({ className="inline-flex h-auto w-full items-center justify-start gap-2 rounded-none bg-transparent p-0 py-1 text-text-secondary hover:bg-transparent hover:text-text-secondary focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-border-heavy focus-visible:ring-offset-0" onClick={handleToggle} aria-expanded={isExpanded} - aria-label={hasReasoning ? `${groupLabel}, ${localize('com_ui_thoughts')}` : groupLabel} + aria-label={groupAriaLabel} > {allSubagents || allAskQuestions ? ( /** Homogeneous category groups get a single category glyph instead @@ -413,13 +510,12 @@ export default function ToolCallGroup({ > {groupLabel} - {/** Hide the tool-name summary for pure-category groups (subagents / - * questions) — every entry deduplicates to the same token, which - * adds noise without info. Single-tool groups already use the tool - * name as their label, while mixed groups keep the summary. */} - {toolNameSummary && !allSubagents && !allAskQuestions && count > 1 && ( - - · {toolNameSummary} + {groupDetail && ( + + · {groupDetail} )} {shouldRenderBody && (
-
- {parts.map(({ part, idx }) => { - if (part.type === ContentTypes.THINK) { - const think = part.think; - const reasoning = typeof think === 'string' ? think : (think?.value ?? ''); - const streaming = isSubmitting && idx === lastContentIdx; - const label = streaming - ? localize('com_ui_thinking') - : localize('com_ui_thoughts'); - return ( - - ); - } - return renderPart(part, idx, isLast && idx === lastContentIdx, handleToolExpand); - })} -
+ +
+ {parts.map(({ part, idx }, partIndex) => { + if (part.type === ContentTypes.THINK) { + const think = part.think; + const reasoning = typeof think === 'string' ? think : (think?.value ?? ''); + const streaming = isSubmitting && idx === lastContentIdx; + const isAfterTool = + partIndex > 0 && parts[partIndex - 1]?.part.type === ContentTypes.TOOL_CALL; + const label = streaming + ? localize('com_ui_thinking') + : localize('com_ui_thoughts'); + return ( + + ); + } + return renderPart(part, idx, isLast && idx === lastContentIdx, handleToolExpand); + })} +
+ + {hasPendingAuthRequest && }
)} diff --git a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx index 45a5fa814f..7ec40c8928 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ContentParts.integration.test.tsx @@ -7,8 +7,11 @@ import ContentParts from '../ContentParts'; jest.mock('~/hooks', () => ({ useLocalize: () => (key: string, values?: Record) => { - if (key === 'com_ui_used_n_tools') { - return `Used ${values?.[0]} tools`; + if (key === 'com_ui_ran_n_actions') { + return `Ran ${values?.[0]} actions`; + } + if (key === 'com_ui_running_n_actions') { + return `Running ${values?.[0]} actions`; } return key; }, @@ -312,7 +315,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - const toggle = screen.getByRole('button', { name: 'Used 2 tools' }); + const toggle = screen.getByRole('button', { name: /^Ran 2 actions/ }); expect(toggle).toHaveAttribute('aria-expanded', 'false'); fireEvent.click(toggle); @@ -324,7 +327,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - expect(screen.getByRole('button', { name: 'Used 2 tools' })).toHaveAttribute( + expect(screen.getByRole('button', { name: /^Ran 2 actions/ })).toHaveAttribute( 'aria-expanded', 'true', ); @@ -418,7 +421,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - const toggle = screen.getByRole('button', { name: 'Used 2 tools' }); + const toggle = screen.getByRole('button', { name: /^Running 2 actions/ }); expect(toggle).toHaveAttribute('aria-expanded', 'true'); fireEvent.click(screen.getAllByTestId('progress-text')[0]); @@ -434,7 +437,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - expect(screen.getByRole('button', { name: 'Used 2 tools' })).toHaveAttribute( + expect(screen.getByRole('button', { name: /^Ran 2 actions/ })).toHaveAttribute( 'aria-expanded', 'true', ); @@ -449,7 +452,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - const toggle = screen.getByRole('button', { name: 'Used 2 tools' }); + const toggle = screen.getByRole('button', { name: /^Ran 2 actions/ }); expect(toggle).toHaveAttribute('aria-expanded', 'false'); fireEvent.click(toggle); @@ -461,7 +464,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - expect(screen.getByRole('button', { name: 'Used 2 tools' })).toHaveAttribute( + expect(screen.getByRole('button', { name: /^Ran 2 actions/ })).toHaveAttribute( 'aria-expanded', 'false', ); @@ -476,7 +479,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - const toggle = screen.getByRole('button', { name: 'Used 2 tools' }); + const toggle = screen.getByRole('button', { name: /^Ran 2 actions/ }); expect(toggle).toHaveAttribute('aria-expanded', 'false'); fireEvent.click(toggle); @@ -488,7 +491,7 @@ describe('ContentParts integration: MCP image hoist and grouping', () => { , ); - expect(screen.getByRole('button', { name: 'Used 2 tools' })).toHaveAttribute( + expect(screen.getByRole('button', { name: /^Ran 2 actions/ })).toHaveAttribute( 'aria-expanded', 'true', ); diff --git a/client/src/components/Chat/Messages/Content/__tests__/ToolCall.test.tsx b/client/src/components/Chat/Messages/Content/__tests__/ToolCall.test.tsx index 967a66cfa4..ea68deae29 100644 --- a/client/src/components/Chat/Messages/Content/__tests__/ToolCall.test.tsx +++ b/client/src/components/Chat/Messages/Content/__tests__/ToolCall.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { RecoilRoot } from 'recoil'; import { Tools, Constants } from 'librechat-data-provider'; import { render, screen, fireEvent } from '@testing-library/react'; +import { ToolAuthWarningContext } from '../auth'; import ToolCall from '../ToolCall'; // Mock dependencies @@ -19,6 +20,7 @@ jest.mock('~/hooks', () => ({ com_assistants_allow_sites_you_trust: 'Only allow sites you trust', com_ui_via_server: `via ${values?.[0]}`, com_ui_tool_failed: 'failed', + com_ui_tool_name_set_memory: 'Save Memory', }; return translations[key] || key; }, @@ -101,6 +103,8 @@ jest.mock('~/utils', () => ({ error: jest.fn(), }, cn: (...classes: any[]) => classes.filter(Boolean).join(' '), + getToolDisplayLabel: (name: string, localize: (key: string) => string) => + name === 'set_memory' ? localize('com_ui_tool_name_set_memory') : name, })); describe('ToolCall', () => { @@ -154,6 +158,21 @@ describe('ToolCall', () => { }); }); + it('uses a friendly label for the set_memory tool', () => { + renderWithRecoil(); + + expect(screen.getByTestId('progress-text')).toHaveTextContent('Completed Save Memory'); + expect(screen.queryByText(/set_memory/)).not.toBeInTheDocument(); + }); + + it('keeps expanded tool content close to its header', () => { + renderWithRecoil(); + + fireEvent.click(screen.getByTestId('progress-text')); + + expect(screen.getByTestId('tool-call-info').parentElement).toHaveClass('mb-2', 'mt-0'); + }); + describe('attachments prop passing', () => { it('should pass attachments to ToolCallInfo when provided', () => { const attachments = [ @@ -432,6 +451,42 @@ describe('ToolCall', () => { describe('MCP OAuth detection', () => { const d = Constants.mcp_delimiter; + it('shows the trust warning for an ungrouped authentication call', () => { + renderWithRecoil( + , + ); + + expect( + screen.getByRole('button', { name: 'Sign in to auth.example.com' }), + ).toBeInTheDocument(); + expect(screen.getByText('Only allow sites you trust')).toBeInTheDocument(); + }); + + it('keeps the sign-in action while a group suppresses the repeated warning', () => { + renderWithRecoil( + + + , + ); + + expect( + screen.getByRole('button', { name: 'Sign in to auth.example.com' }), + ).toBeInTheDocument(); + expect(screen.queryByText('Only allow sites you trust')).not.toBeInTheDocument(); + }); + it('should detect MCP OAuth from delimiter in tool-call name', () => { renderWithRecoil( ({ useLocalize: () => (key: string, values?: Record) => { - if (key === 'com_ui_used_n_tools') { - return `Used ${values?.[0]} tools`; + if (key === 'com_ui_ran_n_actions') { + return `Ran ${values?.[0]} actions`; + } + if (key === 'com_ui_running_n_actions') { + return `Running ${values?.[0]} actions`; + } + if (key === 'com_ui_n_searches') { + return `${values?.[0]} searches`; + } + if (key === 'com_ui_n_actions_failed') { + return `${values?.[0]} failed`; + } + if (key === 'com_ui_one_action_failed') { + return '1 failed'; + } + if (key === 'com_ui_web_searched') { + return 'Searched the web'; + } + if (key === 'com_ui_web_searching') { + return 'Searching the web'; + } + if (key === 'com_ui_retrieved_files') { + return 'Searched your files'; + } + if (key === 'com_ui_searching_files') { + return 'Searching your files'; + } + if (key === 'com_ui_searched_web_and_files') { + return 'Searched web and files'; + } + if (key === 'com_ui_searching_web_and_files') { + return 'Searching web and files'; } if (key === 'com_ui_asked_n_questions') { return `Asked ${values?.[0]} questions`; @@ -32,6 +63,9 @@ jest.mock('~/hooks', () => ({ if (key === 'com_ui_via_server') { return `via ${values?.[0]}`; } + if (key === 'com_assistants_allow_sites_you_trust') { + return 'Only allow sites you trust'; + } return key; }, useExpandCollapse: (isExpanded: boolean) => ({ @@ -57,12 +91,18 @@ jest.mock('../ToolOutput', () => ({ ), getMCPServerName: () => '', + isError: (output: string) => output.startsWith('Error processing tool'), })); jest.mock('lucide-react', () => ({ - ChevronDown: () => {'chevron'}, + ChevronDown: ({ className }: { className?: string }) => ( + + {'chevron'} + + ), Users: () => {'users'}, MessageCircleQuestion: () => {'question'}, + TriangleAlert: () => {'warning'}, })); jest.mock('~/utils/approval', () => ({ @@ -71,10 +111,22 @@ jest.mock('~/utils/approval', () => ({ jest.mock('~/utils', () => ({ cn: (...classes: Array) => classes.filter(Boolean).join(' '), - getToolDisplayLabel: (name: string) => - ['execute_code', 'bash_tool', 'run_tools_with_code', 'run_tools_with_bash'].includes(name) - ? 'Code' - : name, + getToolDisplayLabel: (name: string) => { + if ( + ['execute_code', 'bash_tool', 'run_tools_with_code', 'run_tools_with_bash'].includes(name) + ) { + return 'Code'; + } + const friendlyNames: Record = { + web_search: 'Web Search', + file_search: 'File Search', + retrieval: 'File Search', + create_file: 'Create File', + edit_file: 'Edit File', + ask_user_question: 'Question', + }; + return friendlyNames[name] ?? name; + }, /** Real implementations: the group header resolves its text through these, * so stubbing them out would hide the header logic under test. */ getBatchActivityLabelPart: jest.requireActual('~/utils/activityLabels').getBatchActivityLabelPart, @@ -86,6 +138,9 @@ jest.mock('../Parts', () => ({ AttachmentGroup: ({ attachments }: { attachments?: TAttachment[] }) => (
), + ReasoningCompact: ({ isAfterTool }: { isAfterTool?: boolean }) => ( +
+ ), })); const makePart = ( @@ -134,6 +189,24 @@ const makeSubagentPart = ( }, }) as unknown as TMessageContentParts; +const makeAuthPart = ( + id: string, + name: string, + progress = 0.1, + output = '', +): TMessageContentParts => + ({ + type: ContentTypes.TOOL_CALL, + [ContentTypes.TOOL_CALL]: { + id, + name, + args: '{}', + output, + progress, + auth: `https://${name}.example.com/oauth`, + }, + }) as unknown as TMessageContentParts; + const imageAttachment: TAttachment = { filename: 'foo.png', filepath: '/files/foo.png', @@ -211,6 +284,40 @@ describe('ToolCallGroup image hoisting', () => { expect(screen.queryByTestId('attachment-group')).not.toBeInTheDocument(); }); + it('renders one shared trust warning for multiple pending authentication calls', () => { + const authParts = ['zapier', 'test', 'vercel', 'spotify'].map((name, idx) => ({ + part: makeAuthPart(`auth-${idx}`, name), + idx, + })); + + renderGroup({ + ...baseProps, + parts: authParts, + isSubmitting: true, + lastContentIdx: authParts.length - 1, + renderPart: (_part, idx) => , + }); + + expect(screen.getAllByText('Only allow sites you trust')).toHaveLength(1); + }); + + it('does not render a shared trust warning for completed authentication calls', () => { + renderGroup({ + ...baseProps, + parts: [{ part: makeAuthPart('auth-complete', 'zapier', 1, 'done'), idx: 0 }], + lastContentIdx: 0, + renderPart: () => null, + }); + + expect(screen.queryByText('Only allow sites you trust')).not.toBeInTheDocument(); + }); + + it('keeps the group disclosure chevron visible', () => { + renderGroup(baseProps); + + expect(screen.getByTestId('group-chevron')).not.toHaveClass('opacity-0'); + }); + it('does not reconcile layout for an initially collapsed completed group', () => { renderGroup(baseProps); expect(mockScheduleMessageContentLayoutReconcile).not.toHaveBeenCalled(); @@ -342,7 +449,7 @@ describe('ToolCallGroup image hoisting', () => { renderPart, }); - expect(screen.getByRole('button', { name: 'Used 59 tools' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /^Ran 59 actions/ })).toBeInTheDocument(); expect(renderPart).not.toHaveBeenCalled(); expect(screen.queryByTestId('inner-0')).not.toBeInTheDocument(); }); @@ -350,16 +457,35 @@ describe('ToolCallGroup image hoisting', () => { it('mounts tool bodies when a collapsed group is expanded', () => { renderGroup(baseProps); - fireEvent.click(screen.getByRole('button', { name: 'Used 2 tools' })); + fireEvent.click(screen.getByRole('button', { name: /^Ran 2 actions/ })); expect(screen.getByTestId('inner-0')).toBeInTheDocument(); expect(screen.getByTestId('inner-1')).toBeInTheDocument(); }); + it('removes the extra Thoughts top margin after a tool row', () => { + const reasoningPart = { + type: ContentTypes.THINK, + [ContentTypes.THINK]: 'A useful thought', + } as TMessageContentParts; + + renderGroup({ + ...baseProps, + parts: [ + { part: makePart('t1'), idx: 0 }, + { part: reasoningPart, idx: 1 }, + ], + }); + + fireEvent.click(screen.getByRole('button', { name: /^Fetch_image/ })); + + expect(screen.getByTestId('compact-reasoning')).toHaveAttribute('data-after-tool', 'true'); + }); + it('unmounts tool bodies after a collapsed group finishes transitioning', () => { renderGroup(baseProps); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; fireEvent.click(button); fireEvent.click(button); @@ -385,7 +511,7 @@ describe('ToolCallGroup image hoisting', () => { ), }); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; expect(screen.getByTestId('approval-0')).toBeInTheDocument(); @@ -417,7 +543,7 @@ describe('ToolCallGroup image hoisting', () => { ), }); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; fireEvent.click(button); fireEvent.transitionEnd(collapsible); @@ -447,7 +573,7 @@ describe('ToolCallGroup image hoisting', () => { ), }); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; fireEvent.click(button); fireEvent.transitionEnd(collapsible); @@ -475,7 +601,7 @@ describe('ToolCallGroup image hoisting', () => { }); const { rerender } = renderGroup(propsFor()); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; fireEvent.click(button); fireEvent.transitionEnd(collapsible); @@ -516,7 +642,7 @@ describe('ToolCallGroup image hoisting', () => { }); const { rerender } = renderGroup(propsFor()); - const button = screen.getByRole('button', { name: 'Used 2 tools' }); + const button = screen.getByRole('button', { name: /^Ran 2 actions/ }); const collapsible = button.nextElementSibling as HTMLElement; fireEvent.click(button); @@ -536,10 +662,10 @@ describe('ToolCallGroup image hoisting', () => { it('reconciles layout after the group collapses from an expanded state', async () => { renderGroup(baseProps); - fireEvent.click(screen.getByRole('button', { name: 'Used 2 tools' })); + fireEvent.click(screen.getByRole('button', { name: /^Ran 2 actions/ })); expect(mockScheduleMessageContentLayoutReconcile).not.toHaveBeenCalled(); - fireEvent.click(screen.getByRole('button', { name: 'Used 2 tools' })); + fireEvent.click(screen.getByRole('button', { name: /^Ran 2 actions/ })); await waitFor(() => { expect(mockScheduleMessageContentLayoutReconcile).toHaveBeenCalledTimes(1); @@ -579,7 +705,7 @@ describe('ToolCallGroup image hoisting', () => { ], }); - expect(screen.getByText('· Code')).toBeInTheDocument(); + expect(screen.getByText('· Code ×2')).toBeInTheDocument(); expect(screen.queryByText(/Code, bash_tool/)).not.toBeInTheDocument(); expect(screen.getByTestId('stacked-icons')).toHaveAttribute( 'data-tool-names', @@ -587,6 +713,78 @@ describe('ToolCallGroup image hoisting', () => { ); }); + it('summarizes repeated completed web searches as an outcome and count', () => { + const searchParts = Array.from({ length: 9 }, (_, idx) => ({ + part: makePart(`w${idx}`, 'result', 'web_search'), + idx, + })); + + renderGroup({ + ...baseProps, + parts: searchParts, + lastContentIdx: searchParts.length - 1, + }); + + expect( + screen.getByRole('button', { name: 'Searched the web, 9 searches' }), + ).toBeInTheDocument(); + expect(screen.getByText('Searched the web')).toBeInTheDocument(); + expect(screen.getByText('· 9 searches')).toBeInTheDocument(); + }); + + it('uses the active tense while a web-search group is running', () => { + renderGroup({ + ...baseProps, + isSubmitting: true, + parts: [ + { part: makePart('w1', 'result', 'web_search'), idx: 0 }, + { part: makePart('w2', '', 'web_search'), idx: 1 }, + ], + }); + + expect( + screen.getByRole('button', { name: 'Searching the web, 2 searches' }), + ).toBeInTheDocument(); + }); + + it('summarizes mixed web and file searches without exposing tool ids', () => { + renderGroup({ + ...baseProps, + parts: [ + { part: makePart('w1', 'result', 'web_search'), idx: 0 }, + { part: makePart('f1', 'result', 'file_search'), idx: 1 }, + { part: makePart('f2', 'result', 'retrieval'), idx: 2 }, + ], + lastContentIdx: 2, + }); + + expect( + screen.getByRole('button', { name: 'Searched web and files, 3 searches' }), + ).toBeInTheDocument(); + expect(screen.queryByText(/web_search|file_search|retrieval/)).not.toBeInTheDocument(); + }); + + it('keeps repeated action counts and failed-call status in the compact summary', () => { + renderGroup({ + ...baseProps, + parts: [ + { part: makePart('c1', 'created', 'create_file'), idx: 0 }, + { + part: makePart('c2', 'Error processing tool: disk full', 'create_file'), + idx: 1, + }, + { part: makePart('e1', 'edited', 'edit_file'), idx: 2 }, + ], + lastContentIdx: 2, + }); + + expect( + screen.getByRole('button', { + name: 'Ran 3 actions, Create File ×2, Edit File · 1 failed', + }), + ).toBeInTheDocument(); + }); + it('labels a homogeneous ask_user_question group as its own category', () => { renderGroup({ ...baseProps, @@ -599,7 +797,7 @@ describe('ToolCallGroup image hoisting', () => { // Own verb, not "Used N tools"; question glyph instead of stacked wrenches; // raw-name summary suppressed (like subagent groups). expect(screen.getByRole('button', { name: 'Asked 2 questions' })).toBeInTheDocument(); - expect(screen.queryByText('Used 2 tools')).not.toBeInTheDocument(); + expect(screen.queryByText('Ran 2 actions')).not.toBeInTheDocument(); expect(screen.getByTestId('question-icon')).toBeInTheDocument(); expect(screen.queryByTestId('stacked-icons')).not.toBeInTheDocument(); expect(screen.queryByText(/· ask_user_question/)).not.toBeInTheDocument(); @@ -661,7 +859,7 @@ describe('ToolCallGroup image hoisting', () => { expect(screen.getByRole('button', { name: 'Running agent' })).toBeInTheDocument(); }); - it('keeps the generic "Used N tools" label for a mixed group containing a question', () => { + it('uses an action summary for a mixed group containing a question', () => { renderGroup({ ...baseProps, parts: [ @@ -670,7 +868,9 @@ describe('ToolCallGroup image hoisting', () => { ], }); - expect(screen.getByRole('button', { name: 'Used 2 tools' })).toBeInTheDocument(); + expect( + screen.getByRole('button', { name: 'Ran 2 actions, Web Search, Question' }), + ).toBeInTheDocument(); expect(screen.getByTestId('stacked-icons')).toBeInTheDocument(); }); }); diff --git a/client/src/components/Chat/Messages/Content/auth.tsx b/client/src/components/Chat/Messages/Content/auth.tsx new file mode 100644 index 0000000000..b2b03a7118 --- /dev/null +++ b/client/src/components/Chat/Messages/Content/auth.tsx @@ -0,0 +1,22 @@ +import { createContext, useContext } from 'react'; +import { TriangleAlert } from 'lucide-react'; +import { useLocalize } from '~/hooks'; +import { cn } from '~/utils'; + +export const ToolAuthWarningContext = createContext(false); + +export function ToolAuthWarning({ className }: { className?: string }) { + const localize = useLocalize(); + const suppressed = useContext(ToolAuthWarningContext); + + if (suppressed) { + return null; + } + + return ( +

+

+ ); +}