Skip to content

Commit d7d037b

Browse files
vincentkocvincentkoc
committed
fix(codex): quarantine unsupported dynamic tool schemas
1 parent d0cb7ba commit d7d037b

9 files changed

Lines changed: 581 additions & 22 deletions

‎extensions/codex/src/app-server/dynamic-tools.test.ts‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { AgentToolResult } from "@earendil-works/pi-agent-core";
22
import type { AnyAgentTool } from "openclaw/plugin-sdk/agent-harness";
33
import {
44
HEARTBEAT_RESPONSE_TOOL_NAME,
5+
embeddedAgentLog,
56
wrapToolWithBeforeToolCallHook,
67
} from "openclaw/plugin-sdk/agent-harness-runtime";
78
import {
@@ -257,6 +258,90 @@ describe("createCodexDynamicToolBridge", () => {
257258
expect(heartbeatExecute).not.toHaveBeenCalled();
258259
});
259260

261+
it("keeps available and registered schemas paired with their tools", () => {
262+
const bridge = createCodexDynamicToolBridge({
263+
tools: [
264+
createTool({
265+
name: "message",
266+
parameters: {
267+
type: "object",
268+
properties: { current: { type: "string" } },
269+
},
270+
}),
271+
],
272+
registeredTools: [
273+
createTool({
274+
name: "message",
275+
parameters: {
276+
type: "object",
277+
properties: { durable: { type: "string" } },
278+
},
279+
}),
280+
],
281+
signal: new AbortController().signal,
282+
});
283+
284+
expect(bridge.availableSpecs[0]?.inputSchema).toEqual({
285+
type: "object",
286+
properties: { current: { type: "string" } },
287+
});
288+
expect(bridge.specs[0]?.inputSchema).toEqual({
289+
type: "object",
290+
properties: { durable: { type: "string" } },
291+
});
292+
});
293+
294+
it("quarantines dynamic tools with unsupported input schemas", async () => {
295+
const warn = vi.spyOn(embeddedAgentLog, "warn").mockImplementation(() => undefined);
296+
const badExecute = vi.fn();
297+
const bridge = createCodexDynamicToolBridge({
298+
tools: [
299+
createTool({ name: "message" }),
300+
createTool({
301+
name: "dofbot_move_angles",
302+
parameters: { type: "array", items: { type: "number" } },
303+
execute: badExecute,
304+
}),
305+
],
306+
signal: new AbortController().signal,
307+
});
308+
309+
expect(bridge.availableSpecs.map((tool) => tool.name)).toEqual(["message"]);
310+
expect(bridge.specs.map((tool) => tool.name)).toEqual(["message"]);
311+
expect(bridge.telemetry.quarantinedTools).toEqual([
312+
{
313+
tool: "dofbot_move_angles",
314+
violations: ['dofbot_move_angles.inputSchema.type must be "object"'],
315+
},
316+
]);
317+
expect(warn).toHaveBeenCalledWith(
318+
expect.stringContaining("dofbot_move_angles"),
319+
expect.objectContaining({
320+
tools: [
321+
{
322+
tool: "dofbot_move_angles",
323+
violations: ['dofbot_move_angles.inputSchema.type must be "object"'],
324+
},
325+
],
326+
}),
327+
);
328+
329+
const result = await bridge.handleToolCall({
330+
threadId: "thread-1",
331+
turnId: "turn-1",
332+
callId: "call-1",
333+
namespace: null,
334+
tool: "dofbot_move_angles",
335+
arguments: {},
336+
});
337+
338+
expect(result).toEqual({
339+
success: false,
340+
contentItems: [{ type: "inputText", text: "Unknown OpenClaw tool: dofbot_move_angles" }],
341+
});
342+
expect(badExecute).not.toHaveBeenCalled();
343+
});
344+
260345
it("can expose all dynamic tools directly for compatibility", () => {
261346
const bridge = createCodexDynamicToolBridge({
262347
tools: [createTool({ name: "web_search" }), createTool({ name: "message" })],

‎extensions/codex/src/app-server/dynamic-tools.ts‎

Lines changed: 86 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ import {
66
extractToolResultMediaArtifact,
77
filterToolResultMediaUrls,
88
HEARTBEAT_RESPONSE_TOOL_NAME,
9+
embeddedAgentLog,
910
type EmbeddedRunAttemptParams,
1011
isToolWrappedWithBeforeToolCallHook,
1112
isMessagingTool,
1213
isMessagingToolSendAction,
1314
normalizeHeartbeatToolResponse,
15+
projectRuntimeToolInputSchema,
1416
runAgentHarnessAfterToolCallHook,
1517
setBeforeToolCallDiagnosticsEnabled,
1618
type AnyAgentTool,
@@ -46,6 +48,16 @@ type CodexDynamicToolHookContext = {
4648

4749
type CodexToolResultHookContext = Omit<CodexDynamicToolHookContext, "config">;
4850

51+
type ProjectedCodexDynamicTool = {
52+
tool: AnyAgentTool;
53+
inputSchema: JsonValue;
54+
};
55+
56+
type CodexDynamicToolSchemaQuarantine = {
57+
tool: string;
58+
violations: readonly string[];
59+
};
60+
4961
export type CodexDynamicToolBridge = {
5062
availableSpecs: CodexDynamicToolSpec[];
5163
specs: CodexDynamicToolSpec[];
@@ -63,6 +75,7 @@ export type CodexDynamicToolBridge = {
6375
toolMediaUrls: string[];
6476
toolAudioAsVoice: boolean;
6577
successfulCronAdds?: number;
78+
quarantinedTools: CodexDynamicToolSchemaQuarantine[];
6679
};
6780
};
6881

@@ -83,16 +96,28 @@ export function createCodexDynamicToolBridge(params: {
8396
}): CodexDynamicToolBridge {
8497
const toolResultHookContext = toToolResultHookContext(params.hookContext);
8598
const toolResultMaxChars = resolveCodexDynamicToolResultMaxChars(params.hookContext);
86-
const tools = params.tools.map((tool) => {
99+
const availableProjection = projectCodexDynamicTools(params.tools);
100+
const registeredProjection = params.registeredTools
101+
? projectCodexDynamicTools(params.registeredTools)
102+
: availableProjection;
103+
const availableTools = availableProjection.tools.map(({ tool, inputSchema }) => {
87104
if (isToolWrappedWithBeforeToolCallHook(tool)) {
88105
setBeforeToolCallDiagnosticsEnabled(tool, false);
89-
return tool;
106+
return { tool, inputSchema };
90107
}
91-
return wrapToolWithBeforeToolCallHook(tool, params.hookContext, { emitDiagnostics: false });
108+
return {
109+
tool: wrapToolWithBeforeToolCallHook(tool, params.hookContext, { emitDiagnostics: false }),
110+
inputSchema,
111+
};
92112
});
93-
const toolMap = new Map(tools.map((tool) => [tool.name, tool]));
94-
const registeredTools = params.registeredTools ?? tools;
113+
const toolMap = new Map(availableTools.map(({ tool }) => [tool.name, tool]));
114+
const registeredTools = registeredProjection.tools.map(({ tool }) => tool);
95115
const registeredToolNames = new Set(registeredTools.map((tool) => tool.name));
116+
const quarantinedTools = dedupeQuarantinedDynamicTools([
117+
...availableProjection.quarantinedTools,
118+
...registeredProjection.quarantinedTools,
119+
]);
120+
warnQuarantinedDynamicTools(quarantinedTools);
96121
const telemetry: CodexDynamicToolBridge["telemetry"] = {
97122
didSendViaMessagingTool: false,
98123
messagingToolSentTexts: [],
@@ -101,6 +126,7 @@ export function createCodexDynamicToolBridge(params: {
101126
messagingToolSourceReplyPayloads: [],
102127
toolMediaUrls: [],
103128
toolAudioAsVoice: false,
129+
quarantinedTools,
104130
};
105131
const middlewareRunner = createAgentToolResultMiddlewareRunner({
106132
runtime: "codex",
@@ -114,16 +140,18 @@ export function createCodexDynamicToolBridge(params: {
114140
]);
115141

116142
return {
117-
availableSpecs: tools.map((tool) =>
143+
availableSpecs: availableTools.map(({ tool, inputSchema }) =>
118144
createCodexDynamicToolSpec({
119145
tool,
146+
inputSchema,
120147
loading: params.loading ?? "searchable",
121148
directToolNames,
122149
}),
123150
),
124-
specs: registeredTools.map((tool) =>
151+
specs: registeredProjection.tools.map(({ tool, inputSchema }) =>
125152
createCodexDynamicToolSpec({
126153
tool,
154+
inputSchema,
127155
loading: params.loading ?? "searchable",
128156
directToolNames,
129157
}),
@@ -257,13 +285,14 @@ export function createCodexDynamicToolBridge(params: {
257285

258286
function createCodexDynamicToolSpec(params: {
259287
tool: AnyAgentTool;
288+
inputSchema: JsonValue;
260289
loading: CodexDynamicToolsLoading;
261290
directToolNames: ReadonlySet<string>;
262291
}): CodexDynamicToolSpec {
263292
const base = {
264293
name: params.tool.name,
265294
description: params.tool.description,
266-
inputSchema: toJsonValue(params.tool.parameters),
295+
inputSchema: params.inputSchema,
267296
};
268297
if (params.loading === "direct" || params.directToolNames.has(params.tool.name)) {
269298
return base;
@@ -274,6 +303,55 @@ function createCodexDynamicToolSpec(params: {
274303
deferLoading: true,
275304
};
276305
}
306+
307+
function projectCodexDynamicTools(tools: readonly AnyAgentTool[]): {
308+
tools: ProjectedCodexDynamicTool[];
309+
quarantinedTools: CodexDynamicToolSchemaQuarantine[];
310+
} {
311+
const projectedTools: ProjectedCodexDynamicTool[] = [];
312+
const quarantinedTools: CodexDynamicToolSchemaQuarantine[] = [];
313+
for (const tool of tools) {
314+
const projection = projectRuntimeToolInputSchema(tool.parameters, `${tool.name}.inputSchema`);
315+
if (projection.violations.length > 0) {
316+
quarantinedTools.push({ tool: tool.name, violations: projection.violations });
317+
continue;
318+
}
319+
projectedTools.push({ tool, inputSchema: projection.schema as JsonValue });
320+
}
321+
return { tools: projectedTools, quarantinedTools };
322+
}
323+
324+
function warnQuarantinedDynamicTools(tools: readonly CodexDynamicToolSchemaQuarantine[]): void {
325+
if (tools.length === 0) {
326+
return;
327+
}
328+
const unique = new Map<string, readonly string[]>();
329+
for (const tool of tools) {
330+
unique.set(tool.tool, tool.violations);
331+
}
332+
embeddedAgentLog.warn(
333+
`codex app-server quarantined ${unique.size} dynamic ${unique.size === 1 ? "tool" : "tools"} with unsupported input schemas: ${[...unique.keys()].join(", ")}`,
334+
{
335+
tools: [...unique.entries()].map(([tool, violations]) => ({ tool, violations })),
336+
},
337+
);
338+
}
339+
340+
function dedupeQuarantinedDynamicTools(
341+
tools: readonly CodexDynamicToolSchemaQuarantine[],
342+
): CodexDynamicToolSchemaQuarantine[] {
343+
return [
344+
...new Map(
345+
tools.map((tool) => [
346+
tool.tool,
347+
{
348+
tool: tool.tool,
349+
violations: tool.violations,
350+
},
351+
]),
352+
).values(),
353+
];
354+
}
277355
function toToolResultHookContext(
278356
ctx: CodexDynamicToolHookContext | undefined,
279357
): CodexToolResultHookContext {
@@ -634,18 +712,6 @@ function convertToolContent(
634712
];
635713
}
636714

637-
function toJsonValue(value: unknown): JsonValue {
638-
try {
639-
const text = JSON.stringify(value);
640-
if (!text) {
641-
return {};
642-
}
643-
return JSON.parse(text) as JsonValue;
644-
} catch {
645-
return {};
646-
}
647-
}
648-
649715
function jsonObjectToRecord(value: JsonValue | undefined): Record<string, unknown> {
650716
if (!value || typeof value !== "object" || Array.isArray(value)) {
651717
return {};

‎src/agents/tool-schema-projection.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
inspectRuntimeToolInputSchemas,
4+
projectRuntimeToolInputSchema,
5+
} from "./tool-schema-projection.js";
6+
7+
describe("runtime tool input schema projection", () => {
8+
it("accepts JSON object input schemas", () => {
9+
expect(
10+
projectRuntimeToolInputSchema({
11+
type: "object",
12+
properties: {
13+
angle: { type: "number" },
14+
},
15+
}),
16+
).toEqual({
17+
schema: {
18+
type: "object",
19+
properties: {
20+
angle: { type: "number" },
21+
},
22+
},
23+
violations: [],
24+
});
25+
});
26+
27+
it("reports non-object dynamic tool input schemas", () => {
28+
expect(
29+
inspectRuntimeToolInputSchemas([
30+
{
31+
name: "dofbot_move_angles",
32+
parameters: { type: "array", items: { type: "number" } },
33+
},
34+
] as never),
35+
).toEqual([
36+
{
37+
toolName: "dofbot_move_angles",
38+
toolIndex: 0,
39+
violations: ['dofbot_move_angles.parameters.type must be "object"'],
40+
},
41+
]);
42+
});
43+
44+
it("reports dynamic JSON Schema keywords", () => {
45+
expect(
46+
projectRuntimeToolInputSchema({
47+
type: "object",
48+
anyOf: [{ $dynamicAnchor: "root" }],
49+
properties: {
50+
target: { $dynamicRef: "#target" },
51+
},
52+
}),
53+
).toEqual({
54+
schema: {
55+
type: "object",
56+
anyOf: [{ $dynamicAnchor: "root" }],
57+
properties: {
58+
target: { $dynamicRef: "#target" },
59+
},
60+
},
61+
violations: [
62+
"parameters.anyOf[0].$dynamicAnchor",
63+
"parameters.properties.target.$dynamicRef",
64+
],
65+
});
66+
});
67+
68+
it("does not report schema map field names as dynamic JSON Schema keywords", () => {
69+
expect(
70+
projectRuntimeToolInputSchema({
71+
type: "object",
72+
$defs: {
73+
$dynamicAnchor: { type: "string" },
74+
},
75+
properties: {
76+
$dynamicRef: { type: "string" },
77+
},
78+
}).violations,
79+
).toEqual([]);
80+
});
81+
});

0 commit comments

Comments
 (0)