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.
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 ?? ''))
|
||||
}
|
||||
|
||||
@@ -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' }), '')
|
||||
|
||||
@@ -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, unknown>): 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, unknown>): string
|
||||
}
|
||||
|
||||
function toolStatus(part: ToolPart, resultRecord: Record<string, unknown>): 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
|
||||
|
||||
@@ -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<Extract<ChatMessagePart, { type: 'tool-call' }>>): 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)
|
||||
})
|
||||
})
|
||||
@@ -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 (
|
||||
|
||||
@@ -32,3 +32,10 @@ export function toolResultRecord(source: ToolResultSource): Record<string, unkno
|
||||
// Authoritative result fields win over fallible/abbreviated display hints.
|
||||
return { ...source.toolResultMetadata, ...record }
|
||||
}
|
||||
|
||||
/** The event's own account of a failure, for calls whose result carries none. */
|
||||
export function envelopeErrorText(metadata: ToolResultMetadata | undefined): string {
|
||||
const text = typeof metadata?.error === 'string' ? metadata.error : metadata?.message
|
||||
|
||||
return text?.trim() ?? ''
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user