refactor(shell): centralize shell tool identity

Move shell tool ID checks behind shared helpers so runtime code and tests stop duplicating bash, pwsh, and powershell branches. This keeps shell-specific behavior aligned across consumers and makes follow-on shell changes less error-prone.
This commit is contained in:
LukeParkerDev 2026-04-03 14:56:40 +10:00
parent baf476f431
commit 2eb9ae4d34
19 changed files with 99 additions and 92 deletions

View file

@ -1,20 +1,12 @@
import type { ToolPart } from "@opencode-ai/sdk/v2/client"
import type { Page } from "@playwright/test"
import { test, expect } from "../fixtures"
import { assistantText } from "../actions"
import { promptSelector } from "../selectors"
import { createSdk } from "../utils"
import { createSdk, isShell } from "../utils"
const text = (value: string | null) => (value ?? "").replace(/\u200B/g, "").trim()
type Sdk = ReturnType<typeof createSdk>
const isBash = (part: unknown): part is ToolPart => {
if (!part || typeof part !== "object") return false
if (!("type" in part) || part.type !== "tool") return false
if (!("tool" in part) || part.tool !== "bash") return false
return "state" in part
}
async function wait(page: Page, value: string) {
await expect.poll(async () => text(await page.locator(promptSelector).textContent())).toBe(value)
}
@ -31,7 +23,7 @@ async function shell(sdk: Sdk, sessionID: string, cmd: string, token: string) {
const part = messages
.filter((item) => item.info.role === "assistant")
.flatMap((item) => item.parts)
.filter(isBash)
.filter(isShell)
.find((item) => item.state.input?.command === cmd && item.state.status === "completed")
if (!part || part.state.status !== "completed") return

View file

@ -1,13 +1,6 @@
import type { ToolPart } from "@opencode-ai/sdk/v2/client"
import { test, expect } from "../fixtures"
import { withSession } from "../actions"
const isBash = (part: unknown): part is ToolPart => {
if (!part || typeof part !== "object") return false
if (!("type" in part) || part.type !== "tool") return false
if (!("tool" in part) || part.tool !== "bash") return false
return "state" in part
}
import { isShell } from "../utils"
async function setAutoAccept(page: Parameters<typeof test>[0]["page"], enabled: boolean) {
const button = page.locator('[data-action="prompt-permissions"]').first()
@ -42,7 +35,7 @@ test("shell mode runs a command in the project directory", async ({ page, projec
if (!msg) return
const part = msg.parts
.filter(isBash)
.filter(isShell)
.find((item) => item.state.input?.command === cmd && item.state.status === "completed")
if (!part || part.state.status !== "completed") return

View file

@ -1,4 +1,4 @@
import { createOpencodeClient } from "@opencode-ai/sdk/v2/client"
import { createOpencodeClient, type ToolPart } from "@opencode-ai/sdk/v2/client"
import { base64Encode, checksum } from "@opencode-ai/util/encode"
export const serverHost = process.env.PLAYWRIGHT_SERVER_HOST ?? "127.0.0.1"
@ -18,6 +18,7 @@ const serverLabels = (() => {
export const serverNames = [...new Set(serverLabels)]
export const serverUrls = serverNames.map((name) => `http://${name}`)
const shell = new Set(["bash", "pwsh", "powershell"])
const escape = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")
@ -30,6 +31,13 @@ export function createSdk(directory?: string, baseUrl = serverUrl) {
return createOpencodeClient({ baseUrl, directory, throwOnError: true })
}
export function isShell(part: unknown): part is ToolPart {
if (!part || typeof part !== "object") return false
if (!("type" in part) || part.type !== "tool") return false
if (!("tool" in part) || typeof part.tool !== "string" || !shell.has(part.tool)) return false
return "state" in part
}
export async function resolveDirectory(directory: string, baseUrl = serverUrl) {
return createSdk(directory, baseUrl)
.path.get()

View file

@ -38,6 +38,7 @@ import { Provider } from "../provider/provider"
import { ModelID, ProviderID } from "../provider/schema"
import { Agent as AgentModule } from "../agent/agent"
import { Installation } from "@/installation"
import { ShellTool } from "@/tool/shell/id"
import { MessageV2 } from "@/session/message-v2"
import { Config } from "@/config/config"
import { Todo } from "@/session/todo"
@ -138,7 +139,7 @@ export namespace ACP {
private sessionManager: ACPSessionManager
private eventAbort = new AbortController()
private eventStarted = false
private bashSnapshots = new Map<string, string>()
private shellSnapshots = new Map<string, string>()
private toolStarts = new Set<string>()
private permissionQueues = new Map<string, Promise<void>>()
private permissionOptions: PermissionOption[] = [
@ -277,16 +278,16 @@ export namespace ACP {
switch (part.state.status) {
case "pending":
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
return
case "running":
const output = this.bashOutput(part)
const output = this.shellOutput(part)
const content: ToolCallContent[] = []
if (output) {
const hash = Hash.fast(output)
if (part.tool === "bash" || part.tool === "pwsh" || part.tool === "powershell") {
if (this.bashSnapshots.get(part.callID) === hash) {
if (ShellTool.has(part.tool)) {
if (this.shellSnapshots.get(part.callID) === hash) {
await this.connection
.sessionUpdate({
sessionId,
@ -305,7 +306,7 @@ export namespace ACP {
})
return
}
this.bashSnapshots.set(part.callID, hash)
this.shellSnapshots.set(part.callID, hash)
}
content.push({
type: "content",
@ -336,7 +337,7 @@ export namespace ACP {
case "completed": {
this.toolStarts.delete(part.callID)
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
const kind = toToolKind(part.tool)
const content: ToolCallContent[] = [
{
@ -417,7 +418,7 @@ export namespace ACP {
}
case "error":
this.toolStarts.delete(part.callID)
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
await this.connection
.sessionUpdate({
sessionId,
@ -832,10 +833,10 @@ export namespace ACP {
await this.toolStart(sessionId, part)
switch (part.state.status) {
case "pending":
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
break
case "running":
const output = this.bashOutput(part)
const output = this.shellOutput(part)
const runningContent: ToolCallContent[] = []
if (output) {
runningContent.push({
@ -866,7 +867,7 @@ export namespace ACP {
break
case "completed":
this.toolStarts.delete(part.callID)
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
const kind = toToolKind(part.tool)
const content: ToolCallContent[] = [
{
@ -946,7 +947,7 @@ export namespace ACP {
break
case "error":
this.toolStarts.delete(part.callID)
this.bashSnapshots.delete(part.callID)
this.shellSnapshots.delete(part.callID)
await this.connection
.sessionUpdate({
sessionId,
@ -1100,8 +1101,8 @@ export namespace ACP {
}
}
private bashOutput(part: ToolPart) {
if (part.tool !== "bash" && part.tool !== "pwsh" && part.tool !== "powershell") return
private shellOutput(part: ToolPart) {
if (!ShellTool.has(part.tool)) return
if (!("metadata" in part.state) || !part.state.metadata || typeof part.state.metadata !== "object") return
const output = part.state.metadata["output"]
if (typeof output !== "string") return
@ -1502,11 +1503,9 @@ export namespace ACP {
function toToolKind(toolName: string): ToolKind {
const tool = toolName.toLocaleLowerCase()
if (ShellTool.has(tool)) return "execute"
switch (tool) {
case "bash":
case "pwsh":
case "powershell":
return "execute"
case "webfetch":
return "fetch"
@ -1532,6 +1531,8 @@ export namespace ACP {
function toLocations(toolName: string, input: Record<string, any>): { path: string }[] {
const tool = toolName.toLocaleLowerCase()
if (ShellTool.has(tool)) return []
switch (tool) {
case "read":
case "edit":
@ -1540,10 +1541,6 @@ export namespace ACP {
case "glob":
case "grep":
return input["path"] ? [{ path: input["path"] }] : []
case "bash":
case "pwsh":
case "powershell":
return []
case "list":
return input["path"] ? [{ path: input["path"] }] : []
default:

View file

@ -168,8 +168,6 @@ export namespace Agent {
glob: "allow",
list: "allow",
bash: "allow",
pwsh: "allow",
powershell: "allow",
webfetch: "allow",
websearch: "allow",
codesearch: "allow",

View file

@ -9,15 +9,14 @@ import fs from "fs/promises"
import { Filesystem } from "../../util/filesystem"
import matter from "gray-matter"
import { Instance } from "../../project/instance"
import { ShellTool } from "../../tool/shell/id"
import { EOL } from "os"
import type { Argv } from "yargs"
type AgentMode = "all" | "primary" | "subagent"
const AVAILABLE_TOOLS = [
"bash",
"pwsh",
"powershell",
...ShellTool.ids,
"read",
"write",
"edit",

View file

@ -25,6 +25,7 @@ import { WebSearchTool } from "../../tool/websearch"
import { TaskTool } from "../../tool/task"
import { SkillTool } from "../../tool/skill"
import { BashTool } from "../../tool/shell/bash"
import { ShellTool } from "../../tool/shell/id"
import { TodoWriteTool } from "../../tool/todo"
import { Locale } from "../../util/locale"
@ -411,8 +412,7 @@ export const RunCommand = cmd({
async function execute(sdk: OpencodeClient) {
function tool(part: ToolPart) {
try {
if (part.tool === "bash" || part.tool === "pwsh" || part.tool === "powershell")
return bash(props<typeof BashTool>(part))
if (ShellTool.has(part.tool)) return bash(props<typeof BashTool>(part))
if (part.tool === "glob") return glob(props<typeof GlobTool>(part))
if (part.tool === "grep") return grep(props<typeof GrepTool>(part))
if (part.tool === "list") return list(props<typeof ListTool>(part))

View file

@ -36,6 +36,7 @@ import type { Tool } from "@/tool/tool"
import type { ReadTool } from "@/tool/read"
import type { WriteTool } from "@/tool/write"
import { BashTool } from "@/tool/shell/bash"
import { ShellTool } from "@/tool/shell/id"
import type { GlobTool } from "@/tool/glob"
import { TodoWriteTool } from "@/tool/todo"
import type { GrepTool } from "@/tool/grep"
@ -1519,7 +1520,7 @@ function ToolPart(props: { last: boolean; part: ToolPart; message: AssistantMess
return (
<Show when={!shouldHide()}>
<Switch>
<Match when={props.part.tool === "bash" || props.part.tool === "pwsh" || props.part.tool === "powershell"}>
<Match when={ShellTool.has(props.part.tool)}>
<Bash {...toolprops} />
</Match>
<Match when={props.part.tool === "glob"}>

View file

@ -14,6 +14,7 @@ import { LANGUAGE_EXTENSIONS } from "@/lsp/language"
import { Keybind } from "@/util/keybind"
import { Locale } from "@/util/locale"
import { Global } from "@/global"
import { ShellTool } from "@/tool/shell/id"
import { useDialog } from "../../ui/dialog"
import { getScrollAcceleration } from "../../util/scroll"
import { useTuiConfig } from "../../context/tui-config"
@ -283,7 +284,7 @@ export function PermissionPrompt(props: { request: PermissionRequest }) {
}
}
if (permission === "bash" || permission === "pwsh" || permission === "powershell") {
if (ShellTool.has(permission)) {
const title =
typeof data.description === "string" && data.description ? data.description : "Shell command"
const command = typeof data.command === "string" ? data.command : ""

View file

@ -43,6 +43,7 @@ import { Permission } from "@/permission"
import { SessionStatus } from "./status"
import { LLM } from "./llm"
import { Shell } from "@/shell/shell"
import { ShellTool } from "@/tool/shell/id"
import { AppFileSystem } from "@/filesystem"
import { Truncate } from "@/tool/truncate"
import { decodeDataUrl } from "@/util/data-url"
@ -791,7 +792,7 @@ NOTE: At any point in time through this workflow you should feel free to ask the
yield* sessions.updateMessage(msg)
const sh = Shell.preferred()
const name = Shell.name(sh)
const tool = name === "pwsh" ? "pwsh" : name === "powershell" ? "powershell" : "bash"
const tool = ShellTool.from(name)
const part: MessageV2.ToolPart = {
type: "tool",
id: PartID.ascending(),

View file

@ -32,6 +32,7 @@ import { Effect, Layer, ServiceMap } from "effect"
import { InstanceState } from "@/effect/instance-state"
import { makeRuntime } from "@/effect/run-service"
import { BashTool } from "./shell/bash"
import { ShellTool } from "./shell/id"
import { PwshTool } from "./shell/pwsh"
import { PowershellTool } from "./shell/powershell"
import { Shell } from "@/shell/shell"
@ -39,6 +40,7 @@ import { Env } from "../env"
export namespace ToolRegistry {
const log = Log.create({ service: "tool.registry" })
const shells = { bash: BashTool, pwsh: PwshTool, powershell: PowershellTool } as const
type State = {
custom: Tool.Info[]
@ -118,14 +120,12 @@ export namespace ToolRegistry {
const all = Effect.fn("ToolRegistry.all")(function* (custom: Tool.Info[]) {
const cfg = yield* config.get()
const question = ["app", "cli", "desktop"].includes(Flag.OPENCODE_CLIENT) || Flag.OPENCODE_ENABLE_QUESTION_TOOL
const shellName = Shell.name(Shell.acceptable())
const ActiveShellTool = shellName === "pwsh" ? PwshTool : shellName === "powershell" ? PowershellTool : BashTool
const active = shells[ShellTool.from(Shell.name(Shell.acceptable()))]
return [
InvalidTool,
...(question ? [QuestionTool] : []),
ActiveShellTool,
active,
ReadTool,
GlobTool,
GrepTool,

View file

@ -1,10 +1,8 @@
import { ShellTool } from "./id"
export namespace ShellArity {
export function prefix(tokens: string[], shellType: "bash" | "pwsh" | "powershell") {
if (
(shellType === "pwsh" || shellType === "powershell") &&
tokens.length > 0 &&
/^[a-z]+-[a-z]+$/i.test(tokens[0])
) {
export function prefix(tokens: string[], shellType: ShellTool.ID) {
if (ShellTool.powershell(shellType) && tokens.length > 0 && /^[a-z]+-[a-z]+$/i.test(tokens[0])) {
return [tokens[0]]
}
for (let len = tokens.length; len > 0; len--) {

View file

@ -0,0 +1,19 @@
export namespace ShellTool {
export const ids = ["bash", "pwsh", "powershell"] as const
export type ID = (typeof ids)[number]
const shell = new Set<string>(ids)
const ps = new Set<string>(["pwsh", "powershell"])
export function has(value: string): value is ID {
return shell.has(value)
}
export function from(value: string): ID {
return has(value) ? value : "bash"
}
export function powershell(value: string) {
return ps.has(value)
}
}

View file

@ -1,6 +1,7 @@
import type { Node } from "web-tree-sitter"
import { lazy } from "@/util/lazy"
import { resolveWasm, resolvePath, unquote, home, expand, type Scan, type Part } from "./util"
import { ShellTool } from "./id"
import { Instance } from "@/project/instance"
import { Filesystem } from "@/util/filesystem"
import path from "path"
@ -160,9 +161,9 @@ export namespace ShellParser {
command: string
cwd: string
shell: string
shellType: "bash" | "pwsh" | "powershell"
shellType: ShellTool.ID
}): Promise<Scan> {
const isPwsh = opts.shellType === "pwsh" || opts.shellType === "powershell"
const isPwsh = ShellTool.powershell(opts.shellType)
const parsers = await getParser()
const parser = isPwsh ? parsers.ps : parsers.bash

View file

@ -2,6 +2,7 @@ import { spawn } from "child_process"
import { Shell } from "@/shell/shell"
import { Tool } from "../tool"
import { Plugin } from "@/plugin"
import { ShellTool } from "./id"
const MAX_METADATA_LENGTH = 30_000
@ -27,7 +28,7 @@ exit 1`
}
export function launch(shell: string, name: string, command: string, cwd: string, env: NodeJS.ProcessEnv) {
if (process.platform === "win32" && (name === "powershell" || name === "pwsh")) {
if (process.platform === "win32" && ShellTool.powershell(name)) {
return spawn(shell, ["-NoLogo", "-NoProfile", "-NonInteractive", "-Command", preserveExitCode(command)], {
cwd,
env,

View file

@ -108,12 +108,13 @@ export function formatShellDescription(
import z from "zod"
import DESCRIPTION from "./shell.txt"
import { ShellTool } from "./id"
import { Log } from "@/util/log"
import { Flag } from "@/flag/flag"
import { ShellParser } from "./parser"
import { ShellRunner } from "./runner"
export type ShellType = "bash" | "pwsh" | "powershell"
export type ShellType = ShellTool.ID
const DEFAULT_TIMEOUT = Flag.OPENCODE_EXPERIMENTAL_BASH_DEFAULT_TIMEOUT_MS || 2 * 60 * 1000

View file

@ -16,10 +16,12 @@ import { Effect } from "effect"
import fs from "fs/promises"
import path from "path"
import { Session } from "../../src/session"
import { Shell } from "../../src/shell/shell"
import { LLM } from "../../src/session/llm"
import { SessionPrompt } from "../../src/session/prompt"
import { SessionSummary } from "../../src/session/summary"
import { MessageV2 } from "../../src/session/message-v2"
import { ShellTool } from "../../src/tool/shell/id"
import { Log } from "../../src/util/log"
import { provideTmpdirServer } from "../fixture/fixture"
import { testEffect } from "../lib/effect"
@ -42,7 +44,6 @@ import { SessionCompaction } from "../../src/session/compaction"
import { Instruction } from "../../src/session/instruction"
import { SessionProcessor } from "../../src/session/processor"
import { SessionStatus } from "../../src/session/status"
import { Shell } from "../../src/shell/shell"
import { Snapshot } from "../../src/snapshot"
import { ToolRegistry } from "../../src/tool/registry"
import { Truncate } from "../../src/tool/truncate"
@ -183,13 +184,15 @@ it.live("tool execution produces non-empty session diff (snapshot race)", () =>
permission: [{ permission: "*", pattern: "*", action: "allow" }],
})
// Use bash tool (always registered) to create a file
const shell = ShellTool.from(Shell.name(Shell.acceptable()))
// Use the active shell tool to create a file
const command = `echo 'snapshot race test content' > ${path.join(dir, "race-test.txt")}`
yield* llm.toolMatch((hit) => JSON.stringify(hit.body).includes("create the file"), "bash", {
yield* llm.toolMatch((hit) => JSON.stringify(hit.body).includes("create the file"), shell, {
command,
description: "create test file",
})
yield* llm.textMatch((hit) => JSON.stringify(hit.body).includes("bash"), "done")
yield* llm.textMatch((hit) => JSON.stringify(hit.body).includes(shell), "done")
// Seed user message
yield* prompt.prompt({
@ -217,7 +220,7 @@ it.live("tool execution produces non-empty session diff (snapshot race)", () =>
const allMsgs = yield* MessageV2.filterCompactedEffect(session.id)
const tool = allMsgs
.flatMap((m) => m.parts)
.find((p): p is MessageV2.ToolPart => p.type === "tool" && p.tool === "bash")
.find((p): p is MessageV2.ToolPart => p.type === "tool" && p.tool === shell)
expect(tool?.state.status).toBe("completed")
// Poll for diff — summarize() is fire-and-forget

View file

@ -3,6 +3,7 @@ import os from "os"
import path from "path"
import { Shell } from "../../src/shell/shell"
import { BashTool } from "../../src/tool/shell/bash"
import { ShellTool } from "../../src/tool/shell/id"
import { PwshTool } from "../../src/tool/shell/pwsh"
import { PowershellTool } from "../../src/tool/shell/powershell"
import { Instance } from "../../src/project/instance"
@ -47,15 +48,14 @@ const shells = (() => {
(item, i) => list.findIndex((other) => other.shell.toLowerCase() === item.shell.toLowerCase()) === i,
)
})()
const PS = new Set(["pwsh", "powershell"])
const ps = shells.filter((item) => PS.has(item.label))
const ps = shells.filter((item) => ShellTool.powershell(item.label))
const sh = () => Shell.name(Shell.acceptable())
const evalarg = (text: string) => (sh() === "cmd" ? quote(text) : squote(text))
const js = (code: string, ...args: Array<number | string>) => {
const tail = args.length ? ` ${args.map(String).join(" ")}` : ""
const text = `${bin} -e ${evalarg(code)}${tail}`
if (PS.has(sh())) return `& ${text}`
if (ShellTool.powershell(sh())) return `& ${text}`
return text
}
@ -92,18 +92,12 @@ const withShell = (item: { label: string; shell: string }, fn: () => Promise<voi
}
}
const expectedPermission = () => {
const name = sh()
if (name === "pwsh") return "pwsh"
if (name === "powershell") return "powershell"
return "bash"
}
const expectedPermission = () => ShellTool.from(sh())
const tools = { bash: BashTool, pwsh: PwshTool, powershell: PowershellTool } as const
const getTool = async () => {
const name = sh()
if (name === "pwsh") return await PwshTool.init()
if (name === "powershell") return await PowershellTool.init()
return await BashTool.init()
return await tools[ShellTool.from(sh())].init()
}
const each = (name: string, fn: (item: { label: string; shell: string }) => Promise<void>) => {

View file

@ -278,6 +278,14 @@ function agentTitle(i18n: UiI18n, type?: string) {
export function getToolInfo(tool: string, input: any = {}): ToolInfo {
const i18n = useI18n()
if (SHELL.has(tool)) {
return {
icon: "console",
title: i18n.t("ui.tool.shell"),
subtitle: input.description,
}
}
switch (tool) {
case "read":
return {
@ -332,14 +340,6 @@ export function getToolInfo(tool: string, input: any = {}): ToolInfo {
subtitle: input.description,
}
}
case "bash":
case "pwsh":
case "powershell":
return {
icon: "console",
title: i18n.t("ui.tool.shell"),
subtitle: input.description,
}
case "edit":
return {
icon: "code-lines",