From 5dc007c166cbe3405ad0cce1e2b412e3850b2bea Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:31:15 +0200 Subject: [PATCH] fix: adopt the shared code surface and count background task failures Register surface-code in the versioned theme registry and use it for the diff preview. The token, its light and dark CSS values and its Tailwind mapping were already in place and already consumed by CodeBar, CodeBlock and ResultSwitcher; only the three theme interfaces, the two bundled themes and DiffView were missing. DiffView no longer coordinates two unrelated surface roles through a dark: override, and a custom theme can restyle the code surface centrally. Include background-task failures in grouped outcomes. A backgrounded bash or code task reports its verdict through a background_task_status attachment rather than its output, and its dispatch step keeps a benign handle and usually closes as completed, so the group reported no failed action beside a card showing failure. Group metadata now derives that signal with the same helpers the child card uses, correlated per part so a handoff response repeating a provider tool-call id across agents still routes correctly. Require a title on shopping results. A link-only card had no accessible name and no visible identity: the anchor carried no label, its image is decorative and the title span rendered empty. --- .../Chat/Messages/Content/Parts/DiffView.tsx | 2 +- .../Chat/Messages/Content/ToolCallGroup.tsx | 40 ++++++++++++++++--- .../Chat/Messages/Content/verticals.tsx | 5 ++- .../client/src/theme/semanticTokens.spec.ts | 1 + packages/client/src/theme/themes/dark.ts | 1 + packages/client/src/theme/themes/default.ts | 1 + packages/client/src/theme/types/index.ts | 3 ++ 7 files changed, 46 insertions(+), 7 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx index 9e25b9328d..4065549b01 100644 --- a/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/DiffView.tsx @@ -124,7 +124,7 @@ export default function DiffView({ parsed }: { parsed: ParsedDiff }) { return (
{lines.map((line, index) => { if (line.type === 'hunk') { diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index e85adf9151..3b6b1c8a8a 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -20,6 +20,8 @@ import { getActivityLabelText, } from '~/utils'; import { useLocalize, useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks'; +import { parseBackgroundHandle, splitBackgroundAttachments } from './Parts/handle'; +import { mapAttachments, filterAttachmentsForPart } from '~/utils/map'; import { ToolAuthWarning, ToolAuthWarningContext } from './auth'; import { useMCPIconMap, useMCPServerNames } from '~/hooks/MCP'; import { resolveToolCallPhase } from '~/utils/toolCallPhase'; @@ -94,7 +96,10 @@ function hasPendingAuth(part: TMessageContentParts): boolean { ); } -function getToolMeta(part: TMessageContentParts): ToolMeta | null { +function getToolMeta( + part: TMessageContentParts, + attachmentsByToolCallId?: Record, +): ToolMeta | null { if (part.type !== ContentTypes.TOOL_CALL) { return null; } @@ -109,7 +114,10 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { const isStandard = 'args' in toolCall && (!toolCall.type || toolCall.type === ToolCallTypes.TOOL_CALL); if (isStandard) { - const tc = toolCall as Agents.ToolCall & { progress?: number }; + /** `agentId` disambiguates attachments when a handoff response repeats a + * provider tool-call id across agents; `filterAttachmentsForPart` reads it + * the same way. */ + const tc = toolCall as Agents.ToolCall & { progress?: number; agentId?: string }; /** Subagents can finish with `progress === 1` and no final output * text (the parent saw "" / undefined back). Fall back to progress * so the group header flips from "Running N agents" to "Ran N @@ -125,10 +133,23 @@ function getToolMeta(part: TMessageContentParts): ToolMeta | null { name === 'set_memory' || name === 'delete_memory' ? isMemoryFailureOutput(name, tc.output ?? '') : hasFailedOutput(tc.output); + /** A backgrounded bash/code task reports its verdict through a + * `background_task_status` attachment, not its output: the dispatch step + * keeps a benign handle and usually closes as `completed`. The child card + * folds that marker in as `extraError`, so without it here the group + * reported no failed action beside a card showing failure. Correlated the + * same way the child is, since provider tool-call ids repeat across agents + * in handoff responses. */ + const backgroundFailed = + parseBackgroundHandle(tc.output) != null && + splitBackgroundAttachments( + filterAttachmentsForPart(attachmentsByToolCallId?.[tc.id ?? ''], tc.agentId), + tc.id, + ).backgroundStatus === 'error'; return { name, iconName, - ...resolveOutcome(runStepStatus, completed, failedOutput), + ...resolveOutcome(runStepStatus, completed, failedOutput || backgroundFailed), }; } @@ -205,12 +226,21 @@ export default function ToolCallGroup({ const cancelLayoutReconcileRef = useRef<(() => void) | null>(null); const retainedForPendingApprovalRef = useRef(false); + /** Re-keyed by tool-call id so each part's metadata sees only its own + * attachments: `groupAttachments` arrives flattened across the group. */ + const attachmentsByToolCallId = useMemo( + () => mapAttachments(groupAttachments ?? []), + [groupAttachments], + ); /** `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], + () => + parts + .map((p) => getToolMeta(p.part, attachmentsByToolCallId)) + .filter((m): m is ToolMeta => m != null), + [parts, attachmentsByToolCallId], ); const count = toolMetadata.length; /** Approval state is read from the RAW parts, not `toolMetadata`: a pending diff --git a/client/src/components/Chat/Messages/Content/verticals.tsx b/client/src/components/Chat/Messages/Content/verticals.tsx index 281a634849..0ddd317aee 100644 --- a/client/src/components/Chat/Messages/Content/verticals.tsx +++ b/client/src/components/Chat/Messages/Content/verticals.tsx @@ -65,7 +65,10 @@ export function collectSearchVerticals(attachments?: TAttachment[]): SearchVerti } } for (const item of data.shopping ?? []) { - if (item.link) { + /** A card with no title has no accessible name and no visible identity: + * the anchor carries no label, the image is decorative, and the title + * span renders empty. */ + if (item.link && item.title) { shopping.push(item); } } diff --git a/packages/client/src/theme/semanticTokens.spec.ts b/packages/client/src/theme/semanticTokens.spec.ts index 0b9cc25a19..881bc91ba2 100644 --- a/packages/client/src/theme/semanticTokens.spec.ts +++ b/packages/client/src/theme/semanticTokens.spec.ts @@ -165,6 +165,7 @@ const canvasSurfaces: Array = [ 'rgb-surface-secondary', 'rgb-surface-dialog', 'rgb-surface-chat', + 'rgb-surface-code', 'rgb-presentation', ]; diff --git a/packages/client/src/theme/themes/dark.ts b/packages/client/src/theme/themes/dark.ts index 69cabf4855..93ee5f50bf 100644 --- a/packages/client/src/theme/themes/dark.ts +++ b/packages/client/src/theme/themes/dark.ts @@ -48,6 +48,7 @@ export const darkTheme: IThemeRGB = { 'rgb-surface-destructive': '153 27 27', // #991b1b (red-800) 'rgb-surface-destructive-hover': '127 29 29', // #7f1d1d (red-900) 'rgb-surface-chat': '47 47 47', // #2f2f2f (gray-700) + 'rgb-surface-code': '33 33 33', // #212121 (gray-800) 'rgb-surface-inverted': '255 255 255', // #fff (white) 'rgb-surface-inverted-hover': '236 236 236', // #ececec (gray-100) 'rgb-text-inverted': '23 23 23', // #171717 (gray-850) diff --git a/packages/client/src/theme/themes/default.ts b/packages/client/src/theme/themes/default.ts index 503d02ddc4..1bd371f340 100644 --- a/packages/client/src/theme/themes/default.ts +++ b/packages/client/src/theme/themes/default.ts @@ -48,6 +48,7 @@ export const defaultTheme: IThemeRGB = { 'rgb-surface-destructive': '185 28 28', // #b91c1c (red-700) 'rgb-surface-destructive-hover': '153 27 27', // #991b1b (red-800) 'rgb-surface-chat': '255 255 255', // #fff (white) + 'rgb-surface-code': '247 247 248', // #f7f7f8 (gray-50) 'rgb-surface-inverted': '23 23 23', // #171717 (gray-850) 'rgb-surface-inverted-hover': '47 47 47', // #2f2f2f (gray-700) 'rgb-text-inverted': '255 255 255', // #fff (white) diff --git a/packages/client/src/theme/types/index.ts b/packages/client/src/theme/types/index.ts index b692da887b..52dba7f9a8 100644 --- a/packages/client/src/theme/types/index.ts +++ b/packages/client/src/theme/types/index.ts @@ -46,6 +46,7 @@ export interface IThemeRGB { 'rgb-surface-destructive'?: string; 'rgb-surface-destructive-hover'?: string; 'rgb-surface-chat'?: string; + 'rgb-surface-code'?: string; 'rgb-surface-inverted'?: string; 'rgb-surface-inverted-hover'?: string; 'rgb-text-inverted'?: string; @@ -141,6 +142,7 @@ export interface IThemeVariables { '--surface-destructive': string; '--surface-destructive-hover': string; '--surface-chat': string; + '--surface-code': string; '--surface-inverted': string; '--surface-inverted-hover': string; '--text-inverted': string; @@ -228,6 +230,7 @@ export interface IThemeColors { 'surface-destructive'?: string; 'surface-destructive-hover'?: string; 'surface-chat'?: string; + 'surface-code'?: string; 'surface-inverted'?: string; 'surface-inverted-hover'?: string; 'text-inverted'?: string;