Skip to content

Commit 7e578a3

Browse files
committed
fix(providers): fail the block when a 200 from /v1/responses reports a failed generation
The non-streaming Responses path read only `output`, never `status` or `error`. `/v1/responses` answers HTTP 200 for generations that did not succeed, so a failed run reached the user as a SUCCESS with empty content and billed tokens — while `deriveOpenAIFinishReason` independently wrote `finishReason: 'error'` onto the same trace span, leaving the trace and the block contradicting each other. - Reject a 200 carrying `error != null` or `status: 'failed'`, surfacing the API's own message (and error code when present) rather than a generic string. Matches the vendored AI SDK, which throws an APICallError on a 200 carrying `error`. - Pin the `incomplete` policy to the one `streamResponsesTurn` already applies, so the two loops cannot diverge again: tolerate only `max_output_tokens` truncation with no function call and return the partial prose; error on every other reason, and on any incomplete that truncated a tool call. A truncated `function_call` holds half-written JSON, and executing it made `parseToolArguments` throw — reporting a tool bug instead of the truncation that actually happened. - Gate tool execution on a finished generation, mirroring `toolsExecutable` in the streaming loop. The assertion sits in `postResponses` so the first turn and every tool-loop continuation are covered by construction, and outside the transport `try` so a rejected generation is never misreported as a body stall. A status the API did not send is not asserted against: this path is shared with Azure OpenAI and OpenAI-compatible gateways, and inventing a failure for an absent field would break healthy responses rather than report broken ones.
1 parent 10adda7 commit 7e578a3

2 files changed

Lines changed: 347 additions & 3 deletions

