From 82d1fc3fb4a2709811e134bb210f57adcfee5463 Mon Sep 17 00:00:00 2001 From: Jeff Dupont Date: Thu, 15 Oct 2026 13:01:13 -0700 Subject: [PATCH] fix(openai-agents): propagate tool handler errors instead of sending them to the model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @openai/agents' tool() catches an execute() error and returns it to the model as the tool result unless errorFunction is null, so a failing tool handler never failed the run. TESTING.md §1.4 requires the error to propagate. Set errorFunction: null in both the handler and the native graph adapter. The existing test called a mocked tool's execute() directly, which skips the SDK's wrapper. The new tests build the tool with the real tool() and fail without this fix. Co-Authored-By: Claude Opus 5.5 --- packages/openai-agents/agents.md | 2 ++ .../openai-agents/src/__tests__/handler.test.ts | 15 +++++++++++++++ .../src/__tests__/native-graph.test.ts | 10 ++++++++++ packages/openai-agents/src/handler.ts | 2 ++ packages/openai-agents/src/native-graph.ts | 2 ++ 5 files changed, 31 insertions(+) diff --git a/packages/openai-agents/agents.md b/packages/openai-agents/agents.md index 9fe5fe83..55da7a56 100644 --- a/packages/openai-agents/agents.md +++ b/packages/openai-agents/agents.md @@ -82,6 +82,7 @@ tool({ description: toolConfig.description ?? '', strict: false, parameters: toolConfig.parameters as any, // JSON Schema passed directly + errorFunction: null, // propagate tool errors (TESTING.md §1.4) execute: async (args) => { const result = await toolHandlers[name](args); return String(result); @@ -194,4 +195,5 @@ Span names and attributes are described in [Implementation Details → Telemetry - **`result.state.usage` uses camelCase**: `inputTokens`, `outputTokens`, `totalTokens` — not the snake_case used by the OpenAI REST API. The returned `usage` object converts these back to `input_tokens`/`output_tokens` so the client's normalizer works. - **`execute` receives parsed args**: unlike the Responses API where arguments arrive as a JSON string, the Agents SDK parses them before calling `execute`. Do not call `JSON.parse` in the executor. - **`typeof handler !== 'function'` guard**: the execute callback checks `typeof handler !== 'function'` before calling it, so passing a `NativeTool` instance as a `toolHandlers` value will produce a clear `"No handler registered"` error rather than an opaque TypeError. +- **`errorFunction: null` is required on every `tool()`**: leaving it unset makes the SDK catch a tool error and send it to the model as the tool result, so the run carries on. The spec (TESTING.md §1.4) requires the error to propagate. A test that calls a mocked tool's `execute` directly cannot catch this; build the tool with the real `tool()` and call `invoke`. - **No manual loop**: do not add a tool-call loop. The Agents SDK's `run()` handles everything. Adding a manual loop on top would double-execute tools. diff --git a/packages/openai-agents/src/__tests__/handler.test.ts b/packages/openai-agents/src/__tests__/handler.test.ts index e38dffcb..e0bab769 100644 --- a/packages/openai-agents/src/__tests__/handler.test.ts +++ b/packages/openai-agents/src/__tests__/handler.test.ts @@ -417,6 +417,21 @@ describe('createOpenAIAgentHandler', () => { await expect(toolObj.execute({})).rejects.toThrow('tool exploded'); }); + it('tool handler error propagates through the real SDK tool() rather than reaching the model', async () => { + // The mock above calls `execute` directly, which skips the SDK's error wrapper; build the real tool. + const { tool: realTool, RunContext } = await vi.importActual('@openai/agents'); + mockRun.mockResolvedValue(mockRunResult()); + const config = { + ...baseConfig, + tools: { search: { name: 'search', type: 'function' as const, parameters: { type: 'object' } } }, + }; + await createOpenAIAgentHandler()(config as any, 'q', { + search: vi.fn().mockRejectedValue(new Error('tool exploded')), + }); + const sdkTool = realTool(mockTool.mock.calls[0][0]) as any; + await expect(sdkTool.invoke(new RunContext({}), '{}')).rejects.toThrow('tool exploded'); + }); + it('does not call tool() when config.tools is absent, even if toolHandlers is non-empty', async () => { mockRun.mockResolvedValue(mockRunResult()); await createOpenAIAgentHandler()(baseConfig as any, 'q', { search: vi.fn() }); diff --git a/packages/openai-agents/src/__tests__/native-graph.test.ts b/packages/openai-agents/src/__tests__/native-graph.test.ts index 2e591c55..bafc2ff0 100644 --- a/packages/openai-agents/src/__tests__/native-graph.test.ts +++ b/packages/openai-agents/src/__tests__/native-graph.test.ts @@ -181,6 +181,16 @@ describe('toOpenAIAgents', () => { // ── Topology translation ────────────────────────────────────────────────── + it('tool handler error propagates through the real SDK tool() rather than reaching the model', async () => { + const { tool: realTool, RunContext } = await vi.importActual('@openai/agents'); + const graph = makeTwoNodeGraph(); + (graph.root.config as any).tools = { search: { name: 'search', type: 'function', parameters: { type: 'object' } } }; + const search = vi.fn().mockRejectedValue(new Error('tool exploded')); + await toOpenAIAgents(Promise.resolve(graph), { toolHandlers: { search } }).invoke('hi'); + const sdkTool = realTool(mockTool.mock.calls[0][0]) as any; + await expect(sdkTool.invoke(new RunContext({}), '{}')).rejects.toThrow('tool exploded'); + }); + it('creates one Agent per node in a two-node graph', async () => { await toOpenAIAgents(Promise.resolve(makeTwoNodeGraph())).invoke('hi'); expect(mockAgentConstructor).toHaveBeenCalledTimes(2); diff --git a/packages/openai-agents/src/handler.ts b/packages/openai-agents/src/handler.ts index 320a05a5..e823cdc8 100644 --- a/packages/openai-agents/src/handler.ts +++ b/packages/openai-agents/src/handler.ts @@ -467,6 +467,8 @@ const buildAgentTools = (configTools: Record, toolHandlers: Record strict: false, // biome-ignore lint/suspicious/noExplicitAny: OpenAI Agents SDK tool parameters type does not accept Record parameters: toolConfig.parameters as any, + // null, not undefined: the SDK default feeds a tool error back to the model; the spec (TESTING.md §1.4) says it must propagate. + errorFunction: null, execute: async (args) => { const handler = toolHandlers[name] as (...args: unknown[]) => unknown; const result = await handler(args); diff --git a/packages/openai-agents/src/native-graph.ts b/packages/openai-agents/src/native-graph.ts index e6e52c93..68e8afd8 100644 --- a/packages/openai-agents/src/native-graph.ts +++ b/packages/openai-agents/src/native-graph.ts @@ -48,6 +48,8 @@ const buildNodeTools = (node: GraphNode, toolHandlers: Record parameters: toolConfig.parameters as any, + // null, not undefined: the SDK default feeds a tool error back to the model; the spec (TESTING.md §1.4) says it must propagate. + errorFunction: null, execute: async (args) => { const handler = toolHandlers[name]; if (!handler || (handler instanceof Object && 'id' in handler)) {