From 5345c9897a7e331a06cdda62fa643ca93aa93c6c Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Wed, 22 Apr 2026 13:30:58 +0300 Subject: [PATCH 1/8] fix(acp): emit tool_call_update with content/title and raw I/O --- packages/opencode/src/acp/agent.ts | 94 +++++++++--------------------- 1 file changed, 28 insertions(+), 66 deletions(-) diff --git a/packages/opencode/src/acp/agent.ts b/packages/opencode/src/acp/agent.ts index f12328153b6e..374677350a1d 100644 --- a/packages/opencode/src/acp/agent.ts +++ b/packages/opencode/src/acp/agent.ts @@ -51,6 +51,7 @@ import { LoadAPIKeyError } from "ai" import type { AssistantMessage, Event, OpencodeClient, SessionMessageResponse, ToolPart } from "@opencode-ai/sdk/v2" import { applyPatch } from "diff" import { InstallationVersion } from "@/installation/version" +import { toolCallFromPart, toolResultFromPart } from "./tool-format" type ModeOption = { id: string; name: string; description?: string } type ModelOption = { modelId: string; name: string } @@ -198,16 +199,17 @@ export class Agent implements ACPAgent { .then(async () => { const directory = session.cwd + const permissionInfo = toolCallFromPart(permission.permission, permission.metadata ?? {}) const res = await this.connection .requestPermission({ sessionId: permission.sessionID, toolCall: { toolCallId: permission.tool?.callID ?? permission.id, status: "pending", - title: permission.permission, - rawInput: permission.metadata, - kind: toToolKind(permission.permission), - locations: toLocations(permission.permission, permission.metadata), + title: permissionInfo.title, + rawInput: permissionInfo.rawInput, + kind: permissionInfo.kind, + locations: permissionInfo.locations, }, options: this.permissionOptions, }) @@ -279,13 +281,14 @@ export class Agent implements ACPAgent { if (part.type === "tool") { await this.toolStart(sessionId, part) + const info = toolCallFromPart(part.tool, part.state.input) switch (part.state.status) { case "pending": this.bashSnapshots.delete(part.callID) return - case "running": + case "running": { const output = this.bashOutput(part) const content: ToolCallContent[] = [] if (output) { @@ -299,10 +302,10 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "in_progress", - kind: toToolKind(part.tool), - title: part.tool, - locations: toLocations(part.tool, part.state.input), - rawInput: part.state.input, + kind: info.kind, + title: info.title, + locations: info.locations, + rawInput: info.rawInput, }, }) .catch((error) => { @@ -327,10 +330,10 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "in_progress", - kind: toToolKind(part.tool), - title: part.tool, - locations: toLocations(part.tool, part.state.input), - rawInput: part.state.input, + kind: info.kind, + title: info.title, + locations: info.locations, + rawInput: info.rawInput, ...(content.length > 0 && { content }), }, }) @@ -338,38 +341,12 @@ export class Agent implements ACPAgent { log.error("failed to send tool in_progress to ACP", { error }) }) return + } case "completed": { this.toolStarts.delete(part.callID) this.bashSnapshots.delete(part.callID) - const kind = toToolKind(part.tool) - const content: ToolCallContent[] = [ - { - type: "content", - content: { - type: "text", - text: part.state.output, - }, - }, - ] - - if (kind === "edit") { - const input = part.state.input - const filePath = typeof input["filePath"] === "string" ? input["filePath"] : "" - const oldText = typeof input["oldString"] === "string" ? input["oldString"] : "" - const newText = - typeof input["newString"] === "string" - ? input["newString"] - : typeof input["content"] === "string" - ? input["content"] - : "" - content.push({ - type: "diff", - path: filePath, - oldText, - newText, - }) - } + const result = toolResultFromPart(part.tool, part.state.input, part.state.output, false) if (part.tool === "todowrite") { const parsedTodos = z.array(Todo.Info).safeParse(JSON.parse(part.state.output)) @@ -405,14 +382,9 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "completed", - kind, - content, - title: part.state.title, - rawInput: part.state.input, - rawOutput: { - output: part.state.output, - metadata: part.state.metadata, - }, + content: result.content, + rawOutput: result.rawOutput, + ...(result.title ? { title: result.title } : {}), }, }) .catch((error) => { @@ -420,9 +392,11 @@ export class Agent implements ACPAgent { }) return } - case "error": + case "error": { this.toolStarts.delete(part.callID) this.bashSnapshots.delete(part.callID) + const result = toolResultFromPart(part.tool, part.state.input, part.state.error, true) + await this.connection .sessionUpdate({ sessionId, @@ -430,28 +404,16 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "failed", - kind: toToolKind(part.tool), - title: part.tool, - rawInput: part.state.input, - content: [ - { - type: "content", - content: { - type: "text", - text: part.state.error, - }, - }, - ], - rawOutput: { - error: part.state.error, - metadata: part.state.metadata, - }, + content: result.content, + rawOutput: result.rawOutput, + ...(result.title ? { title: result.title } : {}), }, }) .catch((error) => { log.error("failed to send tool error to ACP", { error }) }) return + } } } From f257541646a2fe75896e047991cff0efa8eb381a Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Wed, 22 Apr 2026 13:31:04 +0300 Subject: [PATCH 2/8] refactor(acp): extract tool and command formatting into dedicated modules --- packages/opencode/src/acp/parse-command.ts | 21 + packages/opencode/src/acp/tool-format.ts | 449 ++++++++++++++++++ .../opencode/test/acp/parse-command.test.ts | 32 ++ 3 files changed, 502 insertions(+) create mode 100644 packages/opencode/src/acp/parse-command.ts create mode 100644 packages/opencode/src/acp/tool-format.ts create mode 100644 packages/opencode/test/acp/parse-command.test.ts diff --git a/packages/opencode/src/acp/parse-command.ts b/packages/opencode/src/acp/parse-command.ts new file mode 100644 index 000000000000..433cd09d9570 --- /dev/null +++ b/packages/opencode/src/acp/parse-command.ts @@ -0,0 +1,21 @@ +import type { ToolKind } from "@agentclientprotocol/sdk" + +export namespace ParseCommand { + export interface Result { + kind: ToolKind + title: string + locations: { path: string }[] + terminalOutput: boolean + } + + export function format(command: string, description: string, cwd: string): Result { + const title = description || command || "Terminal" + + return { + kind: "other", + title, + locations: cwd ? [{ path: cwd }] : [], + terminalOutput: true, + } + } +} diff --git a/packages/opencode/src/acp/tool-format.ts b/packages/opencode/src/acp/tool-format.ts new file mode 100644 index 000000000000..5a89e569c40c --- /dev/null +++ b/packages/opencode/src/acp/tool-format.ts @@ -0,0 +1,449 @@ +import type { ToolCallContent, ToolKind } from "@agentclientprotocol/sdk" +import { ParseCommand } from "./parse-command" + +export interface ToolCallInfo { + title: string + kind: ToolKind + content: ToolCallContent[] + locations: { path: string; line?: number }[] + rawInput: unknown +} + +export interface ToolResultInfo { + content: ToolCallContent[] + rawOutput: unknown + title?: string +} + +function normalize(name: string): string { + return name.toLowerCase().replace(/^mcp__acp__/, "") +} + +function truncate(str: string, max: number): string { + return str.length > max ? str.substring(0, max - 3) + "..." : str +} + +function markdownEscape(text: string): string { + let fence = "```" + for (const match of text.matchAll(/^`{3,}/gm)) { + while (match[0].length >= fence.length) fence += "`" + } + return fence + "\n" + text + (text.endsWith("\n") ? "" : "\n") + fence +} + +function textContent(text: string): ToolCallContent { + return { type: "content", content: { type: "text", text } } +} + +function diffContent(path: string, oldText: string | null, newText: string): ToolCallContent { + return { type: "diff", path, oldText, newText } +} + +function str(v: unknown): string { + return typeof v === "string" ? v : "" +} + +function num(v: unknown): number | undefined { + return typeof v === "number" ? v : undefined +} + +function getFilePath(input: Record): string { + return str(input.filePath ?? input.file_path ?? input.filepath ?? input.path) +} + +function getOldString(input: Record): string { + return str(input.oldString ?? input.old_string) +} + +function getNewString(input: Record): string { + return str(input.newString ?? input.new_string ?? input.content ?? input.new_content) +} + +function getCommand(input: Record): string { + return str(input.command ?? input.cmd) +} + +function getDescription(input: Record): string { + return str(input.description ?? input.desc) +} + +function getPattern(input: Record): string { + return str(input.pattern ?? input.filePattern ?? input.glob) +} + +function getQuery(input: Record): string { + return str(input.query ?? input.q) +} + +function getUrl(input: Record): string { + return str(input.url ?? input.uri) +} + +function getDiff(input: Record): string { + return str(input.diff ?? input.patch ?? input.unifiedDiff) +} + +import { resolve } from "path" + +function abs(p: string): string { + return p && !p.startsWith("/") ? resolve(p) : p +} + +export function toolCallFromPart(tool: string, input: Record): ToolCallInfo { + const name = normalize(tool) + + switch (name) { + case "bash": + case "shell": + case "terminal": { + const command = getCommand(input) + const description = getDescription(input) + const cwd = str(input.cwd ?? input.workdir ?? input.workingDir ?? input.directory) + const result = ParseCommand.format(command, description, cwd) + return { + title: result.title, + kind: result.kind, + content: [], + locations: result.locations, + rawInput: input, + } + } + + case "bashoutput": { + return { + title: "Tail Logs", + kind: "execute", + content: [], + locations: [], + rawInput: input, + } + } + + case "read": + case "view": { + const filePath = getFilePath(input) + const offset = num(input.offset) ?? num(input.line) ?? 0 + const limit = num(input.limit) ?? 0 + let suffix = "" + if (limit) { + suffix = ` (${offset + 1} - ${offset + limit})` + } else if (offset) { + suffix = ` (from line ${offset + 1})` + } + return { + title: filePath ? `Read ${filePath}${suffix}` : "Read File", + kind: "read", + content: [], + locations: filePath ? [{ path: abs(filePath), ...(offset ? { line: offset + 1 } : {}) }] : [], + rawInput: input, + } + } + + case "list": + case "ls": { + const path = str(input.path) + return { + title: path ? `List \`${path}\`` : "List directory", + kind: "read", + content: [], + locations: path ? [{ path: abs(path) }] : [], + rawInput: input, + } + } + + case "edit": + case "str_replace": { + const filePath = getFilePath(input) + const oldString = getOldString(input) + const newString = getNewString(input) + return { + title: filePath ? `Edit \`${filePath}\`` : "Edit", + kind: "edit", + content: filePath ? [diffContent(abs(filePath), oldString, newString)] : [], + locations: filePath ? [{ path: abs(filePath) }] : [], + rawInput: input, + } + } + + case "patch": { + const filePath = getFilePath(input) + const patchText = getDiff(input) + return { + title: filePath ? `Patch \`${filePath}\`` : "Patch", + kind: "edit", + content: patchText ? [textContent(patchText)] : [], + locations: filePath ? [{ path: abs(filePath) }] : [], + rawInput: input, + } + } + + case "write": + case "create": { + const filePath = getFilePath(input) + const content = getNewString(input) + return { + title: filePath ? `Write ${filePath}` : "Write", + kind: "edit", + content: filePath ? [diffContent(abs(filePath), null, content)] : [], + locations: filePath ? [{ path: abs(filePath) }] : [], + rawInput: input, + } + } + + case "glob": + case "find": { + const path = str(input.path) + const pattern = getPattern(input) + let label = "Find" + if (path) label += ` \`${path}\`` + if (pattern) label += ` \`${pattern}\`` + return { + title: label, + kind: "search", + content: [], + locations: path ? [{ path: abs(path) }] : [], + rawInput: input, + } + } + + case "grep": + case "search": { + const pattern = getPattern(input) + const path = str(input.path) + let label = "grep" + if (pattern) label += ` "${truncate(pattern, 30)}"` + if (path) label += ` ${path}` + return { + title: label, + kind: "search", + content: [], + locations: path ? [{ path: abs(path) }] : [], + rawInput: input, + } + } + + case "webfetch": + case "fetch": { + const url = getUrl(input) + const prompt = str(input.prompt) + return { + title: url ? `Fetch ${truncate(url, 40)}` : "Fetch", + kind: "fetch", + content: prompt ? [textContent(prompt)] : [], + locations: [], + rawInput: input, + } + } + + case "websearch": { + const query = getQuery(input) + return { + title: query ? `"${truncate(query, 40)}"` : "Search", + kind: "fetch", + content: [], + locations: [], + rawInput: input, + } + } + + case "task": { + const description = getDescription(input) + const prompt = str(input.prompt) + return { + title: description || "Task", + kind: "think", + content: prompt ? [textContent(prompt)] : [], + locations: [], + rawInput: input, + } + } + + case "todowrite": + case "todoread": { + return { + title: "Update TODOs", + kind: "think", + content: [], + locations: [], + rawInput: input, + } + } + + case "plan_exit": { + return { + title: "Exit Plan Mode", + kind: "switch_mode", + content: [], + locations: [], + rawInput: input, + } + } + + case "plan_enter": { + return { + title: "Enter Plan Mode", + kind: "switch_mode", + content: [], + locations: [], + rawInput: input, + } + } + + case "apply_patch": { + const filePath = getFilePath(input) + const patchText = getDiff(input) + return { + title: filePath ? `Apply Patch \`${filePath}\`` : "Apply Patch", + kind: "edit", + content: patchText ? [textContent(patchText)] : [], + locations: filePath ? [{ path: abs(filePath) }] : [], + rawInput: input, + } + } + + case "multiedit": { + return { + title: "Multi Edit", + kind: "edit", + content: [], + locations: [], + rawInput: input, + } + } + + case "batch": { + return { + title: "Batch", + kind: "other", + content: [], + locations: [], + rawInput: input, + } + } + + case "skill": { + const name = str(input.name) + return { + title: name ? `Skill: ${name}` : "Skill", + kind: "other", + content: [], + locations: [], + rawInput: input, + } + } + + case "question": { + const question = getQuery(input) || str(input.question) + return { + title: question ? truncate(question, 40) : "Question", + kind: "other", + content: [], + locations: [], + rawInput: input, + } + } + + case "lsp": { + return { + title: "LSP", + kind: "other", + content: [], + locations: [], + rawInput: input, + } + } + + case "codesearch": { + const query = getQuery(input) + return { + title: query ? `Search: ${truncate(query, 30)}` : "Code Search", + kind: "search", + content: [], + locations: [], + rawInput: input, + } + } + + default: { + const description = getDescription(input) + const command = getCommand(input) + const title = description || command || tool + return { + title: truncate(title, 50), + kind: "other", + content: [], + locations: [], + rawInput: input, + } + } + } +} + +export function toolResultFromPart( + tool: string, + input: Record, + output: string, + isError: boolean, +): ToolResultInfo { + const name = normalize(tool) + const displayText = isError ? markdownEscape(output) : output + const content: ToolCallContent[] = [textContent(displayText)] + + switch (name) { + case "bash": + case "shell": + case "terminal": { + return { + content, + rawOutput: isError ? { stderr: output } : { stdout: output }, + } + } + + case "edit": + case "str_replace": { + const filePath = getFilePath(input) + const oldString = getOldString(input) + const newString = getNewString(input) + if (filePath && !isError) { + content.push(diffContent(abs(filePath), oldString, newString)) + } + return { + content, + rawOutput: { stdout: output }, + } + } + + case "patch": + case "apply_patch": { + const filePath = getFilePath(input) + const patchText = getDiff(input) + if (filePath && patchText && !isError) { + content.push(textContent(patchText)) + } + return { + content, + rawOutput: { stdout: output }, + } + } + + case "write": + case "create": { + const filePath = getFilePath(input) + const fileContent = getNewString(input) + if (filePath && !isError) { + content.push(diffContent(abs(filePath), null, fileContent)) + } + return { + content, + rawOutput: { stdout: output }, + } + } + + default: { + return { + content, + rawOutput: isError ? { stderr: output } : { stdout: output }, + } + } + } +} diff --git a/packages/opencode/test/acp/parse-command.test.ts b/packages/opencode/test/acp/parse-command.test.ts new file mode 100644 index 000000000000..782cf0fac528 --- /dev/null +++ b/packages/opencode/test/acp/parse-command.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "bun:test" +import { ParseCommand } from "../../src/acp/parse-command" + +describe("ParseCommand", () => { + describe("format", () => { + it("uses description as title when provided", () => { + const result = ParseCommand.format("ls", "List files in current directory", "/home/user") + expect(result.title).toBe("List files in current directory") + expect(result.kind).toBe("other") + }) + + it("falls back to command when no description", () => { + const result = ParseCommand.format("ls -la", "", "/home/user") + expect(result.title).toBe("ls -la") + }) + + it("includes cwd in locations", () => { + const result = ParseCommand.format("ls", "List files", "/home/user") + expect(result.locations).toEqual([{ path: "/home/user" }]) + }) + + it("handles empty cwd", () => { + const result = ParseCommand.format("ls", "List files", "") + expect(result.locations).toEqual([]) + }) + + it("sets terminalOutput to true", () => { + const result = ParseCommand.format("npm install", "Install dependencies", "/home/user") + expect(result.terminalOutput).toBe(true) + }) + }) +}) From a54c878048d2fa8b6a5b0be346fa0495404db1c1 Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Wed, 22 Apr 2026 13:31:22 +0300 Subject: [PATCH 3/8] test(acp): add comprehensive tests for tool call formatting and spec alignment --- .../opencode/test/acp/tool-format.test.ts | 282 ++++++++++++++++++ 1 file changed, 282 insertions(+) create mode 100644 packages/opencode/test/acp/tool-format.test.ts diff --git a/packages/opencode/test/acp/tool-format.test.ts b/packages/opencode/test/acp/tool-format.test.ts new file mode 100644 index 000000000000..c1c3b0d3b9fc --- /dev/null +++ b/packages/opencode/test/acp/tool-format.test.ts @@ -0,0 +1,282 @@ +import { describe, expect, it } from "bun:test" +import { toolCallFromPart, toolResultFromPart } from "../../src/acp/tool-format" + +describe("toolCallFromPart", () => { + describe("bash/shell/terminal", () => { + it("formats bash command with description", () => { + const result = toolCallFromPart("bash", { command: "ls -la", description: "List files", cwd: "/home" }) + expect(result.title).toBe("List files") + expect(result.kind).toBe("other") + expect(result.locations).toEqual([{ path: "/home" }]) + expect(result.rawInput).toEqual({ command: "ls -la", description: "List files", cwd: "/home" }) + }) + + it("falls back to command when no description", () => { + const result = toolCallFromPart("shell", { command: "npm install" }) + expect(result.title).toBe("npm install") + }) + + it("normalizes mcp__acp__ prefix", () => { + const result = toolCallFromPart("mcp__acp__bash", { command: "echo hi" }) + expect(result.title).toBe("echo hi") + }) + }) + + describe("bashoutput", () => { + it("returns Tail Logs title", () => { + const result = toolCallFromPart("bashoutput", {}) + expect(result.title).toBe("Tail Logs") + expect(result.kind).toBe("execute") + }) + }) + + describe("read/view", () => { + it("formats read with file path", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts" }) + expect(result.title).toBe("Read /src/index.ts") + expect(result.kind).toBe("read") + expect(result.locations).toEqual([{ path: "/src/index.ts" }]) + }) + + it("uses 1-based line numbers when offset is provided", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10 }) + expect(result.locations).toEqual([{ path: "/src/index.ts", line: 11 }]) + }) + + it("omits line key when offset is zero", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 0 }) + expect(result.locations).toEqual([{ path: "/src/index.ts" }]) + }) + + it("includes line range suffix", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10, limit: 20 }) + expect(result.title).toBe("Read /src/index.ts (11 - 30)") + }) + + it("includes from-line suffix", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10 }) + expect(result.title).toBe("Read /src/index.ts (from line 11)") + }) + + it("falls back when no file path", () => { + const result = toolCallFromPart("read", {}) + expect(result.title).toBe("Read File") + expect(result.locations).toEqual([]) + }) + }) + + describe("edit/str_replace", () => { + it("formats edit with diff content", () => { + const result = toolCallFromPart("edit", { + filePath: "/src/app.ts", + oldString: "foo", + newString: "bar", + }) + expect(result.title).toBe("Edit `/src/app.ts`") + expect(result.kind).toBe("edit") + expect(result.content).toEqual([{ type: "diff", path: "/src/app.ts", oldText: "foo", newText: "bar" }]) + expect(result.locations).toEqual([{ path: "/src/app.ts" }]) + }) + + it("handles missing file path", () => { + const result = toolCallFromPart("str_replace", { oldString: "a", newString: "b" }) + expect(result.title).toBe("Edit") + expect(result.content).toEqual([]) + }) + }) + + describe("write/create", () => { + it("formats write with diff content (null oldText)", () => { + const result = toolCallFromPart("write", { filePath: "/new.ts", content: "hello" }) + expect(result.title).toBe("Write /new.ts") + expect(result.kind).toBe("edit") + expect(result.content).toEqual([{ type: "diff", path: "/new.ts", oldText: null, newText: "hello" }]) + }) + }) + + describe("glob/find", () => { + it("formats glob with path and pattern", () => { + const result = toolCallFromPart("glob", { path: "/src", pattern: "*.ts" }) + expect(result.title).toBe("Find `/src` `*.ts`") + expect(result.kind).toBe("search") + }) + + it("absolutizes relative paths", () => { + const result = toolCallFromPart("glob", { path: "src", pattern: "*.ts" }) + expect(result.locations[0].path.startsWith("/")).toBe(true) + }) + + it("handles no path or pattern", () => { + const result = toolCallFromPart("find", {}) + expect(result.title).toBe("Find") + }) + }) + + describe("grep/search", () => { + it("formats grep with pattern and path", () => { + const result = toolCallFromPart("grep", { pattern: "TODO", path: "/src" }) + expect(result.title).toBe('grep "TODO" /src') + expect(result.kind).toBe("search") + }) + + it("truncates long patterns", () => { + const long = "a".repeat(50) + const result = toolCallFromPart("grep", { pattern: long }) + expect(result.title.length).toBeLessThanOrEqual(40) + }) + }) + + describe("webfetch/fetch", () => { + it("formats fetch with url", () => { + const result = toolCallFromPart("webfetch", { url: "https://example.com", prompt: "get title" }) + expect(result.title).toBe("Fetch https://example.com") + expect(result.kind).toBe("fetch") + expect(result.content).toHaveLength(1) + }) + }) + + describe("websearch", () => { + it("formats search with query", () => { + const result = toolCallFromPart("websearch", { query: "bun test" }) + expect(result.title).toBe('"bun test"') + expect(result.kind).toBe("fetch") + }) + }) + + describe("task", () => { + it("formats task with description", () => { + const result = toolCallFromPart("task", { description: "Research APIs", prompt: "find REST patterns" }) + expect(result.title).toBe("Research APIs") + expect(result.kind).toBe("think") + expect(result.content).toHaveLength(1) + }) + }) + + describe("plan mode", () => { + it("emits switch_mode kind for plan_enter", () => { + const result = toolCallFromPart("plan_enter", {}) + expect(result.title).toBe("Enter Plan Mode") + expect(result.kind).toBe("switch_mode") + }) + + it("emits switch_mode kind for plan_exit", () => { + const result = toolCallFromPart("plan_exit", {}) + expect(result.title).toBe("Exit Plan Mode") + expect(result.kind).toBe("switch_mode") + }) + }) + + describe("bash kind pinning", () => { + it("uses kind 'other' instead of spec 'execute' to avoid Zed blue run-box styling", () => { + const result = toolCallFromPart("bash", { command: "ls", description: "List files" }) + expect(result.kind).toBe("other") + }) + }) + + describe("list", () => { + it("uses read kind for directory listing", () => { + const result = toolCallFromPart("list", { path: "/src" }) + expect(result.kind).toBe("read") + }) + }) + + describe("default", () => { + it("falls back to tool name for unknown tools", () => { + const result = toolCallFromPart("unknownTool", {}) + expect(result.title).toBe("unknownTool") + expect(result.kind).toBe("other") + }) + + it("uses description if available", () => { + const result = toolCallFromPart("custom", { description: "Custom action" }) + expect(result.title).toBe("Custom action") + }) + }) +}) + +describe("toolResultFromPart", () => { + describe("bash/shell/terminal", () => { + it("returns stdout for success", () => { + const result = toolResultFromPart("bash", { command: "ls" }, "file1\nfile2", false) + expect(result.rawOutput).toEqual({ stdout: "file1\nfile2" }) + expect(result.content).toHaveLength(1) + expect(result.content[0]).toEqual({ type: "content", content: { type: "text", text: "file1\nfile2" } }) + }) + + it("returns stderr for error", () => { + const result = toolResultFromPart("shell", { command: "bad" }, "command not found", true) + expect(result.rawOutput).toEqual({ stderr: "command not found" }) + }) + }) + + describe("edit/str_replace", () => { + it("includes diff content on success", () => { + const result = toolResultFromPart( + "edit", + { filePath: "/src/app.ts", oldString: "foo", newString: "bar" }, + "Applied edit", + false, + ) + expect(result.rawOutput).toEqual({ stdout: "Applied edit" }) + expect(result.content).toHaveLength(2) + expect(result.content[1]).toEqual({ type: "diff", path: "/src/app.ts", oldText: "foo", newText: "bar" }) + }) + + it("skips diff content on error", () => { + const result = toolResultFromPart( + "edit", + { filePath: "/src/app.ts", oldString: "foo", newString: "bar" }, + "old_string not found", + true, + ) + expect(result.content).toHaveLength(1) + }) + }) + + describe("write/create", () => { + it("includes diff content with null oldText on success", () => { + const result = toolResultFromPart("write", { filePath: "/new.ts", content: "hello" }, "Created", false) + expect(result.content).toHaveLength(2) + expect(result.content[1]).toEqual({ type: "diff", path: "/new.ts", oldText: null, newText: "hello" }) + }) + + it("skips diff on error", () => { + const result = toolResultFromPart("write", { filePath: "/new.ts", content: "hello" }, "Permission denied", true) + expect(result.content).toHaveLength(1) + }) + }) + + describe("patch/apply_patch", () => { + it("includes patch text content on success", () => { + const patch = "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n+new" + const result = toolResultFromPart("patch", { filePath: "/file", diff: patch }, "Patched", false) + expect(result.content).toHaveLength(2) + expect(result.content[1]).toEqual({ type: "content", content: { type: "text", text: patch } }) + }) + + it("skips patch content on error", () => { + const result = toolResultFromPart("apply_patch", { filePath: "/file", diff: "bad" }, "Failed", true) + expect(result.content).toHaveLength(1) + }) + }) + + describe("default", () => { + it("returns stdout for success", () => { + const result = toolResultFromPart("unknown", {}, "some output", false) + expect(result.rawOutput).toEqual({ stdout: "some output" }) + expect(result.content).toHaveLength(1) + }) + + it("returns stderr for error", () => { + const result = toolResultFromPart("unknown", {}, "error msg", true) + expect(result.rawOutput).toEqual({ stderr: "error msg" }) + }) + + it("wraps error output in markdown fence", () => { + const result = toolResultFromPart("unknown", {}, "some error", true) + const text = (result.content[0] as any).content.text + expect(text).toContain("```") + expect(text).toContain("some error") + }) + }) +}) From cbbde52d0103876631c45fb7a59a15f907cb4394 Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Thu, 23 Apr 2026 15:57:43 +0300 Subject: [PATCH 4/8] refactor(acp): drop parse-command module, consolidate tool-format helpers - Remove parse-command.ts in favor of logic consolidated in tool-format.ts - Rewrite tool-format helpers (toolCallFromPart, toolResultFromPart, permissionDisplayInfo, fenceWith) as the single source of truth for tool call formatting - Adapt agent.ts to dev's post-namespace-refactor layout (flat exports with `export * as ACP` self-reexport, ProviderID/ModelID branded types, Hash.fast, InstallationVersion, ConfigMCP.Info) - Drop listSessions `unstable_` prefix to match SDK method name - Guard sendUsageUpdate against missing providerID/modelID --- packages/opencode/src/acp/agent.ts | 430 ++++++------------ packages/opencode/src/acp/parse-command.ts | 21 - packages/opencode/src/acp/tool-format.ts | 315 ++++++------- .../test/acp/event-subscription.test.ts | 105 ++++- .../opencode/test/acp/parse-command.test.ts | 32 -- .../opencode/test/acp/tool-format.test.ts | 212 +++++++-- 6 files changed, 587 insertions(+), 528 deletions(-) delete mode 100644 packages/opencode/src/acp/parse-command.ts delete mode 100644 packages/opencode/test/acp/parse-command.test.ts diff --git a/packages/opencode/src/acp/agent.ts b/packages/opencode/src/acp/agent.ts index 374677350a1d..9aa6248f16fd 100644 --- a/packages/opencode/src/acp/agent.ts +++ b/packages/opencode/src/acp/agent.ts @@ -21,13 +21,9 @@ import { type Role, type SessionInfo, type SetSessionModelRequest, - type SessionConfigOption, - type SetSessionConfigOptionRequest, - type SetSessionConfigOptionResponse, type SetSessionModeRequest, type SetSessionModeResponse, type ToolCallContent, - type ToolKind, type Usage, } from "@agentclientprotocol/sdk" @@ -38,20 +34,20 @@ import { Hash } from "@opencode-ai/shared/util/hash" import { ACPSessionManager } from "./session" import type { ACPConfig } from "./types" import { Provider } from "../provider" -import { ModelID, ProviderID } from "../provider/schema" import { Agent as AgentModule } from "../agent/agent" -import { AppRuntime } from "@/effect/app-runtime" import { Installation } from "@/installation" import { MessageV2 } from "@/session/message-v2" import { Config } from "@/config" -import { ConfigMCP } from "@/config/mcp" import { Todo } from "@/session/todo" import { z } from "zod" import { LoadAPIKeyError } from "ai" import type { AssistantMessage, Event, OpencodeClient, SessionMessageResponse, ToolPart } from "@opencode-ai/sdk/v2" import { applyPatch } from "diff" import { InstallationVersion } from "@/installation/version" -import { toolCallFromPart, toolResultFromPart } from "./tool-format" +import { AppRuntime } from "@/effect/app-runtime" +import { ConfigMCP } from "@/config/mcp" +import { ModelID, ProviderID } from "../provider/schema" +import { fenceWith, permissionDisplayInfo, toolCallFromPart, toolResultFromPart } from "./tool-format" type ModeOption = { id: string; name: string; description?: string } type ModelOption = { modelId: string; name: string } @@ -144,9 +140,43 @@ export class Agent implements ACPAgent { private sessionManager: ACPSessionManager private eventAbort = new AbortController() private eventStarted = false - private bashSnapshots = new Map() - private toolStarts = new Set() + private bashSnapshots = new Map>() + private toolStarts = new Map>() private permissionQueues = new Map>() + + private markToolStarted(sessionID: string, callID: string): boolean { + let set = this.toolStarts.get(sessionID) + if (set?.has(callID)) return true + if (!set) { + set = new Set() + this.toolStarts.set(sessionID, set) + } + set.add(callID) + return false + } + + private clearToolCall(sessionID: string, callID: string) { + this.toolStarts.get(sessionID)?.delete(callID) + this.bashSnapshots.get(sessionID)?.delete(callID) + } + + private getBashSnapshot(sessionID: string, callID: string) { + return this.bashSnapshots.get(sessionID)?.get(callID) + } + + private setBashSnapshot(sessionID: string, callID: string, hash: string) { + let map = this.bashSnapshots.get(sessionID) + if (!map) { + map = new Map() + this.bashSnapshots.set(sessionID, map) + } + map.set(callID, hash) + } + + private clearSession(sessionID: string) { + this.toolStarts.delete(sessionID) + this.bashSnapshots.delete(sessionID) + } private permissionOptions: PermissionOption[] = [ { optionId: "once", kind: "allow_once", name: "Allow once" }, { optionId: "always", kind: "allow_always", name: "Always allow" }, @@ -178,7 +208,7 @@ export class Agent implements ACPAgent { }) for await (const event of events.stream) { if (this.eventAbort.signal.aborted) return - const payload = event?.payload + const payload = (event as any)?.payload if (!payload) continue await this.handleEvent(payload as Event).catch((error) => { log.error("failed to handle event", { error, type: payload.type }) @@ -199,7 +229,7 @@ export class Agent implements ACPAgent { .then(async () => { const directory = session.cwd - const permissionInfo = toolCallFromPart(permission.permission, permission.metadata ?? {}) + const permissionInfo = permissionDisplayInfo(permission.permission, permission.metadata ?? {}, session.cwd) const res = await this.connection .requestPermission({ sessionId: permission.sessionID, @@ -245,7 +275,7 @@ export class Agent implements ACPAgent { const newContent = getNewContent(content, diff) if (newContent) { - void this.connection.writeTextFile({ + this.connection.writeTextFile({ sessionId: session.id, path: filepath, content: newContent, @@ -280,48 +310,36 @@ export class Agent implements ACPAgent { const sessionId = session.id if (part.type === "tool") { - await this.toolStart(sessionId, part) - const info = toolCallFromPart(part.tool, part.state.input) + await this.toolStart(sessionId, part, session.cwd) - switch (part.state.status) { - case "pending": - this.bashSnapshots.delete(part.callID) - return + if (part.state.status === "pending") { + this.bashSnapshots.get(sessionId)?.delete(part.callID) + return + } + + const info = toolCallFromPart(part.tool, part.state.input, session.cwd) + const title = ("title" in part.state && part.state.title) || info.title + switch (part.state.status) { case "running": { const output = this.bashOutput(part) const content: ToolCallContent[] = [] if (output) { - const hash = Hash.fast(output) + let changed = true if (part.tool === "bash") { - if (this.bashSnapshots.get(part.callID) === hash) { - await this.connection - .sessionUpdate({ - sessionId, - update: { - sessionUpdate: "tool_call_update", - toolCallId: part.callID, - status: "in_progress", - kind: info.kind, - title: info.title, - locations: info.locations, - rawInput: info.rawInput, - }, - }) - .catch((error) => { - log.error("failed to send tool in_progress to ACP", { error }) - }) - return - } - this.bashSnapshots.set(part.callID, hash) + const hash = Hash.fast(output) + changed = this.getBashSnapshot(sessionId, part.callID) !== hash + this.setBashSnapshot(sessionId, part.callID, hash) + } + if (changed) { + content.push({ + type: "content", + content: { + type: "text", + text: part.tool === "bash" ? fenceWith(output, "sh") : output, + }, + }) } - content.push({ - type: "content", - content: { - type: "text", - text: output, - }, - }) } await this.connection .sessionUpdate({ @@ -331,7 +349,7 @@ export class Agent implements ACPAgent { toolCallId: part.callID, status: "in_progress", kind: info.kind, - title: info.title, + title, locations: info.locations, rawInput: info.rawInput, ...(content.length > 0 && { content }), @@ -344,9 +362,8 @@ export class Agent implements ACPAgent { } case "completed": { - this.toolStarts.delete(part.callID) - this.bashSnapshots.delete(part.callID) - const result = toolResultFromPart(part.tool, part.state.input, part.state.output, false) + this.clearToolCall(sessionId, part.callID) + const result = toolResultFromPart(part.tool, part.state.input, part.state.output, false, session.cwd) if (part.tool === "todowrite") { const parsedTodos = z.array(Todo.Info).safeParse(JSON.parse(part.state.output)) @@ -382,9 +399,12 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "completed", + title, + kind: info.kind, + locations: info.locations, + rawInput: info.rawInput, content: result.content, rawOutput: result.rawOutput, - ...(result.title ? { title: result.title } : {}), }, }) .catch((error) => { @@ -393,9 +413,8 @@ export class Agent implements ACPAgent { return } case "error": { - this.toolStarts.delete(part.callID) - this.bashSnapshots.delete(part.callID) - const result = toolResultFromPart(part.tool, part.state.input, part.state.error, true) + this.clearToolCall(sessionId, part.callID) + const result = toolResultFromPart(part.tool, part.state.input, part.state.error, true, session.cwd) await this.connection .sessionUpdate({ @@ -404,9 +423,12 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "failed", + title, + kind: info.kind, + locations: info.locations, + rawInput: info.rawInput, content: result.content, rawOutput: result.rawOutput, - ...(result.title ? { title: result.title } : {}), }, }) .catch((error) => { @@ -416,12 +438,6 @@ export class Agent implements ACPAgent { } } } - - // ACP clients already know the prompt they just submitted, so replaying - // live user parts duplicates the message. We still replay user history in - // loadSession() and forkSession() via processMessage(). - if (part.type !== "text" && part.type !== "file") return - return } @@ -457,7 +473,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_message_chunk", - messageId: props.messageID, content: { type: "text", text: props.delta, @@ -476,7 +491,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_thought_chunk", - messageId: props.messageID, content: { type: "text", text: props.delta, @@ -561,7 +575,6 @@ export class Agent implements ACPAgent { return { sessionId, - configOptions: load.configOptions, models: load.models, modes: load.modes, _meta: load._meta, @@ -621,11 +634,6 @@ export class Agent implements ACPAgent { result.modes.currentModeId = lastUser.agent this.sessionManager.setMode(sessionId, lastUser.agent) } - result.configOptions = buildConfigOptions({ - currentModelId: result.models.currentModelId, - availableModels: result.models.availableModels, - modes: result.modes, - }) } for (const msg of messages ?? []) { @@ -792,15 +800,22 @@ export class Agent implements ACPAgent { log.debug("process message", message) if (message.info.role !== "assistant" && message.info.role !== "user") return const sessionId = message.info.sessionID + const cwd = + (message.info.role === "assistant" ? message.info.path?.cwd : undefined) ?? + this.sessionManager.tryGet(sessionId)?.cwd ?? + process.cwd() for (const part of message.parts) { if (part.type === "tool") { - await this.toolStart(sessionId, part) + await this.toolStart(sessionId, part, cwd) + if (part.state.status === "pending") { + this.bashSnapshots.get(sessionId)?.delete(part.callID) + continue + } + const info = toolCallFromPart(part.tool, part.state.input, cwd) + const title = ("title" in part.state && part.state.title) || info.title switch (part.state.status) { - case "pending": - this.bashSnapshots.delete(part.callID) - break - case "running": + case "running": { const output = this.bashOutput(part) const runningContent: ToolCallContent[] = [] if (output) { @@ -808,7 +823,7 @@ export class Agent implements ACPAgent { type: "content", content: { type: "text", - text: output, + text: part.tool === "bash" ? fenceWith(output, "sh") : output, }, }) } @@ -819,10 +834,10 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "in_progress", - kind: toToolKind(part.tool), - title: part.tool, - locations: toLocations(part.tool, part.state.input), - rawInput: part.state.input, + kind: info.kind, + title, + locations: info.locations, + rawInput: info.rawInput, ...(runningContent.length > 0 && { content: runningContent }), }, }) @@ -830,37 +845,10 @@ export class Agent implements ACPAgent { log.error("failed to send tool in_progress to ACP", { error: err }) }) break - case "completed": - this.toolStarts.delete(part.callID) - this.bashSnapshots.delete(part.callID) - const kind = toToolKind(part.tool) - const content: ToolCallContent[] = [ - { - type: "content", - content: { - type: "text", - text: part.state.output, - }, - }, - ] - - if (kind === "edit") { - const input = part.state.input - const filePath = typeof input["filePath"] === "string" ? input["filePath"] : "" - const oldText = typeof input["oldString"] === "string" ? input["oldString"] : "" - const newText = - typeof input["newString"] === "string" - ? input["newString"] - : typeof input["content"] === "string" - ? input["content"] - : "" - content.push({ - type: "diff", - path: filePath, - oldText, - newText, - }) - } + } + case "completed": { + this.clearToolCall(sessionId, part.callID) + const result = toolResultFromPart(part.tool, part.state.input, part.state.output, false, cwd) if (part.tool === "todowrite") { const parsedTodos = z.array(Todo.Info).safeParse(JSON.parse(part.state.output)) @@ -896,23 +884,23 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "completed", - kind, - content, - title: part.state.title, - rawInput: part.state.input, - rawOutput: { - output: part.state.output, - metadata: part.state.metadata, - }, + title, + kind: info.kind, + locations: info.locations, + rawInput: info.rawInput, + content: result.content, + rawOutput: result.rawOutput, }, }) .catch((err) => { log.error("failed to send tool completed to ACP", { error: err }) }) break - case "error": - this.toolStarts.delete(part.callID) - this.bashSnapshots.delete(part.callID) + } + case "error": { + this.clearToolCall(sessionId, part.callID) + const result = toolResultFromPart(part.tool, part.state.input, part.state.error, true, cwd) + await this.connection .sessionUpdate({ sessionId, @@ -920,28 +908,19 @@ export class Agent implements ACPAgent { sessionUpdate: "tool_call_update", toolCallId: part.callID, status: "failed", - kind: toToolKind(part.tool), - title: part.tool, - rawInput: part.state.input, - content: [ - { - type: "content", - content: { - type: "text", - text: part.state.error, - }, - }, - ], - rawOutput: { - error: part.state.error, - metadata: part.state.metadata, - }, + title, + kind: info.kind, + locations: info.locations, + rawInput: info.rawInput, + content: result.content, + rawOutput: result.rawOutput, }, }) .catch((err) => { log.error("failed to send tool error to ACP", { error: err }) }) break + } } } else if (part.type === "text") { if (part.text) { @@ -951,7 +930,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: message.info.role === "user" ? "user_message_chunk" : "agent_message_chunk", - messageId: message.info.id, content: { type: "text", text: part.text, @@ -983,7 +961,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, - messageId: message.info.id, content: { type: "resource_link", uri: url, name: filename, mimeType: mime }, }, }) @@ -1005,7 +982,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, - messageId: message.info.id, content: { type: "image", mimeType: effectiveMime, @@ -1034,7 +1010,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, - messageId: message.info.id, content: { type: "resource", resource }, }, }) @@ -1051,7 +1026,6 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_thought_chunk", - messageId: message.info.id, content: { type: "text", text: part.text, @@ -1074,20 +1048,20 @@ export class Agent implements ACPAgent { return output } - private async toolStart(sessionId: string, part: ToolPart) { - if (this.toolStarts.has(part.callID)) return - this.toolStarts.add(part.callID) + private async toolStart(sessionId: string, part: ToolPart, cwd: string) { + if (this.markToolStarted(sessionId, part.callID)) return + const info = toolCallFromPart(part.tool, part.state.input, cwd) await this.connection .sessionUpdate({ sessionId, update: { sessionUpdate: "tool_call", toolCallId: part.callID, - title: part.tool, - kind: toToolKind(part.tool), + title: info.title, + kind: info.kind, status: "pending", - locations: [], - rawInput: {}, + locations: info.locations, + rawInput: info.rawInput, }, }) .catch((error) => { @@ -1124,7 +1098,8 @@ export class Agent implements ACPAgent { (await (async () => { if (!availableModes.length) return undefined const defaultAgentName = await AppRuntime.runPromise(AgentModule.Service.use((svc) => svc.defaultAgent())) - const resolvedModeId = availableModes.find((mode) => mode.name === defaultAgentName)?.id ?? availableModes[0].id + const resolvedModeId = + availableModes.find((mode) => mode.name === defaultAgentName)?.id ?? availableModes[0].id this.sessionManager.setMode(sessionId, resolvedModeId) return resolvedModeId })()) @@ -1215,7 +1190,7 @@ export class Agent implements ACPAgent { ) setTimeout(() => { - void this.connection.sessionUpdate({ + this.connection.sessionUpdate({ sessionId, update: { sessionUpdate: "available_commands_update", @@ -1231,11 +1206,6 @@ export class Agent implements ACPAgent { availableModels, }, modes, - configOptions: buildConfigOptions({ - currentModelId: formatModelIdWithVariant(model, currentVariant, availableVariants, true), - availableModels, - modes, - }), _meta: buildVariantMeta({ model, variant: this.sessionManager.getVariant(sessionId), @@ -1275,44 +1245,6 @@ export class Agent implements ACPAgent { this.sessionManager.setMode(params.sessionId, params.modeId) } - async setSessionConfigOption(params: SetSessionConfigOptionRequest): Promise { - const session = this.sessionManager.get(params.sessionId) - const providers = await this.sdk.config - .providers({ directory: session.cwd }, { throwOnError: true }) - .then((x) => x.data!.providers) - const entries = sortProvidersByName(providers) - - if (params.configId === "model") { - if (typeof params.value !== "string") throw RequestError.invalidParams("model value must be a string") - const selection = parseModelSelection(params.value, providers) - this.sessionManager.setModel(session.id, selection.model) - this.sessionManager.setVariant(session.id, selection.variant) - } else if (params.configId === "mode") { - if (typeof params.value !== "string") throw RequestError.invalidParams("mode value must be a string") - const availableModes = await this.loadAvailableModes(session.cwd) - if (!availableModes.some((mode) => mode.id === params.value)) { - throw RequestError.invalidParams(JSON.stringify({ error: `Mode not found: ${params.value}` })) - } - this.sessionManager.setMode(session.id, params.value) - } else { - throw RequestError.invalidParams(JSON.stringify({ error: `Unknown config option: ${params.configId}` })) - } - - const updatedSession = this.sessionManager.get(session.id) - const model = updatedSession.model ?? (await defaultModel(this.config, session.cwd)) - const availableVariants = modelVariantsFromProviders(entries, model) - const currentModelId = formatModelIdWithVariant(model, updatedSession.variant, availableVariants, true) - const availableModels = buildAvailableModels(entries, { includeVariants: true }) - const modeState = await this.resolveModeState(session.cwd, session.id) - const modes = modeState.currentModeId - ? { availableModes: modeState.availableModes, currentModeId: modeState.currentModeId } - : undefined - - return { - configOptions: buildConfigOptions({ currentModelId, availableModels, modes }), - } - } - async prompt(params: PromptRequest) { const sessionID = params.sessionId const session = this.sessionManager.get(sessionID) @@ -1432,8 +1364,8 @@ export class Agent implements ACPAgent { const response = await this.sdk.session.prompt({ sessionID, model: { - providerID: model.providerID, - modelID: model.modelID, + providerID: ProviderID.make(model.providerID), + modelID: ModelID.make(model.modelID), }, variant: this.sessionManager.getVariant(sessionID), parts, @@ -1480,8 +1412,8 @@ export class Agent implements ACPAgent { { sessionID, directory, - providerID: model.providerID, - modelID: model.modelID, + providerID: ProviderID.make(model.providerID), + modelID: ModelID.make(model.modelID), }, { throwOnError: true }, ) @@ -1498,57 +1430,17 @@ export class Agent implements ACPAgent { async cancel(params: CancelNotification) { const session = this.sessionManager.get(params.sessionId) - await this.config.sdk.session.abort( - { - sessionID: params.sessionId, - directory: session.cwd, - }, - { throwOnError: true }, - ) - } -} - -function toToolKind(toolName: string): ToolKind { - const tool = toolName.toLocaleLowerCase() - switch (tool) { - case "bash": - return "execute" - case "webfetch": - return "fetch" - - case "edit": - case "patch": - case "write": - return "edit" - - case "grep": - case "glob": - case "context7_resolve_library_id": - case "context7_get_library_docs": - return "search" - - case "read": - return "read" - - default: - return "other" - } -} - -function toLocations(toolName: string, input: Record): { path: string }[] { - const tool = toolName.toLocaleLowerCase() - switch (tool) { - case "read": - case "edit": - case "write": - return input["filePath"] ? [{ path: input["filePath"] }] : [] - case "glob": - case "grep": - return input["path"] ? [{ path: input["path"] }] : [] - case "bash": - return [] - default: - return [] + try { + await this.config.sdk.session.abort( + { + sessionID: params.sessionId, + directory: session.cwd, + }, + { throwOnError: true }, + ) + } finally { + this.clearSession(params.sessionId) + } } } @@ -1564,7 +1456,11 @@ async function defaultModel(config: ACPConfig, cwd?: string): Promise<{ provider .then((resp) => { const cfg = resp.data if (!cfg || !cfg.model) return undefined - return Provider.parseModel(cfg.model) + const parsed = Provider.parseModel(cfg.model) + return { + providerID: ProviderID.make(parsed.providerID), + modelID: ModelID.make(parsed.modelID), + } }) .catch((error) => { log.error("failed to load user config for default model", { error }) @@ -1764,36 +1660,4 @@ function parseModelSelection( return { model: parsed, variant: undefined } } -function buildConfigOptions(input: { - currentModelId: string - availableModels: ModelOption[] - modes?: { availableModes: ModeOption[]; currentModeId: string } | undefined -}): SessionConfigOption[] { - const options: SessionConfigOption[] = [ - { - id: "model", - name: "Model", - category: "model", - type: "select", - currentValue: input.currentModelId, - options: input.availableModels.map((m) => ({ value: m.modelId, name: m.name })), - }, - ] - if (input.modes) { - options.push({ - id: "mode", - name: "Session Mode", - category: "mode", - type: "select", - currentValue: input.modes.currentModeId, - options: input.modes.availableModes.map((m) => ({ - value: m.id, - name: m.name, - ...(m.description ? { description: m.description } : {}), - })), - }) - } - return options -} - export * as ACP from "./agent" diff --git a/packages/opencode/src/acp/parse-command.ts b/packages/opencode/src/acp/parse-command.ts deleted file mode 100644 index 433cd09d9570..000000000000 --- a/packages/opencode/src/acp/parse-command.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { ToolKind } from "@agentclientprotocol/sdk" - -export namespace ParseCommand { - export interface Result { - kind: ToolKind - title: string - locations: { path: string }[] - terminalOutput: boolean - } - - export function format(command: string, description: string, cwd: string): Result { - const title = description || command || "Terminal" - - return { - kind: "other", - title, - locations: cwd ? [{ path: cwd }] : [], - terminalOutput: true, - } - } -} diff --git a/packages/opencode/src/acp/tool-format.ts b/packages/opencode/src/acp/tool-format.ts index 5a89e569c40c..223ae0c6361a 100644 --- a/packages/opencode/src/acp/tool-format.ts +++ b/packages/opencode/src/acp/tool-format.ts @@ -1,5 +1,5 @@ +import { resolve } from "path" import type { ToolCallContent, ToolKind } from "@agentclientprotocol/sdk" -import { ParseCommand } from "./parse-command" export interface ToolCallInfo { title: string @@ -12,9 +12,10 @@ export interface ToolCallInfo { export interface ToolResultInfo { content: ToolCallContent[] rawOutput: unknown - title?: string } +const FENCE_RE = /^`{3,}/gm + function normalize(name: string): string { return name.toLowerCase().replace(/^mcp__acp__/, "") } @@ -23,12 +24,14 @@ function truncate(str: string, max: number): string { return str.length > max ? str.substring(0, max - 3) + "..." : str } -function markdownEscape(text: string): string { +export function fenceWith(text: string, lang: string): string { + if (!text) return text let fence = "```" - for (const match of text.matchAll(/^`{3,}/gm)) { + for (const match of text.matchAll(FENCE_RE)) { while (match[0].length >= fence.length) fence += "`" } - return fence + "\n" + text + (text.endsWith("\n") ? "" : "\n") + fence + const trimmed = text.replace(/\n+$/, "") + return `${fence}${lang}\n${trimmed}\n${fence}` } function textContent(text: string): ToolCallContent { @@ -47,82 +50,30 @@ function num(v: unknown): number | undefined { return typeof v === "number" ? v : undefined } -function getFilePath(input: Record): string { - return str(input.filePath ?? input.file_path ?? input.filepath ?? input.path) -} - -function getOldString(input: Record): string { - return str(input.oldString ?? input.old_string) -} - -function getNewString(input: Record): string { - return str(input.newString ?? input.new_string ?? input.content ?? input.new_content) -} - -function getCommand(input: Record): string { - return str(input.command ?? input.cmd) -} - -function getDescription(input: Record): string { - return str(input.description ?? input.desc) -} - -function getPattern(input: Record): string { - return str(input.pattern ?? input.filePattern ?? input.glob) -} - -function getQuery(input: Record): string { - return str(input.query ?? input.q) +function abs(p: string, cwd: string): string { + return p && !p.startsWith("/") ? resolve(cwd, p) : p } -function getUrl(input: Record): string { - return str(input.url ?? input.uri) -} - -function getDiff(input: Record): string { - return str(input.diff ?? input.patch ?? input.unifiedDiff) -} - -import { resolve } from "path" - -function abs(p: string): string { - return p && !p.startsWith("/") ? resolve(p) : p -} - -export function toolCallFromPart(tool: string, input: Record): ToolCallInfo { +export function toolCallFromPart(tool: string, input: Record, cwd: string): ToolCallInfo { const name = normalize(tool) switch (name) { - case "bash": - case "shell": - case "terminal": { - const command = getCommand(input) - const description = getDescription(input) - const cwd = str(input.cwd ?? input.workdir ?? input.workingDir ?? input.directory) - const result = ParseCommand.format(command, description, cwd) + case "bash": { + const command = str(input.command) + const description = str(input.description) + const workdir = str(input.workdir) return { - title: result.title, - kind: result.kind, - content: [], - locations: result.locations, - rawInput: input, - } - } - - case "bashoutput": { - return { - title: "Tail Logs", - kind: "execute", + title: description || command || "Terminal", + kind: "other", content: [], - locations: [], + locations: workdir ? [{ path: workdir }] : [], rawInput: input, } } - case "read": - case "view": { - const filePath = getFilePath(input) - const offset = num(input.offset) ?? num(input.line) ?? 0 + case "read": { + const filePath = str(input.filePath) + const offset = num(input.offset) ?? 0 const limit = num(input.limit) ?? 0 let suffix = "" if (limit) { @@ -134,66 +85,50 @@ export function toolCallFromPart(tool: string, input: Record): title: filePath ? `Read ${filePath}${suffix}` : "Read File", kind: "read", content: [], - locations: filePath ? [{ path: abs(filePath), ...(offset ? { line: offset + 1 } : {}) }] : [], + locations: filePath ? [{ path: abs(filePath, cwd), ...(offset ? { line: offset + 1 } : {}) }] : [], rawInput: input, } } - case "list": - case "ls": { + case "list": { const path = str(input.path) return { title: path ? `List \`${path}\`` : "List directory", kind: "read", content: [], - locations: path ? [{ path: abs(path) }] : [], + locations: path ? [{ path: abs(path, cwd) }] : [], rawInput: input, } } - case "edit": - case "str_replace": { - const filePath = getFilePath(input) - const oldString = getOldString(input) - const newString = getNewString(input) + case "edit": { + const filePath = str(input.filePath) + const oldString = str(input.oldString) + const newString = str(input.newString) return { title: filePath ? `Edit \`${filePath}\`` : "Edit", kind: "edit", - content: filePath ? [diffContent(abs(filePath), oldString, newString)] : [], - locations: filePath ? [{ path: abs(filePath) }] : [], + content: filePath ? [diffContent(abs(filePath, cwd), oldString, newString)] : [], + locations: filePath ? [{ path: abs(filePath, cwd) }] : [], rawInput: input, } } - case "patch": { - const filePath = getFilePath(input) - const patchText = getDiff(input) - return { - title: filePath ? `Patch \`${filePath}\`` : "Patch", - kind: "edit", - content: patchText ? [textContent(patchText)] : [], - locations: filePath ? [{ path: abs(filePath) }] : [], - rawInput: input, - } - } - - case "write": - case "create": { - const filePath = getFilePath(input) - const content = getNewString(input) + case "write": { + const filePath = str(input.filePath) + const content = str(input.content) return { title: filePath ? `Write ${filePath}` : "Write", kind: "edit", - content: filePath ? [diffContent(abs(filePath), null, content)] : [], - locations: filePath ? [{ path: abs(filePath) }] : [], + content: filePath ? [diffContent(abs(filePath, cwd), null, content)] : [], + locations: filePath ? [{ path: abs(filePath, cwd) }] : [], rawInput: input, } } - case "glob": - case "find": { + case "glob": { const path = str(input.path) - const pattern = getPattern(input) + const pattern = str(input.pattern) let label = "Find" if (path) label += ` \`${path}\`` if (pattern) label += ` \`${pattern}\`` @@ -201,14 +136,13 @@ export function toolCallFromPart(tool: string, input: Record): title: label, kind: "search", content: [], - locations: path ? [{ path: abs(path) }] : [], + locations: path ? [{ path: abs(path, cwd) }] : [], rawInput: input, } } - case "grep": - case "search": { - const pattern = getPattern(input) + case "grep": { + const pattern = str(input.pattern) const path = str(input.path) let label = "grep" if (pattern) label += ` "${truncate(pattern, 30)}"` @@ -217,14 +151,13 @@ export function toolCallFromPart(tool: string, input: Record): title: label, kind: "search", content: [], - locations: path ? [{ path: abs(path) }] : [], + locations: path ? [{ path: abs(path, cwd) }] : [], rawInput: input, } } - case "webfetch": - case "fetch": { - const url = getUrl(input) + case "webfetch": { + const url = str(input.url) const prompt = str(input.prompt) return { title: url ? `Fetch ${truncate(url, 40)}` : "Fetch", @@ -236,7 +169,7 @@ export function toolCallFromPart(tool: string, input: Record): } case "websearch": { - const query = getQuery(input) + const query = str(input.query) return { title: query ? `"${truncate(query, 40)}"` : "Search", kind: "fetch", @@ -247,7 +180,7 @@ export function toolCallFromPart(tool: string, input: Record): } case "task": { - const description = getDescription(input) + const description = str(input.description) const prompt = str(input.prompt) return { title: description || "Task", @@ -290,13 +223,12 @@ export function toolCallFromPart(tool: string, input: Record): } case "apply_patch": { - const filePath = getFilePath(input) - const patchText = getDiff(input) + const patchText = str(input.patchText) return { - title: filePath ? `Apply Patch \`${filePath}\`` : "Apply Patch", + title: "Apply Patch", kind: "edit", content: patchText ? [textContent(patchText)] : [], - locations: filePath ? [{ path: abs(filePath) }] : [], + locations: [], rawInput: input, } } @@ -322,9 +254,9 @@ export function toolCallFromPart(tool: string, input: Record): } case "skill": { - const name = str(input.name) + const skillName = str(input.name) return { - title: name ? `Skill: ${name}` : "Skill", + title: skillName ? `Skill: ${skillName}` : "Skill", kind: "other", content: [], locations: [], @@ -333,7 +265,7 @@ export function toolCallFromPart(tool: string, input: Record): } case "question": { - const question = getQuery(input) || str(input.question) + const question = str(input.question) || str(input.query) return { title: question ? truncate(question, 40) : "Question", kind: "other", @@ -354,7 +286,7 @@ export function toolCallFromPart(tool: string, input: Record): } case "codesearch": { - const query = getQuery(input) + const query = str(input.query) return { title: query ? `Search: ${truncate(query, 30)}` : "Code Search", kind: "search", @@ -365,8 +297,8 @@ export function toolCallFromPart(tool: string, input: Record): } default: { - const description = getDescription(input) - const command = getCommand(input) + const description = str(input.description) + const command = str(input.command) const title = description || command || tool return { title: truncate(title, 50), @@ -384,65 +316,144 @@ export function toolResultFromPart( input: Record, output: string, isError: boolean, + cwd: string, ): ToolResultInfo { const name = normalize(tool) - const displayText = isError ? markdownEscape(output) : output - const content: ToolCallContent[] = [textContent(displayText)] + + if (name === "bash") { + const text = fenceWith(output, isError ? "" : "sh") + return { + content: [textContent(text)], + rawOutput: isError ? { stderr: output } : { stdout: output }, + } + } + + const content: ToolCallContent[] = [textContent(fenceWith(output, ""))] switch (name) { - case "bash": - case "shell": - case "terminal": { + case "edit": { + const filePath = str(input.filePath) + const oldString = str(input.oldString) + const newString = str(input.newString) + if (filePath && !isError) { + content.push(diffContent(abs(filePath, cwd), oldString, newString)) + } return { content, rawOutput: isError ? { stderr: output } : { stdout: output }, } } - case "edit": - case "str_replace": { - const filePath = getFilePath(input) - const oldString = getOldString(input) - const newString = getNewString(input) + case "apply_patch": { + const patchText = str(input.patchText) + if (isError) { + return { content, rawOutput: { stderr: output } } + } + const successContent: ToolCallContent[] = patchText + ? [textContent(fenceWith(patchText, "diff"))] + : [] + return { content: successContent, rawOutput: { stdout: output } } + } + + case "write": { + const filePath = str(input.filePath) + const fileContent = str(input.content) if (filePath && !isError) { - content.push(diffContent(abs(filePath), oldString, newString)) + content.push(diffContent(abs(filePath, cwd), null, fileContent)) } return { content, - rawOutput: { stdout: output }, + rawOutput: isError ? { stderr: output } : { stdout: output }, } } - case "patch": - case "apply_patch": { - const filePath = getFilePath(input) - const patchText = getDiff(input) - if (filePath && patchText && !isError) { - content.push(textContent(patchText)) - } + default: { return { content, - rawOutput: { stdout: output }, + rawOutput: isError ? { stderr: output } : { stdout: output }, } } + } +} +export function permissionDisplayInfo( + permission: string, + metadata: Record, + cwd: string, +): ToolCallInfo { + const name = normalize(permission) + switch (name) { + case "edit": case "write": - case "create": { - const filePath = getFilePath(input) - const fileContent = getNewString(input) - if (filePath && !isError) { - content.push(diffContent(abs(filePath), null, fileContent)) + case "apply_patch": { + const filepath = str(metadata.filepath) + const diff = str(metadata.diff) + return { + title: filepath ? `Edit ${filepath}` : "Edit", + kind: "edit", + content: diff ? [textContent(diff)] : [], + locations: filepath ? [{ path: abs(filepath, cwd) }] : [], + rawInput: metadata, } + } + case "bash": { + const command = str(metadata.command) + const description = str(metadata.description) return { - content, - rawOutput: { stdout: output }, + title: description || command || "Terminal", + kind: "execute", + content: [], + locations: [], + rawInput: metadata, + } + } + case "webfetch": { + const url = str(metadata.url) + return { + title: url ? `Fetch ${truncate(url, 40)}` : "Fetch", + kind: "fetch", + content: [], + locations: [], + rawInput: metadata, + } + } + case "websearch": { + const query = str(metadata.query) + return { + title: query ? `"${truncate(query, 40)}"` : "Search", + kind: "fetch", + content: [], + locations: [], + rawInput: metadata, + } + } + case "task": { + const description = str(metadata.description) + return { + title: description || "Task", + kind: "think", + content: [], + locations: [], + rawInput: metadata, + } + } + case "skill": { + const skillName = str(metadata.name) + return { + title: skillName ? `Skill: ${skillName}` : "Skill", + kind: "other", + content: [], + locations: [], + rawInput: metadata, } } - default: { return { - content, - rawOutput: isError ? { stderr: output } : { stdout: output }, + title: permission || "Permission", + kind: "other", + content: [], + locations: [], + rawInput: metadata, } } } diff --git a/packages/opencode/test/acp/event-subscription.test.ts b/packages/opencode/test/acp/event-subscription.test.ts index bce5e94598cf..74a49994607b 100644 --- a/packages/opencode/test/acp/event-subscription.test.ts +++ b/packages/opencode/test/acp/event-subscription.test.ts @@ -563,7 +563,7 @@ describe("acp.agent event subscription", () => { .filter((u) => isToolCallUpdate(u.update)) .map((u) => inProgressText(u.update)) - expect(snapshots).toEqual(["a", undefined, "ab"]) + expect(snapshots).toEqual(["```sh\na\n```", undefined, "```sh\nab\n```"]) stop() }, }) @@ -717,7 +717,108 @@ describe("acp.agent event subscription", () => { .filter((u) => isToolCallUpdate(u.update)) .map((u) => inProgressText(u.update)) - expect(snapshots).toEqual(["a", "a"]) + expect(snapshots).toEqual(["```sh\na\n```", "```sh\na\n```"]) + stop() + }, + }) + }) + + test("isolates bash snapshot state across concurrent sessions with the same callID", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, controller, sessionUpdates, stop } = createFakeAgent() + const cwd = "/tmp/opencode-acp-test" + const sessionA = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const sessionB = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const input = { command: "echo hi" } + + controller.push(toolEvent(sessionA, cwd, { callID: "same", tool: "bash", status: "running", input, metadata: { output: "a_out" } })) + controller.push(toolEvent(sessionB, cwd, { callID: "same", tool: "bash", status: "running", input, metadata: { output: "b_out" } })) + await new Promise((r) => setTimeout(r, 20)) + + const textsA = sessionUpdates + .filter((u) => u.sessionId === sessionA && isToolCallUpdate(u.update)) + .map((u) => inProgressText(u.update) ?? "") + .join("|") + const textsB = sessionUpdates + .filter((u) => u.sessionId === sessionB && isToolCallUpdate(u.update)) + .map((u) => inProgressText(u.update) ?? "") + .join("|") + expect(textsA).toContain("a_out") + expect(textsB).toContain("b_out") + expect(textsA).not.toContain("b_out") + expect(textsB).not.toContain("a_out") + stop() + }, + }) + }) + + test("running tool_call_update carries rawInput and title so clients persist them", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, controller, sessionUpdates, stop } = createFakeAgent() + const cwd = "/tmp/opencode-acp-test" + const sessionId = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const input = { command: "echo hi", description: "say hi" } + + controller.push( + toolEvent(sessionId, cwd, { + callID: "call_1", + tool: "bash", + status: "running", + input, + metadata: { output: "hi" }, + }), + ) + await new Promise((r) => setTimeout(r, 20)) + + const running = sessionUpdates + .filter((u) => u.sessionId === sessionId) + .map((u) => u.update) + .find((u) => u.sessionUpdate === "tool_call_update" && u.status === "in_progress") + expect(running).toBeDefined() + expect((running as any).title).toBe("say hi") + expect((running as any).kind).toBeDefined() + expect((running as any).rawInput).toEqual(input) + stop() + }, + }) + }) + + test("cancel() clears streaming state for the session", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, controller, stop, sdk } = createFakeAgent() + sdk.session.abort = async () => ({ data: true }) + const cwd = "/tmp/opencode-acp-test" + const sessionId = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + + controller.push( + toolEvent(sessionId, cwd, { + callID: "call_1", + tool: "bash", + status: "running", + input: { command: "loop" }, + metadata: { output: "line1" }, + }), + ) + await new Promise((r) => setTimeout(r, 20)) + + const bashMap: Map> = (agent as any).bashSnapshots + const toolMap: Map> = (agent as any).toolStarts + expect(bashMap.get(sessionId)?.has("call_1")).toBe(true) + expect(toolMap.get(sessionId)?.has("call_1")).toBe(true) + + await agent.cancel({ sessionId } as any) + + expect(bashMap.get(sessionId)).toBeUndefined() + expect(toolMap.get(sessionId)).toBeUndefined() stop() }, }) diff --git a/packages/opencode/test/acp/parse-command.test.ts b/packages/opencode/test/acp/parse-command.test.ts deleted file mode 100644 index 782cf0fac528..000000000000 --- a/packages/opencode/test/acp/parse-command.test.ts +++ /dev/null @@ -1,32 +0,0 @@ -import { describe, expect, it } from "bun:test" -import { ParseCommand } from "../../src/acp/parse-command" - -describe("ParseCommand", () => { - describe("format", () => { - it("uses description as title when provided", () => { - const result = ParseCommand.format("ls", "List files in current directory", "/home/user") - expect(result.title).toBe("List files in current directory") - expect(result.kind).toBe("other") - }) - - it("falls back to command when no description", () => { - const result = ParseCommand.format("ls -la", "", "/home/user") - expect(result.title).toBe("ls -la") - }) - - it("includes cwd in locations", () => { - const result = ParseCommand.format("ls", "List files", "/home/user") - expect(result.locations).toEqual([{ path: "/home/user" }]) - }) - - it("handles empty cwd", () => { - const result = ParseCommand.format("ls", "List files", "") - expect(result.locations).toEqual([]) - }) - - it("sets terminalOutput to true", () => { - const result = ParseCommand.format("npm install", "Install dependencies", "/home/user") - expect(result.terminalOutput).toBe(true) - }) - }) -}) diff --git a/packages/opencode/test/acp/tool-format.test.ts b/packages/opencode/test/acp/tool-format.test.ts index c1c3b0d3b9fc..cbb3832beb31 100644 --- a/packages/opencode/test/acp/tool-format.test.ts +++ b/packages/opencode/test/acp/tool-format.test.ts @@ -1,18 +1,33 @@ import { describe, expect, it } from "bun:test" -import { toolCallFromPart, toolResultFromPart } from "../../src/acp/tool-format" +import { + permissionDisplayInfo, + toolCallFromPart as _toolCallFromPart, + toolResultFromPart as _toolResultFromPart, +} from "../../src/acp/tool-format" + +const CWD = process.cwd() +const toolCallFromPart = (tool: string, input: Record, cwd: string = CWD) => + _toolCallFromPart(tool, input, cwd) +const toolResultFromPart = ( + tool: string, + input: Record, + output: string, + isError: boolean, + cwd: string = CWD, +) => _toolResultFromPart(tool, input, output, isError, cwd) describe("toolCallFromPart", () => { - describe("bash/shell/terminal", () => { - it("formats bash command with description", () => { - const result = toolCallFromPart("bash", { command: "ls -la", description: "List files", cwd: "/home" }) + describe("bash", () => { + it("formats bash command with description and workdir location", () => { + const result = toolCallFromPart("bash", { command: "ls -la", description: "List files", workdir: "/home" }) expect(result.title).toBe("List files") expect(result.kind).toBe("other") expect(result.locations).toEqual([{ path: "/home" }]) - expect(result.rawInput).toEqual({ command: "ls -la", description: "List files", cwd: "/home" }) + expect(result.rawInput).toEqual({ command: "ls -la", description: "List files", workdir: "/home" }) }) it("falls back to command when no description", () => { - const result = toolCallFromPart("shell", { command: "npm install" }) + const result = toolCallFromPart("bash", { command: "npm install" }) expect(result.title).toBe("npm install") }) @@ -22,15 +37,7 @@ describe("toolCallFromPart", () => { }) }) - describe("bashoutput", () => { - it("returns Tail Logs title", () => { - const result = toolCallFromPart("bashoutput", {}) - expect(result.title).toBe("Tail Logs") - expect(result.kind).toBe("execute") - }) - }) - - describe("read/view", () => { + describe("read", () => { it("formats read with file path", () => { const result = toolCallFromPart("read", { filePath: "/src/index.ts" }) expect(result.title).toBe("Read /src/index.ts") @@ -65,7 +72,7 @@ describe("toolCallFromPart", () => { }) }) - describe("edit/str_replace", () => { + describe("edit", () => { it("formats edit with diff content", () => { const result = toolCallFromPart("edit", { filePath: "/src/app.ts", @@ -79,13 +86,13 @@ describe("toolCallFromPart", () => { }) it("handles missing file path", () => { - const result = toolCallFromPart("str_replace", { oldString: "a", newString: "b" }) + const result = toolCallFromPart("edit", { oldString: "a", newString: "b" }) expect(result.title).toBe("Edit") expect(result.content).toEqual([]) }) }) - describe("write/create", () => { + describe("write", () => { it("formats write with diff content (null oldText)", () => { const result = toolCallFromPart("write", { filePath: "/new.ts", content: "hello" }) expect(result.title).toBe("Write /new.ts") @@ -94,7 +101,7 @@ describe("toolCallFromPart", () => { }) }) - describe("glob/find", () => { + describe("glob", () => { it("formats glob with path and pattern", () => { const result = toolCallFromPart("glob", { path: "/src", pattern: "*.ts" }) expect(result.title).toBe("Find `/src` `*.ts`") @@ -107,12 +114,12 @@ describe("toolCallFromPart", () => { }) it("handles no path or pattern", () => { - const result = toolCallFromPart("find", {}) + const result = toolCallFromPart("glob", {}) expect(result.title).toBe("Find") }) }) - describe("grep/search", () => { + describe("grep", () => { it("formats grep with pattern and path", () => { const result = toolCallFromPart("grep", { pattern: "TODO", path: "/src" }) expect(result.title).toBe('grep "TODO" /src') @@ -126,7 +133,7 @@ describe("toolCallFromPart", () => { }) }) - describe("webfetch/fetch", () => { + describe("webfetch", () => { it("formats fetch with url", () => { const result = toolCallFromPart("webfetch", { url: "https://example.com", prompt: "get title" }) expect(result.title).toBe("Fetch https://example.com") @@ -195,21 +202,24 @@ describe("toolCallFromPart", () => { }) describe("toolResultFromPart", () => { - describe("bash/shell/terminal", () => { - it("returns stdout for success", () => { + describe("bash", () => { + it("returns stdout for success wrapped in shell code fence", () => { const result = toolResultFromPart("bash", { command: "ls" }, "file1\nfile2", false) expect(result.rawOutput).toEqual({ stdout: "file1\nfile2" }) expect(result.content).toHaveLength(1) - expect(result.content[0]).toEqual({ type: "content", content: { type: "text", text: "file1\nfile2" } }) + expect(result.content[0]).toEqual({ + type: "content", + content: { type: "text", text: "```sh\nfile1\nfile2\n```" }, + }) }) it("returns stderr for error", () => { - const result = toolResultFromPart("shell", { command: "bad" }, "command not found", true) + const result = toolResultFromPart("bash", { command: "bad" }, "command not found", true) expect(result.rawOutput).toEqual({ stderr: "command not found" }) }) }) - describe("edit/str_replace", () => { + describe("edit", () => { it("includes diff content on success", () => { const result = toolResultFromPart( "edit", @@ -222,7 +232,7 @@ describe("toolResultFromPart", () => { expect(result.content[1]).toEqual({ type: "diff", path: "/src/app.ts", oldText: "foo", newText: "bar" }) }) - it("skips diff content on error", () => { + it("skips diff content on error and returns stderr", () => { const result = toolResultFromPart( "edit", { filePath: "/src/app.ts", oldString: "foo", newString: "bar" }, @@ -230,33 +240,49 @@ describe("toolResultFromPart", () => { true, ) expect(result.content).toHaveLength(1) + expect(result.rawOutput).toEqual({ stderr: "old_string not found" }) }) }) - describe("write/create", () => { + describe("write", () => { it("includes diff content with null oldText on success", () => { const result = toolResultFromPart("write", { filePath: "/new.ts", content: "hello" }, "Created", false) expect(result.content).toHaveLength(2) expect(result.content[1]).toEqual({ type: "diff", path: "/new.ts", oldText: null, newText: "hello" }) }) - it("skips diff on error", () => { + it("skips diff on error and returns stderr", () => { const result = toolResultFromPart("write", { filePath: "/new.ts", content: "hello" }, "Permission denied", true) expect(result.content).toHaveLength(1) + expect(result.rawOutput).toEqual({ stderr: "Permission denied" }) }) }) - describe("patch/apply_patch", () => { - it("includes patch text content on success", () => { - const patch = "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n+new" - const result = toolResultFromPart("patch", { filePath: "/file", diff: patch }, "Patched", false) - expect(result.content).toHaveLength(2) - expect(result.content[1]).toEqual({ type: "content", content: { type: "text", text: patch } }) + describe("apply_patch", () => { + it("returns only a diff-fenced patch block on success (title=output dedupes body)", () => { + const patchText = "--- a/file\n+++ b/file\n@@ -1 +1 @@\n-old\n+new" + const result = toolResultFromPart("apply_patch", { patchText }, "Patched", false) + expect(result.content).toHaveLength(1) + expect(result.content[0]).toEqual({ + type: "content", + content: { type: "text", text: "```diff\n" + patchText + "\n```" }, + }) + expect(result.rawOutput).toEqual({ stdout: "Patched" }) }) - it("skips patch content on error", () => { - const result = toolResultFromPart("apply_patch", { filePath: "/file", diff: "bad" }, "Failed", true) + it("returns empty content when patchText is absent on success", () => { + const result = toolResultFromPart("apply_patch", {}, "Patched", false) + expect(result.content).toHaveLength(0) + expect(result.rawOutput).toEqual({ stdout: "Patched" }) + }) + + it("keeps fenced error text and returns stderr on error", () => { + const result = toolResultFromPart("apply_patch", { patchText: "bad" }, "Failed", true) expect(result.content).toHaveLength(1) + const text = (result.content[0] as any).content.text + expect(text).toMatch(/^```\n/) + expect(text).toContain("Failed") + expect(result.rawOutput).toEqual({ stderr: "Failed" }) }) }) @@ -280,3 +306,113 @@ describe("toolResultFromPart", () => { }) }) }) + +describe("apply_patch canonical field", () => { + it("reads patchText from call input and emits content block", () => { + const patchText = "--- a/foo\n+++ b/foo\n@@ -1 +1 @@\n-x\n+y" + const call = toolCallFromPart("apply_patch", { patchText }) + expect(call.title).toBe("Apply Patch") + expect(call.kind).toBe("edit") + expect(call.content).toEqual([{ type: "content", content: { type: "text", text: patchText } }]) + }) + + it("produces empty content when patchText is absent", () => { + const call = toolCallFromPart("apply_patch", {}) + expect(call.content).toEqual([]) + }) +}) + +describe("path resolution against cwd", () => { + it("resolves relative filePath against the passed cwd (not process.cwd)", () => { + const result = toolCallFromPart("read", { filePath: "src/index.ts" }, "/workspace/project") + expect(result.locations).toEqual([{ path: "/workspace/project/src/index.ts" }]) + }) + + it("leaves absolute paths untouched", () => { + const result = toolCallFromPart("read", { filePath: "/abs/path.ts" }, "/workspace/project") + expect(result.locations).toEqual([{ path: "/abs/path.ts" }]) + }) + + it("resolves relative path in edit result content", () => { + const result = toolResultFromPart( + "edit", + { filePath: "rel/file.ts", oldString: "a", newString: "b" }, + "ok", + false, + "/workspace", + ) + expect(result.content[1]).toEqual({ + type: "diff", + path: "/workspace/rel/file.ts", + oldText: "a", + newText: "b", + }) + }) +}) + +describe("non-bash tool output fencing", () => { + it("fences opencode-style MCP tool output (exa_web_search_exa, no mcp__ prefix) to prevent markdown injection", () => { + const output = "# Title\nsome result\n## Section\ncontent" + const result = toolResultFromPart("exa_web_search_exa", {}, output, false) + const text = (result.content[0] as any).content.text + expect(text).toMatch(/^```\n/) + expect(text).toContain(output) + expect(text).toMatch(/```$/) + expect(result.rawOutput).toEqual({ stdout: output }) + }) + + it("fences all default-branch success output (read/list/grep/webfetch/todowrite/unknown) in a plain code block", () => { + for (const tool of ["read", "list", "grep", "webfetch", "todowrite", "codesearch", "unknownTool"]) { + const result = toolResultFromPart(tool, {}, "plain text", false) + const text = (result.content[0] as any).content.text + expect(text).toMatch(/^```\n/) + expect(text).toContain("plain text") + expect(text).toMatch(/\n```$/) + } + }) + + it("preserves widened fence when output contains triple-backticks", () => { + const output = "before\n```\nnested\n```\nafter" + const result = toolResultFromPart("unknown", {}, output, false) + const text = (result.content[0] as any).content.text + expect(text.startsWith("````\n")).toBe(true) + expect(text.endsWith("\n````")).toBe(true) + }) +}) + +describe("permissionDisplayInfo", () => { + it("formats edit permission from lowercase metadata keys", () => { + const info = permissionDisplayInfo( + "edit", + { filepath: "/abs/foo.ts", diff: "--- a\n+++ b\n" }, + CWD, + ) + expect(info.title).toBe("Edit /abs/foo.ts") + expect(info.kind).toBe("edit") + expect(info.locations).toEqual([{ path: "/abs/foo.ts" }]) + expect(info.content).toHaveLength(1) + }) + + it("formats bash permission with description fallback", () => { + const info = permissionDisplayInfo("bash", { command: "rm -rf /", description: "Nuke" }, CWD) + expect(info.title).toBe("Nuke") + expect(info.kind).toBe("execute") + }) + + it("formats webfetch permission", () => { + const info = permissionDisplayInfo("webfetch", { url: "https://example.com" }, CWD) + expect(info.title).toBe("Fetch https://example.com") + expect(info.kind).toBe("fetch") + }) + + it("falls back to permission name on unknown type", () => { + const info = permissionDisplayInfo("doom_loop", {}, CWD) + expect(info.title).toBe("doom_loop") + expect(info.kind).toBe("other") + }) + + it("resolves relative edit filepath against cwd", () => { + const info = permissionDisplayInfo("edit", { filepath: "rel/foo.ts" }, "/ws") + expect(info.locations).toEqual([{ path: "/ws/rel/foo.ts" }]) + }) +}) From 3b705a3f9ff46f5f769a33ab2098de45fe5eb2be Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Thu, 23 Apr 2026 22:20:46 +0300 Subject: [PATCH 5/8] refactor(acp): parse tool outputs, resolve paths, improve agent events --- packages/opencode/src/acp/agent.ts | 103 +++++- packages/opencode/src/acp/tool-format.ts | 207 +++++++++++- .../opencode/test/acp/tool-format.test.ts | 297 +++++++++++++++++- 3 files changed, 590 insertions(+), 17 deletions(-) diff --git a/packages/opencode/src/acp/agent.ts b/packages/opencode/src/acp/agent.ts index 9aa6248f16fd..809b940a921a 100644 --- a/packages/opencode/src/acp/agent.ts +++ b/packages/opencode/src/acp/agent.ts @@ -19,7 +19,10 @@ import { type ResumeSessionRequest, type ResumeSessionResponse, type Role, + type SessionConfigOption, type SessionInfo, + type SetSessionConfigOptionRequest, + type SetSessionConfigOptionResponse, type SetSessionModelRequest, type SetSessionModeRequest, type SetSessionModeResponse, @@ -208,7 +211,7 @@ export class Agent implements ACPAgent { }) for await (const event of events.stream) { if (this.eventAbort.signal.aborted) return - const payload = (event as any)?.payload + const payload = event?.payload if (!payload) continue await this.handleEvent(payload as Event).catch((error) => { log.error("failed to handle event", { error, type: payload.type }) @@ -275,7 +278,7 @@ export class Agent implements ACPAgent { const newContent = getNewContent(content, diff) if (newContent) { - this.connection.writeTextFile({ + void this.connection.writeTextFile({ sessionId: session.id, path: filepath, content: newContent, @@ -438,6 +441,12 @@ export class Agent implements ACPAgent { } } } + + // ACP clients already know the prompt they just submitted, so replaying + // live user parts duplicates the message. We still replay user history in + // loadSession() and forkSession() via processMessage(). + if (part.type !== "text" && part.type !== "file") return + return } @@ -473,6 +482,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_message_chunk", + messageId: props.messageID, content: { type: "text", text: props.delta, @@ -491,6 +501,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_thought_chunk", + messageId: props.messageID, content: { type: "text", text: props.delta, @@ -575,6 +586,7 @@ export class Agent implements ACPAgent { return { sessionId, + configOptions: load.configOptions, models: load.models, modes: load.modes, _meta: load._meta, @@ -634,6 +646,11 @@ export class Agent implements ACPAgent { result.modes.currentModeId = lastUser.agent this.sessionManager.setMode(sessionId, lastUser.agent) } + result.configOptions = buildConfigOptions({ + currentModelId: result.models.currentModelId, + availableModels: result.models.availableModels, + modes: result.modes, + }) } for (const msg of messages ?? []) { @@ -930,6 +947,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: message.info.role === "user" ? "user_message_chunk" : "agent_message_chunk", + messageId: message.info.id, content: { type: "text", text: part.text, @@ -961,6 +979,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, + messageId: message.info.id, content: { type: "resource_link", uri: url, name: filename, mimeType: mime }, }, }) @@ -982,6 +1001,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, + messageId: message.info.id, content: { type: "image", mimeType: effectiveMime, @@ -1010,6 +1030,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: messageChunk, + messageId: message.info.id, content: { type: "resource", resource }, }, }) @@ -1026,6 +1047,7 @@ export class Agent implements ACPAgent { sessionId, update: { sessionUpdate: "agent_thought_chunk", + messageId: message.info.id, content: { type: "text", text: part.text, @@ -1190,7 +1212,7 @@ export class Agent implements ACPAgent { ) setTimeout(() => { - this.connection.sessionUpdate({ + void this.connection.sessionUpdate({ sessionId, update: { sessionUpdate: "available_commands_update", @@ -1206,6 +1228,11 @@ export class Agent implements ACPAgent { availableModels, }, modes, + configOptions: buildConfigOptions({ + currentModelId: formatModelIdWithVariant(model, currentVariant, availableVariants, true), + availableModels, + modes, + }), _meta: buildVariantMeta({ model, variant: this.sessionManager.getVariant(sessionId), @@ -1245,6 +1272,44 @@ export class Agent implements ACPAgent { this.sessionManager.setMode(params.sessionId, params.modeId) } + async setSessionConfigOption(params: SetSessionConfigOptionRequest): Promise { + const session = this.sessionManager.get(params.sessionId) + const providers = await this.sdk.config + .providers({ directory: session.cwd }, { throwOnError: true }) + .then((x) => x.data!.providers) + const entries = sortProvidersByName(providers) + + if (params.configId === "model") { + if (typeof params.value !== "string") throw RequestError.invalidParams("model value must be a string") + const selection = parseModelSelection(params.value, providers) + this.sessionManager.setModel(session.id, selection.model) + this.sessionManager.setVariant(session.id, selection.variant) + } else if (params.configId === "mode") { + if (typeof params.value !== "string") throw RequestError.invalidParams("mode value must be a string") + const availableModes = await this.loadAvailableModes(session.cwd) + if (!availableModes.some((mode) => mode.id === params.value)) { + throw RequestError.invalidParams(JSON.stringify({ error: `Mode not found: ${params.value}` })) + } + this.sessionManager.setMode(session.id, params.value) + } else { + throw RequestError.invalidParams(JSON.stringify({ error: `Unknown config option: ${params.configId}` })) + } + + const updatedSession = this.sessionManager.get(session.id) + const model = updatedSession.model ?? (await defaultModel(this.config, session.cwd)) + const availableVariants = modelVariantsFromProviders(entries, model) + const currentModelId = formatModelIdWithVariant(model, updatedSession.variant, availableVariants, true) + const availableModels = buildAvailableModels(entries, { includeVariants: true }) + const modeState = await this.resolveModeState(session.cwd, session.id) + const modes = modeState.currentModeId + ? { availableModes: modeState.availableModes, currentModeId: modeState.currentModeId } + : undefined + + return { + configOptions: buildConfigOptions({ currentModelId, availableModels, modes }), + } + } + async prompt(params: PromptRequest) { const sessionID = params.sessionId const session = this.sessionManager.get(sessionID) @@ -1660,4 +1725,36 @@ function parseModelSelection( return { model: parsed, variant: undefined } } +function buildConfigOptions(input: { + currentModelId: string + availableModels: ModelOption[] + modes?: { availableModes: ModeOption[]; currentModeId: string } | undefined +}): SessionConfigOption[] { + const options: SessionConfigOption[] = [ + { + id: "model", + name: "Model", + category: "model", + type: "select", + currentValue: input.currentModelId, + options: input.availableModels.map((m) => ({ value: m.modelId, name: m.name })), + }, + ] + if (input.modes) { + options.push({ + id: "mode", + name: "Session Mode", + category: "mode", + type: "select", + currentValue: input.modes.currentModeId, + options: input.modes.availableModes.map((m) => ({ + value: m.id, + name: m.name, + ...(m.description ? { description: m.description } : {}), + })), + }) + } + return options +} + export * as ACP from "./agent" diff --git a/packages/opencode/src/acp/tool-format.ts b/packages/opencode/src/acp/tool-format.ts index 223ae0c6361a..b928c6d63352 100644 --- a/packages/opencode/src/acp/tool-format.ts +++ b/packages/opencode/src/acp/tool-format.ts @@ -1,4 +1,4 @@ -import { resolve } from "path" +import { isAbsolute, resolve } from "path" import type { ToolCallContent, ToolKind } from "@agentclientprotocol/sdk" export interface ToolCallInfo { @@ -46,12 +46,188 @@ function str(v: unknown): string { return typeof v === "string" ? v : "" } +type ReadTruncation = + | { kind: "end"; total: number } + | { kind: "more"; from: number; to: number; total: number; next: number } + | { kind: "cut"; from: number; to: number; maxBytes: string; next: number } + | { kind: "dir_partial"; shown: number; total: number; next: number } + | { kind: "dir_full"; total: number } + +interface ParsedReadOutput { + path: string + type: "file" | "directory" + content: string + truncation?: ReadTruncation + systemReminder?: string +} + +const FILE_FOOTER_END = /\n\n\(End of file - total (\d+) lines\)$/ +const FILE_FOOTER_MORE = /\n\n\(Showing lines (\d+)-(\d+) of (\d+)\. Use offset=(\d+) to continue\.\)$/ +const FILE_FOOTER_CUT = /\n\n\(Output capped at (.+?)\. Showing lines (\d+)-(\d+)\. Use offset=(\d+) to continue\.\)$/ +const DIR_FOOTER_PARTIAL = /\n\(Showing (\d+) of (\d+) entries\. Use 'offset' parameter to read beyond entry (\d+)\)$/ +const DIR_FOOTER_FULL = /\n\((\d+) entries\)$/ +const SYSTEM_REMINDER_RE = /^\n\n\n([\s\S]*)\n<\/system-reminder>$/ + +interface ParsedSkillOutput { + name: string + markdown: string + baseDir: string + files: string[] +} + +interface ParsedTaskOutput { + taskId: string + result: string +} + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&") +} + +export function parseSkillOutput(output: string): ParsedSkillOutput | null { + try { + const open = output.match(/^\n/) + if (!open) return null + const name = open[1] + const close = "\n" + if (!output.endsWith(close)) return null + const body = output.slice(open[0].length, output.length - close.length) + + const headerRe = new RegExp(`^# Skill: ${escapeRegExp(name)}\\n\\n`) + const header = body.match(headerRe) + if (!header) return null + const afterHeader = body.slice(header[0].length) + + const skillFilesOpen = "\n\n\n" + const skillFilesClose = "\n" + const openIdx = afterHeader.lastIndexOf(skillFilesOpen) + if (openIdx < 0) return null + if (!afterHeader.endsWith(skillFilesClose)) return null + const preFiles = afterHeader.slice(0, openIdx) + const filesBlock = afterHeader.slice( + openIdx + skillFilesOpen.length, + afterHeader.length - skillFilesClose.length, + ) + + const preambleRe = + /\n\nBase directory for this skill: (\S+)\nRelative paths in this skill \(e\.g\., scripts\/, reference\/\) are relative to this base directory\.\nNote: file list is sampled\.$/ + const preamble = preFiles.match(preambleRe) + if (!preamble) return null + const markdown = preFiles.slice(0, preFiles.length - preamble[0].length) + const baseDir = preamble[1] + + let files: string[] = [] + if (filesBlock !== "") { + files = filesBlock.split("\n").map((line) => { + const m = line.match(/^(.*)<\/file>$/) + if (!m) throw new Error("bad file line") + return m[1] + }) + } + + return { name, markdown, baseDir, files } + } catch { + return null + } +} + +export function parseTaskOutput(output: string): ParsedTaskOutput | null { + try { + const m = output.match( + /^task_id: (\S+) \(for resuming to continue this task if needed\)\n\n\n([\s\S]*)\n<\/task_result>$/, + ) + if (!m) return null + return { taskId: m[1], result: m[2] } + } catch { + return null + } +} + +export function parseReadOutput(output: string): ParsedReadOutput | null { + try { + const envelope = output.match(/^([^\n]*)<\/path>\n(file|directory)<\/type>\n/) + if (!envelope) return null + const path = envelope[1] + const type = envelope[2] as "file" | "directory" + const afterEnvelope = output.slice(envelope[0].length) + + const openTag = type === "file" ? "\n" : "\n" + const closeTag = type === "file" ? "\n" : "\n" + if (!afterEnvelope.startsWith(openTag)) return null + const bodyStart = openTag.length + const closeIdx = afterEnvelope.lastIndexOf(closeTag) + if (closeIdx < bodyStart) return null + + const body = afterEnvelope.slice(bodyStart, closeIdx) + const afterClose = afterEnvelope.slice(closeIdx + closeTag.length) + + let systemReminder: string | undefined + if (afterClose.length > 0) { + const sr = afterClose.match(SYSTEM_REMINDER_RE) + if (!sr) return null + systemReminder = sr[1] + } + + let truncation: ReadTruncation | undefined + let content = body + if (type === "file") { + const end = body.match(FILE_FOOTER_END) + const more = body.match(FILE_FOOTER_MORE) + const cut = body.match(FILE_FOOTER_CUT) + if (end) { + content = body.slice(0, body.length - end[0].length) + truncation = { kind: "end", total: Number(end[1]) } + } else if (more) { + content = body.slice(0, body.length - more[0].length) + truncation = { + kind: "more", + from: Number(more[1]), + to: Number(more[2]), + total: Number(more[3]), + next: Number(more[4]), + } + } else if (cut) { + content = body.slice(0, body.length - cut[0].length) + truncation = { + kind: "cut", + maxBytes: cut[1], + from: Number(cut[2]), + to: Number(cut[3]), + next: Number(cut[4]), + } + } + } else { + const partial = body.match(DIR_FOOTER_PARTIAL) + const full = body.match(DIR_FOOTER_FULL) + if (partial) { + content = body.slice(0, body.length - partial[0].length) + truncation = { + kind: "dir_partial", + shown: Number(partial[1]), + total: Number(partial[2]), + next: Number(partial[3]), + } + } else if (full) { + content = body.slice(0, body.length - full[0].length) + truncation = { kind: "dir_full", total: Number(full[1]) } + } + } + + const result: ParsedReadOutput = { path, type, content } + if (truncation) result.truncation = truncation + if (systemReminder !== undefined) result.systemReminder = systemReminder + return result + } catch { + return null + } +} + function num(v: unknown): number | undefined { return typeof v === "number" ? v : undefined } function abs(p: string, cwd: string): string { - return p && !p.startsWith("/") ? resolve(cwd, p) : p + return p && !isAbsolute(p) ? resolve(cwd, p) : p } export function toolCallFromPart(tool: string, input: Record, cwd: string): ToolCallInfo { @@ -66,7 +242,7 @@ export function toolCallFromPart(tool: string, input: Record, c title: description || command || "Terminal", kind: "other", content: [], - locations: workdir ? [{ path: workdir }] : [], + locations: workdir ? [{ path: abs(workdir, cwd) }] : [], rawInput: input, } } @@ -331,6 +507,31 @@ export function toolResultFromPart( const content: ToolCallContent[] = [textContent(fenceWith(output, ""))] switch (name) { + case "read": + case "list": { + if (isError) { + return { content, rawOutput: { stderr: output } } + } + const parsed = parseReadOutput(output) + return { content: [], rawOutput: parsed ?? { stdout: output } } + } + + case "skill": { + if (isError) { + return { content, rawOutput: { stderr: output } } + } + const parsed = parseSkillOutput(output) + return { content: [], rawOutput: parsed ?? { stdout: output } } + } + + case "task": { + if (isError) { + return { content, rawOutput: { stderr: output } } + } + const parsed = parseTaskOutput(output) + return { content: [], rawOutput: parsed ?? { stdout: output } } + } + case "edit": { const filePath = str(input.filePath) const oldString = str(input.oldString) diff --git a/packages/opencode/test/acp/tool-format.test.ts b/packages/opencode/test/acp/tool-format.test.ts index cbb3832beb31..771d789dd7b7 100644 --- a/packages/opencode/test/acp/tool-format.test.ts +++ b/packages/opencode/test/acp/tool-format.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "bun:test" +import { isAbsolute, resolve } from "path" import { permissionDisplayInfo, toolCallFromPart as _toolCallFromPart, @@ -110,7 +111,7 @@ describe("toolCallFromPart", () => { it("absolutizes relative paths", () => { const result = toolCallFromPart("glob", { path: "src", pattern: "*.ts" }) - expect(result.locations[0].path.startsWith("/")).toBe(true) + expect(isAbsolute(result.locations[0].path)).toBe(true) }) it("handles no path or pattern", () => { @@ -324,30 +325,39 @@ describe("apply_patch canonical field", () => { describe("path resolution against cwd", () => { it("resolves relative filePath against the passed cwd (not process.cwd)", () => { - const result = toolCallFromPart("read", { filePath: "src/index.ts" }, "/workspace/project") - expect(result.locations).toEqual([{ path: "/workspace/project/src/index.ts" }]) + const cwd = resolve("/workspace/project") + const result = toolCallFromPart("read", { filePath: "src/index.ts" }, cwd) + expect(result.locations).toEqual([{ path: resolve(cwd, "src/index.ts") }]) }) it("leaves absolute paths untouched", () => { - const result = toolCallFromPart("read", { filePath: "/abs/path.ts" }, "/workspace/project") - expect(result.locations).toEqual([{ path: "/abs/path.ts" }]) + const absolutePath = resolve("/abs/path.ts") + const result = toolCallFromPart("read", { filePath: absolutePath }, resolve("/workspace/project")) + expect(result.locations).toEqual([{ path: absolutePath }]) }) it("resolves relative path in edit result content", () => { + const cwd = resolve("/workspace") const result = toolResultFromPart( "edit", { filePath: "rel/file.ts", oldString: "a", newString: "b" }, "ok", false, - "/workspace", + cwd, ) expect(result.content[1]).toEqual({ type: "diff", - path: "/workspace/rel/file.ts", + path: resolve(cwd, "rel/file.ts"), oldText: "a", newText: "b", }) }) + + it("resolves relative bash workdir against cwd", () => { + const cwd = resolve("/workspace") + const result = toolCallFromPart("bash", { command: "ls", workdir: "rel" }, cwd) + expect(result.locations).toEqual([{ path: resolve(cwd, "rel") }]) + }) }) describe("non-bash tool output fencing", () => { @@ -361,8 +371,8 @@ describe("non-bash tool output fencing", () => { expect(result.rawOutput).toEqual({ stdout: output }) }) - it("fences all default-branch success output (read/list/grep/webfetch/todowrite/unknown) in a plain code block", () => { - for (const tool of ["read", "list", "grep", "webfetch", "todowrite", "codesearch", "unknownTool"]) { + it("fences default-branch success output (grep/webfetch/todowrite/unknown) in a plain code block", () => { + for (const tool of ["grep", "webfetch", "todowrite", "codesearch", "unknownTool"]) { const result = toolResultFromPart(tool, {}, "plain text", false) const text = (result.content[0] as any).content.text expect(text).toMatch(/^```\n/) @@ -371,6 +381,270 @@ describe("non-bash tool output fencing", () => { } }) + it("drops content on read/list success (title + locations already carry the UX) but preserves rawOutput", () => { + for (const tool of ["read", "list"]) { + const result = toolResultFromPart(tool, {}, "plain text", false) + expect(result.content).toEqual([]) + // unparseable wrapper → falls back to stdout + expect(result.rawOutput).toEqual({ stdout: "plain text" }) + } + }) + + it("fences read/list error output so the user sees why it failed", () => { + for (const tool of ["read", "list"]) { + const result = toolResultFromPart(tool, { filePath: "/missing" }, "ENOENT: no such file", true) + const text = (result.content[0] as any).content.text + expect(text).toMatch(/^```\n/) + expect(text).toContain("ENOENT: no such file") + expect(text).toMatch(/\n```$/) + expect(result.rawOutput).toEqual({ stderr: "ENOENT: no such file" }) + } + }) + + it("parses read file wrapper into structured rawOutput with end footer", () => { + const output = + "/src/a.ts\nfile\n\n1: line one\n2: line two\n\n(End of file - total 2 lines)\n" + const result = toolResultFromPart("read", { filePath: "/src/a.ts" }, output, false) + expect(result.content).toEqual([]) + expect(result.rawOutput).toEqual({ + path: "/src/a.ts", + type: "file", + content: "1: line one\n2: line two", + truncation: { kind: "end", total: 2 }, + }) + }) + + it("parses read file wrapper with 'more' truncation footer", () => { + const output = + "/src/big.ts\nfile\n\n1: a\n2: b\n\n(Showing lines 1-2 of 500. Use offset=3 to continue.)\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src/big.ts", + type: "file", + content: "1: a\n2: b", + truncation: { kind: "more", from: 1, to: 2, total: 500, next: 3 }, + }) + }) + + it("parses read file wrapper with 'cut' truncation footer", () => { + const output = + "/src/huge.bin\nfile\n\n1: a\n2: b\n\n(Output capped at 256KB. Showing lines 1-2. Use offset=3 to continue.)\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src/huge.bin", + type: "file", + content: "1: a\n2: b", + truncation: { kind: "cut", maxBytes: "256KB", from: 1, to: 2, next: 3 }, + }) + }) + + it("parses read directory wrapper with full footer", () => { + const output = + "/src\ndirectory\n\na.ts\nb.ts\n(2 entries)\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src", + type: "directory", + content: "a.ts\nb.ts", + truncation: { kind: "dir_full", total: 2 }, + }) + }) + + it("parses read directory wrapper with partial footer", () => { + const output = + "/src\ndirectory\n\na.ts\nb.ts\n(Showing 2 of 100 entries. Use 'offset' parameter to read beyond entry 2)\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src", + type: "directory", + content: "a.ts\nb.ts", + truncation: { kind: "dir_partial", shown: 2, total: 100, next: 2 }, + }) + }) + + it("parses read wrapper with trailing system-reminder", () => { + const output = + "/src/a.ts\nfile\n\n1: x\n\n(End of file - total 1 lines)\n\n\n\nfollow project conventions\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src/a.ts", + type: "file", + content: "1: x", + truncation: { kind: "end", total: 1 }, + systemReminder: "follow project conventions", + }) + }) + + it("tolerates literal inside the file body via greedy extraction", () => { + const inner = "1: prose mentioning inline" + const output = + `/src/md.md\nfile\n\n${inner}\n\n(End of file - total 1 lines)\n` + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ + path: "/src/md.md", + type: "file", + content: inner, + truncation: { kind: "end", total: 1 }, + }) + }) + + it("falls back to stdout on malformed wrapper (unexpected tail)", () => { + const output = + "/src/a.ts\nfile\n\n1: x\n\nunexpected trailing text" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ stdout: output }) + }) + + it("falls back to stdout when envelope does not match", () => { + const output = "/src/a.tsfile\n\nmissing newline\n" + const result = toolResultFromPart("read", {}, output, false) + expect(result.rawOutput).toEqual({ stdout: output }) + }) + + it("parses skill wrapper with files into structured rawOutput", () => { + const output = [ + '', + "# Skill: Frontend", + "", + "Do UI things.", + "", + "Base directory for this skill: file:///skills/frontend", + "Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.", + "Note: file list is sampled.", + "", + "", + "/skills/frontend/scripts/build.sh", + "/skills/frontend/reference/guide.md", + "", + "", + ].join("\n") + const result = toolResultFromPart("skill", {}, output, false) + expect(result.content).toEqual([]) + expect(result.rawOutput).toEqual({ + name: "Frontend", + markdown: "Do UI things.", + baseDir: "file:///skills/frontend", + files: ["/skills/frontend/scripts/build.sh", "/skills/frontend/reference/guide.md"], + }) + }) + + it("parses skill wrapper with empty files list", () => { + const output = [ + '', + "# Skill: Bare", + "", + "body", + "", + "Base directory for this skill: file:///skills/bare", + "Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.", + "Note: file list is sampled.", + "", + "", + "", + "", + "", + ].join("\n") + const result = toolResultFromPart("skill", {}, output, false) + expect(result.rawOutput).toEqual({ + name: "Bare", + markdown: "body", + baseDir: "file:///skills/bare", + files: [], + }) + }) + + it("parses skill wrapper with multi-paragraph markdown body", () => { + const output = [ + '', + "# Skill: Deep", + "", + "Para one.", + "", + "Para two with `code`.", + "", + "Base directory for this skill: file:///x", + "Relative paths in this skill (e.g., scripts/, reference/) are relative to this base directory.", + "Note: file list is sampled.", + "", + "", + "", + "", + "", + ].join("\n") + const result = toolResultFromPart("skill", {}, output, false) + expect((result.rawOutput as any).markdown).toBe("Para one.\n\nPara two with `code`.") + }) + + it("falls back to stdout on malformed skill wrapper", () => { + const output = '\n# Skill: X\n\nbroken — no closing tag' + const result = toolResultFromPart("skill", {}, output, false) + expect(result.rawOutput).toEqual({ stdout: output }) + }) + + it("fences skill error output so the user sees why it failed", () => { + const output = 'Skill "Missing" not found. Available skills: Frontend' + const result = toolResultFromPart("skill", {}, output, true) + const text = (result.content[0] as any).content.text + expect(text).toContain(output) + expect(result.rawOutput).toEqual({ stderr: output }) + }) + + it("parses task wrapper into structured rawOutput", () => { + const output = [ + "task_id: ses_abc123 (for resuming to continue this task if needed)", + "", + "", + "Investigation complete. Found 3 issues in auth.ts.", + "", + ].join("\n") + const result = toolResultFromPart("task", { description: "investigate auth" }, output, false) + expect(result.content).toEqual([]) + expect(result.rawOutput).toEqual({ + taskId: "ses_abc123", + result: "Investigation complete. Found 3 issues in auth.ts.", + }) + }) + + it("parses task wrapper with multiline result preserving internal newlines", () => { + const output = [ + "task_id: ses_xyz (for resuming to continue this task if needed)", + "", + "", + "Line one", + "", + "Line three", + "", + ].join("\n") + const result = toolResultFromPart("task", {}, output, false) + expect((result.rawOutput as any).result).toBe("Line one\n\nLine three") + }) + + it("parses task wrapper with empty result", () => { + const output = [ + "task_id: ses_empty (for resuming to continue this task if needed)", + "", + "", + "", + "", + ].join("\n") + const result = toolResultFromPart("task", {}, output, false) + expect(result.rawOutput).toEqual({ taskId: "ses_empty", result: "" }) + }) + + it("falls back to stdout on malformed task wrapper", () => { + const output = "no task_id prefix\n\nhi\n" + const result = toolResultFromPart("task", {}, output, false) + expect(result.rawOutput).toEqual({ stdout: output }) + }) + + it("fences task error output so the user sees why it failed", () => { + const output = "Task aborted by user" + const result = toolResultFromPart("task", {}, output, true) + const text = (result.content[0] as any).content.text + expect(text).toContain(output) + expect(result.rawOutput).toEqual({ stderr: output }) + }) + it("preserves widened fence when output contains triple-backticks", () => { const output = "before\n```\nnested\n```\nafter" const result = toolResultFromPart("unknown", {}, output, false) @@ -412,7 +686,8 @@ describe("permissionDisplayInfo", () => { }) it("resolves relative edit filepath against cwd", () => { - const info = permissionDisplayInfo("edit", { filepath: "rel/foo.ts" }, "/ws") - expect(info.locations).toEqual([{ path: "/ws/rel/foo.ts" }]) + const cwd = resolve("/ws") + const info = permissionDisplayInfo("edit", { filepath: "rel/foo.ts" }, cwd) + expect(info.locations).toEqual([{ path: resolve(cwd, "rel/foo.ts") }]) }) }) From 8140bdadf3f0a0acc3c8cea1bd413ca2581666a5 Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Thu, 23 Apr 2026 22:50:52 +0300 Subject: [PATCH 6/8] fix(acp): correct read offset, seed tool_call status, surface bash cwd - `read` tool: treat `offset` as 1-based (matches the read tool schema); previously off-by-one in the displayed line range and location. - `toolStart`: derive the initial `tool_call` status from `part.state.status` (pending/in_progress/completed/failed) instead of always emitting `pending`, so the first event reflects reality. - `bash`: fall back to the session cwd in both `locations` and `rawInput` when the model omits `workdir`, so Zed always shows where a command runs. --- packages/opencode/src/acp/agent.ts | 12 +++++-- packages/opencode/src/acp/tool-format.ts | 17 +++++----- .../test/acp/event-subscription.test.ts | 12 +++---- .../opencode/test/acp/tool-format.test.ts | 31 ++++++++++++++++--- 4 files changed, 53 insertions(+), 19 deletions(-) diff --git a/packages/opencode/src/acp/agent.ts b/packages/opencode/src/acp/agent.ts index 4fd605884d4c..4935cd322e14 100644 --- a/packages/opencode/src/acp/agent.ts +++ b/packages/opencode/src/acp/agent.ts @@ -1073,6 +1073,14 @@ export class Agent implements ACPAgent { private async toolStart(sessionId: string, part: ToolPart, cwd: string) { if (this.markToolStarted(sessionId, part.callID)) return const info = toolCallFromPart(part.tool, part.state.input, cwd) + const status = + part.state.status === "pending" + ? "pending" + : part.state.status === "error" + ? "failed" + : part.state.status === "completed" + ? "completed" + : "in_progress" await this.connection .sessionUpdate({ sessionId, @@ -1081,13 +1089,13 @@ export class Agent implements ACPAgent { toolCallId: part.callID, title: info.title, kind: info.kind, - status: "pending", + status, locations: info.locations, rawInput: info.rawInput, }, }) .catch((error) => { - log.error("failed to send tool pending to ACP", { error }) + log.error("failed to send tool_call to ACP", { error }) }) } diff --git a/packages/opencode/src/acp/tool-format.ts b/packages/opencode/src/acp/tool-format.ts index b928c6d63352..72455f16954a 100644 --- a/packages/opencode/src/acp/tool-format.ts +++ b/packages/opencode/src/acp/tool-format.ts @@ -238,30 +238,33 @@ export function toolCallFromPart(tool: string, input: Record, c const command = str(input.command) const description = str(input.description) const workdir = str(input.workdir) + const resolvedWorkdir = workdir ? abs(workdir, cwd) : cwd return { title: description || command || "Terminal", kind: "other", content: [], - locations: workdir ? [{ path: abs(workdir, cwd) }] : [], - rawInput: input, + locations: [{ path: resolvedWorkdir }], + rawInput: workdir ? input : { ...input, cwd }, } } case "read": { const filePath = str(input.filePath) - const offset = num(input.offset) ?? 0 + const offset = num(input.offset) ?? 1 const limit = num(input.limit) ?? 0 + const hasMeaningfulOffset = offset > 1 + const rangeStart = Math.max(offset, 1) let suffix = "" if (limit) { - suffix = ` (${offset + 1} - ${offset + limit})` - } else if (offset) { - suffix = ` (from line ${offset + 1})` + suffix = ` (${rangeStart} - ${rangeStart + limit - 1})` + } else if (hasMeaningfulOffset) { + suffix = ` (from line ${offset})` } return { title: filePath ? `Read ${filePath}${suffix}` : "Read File", kind: "read", content: [], - locations: filePath ? [{ path: abs(filePath, cwd), ...(offset ? { line: offset + 1 } : {}) }] : [], + locations: filePath ? [{ path: abs(filePath, cwd), ...(hasMeaningfulOffset ? { line: offset } : {}) }] : [], rawInput: input, } } diff --git a/packages/opencode/test/acp/event-subscription.test.ts b/packages/opencode/test/acp/event-subscription.test.ts index 74a49994607b..3efd1b6428e5 100644 --- a/packages/opencode/test/acp/event-subscription.test.ts +++ b/packages/opencode/test/acp/event-subscription.test.ts @@ -569,7 +569,7 @@ describe("acp.agent event subscription", () => { }) }) - test("emits synthetic pending before first running update for any tool", async () => { + test("first tool_call reflects the tool's actual status (not a synthetic pending)", async () => { await using tmp = await tmpdir() await Instance.provide({ directory: tmp.path, @@ -603,12 +603,12 @@ describe("acp.agent event subscription", () => { .filter((u) => u === "tool_call" || u === "tool_call_update") expect(types).toEqual(["tool_call", "tool_call_update", "tool_call", "tool_call_update"]) - const pendings = sessionUpdates.filter( + const initials = sessionUpdates.filter( (u) => u.sessionId === sessionId && u.update.sessionUpdate === "tool_call", ) - expect(pendings.every((p) => p.update.sessionUpdate === "tool_call" && p.update.status === "pending")).toBe( - true, - ) + expect( + initials.every((p) => p.update.sessionUpdate === "tool_call" && p.update.status === "in_progress"), + ).toBe(true) stop() }, }) @@ -783,7 +783,7 @@ describe("acp.agent event subscription", () => { expect(running).toBeDefined() expect((running as any).title).toBe("say hi") expect((running as any).kind).toBeDefined() - expect((running as any).rawInput).toEqual(input) + expect((running as any).rawInput).toEqual({ ...input, cwd }) stop() }, }) diff --git a/packages/opencode/test/acp/tool-format.test.ts b/packages/opencode/test/acp/tool-format.test.ts index 771d789dd7b7..47512bc4a9be 100644 --- a/packages/opencode/test/acp/tool-format.test.ts +++ b/packages/opencode/test/acp/tool-format.test.ts @@ -36,6 +36,18 @@ describe("toolCallFromPart", () => { const result = toolCallFromPart("mcp__acp__bash", { command: "echo hi" }) expect(result.title).toBe("echo hi") }) + + it("falls back to session cwd in locations and rawInput when workdir is absent", () => { + const cwd = resolve("/workspace/project") + const result = toolCallFromPart("bash", { command: "ls" }, cwd) + expect(result.locations).toEqual([{ path: cwd }]) + expect(result.rawInput).toEqual({ command: "ls", cwd }) + }) + + it("does not inject cwd into rawInput when workdir is provided", () => { + const result = toolCallFromPart("bash", { command: "ls", workdir: "/opt" }, resolve("/workspace")) + expect(result.rawInput).toEqual({ command: "ls", workdir: "/opt" }) + }) }) describe("read", () => { @@ -46,9 +58,9 @@ describe("toolCallFromPart", () => { expect(result.locations).toEqual([{ path: "/src/index.ts" }]) }) - it("uses 1-based line numbers when offset is provided", () => { + it("uses 1-based line numbers when offset is provided (matches read tool contract)", () => { const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10 }) - expect(result.locations).toEqual([{ path: "/src/index.ts", line: 11 }]) + expect(result.locations).toEqual([{ path: "/src/index.ts", line: 10 }]) }) it("omits line key when offset is zero", () => { @@ -56,14 +68,25 @@ describe("toolCallFromPart", () => { expect(result.locations).toEqual([{ path: "/src/index.ts" }]) }) + it("omits line key when offset is 1 (redundant with default)", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 1 }) + expect(result.locations).toEqual([{ path: "/src/index.ts" }]) + expect(result.title).toBe("Read /src/index.ts") + }) + it("includes line range suffix", () => { const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10, limit: 20 }) - expect(result.title).toBe("Read /src/index.ts (11 - 30)") + expect(result.title).toBe("Read /src/index.ts (10 - 29)") + }) + + it("clamps range start to 1 when only limit is given", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", limit: 20 }) + expect(result.title).toBe("Read /src/index.ts (1 - 20)") }) it("includes from-line suffix", () => { const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10 }) - expect(result.title).toBe("Read /src/index.ts (from line 11)") + expect(result.title).toBe("Read /src/index.ts (from line 10)") }) it("falls back when no file path", () => { From 5d6fc50b0c9b3ccea02b8c524c93459307291a67 Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Thu, 23 Apr 2026 23:06:57 +0300 Subject: [PATCH 7/8] fix(acp): reject non-finite values in tool-format num() helper Guards against callers outside the zod-validated tool boundary (MCP tools, permission display, tests) producing NaN/Infinity titles like '(NaN - NaN)' in read ranges. --- packages/opencode/src/acp/tool-format.ts | 2 +- packages/opencode/test/acp/tool-format.test.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/acp/tool-format.ts b/packages/opencode/src/acp/tool-format.ts index 72455f16954a..1ec6643ac5a4 100644 --- a/packages/opencode/src/acp/tool-format.ts +++ b/packages/opencode/src/acp/tool-format.ts @@ -223,7 +223,7 @@ export function parseReadOutput(output: string): ParsedReadOutput | null { } function num(v: unknown): number | undefined { - return typeof v === "number" ? v : undefined + return typeof v === "number" && Number.isFinite(v) ? v : undefined } function abs(p: string, cwd: string): string { diff --git a/packages/opencode/test/acp/tool-format.test.ts b/packages/opencode/test/acp/tool-format.test.ts index 47512bc4a9be..8b495867b113 100644 --- a/packages/opencode/test/acp/tool-format.test.ts +++ b/packages/opencode/test/acp/tool-format.test.ts @@ -84,6 +84,12 @@ describe("toolCallFromPart", () => { expect(result.title).toBe("Read /src/index.ts (1 - 20)") }) + it("ignores non-finite offset and limit values", () => { + const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: NaN, limit: Infinity }) + expect(result.title).toBe("Read /src/index.ts") + expect(result.locations).toEqual([{ path: "/src/index.ts" }]) + }) + it("includes from-line suffix", () => { const result = toolCallFromPart("read", { filePath: "/src/index.ts", offset: 10 }) expect(result.title).toBe("Read /src/index.ts (from line 10)") From 1629d5e259d41bb9668035c1af6c04b947a82002 Mon Sep 17 00:00:00 2001 From: Mert Can Demir Date: Thu, 23 Apr 2026 23:20:29 +0300 Subject: [PATCH 8/8] refactor(acp): unify running-content helper, fix replay dedupe, map tool status - Extract the bash-output hashing and content-block assembly into a single `runningContent(sessionId, part)` helper, and call it from both the `message.part.updated` live path and the `processMessage` replay path. Previously the replay path (loadSession/forkSession) emitted the output verbatim without seeding `bashSnapshots`, so the first live update after replay would re-emit the same content. The helper guarantees both paths stay in lockstep. - Replace the 4-level nested ternary in `toolStart` with a `Record` map plus a \`satisfies\` constraint, so adding a new tool-part state would be a compile error. - Cover the new contract with three tests: replay + live dedupe on identical bash output, first `tool_call` emitted at \`completed\` when a tool part is already completed on first observation, and the analogous \`failed\` status for error parts. --- packages/opencode/src/acp/agent.ts | 65 +++----- .../test/acp/event-subscription.test.ts | 148 ++++++++++++++++++ 2 files changed, 174 insertions(+), 39 deletions(-) diff --git a/packages/opencode/src/acp/agent.ts b/packages/opencode/src/acp/agent.ts index 4935cd322e14..327f2e4c99c0 100644 --- a/packages/opencode/src/acp/agent.ts +++ b/packages/opencode/src/acp/agent.ts @@ -27,6 +27,7 @@ import { type SetSessionModeRequest, type SetSessionModeResponse, type ToolCallContent, + type ToolCallStatus, type Usage, } from "@agentclientprotocol/sdk" @@ -57,6 +58,15 @@ type ModelOption = { modelId: string; name: string } const DEFAULT_VARIANT_VALUE = "default" +type PartStatus = ToolPart["state"]["status"] + +const TOOL_STATUS_MAP = { + pending: "pending", + running: "in_progress", + completed: "completed", + error: "failed", +} as const satisfies Record + const log = Log.create({ service: "acp-agent" }) async function getContextLimit( @@ -325,25 +335,7 @@ export class Agent implements ACPAgent { switch (part.state.status) { case "running": { - const output = this.bashOutput(part) - const content: ToolCallContent[] = [] - if (output) { - let changed = true - if (part.tool === "bash") { - const hash = Hash.fast(output) - changed = this.getBashSnapshot(sessionId, part.callID) !== hash - this.setBashSnapshot(sessionId, part.callID, hash) - } - if (changed) { - content.push({ - type: "content", - content: { - type: "text", - text: part.tool === "bash" ? fenceWith(output, "sh") : output, - }, - }) - } - } + const content = this.runningContent(sessionId, part) await this.connection .sessionUpdate({ sessionId, @@ -833,17 +825,7 @@ export class Agent implements ACPAgent { const title = ("title" in part.state && part.state.title) || info.title switch (part.state.status) { case "running": { - const output = this.bashOutput(part) - const runningContent: ToolCallContent[] = [] - if (output) { - runningContent.push({ - type: "content", - content: { - type: "text", - text: part.tool === "bash" ? fenceWith(output, "sh") : output, - }, - }) - } + const runningContent = this.runningContent(sessionId, part) await this.connection .sessionUpdate({ sessionId, @@ -1070,17 +1052,22 @@ export class Agent implements ACPAgent { return output } + private runningContent(sessionId: string, part: ToolPart): ToolCallContent[] { + const output = this.bashOutput(part) + if (!output) return [] + if (part.tool === "bash") { + const hash = Hash.fast(output) + const changed = this.getBashSnapshot(sessionId, part.callID) !== hash + this.setBashSnapshot(sessionId, part.callID, hash) + if (!changed) return [] + return [{ type: "content", content: { type: "text", text: fenceWith(output, "sh") } }] + } + return [{ type: "content", content: { type: "text", text: output } }] + } + private async toolStart(sessionId: string, part: ToolPart, cwd: string) { if (this.markToolStarted(sessionId, part.callID)) return const info = toolCallFromPart(part.tool, part.state.input, cwd) - const status = - part.state.status === "pending" - ? "pending" - : part.state.status === "error" - ? "failed" - : part.state.status === "completed" - ? "completed" - : "in_progress" await this.connection .sessionUpdate({ sessionId, @@ -1089,7 +1076,7 @@ export class Agent implements ACPAgent { toolCallId: part.callID, title: info.title, kind: info.kind, - status, + status: TOOL_STATUS_MAP[part.state.status], locations: info.locations, rawInput: info.rawInput, }, diff --git a/packages/opencode/test/acp/event-subscription.test.ts b/packages/opencode/test/acp/event-subscription.test.ts index 3efd1b6428e5..385bf539028e 100644 --- a/packages/opencode/test/acp/event-subscription.test.ts +++ b/packages/opencode/test/acp/event-subscription.test.ts @@ -673,6 +673,154 @@ describe("acp.agent event subscription", () => { }) }) + test("replayed bash output is deduped against the first live update with the same output", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, controller, sessionUpdates, stop, sdk } = createFakeAgent() + const cwd = "/tmp/opencode-acp-test" + const sessionId = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const input = { command: "echo hi" } + + sdk.session.messages = async () => ({ + data: [ + { + info: { role: "assistant", sessionID: sessionId }, + parts: [ + { + type: "tool", + callID: "call_1", + tool: "bash", + state: { + status: "running", + input, + metadata: { output: "hi\n" }, + time: { start: Date.now() }, + }, + }, + ], + }, + ], + }) + + await agent.loadSession({ sessionId, cwd, mcpServers: [] } as any) + controller.push( + toolEvent(sessionId, cwd, { + callID: "call_1", + tool: "bash", + status: "running", + input, + metadata: { output: "hi\n" }, + }), + ) + await new Promise((r) => setTimeout(r, 20)) + + const contentTexts = sessionUpdates + .filter((u) => u.sessionId === sessionId) + .map((u) => u.update) + .filter((u) => "toolCallId" in u && u.toolCallId === "call_1") + .map((u) => inProgressText(u)) + .filter((t): t is string => typeof t === "string") + + expect(contentTexts).toEqual(["```sh\nhi\n```"]) + stop() + }, + }) + }) + + test("first tool_call for an already-completed tool part carries status 'completed'", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, sessionUpdates, stop, sdk } = createFakeAgent() + const cwd = "/tmp/opencode-acp-test" + const sessionId = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const input = { command: "echo done" } + + sdk.session.messages = async () => ({ + data: [ + { + info: { role: "assistant", sessionID: sessionId }, + parts: [ + { + type: "tool", + callID: "call_done", + tool: "bash", + state: { + status: "completed", + input, + output: "done\n", + title: "echo done", + metadata: { output: "done\n" }, + time: { start: Date.now(), end: Date.now() }, + }, + }, + ], + }, + ], + }) + + await agent.loadSession({ sessionId, cwd, mcpServers: [] } as any) + await new Promise((r) => setTimeout(r, 20)) + + const initial = sessionUpdates + .filter((u) => u.sessionId === sessionId) + .map((u) => u.update) + .find((u) => u.sessionUpdate === "tool_call" && "toolCallId" in u && u.toolCallId === "call_done") + expect(initial).toBeDefined() + expect((initial as any).status).toBe("completed") + stop() + }, + }) + }) + + test("first tool_call for an already-errored tool part carries status 'failed'", async () => { + await using tmp = await tmpdir() + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { agent, sessionUpdates, stop, sdk } = createFakeAgent() + const cwd = "/tmp/opencode-acp-test" + const sessionId = await agent.newSession({ cwd, mcpServers: [] } as any).then((x) => x.sessionId) + const input = { command: "false" } + + sdk.session.messages = async () => ({ + data: [ + { + info: { role: "assistant", sessionID: sessionId }, + parts: [ + { + type: "tool", + callID: "call_err", + tool: "bash", + state: { + status: "error", + input, + error: "exit code 1", + time: { start: Date.now(), end: Date.now() }, + }, + }, + ], + }, + ], + }) + + await agent.loadSession({ sessionId, cwd, mcpServers: [] } as any) + await new Promise((r) => setTimeout(r, 20)) + + const initial = sessionUpdates + .filter((u) => u.sessionId === sessionId) + .map((u) => u.update) + .find((u) => u.sessionUpdate === "tool_call" && "toolCallId" in u && u.toolCallId === "call_err") + expect(initial).toBeDefined() + expect((initial as any).status).toBe("failed") + stop() + }, + }) + }) + test("clears bash snapshot marker on pending state", async () => { await using tmp = await tmpdir() await Instance.provide({