refactor(bash): use Effect ChildProcess for bash tool execution (#20496)

Author: kitlangton

packages/opencode/src/effect/cross-spawn-spawner.ts

@@ -488,3 +488,14 @@ export const layer: Layer.Layer<ChildProcessSpawner, never, FileSystem.FileSyste
 )
 
 export const defaultLayer = layer.pipe(Layer.provide(NodeFileSystem.layer), Layer.provide(NodePath.layer))
+
+import { lazy } from "@/util/lazy"
+
+const rt = lazy(() => {
+  // Dynamic import to avoid circular dep: cross-spawn-spawner → run-service → Instance → project → cross-spawn-spawner
+  const { makeRuntime } = require("@/effect/run-service") as typeof import("@/effect/run-service")
+  return makeRuntime(ChildProcessSpawner, defaultLayer)
+})
+
+export const runPromiseExit: ReturnType<typeof rt>["runPromiseExit"] = (...args) => rt().runPromiseExit(...(args as [any]))
+export const runPromise: ReturnType<typeof rt>["runPromise"] = (...args) => rt().runPromise(...(args as [any]))

packages/opencode/src/tool/bash.ts

@@ -1,6 +1,5 @@
 import z from "zod"
 import os from "os"
-import { spawn } from "child_process"
 import { Tool } from "./tool"
 import path from "path"
 import DESCRIPTION from "./bash.txt"
@@ -18,6 +17,9 @@ import { Shell } from "@/shell/shell"
 import { BashArity } from "@/permission/arity"
 import { Truncate } from "./truncate"
 import { Plugin } from "@/plugin"
+import { Cause, Effect, Exit, Stream } from "effect"
+import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
+import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner"
 
 const MAX_METADATA_LENGTH = 30_000
 const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000
@@ -293,27 +295,26 @@ async function shellEnv(ctx: Tool.Context, cwd: string) {
   }
 }
 
-function launch(shell: string, name: string, command: string, cwd: string, env: NodeJS.ProcessEnv) {
+function cmd(shell: string, name: string, command: string, cwd: string, env: NodeJS.ProcessEnv) {
   if (process.platform === "win32" && PS.has(name)) {
-    return spawn(shell, ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", command], {
+    return ChildProcess.make(shell, ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", command], {
       cwd,
       env,
-      stdio: ["ignore", "pipe", "pipe"],
+      stdin: "ignore",
       detached: false,
-      windowsHide: true,
     })
   }
 
-  return spawn(command, {
+  return ChildProcess.make(command, [], {
     shell,
     cwd,
     env,
-    stdio: ["ignore", "pipe", "pipe"],
+    stdin: "ignore",
     detached: process.platform !== "win32",
-    windowsHide: process.platform === "win32",
   })
 }
 
+
 async function run(
   input: {
     shell: string
@@ -326,8 +327,9 @@ async function run(
   },
   ctx: Tool.Context,
 ) {
-  const proc = launch(input.shell, input.name, input.command, input.cwd, input.env)
   let output = ""
+  let expired = false
+  let aborted = false
 
   ctx.metadata({
     metadata: {
@@ -336,76 +338,78 @@ async function run(
     },
   })
 
-  const append = (chunk: Buffer) => {
-    output += chunk.toString()
-    ctx.metadata({
-      metadata: {
-        output: preview(output),
-        description: input.description,
-      },
-    })
-  }
-
-  proc.stdout?.on("data", append)
-  proc.stderr?.on("data", append)
-
-  let expired = false
-  let aborted = false
-  let exited = false
-
-  const kill = () => Shell.killTree(proc, { exited: () => exited })
-
-  if (ctx.abort.aborted) {
-    aborted = true
-    await kill()
-  }
+  const exit = await CrossSpawnSpawner.runPromiseExit((spawner) =>
+    Effect.gen(function* () {
+      const handle = yield* spawner.spawn(
+        cmd(input.shell, input.name, input.command, input.cwd, input.env),
+      )
 
-  const abort = () => {
-    aborted = true
-    void kill()
-  }
+      yield* Effect.forkScoped(
+        Stream.runForEach(
+          Stream.decodeText(handle.all),
+          (chunk) =>
+            Effect.sync(() => {
+              output += chunk
+              ctx.metadata({
+                metadata: {
+                  output: preview(output),
+                  description: input.description,
+                },
+              })
+            }),
+        ),
+      )
 
-  ctx.abort.addEventListener("abort", abort, { once: true })
-  const timer = setTimeout(() => {
-    expired = true
-    void kill()
-  }, input.timeout + 100)
+      const abort = Effect.callback<void>((resume) => {
+        if (ctx.abort.aborted) return resume(Effect.void)
+        const handler = () => resume(Effect.void)
+        ctx.abort.addEventListener("abort", handler, { once: true })
+        return Effect.sync(() => ctx.abort.removeEventListener("abort", handler))
+      })
 
-  await new Promise<void>((resolve, reject) => {
-    const cleanup = () => {
-      clearTimeout(timer)
-      ctx.abort.removeEventListener("abort", abort)
-    }
+      const timeout = Effect.sleep(`${input.timeout + 100} millis`)
 
-    proc.once("exit", () => {
-      exited = true
-    })
+      const exit = yield* Effect.raceAll([
+        handle.exitCode.pipe(Effect.map((code) => ({ kind: "exit" as const, code }))),
+        abort.pipe(Effect.map(() => ({ kind: "abort" as const, code: null }))),
+        timeout.pipe(Effect.map(() => ({ kind: "timeout" as const, code: null }))),
+      ])
 
-    proc.once("close", () => {
-      exited = true
-      cleanup()
-      resolve()
-    })
+      if (exit.kind === "abort") {
+        aborted = true
+        yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie)
+      }
+      if (exit.kind === "timeout") {
+        expired = true
+        yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie)
+      }
 
-    proc.once("error", (error) => {
-      exited = true
-      cleanup()
-      reject(error)
-    })
-  })
+      return exit.kind === "exit" ? exit.code : null
+    }).pipe(
+      Effect.scoped,
+      Effect.orDie,
+    ),
+  )
+
+  let code: number | null = null
+  if (Exit.isSuccess(exit)) {
+    code = exit.value
+  } else if (!Cause.hasInterruptsOnly(exit.cause)) {
+    throw Cause.squash(exit.cause)
+  }
 
-  const metadata: string[] = []
-  if (expired) metadata.push(`bash tool terminated command after exceeding timeout ${input.timeout} ms`)
-  if (aborted) metadata.push("User aborted the command")
-  if (metadata.length > 0) {
-    output += "\n\n<bash_metadata>\n" + metadata.join("\n") + "\n</bash_metadata>"
+  const meta: string[] = []
+  if (expired) meta.push(`bash tool terminated command after exceeding timeout ${input.timeout} ms`)
+  if (aborted) meta.push("User aborted the command")
+  if (meta.length > 0) {
+    output += "\n\n<bash_metadata>\n" + meta.join("\n") + "\n</bash_metadata>"
   }
 
   return {
     title: input.description,
     metadata: {
       output: preview(output),
-      exit: proc.exitCode,
+      exit: code,
       description: input.description,
     },
     output,

packages/opencode/test/tool/bash.test.ts

@@ -896,6 +896,121 @@ describe("tool.bash permissions", () => {
   })
 })
 
+describe("tool.bash abort", () => {
+  test("preserves output when aborted", async () => {
+    await Instance.provide({
+      directory: projectRoot,
+      fn: async () => {
+        const bash = await BashTool.init()
+        const controller = new AbortController()
+        const collected: string[] = []
+        const result = bash.execute(
+          {
+            command: `echo before && sleep 30`,
+            description: "Long running command",
+          },
+          {
+            ...ctx,
+            abort: controller.signal,
+            metadata: (input) => {
+              const output = (input.metadata as { output?: string })?.output
+              if (output && output.includes("before") && !controller.signal.aborted) {
+                collected.push(output)
+                controller.abort()
+              }
+            },
+          },
+        )
+        const res = await result
+        expect(res.output).toContain("before")
+        expect(res.output).toContain("User aborted the command")
+        expect(collected.length).toBeGreaterThan(0)
+      },
+    })
+  }, 15_000)
+
+  test("terminates command on timeout", async () => {
+    await Instance.provide({
+      directory: projectRoot,
+      fn: async () => {
+        const bash = await BashTool.init()
+        const result = await bash.execute(
+          {
+            command: `echo started && sleep 60`,
+            description: "Timeout test",
+            timeout: 500,
+          },
+          ctx,
+        )
+        expect(result.output).toContain("started")
+        expect(result.output).toContain("bash tool terminated command after exceeding timeout")
+      },
+    })
+  }, 15_000)
+
+  test.skipIf(process.platform === "win32")("captures stderr in output", async () => {
+    await Instance.provide({
+      directory: projectRoot,
+      fn: async () => {
+        const bash = await BashTool.init()
+        const result = await bash.execute(
+          {
+            command: `echo stdout_msg && echo stderr_msg >&2`,
+            description: "Stderr test",
+          },
+          ctx,
+        )
+        expect(result.output).toContain("stdout_msg")
+        expect(result.output).toContain("stderr_msg")
+        expect(result.metadata.exit).toBe(0)
+      },
+    })
+  })
+
+  test("returns non-zero exit code", async () => {
+    await Instance.provide({
+      directory: projectRoot,
+      fn: async () => {
+        const bash = await BashTool.init()
+        const result = await bash.execute(
+          {
+            command: `exit 42`,
+            description: "Non-zero exit",
+          },
+          ctx,
+        )
+        expect(result.metadata.exit).toBe(42)
+      },
+    })
+  })
+
+  test("streams metadata updates progressively", async () => {
+    await Instance.provide({
+      directory: projectRoot,
+      fn: async () => {
+        const bash = await BashTool.init()
+        const updates: string[] = []
+        const result = await bash.execute(
+          {
+            command: `echo first && sleep 0.1 && echo second`,
+            description: "Streaming test",
+          },
+          {
+            ...ctx,
+            metadata: (input) => {
+              const output = (input.metadata as { output?: string })?.output
+              if (output) updates.push(output)
+            },
+          },
+        )
+        expect(result.output).toContain("first")
+        expect(result.output).toContain("second")
+        expect(updates.length).toBeGreaterThan(1)
+      },
+    })
+  })
+})
+
 describe("tool.bash truncation", () => {
   test("truncates output exceeding line limit", async () => {
     await Instance.provide({