fix(doctor): report failed MCP tool schema loading

This commit is contained in:
Vincent Koc
2026-05-29 05:37:00 +02:00
parent 58149e41dc
commit c037ab5c74
8 changed files with 408 additions and 4 deletions

View File

@@ -165,6 +165,9 @@ export async function materializeBundleMcpToolsForRun(params: {
return {
tools,
...(catalog.diagnostics && catalog.diagnostics.length > 0
? { diagnostics: catalog.diagnostics }
: {}),
dispose: async () => {
if (disposed) {
return;

View File

@@ -35,6 +35,7 @@ async function writeListToolsMcpServer(params: {
logPath: string;
delayMs?: number;
hang?: boolean;
inputSchema?: unknown;
}): Promise<void> {
await writeExecutable(
params.filePath,
@@ -44,6 +45,7 @@ import fs from "node:fs/promises";
const logPath = ${JSON.stringify(params.logPath)};
const delayMs = ${params.delayMs ?? 0};
const hang = ${params.hang === true};
const inputSchema = ${JSON.stringify(params.inputSchema ?? { type: "object", properties: {} })};
let buffer = "";
let pendingTimer;
@@ -90,7 +92,7 @@ function handle(message) {
{
name: "slow_tool",
description: "Returned after a slow catalog response.",
inputSchema: { type: "object", properties: {} },
inputSchema,
},
],
},
@@ -656,6 +658,46 @@ describe("session MCP runtime", () => {
}
});
it("records diagnostics when tools/list returns an invalid tool schema", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "bundle-mcp-invalid-schema-"));
const serverPath = path.join(tempDir, "invalid-schema.mjs");
const logPath = path.join(tempDir, "server.log");
await writeListToolsMcpServer({
filePath: serverPath,
logPath,
inputSchema: { type: "array", items: { type: "number" } },
});
const runtime = await getOrCreateSessionMcpRuntime({
sessionId: "session-invalid-schema",
sessionKey: "agent:test:session-invalid-schema",
workspaceDir: "/workspace",
cfg: {
mcp: {
servers: {
dofbot: {
command: process.execPath,
args: [serverPath],
},
},
},
},
});
try {
const catalog = await runtime.getCatalog();
expect(catalog.servers).toEqual({});
expect(catalog.tools).toEqual([]);
expect(catalog.diagnostics?.[0]?.serverName).toBe("dofbot");
expect(catalog.diagnostics?.[0]?.message).toContain("Invalid input: expected");
expect(catalog.diagnostics?.[0]?.message).toContain("object");
} finally {
await runtime.dispose();
await fs.rm(tempDir, { recursive: true, force: true });
}
});
it("reuses repeated materialization and recreates after explicit disposal", async () => {
const created: SessionMcpRuntime[] = [];
const disposed: string[] = [];

View File

@@ -24,6 +24,7 @@ import type {
McpCatalogTool,
McpServerCatalog,
McpToolCatalog,
McpToolCatalogDiagnostic,
SessionMcpRuntime,
SessionMcpRuntimeManager,
} from "./agent-bundle-mcp-types.js";
@@ -325,6 +326,7 @@ export function createSessionMcpRuntime(params: {
const servers: Record<string, McpServerCatalog> = {};
const tools: McpCatalogTool[] = [];
const diagnostics: McpToolCatalogDiagnostic[] = [];
const usedServerNames = new Set<string>();
try {
@@ -386,11 +388,18 @@ export function createSessionMcpRuntime(params: {
});
}
} catch (error) {
const message = redactErrorUrls(error);
if (!disposed) {
logWarn(
`bundle-mcp: failed to start server "${serverName}" (${resolved.description}): ${redactErrorUrls(error)}`,
`bundle-mcp: failed to start server "${serverName}" (${resolved.description}): ${message}`,
);
}
diagnostics.push({
serverName,
safeServerName,
launchSummary: resolved.description,
message,
});
await disposeSession(session);
sessions.delete(serverName);
failIfDisposed();
@@ -403,6 +412,7 @@ export function createSessionMcpRuntime(params: {
generatedAt: Date.now(),
servers,
tools,
...(diagnostics.length > 0 ? { diagnostics } : {}),
};
} catch (error) {
await Promise.allSettled(

View File

@@ -7,6 +7,7 @@ import {
materializeBundleMcpToolsForRun,
} from "./agent-bundle-mcp-materialize.js";
import type { McpCatalogTool } from "./agent-bundle-mcp-types.js";
import type { McpToolCatalogDiagnostic } from "./agent-bundle-mcp-types.js";
import type { SessionMcpRuntime } from "./agent-bundle-mcp-types.js";
function expectTextContentBlock(block: unknown, text: string) {
@@ -21,6 +22,7 @@ function makeToolRuntime(
serverName?: string;
result?: CallToolResult;
resultText?: string;
diagnostics?: readonly McpToolCatalogDiagnostic[];
} = {},
): SessionMcpRuntime {
const serverName = params.serverName ?? "bundleProbe";
@@ -52,6 +54,7 @@ function makeToolRuntime(
},
},
tools,
...(params.diagnostics ? { diagnostics: params.diagnostics } : {}),
}),
peekCatalog: () => ({
version: 1,
@@ -64,6 +67,7 @@ function makeToolRuntime(
},
},
tools,
...(params.diagnostics ? { diagnostics: params.diagnostics } : {}),
}),
callTool: async () =>
params.result ?? {
@@ -137,6 +141,24 @@ describe("createBundleMcpToolRuntime", () => {
expect(runtime.tools.map((tool) => tool.name)).toEqual(["bundleProbe__bundle_probe-2"]);
});
it("preserves catalog diagnostics when MCP servers fail tool listing", async () => {
const diagnostics = [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type expected "object"',
},
];
const runtime = await materializeBundleMcpToolsForRun({
runtime: makeToolRuntime({ tools: [], diagnostics }),
});
expect(runtime.tools).toEqual([]);
expect(runtime.diagnostics).toEqual(diagnostics);
});
it("materializes configured MCP tools through the session runtime boundary", async () => {
const created: Parameters<
NonNullable<Parameters<typeof createBundleMcpToolRuntime>[0]["createRuntime"]>

View File

@@ -3,6 +3,7 @@ export type {
McpCatalogTool,
McpServerCatalog,
McpToolCatalog,
McpToolCatalogDiagnostic,
SessionMcpRuntime,
SessionMcpRuntimeManager,
} from "./agent-bundle-mcp-types.js";

View File

@@ -5,6 +5,7 @@ import type { AnyAgentTool } from "./tools/common.js";
export type BundleMcpToolRuntime = {
tools: AnyAgentTool[];
diagnostics?: readonly McpToolCatalogDiagnostic[];
dispose: () => Promise<void>;
};
@@ -29,6 +30,14 @@ export type McpToolCatalog = {
generatedAt: number;
servers: Record<string, McpServerCatalog>;
tools: McpCatalogTool[];
diagnostics?: readonly McpToolCatalogDiagnostic[];
};
export type McpToolCatalogDiagnostic = {
serverName: string;
safeServerName: string;
launchSummary: string;
message: string;
};
export type SessionMcpRuntime = {

View File

@@ -101,6 +101,134 @@ describe("doctor runtime tool schema checks", () => {
expect(mocks.disposeBundleRuntime).toHaveBeenCalledTimes(1);
});
it("reports bundle MCP runtime diagnostics when tool listing fails schema validation", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
mcp: {
servers: {
dofbot: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toContainEqual({
checkId: "core/doctor/runtime-tool-schemas",
severity: "error",
message:
'Configured MCP server "dofbot" could not expose runtime tools for schema validation.',
path: "mcp.servers.dofbot",
requirement: 'tools[0].inputSchema.type: Invalid input: expected "object"',
fixHint:
"Fix or disable the offending MCP server, then rerun doctor before relying on assistant tool startup.",
});
expect(mocks.disposeBundleRuntime).toHaveBeenCalledTimes(1);
});
it("reports bundle MCP runtime diagnostics for exact MCP tool allowlists", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
tools: { allow: ["dofbot__healthy"] },
mcp: {
servers: {
dofbot: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toContainEqual(
expect.objectContaining({
checkId: "core/doctor/runtime-tool-schemas",
path: "mcp.servers.dofbot",
}),
);
});
it("reports exact MCP allowlists when the safe server name contains the separator", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "my__server",
safeServerName: "my__server",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
tools: { allow: ["my__server__healthy"] },
mcp: {
servers: {
my__server: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toContainEqual(
expect.objectContaining({
checkId: "core/doctor/runtime-tool-schemas",
path: "mcp.servers.my__server",
}),
);
});
it("reports bundle MCP runtime diagnostics for glob MCP tool allowlists", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
tools: { allow: ["*__healthy"] },
mcp: {
servers: {
dofbot: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toContainEqual(
expect.objectContaining({
checkId: "core/doctor/runtime-tool-schemas",
path: "mcp.servers.dofbot",
}),
);
});
it("reports unsupported schemas exposed only to a non-default configured agent", async () => {
mocks.createOpenClawCodingTools.mockImplementation((options) =>
options?.agentId === "worker"
@@ -245,4 +373,56 @@ describe("doctor runtime tool schema checks", () => {
}),
).resolves.toEqual([]);
});
it("does not report bundle MCP diagnostics filtered out by the final runtime tool policy", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
tools: { deny: ["bundle-mcp"] },
mcp: {
servers: {
dofbot: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toEqual([]);
});
it("does not report bundle MCP diagnostics filtered out by server-level deny policy", async () => {
mocks.createBundleMcpToolRuntime.mockResolvedValueOnce({
tools: [],
diagnostics: [
{
serverName: "dofbot",
safeServerName: "dofbot",
launchSummary: "node dofbot-mcp.mjs",
message: 'tools[0].inputSchema.type: Invalid input: expected "object"',
},
],
dispose: mocks.disposeBundleRuntime,
});
await expect(
collectRuntimeToolSchemaFindings({
tools: { deny: ["dofbot__*"] },
mcp: {
servers: {
dofbot: { command: "node", args: ["dofbot-mcp.mjs"] },
},
},
}),
).resolves.toEqual([]);
});
});

View File

@@ -1,4 +1,8 @@
import { createBundleMcpToolRuntime } from "../agents/agent-bundle-mcp-tools.js";
import { TOOL_NAME_SEPARATOR } from "../agents/agent-bundle-mcp-names.js";
import {
type McpToolCatalogDiagnostic,
createBundleMcpToolRuntime,
} from "../agents/agent-bundle-mcp-tools.js";
import {
listAgentEntries,
listAgentIds,
@@ -6,6 +10,7 @@ import {
resolveDefaultAgentId,
} from "../agents/agent-scope.js";
import { createOpenClawCodingTools } from "../agents/agent-tools.js";
import { resolveEffectiveToolPolicy } from "../agents/agent-tools.policy.js";
import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../agents/defaults.js";
import { applyFinalEffectiveToolPolicy } from "../agents/embedded-agent-runner/effective-tool-policy.js";
import { shouldCreateBundleMcpRuntimeForAttempt } from "../agents/embedded-agent-runner/run/attempt-tool-construction-plan.js";
@@ -18,6 +23,7 @@ import { resolveDefaultModelForAgent } from "../agents/model-selection.js";
import { supportsModelTools } from "../agents/model-tool-support.js";
import { normalizeAgentRuntimeTools } from "../agents/runtime-plan/tools.js";
import { buildWorkspaceSkillStatus, type SkillStatusEntry } from "../agents/skills-status.js";
import { collectExplicitAllowlist, normalizeToolName } from "../agents/tool-policy.js";
import {
inspectRuntimeToolInputSchemas,
type RuntimeToolSchemaDiagnostic,
@@ -27,7 +33,7 @@ import { collectUnavailableAgentSkills } from "../commands/doctor-skills-core.js
import type { OpenClawConfig } from "../config/types.openclaw.js";
import { formatErrorMessage } from "../infra/errors.js";
import type { ProviderRuntimeModel } from "../plugins/provider-runtime-model.types.js";
import { getPluginToolMeta } from "../plugins/tools.js";
import { getPluginToolMeta, setPluginToolMeta } from "../plugins/tools.js";
import { normalizeAgentId } from "../routing/session-key.js";
import type { HealthFinding } from "./health-checks.js";
@@ -159,6 +165,128 @@ function bundleMcpRuntimeLoadFailureFinding(error: unknown): HealthFinding {
};
}
function bundleMcpRuntimeDiagnosticFinding(diagnostic: McpToolCatalogDiagnostic): HealthFinding {
return {
checkId: "core/doctor/runtime-tool-schemas",
severity: "error",
message: `Configured MCP server "${diagnostic.serverName}" could not expose runtime tools for schema validation.`,
path: `mcp.servers.${diagnostic.serverName}`,
requirement: diagnostic.message,
fixHint:
"Fix or disable the offending MCP server, then rerun doctor before relying on assistant tool startup.",
};
}
function makeBundleMcpDiagnosticSentinel(name: string): AnyAgentTool {
const sentinel: AnyAgentTool = {
name,
label: "Bundle MCP diagnostic",
description: "Internal doctor sentinel for bundle MCP schema diagnostics.",
parameters: { type: "object", properties: {} },
execute: async () => ({ content: [], details: {} }),
} as AnyAgentTool;
setPluginToolMeta(sentinel, { pluginId: "bundle-mcp", optional: false });
return sentinel;
}
function synthesizeBundleMcpAllowlistSentinelName(params: {
safeServerName: string;
allowlistEntry: string;
}): string | undefined {
const normalized = normalizeToolName(params.allowlistEntry);
const serverPrefix = normalizeToolName(`${params.safeServerName}${TOOL_NAME_SEPARATOR}`);
if (normalized.startsWith(serverPrefix)) {
return normalized;
}
const separatorIndex = normalized.lastIndexOf(TOOL_NAME_SEPARATOR);
if (separatorIndex < 0) {
return undefined;
}
const toolPattern = normalized.slice(separatorIndex + TOOL_NAME_SEPARATOR.length);
if (!toolPattern) {
return undefined;
}
const concreteToolName = toolPattern.replace(/\*/g, "diagnostic").replace(/\?/g, "x");
return `${params.safeServerName}${TOOL_NAME_SEPARATOR}${concreteToolName}`;
}
function collectBundleMcpDiagnosticSentinels(params: {
cfg: OpenClawConfig;
agentId: string;
modelRef: { provider: string; model: string };
diagnostic: McpToolCatalogDiagnostic;
}): AnyAgentTool[] {
const sentinels = [
makeBundleMcpDiagnosticSentinel(
`${params.diagnostic.safeServerName}${TOOL_NAME_SEPARATOR}runtime_schema`,
),
];
const effectivePolicy = resolveEffectiveToolPolicy({
config: params.cfg,
agentId: params.agentId,
modelProvider: params.modelRef.provider,
modelId: params.modelRef.model,
});
const explicitAllowlist = collectExplicitAllowlist([
effectivePolicy.globalPolicy,
effectivePolicy.globalProviderPolicy,
effectivePolicy.agentPolicy,
effectivePolicy.agentProviderPolicy,
effectivePolicy.profileAlsoAllow ? { allow: effectivePolicy.profileAlsoAllow } : undefined,
effectivePolicy.providerProfileAlsoAllow
? { allow: effectivePolicy.providerProfileAlsoAllow }
: undefined,
]);
if (explicitAllowlist.length === 0) {
return sentinels;
}
for (const entry of explicitAllowlist) {
const sentinelName = synthesizeBundleMcpAllowlistSentinelName({
safeServerName: params.diagnostic.safeServerName,
allowlistEntry: entry,
});
if (sentinelName) {
sentinels.push(makeBundleMcpDiagnosticSentinel(sentinelName));
}
}
return sentinels;
}
function shouldReportBundleMcpRuntimeDiagnostic(params: {
cfg: OpenClawConfig;
agentId: string;
modelRef: { provider: string; model: string };
diagnostic: McpToolCatalogDiagnostic;
}): boolean {
return (
applyFinalEffectiveToolPolicy({
bundledTools: collectBundleMcpDiagnosticSentinels(params),
config: params.cfg,
agentId: params.agentId,
modelProvider: params.modelRef.provider,
modelId: params.modelRef.model,
warn: () => {},
}).length > 0
);
}
function filterPolicyActiveBundleMcpDiagnostics(params: {
diagnostics: readonly McpToolCatalogDiagnostic[];
cfg: OpenClawConfig;
agentId: string;
modelRef: { provider: string; model: string };
}): readonly McpToolCatalogDiagnostic[] {
return params.diagnostics.filter((diagnostic) =>
shouldReportBundleMcpRuntimeDiagnostic({
cfg: params.cfg,
agentId: params.agentId,
modelRef: params.modelRef,
diagnostic,
}),
);
}
function isAcpRuntimeAgent(cfg: OpenClawConfig, agentId: string): boolean {
const entry = listAgentEntries(cfg).find(
(candidate) => normalizeAgentId(candidate.id) === agentId,
@@ -253,6 +381,15 @@ export async function collectRuntimeToolSchemaFindings(
}
const bundleRuntime = bundleRuntimeByWorkspace.get(workspaceDir);
if (bundleRuntime) {
if (bundleRuntime.diagnostics && bundleRuntime.diagnostics.length > 0) {
const policyActiveDiagnostics = filterPolicyActiveBundleMcpDiagnostics({
diagnostics: bundleRuntime.diagnostics,
cfg,
agentId,
modelRef,
});
findings.push(...policyActiveDiagnostics.map(bundleMcpRuntimeDiagnosticFinding));
}
findings.push(
...collectBundleMcpRuntimeToolSchemaFindings({
bundleRuntime,