Skip to content

Commit 8c1e255

Browse files
authored
fix(cli): prevent informational logs from polluting json output (google-gemini#26264)
1 parent 0af1314 commit 8c1e255

4 files changed

Lines changed: 217 additions & 22 deletions

File tree

packages/cli/src/gemini.tsx

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
type OutputPayload,
1414
type ConsoleLogPayload,
1515
type UserFeedbackPayload,
16+
type CoreEvents,
1617
createSessionId,
1718
logUserPrompt,
1819
AuthType,
@@ -265,6 +266,7 @@ export async function startInteractiveUI(
265266
}
266267

267268
export async function main() {
269+
let config: Config | undefined;
268270
const cliStartupHandle = startupProfiler.start('cli_startup');
269271

270272
// Listen for admin controls from parent process (IPC) in non-sandbox mode. In
@@ -277,7 +279,7 @@ export async function main() {
277279
const cleanupStdio = patchStdio();
278280
registerSyncCleanup(() => {
279281
// This is needed to ensure we don't lose any buffered output.
280-
initializeOutputListenersAndFlush();
282+
initializeOutputListenersAndFlush(config);
281283
cleanupStdio();
282284
});
283285

@@ -539,7 +541,7 @@ export async function main() {
539541
// may have side effects.
540542
{
541543
const loadConfigHandle = startupProfiler.start('load_cli_config');
542-
const config = await loadCliConfig(settings.merged, sessionId, argv, {
544+
config = await loadCliConfig(settings.merged, sessionId, argv, {
543545
projectHooks: settings.workspace.settings.hooks,
544546
worktreeSettings: worktreeInfo,
545547
});
@@ -785,7 +787,7 @@ export async function main() {
785787
debugLogger.log('Session ID: %s', sessionId);
786788
}
787789

788-
initializeOutputListenersAndFlush();
790+
initializeOutputListenersAndFlush(config);
789791

790792
await runNonInteractive({
791793
config,
@@ -800,7 +802,7 @@ export async function main() {
800802
}
801803
}
802804

803-
export function initializeOutputListenersAndFlush() {
805+
export function initializeOutputListenersAndFlush(config?: Config) {
804806
// If there are no listeners for output, make sure we flush so output is not
805807
// lost.
806808
if (coreEvents.listenerCount(CoreEvent.Output) === 0) {
@@ -812,24 +814,43 @@ export function initializeOutputListenersAndFlush() {
812814
writeToStdout(payload.chunk, payload.encoding);
813815
}
814816
});
817+
}
815818

816-
if (coreEvents.listenerCount(CoreEvent.ConsoleLog) === 0) {
817-
coreEvents.on(CoreEvent.ConsoleLog, (payload: ConsoleLogPayload) => {
818-
if (payload.type === 'error' || payload.type === 'warn') {
819-
writeToStderr(payload.content + '\n');
820-
} else {
821-
writeToStderr(payload.content + '\n');
822-
}
823-
});
824-
}
819+
if (coreEvents.listenerCount(CoreEvent.ConsoleLog) === 0) {
820+
coreEvents.on(CoreEvent.ConsoleLog, (payload: ConsoleLogPayload) => {
821+
if (payload.type === 'error' || payload.type === 'warn') {
822+
writeToStderr(payload.content + '\n');
823+
} else {
824+
writeToStderr(payload.content + '\n');
825+
}
826+
});
827+
}
825828

826-
if (coreEvents.listenerCount(CoreEvent.UserFeedback) === 0) {
827-
coreEvents.on(CoreEvent.UserFeedback, (payload: UserFeedbackPayload) => {
828-
writeToStderr(payload.message + '\n');
829-
});
830-
}
829+
if (coreEvents.listenerCount(CoreEvent.UserFeedback) === 0) {
830+
coreEvents.on(CoreEvent.UserFeedback, (payload: UserFeedbackPayload) => {
831+
writeToStderr(payload.message + '\n');
832+
});
831833
}
832-
coreEvents.drainBacklogs();
834+
835+
const outputFormat = config?.getOutputFormat();
836+
const forceToStderr = outputFormat === 'json' || config === undefined;
837+
838+
coreEvents.drainBacklogs(
839+
<K extends keyof CoreEvents>(event: K, args: CoreEvents[K]) => {
840+
if (forceToStderr && event === (CoreEvent.Output as string)) {
841+
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
842+
const payload = args[0] as OutputPayload;
843+
if (!payload.isStderr) {
844+
return {
845+
event,
846+
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
847+
args: [{ ...payload, isStderr: true }] as unknown as CoreEvents[K],
848+
};
849+
}
850+
}
851+
return { event, args };
852+
},
853+
);
833854
}
834855

835856
function setupAdminControlsListener() {
Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/**
2+
* @license
3+
* Copyright 2026 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
8+
import { initializeOutputListenersAndFlush } from './gemini.js';
9+
import { coreEvents, CoreEvent, type Config } from '@google/gemini-cli-core';
10+
11+
// Mock core dependencies
12+
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
13+
const actual =
14+
await importOriginal<typeof import('@google/gemini-cli-core')>();
15+
return {
16+
...actual,
17+
writeToStdout: vi.fn(),
18+
writeToStderr: vi.fn(),
19+
};
20+
});
21+
22+
import { writeToStdout, writeToStderr } from '@google/gemini-cli-core';
23+
24+
describe('Output Redirection', () => {
25+
beforeEach(() => {
26+
vi.clearAllMocks();
27+
// Clear listeners to simulate a clean state for each test
28+
coreEvents.removeAllListeners();
29+
});
30+
31+
afterEach(() => {
32+
coreEvents.removeAllListeners();
33+
});
34+
35+
it('should redirect buffered stdout to stderr when output format is json', () => {
36+
const mockConfig = {
37+
getOutputFormat: () => 'json',
38+
} as unknown as Config;
39+
40+
// Simulate buffered output
41+
coreEvents.emitOutput(false, 'informational message');
42+
coreEvents.emitOutput(true, 'error message');
43+
44+
// Initialize listeners and flush
45+
initializeOutputListenersAndFlush(mockConfig);
46+
47+
// Verify informational message was forced to stderr
48+
expect(writeToStderr).toHaveBeenCalledWith(
49+
'informational message',
50+
undefined,
51+
);
52+
expect(writeToStderr).toHaveBeenCalledWith('error message', undefined);
53+
expect(writeToStdout).not.toHaveBeenCalled();
54+
});
55+
56+
it('should NOT redirect buffered stdout to stderr when output format is NOT json', () => {
57+
const mockConfig = {
58+
getOutputFormat: () => 'text',
59+
} as unknown as Config;
60+
61+
// Simulate buffered output
62+
coreEvents.emitOutput(false, 'regular message');
63+
64+
// Initialize listeners and flush
65+
initializeOutputListenersAndFlush(mockConfig);
66+
67+
// Verify regular message went to stdout
68+
expect(writeToStdout).toHaveBeenCalledWith('regular message', undefined);
69+
expect(writeToStderr).not.toHaveBeenCalled();
70+
});
71+
72+
it('should force stdout to stderr when config is undefined (early failure)', () => {
73+
// Simulate buffered output during early init
74+
coreEvents.emitOutput(false, 'early init message');
75+
76+
// Initialize with undefined config
77+
initializeOutputListenersAndFlush(undefined);
78+
79+
// Verify it was forced to stderr
80+
expect(writeToStderr).toHaveBeenCalledWith('early init message', undefined);
81+
expect(writeToStdout).not.toHaveBeenCalled();
82+
});
83+
84+
it('should attach ConsoleLog and UserFeedback listeners even if Output already has one', () => {
85+
// Manually attach an Output listener
86+
coreEvents.on(CoreEvent.Output, vi.fn());
87+
88+
// Initialize - should still attach ConsoleLog and UserFeedback defaults
89+
initializeOutputListenersAndFlush(undefined);
90+
91+
// Simulate events
92+
coreEvents.emitConsoleLog('info', 'stray log');
93+
coreEvents.emitFeedback('info', 'stray feedback');
94+
95+
// Draining happens inside initializeOutputListenersAndFlush for existing backlog,
96+
// but here we check the newly attached listeners for immediate emission.
97+
expect(writeToStderr).toHaveBeenCalledWith('stray log\n');
98+
expect(writeToStderr).toHaveBeenCalledWith('stray feedback\n');
99+
});
100+
});

packages/core/src/utils/events.test.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
CoreEventEmitter,
1010
CoreEvent,
1111
coreEvents,
12+
type CoreEvents,
1213
type UserFeedbackPayload,
1314
type McpProgressPayload,
1415
} from './events.js';
@@ -268,6 +269,61 @@ describe('CoreEventEmitter', () => {
268269
});
269270
});
270271

272+
describe('drainBacklogs Transformation', () => {
273+
it('should transform events during drain', () => {
274+
const listener = vi.fn();
275+
events.emitOutput(false, 'stdout chunk');
276+
events.emitFeedback('info', 'info message');
277+
278+
events.on(CoreEvent.Output, listener);
279+
events.on(CoreEvent.UserFeedback, listener);
280+
281+
events.drainBacklogs(
282+
<K extends keyof CoreEvents>(event: K, args: CoreEvents[K]) => {
283+
if (event === (CoreEvent.Output as string)) {
284+
const payload = args[0] as { isStderr: boolean; chunk: string };
285+
return {
286+
event,
287+
args: [
288+
{ ...payload, isStderr: true },
289+
] as unknown as CoreEvents[K],
290+
};
291+
}
292+
return { event, args };
293+
},
294+
);
295+
296+
expect(listener).toHaveBeenCalledTimes(2);
297+
expect(listener).toHaveBeenCalledWith(
298+
expect.objectContaining({ isStderr: true, chunk: 'stdout chunk' }),
299+
);
300+
expect(listener).toHaveBeenCalledWith(
301+
expect.objectContaining({ message: 'info message' }),
302+
);
303+
});
304+
305+
it('should drop events when transform returns undefined', () => {
306+
const listener = vi.fn();
307+
events.emitOutput(false, 'drop me');
308+
events.emitFeedback('info', 'keep me');
309+
310+
events.on(CoreEvent.Output, listener);
311+
events.on(CoreEvent.UserFeedback, listener);
312+
313+
events.drainBacklogs((event, args) => {
314+
if (event === CoreEvent.Output) {
315+
return undefined;
316+
}
317+
return { event, args };
318+
});
319+
320+
expect(listener).toHaveBeenCalledTimes(1);
321+
expect(listener).toHaveBeenCalledWith(
322+
expect.objectContaining({ message: 'keep me' }),
323+
);
324+
});
325+
});
326+
271327
describe('ModelChanged Event', () => {
272328
it('should emit ModelChanged event with correct payload', () => {
273329
const listener = vi.fn();

packages/core/src/utils/events.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -422,19 +422,37 @@ export class CoreEventEmitter extends EventEmitter<CoreEvents> {
422422
/**
423423
* Flushes buffered messages. Call this immediately after primary UI listener
424424
* subscribes.
425+
*
426+
* @param transform - Optional function to transform events before they are emitted.
425427
*/
426-
drainBacklogs(): void {
428+
drainBacklogs(
429+
transform?: <K extends keyof CoreEvents>(
430+
event: K,
431+
args: CoreEvents[K],
432+
) => { event: K; args: CoreEvents[K] } | undefined,
433+
): void {
427434
const backlog = this._eventBacklog;
428435
const head = this._backlogHead;
429436
this._eventBacklog = [];
430437
this._backlogHead = 0;
431438
for (let i = head; i < backlog.length; i++) {
432439
const item = backlog[i];
433440
if (item === undefined) continue;
441+
442+
let eventToEmit = item.event;
443+
let argsToEmit = item.args;
444+
445+
if (transform) {
446+
const transformed = transform(item.event, item.args);
447+
if (!transformed) continue;
448+
eventToEmit = transformed.event;
449+
argsToEmit = transformed.args;
450+
}
451+
434452
// eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
435453
(this.emit as (event: keyof CoreEvents, ...args: unknown[]) => boolean)(
436-
item.event,
437-
...item.args,
454+
eventToEmit,
455+
...argsToEmit,
438456
);
439457
}
440458
}

0 commit comments

Comments
 (0)