jcarr
/
gemini-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

avoid loading and initializing CLI config twice in non-interactive mode (#5793)

This commit is contained in:
Jacob MacDonald 2025-08-07 14:19:06 -07:00 committed by GitHub
parent 53f8617b24
commit 19491b7b94
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 201 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

142
packages/cli/src/config/config.test.ts
View File

@ -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);
});
});

31
packages/cli/src/config/config.ts
View File

@ -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);

61
packages/cli/src/gemini.tsx
View File

@ -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,
);
}

1
packages/cli/src/nonInteractiveCli.ts
View File

@ -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) => {

12
packages/core/src/config/config.test.ts
View File

@ -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', () => {

15
packages/core/src/config/config.ts
View File

@ -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<void> {
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<GitService> {
if (!this.gitService) {
this.gitService = new GitService(this.targetDir);