feat: add abort signal pass-through to ~19 providers#679
Draft
easonLiangWorldedtech wants to merge 2 commits into
Draft
feat: add abort signal pass-through to ~19 providers#679easonLiangWorldedtech wants to merge 2 commits into
easonLiangWorldedtech wants to merge 2 commits into
Conversation
easonLiangWorldedtech
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
📝 WalkthroughWalkthroughAdds an optional ChangesAbortSignal propagation across all API providers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/api/providers/__tests__/anthropic-vertex.spec.ts`:
- Around line 1654-1686: The test "should not pass signal when abortSignal is
undefined" currently only verifies that chunks are received but does not
actually assert that the signal was omitted from the request. After the mocked
handler["client"].messages.create is called within the loop consuming the
stream, capture the arguments passed to this mock and assert that the second
argument (the options/config object passed to the SDK) is either an empty object
or explicitly does not contain a signal property. This will ensure the test
actually validates that abortSignal is not passed when undefined, rather than
only checking the output.
- Around line 1619-1652: The test "should handle abort signal triggered during
request" at the AnthropicVertexHandler test file is not actually testing the
abort scenario because controller.abort() is never called during the test
execution. To fix this, add a call to controller.abort() inside the for-await
loop that iterates through the stream chunks to actually trigger the abort
signal during streaming. This will ensure the mockStream function's abort check
is evaluated when the signal is truly aborted, validating the cancellation
behavior that the test is supposed to cover.
In `@src/api/providers/__tests__/base-openai-compatible-provider.spec.ts`:
- Around line 560-567: The abort-path test mocks are reading controller.signal
from the outer scope instead of verifying that the SDK call actually receives
the abort signal in its options parameter. This creates false positives where
the test passes even if createMessage doesn't propagate metadata.abortSignal to
the SDK. Modify the mockImplementation at line 560 (and the similar mock at line
618) to either assert that the received options parameter contains the expected
signal property, or drive the abort behavior from the options argument instead
of accessing controller.signal from the outer scope. This ensures the test
actually validates that the abort signal is being correctly forwarded to the SDK
call.
In `@src/api/providers/__tests__/gemini.spec.ts`:
- Around line 370-470: The abort signal tests in the describe block labeled
"abort signal" only verify that the signal is correctly passed through to the
SDK options, but do not validate that the stream actually respects the abort
signal and stops or rejects iteration when aborted. Add a new test case within
the "abort signal" describe block that aborts the controller during stream
iteration and verifies that the stream properly responds to the abort (either by
rejecting the iteration or stopping gracefully), ensuring that signal
cancellation is actually honored by the implementation and not just passed
through unused.
In `@src/api/providers/__tests__/lmstudio.spec.ts`:
- Around line 164-171: The mockCreate.mockImplementation for the
Symbol.asyncIterator is using controller.signal from the outer scope rather than
checking that the signal passed via the options parameter is properly forwarded.
To fix this, modify the abort check logic to use the signal from the options
parameter instead of relying on controller.signal, or add an assertion to verify
that mockCreate was called with signal: controller.signal to ensure signal
forwarding is properly tested and prevent false positives when the signal
passing regresses.
In `@src/api/providers/__tests__/mimo.spec.ts`:
- Around line 1009-1016: The current mock implementation for mockCreate checks
controller.signal from closure scope rather than verifying the signal is
actually passed to the SDK call. To fix this, add an assertion within the
mockCreate.mockImplementation call (around lines 1009, 1070, and 1081) to verify
that the second argument received by mockCreate contains the expected signal
property (either { signal: controller.signal } when abort is provided or
undefined otherwise). This ensures the test proves the abortSignal is actually
being forwarded from the provider to the SDK, not just assuming it works through
closure scope.
In `@src/api/providers/__tests__/minimax.spec.ts`:
- Around line 483-570: The abort signal tests are checking the controller.signal
from the outer scope directly in the mock implementation rather than verifying
that the abortSignal is actually being forwarded from the createMessage call to
messages.create(). To properly verify the signal is being passed through, modify
the mockCreate.mockImplementation functions in all three test cases (the ones
checking abort signal triggered during request, without abortSignal, and abort
immediately if signal already aborted) to extract the abortSignal from the
options parameter passed to the mock and use that signal to determine when to
throw the abort error, instead of referencing controller.signal from the outer
scope. This will ensure the tests fail if createMessage doesn't actually forward
the metadata.abortSignal to the messages.create() call.
In `@src/api/providers/__tests__/moonshot.spec.ts`:
- Around line 475-557: The abort signal tests validate abort behavior but do not
verify that the abortSignal is actually forwarded to the underlying SDK call.
Add assertions after consuming the stream in each test case (in "should handle
abort signal triggered during request", "should work normally without
abortSignal", and "should abort immediately if signal is already aborted") to
check that mockStreamText.mock.calls[0][0].signal matches the expected signal
(or is undefined for the test without abortSignal), ensuring createMessage
properly passes the abort signal through to the mocked streamText call.
In `@src/api/providers/__tests__/openai.spec.ts`:
- Around line 1282-1379: The abort signal tests validate the controller state
but don't verify that the abort signal is properly passed to the underlying
OpenAI SDK's chat.completions.create call. Modify the mockCreate mock
implementations to assert that the options parameter contains the abortSignal
when provided to handler.createMessage, ensuring the signal is properly wired
through to the SDK. Additionally, add a new test case that verifies abort signal
handling for the O-family model route (which uses a separate create path) to
prevent branch-specific regressions and ensure both creation paths properly
handle abort signals.
In `@src/api/providers/__tests__/openrouter.spec.ts`:
- Around line 719-817: The abort signal tests do not verify that the abortSignal
parameter passed to the handler's createMessage method is actually forwarded to
the OpenAI client's create call. Add assertions after the handler.createMessage
calls in both the "should handle abort signal triggered during request" and
"should abort immediately if signal is already aborted" test cases to verify
that mockCreate was invoked with options containing a signal property that
matches the abortSignal passed to createMessage.
In `@src/api/providers/__tests__/poe.spec.ts`:
- Around line 319-327: The mockStreamText mock returns an async iterable
directly, but PoeHandler.createMessage expects result.fullStream and
result.usage properties. Refactor the mockStreamText.mockReturnValue call to
return an object with fullStream property containing the async iterable (the
Symbol.asyncIterator function), and add a usage property with mock usage data
that matches the expected structure, so the mock contract aligns with what
PoeHandler.createMessage actually reads at runtime.
In `@src/api/providers/__tests__/qwen-code-native-tools.spec.ts`:
- Around line 451-539: The abort signal tests verify that the stream aborts but
don't confirm that the abortSignal is actually forwarded to the underlying Qwen
client's create method. Add direct assertions in each test that passes an
abortSignal to createMessage (the first and third test cases) to verify that
mockCreate was called with the signal included in the options parameter. This
ensures that createMessage is actually passing the signal parameter to the API
client, not just relying on external abortion of the iterator.
In `@src/api/providers/__tests__/requesty.spec.ts`:
- Around line 550-580: The abort tests in the mock implementation at lines
550-580 and around line 608 are checking controller.signal.aborted directly
within the mock stream, which allows the tests to pass even if createMessage is
not actually forwarding the abortSignal to the SDK request options. Remove the
abort signal check from inside the mock stream's async iterator implementation
and instead add an assertion to verify that mockCreate was called with the
correct abortSignal included in the options parameter passed to it. This ensures
the test actually proves that the abortSignal is being properly propagated
through createMessage to the underlying SDK.
In `@src/api/providers/__tests__/unbound.spec.ts`:
- Around line 209-246: The test for abort scenarios in the UnboundHandler does
not verify that the abort signal is actually forwarded to the OpenAI client's
chat.completions.create method. Instead of relying on closure state with
controller.signal in the mock iterator, add an assertion to verify that
mockCreateLocal receives the correct request options including the abortSignal
parameter that was passed to handler.createMessage(). Modify the mockCreateLocal
mock to check and assert that the options parameter contains the expected
abortSignal before the async iterator logic, ensuring the signal wiring is
properly tested rather than just simulating the abort behavior.
In `@src/api/providers/__tests__/xai.spec.ts`:
- Around line 308-334: The abort-path tests for XAIHandler do not verify that
the responses.create mock was called with the correct signal option. After the
stream consumption completes and the abort error is caught, add an explicit
assertion to verify that mockResponsesCreate was called with an options object
containing the signal property set to controller.signal. This ensures the
request-options contract is properly tested and confirmed in the abort scenario.
In `@src/api/providers/__tests__/zai.spec.ts`:
- Around line 901-931: The test for abort functionality should explicitly verify
that the abortSignal is forwarded to the underlying mockCreate call, rather than
relying on the mock stream's internal abort logic which could mask regressions.
After the expect().rejects.toThrow assertion in the abort test case, add a
separate assertion to verify that mockCreate was called with arguments that
include a signal property matching the controller.signal that was passed to
createMessage. This ensures that signal-forwarding from createMessage to the
underlying API call is explicitly validated. Apply the same pattern to the
additional abort test case referenced at lines 959-981.
In `@src/api/providers/__tests__/zoo-gateway.spec.ts`:
- Around line 644-674: The abort test for createMessage does not verify that the
abortSignal is actually being propagated to the underlying request options.
Currently, the test passes if the stream aborts, but this could happen even if
createMessage fails to forward the signal to mockCreate. Add an assertion to
verify that mockCreate was called with a second argument containing signal:
controller.signal, ensuring the abort signal from the test is properly passed
through to the underlying request handler. Apply the same fix to the additional
abort test mentioned in the comment (lines 702-724).
In `@src/api/providers/openai.ts`:
- Around line 178-181: The abort signal propagation is missing for O3-family
model requests handled in the handleO3FamilyMessage function. Apply the same
abort signal pattern used in the regular chat completion calls (spreading
metadata?.abortSignal conditionally as signal property) to both SDK calls within
handleO3FamilyMessage at the locations where completions are created for
o1/o3/o4 models. This ensures that stop/cancel operations work consistently
across all model families by including the abort signal in the request options
for both SDK invocations in that handler.
In `@src/api/providers/poe.ts`:
- Line 104: In the streamText options at line 104 within the poe.ts provider
file, change the property name from `signal` to `abortSignal` in the spread
operator condition. The ai SDK's streamText function expects `abortSignal` as
the parameter name for request cancellation, not `signal`. This change aligns
with how other providers like openai-compatible.ts correctly pass the abort
signal and ensures user cancellation requests work properly for Poe requests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e6a7079c-ec4a-4efb-a47a-7812ea789ac1
📒 Files selected for processing (41)
src/api/index.tssrc/api/providers/__tests__/anthropic-vertex.spec.tssrc/api/providers/__tests__/anthropic.spec.tssrc/api/providers/__tests__/base-openai-compatible-provider.spec.tssrc/api/providers/__tests__/deepseek.spec.tssrc/api/providers/__tests__/gemini.spec.tssrc/api/providers/__tests__/lite-llm.spec.tssrc/api/providers/__tests__/lmstudio.spec.tssrc/api/providers/__tests__/mimo.spec.tssrc/api/providers/__tests__/minimax.spec.tssrc/api/providers/__tests__/moonshot.spec.tssrc/api/providers/__tests__/openai.spec.tssrc/api/providers/__tests__/openrouter.spec.tssrc/api/providers/__tests__/poe.spec.tssrc/api/providers/__tests__/qwen-code-native-tools.spec.tssrc/api/providers/__tests__/requesty.spec.tssrc/api/providers/__tests__/unbound.spec.tssrc/api/providers/__tests__/xai.spec.tssrc/api/providers/__tests__/zai.spec.tssrc/api/providers/__tests__/zoo-gateway.spec.tssrc/api/providers/anthropic-vertex.tssrc/api/providers/anthropic.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/deepseek.tssrc/api/providers/gemini.tssrc/api/providers/lite-llm.tssrc/api/providers/lm-studio.tssrc/api/providers/mimo.tssrc/api/providers/minimax.tssrc/api/providers/openai-compatible.tssrc/api/providers/openai.tssrc/api/providers/openrouter.tssrc/api/providers/poe.tssrc/api/providers/qwen-code.tssrc/api/providers/requesty.tssrc/api/providers/unbound.tssrc/api/providers/xai.tssrc/api/providers/zai.tssrc/api/providers/zoo-gateway.tssrc/core/task/Task.tssrc/core/task/__tests__/Task.spec.ts
Comment on lines
+1619
to
+1652
| it("should handle abort signal triggered during request", async () => { | ||
| const controller = new AbortController() | ||
| const handler = new AnthropicVertexHandler({ | ||
| apiModelId: "claude-3-sonnet", | ||
| vertexProjectId: "test-project", | ||
| vertexRegion: "us-central1", | ||
| }) | ||
|
|
||
| const mockStream = async function* () { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new Error("AbortError: The operation was aborted") | ||
| } | ||
| yield { | ||
| type: "message_start", | ||
| message: { usage: { input_tokens: 10, output_tokens: 0 } }, | ||
| } | ||
| } | ||
|
|
||
| ;(handler["client"].messages as any).create = vitest.fn().mockResolvedValue(mockStream()) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }], { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort-path test is not actually aborting.
Line 1619 names a mid-request abort scenario, but controller.abort() is never triggered. This currently validates normal streaming only, not cancellation behavior.
Suggested fix
it("should handle abort signal triggered during request", async () => {
const controller = new AbortController()
+ let capturedRequestOptions: any
- ;(handler["client"].messages as any).create = vitest.fn().mockResolvedValue(mockStream())
+ ;(handler["client"].messages as any).create = vitest.fn().mockImplementation(async (_params, requestOptions) => {
+ capturedRequestOptions = requestOptions
+ return mockStream()
+ })
const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }], {
taskId: "test",
tools: [],
abortSignal: controller.signal,
})
+
+ setTimeout(() => controller.abort(), 20)
- const chunks: any[] = []
- for await (const chunk of stream) {
- chunks.push(chunk)
- }
-
- expect(chunks.length).toBeGreaterThan(0)
+ await expect(async () => {
+ for await (const _chunk of stream) {}
+ }).rejects.toThrow(/abort/i)
+ expect(capturedRequestOptions).toHaveProperty("signal", controller.signal)
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/anthropic-vertex.spec.ts` around lines 1619 -
1652, The test "should handle abort signal triggered during request" at the
AnthropicVertexHandler test file is not actually testing the abort scenario
because controller.abort() is never called during the test execution. To fix
this, add a call to controller.abort() inside the for-await loop that iterates
through the stream chunks to actually trigger the abort signal during streaming.
This will ensure the mockStream function's abort check is evaluated when the
signal is truly aborted, validating the cancellation behavior that the test is
supposed to cover.
Comment on lines
+1654
to
+1686
| it("should not pass signal when abortSignal is undefined", async () => { | ||
| const handler = new AnthropicVertexHandler({ | ||
| apiModelId: "claude-3-sonnet", | ||
| vertexProjectId: "test-project", | ||
| vertexRegion: "us-central1", | ||
| }) | ||
|
|
||
| const mockStream = async function* () { | ||
| yield { | ||
| type: "message_start", | ||
| message: { usage: { input_tokens: 10, output_tokens: 5 } }, | ||
| } | ||
| yield { | ||
| type: "content_block_start", | ||
| content_block: { type: "text", text: "" }, | ||
| } | ||
| yield { | ||
| type: "content_block_delta", | ||
| delta: { type: "text_delta", text: "response" }, | ||
| } | ||
| } | ||
|
|
||
| ;(handler["client"].messages as any).create = vitest.fn().mockResolvedValue(mockStream()) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }]) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| }) |
Contributor
There was a problem hiding this comment.
“No abortSignal” test should assert request options, not only chunks.
Line 1654 currently verifies output only; it does not prove signal omission. Capture/assert the second SDK arg (for this provider, {} without signal) to prevent false positives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/anthropic-vertex.spec.ts` around lines 1654 -
1686, The test "should not pass signal when abortSignal is undefined" currently
only verifies that chunks are received but does not actually assert that the
signal was omitted from the request. After the mocked
handler["client"].messages.create is called within the loop consuming the
stream, capture the arguments passed to this mock and assert that the second
argument (the options/config object passed to the SDK) is either an empty object
or explicitly does not contain a signal property. This will ensure the test
actually validates that abortSignal is not passed when undefined, rather than
only checking the output.
Comment on lines
+560
to
+567
| mockCreate.mockImplementation(async (options: unknown) => { | ||
| return { | ||
| async *[Symbol.asyncIterator]() { | ||
| while (!controller.signal.aborted) { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new Error("AbortError: The operation was aborted") | ||
| } |
Contributor
There was a problem hiding this comment.
Abort-path tests are currently false-positive prone because they don’t verify signal forwarding.
Line 560 and Line 618 mocks read controller.signal from outer scope, so Line 589/Line 639 can still reject even if createMessage never passes metadata.abortSignal to the SDK call. Please assert the SDK call receives { signal: controller.signal } (or drive abort behavior from the received options argument) so this suite protects the propagation contract.
Suggested tightening
mockCreate.mockImplementation(async (options: unknown) => {
+ const requestOptions = options as { signal?: AbortSignal } | undefined
return {
async *[Symbol.asyncIterator]() {
- while (!controller.signal.aborted) {
+ while (!requestOptions?.signal?.aborted) {
await new Promise((resolve) => setTimeout(resolve, 10))
- if (controller.signal.aborted) {
+ if (requestOptions?.signal?.aborted) {
throw new Error("AbortError: The operation was aborted")
}
yield {
choices: [{ delta: { content: "response" } }],
usage: null,
}
}
},
}
})
...
+expect(mockCreate).toHaveBeenCalledWith(
+ expect.anything(),
+ expect.objectContaining({ signal: controller.signal }),
+)Also applies to: 577-590, 618-623, 629-640
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/base-openai-compatible-provider.spec.ts` around
lines 560 - 567, The abort-path test mocks are reading controller.signal from
the outer scope instead of verifying that the SDK call actually receives the
abort signal in its options parameter. This creates false positives where the
test passes even if createMessage doesn't propagate metadata.abortSignal to the
SDK. Modify the mockImplementation at line 560 (and the similar mock at line
618) to either assert that the received options parameter contains the expected
signal property, or drive the abort behavior from the options argument instead
of accessing controller.signal from the outer scope. This ensures the test
actually validates that the abort signal is being correctly forwarded to the SDK
call.
Comment on lines
+370
to
+470
| describe("abort signal", () => { | ||
| it("should pass abortSignal to the SDK options", async () => { | ||
| const controller = new AbortController() | ||
| let capturedConfig: any | ||
|
|
||
| handler["client"].models.generateContentStream = vitest.fn().mockImplementation(async (params) => { | ||
| capturedConfig = params?.config | ||
| return { | ||
| [Symbol.asyncIterator]: async function* () { | ||
| yield { text: "Hello world!" } | ||
| yield { usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 } } | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any, { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| expect(capturedConfig).toHaveProperty("signal", controller.signal) | ||
| }) | ||
|
|
||
| it("should work normally without abortSignal", async () => { | ||
| handler["client"].models.generateContentStream = vitest.fn().mockResolvedValue({ | ||
| [Symbol.asyncIterator]: async function* () { | ||
| yield { text: "Hello world!" } | ||
| yield { usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 } } | ||
| }, | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| }) | ||
|
|
||
| it("should not pass signal when abortSignal is undefined", async () => { | ||
| let capturedConfig: any | ||
|
|
||
| handler["client"].models.generateContentStream = vitest.fn().mockImplementation(async (params) => { | ||
| capturedConfig = params?.config | ||
| return { | ||
| [Symbol.asyncIterator]: async function* () { | ||
| yield { text: "Hello world!" } | ||
| yield { usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 } } | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| expect(capturedConfig).not.toHaveProperty("signal") | ||
| }) | ||
|
|
||
| it("should pass signal when provided", async () => { | ||
| const controller = new AbortController() | ||
| let capturedConfig: any | ||
|
|
||
| handler["client"].models.generateContentStream = vitest.fn().mockImplementation(async (params) => { | ||
| capturedConfig = params?.config | ||
| return { | ||
| [Symbol.asyncIterator]: async function* () { | ||
| yield { text: "Hello world!" } | ||
| yield { usageMetadata: { promptTokenCount: 10, candidatesTokenCount: 5 } } | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any, { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| const chunks: any[] = [] | ||
| for await (const chunk of stream) { | ||
| chunks.push(chunk) | ||
| } | ||
|
|
||
| expect(chunks.length).toBeGreaterThan(0) | ||
| expect(capturedConfig).toHaveProperty("signal", controller.signal) | ||
| }) | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort behavior itself is not validated, only signal plumbing.
These cases confirm config.signal forwarding, but there’s no assertion that iteration actually rejects on mid-stream or already-aborted signals. Add at least one behavioral abort test so this suite catches regressions where signal is passed but cancellation is ignored.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/gemini.spec.ts` around lines 370 - 470, The abort
signal tests in the describe block labeled "abort signal" only verify that the
signal is correctly passed through to the SDK options, but do not validate that
the stream actually respects the abort signal and stops or rejects iteration
when aborted. Add a new test case within the "abort signal" describe block that
aborts the controller during stream iteration and verifies that the stream
properly responds to the abort (either by rejecting the iteration or stopping
gracefully), ensuring that signal cancellation is actually honored by the
implementation and not just passed through unused.
Comment on lines
+164
to
+171
| mockCreate.mockImplementation(async (options: unknown) => { | ||
| return { | ||
| async *[Symbol.asyncIterator]() { | ||
| while (!controller.signal.aborted) { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new DOMException("The operation was aborted.", "AbortError") | ||
| } |
Contributor
There was a problem hiding this comment.
Abort tests should assert request-option signal forwarding to avoid false positives.
Line 164 and Line 211 mocks depend on outer controller.signal, not the signal provided via createMessage options. That means abort assertions can pass even if forwarding regresses. Please assert the mock was called with signal: controller.signal (or use the passed options in the mock’s abort checks).
Also applies to: 181-185, 211-216, 225-229
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/lmstudio.spec.ts` around lines 164 - 171, The
mockCreate.mockImplementation for the Symbol.asyncIterator is using
controller.signal from the outer scope rather than checking that the signal
passed via the options parameter is properly forwarded. To fix this, modify the
abort check logic to use the signal from the options parameter instead of
relying on controller.signal, or add an assertion to verify that mockCreate was
called with signal: controller.signal to ensure signal forwarding is properly
tested and prevent false positives when the signal passing regresses.
Comment on lines
+308
to
+334
| mockResponsesCreate.mockResolvedValueOnce({ | ||
| [Symbol.asyncIterator]: async function* () { | ||
| while (true) { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new Error("AbortError: The operation was aborted") | ||
| } | ||
| yield { type: "response.output_text.delta", delta: "chunk" } | ||
| } | ||
| }, | ||
| }) | ||
|
|
||
| handler = new XAIHandler({ xaiApiKey: "test-key" }) | ||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any, { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| setTimeout(() => controller.abort(), 50) | ||
|
|
||
| await expect(async () => { | ||
| for await (const _chunk of stream) { | ||
| // consume stream | ||
| } | ||
| }).rejects.toThrow(/abort/i) | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort tests miss the request-options contract check.
These cases can pass without confirming responses.create got { signal: controller.signal }. Please assert the second argument explicitly in abort-path tests.
Also applies to: 361-382
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/xai.spec.ts` around lines 308 - 334, The
abort-path tests for XAIHandler do not verify that the responses.create mock was
called with the correct signal option. After the stream consumption completes
and the abort error is caught, add an explicit assertion to verify that
mockResponsesCreate was called with an options object containing the signal
property set to controller.signal. This ensures the request-options contract is
properly tested and confirmed in the abort scenario.
Comment on lines
+901
to
+931
| mockCreate.mockImplementation(async (options: unknown) => { | ||
| return { | ||
| async *[Symbol.asyncIterator]() { | ||
| while (!controller.signal.aborted) { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new Error("AbortError: The operation was aborted") | ||
| } | ||
| yield { | ||
| choices: [{ delta: { content: "response" } }], | ||
| usage: null, | ||
| } | ||
| } | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any, { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| setTimeout(() => controller.abort(), 50) | ||
|
|
||
| await expect(async () => { | ||
| for await (const _chunk of stream) { | ||
| // consume stream | ||
| } | ||
| }).rejects.toThrow(/abort/i) | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort suite should assert create(..., { signal }) explicitly.
Right now the mock stream aborts from closure state, so signal-forwarding regressions may go undetected. Add direct assertions on mockCreate call args for abort cases.
Also applies to: 959-981
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/zai.spec.ts` around lines 901 - 931, The test for
abort functionality should explicitly verify that the abortSignal is forwarded
to the underlying mockCreate call, rather than relying on the mock stream's
internal abort logic which could mask regressions. After the
expect().rejects.toThrow assertion in the abort test case, add a separate
assertion to verify that mockCreate was called with arguments that include a
signal property matching the controller.signal that was passed to createMessage.
This ensures that signal-forwarding from createMessage to the underlying API
call is explicitly validated. Apply the same pattern to the additional abort
test case referenced at lines 959-981.
Comment on lines
+644
to
+674
| mockCreate.mockImplementation(async (options: unknown) => { | ||
| return { | ||
| async *[Symbol.asyncIterator]() { | ||
| while (!controller.signal.aborted) { | ||
| await new Promise((resolve) => setTimeout(resolve, 10)) | ||
| if (controller.signal.aborted) { | ||
| throw new Error("AbortError: The operation was aborted") | ||
| } | ||
| yield { | ||
| choices: [{ delta: { content: "response" } }], | ||
| usage: null, | ||
| } | ||
| } | ||
| }, | ||
| } | ||
| }) | ||
|
|
||
| const stream = handler.createMessage("system", [{ role: "user", content: "Hello" }] as any, { | ||
| taskId: "test", | ||
| tools: [], | ||
| abortSignal: controller.signal, | ||
| }) | ||
|
|
||
| setTimeout(() => controller.abort(), 50) | ||
|
|
||
| await expect(async () => { | ||
| for await (const _chunk of stream) { | ||
| // consume stream | ||
| } | ||
| }).rejects.toThrow(/abort/i) | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort tests should verify signal propagation into request options.
The current checks can pass even if createMessage stops attaching abortSignal, because abort is driven by a closure-scoped controller. Assert mockCreate receives a second arg containing signal: controller.signal.
Also applies to: 702-724
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/__tests__/zoo-gateway.spec.ts` around lines 644 - 674, The
abort test for createMessage does not verify that the abortSignal is actually
being propagated to the underlying request options. Currently, the test passes
if the stream aborts, but this could happen even if createMessage fails to
forward the signal to mockCreate. Add an assertion to verify that mockCreate was
called with a second argument containing signal: controller.signal, ensuring the
abort signal from the test is properly passed through to the underlying request
handler. Apply the same fix to the additional abort test mentioned in the
comment (lines 702-724).
Comment on lines
+178
to
+181
| stream = await this.client.chat.completions.create(requestOptions, { | ||
| ...(isAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}), | ||
| ...(metadata?.abortSignal ? { signal: metadata?.abortSignal } : {}), | ||
| }) |
Contributor
There was a problem hiding this comment.
Abort propagation is still missing for O3-family requests.
Line 92 routes o1/o3/o4 models to handleO3FamilyMessage, but the SDK calls there (Line 371-374 and Line 405-408) still omit metadata?.abortSignal. So stop/cancel remains ineffective for that model family.
Suggested fix
- stream = await this.client.chat.completions.create(
- requestOptions,
- methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {},
- )
+ stream = await this.client.chat.completions.create(requestOptions, {
+ ...(methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}),
+ ...(metadata?.abortSignal ? { signal: metadata.abortSignal } : {}),
+ })
- response = await this.client.chat.completions.create(
- requestOptions,
- methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {},
- )
+ response = await this.client.chat.completions.create(requestOptions, {
+ ...(methodIsAzureAiInference ? { path: OPENAI_AZURE_AI_INFERENCE_PATH } : {}),
+ ...(metadata?.abortSignal ? { signal: metadata.abortSignal } : {}),
+ })Also applies to: 244-247
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/openai.ts` around lines 178 - 181, The abort signal
propagation is missing for O3-family model requests handled in the
handleO3FamilyMessage function. Apply the same abort signal pattern used in the
regular chat completion calls (spreading metadata?.abortSignal conditionally as
signal property) to both SDK calls within handleO3FamilyMessage at the locations
where completions are created for o1/o3/o4 models. This ensures that stop/cancel
operations work consistently across all model families by including the abort
signal in the request options for both SDK invocations in that handler.
| tools: aiSdkTools, | ||
| toolChoice: mapToolChoice(metadata?.tool_choice as any), | ||
| ...(Object.keys(providerOptions).length > 0 && { providerOptions }), | ||
| ...(metadata?.abortSignal ? { signal: metadata.abortSignal } : {}), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In the aipackage version 6.0.77, what is the correct cancellation option name forstreamText(...): abortSignalorsignal?
💡 Result:
In the ai package (Vercel AI SDK), the correct option name for cancellation in streamText(...) is abortSignal [1][2][3]. The abortSignal option accepts an AbortSignal object and is the standard way to handle the cancellation of stream operations [1][2][4]. Official documentation and source code confirm that abortSignal is the designated property used to pass a signal to cancel the call [1][3][5].
Citations:
- 1: https://ai-sdk.dev/docs/reference/ai-sdk-core/stream-text
- 2: https://ai-sdk.dev/docs/advanced/stopping-streams
- 3: https://github.com/vercel/ai/blob/08cdf6ae/packages/ai/src/generate-text/stream-text.ts
- 4: https://ai-sdk.dev/v7/docs/reference/ai-sdk-core/stream-text
- 5: https://cdn.jsdelivr.net/npm/ai@6.0.116/src/generate-text/stream-text.ts
🏁 Script executed:
# Check the code at line 104 in poe.ts
head -n 110 src/api/providers/poe.ts | tail -n 15Repository: Zoo-Code-Org/Zoo-Code
Length of output: 692
🏁 Script executed:
# Find all streamText usages in the codebase to check consistency
rg "streamText" --type ts --type tsx -B 3 -A 10Repository: Zoo-Code-Org/Zoo-Code
Length of output: 93
🏁 Script executed:
# Check package.json for ai package version
cat package.json | grep -A 2 '"ai"'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 47
🏁 Script executed:
# Find all streamText usages in the codebase (TypeScript files only)
rg "streamText" -t ts -B 3 -A 10Repository: Zoo-Code-Org/Zoo-Code
Length of output: 8498
🏁 Script executed:
# Check package.json for ai package version
cat package.json | grep "ai"Repository: Zoo-Code-Org/Zoo-Code
Length of output: 47
🏁 Script executed:
# Also search for other places where abortSignal is used with streamText
rg "abortSignal" -t ts -B 2 -A 2Repository: Zoo-Code-Org/Zoo-Code
Length of output: 34142
Change signal to abortSignal in the streamText options.
Line 104 uses signal, but the ai SDK's streamText() requires abortSignal for cancellation. This pattern is consistent with openai-compatible.ts in the same codebase, which correctly passes abortSignal: metadata?.abortSignal. Using the wrong option name will prevent Poe requests from being cancelled when the user clicks stop.
Suggested fix
- ...(metadata?.abortSignal ? { signal: metadata.abortSignal } : {}),
+ ...(metadata?.abortSignal ? { abortSignal: metadata.abortSignal } : {}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ...(metadata?.abortSignal ? { signal: metadata.abortSignal } : {}), | |
| ...(metadata?.abortSignal ? { abortSignal: metadata.abortSignal } : {}), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/api/providers/poe.ts` at line 104, In the streamText options at line 104
within the poe.ts provider file, change the property name from `signal` to
`abortSignal` in the spread operator condition. The ai SDK's streamText function
expects `abortSignal` as the parameter name for request cancellation, not
`signal`. This change aligns with how other providers like openai-compatible.ts
correctly pass the abort signal and ensures user cancellation requests work
properly for Poe requests.
This was referenced Jun 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Complete abort signal pass-through across 19 HTTP-making API providers. When user clicks stop, the AbortController.signal from Task.ts flows through metadata → provider → HTTP client, cancelling the in-flight request and preventing wasted tokens/compute.
Changes (41 files, +2170/-48)
Core Plumbing
abortSignal?: AbortSignaltoApiHandlerCreateMessageMetadataAbortController.signalinto createMessage metadataProvider Pass-Through (19 providers)
Providers updated to thread
metadata.abortSignalthrough to their HTTP clients:Tests Added (~1800 lines)
Comprehensive test coverage for all updated providers verifying:
Related
Closes #434
Summary by CodeRabbit
New Features
Tests