fix(opencode): respect MCP server capabilities (#31271)
This commit is contained in:
@@ -204,7 +204,7 @@ function fetchFromClient<T extends { name: string }>(
|
|||||||
return Effect.tryPromise({
|
return Effect.tryPromise({
|
||||||
try: () => listFn(client),
|
try: () => listFn(client),
|
||||||
catch: (e: any) => {
|
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
|
return e
|
||||||
},
|
},
|
||||||
}).pipe(
|
}).pipe(
|
||||||
@@ -472,7 +472,7 @@ export const layer = Layer.effect(
|
|||||||
return { status } satisfies CreateResult
|
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) {
|
if (!listed) {
|
||||||
yield* Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore)
|
yield* Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore)
|
||||||
return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult
|
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) {
|
function watch(s: State, name: string, client: MCPClient, bridge: EffectBridge.Shape, timeout?: number) {
|
||||||
|
if (!client.getServerCapabilities()?.tools) return
|
||||||
client.setNotificationHandler(ToolListChangedNotificationSchema, async () => {
|
client.setNotificationHandler(ToolListChangedNotificationSchema, async () => {
|
||||||
log.info("tools list changed notification received", { server: name })
|
log.info("tools list changed notification received", { server: name })
|
||||||
if (s.clients[name] !== client || s.status[name]?.status !== "connected") return
|
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 prompts = Effect.fn("MCP.prompts")(function* () {
|
||||||
const s = yield* InstanceState.get(state)
|
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 resources = Effect.fn("MCP.resources")(function* () {
|
||||||
const s = yield* InstanceState.get(state)
|
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>(
|
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)),
|
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) {
|
if (!client || !listed) {
|
||||||
yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)
|
yield* Effect.tryPromise(() => client?.close() ?? Promise.resolve()).pipe(Effect.ignore)
|
||||||
return { status: "failed", error: "Failed to get tools" } as Status
|
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
|
// Per-client state for controlling mock behavior
|
||||||
interface MockClientState {
|
interface MockClientState {
|
||||||
|
capabilities: { tools?: object; prompts?: object; resources?: object }
|
||||||
tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }>
|
tools: Array<{ name: string; description?: string; inputSchema: object; outputSchema?: object }>
|
||||||
listToolsCalls: number
|
listToolsCalls: number
|
||||||
|
listPromptsCalls: number
|
||||||
|
listResourcesCalls: number
|
||||||
requestCalls: number
|
requestCalls: number
|
||||||
listToolsShouldFail: boolean
|
listToolsShouldFail: boolean
|
||||||
listToolsError: string
|
listToolsError: string
|
||||||
@@ -35,8 +38,11 @@ function getOrCreateClientState(name?: string): MockClientState {
|
|||||||
let state = clientStates.get(key)
|
let state = clientStates.get(key)
|
||||||
if (!state) {
|
if (!state) {
|
||||||
state = {
|
state = {
|
||||||
|
capabilities: { tools: {}, prompts: {}, resources: {} },
|
||||||
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
||||||
listToolsCalls: 0,
|
listToolsCalls: 0,
|
||||||
|
listPromptsCalls: 0,
|
||||||
|
listResourcesCalls: 0,
|
||||||
requestCalls: 0,
|
requestCalls: 0,
|
||||||
listToolsShouldFail: false,
|
listToolsShouldFail: false,
|
||||||
listToolsError: "listTools failed",
|
listToolsError: "listTools failed",
|
||||||
@@ -133,6 +139,10 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||||||
this._state?.notificationHandlers.set(schema, handler)
|
this._state?.notificationHandlers.set(schema, handler)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
getServerCapabilities() {
|
||||||
|
return this._state?.capabilities
|
||||||
|
}
|
||||||
|
|
||||||
async listTools() {
|
async listTools() {
|
||||||
if (this._state) this._state.listToolsCalls++
|
if (this._state) this._state.listToolsCalls++
|
||||||
if (this._state?.listToolsShouldFail) {
|
if (this._state?.listToolsShouldFail) {
|
||||||
@@ -148,6 +158,7 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
async listPrompts() {
|
async listPrompts() {
|
||||||
|
if (this._state) this._state.listPromptsCalls++
|
||||||
if (this._state?.listPromptsShouldFail) {
|
if (this._state?.listPromptsShouldFail) {
|
||||||
throw new Error("listPrompts failed")
|
throw new Error("listPrompts failed")
|
||||||
}
|
}
|
||||||
@@ -155,6 +166,7 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||||||
}
|
}
|
||||||
|
|
||||||
async listResources() {
|
async listResources() {
|
||||||
|
if (this._state) this._state.listResourcesCalls++
|
||||||
if (this._state?.listResourcesShouldFail) {
|
if (this._state?.listResourcesShouldFail) {
|
||||||
throw new Error("listResources failed")
|
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(
|
it.instance(
|
||||||
"prompts() skips disconnected servers",
|
"prompts() skips disconnected servers",
|
||||||
() =>
|
() =>
|
||||||
|
|||||||
@@ -21,6 +21,8 @@ const transportCalls: Array<{
|
|||||||
// auth flow (which calls provider.state()) or a simple UnauthorizedError.
|
// auth flow (which calls provider.state()) or a simple UnauthorizedError.
|
||||||
let simulateAuthFlow = true
|
let simulateAuthFlow = true
|
||||||
let connectSucceedsImmediately = false
|
let connectSucceedsImmediately = false
|
||||||
|
let serverCapabilities: { tools?: object; resources?: object } = { tools: {} }
|
||||||
|
let listToolsCalls = 0
|
||||||
|
|
||||||
// Mock the transport constructors to simulate OAuth auto-auth on 401
|
// Mock the transport constructors to simulate OAuth auto-auth on 401
|
||||||
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
|
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
|
||||||
@@ -91,10 +93,19 @@ void mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
|||||||
|
|
||||||
setNotificationHandler() {}
|
setNotificationHandler() {}
|
||||||
|
|
||||||
|
getServerCapabilities() {
|
||||||
|
return serverCapabilities
|
||||||
|
}
|
||||||
|
|
||||||
async listTools() {
|
async listTools() {
|
||||||
|
listToolsCalls++
|
||||||
return { tools: [{ name: "test_tool", inputSchema: { type: "object", properties: {} } }] }
|
return { tools: [{ name: "test_tool", inputSchema: { type: "object", properties: {} } }] }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async listResources() {
|
||||||
|
return { resources: [{ name: "docs", uri: "docs://readme" }] }
|
||||||
|
}
|
||||||
|
|
||||||
async close() {}
|
async close() {}
|
||||||
},
|
},
|
||||||
}))
|
}))
|
||||||
@@ -108,6 +119,8 @@ beforeEach(() => {
|
|||||||
transportCalls.length = 0
|
transportCalls.length = 0
|
||||||
simulateAuthFlow = true
|
simulateAuthFlow = true
|
||||||
connectSucceedsImmediately = false
|
connectSucceedsImmediately = false
|
||||||
|
serverCapabilities = { tools: {} }
|
||||||
|
listToolsCalls = 0
|
||||||
})
|
})
|
||||||
|
|
||||||
// Import modules after mocking
|
// Import modules after mocking
|
||||||
@@ -234,3 +247,28 @@ mcpTest.instance(
|
|||||||
),
|
),
|
||||||
{ config: config("test-oauth-connect") },
|
{ 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> }) {
|
async connect(transport: { start: () => Promise<void> }) {
|
||||||
await transport.start()
|
await transport.start()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
getServerCapabilities() {
|
||||||
|
return { tools: {} }
|
||||||
|
}
|
||||||
},
|
},
|
||||||
}))
|
}))
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user