From b7f3d01633e540acff24eb02b98bd6018c5e1273 Mon Sep 17 00:00:00 2001 From: Gio Della-Libera Date: Sat, 16 May 2026 22:41:11 -0700 Subject: [PATCH] fix(mcp): inline local refs in bundled tool schemas (#81238) --- CHANGELOG.md | 1 + src/agents/pi-bundle-mcp-materialize.ts | 3 +- .../pi-bundle-mcp-tools.materialize.test.ts | 69 +++++++ src/agents/pi-tools-parameter-schema.ts | 178 +++++++++++++++++- src/agents/pi-tools.schema.test.ts | 117 ++++++++++++ 5 files changed, 363 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07c9bdb08ad..3a70db1384e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -966,6 +966,7 @@ Docs: https://docs.openclaw.ai - Require admin scope for node device token management [AI]. (#81067) Thanks @pgondhi987. - Restrict chat sender allowlist matching [AI]. (#80898) Thanks @pgondhi987. - Update: suppress the false newer-config warning during restart health probing after an update handoff, while keeping future-version mutation guards intact. (#78652) +- Bundled MCP: inline local `$ref` parameter schemas before exposing tools, so Notion-style `oneOf` inputs validate through the bridge. Fixes #78737. - Sessions: redact persisted tool result detail metadata before writing transcripts so diagnostic secrets do not survive tool output redaction. (#80444) Thanks @nimbleenigma. - Codex runtime: allow the official installed `@openclaw/codex` package to use its private task-runtime and MCP projection SDK helpers, fixing `MODULE_NOT_FOUND` during migrated OpenAI/Codex beta runs. - Codex migration: make Enter activate the highlighted checkbox row before continuing, so `Skip for now` and bulk-selection rows work even when planned items start preselected. diff --git a/src/agents/pi-bundle-mcp-materialize.ts b/src/agents/pi-bundle-mcp-materialize.ts index 7a77f0cc207..d3cdeba0bce 100644 --- a/src/agents/pi-bundle-mcp-materialize.ts +++ b/src/agents/pi-bundle-mcp-materialize.ts @@ -11,6 +11,7 @@ import { TOOL_NAME_SEPARATOR, } from "./pi-bundle-mcp-names.js"; import type { BundleMcpToolRuntime, SessionMcpRuntime } from "./pi-bundle-mcp-types.js"; +import { normalizeToolParameterSchema } from "./pi-tools-parameter-schema.js"; import type { AnyAgentTool } from "./tools/common.js"; function toAgentToolResult(params: { @@ -110,7 +111,7 @@ export async function materializeBundleMcpToolsForRun(params: { name: safeToolName, label: tool.title ?? tool.toolName, description: tool.description || tool.fallbackDescription, - parameters: tool.inputSchema, + parameters: normalizeToolParameterSchema(tool.inputSchema), execute: async (_toolCallId: string, input: unknown) => { params.runtime.markUsed(); const result = await params.runtime.callTool(tool.serverName, tool.toolName, input); diff --git a/src/agents/pi-bundle-mcp-tools.materialize.test.ts b/src/agents/pi-bundle-mcp-tools.materialize.test.ts index 8b3496457f6..70f145fb7e9 100644 --- a/src/agents/pi-bundle-mcp-tools.materialize.test.ts +++ b/src/agents/pi-bundle-mcp-tools.materialize.test.ts @@ -1,3 +1,4 @@ +import { validateToolArguments } from "@earendil-works/pi-ai"; import { describe, expect, it } from "vitest"; import { getPluginToolMeta } from "../plugins/tools.js"; import { @@ -169,4 +170,72 @@ describe("createBundleMcpToolRuntime", () => { "multi__zeta", ]); }); + + it("normalizes local $ref schemas from MCP tools before exposing them", async () => { + const runtime = await materializeBundleMcpToolsForRun({ + runtime: makeToolRuntime({ + tools: [ + { + serverName: "notion", + safeServerName: "notion", + toolName: "API-post-page", + description: "Create a page", + inputSchema: { + type: "object", + required: ["parent"], + properties: { + parent: { $ref: "#/$defs/parentRequest" }, + }, + $defs: { + parentRequest: { + oneOf: [ + { + type: "object", + required: ["page_id"], + properties: { page_id: { type: "string" } }, + }, + { + type: "object", + required: ["database_id"], + properties: { database_id: { type: "string" } }, + }, + ], + }, + }, + }, + fallbackDescription: "Create a page", + }, + ], + }), + }); + + expect(runtime.tools[0]?.parameters).toEqual({ + type: "object", + required: ["parent"], + properties: { + parent: { + oneOf: [ + { + type: "object", + required: ["page_id"], + properties: { page_id: { type: "string" } }, + }, + { + type: "object", + required: ["database_id"], + properties: { database_id: { type: "string" } }, + }, + ], + }, + }, + }); + expect( + validateToolArguments(runtime.tools[0], { + type: "toolCall", + id: "call-page", + name: "notion__API-post-page", + arguments: { parent: { page_id: "page-id" } }, + }), + ).toEqual({ parent: { page_id: "page-id" } }); + }); }); diff --git a/src/agents/pi-tools-parameter-schema.ts b/src/agents/pi-tools-parameter-schema.ts index 091903d7722..b584e390ea4 100644 --- a/src/agents/pi-tools-parameter-schema.ts +++ b/src/agents/pi-tools-parameter-schema.ts @@ -309,14 +309,184 @@ function stripEmptyArrayItemsFromArraySchemas(schema: unknown): unknown { return changed ? Object.fromEntries(entries) : schema; } +type SchemaDefs = { + $defs: Map; + definitions: Map; +}; + +function copySchemaMeta(from: Record, to: Record): void { + for (const key of ["title", "description", "default"] as const) { + if (key in from && from[key] !== undefined) { + to[key] = from[key]; + } + } +} + +function extendSchemaDefs( + defs: SchemaDefs | undefined, + schema: Record, +): SchemaDefs | undefined { + const defsEntry = + schema.$defs && typeof schema.$defs === "object" && !Array.isArray(schema.$defs) + ? (schema.$defs as Record) + : undefined; + const legacyDefsEntry = + schema.definitions && + typeof schema.definitions === "object" && + !Array.isArray(schema.definitions) + ? (schema.definitions as Record) + : undefined; + + if (!defsEntry && !legacyDefsEntry) { + return defs; + } + + const next: SchemaDefs = defs + ? { + $defs: new Map(defs.$defs), + definitions: new Map(defs.definitions), + } + : { + $defs: new Map(), + definitions: new Map(), + }; + if (defsEntry) { + for (const [key, value] of Object.entries(defsEntry)) { + next.$defs.set(key, value); + } + } + if (legacyDefsEntry) { + for (const [key, value] of Object.entries(legacyDefsEntry)) { + next.definitions.set(key, value); + } + } + return next; +} + +function decodeJsonPointerSegment(segment: string): string { + return segment.replaceAll("~1", "/").replaceAll("~0", "~"); +} + +function resolveJsonPointerPath(value: unknown, segments: string[]): unknown { + let current = value; + for (const segment of segments) { + if (!current || typeof current !== "object") { + return undefined; + } + const key = decodeJsonPointerSegment(segment); + if (Array.isArray(current)) { + const index = Number(key); + if (!Number.isInteger(index) || index < 0 || index >= current.length) { + return undefined; + } + current = current[index]; + continue; + } + const record = current as Record; + if (!Object.prototype.hasOwnProperty.call(record, key)) { + return undefined; + } + current = record[key]; + } + return current; +} + +function tryResolveLocalRef(ref: string, defs: SchemaDefs | undefined): unknown { + if (!defs) { + return undefined; + } + const match = ref.match(/^#\/(\$defs|definitions)\/([^/]+)(?:\/(.*))?$/); + if (!match) { + return undefined; + } + const namespace = match[1] === "$defs" ? defs.$defs : defs.definitions; + const name = decodeJsonPointerSegment(match[2] ?? ""); + const resolved = name ? namespace.get(name) : undefined; + if (resolved === undefined) { + return undefined; + } + const remainingPath = match[3] ? match[3].split("/") : []; + return resolveJsonPointerPath(resolved, remainingPath); +} + +function inlineLocalSchemaRefsWithDefs( + schema: unknown, + defs: SchemaDefs | undefined, + refStack: Set | undefined, + state: { unresolvedLocalRefs: boolean }, +): unknown { + if (!schema || typeof schema !== "object") { + return schema; + } + if (Array.isArray(schema)) { + return schema.map((entry) => inlineLocalSchemaRefsWithDefs(entry, defs, refStack, state)); + } + + const obj = schema as Record; + const nextDefs = extendSchemaDefs(defs, obj); + const refValue = typeof obj.$ref === "string" ? obj.$ref : undefined; + + if (refValue) { + if (refStack?.has(refValue)) { + return {}; + } + const resolved = tryResolveLocalRef(refValue, nextDefs); + if (resolved === undefined) { + if (refValue.startsWith("#/")) { + state.unresolvedLocalRefs = true; + } + return { ...obj }; + } + const nextRefStack = refStack ? new Set(refStack) : new Set(); + nextRefStack.add(refValue); + const inlined = inlineLocalSchemaRefsWithDefs(resolved, nextDefs, nextRefStack, state); + if (!inlined || typeof inlined !== "object" || Array.isArray(inlined)) { + return inlined; + } + const result: Record = { ...(inlined as Record) }; + copySchemaMeta(obj, result); + return result; + } + + const result: Record = {}; + for (const [key, value] of Object.entries(obj)) { + if (key === "$defs" || key === "definitions") { + continue; + } + result[key] = inlineLocalSchemaRefsWithDefs(value, nextDefs, refStack, state); + } + if (state.unresolvedLocalRefs) { + if ("$defs" in obj) { + result.$defs = obj.$defs; + } + if ("definitions" in obj) { + result.definitions = obj.definitions; + } + } + return result; +} + +export function inlineLocalToolSchemaRefs(schema: unknown): TSchema { + if (!schema || typeof schema !== "object") { + return schema as TSchema; + } + const defs = extendSchemaDefs(undefined, schema as Record); + return inlineLocalSchemaRefsWithDefs(schema, defs, undefined, { + unresolvedLocalRefs: false, + }) as TSchema; +} + export function normalizeToolParameterSchema( schema: unknown, options?: { modelProvider?: string; modelId?: string; modelCompat?: ModelCompatConfig }, ): TSchema { + const inlinedSchema = inlineLocalToolSchemaRefs(schema); const schemaRecord = - schema && typeof schema === "object" ? (schema as Record) : undefined; + inlinedSchema && typeof inlinedSchema === "object" + ? (inlinedSchema as Record) + : undefined; if (!schemaRecord) { - return schema as TSchema; + return inlinedSchema; } // Provider quirks: @@ -378,9 +548,9 @@ export function normalizeToolParameterSchema( if (conditionalKey === "allOf") { // Top-level `allOf` is not safely flattenable with the same heuristics we // use for unions. Keep it explicit rather than silently rewriting it. - return applyProviderCleaning(schema); + return applyProviderCleaning(inlinedSchema); } - return applyProviderCleaning(schema); + return applyProviderCleaning(inlinedSchema); } const variants = schemaRecord[flattenableVariantKey] as unknown[]; const mergedProperties: Record = {}; diff --git a/src/agents/pi-tools.schema.test.ts b/src/agents/pi-tools.schema.test.ts index 4bf59c843fe..f62024d2b47 100644 --- a/src/agents/pi-tools.schema.test.ts +++ b/src/agents/pi-tools.schema.test.ts @@ -136,6 +136,123 @@ describe("normalizeToolParameterSchema", () => { }); }); + it("inlines nested local $ref schemas for provider-neutral tools", () => { + expect( + normalizeToolParameterSchema({ + type: "object", + required: ["parent"], + properties: { + parent: { + $ref: "#/$defs/Parent", + description: "Notion parent", + }, + }, + $defs: { + Parent: { + oneOf: [ + { + type: "object", + required: ["page_id"], + properties: { page_id: { type: "string" } }, + }, + { + type: "object", + required: ["database_id"], + properties: { database_id: { type: "string" } }, + }, + ], + }, + }, + }), + ).toEqual({ + type: "object", + required: ["parent"], + properties: { + parent: { + description: "Notion parent", + oneOf: [ + { + type: "object", + required: ["page_id"], + properties: { page_id: { type: "string" } }, + }, + { + type: "object", + required: ["database_id"], + properties: { database_id: { type: "string" } }, + }, + ], + }, + }, + }); + }); + + it("inlines local $ref schemas that target nested JSON Pointer paths", () => { + expect( + normalizeToolParameterSchema({ + type: "object", + properties: { + pageId: { $ref: "#/$defs/Parent/properties/page_id" }, + legacyDatabaseId: { $ref: "#/definitions/Parent/properties/database_id" }, + }, + $defs: { + Parent: { + type: "object", + properties: { + page_id: { type: "string", description: "Page id" }, + }, + }, + }, + definitions: { + Parent: { + type: "object", + properties: { + database_id: { type: "string", description: "Database id" }, + }, + }, + }, + }), + ).toEqual({ + type: "object", + properties: { + pageId: { type: "string", description: "Page id" }, + legacyDatabaseId: { type: "string", description: "Database id" }, + }, + }); + }); + + it("preserves local definitions when a local $ref cannot be resolved", () => { + expect( + normalizeToolParameterSchema({ + type: "object", + properties: { + missing: { $ref: "#/$defs/Missing/properties/id" }, + }, + $defs: { + Present: { + type: "object", + properties: { + id: { type: "string" }, + }, + }, + }, + }), + ).toEqual({ + type: "object", + properties: { + missing: { $ref: "#/$defs/Missing/properties/id" }, + }, + $defs: { + Present: { + type: "object", + properties: { + id: { type: "string" }, + }, + }, + }, + }); + }); + it("cleans tuple items schemas", () => { const cleaned = cleanToolSchemaForGemini({ type: "object",