Repository navigation
fix(stream): recover from transient model-busy (10605) rejections - #41
Merged
Merged
Conversation
Recognize Qoder's nested 10605 scheduler response and retry only ready rejections before any model output. Bound retries and delays, honor aborts, refresh request IDs and signatures, and clear all per-attempt metadata and usage. Surface a retryable error when local recovery is exhausted, while rejecting retries after partial text, thinking, or tool output. Add synthetic regression coverage including pi-ai retry integration.
Owner
|
Merged (squash: |
jischeng
added a commit
to jischeng/pi-provider-qoder
that referenced
this pull request
Oct 23, 2026
Sync upstream v0.6.2..v0.6.5 while keeping this fork's features, then fix the bugs that surfaced during the review. The upstream changes and the fixes touch the same functions, so they land together rather than as artificially split commits that would not pass on their own. Upstream taken: tool-call aggregation by ID instead of stream index (simonsmh#39), combined thinking-disable signals (simonsmh#40) with the effort-less catalog exception, bounded model-busy recovery (simonsmh#41), omp effort metadata (simonsmh#42), service-enabled Auto retention (simonsmh#43), COSY version tracking, and wire-format tool-call IDs. Fork behaviour preserved: multi-account providers, per-account entitlement pre-flight, price factors, keep-alive SSE, premature-disconnect surfacing, XML <tool_call> recovery, and always-on-thinking effort fallback. Fixes: 1. Cross-region credential leak. The global provider prefix ("qoder") is a strict prefix of the CN one ("qoder-cn"), so `id.startsWith(prefix + "-")` matched CN slots when listing global accounts. A global catalogue refresh then called /model/list with CN credentials against the GLOBAL endpoint and wrote that account into the global cache. Added isProviderOfMode() in region.ts and routed both account discovery and re-registration through it. 2. System-message folding was computed and discarded. The request body still prepended systemText to the unfiltered list, so it carried the top-level prompt plus every system turn instead of one merged message. 3. Orphan `tool` messages. A result whose id no preceding assistant turn declared was passed through (the "no assistant to pair with" case returned null, which read as permission rather than a veto). Upstreams reject that with "tool must follow a message with tool_calls", failing every later request in the session. Affected: resumed mid-turn, a user turn between call and result, and calls the lookahead trimmed. 4. Startup could hang forever. The default export awaits automatic login, and the PAT exchange / token refresh had no deadline, so an upstream that accepted the connection and never answered blocked extension load with no models, no UI, and no error. Bounded via authFetchSignal() (15s, merged with the caller's signal); the usage fetch defaults to a 10s timeout too. 5. Multi-pane torn reads of the COSY version cache. Plain writeFileSync let a second process parse a half-written body, throwing and leaving that pane on the bundled fallback version for the rest of the day. Now write-then-rename, matching the model catalogue. 6. Non-object tool arguments. `<tool_call>` XML whose arguments were an array, scalar, null, or a non-JSON string produced a value violating `ToolCall.arguments: JsonObject`, so argument validation failed instead of the tool running with no arguments. 7. Content order shuffled around tool calls. When a stream produced text, a tool call, then more text, the trailing text was appended to the block BEFORE the call: content read ["BEFOREAFTER", toolCall] and events addressed indexes that no longer matched. Root cause was a cached text-block index in ThinkingTagParser.emitText (and the equivalent branch in stream.ts) that was never re-checked once tool calls appended to `content`. Present upstream in v0.6.5 as well; its test asserted the shuffled order, so that assertion is corrected and a contentIndex-consistency check added. Also fixes provider re-registration across session replacement (issue #1): the registered-provider set was module-level, and ESM modules stay cached, so /resume, /new and /fork built a ModelRuntime with zero Qoder providers and the picker fell back to the default model. Now a per-instance WeakMap cleared on session_shutdown. Deviating points from upstream are marked `// fork:` in the source so the next sync can enumerate them mechanically.
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
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
Recover from transient Qoder model-busy rejections (
10605/RESOURCE_LIMIT) instead of failing the entire assistant turn immediately.Qoder can return HTTP 200 with an SSE envelope whose
statusCodeValueis 403. Its body contains nested JSON strings describing error10605and the scheduler state. Previously, the provider threw a genericUpstream status 403error; that message was not classified as retryable by pi-ai.Changes
isQueued === falseandserviceAvailable === true. Real queues, missing readiness flags, and unrelated errors retain their previous behavior.retryAfterSeconds. Default to two retries, honor an explicitmaxRetries(including zero), and cap local retries at 10. Bound each wait by 5 seconds and any smaller positivemaxRetryDelayMs;maxRetryDelayMs: 0keeps the provider's 5-second cap.request_set_idandsession_id, regeneraterequest_idandchat_record_idtogether, and re-encode/re-sign each retry.startevent and one terminal event. Once text, thinking (including buffered tag fragments), or tool-call deltas have arrived, fail without retrying to avoid duplicating output.model is busyerror so the host's existing retry policy can take over.This is bounded recovery for a ready scheduler response, not queue/status polling. It does not change the
isQueued: truepath.Tests
Regression coverage includes nested-envelope parsing, readiness checks, retry limits and delay caps, request identity and signatures, state cleanup, partial-output protection, abort behavior, reader cleanup, and integration with pi-ai's
retryAssistantCall.npm run lintnpm run checknpm test— 22 test files, 399 tests passednpm run buildgit diff --cached --checkAll regression fixtures are synthetic; no live Qoder requests are required.
Scope
Based on the latest upstream
main, preserving its thinking-off fixes. Onlysrc/protocol/stream.tsand the newsrc/__tests__/model-busy-retry.test.tsare changed; no generateddistfiles or unrelated fork commits are included. Most of the stream diff is indentation from moving one request attempt into a retryable function;git diff -wmakes the behavioral changes easier to review.