Merge branch 'worktree-agent-a56fcc6a0c7d751e7' into fix/openai-provider-transport-diagnostics

Author: waleedlatif1

apps/sim/providers/openai/core.response-status.test.ts

@@ -0,0 +1,258 @@
+/**
+ * @vitest-environment node
+ *
+ * `/v1/responses` answers HTTP 200 for generations that did not succeed — `status:
+ * 'failed'` with a populated `error`, or `status: 'incomplete'` with a reason. The
+ * non-streaming path read only `output`, so those reached the user as a success with
+ * empty content and billed tokens, while the trace span independently recorded
+ * `finishReason: 'error'`.
+ *
+ * These cover the status/error gate and pin the `incomplete` policy to the one the
+ * streaming loop already applies, so the two paths cannot silently diverge again.
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+import { executeResponsesProviderRequest } from '@/providers/openai/core'
+import type { ProviderRequest, ProviderResponse } from '@/providers/types'
+
+vi.mock('@/providers', () => ({ MAX_TOOL_ITERATIONS: 5 }))
+
+vi.mock('@/providers/utils', () => ({
+  isFunctionToolCall: () => false,
+  calculateCost: () => ({ input: 0, output: 0, total: 0 }),
+  sumToolCosts: () => 0,
+  enforceStrictSchema: (schema: unknown) => schema,
+  prepareToolExecution: () => ({ toolParams: {}, executionParams: {} }),
+  prepareToolsWithUsageControl: (tools: unknown[]) => ({
+    tools,
+    toolChoice: undefined,
+    forcedTools: [],
+    hasFilteredTools: false,
+  }),
+  trackForcedToolUsage: () => ({ hasUsedForcedTool: false, usedForcedTools: [] }),
+  supportsReasoningEffort: () => false,
+}))
+
+const { mockExecuteProviderTool } = vi.hoisted(() => ({
+  mockExecuteProviderTool: vi.fn(),
+}))
+
+vi.mock('@/providers/runtime-context', () => ({
+  executeProviderTool: mockExecuteProviderTool,
+}))
+
+function jsonResponse(body: unknown) {
+  return {
+    ok: true,
+    status: 200,
+    headers: new Headers(),
+    json: () => Promise.resolve(body),
+  }
+}
+
+const USAGE = { input_tokens: 1, output_tokens: 1, total_tokens: 2 }
+
+function message(text: string) {
+  return {
+    type: 'message',
+    role: 'assistant',
+    content: [{ type: 'output_text', text }],
+  }
+}
+
+function functionCall(args: string) {
+  return { type: 'function_call', call_id: 'call_1', name: 'exa_search', arguments: args }
+}
+
+const COMPLETED_RESPONSE = {
+  id: 'resp_1',
+  status: 'completed',
+  error: null,
+  incomplete_details: null,
+  output: [message('hello')],
+  usage: USAGE,
+}
+
+describe('OpenAI non-streaming response status handling', () => {
+  const logger = { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() } as any
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockExecuteProviderTool.mockResolvedValue({ success: true, output: { results: [] } })
+  })
+
+  function run(fetchMock: unknown, request: Partial<ProviderRequest> = {}) {
+    return executeResponsesProviderRequest(
+      { apiKey: 'k', model: 'gpt-5.5', messages: [{ role: 'user', content: 'hi' }], ...request },
+      {
+        providerId: 'openai',
+        providerLabel: 'OpenAI',
+        modelName: 'gpt-5.5',
+        endpoint: 'https://api.openai.com/v1/responses',
+        headers: { Authorization: 'Bearer k' },
+        logger,
+        fetch: fetchMock as typeof fetch,
+      }
+    )
+  }
+
+  const TOOL_REQUEST: Partial<ProviderRequest> = {
+    tools: [{ id: 'exa_search', name: 'exa_search', description: 'search', params: {} }],
+  }
+
+  it('fails the block on a 200 carrying status "failed", surfacing the API error message', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      jsonResponse({
+        id: 'resp_1',
+        status: 'failed',
+        error: { code: 'server_error', message: 'The model produced an invalid response.' },
+        incomplete_details: null,
+        output: [],
+        usage: USAGE,
+      })
+    )
+
+    await expect(run(fetchMock)).rejects.toThrow('The model produced an invalid response.')
+  })
+
+  it('fails the block when error is populated but status is absent', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      jsonResponse({
+        id: 'resp_1',
+        error: { code: null, message: 'Upstream provider rejected the request.' },
+        incomplete_details: null,
+        output: [],
+        usage: USAGE,
+      })
+    )
+
+    await expect(run(fetchMock)).rejects.toThrow('Upstream provider rejected the request.')
+  })
+
+  /**
+   * Decision, matching `streamResponsesTurn`: an `incomplete` response truncated by
+   * `max_output_tokens` with no tool call is NOT an error — the partial prose is a
+   * usable answer and is returned as the block content.
+   */
+  it('returns the partial content of a max_output_tokens incomplete response instead of failing', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      jsonResponse({
+        id: 'resp_1',
+        status: 'incomplete',
+        error: null,
+        incomplete_details: { reason: 'max_output_tokens' },
+        output: [message('a truncated but usable answer')],
+        usage: USAGE,
+      })
+    )
+
+    const result = (await run(fetchMock)) as ProviderResponse
+    expect(result.content).toBe('a truncated but usable answer')
+  })
+
+  /**
+   * The other half of the same decision: every other incomplete reason is an error,
+   * because the generation stopped for a reason the caller must be told about.
+   */
+  it('fails the block on an incomplete response whose reason is not max_output_tokens', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      jsonResponse({
+        id: 'resp_1',
+        status: 'incomplete',
+        error: null,
+        incomplete_details: { reason: 'content_filter' },
+        output: [message('partial')],
+        usage: USAGE,
+      })
+    )
+
+    await expect(run(fetchMock)).rejects.toThrow(/content_filter/)
+  })
+
+  /**
+   * The confusing-failure case: a truncated `function_call` holds half-written JSON.
+   * Executing it made `parseToolArguments` throw, reporting a tool bug rather than the
+   * truncation that actually happened.
+   */
+  it('does not execute a tool call from a non-completed response', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(
+      jsonResponse({
+        id: 'resp_1',
+        status: 'incomplete',
+        error: null,
+        incomplete_details: { reason: 'max_output_tokens' },
+        output: [functionCall('{"query": "half writ')],
+        usage: USAGE,
+      })
+    )
+
+    await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow(/max_output_tokens/)
+    expect(mockExecuteProviderTool).not.toHaveBeenCalled()
+  })
+
+  it('leaves a healthy completed response entirely unaffected', async () => {
+    const fetchMock = vi.fn().mockResolvedValue(jsonResponse(COMPLETED_RESPONSE))
+
+    const result = (await run(fetchMock)) as ProviderResponse
+    expect(result.content).toBe('hello')
+    expect(result.toolCalls).toBeUndefined()
+    expect(result.tokens?.total).toBe(2)
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+
+  it('still runs the multi-turn tool loop end to end', async () => {
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValueOnce(
+        jsonResponse({
+          id: 'resp_tool',
+          status: 'completed',
+          error: null,
+          incomplete_details: null,
+          output: [functionCall('{"query":"sim"}')],
+          usage: USAGE,
+        })
+      )
+      .mockResolvedValueOnce(jsonResponse(COMPLETED_RESPONSE))
+
+    const result = (await run(fetchMock, TOOL_REQUEST)) as ProviderResponse
+
+    expect(fetchMock).toHaveBeenCalledTimes(2)
+    expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1)
+    expect(result.toolCalls).toHaveLength(1)
+    expect(result.toolCalls?.[0].success).toBe(true)
+    expect(result.content).toBe('hello')
+    expect(result.tokens?.total).toBe(4)
+  })
+
+  /**
+   * The gate sits in `postResponses`, so it must cover continuation turns too — a loop
+   * that starts healthy and fails on turn two must still fail the block.
+   */
+  it('fails the block when a later tool-loop turn comes back failed', async () => {
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValueOnce(
+        jsonResponse({
+          id: 'resp_tool',
+          status: 'completed',
+          error: null,
+          incomplete_details: null,
+          output: [functionCall('{"query":"sim"}')],
+          usage: USAGE,
+        })
+      )
+      .mockResolvedValueOnce(
+        jsonResponse({
+          id: 'resp_2',
+          status: 'failed',
+          error: { code: 'server_error', message: 'Second turn blew up.' },
+          incomplete_details: null,
+          output: [],
+          usage: USAGE,
+        })
+      )
+
+    await expect(run(fetchMock, TOOL_REQUEST)).rejects.toThrow('Second turn blew up.')
+    expect(mockExecuteProviderTool).toHaveBeenCalledTimes(1)
+  })
+})

