Skip to content

fix(mcp): harden request timeouts and initialization #1256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Astro-Han merged 1 commit into from
Jun 10, 2026
Merged

fix(mcp): harden request timeouts and initialization #1256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 53 additions & 28 deletions packages/opencode/src/mcp/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { McpAuth } from "./auth"
import { BusEvent } from "../bus/bus-event"
import { Bus } from "@/bus"
import open from "open"
import { Effect, Exit, Layer, Option, Context, Stream } from "effect"
import { Cause, Effect, Exit, Layer, Option, Context, Stream } from "effect"
import { EffectBridge, type Shape as EffectBridgeShape } from "@/effect/bridge"
import { InstanceState } from "@/effect/instance-state"
import { makeRuntime } from "@/effect/run-service"
Expand Down Expand Up @@ -504,30 +504,45 @@ export namespace MCP {
})

const create = Effect.fn("MCP.create")(function* (key: string, mcp: Config.Mcp) {
if (mcp.enabled === false) {
log.info("mcp server disabled", { key })
return DISABLED_RESULT
}
return yield* Effect.gen(function* () {
if (mcp.enabled === false) {
log.info("mcp server disabled", { key })
return DISABLED_RESULT
}

log.info("found", { key, type: mcp.type })
log.info("found", { key, type: mcp.type })

const { client: mcpClient, status } =
mcp.type === "remote"
? yield* connectRemote(key, mcp as Config.Mcp & { type: "remote" })
: yield* connectLocal(key, mcp as Config.Mcp & { type: "local" })
const { client: mcpClient, status } =
mcp.type === "remote"
? yield* connectRemote(key, mcp as Config.Mcp & { type: "remote" })
: yield* connectLocal(key, mcp as Config.Mcp & { type: "local" })

if (!mcpClient) {
return { status } satisfies CreateResult
}
if (!mcpClient) {
return { status } satisfies CreateResult
}

const listed = hasCapability(mcpClient, "tools") ? yield* defs(key, mcpClient, mcp.timeout) : []
if (!listed) {
yield* Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore)
return { status: { status: "failed", error: "Failed to get tools" } } satisfies CreateResult
}
return yield* Effect.gen(function* () {
const listed = hasCapability(mcpClient, "tools") ? yield* defs(key, mcpClient, mcp.timeout) : []
if (!listed) {
return yield* Effect.fail(new Error("Failed to get tools"))
}

log.info("create() successfully created client", { key, toolCount: listed.length })
return { mcpClient, status, defs: listed } satisfies CreateResult
log.info("create() successfully created client", { key, toolCount: listed.length })
return { mcpClient, status, defs: listed } satisfies CreateResult
}).pipe(
Effect.catchCause((cause) =>
Effect.tryPromise(() => mcpClient.close()).pipe(Effect.ignore, Effect.andThen(Effect.failCause(cause))),
),
)
}).pipe(
Effect.catchCause((cause) => {
if (Cause.hasInterruptsOnly(cause)) return Effect.interrupt
const error = Cause.squash(cause)
return Effect.succeed<CreateResult>({
status: { status: "failed", error: error instanceof Error ? error.message : String(error) },
})
}),
)
})
const cfgSvc = yield* Config.Service

