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 a985fc35d3..b4bdf87c04 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 @@ -523,4 +523,109 @@ describe("OpenAiCodexHandler native tool calls", () => { }), ) }) + + describe("createMessage abort signal", () => { + it("should bridge the external abortSignal into the internal AbortController", async () => { + vi.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") + vi.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + + // The mock transport pauses mid-flight until the request-local signal aborts + const mockCreate = vi.fn().mockImplementation(async (_body: unknown, init?: { signal?: AbortSignal }) => { + return { + async *[Symbol.asyncIterator]() { + yield { type: "response.text.delta", delta: "test" } + await new Promise((resolve) => { + const signal = init?.signal + if (!signal || signal.aborted) { + resolve() + return + } + signal.addEventListener("abort", () => resolve(), { once: true }) + }) + yield { + type: "response.completed", + response: { + id: "resp_1", + status: "completed", + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 1, output_tokens: 1 }, + }, + } + }, + } + }) + Object.assign(handler, { + client: { + responses: { create: mockCreate }, + }, + }) + + const controller = new AbortController() + const stream = handler.createMessage("system", [{ role: "user", content: "hello" }], { + taskId: "t", + abortSignal: controller.signal, + }) + + // Consume the stream (the mock transport pauses mid-flight) + const collected = collectStream(stream) + + // Wait until the request has started; the bridge listener is registered before + // the SDK call, so aborting now lands mid-flight + await vi.waitFor(() => expect(mockCreate).toHaveBeenCalled()) + + // Abort the external signal mid-flight; the bridge must abort the request-local controller + controller.abort() + + const chunks = await collected + expect(chunks.length).toBeGreaterThan(0) + + expect(mockCreate).toHaveBeenCalled() + const createCallArgs = mockCreate.mock.calls[0][1] as { signal?: AbortSignal } + // The captured (request-local) signal passed to the SDK must now be aborted + expect(createCallArgs.signal).toBeDefined() + expect(createCallArgs.signal).toBeInstanceOf(AbortSignal) + expect(createCallArgs.signal?.aborted).toBe(true) + }) + + it("should immediately abort when the external signal is already aborted", async () => { + vi.spyOn(openAiCodexOAuthManager, "getAccessToken").mockResolvedValue("test-token") + vi.spyOn(openAiCodexOAuthManager, "getAccountId").mockResolvedValue("acct_test") + + const mockCreate = vi.fn().mockResolvedValue({ + async *[Symbol.asyncIterator]() { + yield { type: "response.text.delta", delta: "test" } + yield { + type: "response.completed", + response: { + id: "resp_1", + status: "completed", + output: [{ type: "message", content: [{ type: "output_text", text: "test" }] }], + usage: { input_tokens: 1, output_tokens: 1 }, + }, + } + }, + }) + Object.assign(handler, { + client: { + responses: { create: mockCreate }, + }, + }) + + const controller = new AbortController() + controller.abort() // Pre-abort + + const stream = handler.createMessage("system", [{ role: "user", content: "hello" }], { + taskId: "t", + abortSignal: controller.signal, + }) + + // Consume the stream to trigger the request + await collectStream(stream) + + expect(mockCreate).toHaveBeenCalled() + const createCallArgs = mockCreate.mock.calls[0][1] as { signal?: AbortSignal } + // The internal signal should already be aborted since the external one was pre-aborted + expect(createCallArgs.signal?.aborted).toBe(true) + }) + }) }) diff --git a/src/api/providers/__tests__/openai-codex.spec.ts b/src/api/providers/__tests__/openai-codex.spec.ts index a1f8605abe..54eca6134f 100644 --- a/src/api/providers/__tests__/openai-codex.spec.ts +++ b/src/api/providers/__tests__/openai-codex.spec.ts @@ -958,3 +958,95 @@ describe("OpenAiCodexHandler Luna Responses Lite requests", () => { }) }) }) + +describe("OpenAiCodexHandler.completePrompt timeout", () => { + function createHandler() { + const handler = new OpenAiCodexHandler({ apiModelId: "gpt-5.1-codex" }) + 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() + }) + + // timeoutMs <= 0 must not install a timer: the completion runs to the end and the + // request signal never aborts on its own. + it("treats timeoutMs=0 as no timeout", async () => { + const handler = createHandler() + const create = injectStream(handler, [ + { type: "response.output_text.delta", delta: "response" }, + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]) + + await expect(handler.completePrompt("test prompt", { timeoutMs: 0 })).resolves.toBe("response") + + expect(create.mock.calls[0][1].signal.aborted).toBe(false) + }) + + // A timeout and an external abort must cancel the same request: the signal the transport + // receives aborts when either of them fires. + it("merges abortSignal and timeoutMs into the request signal", 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 the listener linking the caller signal is attached. + controller.abort() + return Promise.resolve( + asyncStreamFrom([ + { type: "response.output_text.delta", delta: "feat: half a" }, + { type: "response.completed", response: { id: "r1", status: "completed", output: [] } }, + ]), + ) + }) + Reflect.set(handler, "client", { responses: { create } }) + + await expect( + handler.completePrompt("test prompt", { abortSignal: controller.signal, timeoutMs: 5000 }), + ).rejects.toMatchObject({ name: "AbortError" }) + + expect(signalDuringRequest).toBeInstanceOf(AbortSignal) + expect(signalDuringRequest!.aborted).toBe(true) + }) + + // Native AbortSignal.timeout self-manages its timer, so a transport that rejects on abort + // is all the timeout needs to be observed end to end. + it("rejects with an AbortError when the timeout elapses", async () => { + const handler = createHandler() + let signalDuringRequest: AbortSignal | undefined + + const create = vitest.fn().mockImplementation( + (_body: unknown, options: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + signalDuringRequest = options.signal + // Reject the way the SDK does once the signal it was handed aborts. + options.signal.addEventListener("abort", () => reject(new Error("The operation was aborted")), { + once: true, + }) + }), + ) + Reflect.set(handler, "client", { responses: { create } }) + const mockFetch = vitest.fn() + vitest.stubGlobal("fetch", mockFetch) + + await expect(handler.completePrompt("test prompt", { timeoutMs: 50 })).rejects.toMatchObject({ + name: "AbortError", + }) + + // The timeout must have fired on the request signal, and a timed-out completion is not a + // transport failure, so it must not be replayed over SSE. + expect(signalDuringRequest!.aborted).toBe(true) + expect(mockFetch).not.toHaveBeenCalled() + }) +}) diff --git a/src/api/providers/__tests__/request-config-builder.spec.ts b/src/api/providers/__tests__/request-config-builder.spec.ts index 977b09df6b..f9557a123f 100644 --- a/src/api/providers/__tests__/request-config-builder.spec.ts +++ b/src/api/providers/__tests__/request-config-builder.spec.ts @@ -505,4 +505,44 @@ describe("RequestConfigBuilder", () => { expect(config.maxTokens).toBe(2000) }) }) + + describe("static merge helpers (canonical abort-signal entry points)", () => { + it("returns undefined from mergeAbortSignalAndTimeout when no external signal and no valid timeout", () => { + expect(RequestConfigBuilder.mergeAbortSignalAndTimeout(undefined, undefined)).toBeUndefined() + expect(RequestConfigBuilder.mergeAbortSignalAndTimeout(undefined, 0)).toBeUndefined() + expect(RequestConfigBuilder.mergeAbortSignalAndTimeout(undefined, -5)).toBeUndefined() + }) + + it("returns the external signal directly when no timeout is merged", () => { + const controller = new AbortController() + expect(RequestConfigBuilder.mergeAbortSignalAndTimeout(controller.signal, undefined)).toBe( + controller.signal, + ) + expect(RequestConfigBuilder.mergeAbortSignalAndTimeout(controller.signal, 0)).toBe(controller.signal) + }) + + it("returns the primary signal directly from mergeAbortSignals when there is no secondary", () => { + const controller = new AbortController() + expect(RequestConfigBuilder.mergeAbortSignals(controller.signal)).toBe(controller.signal) + expect(RequestConfigBuilder.mergeAbortSignals(controller.signal, undefined)).toBe(controller.signal) + }) + + it("delegates to AbortSignal.any when two distinct signals are merged", () => { + const a = new AbortController() + const b = new AbortController() + const merged = RequestConfigBuilder.mergeAbortSignals(a.signal, b.signal) + expect(merged.aborted).toBe(false) + b.abort() + expect(merged.aborted).toBe(true) + }) + + it("aborts the merged signal when the primary signal aborts", () => { + const a = new AbortController() + const b = new AbortController() + const merged = RequestConfigBuilder.mergeAbortSignals(a.signal, b.signal) + expect(merged.aborted).toBe(false) + a.abort() + expect(merged.aborted).toBe(true) + }) + }) }) diff --git a/src/api/providers/config-builder/request-config-builder.ts b/src/api/providers/config-builder/request-config-builder.ts index 2201d735bc..a3c1ba7ebb 100644 --- a/src/api/providers/config-builder/request-config-builder.ts +++ b/src/api/providers/config-builder/request-config-builder.ts @@ -163,4 +163,19 @@ export class RequestConfigBuilder @@ -1311,6 +1312,10 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion * from having to be duplicated here. */ async completePrompt(prompt: string, options?: CompletePromptOptions): Promise { + // Merge an optional timeout into the caller's abort signal so a timeout cancels the + // completion the same way an external abort does (timeoutMs <= 0 disables it). + const requestSignal = RequestConfigBuilder.mergeAbortSignalAndTimeout(options?.abortSignal, options?.timeoutMs) + try { const model = this.getModel() @@ -1325,7 +1330,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion // `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, + requestSignal, )) { // Refusals are streamed as text for the chat, but they are not output: the // non-streaming request this replaced read `output_text`, which never carries them. @@ -1338,7 +1343,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion // Both transports end quietly on abort - they break out of their loops rather than // throwing - so returning here would report a cancelled generation as a finished one and // hand the caller whatever partial text had arrived. - if (options?.abortSignal?.aborted) { + if (requestSignal?.aborted) { throw new DOMException("OpenAI Codex completion was aborted", "AbortError") } @@ -1348,7 +1353,7 @@ export class OpenAiCodexHandler extends BaseProvider implements SingleCompletion // reported to telemetry nor relabelled as a completion error. A transport that rejects // on abort reports it in its own words, so it is restated here: callers get one abort // result whether the stream ended quietly or the request threw. - if (options?.abortSignal?.aborted) { + if (requestSignal?.aborted) { throw error instanceof DOMException && error.name === "AbortError" ? error : new DOMException("OpenAI Codex completion was aborted", "AbortError")