refactor(opencode): narrow MCP resource fix
This commit is contained in:
parent
08609209bb
commit
346c9615c4
4 changed files with 40 additions and 242 deletions
|
|
@ -219,10 +219,10 @@ export const toModelMessagesEffect = Effect.fnUntraced(function* (
|
||||||
type: "text",
|
type: "text",
|
||||||
text: part.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 (
|
if (
|
||||||
part.type === "file" &&
|
part.type === "file" &&
|
||||||
part.mime.split(";")[0]?.trim().toLowerCase() !== "text/plain" &&
|
part.mime !== "text/plain" &&
|
||||||
part.mime !== "application/x-directory" &&
|
part.mime !== "application/x-directory" &&
|
||||||
part.source?.type !== "resource"
|
part.source?.type !== "resource"
|
||||||
) {
|
) {
|
||||||
|
|
|
||||||
|
|
@ -728,7 +728,7 @@ export const layer = Layer.effect(
|
||||||
const exit = yield* mcp.readResource(clientName, uri).pipe(Effect.exit)
|
const exit = yield* mcp.readResource(clientName, uri).pipe(Effect.exit)
|
||||||
if (Exit.isSuccess(exit)) {
|
if (Exit.isSuccess(exit)) {
|
||||||
const content = exit.value
|
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]
|
const items = Array.isArray(content.contents) ? content.contents : [content.contents]
|
||||||
for (const c of items) {
|
for (const c of items) {
|
||||||
if ("text" in c && c.text) {
|
if ("text" in c && c.text) {
|
||||||
|
|
@ -740,40 +740,13 @@ export const layer = Layer.effect(
|
||||||
text: c.text,
|
text: c.text,
|
||||||
})
|
})
|
||||||
} else if ("blob" in c && c.blob) {
|
} else if ("blob" in c && c.blob) {
|
||||||
const mime = ("mimeType" in c ? c.mimeType : undefined) ?? part.mime
|
const mime = "mimeType" in c ? c.mimeType : 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}]`,
|
|
||||||
})
|
|
||||||
}
|
|
||||||
pieces.push({
|
pieces.push({
|
||||||
messageID: info.id,
|
messageID: info.id,
|
||||||
sessionID: input.sessionID,
|
sessionID: input.sessionID,
|
||||||
type: "file",
|
type: "text",
|
||||||
mime,
|
synthetic: true,
|
||||||
filename: part.filename,
|
text: `[Binary content: ${mime}]`,
|
||||||
url,
|
|
||||||
source: supported || mediaType === "text/plain" ? undefined : part.source,
|
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -1019,7 +992,7 @@ export const layer = Layer.effect(
|
||||||
)
|
)
|
||||||
|
|
||||||
const parts = yield* Effect.forEach(resolvedParts, (part) =>
|
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(
|
? image.normalize(part).pipe(
|
||||||
Effect.catchIf(
|
Effect.catchIf(
|
||||||
(error) => error instanceof Image.ResizerUnavailableError,
|
(error) => error instanceof Image.ResizerUnavailableError,
|
||||||
|
|
|
||||||
|
|
@ -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 messageID = "m-user"
|
||||||
const input: SessionV1.WithParts[] = [
|
const input: SessionV1.WithParts[] = [
|
||||||
{
|
{
|
||||||
|
|
@ -334,34 +334,14 @@ describe("session.message-v2.toModelMessage", () => {
|
||||||
{
|
{
|
||||||
...basePart(messageID, "p2"),
|
...basePart(messageID, "p2"),
|
||||||
type: "file",
|
type: "file",
|
||||||
mime: "text/markdown",
|
mime: "application/json",
|
||||||
filename: "guide",
|
filename: "status",
|
||||||
url: "opencode-fixture://guide",
|
url: "status://info",
|
||||||
source: {
|
source: {
|
||||||
type: "resource",
|
type: "resource",
|
||||||
clientName: "resource-only-fixture",
|
clientName: "resource-only-fixture",
|
||||||
uri: "opencode-fixture://guide",
|
uri: "status://info",
|
||||||
text: { value: "@fixture-guide", start: 0, end: 14 },
|
text: { value: "@status", start: 0, end: 7 },
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
|
||||||
...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 },
|
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
] as SessionV1.Part[],
|
] as SessionV1.Part[],
|
||||||
|
|
|
||||||
|
|
@ -108,32 +108,28 @@ function errorTool(parts: SessionV1.Part[]) {
|
||||||
return part?.state.status === "error" ? (part as ErrorToolPart) : undefined
|
return part?.state.status === "error" ? (part as ErrorToolPart) : undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
function makeMcp(readResource: MCP.Interface["readResource"] = () => Effect.succeed(undefined)) {
|
const mcp = Layer.succeed(
|
||||||
return Layer.succeed(
|
MCP.Service,
|
||||||
MCP.Service,
|
MCP.Service.of({
|
||||||
MCP.Service.of({
|
status: () => Effect.succeed({}),
|
||||||
status: () => Effect.succeed({}),
|
clients: () => Effect.succeed({}),
|
||||||
clients: () => Effect.succeed({}),
|
tools: () => Effect.succeed({}),
|
||||||
tools: () => Effect.succeed({}),
|
prompts: () => Effect.succeed({}),
|
||||||
prompts: () => Effect.succeed({}),
|
resources: () => Effect.succeed({}),
|
||||||
resources: () => Effect.succeed({}),
|
add: () => Effect.succeed({ status: { status: "disabled" as const } }),
|
||||||
add: () => Effect.succeed({ status: { status: "disabled" as const } }),
|
connect: () => Effect.void,
|
||||||
connect: () => Effect.void,
|
disconnect: () => Effect.void,
|
||||||
disconnect: () => Effect.void,
|
getPrompt: () => Effect.succeed(undefined),
|
||||||
getPrompt: () => Effect.succeed(undefined),
|
readResource: () => Effect.succeed(undefined),
|
||||||
readResource,
|
startAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"),
|
||||||
startAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"),
|
authenticate: () => 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"),
|
||||||
finishAuth: () => Effect.die("unexpected MCP auth in prompt-effect tests"),
|
removeAuth: () => Effect.void,
|
||||||
removeAuth: () => Effect.void,
|
supportsOAuth: () => Effect.succeed(false),
|
||||||
supportsOAuth: () => Effect.succeed(false),
|
hasStoredTokens: () => Effect.succeed(false),
|
||||||
hasStoredTokens: () => Effect.succeed(false),
|
getAuthStatus: () => Effect.succeed("not_authenticated" as const),
|
||||||
getAuthStatus: () => Effect.succeed("not_authenticated" as const),
|
}),
|
||||||
}),
|
)
|
||||||
)
|
|
||||||
}
|
|
||||||
|
|
||||||
const mcp = makeMcp()
|
|
||||||
|
|
||||||
const lsp = Layer.succeed(
|
const lsp = Layer.succeed(
|
||||||
LSP.Service,
|
LSP.Service,
|
||||||
|
|
@ -167,9 +163,7 @@ const blockingProcessor = Layer.succeed(
|
||||||
}),
|
}),
|
||||||
)
|
)
|
||||||
|
|
||||||
type PromptOptions = { processor?: "blocking"; mcp?: ReturnType<typeof makeMcp> }
|
function makePrompt(input?: { processor?: "blocking" }) {
|
||||||
|
|
||||||
function makePrompt(input?: PromptOptions) {
|
|
||||||
const deps = Layer.mergeAll(
|
const deps = Layer.mergeAll(
|
||||||
Session.defaultLayer,
|
Session.defaultLayer,
|
||||||
Snapshot.defaultLayer,
|
Snapshot.defaultLayer,
|
||||||
|
|
@ -182,7 +176,7 @@ function makePrompt(input?: PromptOptions) {
|
||||||
Config.defaultLayer,
|
Config.defaultLayer,
|
||||||
ProviderSvc.defaultLayer,
|
ProviderSvc.defaultLayer,
|
||||||
lsp,
|
lsp,
|
||||||
input?.mcp ?? mcp,
|
mcp,
|
||||||
FSUtil.defaultLayer,
|
FSUtil.defaultLayer,
|
||||||
BackgroundJob.defaultLayer,
|
BackgroundJob.defaultLayer,
|
||||||
status,
|
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))
|
return Layer.mergeAll(TestLLMServer.layer, makePrompt(input))
|
||||||
}
|
}
|
||||||
|
|
||||||
function makeHttpNoLLMServer(input?: PromptOptions) {
|
function makeHttpNoLLMServer(input?: { processor?: "blocking" }) {
|
||||||
return makePrompt(input)
|
return makePrompt(input)
|
||||||
}
|
}
|
||||||
|
|
||||||
const it = testEffect(makeHttp())
|
const it = testEffect(makeHttp())
|
||||||
const noLLMServer = testEffect(makeHttpNoLLMServer())
|
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 raceNoLLMServer = testEffect(makeHttpNoLLMServer({ processor: "blocking" }))
|
||||||
const unix = process.platform !== "win32" ? it.instance : it.instance.skip
|
const unix = process.platform !== "win32" ? it.instance : it.instance.skip
|
||||||
const unixNoLLMServer = process.platform !== "win32" ? noLLMServer.instance : noLLMServer.instance.skip
|
const unixNoLLMServer = process.platform !== "win32" ? noLLMServer.instance : noLLMServer.instance.skip
|
||||||
|
|
@ -2066,115 +2020,6 @@ noLLMServer.instance(
|
||||||
{ config: cfg },
|
{ 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(
|
noLLMServer.instance(
|
||||||
"keeps stored part order stable when file resolution is async",
|
"keeps stored part order stable when file resolution is async",
|
||||||
() =>
|
() =>
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue