From ea487bc870535c2ba36a47a2102e82df00513096 Mon Sep 17 00:00:00 2001 From: vhqtvn <8930337+vhqtvn@users.noreply.github.com> Date: Sat, 4 Apr 2026 16:37:27 +0700 Subject: [PATCH 1/2] fix(tool): fix bash tool execution hangs and mitigate database locking This commit addresses two major stability issues during tool execution: 1. **Race Condition in Process Spawning (Fixes Hangs):** Fast-exiting processes (like 'jq') could terminate so quickly that the 'spawn' event was omitted or misordered. This left the 'resume()' callback in 'CrossSpawnSpawner' blocked forever, causing the session to hang indefinitely. We now defensively trigger 'resume' on 'error', 'exit', or 'close' to guarantee the execution promise resolves. 2. **Event Loop Blockage on High-Volume Output (Mitigates DB Locking & Fixes Output Truncation):** Commands emitting thousands of lines quickly (e.g. 'npm install', large 'cat' or 'grep') caused 'Stream.runForEach' to synchronously queue thousands of 'db.insert().run()' calls via 'ctx.metadata(...)' on the main thread, choking the event loop and exacerbating 'database is locked' exceptions. - Throttled metadata updates to 250ms ('now - lastUpdate > 250'). - Added a deferred 'setTimeout(flush, 250)' to ensure trailing output chunks are not missed. - Fixed a race condition where 'runPromiseExit' resolved on process 'close' and forcefully interrupted the stream fiber ('handle.all'), truncating output. Now uses 'Fiber.join(streamFiber)' to completely drain pipes before returning. --- .../src/effect/cross-spawn-spawner.ts | 19 +++++++- packages/opencode/src/tool/bash.ts | 47 +++++++++++++++---- 2 files changed, 54 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/effect/cross-spawn-spawner.ts b/packages/opencode/src/effect/cross-spawn-spawner.ts index 92e5b3ba2d07..dc081001e8bc 100644 --- a/packages/opencode/src/effect/cross-spawn-spawner.ts +++ b/packages/opencode/src/effect/cross-spawn-spawner.ts @@ -268,19 +268,34 @@ export const make = Effect.gen(function* () { const proc = launch(command.command, command.args, opts) let end = false let exit: readonly [code: number | null, signal: NodeJS.Signals | null] | undefined + let spawned = false proc.on("error", (err) => { - resume(Effect.fail(toPlatformError("spawn", err, command))) + if (!spawned) { + spawned = true + resume(Effect.fail(toPlatformError("spawn", err, command))) + } }) proc.on("exit", (...args) => { + if (!spawned) { + spawned = true + resume(Effect.succeed([proc, signal])) + } exit = args }) proc.on("close", (...args) => { + if (!spawned) { + spawned = true + resume(Effect.succeed([proc, signal])) + } if (end) return end = true Deferred.doneUnsafe(signal, Exit.succeed(exit ?? args)) }) proc.on("spawn", () => { - resume(Effect.succeed([proc, signal])) + if (!spawned) { + spawned = true + resume(Effect.succeed([proc, signal])) + } }) return Effect.sync(() => { proc.kill("SIGTERM") diff --git a/packages/opencode/src/tool/bash.ts b/packages/opencode/src/tool/bash.ts index e50f09cc38ce..1a572a1e1266 100644 --- a/packages/opencode/src/tool/bash.ts +++ b/packages/opencode/src/tool/bash.ts @@ -17,7 +17,7 @@ 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 { Cause, Effect, Exit, Stream, Fiber } from "effect" import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process" import * as CrossSpawnSpawner from "@/effect/cross-spawn-spawner" @@ -341,16 +341,42 @@ async function run( Effect.gen(function* () { const handle = yield* spawner.spawn(cmd(input.shell, input.name, input.command, input.cwd, input.env)) - yield* Effect.forkScoped( + let lastUpdate = 0 + let timeoutId: any = null + + const flush = () => { + if (timeoutId) { + clearTimeout(timeoutId) + timeoutId = null + } + lastUpdate = Date.now() + ctx.metadata({ + metadata: { + output: preview(output), + description: input.description, + }, + }) + } + + yield* Effect.addFinalizer(() => + Effect.sync(() => { + if (timeoutId) { + clearTimeout(timeoutId) + timeoutId = null + } + }), + ) + + const streamFiber = yield* Effect.forkScoped( Stream.runForEach(Stream.decodeText(handle.all), (chunk) => Effect.sync(() => { output += chunk - ctx.metadata({ - metadata: { - output: preview(output), - description: input.description, - }, - }) + const now = Date.now() + if (now - lastUpdate > 250) { + flush() + } else if (!timeoutId) { + timeoutId = setTimeout(flush, 250) + } }), ), ) @@ -373,10 +399,11 @@ async function run( if (exit.kind === "abort") { aborted = true yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie) - } - if (exit.kind === "timeout") { + } else if (exit.kind === "timeout") { expired = true yield* handle.kill({ forceKillAfter: "3 seconds" }).pipe(Effect.orDie) + } else { + yield* Fiber.join(streamFiber).pipe(Effect.ignore) } return exit.kind === "exit" ? exit.code : null From 8d24b0ebb41d482ce5e590ce9e60ec152cd0a43c Mon Sep 17 00:00:00 2001 From: vhqtvn <8930337+vhqtvn@users.noreply.github.com> Date: Sat, 4 Apr 2026 20:21:12 +0700 Subject: [PATCH 2/2] fix(tool): fix bash tool execution hangs and mitigate database locking This commit addresses two major stability issues during tool execution: 1. **Race Condition in Process Spawning (Fixes Hangs):** Fast-exiting processes (like 'jq') could terminate so quickly that the 'spawn' event was omitted or misordered. This left the 'resume()' callback in 'CrossSpawnSpawner' blocked forever, causing the session to hang indefinitely. We now defensively trigger 'resume' on 'error', 'exit', or 'close' to guarantee the execution promise resolves. 2. **Event Loop Blockage on High-Volume Output (Mitigates DB Locking & Fixes Output Truncation):** Commands emitting thousands of lines quickly (e.g. 'npm install', large 'cat' or 'grep') caused 'Stream.runForEach' to synchronously queue thousands of 'db.insert().run()' calls via 'ctx.metadata(...)' on the main thread, choking the event loop and exacerbating 'database is locked' exceptions. - Throttled metadata updates to 250ms ('now - lastUpdate > 250'). - Added a deferred 'setTimeout(flush, 250)' to ensure trailing output chunks are not missed. - Fixed a race condition where 'runPromiseExit' resolved on process 'close' and forcefully interrupted the stream fiber ('handle.all'), truncating output. Now uses 'Fiber.join(streamFiber)' to completely drain pipes before returning. Includes test fixes for: - tool.bash stream metadata throttling flakiness - tool.write file permission flakiness depending on OS umask --- packages/opencode/test/tool/bash.test.ts | 2 +- packages/opencode/test/tool/write.test.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/opencode/test/tool/bash.test.ts b/packages/opencode/test/tool/bash.test.ts index e4ba881fb166..b20e333156b2 100644 --- a/packages/opencode/test/tool/bash.test.ts +++ b/packages/opencode/test/tool/bash.test.ts @@ -992,7 +992,7 @@ describe("tool.bash abort", () => { const updates: string[] = [] const result = await bash.execute( { - command: `echo first && sleep 0.1 && echo second`, + command: `echo first && sleep 0.3 && echo second`, description: "Streaming test", }, { diff --git a/packages/opencode/test/tool/write.test.ts b/packages/opencode/test/tool/write.test.ts index 97939c10519e..b2542552ead0 100644 --- a/packages/opencode/test/tool/write.test.ts +++ b/packages/opencode/test/tool/write.test.ts @@ -171,7 +171,8 @@ describe("tool.write", () => { // On Unix systems, check permissions if (process.platform !== "win32") { const stats = await fs.stat(filepath) - expect(stats.mode & 0o777).toBe(0o644) + const mode = stats.mode & 0o777 + expect(mode === 0o644 || mode === 0o664).toBe(true) } }, })