fix(core): improve Alpine shell compatibility (google-gemini#26770)

dibyx · web-flow · commit aebdba656583 · 2026-05-20T17:16:22.000Z
diff --git a/packages/core/src/services/gitService.ts b/packages/core/src/services/gitService.ts
@@ -24,6 +24,30 @@ import {
 export const SHADOW_REPO_AUTHOR_NAME = 'Gemini CLI';
 export const SHADOW_REPO_AUTHOR_EMAIL = 'gemini-cli@google.com';
 
+const SHADOW_REPO_UNSAFE_OPTIONS = {
+  allowUnsafeAlias: true,
+  allowUnsafeAskPass: true,
+  allowUnsafeConfigEnvCount: true,
+  allowUnsafeConfigPaths: true,
+  allowUnsafeCredentialHelper: true,
+  allowUnsafeCustomBinary: true,
+  allowUnsafeDiffExternal: true,
+  allowUnsafeDiffTextConv: true,
+  allowUnsafeEditor: true,
+  allowUnsafeFilter: true,
+  allowUnsafeFsMonitor: true,
+  allowUnsafeGitProxy: true,
+  allowUnsafeGpgProgram: true,
+  allowUnsafeHooksPath: true,
+  allowUnsafeMergeDriver: true,
+  allowUnsafePack: true,
+  allowUnsafePager: true,
+  allowUnsafeProtocolOverride: true,
+  allowUnsafeSshCommand: true,
+  allowUnsafeTemplateDir: true,
+} satisfies NonNullable<SimpleGitOptions['unsafe']> &
+  Record<`allowUnsafe${string}`, boolean>;
+
 /**
  * Common configuration for the shadow Git repository used for checkpointing.
  *
@@ -32,28 +56,7 @@ export const SHADOW_REPO_AUTHOR_EMAIL = 'gemini-cli@google.com';
  * regardless of the user's local environment (e.g., PAGER, EDITOR, or SSH settings).
  */
 const SHADOW_REPO_GIT_OPTIONS: Partial<SimpleGitOptions> = {
-  unsafe: {
-    allowUnsafeAlias: true,
-    allowUnsafeAskPass: true,
-    allowUnsafeConfigEnvCount: true,
-    allowUnsafeConfigPaths: true,
-    allowUnsafeCredentialHelper: true,
-    allowUnsafeCustomBinary: true,
-    allowUnsafeDiffExternal: true,
-    allowUnsafeDiffTextConv: true,
-    allowUnsafeEditor: true,
-    allowUnsafeFilter: true,
-    allowUnsafeFsMonitor: true,
-    allowUnsafeGitProxy: true,
-    allowUnsafeGpgProgram: true,
-    allowUnsafeHooksPath: true,
-    allowUnsafeMergeDriver: true,
-    allowUnsafePack: true,
-    allowUnsafePager: true,
-    allowUnsafeProtocolOverride: true,
-    allowUnsafeSshCommand: true,
-    allowUnsafeTemplateDir: true,
-  },
+  unsafe: SHADOW_REPO_UNSAFE_OPTIONS,
 };
 
 export class GitService {
diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts
@@ -152,6 +152,7 @@ describe('ShellTool', () => {
       getGeminiClient: vi.fn().mockReturnValue({}),
       getShellToolInactivityTimeout: vi.fn().mockReturnValue(1000),
       getEnableInteractiveShell: vi.fn().mockReturnValue(false),
+      isInteractiveShellEnabled: vi.fn().mockReturnValue(false),
       getShellBackgroundCompletionBehavior: vi.fn().mockReturnValue('silent'),
       getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
       getSandboxEnabled: vi.fn().mockReturnValue(false),
@@ -213,7 +214,7 @@ describe('ShellTool', () => {
         callback: (event: ShellOutputEvent) => void,
       ) => {
         mockShellOutputCallback = callback;
-        const match = cmd.match(/pgrep -g 0 >([^ ]+)/);
+        const match = cmd.match(/_bgpids_file=([^\r\n]+)/);
         if (match) {
           extractedTmpFile = match[1].replace(/['"]/g, '');
         }
@@ -308,19 +309,22 @@ describe('ShellTool', () => {
       resolveExecutionPromise(fullResult);
     };
 
-    it('should wrap command on linux and parse pgrep output', async () => {
+    it('should wrap command on linux and parse background PID output', async () => {
       const invocation = shellTool.build({ command: 'my-command &' });
       const promise = invocation.execute({ abortSignal: mockAbortSignal });
 
-      // Simulate pgrep output file creation by the shell command
+      // Simulate background PID output file creation by the shell command
       fs.writeFileSync(extractedTmpFile, `54321${os.EOL}54322${os.EOL}`);
 
       resolveShellExecution({ pid: 54321 });
 
       const result = await promise;
+      const wrappedCommand = mockShellExecutionService.mock.calls[0][0];
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(
+          /_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp['"]?\n\(\n {2}trap 'jobs -p > "\$_bgpids_file"' EXIT/,
+        ),
         tempRootDir,
         expect.any(Function),
         expect.any(AbortSignal),
@@ -331,19 +335,67 @@ describe('ShellTool', () => {
           sandboxManager: expect.any(Object),
         }),
       );
+      expect(wrappedCommand).toMatch(
+        /^_bgpids_file=.*\n\(\n {2}trap 'jobs -p > "\$_bgpids_file"' EXIT\nmy-command &\n\)\n__code=\$\?\nexit \$__code$/,
+      );
       expect(result.llmContent).toContain('Background PIDs: 54322');
       // The file should be deleted by the tool
       expect(fs.existsSync(extractedTmpFile)).toBe(false);
     });
 
+    it('should preserve exit code and capture background PIDs when command uses explicit exit', async () => {
+      const invocation = shellTool.build({
+        command: "sh -c 'sleep 60 & exit 1'",
+      });
+      const promise = invocation.execute({ abortSignal: mockAbortSignal });
+
+      fs.writeFileSync(extractedTmpFile, `67890${os.EOL}`);
+      expect(fs.readFileSync(extractedTmpFile, 'utf8').trim()).toBe('67890');
+
+      resolveShellExecution({ exitCode: 1, output: '' });
+
+      const result = await promise;
+      const wrappedCommand = mockShellExecutionService.mock.calls[0][0];
+
+      expect(wrappedCommand).toContain(
+        'trap \'jobs -p > "$_bgpids_file"\' EXIT',
+      );
+      expect(wrappedCommand).toContain('sleep 60 & exit 1');
+      expect(result.llmContent).toContain('Exit Code: 1');
+      expect(result.llmContent).toContain('Background PIDs: 67890');
+      expect(fs.existsSync(extractedTmpFile)).toBe(false);
+    });
+
+    it('should disable PTY execution when interactive shell is unavailable', async () => {
+      (mockConfig.getEnableInteractiveShell as Mock).mockReturnValue(true);
+      (mockConfig.isInteractiveShellEnabled as Mock).mockReturnValue(false);
+
+      const invocation = shellTool.build({ command: 'python --version' });
+      const promise = invocation.execute({ abortSignal: mockAbortSignal });
+      resolveShellExecution();
+
+      await promise;
+
+      expect(mockShellExecutionService).toHaveBeenCalledWith(
+        expect.any(String),
+        tempRootDir,
+        expect.any(Function),
+        expect.any(AbortSignal),
+        false,
+        expect.objectContaining({
+          pager: 'cat',
+        }),
+      );
+    });
+
     it('should add a space when command ends with a backslash to prevent escaping newline', async () => {
       const invocation = shellTool.build({ command: 'ls\\' });
       const promise = invocation.execute({ abortSignal: mockAbortSignal });
       resolveShellExecution();
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(/_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp/),
         tempRootDir,
         expect.any(Function),
         expect.any(AbortSignal),
@@ -359,7 +411,7 @@ describe('ShellTool', () => {
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(/_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp/),
         tempRootDir,
         expect.any(Function),
         expect.any(AbortSignal),
@@ -379,7 +431,7 @@ describe('ShellTool', () => {
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(/_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp/),
         subdir,
         expect.any(Function),
         expect.any(AbortSignal),
@@ -402,7 +454,7 @@ describe('ShellTool', () => {
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(/_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp/),
         path.join(tempRootDir, 'subdir'),
         expect.any(Function),
         expect.any(AbortSignal),
@@ -481,7 +533,7 @@ EOF`;
       await promise;
 
       expect(mockShellExecutionService).toHaveBeenCalledWith(
-        expect.stringMatching(/pgrep -g 0 >.*gemini-shell-.*[/\\]pgrep\.tmp/),
+        expect.stringMatching(/_bgpids_file=.*gemini-shell-.*[/\\]bgpids\.tmp/),
         tempRootDir,
         expect.any(Function),
         expect.any(AbortSignal),
@@ -508,7 +560,7 @@ EOF`;
 
       const result = await promise;
       expect(result.llmContent).toContain('Error: wrapped command failed');
-      expect(result.llmContent).not.toContain('pgrep');
+      expect(result.llmContent).not.toContain('background pid output');
       expect(result.display).toEqual(
         expect.objectContaining({
           name: 'Shell',
@@ -599,7 +651,7 @@ EOF`;
     it('should clean up the temp file on synchronous execution error', async () => {
       const error = new Error('sync spawn error');
       mockShellExecutionService.mockImplementation((cmd: string) => {
-        const match = cmd.match(/pgrep -g 0 >([^ ]+)/);
+        const match = cmd.match(/_bgpids_file=([^\r\n]+)/);
         if (match) {
           extractedTmpFile = match[1].replace(/['"]/g, ''); // remove any quotes if present
           // Create the temp file before throwing to simulate it being left behind
@@ -616,7 +668,7 @@ EOF`;
       expect(fs.existsSync(extractedTmpFile)).toBe(false);
     });
 
-    it('should not log "missing pgrep output" when process is backgrounded', async () => {
+    it('should not log "missing background pid output" when process is backgrounded', async () => {
       vi.useFakeTimers();
       const debugErrorSpy = vi.spyOn(debugLogger, 'error');
 
@@ -631,7 +683,9 @@ EOF`;
 
       await promise;
 
-      expect(debugErrorSpy).not.toHaveBeenCalledWith('missing pgrep output');
+      expect(debugErrorSpy).not.toHaveBeenCalledWith(
+        'missing background pid output',
+      );
     });
 
     describe('Streaming to `updateOutput`', () => {
diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts
@@ -108,15 +108,16 @@ export class ShellToolInvocation extends BaseToolInvocation<
   }
 
   /**
-   * Wraps a command in a subshell `()` to capture background process IDs (PIDs) using pgrep.
-   * Uses newlines to prevent breaking heredocs or trailing comments.
+   * Wraps a command in a subshell to capture background process IDs (PIDs)
+   * using an EXIT trap. Uses newlines to prevent breaking heredocs or trailing
+   * comments.
    *
    * @param command The raw command string to execute.
    * @param tempFilePath Path to the temporary file where PIDs will be written.
    * @param isWindows Whether the current platform is Windows (if true, the command is returned as-is).
    * @returns The wrapped command string.
    */
-  private wrapCommandForPgrep(
+  private wrapCommandForBackgroundPIDs(
     command: string,
     tempFilePath: string,
     isWindows: boolean,
@@ -132,7 +133,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
       trimmed += ' ';
     }
     const escapedTempFilePath = escapeShellArg(tempFilePath, 'bash');
-    return `(\n${trimmed}\n)\n__code=$?; pgrep -g 0 >${escapedTempFilePath} 2>&1; exit $__code;`;
+    return `_bgpids_file=${escapedTempFilePath}\n(\n  trap 'jobs -p > "$_bgpids_file"' EXIT\n${trimmed}\n)\n__code=$?\nexit $__code`;
   }
 
   private getContextualDetails(): string {
@@ -497,10 +498,10 @@ export class ShellToolInvocation extends BaseToolInvocation<
     const onAbort = () => combinedController.abort();
     try {
       tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gemini-shell-'));
-      tempFilePath = path.join(tempDir, 'pgrep.tmp');
+      tempFilePath = path.join(tempDir, 'bgpids.tmp');
 
-      // pgrep is not available on Windows, so we can't get background PIDs
-      const commandToExecute = this.wrapCommandForPgrep(
+      // Windows shells do not support the POSIX jobs output used here.
+      const commandToExecute = this.wrapCommandForBackgroundPIDs(
         strippedCommand,
         tempFilePath,
         isWindows,
@@ -651,7 +652,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
             }
           },
           combinedController.signal,
-          this.context.config.getEnableInteractiveShell(),
+          this.context.config.isInteractiveShellEnabled(),
           {
             ...shellExecutionConfig,
             sessionId: this.context.config?.getSessionId?.() ?? 'default',
@@ -736,12 +737,15 @@ export class ShellToolInvocation extends BaseToolInvocation<
         }
 
         if (tempFileExists) {
-          const pgrepContent = await fsPromises.readFile(tempFilePath, 'utf8');
-          const pgrepLines = pgrepContent
+          const backgroundPIDContent = await fsPromises.readFile(
+            tempFilePath,
+            'utf8',
+          );
+          const backgroundPIDLines = backgroundPIDContent
             .split('\n')
             .map((line) => line.trim())
             .filter(Boolean);
-          for (const line of pgrepLines) {
+          for (const line of backgroundPIDLines) {
             if (!/^\d+$/.test(line)) {
               if (
                 line.includes('sysmond service not found') ||
@@ -750,7 +754,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
               ) {
                 continue;
               }
-              debugLogger.error(`pgrep: ${line}`);
+              debugLogger.error(`background pid output: ${line}`);
             }
             const pid = Number(line);
             if (pid !== result.pid) {
@@ -759,7 +763,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
           }
         } else {
           if (!signal.aborted && !result.backgrounded) {
-            debugLogger.error('missing pgrep output');
+            debugLogger.error('missing background pid output');
           }
         }
       }
@@ -1089,7 +1093,7 @@ export class ShellTool extends BaseDeclarativeTool<
       // Errors are surfaced when parsing commands.
     });
     const definition = getShellDefinition(
-      context.config.getEnableInteractiveShell(),
+      context.config.isInteractiveShellEnabled(),
       context.config.getEnableShellOutputEfficiency(),
       context.config.getSandboxEnabled(),
     );
@@ -1139,7 +1143,7 @@ export class ShellTool extends BaseDeclarativeTool<
 
   override getSchema(modelId?: string) {
     const definition = getShellDefinition(
-      this.context.config.getEnableInteractiveShell(),
+      this.context.config.isInteractiveShellEnabled(),
       this.context.config.getEnableShellOutputEfficiency(),
       this.context.config.getSandboxEnabled(),
     );
diff --git a/packages/core/src/tools/shell_proactive.test.ts b/packages/core/src/tools/shell_proactive.test.ts
@@ -83,6 +83,7 @@ describe('ShellTool Proactive Expansion', () => {
         getModeConfig: vi.fn().mockReturnValue({ readonly: false }),
       },
       getEnableInteractiveShell: vi.fn().mockReturnValue(false),
+      isInteractiveShellEnabled: vi.fn().mockReturnValue(false),
       getEnableShellOutputEfficiency: vi.fn().mockReturnValue(true),
       getShellToolInactivityTimeout: vi.fn().mockReturnValue(1000),
     } as unknown as Config;