apps/sim/providers/openai/core.ts

@@ -36,9 +36,11 @@ import {
   createReadableStreamFromResponses,
   extractResponseText,
   extractResponseToolCalls,
+  isMaxOutputTokensIncompleteResponse,
   parseResponsesUsage,
   type ResponsesInputItem,
   type ResponsesToolCall,
+  responseContainsFunctionCall,
   toResponsesToolChoice,
 } from './utils'
 
@@ -113,6 +115,59 @@ function readRetryAfterMs(headers: Headers): number | null {
 type PreparedTools = ReturnType<typeof prepareToolsWithUsageControl>
 type ToolChoice = PreparedTools['toolChoice']
 
+/**
+ * Rejects a `/v1/responses` body that reports a generation which did not succeed.
+ *
+ * The endpoint answers HTTP 200 for failures: `status: 'failed'` with a populated
+ * `error`, or `status: 'incomplete'` with an `incomplete_details.reason`. Reading
+ * only `output` therefore reports a failed generation to the user as a success with
+ * empty content and billed tokens — while `deriveOpenAIFinishReason` independently
+ * records `finishReason: 'error'` on the same span, so the trace and the block
+ * contradict each other.
+ *
+ * The tolerated case is copied from `streamResponsesTurn` and must keep matching it:
+ * an `incomplete` response is accepted only when it was truncated by
+ * `max_output_tokens` AND carries no function call. Truncated prose is still a usable
+ * partial answer, but a truncated `function_call` holds half-written JSON — executing
+ * it makes `parseToolArguments` throw, surfacing a confusing tool failure instead of
+ * the truncation that actually happened.
+ *
+ * A status the API did not send is not asserted against: this path is shared with
+ * Azure OpenAI and any OpenAI-compatible gateway, and inventing a failure for an
+ * absent field would break healthy responses rather than report broken ones.
+ */
+function assertUsableResponse(response: OpenAI.Responses.Response, providerLabel: string): void {
+  if (response.error) {
+    const code = response.error.code ? ` (${response.error.code})` : ''
+    throw new Error(`${providerLabel} generation failed${code}: ${response.error.message}`)
+  }
+
+  if (response.status === 'failed') {
+    throw new Error(
+      `${providerLabel} generation failed, and the API returned no error detail explaining why.`
+    )
+  }
+
+  if (response.status === 'incomplete') {
+    const reason = response.incomplete_details?.reason ?? 'unknown'
+    if (responseContainsFunctionCall(response)) {
+      throw new Error(
+        `${providerLabel} generation stopped before completion (${reason}), truncating a tool call mid-argument. Raise the max output tokens or reduce the tool schema size.`
+      )
+    }
+    if (!isMaxOutputTokensIncompleteResponse(response)) {
+      throw new Error(`${providerLabel} generation stopped before completion: ${reason}.`)
+    }
+    return
+  }
+
+  if (response.status && response.status !== 'completed') {
+    throw new Error(
+      `${providerLabel} returned a response with status "${response.status}", which carries no finished generation.`
+    )
+  }
+}
+
 /**
  * Stable routing key for OpenAI's prompt cache, scoped to one agent block.
  *
@@ -556,13 +611,22 @@ export async function executeResponsesProviderRequest(
       bodyDeadline.abort(new DOMException('response body stalled', 'TimeoutError'))
     }, RESPONSE_BODY_BUDGET_MS)
 
+    let parsed: OpenAI.Responses.Response
     try {
-      return await response.json()
+      parsed = await response.json()
     } catch (error) {
       throw annotateTransportFailure(error, 'reading-response-body', startedAt, responseMeta)
     } finally {
       clearTimeout(timer)
     }
+
+    /**
+     * Asserted here rather than at the call sites so every non-streaming turn — the
+     * first and each tool-loop continuation — is covered by construction, and outside
+     * the transport `try` so a rejected generation is never mistaken for a body stall.
+     */
+    assertUsableResponse(parsed, config.providerLabel)
+    return parsed
   }
 
   const providerStartTime = Date.now()
