From 044ca11790cb4d486bd29b021824f8086607cd3c Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Fri, 3 Jul 2026 01:57:12 -0500 Subject: [PATCH] refactor(opencode): tighten code-mode adapter surface --- packages/opencode/src/tool/code-mode.ts | 106 ++++++----- packages/opencode/test/tool/code-mode.test.ts | 166 +----------------- 2 files changed, 59 insertions(+), 213 deletions(-) diff --git a/packages/opencode/src/tool/code-mode.ts b/packages/opencode/src/tool/code-mode.ts index aab2463e1c..0b24aab186 100644 --- a/packages/opencode/src/tool/code-mode.ts +++ b/packages/opencode/src/tool/code-mode.ts @@ -34,16 +34,16 @@ export const Parameters = Schema.Struct({ }), }) -export type CallEntry = { tool: string; status: "running" | "completed" | "error"; input?: Record } +type CallEntry = { tool: string; status: "running" | "completed" | "error"; input?: Record } type Metadata = { toolCalls: CallEntry[] error?: boolean } -export type Attachment = NonNullable[number] +type Attachment = NonNullable[number] -export type CatalogEntry = { +type CatalogEntry = { path: string key: string server: string @@ -62,7 +62,7 @@ function fallbackInputSchema(tool: AITool): JsonSchema { return { type: "object", properties: {} } } -export function groupByServer( +function groupByServer( mcpTools: Record, servers: readonly string[], mcpDefs: Record = {}, @@ -70,7 +70,8 @@ export function groupByServer( const byLongest = [...servers].sort((a, b) => b.length - a.length) const groups = new Map() for (const key of Object.keys(mcpTools).sort((a, b) => a.localeCompare(b))) { - const server = byLongest.find((name) => key.startsWith(name + "_")) ?? (key.includes("_") ? key.slice(0, key.indexOf("_")) : key) + const server = + byLongest.find((name) => key.startsWith(name + "_")) ?? (key.includes("_") ? key.slice(0, key.indexOf("_")) : key) const local = server && key.startsWith(server + "_") ? key.slice(server.length + 1) : key const def = mcpDefs[key] const entry: CatalogEntry = { @@ -95,7 +96,9 @@ export function describeCatalog( ): string { return CodeMode.make({ tools: toolTree( - [...groupByServer(mcpTools, servers, mcpDefs).values()].flat().filter((entry) => entry.tool.execute !== undefined), + [...groupByServer(mcpTools, servers, mcpDefs).values()] + .flat() + .filter((entry) => entry.tool.execute !== undefined), () => () => Effect.fail(toolError("Tool preview is not executable.")), ), }).instructions() @@ -124,7 +127,7 @@ const mediaMarker = (files: number, images: number) => { return `[${files} ${noun}${files === 1 ? "" : "s"} attached to the result]` } -export function toSandboxResult(raw: unknown, collect: (attachment: Attachment) => void): unknown { +function toProgramValue(raw: unknown, collect: (attachment: Attachment) => void): unknown { if (raw === null || typeof raw !== "object") return raw const record = raw as { structuredContent?: unknown; content?: unknown } const content = Array.isArray(record.content) ? record.content : [] @@ -182,22 +185,6 @@ export function toSandboxResult(raw: unknown, collect: (attachment: Attachment) return raw } -export function withLogs(output: string, logs: ReadonlyArray = []): string { - if (logs.length === 0) return output - const section = "Logs:\n" + logs.join("\n") - return output.length > 0 ? `${output}\n\n${section}` : section -} - -export function formatValue(value: unknown): string { - if (typeof value === "string") return value - if (value === undefined) return "undefined" - try { - return JSON.stringify(value, null, 2) ?? String(value) - } catch { - return String(value) - } -} - type Run = (input: unknown) => Effect.Effect function toolTree(catalog: readonly CatalogEntry[], run: (entry: CatalogEntry) => Run) { @@ -214,15 +201,6 @@ function toolTree(catalog: readonly CatalogEntry[], run: (entry: CatalogEntry) = return tree } -const toCatchable = (effect: Effect.Effect) => - effect.pipe( - Effect.catchCause((cause) => { - if (Cause.hasInterruptsOnly(cause)) return Effect.interrupt - const error = Cause.squash(cause) - return Effect.fail(toolError(error instanceof Error ? error.message : String(error), error)) - }), - ) - const invokeChildTool = Effect.fn("CodeMode.invokeChildTool")(function* (input: { plugin: Plugin.Interface entry: CatalogEntry @@ -289,23 +267,28 @@ export const CodeModeTool = Tool.define( const calls: CallEntry[] = [] const attachments: Attachment[] = [] const collect = (attachment: Attachment) => void attachments.push(attachment) - const publish = () => ctx.metadata({ title: CODE_MODE_TOOL, metadata: { toolCalls: calls.map((c) => ({ ...c })) } }) + const publish = () => + ctx.metadata({ title: CODE_MODE_TOOL, metadata: { toolCalls: calls.map((c) => ({ ...c })) } }) let childCalls = 0 const callTool = (entry: CatalogEntry) => (input: unknown) => - toCatchable( - Effect.gen(function* () { - childCalls += 1 - const raw = yield* invokeChildTool({ - plugin, - entry, - args: input ?? {}, - callID: `${ctx.callID ?? entry.key}/${childCalls}`, - options: { toolCallId: ctx.callID ?? entry.key, abortSignal: ctx.abort, messages: [] }, - ctx, - execute: entry.tool.execute!, - }) - return toSandboxResult(raw, collect) + Effect.gen(function* () { + childCalls += 1 + const raw = yield* invokeChildTool({ + plugin, + entry, + args: input ?? {}, + callID: `${ctx.callID ?? entry.key}/${childCalls}`, + options: { toolCallId: ctx.callID ?? entry.key, abortSignal: ctx.abort, messages: [] }, + ctx, + execute: entry.tool.execute!, + }) + return toProgramValue(raw, collect) + }).pipe( + Effect.catchCause((cause) => { + if (Cause.hasInterruptsOnly(cause)) return Effect.interrupt + const error = Cause.squash(cause) + return Effect.fail(toolError(error instanceof Error ? error.message : String(error), error)) }), ) @@ -343,20 +326,31 @@ export const CodeModeTool = Tool.define( const result = yield* Effect.raceFirst(runtime.execute(params.code), cancelled) const logs = result.logs ?? [] const attached = attachments.length > 0 ? { attachments } : {} - + const hints = result.ok + ? [] + : (result.error.suggestions ?? []).filter((hint) => !result.error.message.includes(hint)) + const metadata: Metadata = result.ok ? { toolCalls: calls } : { toolCalls: calls, error: true } + let output: string if (result.ok) { - return { - title: CODE_MODE_TOOL, - metadata: { toolCalls: calls }, - output: withLogs(formatValue(result.value), logs), - ...attached, - } satisfies Tool.ExecuteResult + if (typeof result.value === "string") output = result.value + else if (result.value === undefined) output = "undefined" + else { + try { + output = JSON.stringify(result.value, null, 2) ?? String(result.value) + } catch { + output = String(result.value) + } + } + } else { + output = [result.error.message, ...hints].join("\n") } - const hints = (result.error.suggestions ?? []).filter((hint) => !result.error.message.includes(hint)) + if (logs.length > 0) + output = output.length > 0 ? `${output}\n\nLogs:\n${logs.join("\n")}` : `Logs:\n${logs.join("\n")}` + return { title: CODE_MODE_TOOL, - metadata: { toolCalls: calls, error: true }, - output: withLogs([result.error.message, ...hints].join("\n"), logs), + metadata, + output, ...attached, } satisfies Tool.ExecuteResult }), diff --git a/packages/opencode/test/tool/code-mode.test.ts b/packages/opencode/test/tool/code-mode.test.ts index 79c27f8f33..4e7ce3b066 100644 --- a/packages/opencode/test/tool/code-mode.test.ts +++ b/packages/opencode/test/tool/code-mode.test.ts @@ -4,11 +4,6 @@ import { CodeModeTool, Parameters, describeCatalog, - formatValue, - groupByServer, - toSandboxResult, - withLogs, - type Attachment, } from "@/tool/code-mode" import type { Tool as MCPToolDef } from "@modelcontextprotocol/sdk/types.js" import type { PermissionV1 } from "@opencode-ai/core/v1/permission" @@ -111,27 +106,18 @@ describe("code mode execute", () => { }) test("groups multi-underscore server names by longest matching prefix", () => { - const groups = groupByServer({ my_server_do_thing: mcpTool("do_thing", () => "") }, ["my_server"]) - expect([...groups.keys()]).toEqual(["my_server"]) - expect(groups.get("my_server")![0]).toMatchObject({ - path: "my_server.do_thing", - local: "do_thing", - key: "my_server_do_thing", - }) + const description = describeFor({ my_server_do_thing: mcpTool("do_thing", () => "") }, {}, ["my_server"]) + expect(description).toContain("- my_server (1 tool)") + expect(description).toContain("tools.my_server.do_thing(") }) test("groupByServer uses the whole key as the server name when it has no underscore", () => { - const groups = groupByServer({ standalone: mcpTool("standalone", () => "") }, []) - expect([...groups.keys()]).toEqual(["standalone"]) - expect(groups.get("standalone")![0]).toMatchObject({ - path: "standalone.standalone", - server: "standalone", - local: "standalone", - key: "standalone", - }) + const description = describeFor({ standalone: mcpTool("standalone", () => "") }, {}, []) + expect(description).toContain("- standalone (1 tool)") + expect(description).toContain("tools.standalone.standalone(") }) - test("groupByServer carries the raw MCP schemas for rendering", () => { + test("describeCatalog carries the raw MCP schemas for rendering", () => { const defs: Record = { weather_current: { name: "current", @@ -139,10 +125,8 @@ describe("code mode execute", () => { outputSchema: { type: "object", properties: { tempC: { type: "number" } }, required: ["tempC"] }, } as any, } - const groups = groupByServer({ weather_current: mcpTool("current", () => "") }, ["weather"], defs) - const entry = groups.get("weather")![0]! - expect(entry.inputSchema).toEqual(defs.weather_current!.inputSchema as any) - expect(entry.outputSchema).toEqual(defs.weather_current!.outputSchema as any) + const description = describeFor({ weather_current: mcpTool("current", () => "") }, defs, ["weather"]) + expect(description).toContain("tools.weather.current(input: { city: string }): Promise<{ tempC: number }>") }) test("the static base description carries no catalog; the registry appends it", async () => { @@ -681,135 +665,3 @@ describe("code mode permission visibility", () => { expect(Object.keys(visible)).toEqual(["b_tool", "c_tool"]) }) }) - -describe("toSandboxResult", () => { - const collector = () => { - const attachments: Attachment[] = [] - return { attachments, collect: (a: Attachment) => void attachments.push(a) } - } - - test("prefers structuredContent over text", () => { - const { collect } = collector() - expect(toSandboxResult({ structuredContent: { x: 1 }, content: [{ type: "text", text: "hi" }] }, collect)).toEqual( - { x: 1 }, - ) - }) - - test("joins text content when no structured content is present", () => { - const { collect } = collector() - expect( - toSandboxResult( - { content: [{ type: "text", text: "one" }, { type: "text", text: "two" }] }, - collect, - ), - ).toBe("one\ntwo") - }) - - test("passes non-MCP values through untouched", () => { - const { collect } = collector() - expect(toSandboxResult("raw", collect)).toBe("raw") - expect(toSandboxResult(42, collect)).toBe(42) - expect(toSandboxResult(null, collect)).toBeNull() - expect(toSandboxResult({ some: "object" }, collect)).toEqual({ some: "object" }) - }) - - test("strips media into the accumulator; text stays the sandbox value", () => { - const { attachments, collect } = collector() - const value = toSandboxResult( - { - content: [ - { type: "text", text: "see image" }, - { type: "image", data: "AAAA", mimeType: "image/png" }, - ], - }, - collect, - ) - expect(value).toBe("see image") - expect(attachments).toEqual([{ type: "file", mime: "image/png", url: "data:image/png;base64,AAAA" }]) - }) - - test("a media-only result yields a marker; counts and nouns follow the content", () => { - const one = collector() - expect(toSandboxResult({ content: [{ type: "image", data: "A", mimeType: "image/png" }] }, one.collect)).toBe( - "[1 image attached to the result]", - ) - - const two = collector() - expect( - toSandboxResult( - { - content: [ - { type: "image", data: "A", mimeType: "image/png" }, - { type: "image", data: "B", mimeType: "image/jpeg" }, - ], - }, - two.collect, - ), - ).toBe("[2 images attached to the result]") - expect(two.attachments).toHaveLength(2) - - const mixed = collector() - expect( - toSandboxResult( - { - content: [ - { type: "image", data: "A", mimeType: "image/png" }, - { type: "audio", data: "B", mimeType: "audio/wav" }, - ], - }, - mixed.collect, - ), - ).toBe("[2 files attached to the result]") - }) - - test("extracts embedded resources: text inline, blobs as attachments with filenames", () => { - const { attachments, collect } = collector() - const value = toSandboxResult( - { - content: [ - { type: "resource", resource: { uri: "file:///tmp/notes.txt", mimeType: "text/plain", text: "note text" } }, - { type: "resource", resource: { uri: "file:///tmp/doc.pdf", mimeType: "application/pdf", blob: "PDF" } }, - ], - }, - collect, - ) - expect(value).toBe("note text") - expect(attachments).toEqual([ - { type: "file", mime: "application/pdf", url: "data:application/pdf;base64,PDF", filename: "doc.pdf" }, - ]) - }) - - test("collects resource_link blocks as external-URL attachments", () => { - const { attachments, collect } = collector() - const value = toSandboxResult( - { content: [{ type: "resource_link", uri: "https://example.com/report.csv", mimeType: "text/csv" }] }, - collect, - ) - expect(value).toBe("[1 file attached to the result]") - expect(attachments).toEqual([ - { type: "file", mime: "text/csv", url: "https://example.com/report.csv", filename: "report.csv" }, - ]) - }) - - test("an MCP-shaped result with nothing extractable becomes null", () => { - const { collect } = collector() - expect(toSandboxResult({ content: [] }, collect)).toBeNull() - expect(toSandboxResult({ content: [{ type: "mystery" }] }, collect)).toBeNull() - }) -}) - -describe("formatting helpers", () => { - test("formatValue", () => { - expect(formatValue("text")).toBe("text") - expect(formatValue({ a: 1 })).toBe(JSON.stringify({ a: 1 }, null, 2)) - expect(formatValue(null)).toBe("null") - expect(formatValue(undefined)).toBe("undefined") - }) - - test("withLogs", () => { - expect(withLogs("result", [])).toBe("result") - expect(withLogs("result")).toBe("result") - expect(withLogs("result", ["a", "[warn] b"])).toBe("result\n\nLogs:\na\n[warn] b") - expect(withLogs("", ["[error] boom"])).toBe("Logs:\n[error] boom") - }) -})