fix(cli): allow ask question dialog to take full window height (#23693)

2026-04-18 09:11:55 -07:00 · 2026-03-25 16:26:34 -07:00
parent b91758bf6b
commit a86935b6de
12 changed files with 494 additions and 53 deletions
--- a/packages/cli/src/ui/components/AskUserDialog.test.tsx
+++ b/packages/cli/src/ui/components/AskUserDialog.test.tsx
@@ -1491,4 +1491,47 @@ describe('AskUserDialog', () => {
      expect(frame).toContain('3.  Option 3');
    });
  });
+
+  it('allows the question to exceed 15 lines in a tall terminal', async () => {
+    const longQuestion = Array.from(
+      { length: 25 },
+      (_, i) => `Line ${i + 1}`,
+    ).join('\n');
+    const questions: Question[] = [
+      {
+        question: longQuestion,
+        header: 'Tall Test',
+        type: QuestionType.CHOICE,
+        options: [
+          { label: 'Option 1', description: 'D1' },
+          { label: 'Option 2', description: 'D2' },
+          { label: 'Option 3', description: 'D3' },
+        ],
+        multiSelect: false,
+        unconstrainedHeight: false,
+      },
+    ];
+
+    const { lastFrame, waitUntilReady } = await renderWithProviders(
+      <AskUserDialog
+        questions={questions}
+        onSubmit={vi.fn()}
+        onCancel={vi.fn()}
+        width={80}
+        availableHeight={40} // Tall terminal
+      />,
+      { width: 80 },
+    );
+
+    await waitFor(async () => {
+      await waitUntilReady();
+      const frame = lastFrame();
+      // Should show more than 15 lines of the question
+      // (The limit was previously 15, so showing Line 20 proves it's working)
+      expect(frame).toContain('Line 20');
+      expect(frame).toContain('Line 25');
+      // Should still show the options
+      expect(frame).toContain('1.  Option 1');
+    });
+  });
 });
