fix+refactor(mcp): lifecycle tests, cancelPending fix, Effect migration (#19042)
Kit Langton
GitHub
parent
8864fdce2f
commit
b90de755f9
|
|
@ -175,4 +175,4 @@ Still open and likely worth migrating:
|
|||
- [ ] `Provider`
|
||||
- [x] `Project`
|
||||
- [ ] `LSP`
|
||||
- [ ] `MCP`
|
||||
- [x] `MCP`
|
||||
|
|
|
|||
|
|
@ -1,7 +1,9 @@
|
|||
import path from "path"
|
||||
import z from "zod"
|
||||
import { Global } from "../global"
|
||||
import { Filesystem } from "../util/filesystem"
|
||||
import { Effect, Layer, ServiceMap } from "effect"
|
||||
import { AppFileSystem } from "@/filesystem"
|
||||
import { makeRunPromise } from "@/effect/run-service"
|
||||
|
||||
export namespace McpAuth {
|
||||
export const Tokens = z.object({
|
||||
|
|
@ -25,106 +27,155 @@ export namespace McpAuth {
|
|||
clientInfo: ClientInfo.optional(),
|
||||
codeVerifier: z.string().optional(),
|
||||
oauthState: z.string().optional(),
|
||||
serverUrl: z.string().optional(), // Track the URL these credentials are for
|
||||
serverUrl: z.string().optional(),
|
||||
})
|
||||
export type Entry = z.infer<typeof Entry>
|
||||
|
||||
const filepath = path.join(Global.Path.data, "mcp-auth.json")
|
||||
|
||||
export async function get(mcpName: string): Promise<Entry | undefined> {
|
||||
const data = await all()
|
||||
return data[mcpName]
|
||||
export interface Interface {
|
||||
readonly all: () => Effect.Effect<Record<string, Entry>>
|
||||
readonly get: (mcpName: string) => Effect.Effect<Entry | undefined>
|
||||
readonly getForUrl: (mcpName: string, serverUrl: string) => Effect.Effect<Entry | undefined>
|
||||
readonly set: (mcpName: string, entry: Entry, serverUrl?: string) => Effect.Effect<void>
|
||||
readonly remove: (mcpName: string) => Effect.Effect<void>
|
||||
readonly updateTokens: (mcpName: string, tokens: Tokens, serverUrl?: string) => Effect.Effect<void>
|
||||
readonly updateClientInfo: (mcpName: string, clientInfo: ClientInfo, serverUrl?: string) => Effect.Effect<void>
|
||||
readonly updateCodeVerifier: (mcpName: string, codeVerifier: string) => Effect.Effect<void>
|
||||
readonly clearCodeVerifier: (mcpName: string) => Effect.Effect<void>
|
||||
readonly updateOAuthState: (mcpName: string, oauthState: string) => Effect.Effect<void>
|
||||
readonly getOAuthState: (mcpName: string) => Effect.Effect<string | undefined>
|
||||
readonly clearOAuthState: (mcpName: string) => Effect.Effect<void>
|
||||
readonly isTokenExpired: (mcpName: string) => Effect.Effect<boolean | null>
|
||||
}
|
||||
|
||||
/**
|
||||
* Get auth entry and validate it's for the correct URL.
|
||||
* Returns undefined if URL has changed (credentials are invalid).
|
||||
*/
|
||||
export async function getForUrl(mcpName: string, serverUrl: string): Promise<Entry | undefined> {
|
||||
const entry = await get(mcpName)
|
||||
if (!entry) return undefined
|
||||
export class Service extends ServiceMap.Service<Service, Interface>()("@opencode/McpAuth") {}
|
||||
|
||||
// If no serverUrl is stored, this is from an old version - consider it invalid
|
||||
if (!entry.serverUrl) return undefined
|
||||
export const layer = Layer.effect(
|
||||
Service,
|
||||
Effect.gen(function* () {
|
||||
const fs = yield* AppFileSystem.Service
|
||||
|
||||
// If URL has changed, credentials are invalid
|
||||
if (entry.serverUrl !== serverUrl) return undefined
|
||||
const all = Effect.fn("McpAuth.all")(function* () {
|
||||
return yield* fs.readJson(filepath).pipe(
|
||||
Effect.map((data) => data as Record<string, Entry>),
|
||||
Effect.catch(() => Effect.succeed({} as Record<string, Entry>)),
|
||||
)
|
||||
})
|
||||
|
||||
return entry
|
||||
}
|
||||
const get = Effect.fn("McpAuth.get")(function* (mcpName: string) {
|
||||
const data = yield* all()
|
||||
return data[mcpName]
|
||||
})
|
||||
|
||||
export async function all(): Promise<Record<string, Entry>> {
|
||||
return Filesystem.readJson<Record<string, Entry>>(filepath).catch(() => ({}))
|
||||
}
|
||||
const getForUrl = Effect.fn("McpAuth.getForUrl")(function* (mcpName: string, serverUrl: string) {
|
||||
const entry = yield* get(mcpName)
|
||||
if (!entry) return undefined
|
||||
if (!entry.serverUrl) return undefined
|
||||
if (entry.serverUrl !== serverUrl) return undefined
|
||||
return entry
|
||||
})
|
||||
|
||||
export async function set(mcpName: string, entry: Entry, serverUrl?: string): Promise<void> {
|
||||
const data = await all()
|
||||
// Always update serverUrl if provided
|
||||
if (serverUrl) {
|
||||
entry.serverUrl = serverUrl
|
||||
}
|
||||
await Filesystem.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600)
|
||||
}
|
||||
const set = Effect.fn("McpAuth.set")(function* (mcpName: string, entry: Entry, serverUrl?: string) {
|
||||
const data = yield* all()
|
||||
if (serverUrl) entry.serverUrl = serverUrl
|
||||
yield* fs.writeJson(filepath, { ...data, [mcpName]: entry }, 0o600).pipe(Effect.orDie)
|
||||
})
|
||||
|
||||
export async function remove(mcpName: string): Promise<void> {
|
||||
const data = await all()
|
||||
delete data[mcpName]
|
||||
await Filesystem.writeJson(filepath, data, 0o600)
|
||||
}
|
||||
const remove = Effect.fn("McpAuth.remove")(function* (mcpName: string) {
|
||||
const data = yield* all()
|
||||
delete data[mcpName]
|
||||
yield* fs.writeJson(filepath, data, 0o600).pipe(Effect.orDie)
|
||||
})
|
||||
|
||||
export async function updateTokens(mcpName: string, tokens: Tokens, serverUrl?: string): Promise<void> {
|
||||
const entry = (await get(mcpName)) ?? {}
|
||||
entry.tokens = tokens
|
||||
await set(mcpName, entry, serverUrl)
|
||||
}
|
||||
const updateField = <K extends keyof Entry>(field: K, spanName: string) =>
|
||||
Effect.fn(`McpAuth.${spanName}`)(function* (mcpName: string, value: NonNullable<Entry[K]>, serverUrl?: string) {
|
||||
const entry = (yield* get(mcpName)) ?? {}
|
||||
entry[field] = value
|
||||
yield* set(mcpName, entry, serverUrl)
|
||||
})
|
||||
|
||||
export async function updateClientInfo(mcpName: string, clientInfo: ClientInfo, serverUrl?: string): Promise<void> {
|
||||
const entry = (await get(mcpName)) ?? {}
|
||||
entry.clientInfo = clientInfo
|
||||
await set(mcpName, entry, serverUrl)
|
||||
}
|
||||
const clearField = <K extends keyof Entry>(field: K, spanName: string) =>
|
||||
Effect.fn(`McpAuth.${spanName}`)(function* (mcpName: string) {
|
||||
const entry = yield* get(mcpName)
|
||||
if (entry) {
|
||||
delete entry[field]
|
||||
yield* set(mcpName, entry)
|
||||
}
|
||||
})
|
||||
|
||||
export async function updateCodeVerifier(mcpName: string, codeVerifier: string): Promise<void> {
|
||||
const entry = (await get(mcpName)) ?? {}
|
||||
entry.codeVerifier = codeVerifier
|
||||
await set(mcpName, entry)
|
||||
}
|
||||
const updateTokens = updateField("tokens", "updateTokens")
|
||||
const updateClientInfo = updateField("clientInfo", "updateClientInfo")
|
||||
const updateCodeVerifier = updateField("codeVerifier", "updateCodeVerifier")
|
||||
const updateOAuthState = updateField("oauthState", "updateOAuthState")
|
||||
const clearCodeVerifier = clearField("codeVerifier", "clearCodeVerifier")
|
||||
const clearOAuthState = clearField("oauthState", "clearOAuthState")
|
||||
|
||||
export async function clearCodeVerifier(mcpName: string): Promise<void> {
|
||||
const entry = await get(mcpName)
|
||||
if (entry) {
|
||||
delete entry.codeVerifier
|
||||
await set(mcpName, entry)
|
||||
}
|
||||
}
|
||||
const getOAuthState = Effect.fn("McpAuth.getOAuthState")(function* (mcpName: string) {
|
||||
const entry = yield* get(mcpName)
|
||||
return entry?.oauthState
|
||||
})
|
||||
|
||||
export async function updateOAuthState(mcpName: string, oauthState: string): Promise<void> {
|
||||
const entry = (await get(mcpName)) ?? {}
|
||||
entry.oauthState = oauthState
|
||||
await set(mcpName, entry)
|
||||
}
|
||||
const isTokenExpired = Effect.fn("McpAuth.isTokenExpired")(function* (mcpName: string) {
|
||||
const entry = yield* get(mcpName)
|
||||
if (!entry?.tokens) return null
|
||||
if (!entry.tokens.expiresAt) return false
|
||||
return entry.tokens.expiresAt < Date.now() / 1000
|
||||
})
|
||||
|
||||
export async function getOAuthState(mcpName: string): Promise<string | undefined> {
|
||||
const entry = await get(mcpName)
|
||||
return entry?.oauthState
|
||||
}
|
||||
return Service.of({
|
||||
all,
|
||||
get,
|
||||
getForUrl,
|
||||
set,
|
||||
remove,
|
||||
updateTokens,
|
||||
updateClientInfo,
|
||||
updateCodeVerifier,
|
||||
clearCodeVerifier,
|
||||
updateOAuthState,
|
||||
getOAuthState,
|
||||
clearOAuthState,
|
||||
isTokenExpired,
|
||||
})
|
||||
}),
|
||||
)
|
||||
|
||||
export async function clearOAuthState(mcpName: string): Promise<void> {
|
||||
const entry = await get(mcpName)
|
||||
if (entry) {
|
||||
delete entry.oauthState
|
||||
await set(mcpName, entry)
|
||||
}
|
||||
}
|
||||
const defaultLayer = layer.pipe(Layer.provide(AppFileSystem.defaultLayer))
|
||||
|
||||
/**
|
||||
* Check if stored tokens are expired.
|
||||
* Returns null if no tokens exist, false if no expiry or not expired, true if expired.
|
||||
*/
|
||||
export async function isTokenExpired(mcpName: string): Promise<boolean | null> {
|
||||
const entry = await get(mcpName)
|
||||
if (!entry?.tokens) return null
|
||||
if (!entry.tokens.expiresAt) return false
|
||||
return entry.tokens.expiresAt < Date.now() / 1000
|
||||
}
|
||||
const runPromise = makeRunPromise(Service, defaultLayer)
|
||||
|
||||
// Async facades for backward compat (used by McpOAuthProvider, CLI)
|
||||
|
||||
export const get = async (mcpName: string) => runPromise((svc) => svc.get(mcpName))
|
||||
|
||||
export const getForUrl = async (mcpName: string, serverUrl: string) =>
|
||||
runPromise((svc) => svc.getForUrl(mcpName, serverUrl))
|
||||
|
||||
export const all = async () => runPromise((svc) => svc.all())
|
||||
|
||||
export const set = async (mcpName: string, entry: Entry, serverUrl?: string) =>
|
||||
runPromise((svc) => svc.set(mcpName, entry, serverUrl))
|
||||
|
||||
export const remove = async (mcpName: string) => runPromise((svc) => svc.remove(mcpName))
|
||||
|
||||
export const updateTokens = async (mcpName: string, tokens: Tokens, serverUrl?: string) =>
|
||||
runPromise((svc) => svc.updateTokens(mcpName, tokens, serverUrl))
|
||||
|
||||
export const updateClientInfo = async (mcpName: string, clientInfo: ClientInfo, serverUrl?: string) =>
|
||||
runPromise((svc) => svc.updateClientInfo(mcpName, clientInfo, serverUrl))
|
||||
|
||||
export const updateCodeVerifier = async (mcpName: string, codeVerifier: string) =>
|
||||
runPromise((svc) => svc.updateCodeVerifier(mcpName, codeVerifier))
|
||||
|
||||
export const clearCodeVerifier = async (mcpName: string) => runPromise((svc) => svc.clearCodeVerifier(mcpName))
|
||||
|
||||
export const updateOAuthState = async (mcpName: string, oauthState: string) =>
|
||||
runPromise((svc) => svc.updateOAuthState(mcpName, oauthState))
|
||||
|
||||
export const getOAuthState = async (mcpName: string) => runPromise((svc) => svc.getOAuthState(mcpName))
|
||||
|
||||
export const clearOAuthState = async (mcpName: string) => runPromise((svc) => svc.clearOAuthState(mcpName))
|
||||
|
||||
export const isTokenExpired = async (mcpName: string) => runPromise((svc) => svc.isTokenExpired(mcpName))
|
||||
}
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load Diff
|
|
@ -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
|
||||
|
||||
|
|
@ -98,6 +101,12 @@ export namespace McpOAuthCallback {
|
|||
const pending = pendingAuths.get(state)!
|
||||
clearTimeout(pending.timeout)
|
||||
pendingAuths.delete(state)
|
||||
for (const [name, s] of mcpNameToState) {
|
||||
if (s === state) {
|
||||
mcpNameToState.delete(name)
|
||||
break
|
||||
}
|
||||
}
|
||||
pending.reject(new Error(errorMsg))
|
||||
}
|
||||
return new Response(HTML_ERROR(errorMsg), {
|
||||
|
|
@ -126,6 +135,13 @@ export namespace McpOAuthCallback {
|
|||
|
||||
clearTimeout(pending.timeout)
|
||||
pendingAuths.delete(state)
|
||||
// Clean up reverse index
|
||||
for (const [name, s] of mcpNameToState) {
|
||||
if (s === state) {
|
||||
mcpNameToState.delete(name)
|
||||
break
|
||||
}
|
||||
}
|
||||
pending.resolve(code)
|
||||
|
||||
return new Response(HTML_SUCCESS, {
|
||||
|
|
@ -137,11 +153,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 +169,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 +206,7 @@ export namespace McpOAuthCallback {
|
|||
pending.reject(new Error("OAuth callback server stopped"))
|
||||
}
|
||||
pendingAuths.clear()
|
||||
mcpNameToState.clear()
|
||||
}
|
||||
|
||||
export function isRunning(): boolean {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,660 @@
|
|||
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 }>
|
||||
listToolsCalls: number
|
||||
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: {} } }],
|
||||
listToolsCalls: 0,
|
||||
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) this._state.listToolsCalls++
|
||||
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()
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
// Test: tools() are cached after connect
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"tools() reuses cached tool definitions after connect",
|
||||
withInstance({}, 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")
|
||||
|
||||
expect(serverState.listToolsCalls).toBe(1)
|
||||
|
||||
const toolsA = await MCP.tools()
|
||||
const toolsB = await MCP.tools()
|
||||
expect(Object.keys(toolsA).length).toBeGreaterThan(0)
|
||||
expect(Object.keys(toolsB).length).toBeGreaterThan(0)
|
||||
expect(serverState.listToolsCalls).toBe(1)
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: tool change notifications refresh the cache
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
"tool change notifications refresh cached tool definitions",
|
||||
withInstance({}, async () => {
|
||||
lastCreatedClientName = "status-server"
|
||||
const serverState = getOrCreateClientState("status-server")
|
||||
|
||||
await MCP.add("status-server", {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
})
|
||||
|
||||
const before = await MCP.tools()
|
||||
expect(Object.keys(before).some((key) => key.includes("test_tool"))).toBe(true)
|
||||
expect(serverState.listToolsCalls).toBe(1)
|
||||
|
||||
serverState.tools = [{ name: "next_tool", description: "next", inputSchema: { type: "object", properties: {} } }]
|
||||
|
||||
const handler = Array.from(serverState.notificationHandlers.values())[0]
|
||||
expect(handler).toBeDefined()
|
||||
await handler?.()
|
||||
|
||||
const after = await MCP.tools()
|
||||
expect(Object.keys(after).some((key) => key.includes("next_tool"))).toBe(true)
|
||||
expect(Object.keys(after).some((key) => key.includes("test_tool"))).toBe(false)
|
||||
expect(serverState.listToolsCalls).toBe(2)
|
||||
}),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// 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)
|
||||
},
|
||||
),
|
||||
)
|
||||
Loading…
Reference in New Issue