fix(desktop): handle missing Windows taskkill #1091

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', () => {

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()