diff --git a/src/agents/agent-bundle-mcp-materialize.ts b/src/agents/agent-bundle-mcp-materialize.ts index c549af48cd29..e2df35cb22f0 100644 --- a/src/agents/agent-bundle-mcp-materialize.ts +++ b/src/agents/agent-bundle-mcp-materialize.ts @@ -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; diff --git a/src/agents/agent-bundle-mcp-runtime.test.ts b/src/agents/agent-bundle-mcp-runtime.test.ts index bd648121679b..b8bb4de51c50 100644 --- a/src/agents/agent-bundle-mcp-runtime.test.ts +++ b/src/agents/agent-bundle-mcp-runtime.test.ts @@ -35,6 +35,7 @@ async function writeListToolsMcpServer(params: { logPath: string; delayMs?: number; hang?: boolean; + inputSchema?: unknown; }): Promise { 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[] = []; diff --git a/src/agents/agent-bundle-mcp-runtime.ts b/src/agents/agent-bundle-mcp-runtime.ts index b49e8c5362af..872584208f0f 100644 --- a/src/agents/agent-bundle-mcp-runtime.ts +++ b/src/agents/agent-bundle-mcp-runtime.ts @@ -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 = {}; const tools: McpCatalogTool[] = []; + const diagnostics: McpToolCatalogDiagnostic[] = []; const usedServerNames = new Set(); 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( diff --git a/src/agents/agent-bundle-mcp-tools.materialize.test.ts b/src/agents/agent-bundle-mcp-tools.materialize.test.ts index cdfa1a88dccd..1a4b28fbd42b 100644 --- a/src/agents/agent-bundle-mcp-tools.materialize.test.ts +++ b/src/agents/agent-bundle-mcp-tools.materialize.test.ts @@ -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[0]["createRuntime"]> diff --git a/src/agents/agent-bundle-mcp-tools.ts b/src/agents/agent-bundle-mcp-tools.ts index 0f8cabae63aa..889ea40cae70 100644 --- a/src/agents/agent-bundle-mcp-tools.ts +++ b/src/agents/agent-bundle-mcp-tools.ts @@ -3,6 +3,7 @@ export type { McpCatalogTool, McpServerCatalog, McpToolCatalog, + McpToolCatalogDiagnostic, SessionMcpRuntime, SessionMcpRuntimeManager, } from "./agent-bundle-mcp-types.js"; diff --git a/src/agents/agent-bundle-mcp-types.ts b/src/agents/agent-bundle-mcp-types.ts index 4e389365bf9a..fe3162211f21 100644 --- a/src/agents/agent-bundle-mcp-types.ts +++ b/src/agents/agent-bundle-mcp-types.ts @@ -5,6 +5,7 @@ import type { AnyAgentTool } from "./tools/common.js"; export type BundleMcpToolRuntime = { tools: AnyAgentTool[]; + diagnostics?: readonly McpToolCatalogDiagnostic[]; dispose: () => Promise; }; @@ -29,6 +30,14 @@ export type McpToolCatalog = { generatedAt: number; servers: Record; tools: McpCatalogTool[]; + diagnostics?: readonly McpToolCatalogDiagnostic[]; +}; + +export type McpToolCatalogDiagnostic = { + serverName: string; + safeServerName: string; + launchSummary: string; + message: string; }; export type SessionMcpRuntime = { diff --git a/src/flows/doctor-core-checks.runtime.test.ts b/src/flows/doctor-core-checks.runtime.test.ts index 2f08411a792f..842cd251b523 100644 --- a/src/flows/doctor-core-checks.runtime.test.ts +++ b/src/flows/doctor-core-checks.runtime.test.ts @@ -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([]); + }); }); diff --git a/src/flows/doctor-core-checks.runtime.ts b/src/flows/doctor-core-checks.runtime.ts index 02dcaba71925..e3cc6a9f37c9 100644 --- a/src/flows/doctor-core-checks.runtime.ts +++ b/src/flows/doctor-core-checks.runtime.ts @@ -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,