feat(api): abort signal support for openai, openai-compatible base, zai, kimi-code (round 2) by easonLiangWorldedtech · Pull Request #1311 · Zoo-Code-Org/Zoo-Code

easonLiangWorldedtech · 2026-08-20T21:20:34Z

Related GitHub Issue

Closes #404 — the OpenAI-compatible cancellation objective, for the OpenAI provider family implemented in this PR (openai, base-openai-compatible-provider and its inheritors, zai, kimi-code).

This PR covers only the OpenAI-family portion of the broader provider sweep; the remaining scope lands via sibling PRs in this series:

Part of feat(api): abort signal pass-through for simple providers #616 — openai-codex feat(api): abort signal support for openai-codex (completePrompt + createMessage) #1290; openai-native + openai-compatible feat(api): abort signal support for openai-native and openai-compatible (completePrompt + createMessage) #1291; anthropic + anthropic-vertex + xai + minimax feat(api): abort signal support for anthropic, anthropic-vertex, xai, minimax #1293; unbound + vercel-ai-gateway + zoo-gateway feat(api): abort signal support for opencode-go, unbound, vercel-ai-gateway, zoo-gateway #1295; native-ollama feat(api): abort signal support for native-ollama (completePrompt + createMessage) #1299; vscode-lm feat(api): abort signal support for vscode-lm (completePrompt + createMessage) #1300; gemini + mistral + lite-llm feat(api): abort signal support for gemini, mistral, lite-llm (completePrompt + createMessage) #1303; lm-studio + qwen-code feat(api): abort-signal wiring for lm-studio and qwen-code (round 2) #1309; poe + requesty feat(api): abort signal support for poe (completePrompt + createMessage) #1535/feat(api): abort signal support for requesty (completePrompt + shared helpers) #1537/feat(api): abort signal support for requesty (createMessage + kill tests) #1538; openrouter feat(api): abort signal support for openrouter (completePrompt + createMessage) #1545 (bedrock already landed on main via feat(api): abort signal support for bedrock (completePrompt + createMessage) #1292).
Part of feat(api): abort signal bridging for openai-native, openai-codex, bedrock, native-ollama #617 — openai-native and openai-codex via feat(api): abort signal support for openai-native and openai-compatible (completePrompt + createMessage) #1291 and feat(api): abort signal support for openai-codex (completePrompt + createMessage) #1290; bedrock already landed on main via feat(api): abort signal support for bedrock (completePrompt + createMessage) #1292; native-ollama via feat(api): abort signal support for native-ollama (completePrompt + createMessage) #1299.
Part of feat(api): abort signal for vscode-lm, gemini, mistral (SDK-specific quirks) #618 — vscode-lm via feat(api): abort signal support for vscode-lm (completePrompt + createMessage) #1300; gemini and mistral via feat(api): abort signal support for gemini, mistral, lite-llm (completePrompt + createMessage) #1303.

Description

Round 2 of the abort-signal series: wires request-cancellation signals through the OpenAI family of providers.

openai.ts: all five client.chat.completions.create sites (createMessage streaming + non-streaming, O3-family streaming + non-streaming, completePrompt) build their request config through RequestConfigBuilder, adopted from the start of this PR — the Azure AI Inference path option and the abort signal compose in one builder (setOption("path", ...) + setAbortSignal). Every catch now normalizes abort failures to the Task.ts contract shape (name === "AbortError", message ending in aborted) via an abort-aware handleOpenAIRequestError, while non-abort errors keep the existing provider-prefix wrap. Both streaming loops (the main createMessage path and the O3-family handleStreamResponse) gain the loop-defense contract: a top-of-loop break stops processing buffered chunks that arrived after the caller's abort, and a post-loop check surfaces the Task.ts abort contract when the stream would otherwise end normally on an already-aborted signal.
base-openai-compatible-provider.ts: the shared createMessage / createStream / completePrompt path adopted RequestConfigBuilder for signal forwarding and gains the exported abort-aware error helper handleOpenAIRequestError (reused by zai.ts). Subclasses that do not override these methods (fireworks, sambanova, baseten) inherit the wiring, including the streaming loop's top-of-loop break and post-loop abort check.
zai.ts: audit finding fixed — the GLM thinking path in createStream no longer drops requestOptions; the thinking path and the glm-5.3 completePrompt path forward a merged signal (external signal + timeoutMs via mergeAbortSignalAndTimeout).
kimi-code.ts: completePrompt no longer drops CompletePromptOptions — options are forwarded on both the initial call and the 401 OAuth retry. createMessage inherits the openai.ts wiring via metadata passthrough.

