mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 21:34:46 +00:00
fix(mcp): inline local refs in bundled tool schemas (#81238)
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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" } });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -309,14 +309,184 @@ function stripEmptyArrayItemsFromArraySchemas(schema: unknown): unknown {
|
||||
return changed ? Object.fromEntries(entries) : schema;
|
||||
}
|
||||
|
||||
type SchemaDefs = {
|
||||
$defs: Map<string, unknown>;
|
||||
definitions: Map<string, unknown>;
|
||||
};
|
||||
|
||||
function copySchemaMeta(from: Record<string, unknown>, to: Record<string, unknown>): 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<string, unknown>,
|
||||
): SchemaDefs | undefined {
|
||||
const defsEntry =
|
||||
schema.$defs && typeof schema.$defs === "object" && !Array.isArray(schema.$defs)
|
||||
? (schema.$defs as Record<string, unknown>)
|
||||
: undefined;
|
||||
const legacyDefsEntry =
|
||||
schema.definitions &&
|
||||
typeof schema.definitions === "object" &&
|
||||
!Array.isArray(schema.definitions)
|
||||
? (schema.definitions as Record<string, unknown>)
|
||||
: undefined;
|
||||
|
||||
if (!defsEntry && !legacyDefsEntry) {
|
||||
return defs;
|
||||
}
|
||||
|
||||
const next: SchemaDefs = defs
|
||||
? {
|
||||
$defs: new Map(defs.$defs),
|
||||
definitions: new Map(defs.definitions),
|
||||
}
|
||||
: {
|
||||
$defs: new Map<string, unknown>(),
|
||||
definitions: new Map<string, unknown>(),
|
||||
};
|
||||
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<string, unknown>;
|
||||
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<string> | 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<string, unknown>;
|
||||
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<string>();
|
||||
nextRefStack.add(refValue);
|
||||
const inlined = inlineLocalSchemaRefsWithDefs(resolved, nextDefs, nextRefStack, state);
|
||||
if (!inlined || typeof inlined !== "object" || Array.isArray(inlined)) {
|
||||
return inlined;
|
||||
}
|
||||
const result: Record<string, unknown> = { ...(inlined as Record<string, unknown>) };
|
||||
copySchemaMeta(obj, result);
|
||||
return result;
|
||||
}
|
||||
|
||||
const result: Record<string, unknown> = {};
|
||||
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<string, unknown>);
|
||||
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<string, unknown>) : undefined;
|
||||
inlinedSchema && typeof inlinedSchema === "object"
|
||||
? (inlinedSchema as Record<string, unknown>)
|
||||
: 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<string, unknown> = {};
|
||||
|
||||
@@ -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",
|
||||
|
||||
Reference in New Issue
Block a user