diff --git a/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts b/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts index d9fcdcb967..a985fc35d3 100644 --- a/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts +++ b/src/api/providers/__tests__/openai-codex-native-tool-calls.spec.ts @@ -475,15 +475,20 @@ describe("OpenAiCodexHandler native tool calls", () => { vi.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") vi.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + // Completions stream like everything else, so the SDK path is forced to fail and the + // hand-built SSE request is what these assertions inspect. + Reflect.set(handler, "client", { + responses: { create: vi.fn().mockRejectedValue(new Error("SDK unavailable")) }, + }) const mockFetch = vi.fn().mockResolvedValue({ ok: true, - json: vi.fn().mockResolvedValue({ - output: [ - { - type: "message", - content: [{ type: "output_text", text: "done" }], - }, - ], + body: new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode('data: {"type":"response.output_text.delta","delta":"done"}\n\n'), + ) + controller.close() + }, }), }) global.fetch = mockFetch as any diff --git a/src/api/providers/__tests__/openai-codex.spec.ts b/src/api/providers/__tests__/openai-codex.spec.ts index 9a256535c1..1df16f671f 100644 --- a/src/api/providers/__tests__/openai-codex.spec.ts +++ b/src/api/providers/__tests__/openai-codex.spec.ts @@ -247,23 +247,26 @@ describe("OpenAiCodexHandler.completePrompt service tier", () => { it.each<[string, OpenAiCodexServiceTier | undefined, typeof OpenAiCodexServiceTier.Priority | undefined]>([ ["Fast", OpenAiCodexServiceTier.Priority, OpenAiCodexServiceTier.Priority], ["Standard", undefined, undefined], - ])("uses the %s preference in non-streaming requests", async (_mode, configuredTier, expectedTier) => { + ])("uses the %s preference in completion requests", async (_mode, configuredTier, expectedTier) => { const handler = new OpenAiCodexHandler({ apiModelId: "gpt-5.6-sol", ...(configuredTier ? { [OPEN_AI_CODEX_SERVICE_TIER_KEY]: configuredTier } : {}), }) vitest.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") vitest.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") - const mockFetch = vitest.fn().mockResolvedValue({ - ok: true, - json: vitest.fn().mockResolvedValue({ text: "Complete" }), - }) - vitest.stubGlobal("fetch", mockFetch) + const create = vitest.fn().mockResolvedValue( + asyncStreamFrom([ + { type: "response.output_text.delta", delta: "Complete" }, + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]), + ) + Reflect.set(handler, "client", { responses: { create } }) await expect(handler.completePrompt("Hello")).resolves.toBe("Complete") - const body = JSON.parse(mockFetch.mock.calls[0][1].body) - expect(body.stream).toBe(false) + const body = create.mock.calls[0][0] + // The Codex subscription endpoint rejects `stream: false` outright. + expect(body.stream).toBe(true) if (expectedTier) { expect(body[SERVICE_TIER_KEY]).toBe(expectedTier) } else { @@ -272,6 +275,157 @@ describe("OpenAiCodexHandler.completePrompt service tier", () => { }) }) +describe("OpenAiCodexHandler.completePrompt streaming", () => { + function createHandler() { + const handler = new OpenAiCodexHandler({ apiModelId: "gpt-5.6-sol" }) + vitest.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") + vitest.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + return handler + } + + function injectStream(handler: OpenAiCodexHandler, events: unknown[]) { + const create = vitest.fn().mockResolvedValue(asyncStreamFrom(events)) + Reflect.set(handler, "client", { responses: { create } }) + return create + } + + afterEach(() => { + vitest.restoreAllMocks() + vitest.unstubAllGlobals() + }) + + it("joins consecutive text deltas into one string", async () => { + const handler = createHandler() + injectStream(handler, [ + { type: "response.output_text.delta", delta: "feat: " }, + { type: "response.output_text.delta", delta: "add commit messages" }, + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]) + + await expect(handler.completePrompt("Hello")).resolves.toBe("feat: add commit messages") + }) + + // A commit message is written straight into the Source Control box, so reasoning must never + // become part of it. + it("omits reasoning from the completion", async () => { + const handler = createHandler() + injectStream(handler, [ + { type: "response.reasoning_summary_text.delta", delta: "Thinking about the diff" }, + { type: "response.output_text.delta", delta: "fix: correct the parser" }, + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]) + + const result = await handler.completePrompt("Hello") + + expect(result).toBe("fix: correct the parser") + expect(result).not.toContain("Thinking") + }) + + it("omits usage and tool calls from the completion", async () => { + const handler = createHandler() + injectStream(handler, [ + { type: "response.output_text.delta", delta: "chore: tidy" }, + { + type: "response.output_item.done", + item: { type: "function_call", call_id: "call_1", name: "read_file", arguments: "{}" }, + }, + { + type: "response.completed", + response: { + id: "r1", + status: "completed", + output: [], + usage: { input_tokens: 10, output_tokens: 5 }, + }, + }, + ]) + + await expect(handler.completePrompt("Hello")).resolves.toBe("chore: tidy") + }) + + // The SDK path swallows its own errors into the SSE fallback, so an auth failure only reaches + // the retry loop from the fallback - the same shape the streaming Luna retry test relies on. + it("retries once with a refreshed token when the first attempt is unauthorized", async () => { + const handler = createHandler() + const refresh = vitest + .spyOn(openAiCodexOAuthManager, "forceRefreshAccessToken") + .mockResolvedValue("fresh-token") + Reflect.set(handler, "client", { + responses: { create: vitest.fn().mockRejectedValue(new Error("SDK unavailable")) }, + }) + const mockFetch = vitest + .fn() + .mockResolvedValueOnce({ + ok: false, + status: 401, + text: vitest.fn().mockResolvedValue('{"error":{"message":"Codex API invalid token"}}'), + }) + .mockResolvedValueOnce({ + ok: true, + body: new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode( + 'data: {"type":"response.output_text.delta","delta":"docs: update"}\n\n', + ), + ) + controller.close() + }, + }), + }) + vitest.stubGlobal("fetch", mockFetch) + + await expect(handler.completePrompt("Hello")).resolves.toBe("docs: update") + expect(refresh).toHaveBeenCalled() + expect(mockFetch).toHaveBeenCalledTimes(2) + }) + + // The caller's signal is linked to the internal controller rather than passed through, so what + // matters is that aborting the caller's one aborts the signal the request is actually using. + it("passes the caller's abort signal down to the request", async () => { + const handler = createHandler() + const controller = new AbortController() + let signalDuringRequest: AbortSignal | undefined + + const create = vitest.fn().mockImplementation((_body: unknown, options: { signal: AbortSignal }) => { + signalDuringRequest = options.signal + // Abort mid-flight, while `executeRequest`'s listener is still attached. + controller.abort() + return Promise.resolve( + asyncStreamFrom([ + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]), + ) + }) + Reflect.set(handler, "client", { responses: { create } }) + + await handler.completePrompt("Hello", { abortSignal: controller.signal }) + + expect(signalDuringRequest).toBeInstanceOf(AbortSignal) + expect(signalDuringRequest!.aborted).toBe(true) + }) + + it("aborts immediately when the caller's signal is already aborted", async () => { + const handler = createHandler() + const create = injectStream(handler, [ + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]) + + await handler.completePrompt("Hello", { abortSignal: AbortSignal.abort() }) + + expect(create.mock.calls[0][1].signal.aborted).toBe(true) + }) + + it("wraps failures from both transports as a completion error", async () => { + const handler = createHandler() + const create = vitest.fn().mockRejectedValue(new Error("sdk down")) + Reflect.set(handler, "client", { responses: { create } }) + vitest.stubGlobal("fetch", vitest.fn().mockRejectedValue(new Error("network down"))) + + await expect(handler.completePrompt("Hello")).rejects.toThrow(/completionError|network down/) + }) +}) + describe("transformLunaResponsesLiteBody", () => { it("creates the exact Responses Lite body while preserving unrelated fields and reasoning", () => { const tools = [{ type: "function", name: "read_file", parameters: { type: "object" } }] @@ -601,15 +755,20 @@ describe("OpenAiCodexHandler Luna Responses Lite requests", () => { const handler = new OpenAiCodexHandler({ apiModelId: "gpt-5.6-luna", reasoningEffort: "disable" }) vitest.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") vitest.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + // Forcing the SDK path to fail exercises the SSE fallback, which is where the request body + // and the Codex-specific headers are assembled by hand. + Reflect.set(handler, "client", { + responses: { create: vitest.fn().mockRejectedValue(new Error("SDK unavailable")) }, + }) const mockFetch = vitest.fn().mockResolvedValue({ ok: true, - json: vitest.fn().mockResolvedValue({ - output: [ - { - type: "message", - content: [{ type: "output_text", text: "Complete" }], - }, - ], + body: new ReadableStream({ + start(controller) { + controller.enqueue( + new TextEncoder().encode('data: {"type":"response.output_text.delta","delta":"Complete"}\n\n'), + ) + controller.close() + }, }), }) vitest.stubGlobal("fetch", mockFetch) @@ -622,7 +781,7 @@ describe("OpenAiCodexHandler Luna Responses Lite requests", () => { expect(sessionId).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-7[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i) expect(body).toMatchObject({ model: "gpt-5.6-luna", - stream: false, + stream: true, tool_choice: "auto", parallel_tool_calls: false, reasoning: { context: "all_turns" }, diff --git a/src/api/providers/openai-codex.ts b/src/api/providers/openai-codex.ts index e9bc3bbf5d..83570debc1 100644 --- a/src/api/providers/openai-codex.ts +++ b/src/api/providers/openai-codex.ts @@ -217,7 +217,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion metadata?: ApiHandlerCreateMessageMetadata, ): ApiStream { const model = this.getModel() - yield* this.handleResponsesApiMessage(model, systemPrompt, messages, metadata) + yield* this.handleResponsesApiMessage(model, systemPrompt, messages, metadata, metadata?.abortSignal) } private async *handleResponsesApiMessage( @@ -225,6 +225,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion systemPrompt: string, messages: Anthropic.Messages.MessageParam[], metadata?: ApiHandlerCreateMessageMetadata, + abortSignal?: AbortSignal, ): ApiStream { // Reset state for this request this.lastResponseOutput = undefined @@ -274,7 +275,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion // Make the request with retry on auth failure for (let attempt = 0; attempt < 2; attempt++) { try { - yield* this.executeRequest(requestBody, model, accessToken, effectiveSessionId) + yield* this.executeRequest(requestBody, model, accessToken, effectiveSessionId, abortSignal) return } catch (error) { const message = error instanceof Error ? error.message : String(error) @@ -438,10 +439,23 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion model: OpenAiCodexModel, accessToken: string, effectiveSessionId: string, + abortSignal?: AbortSignal, ): ApiStream { // Create AbortController for cancellation this.abortController = new AbortController() + // A caller's signal has to be linked rather than used directly, since both transports below + // abort through `this.abortController`. Without this the signal never reaches the wire. + const abortFromCaller = () => this.abortController?.abort() + + if (abortSignal) { + if (abortSignal.aborted) { + this.abortController.abort() + } else { + abortSignal.addEventListener("abort", abortFromCaller, { once: true }) + } + } + try { // Prefer OpenAI SDK streaming (same approach as openai-native) so event handling // is consistent across providers. @@ -491,6 +505,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion yield* this.makeCodexRequest(requestBody, model, accessToken, effectiveSessionId) } } finally { + abortSignal?.removeEventListener("abort", abortFromCaller) this.abortController = undefined } } @@ -1256,98 +1271,38 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion return this.lastResponseId } + /** + * The Codex subscription endpoint only accepts streaming requests - a body with `stream: false` + * is rejected with `Stream must be set to true` - so a one-shot completion is the streaming + * request with its text chunks joined back together. + * + * Going through `handleResponsesApiMessage` rather than issuing its own request is what keeps + * the OAuth refresh-and-retry, the SDK-then-SSE fallback, the Luna body and the service tier + * from having to be duplicated here. + */ async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { - this.abortController = new AbortController() - try { const model = this.getModel() - // Get access token - const accessToken = await openAiCodexOAuthManager.getAccessToken() - if (!accessToken) { - throw new Error( - t("common:errors.openAiCodex.notAuthenticated", { - defaultValue: - "Not authenticated with OpenAI Codex. Please sign in using the OpenAI Codex OAuth flow.", - }), - ) - } - - const reasoningEffort = this.getReasoningEffort(model) - const serviceTier = getOpenAiCodexServiceTier(this.options) - - const baseRequestBody: any = { - model: model.id, - input: [ - { - role: "user", - content: [{ type: "input_text", text: prompt }], - }, - ], - stream: false, - store: false, - ...(serviceTier ? { [SERVICE_TIER_KEY]: serviceTier } : {}), - ...(reasoningEffort ? { include: ["reasoning.encrypted_content"] } : {}), - } - - if (reasoningEffort) { - baseRequestBody.reasoning = { - effort: reasoningEffort, - summary: "auto" as const, + // Only the answer is wanted. Reasoning is deliberately dropped rather than concatenated: + // callers such as commit-message generation write this straight into the editor. + let text = "" + + for await (const chunk of this.handleResponsesApiMessage( + model, + "", + [{ role: "user", content: prompt }], + // `taskId` is required, and resolves to the same session id this used to send + // directly, so `prompt_cache_key` is unchanged. + { taskId: this.sessionId }, + options?.abortSignal, + )) { + if (chunk.type === "text") { + text += chunk.text } } - const requestBody = - model.id === LUNA_MODEL_ID - ? this.buildLunaRequestBody(baseRequestBody, this.sessionId) - : baseRequestBody - - const url = `${CODEX_API_BASE_URL}/responses` - - // Get ChatGPT account ID for organization subscriptions - const accountId = await openAiCodexOAuthManager.getAccountId() - - // Build headers with required Codex-specific fields - const headers: Record = { - ...this.buildCodexHeaders(model, this.sessionId, accountId), - "Content-Type": "application/json", - Authorization: `Bearer ${accessToken}`, - } - - const response = await fetch(url, { - method: "POST", - headers, - body: JSON.stringify(requestBody), - signal: this.abortController.signal, - }) - - if (!response.ok) { - const errorText = await response.text() - throw new Error( - t("common:errors.openAiCodex.genericError", { status: response.status }) + - (errorText ? `: ${errorText}` : ""), - ) - } - - const responseData = await response.json() - - if (responseData?.output && Array.isArray(responseData.output)) { - for (const outputItem of responseData.output) { - if (outputItem.type === "message" && outputItem.content) { - for (const content of outputItem.content) { - if (content.type === "output_text" && content.text) { - return content.text - } - } - } - } - } - - if (responseData?.text) { - return responseData.text - } - - return "" + return text } catch (error) { const errorModel = this.getModel() const errorMessage = error instanceof Error ? error.message : String(error) @@ -1358,8 +1313,6 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion throw new Error(t("common:errors.openAiCodex.completionError", { message: error.message })) } throw error - } finally { - this.abortController = undefined } } } diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 569c846c29..0fcf33e149 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -396,7 +396,7 @@ }, "api/providers/openai-codex.ts": { "@typescript-eslint/no-explicit-any": { - "count": 35 + "count": 34 } }, "api/providers/openai-native.ts": {