fix(windows): tighten PowerShell shell handling

Centralize shell normalization and move bash permission inference onto native path handling so Windows shells behave consistently without Git Bash internals. Add focused shell, PTY, and bash tool coverage to keep Git Bash, pwsh, powershell, and cmd working on Windows.
This commit is contained in:
LukeParkerDev
2026-03-06 15:02:17 +10:00
parent a6265531d6
commit 4bcfb37567
7 changed files with 669 additions and 459 deletions

View File

@@ -121,7 +121,7 @@ export namespace Pty {
const id = Identifier.create("pty", false)
const command = input.command || Shell.preferred()
const args = input.args || []
if (command.endsWith("sh")) {
if (Shell.login(command)) {
args.push("-l")
}

View File

@@ -1565,9 +1565,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
}
await Session.updatePart(part)
const shell = Shell.preferred()
const shellName = (
process.platform === "win32" ? path.win32.basename(shell, ".exe") : path.basename(shell)
).toLowerCase()
const shellName = Shell.name(shell)
const invocations: Record<string, { args: string[] }> = {
nu: {

View File

@@ -8,6 +8,9 @@ import { setTimeout as sleep } from "node:timers/promises"
const SIGKILL_TIMEOUT_MS = 200
export namespace Shell {
const BLACKLIST = new Set(["fish", "nu"])
const LOGIN = new Set(["bash", "dash", "fish", "ksh", "sh", "zsh"])
export async function killTree(proc: ChildProcess, opts?: { exited?: () => boolean }): Promise<void> {
const pid = proc.pid
if (!pid || opts?.exited?.()) return
@@ -35,12 +38,12 @@ export namespace Shell {
}
}
}
const BLACKLIST = new Set(["fish", "nu"])
function full(file: string) {
if (process.platform !== "win32") return file
if (path.win32.dirname(file) !== ".") return file
return Bun.which(file) || file
const shell = Filesystem.windowsPath(file)
if (path.win32.dirname(shell) !== ".") return shell
return Bun.which(shell) || shell
}
function pick() {
@@ -50,6 +53,15 @@ export namespace Shell {
if (powershell) return powershell
}
function select(file: string | undefined, opts?: { acceptable?: boolean }) {
if (file && (!opts?.acceptable || !BLACKLIST.has(name(file)))) return full(file)
if (process.platform === "win32") {
const shell = pick()
if (shell) return shell
}
return fallback()
}
function fallback() {
if (process.platform === "win32") {
if (Flag.OPENCODE_GIT_BASH_PATH) return Flag.OPENCODE_GIT_BASH_PATH
@@ -68,25 +80,16 @@ export namespace Shell {
return "/bin/sh"
}
export const preferred = lazy(() => {
const s = process.env.SHELL
if (s) return full(s)
if (process.platform === "win32") {
const shell = pick()
if (shell) return shell
}
return fallback()
})
export function name(file: string) {
if (process.platform === "win32") return path.win32.parse(Filesystem.windowsPath(file)).name.toLowerCase()
return path.basename(file).toLowerCase()
}
export const acceptable = lazy(() => {
const s = process.env.SHELL
if (s && !BLACKLIST.has(process.platform === "win32" ? path.win32.basename(s, ".exe") : path.basename(s))) {
return full(s)
}
if (process.platform === "win32") {
const shell = pick()
if (shell) return shell
}
return fallback()
})
export function login(file: string) {
return LOGIN.has(name(file))
}
export const preferred = lazy(() => select(process.env.SHELL))
export const acceptable = lazy(() => select(process.env.SHELL, { acceptable: true }))
}

View File

@@ -1,4 +1,5 @@
import z from "zod"
import os from "os"
import { spawn } from "child_process"
import { Tool } from "./tool"
import path from "path"
@@ -8,7 +9,6 @@ import { Instance } from "../project/instance"
import { lazy } from "@/util/lazy"
import { Language, type Node } from "web-tree-sitter"
import { $ } from "bun"
import { Filesystem } from "@/util/filesystem"
import { fileURLToPath } from "url"
import { Flag } from "@/flag/flag.ts"
@@ -21,8 +21,9 @@ import { Plugin } from "@/plugin"
const MAX_METADATA_LENGTH = 30_000
const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000
const PS = new Set(["powershell", "pwsh"])
const PATHS = new Set([
"cd",
const CWD = new Set(["cd", "push-location", "set-location"])
const FILES = new Set([
...CWD,
"rm",
"cp",
"mv",
@@ -43,6 +44,19 @@ const PATHS = new Set([
"new-item",
"rename-item",
])
const FLAGS = new Set(["-destination", "-literalpath", "-path"])
const SWITCHES = new Set(["-confirm", "-debug", "-force", "-nonewline", "-recurse", "-verbose", "-whatif"])
type Part = {
type: string
text: string
}
type Scan = {
dirs: Set<string>
patterns: Set<string>
always: Set<string>
}
export const log = Log.create({ service: "bash-tool" })
@@ -54,7 +68,7 @@ const resolveWasm = (asset: string) => {
}
function parts(node: Node) {
const out = []
const out: Part[] = []
for (let i = 0; i < node.childCount; i++) {
const child = node.child(i)
if (!child) continue
@@ -62,7 +76,7 @@ function parts(node: Node) {
for (let j = 0; j < child.childCount; j++) {
const item = child.child(j)
if (!item || item.type === "command_argument_sep" || item.type === "redirection") continue
out.push(item.text)
out.push({ type: item.type, text: item.text })
}
continue
}
@@ -76,11 +90,322 @@ function parts(node: Node) {
) {
continue
}
out.push(child.text)
out.push({ type: child.type, text: child.text })
}
return out
}
function commandText(node: Node) {
return (node.parent?.type === "redirected_statement" ? node.parent.text : node.text).trim()
}
function nested(node: Node) {
let parent = node.parent
while (parent) {
if (parent.type === "command") return true
parent = parent.parent
}
return false
}
function commands(node: Node) {
const out: Node[] = []
for (const child of node.descendantsOfType("command")) {
if (!child || nested(child)) continue
out.push(child)
}
return out
}
function unquote(text: string) {
if (text.length < 2) return text
const first = text[0]
const last = text[text.length - 1]
if ((first === '"' || first === "'") && first === last) return text.slice(1, -1)
return text
}
function home(text: string) {
if (text === "~") return os.homedir()
if (text.startsWith("~/") || text.startsWith("~\\")) return path.join(os.homedir(), text.slice(2))
return text
}
function envValue(key: string) {
if (process.platform !== "win32") return process.env[key]
const name = Object.keys(process.env).find((item) => item.toLowerCase() === key.toLowerCase())
return name ? process.env[name] : undefined
}
function expandEnv(text: string) {
let ok = true
const out = unquote(text).replace(/\$env:([A-Za-z_][A-Za-z0-9_]*)/gi, (_, key: string) => {
const value = envValue(key)
if (!value) {
ok = false
return ""
}
return value
})
if (!ok) return
return home(out)
}
function drive(text: string) {
return /^[A-Za-z]:($|[\\/])/.test(text)
}
function provider(text: string) {
return /^[A-Za-z]+:/.test(text) && !drive(text)
}
function dynamicPath(text: string, ps: boolean) {
if (text.startsWith("(") || text.startsWith("@(")) return true
if (text.includes("$(") || text.includes("${") || text.includes("`")) return true
if (/[?*\[]/.test(text)) return true
if (ps) return /\$(?!env:)/i.test(text)
return text.includes("$")
}
function resolvePath(text: string, root: string) {
if (process.platform === "win32") {
const file = Filesystem.windowsPath(text)
const full = path.isAbsolute(file) ? path.resolve(file) : path.resolve(root, file)
return Filesystem.normalizePath(full)
}
return path.isAbsolute(text) ? path.resolve(text) : path.resolve(root, text)
}
function argPath(arg: string, cwd: string, ps: boolean) {
const text = ps ? expandEnv(arg) : home(unquote(arg))
if (!text || dynamicPath(text, ps)) return
if (ps && provider(text)) return
return resolvePath(text, cwd)
}
function pathArgs(list: Part[], ps: boolean) {
if (!ps) {
return list
.slice(1)
.filter((item) => !item.text.startsWith("-") && !(list[0]?.text === "chmod" && item.text.startsWith("+")))
.map((item) => item.text)
}
const out: string[] = []
let want = false
let skip = false
for (const item of list.slice(1)) {
if (want) {
out.push(item.text)
want = false
continue
}
if (skip) {
skip = false
continue
}
if (item.type === "command_parameter") {
const flag = item.text.toLowerCase()
if (SWITCHES.has(flag)) continue
want = FLAGS.has(flag)
skip = !want
continue
}
out.push(item.text)
}
return out
}
async function collect(root: Node, cwd: string, ps: boolean): Promise<Scan> {
const scan: Scan = {
dirs: new Set<string>(),
patterns: new Set<string>(),
always: new Set<string>(),
}
for (const node of commands(root)) {
const command = parts(node)
const tokens = command.map((item) => item.text)
const cmd = ps ? tokens[0]?.toLowerCase() : tokens[0]
if (cmd && FILES.has(cmd)) {
for (const arg of pathArgs(command, ps)) {
const resolved = argPath(arg, cwd, ps)
log.info("resolved path", { arg, resolved })
if (!resolved || Instance.containsPath(resolved)) continue
const dir = (await Filesystem.isDir(resolved)) ? resolved : path.dirname(resolved)
scan.dirs.add(dir)
}
}
if (tokens.length && (!cmd || !CWD.has(cmd))) {
scan.patterns.add(commandText(node))
scan.always.add(BashArity.prefix(tokens).join(" ") + " *")
}
}
return scan
}
function preview(text: string) {
if (text.length <= MAX_METADATA_LENGTH) return text
return text.slice(0, MAX_METADATA_LENGTH) + "\n\n..."
}
async function parse(command: string, ps: boolean) {
const tree = await parser().then((p) => (ps ? p.ps : p.bash).parse(command))
if (!tree) throw new Error("Failed to parse command")
return tree.rootNode
}
async function ask(ctx: Tool.Context, scan: Scan) {
if (scan.dirs.size > 0) {
const globs = Array.from(scan.dirs).map((dir) => {
if (process.platform === "win32") return Filesystem.normalizePathPattern(path.join(dir, "*"))
return path.join(dir, "*")
})
await ctx.ask({
permission: "external_directory",
patterns: globs,
always: globs,
metadata: {},
})
}
if (scan.patterns.size === 0) return
await ctx.ask({
permission: "bash",
patterns: Array.from(scan.patterns),
always: Array.from(scan.always),
metadata: {},
})
}
async function shellEnv(ctx: Tool.Context, cwd: string) {
const extra = await Plugin.trigger("shell.env", { cwd, sessionID: ctx.sessionID, callID: ctx.callID }, { env: {} })
return {
...process.env,
...extra.env,
}
}
function launch(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], {
cwd,
env,
stdio: ["ignore", "pipe", "pipe"],
detached: false,
})
}
return spawn(command, {
shell,
cwd,
env,
stdio: ["ignore", "pipe", "pipe"],
detached: process.platform !== "win32",
})
}
async function run(
input: {
shell: string
name: string
command: string
cwd: string
env: NodeJS.ProcessEnv
timeout: number
description: string
},
ctx: Tool.Context,
) {
const proc = launch(input.shell, input.name, input.command, input.cwd, input.env)
let output = ""
ctx.metadata({
metadata: {
output: "",
description: input.description,
},
})
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 timedOut = false
let aborted = false
let exited = false
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(() => {
timedOut = true
void kill()
}, input.timeout + 100)
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
clearTimeout(timer)
ctx.abort.removeEventListener("abort", abort)
}
proc.once("exit", () => {
exited = true
})
proc.once("close", () => {
exited = true
cleanup()
resolve()
})
proc.once("error", (error) => {
exited = true
cleanup()
reject(error)
})
})
const metadata: string[] = []
if (timedOut) 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>"
}
return {
title: input.description,
metadata: {
output: preview(output),
exit: proc.exitCode,
description: input.description,
},
output,
}
}
const parser = lazy(async () => {
const { Parser } = await import("web-tree-sitter")
const { default: treeWasm } = await import("web-tree-sitter/tree-sitter.wasm" as string, {
@@ -111,10 +436,9 @@ 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 = Shell.acceptable()
const name = process.platform === "win32" ? path.win32.basename(shell, ".exe") : path.basename(shell)
const lower = name.toLowerCase()
const name = Shell.name(shell)
const chain =
lower === "powershell"
name === "powershell"
? "If the commands depend on each other and must run sequentially, avoid '&&' in this shell because Windows PowerShell 5.1 does not support it. Use PowerShell conditionals such as `cmd1; if ($?) { cmd2 }` when later commands must depend on earlier success."
: "If the commands depend on each other and must run sequentially, use a single Bash call with '&&' to chain them together (e.g., `git add . && git commit -m \"message\" && git push`). For instance, if one operation must complete before another starts (like mkdir before cp, Write before Bash for git operations, or git add before git commit), run these operations sequentially instead."
log.info("bash tool using shell", { shell })
@@ -142,203 +466,29 @@ export const BashTool = Tool.define("bash", async () => {
),
}),
async execute(params, ctx) {
const cwd =
process.platform === "win32" && path.isAbsolute(params.workdir || Instance.directory)
? Filesystem.normalizePath(params.workdir || Instance.directory)
: params.workdir || Instance.directory
const cwd = resolvePath(params.workdir || Instance.directory, Instance.directory)
if (params.timeout !== undefined && params.timeout < 0) {
throw new Error(`Invalid timeout value: ${params.timeout}. Timeout must be a positive number.`)
}
const timeout = params.timeout ?? DEFAULT_TIMEOUT
const tree = await parser().then((p) => (PS.has(lower) ? p.ps : p.bash).parse(params.command))
if (!tree) {
throw new Error("Failed to parse command")
}
const directories = new Set<string>()
if (!Instance.containsPath(cwd)) directories.add(cwd)
const patterns = new Set<string>()
const always = new Set<string>()
const ps = PS.has(name)
const root = await parse(params.command, ps)
const scan = await collect(root, cwd, ps)
if (!Instance.containsPath(cwd)) scan.dirs.add(cwd)
await ask(ctx, scan)
for (const node of tree.rootNode.descendantsOfType("command")) {
if (!node) continue
// Get full command text including redirects if present
let commandText = node.parent?.type === "redirected_statement" ? node.parent.text : node.text
commandText = commandText.trim()
const command = parts(node)
const cmd = PS.has(lower) ? command[0]?.toLowerCase() : command[0]
// not an exhaustive list, but covers most common cases
if (cmd && PATHS.has(cmd)) {
for (const arg of command.slice(1)) {
if (arg.startsWith("-") || (cmd === "chmod" && arg.startsWith("+"))) continue
const resolved = await $`realpath ${arg}`
.cwd(cwd)
.quiet()
.nothrow()
.text()
.then((x) => x.trim())
log.info("resolved path", { arg, resolved })
if (resolved) {
const normalized = process.platform === "win32" ? Filesystem.normalizePath(resolved) : resolved
if (!Instance.containsPath(normalized)) {
const dir = (await Filesystem.isDir(normalized)) ? normalized : path.dirname(normalized)
directories.add(dir)
}
}
}
}
// cd covered by above check
if (command.length && cmd !== "cd") {
patterns.add(commandText)
always.add(BashArity.prefix(command).join(" ") + " *")
}
}
if (directories.size > 0) {
const globs = Array.from(directories).map((dir) => {
if (process.platform === "win32") return Filesystem.normalizePathPattern(path.join(dir, "*"))
// Preserve POSIX-looking paths with /s, even on Windows
if (dir.startsWith("/")) return `${dir.replace(/[\\/]+$/, "")}/*`
return path.join(dir, "*")
})
await ctx.ask({
permission: "external_directory",
patterns: globs,
always: globs,
metadata: {},
})
}
if (patterns.size > 0) {
await ctx.ask({
permission: "bash",
patterns: Array.from(patterns),
always: Array.from(always),
metadata: {},
})
}
const shellEnv = await Plugin.trigger(
"shell.env",
{ cwd, sessionID: ctx.sessionID, callID: ctx.callID },
{ env: {} },
return run(
{
shell,
name,
command: params.command,
cwd,
env: await shellEnv(ctx, cwd),
timeout,
description: params.description,
},
ctx,
)
const env = {
...process.env,
...shellEnv.env,
}
const proc =
process.platform === "win32" && ["pwsh", "powershell"].includes(name.toLowerCase())
? spawn(shell, ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", params.command], {
cwd,
env,
stdio: ["ignore", "pipe", "pipe"],
detached: process.platform !== "win32",
})
: spawn(params.command, {
shell,
cwd,
env,
stdio: ["ignore", "pipe", "pipe"],
detached: process.platform !== "win32",
})
let output = ""
// Initialize metadata with empty output
ctx.metadata({
metadata: {
output: "",
description: params.description,
},
})
const append = (chunk: Buffer) => {
output += chunk.toString()
ctx.metadata({
metadata: {
// truncate the metadata to avoid GIANT blobs of data (has nothing to do w/ what agent can access)
output: output.length > MAX_METADATA_LENGTH ? output.slice(0, MAX_METADATA_LENGTH) + "\n\n..." : output,
description: params.description,
},
})
}
proc.stdout?.on("data", append)
proc.stderr?.on("data", append)
let timedOut = false
let aborted = false
let exited = false
const kill = () => Shell.killTree(proc, { exited: () => exited })
if (ctx.abort.aborted) {
aborted = true
await kill()
}
const abortHandler = () => {
aborted = true
void kill()
}
ctx.abort.addEventListener("abort", abortHandler, { once: true })
const timeoutTimer = setTimeout(() => {
timedOut = true
void kill()
}, timeout + 100)
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
clearTimeout(timeoutTimer)
ctx.abort.removeEventListener("abort", abortHandler)
}
proc.once("exit", () => {
exited = true
})
proc.once("close", () => {
exited = true
cleanup()
resolve()
})
proc.once("error", (error) => {
exited = true
cleanup()
reject(error)
})
})
const resultMetadata: string[] = []
if (timedOut) {
resultMetadata.push(`bash tool terminated command after exceeding timeout ${timeout} ms`)
}
if (aborted) {
resultMetadata.push("User aborted the command")
}
if (resultMetadata.length > 0) {
output += "\n\n<bash_metadata>\n" + resultMetadata.join("\n") + "\n</bash_metadata>"
}
return {
title: params.description,
metadata: {
output: output.length > MAX_METADATA_LENGTH ? output.slice(0, MAX_METADATA_LENGTH) + "\n\n..." : output,
exit: proc.exitCode,
description: params.description,
},
output,
}
},
}
})

