From 8e9c104514b5d4a2bfb26672244c2968b294ff13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F=28Relakkes?= =?UTF-8?q?=29?= Date: Fri, 24 Jul 2026 05:13:36 +0800 Subject: [PATCH] fix(desktop): handle missing Windows taskkill #1091 --- .../electron/services/sidecarManager.test.ts | 83 +++++++++++++++++-- desktop/electron/services/sidecarManager.ts | 41 ++++++++- 2 files changed, 116 insertions(+), 8 deletions(-) diff --git a/desktop/electron/services/sidecarManager.test.ts b/desktop/electron/services/sidecarManager.test.ts index 33bb8891..1ad1fa2e 100644 --- a/desktop/electron/services/sidecarManager.test.ts +++ b/desktop/electron/services/sidecarManager.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest' +import { EventEmitter } from 'node:events' import net from 'node:net' import http from 'node:http' import path from 'node:path' @@ -24,6 +25,7 @@ import { reserveServerPort, resolveBundledRipgrepExecutable, resolveHostTriple, + resolveWindowsTaskkillExecutable, RIPGREP_PATH_ENV, SERVER_STATE_FILE, SYSTEM_PROXY_BRIDGE_ENV, @@ -383,21 +385,90 @@ describe('Electron sidecar manager', () => { it('uses async taskkill on Windows by default', () => { const child = fakeChild(777) - const spawnAsync = vi.fn() + const taskkill = new EventEmitter() + const spawnAsync = vi.fn(() => taskkill) const spawnSyncFn = vi.fn() - killSidecar(child, false, { platform: 'win32', spawnAsync: spawnAsync as never, spawnSyncFn: spawnSyncFn as never }) - expect(spawnAsync).toHaveBeenCalledWith('taskkill', ['/F', '/T', '/PID', '777'], { stdio: 'ignore', windowsHide: true }) + killSidecar(child, false, { + platform: 'win32', + env: { SystemRoot: 'C:\\Windows' }, + spawnAsync: spawnAsync as never, + spawnSyncFn: spawnSyncFn as never, + }) + expect(spawnAsync).toHaveBeenCalledWith('C:\\Windows\\System32\\taskkill.exe', ['/F', '/T', '/PID', '777'], { stdio: 'ignore', windowsHide: true }) expect(spawnSyncFn).not.toHaveBeenCalled() expect(child.kill).not.toHaveBeenCalled() }) + it('falls back without crashing when async taskkill is unavailable on Windows', () => { + const child = fakeChild(777) + const taskkill = new EventEmitter() + const spawnAsync = vi.fn(() => taskkill) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + killSidecar(child, false, { + platform: 'win32', + env: { SystemRoot: 'C:\\Windows' }, + spawnAsync: spawnAsync as never, + }) + + const error = Object.assign(new Error('spawn taskkill ENOENT'), { code: 'ENOENT' }) + expect(() => taskkill.emit('error', error)).not.toThrow() + expect(child.kill).toHaveBeenCalledTimes(1) + expect(errorSpy).toHaveBeenCalledWith( + '[desktop] taskkill failed; falling back to direct sidecar termination', + error, + ) + } finally { + errorSpy.mockRestore() + } + }) + it('uses synchronous taskkill on Windows during shutdown to avoid orphaned sidecars', () => { const child = fakeChild(777) const spawnAsync = vi.fn() - const spawnSyncFn = vi.fn() - killSidecar(child, true, { platform: 'win32', spawnAsync: spawnAsync as never, spawnSyncFn: spawnSyncFn as never }) - expect(spawnSyncFn).toHaveBeenCalledWith('taskkill', ['/F', '/T', '/PID', '777'], { stdio: 'ignore', windowsHide: true }) + const spawnSyncFn = vi.fn(() => ({ error: undefined })) + killSidecar(child, true, { + platform: 'win32', + env: { SystemRoot: 'C:\\Windows' }, + spawnAsync: spawnAsync as never, + spawnSyncFn: spawnSyncFn as never, + }) + expect(spawnSyncFn).toHaveBeenCalledWith('C:\\Windows\\System32\\taskkill.exe', ['/F', '/T', '/PID', '777'], { stdio: 'ignore', windowsHide: true }) expect(spawnAsync).not.toHaveBeenCalled() + expect(child.kill).not.toHaveBeenCalled() + }) + + it('falls back when synchronous taskkill is unavailable on Windows', () => { + const child = fakeChild(777) + const error = Object.assign(new Error('spawnSync taskkill ENOENT'), { code: 'ENOENT' }) + const spawnSyncFn = vi.fn(() => ({ error })) + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + + try { + killSidecar(child, true, { + platform: 'win32', + env: { SystemRoot: 'C:\\Windows' }, + spawnSyncFn: spawnSyncFn as never, + }) + + expect(child.kill).toHaveBeenCalledTimes(1) + expect(errorSpy).toHaveBeenCalledWith( + '[desktop] taskkill failed; falling back to direct sidecar termination', + error, + ) + } finally { + errorSpy.mockRestore() + } + }) + + it('resolves taskkill from Windows system directories without relying on PATH', () => { + expect(resolveWindowsTaskkillExecutable({ SYSTEMROOT: 'D:\\Windows' })) + .toBe('D:\\Windows\\System32\\taskkill.exe') + expect(resolveWindowsTaskkillExecutable({ windir: 'E:\\WinDir' })) + .toBe('E:\\WinDir\\System32\\taskkill.exe') + expect(resolveWindowsTaskkillExecutable({})) + .toBe('taskkill.exe') }) it('hides Windows console windows when launching sidecars', () => { diff --git a/desktop/electron/services/sidecarManager.ts b/desktop/electron/services/sidecarManager.ts index e7483177..98fcfb00 100644 --- a/desktop/electron/services/sidecarManager.ts +++ b/desktop/electron/services/sidecarManager.ts @@ -588,10 +588,33 @@ export function spawnSidecar(plan: SidecarPlan, deps: SpawnSidecarDeps = {}): Si export type KillSidecarDeps = { platform?: NodeJS.Platform + env?: NodeJS.ProcessEnv spawnAsync?: typeof spawn spawnSyncFn?: typeof spawnSync } +function getWindowsEnv(env: NodeJS.ProcessEnv, name: string): string | undefined { + const normalizedName = name.toLowerCase() + return Object.entries(env) + .find(([key, value]) => key.toLowerCase() === normalizedName && value)?.[1] +} + +export function resolveWindowsTaskkillExecutable(env: NodeJS.ProcessEnv = process.env): string { + const systemRoot = getWindowsEnv(env, 'SystemRoot') ?? getWindowsEnv(env, 'windir') + return systemRoot + ? path.win32.join(systemRoot, 'System32', 'taskkill.exe') + : 'taskkill.exe' +} + +function fallbackToDirectSidecarKill(child: SidecarChild, error: unknown) { + console.error('[desktop] taskkill failed; falling back to direct sidecar termination', error) + try { + child.kill() + } catch (fallbackError) { + console.error('[desktop] direct sidecar termination failed', fallbackError) + } +} + /** * Terminate a sidecar process. On Windows we shell out to `taskkill /T` to also * reap the child process tree (the Bun sidecar spawns workers). Pass `sync=true` @@ -601,10 +624,24 @@ export type KillSidecarDeps = { export function killSidecar(child: SidecarChild, sync = false, deps: KillSidecarDeps = {}) { const platform = deps.platform ?? process.platform if (platform === 'win32' && child.pid) { + const command = resolveWindowsTaskkillExecutable(deps.env) const args = ['/F', '/T', '/PID', String(child.pid)] const options = { stdio: 'ignore', windowsHide: true } as const - if (sync) (deps.spawnSyncFn ?? spawnSync)('taskkill', args, options) - else (deps.spawnAsync ?? spawn)('taskkill', args, options) + if (sync) { + try { + const result = (deps.spawnSyncFn ?? spawnSync)(command, args, options) + if (result.error) fallbackToDirectSidecarKill(child, result.error) + } catch (error) { + fallbackToDirectSidecarKill(child, error) + } + } else { + try { + const killer = (deps.spawnAsync ?? spawn)(command, args, options) + killer.once('error', error => fallbackToDirectSidecarKill(child, error)) + } catch (error) { + fallbackToDirectSidecarKill(child, error) + } + } return } child.kill()