Skip to content

Commit 8190400

Browse files
authored
Fix issues where escape codes could end up on startup in the input prompt (google-gemini#7267)
1 parent dfd622e commit 8190400

File tree

5 files changed

+168
-51
lines changed

5 files changed

+168
-51
lines changed

packages/cli/src/gemini.test.tsx

Lines changed: 96 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,17 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
87
import {
8+
describe,
9+
it,
10+
expect,
11+
vi,
12+
beforeEach,
13+
afterEach,
14+
type MockInstance,
15+
} from 'vitest';
16+
import {
17+
main,
918
setupUnhandledRejectionHandler,
1019
validateDnsResolutionOrder,
1120
startInteractiveUI,
@@ -33,14 +42,10 @@ vi.mock('./config/settings.js', async (importOriginal) => {
3342

3443
vi.mock('./config/config.js', () => ({
3544
loadCliConfig: vi.fn().mockResolvedValue({
36-
config: {
37-
getSandbox: vi.fn(() => false),
38-
getQuestion: vi.fn(() => ''),
39-
},
40-
modelWasSwitched: false,
41-
originalModelBeforeSwitch: null,
42-
finalModel: 'test-model',
43-
}),
45+
getSandbox: vi.fn(() => false),
46+
getQuestion: vi.fn(() => ''),
47+
} as unknown as Config),
48+
parseArguments: vi.fn().mockResolvedValue({ sessionSummary: null }),
4449
}));
4550

4651
vi.mock('read-package-up', () => ({
@@ -157,6 +162,87 @@ describe('gemini.tsx main function', () => {
157162
});
158163
});
159164

165+
describe('gemini.tsx main function kitty protocol', () => {
166+
let setRawModeSpy: MockInstance<
167+
(mode: boolean) => NodeJS.ReadStream & { fd: 0 }
168+
>;
169+
170+
beforeEach(() => {
171+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
172+
if (!(process.stdin as any).setRawMode) {
173+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
174+
(process.stdin as any).setRawMode = vi.fn();
175+
}
176+
setRawModeSpy = vi.spyOn(process.stdin, 'setRawMode');
177+
});
178+
179+
it('should call setRawMode and detectAndEnableKittyProtocol when isInteractive is true', async () => {
180+
const { detectAndEnableKittyProtocol } = await import(
181+
'./ui/utils/kittyProtocolDetector.js'
182+
);
183+
const { loadCliConfig, parseArguments } = await import(
184+
'./config/config.js'
185+
);
186+
const { loadSettings } = await import('./config/settings.js');
187+
vi.mocked(loadCliConfig).mockResolvedValue({
188+
isInteractive: () => true,
189+
getQuestion: () => '',
190+
getSandbox: () => false,
191+
getDebugMode: () => false,
192+
getListExtensions: () => false,
193+
getMcpServers: () => ({}),
194+
initialize: vi.fn(),
195+
getIdeMode: () => false,
196+
getExperimentalZedIntegration: () => false,
197+
getScreenReader: () => false,
198+
} as unknown as Config);
199+
vi.mocked(loadSettings).mockReturnValue({
200+
errors: [],
201+
merged: {
202+
advanced: {},
203+
security: { auth: {} },
204+
ui: {},
205+
},
206+
setValue: vi.fn(),
207+
} as never);
208+
vi.mocked(parseArguments).mockResolvedValue({
209+
model: undefined,
210+
sandbox: undefined,
211+
sandboxImage: undefined,
212+
debug: undefined,
213+
prompt: undefined,
214+
promptInteractive: undefined,
215+
allFiles: undefined,
216+
showMemoryUsage: undefined,
217+
yolo: undefined,
218+
approvalMode: undefined,
219+
telemetry: undefined,
220+
checkpointing: undefined,
221+
telemetryTarget: undefined,
222+
telemetryOtlpEndpoint: undefined,
223+
telemetryOtlpProtocol: undefined,
224+
telemetryLogPrompts: undefined,
225+
telemetryOutfile: undefined,
226+
allowedMcpServerNames: undefined,
227+
allowedTools: undefined,
228+
experimentalAcp: undefined,
229+
extensions: undefined,
230+
listExtensions: undefined,
231+
proxy: undefined,
232+
includeDirectories: undefined,
233+
screenReader: undefined,
234+
useSmartEdit: undefined,
235+
sessionSummary: undefined,
236+
promptWords: undefined,
237+
});
238+
239+
await main();
240+
241+
expect(setRawModeSpy).toHaveBeenCalledWith(true);
242+
expect(detectAndEnableKittyProtocol).toHaveBeenCalledTimes(1);
243+
});
244+
});
245+
160246
describe('validateDnsResolutionOrder', () => {
161247
let consoleWarnSpy: ReturnType<typeof vi.spyOn>;
162248

@@ -213,7 +299,7 @@ describe('startInteractiveUI', () => {
213299
}));
214300

215301
vi.mock('./ui/utils/kittyProtocolDetector.js', () => ({
216-
detectAndEnableKittyProtocol: vi.fn(() => Promise.resolve()),
302+
detectAndEnableKittyProtocol: vi.fn(() => Promise.resolve(true)),
217303
}));
218304

219305
vi.mock('./ui/utils/updateCheck.js', () => ({
@@ -260,9 +346,6 @@ describe('startInteractiveUI', () => {
260346

261347
it('should perform all startup tasks in correct order', async () => {
262348
const { getCliVersion } = await import('./utils/version.js');
263-
const { detectAndEnableKittyProtocol } = await import(
264-
'./ui/utils/kittyProtocolDetector.js'
265-
);
266349
const { checkForUpdates } = await import('./ui/utils/updateCheck.js');
267350
const { registerCleanup } = await import('./utils/cleanup.js');
268351

@@ -275,7 +358,6 @@ describe('startInteractiveUI', () => {
275358

276359
// Verify all startup tasks were called
277360
expect(getCliVersion).toHaveBeenCalledTimes(1);
278-
expect(detectAndEnableKittyProtocol).toHaveBeenCalledTimes(1);
279361
expect(registerCleanup).toHaveBeenCalledTimes(1);
280362

281363
// Verify cleanup handler is registered with unmount function

packages/cli/src/gemini.tsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,6 @@ export async function startInteractiveUI(
172172
workspaceRoot: string = process.cwd(),
173173
) {
174174
const version = await getCliVersion();
175-
// Detect and enable Kitty keyboard protocol once at startup
176-
await detectAndEnableKittyProtocol();
177175
setWindowTitle(basename(workspaceRoot), settings);
178176
const instance = render(
179177
<React.StrictMode>
@@ -218,6 +216,24 @@ export async function main() {
218216
argv,
219217
);
220218

219+
const wasRaw = process.stdin.isRaw;
220+
let kittyProtocolDetectionComplete: Promise<boolean> | undefined;
221+
if (config.isInteractive() && !wasRaw) {
222+
// Set this as early as possible to avoid spurious characters from
223+
// input showing up in the output.
224+
process.stdin.setRawMode(true);
225+
226+
// This cleanup isn't strictly needed but may help in certain situations.
227+
process.on('SIGTERM', () => {
228+
process.stdin.setRawMode(wasRaw);
229+
});
230+
process.on('SIGINT', () => {
231+
process.stdin.setRawMode(wasRaw);
232+
});
233+
234+
// Detect and enable Kitty keyboard protocol once at startup.
235+
kittyProtocolDetectionComplete = detectAndEnableKittyProtocol();
236+
}
221237
if (argv.sessionSummary) {
222238
registerCleanup(() => {
223239
const metrics = uiTelemetryService.getMetrics();
@@ -385,6 +401,8 @@ export async function main() {
385401

386402
// Render UI, passing necessary config values. Check that there is no command line question.
387403
if (config.isInteractive()) {
404+
// Need kitty detection to be complete before we can start the interactive UI.
405+
await kittyProtocolDetectionComplete;
388406
await startInteractiveUI(config, settings, startupWarnings);
389407
return;
390408
}

packages/cli/src/ui/contexts/KeypressContext.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,10 @@ export function KeypressProvider({
115115
}
116116
};
117117

118-
setRawMode(true);
118+
const wasRaw = stdin.isRaw;
119+
if (wasRaw === false) {
120+
setRawMode(true);
121+
}
119122

120123
const keypressStream = new PassThrough();
121124
let usePassthrough = false;
@@ -677,7 +680,9 @@ export function KeypressProvider({
677680
rl.close();
678681

679682
// Restore the terminal to its original state.
680-
setRawMode(false);
683+
if (wasRaw === false) {
684+
setRawMode(false);
685+
}
681686

682687
if (backslashTimeout) {
683688
clearTimeout(backslashTimeout);

packages/cli/src/ui/hooks/useKeypress.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ vi.mock('readline', () => {
5454

5555
class MockStdin extends EventEmitter {
5656
isTTY = true;
57+
isRaw = false;
5758
setRawMode = vi.fn();
5859
on = this.addListener;
5960
removeListener = this.removeListener;

packages/cli/src/ui/utils/kittyProtocolDetector.ts

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -32,40 +32,58 @@ export async function detectAndEnableKittyProtocol(): Promise<boolean> {
3232

3333
let responseBuffer = '';
3434
let progressiveEnhancementReceived = false;
35-
let checkFinished = false;
35+
let timeoutId: NodeJS.Timeout | undefined;
36+
37+
const onTimeout = () => {
38+
timeoutId = undefined;
39+
process.stdin.removeListener('data', handleData);
40+
if (!originalRawMode) {
41+
process.stdin.setRawMode(false);
42+
}
43+
detectionComplete = true;
44+
resolve(false);
45+
};
3646

3747
const handleData = (data: Buffer) => {
48+
if (timeoutId === undefined) {
49+
// Race condition. We have already timed out.
50+
return;
51+
}
3852
responseBuffer += data.toString();
3953

4054
// Check for progressive enhancement response (CSI ? <flags> u)
4155
if (responseBuffer.includes('\x1b[?') && responseBuffer.includes('u')) {
4256
progressiveEnhancementReceived = true;
57+
// Give more time to get the full set of kitty responses if we have an
58+
// indication the terminal probably supports kitty and we just need to
59+
// wait a bit longer for a response.
60+
clearTimeout(timeoutId);
61+
timeoutId = setTimeout(onTimeout, 1000);
4362
}
4463

4564
// Check for device attributes response (CSI ? <attrs> c)
4665
if (responseBuffer.includes('\x1b[?') && responseBuffer.includes('c')) {
47-
if (!checkFinished) {
48-
checkFinished = true;
49-
process.stdin.removeListener('data', handleData);
50-
51-
if (!originalRawMode) {
52-
process.stdin.setRawMode(false);
53-
}
54-
55-
if (progressiveEnhancementReceived) {
56-
// Enable the protocol
57-
process.stdout.write('\x1b[>1u');
58-
protocolSupported = true;
59-
protocolEnabled = true;
60-
61-
// Set up cleanup on exit
62-
process.on('exit', disableProtocol);
63-
process.on('SIGTERM', disableProtocol);
64-
}
65-
66-
detectionComplete = true;
67-
resolve(protocolSupported);
66+
clearTimeout(timeoutId);
67+
timeoutId = undefined;
68+
process.stdin.removeListener('data', handleData);
69+
70+
if (!originalRawMode) {
71+
process.stdin.setRawMode(false);
6872
}
73+
74+
if (progressiveEnhancementReceived) {
75+
// Enable the protocol
76+
process.stdout.write('\x1b[>1u');
77+
protocolSupported = true;
78+
protocolEnabled = true;
79+
80+
// Set up cleanup on exit
81+
process.on('exit', disableProtocol);
82+
process.on('SIGTERM', disableProtocol);
83+
}
84+
85+
detectionComplete = true;
86+
resolve(protocolSupported);
6987
}
7088
};
7189

@@ -75,17 +93,10 @@ export async function detectAndEnableKittyProtocol(): Promise<boolean> {
7593
process.stdout.write('\x1b[?u'); // Query progressive enhancement
7694
process.stdout.write('\x1b[c'); // Query device attributes
7795

78-
// Timeout after 50ms
79-
setTimeout(() => {
80-
if (!checkFinished) {
81-
process.stdin.removeListener('data', handleData);
82-
if (!originalRawMode) {
83-
process.stdin.setRawMode(false);
84-
}
85-
detectionComplete = true;
86-
resolve(false);
87-
}
88-
}, 50);
96+
// Timeout after 200ms
97+
// When a iterm2 terminal does not have focus this can take over 90s on a
98+
// fast macbook so we need a somewhat longer threshold than would be ideal.
99+
timeoutId = setTimeout(onTimeout, 200);
89100
});
90101
}
91102

0 commit comments

Comments
 (0)