From 519c75f4107b4add3b1d69b4cd716744950dd57d Mon Sep 17 00:00:00 2001 From: Coco Sheng Date: Tue, 10 Mar 2026 16:30:58 -0400 Subject: [PATCH] fix: prevent hangs in non-interactive mode and improve agent guidance (#20893) Co-authored-by: Keith Schaab --- .../core/__snapshots__/prompts.test.ts.snap | 4 +- packages/core/src/prompts/snippets.ts | 2 +- .../services/shellExecutionService.test.ts | 91 +++++++++++++++++++ .../src/services/shellExecutionService.ts | 54 +++++++++-- 4 files changed, 140 insertions(+), 11 deletions(-) diff --git a/packages/core/src/core/__snapshots__/prompts.test.ts.snap b/packages/core/src/core/__snapshots__/prompts.test.ts.snap index e345fc3882..f11af69e7b 100644 --- a/packages/core/src/core/__snapshots__/prompts.test.ts.snap +++ b/packages/core/src/core/__snapshots__/prompts.test.ts.snap @@ -851,7 +851,7 @@ Use the following guidelines to optimize your search and read patterns. - **Explaining Changes:** After completing a code modification or file operation *do not* provide summaries unless asked. - **Do Not revert changes:** Do not revert changes to the codebase unless asked to do so by the user. Only revert changes made by you if they have resulted in an error or if the user has explicitly asked you to revert the changes. - **Explain Before Acting:** Never call tools in silence. You MUST provide a concise, one-sentence explanation of your intent or strategy immediately before executing tool calls. This is essential for transparency, especially when confirming a request or answering a question. Silence is only acceptable for repetitive, low-level discovery operations (e.g., sequential file reads) where narration would be noisy. - - **Continue the work** You are not to interact with the user. Do your best to complete the task at hand, using your best judgement and avoid asking user for any additional information. +- **Non-Interactive Environment:** You are running in a headless/CI environment and cannot interact with the user. Do not ask the user questions or request additional information, as the session will terminate. Use your best judgment to complete the task. If a tool fails because it requires user interaction, do not retry it indefinitely; instead, explain the limitation and suggest how the user can provide the required data (e.g., via environment variables). # Hook Context @@ -973,7 +973,7 @@ Use the following guidelines to optimize your search and read patterns. - **Explaining Changes:** After completing a code modification or file operation *do not* provide summaries unless asked. - **Do Not revert changes:** Do not revert changes to the codebase unless asked to do so by the user. Only revert changes made by you if they have resulted in an error or if the user has explicitly asked you to revert the changes. - **Explain Before Acting:** Never call tools in silence. You MUST provide a concise, one-sentence explanation of your intent or strategy immediately before executing tool calls. This is essential for transparency, especially when confirming a request or answering a question. Silence is only acceptable for repetitive, low-level discovery operations (e.g., sequential file reads) where narration would be noisy. - - **Continue the work** You are not to interact with the user. Do your best to complete the task at hand, using your best judgement and avoid asking user for any additional information. +- **Non-Interactive Environment:** You are running in a headless/CI environment and cannot interact with the user. Do not ask the user questions or request additional information, as the session will terminate. Use your best judgment to complete the task. If a tool fails because it requires user interaction, do not retry it indefinitely; instead, explain the limitation and suggest how the user can provide the required data (e.g., via environment variables). # Hook Context diff --git a/packages/core/src/prompts/snippets.ts b/packages/core/src/prompts/snippets.ts index 3b3334f96b..bad6827ae7 100644 --- a/packages/core/src/prompts/snippets.ts +++ b/packages/core/src/prompts/snippets.ts @@ -573,7 +573,7 @@ function mandateConflictResolution(hasHierarchicalMemory: boolean): string { function mandateContinueWork(interactive: boolean): string { if (interactive) return ''; return ` - - **Continue the work** You are not to interact with the user. Do your best to complete the task at hand, using your best judgement and avoid asking user for any additional information.`; +- **Non-Interactive Environment:** You are running in a headless/CI environment and cannot interact with the user. Do not ask the user questions or request additional information, as the session will terminate. Use your best judgment to complete the task. If a tool fails because it requires user interaction, do not retry it indefinitely; instead, explain the limitation and suggest how the user can provide the required data (e.g., via environment variables).`; } function workflowStepResearch(options: PrimaryWorkflowsOptions): string { diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index c99bb43292..8399bb6d0c 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -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(); + }); }); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index e393767148..f3358af992 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -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 = {