fix(api): propagate AbortSignal to OpenAI-compatible providers for mid-stream cancellation#429
Open
proyectoauraorg wants to merge 2 commits into
Conversation
…d-stream cancellation Both BaseOpenAiCompatibleProvider and OpenAICompatibleHandler did not create an AbortController or pass a signal to their underlying SDK calls. When a user hit Stop, the Task-level Promise.race would break out of the chunk loop, but the HTTP request continued running server-side until it finished naturally. This fix adds an AbortController to both providers: - BaseOpenAiCompatibleProvider: signal passed via requestOptions to chat.completions.create() and checked in the stream iteration loop - OpenAICompatibleHandler: abortSignal passed to streamText() and generateText() from the AI SDK - Both providers clean up the controller in finally blocks - completePrompt() also gets abort support in both providers Includes 5 regression tests covering signal propagation, mid-stream abort, and controller cleanup. Closes: Zoo-Code-Org#404
proyectoauraorg
requested review from
JamesRobert20,
edelauna,
hannesrudolph,
navedmerchant and
taltas
as code owners
Contributor
📝 WalkthroughWalkthroughAdds per-request cancellation: handlers and the base provider create an AbortController for each request, pass its signal to streaming and non-streaming SDK calls, stop processing when aborted, and always clear the controller. Tests updated to assert signal propagation, mid-stream abort behavior, and controller cleanup. ChangesRequest Cancellation via AbortController
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/api/providers/__tests__/fireworks.spec.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. src/api/providers/__tests__/sambanova.spec.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. src/api/providers/__tests__/zai.spec.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. 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: 2
♻️ Duplicate comments (3)
src/api/providers/openai-compatible.ts (2)
215-234:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
completePromptfinallyalso needsabort().Same fix as the streaming path: abort before clearing the controller.
🤖 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-compatible.ts` around lines 215 - 234, In completePrompt, the finally block currently clears this.abortController but doesn't call abort; update the finally to first call this.abortController.abort() (guarded with a truthy check) to cancel any in-flight generateText, then set this.abortController = undefined; reference the completePrompt method and the abortController property to locate and modify the finally block.
160-209:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftSame cancellation defect as
BaseOpenAiCompatibleProvider—finallyclears but never aborts.The
finally(Line 207-209) nullsthis.abortControllerwithout calling.abort(), so closing the generator early does not cancel thestreamTextrequest, andsignal.aborted(Line 192) never becomes true in production. Addthis.abortController?.abort()infinally(and consider the same local-controller approach to avoid the shared-field race).Additionally note: once abort actually fires,
await result.usageat Line 203 may reject for an aborted stream — guard that path or skip the usage read when aborted.🤖 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-compatible.ts` around lines 160 - 209, The finally block currently only clears this.abortController without aborting the request; update the generator in openai-compatible.ts to call this.abortController?.abort() before setting it to undefined (or better: create a local const controller = new AbortController(), use controller for request/abort and assign to this.abortController = controller to avoid races), ensure streamText is given controller.signal, and guard the usage read (result.usage) so it is skipped or try/catch'd when the controller.signal.aborted to avoid an exception after an abort; reference symbols: this.abortController, streamText, result.usage, processAiSdkStreamPart, and consider mirroring the local-controller approach used in BaseOpenAiCompatibleProvider.src/api/providers/base-openai-compatible-provider.ts (1)
243-274:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame missing
abort()incompletePrompt'sfinally.Same root cause as
createMessage: the controller is cleared but never aborted, so the signal cannot cancel the request. Apply the matching fix here.🤖 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/base-openai-compatible-provider.ts` around lines 243 - 274, The finally block in completePrompt currently clears this.abortController without aborting the in-flight request; mirror the fix from createMessage by calling this.abortController?.abort() before setting this.abortController = undefined so the AbortSignal actually cancels the request (apply to the completePrompt method in class BaseOpenAICompatibleProvider and ensure the same pattern matches createMessage).
🧹 Nitpick comments (1)
src/api/providers/__tests__/base-openai-compatible-provider.spec.ts (1)
578-628: ⚡ Quick winTest exercises the loop
breakbut not actual request cancellation — it can't catch the missingabort().This test signals abort via the private field and asserts the loop stops yielding, which the current code already satisfies. It does not assert that the underlying SDK request is aborted when the generator is closed early (the real
#404scenario). Consider adding a test that closes the generator (iterator.return()) without manually aborting, then asserts the controller's signal became aborted — this would have surfaced thefinallygap.💚 Suggested coverage
+ it("should abort the underlying request when the generator is closed early", async () => { + mockCreate.mockImplementationOnce(() => ({ + [Symbol.asyncIterator]: () => ({ + next: vi.fn().mockResolvedValue({ + done: false, + value: { choices: [{ delta: { content: "chunk" } }] }, + }), + }), + })) + + const stream = handler.createMessage("system prompt", []) + const iterator = stream[Symbol.asyncIterator]() + await iterator.next() + const controller = (handler as any).abortController as AbortController + const signal = controller.signal + await iterator.return?.() + expect(signal.aborted).toBe(true) + })🤖 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 578 - 628, Add a new test case (or modify the existing one) that verifies the underlying request is actually aborted when the async generator is closed: call handler.createMessage(...) to get the stream, obtain its async iterator, then invoke iterator.return() (instead of calling controller.abort()), and assert that the handler's private abortController.signal.aborted (or that abortController was signaled) becomes true; this will exercise the generator-close path and catch the missing cancellation in the finally/cleanup logic around createMessage/abortController/iterator.return().
🤖 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/base-openai-compatible-provider.ts`:
- Around line 120-217: The finally block in the stream-handling code for
createStream never calls abort on the AbortController, so in-flight requests
won't be cancelled; update the finally in the method that creates the stream
(where this.abortController is set) to call this.abortController.abort() if
present before clearing it (keep the existing this.abortController = undefined),
ensuring you reference the same AbortController instance used when calling
createStream (symbols: this.abortController, createStream, and the surrounding
try/catch/finally block in base-openai-compatible-provider.ts).
- Around line 39-40: BaseOpenAiCompatibleProvider currently uses an instance
field abortController shared across requests; change createMessage and
completePrompt to create a local AbortController (const abortController = new
AbortController()) and use its signal for all abort checks and API calls within
that invocation instead of this.abortController, pass that local signal into any
fetch/SDK options, and remove any writes/clears to this.abortController so
finally blocks do not unset a controller used by another concurrent call; keep
the per-invocation signal local throughout the method and only check/clear that
local controller.
---
Duplicate comments:
In `@src/api/providers/base-openai-compatible-provider.ts`:
- Around line 243-274: The finally block in completePrompt currently clears
this.abortController without aborting the in-flight request; mirror the fix from
createMessage by calling this.abortController?.abort() before setting
this.abortController = undefined so the AbortSignal actually cancels the request
(apply to the completePrompt method in class BaseOpenAICompatibleProvider and
ensure the same pattern matches createMessage).
In `@src/api/providers/openai-compatible.ts`:
- Around line 215-234: In completePrompt, the finally block currently clears
this.abortController but doesn't call abort; update the finally to first call
this.abortController.abort() (guarded with a truthy check) to cancel any
in-flight generateText, then set this.abortController = undefined; reference the
completePrompt method and the abortController property to locate and modify the
finally block.
- Around line 160-209: The finally block currently only clears
this.abortController without aborting the request; update the generator in
openai-compatible.ts to call this.abortController?.abort() before setting it to
undefined (or better: create a local const controller = new AbortController(),
use controller for request/abort and assign to this.abortController = controller
to avoid races), ensure streamText is given controller.signal, and guard the
usage read (result.usage) so it is skipped or try/catch'd when the
controller.signal.aborted to avoid an exception after an abort; reference
symbols: this.abortController, streamText, result.usage, processAiSdkStreamPart,
and consider mirroring the local-controller approach used in
BaseOpenAiCompatibleProvider.
---
Nitpick comments:
In `@src/api/providers/__tests__/base-openai-compatible-provider.spec.ts`:
- Around line 578-628: Add a new test case (or modify the existing one) that
verifies the underlying request is actually aborted when the async generator is
closed: call handler.createMessage(...) to get the stream, obtain its async
iterator, then invoke iterator.return() (instead of calling controller.abort()),
and assert that the handler's private abortController.signal.aborted (or that
abortController was signaled) becomes true; this will exercise the
generator-close path and catch the missing cancellation in the finally/cleanup
logic around createMessage/abortController/iterator.return().
🪄 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: ea6e5497-adae-403f-9d0f-789f4c0b585e
📒 Files selected for processing (3)
src/api/providers/__tests__/base-openai-compatible-provider.spec.tssrc/api/providers/base-openai-compatible-provider.tssrc/api/providers/openai-compatible.ts
Comment on lines
+39
to
+40
| // Abort controller for cancelling ongoing requests (fixes #404) | ||
| private abortController?: AbortController |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'createMessage|completePrompt' --type=ts -C2 -g '!**/*.spec.ts' -g '!**/__tests__/**'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 33232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== abortController occurrences =="
rg -n "abortController" "$FILE" || true
echo
echo "== createMessage around abortController usage =="
rg -n "createMessage\\(" "$FILE" -n || true
echo
echo "== completePrompt around abortController usage =="
rg -n "completePrompt\\(" "$FILE" || true
echo
echo "== context: lines 1-120 =="
nl -ba "$FILE" | sed -n '1,160p'
echo
echo "== context: around createMessage & completePrompt =="
nl -ba "$FILE" | sed -n '100,320p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 757
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Providers factory / instantiation =="
rg -n "new (.*Provider)" src/api -S --type=ts || true
echo
echo "== Where api handler is created and passed to Task =="
rg -n "new .*Provider|getProvider|provider" src/core/task/Task.ts src/api/index.ts src/api -S --type=ts || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== abortController occurrences =="
rg -n "abortController" "$FILE" || true
echo
echo "== context: lines 1-220 =="
nl -ba "$FILE" | sed -n '1,260p'
echo
echo "== context: lines 220-420 =="
nl -ba "$FILE" | sed -n '220,420p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 559
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== abortController occurrences =="
rg -n "abortController" "$FILE" || true
echo
echo "== context: lines 1-260 =="
nl -ba "$FILE" | sed -n '1,260p'
echo
echo "== context: lines 260-460 =="
nl -ba "$FILE" | sed -n '260,460p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 559
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== show code around abortController in base-openai-compatible-provider.ts =="
# print with line numbers using awk
awk 'NR>=1 && NR<=320 {printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '1,260p'
echo
echo "== specific ranges (100-190) =="
awk 'NR>=90 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== specific ranges (210-310) =="
awk 'NR>=210 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== search for api handler instance creation in src/api =="
rg -n "getProvider|createProvider|createHandler|new .*Provider\\(|BaseOpenAiCompatibleProvider" src/api --type=ts || true
echo
echo "== search Task.ts for createMessage/completePrompt call sites =="
rg -n "createMessage\\(|completePrompt\\(" src/core/task/Task.ts --type=ts || true
echo
echo "== show Task.ts around createMessage call (near reported line 4167) =="
awk 'NR>=4120 && NR<=4225 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts || true
echo
echo "== search for Promise.all or other concurrency patterns around API calls =="
rg -n "Promise\\.all|allSettled|await .*\\[|concurrent|subtask|parallel" src/core/task/Task.ts --type=ts || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 29277
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== show code around abortController in base-openai-compatible-provider.ts =="
awk 'NR>=1 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '1,160p'
echo
echo "== specific ranges (100-190) =="
awk 'NR>=90 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== specific ranges (210-310) =="
awk 'NR>=200 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== search for api handler instance creation in src/api =="
rg -n "getProvider|createProvider|createHandler|new .*Provider\\(|BaseOpenAiCompatibleProvider" src/api --type=ts || true
echo
echo "== search Task.ts for createMessage/completePrompt call sites =="
rg -n "createMessage\\(|completePrompt\\(" src/core/task/Task.ts --type=ts || true
echo
echo "== show Task.ts around createMessage call (near reported line 4167) =="
awk 'NR>=4140 && NR<=4210 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts
echo
echo "== search for concurrency patterns in nearby Task.ts region =="
awk 'NR>=4050 && NR<=4300 {print NR":"$0}' src/core/task/Task.ts | rg -n "Promise\\.all|allSettled|parallel|concurrent|subtask" || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 23269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== show code around abortController in base-openai-compatible-provider.ts =="
awk 'NR>=1 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE" | sed -n '1,160p'
echo
echo "== specific ranges (100-190) =="
awk 'NR>=90 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== specific ranges (200-310) =="
awk 'NR>=200 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== search for api handler instance creation in src/api =="
rg -n "getProvider|createProvider|createHandler|new .*Provider\\(|BaseOpenAiCompatibleProvider" src/api --type=ts || true
echo
echo "== search Task.ts for createMessage/completePrompt call sites =="
rg -n "createMessage\\(|completePrompt\\(" src/core/task/Task.ts --type=ts || true
echo
echo "== show Task.ts around createMessage call (near reported line 4167) =="
awk 'NR>=4140 && NR<=4210 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts
echo
echo "== search for concurrency patterns in nearby Task.ts region =="
awk 'NR>=4050 && NR<=4300 {print NR":"$0}' src/core/task/Task.ts | rg -n "Promise\\.all|allSettled|parallel|concurrent|subtask" || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 23269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== specific ranges (90-200) =="
awk 'NR>=90 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== specific ranges (200-310) =="
awk 'NR>=200 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== search api handler instance creation in src/api =="
rg -n "getProvider|createProvider|createHandler|new .*Provider\\(|BaseOpenAiCompatibleProvider" src/api --type=ts || true
echo
echo "== search Task.ts for createMessage/completePrompt call sites =="
rg -n "createMessage\\(|completePrompt\\(" src/core/task/Task.ts --type=ts || true
echo
echo "== show Task.ts around createMessage call (near 4160-4210) =="
awk 'NR>=4140 && NR<=4210 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts
echo
echo "== search for Promise.all / concurrent patterns in Task.ts =="
rg -n "Promise\\.all|allSettled|concurrent|parallel|subtask" src/core/task/Task.ts --type=ts || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 17909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== specific ranges (90-200) =="
awk 'NR>=90 && NR<=200 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== specific ranges (200-310) =="
awk 'NR>=200 && NR<=310 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "== search api handler instance creation in src/api =="
rg -n "getProvider|createProvider|createHandler|new .*Provider\\(|BaseOpenAiCompatibleProvider" src/api --type=ts || true
echo
echo "== search Task.ts for createMessage/completePrompt call sites =="
rg -n "createMessage\\(|completePrompt\\(" src/core/task/Task.ts --type=ts || true
echo
echo "== show Task.ts around createMessage call (near 4140-4210) =="
awk 'NR>=4140 && NR<=4210 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts
echo
echo "== search for Promise.all / concurrent patterns in Task.ts =="
rg -n "Promise\\.all|allSettled|concurrent|parallel|subtask" src/core/task/Task.ts --type=ts || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 17909
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Look for .abort usage on abortController or related cancellation in the provider
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== abort() calls in base-openai-compatible-provider.ts =="
rg -n "abortController\\?\\.abort|abortController\\.abort|\\.abort\\(" "$FILE" || true
echo
echo "== abortController usage (broader) =="
rg -n "abortController" "$FILE"
echo
echo "== search for currentRequestAbortController usage in Task.ts =="
rg -n "currentRequestAbortController" src/core/task/Task.ts --type=ts || true
echo
echo "== search for anything that calls api/provider abort in Task.ts near abort handling =="
rg -n "abort\\(|cancel\\(|AbortSignal|Request cancelled" src/core/task/Task.ts --type=ts | head -n 80
echo
echo "== inspect Task.ts around where abort might be handled (recent lines) =="
awk 'NR>=1060 && NR<=1220 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.ts | sed -n '1,200p'
echo
echo "== inspect Task.ts around abort logic near attemptApiRequest loop start =="
rg -n "abort" src/core/task/Task.ts --type=ts | head -n 50Repository: Zoo-Code-Org/Zoo-Code
Length of output: 12365
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Look for how cancellation is actually triggered (abort() calls) in the provider
FILE="src/api/providers/base-openai-compatible-provider.ts"
echo "== abort() calls in base-openai-compatible-provider.ts =="
rg -n "abortController\\?\\.abort|abortController\\.abort|\\.abort\\(" "$FILE" || true
echo
echo "== abortController usage (broader) =="
rg -n "abortController" "$FILE"
echo
echo "== search for currentRequestAbortController usage in Task.ts =="
rg -n "currentRequestAbortController" src/core/task/Task.ts --type=ts || true
echo
echo "== search for cancellation/error text in Task.ts =="
rg -n "Request cancelled by user|AbortSignal|AbortSignal|cancel|abort\\(" src/core/task/Task.ts --type=ts | head -n 80
echo
echo "== inspect Task.ts around createMessage abort handling =="
awk 'NR>=4120 && NR<=4245 {printf "%5d:%s\n", NR, $0}' src/core/task/Task.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 10154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find how Task assigns/creates this.api (the handler/provider instance)
rg -n "this\\.api\\s*=" src/core/task/Task.ts --type=ts || true
echo
rg -n "api\\b.*new|create.*api|getApi|provider.*api" src/core/task/Task.ts --type=ts || true
echo
rg -n "class Task|constructor\\(" src/core/task/Task.ts --type=ts | head -n 50 || true
echo
# Find type of this.api property declaration
rg -n "api\\?:" src/core/task/Task.ts --type=ts || true
echo
# Search for assignments to this.api.current or similar
rg -n "this\\.api\\." src/core/task/Task.ts --type=ts | head -n 50 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 1697
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TASK="src/core/task/Task.ts"
echo "== show Task.ts around this.api assignment at ~440-520 =="
awk 'NR>=440 && NR<=520 {print NR":"$0}' "$TASK"
echo
echo "== show Task.ts around this.api assignment at ~1400-1460 =="
awk 'NR>=1380 && NR<=1460 {print NR":"$0}' "$TASK"
echo
echo "== find Task.ts call sites for completePrompt on this.api =="
rg -n "this\\.api\\.(completePrompt|createMessage)\\(" "$TASK" --type=ts || true
echo
echo "== find buildApiHandler definition =="
rg -n "function buildApiHandler|buildApiHandler\\(" src/api --type=ts || true
echo
echo "== inspect buildApiHandler implementation (if found) =="
# try to locate file containing buildApiHandler
FILE=$(rg -l "buildApiHandler\\(" src/api --type=ts | head -n 1 || true)
if [ -n "${FILE:-}" ]; then
echo "Found: $FILE"
awk 'NR>=1 && NR<=220 {print NR":"$0}' "$FILE" | sed -n '1,220p'
else
echo "No buildApiHandler implementation found under src/api"
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 13351
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for new Task() usage (subtasks likely)
rg -n "new Task\\(" src/core/task/Task.ts src/core/task -S --type=ts || true
# Search where request loop calls completePrompt (condense / summarization)
rg -n "\\.completePrompt\\(" src/core src/api -S --type=ts | head -n 100 || true
# Search for initiateTaskLoop / parallel subtask execution patterns
rg -n "initiateTaskLoop\\(|subtask|delegateParentAndOpenChild|childTask|currentTask|rootTaskId|parentTaskId" src/core/task/Task.ts --type=ts | head -n 120 || true
# Inspect relevant parts around initiateTaskLoop and any subtask branching
awk 'NR>=2200 && NR<=2400 {print NR":"$0}' src/core/task/Task.ts | sed -n '1,220p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 28161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg for completePrompt call sites =="
rg -n "completePrompt\\(" src/core src/api --type=ts -S --glob '!**/*.spec.ts' --glob '!**/__tests__/**' | head -n 200
echo
echo "== rg for this.api.completePrompt =="
rg -n "this\\.api\\.completePrompt\\(" src/core --type=ts -S || true
echo
echo "== rg for apiHandler.completePrompt =="
rg -n "apiHandler\\.?completePrompt\\(" src/core --type=ts -S || true
echo
echo "== Show condense/index.ts completePrompt call area =="
rg -n "completePrompt" src/core/condense/index.ts --type=ts || true
if [ -f src/core/condense/index.ts ]; then
awk 'NR>=280 && NR<=380 {print NR":"$0}' src/core/condense/index.ts
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 6788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rg for completePrompt call sites =="
rg -n "completePrompt\\(" src/core src/api --type=ts -S --glob '!**/*.spec.ts' --glob '!**/__tests__/**' | head -n 200
echo
echo "== rg for this.api.completePrompt =="
rg -n "this\\.api\\.completePrompt\\(" src/core --type=ts -S || true
echo
echo "== rg for apiHandler.completePrompt =="
rg -n "apiHandler\\.?completePrompt\\(" src/core --type=ts -S || true
echo
echo "== Show condense/index.ts completePrompt call area =="
rg -n "completePrompt" src/core/condense/index.ts --type=ts || true
if [ -f src/core/condense/index.ts ]; then
awk 'NR>=280 && NR<=380 {print NR":"$0}' src/core/condense/index.ts
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 6788
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find usage of single-completion-handler.ts =="
rg -n "single-completion-handler|SingleCompletionHandler" src --type=ts -S --glob '!**/*.spec.ts' --glob '!**/__tests__/**' || true
echo
echo "== find completePrompt call sites in non-test code =="
rg -n "completePrompt\\(" src --type=ts -S --glob '!**/*.spec.ts' --glob '!**/__tests__/**' || true
echo
echo "== show src/utils/single-completion-handler.ts =="
sed -n '1,200p' src/utils/single-completion-handler.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 10262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "404|fixes `#404`|AbortSignal|cancelCurrentRequest|currentRequestAbortController|abortController\\." src --type=ts --glob '!**/*.spec.ts' --glob '!**/__tests__/**' || true
echo "--- tests ---"
rg -n "404|fixes `#404`|AbortSignal|currentRequestAbortController|cancelCurrentRequest|abortController\\." src --type=ts -S --glob '**/*.spec.ts' --glob '**/__tests__/**/*.ts' | head -n 120 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 11745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/api/providers/openai-compatible.ts"
echo "== abortController occurrences in openai-compatible.ts =="
rg -n "abortController" "$FILE" --type=ts || true
echo
echo "== show code around createMessage/completePrompt abort usage =="
# show around each line with abortController
rg -n "abortController" "$FILE" --type=ts | cut -d: -f1 | while read -r ln; do
start=$((ln-15)); end=$((ln+20));
echo "--- around $ln ($start-$end) ---"
awk -v s=$start -v e=$end 'NR>=s && NR<=e {print NR":"$0}' "$FILE"
echo
doneRepository: Zoo-Code-Org/Zoo-Code
Length of output: 8912
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for condense entrypoints and how they’re triggered from Task
rg -n "condense" src/core/task/Task.ts --type=ts -S || true
rg -n "condense.*(Task|Context)|condenseTask|condense.*context|SUMMARY_PROMPT" src/core --type=ts -S || true
# Find where condense/index.ts is imported/called
rg -n "from \"\\.\\./condense|from \"\\.\\./\\.\\./condense|condense/index|core/condense" src/core --type=ts -S || true
# Specifically search for uses of condense that might run without awaiting (e.g., void/Promise)
rg -n "void .*condense|Promise\\..*condense|condense\\(.*\\)\\s*;" src/core --type=ts -S || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 13097
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TASK="src/core/task/Task.ts"
ClineProvider="src/core/webview/ClineProvider.ts"
webviewMsgHandler="src/core/webview/webviewMessageHandler.ts"
condenseIdx="src/core/condense/index.ts"
contextMgmt="src/core/context-management/index.ts"
echo "== Task.ts: condenseContext() implementation (~1480-1705) =="
awk 'NR>=1460 && NR<=1710 {print NR":"$0}' "$TASK" | sed -n '1,260p'
echo
echo "== Task.ts: condenseContext invocation / context management (~3680-4100) =="
awk 'NR>=3680 && NR<=4105 {print NR":"$0}' "$TASK" | sed -n '1,340p'
echo
echo "== ClineProvider.ts: condenseTaskContext() (~1770-1835) =="
awk 'NR>=1760 && NR<=1845 {print NR":"$0}' "$ClineProvider"
echo
echo "== webviewMessageHandler.ts: condenseTaskContextRequest handling (~780-840) =="
awk 'NR>=760 && NR<=840 {print NR":"$0}' "$webviewMsgHandler"
echo
echo "== condense/index.ts: summarizeConversation signature & how it calls apiHandler.createMessage (~90-390) =="
awk 'NR>=1 && NR<=390 {print NR":"$0}' "$condenseIdx" | sed -n '1,220p'
echo
echo "== context-management/index.ts: where condensing runs (~250-380 and surrounding) =="
rg -n "condenseContext|summarizeConversation|truncate|autoCondenseContext" "$contextMgmt" --type=ts -S
awk 'NR>=250 && NR<=430 {print NR":"$0}' "$contextMgmt" | sed -n '1,240p'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 44821
Avoid shared abortController across overlapping createMessage/completePrompt calls
BaseOpenAiCompatibleProvider stores a single abortController on the handler instance, but both createMessage and completePrompt overwrite/read/clear it (this.abortController = new AbortController(), this.abortController?.signal.aborted, and finally { this.abortController = undefined }). If the same handler instance can serve overlapping requests (e.g., Task.condenseContext()/context management calls this.api.createMessage() while the main request is still streaming, and can be triggered via condenseTaskContextRequest), calls can consult/clear the wrong controller. Use a per-invocation local AbortController/signal and keep abort checks tied to that local controller (don’t rely on a shared instance field).
🤖 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/base-openai-compatible-provider.ts` around lines 39 - 40,
BaseOpenAiCompatibleProvider currently uses an instance field abortController
shared across requests; change createMessage and completePrompt to create a
local AbortController (const abortController = new AbortController()) and use
its signal for all abort checks and API calls within that invocation instead of
this.abortController, pass that local signal into any fetch/SDK options, and
remove any writes/clears to this.abortController so finally blocks do not unset
a controller used by another concurrent call; keep the per-invocation signal
local throughout the method and only check/clear that local controller.
Comment on lines
+120
to
217
| // Create AbortController for cancellation (fixes #404) | ||
| this.abortController = new AbortController() | ||
|
|
||
| const matcher = new TagMatcher( | ||
| "think", | ||
| (chunk) => | ||
| ({ | ||
| type: chunk.matched ? "reasoning" : "text", | ||
| text: chunk.data, | ||
| }) as const, | ||
| ) | ||
|
|
||
| let lastUsage: OpenAI.CompletionUsage | undefined | ||
| const activeToolCallIds = new Set<string>() | ||
| try { | ||
| const stream = await this.createStream(systemPrompt, messages, metadata, { | ||
| signal: this.abortController.signal, | ||
| }) | ||
|
|
||
| const matcher = new TagMatcher( | ||
| "think", | ||
| (chunk) => | ||
| ({ | ||
| type: chunk.matched ? "reasoning" : "text", | ||
| text: chunk.data, | ||
| }) as const, | ||
| ) | ||
|
|
||
| let lastUsage: OpenAI.CompletionUsage | undefined | ||
| const activeToolCallIds = new Set<string>() | ||
|
|
||
| for await (const chunk of stream) { | ||
| // Check if request was aborted (fixes #404) | ||
| if (this.abortController?.signal.aborted) { | ||
| break | ||
| } | ||
|
|
||
| for await (const chunk of stream) { | ||
| // Check for provider-specific error responses (e.g., MiniMax base_resp) | ||
| const chunkAny = chunk as any | ||
| if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { | ||
| throw new Error( | ||
| `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, | ||
| ) | ||
| } | ||
| // Check for provider-specific error responses (e.g., MiniMax base_resp) | ||
| const chunkAny = chunk as any | ||
| if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { | ||
| throw new Error( | ||
| `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, | ||
| ) | ||
| } | ||
|
|
||
| const delta = chunk.choices?.[0]?.delta | ||
| const finishReason = chunk.choices?.[0]?.finish_reason | ||
| const delta = chunk.choices?.[0]?.delta | ||
| const finishReason = chunk.choices?.[0]?.finish_reason | ||
|
|
||
| if (delta?.content) { | ||
| for (const processedChunk of matcher.update(delta.content)) { | ||
| yield processedChunk | ||
| if (delta?.content) { | ||
| for (const processedChunk of matcher.update(delta.content)) { | ||
| yield processedChunk | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (delta) { | ||
| for (const key of ["reasoning_content", "reasoning"] as const) { | ||
| if (key in delta) { | ||
| const reasoning_content = ((delta as any)[key] as string | undefined) || "" | ||
| if (reasoning_content?.trim()) { | ||
| yield { type: "reasoning", text: reasoning_content } | ||
| if (delta) { | ||
| for (const key of ["reasoning_content", "reasoning"] as const) { | ||
| if (key in delta) { | ||
| const reasoning_content = ((delta as any)[key] as string | undefined) || "" | ||
| if (reasoning_content?.trim()) { | ||
| yield { type: "reasoning", text: reasoning_content } | ||
| } | ||
| break | ||
| } | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Emit raw tool call chunks - NativeToolCallParser handles state management | ||
| if (delta?.tool_calls) { | ||
| for (const toolCall of delta.tool_calls) { | ||
| if (toolCall.id) { | ||
| activeToolCallIds.add(toolCall.id) | ||
| // Emit raw tool call chunks - NativeToolCallParser handles state management | ||
| if (delta?.tool_calls) { | ||
| for (const toolCall of delta.tool_calls) { | ||
| if (toolCall.id) { | ||
| activeToolCallIds.add(toolCall.id) | ||
| } | ||
| yield { | ||
| type: "tool_call_partial", | ||
| index: toolCall.index, | ||
| id: toolCall.id, | ||
| name: toolCall.function?.name, | ||
| arguments: toolCall.function?.arguments, | ||
| } | ||
| } | ||
| yield { | ||
| type: "tool_call_partial", | ||
| index: toolCall.index, | ||
| id: toolCall.id, | ||
| name: toolCall.function?.name, | ||
| arguments: toolCall.function?.arguments, | ||
| } | ||
|
|
||
| // Emit tool_call_end events when finish_reason is "tool_calls" | ||
| // This ensures tool calls are finalized even if the stream doesn't properly close | ||
| if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { | ||
| for (const id of activeToolCallIds) { | ||
| yield { type: "tool_call_end", id } | ||
| } | ||
| activeToolCallIds.clear() | ||
| } | ||
| } | ||
|
|
||
| // Emit tool_call_end events when finish_reason is "tool_calls" | ||
| // This ensures tool calls are finalized even if the stream doesn't properly close | ||
| if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { | ||
| for (const id of activeToolCallIds) { | ||
| yield { type: "tool_call_end", id } | ||
| if (chunk.usage) { | ||
| lastUsage = chunk.usage | ||
| } | ||
| activeToolCallIds.clear() | ||
| } | ||
|
|
||
| if (chunk.usage) { | ||
| lastUsage = chunk.usage | ||
| if (lastUsage) { | ||
| yield this.processUsageMetrics(lastUsage, this.getModel().info) | ||
| } | ||
| } | ||
|
|
||
| if (lastUsage) { | ||
| yield this.processUsageMetrics(lastUsage, this.getModel().info) | ||
| } | ||
|
|
||
| // Process any remaining content | ||
| for (const processedChunk of matcher.final()) { | ||
| yield processedChunk | ||
| // Process any remaining content | ||
| for (const processedChunk of matcher.final()) { | ||
| yield processedChunk | ||
| } | ||
| } catch (error) { | ||
| throw handleOpenAIError(error, this.providerName) | ||
| } finally { | ||
| this.abortController = undefined | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any abort() invocations or public abort surface tied to these providers
rg -nP '\.abort\s*\(' --type=ts -g '!**/__tests__/**' -g '!**/*.spec.ts'
echo '--- abortController references ---'
rg -nP 'abortController' --type=ts -C2 -g '!**/*.spec.ts'
echo '--- how Task/stop interacts with providers (createMessage consumers) ---'
rg -nP '\.return\(|abort|cancelStream|stopStream' --type=ts -C2 -g '*[Tt]ask*'Repository: Zoo-Code-Org/Zoo-Code
Length of output: 48095
Call abort() in finally to cancel in-flight streaming on Stop
In src/api/providers/base-openai-compatible-provider.ts, the stream generator passes signal: this.abortController.signal to createStream, but its finally block only clears the reference (this.abortController = undefined) and never calls this.abortController.abort(). So when the consumer stops pulling/returns the generator, cancellation won’t occur via this provider’s AbortController, and the underlying request may keep running.
🐛 Proposed fix
} catch (error) {
throw handleOpenAIError(error, this.providerName)
} finally {
+ // Abort the underlying request if the generator is closed early (e.g. Stop)
+ this.abortController?.abort()
this.abortController = undefined
}📝 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
| // Create AbortController for cancellation (fixes #404) | |
| this.abortController = new AbortController() | |
| const matcher = new TagMatcher( | |
| "think", | |
| (chunk) => | |
| ({ | |
| type: chunk.matched ? "reasoning" : "text", | |
| text: chunk.data, | |
| }) as const, | |
| ) | |
| let lastUsage: OpenAI.CompletionUsage | undefined | |
| const activeToolCallIds = new Set<string>() | |
| try { | |
| const stream = await this.createStream(systemPrompt, messages, metadata, { | |
| signal: this.abortController.signal, | |
| }) | |
| const matcher = new TagMatcher( | |
| "think", | |
| (chunk) => | |
| ({ | |
| type: chunk.matched ? "reasoning" : "text", | |
| text: chunk.data, | |
| }) as const, | |
| ) | |
| let lastUsage: OpenAI.CompletionUsage | undefined | |
| const activeToolCallIds = new Set<string>() | |
| for await (const chunk of stream) { | |
| // Check if request was aborted (fixes #404) | |
| if (this.abortController?.signal.aborted) { | |
| break | |
| } | |
| for await (const chunk of stream) { | |
| // Check for provider-specific error responses (e.g., MiniMax base_resp) | |
| const chunkAny = chunk as any | |
| if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { | |
| throw new Error( | |
| `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, | |
| ) | |
| } | |
| // Check for provider-specific error responses (e.g., MiniMax base_resp) | |
| const chunkAny = chunk as any | |
| if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { | |
| throw new Error( | |
| `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, | |
| ) | |
| } | |
| const delta = chunk.choices?.[0]?.delta | |
| const finishReason = chunk.choices?.[0]?.finish_reason | |
| const delta = chunk.choices?.[0]?.delta | |
| const finishReason = chunk.choices?.[0]?.finish_reason | |
| if (delta?.content) { | |
| for (const processedChunk of matcher.update(delta.content)) { | |
| yield processedChunk | |
| if (delta?.content) { | |
| for (const processedChunk of matcher.update(delta.content)) { | |
| yield processedChunk | |
| } | |
| } | |
| } | |
| if (delta) { | |
| for (const key of ["reasoning_content", "reasoning"] as const) { | |
| if (key in delta) { | |
| const reasoning_content = ((delta as any)[key] as string | undefined) || "" | |
| if (reasoning_content?.trim()) { | |
| yield { type: "reasoning", text: reasoning_content } | |
| if (delta) { | |
| for (const key of ["reasoning_content", "reasoning"] as const) { | |
| if (key in delta) { | |
| const reasoning_content = ((delta as any)[key] as string | undefined) || "" | |
| if (reasoning_content?.trim()) { | |
| yield { type: "reasoning", text: reasoning_content } | |
| } | |
| break | |
| } | |
| break | |
| } | |
| } | |
| } | |
| // Emit raw tool call chunks - NativeToolCallParser handles state management | |
| if (delta?.tool_calls) { | |
| for (const toolCall of delta.tool_calls) { | |
| if (toolCall.id) { | |
| activeToolCallIds.add(toolCall.id) | |
| // Emit raw tool call chunks - NativeToolCallParser handles state management | |
| if (delta?.tool_calls) { | |
| for (const toolCall of delta.tool_calls) { | |
| if (toolCall.id) { | |
| activeToolCallIds.add(toolCall.id) | |
| } | |
| yield { | |
| type: "tool_call_partial", | |
| index: toolCall.index, | |
| id: toolCall.id, | |
| name: toolCall.function?.name, | |
| arguments: toolCall.function?.arguments, | |
| } | |
| } | |
| yield { | |
| type: "tool_call_partial", | |
| index: toolCall.index, | |
| id: toolCall.id, | |
| name: toolCall.function?.name, | |
| arguments: toolCall.function?.arguments, | |
| } | |
| // Emit tool_call_end events when finish_reason is "tool_calls" | |
| // This ensures tool calls are finalized even if the stream doesn't properly close | |
| if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { | |
| for (const id of activeToolCallIds) { | |
| yield { type: "tool_call_end", id } | |
| } | |
| activeToolCallIds.clear() | |
| } | |
| } | |
| // Emit tool_call_end events when finish_reason is "tool_calls" | |
| // This ensures tool calls are finalized even if the stream doesn't properly close | |
| if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { | |
| for (const id of activeToolCallIds) { | |
| yield { type: "tool_call_end", id } | |
| if (chunk.usage) { | |
| lastUsage = chunk.usage | |
| } | |
| activeToolCallIds.clear() | |
| } | |
| if (chunk.usage) { | |
| lastUsage = chunk.usage | |
| if (lastUsage) { | |
| yield this.processUsageMetrics(lastUsage, this.getModel().info) | |
| } | |
| } | |
| if (lastUsage) { | |
| yield this.processUsageMetrics(lastUsage, this.getModel().info) | |
| } | |
| // Process any remaining content | |
| for (const processedChunk of matcher.final()) { | |
| yield processedChunk | |
| // Process any remaining content | |
| for (const processedChunk of matcher.final()) { | |
| yield processedChunk | |
| } | |
| } catch (error) { | |
| throw handleOpenAIError(error, this.providerName) | |
| } finally { | |
| this.abortController = undefined | |
| } | |
| // Create AbortController for cancellation (fixes `#404`) | |
| this.abortController = new AbortController() | |
| try { | |
| const stream = await this.createStream(systemPrompt, messages, metadata, { | |
| signal: this.abortController.signal, | |
| }) | |
| const matcher = new TagMatcher( | |
| "think", | |
| (chunk) => | |
| ({ | |
| type: chunk.matched ? "reasoning" : "text", | |
| text: chunk.data, | |
| }) as const, | |
| ) | |
| let lastUsage: OpenAI.CompletionUsage | undefined | |
| const activeToolCallIds = new Set<string>() | |
| for await (const chunk of stream) { | |
| // Check if request was aborted (fixes `#404`) | |
| if (this.abortController?.signal.aborted) { | |
| break | |
| } | |
| // Check for provider-specific error responses (e.g., MiniMax base_resp) | |
| const chunkAny = chunk as any | |
| if (chunkAny.base_resp?.status_code && chunkAny.base_resp.status_code !== 0) { | |
| throw new Error( | |
| `${this.providerName} API Error (${chunkAny.base_resp.status_code}): ${chunkAny.base_resp.status_msg || "Unknown error"}`, | |
| ) | |
| } | |
| const delta = chunk.choices?.[0]?.delta | |
| const finishReason = chunk.choices?.[0]?.finish_reason | |
| if (delta?.content) { | |
| for (const processedChunk of matcher.update(delta.content)) { | |
| yield processedChunk | |
| } | |
| } | |
| if (delta) { | |
| for (const key of ["reasoning_content", "reasoning"] as const) { | |
| if (key in delta) { | |
| const reasoning_content = ((delta as any)[key] as string | undefined) || "" | |
| if (reasoning_content?.trim()) { | |
| yield { type: "reasoning", text: reasoning_content } | |
| } | |
| break | |
| } | |
| } | |
| } | |
| // Emit raw tool call chunks - NativeToolCallParser handles state management | |
| if (delta?.tool_calls) { | |
| for (const toolCall of delta.tool_calls) { | |
| if (toolCall.id) { | |
| activeToolCallIds.add(toolCall.id) | |
| } | |
| yield { | |
| type: "tool_call_partial", | |
| index: toolCall.index, | |
| id: toolCall.id, | |
| name: toolCall.function?.name, | |
| arguments: toolCall.function?.arguments, | |
| } | |
| } | |
| } | |
| // Emit tool_call_end events when finish_reason is "tool_calls" | |
| // This ensures tool calls are finalized even if the stream doesn't properly close | |
| if (finishReason === "tool_calls" && activeToolCallIds.size > 0) { | |
| for (const id of activeToolCallIds) { | |
| yield { type: "tool_call_end", id } | |
| } | |
| activeToolCallIds.clear() | |
| } | |
| if (chunk.usage) { | |
| lastUsage = chunk.usage | |
| } | |
| } | |
| if (lastUsage) { | |
| yield this.processUsageMetrics(lastUsage, this.getModel().info) | |
| } | |
| // Process any remaining content | |
| for (const processedChunk of matcher.final()) { | |
| yield processedChunk | |
| } | |
| } catch (error) { | |
| throw handleOpenAIError(error, this.providerName) | |
| } finally { | |
| // Abort the underlying request if the generator is closed early (e.g. Stop) | |
| this.abortController?.abort() | |
| this.abortController = undefined | |
| } |
🤖 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/base-openai-compatible-provider.ts` around lines 120 - 217,
The finally block in the stream-handling code for createStream never calls abort
on the AbortController, so in-flight requests won't be cancelled; update the
finally in the method that creates the stream (where this.abortController is
set) to call this.abortController.abort() if present before clearing it (keep
the existing this.abortController = undefined), ensuring you reference the same
AbortController instance used when calling createStream (symbols:
this.abortController, createStream, and the surrounding try/catch/finally block
in base-openai-compatible-provider.ts).
…rovider specs
The AbortSignal propagation change passes a second argument
`{ signal }` to `chat.completions.create`. Fireworks, SambaNova and
Z AI specs still asserted the second arg as `undefined`, causing 6
failures in platform-unit-test. Update them to match the base provider
spec pattern: `expect.objectContaining({ signal: expect.any(AbortSignal) })`.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/api/providers/__tests__/zai.spec.ts (1)
498-504:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate ZAI provider tests to include the
{ signal }argument intoHaveBeenCalledWithassertions
createMessagecallsthis.client.chat.completions.create(params, { signal }), but severalmockCreate.toHaveBeenCalledWithassertions insrc/api/providers/__tests__/zai.spec.tsonly check the first (params) argument—omitting the second options argument—so they should fail (missingexpect.objectContaining({ signal: expect.any(AbortSignal) })at lines 498-504, 528-534, 559-565, 590-596, 643-649, 673-679).🐛 Proposed fix (apply the same pattern to each affected assertion)
expect(mockCreate).toHaveBeenCalledWith( expect.objectContaining({ model: "glm-5.1", max_tokens: 40_000, }), + expect.objectContaining({ signal: expect.any(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/__tests__/zai.spec.ts` around lines 498 - 504, The failing tests omit the second options argument with the AbortSignal when asserting calls to the ZAI client: update each mockCreate.toHaveBeenCalledWith(...) in the zai.spec.ts tests to include the second arg by asserting { signal: expect.any(AbortSignal) } (e.g. use toHaveBeenCalledWith(expect.objectContaining({...params...}), expect.objectContaining({ signal: expect.any(AbortSignal) })); this matches how createMessage calls this.client.chat.completions.create(params, { signal }) and should be applied to the mockCreate assertions mentioned (those referencing createMessage / this.client.chat.completions.create / mockCreate).
🤖 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.
Outside diff comments:
In `@src/api/providers/__tests__/zai.spec.ts`:
- Around line 498-504: The failing tests omit the second options argument with
the AbortSignal when asserting calls to the ZAI client: update each
mockCreate.toHaveBeenCalledWith(...) in the zai.spec.ts tests to include the
second arg by asserting { signal: expect.any(AbortSignal) } (e.g. use
toHaveBeenCalledWith(expect.objectContaining({...params...}),
expect.objectContaining({ signal: expect.any(AbortSignal) })); this matches how
createMessage calls this.client.chat.completions.create(params, { signal }) and
should be applied to the mockCreate assertions mentioned (those referencing
createMessage / this.client.chat.completions.create / mockCreate).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e741f9d-aa12-4c6b-b8cf-b5eefd492dcb
📒 Files selected for processing (3)
src/api/providers/__tests__/fireworks.spec.tssrc/api/providers/__tests__/sambanova.spec.tssrc/api/providers/__tests__/zai.spec.ts
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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
1 participant
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.
Related GitHub Issue
Closes: #404
Description
Root cause: Both
BaseOpenAiCompatibleProviderandOpenAICompatibleHandlerdid not create anAbortControlleror pass a signal to their underlying SDK calls. When a user hit Stop, the Task-levelPromise.racewould break out of the chunk consumption loop, but the HTTP request continued running server-side until it finished naturally — wasting compute on local providers like llama.cpp.In contrast,
OpenAINativeHandlerandOpenAICodexHandlerboth correctly create anAbortControllerand passsignal: this.abortController.signalto their API calls.Fix:
BaseOpenAiCompatibleProvider(base-openai-compatible-provider.ts):abortControllerfieldcreateMessage(): createsAbortController, passes{ signal }viarequestOptionstoclient.chat.completions.create(), checkssignal.abortedin the stream loop, cleans up infinallycompletePrompt(): createsAbortController, passes{ signal }toclient.chat.completions.create(), cleans up infinallyOpenAICompatibleHandler(openai-compatible.ts):abortControllerfieldcreateMessage(): createsAbortController, passesabortSignaltostreamText(), checkssignal.abortedin thefullStreamloop, cleans up infinallycompletePrompt(): createsAbortController, passesabortSignaltogenerateText(), cleans up infinallyTest Procedure
signalis passed tochat.completions.createincreateMessagesignalis passed tochat.completions.createincompletePromptabortControlleris cleaned up aftercreateMessagecompletesabortControlleris cleaned up aftercompletePromptcompletesPre-Submission Checklist
Summary by CodeRabbit