View File

@@ -0,0 +1,52 @@
import { describe, expect, test } from "bun:test"
import { Instance } from "../../src/project/instance"
import { Pty } from "../../src/pty"
import { tmpdir } from "../fixture/fixture"
describe("pty shell args", () => {
if (process.platform !== "win32") return
const ps = Bun.which("pwsh") || Bun.which("powershell")
if (ps) {
test(
"does not add login args to pwsh",
async () => {
await using dir = await tmpdir()
await Instance.provide({
directory: dir.path,
fn: async () => {
const info = await Pty.create({ command: ps, title: "pwsh" })
try {
expect(info.args).toEqual([])
} finally {
await Pty.remove(info.id)
}
},
})
},
{ timeout: 30000 },
)
}
const bash = Bun.which("bash")
if (bash) {
test(
"adds login args to bash",
async () => {
await using dir = await tmpdir()
await Instance.provide({
directory: dir.path,
fn: async () => {
const info = await Pty.create({ command: bash, title: "bash" })
try {
expect(info.args).toEqual(["-l"])
} finally {
await Pty.remove(info.id)
}
},
})
},
{ timeout: 30000 },
)
}
})

View File

@@ -0,0 +1,58 @@
import { describe, expect, test } from "bun:test"
import path from "path"
import { Shell } from "../../src/shell/shell"
import { Filesystem } from "../../src/util/filesystem"
const withShell = async (shell: string | undefined, fn: () => void | Promise<void>) => {
const prev = process.env.SHELL
if (shell === undefined) delete process.env.SHELL
else process.env.SHELL = shell
Shell.acceptable.reset()
Shell.preferred.reset()
try {
await fn()
} finally {
if (prev === undefined) delete process.env.SHELL
else process.env.SHELL = prev
Shell.acceptable.reset()
Shell.preferred.reset()
}
}
describe("shell", () => {
test("normalizes shell names", () => {
expect(Shell.name("/bin/bash")).toBe("bash")
if (process.platform === "win32") {
expect(Shell.name("C:/tools/NU.EXE")).toBe("nu")
expect(Shell.name("C:/tools/PWSH.EXE")).toBe("pwsh")
}
})
test("detects login shells", () => {
expect(Shell.login("/bin/bash")).toBe(true)
expect(Shell.login("C:/tools/pwsh.exe")).toBe(false)
})
if (process.platform === "win32") {
test("rejects blacklisted shells case-insensitively", async () => {
await withShell("NU.EXE", async () => {
expect(Shell.name(Shell.acceptable())).not.toBe("nu")
})
})
test("normalizes Git Bash shell paths from env", async () => {
const shell = "/cygdrive/c/Program Files/Git/bin/bash.exe"
await withShell(shell, async () => {
expect(Shell.preferred()).toBe(Filesystem.windowsPath(shell))
})
})
test("resolves bare PowerShell shells", async () => {
const shell = Bun.which("pwsh") || Bun.which("powershell")
if (!shell) return
await withShell(path.win32.basename(shell), async () => {
expect(Shell.preferred()).toBe(shell)
})
})
}
})

