ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-25 03:45:46 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(gateway): reduce mcp loopback schema warning noise (#103171)

Browse Source 
* fix(gateway): reduce mcp loopback schema warning noise

Signed-off-by: Ho Lim <subhoya@gmail.com>

* test(gateway): prove mcp warning behavior over loopback HTTP

Signed-off-by: Ho Lim <subhoya@gmail.com>

* test(gateway): update mcp loopback warning prefix

---------

Signed-off-by: Ho Lim <subhoya@gmail.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Ho Lim
2026-07-21 00:28:22 -07:00
committed by GitHub
parent 2fcbc136c4
commit f3126c34ef
4 changed files with 170 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/gateway/mcp-http.schema.ts
+59 -3
View File
@@ -5,6 +5,8 @@ import { isRecord } from "@openclaw/normalization-core/record-coerce";
import { logWarn } from "../logger.js";
import { resolveGatewayScopedTools } from "./tool-resolution.js";
const MCP_LOOPBACK_LOG_PREFIX = "mcp-loopback";
// MCP loopback schema projection adapts gateway tool definitions into MCP
// tools/list entries. It flattens provider-hostile union schemas into object
// schemas because some MCP clients cannot render anyOf/oneOf controls.
@@ -145,7 +147,7 @@ function flattenUnionSchema(
for (const [key, schema] of Object.entries(props)) {
if (!isPropertySchema(schema)) {
warnSchemaOnce(
`mcp loopback: malformed schema definition for "${toolName}.${key}", ignoring that variant`,
`${MCP_LOOPBACK_LOG_PREFIX}: malformed schema definition for "${toolName}.${key}", ignoring that variant`,
);
continue;
}
@@ -166,10 +168,13 @@ function flattenUnionSchema(
if (incoming === false) {
continue;
}
if (areSchemaValuesEquivalent(existing, incoming)) {
continue;
}
if (!isRecord(existing) || !isRecord(incoming)) {
if (existing !== incoming) {
warnSchemaOnce(
`mcp loopback: conflicting schema definitions for "${toolName}.${key}", keeping the first variant`,
`${MCP_LOOPBACK_LOG_PREFIX}: conflicting schema definitions for "${toolName}.${key}", keeping the first variant`,
);
}
continue;
@@ -185,7 +190,7 @@ function flattenUnionSchema(
continue;
}
warnSchemaOnce(
`mcp loopback: conflicting schema definitions for "${toolName}.${key}", keeping the first variant`,
`${MCP_LOOPBACK_LOG_PREFIX}: conflicting schema definitions for "${toolName}.${key}", keeping the first variant`,
);
}
}
@@ -207,6 +212,57 @@ function isPropertySchema(value: unknown): value is boolean | Record<string, unk
return typeof value === "boolean" || isRecord(value);
}
function rememberSchemaPair(
left: object,
right: object,
seen: WeakMap<object, WeakSet<object>>,
): boolean {
const existing = seen.get(left);
if (existing?.has(right)) {
return true;
}
const next = existing ?? new WeakSet<object>();
next.add(right);
if (!existing) {
seen.set(left, next);
}
return false;
}
function areSchemaValuesEquivalent(
left: unknown,
right: unknown,
seen = new WeakMap<object, WeakSet<object>>(),
): boolean {
if (Object.is(left, right)) {
return true;
}
if (Array.isArray(left) || Array.isArray(right)) {
if (!Array.isArray(left) || !Array.isArray(right) || left.length !== right.length) {
return false;
}
if (rememberSchemaPair(left, right, seen)) {
return true;
}
return left.every((value, index) => areSchemaValuesEquivalent(value, right[index], seen));
}
if (!isRecord(left) || !isRecord(right)) {
return false;
}
if (rememberSchemaPair(left, right, seen)) {
return true;
}
const leftKeys = Object.keys(left).toSorted();
const rightKeys = Object.keys(right).toSorted();
if (leftKeys.length !== rightKeys.length) {
return false;
}
return leftKeys.every(
(key, index) =>
key === rightKeys[index] && areSchemaValuesEquivalent(left[key], right[key], seen),
);
}
// Loopback schemas are rebuilt on every cache miss (per session/owner context and
// after TTL expiry), so raw logWarn would repeat the same field warning endlessly.
// Dedupe on the full message: distinct (tool, field, reason) still each warn once,
src/gateway/mcp-http.test.ts
+93 -6
View File
@@ -863,7 +863,7 @@ describe("buildMcpToolSchema", () => {
},
});
expect(logWarnMock.mock.calls.map(([message]) => message)).toEqual([
'mcp loopback: conflicting schema definitions for "codex_threads_constrained_literals.action", keeping the first variant',
'mcp-loopback: conflicting schema definitions for "codex_threads_constrained_literals.action", keeping the first variant',
]);
});
@@ -930,11 +930,53 @@ describe("buildMcpToolSchema", () => {
}
expect(logWarnMock.mock.calls.map(([message]) => message)).toEqual([
'mcp loopback: conflicting schema definitions for "mcp_message_send_rebuild.action", keeping the first variant',
'mcp loopback: conflicting schema definitions for "mcp_message_send_rebuild.callId", keeping the first variant',
'mcp-loopback: conflicting schema definitions for "mcp_message_send_rebuild.action", keeping the first variant',
'mcp-loopback: conflicting schema definitions for "mcp_message_send_rebuild.callId", keeping the first variant',
]);
});
it("does not warn for structurally identical union property schemas", () => {
const tool = makeMockTool({
name: "lark_doc_read",
parameters: {
anyOf: [
{
type: "object",
properties: {
doc_token: {
type: "string",
description: "Lark document token",
minLength: 1,
},
},
},
{
type: "object",
properties: {
doc_token: {
minLength: 1,
description: "Lark document token",
type: "string",
},
},
},
],
},
});
expect(buildMockMcpToolSchema([tool])[0]?.inputSchema).toMatchObject({
type: "object",
properties: {
doc_token: {
type: "string",
description: "Lark document token",
minLength: 1,
},
},
});
expect(logWarnMock).not.toHaveBeenCalled();
});
it("warns per tool for the same conflicting field name across different tools", () => {
const conflictingUnion = (label: string) => ({
anyOf: [
@@ -956,8 +998,8 @@ describe("buildMcpToolSchema", () => {
buildMockMcpToolSchema([messageTool, calendarTool]);
expect(logWarnMock.mock.calls.map(([message]) => message)).toEqual([
'mcp loopback: conflicting schema definitions for "mcp_message_send_per_tool.action", keeping the first variant',
'mcp loopback: conflicting schema definitions for "mcp_calendar_create_per_tool.action", keeping the first variant',
'mcp-loopback: conflicting schema definitions for "mcp_message_send_per_tool.action", keeping the first variant',
'mcp-loopback: conflicting schema definitions for "mcp_calendar_create_per_tool.action", keeping the first variant',
]);
});
@@ -976,12 +1018,57 @@ describe("buildMcpToolSchema", () => {
buildMockMcpToolSchema([tool]);
expect(logWarnMock.mock.calls.map(([message]) => message)).toEqual([
'mcp loopback: malformed schema definition for "mcp_message_send_malformed.action", ignoring that variant',
'mcp-loopback: malformed schema definition for "mcp_message_send_malformed.action", ignoring that variant',
]);
});
});
describe("mcp loopback server", () => {
it("keeps equal schemas quiet and dedupes genuine conflicts across HTTP cache misses", async () => {
mockScopedTools([
makeMockTool({
name: "lark_doc_read",
parameters: {
anyOf: [
{
type: "object",
properties: {
doc_token: { type: "string", description: "Lark document token" },
action: { type: "string" },
},
},
{
type: "object",
properties: {
doc_token: { description: "Lark document token", type: "string" },
action: { type: "number" },
},
},
],
},
}),
]);
const { runtime } = await startLoopbackServerForTest();
for (let index = 0; index < 3; index += 1) {
const payload = await readOkMcpPayload(
await sendLoopbackToolsList({
token: runtime.ownerToken,
headers: {
...MAIN_SESSION_HEADER,
"x-openclaw-current-message-id": `message-${index}`,
},
}),
);
expectMcpToolNames(payload, ["lark_doc_read"]);
}
expect(resolveGatewayScopedToolsMock).toHaveBeenCalledTimes(3);
expect(logWarnMock.mock.calls.map(([message]) => message)).toEqual([
'mcp-loopback: conflicting schema definitions for "lark_doc_read.action", keeping the first variant',
]);
});
it("rejects reserved harness contexts before tool resolution", async () => {
const { runtime } = await startLoopbackServerForTest();
const response = await sendLoopbackToolsList({
src/gateway/mcp-http.ts
+1 -1
View File
@@ -418,7 +418,7 @@ async function startMcpLoopbackServer(port = 0): Promise<{
res.writeHead(200, { "Content-Type": "application/json" });
res.end(payload);
} catch (error) {
logWarn(`mcp loopback: request handling failed: ${formatErrorMessage(error)}`);
logWarn(`mcp-loopback: request handling failed: ${formatErrorMessage(error)}`);
logMcpLoopbackTraffic("request-failed", {
message: formatErrorMessage(error),
});
src/logging/console-capture.test.ts
+17
View File
@@ -1,6 +1,8 @@
// Console capture tests cover intercepting and restoring console output.
import fs from "node:fs";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import { setVerbose } from "../global-state.js";
import { logWarn } from "../logger.js";
import {
enableConsoleCapture,
resetLogger,
@@ -137,6 +139,17 @@ describe("enableConsoleCapture", () => {
expect(stdoutWrite).toHaveBeenCalledWith('{\n "ok": true\n}\n');
});
it("routes subsystem-prefixed warnings through one file-log sink", () => {
const logPath = tempLogPath();
setLoggerOverride({ level: "info", file: logPath });
enableConsoleCapture();
logWarn("mcp-loopback: conflicting schema definitions");
const content = fs.readFileSync(logPath, "utf-8");
expect(countMatchingLines(content, "conflicting schema definitions")).toBe(1);
});
it("redacts credentials before forwarding console output", () => {
setLoggerOverride({ level: "info", file: tempLogPath() });
const log = vi.fn();
@@ -245,6 +258,10 @@ function tempLogPath() {
return logPathTracker.nextPath();
}
function countMatchingLines(value: string, needle: string): number {
return value.split(/\r?\n/u).filter((line) => line.includes(needle)).length;
}
function eioError() {
const err = new Error("EIO") as NodeJS.ErrnoException;
err.code = "EIO";