diff --git a/packages/cli/src/acp/acpClient.test.ts b/packages/cli/src/acp/acpClient.test.ts index e10f0e3e3d..9e4b89ea20 100644 --- a/packages/cli/src/acp/acpClient.test.ts +++ b/packages/cli/src/acp/acpClient.test.ts @@ -99,6 +99,8 @@ vi.mock( const actual = await importOriginal(); return { ...actual, + updatePolicy: vi.fn(), + createPolicyUpdater: vi.fn(), ReadManyFilesTool: vi.fn().mockImplementation(() => ({ name: 'read_many_files', kind: 'read', @@ -181,6 +183,20 @@ describe('GeminiAgent', () => { getWorkspaceContext: vi.fn().mockReturnValue({ addReadOnlyPath: vi.fn(), }), + getPolicyEngine: vi.fn().mockReturnValue({ + addRule: vi.fn(), + }), + messageBus: { + publish: vi.fn(), + subscribe: vi.fn(), + unsubscribe: vi.fn(), + }, + storage: { + getWorkspaceAutoSavedPolicyPath: vi.fn(), + getAutoSavedPolicyPath: vi.fn(), + setClientName: vi.fn(), + }, + setClientName: vi.fn(), get config() { return this; }, @@ -201,7 +217,10 @@ describe('GeminiAgent', () => { (loadCliConfig as unknown as Mock).mockResolvedValue(mockConfig); (loadSettings as unknown as Mock).mockImplementation(() => ({ merged: { - security: { auth: { selectedType: AuthType.LOGIN_WITH_GOOGLE } }, + security: { + auth: { selectedType: AuthType.LOGIN_WITH_GOOGLE }, + enablePermanentToolApproval: true, + }, mcpServers: {}, }, setValue: vi.fn(), @@ -687,7 +706,10 @@ describe('Session', () => { systemDefaults: { settings: {} }, user: { settings: {} }, workspace: { settings: {} }, - merged: { settings: {} }, + merged: { + security: { enablePermanentToolApproval: true }, + mcpServers: {}, + }, errors: [], } as unknown as LoadedSettings); }); @@ -1026,6 +1048,166 @@ describe('Session', () => { ); }); + it('should exclude always allow and save permanent option when enablePermanentToolApproval is false', async () => { + mockConfig.getDisableAlwaysAllow = vi.fn().mockReturnValue(false); + const confirmationDetails = { + type: 'edit', + onConfirm: vi.fn(), + }; + mockTool.build.mockReturnValue({ + getDescription: () => 'Test Tool', + toolLocations: () => [], + shouldConfirmExecute: vi.fn().mockResolvedValue(confirmationDetails), + execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }), + }); + + const customSettings = { + system: { settings: {} }, + systemDefaults: { settings: {} }, + user: { settings: {} }, + workspace: { settings: {} }, + merged: { + security: { enablePermanentToolApproval: false }, + mcpServers: {}, + }, + errors: [], + } as unknown as LoadedSettings; + + const localSession = new Session( + 'session-2', + mockChat, + mockConfig, + mockConnection, + customSettings, + ); + + mockConnection.requestPermission.mockResolvedValueOnce({ + outcome: { + outcome: 'selected', + optionId: ToolConfirmationOutcome.ProceedOnce, + }, + }); + + const stream1 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { + functionCalls: [{ name: 'test_tool', args: {} }], + }, + }, + ]); + const stream2 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { candidates: [] }, + }, + ]); + + mockChat.sendMessageStream + .mockResolvedValueOnce(stream1) + .mockResolvedValueOnce(stream2); + + await localSession.prompt({ + sessionId: 'session-2', + prompt: [{ type: 'text', text: 'Call tool' }], + }); + + expect(mockConnection.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.not.arrayContaining([ + expect.objectContaining({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + }), + ]), + }), + ); + expect(mockConnection.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + optionId: ToolConfirmationOutcome.ProceedAlways, + }), + ]), + }), + ); + }); + + it('should include always allow and save permanent option when enablePermanentToolApproval is true', async () => { + mockConfig.getDisableAlwaysAllow = vi.fn().mockReturnValue(false); + const confirmationDetails = { + type: 'edit', + onConfirm: vi.fn(), + }; + mockTool.build.mockReturnValue({ + getDescription: () => 'Test Tool', + toolLocations: () => [], + shouldConfirmExecute: vi.fn().mockResolvedValue(confirmationDetails), + execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }), + }); + + const customSettings = { + system: { settings: {} }, + systemDefaults: { settings: {} }, + user: { settings: {} }, + workspace: { settings: {} }, + merged: { + security: { enablePermanentToolApproval: true }, + mcpServers: {}, + }, + errors: [], + } as unknown as LoadedSettings; + + const localSession = new Session( + 'session-2', + mockChat, + mockConfig, + mockConnection, + customSettings, + ); + + mockConnection.requestPermission.mockResolvedValueOnce({ + outcome: { + outcome: 'selected', + optionId: ToolConfirmationOutcome.ProceedOnce, + }, + }); + + const stream1 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { + functionCalls: [{ name: 'test_tool', args: {} }], + }, + }, + ]); + const stream2 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { candidates: [] }, + }, + ]); + + mockChat.sendMessageStream + .mockResolvedValueOnce(stream1) + .mockResolvedValueOnce(stream2); + + await localSession.prompt({ + sessionId: 'session-2', + prompt: [{ type: 'text', text: 'Call tool' }], + }); + + expect(mockConnection.requestPermission).toHaveBeenCalledWith( + expect.objectContaining({ + options: expect.arrayContaining([ + expect.objectContaining({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + name: 'Allow for this file in all future sessions', + }), + ]), + }), + ); + }); + it('should use filePath for ACP diff content in permission request', async () => { const confirmationDetails = { type: 'edit', @@ -1154,6 +1336,56 @@ describe('Session', () => { ); }); + it('should call updatePolicy when tool permission triggers always allow', async () => { + const confirmationDetails = { + type: 'info', + onConfirm: vi.fn(), + }; + mockTool.build.mockReturnValue({ + getDescription: () => 'Test Tool', + toolLocations: () => [], + shouldConfirmExecute: vi.fn().mockResolvedValue(confirmationDetails), + execute: vi.fn().mockResolvedValue({ llmContent: 'Tool Result' }), + }); + + mockConnection.requestPermission.mockResolvedValue({ + outcome: { + outcome: 'selected', + optionId: ToolConfirmationOutcome.ProceedAlways, + }, + }); + + const stream1 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { + functionCalls: [{ name: 'test_tool', args: {} }], + }, + }, + ]); + const stream2 = createMockStream([ + { + type: StreamEventType.CHUNK, + value: { candidates: [] }, + }, + ]); + + mockChat.sendMessageStream + .mockResolvedValueOnce(stream1) + .mockResolvedValueOnce(stream2); + + const { updatePolicy } = await import('@google/gemini-cli-core'); + + await session.prompt({ + sessionId: 'session-1', + prompt: [{ type: 'text', text: 'Call tool' }], + }); + + expect(confirmationDetails.onConfirm).toHaveBeenCalled(); + + expect(updatePolicy).toHaveBeenCalled(); + }); + it('should use filePath for ACP diff content in tool result', async () => { mockTool.build.mockReturnValue({ getDescription: () => 'Test Tool', diff --git a/packages/cli/src/acp/acpClient.ts b/packages/cli/src/acp/acpClient.ts index 1a300413b0..59c6cb2b3f 100644 --- a/packages/cli/src/acp/acpClient.ts +++ b/packages/cli/src/acp/acpClient.ts @@ -49,6 +49,7 @@ import { getDisplayString, processSingleFileContent, type AgentLoopContext, + updatePolicy, } from '@google/gemini-cli-core'; import * as acp from '@agentclientprotocol/sdk'; import { AcpFileSystemService } from './fileSystemService.js'; @@ -64,6 +65,7 @@ import { loadSettings, type LoadedSettings, } from '../config/settings.js'; +import { createPolicyUpdater } from '../config/policy.js'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import { z } from 'zod'; @@ -133,6 +135,7 @@ export class GeminiAgent { args: acp.InitializeRequest, ): Promise { this.clientCapabilities = args.clientCapabilities; + const authMethods = [ { id: AuthType.LOGIN_WITH_GOOGLE, @@ -322,6 +325,7 @@ export class GeminiAgent { const geminiClient = config.getGeminiClient(); const chat = await geminiClient.startChat(); + const session = new Session( sessionId, chat, @@ -512,6 +516,12 @@ export class GeminiAgent { const config = await loadCliConfig(settings, sessionId, this.argv, { cwd }); + createPolicyUpdater( + config.getPolicyEngine(), + config.messageBus, + config.storage, + ); + return config; } @@ -1012,6 +1022,7 @@ export class Session { options: toPermissionOptions( confirmationDetails, this.context.config, + this.settings.merged.security.enablePermanentToolApproval, ), toolCall: { toolCallId: callId, @@ -1036,6 +1047,16 @@ export class Session { await confirmationDetails.onConfirm(outcome); + // Update policy to enable Always Allow persistence + await updatePolicy( + tool, + outcome, + confirmationDetails, + this.context, + this.context.messageBus, + invocation, + ); + switch (outcome) { case ToolConfirmationOutcome.Cancel: return errorResponse( @@ -1785,6 +1806,7 @@ const basicPermissionOptions = [ function toPermissionOptions( confirmation: ToolCallConfirmationDetails, config: Config, + enablePermanentToolApproval: boolean = false, ): acp.PermissionOption[] { const disableAlwaysAllow = config.getDisableAlwaysAllow(); const options: acp.PermissionOption[] = []; @@ -1794,37 +1816,65 @@ function toPermissionOptions( case 'edit': options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, - name: 'Allow All Edits', + name: 'Allow for this session', kind: 'allow_always', }); + if (enablePermanentToolApproval) { + options.push({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + name: 'Allow for this file in all future sessions', + kind: 'allow_always', + }); + } break; case 'exec': options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, - name: `Always Allow ${confirmation.rootCommand}`, + name: 'Allow for this session', kind: 'allow_always', }); + if (enablePermanentToolApproval) { + options.push({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + name: 'Allow this command for all future sessions', + kind: 'allow_always', + }); + } break; case 'mcp': options.push( { optionId: ToolConfirmationOutcome.ProceedAlwaysServer, - name: `Always Allow ${confirmation.serverName}`, + name: 'Allow all server tools for this session', kind: 'allow_always', }, { optionId: ToolConfirmationOutcome.ProceedAlwaysTool, - name: `Always Allow ${confirmation.toolName}`, + name: 'Allow tool for this session', kind: 'allow_always', }, ); + if (enablePermanentToolApproval) { + options.push({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + name: 'Allow tool for all future sessions', + kind: 'allow_always', + }); + } break; case 'info': options.push({ optionId: ToolConfirmationOutcome.ProceedAlways, - name: `Always Allow`, + name: 'Allow for this session', kind: 'allow_always', }); + if (enablePermanentToolApproval) { + options.push({ + optionId: ToolConfirmationOutcome.ProceedAlwaysAndSave, + name: 'Allow for all future sessions', + kind: 'allow_always', + }); + } break; case 'ask_user': case 'exit_plan_mode': diff --git a/packages/cli/src/acp/acpResume.test.ts b/packages/cli/src/acp/acpResume.test.ts index 77021004ca..3f75119d0b 100644 --- a/packages/cli/src/acp/acpResume.test.ts +++ b/packages/cli/src/acp/acpResume.test.ts @@ -91,6 +91,14 @@ describe('GeminiAgent Session Resume', () => { storage: { getProjectTempDir: vi.fn().mockReturnValue('/tmp/project'), }, + getPolicyEngine: vi.fn().mockReturnValue({ + addRule: vi.fn(), + }), + messageBus: { + publish: vi.fn(), + subscribe: vi.fn(), + unsubscribe: vi.fn(), + }, getApprovalMode: vi.fn().mockReturnValue('default'), isPlanEnabled: vi.fn().mockReturnValue(true), getModel: vi.fn().mockReturnValue('gemini-pro'), diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 09ea05871a..9b98a1bbe2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -46,6 +46,7 @@ export * from './core/geminiRequest.js'; export * from './scheduler/scheduler.js'; export * from './scheduler/types.js'; export * from './scheduler/tool-executor.js'; +export * from './scheduler/policy.js'; export * from './core/recordingContentGenerator.js'; export * from './fallback/types.js';