fix: similar to policy-engine, throw error in case of requiring tool execution confirmation for non-interactive mode (#14702)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
This commit is contained in:
Mayur Vaid
2025-12-16 00:04:27 +05:30
committed by GitHub
parent d030a1f62f
commit 217e2b0eb4
4 changed files with 89 additions and 16 deletions

View File

@@ -32,6 +32,7 @@ export function createMockConfig(
}),
getApprovalMode: vi.fn().mockReturnValue(ApprovalMode.DEFAULT),
getIdeMode: vi.fn().mockReturnValue(false),
isInteractive: () => true,
getAllowedTools: vi.fn().mockReturnValue([]),
getWorkspaceContext: vi.fn().mockReturnValue({
isPathWithinWorkspace: () => true,

View File

@@ -88,6 +88,10 @@ const mockConfig = {
mockConfig.getMessageBus = vi.fn().mockReturnValue(createMockMessageBus());
mockConfig.getHookSystem = vi.fn().mockReturnValue(new HookSystem(mockConfig));
function createMockConfigOverride(overrides: Partial<Config> = {}): Config {
return { ...mockConfig, ...overrides } as Config;
}
const mockTool = new MockTool({
name: 'mockTool',
displayName: 'Mock Tool',
@@ -262,13 +266,9 @@ describe('useReactToolScheduler', () => {
vi.useRealTimers();
});
const renderScheduler = () =>
const renderScheduler = (config: Config = mockConfig) =>
renderHook(() =>
useReactToolScheduler(
onComplete,
mockConfig as unknown as Config,
() => undefined,
),
useReactToolScheduler(onComplete, config, () => undefined),
);
it('initial state should be empty', () => {
@@ -494,13 +494,16 @@ describe('useReactToolScheduler', () => {
it('should handle tool requiring confirmation - approved', async () => {
mockToolRegistry.getTool.mockReturnValue(mockToolRequiresConfirmation);
const config = createMockConfigOverride({
isInteractive: () => true,
});
const expectedOutput = 'Confirmed output';
(mockToolRequiresConfirmation.execute as Mock).mockResolvedValue({
llmContent: expectedOutput,
returnDisplay: 'Confirmed display',
} as ToolResult);
const { result } = renderScheduler();
const { result } = renderScheduler(config);
const schedule = result.current[1];
const request: ToolCallRequestInfo = {
callId: 'callConfirm',
@@ -544,7 +547,10 @@ describe('useReactToolScheduler', () => {
it('should handle tool requiring confirmation - cancelled by user', async () => {
mockToolRegistry.getTool.mockReturnValue(mockToolRequiresConfirmation);
const { result } = renderScheduler();
const config = createMockConfigOverride({
isInteractive: () => true,
});
const { result } = renderScheduler(config);
const schedule = result.current[1];
const request: ToolCallRequestInfo = {
callId: 'callConfirmCancel',

View File

@@ -6,12 +6,16 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import type { Mock } from 'vitest';
import type { ToolCall, WaitingToolCall } from './coreToolScheduler.js';
import {
CoreToolScheduler,
convertToFunctionResponse,
truncateAndSaveToFile,
} from './coreToolScheduler.js';
import type {
ToolCall,
WaitingToolCall,
ErroredToolCall,
} from './coreToolScheduler.js';
import type {
ToolCallConfirmationDetails,
ToolConfirmationPayload,
@@ -232,6 +236,7 @@ function createMockConfig(overrides: Partial<Config> = {}): Config {
getSessionId: () => 'test-session-id',
getUsageStatisticsEnabled: () => true,
getDebugMode: () => false,
isInteractive: () => true,
getApprovalMode: () => ApprovalMode.DEFAULT,
setApprovalMode: () => {},
getAllowedTools: () => [],
@@ -353,7 +358,6 @@ describe('CoreToolScheduler', () => {
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
isInteractive: () => false,
getHookSystem: () => undefined,
});
@@ -455,7 +459,6 @@ describe('CoreToolScheduler', () => {
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
isInteractive: () => false,
getHookSystem: () => undefined,
});
@@ -582,6 +585,67 @@ describe('CoreToolScheduler', () => {
expect(statuses).not.toContain('error');
});
it('should error when tool requires confirmation in non-interactive mode', async () => {
const mockTool = new MockTool({
name: 'mockTool',
shouldConfirmExecute: MOCK_TOOL_SHOULD_CONFIRM_EXECUTE,
});
const declarativeTool = mockTool;
const mockToolRegistry = {
getTool: () => declarativeTool,
getFunctionDeclarations: () => [],
tools: new Map(),
discovery: {},
registerTool: () => {},
getToolByName: () => declarativeTool,
getToolByDisplayName: () => declarativeTool,
getTools: () => [],
discoverTools: async () => {},
getAllTools: () => [],
getToolsByServer: () => [],
} as unknown as ToolRegistry;
const onAllToolCallsComplete = vi.fn();
const onToolCallsUpdate = vi.fn();
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
isInteractive: () => false,
});
const scheduler = new CoreToolScheduler({
config: mockConfig,
onAllToolCallsComplete,
onToolCallsUpdate,
getPreferredEditor: () => 'vscode',
});
const abortController = new AbortController();
const request = {
callId: '1',
name: 'mockTool',
args: {},
isClientInitiated: false,
prompt_id: 'prompt-id-1',
};
await scheduler.schedule([request], abortController.signal);
expect(onAllToolCallsComplete).toHaveBeenCalled();
const completedCalls = onAllToolCallsComplete.mock
.calls[0][0] as ToolCall[];
expect(completedCalls[0].status).toBe('error');
const erroredCall = completedCalls[0] as ErroredToolCall;
const errorResponse = erroredCall.response;
const errorParts = errorResponse.responseParts;
// @ts-expect-error - accessing internal structure of FunctionResponsePart
const errorMessage = errorParts[0].functionResponse.response.error;
expect(errorMessage).toContain(
'Tool execution for "mockTool" requires user confirmation, which is not supported in non-interactive mode.',
);
});
describe('getToolSuggestion', () => {
it('should suggest the top N closest tool names for a typo', () => {
// Create mocked tool registry
@@ -643,7 +707,6 @@ describe('CoreToolScheduler with payload', () => {
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
isInteractive: () => false,
});
const mockMessageBus = createMockMessageBus();
mockConfig.getMessageBus = vi.fn().mockReturnValue(mockMessageBus);
@@ -950,7 +1013,6 @@ describe('CoreToolScheduler edit cancellation', () => {
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
isInteractive: () => false,
});
const mockMessageBus = createMockMessageBus();
mockConfig.getMessageBus = vi.fn().mockReturnValue(mockMessageBus);
@@ -1366,7 +1428,6 @@ describe('CoreToolScheduler request queueing', () => {
terminalHeight: 24,
}),
getToolRegistry: () => toolRegistry,
isInteractive: () => false,
getHookSystem: () => undefined,
});
@@ -1423,7 +1484,6 @@ describe('CoreToolScheduler request queueing', () => {
const mockConfig = createMockConfig({
getToolRegistry: () => mockToolRegistry,
getApprovalMode: () => ApprovalMode.YOLO,
isInteractive: () => false,
});
const mockMessageBus = createMockMessageBus();
mockConfig.getMessageBus = vi.fn().mockReturnValue(mockMessageBus);
@@ -1484,7 +1544,6 @@ describe('CoreToolScheduler request queueing', () => {
setApprovalMode: (mode: ApprovalMode) => {
approvalMode = mode;
},
isInteractive: () => false,
});
const mockMessageBus = createMockMessageBus();
mockConfig.getMessageBus = vi.fn().mockReturnValue(mockMessageBus);

View File

@@ -870,6 +870,13 @@ export class CoreToolScheduler {
);
this.setStatusInternal(reqInfo.callId, 'scheduled', signal);
} else {
if (!this.config.isInteractive()) {
throw new Error(
`Tool execution for "${
toolCall.tool.displayName || toolCall.tool.name
}" requires user confirmation, which is not supported in non-interactive mode.`,
);
}
// Fire Notification hook before showing confirmation to user
const messageBus = this.config.getMessageBus();
const hooksEnabled = this.config.getEnableHooks();