fix(admin): Ensure CLI commands run in non-interactive mode (#17218)

This commit is contained in:
Shreya Keshive
2026-01-21 12:38:20 -05:00
committed by GitHub
parent 7399c623d8
commit d0cae4547e
8 changed files with 181 additions and 21 deletions

View File

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

View File

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

View File

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

View File

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

View File

@@ -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<typeof import('fs')>();
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', () => {

View File

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

View File

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

View File

@@ -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,