fix(cli): Prevent stdout/stderr patching for extension commands (#13600)

Co-authored-by: jacob314 <jacob314@gmail.com>
This commit is contained in:
christine betts
2025-11-21 21:08:06 -05:00
committed by GitHub
parent 5e218a5630
commit bdf80ea7c0
30 changed files with 83 additions and 14 deletions

View File

@@ -14,6 +14,7 @@ import { enableCommand } from './extensions/enable.js';
import { linkCommand } from './extensions/link.js'; import { linkCommand } from './extensions/link.js';
import { newCommand } from './extensions/new.js'; import { newCommand } from './extensions/new.js';
import { validateCommand } from './extensions/validate.js'; import { validateCommand } from './extensions/validate.js';
import { initializeOutputListenersAndFlush } from '../gemini.js';
export const extensionsCommand: CommandModule = { export const extensionsCommand: CommandModule = {
command: 'extensions <command>', command: 'extensions <command>',
@@ -21,6 +22,7 @@ export const extensionsCommand: CommandModule = {
describe: 'Manage Gemini CLI extensions.', describe: 'Manage Gemini CLI extensions.',
builder: (yargs) => builder: (yargs) =>
yargs yargs
.middleware(() => initializeOutputListenersAndFlush())
.command(installCommand) .command(installCommand)
.command(uninstallCommand) .command(uninstallCommand)
.command(listCommand) .command(listCommand)

View File

@@ -56,6 +56,9 @@ vi.mock('../../config/extensions/consent.js', () => ({
vi.mock('../../config/extensions/extensionSettings.js', () => ({ vi.mock('../../config/extensions/extensionSettings.js', () => ({
promptForSetting: vi.fn(), promptForSetting: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions disable command', () => { describe('extensions disable command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -11,6 +11,7 @@ import { debugLogger } from '@google/gemini-cli-core';
import { ExtensionManager } from '../../config/extension-manager.js'; import { ExtensionManager } from '../../config/extension-manager.js';
import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface DisableArgs { interface DisableArgs {
name: string; name: string;
@@ -81,5 +82,6 @@ export const disableCommand: CommandModule = {
name: argv['name'] as string, name: argv['name'] as string,
scope: argv['scope'] as string, scope: argv['scope'] as string,
}); });
await exitCli();
}, },
}; };

View File

@@ -58,6 +58,9 @@ vi.mock('../../config/extension-manager.js');
vi.mock('../../config/settings.js'); vi.mock('../../config/settings.js');
vi.mock('../../config/extensions/consent.js'); vi.mock('../../config/extensions/consent.js');
vi.mock('../../config/extensions/extensionSettings.js'); vi.mock('../../config/extensions/extensionSettings.js');
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions enable command', () => { describe('extensions enable command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -14,6 +14,7 @@ import {
getErrorMessage, getErrorMessage,
} from '@google/gemini-cli-core'; } from '@google/gemini-cli-core';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface EnableArgs { interface EnableArgs {
name: string; name: string;
@@ -86,5 +87,6 @@ export const enableCommand: CommandModule = {
name: argv['name'] as string, name: argv['name'] as string,
scope: argv['scope'] as string, scope: argv['scope'] as string,
}); });
await exitCli();
}, },
}; };

View File

@@ -48,6 +48,10 @@ vi.mock('node:fs/promises', () => ({
}, },
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions install command', () => { describe('extensions install command', () => {
it('should fail if no source is provided', () => { it('should fail if no source is provided', () => {
const validationParser = yargs([]).command(installCommand).fail(false); const validationParser = yargs([]).command(installCommand).fail(false);

View File

@@ -18,6 +18,7 @@ import {
import { ExtensionManager } from '../../config/extension-manager.js'; import { ExtensionManager } from '../../config/extension-manager.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface InstallArgs { interface InstallArgs {
source: string; source: string;
@@ -130,5 +131,6 @@ export const installCommand: CommandModule = {
allowPreRelease: argv['pre-release'] as boolean | undefined, allowPreRelease: argv['pre-release'] as boolean | undefined,
consent: argv['consent'] as boolean | undefined, consent: argv['consent'] as boolean | undefined,
}); });
await exitCli();
}, },
}; };

View File

@@ -52,6 +52,9 @@ vi.mock('../../config/extensions/consent.js', () => ({
vi.mock('../../config/extensions/extensionSettings.js', () => ({ vi.mock('../../config/extensions/extensionSettings.js', () => ({
promptForSetting: vi.fn(), promptForSetting: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions link command', () => { describe('extensions link command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -15,6 +15,7 @@ import { requestConsentNonInteractive } from '../../config/extensions/consent.js
import { ExtensionManager } from '../../config/extension-manager.js'; import { ExtensionManager } from '../../config/extension-manager.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface InstallArgs { interface InstallArgs {
path: string; path: string;
@@ -60,5 +61,6 @@ export const linkCommand: CommandModule = {
await handleLink({ await handleLink({
path: argv['path'] as string, path: argv['path'] as string,
}); });
await exitCli();
}, },
}; };

View File

@@ -44,6 +44,9 @@ vi.mock('../../config/extensions/consent.js', () => ({
vi.mock('../../config/extensions/extensionSettings.js', () => ({ vi.mock('../../config/extensions/extensionSettings.js', () => ({
promptForSetting: vi.fn(), promptForSetting: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions list command', () => { describe('extensions list command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -11,6 +11,7 @@ import { ExtensionManager } from '../../config/extension-manager.js';
import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
export async function handleList() { export async function handleList() {
try { try {
@@ -45,5 +46,6 @@ export const listCommand: CommandModule = {
builder: (yargs) => yargs, builder: (yargs) => yargs,
handler: async () => { handler: async () => {
await handleList(); await handleList();
await exitCli();
}, },
}; };

View File

@@ -11,6 +11,9 @@ import * as fsPromises from 'node:fs/promises';
import path from 'node:path'; import path from 'node:path';
vi.mock('node:fs/promises'); vi.mock('node:fs/promises');
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
const mockedFs = vi.mocked(fsPromises); const mockedFs = vi.mocked(fsPromises);

View File

@@ -9,6 +9,7 @@ import { join, dirname, basename } from 'node:path';
import type { CommandModule } from 'yargs'; import type { CommandModule } from 'yargs';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { debugLogger } from '@google/gemini-cli-core'; import { debugLogger } from '@google/gemini-cli-core';
import { exitCli } from '../utils.js';
interface NewArgs { interface NewArgs {
path: string; path: string;
@@ -100,5 +101,6 @@ export const newCommand: CommandModule = {
path: args['path'] as string, path: args['path'] as string,
template: args['template'] as string | undefined, template: args['template'] as string | undefined,
}); });
await exitCli();
}, },
}; };

View File

@@ -75,6 +75,9 @@ vi.mock('../../config/extensions/consent.js', () => ({
vi.mock('../../config/extensions/extensionSettings.js', () => ({ vi.mock('../../config/extensions/extensionSettings.js', () => ({
promptForSetting: vi.fn(), promptForSetting: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions uninstall command', () => { describe('extensions uninstall command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -11,6 +11,7 @@ import { requestConsentNonInteractive } from '../../config/extensions/consent.js
import { ExtensionManager } from '../../config/extension-manager.js'; import { ExtensionManager } from '../../config/extension-manager.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface UninstallArgs { interface UninstallArgs {
names: string[]; // can be extension names or source URLs. names: string[]; // can be extension names or source URLs.
@@ -72,5 +73,6 @@ export const uninstallCommand: CommandModule = {
await handleUninstall({ await handleUninstall({
names: argv['names'] as string[], names: argv['names'] as string[],
}); });
await exitCli();
}, },
}; };

View File

@@ -56,6 +56,9 @@ vi.mock('../../config/extensions/consent.js', () => ({
vi.mock('../../config/extensions/extensionSettings.js', () => ({ vi.mock('../../config/extensions/extensionSettings.js', () => ({
promptForSetting: vi.fn(), promptForSetting: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions update command', () => { describe('extensions update command', () => {
const mockLoadSettings = vi.mocked(loadSettings); const mockLoadSettings = vi.mocked(loadSettings);

View File

@@ -19,6 +19,7 @@ import { ExtensionManager } from '../../config/extension-manager.js';
import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
interface UpdateArgs { interface UpdateArgs {
name?: string; name?: string;
@@ -144,5 +145,6 @@ export const updateCommand: CommandModule = {
name: argv['name'] as string | undefined, name: argv['name'] as string | undefined,
all: argv['all'] as boolean | undefined, all: argv['all'] as boolean | undefined,
}); });
await exitCli();
}, },
}; };

View File

@@ -13,6 +13,10 @@ import path from 'node:path';
import * as os from 'node:os'; import * as os from 'node:os';
import { debugLogger } from '@google/gemini-cli-core'; import { debugLogger } from '@google/gemini-cli-core';
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('extensions validate command', () => { describe('extensions validate command', () => {
it('should fail if no path is provided', () => { it('should fail if no path is provided', () => {
const validationParser = yargs([]).command(validateCommand).fail(false); const validationParser = yargs([]).command(validateCommand).fail(false);

View File

@@ -15,6 +15,7 @@ import { ExtensionManager } from '../../config/extension-manager.js';
import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { loadSettings } from '../../config/settings.js'; import { loadSettings } from '../../config/settings.js';
import { exitCli } from '../utils.js';
interface ValidateArgs { interface ValidateArgs {
path: string; path: string;
@@ -101,5 +102,6 @@ export const validateCommand: CommandModule = {
await handleValidate({ await handleValidate({
path: args['path'] as string, path: args['path'] as string,
}); });
await exitCli();
}, },
}; };

View File

@@ -56,6 +56,7 @@ describe('mcp command', () => {
command: vi.fn().mockReturnThis(), command: vi.fn().mockReturnThis(),
demandCommand: vi.fn().mockReturnThis(), demandCommand: vi.fn().mockReturnThis(),
version: vi.fn().mockReturnThis(), version: vi.fn().mockReturnThis(),
middleware: vi.fn().mockReturnThis(),
}; };
(mcpCommand.builder as (y: Argv) => Argv)(mockYargs as unknown as Argv); (mcpCommand.builder as (y: Argv) => Argv)(mockYargs as unknown as Argv);

View File

@@ -9,12 +9,14 @@ import type { CommandModule, Argv } from 'yargs';
import { addCommand } from './mcp/add.js'; import { addCommand } from './mcp/add.js';
import { removeCommand } from './mcp/remove.js'; import { removeCommand } from './mcp/remove.js';
import { listCommand } from './mcp/list.js'; import { listCommand } from './mcp/list.js';
import { initializeOutputListenersAndFlush } from '../gemini.js';
export const mcpCommand: CommandModule = { export const mcpCommand: CommandModule = {
command: 'mcp', command: 'mcp',
describe: 'Manage MCP servers', describe: 'Manage MCP servers',
builder: (yargs: Argv) => builder: (yargs: Argv) =>
yargs yargs
.middleware(() => initializeOutputListenersAndFlush())
.command(addCommand) .command(addCommand)
.command(removeCommand) .command(removeCommand)
.command(listCommand) .command(listCommand)

View File

@@ -10,6 +10,10 @@ import { addCommand } from './add.js';
import { loadSettings, SettingScope } from '../../config/settings.js'; import { loadSettings, SettingScope } from '../../config/settings.js';
import { debugLogger } from '@google/gemini-cli-core'; import { debugLogger } from '@google/gemini-cli-core';
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
vi.mock('fs/promises', () => ({ vi.mock('fs/promises', () => ({
readFile: vi.fn(), readFile: vi.fn(),
writeFile: vi.fn(), writeFile: vi.fn(),

View File

@@ -8,6 +8,7 @@
import type { CommandModule } from 'yargs'; import type { CommandModule } from 'yargs';
import { loadSettings, SettingScope } from '../../config/settings.js'; import { loadSettings, SettingScope } from '../../config/settings.js';
import { debugLogger, type MCPServerConfig } from '@google/gemini-cli-core'; import { debugLogger, type MCPServerConfig } from '@google/gemini-cli-core';
import { exitCli } from '../utils.js';
async function addMcpServer( async function addMcpServer(
name: string, name: string,
@@ -230,5 +231,6 @@ export const addCommand: CommandModule = {
excludeTools: argv['excludeTools'] as string[] | undefined, excludeTools: argv['excludeTools'] as string[] | undefined,
}, },
); );
await exitCli();
}, },
}; };

View File

@@ -44,6 +44,10 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
}); });
vi.mock('@modelcontextprotocol/sdk/client/index.js'); vi.mock('@modelcontextprotocol/sdk/client/index.js');
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
const mockedGetUserExtensionsDir = const mockedGetUserExtensionsDir =
ExtensionStorage.getUserExtensionsDir as Mock; ExtensionStorage.getUserExtensionsDir as Mock;
const mockedLoadSettings = loadSettings as Mock; const mockedLoadSettings = loadSettings as Mock;

View File

@@ -17,6 +17,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js';
import { ExtensionManager } from '../../config/extension-manager.js'; import { ExtensionManager } from '../../config/extension-manager.js';
import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js';
import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js';
import { exitCli } from '../utils.js';
const COLOR_GREEN = '\u001b[32m'; const COLOR_GREEN = '\u001b[32m';
const COLOR_YELLOW = '\u001b[33m'; const COLOR_YELLOW = '\u001b[33m';
@@ -145,5 +146,6 @@ export const listCommand: CommandModule = {
describe: 'List all configured MCP servers', describe: 'List all configured MCP servers',
handler: async () => { handler: async () => {
await listMcpServers(); await listMcpServers();
await exitCli();
}, },
}; };

View File

@@ -26,6 +26,10 @@ vi.mock('fs/promises', () => ({
writeFile: vi.fn(), writeFile: vi.fn(),
})); }));
vi.mock('../utils.js', () => ({
exitCli: vi.fn(),
}));
describe('mcp remove command', () => { describe('mcp remove command', () => {
describe('unit tests with mocks', () => { describe('unit tests with mocks', () => {
let parser: Argv; let parser: Argv;

View File

@@ -8,6 +8,7 @@
import type { CommandModule } from 'yargs'; import type { CommandModule } from 'yargs';
import { loadSettings, SettingScope } from '../../config/settings.js'; import { loadSettings, SettingScope } from '../../config/settings.js';
import { debugLogger } from '@google/gemini-cli-core'; import { debugLogger } from '@google/gemini-cli-core';
import { exitCli } from '../utils.js';
async function removeMcpServer( async function removeMcpServer(
name: string, name: string,
@@ -57,5 +58,6 @@ export const removeCommand: CommandModule = {
await removeMcpServer(argv['name'] as string, { await removeMcpServer(argv['name'] as string, {
scope: argv['scope'] as string, scope: argv['scope'] as string,
}); });
await exitCli();
}, },
}; };

View File

@@ -0,0 +1,12 @@
/**
* @license
* Copyright 2025 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { runExitCleanup } from '../utils/cleanup.js';
export async function exitCli(exitCode = 0) {
await runExitCleanup();
process.exit(exitCode);
}

View File

@@ -312,19 +312,6 @@ export async function parseArguments(settings: Settings): Promise<CliArgs> {
process.exit(0); process.exit(0);
} }
// If yargs handled --help/--version it will have exited; nothing to do here.
// Handle case where MCP subcommands are executed - they should exit the process
// and not return to main CLI logic
if (
result._.length > 0 &&
(result._[0] === 'mcp' || result._[0] === 'extensions')
) {
// MCP commands handle their own execution and process exit
await runExitCleanup();
process.exit(0);
}
// Normalize query args: handle both quoted "@path file" and unquoted @path file // Normalize query args: handle both quoted "@path file" and unquoted @path file
const queryArg = (result as { query?: string | string[] | undefined }).query; const queryArg = (result as { query?: string | string[] | undefined }).query;
const q: string | undefined = Array.isArray(queryArg) const q: string | undefined = Array.isArray(queryArg)

View File

@@ -634,7 +634,7 @@ function setWindowTitle(title: string, settings: LoadedSettings) {
} }
} }
function initializeOutputListenersAndFlush() { export function initializeOutputListenersAndFlush() {
// If there are no listeners for output, make sure we flush so output is not // If there are no listeners for output, make sure we flush so output is not
// lost. // lost.
if (coreEvents.listenerCount(CoreEvent.Output) === 0) { if (coreEvents.listenerCount(CoreEvent.Output) === 0) {