From 666d7f7782979228d34f15c72f021882da51e646 Mon Sep 17 00:00:00 2001 From: tanzhenxin Date: Fri, 8 May 2026 17:14:18 +0800 Subject: [PATCH] fix(cli): show tool details in subagent approval banner The compactMode early-return in ToolConfirmationMessage hid the per-type body and question, so the inline subagent banner showed only "Approval requested by : / Do you want to proceed?" with three options and no indication of which command, file, or MCP tool was being approved. Move the compact-mode handling to the unified return path so per-type body and question render in compact form too. Compact mode also: - Swaps the type-specific exec/mcp question for the generic prompt (the body already shows the command or labeled server + tool, and the exec rootCommand summary surfaces a pre-existing core parser oddity for heredocs that we'd rather not echo into every banner). - Caps the body at 5 lines with MaxSizedBox so a long heredoc can't push other content off-screen; the overflow indicator tells the user content was elided. - Sets MaxSizedBox overflowDirection="bottom" on exec so the head of the command (the action verb + redirection target) stays visible while the tail elides. --- .../messages/ToolConfirmationMessage.test.tsx | 101 ++++++++++++++ .../messages/ToolConfirmationMessage.tsx | 131 +++++++++++------- 2 files changed, 179 insertions(+), 53 deletions(-) diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx index 17b7ea44ed1..bb18d421ccf 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx @@ -247,4 +247,105 @@ describe('ToolConfirmationMessage', () => { expect(lastFrame()).not.toContain('Modify with external editor'); }); }); + + describe('compactMode', () => { + it('renders the command and exec-specific question for exec confirmations', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command: 'rm -f /tmp/foo.txt', + rootCommand: 'rm', + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('rm -f /tmp/foo.txt'); + expect(frame).toContain('Do you want to proceed?'); + expect(frame).toContain('Yes, allow once'); + expect(frame).toContain('Allow always'); + expect(frame).toContain('No'); + // Compact mode swaps the type-specific exec question for the + // generic prompt (the body already shows the command) and trims + // project/user-scope variants. + expect(frame).not.toContain('Allow execution of:'); + expect(frame).not.toContain('Always allow in this project'); + expect(frame).not.toContain('Always allow for this user'); + }); + + it('renders MCP server and tool name for mcp confirmations', () => { + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'mcp', + title: 'Confirm MCP Tool', + serverName: 'my-server', + toolName: 'my-tool', + toolDisplayName: 'My Tool', + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + const frame = lastFrame() ?? ''; + expect(frame).toContain('MCP Server: my-server'); + expect(frame).toContain('Tool: my-tool'); + expect(frame).toContain('Do you want to proceed?'); + expect(frame).toContain('Yes, allow once'); + expect(frame).toContain('Allow always'); + expect(frame).toContain('No'); + // Compact mode swaps the type-specific mcp question for the + // generic prompt (the body already shows server + tool) and trims + // project/user-scope variants. + expect(frame).not.toContain('Allow execution of MCP tool'); + expect(frame).not.toContain('Always allow in this project'); + expect(frame).not.toContain('Always allow for this user'); + }); + + it('caps multi-line exec body at 5 lines with overflow indicator', () => { + const lines = Array.from({ length: 12 }, (_, i) => `Line ${i + 1}`); + const command = `cat <<'EOF'\n${lines.join('\n')}\nEOF`; + const confirmationDetails: ToolCallConfirmationDetails = { + type: 'exec', + title: 'Confirm Execution', + command, + rootCommand: 'cat', + onConfirm: vi.fn(), + }; + + const { lastFrame } = renderWithProviders( + , + ); + + const frame = lastFrame() ?? ''; + // Head of the command is preserved (so the user sees what's being + // run); the heredoc tail elides behind the overflow indicator. + expect(frame).toContain("cat <<'EOF'"); + expect(frame).toContain('Line 1'); + expect(frame).not.toContain('Line 8'); + expect(frame).not.toContain('Line 12'); + expect(frame).toMatch(/\.{3} last \d+ lines hidden \.{3}/); + }); + }); }); diff --git a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx index 13e2b502898..07b66b2dd25 100644 --- a/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx @@ -31,6 +31,11 @@ import { theme } from '../../semantic-colors.js'; import { t } from '../../../i18n/index.js'; import { AskUserQuestionDialog } from './AskUserQuestionDialog.js'; +// Cap the body height of inline subagent approval banners so a +// multi-line command can't dominate the screen. MaxSizedBox renders +// a "... N more lines" footer past this cap. +const COMPACT_BODY_MAX_LINES = 5; + export interface ToolConfirmationMessageProps { confirmationDetails: ToolCallConfirmationDetails; config: Config; @@ -110,43 +115,6 @@ export const ToolConfirmationMessage: React.FC< const handleSelect = (item: ToolConfirmationOutcome) => handleConfirm(item); - // Compact mode: return simple 3-option display - if (compactMode) { - const compactOptions: Array> = [ - { - key: 'proceed-once', - label: t('Yes, allow once'), - value: ToolConfirmationOutcome.ProceedOnce, - }, - { - key: 'proceed-always', - label: t('Allow always'), - value: ToolConfirmationOutcome.ProceedAlways, - }, - { - key: 'cancel', - label: t('No'), - value: ToolConfirmationOutcome.Cancel, - }, - ]; - - return ( - - - {t('Do you want to proceed?')} - - - - - - ); - } - - // Original logic continues unchanged below let bodyContent: React.ReactNode | null = null; // Removed contextDisplay here let question: string; @@ -168,12 +136,14 @@ export const ToolConfirmationMessage: React.FC< } // Calculate the vertical space (in lines) consumed by UI elements - // surrounding the main body content. - const PADDING_OUTER_Y = 2; // Main container has `padding={1}` (top & bottom). - const MARGIN_BODY_BOTTOM = 1; // margin on the body container. - const HEIGHT_QUESTION = 1; // The question text is one line. - const MARGIN_QUESTION_BOTTOM = 1; // Margin on the question container. - const HEIGHT_OPTIONS = options.length; // Each option in the radio select takes one line. + // surrounding the main body content. Compact mode drops outer padding + // and inter-section margins, and renders a fixed 3-option list rather + // than the full options array. + const PADDING_OUTER_Y = compactMode ? 0 : 2; + const MARGIN_BODY_BOTTOM = compactMode ? 0 : 1; + const HEIGHT_QUESTION = 1; + const MARGIN_QUESTION_BOTTOM = compactMode ? 0 : 1; + const HEIGHT_OPTIONS = compactMode ? 3 : options.length; const surroundingElementsHeight = PADDING_OUTER_Y + @@ -284,12 +254,19 @@ export const ToolConfirmationMessage: React.FC< if (bodyContentHeight !== undefined) { bodyContentHeight -= 2; // Account for padding; } + if (compactMode) { + bodyContentHeight = Math.min( + bodyContentHeight ?? COMPACT_BODY_MAX_LINES, + COMPACT_BODY_MAX_LINES, + ); + } bodyContent = ( {executionProps.command} @@ -325,12 +302,18 @@ export const ToolConfirmationMessage: React.FC< value: ToolConfirmationOutcome.Cancel, }); + const planHeight = compactMode + ? Math.min( + availableBodyContentHeight() ?? COMPACT_BODY_MAX_LINES, + COMPACT_BODY_MAX_LINES, + ) + : availableBodyContentHeight(); bodyContent = ( @@ -462,25 +445,67 @@ export const ToolConfirmationMessage: React.FC< }); } + // For exec/mcp confirmations the type-specific question text would + // restate what the body already shows (the full command, or the labeled + // server + tool). Use the generic prompt so the question line acts as a + // body→options transition without duplicating information. + const renderedQuestion = + compactMode && + (confirmationDetails.type === 'exec' || confirmationDetails.type === 'mcp') + ? t('Do you want to proceed?') + : question; + + // Compact mode trims the option list to a fixed 3-option set (the + // project/user-scope "Always allow" variants would clutter the inline + // subagent banner) but still shows the per-type body and question so the + // parent knows what is being approved. + const renderedOptions: Array> = + compactMode + ? [ + { + key: 'proceed-once', + label: t('Yes, allow once'), + value: ToolConfirmationOutcome.ProceedOnce, + }, + { + key: 'proceed-always', + label: t('Allow always'), + value: ToolConfirmationOutcome.ProceedAlways, + }, + { + key: 'cancel', + label: t('No'), + value: ToolConfirmationOutcome.Cancel, + }, + ] + : options; + + // Compact mode strips outer padding, inter-section margins, and explicit + // width — the parent (SubagentExecutionRenderer) already provides those. + const outerPadding = compactMode ? 0 : 1; + const sectionMargin = compactMode ? 0 : 1; + const outerWidth = compactMode ? undefined : contentWidth; + return ( - - {/* Body Content (Diff Renderer or Command Info) */} - {/* No separate context display here anymore for edits */} - + + {bodyContent} - {/* Confirmation Question */} - + - {question} + {renderedQuestion} - {/* Select Input for Options */}