Skip to content

Commit 4fecfc9

Browse files
committed
fix(chat): keep tool failures in expanded history
1 parent 1486a51 commit 4fecfc9

5 files changed

Lines changed: 140 additions & 30 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream.test.tsx‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -121,26 +121,43 @@ describe.each(['mothership', 'workflow', 'browser', 'deploy'])('%s activity', (a
121121
expect(header()?.textContent).toBe('Reading third')
122122
})
123123

124-
it.each(['error', 'cancelled', 'interrupted', 'rejected', 'skipped'] as const)(
124+
it.each(['cancelled', 'interrupted', 'skipped'] as const)(
125125
'shows %s immediately and cancels a pending cosmetic update',
126126
(status) => {
127127
render([tool('first')])
128128
advance(100)
129129
render([tool('first', 'success'), tool('second')])
130130
render([tool('first', 'success'), tool('second', status)])
131-
const prefix =
132-
status === 'error' || status === 'rejected'
133-
? 'Failed'
134-
: status === 'skipped'
135-
? 'Skipped'
136-
: 'Stopped'
131+
const prefix = status === 'skipped' ? 'Skipped' : 'Stopped'
137132
expect(header()?.textContent).toBe(`${prefix} reading second`)
138133
expect(container.querySelector('[class*="shimmer"]')).toBeNull()
139134
advance(1500)
140135
expect(header()?.textContent).toBe(`${prefix} reading second`)
141136
}
142137
)
143138

139+
it.each(['error', 'rejected'] as const)(
140+
'keeps a %s attempt in history without promoting it into the summary',
141+
(status) => {
142+
render([tool('first')])
143+
advance(100)
144+
render([tool('first', 'success'), tool('second')])
145+
render([tool('first', 'success'), tool('second', status)])
146+
const label = agentName === 'mothership' ? 'Read first' : 'Reading first'
147+
expect(header()?.textContent).toBe(label)
148+
advance(1500)
149+
expect(header()?.textContent).toBe(label)
150+
render([tool('first', 'success'), tool('second', status)], false)
151+
expect(header()?.textContent).toBe(
152+
agentName === 'mothership' ? 'Read first + 1' : 'Read files'
153+
)
154+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
155+
expect(container.querySelector('[data-state="open"]')?.textContent).toContain(
156+
'Failed reading second'
157+
)
158+
}
159+
)
160+
144161
it('shows final completion immediately and never replays the held action', () => {
145162
render([tool('first')])
146163
advance(100)

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group-view.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
'use client'
22

33
import { type ComponentType, type ReactNode, useState } from 'react'
4-
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
54
import { ThinkingLoader } from '@/components/ui/thinking-loader'
65
import { isBrowserAgentAvailable } from '@/lib/browser-agent/transport'
6+
import type { ToolActivity } from '@/lib/mothership/generated/protocol'
77
import { RETIRED_BROWSER_REQUEST_TAKEOVER_ID } from '@/lib/mothership/tools/retired-tools'
88
import { getToolDisplayTitle, getToolStatusDisplayTitle } from '@/lib/mothership/tools/tool-display'
99
import { ActivityStream } from '@/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/activity-stream'
@@ -275,6 +275,8 @@ export function AgentGroupView({
275275
meaningfulItems.some(
276276
(item) =>
277277
item.type !== 'tool' ||
278+
item.data.status === ToolCallStatus.error ||
279+
item.data.status === ToolCallStatus.rejected ||
278280
needsToolInput(item.data) ||
279281
item.data.toolName === RETIRED_BROWSER_REQUEST_TAKEOVER_ID
280282
)

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/agent-group.test.ts‎

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,8 @@ describe('AgentGroup inline main activity', () => {
162162
it.each([
163163
['executing', 'Reading notes'],
164164
['success', 'Read notes'],
165-
['error', 'Failed reading notes'],
166165
['cancelled', 'Stopped reading notes'],
167166
['skipped', 'Skipped reading notes'],
168-
['rejected', 'Failed reading notes'],
169167
['interrupted', 'Stopped reading notes'],
170168
] as const)('renders a single %s tool once without a disclosure', (status, expected) => {
171169
act(() =>
@@ -223,11 +221,11 @@ describe('AgentGroup inline main activity', () => {
223221

224222
it.each([
225223
['success', 'Checked search requirements'],
226-
['error', 'Failed searching'],
224+
['error', '1 tool call'],
227225
['cancelled', 'Stopped searching'],
228226
['skipped', 'Skipped searching'],
229227
['interrupted', 'Stopped searching'],
230-
['rejected', 'Failed searching'],
228+
['rejected', '1 tool call'],
231229
] as const)('uses an honest grouped activity label after %s', (status, expected) => {
232230
const item = tool(status)
233231
act(() =>
@@ -387,7 +385,7 @@ describe('AgentGroup inline main activity', () => {
387385
expect(container.textContent).not.toContain('Built API')
388386
})
389387

390-
it('retains earlier failed outcomes when completed activities collapse into one summary', () => {
388+
it('keeps earlier failures in expanded history when completed activities collapse', () => {
391389
act(() =>
392390
root.render(
393391
createElement(AgentGroup, {
@@ -420,11 +418,90 @@ describe('AgentGroup inline main activity', () => {
420418
})
421419
)
422420
)
423-
expect(container.querySelector('[role="status"]')?.textContent).toBe(
424-
'Checked inputs + 1 · 1 failed'
425-
)
421+
expect(container.querySelector('[role="status"]')?.textContent).toBe('Checked inputs + 1')
422+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
423+
expect(container.textContent).toContain('Failed reading first document')
426424
})
427425

426+
it.each(['mothership', 'workflow'])(
427+
'keeps a lone failed %s call accessible under a neutral disclosure',
428+
(agentName) => {
429+
act(() =>
430+
root.render(
431+
createElement(AgentGroup, {
432+
agentName,
433+
agentLabel: agentName,
434+
items: [
435+
{
436+
type: 'tool',
437+
data: {
438+
id: 'failed-read',
439+
toolName: 'read',
440+
displayTitle: 'Reading notes',
441+
status: 'error',
442+
},
443+
},
444+
],
445+
})
446+
)
447+
)
448+
expect(container.textContent).not.toContain('Failed')
449+
expect(container.textContent).toContain('1 tool call')
450+
const disclosure = container.querySelector<HTMLElement>('[role="button"]')
451+
expect(disclosure?.getAttribute('aria-expanded')).toBe('false')
452+
act(() => disclosure?.click())
453+
expect(container.textContent).toContain('Failed reading notes')
454+
}
455+
)
456+
457+
it.each(['executing', 'success'] as const)(
458+
'keeps failures out of a %s summary while retaining them in expanded history',
459+
(status) => {
460+
const items: AgentGroupItem[] = [
461+
{
462+
type: 'tool',
463+
data: {
464+
id: 'read-failed',
465+
toolName: 'read',
466+
displayTitle: 'Reading image.png',
467+
status: 'error',
468+
},
469+
},
470+
{
471+
type: 'tool',
472+
data: { id: 'view', toolName: 'read', displayTitle: 'Viewing image.png', status },
473+
},
474+
{
475+
type: 'tool',
476+
data: {
477+
id: 'later-failed',
478+
toolName: 'read',
479+
displayTitle: 'Reading notes',
480+
status: 'error',
481+
},
482+
},
483+
]
484+
act(() =>
485+
root.render(
486+
createElement(AgentGroup, {
487+
agentName: 'mothership',
488+
agentLabel: 'Sim',
489+
items,
490+
activity: { id: 'inspect', completedTitle: 'Inspected images' },
491+
isStreaming: status === 'executing',
492+
isLaneOpen: status === 'executing',
493+
})
494+
)
495+
)
496+
expect(container.textContent).toBe(
497+
status === 'executing' ? 'Viewing image.png' : 'Viewed image.png + 2'
498+
)
499+
act(() => container.querySelector<HTMLElement>('[role="button"]')?.click())
500+
expect(container.textContent).toContain('Failed reading image.png')
501+
expect(container.textContent).toContain('Failed reading notes')
502+
}
503+
)
504+
428505
it('paces the active status in place and expands the full completed history', () => {
429506
vi.useFakeTimers()
430507
const first: AgentGroupItem = {

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,13 @@ describe('getToolActivitySummary', () => {
6464
tool('apply_file_edit', 'error'),
6565
tool('terminal_run', 'interrupted'),
6666
])
67-
).toBe('Tool activity · 1 stopped')
67+
).toBe('2 tool calls · 1 stopped')
6868
})
6969

7070
it('uses a neutral summary when every call failed', () => {
7171
expect(
7272
getToolActivitySummary([tool('run_workflow', 'error'), tool('terminal', 'rejected')])
73-
).toBe('Tool activity')
73+
).toBe('2 tool calls')
7474
})
7575

7676
it('does not infer tool failures from workflow results', () => {
@@ -89,7 +89,7 @@ describe('getToolActivitySummary', () => {
8989
})
9090

9191
it.each([
92-
['rejected', 'Failed running checks'],
92+
['rejected', '1 tool call'],
9393
['skipped', 'Skipped running checks'],
9494
['interrupted', 'Stopped running checks'],
9595
] as const)('labels a single %s tool as finished', (status, expected) => {
@@ -134,9 +134,9 @@ describe('getToolActivitySummary', () => {
134134

135135
it('keeps individual failures explicit without adding aggregate failure badges', () => {
136136
const rejected = { ...tool('terminal', 'rejected'), displayTitle: 'Running checks' }
137-
expect(getToolActivitySummary([rejected])).toBe('Failed running checks')
137+
expect(getToolActivitySummary([rejected])).toBe('1 tool call')
138138
expect(getToolActivitySummary([rejected, tool('read', 'skipped')])).toBe(
139-
'Tool activity · 1 skipped'
139+
'2 tool calls · 1 skipped'
140140
)
141141
})
142142

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/agent-group/tool-activity-group.tsx‎

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,19 @@ import { type ToolCallData, ToolCallStatus } from '@/app/workspace/[workspaceId]
1616

1717
const MAX_SUMMARY_ACTIONS = 3
1818

19+
function isFailedTool(tool: ToolCallData): boolean {
20+
return tool.status === ToolCallStatus.error || tool.status === ToolCallStatus.rejected
21+
}
22+
23+
function toolCountLabel(tools: ToolCallData[]): string {
24+
return `${tools.length} tool ${tools.length === 1 ? 'call' : 'calls'}`
25+
}
26+
1927
/** Summarize completed actions without describing failed or skipped work as successful. */
2028
export function getToolActivitySummary(tools: ToolCallData[]): string {
2129
if (tools.length === 1) {
2230
const tool = tools[0]
31+
if (isFailedTool(tool)) return toolCountLabel(tools)
2332
return getToolStatusDisplayTitle(
2433
tool.displayTitle,
2534
tool.status,
@@ -32,7 +41,7 @@ export function getToolActivitySummary(tools: ToolCallData[]): string {
3241
MAX_SUMMARY_ACTIONS
3342
)
3443
const summary = labels.join(', ')
35-
const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : 'Tool activity'
44+
const summaryLabel = summary ? summary[0].toUpperCase() + summary.slice(1) : toolCountLabel(tools)
3645
return [
3746
additionalActions > 0 ? `${summaryLabel} +${additionalActions} more` : summaryLabel,
3847
...getToolActivityInterruptions(tools),
@@ -56,6 +65,7 @@ export function getActiveToolActivityTitle(
5665
tool: ToolCallData,
5766
tools: ToolCallData[]
5867
): string {
68+
if (isFailedTool(tool)) return toolCountLabel(tools)
5969
return tool.status === ToolCallStatus.executing || tool.status === ToolCallStatus.success
6070
? [label, ...getToolActivityInterruptions(tools)].join(' · ')
6171
: label
@@ -70,7 +80,9 @@ export function getActivityStatusTool(tools: ToolCallData[]): ToolCallData | und
7080
? tool
7181
: newest,
7282
undefined
73-
) ?? tools.at(-1)
83+
) ??
84+
tools.filter((tool) => !isFailedTool(tool)).at(-1) ??
85+
tools.at(-1)
7486
)
7587
}
7688

@@ -116,12 +128,14 @@ export function ToolActivityGroup({
116128
)
117129
const completedActivityLabel = groupedActivity?.completedTitle
118130
? failedActivityTool
119-
? getToolStatusDisplayTitle(
120-
failedActivityTool.displayTitle,
121-
failedActivityTool.status,
122-
failedActivityTool.toolName,
123-
failedActivityTool.activityDescription
124-
)
131+
? isFailedTool(failedActivityTool)
132+
? undefined
133+
: getToolStatusDisplayTitle(
134+
failedActivityTool.displayTitle,
135+
failedActivityTool.status,
136+
failedActivityTool.toolName,
137+
failedActivityTool.activityDescription
138+
)
125139
: groupedActivity.completedTitle
126140
: undefined
127141
const completedLabel = completedActivityLabel
@@ -160,7 +174,7 @@ export function ToolActivityGroup({
160174
}}
161175
activityKey={statusTool.id}
162176
attentionKey={attentionKey}
163-
collapsible={tools.length > 1}
177+
collapsible={tools.length > 1 || isFailedTool(statusTool)}
164178
expanded={expanded}
165179
onToggle={() => setExpanded(!expanded)}
166180
isStreaming={working && autoScrollActivity}

0 commit comments

Comments
 (0)