MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-04-29 06:25:16 -07:00
Code Issues Packages Projects Releases Wiki Activity

Protect stdout and stderr so JavaScript code can't accidentally write to stdout corrupting ink rendering (#13247)

Browse Source 
Bypassing rules as link checker failure is spurious.
This commit is contained in:
Jacob Richman
2025-11-20 10:44:02 -08:00
committed by GitHub
parent e20d282088
commit d1e35f8660
82 changed files with 1523 additions and 868 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/ui/AppContainer.test.tsx
+72 -25
View File
@@ -33,7 +33,7 @@ import {
const mockCoreEvents = vi.hoisted(() => ({
on: vi.fn(),
off: vi.fn(),
drainFeedbackBacklog: vi.fn(),
drainBacklogs: vi.fn(),
emit: vi.fn(),
}));
@@ -42,6 +42,11 @@ const mockIdeClient = vi.hoisted(() => ({
getInstance: vi.fn().mockReturnValue(new Promise(() => {})),
}));
// Mock stdout
const mocks = vi.hoisted(() => ({
mockStdout: { write: vi.fn() },
}));
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@google/gemini-cli-core')>();
@@ -61,12 +66,11 @@ import {
} from './contexts/UIActionsContext.js';
// Mock useStdout to capture terminal title writes
let mockStdout: { write: ReturnType<typeof vi.fn> };
vi.mock('ink', async (importOriginal) => {
const actual = await importOriginal<typeof import('ink')>();
return {
...actual,
useStdout: () => ({ stdout: mockStdout }),
useStdout: () => ({ stdout: mocks.mockStdout }),
measureElement: vi.fn(),
};
});
@@ -122,6 +126,19 @@ vi.mock('./utils/mouse.js', () => ({
enableMouseEvents: vi.fn(),
disableMouseEvents: vi.fn(),
}));
vi.mock('../utils/stdio.js', () => ({
writeToStdout: vi.fn((...args) =>
process.stdout.write(...(args as Parameters<typeof process.stdout.write>)),
),
writeToStderr: vi.fn((...args) =>
process.stderr.write(...(args as Parameters<typeof process.stderr.write>)),
),
patchStdio: vi.fn(() => () => {}),
createInkStdio: vi.fn(() => ({
stdout: process.stdout,
stderr: process.stderr,
})),
}));
import { useHistory } from './hooks/useHistoryManager.js';
import { useThemeCommand } from './hooks/useThemeCommand.js';
@@ -149,6 +166,7 @@ import { useTerminalSize } from './hooks/useTerminalSize.js';
import { ShellExecutionService } from '@google/gemini-cli-core';
import { type ExtensionManager } from '../config/extension-manager.js';
import { enableMouseEvents, disableMouseEvents } from './utils/mouse.js';
import { writeToStdout } from '../utils/stdio.js';
describe('AppContainer State Management', () => {
let mockConfig: Config;
@@ -215,7 +233,7 @@ describe('AppContainer State Management', () => {
vi.clearAllMocks();
// Initialize mock stdout for terminal title tests
mockStdout = { write: vi.fn() };
mocks.mockStdout.write.mockClear();
// Mock computeWindowTitle function to centralize title logic testing
vi.mock('../utils/windowTitle.js', async () => ({
@@ -886,7 +904,13 @@ describe('AppContainer State Management', () => {
describe('Terminal Title Update Feature', () => {
beforeEach(() => {
// Reset mock stdout for each test
mockStdout = { write: vi.fn() };
mocks.mockStdout.write.mockClear();
});
it('verifies useStdout is mocked', async () => {
const { useStdout } = await import('ink');
const { stdout } = useStdout();
expect(stdout).toBe(mocks.mockStdout);
});
it('should not update terminal title when showStatusInTitle is false', () => {
@@ -909,9 +933,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that no title-related writes occurred
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(0);
unmount();
});
@@ -936,9 +961,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that no title-related writes occurred
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(0);
unmount();
});
@@ -974,9 +1000,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that title was updated with thought subject
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
expect(titleWrites[0][0]).toBe(
`\x1b]2;${thoughtSubject.padEnd(80, ' ')}\x07`,
@@ -1014,9 +1041,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that title was updated with default Idle text
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
expect(titleWrites[0][0]).toBe(
`\x1b]2;${'Gemini - workspace'.padEnd(80, ' ')}\x07`,
@@ -1055,9 +1083,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that title was updated with confirmation text
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
expect(titleWrites[0][0]).toBe(
`\x1b]2;${thoughtSubject.padEnd(80, ' ')}\x07`,
@@ -1096,9 +1125,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that title is padded to exactly 80 characters
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
const calledWith = titleWrites[0][0];
const expectedTitle = shortTitle.padEnd(80, ' ');
@@ -1141,9 +1171,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that the correct ANSI escape sequence is used
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
const expectedEscapeSequence = `\x1b]2;${title.padEnd(80, ' ')}\x07`;
expect(titleWrites[0][0]).toBe(expectedEscapeSequence);
@@ -1183,9 +1214,10 @@ describe('AppContainer State Management', () => {
});
// Assert: Check that title was updated with CLI_TITLE value
const titleWrites = mockStdout.write.mock.calls.filter((call) =>
const titleWrites = mocks.mockStdout.write.mock.calls.filter((call) =>
call[0].includes('\x1b]2;'),
);
expect(titleWrites).toHaveLength(1);
expect(titleWrites[0][0]).toBe(
`\x1b]2;${'Custom Gemini Title'.padEnd(80, ' ')}\x07`,
@@ -1493,7 +1525,7 @@ describe('AppContainer State Management', () => {
};
beforeEach(() => {
mockStdout.write.mockClear();
mocks.mockStdout.write.mockClear();
mockedUseKeypress.mockImplementation((callback: (key: Key) => void) => {
handleGlobalKeypress = callback;
});
@@ -1518,7 +1550,7 @@ describe('AppContainer State Management', () => {
])('$modeName', ({ isAlternateMode, shouldEnable }) => {
it(`should ${shouldEnable ? 'toggle' : 'NOT toggle'} mouse off when Ctrl+S is pressed`, async () => {
await setupCopyModeTest(isAlternateMode);
mockStdout.write.mockClear(); // Clear initial enable call
mocks.mockStdout.write.mockClear(); // Clear initial enable call
act(() => {
handleGlobalKeypress({
@@ -1544,7 +1576,7 @@ describe('AppContainer State Management', () => {
if (shouldEnable) {
it('should toggle mouse back on when Ctrl+S is pressed again', async () => {
await setupCopyModeTest(isAlternateMode);
mockStdout.write.mockClear();
(writeToStdout as Mock).mockClear();
// Turn it on (disable mouse)
act(() => {
@@ -1596,7 +1628,7 @@ describe('AppContainer State Management', () => {
});
rerender();
mockStdout.write.mockClear();
(writeToStdout as Mock).mockClear();
// Press any other key
act(() => {
@@ -1665,7 +1697,7 @@ describe('AppContainer State Management', () => {
CoreEvent.UserFeedback,
expect.any(Function),
);
expect(mockCoreEvents.drainFeedbackBacklog).toHaveBeenCalledTimes(1);
expect(mockCoreEvents.drainBacklogs).toHaveBeenCalledTimes(1);
unmount();
});
@@ -1782,7 +1814,7 @@ describe('AppContainer State Management', () => {
// Helper to extract arguments from the useGeminiStream hook call
// This isolates the positional argument dependency to a single location
const extractUseGeminiStreamArgs = (args: unknown[]) => ({
onCancelSubmit: args[14] as (shouldRestorePrompt?: boolean) => void,
onCancelSubmit: args[13] as (shouldRestorePrompt?: boolean) => void,
});
beforeEach(() => {
@@ -1846,9 +1878,19 @@ describe('AppContainer State Management', () => {
loadHistory: vi.fn(),
});
// Mock logger to resolve so userMessages gets populated
let resolveLoggerPromise: (val: string[]) => void;
const loggerPromise = new Promise<string[]>((resolve) => {
resolveLoggerPromise = resolve;
});
// Mock logger to control when userMessages updates
const getPreviousUserMessagesMock = vi
.fn()
.mockResolvedValueOnce([]) // Initial mount
.mockReturnValueOnce(loggerPromise); // Second render (simulated update)
mockedUseLogger.mockReturnValue({
getPreviousUserMessages: vi.fn().mockResolvedValue([]),
getPreviousUserMessages: getPreviousUserMessagesMock,
});
const { unmount, rerender } = renderAppContainer();
@@ -1871,20 +1913,25 @@ describe('AppContainer State Management', () => {
});
// Rerender to reflect the history change.
// This triggers the effect to update userMessages, but it's async.
// This triggers the effect to update userMessages, but it hangs on loggerPromise.
rerender(getAppContainer());
const { onCancelSubmit } = extractUseGeminiStreamArgs(
mockedUseGeminiStream.mock.lastCall!,
);
// Call onCancelSubmit immediately (simulating the race condition where
// the overflow event comes in before the effect updates userMessages)
// Call onCancelSubmit immediately. userMessages is still stale (has only 'Previous Prompt')
// because the effect is waiting on loggerPromise.
act(() => {
onCancelSubmit(true);
});
// With the fix, it should wait for userMessages to update and then set the new prompt
// Now resolve the promise to let the effect complete and update userMessages
await act(async () => {
resolveLoggerPromise!([]);
});
// With the fix, it should have waited for userMessages to update and then set the new prompt
await waitFor(() => {
expect(mockSetText).toHaveBeenCalledWith(newPrompt);
});