mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
fix(shell): avoid Unix pager default on Windows (#6390)
* fix(shell): avoid Unix pager default on Windows * fix(shell): clear inherited pager env on Windows * docs(shell): clarify platform-specific pager default * fix(shell): normalize pager env handling * fix(shell): preserve git pager fallback behavior * test(shell): stabilize pager env coverage
This commit is contained in:
parent
067cfbba62
commit
9ee8546a60
11 changed files with 305 additions and 12 deletions
|
|
@ -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`:**
|
||||
|
||||
|
|
|
|||
|
|
@ -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: {
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<unknown>): Promise<void> {
|
||||
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));
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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<typeof import('node:os')>();
|
||||
|
||||
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',
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
},
|
||||
});
|
||||
|
|
|
|||
64
packages/core/src/utils/shell-pager-env.test.ts
Normal file
64
packages/core/src/utils/shell-pager-env.test.ts
Normal file
|
|
@ -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',
|
||||
});
|
||||
});
|
||||
});
|
||||
32
packages/core/src/utils/shell-pager-env.ts
Normal file
32
packages/core/src/utils/shell-pager-env.ts
Normal file
|
|
@ -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 } : {}),
|
||||
};
|
||||
}
|
||||
|
|
@ -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.",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue