Skip to content

Commit d91ca38

Browse files
authored
Fix multiple bugs with auth flow including using the implemented but unused restart support. (#13565)
1 parent 215bd2a commit d91ca38

File tree

13 files changed

+306
-136
lines changed

13 files changed

+306
-136
lines changed

packages/cli/index.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,23 +8,26 @@
88

99
import './src/gemini.js';
1010
import { main } from './src/gemini.js';
11-
import { debugLogger, FatalError } from '@google/gemini-cli-core';
11+
import { FatalError, writeToStderr } from '@google/gemini-cli-core';
12+
import { runExitCleanup } from './src/utils/cleanup.js';
1213

1314
// --- Global Entry Point ---
14-
main().catch((error) => {
15+
main().catch(async (error) => {
16+
await runExitCleanup();
17+
1518
if (error instanceof FatalError) {
1619
let errorMessage = error.message;
1720
if (!process.env['NO_COLOR']) {
1821
errorMessage = `\x1b[31m${errorMessage}\x1b[0m`;
1922
}
20-
debugLogger.error(errorMessage);
23+
writeToStderr(errorMessage + '\n');
2124
process.exit(error.exitCode);
2225
}
23-
debugLogger.error('An unexpected critical error occurred:');
26+
writeToStderr('An unexpected critical error occurred:');
2427
if (error instanceof Error) {
25-
debugLogger.error(error.stack);
28+
writeToStderr(error.stack + '\n');
2629
} else {
27-
debugLogger.error(String(error));
30+
writeToStderr(String(error) + '\n');
2831
}
2932
process.exit(1);
3033
});

packages/cli/src/gemini.tsx

Lines changed: 21 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,11 @@ import {
3333
runExitCleanup,
3434
} from './utils/cleanup.js';
3535
import { getCliVersion } from './utils/version.js';
36-
import type {
37-
Config,
38-
ResumedSessionData,
39-
OutputPayload,
40-
ConsoleLogPayload,
41-
} from '@google/gemini-cli-core';
4236
import {
37+
type Config,
38+
type ResumedSessionData,
39+
type OutputPayload,
40+
type ConsoleLogPayload,
4341
sessionId,
4442
logUserPrompt,
4543
AuthType,
@@ -53,6 +51,11 @@ import {
5351
patchStdio,
5452
writeToStdout,
5553
writeToStderr,
54+
disableMouseEvents,
55+
enableMouseEvents,
56+
enterAlternateScreen,
57+
disableLineWrapping,
58+
shouldEnterAlternateScreen,
5659
} from '@google/gemini-cli-core';
5760
import {
5861
initializeApp,
@@ -85,9 +88,7 @@ import { deleteSession, listSessions } from './utils/sessions.js';
8588
import { ExtensionManager } from './config/extension-manager.js';
8689
import { createPolicyUpdater } from './config/policy.js';
8790
import { requestConsentNonInteractive } from './config/extensions/consent.js';
88-
import { disableMouseEvents, enableMouseEvents } from './ui/utils/mouse.js';
8991
import { ScrollProvider } from './ui/contexts/ScrollProvider.js';
90-
import ansiEscapes from 'ansi-escapes';
9192
import { isAlternateBufferEnabled } from './ui/hooks/useAlternateBuffer.js';
9293

9394
import { profiler } from './ui/components/DebugProfiler.js';
@@ -176,8 +177,10 @@ export async function startInteractiveUI(
176177
// as there is no benefit of alternate buffer mode when using a screen reader
177178
// and the Ink alternate buffer mode requires line wrapping harmful to
178179
// screen readers.
179-
const useAlternateBuffer =
180-
isAlternateBufferEnabled(settings) && !config.getScreenReader();
180+
const useAlternateBuffer = shouldEnterAlternateScreen(
181+
isAlternateBufferEnabled(settings),
182+
config.getScreenReader(),
183+
);
181184
const mouseEventsEnabled = useAlternateBuffer;
182185
if (mouseEventsEnabled) {
183186
enableMouseEvents();
@@ -481,8 +484,14 @@ export async function main() {
481484
// input showing up in the output.
482485
process.stdin.setRawMode(true);
483486

484-
if (isAlternateBufferEnabled(settings)) {
485-
writeToStdout(ansiEscapes.enterAlternativeScreen);
487+
if (
488+
shouldEnterAlternateScreen(
489+
isAlternateBufferEnabled(settings),
490+
config.getScreenReader(),
491+
)
492+
) {
493+
enterAlternateScreen();
494+
disableLineWrapping();
486495

487496
// Ink will cleanup so there is no need for us to manually cleanup.
488497
}

packages/cli/src/ui/AppContainer.test.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
6969
stdout: process.stdout,
7070
stderr: process.stderr,
7171
})),
72+
enableMouseEvents: vi.fn(),
73+
disableMouseEvents: vi.fn(),
7274
};
7375
});
7476
import type { LoadedSettings } from '../config/settings.js';
@@ -137,10 +139,6 @@ vi.mock('../utils/events.js');
137139
vi.mock('../utils/handleAutoUpdate.js');
138140
vi.mock('./utils/ConsolePatcher.js');
139141
vi.mock('../utils/cleanup.js');
140-
vi.mock('./utils/mouse.js', () => ({
141-
enableMouseEvents: vi.fn(),
142-
disableMouseEvents: vi.fn(),
143-
}));
144142

145143
import { useHistory } from './hooks/useHistoryManager.js';
146144
import { useThemeCommand } from './hooks/useThemeCommand.js';
@@ -165,9 +163,13 @@ import { useLoadingIndicator } from './hooks/useLoadingIndicator.js';
165163
import { useKeypress, type Key } from './hooks/useKeypress.js';
166164
import { measureElement } from 'ink';
167165
import { useTerminalSize } from './hooks/useTerminalSize.js';
168-
import { ShellExecutionService, writeToStdout } from '@google/gemini-cli-core';
166+
import {
167+
ShellExecutionService,
168+
writeToStdout,
169+
enableMouseEvents,
170+
disableMouseEvents,
171+
} from '@google/gemini-cli-core';
169172
import { type ExtensionManager } from '../config/extension-manager.js';
170-
import { enableMouseEvents, disableMouseEvents } from './utils/mouse.js';
171173

172174
describe('AppContainer State Management', () => {
173175
let mockConfig: Config;

packages/cli/src/ui/AppContainer.tsx

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ import {
5252
refreshServerHierarchicalMemory,
5353
type ModelChangedPayload,
5454
type MemoryChangedPayload,
55+
writeToStdout,
56+
disableMouseEvents,
57+
enterAlternateScreen,
58+
enableMouseEvents,
59+
disableLineWrapping,
60+
shouldEnterAlternateScreen,
5561
} from '@google/gemini-cli-core';
5662
import { validateAuthMethod } from '../config/auth.js';
5763
import process from 'node:process';
@@ -92,6 +98,7 @@ import { appEvents, AppEvent } from '../utils/events.js';
9298
import { type UpdateObject } from './utils/updateCheck.js';
9399
import { setUpdateHandler } from '../utils/handleAutoUpdate.js';
94100
import { registerCleanup, runExitCleanup } from '../utils/cleanup.js';
101+
import { RELAUNCH_EXIT_CODE } from '../utils/processUtils.js';
95102
import { useMessageQueue } from './hooks/useMessageQueue.js';
96103
import { useAutoAcceptIndicator } from './hooks/useAutoAcceptIndicator.js';
97104
import { useSessionStats } from './contexts/SessionContext.js';
@@ -106,11 +113,9 @@ import { type ExtensionManager } from '../config/extension-manager.js';
106113
import { requestConsentInteractive } from '../config/extensions/consent.js';
107114
import { useIncludeDirsTrust } from './hooks/useIncludeDirsTrust.js';
108115
import { isWorkspaceTrusted } from '../config/trustedFolders.js';
109-
import { disableMouseEvents, enableMouseEvents } from './utils/mouse.js';
110116
import { useAlternateBuffer } from './hooks/useAlternateBuffer.js';
111117
import { useSettings } from './contexts/SettingsContext.js';
112118
import { enableSupportedProtocol } from './utils/kittyProtocolDetector.js';
113-
import { writeToStdout } from '@google/gemini-cli-core';
114119

115120
const WARNING_PROMPT_DURATION_MS = 1000;
116121
const QUEUE_ERROR_DISPLAY_DURATION_MS = 3000;
@@ -372,16 +377,19 @@ export const AppContainer = (props: AppContainerProps) => {
372377
setHistoryRemountKey((prev) => prev + 1);
373378
}, [setHistoryRemountKey, isAlternateBuffer, stdout]);
374379
const handleEditorClose = useCallback(() => {
375-
if (isAlternateBuffer) {
380+
if (
381+
shouldEnterAlternateScreen(isAlternateBuffer, config.getScreenReader())
382+
) {
376383
// The editor may have exited alternate buffer mode so we need to
377384
// enter it again to be safe.
378-
writeToStdout(ansiEscapes.enterAlternativeScreen);
385+
enterAlternateScreen();
379386
enableMouseEvents();
387+
disableLineWrapping();
380388
app.rerender();
381389
}
382390
enableSupportedProtocol();
383391
refreshStatic();
384-
}, [refreshStatic, isAlternateBuffer, app]);
392+
}, [refreshStatic, isAlternateBuffer, app, config]);
385393

386394
useEffect(() => {
387395
coreEvents.on(CoreEvent.ExternalEditorClosed, handleEditorClose);
@@ -458,12 +466,12 @@ export const AppContainer = (props: AppContainerProps) => {
458466
config.isBrowserLaunchSuppressed()
459467
) {
460468
await runExitCleanup();
461-
debugLogger.log(`
469+
writeToStdout(`
462470
----------------------------------------------------------------
463-
Logging in with Google... Please restart Gemini CLI to continue.
471+
Logging in with Google... Restarting Gemini CLI to continue.
464472
----------------------------------------------------------------
465473
`);
466-
process.exit(0);
474+
process.exit(RELAUNCH_EXIT_CODE);
467475
}
468476
}
469477
setAuthState(AuthState.Authenticated);

packages/cli/src/ui/auth/AuthDialog.test.tsx

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { validateAuthMethodWithSettings } from './useAuth.js';
2525
import { runExitCleanup } from '../../utils/cleanup.js';
2626
import { clearCachedCredentialFile } from '@google/gemini-cli-core';
2727
import { Text } from 'ink';
28+
import { RELAUNCH_EXIT_CODE } from '../../utils/processUtils.js';
2829

2930
// Mocks
3031
vi.mock('@google/gemini-cli-core', async (importOriginal) => {
@@ -229,6 +230,7 @@ describe('AuthDialog', () => {
229230
});
230231

231232
it('exits process for Login with Google when browser is suppressed', async () => {
233+
vi.useFakeTimers();
232234
const exitSpy = vi
233235
.spyOn(process, 'exit')
234236
.mockImplementation(() => undefined as never);
@@ -241,14 +243,14 @@ describe('AuthDialog', () => {
241243
mockedRadioButtonSelect.mock.calls[0][0];
242244
await handleAuthSelect(AuthType.LOGIN_WITH_GOOGLE);
243245

246+
await vi.runAllTimersAsync();
247+
244248
expect(mockedRunExitCleanup).toHaveBeenCalled();
245-
expect(logSpy).toHaveBeenCalledWith(
246-
expect.stringContaining('Please restart Gemini CLI'),
247-
);
248-
expect(exitSpy).toHaveBeenCalledWith(0);
249+
expect(exitSpy).toHaveBeenCalledWith(RELAUNCH_EXIT_CODE);
249250

250251
exitSpy.mockRestore();
251252
logSpy.mockRestore();
253+
vi.useRealTimers();
252254
});
253255
});
254256

packages/cli/src/ui/auth/AuthDialog.tsx

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*/
66

77
import type React from 'react';
8-
import { useCallback } from 'react';
8+
import { useCallback, useState } from 'react';
99
import { Box, Text } from 'ink';
1010
import { theme } from '../semantic-colors.js';
1111
import { RadioButtonSelect } from '../components/shared/RadioButtonSelect.js';
@@ -17,13 +17,13 @@ import { SettingScope } from '../../config/settings.js';
1717
import {
1818
AuthType,
1919
clearCachedCredentialFile,
20-
debugLogger,
2120
type Config,
2221
} from '@google/gemini-cli-core';
2322
import { useKeypress } from '../hooks/useKeypress.js';
2423
import { AuthState } from '../types.js';
2524
import { runExitCleanup } from '../../utils/cleanup.js';
2625
import { validateAuthMethodWithSettings } from './useAuth.js';
26+
import { RELAUNCH_EXIT_CODE } from '../../utils/processUtils.js';
2727

2828
interface AuthDialogProps {
2929
config: Config;
@@ -40,6 +40,7 @@ export function AuthDialog({
4040
authError,
4141
onAuthError,
4242
}: AuthDialogProps): React.JSX.Element {
43+
const [exiting, setExiting] = useState(false);
4344
let items = [
4445
{
4546
label: 'Login with Google',
@@ -111,6 +112,9 @@ export function AuthDialog({
111112

112113
const onSelect = useCallback(
113114
async (authType: AuthType | undefined, scope: LoadableSettingScope) => {
115+
if (exiting) {
116+
return;
117+
}
114118
if (authType) {
115119
await clearCachedCredentialFile();
116120

@@ -119,15 +123,12 @@ export function AuthDialog({
119123
authType === AuthType.LOGIN_WITH_GOOGLE &&
120124
config.isBrowserLaunchSuppressed()
121125
) {
122-
runExitCleanup();
123-
debugLogger.log(
124-
`
125-
----------------------------------------------------------------
126-
Logging in with Google... Please restart Gemini CLI to continue.
127-
----------------------------------------------------------------
128-
`,
129-
);
130-
process.exit(0);
126+
setExiting(true);
127+
setTimeout(async () => {
128+
await runExitCleanup();
129+
process.exit(RELAUNCH_EXIT_CODE);
130+
}, 100);
131+
return;
131132
}
132133
}
133134
if (authType === AuthType.USE_GEMINI) {
@@ -136,7 +137,7 @@ Logging in with Google... Please restart Gemini CLI to continue.
136137
}
137138
setAuthState(AuthState.Unauthenticated);
138139
},
139-
[settings, config, setAuthState],
140+
[settings, config, setAuthState, exiting],
140141
);
141142

142143
const handleAuthSelect = (authMethod: AuthType) => {
@@ -169,6 +170,23 @@ Logging in with Google... Please restart Gemini CLI to continue.
169170
{ isActive: true },
170171
);
171172

173+
if (exiting) {
174+
return (
175+
<Box
176+
borderStyle="round"
177+
borderColor={theme.border.focused}
178+
flexDirection="row"
179+
padding={1}
180+
width="100%"
181+
alignItems="flex-start"
182+
>
183+
<Text color={theme.text.primary}>
184+
Logging in with Google... Restarting Gemini CLI to continue.
185+
</Text>
186+
</Box>
187+
);
188+
}
189+
172190
return (
173191
<Box
174192
borderStyle="round"

packages/cli/src/ui/components/DialogManager.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import { ApiAuthDialog } from '../auth/ApiAuthDialog.js';
1818
import { EditorSettingsDialog } from './EditorSettingsDialog.js';
1919
import { PrivacyNotice } from '../privacy/PrivacyNotice.js';
2020
import { ProQuotaDialog } from './ProQuotaDialog.js';
21+
import { runExitCleanup } from '../../utils/cleanup.js';
22+
import { RELAUNCH_EXIT_CODE } from '../../utils/processUtils.js';
2123
import { PermissionsModifyTrustDialog } from './PermissionsModifyTrustDialog.js';
2224
import { ModelDialog } from './ModelDialog.js';
2325
import { theme } from '../semantic-colors.js';
@@ -137,7 +139,10 @@ export const DialogManager = ({
137139
<SettingsDialog
138140
settings={settings}
139141
onSelect={() => uiActions.closeSettingsDialog()}
140-
onRestartRequest={() => process.exit(0)}
142+
onRestartRequest={async () => {
143+
await runExitCleanup();
144+
process.exit(RELAUNCH_EXIT_CODE);
145+
}}
141146
availableTerminalHeight={terminalHeight - staticExtraHeight}
142147
config={config}
143148
/>

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,19 @@ export async function detectAndEnableKittyProtocol(): Promise<void> {
9797
});
9898
}
9999

100+
import {
101+
enableKittyKeyboardProtocol,
102+
disableKittyKeyboardProtocol,
103+
} from '@google/gemini-cli-core';
104+
100105
export function isKittyProtocolEnabled(): boolean {
101106
return kittyEnabled;
102107
}
103108

104109
function disableAllProtocols() {
105110
try {
106111
if (kittyEnabled) {
107-
// use writeSync to avoid race conditions
108-
fs.writeSync(process.stdout.fd, '\x1b[<u');
112+
disableKittyKeyboardProtocol();
109113
kittyEnabled = false;
110114
}
111115
} catch {
@@ -120,8 +124,7 @@ function disableAllProtocols() {
120124
export function enableSupportedProtocol(): void {
121125
try {
122126
if (kittySupported) {
123-
// use writeSync to avoid race conditions
124-
fs.writeSync(process.stdout.fd, '\x1b[>1u');
127+
enableKittyKeyboardProtocol();
125128
kittyEnabled = true;
126129
}
127130
} catch {

0 commit comments

Comments
 (0)