Compare commits

...

14 Commits

Author SHA1 Message Date
xuewenjie
8673426d5c fix(core): use current chunk for shell output update instead of cumulative 2025-12-16 10:26:20 +08:00
xuewenjie
b272ac0119 Fix: Make cleanup strategy dynamic to support testing mocks 2025-12-12 17:47:03 +08:00
xuewenjie
574d89da14 Refactor ShellExecutionService cleanup to use strategy pattern 2025-12-12 17:03:04 +08:00
xuewenjie
16939c0bc8 Refactor ShellTool: remove ping hack and timeout, optimize cleanup 2025-12-10 13:49:51 +08:00
xuewenjie
6fc09a82fb fix: use && for windows background keep-alive ping and add test 2025-12-09 13:33:42 +08:00
xuewenjie
d622f8d1bf Merge branch 'main' of github.com:QwenLM/qwen-code into fix/windows-background-terminal-execute-x 2025-12-09 11:32:17 +08:00
xuewenjie
28d178b5c1 fix: handle windows background execution errors and add tests 2025-12-09 11:24:30 +08:00
tanzhenxin
5fddcd509c pump versionm to 0.4.1 (#1177) 2025-12-09 10:25:07 +08:00
pomelo
a6a572336c Merge pull request #1157 from QwenLM/fix/windows-shell-output-garbled 2025-12-08 16:45:05 +08:00
xuewenjie
e5e1e6a3da Merge branch 'main' of github.com:QwenLM/qwen-code into fix/windows-shell-output-garbled 2025-12-08 11:35:47 +08:00
xuewenjie
2c1a836f18 fix: prefer UTF-8 encoding for shell output on Windows when detected 2025-12-05 16:49:26 +08:00
xuewenjie
4c69d536ac test: fix shell tool tests by updating pid expectation and AbortSignal matching 2025-12-05 10:47:06 +08:00
xuewenjie
403fd06117 chore: update .gitignore 2025-12-04 15:55:17 +08:00
xuewenjie
d9928eab66 fix: improve windows background process handling and cleanup 2025-12-04 15:55:11 +08:00
12 changed files with 262 additions and 40 deletions

12
package-lock.json generated
View File

@@ -1,12 +1,12 @@
{
"name": "@qwen-code/qwen-code",
"version": "0.4.0",
"version": "0.4.1",
"lockfileVersion": 3,
"requires": true,
"packages": {
"": {
"name": "@qwen-code/qwen-code",
"version": "0.4.0",
"version": "0.4.1",
"workspaces": [
"packages/*"
],
@@ -16422,7 +16422,7 @@
},
"packages/cli": {
"name": "@qwen-code/qwen-code",
"version": "0.4.0",
"version": "0.4.1",
"dependencies": {
"@google/genai": "1.16.0",
"@iarna/toml": "^2.2.5",
@@ -16537,7 +16537,7 @@
},
"packages/core": {
"name": "@qwen-code/qwen-code-core",
"version": "0.4.0",
"version": "0.4.1",
"hasInstallScript": true,
"dependencies": {
"@google/genai": "1.16.0",
@@ -19106,7 +19106,7 @@
},
"packages/test-utils": {
"name": "@qwen-code/qwen-code-test-utils",
"version": "0.4.0",
"version": "0.4.1",
"dev": true,
"license": "Apache-2.0",
"devDependencies": {
@@ -19118,7 +19118,7 @@
},
"packages/vscode-ide-companion": {
"name": "qwen-code-vscode-ide-companion",
"version": "0.4.0",
"version": "0.4.1",
"license": "LICENSE",
"dependencies": {
"@modelcontextprotocol/sdk": "^1.15.1",

View File

@@ -1,6 +1,6 @@
{
"name": "@qwen-code/qwen-code",
"version": "0.4.0",
"version": "0.4.1",
"engines": {
"node": ">=20.0.0"
},
@@ -13,7 +13,7 @@
"url": "git+https://github.com/QwenLM/qwen-code.git"
},
"config": {
"sandboxImageUri": "ghcr.io/qwenlm/qwen-code:0.4.0"
"sandboxImageUri": "ghcr.io/qwenlm/qwen-code:0.4.1"
},
"scripts": {
"start": "cross-env node scripts/start.js",

View File

@@ -1,6 +1,6 @@
{
"name": "@qwen-code/qwen-code",
"version": "0.4.0",
"version": "0.4.1",
"description": "Qwen Code",
"repository": {
"type": "git",
@@ -33,7 +33,7 @@
"dist"
],
"config": {
"sandboxImageUri": "ghcr.io/qwenlm/qwen-code:0.4.0"
"sandboxImageUri": "ghcr.io/qwenlm/qwen-code:0.4.1"
},
"dependencies": {
"@google/genai": "1.16.0",

View File

@@ -1,6 +1,6 @@
{
"name": "@qwen-code/qwen-code-core",
"version": "0.4.0",
"version": "0.4.1",
"description": "Qwen Code Core",
"repository": {
"type": "git",

View File

@@ -589,7 +589,7 @@ describe('ShellExecutionService child_process fallback', () => {
expect(result.error).toBeNull();
expect(result.aborted).toBe(false);
expect(result.output).toBe('file1.txt\na warning');
expect(handle.pid).toBe(undefined);
expect(handle.pid).toBe(12345);
expect(onOutputEventMock).toHaveBeenCalledWith({
type: 'data',

View File

@@ -7,7 +7,7 @@
import stripAnsi from 'strip-ansi';
import type { PtyImplementation } from '../utils/getPty.js';
import { getPty } from '../utils/getPty.js';
import { spawn as cpSpawn } from 'node:child_process';
import { spawn as cpSpawn, spawnSync } from 'node:child_process';
import { TextDecoder } from 'node:util';
import os from 'node:os';
import type { IPty } from '@lydell/node-pty';
@@ -98,6 +98,48 @@ const getFullBufferText = (terminal: pkg.Terminal): string => {
return lines.join('\n').trimEnd();
};
interface ProcessCleanupStrategy {
killPty(pid: number, pty: ActivePty): void;
killChildProcesses(pids: Set<number>): void;
}
const windowsStrategy: ProcessCleanupStrategy = {
killPty: (_pid, pty) => {
pty.ptyProcess.kill();
},
killChildProcesses: (pids) => {
if (pids.size > 0) {
try {
const args = ['/f', '/t'];
for (const pid of pids) {
args.push('/pid', pid.toString());
}
spawnSync('taskkill', args);
} catch {
// ignore
}
}
},
};
const posixStrategy: ProcessCleanupStrategy = {
killPty: (pid, _pty) => {
process.kill(-pid, 'SIGKILL');
},
killChildProcesses: (pids) => {
for (const pid of pids) {
try {
process.kill(-pid, 'SIGKILL');
} catch {
// ignore
}
}
},
};
const getCleanupStrategy = () =>
os.platform() === 'win32' ? windowsStrategy : posixStrategy;
/**
* A centralized service for executing shell commands with robust process
* management, cross-platform compatibility, and streaming output capabilities.
@@ -106,6 +148,29 @@ const getFullBufferText = (terminal: pkg.Terminal): string => {
export class ShellExecutionService {
private static activePtys = new Map<number, ActivePty>();
private static activeChildProcesses = new Set<number>();
static cleanup() {
const strategy = getCleanupStrategy();
// Cleanup PTYs
for (const [pid, pty] of this.activePtys) {
try {
strategy.killPty(pid, pty);
} catch {
// ignore
}
}
// Cleanup child processes
strategy.killChildProcesses(this.activeChildProcesses);
}
static {
process.on('exit', () => {
ShellExecutionService.cleanup();
});
}
/**
* Executes a shell command using `node-pty`, capturing all output and lifecycle events.
*
@@ -281,9 +346,13 @@ export class ShellExecutionService {
abortSignal.addEventListener('abort', abortHandler, { once: true });
if (child.pid) {
this.activeChildProcesses.add(child.pid);
}
child.on('exit', (code, signal) => {
if (child.pid) {
this.activePtys.delete(child.pid);
this.activeChildProcesses.delete(child.pid);
}
handleExit(code, signal);
});
@@ -310,7 +379,7 @@ export class ShellExecutionService {
}
});
return { pid: undefined, result };
return { pid: child.pid, result };
} catch (e) {
const error = e as Error;
return {

View File

@@ -210,7 +210,7 @@ describe('ShellTool', () => {
wrappedCommand,
'/test/dir',
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -237,7 +237,7 @@ describe('ShellTool', () => {
wrappedCommand,
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -262,7 +262,7 @@ describe('ShellTool', () => {
wrappedCommand,
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -287,7 +287,7 @@ describe('ShellTool', () => {
wrappedCommand,
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -312,7 +312,7 @@ describe('ShellTool', () => {
wrappedCommand,
'/test/dir/subdir',
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -340,7 +340,7 @@ describe('ShellTool', () => {
'dir',
'/test/dir',
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -433,7 +433,7 @@ describe('ShellTool', () => {
expect(summarizer.summarizeToolOutput).toHaveBeenCalledWith(
expect.any(String),
mockConfig.getGeminiClient(),
mockAbortSignal,
expect.any(AbortSignal),
1000,
);
expect(result.llmContent).toBe('summarized output');
@@ -542,7 +542,7 @@ describe('ShellTool', () => {
),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -572,7 +572,7 @@ describe('ShellTool', () => {
),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -602,7 +602,7 @@ describe('ShellTool', () => {
),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -631,7 +631,7 @@ describe('ShellTool', () => {
expect.stringContaining('npm install'),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -660,7 +660,7 @@ describe('ShellTool', () => {
expect.stringContaining('git commit'),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -690,7 +690,7 @@ describe('ShellTool', () => {
),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -726,7 +726,7 @@ describe('ShellTool', () => {
expect.stringContaining('git commit -m "Initial commit"'),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -763,7 +763,7 @@ describe('ShellTool', () => {
),
expect.any(String),
expect.any(Function),
mockAbortSignal,
expect.any(AbortSignal),
false,
{},
);
@@ -831,4 +831,68 @@ describe('ShellTool', () => {
expect(shellTool.description).toMatchSnapshot();
});
});
describe('Windows background execution', () => {
it('should clean up trailing ampersand on Windows for background tasks', async () => {
vi.mocked(os.platform).mockReturnValue('win32');
const mockAbortSignal = new AbortController().signal;
const invocation = shellTool.build({
command: 'npm start &',
is_background: true,
});
const promise = invocation.execute(mockAbortSignal);
// Simulate immediate success (process started)
resolveExecutionPromise({
rawOutput: Buffer.from(''),
output: '',
exitCode: 0,
signal: null,
error: null,
aborted: false,
pid: 12345,
executionMethod: 'child_process',
});
await promise;
expect(mockShellExecutionService).toHaveBeenCalledWith(
'npm start',
expect.any(String),
expect.any(Function),
expect.any(AbortSignal),
false,
{},
);
});
it('should detect immediate failure in Windows background task', async () => {
vi.mocked(os.platform).mockReturnValue('win32');
const mockAbortSignal = new AbortController().signal;
const invocation = shellTool.build({
command: 'invalid_command',
is_background: true,
});
const promise = invocation.execute(mockAbortSignal);
// Wait a tick to ensure mockShellOutputCallback is assigned
await new Promise((resolve) => setTimeout(resolve, 0));
if (mockShellOutputCallback) {
mockShellOutputCallback({
type: 'data',
chunk:
"'invalid_command' is not recognized as an internal or external command,\r\noperable program or batch file.\r\n",
});
}
const result = await promise;
expect(result.error).toBeDefined();
expect(result.llmContent).toContain('Command failed to start');
});
});
});

View File

@@ -30,6 +30,7 @@ import { summarizeToolOutput } from '../utils/summarizer.js';
import type {
ShellExecutionConfig,
ShellOutputEvent,
ShellExecutionResult,
} from '../services/shellExecutionService.js';
import { ShellExecutionService } from '../services/shellExecutionService.js';
import { formatMemoryUsage } from '../utils/formatters.js';
@@ -143,11 +144,29 @@ export class ShellToolInvocation extends BaseToolInvocation<
const shouldRunInBackground = this.params.is_background;
let finalCommand = processedCommand;
// If explicitly marked as background and doesn't already end with &, add it
if (shouldRunInBackground && !finalCommand.trim().endsWith('&')) {
// On non-Windows, use & to run in background.
// On Windows, we don't use start /B because it creates a detached process that
// doesn't die when the parent dies. Instead, we rely on the race logic below
// to return early while keeping the process attached (detached: false).
if (
!isWindows &&
shouldRunInBackground &&
!finalCommand.trim().endsWith('&')
) {
finalCommand = finalCommand.trim() + ' &';
}
// On Windows, we rely on the race logic below to handle background tasks.
// We just ensure the command string is clean.
if (isWindows && shouldRunInBackground) {
let cmd = finalCommand.trim();
// Remove trailing & (common Linux habit, invalid on Windows at end of line)
while (cmd.endsWith('&')) {
cmd = cmd.slice(0, -1).trim();
}
finalCommand = cmd;
}
// pgrep is not available on Windows, so we can't get background PIDs
const commandToExecute = isWindows
? finalCommand
@@ -169,10 +188,6 @@ export class ShellToolInvocation extends BaseToolInvocation<
commandToExecute,
cwd,
(event: ShellOutputEvent) => {
if (!updateOutput) {
return;
}
let shouldUpdate = false;
switch (event.type) {
@@ -201,7 +216,7 @@ export class ShellToolInvocation extends BaseToolInvocation<
}
}
if (shouldUpdate) {
if (shouldUpdate && updateOutput) {
updateOutput(
typeof cumulativeOutput === 'string'
? cumulativeOutput
@@ -219,7 +234,59 @@ export class ShellToolInvocation extends BaseToolInvocation<
setPidCallback(pid);
}
const result = await resultPromise;
let result: ShellExecutionResult;
if (shouldRunInBackground && isWindows) {
// For Windows background tasks, we wait a short time to catch immediate errors.
// If it's still running, we return early.
const startupDelay = 1000;
const raceResult = await Promise.race([
resultPromise,
new Promise<null>((resolve) =>
setTimeout(() => resolve(null), startupDelay),
),
]);
if (raceResult === null) {
// Timeout reached, process is still running.
// throw new Error(`DEBUG: raceResult is null. Output: ${JSON.stringify(cumulativeOutput)}`);
// Check for common Windows error messages in the output
const outputStr =
typeof cumulativeOutput === 'string'
? cumulativeOutput
: JSON.stringify(cumulativeOutput);
console.log('DEBUG: outputStr:', outputStr);
const errorPatterns = [
'is not recognized as an internal or external command',
'The system cannot find the path specified',
'Access is denied',
];
if (errorPatterns.some((pattern) => outputStr.includes(pattern))) {
return {
llmContent: `Command failed to start: ${outputStr}`,
returnDisplay: `Command failed to start: ${outputStr}`,
error: {
type: ToolErrorType.EXECUTION_FAILED,
message: `Command failed to start: ${outputStr}`,
},
};
}
const pidMsg = pid ? ` PID: ${pid}` : '';
const winHint = isWindows
? ' (Note: Use taskkill /F /T /PID <pid> to stop)'
: '';
return {
llmContent: `Background command started.${pidMsg}${winHint}`,
returnDisplay: `Background command started.${pidMsg}${winHint}`,
};
} else {
result = raceResult;
}
} else {
result = await resultPromise;
}
const backgroundPIDs: number[] = [];
if (os.platform() !== 'win32') {

View File

@@ -391,6 +391,19 @@ describe('Shell Command Processor - Encoding Functions', () => {
expect(result).toBe('windows-1252');
});
it('should prioritize UTF-8 detection over Windows system encoding', () => {
mockedOsPlatform.mockReturnValue('win32');
mockedExecSync.mockReturnValue('Active code page: 936'); // GBK
const buffer = Buffer.from('test');
// Mock chardet to return UTF-8
mockedChardetDetect.mockReturnValue('UTF-8');
const result = getCachedEncodingForBuffer(buffer);
expect(result).toBe('utf-8');
});
it('should cache null system encoding result', () => {
// Reset the cache specifically for this test
resetEncodingCache();

View File

@@ -34,6 +34,15 @@ export function getCachedEncodingForBuffer(buffer: Buffer): string {
// If we have a cached system encoding, use it
if (cachedSystemEncoding) {
// If the system encoding is not UTF-8 (e.g. Windows CP936), but the buffer
// is detected as UTF-8, prefer UTF-8. This handles tools like 'git' which
// often output UTF-8 regardless of the system code page.
if (cachedSystemEncoding !== 'utf-8') {
const detected = detectEncodingFromBuffer(buffer);
if (detected === 'utf-8') {
return 'utf-8';
}
}
return cachedSystemEncoding;
}

View File

@@ -1,6 +1,6 @@
{
"name": "@qwen-code/qwen-code-test-utils",
"version": "0.4.0",
"version": "0.4.1",
"private": true,
"main": "src/index.ts",
"license": "Apache-2.0",

View File

@@ -2,7 +2,7 @@
"name": "qwen-code-vscode-ide-companion",
"displayName": "Qwen Code Companion",
"description": "Enable Qwen Code with direct access to your VS Code workspace.",
"version": "0.4.0",
"version": "0.4.1",
"publisher": "qwenlm",
"icon": "assets/icon.png",
"repository": {