From 49aa4c8ab584f22aa5aba9d1d5ea59bcbf1d76cc Mon Sep 17 00:00:00 2001 From: AlexHuang Date: Wed, 8 Jul 2026 13:26:47 +0800 Subject: [PATCH] fix(cli): show file path in compact tool summary for single collapsible tools (#6448) * fix(cli): show file path in compact tool summary for single collapsible tools buildToolSummary() previously discarded the description field from collapsible tools (ReadFile, Grep, Glob, ListFiles), showing only generic counts like 'Read 1 file'. Now shows the actual file path or search pattern for single tools, while preserving count format for batches of multiple tools of the same type. Falls back to count format when description is unavailable. Signed-off-by: Alex * fix(cli): sanitize description in buildToolSummary for error args and ANSI When a tool call errors, useReactToolScheduler sets description to JSON.stringify(args) which produces '{...}' blobs. Strip ANSI escape sequences and reject JSON-looking descriptions so the summary falls back to the count format instead of rendering raw JSON. Signed-off-by: Alex * fix(cli): broaden ANSI stripping and replace newlines with spaces in summary Strip all common ANSI escape sequences (OSC, charset, CSI, single-byte ESC) instead of just CSI. Replace all C0 control characters including newlines with spaces so embedded \n in shell descriptions does not break the single-line compact summary layout. Signed-off-by: Alex * test(cli): update ToolGroupMessage expectations for description-based summary The buildToolSummary change from 'Read 1 file' to 'Read a.ts' broke 4 assertions in ToolGroupMessage.test.tsx. Update all 5 occurrences to use the new description-based format. Signed-off-by: Alex --------- Signed-off-by: Alex --- .../messages/CompactToolGroupDisplay.test.tsx | 83 ++++++++++++++++--- .../messages/CompactToolGroupDisplay.tsx | 58 ++++++++++--- .../messages/ToolGroupMessage.test.tsx | 10 +-- 3 files changed, 125 insertions(+), 26 deletions(-) diff --git a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx index 5a037f45cd..cbce2e7b87 100644 --- a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx +++ b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.test.tsx @@ -123,7 +123,7 @@ describe(' — summary label', () => { ); const frame = lastFrame()!; // CATEGORY_ORDER: search → read → list → ... - expect(frame).toContain('Searched 1 pattern'); + expect(frame).toContain('Searched search pattern'); expect(frame).toContain('read 2 files'); }); @@ -145,7 +145,7 @@ describe(' — summary label', () => { const { lastFrame } = render( , ); - expect(lastFrame()).toContain('Ran 1 command'); + expect(lastFrame()).toContain('Ran ls -la'); }); }); @@ -166,12 +166,12 @@ describe('buildToolSummary', () => { expect(buildToolSummary([], false)).toBe(''); }); - it('single tool uses count format', () => { - expect(buildToolSummary([make({})], false)).toBe('Read 1 file'); + it('single tool uses description format', () => { + expect(buildToolSummary([make({})], false)).toBe('Read a.ts'); }); it('single tool uses progressive verb when active', () => { - expect(buildToolSummary([make({})], true)).toBe('Reading 1 file'); + expect(buildToolSummary([make({})], true)).toBe('Reading a.ts'); }); it('multiple same-type tools use count', () => { @@ -191,7 +191,7 @@ describe('buildToolSummary', () => { ]; // CATEGORY_ORDER: search → read → list → command → edit expect(buildToolSummary(tools, false)).toBe( - 'Read 1 file, ran 1 command, edited 1 file', + 'Read a.ts, ran npm test, edited b.ts', ); }); @@ -201,14 +201,77 @@ describe('buildToolSummary', () => { make({ callId: 'c2', name: 'Shell', description: 'ls' }), ]; const result = buildToolSummary(tools, false); - expect(result).toBe('Read 1 file, ran 1 command'); + expect(result).toBe('Read a.ts, ran ls'); }); it('unknown tool names fall to other category', () => { const tools = [ make({ callId: 'c1', name: 'UnknownTool', description: 'something' }), ]; - expect(buildToolSummary(tools, false)).toBe('Used 1 tool'); + expect(buildToolSummary(tools, false)).toBe('Used something'); + }); + + it('falls back to count format when description is empty', () => { + const tools = [make({ callId: 'c1', name: 'ReadFile', description: '' })]; + expect(buildToolSummary(tools, false)).toBe('Read 1 file'); + }); + + it('falls back to count format when description is undefined', () => { + const tools = [ + make({ + callId: 'c1', + name: 'ReadFile', + description: undefined as unknown as string, + }), + ]; + expect(buildToolSummary(tools, false)).toBe('Read 1 file'); + }); + + it('falls back to count format when description is JSON (error args)', () => { + const tools = [ + make({ + callId: 'c1', + name: 'ReadFile', + description: '{"file_path":"/tmp/test.txt"}', + }), + ]; + expect(buildToolSummary(tools, false)).toBe('Read 1 file'); + }); + + it('falls back to count format when description starts with array bracket', () => { + const tools = [ + make({ callId: 'c1', name: 'Shell', description: '["ls", "-la"]' }), + ]; + expect(buildToolSummary(tools, false)).toBe('Ran 1 command'); + }); + + it('strips ANSI CSI escape sequences from description', () => { + const tools = [ + make({ + callId: 'c1', + name: 'ReadFile', + description: '\x1b[32ma.ts\x1b[0m', + }), + ]; + expect(buildToolSummary(tools, false)).toBe('Read a.ts'); + }); + + it('strips non-CSI ANSI sequences (charset, OSC) from description', () => { + const tools = [ + make({ callId: 'c1', name: 'Shell', description: '\x1b(Bls -la\x1b[0m' }), + ]; + expect(buildToolSummary(tools, false)).toBe('Ran ls -la'); + }); + + it('replaces embedded newlines with spaces in description', () => { + const tools = [ + make({ + callId: 'c1', + name: 'Shell', + description: 'echo hello\nworld', + }), + ]; + expect(buildToolSummary(tools, false)).toBe('Ran echo hello world'); }); it('mixed group with count per category', () => { @@ -217,7 +280,7 @@ describe('buildToolSummary', () => { make({ callId: 'c2', name: 'ReadFile', description: 'b.ts' }), make({ callId: 'c3', name: 'Shell', description: 'npm test' }), ]; - expect(buildToolSummary(tools, false)).toBe('Read 2 files, ran 1 command'); + expect(buildToolSummary(tools, false)).toBe('Read 2 files, ran npm test'); }); it('legacy display names map to correct categories', () => { @@ -226,7 +289,7 @@ describe('buildToolSummary', () => { make({ callId: 'c2', name: 'ReadFolder', description: '/src' }), ]; expect(buildToolSummary(tools, false)).toBe( - 'Searched 1 pattern, listed 1 directory', + 'Searched pattern, listed /src', ); }); }); diff --git a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx index e97a700b25..9f115a837e 100644 --- a/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx +++ b/packages/cli/src/ui/components/messages/CompactToolGroupDisplay.tsx @@ -189,14 +189,43 @@ export function isCollapsibleTool(toolName: string): boolean { return COLLAPSIBLE_CATEGORIES.has(getToolCategory(toolName)); } +/** + * Strip ANSI control sequences and reject JSON-looking error fallbacks. + * + * When a tool call errors, `useReactToolScheduler` sets description to + * `JSON.stringify(request.args)` which produces `{...}` blobs. Return + * `undefined` for those so the caller falls back to count format. + */ +function safeDescription(raw: string | undefined): string | undefined { + if (!raw) return undefined; + + /* eslint-disable no-control-regex */ + // Strip all common ANSI escape sequences: OSC, charset, CSI, and single-byte ESC + const stripped = raw.replace( + /\x1b\][^\x07]*\x07|\x1b[()][A-Z0-9]|\x1b\[[0-9;]*[a-zA-Z]|\x1b./g, + '', + ); + // Replace all C0 control characters (including \n, \r) with spaces + const cleaned = stripped.replace(/[\x00-\x1f\x7f]/g, ' ').trim(); + /* eslint-enable no-control-regex */ + + // Reject JSON-looking blobs (error fallback from args) + if (cleaned.startsWith('{') || cleaned.startsWith('[')) return undefined; + + return cleaned || undefined; +} + /** * Build a semantic summary line from a batch of tool calls. * - * Single tool → "Read 1 file" / "Ran 1 command" - * Multi same → "Read 3 files" - * Multi mixed → "Read 3 files, edited 2 files, ran 1 command" + * Single tool (with description) → "Read a.ts" / "Ran ls -la" + * Single tool (no description) → "Read 1 file" / "Ran 1 command" + * Multi same → "Read 3 files" + * Multi mixed → "Read a.ts, ran npm test, edited b.ts" * * Uses past tense when all tools are done, present progressive when active. + * Falls back to count format when description is empty, contains control + * characters, or looks like a JSON blob (e.g. error fallback from args). */ export function buildToolSummary( toolCalls: IndividualToolCallDisplay[], @@ -204,28 +233,35 @@ export function buildToolSummary( ): string { if (toolCalls.length === 0) return ''; - // Group by category and count - const counts = new Map(); + // Group by category to preserve tool references for description access + const toolsByCategory = new Map(); for (const tool of toolCalls) { const cat = getToolCategory(tool.name); - counts.set(cat, (counts.get(cat) ?? 0) + 1); + const arr = toolsByCategory.get(cat) ?? []; + arr.push(tool); + toolsByCategory.set(cat, arr); } const parts: string[] = []; for (const cat of CATEGORY_ORDER) { - const count = counts.get(cat); - if (!count) continue; + const tools = toolsByCategory.get(cat); + if (!tools || tools.length === 0) continue; const template = CATEGORY_TEMPLATES[cat]; const verb = isActive ? template.activeVerb : template.pastVerb; const lower = parts.length > 0; const v = lower ? verb.toLowerCase() : verb; - if (count === 1) { - parts.push(`${v} 1 ${template.singular}`); + if (tools.length === 1) { + const safeDesc = safeDescription(tools[0].description); + if (safeDesc !== undefined) { + parts.push(`${v} ${safeDesc}`); + } else { + parts.push(`${v} 1 ${template.singular}`); + } } else { - parts.push(`${v} ${count} ${template.plural}`); + parts.push(`${v} ${tools.length} ${template.plural}`); } } diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx index 0d702d5c4f..efe39a69c6 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.test.tsx @@ -185,7 +185,7 @@ describe('', () => { ); const frame = lastFrame() ?? ''; // CATEGORY_ORDER: search first (capitalized), then read (lowercased) - expect(frame).toContain('Searched 1 pattern'); + expect(frame).toContain('Searched pattern'); expect(frame).toContain('read 2 files'); expect(frame).not.toContain('MockTool'); }); @@ -204,7 +204,7 @@ describe('', () => { ); const frame = lastFrame() ?? ''; // Collapsible → summary line - expect(frame).toContain('Read 1 file'); + expect(frame).toContain('Read a.ts'); // Non-collapsible → individual ToolMessage expect(frame).toContain('MockTool[s1]'); }); @@ -226,7 +226,7 @@ describe('', () => { // All tools render individually — no summary line expect(frame).toContain('MockTool[r1]'); expect(frame).toContain('MockTool[e1]'); - expect(frame).not.toContain('Read 1 file'); + expect(frame).not.toContain('Read a.ts'); }); it('forceExpandAll passes forceShowResult to Success siblings in error group', () => { @@ -276,7 +276,7 @@ describe('', () => { ); const frame = lastFrame() ?? ''; // Successful ReadFile → summary line - expect(frame).toContain('Read 1 file'); + expect(frame).toContain('Read a.ts'); // Canceled ReadFile → individual ToolMessage (partial output visible) expect(frame).toContain('MockTool[r2]'); }); @@ -306,7 +306,7 @@ describe('', () => { const frame = lastFrame() ?? ''; expect(frame).toContain('Recalled 2 memories'); // Collapsible tool still summarized - expect(frame).toContain('Read 1 file'); + expect(frame).toContain('Read config.yaml'); // Non-collapsible tool rendered individually expect(frame).toContain('MockTool[s1]'); });