Repository navigation
fix(catalog): keep service-enabled Auto in model list - #43
Merged
Merged
Conversation
…factor getCachedModels has dropped Auto unconditionally since 9d88e99: that refactor changed updateQoderModelsCache to key configs by display id ("Auto"), but the legacy-cleanup check still looked for the raw upstream key (data.configs.auto), which no longer exists in any cache written since. Check both spellings so an explicitly enabled service model survives, while caches without any auto config still get the legacy fallback removed.
Owner
|
Merged — thank you! Squash-merged as Verified independently before merging:
Good catch — this one silently ate a service model on every cache read. |
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.
OnlyTomInSecond
added a commit
to OnlyTomInSecond/pi-provider-qoder
that referenced
this pull request
Oct 27, 2026
Bring origin/main (v0.6.5) into the local architecture. Upstream's stream.ts monolith is NOT taken; its behaviour is re-implemented in the local modules. Adopted from upstream (conflict-free or re-implemented): - credits/quota feature (simonsmh#31): credit-display/credit-footer/quota/session-usage, billing fixtures, /qoder-usage footer flag, credit fields on assistant usage (qoder_provider, charged_credits, usd_equivalent) via credits.ts. - dynamic COSY identity (simonsmh#35, simonsmh#36): cosy-version.ts tracks the Qoder CLI release; catalog refreshes it before signing (throttled, abortable) and run-state / auth headers sign with the resolved version. - tool-call ID rewrite for Qoder's wire format (simonsmh#33): tool-call-id.ts applied in transform for tool_calls ids and tool_call_id. - DSML residue + degenerate turn (simonsmh#28): stripDsmlResidue/isDegenerateDsmlTurn in thinking.ts, residue stripped from replayed reasoning and the thinking channel, degenerate turns surfaced as a retryable "server error" instead of a silent stop. - model-busy recovery (simonsmh#41): new protocol/model-busy.ts parses the nested 10605 envelope, retries with a bounded budget, resets per-attempt output, and re-encodes/re-signs request_id/chat_record_id while keeping the request set. Wired through sse.ts and streamQoder. - thinking-off wire signals (simonsmh#40, 030a1fb): request-builder sends enable_thinking:false plus reasoning_effort:"none" (omitted when the catalog shows no disabled branch), and flips model_config.is_reasoning. - flat Context fallback (simonsmh#38): request-builder keeps top-level systemPrompt and tools when the transcript has no folded system message. - tool-call identity by ID (simonsmh#39): ToolCallAccumulator now keys native deltas by id and index, keeps parallel index-less calls apart, rebinds a reused index, and rejects ambiguous continuations. - catalog metadata (simonsmh#34, simonsmh#42, simonsmh#43): Fast model entry, omp `thinking` efforts, display-keyed Auto config handling. Kept local implementations where they were already ahead: - refactored stream modules (request-builder/sse/delta-coalescer/resume/ thinking-sink/tool-calls/json-repair/dsml), DSML tool-call replay, image batching in transform, reasoning replay via reasoning_content. - request options object (QoderRequestOptions) instead of a bare AbortSignal; quota/tests adapted to it. Skipped: upstream's monolithic stream.ts internals and its tests for them; upstream resolveRequestContext (covered by the request-builder fallback). Verification: tsc --noEmit, biome check, vitest (572 tests) and esbuild build.
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.
Problem
Since 9d88e99, the
Automodel no longer appears in the model list, even though the service explicitly returns it (key: "auto",enable: true).Root cause
55e87ch introduced a legacy-cleanup filter in
getCachedModelswith a dual-key cache writer:configs[rawkey]andconfigs[displayId], so!data.configs.autocorrectly detected a service-enabled auto and kept it.9d88e99 changed the writer to store configs only under the display id (
configs[modelInfo.id], raw-key alias removed and asserted in tests), but the reader-side check still testsdata.configs.auto. That is now alwaysundefined, so the filter fires on every cache read and unconditionally dropsAuto.Fix
Check both spellings (
configs.autoandconfigs.Auto) and only filter when neither exists. This restores the original 55e87cb intent: keep an explicitly enabled service model, drop only the legacy injected fallback.Tests
{ key: "auto", enable: true, display_name: "Auto" }keepsAutoas the first model.Verification
~/.pi/agent/qoder-models-cache.json) shows upstream returns auto withenable: true; after the fixpi --list-modelslistsAutofirst for theqoderprovider.Auto.Note
Autocurrently reports no thinking support in model metadata while thinking content is actually returned -qoderclibehaves the same (effort is not selectable, but thinking content is still output).