mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-24 04:52:43 -07:00
feat(core,cli): harden headless mode detection and align mocks
- Update isHeadlessMode in packages/core to check both stdin.isTTY and stdout.isTTY. - Synchronize isHeadlessMode mock in packages/cli tests and add global TTY stubs to ensure consistent test environments. - Add isMounted guard to useFolderTrust hook to prevent state updates on unmounted components in headless mode. - Expand unit tests in packages/core to cover new TTY check combinations and edge cases. - Stabilize flaky MCP initialization test in packages/core/src/config/config.test.ts by using a deterministic promise. - Address review findings regarding environment detection consistency and CI indicator checks.
This commit is contained in:
@@ -12,8 +12,12 @@ import { coreEvents } from './events.js';
|
||||
import { getConsentForOauth } from './authConsent.js';
|
||||
import { FatalAuthenticationError } from './errors.js';
|
||||
import { writeToStdout } from './stdio.js';
|
||||
import { isHeadlessMode } from './headless.js';
|
||||
|
||||
vi.mock('node:readline');
|
||||
vi.mock('./headless.js', () => ({
|
||||
isHeadlessMode: vi.fn(),
|
||||
}));
|
||||
vi.mock('./stdio.js', () => ({
|
||||
writeToStdout: vi.fn(),
|
||||
createWorkingStdio: vi.fn(() => ({
|
||||
@@ -49,16 +53,12 @@ describe('getConsentForOauth', () => {
|
||||
mockEmitConsentRequest.mockRestore();
|
||||
});
|
||||
|
||||
it('should use readline when no listeners are present and stdin is a TTY', async () => {
|
||||
it('should use readline when no listeners are present and not headless', async () => {
|
||||
vi.restoreAllMocks();
|
||||
const mockListenerCount = vi
|
||||
.spyOn(coreEvents, 'listenerCount')
|
||||
.mockReturnValue(0);
|
||||
const originalIsTTY = process.stdin.isTTY;
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: true,
|
||||
configurable: true,
|
||||
});
|
||||
(isHeadlessMode as Mock).mockReturnValue(false);
|
||||
|
||||
const mockReadline = {
|
||||
on: vi.fn((event, callback) => {
|
||||
@@ -81,31 +81,19 @@ describe('getConsentForOauth', () => {
|
||||
);
|
||||
|
||||
mockListenerCount.mockRestore();
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: originalIsTTY,
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('should throw FatalAuthenticationError when no listeners and not a TTY', async () => {
|
||||
it('should throw FatalAuthenticationError when no listeners and headless', async () => {
|
||||
vi.restoreAllMocks();
|
||||
const mockListenerCount = vi
|
||||
.spyOn(coreEvents, 'listenerCount')
|
||||
.mockReturnValue(0);
|
||||
const originalIsTTY = process.stdin.isTTY;
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
(isHeadlessMode as Mock).mockReturnValue(true);
|
||||
|
||||
await expect(getConsentForOauth('Login required.')).rejects.toThrow(
|
||||
FatalAuthenticationError,
|
||||
);
|
||||
|
||||
mockListenerCount.mockRestore();
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: originalIsTTY,
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -8,6 +8,7 @@ import readline from 'node:readline';
|
||||
import { CoreEvent, coreEvents } from './events.js';
|
||||
import { FatalAuthenticationError } from './errors.js';
|
||||
import { createWorkingStdio, writeToStdout } from './stdio.js';
|
||||
import { isHeadlessMode } from './headless.js';
|
||||
|
||||
/**
|
||||
* Requests consent from the user for OAuth login.
|
||||
@@ -17,7 +18,7 @@ export async function getConsentForOauth(prompt: string): Promise<boolean> {
|
||||
const finalPrompt = prompt + ' Opening authentication page in your browser. ';
|
||||
|
||||
if (coreEvents.listenerCount(CoreEvent.ConsentRequest) === 0) {
|
||||
if (!process.stdin.isTTY) {
|
||||
if (isHeadlessMode()) {
|
||||
throw new FatalAuthenticationError(
|
||||
'Interactive consent could not be obtained.\n' +
|
||||
'Please run Gemini CLI in an interactive terminal to authenticate, or use NO_BROWSER=true for manual authentication.',
|
||||
|
||||
146
packages/core/src/utils/headless.test.ts
Normal file
146
packages/core/src/utils/headless.test.ts
Normal file
@@ -0,0 +1,146 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { isHeadlessMode } from './headless.js';
|
||||
import process from 'node:process';
|
||||
|
||||
describe('isHeadlessMode', () => {
|
||||
const originalStdoutIsTTY = process.stdout.isTTY;
|
||||
const originalStdinIsTTY = process.stdin.isTTY;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.stubEnv('CI', '');
|
||||
vi.stubEnv('GITHUB_ACTIONS', '');
|
||||
// We can't easily stub process.stdout.isTTY with vi.stubEnv
|
||||
// So we'll use Object.defineProperty
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: true,
|
||||
configurable: true,
|
||||
});
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: true,
|
||||
configurable: true,
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs();
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: originalStdoutIsTTY,
|
||||
configurable: true,
|
||||
});
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: originalStdinIsTTY,
|
||||
configurable: true,
|
||||
});
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('should return false in a normal TTY environment', () => {
|
||||
expect(isHeadlessMode()).toBe(false);
|
||||
});
|
||||
|
||||
it('should return true if CI environment variable is "true"', () => {
|
||||
vi.stubEnv('CI', 'true');
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if GITHUB_ACTIONS environment variable is "true"', () => {
|
||||
vi.stubEnv('GITHUB_ACTIONS', 'true');
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if stdout is not a TTY', () => {
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if stdin is not a TTY', () => {
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if stdin is a TTY but stdout is not', () => {
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: true,
|
||||
configurable: true,
|
||||
});
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if stdout is a TTY but stdin is not', () => {
|
||||
Object.defineProperty(process.stdin, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: true,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isHeadlessMode()).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true if a prompt option is provided', () => {
|
||||
expect(isHeadlessMode({ prompt: 'test prompt' })).toBe(true);
|
||||
expect(isHeadlessMode({ prompt: true })).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false if query is provided but it is still a TTY', () => {
|
||||
// Note: per current logic, query alone doesn't force headless if TTY
|
||||
// This matches the existing behavior in packages/cli/src/config/config.ts
|
||||
expect(isHeadlessMode({ query: 'test query' })).toBe(false);
|
||||
});
|
||||
|
||||
it('should handle undefined process.stdout gracefully', () => {
|
||||
const originalStdout = process.stdout;
|
||||
// @ts-expect-error - testing edge case
|
||||
delete process.stdout;
|
||||
|
||||
try {
|
||||
expect(isHeadlessMode()).toBe(false);
|
||||
} finally {
|
||||
Object.defineProperty(process, 'stdout', {
|
||||
value: originalStdout,
|
||||
configurable: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('should handle undefined process.stdin gracefully', () => {
|
||||
const originalStdin = process.stdin;
|
||||
// @ts-expect-error - testing edge case
|
||||
delete process.stdin;
|
||||
|
||||
try {
|
||||
expect(isHeadlessMode()).toBe(false);
|
||||
} finally {
|
||||
Object.defineProperty(process, 'stdin', {
|
||||
value: originalStdin,
|
||||
configurable: true,
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it('should return true if multiple headless indicators are set', () => {
|
||||
vi.stubEnv('CI', 'true');
|
||||
Object.defineProperty(process.stdout, 'isTTY', {
|
||||
value: false,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isHeadlessMode({ prompt: true })).toBe(true);
|
||||
});
|
||||
});
|
||||
38
packages/core/src/utils/headless.ts
Normal file
38
packages/core/src/utils/headless.ts
Normal file
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2025 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import process from 'node:process';
|
||||
|
||||
/**
|
||||
* Options for headless mode detection.
|
||||
*/
|
||||
export interface HeadlessModeOptions {
|
||||
/** Explicit prompt string or flag. */
|
||||
prompt?: string | boolean;
|
||||
/** Initial query positional argument. */
|
||||
query?: string | boolean;
|
||||
}
|
||||
|
||||
/**
|
||||
* Detects if the CLI is running in a "headless" (non-interactive) mode.
|
||||
*
|
||||
* Headless mode is triggered by:
|
||||
* 1. process.env.CI being set to 'true'.
|
||||
* 2. process.stdout not being a TTY.
|
||||
* 3. Presence of an explicit prompt flag.
|
||||
*
|
||||
* @param options - Optional flags and arguments from the CLI.
|
||||
* @returns true if the environment is considered headless.
|
||||
*/
|
||||
export function isHeadlessMode(options?: HeadlessModeOptions): boolean {
|
||||
return (
|
||||
process.env['CI'] === 'true' ||
|
||||
process.env['GITHUB_ACTIONS'] === 'true' ||
|
||||
!!options?.prompt ||
|
||||
(!!process.stdin && !process.stdin.isTTY) ||
|
||||
(!!process.stdout && !process.stdout.isTTY)
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user