diff --git a/packages/opencode/src/mcp/index.ts b/packages/opencode/src/mcp/index.ts index 08d58118c992..0d2cf3a8953d 100644 --- a/packages/opencode/src/mcp/index.ts +++ b/packages/opencode/src/mcp/index.ts @@ -139,6 +139,7 @@ interface CreateResult { mcpClient?: MCPClient status: Status defs?: MCPToolDef[] + instructions?: string } interface AuthResult { @@ -154,11 +155,13 @@ interface State { status: Record clients: Record defs: Record + instructions: Record } 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: () => Effect.Effect> @@ -379,7 +382,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))), @@ -426,6 +429,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( @@ -480,6 +484,7 @@ export const layer = Layer.effect( status: {}, clients: {}, defs: {}, + instructions: {}, } yield* Effect.forEach( @@ -501,6 +506,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) } }), @@ -512,6 +518,7 @@ export const layer = Layer.effect( const clients = Object.values(s.clients) s.clients = {} s.defs = {} + s.instructions = {} yield* Effect.forEach( clients, (client) => @@ -541,6 +548,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) } @@ -550,6 +558,7 @@ export const layer = Layer.effect( name: string, client: MCPClient, listed: MCPToolDef[], + instructions: string | undefined, timeout?: number, ) { const bridge = yield* EffectBridge.make() @@ -557,6 +566,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] @@ -586,6 +597,17 @@ 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]) => + `Instructions from: MCP server ${name}\nThese instructions apply to MCP tools whose names start with \`${McpCatalog.sanitize(name)}_\`, and to prompts/resources from this MCP server.\n\n${item}`, + ) + }) + const createAndStore = Effect.fn("MCP.createAndStore")(function* (name: string, mcp: ConfigMCPV1.Info) { const s = yield* InstanceState.get(state) const result = yield* create(name, mcp) @@ -597,7 +619,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) { @@ -830,7 +852,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) @@ -917,6 +939,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 b3f85c813f20..883299de523f 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -1324,13 +1324,14 @@ 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), + mcp.instructions(), MessageV2.toModelMessagesEffect(msgs, model), ]) - const system = [...env, ...instructions, ...(skills ? [skills] : [])] + const system = [...env, ...instructions, ...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/test/mcp/lifecycle.test.ts b/packages/opencode/test/mcp/lifecycle.test.ts index 34304624e680..4e897129a15c 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 @@ -179,6 +180,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) { @@ -331,6 +336,63 @@ it.instance( { config: { mcp: {} } }, ) +it.instance( + "instructions() returns connected server instructions with tool prefix guidance", + () => + 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()).toContain( + [ + "Instructions from: MCP server guide-server", + "These instructions apply to MCP tools whose names start with `guide-server_`, and to prompts/resources from this MCP server.", + "", + "Use lookup before mutate.", + ].join("\n"), + ) + }), + ), + { 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.includes("temporary-server"))).toBe(false) + expect(instructions.some((item) => item.includes("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 08828018a4a3..441fd1116fd7 100644 --- a/packages/opencode/test/session/prompt.test.ts +++ b/packages/opencode/test/session/prompt.test.ts @@ -108,28 +108,31 @@ 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({}), - 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: string[] = []) { + 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({}), + 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, @@ -163,7 +166,7 @@ const blockingProcessor = Layer.succeed( }), ) -function makePrompt(input?: { processor?: "blocking" }) { +function makePrompt(input?: { mcpInstructions?: string[]; processor?: "blocking" }) { const deps = Layer.mergeAll( Session.defaultLayer, Snapshot.defaultLayer, @@ -176,7 +179,7 @@ function makePrompt(input?: { processor?: "blocking" }) { Config.defaultLayer, ProviderSvc.defaultLayer, lsp, - mcp, + makeMcp(input?.mcpInstructions), FSUtil.defaultLayer, BackgroundJob.defaultLayer, status, @@ -229,17 +232,29 @@ function makePrompt(input?: { processor?: "blocking" }) { ) } -function makeHttp(input?: { processor?: "blocking" }) { +function makeHttp(input?: { mcpInstructions?: string[]; processor?: "blocking" }) { return Layer.mergeAll(TestLLMServer.layer, makePrompt(input)) } -function makeHttpNoLLMServer(input?: { processor?: "blocking" }) { +function makeHttpNoLLMServer(input?: { mcpInstructions?: string[]; processor?: "blocking" }) { return makePrompt(input) } const it = testEffect(makeHttp()) const noLLMServer = testEffect(makeHttpNoLLMServer()) const raceNoLLMServer = testEffect(makeHttpNoLLMServer({ processor: "blocking" })) +const withMcpInstructions = testEffect( + makeHttp({ + mcpInstructions: [ + [ + "Instructions from: MCP server guide-server", + "These instructions apply to MCP tools whose names start with `guide-server_`, and to prompts/resources from this MCP server.", + "", + "Use lookup before mutate.", + ].join("\n"), + ], + }), +) const unix = process.platform !== "win32" ? it.instance : it.instance.skip const unixNoLLMServer = process.platform !== "win32" ? noLLMServer.instance : noLLMServer.instance.skip @@ -506,6 +521,31 @@ 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("Instructions from: MCP server guide-server") + expect(body).toContain("guide-server_") + 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 8a3701e12518..21ffb6dfc424 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({}),