diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 1d83ccbc..701ae267 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -8,6 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'os'; import * as fs from 'fs'; import * as path from 'path'; +import { ShellTool, EditTool, WriteFileTool } from '@google/gemini-cli-core'; import { loadCliConfig, parseArguments } from './config.js'; import { Settings } from './settings.js'; import { Extension } from './extension.js'; @@ -561,6 +562,17 @@ describe('mergeMcpServers', () => { }); describe('mergeExcludeTools', () => { + const defaultExcludes = [ShellTool.Name, EditTool.Name, WriteFileTool.Name]; + const originalIsTTY = process.stdin.isTTY; + + beforeEach(() => { + process.stdin.isTTY = true; + }); + + afterEach(() => { + process.stdin.isTTY = originalIsTTY; + }); + it('should merge excludeTools from settings and extensions', async () => { const settings: Settings = { excludeTools: ['tool1', 'tool2'] }; const extensions: Extension[] = [ @@ -655,7 +667,8 @@ describe('mergeExcludeTools', () => { expect(config.getExcludeTools()).toHaveLength(4); }); - it('should return an empty array when no excludeTools are specified', async () => { + it('should return an empty array when no excludeTools are specified and it is interactive', async () => { + process.stdin.isTTY = true; const settings: Settings = {}; const extensions: Extension[] = []; process.argv = ['node', 'script.js']; @@ -669,6 +682,21 @@ describe('mergeExcludeTools', () => { expect(config.getExcludeTools()).toEqual([]); }); + it('should return default excludes when no excludeTools are specified and it is not interactive', async () => { + process.stdin.isTTY = false; + const settings: Settings = {}; + const extensions: Extension[] = []; + process.argv = ['node', 'script.js', '-p', 'test']; + const argv = await parseArguments(); + const config = await loadCliConfig( + settings, + extensions, + 'test-session', + argv, + ); + expect(config.getExcludeTools()).toEqual(defaultExcludes); + }); + it('should handle settings with excludeTools but no extensions', async () => { process.argv = ['node', 'script.js']; const argv = await parseArguments(); @@ -1214,3 +1242,115 @@ describe('loadCliConfig chatCompression', () => { expect(config.getChatCompression()).toBeUndefined(); }); }); + +describe('loadCliConfig tool exclusions', () => { + const originalArgv = process.argv; + const originalEnv = { ...process.env }; + const originalIsTTY = process.stdin.isTTY; + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); + process.env.GEMINI_API_KEY = 'test-api-key'; + process.stdin.isTTY = true; + }); + + afterEach(() => { + process.argv = originalArgv; + process.env = originalEnv; + process.stdin.isTTY = originalIsTTY; + vi.restoreAllMocks(); + }); + + it('should not exclude interactive tools in interactive mode without YOLO', async () => { + process.stdin.isTTY = true; + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getExcludeTools()).not.toContain('run_shell_command'); + expect(config.getExcludeTools()).not.toContain('replace'); + expect(config.getExcludeTools()).not.toContain('write_file'); + }); + + it('should not exclude interactive tools in interactive mode with YOLO', async () => { + process.stdin.isTTY = true; + process.argv = ['node', 'script.js', '--yolo']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getExcludeTools()).not.toContain('run_shell_command'); + expect(config.getExcludeTools()).not.toContain('replace'); + expect(config.getExcludeTools()).not.toContain('write_file'); + }); + + it('should exclude interactive tools in non-interactive mode without YOLO', async () => { + process.stdin.isTTY = false; + process.argv = ['node', 'script.js', '-p', 'test']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getExcludeTools()).toContain('run_shell_command'); + expect(config.getExcludeTools()).toContain('replace'); + expect(config.getExcludeTools()).toContain('write_file'); + }); + + it('should not exclude interactive tools in non-interactive mode with YOLO', async () => { + process.stdin.isTTY = false; + process.argv = ['node', 'script.js', '-p', 'test', '--yolo']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.getExcludeTools()).not.toContain('run_shell_command'); + expect(config.getExcludeTools()).not.toContain('replace'); + expect(config.getExcludeTools()).not.toContain('write_file'); + }); +}); + +describe('loadCliConfig interactive', () => { + const originalArgv = process.argv; + const originalEnv = { ...process.env }; + const originalIsTTY = process.stdin.isTTY; + + beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(os.homedir).mockReturnValue('/mock/home/user'); + process.env.GEMINI_API_KEY = 'test-api-key'; + process.stdin.isTTY = true; + }); + + afterEach(() => { + process.argv = originalArgv; + process.env = originalEnv; + process.stdin.isTTY = originalIsTTY; + vi.restoreAllMocks(); + }); + + it('should be interactive if isTTY and no prompt', async () => { + process.stdin.isTTY = true; + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.isInteractive()).toBe(true); + }); + + it('should be interactive if prompt-interactive is set', async () => { + process.stdin.isTTY = false; + process.argv = ['node', 'script.js', '--prompt-interactive', 'test']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.isInteractive()).toBe(true); + }); + + it('should not be interactive if not isTTY and no prompt', async () => { + process.stdin.isTTY = false; + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.isInteractive()).toBe(false); + }); + + it('should not be interactive if prompt is set', async () => { + process.stdin.isTTY = true; + process.argv = ['node', 'script.js', '--prompt', 'test']; + const argv = await parseArguments(); + const config = await loadCliConfig({}, [], 'test-session', argv); + expect(config.isInteractive()).toBe(false); + }); +}); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 3104e4c1..d142bd12 100644 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -23,6 +23,9 @@ import { FileDiscoveryService, TelemetryTarget, FileFilteringOptions, + ShellTool, + EditTool, + WriteFileTool, } from '@google/gemini-cli-core'; import { Settings } from './settings.js'; @@ -365,7 +368,22 @@ export async function loadCliConfig( ); let mcpServers = mergeMcpServers(settings, activeExtensions); - const excludeTools = mergeExcludeTools(settings, activeExtensions); + const question = argv.promptInteractive || argv.prompt || ''; + const approvalMode = + argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT; + const interactive = + !!argv.promptInteractive || (process.stdin.isTTY && question.length === 0); + // In non-interactive and non-yolo mode, exclude interactive built in tools. + const extraExcludes = + !interactive && approvalMode !== ApprovalMode.YOLO + ? [ShellTool.Name, EditTool.Name, WriteFileTool.Name] + : undefined; + + const excludeTools = mergeExcludeTools( + settings, + activeExtensions, + extraExcludes, + ); const blockedMcpServers: Array<{ name: string; extensionName: string }> = []; if (!argv.allowedMcpServerNames) { @@ -427,7 +445,7 @@ export async function loadCliConfig( settings.loadMemoryFromIncludeDirectories || false, debugMode, - question: argv.promptInteractive || argv.prompt || '', + question, fullContext: argv.allFiles || argv.all_files || false, coreTools: settings.coreTools || undefined, excludeTools, @@ -437,7 +455,7 @@ export async function loadCliConfig( mcpServers, userMemory: memoryContent, geminiMdFileCount: fileCount, - approvalMode: argv.yolo || false ? ApprovalMode.YOLO : ApprovalMode.DEFAULT, + approvalMode, showMemoryUsage: argv.showMemoryUsage || argv.show_memory_usage || @@ -486,6 +504,7 @@ export async function loadCliConfig( ideModeFeature, chatCompression: settings.chatCompression, folderTrustFeature, + interactive, folderTrust, }); } @@ -514,8 +533,12 @@ function mergeMcpServers(settings: Settings, extensions: Extension[]) { function mergeExcludeTools( settings: Settings, extensions: Extension[], + extraExcludes?: string[] | undefined, ): string[] { - const allExcludeTools = new Set(settings.excludeTools || []); + const allExcludeTools = new Set([ + ...(settings.excludeTools || []), + ...(extraExcludes || []), + ]); for (const extension of extensions) { for (const tool of extension.config.excludeTools || []) { allExcludeTools.add(tool); diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 48dbd271..771fcacb 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -7,7 +7,7 @@ import React from 'react'; import { render } from 'ink'; import { AppWrapper } from './ui/App.js'; -import { loadCliConfig, parseArguments, CliArgs } from './config/config.js'; +import { loadCliConfig, parseArguments } from './config/config.js'; import { readStdin } from './utils/readStdin.js'; import { basename } from 'node:path'; import v8 from 'node:v8'; @@ -25,15 +25,11 @@ import { themeManager } from './ui/themes/theme-manager.js'; import { getStartupWarnings } from './utils/startupWarnings.js'; import { getUserStartupWarnings } from './utils/userStartupWarnings.js'; import { runNonInteractive } from './nonInteractiveCli.js'; -import { loadExtensions, Extension } from './config/extension.js'; +import { loadExtensions } from './config/extension.js'; import { cleanupCheckpoints, registerCleanup } from './utils/cleanup.js'; import { getCliVersion } from './utils/version.js'; import { - ApprovalMode, Config, - EditTool, - ShellTool, - WriteFileTool, sessionId, logUserPrompt, AuthType, @@ -255,11 +251,8 @@ export async function main() { ...(await getUserStartupWarnings(workspaceRoot)), ]; - const shouldBeInteractive = - !!argv.promptInteractive || (process.stdin.isTTY && input?.length === 0); - // Render UI, passing necessary config values. Check that there is no command line question. - if (shouldBeInteractive) { + if (config.isInteractive()) { const version = await getCliVersion(); setWindowTitle(basename(workspaceRoot), settings); const instance = render( @@ -308,12 +301,10 @@ export async function main() { prompt_length: input.length, }); - // Non-interactive mode handled by runNonInteractive - const nonInteractiveConfig = await loadNonInteractiveConfig( + const nonInteractiveConfig = await validateNonInteractiveAuth( + settings.merged.selectedAuthType, + settings.merged.useExternalAuth, config, - extensions, - settings, - argv, ); await runNonInteractive(nonInteractiveConfig, input, prompt_id); @@ -334,43 +325,3 @@ function setWindowTitle(title: string, settings: LoadedSettings) { }); } } - -async function loadNonInteractiveConfig( - config: Config, - extensions: Extension[], - settings: LoadedSettings, - argv: CliArgs, -) { - let finalConfig = config; - if (config.getApprovalMode() !== ApprovalMode.YOLO) { - // Everything is not allowed, ensure that only read-only tools are configured. - const existingExcludeTools = settings.merged.excludeTools || []; - const interactiveTools = [ - ShellTool.Name, - EditTool.Name, - WriteFileTool.Name, - ]; - - const newExcludeTools = [ - ...new Set([...existingExcludeTools, ...interactiveTools]), - ]; - - const nonInteractiveSettings = { - ...settings.merged, - excludeTools: newExcludeTools, - }; - finalConfig = await loadCliConfig( - nonInteractiveSettings, - extensions, - config.getSessionId(), - argv, - ); - await finalConfig.initialize(); - } - - return await validateNonInteractiveAuth( - settings.merged.selectedAuthType, - settings.merged.useExternalAuth, - finalConfig, - ); -} diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 8b056a28..95ed70cf 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -30,7 +30,6 @@ export async function runNonInteractive( }); try { - await config.initialize(); consolePatcher.patch(); // Handle EPIPE errors when the output is piped to a command that closes early. process.stdout.on('error', (err: NodeJS.ErrnoException) => { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 64692139..8e6ca38f 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -150,6 +150,18 @@ describe('Server Config (config.ts)', () => { await expect(config.initialize()).resolves.toBeUndefined(); }); + + it('should throw an error if initialized more than once', async () => { + const config = new Config({ + ...baseParams, + checkpointing: false, + }); + + await expect(config.initialize()).resolves.toBeUndefined(); + await expect(config.initialize()).rejects.toThrow( + 'Config was already initialized', + ); + }); }); describe('refreshAuth', () => { diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index db226c76..473ab5a6 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -197,6 +197,7 @@ export interface ConfigParameters { ideMode?: boolean; loadMemoryFromIncludeDirectories?: boolean; chatCompression?: ChatCompressionSettings; + interactive?: boolean; } export class Config { @@ -260,6 +261,8 @@ export class Config { private readonly experimentalAcp: boolean = false; private readonly loadMemoryFromIncludeDirectories: boolean = false; private readonly chatCompression: ChatCompressionSettings | undefined; + private readonly interactive: boolean; + private initialized: boolean = false; constructor(params: ConfigParameters) { this.sessionId = params.sessionId; @@ -326,6 +329,7 @@ export class Config { this.loadMemoryFromIncludeDirectories = params.loadMemoryFromIncludeDirectories ?? false; this.chatCompression = params.chatCompression; + this.interactive = params.interactive ?? false; if (params.contextFileName) { setGeminiMdFilename(params.contextFileName); @@ -344,7 +348,14 @@ export class Config { } } + /** + * Must only be called once, throws if called again. + */ async initialize(): Promise { + if (this.initialized) { + throw Error('Config was already initialized'); + } + this.initialized = true; // Initialize centralized FileDiscoveryService this.getFileService(); if (this.getCheckpointingEnabled()) { @@ -685,6 +696,10 @@ export class Config { return this.chatCompression; } + isInteractive(): boolean { + return this.interactive; + } + async getGitService(): Promise { if (!this.gitService) { this.gitService = new GitService(this.targetDir);