mirror of
https://github.com/anomalyco/opencode.git
synced 2026-05-30 03:54:59 +00:00
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.
This commit is contained in:
parent
32ec3666b7
commit
25551172c9
3 changed files with 86 additions and 34 deletions
|
|
@ -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<Scan> {
|
||||
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<string>(),
|
||||
}
|
||||
|
||||
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 })
|
||||
|
|
|
|||
|
|
@ -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<typeof setTimeout>
|
||||
|
||||
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<void>((resolve, reject) => {
|
||||
const wait = new Promise<void>((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")
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue