mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(mcp): doctor probes no longer start every server at once (#109870)
* fix(mcp): bound doctor server checks * fix(mcp): defer doctor concurrency helper loading * fix(mcp): preserve doctor check coverage --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
co-authored by
Peter Steinberger
parent
601ffa2530
commit
c6d4559811
@@ -6,6 +6,7 @@ import { Command } from "commander";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import * as mcpHttpFetch from "../agents/mcp-http-fetch.js";
|
||||
import { withTempHome } from "../config/home-env.test-harness.js";
|
||||
import { createDeferred } from "../shared/deferred.js";
|
||||
import { registerMcpCli } from "./mcp-cli.js";
|
||||
|
||||
const mocks = vi.hoisted(() => {
|
||||
@@ -527,6 +528,72 @@ describe("mcp cli", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("bounds concurrent MCP doctor server checks", async () => {
|
||||
await withTempHome("openclaw-cli-mcp-home-", async () => {
|
||||
const workspaceDir = await createWorkspace();
|
||||
vi.spyOn(process, "cwd").mockReturnValue(workspaceDir);
|
||||
for (let index = 0; index < 6; index += 1) {
|
||||
await runMcpCommand([
|
||||
"mcp",
|
||||
"set",
|
||||
`server-${index}`,
|
||||
JSON.stringify({
|
||||
url: `https://mcp-${index}.example.com`,
|
||||
transport: "streamable-http",
|
||||
auth: "oauth",
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
const checksBlocked = createDeferred();
|
||||
readMcpOAuthCredentialsStatus.mockImplementation(async () => {
|
||||
await checksBlocked.promise;
|
||||
return {
|
||||
hasTokens: false,
|
||||
hasClientInformation: false,
|
||||
hasCodeVerifier: false,
|
||||
hasDiscoveryState: false,
|
||||
hasLastAuthorizationUrl: false,
|
||||
};
|
||||
});
|
||||
|
||||
const doctorPromise = runMcpCommand(["mcp", "doctor", "--json"]);
|
||||
await vi.waitFor(() => {
|
||||
expect(readMcpOAuthCredentialsStatus.mock.calls.length).toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
await new Promise<void>((resolve) => {
|
||||
setImmediate(resolve);
|
||||
});
|
||||
const startedBeforeRelease = readMcpOAuthCredentialsStatus.mock.calls.length;
|
||||
checksBlocked.resolve();
|
||||
await doctorPromise;
|
||||
|
||||
expect(readMcpOAuthCredentialsStatus).toHaveBeenCalledTimes(6);
|
||||
expect(startedBeforeRelease).toBe(4);
|
||||
expect(
|
||||
JSON.parse(lastLogLine()).servers.map((server: { name: string }) => server.name),
|
||||
).toEqual(["server-0", "server-1", "server-2", "server-3", "server-4", "server-5"]);
|
||||
});
|
||||
});
|
||||
|
||||
it("surfaces unexpected MCP doctor check errors", async () => {
|
||||
await withTempHome("openclaw-cli-mcp-home-", async () => {
|
||||
const workspaceDir = await createWorkspace();
|
||||
vi.spyOn(process, "cwd").mockReturnValue(workspaceDir);
|
||||
await runMcpCommand([
|
||||
"mcp",
|
||||
"set",
|
||||
"docs",
|
||||
'{"url":"https://mcp.example.com","transport":"streamable-http","auth":"oauth"}',
|
||||
]);
|
||||
readMcpOAuthCredentialsStatus.mockRejectedValueOnce(new Error("credential store failed"));
|
||||
|
||||
await expect(runMcpCommand(["mcp", "doctor", "--json"])).rejects.toThrow(
|
||||
"credential store failed",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
it("does not fail MCP doctor for disabled-only overrides", async () => {
|
||||
await withTempHome("openclaw-cli-mcp-home-", async () => {
|
||||
const workspaceDir = await createWorkspace();
|
||||
|
||||
+32
-18
@@ -35,6 +35,7 @@ import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import { serveOpenClawChannelMcp } from "../mcp/channel-server.js";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { runTasksWithConcurrency } from "../utils/run-with-concurrency.js";
|
||||
import { formatCliCommand } from "./command-format.js";
|
||||
import { resolveGatewayAuthOptions } from "./gateway-secret-options.js";
|
||||
import { applyParentDefaultHelpAction } from "./program/parent-default-help.js";
|
||||
@@ -183,6 +184,8 @@ type McpDoctorServerResult = {
|
||||
issues: McpDoctorIssue[];
|
||||
};
|
||||
|
||||
const MCP_DOCTOR_CONCURRENCY = 4;
|
||||
|
||||
const SENSITIVE_HEADER_NAMES = new Set([
|
||||
"authorization",
|
||||
"proxy-authorization",
|
||||
@@ -799,24 +802,35 @@ export function registerMcpCli(program: Command) {
|
||||
`No MCP server named "${name}" in ${loaded.path}. Run ${formatCliCommand("openclaw mcp list")} to see configured servers.`,
|
||||
);
|
||||
}
|
||||
const servers = await Promise.all(
|
||||
Object.entries(selected)
|
||||
.toSorted(([a], [b]) => a.localeCompare(b))
|
||||
.map(async ([serverName, server]): Promise<McpDoctorServerResult> => {
|
||||
const issues = await collectMcpDoctorIssues({
|
||||
name: serverName,
|
||||
server,
|
||||
config: loaded.config,
|
||||
path: loaded.path,
|
||||
probe: Boolean(opts.probe),
|
||||
});
|
||||
return {
|
||||
name: serverName,
|
||||
ok: !issues.some((entry) => entry.level === "error"),
|
||||
issues,
|
||||
};
|
||||
}),
|
||||
);
|
||||
const tasks = Object.entries(selected)
|
||||
.toSorted(([a], [b]) => a.localeCompare(b))
|
||||
.map(([serverName, server]) => async (): Promise<McpDoctorServerResult> => {
|
||||
const issues = await collectMcpDoctorIssues({
|
||||
name: serverName,
|
||||
server,
|
||||
config: loaded.config,
|
||||
path: loaded.path,
|
||||
probe: Boolean(opts.probe),
|
||||
});
|
||||
return {
|
||||
name: serverName,
|
||||
ok: !issues.some((entry) => entry.level === "error"),
|
||||
issues,
|
||||
};
|
||||
});
|
||||
// A probe can start one process or connection per server. Keep large
|
||||
// registries from fanning out every transport at once.
|
||||
const {
|
||||
results: servers,
|
||||
firstError,
|
||||
hasError,
|
||||
} = await runTasksWithConcurrency({
|
||||
tasks,
|
||||
limit: MCP_DOCTOR_CONCURRENCY,
|
||||
});
|
||||
if (hasError) {
|
||||
throw firstError;
|
||||
}
|
||||
const ok = servers.every((server) => server.ok);
|
||||
if (opts.json) {
|
||||
printJson({ path: loaded.path, ok, servers });
|
||||
|
||||
Reference in New Issue
Block a user