From 0a8da988ed81af2b660c370b671b14e32d38597a Mon Sep 17 00:00:00 2001 From: Spencer Date: Mon, 6 Apr 2026 22:40:23 -0400 Subject: [PATCH] fix(cli): ensure skills list outputs to stdout in non-interactive environments (#24566) --- packages/cli/src/commands/skills/list.test.ts | 53 +++++++++---------- packages/cli/src/commands/skills/list.ts | 15 +++--- 2 files changed, 33 insertions(+), 35 deletions(-) diff --git a/packages/cli/src/commands/skills/list.test.ts b/packages/cli/src/commands/skills/list.test.ts index 391749242b..37f6b26613 100644 --- a/packages/cli/src/commands/skills/list.test.ts +++ b/packages/cli/src/commands/skills/list.test.ts @@ -4,8 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; -import { coreEvents, type Config } from '@google/gemini-cli-core'; +import { + vi, + describe, + it, + expect, + beforeEach, + afterEach, + type MockInstance, +} from 'vitest'; +import { type Config } from '@google/gemini-cli-core'; import { handleList, listCommand } from './list.js'; import { loadSettings, type LoadedSettings } from '../../config/settings.js'; import { loadCliConfig } from '../../config/config.js'; @@ -32,12 +40,16 @@ vi.mock('../utils.js', () => ({ describe('skills list command', () => { const mockLoadSettings = vi.mocked(loadSettings); const mockLoadCliConfig = vi.mocked(loadCliConfig); + let stdoutWriteSpy: MockInstance; beforeEach(async () => { vi.clearAllMocks(); mockLoadSettings.mockReturnValue({ merged: {}, } as unknown as LoadedSettings); + stdoutWriteSpy = vi + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); }); afterEach(() => { @@ -56,10 +68,7 @@ describe('skills list command', () => { await handleList({}); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', - 'No skills discovered.', - ); + expect(stdoutWriteSpy).toHaveBeenCalledWith('No skills discovered.\n'); }); it('should list all discovered skills', async () => { @@ -87,24 +96,19 @@ describe('skills list command', () => { await handleList({}); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', - chalk.bold('Discovered Agent Skills:'), + expect(stdoutWriteSpy).toHaveBeenCalledWith( + chalk.bold('Discovered Agent Skills:') + '\n\n', ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining('skill1'), ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining(chalk.green('[Enabled]')), ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining('skill2'), ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining(chalk.red('[Disabled]')), ); }); @@ -135,12 +139,10 @@ describe('skills list command', () => { // Default await handleList({ all: false }); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining('regular'), ); - expect(coreEvents.emitConsoleLog).not.toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).not.toHaveBeenCalledWith( expect.stringContaining('builtin'), ); @@ -148,16 +150,13 @@ describe('skills list command', () => { // With all: true await handleList({ all: true }); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining('regular'), ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining('builtin'), ); - expect(coreEvents.emitConsoleLog).toHaveBeenCalledWith( - 'log', + expect(stdoutWriteSpy).toHaveBeenCalledWith( expect.stringContaining(chalk.gray(' [Built-in]')), ); }); diff --git a/packages/cli/src/commands/skills/list.ts b/packages/cli/src/commands/skills/list.ts index 49fc3a54f1..bc05c6c2af 100644 --- a/packages/cli/src/commands/skills/list.ts +++ b/packages/cli/src/commands/skills/list.ts @@ -5,7 +5,6 @@ */ import type { CommandModule } from 'yargs'; -import { debugLogger } from '@google/gemini-cli-core'; import { loadSettings } from '../../config/settings.js'; import { loadCliConfig, type CliArgs } from '../../config/config.js'; import { exitCli } from '../utils.js'; @@ -42,12 +41,11 @@ export async function handleList(args: { all?: boolean }) { }); if (skills.length === 0) { - debugLogger.log('No skills discovered.'); + process.stdout.write('No skills discovered.\n'); return; } - debugLogger.log(chalk.bold('Discovered Agent Skills:')); - debugLogger.log(''); + process.stdout.write(chalk.bold('Discovered Agent Skills:') + '\n\n'); for (const skill of skills) { const status = skill.disabled @@ -56,10 +54,11 @@ export async function handleList(args: { all?: boolean }) { const builtinSuffix = skill.isBuiltin ? chalk.gray(' [Built-in]') : ''; - debugLogger.log(`${chalk.bold(skill.name)} ${status}${builtinSuffix}`); - debugLogger.log(` Description: ${skill.description}`); - debugLogger.log(` Location: ${skill.location}`); - debugLogger.log(''); + process.stdout.write( + `${chalk.bold(skill.name)} ${status}${builtinSuffix}\n`, + ); + process.stdout.write(` Description: ${skill.description}\n`); + process.stdout.write(` Location: ${skill.location}\n\n`); } }