liuyu/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-06-06 05:51:15 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(codex): quarantine unreadable dynamic tools

Browse Source
This commit is contained in:
Vincent Koc
2026-06-03 23:14:03 +02:00
parent 7b82901e58
commit 26135a1bc9
2 changed files with 281 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

110
extensions/codex/src/app-server/dynamic-tools.test.ts
View File

@@ -371,6 +371,116 @@ describe("createCodexDynamicToolBridge", () => {
expect(badExecute).not.toHaveBeenCalled();
});
it("quarantines unreadable dynamic tool descriptors without dropping healthy siblings", () => {
const warn = vi.spyOn(embeddedAgentLog, "warn").mockImplementation(() => undefined);
const poisonedName = createTool({
name: "fuzzplugin_unreadable_name",
execute: vi.fn(),
});
Object.defineProperty(poisonedName, "name", {
enumerable: true,
get() {
throw new Error("fuzzplugin dynamic tool name getter exploded");
},
});
const poisonedSchema = createTool({
name: "fuzzplugin_unreadable_schema",
execute: vi.fn(),
});
Object.defineProperty(poisonedSchema, "parameters", {
enumerable: true,
get() {
throw new Error("fuzzplugin dynamic tool schema getter exploded");
},
});
const invalidName = createTool({
name: "",
execute: vi.fn(),
});
const poisonedExecute = createTool({
name: "fuzzplugin_unreadable_execute",
});
Object.defineProperty(poisonedExecute, "execute", {
enumerable: true,
get() {
throw new Error("fuzzplugin dynamic tool execute getter exploded");
},
});
const bridge = createCodexDynamicToolBridge({
tools: [
poisonedName,
poisonedSchema,
invalidName,
poisonedExecute,
createTool({ name: "message" }),
],
signal: new AbortController().signal,
});
expect(bridge.availableSpecs.map((tool) => tool.name)).toEqual(["message"]);
expect(bridge.specs.map((tool) => tool.name)).toEqual(["message"]);
expect(bridge.telemetry.quarantinedTools).toEqual([
{
tool: "tool[0]",
violations: ["tool[0].name is unreadable"],
},
{
tool: "fuzzplugin_unreadable_schema",
violations: ["fuzzplugin_unreadable_schema.inputSchema is unreadable"],
},
{
tool: "tool[2]",
violations: ["tool[2].name must be a non-empty string"],
},
{
tool: "fuzzplugin_unreadable_execute",
violations: [
"fuzzplugin_unreadable_execute could not be wrapped for before-tool-call hooks",
],
},
]);
expect(warn).toHaveBeenCalledWith(
expect.stringContaining(
"tool[0], fuzzplugin_unreadable_schema, tool[2], fuzzplugin_unreadable_execute",
),
expect.objectContaining({
tools: [
{
tool: "tool[0]",
violations: ["tool[0].name is unreadable"],
},
{
tool: "fuzzplugin_unreadable_schema",
violations: ["fuzzplugin_unreadable_schema.inputSchema is unreadable"],
},
{
tool: "tool[2]",
violations: ["tool[2].name must be a non-empty string"],
},
{
tool: "fuzzplugin_unreadable_execute",
violations: [
"fuzzplugin_unreadable_execute could not be wrapped for before-tool-call hooks",
],
},
],
}),
);
const registeredBridge = createCodexDynamicToolBridge({
tools: [poisonedExecute, createTool({ name: "message" })],
registeredTools: [
createTool({ name: "fuzzplugin_unreadable_execute" }),
createTool({ name: "message" }),
],
signal: new AbortController().signal,
});
expect(registeredBridge.availableSpecs.map((tool) => tool.name)).toEqual(["message"]);
expect(registeredBridge.specs.map((tool) => tool.name)).toEqual(["message"]);
});
it("can expose all dynamic tools directly for compatibility", () => {
const bridge = createCodexDynamicToolBridge({
tools: [createTool({ name: "web_search" }), createTool({ name: "message" })],

209
extensions/codex/src/app-server/dynamic-tools.ts
View File

@@ -51,6 +51,8 @@ type CodexToolResultHookContext = Omit<CodexDynamicToolHookContext, "config">;
type ProjectedCodexDynamicTool = {
tool: AnyAgentTool;
name: string;
description: string;
inputSchema: JsonValue;
};
@@ -101,22 +103,25 @@ export function createCodexDynamicToolBridge(params: {
const registeredProjection = params.registeredTools
? projectCodexDynamicTools(params.registeredTools)
: availableProjection;
const availableTools = availableProjection.tools.map(({ tool, inputSchema }) => {
if (isToolWrappedWithBeforeToolCallHook(tool)) {
setBeforeToolCallDiagnosticsEnabled(tool, false);
return { tool, inputSchema };
}
return {
tool: wrapToolWithBeforeToolCallHook(tool, params.hookContext, { emitDiagnostics: false }),
inputSchema,
};
});
const toolMap = new Map(availableTools.map(({ tool }) => [tool.name, tool]));
const registeredTools = registeredProjection.tools.map(({ tool }) => tool);
const registeredToolNames = new Set(registeredTools.map((tool) => tool.name));
const wrappedAvailableProjection = wrapProjectedCodexDynamicTools(
availableProjection.tools,
params.hookContext,
);
const availableTools = wrappedAvailableProjection.tools;
const quarantinedAvailableToolNames = new Set(
[...availableProjection.quarantinedTools, ...wrappedAvailableProjection.quarantinedTools].map(
(tool) => tool.tool,
),
);
const registeredSpecTools = (
params.registeredTools ? registeredProjection.tools : availableTools
).filter((entry) => !quarantinedAvailableToolNames.has(entry.name));
const toolMap = new Map(availableTools.map((entry) => [entry.name, entry]));
const registeredToolNames = new Set(registeredSpecTools.map((entry) => entry.name));
const quarantinedTools = dedupeQuarantinedDynamicTools([
...availableProjection.quarantinedTools,
...registeredProjection.quarantinedTools,
...wrappedAvailableProjection.quarantinedTools,
]);
warnQuarantinedDynamicTools(quarantinedTools);
emitQuarantinedDynamicToolDiagnostics(quarantinedTools, params.hookContext);
@@ -142,26 +147,24 @@ export function createCodexDynamicToolBridge(params: {
]);
return {
availableSpecs: availableTools.map(({ tool, inputSchema }) =>
availableSpecs: availableTools.map((entry) =>
createCodexDynamicToolSpec({
tool,
inputSchema,
entry,
loading: params.loading ?? "searchable",
directToolNames,
}),
),
specs: registeredProjection.tools.map(({ tool, inputSchema }) =>
specs: registeredSpecTools.map((entry) =>
createCodexDynamicToolSpec({
tool,
inputSchema,
entry,
loading: params.loading ?? "searchable",
directToolNames,
}),
),
telemetry,
handleToolCall: async (call, options) => {
const tool = toolMap.get(call.tool);
if (!tool) {
const toolEntry = toolMap.get(call.tool);
if (!toolEntry) {
if (registeredToolNames.has(call.tool)) {
return {
contentItems: [
@@ -178,6 +181,7 @@ export function createCodexDynamicToolBridge(params: {
success: false,
};
}
const { tool, name: toolName } = toolEntry;
const args = jsonObjectToRecord(call.arguments);
const startedAt = Date.now();
const signal = composeAbortSignals(params.signal, options?.signal);
@@ -191,7 +195,7 @@ export function createCodexDynamicToolBridge(params: {
threadId: call.threadId,
turnId: call.turnId,
toolCallId: call.callId,
toolName: tool.name,
toolName,
args,
isError: rawIsError,
result: rawResult,
@@ -200,13 +204,13 @@ export function createCodexDynamicToolBridge(params: {
threadId: call.threadId,
turnId: call.turnId,
toolCallId: call.callId,
toolName: tool.name,
toolName,
args,
result: middlewareResult,
});
const resultIsError = rawIsError || isToolResultError(result);
collectToolTelemetry({
toolName: tool.name,
toolName,
args,
result,
mediaTrustResult: rawResult,
@@ -214,7 +218,7 @@ export function createCodexDynamicToolBridge(params: {
isError: resultIsError,
});
void runAgentHarnessAfterToolCallHook({
toolName: tool.name,
toolName,
toolCallId: call.callId,
runId: toolResultHookContext.runId,
agentId: toolResultHookContext.agentId,
@@ -247,14 +251,14 @@ export function createCodexDynamicToolBridge(params: {
return withSideEffectEvidence(response, terminalType !== "blocked");
} catch (error) {
collectToolTelemetry({
toolName: tool.name,
toolName,
args,
result: undefined,
telemetry,
isError: true,
});
void runAgentHarnessAfterToolCallHook({
toolName: tool.name,
toolName,
toolCallId: call.callId,
runId: toolResultHookContext.runId,
agentId: toolResultHookContext.agentId,
@@ -285,18 +289,49 @@ export function createCodexDynamicToolBridge(params: {
};
}
function wrapProjectedCodexDynamicTools(
tools: readonly ProjectedCodexDynamicTool[],
hookContext: CodexDynamicToolHookContext | undefined,
): {
tools: ProjectedCodexDynamicTool[];
quarantinedTools: CodexDynamicToolSchemaQuarantine[];
} {
const wrappedTools: ProjectedCodexDynamicTool[] = [];
const quarantinedTools: CodexDynamicToolSchemaQuarantine[] = [];
for (const entry of tools) {
try {
if (isToolWrappedWithBeforeToolCallHook(entry.tool)) {
setBeforeToolCallDiagnosticsEnabled(entry.tool, false);
wrappedTools.push(entry);
continue;
}
wrappedTools.push({
...entry,
tool: wrapToolWithBeforeToolCallHook(entry.tool, hookContext, {
emitDiagnostics: false,
}),
});
} catch {
quarantinedTools.push({
tool: entry.name,
violations: [`${entry.name} could not be wrapped for before-tool-call hooks`],
});
}
}
return { tools: wrappedTools, quarantinedTools };
}
function createCodexDynamicToolSpec(params: {
tool: AnyAgentTool;
inputSchema: JsonValue;
entry: ProjectedCodexDynamicTool;
loading: CodexDynamicToolsLoading;
directToolNames: ReadonlySet<string>;
}): CodexDynamicToolSpec {
const base = {
name: params.tool.name,
description: params.tool.description,
inputSchema: params.inputSchema,
name: params.entry.name,
description: params.entry.description,
inputSchema: params.entry.inputSchema,
};
if (params.loading === "direct" || params.directToolNames.has(params.tool.name)) {
if (params.loading === "direct" || params.directToolNames.has(params.entry.name)) {
return base;
}
return {
@@ -312,17 +347,115 @@ function projectCodexDynamicTools(tools: readonly AnyAgentTool[]): {
} {
const projectedTools: ProjectedCodexDynamicTool[] = [];
const quarantinedTools: CodexDynamicToolSchemaQuarantine[] = [];
for (const tool of tools) {
const projection = projectRuntimeToolInputSchema(tool.parameters, `${tool.name}.inputSchema`);
if (projection.violations.length > 0) {
quarantinedTools.push({ tool: tool.name, violations: projection.violations });
let length: number;
try {
length = tools.length;
} catch {
return {
tools: [],
quarantinedTools: [{ tool: "tool[0]", violations: ["tool[0] is unreadable"] }],
};
}
for (let toolIndex = 0; toolIndex < length; toolIndex += 1) {
let tool: AnyAgentTool;
try {
tool = tools[toolIndex]!;
} catch {
quarantinedTools.push({
tool: `tool[${toolIndex}]`,
violations: [`tool[${toolIndex}] is unreadable`],
});
continue;
}
projectedTools.push({ tool, inputSchema: projection.schema as JsonValue });
const descriptor = readCodexDynamicToolDescriptor(tool, toolIndex);
if (!descriptor.ok) {
quarantinedTools.push(descriptor.diagnostic);
continue;
}
const projection = projectRuntimeToolInputSchema(
descriptor.parameters,
`${descriptor.name}.inputSchema`,
);
if (projection.violations.length > 0) {
quarantinedTools.push({ tool: descriptor.name, violations: projection.violations });
continue;
}
projectedTools.push({
tool,
name: descriptor.name,
description: descriptor.description,
inputSchema: projection.schema as JsonValue,
});
}
return { tools: projectedTools, quarantinedTools };
}
type CodexDynamicToolDescriptorRead =
| {
ok: true;
name: string;
description: string;
parameters: unknown;
}
| {
ok: false;
diagnostic: CodexDynamicToolSchemaQuarantine;
};
function readCodexDynamicToolDescriptor(
tool: AnyAgentTool,
toolIndex: number,
): CodexDynamicToolDescriptorRead {
const fallbackName = `tool[${toolIndex}]`;
let name: string;
try {
const rawName = tool.name;
if (typeof rawName !== "string" || !rawName) {
return {
ok: false,
diagnostic: {
tool: fallbackName,
violations: [`${fallbackName}.name must be a non-empty string`],
},
};
}
name = rawName;
} catch {
return {
ok: false,
diagnostic: {
tool: fallbackName,
violations: [`${fallbackName}.name is unreadable`],
},
};
}
let description: string;
try {
description = typeof tool.description === "string" ? tool.description : "";
} catch {
return {
ok: false,
diagnostic: {
tool: name,
violations: [`${name}.description is unreadable`],
},
};
}
let parameters: unknown;
try {
parameters = tool.parameters;
} catch {
return {
ok: false,
diagnostic: {
tool: name,
violations: [`${name}.inputSchema is unreadable`],
},
};
}
return { ok: true, name, description, parameters };
}
function warnQuarantinedDynamicTools(tools: readonly CodexDynamicToolSchemaQuarantine[]): void {
if (tools.length === 0) {
return;