fix: prevent hangs in non-interactive mode and improve agent guidance (#20893)

Co-authored-by: Keith Schaab <keith.schaab@gmail.com>
This commit is contained in:
Coco Sheng
2026-03-10 16:30:58 -04:00
committed by GitHub
parent 3ff68a9e55
commit 519c75f410
4 changed files with 140 additions and 11 deletions
@@ -1703,4 +1703,95 @@ describe('ShellExecutionService environment variables', () => {
mockChildProcess.emit('close', 0, null);
await new Promise(process.nextTick);
});
it('should include headless git and gh environment variables in non-interactive mode and append git config safely', async () => {
vi.resetModules();
vi.stubEnv('GIT_CONFIG_COUNT', '2');
vi.stubEnv('GIT_CONFIG_KEY_0', 'core.editor');
vi.stubEnv('GIT_CONFIG_VALUE_0', 'vim');
vi.stubEnv('GIT_CONFIG_KEY_1', 'pull.rebase');
vi.stubEnv('GIT_CONFIG_VALUE_1', 'true');
const { ShellExecutionService } = await import(
'./shellExecutionService.js'
);
mockGetPty.mockResolvedValue(null); // Force child_process fallback
await ShellExecutionService.execute(
'test-cp-headless-git',
'/',
vi.fn(),
new AbortController().signal,
false, // non-interactive
shellExecutionConfig,
);
expect(mockCpSpawn).toHaveBeenCalled();
const cpEnv = mockCpSpawn.mock.calls[0][2].env;
expect(cpEnv).toHaveProperty('GIT_TERMINAL_PROMPT', '0');
expect(cpEnv).toHaveProperty('GIT_ASKPASS', '');
expect(cpEnv).toHaveProperty('SSH_ASKPASS', '');
expect(cpEnv).toHaveProperty('GH_PROMPT_DISABLED', '1');
expect(cpEnv).toHaveProperty('GCM_INTERACTIVE', 'never');
expect(cpEnv).toHaveProperty('DISPLAY', '');
expect(cpEnv).toHaveProperty('DBUS_SESSION_BUS_ADDRESS', '');
// Existing values should be preserved
expect(cpEnv).toHaveProperty('GIT_CONFIG_KEY_0', 'core.editor');
expect(cpEnv).toHaveProperty('GIT_CONFIG_VALUE_0', 'vim');
expect(cpEnv).toHaveProperty('GIT_CONFIG_KEY_1', 'pull.rebase');
expect(cpEnv).toHaveProperty('GIT_CONFIG_VALUE_1', 'true');
// The new credential.helper override should be appended at index 2
expect(cpEnv).toHaveProperty('GIT_CONFIG_COUNT', '3');
expect(cpEnv).toHaveProperty('GIT_CONFIG_KEY_2', 'credential.helper');
expect(cpEnv).toHaveProperty('GIT_CONFIG_VALUE_2', '');
// Ensure child_process exits
mockChildProcess.emit('exit', 0, null);
mockChildProcess.emit('close', 0, null);
await new Promise(process.nextTick);
vi.unstubAllEnvs();
});
it('should NOT include headless git and gh environment variables in interactive fallback mode', async () => {
vi.resetModules();
vi.stubEnv('GIT_TERMINAL_PROMPT', undefined);
vi.stubEnv('GIT_ASKPASS', undefined);
vi.stubEnv('SSH_ASKPASS', undefined);
vi.stubEnv('GH_PROMPT_DISABLED', undefined);
vi.stubEnv('GCM_INTERACTIVE', undefined);
vi.stubEnv('GIT_CONFIG_COUNT', undefined);
const { ShellExecutionService } = await import(
'./shellExecutionService.js'
);
mockGetPty.mockResolvedValue(null); // Force child_process fallback
await ShellExecutionService.execute(
'test-cp-interactive-fallback',
'/',
vi.fn(),
new AbortController().signal,
true, // isInteractive (shouldUseNodePty)
shellExecutionConfig,
);
expect(mockCpSpawn).toHaveBeenCalled();
const cpEnv = mockCpSpawn.mock.calls[0][2].env;
expect(cpEnv).not.toHaveProperty('GIT_TERMINAL_PROMPT');
expect(cpEnv).not.toHaveProperty('GIT_ASKPASS');
expect(cpEnv).not.toHaveProperty('SSH_ASKPASS');
expect(cpEnv).not.toHaveProperty('GH_PROMPT_DISABLED');
expect(cpEnv).not.toHaveProperty('GCM_INTERACTIVE');
expect(cpEnv).not.toHaveProperty('GIT_CONFIG_COUNT');
// Ensure child_process exits
mockChildProcess.emit('exit', 0, null);
mockChildProcess.emit('close', 0, null);
await new Promise(process.nextTick);
vi.unstubAllEnvs();
});
});
@@ -252,6 +252,7 @@ export class ShellExecutionService {
onOutputEvent,
abortSignal,
shellExecutionConfig.sanitizationConfig,
shouldUseNodePty,
);
}
@@ -298,6 +299,7 @@ export class ShellExecutionService {
onOutputEvent: (event: ShellOutputEvent) => void,
abortSignal: AbortSignal,
sanitizationConfig: EnvironmentSanitizationConfig,
isInteractive: boolean,
): ShellExecutionHandle {
try {
const isWindows = os.platform() === 'win32';
@@ -305,20 +307,56 @@ export class ShellExecutionService {
const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
const spawnArgs = [...argsPrefix, guardedCommand];
// Specifically allow GIT_CONFIG_* variables to pass through sanitization
// in non-interactive mode so we can safely append our overrides.
const gitConfigKeys = !isInteractive
? Object.keys(process.env).filter((k) => k.startsWith('GIT_CONFIG_'))
: [];
const sanitizedEnv = sanitizeEnvironment(process.env, {
...sanitizationConfig,
allowedEnvironmentVariables: [
...(sanitizationConfig.allowedEnvironmentVariables || []),
...gitConfigKeys,
],
});
const env: NodeJS.ProcessEnv = {
...sanitizedEnv,
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
TERM: 'xterm-256color',
PAGER: 'cat',
GIT_PAGER: 'cat',
};
if (!isInteractive) {
const gitConfigCount = parseInt(
sanitizedEnv['GIT_CONFIG_COUNT'] || '0',
10,
);
Object.assign(env, {
// Disable interactive prompts and session-linked credential helpers
// in non-interactive mode to prevent hangs in detached process groups.
GIT_TERMINAL_PROMPT: '0',
GIT_ASKPASS: '',
SSH_ASKPASS: '',
GH_PROMPT_DISABLED: '1',
GCM_INTERACTIVE: 'never',
DISPLAY: '',
DBUS_SESSION_BUS_ADDRESS: '',
GIT_CONFIG_COUNT: (gitConfigCount + 1).toString(),
[`GIT_CONFIG_KEY_${gitConfigCount}`]: 'credential.helper',
[`GIT_CONFIG_VALUE_${gitConfigCount}`]: '',
});
}
const child = cpSpawn(executable, spawnArgs, {
cwd,
stdio: ['ignore', 'pipe', 'pipe'],
windowsVerbatimArguments: isWindows ? false : undefined,
shell: false,
detached: !isWindows,
env: {
...sanitizeEnvironment(process.env, sanitizationConfig),
[GEMINI_CLI_IDENTIFICATION_ENV_VAR]:
GEMINI_CLI_IDENTIFICATION_ENV_VAR_VALUE,
TERM: 'xterm-256color',
PAGER: 'cat',
GIT_PAGER: 'cat',
},
env,
});
const state = {