refactor(runtime): unify per-model protocol routing - #4957
Conversation
Normalize static SDK overrides at generation, select one call contract from effective model facts, and share it across SDK construction, thinking, probes and replay. Replace Kimi and Copilot dispatch branches with declared protocol adapters and preserve default Chat for unannotated models. Include Command Code discovery normalization and its catalog entry from #4943. Generated-by: Codex
Probe account availability before resolving an inference wire. Exercise both discovered Responses and Claude models through the same directory probe. Generated-by: Codex
Declare Messages and Responses for models absent from static routing metadata. Live tests confirmed Chat, Messages and Responses streaming tool loops; replace duplicate static SDK-name checks with the missing explicit-protocol regression. Generated-by: Codex
jackwener
left a comment
There was a problem hiding this comment.
Approving at exact head a14e84ef, at the author's request. test and label are terminal green on this commit, no review threads are open, and one [P3] is filed inline.
The scope of this approval, stated plainly, because one dimension was not covered the way the rest were. Five of the six areas raised were each reviewed independently and returned no P0–P2:
- Protocol routing is one contract, not a per-consumer guess. The resolver returns a single
{adapter, wire, reasoningReplay}that the factory, reasoning options, endpoint selection and the tool loop all consume. Worth correcting a phrase that recurs in discussion: "a protocol-A model on SDK B" is not what happens here — anapiProtocolselects an adapter that speaks A, and the types forbid pairing a Responses wire with Chat replay. - A remote catalog refresh cannot gain execution-switch authority, shown by naming the interception rather than by failing to find a call: a refresh swaps only the metadata table feeding
lookupModelMetadata, while adapter, protocol and endpoint come from a build-time generated constant a refresh cannot install. The field set a remote catalog can move was enumerated; it reaches display, thinking and output caps, and stops there. - User protocol overrides survive a Fetch, because execution always re-applies model-fact overrides over the discovered row.
- The removed Kimi/Copilot dispatch has a named successor for every branch, and that was demonstrated by mutation rather than by reading: deleting a
protocolAdaptersentry rejects only that alternate wire, removing the Kimi usage normalization removes the normalized values, and flipping Copilot'sincludeUsagemakesstream_optionsreappear. Declarations that change behaviour when altered are load-bearing; there is no hidden second dispatcher. - A relay does not inherit a capability from a protocol, and Copilot's catalog probe stays distinct from inference — a successful connection test attests account access and selection, not a request to the model, and nothing in production consumes it as if it did.
The sixth — explicit protocol versus static SDK conflict, and cross-protocol URL and auth — I checked myself rather than through an independent seat, because the seat assigned to it was still finishing other work. What I found: the selected protocol's adapter carries its own auth, so a declared protocol uses that adapter's credentials rather than the base provider's; base-URL normalization follows the selected adapter; and an override base URL is confined to calls belonging to the base adapter, so it cannot be applied to a different protocol's. I found no path that sends one protocol's credentials or headers to another's endpoint. That is one reviewer's reading, not the independent confirmation the other five dimensions received, and it should be weighed as such.
The author's live combinations were treated as claims to verify, not as evidence, and the parts that were reproduced were reproduced with fake HTTP rather than streaming or Desktop — an acceptance boundary the seat named rather than glossed.
This is not the independent human review CONTRIBUTING.md requires, and it is not a merge decision.
简体中文
在 exact head a14e84ef 上批准,应作者要求。test 与 label 在该 commit 上已终态转绿,无未决 review 线程,一条 [P3] 已发在行内。
本次批准的范围我明说,因为其中有一个面没有得到和其余同等的覆盖。 所点的六个面中,五个各自独立审过,均无 P0–P2:
- 协议路由是一份契约,不是各消费者各自猜。 解析器返回单一的
{adapter, wire, reasoningReplay},工厂、reasoning 选项、端点选择与工具循环都消费同一份。有一句在讨论中反复出现的说法值得纠正:「协议 A 的模型走 SDK B」并不是这里发生的事 ——apiProtocol选中的是一个会说 A 的适配器,而且类型上禁止把 Responses 线与 Chat 回放配对。 - 远端目录刷新拿不到执行切换权限,而且是通过点名拦截点证明的,不是「没搜到调用」:刷新只替换喂给
lookupModelMetadata的元数据表,而适配器、协议与端点来自构建期生成的常量,刷新装不进去。远端能移动的字段集合已被逐一列举:止于展示、思考与输出上限。 - 用户的协议覆盖能在 Fetch 之后存活,因为执行侧总是在发现行之上重新施加模型事实覆盖。
- 被删的 Kimi/Copilot 分派,每个分支都有指名的新归宿,而且是用变异而不是阅读证明的:删掉一个
protocolAdapters条目只拒该备用线、移除 Kimi 的用量归一化则归一化值消失、把 Copilot 的includeUsage翻转则stream_options重新出现。改动会改变行为的声明才是承重的;不存在隐藏的第二个分派器。 - relay 不会因协议不同而继承能力,Copilot 的目录探测与推理保持分离 —— 连接测试成功证明的是账号访问与可选性,不是对模型发过请求,而且生产侧没有任何东西把它当作后者消费。
第六个 —— 显式协议与静态 SDK 冲突、以及跨协议的 URL 与认证 —— 是我自己核的,不是由独立席位核的,因为分配到它的席位当时仍在收尾其他工作。我核到的是:选中协议的适配器自带 auth,所以声明了协议时用的是该适配器的凭据而非基础 provider 的;base URL 的归一化跟随选中的适配器;而 override base URL 被限定在属于基础适配器的 call 上,不会被套用到另一协议的适配器。我没有找到把一种协议的凭据或头部发往另一种协议端点的路径。 但这是一位评审者的判读,不是其余五个面所得到的那种独立确认,应当照此权衡。
作者列出的实测组合被当作待验证主张而非证据;被复现的那部分用的是假 HTTP,不是流式、也不是 Desktop —— 这是席位主动点名而非含糊带过的验收边界。
这不是 CONTRIBUTING.md 所要求的独立人类审查,也不是合并决定。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
jackwener
left a comment
There was a problem hiding this comment.
A follow-up to my approval, not a change to it: one more [P3], filed because it is a behaviour delta rather than a defect, and behaviour deltas are worth naming even when they turn out to be intended.
The last outstanding dimension — explicit protocol versus static SDK, and cross-protocol URL and auth — has since been closed independently, and it reached the same conclusion I did by a stronger route. The decisive evidence was a reverse mutation: changing Command Code's Messages declaration from {auth: bearer, normalizeBaseUrl: true} to {auth: api-key, normalizeBaseUrl: false} moved the real request from /…/v1/messages with Bearer to /…/messages with x-api-key, on both the send path and the connection test. A declaration that changes the wire when you change it is being consumed, which is what reading the code can only suggest. Explicit apiProtocol wins the wire; a static model override keeps supplying a concrete SDK only while its own adapter can implement that same wire; no second dispatcher believes it is still in charge.
That review also corrected its own earlier phrasing, which is worth repeating here because it is the more accurate claim: URL, auth, model and replay are consistent across the unification — not every optional header byte. The [P3] above is the one place that distinction has teeth.
简体中文
这是对我那条批准的补充,不是对它的更改:再补一条 [P3],之所以要发,是因为它是一处行为差异而不是缺陷 —— 而行为差异即便最后被判定为有意为之,也值得被点名。
最后一个未决面 —— 显式协议对静态 SDK,以及跨协议的 URL 与认证 —— 此后已被独立收口,并且以一条更强的路径得到了与我相同的结论。决定性证据是一次反向突变:把 Command Code 的 Messages 声明从 {auth: bearer, normalizeBaseUrl: true} 改成 {auth: api-key, normalizeBaseUrl: false},真实请求就从 /…/v1/messages + Bearer 变成了 /…/messages + x-api-key,发送路径与连接测试两侧都变。一个「你改它、线上就跟着变」的声明,才是被消费的声明 —— 这一点读代码只能提示,不能证明。 显式 apiProtocol 赢得 wire;静态 model override 只有在其自身适配器能实现同一 wire 时才继续提供具体 SDK;不存在第二个仍以为自己在负责的分派器。
那次复核还修正了它自己此前的措辞,这里值得重复,因为后者才是更准确的说法:在这次统一中保持一致的是 URL、auth、model 与 replay —— 不是每一个可选的 header 字节。 上面那条 [P3],正是这个区分真正起作用的地方。
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
Generated-by: Codex
Generated-by: Codex
Summary
One connection can now route models through different supported protocols. For example, Command Code's discovered Claude models use Messages while its other models keep Chat Completions, using the same connection and credential. Unannotated compatible models still use the provider default.
The runtime resolves effective model facts before choosing a concrete SDK, endpoint, thinking parameters and replay contract. User overrides survive discovery refreshes; explicit protocols can override a static model SDK when the provider supports that protocol. Dedicated relay protocol boundaries remain enforced. Static models.dev SDK data is normalized during generation, without granting metadata hot refresh new execution privileges.
Command Code discovery stamps only recognized Claude IDs as Messages; unmatched IDs remain unannotated and retain the default Chat path. This preserves unknown facts rather than claiming automatic detection of future model IDs.
Copilot explicitly opts out of the generic Anthropic beta headers in its protocol adapter declaration, preserving its pre-refactor request behavior. Other Anthropic adapters retain their existing beta headers.
Kimi and Copilot use declared protocol adapters instead of separate dispatch branches. Downstream consumers share the resolved contract, and edge-model tests use independent wire expectations instead of copying resolution logic.
Fixes #4948. Includes the Command Code catalog/UI changes from #4943; that draft remains unchanged. These are one complete integration and should not be merged as competing implementations.
Verification
Review follow-up: Core/Runtime builds and 95 focused catalog, discovery, factory and conformance tests pass. Both regressions were observed before their fixes: unmatched IDs carried an explicit Chat stamp, and Copilot SDK requests included the two generic beta opt-ins. The tests also preserve default Chat/tool continuation, user overrides through refresh, bearer auth and native Anthropic beta headers. Format, lint and ASF header checks pass. No new live-provider test was performed for these follow-up changes.
Core, Storage, Runtime and Runtime Host builds passed. Desktop typecheck and renderer build passed.
510 focused Core/Storage/Runtime tests passed. After final simplification, all 216 protocol, factory, conformance and matrix tests passed (overlaps the earlier set).
All 65 affected Host tests passed. One child-process case initially timed out under the grouped run; the isolated test and the complete grouped rerun passed.
All 15 metadata generator tests and 101 renderer architecture checker tests passed; renderer architecture validation passed against current main
b06eb02e.Format, lint, ASF headers and diff whitespace checks passed.
Independent review found a Copilot Claude probe URL regression; fixed at the account-probe boundary, reproduced with the old URL, and rechecked by the reviewer. That review reported no remaining findings in its scope.
Regression tests exercised discovery → durable catalog → effective user overrides → actual SDK requests → tool-result continuation on both Chat and Messages. Protocol override and Grok thinking tests failed before their corresponding fixes.
Live Command Code: discovered 67 models; DeepSeek completed a two-step Chat tool loop through Maka's model factory. Claude requests reached
/provider/v1/messages, but the account returned HTTP 403MODEL_NOT_IN_PLAN. No credentials were committed or copied into Maka settings.Live OpenCode Go, same account and connection: DeepSeek V4 Flash (Chat), MiniMax M2.7 (Messages), and GPT-5.6 Luna (Responses) each completed streaming tool-result continuation through ModelAdapter and the model factory, with two HTTP 200 requests, one tool execution and the exact expected final text. Streaming usage was populated.
The additional live check exposed missing Go protocol declarations: Muse Spark 1.3 defaulted to Chat and failed; an explicit Responses setting was rejected locally. Declaring Go's two supported alternative protocols fixes that rejection. After the fix, Muse Spark 1.3 with explicit Responses and Qwen 3.8 Max with explicit Messages also completed the same streaming tool loop through the normal resolver. The targeted 216 tests, Core/Runtime builds, format and lint pass; independent review found no outstanding issues in this follow-up.
OpenCode Go
/modelsreturns IDs without protocol fields. New IDs absent from bundled routing metadata still need explicit model facts or a metadata update; this change does not claim automatic protocol detection for every newly listed model.No full repository test run or full Desktop session E2E was performed. The reused catalog UI is unchanged from Add Command Code GOAT provider #4943's recorded light/dark acceptance.
Remaining validation
AI use
Tool(s) and scope: Codex implemented and tested the routing consolidation and performed an independent review. The included #4943 catalog work was originally authored with Maka and Codex. Preserve Generated-by trailers when squashing.
Checklist
Does this PR entail a change in behavior?