Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/src/tool/builtins.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export * as BuiltInTools from "./builtins"

import { Layer } from "effect"
import { BashTool } from "./bash"
import { TerminalTool } from "./terminal"
import { ApplyPatchTool } from "./apply-patch"
import { EditTool } from "./edit"
import { GlobTool } from "./glob"
Expand Down Expand Up @@ -30,7 +30,7 @@ import { WriteTool } from "./write"
*/
export const locationLayer = Layer.mergeAll(
ApplyPatchTool.layer,
BashTool.layer,
TerminalTool.layer,
EditTool.layer,
GlobTool.layer,
GrepTool.layer,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export * as BashTool from "./bash"
export * as TerminalTool from "./terminal"

import path from "path"
import { ToolFailure } from "@opencode-ai/llm"
Expand All @@ -13,7 +13,7 @@ import { PositiveInt } from "../schema"
import { Tool } from "./tool"
import { Tools } from "./tools"

export const name = "bash"
export const name = "terminal"
export const DEFAULT_TIMEOUT_MS = 2 * 60 * 1_000
export const MAX_TIMEOUT_MS = 10 * 60 * 1_000
export const MAX_CAPTURE_BYTES = 1024 * 1024
Expand Down
19 changes: 16 additions & 3 deletions packages/core/src/v1/config/permission.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ const InputObject = Schema.StructWithRest(
glob: Schema.optional(Rule),
grep: Schema.optional(Rule),
list: Schema.optional(Rule),
bash: Schema.optional(Rule),
bash: Schema.optional(Rule), // kept for backward compatibility but mapped to "terminal"
terminal: Schema.optional(Rule),
task: Schema.optional(Rule),
external_directory: Schema.optional(Rule),
todowrite: Schema.optional(Action),
Expand All @@ -37,8 +38,20 @@ const InputObject = Schema.StructWithRest(

const InputSchema = Schema.Union([Action, InputObject])

const normalizeInput = (input: Schema.Schema.Type<typeof InputSchema>): Schema.Schema.Type<typeof InputObject> =>
typeof input === "string" ? { "*": input } : input
const normalizeInput = (input: Schema.Schema.Type<typeof InputSchema>): Schema.Schema.Type<typeof InputObject> => {
if (typeof input === "string") return { "*": input }
const result: any = {}
for (const [key, value] of globalThis.Object.entries(input)) {
if (key === "bash") {
if (!("terminal" in input)) {
result.terminal = value
}
} else {
result[key] = value
}
}
return result
}

export const Info = InputSchema.pipe(
Schema.decodeTo(InputObject, {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/config/agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ describe("ConfigAgentPlugin.Plugin", () => {
{ action: "read", resource: "*", effect: "allow" },
{ action: "bash", resource: "git *", effect: "allow" },
])
expect(PermissionV2.evaluate("bash", "git status", buildAgent.permissions).effect).toBe("allow")
expect(PermissionV2.evaluate("bash", "bun test", buildAgent.permissions).effect).toBe("ask")
expect(PermissionV2.evaluate("terminal", "git status", buildAgent.permissions).effect).toBe("allow")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: The renamed assertion calls evaluate("terminal", ...) but the permission rules under test still carry action "bash", which PermissionV2.evaluate matches by exact wildcard with no bash→terminal remap. "git status" therefore never matches the "bash" allow rule and the first assertion expecting "allow" will fail with "ask". Update the permission rules (and the expected buildAgent.permissions/toMatchObject lists) to use action "terminal", or keep evaluating with "bash" — the test is currently internally inconsistent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/test/config/agent.test.ts, line 84:

<comment>The renamed assertion calls evaluate("terminal", ...) but the permission rules under test still carry action "bash", which PermissionV2.evaluate matches by exact wildcard with no bash→terminal remap. "git status" therefore never matches the "bash" allow rule and the first assertion expecting "allow" will fail with "ask". Update the permission rules (and the expected buildAgent.permissions/toMatchObject lists) to use action "terminal", or keep evaluating with "bash" — the test is currently internally inconsistent.</comment>

<file context>
@@ -81,8 +81,8 @@ describe("ConfigAgentPlugin.Plugin", () => {
       ])
-      expect(PermissionV2.evaluate("bash", "git status", buildAgent.permissions).effect).toBe("allow")
-      expect(PermissionV2.evaluate("bash", "bun test", buildAgent.permissions).effect).toBe("ask")
+      expect(PermissionV2.evaluate("terminal", "git status", buildAgent.permissions).effect).toBe("allow")
+      expect(PermissionV2.evaluate("terminal", "bun test", buildAgent.permissions).effect).toBe("ask")
 
</file context>

expect(PermissionV2.evaluate("terminal", "bun test", buildAgent.permissions).effect).toBe("ask")

const reviewer = yield* agents.get(AgentV2.ID.make("reviewer"))
if (!reviewer) throw new Error("expected configured reviewer agent")
Expand Down
7 changes: 3 additions & 4 deletions packages/core/test/session-runner-tool-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,16 +58,15 @@ describe("ToolRegistry", () => {
const service = yield* ToolRegistry.Service
yield* service.register({
question: make(),
bash: make(),
terminal: make(),
edit: make("edit"),
write: make("edit"),
apply_patch: make("edit"),
})
const names = (rules: Parameters<ToolRegistry.Interface["materialize"]>[0]) =>
toolDefinitions(service, rules).pipe(Effect.map((definitions) => definitions.map((tool) => tool.name)))

expect(yield* names([{ action: "question", resource: "*", effect: "deny" }])).toEqual([
"bash",
expect(yield* names([{ action: "question", resource: "*", effect: "deny" }])).toEqual(["terminal",
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
"edit",
"write",
"apply_patch",
Expand All @@ -84,7 +83,7 @@ describe("ToolRegistry", () => {
{ action: "*", resource: "*", effect: "deny" },
]),
).toEqual([])
expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual(["question", "bash"])
expect(yield* names([{ action: "edit", resource: "*", effect: "deny" }])).toEqual(["question", "terminal"])
}),
)

Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/session-tool-progress.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,14 @@ describe("Tool.Progress", () => {
timestamp,
assistantMessageID,
callID,
name: "bash",
name: "terminal",
})
yield* service.publish(SessionEvent.Tool.Called, {
sessionID,
timestamp,
assistantMessageID,
callID,
tool: "bash",
tool: "terminal",
input: { command: "pwd" },
provider: { executed: false },
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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 { TerminalTool } from "@opencode-ai/core/tool/terminal"
import { ToolRegistry } from "@opencode-ai/core/tool/registry"
import { location } from "./fixture/location"
import { tmpdir } from "./fixture/tmpdir"
Expand Down Expand Up @@ -102,7 +102,7 @@ const withTool = <A, E, R>(
)
const mutation = LocationMutation.layer.pipe(Layer.provide(filesystem), Layer.provide(activeLocation))
const registry = ToolRegistry.defaultLayer.pipe(Layer.provide(permission))
const bash = BashTool.layer.pipe(
const bash = TerminalTool.layer.pipe(
Layer.provide(registry),
Layer.provide(permission),
Layer.provide(mutation),
Expand All @@ -115,15 +115,15 @@ const withTool = <A, E, R>(
}).pipe(Effect.provide(Layer.mergeAll(registry, bash)))
}

const call = (input: typeof BashTool.Input.Type, id = "call-bash") => ({
const call = (input: typeof TerminalTool.Input.Type, id = "call-bash") => ({
sessionID,
...toolIdentity,
call: { type: "tool-call" as const, id, name: "bash", input },
call: { type: "tool-call" as const, id, name: "terminal", input },
})

const it = testEffect(Layer.empty)

describe("BashTool", () => {
describe("TerminalTool", () => {
it.live("registers and returns structured successful output from the active Location", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
Expand All @@ -132,9 +132,9 @@ describe("BashTool", () => {
return withTool(tmp.path, (registry) =>
Effect.gen(function* () {
const definitions = yield* toolDefinitions(registry)
expect(definitions.map((tool) => tool.name)).toEqual(["bash"])
expect(definitions.map((tool) => tool.name)).toEqual(["terminal"])
expect(definitions[0]?.inputSchema).not.toHaveProperty("properties.background")
expect(yield* toolDefinitions(registry, [{ action: "bash", resource: "*", effect: "deny" }])).toEqual([])
expect(yield* toolDefinitions(registry, [{ action: "terminal", resource: "*", effect: "deny" }])).toEqual([])
expect(
yield* settleTool(registry, call({ command: "pwd", description: "Print working directory" })),
).toEqual({
Expand All @@ -152,10 +152,10 @@ describe("BashTool", () => {
})
expect(runs).toMatchObject([{ command: "pwd", cwd: realpathSync(tmp.path) }])
expect(runs[0]?.options).toMatchObject({
maxOutputBytes: BashTool.MAX_CAPTURE_BYTES,
maxErrorBytes: BashTool.MAX_CAPTURE_BYTES,
maxOutputBytes: TerminalTool.MAX_CAPTURE_BYTES,
maxErrorBytes: TerminalTool.MAX_CAPTURE_BYTES,
})
expect(assertions).toMatchObject([{ sessionID, action: "bash", resources: ["pwd"], save: ["pwd"] }])
expect(assertions).toMatchObject([{ sessionID, action: "terminal", resources: ["pwd"], save: ["pwd"] }])
}),
)
},
Expand Down Expand Up @@ -188,7 +188,7 @@ describe("BashTool", () => {
reset()
const workdir = path.join(tmp.path, "src")
afterPermission = (input) =>
input.action === "bash"
input.action === "terminal"
? Effect.promise(async () => {
await fs.rm(workdir, { recursive: true })
await fs.writeFile(workdir, "not a directory")
Expand All @@ -201,7 +201,7 @@ describe("BashTool", () => {
Effect.andThen(
Effect.sync(() => {
expect(runs).toEqual([])
expect(assertions.map((input) => input.action)).toEqual(["bash"])
expect(assertions.map((input) => input.action)).toEqual(["terminal"])
}),
),
)
Expand Down Expand Up @@ -249,7 +249,7 @@ describe("BashTool", () => {
).pipe(
Effect.andThen(
Effect.sync(() => {
expect(assertions.map((item) => item.action)).toEqual(["external_directory", "bash"])
expect(assertions.map((item) => item.action)).toEqual(["external_directory", "terminal"])
expect(assertions[0]).toMatchObject({
resources: [path.join(realpathSync(outside.path), "*").replaceAll("\\", "/")],
})
Expand All @@ -265,7 +265,7 @@ describe("BashTool", () => {
),
)

it.live("does not execute after external-directory or bash denial", () =>
it.live("does not execute after external-directory or terminal denial", () =>
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) =>
Expand All @@ -279,9 +279,9 @@ describe("BashTool", () => {
expect(runs).toEqual([])

reset()
denyAction = "bash"
denyAction = "terminal"
yield* withTool(active.path, (registry) => executeTool(registry, call({ command: "pwd" })))
expect(assertions.map((item) => item.action)).toEqual(["bash"])
expect(assertions.map((item) => item.action)).toEqual(["terminal"])
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
expect(runs).toEqual([])
}),
([active, outside]) =>
Expand All @@ -301,7 +301,7 @@ describe("BashTool", () => {
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(assertions.map((item) => item.action)).toEqual(["terminal"])
expect(runs).toHaveLength(1)
expect(settled.output?.structured).toMatchObject({
warnings: [
Expand Down Expand Up @@ -399,7 +399,7 @@ describe("BashTool", () => {
})

test("keeps locked deferred parity TODOs visible", async () => {
const source = await fs.readFile(new URL("../src/tool/bash.ts", import.meta.url), "utf8")
const source = await fs.readFile(new URL("../src/tool/terminal.ts", import.meta.url), "utf8")
for (const todo of [
"Port tree-sitter bash / PowerShell parser-based approval reduction.",
"Port BashArity reusable command-prefix approvals.",
Expand Down
2 changes: 1 addition & 1 deletion packages/opencode/specs/v2/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const opencode = OpenCode.make({})
opencode.tool.add(ReadTool)

opencode.tool.add({
name: "bash",
name: "terminal",
schema: {
type: "object",
properties: {
Expand Down
2 changes: 1 addition & 1 deletion packages/opencode/src/acp/event.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export class Subscription {
private async runningTool(sessionId: string, part: ToolPart, cwd: string) {
if (part.state.status !== "running") return

const output = part.tool === "bash" ? shellOutputSnapshot(part.state) : undefined
const output = part.tool === "terminal" ? shellOutputSnapshot(part.state) : undefined
if (output !== undefined) {
if (this.shellSnapshots.get(part.callID) === output) {
await this.input.connection.sessionUpdate({
Expand Down
4 changes: 3 additions & 1 deletion packages/opencode/src/acp/tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export function toToolKind(toolName: string): ToolKind {
const tool = toolName.toLocaleLowerCase()

switch (tool) {
case "terminal":
case "bash":
case "shell":
return "execute"
Expand Down Expand Up @@ -74,6 +75,7 @@ export function toLocations(toolName: string, input: ToolInput, cwd?: string): T
const tool = toolName.toLocaleLowerCase()

switch (tool) {
case "terminal":
case "bash":
case "shell": {
const workdir = shellWorkdir(input, cwd)
Expand Down Expand Up @@ -296,7 +298,7 @@ function shellCommand(input: ToolInput) {

function isShell(toolName: string) {
const tool = toolName.toLocaleLowerCase()
return tool === "bash" || tool === "shell"
return tool === "terminal" || tool === "shell" || tool === "bash"
}
Comment on lines 299 to 302

export const mapToolKind = toToolKind
Expand Down
32 changes: 16 additions & 16 deletions packages/opencode/src/agent/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,8 @@ export const layer = Layer.effect(
"*.env.*": "ask",
"*.env.example": "allow",
},
// altimate_change start - bash safety defaults for destructive file/git/DDL commands
// Safety defaults for bash commands.
// altimate_change start - terminal safety defaults for destructive file/git/DDL commands
// Safety defaults for terminal commands.
// IMPORTANT: "*": "ask" must come FIRST because evaluation uses last-match-wins.
//
// "ask" = user sees prompt and can approve. Used for destructive file/git
Expand All @@ -162,7 +162,7 @@ export const layer = Layer.effect(
// almost never intentional in an agent context.
//
// Users can override any of these in altimate-code.json.
bash: {
terminal: {
"*": "ask",
"rm -rf *": "ask",
"rm -fr *": "ask",
Expand All @@ -186,11 +186,11 @@ export const layer = Layer.effect(
// Safety deny rules that CANNOT be overridden by wildcard allows.
// Appended after user config so they always take precedence via last-match-wins.
// Users who need to override must use specific patterns like
// `"DROP DATABASE test_db": "allow"` — wildcard `bash: "allow"` won't work.
// `"DROP DATABASE test_db": "allow"` — wildcard `terminal: "allow"` won't work.
// Both UPPER and lowercase variants are included because Wildcard.match
// is case-sensitive on Linux/macOS.
const safetyDenials = Permission.fromConfig({
bash: {
terminal: {
"DROP DATABASE *": "deny",
"DROP SCHEMA *": "deny",
"TRUNCATE *": "deny",
Expand Down Expand Up @@ -291,8 +291,8 @@ export const layer = Layer.effect(
websearch: "allow",
question: "allow",
tool_lookup: "allow",
// Bash: last-match-wins — "*": "deny" MUST come first, then specific allows override
bash: {
// Terminal: last-match-wins — "*": "deny" MUST come first, then specific allows override
terminal: {
"*": "deny",
"ls *": "allow",
"grep *": "allow",
Expand All @@ -319,7 +319,7 @@ export const layer = Layer.effect(
reviewer: {
name: "reviewer",
description:
"dbt PR reviewer. Runs the dbt_pr_review verdict engine (lineage, equivalence, PII, grade) plus read-only analysis tools and posts findings. Edit/write tools are denied; bash prompts for approval.",
"dbt PR reviewer. Runs the dbt_pr_review verdict engine (lineage, equivalence, PII, grade) plus read-only analysis tools and posts findings. Edit/write tools are denied; terminal prompts for approval.",
prompt: PROMPT_REVIEWER,
options: {},
permission: Permission.merge(
Expand All @@ -340,7 +340,7 @@ export const layer = Layer.effect(
schema_detect_pii: "allow",
// Writes denied — review never mutates the project.
sql_execute_write: "deny",
// Read-only file + repo access (structured tools, not bash).
// Read-only file + repo access (structured tools, not terminal).
read: "allow",
grep: "allow",
glob: "allow",
Expand All @@ -354,20 +354,20 @@ export const layer = Layer.effect(
// Read-only web access so the reviewer can pull PR/issue URLs.
webfetch: "allow",
websearch: "allow",
// Bash PROMPTS instead of hard-denying (#978: `gh pr view` is the
// terminal PROMPTS instead of hard-denying (#978: `gh pr view` is the
// primary way to review a PR URL). A string-prefix allowlist can't
// safely bound argv (redirects ride inside the matched command), so
// every bash command requires explicit user approval here — the
// every terminal command requires explicit user approval here — the
// reviewer still never runs shell commands silently.
bash: "ask",
terminal: "ask",
}),
// altimate_change start — reviewer safety must not be overridable by a permissive user
// config (e.g. global `permission: {"*":"allow"}` or `bash:"allow"`). Merge user config,
// config (e.g. global `permission: {"*":"allow"}` or `terminal:"allow"`). Merge user config,
// THEN re-apply the reviewer read-only invariants, THEN safetyDenials LAST so DDL denies
// still win over the reviewer's bash:"ask". (edit covers write/edit/apply_patch.)
// still win over the reviewer's terminal:"ask". (edit covers write/edit/apply_patch.)
user,
Permission.fromConfig({
bash: "ask",
terminal: "ask",
edit: "deny",
sql_execute_write: "deny",
}),
Expand Down Expand Up @@ -435,7 +435,7 @@ export const layer = Layer.effect(
codesearch: "allow",
// altimate_change end
list: "allow",
bash: "allow",
terminal: "allow",
webfetch: "allow",
websearch: "allow",
read: "allow",
Expand Down
Loading
Loading