Design notes:

CompletePromptOptions is not assignable to ApiHandlerCreateMessageMetadata (required taskId) — gap G7 — so completePrompt paths use setOption("signal", mergeAbortSignalAndTimeout(...)) instead of setAbortSignal(metadata).
Gap G5 (zero timeout must mean "no explicit timeout"): mergeAbortSignalAndTimeout treats timeoutMs <= 0 as no timeout internally, so a timeoutMs: 0 call site passes no signal rather than a timeout that would abort immediately.
The OpenAI SDK v5 RequestOptions type does not satisfy the builder's RequestConfigOptionsBase constraint (its headers/signal shapes differ), so each provider declares a minimal local OpenAiRequestConfig shape as the builder generic parameter.
Each call builds a fresh request-local config (no class-field abort controller), and the per-entry-point throwIfAborted guard rejects before any network I/O when the signal is already aborted.

This branch is STACKED on #1288: the foundation commit e61feb13e (generic RequestConfigBuilder, mergeAbortSignalAndTimeout, mergeAbortSignals, throwIfAborted) rides inside by design.

Test Procedure

pnpm --dir src exec vitest run api/providers/__tests__/openai.spec.ts api/providers/__tests__/base-openai-compatible-provider.spec.ts api/providers/__tests__/zai.spec.ts api/providers/__tests__/kimi-code.spec.ts — all green. New per-provider "abort signal wiring" suites cover: signal identity at every create site (including Azure path composition), signal + timeout merging, the timeoutMs: 0 guard, pre-aborted rejection before any request, SDK APIUserAbortError and fetch-level AbortError normalization to the Task.ts contract shape, and non-abort provider-prefix wrap regression. Deferred-chunk kill tests (2 in openai.spec.ts incl. the O3-family path, 1 in the base spec) prove the loop defense structurally: the second chunk is released only after the abort, so the top-of-loop break is the only thing that prevents the leak (asserted via a leaked collection), and captureError proves the post-loop check rejects with the contract message rather than a normal stream end.
100% changed-line coverage for the four provider files, measured with vitest run <specs> --coverage (v8/lcov) and cross-referenced against the git diff added lines.
pnpm --dir src exec tsc --noEmit — exit 0.
pnpm --dir src exec eslint --prune-suppressions --max-warnings=0 <changed files> — zero warnings; one stale suppression entry pruned (kimi-code.spec.ts @typescript-eslint/no-explicit-any 1 -> 0, the spec rewrite removed the only as-any cast); no suppression count increased.

Pre-Submission Checklist

Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
Scope: My changes are focused on the linked issue (abort-signal wiring for the OpenAI provider family only; the four providers and their specs).
Self-Review: I have performed a thorough self-review of my code.
Testing: New and/or updated tests have been added to cover my changes (if applicable).
Visual Snapshot (UI changes only): N/A — no UI changes.
Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Documentation Updates

No documentation updates are required.
Yes, documentation updates are required. (Please describe what needs to be updated or link to a PR in the docs repository.)

Gate evidence (local, pre-push at `393b516`)