View File

@@ -21,17 +21,8 @@ const ctx = {
}
const projectRoot = path.join(__dirname, "../..")
const win = process.env.WINDIR?.replaceAll("\\", "/")
const bin = process.execPath.replaceAll("\\", "/")
const file = path.join(projectRoot, "test/tool/fixtures/output.ts").replaceAll("\\", "/")
const kind = () => path.win32.basename(process.env.SHELL || "", ".exe").toLowerCase()
const fill = (mode: "lines" | "bytes", n: number) => {
if (["pwsh", "powershell"].includes(kind())) {
if (mode === "lines") return `1..${n} | ForEach-Object { $_ }`
return `Write-Output ('a' * ${n})`
}
return `${bin} ${file} ${mode} ${n}`
}
const shells = (() => {
if (process.platform !== "win32") {
const shell = process.env.SHELL || Bun.which("bash") || "/bin/sh"
@@ -45,11 +36,16 @@ const shells = (() => {
{ label: "cmd", shell: process.env.COMSPEC || Bun.which("cmd.exe") },
].filter((item): item is { label: string; shell: string } => Boolean(item.shell))
return list.filter((item, i) => list.findIndex((x) => x.shell.toLowerCase() === item.shell.toLowerCase()) === i)
return list.filter(
(item, i) => list.findIndex((other) => other.shell.toLowerCase() === item.shell.toLowerCase()) === i,
)
})()
const ps = shells.filter((item) => ["pwsh", "powershell"].includes(item.label))
const ps = shells.filter((item) => item.label === "pwsh" || item.label === "powershell")
const fill = (mode: "lines" | "bytes", n: number) => `${bin} ${file} ${mode} ${n}`
const glob = (p: string) =>
process.platform === "win32" ? Filesystem.normalizePathPattern(p) : p.replaceAll("\\", "/")
const forms = (dir: string) => {
if (process.platform !== "win32") return [dir]
const full = Filesystem.normalizePath(dir)
@@ -58,39 +54,45 @@ const forms = (dir: string) => {
return Array.from(new Set([full, slash, root, root.toLowerCase()]))
}
const withShell =
(item: { label: string; shell: string }, fn: (item: { label: string; shell: string }) => Promise<void>) =>
async () => {
const prev = process.env.SHELL
process.env.SHELL = item.shell
const withShell = (item: { label: string; shell: string }, fn: () => Promise<void>) => async () => {
const prev = process.env.SHELL
process.env.SHELL = item.shell
Shell.acceptable.reset()
Shell.preferred.reset()
try {
await fn()
} finally {
if (prev === undefined) delete process.env.SHELL
else process.env.SHELL = prev
Shell.acceptable.reset()
Shell.preferred.reset()
try {
await fn(item)
} finally {
if (prev === undefined) delete process.env.SHELL
else process.env.SHELL = prev
Shell.acceptable.reset()
Shell.preferred.reset()
}
}
const each = (name: string, fn: (item: { label: string; shell: string }) => Promise<void>) => {
for (const item of shells) {
test(`${name} [${item.label}]`, withShell(item, fn))
}
}
const mustTruncate = (result: { metadata: any; output: string }, item: { label: string; shell: string }) => {
const each = (name: string, fn: (item: { label: string; shell: string }) => Promise<void>) => {
for (const item of shells) {
test(
`${name} [${item.label}]`,
withShell(item, () => fn(item)),
)
}
}
const capture = (requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">>, stop?: Error) => ({
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
if (stop) throw stop
},
})
const mustTruncate = (result: {
metadata: { truncated?: boolean; exit?: number | null } & Record<string, unknown>
output: string
}) => {
if (result.metadata.truncated) return
throw new Error(
[
`shell: ${item.label}`,
`path: ${item.shell}`,
`exit: ${String(result.metadata.exit)}`,
"output:",
result.output,
].join("\n"),
[`shell: ${process.env.SHELL || ""}`, `exit: ${String(result.metadata.exit)}`, "output:", result.output].join("\n"),
)
}
@@ -102,7 +104,7 @@ describe("tool.bash", () => {
const bash = await BashTool.init()
const result = await bash.execute(
{
command: "echo 'test'",
command: "echo test",
description: "Echo test message",
},
ctx,
@@ -116,24 +118,18 @@ describe("tool.bash", () => {
describe("tool.bash permissions", () => {
each("asks for bash permission with correct pattern", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "echo hello",
description: "Echo hello",
},
testCtx,
capture(requests),
)
expect(requests.length).toBe(1)
expect(requests[0].permission).toBe("bash")
@@ -143,24 +139,18 @@ describe("tool.bash permissions", () => {
})
each("asks for bash permission with multiple commands", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "echo foo && echo bar",
description: "Echo twice",
},
testCtx,
capture(requests),
)
expect(requests.length).toBe(1)
expect(requests[0].permission).toBe("bash")
@@ -179,54 +169,40 @@ describe("tool.bash permissions", () => {
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "Write-Host foo; if ($?) { Write-Host bar }",
description: "Check PowerShell conditional",
},
testCtx,
capture(requests),
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("Write-Host foo")
expect(bashReq!.patterns).toContain("Write-Host bar")
expect(bashReq!.patterns).not.toContain("0")
expect(bashReq!.always).toContain("Write-Host *")
expect(bashReq!.always).not.toContain("0 *")
},
})
}),
)
}
if (win) {
if (process.platform === "win32") {
for (const item of ps) {
test(
`asks for external_directory permission for PowerShell aliases [${item.label}]`,
`asks for external_directory permission for PowerShell env paths [${item.label}]`,
withShell(item, async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: `cat ${win}/win.ini`,
description: "Read Windows ini",
command: "Get-Content $env:WINDIR/win.ini",
description: "Read Windows ini from env",
},
testCtx,
capture(requests),
)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
@@ -241,31 +217,53 @@ describe("tool.bash permissions", () => {
for (const item of ps) {
test(
`asks for external_directory permission for PowerShell cmdlets [${item.label}]`,
`treats Set-Location like cd for permissions [${item.label}]`,
withShell(item, async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: `Get-Content ${win}/win.ini`,
description: "Read Windows ini",
command: "Set-Location C:/Windows",
description: "Change location",
},
testCtx,
capture(requests),
)
const extDirReq = requests.find((r) => r.permission === "external_directory")
const bashReq = requests.find((r) => r.permission === "bash")
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns).toContain(
Filesystem.normalizePathPattern(path.join(process.env.WINDIR!, "*")),
)
expect(bashReq).toBeUndefined()
},
})
}),
)
}
for (const item of ps) {
test(
`does not add nested PowerShell expressions to permission prompts [${item.label}]`,
withShell(item, async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
await bash.execute(
{
command: "Write-Output ('a' * 3)",
description: "Write repeated text",
},
capture(requests),
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).not.toContain("a * 3")
expect(bashReq!.always).not.toContain("a *")
},
})
}),
@@ -274,25 +272,22 @@ describe("tool.bash permissions", () => {
}
each("asks for external_directory permission when cd to parent", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const err = new Error("stop after permission")
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "cd ../",
description: "Change to parent directory",
},
testCtx,
)
await expect(
bash.execute(
{
command: "cd ../",
description: "Change to parent directory",
},
capture(requests, err),
),
).rejects.toThrow(err.message)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
},
@@ -300,26 +295,23 @@ describe("tool.bash permissions", () => {
})
each("asks for external_directory permission when workdir is outside project", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const err = new Error("stop after permission")
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "echo ok",
workdir: os.tmpdir(),
description: "Echo from temp dir",
},
testCtx,
)
await expect(
bash.execute(
{
command: "echo ok",
workdir: os.tmpdir(),
description: "Echo from temp dir",
},
capture(requests, err),
),
).rejects.toThrow(err.message)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns).toContain(glob(path.join(os.tmpdir(), "*")))
@@ -328,54 +320,39 @@ describe("tool.bash permissions", () => {
})
if (process.platform === "win32") {
for (const item of shells) {
test(
`normalizes external_directory workdir variants on Windows [${item.label}]`,
withShell(item, async () => {
// This test only cares about the permission payload, so stop before the
// shell spawns to avoid slow Windows PowerShell startup dominating CI.
const err = new Error("stop after permission")
await using outerTmp = await tmpdir()
await using tmp = await tmpdir({ git: true })
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const want = Filesystem.normalizePathPattern(path.join(outerTmp.path, "*"))
test("normalizes external_directory workdir variants on Windows", async () => {
const err = new Error("stop after permission")
await using outerTmp = await tmpdir()
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const want = Filesystem.normalizePathPattern(path.join(outerTmp.path, "*"))
for (const dir of forms(outerTmp.path)) {
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
throw err
},
}
for (const dir of forms(outerTmp.path)) {
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
await expect(
bash.execute(
{
command: "echo ok",
workdir: dir,
description: "Echo from external dir",
},
capture(requests, err),
),
).rejects.toThrow(err.message)
await expect(
bash.execute(
{
command: "echo ok",
workdir: dir,
description: "Echo from external dir",
},
testCtx,
),
).rejects.toThrow(err.message)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect({ dir, patterns: extDirReq?.patterns, always: extDirReq?.always }).toEqual({
dir,
patterns: [want],
always: [want],
})
}
},
})
}),
)
}
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect({ dir, patterns: extDirReq?.patterns, always: extDirReq?.always }).toEqual({
dir,
patterns: [want],
always: [want],
})
}
},
})
})
}
each("asks for external_directory permission when file arg is outside project", async () => {
@@ -384,28 +361,25 @@ describe("tool.bash permissions", () => {
await Bun.write(path.join(dir, "outside.txt"), "x")
},
})
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const err = new Error("stop after permission")
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
const filepath = path.join(outerTmp.path, "outside.txt")
await bash.execute(
{
command: `cat ${filepath}`,
description: "Read external file",
},
testCtx,
)
await expect(
bash.execute(
{
command: `cat ${filepath}`,
description: "Read external file",
},
capture(requests, err),
),
).rejects.toThrow(err.message)
const extDirReq = requests.find((r) => r.permission === "external_directory")
const expected = path.join(outerTmp.path, "*")
const expected = glob(path.join(outerTmp.path, "*"))
expect(extDirReq).toBeDefined()
expect(extDirReq!.patterns).toContain(expected)
expect(extDirReq!.always).toContain(expected)
@@ -414,29 +388,23 @@ describe("tool.bash permissions", () => {
})
each("does not ask for external_directory permission when rm inside project", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir({
init: async (dir) => {
await Bun.write(path.join(dir, "tmpfile"), "x")
},
})
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await Bun.write(path.join(tmp.path, "tmpfile"), "x")
await bash.execute(
{
command: `rm -rf ${path.join(tmp.path, "nested")}`,
description: "remove nested dir",
description: "Remove nested dir",
},
testCtx,
capture(requests),
)
const extDirReq = requests.find((r) => r.permission === "external_directory")
expect(extDirReq).toBeUndefined()
},
@@ -444,51 +412,39 @@ describe("tool.bash permissions", () => {
})
each("includes always patterns for auto-approval", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "git log --oneline -5",
description: "Git log",
},
testCtx,
capture(requests),
)
expect(requests.length).toBe(1)
expect(requests[0].always.length).toBeGreaterThan(0)
expect(requests[0].always.some((p) => p.endsWith("*"))).toBe(true)
expect(requests[0].always.some((item) => item.endsWith("*"))).toBe(true)
},
})
})
each("does not ask for bash permission when command is cd only", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute(
{
command: "cd .",
description: "Stay in current directory",
},
testCtx,
capture(requests),
)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeUndefined()
@@ -497,19 +453,19 @@ describe("tool.bash permissions", () => {
})
each("matches redirects in permission pattern", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const err = new Error("stop after permission")
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute({ command: "echo test > output.txt", description: "Redirect test output" }, testCtx)
await expect(
bash.execute(
{ command: "echo test > output.txt", description: "Redirect test output" },
capture(requests, err),
),
).rejects.toThrow(err.message)
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
expect(bashReq!.patterns).toContain("echo test > output.txt")
@@ -518,30 +474,23 @@ describe("tool.bash permissions", () => {
})
each("always pattern has space before wildcard to not include different commands", async () => {
await using tmp = await tmpdir({ git: true })
await using tmp = await tmpdir()
await Instance.provide({
directory: tmp.path,
fn: async () => {
const bash = await BashTool.init()
const requests: Array<Omit<PermissionNext.Request, "id" | "sessionID" | "tool">> = []
const testCtx = {
...ctx,
ask: async (req: Omit<PermissionNext.Request, "id" | "sessionID" | "tool">) => {
requests.push(req)
},
}
await bash.execute({ command: "ls -la", description: "List" }, testCtx)
await bash.execute({ command: "ls -la", description: "List" }, capture(requests))
const bashReq = requests.find((r) => r.permission === "bash")
expect(bashReq).toBeDefined()
const pattern = bashReq!.always[0]
expect(pattern).toBe("ls *")
expect(bashReq!.always[0]).toBe("ls *")
},
})
})
})
describe("tool.bash truncation", () => {
each("truncates output exceeding line limit", async (item) => {
test("truncates output exceeding line limit", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
@@ -554,14 +503,14 @@ describe("tool.bash truncation", () => {
},
ctx,
)
mustTruncate(result, item)
mustTruncate(result)
expect(result.output).toContain("truncated")
expect(result.output).toContain("The tool call succeeded but the output was truncated")
},
})
})
each("truncates output exceeding byte limit", async (item) => {
test("truncates output exceeding byte limit", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
@@ -574,14 +523,14 @@ describe("tool.bash truncation", () => {
},
ctx,
)
mustTruncate(result, item)
mustTruncate(result)
expect(result.output).toContain("truncated")
expect(result.output).toContain("The tool call succeeded but the output was truncated")
},
})
})
each("does not truncate small output", async () => {
test("does not truncate small output", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
@@ -593,13 +542,13 @@ describe("tool.bash truncation", () => {
},
ctx,
)
expect((result.metadata as any).truncated).toBe(false)
expect((result.metadata as { truncated?: boolean }).truncated).toBe(false)
expect(result.output).toContain("hello")
},
})
})
each("full output is saved to file when truncated", async (item) => {
test("full output is saved to file when truncated", async () => {
await Instance.provide({
directory: projectRoot,
fn: async () => {
@@ -612,12 +561,12 @@ describe("tool.bash truncation", () => {
},
ctx,
)
mustTruncate(result, item)
mustTruncate(result)
const filepath = (result.metadata as any).outputPath
const filepath = (result.metadata as { outputPath?: string }).outputPath
expect(filepath).toBeTruthy()
const saved = await Filesystem.readText(filepath)
const saved = await Filesystem.readText(filepath!)
const lines = saved.trim().split(/\r?\n/)
expect(lines.length).toBe(lineCount)
expect(lines[0]).toBe("1")