Skip to content

Commit 6e20a32

Browse files
committed
fix(agents): tolerate provider tool schema hook failures
1 parent 8071b06 commit 6e20a32

14 files changed

Lines changed: 389 additions & 26 deletions

File tree

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,32 @@ describe("Codex app-server dynamic tool build", () => {
205205
await expect(buildDynamicToolsForTest(params, workspaceDir)).resolves.toEqual([messageTool]);
206206
});
207207

208+
it("uses assistant-tolerant provider schema hooks for dynamic tool projection", async () => {
209+
const messageTool = createRuntimeDynamicTool("message");
210+
setOpenClawCodingToolsFactoryForTests(() => [messageTool]);
211+
const sessionFile = path.join(tempDir, "session.jsonl");
212+
const workspaceDir = path.join(tempDir, "workspace");
213+
const params = createParams(sessionFile, workspaceDir);
214+
const normalize = vi.fn((tools: unknown[]) => tools);
215+
params.disableTools = false;
216+
params.runtimePlan = {
217+
...createCodexRuntimePlanFixture(),
218+
tools: {
219+
normalize,
220+
logDiagnostics: () => undefined,
221+
},
222+
} as never;
223+
224+
await expect(buildDynamicToolsForTest(params, workspaceDir)).resolves.toEqual([messageTool]);
225+
226+
expect(normalize).toHaveBeenCalledWith([messageTool], {
227+
workspaceDir,
228+
modelApi: params.model.api,
229+
model: params.model,
230+
schemaHookFailureMode: "warn",
231+
});
232+
});
233+
208234
it("limits Codex memory flush runs to managed read and write tools", async () => {
209235
const factoryOptions: unknown[] = [];
210236
setOpenClawCodingToolsFactoryForTests((options) => {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,7 @@ export async function buildDynamicTools(input: DynamicToolBuildParams) {
301301
modelId: params.modelId,
302302
modelApi: params.model.api,
303303
model: params.model,
304+
schemaHookFailureMode: "warn",
304305
onPreNormalizationSchemaDiagnostics: (diagnostics) =>
305306
preNormalizationDiagnostics.push(...diagnostics),
306307
});

src/agents/embedded-agent-runner/compact.hooks.harness.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,8 @@ export const rotateTranscriptAfterCompactionMock: Mock<
174174
rotated: false,
175175
}));
176176
export const enqueueCommandInLaneMock = vi.fn((_lane: unknown, task: () => unknown) => task());
177+
export const runtimePlanNormalizeToolsMock = vi.fn((tools: unknown[]) => tools);
178+
export const runtimePlanLogDiagnosticsMock = vi.fn();
177179

