fix(core): improve Alpine shell compatibility (#26770)

2026-05-24 21:24:25 +00:00 · 2026-05-20 22:46:22 +05:30
parent 26f8c0f65e
commit aebdba6565
4 changed files with 112 additions and 50 deletions
--- 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 {
--- 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,11 +335,59 @@ 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 });
@@ -343,7 +395,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),
@@ -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`', () => {
--- 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(),
    );
--- 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;