mirror of
https://github.com/anomalyco/opencode.git
synced 2026-04-24 06:45:22 +00:00
fix(mcp): fix cancelPending key mismatch, add lifecycle tests
McpOAuthCallback.cancelPending(mcpName) was a no-op because pendingAuths is keyed by oauthState (a hex string), not mcpName. Add a reverse index so removeAuth() can actually cancel in-flight OAuth flows. Add comprehensive MCP lifecycle tests covering connect/disconnect, add/replace, prompts, resources, disabled servers, error paths, and tool name sanitization. Two tests for tools() transient-failure recovery are marked .todo — the current behavior permanently deletes clients on listTools() failure with no reconnect path; to be fixed during Effect migration.
This commit is contained in:
Kit Langton
@@ -844,7 +844,7 @@ export namespace MCP {
|
||||
|
||||
// Register the callback BEFORE opening the browser to avoid race condition
|
||||
// when the IdP has an active SSO session and redirects immediately
|
||||
const callbackPromise = McpOAuthCallback.waitForCallback(oauthState)
|
||||
const callbackPromise = McpOAuthCallback.waitForCallback(oauthState, mcpName)
|
||||
|
||||
try {
|
||||
const subprocess = await open(authorizationUrl)
|
||||
|
||||
@@ -54,6 +54,9 @@ interface PendingAuth {
|
||||
export namespace McpOAuthCallback {
|
||||
let server: ReturnType<typeof Bun.serve> | undefined
|
||||
const pendingAuths = new Map<string, PendingAuth>()
|
||||
// Reverse index: mcpName → oauthState, so cancelPending(mcpName) can
|
||||
// find the right entry in pendingAuths (which is keyed by oauthState).
|
||||
const mcpNameToState = new Map<string, string>()
|
||||
|
||||
const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000 // 5 minutes
|
||||
|
||||
@@ -137,11 +140,13 @@ export namespace McpOAuthCallback {
|
||||
log.info("oauth callback server started", { port: OAUTH_CALLBACK_PORT })
|
||||
}
|
||||
|
||||
export function waitForCallback(oauthState: string): Promise<string> {
|
||||
export function waitForCallback(oauthState: string, mcpName?: string): Promise<string> {
|
||||
if (mcpName) mcpNameToState.set(mcpName, oauthState)
|
||||
return new Promise((resolve, reject) => {
|
||||
const timeout = setTimeout(() => {
|
||||
if (pendingAuths.has(oauthState)) {
|
||||
pendingAuths.delete(oauthState)
|
||||
if (mcpName) mcpNameToState.delete(mcpName)
|
||||
reject(new Error("OAuth callback timeout - authorization took too long"))
|
||||
}
|
||||
}, CALLBACK_TIMEOUT_MS)
|
||||
@@ -151,10 +156,14 @@ export namespace McpOAuthCallback {
|
||||
}
|
||||
|
||||
export function cancelPending(mcpName: string): void {
|
||||
const pending = pendingAuths.get(mcpName)
|
||||
// Look up the oauthState for this mcpName via the reverse index
|
||||
const oauthState = mcpNameToState.get(mcpName)
|
||||
const key = oauthState ?? mcpName
|
||||
const pending = pendingAuths.get(key)
|
||||
if (pending) {
|
||||
clearTimeout(pending.timeout)
|
||||
pendingAuths.delete(mcpName)
|
||||
pendingAuths.delete(key)
|
||||
mcpNameToState.delete(mcpName)
|
||||
pending.reject(new Error("Authorization cancelled"))
|
||||
}
|
||||
}
|
||||
@@ -184,6 +193,7 @@ export namespace McpOAuthCallback {
|
||||
pending.reject(new Error("OAuth callback server stopped"))
|
||||
}
|
||||
pendingAuths.clear()
|
||||
mcpNameToState.clear()
|
||||
}
|
||||
|
||||
export function isRunning(): boolean {
|
||||
|
||||
689
packages/opencode/test/mcp/lifecycle.test.ts
Normal file
689
packages/opencode/test/mcp/lifecycle.test.ts
Normal file
@@ -0,0 +1,689 @@
|
||||
import { test, expect, mock, beforeEach } from "bun:test"
|
||||
|
||||
// --- Mock infrastructure ---
|
||||
|
||||
// Per-client state for controlling mock behavior
|
||||
interface MockClientState {
|
||||
tools: Array<{ name: string; description?: string; inputSchema: object }>
|
||||
listToolsShouldFail: boolean
|
||||
listToolsError: string
|
||||
listPromptsShouldFail: boolean
|
||||
listResourcesShouldFail: boolean
|
||||
prompts: Array<{ name: string; description?: string }>
|
||||
resources: Array<{ name: string; uri: string; description?: string }>
|
||||
closed: boolean
|
||||
notificationHandlers: Map<unknown, (...args: any[]) => any>
|
||||
}
|
||||
|
||||
const clientStates = new Map<string, MockClientState>()
|
||||
let lastCreatedClientName: string | undefined
|
||||
let connectShouldFail = false
|
||||
let connectError = "Mock transport cannot connect"
|
||||
// Tracks how many Client instances were created (detects leaks)
|
||||
let clientCreateCount = 0
|
||||
|
||||
function getOrCreateClientState(name?: string): MockClientState {
|
||||
const key = name ?? "default"
|
||||
let state = clientStates.get(key)
|
||||
if (!state) {
|
||||
state = {
|
||||
tools: [{ name: "test_tool", description: "A test tool", inputSchema: { type: "object", properties: {} } }],
|
||||
listToolsShouldFail: false,
|
||||
listToolsError: "listTools failed",
|
||||
listPromptsShouldFail: false,
|
||||
listResourcesShouldFail: false,
|
||||
prompts: [],
|
||||
resources: [],
|
||||
closed: false,
|
||||
notificationHandlers: new Map(),
|
||||
}
|
||||
clientStates.set(key, state)
|
||||
}
|
||||
return state
|
||||
}
|
||||
|
||||
// Mock transport that succeeds or fails based on connectShouldFail
|
||||
class MockStdioTransport {
|
||||
stderr: null = null
|
||||
pid = 12345
|
||||
constructor(_opts: any) {}
|
||||
async start() {
|
||||
if (connectShouldFail) throw new Error(connectError)
|
||||
}
|
||||
async close() {}
|
||||
}
|
||||
|
||||
class MockStreamableHTTP {
|
||||
constructor(_url: URL, _opts?: any) {}
|
||||
async start() {
|
||||
if (connectShouldFail) throw new Error(connectError)
|
||||
}
|
||||
async close() {}
|
||||
async finishAuth() {}
|
||||
}
|
||||
|
||||
class MockSSE {
|
||||
constructor(_url: URL, _opts?: any) {}
|
||||
async start() {
|
||||
throw new Error("SSE fallback - not used in these tests")
|
||||
}
|
||||
async close() {}
|
||||
}
|
||||
|
||||
mock.module("@modelcontextprotocol/sdk/client/stdio.js", () => ({
|
||||
StdioClientTransport: MockStdioTransport,
|
||||
}))
|
||||
|
||||
mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
|
||||
StreamableHTTPClientTransport: MockStreamableHTTP,
|
||||
}))
|
||||
|
||||
mock.module("@modelcontextprotocol/sdk/client/sse.js", () => ({
|
||||
SSEClientTransport: MockSSE,
|
||||
}))
|
||||
|
||||
mock.module("@modelcontextprotocol/sdk/client/auth.js", () => ({
|
||||
UnauthorizedError: class extends Error {
|
||||
constructor() {
|
||||
super("Unauthorized")
|
||||
}
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock Client that delegates to per-name MockClientState
|
||||
mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
|
||||
Client: class MockClient {
|
||||
_state: MockClientState
|
||||
transport: any
|
||||
|
||||
constructor(_opts: any) {
|
||||
clientCreateCount++
|
||||
}
|
||||
|
||||
async connect(transport: { start: () => Promise<void> }) {
|
||||
this.transport = transport
|
||||
await transport.start()
|
||||
// After successful connect, bind to the last-created client name
|
||||
this._state = getOrCreateClientState(lastCreatedClientName)
|
||||
}
|
||||
|
||||
setNotificationHandler(schema: unknown, handler: (...args: any[]) => any) {
|
||||
this._state?.notificationHandlers.set(schema, handler)
|
||||
}
|
||||
|
||||
async listTools() {
|
||||
if (this._state?.listToolsShouldFail) {
|
||||
throw new Error(this._state.listToolsError)
|
||||
}
|
||||
return { tools: this._state?.tools ?? [] }
|
||||
}
|
||||
|
||||
async listPrompts() {
|
||||
if (this._state?.listPromptsShouldFail) {
|
||||
throw new Error("listPrompts failed")
|
||||
}
|
||||
return { prompts: this._state?.prompts ?? [] }
|
||||
}
|
||||
|
||||
async listResources() {
|
||||
if (this._state?.listResourcesShouldFail) {
|
||||
throw new Error("listResources failed")
|
||||
}
|
||||
return { resources: this._state?.resources ?? [] }
|
||||
}
|
||||
|
||||
async close() {
|
||||
if (this._state) this._state.closed = true
|
||||
}
|
||||
},
|
||||
}))
|
||||
|
||||
beforeEach(() => {
|
||||
clientStates.clear()
|
||||
lastCreatedClientName = undefined
|
||||
connectShouldFail = false
|
||||
connectError = "Mock transport cannot connect"
|
||||
clientCreateCount = 0
|
||||
})
|
||||
|
||||
// Import after mocks
|
||||
const { MCP } = await import("../../src/mcp/index")
|
||||
const { Instance } = await import("../../src/project/instance")
|
||||
const { tmpdir } = await import("../fixture/fixture")
|
||||
|
||||
// --- Helper ---
|
||||
|
||||
function withInstance(
|
||||
config: Record<string, any>,
|
||||
fn: () => Promise<void>,
|
||||
) {
|
||||
return async () => {
|
||||
await using tmp = await tmpdir({
|
||||
init: async (dir) => {
|
||||
await Bun.write(
|
||||
`${dir}/opencode.json`,
|
||||
JSON.stringify({
|
||||
$schema: "https://opencode.ai/config.json",
|
||||
mcp: config,
|
||||
}),
|
||||
)
|
||||
},
|
||||
})
|
||||
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
await fn()
|
||||
// dispose instance to clean up state between tests
|
||||
await Instance.dispose()
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Bug #1: tools() silently deletes clients on transient listTools() failure
|
||||
// ========================================================================
|
||||
|
||||
test.todo(
|
||||
"tools() removes client from state when listTools fails",
|
||||
withInstance(
|
||||
{
|
||||
"my-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "my-server"
|
||||
const serverState = getOrCreateClientState("my-server")
|
||||
serverState.tools = [
|
||||
{ name: "do_thing", description: "does a thing", inputSchema: { type: "object", properties: {} } },
|
||||
]
|
||||
|
||||
// First: add the server successfully
|
||||
const addResult = await MCP.add("my-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
expect((addResult.status as any)["my-server"]?.status ?? (addResult.status as any).status).toBe("connected")
|
||||
|
||||
// Verify tools work initially
|
||||
const toolsBefore = await MCP.tools()
|
||||
expect(Object.keys(toolsBefore).length).toBeGreaterThan(0)
|
||||
|
||||
// Simulate a transient listTools failure
|
||||
serverState.listToolsShouldFail = true
|
||||
await MCP.tools()
|
||||
|
||||
// After the transient error clears, tools should recover
|
||||
// automatically — the server shouldn't be permanently removed.
|
||||
serverState.listToolsShouldFail = false
|
||||
|
||||
const toolsRecovered = await MCP.tools()
|
||||
expect(Object.keys(toolsRecovered).length).toBeGreaterThan(0)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Bug #2: status() shows stale data after tools() deletes a client
|
||||
// ========================================================================
|
||||
|
||||
test.todo(
|
||||
"status shows 'failed' after tools() encounters a transient error",
|
||||
withInstance(
|
||||
{
|
||||
"status-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "status-server"
|
||||
const serverState = getOrCreateClientState("status-server")
|
||||
|
||||
await MCP.add("status-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const statusBefore = await MCP.status()
|
||||
expect(statusBefore["status-server"]?.status).toBe("connected")
|
||||
|
||||
// Simulate a transient failure then recovery
|
||||
serverState.listToolsShouldFail = true
|
||||
await MCP.tools()
|
||||
serverState.listToolsShouldFail = false
|
||||
|
||||
// After the error clears, status should recover automatically
|
||||
await MCP.tools()
|
||||
const statusAfter = await MCP.status()
|
||||
expect(statusAfter["status-server"]?.status).toBe("connected")
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect() / disconnect() lifecycle
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"disconnect sets status to disabled and removes client",
|
||||
withInstance(
|
||||
{
|
||||
"disc-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "disc-server"
|
||||
getOrCreateClientState("disc-server")
|
||||
|
||||
await MCP.add("disc-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const statusBefore = await MCP.status()
|
||||
expect(statusBefore["disc-server"]?.status).toBe("connected")
|
||||
|
||||
await MCP.disconnect("disc-server")
|
||||
|
||||
const statusAfter = await MCP.status()
|
||||
expect(statusAfter["disc-server"]?.status).toBe("disabled")
|
||||
|
||||
// Tools should be empty after disconnect
|
||||
const tools = await MCP.tools()
|
||||
const serverTools = Object.keys(tools).filter((k) => k.startsWith("disc-server"))
|
||||
expect(serverTools.length).toBe(0)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
test(
|
||||
"connect() after disconnect() re-establishes the server",
|
||||
withInstance(
|
||||
{
|
||||
"reconn-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "reconn-server"
|
||||
const serverState = getOrCreateClientState("reconn-server")
|
||||
serverState.tools = [
|
||||
{ name: "my_tool", description: "a tool", inputSchema: { type: "object", properties: {} } },
|
||||
]
|
||||
|
||||
await MCP.add("reconn-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
await MCP.disconnect("reconn-server")
|
||||
expect((await MCP.status())["reconn-server"]?.status).toBe("disabled")
|
||||
|
||||
// Reconnect
|
||||
await MCP.connect("reconn-server")
|
||||
expect((await MCP.status())["reconn-server"]?.status).toBe("connected")
|
||||
|
||||
const tools = await MCP.tools()
|
||||
expect(Object.keys(tools).some((k) => k.includes("my_tool"))).toBe(true)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: add() closes existing client before replacing
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"add() closes the old client when replacing a server",
|
||||
// Don't put the server in config — add it dynamically so we control
|
||||
// exactly which client instance is "first" vs "second".
|
||||
withInstance({}, async () => {
|
||||
lastCreatedClientName = "replace-server"
|
||||
const firstState = getOrCreateClientState("replace-server")
|
||||
|
||||
await MCP.add("replace-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
expect(firstState.closed).toBe(false)
|
||||
|
||||
// Create new state for second client
|
||||
clientStates.delete("replace-server")
|
||||
const secondState = getOrCreateClientState("replace-server")
|
||||
|
||||
// Re-add should close the first client
|
||||
await MCP.add("replace-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
expect(firstState.closed).toBe(true)
|
||||
expect(secondState.closed).toBe(false)
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: state init with mixed success/failure
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"init connects available servers even when one fails",
|
||||
withInstance(
|
||||
{
|
||||
"good-server": {
|
||||
type: "local",
|
||||
command: ["echo", "good"],
|
||||
},
|
||||
"bad-server": {
|
||||
type: "local",
|
||||
command: ["echo", "bad"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
// Set up good server
|
||||
const goodState = getOrCreateClientState("good-server")
|
||||
goodState.tools = [
|
||||
{ name: "good_tool", description: "works", inputSchema: { type: "object", properties: {} } },
|
||||
]
|
||||
|
||||
// Set up bad server - will fail on listTools during create()
|
||||
const badState = getOrCreateClientState("bad-server")
|
||||
badState.listToolsShouldFail = true
|
||||
|
||||
// Add good server first
|
||||
lastCreatedClientName = "good-server"
|
||||
await MCP.add("good-server", {
|
||||
type: "local",
|
||||
command: ["echo", "good"],
|
||||
})
|
||||
|
||||
// Add bad server - should fail but not affect good server
|
||||
lastCreatedClientName = "bad-server"
|
||||
await MCP.add("bad-server", {
|
||||
type: "local",
|
||||
command: ["echo", "bad"],
|
||||
})
|
||||
|
||||
const status = await MCP.status()
|
||||
expect(status["good-server"]?.status).toBe("connected")
|
||||
expect(status["bad-server"]?.status).toBe("failed")
|
||||
|
||||
// Good server's tools should still be available
|
||||
const tools = await MCP.tools()
|
||||
expect(Object.keys(tools).some((k) => k.includes("good_tool"))).toBe(true)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: disabled server via config
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"disabled server is marked as disabled without attempting connection",
|
||||
withInstance(
|
||||
{
|
||||
"disabled-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
const countBefore = clientCreateCount
|
||||
|
||||
await MCP.add("disabled-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
enabled: false,
|
||||
} as any)
|
||||
|
||||
// No client should have been created
|
||||
expect(clientCreateCount).toBe(countBefore)
|
||||
|
||||
const status = await MCP.status()
|
||||
expect(status["disabled-server"]?.status).toBe("disabled")
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: prompts() and resources()
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"prompts() returns prompts from connected servers",
|
||||
withInstance(
|
||||
{
|
||||
"prompt-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "prompt-server"
|
||||
const serverState = getOrCreateClientState("prompt-server")
|
||||
serverState.prompts = [
|
||||
{ name: "my-prompt", description: "A test prompt" },
|
||||
]
|
||||
|
||||
await MCP.add("prompt-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const prompts = await MCP.prompts()
|
||||
expect(Object.keys(prompts).length).toBe(1)
|
||||
const key = Object.keys(prompts)[0]
|
||||
expect(key).toContain("prompt-server")
|
||||
expect(key).toContain("my-prompt")
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
test(
|
||||
"resources() returns resources from connected servers",
|
||||
withInstance(
|
||||
{
|
||||
"resource-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "resource-server"
|
||||
const serverState = getOrCreateClientState("resource-server")
|
||||
serverState.resources = [
|
||||
{ name: "my-resource", uri: "file:///test.txt", description: "A test resource" },
|
||||
]
|
||||
|
||||
await MCP.add("resource-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const resources = await MCP.resources()
|
||||
expect(Object.keys(resources).length).toBe(1)
|
||||
const key = Object.keys(resources)[0]
|
||||
expect(key).toContain("resource-server")
|
||||
expect(key).toContain("my-resource")
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
test(
|
||||
"prompts() skips disconnected servers",
|
||||
withInstance(
|
||||
{
|
||||
"prompt-disc-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "prompt-disc-server"
|
||||
const serverState = getOrCreateClientState("prompt-disc-server")
|
||||
serverState.prompts = [{ name: "hidden-prompt", description: "Should not appear" }]
|
||||
|
||||
await MCP.add("prompt-disc-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
await MCP.disconnect("prompt-disc-server")
|
||||
|
||||
const prompts = await MCP.prompts()
|
||||
expect(Object.keys(prompts).length).toBe(0)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect() on nonexistent server
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"connect() on nonexistent server does not throw",
|
||||
withInstance({}, async () => {
|
||||
// Should not throw
|
||||
await MCP.connect("nonexistent")
|
||||
const status = await MCP.status()
|
||||
expect(status["nonexistent"]).toBeUndefined()
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: disconnect() on nonexistent server
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"disconnect() on nonexistent server does not throw",
|
||||
withInstance({}, async () => {
|
||||
await MCP.disconnect("nonexistent")
|
||||
// Should complete without error
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: tools() with no MCP servers configured
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"tools() returns empty when no MCP servers are configured",
|
||||
withInstance({}, async () => {
|
||||
const tools = await MCP.tools()
|
||||
expect(Object.keys(tools).length).toBe(0)
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect failure during create()
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"server that fails to connect is marked as failed",
|
||||
withInstance(
|
||||
{
|
||||
"fail-connect": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "fail-connect"
|
||||
getOrCreateClientState("fail-connect")
|
||||
connectShouldFail = true
|
||||
connectError = "Connection refused"
|
||||
|
||||
await MCP.add("fail-connect", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const status = await MCP.status()
|
||||
expect(status["fail-connect"]?.status).toBe("failed")
|
||||
if (status["fail-connect"]?.status === "failed") {
|
||||
expect(status["fail-connect"].error).toContain("Connection refused")
|
||||
}
|
||||
|
||||
// No tools should be available
|
||||
const tools = await MCP.tools()
|
||||
expect(Object.keys(tools).length).toBe(0)
|
||||
},
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Bug #5: McpOAuthCallback.cancelPending uses wrong key
|
||||
// ========================================================================
|
||||
|
||||
test("McpOAuthCallback.cancelPending is keyed by mcpName but pendingAuths uses oauthState", async () => {
|
||||
const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback")
|
||||
|
||||
// Register a pending auth with an oauthState key, associated to an mcpName
|
||||
const oauthState = "abc123hexstate"
|
||||
const callbackPromise = McpOAuthCallback.waitForCallback(oauthState, "my-mcp-server")
|
||||
|
||||
// cancelPending is called with mcpName — should find the entry via reverse index
|
||||
McpOAuthCallback.cancelPending("my-mcp-server")
|
||||
|
||||
// The callback should still be pending because cancelPending looked up
|
||||
// "my-mcp-server" in a map keyed by "abc123hexstate"
|
||||
let resolved = false
|
||||
let rejected = false
|
||||
callbackPromise.then(() => (resolved = true)).catch(() => (rejected = true))
|
||||
|
||||
// Give it a tick
|
||||
await new Promise((r) => setTimeout(r, 50))
|
||||
|
||||
// cancelPending("my-mcp-server") should have rejected the pending callback
|
||||
expect(rejected).toBe(true)
|
||||
|
||||
await McpOAuthCallback.stop()
|
||||
})
|
||||
|
||||
// ========================================================================
|
||||
// Test: multiple tools from same server get correct name prefixes
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"tools() prefixes tool names with sanitized server name",
|
||||
withInstance(
|
||||
{
|
||||
"my.special-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
async () => {
|
||||
lastCreatedClientName = "my.special-server"
|
||||
const serverState = getOrCreateClientState("my.special-server")
|
||||
serverState.tools = [
|
||||
{ name: "tool-a", description: "Tool A", inputSchema: { type: "object", properties: {} } },
|
||||
{ name: "tool.b", description: "Tool B", inputSchema: { type: "object", properties: {} } },
|
||||
]
|
||||
|
||||
await MCP.add("my.special-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const tools = await MCP.tools()
|
||||
const keys = Object.keys(tools)
|
||||
|
||||
// Server name dots should be replaced with underscores
|
||||
expect(keys.some((k) => k.startsWith("my_special-server_"))).toBe(true)
|
||||
// Tool name dots should be replaced with underscores
|
||||
expect(keys.some((k) => k.endsWith("tool_b"))).toBe(true)
|
||||
expect(keys.length).toBe(2)
|
||||
},
|
||||
),
|
||||
)
|
||||
Reference in New Issue
Block a user