File tree

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/**
2+
* @vitest-environment node
3+
*
4+
* `/v1/responses` answers HTTP 200 for generations that did not succeed — `status:
5+
* 'failed'` with a populated `error`, or `status: 'incomplete'` with a reason. The
6+
* non-streaming path read only `output`, so those reached the user as a success with
7+
* empty content and billed tokens, while the trace span independently recorded
8+
* `finishReason: 'error'`.
9+
*
10+
* These cover the status/error gate and pin the `incomplete` policy to the one the
11+
* streaming loop already applies, so the two paths cannot silently diverge again.
12+
*/
13+
import { beforeEach, describe, expect, it, vi } from 'vitest'
14+
import { executeResponsesProviderRequest } from '@/providers/openai/core'
15+
import type { ProviderRequest, ProviderResponse } from '@/providers/types'
16+
17+
vi.mock('@/providers', () => ({ MAX_TOOL_ITERATIONS: 5 }))
18+
19+
vi.mock('@/providers/utils', () => ({
20+
isFunctionToolCall: () => false,
21+
calculateCost: () => ({ input: 0, output: 0, total: 0 }),
22+
sumToolCosts: () => 0,
23+
enforceStrictSchema: (schema: unknown) => schema,
24+
prepareToolExecution: () => ({ toolParams: {}, executionParams: {} }),
25+
prepareToolsWithUsageControl: (tools: unknown[]) => ({
26+
tools,
27+
toolChoice: undefined,
28+
forcedTools: [],
29+
hasFilteredTools: false,
30+
}),
31+
trackForcedToolUsage: () => ({ hasUsedForcedTool: false, usedForcedTools: [] }),
32+
supportsReasoningEffort: () => false,
33+
}))
34+
35+
const { mockExecuteProviderTool } = vi.hoisted(() => ({
36+
mockExecuteProviderTool: vi.fn(),
37+
}))
38+
39+
vi.mock('@/providers/runtime-context', () => ({
40+
executeProviderTool: mockExecuteProviderTool,
41+
}))
42+
43+
function jsonResponse(body: unknown) {
44+
return {
45+
ok: true,
46+
status: 200,
47+
headers: new Headers(),
48+
json: () => Promise.resolve(body),
49+
}
50+
}
51+
52+
const USAGE = { input_tokens: 1, output_tokens: 1, total_tokens: 2 }
53+
54+
function message(text: string) {
55+
return {
56+
type: 'message',
57+
role: 'assistant',
58+
content: [{ type: 'output_text', text }],
59+
}
60+
}
61+
62+
function functionCall(args: string) {
63+
return { type: 'function_call', call_id: 'call_1', name: 'exa_search', arguments: args }
64+
}
65+
66+
const COMPLETED_RESPONSE = {
67+
id: 'resp_1',
68+
status: 'completed',
69+
error: null,
70+
incomplete_details: null,
71+
output: [message('hello')],
72+
usage: USAGE,
73+
}
74+
75+
describe('OpenAI non-streaming response status handling', () => {
76+
const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any
77+
78+
beforeEach(() => {
79+
vi.clearAllMocks()
80+
mockExecuteProviderTool.mockResolvedValue({ success: true, output: { results: [] } })
81+
})
82+
83+
function run(fetchMock: unknown, request: Partial<ProviderRequest> = {}) {
84+
return executeResponsesProviderRequest(
85+
{ apiKey: 'k', model: 'gpt-5.5', messages: [{ role: 'user', content: 'hi' }], ...request },
86+
{
87+
providerId: 'openai',
88+
providerLabel: 'OpenAI',
89+
modelName: 'gpt-5.5',
90+
endpoint: 'https://api.openai.com/v1/responses',
91+
headers: { Authorization: 'Bearer k' },
92+
logger,
93+
fetch: fetchMock as typeof fetch,
94+
}
95+
)
96+
}
97+
98+
const TOOL_REQUEST: Partial<ProviderRequest> = {
99+
tools: [{ id: 'exa_search', name: 'exa_search', description: 'search', params: {} }],
100+
}
101+
102+
it('fails the block on a 200 carrying status "failed", surfacing the API error message', async () => {
103+
const fetchMock = vi.fn().mockResolvedValue(
104+
jsonResponse({
105+
id: 'resp_1',
106+
status: 'failed',
107+
error: { code: 'server_error', message: 'The model produced an invalid response.' },
108+
incomplete_details: null,
109+
output: [],
110+
usage: USAGE,
111+
})
112+
)
113+
114+
await expect(run(fetchMock)).rejects.toThrow('The model produced an invalid response.')
115+
})
116+
117+
it('fails the block when error is populated but status is absent', async () => {
118+
const fetchMock = vi.fn().mockResolvedValue(
119+
jsonResponse({
120+
id: 'resp_1',
121+
error: { code: null, message: 'Upstream provider rejected the request.' },
122+
incomplete_details: null,
123+
output: [],
124+
usage: USAGE,
125+
})
126+
)
127+
128+
await expect(run(fetchMock)).rejects.toThrow('Upstream provider rejected the request.')
129+
})
130+
131+
/**
132+
* Decision, matching `streamResponsesTurn`: an `incomplete` response truncated by
133+
* `max_output_tokens` with no tool call is NOT an error — the partial prose is a
134+
* usable answer and is returned as the block content.
135+
*/
136+
it('returns the partial content of a max_output_tokens incomplete response instead of failing', async () => {
137+
const fetchMock = vi.fn().mockResolvedValue(
138+
jsonResponse({
139+
id: 'resp_1',
140+
status: 'incomplete',
141+
error: null,
142+
incomplete_details: { reason: 'max_output_tokens' },
143+
output: [message('a truncated but usable answer')],
144+
usage: USAGE,
145+
})
146+
)
147+
148+
const result = (await run(fetchMock)) as ProviderResponse
149+
expect(result.content).toBe('a truncated but usable answer')
150+
})
151+
152+
/**
153+
* The other half of the same decision: every other incomplete reason is an error,
154+
* because the generation stopped for a reason the caller must be told about.
155+
*/
156+
it('fails the block on an incomplete response whose reason is not max_output_tokens', async () => {
157+
const fetchMock = vi.fn().mockResolvedValue(
158+
jsonResponse({
159+
id: 'resp_1',
160+
status: 'incomplete',
161+
error: null,
162+
incomplete_details: { reason: 'content_filter' },
163+
output: [message('partial')],
164+
usage: USAGE,
165+
})
166+
)
167+
168+
await expect(run(fetchMock)).rejects.toThrow(/content_filter/)
169+
})
170+
171+
/**
172+
* The confusing-failure case: a truncated `function_call` holds half-written JSON.
173+
* Executing it made `parseToolArguments` throw, reporting a tool bug rather than the
174+
* truncation that actually happened.
175+
*/
176+
it('does not execute a tool call from a non-completed response', async () => {
177+
const fetchMock = vi.fn().mockResolvedValue(
178+
jsonResponse({
179+
id: 'resp_1',
180+
status: 'incomplete',
181+
error: null,
182+
incomplete_details: { reason: 'max_output_tokens' },
183+
output: [functionCall('{"query": "half writ')],
184+
usage: USAGE,
185+
})
186+
)
187+
188+
await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow(/max_output_tokens/)
189+
expect(mockExecuteProviderTool).not.toHaveBeenCalled()
190+
})
191+
192+
it('leaves a healthy completed response entirely unaffected', async () => {
193+
const fetchMock = vi.fn().mockResolvedValue(jsonResponse(COMPLETED_RESPONSE))
194+
195+
const result = (await run(fetchMock)) as ProviderResponse
196+
expect(result.content).toBe('hello')
197+
expect(result.toolCalls).toBeUndefined()
198+
expect(result.tokens?.total).toBe(2)
199+
expect(fetchMock).toHaveBeenCalledTimes(1)
200+
})
201+
202+
it('still runs the multi-turn tool loop end to end', async () => {
203+
const fetchMock = vi
204+
.fn()
205+
.mockResolvedValueOnce(
206+
jsonResponse({
207+
id: 'resp_tool',
208+
status: 'completed',
209+
error: null,
210+
incomplete_details: null,
211+
output: [functionCall('{"query":"sim"}')],
212+
usage: USAGE,
213+
})
214+
)
215+
.mockResolvedValueOnce(jsonResponse(COMPLETED_RESPONSE))
216+
217+
const result = (await run(fetchMock, TOOL_REQUEST)) as ProviderResponse
218+
219+
expect(fetchMock).toHaveBeenCalledTimes(2)
220+
expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1)
221+
expect(result.toolCalls).toHaveLength(1)
222+
expect(result.toolCalls?.[0].success).toBe(true)
223+
expect(result.content).toBe('hello')
224+
expect(result.tokens?.total).toBe(4)
225+
})
226+
227+
/**
228+
* The gate sits in `postResponses`, so it must cover continuation turns too — a loop
229+
* that starts healthy and fails on turn two must still fail the block.
230+
*/
231+
it('fails the block when a later tool-loop turn comes back failed', async () => {
232+
const fetchMock = vi
233+
.fn()
234+
.mockResolvedValueOnce(
235+
jsonResponse({
236+
id: 'resp_tool',
237+
status: 'completed',
238+
error: null,
239+
incomplete_details: null,
240+
output: [functionCall('{"query":"sim"}')],
241+
usage: USAGE,
242+
})
243+
)
244+
.mockResolvedValueOnce(
245+
jsonResponse({
246+
id: 'resp_2',
247+
status: 'failed',
248+
error: { code: 'server_error', message: 'Second turn blew up.' },
249+
incomplete_details: null,
250+
output: [],
251+
usage: USAGE,
252+
})
253+
)
254+
255+
await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow('Second turn blew up.')
256+
expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1)
257+
})
258+
})

