shell tweaks, better handling for windows (#5455)

Co-authored-by: GitHub Action <[email protected]>

packages/opencode/src/flag/flag.ts

@@ -1,5 +1,6 @@
 export namespace Flag {
   export const OPENCODE_AUTO_SHARE = truthy("OPENCODE_AUTO_SHARE")
+  export const OPENCODE_GIT_BASH_PATH = process.env["OPENCODE_GIT_BASH_PATH"]
   export const OPENCODE_CONFIG = process.env["OPENCODE_CONFIG"]
   export const OPENCODE_CONFIG_DIR = process.env["OPENCODE_CONFIG_DIR"]
   export const OPENCODE_CONFIG_CONTENT = process.env["OPENCODE_CONFIG_CONTENT"]

packages/opencode/src/pty/index.ts

@@ -6,10 +6,10 @@ import { Identifier } from "../id/id"
 import { Log } from "../util/log"
 import type { WSContext } from "hono/ws"
 import { Instance } from "../project/instance"
-import { shell } from "@opencode-ai/util/shell"
 import { lazy } from "@opencode-ai/util/lazy"
 import {} from "process"
 import { Installation } from "@/installation"
+import { Shell } from "@/shell/shell"
 
 export namespace Pty {
   const log = Log.create({ service: "pty" })
@@ -112,7 +112,7 @@ export namespace Pty {
 
   export async function create(input: CreateInput) {
     const id = Identifier.create("pty", false)
-    const command = input.command || shell()
+    const command = input.command || Shell.preferred()
     const args = input.args || []
     const cwd = input.cwd || Instance.directory
     const env = { ...process.env, ...input.env } as Record<string, string>

packages/opencode/src/session/prompt.ts

@@ -50,6 +50,7 @@ import { fn } from "@/util/fn"
 import { SessionProcessor } from "./processor"
 import { TaskTool } from "@/tool/task"
 import { SessionStatus } from "./status"
+import { Shell } from "@/shell/shell"
 
 // @ts-ignore
 globalThis.AI_SDK_LOG_WARNINGS = false
@@ -1172,6 +1173,12 @@ export namespace SessionPrompt {
   })
   export type ShellInput = z.infer<typeof ShellInput>
   export async function shell(input: ShellInput) {
+    const abort = start(input.sessionID)
+    if (!abort) {
+      throw new Session.BusyError(input.sessionID)
+    }
+    using _ = defer(() => cancel(input.sessionID))
+
     const session = await Session.get(input.sessionID)
     if (session.revert) {
       SessionRevert.cleanup(session)
@@ -1244,8 +1251,10 @@ export namespace SessionPrompt {
       },
     }
     await Session.updatePart(part)
-    const shell = process.env["SHELL"] ?? (process.platform === "win32" ? process.env["COMSPEC"] || "cmd.exe" : "bash")
-    const shellName = path.basename(shell).toLowerCase()
+    const shell = Shell.preferred()
+    const shellName = (
+      process.platform === "win32" ? path.win32.basename(shell, ".exe") : path.basename(shell)
+    ).toLowerCase()
 
     const invocations: Record<string, { args: string[] }> = {
       nu: {
@@ -1275,12 +1284,15 @@ export namespace SessionPrompt {
           `,
         ],
       },
-      // Windows cmd.exe
-      "cmd.exe": {
+      // Windows cmd
+      cmd: {
         args: ["/c", input.command],
       },
       // Windows PowerShell
-      "powershell.exe": {
+      powershell: {
+        args: ["-NoProfile", "-Command", input.command],
+      },
+      pwsh: {
         args: ["-NoProfile", "-Command", input.command],
       },
       // Fallback: any shell that doesn't match those above
@@ -1327,11 +1339,34 @@ export namespace SessionPrompt {
       }
     })
 
+    let aborted = false
+    let exited = false
+
+    const kill = () => Shell.killTree(proc, { exited: () => exited })
+
+    if (abort.aborted) {
+      aborted = true
+      await kill()
+    }
+
+    const abortHandler = () => {
+      aborted = true
+      void kill()
+    }
+
+    abort.addEventListener("abort", abortHandler, { once: true })
+
     await new Promise<void>((resolve) => {
       proc.on("close", () => {
+        exited = true
+        abort.removeEventListener("abort", abortHandler)
         resolve()
       })
     })
+
+    if (aborted) {
+      output += "\n\n" + ["<metadata>", "User aborted the command", "</metadata>"].join("\n")
+    }
     msg.time.completed = Date.now()
     await Session.updateMessage(msg)
     if (part.state.status === "running") {

packages/opencode/src/shell/shell.ts

@@ -0,0 +1,67 @@
+import { Flag } from "@/flag/flag"
+import { lazy } from "@/util/lazy"
+import path from "path"
+import { spawn, type ChildProcess } from "child_process"
+
+const SIGKILL_TIMEOUT_MS = 200
+
+export namespace Shell {
+  export async function killTree(proc: ChildProcess, opts?: { exited?: () => boolean }): Promise<void> {
+    const pid = proc.pid
+    if (!pid || opts?.exited?.()) return
+
+    if (process.platform === "win32") {
+      await new Promise<void>((resolve) => {
+        const killer = spawn("taskkill", ["/pid", String(pid), "/f", "/t"], { stdio: "ignore" })
+        killer.once("exit", () => resolve())
+        killer.once("error", () => resolve())
+      })
+      return
+    }
+
+    try {
+      process.kill(-pid, "SIGTERM")
+      await Bun.sleep(SIGKILL_TIMEOUT_MS)
+      if (!opts?.exited?.()) {
+        process.kill(-pid, "SIGKILL")
+      }
+    } catch (_e) {
+      proc.kill("SIGTERM")
+      await Bun.sleep(SIGKILL_TIMEOUT_MS)
+      if (!opts?.exited?.()) {
+        proc.kill("SIGKILL")
+      }
+    }
+  }
+  const BLACKLIST = new Set(["fish", "nu"])
+
+  function fallback() {
+    if (process.platform === "win32") {
+      if (Flag.OPENCODE_GIT_BASH_PATH) return Flag.OPENCODE_GIT_BASH_PATH
+      const git = Bun.which("git")
+      if (git) {
+        // git.exe is typically at: C:\Program Files\Git\cmd\git.exe
+        // bash.exe is at: C:\Program Files\Git\bin\bash.exe
+        const bash = path.join(git, "..", "..", "bin", "bash.exe")
+        if (Bun.file(bash).size) return bash
+      }
+      return process.env.COMSPEC || "cmd.exe"
+    }
+    if (process.platform === "darwin") return "/bin/zsh"
+    const bash = Bun.which("bash")
+    if (bash) return bash
+    return "/bin/sh"
+  }
+
+  export const preferred = lazy(() => {
+    const s = process.env.SHELL
+    if (s) return s
+    return fallback()
+  })
+
+  export const acceptable = lazy(() => {
+    const s = process.env.SHELL
+    if (s && !BLACKLIST.has(process.platform === "win32" ? path.win32.basename(s) : path.basename(s))) return s
+    return fallback()
+  })
+}

packages/opencode/src/tool/bash.ts

@@ -14,11 +14,10 @@ import { Permission } from "@/permission"
 import { fileURLToPath } from "url"
 import { Flag } from "@/flag/flag.ts"
 import path from "path"
-import { iife } from "@/util/iife"
+import { Shell } from "@/shell/shell"
 
 const MAX_OUTPUT_LENGTH = Flag.OPENCODE_EXPERIMENTAL_BASH_MAX_OUTPUT_LENGTH || 30_000
 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000
-const SIGKILL_TIMEOUT_MS = 200
 
 export const log = Log.create({ service: "bash-tool" })
 
@@ -53,32 +52,7 @@ const parser = lazy(async () => {
 // TODO: we may wanna rename this tool so it works better on other shells
 
 export const BashTool = Tool.define("bash", async () => {
-  const shell = iife(() => {
-    const s = process.env.SHELL
-    if (s) {
-      const basename = path.basename(s)
-      if (!new Set(["fish", "nu"]).has(basename)) {
-        return s
-      }
-    }
-
-    if (process.platform === "darwin") {
-      return "/bin/zsh"
-    }
-
-    if (process.platform === "win32") {
-      // Let Bun / Node pick COMSPEC (usually cmd.exe)
-      // or explicitly:
-      return process.env.COMSPEC || true
-    }
-
-    const bash = Bun.which("bash")
-    if (bash) {
-      return bash
-    }
-
-    return true
-  })
+  const shell = Shell.acceptable()
   log.info("bash tool using shell", { shell })
 
   return {
@@ -261,51 +235,23 @@ export const BashTool = Tool.define("bash", async () => {
       let aborted = false
       let exited = false
 
-      const killTree = async () => {
-        const pid = proc.pid
-        if (!pid || exited) {
-          return
-        }
-
-        if (process.platform === "win32") {
-          await new Promise<void>((resolve) => {
-            const killer = spawn("taskkill", ["/pid", String(pid), "/f", "/t"], { stdio: "ignore" })
-            killer.once("exit", resolve)
-            killer.once("error", resolve)
-          })
-          return
-        }
-
-        try {
-          process.kill(-pid, "SIGTERM")
-          await Bun.sleep(SIGKILL_TIMEOUT_MS)
-          if (!exited) {
-            process.kill(-pid, "SIGKILL")
-          }
-        } catch (_e) {
-          proc.kill("SIGTERM")
-          await Bun.sleep(SIGKILL_TIMEOUT_MS)
-          if (!exited) {
-            proc.kill("SIGKILL")
-          }
-        }
-      }
+      const kill = () => Shell.killTree(proc, { exited: () => exited })
 
       if (ctx.abort.aborted) {
         aborted = true
-        await killTree()
+        await kill()
       }
 
       const abortHandler = () => {
         aborted = true
-        void killTree()
+        void kill()
       }
 
       ctx.abort.addEventListener("abort", abortHandler, { once: true })
 
       const timeoutTimer = setTimeout(() => {
         timedOut = true
-        void killTree()
+        void kill()
       }, timeout + 100)
 
       await new Promise<void>((resolve, reject) => {

packages/util/src/shell.ts

@@ -1,13 +0,0 @@
-export function shell() {
-  const s = process.env.SHELL
-  if (s) return s
-  if (process.platform === "darwin") {
-    return "/bin/zsh"
-  }
-  if (process.platform === "win32") {
-    return process.env.COMSPEC || "cmd.exe"
-  }
-  const bash = Bun.which("bash")
-  if (bash) return bash
-  return "bash"
-}