From bf7ae2549b9a529a7737b073ee285514603a9dc5 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 072da1f34b..d7462f03c6 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'; @@ -95,7 +97,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; } @@ -110,7 +115,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 @@ -126,10 +134,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), }; } @@ -215,12 +236,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 ec7f1f4150..7eb06a0378 100644 --- a/packages/client/src/theme/semanticTokens.spec.ts +++ b/packages/client/src/theme/semanticTokens.spec.ts @@ -166,6 +166,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 e4960c78d5..d16ecc3d61 100644 --- a/packages/client/src/theme/themes/dark.ts +++ b/packages/client/src/theme/themes/dark.ts @@ -53,6 +53,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 2bf971582b..9e688ff446 100644 --- a/packages/client/src/theme/themes/default.ts +++ b/packages/client/src/theme/themes/default.ts @@ -53,6 +53,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 5848850b58..94fde9189a 100644 --- a/packages/client/src/theme/types/index.ts +++ b/packages/client/src/theme/types/index.ts @@ -54,6 +54,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; @@ -176,6 +177,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; @@ -277,6 +279,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;