apps/sim/providers/openai/core.ts

Lines changed: 89 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,11 @@ import {
3636
createReadableStreamFromResponses,
3737
extractResponseText,
3838
extractResponseToolCalls,
39+
isMaxOutputTokensIncompleteResponse,
3940
parseResponsesUsage,
4041
type ResponsesInputItem,
4142
type ResponsesToolCall,
43+
responseContainsFunctionCall,
4244
toResponsesToolChoice,
4345
} from './utils'
4446

@@ -113,6 +115,59 @@ function readRetryAfterMs(headers: Headers): number | null {
113115
type PreparedTools = ReturnType<typeof prepareToolsWithUsageControl>
114116
type ToolChoice = PreparedTools['toolChoice']
115117

118+
/**
119+
* Rejects a `/v1/responses` body that reports a generation which did not succeed.
120+
*
121+
* The endpoint answers HTTP 200 for failures: `status: 'failed'` with a populated
122+
* `error`, or `status: 'incomplete'` with an `incomplete_details.reason`. Reading
123+
* only `output` therefore reports a failed generation to the user as a success with
124+
* empty content and billed tokens — while `deriveOpenAIFinishReason` independently
125+
* records `finishReason: 'error'` on the same span, so the trace and the block
126+
* contradict each other.
127+
*
128+
* The tolerated case is copied from `streamResponsesTurn` and must keep matching it:
129+
* an `incomplete` response is accepted only when it was truncated by
130+
* `max_output_tokens` AND carries no function call. Truncated prose is still a usable
131+
* partial answer, but a truncated `function_call` holds half-written JSON — executing
132+
* it makes `parseToolArguments` throw, surfacing a confusing tool failure instead of
133+
* the truncation that actually happened.
134+
*
135+
* A status the API did not send is not asserted against: this path is shared with
136+
* Azure OpenAI and any OpenAI-compatible gateway, and inventing a failure for an
137+
* absent field would break healthy responses rather than report broken ones.
138+
*/
139+
function assertUsableResponse(response: OpenAI.Responses.Response, providerLabel: string): void {
140+
if (response.error) {
141+
const code = response.error.code ? ` (${response.error.code})` : ''
142+
throw new Error(`${providerLabel} generation failed${code}: ${response.error.message}`)
143+
}
144+
145+
if (response.status === 'failed') {
146+
throw new Error(
147+
`${providerLabel} generation failed, and the API returned no error detail explaining why.`
148+
)
149+
}
150+
151+
if (response.status === 'incomplete') {
152+
const reason = response.incomplete_details?.reason ?? 'unknown'
153+
if (responseContainsFunctionCall(response)) {
154+
throw new Error(
155+
`${providerLabel} generation stopped before completion (${reason}), truncating a tool call mid-argument. Raise the max output tokens or reduce the tool schema size.`
156+
)
157+
}
158+
if (!isMaxOutputTokensIncompleteResponse(response)) {
159+
throw new Error(`${providerLabel} generation stopped before completion: ${reason}.`)
160+
}
161+
return
162+
}
163+
164+
if (response.status && response.status !== 'completed') {
165+
throw new Error(
166+
`${providerLabel} returned a response with status "${response.status}", which carries no finished generation.`
167+
)
168+
}
169+
}
170+
116171
/**
117172
* Stable routing key for OpenAI's prompt cache, scoped to one agent block.
118173
*
@@ -556,13 +611,22 @@ export async function executeResponsesProviderRequest(
556611
bodyDeadline.abort(new DOMException('response body stalled', 'TimeoutError'))
557612
}, RESPONSE_BODY_BUDGET_MS)
558613

614+
let parsed: OpenAI.Responses.Response
559615
try {
560-
return await response.json()
616+
parsed = await response.json()
561617
} catch (error) {
562618
throw annotateTransportFailure(error, 'reading-response-body', startedAt, responseMeta)
563619
} finally {
564620
clearTimeout(timer)
565621
}
622+
623+
/**
624+
* Asserted here rather than at the call sites so every non-streaming turn — the
625+
* first and each tool-loop continuation — is covered by construction, and outside
626+
* the transport `try` so a rejected generation is never mistaken for a body stall.
627+
*/
628+
assertUsableResponse(parsed, config.providerLabel)
629+
return parsed
566630
}
567631

