fix(opencode): respect MCP server capabilities (#31271)
This commit is contained in:
parent
4d09a71ef4
commit
b5cb9aae7f
4 changed files with 151 additions and 5 deletions
|
|
@ -204,7 +204,7 @@ function fetchFromClient<T extends { name: string }>(
|
|||
return Effect.tryPromise({
|
||||
try: () => listFn(client),
|
||||
catch: (e: any) => {
|
||||
log.error(`failed to get ${label}`, { clientName, error: e.message })
|
||||
log.warn(`failed to get ${label}`, { clientName, error: e.message })
|
||||
return e
|
||||
},
|
||||
}).pipe(
|
||||
|
|
@ -472,7 +472,7 @@ export const layer = Layer.effect(
|
|||
return { status } satisfies CreateResult
|
||||
}
|
||||
|
||||
const listed = yield* defs(key, mcpClient, mcp.timeout)
|
||||
const listed = mcpClient.getServerCapabilities()?.tools ? yield* defs(key, mcpClient, mcp.timeout) : []
|
||||
if (!listed) {
|
||||
yield* Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore)
|
||||
return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult
|
||||
|
|
@ -508,6 +508,7 @@ export const layer = Layer.effect(
|
|||
)
|
||||
|
||||
function watch(s: State, name: string, client: MCPClient, bridge: EffectBridge.Shape, timeout?: number) {
|
||||
if (!client.getServerCapabilities()?.tools) return
|
||||
client.setNotificationHandler(ToolListChangedNotificationSchema, async () => {
|
||||
log.info("tools list changed notification received", { server: name })
|
||||
if (s.clients[name] !== client || s.status[name]?.status !== "connected") return
|
||||
|
|
@ -718,12 +719,21 @@ export const layer = Layer.effect(
|
|||
|
||||
const prompts = Effect.fn("MCP.prompts")(function* () {
|
||||
const s = yield* InstanceState.get(state)
|
||||
return yield* collectFromConnected(s, (c) => c.listPrompts().then((r) => r.prompts), "prompts")
|
||||
return yield* collectFromConnected(
|
||||
s,
|
||||
(c) => (c.getServerCapabilities()?.prompts ? c.listPrompts().then((r) => r.prompts) : Promise.resolve([])),
|
||||
"prompts",
|
||||
)
|
||||
})
|
||||
|
||||
const resources = Effect.fn("MCP.resources")(function* () {
|
||||
const s = yield* InstanceState.get(state)
|
||||
return yield* collectFromConnected(s, (c) => c.listResources().then((r) => r.resources), "resources")
|
||||
return yield* collectFromConnected(
|
||||
s,
|
||||
(c) =>
|
||||
c.getServerCapabilities()?.resources ? c.listResources().then((r) => r.resources) : Promise.resolve([]),
|
||||
"resources",
|
||||
)
|
||||
})
|
||||
|
||||
const withClient = Effect.fnUntraced(function* <A>(
|
||||
|
|
@ -848,7 +858,11 @@ export const layer = Layer.effect(
|
|||
Effect.tapError(() => Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)),
|
||||
)
|
||||
|
||||
const listed = client ? yield* defs(mcpName, client, mcpConfig.timeout) : undefined
|
||||
const listed = client
|
||||
? client.getServerCapabilities()?.tools
|
||||
? yield* defs(mcpName, client, mcpConfig.timeout)
|
||||
: []
|
||||
: undefined
|
||||
if (!client || !listed) {
|
||||
yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)
|
||||
return { status: "failed", error: "Failed to get tools" } as Status
|
||||
|
|
|
|||
|
|
@ -7,8 +7,11 @@ import { testEffect } from "../lib/effect"
|
|||
|
||||
// Per-client state for controlling mock behavior
|
||||
interface MockClientState {
|
||||
capabilities: { tools?: object; prompts?: object; resources?: object }
|
||||
tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }>
|
||||
listToolsCalls: number
|
||||
listPromptsCalls: number
|
||||
listResourcesCalls: number
|
||||
requestCalls: number
|
||||
listToolsShouldFail: boolean
|
||||
listToolsError: string
|
||||
|
|
@ -35,8 +38,11 @@ function getOrCreateClientState(name?: string): MockClientState {
|
|||
let state = clientStates.get(key)
|
||||
if (!state) {
|
||||
state = {
|
||||
capabilities: { tools: {}, prompts: {}, resources: {} },
|
||||
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
||||
listToolsCalls: 0,
|
||||
listPromptsCalls: 0,
|
||||
listResourcesCalls: 0,
|
||||
requestCalls: 0,
|
||||
listToolsShouldFail: false,
|
||||
listToolsError: "listTools failed",
|
||||
|
|
@ -133,6 +139,10 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||
this._state?.notificationHandlers.set(schema, handler)
|
||||
}
|
||||
|
||||
getServerCapabilities() {
|
||||
return this._state?.capabilities
|
||||
}
|
||||
|
||||
async listTools() {
|
||||
if (this._state) this._state.listToolsCalls++
|
||||
if (this._state?.listToolsShouldFail) {
|
||||
|
|
@ -148,6 +158,7 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||
}
|
||||
|
||||
async listPrompts() {
|
||||
if (this._state) this._state.listPromptsCalls++
|
||||
if (this._state?.listPromptsShouldFail) {
|
||||
throw new Error("listPrompts failed")
|
||||
}
|
||||
|
|
@ -155,6 +166,7 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||
}
|
||||
|
||||
async listResources() {
|
||||
if (this._state) this._state.listResourcesCalls++
|
||||
if (this._state?.listResourcesShouldFail) {
|
||||
throw new Error("listResources failed")
|
||||
}
|
||||
|
|
@ -598,6 +610,84 @@ it.instance(
|
|||
},
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"resource-only servers connect without listing tools",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "resource-only-server"
|
||||
const serverState = getOrCreateClientState("resource-only-server")
|
||||
serverState.capabilities = { resources: {} }
|
||||
serverState.resources = [{ name: "docs", uri: "docs://readme" }]
|
||||
|
||||
const result = yield* mcp.add("resource-only-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
expect(statusName(result.status, "resource-only-server")).toBe("connected")
|
||||
expect(serverState.listToolsCalls).toBe(0)
|
||||
expect(Object.keys(yield* mcp.tools())).toHaveLength(0)
|
||||
expect(Object.keys(yield* mcp.resources())).toEqual(["resource-only-server:docs"])
|
||||
expect(serverState.listResourcesCalls).toBe(1)
|
||||
expect(serverState.listPromptsCalls).toBe(0)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"prompt-only servers connect without listing tools",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "prompt-only-server"
|
||||
const serverState = getOrCreateClientState("prompt-only-server")
|
||||
serverState.capabilities = { prompts: {} }
|
||||
serverState.prompts = [{ name: "review" }]
|
||||
|
||||
const result = yield* mcp.add("prompt-only-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
expect(statusName(result.status, "prompt-only-server")).toBe("connected")
|
||||
expect(serverState.listToolsCalls).toBe(0)
|
||||
expect(Object.keys(yield* mcp.tools())).toHaveLength(0)
|
||||
expect(Object.keys(yield* mcp.prompts())).toEqual(["prompt-only-server:review"])
|
||||
expect(serverState.listPromptsCalls).toBe(1)
|
||||
expect(serverState.listResourcesCalls).toBe(0)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"tools-only servers skip optional prompt and resource discovery",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "tools-only-server"
|
||||
const serverState = getOrCreateClientState("tools-only-server")
|
||||
serverState.capabilities = { tools: {} }
|
||||
|
||||
const result = yield* mcp.add("tools-only-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
expect(statusName(result.status, "tools-only-server")).toBe("connected")
|
||||
expect(serverState.listToolsCalls).toBe(1)
|
||||
expect(Object.keys(yield* mcp.tools())).toEqual(["tools-only-server_test_tool"])
|
||||
expect(yield* mcp.prompts()).toEqual({})
|
||||
expect(yield* mcp.resources()).toEqual({})
|
||||
expect(serverState.listPromptsCalls).toBe(0)
|
||||
expect(serverState.listResourcesCalls).toBe(0)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"prompts() skips disconnected servers",
|
||||
() =>
|
||||
|
|
|
|||
|
|
@ -21,6 +21,8 @@ const transportCalls: Array<{
|
|||
// auth flow (which calls provider.state()) or a simple UnauthorizedError.
|
||||
let simulateAuthFlow = true
|
||||
let connectSucceedsImmediately = false
|
||||
let serverCapabilities: { tools?: object; resources?: object } = { tools: {} }
|
||||
let listToolsCalls = 0
|
||||
|
||||
// Mock the transport constructors to simulate OAuth auto-auth on 401
|
||||
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
|
||||
|
|
@ -91,10 +93,19 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||
|
||||
setNotificationHandler() {}
|
||||
|
||||
getServerCapabilities() {
|
||||
return serverCapabilities
|
||||
}
|
||||
|
||||
async listTools() {
|
||||
listToolsCalls++
|
||||
return { tools: [{ name: "test_tool", inputSchema: { type: "object", properties: {} } }] }
|
||||
}
|
||||
|
||||
async listResources() {
|
||||
return { resources: [{ name: "docs", uri: "docs://readme" }] }
|
||||
}
|
||||
|
||||
async close() {}
|
||||
},
|
||||
}))
|
||||
|
|
@ -108,6 +119,8 @@ beforeEach(() => {
|
|||
transportCalls.length = 0
|
||||
simulateAuthFlow = true
|
||||
connectSucceedsImmediately = false
|
||||
serverCapabilities = { tools: {} }
|
||||
listToolsCalls = 0
|
||||
})
|
||||
|
||||
// Import modules after mocking
|
||||
|
|
@ -234,3 +247,28 @@ mcpTest.instance(
|
|||
),
|
||||
{ config: config("test-oauth-connect") },
|
||||
)
|
||||
|
||||
mcpTest.instance(
|
||||
"authenticate() connects a resource-only server without listing tools",
|
||||
() =>
|
||||
MCP.Service.use((mcp) =>
|
||||
Effect.gen(function* () {
|
||||
const added = yield* mcp.add("test-oauth-resources", {
|
||||
type: "remote",
|
||||
url: "https://example.com/mcp",
|
||||
})
|
||||
const before = added.status as Record<string, { status: string }>
|
||||
expect(before["test-oauth-resources"]?.status).toBe("needs_auth")
|
||||
|
||||
simulateAuthFlow = false
|
||||
connectSucceedsImmediately = true
|
||||
serverCapabilities = { resources: {} }
|
||||
|
||||
const result = yield* mcp.authenticate("test-oauth-resources")
|
||||
expect(result.status).toBe("connected")
|
||||
expect(listToolsCalls).toBe(0)
|
||||
expect(Object.keys(yield* mcp.resources())).toEqual(["test-oauth-resources:docs"])
|
||||
}),
|
||||
),
|
||||
{ config: config("test-oauth-resources") },
|
||||
)
|
||||
|
|
|
|||
|
|
@ -89,6 +89,10 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||
async connect(transport: { start: () => Promise<void> }) {
|
||||
await transport.start()
|
||||
}
|
||||
|
||||
getServerCapabilities() {
|
||||
return { tools: {} }
|
||||
}
|
||||
},
|
||||
}))
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue