feat(mcp): append server instructions to context (#32490)
Co-authored-by: Aiden Cline <aidenpcline@gmail.com>
This commit is contained in:
parent
2e909334c1
commit
e8e83afbce
8 changed files with 269 additions and 36 deletions
|
|
@ -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",
|
||||
() =>
|
||||
|
|
|
|||
|
|
@ -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('<server name=\\"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)
|
||||
|
|
|
|||
|
|
@ -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({}),
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
[
|
||||
"<mcp_instructions>",
|
||||
' <server name="guide-server">',
|
||||
" Use lookup before mutate.",
|
||||
" </server>",
|
||||
' <server name="tool-server">',
|
||||
" Prefer search before update.",
|
||||
" </server>",
|
||||
"</mcp_instructions>",
|
||||
].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(
|
||||
[
|
||||
"<mcp_instructions>",
|
||||
' <server name="guide-server">',
|
||||
" Use lookup before mutate.",
|
||||
" </server>",
|
||||
"</mcp_instructions>",
|
||||
].join("\n"),
|
||||
)
|
||||
}),
|
||||
)
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue