merge: pull main into channels and resolve conflicts

This commit is contained in:
Jack Wotherspoon
2026-03-27 13:54:41 -04:00
50 changed files with 2050 additions and 1218 deletions
+234 -2
View File
@@ -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',
+55 -5
View File
@@ -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<acp.InitializeResponse> {
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':
+8
View File
@@ -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'),
+1 -1
View File
@@ -728,7 +728,7 @@ export const AppContainer = (props: AppContainerProps) => {
// Wrap handleDeleteSession to return a Promise for UIActions interface
const handleDeleteSession = useCallback(
async (session: SessionInfo): Promise<void> => {
handleDeleteSessionSync(session);
await handleDeleteSessionSync(session);
},
[handleDeleteSessionSync],
);
@@ -98,7 +98,7 @@ export const useSessionBrowser = (
* Deletes a session by ID using the ChatRecordingService.
*/
handleDeleteSession: useCallback(
(session: SessionInfo) => {
async (session: SessionInfo) => {
// Note: Chat sessions are stored on disk using a filename derived from
// the session, e.g. "session-<timestamp>-<sessionIdPrefix>.json".
// The ChatRecordingService.deleteSession API expects this file basename
@@ -108,7 +108,7 @@ export const useSessionBrowser = (
.getGeminiClient()
?.getChatRecordingService();
if (chatRecordingService) {
chatRecordingService.deleteSession(session.file);
await chatRecordingService.deleteSession(session.file);
}
} catch (error) {
coreEvents.emitFeedback('error', 'Error deleting session:', error);
+4 -3
View File
@@ -19,6 +19,7 @@ import {
debugLogger,
coreEvents,
getErrorMessage,
getErrorType,
} from '@google/gemini-cli-core';
import { runSyncCleanup } from './cleanup.js';
@@ -82,7 +83,7 @@ export function handleError(
timestamp: new Date().toISOString(),
status: 'error',
error: {
type: error instanceof Error ? error.constructor.name : 'Error',
type: getErrorType(error),
message: errorMessage,
},
stats: streamFormatter.convertToStreamStats(metrics, 0),
@@ -177,7 +178,7 @@ export function handleCancellationError(config: Config): never {
timestamp: new Date().toISOString(),
status: 'error',
error: {
type: 'FatalCancellationError',
type: getErrorType(cancellationError),
message: cancellationError.message,
},
stats: streamFormatter.convertToStreamStats(metrics, 0),
@@ -218,7 +219,7 @@ export function handleMaxTurnsExceededError(config: Config): never {
timestamp: new Date().toISOString(),
status: 'error',
error: {
type: 'FatalTurnLimitedError',
type: getErrorType(maxTurnsError),
message: maxTurnsError.message,
},
stats: streamFormatter.convertToStreamStats(metrics, 0),
@@ -106,6 +106,8 @@ describe('Session Cleanup (Refactored)', () => {
);
// Session directory
await fs.mkdir(path.join(testTempDir, sessionId), { recursive: true });
// Subagent chats directory
await fs.mkdir(path.join(chatsDir, sessionId), { recursive: true });
}
async function seedSessions() {
@@ -274,6 +276,7 @@ describe('Session Cleanup (Refactored)', () => {
existsSync(path.join(toolOutputsDir, `session-${sessions[1].id}`)),
).toBe(false);
expect(existsSync(path.join(testTempDir, sessions[1].id))).toBe(false); // Session directory should be deleted
expect(existsSync(path.join(chatsDir, sessions[1].id))).toBe(false); // Subagent chats directory should be deleted
});
it('should NOT delete sessions within the cutoff date', async () => {
+8 -36
View File
@@ -13,6 +13,8 @@ import {
Storage,
TOOL_OUTPUTS_DIR,
type Config,
deleteSessionArtifactsAsync,
deleteSubagentSessionDirAndArtifactsAsync,
} from '@google/gemini-cli-core';
import type { Settings, SessionRetentionSettings } from '../config/settings.js';
import { getAllSessionFiles, type SessionFileEntry } from './sessionUtils.js';
@@ -59,48 +61,18 @@ function deriveShortIdFromFileName(fileName: string): string | null {
return null;
}
/**
* Gets the log path for a session ID.
*/
function getSessionLogPath(tempDir: string, safeSessionId: string): string {
return path.join(tempDir, 'logs', `session-${safeSessionId}.jsonl`);
}
/**
* Cleans up associated artifacts (logs, tool-outputs, directory) for a session.
*/
async function deleteSessionArtifactsAsync(
async function cleanupSessionAndSubagentsAsync(
sessionId: string,
config: Config,
): Promise<void> {
const tempDir = config.storage.getProjectTempDir();
const chatsDir = path.join(tempDir, 'chats');
// Cleanup logs
const logsDir = path.join(tempDir, 'logs');
const safeSessionId = sanitizeFilenamePart(sessionId);
const logPath = getSessionLogPath(tempDir, safeSessionId);
if (logPath.startsWith(logsDir)) {
await fs.unlink(logPath).catch(() => {});
}
// Cleanup tool outputs
const toolOutputDir = path.join(
tempDir,
TOOL_OUTPUTS_DIR,
`session-${safeSessionId}`,
);
const toolOutputsBase = path.join(tempDir, TOOL_OUTPUTS_DIR);
if (toolOutputDir.startsWith(toolOutputsBase)) {
await fs
.rm(toolOutputDir, { recursive: true, force: true })
.catch(() => {});
}
// Cleanup session directory
const sessionDir = path.join(tempDir, safeSessionId);
if (safeSessionId && sessionDir.startsWith(tempDir + path.sep)) {
await fs.rm(sessionDir, { recursive: true, force: true }).catch(() => {});
}
await deleteSessionArtifactsAsync(sessionId, tempDir);
await deleteSubagentSessionDirAndArtifactsAsync(sessionId, chatsDir, tempDir);
}
/**
@@ -201,7 +173,7 @@ export async function cleanupExpiredSessions(
await fs.unlink(filePath);
if (fullSessionId) {
await deleteSessionArtifactsAsync(fullSessionId, config);
await cleanupSessionAndSubagentsAsync(fullSessionId, config);
}
result.deleted++;
} else {
@@ -230,7 +202,7 @@ export async function cleanupExpiredSessions(
const sessionId = sessionToDelete.sessionInfo?.id;
if (sessionId) {
await deleteSessionArtifactsAsync(sessionId, config);
await cleanupSessionAndSubagentsAsync(sessionId, config);
}
if (config.getDebugMode()) {
+1 -1
View File
@@ -97,7 +97,7 @@ export async function deleteSession(
try {
// Use ChatRecordingService to delete the session
const chatRecordingService = new ChatRecordingService(config);
chatRecordingService.deleteSession(sessionToDelete.file);
await chatRecordingService.deleteSession(sessionToDelete.file);
const time = formatRelativeTime(sessionToDelete.lastUpdated);
writeToStdout(