From 25551172c9c546a32c79703d62d04f97653329d2 Mon Sep 17 00:00:00 2001 From: LukeParkerDev <10430890+Hona@users.noreply.github.com> Date: Fri, 3 Apr 2026 16:04:41 +1000 Subject: [PATCH] fix(shell): avoid abort hangs and utf8 corruption Attach shell process listeners before handling already-aborted tool signals so canceled runs always settle, and decode shell output as UTF-8 to preserve multibyte characters across chunk boundaries. Also lazy-load shell-specific parsers and hoist command sets so parsing work stays focused on the active shell. --- packages/opencode/src/tool/shell/parser.ts | 43 ++++++++++---------- packages/opencode/src/tool/shell/runner.ts | 31 ++++++++------- packages/opencode/test/tool/shell.test.ts | 46 ++++++++++++++++++++++ 3 files changed, 86 insertions(+), 34 deletions(-) diff --git a/packages/opencode/src/tool/shell/parser.ts b/packages/opencode/src/tool/shell/parser.ts index 3860f65ea5..7b8dff37b7 100644 --- a/packages/opencode/src/tool/shell/parser.ts +++ b/packages/opencode/src/tool/shell/parser.ts @@ -22,6 +22,8 @@ const FILES_PWSH = new Set([ "new-item", "rename-item", ]) +const FILES_BASH = new Set([...CWD, ...FILES_BASE]) +const FILES_PWSH_ALL = new Set([...FILES_BASH, ...FILES_PWSH]) const FLAGS = new Set(["-destination", "-literalpath", "-path"]) const SWITCHES = new Set(["-confirm", "-debug", "-force", "-nonewline", "-recurse", "-verbose", "-whatif"]) @@ -125,36 +127,38 @@ function pathArgs(list: Part[], isPwsh: boolean) { } export namespace ShellParser { - const getParser = lazy(async () => { - const { Parser } = await import("web-tree-sitter") + const getCore = lazy(async () => { + const tree = await import("web-tree-sitter") const { default: treeWasm } = await import("web-tree-sitter/tree-sitter.wasm" as string, { with: { type: "wasm" }, }) const treePath = resolveWasm(treeWasm) - await Parser.init({ + await tree.Parser.init({ locateFile() { return treePath }, }) + return tree + }) + + const getBashParser = lazy(async () => { + const tree = await getCore() const { default: bashWasm } = await import("tree-sitter-bash/tree-sitter-bash.wasm" as string, { with: { type: "wasm" }, }) + const bash = new tree.Parser() + bash.setLanguage(await tree.Language.load(resolveWasm(bashWasm))) + return bash + }) + + const getPsParser = lazy(async () => { + const tree = await getCore() const { default: psWasm } = await import("tree-sitter-powershell/tree-sitter-powershell.wasm" as string, { with: { type: "wasm" }, }) - const { Language } = await import("web-tree-sitter") - const bashPath = resolveWasm(bashWasm) - const psPath = resolveWasm(psWasm) - const bashLanguage = await Language.load(bashPath) - const psLanguage = await Language.load(psPath) - - const bash = new Parser() - bash.setLanguage(bashLanguage) - - const ps = new Parser() - ps.setLanguage(psLanguage) - - return { bash, ps } + const ps = new tree.Parser() + ps.setLanguage(await tree.Language.load(resolveWasm(psWasm))) + return ps }) export async function collect(opts: { @@ -164,8 +168,7 @@ export namespace ShellParser { shellType: ShellTool.ID }): Promise { const isPwsh = ShellTool.powershell(opts.shellType) - const parsers = await getParser() - const parser = isPwsh ? parsers.ps : parsers.bash + const parser = isPwsh ? await getPsParser() : await getBashParser() const tree = parser.parse(opts.command) if (!tree) throw new Error("Failed to parse command") @@ -177,14 +180,14 @@ export namespace ShellParser { always: new Set(), } - const filesSet = new Set([...CWD, ...FILES_BASE, ...(isPwsh ? FILES_PWSH : [])]) + const files = isPwsh ? FILES_PWSH_ALL : FILES_BASH for (const node of commands(root)) { const commandParts = parts(node) const tokens = commandParts.map((item) => item.text) const cmd = isPwsh ? tokens[0]?.toLowerCase() : tokens[0] - if (cmd && filesSet.has(cmd)) { + if (cmd && files.has(cmd)) { for (const arg of pathArgs(commandParts, isPwsh)) { const resolved = await argPath(arg, opts.cwd, opts.shell, isPwsh) log.info("resolved path", { arg, resolved }) diff --git a/packages/opencode/src/tool/shell/runner.ts b/packages/opencode/src/tool/shell/runner.ts index b86d39cecb..647605d599 100644 --- a/packages/opencode/src/tool/shell/runner.ts +++ b/packages/opencode/src/tool/shell/runner.ts @@ -71,8 +71,11 @@ exit 1` }, }) - const append = (chunk: Buffer) => { - output += chunk.toString() + proc.stdout?.setEncoding("utf8") + proc.stderr?.setEncoding("utf8") + + const append = (chunk: string) => { + output += chunk ctx.metadata({ metadata: { output: preview(output), @@ -87,26 +90,16 @@ exit 1` let expired = false let aborted = false let exited = false + let timer: ReturnType const kill = () => Shell.killTree(proc, { exited: () => exited }) - if (ctx.abort.aborted) { - aborted = true - await kill() - } - const abort = () => { aborted = true void kill() } - ctx.abort.addEventListener("abort", abort, { once: true }) - const timer = setTimeout(() => { - expired = true - void kill() - }, input.timeout + 100) - - await new Promise((resolve, reject) => { + const wait = new Promise((resolve, reject) => { const cleanup = () => { clearTimeout(timer) ctx.abort.removeEventListener("abort", abort) @@ -131,6 +124,16 @@ exit 1` }) }) + ctx.abort.addEventListener("abort", abort, { once: true }) + timer = setTimeout(() => { + expired = true + void kill() + }, input.timeout + 100) + + if (ctx.abort.aborted) abort() + + await wait + const metadata: string[] = [] if (expired) metadata.push(`${input.name} tool terminated command after exceeding timeout ${input.timeout} ms`) if (aborted) metadata.push("User aborted the command") diff --git a/packages/opencode/test/tool/shell.test.ts b/packages/opencode/test/tool/shell.test.ts index 26a6e7d056..aedfacc3a2 100644 --- a/packages/opencode/test/tool/shell.test.ts +++ b/packages/opencode/test/tool/shell.test.ts @@ -6,6 +6,7 @@ import { BashTool } from "../../src/tool/shell/bash" import { ShellTool } from "../../src/tool/shell/id" import { PwshTool } from "../../src/tool/shell/pwsh" import { PowershellTool } from "../../src/tool/shell/powershell" +import { ShellRunner } from "../../src/tool/shell/runner" import { Instance } from "../../src/project/instance" import { Filesystem } from "../../src/util/filesystem" import { tmpdir } from "../fixture/fixture" @@ -966,6 +967,32 @@ describe("tool.shell runtime", () => { }) }) + each("does not hang when already aborted", async (item) => { + const controller = new AbortController() + controller.abort() + const result = await Promise.race([ + ShellRunner.run( + { + shell: item.shell, + name: item.label, + command: js("setTimeout(()=>{},30000)"), + cwd: projectRoot, + env: process.env, + timeout: 500, + description: "Already aborted", + }, + { + ...ctx, + abort: controller.signal, + }, + ), + Bun.sleep(1500).then(() => "timeout" as const), + ]) + expect(result).not.toBe("timeout") + if (result === "timeout") return + expect(result.output).toContain("User aborted the command") + }) + each("terminates command on timeout", async () => { await Instance.provide({ directory: projectRoot, @@ -1055,6 +1082,25 @@ describe("tool.shell runtime", () => { }) }) + each("preserves multibyte utf8 output across chunks", async (item) => { + const result = await ShellRunner.run( + { + shell: item.shell, + name: item.label, + command: js( + "process.stdout.write(Buffer.from([0xF0,0x9F]));setTimeout(()=>process.stdout.write(Buffer.from([0x98,0x80])),20);setTimeout(()=>process.exit(0),40)", + ), + cwd: projectRoot, + env: process.env, + timeout: 1000, + description: "Utf8 output", + }, + ctx, + ) + expect(result.output).toContain("😀") + expect(result.output).not.toContain("\ufffd") + }) + each("streams metadata updates progressively", async () => { await Instance.provide({ directory: projectRoot,