From d0cae4547e248282118276f6d65ec7f27bbf407b Mon Sep 17 00:00:00 2001 From: Shreya Keshive Date: Wed, 21 Jan 2026 12:38:20 -0500 Subject: [PATCH] fix(admin): Ensure CLI commands run in non-interactive mode (#17218) --- packages/cli/src/commands/extensions.tsx | 5 +- packages/cli/src/commands/hooks.tsx | 5 +- packages/cli/src/commands/mcp.ts | 5 +- packages/cli/src/commands/skills.tsx | 5 +- packages/cli/src/config/config.test.ts | 54 +++++++++++ packages/cli/src/config/config.ts | 4 +- packages/cli/src/gemini.test.tsx | 112 +++++++++++++++++++++-- packages/cli/src/gemini.tsx | 12 +-- 8 files changed, 181 insertions(+), 21 deletions(-) diff --git a/packages/cli/src/commands/extensions.tsx b/packages/cli/src/commands/extensions.tsx index 893af14d11..ec646cfc82 100644 --- a/packages/cli/src/commands/extensions.tsx +++ b/packages/cli/src/commands/extensions.tsx @@ -24,7 +24,10 @@ export const extensionsCommand: CommandModule = { describe: 'Manage Gemini CLI extensions.', builder: (yargs) => yargs - .middleware(() => initializeOutputListenersAndFlush()) + .middleware((argv) => { + initializeOutputListenersAndFlush(); + argv['isCommand'] = true; + }) .command(defer(installCommand, 'extensions')) .command(defer(uninstallCommand, 'extensions')) .command(defer(listCommand, 'extensions')) diff --git a/packages/cli/src/commands/hooks.tsx b/packages/cli/src/commands/hooks.tsx index 4475d33ab9..fdb4594d04 100644 --- a/packages/cli/src/commands/hooks.tsx +++ b/packages/cli/src/commands/hooks.tsx @@ -14,7 +14,10 @@ export const hooksCommand: CommandModule = { describe: 'Manage Gemini CLI hooks.', builder: (yargs) => yargs - .middleware(() => initializeOutputListenersAndFlush()) + .middleware((argv) => { + initializeOutputListenersAndFlush(); + argv['isCommand'] = true; + }) .command(migrateCommand) .demandCommand(1, 'You need at least one command before continuing.') .version(false), diff --git a/packages/cli/src/commands/mcp.ts b/packages/cli/src/commands/mcp.ts index c14def2199..8f12fc50fd 100644 --- a/packages/cli/src/commands/mcp.ts +++ b/packages/cli/src/commands/mcp.ts @@ -17,7 +17,10 @@ export const mcpCommand: CommandModule = { describe: 'Manage MCP servers', builder: (yargs: Argv) => yargs - .middleware(() => initializeOutputListenersAndFlush()) + .middleware((argv) => { + initializeOutputListenersAndFlush(); + argv['isCommand'] = true; + }) .command(defer(addCommand, 'mcp')) .command(defer(removeCommand, 'mcp')) .command(defer(listCommand, 'mcp')) diff --git a/packages/cli/src/commands/skills.tsx b/packages/cli/src/commands/skills.tsx index b8879681cd..1559cf42ff 100644 --- a/packages/cli/src/commands/skills.tsx +++ b/packages/cli/src/commands/skills.tsx @@ -19,7 +19,10 @@ export const skillsCommand: CommandModule = { describe: 'Manage agent skills.', builder: (yargs) => yargs - .middleware(() => initializeOutputListenersAndFlush()) + .middleware((argv) => { + initializeOutputListenersAndFlush(); + argv['isCommand'] = true; + }) .command(defer(listCommand, 'skills')) .command(defer(enableCommand, 'skills')) .command(defer(disableCommand, 'skills')) diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index a1ffeb0c36..b2e38c94b9 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -34,6 +34,10 @@ vi.mock('./sandboxConfig.js', () => ({ loadSandboxConfig: vi.fn(async () => undefined), })); +vi.mock('../commands/utils.js', () => ({ + exitCli: vi.fn(), +})); + vi.mock('fs', async (importOriginal) => { const actualFs = await importOriginal(); const pathMod = await import('node:path'); @@ -534,6 +538,42 @@ describe('parseArguments', () => { 'Use whoami to write a poem in file poem.md about my username in pig latin and use wc to tell me how many lines are in the poem you wrote.', ); }); + + it('should set isCommand to true for mcp command', async () => { + process.argv = ['node', 'script.js', 'mcp', 'list']; + const argv = await parseArguments(createTestMergedSettings()); + expect(argv.isCommand).toBe(true); + }); + + it('should set isCommand to true for extensions command', async () => { + process.argv = ['node', 'script.js', 'extensions', 'list']; + // Extensions command uses experimental settings + const settings = createTestMergedSettings({ + experimental: { extensionManagement: true }, + }); + const argv = await parseArguments(settings); + expect(argv.isCommand).toBe(true); + }); + + it('should set isCommand to true for skills command', async () => { + process.argv = ['node', 'script.js', 'skills', 'list']; + // Skills command enabled by default or via experimental + const settings = createTestMergedSettings({ + experimental: { skills: true }, + }); + const argv = await parseArguments(settings); + expect(argv.isCommand).toBe(true); + }); + + it('should set isCommand to true for hooks command', async () => { + process.argv = ['node', 'script.js', 'hooks', 'migrate']; + // Hooks command enabled via tools settings + const settings = createTestMergedSettings({ + tools: { enableHooks: true }, + }); + const argv = await parseArguments(settings); + expect(argv.isCommand).toBe(true); + }); }); describe('loadCliConfig', () => { @@ -639,6 +679,20 @@ describe('loadCliConfig', () => { ); expect(config.getApprovalMode()).toBe(ApprovalMode.DEFAULT); }); + + it('should be non-interactive when isCommand is set', async () => { + process.argv = ['node', 'script.js', 'mcp', 'list']; + const argv = await parseArguments(createTestMergedSettings()); + argv.isCommand = true; // explicitly set it as if middleware ran (it does in parseArguments but we want to be sure for this isolated test if we were mocking argv) + + // reset tty for this test + process.stdin.isTTY = true; + + const settings = createTestMergedSettings(); + const config = await loadCliConfig(settings, 'test-session', argv); + + expect(config.isInteractive()).toBe(false); + }); }); describe('Hierarchical Memory Loading (config.ts) - Placeholder Suite', () => { diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 65ac1fc4c3..454c0b3a58 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -85,6 +85,7 @@ export interface CliArgs { recordResponses: string | undefined; rawOutput: boolean | undefined; acceptRawOutputRisk: boolean | undefined; + isCommand: boolean | undefined; } export async function parseArguments( @@ -97,7 +98,6 @@ export async function parseArguments( .usage( 'Usage: gemini [options] [command]\n\nGemini CLI - Launch an interactive CLI, use -p/--prompt for non-interactive mode', ) - .option('debug', { alias: 'd', type: 'boolean', @@ -575,7 +575,7 @@ export async function loadCliConfig( const interactive = !!argv.promptInteractive || !!argv.experimentalAcp || - (process.stdin.isTTY && !hasQuery && !argv.prompt); + (process.stdin.isTTY && !hasQuery && !argv.prompt && !argv.isCommand); const allowedTools = argv.allowedTools || settings.tools?.allowed || []; const allowedToolsSet = new Set(allowedTools); diff --git a/packages/cli/src/gemini.test.tsx b/packages/cli/src/gemini.test.tsx index ef801786cf..59f59de4b1 100644 --- a/packages/cli/src/gemini.test.tsx +++ b/packages/cli/src/gemini.test.tsx @@ -205,10 +205,14 @@ vi.mock('./config/sandboxConfig.js', () => ({ vi.mock('./ui/utils/mouse.js', () => ({ enableMouseEvents: vi.fn(), disableMouseEvents: vi.fn(), - parseMouseEvent: vi.fn(), isIncompleteMouseSequence: vi.fn(), })); +const runNonInteractiveSpy = vi.hoisted(() => vi.fn()); +vi.mock('./nonInteractiveCli.js', () => ({ + runNonInteractive: runNonInteractiveSpy, +})); + describe('gemini.tsx main function', () => { let originalEnvGeminiSandbox: string | undefined; let originalEnvSandbox: string | undefined; @@ -492,6 +496,7 @@ describe('gemini.tsx main function kitty protocol', () => { recordResponses: undefined, rawOutput: undefined, acceptRawOutputRisk: undefined, + isCommand: undefined, }); await act(async () => { @@ -561,6 +566,7 @@ describe('gemini.tsx main function kitty protocol', () => { getProjectRoot: () => '/', getRemoteAdminSettings: () => undefined, setTerminalBackground: vi.fn(), + refreshAuth: vi.fn(), } as unknown as Config; vi.mocked(loadCliConfig).mockResolvedValue(mockConfig); @@ -573,10 +579,13 @@ describe('gemini.tsx main function kitty protocol', () => { .spyOn(debugLogger, 'log') .mockImplementation(() => {}); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); } catch (e) { if (!(e instanceof MockProcessExitError)) throw e; + } finally { + delete process.env['GEMINI_API_KEY']; } if (flag === 'listExtensions') { @@ -656,10 +665,13 @@ describe('gemini.tsx main function kitty protocol', () => { await fn(); }); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); } catch (e) { if (!(e instanceof MockProcessExitError)) throw e; + } finally { + delete process.env['GEMINI_API_KEY']; } expect(start_sandbox).toHaveBeenCalled(); @@ -737,10 +749,13 @@ describe('gemini.tsx main function kitty protocol', () => { vi.spyOn(themeManager, 'setActiveTheme').mockReturnValue(false); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); } catch (e) { if (!(e instanceof MockProcessExitError)) throw e; + } finally { + delete process.env['GEMINI_API_KEY']; } expect(debugLoggerWarnSpy).toHaveBeenCalledWith( @@ -990,14 +1005,6 @@ describe('gemini.tsx main function kitty protocol', () => { vi.mock('./utils/readStdin.js', () => ({ readStdin: vi.fn().mockResolvedValue('stdin-data'), })); - const runNonInteractiveSpy = vi.hoisted(() => vi.fn()); - vi.mock('./nonInteractiveCli.js', () => ({ - runNonInteractive: runNonInteractiveSpy, - })); - runNonInteractiveSpy.mockClear(); - vi.mock('./validateNonInterActiveAuth.js', () => ({ - validateNonInteractiveAuth: vi.fn().mockResolvedValue({}), - })); // Mock stdin to be non-TTY Object.defineProperty(process.stdin, 'isTTY', { @@ -1005,10 +1012,13 @@ describe('gemini.tsx main function kitty protocol', () => { configurable: true, }); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); } catch (e) { if (!(e instanceof MockProcessExitError)) throw e; + } finally { + delete process.env['GEMINI_API_KEY']; } expect(readStdin).toHaveBeenCalled(); @@ -1151,6 +1161,7 @@ describe('gemini.tsx main function exit codes', () => { storage: { getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), }, + refreshAuth: vi.fn(), } as unknown as Config); vi.mocked(loadSettings).mockReturnValue( createMockSettings({ @@ -1169,12 +1180,15 @@ describe('gemini.tsx main function exit codes', () => { })), })); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); expect.fail('Should have thrown MockProcessExitError'); } catch (e) { expect(e).toBeInstanceOf(MockProcessExitError); expect((e as MockProcessExitError).code).toBe(42); + } finally { + delete process.env['GEMINI_API_KEY']; } }); @@ -1219,6 +1233,7 @@ describe('gemini.tsx main function exit codes', () => { storage: { getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), }, + refreshAuth: vi.fn(), getRemoteAdminSettings: () => undefined, } as unknown as Config); vi.mocked(loadSettings).mockReturnValue( @@ -1232,14 +1247,93 @@ describe('gemini.tsx main function exit codes', () => { configurable: true, }); + process.env['GEMINI_API_KEY'] = 'test-key'; try { await main(); expect.fail('Should have thrown MockProcessExitError'); } catch (e) { expect(e).toBeInstanceOf(MockProcessExitError); expect((e as MockProcessExitError).code).toBe(42); + } finally { + delete process.env['GEMINI_API_KEY']; } }); + + it('should validate and refresh auth in non-interactive mode when no auth type is selected but env var is present', async () => { + const { loadCliConfig, parseArguments } = await import( + './config/config.js' + ); + const { loadSettings } = await import('./config/settings.js'); + const { AuthType } = await import('@google/gemini-cli-core'); + + const refreshAuthSpy = vi.fn(); + + vi.mocked(loadCliConfig).mockResolvedValue({ + isInteractive: () => false, + getQuestion: () => 'test prompt', + getSandbox: () => false, + getDebugMode: () => false, + getListExtensions: () => false, + getListSessions: () => false, + getDeleteSession: () => undefined, + getMcpServers: () => ({}), + getMcpClientManager: vi.fn(), + initialize: vi.fn(), + getIdeMode: () => false, + getExperimentalZedIntegration: () => false, + getScreenReader: () => false, + getGeminiMdFileCount: () => 0, + getPolicyEngine: vi.fn(), + getMessageBus: () => ({ subscribe: vi.fn() }), + getEnableHooks: () => false, + getHookSystem: () => undefined, + getToolRegistry: vi.fn(), + getContentGeneratorConfig: vi.fn(), + getModel: () => 'gemini-pro', + getEmbeddingModel: () => 'embedding-001', + getApprovalMode: () => 'default', + getCoreTools: () => [], + getTelemetryEnabled: () => false, + getTelemetryLogPromptsEnabled: () => false, + getFileFilteringRespectGitIgnore: () => true, + getOutputFormat: () => 'text', + getExtensions: () => [], + getUsageStatisticsEnabled: () => false, + setTerminalBackground: vi.fn(), + storage: { + getProjectTempDir: vi.fn().mockReturnValue('/tmp/gemini-test'), + }, + refreshAuth: refreshAuthSpy, + getRemoteAdminSettings: () => undefined, + } as unknown as Config); + + vi.mocked(loadSettings).mockReturnValue( + createMockSettings({ + merged: { security: { auth: { selectedType: undefined } }, ui: {} }, + }), + ); + vi.mocked(parseArguments).mockResolvedValue({} as unknown as CliArgs); + + runNonInteractiveSpy.mockImplementation(() => Promise.resolve()); + + const processExitSpy = vi + .spyOn(process, 'exit') + .mockImplementation((code) => { + throw new MockProcessExitError(code); + }); + + process.env['GEMINI_API_KEY'] = 'test-key'; + try { + await main(); + } catch (e) { + if (!(e instanceof MockProcessExitError)) throw e; + } finally { + delete process.env['GEMINI_API_KEY']; + processExitSpy.mockRestore(); + } + + expect(refreshAuthSpy).toHaveBeenCalledWith(AuthType.USE_GEMINI); + }); }); describe('validateDnsResolutionOrder', () => { diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 733444fdba..14bb41b8c0 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -373,12 +373,12 @@ export async function main() { // Refresh auth to fetch remote admin settings from CCPA and before entering // the sandbox because the sandbox will interfere with the Oauth2 web // redirect. - if ( - settings.merged.security.auth.selectedType && - !settings.merged.security.auth.useExternal - ) { + if (!settings.merged.security.auth.useExternal) { try { - if (partialConfig.isInteractive()) { + if ( + partialConfig.isInteractive() && + settings.merged.security.auth.selectedType + ) { const err = validateAuthMethod( settings.merged.security.auth.selectedType, ); @@ -389,7 +389,7 @@ export async function main() { await partialConfig.refreshAuth( settings.merged.security.auth.selectedType, ); - } else { + } else if (!partialConfig.isInteractive()) { const authType = await validateNonInteractiveAuth( settings.merged.security.auth.selectedType, settings.merged.security.auth.useExternal,