Skip to content

Commit 8fd9d20

Browse files
vincentkocvincentkoc
committed
fix(agents): quarantine normalized runtime tools
1 parent 1d62f4c commit 8fd9d20

8 files changed

Lines changed: 245 additions & 45 deletions

File tree

‎extensions/codex/src/app-server/dynamic-tool-build.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,9 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
267267
},
268268
});
269269
toolBuildStages.mark("create-openclaw-coding-tools");
270-
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [];
270+
const toolSchemaDiagnostics: RuntimeToolSchemaDiagnostic[] = [];
271271
const readableAllToolProjection = filterProviderNormalizableTools(allTools);
272-
preNormalizationDiagnostics.push(...readableAllToolProjection.diagnostics);
272+
toolSchemaDiagnostics.push(...readableAllToolProjection.diagnostics);
273273
const readableAllTools = [...readableAllToolProjection.tools];
274274
const codexFilteredTools = addNodeShellDynamicToolsIfNeeded(
275275
addSandboxShellDynamicToolsIfAvailable(
@@ -301,17 +301,16 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
301301
modelId: params.modelId,
302302
modelApi: params.model.api,
303303
model: params.model,
304-
onPreNormalizationSchemaDiagnostics: (diagnostics) =>
305-
preNormalizationDiagnostics.push(...diagnostics),
304+
onToolSchemaDiagnostics: (diagnostics) => toolSchemaDiagnostics.push(...diagnostics),
306305
});
307306
toolBuildStages.mark("runtime-normalization");
308-
if (preNormalizationDiagnostics.length > 0) {
307+
if (toolSchemaDiagnostics.length > 0) {
309308
embeddedAgentLog.warn(
310-
`codex app-server quarantined ${preNormalizationDiagnostics.length} unsupported runtime tool schema${preNormalizationDiagnostics.length === 1 ? "" : "s"} before dynamic tool registration`,
309+
`codex app-server quarantined ${toolSchemaDiagnostics.length} unsupported runtime tool schema${toolSchemaDiagnostics.length === 1 ? "" : "s"} before dynamic tool registration`,
311310
{
312311
runId: params.runId,
313312
sessionId: params.sessionId,
314-
diagnostics: preNormalizationDiagnostics.map((diagnostic) => ({
313+
diagnostics: toolSchemaDiagnostics.map((diagnostic) => ({
315314
index: diagnostic.toolIndex,
316315
tool: diagnostic.toolName,
317316
violations: diagnostic.violations.slice(0, 12),

‎src/agents/embedded-agent-runner/run/attempt.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,7 +1413,7 @@ export async function runEmbeddedAttempt(
14131413
modelApi: params.model.api,
14141414
model: params.model,
14151415
runtimeHandle: getProviderRuntimeHandle(),
1416-
onPreNormalizationSchemaDiagnostics: (diagnostics, sourceTools) =>
1416+
onToolSchemaDiagnostics: (diagnostics, sourceTools) =>
14171417
logRuntimeToolSchemaQuarantine({
14181418
diagnostics,
14191419
tools: sourceTools,
@@ -1509,7 +1509,7 @@ export async function runEmbeddedAttempt(
15091509
modelApi: params.model.api,
15101510
model: params.model,
15111511
runtimeHandle: getProviderRuntimeHandle(),
1512-
onPreNormalizationSchemaDiagnostics: (diagnostics, sourceTools) =>
1512+
onToolSchemaDiagnostics: (diagnostics, sourceTools) =>
15131513
logRuntimeToolSchemaQuarantine({
15141514
diagnostics,
15151515
tools: sourceTools,

‎src/agents/runtime-plan/tools.test.ts‎

Lines changed: 156 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ describe("AgentRuntimePlan tool policy helpers", () => {
8080
runtimePlan,
8181
tools,
8282
provider: "openai",
83-
onPreNormalizationSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
83+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
8484
}),
8585
).toEqual([healthy]);
8686
expect(normalize).toHaveBeenCalledWith([healthy], {
@@ -112,7 +112,7 @@ describe("AgentRuntimePlan tool policy helpers", () => {
112112
normalizeAgentRuntimeTools({
113113
tools: [arraySchema, healthy],
114114
provider: "openai",
115-
onPreNormalizationSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
115+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
116116
}),
117117
).toEqual([healthy]);
118118
expect(mocks.normalizeProviderToolSchemas).toHaveBeenCalledWith(
@@ -132,6 +132,53 @@ describe("AgentRuntimePlan tool policy helpers", () => {
132132
]);
133133
});
134134

135+
it("quarantines provider-normalized tools that remain runtime-incompatible", () => {
136+
const source = {
137+
...createParameterFreeTool("fuzzplugin_dynamic_ref"),
138+
} as unknown as AgentTool;
139+
setPluginToolMeta(source, {
140+
pluginId: "fuzzplugin",
141+
optional: false,
142+
});
143+
const dynamicSchema = {
144+
...source,
145+
parameters: { type: "object", $dynamicRef: "#fuzz" },
146+
} as unknown as AgentTool;
147+
const healthy = { ...createParameterFreeTool(), name: "healthy" } as AgentTool;
148+
const diagnostics: RuntimeToolSchemaDiagnostic[][] = [];
149+
const diagnosticSources: (readonly AgentTool[])[] = [];
150+
mocks.normalizeProviderToolSchemas.mockReturnValueOnce([dynamicSchema, healthy]);
151+
152+
expect(
153+
normalizeAgentRuntimeTools({
154+
tools: [source, healthy],
155+
provider: "openai",
156+
onToolSchemaDiagnostics: (entries, sourceTools) => {
157+
diagnostics.push([...entries]);
158+
diagnosticSources.push(sourceTools);
159+
},
160+
}),
161+
).toEqual([healthy]);
162+
expect(mocks.normalizeProviderToolSchemas).toHaveBeenCalledWith(
163+
expect.objectContaining({
164+
tools: [source, healthy],
165+
provider: "openai",
166+
}),
167+
);
168+
expect(diagnostics).toEqual([
169+
[
170+
{
171+
toolName: "fuzzplugin_dynamic_ref",
172+
toolIndex: 0,
173+
violations: ["fuzzplugin_dynamic_ref.parameters.$dynamicRef"],
174+
},
175+
],
176+
]);
177+
expect(getPluginToolMeta(diagnosticSources[0]?.[0])).toMatchObject({
178+
pluginId: "fuzzplugin",
179+
});
180+
});
181+
135182
it("accepts legacy optional model fields while normalizing RuntimePlan context", () => {
136183
const tools = [createParameterFreeTool()] as AgentTool[];
137184
const normalize = vi.fn(() => tools);
@@ -253,7 +300,60 @@ describe("AgentRuntimePlan tool policy helpers", () => {
253300
const result = normalizeAgentRuntimeTools({
254301
tools: [unreadableName, healthy],
255302
provider: "openai",
256-
onPreNormalizationSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
303+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
304+
});
305+
306+
expect(result).toEqual([normalized]);
307+
expect(getPluginToolMeta(result[0])).toMatchObject({
308+
pluginId: "bundle-mcp",
309+
mcp: {
310+
serverName: "fixture",
311+
toolName: "lookup_note",
312+
},
313+
});
314+
expect(diagnostics).toEqual([
315+
[
316+
{
317+
toolName: "tool[0]",
318+
toolIndex: 0,
319+
violations: ["tool[0].name is unreadable"],
320+
},
321+
],
322+
]);
323+
});
324+
325+
it("quarantines unreadable provider-normalized tools before preserving metadata", () => {
326+
const source = createParameterFreeTool("fixture__lookup_note") as AgentTool;
327+
setPluginToolMeta(source, {
328+
pluginId: "bundle-mcp",
329+
optional: false,
330+
mcp: {
331+
serverName: "fixture",
332+
safeServerName: "fixture",
333+
toolName: "lookup_note",
334+
operation: "tool",
335+
},
336+
});
337+
const unreadableNormalized = {
338+
...createParameterFreeTool("fuzzplugin_normalized_unreadable"),
339+
} as AgentTool;
340+
Object.defineProperty(unreadableNormalized, "name", {
341+
enumerable: true,
342+
get() {
343+
throw new Error("fuzzplugin normalized name getter exploded");
344+
},
345+
});
346+
const normalized = {
347+
...source,
348+
parameters: normalizedParameterFreeSchema(),
349+
};
350+
const diagnostics: RuntimeToolSchemaDiagnostic[][] = [];
351+
mocks.normalizeProviderToolSchemas.mockReturnValueOnce([unreadableNormalized, normalized]);
352+
353+
const result = normalizeAgentRuntimeTools({
354+
tools: [source],
355+
provider: "openai",
356+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
257357
});
258358

259359
expect(result).toEqual([normalized]);
@@ -275,6 +375,58 @@ describe("AgentRuntimePlan tool policy helpers", () => {
275375
]);
276376
});
277377

378+
it("quarantines unreadable provider-normalized array entries before preserving metadata", () => {
379+
const source = createParameterFreeTool("fixture__lookup_note") as AgentTool;
380+
setPluginToolMeta(source, {
381+
pluginId: "bundle-mcp",
382+
optional: false,
383+
mcp: {
384+
serverName: "fixture",
385+
safeServerName: "fixture",
386+
toolName: "lookup_note",
387+
operation: "tool",
388+
},
389+
});
390+
const normalized = {
391+
...source,
392+
parameters: normalizedParameterFreeSchema(),
393+
};
394+
const normalizedTools = new Proxy([undefined, normalized] as unknown as AgentTool[], {
395+
get(target, property, receiver) {
396+
if (property === "0") {
397+
throw new Error("fuzzplugin normalized array slot exploded");
398+
}
399+
return Reflect.get(target, property, receiver);
400+
},
401+
});
402+
const diagnostics: RuntimeToolSchemaDiagnostic[][] = [];
403+
mocks.normalizeProviderToolSchemas.mockReturnValueOnce(normalizedTools);
404+
405+
const result = normalizeAgentRuntimeTools({
406+
tools: [source],
407+
provider: "openai",
408+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
409+
});
410+
411+
expect(result).toEqual([normalized]);
412+
expect(getPluginToolMeta(result[0])).toMatchObject({
413+
pluginId: "bundle-mcp",
414+
mcp: {
415+
serverName: "fixture",
416+
toolName: "lookup_note",
417+
},
418+
});
419+
expect(diagnostics).toEqual([
420+
[
421+
{
422+
toolName: "tool[0]",
423+
toolIndex: 0,
424+
violations: ["tool[0] is unreadable"],
425+
},
426+
],
427+
]);
428+
});
429+
278430
it("quarantines unreadable tools before provider schema normalization", () => {
279431
const healthy = { ...createParameterFreeTool(), name: "healthy" } as AgentTool;
280432
const unreadable = { ...createParameterFreeTool(), name: "fuzzplugin_unreadable" } as AgentTool;
@@ -292,7 +444,7 @@ describe("AgentRuntimePlan tool policy helpers", () => {
292444
normalizeAgentRuntimeTools({
293445
tools,
294446
provider: "openai",
295-
onPreNormalizationSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
447+
onToolSchemaDiagnostics: (entries) => diagnostics.push([...entries]),
296448
}),
297449
).toEqual([healthy]);
298450
expect(mocks.normalizeProviderToolSchemas).toHaveBeenCalledWith(

‎src/agents/runtime-plan/tools.ts‎

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
import type { AgentTool } from "../runtime/index.js";
1212
import {
1313
filterProviderNormalizableTools,
14+
filterRuntimeCompatibleTools,
1415
type RuntimeToolSchemaDiagnostic,
1516
} from "../tool-schema-projection.js";
1617
import type { AgentRuntimePlan } from "./types.js";
@@ -27,7 +28,7 @@ type AgentRuntimeToolPolicyParams<TSchemaType extends TSchema = TSchema, TResult
2728
model?: ProviderRuntimeModel;
2829
runtimeHandle?: ProviderRuntimePluginHandle;
2930
allowProviderRuntimePluginLoad?: boolean;
30-
onPreNormalizationSchemaDiagnostics?: (
31+
onToolSchemaDiagnostics?: (
3132
diagnostics: readonly RuntimeToolSchemaDiagnostic[],
3233
tools: readonly AgentTool<TSchemaType, TResult>[],
3334
) => void;
@@ -53,14 +54,49 @@ function copyRuntimeToolMetadata(source: AgentTool, target: AgentTool): void {
5354
copyChannelAgentToolMeta(source as never, target as never);
5455
}
5556

57+
function readRuntimeToolName(tool: AgentTool): string | undefined {
58+
try {
59+
return typeof tool.name === "string" && tool.name ? tool.name : undefined;
60+
} catch {
61+
return undefined;
62+
}
63+
}
64+
65+
function readRuntimeToolArrayLength(tools: readonly AgentTool[]): number {
66+
try {
67+
return tools.length;
68+
} catch {
69+
return 0;
70+
}
71+
}
72+
73+
function readRuntimeToolAt<TSchemaType extends TSchema = TSchema, TResult = unknown>(
74+
tools: readonly AgentTool<TSchemaType, TResult>[],
75+
index: number,
76+
): AgentTool<TSchemaType, TResult> | undefined {
77+
try {
78+
return tools[index];
79+
} catch {
80+
return undefined;
81+
}
82+
}
83+
5684
function preserveRuntimeToolMetadata<TSchemaType extends TSchema = TSchema, TResult = unknown>(
5785
sourceTools: AgentTool<TSchemaType, TResult>[],
5886
normalizedTools: AgentTool<TSchemaType, TResult>[],
5987
): AgentTool<TSchemaType, TResult>[] {
6088
const sourcesByUniqueName = new Map<string, AgentTool<TSchemaType, TResult>>();
6189
const duplicateNames = new Set<string>();
62-
for (const source of sourceTools) {
63-
const name = source.name;
90+
const sourceLength = readRuntimeToolArrayLength(sourceTools);
91+
for (let index = 0; index < sourceLength; index += 1) {
92+
const source = readRuntimeToolAt(sourceTools, index);
93+
if (!source) {
94+
continue;
95+
}
96+
const name = readRuntimeToolName(source);
97+
if (!name) {
98+
continue;
99+
}
64100
if (sourcesByUniqueName.has(name)) {
65101
duplicateNames.add(name);
66102
sourcesByUniqueName.delete(name);
@@ -70,10 +106,20 @@ function preserveRuntimeToolMetadata<TSchemaType extends TSchema = TSchema, TRes
70106
sourcesByUniqueName.set(name, source);
71107
}
72108
}
73-
for (const [index, target] of normalizedTools.entries()) {
109+
const targetLength = readRuntimeToolArrayLength(normalizedTools);
110+
for (let index = 0; index < targetLength; index += 1) {
111+
const target = readRuntimeToolAt(normalizedTools, index);
112+
if (!target) {
113+
continue;
114+
}
115+
const targetName = readRuntimeToolName(target);
116+
if (!targetName) {
117+
continue;
118+
}
74119
const indexedSource = sourceTools[index];
120+
const indexedSourceName = indexedSource ? readRuntimeToolName(indexedSource) : undefined;
75121
const source =
76-
indexedSource?.name === target.name ? indexedSource : sourcesByUniqueName.get(target.name);
122+
indexedSourceName === targetName ? indexedSource : sourcesByUniqueName.get(targetName);
77123
if (source) {
78124
copyRuntimeToolMetadata(source, target);
79125
}
@@ -88,10 +134,7 @@ export function normalizeAgentRuntimeTools<
88134
const planContext = runtimePlanToolContext(params);
89135
const normalizableToolProjection = filterProviderNormalizableTools(params.tools);
90136
if (normalizableToolProjection.diagnostics.length > 0) {
91-
params.onPreNormalizationSchemaDiagnostics?.(
92-
normalizableToolProjection.diagnostics,
93-
params.tools,
94-
);
137+
params.onToolSchemaDiagnostics?.(normalizableToolProjection.diagnostics, params.tools);
95138
}
96139
const normalizableTools = [...normalizableToolProjection.tools] as AgentTool<
97140
TSchemaType,
@@ -112,7 +155,16 @@ export function normalizeAgentRuntimeTools<
112155
allowRuntimePluginLoad: params.allowProviderRuntimePluginLoad,
113156
});
114157
const normalizedTools = Array.isArray(normalized) ? normalized : normalizableTools;
115-
return preserveRuntimeToolMetadata(normalizableTools, normalizedTools);
158+
const metadataPreservedTools = preserveRuntimeToolMetadata(normalizableTools, normalizedTools);
159+
const runtimeToolProjection = filterRuntimeCompatibleTools(metadataPreservedTools);
160+
if (runtimeToolProjection.diagnostics.length > 0) {
161+
params.onToolSchemaDiagnostics?.(runtimeToolProjection.diagnostics, metadataPreservedTools);
162+
}
163+
const runtimeCompatibleTools =
164+
runtimeToolProjection.diagnostics.length > 0
165+
? [...runtimeToolProjection.tools]
166+
: metadataPreservedTools;
167+
return runtimeCompatibleTools;
116168
}
117169

118170
export function logAgentRuntimeToolDiagnostics(params: AgentRuntimeToolPolicyParams): void {

‎src/agents/tools-effective-inventory-build.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ export function buildRuntimeCompatibleToolInventory(params: {
208208
} {
209209
const rawToolsByName = buildReadableRawToolsByName(params.tools);
210210
const preNormalizationProjection = filterProviderNormalizableTools(params.tools);
211-
const preNormalizationDiagnostics: RuntimeToolSchemaDiagnostic[] = [
211+
const toolSchemaDiagnostics: RuntimeToolSchemaDiagnostic[] = [
212212
...preNormalizationProjection.diagnostics,
213213
];
214214
const normalizedTools = normalizeAgentRuntimeTools({
@@ -221,11 +221,10 @@ export function buildRuntimeCompatibleToolInventory(params: {
221221
modelId: params.modelId,
222222
modelApi: params.modelApi ?? undefined,
223223
model: params.runtimeModel,
224-
onPreNormalizationSchemaDiagnostics: (diagnostics) =>
225-
preNormalizationDiagnostics.push(...diagnostics),
224+
onToolSchemaDiagnostics: (diagnostics) => toolSchemaDiagnostics.push(...diagnostics),
226225
});
227226
const projection = filterRuntimeCompatibleTools(normalizedTools);
228-
const diagnostics = [...preNormalizationDiagnostics, ...projection.diagnostics];
227+
const diagnostics = [...toolSchemaDiagnostics, ...projection.diagnostics];
229228
return {
230229
entries: buildEffectiveToolInventoryEntries(projection.tools, rawToolsByName),
231230
notices: buildUnsupportedToolSchemaNotices({

0 commit comments

Comments
 (0)