diff --git a/packages/opencode/src/mcp/catalog.ts b/packages/opencode/src/mcp/catalog.ts index 0cd2238edffe..466185f09ac9 100644 --- a/packages/opencode/src/mcp/catalog.ts +++ b/packages/opencode/src/mcp/catalog.ts @@ -113,6 +113,8 @@ export function fetch( export const sanitize = (value: string) => value.replace(/[^a-zA-Z0-9_-]/g, "_") +export const toolName = (clientName: string, name: string) => sanitize(clientName) + "_" + sanitize(name) + export function prompts(client: Client, timeout?: number) { if (!client.getServerCapabilities()?.prompts) return Promise.resolve([]) return paginate( diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index f2aa9699bd23..3cab9fc93082 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -140,6 +140,7 @@ interface CreateResult { mcpClient?: MCPClient status: Status defs?: MCPToolDef[] + instructions?: string } interface AuthResult { @@ -155,11 +156,19 @@ interface State { status: Record clients: Record defs: Record + instructions: Record +} + +export interface ServerInstructions { + name: string + instructions: string + tools: string[] } export interface Interface { readonly status: () => Effect.Effect> readonly clients: () => Effect.Effect> + readonly instructions: () => Effect.Effect readonly tools: () => Effect.Effect> readonly prompts: () => Effect.Effect> readonly resources: (clientName?: string) => Effect.Effect> @@ -383,7 +392,7 @@ export const layer = Layer.effect( if (!listed) { return yield* Effect.fail(new Error("Failed to get tools")) } - return { mcpClient, status, defs: listed } satisfies CreateResult + return { mcpClient, status, defs: listed, instructions: mcpClient.getInstructions()?.trim() } satisfies CreateResult }).pipe( Effect.catchCause((cause) => Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore, Effect.andThen(Effect.failCause(cause))), @@ -430,6 +439,7 @@ export const layer = Layer.effect( if (s.clients[name] !== client) return delete s.clients[name] delete s.defs[name] + delete s.instructions[name] s.status[name] = { status: "failed", error: "Connection closed" } bridge.fork( Effect.logWarning("MCP connection closed", { server: name }).pipe( @@ -484,6 +494,7 @@ export const layer = Layer.effect( status: {}, clients: {}, defs: {}, + instructions: {}, } yield* Effect.forEach( @@ -505,6 +516,7 @@ export const layer = Layer.effect( if (result.mcpClient) { s.clients[key] = result.mcpClient s.defs[key] = result.defs! + if (result.instructions) s.instructions[key] = result.instructions watch(s, key, result.mcpClient, bridge, mcp.timeout) } }), @@ -516,6 +528,7 @@ export const layer = Layer.effect( const clients = Object.values(s.clients) s.clients = {} s.defs = {} + s.instructions = {} yield* Effect.forEach( clients, (client) => @@ -545,6 +558,7 @@ export const layer = Layer.effect( const client = s.clients[name] delete s.clients[name] delete s.defs[name] + delete s.instructions[name] if (!client) return Effect.void return Effect.tryPromise(() => client.close()).pipe(Effect.ignore) } @@ -554,6 +568,7 @@ export const layer = Layer.effect( name: string, client: MCPClient, listed: MCPToolDef[], + instructions: string | undefined, timeout?: number, ) { const bridge = yield* EffectBridge.make() @@ -561,6 +576,8 @@ export const layer = Layer.effect( s.status[name] = { status: "connected" } s.clients[name] = client s.defs[name] = listed + if (instructions) s.instructions[name] = instructions + else delete s.instructions[name] watch(s, name, client, bridge, timeout) if (previous) yield* Effect.tryPromise(() => previous.close()).pipe(Effect.ignore) return s.status[name] @@ -590,6 +607,18 @@ export const layer = Layer.effect( return s.clients }) + const instructions = Effect.fn("MCP.instructions")(function* () { + const s = yield* InstanceState.get(state) + return Object.entries(s.instructions) + .filter(([name]) => s.status[name]?.status === "connected") + .sort(([a], [b]) => a.localeCompare(b)) + .map(([name, item]) => ({ + name, + instructions: item, + tools: (s.defs[name] ?? []).map((tool) => McpCatalog.toolName(name, tool.name)), + })) + }) + const createAndStore = Effect.fn("MCP.createAndStore")(function* (name: string, mcp: ConfigMCPV1.Info) { const s = yield* InstanceState.get(state) const result = yield* create(name, mcp) @@ -601,7 +630,7 @@ export const layer = Layer.effect( return result.status } - return yield* storeClient(s, name, result.mcpClient, result.defs!, mcp.timeout) + return yield* storeClient(s, name, result.mcpClient, result.defs!, result.instructions, mcp.timeout) }) const add = Effect.fn("MCP.add")(function* (name: string, mcp: ConfigMCPV1.Info) { @@ -647,7 +676,7 @@ export const layer = Layer.effect( } const timeout = requestTimeout(s, clientName, mcpConfig, defaultTimeout) for (const mcpTool of listed) { - const key = McpCatalog.sanitize(clientName) + "_" + McpCatalog.sanitize(mcpTool.name) + const key = McpCatalog.toolName(clientName, mcpTool.name) result[key] = McpCatalog.convertTool(mcpTool, client, timeout) } } @@ -855,7 +884,7 @@ export const layer = Layer.effect( const s = yield* InstanceState.get(state) yield* auth.clearOAuthState(mcpName) - return yield* storeClient(s, mcpName, client, listed, mcpConfig.timeout) + return yield* storeClient(s, mcpName, client, listed, client.getInstructions()?.trim(), mcpConfig.timeout) } const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName) @@ -942,6 +971,7 @@ export const layer = Layer.effect( return Service.of({ status, clients, + instructions, tools, prompts, resources, diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index af2ded7488a3..e24bfd3df23e 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1356,13 +1356,19 @@ export const layer = Layer.effect( yield* plugin.trigger("experimental.chat.messages.transform", {}, { messages: msgs }) - const [skills, env, instructions, modelMsgs] = yield* Effect.all([ + const [skills, env, instructions, mcpInstructions, modelMsgs] = yield* Effect.all([ sys.skills(agent), sys.environment(model), instruction.system().pipe(Effect.orDie), + sys.mcp(agent, session.permission), MessageV2.toModelMessagesEffect(msgs, model), ]) - const system = [...env, ...instructions, ...(skills ? [skills] : [])] + const system = [ + ...env, + ...instructions, + ...(mcpInstructions ? [mcpInstructions] : []), + ...(skills ? [skills] : []), + ] const format = lastUser.format ?? { type: "text" as const } if (format.type === "json_schema") system.push(STRUCTURED_OUTPUT_SYSTEM_PROMPT) const result = yield* handle.process({ diff --git a/packages/opencode/src/session/system.ts b/packages/opencode/src/session/system.ts index 49b79018579b..0158690df398 100644 --- a/packages/opencode/src/session/system.ts +++ b/packages/opencode/src/session/system.ts @@ -20,6 +20,8 @@ import { AbsolutePath } from "@opencode-ai/core/schema" import { Location } from "@opencode-ai/core/location" import { LocationServiceMap } from "@opencode-ai/core/location-layer" import { Reference } from "@opencode-ai/core/reference" +import { MCP } from "@/mcp" +import { PermissionV1 } from "@opencode-ai/core/v1/permission" export function provider(model: Provider.Model) { if (model.api.id.includes("gpt-4") || model.api.id.includes("o1") || model.api.id.includes("o3")) @@ -40,6 +42,7 @@ export function provider(model: Provider.Model) { export interface Interface { readonly environment: (model: Provider.Model) => Effect.Effect readonly skills: (agent: Agent.Info) => Effect.Effect + readonly mcp: (agent: Agent.Info, permission?: PermissionV1.Ruleset) => Effect.Effect } export class Service extends Context.Service()("@opencode/SystemPrompt") {} @@ -48,6 +51,7 @@ export const layer = Layer.effect( Service, Effect.gen(function* () { const skill = yield* Skill.Service + const mcp = yield* MCP.Service const locations = yield* LocationServiceMap return Service.of({ @@ -102,14 +106,36 @@ export const layer = Layer.effect( Skill.fmt(list, { verbose: true }), ].join("\n") }), + + mcp: Effect.fn("SystemPrompt.mcp")(function* (agent: Agent.Info, permission?: PermissionV1.Ruleset) { + const ruleset = Permission.merge(agent.permission, permission ?? []) + const instructions = (yield* mcp.instructions()).filter( + (item) => item.tools.length === 0 || Permission.disabled(item.tools, ruleset).size < item.tools.length, + ) + if (instructions.length === 0) return + + return [ + "", + ...instructions.flatMap((item) => [ + ` `, + ...item.instructions.split("\n").map((line) => ` ${line}`), + " ", + ]), + "", + ].join("\n") + }), }) }), ) -export const defaultLayer = layer.pipe(Layer.provide(Skill.defaultLayer), Layer.provide(LocationServiceMap.layer)) +export const defaultLayer = layer.pipe( + Layer.provide(Skill.defaultLayer), + Layer.provide(MCP.defaultLayer), + Layer.provide(LocationServiceMap.layer), +) const locationServiceMapNode = LayerNode.make(LocationServiceMap.layer, []) -export const node = LayerNode.make(layer, [Skill.node, locationServiceMapNode]) +export const node = LayerNode.make(layer, [Skill.node, MCP.node, locationServiceMapNode]) export * as SystemPrompt from "./system" diff --git a/packages/opencode/test/mcp/lifecycle.test.ts b/packages/opencode/test/mcp/lifecycle.test.ts index b9dfb9330e91..ec82d4f47f1f 100644 --- a/packages/opencode/test/mcp/lifecycle.test.ts +++ b/packages/opencode/test/mcp/lifecycle.test.ts @@ -13,6 +13,7 @@ import { TestInstance } from "../fixture/fixture" interface MockClientState { capabilities: { tools?: object; prompts?: object; resources?: object } capabilitiesShouldThrow: boolean + instructions?: string tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }> listToolsCalls: number listPromptsCalls: number @@ -188,6 +189,10 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({ return this._state?.capabilities } + getInstructions() { + return this._state?.instructions + } + async listTools(params?: { cursor?: string }) { if (this._state) this._state.listToolsCalls++ if (this._state?.listToolsShouldFail) { @@ -347,6 +352,60 @@ it.instance( { config: { mcp: {} } }, ) +it.instance( + "instructions() returns connected server instructions with tool names", + () => + MCP.Service.use((mcp: MCPNS.Interface) => + Effect.gen(function* () { + lastCreatedClientName = "guide-server" + const serverState = getOrCreateClientState("guide-server") + serverState.instructions = "Use lookup before mutate." + + yield* mcp.add("guide-server", { + type: "local", + command: ["echo", "test"], + }) + + expect(yield* mcp.instructions()).toContainEqual({ + name: "guide-server", + instructions: "Use lookup before mutate.", + tools: ["guide-server_test_tool"], + }) + }), + ), + { config: { mcp: {} } }, +) + +it.instance( + "instructions() omits empty and disconnected server instructions", + () => + MCP.Service.use((mcp: MCPNS.Interface) => + Effect.gen(function* () { + lastCreatedClientName = "temporary-server" + getOrCreateClientState("temporary-server").instructions = "Temporary guidance." + + yield* mcp.add("temporary-server", { + type: "local", + command: ["echo", "test"], + }) + yield* mcp.disconnect("temporary-server") + + lastCreatedClientName = "blank-server" + getOrCreateClientState("blank-server").instructions = " " + + yield* mcp.add("blank-server", { + type: "local", + command: ["echo", "test"], + }) + + const instructions = yield* mcp.instructions() + expect(instructions.some((item) => item.name === "temporary-server")).toBe(false) + expect(instructions.some((item) => item.name === "blank-server")).toBe(false) + }), + ), + { config: { mcp: {} } }, +) + it.instance( "follows cursors when listing tools, prompts, and resources", () => diff --git a/packages/opencode/test/session/prompt.test.ts b/packages/opencode/test/session/prompt.test.ts index ca1683e90156..96e37f2071b6 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -56,6 +56,7 @@ import { reply, TestLLMServer } from "../lib/llm-server" import { RuntimeFlags } from "@/effect/runtime-flags" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" +import { LocationServiceMap } from "@opencode-ai/core/location-layer" const summary = Layer.succeed( SessionSummary.Service, @@ -108,29 +109,32 @@ function errorTool(parts: SessionV1.Part[]) { return part?.state.status === "error" ? (part as ErrorToolPart) : undefined } -const mcp = Layer.succeed( - MCP.Service, - MCP.Service.of({ - status: () => Effect.succeed({}), - clients: () => Effect.succeed({}), - tools: () => Effect.succeed({}), - prompts: () => Effect.succeed({}), - resources: () => Effect.succeed({}), - resourceTemplates: () => 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), - }), -) +function makeMcp(instructions: MCP.ServerInstructions[] = []) { + return Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + instructions: () => Effect.succeed(instructions), + tools: () => Effect.succeed({}), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + resourceTemplates: () => 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, @@ -164,7 +168,10 @@ const blockingProcessor = Layer.succeed( }), ) -function makePrompt(input?: { processor?: "blocking" }) { +function makePrompt(input?: { + mcpInstructions?: MCP.ServerInstructions[] + processor?: "blocking" +}) { const deps = Layer.mergeAll( Session.defaultLayer, Snapshot.defaultLayer, @@ -177,7 +184,7 @@ function makePrompt(input?: { processor?: "blocking" }) { Config.defaultLayer, ProviderSvc.defaultLayer, lsp, - mcp, + makeMcp(input?.mcpInstructions), FSUtil.defaultLayer, BackgroundJob.defaultLayer, status, @@ -223,24 +230,47 @@ function makePrompt(input?: { processor?: "blocking" }) { Layer.provideMerge(registry), Layer.provideMerge(trunc), Layer.provide(Instruction.defaultLayer), - Layer.provide(SystemPrompt.defaultLayer), + Layer.provide( + SystemPrompt.layer.pipe( + Layer.provide(Skill.defaultLayer), + Layer.provide(LocationServiceMap.layer), + Layer.provide(deps), + ), + ), Layer.provide(RuntimeFlags.layer({ experimentalEventSystem: true })), Layer.provideMerge(deps), Layer.provide(summary), ) } -function makeHttp(input?: { processor?: "blocking" }) { +function makeHttp(input?: { + mcpInstructions?: MCP.ServerInstructions[] + processor?: "blocking" +}) { return Layer.mergeAll(TestLLMServer.layer, makePrompt(input)) } -function makeHttpNoLLMServer(input?: { processor?: "blocking" }) { +function makeHttpNoLLMServer(input?: { + mcpInstructions?: MCP.ServerInstructions[] + processor?: "blocking" +}) { return makePrompt(input) } const it = testEffect(makeHttp()) const noLLMServer = testEffect(makeHttpNoLLMServer()) const raceNoLLMServer = testEffect(makeHttpNoLLMServer({ processor: "blocking" })) +const withMcpInstructions = testEffect( + makeHttp({ + mcpInstructions: [ + { + name: "guide-server", + instructions: "Use lookup before mutate.", + tools: ["guide-server_lookup"], + }, + ], + }), +) const unix = process.platform !== "win32" ? it.instance : it.instance.skip const unixNoLLMServer = process.platform !== "win32" ? noLLMServer.instance : noLLMServer.instance.skip @@ -507,6 +537,30 @@ it.instance("loop calls LLM and returns assistant message", () => }), ) +withMcpInstructions.instance("loop includes MCP instructions in model system context", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig(providerCfg) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ + title: "Pinned", + permission: [{ permission: "*", pattern: "*", action: "allow" }], + }) + yield* llm.hang + yield* user(chat.id, "hello") + + const fiber = yield* prompt.loop({ sessionID: chat.id }).pipe(Effect.forkChild) + yield* awaitWithTimeout(llm.wait(1), "timed out waiting for MCP instruction request", "10 seconds") + + const hits = yield* llm.hits + const body = JSON.stringify(hits[0]?.body) + expect(body).toContain('') + expect(body).toContain("Use lookup before mutate.") + yield* Fiber.interrupt(fiber) + }), + 15_000, +) + it.instance("loop surfaces content-filter finishes as session errors", () => Effect.gen(function* () { const { llm } = yield* useServerConfig(providerCfg) diff --git a/packages/opencode/test/session/snapshot-tool-race.test.ts b/packages/opencode/test/session/snapshot-tool-race.test.ts index 9f5b36f353a0..0a5065837d0a 100644 --- a/packages/opencode/test/session/snapshot-tool-race.test.ts +++ b/packages/opencode/test/session/snapshot-tool-race.test.ts @@ -37,6 +37,7 @@ const mcp = Layer.succeed( MCP.Service.of({ status: () => Effect.succeed({}), clients: () => Effect.succeed({}), + instructions: () => Effect.succeed([]), tools: () => Effect.succeed({}), prompts: () => Effect.succeed({}), resources: () => Effect.succeed({}), diff --git a/packages/opencode/test/session/system.test.ts b/packages/opencode/test/session/system.test.ts index 69cec7bdcca7..548f0028c574 100644 --- a/packages/opencode/test/session/system.test.ts +++ b/packages/opencode/test/session/system.test.ts @@ -5,6 +5,7 @@ import { NamedError } from "@opencode-ai/core/util/error" import { Skill } from "../../src/skill" import { Permission } from "../../src/permission" import { SystemPrompt } from "../../src/session/system" +import { MCP } from "../../src/mcp" import { LocationServiceMap } from "@opencode-ai/core/location-layer" import { testEffect } from "../lib/effect" @@ -44,6 +45,23 @@ const build: Agent.Info = { const it = testEffect( SystemPrompt.layer.pipe( Layer.provide(LocationServiceMap.layer), + Layer.provide( + Layer.mock(MCP.Service, { + instructions: () => + Effect.succeed([ + { + name: "guide-server", + instructions: "Use lookup before mutate.", + tools: [], + }, + { + name: "tool-server", + instructions: "Prefer search before update.", + tools: ["tool-server_search", "tool-server_update"], + }, + ]), + }), + ), Layer.provide( Layer.succeed( Skill.Service, @@ -83,4 +101,41 @@ describe("session.system", () => { expect(output).not.toContain("manual-skill") }), ) + + it.effect("MCP output includes connected server instructions", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + const output = yield* prompt.mcp(build) + + expect(output).toBe( + [ + "", + ' ', + " Use lookup before mutate.", + " ", + ' ', + " Prefer search before update.", + " ", + "", + ].join("\n"), + ) + }), + ) + + it.effect("MCP output omits servers when all advertised tools are denied", () => + Effect.gen(function* () { + const prompt = yield* SystemPrompt.Service + const output = yield* prompt.mcp(build, Permission.fromConfig({ "tool-server_*": "deny" })) + + expect(output).toBe( + [ + "", + ' ', + " Use lookup before mutate.", + " ", + "", + ].join("\n"), + ) + }), + ) })