feat(core): send session ID to Routify endpoints - #10896
Conversation
E2E test reportBaselineThe supplied AP recreation archive contains 37 Anthropic requests to Changed behaviorFocused tests verify that HTTPS requests to all three documented Routify hosts receive the current Qwen Code session ID, that Local verification
A live post-change Routify request was not replayed because the supplied archive contains a plaintext bearer token. |
|
Re-run 2026-09-05 — gate outcome: PASSED, and the PR has since merged. This section is preserved verbatim from the first pass at
Thanks for the PR — the implementation is careful, which makes the direction question below the one worth settling first. Template ✓ — complete and bilingual, with a reviewer test plan. Problem. Real within its stated scope: ModelRouter accepts Direction — escalating to a maintainer before code review. Three things give me pause:
Size. Core paths touched: 128 production lines, 154 test lines, 62 docs lines — well under any size threshold. Not a scale concern. Approach. The how is solid: one small module, exact hostname match over HTTPS only, fail-closed on invalid URLs, session ID re-read per request (so Risk. The provider paths here match the repo's revert-history high-risk pattern ( ⏸️ Stopping before code review and deferring to @LaZzyMan — the correlation-header principle you established in the #4390 review is what this PR's docs change removes. Does the Routify-only scope supersede the operator-consent requirement, or should this be gated under 中文说明感谢贡献——实现本身很细致,因此下面的方向问题更需要先解决。 模板 ✓——完整、双语、包含评审测试计划。 问题。 在其陈述的范围内是真实的:ModelRouter 支持 方向——在进入代码审查前升级给维护者。 有三点顾虑:
规模。 触及核心路径:128 行生产代码、154 行测试、62 行文档——远低于任何规模阈值。不是规模问题。 方案。 实现方式是可靠的:单一小模块、仅 HTTPS 的精确主机名匹配、非法 URL 时 fail-closed、每次请求重新读取会话 ID(因此 风险。 此处的提供方路径命中了本仓库回滚历史的高风险模式( ⏸️ 在代码审查前暂停并转交 @LaZzyMan——您在 #4390 评审中确立的关联请求头原则正是本 PR 文档改动所删除的内容。仅 Routify 的范围是否可以豁免操作者同意要求,还是应通过 — Qwen Code · qwen3.8-max-2026-09-02 Re-run reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 4": none — no Budget gap: lines; all planned checks completed..
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/core/outbound-session-id.ts:40 — [probe] session_id skip decision leaves no debug tracedocs/design/2026-09-03-outbound-session-id-header.md:23 — [review] streaming session_id injection claimed by the design doc is pinned by no test
中文说明
未探索到全部深度（达到工具调用预算）："agent 4"：none — no Budget gap: lines; all planned checks completed.。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/core/llm-content-generator/llm-content-generator.ts:396 — [probe] embedContent injection branch's ...request.config preservation is unpinned — the sole embedding test passes no configpackages/core/src/core/llm-content-generator/llm-content-generator.test.ts:117 — [probe] generateContentStream's session_id injection is unpinned — the streaming path shares buildGenerateContentConfig only by today's wiring
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/core/llm-content-generator/llm-content-generator.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/core/src/core/llm-content-generator/llm-content-generator.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
Maintainer verification report — live Routify rig (head
|
| Claim | Result |
|---|---|
| Header reaches all three documented hosts over HTTPS | ✅ routify, routify-online, routify-pub — all injected |
| Value is the live session id | ✅ every value matches the session id the CLI recorded for that process |
| Subdomain / lookalike / suffix-lookalike / plain HTTP / unrelated provider excluded | ✅ sub.routify-pub…, routify-preview…, routify.alibaba-inc.com.evil.example, http://routify-pub…, api.openai.com — absent in all of them |
| All five provider protocols covered | ✅ OpenAI-compatible, DashScope, Anthropic, Gemini (:streamGenerateContent), Vertex (/v1beta1/publishers/google/models/…) |
| Existing headers preserved | ✅ same-lane base↔head header diff is exactly +session_id; nothing dropped, no value changed, request body byte-identical |
| Proxy-aware fetch preserved | ✅ with HTTPS_PROXY set, the proxy logs the CONNECT and the header is still injected |
Operator customHeaders.session_id is overridden, other custom headers survive |
✅ session_id = CLI value, X-Op-Marker untouched |
| Side queries and subagents inherit the same value | ✅ the memory-extraction subagent and the auto-mode security classifier requests carry the identical id |
/clear rotation, in a live TUI process (same process, same SDK client):
2. Findings
F1 — web_search talks to the same Routify host without the header. packages/core/src/tools/web-search.ts:652 builds its own new OpenAI({…}) with buildRuntimeFetchOptions(...) and no session wrapper. Its gate (classifyDashScopeBaseUrl, web-search.ts:127-152) explicitly accepts alibaba-inc.com and its subdomains, so WEB_SEARCH_BASE_URL=https://routify-pub.alibaba-inc.com/protocol/websearch/v1 is a supported configuration — and in the run above its /responses calls arrive at Routify with Authorization but no session_id, in the very same session whose chat turns carry it. This is a real model request, not "non-LLM traffic", so today the docs sentence ("Qwen Code's LLM requests … include the current Qwen Code session ID") overstates the coverage. Fix is one line (fetch: buildSessionAwareFetch(...) on that client) or a narrower doc claim.
F2 — a Routify 302 forwards the identifier to another registrable domain. The design doc mentions this in one clause; here is what it costs. When the endpoint answers 302 → https://routify.alibaba-inc.com.evil.example/…, fetch strips Authorization (its own cross-origin rule) but forwards session_id verbatim. So the redirect hop that is considered too dangerous for the credential still carries the new stable identifier. The destination check runs once, before the request; the wrapper cannot see the redirect. If that is acceptable, it deserves a sentence in the security-boundary section saying so explicitly ("a compromised or misconfigured Routify response can move the affinity id to any host"); if it is not, the wrapper needs redirect: 'manual' plus a re-check per hop.
F3 — the streaming Gemini/Vertex path is unpinned by tests (the same thing the review bot deferred in round 3, now confirmed with a mutant). Deleting the injection from generateContentStream only — delete streamConfig.httpOptions — keeps all 402 tests green. Streaming is the path production actually uses (my live Gemini lane hit :streamGenerateContent), so the primary Gemini behaviour rests on today's wiring rather than on a test. One assertion mirroring the existing non-streaming test closes it.
F4 — a trailing-dot FQDN reaches the same host but falls outside the match set (minor, fail-closed). https://routify-pub.alibaba-inc.com./… and its %2e form parse to hostname routify-pub.alibaba-inc.com., which is not in the exact-match list, so no header — while DNS resolves it to the same Routify endpoint (lane 10 above completed normally). Same spelling also drops it out of the DashScope provider's *.alibaba-inc.com rule, so that user silently loses both the affinity value and the DashScope headers. No leak risk; only affinity loss.
F5 — two lines of the new module are provably dead (nit). url.hostname.toLowerCase(): the WHATWG parser already lower-cases https: hostnames — the probe in the figure below tries uppercase, mixed case, ports, userinfo and percent-encoded forms and never produces a hostname that toLowerCase() changes. And the catch in buildSessionIdHeaders is unreachable, because requestUrl() already swallows the only throwing call. Both are harmless; both are also the two mutants my matrix could not kill for a good reason.
O1 — for all three Routify hosts the OpenAI protocol resolves to the DashScope provider, not the default one (isDashScopeProvider matches any *.alibaba-inc.com origin — visible as the X-DashScope-* headers in every Routify lane). So provider/default.ts's hook is defensive-only for today's host list; the DashScope and Anthropic hooks are the ones carrying the feature. Worth knowing when reviewing which paths need tests.
3. Measured facts for the direction question (@LaZzyMan)
Not my call to make, but these are the numbers behind it:
- There is no opt-out. Every lane above ran with
telemetry.enabled: falseandprivacy.usageStatisticsEnabled: false, and the header was still sent. The host list is a module constant; no setting, env var or flag suppresses it. - It is the first stable cross-request identifier on this path. The base run's request headers to the same endpoint contain only
User-Agent,x-stainless-*andX-DashScope-*— nothing that correlates two requests. (The Gemini path already hasx-gemini-api-privileged-user-id, but only when usage statistics are enabled.) - The existing precedent in that namespace is opt-in:
outboundCorrelation.propagateTraceContextdefaults tofalse(config.ts:2616). This PR's behaviour is on by default and explicitly outside that namespace. - Blast radius per session: every model request in a session shares one value — 2/2 in a plain headless run, and 3/3 in a tool-call run (main turn, auto-mode security classifier, follow-up turn); side queries and subagents inherit it. So it is per-session, not per-request — exactly what affinity needs, and exactly what makes it a correlator.
4. Not verified
Real Routify traffic (no access); Windows/macOS runtime lanes (rig is Linux/arm64 in Docker; the code is platform-independent); whether ModelRouter actually honours the header; behaviour of a Routify endpoint that answers with Set-Cookie/other affinity mechanisms; MCP, tool and OAuth-refresh traffic (explicitly out of scope in the PR).
5. Repro
Bundled head + base under bundle/ and bundle-base/; docker run --add-host routify{,-online,-pub}.alibaba-inc.com:127.0.0.1 … node:24-bookworm-slim, one fake-Routify HTTPS server with a private CA (NODE_EXTRA_CA_CERTS), one settings.json per lane (modelProviders + security.auth.selectedType), qwen -p … per lane, ledger = every request's raw headers. TUI lane drives the real UI under tmux and sends /clear between turns. Mutation matrix flips one decision at a time and reruns the six touched suites (vitest, 402 tests). Environment: macOS 26.6.2 host (Node v24.18.1 for the build), container Node v24.19.0, head 0c63f0f920, base 9bb2f85303.
Full test state on head: 402/402 in the six touched suites; 1272/1272 across src/core/{openaiContentGenerator,anthropicContentGenerator,llm-content-generator} + outbound-session-id + web-search.
中文说明
维护者验证报告 —— 真实 Routify 链路实测（head 0c63f0f9 vs base 9bb2f853）
我没有只看单测，而是搭了一套真实环境：把打包后的 CLI（npm run build && npm run bundle）放进 Linux 容器，把三个 ModelRouter 主机名（外加子域名、相似域名、后缀相似域名和 api.openai.com）劫持到本地 HTTPS 服务（私有 CA 签发证书）。该服务把每个进来的请求（原始请求头 + 请求体）写成 JSONL 流水账，因此下面的证据是端点真实收到的字节，不是 mock 记录。共 20 条泳道，覆盖 OpenAI 兼容、DashScope、Anthropic、Gemini、Vertex 五种协议，另有一条交互式 TUI 泳道；每条都在 PR head 和改动前 base 两个包上各跑一遍。
结论：机制与 PR 描述完全一致，A/B 干净——head 49 个请求中 26 个带 session_id，base 43 个请求 0 个带。 每个出现的值都等于该进程自己的会话 ID；没有任何既有请求头被丢弃或改写；所有反例目标都没有收到该头。我发现的是：一个行为缺口（web_search）、一个需要显式拍板的暴露面（重定向）、一个真实的测试盲区，以及两个吹毛求疵项。这些都不是注入逻辑本身的缺陷；真正卡住这个 PR 的仍是 triage 升级的方向问题，我在第 3 节给出了可量化的事实。
1. 链路上已确认的部分
| 断言 | 结果 |
|---|---|
| HTTPS 下三个文档主机都收到该头 | ✅ routify、routify-online、routify-pub 全部注入 |
| 值就是当前会话 ID | ✅ 每个值都等于该进程记录的 sessionId |
| 子域名 / 相似域名 / 后缀相似域名 / 明文 HTTP / 无关服务 均排除 | ✅ 五类反例全部没有该头 |
| 五种协议全覆盖 | ✅ OpenAI 兼容、DashScope、Anthropic、Gemini（:streamGenerateContent）、Vertex |
| 既有请求头保持不变 | ✅ 同泳道 base↔head 头部差异恰好是 +session_id，无删除无改值，请求体逐字节相同 |
| 代理链路仍然生效 | ✅ 设置 HTTPS_PROXY 后代理记录到 CONNECT，且该头照常注入 |
运维自定义的 session_id 被覆盖，其它自定义头保留 |
✅ session_id 用 CLI 的值，X-Op-Marker 原样保留 |
| 侧查询与子 agent 共享同一个值 | ✅ 记忆抽取子 agent、auto 模式安全分类器的请求都带同一个 ID |
/clear 轮换：在同一个 TUI 进程（不重启、不重建 SDK 客户端）内，第二轮的值确实换成了新的会话 ID，且两个值都与 CLI 自己记录的会话 ID 一一对应。
2. 发现
F1 —— web_search 打同一个 Routify 主机却没有该头。 packages/core/src/tools/web-search.ts:652 自己 new OpenAI({…})，只用了 buildRuntimeFetchOptions(...)，没有接会话包装；而它的准入检查（classifyDashScopeBaseUrl，web-search.ts:127-152）明确接受 alibaba-inc.com 及其子域名，所以 WEB_SEARCH_BASE_URL=https://routify-pub.alibaba-inc.com/... 是受支持的配置。实测中它的 /responses 请求带着 Authorization 到达 Routify，却没有 session_id，而同一会话的对话请求是带的。这是真实的模型请求，不属于"非大模型流量"，因此当前文档里"Qwen Code 的大模型请求都会带上会话 ID"的说法覆盖过宽。修法要么给该客户端加一行 fetch: buildSessionAwareFetch(...)，要么收窄文档表述。
F2 —— Routify 返回 302 会把标识符转发到另一个可注册域。 设计文档用一句话提了这件事，这里是它的代价：当端点回 302 → https://routify.alibaba-inc.com.evil.example/… 时，fetch 会按跨源规则剥掉 Authorization，却把 session_id 原样转发。也就是说，被认为不安全到不能带凭据的那一跳，仍然带走了这个稳定标识符。目标检查只在发出前做一次，包装器看不到重定向。如果接受这一行为，安全边界一节应当明写（"被攻陷或配置错误的 Routify 响应可以把亲和 ID 转移到任意主机"）；如果不接受，则需要 redirect: 'manual' 并逐跳重新判定。
F3 —— Gemini/Vertex 的流式路径没有任何测试钉住（正是评审 bot 第 3 轮延后的那条，现在用变异体坐实）。只在 generateContentStream 里删掉注入（delete streamConfig.httpOptions），402 个测试全绿。而流式恰恰是生产实际走的路径（实测 Gemini 泳道打的就是 :streamGenerateContent），所以主路径行为目前只靠"当前接线方式"保证。补一条对照非流式用例的断言即可闭合。
F4 —— FQDN 末尾带点的写法能打到同一台主机，却落在精确匹配集之外（次要，且是 fail-closed）。https://routify-pub.alibaba-inc.com./…（含 %2e 形式）解析出的 hostname 是 routify-pub.alibaba-inc.com.，不在列表里，因此不注入；而 DNS 会把它解析到同一个 Routify 端点（泳道 10 正常完成）。同样的写法还会掉出 DashScope provider 的 *.alibaba-inc.com 规则，于是该用户同时失去亲和值和 DashScope 请求头。没有泄漏风险，只是亲和失效。
F5 —— 新模块里有两行是死代码（吹毛求疵）。url.hostname.toLowerCase()：WHATWG 解析器对 https: 主机名已经做了小写化——探针试了大写、混合大小写、端口、userinfo、百分号编码等形式，没有任何一种能让 toLowerCase() 改变结果；buildSessionIdHeaders 的 catch 也不可达，因为唯一会抛的调用已经被 requestUrl() 吞掉了。两处都无害，也正是变异矩阵中"杀不掉且理应杀不掉"的两个变异体。
O1 —— 三个 Routify 主机在 OpenAI 协议下走的是 DashScope provider，不是 default（isDashScopeProvider 匹配任意 *.alibaba-inc.com origin——从每条 Routify 泳道都带 X-DashScope-* 头可以看出）。因此对当前的主机列表来说，provider/default.ts 的挂点只是防御性的；真正承载该功能的是 DashScope 和 Anthropic 两个挂点。评审时判断"哪些路径需要测试"要以此为准。
3. 关于方向问题的可量化事实（@LaZzyMan）
这不是我能替你拍板的事，但下面是相关数据：
- 没有关闭开关。 上述所有泳道都在
telemetry.enabled: false且privacy.usageStatisticsEnabled: false下运行，该头照样发送。主机列表是模块常量，没有任何设置、环境变量或命令行开关能抑制它。 - 它是这条链路上第一个跨请求稳定标识符。 base 运行时发往同一端点的请求头里只有
User-Agent、x-stainless-*和X-DashScope-*，没有任何能关联两次请求的东西。（Gemini 路径本来就有x-gemini-api-privileged-user-id，但仅在开启用量统计时。） - 同命名空间既有先例是显式开启：
outboundCorrelation.propagateTraceContext默认为false（config.ts:2616）。本 PR 的行为是默认开启，且明确置于该命名空间之外。 - 单会话暴露量： 同一会话内所有模型请求共用一个值——普通 headless 运行 2/2，带工具调用的运行 3/3（主轮次、auto 模式安全分类器、后续轮次）；侧查询与子 agent 继承同一个值。也就是说它是按会话而非按请求的——这正是亲和所需要的，也正是它成为关联标识的原因。
4. 未验证
真实 Routify 线上流量（无访问权限）；Windows/macOS 运行时泳道（本次靶场是 Docker 内 Linux/arm64，代码本身与平台无关）；ModelRouter 是否真的消费该头；Routify 端返回 Set-Cookie 等其它亲和机制时的行为；MCP、工具和 OAuth 刷新流量（PR 已明确列为范围外）。
5. 复现方式
head 与 base 分别打包到 bundle/ 和 bundle-base/；docker run --add-host routify{,-online,-pub}.alibaba-inc.com:127.0.0.1 … node:24-bookworm-slim，容器内起一个假 Routify HTTPS 服务（私有 CA + NODE_EXTRA_CA_CERTS），每条泳道一份 settings.json（modelProviders + security.auth.selectedType），逐条跑 qwen -p …，流水账记录每个请求的原始请求头。TUI 泳道用 tmux 驱动真实界面并在两轮之间发 /clear。变异矩阵每次只翻一个判定并重跑六个受影响测试文件（vitest，402 个用例）。环境：macOS 26.6.2 宿主（构建用 Node v24.18.1），容器 Node v24.19.0，head 0c63f0f920，base 9bb2f85303。
head 上的完整测试状态：六个受影响套件 402/402；src/core/{openaiContentGenerator,anthropicContentGenerator,llm-content-generator} + outbound-session-id + web-search 合计 1272/1272。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R4-1 streaming session_id injection unpinned (llm-content-generator.test.ts:117) — already recorded in the round-3 deferral list (review 5111238830)（与第 3 轮延后记录重复，未重新发布）
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/core/src/core/llm-content-generator/llm-content-generator.ts:109 — [probe] R3-1 buildHttpOptions' request-level httpOptions branches (destination gate, header-merge precedence) have no test; the precedence-flip mutant ships green a…
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only.
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-1 generateContentStream session_id injection unpinned (llm-content-generator.test.ts:425) — already recorded in the round-3 deferral list (review 5111238830) and re-confirmed as a duplicate in the round-4 body (review 5117011624)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/core/llm-content-generator/llm-content-generator.test.ts:186 — [review] stale test title names a removed fallback tier
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the unit suite ran locally on Linux only.
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 313 passed · 0 failed · 313 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：313 通过 · 0 失败 · 313 总计 抖动门：✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10896 deep verification —
|
| mechanism | paths | destination tested |
|---|---|---|
A — buildSessionAwareFetch() installed as the SDK's fetch |
Anthropic, OpenAI-compatible default, DashScope | the real request URL at fetch time |
B — request-level httpOptions.headers |
Gemini, Vertex (LlmContentGenerator) |
the configured baseUrl (httpOptions?.baseUrl ?? this.clientBaseUrl) |
Out of scope by choice: streaming and embedContent on the wire, a live TUI
/clear, a live Routify request, Windows/macOS, repo-wide lint and the full
test suite. Listed in Not covered.
2. Central claim — A/B table
Witness: 01-ab-session-id-none-at-base-sid-at-head.png.
Both arms ran the identical harness file against the identical fake
peer; only CORE_DIST differs. ARM=base rewrites every expectation to
"header absent", so a base-arm red would be an unexpected outcome — there were
none.
ARM=head 74 assertions passed, 0 unexpected, 0 cells never reached the peer (46 cells, 52 requests)
ARM=base 49 assertions passed, 0 unexpected, 0 cells never reached the peer (46 cells, 52 requests)
COMPARE 119 assertions passed, 0 unexpected
| cell group | driver (class actually serving it, read at runtime) | base | head |
|---|---|---|---|
| positive ×3 hosts | anthropic → AnthropicContentGenerator |
none |
SID |
| positive ×3 hosts | openai-default → direct:DefaultOpenAICompatibleProvider |
none |
SID |
| positive ×3 hosts | openai-factory → factory:DashScopeOpenAICompatibleProvider |
none |
SID |
| positive ×3 hosts | dashscope → direct:DashScopeOpenAICompatibleProvider |
none |
SID |
| positive ×3 hosts | gemini → LlmContentGenerator(gemini) |
none |
SID |
| positive ×3 hosts | vertex → LlmContentGenerator(vertex) |
none |
SID |
| uppercase host | anthropic, gemini |
none |
SID |
| flip total | 25/25 none → SID |
||
subdomain / suffix lookalike / prefix lookalike / sibling / evil.test |
anthropic, gemini |
none |
none |
trailing-dot FQDN routify-pub.alibaba-inc.com. |
anthropic, gemini |
none |
none |
plain http:// to a listed host |
anthropic, gemini |
none |
none |
userinfo https://routify-pub…@evil.test/ |
anthropic, openai-default, gemini |
refused | refused |
rotation (startNewSession() mid-client) |
all 5 paths | none |
A → B |
redirect 302 → evil.test |
anthropic, openai-default, openai-factory, gemini |
none,none |
SID,SID ← F2 |
Every cell asserts it reached the peer before "absent" is accepted, so no
negative cell is vacuous; 0 of 46 cells failed to reach it on either arm. The
userinfo cells assert the platform refusal itself (no request emitted, driver
error raised) rather than being counted as an absence.
Secondary claim 1 verified as stated. The rotation cells reuse the same
generator instance across cliConfig.startNewSession(SESSION_B) — the exact
call /clear makes (packages/cli/src/ui/commands/clearCommand.ts:98) — and
observed A → B on all five paths.
Header preservation: 43/43 cells have an identical header-name set across
arms (head minus session_id), user-agent byte-identical on all 20
positive cells, and every credential header (authorization, x-api-key)
byte-identical. This disproves the risk I went in expecting on mechanism B:
that injecting request-level httpOptions.headers would displace the
client-level User-Agent / x-gemini-api-privileged-user-id set in
createLlmContentGenerator. It does not — @google/genai 2.6.0 merges them.
Destination sweep (witness 03-destination-sweep-47-of-47-fail-closed.png):
47/47 probes of buildSessionIdHeaders() match the specification quoted from
the PR's own design doc. Fail-closed on: http://, HTTP://, file:, ws:,
subdomain, deep subdomain, …com.attacker.test, no-dot suffix lookalike,
prefix lookalike, sibling host, truncated TLD, userinfo, backslash-before-@,
listed host in fragment / query / path, embedded tab, embedded newline,
Cyrillic-о homograph, percent-encoded slash, IPv6 literal, trailing dot,
empty string, unparseable string, protocol-relative, scheme-only. Present on:
the three hosts, explicit :443, non-default port, no path, root path,
uppercase and mixed case, surrounding whitespace, URL object, Request
object. Empty session id yields {}, not an empty header. A caller-set
session_id is replaced by the active session.
3. Corrections to the PR description
Labelled as corrections to the description, not requests to change code.
- "OpenAI-compatible … request paths are covered." For the three Routify
hostnames the OpenAI-compatible path that actually runs is DashScope,
never the default provider:isDashScopeProvider()treats any
*.alibaba-inc.comhost as DashScope
(provider/dashscope.ts:231-236, "Internal Alibaba domains proxying to
DashScope-compatible APIs") anddetermineProvider()tests it first. Probed
on both arms —header-eligible hosts that reach default.ts buildClient(): 0 of 3. See F3. - The design doc's redirect sentence ("Standard fetch redirect behavior
applies after the initial destination check, so a Routify response can
forward the header by redirecting the request") describes as benign
forwarding what is in fact a defeat of the allowlist that the same document
presents as the security boundary. See F2. - Test count. The plan states "392 focused unit tests". I measured 404
passing across the six touched test files (6 files, 16.9 s). 0 failures
either way; the difference is almost certainly file selection.
4. Findings
F1 — a third-party correlation identifier lands outside the repo's default-off consent namespace, and the paragraph recording that requirement is deleted
Severity: needs a maintainer decision. Not a code defect.
The repository already has a settings namespace whose sole documented purpose
is this exact data flow, and whose documented policy is opt-in:
packages/core/src/config/config.ts:616-628— "Security-relevant settings
controlling what client-side correlation data qwen-code writes into outbound
LLM API requests. … telemetry controls data flow into the user's OWN
observability backend … The settings here control data flow OUT of the
qwen-code process and INTO third-party LLM provider request streams
(DashScope, OpenAI, Anthropic, etc.). Different recipients = different
consent decision, so a different settings tree. All values default to off
/ no propagation. Operators who want to propagate trace context … opt in
explicitly."
packages/cli/src/config/settingsSchema.ts:1440— "SECURITY-RELEVANT. …
All values default to off."
This PR deletes the paragraph in docs/developers/development/telemetry.md
that recorded how a session-ID header must be proposed:
"
X-Qwen-Code-Session-IdandX-Qwen-Code-Request-Idare not part of
this PR. They will be designed and proposed in their own follow-up PR(s)
under the sameoutboundCorrelation.*namespace, each with its own
threat model and operator-consent flow. PR #4390 review (LaZzyMan)
established the principle: 'telemetry's scope of work doesn't include sending
identifiers to LLM providers' …"
and replaces it with: "This behavior is not controlled by telemetry.enabled
or outboundCorrelation.*." The new design doc states: "The behavior is
intentionally not configurable." The module's own debug logger is named
OUTBOUND_CORRELATION (packages/core/src/core/outbound-session-id.ts:10) —
the namespace it does not join.
Reproduce:
git diff HEAD^1..HEAD -- docs/developers/development/telemetry.md
sed -n '616,650p' packages/core/src/config/config.ts
sed -n '1433,1442p' packages/cli/src/config/settingsSchema.tsBounded as usual. The technical exposure this round measured is narrow and
the code is correct: three exact hostnames, HTTPS only, no suffix or wildcard
match, 47/47 hostile shapes fail closed, no displacement of existing headers,
no opt-out needed to keep working. The finding is not "this leaks"; it is that
the repo's recorded answer to "may qwen-code write a client-side correlation
identifier into a third-party LLM request stream?" was "only behind an explicit
opt-in under outboundCorrelation.*", and this PR answers it the other way
while removing the record of the question. Whether a first-party Alibaba
ModelRouter endpoint is a "third-party recipient" for that policy is a product
call, not something this verification can settle — but it should be made by a
maintainer explicitly, and the deleted paragraph should not silently disappear.
F2 — the allowlist is enforced on the first hop only: session_id follows a 302 to a host outside it, while authorization does not
Severity: medium. Measured, 4/4 cells. Witness
04-redirect-leak-session-id-outlives-authorization.png.
The design doc names the exact-host comparison as the security boundary. It
holds on the first hop and is then bypassed by standard redirect following:
driver: anthropic provider: AnthropicContentGenerator
hop 1 (allowlisted) Host=routify-pub.alibaba-inc.com:8443 POST /rd/v1/messages
session_id = sess-A-aaaaaaaa-1111
authorization=Bearer test-key
hop 2 (302 target) Host=evil.test:8443 GET /v1/messages
session_id = sess-A-aaaaaaaa-1111
authorization=<ABSENT>
HEAD: session_id crossed to a non-allowlisted host on 4 of 4 redirect cells;
a credential header crossed on 1.
BASE: session_id crossed on 0 (the header does not exist at base).
The asymmetry is the point: the Fetch standard strips Authorization on a
cross-origin redirect, so the platform already treats a credential as
something that must not outlive the origin it was issued for. Nothing strips
session_id, because the check lives in the caller, not per hop. On the Gemini
path x-goog-api-key also crosses — that is pre-existing @google/genai +
Fetch behaviour and is not this PR's contribution; the PR's contribution is
session_id.
Reproduce (both arms; the base arm shows 0 crossings):
DIR=tmp/pr10896-verify-20260905-045351
NODE_EXTRA_CA_CERTS=$DIR/certs/ca.pem ARM=head \
CORE_DIST=$PWD/packages/core/dist/src OUT_JSON=$DIR/raw/requests-head.json \
CERTS=$DIR/certs node --import ./$DIR/dns-preload.mjs $DIR/01-wire-oracle.mjs
DIR=$DIR node $DIR/06-redirect-evidence.mjsPrecondition, stated plainly so this is not over-read: it requires the
allowlisted endpoint — or anything able to make it respond — to emit a 3xx to
an attacker-chosen host. That is a compromised or misconfigured router, or an
open redirect on any path of those three hosts. It is not triggerable by an
arbitrary third party with no foothold there, and no exploit is demonstrated
here. What the measurement does establish is that the boundary the design doc
advertises ("It does not use suffix matching, wildcards, path matching, or a
user-configurable allowlist") is a first-hop property, and the doc's
sentence about redirects reads as a feature rather than as that limitation.
I did not implement or measure a fix. The options each cost something:
redirect: 'manual'/'error' would change SDK retry and streaming semantics;
re-checking per hop needs a dispatcher rather than a fetch wrapper; accepting
it needs the trust assumption on the Routify endpoint written down as such.
F3 — default.ts's hunk cannot carry the header in production: 0 of 3 header-eligible hosts select it
Severity: low (description accuracy / dead switch).
baseUrl provider chosen by determineProvider() default.ts hunk reachable?
https://routify.alibaba-inc.com:8443 DashScopeOpenAICompatibleProvider no
https://routify-online.alibaba-inc.com:8443 DashScopeOpenAICompatibleProvider no
https://routify-pub.alibaba-inc.com:8443 DashScopeOpenAICompatibleProvider no
https://api.example.com:8443 DefaultOpenAICompatibleProvider YES (but not a Routify host)
header-eligible hosts that reach default.ts buildClient(): 0 of 3 (identical on both arms)
isDashScopeProvider() returns true for any hostname ending .alibaba-inc.com
(provider/dashscope.ts:231-236) and determineProvider() tests DashScope
first, so the two sets — "hosts that select the default provider" and "hosts
eligible for the header" — do not intersect. The same ordering also excludes the
seven subclasses that inherit buildClient().
The hunk is not broken: constructed directly it works, and the harness
proves it (direct:DefaultOpenAICompatibleProvider cells show SID on all
three hosts, and mutation M14 removing the wrapper was killed by
default.test.ts). It is defensible as consistency for a future allowlist
entry outside alibaba-inc.com. But per the AGENTS.md rule on added switches,
its read site decides the severity: today no production configuration can reach
it with a header-eligible destination, so the description's "OpenAI-compatible
… paths are covered" is carried entirely by the DashScope hunk.
Reproduce: CORE_DIST=$PWD/packages/core/dist/src ARM=head OUT_JSON=/tmp/ps.json node tmp/pr10896-verify-20260905-045351/04-provider-selection.mjs
F4 — web-search.ts builds an OpenAI client that its own gate accepts for *.alibaba-inc.com, but never wraps it
Severity: low. Static evidence (read + census over the shipped artifact), not driven end-to-end.
classifyDashScopeBaseUrl() accepts 'alibaba-inc.com' as a suffix
(packages/core/src/tools/web-search.ts:145), so
WEB_SEARCH_BASE_URL=https://routify-pub.alibaba-inc.com/… passes the tool's
own admission gate. That tool then builds a client at web-search.ts:652 with
buildRuntimeFetchOptions(...) and no fetch: wrapper.
Census over the compiled output — which modules install the wrapper:
packages/core/dist/src/core/anthropicContentGenerator/anthropicContentGenerator.js
packages/core/dist/src/core/openaiContentGenerator/provider/dashscope.js
packages/core/dist/src/core/openaiContentGenerator/provider/default.js
packages/core/dist/src/tools/web-search.js -> 0 occurrences
So an LLM chat-completions request to one of the exact three hosts carries no
session_id when it comes from web search, while every content-generator
request to the same host carries one — an operator correlating a session would
see a gap. The PR explicitly scopes out "MCP/tool requests", so this may be
intentional; the finding is that the design doc's Provider coverage list does
not mention that a fifth in-tree OpenAI client exists and can be pointed at the
same hosts.
Disproved — consequences that do not hold
Recorded so the findings above are not over-read:
- No header displacement on the Gemini path. 43/43 cells identical
header-name sets;user-agentbyte-identical; credentials byte-identical. - No hostname confusion. 47/47 destination probes fail closed on every
spoofing shape tried, including the ones that defeat naive string matching. - No plaintext exposure.
http://to a listed host yields no header on
both mechanisms. - No empty header. An empty session id yields
{}. - No credential-bearing-URL bypass. Fetch refuses the URL before the
allowlist is consulted (3/3 cells, both arms). - No non-LLM blast radius. The wrapper is installed at exactly three SDK
client constructors; MCP, tool fetches and subprocesses are untouched.
5. Mutation matrix — vacuity check on the PR's tests
Witness 02-mutation-matrix-15-killed-1-redundant.png. Suite under mutation:
the six test files the PR touches, baseline 404 passing.
| ID | guard mutated | result | failing tests |
|---|---|---|---|
| M0 | POSITIVE CONTROL: header renamed | killed | 7/404 |
| M1 | HTTPS-only requirement | killed | 1/404 |
| M2 | hostname .toLowerCase() |
SURVIVED | 0/404 |
| M3 | exact-host allowlist | killed | 6/404 |
| M4 | empty session id yields no header | killed | 1/404 |
| M5 | invalid URL fails closed | killed | 1/404 |
| M6 | wrapper preserves Request headers |
killed | 2/404 |
| M7 | wrapper preserves init headers |
killed | 2/404 |
| M8 | active session overrides caller-set session_id |
killed | 1/404 |
| M9 | runtime/proxy fetch preserved as the base | killed | 4/404 |
| M10 | Gemini falls back to client-level baseUrl |
killed | 3/404 |
| M11 | embedContent gets the header |
killed | 1/404 |
| M12 | Anthropic installs the wrapper | killed | 1/404 |
| M13 | DashScope installs the wrapper | killed | 2/404 |
| M14 | Default provider installs the wrapper | killed | 2/404 |
| M15 | COMBINATION: all three provider wrappers removed together | killed | 5/404 |
15 killed / 1 survived / 0 not applied.
The positive control M0 mutated the header name in outbound-session-id.ts and
turned 7 tests red across four files (anthropicContentGenerator,
llm-content-generator, dashscope, default), so the chosen command does
collect tests that exercise the mutated module — the survivor is about the
suite, not about the harness. M15 is the combination row: removing the three
provider wrappers together kills 5 tests, so the set is load-bearing.
One incidental observation from M0, which is why its reds are worth listing:
none of the 7 fell in outbound-session-id.test.ts, the mutant's own file.
That suite contains zero occurrences of the literal 'session_id' — every
assertion keys off the exported SESSION_ID_HEADER constant
(outbound-session-id.test.ts:34,62,63,113,137,152,168), so it is
self-referential on the wire contract and cannot detect a change to it. The
actual header name is pinned only by the three provider tests that spell
'session_id' literally. Not a defect and not a merge condition — the constant
is the right thing for those assertions to use — but it means a rename of the
wire contract is caught by the integration layer, not by the unit layer, which
is the reverse of the usual expectation.
M2 adjudicated: redundant defence, not a coverage gap. Deleting
.toLowerCase() leaves every test green — and it also leaves every behaviour
unchanged, because WHATWG URL already lowercases the host during parsing
(new URL('https://ROUTIFY-PUB.ALIBABA-INC.COM/x').hostname →
routify-pub.alibaba-inc.com). I proved this rather than inferring it: I
applied M2 to the compiled module and re-ran the 47-probe destination sweep
against it — 47/47 passed, identical output, with the uppercase and
mixed-case hosts still receiving the header. So the clause cannot decide any
outcome; nothing is untested and no code should change. (My initial
expectKilled prediction for M2 was wrong; the aggregation counts it as an
adjudicated pass and this paragraph is the audit trail.)
6. Reviewer Test Plan, walked step by step
| step | outcome |
|---|---|
1. Point each provider protocol at a Routify HTTPS endpoint, capture the request, confirm one non-empty session_id with auth/content headers unchanged |
done. 25/25 flip cells (20 positive = 6 driver constructions × 3 hosts + 2 uppercase, plus 5 rotation); exactly one session_id; 43/43 identical header-name sets; credential values byte-identical |
2. /clear, send again, confirm session_id changes |
done, with a substitution. No interactive TUI in this sandbox, so I drove the exact call /clear makes (clearCommand.ts:98 → config.startNewSession()) on the same Config and the same generator instance: A → B on all 5 paths |
3. Subdomain, lookalike alibaba-inc.com host, plain-HTTP Routify URL, unrelated provider → absent |
done. 14 wire cells (plus 3 userinfo cells asserting platform refusal) and the 47-probe sweep, all absent / fail closed |
| 4. Run the focused core tests ("392 tests") | done, count differs. 6 files, 404 passed, 0 failed, 16.9 s |
7. Not covered
- Per-commit attribution.
git rev-list HEAD^1..HEAD^2returns 1 at the
depth-2 shallow boundary while the metadata snapshot lists 7 commits, so
individual commits (notablyfix(core): align Gemini session destinationand
fix(core): address session header review feedback) were not exercised
separately. I verified the aggregateHEAD^1..HEADdiff only. - Trial merge into current
main. The snapshot'sbaseRefOid
(9bb2f853…) is not present locally (git cat-filefails), so I could not
test the merge against today's main tip. The base used isHEAD^1=
74fe3a659d, correct for a merge-ref checkout. - Streaming and embeddings on the wire. Only
generateContentwas driven.
generateContentStream(Anthropic SSE, Gemini:streamGenerateContent) and
embedContentwere not exercised against the peer;embedContent's header
injection is covered by the PR's own tests and mutation M11 killed it. - A live TUI
/clearand a live Routify request (no credentials in this
sandbox; the PR itself did not replay one either). - Windows and macOS runtimes — the PR marks both unverified.
- Repo-wide gates. Only
packages/coretsc --noEmitand the six touched
test files ran. No lint, no CLI package tests, no full suite, no integration
tests. - Base-worktree build noise. The isolated
tsc --buildin the base worktree
reported pre-existing errors inajv/ignore/qwenIgnoreParser.ts/
schemaValidator.ts. Those files are untouched by this PR, they come from
resolving the root copies of two packages that are also nested under
packages/core/node_modules, and head'stsc --noEmitin the real workspace
is clean. I did not chase them. - F2's mitigation was not implemented or measured; the finding states the
options and their costs instead. - F4 is read-and-census evidence; I did not drive the web-search tool
end-to-end.
8. Methodology
Environment: the CI merge-ref checkout (HEAD = merge, HEAD^1 = base tip
74fe3a659d, HEAD^2 = PR head 82494329cf), node:22-bookworm, Node
v22.23.2, npm ci + npm run build already completed at head. The wire oracle
(01-wire-oracle.mjs) drives the real compiled generators from
packages/core/dist/src — no stub of any code under test. Its peer is a real
https.createServer on 127.0.0.1 presenting a locally generated CA-signed
certificate whose SANs cover every test hostname, plus a plain-HTTP server for
the scheme cells, and a local GCE-metadata stub provisioned so that no ADC
lookup could reach the network. In the event the Vertex cells supplied an API
key, which the SDK reports takes precedence over project/location, so the stub
was not needed and no ADC traffic was generated. The three Routify hostnames (and the
negative shapes) are resolved to 127.0.0.1 by a --import preload that wraps
dns.lookup, scoped to the harness process; nothing system-wide was modified
(the container is not root, so /etc/hosts was never an option). TLS trusts the
local CA via NODE_EXTRA_CA_CERTS; NODE_TLS_REJECT_UNAUTHORIZED was not
disabled. A real Config is constructed per cell, so getSessionId(),
getProxy() and startNewSession() are the production implementations. The
base arm ran the byte-identical harness against packages/core built at
HEAD^1 in a scratch worktree under tmp/ (since removed): the PR touches
0 dependency manifests, so reusing the root node_modules is a clean
control — I asserted openai, @anthropic-ai/sdk and @google/genai all
realpath into the root tree (via import.meta.resolve, not require.resolve,
which throws ERR_PACKAGE_PATH_NOT_EXPORTED on these ESM-only packages), linked
only the nested third-party packages/core/node_modules (it contains no
@qwen-code/* entry, so no workspace symlink can pull head code in), and
confirmed the base dist contains 0 occurrences of buildSessionIdHeaders
/ buildSessionAwareFetch. Both arms recorded 52 requests. Raw per-arm logs,
both full peer record streams, the mutation matrix, the sweep results and the
aggregation live in raw/; every harness is a .mjs file in this directory and
reruns as printed.
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts
file packages/core/src/core/llm-content-generator/index.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/llm-content-generator/index.test.ts
file packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/llm-content-generator/llm-content-generator.test.ts
file packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/openaiContentGenerator/provider/dashscope.test.ts
file packages/core/src/core/openaiContentGenerator/provider/default.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/openaiContentGenerator/provider/default.test.ts
file packages/core/src/core/outbound-session-id.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/outbound-session-id.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: PPPPP
packages/core/src/core/llm-content-generator/index.test.ts: PPPPP
packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: PPPPP
packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: PPPPP
packages/core/src/core/openaiContentGenerator/provider/default.test.ts: PPPPP
packages/core/src/core/outbound-session-id.test.ts: PPPPP
verdict: pass
summary: 6 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: P (exit 0)
round 1 · packages/core/src/core/llm-content-generator/index.test.ts: P (exit 0)
round 1 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 1 · packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: P (exit 0)
round 1 · packages/core/src/core/openaiContentGenerator/provider/default.test.ts: P (exit 0)
round 1 · packages/core/src/core/outbound-session-id.test.ts: P (exit 0)
round 2 · packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: P (exit 0)
round 2 · packages/core/src/core/llm-content-generator/index.test.ts: P (exit 0)
round 2 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 2 · packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: P (exit 0)
round 2 · packages/core/src/core/openaiContentGenerator/provider/default.test.ts: P (exit 0)
round 2 · packages/core/src/core/outbound-session-id.test.ts: P (exit 0)
round 3 · packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: P (exit 0)
round 3 · packages/core/src/core/llm-content-generator/index.test.ts: P (exit 0)
round 3 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 3 · packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: P (exit 0)
round 3 · packages/core/src/core/openaiContentGenerator/provider/default.test.ts: P (exit 0)
round 3 · packages/core/src/core/outbound-session-id.test.ts: P (exit 0)
round 4 · packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: P (exit 0)
round 4 · packages/core/src/core/llm-content-generator/index.test.ts: P (exit 0)
round 4 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 4 · packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: P (exit 0)
round 4 · packages/core/src/core/openaiContentGenerator/provider/default.test.ts: P (exit 0)
round 4 · packages/core/src/core/outbound-session-id.test.ts: P (exit 0)
round 5 · packages/core/src/core/anthropicContentGenerator/anthropicContentGenerator.test.ts: P (exit 0)
round 5 · packages/core/src/core/llm-content-generator/index.test.ts: P (exit 0)
round 5 · packages/core/src/core/llm-content-generator/llm-content-generator.test.ts: P (exit 0)
round 5 · packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts: P (exit 0)
round 5 · packages/core/src/core/openaiContentGenerator/provider/default.test.ts: P (exit 0)
round 5 · packages/core/src/core/outbound-session-id.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE (verified at head 8249432)
Historical blocking item — fixed at head code
R2-1, the sole Critical (round-2 CHANGES_REQUESTED): buildHttpOptions used to fall through to contentGeneratorConfig.baseUrl — a destination the GoogleGenAI client never sends to — letting the exact-host boundary be blessed by a divergent constructor shape. At this head the third fallback is gone verbatim per the requested fix (destination = httpOptions?.baseUrl ?? this.clientBaseUrl), and the guard test it asked for exists and goes under the honest name: “does not infer the SDK destination from content generator config” (test file line 293), alongside three more destination-precedence tests (constructor-only, request-over-constructor, request-level Routify). Round 5 at this exact head posted zero findings. GitHub’s reviewDecision still reads CHANGES_REQUESTED only because the requesting bot’s later rounds at this head were COMMENT-class reviews, which do not flip the flag procedurally — the code it blocked has since been rewritten to its prescription.
My Critical-only scan (injection boundary)
- The allow-list is exact-host (
https:protocol + hostname lowercased +includeson the three Routify hosts), anything else returns{}; every failure path (URL parse throw, missing fetch input) is fail-closed toward no-injection. The session identifier can only reach the three first-party Routify hosts. - The fetch wrapper composes correctly on all three wiring styles: OpenAI providers and Anthropic pass
runtimeOptions?.fetchas the wrapper's base (no override loss — thefetch:key sits after the...runtimeOptionsspread deliberately); the wrapper merges Request-headers + init-headers + session_id rather than clobbering, and the no-session case short-circuits to the base fetch untouched. - Gemini path reads destination from exactly the value the client was constructed with (
clientBaseUrl=finalOptions.httpOptions?.baseUrl), so injection and delivery destination cannot diverge — the R2-1 class, closed; andcreateLlmContentGeneratorkeeps mirroringconfig.baseUrlintohttpOptions.baseUrl, with argument order to the new constructor parameters verified against the signature. - No new Critical found in the docs change or test additions; live behavior was independently confirmed against a real Routify rig by the maintainer’s verification report on the thread (head then
0c63f0f9), and the human maintainer has approved this exact head.
CI at head
17 green, zero failures; Test (ubuntu-latest) and the fleet-web-shell smoke show cancellations (the recurring contended-pool condition this week, on a PR whose core-package change the smoke lane never executes) — non-attributable, non-gating per policy.
|
Code review for the re-run @wenshao asked for. Heads up on the framing: this PR merged at 04:20:32Z, five minutes after the trigger, so nothing below gates it — the two follow-ups at the end are for a subsequent PR, not for this one. Code reviewI wrote my own baseline before opening the diff: one small module holding an exact-host + HTTPS gate, a It beats my baseline in one place I'd have got wrong. My instinct for the Gemini destination would have been What I checked beyond the diff:
No AGENTS.md violations. No Critical findings at head. The only Critical ever raised on this PR (R2-1) is verifiably gone. sequenceDiagram
participant P1 as Provider client (OpenAI, DashScope, Anthropic)
participant P2 as buildSessionAwareFetch wrapper
participant P3 as outbound-session-id gate
participant P4 as Config
participant P5 as base runtime fetch
participant P6 as Routify endpoint
P1->>P2: fetch(input, init) once per HTTP request
P2->>P3: buildSessionIdHeaders(config, input)
P3->>P3: parse URL, require https, exact host match
alt parse fails or host not allowlisted
P3-->>P2: empty headers
P2->>P5: pass through, original args untouched
else one of the three Routify hosts
P3->>P4: getSessionId() read now, not at construction
P4-->>P3: current session id
P3-->>P2: session_id header
P2->>P2: Request headers, then init headers, then session_id
P2->>P5: fetch(input, init with merged headers)
end
P5->>P6: HTTPS request
Files changed (14 of 14 shown)
TestingThis is an unattended CI run, so per the skill's rules I did not build or execute anything from this PR. Everything below is read from the PR's own CI via the API, plus the live-rig report already on this thread. The honest headline: the ubuntu unit lane never went green on the merged head. It was cancelled by its own 120-minute job timeout with
What actually happened inside the cancelled Classifying each, from the diff and the check identity rather than from anything the log claims about itself:
Why the missing Live wire evidence — the maintainer's rig, already on this thread. @wenshao's report is the strongest evidence in this thread and it is not a mock: a bundled CLI in a container with the three ModelRouter hostnames (plus a subdomain, a lookalike, a suffix-lookalike and Not verified, and why: real Routify traffic (nobody on the thread has access); whether ModelRouter actually honours the header; Windows/macOS runtime lanes (skipped in CI — the author covered macOS locally, the maintainer's rig is Linux/arm64 in Docker, and the code is platform-independent); On the sandboxed lanes: no additional lane is needed for this PR's central claim. Follow-ups for a subsequent PRNone of these blocked this one and none should have; they're what I'd file now that it's landed.
中文说明这是 @wenshao 要求 re-run 的代码审查。先说明背景：本 PR 已于 04:20:32Z 合入，就在触发命令五分钟后，因此下面的内容都不构成阻断——最后两条后续项是给后续 PR 的，不是给这个 PR 的。 代码审查。 我在看 diff 之前先写了自己的方案：一个小模块承载"精确主机 + HTTPS"准入判定、一个每次请求重读会话 ID 的 fetch 包装器（这样 有一处它比我的方案更好，而那一处我本来会写错。我对 Gemini 目的地的直觉会是回退到 diff 之外我核对的内容：
无 AGENTS.md 违规。 head 上无 Critical。本 PR 唯一被提过的 Critical（R2-1）已确证消失。 测试。 这是无人值守的 CI 运行，因此按 skill 规则我没有构建或执行任何来自本 PR 的代码。下面全部内容来自通过 API 读取的 PR 自身 CI，加上本线程已有的实测报告。 老实说的结论：ubuntu 单测泳道在被合入的那个 head 上从未变绿。 它被自身 120 分钟的 job 超时取消，当时 被取消的 分类依据是 diff 与 check 身份，而不是日志正文对自己说了什么：
为什么缺失的 链路实测证据——维护者的靶场，已在本线程。 @wenshao 的报告是本线程里最强的证据，而且不是 mock：打包后的 CLI 跑在容器里，三个 ModelRouter 主机名（外加子域名、相似域名、后缀相似域名和 未验证项及原因：真实 Routify 线上流量（线程上无人有访问权限）；ModelRouter 是否真的消费该头；Windows/macOS 运行时泳道（CI 中 skipped——作者本地覆盖了 macOS，维护者靶场是 Docker 内 Linux/arm64，代码本身与平台无关）； 关于沙箱泳道： 本 PR 的核心断言不需要额外泳道。 给后续 PR 的待办（都不构成对本 PR 的阻断，本来也不该阻断）：
— Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid implementation, direction settled by two maintainers, wire behaviour A/B'd on a real rig; the reservations are follow-ups, not defects. Stepping back. My independent proposal for this problem was the same shape the author built, so I have no "you could have done it in a third of the code" complaint — and in the one place I'd have improvised (where the Gemini injection decision reads its destination from), the author's version is correct and mine would have been the R2-1 bug. That's the strongest signal I have that this was written by someone who understood the failure modes rather than pattern-matching a header injector. Does it solve something users care about? Narrowly, yes: Routify's ModelRouter documents Is the code straightforward? Yes. 86 lines of production module, one decision function, one wrapper, one helper; every provider hook is a single line placed deliberately after the runtime-options spread. Six months from now I'd thank the author, not curse them — the allowlist is one constant array, the gate is fail-closed on every branch I could find, and the injection decision provably reads the same URL the SDK sends to. Is every change necessary? Almost. The diff is 133 production lines, 500 test lines, 66 docs lines — the test-to-production ratio is nearly 4:1, which is what five rounds of review produced, and it's the reason the mutation evidence in those rounds kept getting answered with effect-shaped tests instead of Did the results match what the PR promised? Yes, and better than the PR's own evidence did. The author could only offer focused unit tests plus a macOS-local claim (they declined to replay live traffic because the supplied archive contained a plaintext bearer token — the right call). The maintainer's rig supplied what was missing: 26/49 head requests carrying the header against 0/43 on base, exact-host boundary holding against a subdomain, a lookalike, a suffix-lookalike, plain HTTP and The one thing I'd want a maintainer to see before filing this away as clean: the ubuntu unit lane never went green on the merged head. It hit its 120-minute timeout with Am I approving because it's good or because I ran out of reasons to say no? Neither, in the end — this PR is already merged, so there is no approval for me to give and no @DragonnZhang since you have write access and merged this yourself, the four follow-ups in my Stage 2 comment are yours to file or drop: the 中文说明信心度：4/5 —— 实现扎实，方向已由两位维护者拍板，链路行为已在真实靶场上做过 A/B；我保留的意见是后续项，不是缺陷。 退一步看整体。我对这个问题独立想到的方案与作者构建的形状相同，所以我没有"本可以用三分之一的代码完成"这类意见——而在我本来会随手发挥的那一处（Gemini 注入判定从哪里读取目的地），作者的版本是对的，我的版本就会是 R2-1 那个 bug。这是我能拿出的最强信号：写这段代码的人理解失效模式，而不是照着"请求头注入器"的模式套。 它解决了用户在意的问题吗？范围很窄，但是：Routify 的 ModelRouter 文档支持用 代码是否直白？是。86 行生产模块、一个判定函数、一个包装器、一个 helper；每个 provider 挂点都是一行，且刻意放在 runtime-options 展开之后。六个月后我会感谢作者而不是骂他——allowlist 是一个常量数组，准入判定在我能找到的每条分支上都 fail-closed，注入判定读的确实是 SDK 实际发送的那个 URL。 每一处改动都必要吗？几乎。diff 是 133 行生产代码、500 行测试、66 行文档——测试与生产代码之比接近 4:1，这是五轮评审的产物，也正是各轮变异证据每次都能被"按效果"的测试回应、而不是被 结果与 PR 的承诺相符吗？相符，而且比 PR 自己给出的证据更好。作者当时只能提供定向单测加一个 macOS 本地的说法（他们拒绝重放真实流量，因为提供的压缩包含有明文 bearer token——这个判断是对的）。维护者的靶场补上了缺失的部分：head 49 个请求中 26 个携带该头，base 0/43；精确主机边界在子域名、相似域名、后缀相似域名、明文 HTTP 和 有一件事我希望维护者在把这份归档为"干净"之前看到：ubuntu 单测泳道在被合入的 head 上从未变绿。 它撞上自身 120 分钟超时，当时 我批准是因为它好，还是因为我找不到拒绝的理由？最终两者都不是——本 PR 已经合入，所以没有批准可给，也没有我可以正当提交的 @DragonnZhang 由于你有 write 权限且是自己合入的，我 Stage 2 评论里的四条后续项由你决定立项或放弃： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
5 participants
What this PR does
Adds the active Qwen Code session ID as the
session_idrequest header for HTTPS LLM calls to the three ModelRouter endpoints documented by Routify:routify.alibaba-inc.com,routify-online.alibaba-inc.com, androutify-pub.alibaba-inc.com.The session value is resolved for every request, so
/clearand other session changes are reflected without rebuilding the model client. OpenAI-compatible, DashScope, Anthropic, Gemini, and Vertex request paths are covered. Existing request headers and proxy-aware fetch behavior are preserved.The destination check is intentionally fixed and exact. Subdomains, lookalike hosts, plain HTTP URLs, and all other providers do not receive the header.
Why it's needed
Routify's ModelRouter accepts
session_idfor session affinity, traffic marking, and troubleshooting. The supplied AP recreation log contains 37 Anthropic requests toroutify-pub.alibaba-inc.com, but none includes any affinity header documented by ModelRouter. Reusing Qwen Code's existing session ID lets ModelRouter keep requests from one CLI session correlated without introducing another identifier.Reviewer Test Plan
How to verify
session_idheader while authorization and content headers remain unchanged./clearand send another request. Confirm thatsession_idchanges to the new active session value.alibaba-inc.comhostname, a plain HTTP Routify URL, or an unrelated provider. Confirm thatsession_idis absent.Evidence (Before & After)
N/A (no UI change). Before: all 37 requests in the supplied Routify AP recreation log lacked a documented affinity header. After: focused tests confirm the header is added only to the three exact HTTPS Routify destinations and follows session rotation. A live request was not replayed because the supplied archive contains a plaintext bearer token.
Tested on
Environment (optional)
Local Node.js 24.18.0, no sandbox.
npm run build,npm run typecheck, targeted ESLint, and 392 focused unit tests passed.Risk & Scope
Linked Issues
Related to #4384.
中文说明
本 PR 做了什么
为发往 Routify 文档列出的三个 ModelRouter 端点的 HTTPS 大模型请求增加当前 Qwen Code 会话 ID，使用的请求头是
session_id。三个端点分别是routify.alibaba-inc.com、routify-online.alibaba-inc.com和routify-pub.alibaba-inc.com。每次请求都会读取当前会话值，因此执行
/clear或发生其它会话切换后，无需重建模型客户端即可使用新的值。OpenAI 兼容、DashScope、Anthropic、Gemini 和 Vertex 请求链路均已覆盖，同时保留已有请求头和支持代理的 fetch 行为。目标地址检查采用固定的精确匹配。子域名、相似域名、普通 HTTP 地址以及其它所有模型服务都不会收到该请求头。
为什么需要这个改动
Routify 的 ModelRouter 支持使用
session_id做会话亲和、流量染色和问题排查。用户提供的 AP 复现日志中有 37 个发往routify-pub.alibaba-inc.com的 Anthropic 请求，但均未携带 ModelRouter 文档列出的任何亲和请求头。复用 Qwen Code 已有的会话 ID，可以在不新增标识符的情况下，让 ModelRouter 关联同一个 CLI 会话中的请求。Reviewer 测试计划
如何验证
session_id，同时认证头和内容相关请求头保持不变。/clear创建新会话后再次发送请求，确认session_id已更新为新的当前会话值。alibaba-inc.com域名、普通 HTTP Routify 地址或无关模型服务，确认请求中没有session_id。证据（改动前后）
不适用（无 UI 改动）。改动前：用户提供的 Routify AP 复现日志中 37 个请求均未携带文档所列亲和请求头。改动后：定向测试确认仅三个精确的 HTTPS Routify 目标会添加该请求头，并且会话切换后值会更新。由于用户提供的压缩包中含有明文 bearer token，没有重放真实线上请求。
测试平台
环境（可选）
本地 Node.js 24.18.0，无沙箱。
npm run build、npm run typecheck、定向 ESLint 以及 392 项定向单元测试均通过。风险与范围
关联 Issue
关联 #4384。