178180
function createCompactHooksRuntimePlan(params: BuildAgentRuntimePlanParams): AgentRuntimePlan {
179181
const modelApi = params.modelApi ?? params.model?.api ?? undefined;
@@ -215,8 +217,8 @@ function createCompactHooksRuntimePlan(params: BuildAgentRuntimePlanParams): Age
215217
preparedPlanning: {
216218
loadMetadataSnapshot: () => ({}),
217219
},
218-
normalize: vi.fn((tools) => tools),
219-
logDiagnostics: vi.fn(),
220+
normalize: runtimePlanNormalizeToolsMock,
221+
logDiagnostics: runtimePlanLogDiagnosticsMock,
220222
},
221223
transcript: {
222224
policy: transcriptPolicy,
@@ -336,6 +338,9 @@ export function resetCompactSessionStateMocks(): void {
336338
rotateTranscriptAfterCompactionMock.mockResolvedValue({ rotated: false });
337339
enqueueCommandInLaneMock.mockReset();
338340
enqueueCommandInLaneMock.mockImplementation((_lane: unknown, task: () => unknown) => task());
341+
runtimePlanNormalizeToolsMock.mockReset();
342+
runtimePlanNormalizeToolsMock.mockImplementation((tools: unknown[]) => tools);
343+
runtimePlanLogDiagnosticsMock.mockReset();
339344
listRegisteredPluginAgentPromptGuidanceMock.mockReset();
340345
listRegisteredPluginAgentPromptGuidanceMock.mockImplementation((params?: { surface?: string }) =>
341346
params?.surface === "subagent"

src/agents/embedded-agent-runner/compact.hooks.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import {
2424
resolveEmbeddedAgentStreamFnMock,
2525
resolveMemorySearchConfigMock,
2626
resolveModelMock,
27+
runtimePlanLogDiagnosticsMock,
28+
runtimePlanNormalizeToolsMock,
2729
resolveSandboxContextMock,
2830
resolveSessionAgentIdMock,
2931
resolveSessionAgentIdsMock,
@@ -496,6 +498,49 @@ describe("compactEmbeddedAgentSessionDirect hooks", () => {
496498
expect(sessionOptions.tools).toEqual(["healthy_lookup"]);
497499
});
498500

501+
it("uses assistant-tolerant provider schema hooks during compaction tool projection", async () => {
502+
resolveContextEngineMock.mockResolvedValueOnce({
503+
info: { ownsCompaction: false },
504+
compact: contextEngineCompactMock,
505+
});
506+
resolveModelMock.mockReturnValueOnce({
507+
model: { provider: "openai", api: "openai-responses", id: "fake", input: [] },
508+
error: null,
509+
authStorage: { setRuntimeApiKey: vi.fn() },
510+
modelRegistry: {},
511+
});
512+
createOpenClawCodingToolsMock.mockReturnValueOnce([
513+
{
514+
name: "healthy_lookup",
515+
label: "Healthy Lookup",
516+
description: "Look up safe data.",
517+
parameters: { type: "object", properties: {} },
518+
execute: async () => ({ text: "ok" }),
519+
},
520+
] as never);
521+
522+
await compactEmbeddedAgentSessionDirect({
523+
sessionId: "session-1",
524+
sessionKey: "agent:main:session-1",
525+
sessionFile: "/tmp/session.jsonl",
526+
workspaceDir: "/tmp/workspace",
527+
runId: "run-tool-schema-hook-tolerance",
528+
});
529+
530+
expect(runtimePlanNormalizeToolsMock).toHaveBeenCalledWith(
531+
expect.arrayContaining([expect.objectContaining({ name: "healthy_lookup" })]),
532+
expect.objectContaining({
533+
schemaHookFailureMode: "warn",
534+
}),
535+
);
536+
expect(runtimePlanLogDiagnosticsMock).toHaveBeenCalledWith(
537+
expect.arrayContaining([expect.objectContaining({ name: "healthy_lookup" })]),
538+
expect.objectContaining({
539+
schemaHookFailureMode: "warn",
540+
}),
541+
);
542+
});
543+
499544
it("clamps the caller context token budget to the compaction model", async () => {
500545
resolveContextWindowInfoMock.mockReturnValueOnce({ tokens: 32_000 });
501546

src/agents/embedded-agent-runner/compact.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -829,6 +829,7 @@ async function compactEmbeddedAgentSessionDirectOnce(
829829
workspaceDir: effectiveWorkspace,
830830
modelApi: model.api,
831831
model,
832+
schemaHookFailureMode: "warn" as const,
832833
};
833834
const normalizableToolProjection = filterProviderNormalizableTools(
834835
toolsEnabled ? toolsRaw : [],

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1413,6 +1413,7 @@ export async function runEmbeddedAttempt(
14131413
modelApi: params.model.api,
14141414
model: params.model,
14151415
runtimeHandle: getProviderRuntimeHandle(),
1416+
schemaHookFailureMode: "warn",
14161417
onPreNormalizationSchemaDiagnostics: (diagnostics, sourceTools) =>
14171418
logRuntimeToolSchemaQuarantine({
14181419
diagnostics,
@@ -1509,6 +1510,7 @@ export async function runEmbeddedAttempt(
15091510
modelApi: params.model.api,
15101511
model: params.model,
15111512
runtimeHandle: getProviderRuntimeHandle(),
1513+
schemaHookFailureMode: "warn",
15121514
onPreNormalizationSchemaDiagnostics: (diagnostics, sourceTools) =>
15131515
logRuntimeToolSchemaQuarantine({
15141516
diagnostics,
@@ -1666,6 +1668,7 @@ export async function runEmbeddedAttempt(
16661668
modelApi: params.model.api,
16671669
model: params.model,
16681670
runtimeHandle: getProviderRuntimeHandle(),
1671+
schemaHookFailureMode: "warn",
16691672
});
16701673

16711674
const machineName = await getMachineDisplayName();

src/agents/embedded-agent-runner/tool-schema-runtime.test.ts

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it, vi } from "vitest";
1+
import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const mocks = vi.hoisted(() => ({
44
inspectProviderToolSchemasWithPlugin: vi.fn(),
@@ -21,7 +21,25 @@ vi.mock("./logger.js", () => ({
2121
const { logProviderToolSchemaDiagnostics, normalizeProviderToolSchemas } =
2222
await import("./tool-schema-runtime.js");
2323

24+
function createHostileThrownValue() {
25+
return new Proxy(() => undefined, {
26+
get(target, property, receiver) {
27+
if (property === Symbol.toStringTag) {
28+
throw new Error("tag exploded");
29+
}
30+
return Reflect.get(target, property, receiver);
31+
},
32+
}) as unknown as Error;
33+
}
34+
2435
describe("tool schema runtime diagnostics", () => {
36+
beforeEach(() => {
37+
mocks.log.info.mockReset();
38+
mocks.log.warn.mockReset();
39+
mocks.inspectProviderToolSchemasWithPlugin.mockReset();
40+
mocks.normalizeProviderToolSchemasWithPlugin.mockReset();
41+
});
42+
2543
it("stays quiet when a provider reports no diagnostics", () => {
2644
mocks.inspectProviderToolSchemasWithPlugin.mockReturnValueOnce([]);
2745

@@ -57,6 +75,68 @@ describe("tool schema runtime diagnostics", () => {
5775
);
5876
});
5977

78+
it("keeps the current runtime tools when provider schema normalization throws", () => {
79+
const tools = [{ name: "alpha" }] as never;
80+
mocks.normalizeProviderToolSchemasWithPlugin.mockImplementationOnce(() => {
81+
throw new Error("bad\nnormalizer");
82+
});
83+
84+
expect(
85+
normalizeProviderToolSchemas({
86+
provider: "fuzz-provider\nWARN forged",
87+
tools,
88+
hookFailureMode: "warn",
89+
}),
90+
).toBe(tools);
91+
92+
expect(mocks.log.warn).toHaveBeenCalledWith(
93+
"provider tool schema normalizeToolSchemas hook failed for fuzz-providerWARN forged; keeping current runtime tools: badnormalizer",
94+
{
95+
provider: "fuzz-providerWARN forged",
96+
hookName: "normalizeToolSchemas",
97+
toolCount: 1,
98+
},
99+
);
100+
});
101+
102+
it("does not crash warn-mode normalization for hostile thrown values", () => {
103+
const tools = [{ name: "alpha" }] as never;
104+
mocks.normalizeProviderToolSchemasWithPlugin.mockImplementationOnce(() => {
105+
throw createHostileThrownValue();
106+
});
107+
108+
expect(
109+
normalizeProviderToolSchemas({
110+
provider: "example",
111+
tools,
112+
hookFailureMode: "warn",
113+
}),
114+
).toBe(tools);
115+
116+
expect(mocks.log.warn).toHaveBeenCalledWith(
117+
"provider tool schema normalizeToolSchemas hook failed for example; keeping current runtime tools: [object Function]",
118+
{
119+
provider: "example",
120+
hookName: "normalizeToolSchemas",
121+
toolCount: 1,
122+
},
123+
);
124+
});
125+
126+
it("keeps doctor-style schema normalization strict by default", () => {
127+
mocks.normalizeProviderToolSchemasWithPlugin.mockImplementationOnce(() => {
128+
throw new Error("bad normalizer");
129+
});
130+
131+
expect(() =>
132+
normalizeProviderToolSchemas({
133+
provider: "example",
134+
tools: [{ name: "alpha" }] as never,
135+
}),
136+
).toThrow("bad normalizer");
137+
expect(mocks.log.warn).not.toHaveBeenCalled();
138+
});
139+
60140
it("logs one summarized warning for provider tool schema diagnostics", () => {
61141
mocks.inspectProviderToolSchemasWithPlugin.mockReturnValueOnce([
62142
{ toolName: "alpha", toolIndex: 0, violations: ["one", "two"] },
@@ -84,4 +164,39 @@ describe("tool schema runtime diagnostics", () => {
84164
},
85165
);
86166
});
167+
168+
it("does not crash runtime diagnostics when provider schema inspection throws", () => {
169+
mocks.inspectProviderToolSchemasWithPlugin.mockImplementationOnce(() => {
170+
throw new Error("bad inspector");
171+
});
172+
173+
logProviderToolSchemaDiagnostics({
174+
provider: "example",
175+
tools: [{ name: "alpha" }] as never,
176+
hookFailureMode: "warn",
177+
});
178+
179+
expect(mocks.log.warn).toHaveBeenCalledWith(
180+
"provider tool schema inspectToolSchemas hook failed for example; keeping current runtime tools: bad inspector",
181+
{
182+
provider: "example",
183+
hookName: "inspectToolSchemas",
184+
toolCount: 1,
185+
},
186+
);
187+
});
188+
189+
it("keeps doctor-style schema inspection strict by default", () => {
190+
mocks.inspectProviderToolSchemasWithPlugin.mockImplementationOnce(() => {
191+
throw new Error("bad inspector");
192+
});
193+
194+
expect(() =>
195+
logProviderToolSchemaDiagnostics({
196+
provider: "example",
197+
tools: [{ name: "alpha" }] as never,
198+
}),
199+
).toThrow("bad inspector");
200+
expect(mocks.log.warn).not.toHaveBeenCalled();
201+
});
87202
});

0 commit comments

Comments
 (0)