568632
const providerStartTime = Date.now()
@@ -721,16 +785,38 @@ export async function executeResponsesProviderRequest(
721785
content = responseText
722786
}
723787

724-
const toolCallsInResponse = extractResponseToolCalls(currentResponse.output)
788+
const emittedToolCalls = extractResponseToolCalls(currentResponse.output)
725789

726790
enrichLastModelSegmentFromOpenAIResponse(
727791
timeSegments,
728792
currentResponse,
729793
responseText,
730-
toolCallsInResponse,
794+
emittedToolCalls,
731795
{ model: request.model }
732796
)
733797

798+
/**
799+
* Mirrors `toolsExecutable` in the streaming tool loop: a tool call only runs
800+
* when it came from a finished generation.
801+
*
802+
* Unreachable today, and deliberately kept. `assertUsableResponse` already
803+
* rejects every status that could carry a tool call from an unfinished
804+
* generation — and because both it and `extractResponseToolCalls` key off the
805+
* same `function_call` output item, no response can reach here non-completed
806+
* with a tool call to run. It stays as the second lock on the invariant: these
807+
* two loops diverging on exactly this check is what produced the bug, and a
808+
* later relaxation of the assert would otherwise re-open it silently.
809+
*/
810+
const toolsExecutable = !currentResponse.status || currentResponse.status === 'completed'
811+
const toolCallsInResponse = toolsExecutable ? emittedToolCalls : []
812+
813+
if (emittedToolCalls.length > 0 && !toolsExecutable) {
814+
logger.warn('Skipping OpenAI tool execution', {
815+
status: currentResponse.status,
816+
toolCount: emittedToolCalls.length,
817+
})
818+
}
819+
734820
if (!toolCallsInResponse.length) {
735821
break
736822
}

0 commit comments

Comments
 (0)