diff --git a/docs/developers/tools/shell.md b/docs/developers/tools/shell.md index 5325748b51..50a850201f 100644 --- a/docs/developers/tools/shell.md +++ b/docs/developers/tools/shell.md @@ -146,7 +146,7 @@ To show color in the shell output, you need to set the `tools.shell.showColor` s ### Setting the Pager -You can set a custom pager for the shell output by setting the `tools.shell.pager` setting. The default pager is `cat`. **Note: This setting only applies when `tools.shell.enableInteractiveShell` is enabled.** +You can set a custom pager for the shell output by setting the `tools.shell.pager` setting. The default pager is `cat` on non-Windows platforms. No default is set on Windows. Set `tools.shell.pager` to an empty string to disable pager environment variables. **Note: This setting only applies when `tools.shell.enableInteractiveShell` is enabled.** **Example `settings.json`:** diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 6899e7e8b6..fc4e2cca4e 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -2189,9 +2189,9 @@ const SETTINGS_SCHEMA = { label: 'Pager', category: 'Tools', requiresRestart: false, - default: 'cat' as string | undefined, + default: undefined as string | undefined, description: - 'The pager command to use for shell output. Defaults to `cat`.', + 'The pager command to use for shell output. Defaults to `cat` on non-Windows platforms and unset on Windows. Set to an empty string to disable pager environment variables.', showInDialog: false, }, showColor: { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index 3d80069b31..3a711eb8e5 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -502,6 +502,28 @@ describe('Server Config (config.ts)', () => { ); }); + describe('shell execution config', () => { + it('allows explicitly clearing the configured pager', () => { + const config = new Config(baseParams); + + config.setShellExecutionConfig({ pager: 'less' }); + expect(config.getShellExecutionConfig().pager).toBe('less'); + + config.setShellExecutionConfig({ pager: undefined }); + expect(config.getShellExecutionConfig().pager).toBeUndefined(); + }); + + it('preserves the existing pager when an update omits the pager key', () => { + const config = new Config(baseParams); + + config.setShellExecutionConfig({ pager: 'less' }); + expect(config.getShellExecutionConfig().pager).toBe('less'); + + config.setShellExecutionConfig({ terminalWidth: 120 }); + expect(config.getShellExecutionConfig().pager).toBe('less'); + }); + }); + describe('getMaxSubagentDepth', () => { it('defaults to 5 when unset', () => { expect(new Config(baseParams).getMaxSubagentDepth()).toBe(5); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 9eb96456a2..3a576abb3f 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1919,7 +1919,7 @@ export class Config { terminalWidth: params.shellExecutionConfig?.terminalWidth ?? 80, terminalHeight: params.shellExecutionConfig?.terminalHeight ?? 24, showColor: params.shellExecutionConfig?.showColor ?? false, - pager: params.shellExecutionConfig?.pager ?? 'cat', + pager: params.shellExecutionConfig?.pager, maxBufferedOutputBytes: params.shellExecutionConfig?.maxBufferedOutputBytes, }; @@ -5694,7 +5694,10 @@ export class Config { terminalHeight: config.terminalHeight ?? this.shellExecutionConfig.terminalHeight, showColor: config.showColor ?? this.shellExecutionConfig.showColor, - pager: config.pager ?? this.shellExecutionConfig.pager, + // pager: undefined is a valid explicit clear; ?? would preserve the old value. + pager: Object.prototype.hasOwnProperty.call(config, 'pager') + ? config.pager + : this.shellExecutionConfig.pager, maxBufferedOutputBytes: config.maxBufferedOutputBytes ?? this.shellExecutionConfig.maxBufferedOutputBytes, diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 3927896b63..c01045c61a 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -133,6 +133,13 @@ const shellExecutionConfig = { disableDynamicLineTrimming: true, } satisfies ShellExecutionConfig; +const shellExecutionConfigWithoutPager = { + terminalWidth: 80, + terminalHeight: 24, + showColor: false, + disableDynamicLineTrimming: true, +} satisfies ShellExecutionConfig; + const WINDOWS_SYSTEM_PATH = 'C:\\Windows\\System32;C:\\Shared\\Tools'; const WINDOWS_USER_PATH = 'C:\\Users\\tester\\bin;C:\\Shared\\Tools'; const EXPECTED_MERGED_WINDOWS_PATH = @@ -140,6 +147,20 @@ const EXPECTED_MERGED_WINDOWS_PATH = let originalProcessEnv: NodeJS.ProcessEnv; +async function withoutGitPagerEnv(run: () => Promise): Promise { + const originalGitPager = process.env['GIT_PAGER']; + delete process.env['GIT_PAGER']; + try { + await run(); + } finally { + if (originalGitPager === undefined) { + delete process.env['GIT_PAGER']; + } else { + process.env['GIT_PAGER'] = originalGitPager; + } + } +} + const createExpectedAnsiOutput = (text: string | string[]): AnsiOutput => { const lines = Array.isArray(text) ? text : text.split('\n'); const expected: AnsiOutput = Array.from( @@ -1896,6 +1917,52 @@ describe('ShellExecutionService', () => { }); }); + it('does not inject Unix pager defaults into Windows pty env when unset', async () => { + mockPlatform.mockReturnValue('win32'); + mockGetShellConfiguration.mockReturnValue({ + executable: 'cmd.exe', + argsPrefix: ['/d', '/s', '/c'], + shell: 'cmd', + }); + + await simulateExecution( + 'echo hello', + (pty) => pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }), + shellExecutionConfigWithoutPager, + ); + + const spawnOptions = mockPtySpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe(''); + expect(spawnOptions.env['GIT_PAGER']).toBe(''); + mockGetShellConfiguration.mockReturnValue({ + executable: 'bash', + argsPrefix: ['-c'], + shell: 'bash', + }); + }); + + it('preserves explicit pager configuration in Windows pty env', async () => { + mockPlatform.mockReturnValue('win32'); + mockGetShellConfiguration.mockReturnValue({ + executable: 'cmd.exe', + argsPrefix: ['/d', '/s', '/c'], + shell: 'cmd', + }); + + await simulateExecution('echo hello', (pty) => + pty.onExit.mock.calls[0][0]({ exitCode: 0, signal: null }), + ); + + const spawnOptions = mockPtySpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBe('cat'); + mockGetShellConfiguration.mockReturnValue({ + executable: 'bash', + argsPrefix: ['-c'], + shell: 'bash', + }); + }); + it('should use bash on Linux', async () => { mockPlatform.mockReturnValue('linux'); await simulateExecution('ls "foo bar"', (pty) => @@ -3163,6 +3230,54 @@ describe('ShellExecutionService child_process fallback', () => { expectNormalizedWindowsPathEnv(spawnOptions.env); }); + it('does not inject Unix pager defaults into Windows child_process env when unset', async () => { + mockPlatform.mockReturnValue('win32'); + mockGetShellConfiguration.mockReturnValue({ + executable: 'cmd.exe', + argsPrefix: ['/d', '/s', '/c'], + shell: 'cmd', + }); + + await withoutGitPagerEnv(() => + simulateExecutionWithConfig( + 'echo hello', + (cp) => cp.emit('exit', 0, null), + shellExecutionConfigWithoutPager, + ), + ); + + const spawnOptions = mockCpSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe(''); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); + mockGetShellConfiguration.mockReturnValue({ + executable: 'bash', + argsPrefix: ['-c'], + shell: 'bash', + }); + }); + + it('preserves explicit pager configuration in Windows child_process env', async () => { + mockPlatform.mockReturnValue('win32'); + mockGetShellConfiguration.mockReturnValue({ + executable: 'cmd.exe', + argsPrefix: ['/d', '/s', '/c'], + shell: 'cmd', + }); + + await withoutGitPagerEnv(() => + simulateExecution('echo hello', (cp) => cp.emit('exit', 0, null)), + ); + + const spawnOptions = mockCpSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBeUndefined(); + mockGetShellConfiguration.mockReturnValue({ + executable: 'bash', + argsPrefix: ['-c'], + shell: 'bash', + }); + }); + it('should use bash and detached process group on Linux', async () => { mockPlatform.mockReturnValue('linux'); await simulateExecution('ls "foo bar"', (cp) => cp.emit('exit', 0, null)); diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 12acb22c21..a4fac7823a 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -24,6 +24,7 @@ import { normalizePathEnvForWindows } from '../utils/windowsPath.js'; import { formatMemoryUsage } from '../utils/formatters.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { getShellPagerEnv } from '../utils/shell-pager-env.js'; const { Terminal } = pkg; const debugLogger = createDebugLogger('SHELL_EXECUTION'); @@ -683,6 +684,7 @@ export class ShellExecutionService { abortSignal, options.streamStdout ?? false, getMaxBufferedOutputBytes(shellExecutionConfig), + shellExecutionConfig.pager, options.postPromote, ); } @@ -694,6 +696,7 @@ export class ShellExecutionService { abortSignal: AbortSignal, streamStdout: boolean, maxBufferedOutputBytes: number, + pager: string | undefined, postPromote?: ShellPostPromoteHandlers, ): ShellExecutionHandle { try { @@ -719,7 +722,10 @@ export class ShellExecutionService { ...normalizePathEnvForWindows(process.env), QWEN_CODE: '1', TERM: 'xterm-256color', - PAGER: 'cat', + ...getShellPagerEnv(pager, { + includeGitPager: false, + platform: os.platform(), + }), ...getShellContextEnvVars(), }, }); @@ -1419,8 +1425,10 @@ export class ShellExecutionService { ...normalizePathEnvForWindows(process.env), QWEN_CODE: '1', TERM: 'xterm-256color', - PAGER: shellExecutionConfig.pager ?? 'cat', - GIT_PAGER: shellExecutionConfig.pager ?? 'cat', + ...getShellPagerEnv(shellExecutionConfig.pager, { + includeGitPager: true, + platform: os.platform(), + }), ...getShellContextEnvVars(), }, handleFlowControl: true, diff --git a/packages/core/src/tools/monitor.test.ts b/packages/core/src/tools/monitor.test.ts index 53a6960572..177d5f3719 100644 --- a/packages/core/src/tools/monitor.test.ts +++ b/packages/core/src/tools/monitor.test.ts @@ -9,6 +9,22 @@ import { EventEmitter } from 'node:events'; import type { Readable } from 'node:stream'; import type { ChildProcess } from 'node:child_process'; +const mockOsPlatform = vi.hoisted(() => + vi.fn<() => NodeJS.Platform>(() => 'linux'), +); +vi.mock('node:os', async (importOriginal) => { + const actual = await importOriginal(); + + return { + ...actual, + default: { + ...actual, + platform: mockOsPlatform, + }, + platform: mockOsPlatform, + }; +}); + // Mock child_process.spawn const mockSpawn = vi.hoisted(() => vi.fn()); vi.mock('node:child_process', async (importOriginal) => { @@ -178,6 +194,7 @@ describe('MonitorTool', () => { beforeEach(() => { vi.clearAllMocks(); + mockOsPlatform.mockReturnValue('linux'); monitorRegistry = new MonitorRegistry(); mockIsPathWithinWorkspace = vi.fn().mockReturnValue(true); @@ -195,6 +212,7 @@ describe('MonitorTool', () => { isPathWithinWorkspace: mockIsPathWithinWorkspace, }), getSessionId: vi.fn().mockReturnValue('test-session-id'), + getShellExecutionConfig: vi.fn().mockReturnValue({}), storage: { getUserSkillsDirs: vi .fn() @@ -655,6 +673,33 @@ describe('MonitorTool', () => { expect(result.returnDisplay).toContain('watch app logs'); }); + it('uses default pager env for spawned processes when pager is unset', async () => { + const invocation = createInvocation({ + command: 'tail -f /var/log/app.log', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('cat'); + expect(spawnOptions.env['GIT_PAGER']).toBe('cat'); + }); + + it('propagates explicit pager configuration to spawned processes', async () => { + vi.mocked(mockConfig.getShellExecutionConfig).mockReturnValue({ + pager: 'more', + }); + const invocation = createInvocation({ + command: 'tail -f /var/log/app.log', + }); + + await invocation.execute(new AbortController().signal); + + const spawnOptions = mockSpawn.mock.calls[0][2]; + expect(spawnOptions.env['PAGER']).toBe('more'); + expect(spawnOptions.env['GIT_PAGER']).toBe('more'); + }); + it('does not spawn when the turn signal is already aborted', async () => { const invocation = createInvocation({ command: 'tail -f /var/log/app.log', diff --git a/packages/core/src/tools/monitor.ts b/packages/core/src/tools/monitor.ts index 21ace5d81e..0e5c1a4b5c 100644 --- a/packages/core/src/tools/monitor.ts +++ b/packages/core/src/tools/monitor.ts @@ -17,6 +17,7 @@ */ import { spawn } from 'node:child_process'; +import os from 'node:os'; import path from 'node:path'; import { randomUUID } from 'node:crypto'; import stripAnsi from 'strip-ansi'; @@ -54,6 +55,7 @@ import { } from '../utils/shellAstParser.js'; import { getCurrentAgentId } from '../agents/runtime/agent-context.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; +import { getShellPagerEnv } from '../utils/shell-pager-env.js'; const debugLogger = createDebugLogger('MONITOR'); @@ -366,7 +368,10 @@ class MonitorToolInvocation extends BaseToolInvocation< ...process.env, QWEN_CODE: '1', TERM: 'dumb', // no color codes for streaming - PAGER: 'cat', + ...getShellPagerEnv(this.config.getShellExecutionConfig().pager, { + includeGitPager: true, + platform: os.platform(), + }), ...getShellContextEnvVars(), }, }); diff --git a/packages/core/src/utils/shell-pager-env.test.ts b/packages/core/src/utils/shell-pager-env.test.ts new file mode 100644 index 0000000000..ef10aa8475 --- /dev/null +++ b/packages/core/src/utils/shell-pager-env.test.ts @@ -0,0 +1,64 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { getDefaultShellPager, getShellPagerEnv } from './shell-pager-env.js'; + +describe('shellPagerEnv', () => { + it('defaults to cat on non-Windows platforms', () => { + expect(getDefaultShellPager('linux')).toBe('cat'); + expect(getDefaultShellPager('darwin')).toBe('cat'); + }); + + it('does not default to Unix-only cat on Windows', () => { + expect(getDefaultShellPager('win32')).toBeUndefined(); + expect(getShellPagerEnv(undefined, { platform: 'win32' })).toEqual({ + PAGER: '', + }); + }); + + it('clears inherited git pager values when requested without an effective pager', () => { + expect( + getShellPagerEnv(undefined, { + includeGitPager: true, + platform: 'win32', + }), + ).toEqual({ + PAGER: '', + GIT_PAGER: '', + }); + }); + + it('sets both PAGER and GIT_PAGER to cat on non-Windows when pager is unset', () => { + expect( + getShellPagerEnv(undefined, { + includeGitPager: true, + platform: 'linux', + }), + ).toEqual({ + PAGER: 'cat', + GIT_PAGER: 'cat', + }); + }); + + it('treats an empty pager as an explicit request to disable pager env values', () => { + expect( + getShellPagerEnv('', { includeGitPager: true, platform: 'linux' }), + ).toEqual({ + PAGER: '', + GIT_PAGER: '', + }); + }); + + it('preserves explicit pager configuration on Windows', () => { + expect( + getShellPagerEnv('more', { includeGitPager: true, platform: 'win32' }), + ).toEqual({ + PAGER: 'more', + GIT_PAGER: 'more', + }); + }); +}); diff --git a/packages/core/src/utils/shell-pager-env.ts b/packages/core/src/utils/shell-pager-env.ts new file mode 100644 index 0000000000..ed7ca6894f --- /dev/null +++ b/packages/core/src/utils/shell-pager-env.ts @@ -0,0 +1,32 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +export function getDefaultShellPager( + platform: NodeJS.Platform = process.platform, +): string | undefined { + return platform === 'win32' ? undefined : 'cat'; +} + +export function getShellPagerEnv( + pager: string | undefined, + options: { + includeGitPager?: boolean; + platform?: NodeJS.Platform; + } = {}, +): NodeJS.ProcessEnv { + const effectivePager = pager ?? getDefaultShellPager(options.platform); + if (!effectivePager) { + return { + PAGER: '', + ...(options.includeGitPager ? { GIT_PAGER: '' } : {}), + }; + } + + return { + PAGER: effectivePager, + ...(options.includeGitPager ? { GIT_PAGER: effectivePager } : {}), + }; +} diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 2fdcb1e6a3..a26d9fe059 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -1011,9 +1011,8 @@ "default": true }, "pager": { - "description": "The pager command to use for shell output. Defaults to `cat`.", - "type": "string", - "default": "cat" + "description": "The pager command to use for shell output. Defaults to `cat` on non-Windows platforms and unset on Windows. Set to an empty string to disable pager environment variables.", + "type": "string" }, "showColor": { "description": "Show color in shell output.",