--- a/packages/cli/src/ui/components/AskUserDialog.tsx
+++ b/packages/cli/src/ui/components/AskUserDialog.tsx
@@ -855,13 +855,7 @@ const ChoiceQuestionView: React.FC<ChoiceQuestionViewProps> = ({
    listHeight && !isAlternateBuffer
      ? question.unconstrainedHeight
        ? Math.max(1, listHeight - selectionItems.length * 2)
-        : Math.min(
-            15,
-            Math.max(
-              1,
-              listHeight - Math.max(DIALOG_PADDING, reservedListHeight),
-            ),
-          )
+        : Math.max(1, listHeight - Math.max(DIALOG_PADDING, reservedListHeight))
      : undefined;

  const maxItemsToShow =
--- a/packages/cli/src/ui/components/MainContent.test.tsx
+++ b/packages/cli/src/ui/components/MainContent.test.tsx
@@ -21,6 +21,10 @@ import {
  type UIState,
 } from '../contexts/UIStateContext.js';
 import { type IndividualToolCallDisplay } from '../types.js';
+import {
+  type ConfirmingToolState,
+  useConfirmingTool,
+} from '../hooks/useConfirmingTool.js';

 // Mock dependencies
 const mockUseSettings = vi.fn().mockReturnValue({
@@ -53,6 +57,10 @@ vi.mock('../hooks/useAlternateBuffer.js', () => ({
  useAlternateBuffer: vi.fn(),
 }));

+vi.mock('../hooks/useConfirmingTool.js', () => ({
+  useConfirmingTool: vi.fn(),
+}));
+
 vi.mock('./AppHeader.js', () => ({
  AppHeader: ({ showDetails = true }: { showDetails?: boolean }) => (
    <Text>{showDetails ? 'AppHeader(full)' : 'AppHeader(minimal)'}</Text>
@@ -503,6 +511,54 @@ describe('MainContent', () => {
    unmount();
  });

+  it('renders a subagent with a complete box including bottom border', async () => {
+    const subagentCall = {
+      callId: 'subagent-1',
+      name: 'codebase_investigator',
+      description: 'Investigating codebase',
+      status: CoreToolCallStatus.Executing,
+      kind: 'agent',
+      resultDisplay: {
+        isSubagentProgress: true,
+        agentName: 'codebase_investigator',
+        recentActivity: [
+          {
+            id: '1',
+            type: 'tool_call',
+            content: 'run_shell_command',
+            args: '{"command": "echo hello"}',
+            status: 'running',
+          },
+        ],
+        state: 'running',
+      },
+    } as Partial<IndividualToolCallDisplay> as IndividualToolCallDisplay;
+
+    const uiState = {
+      ...defaultMockUiState,
+      history: [{ id: 1, type: 'user', text: 'Investigate' }],
+      pendingHistoryItems: [
+        {
+          type: 'tool_group' as const,
+          tools: [subagentCall],
+          borderBottom: true,
+        },
+      ],
+    };
+
+    const { lastFrame, unmount } = await renderWithProviders(<MainContent />, {
+      uiState: uiState as Partial<UIState>,
+      config: makeFakeConfig({ useAlternateBuffer: false }),
+    });
+
+    await waitFor(() => {
+      expect(lastFrame()).toContain('codebase_investigator');
+    });
+
+    expect(lastFrame()).toMatchSnapshot();
+    unmount();
+  });
+
  it('renders a split tool group without a gap between static and pending areas', async () => {
    const toolCalls = [
      {
@@ -547,13 +603,124 @@ describe('MainContent', () => {
    const { lastFrame, unmount } = await renderWithProviders(<MainContent />, {
      uiState: uiState as Partial<UIState>,
    });
-    const output = lastFrame();
-    // Verify Part 1 and Part 2 are rendered.
-    expect(output).toContain('Part 1');
-    expect(output).toContain('Part 2');
+
+    await waitFor(() => {
+      const output = lastFrame();
+      // Verify Part 1 and Part 2 are rendered.
+      expect(output).toContain('Part 1');
+      expect(output).toContain('Part 2');
+    });

    // The snapshot will be the best way to verify there is no gap (empty line) between them.
-    expect(output).toMatchSnapshot();
+    expect(lastFrame()).toMatchSnapshot();
+    unmount();
+  });
+
+  it('renders a ToolConfirmationQueue without an extra line when preceded by hidden tools', async () => {
+    const { ApprovalMode, WRITE_FILE_DISPLAY_NAME } = await import(
+      '@google/gemini-cli-core'
+    );
+    const hiddenToolCalls = [
+      {
+        callId: 'tool-hidden',
+        name: WRITE_FILE_DISPLAY_NAME,
+        approvalMode: ApprovalMode.PLAN,
+        status: CoreToolCallStatus.Success,
+        resultDisplay: 'Hidden content',
+      } as Partial<IndividualToolCallDisplay> as IndividualToolCallDisplay,
+    ];
+
+    const confirmingTool = {
+      tool: {
+        callId: 'call-1',
+        name: 'exit_plan_mode',
+        status: CoreToolCallStatus.AwaitingApproval,
+        confirmationDetails: {
+          type: 'exit_plan_mode' as const,
+          planPath: '/path/to/plan',
+        },
+      },
+      index: 1,
+      total: 1,
+    };
+
+    const uiState = {
+      ...defaultMockUiState,
+      history: [{ id: 1, type: 'user', text: 'Apply plan' }],
+      pendingHistoryItems: [
+        {
+          type: 'tool_group' as const,
+          tools: hiddenToolCalls,
+          borderBottom: true,
+        },
+      ],
+    };
+
+    // We need to mock useConfirmingTool to return our confirmingTool
+    vi.mocked(useConfirmingTool).mockReturnValue(
+      confirmingTool as unknown as ConfirmingToolState,
+    );
+
+    mockUseSettings.mockReturnValue(
+      createMockSettings({
+        security: { enablePermanentToolApproval: true },
+        ui: { errorVerbosity: 'full' },
+      }),
+    );
+
+    const { lastFrame, unmount } = await renderWithProviders(<MainContent />, {
+      uiState: uiState as Partial<UIState>,
+      config: makeFakeConfig({ useAlternateBuffer: false }),
+    });
+
+    await waitFor(() => {
+      const output = lastFrame();
+      // The output should NOT contain 'Hidden content'
+      expect(output).not.toContain('Hidden content');
+      // The output should contain the confirmation header
+      expect(output).toContain('Ready to start implementation?');
+    });
+
+    // Snapshot will reveal if there are extra blank lines
+    expect(lastFrame()).toMatchSnapshot();
+    unmount();
+  });
+
+  it('renders a spurious line when a tool group has only hidden tools and borderBottom true', async () => {
+    const { ApprovalMode, WRITE_FILE_DISPLAY_NAME } = await import(
+      '@google/gemini-cli-core'
+    );
+    const uiState = {
+      ...defaultMockUiState,
+      history: [{ id: 1, type: 'user', text: 'Apply plan' }],
+      pendingHistoryItems: [
+        {
+          type: 'tool_group' as const,
+          tools: [
+            {
+              callId: 'tool-1',
+              name: WRITE_FILE_DISPLAY_NAME,
+              approvalMode: ApprovalMode.PLAN,
+              status: CoreToolCallStatus.Success,
+              resultDisplay: 'hidden',
+            } as Partial<IndividualToolCallDisplay> as IndividualToolCallDisplay,
+          ],
+          borderBottom: true,
+        },
+      ],
+    };
+
+    const { lastFrame, unmount } = await renderWithProviders(<MainContent />, {
+      uiState: uiState as Partial<UIState>,
+      config: makeFakeConfig({ useAlternateBuffer: false }),
+    });
+
+    await waitFor(() => {
+      expect(lastFrame()).toContain('Apply plan');
+    });
+
+    // This snapshot will show no spurious line because the group is now correctly suppressed.
+    expect(lastFrame()).toMatchSnapshot();
    unmount();
  });

--- a/packages/cli/src/ui/components/snapshots/MainContent.test.tsx.snap
+++ b/packages/cli/src/ui/components/snapshots/MainContent.test.tsx.snap
@@ -91,6 +91,19 @@ exports[`MainContent > MainContent Tool Output Height Logic > 'Normal mode - Unc
 "
 `;

+exports[`MainContent > renders a ToolConfirmationQueue without an extra line when preceded by hidden tools 1`] = `
+"AppHeader(full)
+▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
+ > Apply plan                                                                   
+▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
+╭──────────────────────────────────────────────────────────────────────────────╮
+│ Ready to start implementation?                                               │
+│                                                                              │
+│  Error reading plan: Storage must be initialized before use                  │
+╰──────────────────────────────────────────────────────────────────────────────╯
+"
+`;
+
 exports[`MainContent > renders a split tool group without a gap between static and pending areas 1`] = `
 "AppHeader(full)
 ╭──────────────────────────────────────────────────────────────────────────╮
@@ -105,6 +118,30 @@ exports[`MainContent > renders a split tool group without a gap between static a
 "
 `;

+exports[`MainContent > renders a spurious line when a tool group has only hidden tools and borderBottom true 1`] = `
+"AppHeader(full)
+▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
+ > Apply plan                                                                   
+▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
+"
+`;
+
+exports[`MainContent > renders a subagent with a complete box including bottom border 1`] = `
+"AppHeader(full)
+▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
+ > Investigate                                                                  
+▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
+╭──────────────────────────────────────────────────────────────────────────╮
+│ ≡ Running Agent... (ctrl+o to collapse)                                  │
+│                                                                          │
+│ Running subagent codebase_investigator...                                │
+│                                                                          │
+│ ⠋  run_shell_command echo hello                                          │
+│                                                                          │
+╰──────────────────────────────────────────────────────────────────────────╯
+"
+`;
+
 exports[`MainContent > renders mixed history items (user + gemini) with single line padding between them 1`] = `
 "ScrollableList
 AppHeader(full)
--- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx
+++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx
@@ -172,12 +172,10 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
  // If all tools are filtered out (e.g., in-progress AskUser tools, low-verbosity
  // internal errors, plan-mode hidden write/edit), we should not emit standalone
  // border fragments. The only case where an empty group should render is the
-  // explicit "closing slice" (tools: []) used to bridge static/pending sections.
+  // explicit "closing slice" (tools: []) used to bridge static/pending sections,
+  // and only if it's actually continuing an open box from above.
  const isExplicitClosingSlice = allToolCalls.length === 0;
-  if (
-    visibleToolCalls.length === 0 &&
-    (!isExplicitClosingSlice || borderBottomOverride !== true)
-  ) {
+  if (visibleToolCalls.length === 0 && !isExplicitClosingSlice) {
    return null;
  }

@@ -269,19 +267,20 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
            We have to keep the bottom border separate so it doesn't get
            drawn over by the sticky header directly inside it.
           */
-        (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) && (
-          <Box
-            height={0}
-            width={contentWidth}
-            borderLeft={true}
-            borderRight={true}
-            borderTop={false}
-            borderBottom={borderBottomOverride ?? true}
-            borderColor={borderColor}
-            borderDimColor={borderDimColor}
-            borderStyle="round"
-          />
-        )
+        (visibleToolCalls.length > 0 || borderBottomOverride !== undefined) &&
+          borderBottomOverride !== false && (
+            <Box
+              height={0}
+              width={contentWidth}
+              borderLeft={true}
+              borderRight={true}
+              borderTop={false}
+              borderBottom={borderBottomOverride ?? true}
+              borderColor={borderColor}
+              borderDimColor={borderDimColor}
+              borderStyle="round"
+            />
+          )
      }
    </Box>
  );
--- a/packages/cli/src/ui/components/messages/ToolGroupMessageRegression.test.tsx
+++ b/packages/cli/src/ui/components/messages/ToolGroupMessageRegression.test.tsx
@@ -0,0 +1,160 @@
+/**
+ * @license
+ * Copyright 2026 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { renderWithProviders } from '../../../test-utils/render.js';
+import { describe, it, expect } from 'vitest';
+import { ToolGroupMessage } from './ToolGroupMessage.js';
+import {
+  makeFakeConfig,
+  CoreToolCallStatus,
+  ApprovalMode,
+  WRITE_FILE_DISPLAY_NAME,
+  Kind,
+} from '@google/gemini-cli-core';
+import os from 'node:os';
+import { createMockSettings } from '../../../test-utils/settings.js';
+import type { IndividualToolCallDisplay } from '../../types.js';
+
+describe('ToolGroupMessage Regression Tests', () => {
+  const baseMockConfig = makeFakeConfig({
+    model: 'gemini-pro',
+    targetDir: os.tmpdir(),
+  });
+  const fullVerbositySettings = createMockSettings({
+    ui: { errorVerbosity: 'full' },
+  });
+
+  const createToolCall = (
+    overrides: Partial<IndividualToolCallDisplay> = {},
+  ): IndividualToolCallDisplay =>
+    ({
+      callId: 'tool-123',
+      name: 'test-tool',
+      status: CoreToolCallStatus.Success,
+      ...overrides,
+    }) as IndividualToolCallDisplay;
+
+  const createItem = (tools: IndividualToolCallDisplay[]) => ({
+    id: 1,
+    type: 'tool_group' as const,
+    tools,
+  });
+
+  it('Plan Mode: suppresses phantom tool group (hidden tools)', async () => {
+    const toolCalls = [
+      createToolCall({
+        name: WRITE_FILE_DISPLAY_NAME,
+        approvalMode: ApprovalMode.PLAN,
+        status: CoreToolCallStatus.Success,
+      }),
+    ];
+    const item = createItem(toolCalls);
+
+    const { lastFrame, unmount } = await renderWithProviders(
+      <ToolGroupMessage
+        terminalWidth={80}
+        item={item}
+        toolCalls={toolCalls}
+        borderBottom={true}
+      />,
+      { config: baseMockConfig, settings: fullVerbositySettings },
+    );
+
+    expect(lastFrame({ allowEmpty: true })).toBe('');
+    unmount();
+  });
+
+  it('Agent Case: suppresses the bottom border box for ongoing agents (no vertical ticks)', async () => {
+    const toolCalls = [
+      createToolCall({
+        name: 'agent',
+        kind: Kind.Agent,
+        status: CoreToolCallStatus.Executing,
+        resultDisplay: {
+          isSubagentProgress: true,
+          agentName: 'TestAgent',
+          state: 'running',
+          recentActivity: [],
+        },
+      }),
+    ];
+    const item = createItem(toolCalls);
+
+    const { lastFrame, unmount } = await renderWithProviders(
+      <ToolGroupMessage
+        terminalWidth={80}
+        item={item}
+        toolCalls={toolCalls}
+        borderBottom={false} // Ongoing
+      />,
+      { config: baseMockConfig, settings: fullVerbositySettings },
+    );
+
+    const output = lastFrame();
+    expect(output).toContain('Running Agent...');
+    // It should render side borders from the content
+    expect(output).toContain('│');
+    // It should NOT render the bottom border box (no corners ╰ ╯)
+    expect(output).not.toContain('╰');
+    expect(output).not.toContain('╯');
+    unmount();
+  });
+
+  it('Agent Case: renders a bottom border horizontal line for completed agents', async () => {
+    const toolCalls = [
+      createToolCall({
+        name: 'agent',
+        kind: Kind.Agent,
+        status: CoreToolCallStatus.Success,
+        resultDisplay: {
+          isSubagentProgress: true,
+          agentName: 'TestAgent',
+          state: 'completed',
+          recentActivity: [],
+        },
+      }),
+    ];
+    const item = createItem(toolCalls);
+
+    const { lastFrame, unmount } = await renderWithProviders(
+      <ToolGroupMessage
+        terminalWidth={80}
+        item={item}
+        toolCalls={toolCalls}
+        borderBottom={true} // Completed
+      />,
+      { config: baseMockConfig, settings: fullVerbositySettings },
+    );
+
+    const output = lastFrame();
+    // Verify it rendered subagent content
+    expect(output).toContain('Agent');
+    // It should render the bottom horizontal line
+    expect(output).toContain(
+      '╰──────────────────────────────────────────────────────────────────────────╯',
+    );
+    unmount();
+  });
+
+  it('Bridges: still renders a bridge if it has a top border', async () => {
+    const toolCalls: IndividualToolCallDisplay[] = [];
+    const item = createItem(toolCalls);
+
+    const { lastFrame, unmount } = await renderWithProviders(
+      <ToolGroupMessage
+        terminalWidth={80}
+        item={item}
+        toolCalls={toolCalls}
+        borderTop={true}
+        borderBottom={true}
+      />,
+      { config: baseMockConfig, settings: fullVerbositySettings },
+    );
+
+    expect(lastFrame({ allowEmpty: true })).not.toBe('');
+    unmount();
+  });
+});