Merge branch 'main' of github.com:QwenLM/qwen-code into fix/windows-background-terminal-execute-x

docs/users/overview.md

@@ -1,6 +1,4 @@
 # Qwen Code overview
-[![@qwen-code/qwen-code downloads](https://img.shields.io/npm/dw/@qwen-code/qwen-code.svg)](https://npm-compare.com/@qwen-code/qwen-code) 
-[![@qwen-code/qwen-code version](https://img.shields.io/npm/v/@qwen-code/qwen-code.svg)](https://www.npmjs.com/package/@qwen-code/qwen-code)
 
 > Learn about Qwen Code, Qwen's agentic coding tool that lives in your terminal and helps you turn ideas into code faster than ever before.
 
@@ -48,7 +46,7 @@ You'll be prompted to log in on first use. That's it! [Continue with Quickstart
 
 > [!note]
 >
-> **New VS Code Extension (Beta)**: Prefer a graphical interface? Our new **VS Code extension** provides an easy-to-use native IDE experience without requiring terminal familiarity. Simply install from the marketplace and start coding with Qwen Code directly in your sidebar. Download and install the [Qwen Code Companion](https://marketplace.visualstudio.com/items?itemName=qwenlm.qwen-code-vscode-ide-companion) now.
+> **New VS Code Extension (Beta)**: Prefer a graphical interface? Our new **VS Code extension** provides an easy-to-use native IDE experience without requiring terminal familiarity. Simply install from the marketplace and start coding with Qwen Code directly in your sidebar. You can search for **Qwen Code** in the VS Code Marketplace and download it.
 
 ## What Qwen Code does for you
 

integration-tests/file-system.test.ts

@@ -5,6 +5,8 @@
  */
 
 import { describe, it, expect } from 'vitest';
+import { existsSync } from 'node:fs';
+import * as path from 'node:path';
 import { TestRig, printDebugInfo, validateModelOutput } from './test-helper.js';
 
 describe('file-system', () => {
@@ -243,5 +245,12 @@ describe('file-system', () => {
       successfulReplace,
       'A successful replace should not have occurred',
     ).toBeUndefined();
+
+    // Final verification: ensure the file was not created.
+    const filePath = path.join(rig.testDir!, fileName);
+    const fileExists = existsSync(filePath);
+    expect(fileExists, 'The non-existent file should not be created').toBe(
+      false,
+    );
   });
 });

packages/cli/src/ui/hooks/useLaunchEditor.ts

@@ -7,76 +7,15 @@
 import { useCallback } from 'react';
 import { useStdin } from 'ink';
 import type { EditorType } from '@qwen-code/qwen-code-core';
-import { spawnSync, execSync } from 'child_process';
+import { spawnSync } from 'child_process';
 import { useSettings } from '../contexts/SettingsContext.js';
 
-/**
- * Editor command configurations for different platforms.
- * Each editor can have multiple possible command names, listed in order of preference.
- */
-const editorCommands: Record<
-  EditorType,
-  { win32: string[]; default: string[] }
-> = {
-  vscode: { win32: ['code.cmd'], default: ['code'] },
-  vscodium: { win32: ['codium.cmd'], default: ['codium'] },
-  windsurf: { win32: ['windsurf'], default: ['windsurf'] },
-  cursor: { win32: ['cursor'], default: ['cursor'] },
-  vim: { win32: ['vim'], default: ['vim'] },
-  neovim: { win32: ['nvim'], default: ['nvim'] },
-  zed: { win32: ['zed'], default: ['zed', 'zeditor'] },
-  emacs: { win32: ['emacs.exe'], default: ['emacs'] },
-  trae: { win32: ['trae'], default: ['trae'] },
-};
-
-/**
- * Cache for command existence checks to avoid repeated execSync calls.
- */
-const commandExistsCache = new Map<string, boolean>();
-
-/**
- * Check if a command exists in the system.
- * Results are cached to improve performance in test environments.
- */
-function commandExists(cmd: string): boolean {
-  if (commandExistsCache.has(cmd)) {
-    return commandExistsCache.get(cmd)!;
-  }
-
-  try {
-    execSync(
-      process.platform === 'win32' ? `where.exe ${cmd}` : `command -v ${cmd}`,
-      { stdio: 'ignore' },
-    );
-    commandExistsCache.set(cmd, true);
-    return true;
-  } catch {
-    commandExistsCache.set(cmd, false);
-    return false;
-  }
-}
-
-/**
- * Get the actual executable command for an editor type.
- */
-function getExecutableCommand(editorType: EditorType): string {
-  const commandConfig = editorCommands[editorType];
-  const commands =
-    process.platform === 'win32' ? commandConfig.win32 : commandConfig.default;
-
-  // Try to find the first available command
-  const availableCommand = commands.find((cmd) => commandExists(cmd));
-
-  // Return the first available command, or fall back to the last one in the list
-  return availableCommand || commands[commands.length - 1];
-}
-
 /**
  * Determines the editor command to use based on user preferences and platform.
  */
 function getEditorCommand(preferredEditor?: EditorType): string {
   if (preferredEditor) {
-    return getExecutableCommand(preferredEditor);
+    return preferredEditor;
   }
 
   // Platform-specific defaults with UI preference for macOS
@@ -124,14 +63,8 @@ export function useLaunchEditor() {
       try {
         setRawMode?.(false);
 
-        // On Windows, .cmd and .bat files need shell: true
-        const needsShell =
-          process.platform === 'win32' &&
-          (editorCommand.endsWith('.cmd') || editorCommand.endsWith('.bat'));
-
         const { status, error } = spawnSync(editorCommand, editorArgs, {
           stdio: 'inherit',
-          shell: needsShell,
         });
 
         if (error) throw error;

packages/core/src/services/shellExecutionService.test.ts

@@ -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',

packages/core/src/services/shellExecutionService.ts

@@ -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 {

packages/core/src/tools/shell.test.ts

@@ -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,
           {},
         );
@@ -661,7 +661,7 @@ describe('ShellTool', () => {
           expect.stringContaining('npm install'),
           expect.any(String),
           expect.any(Function),
-          mockAbortSignal,
+          expect.any(AbortSignal),
           false,
           {},
         );
@@ -690,7 +690,7 @@ describe('ShellTool', () => {
           expect.stringContaining('git commit'),
           expect.any(String),
           expect.any(Function),
-          mockAbortSignal,
+          expect.any(AbortSignal),
           false,
           {},
         );
@@ -720,7 +720,7 @@ describe('ShellTool', () => {
           ),
           expect.any(String),
           expect.any(Function),
-          mockAbortSignal,
+          expect.any(AbortSignal),
           false,
           {},
         );
@@ -756,7 +756,7 @@ describe('ShellTool', () => {
           expect.stringContaining('git commit -m "Initial commit"'),
           expect.any(String),
           expect.any(Function),
-          mockAbortSignal,
+          expect.any(AbortSignal),
           false,
           {},
         );
@@ -793,7 +793,7 @@ describe('ShellTool', () => {
           ),
           expect.any(String),
           expect.any(Function),
-          mockAbortSignal,
+          expect.any(AbortSignal),
           false,
           {},
         );
@@ -924,4 +924,68 @@ spanning multiple lines"`;
       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');
+    });
+  });
 });

packages/core/src/tools/shell.ts

@@ -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') {