From 8851e4de2bb7ed34c568b360ef5859155e9078a7 Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Wed, 3 Jun 2026 19:36:27 +0530 Subject: [PATCH] fix(acp): clean read tool display content (#30569) --- packages/opencode/src/acp/tool.ts | 16 ++++++- packages/opencode/src/tool/read.ts | 53 +++++++++++++++++++-- packages/opencode/test/acp/event.test.ts | 60 ++++++++++++++++++++++-- packages/opencode/test/acp/tool.test.ts | 40 ++++++++++++++++ packages/opencode/test/tool/read.test.ts | 17 +++++++ 5 files changed, 179 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/acp/tool.ts b/packages/opencode/src/acp/tool.ts index bf16a78de..0e8b4f098 100644 --- a/packages/opencode/src/acp/tool.ts +++ b/packages/opencode/src/acp/tool.ts @@ -98,12 +98,14 @@ export function toLocations(toolName: string, input: ToolInput): ToolCallLocatio } export function completedToolContent(toolName: string, state: CompletedToolState): ToolCallContent[] { + const text = + toolName.toLocaleLowerCase() === "read" ? (readDisplayText(state.metadata) ?? state.output) : state.output const content: ToolCallContent[] = [ { type: "content", content: { type: "text", - text: state.output, + text, }, }, ] @@ -288,6 +290,18 @@ function diffContent(input: ToolInput): ToolCallContent[] { ] } +function readDisplayText(metadata: unknown) { + if (!metadata || typeof metadata !== "object") return undefined + const display = (metadata as Record).display + if (!display || typeof display !== "object") return undefined + const info = display as Record + if (info.type === "file") return stringValue(info.text) + if (info.type === "directory" && Array.isArray(info.entries)) { + return info.entries.filter((item): item is string => typeof item === "string").join("\n") + } + return undefined +} + function dataUrlImage(attachment: ToolAttachment) { const match = stringValue(attachment.url)?.match(/^data:([^;,]+)(?:;[^,]*)*;base64,(.*)$/) const mime = match?.[1] ?? stringValue(attachment.mime) diff --git a/packages/opencode/src/tool/read.ts b/packages/opencode/src/tool/read.ts index 75526f258..be6557e3c 100644 --- a/packages/opencode/src/tool/read.ts +++ b/packages/opencode/src/tool/read.ts @@ -36,7 +36,37 @@ export const Parameters = Schema.Struct({ }), }) -export const ReadTool = Tool.define( +type Display = + | { + type: "directory" + path: string + entries: string[] + offset: number + totalEntries: number + truncated: boolean + } + | { + type: "file" + path: string + text: string + lineStart: number + lineEnd: number + totalLines: number + truncated: boolean + } + +type Metadata = { + preview: string + truncated: boolean + loaded: string[] + display?: Display +} + +export const ReadTool = Tool.define< + typeof Parameters, + Metadata, + FSUtil.Service | Instruction.Service | LSP.Service | Reference.Service | Scope.Scope +>( "read", Effect.gen(function* () { const fs = yield* FSUtil.Service @@ -200,7 +230,7 @@ export const ReadTool = Tool.define( const run = Effect.fn("ReadTool.execute")(function* ( params: Schema.Schema.Type, - ctx: Tool.Context, + ctx: Tool.Context, ) { const instance = yield* InstanceState.context let filepath = params.filePath @@ -258,6 +288,14 @@ export const ReadTool = Tool.define( preview: sliced.slice(0, 20).join("\n"), truncated, loaded: [] as string[], + display: { + type: "directory" as const, + path: filepath, + entries: sliced, + offset, + totalEntries: items.length, + truncated, + }, }, } } @@ -328,6 +366,15 @@ export const ReadTool = Tool.define( preview: file.raw.slice(0, 20).join("\n"), truncated, loaded: loaded.map((item) => item.filepath), + display: { + type: "file" as const, + path: filepath, + text: file.raw.join("\n"), + lineStart: file.offset, + lineEnd: last, + totalLines: file.count, + truncated, + }, }, } }) @@ -335,7 +382,7 @@ export const ReadTool = Tool.define( return { description: DESCRIPTION, parameters: Parameters, - execute: (params: Schema.Schema.Type, ctx: Tool.Context) => + execute: (params: Schema.Schema.Type, ctx: Tool.Context) => run(params, ctx).pipe(Effect.orDie), } }), diff --git a/packages/opencode/test/acp/event.test.ts b/packages/opencode/test/acp/event.test.ts index c79baf035..6edd7f0fb 100644 --- a/packages/opencode/test/acp/event.test.ts +++ b/packages/opencode/test/acp/event.test.ts @@ -251,6 +251,11 @@ function completedTool( callID: string, output = "done", attachments: Extract["attachments"] = [], + options: { + readonly tool?: string + readonly input?: Record + readonly metadata?: Record + } = {}, ) { return { id: `part_${callID}`, @@ -258,13 +263,13 @@ function completedTool( messageID: `msg_${callID}`, type: "tool", callID, - tool: "bash", + tool: options.tool ?? "bash", state: { status: "completed", - input: { cmd: "printf done" }, + input: options.input ?? { cmd: "printf done" }, output, title: "bash", - metadata: { exit: 0 }, + metadata: options.metadata ?? { exit: 0 }, time: { start: Date.now() - 1, end: Date.now() }, ...(attachments.length ? { attachments } : {}), }, @@ -605,6 +610,55 @@ describe("acp event routing", () => { }) }) + it("emits clean read display content and preserves rawOutput", async () => { + const harness = createHarness() + await Effect.runPromise(harness.session.create({ id: "ses_read", cwd: "/workspace" })) + const output = [ + "/workspace/file.ts", + "file", + "", + "1: import { value } from './value'", + "2: export { value }", + "", + "(End of file - total 2 lines)", + "", + ].join("\n") + const metadata = { + display: { + type: "file", + path: "/workspace/file.ts", + text: "import { value } from './value'\nexport { value }", + lineStart: 1, + lineEnd: 2, + totalLines: 2, + truncated: false, + }, + } + + await harness.subscription.handle( + toolUpdated( + completedTool("ses_read", "call_read", output, [], { + tool: "read", + input: { filePath: "/workspace/file.ts" }, + metadata, + }), + ), + ) + + expect(harness.updates.at(-1)?.update).toMatchObject({ + sessionUpdate: "tool_call_update", + toolCallId: "call_read", + status: "completed", + content: [ + { + type: "content", + content: { type: "text", text: "import { value } from './value'\nexport { value }" }, + }, + ], + rawOutput: { output, metadata }, + }) + }) + it("emits error tool output", async () => { const harness = createHarness() await Effect.runPromise(harness.session.create({ id: "ses_error", cwd: "/workspace" })) diff --git a/packages/opencode/test/acp/tool.test.ts b/packages/opencode/test/acp/tool.test.ts index bc3c7c359..e7ad1c1b1 100644 --- a/packages/opencode/test/acp/tool.test.ts +++ b/packages/opencode/test/acp/tool.test.ts @@ -104,6 +104,46 @@ describe("acp tool conversion", () => { ]) }) + test("uses clean read display text for completed content", () => { + const output = [ + "/tmp/file.ts", + "file", + "", + "7: first", + "8: second", + "", + "(End of file - total 8 lines)", + "", + ].join("\n") + const state = { + status: "completed" as const, + input: { filePath: "/tmp/file.ts" }, + output, + metadata: { + display: { + type: "file", + path: "/tmp/file.ts", + text: "first\nsecond", + lineStart: 7, + lineEnd: 8, + totalLines: 8, + truncated: false, + }, + }, + } + + expect(completedToolContent("read", state)).toEqual([ + { + type: "content", + content: { type: "text", text: "first\nsecond" }, + }, + ]) + expect(completedToolRawOutput(state)).toEqual({ + output, + metadata: state.metadata, + }) + }) + test("builds completed raw output with optional metadata and attachments", () => { const attachments = [ { diff --git a/packages/opencode/test/tool/read.test.ts b/packages/opencode/test/tool/read.test.ts index 513574393..fe9cd0680 100644 --- a/packages/opencode/test/tool/read.test.ts +++ b/packages/opencode/test/tool/read.test.ts @@ -428,6 +428,15 @@ describe("tool.read truncation", () => { const result = yield* run({ filePath: path.join(test.directory, "small.txt") }) expect(result.metadata.truncated).toBe(false) expect(result.output).toContain("End of file") + expect(result.metadata.display).toMatchObject({ + type: "file", + path: path.join(test.directory, "small.txt"), + text: "hello world", + lineStart: 1, + lineEnd: 1, + totalLines: 1, + truncated: false, + }) }), ) @@ -495,6 +504,14 @@ describe("tool.read truncation", () => { const result = yield* exec(dir, { filePath: path.join(dir, "dir"), offset: 6, limit: 5 }) expect(result.metadata.truncated).toBe(false) expect(result.output).not.toContain("Showing 5 of 10 entries") + expect(result.metadata.display).toMatchObject({ + type: "directory", + path: path.join(dir, "dir"), + entries: ["file-5.txt", "file-6.txt", "file-7.txt", "file-8.txt", "file-9.txt"], + offset: 6, + totalEntries: 10, + truncated: false, + }) }), )