refactor(core): replace bash tool with shell tool
This commit is contained in:
Dax Raad
parent
595c6bd4a7
commit
5ae93092aa
37 changed files with 2260 additions and 901 deletions
|
|
@ -143,6 +143,27 @@ describe("Config", () => {
|
|||
}),
|
||||
)
|
||||
|
||||
it.effect("normalizes renamed permission actions when migrating v1 permissions", () =>
|
||||
Effect.sync(() => {
|
||||
expect(
|
||||
ConfigMigrateV1.migrate({
|
||||
permission: {
|
||||
task: "ask",
|
||||
bash: { "git status": "allow", "*": "deny" },
|
||||
write: "deny",
|
||||
read: "allow",
|
||||
},
|
||||
}).permissions,
|
||||
).toEqual([
|
||||
{ action: "subagent", resource: "*", effect: "ask" },
|
||||
{ action: "shell", resource: "git status", effect: "allow" },
|
||||
{ action: "shell", resource: "*", effect: "deny" },
|
||||
{ action: "edit", resource: "*", effect: "deny" },
|
||||
{ action: "read", resource: "*", effect: "allow" },
|
||||
])
|
||||
}),
|
||||
)
|
||||
|
||||
it.live("returns an empty configuration when directory files do not exist", () =>
|
||||
Effect.acquireRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
|
|
@ -566,7 +587,7 @@ describe("Config", () => {
|
|||
expect(documents[0]?.info.snapshots).toBe(false)
|
||||
expect(documents[0]?.info.share).toBe("auto")
|
||||
expect(documents[0]?.info.permissions).toEqual([
|
||||
{ action: "bash", resource: "*", effect: "ask" },
|
||||
{ action: "shell", resource: "*", effect: "ask" },
|
||||
{ action: "edit", resource: "*.md", effect: "allow" },
|
||||
{ action: "edit", resource: "*", effect: "deny" },
|
||||
{ action: "question", resource: "*", effect: "deny" },
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { describe, expect, test } from "bun:test"
|
||||
import path from "path"
|
||||
import { Shell } from "@opencode-ai/core/shell"
|
||||
import { ShellSelect } from "@opencode-ai/core/shell/select"
|
||||
import { FSUtil } from "@opencode-ai/core/fs-util"
|
||||
import { which } from "@opencode-ai/core/util/which"
|
||||
|
||||
|
|
@ -8,56 +8,56 @@ const withShell = async (shell: string | undefined, fn: () => void | Promise<voi
|
|||
const prev = process.env.SHELL
|
||||
if (shell === undefined) delete process.env.SHELL
|
||||
else process.env.SHELL = shell
|
||||
Shell.acceptable.reset()
|
||||
Shell.preferred.reset()
|
||||
ShellSelect.acceptable.reset()
|
||||
ShellSelect.preferred.reset()
|
||||
try {
|
||||
await fn()
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.SHELL
|
||||
else process.env.SHELL = prev
|
||||
Shell.acceptable.reset()
|
||||
Shell.preferred.reset()
|
||||
ShellSelect.acceptable.reset()
|
||||
ShellSelect.preferred.reset()
|
||||
}
|
||||
}
|
||||
|
||||
describe("shell", () => {
|
||||
test("normalizes shell names", () => {
|
||||
expect(Shell.name("/bin/bash")).toBe("bash")
|
||||
expect(ShellSelect.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")
|
||||
expect(ShellSelect.name("C:/tools/NU.EXE")).toBe("nu")
|
||||
expect(ShellSelect.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)
|
||||
expect(ShellSelect.login("/bin/bash")).toBe(true)
|
||||
expect(ShellSelect.login("C:/tools/pwsh.exe")).toBe(false)
|
||||
})
|
||||
|
||||
test("detects posix shells", () => {
|
||||
expect(Shell.posix("/bin/bash")).toBe(true)
|
||||
expect(Shell.posix("/bin/fish")).toBe(false)
|
||||
expect(Shell.posix("C:/tools/pwsh.exe")).toBe(false)
|
||||
expect(ShellSelect.posix("/bin/bash")).toBe(true)
|
||||
expect(ShellSelect.posix("/bin/fish")).toBe(false)
|
||||
expect(ShellSelect.posix("C:/tools/pwsh.exe")).toBe(false)
|
||||
})
|
||||
|
||||
test("falls back when configured shell cannot be resolved", async () => {
|
||||
await withShell(undefined, async () => {
|
||||
const preferred = Shell.preferred()
|
||||
const acceptable = Shell.acceptable()
|
||||
expect(Shell.preferred("opencode-missing-shell")).toBe(preferred)
|
||||
expect(Shell.acceptable("opencode-missing-shell")).toBe(acceptable)
|
||||
const preferred = ShellSelect.preferred()
|
||||
const acceptable = ShellSelect.acceptable()
|
||||
expect(ShellSelect.preferred("opencode-missing-shell")).toBe(preferred)
|
||||
expect(ShellSelect.acceptable("opencode-missing-shell")).toBe(acceptable)
|
||||
})
|
||||
})
|
||||
|
||||
test("falls back for terminal-only acceptable shells", () => {
|
||||
expect(Shell.name(Shell.acceptable("fish"))).not.toBe("fish")
|
||||
expect(Shell.name(Shell.acceptable("nu"))).not.toBe("nu")
|
||||
expect(ShellSelect.name(ShellSelect.acceptable("fish"))).not.toBe("fish")
|
||||
expect(ShellSelect.name(ShellSelect.acceptable("nu"))).not.toBe("nu")
|
||||
})
|
||||
|
||||
test("builds command args per shell family", () => {
|
||||
expect(Shell.args("/bin/sh", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
|
||||
expect(Shell.args("/usr/bin/fish", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
|
||||
const zsh = Shell.args("/bin/zsh", "echo hi", "/tmp")
|
||||
expect(ShellSelect.args("/bin/sh", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
|
||||
expect(ShellSelect.args("/usr/bin/fish", "echo hi", "/tmp")).toEqual(["-c", "echo hi"])
|
||||
const zsh = ShellSelect.args("/bin/zsh", "echo hi", "/tmp")
|
||||
expect(zsh[0]).toBe("-l")
|
||||
expect(zsh[1]).toBe("-c")
|
||||
expect(zsh.at(-1)).toBe("/tmp")
|
||||
|
|
@ -66,34 +66,34 @@ describe("shell", () => {
|
|||
if (process.platform === "win32") {
|
||||
test("rejects blacklisted shells case-insensitively", async () => {
|
||||
await withShell("NU.EXE", async () => {
|
||||
expect(Shell.name(Shell.acceptable())).not.toBe("nu")
|
||||
expect(ShellSelect.name(ShellSelect.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(FSUtil.windowsPath(shell))
|
||||
expect(ShellSelect.preferred()).toBe(FSUtil.windowsPath(shell))
|
||||
})
|
||||
})
|
||||
|
||||
test("resolves /usr/bin/bash from env to Git Bash", async () => {
|
||||
const bash = Shell.gitbash()
|
||||
const bash = ShellSelect.gitbash()
|
||||
if (!bash) return
|
||||
await withShell("/usr/bin/bash", async () => {
|
||||
expect(Shell.acceptable()).toBe(bash)
|
||||
expect(Shell.preferred()).toBe(bash)
|
||||
expect(ShellSelect.acceptable()).toBe(bash)
|
||||
expect(ShellSelect.preferred()).toBe(bash)
|
||||
})
|
||||
})
|
||||
|
||||
test("resolves bare bash to Git Bash before PATH", async () => {
|
||||
const bash = Shell.gitbash()
|
||||
const bash = ShellSelect.gitbash()
|
||||
if (!bash) return
|
||||
expect(Shell.acceptable("bash")).toBe(bash)
|
||||
expect(Shell.preferred("bash")).toBe(bash)
|
||||
expect(ShellSelect.acceptable("bash")).toBe(bash)
|
||||
expect(ShellSelect.preferred("bash")).toBe(bash)
|
||||
await withShell("bash", async () => {
|
||||
expect(Shell.acceptable()).toBe(bash)
|
||||
expect(Shell.preferred()).toBe(bash)
|
||||
expect(ShellSelect.acceptable()).toBe(bash)
|
||||
expect(ShellSelect.preferred()).toBe(bash)
|
||||
})
|
||||
})
|
||||
|
||||
|
|
@ -101,7 +101,7 @@ describe("shell", () => {
|
|||
const shell = which("pwsh") || which("powershell")
|
||||
if (!shell) return
|
||||
await withShell(path.win32.basename(shell), async () => {
|
||||
expect(Shell.preferred()).toBe(shell)
|
||||
expect(ShellSelect.preferred()).toBe(shell)
|
||||
})
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,432 +0,0 @@
|
|||
import fs from "fs/promises"
|
||||
import { realpathSync } from "node:fs"
|
||||
import path from "path"
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { Effect, Layer } from "effect"
|
||||
import { ChildProcess } from "effect/unstable/process"
|
||||
import { FSUtil } from "@opencode-ai/core/fs-util"
|
||||
import { Config } from "@opencode-ai/core/config"
|
||||
import { Location } from "@opencode-ai/core/location"
|
||||
import { LocationMutation } from "@opencode-ai/core/location-mutation"
|
||||
import { PermissionV2 } from "@opencode-ai/core/permission"
|
||||
import { AppProcess } from "@opencode-ai/core/process"
|
||||
import { AbsolutePath } from "@opencode-ai/core/schema"
|
||||
import { SessionV2 } from "@opencode-ai/core/session"
|
||||
import { BashTool } from "@opencode-ai/core/tool/bash"
|
||||
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
|
||||
import { location } from "./fixture/location"
|
||||
import { tmpdir } from "./fixture/tmpdir"
|
||||
import { testEffect } from "./lib/effect"
|
||||
import { toolIdentity, executeTool, settleTool, toolDefinitions } from "./lib/tool"
|
||||
|
||||
const sessionID = SessionV2.ID.make("ses_bash_tool_test")
|
||||
const assertions: PermissionV2.AssertInput[] = []
|
||||
const runs: Array<{
|
||||
readonly command: string
|
||||
readonly cwd?: string
|
||||
readonly shell?: string | boolean
|
||||
readonly options?: AppProcess.RunOptions
|
||||
}> = []
|
||||
let denyAction: string | undefined
|
||||
let result: AppProcess.RunResult = {
|
||||
command: "mock",
|
||||
exitCode: 0,
|
||||
output: Buffer.from("hello\n"),
|
||||
stdout: Buffer.from("hello\n"),
|
||||
stderr: Buffer.alloc(0),
|
||||
outputTruncated: false,
|
||||
stdoutTruncated: false,
|
||||
stderrTruncated: false,
|
||||
}
|
||||
let runFailure: AppProcess.AppProcessError | undefined
|
||||
let afterPermission = (_input: PermissionV2.AssertInput): Effect.Effect<void> => Effect.void
|
||||
|
||||
const permission = Layer.succeed(
|
||||
PermissionV2.Service,
|
||||
PermissionV2.Service.of({
|
||||
assert: (input) =>
|
||||
Effect.sync(() => assertions.push(input)).pipe(
|
||||
Effect.andThen(Effect.suspend(() => afterPermission(input))),
|
||||
Effect.andThen(
|
||||
input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
|
||||
),
|
||||
),
|
||||
ask: () => Effect.die("unused"),
|
||||
reply: () => Effect.die("unused"),
|
||||
get: () => Effect.die("unused"),
|
||||
forSession: () => Effect.die("unused"),
|
||||
list: () => Effect.die("unused"),
|
||||
}),
|
||||
)
|
||||
const appProcess = Layer.succeed(
|
||||
AppProcess.Service,
|
||||
AppProcess.Service.of({
|
||||
run: (command: ChildProcess.Command, options?: AppProcess.RunOptions) =>
|
||||
Effect.suspend(() => {
|
||||
if (command._tag !== "StandardCommand") throw new Error("expected standard command")
|
||||
runs.push({ command: command.command, cwd: command.options.cwd, shell: command.options.shell, options })
|
||||
return runFailure ? Effect.fail(runFailure) : Effect.succeed(result)
|
||||
}),
|
||||
} as unknown as AppProcess.Interface),
|
||||
)
|
||||
const config = Layer.succeed(
|
||||
Config.Service,
|
||||
Config.Service.of({
|
||||
entries: () => Effect.succeed([]),
|
||||
}),
|
||||
)
|
||||
|
||||
const reset = () => {
|
||||
assertions.length = 0
|
||||
runs.length = 0
|
||||
denyAction = undefined
|
||||
runFailure = undefined
|
||||
afterPermission = () => Effect.void
|
||||
result = {
|
||||
command: "mock",
|
||||
exitCode: 0,
|
||||
output: Buffer.from("hello\n"),
|
||||
stdout: Buffer.from("hello\n"),
|
||||
stderr: Buffer.alloc(0),
|
||||
outputTruncated: false,
|
||||
stdoutTruncated: false,
|
||||
stderrTruncated: false,
|
||||
}
|
||||
}
|
||||
|
||||
const withTool = <A, E, R>(
|
||||
directory: string,
|
||||
body: (registry: ToolRegistry.Interface) => Effect.Effect<A, E, R>,
|
||||
processLayer: Layer.Layer<AppProcess.Service> = appProcess,
|
||||
) => {
|
||||
const filesystem = FSUtil.defaultLayer
|
||||
const activeLocation = Layer.succeed(
|
||||
Location.Service,
|
||||
Location.Service.of(location({ directory: AbsolutePath.make(directory) })),
|
||||
)
|
||||
const mutation = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
|
||||
const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
|
||||
const bash = BashTool.layer.pipe(
|
||||
Layer.provide(registry),
|
||||
Layer.provide(permission),
|
||||
Layer.provide(mutation),
|
||||
Layer.provide(filesystem),
|
||||
Layer.provide(processLayer),
|
||||
Layer.provide(config),
|
||||
)
|
||||
return Effect.gen(function* () {
|
||||
return yield* body(yield* ToolRegistry.Service)
|
||||
}).pipe(Effect.provide(Layer.mergeAll(registry, bash)))
|
||||
}
|
||||
|
||||
const call = (input: typeof BashTool.Input.Type, id = "call-bash") => ({
|
||||
sessionID,
|
||||
...toolIdentity,
|
||||
call: { type: "tool-call" as const, id, name: "bash", input },
|
||||
})
|
||||
|
||||
const it = testEffect(Layer.empty)
|
||||
|
||||
describe("BashTool", () => {
|
||||
it.live("registers and returns structured successful output from the active Location", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
return withTool(tmp.path, (registry) =>
|
||||
Effect.gen(function* () {
|
||||
const definitions = yield* toolDefinitions(registry)
|
||||
expect(definitions.map((tool) => tool.name)).toEqual(["bash"])
|
||||
expect(definitions[0]?.inputSchema).not.toHaveProperty("properties.background")
|
||||
expect(definitions[0]?.inputSchema).not.toHaveProperty("properties.description")
|
||||
expect(definitions[0]?.outputSchema).not.toHaveProperty("properties.output")
|
||||
expect(definitions[0]?.outputSchema).not.toHaveProperty("properties.command")
|
||||
expect(definitions[0]?.outputSchema).not.toHaveProperty("properties.cwd")
|
||||
expect(yield* toolDefinitions(registry, [{ action: "bash", resource: "*", effect: "deny" }])).toEqual([])
|
||||
expect(yield* settleTool(registry, call({ command: "pwd" }))).toEqual({
|
||||
result: {
|
||||
type: "content",
|
||||
value: [
|
||||
{ type: "text", text: "hello\n" },
|
||||
{ type: "text", text: "Command exited with code 0." },
|
||||
],
|
||||
},
|
||||
output: {
|
||||
structured: {
|
||||
exit: 0,
|
||||
truncated: false,
|
||||
},
|
||||
content: [
|
||||
{ type: "text", text: "hello\n" },
|
||||
{ type: "text", text: "Command exited with code 0." },
|
||||
],
|
||||
},
|
||||
})
|
||||
expect(runs).toMatchObject([{ command: "pwd", cwd: realpathSync(tmp.path) }])
|
||||
expect(runs[0]?.options).toMatchObject({
|
||||
combineOutput: true,
|
||||
maxOutputBytes: BashTool.MAX_CAPTURE_BYTES,
|
||||
})
|
||||
expect(assertions).toMatchObject([{ sessionID, action: "bash", resources: ["pwd"], save: ["pwd"] }])
|
||||
}),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("resolves a relative workdir from the active Location", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
return Effect.promise(() => fs.mkdir(path.join(tmp.path, "src"))).pipe(
|
||||
Effect.andThen(
|
||||
withTool(tmp.path, (registry) => executeTool(registry, call({ command: "pwd", workdir: "src" }))),
|
||||
),
|
||||
Effect.andThen(
|
||||
Effect.sync(() => expect(runs).toMatchObject([{ cwd: realpathSync(path.join(tmp.path, "src")) }])),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("rejects a workdir that stops being a directory during approval", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
const workdir = path.join(tmp.path, "src")
|
||||
afterPermission = (input) =>
|
||||
input.action === "bash"
|
||||
? Effect.promise(async () => {
|
||||
await fs.rm(workdir, { recursive: true })
|
||||
await fs.writeFile(workdir, "not a directory")
|
||||
}).pipe(Effect.orDie)
|
||||
: Effect.void
|
||||
return Effect.promise(() => fs.mkdir(workdir)).pipe(
|
||||
Effect.andThen(
|
||||
withTool(tmp.path, (registry) => executeTool(registry, call({ command: "pwd", workdir: "src" }))),
|
||||
),
|
||||
Effect.andThen(
|
||||
Effect.sync(() => {
|
||||
expect(runs).toEqual([])
|
||||
expect(assertions.map((input) => input.action)).toEqual(["bash"])
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
|
||||
if (process.platform !== "win32") {
|
||||
it.live("executes a real shell command through AppProcess", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
return withTool(
|
||||
tmp.path,
|
||||
(registry) => settleTool(registry, call({ command: "printf core-bash" })),
|
||||
AppProcess.defaultLayer,
|
||||
).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.result).toEqual({
|
||||
type: "content",
|
||||
value: [
|
||||
{ type: "text", text: "core-bash" },
|
||||
{ type: "text", text: "Command exited with code 0." },
|
||||
],
|
||||
})
|
||||
expect(settled.output?.structured).toMatchObject({
|
||||
exit: 0,
|
||||
})
|
||||
expect(settled.output?.structured).not.toHaveProperty("output")
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
it.live("approves an explicit external workdir before bash execution", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([active, outside]) => {
|
||||
reset()
|
||||
return withTool(active.path, (registry) =>
|
||||
executeTool(registry, call({ command: "pwd", workdir: outside.path })),
|
||||
).pipe(
|
||||
Effect.andThen(
|
||||
Effect.sync(() => {
|
||||
expect(assertions.map((item) => item.action)).toEqual(["external_directory", "bash"])
|
||||
expect(assertions[0]).toMatchObject({
|
||||
resources: [path.join(realpathSync(outside.path), "*").replaceAll("\\", "/")],
|
||||
})
|
||||
expect(runs).toHaveLength(1)
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("does not execute after external-directory or bash denial", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([active, outside]) =>
|
||||
Effect.gen(function* () {
|
||||
reset()
|
||||
denyAction = "external_directory"
|
||||
yield* withTool(active.path, (registry) =>
|
||||
executeTool(registry, call({ command: "pwd", workdir: outside.path })),
|
||||
)
|
||||
expect(assertions.map((item) => item.action)).toEqual(["external_directory"])
|
||||
expect(runs).toEqual([])
|
||||
|
||||
reset()
|
||||
denyAction = "bash"
|
||||
yield* withTool(active.path, (registry) => executeTool(registry, call({ command: "pwd" })))
|
||||
expect(assertions.map((item) => item.action)).toEqual(["bash"])
|
||||
expect(runs).toEqual([])
|
||||
}),
|
||||
([active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("reports external command arguments as advisory warnings without enforcing approval", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([active, outside]) => {
|
||||
reset()
|
||||
denyAction = "external_directory"
|
||||
const target = path.join(outside.path, "secret.txt")
|
||||
return withTool(active.path, (registry) => settleTool(registry, call({ command: `cat ${target}` }))).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(assertions.map((item) => item.action)).toEqual(["bash"])
|
||||
expect(runs).toHaveLength(1)
|
||||
expect(settled.output?.structured).toMatchObject({
|
||||
truncated: false,
|
||||
})
|
||||
expect(settled.output?.structured).not.toHaveProperty("warnings")
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Warnings:"),
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("keeps non-zero exits useful", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
result = { ...result, exitCode: 7, output: Buffer.from("HEAD full output TAIL") }
|
||||
return withTool(tmp.path, (registry) => settleTool(registry, call({ command: "false" }, "call-overflow"))).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Command exited with code 7"),
|
||||
})
|
||||
expect(settled.output?.structured).toMatchObject({
|
||||
exit: 7,
|
||||
truncated: false,
|
||||
})
|
||||
expect(settled.output?.content[0]).toEqual({ type: "text", text: "HEAD full output TAIL" })
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("surfaces bounded process-capture truncation", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
result = { ...result, outputTruncated: true }
|
||||
return withTool(tmp.path, (registry) => settleTool(registry, call({ command: "verbose" }))).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.structured).toMatchObject({ truncated: true })
|
||||
expect(settled.output?.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("output capture truncated"),
|
||||
})
|
||||
expect(settled.output?.structured).not.toHaveProperty("resource")
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("returns a useful timeout settlement", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => tmpdir()),
|
||||
(tmp) => {
|
||||
reset()
|
||||
runFailure = new AppProcess.AppProcessError({ command: "sleep", cause: new Error("Timed out") })
|
||||
return withTool(tmp.path, (registry) => settleTool(registry, call({ command: "sleep 60", timeout: 10 }))).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Command timed out"),
|
||||
})
|
||||
expect(settled.output?.structured).toMatchObject({
|
||||
timeout: true,
|
||||
truncated: false,
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
|
||||
),
|
||||
)
|
||||
})
|
||||
|
||||
test("keeps locked deferred parity TODOs visible", async () => {
|
||||
const source = await fs.readFile(new URL("../src/tool/bash.ts", import.meta.url), "utf8")
|
||||
for (const todo of [
|
||||
"Port tree-sitter bash / PowerShell parser-based approval reduction.",
|
||||
"Port BashArity reusable command-prefix approvals.",
|
||||
"Replace token-based command-argument external-directory advisories with parser-based detection.",
|
||||
"Restore PowerShell and cmd-specific invocation/path handling on Windows.",
|
||||
"Add plugin shell.env environment augmentation once V2 plugin hooks exist.",
|
||||
"Add durable/live progress metadata streaming for long-running commands once V2 tool invocation progress context is wired.",
|
||||
"Persist background job status and define restart recovery before exposing remote observation.",
|
||||
"Revisit process-group cleanup and platform coverage with shell-specific tests if current AppProcess semantics do not fully cover it.",
|
||||
"Revisit binary output handling if stdout/stderr decoding is text-only.",
|
||||
"Stream full shell output into managed storage while retaining only a bounded in-memory preview.",
|
||||
]) {
|
||||
expect(source).toContain(`TODO: ${todo}`)
|
||||
}
|
||||
})
|
||||
366
packages/core/test/tool-shell.test.ts
Normal file
366
packages/core/test/tool-shell.test.ts
Normal file
|
|
@ -0,0 +1,366 @@
|
|||
import fs from "fs/promises"
|
||||
import { realpathSync } from "node:fs"
|
||||
import path from "path"
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { Effect, Layer } from "effect"
|
||||
import { Config } from "@opencode-ai/core/config"
|
||||
import { EventV2 } from "@opencode-ai/core/event"
|
||||
import { FSUtil } from "@opencode-ai/core/fs-util"
|
||||
import { Global } from "@opencode-ai/core/global"
|
||||
import { Location } from "@opencode-ai/core/location"
|
||||
import { LocationMutation } from "@opencode-ai/core/location-mutation"
|
||||
import { PermissionV2 } from "@opencode-ai/core/permission"
|
||||
import { AppProcess } from "@opencode-ai/core/process"
|
||||
import { Project } from "@opencode-ai/core/project"
|
||||
import { AbsolutePath } from "@opencode-ai/core/schema"
|
||||
import { SessionV2 } from "@opencode-ai/core/session"
|
||||
import { Shell } from "@opencode-ai/core/shell"
|
||||
import { ShellTool } from "@opencode-ai/core/tool/shell"
|
||||
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
|
||||
import { tmpdir } from "./fixture/tmpdir"
|
||||
import { testEffect } from "./lib/effect"
|
||||
import { toolIdentity, executeTool, settleTool, toolDefinitions } from "./lib/tool"
|
||||
|
||||
const sessionID = SessionV2.ID.make("ses_shell_tool_test")
|
||||
const assertions: PermissionV2.AssertInput[] = []
|
||||
let denyAction: string | undefined
|
||||
let afterPermission = (_input: PermissionV2.AssertInput): Effect.Effect<void> => Effect.void
|
||||
|
||||
const permission = Layer.succeed(
|
||||
PermissionV2.Service,
|
||||
PermissionV2.Service.of({
|
||||
assert: (input) =>
|
||||
Effect.sync(() => assertions.push(input)).pipe(
|
||||
Effect.andThen(Effect.suspend(() => afterPermission(input))),
|
||||
Effect.andThen(
|
||||
input.action === denyAction ? Effect.fail(new PermissionV2.DeniedError({ rules: [] })) : Effect.void,
|
||||
),
|
||||
),
|
||||
ask: () => Effect.die("unused"),
|
||||
reply: () => Effect.die("unused"),
|
||||
get: () => Effect.die("unused"),
|
||||
forSession: () => Effect.die("unused"),
|
||||
list: () => Effect.die("unused"),
|
||||
}),
|
||||
)
|
||||
|
||||
const reset = () => {
|
||||
assertions.length = 0
|
||||
denyAction = undefined
|
||||
afterPermission = () => Effect.void
|
||||
}
|
||||
|
||||
const withTool = <A, E, R>(
|
||||
data: string,
|
||||
directory: string,
|
||||
body: (registry: ToolRegistry.Interface) => Effect.Effect<A, E, R>,
|
||||
) => {
|
||||
const filesystem = FSUtil.defaultLayer
|
||||
const location = Location.layer(Location.Ref.make({ directory: AbsolutePath.make(directory) })).pipe(
|
||||
Layer.provide(Project.defaultLayer),
|
||||
)
|
||||
const global = Global.layerWith({ data, config: path.join(data, "config") })
|
||||
const mutation = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(location))
|
||||
const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
|
||||
const shellService = Shell.layer.pipe(
|
||||
Layer.provide(EventV2.defaultLayer),
|
||||
Layer.provide(location),
|
||||
Layer.provide(Config.locationLayer.pipe(Layer.provide(location), Layer.provide(filesystem), Layer.provide(global))),
|
||||
Layer.provide(global),
|
||||
Layer.provide(filesystem),
|
||||
Layer.provide(AppProcess.defaultLayer),
|
||||
)
|
||||
const shell = ShellTool.layer.pipe(
|
||||
Layer.provide(registry),
|
||||
Layer.provide(permission),
|
||||
Layer.provide(mutation),
|
||||
Layer.provide(filesystem),
|
||||
Layer.provide(shellService),
|
||||
)
|
||||
return Effect.gen(function* () {
|
||||
return yield* body(yield* ToolRegistry.Service)
|
||||
}).pipe(Effect.provide(Layer.mergeAll(registry, shell, filesystem)))
|
||||
}
|
||||
|
||||
const call = (input: typeof ShellTool.Input.Type, id = "call-shell") => ({
|
||||
sessionID,
|
||||
...toolIdentity,
|
||||
call: { type: "tool-call" as const, id, name: "shell", input },
|
||||
})
|
||||
|
||||
const it = testEffect(Layer.empty)
|
||||
|
||||
describe("ShellTool", () => {
|
||||
it.live("registers and returns real successful output from the active Location", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
return withTool(data.path, tmp.path, (registry) =>
|
||||
Effect.gen(function* () {
|
||||
const definitions = yield* toolDefinitions(registry)
|
||||
expect(definitions.map((tool) => tool.name)).toEqual(["shell"])
|
||||
expect(definitions[0]?.outputSchema).not.toHaveProperty("properties.output")
|
||||
expect(yield* toolDefinitions(registry, [{ action: "shell", resource: "*", effect: "deny" }])).toEqual([])
|
||||
|
||||
const settled = yield* settleTool(registry, call({ command: "printf hello" }))
|
||||
expect(settled.output?.structured).toMatchObject({ exit: 0, truncated: false })
|
||||
expect(settled.output?.content[0]).toEqual({ type: "text", text: "hello" })
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Command exited with code 0."),
|
||||
})
|
||||
expect(assertions).toMatchObject([{ sessionID, action: "shell", resources: ["printf hello"] }])
|
||||
}),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("resolves a relative workdir from the active Location", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
return Effect.promise(() => fs.mkdir(path.join(tmp.path, "src"))).pipe(
|
||||
Effect.andThen(
|
||||
withTool(data.path, tmp.path, (registry) => settleTool(registry, call({ command: "pwd", workdir: "src" }))),
|
||||
),
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() =>
|
||||
expect(settled.output?.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining(realpathSync(path.join(tmp.path, "src"))),
|
||||
}),
|
||||
),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("rejects a workdir that stops being a directory during approval", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
const workdir = path.join(tmp.path, "src")
|
||||
afterPermission = (input) =>
|
||||
input.action === "shell"
|
||||
? Effect.promise(async () => {
|
||||
await fs.rm(workdir, { recursive: true })
|
||||
await fs.writeFile(workdir, "not a directory")
|
||||
}).pipe(Effect.orDie)
|
||||
: Effect.void
|
||||
return Effect.promise(() => fs.mkdir(workdir)).pipe(
|
||||
Effect.andThen(
|
||||
withTool(data.path, tmp.path, (registry) =>
|
||||
executeTool(registry, call({ command: "pwd", workdir: "src" })),
|
||||
),
|
||||
),
|
||||
Effect.andThen(Effect.sync(() => expect(assertions.map((input) => input.action)).toEqual(["shell"]))),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("approves an explicit external workdir before shell execution", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir(), tmpdir()])),
|
||||
([data, active, outside]) => {
|
||||
reset()
|
||||
return withTool(data.path, active.path, (registry) =>
|
||||
executeTool(registry, call({ command: "pwd", workdir: outside.path })),
|
||||
).pipe(
|
||||
Effect.andThen(
|
||||
Effect.sync(() => {
|
||||
expect(assertions.map((item) => item.action)).toEqual(["external_directory", "shell"])
|
||||
expect(assertions[0]).toMatchObject({
|
||||
resources: [path.join(realpathSync(outside.path), "*").replaceAll("\\", "/")],
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([
|
||||
data[Symbol.asyncDispose](),
|
||||
active[Symbol.asyncDispose](),
|
||||
outside[Symbol.asyncDispose](),
|
||||
]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("does not execute after external-directory or shell denial", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir(), tmpdir()])),
|
||||
([data, active, outside]) =>
|
||||
Effect.gen(function* () {
|
||||
reset()
|
||||
denyAction = "external_directory"
|
||||
yield* withTool(data.path, active.path, (registry) =>
|
||||
executeTool(registry, call({ command: "pwd", workdir: outside.path })),
|
||||
)
|
||||
expect(assertions.map((item) => item.action)).toEqual(["external_directory"])
|
||||
|
||||
reset()
|
||||
denyAction = "shell"
|
||||
yield* withTool(data.path, active.path, (registry) => executeTool(registry, call({ command: "pwd" })))
|
||||
expect(assertions.map((item) => item.action)).toEqual(["shell"])
|
||||
}),
|
||||
([data, active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([
|
||||
data[Symbol.asyncDispose](),
|
||||
active[Symbol.asyncDispose](),
|
||||
outside[Symbol.asyncDispose](),
|
||||
]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("reports external command arguments as advisory warnings without enforcing approval", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir(), tmpdir()])),
|
||||
([data, active, outside]) => {
|
||||
reset()
|
||||
denyAction = "external_directory"
|
||||
const target = path.join(outside.path, "secret.txt")
|
||||
return withTool(data.path, active.path, (registry) =>
|
||||
settleTool(registry, call({ command: `cat ${target}` })),
|
||||
).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(assertions.map((item) => item.action)).toEqual(["shell"])
|
||||
expect(settled.output?.structured).not.toHaveProperty("warnings")
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Warnings:"),
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, active, outside]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([
|
||||
data[Symbol.asyncDispose](),
|
||||
active[Symbol.asyncDispose](),
|
||||
outside[Symbol.asyncDispose](),
|
||||
]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("keeps non-zero exits useful", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
return withTool(data.path, tmp.path, (registry) =>
|
||||
settleTool(registry, call({ command: "printf body && exit 7" }, "call-nonzero")),
|
||||
).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.structured).toMatchObject({ exit: 7, truncated: false })
|
||||
expect(settled.output?.content[0]).toEqual({ type: "text", text: "body" })
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Command exited with code 7"),
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("truncates the model view and points at the saved output file when output overflows", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
const bytes = ShellTool.MAX_CAPTURE_BYTES + 1024
|
||||
return withTool(data.path, tmp.path, (registry) =>
|
||||
settleTool(registry, call({ command: `head -c ${bytes} /dev/zero | tr '\\0' 'x'` }, "call-overflow")),
|
||||
).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.structured).toMatchObject({ exit: 0, truncated: true })
|
||||
expect(settled.output?.content[0]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("output truncated; full output saved to:"),
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
|
||||
it.live("returns a useful timeout settlement", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
|
||||
([data, tmp]) => {
|
||||
reset()
|
||||
return withTool(data.path, tmp.path, (registry) =>
|
||||
settleTool(registry, call({ command: "sleep 60", timeout: 50 })),
|
||||
).pipe(
|
||||
Effect.andThen((settled) =>
|
||||
Effect.sync(() => {
|
||||
expect(settled.output?.structured).toMatchObject({ timeout: true, truncated: false })
|
||||
expect(settled.output?.content[1]).toMatchObject({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Command timed out"),
|
||||
})
|
||||
}),
|
||||
),
|
||||
)
|
||||
},
|
||||
([data, tmp]) =>
|
||||
Effect.promise(() =>
|
||||
Promise.all([data[Symbol.asyncDispose](), tmp[Symbol.asyncDispose]()]).then(() => undefined),
|
||||
),
|
||||
),
|
||||
)
|
||||
})
|
||||
|
||||
test("keeps locked deferred parity TODOs visible", async () => {
|
||||
const source = await fs.readFile(new URL("../src/tool/shell.ts", import.meta.url), "utf8")
|
||||
for (const todo of [
|
||||
"Port tree-sitter bash / PowerShell parser-based approval reduction.",
|
||||
"Port BashArity reusable command-prefix approvals.",
|
||||
"Replace token-based command-argument external-directory advisories with parser-based detection.",
|
||||
"Restore PowerShell and cmd-specific invocation/path handling on Windows.",
|
||||
"Add plugin shell.env environment augmentation once V2 plugin hooks exist.",
|
||||
"Add durable/live progress metadata streaming for long-running commands once V2 tool invocation progress context is wired.",
|
||||
"Persist background job status and define restart recovery before exposing remote observation.",
|
||||
"Revisit process-group cleanup and platform coverage with shell-specific tests if current AppProcess semantics do not fully cover it.",
|
||||
"Revisit binary output handling if stdout/stderr decoding is text-only.",
|
||||
"Stream full shell output into managed storage while retaining only a bounded in-memory preview.",
|
||||
]) {
|
||||
expect(source).toContain(`TODO: ${todo}`)
|
||||
}
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue