feat(cli): expose reasoning effort through ACP - #8526
Conversation
Local verification reportVerified commits: Failure-first regressionBefore the original implementation, the focused ACP regression failed because The follow-up additionally covers all five review findings:
Real production-process verificationBuilt the production bundle and ran the repository's ACP integration harness against a real Regression and static validationEnvironment: macOS arm64, Node.js 中文验证报告本地验证报告验证提交: 失败优先回归原始实现之前,聚焦 ACP 回归测试会失败,因为 跟进修复覆盖了全部 5 条审查意见:
真实生产进程验证先构建生产 bundle,再通过仓库 ACP 集成框架启动真实的 回归与静态验证环境:macOS arm64,Node.js |
|
Re-run after two more autofix commits ( Template ✓ — all required sections present, bilingual body complete. Problem: real, user-reported gap. Linked issue #8514 asks for the five reasoning-effort tiers to be reachable from ACP hosts (JetBrains), which today can only configure mode and model. Not theoretical — the runtime already supports all tiers, the ACP surface just never exposed them. Direction: aligned. The terminal already exposes the full ladder via Size: core paths are touched ( Approach: minimal and honest. One new Risk: Stage 1e matches the Moving on to code review. 🔍 中文说明在之前审查的 head 之上又落了两个 autofix 提交( 模板 ✓ —— 必填章节齐全,中英双语正文完整。 问题:真实、用户上报的缺口。关联 issue #8514 要求 ACP 宿主(JetBrains)能触达五档 reasoning effort,而目前宿主只能配置 mode 和 model。不是理论问题——运行时早已支持全部档位,只是 ACP 面从未暴露。 方向:对齐。终端已通过 规模:触及核心路径( 方案:最小且诚实。stdio agent 新增一个复用现有档位列表与运行时 setter 的 风险:Stage 1e 命中 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent proposal first (written before reading the diff): add a
Verified against base code: No correctness, security, or regression findings. Non-blocking nits, carried from the prior round and unchanged: Files changed (13)
Test evidence — the PR's own CI on
|
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | success |
| Serve A/B (ubuntu-latest, Node 22.x) | success |
| Desktop Shell (ubuntu-22.04) | success |
| Desktop Shell (windows-2022) | success |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | success |
| Real daemon E2E / Java 11 | success |
| ubuntu-latest / Java 11, 17, 21 · macos-latest / Java 21 · windows-latest / Java 21 (SDK Java) | success |
| Integration Tests (CLI, No Sandbox) | skipped — merge-queue-only |
| Test (macos-latest / windows-latest, Node 22.x) | skipped — merge-queue-gated |
Sandboxed verification, if a maintainer wants live-subprocess A/B proof before merge: @qwen-code /verify — the stdio end-to-end path (option advertised → set xhigh → read-back → clear → invalid value rejected) runs only in the merge queue, and the author's live evidence is macOS-only. That said, maintainer @wenshao has already independently deep-verified the live behavior and approved this head, which substantially narrows the gap this lane would close.
Not verified here: real rendering inside JetBrains (explicitly out of scope for the PR), and provider-specific inference behavior per tier (unchanged mapping/clamping path).
中文说明
代码审查:先独立给出方案再看 diff——我的方案是给 stdio ACP agent 增加标准 thought_level 类别下的 reasoning_effort 选项,值走现有核心档位列表校验,复用 /effort 的运行时 setter,非法值在协议边界拒绝,HTTP 侧要么镜像要么诚实门禁。PR 与此一致,且两处更好:一是新增 core 帮助函数 applyReasoningEffort,把四处重复的 set→read-back 收敛为一处并全部迁移(逐一对比基础代码确认上报语义不变,迁移后无遗留手写 read-back);二是 HTTP 传输层无法路由该设置,便不再广播无法兑现的选项(正是早前评审发现的不一致),改为按共享常量过滤为可路由 id,并给出明确的支持列表报错。另核实:default 通过 setReasoningEffort(undefined) 清除覆盖;thinking 关闭时以明确的 invalid-params 错误失败关闭而非假装成功。无正确性/安全/回归问题。非阻塞小问题(沿袭上一轮):对必选方法使用了可选链 ?.()(装饰性);stdio 端到端流程仅由合并队列集成测试固定(PR CI 按设计跳过)。测试证据:受审提交 6c4a28e5 的 CI 全绿(20 项通过,含单测套件、Serve A/B、Desktop Shell、web-shell E2E、真实 daemon E2E、SDK Java 全矩阵),55 项跳过为合并队列门禁与机器人编排任务;无失败、无进行中。若维护者合并前想要真实子进程 A/B 证据,可触发 @qwen-code /verify;不过 @wenshao 已对该 head 完成独立深度验证并批准。未验证:JetBrains 内真实渲染(PR 明示范围外)与各 provider 分档推理行为(映射/钳制路径未变)。
— Qwen Code · qwen3.8-max
Reviewed at 6c4a28e556fab7af9a67d3ee0da134d209ac69ee · re-run with @qwen-code /triage
|
Confidence: 4/5 — clean, well-tested implementation of a real user-facing gap; the withheld point is a cosmetic This re-run found the head unchanged ( Stepping back: the problem is real — issue #8514 is a concrete user report (JetBrains ACP users can configure mode and model but cannot reach the reasoning-effort tiers the runtime already supports), and the change is still the smallest thing that works: one The evidence is stronger than on any earlier pass. PR CI is fully green on this commit — 20 successful checks including the unit suite ( Non-blocking reservations, unchanged: Approving, pinned to the reviewed commit. For transparency: this becomes this account's latest review state and supersedes the changes-requested review the 中文说明置信度:4/5 —— 干净、测试充分的实现,解决真实用户缺口;保留的一分给装饰性的 本次重跑时 head 未变( 退一步看:问题真实——issue #8514 是具体的用户报告(JetBrains ACP 用户能配置 mode 和 model,却触达不了运行时早已支持的 reasoning effort 档位),方案仍是可行的最小实现:标准 证据比之前任何一轮都强。PR CI 在该提交上全绿——20 项检查通过,含单测套件( 非阻塞保留项(不变): 批准将固定到受审提交。透明起见:这将成为本账号的最新评审状态,并取代 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.21.5)
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI at the reviewed commit; a review agent ran the suite locally and it passed, but no CI job gates it.
中文说明
已审查。 建议见行内评论。 未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI at the reviewed commit; a review agent ran the suite locally and it passed, but no CI job gates it。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| session.getConfig().setReasoningEffort(effort); | ||
| if ( | ||
| effort !== undefined && | ||
| session.getConfig().getReasoningEffort() !== effort | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R2-1: This adds a fourth copy of the "set reasoning effort, then read it back to detect the silent no-op when thinking is disabled" pattern. The same set→read-back comparison now lives in four call sites across three files: this ACP branch (throws -32602), packages/cli/src/ui/commands/effort-command.ts (returns an info message), and twice in packages/cli/src/nonInteractive/control/controllers/systemController.ts (handleInitialize warns via debugLogger, handleSetEffort returns an applied boolean). — Concrete cost: any change to the no-op semantics requires four coordinated edits, and the next surface to copy one variant without the read-back will report success while the requested tier is silently discarded.
Fix (spans multiple files, so no one-click suggestion): extract one shared helper beside packages/core/src/core/reasoning-effort.ts — e.g. applyReasoningEffort(config, effort): boolean that performs the set + read-back — and have each surface wrap it with its own reporting semantics.
中文说明
此处新增了"设置 reasoning effort 后读回,以检测 thinking 被禁用时的静默 no-op"这一模式的第四份拷贝。同样的 set→读回比较现在出现在三个文件的四处调用点:本 ACP 分支(抛出 -32602)、packages/cli/src/ui/commands/effort-command.ts(返回提示信息)、以及 packages/cli/src/nonInteractive/control/controllers/systemController.ts 中的两处(handleInitialize 打警告日志、handleSetEffort 返回 applied 布尔值)。——具体代价:no-op 语义的任何变化都需要四处同步修改;下一个照抄某个变体却漏掉读回检查的调用面,会在请求的档位被静默丢弃时向调用方报告成功。
修复(跨多文件,故不提供一键 suggestion):在 packages/core/src/core/reasoning-effort.ts 旁提取一个共享辅助函数——例如 applyReasoningEffort(config, effort): boolean,执行 set + 读回——各调用面再分别包装自己的上报语义。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| ? co.filter( | ||
| (option) => | ||
| isObject(option) && | ||
| (option['id'] === 'model' || option['id'] === 'mode'), | ||
| ) |
There was a problem hiding this comment.
[Suggestion] R2-2: The HTTP transport's supported config-option ids are encoded twice independently — here as a hardcoded allowlist on the read path, and in the session/set_config_option routing chain (~line 2207: if (configId === 'model') … else if (configId === 'mode') … else Unknown configId) on the write path — with no shared constant or cross-reference. The follow-up that adds daemon routing for reasoning_effort (anticipated by the "currently model and mode" comment above) must keep both in sync by hand. — Failure scenario: that follow-up adds a bridge arm to the routing chain but forgets this filter → the option becomes settable over HTTP ACP yet stays silently absent from every session/new, session/load, and set_config_option response, so no client can discover it and no test fails. Reverse drift (relaxing the filter without the handler) advertises an option that then fails sets with Unknown configId.
Fix (spans multiple locations): hoist one shared constant in AcpDispatcher (e.g. const HTTP_ACP_CONFIG_OPTION_IDS: readonly string[] = ['model', 'mode'];) consumed by both this filter predicate and the routing chain's accepted-id check.
中文说明
HTTP 传输层支持的配置项 id 现在被独立编码了两次——读路径上是这里的硬编码允许列表,写路径上是 session/set_config_option 路由链(约第 2207 行:if (configId === 'model') … else if (configId === 'mode') … else Unknown configId)——两者之间没有共享常量或交叉引用。为 reasoning_effort 新增 daemon 路由的后续 PR(上方 "currently model and mode" 注释已预告)必须手工保持两处同步。——失败场景:该后续 PR 在路由链中加入了 bridge 分支却忘了这里的过滤器 → 该选项在 HTTP ACP 上可设置,却悄悄从所有 session/new、session/load、set_config_option 响应中消失,任何客户端都无法发现它,且没有测试会失败。反向漂移(放宽过滤器而未改处理器)则会广播一个设置时返回 Unknown configId 失败的选项。
修复(跨多处):在 AcpDispatcher 中提取一个共享常量(例如 const HTTP_ACP_CONFIG_OPTION_IDS: readonly string[] = ['model', 'mode'];),让此过滤谓词与路由链的可接受 id 检查共同消费。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(frame.result.configOptions.map((option) => option.id)).toEqual([ | ||
| 'model', | ||
| ]); |
There was a problem hiding this comment.
[Suggestion] R2-3: This new test for the configOptionsFor allow-list never exercises the 'mode' half of the predicate — the FakeBridge fixture (lines 323–343) supplies only model and reasoning_effort options. A mutation check during this review confirmed the mutant survives: deleting the option['id'] === 'mode' disjunct leaves all 291 tests in this file green, while adding a mode option to the fixture and asserting both ids fails the mutant and passes the correct code. — Failure scenario: someone drops or inverts the mode disjunct → the real HTTP ACP surface silently stops returning the approval-mode selector in session/new and set_config_option responses, regressing mode switching for JetBrains/HTTP clients with no CI signal.
Fix (two edits, so no one-click suggestion): add a mode option to the FakeBridge fixture's configOptions (e.g. { id: 'mode', name: 'Mode', category: 'mode', type: 'select', currentValue: 'default', options: [] }), then tighten this assertion to cover both entries, e.g. toEqual(['model', 'mode']).
中文说明
这个针对 configOptionsFor 允许列表的新测试从未覆盖谓词中 'mode' 的那一半——FakeBridge 夹具(第 323–343 行)只提供了 model 和 reasoning_effort 两个选项。本次评审中的变异检查确认该变异体可以存活:删除 option['id'] === 'mode' 这一析取项后,本文件全部 291 个测试仍然全绿;而在夹具中加入 mode 选项并断言两个 id 后,变异体失败、正确代码通过。——失败场景:有人删掉或反转 mode 析取项 → 真实 HTTP ACP 面会在 session/new 与 set_config_option 响应中悄悄不再返回审批模式选择器,使 JetBrains/HTTP 客户端的模式切换回归,且 CI 毫无信号。
修复(两处编辑,故不提供一键 suggestion):在 FakeBridge 夹具的 configOptions 中加入 mode 选项(例如 { id: 'mode', name: 'Mode', category: 'mode', type: 'select', currentValue: 'default', options: [] }),再收紧此断言以覆盖两个条目,例如 toEqual(['model', 'mode'])。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| ? co.filter( | ||
| (option) => | ||
| isObject(option) && | ||
| (option['id'] === 'model' || option['id'] === 'mode'), | ||
| ) |
There was a problem hiding this comment.
[Suggestion] R2-4: The model/mode allow-list applies only to the standard ACP responses via configOptionsFor; the daemon's raw-state surfaces still pass the child's unfiltered configOptions — now including reasoning_effort: GET /session/:id/context (routes/session.ts:3000–3005), the _qwen/session/context vendor method (dispatch.ts:2347–2356), and REST POST /session/:id/load/resume state. The SDK documents DaemonSessionState.configOptions as "per-session toggles the client can flip", while the daemon's session/set_config_option rejects this id with Unknown configId. — Failure scenario: a generic daemon/SDK client rendering those surfaces shows a Reasoning-effort selector; the user flips it → INVALID_PARAMS: Unknown configId: reasoning_effort for a toggle the daemon itself advertised. Clean error, no wrong state; no in-repo consumer renders these surfaces today, so impact is limited to external clients (hence Suggestion, not Critical).
Fix (spans multiple locations): apply the same allow-list wherever the daemon re-emits the child's state.configOptions (context status and restore state), or document at buildSessionContextStatus/this filter that the diagnostic endpoints intentionally carry the full child set while the setter is gated.
中文说明
model/mode 允许列表只通过 configOptionsFor 作用于标准 ACP 响应;daemon 的原始状态面仍然透传子进程未过滤的 configOptions——现在其中包含 reasoning_effort:GET /session/:id/context(routes/session.ts:3000–3005)、_qwen/session/context vendor 方法(dispatch.ts:2347–2356)、以及 REST POST /session/:id/load/resume 的 state。SDK 将 DaemonSessionState.configOptions 描述为"客户端可切换的会话级开关",而 daemon 的 session/set_config_option 会以 Unknown configId 拒绝该 id。——失败场景:某个按这些面渲染会话状态的 daemon/SDK 客户端会显示 Reasoning-effort 选择器;用户切换 → 对 daemon 自己广播的开关返回 INVALID_PARAMS: Unknown configId: reasoning_effort。错误是干净的,状态不会出错;目前仓库内没有任何消费方渲染这些面,因此影响仅限于外部客户端(所以是 Suggestion 而非 Critical)。
修复(跨多处):在 daemon 重新输出子进程 state.configOptions 的所有位置(context status 与 restore state)应用同一允许列表;或在 buildSessionContextStatus/此过滤器处注明:诊断类端点有意携带完整子集,而 setter 受门控。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。本 PR 来自 fork,首轮处理将由下一次定时扫描执行(通常几分钟内)。移除 |
|
🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。 |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下: Review round summary — PR #8526 (round 2)All four round-2 suggestions are addressed in commit Feedback dispositionsRound 2 (new this round)
Round 1 (fixed in
|
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 799 passed · 0 failed · 799 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:799 通过 · 0 失败 · 799 总计 Verification reportPR #8526 — feat(cli): expose reasoning effort through ACPVerdict: 中文摘要
Central claim + A/BCentral claim: an ACP stdio client receives a Mock-free wire harness (
The flip is exactly the feature: base advertises two options and rejects the id; head advertises the third option, applies all five tiers, clears to Control purity: base worktree Reviewer Test Plan walk-through (per step): step "options include thought_level with Default + five tiers" — performed, exact match; "set Extra high → xhigh" — performed; "set Default → default" — performed; "ultra → invalid-params error without changing current setting" — performed, exact message, state unchanged; "preserves the option after a model switch" — performed, option and value ( Secondary claim: HTTP transport consistencyThe Secondary claim:
|
| # | guard | pinning suite | control | mutant | verdict |
|---|---|---|---|---|---|
| M1 | core read-back (return true) |
core reasoning-effort.test.ts | green | exit 1, expected true to be false |
KILLED |
| M2 | acpAgent thinking-disabled throw | acpAgent.test.ts (new test) | green | exit 1, promise resolved … instead of rejecting |
KILLED |
| M3 | HTTP advertisement filter | transport.test.ts (session/new ids) | green | exit 1, ['model','reasoning_effort','mode'] ≠ ['model','mode'] |
KILLED |
| M4 | HTTP setter accepted-id gate | transport.test.ts (routable-set test) | green | exit 1, got invalid mode "high" instead of Unknown configId |
KILLED |
4/4 killed, each by its intended assertion; all files restored byte-identical (sha256-verified). No survivors.
Findings
None blocking. Informational:
- Non-string
value→-32603 Internal error(zodinvalid_typefrom the ACP SDK request schema) rather than-32602. Pre-existing and id-agnostic (A/A identical on basemode); string violations of the new id get the PR's precise-32602. No action needed. - Raw-state HTTP surfaces still carry the full configOptions (context status), so a client enumerating there and calling
set_config_optionover HTTP gets a clean-32602 Unknown configIdwith no state change. The dispatch doc comment declares this intentional (raw surfaces report session state; only setter-paired responses are gated). Coherent, but worth a line in the PR description if the author wants hosts to know.
Not covered
- Per-commit attribution: depth-2 checkout makes only the PR head reachable (snapshot lists 5 commits); verified the aggregate
HEAD^1..HEADdiff. - Live JetBrains rendering and provider-specific inference behavior (PR-declared out of scope; mapping/clamping path untouched by the diff).
- Trial merge into current
main(no network in this environment); base tip4ec0371is the merge base the workflow provided, and the PR already carries two main-merge commits from 2026-08-07. - Repo-wide gates; only affected-workspace suites ran (core, cli targeted files, transport, ACP integration file).
session/loadwith a previously-set effort:buildConfigOptionsis shared and exercised viasession/new/set paths; load-time restoration of a runtime tier was not separately driven (effort is session-scoped by design, not persisted).
Methodology
Environment: CI node:22-bookworm container, merge-ref checkout, npm ci + npm run build pre-run at head; head cells drove the production bundle and the dist entry; the base cell used a scratch worktree at HEAD^1 with a symlinked root node_modules (third-party, lockfile unchanged) plus re-pointed @qwen-code/* links (realpath-asserted) and mirrored per-package nested node_modules, rebuilt via scripts/build.js. Harnesses are plain JSON-RPC stdio clients (acp-harness.mjs, aa-probe.mjs) and the repo's own vitest suites; mutation matrix scripted in mutation-matrix.mjs. Raw logs in logs/, harnesses and captures in this directory. Assertion tally: harness 24+24+8, core 13, cli 417, transport 292, integration 11, matrix 9 (4 controls + 4 kills + 1 restore check), A/A probe 1 = 799.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: AutoFix round — no action requiredNo actionable feedback remains on this PR in this round, so no code changes were made. What was checked
ResultThe branch stays at 中文说明AutoFix 轮次 — 无需处理本轮该 PR 上没有需要处理的反馈意见,因此未做任何代码改动。 检查内容
结果分支保持在 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/31243187716)._ |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| } else { | ||
| // 'mode' — the only other id in HTTP_ACP_CONFIG_OPTION_IDS. |
There was a problem hiding this comment.
[Suggestion] R3-1: HTTP_ACP_CONFIG_OPTION_IDS has three consumers — the accepted-id check, the advertisement filter, and this routing if/else — but only the first two are keyed to it; routing silently assumes the array is exactly ['model', 'mode']. — Failure scenario: when a future change adds a third id (e.g. reasoning_effort — the constant's doc comment invites exactly this, and this PR is the proof of concept for such an addition on the stdio side), the filter and accepted-id check pick it up automatically, but routing falls into this bare else and handles the request as an approval-mode change: value: 'default' (the reasoning-effort reset sentinel) collides with ApprovalMode.DEFAULT and would silently call setSessionApprovalMode(sessionId, 'default', ...) — changing the permission mode instead of clearing reasoning effort; other tier values fail with a misleading invalid mode "high". Today's two-id array is correct; the hazard is the unguarded coupling this diff introduced.
Suggested fix — restore the exhaustive shape so routing fails loud on any id it can't route:
} else if (configId === 'mode') {
// ... existing mode handling ...
} else {
// unreachable unless HTTP_ACP_CONFIG_OPTION_IDS grew an id without a branch
// reply INVALID_PARAMS `Unknown configId: ${configId}`
}中文说明
HTTP_ACP_CONFIG_OPTION_IDS 有三个消费方——可接受 id 检查、广播过滤器、以及此处路由 if/else——但只有前两者与该常量联动;路由隐式假设数组恰好是 ['model', 'mode']。——失败场景:未来新增第三个 id(例如 reasoning_effort——常量注释正鼓励这种扩展,且本 PR 就是 stdio 侧此类扩展的实例)时,过滤器与接受检查会自动生效,但路由会落入这个裸 else 分支并按审批模式处理:value: 'default'(reasoning effort 的重置哨兵值)与 ApprovalMode.DEFAULT 冲突,将静默调用 setSessionApprovalMode(sessionId, 'default', ...)——把权限模式改掉而不是清除 reasoning effort;其他档位取值会报出误导性的 invalid mode "high"。当前两元素数组是正确的;问题在于本 diff 引入的无守卫耦合。建议恢复穷举式路由结构,使无法路由的 id 显式报错。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| * `session/set_config_option`. Raw-state surfaces (context status, REST | ||
| * load/resume state) intentionally carry the child's full unfiltered set — | ||
| * they report session state; only these setter-paired responses are gated | ||
| * to the ids the transport can route. Best-effort — `undefined` on error. |
There was a problem hiding this comment.
[Suggestion] R3-2: This new docstring claims "only these setter-paired responses are gated to the ids the transport can route", but the filtered configOptionsFor is also consumed by the ACP session/load / session/resume response (~line 1593, overriding loadState.configOptions) and session/fork (~line 1856) — those responses are gated too. The "unfiltered load/resume state" claim is only true of the REST/raw-state surfaces, not the ACP load/resume responses built right below this comment. — Concrete cost: a maintainer debugging why reasoning_effort is absent from an HTTP session/load response reads that load/resume intentionally carry the child's full unfiltered set, and hunts for the filter in the REST layer or the bridge instead of this method — or concludes a load-response bug is intended behavior.
| * `session/set_config_option`. Raw-state surfaces (context status, REST | |
| * load/resume state) intentionally carry the child's full unfiltered set — | |
| * they report session state; only these setter-paired responses are gated | |
| * to the ids the transport can route. Best-effort — `undefined` on error. | |
| * `session/set_config_option`. Raw-state surfaces (context status, REST | |
| * load/resume state) intentionally carry the child's full unfiltered set — | |
| * they report session state; only the responses built from this helper | |
| * (`session/new`, `session/load`/`session/resume`, `session/fork`, and the | |
| * `session/set_config_option` result) are gated to the ids the transport | |
| * can route. Best-effort — `undefined` on error. |
中文说明
新增的文档注释声称"只有这些与 setter 配对的响应被门控到传输层可路由的 id",但过滤后的 configOptionsFor 同样被 ACP session/load/session/resume 响应(约 1593 行,覆盖 loadState.configOptions)与 session/fork(约 1856 行)消费——这些响应同样被门控。"load/resume 状态不过滤"的说法只对 REST/原始状态面成立,不适用于就在这段注释下方构建的 ACP load/resume 响应。注释与五个调用点中的三个行为相反,会误导排查 HTTP session/load 响应中为何没有 reasoning_effort 的维护者——他们可能去 REST 层或 bridge 里找过滤器,或误以为 load 响应的某个 bug 是有意行为。建议按实际门控面准确列举。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| RPC.INVALID_PARAMS, | ||
| `Unknown configId: ${configId}`, |
There was a problem hiding this comment.
[Suggestion] R3-3: The raw-state surfaces on this same transport (context status via _qwen/session/context, REST load/resume state) intentionally advertise reasoning_effort in configOptions (the R2-4 resolution documented this), while the setter rejects that same id with Unknown configId: reasoning_effort. "Unknown" asserts the id does not exist anywhere, which the server's own advertisement on the same transport contradicts. — Failure scenario: an HTTP ACP client renders the session's advertised config options from context status, the user picks a reasoning-effort tier, the server replies -32602 Unknown configId: reasoning_effort; the integrator concludes a client-side typo or stale schema and burns debugging time on the client before finding the deliberate transport-level gate.
| RPC.INVALID_PARAMS, | |
| `Unknown configId: ${configId}`, | |
| RPC.INVALID_PARAMS, | |
| `ConfigId not supported by this transport: ${configId} (supported: ${HTTP_ACP_CONFIG_OPTION_IDS.join(', ')})`, |
Note: transport.test.ts pins the current Unknown configId text — update that expectation together with this message.
中文说明
同一传输层的原始状态面(_qwen/session/context、REST load/resume state)有意在 configOptions 中广播 reasoning_effort(R2-4 的决议已将其文档化),而 setter 却以 Unknown configId: reasoning_effort 拒绝同一 id。"Unknown" 断言该 id 在任何地方都不存在,与服务器自身在同一传输层上的广播相矛盾。——失败场景:HTTP ACP 客户端依据 context status 渲染广播的配置选项,用户选择了 reasoning effort 档位,服务器回复 -32602 Unknown configId: reasoning_effort;集成方会误以为是客户端拼写错误或 schema 过期,在客户端上空耗调试时间,最后才发现这是传输层有意设置的门控。建议改为"此传输层不支持该 configId(支持:...)"。注意:transport.test.ts 固定了当前 Unknown configId 文案,需同步更新该断言。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| throw RequestError.invalidParams( | ||
| undefined, | ||
| `Unknown reasoning effort: ${value}`, |
There was a problem hiding this comment.
[Suggestion] R3-4: Every other effort surface in this PR enumerates the accepted values in its rejection message — effort-command.ts ("Choose one of: ..."), systemController.ts ("Supported: low, medium, high, xhigh, max"), and the HTTP mode branch ("expected one of: ..."). This stdio ACP path alone throws bare Unknown reasoning effort: ${value}. — Failure scenario: an ACP integrator (this is the protocol boundary JetBrains and other clients hit) sends 'High', 'maximum' or 'x-high' — all accepted by normalizeReasoningEffort on the slash-command and systemController surfaces — and receives a rejection with no hint of the valid set, forcing discovery from the option list or source.
| throw RequestError.invalidParams( | |
| undefined, | |
| `Unknown reasoning effort: ${value}`, | |
| throw RequestError.invalidParams( | |
| undefined, | |
| `Unknown reasoning effort: ${value}. Choose one of: ${ACP_REASONING_EFFORT_DEFAULT}, ${REASONING_EFFORT_TIERS.join(', ')}`, |
Note: two test expectations pin the current message text and need updating with it.
中文说明
本 PR 中其他所有 effort 表面的拒绝消息都列举了合法取值——effort-command.ts("Choose one of: ...")、systemController.ts("Supported: low, medium, high, xhigh, max")、HTTP mode 分支("expected one of: ...")——唯独这里的 stdio ACP 路径只抛出裸的 Unknown reasoning effort: ${value}。——失败场景:ACP 集成方(这正是 JetBrains 等客户端接触的协议边界)发送 'High'、'maximum' 或 'x-high'——这些别名在斜杠命令与 systemController 表面都被 normalizeReasoningEffort 接受——却收到不带任何合法取值提示的拒绝,只能从选项列表或源码中自行发现。注意:有两处测试断言固定了当前文案,需同步更新。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if (!HTTP_ACP_CONFIG_OPTION_IDS.includes(configId)) { | ||
| if (id !== undefined) { |
There was a problem hiding this comment.
[Suggestion] R3-5: Test-efficacy probe (harness validated): a mutant replacing this notification-guard condition with true left every affected test green — no test pins the negative side of the if (id !== undefined) guard in the new Unknown configId branch. — Failure scenario: a client sends session/set_config_option as a JSON-RPC notification (no id) with an unknown configId; if a future refactor drops the guard (the exact shape the surviving mutant models), the server emits an error response with id: undefined — a JSON-RPC protocol violation a strict client can reject or misroute — and no test in this diff would fail.
Suggested fix: add one transport test that sends session/set_config_option with an unknown configId and no id, asserting no response message is written (alongside the existing unknown-configId error test).
中文说明
测试效能探针(harness 已验证)显示:把该通知守卫条件替换为 true 的变异体不会让任何受影响的测试变红——新增 Unknown configId 分支中 if (id !== undefined) 守卫的反面没有测试固定。——失败场景:客户端以 JSON-RPC 通知形式(无 id)发送未知 configId 的 session/set_config_option;若未来重构丢掉该守卫(正是存活变异体的形状),服务器会发出 id: undefined 的错误响应——严格 JSON-RPC 客户端可能拒绝或误路由的协议违规——且本 diff 中没有任何测试会失败。建议补一条 transport 测试:无 id 的未知 configId 请求不产生任何响应消息。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下: Autofix review round — PR #8526All five round-3 suggestions from the automated reviewer were implemented in one commit ( Feedback points and dispositions
Changed files
Verification
中文说明Autofix 审查轮次 — PR #8526自动审查器第三轮的全部五条建议已在一次提交中实现( 反馈点与处理
变更文件
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const MCP_OAUTH_START_TIMEOUT_MS = 30_000; | ||
| const SESSION_DRAIN_TIMEOUT_MS = 30_000; | ||
| const ACP_REASONING_EFFORT_DEFAULT = 'default'; | ||
| const ACP_REASONING_EFFORT_NAMES: Record<ReasoningEffort, string> = { |
There was a problem hiding this comment.
[Suggestion] The new ACP_REASONING_EFFORT_NAMES map duplicates the per-tier display labels already defined for the same five tiers in the reasoningEffort enum options at packages/cli/src/config/settingsSchema.ts:1463 — and the two copies have already drifted in casing: xhigh: 'Extra high' here vs label: 'Extra High' in the schema (showInDialog: true). A third copy ("thinking.xhigh": "Extra High") lives in desktop/packages/shared/src/i18n/locales/en.json. The diff's own new test asserts the ACP spelling, so both spellings now ship. — Concrete cost: the same tier renders with different labels depending on surface (an ACP client such as JetBrains shows "Extra high" while the CLI settings dialog shows "Extra High"), and any future label edit or tier addition must be made in several independent places — missing one silently widens the divergence.
Suggested fix (spans multiple files, so no one-click suggestion): extract one shared tier→label record next to REASONING_EFFORT_TIERS in packages/core/src/core/reasoning-effort.ts and consume it from buildConfigOptions, the settings schema, and the desktop i18n copy, aligning the casing.
中文说明
新增的 ACP_REASONING_EFFORT_NAMES 映射重复定义了 packages/cli/src/config/settingsSchema.ts:1463 中 reasoningEffort 枚举选项里同样五个档位已有的显示标签——且两份拷贝在大小写上已经出现漂移:此处为 xhigh: 'Extra high',schema 中为 label: 'Extra High'(showInDialog: true)。第三份拷贝("thinking.xhigh": "Extra High")位于 desktop/packages/shared/src/i18n/locales/en.json。本 diff 新增的测试断言的是 ACP 侧拼写,因此两种拼写现在都会随产品发布。——具体代价:同一档位在不同界面显示不同标签(JetBrains 等 ACP 客户端显示 "Extra high",CLI 设置对话框显示 "Extra High");未来任何标签修改或新增档位都必须同时改多处独立代码,漏改其一会在界面之间悄悄扩大分歧。
建议修复(跨多文件,故不提供一键 suggestion):在 packages/core/src/core/reasoning-effort.ts 中 REASONING_EFFORT_TIERS 旁提取一份共享的 档位→标签 记录,供 buildConfigOptions、settings schema 与 desktop i18n 拷贝共同消费,并统一大小写。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| }; | ||
|
|
||
| return [modeConfigOption, modelConfigOption]; | ||
| const reasoningEffortConfigOption: SessionConfigOption = { |
There was a problem hiding this comment.
[Suggestion] buildConfigOptions advertises the reasoning_effort selector unconditionally, but when thinking is disabled (ContentGeneratorConfig.reasoning === false — a user-configurable model generation field) every listed non-default value deterministically fails with -32602 'Reasoning effort cannot be applied while thinking is disabled'. — Failure scenario: a session whose active model/user config disables thinking still receives reasoning_effort in session/new configOptions with currentValue: 'default' and five selectable tiers; the ACP client (JetBrains, per the docs bullet this PR adds) renders the selector from that list, the user picks e.g. high, and each tier pick reproduces the same error, with no advertised way to re-enable thinking. Sibling mode/model options list only settable values.
Suggested fix: omit the option or mark it unavailable when config.getContentGeneratorConfig()?.reasoning === false (which is distinguishable from an unset tier), so clients only render a selector that can succeed — or keep the advertise-always behavior deliberately and document it as such.
中文说明
buildConfigOptions 无条件地广播 reasoning_effort 选择器,但当 thinking 被禁用时(ContentGeneratorConfig.reasoning === false——这是用户可配置的模型生成字段),列表中除 default 外的每个取值都必然以 -32602 'Reasoning effort cannot be applied while thinking is disabled' 失败。——失败场景:活动模型/用户配置禁用了 thinking 的会话,仍然会在 session/new 的 configOptions 中收到 reasoning_effort(currentValue: 'default' 加五个可选档位);ACP 客户端(按本 PR 新增的文档条目,即 JetBrains)会据此渲染选择器,用户选择如 high 后每次都会得到同样的错误,且没有任何广播出来的途径可以重新启用 thinking。同级的 mode/model 选项只列出可设置的取值。
建议修复:当 config.getContentGeneratorConfig()?.reasoning === false 时省略该选项或将其标记为不可用(该状态与未设置档位可区分),使客户端只渲染能够成功的选择器——或有意保留“始终广播”的行为并在文档中注明。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 4/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31261778101 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 5/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31267311990 🧠 Handled by Qwen Code · model/模型 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round: no action required (PR #8526)This round found no actionable feedback, so no changes were made and no commit was created. Triage summary:
Note on the budget warning: the previous two rounds exhausted the time budget, but since this round's actionable sections are empty there was nothing to address or verify — no retry of prior work was needed or performed. Current state: branch 中文说明Autofix 评审轮次:无需处理(PR #8526)本轮没有发现可执行的反馈,因此未做任何改动,也未创建提交。 分类摘要:
关于预算警告: 此前两轮耗尽了时间预算,但本轮可执行区域为空,没有任何需要处理或验证的内容 —— 无需也没有重试此前的工作。 当前状态: 分支 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
Maintainer-local deep verification round (runs on top of the CI Verdict
中文摘要
Central claim + A/BCentral claim: an ACP session over Method: a mock-free wire harness spawns the bundled CLI as a real ACP stdio subprocess (
Mutation matrix (vacuity)Single-point mutations of the PR's production code, each run against its target test — 6/6 killed, 0 survivors (no vacuous new tests):
Targeted gates (head)
FindingsNo blocking findings. Informational only:
Not covered
Evidence images |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round: no action takenRound inputs (PR #8526, head
Triage result: nothing to address. The only feedback this round is the deep verification report, and its verdict is The report's two observations are explicitly informational, non-blocking, and need no code change:
Additionally, this PR is in Critical-only mode (after 5 change-producing rounds); the deferred non-Critical feedback section for this round is empty, so no deferred items apply either. Outcome: no code changes, no commit. Adding a commit now would only widen the diff with nothing to fix. The branch stays at 中文说明Autofix 审查轮次:无需处理本轮输入(PR #8526,head
分诊结果:没有需要处理的内容。 本轮唯一的反馈是深度验证报告,其结论为 报告中的两条观察均为信息性、非阻断,无需任何代码改动:
此外,本 PR 已处于 Critical-only 模式(在 5 个产生改动的轮次之后);本轮的延后非 Critical 反馈部分为空,因此也没有任何延后条目需要处理。 结果:不做任何代码改动,不创建提交。此时新增提交只会扩大 diff 而没有任何可修复的内容。分支保持在已验证的 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: The verification run did not complete, so the phases below may be partial or missing entirely. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 848 passed · 0 failed · 848 total 中文 — 判定:
|
| # | Round-1 finding | Severity | Status at 6c4a28e |
|---|---|---|---|
| 1 | Non-string value → -32603 Internal error (ACP SDK request schema), id-agnostic, pre-existing |
informational | stands, unchanged — re-measured via A/A probe: null/42/{a:1} on pre-existing id mode return {"code":-32603,"message":"Internal error"} byte-identical on base and head (scripted equality check passed); head's reasoning_effort id returns the same code/message class. No action needed. |
| 2 | HTTP raw-state surfaces carry the full configOptions; setting one returns a clean -32602 Unknown configId |
informational | addressed (hardened) — the rejection is now ConfigId not supported by this transport: reasoning_effort (supported: model, mode), naming both the transport limitation and the routable set; the transport suite's routable-set test pins the exact message (M4 kills its removal). Raw-state retention remains by documented design (dispatch doc comment now enumerates the gated surfaces); the setter-side rejection is verified by the 293/293 wire suite. |
Central claim + A/B
Central claim: an ACP stdio client receives a reasoning_effort session config option (category thought_level, six choices) and can apply/clear tiers via session/set_config_option, with invalid values rejected at the protocol boundary without state change.
Mock-free wire harness (acp-harness.mjs) drives a real spawned CLI process through initialize → authenticate → session/new → set_config_option…, isolated via per-run QWEN_HOME and a modelProviders openai registry entry; no inference is performed.
| cell | entry | oracle | result |
|---|---|---|---|
head (6c4a28e) |
production bundle dist/cli.js |
option ids/category/values/names/descriptions exact, per-tier read-back (5 tiers), clear→default, ultra exact error text + state unchanged via non-mutating mode read-back, XHIGH/'' boundary, model-switch retention (option + selected tier), unknown-id rejection, non-string probes |
31/31 (01-ab-head-bundle.png) |
| head parity | packages/cli/dist/index.js |
same | 31/31 |
base control (4a79517) |
base worktree packages/cli/dist/index.js |
ids exactly ['mode','model'], no thought_level, setter rejects reasoning_effort with Unsupported configId, mode/model positive controls, unknown-id same default branch |
13/13 (02-ab-base-cell.png) |
| base control (swap check) | base bundle swapped into dist/cli.js |
same 13 oracles (validates the A/A bundle swap) | 13/13 |
The flip is exactly the feature: base advertises two options and rejects the id; head advertises the third, applies all five tiers, clears to default, and rejects ultra/XHIGH/'' with -32602 leaving state untouched (proven over the wire by re-setting the current mode, which returns configOptions without touching effort).
Reviewer Test Plan walk-through (re-performed per step): options include thought_level with Default + five tiers — exact match; set Extra high → xhigh; set Default → default; ultra → invalid-params error with state unchanged; option preserved after a model switch — option and value (xhigh) both retained.
Control purity: base worktree at HEAD^1 with a root node_modules symlink farm (1163 third-party entries; lockfile/package.json untouched by the PR) plus mirrored per-package nested node_modules (123 third-party pins); every @qwen-code/* link realpath-asserted into the base tree before building (scripts/build.js --cli-only). Built base acpAgent.js contains 0 occurrences of reasoning_effort and base reasoning-effort.js 0 of applyReasoningEffort; head's contain both. For the bundle-level A/A, dist/ was swapped to the base bundle and back; each swap was sha256-verified and probed with the 13-assertion harness in the expected mode.
Secondary claim: HTTP transport routing + hardened rejection (the delta)
The qwen serve HTTP transport routes only model/mode; 6c4a28e hardens both ends from the shared constant HTTP_ACP_CONFIG_OPTION_IDS = ['model','mode']:
configOptionsForfilters every setter-paired response. All four call sites verified structurally:session/new(dispatch.ts:1403),session/load/resume (:1594),session/fork(:1857), and the setter result (:2270) — one choke point, so a single mutation kills every gated surface.- The setter rejects unroutable ids with
-32602 ConfigId not supported by this transport: <id> (supported: model, mode)instead of the oldUnknown configId; a notification (noid) for an unroutable configId writes no response and forwards nothing to the bridge. - Raw-state surfaces (context status) intentionally keep the child's full unfiltered set — the doc comment says so, and the loud setter rejection is the compensating control.
Over-the-wire suite (transport.test.ts, real HTTP loopback, FakeBridge child advertising reasoning_effort): 293/293 (04-transport-gate.png) — round 1's 292 plus the new notification test; the routable-set test now pins the hardened message text.
Secondary claim: applyReasoningEffort single-implementation refactor
The core helper (set + read-back; clearing always true; returns false when Config.setReasoningEffort no-ops under reasoning: false) is the implementation used by all four surfaces: the ACP setter, systemController (both sites), the /effort slash command, and — new in this round (54163b7) — the /effort dialog (use-effort-command.ts). Behavior preservation: the read-back source (contentGeneratorConfig) is independent of settings persistence, and each surface's chat/debug message keys off the helper's boolean exactly as before. The dialog's tests exercise the real helper through a mocked Config seam (no core mock), so the mutation below bites the production code path.
Mutation matrix (05-mutation-matrix.png, details in logs/mutation-matrix.json + logs/mutation-extras.json)
| # | guard (mutated) | pinning suite | control | verdict |
|---|---|---|---|---|
| M1 | core read-back → return true |
core reasoning-effort.test.ts | green (13/13) | KILLED — expected true to be false in "reports false when setReasoningEffort no-ops" |
| M2 | acpAgent thinking-disabled block removed (coarse) | acpAgent.test.ts (new test) | green (387/387) | KILLED — expected "spy" to be called with arguments: ['xhigh'] |
| M2b | acpAgent throw disabled, apply call kept (fine) | same | green | KILLED — promise resolved … instead of rejecting at the .rejects.toThrow( (line 6379) |
| M3 | HTTP advertisement filter removed | transport.test.ts (session/new ids) | green (293/293) | KILLED — ['model','reasoning_effort','mode'] ≠ ['model','mode'] |
| M4 | HTTP setter gate: unroutable ids fall into mode branch | transport.test.ts (routable-set) | green | KILLED — got invalid mode "high" … instead of ConfigId not supported by this transport… |
| M5 | dialog guard !applied → false |
use-effort-command.test.ts (warn case) | green (5/5) | KILLED — success text where warning expected |
| M5b | dialog guard !applied → true |
use-effort-command.test.ts (success case) | green | KILLED — warning text where requested confirmation expected |
| M6 | slash-command guard !applied → false |
effort-command.test.ts | green (7/7) | KILLED — "reports thinking is disabled when setReasoningEffort is a no-op" |
| M7 | dispatch forwards unroutable id to bridge before the id check (notification path) | transport.test.ts (notification test) | green | KILLED — expected 'high' to be undefined (bridge.lastApprovalMode, line 4778) |
9/9 killed, each by its intended behavioral assertion; every unmutated control green; all sources restored byte-identical (sha256-verified, git status clean). M2 vs M2b shows the coarse mutant removed the apply call along with the throw (it sat in the condition); the fine mutant proves the test also pins the throw itself. No survivors.
Findings
None blocking. Informational:
- Live-inference integration tests time out in this container — pre-existing / load-induced, not PR-attributed (
06-integration-aa.png). Full-file run at head:basic smoke testandhandles exit plan mode…failed 3/3 retries onsession/prompttimeout. Isolated A/A against the base bundle:handles exit plan mode…fails identically on both arms (Request 5 (session/prompt) timed out,1 failed | 10 skipped (11)each; scripted equality check passed) → pre-existing at base tip4a79517815.basic smoke testpasses on both arms in isolation: base 97.9 s, head 100.6 s. These tests run real inference against the ambient model endpoint with no fake server; their natural round-trip (~50–90 s here) straddles the hard 60 sREQUEST_TIMEOUT_MS, so parallel file load on the shared runner tips them over — a speed-correlated timeout, fully correlated across retries, not random flake. The PR's own config-option integration test (no inference) passed in the full-file run. Nothing in the diff touches the prompt path; the A/A isolates the cause to the base tip + ambient latency.
- Non-string
value→-32603 Internal error(carried from round 1, re-measured): zodinvalid_typewrapped by the ACP SDK, id-agnostic, byte-identical on base and head formode; string violations of the new id get the PR's precise-32602. Pre-existing SDK behavior; no action needed.
Not covered
- Per-commit attribution: depth-2 checkout —
git rev-list HEAD^1..HEAD^2reaches 1 commit while the snapshot lists 8; verified the aggregateHEAD^1..HEADdiff. The three post-round-1 commits are covered behaviorally (docs merge via conflict-marker check + rendered bullet; dialog migration via M5/M5b; hardening via M4/M7 + transport suite), not individually diffed. - Live JetBrains rendering and provider-specific inference behavior (PR-declared out of scope; mapping/clamping path untouched by the diff).
- Trial merge into current
main(no network in this environment); base tip4a79517815is what the workflow provided, and the PR already carries the 2026-08-08 main merge (d703074) whose one conflict (JetBrains doc) was checked clean of markers. - Repo-wide gates; only affected-workspace suites ran (core reasoning-effort 13, cli acpAgent 387 + worktree/effort-command/use-effort-command/systemController 36, transport 293, integration ACP file).
session/load/session/forkresponses carrying a previously-set effort were not driven end-to-end; their gating rests on the singleconfigOptionsForchoke point proven by M3 through thesession/newassertion (effort is session-scoped by design, not persisted).- Live
qwen serve /acpprobe against a real child (round-1 scope kept): HTTP behavior is verified by the real-loopback transport suite whose FakeBridge child advertisesreasoning_effort.
Methodology
Environment: CI node:22-bookworm container, merge-ref checkout (HEAD merge commit, HEAD^1 base tip, HEAD^2 PR head), npm ci + npm run build pre-run at head; the production bundle was produced via npm run bundle at head and at the base worktree. Head cells drove the bundle and the tsc dist entry; the base cell used a scratch worktree at HEAD^1 with a symlinked root node_modules farm (lockfile unchanged), mirrored per-package nested node_modules, and @qwen-code/* links realpath-asserted into the base tree, rebuilt via scripts/build.js --cli-only. Harnesses are plain JSON-RPC stdio clients (acp-harness.mjs) plus the repo's own vitest suites; the bundle A/A swapped dist/ with sha256 + harness-mode probes on each side. Mutation matrix scripted in mutation-matrix.mjs (five mutants) with four finer mutants applied ad hoc; every mutation restored and sha256-verified. Assertion tally is generated by tally-assertions.mjs from the real logs, including the two A/A equality checks: harness cells 88, suites 738 (core 13 + acpAgent 387 + transport 293 + cli-four 36 + integration 9), isolated A/A cells 2, A/A equality checks 2, mutation matrix 18 = 848. Raw logs in logs/, captures in evidence/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下: Autofix review round — no action neededNo actionable feedback arrived this round, so no code changes were made and no commit was created. The branch remains at its current head.
Critical-only mode is active after five change-producing rounds. The single deferred non-Critical item (the bot's own prior round-summary comment) is an audit record and remains open for human follow-up; per policy it is not acted on in this round. 中文说明Autofix 审阅轮次——无需处理本轮没有收到可处理的反馈,因此未做任何代码改动,也未创建提交。分支保持在当前 head。
在完成 5 个产生改动的轮次后,已进入仅处理 Critical 的模式。唯一被延后的非 Critical 条目(机器人此前的轮次总结评论)属于审计记录,保持开放留待人工跟进;按策略本轮不予处理。 Deferred non-Critical feedbackCritical-only mode is active after 5 change-producing rounds. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明完成 5 个产生改动的轮次后进入仅处理 Critical 的模式。本轮可执行区域已排除下方非 Critical 反馈;这些条目保持开放,留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后;达到预算的作者(如有)在下方点名。(评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (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: 863 passed · 0 failed · 863 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:863 通过 · 0 失败 · 863 总计 Verification reportPR #8526 — feat(cli): expose reasoning effort through ACP (follow-up round 3)Verdict: 中文 — 判定:merge-ready · 863/863 断言通过 · 0 意外失败
Previous-finding status (round 2 → round 3)Round 2 verified head
Central claim + A/BCentral claim: an ACP stdio client receives a Mock-free wire harness (
The flip is exactly the feature: base advertises two options and rejects the id; head advertises the third, applies all five tiers, clears to Reviewer Test Plan walk-through (re-performed per step at the new head): options include Control purity: base worktree at Secondary claim: HTTP transport routing + hardened rejectionThe
Over-the-wire suite ( Secondary claim:
|
| # | guard (mutated) | pinning suite (unmutated control) | verdict |
|---|---|---|---|
| M1 | core read-back → return true |
core reasoning-effort.test.ts (13/13 green) | KILLED — expected true to be false in "reports false when setReasoningEffort no-ops" |
| M2b | acpAgent throw disabled, apply call kept (fine mutant) | acpAgent.test.ts (396/396 green) | KILLED — promise resolved … instead of rejecting in the PR's own selector test |
| M3 | HTTP advertisement filter removed | transport.test.ts (300/300 green) | KILLED — ['model','reasoning_effort','mode'] ≠ ['model','mode'] on session/new |
| M4 | HTTP setter gate: unroutable ids fall into mode branch | transport.test.ts | KILLED — got invalid mode "high" (expected one of: plan, default, auto-edit, auto, yolo) instead of the hardened message |
| M5 | dialog guard !applied → false |
use-effort-command.test.ts (green) | KILLED — success text where "thinking is currently disabled" warning expected |
| M5b | dialog guard !applied → true |
use-effort-command.test.ts | KILLED — warning text where requested confirmation expected |
| M6 | slash-command guard !applied → false |
effort-command.test.ts (green) | KILLED — "reports thinking is disabled when setReasoningEffort is a no-op" |
| M7 | dispatch forwards unroutable id to bridge (notification path) | transport.test.ts | KILLED — expected 'high' to be undefined (bridge.lastApprovalMode); routable-set test also red |
7/7 killed, each by its intended behavioral assertion; every unmutated control green in this round; all mutated files restored byte-identical (sha256-verified, git status clean). Round 2's coarse M2 is subsumed by the fine M2b (the throw itself is pinned, not just the apply call). No survivors.
Findings
None blocking. Informational:
- Integration-test timeout attribution confirmed environmental (carried, re-measured). Round 2 saw
basic smoke test/handles exit plan mode…fail 3/3 retries on the 60 ssession/prompttimeout while passing only in isolation (~98–101 s). This round the identical file passes 11/11 at the merged head with smoke at 9.1 s — a ~10× swing in natural duration on unchanged prompt-path code, which is the definition of the speed-correlated regime round 2 described (the diff touches no prompt path). The PR's own config-option integration test passed in both rounds. No action needed; the tests themselves are sound given a normal-latency endpoint. - Non-string
value→-32603 Internal error(carried, re-measured). ACP SDK request-schema validation, id-agnostic: head'sreasoning_effortand base'smodereturn byte-identical{code: -32603, message: "Internal error"}fornull/42/{a:1}(12 scripted checks). String violations of the new id get the PR's precise-32602. Pre-existing SDK behavior; no action needed.
Not covered
- Per-commit attribution: depth-2 checkout —
git rev-list HEAD^1..HEAD^2reaches 1 commit while the snapshot lists 8 (and the shallow boundary makes the count itself unreliable); verified the aggregateHEAD^1..HEADdiff. The PR head is the same commit round 2 covered behaviorally, and this round re-ran everything at the new merge point. - Live JetBrains rendering and provider-specific inference behavior (PR-declared out of scope; mapping/clamping path untouched by the diff).
- Trial merge into current
main(no network in this environment); base tipe20601d6c4is what the workflow provided, and the merge into it was clean (effective diff = exactly the PR's 13 files). - Repo-wide gates; only affected-workspace suites ran (core 13, cli acpAgent 396 + worktree/effort-command/use-effort-command/systemController 36, transport 300, integration ACP file 11).
session/resume/session/forkresponses carrying a previously-set effort were not driven end-to-end; their gating rests on the singleconfigOptionsForchoke point proven by M3 through thesession/newassertion (effort is session-scoped by design, not persisted).- Live
qwen serve /acpprobe against a real child (round 1–2 scope kept): HTTP behavior is verified by the real-loopback transport suite whose FakeBridge child advertisesreasoning_effort. - No base-arm A/A for the integration timeout finding this round — the failure did not reproduce at head, so there was nothing to attribute; the head-side re-run itself is the re-measurement.
Methodology
Environment: CI node:22-bookworm container, merge-ref checkout (HEAD = merge commit 9f0b36c684, HEAD^1 = base tip e20601d6c4, HEAD^2 = PR head 6c4a28e556), npm ci + npm run build pre-run at head; the production bundle was produced via npm run bundle at head. Head cells drove the bundle and the tsc dist entry; the base cell used a scratch worktree at HEAD^1 with a symlinked root node_modules farm (lockfile unchanged), mirrored third-party nested node_modules, and @qwen-code/* links realpath-asserted into the base tree, rebuilt via scripts/build.js --cli-only. The stdio harness (acp-harness.mjs) is a plain JSON-RPC client spawning the real CLI with per-run QWEN_HOME; the transport suite is the repo's own real-loopback vitest file. Mutations were applied by exact-text substitution, run against the pinning vitest suites (which execute from source), and restored with git checkout + sha256 verification. Assertion tally: harness cells 91 (head 36 + head-dist 36 + base 19), unit suites 745 (core 13 + cli-four 36 + acpAgent 396 + transport 300), integration 11, mutation kills 7 + unmutated controls 4, structural checks 5 (four configOptionsFor call-site mappings + docs conflict-marker check) = 863, all pass. Raw logs in logs/, captures in evidence/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.10. |





















What this PR does
This adds a standard ACP session selector for reasoning effort. ACP clients now receive a
thought_leveloption with Default, Low, Medium, High, Extra high, and Max choices, and can apply or clear the requested tier throughsession/set_config_option.The selector reuses Qwen Code's existing reasoning-effort ladder and runtime setter, so provider-specific mapping and clamping remain unchanged. Invalid values are rejected at the protocol boundary. The JetBrains integration guide now documents the new control.
Why it's needed
Qwen Code already exposes reasoning effort in the terminal, but ACP hosts such as JetBrains could only configure mode and model. Users therefore could not control thinking effort from the host's native agent controls even though the runtime already supported all five tiers.
Reviewer Test Plan
How to verify
Start Qwen Code as an ACP agent and create a session. Confirm that the returned configuration options include
reasoning_effortin the standardthought_levelcategory with Default plus all five tiers. Set the option to Extra high and confirm the returned current value becomesxhigh; set it back to Default and confirm it becomesdefault. An unsupported value such asultrashould return an invalid-params error without changing the current setting.Evidence (Before & After)
Before: ACP session responses exposed only mode and model;
reasoning_effortwas absent and the setter rejected that config id.After: a real bundled
qwen --acpprocess advertises all six choices, preserves the option after a model switch, appliesxhigh, and clears it back todefault.Tested on
Environment (optional)
macOS arm64, Node.js 22.23.2. Verified with the production bundle over a real ACP stdio subprocess; no model request or external network call was required.
Risk & Scope
Linked Issues
Closes #8514
中文说明
此 PR 做了什么
此 PR 为 reasoning effort 增加标准 ACP 会话选择器。ACP 客户端现在会收到一个
thought_level选项,其中包含 Default、Low、Medium、High、Extra high 和 Max,并可通过session/set_config_option应用或清除所请求的档位。该选择器复用 Qwen Code 现有的 reasoning-effort 档位列表与运行时 setter,因此各 provider 原有的映射和钳制行为保持不变。非法值会在协议边界被拒绝。JetBrains 集成文档也补充了这一控制项。
为什么需要
Qwen Code 已经在终端中暴露 reasoning effort,但 JetBrains 等 ACP 宿主此前只能配置 mode 和 model。虽然运行时已经支持全部五档,用户仍无法通过宿主原生的 agent 控件调整思考强度。
Reviewer 测试计划
如何验证
以 ACP agent 启动 Qwen Code 并创建会话。确认返回的配置选项包含标准
thought_level类别下的reasoning_effort,并提供 Default 与全部五个档位。将其设置为 Extra high,确认返回的当前值变为xhigh;再设置回 Default,确认变为default。传入ultra等不支持的值时,应返回 invalid-params 错误且不改变当前设置。证据(修改前与修改后)
修改前:ACP 会话响应只暴露 mode 和 model;
reasoning_effort不存在,setter 会拒绝该 config id。修改后:真实打包的
qwen --acp进程会广播全部六个选项,在模型切换后继续保留该选项,能够应用xhigh,并能清除回default。测试平台
环境(可选)
macOS arm64,Node.js 22.23.2。通过生产 bundle 和真实 ACP stdio 子进程完成验证;无需发送模型请求,也无需外部网络调用。
风险与范围
关联 Issue
Closes #8514