mirror of
https://github.com/cline/cline.git
synced 2026-08-29 03:52:41 +08:00
fix(mcp): give unconfigured stdio servers a 30s initialize budget (#13067)
The stdio MCP client gave servers without a configured `timeout` only 1.5 seconds to answer initialize before killing the process, so slow-starting servers (e.g. Oracle SQLcl's JVM-based `sql -mcp`) could never load and were silently skipped at session start. Raise the default connect budget to 30s, in line with the startup budget other MCP clients allow. A configured `timeout` still overrides it in either direction, dead commands still fail fast through the spawn error/exit path, and the newline -> Content-Length framing fallback is unchanged. Fixes #13035 Co-authored-by: Saoud Rizwan <saoudrizwan@users.noreply.github.com>
This commit is contained in:
@@ -2,13 +2,13 @@ import { describe, expect, it } from "vitest";
|
||||
import { getMcpDescription } from "./interactive-config";
|
||||
|
||||
describe("getMcpDescription", () => {
|
||||
it("discloses the fast initialize probe for unconfigured stdio servers", () => {
|
||||
it("discloses the default initialize timeout for unconfigured stdio servers", () => {
|
||||
expect(
|
||||
getMcpDescription({
|
||||
name: "local",
|
||||
transport: { type: "stdio", command: "node" },
|
||||
}),
|
||||
).toBe("stdio, local, request timeout 60s, initialize probe 1.5s");
|
||||
).toBe("stdio, local, request timeout 60s, initialize timeout 30s");
|
||||
});
|
||||
|
||||
it("shows one configured timeout when it also applies to initialize", () => {
|
||||
@@ -40,6 +40,6 @@ describe("getMcpDescription", () => {
|
||||
transport: { type: "stdio", command: "node" },
|
||||
timeoutSeconds: Number.NaN,
|
||||
}),
|
||||
).toBe("stdio, local, request timeout 60s, initialize probe 1.5s");
|
||||
).toBe("stdio, local, request timeout 60s, initialize timeout 30s");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "node:path";
|
||||
import {
|
||||
type BuiltinToolAvailabilityContext,
|
||||
DEFAULT_MCP_CONNECT_TIMEOUT_MS,
|
||||
discoverPluginModulePaths,
|
||||
getPluginDisplayName,
|
||||
hasMcpSettingsFile,
|
||||
@@ -183,7 +184,7 @@ export function getMcpDescription(registration: McpServerRegistration): string {
|
||||
const timeoutDescription =
|
||||
registration.transport.type === "stdio" &&
|
||||
!isMcpTimeoutConfigured(registration.timeoutSeconds)
|
||||
? `request timeout ${timeoutSeconds}s, initialize probe 1.5s`
|
||||
? `request timeout ${timeoutSeconds}s, initialize timeout ${DEFAULT_MCP_CONNECT_TIMEOUT_MS / 1000}s`
|
||||
: `timeout ${timeoutSeconds}s`;
|
||||
return `${registration.transport.type}, ${getMcpAuthLabel(registration)}, ${timeoutDescription}`;
|
||||
}
|
||||
|
||||
@@ -256,23 +256,24 @@ describe("mcp client request timeout", () => {
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it("keeps the fast probe default when no timeout is configured", async () => {
|
||||
it("connects a slow-starting server without a configured timeout", async () => {
|
||||
const factory = createDefaultMcpServerClientFactory();
|
||||
// 3s of startup work exceeds the 1.5s default probe, so connect must
|
||||
// fail quickly instead of stalling startup.
|
||||
// Regression test for https://github.com/cline/cline/issues/13035:
|
||||
// JVM-based servers (e.g. Oracle SQLcl's `sql -mcp`) need several
|
||||
// seconds to start. The old 1.5s initialize probe killed them before
|
||||
// they could answer, so they never loaded without a `timeout` field.
|
||||
const client = await factory(
|
||||
fakeServerRegistration({ delayMs: 0, initDelayMs: 3_000 }),
|
||||
);
|
||||
const startedAt = Date.now();
|
||||
try {
|
||||
await expect(client.connect()).rejects.toThrow(/timed out/);
|
||||
expect(Date.now() - startedAt).toBeLessThan(8_000);
|
||||
await client.connect();
|
||||
expect(await client.listTools()).toEqual([]);
|
||||
} finally {
|
||||
await client.disconnect();
|
||||
}
|
||||
}, 30_000);
|
||||
|
||||
it("keeps the fast probe when a malformed settings timeout is ignored", async () => {
|
||||
it("uses the default connect budget when a malformed settings timeout is ignored", async () => {
|
||||
const filePath = join(tempRoot, `malformed-timeout-${Date.now()}.json`);
|
||||
writeFileSync(
|
||||
filePath,
|
||||
@@ -292,10 +293,9 @@ describe("mcp client request timeout", () => {
|
||||
const [registration] = resolveMcpServerRegistrations({ filePath });
|
||||
expect(registration.timeoutSeconds).toBeUndefined();
|
||||
const client = await createDefaultMcpServerClientFactory()(registration);
|
||||
const startedAt = Date.now();
|
||||
try {
|
||||
await expect(client.connect()).rejects.toThrow(/timed out/);
|
||||
expect(Date.now() - startedAt).toBeLessThan(8_000);
|
||||
await client.connect();
|
||||
expect(await client.listTools()).toEqual([]);
|
||||
} finally {
|
||||
await client.disconnect();
|
||||
}
|
||||
|
||||
@@ -43,10 +43,13 @@ type JsonRpcMessage = {
|
||||
};
|
||||
|
||||
const MCP_PROTOCOL_VERSION = "2024-11-05";
|
||||
// Initialize budget when no timeout is configured. A configured `timeout`
|
||||
// raises it, which lets slow-starting servers (e.g. uvx downloading on first
|
||||
// run) get through initialize.
|
||||
const MCP_CONNECT_PROBE_TIMEOUT_MS = 1_500;
|
||||
// Initialize budget when no timeout is configured. Stdio servers routinely
|
||||
// need several seconds to become ready (JVM-based servers like Oracle SQLcl,
|
||||
// uvx downloading a package on first run), so the default matches the ~30s
|
||||
// startup budget other MCP clients allow. A configured `timeout` overrides
|
||||
// it in either direction. Dead commands still fail fast through the spawn
|
||||
// error/exit path; only an alive-but-silent server waits out this budget.
|
||||
export const DEFAULT_MCP_CONNECT_TIMEOUT_MS = 30_000;
|
||||
const DEFAULT_HTTP_MCP_REDIRECT_URL =
|
||||
"http://127.0.0.1:1456/mcp/oauth/callback";
|
||||
|
||||
@@ -178,14 +181,14 @@ class StdioMcpClient implements McpServerClient {
|
||||
this.requestTimeoutMs = resolveMcpRequestTimeoutMs(
|
||||
registration.timeoutSeconds,
|
||||
);
|
||||
// Keep the fast probe default unless the user opted into patience:
|
||||
// an unconfigured server must not stall startup longer than it did
|
||||
// before per-server timeouts existed.
|
||||
// Initialize gets its own default budget so slow-starting servers
|
||||
// connect out of the box; an explicit `timeout` overrides it in
|
||||
// either direction.
|
||||
this.connectAttemptTimeoutMs = isMcpTimeoutConfigured(
|
||||
registration.timeoutSeconds,
|
||||
)
|
||||
? this.requestTimeoutMs
|
||||
: MCP_CONNECT_PROBE_TIMEOUT_MS;
|
||||
: DEFAULT_MCP_CONNECT_TIMEOUT_MS;
|
||||
}
|
||||
|
||||
async connect(): Promise<void> {
|
||||
|
||||
@@ -31,7 +31,7 @@ import type {
|
||||
const stringRecordSchema = z.record(z.string(), z.string());
|
||||
const metadataSchema = z.record(z.string(), z.unknown());
|
||||
|
||||
// Preserve omission and malformed values for the fast stdio initialize probe.
|
||||
// Preserve omission and malformed values for the stdio initialize budget.
|
||||
// Finite numbers clamp through the shared resolver without rejecting otherwise
|
||||
// valid servers in the settings file. Ordinary requests resolve undefined to
|
||||
// the shared default later, while initialize can still distinguish whether the
|
||||
|
||||
@@ -5,6 +5,7 @@ export type {
|
||||
} from "./client";
|
||||
export {
|
||||
createDefaultMcpServerClientFactory,
|
||||
DEFAULT_MCP_CONNECT_TIMEOUT_MS,
|
||||
probeMcpServerConnection,
|
||||
} from "./client";
|
||||
export type {
|
||||
|
||||
@@ -69,7 +69,7 @@ export class InMemoryMcpManager implements McpManager {
|
||||
JSON.stringify(registration.transport);
|
||||
// A client snapshots the timeout at construction. Preserve an
|
||||
// unconfigured or malformed value as distinct from an explicit default
|
||||
// because stdio initialize uses the fast compatibility probe only when
|
||||
// because stdio initialize uses its default connect budget only when
|
||||
// the timeout is not explicitly configured.
|
||||
const didTimeoutChange =
|
||||
isMcpTimeoutConfigured(existing.registration.timeoutSeconds) !==
|
||||
|
||||
@@ -84,7 +84,7 @@ export interface McpServerRegistration {
|
||||
/**
|
||||
* Per-server request timeout in seconds, from the `timeout` field in
|
||||
* cline_mcp_settings.json. Undefined means the shared default for ordinary
|
||||
* requests; the stdio client preserves its fast compatibility probe for
|
||||
* requests; the stdio client uses its default connect budget for
|
||||
* initialize until a finite timeout is explicitly configured. Registrations are
|
||||
* resolved when the runtime is built, so changes take effect on the next
|
||||
* session.
|
||||
|
||||
@@ -292,6 +292,7 @@ export {
|
||||
createDisabledMcpToolPolicies,
|
||||
createDisabledMcpToolPolicy,
|
||||
createMcpTools,
|
||||
DEFAULT_MCP_CONNECT_TIMEOUT_MS,
|
||||
type DefaultMcpServerClientFactoryOptions,
|
||||
getMcpServerOAuthState,
|
||||
getMcpServerOAuthStatus,
|
||||
|
||||
@@ -603,6 +603,10 @@ process.stdin.on("data", (chunk) => {
|
||||
broken: {
|
||||
command: process.execPath,
|
||||
args: [serverPath],
|
||||
// Keep the test fast: the Content-Length fallback
|
||||
// attempt otherwise waits out the default 30s
|
||||
// connect budget against this silent server.
|
||||
timeout: 1,
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user