Expand Down Expand Up @@ -795,7 +810,7 @@ export namespace MCP {

const withClient = Effect.fnUntraced(function* <A>(
clientName: string,
fn: (client: MCPClient) => Promise<A>,
fn: (client: MCPClient, timeout?: number) => Promise<A>,
label: string,
meta?: Record<string, unknown>,
) {
Expand All @@ -805,8 +820,12 @@ export namespace MCP {
log.warn(`client not found for ${label}`, { clientName })
return undefined
}
const cfg = yield* cfgSvc.get()
const configured = s.config[clientName] ?? cfg.mcp?.[clientName]
const entry = configured && isMcpConfigured(configured) ? configured : undefined
const timeout = entry?.timeout ?? cfg.experimental?.mcp_timeout
return yield* Effect.tryPromise({
try: () => fn(client),
try: () => fn(client, timeout),
catch: (e: any) => {
log.error(`failed to ${label}`, { clientName, ...meta, error: e?.message })
return e
Expand All @@ -819,15 +838,21 @@ export namespace MCP {
name: string,
args?: Record<string, string>,
) {
return yield* withClient(clientName, (client) => client.getPrompt({ name, arguments: args }), "getPrompt", {
promptName: name,
})
return yield* withClient(
clientName,
(client, timeout) => client.getPrompt({ name, arguments: args }, { timeout }),
"getPrompt",
{ promptName: name },
)
})

const readResource = Effect.fn("MCP.readResource")(function* (clientName: string, resourceUri: string) {
return yield* withClient(clientName, (client) => client.readResource({ uri: resourceUri }), "readResource", {
resourceUri,
})
return yield* withClient(
clientName,
(client, timeout) => client.readResource({ uri: resourceUri }, { timeout }),
"readResource",
{ resourceUri },
)
})

const getMcpConfig = Effect.fnUntraced(function* (mcpName: string) {
Expand Down
80 changes: 79 additions & 1 deletion packages/opencode/test/mcp/lifecycle.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ interface MockClientState {
listToolsCalls: number
listPromptsCalls: number
listResourcesCalls: number
getPromptTimeouts: Array<number | undefined>
readResourceTimeouts: Array<number | undefined>
requestCalls: number
capabilitiesShouldThrow: boolean
listToolsShouldFail: boolean
listToolsError: string
listPromptsShouldFail: boolean
Expand Down Expand Up @@ -54,7 +57,10 @@ function getOrCreateClientState(name?: string): MockClientState {
listToolsCalls: 0,
listPromptsCalls: 0,
listResourcesCalls: 0,
getPromptTimeouts: [],
readResourceTimeouts: [],
requestCalls: 0,
capabilitiesShouldThrow: false,
listToolsShouldFail: false,
listToolsError: "listTools failed",
listPromptsShouldFail: false,
Expand Down Expand Up @@ -153,6 +159,7 @@ mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
}

getServerCapabilities() {
if (this._state?.capabilitiesShouldThrow) throw new Error("capability discovery failed")
return this._state?.capabilities
}

Expand Down Expand Up @@ -201,6 +208,16 @@ mock.module("@modelcontextprotocol/sdk/client/index.js", () => ({
return { resources: this._state?.resources ?? [] }
}

async getPrompt(_params: unknown, options?: { timeout?: number }) {
this._state?.getPromptTimeouts.push(options?.timeout)
return { messages: [] }
}

async readResource(params: { uri: string }, options?: { timeout?: number }) {
this._state?.readResourceTimeouts.push(options?.timeout)
return { contents: [{ uri: params.uri, text: "test" }] }
}

async callTool(_args: unknown, _schema: unknown, options?: { signal?: AbortSignal; timeout?: number }) {
this._state?.callToolSignals.push(options?.signal)
this._state?.callToolTimeouts.push(options?.timeout)
Expand Down Expand Up @@ -229,17 +246,20 @@ const { Bus } = await import("../../src/bus/index")
const { Instance } = await import("../../src/project/instance")
const { NotFoundError } = await import("../../src/storage/db")
const { tmpdir } = await import("../fixture/fixture")
const { makeRuntime } = await import("../../src/effect/run-service")
const mcpRuntime = makeRuntime(MCP.Service, MCP.defaultLayer)

// --- Helper ---

function withInstance(config: Record<string, any>, fn: () => Promise<void>) {
function withInstance(config: Record<string, any>, fn: () => Promise<void>, extraConfig: Record<string, any> = {}) {
return async () => {
await using tmp = await tmpdir({
init: async (dir) => {
await Bun.write(
`${dir}/opencode.json`,
JSON.stringify({
$schema: "https://opencode.ai/config.json",
...extraConfig,
mcp: config,
}),
)
Expand Down Expand Up @@ -418,6 +438,28 @@ test(
}),
)

test(
"add records failed status and closes the client when capability probing throws",
withInstance({}, async () => {
lastCreatedClientName = "defective-server"
const serverState = getOrCreateClientState("defective-server")
serverState.capabilitiesShouldThrow = true

const addResult = await MCP.add("defective-server", {
type: "local",
command: ["echo", "test"],
})

const serverStatus = (addResult.status as any)["defective-server"] ?? addResult.status
expect(serverStatus).toEqual({ status: "failed", error: "capability discovery failed" })
expect((await MCP.status())["defective-server"]).toEqual({
status: "failed",
error: "capability discovery failed",
})
expect(serverState.closed).toBe(true)
}),
)

test(
"tool execution forwards abort signals to MCP callTool",
withInstance({}, async () => {
Expand All @@ -441,6 +483,42 @@ test(
}),
)

test(
"prompt and resource requests use per-server timeout before experimental fallback",
withInstance(
{},
async () => {
lastCreatedClientName = "timeout-server"
const timeoutState = getOrCreateClientState("timeout-server")

await MCP.add("timeout-server", {
type: "local",
command: ["echo", "test"],
timeout: 2500,
})
await mcpRuntime.runPromise((mcp) => mcp.getPrompt("timeout-server", "test"))
await mcpRuntime.runPromise((mcp) => mcp.readResource("timeout-server", "test://resource"))

expect(timeoutState.getPromptTimeouts).toEqual([2500])
expect(timeoutState.readResourceTimeouts).toEqual([2500])

lastCreatedClientName = "fallback-server"
const fallbackState = getOrCreateClientState("fallback-server")

await MCP.add("fallback-server", {
type: "local",
command: ["echo", "test"],
})
await mcpRuntime.runPromise((mcp) => mcp.getPrompt("fallback-server", "test"))
await mcpRuntime.runPromise((mcp) => mcp.readResource("fallback-server", "test://resource"))

expect(fallbackState.getPromptTimeouts).toEqual([5000])
expect(fallbackState.readResourceTimeouts).toEqual([5000])
},
{ experimental: { mcp_timeout: 5000 } },
),
)

// ========================================================================
// Test: tool change notifications publish ToolsChanged on the instance bus (#22504)
// The MCP SDK fires the notification handler from a detached transport callback,
Expand Down
Loading