feat(plan): hide plan write and edit operations on plans in Plan Mode (#19012)

This commit is contained in:
Jerop Kipruto
2026-02-13 18:15:21 -05:00
committed by GitHub
parent 4e1b3b5f57
commit 9df604b01b
20 changed files with 373 additions and 108 deletions
@@ -10,9 +10,14 @@ import { ToolGroupMessage } from './ToolGroupMessage.js';
import type { IndividualToolCallDisplay } from '../../types.js'; import type { IndividualToolCallDisplay } from '../../types.js';
import { Scrollable } from '../shared/Scrollable.js'; import { Scrollable } from '../shared/Scrollable.js';
import { import {
ASK_USER_DISPLAY_NAME,
makeFakeConfig, makeFakeConfig,
CoreToolCallStatus, CoreToolCallStatus,
ApprovalMode,
ASK_USER_DISPLAY_NAME,
WRITE_FILE_DISPLAY_NAME,
EDIT_DISPLAY_NAME,
READ_FILE_DISPLAY_NAME,
GLOB_DISPLAY_NAME,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import os from 'node:os'; import os from 'node:os';
@@ -511,4 +516,42 @@ describe('<ToolGroupMessage />', () => {
unmount(); unmount();
}); });
}); });
describe('Plan Mode Filtering', () => {
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 },
{ name: GLOB_DISPLAY_NAME, mode: ApprovalMode.PLAN, visible: true },
])('filtering logic for $name in $mode mode', ({ name, mode, visible }) => {
const toolCalls = [
createToolCall({
callId: 'test-call',
name,
approvalMode: mode,
}),
];
const { lastFrame, unmount } = renderWithProviders(
<ToolGroupMessage {...baseProps} toolCalls={toolCalls} />,
{ config: baseMockConfig },
);
if (visible) {
expect(lastFrame()).toContain(name);
} else {
expect(lastFrame()).toBe('');
}
unmount();
});
});
}); });
@@ -15,8 +15,8 @@ import { theme } from '../../semantic-colors.js';
import { useConfig } from '../../contexts/ConfigContext.js'; import { useConfig } from '../../contexts/ConfigContext.js';
import { isShellTool, isThisShellFocused } from './ToolShared.js'; import { isShellTool, isThisShellFocused } from './ToolShared.js';
import { import {
shouldHideAskUserTool,
CoreToolCallStatus, CoreToolCallStatus,
shouldHideToolCall,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import { ShowMoreLines } from '../ShowMoreLines.js'; import { ShowMoreLines } from '../ShowMoreLines.js';
import { useUIState } from '../../contexts/UIStateContext.js'; import { useUIState } from '../../contexts/UIStateContext.js';
@@ -45,13 +45,18 @@ export const ToolGroupMessage: React.FC<ToolGroupMessageProps> = ({
borderTop: borderTopOverride, borderTop: borderTopOverride,
borderBottom: borderBottomOverride, borderBottom: borderBottomOverride,
}) => { }) => {
// Filter out Ask User tools that should be hidden (e.g. in-progress or errors without result) // Filter out tool calls that should be hidden (e.g. in-progress Ask User, or Plan Mode operations).
const toolCalls = useMemo( const toolCalls = useMemo(
() => () =>
allToolCalls.filter((t) => { allToolCalls.filter(
const displayStatus = mapCoreStatusToDisplayStatus(t.status); (t) =>
return !shouldHideAskUserTool(t.name, displayStatus, !!t.resultDisplay); !shouldHideToolCall({
displayName: t.name,
status: t.status,
approvalMode: t.approvalMode,
hasResultDisplay: !!t.resultDisplay,
}), }),
),
[allToolCalls], [allToolCalls],
); );
+1
View File
@@ -95,6 +95,7 @@ export function mapToDisplay(
outputFile, outputFile,
ptyId, ptyId,
correlationId, correlationId,
approvalMode: call.approvalMode,
}; };
}); });
+2
View File
@@ -14,6 +14,7 @@ import {
type RetrieveUserQuotaResponse, type RetrieveUserQuotaResponse,
type SkillDefinition, type SkillDefinition,
type AgentDefinition, type AgentDefinition,
type ApprovalMode,
CoreToolCallStatus, CoreToolCallStatus,
checkExhaustive, checkExhaustive,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
@@ -106,6 +107,7 @@ export interface IndividualToolCallDisplay {
ptyId?: number; ptyId?: number;
outputFile?: string; outputFile?: string;
correlationId?: string; correlationId?: string;
approvalMode?: ApprovalMode;
} }
export interface CompressionProps { export interface CompressionProps {
@@ -2274,4 +2274,87 @@ describe('CoreToolScheduler Sequential Execution', () => {
); );
}); });
}); });
describe('ApprovalMode Preservation', () => {
it('should preserve approvalMode throughout tool lifecycle', async () => {
// Arrange
const executeFn = vi.fn().mockResolvedValue({
llmContent: 'Tool executed',
returnDisplay: 'Tool executed',
});
const mockTool = new MockTool({
name: 'mockTool',
execute: executeFn,
shouldConfirmExecute: MOCK_TOOL_SHOULD_CONFIRM_EXECUTE,
});
const mockToolRegistry = {
getTool: () => mockTool,
getAllToolNames: () => ['mockTool'],
} as unknown as ToolRegistry;
const onAllToolCallsComplete = vi.fn();
const onToolCallsUpdate = vi.fn();
// Set approval mode to PLAN
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
getApprovalMode: () => ApprovalMode.PLAN,
// Ensure policy engine returns ASK_USER to trigger AwaitingApproval state
getPolicyEngine: () =>
({
check: async () => ({ decision: PolicyDecision.ASK_USER }),
}) as unknown as PolicyEngine,
});
mockConfig.getHookSystem = vi.fn().mockReturnValue(undefined);
const scheduler = new CoreToolScheduler({
config: mockConfig,
onAllToolCallsComplete,
onToolCallsUpdate,
getPreferredEditor: () => 'vscode',
});
const abortController = new AbortController();
const request = {
callId: '1',
name: 'mockTool',
args: { param: 'value' },
isClientInitiated: false,
prompt_id: 'test-prompt',
};
// Act - Schedule
const schedulePromise = scheduler.schedule(
request,
abortController.signal,
);
// Assert - Check AwaitingApproval state
const awaitingCall = (await waitForStatus(
onToolCallsUpdate,
CoreToolCallStatus.AwaitingApproval,
)) as WaitingToolCall;
expect(awaitingCall).toBeDefined();
expect(awaitingCall.approvalMode).toBe(ApprovalMode.PLAN);
// Act - Confirm
await (
awaitingCall.confirmationDetails as ToolCallConfirmationDetails
).onConfirm(ToolConfirmationOutcome.ProceedOnce);
// Wait for completion
await schedulePromise;
// Assert - Check Success state
expect(onAllToolCallsComplete).toHaveBeenCalled();
const completedCalls = onAllToolCallsComplete.mock
.calls[0][0] as ToolCall[];
expect(completedCalls).toHaveLength(1);
expect(completedCalls[0].status).toBe(CoreToolCallStatus.Success);
expect(completedCalls[0].approvalMode).toBe(ApprovalMode.PLAN);
});
});
}); });
+15 -1
View File
@@ -217,6 +217,7 @@ export class CoreToolScheduler {
const invocation = currentCall.invocation; const invocation = currentCall.invocation;
const outcome = currentCall.outcome; const outcome = currentCall.outcome;
const approvalMode = currentCall.approvalMode;
switch (newStatus) { switch (newStatus) {
case CoreToolCallStatus.Success: { case CoreToolCallStatus.Success: {
@@ -232,6 +233,7 @@ export class CoreToolScheduler {
response: auxiliaryData as ToolCallResponseInfo, response: auxiliaryData as ToolCallResponseInfo,
durationMs, durationMs,
outcome, outcome,
approvalMode,
} as SuccessfulToolCall; } as SuccessfulToolCall;
} }
case CoreToolCallStatus.Error: { case CoreToolCallStatus.Error: {
@@ -246,6 +248,7 @@ export class CoreToolScheduler {
response: auxiliaryData as ToolCallResponseInfo, response: auxiliaryData as ToolCallResponseInfo,
durationMs, durationMs,
outcome, outcome,
approvalMode,
} as ErroredToolCall; } as ErroredToolCall;
} }
case CoreToolCallStatus.AwaitingApproval: case CoreToolCallStatus.AwaitingApproval:
@@ -259,6 +262,7 @@ export class CoreToolScheduler {
startTime: existingStartTime, startTime: existingStartTime,
outcome, outcome,
invocation, invocation,
approvalMode,
} as WaitingToolCall; } as WaitingToolCall;
case CoreToolCallStatus.Scheduled: case CoreToolCallStatus.Scheduled:
return { return {
@@ -268,6 +272,7 @@ export class CoreToolScheduler {
startTime: existingStartTime, startTime: existingStartTime,
outcome, outcome,
invocation, invocation,
approvalMode,
} as ScheduledToolCall; } as ScheduledToolCall;
case CoreToolCallStatus.Cancelled: { case CoreToolCallStatus.Cancelled: {
const durationMs = existingStartTime const durationMs = existingStartTime
@@ -316,6 +321,7 @@ export class CoreToolScheduler {
}, },
durationMs, durationMs,
outcome, outcome,
approvalMode,
} as CancelledToolCall; } as CancelledToolCall;
} }
case CoreToolCallStatus.Validating: case CoreToolCallStatus.Validating:
@@ -326,6 +332,7 @@ export class CoreToolScheduler {
startTime: existingStartTime, startTime: existingStartTime,
outcome, outcome,
invocation, invocation,
approvalMode,
} as ValidatingToolCall; } as ValidatingToolCall;
case CoreToolCallStatus.Executing: case CoreToolCallStatus.Executing:
return { return {
@@ -335,6 +342,7 @@ export class CoreToolScheduler {
startTime: existingStartTime, startTime: existingStartTime,
outcome, outcome,
invocation, invocation,
approvalMode,
} as ExecutingToolCall; } as ExecutingToolCall;
default: { default: {
const exhaustiveCheck: never = newStatus; const exhaustiveCheck: never = newStatus;
@@ -373,6 +381,7 @@ export class CoreToolScheduler {
status: CoreToolCallStatus.Error, status: CoreToolCallStatus.Error,
tool: call.tool, tool: call.tool,
response, response,
approvalMode: call.approvalMode,
} as ErroredToolCall; } as ErroredToolCall;
} }
@@ -496,6 +505,7 @@ export class CoreToolScheduler {
); );
} }
const requestsToProcess = Array.isArray(request) ? request : [request]; const requestsToProcess = Array.isArray(request) ? request : [request];
const currentApprovalMode = this.config.getApprovalMode();
this.completedToolCallsForBatch = []; this.completedToolCallsForBatch = [];
const newToolCalls: ToolCall[] = requestsToProcess.map( const newToolCalls: ToolCall[] = requestsToProcess.map(
@@ -518,6 +528,7 @@ export class CoreToolScheduler {
ToolErrorType.TOOL_NOT_REGISTERED, ToolErrorType.TOOL_NOT_REGISTERED,
), ),
durationMs: 0, durationMs: 0,
approvalMode: currentApprovalMode,
}; };
} }
@@ -536,6 +547,7 @@ export class CoreToolScheduler {
ToolErrorType.INVALID_TOOL_PARAMS, ToolErrorType.INVALID_TOOL_PARAMS,
), ),
durationMs: 0, durationMs: 0,
approvalMode: currentApprovalMode,
}; };
} }
@@ -545,6 +557,7 @@ export class CoreToolScheduler {
tool: toolInstance, tool: toolInstance,
invocation: invocationOrError, invocation: invocationOrError,
startTime: Date.now(), startTime: Date.now(),
approvalMode: currentApprovalMode,
}; };
}, },
); );
@@ -920,7 +933,7 @@ export class CoreToolScheduler {
this.toolCalls = this.toolCalls.map((tc) => this.toolCalls = this.toolCalls.map((tc) =>
tc.request.callId === completedCall.request.callId tc.request.callId === completedCall.request.callId
? completedCall ? { ...completedCall, approvalMode: tc.approvalMode }
: tc, : tc,
); );
this.notifyToolCallsUpdate(); this.notifyToolCallsUpdate();
@@ -1049,6 +1062,7 @@ export class CoreToolScheduler {
}, },
durationMs, durationMs,
outcome: ToolConfirmationOutcome.Cancel, outcome: ToolConfirmationOutcome.Cancel,
approvalMode: queuedCall.approvalMode,
}); });
} }
} }
@@ -320,6 +320,20 @@ describe('Scheduler (Orchestrator)', () => {
]), ]),
); );
}); });
it('should set approvalMode to PLAN when config returns PLAN', async () => {
mockConfig.getApprovalMode.mockReturnValue(ApprovalMode.PLAN);
await scheduler.schedule(req1, signal);
expect(mockStateManager.enqueue).toHaveBeenCalledWith(
expect.arrayContaining([
expect.objectContaining({
status: CoreToolCallStatus.Validating,
approvalMode: ApprovalMode.PLAN,
}),
]),
);
});
}); });
describe('Phase 2: Queue Management', () => { describe('Phase 2: Queue Management', () => {
+11 -1
View File
@@ -22,6 +22,7 @@ import {
CoreToolCallStatus, CoreToolCallStatus,
} from './types.js'; } from './types.js';
import { ToolErrorType } from '../tools/tool-error.js'; import { ToolErrorType } from '../tools/tool-error.js';
import type { ApprovalMode } from '../policy/types.js';
import { PolicyDecision } from '../policy/types.js'; import { PolicyDecision } from '../policy/types.js';
import { import {
ToolConfirmationOutcome, ToolConfirmationOutcome,
@@ -243,6 +244,7 @@ export class Scheduler {
this.isProcessing = true; this.isProcessing = true;
this.isCancelling = false; this.isCancelling = false;
this.state.clearBatch(); this.state.clearBatch();
const currentApprovalMode = this.config.getApprovalMode();
try { try {
const toolRegistry = this.config.getToolRegistry(); const toolRegistry = this.config.getToolRegistry();
@@ -260,10 +262,15 @@ export class Scheduler {
enrichedRequest, enrichedRequest,
toolRegistry.getAllToolNames(), toolRegistry.getAllToolNames(),
), ),
approvalMode: currentApprovalMode,
}; };
} }
return this._validateAndCreateToolCall(enrichedRequest, tool); return this._validateAndCreateToolCall(
enrichedRequest,
tool,
currentApprovalMode,
);
}); });
this.state.enqueue(newCalls); this.state.enqueue(newCalls);
@@ -297,6 +304,7 @@ export class Scheduler {
private _validateAndCreateToolCall( private _validateAndCreateToolCall(
request: ToolCallRequestInfo, request: ToolCallRequestInfo,
tool: AnyDeclarativeTool, tool: AnyDeclarativeTool,
approvalMode: ApprovalMode,
): ValidatingToolCall | ErroredToolCall { ): ValidatingToolCall | ErroredToolCall {
return runWithToolCallContext( return runWithToolCallContext(
{ {
@@ -314,6 +322,7 @@ export class Scheduler {
invocation, invocation,
startTime: Date.now(), startTime: Date.now(),
schedulerId: this.schedulerId, schedulerId: this.schedulerId,
approvalMode,
}; };
} catch (e) { } catch (e) {
return { return {
@@ -327,6 +336,7 @@ export class Scheduler {
), ),
durationMs: 0, durationMs: 0,
schedulerId: this.schedulerId, schedulerId: this.schedulerId,
approvalMode,
}; };
} }
}, },
@@ -24,6 +24,7 @@ import {
} from '../tools/tools.js'; } from '../tools/tools.js';
import { MessageBusType } from '../confirmation-bus/types.js'; import { MessageBusType } from '../confirmation-bus/types.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ApprovalMode } from '../policy/types.js';
describe('SchedulerStateManager', () => { describe('SchedulerStateManager', () => {
const mockRequest: ToolCallRequestInfo = { const mockRequest: ToolCallRequestInfo = {
@@ -43,12 +44,16 @@ describe('SchedulerStateManager', () => {
shouldConfirmExecute: vi.fn(), shouldConfirmExecute: vi.fn(),
} as unknown as AnyToolInvocation; } as unknown as AnyToolInvocation;
const createValidatingCall = (id = 'call-1'): ValidatingToolCall => ({ const createValidatingCall = (
id = 'call-1',
mode: ApprovalMode = ApprovalMode.DEFAULT,
): ValidatingToolCall => ({
status: CoreToolCallStatus.Validating, status: CoreToolCallStatus.Validating,
request: { ...mockRequest, callId: id }, request: { ...mockRequest, callId: id },
tool: mockTool, tool: mockTool,
invocation: mockInvocation, invocation: mockInvocation,
startTime: Date.now(), startTime: Date.now(),
approvalMode: mode,
}); });
const createMockResponse = (id: string): ToolCallResponseInfo => ({ const createMockResponse = (id: string): ToolCallResponseInfo => ({
@@ -208,7 +213,7 @@ describe('SchedulerStateManager', () => {
describe('Status Transitions', () => { describe('Status Transitions', () => {
it('should transition validating to scheduled', () => { it('should transition validating to scheduled', () => {
const call = createValidatingCall(); const call = createValidatingCall('call-1', ApprovalMode.PLAN);
stateManager.enqueue([call]); stateManager.enqueue([call]);
stateManager.dequeue(); stateManager.dequeue();
@@ -220,6 +225,7 @@ describe('SchedulerStateManager', () => {
const snapshot = stateManager.getSnapshot(); const snapshot = stateManager.getSnapshot();
expect(snapshot[0].status).toBe(CoreToolCallStatus.Scheduled); expect(snapshot[0].status).toBe(CoreToolCallStatus.Scheduled);
expect(snapshot[0].request.callId).toBe(call.request.callId); expect(snapshot[0].request.callId).toBe(call.request.callId);
expect(snapshot[0].approvalMode).toBe(ApprovalMode.PLAN);
}); });
it('should transition scheduled to executing', () => { it('should transition scheduled to executing', () => {
@@ -242,7 +248,7 @@ describe('SchedulerStateManager', () => {
}); });
it('should transition to success and move to completed batch', () => { it('should transition to success and move to completed batch', () => {
const call = createValidatingCall(); const call = createValidatingCall('call-1', ApprovalMode.PLAN);
stateManager.enqueue([call]); stateManager.enqueue([call]);
stateManager.dequeue(); stateManager.dequeue();
@@ -272,6 +278,7 @@ describe('SchedulerStateManager', () => {
expect(completed.status).toBe(CoreToolCallStatus.Success); expect(completed.status).toBe(CoreToolCallStatus.Success);
expect(completed.response).toEqual(response); expect(completed.response).toEqual(response);
expect(completed.durationMs).toBeDefined(); expect(completed.durationMs).toBeDefined();
expect(completed.approvalMode).toBe(ApprovalMode.PLAN);
}); });
it('should transition to error and move to completed batch', () => { it('should transition to error and move to completed batch', () => {
@@ -350,6 +350,7 @@ export class SchedulerStateManager {
durationMs: startTime ? Date.now() - startTime : undefined, durationMs: startTime ? Date.now() - startTime : undefined,
outcome: call.outcome, outcome: call.outcome,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -366,6 +367,7 @@ export class SchedulerStateManager {
durationMs: startTime ? Date.now() - startTime : undefined, durationMs: startTime ? Date.now() - startTime : undefined,
outcome: call.outcome, outcome: call.outcome,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -399,6 +401,7 @@ export class SchedulerStateManager {
outcome: call.outcome, outcome: call.outcome,
invocation: call.invocation, invocation: call.invocation,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -424,6 +427,7 @@ export class SchedulerStateManager {
outcome: call.outcome, outcome: call.outcome,
invocation: call.invocation, invocation: call.invocation,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -479,6 +483,7 @@ export class SchedulerStateManager {
durationMs: startTime ? Date.now() - startTime : undefined, durationMs: startTime ? Date.now() - startTime : undefined,
outcome: call.outcome, outcome: call.outcome,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -500,6 +505,7 @@ export class SchedulerStateManager {
outcome: call.outcome, outcome: call.outcome,
invocation: call.invocation, invocation: call.invocation,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
@@ -522,6 +528,7 @@ export class SchedulerStateManager {
liveOutput, liveOutput,
pid, pid,
schedulerId: call.schedulerId, schedulerId: call.schedulerId,
approvalMode: call.approvalMode,
}; };
} }
} }
+8
View File
@@ -15,6 +15,7 @@ import type {
import type { AnsiOutput } from '../utils/terminalSerializer.js'; import type { AnsiOutput } from '../utils/terminalSerializer.js';
import type { ToolErrorType } from '../tools/tool-error.js'; import type { ToolErrorType } from '../tools/tool-error.js';
import type { SerializableConfirmationDetails } from '../confirmation-bus/types.js'; import type { SerializableConfirmationDetails } from '../confirmation-bus/types.js';
import { type ApprovalMode } from '../policy/types.js';
export const ROOT_SCHEDULER_ID = 'root'; export const ROOT_SCHEDULER_ID = 'root';
@@ -65,6 +66,7 @@ export type ValidatingToolCall = {
startTime?: number; startTime?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type ScheduledToolCall = { export type ScheduledToolCall = {
@@ -75,6 +77,7 @@ export type ScheduledToolCall = {
startTime?: number; startTime?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type ErroredToolCall = { export type ErroredToolCall = {
@@ -85,6 +88,7 @@ export type ErroredToolCall = {
durationMs?: number; durationMs?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type SuccessfulToolCall = { export type SuccessfulToolCall = {
@@ -96,6 +100,7 @@ export type SuccessfulToolCall = {
durationMs?: number; durationMs?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type ExecutingToolCall = { export type ExecutingToolCall = {
@@ -108,6 +113,7 @@ export type ExecutingToolCall = {
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
pid?: number; pid?: number;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type CancelledToolCall = { export type CancelledToolCall = {
@@ -119,6 +125,7 @@ export type CancelledToolCall = {
durationMs?: number; durationMs?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type WaitingToolCall = { export type WaitingToolCall = {
@@ -141,6 +148,7 @@ export type WaitingToolCall = {
startTime?: number; startTime?: number;
outcome?: ToolConfirmationOutcome; outcome?: ToolConfirmationOutcome;
schedulerId?: string; schedulerId?: string;
approvalMode?: ApprovalMode;
}; };
export type Status = ToolCall['status']; export type Status = ToolCall['status'];
+1 -53
View File
@@ -5,11 +5,7 @@
*/ */
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest';
import { import { AskUserTool, isCompletedAskUserTool } from './ask-user.js';
AskUserTool,
shouldHideAskUserTool,
isCompletedAskUserTool,
} from './ask-user.js';
import { QuestionType, type Question } from '../confirmation-bus/types.js'; import { QuestionType, type Question } from '../confirmation-bus/types.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ToolConfirmationOutcome } from './tools.js'; import { ToolConfirmationOutcome } from './tools.js';
@@ -17,54 +13,6 @@ import { ToolErrorType } from './tool-error.js';
import { ASK_USER_DISPLAY_NAME } from './tool-names.js'; import { ASK_USER_DISPLAY_NAME } from './tool-names.js';
describe('AskUserTool Helpers', () => { describe('AskUserTool Helpers', () => {
describe('shouldHideAskUserTool', () => {
it('returns false for non-AskUser tools', () => {
expect(shouldHideAskUserTool('other-tool', 'Success', true)).toBe(false);
});
it('hides Pending AskUser tool', () => {
expect(
shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Pending', false),
).toBe(true);
});
it('hides Executing AskUser tool', () => {
expect(
shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Executing', false),
).toBe(true);
});
it('hides Confirming AskUser tool', () => {
expect(
shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Confirming', false),
).toBe(true);
});
it('shows Success AskUser tool', () => {
expect(
shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Success', true),
).toBe(false);
});
it('shows Canceled AskUser tool', () => {
expect(
shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Canceled', true),
).toBe(false);
});
it('hides Error AskUser tool without result display', () => {
expect(shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Error', false)).toBe(
true,
);
});
it('shows Error AskUser tool with result display', () => {
expect(shouldHideAskUserTool(ASK_USER_DISPLAY_NAME, 'Error', true)).toBe(
false,
);
});
});
describe('isCompletedAskUserTool', () => { describe('isCompletedAskUserTool', () => {
it('returns false for non-AskUser tools', () => { it('returns false for non-AskUser tools', () => {
expect(isCompletedAskUserTool('other-tool', 'Success')).toBe(false); expect(isCompletedAskUserTool('other-tool', 'Success')).toBe(false);
-32
View File
@@ -259,38 +259,6 @@ export class AskUserInvocation extends BaseToolInvocation<
} }
} }
/**
* Determines if an 'Ask User' tool call should be hidden from the standard tool history UI.
*
* We hide Ask User tools in two cases:
* 1. They are in progress because they are displayed using a specialized UI (AskUserDialog).
* 2. They have errored without a result display (e.g. validation errors), in which case
* the agent self-corrects and we don't want to clutter the UI.
*
* NOTE: The 'status' parameter values are intended to match the CLI's ToolCallStatus enum.
*/
export function shouldHideAskUserTool(
name: string,
status: string,
hasResultDisplay: boolean,
): boolean {
if (name !== ASK_USER_DISPLAY_NAME) {
return false;
}
// Case 1: In-progress tools (Pending, Executing, Confirming)
if (['Pending', 'Executing', 'Confirming'].includes(status)) {
return true;
}
// Case 2: Error without result display
if (status === 'Error' && !hasResultDisplay) {
return true;
}
return false;
}
/** /**
* Returns true if the tool name and status correspond to a completed 'Ask User' tool call. * Returns true if the tool name and status correspond to a completed 'Ask User' tool call.
*/ */
+6 -2
View File
@@ -43,7 +43,11 @@ import { EditCorrectionEvent } from '../telemetry/types.js';
import { logEditCorrectionEvent } from '../telemetry/loggers.js'; import { logEditCorrectionEvent } from '../telemetry/loggers.js';
import { correctPath } from '../utils/pathCorrector.js'; import { correctPath } from '../utils/pathCorrector.js';
import { EDIT_TOOL_NAME, READ_FILE_TOOL_NAME } from './tool-names.js'; import {
EDIT_TOOL_NAME,
READ_FILE_TOOL_NAME,
EDIT_DISPLAY_NAME,
} from './tool-names.js';
import { debugLogger } from '../utils/debugLogger.js'; import { debugLogger } from '../utils/debugLogger.js';
import { EDIT_DEFINITION } from './definitions/coreTools.js'; import { EDIT_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js'; import { resolveToolDeclaration } from './definitions/resolver.js';
@@ -908,7 +912,7 @@ export class EditTool
) { ) {
super( super(
EditTool.Name, EditTool.Name,
'Edit', EDIT_DISPLAY_NAME,
EDIT_DEFINITION.base.description!, EDIT_DEFINITION.base.description!,
Kind.Edit, Kind.Edit,
EDIT_DEFINITION.base.parametersJsonSchema, EDIT_DEFINITION.base.parametersJsonSchema,
+2 -2
View File
@@ -14,7 +14,7 @@ import { shortenPath, makeRelative } from '../utils/paths.js';
import { type Config } from '../config/config.js'; import { type Config } from '../config/config.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js'; import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
import { ToolErrorType } from './tool-error.js'; import { ToolErrorType } from './tool-error.js';
import { GLOB_TOOL_NAME } from './tool-names.js'; import { GLOB_TOOL_NAME, GLOB_DISPLAY_NAME } from './tool-names.js';
import { getErrorMessage } from '../utils/errors.js'; import { getErrorMessage } from '../utils/errors.js';
import { debugLogger } from '../utils/debugLogger.js'; import { debugLogger } from '../utils/debugLogger.js';
import { GLOB_DEFINITION } from './definitions/coreTools.js'; import { GLOB_DEFINITION } from './definitions/coreTools.js';
@@ -271,7 +271,7 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
) { ) {
super( super(
GlobTool.Name, GlobTool.Name,
'FindFiles', GLOB_DISPLAY_NAME,
GLOB_DEFINITION.base.description!, GLOB_DEFINITION.base.description!,
Kind.Search, Kind.Search,
GLOB_DEFINITION.base.parametersJsonSchema, GLOB_DEFINITION.base.parametersJsonSchema,
+2 -2
View File
@@ -21,7 +21,7 @@ import { FileOperation } from '../telemetry/metrics.js';
import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js'; import { getProgrammingLanguage } from '../telemetry/telemetry-utils.js';
import { logFileOperation } from '../telemetry/loggers.js'; import { logFileOperation } from '../telemetry/loggers.js';
import { FileOperationEvent } from '../telemetry/types.js'; import { FileOperationEvent } from '../telemetry/types.js';
import { READ_FILE_TOOL_NAME } from './tool-names.js'; import { READ_FILE_TOOL_NAME, READ_FILE_DISPLAY_NAME } from './tool-names.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js'; import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { READ_FILE_DEFINITION } from './definitions/coreTools.js'; import { READ_FILE_DEFINITION } from './definitions/coreTools.js';
import { resolveToolDeclaration } from './definitions/resolver.js'; import { resolveToolDeclaration } from './definitions/resolver.js';
@@ -173,7 +173,7 @@ export class ReadFileTool extends BaseDeclarativeTool<
) { ) {
super( super(
ReadFileTool.Name, ReadFileTool.Name,
'ReadFile', READ_FILE_DISPLAY_NAME,
READ_FILE_DEFINITION.base.description!, READ_FILE_DEFINITION.base.description!,
Kind.Read, Kind.Read,
READ_FILE_DEFINITION.base.parametersJsonSchema, READ_FILE_DEFINITION.base.parametersJsonSchema,
+7 -1
View File
@@ -40,10 +40,16 @@ export const GET_INTERNAL_DOCS_TOOL_NAME = 'get_internal_docs';
export const ACTIVATE_SKILL_TOOL_NAME = 'activate_skill'; export const ACTIVATE_SKILL_TOOL_NAME = 'activate_skill';
export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]); export const EDIT_TOOL_NAMES = new Set([EDIT_TOOL_NAME, WRITE_FILE_TOOL_NAME]);
export const ASK_USER_TOOL_NAME = 'ask_user'; export const ASK_USER_TOOL_NAME = 'ask_user';
export const ASK_USER_DISPLAY_NAME = 'Ask User';
export const EXIT_PLAN_MODE_TOOL_NAME = 'exit_plan_mode'; export const EXIT_PLAN_MODE_TOOL_NAME = 'exit_plan_mode';
export const ENTER_PLAN_MODE_TOOL_NAME = 'enter_plan_mode'; export const ENTER_PLAN_MODE_TOOL_NAME = 'enter_plan_mode';
// Tool Display Names
export const WRITE_FILE_DISPLAY_NAME = 'WriteFile';
export const EDIT_DISPLAY_NAME = 'Edit';
export const ASK_USER_DISPLAY_NAME = 'Ask User';
export const READ_FILE_DISPLAY_NAME = 'ReadFile';
export const GLOB_DISPLAY_NAME = 'FindFiles';
/** /**
* Mapping of legacy tool names to their current names. * Mapping of legacy tool names to their current names.
* This ensures backward compatibility for user-defined policies, skills, and hooks. * This ensures backward compatibility for user-defined policies, skills, and hooks.
+2 -2
View File
@@ -9,7 +9,7 @@ import fsPromises from 'node:fs/promises';
import path from 'node:path'; import path from 'node:path';
import os from 'node:os'; import os from 'node:os';
import * as Diff from 'diff'; import * as Diff from 'diff';
import { WRITE_FILE_TOOL_NAME } from './tool-names.js'; import { WRITE_FILE_TOOL_NAME, WRITE_FILE_DISPLAY_NAME } from './tool-names.js';
import type { Config } from '../config/config.js'; import type { Config } from '../config/config.js';
import { ApprovalMode } from '../policy/types.js'; import { ApprovalMode } from '../policy/types.js';
@@ -446,7 +446,7 @@ export class WriteFileTool
) { ) {
super( super(
WriteFileTool.Name, WriteFileTool.Name,
'WriteFile', WRITE_FILE_DISPLAY_NAME,
WRITE_FILE_DEFINITION.base.description!, WRITE_FILE_DEFINITION.base.description!,
Kind.Edit, Kind.Edit,
WRITE_FILE_DEFINITION.base.parametersJsonSchema, WRITE_FILE_DEFINITION.base.parametersJsonSchema,
+92 -2
View File
@@ -5,11 +5,101 @@
*/ */
import { expect, describe, it } from 'vitest'; import { expect, describe, it } from 'vitest';
import { doesToolInvocationMatch, getToolSuggestion } from './tool-utils.js'; import {
doesToolInvocationMatch,
getToolSuggestion,
shouldHideToolCall,
} from './tool-utils.js';
import type { AnyToolInvocation, Config } from '../index.js'; import type { AnyToolInvocation, Config } from '../index.js';
import { ReadFileTool } from '../index.js'; import {
ReadFileTool,
ApprovalMode,
CoreToolCallStatus,
ASK_USER_DISPLAY_NAME,
WRITE_FILE_DISPLAY_NAME,
EDIT_DISPLAY_NAME,
READ_FILE_DISPLAY_NAME,
} from '../index.js';
import { createMockMessageBus } from '../test-utils/mock-message-bus.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);
},
);
});
describe('getToolSuggestion', () => { describe('getToolSuggestion', () => {
it('should suggest the top N closest tool names for a typo', () => { it('should suggest the top N closest tool names for a typo', () => {
const allToolNames = ['list_files', 'read_file', 'write_file']; const allToolNames = ['list_files', 'read_file', 'write_file'];
+55
View File
@@ -8,6 +8,61 @@ import type { AnyDeclarativeTool, AnyToolInvocation } from '../index.js';
import { isTool } from '../index.js'; import { isTool } from '../index.js';
import { SHELL_TOOL_NAMES } from './shell-utils.js'; import { SHELL_TOOL_NAMES } from './shell-utils.js';
import levenshtein from 'fast-levenshtein'; import levenshtein from 'fast-levenshtein';
import { ApprovalMode } from '../policy/types.js';
import { CoreToolCallStatus } from '../scheduler/types.js';
import {
ASK_USER_DISPLAY_NAME,
WRITE_FILE_DISPLAY_NAME,
EDIT_DISPLAY_NAME,
} from '../tools/tool-names.js';
/**
* 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;
}
/**
* Determines if a tool call should be hidden from the standard tool history UI.
*
* We hide tools in several cases:
* 1. Ask User tools that are in progress, displayed via specialized UI.
* 2. Ask User tools that errored without result display, typically param
* validation errors that the agent automatically recovers from.
* 3. 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 } = params;
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. * Generates a suggestion string for a tool name that was not found in the registry.