Parcourir la source

fix(windows): use cross-spawn for shim-backed commands (#18010)

Luke Parker il y a 1 mois
Parent
commit
54ed87d53c

+ 4 - 0
bun.lock

@@ -352,6 +352,7 @@
         "bun-pty": "0.4.8",
         "chokidar": "4.0.3",
         "clipboardy": "4.0.0",
+        "cross-spawn": "^7.0.6",
         "decimal.js": "10.5.0",
         "diff": "catalog:",
         "drizzle-orm": "1.0.0-beta.16-ea816b6",
@@ -401,6 +402,7 @@
         "@tsconfig/bun": "catalog:",
         "@types/babel__core": "7.20.5",
         "@types/bun": "catalog:",
+        "@types/cross-spawn": "6.0.6",
         "@types/mime-types": "3.0.1",
         "@types/semver": "^7.5.8",
         "@types/turndown": "5.0.5",
@@ -2059,6 +2061,8 @@
 
     "@types/connect": ["@types/[email protected]", "", { "dependencies": { "@types/node": "*" } }, "sha512-K6uROf1LD88uDQqJCktA4yzL1YYAK6NgfsI0v/mTgyPKWsX1CnJ0XPSDhViejru1GcRkLWb8RlzFYJRqGUbaug=="],
 
+    "@types/cross-spawn": ["@types/[email protected]", "", { "dependencies": { "@types/node": "*" } }, "sha512-fXRhhUkG4H3TQk5dBhQ7m/JDdSNHKwR2BBia62lhwEIq9xGiQKLxd6LymNhn47SjXhsUEPmxi+PKw2OkW4LLjA=="],
+
     "@types/debug": ["@types/[email protected]", "", { "dependencies": { "@types/ms": "*" } }, "sha512-vIChWdVG3LG1SMxEvI/AK+FWJthlrqlTu7fbrlywTkkaONwk/UAGaULXRlf8vkzFBLVm0zkMdCquhL5aOjhXPQ=="],
 
     "@types/deep-eql": ["@types/[email protected]", "", {}, "sha512-c9h9dVVMigMPc4bwTvC5dxqtqJZwQPePsWjPlpSOnojbor6pGqdk541lfA7AqFQr5pB1BRdq0juY9db81BwyFw=="],

+ 4 - 2
packages/opencode/package.json

@@ -42,11 +42,12 @@
     "@tsconfig/bun": "catalog:",
     "@types/babel__core": "7.20.5",
     "@types/bun": "catalog:",
+    "@types/cross-spawn": "6.0.6",
     "@types/mime-types": "3.0.1",
     "@types/semver": "^7.5.8",
     "@types/turndown": "5.0.5",
-    "@types/yargs": "17.0.33",
     "@types/which": "3.0.4",
+    "@types/yargs": "17.0.33",
     "@typescript/native-preview": "catalog:",
     "drizzle-kit": "1.0.0-beta.16-ea816b6",
     "drizzle-orm": "1.0.0-beta.16-ea816b6",
@@ -80,6 +81,7 @@
     "@ai-sdk/xai": "2.0.51",
     "@aws-sdk/credential-providers": "3.993.0",
     "@clack/prompts": "1.0.0-alpha.1",
+    "@effect/platform-node": "4.0.0-beta.31",
     "@gitlab/gitlab-ai-provider": "3.6.0",
     "@gitlab/opencode-gitlab-auth": "1.3.3",
     "@hono/standard-validator": "0.1.5",
@@ -95,7 +97,6 @@
     "@openrouter/ai-sdk-provider": "1.5.4",
     "@opentui/core": "0.1.87",
     "@opentui/solid": "0.1.87",
-    "@effect/platform-node": "4.0.0-beta.31",
     "@parcel/watcher": "2.5.1",
     "@pierre/diffs": "catalog:",
     "@solid-primitives/event-bus": "1.1.2",
@@ -108,6 +109,7 @@
     "bun-pty": "0.4.8",
     "chokidar": "4.0.3",
     "clipboardy": "4.0.0",
+    "cross-spawn": "^7.0.6",
     "decimal.js": "10.5.0",
     "diff": "catalog:",
     "drizzle-orm": "1.0.0-beta.16-ea816b6",

+ 6 - 12
packages/opencode/src/cli/cmd/pr.ts

@@ -112,21 +112,15 @@ export const PrCommand = cmd({
         UI.println("Starting opencode...")
         UI.println()
 
-        // Launch opencode TUI with session ID if available
-        const { spawn } = await import("child_process")
         const opencodeArgs = sessionId ? ["-s", sessionId] : []
-        const opencodeProcess = spawn("opencode", opencodeArgs, {
-          stdio: "inherit",
+        const opencodeProcess = Process.spawn(["opencode", ...opencodeArgs], {
+          stdin: "inherit",
+          stdout: "inherit",
+          stderr: "inherit",
           cwd: process.cwd(),
         })
-
-        await new Promise<void>((resolve, reject) => {
-          opencodeProcess.on("exit", (code) => {
-            if (code === 0) resolve()
-            else reject(new Error(`opencode exited with code ${code}`))
-          })
-          opencodeProcess.on("error", reject)
-        })
+        const code = await opencodeProcess.exited
+        if (code !== 0) throw new Error(`opencode exited with code ${code}`)
       },
     })
   },

+ 6 - 8
packages/opencode/src/ide/index.ts

@@ -1,9 +1,9 @@
 import { BusEvent } from "@/bus/bus-event"
 import { Bus } from "@/bus"
-import { spawn } from "bun"
 import z from "zod"
 import { NamedError } from "@opencode-ai/util/error"
 import { Log } from "../util/log"
+import { Process } from "@/util/process"
 
 const SUPPORTED_IDES = [
   { name: "Windsurf" as const, cmd: "windsurf" },
@@ -52,13 +52,11 @@ export namespace Ide {
     const cmd = SUPPORTED_IDES.find((i) => i.name === ide)?.cmd
     if (!cmd) throw new Error(`Unknown IDE: ${ide}`)
 
-    const p = spawn([cmd, "--install-extension", "sst-dev.opencode"], {
-      stdout: "pipe",
-      stderr: "pipe",
+    const p = await Process.run([cmd, "--install-extension", "sst-dev.opencode"], {
+      nothrow: true,
     })
-    await p.exited
-    const stdout = await new Response(p.stdout).text()
-    const stderr = await new Response(p.stderr).text()
+    const stdout = p.stdout.toString()
+    const stderr = p.stderr.toString()
 
     log.info("installed", {
       ide,
@@ -66,7 +64,7 @@ export namespace Ide {
       stderr,
     })
 
-    if (p.exitCode !== 0) {
+    if (p.code !== 0) {
       throw new InstallFailedError({ stderr })
     }
     if (stdout.includes("already installed")) {

+ 2 - 1
packages/opencode/src/lsp/client.ts

@@ -5,6 +5,7 @@ import { pathToFileURL, fileURLToPath } from "url"
 import { createMessageConnection, StreamMessageReader, StreamMessageWriter } from "vscode-jsonrpc/node"
 import type { Diagnostic as VSCodeDiagnostic } from "vscode-languageserver-types"
 import { Log } from "../util/log"
+import { Process } from "../util/process"
 import { LANGUAGE_EXTENSIONS } from "./language"
 import z from "zod"
 import type { LSPServer } from "./server"
@@ -239,7 +240,7 @@ export namespace LSPClient {
         l.info("shutting down")
         connection.end()
         connection.dispose()
-        input.server.process.kill()
+        await Process.stop(input.server.process)
         l.info("shutdown")
       },
     }

+ 6 - 7
packages/opencode/src/lsp/index.ts

@@ -7,9 +7,10 @@ import { pathToFileURL, fileURLToPath } from "url"
 import { LSPServer } from "./server"
 import z from "zod"
 import { Config } from "../config/config"
-import { spawn } from "child_process"
 import { Instance } from "../project/instance"
 import { Flag } from "@/flag/flag"
+import { Process } from "../util/process"
+import { spawn as lspspawn } from "./launch"
 
 export namespace LSP {
   const log = Log.create({ service: "lsp" })
@@ -112,9 +113,8 @@ export namespace LSP {
           extensions: item.extensions ?? existing?.extensions ?? [],
           spawn: async (root) => {
             return {
-              process: spawn(item.command[0], item.command.slice(1), {
+              process: lspspawn(item.command[0], item.command.slice(1), {
                 cwd: root,
-                windowsHide: true,
                 env: {
                   ...process.env,
                   ...item.env,
@@ -200,21 +200,20 @@ export namespace LSP {
         serverID: server.id,
         server: handle,
         root,
-      }).catch((err) => {
+      }).catch(async (err) => {
         s.broken.add(key)
-        handle.process.kill()
+        await Process.stop(handle.process)
         log.error(`Failed to initialize LSP client ${server.id}`, { error: err })
         return undefined
       })
 
       if (!client) {
-        handle.process.kill()
         return undefined
       }
 
       const existing = s.clients.find((x) => x.root === root && x.serverID === server.id)
       if (existing) {
-        handle.process.kill()
+        await Process.stop(handle.process)
         return existing
       }
 

+ 21 - 0
packages/opencode/src/lsp/launch.ts

@@ -0,0 +1,21 @@
+import type { ChildProcessWithoutNullStreams } from "child_process"
+import { Process } from "../util/process"
+
+type Child = Process.Child & ChildProcessWithoutNullStreams
+
+export function spawn(cmd: string, args: string[], opts?: Process.Options): Child
+export function spawn(cmd: string, opts?: Process.Options): Child
+export function spawn(cmd: string, argsOrOpts?: string[] | Process.Options, opts?: Process.Options) {
+  const args = Array.isArray(argsOrOpts) ? [...argsOrOpts] : []
+  const cfg = Array.isArray(argsOrOpts) ? opts : argsOrOpts
+  const proc = Process.spawn([cmd, ...args], {
+    ...(cfg ?? {}),
+    stdin: "pipe",
+    stdout: "pipe",
+    stderr: "pipe",
+  }) as Child
+
+  if (!proc.stdin || !proc.stdout || !proc.stderr) throw new Error("Process output not available")
+
+  return proc
+}

+ 3 - 7
packages/opencode/src/lsp/server.ts

@@ -1,4 +1,4 @@
-import { spawn as launch, type ChildProcessWithoutNullStreams } from "child_process"
+import type { ChildProcessWithoutNullStreams } from "child_process"
 import path from "path"
 import os from "os"
 import { Global } from "../global"
@@ -13,11 +13,7 @@ import { Archive } from "../util/archive"
 import { Process } from "../util/process"
 import { which } from "../util/which"
 import { Module } from "@opencode-ai/util/module"
-
-const spawn = ((cmd, args, opts) => {
-  if (Array.isArray(args)) return launch(cmd, [...args], { ...(opts ?? {}), windowsHide: true })
-  return launch(cmd, { ...(args ?? {}), windowsHide: true })
-}) as typeof launch
+import { spawn } from "./launch"
 
 export namespace LSPServer {
   const log = Log.create({ service: "lsp.server" })
@@ -273,7 +269,7 @@ export namespace LSPServer {
       }
 
       if (lintBin) {
-        const proc = Process.spawn([lintBin, "--help"], { stdout: "pipe" })
+        const proc = spawn(lintBin, ["--help"])
         await proc.exited
         if (proc.stdout) {
           const help = await text(proc.stdout)

+ 17 - 1
packages/opencode/src/util/process.ts

@@ -1,4 +1,5 @@
-import { spawn as launch, type ChildProcess } from "child_process"
+import { type ChildProcess } from "child_process"
+import launch from "cross-spawn"
 import { buffer } from "node:stream/consumers"
 
 export namespace Process {
@@ -113,6 +114,7 @@ export namespace Process {
       cwd: opts.cwd,
       env: opts.env,
       stdin: opts.stdin,
+      shell: opts.shell,
       abort: opts.abort,
       kill: opts.kill,
       timeout: opts.timeout,
@@ -140,6 +142,20 @@ export namespace Process {
     throw new RunFailedError(cmd, out.code, out.stdout, out.stderr)
   }
 
+  export async function stop(proc: ChildProcess) {
+    if (process.platform !== "win32" || !proc.pid) {
+      proc.kill()
+      return
+    }
+
+    const out = await run(["taskkill", "/pid", String(proc.pid), "/T", "/F"], {
+      nothrow: true,
+    })
+
+    if (out.code === 0) return
+    proc.kill()
+  }
+
   export async function text(cmd: string[], opts: RunOptions = {}): Promise<TextResult> {
     const out = await run(cmd, opts)
     return {

+ 22 - 0
packages/opencode/test/lsp/launch.test.ts

@@ -0,0 +1,22 @@
+import { describe, expect, test } from "bun:test"
+import fs from "fs/promises"
+import path from "path"
+import { spawn } from "../../src/lsp/launch"
+import { tmpdir } from "../fixture/fixture"
+
+describe("lsp.launch", () => {
+  test("spawns cmd scripts with spaces on Windows", async () => {
+    if (process.platform !== "win32") return
+
+    await using tmp = await tmpdir()
+    const dir = path.join(tmp.path, "with space")
+    const file = path.join(dir, "echo cmd.cmd")
+
+    await fs.mkdir(dir, { recursive: true })
+    await Bun.write(file, "@echo off\r\nif %~1==--stdio exit /b 0\r\nexit /b 7\r\n")
+
+    const proc = spawn(file, ["--stdio"])
+
+    expect(await proc.exited).toBe(0)
+  })
+})

+ 35 - 0
packages/opencode/test/util/process.test.ts

@@ -1,4 +1,6 @@
 import { describe, expect, test } from "bun:test"
+import fs from "fs/promises"
+import path from "path"
 import { Process } from "../../src/util/process"
 import { tmpdir } from "../fixture/fixture"
 
@@ -74,4 +76,37 @@ describe("util.process", () => {
     })
     expect(out.stdout.toString()).toBe("set")
   })
+
+  test("uses shell in run on Windows", async () => {
+    if (process.platform !== "win32") return
+
+    const out = await Process.run(["set", "OPENCODE_TEST_SHELL"], {
+      shell: true,
+      env: {
+        OPENCODE_TEST_SHELL: "ok",
+      },
+    })
+
+    expect(out.code).toBe(0)
+    expect(out.stdout.toString()).toContain("OPENCODE_TEST_SHELL=ok")
+  })
+
+  test("runs cmd scripts with spaces on Windows without shell", async () => {
+    if (process.platform !== "win32") return
+
+    await using tmp = await tmpdir()
+    const dir = path.join(tmp.path, "with space")
+    const file = path.join(dir, "echo cmd.cmd")
+
+    await fs.mkdir(dir, { recursive: true })
+    await Bun.write(file, "@echo off\r\nif %~1==--stdio exit /b 0\r\nexit /b 7\r\n")
+
+    const proc = Process.spawn([file, "--stdio"], {
+      stdin: "pipe",
+      stdout: "pipe",
+      stderr: "pipe",
+    })
+
+    expect(await proc.exited).toBe(0)
+  })
 })