Skip to content

fix(core): emit OpenRouter's reasoning disable when thinking is off - #9758

Merged
yiliang114 merged 2 commits into
QwenLM:mainfrom
yiliang114:fix/issue-9757-openrouter-classifier-thinking
Aug 25, 2026
Merged

fix(core): emit OpenRouter's reasoning disable when thinking is off#9758
yiliang114 merged 2 commits into
QwenLM:mainfrom
yiliang114:fix/issue-9757-openrouter-classifier-thinking

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

When reasoning is disabled (includeThoughts: false on a request, e.g. the AUTO-mode classifier's stage-1 side query, or config-level reasoning: false) and the endpoint is OpenRouter (openrouter.ai or a subdomain), the OpenAI-compatible pipeline now emits OpenRouter's native thinking-disable parameter reasoning: { enabled: false } on the wire request. The detection is a hostname-only helper (isOpenRouterHostname) mirroring the existing isDeepSeekHostname precedent, and the emission sits in the reasoningDisabled branch after the unconditional reasoning / reasoning_effort strip, because the provider buildRequest hook runs before that strip and anything emitted there would be removed again.

Why it's needed

Fixes the failure chain in #9757: the classifier's stage 1 is a structured side query that forces a respond_in_schema tool call (tool_choice: 'required', forced-tool mapping from #6929) with a 256-token budget and thinkingConfig: { includeThoughts: false }. On OpenRouter the thinking-disable only rendered into shapes the gateway ignores — chat_template_kwargs.enable_thinking (the vLLM/SGLang shape) for qwen-family models — and the pipeline then stripped the reasoning object, which is exactly OpenRouter's native thinking knob. Thinking stayed enabled, the model spent the whole 256-token budget on reasoning and never emitted the forced tool call, generateJson returned {}, schema validation failed, and the classifier fail-closed with Classifier stage 1 unavailable, forcing manual approval for every AUTO-mode action. This is a residual gap next to #6929 (which added the forced tool call + budget), not a duplicate: #6929 works wherever the thinking-disable actually lands, but OpenRouter was missing from the disable-rendering logic. It is also disjoint from #9590 (still open), which covers DeepSeek/GLM/Kimi thinking controls on the WebShell UI and does not touch the OpenRouter classifier path.

Reviewer Test Plan

How to verify

Detector-level reproduction and regression coverage live in packages/core/src/core/openaiContentGenerator/pipeline.test.ts (search OpenRouter): with baseUrl: https://openrouter.ai/api/v1 and thinkingConfig: { includeThoughts: false }, the captured chat.completions.create body is asserted to carry reasoning: { enabled: false }. On unpatched main these tests fail with reasoning: undefined; with this PR they pass. Negative guards: no reasoning field leaks to the official OpenAI endpoint, to non-OpenRouter OpenAI-compatible gateways (vLLM/SGLang), or to lookalike hostnames (openrouter.ai.evil.com), and thinkingMandatory models on OpenRouter stay exempt. Existing behavior preserved: DashScope tiered (reasoning_effort: 'none') and boolean (enable_thinking: false) branches, vLLM/SGLang chat_template_kwargs, and DeepSeek hostname thinking: { type: 'disabled' } are untouched and covered by their existing tests. Run: cd packages/core && npx vitest run src/core/openaiContentGenerator/pipeline.test.ts — 167 passed locally, plus npm run typecheck (clean).

Evidence (Before & After)

N/A — non-user-visible wire-shape fix; evidence is the unit-test red/green pair described above (3 emission tests failed before the fix, all 167 pass after).

Tested on

OS Status
🍏 macOS N/A
🪟 Windows N/A
🐧 Linux

Environment (optional)

Unit tests only (vitest + tsc --noEmit in packages/core); no live OpenRouter account was used.

Risk & Scope

  • Main risk or tradeoff: reasoning: { enabled: false } is now sent on every reasoning-disabled request to OpenRouter hostnames regardless of model family. That is intentional — reasoning is an OpenRouter provider-level parameter the gateway routes to whatever model supports it (unlike enable_thinking, a qwen-specific wire field that the existing code deliberately family-gates to avoid leaking upstream) — but it means OpenRouter-side handling of the parameter is the only validation so far.
  • Not validated / out of scope: live end-to-end capture against OpenRouter (no API key in the test environment); qwen-family models on OpenRouter still also receive the harmless chat_template_kwargs.enable_thinking=false field the gateway ignores — left in place to keep the diff minimal; no changes to classifier budgets or stage-2 behavior.
  • Breaking changes / migration notes: none — additive wire field gated on hostname, only when reasoning is already being disabled.

Linked Issues

Fixes #9757

Related: #6929 (forced respond_in_schema tool call + 256-token budget for side queries — this PR fixes the OpenRouter gap its failure comment describes), #6791 (same failure family), #9590 (open; DeepSeek/GLM/Kimi WebShell thinking controls, disjoint scope)

中文说明

本 PR 做了什么

当推理被禁用时(请求级 includeThoughts: false,例如 AUTO 模式分类器的 stage-1 side query;或配置级 reasoning: false),如果端点是 OpenRouter(openrouter.ai或其子域名),OpenAI 兼容管道现在会在出站请求上带上 OpenRouter 原生的思考关闭参数 reasoning: { enabled: false }。检测是一个仅看 hostname 的辅助函数(isOpenRouterHostname),对齐现有 isDeepSeekHostname 先例;补发逻辑放在 reasoningDisabled 分支里对 reasoning / reasoning_effort 的无条件剥离之后,因为 provider 的 buildRequest hook 先于剥离运行,在那里发出的字段会被再次删掉。

为什么需要

修复 #9757 的失败链:分类器 stage 1 是一个结构化 side query,强制 respond_in_schema 工具调用(tool_choice: 'required',来自 #6929 的强制工具映射),预算 256 token,并带 thinkingConfig: { includeThoughts: false }。在 OpenRouter 上,思考关闭只渲染成网关忽略的格式——qwen family 模型拿到 chat_template_kwargs.enable_thinking(vLLM/SGLang 形状)——随后管道又把 reasoning 对象剥掉,而这恰恰是 OpenRouter 原生的思考开关。于是思考仍然开启,模型把 256 token 预算全花在推理上,永远发不出强制工具调用,generateJson 返回 {},schema 校验失败,分类器 fail-closed 报 Classifier stage 1 unavailable,AUTO 模式每个操作都转手动审批。这是 #6929 之后的残留缺口而非重复:#6929 在思考关闭能真正生效的端点上是有效的,只是关闭逻辑的渲染里漏了 OpenRouter。与仍 open 的 #9590 也不重叠——那个 PR 覆盖 WebShell UI 上 DeepSeek/GLM/Kimi 的思考控件,不涉及 OpenRouter 分类器路径。

审阅者测试计划

如何验证

检测器级复现与回归保护在 packages/core/src/core/openaiContentGenerator/pipeline.test.ts(搜 OpenRouter):baseUrl: https://openrouter.ai/api/v1 + thinkingConfig: { includeThoughts: false } 时,断言捕获到的 chat.completions.create 请求体带 reasoning: { enabled: false }。在未修复的 main 上这些测试以 reasoning: undefined 失败;本 PR 下通过。负向保护:官方 OpenAI 端点、非 OpenRouter 的 OpenAI 兼容网关(vLLM/SGLang)、仿冒域名(openrouter.ai.evil.com)都不会多收 reasoning 字段;OpenRouter 上的 thinkingMandatory 模型保持豁免。既有行为保留:DashScope 分层(reasoning_effort: 'none')与布尔(enable_thinking: false)分支、vLLM/SGLang 的 chat_template_kwargs、DeepSeek hostname 的 thinking: { type: 'disabled' } 均未改动且由既有测试覆盖。运行:cd packages/core && npx vitest run src/core/openaiContentGenerator/pipeline.test.ts——本地 167 个全部通过,另有 npm run typecheck 无错误。

前后证据

N/A——非用户可见的线上请求形状修复;证据是上述单元测试的修复前红/修复后绿(3 个补发断言修复前失败,修复后 167 个全过)。

测试环境

OS 状态
🍏 macOS N/A
🪟 Windows N/A
🐧 Linux

环境(可选)

仅单元测试(packages/core 里的 vitest + tsc --noEmit);未使用真实 OpenRouter 账号。

风险与范围

  • 主要风险或取舍:现在所有发往 OpenRouter 域名的禁用推理请求都会带 reasoning: { enabled: false },不限模型族。这是有意为之——reasoning 是 OpenRouter 的 provider 级参数,网关会把它路由给支持的模型(不同于 enable_thinking,那是 qwen 专属线上字段,现有代码刻意按模型族门控以防泄漏到上游)——但这也意味着目前对 reasoning 参数的验证只到 OpenRouter 文档层面。
  • 未验证 / 超出范围:对 OpenRouter 的真实端到端抓包(测试环境无 API key);OpenRouter 上的 qwen family 模型仍会同时带上网关忽略的 chat_template_kwargs.enable_thinking=false 字段,为保持最小 diff 保留;分类器预算与 stage-2 行为均未改动。
  • 破坏性变更 / 迁移说明:无——仅在 hostname 命中且推理本就要关闭时附加一个线上字段。

关联 Issue

Fixes #9757

相关:#6929(side query 的强制 respond_in_schema 工具调用 + 256 token 预算——本 PR 修复的正是其失败评论描述的 OpenRouter 缺口)、#6791(同族问题)、#9590(open;DeepSeek/GLM/Kimi WebShell 思考控件,范围不重叠)

The AUTO-mode permission classifier's stage-1 side query forces a
respond_in_schema tool call (tool_choice: 'required') with a 256-token
budget and includeThoughts: false. On OpenRouter endpoints the
thinking-disable rendered only into shapes the gateway ignores
(chat_template_kwargs.enable_thinking for qwen-family models), and the
pipeline's unconditional strip then removed the `reasoning` object —
OpenRouter's native thinking knob. Thinking stayed on, the model spent
the whole budget on reasoning, never emitted the tool call, and the
classifier fail-closed with "Classifier stage 1 unavailable" (QwenLM#9757).

Mirror the isDeepSeekHostname precedent: hostname-gated detection
(openrouter.ai / *.openrouter.ai) and emit `reasoning: { enabled: false }`
in the reasoningDisabled branch after the strip — the provider
buildRequest hook runs before the strip, so emitting earlier would be
removed again. Applied endpoint-wide rather than qwen-family-gated:
`reasoning` is an OpenRouter provider-level parameter, unlike
`enable_thinking`, which is a qwen wire field that leaks upstream on
non-qwen routings. thinkingMandatory models stay exempt; DashScope
(both shapes), vLLM/SGLang, DeepSeek hostname, and the official OpenAI
endpoint are untouched.

Repro + regression coverage added in pipeline.test.ts (red before the
fix, green after).
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on e782cf1 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— e782cf1 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed bug, not theory — #9757 is open with a concrete reproduction (OpenRouter + AUTO mode → Classifier stage 1 unavailable, every action falls back to manual approval), labeled type/bug / priority/P2. I traced the failure chain in the code and it matches the description: the stage-1 side query (permissions/classifier.ts) runs with a 256-token budget and includeThoughts: false, and on OpenRouter the thinking-disable never renders into a shape the gateway honours — the model spends the whole budget on reasoning, the forced respond_in_schema tool call never ships, and the classifier fail-closes.

Direction: aligned. This completes the per-provider thinking-disable matrix — DashScope, vLLM/SGLang, and DeepSeek hostnames each already emit their own native disable shape; OpenRouter was the missing one. The placement problem is real too: the unconditional reasoning strip in the pipeline runs after the provider buildRequest hook, so a hook-side emission would be deleted again.

Size: 65 production lines (pipeline.ts +25, new provider/openrouter.ts +40) plus 278 test lines. Well under any escalation threshold.

Approach: minimal and on-pattern — a hostname detector mirroring the existing isDeepSeekHostname precedent, one emission block placed after the strip, thinkingMandatory models exempt. No unrelated changes in the diff. Keeping the (gateway-ignored) chat_template_kwargs.enable_thinking=false for qwen models on OpenRouter to keep the diff small is a reasonable call.

Risk: openaiContentGenerator is on this repo's high-risk path list (correlated with post-merge reverts), so this gets full CI evidence before approval — see the Stage 2 table.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的 bug,不是理论问题——#9757 仍 open,有具体复现(OpenRouter + AUTO 模式 → Classifier stage 1 unavailable,每个操作都回退到手动审批),标签为 type/bug / priority/P2。我在代码里追了失败链,与描述一致:stage-1 side query(permissions/classifier.ts)预算 256 token 且 includeThoughts: false,而在 OpenRouter 上思考关闭从未渲染成网关认的形状——模型把整个预算花在推理上,强制的 respond_in_schema 工具调用永远发不出去,分类器 fail-closed。

方向:对齐。这补全了各供应商的思考禁用矩阵——DashScope、vLLM/SGLang、DeepSeek hostname 各自已有原生禁用形状,缺的就是 OpenRouter。放置位置的问题也是真实的:管道里无条件的 reasoning 剥离发生在 provider buildRequest hook 之后,在 hook 里补发会被再次删掉。

规模:65 行生产代码(pipeline.ts +25,新文件 provider/openrouter.ts +40),另有 278 行测试,远低于任何升级阈值。

方案:最小且符合既有模式——镜像现有 isDeepSeekHostname 先例的 hostname 检测器、放在剥离之后的一个补发分支、thinkingMandatory 模型豁免。diff 里没有无关改动。为保持最小 diff 而保留 OpenRouter 上 qwen 模型那个被网关忽略的 chat_template_kwargs.enable_thinking=false,是合理取舍。

风险:openaiContentGenerator 在本仓库的高风险路径清单上(与合并后 revert 相关),因此批准前要求完整 CI 证据——见 Stage 2 的表格。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at e782cf1b3abb689d1751f241e8df1ac86d4ddcb2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Code review

The implementation matches what I'd have proposed before reading the diff — a hostname detector mirroring isDeepSeekHostname, one emission placed after the strip — and it goes further than my baseline (config-level reasoning: false and non-qwen models are covered too). Verified against the base code:

  • Placement is correct. provider.buildRequest runs before the reasoningDisabled branch; the unconditional reasoning / reasoning_effort strip sits earlier in that same branch, and the new OpenRouter emission comes after it, so the field survives to the wire. The later thinkingMandatory cleanup block never touches reasoning, and the emission itself is skipped for thinkingMandatory models — no interaction bug between the two.
  • Hostname matching is exact and lookalike-safe: openrouter.ai or a .openrouter.ai subdomain via new URL(...), invalid URLs treated as non-OpenRouter. Same shape as isDeepSeekHostname and consistent with openRouterProvider.ownsModel in the presets module.
  • Provider-level gating is the right call for a gateway parameter (unlike the family-gated enable_thinking), and the tests pin it: assertions read the body captured at chat.completions.create, so an emission placed before the strip — or one leaking to other endpoints — would fail them. Seven new cases cover the positive paths plus thinkingMandatory exemption, non-OpenRouter gateways, lookalike hostnames, and the official OpenAI endpoint.
  • No blockers, no convention violations. One non-blocking note: qwen-family models on OpenRouter now carry both the gateway-ignored chat_template_kwargs.enable_thinking=false and the new reasoning field — left in place intentionally for a minimal diff, and harmless.

Request-build flow, since after-the-strip placement is the subtle invariant here:

sequenceDiagram
    participant P1 as pipeline.buildRequest
    participant P2 as provider.buildRequest hook
    participant P3 as reasoningDisabled branch
    participant P4 as chat.completions.create
    P1->>P2: build base request
    P2-->>P1: providerRequest with hook fields
    P1->>P3: emit per-provider disable shapes
    Note over P3: qwen branches first, then strip reasoning and reasoning_effort
    P3->>P3: emit reasoning enabled=false after the strip (new)
    P3->>P4: final wire body
Loading

CI evidence

The PR's own CI on the reviewed commit: security checks, precheck, and desktop shell builds are green; the Linux unit suite — this repo's PR test gate — is still running. The macOS/Windows Test jobs are merge-queue-only per ci.yml, so their skip at the PR stage is expected, not a gap. No polling here — the table below is updated by the finalize job once CI settles.

Final CI results for e782cf1 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle the remaining behavioural gap: @qwen-code /verify — the unit tests capture the request body at the mocked SDK boundary, so a mock-free wire-oracle run would prove reasoning: { enabled: false } actually lands on the socket, and only for OpenRouter hostnames. Whether OpenRouter's gateway honours the parameter end-to-end still needs a live key, which CI doesn't have — that part stays on the author's risk note, not verified here.

中文说明

代码审查

实现与我读 diff 前独立提出的方案一致——镜像 isDeepSeekHostname 的 hostname 检测器、放在剥离之后的一处补发——并且比我的基线更完整(配置级 reasoning: false 和非 qwen 模型也覆盖了)。对照基线代码核实:

  • 放置位置正确。 provider.buildRequest 先于 reasoningDisabled 分支运行;无条件的 reasoning / reasoning_effort 剥离在同一分支更早处,新的 OpenRouter 补发在其之后,因此字段能保留到出站请求。后面的 thinkingMandatory 清理块从不触碰 reasoning,而补发本身对 thinkingMandatory 模型跳过——两者之间没有交互 bug。
  • hostname 匹配精确且防仿冒:通过 new URL(...) 精确匹配 openrouter.ai.openrouter.ai 子域,非法 URL 按非 OpenRouter 处理。与 isDeepSeekHostname 同形,也与 presets 模块里 openRouterProvider.ownsModel 一致。
  • 按供应商(而非模型族)门控是正确的,因为这是网关级参数(不同于按族门控的 enable_thinking),且测试钉住了这一点:断言读取 chat.completions.create 捕获的请求体,若补发放错位置(在剥离之前)或泄漏到其他端点都会失败。7 个新用例覆盖正向路径、thinkingMandatory 豁免、非 OpenRouter 网关、仿冒域名和官方 OpenAI 端点。
  • 无阻塞项、无规范违规。一个非阻塞说明:OpenRouter 上的 qwen 族模型现在同时带着被网关忽略的 chat_template_kwargs.enable_thinking=false 和新的 reasoning 字段——为保持最小 diff 有意保留,无害。

CI 证据

被审提交上 PR 自己的 CI:安全检查、precheck、桌面壳构建均绿;Linux 单元测试(本仓库 PR 的测试门禁)仍在运行。macOS/Windows 的 Test 作业按 ci.yml 只在合并队列跑,PR 阶段跳过属预期,不是缺口。此处不轮询——CI 落定后 finalize 作业会更新上方表格。

沙箱验证可以补上剩余的行为缺口:@qwen-code /verify —— 单测在 mock 的 SDK 边界捕获请求体,一次免 mock 的 wire-oracle 运行可以证明 reasoning: { enabled: false } 真正落到 socket 上,且只对 OpenRouter 域名生效。至于 OpenRouter 网关端到端是否认这个参数,仍需真实 API key,CI 环境没有——这部分留在作者的风险说明里,本次未验证。

Qwen Code · qwen3.8-max

Reviewed at e782cf1b3abb689d1751f241e8df1ac86d4ddcb2 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal fix that closes a real per-provider gap with regression-pinning tests; the missing point is untested live-gateway behaviour, not anything in the diff.

Stepping back: the problem is real and user-visible — AUTO mode is effectively unusable on OpenRouter — and the fix is exactly the missing piece, not a new mechanism but one more branch in an established pattern. Every line in the diff serves the stated goal; no drive-by edits. My independent proposal before reading the code was essentially what landed, minus the config-level and non-qwen coverage, which are genuine improvements. The one subtle invariant — emit after the strip, not in the provider hook — is the kind of thing that breaks silently if gotten wrong, and the tests would catch exactly that breakage, which is what I want to see from a change in this file.

Reservations, none blocking: live OpenRouter behaviour is unverified for lack of an API key (OpenRouter's documented reasoning parameter plus the unit tests are the evidence so far), and the Linux unit suite is still running — approval waits for it.

Approval deferred until CI lands green on e782cf1b3abb689d1751f241e8df1ac86d4ddcb2.

中文说明

置信度:4/5 —— 干净、最小的修复,用带回归保护的测试补上了真实的供应商缺口;扣掉的一分是未经真实网关验证的行为,而不是 diff 本身有任何问题。

退一步看:问题真实且用户可感——AUTO 模式在 OpenRouter 上实际不可用——而修复恰好是缺的那一块,不是新机制,只是既有模式里再加一个分支。diff 里每一行都服务于既定目标,没有顺手改动。我在读代码前独立提出的方案基本就是落地的方案,只是少了配置级和非 qwen 的覆盖,而那两点是真正的改进。唯一微妙的不变量——在剥离之后补发、而不是在 provider hook 里——正是那种搞错了就会静默坏掉的点,而测试恰好能捕获那种坏法,这是我对这个文件的改动想看到的。

保留意见,均不阻塞:因缺少 API key,OpenRouter 真实行为未验证(目前的证据是 OpenRouter 文档化的 reasoning 参数加单元测试);Linux 单元测试仍在运行——批准等它落定。

Qwen Code · qwen3.8-max

Reviewed at e782cf1b3abb689d1751f241e8df1ac86d4ddcb2 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship — CI landed green after the review. ✅

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

Test Plan (not a blocker): 167 passed — this review observed 20918, 1685, 23284, 1653, 495, 4125, 595 passed.

中文说明

已审查——无阻断问题。 建议见行内评论。

Test Plan(非阻断):167 passed — this review observed 20918, 1685, 23284, 1653, 495, 4125, 595 passed

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread packages/core/src/core/openaiContentGenerator/pipeline.ts
Comment thread packages/core/src/core/openaiContentGenerator/pipeline.ts
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Closeout update: added colocated OpenRouter hostname coverage, pinned that the OpenRouter disable shape is only emitted on the disabled-thinking path, and documented the OpenRouter native disable shape. Verified with npx vitest run src/core/openaiContentGenerator/provider/openrouter.test.ts src/core/openaiContentGenerator/pipeline.test.ts, Prettier check, and git diff --check. CI is pending on the new head.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI failing: Test (macos-latest, Node 22.x), Post Coverage Comment, Desktop Shell (${{ matrix.os }}), Test (windows-latest, Node 22.x), Integration Tests (CLI, No Sandbox), fallback-comment, review-pr, delay-automatic-review, resolve-pr, ack-review-request, authorize, precheck-pr / precheck. Reviewed.

中文说明

⚠️ 已从批准降级为评论:CI failing: Test (macos-latest, Node 22.x), Post Coverage Comment, Desktop Shell (${{ matrix.os }}), Test (windows-latest, Node 22.x), Integration Tests (CLI, No Sandbox), fallback-comment, review-pr, delay-automatic-review, resolve-pr, ack-review-request, authorize, precheck-pr / precheck。 已审查。

— qwen3.8-max via Qwen Code /review (v0.22.0)

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

Not explored to full depth (tool budget reached): "agent 1a": running the 8 new pipeline.test.ts tests end-to-end — blocked because this shared worktree lacks a complete built dist/ (vitest globalSetup prerequisite), and….

Test Plan (not a blocker): 167 passed — this review observed 20925, 1685, 23284, 1653, 495, 4125, 595 passed.

中文说明

未发现问题。LGTM!✅

未探索到全部深度(达到工具调用预算):"agent 1a"running the 8 new pipeline.test.ts tests end-to-end — blocked because this shared worktree lacks a complete built dist/ (vitest globalSetup prerequisite), and…

Test Plan(非阻断):167 passed — this review observed 20925, 1685, 23284, 1653, 495, 4125, 595 passed

— qwen3.8-max via Qwen Code /review (v0.22.0)

@yiliang114

yiliang114 commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator Author

Local runtime verification: PASS (wire-level request-body evidence)

  • Revision: 8030cdeddc7b584021fc00be2f2fe793b6355e75
  • Runtime: node packages/cli/dist/index.js --prompt ... --openai-logging --openai-logging-dir ... (captures the exact OpenAI request body the pipeline puts on the wire), with a temporary fastModel entry whose baseUrl is https://openrouter.ai/api/v1 and an invalid key — the resulting 401 Missing Authentication header proves the request actually reached OpenRouter; what matters here is the captured request body.
# Item Observed wire body Verdict
1 includeThoughts: false request (side-query auto-memory-recall) to an openrouter.ai host "reasoning": {"enabled": false} present
2 Same entry, thinking not disabled (managed subagent side query) "reasoning": {"effort": "medium"} preserved — the disable branch did not misfire
3 Main-session request on a non-OpenRouter host (gpt-5.6-sol via custom proxy) configured reasoning untouched — the new OpenRouter branch did not leak to other hosts

Captured request #1 (context.promptId = "side-query:auto-memory-recall", error 401 Missing Authentication header):

{
 "model": "or-verify",
 "temperature": 0,
 "stream": false,
 "max_tokens": 32000,
 "reasoning": { "enabled": false }
}

Captured control #2 (same baseUrl, thinking enabled):

{ "model": "or-verify", "reasoning": { "effort": "medium" } }

So on OpenRouter, thinking-off requests now ship the provider's canonical disable switch while thinking-on requests keep their configured effort — exactly the behavior the fix targets (#9757). Runtime evidence only; the PR's unit tests already run in CI.

Visual evidence (terminal screenshot of the captured wire bodies at PR head 8030cdeddc7b):

evidence-9758

1 similar comment
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Local runtime verification: PASS (wire-level request-body evidence)

  • Revision: 8030cdeddc7b584021fc00be2f2fe793b6355e75
  • Runtime: node packages/cli/dist/index.js --prompt ... --openai-logging --openai-logging-dir ... (captures the exact OpenAI request body the pipeline puts on the wire), with a temporary fastModel entry whose baseUrl is https://openrouter.ai/api/v1 and an invalid key — the resulting 401 Missing Authentication header proves the request actually reached OpenRouter; what matters here is the captured request body.
# Item Observed wire body Verdict
1 includeThoughts: false request (side-query auto-memory-recall) to an openrouter.ai host "reasoning": {"enabled": false} present
2 Same entry, thinking not disabled (managed subagent side query) "reasoning": {"effort": "medium"} preserved — the disable branch did not misfire
3 Main-session request on a non-OpenRouter host (gpt-5.6-sol via custom proxy) configured reasoning untouched — the new OpenRouter branch did not leak to other hosts

Captured request #1 (context.promptId = "side-query:auto-memory-recall", error 401 Missing Authentication header):

{
 "model": "or-verify",
 "temperature": 0,
 "stream": false,
 "max_tokens": 32000,
 "reasoning": { "enabled": false }
}

Captured control #2 (same baseUrl, thinking enabled):

{ "model": "or-verify", "reasoning": { "effort": "medium" } }

So on OpenRouter, thinking-off requests now ship the provider's canonical disable switch while thinking-on requests keep their configured effort — exactly the behavior the fix targets (#9757). Runtime evidence only; the PR's unit tests already run in CI.

Visual evidence (terminal screenshot of the captured wire bodies at PR head 8030cdeddc7b):

evidence-9758

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking findings.

Scope: Standard tier — 5 files, 456 lines diff (mostly new tests).

Checked:

  • openrouter.ts: isOpenRouterHostname parses baseUrl with new URL(), matches hostname === "openrouter.ai" || hostname.endsWith(".openrouter.ai"). Correct — no substring false-positives (evil-hostname test confirms). Mirrors isDeepSeekHostname.
  • pipeline.ts: The new reasoning: { enabled: false } block fires after the existing DeepSeek block, inside the includeThoughts === false branch. Placement is after any provider-hook reasoning strip, so the OpenRouter shape is not immediately clobbered. thinkingMandatory guard is preserved.
  • Test matrix: 8 tests cover — includeThoughts:false on OpenRouter (emits), thinking enabled (does NOT emit), config reasoning:false (emits), non-qwen model on OpenRouter (emits), thinkingMandatory=true (does NOT emit), non-OpenRouter endpoint (does NOT emit), evil hostname (does NOT emit), official api.openai.com (does NOT emit). Full branch coverage.
  • openrouter.test.ts: 5 it.each cases including openrouter.ai.evil.com false-positive guard.

CI: precheck-pr SKIPPED — fork PR security model. Test plan ran 8 new cases locally per prior review (167 passed).

Cross-check: Prior suggestion about missing openrouter.test.ts is addressed — the file is in this diff. No prior criticals.

Reviewed with AI assistance.

@yiliang114
yiliang114 enabled auto-merge August 25, 2026 02:41
@wenshao

wenshao commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Local runtime verification — PASS (socket-level wire capture + user-visible A/B)

Maintainer-side verification of head 8030cded against merge-base 1007bcac. This is not a re-run of the PR's unit tests: the request body is captured on the TLS socket, out of the real dist/cli.js bundle, with the whole pipeline (provider.buildRequest → unconditional reasoning/reasoning_effort strip → the new emission) running unmodified. Thanks @yiliang114 for the --openai-logging evidence already on this PR — this adds the socket-level capture, a merge-base A/B, an 11-row hostname/config matrix, and a reproduction of the user-visible symptom from #9757.

Harness (how the capture is made honest)
  • Two worktrees built from source with npm run bundle: head 8030cdeddc7b584021fc00be2f2fe793b6355e75, base 1007bcacfcdfe70cf7692c21ef7f402fff186af9 (merge-base). The arms differ only by this PR's diff.
  • A local CONNECT proxy (settings.proxy) terminates TLS with a CA the CLI trusts via NODE_EXTRA_CA_CERTS, so the client really dials https://openrouter.ai/api/v1/chat/completions — real hostname, real SNI, real Host header — and the gateway records the exact request bytes. No SDK stub, no in-process interception.
  • The fake gateway emulates OpenRouter's thinking semantics: thinking stays ON unless the request carries reasoning: { enabled: false }. With thinking on, the 256-token forced-tool stage-1 side query comes back finish_reason: "length" with no tool_calls — the exact failure shape described in Auto Mode classifier stage 1 unavailable with OpenRouter #9757.
  • Drivers: the real interactive TUI in tmux under AUTO approval mode, plus a headless --input-format stream-json driver for the matrix (that input mode keeps the AUTO classifier live; plain qwen -p denies Shell/Edit/Write up front, so it can't exercise this path at all).
  • Fairness control: rows M3–M8 run the same head build but on hostnames the detector rejects, so the field is not emitted and the classifier fails there too. The oracle therefore keys on the wire field, not on which build is running.

1. User-visible A/B — the #9757 symptom, reproduced and fixed

Same prompt, same fake gateway, same AUTO mode; only the build differs.

merge-base 1007bcacClassifier stage 1 unavailable, AUTO falls back to manual approval:

PR head 8030cded — classifier answers, the call is auto-approved and executes:

Side-effect oracle (a marker file the shell call writes): base = never created, head = created. Stage-1 bodies captured in those two TUI sessions:

arm host reasoning chat_template_kwargs AUTO outcome
base 1007bcac openrouter.ai absent {enable_thinking:false} manual approval prompt, tool not run
head 8030cded openrouter.ai {"enabled":false} {enable_thinking:false} auto-approved, tool ran

2. Captured request body (bytes off the socket)

The two bodies are byte-identical apart from the added field:

...,"tool_choice":"required","chat_template_kwargs":{"enable_thinking":false},"reasoning":{"enabled":false}}

3. Hostname / config matrix (all rows on the head build)

# baseURL host wire model reasoning on the captured stage-1 body other disable shapes on the same body expected
M1 openrouter.ai qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M2 eu.openrouter.ai (subdomain) qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M9 openrouter.ai (non-qwen) deepseek/deepseek-r1 {"enabled": false} emitted ✅
M10 openrouter.ai + config reasoning:false qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M11 openrouter.ai + thinkingMandatory qwen/qwen3.8-27b absent absent ✅
M3 openrouter.ai.evil.com qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M4 evilopenrouter.ai qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M5 my-vllm.example.com qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M6 api.openai.com gpt-5 absent absent ✅
M7 api.deepseek.com deepseek-chat absent thinking:{type:'disabled'} absent ✅
M8 dashscope.aliyuncs.com qwen/qwen3.8-27b absent enable_thinking:false absent ✅

Notes on the matrix:

  • M9 confirms the provider-level (not family-gated) decision: a non-qwen model on OpenRouter gets reasoning and correctly gets no chat_template_kwargs.
  • M10 is the config-level reasoning: false path, and its main agent turn is a streaming request — so the emission is verified on the streaming wire too, not only on the non-streaming side query.
  • M7 / M8 show the pre-existing DeepSeek (thinking:{type:'disabled'}) and DashScope (enable_thinking:false) shapes are untouched and pick up no reasoning leak.

4. Static checks (merge-base counterfactual)

Check Result
vitest run pipeline.test.ts openrouter.test.ts on head 174 passed (168 + 6)
Same test files replayed against merge-base source 3 failed / 165 passed, plus openrouter.test.ts fails to resolve ./openrouter.js — exactly the 3 emission assertions, so they are real red→green probes, not tautologies
tsc --noEmit in packages/core, head vs base identical output; the PR adds 0 typecheck errors (my box has 2 pre-existing @opentelemetry drift errors in telemetry/sdk-impl.ts on both arms)
prettier --check + eslint on the 5 changed files clean
PR CI on 8030cded all green (Test (ubuntu-latest, Node 22.x), Desktop Shell ×2, web-shell E2E, CVE audit, TruffleHog)

5. Non-blocking notes

  1. enabled:false vs effort:"none". OpenRouter's reasoning-tokens page documents reasoning.effort: "none" with the words "disables reasoning entirely", while enabled is documented as "enabled": true // Default: inferred from effort or max_tokens. What makes enabled:false a genuine user switch is the models API: it exposes default_enabled as "Default on/off state when the user has not set reasoning.enabled" — i.e. reasoning.enabled is settable by the caller in both directions. So the chosen shape is defensible, but two disable spellings exist and only a live key settles which one every upstream provider honours. This is the same gap the PR already declares; I could not close it either (no OpenRouter key in the harness).
  2. Blast radius is wider than the classifier. utils/sideQuery.ts defaults every side query to includeThoughts: false, so on an OpenRouter base URL this field now ships on compression, hooks, goal verification, forked queries, memory recall — not just the AUTO classifier. That is the upside if OpenRouter honours it, and the exposure if it ever 400s on a model whose provider rejects the parameter. Worth watching in the first release after merge.
  3. qwen models on OpenRouter carry both shapes. Confirmed on the wire (M1/M2/M10): chat_template_kwargs.enable_thinking:false and reasoning:{enabled:false}. The PR keeps this deliberately for a minimal diff; harmless if the gateway drops unknown fields, unverified against the live gateway.
  4. Trailing-dot FQDN. https://openrouter.ai./api/v1 yields hostname openrouter.ai. and does not match, so the fix would not apply there. Cosmetic, and identical to the isDeepSeekHostname precedent it mirrors — not a regression introduced here. https://OpenRouter.AI/..., :443, userinfo, and eu.openrouter.ai all match correctly; openrouter.ai.evil.com and evilopenrouter.ai correctly do not.

Verdict

LGTM — recommend merge. The failure in #9757 reproduces deterministically on merge-base and disappears at head; the added wire field lands exactly where it should and nowhere else; existing per-provider disable shapes are untouched; no new lint/type/test debt. The only thing local verification cannot settle is whether OpenRouter's gateway honours reasoning.enabled:false end-to-end, which the PR description already scopes out.

中文说明

本地运行时验证 —— 通过(socket 级线上抓包 + 用户可见 A/B)

以维护者身份对 head 8030cded 与 merge-base 1007bcac 做了对照验证。这不是重跑 PR 的单测:请求体是在 TLS socket 上抓的,来自真实的 dist/cli.js bundle,整条管道(provider.buildRequest → 无条件剥离 reasoning/reasoning_effort → 新增补发)原样运行。感谢 @yiliang114 已经贴出的 --openai-logging 证据 —— 本次补充的是 socket 级抓包、与 merge-base 的 A/B、11 行 hostname/配置矩阵,以及 #9757 用户可见故障的复现。

验证环境(抓包为什么可信)
  • 两个 worktree 各自 npm run bundle 从源码构建:head 8030cdeddc7b584021fc00be2f2fe793b6355e75,base 1007bcacfcdfe70cf7692c21ef7f402fff186af9(merge-base)。两个 arm 只差本 PR 的 diff。
  • 本地 CONNECT 代理(settings.proxy)用一张 CLI 通过 NODE_EXTRA_CA_CERTS 信任的 CA 终结 TLS,因此客户端是真的去连 https://openrouter.ai/api/v1/chat/completions —— 真实域名、真实 SNI、真实 Host 头 —— 网关侧记录到的是原始请求字节。没有 SDK 打桩,没有进程内拦截。
  • 假网关模拟 OpenRouter 的思考语义:除非请求带 reasoning: { enabled: false },否则思考保持开启。 思考开启时,256 token 预算的强制工具 stage-1 side query 会以 finish_reason: "length" 且无 tool_calls 返回 —— 正是 Auto Mode classifier stage 1 unavailable with OpenRouter #9757 描述的失败形状。
  • 驱动方式:tmux 里跑真实交互式 TUI(AUTO 审批模式),矩阵部分用 headless --input-format stream-json(该输入模式下 AUTO 分类器仍然生效;普通 qwen -p 会直接把 Shell/Edit/Write 列入 deny,根本走不到这条路径)。
  • 公平性对照:M3–M8 用的是同一个 head 构建,只是 hostname 不被检测器命中,于是不补发该字段,分类器同样失败。也就是说判据取决于线上字段本身,而不是取决于跑的是哪个构建。

1. 用户可见 A/B —— #9757 的现象复现并被修复

同样的 prompt、同样的假网关、同样的 AUTO 模式,只有构建不同。

merge-base 1007bcac —— Classifier stage 1 unavailable,AUTO 回退到手动审批:

PR head 8030cded —— 分类器正常返回,工具调用被自动放行并执行:

副作用判据(shell 调用写出的 marker 文件):base 从未生成head 已生成。这两次 TUI 会话抓到的 stage-1 请求体:

arm host reasoning chat_template_kwargs AUTO 结果
base 1007bcac openrouter.ai {enable_thinking:false} 弹出手动审批,工具未执行
head 8030cded openrouter.ai {"enabled":false} {enable_thinking:false} 自动放行,工具已执行

2. 抓到的请求体(socket 上的字节)

两个请求体除了新增字段之外逐字节相同:

...,"tool_choice":"required","chat_template_kwargs":{"enable_thinking":false},"reasoning":{"enabled":false}}

3. Hostname / 配置矩阵(全部在 head 构建上跑)

# baseURL host wire model reasoning on the captured stage-1 body other disable shapes on the same body expected
M1 openrouter.ai qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M2 eu.openrouter.ai (subdomain) qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M9 openrouter.ai (non-qwen) deepseek/deepseek-r1 {"enabled": false} emitted ✅
M10 openrouter.ai + config reasoning:false qwen/qwen3.8-27b {"enabled": false} chat_template_kwargs.enable_thinking:false emitted ✅
M11 openrouter.ai + thinkingMandatory qwen/qwen3.8-27b absent absent ✅
M3 openrouter.ai.evil.com qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M4 evilopenrouter.ai qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M5 my-vllm.example.com qwen/qwen3.8-27b absent chat_template_kwargs.enable_thinking:false absent ✅
M6 api.openai.com gpt-5 absent absent ✅
M7 api.deepseek.com deepseek-chat absent thinking:{type:'disabled'} absent ✅
M8 dashscope.aliyuncs.com qwen/qwen3.8-27b absent enable_thinking:false absent ✅

矩阵要点:

  • M9 印证了按供应商(而非模型族)门控是对的:OpenRouter 上的非 qwen 模型拿到了 reasoning,并且正确地没有 chat_template_kwargs
  • M10 是配置级 reasoning: false 路径,其主 agent 轮次是流式请求 —— 因此流式线路上的补发也被验证到了,不只是非流式的 side query。
  • M7 / M8 说明既有的 DeepSeek(thinking:{type:'disabled'})与 DashScope(enable_thinking:false)形状原样保留,且没有多出 reasoning

4. 静态检查(含 merge-base 反事实)

检查项 结果
head 上 vitest run pipeline.test.ts openrouter.test.ts 174 通过(168 + 6)
同样的测试文件回放到 merge-base 源码 3 失败 / 165 通过,另加 openrouter.test.ts 无法解析 ./openrouter.js —— 恰好是那 3 条补发断言,说明它们是真正的红→绿探针而非恒真断言
packages/coretsc --noEmit,head vs base 输出完全一致,本 PR 新增 0 个类型错误(我这台机器上两个 arm 都有 2 个既有的 @opentelemetry 依赖漂移错误,位于 telemetry/sdk-impl.ts
5 个改动文件的 prettier --check + eslint 干净
8030cded 上的 PR CI 全绿(Test (ubuntu-latest, Node 22.x)、Desktop Shell ×2、web-shell E2E、CVE 审计、TruffleHog)

5. 非阻塞说明

  1. enabled:falseeffort:"none" 之争。 OpenRouter 的 reasoning-tokens 文档对 reasoning.effort: "none" 明确写了 "disables reasoning entirely",而 enabled 的说明是 "enabled": true // Default: inferred from effort or max_tokens。让 enabled:false 成为真正调用方开关的依据在 models API:它暴露 default_enabled,描述为 "Default on/off state when the user has not set reasoning.enabled" —— 即 reasoning.enabled 是调用方可双向设置的。所以本 PR 选的形状站得住,但确实存在两种关闭写法,究竟哪种被每个上游供应商认,只有真实 key 能定论。这正是 PR 已声明的缺口,我这边也没能补上(harness 没有 OpenRouter key)。
  2. 影响面比分类器更广。 utils/sideQuery.ts所有 side query 默认 includeThoughts: false,所以在 OpenRouter base URL 上,这个字段现在会出现在压缩、hooks、目标校验、forked query、记忆召回等请求上,不只是 AUTO 分类器。如果 OpenRouter 认这个参数,这是收益;如果某个模型的上游供应商对该参数返回 400,这就是暴露面。合入后的第一个版本值得观察。
  3. OpenRouter 上的 qwen 模型会同时带两种形状。 线上已确认(M1/M2/M10):chat_template_kwargs.enable_thinking:falsereasoning:{enabled:false} 同时存在。PR 为保持最小 diff 有意保留;若网关丢弃未知字段则无害,但未对真实网关验证。
  4. 尾点 FQDN。 https://openrouter.ai./api/v1 解析出的 hostname 是 openrouter.ai.匹配,修复对它不生效。属于细枝末节,且与它所对标的 isDeepSeekHostname 行为一致 —— 不是本 PR 引入的回归。https://OpenRouter.AI/...:443、userinfo、eu.openrouter.ai 都能正确匹配;openrouter.ai.evil.comevilopenrouter.ai 都正确不匹配。

结论

LGTM —— 建议合入。 #9757 的故障在 merge-base 上可确定性复现,在 head 上消失;新增的线上字段落在该落的地方、且没有外溢;既有的各供应商关闭形状未受影响;没有新增 lint/类型/测试债务。本地验证唯一无法定论的,是 OpenRouter 网关端到端是否认 reasoning.enabled:false —— 这一点 PR 描述里已经划出范围。

@yiliang114
yiliang114 added this pull request to the merge queue Aug 25, 2026
Merged via the queue into QwenLM:main with commit bd42e67 Aug 25, 2026
116 of 134 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.22.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto Mode classifier stage 1 unavailable with OpenRouter

4 participants