From 3b733a7c8a48ca2af20df078157a120012ea280c Mon Sep 17 00:00:00 2001 From: yoniebans Date: Mon, 14 Sep 2026 10:51:37 +0200 Subject: [PATCH] fix(desktop): keep envelope errors and sealed calls settled outside the tool row Three consumers still read `result === undefined` as an open call or an error without text after the result and display-hint split. The tool row lost the event's error explanation when the result carried none: `toolErrorText` now reads `toolResultMetadata.error` / `.message` when `isError` is set, and `toolStatus` runs the error path for sealed error rows so an envelope-only read miss stays on the notice tier. The turn-activity signature and wait narration count a sealed call as settled. Onboarding's start-with-connections offer withdraws once a connection wait is sealed. Four tests fail on 235ec0f and pass here. --- .../assistant-ui/thread/turn-activity.test.ts | 9 ++++++ .../assistant-ui/thread/turn-activity.ts | 10 +++--- .../assistant-ui/tool/fallback-model.test.ts | 32 +++++++++++++++++++ .../assistant-ui/tool/fallback-model/index.ts | 17 +++++++--- .../desktop/src/lib/first-build-start.test.ts | 24 ++++++++++++++ apps/desktop/src/lib/first-build-start.ts | 2 +- apps/desktop/src/lib/tool-result-metadata.ts | 7 ++++ 7 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 apps/desktop/src/lib/first-build-start.test.ts diff --git a/apps/desktop/src/components/assistant-ui/thread/turn-activity.test.ts b/apps/desktop/src/components/assistant-ui/thread/turn-activity.test.ts index 54a5d26013..10ebf6d4eb 100644 --- a/apps/desktop/src/components/assistant-ui/thread/turn-activity.test.ts +++ b/apps/desktop/src/components/assistant-ui/thread/turn-activity.test.ts @@ -53,6 +53,15 @@ describe('toolNarratesWait', () => { expect(toolNarratesWait([call('react_to_message', false)])).toBe(false) }) + it('does not defer to a call sealed without a result', () => { + const sealed = { completedAt: 5, toolName: 'terminal', type: 'tool-call' } + + expect(toolNarratesWait([text('working'), sealed])).toBe(false) + expect(activitySignature([text('working'), sealed])).not.toBe( + activitySignature([text('working'), call('terminal', false)]) + ) + }) + it('defers to a call in flight even when a later part follows it', () => { // Tail-part-only detection missed this: a turn that starts prose while a // call is still running showed the row under the tool's own timer. diff --git a/apps/desktop/src/components/assistant-ui/thread/turn-activity.ts b/apps/desktop/src/components/assistant-ui/thread/turn-activity.ts index 74948efe58..4704043f80 100644 --- a/apps/desktop/src/components/assistant-ui/thread/turn-activity.ts +++ b/apps/desktop/src/components/assistant-ui/thread/turn-activity.ts @@ -11,12 +11,16 @@ import { isSilentTool } from '@/lib/tool-render-class' export const TURN_QUIET_S = 2 export interface ActivityPart { + completedAt?: number result?: unknown text?: unknown toolName?: string type: string } +// A sealed call carries a completion marker and no result. +const settled = (part: ActivityPart): boolean => part.result !== undefined || part.completedAt !== undefined + /** * What the tail message has produced so far, as a value that changes exactly * when the turn makes visible progress. @@ -36,7 +40,7 @@ export function activitySignature(content: readonly ActivityPart[]): string { textLength += part.text.length } - if (part.type === 'tool-call' && part.result !== undefined) { + if (part.type === 'tool-call' && settled(part)) { settledTools += 1 } } @@ -54,7 +58,5 @@ export function activitySignature(content: readonly ActivityPart[]): string { * of those is as unnarrated as a wait on nothing at all. */ export function toolNarratesWait(content: readonly ActivityPart[]): boolean { - return content.some( - part => part.type === 'tool-call' && part.result === undefined && !isSilentTool(part.toolName ?? '') - ) + return content.some(part => part.type === 'tool-call' && !settled(part) && !isSilentTool(part.toolName ?? '')) } diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts index 5e2351a61e..26297ae2d4 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model.test.ts @@ -117,6 +117,38 @@ describe('buildToolView error confidence', () => { }) }) +describe('buildToolView envelope errors', () => { + it('shows the event error when the result carries no explanation', () => { + const view = buildToolView( + part({ + isError: true, + result: 'partial output', + toolName: 'terminal', + toolResultMetadata: { error: 'killed by signal 9' } + }), + '' + ) + + expect(view.status).toBe('error') + expect(view.subtitle).toBe('killed by signal 9') + }) + + it('keeps an envelope-only read miss on the notice tier', () => { + const view = buildToolView( + part({ + isError: true, + result: undefined, + completedAt: 5, + toolName: 'read_file', + toolResultMetadata: { error: 'File not found: /repo/missing.ts' } + }), + '' + ) + + expect(view.status).toBe('notice') + }) +}) + describe('buildToolView browser_exec step label', () => { const bexec = (code: string) => buildToolView(part({ args: { code }, result: undefined, toolName: 'browser_exec' }), '') diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts index ff9156dfe9..a785a79cda 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts +++ b/apps/desktop/src/components/assistant-ui/tool/fallback-model/index.ts @@ -5,7 +5,7 @@ import { normalizeExternalUrl } from '@/lib/external-link' import { summarizeShellCommand } from '@/lib/summarize-command' import { capitalize, firstStringField, normalize } from '@/lib/text' import { isCardTool, isFileEditTool, isSilentTool } from '@/lib/tool-render-class' -import { toolResultRecord } from '@/lib/tool-result-metadata' +import { envelopeErrorText, toolResultRecord } from '@/lib/tool-result-metadata' import { extractToolErrorMessage, formatToolResultSummary } from '@/lib/tool-result-summary' import { skillActivityTitle } from '../skill-activity' @@ -662,7 +662,12 @@ function toolErrorText(part: ToolPart, result: Record): string const extractedError = extractToolErrorMessage(part.result) if (part.isError) { - return extractedError || (typeof part.result === 'string' && part.result.trim()) || 'Tool returned an error.' + return ( + extractedError || + envelopeErrorText(part.toolResultMetadata) || + (typeof part.result === 'string' && part.result.trim()) || + 'Tool returned an error.' + ) } if (extractedError) { @@ -696,8 +701,12 @@ function toolErrorText(part: ToolPart, result: Record): string } function toolStatus(part: ToolPart, resultRecord: Record): ToolStatus { - if (part.result === undefined) { - return part.completedAt === undefined ? 'running' : part.isError ? 'error' : 'warning' + if (part.result === undefined && part.completedAt === undefined) { + return 'running' + } + + if (part.result === undefined && !part.isError) { + return 'warning' } // Explicit success wins over isError / nested-error heuristics. Memory writes diff --git a/apps/desktop/src/lib/first-build-start.test.ts b/apps/desktop/src/lib/first-build-start.test.ts new file mode 100644 index 0000000000..951ecd58b2 --- /dev/null +++ b/apps/desktop/src/lib/first-build-start.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' + +import type { ChatMessagePart } from '@/lib/chat-messages' + +import { canStartWithConnections } from './first-build-start' + +const wait = (overrides: Partial>): ChatMessagePart => + ({ + type: 'tool-call', + toolCallId: 'wait-1', + toolName: 'manage_connections', + args: { action: 'wait' }, + ...overrides + }) as ChatMessagePart + +describe('canStartWithConnections', () => { + it('offers the start while a connection wait is in flight', () => { + expect(canStartWithConnections(wait({}))).toBe(true) + }) + + it('withdraws the start once the wait is sealed without a result', () => { + expect(canStartWithConnections(wait({ completedAt: 5 }))).toBe(false) + }) +}) diff --git a/apps/desktop/src/lib/first-build-start.ts b/apps/desktop/src/lib/first-build-start.ts index c953422656..8743e40702 100644 --- a/apps/desktop/src/lib/first-build-start.ts +++ b/apps/desktop/src/lib/first-build-start.ts @@ -25,7 +25,7 @@ export function canStartWithConnections(part: ChatMessagePart): boolean { const output = recordOf(part.result) if (action === 'wait') { - return part.result === undefined || output.status === 'pending' + return (part.result === undefined && part.completedAt === undefined) || output.status === 'pending' } return ( diff --git a/apps/desktop/src/lib/tool-result-metadata.ts b/apps/desktop/src/lib/tool-result-metadata.ts index bace491003..ec3dce7639 100644 --- a/apps/desktop/src/lib/tool-result-metadata.ts +++ b/apps/desktop/src/lib/tool-result-metadata.ts @@ -32,3 +32,10 @@ export function toolResultRecord(source: ToolResultSource): Record