Skip to content

Commit c59ef74

Browse files
authored
fix(core, a2a-server): prevent hang during OAuth in non-interactive sessions (google-gemini#21045)
1 parent 29b3aa8 commit c59ef74

5 files changed

Lines changed: 335 additions & 13 deletions

File tree

packages/a2a-server/src/config/config.test.ts

Lines changed: 225 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import {
1616
ExperimentFlags,
1717
fetchAdminControlsOnce,
1818
type FetchAdminControlsResponse,
19+
AuthType,
20+
isHeadlessMode,
21+
FatalAuthenticationError,
1922
} from '@google/gemini-cli-core';
2023

2124
// Mock dependencies
@@ -50,6 +53,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
5053
startupProfiler: {
5154
flush: vi.fn(),
5255
},
56+
isHeadlessMode: vi.fn().mockReturnValue(false),
5357
FileDiscoveryService: vi.fn(),
5458
getCodeAssistServer: vi.fn(),
5559
fetchAdminControlsOnce: vi.fn(),
@@ -62,6 +66,7 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
6266
vi.mock('../utils/logger.js', () => ({
6367
logger: {
6468
info: vi.fn(),
69+
warn: vi.fn(),
6570
error: vi.fn(),
6671
},
6772
}));
@@ -73,12 +78,11 @@ describe('loadConfig', () => {
7378

7479
beforeEach(() => {
7580
vi.clearAllMocks();
76-
process.env['GEMINI_API_KEY'] = 'test-key';
81+
vi.stubEnv('GEMINI_API_KEY', 'test-key');
7782
});
7883

7984
afterEach(() => {
80-
delete process.env['CUSTOM_IGNORE_FILE_PATHS'];
81-
delete process.env['GEMINI_API_KEY'];
85+
vi.unstubAllEnvs();
8286
});
8387

8488
describe('admin settings overrides', () => {
@@ -199,7 +203,7 @@ describe('loadConfig', () => {
199203

200204
it('should set customIgnoreFilePaths when CUSTOM_IGNORE_FILE_PATHS env var is present', async () => {
201205
const testPath = '/tmp/ignore';
202-
process.env['CUSTOM_IGNORE_FILE_PATHS'] = testPath;
206+
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
203207
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId);
204208
// eslint-disable-next-line @typescript-eslint/no-explicit-any
205209
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual([
@@ -224,7 +228,7 @@ describe('loadConfig', () => {
224228
it('should merge customIgnoreFilePaths from settings and env var', async () => {
225229
const envPath = '/env/ignore';
226230
const settingsPath = '/settings/ignore';
227-
process.env['CUSTOM_IGNORE_FILE_PATHS'] = envPath;
231+
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', envPath);
228232
const settings: Settings = {
229233
fileFiltering: {
230234
customIgnoreFilePaths: [settingsPath],
@@ -240,7 +244,7 @@ describe('loadConfig', () => {
240244

241245
it('should split CUSTOM_IGNORE_FILE_PATHS using system delimiter', async () => {
242246
const paths = ['/path/one', '/path/two'];
243-
process.env['CUSTOM_IGNORE_FILE_PATHS'] = paths.join(path.delimiter);
247+
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', paths.join(path.delimiter));
244248
const config = await loadConfig(mockSettings, mockExtensionLoader, taskId);
245249
// eslint-disable-next-line @typescript-eslint/no-explicit-any
246250
expect((config as any).fileFiltering.customIgnoreFilePaths).toEqual(paths);
@@ -254,7 +258,7 @@ describe('loadConfig', () => {
254258

255259
it('should initialize FileDiscoveryService with correct options', async () => {
256260
const testPath = '/tmp/ignore';
257-
process.env['CUSTOM_IGNORE_FILE_PATHS'] = testPath;
261+
vi.stubEnv('CUSTOM_IGNORE_FILE_PATHS', testPath);
258262
const settings: Settings = {
259263
fileFiltering: {
260264
respectGitIgnore: false,
@@ -311,5 +315,219 @@ describe('loadConfig', () => {
311315
}),
312316
);
313317
});
318+
319+
describe('interactivity', () => {
320+
it('should set interactive true when not headless', async () => {
321+
vi.mocked(isHeadlessMode).mockReturnValue(false);
322+
await loadConfig(mockSettings, mockExtensionLoader, taskId);
323+
expect(Config).toHaveBeenCalledWith(
324+
expect.objectContaining({
325+
interactive: true,
326+
enableInteractiveShell: true,
327+
}),
328+
);
329+
});
330+
331+
it('should set interactive false when headless', async () => {
332+
vi.mocked(isHeadlessMode).mockReturnValue(true);
333+
await loadConfig(mockSettings, mockExtensionLoader, taskId);
334+
expect(Config).toHaveBeenCalledWith(
335+
expect.objectContaining({
336+
interactive: false,
337+
enableInteractiveShell: false,
338+
}),
339+
);
340+
});
341+
});
342+
343+
describe('authentication fallback', () => {
344+
beforeEach(() => {
345+
vi.stubEnv('USE_CCPA', 'true');
346+
vi.stubEnv('GEMINI_API_KEY', '');
347+
});
348+
349+
afterEach(() => {
350+
vi.unstubAllEnvs();
351+
});
352+
353+
it('should fall back to COMPUTE_ADC in Cloud Shell if LOGIN_WITH_GOOGLE fails', async () => {
354+
vi.stubEnv('CLOUD_SHELL', 'true');
355+
vi.mocked(isHeadlessMode).mockReturnValue(false);
356+
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
357+
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
358+
throw new FatalAuthenticationError('Non-interactive session');
359+
}
360+
return Promise.resolve();
361+
});
362+
363+
// Update the mock implementation for this test
364+
vi.mocked(Config).mockImplementation(
365+
(params: unknown) =>
366+
({
367+
...(params as object),
368+
initialize: vi.fn(),
369+
waitForMcpInit: vi.fn(),
370+
refreshAuth: refreshAuthMock,
371+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
372+
getRemoteAdminSettings: vi.fn(),
373+
setRemoteAdminSettings: vi.fn(),
374+
}) as unknown as Config,
375+
);
376+
377+
await loadConfig(mockSettings, mockExtensionLoader, taskId);
378+
379+
expect(refreshAuthMock).toHaveBeenCalledWith(
380+
AuthType.LOGIN_WITH_GOOGLE,
381+
);
382+
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
383+
});
384+
385+
it('should not fall back to COMPUTE_ADC if not in cloud environment', async () => {
386+
vi.mocked(isHeadlessMode).mockReturnValue(false);
387+
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
388+
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
389+
throw new FatalAuthenticationError('Non-interactive session');
390+
}
391+
return Promise.resolve();
392+
});
393+
394+
vi.mocked(Config).mockImplementation(
395+
(params: unknown) =>
396+
({
397+
...(params as object),
398+
initialize: vi.fn(),
399+
waitForMcpInit: vi.fn(),
400+
refreshAuth: refreshAuthMock,
401+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
402+
getRemoteAdminSettings: vi.fn(),
403+
setRemoteAdminSettings: vi.fn(),
404+
}) as unknown as Config,
405+
);
406+
407+
await expect(
408+
loadConfig(mockSettings, mockExtensionLoader, taskId),
409+
).rejects.toThrow('Non-interactive session');
410+
411+
expect(refreshAuthMock).toHaveBeenCalledWith(
412+
AuthType.LOGIN_WITH_GOOGLE,
413+
);
414+
expect(refreshAuthMock).not.toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
415+
});
416+
417+
it('should skip LOGIN_WITH_GOOGLE and use COMPUTE_ADC directly in headless Cloud Shell', async () => {
418+
vi.stubEnv('CLOUD_SHELL', 'true');
419+
vi.mocked(isHeadlessMode).mockReturnValue(true);
420+
421+
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
422+
423+
vi.mocked(Config).mockImplementation(
424+
(params: unknown) =>
425+
({
426+
...(params as object),
427+
initialize: vi.fn(),
428+
waitForMcpInit: vi.fn(),
429+
refreshAuth: refreshAuthMock,
430+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
431+
getRemoteAdminSettings: vi.fn(),
432+
setRemoteAdminSettings: vi.fn(),
433+
}) as unknown as Config,
434+
);
435+
436+
await loadConfig(mockSettings, mockExtensionLoader, taskId);
437+
438+
expect(refreshAuthMock).not.toHaveBeenCalledWith(
439+
AuthType.LOGIN_WITH_GOOGLE,
440+
);
441+
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
442+
});
443+
444+
it('should skip LOGIN_WITH_GOOGLE and use COMPUTE_ADC directly if GEMINI_CLI_USE_COMPUTE_ADC is true', async () => {
445+
vi.stubEnv('GEMINI_CLI_USE_COMPUTE_ADC', 'true');
446+
vi.mocked(isHeadlessMode).mockReturnValue(false); // Even if not headless
447+
448+
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
449+
450+
vi.mocked(Config).mockImplementation(
451+
(params: unknown) =>
452+
({
453+
...(params as object),
454+
initialize: vi.fn(),
455+
waitForMcpInit: vi.fn(),
456+
refreshAuth: refreshAuthMock,
457+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
458+
getRemoteAdminSettings: vi.fn(),
459+
setRemoteAdminSettings: vi.fn(),
460+
}) as unknown as Config,
461+
);
462+
463+
await loadConfig(mockSettings, mockExtensionLoader, taskId);
464+
465+
expect(refreshAuthMock).not.toHaveBeenCalledWith(
466+
AuthType.LOGIN_WITH_GOOGLE,
467+
);
468+
expect(refreshAuthMock).toHaveBeenCalledWith(AuthType.COMPUTE_ADC);
469+
});
470+
471+
it('should throw FatalAuthenticationError in headless mode if no ADC fallback available', async () => {
472+
vi.mocked(isHeadlessMode).mockReturnValue(true);
473+
474+
const refreshAuthMock = vi.fn().mockResolvedValue(undefined);
475+
476+
vi.mocked(Config).mockImplementation(
477+
(params: unknown) =>
478+
({
479+
...(params as object),
480+
initialize: vi.fn(),
481+
waitForMcpInit: vi.fn(),
482+
refreshAuth: refreshAuthMock,
483+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
484+
getRemoteAdminSettings: vi.fn(),
485+
setRemoteAdminSettings: vi.fn(),
486+
}) as unknown as Config,
487+
);
488+
489+
await expect(
490+
loadConfig(mockSettings, mockExtensionLoader, taskId),
491+
).rejects.toThrow(
492+
'Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.',
493+
);
494+
495+
expect(refreshAuthMock).not.toHaveBeenCalled();
496+
});
497+
498+
it('should include both original and fallback error when COMPUTE_ADC fallback fails', async () => {
499+
vi.stubEnv('CLOUD_SHELL', 'true');
500+
vi.mocked(isHeadlessMode).mockReturnValue(false);
501+
502+
const refreshAuthMock = vi.fn().mockImplementation((authType) => {
503+
if (authType === AuthType.LOGIN_WITH_GOOGLE) {
504+
throw new FatalAuthenticationError('OAuth failed');
505+
}
506+
if (authType === AuthType.COMPUTE_ADC) {
507+
throw new Error('ADC failed');
508+
}
509+
return Promise.resolve();
510+
});
511+
512+
vi.mocked(Config).mockImplementation(
513+
(params: unknown) =>
514+
({
515+
...(params as object),
516+
initialize: vi.fn(),
517+
waitForMcpInit: vi.fn(),
518+
refreshAuth: refreshAuthMock,
519+
getExperiments: vi.fn().mockReturnValue({ flags: {} }),
520+
getRemoteAdminSettings: vi.fn(),
521+
setRemoteAdminSettings: vi.fn(),
522+
}) as unknown as Config,
523+
);
524+
525+
await expect(
526+
loadConfig(mockSettings, mockExtensionLoader, taskId),
527+
).rejects.toThrow(
528+
'OAuth failed. Fallback to COMPUTE_ADC also failed: ADC failed',
529+
);
530+
});
531+
});
314532
});
315533
});

packages/a2a-server/src/config/config.ts

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ import {
2323
fetchAdminControlsOnce,
2424
getCodeAssistServer,
2525
ExperimentFlags,
26+
isHeadlessMode,
27+
FatalAuthenticationError,
28+
isCloudShell,
2629
type TelemetryTarget,
2730
type ConfigParameters,
2831
type ExtensionLoader,
@@ -103,8 +106,8 @@ export async function loadConfig(
103106
trustedFolder: true,
104107
extensionLoader,
105108
checkpointing,
106-
interactive: true,
107-
enableInteractiveShell: true,
109+
interactive: !isHeadlessMode(),
110+
enableInteractiveShell: !isHeadlessMode(),
108111
ptyInfo: 'auto',
109112
};
110113

@@ -255,7 +258,61 @@ async function refreshAuthentication(
255258
`[${logPrefix}] USE_CCPA env var is true but unable to resolve GOOGLE_APPLICATION_CREDENTIALS file path ${adcFilePath}. Error ${e}`,
256259
);
257260
}
258-
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);
261+
262+
const useComputeAdc = process.env['GEMINI_CLI_USE_COMPUTE_ADC'] === 'true';
263+
const isHeadless = isHeadlessMode();
264+
const shouldSkipOauth = isHeadless || useComputeAdc;
265+
266+
if (shouldSkipOauth) {
267+
if (isCloudShell() || useComputeAdc) {
268+
logger.info(
269+
`[${logPrefix}] Skipping LOGIN_WITH_GOOGLE due to ${isHeadless ? 'headless mode' : 'GEMINI_CLI_USE_COMPUTE_ADC'}. Attempting COMPUTE_ADC.`,
270+
);
271+
try {
272+
await config.refreshAuth(AuthType.COMPUTE_ADC);
273+
logger.info(`[${logPrefix}] COMPUTE_ADC successful.`);
274+
} catch (adcError) {
275+
const adcMessage =
276+
adcError instanceof Error ? adcError.message : String(adcError);
277+
throw new FatalAuthenticationError(
278+
`COMPUTE_ADC failed: ${adcMessage}. (Skipped LOGIN_WITH_GOOGLE due to ${isHeadless ? 'headless mode' : 'GEMINI_CLI_USE_COMPUTE_ADC'})`,
279+
);
280+
}
281+
} else {
282+
throw new FatalAuthenticationError(
283+
`Interactive terminal required for LOGIN_WITH_GOOGLE. Run in an interactive terminal or set GEMINI_CLI_USE_COMPUTE_ADC=true to use Application Default Credentials.`,
284+
);
285+
}
286+
} else {
287+
try {
288+
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);
289+
} catch (e) {
290+
if (
291+
e instanceof FatalAuthenticationError &&
292+
(isCloudShell() || useComputeAdc)
293+
) {
294+
logger.warn(
295+
`[${logPrefix}] LOGIN_WITH_GOOGLE failed. Attempting COMPUTE_ADC fallback.`,
296+
);
297+
try {
298+
await config.refreshAuth(AuthType.COMPUTE_ADC);
299+
logger.info(`[${logPrefix}] COMPUTE_ADC fallback successful.`);
300+
} catch (adcError) {
301+
logger.error(
302+
`[${logPrefix}] COMPUTE_ADC fallback failed: ${adcError}`,
303+
);
304+
const originalMessage = e instanceof Error ? e.message : String(e);
305+
const adcMessage =
306+
adcError instanceof Error ? adcError.message : String(adcError);
307+
throw new FatalAuthenticationError(
308+
`${originalMessage}. Fallback to COMPUTE_ADC also failed: ${adcMessage}`,
309+
);
310+
}
311+
} else {
312+
throw e;
313+
}
314+
}
315+
}
259316
logger.info(
260317
`[${logPrefix}] GOOGLE_CLOUD_PROJECT: ${process.env['GOOGLE_CLOUD_PROJECT']}`,
261318
);

0 commit comments

Comments
 (0)