mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-08-25 16:44:36 +00:00
fix(web-shell): soften tool execution failure hints (#9053)
* fix(web-shell): soften tool execution failure hints Tool failures no longer advertise themselves as text in the collapsed tool-group summaries: the "Failed"/"执行失败" label is replaced with a small error icon, the collapsed summary (single tool or aggregated) drops the error marker entirely, and parallel agent summaries append a plain-text failed count after the done counter. A failed single tool shows the error icon in its expanded card title. The failure is still discoverable inside the expanded view via the icon and the tool output itself. * test(web-shell): lock error-icon rendering and harden card title overflow * fix(web-shell): keep failure evidence visible in icon-only error display Address review feedback on the weakened tool error display: swap the hand-rolled circle-X SVG for lucide's CircleXIcon, move the parallel agents failed count ahead of the done counter so it survives the summary tail truncation, and restore the error icon for shapes the label-to-icon conversion left without any failure indicator (single read tools and failed tools without result text). Lock each behavior, including the cancelled-agent count, with focused tests. * test(web-shell): pin failure icon accessible name and completed-card icon absence (#9053) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: 钉萁 <dingqi.jww@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
parent
6d18c8265f
commit
933a5203cb
9 changed files with 258 additions and 42 deletions
|
|
@ -776,12 +776,122 @@ describe('tool row rendering', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('shows failed status in the collapsed chat summary', () => {
|
||||
it('keeps the failed label out of the collapsed chat summary', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({ toolName: 'Shell', status: 'failed' }),
|
||||
]);
|
||||
|
||||
expect(container.querySelector('button')?.textContent).toContain('Failed');
|
||||
const summary = container.querySelector('button');
|
||||
expect(summary?.textContent).toContain('Shell');
|
||||
expect(summary?.textContent).not.toContain('Failed');
|
||||
expect(summary?.querySelector('[class*="iconError"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('shows the error icon in a failed tool line header', () => {
|
||||
const container = renderToolLine(
|
||||
makeTool({ toolName: 'Shell', status: 'failed' }),
|
||||
);
|
||||
|
||||
const errorIcon = container.querySelector('[class*="iconError"]');
|
||||
expect(errorIcon).not.toBeNull();
|
||||
expect(errorIcon?.getAttribute('role')).toBe('img');
|
||||
expect(errorIcon?.getAttribute('aria-label')).toBe('Failed');
|
||||
expect(errorIcon?.querySelector('svg')).not.toBeNull();
|
||||
expect(container.textContent).not.toContain('Failed');
|
||||
});
|
||||
|
||||
it('shows an error icon instead of the failed label on expanded tool rows', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({
|
||||
toolName: 'Shell',
|
||||
status: 'failed',
|
||||
content: [{ type: 'content', content: { text: 'boom' } }],
|
||||
}),
|
||||
makeTool({ callId: 'call-2', toolName: 'Grep', status: 'completed' }),
|
||||
]);
|
||||
|
||||
const summary = container.querySelector('button') as HTMLButtonElement;
|
||||
act(() => summary.click());
|
||||
|
||||
const errorIcon = container.querySelector('[class*="iconError"]');
|
||||
expect(errorIcon).not.toBeNull();
|
||||
expect(errorIcon?.querySelector('svg')).not.toBeNull();
|
||||
expect(errorIcon?.textContent).not.toContain('Failed');
|
||||
});
|
||||
|
||||
it('shows an error icon in the expanded single-tool card title', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({
|
||||
toolName: 'Shell',
|
||||
status: 'failed',
|
||||
content: [{ type: 'content', content: { text: 'boom' } }],
|
||||
}),
|
||||
]);
|
||||
|
||||
const summary = container.querySelector('button') as HTMLButtonElement;
|
||||
act(() => summary.click());
|
||||
|
||||
const titleRow = container.querySelector('[class*="expandedCardTitleRow"]');
|
||||
expect(titleRow).not.toBeNull();
|
||||
expect(titleRow?.querySelector('[class*="iconError"] svg')).not.toBeNull();
|
||||
expect(titleRow?.textContent).not.toContain('Failed');
|
||||
});
|
||||
|
||||
it('renders no status icon in the expanded completed tool card title', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({
|
||||
toolName: 'Shell',
|
||||
status: 'completed',
|
||||
content: [{ type: 'content', content: { text: 'ok' } }],
|
||||
}),
|
||||
]);
|
||||
|
||||
const summary = container.querySelector('button') as HTMLButtonElement;
|
||||
act(() => summary.click());
|
||||
|
||||
const titleRow = container.querySelector('[class*="expandedCardTitleRow"]');
|
||||
expect(titleRow).not.toBeNull();
|
||||
expect(titleRow?.querySelector('[class*="iconError"]')).toBeNull();
|
||||
});
|
||||
|
||||
it('shows an error icon in the expanded failed todo card title', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({
|
||||
toolName: 'todo_write',
|
||||
status: 'failed',
|
||||
args: {
|
||||
todos: [{ id: '1', content: 'Check UI', status: 'in_progress' }],
|
||||
},
|
||||
}),
|
||||
]);
|
||||
|
||||
const titleRow = container.querySelector('[class*="expandedCardTitleRow"]');
|
||||
expect(titleRow).not.toBeNull();
|
||||
expect(titleRow?.querySelector('[class*="iconError"] svg')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('shows an error icon for a single failed read tool', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({
|
||||
toolName: 'read_file',
|
||||
status: 'failed',
|
||||
content: [{ type: 'content', content: { text: 'Permission denied' } }],
|
||||
}),
|
||||
]);
|
||||
|
||||
const titleRow = container.querySelector('[class*="expandedCardTitleRow"]');
|
||||
expect(titleRow).not.toBeNull();
|
||||
expect(titleRow?.querySelector('[class*="iconError"] svg')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('shows an error icon for a single failed tool without result text', () => {
|
||||
const container = renderToolGroup([
|
||||
makeTool({ toolName: 'glob', status: 'failed' }),
|
||||
]);
|
||||
|
||||
const titleRow = container.querySelector('[class*="expandedCardTitleRow"]');
|
||||
expect(titleRow).not.toBeNull();
|
||||
expect(titleRow?.querySelector('[class*="iconError"] svg')).not.toBeNull();
|
||||
});
|
||||
|
||||
it('renders ANSI shell output as styled spans instead of escape text', () => {
|
||||
|
|
@ -1353,7 +1463,9 @@ describe('tool row rendering', () => {
|
|||
|
||||
act(() => header.click());
|
||||
|
||||
const cardTitle = container.querySelector('[class*="expandedCardTitle"]');
|
||||
const cardTitle = container.querySelector(
|
||||
'[class*="expandedCardTitleRow"] [class*="expandedCardTitle"]',
|
||||
);
|
||||
expect(cardTitle?.textContent).toBe('Shell');
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -348,16 +348,21 @@ function ExpandedEditContent({ tool }: { tool: ACPToolCall }) {
|
|||
function ToolExpandedCard({
|
||||
title,
|
||||
detail,
|
||||
status,
|
||||
children,
|
||||
}: {
|
||||
title: string;
|
||||
detail?: string;
|
||||
status?: ACPToolCall['status'];
|
||||
children?: ReactNode;
|
||||
}) {
|
||||
return (
|
||||
<div className={styles.expandedCard}>
|
||||
<div className={styles.expandedCardHeader}>
|
||||
<span className={styles.expandedCardTitle}>{title}</span>
|
||||
<span className={styles.expandedCardTitleRow}>
|
||||
{status && <StatusIcon status={status} />}
|
||||
<span className={styles.expandedCardTitle}>{title}</span>
|
||||
</span>
|
||||
{detail && <span className={styles.expandedCardDetail}>{detail}</span>}
|
||||
</div>
|
||||
{children && <div className={styles.expandedCardBody}>{children}</div>}
|
||||
|
|
@ -397,7 +402,7 @@ function TodoToolBody({
|
|||
const timeline = useContext(TodoTimelineContext);
|
||||
const events = timeline.get(tool.callId)?.events ?? [];
|
||||
return expanded ? (
|
||||
<ToolExpandedCard title={title}>
|
||||
<ToolExpandedCard title={title} status={tool.status}>
|
||||
<div className={styles.todoBody}>
|
||||
<TodoFullList todos={todos} />
|
||||
</div>
|
||||
|
|
@ -1301,8 +1306,13 @@ export const ToolLine = memo(function ToolLine({
|
|||
const hideDescriptionInHeader =
|
||||
showDescriptionInDetail && !isShell && !isSearch && !isRead;
|
||||
const expandedCardDetail = fullDescription;
|
||||
// A failed tool with no result text still gets the titled card so its
|
||||
// title-row error icon remains visible when expanded.
|
||||
const showExpandedSummaryPanel =
|
||||
!isTodo && expanded && !detailView && (showDescriptionInDetail || result);
|
||||
!isTodo &&
|
||||
expanded &&
|
||||
!detailView &&
|
||||
(showDescriptionInDetail || result || tool.status === 'failed');
|
||||
|
||||
return (
|
||||
<div className={styles.line}>
|
||||
|
|
@ -1413,7 +1423,11 @@ export const ToolLine = memo(function ToolLine({
|
|||
</div>
|
||||
)}
|
||||
{showExpandedSummaryPanel && (
|
||||
<ToolExpandedCard title={displayName} detail={expandedCardDetail}>
|
||||
<ToolExpandedCard
|
||||
title={displayName}
|
||||
detail={expandedCardDetail}
|
||||
status={tool.status}
|
||||
>
|
||||
{result && (
|
||||
<div
|
||||
className={`${styles.lineOutput} ${styles.expandedLineOutput}`}
|
||||
|
|
@ -1448,9 +1462,15 @@ export const ToolLine = memo(function ToolLine({
|
|||
}
|
||||
>
|
||||
{isRead ? (
|
||||
<ExpandedReadContent tool={tool} />
|
||||
<ToolExpandedCard title={displayName} status={tool.status}>
|
||||
<ExpandedReadContent tool={tool} />
|
||||
</ToolExpandedCard>
|
||||
) : (
|
||||
<ToolExpandedCard title={displayName} detail={expandedCardDetail}>
|
||||
<ToolExpandedCard
|
||||
title={displayName}
|
||||
detail={expandedCardDetail}
|
||||
status={tool.status}
|
||||
>
|
||||
{isShellToolName(name) && <ExpandedBashOutput tool={tool} />}
|
||||
{(name === 'write_file' || name === 'writefile') && (
|
||||
<ExpandedEditContent tool={tool} />
|
||||
|
|
@ -1485,7 +1505,6 @@ export const ToolGroup = memo(function ToolGroup({
|
|||
const [chatExpanded, setChatExpanded] = useState(false);
|
||||
const monitorDetailsRequestRef = useRef<object | null>(null);
|
||||
const hasRunningTool = hasActiveAgents(tools);
|
||||
const hasFailedTool = tools.some((tool) => tool.status === 'failed');
|
||||
const activeTool =
|
||||
tools.find(
|
||||
(tool) =>
|
||||
|
|
@ -1563,7 +1582,6 @@ export const ToolGroup = memo(function ToolGroup({
|
|||
<ToolGroupIcon />
|
||||
)}
|
||||
</span>
|
||||
{hasFailedTool && <StatusIcon status="failed" />}
|
||||
<span
|
||||
className={
|
||||
animateSummary
|
||||
|
|
|
|||
|
|
@ -53,13 +53,6 @@
|
|||
display: block;
|
||||
}
|
||||
|
||||
.summaryStatus {
|
||||
display: inline-flex;
|
||||
align-items: center;
|
||||
flex-shrink: 0;
|
||||
line-height: 1;
|
||||
}
|
||||
|
||||
.summaryToolIcon {
|
||||
width: 14px;
|
||||
height: 14px;
|
||||
|
|
|
|||
|
|
@ -1284,6 +1284,73 @@ describe('ParallelAgentsGroup activity rendering', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('shows a failed count in the collapsed summary', () => {
|
||||
const container = renderExpandedGroup([
|
||||
agent({ callId: 'done', status: 'completed' }),
|
||||
agent({ callId: 'failed', status: 'failed' }),
|
||||
]);
|
||||
|
||||
expect(container.textContent).toContain('2/2 done');
|
||||
expect(container.textContent).toContain('1 failed');
|
||||
expect(container.textContent).not.toContain('Failed');
|
||||
expect(
|
||||
groupSummary(container).querySelector('[class*="iconError"]'),
|
||||
).toBeNull();
|
||||
// The failed count must precede the done counter: summaryText truncates
|
||||
// from the tail, so this order keeps failure evidence visible when the
|
||||
// row is narrow.
|
||||
const summaryText = groupSummary(container).textContent ?? '';
|
||||
expect(summaryText.indexOf('1 failed')).toBeGreaterThanOrEqual(0);
|
||||
expect(summaryText.indexOf('1 failed')).toBeLessThan(
|
||||
summaryText.indexOf('2/2 done'),
|
||||
);
|
||||
});
|
||||
|
||||
it('counts a cancelled agent in the failed count', () => {
|
||||
const container = renderExpandedGroup([
|
||||
agent({ callId: 'done', status: 'completed' }),
|
||||
agent({
|
||||
callId: 'cancelled',
|
||||
status: 'completed',
|
||||
rawOutput: {
|
||||
type: 'task_execution',
|
||||
status: 'cancelled',
|
||||
reason: 'Cancelled by user',
|
||||
},
|
||||
}),
|
||||
]);
|
||||
|
||||
expect(container.textContent).toContain('1 failed');
|
||||
});
|
||||
|
||||
it('shows the failed count alongside live progress', () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(10_000);
|
||||
try {
|
||||
const container = renderExpandedGroup([
|
||||
agent({
|
||||
callId: 'done',
|
||||
status: 'completed',
|
||||
startTime: 1_000,
|
||||
endTime: 5_000,
|
||||
}),
|
||||
agent({
|
||||
callId: 'failed',
|
||||
status: 'failed',
|
||||
startTime: 2_000,
|
||||
endTime: 6_000,
|
||||
}),
|
||||
agent({ callId: 'running', status: 'pending', startTime: 3_000 }),
|
||||
]);
|
||||
|
||||
expect(container.textContent).toContain('7s');
|
||||
expect(container.textContent).toContain('2/3 done');
|
||||
expect(container.textContent).toContain('1 failed');
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the header clock monotonic when the earliest agent finishes', () => {
|
||||
vi.useFakeTimers();
|
||||
vi.setSystemTime(150_000);
|
||||
|
|
|
|||
|
|
@ -4,12 +4,7 @@ import type { ACPToolCall, PermissionRequest } from '../../../adapters/types';
|
|||
import { hasActiveAgents } from '../../../adapters/toolClassification';
|
||||
import { useI18n } from '../../../i18n';
|
||||
import { useSubagentDetails } from '../../../subagentDetailsContext';
|
||||
import {
|
||||
formatElapsed,
|
||||
formatLiveElapsed,
|
||||
StatusIcon,
|
||||
truncateText,
|
||||
} from './toolDisplay';
|
||||
import { formatElapsed, formatLiveElapsed, truncateText } from './toolDisplay';
|
||||
import {
|
||||
getTaskExecutionRecord,
|
||||
getAgentType,
|
||||
|
|
@ -357,19 +352,15 @@ export function ParallelAgentsGroup({
|
|||
const doneCount = agents.filter(
|
||||
(a) => a.status === 'completed' || a.status === 'failed',
|
||||
).length;
|
||||
const failedCount = agents.filter(
|
||||
(a) => getAgentDisplayStatus(a) === 'failed',
|
||||
).length;
|
||||
const total = agents.length;
|
||||
|
||||
const showGroup = groupExpanded || !!approvalAgent;
|
||||
const renderGroup = showGroup || automaticCollapseAnimating;
|
||||
const automaticCollapseClosing =
|
||||
automaticCollapseAnimating && !hasApprovalAgent;
|
||||
const summaryStatus = agents.some(
|
||||
(a) => getAgentDisplayStatus(a) === 'failed',
|
||||
)
|
||||
? 'failed'
|
||||
: hasActive
|
||||
? 'in_progress'
|
||||
: 'completed';
|
||||
|
||||
return (
|
||||
<div className={styles.wrap} ref={wrapRef}>
|
||||
|
|
@ -396,15 +387,9 @@ export function ParallelAgentsGroup({
|
|||
aria-expanded={showGroup}
|
||||
title={showGroup ? t('tool.collapseHint') : t('tool.expand')}
|
||||
>
|
||||
{summaryStatus === 'failed' ? (
|
||||
<span className={styles.summaryStatus}>
|
||||
<StatusIcon status={summaryStatus} />
|
||||
</span>
|
||||
) : (
|
||||
<span className={styles.summaryIcon} aria-hidden="true">
|
||||
<ToolGroupIcon />
|
||||
</span>
|
||||
)}
|
||||
<span className={styles.summaryIcon} aria-hidden="true">
|
||||
<ToolGroupIcon />
|
||||
</span>
|
||||
<span
|
||||
className={
|
||||
hasActive
|
||||
|
|
@ -414,6 +399,14 @@ export function ParallelAgentsGroup({
|
|||
>
|
||||
{t('parallelAgents.title')}
|
||||
{runningDuration && <> {runningDuration}</>}
|
||||
{/* Ahead of the done counter so it survives the summaryText
|
||||
tail truncation in narrow layouts. */}
|
||||
{failedCount > 0 && (
|
||||
<>
|
||||
<span className={styles.summaryDot}>·</span>
|
||||
{t('parallelAgents.failed', { count: failedCount })}
|
||||
</>
|
||||
)}
|
||||
<span className={styles.summaryDot}>·</span>
|
||||
{t('parallelAgents.done', { done: doneCount, total })}
|
||||
</span>
|
||||
|
|
|
|||
|
|
@ -58,6 +58,21 @@ function makeAgentWithSubTool(subTool: ACPToolCall): ACPToolCall {
|
|||
}
|
||||
|
||||
describe('SubAgentPanel sub-tool timestamps', () => {
|
||||
it('marks a failed sub-tool with an error icon instead of text', () => {
|
||||
const container = renderPanel(
|
||||
makeAgentWithSubTool({
|
||||
callId: 'sub-1',
|
||||
toolName: 'Read',
|
||||
status: 'failed',
|
||||
}),
|
||||
);
|
||||
|
||||
const errorIcon = container.querySelector('[class*="iconError"]');
|
||||
expect(errorIcon).not.toBeNull();
|
||||
expect(errorIcon?.querySelector('svg')).not.toBeNull();
|
||||
expect(container.textContent).not.toContain('Failed');
|
||||
});
|
||||
|
||||
it('renders completed result content through assistant markdown', () => {
|
||||
const container = renderPanel({
|
||||
callId: 'agent-1',
|
||||
|
|
|
|||
|
|
@ -333,8 +333,18 @@
|
|||
padding: 8px 12px 8px;
|
||||
}
|
||||
|
||||
.expandedCardTitleRow {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 6px;
|
||||
min-width: 0;
|
||||
}
|
||||
|
||||
.expandedCardTitle {
|
||||
flex-shrink: 0;
|
||||
min-width: 0;
|
||||
max-width: 100%;
|
||||
overflow-wrap: anywhere;
|
||||
color: var(--foreground);
|
||||
font-size: 12px;
|
||||
font-weight: 500;
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
import { CircleXIcon } from 'lucide-react';
|
||||
import styles from './ToolChrome.module.css';
|
||||
import { useI18n } from '../../../i18n';
|
||||
export {
|
||||
|
|
@ -17,8 +18,13 @@ export function StatusIcon({ status }: { status: string }) {
|
|||
case 'cancelled':
|
||||
case 'canceled':
|
||||
return (
|
||||
<span className={`${styles.icon} ${styles.iconError}`}>
|
||||
{t('tool.status.failed')}
|
||||
<span
|
||||
className={`${styles.icon} ${styles.iconError}`}
|
||||
role="img"
|
||||
aria-label={t('tool.status.failed')}
|
||||
title={t('tool.status.failed')}
|
||||
>
|
||||
<CircleXIcon size={14} strokeWidth={1.25} aria-hidden="true" />
|
||||
</span>
|
||||
);
|
||||
case 'in_progress':
|
||||
|
|
|
|||
|
|
@ -2201,6 +2201,7 @@ const EN: Messages = {
|
|||
'resume.title': 'Resume Session',
|
||||
'parallelAgents.title': 'Parallel agents',
|
||||
'parallelAgents.done': (v) => `${v?.done ?? 0}/${v?.total ?? 0} done`,
|
||||
'parallelAgents.failed': (v) => `${v?.count ?? 0} failed`,
|
||||
'skills.actions': 'Skill actions',
|
||||
'skills.disable': 'Disable',
|
||||
'skills.disabled': 'Skill disabled.',
|
||||
|
|
@ -4987,6 +4988,7 @@ const ZH: Messages = {
|
|||
'resume.title': '恢复会话',
|
||||
'parallelAgents.title': '并行智能体',
|
||||
'parallelAgents.done': (v) => `${v?.done ?? 0}/${v?.total ?? 0} 完成`,
|
||||
'parallelAgents.failed': (v) => `失败 ${v?.count ?? 0} 个`,
|
||||
'skills.actions': 'Skill 操作',
|
||||
'skills.disable': '禁用',
|
||||
'skills.disabled': 'Skill 已禁用。',
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue