Conversation
Contributor
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe Alibaba Token Plan registry now pins three models to the OpenAI Responses wire for Responses inbound traffic. Chat and Anthropic inbound traffic remains on Chat. Tests validate routing, overrides, endpoint selection, provider isolation, and reasoning replay. ChangesAlibaba Token Plan Responses routing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant Client
participant handleResponses
participant WireResolver
participant AlibabaGateway
Client->>handleResponses: Submit Responses request
handleResponses->>WireResolver: Resolve model and inbound wire
WireResolver-->>handleResponses: Return openai-responses or openai-chat
handleResponses->>AlibabaGateway: Send request to selected endpoint
AlibabaGateway-->>handleResponses: Return response and reasoning content
handleResponses-->>Client: Replay or return Responses data
Suggested reviewers: Merge Risk: ⚪ Minimal · up to The Beijing model defaults remain scoped to Responses inbound traffic, with Chat and Anthropic behavior unchanged. No actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (3 skipped: 3 unsupported.)
✨ 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 |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
Contributor
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
mdwsk88
force-pushed
the
feat/alibaba-token-plan-responses-wire
branch
from
September 19, 2026 15:46
b6ae694 to
8d43ee5
Compare
7 tasks
Owner
리뷰 · 우선순위 58 / 80이 풀리퀘스트는 알리바바 토큰 플랜 베이징에서 모델 세 개의 길만 바꾼다. 세 개는 고친 자리는 깃허브에서는 아직 초안이다. 준비 체크는 0/4다. 머리 테스트 파일은 라인 - 라인 - 딥시크는 같은 고정에 메인테이너의 판단이 필요한 지점
너의 추천 제공자 전체 어댑터를 바꾸지 말고, 검증된 세 모델만 응답 입구에 고정하는 쪽이 맞다. #5097이 말한 대로 이 댓글은 grok-bot이 작성했습니다 |
3 tasks
lidge-jun
added a commit
that referenced
this pull request
Sep 19, 2026
…t-in Alibaba Token Plan (Beijing) serves its models over an OpenAI-compatible Responses API on the same /compatible-mode/v1 base and ships an official Codex integration guide on wire_api = "responses". Three models carry live end-to-end evidence on that gateway -- qwen3.8-flash, qwen3.7-plus and glm-5.3 -- covering custom tools, reasoning replay, streaming and multi-turn continuation (#5097). The issue asked for a validated opt-in or a default flip. This lands the opt-in and deliberately declines the flip. #5188 proposed pinning those three models through modelWireDefaults, which would move every existing Codex user of them onto a different upstream with no config change, and one delta is unresolved: the entry's preserveReasoningContentModels is read by the CHAT adapter, while the Responses serializer reads preserveResponsesReasoningContent, which this entry does not set. Pinned models would therefore replay with blanked reasoning content -- strictly less state than they carry on the Chat wire today. Z.AI and DeepSeek set both flags together, and their entry comments say why. Blanking is the fail-safe direction, so leaving the default alone costs nobody a working setup; setting the Responses flag on faith could 400 a continuation. The registry entry records the evidence and the open precondition, the modelAdapters reference documents the opt-in, and the new suite holds both halves: the three models resolve to openai-responses once opted in and the request actually reaches /responses rather than being flipped back by the handleResponses replay, the wire default stays Chat on every inbound, and a guard fails if a Responses wire default is ever declared for this entry without preserveResponsesReasoningContent beside it. Refs #5097 Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com>
lidge-jun
added a commit
that referenced
this pull request
Sep 19, 2026
… Responses, Token Plan opt-in, Command Code ladders (#5198) * fix(transport): fill a default User-Agent on proxy-originated provider outbound Proxy-originated provider requests -- model discovery, connection tests, the Ollama show probe, the Antigravity quota probe -- are assembled by the proxy itself, so there is no client request to inherit a User-Agent from, and the pinned Node-style transport sends none. WAF-fronted gateways answer a UA-less request with 403, which surfaces as "provider added but no models" because the pending initial-model-selection state hides every row (#5104). The outbound wrapper now fills User-Agent: opencodex when the caller names no User-Agent of its own. The check is case-insensitive and covers all three HeadersInit shapes, so registry static headers, provider headers values, and vendor client fingerprints (Copilot, Kimi CLI, Antigravity) keep their value and never gain a second User-Agent beside it. Inference traffic never reaches this wrapper: its only call sites are catalog model discovery, the management connection test, the Antigravity quota probe, and the Ollama show enrichment. Carried from #5186 with two corrections. The regression suite now also asserts the caller-owned-executor branch, which leaves the wrapper without touching the pinned transport the original tests stubbed -- a fill applied only on the pinned path would have left that branch UA-less and still 403 behind the same WAF. And the claim that a caller keeps its header-name spelling is dropped from the comment, the structure doc, and a test name: the pinned and SOCKS transports both rebuild the set through new Headers(), which lowercases every name, so the value is what survives. Closes #5104 Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com> * fix(providers): point the Volcengine Coding Plan preset at the native Responses API Ark Coding Plan documents a native Responses endpoint at /api/coding/v3/responses, but the built-in preset still shipped openai-chat, so Codex clients paid for a translation hop the gateway does not need (#5159). The preset now carries adapter openai-responses with responsesPath /responses, declares supportsServiceTier: false so an unsupported service_tier fails closed instead of reaching the gateway, and keeps the retired Chat destination as an alias so an existing row still resolves this entry's metadata. Validated Ark continuations reject the reasoning item the previous turn returned, answering 400 InvalidParameter, so the entry sets a new provider-scoped dropResponsesReasoningItems flag. It removes replayed reasoning items from continuation input without enabling orphan tool repair. The flag is lossy -- summaries, item ids and encrypted_content go with the item -- so it is documented as such in the configuration reference and an operator can set it to false. Carried from #5173 with the startup config migration removed. That migration would have rewritten every stored canonical Chat row to Responses on the next boot. It borrowed the shape of the Z.AI wire migration while inverting the property that makes that one safe: zai-responses-migration.ts gates on providerMatchesRegistryTransport and therefore only rewrites rows the router already canonicalizes at request time, which is why its comment can call itself behavior-preserving by construction. A Volcengine Chat row is not canonicalized -- volcengine-coding-plan is a preserveCustomDestination key entry, so the adapter mismatch makes routedProviderConfig return the stored row untouched -- and a version marker introduced now cannot distinguish the old default from a deliberate pre-upgrade Chat choice. The preset default therefore applies to new rows only, existing rows keep their wire, and the docs say how to switch by hand. Dropping that migration also drops the src/server/index.ts hunk, which would have taken the file from 892 to 895 lines against its 893-line ratchet cap once merged with dev. Closes #5159 Co-authored-by: cubebox <58514883+juzijia@users.noreply.github.com> * feat(registry): document and lock the Alibaba Token Plan Responses opt-in Alibaba Token Plan (Beijing) serves its models over an OpenAI-compatible Responses API on the same /compatible-mode/v1 base and ships an official Codex integration guide on wire_api = "responses". Three models carry live end-to-end evidence on that gateway -- qwen3.8-flash, qwen3.7-plus and glm-5.3 -- covering custom tools, reasoning replay, streaming and multi-turn continuation (#5097). The issue asked for a validated opt-in or a default flip. This lands the opt-in and deliberately declines the flip. #5188 proposed pinning those three models through modelWireDefaults, which would move every existing Codex user of them onto a different upstream with no config change, and one delta is unresolved: the entry's preserveReasoningContentModels is read by the CHAT adapter, while the Responses serializer reads preserveResponsesReasoningContent, which this entry does not set. Pinned models would therefore replay with blanked reasoning content -- strictly less state than they carry on the Chat wire today. Z.AI and DeepSeek set both flags together, and their entry comments say why. Blanking is the fail-safe direction, so leaving the default alone costs nobody a working setup; setting the Responses flag on faith could 400 a continuation. The registry entry records the evidence and the open precondition, the modelAdapters reference documents the opt-in, and the new suite holds both halves: the three models resolve to openai-responses once opted in and the request actually reaches /responses rather than being flipped back by the handleResponses replay, the wire default stays Chat on every inbound, and a guard fails if a Responses wire default is ever declared for this entry without preserveResponsesReasoningContent beside it. Refs #5097 Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com> * fix(command-code): let an operator ladder outrank the shipped effort table at the wire The Command Code adapter resolved its wire effort as commandCodeReasoningEfforts() ?? configuredReasoningEfforts(), so a model WITH a row in the shipped table ignored providers.command-code.modelReasoningEfforts outright while a model WITHOUT one honoured it. The catalog never agreed with that split: it advertises the picker from configuredReasoningEfforts, so an operator who widened a pinned row saw the wider ladder offered in Codex and then watched the adapter strip the rung on the way out, with no error to explain it (#5096). An operator row now resolves through the same function the catalog uses, so the picker and the wire cannot disagree, and sanitization, tier healing and learned-refusal dropping apply to it. Rows the operator never touched keep the shipped table, including a value learned by a profile refresh. The seeded copy is what makes this subtle, and it is why a plain config-first flip would have been wrong: providerConfigSeed writes the whole shipped table into every materialized preset, so "the config has a row for this model" proves nothing about who wrote it. Only a row that DIFFERS from the shipped value counts as a decision, and the comparison is against the shipped value rather than the resolved one so that a profile refresh narrowing a ladder is never mistaken for an operator edit. A regression case asserts that a preset carrying the seeded table produces byte-identical wire efforts to carrying no config at all. An operator-authorized rung also stops being silently downgraded. The effort-rejection path exists for rungs the shipped table guessed wrong, where replaying without the effort is a repair; when the operator wrote the ladder, the same replay would answer at the provider default and hide a wrong configuration behind a successful-looking response, so the upstream rejection is returned unchanged and no profile fetch is made. Scope: this closes the structural half of #5096 only. The issue also reports that seven shipped rows are narrower than the live API accepts and that 38 live models have no row. Those rows are not adopted here. The table's own provenance rules require per-row evidence, the measurements are a third party's and cannot be reproduced without a GOAT-plan key, the ids double as the router's known-ids decode source via knownModelIdsForProvider so a mis-cased id has routing consequences, and at least one proposed widening contradicts an alias this file documents from the model profile (xhigh -> max on deepseek/deepseek-v4-flash). With this change an operator can apply the measured ladders from config today, and the reporter offered to open the full 46-model table as its own PR, which is where that provenance belongs. Refs #5096 * fix(providers): finish wiring the Volcengine replay-drop flag through registry and routing Adversarial static review of the branch found the carried #5173 flag reached the adapter but not three places that must know about it. src/providers/registry/model-ids.ts classifies every ProviderRegistryEntry key through a satisfies Record<keyof ProviderRegistryEntry, ...> clause. Adding dropResponsesReasoningItems to the interface without classifying it does not compile, and the parity test rejects an entry carrying an unclassified field. The flag names no model id, so it is NONE. The compatibility behavior record described reasoning replay through preserveResponses alone, so two routes that disagree about whether replayed reasoning items are dropped produced the same behavior fingerprint and could share compatibility evidence. Dropping an item changes the continuation body, so it is now part of reasoning.replayMode. The resolved static policy projection omitted the field for the same reason and now carries it. routedProviderConfig returns early for a row whose adapter no longer matches its registry entry, which is exactly the shape this branch deliberately leaves alone: a Volcengine Coding Plan config saved on Chat. That row still reaches the Responses adapter when one model opts in through modelAdapters, and it arrived without the flag, so the continuation forwarded the reasoning item Ark answers 400 to. The flag belongs to the destination rather than to the provider-wide wire, so it is filled on that path too, from the destination matcher that already refuses templated and overridable base URLs. An explicit value still wins. Also updates the ja, ko, fr, ru and zh-TW provider guides, which still described Agent Plan as the only native Responses preset and so contradicted the English and zh-CN source, and softens an overclaiming test comment: the routing case pins what a user observes, and the absence of a startup migration is the absence of a module rather than something that case can prove. * fix(command-code): make the operator ladder override an explicit declaration Adversarial static review rejected the provenance test the previous commit used. It decided a configured row was an operator decision when that row DIFFERED from the shipped table. That is not sound: providerConfigSeed copies the whole table into every materialized preset, and enrichment (derive.ts) and routing (mergeStringArrayRecord) both keep a persisted row over the current seed. A row written by an older release therefore keeps its old value, and the moment the shipped table is corrected that untouched seed starts looking like an operator edit -- at which point it would outrank the correction AND disable the effort-rejection repair. The follow-up this issue asks for is exactly a table correction, so the misfire was not hypothetical. Provenance is now declared instead of inferred. A provider opts in with modelReasoningEffortsAuthoritative, which providerConfigSeed never writes, so its presence can only have come from a human. Without it a configured row changes nothing at the wire, seeded or stale or hand-written; with it, the ladder resolves through the same function the catalog uses and a refused rung returns the upstream error rather than being replayed without the effort. Also fixes an alias asymmetry the override made reachable. The xhigh branch aliases to max only when the ladder does not advertise xhigh, but the ultra branch aliased whenever max existed. An authoritative ladder offering ultra would have advertised ultra in the picker and quietly sent max -- the same catalog/wire disagreement this change exists to remove. No shipped row offers ultra, so the built-in table is unaffected. The regression cases follow: an authoritative ladder widens, narrows, and sends ultra as itself; a seeded map and a fully widened stale map both produce byte-identical wire efforts to carrying no config at all; and an authorized rung the upstream refuses surfaces the 400 with one generate call and no profile fetch. Refs #5096 * fix(compat): record the Command Code ladder authority in behavior identity Three independent adversarial reviews converged on the same gap: commit 6 added a flag that changes the wire effort AND suppresses the downgrade retry, but the compatibility resolver recorded only the configured ladder. Two routes with the same provider, destination, adapter, model and ladder therefore produced the same behavior fingerprint while sending different bytes and recovering differently, so evidence collected under one could admit the other. That is the same defect class commit 5 fixed for dropResponsesReasoningItems, left unfixed for its sibling. reasoning.effortsAuthoritative joins the closed behavior key set and is emitted from the production resolver, and the resolved static policy carries the flag as an operator-owned value so a policy reader no longer reports the same effective ladder for two providers that send different efforts. Also documents both new contracts in structure/providers-and-adapters.md, which owns this source area: the Coding Plan native Responses default, the lossy replay drop and why it is filled on the early-return path, why there is no startup migration, and why the Command Code ladder override is a declared flag rather than an inference. Softens the dropResponsesReasoningItems reference row, which promised the upstream sees no previous-turn reasoning state at all — the flag removes reasoning items from the forwarded input and does not touch previous_response_id. Corrects a test comment left describing the provenance inference commit 6 replaced. * docs: recount the provider preset totals from the registry and pin them Seventeen pages restate how many built-in presets ship, and sixteen of them had drifted. The English provider guide said 95 total with 79 key-based; the ja, ko, fr, ru, tr, zh-CN and zh-TW guides, all eight quickstarts including the English one, and structure/ops/docs-and-release.md still said 94 and 78. Nothing caught it, because both numbers read as plausible and no check compared either to the registry. The registry says 95. Every authKind declaration in the two entry files that compose PROVIDER_REGISTRY is a string literal, and they group as 79 key, 12 oauth, 3 local, 1 forward, which is what the English guide already claimed. The other sixteen places now say the same. AGENTS.md asks for a count to be derived from the thing it describes rather than restated, and this is the failure it describes: a preset lands, whoever adds it updates the English guide, and fifteen translated or secondary copies quietly keep the old number. A new ci-workflows check derives the total and the key-based split from PROVIDER_REGISTRY and asserts them against each page, so the next preset fails every locale at once instead of drifting. Each page is located by a locale-specific phrase rather than by its number, so rewording a sentence fails the check and asks to be re-anchored — a sentence nobody can locate is a sentence nobody is checking. * docs: keep the structure preset-split line on one line for its own check The new count check locates each page by a locale-specific phrase and asserts exactly one line carries it. Rewording the structure ops sentence pushed "documented split is" across a line break, so the anchor matched nothing and the check failed in test 2/4 and macos 1/2 — which is the behavior it was written for: a sentence nobody can locate is a sentence nobody is checking. Reflowed so the anchor, the total and the key-based split sit on one line again. --------- Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com> Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com> Co-authored-by: cubebox <58514883+juzijia@users.noreply.github.com>
3 tasks
Owner
|
Post-main triage at 7c625fc (2.60.0): #5198 delivered the verified Token Plan opt-in path, but explicitly did not enable the per-model Responses defaults proposed here. This PR is therefore not fully superseded and remains open. The remaining delta should be reviewed against the new main/dev policy authority: default pins for the specifically verified Beijing models, unchanged Chat/Anthropic inbound behavior, operator override/opt-out precedence, preservation of Responses reasoning content and replay, and the per-model effort/budget mapping. #5198 records why successful initial custom-tool output alone was not sufficient for the default flip. The international endpoint still needs its own evidence. Keep #5097 as the verification/rollout owner. Do not reimplement the already landed opt-in or fold a provider-wide default flip into this narrower change; test the final combination rather than counting the earlier opt-in verification as proof of the new default. |
3 tasks
…sponses wire Rebased onto dev after lidge-jun#5198 landed the opt-in path. The pin that lidge-jun#5198 deliberately held back is now justified with its own live evidence on the Beijing gateway (260922): - a two-turn replay round-tripping a reasoning item WITH its plaintext content array is accepted (HTTP 200, correct continuation), so preserveResponsesReasoningContent is set beside the pins — the Z.AI / DeepSeek pairing the lidge-jun#5198 comment called for; - qwen3.7-plus (the one pinned model in thinkingBudgetModels) accepts its full low/medium/high/xhigh/max ladder as reasoning.effort strings on the Responses wire, so the numeric thinking_budget translation is not needed; - qwen3.8-flash / glm-5.3 replay and effort acceptance re-confirmed. Tests: the wire-defaults suite gains the qwen3.7-plus upstream-URL case the review asked for, plus an end-to-end plaintext-replay case asserting the flag preserves (and its absence blanks) reasoning content through handleResponses. The lidge-jun#5198 opt-in suite is updated to the pinned world: chat/anthropic inbound stay on chat, the pins are exactly the three live-verified models scoped to responses inbound, and an unpinned family member still opts in/out through modelAdapters. Its flag guard now exercises the real pins.
mdwsk88
force-pushed
the
feat/alibaba-token-plan-responses-wire
branch
from
September 21, 2026 16:46
8d43ee5 to
392fa05
Compare
github-actions
Bot
requested review from
Ingwannu and
lidge-jun
as code owners
September 21, 2026 16:47
3 tasks
lidge-jun
added a commit
that referenced
this pull request
Sep 23, 2026
…web_search, artifact connect deadline, Alibaba Responses pins, Windows kiro.exe (#5673) * docs(devlog): plan bundle lane F1 (provider registry) * fix(cursor): continue composer-2.5-fast tool turns as userMessageAction composer-2.5-fast stayed on resumeAction after the 2026-08-20 capture because it answered on that path then. A 2026-09-21 proxy log shows the fast build completing a tool-result turn with no text and no tool call, the same empty stop that moved composer-2.5 to the external continuation path. Route the fast id through cursorNeedsExternalToolContinuation too. Tests that pinned fast to resumeAction now use composer-1 as the native counterexample, the live-transport screenshot case expects the Composer continuation for both 2.5 builds, and the clipped-invocation restoration case covers fast. cursor-blob.test.ts stays at its line cap. Carries #5362. Co-authored-by: Play <99410048+001005HS@users.noreply.github.com> * fix(adapters): strip the refused web_search fields on direct Meta for every Muse id Direct Meta Muse / Meta Model Responses refuses search_content_types and indexed_web_access on a plain web_search tool as a gateway schema rule, before inference, for every Muse model it serves. Dev only stripped them for the Contributor ids, so the non-Contributor default muse-spark-1.3 (both direct-Meta presets) still sent them and 400ed every Codex turn that attached web_search. The direct Meta destination is now the whole predicate, including a missing model id; the two OpenCode Zen destinations keep the Contributor-id gate because they serve nothing else. Preview tools keep their accepted shape. The contract moves to structure/transports/responses-wire-shapes.md, replacing the stale "unrelated models" wording. Carries #5314. Co-authored-by: Ivan Fokeev <2017148+ifokeev@users.noreply.github.com> * fix(images): add a connect deadline to provider artifact downloads Provider-returned image and video URLs are downloaded through connectPublicHttps and the pinned-IP transport. That path bounded the idle phases and the first byte but did not arm a separate TCP/TLS connect deadline, so a peer that never completed the handshake held the download for the full first-byte window. connectPublicHttps now forwards a 10 s DOWNLOAD_CONNECT_TIMEOUT_MS and pinnedHttpsGet accepts a per-call connectTimeoutMs with the same default; a stalled connect fails with connect_timeout. The idle timer and the 50 MiB cap are unchanged. The production-path test lives in a new sibling file registered in both layout manifests; the transport inventory records the deadline. Carries #5349. Co-authored-by: ahmedfrawelo <247386484+ahmedfrawelo@users.noreply.github.com> * feat(registry): pin live-verified Alibaba Token Plan models to the Responses wire Alibaba Token Plan (Beijing) documents a native Responses API on the same compatible-mode base and an official Codex guide on wire_api = "responses" (#5097). qwen3.8-flash, qwen3.7-plus and glm-5.3 were live-verified end to end on that gateway, so the registry now defaults them to openai-responses for Responses inbound only. Chat and Anthropic inbound keep the provider-wide Chat wire and its measured prefix-cache behaviour, and modelAdapters still wins in both directions. The entry sets preserveResponsesReasoningContent beside the pins: the Responses serializer reads that flag rather than the Chat-side preserveReasoningContentModels list, and the gateway accepted replayed plaintext reasoning content live. qwen3.7-plus sends its effort as a reasoning.effort string on this wire instead of the Chat-side numeric thinking_budget. The intl sibling stays unpinned. Tests cover resolver defaults per inbound, the upstream URL through handleResponses for all three pinned models (glm-5.3 now asserts the Responses default, not only the Anthropic path), the qwen3.7-plus effort payload, overrides, and the reasoning replay flag. The provider reference row and structure/transports/responses-wire-shapes.md describe the pins. Carries #5188. Closes #5097. Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com> * fix(oauth): fall back to kiro.exe inside the dedicated Windows Kiro-Cli folders Some Windows installs keep the CLI as kiro.exe in %LOCALAPPDATA%\Kiro-Cli or Program Files\Kiro-Cli, so forced and add-account Kiro login could not find it. After every canonical kiro-cli candidate misses, the resolver now accepts kiro.exe inside those two folders only, which are already trusted for kiro-cli.exe, and only when the base is a fully qualified drive path. A short name is never resolved from PATH or from the shared POSIX bin directories (~/.local/bin, /usr/local/bin, /opt/homebrew/bin): an unrelated kiro there, such as the Kiro IDE launcher, must not receive credential-flow arguments. Negative tests cover a short name on PATH, relative and drive-relative bases, and the POSIX directories. The provider guide and structure/providers/kiro.md state the order. Partial carry of #5000: its Unix short-name fallback is left out. Co-authored-by: 정우철 <86232509+oocheol@users.noreply.github.com> * fix(bundle-f1): fold the adversarial review nits - src/images/artifacts.ts: state the connect-deadline rationale correctly; a 60 s first-byte timer already runs before the connection exists, and the new deadline bounds TCP/TLS setup on its own. - fr, tr and zh-tw provider guides: add the Windows kiro.exe fallback and the never-a-short-name-from-PATH rule next to the existing Kiro-Cli paragraph. - devlog lane plan: drop trailing blank lines and add the delivery doc. --------- Co-authored-by: Play <99410048+001005HS@users.noreply.github.com> Co-authored-by: Ivan Fokeev <2017148+ifokeev@users.noreply.github.com> Co-authored-by: ahmedfrawelo <247386484+ahmedfrawelo@users.noreply.github.com> Co-authored-by: mdwsk88 <11055210+mdwsk88@users.noreply.github.com> Co-authored-by: 정우철 <86232509+oocheol@users.noreply.github.com>
Owner
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
preserveResponsesReasoningContent: true— the pairing the Provider discovery and model surface: outbound User-Agent, Volcengine Responses, Token Plan opt-in, Command Code ladders #5198 comment and the opt-in test guard require:preserveResponsesReasoningContentkeeps. Without the flag the serializer blanks that array, so the flag is what carries Chat-side parity onto the pinned wire.reasoning.effortstrings on the Responses wire (HTTP 200, reasoning_tokens in usage, x_billing_type=response_api for each), so the numeric thinking_budget translation the Chat wire applies is not needed on this wire.Verification
bun test tests/providers/alibaba-token-plan-wire-defaults.test.ts tests/providers/alibaba-token-plan-responses-optin.test.ts- 41 pass;tests/providers/provider-registry-parity.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/providers/deepseek-inbound-wire.test.ts tests/providers/github-copilot/github-copilot-wire-defaults.test.ts- 182 pass. The wire-defaults suite now covers all three pinned models' upstream URLs plus the unpinned control, and the reasoning-content preserve/blank pair through handleResponses.bun run typecheck- clean.bun run structure:check- clean.bun run privacy:scan- clean.bun run test:changed- 23,173 pass / 56 fail, all 56 reproduced identically on a cleanupstream/devworktree on the same host (management-validation group: identical 40-test failure set on the dev base - local node_modules/dependency skew; issue-702/compaction-routing group: identical flaky pattern, both branches pass 2 of 3 runs; remainder are the known macOS-host systemd/mtime environment families). No failure differs between this branch and the dev base.Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Closes #5097
Design discussion, live verification logs, and the rollout plan (registry opt-in, then field validation, then the entry-wide default flip) live in #5097. Deliberately out of scope here: flipping the entry-wide adapter default, the intl entry pins, and the routedProviderConfig() normalization question raised in the issue follow-up.
Review follow-ups addressed (260922):
Summary by CodeRabbit
New Features
Documentation