diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 971e21d99b..04a370d7e9 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -31,7 +31,6 @@ import { ValidationRequiredError, type AdminControlsSettings, debugLogger, - isHeadlessMode, } from '@google/gemini-cli-core'; import { loadCliConfig, parseArguments } from './config/config.js'; @@ -272,7 +271,6 @@ export async function main() { const isDebugMode = cliConfig.isDebugMode(argv); const consolePatcher = new ConsolePatcher({ stderr: true, - suppressConsoleOutput: isHeadlessMode() && !isDebugMode, debugMode: isDebugMode, onNewMessage: (msg) => { coreEvents.emitConsoleLog(msg.type, msg.content); diff --git a/packages/cli/src/nonInteractiveCli.test.ts b/packages/cli/src/nonInteractiveCli.test.ts index 81729f3900..fb3095a04c 100644 --- a/packages/cli/src/nonInteractiveCli.test.ts +++ b/packages/cli/src/nonInteractiveCli.test.ts @@ -235,30 +235,6 @@ describe('runNonInteractive', () => { const getWrittenOutput = () => processStdoutSpy.mock.calls.map((c) => c[0]).join(''); - it('should initialize ConsolePatcher with correct value', async () => { - const { ConsolePatcher } = await import('./ui/utils/ConsolePatcher.js'); - const patcherSpy = vi.spyOn(ConsolePatcher.prototype, 'patch'); - - const events: ServerGeminiStreamEvent[] = [ - { - type: GeminiEventType.Finished, - value: { reason: undefined, usageMetadata: undefined }, - }, - ]; - mockGeminiClient.sendMessageStream.mockReturnValueOnce( - createStreamFromEvents(events), - ); - - await runNonInteractive({ - config: mockConfig, - settings: mockSettings, - input: 'test input', - prompt_id: 'test-prompt-id', - }); - - expect(patcherSpy).toHaveBeenCalled(); - }); - it('should process input and write text output', async () => { const events: ServerGeminiStreamEvent[] = [ { type: GeminiEventType.Content, value: 'Hello' }, diff --git a/packages/cli/src/nonInteractiveCli.ts b/packages/cli/src/nonInteractiveCli.ts index 592b73e16d..891e3d0ee9 100644 --- a/packages/cli/src/nonInteractiveCli.ts +++ b/packages/cli/src/nonInteractiveCli.ts @@ -27,7 +27,6 @@ import { CoreEvent, createWorkingStdio, recordToolCallInteractions, - isHeadlessMode, ToolErrorType, Scheduler, ROOT_SCHEDULER_ID, @@ -64,10 +63,8 @@ export async function runNonInteractive({ resumedSessionData, }: RunNonInteractiveParams): Promise { return promptIdContext.run(prompt_id, async () => { - const suppressConsoleOutput = isHeadlessMode() && !config.getDebugMode(); const consolePatcher = new ConsolePatcher({ stderr: true, - suppressConsoleOutput, debugMode: config.getDebugMode(), onNewMessage: (msg) => { coreEvents.emitConsoleLog(msg.type, msg.content); diff --git a/packages/cli/src/ui/utils/ConsolePatcher.test.ts b/packages/cli/src/ui/utils/ConsolePatcher.test.ts index 2c5c0b6601..2e50c2bba6 100644 --- a/packages/cli/src/ui/utils/ConsolePatcher.test.ts +++ b/packages/cli/src/ui/utils/ConsolePatcher.test.ts @@ -6,135 +6,180 @@ /* eslint-disable no-console */ -import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, vi, afterEach } from 'vitest'; import { ConsolePatcher } from './ConsolePatcher.js'; describe('ConsolePatcher', () => { let patcher: ConsolePatcher; const onNewMessage = vi.fn(); - beforeEach(() => { - vi.clearAllMocks(); - }); - afterEach(() => { if (patcher) { patcher.cleanup(); } + vi.restoreAllMocks(); + vi.clearAllMocks(); }); - it('should suppress output when suppressConsoleOutput is true and debugMode is false', () => { - const originalError = console.error; - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + it('should patch and restore console methods', () => { + const beforeLog = console.log; + const beforeWarn = console.warn; + const beforeError = console.error; + const beforeDebug = console.debug; + const beforeInfo = console.info; - patcher = new ConsolePatcher({ - debugMode: false, - suppressConsoleOutput: true, - stderr: true, - }); + patcher = new ConsolePatcher({ onNewMessage, debugMode: false }); patcher.patch(); - console.log('test log'); - - expect(errorSpy).not.toHaveBeenCalled(); + expect(console.log).not.toBe(beforeLog); + expect(console.warn).not.toBe(beforeWarn); + expect(console.error).not.toBe(beforeError); + expect(console.debug).not.toBe(beforeDebug); + expect(console.info).not.toBe(beforeInfo); patcher.cleanup(); - errorSpy.mockRestore(); - console.error = originalError; + + expect(console.log).toBe(beforeLog); + expect(console.warn).toBe(beforeWarn); + expect(console.error).toBe(beforeError); + expect(console.debug).toBe(beforeDebug); + expect(console.info).toBe(beforeInfo); }); - it('should NOT suppress console.error even when suppressConsoleOutput is true', () => { - const originalError = console.error; - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + describe('when stderr is false', () => { + it('should call onNewMessage for log, warn, error, and info', () => { + patcher = new ConsolePatcher({ + onNewMessage, + debugMode: false, + stderr: false, + }); + patcher.patch(); - patcher = new ConsolePatcher({ - debugMode: false, - suppressConsoleOutput: true, - stderr: true, - }); - patcher.patch(); - - console.error('test error'); - - expect(errorSpy).toHaveBeenCalled(); - - patcher.cleanup(); - errorSpy.mockRestore(); - console.error = originalError; - }); - - it('should NOT suppress output when suppressConsoleOutput is true but debugMode is true', () => { - const originalError = console.error; - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - patcher = new ConsolePatcher({ - debugMode: true, - suppressConsoleOutput: true, - stderr: true, - }); - patcher.patch(); - - console.log('test log'); - // When stderr is true, log goes to originalConsoleError - expect(errorSpy).toHaveBeenCalled(); - - patcher.cleanup(); - errorSpy.mockRestore(); - console.error = originalError; - }); - - it('should NOT suppress output when suppressConsoleOutput is false', () => { - const originalError = console.error; - const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); - - patcher = new ConsolePatcher({ - debugMode: false, - suppressConsoleOutput: false, - stderr: true, - }); - patcher.patch(); - - console.error('test error'); - expect(errorSpy).toHaveBeenCalled(); - - patcher.cleanup(); - errorSpy.mockRestore(); - console.error = originalError; - }); - - it('should call onNewMessage when stderr is false and not suppressed', () => { - patcher = new ConsolePatcher({ - debugMode: false, - suppressConsoleOutput: false, - stderr: false, - onNewMessage, - }); - patcher.patch(); - - console.log('test log'); - expect(onNewMessage).toHaveBeenCalledWith( - expect.objectContaining({ + console.log('test log'); + expect(onNewMessage).toHaveBeenCalledWith({ type: 'log', content: 'test log', - }), - ); + count: 1, + }); - patcher.cleanup(); + console.warn('test warn'); + expect(onNewMessage).toHaveBeenCalledWith({ + type: 'warn', + content: 'test warn', + count: 1, + }); + + console.error('test error'); + expect(onNewMessage).toHaveBeenCalledWith({ + type: 'error', + content: 'test error', + count: 1, + }); + + console.info('test info'); + expect(onNewMessage).toHaveBeenCalledWith({ + type: 'info', + content: 'test info', + count: 1, + }); + }); + + it('should not call onNewMessage for debug when debugMode is false', () => { + patcher = new ConsolePatcher({ + onNewMessage, + debugMode: false, + stderr: false, + }); + patcher.patch(); + + console.debug('test debug'); + expect(onNewMessage).not.toHaveBeenCalled(); + }); + + it('should call onNewMessage for debug when debugMode is true', () => { + patcher = new ConsolePatcher({ + onNewMessage, + debugMode: true, + stderr: false, + }); + patcher.patch(); + + console.debug('test debug'); + expect(onNewMessage).toHaveBeenCalledWith({ + type: 'debug', + content: 'test debug', + count: 1, + }); + }); + + it('should format multiple arguments using util.format', () => { + patcher = new ConsolePatcher({ + onNewMessage, + debugMode: false, + stderr: false, + }); + patcher.patch(); + + console.log('test %s %d', 'string', 123); + expect(onNewMessage).toHaveBeenCalledWith({ + type: 'log', + content: 'test string 123', + count: 1, + }); + }); }); - it('should NOT suppress output when suppressConsoleOutput is true but debugMode is true (explicit check)', () => { - const onNewMessage = vi.fn(); - patcher = new ConsolePatcher({ - debugMode: true, - suppressConsoleOutput: true, - stderr: false, - onNewMessage, + describe('when stderr is true', () => { + it('should redirect warn and error to originalConsoleError', () => { + const spyError = vi.spyOn(console, 'error').mockImplementation(() => {}); + patcher = new ConsolePatcher({ debugMode: false, stderr: true }); + patcher.patch(); + + console.warn('test warn'); + expect(spyError).toHaveBeenCalledWith('test warn'); + + console.error('test error'); + expect(spyError).toHaveBeenCalledWith('test error'); }); - patcher.patch(); - console.log('test log'); + it('should ignore log and info when debugMode is false', () => { + const spyError = vi.spyOn(console, 'error').mockImplementation(() => {}); + patcher = new ConsolePatcher({ debugMode: false, stderr: true }); + patcher.patch(); - expect(onNewMessage).toHaveBeenCalled(); - patcher.cleanup(); + console.log('test log'); + console.info('test info'); + expect(spyError).not.toHaveBeenCalled(); + }); + + it('should redirect log and info to originalConsoleError when debugMode is true', () => { + const spyError = vi.spyOn(console, 'error').mockImplementation(() => {}); + patcher = new ConsolePatcher({ debugMode: true, stderr: true }); + patcher.patch(); + + console.log('test log'); + expect(spyError).toHaveBeenCalledWith('test log'); + + console.info('test info'); + expect(spyError).toHaveBeenCalledWith('test info'); + }); + + it('should ignore debug when debugMode is false', () => { + const spyError = vi.spyOn(console, 'error').mockImplementation(() => {}); + patcher = new ConsolePatcher({ debugMode: false, stderr: true }); + patcher.patch(); + + console.debug('test debug'); + expect(spyError).not.toHaveBeenCalled(); + }); + + it('should redirect debug to originalConsoleError when debugMode is true', () => { + const spyError = vi.spyOn(console, 'error').mockImplementation(() => {}); + patcher = new ConsolePatcher({ debugMode: true, stderr: true }); + patcher.patch(); + + console.debug('test debug'); + expect(spyError).toHaveBeenCalledWith('test debug'); + }); }); }); diff --git a/packages/cli/src/ui/utils/ConsolePatcher.ts b/packages/cli/src/ui/utils/ConsolePatcher.ts index f1aa83b8e8..e0b5814199 100644 --- a/packages/cli/src/ui/utils/ConsolePatcher.ts +++ b/packages/cli/src/ui/utils/ConsolePatcher.ts @@ -13,7 +13,6 @@ interface ConsolePatcherParams { onNewMessage?: (message: Omit) => void; debugMode: boolean; stderr?: boolean; - suppressConsoleOutput?: boolean; } export class ConsolePatcher { @@ -50,15 +49,10 @@ export class ConsolePatcher { private patchConsoleMethod = (type: 'log' | 'warn' | 'error' | 'debug' | 'info') => (...args: unknown[]) => { - if ( - this.params.suppressConsoleOutput && - !this.params.debugMode && - type !== 'error' - ) { - return; - } - if (this.params.stderr) { + if ((type === 'info' || type === 'log') && !this.params.debugMode) { + return; + } if (type !== 'debug' || this.params.debugMode) { this.originalConsoleError(this.formatArgs(args)); }