Skip to content

Commit d3459eb

Browse files
authored
test(mcp): replace module mocks with real servers (anomalyco#35450)
1 parent be73f46 commit d3459eb

7 files changed

Lines changed: 1071 additions & 1876 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { LayerNode } from "@opencode-ai/core/effect/layer-node"
2+
import { Context, Effect, Layer } from "effect"
3+
import open from "open"
4+
5+
export interface Interface {
6+
readonly open: (url: string) => Effect.Effect<void, Error>
7+
}
8+
9+
export class Service extends Context.Service<Service, Interface>()("@opencode/McpBrowser") {}
10+
11+
const layer = Layer.succeed(
12+
Service,
13+
Service.of({
14+
open: Effect.fn("McpBrowser.open")(function* (url: string) {
15+
const subprocess = yield* Effect.tryPromise({
16+
try: () => open(url),
17+
catch: (error) => (error instanceof Error ? error : new Error(String(error))),
18+
})
19+
yield* Effect.callback<void, Error>((resume) => {
20+
const timer = setTimeout(() => resume(Effect.void), 500)
21+
subprocess.on("error", (error) => {
22+
clearTimeout(timer)
23+
resume(Effect.fail(error))
24+
})
25+
subprocess.on("exit", (code) => {
26+
if (code === null || code === 0) return
27+
clearTimeout(timer)
28+
resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
29+
})
30+
})
31+
}),
32+
}),
33+
)
34+
35+
export const node = LayerNode.make({ service: Service, layer, deps: [] })
36+
37+
export * as McpBrowser from "./browser"

packages/opencode/src/mcp/index.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ import { McpOAuthCallback } from "./oauth-callback"
2626
import { McpAuth } from "./auth"
2727
import { EventV2Bridge } from "@/event-v2-bridge"
2828
import { TuiEvent } from "@/server/tui-event"
29-
import open from "open"
3029
import { Cause, Effect, Exit, Layer, Context, Schema, Stream } from "effect"
3130
import { EffectBridge } from "@/effect/bridge"
3231
import { InstanceState } from "@/effect/instance-state"
3332
import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"
3433
import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner"
3534
import { McpCatalog } from "./catalog"
3635
import { McpEvent } from "@opencode-ai/schema/mcp-event"
36+
import { McpBrowser } from "./browser"
3737

3838
const DEFAULT_TIMEOUT = 30_000
3939
const CLIENT_OPTIONS = {
@@ -207,6 +207,7 @@ const layer = Layer.effect(
207207
const spawner = yield* ChildProcessSpawner.ChildProcessSpawner
208208
const auth = yield* McpAuth.Service
209209
const events = yield* EventV2Bridge.Service
210+
const browser = yield* McpBrowser.Service
210211

211212
type Transport = StdioClientTransport | StreamableHTTPClientTransport | SSEClientTransport
212213

@@ -897,22 +898,7 @@ const layer = Layer.effect(
897898
const callbackPromise = McpOAuthCallback.waitForCallback(result.oauthState, mcpName)
898899
onAuthorization?.(result.authorizationUrl)
899900

900-
yield* Effect.tryPromise(() => open(result.authorizationUrl)).pipe(
901-
Effect.flatMap((subprocess) =>
902-
Effect.callback<void, Error>((resume) => {
903-
const timer = setTimeout(() => resume(Effect.void), 500)
904-
subprocess.on("error", (err) => {
905-
clearTimeout(timer)
906-
resume(Effect.fail(err))
907-
})
908-
subprocess.on("exit", (code) => {
909-
if (code !== null && code !== 0) {
910-
clearTimeout(timer)
911-
resume(Effect.fail(new Error(`Browser open failed with exit code ${code}`)))
912-
}
913-
})
914-
}),
915-
),
901+
yield* browser.open(result.authorizationUrl).pipe(
916902
Effect.catch(() => {
917903
return events.publish(BrowserOpenFailed, { mcpName, url: result.authorizationUrl }).pipe(Effect.ignore)
918904
}),
@@ -1012,7 +998,7 @@ export type AuthStatus = "authenticated" | "expired" | "not_authenticated"
1012998
export const node = LayerNode.make({
1013999
service: Service,
10141000
layer: layer,
1015-
deps: [CrossSpawnSpawner.node, McpAuth.node, EventV2Bridge.node, Config.node],
1001+
deps: [CrossSpawnSpawner.node, McpAuth.node, EventV2Bridge.node, Config.node, McpBrowser.node],
10161002
})
10171003

10181004
export * as MCP from "."
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { Server } from "@modelcontextprotocol/sdk/server/index.js"
2+
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js"
3+
import { ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"
4+
5+
if (process.argv.includes("--hang")) {
6+
const pidFile = process.env.MCP_LIFECYCLE_PID_FILE
7+
if (!pidFile) throw new Error("MCP_LIFECYCLE_PID_FILE is required")
8+
await Bun.write(pidFile, String(process.pid))
9+
await new Promise(() => {})
10+
}
11+
12+
const server = new Server({ name: "mcp-lifecycle-stdio", version: "1.0.0" }, { capabilities: { tools: {} } })
13+
14+
server.setRequestHandler(ListToolsRequestSchema, () =>
15+
Promise.resolve({
16+
tools: [
17+
{
18+
name: "current_directory",
19+
description: process.cwd(),
20+
inputSchema: { type: "object", properties: {} },
21+
},
22+
],
23+
}),
24+
)
25+
26+
await server.connect(new StdioServerTransport())

packages/opencode/test/mcp/headers.test.ts

Lines changed: 69 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,126 +1,101 @@
1-
import { describe, expect, mock, beforeEach } from "bun:test"
1+
import { describe, expect } from "bun:test"
2+
import { Server } from "@modelcontextprotocol/sdk/server/index.js"
3+
import { WebStandardStreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/webStandardStreamableHttp.js"
4+
import { ListToolsRequestSchema } from "@modelcontextprotocol/sdk/types.js"
25
import { LayerNode } from "@opencode-ai/core/effect/layer-node"
36
import { Effect } from "effect"
47
import { testEffect } from "../lib/effect"
8+
import { MCP } from "../../src/mcp/index"
59

6-
// Track what options were passed to each transport constructor
7-
const transportCalls: Array<{
8-
type: "streamable" | "sse"
9-
url: string
10-
options: { authProvider?: unknown; requestInit?: RequestInit }
11-
}> = []
12-
13-
// Mock the transport constructors to capture their arguments
14-
void mock.module("@modelcontextprotocol/sdk/client/streamableHttp.js", () => ({
15-
StreamableHTTPClientTransport: class MockStreamableHTTP {
16-
constructor(url: URL, options?: { authProvider?: unknown; requestInit?: RequestInit }) {
17-
transportCalls.push({
18-
type: "streamable",
19-
url: url.toString(),
20-
options: options ?? {},
21-
})
22-
}
23-
async start() {
24-
throw new Error("Mock transport cannot connect")
25-
}
26-
},
27-
}))
10+
const it = testEffect(LayerNode.compile(MCP.node))
2811

29-
void mock.module("@modelcontextprotocol/sdk/client/sse.js", () => ({
30-
SSEClientTransport: class MockSSE {
31-
constructor(url: URL, options?: { authProvider?: unknown; requestInit?: RequestInit }) {
32-
transportCalls.push({
33-
type: "sse",
34-
url: url.toString(),
35-
options: options ?? {},
36-
})
12+
const serve = Effect.acquireRelease(
13+
Effect.promise(async () => {
14+
const requests: Headers[] = []
15+
const protocol = new Server({ name: "headers", version: "1.0.0" }, { capabilities: { tools: {} } })
16+
protocol.setRequestHandler(ListToolsRequestSchema, () => Promise.resolve({ tools: [] }))
17+
const transport = new WebStandardStreamableHTTPServerTransport({
18+
sessionIdGenerator: () => crypto.randomUUID(),
19+
enableJsonResponse: true,
20+
})
21+
await protocol.connect(transport)
22+
const http = Bun.serve({
23+
port: 0,
24+
fetch(request) {
25+
requests.push(new Headers(request.headers))
26+
return transport.handleRequest(request)
27+
},
28+
})
29+
return {
30+
requests,
31+
url: http.url.toString(),
32+
close: async () => {
33+
await http.stop(true)
34+
await protocol.close()
35+
},
3736
}
38-
async start() {
39-
throw new Error("Mock transport cannot connect")
40-
}
41-
},
42-
}))
43-
44-
beforeEach(() => {
45-
transportCalls.length = 0
46-
})
47-
48-
// Import MCP after mocking
49-
const { MCP } = await import("../../src/mcp/index")
50-
const it = testEffect(LayerNode.compile(MCP.node))
37+
}),
38+
(server) => Effect.promise(server.close),
39+
)
5140

5241
describe("mcp.headers", () => {
5342
it.instance("headers are passed to transports when oauth is enabled (default)", () =>
5443
Effect.gen(function* () {
44+
const server = yield* serve
5545
const mcp = yield* MCP.Service
56-
yield* mcp
57-
.add("test-server", {
58-
type: "remote",
59-
url: "https://example.com/mcp",
60-
headers: {
61-
Authorization: "Bearer test-token",
62-
"X-Custom-Header": "custom-value",
63-
},
64-
})
65-
.pipe(Effect.catch(() => Effect.void))
66-
67-
// Both transports should have been created with headers
68-
expect(transportCalls.length).toBeGreaterThanOrEqual(1)
69-
70-
for (const call of transportCalls) {
71-
expect(call.options.requestInit).toBeDefined()
72-
expect(call.options.requestInit?.headers).toEqual({
46+
const result = yield* mcp.add("test-server", {
47+
type: "remote",
48+
url: server.url,
49+
headers: {
7350
Authorization: "Bearer test-token",
7451
"X-Custom-Header": "custom-value",
75-
})
76-
// OAuth should be enabled by default, so authProvider should exist
77-
expect(call.options.authProvider).toBeDefined()
52+
},
53+
})
54+
55+
expect(result.status).toMatchObject({ "test-server": { status: "connected" } })
56+
expect(server.requests.length).toBeGreaterThan(0)
57+
for (const headers of server.requests) {
58+
expect(headers.get("authorization")).toBe("Bearer test-token")
59+
expect(headers.get("x-custom-header")).toBe("custom-value")
7860
}
7961
}),
8062
)
8163

8264
it.instance("headers are passed to transports when oauth is explicitly disabled", () =>
8365
Effect.gen(function* () {
66+
const server = yield* serve
8467
const mcp = yield* MCP.Service
85-
yield* mcp
86-
.add("test-server-no-oauth", {
87-
type: "remote",
88-
url: "https://example.com/mcp",
89-
oauth: false,
90-
headers: {
91-
Authorization: "Bearer test-token",
92-
},
93-
})
94-
.pipe(Effect.catch(() => Effect.void))
95-
96-
expect(transportCalls.length).toBeGreaterThanOrEqual(1)
97-
98-
for (const call of transportCalls) {
99-
expect(call.options.requestInit).toBeDefined()
100-
expect(call.options.requestInit?.headers).toEqual({
68+
const result = yield* mcp.add("test-server-no-oauth", {
69+
type: "remote",
70+
url: server.url,
71+
oauth: false,
72+
headers: {
10173
Authorization: "Bearer test-token",
102-
})
103-
// OAuth is disabled, so no authProvider
104-
expect(call.options.authProvider).toBeUndefined()
74+
},
75+
})
76+
77+
expect(result.status).toMatchObject({ "test-server-no-oauth": { status: "connected" } })
78+
expect(server.requests.length).toBeGreaterThan(0)
79+
for (const headers of server.requests) {
80+
expect(headers.get("authorization")).toBe("Bearer test-token")
10581
}
10682
}),
10783
)
10884

10985
it.instance("no requestInit when headers are not provided", () =>
11086
Effect.gen(function* () {
87+
const server = yield* serve
11188
const mcp = yield* MCP.Service
112-
yield* mcp
113-
.add("test-server-no-headers", {
114-
type: "remote",
115-
url: "https://example.com/mcp",
116-
})
117-
.pipe(Effect.catch(() => Effect.void))
118-
119-
expect(transportCalls.length).toBeGreaterThanOrEqual(1)
89+
const result = yield* mcp.add("test-server-no-headers", {
90+
type: "remote",
91+
url: server.url,
92+
})
12093

121-
for (const call of transportCalls) {
122-
// No headers means requestInit should be undefined
123-
expect(call.options.requestInit).toBeUndefined()
94+
expect(result.status).toMatchObject({ "test-server-no-headers": { status: "connected" } })
95+
expect(server.requests.length).toBeGreaterThan(0)
96+
for (const headers of server.requests) {
97+
expect(headers.has("authorization")).toBe(false)
98+
expect(headers.has("x-custom-header")).toBe(false)
12499
}
125100
}),
126101
)

0 commit comments

Comments
 (0)