From 0957f7d3e2e6080897c56a11547382cc57c33e34 Mon Sep 17 00:00:00 2001 From: Abhijit Balaji Date: Fri, 10 Apr 2026 16:04:59 -0700 Subject: [PATCH] fix(cli): exclude update_topic from confirmation queue count (#24945) --- .../components/messages/ToolGroupMessage.tsx | 45 +---- packages/cli/src/ui/hooks/useGeminiStream.ts | 26 +-- packages/cli/src/ui/utils/confirmingTool.ts | 18 +- packages/cli/src/ui/utils/historyUtils.ts | 18 ++ packages/core/src/index.ts | 1 + packages/core/src/utils/tool-utils.test.ts | 107 +---------- packages/core/src/utils/tool-utils.ts | 67 +------ .../core/src/utils/tool-visibility.test.ts | 173 ++++++++++++++++++ packages/core/src/utils/tool-visibility.ts | 164 +++++++++++++++++ 9 files changed, 385 insertions(+), 234 deletions(-) create mode 100644 packages/core/src/utils/tool-visibility.test.ts create mode 100644 packages/core/src/utils/tool-visibility.ts diff --git a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx index 3a37f3ff5e..38be3a24da 100644 --- a/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx +++ b/packages/cli/src/ui/components/messages/ToolGroupMessage.tsx @@ -22,8 +22,7 @@ import { theme } from '../../semantic-colors.js'; import { useConfig } from '../../contexts/ConfigContext.js'; import { isShellTool } from './ToolShared.js'; import { - shouldHideToolCall, - CoreToolCallStatus, + isVisibleInToolGroup, Kind, EDIT_DISPLAY_NAME, GLOB_DISPLAY_NAME, @@ -36,6 +35,7 @@ import { READ_MANY_FILES_DISPLAY_NAME, isFileDiff, } from '@google/gemini-cli-core'; +import { buildToolVisibilityContextFromDisplay } from '../../utils/historyUtils.js'; import { useUIState } from '../../contexts/UIStateContext.js'; import { getToolGroupBorderAppearance } from '../../utils/borderStyles.js'; import { useSettings } from '../../contexts/SettingsContext.js'; @@ -125,40 +125,13 @@ export const ToolGroupMessage: React.FC = ({ // Filter out tool calls that should be hidden (e.g. in-progress Ask User, or Plan Mode operations). const visibleToolCalls = useMemo( () => - allToolCalls.filter((t) => { - // Hide internal errors unless full verbosity - if ( - isLowErrorVerbosity && - t.status === CoreToolCallStatus.Error && - !t.isClientInitiated - ) { - return false; - } - // Standard hiding logic (e.g. Plan Mode internal edits) - if ( - shouldHideToolCall({ - displayName: t.name, - status: t.status, - approvalMode: t.approvalMode, - hasResultDisplay: !!t.resultDisplay, - parentCallId: t.parentCallId, - }) - ) { - return false; - } - - // We HIDE tools that are still in pre-execution states (Confirming, Pending) - // from the History log. They live in the Global Queue or wait for their turn. - // Only show tools that are actually running or finished. - const displayStatus = mapCoreStatusToDisplayStatus(t.status); - - // We hide Confirming tools from the history log because they are - // currently being rendered in the interactive ToolConfirmationQueue. - // We show everything else, including Pending (waiting to run) and - // Canceled (rejected by user), to ensure the history is complete - // and to avoid tools "vanishing" after approval. - return displayStatus !== ToolCallStatus.Confirming; - }), + allToolCalls.filter((t) => + // Use the unified visibility utility + isVisibleInToolGroup( + buildToolVisibilityContextFromDisplay(t), + isLowErrorVerbosity ? 'low' : 'full', + ), + ), [allToolCalls, isLowErrorVerbosity], ); diff --git a/packages/cli/src/ui/hooks/useGeminiStream.ts b/packages/cli/src/ui/hooks/useGeminiStream.ts index c0e3fcdd04..eee0241a58 100644 --- a/packages/cli/src/ui/hooks/useGeminiStream.ts +++ b/packages/cli/src/ui/hooks/useGeminiStream.ts @@ -39,7 +39,8 @@ import { isBackgroundExecutionData, Kind, ACTIVATE_SKILL_TOOL_NAME, - shouldHideToolCall, + isRenderedInHistory, + buildToolVisibilityContext, UPDATE_TOPIC_TOOL_NAME, UPDATE_TOPIC_DISPLAY_NAME, } from '@google/gemini-cli-core'; @@ -647,29 +648,8 @@ export const useGeminiStream = ( toolCalls.every((tc) => pushedToolCallIds.has(tc.request.callId)); const isToolVisible = (tc: TrackedToolCall) => { - const displayName = tc.tool?.displayName ?? tc.request.name; - - let hasResultDisplay = false; - if ( - tc.status === CoreToolCallStatus.Success || - tc.status === CoreToolCallStatus.Error || - tc.status === CoreToolCallStatus.Cancelled - ) { - hasResultDisplay = !!tc.response?.resultDisplay; - } else if (tc.status === CoreToolCallStatus.Executing) { - hasResultDisplay = !!tc.liveOutput; - } - // AskUser tools and Plan Mode write/edit are handled by this logic - if ( - shouldHideToolCall({ - displayName, - status: tc.status, - approvalMode: tc.approvalMode, - hasResultDisplay, - parentCallId: tc.request.parentCallId, - }) - ) { + if (!isRenderedInHistory(buildToolVisibilityContext(tc))) { return false; } diff --git a/packages/cli/src/ui/utils/confirmingTool.ts b/packages/cli/src/ui/utils/confirmingTool.ts index c7edf8d790..2fb3c7df5e 100644 --- a/packages/cli/src/ui/utils/confirmingTool.ts +++ b/packages/cli/src/ui/utils/confirmingTool.ts @@ -4,12 +4,18 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { CoreToolCallStatus } from '@google/gemini-cli-core'; +import { + CoreToolCallStatus, + belongsInConfirmationQueue, +} from '@google/gemini-cli-core'; import { type HistoryItemWithoutId, type IndividualToolCallDisplay, } from '../types.js'; -import { getAllToolCalls } from './historyUtils.js'; +import { + getAllToolCalls, + buildToolVisibilityContextFromDisplay, +} from './historyUtils.js'; export interface ConfirmingToolState { tool: IndividualToolCallDisplay; @@ -33,14 +39,18 @@ export function getConfirmingToolState( return null; } + const actionablePendingTools = allPendingTools.filter((tool) => + belongsInConfirmationQueue(buildToolVisibilityContextFromDisplay(tool)), + ); + const head = confirmingTools[0]; - const headIndexInFullList = allPendingTools.findIndex( + const headIndexInFullList = actionablePendingTools.findIndex( (tool) => tool.callId === head.callId, ); return { tool: head, index: headIndexInFullList + 1, - total: allPendingTools.length, + total: actionablePendingTools.length, }; } diff --git a/packages/cli/src/ui/utils/historyUtils.ts b/packages/cli/src/ui/utils/historyUtils.ts index ee607dca96..938879fe66 100644 --- a/packages/cli/src/ui/utils/historyUtils.ts +++ b/packages/cli/src/ui/utils/historyUtils.ts @@ -4,6 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ +import { type ToolVisibilityContext } from '@google/gemini-cli-core'; import { CoreToolCallStatus } from '../types.js'; import type { HistoryItem, @@ -12,6 +13,23 @@ import type { IndividualToolCallDisplay, } from '../types.js'; +/** + * Maps an IndividualToolCallDisplay from the CLI to a ToolVisibilityContext for core logic. + */ +export function buildToolVisibilityContextFromDisplay( + tool: IndividualToolCallDisplay, +): ToolVisibilityContext { + return { + name: tool.originalRequestName ?? tool.name, + displayName: tool.name, // In CLI, 'name' is usually the resolved display name + status: tool.status, + hasResult: !!tool.resultDisplay, + approvalMode: tool.approvalMode, + isClientInitiated: tool.isClientInitiated, + parentCallId: tool.parentCallId, + }; +} + export function getLastTurnToolCallIds( history: HistoryItem[], pendingHistoryItems: HistoryItemWithoutId[], diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 04456a2964..3f065da0af 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -100,6 +100,7 @@ export { PRIORITY_YOLO_ALLOW_ALL, } from './policy/types.js'; export * from './utils/tool-utils.js'; +export * from './utils/tool-visibility.js'; export * from './utils/terminalSerializer.js'; export * from './utils/systemEncoding.js'; export * from './utils/textUtils.js'; diff --git a/packages/core/src/utils/tool-utils.test.ts b/packages/core/src/utils/tool-utils.test.ts index cddbec66b0..3a53020294 100644 --- a/packages/core/src/utils/tool-utils.test.ts +++ b/packages/core/src/utils/tool-utils.test.ts @@ -5,113 +5,10 @@ */ import { expect, describe, it } from 'vitest'; -import { - doesToolInvocationMatch, - getToolSuggestion, - shouldHideToolCall, -} from './tool-utils.js'; -import { - ReadFileTool, - ApprovalMode, - CoreToolCallStatus, - ASK_USER_DISPLAY_NAME, - WRITE_FILE_DISPLAY_NAME, - EDIT_DISPLAY_NAME, - READ_FILE_DISPLAY_NAME, - type AnyToolInvocation, - type Config, -} from '../index.js'; +import { doesToolInvocationMatch, getToolSuggestion } from './tool-utils.js'; +import { ReadFileTool, type AnyToolInvocation, type Config } from '../index.js'; import { createMockMessageBus } from '../test-utils/mock-message-bus.js'; -describe('shouldHideToolCall', () => { - it.each([ - { - status: CoreToolCallStatus.Scheduled, - hasResult: true, - shouldHide: true, - }, - { - status: CoreToolCallStatus.Executing, - hasResult: true, - shouldHide: true, - }, - { - status: CoreToolCallStatus.AwaitingApproval, - hasResult: true, - shouldHide: true, - }, - { - status: CoreToolCallStatus.Validating, - hasResult: true, - shouldHide: true, - }, - { - status: CoreToolCallStatus.Success, - hasResult: true, - shouldHide: false, - }, - { - status: CoreToolCallStatus.Error, - hasResult: false, - shouldHide: true, - }, - { - status: CoreToolCallStatus.Error, - hasResult: true, - shouldHide: false, - }, - ])( - 'AskUser: status=$status, hasResult=$hasResult -> hide=$shouldHide', - ({ status, hasResult, shouldHide }) => { - expect( - shouldHideToolCall({ - displayName: ASK_USER_DISPLAY_NAME, - status, - hasResultDisplay: hasResult, - }), - ).toBe(shouldHide); - }, - ); - - it.each([ - { - name: WRITE_FILE_DISPLAY_NAME, - mode: ApprovalMode.PLAN, - visible: false, - }, - { name: EDIT_DISPLAY_NAME, mode: ApprovalMode.PLAN, visible: false }, - { - name: WRITE_FILE_DISPLAY_NAME, - mode: ApprovalMode.DEFAULT, - visible: true, - }, - { name: READ_FILE_DISPLAY_NAME, mode: ApprovalMode.PLAN, visible: true }, - ])( - 'Plan Mode: tool=$name, mode=$mode -> visible=$visible', - ({ name, mode, visible }) => { - expect( - shouldHideToolCall({ - displayName: name, - status: CoreToolCallStatus.Success, - approvalMode: mode, - hasResultDisplay: true, - }), - ).toBe(!visible); - }, - ); - - it('hides tool calls with a parentCallId', () => { - expect( - shouldHideToolCall({ - displayName: 'any_tool', - status: CoreToolCallStatus.Success, - hasResultDisplay: true, - parentCallId: 'some-parent', - }), - ).toBe(true); - }); -}); - describe('getToolSuggestion', () => { it('should suggest the top N closest tool names for a typo', () => { const allToolNames = ['list_files', 'read_file', 'write_file']; diff --git a/packages/core/src/utils/tool-utils.ts b/packages/core/src/utils/tool-utils.ts index 591df6a87b..fcc10d127d 100644 --- a/packages/core/src/utils/tool-utils.ts +++ b/packages/core/src/utils/tool-utils.ts @@ -11,16 +11,7 @@ import { } from '../index.js'; import { SHELL_TOOL_NAMES } from './shell-utils.js'; import levenshtein from 'fast-levenshtein'; -import { ApprovalMode } from '../policy/types.js'; -import { - CoreToolCallStatus, - type ToolCallResponseInfo, -} from '../scheduler/types.js'; -import { - ASK_USER_DISPLAY_NAME, - WRITE_FILE_DISPLAY_NAME, - EDIT_DISPLAY_NAME, -} from '../tools/tool-names.js'; +import type { ToolCallResponseInfo } from '../scheduler/types.js'; /** * Validates if an object is a ToolCallResponseInfo. @@ -36,62 +27,6 @@ export function isToolCallResponseInfo( ); } -/** - * Options for determining if a tool call should be hidden in the CLI history. - */ -export interface ShouldHideToolCallParams { - /** The display name of the tool. */ - displayName: string; - /** The current status of the tool call. */ - status: CoreToolCallStatus; - /** The approval mode active when the tool was called. */ - approvalMode?: ApprovalMode; - /** Whether the tool has produced a result for display. */ - hasResultDisplay: boolean; - /** The ID of the parent tool call, if any. */ - parentCallId?: string; -} - -/** - * Determines if a tool call should be hidden from the standard tool history UI. - * - * We hide tools in several cases: - * 1. Tool calls that have a parent, as they are "internal" to another tool (e.g. subagent). - * 2. Ask User tools that are in progress, displayed via specialized UI. - * 3. Ask User tools that errored without result display, typically param - * validation errors that the agent automatically recovers from. - * 4. WriteFile and Edit tools when in Plan Mode, redundant because the - * resulting plans are displayed separately upon exiting plan mode. - */ -export function shouldHideToolCall(params: ShouldHideToolCallParams): boolean { - const { displayName, status, approvalMode, hasResultDisplay, parentCallId } = - params; - - if (parentCallId) { - return true; - } - - switch (displayName) { - case ASK_USER_DISPLAY_NAME: - switch (status) { - case CoreToolCallStatus.Scheduled: - case CoreToolCallStatus.Validating: - case CoreToolCallStatus.Executing: - case CoreToolCallStatus.AwaitingApproval: - return true; - case CoreToolCallStatus.Error: - return !hasResultDisplay; - default: - return false; - } - case WRITE_FILE_DISPLAY_NAME: - case EDIT_DISPLAY_NAME: - return approvalMode === ApprovalMode.PLAN; - default: - return false; - } -} - /** * Generates a suggestion string for a tool name that was not found in the registry. * It finds the closest matches based on Levenshtein distance. diff --git a/packages/core/src/utils/tool-visibility.test.ts b/packages/core/src/utils/tool-visibility.test.ts new file mode 100644 index 0000000000..ea9865efd1 --- /dev/null +++ b/packages/core/src/utils/tool-visibility.test.ts @@ -0,0 +1,173 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { expect, describe, it } from 'vitest'; +import { + isRenderedInHistory, + belongsInConfirmationQueue, + isVisibleInToolGroup, +} from './tool-visibility.js'; +import { CoreToolCallStatus } from '../scheduler/types.js'; +import { ApprovalMode } from '../policy/types.js'; +import { + ASK_USER_DISPLAY_NAME, + WRITE_FILE_DISPLAY_NAME, + EDIT_DISPLAY_NAME, + UPDATE_TOPIC_TOOL_NAME, + READ_FILE_DISPLAY_NAME, +} from '../tools/tool-names.js'; + +describe('ToolVisibility Rules', () => { + const createCtx = (overrides = {}) => ({ + name: 'some_tool', + displayName: 'Some Tool', + status: CoreToolCallStatus.Success, + hasResult: true, + parentCallId: undefined, + isClientInitiated: false, + ...overrides, + }); + + describe('isRenderedInHistory', () => { + it('hides tools with parents', () => { + expect( + isRenderedInHistory(createCtx({ parentCallId: 'parent-123' })), + ).toBe(false); + }); + + it('hides AskUser errors without results', () => { + expect( + isRenderedInHistory( + createCtx({ + displayName: ASK_USER_DISPLAY_NAME, + status: CoreToolCallStatus.Error, + hasResult: false, + }), + ), + ).toBe(false); + }); + + it('shows AskUser success', () => { + expect( + isRenderedInHistory( + createCtx({ + displayName: ASK_USER_DISPLAY_NAME, + status: CoreToolCallStatus.Success, + }), + ), + ).toBe(true); + }); + + it('hides WriteFile/Edit in Plan Mode', () => { + expect( + isRenderedInHistory( + createCtx({ + displayName: WRITE_FILE_DISPLAY_NAME, + approvalMode: ApprovalMode.PLAN, + }), + ), + ).toBe(false); + expect( + isRenderedInHistory( + createCtx({ + displayName: EDIT_DISPLAY_NAME, + approvalMode: ApprovalMode.PLAN, + }), + ), + ).toBe(false); + }); + + it('shows ReadFile in Plan Mode', () => { + expect( + isRenderedInHistory( + createCtx({ + displayName: READ_FILE_DISPLAY_NAME, + approvalMode: ApprovalMode.PLAN, + }), + ), + ).toBe(true); + }); + }); + + describe('belongsInConfirmationQueue', () => { + it('returns false for update_topic', () => { + expect( + belongsInConfirmationQueue(createCtx({ name: UPDATE_TOPIC_TOOL_NAME })), + ).toBe(false); + }); + + it('returns true for standard tools', () => { + expect( + belongsInConfirmationQueue(createCtx({ name: 'write_file' })), + ).toBe(true); + }); + }); + + describe('isVisibleInToolGroup', () => { + it('shows tools with parents (agent tools)', () => { + expect( + isVisibleInToolGroup(createCtx({ parentCallId: 'parent-123' }), 'full'), + ).toBe(true); + }); + + it('hides WriteFile/Edit in Plan Mode', () => { + expect( + isVisibleInToolGroup( + createCtx({ + displayName: WRITE_FILE_DISPLAY_NAME, + approvalMode: ApprovalMode.PLAN, + }), + 'full', + ), + ).toBe(false); + }); + + it('hides non-client-initiated errors on low verbosity', () => { + expect( + isVisibleInToolGroup( + createCtx({ + status: CoreToolCallStatus.Error, + isClientInitiated: false, + }), + 'low', + ), + ).toBe(false); + }); + + it('shows non-client-initiated errors on full verbosity', () => { + expect( + isVisibleInToolGroup( + createCtx({ + status: CoreToolCallStatus.Error, + isClientInitiated: false, + }), + 'full', + ), + ).toBe(true); + }); + + it('hides confirming tools', () => { + expect( + isVisibleInToolGroup( + createCtx({ status: CoreToolCallStatus.AwaitingApproval }), + 'full', + ), + ).toBe(false); + }); + + it('hides AskUser while in progress', () => { + expect( + isVisibleInToolGroup( + createCtx({ + displayName: ASK_USER_DISPLAY_NAME, + status: CoreToolCallStatus.Executing, + }), + 'full', + ), + ).toBe(false); + }); + }); +}); diff --git a/packages/core/src/utils/tool-visibility.ts b/packages/core/src/utils/tool-visibility.ts new file mode 100644 index 0000000000..70b30011d4 --- /dev/null +++ b/packages/core/src/utils/tool-visibility.ts @@ -0,0 +1,164 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { ApprovalMode } from '../policy/types.js'; +import { CoreToolCallStatus, type ToolCall } from '../scheduler/types.js'; +import { + ASK_USER_DISPLAY_NAME, + WRITE_FILE_DISPLAY_NAME, + EDIT_DISPLAY_NAME, + UPDATE_TOPIC_TOOL_NAME, + UPDATE_TOPIC_DISPLAY_NAME, +} from '../tools/tool-names.js'; + +export interface ToolVisibilityContext { + /** The internal name of the tool. */ + name: string; + /** The display name of the tool. */ + displayName?: string; + /** The current status of the tool call. */ + status: CoreToolCallStatus; + /** The approval mode active when the tool was called. */ + approvalMode?: ApprovalMode; + /** Whether the tool has produced a result for display (e.g., resultDisplay or liveOutput). */ + hasResult: boolean; + /** The ID of the parent tool call, if any. */ + parentCallId?: string; + /** True if the tool was initiated directly by the user via a slash command. */ + isClientInitiated?: boolean; +} + +/** + * Maps a core ToolCall to a ToolVisibilityContext. + */ +export function buildToolVisibilityContext( + tc: ToolCall, +): ToolVisibilityContext { + let hasResult = false; + if ( + tc.status === CoreToolCallStatus.Success || + tc.status === CoreToolCallStatus.Error || + tc.status === CoreToolCallStatus.Cancelled + ) { + hasResult = !!tc.response.resultDisplay; + } else if (tc.status === CoreToolCallStatus.Executing) { + hasResult = !!tc.liveOutput; + } + + return { + name: tc.request.name, + displayName: tc.tool?.displayName ?? tc.request.name, + status: tc.status, + approvalMode: tc.approvalMode, + hasResult, + parentCallId: tc.request.parentCallId, + isClientInitiated: tc.request.isClientInitiated, + }; +} + +/** + * Determines if a tool should ever appear as a completed block in the main chat log. + */ +export function isRenderedInHistory(ctx: ToolVisibilityContext): boolean { + if (ctx.parentCallId) { + return false; + } + + const displayName = ctx.displayName ?? ctx.name; + + switch (displayName) { + case ASK_USER_DISPLAY_NAME: + // We only render AskUser in history if it errored with a result or succeeded. + // If it errored without a result, it was an internal validation failure. + if (ctx.status === CoreToolCallStatus.Error) { + return ctx.hasResult; + } + return ctx.status === CoreToolCallStatus.Success; + + case WRITE_FILE_DISPLAY_NAME: + case EDIT_DISPLAY_NAME: + // In Plan Mode, edits are redundant because the plan shows the diffs. + return ctx.approvalMode !== ApprovalMode.PLAN; + + default: + return true; + } +} + +/** + * Determines if a tool belongs in the Awaiting Approval confirmation queue. + */ +export function belongsInConfirmationQueue( + ctx: ToolVisibilityContext, +): boolean { + const displayName = ctx.displayName ?? ctx.name; + + // Narrative background tools auto-execute and never require confirmation + if ( + ctx.name === UPDATE_TOPIC_TOOL_NAME || + displayName === UPDATE_TOPIC_DISPLAY_NAME + ) { + return false; + } + + // All other standard tools could theoretically require confirmation + return true; +} + +/** + * Determines if a tool should be actively rendered in the dynamic ToolGroupMessage UI right now. + * This takes into account current execution states and UI settings. + */ +export function isVisibleInToolGroup( + ctx: ToolVisibilityContext, + errorVerbosity: 'full' | 'low', +): boolean { + const displayName = ctx.displayName ?? ctx.name; + + // Hide internal errors unless the user explicitly requested full verbosity + if ( + errorVerbosity === 'low' && + ctx.status === CoreToolCallStatus.Error && + !ctx.isClientInitiated + ) { + return false; + } + + // We hide AskUser while it's in progress because it renders in its own modal. + // We also hide terminal states that don't meet history rendering criteria (e.g. errors without results). + if (displayName === ASK_USER_DISPLAY_NAME) { + switch (ctx.status) { + case CoreToolCallStatus.Scheduled: + case CoreToolCallStatus.Validating: + case CoreToolCallStatus.Executing: + case CoreToolCallStatus.AwaitingApproval: + return false; + case CoreToolCallStatus.Error: + return ctx.hasResult; + case CoreToolCallStatus.Success: + return true; + default: + return false; + } + } + + // In Plan Mode, edits are redundant because the plan shows the diffs. + if ( + (displayName === WRITE_FILE_DISPLAY_NAME || + displayName === EDIT_DISPLAY_NAME) && + ctx.approvalMode === ApprovalMode.PLAN + ) { + return false; + } + + // We hide confirming tools from the active group because they render in the + // ToolConfirmationQueue at the bottom of the screen instead. + if (ctx.status === CoreToolCallStatus.AwaitingApproval) { + return false; + } + + return true; +}