From 346c9615c48fd5caec2eb4052c26e35971da6c5a Mon Sep 17 00:00:00 2001 From: Aiden Cline Date: Thu, 11 Jun 2026 21:29:29 -0500 Subject: [PATCH] refactor(opencode): narrow MCP resource fix --- packages/opencode/src/session/message-v2.ts | 4 +- packages/opencode/src/session/prompt.ts | 39 +--- .../opencode/test/session/message-v2.test.ts | 32 +-- packages/opencode/test/session/prompt.test.ts | 207 +++--------------- 4 files changed, 40 insertions(+), 242 deletions(-) diff --git a/packages/opencode/src/session/message-v2.ts b/packages/opencode/src/session/message-v2.ts index 3cff392a12b..cb9e66304a9 100644 --- a/packages/opencode/src/session/message-v2.ts +++ b/packages/opencode/src/session/message-v2.ts @@ -219,10 +219,10 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* ( type: "text", text: part.text, }) - // Text, directories, and MCP references are resolved before provider conversion. + // text/plain and directory files are converted into text parts, ignore them if ( part.type === "file" && - part.mime.split(";")[0]?.trim().toLowerCase() !== "text/plain" && + part.mime !== "text/plain" && part.mime !== "application/x-directory" && part.source?.type !== "resource" ) { diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index cbebd8605b3..c5047a6059e 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -728,7 +728,7 @@ export const layer = Layer.effect( const exit = yield* mcp.readResource(clientName, uri).pipe(Effect.exit) if (Exit.isSuccess(exit)) { const content = exit.value - if (!content) throw new Error(`Failed to read MCP resource: ${clientName}/${uri}`) + if (!content) throw new Error(`Resource not found: ${clientName}/${uri}`) const items = Array.isArray(content.contents) ? content.contents : [content.contents] for (const c of items) { if ("text" in c && c.text) { @@ -740,40 +740,13 @@ export const layer = Layer.effect( text: c.text, }) } else if ("blob" in c && c.blob) { - const mime = ("mimeType" in c ? c.mimeType : undefined) ?? part.mime - const url = `data:${mime};base64,${c.blob}` - const mediaType = mime.split(";")[0]?.trim().toLowerCase() - if (mediaType === "text/plain") { - pieces.push({ - messageID: info.id, - sessionID: input.sessionID, - type: "text", - synthetic: true, - text: decodeDataUrl(url), - }) - } - const supported = - mediaType?.startsWith("image/") || - mediaType?.startsWith("audio/") || - mediaType?.startsWith("video/") || - mediaType === "application/pdf" - if (mediaType !== "text/plain" && !supported) { - pieces.push({ - messageID: info.id, - sessionID: input.sessionID, - type: "text", - synthetic: true, - text: `[Binary content: ${mime}]`, - }) - } + const mime = "mimeType" in c ? c.mimeType : part.mime pieces.push({ messageID: info.id, sessionID: input.sessionID, - type: "file", - mime, - filename: part.filename, - url, - source: supported || mediaType === "text/plain" ? undefined : part.source, + type: "text", + synthetic: true, + text: `[Binary content: ${mime}]`, }) } } @@ -1019,7 +992,7 @@ export const layer = Layer.effect( ) const parts = yield* Effect.forEach(resolvedParts, (part) => - part.type === "file" && part.source?.type !== "resource" && part.mime.startsWith("image/") + part.type === "file" && part.mime.startsWith("image/") ? image.normalize(part).pipe( Effect.catchIf( (error) => error instanceof Image.ResizerUnavailableError, diff --git a/packages/opencode/test/session/message-v2.test.ts b/packages/opencode/test/session/message-v2.test.ts index 29d4e596ec9..620cf5499b2 100644 --- a/packages/opencode/test/session/message-v2.test.ts +++ b/packages/opencode/test/session/message-v2.test.ts @@ -319,7 +319,7 @@ describe("session.message-v2.toModelMessage", () => { ]) }) - test("does not forward resolved MCP resource references as file downloads", async () => { + test("does not forward MCP resource references as file downloads", async () => { const messageID = "m-user" const input: SessionV1.WithParts[] = [ { @@ -334,34 +334,14 @@ describe("session.message-v2.toModelMessage", () => { { ...basePart(messageID, "p2"), type: "file", - mime: "text/markdown", - filename: "guide", - url: "opencode-fixture://guide", + mime: "application/json", + filename: "status", + url: "status://info", source: { type: "resource", clientName: "resource-only-fixture", - uri: "opencode-fixture://guide", - text: { value: "@fixture-guide", start: 0, end: 14 }, - }, - }, - { - ...basePart(messageID, "p3"), - type: "file", - mime: "text/plain; charset=utf-8", - filename: "guide.txt", - url: "data:text/plain; charset=utf-8;base64,IyBSZXNvdXJjZSBjb250ZW50cw==", - }, - { - ...basePart(messageID, "p4"), - type: "file", - mime: "application/octet-stream", - filename: "resource.bin", - url: "data:application/octet-stream;base64,AAEC", - source: { - type: "resource", - clientName: "resource-only-fixture", - uri: "opencode-fixture://binary", - text: { value: "@fixture-binary", start: 15, end: 30 }, + uri: "status://info", + text: { value: "@status", start: 0, end: 7 }, }, }, ] as SessionV1.Part[], diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index c674061322c..12fa11869e6 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -108,32 +108,28 @@ function errorTool(parts: SessionV1.Part[]) { return part?.state.status === "error" ? (part as ErrorToolPart) : undefined } -function makeMcp(readResource: MCP.Interface["readResource"] = () => Effect.succeed(undefined)) { - return Layer.succeed( - MCP.Service, - MCP.Service.of({ - status: () => Effect.succeed({}), - clients: () => Effect.succeed({}), - tools: () => Effect.succeed({}), - prompts: () => Effect.succeed({}), - resources: () => Effect.succeed({}), - add: () => Effect.succeed({ status: { status: "disabled" as const } }), - connect: () => Effect.void, - disconnect: () => Effect.void, - getPrompt: () => Effect.succeed(undefined), - readResource, - startAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"), - authenticate: () => Effect.die("unexpected MCP auth in prompt-effect tests"), - finishAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"), - removeAuth: () => Effect.void, - supportsOAuth: () => Effect.succeed(false), - hasStoredTokens: () => Effect.succeed(false), - getAuthStatus: () => Effect.succeed("not_authenticated" as const), - }), - ) -} - -const mcp = makeMcp() +const mcp = Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + tools: () => Effect.succeed({}), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + add: () => Effect.succeed({ status: { status: "disabled" as const } }), + connect: () => Effect.void, + disconnect: () => Effect.void, + getPrompt: () => Effect.succeed(undefined), + readResource: () => Effect.succeed(undefined), + startAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"), + authenticate: () => Effect.die("unexpected MCP auth in prompt-effect tests"), + finishAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"), + removeAuth: () => Effect.void, + supportsOAuth: () => Effect.succeed(false), + hasStoredTokens: () => Effect.succeed(false), + getAuthStatus: () => Effect.succeed("not_authenticated" as const), + }), +) const lsp = Layer.succeed( LSP.Service, @@ -167,9 +163,7 @@ const blockingProcessor = Layer.succeed( }), ) -type PromptOptions = { processor?: "blocking"; mcp?: ReturnType } - -function makePrompt(input?: PromptOptions) { +function makePrompt(input?: { processor?: "blocking" }) { const deps = Layer.mergeAll( Session.defaultLayer, Snapshot.defaultLayer, @@ -182,7 +176,7 @@ function makePrompt(input?: PromptOptions) { Config.defaultLayer, ProviderSvc.defaultLayer, lsp, - input?.mcp ?? mcp, + mcp, FSUtil.defaultLayer, BackgroundJob.defaultLayer, status, @@ -235,56 +229,16 @@ function makePrompt(input?: PromptOptions) { ) } -function makeHttp(input?: PromptOptions) { +function makeHttp(input?: { processor?: "blocking" }) { return Layer.mergeAll(TestLLMServer.layer, makePrompt(input)) } -function makeHttpNoLLMServer(input?: PromptOptions) { +function makeHttpNoLLMServer(input?: { processor?: "blocking" }) { return makePrompt(input) } const it = testEffect(makeHttp()) const noLLMServer = testEffect(makeHttpNoLLMServer()) -const resourceNoLLMServer = testEffect( - makeHttpNoLLMServer({ - mcp: makeMcp((_clientName, uri) => - Effect.succeed({ - contents: - uri === "opencode-fixture://guide" - ? [ - { - uri, - mimeType: "text/markdown", - text: "# MCP resource fixture", - }, - ] - : uri === "opencode-fixture://text-blob" - ? [ - { - uri, - mimeType: "text/plain; charset=utf-8", - blob: Buffer.from("MCP text blob fixture").toString("base64"), - }, - ] - : uri === "opencode-fixture://binary" - ? [ - { - uri, - mimeType: "application/octet-stream", - blob: "AAEC", - }, - ] - : [ - { - uri, - mimeType: "image/png", - blob: "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=", - }, - ], - }), - ), - }), -) const raceNoLLMServer = testEffect(makeHttpNoLLMServer({ processor: "blocking" })) const unix = process.platform !== "win32" ? it.instance : it.instance.skip const unixNoLLMServer = process.platform !== "win32" ? noLLMServer.instance : noLLMServer.instance.skip @@ -2066,115 +2020,6 @@ noLLMServer.instance( { config: cfg }, ) -resourceNoLLMServer.instance( - "resolves MCP resource text and blobs without treating custom URIs as files", - () => - Effect.gen(function* () { - const prompt = yield* SessionPrompt.Service - const sessions = yield* Session.Service - const session = yield* sessions.create({}) - - const message = yield* prompt.prompt({ - sessionID: session.id, - agent: "build", - noReply: true, - parts: [ - { - type: "file", - mime: "text/markdown", - url: "opencode-fixture://guide", - filename: "fixture-guide", - source: { - type: "resource", - clientName: "resource-only-fixture", - uri: "opencode-fixture://guide", - text: { value: "@fixture-guide", start: 0, end: 14 }, - }, - }, - { - type: "file", - mime: "text/plain", - url: "opencode-fixture://text-blob", - filename: "fixture-text-blob", - source: { - type: "resource", - clientName: "resource-only-fixture", - uri: "opencode-fixture://text-blob", - text: { value: "@fixture-text-blob", start: 15, end: 33 }, - }, - }, - { - type: "file", - mime: "application/octet-stream", - url: "opencode-fixture://binary", - filename: "fixture-binary", - source: { - type: "resource", - clientName: "resource-only-fixture", - uri: "opencode-fixture://binary", - text: { value: "@fixture-binary", start: 34, end: 49 }, - }, - }, - { - type: "file", - mime: "image/png", - url: "opencode-fixture://pixel", - filename: "fixture-pixel", - source: { - type: "resource", - clientName: "resource-only-fixture", - uri: "opencode-fixture://pixel", - text: { value: "@fixture-pixel", start: 15, end: 29 }, - }, - }, - ], - }) - - expect(message.parts.some((part) => part.type === "text" && part.text === "# MCP resource fixture")).toBe(true) - expect(message.parts.some((part) => part.type === "text" && part.text === "MCP text blob fixture")).toBe(true) - expect( - message.parts.some( - (part) => part.type === "text" && part.text === "[Binary content: application/octet-stream]", - ), - ).toBe(true) - expect( - message.parts.some( - (part) => - part.type === "file" && - !part.source && - part.mime === "text/plain; charset=utf-8" && - part.url.startsWith("data:text/plain; charset=utf-8;base64,"), - ), - ).toBe(true) - expect( - message.parts.some( - (part) => - part.type === "file" && - part.source?.type === "resource" && - part.mime === "application/octet-stream" && - part.url.startsWith("data:application/octet-stream;base64,"), - ), - ).toBe(true) - expect( - message.parts.some( - (part) => part.type === "file" && part.source?.type === "resource" && part.url === "opencode-fixture://pixel", - ), - ).toBe(true) - expect( - message.parts.some( - (part) => - part.type === "file" && - !part.source && - part.mime === "image/png" && - part.url.startsWith("data:image/png;base64,"), - ), - ).toBe(true) - - yield* sessions.remove(session.id) - }), - { config: cfg }, -) - noLLMServer.instance( "keeps stored part order stable when file resolution is async", () =>