test(mcp): migrate lifecycle tests to Effect runner (#27205)
This commit is contained in:
Kit Langton
GitHub
parent
6d3b2fe08b
commit
68c4951318
1 changed files with 375 additions and 347 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { test, expect, mock, beforeEach } from "bun:test"
|
||||
import { InstanceRuntime } from "../../src/project/instance-runtime"
|
||||
import { Effect } from "effect"
|
||||
import { expect, mock, beforeEach } from "bun:test"
|
||||
import { Effect, Exit } from "effect"
|
||||
import type { MCP as MCPNS } from "../../src/mcp/index"
|
||||
import { testEffect } from "../lib/effect"
|
||||
|
||||
// --- Mock infrastructure ---
|
||||
|
||||
|
|
@ -179,39 +179,9 @@ beforeEach(() => {
|
|||
|
||||
// Import after mocks
|
||||
const { MCP } = await import("../../src/mcp/index")
|
||||
const { Instance } = await import("../../src/project/instance")
|
||||
const { WithInstance } = await import("../../src/project/with-instance")
|
||||
const { tmpdir } = await import("../fixture/fixture")
|
||||
const { McpOAuthCallback } = await import("../../src/mcp/oauth-callback")
|
||||
|
||||
// --- Helper ---
|
||||
|
||||
function withInstance(
|
||||
config: Record<string, unknown>,
|
||||
fn: (mcp: MCPNS.Interface) => Effect.Effect<void, unknown, never>,
|
||||
) {
|
||||
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 WithInstance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
await Effect.runPromise(MCP.Service.use(fn).pipe(Effect.provide(MCP.defaultLayer)))
|
||||
// dispose instance to clean up state between tests
|
||||
await InstanceRuntime.disposeInstance(Instance.current)
|
||||
},
|
||||
})
|
||||
}
|
||||
}
|
||||
const it = testEffect(MCP.defaultLayer)
|
||||
|
||||
function statusName(status: Record<string, MCPNS.Status> | MCPNS.Status, server: string) {
|
||||
if ("status" in status) return status.status
|
||||
|
|
@ -222,9 +192,10 @@ function statusName(status: Record<string, MCPNS.Status> | MCPNS.Status, server:
|
|||
// Test: tools() are cached after connect
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"tools() reuses cached tool definitions after connect",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "my-server"
|
||||
const serverState = getOrCreateClientState("my-server")
|
||||
|
|
@ -248,15 +219,17 @@ test(
|
|||
expect(serverState.listToolsCalls).toBe(1)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: tool change notifications refresh the cache
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"tool change notifications refresh cached tool definitions",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "status-server"
|
||||
const serverState = getOrCreateClientState("status-server")
|
||||
|
|
@ -270,7 +243,9 @@ test(
|
|||
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: {} } }]
|
||||
serverState.tools = [
|
||||
{ name: "next_tool", description: "next", inputSchema: { type: "object", properties: {} } },
|
||||
]
|
||||
|
||||
const handler = Array.from(serverState.notificationHandlers.values())[0]
|
||||
expect(handler).toBeDefined()
|
||||
|
|
@ -282,22 +257,17 @@ test(
|
|||
expect(serverState.listToolsCalls).toBe(2)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect() / disconnect() lifecycle
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"disconnect sets status to disabled and removes client",
|
||||
withInstance(
|
||||
{
|
||||
"disc-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "disc-server"
|
||||
getOrCreateClientState("disc-server")
|
||||
|
|
@ -315,24 +285,27 @@ test(
|
|||
const statusAfter = yield* mcp.status()
|
||||
expect(statusAfter["disc-server"]?.status).toBe("disabled")
|
||||
|
||||
// Tools should be empty after disconnect
|
||||
const tools = yield* 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": {
|
||||
config: {
|
||||
mcp: {
|
||||
"disc-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"connect() after disconnect() re-establishes the server",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "reconn-server"
|
||||
const serverState = getOrCreateClientState("reconn-server")
|
||||
|
|
@ -348,7 +321,6 @@ test(
|
|||
yield* mcp.disconnect("reconn-server")
|
||||
expect((yield* mcp.status())["reconn-server"]?.status).toBe("disabled")
|
||||
|
||||
// Reconnect
|
||||
yield* mcp.connect("reconn-server")
|
||||
expect((yield* mcp.status())["reconn-server"]?.status).toBe("connected")
|
||||
|
||||
|
|
@ -356,17 +328,28 @@ test(
|
|||
expect(Object.keys(tools).some((k) => k.includes("my_tool"))).toBe(true)
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"reconn-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: add() closes existing client before replacing
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"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({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "replace-server"
|
||||
const firstState = getOrCreateClientState("replace-server")
|
||||
|
|
@ -392,26 +375,17 @@ test(
|
|||
expect(secondState.closed).toBe(false)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: state init with mixed success/failure
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"init connects available servers even when one fails",
|
||||
withInstance(
|
||||
{
|
||||
"good-server": {
|
||||
type: "local",
|
||||
command: ["echo", "good"],
|
||||
},
|
||||
"bad-server": {
|
||||
type: "local",
|
||||
command: ["echo", "bad"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
// Set up good server
|
||||
const goodState = getOrCreateClientState("good-server")
|
||||
|
|
@ -444,11 +418,26 @@ test(
|
|||
expect(Object.keys(tools).some((k) => k.includes("good_tool"))).toBe(true)
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"good-server": {
|
||||
type: "local",
|
||||
command: ["echo", "good"],
|
||||
},
|
||||
"bad-server": {
|
||||
type: "local",
|
||||
command: ["echo", "bad"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"falls back when MCP output schema refs fail SDK tool discovery",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "stitch-like-server"
|
||||
const serverState = getOrCreateClientState("stitch-like-server")
|
||||
|
|
@ -476,11 +465,13 @@ test(
|
|||
expect(serverState.requestCalls).toBe(1)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"does not fall back for non-schema MCP tool discovery errors",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "broken-server"
|
||||
const serverState = getOrCreateClientState("broken-server")
|
||||
|
|
@ -497,23 +488,17 @@ test(
|
|||
expect(serverState.requestCalls).toBe(0)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: disabled server via config
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"disabled server is marked as disabled without attempting connection",
|
||||
withInstance(
|
||||
{
|
||||
"disabled-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
const countBefore = clientCreateCount
|
||||
|
||||
|
|
@ -530,22 +515,27 @@ test(
|
|||
expect(status["disabled-server"]?.status).toBe("disabled")
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"disabled-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
enabled: false,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: prompts() and resources()
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"prompts() returns prompts from connected servers",
|
||||
withInstance(
|
||||
{
|
||||
"prompt-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "prompt-server"
|
||||
const serverState = getOrCreateClientState("prompt-server")
|
||||
|
|
@ -563,18 +553,22 @@ test(
|
|||
expect(key).toContain("my-prompt")
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
test(
|
||||
"resources() returns resources from connected servers",
|
||||
withInstance(
|
||||
{
|
||||
"resource-server": {
|
||||
config: {
|
||||
mcp: {
|
||||
"prompt-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"resources() returns resources from connected servers",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "resource-server"
|
||||
const serverState = getOrCreateClientState("resource-server")
|
||||
|
|
@ -592,18 +586,22 @@ test(
|
|||
expect(key).toContain("my-resource")
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
test(
|
||||
"prompts() skips disconnected servers",
|
||||
withInstance(
|
||||
{
|
||||
"prompt-disc-server": {
|
||||
config: {
|
||||
mcp: {
|
||||
"resource-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
it.instance(
|
||||
"prompts() skips disconnected servers",
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "prompt-disc-server"
|
||||
const serverState = getOrCreateClientState("prompt-disc-server")
|
||||
|
|
@ -620,15 +618,26 @@ test(
|
|||
expect(Object.keys(prompts).length).toBe(0)
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"prompt-disc-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect() on nonexistent server
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"connect() on nonexistent server does not throw",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
// Should not throw
|
||||
yield* mcp.connect("nonexistent")
|
||||
|
|
@ -636,50 +645,49 @@ test(
|
|||
expect(status["nonexistent"]).toBeUndefined()
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: disconnect() on nonexistent server
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"disconnect() on nonexistent server does not throw",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
yield* mcp.disconnect("nonexistent")
|
||||
// Should complete without error
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: tools() with no MCP servers configured
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"tools() returns empty when no MCP servers are configured",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
const tools = yield* mcp.tools()
|
||||
expect(Object.keys(tools).length).toBe(0)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: connect failure during create()
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"server that fails to connect is marked as failed",
|
||||
withInstance(
|
||||
{
|
||||
"fail-connect": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "fail-connect"
|
||||
getOrCreateClientState("fail-connect")
|
||||
|
|
@ -702,50 +710,54 @@ test(
|
|||
expect(Object.keys(tools).length).toBe(0)
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"fail-connect": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// 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
|
||||
it.live("McpOAuthCallback.cancelPending is keyed by mcpName but pendingAuths uses oauthState", () =>
|
||||
Effect.acquireUseRelease(
|
||||
Effect.sync(() => McpOAuthCallback.waitForCallback("abc123hexstate", "my-mcp-server")),
|
||||
(callback) =>
|
||||
Effect.gen(function* () {
|
||||
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 rejected = false
|
||||
callbackPromise.then(() => {}).catch(() => (rejected = true))
|
||||
const exit = yield* Effect.tryPromise({
|
||||
try: () => callback,
|
||||
catch: (error) => (error instanceof Error ? error : new Error(String(error))),
|
||||
}).pipe(
|
||||
Effect.timeoutOrElse({
|
||||
duration: "1 second",
|
||||
orElse: () => Effect.fail(new Error("timed out waiting for OAuth cancellation")),
|
||||
}),
|
||||
Effect.exit,
|
||||
)
|
||||
|
||||
// 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()
|
||||
})
|
||||
expect(Exit.isFailure(exit)).toBe(true)
|
||||
}),
|
||||
() => Effect.promise(() => McpOAuthCallback.stop()).pipe(Effect.ignore),
|
||||
),
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: multiple tools from same server get correct name prefixes
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"tools() prefixes tool names with sanitized server name",
|
||||
withInstance(
|
||||
{
|
||||
"my.special-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
(mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "my.special-server"
|
||||
const serverState = getOrCreateClientState("my.special-server")
|
||||
|
|
@ -769,15 +781,26 @@ test(
|
|||
expect(keys.length).toBe(2)
|
||||
}),
|
||||
),
|
||||
{
|
||||
config: {
|
||||
mcp: {
|
||||
"my.special-server": {
|
||||
type: "local",
|
||||
command: ["echo", "test"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: transport leak — local stdio timeout (#19168)
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"local stdio transport is closed when connect times out (no process leak)",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "hanging-server"
|
||||
getOrCreateClientState("hanging-server")
|
||||
|
|
@ -796,15 +819,17 @@ test(
|
|||
expect(transportCloseCount).toBeGreaterThanOrEqual(1)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: transport leak — remote timeout (#19168)
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"remote transport is closed when connect times out",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "hanging-remote"
|
||||
getOrCreateClientState("hanging-remote")
|
||||
|
|
@ -823,15 +848,17 @@ test(
|
|||
expect(transportCloseCount).toBeGreaterThanOrEqual(1)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
||||
// ========================================================================
|
||||
// Test: transport leak — failed remote transports not closed (#19168)
|
||||
// ========================================================================
|
||||
|
||||
test(
|
||||
it.instance(
|
||||
"failed remote transport is closed before trying next transport",
|
||||
withInstance({}, (mcp) =>
|
||||
() =>
|
||||
MCP.Service.use((mcp: MCPNS.Interface) =>
|
||||
Effect.gen(function* () {
|
||||
lastCreatedClientName = "fail-remote"
|
||||
getOrCreateClientState("fail-remote")
|
||||
|
|
@ -851,4 +878,5 @@ test(
|
|||
expect(transportCloseCount).toBeGreaterThanOrEqual(2)
|
||||
}),
|
||||
),
|
||||
{ config: { mcp: {} } },
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue