From f53189a0927582a57b33c01cc1df4baa4c032990 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 22:18:19 +0000 Subject: [PATCH 1/2] fix(ai): return Chat Completions refusals and retry reasoning tool turns on gateways Chat Completions refusals leave content null and put the explanation in message.refusal, so the agent returned an empty string as a normal stop, and a schema call spent a repair request on it. Return the refusal text with stop reason "refusal", and skip the structured-output repair after a refusal on either OpenAI wire. #1816 moved api.openai.com tool turns to the Responses API and dropped the reasoning_effort "none" retry. Gateways (Azure OpenAI, OpenRouter, LiteLLM) can still serve gpt-6 and gpt-5.6 over Chat Completions, where function tools are rejected while the model reasons. Restore the retry for the Chat Completions wire only; api.openai.com never takes it. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df9e-e548-71c1-acd1-5cee32990534 Co-authored-by: Amp Co-authored-by: Christian Bager Bach Houmann --- docs/src/content/docs/docs/QuickAddAPI.md | 4 +- src/ai/OpenAIRequest.toolReasoning.test.ts | 181 +++++++++++++++++++++ src/ai/OpenAIRequest.toolTurns.test.ts | 19 +++ src/ai/OpenAIRequest.ts | 47 +++++- src/ai/tools/Agent.test.ts | 14 ++ src/ai/tools/Agent.ts | 3 + src/ai/tools/providerToolMapping.test.ts | 22 +++ src/ai/tools/providerToolMapping.ts | 12 +- 8 files changed, 296 insertions(+), 6 deletions(-) create mode 100644 src/ai/OpenAIRequest.toolReasoning.test.ts diff --git a/docs/src/content/docs/docs/QuickAddAPI.md b/docs/src/content/docs/docs/QuickAddAPI.md index 6c6d06431..723485066 100644 --- a/docs/src/content/docs/docs/QuickAddAPI.md +++ b/docs/src/content/docs/docs/QuickAddAPI.md @@ -797,7 +797,9 @@ These accept only the default `temperature` (omit it from `modelOptions`). The a Agent turns to OpenAI's own API (`https://api.openai.com/v1`) use the Responses API, which lets reasoning models such as GPT-5.6 and GPT-6 call tools with reasoning on. Other OpenAI-compatible -providers use Chat Completions. `modelOptions` keep their Chat Completions names either way: +providers use Chat Completions. If a gateway there (Azure OpenAI, OpenRouter, LiteLLM, ...) rejects +a tool turn because the model reasons by default, QuickAdd retries it once with +`reasoning_effort: "none"`, unless you set `reasoning_effort` yourself. `modelOptions` keep their Chat Completions names either way: QuickAdd sends `reasoning_effort` as `reasoning.effort` and `max_tokens` as `max_output_tokens` on the Responses API, and sends `maxOutputTokens` as `max_completion_tokens` to reasoning models on Chat Completions. diff --git a/src/ai/OpenAIRequest.toolReasoning.test.ts b/src/ai/OpenAIRequest.toolReasoning.test.ts new file mode 100644 index 000000000..b8ba0f833 --- /dev/null +++ b/src/ai/OpenAIRequest.toolReasoning.test.ts @@ -0,0 +1,181 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import type { AIProvider, Model } from "./Provider"; +import type { NormalizedChatRequest } from "./tools/NormalizedTools"; + +import { storeState, mocks, makeApp } from "../../tests/helpers/ai/requestHarness"; + +const { requestUrlMock, noticeMock } = mocks; + +const { chatRequest } = await import("./OpenAIRequest"); + +// A gateway serving OpenAI models over Chat Completions (OpenAI's own endpoint +// uses the Responses API, where this rejection doesn't happen). +const gatewayProvider: AIProvider = { + name: "Gateway", + endpoint: "https://openrouter.ai/api/v1", + kind: "openai", + apiKey: "sk", + models: [], + modelSource: "modelsDev", +}; + +const gpt6: Model = { + name: "gpt-6-sol", + maxTokens: 1_050_000, + maxOutputTokens: 128_000, + supportsTemperature: false, +}; + +// Exact live error for gpt-6-sol with function tools on /v1/chat/completions +// (2026-09-26); gpt-5.6-* and the other gpt-6-* models return the same text. +function toolsWhileReasoningFailure(model = "gpt-6-sol") { + return { + status: 400, + json: { + error: { + message: `Function tools with reasoning_effort are not supported for ${model} in /v1/chat/completions. To use function tools, use /v1/responses or set reasoning_effort to 'none'.`, + type: "invalid_request_error", + param: null, + code: null, + }, + }, + }; +} + +function toolCallSuccess() { + return { + status: 200, + json: Promise.resolve({ + id: "1", + model: "gpt-6-sol", + choices: [ + { + finish_reason: "tool_calls", + index: 0, + message: { + role: "assistant", + content: null, + tool_calls: [ + { + id: "call_1", + type: "function", + function: { name: "get_weather", arguments: '{"city":"Paris"}' }, + }, + ], + }, + }, + ], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, + created: 0, + }), + }; +} + +function toolRequest( + modelParams: Record = {}, +): NormalizedChatRequest { + return { + messages: [{ role: "user", content: "Weather in Paris?" }], + modelParams, + tools: [ + { + name: "get_weather", + description: "Get weather", + parameters: { + type: "object", + properties: { city: { type: "string" } }, + required: ["city"], + }, + }, + ], + toolChoice: "auto", + }; +} + +function sentBody(callIndex: number): Record { + return JSON.parse(requestUrlMock.mock.calls[callIndex][0].body as string); +} + +beforeEach(() => { + requestUrlMock.mockReset(); + noticeMock.mockReset(); + storeState.disableOnlineFeatures = false; +}); + +describe("function tools on reasoning models behind a Chat Completions gateway", () => { + it("retries once with reasoning_effort 'none' when the model rejects tools while reasoning", async () => { + requestUrlMock + .mockReturnValueOnce(Promise.resolve(toolsWhileReasoningFailure())) + .mockReturnValueOnce(Promise.resolve(toolCallSuccess())); + + const res = await chatRequest(makeApp(), "sk", gpt6, gatewayProvider, toolRequest()); + + expect(res.toolCalls?.map((call) => call.name)).toEqual(["get_weather"]); + expect(requestUrlMock).toHaveBeenCalledTimes(2); + expect(sentBody(0).reasoning_effort).toBeUndefined(); + expect(sentBody(1).reasoning_effort).toBe("none"); + expect(sentBody(1).tools).toEqual(sentBody(0).tools); + }); + + it("keeps a reasoning effort the caller chose and surfaces the error", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve(toolsWhileReasoningFailure()), + ); + + await expect( + chatRequest( + makeApp(), + "sk", + gpt6, + gatewayProvider, + toolRequest({ reasoning_effort: "high" }), + ), + ).rejects.toThrow(/reasoning_effort/); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); + + it("does not retry other 400s", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve({ + status: 400, + json: { + error: { + message: "Invalid schema for function 'get_weather'.", + type: "invalid_request_error", + }, + }, + }), + ); + + await expect( + chatRequest(makeApp(), "sk", gpt6, gatewayProvider, toolRequest()), + ).rejects.toThrow(/Invalid schema/); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); + + it("retries at the gateway's Chat Completions URL", async () => { + requestUrlMock + .mockReturnValueOnce(Promise.resolve(toolsWhileReasoningFailure())) + .mockReturnValueOnce(Promise.resolve(toolCallSuccess())); + + await chatRequest(makeApp(), "sk", gpt6, gatewayProvider, toolRequest()); + + expect(requestUrlMock.mock.calls.map((call) => call[0].url)).toEqual([ + "https://openrouter.ai/api/v1/chat/completions", + "https://openrouter.ai/api/v1/chat/completions", + ]); + }); + + it("does not add reasoning_effort to a request without tools", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve(toolsWhileReasoningFailure()), + ); + + await expect( + chatRequest(makeApp(), "sk", gpt6, gatewayProvider, { + messages: [{ role: "user", content: "hi" }], + }), + ).rejects.toThrow(); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/ai/OpenAIRequest.toolTurns.test.ts b/src/ai/OpenAIRequest.toolTurns.test.ts index 8ad421f35..3499e0df4 100644 --- a/src/ai/OpenAIRequest.toolTurns.test.ts +++ b/src/ai/OpenAIRequest.toolTurns.test.ts @@ -150,6 +150,25 @@ describe("OpenAI tool turns use the Responses API on api.openai.com", () => { ).rejects.toThrow(/Invalid schema/); expect(requestUrlMock).toHaveBeenCalledTimes(1); }); + + it("keeps the reasoning_effort retry off the Responses API", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve({ + status: 400, + json: { + error: { + message: "Function tools with reasoning_effort are not supported for gpt-6-luna in /v1/chat/completions. To use function tools, use /v1/responses or set reasoning_effort to 'none'.", + type: "invalid_request_error", + }, + }, + }), + ); + + await expect( + chatRequest(makeApp(), "sk", gpt6, openaiCompatible("https://api.openai.com/v1"), toolRequest()), + ).rejects.toThrow(/reasoning_effort/); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); }); describe("OpenAI-compatible endpoints keep Chat Completions", () => { diff --git a/src/ai/OpenAIRequest.ts b/src/ai/OpenAIRequest.ts index 69b5b2cd8..3ed3498b2 100644 --- a/src/ai/OpenAIRequest.ts +++ b/src/ai/OpenAIRequest.ts @@ -15,7 +15,7 @@ import { } from "./requestLog"; import { preventCursorChange } from "./preventCursorChange"; import { reportError } from "../utils/errorUtils"; -import type { AIProvider, Model } from "./Provider"; +import type { AIProvider, ChatWire, Model } from "./Provider"; import { getChatWire, getProviderKind } from "./Provider"; import type { NormalizedChatRequest } from "./tools/NormalizedTools"; import { @@ -209,10 +209,26 @@ export async function chatRequest( }); try { - const dispatch = (body: Record) => dispatchProviderRequest>({ + const send = (body: Record) => dispatchProviderRequest>({ kind: wire, apiKey, provider: modelProvider, model, body, afterRequest: afterRequestCallback, }); + const dispatch = async (body: Record) => { + try { + return await send(body); + } catch (error) { + const retryBody = toolReasoningRetryBody( + wire, + body, + (error as { message?: string }).message ?? String(error), + ); + if (!retryBody) throw error; + log.logMessage( + `[AI Chat ${requestLogId}] ${model.name} rejected function tools while reasoning; retrying with reasoning_effort "none".`, + ); + return send(retryBody); + } + }; const json = await retrySampling( () => dispatch(body), () => dispatch(buildChatBody(wire, model.name, { @@ -266,6 +282,33 @@ export async function chatRequest( } } +// "Function tools with reasoning_effort are not supported for gpt-6-sol in +// /v1/chat/completions. To use function tools, use /v1/responses or set +// reasoning_effort to 'none'." (verified live 2026-09-26 for the gpt-6 and +// gpt-5.6 families, which reason by default; gpt-5.5 and older default to none). +const TOOLS_NEED_NO_REASONING_RE = + /function tools with reasoning_effort are not supported[\s\S]*reasoning_effort to 'none'/i; + +/** + * The body to retry a Chat Completions tool request with when the model + * rejected function tools because it reasons by default, or null when the + * error is anything else or the caller already chose a reasoning effort. + * Only the Chat Completions wire can hit this: OpenAI's own endpoint uses the + * Responses API, but gateways (Azure OpenAI, OpenRouter, LiteLLM, ...) can + * still serve these models over Chat Completions. + */ +export function toolReasoningRetryBody( + wire: ChatWire, + body: Record, + errorText: string, +): Record | null { + if (wire !== "openai") return null; + if (!Array.isArray(body.tools) || body.tools.length === 0) return null; + if (body.reasoning_effort !== undefined) return null; + if (!TOOLS_NEED_NO_REASONING_RE.test(errorText)) return null; + return { ...body, reasoning_effort: "none" }; +} + async function retrySampling(attempt: () => Promise, retry: () => Promise, context: { sentKeys: ReturnType; model: Model; diff --git a/src/ai/tools/Agent.test.ts b/src/ai/tools/Agent.test.ts index ddf5c405d..d39bcef9d 100644 --- a/src/ai/tools/Agent.test.ts +++ b/src/ai/tools/Agent.test.ts @@ -134,6 +134,20 @@ describe("Agent.generate — structured output", () => { expect(res.object).toEqual({ title: "Fixed" }); expect(chatRequestMock).toHaveBeenCalledTimes(2); }); + + it("does not spend a repair request on a refusal", async () => { + chatRequestMock.mockResolvedValueOnce( + turnResponse({ content: "I can't help with that.", stopReason: "refusal", normalizedStopReason: "other" }), + ); + const agent = makeAgent(); + const res = await agent.generate({ + prompt: "extract", + schema: { type: "object", properties: { title: { type: "string" } }, required: ["title"] }, + }); + expect(res.object).toBeUndefined(); + expect(res.text).toBe("I can't help with that."); + expect(chatRequestMock).toHaveBeenCalledTimes(1); + }); }); describe("Agent construction validation", () => { diff --git a/src/ai/tools/Agent.ts b/src/ai/tools/Agent.ts index a92289345..7ee401c8d 100644 --- a/src/ai/tools/Agent.ts +++ b/src/ai/tools/Agent.ts @@ -445,9 +445,12 @@ export class Agent { // new outbound call, so DON'T do it when the loop ended in a terminal // non-success state (online disabled mid-run → "aborted", or "context-overflow") // or online features are now off — that would bypass the mid-run stop. + // A refusal is also terminal: the model declined, and re-asking for JSON + // would only spend another request on the same refusal. if ( loop.finishReason === "aborted" || loop.finishReason === "context-overflow" || + loop.finalTurn.rawStopReason === "refusal" || settingsStore.getState().disableOnlineFeatures ) { return undefined; diff --git a/src/ai/tools/providerToolMapping.test.ts b/src/ai/tools/providerToolMapping.test.ts index 84b7878c5..803004046 100644 --- a/src/ai/tools/providerToolMapping.test.ts +++ b/src/ai/tools/providerToolMapping.test.ts @@ -276,6 +276,28 @@ describe("injectStrictObjectSchema", () => { }); }); +describe("OpenAI Chat Completions refusals", () => { + it("returns the refusal's explanation instead of an empty answer", () => { + const parsed = parseChatResponse("openai", { + choices: [ + { finish_reason: "stop", message: { role: "assistant", content: null, refusal: "I can't help with that." } }, + ], + }); + expect(parsed.content).toBe("I can't help with that."); + expect(parsed.normalizedStopReason).toBe("other"); + expect(parsed.rawStopReason).toBe("refusal"); + }); + + it("an ordinary answer with a null refusal field is a normal stop", () => { + const parsed = parseChatResponse("openai", { + choices: [{ finish_reason: "stop", message: { role: "assistant", content: "Hi", refusal: null } }], + }); + expect(parsed.content).toBe("Hi"); + expect(parsed.normalizedStopReason).toBe("stop"); + expect(parsed.rawStopReason).toBe("stop"); + }); +}); + describe("OpenAI Responses mapping (api.openai.com)", () => { it("minimal body: model, input, stateless storage, encrypted reasoning", () => { const body = buildChatBody("openai-responses", "gpt-6-luna", { diff --git a/src/ai/tools/providerToolMapping.ts b/src/ai/tools/providerToolMapping.ts index 93a112ef4..f8e8068eb 100644 --- a/src/ai/tools/providerToolMapping.ts +++ b/src/ai/tools/providerToolMapping.ts @@ -180,18 +180,24 @@ function parseOpenAIResponse(json: Record): ParsedChatResult { const toolCalls = rawCalls.map((tc) => parseOpenAIToolCall(tc)); const finish = String(choice.finish_reason ?? ""); const usage = (json.usage as Record) ?? {}; + // A safety refusal leaves content null and explains itself in `refusal`. + // Return the explanation rather than an empty answer, and don't call it a + // normal stop (matches the Responses API parser). + const text = typeof msg.content === "string" ? msg.content : ""; + const refusal = typeof msg.refusal === "string" ? msg.refusal : ""; + const refused = !text && refusal.length > 0 && toolCalls.length === 0; return { - content: (msg.content as string) ?? "", + content: refused ? refusal : text, toolCalls, normalizedStopReason: toolCalls.length > 0 || finish === "tool_calls" ? "tool_calls" : finish === "length" ? "length" - : finish === "stop" + : finish === "stop" && !refused ? "stop" : "other", - rawStopReason: finish, + rawStopReason: refused ? "refusal" : finish, usage: { promptTokens: usage.prompt_tokens ?? 0, completionTokens: usage.completion_tokens ?? 0, From cded30f2519a6a7c63df805403d1ea95c3ceb522 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 22:27:02 +0000 Subject: [PATCH 2/2] fix(ai): never return a refusal as the structured object Check for a refusal before parsing, so refusal text that happens to be valid JSON can't become result.object. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df9e-e548-71c1-acd1-5cee32990534 Co-authored-by: Amp Co-authored-by: Christian Bager Bach Houmann --- src/ai/tools/Agent.test.ts | 13 +++++++++++++ src/ai/tools/Agent.ts | 8 +++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/ai/tools/Agent.test.ts b/src/ai/tools/Agent.test.ts index d39bcef9d..fcf72b603 100644 --- a/src/ai/tools/Agent.test.ts +++ b/src/ai/tools/Agent.test.ts @@ -148,6 +148,19 @@ describe("Agent.generate — structured output", () => { expect(res.text).toBe("I can't help with that."); expect(chatRequestMock).toHaveBeenCalledTimes(1); }); + + it("never returns a refusal as the structured object, even when it parses", async () => { + chatRequestMock.mockResolvedValueOnce( + turnResponse({ content: '{"title":"refused"}', stopReason: "refusal", normalizedStopReason: "other" }), + ); + const agent = makeAgent(); + const res = await agent.generate({ + prompt: "extract", + schema: { type: "object", properties: { title: { type: "string" } }, required: ["title"] }, + }); + expect(res.object).toBeUndefined(); + expect(chatRequestMock).toHaveBeenCalledTimes(1); + }); }); describe("Agent construction validation", () => { diff --git a/src/ai/tools/Agent.ts b/src/ai/tools/Agent.ts index 7ee401c8d..d5176d174 100644 --- a/src/ai/tools/Agent.ts +++ b/src/ai/tools/Agent.ts @@ -438,6 +438,11 @@ export class Agent { schema: JSONSchema, restoreCursor: () => void, ): Promise { + // A refusal is terminal: the model declined, so its text is never the + // structured result (even if it happens to parse), and re-asking for JSON + // would only spend another request on the same refusal. + if (loop.finalTurn.rawStopReason === "refusal") return undefined; + const first = parseStructured(loop.finalTurn.content, schema); if (first.ok) return first.value; @@ -445,12 +450,9 @@ export class Agent { // new outbound call, so DON'T do it when the loop ended in a terminal // non-success state (online disabled mid-run → "aborted", or "context-overflow") // or online features are now off — that would bypass the mid-run stop. - // A refusal is also terminal: the model declined, and re-asking for JSON - // would only spend another request on the same refusal. if ( loop.finishReason === "aborted" || loop.finishReason === "context-overflow" || - loop.finalTurn.rawStopReason === "refusal" || settingsStore.getState().disableOnlineFeatures ) { return undefined;