@@ -721,16 +785,38 @@ export async function executeResponsesProviderRequest(
         content = responseText
       }
 
-      const toolCallsInResponse = extractResponseToolCalls(currentResponse.output)
+      const emittedToolCalls = extractResponseToolCalls(currentResponse.output)
 
       enrichLastModelSegmentFromOpenAIResponse(
         timeSegments,
         currentResponse,
         responseText,
-        toolCallsInResponse,
+        emittedToolCalls,
         { model: request.model }
       )
 
+      /**
+       * Mirrors `toolsExecutable` in the streaming tool loop: a tool call only runs
+       * when it came from a finished generation.
+       *
+       * Unreachable today, and deliberately kept. `assertUsableResponse` already
+       * rejects every status that could carry a tool call from an unfinished
+       * generation — and because both it and `extractResponseToolCalls` key off the
+       * same `function_call` output item, no response can reach here non-completed
+       * with a tool call to run. It stays as the second lock on the invariant: these
+       * two loops diverging on exactly this check is what produced the bug, and a
+       * later relaxation of the assert would otherwise re-open it silently.
+       */
+      const toolsExecutable = !currentResponse.status || currentResponse.status === 'completed'
+      const toolCallsInResponse = toolsExecutable ? emittedToolCalls : []
+
+      if (emittedToolCalls.length > 0 && !toolsExecutable) {
+        logger.warn('Skipping OpenAI tool execution', {
+          status: currentResponse.status,
+          toolCount: emittedToolCalls.length,
+        })
+      }
+
       if (!toolCallsInResponse.length) {
         break
       }