align check (zdt, unit = this PR's provider set): the 4 streaming-loop ERRORs on this unit's own loops (openai.ts + base-openai-compatible-provider.ts: missing top-of-loop check / missing post-loop check) are cleared by the loop defense above. Residual ERRORs on nanogpt.ts are main-side drift, not this unit: the file is byte-identical to upstream/main (introduced by main fix(nanogpt): preserve optional tool parameters #1590 after this branch's fork point) and is outside the unit's scope. The 8 other series units share the same loop-defense gap; series-wide application is a separate maintainer decision.
split contract / verify (zdt, kind=new, design-issue [BUG] Stop does not work on OpenAI Compatible API Provider #404): no duplicate symbols, no foreign content. The only violation is budget-hard (1859 a+d vs the 1000 cap) — pre-existing unit scope already accepted through prior review cycles; re-splitting would re-cut already-merged round-1 units.
mutation preflight (local Stryker on the unit delta): 185 valid / 184 killed / 1 timeout / 0 survived / 0 noCoverage. The single timeout is the zai.ts setOption("signal", ...) StringLiteral mutant, a known flake class (the mutant drops the signal, so the abort test can only fail through the vitest timeout).

Additional Notes

Part of the abort-signal series (round 2). Builds on #674, #901, #1008, and #1288. Addresses #404.

In-scope justification for the two non-#404-looking items

Root package.json devDependency vitest 4.1.11: mutation-gate infrastructure — the changed-code mutation gate (Stryker via zdt) must resolve a vitest binary at the repository root to run this PR's gate (commit 0703c58), and the 4.1.11 pin is the CI dependency-review advisory bump (commit 4238bd1). It is the mechanism by which this PR's [BUG] Stop does not work on OpenAI Compatible API Provider #404 changes are gate-verified, not a functional change.
MiniMax base_resp re-typing: the hunk is inside this PR's new abort-aware streaming loop in base-openai-compatible-provider.ts — the same change adds the top-of-loop if (signal?.aborted) break defense and the try wrapper that normalizes abort errors raised during stream iteration ([BUG] Stop does not work on OpenAI Compatible API Provider #404 round 2, commits 393b516 and 35c95ea). The previous chunk as any read was re-typed with an unknown guard in that restructure; behavior is preserved (same status_code/status_msg semantics and error message shape), covered by the base_resp stream-error path test (commit 217f120).

Updated 2026-10-11 (head `6e46bc78c`)

Merged current upstream/main (995167eed) (merge commit 6e46bc78c), clearing the has-conflicts state.
One conflict resolved: base-openai-compatible-provider.spec.ts — main prettier-collapsed the custom-model assertion to one line; this branch extends it to pin the two-argument create(body, requestConfig) call shape (the abort wiring's contract). The two-argument assertion is kept (it doesn't fit one line, so main's reformat doesn't apply to it).

Verification at this head: affected provider/fetcher/util specs 512/514 passed — the 2 failures are strict: true default-path cases and are a local-environment artifact: this worktree's @roo-code/types resolves through a junction to a donor tree that predates the strict-tool-schemas export, so DEFAULT_OPEN_AI_STRICT_TOOL_SCHEMAS is undefined at runtime there. The full strict-schema suite (8 tests) passes on a fresh-install worktree against the same main merge, and the merged production strict path is byte-identical to main's. ESLint --max-warnings=0 clean on the resolved file; tsc shows only stale-donor missing-export artifacts (0 merge-caused errors in touched files); Prettier clean; src/eslint-suppressions.json strictly below main (this PR removes one entry and lowers one count). No new as any, no changeset/CHANGELOG files.

Updated this round (2026-10-13)

Merged current upstream main into the branch to clear the dirty state. Main's model-cache redesign (handler.fetchModels(options, signal) + getModelCacheScope) superseded this branch's signal threading through the modelCache fetcher-dispatch switch, so fetchers/modelCache.ts and its spec took main's versions wholesale — the kimi signal forwarding the branch's two modelCache spec tests pinned now lives at the fetcher level (fetchers/kimi-code.spec.ts) and at the call site (options.signal feeds the pre-abort fast-fail). In kimi-code.ts both sides' imports were merged and the discovery call site combined: main's handler argument plus the branch's options.signal.

Verification at this head: 512/512 green across the eleven touched specs; tsc --noEmit → 0 errors; per-file ESLint (--max-warnings=0) clean; prettier --check clean on touched files.

Updated this round (2026-10-13, review follow-up)

Discovery cancellation no longer consumes the single attempt: the catch in prepareRequest resets modelDiscoveryAttempted only for cancellations (isRequestAborted against the caller's signal), so a later request on the same handler retries discovery instead of keeping fallback metadata; ordinary discovery failures keep the one-attempt behavior.
New test aborts during getModels and asserts a second request rediscovers (negative control verified; order-independent fetch stub).

Verification at this head: 25/25 green in kimi-code.spec.ts; tsc --noEmit → 0 errors; per-file ESLint (--max-warnings=0) clean; prettier --check clean on touched files.

Design contract: cancellation stops the underlying OpenAI-compatible HTTP request (Issue #404)

Registered evidence for the pre-merge Linked-Issues check at head 7f085ab:

createMessage hands the caller's metadata.abortSignal to the inherited OpenAI SDK request as chat.completions.create(params, { signal }) — the SDK passes RequestOptions.signal straight to fetch, so a cancellation aborts the in-flight HTTP request at the transport level, not merely stream consumption. Pinned by the test "forwards the metadata abort signal to the inherited OpenAI SDK request", which asserts the SDK call receives the exact caller signal instance.
completePrompt passes mergeAbortSignalAndTimeout(options.abortSignal, options.timeoutMs) as the SDK request's signal (RequestConfigBuilder .setOption("signal", ...) in the shared OpenAI wiring), pinned by "forwards completePrompt abort options and timeoutMs through the override on both 401 retry attempts".
Pre-flight guards (throwIfAborted before token/discovery work and before the SDK call, plus the post-prepare re-check) ensure a cancelled request never issues a POST — pinned by "rejects before any request when the createMessage abort signal is already aborted" and "stops before the completion request when the signal aborts during model preparation".
Post-loop abort defenses reject any content that raced the cancellation with the contract AbortError (name === "AbortError").

Stryker directive removal (equivalence notes)

Five directives removed (base-openai-compatible-provider.ts 3, openai.ts 2):

ConditionalExpression,LogicalOperator on the baseResp object guard: equivalent for all JSON-reachable baseResponse values (only null/object/number/string/boolean reach it; each mutant agrees with the original on every one).
ConditionalExpression on baseRespStatusCode !== 0: a truthy statusCode is necessarily !== 0, so deleting the check cannot change the branch outcome.
ConditionalExpression,EqualityOperator on the tool_calls guard: with an empty activeToolCallIds the guard is the only bail; the mutants keep identical behavior for the covered finish reasons.
LogicalOperator (&& -> ||) on the strict-tool-schemas option read (x2): an unset option makes && yield undefined, which the parameter default in convertToolsForOpenAI turns back into the same strict value as ?? - equivalent.

Timeout guard hardening (helper port)

Port of the #1300 CodeRabbit finding to the shared mergeAbortSignalAndTimeout helper: non-finite timeoutMs (Infinity) now means "no timeout" instead of throwing ERR_OUT_OF_RANGE from AbortSignal.timeout, and finite delays are capped at 2^31-1 ms (Node clamps larger delays to 1ms, which aborted requests almost immediately). Regression tests added; negative controls fail exactly the two new tests. Identical helper fix lands in every unit of the abort series (merge-order aware); canonical copy in #1293.

Fractional-timeout rounding wave

Follow-up CR round: Node validates AbortSignal.timeout delays as uint32, so finite fractions (1500.5, 0.5) threw ERR_OUT_OF_RANGE like Infinity did. mergeAbortSignalAndTimeout now rounds delays up (a positive fraction never becomes 0 ms) before capping at 2^31-1 ms; regression test mergeAbortSignalAndTimeout(undefined, 1.5). Identical helper patch in every unit of the series (merge-order aware, canonical in #1293).

Assertion strengthening (rounding wave follow-up)

The fractional-timeout test now spies AbortSignal.timeout and pins the received delay (2 for 1.5, 1 for 0.5, never 0) instead of only asserting no-throw plus eventual abort; a round-down mutation (Math.ceil -> Math.floor) fails exactly this test. Identical patch in every unit of the series (canonical in #1293).

Pre-merge checks: Kimi discovery cancellation (`6088f40`)

Linked Issues error fixed: KimiCodeHandler.fetchModels now forwards cancellation (explicit model-cache signal, else options.signal) to getKimiCodeModels, so cancellation during handler-driven discovery aborts the in-flight discovery HTTP request (Issue #404). Regression Evidence warning fixed: the discovery-retry test asserts getModels receives the caller signal, and rejected-Promise request-creation cases (mockRejectedValueOnce(new APIUserAbortError())) added for the awaited create() paths in base-openai-compatible-provider and zai - the sync-throw-only tests passed even with the await removed. Negative controls: reverting the forwarding, dropping the signal, and removing each await each fail exactly one test.

Lifecycle Resource Cleanup warning (optional per CR): the discovery half is fixed above; the OAuth-refresh half (threading the signal through resolveAccessToken -> kimiCodeOAuthManager -> refreshKimiCodeAccessToken -> postForm) lives in src/integrations/kimi-code/oauth.ts, outside this unit's diff - registered as a follow-up for the OAuth owner.

Updated 2026-10-22 (head `fb2b47870`)

Merged current upstream/main (05d01f33d), clearing has-conflicts.
Note for the series: this unit owns the shared BaseOpenAICompatibleProvider + openai-family abort contract. feat(api): abort signal support for openai-native and openai-compatible (completePrompt + createMessage) #1291's openai-compatible.ts half was dropped because upstream/main deleted the old OpenAICompatibleHandler class (deps(knip): enable production mode and remove dead code #527); that surface lives here.
Local proof on the refreshed head: ESLint clean on the changed files, tsc --noEmit clean, vitest run over 11 provider/base/test-utils specs → 523/523 pass.

…ssion tests Add a fast-fail throwIfAborted guard to the shared abort-signal utilities and regression tests for the CompletePromptOptions interface (added by Zoo-Code-Org#901).

…ai, kimi-code (round 2) Round 2 of the abort-signal series: wires request-cancellation signals through the OpenAI family of providers (addresses Zoo-Code-Org#404). - openai.ts: all five client.chat.completions.create sites (createMessage streaming + non-streaming, O3-family streaming + non-streaming, completePrompt) build their request config through RequestConfigBuilder; the Azure AI Inference path option and the abort signal compose in one builder (setOption("path", ...) + setAbortSignal). Every catch normalizes abort failures to the Task.ts contract shape (name === "AbortError", message ending in "aborted") via an abort-aware handleOpenAIRequestError; non-abort errors keep the existing provider-prefix wrap. - base-openai-compatible-provider.ts: the shared createMessage / createStream / completePrompt path adopts RequestConfigBuilder for signal forwarding and gains the exported abort-aware error helper handleOpenAIRequestError (reused by zai.ts); subclasses that do not override these methods inherit the wiring. - zai.ts: audit finding fixed - the GLM thinking path in createStream no longer drops requestOptions; the thinking path and the glm-5.3 completePrompt path forward a merged signal (external signal + timeoutMs via mergeAbortSignalAndTimeout). - kimi-code.ts: completePrompt no longer drops CompletePromptOptions - options are forwarded on both the initial call and the 401 OAuth retry. - Design notes: CompletePromptOptions is not ApiHandlerCreateMessageMetadata (required taskId, gap G7), so completePrompt paths use setOption("signal", mergeAbortSignalAndTimeout(...)) instead of setAbortSignal(metadata); gap G5 - mergeAbortSignalAndTimeout treats timeoutMs <= 0 as no explicit timeout. Each call builds a fresh request-local config (no class-field abort controller) with a per-entry-point throwIfAborted guard that rejects before any network I/O. - eslint-suppressions.json: one stale suppression entry pruned (kimi-code.spec.ts @typescript-eslint/no-explicit-any 1 -> 0 - the spec rewrite removed the only as-any cast); no suppression count increased. This branch is STACKED on open PR Zoo-Code-Org#1288: the foundation commit e61feb1 (generic RequestConfigBuilder, mergeAbortSignalAndTimeout, mergeAbortSignals, throwIfAborted) rides inside by design.

coderabbitai · 2026-08-20T21:21:23Z

No actionable comments were generated in the recent review.

Recent review info

Run configuration

Source mode: Auto
Repository: Zoo-Code-Org/Zoo-Code
Branch: main
File: .coderabbit.yaml
Commit: 05d01f3
Layers: Repository YAML
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 7ac6b7d1-1175-4643-a38e-aa3ec7594432

Commits

Reviewing files that changed from the base of the PR and between fb2b478 and 1e8f68e.

Files selected for processing: 2

src/api/providers/__tests__/base-openai-compatible-provider.spec.ts
src/api/providers/base-openai-compatible-provider.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Recent review details

Context from checks skipped due to timeout: 5

GitHub Check: platform-unit-test (windows-latest)
GitHub Check: compile
GitHub Check: platform-unit-test (ubuntu-latest)
GitHub Check: mutation-diff
GitHub Check: e2e-mock

Additional context used

Path-based instructions: 5

Treat model, provider, MCP, path, command, and tool data as untrusted.