fix(web-shell): persist model reasoning preferences - #10489
Conversation
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
E2E test reportEnvironment: local macOS checkout, fake daemon/request ledger, Playwright Chromium.
The scenarios verify Welcome lazy session creation and first-send ordering, persisted tier changes in existing and running sessions, Supporting regression evidence: CLI Session 730/730, ACP agent 517/517, TypeScript SDK 1,700/1,700, WebUI 645/645, WebShell 4,572/4,572, plus root build and typecheck. |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 6 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Template looks good ✓ Problem: observed gap, not theory. WebShell's model/reasoning choice today lives only on the live daemon session — a new session starts from scratch, and an incompatible persisted tier could leak into a later model switch. This is the persistence half of the reasoning-controls work already landed in #9599, #10008, #9574 and #10244, superseding the closed #10011 with a deliberately smaller contract (reuses Direction: aligned. Persisting a preference the product already exposes is expected behavior, and reusing the existing Size: cross-package (cli, sdk-typescript, web-shell, webui, vscode schema) and touches Approach: scope feels right. Every file in the diff serves the feature — no drive-by edits — and the committed design doc pins the contract (absent = model default, Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 真实存在的缺口，不是理论问题。WebShell 的模型/思考选择目前只存在于存活的 daemon 会话上——新会话从零开始，且与能力不兼容的持久化档位可能泄漏到之后的模型切换。这是 #9599、#10008、#9574、#10244 已落地的思考控制系列中"持久化"这半部分，用更小的契约取代已关闭的 #10011（复用 方向： 对齐。持久化产品已暴露的偏好是预期行为；复用现有 规模： 跨包（cli、sdk-typescript、web-shell、webui、vscode schema）并触及 方案： 范围合理。diff 中每个文件都服务于该功能，无顺手改动；已提交的设计文档固定了契约（未设置 = 模型默认、 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Visual CI noteThe visual workflow's incomplete-preview comment is a baseline failure, not a PR regression. Both the PR head and the merge base fail the same two |
Code reviewMy independent baseline for "persist WebShell reasoning preferences" was: reuse Things I checked against the surrounding code, all clean:
The two questions from the first pass are now settled by author confirmation — both behaviors are deliberate. Daemon-owned Follow-up commit
|
| File | What changed |
|---|---|
docs/design/webshell-reasoning-preferences.md |
New design doc pinning the stored-value semantics, lazy lifecycle, and reconciliation rules |
packages/cli/src/acp-integration/acpAgent.ts |
reasoning_effort mutation validated against live options, persistence wired via _meta, option hidden on unsupported models |
packages/cli/src/acp-integration/acpAgent.test.ts |
Mutation validation, persist meta round-trip, unsupported-model rejection tests |
packages/cli/src/acp-integration/model-configuration.ts |
Strict selection types plus parse/validate/apply helpers and the persisted-state projection |
packages/cli/src/acp-integration/model-configuration.test.ts |
Capability matrix and persisted-projection cases |
packages/cli/src/acp-integration/session/Session.ts |
persistReasoningSelection, post-switch reconciliation, scoped clearing with write rollback |
packages/cli/src/acp-integration/session/Session.test.ts |
Persistence and reconciliation coverage incl. user/workspace scope writes |
packages/cli/src/config/settingsSchema.ts |
Adds the none option and rewords the setting description |
packages/cli/src/config/settingsSchema.test.ts |
none accepted, default never stored |
packages/cli/src/serve/routes/session.ts |
Validates the persist flag, forwards it via _meta, returns shaped configOptions + persisted |
packages/cli/src/serve/server.test.ts |
Route persist contract and the untrusted-_meta case |
packages/cli/src/serve/workspace-providers-status.ts |
Welcome previews project the persisted selection per model |
packages/cli/src/serve/workspace-providers-status.test.ts |
Preview projection matrix incl. incompatible and mandatory cases |
packages/cli/src/utils/modelConfigUtils.ts |
CLI startup reads none as reasoning disabled unless thinking is mandatory |
packages/cli/src/utils/modelConfigUtils.test.ts |
none loads as an explicit disable without a warning |
packages/sdk-typescript/src/daemon/DaemonClient.ts |
Strict ReasoningSelection value type and an options overload carrying persist |
packages/sdk-typescript/src/daemon/DaemonSessionClient.ts |
Passes the persist option through |
packages/sdk-typescript/src/daemon/index.ts |
Exports ReasoningSelection |
packages/sdk-typescript/src/daemon/types.ts |
ReasoningSelection type and persisted result field |
packages/sdk-typescript/test/unit/DaemonClient.test.ts |
Request shape with persist |
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts |
Session client pass-through |
packages/vscode-ide-companion/schemas/settings.schema.json |
Enum gains none, description synced |
packages/web-shell/client/App.tsx |
Welcome intent bookkeeping, clear-don't-restore on model switch, persist on live changes |
packages/web-shell/client/App.test.tsx |
Updated stale-disable expectations to the clear semantics |
packages/web-shell/client/components/ChatEditor.tsx |
Strict selection callbacks, toggle sends default, label fallback for none/default |
packages/web-shell/client/components/ChatEditor.test.tsx |
Runtime language switch localization and unknown-default rendering |
packages/web-shell/client/components/ChatPane.tsx |
Persist pass-through on live changes |
packages/web-shell/client/components/messages/SettingsMessage.tsx |
Hides model.reasoningEffort from the generic settings panel |
packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx |
Hidden-key coverage |
packages/web-shell/client/e2e/utils/mockDaemon.ts |
Mock daemon honors persist and the new response shape |
| …and 11 more files | e2e smoke scenarios (10 reasoning flows + the New-task preview round trip), i18n keys for high/max, sessionPreparation persist-on-first-send, webui provider/actions/mappers/types plus their tests |
Testing evidence — the PR's own CI, read via API
This is an unattended CI run, so no PR code was built or executed here; the evidence below is the PR's own check results fetched once via the API. Context on the previous head: 14673c7 settled with the primary ubuntu unit suite (Qwen Code CI) cancelled — never green — and the Java 17 transport flake below; the approve-on-green marker on that head correctly never fired. The follow-up commit re-triggers everything; the table tracks the current head 6a9bf199 and the finalize job rewrites it in place once CI settles.
Final CI results for 6a9bf19 (auto-updated by the triage finalize job after CI completed):
| Check | Conclusion |
|---|---|
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Integration Tests (no-AK, No Sandbox) |
✅ success |
Live Host (macos-latest) |
✅ success |
macos-latest / Java 21 |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Serve A/B (ubuntu-latest, Node 22.x) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
ubuntu-latest / Java 17 carries a known-flake history. On 14673c7 its 5 errors were all in DaemonSessionClientTest and all transport/timing failures (GET /capabilities transport failed, Timeout, SessionCreationOutcomeUnknown POST /session) — no reasoning/model assertions. This PR changes zero Java files, and the identical suite passed on Java 11, Java 21 (ubuntu), and Java 21 (macOS, Windows) at that same commit; the repo also runs a scheduled CI failure patrol, consistent with known flakiness. If it lands red on the new head with the same signature, it stays classified pre-existing; any new failure shape in that suite would be a real finding.
The serve-daemon A/B run on the previous head diffed base vs PR responses across 12 scenarios and found no response changes; it re-runs on the new head (queued above).
Sandboxed verification would still settle the one claim CI cannot: @qwen-code /verify — that with a real daemon the persisted model.reasoningEffort actually hits disk in the documented first-send order (create/attach → model → reasoning → prompt) and that an incompatible tier round-trip clears it on disk. The earlier /verify run on this PR was cancelled before producing a report, and the disk-level evidence otherwise remains mocked-settings unit tests plus mock-daemon Playwright flows — the author's real-daemon local results (including the New-task regression path) are their claim, attributed here, not evidence this run carries.
中文说明
代码审查
我对"持久化 WebShell 思考偏好"的独立基线方案是：复用 model.reasoningEffort、在 daemon mutation 上加可选 persist 标志、模型切换时"清除而非降级"、界面只根据 daemon 确认更新。本 PR 正是这个形状，且在每个边界上用了严格类型——几处比基线更好：mapper 完全移除了旧的 fallbackEffort 回退（状态只来自 daemon 确认的 configOptions，消灭了"恢复旧档位"这类 bug）；default 只是清除命令，不落盘也不渲染。
与周边代码核对过的点，均无问题：
- 顺序：
Session.setModel中reconcileReasoningSelection在await switchModel(...)成功之后执行——切换失败不会动持久化设置；thinkingMandatory读的是切换后的配置。 none/default跳过extra_body/samplingParams清理： 安全。选none时 Core 请求管线本身会强制下线形态（pipeline.ts删除enable_thinking/thinking_budget并对分档 DashScope 模型发reasoning_effort: 'none'），即使extra_body里有静态enable_thinking: true；选default时遵从静态配置本就是正确语义。- 路由信任边界： HTTP 路由只从校验过的布尔
persist字段派生_meta，从不转发客户端自带的_meta（有显式测试覆盖）；写设置失败会先reloadScopeFromDisk回滚再抛出。 - 响应结构变化： TypeScript SDK 同 PR 更新；Java SDK 完全不调用该端点，无孤儿消费者。
- 新增字段读取点：
persist、persisted、defaultEffort、两个_meta键均有设置有读取；model.reasoningEffort从通用设置面板隐藏，保证只有一个入口。
第一遍的两个问题已由作者确认，均为有意设计。 受管 standalone 会话保持"不写共享设置"的边界：这类会话上持久化修改直接失败，优于悄悄降级为仅 live 写入共享的用户/工作区默认值；旧版 daemon 不带 persisted: true 的响应无法证明持久化契约，因此报错而不是把值显示为已持久。两点确认为意图后，第一遍已无未决问题。
后续提交 6a9bf199 —— Welcome 预览回归
作者自行发现了本 PR 中的一个真实回归（Welcome 上选 medium → 首次提示词已持久化 → New task 却用陈旧的缓存 provider 预览重建控件，仍显示 low），并在状态边界处修复，而不是在清除时重新拉取 providers——这是更小的修法，也是我会选的：
withPersistedReasoningPreview是对缓存 workspace-provider 快照的纯函数式不可变更新：只改匹配的模型条目、只改本就声明了reasoning_effort选项的模型、无变化时返回同一引用。失败或未确认的持久化不动预览。clearSession()在推导下一个 Welcome 模型列表前等待在途的持久化 mutation。等待的是"完成"而非"成功"（finally中 resolve，清除侧用.catch(() => undefined)），因此失败的持久化依然能迅速清除、不泄漏进预览。- 迟到或被取代的结果不会污染预览：应用路径要求
actionToken > appliedReasoningActionToken，且setConnection更新器内再次校验会话身份、模型代际与当前模型；clearSession在推导清除后状态前已将 session ref 置空——我推演了三种交错（先慢后快、先快后慢、清除时在途），结果都收敛为 daemon 确认值或不更新。
测试钉住的是回归本身而非实现：清除后预览保留、清除等待在途 mutation 再 detach、未确认持久化时预览停在旧值且清除仍可完成；e2e 直接走报告路径（持久化 → 发送 → turn complete → New task → 模型按钮仍显示 Medium），并断言没有额外的 providers GET、没有创建第二个会话。
两个提交均无阻塞问题。
测试证据
这是无人值守 CI 运行，未构建或执行任何 PR 代码；以上证据为通过 API 一次性读取的 PR 自身检查结果。前一提交 14673c7 的最终状态：主 ubuntu 单测套件（Qwen Code CI）被取消、始终未绿，另有下述 Java 17 传输类易失败；该提交上的 approve-on-green 标记因此正确地未触发。后续提交重新触发全部 CI；表格跟踪当前提交 6a9bf199，finalize 作业会在 CI 结束后原地更新。
ubuntu-latest / Java 17 有已知易失败历史。 在 14673c7 上其 5 个错误全部位于 DaemonSessionClientTest，均为传输/超时类失败，不涉及思考/模型断言。本 PR 未改动任何 Java 文件，同一套测试在同一提交的 Java 11、Java 21（ubuntu）及 macOS/Windows 的 Java 21 上全部通过；仓库亦有定期 CI 失败巡检，与已知易失败一致。若新提交上以同样特征再次变红，仍判定为既有问题；该套件中任何新形态的失败才是真实发现。
前一提交上的 serve daemon A/B 对比了 base 与 PR 在 12 个场景下的响应，无差异；新提交上该作业重新运行（排队中）。
沙箱验证仍可补齐 CI 无法覆盖的唯一声明：@qwen-code /verify —— 在真实 daemon 下验证 model.reasoningEffort 确实按文档顺序（创建/挂载 → 模型 → 思考 → 提示词）写盘，且不兼容档位往返确实从磁盘清除。此前本 PR 的 /verify 运行在生成报告前被取消；否则磁盘层证据仍只有 mock 设置的单测与 mock daemon 的 Playwright 流程——作者的真实 daemon 本地结果（含 New-task 回归路径）在此仅作为其声明转述，不是本次运行携带的证据。
— Qwen Code · qwen3.8-max
Reviewed at 6a9bf199d5cd0dd6b58ed88b55a9c51b358eb36c · re-run with @qwen-code /triage
|
Confidence: 4/5 — the same clean, tightly scoped consolidation as the first pass, now including a self-discovered regression fixed with a minimal, well-guarded patch; both first-pass questions confirmed as intent; waiting on the new head's CI before approving. Stepping back: this is the sixth PR in the author's reasoning-controls series, and it still reads like consolidation rather than expansion — it reuses the existing setting key, deletes a fallback heuristic instead of adding one, and pins the contract in a design doc. Against my independent proposal (reuse The follow-up commit is the strongest signal in this re-run. The author found the Welcome-preview regression themselves, reported it precisely (stale cached provider snapshot after a confirmed persist), and fixed it at the state boundary — a pure snapshot update plus a clear-time wait — instead of the heavier clear-time provider refresh, explicitly avoiding a provider GET, cross-workspace ownership fallback, and late-refresh overwrite. I verified the guards statically rather than taking that framing on faith: the token ordering ( Both first-pass questions came back confirmed as deliberate (standalone sessions keep the no-shared-persistence boundary; unversioned What keeps this at 4 rather than 5 is evidence, not code: the primary ubuntu suite on the first head ended cancelled rather than green, the Approval is therefore deferred until CI lands green on 中文说明置信度：4/5 —— 与第一遍相同的干净、收口的整合，现已包含一个自发现回归的最小化、有防护的修复；第一遍的两个问题均已确认为有意设计；批准前等待新提交的 CI。 整体来看：这是作者思考控制系列的第六个 PR，读起来仍是收口而非扩张——复用既有设置键、删除回退启发式而不是新增、并用设计文档固定契约。与我的独立方案（复用 后续提交是本次重审中最强的信号。作者自行发现了 Welcome 预览回归，精确描述了成因（确认持久化后缓存 provider 快照陈旧），并在状态边界处修复——纯快照更新加清除时等待——而不是更重的清除时 provider 刷新，明确避免了 provider GET、跨工作区所有权回退和迟到刷新覆盖。我没有轻信这一表述，而是静态验证了防护：令牌顺序（ 第一遍的两个问题均被确认为有意设计（standalone 会话保持不写共享设置的边界；无版本协商的 停在 4 而非 5 的原因是证据而非代码：第一个提交的主 ubuntu 套件以取消告终而非绿色， 因此批准将延迟到 — Qwen Code · qwen3.8-max Reviewed at |
|
Author confirmation on the two non-blocking questions:
I am also requesting the suggested real-daemon verification for the on-disk first-send ordering and incompatible-tier clearing paths. |
|
@qwen-code /verify |
|
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: 92 passed · 0 failed · 92 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：92 通过 · 0 失败 · 92 总计 抖动门： Verification reportPR #10489 Deep Verification — fix(web-shell): persist model reasoning preferencesVerdict:
中文摘要结论： A/B 结论（真实 daemon，head 对 base）：
Findings： 1 条 Suggestion（ 未覆盖： TUI 流程（PR 自述范围外）、真实模型 prompt 往返（持久化不经过 prompt 路径）、Live Voice managed 会话（环境不可达）、Playwright visuals/perf、运行时中英文切换（仅静态核对 i18n key + DOM 测试）、Java SDK/公共 ACP（仅 diff 级核对）、整仓套件（只跑定向门禁）。 Central claim and A/B proofCentral claim: WebShell reasoning selection persists across daemon sessions via Method: two real daemons — head build (port 18999) and a control build from Control purity: the PR's diff against Cell table (36 scripted assertions per arm; all passed)
The flip count: 5 behavior classes (write-on-persist, CorrectionsNone (first round; no prior review claims to correct). FindingsS1 (Suggestion) — the managed-conversation guard in
|
| Row | Mutation | Suite | Result |
|---|---|---|---|
| CTL (positive control) | persist key typo in persistReasoningSelection |
Session reconcile (7) | KILLED 1/7 — expected 'max' to be 'none', the intended write-path assertion |
| M1 | route persist plumbing reverted (whole hunk) | server config-option (3) | KILLED 3/3 |
| M2 | non-boolean persist 400 check deleted |
server config-option (3) | KILLED 1/3 (its own test) |
| M3 | reconcile call deleted from setModel |
Session reconcile (7) | KILLED 4/7 (survivors bypass setModel) |
| M6 | mustClear: !supported → false |
Session reconcile (7) | KILLED 4/7 |
| M7 | mustClear → always true |
Session reconcile (7) | KILLED 1/7 — proves keeps a compatible persisted tier pins the over-clearing direction |
| M5 | none semantics reverted in resolveCliGenerationConfig |
modelConfigUtils (61) | KILLED 1/61 — expected { effort: 'xhigh' } to be false, intended assertion |
| M4 | parse accepts any string | model-config (43) + Session (7) | SURVIVED → redundant defence (N1) |
Note on the 3 M3/M6 survivors: two (persists none…, treats default…) exercise persistReasoningSelection directly and correctly don't depend on reconcile; keeps a compatible persisted tier passes trivially when reconcile never runs but is pinned against over-clearing by M7 — the suite covers both directions between rows.
Targeted gates (all at verified head)
| Gate | Scope | Result |
|---|---|---|
| cli | acpAgent, Session, model-configuration, settingsSchema, server, workspace-providers-status, modelConfigUtils test files | 2529/2529 passed (7 files) |
| sdk-typescript | DaemonClient, DaemonSessionClient unit tests | 432/432 passed |
| webui | actions, mappers, DaemonSessionProvider | 394/394 passed (149 + 245) |
| web-shell (vitest) | App, ChatEditor, SettingsMessage.dom, sessionPreparation | 716/716 passed |
| web-shell (Playwright smoke) | full @smoke suite incl. the 10 reasoning scenarios (fake daemon + Chromium 1228) |
45/45 passed (4.3 min) |
| Generated artifact | scripts/generate-settings-schema.ts re-run |
regenerated settings.schema.json byte-identical to committed (clean git status) |
Repo-suite totals are reported here, not folded into assertions.json (which counts only this round's harness assertions).
Not covered
- TUI flows — PR declares them out of scope; not exercised.
- No prompt was ever sent — the persistence path is config-only; model-request wire behavior (provider clamping) is untouched by the diff and was not A/B'd. Session creation and model switching did run against the lane's real model proxy.
- Managed-conversation (Live Voice) sessions — unreachable from REST in this environment; their guard is the subject of S1.
- Runtime EN/ZH switching in a browser — i18n keys verified statically (added
reasoning.effort.high/maxin both EN and ZH tables) and label rendering covered by the DOM/unit tests; no live language-toggle e2e was run. - Playwright visuals/perf suites — not run (smoke only).
- Java SDK / public ACP protocol — not in this repo / diff-level check only: the change rides on
_metakeys inside the daemon boundary; no ACP schema types changed. - Full repo-wide suite — targeted gates only; PR's own CI covers the rest.
- Uncalibrated-replay caveat does not apply (no workflow/CI YAML changes in this PR).
Methodology
Ran inside the CI verify container (node v22.23.2, merge-ref checkout at depth 2, npm ci + npm run build pre-done at head). A/B daemons were the real packages/cli/dist/index.js serve on each arm with isolated QWEN_HOME/workspace fixtures; the base arm ran a base-tree rebuild of packages/cli only (tsc --build --noCheck, justified above; logs in logs/). The SDK wire harness pointed each arm's compiled SDK at a real loopback capture server and asserted both sides of the wire (wire-sdk.mjs, wire-*.json). Mutations ran in a scratch worktree at HEAD with dependency dists symlinked from untouched packages; every mutant row includes the exact vitest filter and log (logs/matrix-*.log). Playwright smoke used the PR's own @smoke suite after installing the matching Chromium (1228) into the lane's browser cache. All raw per-cell daemon responses are in raw-head.json/raw-base.json; harness sources are in this directory for rerun.
Flakiness gate log
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.smoke.spec.ts
rounds=5 files=16 skipped=1
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/model-configuration.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/model-configuration.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/config/settingsSchema.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/settingsSchema.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/serve/workspace-providers-status.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/workspace-providers-status.test.ts
file packages/cli/src/utils/modelConfigUtils.test.ts: (cd packages/cli) npx --no-install vitest run ./src/utils/modelConfigUtils.test.ts
file packages/sdk-typescript/test/unit/DaemonClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClient.test.ts
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/ChatEditor.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatEditor.test.tsx
file packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/SettingsMessage.dom.test.tsx
file packages/web-shell/client/utils/sessionPreparation.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/sessionPreparation.test.ts
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
file packages/webui/src/daemon/session/actions.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/actions.test.ts
file packages/webui/src/daemon/session/mappers.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/mappers.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/acp-integration/acpAgent.test.ts: PP
packages/cli/src/acp-integration/model-configuration.test.ts: PP
packages/cli/src/acp-integration/session/Session.test.ts: P
packages/cli/src/config/settingsSchema.test.ts: P
packages/cli/src/serve/server.test.ts: P
packages/cli/src/serve/workspace-providers-status.test.ts: P
packages/cli/src/utils/modelConfigUtils.test.ts: P
packages/sdk-typescript/test/unit/DaemonClient.test.ts: P
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P
packages/web-shell/client/App.test.tsx: P
packages/web-shell/client/components/ChatEditor.test.tsx: P
packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx: P
packages/web-shell/client/utils/sessionPreparation.test.ts: P
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P
packages/webui/src/daemon/session/actions.test.ts: P
packages/webui/src/daemon/session/mappers.test.ts: P
verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/model-configuration.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/config/settingsSchema.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/workspace-providers-status.test.ts: P (exit 0)
round 1 · packages/cli/src/utils/modelConfigUtils.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/webui/src/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/model-configuration.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
Follow-up: Welcome reasoning preview regressionThe reported local regression was real: after selecting The follow-up commit fixes that state boundary without adding a clear-time provider refresh:
Real Chrome verification on the local daemon now passes the exact report path: Regression evidence on
This still uses only |
|
@qwen-code /verify |
|
@qwen-code /triage |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
|
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: 77 passed · 0 failed · 77 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：77 通过 · 0 失败 · 77 总计 抖动门： Verification reportPR #10489 — Deep Verification ReportVerdict: The central claim — WebShell model/reasoning preferences persist across daemon sessions via 中文摘要结论：
Central claim and A/B proofClaim under test: reasoning selection is persisted to Method: a mock-free wire harness (
Head: 42/42 pass. Base control: 35/35 pass (base asserts fewer because head-only confirmations are skipped). Witness images: Two control-run behaviors worth flagging because they sharpen the claim: on base, sending CorrectionsNone — no earlier review round or bot comment in this workspace misdescribed the code. FindingsNo blocking findings. Non-blocking observations:
Vacuity / mutation check (the new tests are not vacuous)The PR's central unit tests (
Both mutants die on the intended behavioral assertion (not an import/compile break), and the surviving sibling tests are consistent with each mutant's scope (e.g. the Targeted gates (all green)
Raw gate logs: Not covered
MethodologyEnvironment: the repo's own compiled output ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
…asoning-preferences # Conflicts: # packages/webui/src/daemon/session/DaemonSessionProvider.tsx
|
Local retest follow-up: reproduced the Welcome reasoning regression by holding a pre-mutation provider snapshot for 500 ms. The daemon had already confirmed medium with persisted=true, but the late provider response replaced the confirmed preview and the next task rebuilt Welcome as the old tier. The update now applies the existing config-generation guard to provider snapshots as well. The same probe failed before the fix and passes at 41beaa6; the 9-case reasoning Playwright matrix, 408 focused WebUI tests, 139 focused WebShell tests, 438 TypeScript SDK tests, package typechecks/builds, lint, and format all pass. The broader CLI run passed 2540/2541; its unrelated side-task timing case passed immediately when rerun alone. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
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.22.3)
…asoning-preferences Resolve sessionPreparation.ts import: take main's @qwen-code/web-shell/daemon-react-sdk specifier and keep this branch's ReasoningSelection type import.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Independent static review @ head 7ac6790 / 独立静态审查 @ head 7ac6790
Pure static review (no tests executed). Four module batches (ACP core / serve+schema+docs / SDK+session actions / web-shell UI) walked all 41 files at this head — a clean merge of origin/main whose substantive content is commits 14673c7, 6a9bf19, 4885a6b. I independently re-verified every one of the 25 findings from the latest /review round against the head tree, and also re-tested the earlier triage/verify "path is clean" conclusions on the exact disputed code paths. This message reports my own verdicts with evidence; it does not re-argue points where I simply agree with the inline threads.
纯静态审查（未运行测试）。4 个模块批次（ACP 核心 / serve+schema+docs / SDK+会话动作层 / web-shell UI）走查全部 41 个文件。head 是干净的 origin/main 合并，实质内容为三个提交 14673c7、6a9bf199、4885a6b0。我对最新一轮 /review 的 25 条 findings 逐条在 head tree 中独立复验，并对更早 triage/verify 轮"路径干净"的结论在争议代码路径上重新核对。本文只报告带证据的独立裁决。
What holds up / 站得住的部分
The boundaries are genuinely strict: the HTTP route derives _meta only from a validated boolean persist and never forwards client-supplied _meta (server.test pins all three properties); ReasoningSelection is a closed literal union across daemon/SDK/webui; the UI updates only after daemon confirmation with token/generation/model re-checks; fallbackEffort is gone tree-wide with no residue; the design doc matches the implemented contract point-for-point (default never stored, clear-don't-downgrade, no restore of dormant effort). The self-found Welcome preview regression fixes (6a9bf19, 4885a6b) are minimal and correctly guarded.
边界确实严格：HTTP 路由只从已校验的布尔 persist 派生 _meta、从不透传客户端 _meta（server.test 三个用例钉住）；ReasoningSelection 在 daemon/SDK/webui 全链路是封闭字面量联合；界面只在 daemon 确认后更新并有 token/代际/模型复核；fallbackEffort 全树删净无残留；设计文档与实现契约逐条一致（default 不落盘、不兼容即清不降级、不恢复旧档位）。两次自发现的 Welcome 预览回归修复（6a9bf199、4885a6b0）最小化且防护正确。
Independently confirmed Criticals / 独立复验成立的 Critical
All 11 Critical findings from the /review round were re-verified line-by-line in the head tree and still hold. The earlier triage/verify conclusions on these same paths do not survive contact with the code (on each disputed path — reconcile ordering, none/default cleanup, refreshAuth interaction — the failure chain below walks through). Locations as they read at this head:
最新一轮 /review 的全部 11 条 Critical 均在 head tree 中逐行复验成立。更早的 triage/verify 轮对这些路径的"干净"结论与代码不符（每条争议路径——reconcile 顺序、none/default 清理、refreshAuth 交互——下述失败链都能走通）。位置以当前 head 为准：
- R1-1
acpAgent.ts:5763— on the generic pathapplyReasoningSelectionmutates the live generation config beforesetReasoningEffortruns and before the confirm readback; whengeneration.reasoning === false,{...(generation.reasoning || {}), effort}silently re-enables thinking (false || {} → {}). The merge base rejected the same request.
通用路径先变异后回读：generation.reasoning === false时{...(false || {}), effort}静默重启用思考。合并基线对同一请求是拒绝。 - R1-2
config.ts:4109-4147—refreshAuthrestores only truthy prior reasoning (priorReasoning.effort), so the persistednoneseed (reasoning = false) this PR introduces is wiped on every auth refresh and never restored; tier values mask the gap,falsedoes not. Reconcile has exactly one call site (Session.setModel) — nothing re-seeds at session start.
refreshAuth只恢复 truthy 的旧值，本 PR 新引入的none种子（reasoning=false）每次鉴权刷新即被抹掉且不恢复；档位值掩盖缺口，false不能。reconcile 唯一调用点是 setModel，会话启动无重播种。 - R1-3
acpAgent.ts:5743-5785— settings write happens before live apply + confirm;defaultskips the static-knob cleanup (gate at 5747-5752), so on a model withextra_body: {enable_thinking: false}the confirm fails — but the persisted preference was already permanently cleared before the throw.
设置写盘先于应用+确认；default跳过静态旋钮清理，带enable_thinking: false静态旋钮的模型上确认必失败，但抛错前持久化值已被永久清除。 - R1-4 same gate — with a static knob configured, a "Default" reset can never complete its confirmation; pre-PR the cleanup ran for every selection on non-toggleOnly models.
同一门控：有静态旋钮时 "Default" 复位永远无法通过确认；PR 前该清理对所有选择执行。 - R1-5
model-configuration.ts:160-174— thedefaultbranch blank-wipes the wholereasoningobject on both live and rebuildable config, destroying the siblingbudget_tokensescape hatch;Config.setReasoningEffort(config.ts:5084-5097) deliberately deletes onlyeffortand preserves siblings. The wipe on the rebuildable config also survivesrefreshAuth.
default分支整体擦除reasoning对象（live+rebuildable），摧毁兄弟字段budget_tokens；与setReasoningEffort只删 effort 保兄弟的文档化不变量背离，且 rebuildable 上的擦除跨 refreshAuth 存续。 - R1-6
Session.ts:9827-9845— reconcile runs after the irreversibleswitchModel; the mustClear path writes withthrowOnWriteFailure: true, so a settings-write failure rejectssetModelfor a switch that already took effect, skips the subsequent model.name/baseUrl persistence (Session.ts:9889-9908, which is best-effort elsewhere in the same function), and re-throws on every later switch to an incompatible model until the disk is fixed.
reconcile 在不可逆switchModel之后；mustClear 用throwOnWriteFailure写盘，写盘失败会使已生效的切换报错、跳过后续 model.name 持久化，且每次切不兼容模型重复抛错直到磁盘修复。 - R1-7
App.tsx:11460-11489— the no-session branch derives a source selection from the model'sreasoningPrevieweven when the user never touched a reasoning control, and fabricates{modelId, value: 'default'}for unsupported targets;defaultpasses every support gate and reachespersistReasoningSelection('default')which deletes the stored value from all writable scopes. Concrete walk: persistedxhigh→ on Welcome with zero reasoning interaction click a toggle-only model and back → first prompt sendssetReasoningEffort('default', {persist: true})→ persisted setting silently deleted. The new e2ediscards an incompatible welcome effort when switching away and backpins this destructive behavior as expected.
无会话分支即使用户未碰推理控件也从预览推导源选择，并对不支持的目标伪造{modelId,'default'}；default通过一切门控到达persistReasoningSelection('default')并删除所有可写 scope 的已存值。具体路径：持久化xhigh→ Welcome 零交互点仅开关模型再点回 → 首条提示发出('default', {persist:true})→ 持久化设置被静默删除。新 e2e 用例把这一破坏性行为钉成预期。 - R1-8
Session.ts:9935-9995—persistReasoningSelectiondeletes the key from all sibling writable scopes (each a durable disk commit viathrowOnWriteFailure) before writing the target scope; on target write failure the catch reloads only the target scope — the siblings' preferences are permanently destroyed.LoadedSettings.setValues(settings.ts:566-603) already implements correct multi-scope rollback but is not reused.
persist 先删所有兄弟 scope（逐个落盘即永久提交）再写目标；目标写失败时 catch 只回滚目标 scope，兄弟偏好永久丢失。setValues已有正确的多 scope 回滚但未被复用。 - R1-9
App.tsx:6747-6756+11480-11486— the none→defaultconversion is a one-way latch in both the effect andhandleModelSelect; no transition ever restoresnonewhen disabling becomes supported again. Baseline (setPendingReasoningIntent(undefined)in the same effect) sent nothing for this sequence — this is a regression. The rewritten unit testclears a stale welcome disable after reasoning becomes mandatorypins the latch.
none→default单向闩锁（effect 与handleModelSelect两处），禁用重新受支持后也不恢复。合并基线该序列什么都不发——是回归。改写的单测把闩锁钉成预期。 - R1-10
App.tsx:11463-11464— the welcome branch ofhandleModelSelectis the only read site ofpendingReasoningIntentwithout themodelId === currentModelgate (the other three reads at 6222, 6749, 6764 all gate). A stale intent left by the/modelno-session branch revives on the next model click and is applied — or, combined with R1-7, converted to a destructivedefault— at first-prompt time with{persist: true}. Baseline applied nothing here; regression, no timing dependency.
welcome 分支是唯一不做modelId === currentModel门控的意图读取点（其余三处 6222/6749/6764 均有门控）。/model无会话分支留下的孤儿意图在下次点模型时复活并被应用，或与 R1-7 叠加转成破坏性default。基线此处不应用任何东西；回归，无时序依赖。 - R1-11
actions.ts:1707-1712—clearSessionawaits the pending persisted mutation before capturingsessionRef.currentand settingmanualSessionClearRef.current = true(both synchronous at entry on the merge base). The await window is up toDEFAULT_ACTION_TIMEOUT_MS(30s); a session create/attach landing inside it sets the flag false at entry (createSession:1492), passes its own abort check, and commitssessionRef.current = nextSession— clearSession then detaches the freshly created session, and the designed AbortError interruption path is bypassed. Fix direction: keep the await, move the capture + flag-set before it.
clearSession在捕获会话与置清除标志之前 await 挂起的持久化动作（基线这两步同步）。窗口最长 30s；窗口内落地的 create 会置标志为 false、通过自身检查并提交新会话，随后被恢复的 clearSession 剥离 detach，设计内的中断路径被绕过。修复方向：保留 await，把捕获与置标志前移。
Two test files pin the R1-7/R1-9 defective behaviors and must be rewritten together with the fixes, otherwise they will block correct repairs: web-shell.smoke.spec.ts (the discards-an-incompatible-welcome-effort case) and App.test.tsx (clears-a-stale-welcome-disable).
两个测试文件把 R1-7/R1-9 的缺陷行为钉成预期，修复时必须同步改写，否则会阻挡正确修复。
Three root-cause patterns cover most of the above: (a) write-first/mutate-first, confirm-later non-atomic ordering (3/6/8); (b) the new reasoning === false state is not accepted by the existing "restore only truthy" machinery (1/2); (c) default semantics interact badly with static knobs and sibling fields (3/4/5).
三类根因覆盖大部分问题：(a) 先写盘/先变异、后确认的非原子顺序（3/6/8）；(b) 新状态 reasoning === false 未被既有"仅恢复 truthy"机制接纳（1/2）；(c) default 与静态旋钮/兄弟字段的交互未清理（3/4/5）。
New finding (this review) / 本次新发现
🟡 Important — out-of-catalog Qwen-family models lose their entire ACP reasoning surface. buildConfigOptions gains an early return (acpAgent.ts:13156-13165) when !isReasoningSelectionSupported(rawCurrentModelId, 'default'), and that function returns false for every selection — including default — whenever the model has no MODEL_CONFIGURATIONS entry but matches the qwen family prefix (model-configuration.ts:136-138). So qwen-plus / dated aliases / coder variants that previously went through the generic path (effort applied via applyReasoningEffort, set_config_option accepted) now get no reasoning option at all and a "not supported" rejection. The PR description states this hiding is deliberate for unknown Qwen models — if so, please confirm the product consequence for existing ACP users of those model ids and pin the rejection with a test; nothing in the diff currently does. (Same shape as the deferred-model decision in the predecessor PR; still worth an explicit confirmation because it shrinks a previously working surface.)
🟡 Important — 表外 Qwen 系模型整体失去 ACP 推理控制面。 buildConfigOptions 新增早退（acpAgent.ts:13156-13165）：!isReasoningSelectionSupported(rawCurrentModelId, 'default') 时直接返回；而该函数对无 MODEL_CONFIGURATIONS 条目但匹配 qwen 系前缀的模型（model-configuration.ts:136-138）对包括 default 在内的一切选择返回 false。于是 qwen-plus / 带日期别名 / coder 变体等此前走通用路径（档位经 applyReasoningEffort 生效、set_config_option 被接受）的模型，现在完全没有推理选项并得到 "not supported" 拒绝。PR 描述称对未知 Qwen 模型隐藏控件是有意为之——若是，请确认这些模型 id 的既有 ACP 用户的产品后果，并用测试钉住该拒绝；diff 中目前没有任何测试覆盖。（与前序 PR 的 deferred-model 决定同形；因它收缩了此前可用的表面，仍值得显式确认。）
Confirmed suggestions (compact) / 成立的建议（摘要）
All 15 suggestion-level findings from the /review round were re-verified and hold, with two refinements: R1-13's stale object is the concurrently-running runtime sub-agent generator config, not the primary (daemon RPC handlers execute outside the ALS); R1-20's premise needs correction — the /model branch's drop of pending intent pre-existed this PR, what is new is the menu-path-vs-slash-path inconsistency (design doc line "Every model switch reconciles the stored selection") and the fact that /model orphans can now feed the R1-10 revival. Additionally R1-21 (two interpreters) upgrades in practice: a hand-edited non-canonical value (e.g. "High") is applied by the lenient CLI path but the strict parseReasoningSelection makes the first reconcile treat it as mustClear — silent permanent deletion of a value another surface considers valid. Product writers only ever write canonical values, so trigger requires hand-edited settings; still worth normalizing at read or at load.
/review 轮的 15 条 Suggestion 级 findings 全部复验成立，两点精化：R1-13 真正滞留的是并发运行中的运行时子代理 generator 配置而非 primary（daemon RPC handler 在 ALS 外执行）；R1-20 前提需更正——/model 分支丢弃意图早于本 PR，新增的是菜单路径与斜杠路径的不一致（设计文档声称"每次模型切换都协调已存选择"）以及 /model 孤儿意图现在会喂养 R1-10 的复活。另 R1-21（双解释器）实际后果升级：手编非规范值（如 "High"）被宽容 CLI 路径应用，但严格 parseReasoningSelection 使首次 reconcile 判其 mustClear——被另一读取面认为合法的值遭静默永久删除。产品内写入方只写规范值，触发需手改 settings；仍建议读取侧或加载侧归一化。
CI status at head / head 上的 CI 状态
Facts, not verdict: at this head the primary Qwen Code CI run (Test ubuntu, web-shell E2E Smoke, Serve A/B) was cancelled mid-run — there is no green primary-suite result for it. The SDK Java lane failed on Java 11 only (DaemonSessionClientTest transport timeouts, 15/103; Java 17/21 green); this PR contains no Java changes and the same lane was green on the previous head, so that failure looks environmental, but CI must land green before the merge queue regardless.
事实陈述而非裁决：当前 head 的主 CI 运行（Test ubuntu、web-shell E2E Smoke、Serve A/B）中途被取消——没有绿色主套件结果。SDK Java 仅 Java 11 失败（DaemonSessionClientTest 传输超时，15/103；Java 17/21 绿）；本 PR 无任何 Java 改动且同一 lane 在上一个 head 是绿的，该失败更像环境问题，但无论如何入合并队列前 CI 需全绿。
Verdict / 结论
💬 Comment — changes needed. The contract and typing work are solid, but 11 Criticals are independently confirmed at this head (several are regressions against the merge base with concrete data-loss walks), plus one new Important. I cannot approve until these are addressed; the fix directions above are consistent with the inline threads.
💬 评论——需要修改。 契约与类型工作是扎实的，但当前 head 上 11 条 Critical 经独立复验全部成立（其中数条是对合并基线的回归，有具体的数据丢失路径），另加一条新 Important。在这些解决之前无法批准；上述修复方向与行内讨论一致。
|
Review follow-up at b5f5359 Accepted and fixed:
Intentional scope decisions:
Validation:
|
|
@qwen-code /review |
|
CI follow-up at
|
|
@qwen-code /review |
…asoning-preferences
|
Baseline follow-up at #10526 has merged, so I normally merged the latest
No additional feature logic was added. |
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is running in workflow run. A command-triggered review is not listed under the checks of this PR; the result is posted here as a review when it finishes. |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
14 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-13 applyReasoningSelection context-dependent config target — already reported (round 1 inline comment), author deferred
- R1-14 strict === 'none' vs normalized tiers — already reported, author deferred
- R1-16 persist gate negative test absent — already reported, author deferred
- R1-17 route/runtime allowReasoning:false clear untested — already reported, author deferred (re-discovered independently this round)
- R1-18 none-seed thinkingMandatory guard untested — already reported, author deferred (re-discovered three times independently)
- R1-19 SDK positional clientId overload untested — already reported, author deferred (re-discovered)
- R1-20 /model slash path bypasses intent reconciliation — already reported, author deferred (re-discovered; the PR's own design doc says 'Every model switch reconciles the stored selection')
- R1-21 two disagreeing interpreters of model.reasoningEffort — already reported, author deferred (alias-deletion escalation re-discovered by this round's audit and the independent review)
- R1-22 smoke test rewrite dropped cross-model coverage — already reported, author deferred
- R1-23 non-persist success test converted to persist:true — already reported, author deferred
- R1-24 preview merged into target model provider entry only — already reported, author deferred
- R1-25 pendingPersistedReasoningAction unconditional overwrite — already reported, author deferred
- R1-26 unhonored 'none' silently dropped without warning — already reported, author deferred (re-discovered)
- R1-27 no-refetch guard lacks settling — already reported, author deferred
Not reviewed: build-and-test — no test suite could be executed in this review: the build-test run exhausted its budget on install + dependency builds (packages/cli build timed out — infrastructure; web-shell, vscode-ide-companion, webui never built), and the test-efficacy probe was 18/18 inconclusive (harnessValidated null).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Test (ubuntu-latest, Node 22.x) failed at this commit.
Not explored to full depth (tool budget reached): "agent 1b": none — though I did not trace where resolveCliGenerationConfig 's output is stored in ModelsConfig at daemon boot, so I could not rule in/out that a persiste…; chunk 4: executing Session.test.ts reasoning tests in the worktree (no node_modules present; dependency install plus prerequisite builds would exceed the tool budget) — ….
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.ts:9925 — [review] managed-session persist guard untested (D2-1)packages/cli/src/acp-integration/acpAgent.ts:7042 — [review] acpAgent-side persisted projection untested (D2-2)packages/web-shell/client/e2e/web-shell.smoke.spec.ts:586 — [review] no-refetch guard counts an endpoint the client never fetches (D2-3)packages/web-shell/client/e2e/web-shell.smoke.spec.ts:554 — [review] turn-complete event promptId can never match — silent no-op (D2-4)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 14 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — no test suite could be executed in this review: the build-test run exhausted its budget on install + dependency builds (packages/cli build timed out — infrastructure; web-shell, vscode-ide-companion, webui never built), and the test-efficacy probe was 18/18 inconclusive (harnessValidated null)。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Test (ubuntu-latest, Node 22.x) failed at this commit。
未探索到全部深度（达到工具调用预算）："agent 1b"：none — though I did not trace where resolveCliGenerationConfig 's output is stored in ModelsConfig at daemon boot, so I could not rule in/out that a persiste…；chunk 4：executing Session.test.ts reasoning tests in the worktree (no node_modules present; dependency install plus prerequisite builds would exceed the tool budget) — …。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| 'The persisted reasoning preference. Use none to disable thinking, a supported effort tier to enable it, or leave it unset to use the model/provider default.', | ||
| showInDialog: true, | ||
| options: [ | ||
| { value: 'none', label: 'None' }, |
There was a problem hiding this comment.
[Critical] R2-1: [certifies-falsely] [new-surface] This diff adds none to the dialog-visible (showInDialog: true) model.reasoningEffort options and rewrites the description to tell users "Use none to disable thinking" — but the TUI settings dialog is the one surface where none is never honored. A TUI user who follows the new description and picks None saves model.reasoningEffort: 'none'; every subsequent TUI/headless session seeds reasoning: false in resolveCliGenerationConfig, then performInitialAuth runs the plain core Config.refreshAuth, whose syncAfterAuthRefresh overwrites every MODEL_GENERATION_CONFIG_FIELDS entry (including reasoning) and whose restore block re-applies only a TRUTHY prior effort — false is excluded (the code comment says so). Only the daemon's refreshAuthWithPersistedReasoning replays a persisted none; no TUI/headless path uses it. Net effect: thinking stays ON (thinking tokens billed) in every session while the setting still says none. Pre-PR the dialog offered no none option, so this trap was unreachable — the PR creates the surface the pre-existing wipe defeats.
Witness:
probe (packages/core scratch tree, seeded generationConfig.reasoning=false, refreshAuth as performInitialAuth does):
PR: FAIL AssertionError: expected undefined to be false <- seeded disable dropped by the initial auth refresh
FIXED: 1 passed <- with a restore branch for priorReasoning === false; probe flips
Until TUI/headless flows honor none, stop advertising it in the TUI dialog: set showInDialog: false for model.reasoningEffort (Web Shell already hides this key via SettingsMessage), or exclude none from the dialog options while keeping it in the schema. Note that enum writes are validated against def.options at packages/cli/src/config/settingsUtils.ts:235 — removing none from the schema options outright would reject API-driven none writes, so hide it from the dialog instead; settingsSchema.test.ts 'accepts none but never stores the default reasoning command' asserts the options list contains none. Please add a settingsSchema/settingsUtils test asserting the TUI settings dialog does not offer none for model.reasoningEffort, then remove that protection and confirm the new test reds.
中文说明
本 diff 把 none 加入了对话框可见（showInDialog: true）的 model.reasoningEffort 选项，并把描述改写为告诉用户 "Use none to disable thinking"——但 TUI 设置对话框恰恰是 none 从不生效的表面。TUI 用户按新描述选择 None 后会保存 model.reasoningEffort: 'none'；此后每个 TUI/headless 会话先经 resolveCliGenerationConfig 播种 reasoning: false，随后 performInitialAuth 直接调用 core 的 Config.refreshAuth，其 syncAfterAuthRefresh 会用预设覆盖所有 MODEL_GENERATION_CONFIG_FIELDS 条目（含 reasoning），而恢复块只恢复 truthy 的旧 effort——false 被排除（代码注释明确说明）。只有 daemon 的 refreshAuthWithPersistedReasoning 会重放持久化的 none；TUI/headless 路径均不使用。结果：设置仍是 none，但每个会话思考都保持开启（计费思考 token）。PR 前对话框没有 none 选项，该陷阱不可达——是本 PR 创造了这个被既有刷新机制击败的表面。
见证（probe，packages/core 临时树，播种 generationConfig.reasoning=false，按 performInitialAuth 方式调用 refreshAuth）：PR 原样 FAIL（AssertionError: expected undefined to be false，初始鉴权刷新丢弃了播种的禁用）；加上 priorReasoning === false 恢复分支后 1 passed，探针翻转。
在 TUI/headless 支持 none 之前，请停止在 TUI 对话框中提供它：将 model.reasoningEffort 设为 showInDialog: false（Web Shell 已通过 SettingsMessage 隐藏该键），或保留 schema 选项但在对话框中排除 none。注意枚举写入在 packages/cli/src/config/settingsUtils.ts:235 按 def.options 校验——直接从 schema 删除 none 会拒绝 API 驱动的 none 写入，因此应只在对话框层面隐藏；settingsSchema.test.ts 的 'accepts none but never stores the default reasoning command' 断言选项列表包含 none。请新增 settingsSchema/settingsUtils 测试，断言 TUI 设置对话框不为 model.reasoningEffort 提供 none，然后移除该保护并确认新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await this.refreshAuthWithPersistedReasoning( | ||
| config, | ||
| this.settings, | ||
| authType, | ||
| ); |
There was a problem hiding this comment.
[Critical] R2-2: [certifies-falsely] [new-surface] workspaceReload's model-name-change branch (await config.switchModel(authType, newModelName), ~12194) wipes the session's live reasoning: false and never re-applies the persisted model.reasoningEffort: 'none' — unlike this wrapped sibling branch here, and unlike Session.setModel, whose post-switch reconcileReasoningSelection re-applies a compatible persisted selection after the same core switchModel call. applyResolvedModelDefaults overwrites every MODEL_GENERATION_CONFIG_FIELDS entry (reasoning included); on the full-refresh path the restore re-applies only truthy effort. Concretely: a user persists none (settings written, live reasoning = false); model.name changes in settings.json and a workspace reload runs — the handler takes the model-change branch, reasoning is wiped, and the session silently runs with thinking ON while settings still say none, until some unrelated replay fires. The identical switch via setSessionConfigOption reconciles correctly. At merge base none was not a storable value, so this state could not be lost.
Witness:
probe (real ModelsConfig, scratch tree):
PR: seeded reasoning=false; switch to a preset-less model -> reasoning=undefined
CTRL: switch to a model whose preset carries reasoning:false -> reasoning=false (preserved)
Reuse the replay half of refreshAuthWithPersistedReasoning (persisted-none check + support gate + applyReasoningSelection(config, REASONING_EFFORT_NONE)) after await config.switchModel(authType, newModelName) succeeds in that branch. Keep the wrapper's support gate isReasoningSelectionSupported(modelId, REASONING_EFFORT_NONE, generation?.thinkingMandatory === true) (model-configuration.ts:130) so none is never replayed onto a thinking-mandatory model. Please extend the acpAgent.test.ts workspaceReload suite: merged settings { model: { name: 'next-model', reasoningEffort: 'none' } }, live reasoning: false; after workspaceReload assert the session's generation config still has reasoning === false on the new model — then remove the post-switchModel replay and confirm the new test reds.
中文说明
workspaceReload 的模型名变更分支（await config.switchModel(authType, newModelName)，约 12194 行）会抹掉会话 live 的 reasoning: false，且从不重新应用持久化的 model.reasoningEffort: 'none'——不同于此处被包装的兄弟分支，也不同于 Session.setModel（其在同一个 core switchModel 调用之后通过 reconcileReasoningSelection 重新应用兼容的持久化选择）。applyResolvedModelDefaults 会覆盖所有 MODEL_GENERATION_CONFIG_FIELDS 条目（含 reasoning）；全量刷新路径上的恢复只恢复 truthy effort。具体场景：用户持久化 none（设置已写盘，live reasoning = false）；settings.json 中 model.name 变化触发 workspace reload——handler 走模型变更分支，reasoning 被抹掉，设置在仍为 none 的情况下会话静默以思考开启运行，直到某个无关的重放触发。同样的切换经 setSessionConfigOption 则会正确 reconcile。合并基线上 none 不可持久化，因此该状态不可能丢失。
见证（probe，真实 ModelsConfig，临时树）：播种 reasoning=false 后切换到无预设模型 → reasoning=undefined；对照组切换到预设携带 reasoning:false 的模型 → reasoning=false（保留）。
请在该分支的 await config.switchModel(authType, newModelName) 成功后，复用 refreshAuthWithPersistedReasoning 的重放半边（持久化 none 检查 + 支持门控 + applyReasoningSelection(config, REASONING_EFFORT_NONE)）。保留包装器的支持门控 isReasoningSelectionSupported(modelId, REASONING_EFFORT_NONE, generation?.thinkingMandatory === true)（model-configuration.ts:130），避免把 none 重放到强制思考模型上。请扩展 acpAgent.test.ts 的 workspaceReload 套件：合并设置 { model: { name: 'next-model', reasoningEffort: 'none' } }、live reasoning: false；workspaceReload 后断言新模型上会话的 generation config 仍为 reasoning === false——然后移除 switchModel 后的重放并确认新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (selection === REASONING_EFFORT_NONE) { | ||
| generation.reasoning = false; | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Critical] R2-3: [certifies-falsely] [new-surface] The 'none' branch assigns generation.reasoning = false, destroying sibling fields — user-configured budget_tokens, the Anthropic escape hatch, which is settings-reachable because reasoning is a MODEL_GENERATION_CONFIG_FIELDS member — on both the live config and the rebuildable ModelsConfig._generationConfig. No later selection restores them: 'default' hits !generation.reasoning → undefined, tier branches spread false || {}, and mergeSettingsGenerationConfig cannot repair because the reasoning key was assigned, not deleted (its guard is !(field in this._generationConfig), modelsConfig.ts:645-648). This defeats the sibling-preservation guarantee your R1-5 fix pins in 'preserves reasoning siblings when returning to the model default'. A user with model.generationConfig.reasoning: { budget_tokens: N } who gets 'none' applied (Web Shell toggle on a generic model, reconcileReasoningSelection, or the auth-refresh replay) loses budget_tokens permanently: turning thinking back on never restores it, settings still declare budget_tokens: N, and every request runs effort-derived thinking for the daemon's lifetime. The seed site has the same defect: resolveCliGenerationConfig (modelConfigUtils.ts:445) assigns reasoning = false at daemon session construction, discarding settings-derived siblings before this function ever runs.
Witness:
probe (real applyReasoningSelection, scratch tree):
baseline-default live={"reasoning":{"budget_tokens":42000}} <- control arm preserves
after-none live={"reasoning":false} -> after-default live={} -> after-high live={"reasoning":{"effort":"high"}}
<- budget_tokens gone permanently
Preserve siblings across the false transition: capture the prior reasoning object before overwriting with false (e.g. a WeakMap keyed by the generation config) and re-spread the stashed siblings (minus effort for 'default') when leaving false — or re-merge settings-derived reasoning siblings on leaving false; cover the seed site in resolveCliGenerationConfig too, since a stash inside this function has nothing to stash there. The reasoning === false wire signal must survive: core Config.setReasoningEffort (if (!cfg || cfg.reasoning === false) return;, config.ts:5091) and anthropicContentGenerator.ts both key off it, and restoring siblings on 'default' must not resurrect a stale effort. Please extend 'preserves reasoning siblings when returning to the model default' into a none→default cycle (start live/rebuildable at { effort: 'high', budget_tokens: 42000 }, apply 'none' then 'default', assert { budget_tokens: 42000 } — it reds today), and confirm it still reds when the preservation is removed.
中文说明
'none' 分支直接赋值 generation.reasoning = false，会摧毁同级字段——用户配置的 budget_tokens（Anthropic 逃生舱；reasoning 是 MODEL_GENERATION_CONFIG_FIELDS 成员，可经设置写入）——且同时作用于 live 与可重建的 ModelsConfig._generationConfig。之后任何选择都无法恢复它们：'default' 命中 !generation.reasoning → undefined，档位分支展开 false || {}，而 mergeSettingsGenerationConfig 因 reasoning 键是被赋值而非删除（其守卫为 !(field in this._generationConfig)，modelsConfig.ts:645-648）无法修复。这击败了 R1-5 修复在 'preserves reasoning siblings when returning to the model default' 中钉住的同级保留保证。配置了 model.generationConfig.reasoning: { budget_tokens: N } 的用户一旦被应用 'none'（通用模型上的 Web Shell 开关、reconcileReasoningSelection 或鉴权刷新重放），budget_tokens 永久丢失：重新开启思考不会恢复它，设置仍声明 budget_tokens: N，而整个 daemon 生命周期内每个请求都按 effort 推导思考。播种点存在同样缺陷：resolveCliGenerationConfig（modelConfigUtils.ts:445）在 daemon 会话构造时赋值 reasoning = false，在本函数运行前就丢弃了来自设置的同级字段。
见证（probe，真实 applyReasoningSelection，临时树）：基线 default 时 live={"reasoning":{"budget_tokens":42000}}（对照组保留）；应用 none 后 live={"reasoning":false} → default 后 live={} → high 后 live={"reasoning":{"effort":"high"}}——budget_tokens 永久消失。
请跨 false 转换保留同级字段：在用 false 覆盖前捕获原 reasoning 对象（如以 generation config 为键的 WeakMap），离开 false 时重新展开暂存的同级字段（'default' 时去掉 effort）——或在离开 false 时重新合并来自设置的 reasoning 同级字段；同时覆盖 resolveCliGenerationConfig 播种点，因为本函数内的暂存在那里无物可存。reasoning === false 线信号必须保留：core Config.setReasoningEffort（if (!cfg || cfg.reasoning === false) return;，config.ts:5091）与 anthropicContentGenerator.ts 都依赖它，且 'default' 恢复同级字段时不得复活陈旧的 effort。请把 'preserves reasoning siblings when returning to the model default' 扩展为 none→default 循环（起始 live/可重建为 { effort: 'high', budget_tokens: 42000 }，依次应用 'none' 与 'default'，断言 { budget_tokens: 42000 }——当前即为红色），并确认移除保留逻辑后该测试仍变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const session = sessionRef.current; | ||
| manualSessionClearRef.current = true; | ||
| if (pendingPersistedReasoningAction) { | ||
| await pendingPersistedReasoningAction.catch(() => undefined); | ||
| } |
There was a problem hiding this comment.
[Critical] R2-4: [certifies-falsely] [new-surface] After your R1-11 fix (session captured + manualSessionClearRef set before the await — good), clearSession still resumes into the unconditional global resets below (clearActiveSessionState(); sessionRef.current = undefined) without re-validating the captured session — and the flag blocks no creation/attach path: createSession resets manualSessionClearRef as its first act (actions.ts:1492), submitPrompt never checks it, and the UI stays interactive during the up-to-30s await (DEFAULT_ACTION_TIMEOUT_MS; the only composer gate, sessionWriteBlocked = Boolean(connection.loadingTranscript), is not set). A session committed inside that window is destroyed: clearActiveSessionState() aborts its in-flight prompt controller, the transcript store is reset, sessionRef.current = undefined orphans it, and getConnectionAfterSessionClear skips field-stripping (current.sessionId !== clearedSessionId) — leaving a connection claiming session-b with no client, and session-b never detached on the daemon.
Witness:
probe (real createDaemonSessionActions, scratch tree; persist pending, replacement session committed mid-await):
PR: after={"sessionRef":"undefined","connectionSessionId":"session-b","promptAborted":true,"storeResetCalls":1,"sessionADetachCalls":1,"sessionBDetachCalls":0}
FIXED: after={"sessionRef":"session-b","connectionSessionId":"session-b","promptAborted":false,"storeResetCalls":0,...} <- probe flips
After the await, skip the global resets when the session was replaced during the wait (sessionRef.current !== undefined && sessionRef.current !== session), while still completing the captured session's connection cleanup and detach. The test added by this diff, 'captures and marks a clear before waiting for persisted reasoning', pins that a mid-window swap still completes the captured session's cleanup (session.detach once, replacement not detached) — the guard must skip only the global resets, not early-return wholesale. Please add an actions.test.ts case that fully attaches a replacement during the unresolved persist and asserts sessionRef.current is still the replacement, connection.sessionId is still 'session-b', and no registered active prompt was aborted — then remove the guard and confirm it reds.
中文说明
在 R1-11 修复之后（await 之前捕获会话并置位 manualSessionClearRef——很好），clearSession 恢复后仍会无条件执行下方的全局重置（clearActiveSessionState(); sessionRef.current = undefined），没有复核捕获的会话是否仍然有效——而且该标志无法阻止任何创建/挂载路径：createSession 第一步就重置 manualSessionClearRef（actions.ts:1492），submitPrompt 从不检查它，且 UI 在最长 30 秒的 await 期间保持可交互（DEFAULT_ACTION_TIMEOUT_MS；唯一的输入框门控 sessionWriteBlocked = Boolean(connection.loadingTranscript) 未置位）。在该窗口内落地的会话会被摧毁：clearActiveSessionState() 中止其进行中的 prompt 控制器，转写存储被重置，sessionRef.current = undefined 使其成为孤儿，而 getConnectionAfterSessionClear 跳过字段剥离（current.sessionId !== clearedSessionId）——留下一个声称 session-b 却无客户端的连接，且 session-b 从未在 daemon 上 detach。
见证（probe，真实 createDaemonSessionActions，临时树；persist 挂起期间替换会话落地）：PR 原样 after={"sessionRef":"undefined","connectionSessionId":"session-b","promptAborted":true,"storeResetCalls":1,...}；修复后 after={"sessionRef":"session-b","connectionSessionId":"session-b","promptAborted":false,"storeResetCalls":0,...}，探针翻转。
请在 await 之后、当等待期间会话已被替换时（sessionRef.current !== undefined && sessionRef.current !== session）跳过全局重置，同时继续完成被捕获会话的连接清理与 detach。本 diff 新增的测试 'captures and marks a clear before waiting for persisted reasoning' 钉住了窗口内替换仍完成被捕获会话的清理（session.detach 一次、替换会话不 detach）——守卫只应跳过全局重置，而不是整体提前返回。请新增 actions.test.ts 用例：在未解析的 persist 期间完整挂载替换会话，断言 sessionRef.current 仍是替换会话、connection.sessionId 仍是 'session-b'、已注册的活跃 prompt 未被中止——然后移除守卫并确认变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (persist) { | ||
| reasoningSelectionPersisted = | ||
| session.persistReasoningSelection(selected); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-5: A persistence failure throws AFTER the live reasoning selection was applied and confirmed here — the request reports failure while the daemon session keeps the changed live state, and nothing rolls it back. Two reachable triggers at head: an unwritable settings file (read-only checkout, EROFS/ENOSPC — writeReasoningSelection uses throwOnWriteFailure: true) with persist: true, which Web Shell always sends; and a standalone-source session whose persistReasoningSelection deliberately throws RequestError.invalidParams. The client then shows "Failed to update reasoning options" and keeps its old connection.reasoning, while the daemon session actually runs with the new selection until the next context resync — error and reality disagree. Creation-path consequence: sessionPreparation.ts:146 hard-codes { persist: true }, so the same failure aborts the whole first prompt — the freshly created session is released and cleared, the prompt is never sent, and every retry fails identically in that environment; the mid-session toggle path, by contrast, keeps running. This ordering is fix-induced: the R1-3 fix moved persist after confirm, which created the apply-then-throw path.
Witness:
witness: not run — the ordering is unconditional in the quoted source (apply -> confirm -> persist, no try/catch between), and both throw paths were verified at their definitions (settings.ts saveSettings rethrows under throwOnWriteFailure; Session.ts managed-session invalidParams)
Wrap the persistence call so a failure rolls the live/rebuildable generation config back to its pre-request reasoning value before rethrowing (or persist before applying) — but do not swallow the failure: the design doc's "writes the setting and live state before returning success" must hold (no success on a failed write), and genuine apply failures must still abort the creation path (pinned by 'releases and clears the session when an explicit reasoning effort cannot be applied'). Note the client confirmation contract at actions.ts:1254 (if (!confirmed || (opts?.persist && result.persisted !== true))) must survive any fix. Please add an acpAgent.test.ts case where persistReasoningSelection throws with the persist meta set — assert the request rejects AND generation.reasoning is restored to its prior value; removing the rollback/reorder must turn it red.
中文说明
持久化失败会在 live 思考选择已应用并确认之后才抛出——请求报告失败，但 daemon 会话保持已变更的 live 状态，且没有任何回滚。当前 head 有两个可达触发：设置文件不可写（只读 checkout、EROFS/ENOSPC——writeReasoningSelection 使用 throwOnWriteFailure: true）且带 persist: true（Web Shell 总是发送）；以及 persistReasoningSelection 故意抛出 RequestError.invalidParams 的 standalone 来源会话。客户端随后显示 "Failed to update reasoning options" 并保留旧的 connection.reasoning，而 daemon 会话实际以新选择运行直到下次上下文重同步——错误与现实不一致。创建路径后果：sessionPreparation.ts:146 硬编码 { persist: true }，同样的失败会中止整个首条 prompt——新创建的会话被释放清除、prompt 从未发送，且在该环境下每次重试都以相同方式失败；而会话中的切换路径则继续运行。该顺序是修复引入的：R1-3 修复把持久化移到确认之后，从而产生了先应用后抛错的路径。
见证：未运行——顺序在引用源码中是无条件的（应用 → 确认 → 持久化，中间无 try/catch），两个抛出路径均已在其定义处核实（settings.ts 的 saveSettings 在 throwOnWriteFailure 下重新抛出；Session.ts 的 managed-session invalidParams）。
请包装持久化调用，使失败时先把 live/可重建 generation config 回滚到请求前的 reasoning 值再重新抛出（或先持久化后应用）——但不要吞掉失败：设计文档的 "在返回成功前写入设置与 live 状态" 必须保持（写盘失败不得返回成功），真正的应用失败仍须中止创建路径（由 'releases and clears the session when an explicit reasoning effort cannot be applied' 钉住）。注意 actions.ts:1254 的客户端确认契约（if (!confirmed || (opts?.persist && result.persisted !== true))）必须在任何修复后保持。请新增 acpAgent.test.ts 用例：设置 persist meta 且 persistReasoningSelection 抛错——断言请求被拒绝且 generation.reasoning 恢复为原值；移除回滚/调整顺序必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| !modelReasoning && | ||
| tierSelected && | ||
| generation.reasoning === false | ||
| ) { | ||
| config.setReasoningEffort?.(selected); | ||
| } else { |
There was a problem hiding this comment.
[Suggestion] R2-6: This R1-1 fix branch calls config.setReasoningEffort?.(selected), which is a documented no-op precisely when generation.reasoning === false (config.ts:5087-5092) — so this branch can never apply the tier; the rejection actually comes from the confirm check below, while the dead call reads as an attempt to apply. There is also a cross-class inconsistency: on non-MODEL_CONFIGURATIONS models a tier-after-off gesture errors ('Reasoning effort cannot be applied while thinking is disabled' — user must select Default first, then the tier), while on manifest models the identical gesture silently re-enables via applyReasoningSelection's {...(false || {}), effort} spread. The no-op invariant itself should stand — it is the R1-1 contract, pinned by 'does not re-enable an explicitly disabled generic model with a tier' — the defect is the dead call's opacity and the divergent gesture behavior. The Web Shell disables tier buttons while thinking is off, so the divergence is reachable only through the daemon/ACP API.
Witness:
probe (scratch tree, head): vitest -t "does not re-enable an explicitly disabled generic model with a tier" -> Tests 1 passed | 528 skipped
contradicting behavior: `if (!cfg || cfg.reasoning === false) return;` (config.ts:5091) vs `{...(generation.reasoning || {}), effort: selection}` in applyReasoningSelection
Replace the dead no-op call with an explicit throw so the intent is visible (keeping the rejection semantics), or delete the branch and let the confirm check alone produce the error — either way, pin the chosen behavior: add an acpAgent.test.ts case with a non-MODEL_CONFIGURATIONS model, reasoning === false, setSessionConfigOption({configId: 'reasoning_effort', value: 'high'}), asserting the explicit rejection and that generation.reasoning stays false. The fix must not rely on setReasoningEffort to re-enable thinking ("No-op when thinking is explicitly disabled so effort cannot silently re-enable it", config.ts:5084-5087); removing the chosen guard must turn the new test red.
中文说明
这个 R1-1 修复分支调用 config.setReasoningEffort?.(selected)，而它在 generation.reasoning === false 时恰好是文档化的空操作（config.ts:5087-5092）——因此该分支永远无法应用档位；拒绝实际来自下方的确认检查，而这个死调用读起来像是在尝试应用。还存在跨类别不一致：对非 MODEL_CONFIGURATIONS 模型，关闭后直接选档位会报错（'Reasoning effort cannot be applied while thinking is disabled'——用户必须先选 Default 再选档位），而对清单模型同样的手势会经 applyReasoningSelection 的 {...(false || {}), effort} 展开静默重新启用。空操作不变量本身应当保留——它是 R1-1 的契约，由 'does not re-enable an explicitly disabled generic model with a tier' 钉住——缺陷在于死调用的不透明和手势行为的不一致。Web Shell 在思考关闭时禁用档位按钮，因此该分歧只能经 daemon/ACP API 触达。
见证（probe，临时树，head）：vitest -t "does not re-enable an explicitly disabled generic model with a tier" → Tests 1 passed | 528 skipped；矛盾行为：if (!cfg || cfg.reasoning === false) return;（config.ts:5091）对比 applyReasoningSelection 中的 {...(generation.reasoning || {}), effort: selection}。
请把死空操作调用替换为显式 throw 使意图可见（保留拒绝语义），或删除该分支让确认检查单独产生错误——无论哪种，都请钉住所选行为：新增 acpAgent.test.ts 用例，非 MODEL_CONFIGURATIONS 模型、reasoning === false、setSessionConfigOption({configId: 'reasoning_effort', value: 'high'})，断言显式拒绝且 generation.reasoning 保持 false。修复不得依赖 setReasoningEffort 重新启用思考（"思考被显式禁用时空操作，防止档位静默重启用"，config.ts:5084-5087）；移除所选守卫必须使新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| switchModelSpy.mockImplementation( | ||
| async (authType: AuthType, modelId: string) => { | ||
| currentAuthType = authType; | ||
| currentModel = modelId; |
There was a problem hiding this comment.
[Suggestion] R2-7: The new installReasoningPreference helper mocks switchModel as updating only model/authType — but the real Config.switchModel wipes/rebuilds reasoning: applyResolvedModelDefaults overwrites every MODEL_GENERATION_CONFIG_FIELDS entry (reasoning included, constants.ts:34), and the full-refresh restore re-applies only truthy effort. The mock keeps live/rebuildable reasoning intact across the switch, so the setModel reconcile tests pass vacuously for the restore branch. Mutation at head proves it: deleting the final applyReasoningSelection(this.config, selection) (supported branch) from reconcileReasoningSelection leaves all 22 setModel tests green, because {effort:'low'} was never wiped by the mock. This same PR contains an unguarded instance of exactly that wipe (the workspaceReload branch, flagged separately), and no case covers a persisted 'none' surviving a compatible switch — the one case core's restore provably cannot cover. A restore regression ships green.
Witness:
probe (scratch tree, packages/cli vitest):
intact 'keeps a compatible persisted tier when switching models' -> 1 passed
mutant (supported-branch re-apply removed) -t setModel -> 22 passed | 0 failed
control (mustClear-branch apply removed instead) -> 6 failed | 16 passed
<- the harness detects reconcile mutations; the tests pin the clear path, not the restore path
Have the mocked switchModel reproduce the wipe (live.reasoning = undefined; rebuildable.reasoning = undefined; before setting the new model, mirroring applyResolvedModelDefaults for a preset-less model), and add a persisted-'none'-survives-compatible-switch case. Core's restore cannot help there: if (priorReasoningEffort) { (config.ts:5390) re-applies only truthy effort, and getReasoningEffort() returns undefined for reasoning: false (config.ts:5011) — restoring a persisted 'none' depends solely on the session-level reconcile, so the mock must not paper over that by carrying false across the switch. 'keeps a compatible persisted tier when switching models' must go red when the supported-branch apply is removed; the new 'none' case must go red when the restore is skipped.
中文说明
新的 installReasoningPreference 助手把 switchModel mock 为只更新 model/authType——但真实 Config.switchModel 会抹除/重建 reasoning：applyResolvedModelDefaults 覆盖所有 MODEL_GENERATION_CONFIG_FIELDS 条目（含 reasoning，constants.ts:34），全量刷新恢复只恢复 truthy effort。mock 在切换前后保持 live/可重建 reasoning 不变，因此 setModel reconcile 测试对恢复分支是空转通过。head 上的变异证明了这一点：从 reconcileReasoningSelection 删除最后的 applyReasoningSelection(this.config, selection)（支持分支）后全部 22 个 setModel 测试仍绿，因为 {effort:'low'} 从未被 mock 抹掉。同一个 PR 里就存在一处对该抹除毫无防护的实例（workspaceReload 分支，已另行指出），且没有任何用例覆盖持久化 'none' 在兼容切换后存活——而这恰是 core 恢复逻辑证明无法覆盖的情形。恢复分支回归会绿灯通过。
见证（probe，临时树，packages/cli vitest）：原样 'keeps a compatible persisted tier when switching models' → 1 passed；变异（移除支持分支重应用）-t setModel → 22 passed | 0 failed；对照（改移除 mustClear 分支应用）→ 6 failed | 16 passed——脚手架能检测 reconcile 变异；测试钉住的是清除路径而非恢复路径。
请让 mock 的 switchModel 复现抹除（设置新模型前 live.reasoning = undefined; rebuildable.reasoning = undefined;，模拟无预设模型的 applyResolvedModelDefaults），并新增持久化 'none' 兼容切换存活用例。core 恢复在此无能为力：if (priorReasoningEffort) {（config.ts:5390）只恢复 truthy effort，且 getReasoningEffort() 对 reasoning: false 返回 undefined（config.ts:5011）——持久化 'none' 的恢复完全依赖会话级 reconcile，mock 不得通过在切换间携带 false 来掩盖这一点。移除支持分支应用时 'keeps a compatible persisted tier when switching models' 必须变红；跳过恢复时新 'none' 用例必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| setPendingReasoningIntent( | ||
| sourceReasoningIntent && keepReasoningIntent | ||
| ? { modelId, value: sourceReasoningIntent.value } |
There was a problem hiding this comment.
[Suggestion] R2-8: The retarget branch of handleModelSelect's welcome path — an explicit reasoning intent bound to the current model that the target supports is carried over with its value — has zero test witness. The only welcome-mode onSelectModel test ('does not retarget a stale model-bound reasoning intent') routes its model switch through /model (the slash path at ~10204 bypasses handleModelSelect), so at its step 3 the intent is stale (modelId mismatch), sourceReasoningIntent is undefined, and this branch never executes in any test — the other onSelectModel sites run with live sessions and take the session branch. A mutation proves the branch is load-bearing: dropping it (or substituting 'default' for the carried value) leaves the entire App.test.tsx suite green while every picker-driven welcome model switch silently loses the explicit selection — user picks max on model A, picker-switches to model B supporting max, sends the first prompt, and Web Shell persists 'default' instead of max, wiping the stored preference and starting on the target's default.
Witness:
probe (scratch tree):
INTACT: reasoning.effort === 'max' carried with its value (Tests 2 passed)
MUTANT (branch removed): AssertionError: expected 'medium' to be 'max' <- explicit 'max' silently dropped
stale-intent test still passes; FULL SUITE 1 failed | 582 passed — the only failure is the probe
Add an App.test.tsx case: welcome mode (mockConnection.sessionId = undefined), onSelectReasoningEffort('max'), then onSelectModel to a target whose preview efforts include 'max'; assert reasoning.effort still displays 'max', then submit and assert mockSessionActions.setReasoningEffort was called with ('max', { persist: true }) — the design doc's Capability reconciliation says "a supported tier and an allowed none value are retained", and createAndAttachSessionForPrompt applies the carried intent with { persist: true } (sessionPreparation.ts), so the assertion must include the persist option. Removing the retarget branch or mutating it to 'default' must turn the new test red.
中文说明
handleModelSelect welcome 路径的重定向分支——绑定当前模型且目标支持的显式推理意图按原值携带——没有任何测试见证。唯一的 welcome 态 onSelectModel 测试（'does not retarget a stale model-bound reasoning intent'）经由 /model 路由切换模型（约 10204 行的斜杠路径绕过 handleModelSelect），因此其第 3 步时意图已过期（modelId 不匹配）、sourceReasoningIntent 为 undefined，该分支在任何测试中都未执行——其余 onSelectModel 调用点都在 live 会话下走会话分支。变异证明该分支承重：删除它（或把携带值替换为 'default'）时整个 App.test.tsx 套件仍绿，而每次选层器驱动的 welcome 模型切换都会静默丢失显式选择——用户在模型 A 选 max，选层器切到支持 max 的模型 B，发送首条 prompt，Web Shell 却持久化 'default' 而非 max，清除已存偏好并以目标默认启动。
见证（probe，临时树）：原样 reasoning.effort === 'max' 按原值携带（Tests 2 passed）；变异（移除分支）AssertionError: expected 'medium' to be 'max'——显式 'max' 被静默丢弃；陈旧意图测试仍通过；全套件 1 failed | 582 passed，唯一失败即探针。
请新增 App.test.tsx 用例：welcome 态（mockConnection.sessionId = undefined）、onSelectReasoningEffort('max')，然后 onSelectModel 到预览 efforts 包含 'max' 的目标；断言 reasoning.effort 仍显示 'max'，再提交并断言 mockSessionActions.setReasoningEffort 以 ('max', { persist: true }) 被调用——设计文档的能力协调写明 "支持的档位与允许的 none 值会被保留"，且 createAndAttachSessionForPrompt 以 { persist: true } 应用携带的意图（sessionPreparation.ts），断言必须包含 persist 选项。移除重定向分支或将其变异为 'default' 必须使新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
14 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 round-1 blocker residual (persisted 'none' wiped on every non-ACP refreshAuth; TUI dialog now offers None) — already reported at this head as R2-1 (comment 3889107196)
- R1-3 fix-induced residual (managed-session apply-then-throw) — already reported at this head as R2-5 (comment 3889107205), as a Suggestion; this run's verification graded the same mechanism Critical
- clearSession destroys a session installed during the wait — already reported at this head as R2-4 (comment 3889107203)
- workspaceReload model-change branch wipes a persisted none — already reported at this head as R2-2 (comment 3889107198)
- reconcile supported-branch apply has no test witness — already reported at this head as R2-7 (comment 3889107211)
- retarget branch of the welcome reconciliation untested — already reported at this head as R2-8 (comment 3889107214)
- untested route/runtime allowReasoning gate — already reported as R1-17 (comment 3887695492), author deferred
- untested thinkingMandatory none-guard — already reported as R1-18 (comment 3887695493), author deferred
- untested positional clientId overload — already reported as R1-19 (comment 3887695496), author deferred
- two disagreeing persisted-value parsers — already reported as R1-21 (comment 3887695501), author deferred
- strict === 'none' case sensitivity — already reported as R1-14 (comment 3887695485), author deferred
- silent none-skip on thinking-mandatory models — already reported as R1-26 (comment 3887695515), author deferred
- cross-model non-application e2e coverage removed — already reported as R1-22 (comment 3887695502), author deferred
- none→default conversion at the mandatory-thinking boundary — already discussed in R1-9 (comment 3887695470): author maintains it as the shipped clear-don't-restore contract; the rewritten test pins it
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 11: exhaustive read of App.tsx / DaemonSessionProvider.tsx for a session-attach-triggered GET /workspace/providers reload (the uncertainty behind the low-confiden….
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.ts:9925 — [review] managed-conversation guards untested (probe: removing both guards keeps Session.test.ts at 747/747 green) — deferred, code-age rulepackages/cli/src/acp-integration/acpAgent.ts:7047 — [review] buildWorkspaceProvidersStatus persisted-reasoning projection untested (probe: hunk reverted, 6/6 stay green) — deferred, code-age rulepackages/cli/src/acp-integration/model-configuration.ts:54 — [review] isQwenFamilyModel is a verbatim duplicate of core's isQwenFamilyWireModel (second source of truth for the family set) — deferred, code-age rulepackages/web-shell/client/e2e/web-shell.smoke.spec.ts:583 — [review] New-task no-refetch guard counts a route this flow never issues (structurally 0 === 0, probe: 8 GETs all on the global route) — deferred, code-age rulepackages/cli/src/acp-integration/acpAgent.ts:12207 — [review] workspaceReload never propagates a reasoningEffort-only settings edit to live sessions (UI and wire disagree until restart) — deferred, code-age rule
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 14 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 11：exhaustive read of App.tsx / DaemonSessionProvider.tsx for a session-attach-triggered GET /workspace/providers reload (the uncertainty behind the low-confiden…。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| setPendingReasoningIntent( | ||
| sourceReasoningIntent && keepReasoningIntent | ||
| ? { modelId, value: sourceReasoningIntent.value } | ||
| : sourceReasoningSelection && !keepReasoningIntent | ||
| ? { modelId, value: 'default' } | ||
| : undefined, | ||
| ); |
There was a problem hiding this comment.
[Critical] R1-7: [certifies-falsely] [new-surface] Round-1 mechanism re-checked at this head and still present: this no-session branch synthesizes a 'default' reasoning intent from a PREVIEW-DERIVED selection even when the user never touched a reasoning control — sourceReasoningSelection falls back to getReasoningSelection(sourceReasoningPreview). 'default' passes ensureSessionForPrompt's gates unconditionally (reasoningPreviewSupports returns true for 'default', App.tsx:1285), the first prompt sends setReasoningEffort('default', { persist: true }), and daemon-side persistReasoningSelection('default') clears the stored preference from every writable scope. A second entrance of the same arm: an explicit tier intent laundered through a preview-less intermediate model is rewritten to 'default' (keepReasoningIntent is falsy without a target preview to check) and later retargeted by the first arm — the explicit selection is destroyed and the stored preference wiped. Persisted medium projecting into the welcome preview → two model clicks through a reasoning-less model and back, zero reasoning interaction → the stored medium is deleted from every scope; pre-diff no intent was synthesized and the stored value survived. The design doc's Capability reconciliation speaks of removing 'every explicit selection', and default is described as a control command sent when the user turns thinking on again — this input is neither. The author maintains this is the explicit PR contract (round-1 thread); the re-check rules the mechanism still stands as filed, and the doc's own 'explicit' qualifier does not describe it.
Witness:
probe (App.test.tsx harness, scratch tree — welcome preview effort 'medium', zero reasoning interaction,
onSelectModel('qwen-plus') then onSelectModel('qwen3.8-max'), first prompt):
PROBE_C20_SET_REASONING_CALLS: [["default",{"persist":true}]]
flip (synthesis gated on an explicit sourceReasoningIntent): PROBE_C20_SET_REASONING_CALLS: [] and the displayed effort stays 'medium'
| setPendingReasoningIntent( | |
| sourceReasoningIntent && keepReasoningIntent | |
| ? { modelId, value: sourceReasoningIntent.value } | |
| : sourceReasoningSelection && !keepReasoningIntent | |
| ? { modelId, value: 'default' } | |
| : undefined, | |
| ); | |
| setPendingReasoningIntent( | |
| sourceReasoningIntent && keepReasoningIntent | |
| ? { modelId, value: sourceReasoningIntent.value } | |
| : sourceReasoningIntent && !keepReasoningIntent | |
| ? { modelId, value: 'default' } | |
| : undefined, | |
| ); |
The fix must not turn a preview-derived (non-explicit) selection into an implicit default command — the daemon-side reconcileReasoningSelection at the create-time setModel still removes an unsupported persisted tier, preserving the design's end state (docs/design/webshell-reasoning-preferences.md 'Capability reconciliation': 'a model without reasoning controls removes every explicit selection'). Note the e2e 'discards an incompatible welcome effort when switching away and back' currently pins the destructive no-interaction behavior and must be re-pinned with the fix. Fix witness: add an App.test.tsx case — welcome preview projecting 'medium', no explicit selection, switch to a reasoning-less model and back, submit — asserting mockSessionActions.setReasoningEffort was NOT called; it goes red if the synthesis for non-explicit selections is reintroduced.
中文说明
第一轮 R1-7 的机制在当前 head 复查后仍然存在：这个无会话分支即使用户从未触碰推理控件，也会从预览推导的选择合成 'default' 推理意图——sourceReasoningSelection 回退到 getReasoningSelection(sourceReasoningPreview)。'default' 无条件通过 ensureSessionForPrompt 的门控（reasoningPreviewSupports 对 'default' 恒返回 true，App.tsx:1285），首条提示发出 setReasoningEffort('default', { persist: true })，daemon 侧 persistReasoningSelection('default') 从所有可写 scope 清除已存偏好。同一分支还有第二个入口：显式档位意图经过一个无预览的中间模型时被改写为 'default'（没有目标预览可判断时 keepReasoningIntent 为假），随后被第一个分支重定向——显式选择被摧毁、已存偏好被清除。具体路径：已存 medium 投射到 Welcome 预览 → 零推理交互地切换到一个无推理控件的模型再切回 → 已存的 medium 从所有 scope 被删除；PR 前不会合成意图、不会调用 setReasoningEffort，已存值得以保留。设计文档的 Capability reconciliation 写的是移除 "every explicit selection"，且 default 被描述为用户重新开启思考时发出的控制命令——本输入两者皆非。作者坚持这是 PR 的明确契约（第一轮线程）；复查裁决该机制按第一轮所述仍然存在，且文档自身的 "explicit" 限定无法描述该输入。
见证（probe，App.test.tsx 脚手架，临时树——Welcome 预览 effort 'medium'、零推理交互、onSelectModel('qwen-plus') 后 onSelectModel('qwen3.8-max')、首条提示）：PROBE_C20_SET_REASONING_CALLS: [["default",{"persist":true}]]；将合成门控为仅显式 sourceReasoningIntent 后翻转：PROBE_C20_SET_REASONING_CALLS: [] 且显示 effort 保持 'medium'。
修复不得把预览推导（非显式）的选择变成隐式 default 命令——创建时 setModel 的 daemon 侧 reconcileReasoningSelection 仍会移除不兼容的已存档位，设计终态得以保留（设计文档 'Capability reconciliation'："无推理控件的模型移除所有显式选择"）。注意 e2e 'discards an incompatible welcome effort when switching away and back' 目前钉住了这一无交互破坏行为，需随修复重新钉住。修复见证：新增 App.test.tsx 用例——Welcome 预览投射 'medium'、无显式选择、切到无推理模型再切回、提交——断言 mockSessionActions.setReasoningEffort 未被调用；重新引入非显式合成时该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const models = connectionRef.current.models; | ||
| const reasoningIntent = pendingReasoningIntentRef.current; |
There was a problem hiding this comment.
[Critical] R2-c9: [certifies-falsely] The /model <id> slash command's welcome branch (App.tsx:10203-10206, unchanged lines) does only setPendingModel(modelArg) and never runs this new reasoning-intent reconciliation; setPendingModel touches only the model state (5995-5997). An explicit reasoning selection therefore survives a picker switch but is silently dropped after a slash switch: ensureSessionForPrompt gates on reasoningIntent.modelId === modelId (6227-6231), the stale-bound intent yields reasoningEffort = undefined, and createAndAttachSessionForPrompt never calls setReasoningEffort. Concretely: Welcome on qwen3.8-max, the user selects 'Thinking off' (intent {modelId: 'qwen3.8-max', value: 'none'}), submits /model qwen3.7-plus (toggle-only, canDisable !== false), and sends the first prompt — the intent stays bound to qwen3.8-max, the modelId gate fails, and the new session starts with thinking ON, the exact opposite of the explicit choice; the identical switch through the picker retargets the intent and applies + persists 'none'. This contradicts the PR's own design doc ('Every model switch reconciles the stored selection against the target model'; 'a supported tier and an allowed none value are retained'). The bypass itself was noted as R1-20 (Suggestion, deferred); this is the traced consequence for explicit new selections.
Witness:
witness: not run — the scenario is a React-state sequence through a private ref; the nearest capability was a DOM-level probe
of onSubmit('/model …') after an explicit 'none' selection, but each of the four code links (slash branch has no reconciliation
call; setPendingModel touches only model state; ensureSessionForPrompt's modelId gate; createAndAttachSessionForPrompt's
truthy-only setReasoningEffort) was read and quoted at HEAD and leaves no gap for the intent to be cleared or retargeted on the slash path.
Extract the reconciliation block into a shared helper and call it in the slash welcome branch:
// App.tsx — shared helper used by BOTH handleModelSelect's welcome branch and the /model welcome branch
const reconcilePendingReasoningIntentForSwitch = (targetModelId: string) => {
const models = connectionRef.current.models;
const reasoningIntent = pendingReasoningIntentRef.current;
const sourceReasoningIntent =
reasoningIntent?.modelId === currentModelRef.current ? reasoningIntent : undefined;
// …same derivation/keep/synthesize logic as handleModelSelect…
};
// in the /model welcome branch (~10204): reconcilePendingReasoningIntentForSwitch(modelArg); setPendingModel(modelArg);The fix must not retroactively retarget an already-stale intent — App.test.tsx:14649 pins that a stale intent's value stays unapplied; the picker-path reconciliation here is the reference behavior the slash fix must reach parity with. Fix witness: when the slash welcome branch reconciles, flip App.test.tsx 'does not retarget a stale model-bound reasoning intent' (~14685) to the reconciled outcome for a supported intent — its current assertion (preview value after the slash switch) pins the drop and goes red the moment the fix lands.
中文说明
/model <id> 斜杠命令的 welcome 分支（App.tsx:10203-10206，未改动行）只做 setPendingModel(modelArg)，从不运行这里新增的推理意图协调；setPendingModel 只更新模型状态（5995-5997）。因此显式的推理选择在选层器切换后存活，但在斜杠切换后被静默丢弃：ensureSessionForPrompt 以 reasoningIntent.modelId === modelId 门控（6227-6231），过期绑定的意图得到 reasoningEffort = undefined，createAndAttachSessionForPrompt 不再调用 setReasoningEffort。具体场景：Welcome 处于 qwen3.8-max，用户选择 'Thinking off'（意图 {modelId: 'qwen3.8-max', value: 'none'}），提交 /model qwen3.7-plus（仅开关、canDisable !== false）后发送首条提示——意图仍绑定 qwen3.8-max，modelId 门控失败，新会话以思考开启启动，与显式选择完全相反；同样的切换经选层器则会重定向意图并应用+持久化 'none'。这与 PR 自身设计文档矛盾（"每次模型切换都会对目标模型协调已存选择"；"支持的档位与允许的 none 值会被保留"）。该绕过本身在 R1-20（Suggestion，已延期）中被提及；本条是对显式新选择追踪出的实际后果。
见证：未运行——场景是经由私有 ref 的 React 状态序列；最近的能力是显式 'none' 选择后驱动 onSubmit('/model …') 的 DOM 级探针，但四个代码链接（斜杠分支无协调调用；setPendingModel 只动模型状态；ensureSessionForPrompt 的 modelId 门控；createAndAttachSessionForPrompt 仅对 truthy 调用 setReasoningEffort）均已在 HEAD 逐一定位并引用，斜杠路径上不存在清空或重定向意图的缝隙。
请把协调块抽成共享 helper 并在斜杠 welcome 分支调用（见英文侧代码）。修复不得追溯重定向已过期意图——App.test.tsx:14649 钉住过期意图的值不被应用；此处选层器路径的协调是斜杠修复必须对齐的参照行为。修复见证：斜杠 welcome 分支实现协调后，将 App.test.tsx 'does not retarget a stale model-bound reasoning intent'（~14685）翻转为支持意图的协调结果——其当前断言（斜杠切换后显示预览值）钉住了丢弃行为，修复落地即变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| this.reconcileReasoningSelection(effectiveModelId, { | ||
| persist: !this.requiresManagedConversationBinding, | ||
| allowReasoning: !isRuntime && !rawModelId.startsWith(ACP_ROUTE_ID_PREFIX), | ||
| }); |
There was a problem hiding this comment.
[Critical] R2-c14: [certifies-falsely] [new-surface] CLASS FINDING, two entrances (bounded family): this allowReasoning gate judges the OPAQUE id shape, not the resolved model's capabilities, so selecting a runtime snapshot model OR a colliding qwen-route:v1:<hash> route permanently deletes a persisted reasoning preference that the resolved BASE model fully supports — while the same setModel call persists that base model as the durable default (LoadedSettings.setValue strips the runtime prefix, 'Never persist a runtime snapshot ID to model.name', settings.ts:541-543; route selection writes model.name = effectiveModelId at Session.ts:9894). The clear decision and the persisted outcome disagree. While a route id is current, buildConfigOptions omits the reasoning_effort option entirely (acpAgent.ts:13214-13221), so there is no UI path to re-persist. Route entrance: two providers exposing the same model id+authType make buildAcpModelOptions emit ONLY route ids for that model (counts.get(legacyModelId) !== 1, acpModelUtils.ts:104-110), so the picker has no plain entry and every selection of that model clears the preference. With persisted 'none', the next start re-enables thinking (paid tokens) against the explicit choice; switching back never restores it. The identical model selected by plain id retains the preference (pinned by 'keeps a compatible persisted tier when switching models').
Witness:
probe (persisted 'low', two colliding USE_OPENAI providers for qwen3.8-max, scratch tree):
PROBE-C29 routeId: qwen-route:v1:OS5gD3-V5wq6z6fg
PROBE-C29 after ROUTE selection, user reasoningEffort: undefined (deleted)
PROBE-C29 after ROUTE selection, workspace reasoningEffort: undefined
PROBE-C29 persisted model.name: [["User","model.name","qwen3.8-max"]]
PROBE-C29 after PLAIN selection, user reasoningEffort: "low" (retained)
flip (allowReasoning: !isRuntime): route arm retains "low" in both scopes
| this.reconcileReasoningSelection(effectiveModelId, { | |
| persist: !this.requiresManagedConversationBinding, | |
| allowReasoning: !isRuntime && !rawModelId.startsWith(ACP_ROUTE_ID_PREFIX), | |
| }); | |
| this.reconcileReasoningSelection(effectiveModelId, { | |
| persist: !this.requiresManagedConversationBinding, | |
| allowReasoning: !isRuntime, | |
| }); |
resolveAcpModelOption returns the plain id (modelId: matched.effectiveModelId, packages/cli/src/utils/acpModelUtils.ts:129) and effectiveModelId is what reconcile receives and what persistDefault writes as model.name (Session.ts:9894) — the fix must keep judging/persisting that resolved id, not the opaque route id. Fix witness: new Session.test.ts case mirroring the existing route switches (Session.test.ts:5008): build a two-provider colliding list for a reasoning-capable model, installReasoningPreference('low'), setModel({ modelId: routeId }) → assert state.user.settings.model.reasoningEffort is still 'low' and state.live.reasoning equals { effort: 'low' }; re-adding !rawModelId.startsWith(ACP_ROUTE_ID_PREFIX) makes it red.
中文说明
类发现，两个入口（有界族）：该 allowReasoning 门控按不透明 id 的形状判断，而不是按解析后模型的能力判断，因此选择运行时快照模型或冲突产生的 qwen-route:v1:<hash> 路由会永久删除一个解析后基础模型完全支持的已存推理偏好——而同一个 setModel 调用却把该基础模型作为持久默认写入（LoadedSettings.setValue 会剥离运行时前缀，"绝不把运行时快照 ID 持久化到 model.name（重启时会重新包装）"，settings.ts:541-543；路由选择把 model.name = effectiveModelId 写入 Session.ts:9894）。清除决定与持久化结果互相矛盾。路由 id 为当前模型时，buildConfigOptions 完全省略 reasoning_effort 选项（acpAgent.ts:13214-13221），没有任何 UI 路径可以重新持久化。路由入口：两个 provider 暴露相同 model id+authType 时，buildAcpModelOptions 对该模型只发出路由 id（counts.get(legacyModelId) !== 1，acpModelUtils.ts:104-110），选层器没有普通条目，该模型的每次选择都会清除偏好。若已存 'none'，下次启动会在违背显式选择的情况下重新开启思考（计费 token）；切回也永不恢复。同一模型以普通 id 选择时偏好被保留（由 'keeps a compatible persisted tier when switching models' 钉住）。
见证（probe，已存 'low'、qwen3.8-max 的两个冲突 USE_OPENAI provider，临时树）：路由选择后 user/workspace reasoningEffort 均为 undefined（被删除），持久化 model.name 为 [["User","model.name","qwen3.8-max"]]；普通 id 选择后 user reasoningEffort 为 "low"（保留）；将门控改为 allowReasoning: !isRuntime 后，路由分支两个 scope 均保留 "low"。
resolveAcpModelOption 返回普通 id（modelId: matched.effectiveModelId，packages/cli/src/utils/acpModelUtils.ts:129），effectiveModelId 正是 reconcile 收到的、也是 persistDefault 写入 model.name 的值（Session.ts:9894）——修复必须继续判断/持久化该解析后的 id，而不是不透明路由 id。修复见证：仿照现有路由切换测试（Session.test.ts:5008）新增 Session.test.ts 用例：为支持推理的模型构造双 provider 冲突列表，installReasoningPreference('low')，setModel({ modelId: routeId }) → 断言 state.user.settings.model.reasoningEffort 仍为 'low' 且 state.live.reasoning 等于 { effort: 'low' }；重新加回 !rawModelId.startsWith(ACP_ROUTE_ID_PREFIX) 时变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| generation.reasoning = { | ||
| ...(generation.reasoning || {}), | ||
| effort: selection, | ||
| }; |
There was a problem hiding this comment.
[Critical] R2-c22: [certifies-falsely] [regression] CLASS FINDING, two branches (bounded family): applyReasoningSelection bypasses the documented Config.setReasoningEffort invariant — 'No-op when thinking is explicitly disabled (reasoning: false) so effort cannot silently re-enable it' (config.ts:5083-5093). Tier branch (here): the model-switch reconcile applies a persisted effort tier over an explicit reasoning: false preset of the target model — isReasoningSelectionSupported judges by model id only (generic branch REASONING_EFFORT_TIERS.includes(selection)), so the selection counts as 'supported', {...(generation.reasoning || {}), effort} spreads false || {} → {}, and the disable is overwritten on BOTH live and rebuildable configs; the selection is retained and re-applied on every later switch. DEFAULT branch (160-162): !generation.reasoning treats an explicit false identically to 'no reasoning set' and erases it to undefined on the mustClear cleanup path (reconcileReasoningSelection calls applyReasoningSelection(config, 'default'), Session.ts:9960-9977), silently re-enabling thinking without any user selection of 'default'. A fix guarding only one branch leaves the other erasing the same disable. Trigger: persisted model.reasoningEffort: 'medium'; switch to a model whose provider/registry generationConfig sets reasoning: false (constructible: ModelGenerationConfig includes 'reasoning', models/types.ts:44; applyResolvedModelDefaults copies it on switch, modelsConfig.ts:915-921) — thinking the provider explicitly disabled is silently re-enabled (paid tokens, changed output) and the selection is kept, so every subsequent switch/restart re-applies it. Contrast: choosing the tier directly via the reasoning_effort option takes the guarded branch (acpAgent.ts:5764-5766) and rejects; core's own switch restoration deliberately skips this case (config.ts:5379-5392). Distinct from the posted R2-3 (sibling destruction on the none branch) and R2-6 (dead no-op call in the R1-1 fix branch): this is the switch-time reconcile overriding a preset disable.
Witness:
probe (real Session.setModel, scratch tree):
tier branch — persisted 'max', target preset reasoning:false:
intact: live.reasoning = {"effort":"max"} rebuildable.reasoning = {"effort":"max"} persisted = "max" (disable overwritten)
with guard: live.reasoning = false rebuildable.reasoning = false
DEFAULT branch — mustClear over reasoning:false preset:
intact: live/rebuildable reasoning = undefined (explicit disable erased)
with fix: false (disable kept)
Guard both branches:
// tier branch (here): skip the apply when the config explicitly disables thinking
if (generation.reasoning === false) return;
generation.reasoning = {
...(generation.reasoning || {}),
effort: selection,
};
// DEFAULT branch (160-162): keep an explicit disable sticky
if (selection === REASONING_EFFORT_DEFAULT) {
if (generation.reasoning === false) return;
if (!generation.reasoning) {The fix must make applyReasoningSelection consistent with the config.ts:5083-5093 invariant, and the existing 'preserves reasoning siblings when returning to the model default' test must still pass for truthy reasoning objects. Fix witness: model-configuration.test.ts cases starting from { reasoning: false } on both live and rebuildable — applyReasoningSelection(config, 'max') and ('default') must both leave reasoning === false; removing either guard makes them red. Plus a Session.test.ts variant whose switchModelSpy lands reasoning: false on both configs — the persisted tier must be cleared, not applied.
中文说明
类发现，两个分支（有界族）：applyReasoningSelection 绕过了 Config.setReasoningEffort 的文档化不变量——"思考被显式禁用（reasoning: false）时空操作，防止档位静默重新启用它"（config.ts:5083-5093）。档位分支（此处）：模型切换 reconcile 会把已存档位应用在目标模型显式 reasoning: false 的预设之上——isReasoningSelectionSupported 只按模型 id 判断（通用分支 REASONING_EFFORT_TIERS.includes(selection)），该选择被视为"支持"，{...(generation.reasoning || {}), effort} 展开 false || {} → {}，live 与可重建两个 config 上的禁用都被覆盖；选择被保留并在之后每次切换时重新应用。DEFAULT 分支（160-162）：!generation.reasoning 把显式 false 与"未设置"同等对待，在 mustClear 清理路径上把它抹成 undefined（reconcileReasoningSelection 调用 applyReasoningSelection(config, 'default')，Session.ts:9960-9977），在用户没有选择 'default' 的情况下静默重新启用思考。只修一个分支，另一个仍会抹掉同一个禁用。触发：已存 model.reasoningEffort: 'medium'；切换到一个 provider/注册表 generationConfig 设置 reasoning: false 的模型（可构造：ModelGenerationConfig 含 'reasoning'，models/types.ts:44；applyResolvedModelDefaults 在切换时拷贝，modelsConfig.ts:915-921）——provider 显式禁用的思考被静默重新启用（计费 token、输出变化），且选择被保留，之后每次切换/重启都会重新应用。对照：直接经 reasoning_effort 选项选择档位会走有守卫的分支（acpAgent.ts:5764-5766）并被拒绝；core 自身的切换恢复也刻意跳过该情形（config.ts:5379-5392）。与已发布的 R2-3（none 分支摧毁同级字段）和 R2-6（R1-1 修复分支的死空操作调用）不同：本条是切换时 reconcile 覆盖预设禁用。
见证（probe，真实 Session.setModel，临时树）：档位分支——已存 'max'、目标预设 reasoning:false：原样时 live/rebuildable 均为 {"effort":"max"}、persisted = "max"（禁用被覆盖）；加守卫后均为 false。DEFAULT 分支——对 reasoning:false 预设执行 mustClear：原样时 live/可重建 reasoning = undefined（显式禁用被抹除）；修复后为 false（禁用保留）。
修复必须使 applyReasoningSelection 与 config.ts:5083-5093 不变量一致，且现有 'preserves reasoning siblings when returning to the model default' 测试对 truthy reasoning 对象仍须通过。修复见证：model-configuration.test.ts 用例，从 live/可重建均为 { reasoning: false } 开始——applyReasoningSelection(config, 'max') 与 ('default') 后 reasoning 均须保持 === false；移除任一首卫即变红。另加 Session.test.ts 变体，其 switchModelSpy 在两个 config 上落地 reasoning: false——已存档位必须被清除而不是被应用。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (isInitialAuth === undefined) { | ||
| await config.refreshAuth(authType); | ||
| } else { | ||
| await config.refreshAuth(authType, isInitialAuth); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-c1: Both branches of this isInitialAuth === undefined check make the identical call: Config.refreshAuth is async refreshAuth(authMethod: AuthType, isInitialAuth?: boolean) with no default initializer (packages/core/src/config/config.ts:4109), and passing undefined explicitly is indistinguishable from omitting the optional parameter. The branch encodes a semantic difference that does not exist at the single choke point through which every ACP-agent auth refresh now flows — a maintainer reading it infers a false omit-vs-pass distinction, cargo-cults it into new call sites, or avoids simplifying the call out of fear of breaking a non-existent difference.
Witness:
witness: not run — no probe can distinguish the two arms; they both reduce to refreshAuth(authType, undefined) by language
semantics, and the sole read of the parameter (`requireCached ? true : isInitialAuth`, packages/core/src/config/config.ts:4146)
was quoted from the real source.
| if (isInitialAuth === undefined) { | |
| await config.refreshAuth(authType); | |
| } else { | |
| await config.refreshAuth(authType, isInitialAuth); | |
| } | |
| await config.refreshAuth(authType, isInitialAuth); |
The collapse must keep forwarding isInitialAuth — async refreshAuth(authMethod: AuthType, isInitialAuth?: boolean) at packages/core/src/config/config.ts:4109 — because ensureAuthenticated deliberately passes true (initial-auth semantics feed createContentGenerator(..., requireCached ? true : isInitialAuth)).
中文说明
这个 isInitialAuth === undefined 判断的两个分支执行的是完全相同的调用：Config.refreshAuth 是 async refreshAuth(authMethod: AuthType, isInitialAuth?: boolean)，无默认初始值（packages/core/src/config/config.ts:4109），显式传 undefined 与省略可选参数没有任何区别。该分支在"所有 ACP agent 鉴权刷新唯一必经之路"上编码了一个并不存在的语义差异——维护者读到它会误以为传与不传不同，把它照抄进新的调用点，或因为害怕破坏一个不存在的差异而不敢简化调用。
见证：未运行——没有任何探针能区分两个分支；按语言语义两者都归约为 refreshAuth(authType, undefined)，参数的唯一读取处（requireCached ? true : isInitialAuth，packages/core/src/config/config.ts:4146）已从真实源码引用。
折叠后必须继续转发 isInitialAuth——packages/core/src/config/config.ts:4109 的 async refreshAuth(authMethod: AuthType, isInitialAuth?: boolean)——因为 ensureAuthenticated 刻意传入 true（初始鉴权语义会进入 createContentGenerator(..., requireCached ? true : isInitialAuth)）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const REASONING_SELECTIONS = new Set<ReasoningSelection>([ | ||
| 'none', | ||
| 'default', | ||
| 'low', | ||
| 'medium', | ||
| 'high', | ||
| 'xhigh', | ||
| 'max', | ||
| ]); |
There was a problem hiding this comment.
[Suggestion] R2-c2: AGGREGATE location 2 (of 2) — the web-shell twin of the hand-listed ladder: this file lists the ladder TWICE (here all 7 selections, plus isReasoningEffortSelection's 5 tiers at mappers.ts:48-59), decoupled from the SDK union (ReasoningSelection is a type with no runtime counterpart, packages/sdk-typescript/src/daemon/types.ts:2808). When core ships a new tier, the daemon advertises it in reasoning_effort options/currentValue; parseReasoningSelection rejects the unknown string, and with currentValue on the new tier the !currentValue || !values.includes(currentValue) guard makes mapReasoningControls return undefined — the composer's thinking toggle/effort buttons and the model-picker preview silently disappear for a thinking-capable model. No compile error catches the drift (Set<ReasoningSelection> is satisfied by a subset), and the two in-file lists can diverge from each other: a value added to the Set but not the predicate parses as a valid current value yet is excluded from efforts, so the selected tier renders no button and nothing shows pressed.
Witness:
probe (mapReasoningControls, scratch tree):
PROBE_C17_CONTROLS_FOR_ULTRA: undefined ← unknown tier hides the controls
flip (adding 'ultra' to both in-file lists): {"enabled":true,"effort":"ultra","efforts":["low","ultra"],...}
Export a runtime selection list from @qwen-code/sdk/daemon and derive both REASONING_SELECTIONS and isReasoningEffortSelection from it; at minimum derive the predicate from the Set (value !== 'none' && value !== 'default' && REASONING_SELECTIONS.has(value)) so the two in-file lists cannot diverge. ReasoningSelection exists today only as a type (packages/sdk-typescript/src/daemon/types.ts:2808) and AGENTS.md forbids relative imports between packages — the shared constant must be added to @qwen-code/sdk/daemon; web-shell must not reach into packages/core for REASONING_EFFORT_TIERS. Fix witness: an mappers.test.ts case building configOptions by iterating the SDK's exported runtime selection list and asserting mapReasoningControls returns defined controls for every member — restoring a stale hand-list makes it red.
中文说明
聚合发现第 2 处（共 2 处）——手工列举阶梯的 web-shell 孪生：本文件把阶梯列了两遍（此处为全部 7 个选择，另有 mappers.ts:48-59 的 isReasoningEffortSelection 的 5 个档位），与 SDK 联合类型脱钩（ReasoningSelection 只是类型，无运行时对应物，packages/sdk-typescript/src/daemon/types.ts:2808）。当 core 发布新档位时，daemon 会在 reasoning_effort 选项/currentValue 中声明它；parseReasoningSelection 拒绝未知字符串，currentValue 落在新档位上时 !currentValue || !values.includes(currentValue) 守卫使 mapReasoningControls 返回 undefined——支持思考的模型的输入框思考开关/档位按钮与选层器预览静默消失。编译错误抓不到这种漂移（Set<ReasoningSelection> 对子集也成立），且文件内两个列表可以互相漂移：加入 Set 但未加入谓词的值能解析为有效 currentValue 却被排除在 efforts 之外，所选档位不渲染按钮、也无按下状态。
见证（probe，mapReasoningControls，临时树）：PROBE_C17_CONTROLS_FOR_ULTRA: undefined（未知档位隐藏控件）；把 'ultra' 加入文件内两个列表后翻转：{"enabled":true,"effort":"ultra","efforts":["low","ultra"],...}。
请从 @qwen-code/sdk/daemon 导出运行时选择列表，并用它推导 REASONING_SELECTIONS 与 isReasoningEffortSelection；至少让谓词从 Set 推导（value !== 'none' && value !== 'default' && REASONING_SELECTIONS.has(value)），使文件内两个列表无法漂移。ReasoningSelection 目前只是类型（packages/sdk-typescript/src/daemon/types.ts:2808），AGENTS.md 禁止包间相对导入——共享常量必须加在 @qwen-code/sdk/daemon；web-shell 不得伸手到 packages/core 取 REASONING_EFFORT_TIERS。修复见证：新增 mappers.test.ts 用例，按 SDK 导出的运行时选择列表构造 configOptions，断言 mapReasoningControls 对每个成员返回已定义控件——恢复陈旧的手工列表时变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (!confirmed || (opts?.persist && result.persisted !== true)) { | ||
| throw new Error( | ||
| `Daemon did not confirm reasoning effort ${JSON.stringify(value)}`, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-c13: This branch reuses the 'Daemon did not confirm reasoning effort' error even when the daemon DID confirm the effort (confirmed === true, only result.persisted !== true), misattributing a persistence failure as a confirmation failure — and the throw skips the setConnection block below (1258-1295), with the catch path going through dispatchActionError, which only appends a notice — so connection.reasoning stays stale while the daemon session's effort actually changed. The old-daemon case is exactly when this fires: such a daemon applies the change live and confirms it via configOptions, yet the user sees a confirmation-denied message, retries a change that is already live, and the reasoning menu keeps showing the old effort until the next context snapshot re-syncs it.
Witness:
witness: not run — both facts are directly quotable at the throw site (condition, message, and throw-before-setConnection
ordering) and in dispatchActionError (actions.ts:2658-2695, notice-only); the committed actions.test.ts case covers the same
path but was not re-run because executing web-shell tests writes artifacts into the shared review worktree.
| if (!confirmed || (opts?.persist && result.persisted !== true)) { | |
| throw new Error( | |
| `Daemon did not confirm reasoning effort ${JSON.stringify(value)}`, | |
| ); | |
| if (!confirmed || (opts?.persist && result.persisted !== true)) { | |
| throw new Error( | |
| confirmed | |
| ? `Daemon did not persist reasoning effort ${JSON.stringify(value)}` | |
| : `Daemon did not confirm reasoning effort ${JSON.stringify(value)}`, | |
| ); |
Consider also applying the confirmed live reasoning state instead of discarding it when only persistence failed. Fix witness: update 'does not update reasoning when persistence is not confirmed' to assert the persistence-specific message; reverting to the conflated message makes it red.
中文说明
该分支即使在 daemon 已经确认了档位（confirmed === true，仅 result.persisted !== true）时也复用 'Daemon did not confirm reasoning effort' 错误，把持久化失败误标为确认失败——而且 throw 跳过了下方的 setConnection 块（1258-1295），catch 路径只走 dispatchActionError（仅追加通知）——于是 daemon 会话的档位实际已变，connection.reasoning 却保持陈旧。旧版 daemon 正是触发场景：它会在 live 应用更改并通过 configOptions 确认，但用户看到的是"确认被拒"的提示，进而重试一个已经生效的更改，推理菜单持续显示旧档位直到下一次上下文快照重新同步。
见证：未运行——两个事实均可在 throw 处直接引用（条件、消息、先 throw 后 setConnection 的顺序）以及 dispatchActionError（actions.ts:2658-2695，仅通知）中引用；已提交的 actions.test.ts 用例覆盖同一路径，但未重跑，因为执行 web-shell 测试会向共享审查工作树写入产物。
也可考虑在仅持久化失败时应用已确认的 live 推理状态而不是丢弃它。修复见证：更新 'does not update reasoning when persistence is not confirmed'，断言持久化专用消息；回退为混用消息时变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const session = sessionRef.current; | ||
| manualSessionClearRef.current = true; | ||
| if (pendingPersistedReasoningAction) { | ||
| await pendingPersistedReasoningAction.catch(() => undefined); |
There was a problem hiding this comment.
[Suggestion] R2-c16: clearSession blocks session teardown on the pending persisted-reasoning action, which resolves only in setReasoningEffort's finally after withActionTimeout settles — DEFAULT_ACTION_TIMEOUT_MS = 30_000 (daemon/timing.ts:9). All three UI sites pass persist: true (App.tsx handleReasoningEffort, ChatPane.tsx:1140, sessionPreparation.ts:145-147), so any reasoning change arms the pending action; a slow or hung daemon then stalls 'New task' (createNewSession awaits clearSession) for up to 30 seconds while the old session stays fully operable — pre-diff clearSession cleared state synchronously. The await's documented purpose (letting the confirmed persisted preview land in providers before Welcome renders) answers the happy path; it names no answer for the hung-daemon path. Distinct from the posted R2-4 replacement-destruction defect: this is the latency itself, no race required.
Witness:
probe (actions harness, deferred setConfigOption, scratch tree):
PROBE_C16_WHILE_PENDING: {"clearCompleted":false,"connectionSessionId":"session-a","sessionRefStillSet":true}
after settle: {"clearCompleted":true,"sessionRefStillSet":false}
flip (await removed): clear immediate — sessionRef already undefined while the persist is still pending
| await pendingPersistedReasoningAction.catch(() => undefined); | |
| await Promise.race([ | |
| pendingPersistedReasoningAction.catch(() => undefined), | |
| new Promise((resolve) => setTimeout(resolve, 1_500)), | |
| ]); |
The grace bound must not be shorter than the settle window relied on by 'keeps the confirmed persisted reasoning preview after clearing the session' (actions.test.ts:3236), which pins that a settled persist is captured into the post-clear models preview. Fix witness: new actions.test.ts case — leave the setConfigOption mock unresolved, start setReasoningEffort('medium', { persist: true }) then clearSession(), advance timers past the grace period → assert getConnection().sessionId is cleared while the persist is still pending; the preview-capture test must stay green.
中文说明
clearSession 将会话拆除阻塞在挂起的持久化推理动作上，而该动作只在 withActionTimeout 落定后由 setReasoningEffort 的 finally 解析——DEFAULT_ACTION_TIMEOUT_MS = 30_000（daemon/timing.ts:9）。三个 UI 调用点都传 persist: true（App.tsx handleReasoningEffort、ChatPane.tsx:1140、sessionPreparation.ts:145-147），因此任何推理更改都会挂上该动作；daemon 缓慢或挂起时，"新建任务"（createNewSession await clearSession）会被卡住最长 30 秒，而旧会话在此期间完全可操作——PR 前 clearSession 是同步清理状态的。该 await 的文档化目的（让已确认的持久化预览在 Welcome 渲染前落入 providers）只覆盖顺利路径；对挂起的 daemon 没有任何方案。与已发布的 R2-4（替换会话被摧毁）不同：本条是延迟本身，无需竞态。
见证（probe，actions 脚手架，延迟 setConfigOption，临时树）：PROBE_C16_WHILE_PENDING: {"clearCompleted":false,"connectionSessionId":"session-a","sessionRefStillSet":true}；解析后：{"clearCompleted":true,"sessionRefStillSet":false}；移除 await 后翻转：清理立即完成——persist 仍挂起时 sessionRef 已为 undefined。
宽限下限不得短于 'keeps the confirmed persisted reasoning preview after clearing the session'（actions.test.ts:3236）依赖的解析窗口——该测试钉住已解析的 persist 会被捕获进清理后的 models 预览。修复见证：新增 actions.test.ts 用例——保持 setConfigOption mock 未解析，启动 setReasoningEffort('medium', { persist: true }) 后调用 clearSession()，推进定时器越过宽限期→断言 persist 仍挂起时 getConnection().sessionId 已被清除；预览捕获测试须保持绿色。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| ).rejects.toThrow( | ||
| 'Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-c25: The new supported check accepts 'default' for registered tiered models (selected === REASONING_EFFORT_DEFAULT || choices.includes(selected), acpAgent.ts:5727-5731), but the registered-model rejection branch still builds allowedChoices = choices (5733-5734), and buildModelReasoningConfigOption never emits a 'default' option for tiered models — so 'default' is valid yet absent from the 'Choose one of' list, while the unregistered branch explicitly prepends it (5735-5740) and toggle-only models carry it in choices. The same test proves 'default' works for that branch three lines earlier (the reset call at ~8318); the same omission appears in the thinking-mandatory rejection (~8835). An ACP client sending an unsupported tier (e.g. 'high') to qwen3.8-max receives 'Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh' — any consumer deriving retry values from the message cannot discover the reset-to-default control this same PR's reset flow relies on.
Witness:
probe (real agent boot via the file's own scaffolding, scratch tree):
PROBE-C25 default accepted; currentValue=xhigh
PROBE-C25 rejection message: Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh
flip (default added to the registered branch): Choose one of: default, none, low, medium, xhigh
| ).rejects.toThrow( | |
| 'Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh', | |
| ); | |
| ).rejects.toThrow( | |
| 'Unknown reasoning effort: high. Choose one of: default, none, low, medium, xhigh', | |
| ); |
Build allowedChoices the same way for both branches in setSessionConfigOption — [REASONING_EFFORT_DEFAULT, ...choices.filter((choice) => choice !== REASONING_EFFORT_DEFAULT)] (drop the modelReasoning ? choices : … ternary) — and update this assertion. The generic-model message is pinned verbatim at acpAgent.test.ts:8138 ('Unknown reasoning effort: ultra. Choose one of: default, none, low, medium, high, xhigh, max'), so the unified construction must keep 'default' first and include 'none' there too. Fix witness: the updated rejects assertion at acpAgent.test.ts:8334 goes red if the fix reverts allowedChoices to choices for registered models.
中文说明
新的 supported 检查对注册的分档模型接受 'default'（selected === REASONING_EFFORT_DEFAULT || choices.includes(selected)，acpAgent.ts:5727-5731），但注册模型的拒绝分支仍构造 allowedChoices = choices（5733-5734），且 buildModelReasoningConfigOption 从不为分档模型发出 'default' 选项——于是 'default' 合法却缺席 'Choose one of' 列表，而未注册分支会显式前置它（5735-5740）、仅开关模型的 choices 也天然含它。同一测试在三行之前已证明 'default' 对该分支有效（~8318 的 reset 调用）；思考强制拒绝（~8835）存在同样的遗漏。向 qwen3.8-max 发送不支持档位（如 'high'）的 ACP 客户端会收到 'Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh'——任何从错误消息推导重试值的消费者都无法发现本 PR 的 reset 流程所依赖的 default 复位控制。
见证（probe，经该文件自身脚手架真实启动 agent，临时树）：PROBE-C25 default 被接受；currentValue=xhigh；拒绝消息：Unknown reasoning effort: high. Choose one of: none, low, medium, xhigh；把 default 加入注册分支后翻转：Choose one of: default, none, low, medium, xhigh。
请在 setSessionConfigOption 中为两个分支用同一方式构造 allowedChoices——[REASONING_EFFORT_DEFAULT, ...choices.filter((choice) => choice !== REASONING_EFFORT_DEFAULT)]（去掉 modelReasoning ? choices : … 三元式）——并更新此断言。通用模型消息在 acpAgent.test.ts:8138 被逐字钉住（'Unknown reasoning effort: ultra. Choose one of: default, none, low, medium, high, xhigh, max'），统一构造必须保持 'default' 在前且同样包含 'none'。修复见证：若修复把注册模型的 allowedChoices 还原为 choices，acpAgent.test.ts:8334 更新后的 rejects 断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| private clearPersistedReasoningSelection(except?: SettingScope): void { | ||
| this.commitReasoningSelectionWrites( | ||
| this.getReasoningSelectionClearWrites(except), | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R2-c26: clearPersistedReasoningSelection(except?) declares and forwards except, but no caller ever sets it — the only call site (reconcile's mustClear path, Session.ts:9967) passes nothing, and the one place that actually needs scope exclusion (persistReasoningSelection, Session.ts:9934, excluding persistScope) bypasses this wrapper and calls getReasoningSelectionClearWrites directly. Per the AGENTS.md read-site rule for added optional parameters this is a dead switch: the exclusion branch is never exercised by any production path, so a future maintainer wiring a new clear site through the wrapper with except would rely on exclusion semantics that have never run and have no test — a silent first-use experiment on the path that deletes user preferences.
Witness:
call-site sweep at HEAD: clearPersistedReasoningSelection has exactly one call site (Session.ts:9967, passes nothing);
the only site needing exclusion (Session.ts:9934) calls getReasoningSelectionClearWrites directly — 1 caller, 0 passing except.
| private clearPersistedReasoningSelection(except?: SettingScope): void { | |
| this.commitReasoningSelectionWrites( | |
| this.getReasoningSelectionClearWrites(except), | |
| ); | |
| } | |
| private clearPersistedReasoningSelection(): void { | |
| this.commitReasoningSelectionWrites( | |
| this.getReasoningSelectionClearWrites(), | |
| ); | |
| } |
Or inline it into the sole caller and delete the method.
中文说明
clearPersistedReasoningSelection(except?) 声明并转发了 except，但没有任何调用方设置它——唯一调用点（reconcile 的 mustClear 路径，Session.ts:9967）什么都不传，而真正需要 scope 排除的地方（persistReasoningSelection，Session.ts:9934，排除 persistScope）绕过该包装器、直接调用 getReasoningSelectionClearWrites。按 AGENTS.md 对新增可选参数的"读点"规则，这是一个死开关：排除分支在生产路径上从未执行过，未来若有维护者通过该包装器带着 except 接入新的清除点，将依赖一套从未运行过、也没有测试的排除语义——在删除用户偏好的路径上做一次无声的首用实验。
见证：HEAD 上的调用点扫描：clearPersistedReasoningSelection 恰好一个调用点（Session.ts:9967，不传参）；唯一需要排除的调用点（Session.ts:9934）直接调用 getReasoningSelectionClearWrites——1 个调用方，0 个传 except。
或直接内联到唯一调用点并删除该方法。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/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. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
17 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- dead/misleading setReasoningEffort call in the R1-1 fix branch (acpAgent.ts:~5773) — already reported (comment 3889107207)
- live-apply-then-persist-throw divergence (acpAgent.ts:~5806) — already reported as R2-5 (comment 3889107205)
- hand-listed reasoning ladder, model-configuration.ts:67-76 + mappers.ts:29-37 — already reported as R2-c2 aggregate (comments 3889181457, 3889181463)
- strict-vs-tolerant persisted-value interpreters (alias/case wipe via parseReasoningSelection) — already reported as R1-14/R1-21 (comments 3887695485, 3887695501), author deferred
- 'None' case-sensitivity in the modelConfigUtils seed — already reported as R1-14 (comment 3887695485), author deferred
- mandatory-'none' skip without warning — already reported as R1-26 (comment 3887695515), author deferred
- single-slot pendingPersistedReasoningAction drain race (clearSession/newSession) — already reported as R1-25 (comment 3887695512), author deferred
- withPersistedReasoningPreview patches only the current model's entry — already reported as R1-24 (comment 3887695508), author deferred
- none→'default' one-way latch on canDisable flip (App.tsx:6753-6756) — already discussed as R1-9 (comment 3887695470); author maintains the shipped clear-never-restore contract, the rewritten test pins it
- no-refetch guard counts an endpoint the flow never fetches (web-shell.smoke.spec.ts:586) — deferred in round 2 (D2-3)
- workspaceReload never propagates reasoningEffort-only settings edits — deferred in round 2 (code-age rule)
- isQwenFamilyModel duplicate of core isQwenFamilyWireModel — deferred in round 2 (code-age rule)
- isInitialAuth identical-branch call (acpAgent.ts:~12916) — already reported as R2-c1 (comment 3889181450), still standing, not re-posted
- 'Daemon did not confirm reasoning effort' misattribution on persist-only failure — already reported as R2-c13 (comment 3889181467), still standing, not re-posted
- clearSession blocks teardown up to 30s — already reported as R2-c16 (comment 3889181474), still standing, not re-posted
- 'default' absent from registered-model rejection message — already reported as R2-c25 (comment 3889181482), still standing, not re-posted
- clearPersistedReasoningSelection(except?) dead parameter — already reported as R2-c26 (comment 3889181489), still standing, not re-posted
Not reviewed: build-and-test — test suites never ran in this review: the packages/cli build fails (TS2554, see R3-1), which ended the build-test run before its test phase; test-efficacy probe 18/18 inconclusive (harnessValidated null).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
docs/users/configuration/settings.md:176 — [review] reasoningEffort row left stale by the new 'none' semantics (tiers-only via /effort; /effort rejects none) — deferred, code-age rule (file outside the diff, unchanged since round 2)
Convergence: round 3 posted 5 inline comment(s), 4 of them reported for the first time; the previous round posted 11 (10 new). Findings keep coming back to the same files: packages/cli/src/acp-integration/acpAgent.ts (findings in round 2; 2 more now); packages/cli/src/acp-integration/session/Session.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
[Critical] R2-1: [certifies-falsely] [new-surface] The /model <id> slash command's welcome branch (App.tsx:10203-10206) does only setPendingModel(modelArg) and never runs the new reasoning-intent reconciliation that the picker path runs — an explicit reasoning selection survives a picker switch but is silently dropped after a slash switch: ensureSessionForPrompt gates on reasoningIntent.modelId === modelId, the stale-bound intent yields reasoningEffort = undefined, and the new session starts with thinking ON against the explicit 'none' choice, contradicting the design doc's 'Every model switch reconciles the stored selection against the target model'. Witness: probe (App.test.tsx harness, scratch tree — explicit 'none' on qwen3.8-max, onSubmit('/model qwen3.7-plus'), first prompt): INTACT AssertionError: expected "spy" to be called with arguments: [ 'none', { persist: true } ] — Number of calls: 0; FIXED (reconciliation ported into the /model branch): probe passes. Fix: extract the reconciliation block from handleModelSelect's welcome branch into a shared helper and call it in the /model welcome branch before setPendingModel(modelArg); re-baseline the new test 'does not retarget a stale model-bound reasoning intent' (App.test.tsx:~14685), which manufactures staleness via this very branch and must not be satisfied by a false fix that merely clears the intent. (Unanchorable: the mechanism lives on unchanged lines outside the diff's hunks.)
[Critical] R3-5: [certifies-falsely] [new-surface] workspaceReload's model-name-change branch (acpAgent.ts:~12255-12266) calls raw config.switchModel(authType, newModelName) and never re-applies the persisted model.reasoningEffort: 'none' — unlike the PR-added sibling branch (which uses refreshAuthWithPersistedReasoning) and unlike Session.setModel, whose post-switch reconcile re-applies a compatible persisted selection after the same core switchModel call; applyResolvedModelDefaults overwrites every MODEL_GENERATION_CONFIG_FIELDS entry (reasoning included). A user who persisted 'none' and then changes model.name in settings.json gets a workspace reload that wipes the disable: the session silently runs with thinking ON while settings still say 'none', until an unrelated replay fires; the identical switch via setSessionConfigOption reconciles correctly. Witness: probe (booted QwenAgent, live generation.reasoning=false, settings reload flipping model.name with reasoningEffort 'none' persisted): branch taken — switchModel called — then AssertionError: expected undefined to be false (disable wiped, no replay); replaying via refreshAuthWithPersistedReasoning after the switch passes the probe. Fix: reuse the replay half of refreshAuthWithPersistedReasoning (persisted-none check + support gate + applyReasoningSelection(config, REASONING_EFFORT_NONE)) after await config.switchModel(authType, newModelName) succeeds in that branch, keeping the support gate so 'none' is never replayed onto a thinking-mandatory model. Fix witness: extend the acpAgent.test.ts workspaceReload suite — merged settings { model: { name: 'next-model', reasoningEffort: 'none' } }, live reasoning: false; after workspaceReload assert the session's generation config still has reasoning === false on the new model, then remove the post-switchModel replay and confirm the new test reds. (Unanchorable: the model-change branch is pre-existing code outside the diff's hunks.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 17 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — test suites never ran in this review: the packages/cli build fails (TS2554, see R3-1), which ended the build-test run before its test phase; test-efficacy probe 18/18 inconclusive (harnessValidated null)。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 5 条行内评论，其中 4 条是首次提出；上一轮发布了 11 条（其中 10 条首次提出）。发现反复回到同一批文件：packages/cli/src/acp-integration/acpAgent.ts（第 2 轮已出过发现，本轮又有 2 条）；packages/cli/src/acp-integration/session/Session.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
[Critical] R2-1: [certifies-falsely] [new-surface] The /model <id> slash command's welcome branch (App.tsx:10203-10206) does only setPendingModel(modelArg) and never runs the new reasoning-intent reconciliation that the picker path runs — an explicit reasoning selection survives a picker switch but is silently dropped after a slash switch: ensureSessionForPrompt gates on reasoningIntent.modelId === modelId, the stale-bound intent yields reasoningEffort = undefined, and the new session starts with thinking ON against the explicit 'none' choice, contradicting the design doc's 'Every model switch reconciles the stored selection against the target model'. Witness: probe (App.test.tsx harness, scratch tree — explicit 'none' on qwen3.8-max, onSubmit('/model qwen3.7-plus'), first prompt): INTACT AssertionError: expected "spy" to be called with arguments: [ 'none', { persist: true } ] — Number of calls: 0; FIXED (reconciliation ported into the /model branch): probe passes. Fix: extract the reconciliation block from handleModelSelect's welcome branch into a shared helper and call it in the /model welcome branch before setPendingModel(modelArg); re-baseline the new test 'does not retarget a stale model-bound reasoning intent' (App.test.tsx:~14685), which manufactures staleness via this very branch and must not be satisfied by a false fix that merely clears the intent. (Unanchorable: the mechanism lives on unchanged lines outside the diff's hunks.)
[Critical] R3-5: [certifies-falsely] [new-surface] workspaceReload's model-name-change branch (acpAgent.ts:~12255-12266) calls raw config.switchModel(authType, newModelName) and never re-applies the persisted model.reasoningEffort: 'none' — unlike the PR-added sibling branch (which uses refreshAuthWithPersistedReasoning) and unlike Session.setModel, whose post-switch reconcile re-applies a compatible persisted selection after the same core switchModel call; applyResolvedModelDefaults overwrites every MODEL_GENERATION_CONFIG_FIELDS entry (reasoning included). A user who persisted 'none' and then changes model.name in settings.json gets a workspace reload that wipes the disable: the session silently runs with thinking ON while settings still say 'none', until an unrelated replay fires; the identical switch via setSessionConfigOption reconciles correctly. Witness: probe (booted QwenAgent, live generation.reasoning=false, settings reload flipping model.name with reasoningEffort 'none' persisted): branch taken — switchModel called — then AssertionError: expected undefined to be false (disable wiped, no replay); replaying via refreshAuthWithPersistedReasoning after the switch passes the probe. Fix: reuse the replay half of refreshAuthWithPersistedReasoning (persisted-none check + support gate + applyReasoningSelection(config, REASONING_EFFORT_NONE)) after await config.switchModel(authType, newModelName) succeeds in that branch, keeping the support gate so 'none' is never replayed onto a thinking-mandatory model. Fix witness: extend the acpAgent.test.ts workspaceReload suite — merged settings { model: { name: 'next-model', reasoningEffort: 'none' } }, live reasoning: false; after workspaceReload assert the session's generation config still has reasoning === false on the new model, then remove the post-switchModel replay and confirm the new test reds. (Unanchorable: the model-change branch is pre-existing code outside the diff's hunks.)
— qwen3.8-max via Qwen Code /review (v0.22.3)
| private async ensureAuthenticated( | ||
| config: Config, | ||
| settings: LoadedSettings, | ||
| ): Promise<void> { |
There was a problem hiding this comment.
[Critical] R3-1: packages/cli no longer compiles. This PR changed ensureAuthenticated to require (config: Config, settings: LoadedSettings) and updated 7 of the 8 call sites, but missed await this.ensureAuthenticated(config); at acpAgent.ts:13155 — inside the pre-publish model-provider reload loop of createAndStoreSession that arrived via the merge of main (#10269). Every build/bundle of the CLI package fails with TS2554, and no test suite can run until it is fixed. Pass the settings parameter that is already in scope there, mirroring the sibling call this PR updated at acpAgent.ts:13079 — it also gives this pre-publish auth refresh the same persisted-none replay as the other call sites.
Witness:
npm run build --workspace="packages/cli" → exit 1
src/acp-integration/acpAgent.ts(13155,24): error TS2554: Expected 2 arguments, but got 1.
Fix (acpAgent.ts:13155):
await this.ensureAuthenticated(config, settings);Fix witness: the deterministic gate is the build itself — npm run build --workspace="packages/cli" is red with TS2554 at acpAgent.ts:13155 without the one-token fix and must go green with it; please confirm the build passes after applying it.
中文说明
packages/cli 已无法编译。本 PR 将 ensureAuthenticated 改为必须传入 (config: Config, settings: LoadedSettings)，更新了 8 个调用点中的 7 个，但遗漏了 acpAgent.ts:13155 处 createAndStoreSession 发布前 model-provider 重载循环里的 await this.ensureAuthenticated(config);（该调用点随 main 合并 (#10269) 进入）。CLI 包的每次构建/打包都会以 TS2554 失败，修复前任何测试套件都无法运行。请传入该处已在作用域内的 settings 参数，与本 PR 已更新的 acpAgent.ts:13079 兄弟调用保持一致——这同时让该发布前鉴权刷新获得与其他调用点相同的持久化 none 重放。
见证：npm run build --workspace="packages/cli" 退出码 1，报错 src/acp-integration/acpAgent.ts(13155,24): error TS2554: Expected 2 arguments, but got 1.
修复见证：确定性门控就是构建本身——不加这一个 token 的修复，npm run build --workspace="packages/cli" 在 acpAgent.ts:13155 处红；修复后应变绿，请确认后回复。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const sourceReasoningSelection = | ||
| sourceReasoningIntent?.value ?? | ||
| (sourceReasoningPreview | ||
| ? getReasoningSelection(sourceReasoningPreview) | ||
| : undefined); |
There was a problem hiding this comment.
[Critical] R1-7: [certifies-falsely] [regression] Round-1 mechanism re-checked at this head and still present: this welcome no-session branch synthesizes a {modelId, value: 'default'} reasoning intent from a PREVIEW-DERIVED selection even when the user never touched a reasoning control — sourceReasoningSelection falls back to getReasoningSelection(sourceReasoningPreview). 'default' passes ensureSessionForPrompt's gates unconditionally (reasoningPreviewSupports returns true for 'default'), the first prompt sends setReasoningEffort('default', { persist: true }), and daemon-side persistReasoningSelection('default') clears the stored preference from every writable scope. Concretely: persisted medium projecting into the welcome preview → two model clicks through a reasoning-less model and back, zero reasoning interaction → the first prompt deletes the stored medium from every scope; pre-diff no intent was synthesized and the stored value survived. The design doc's Capability reconciliation speaks of removing 'every explicit selection' — a preview-derived proxy selection is not one, and at deletion time the selection is compatible: the user ends back on the model that supports the stored tier.
Witness:
probe (App.test.tsx harness, scratch tree — welcome preview 'medium', zero reasoning interaction,
clicks through a reasoning-less model and back, first prompt):
INTACT: AssertionError: expected "spy" to not be called at all, but actually been called 1 times
1st spy call: [ "default", { "persist": true } ]
FIXED (synthesis gated on an explicit intent): probe passes, no call
Fix (gate the synthesis on an explicit selection; the daemon-side create-time reconcile still removes an unsupported persisted tier, preserving the design's end state):
setPendingReasoningIntent(
sourceReasoningIntent && keepReasoningIntent
? { modelId, value: sourceReasoningIntent.value }
: sourceReasoningIntent && !keepReasoningIntent
? { modelId, value: 'default' }
: undefined,
);Fix witness: add an App.test.tsx case — welcome preview projecting 'medium', no explicit selection, switch to a reasoning-less model and back, submit — asserting mockSessionActions.setReasoningEffort was NOT called; it goes red if the synthesis for non-explicit selections is reintroduced. The e2e discards an incompatible welcome effort when switching away and back currently pins the destructive no-interaction behavior and must be re-pinned with the fix.
中文说明
第一轮 R1-7 的机制在当前 head 复查后仍然存在：这个无会话分支即使用户从未触碰推理控件，也会从预览推导的选择合成 {modelId, value: 'default'} 推理意图——sourceReasoningSelection 回退到 getReasoningSelection(sourceReasoningPreview)。'default' 无条件通过 ensureSessionForPrompt 的门控（reasoningPreviewSupports 对 'default' 恒返回 true），首条提示发出 setReasoningEffort('default', { persist: true })，daemon 侧 persistReasoningSelection('default') 从所有可写 scope 清除已存偏好。具体路径：已存 medium 投射到 Welcome 预览 → 零推理交互地切换到一个无推理控件的模型再切回 → 首条提示把已存的 medium 从所有 scope 删除；PR 前不会合成意图，已存值得以保留。设计文档的 Capability reconciliation 写的是移除 "every explicit selection"——预览推导的代理选择不属于此列，且删除时该选择是兼容的：用户最终停在支持该档位的模型上。
见证（探针，临时树）：INTACT AssertionError: expected "spy" to not be called at all, but actually been called 1 times — 1st spy call: [ "default", { "persist": true } ]；将合成门控为仅显式意图后探针通过、无调用。
修复见证：新增 App.test.tsx 用例——Welcome 预览投射 'medium'、无显式选择、切到无推理模型再切回、提交——断言 mockSessionActions.setReasoningEffort 未被调用；重新引入非显式合成时该测试变红。e2e discards an incompatible welcome effort when switching away and back 目前钉住了这一无交互破坏行为，需随修复重新钉住。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const persistDefault = | ||
| !this.requiresManagedConversationBinding && | ||
| (options.persistDefault ?? true); |
There was a problem hiding this comment.
[Critical] R3-2: [certifies-falsely] [new-surface] A session-scoped model switch (persistDefault: false) still permanently deletes the stored reasoning preference, crossing the operation's own persistence boundary. setModel passes persist: !this.requiresManagedConversationBinding to reconcileReasoningSelection (three lines below), ignoring the just-computed persistDefault. The only production caller passing { persistDefault: false } is the ACP setSessionConfigOption 'model' case (acpAgent.ts:5711-5718) — a session-scoped model change from any ACP host. With persisted model.reasoningEffort: 'max' valid for the user's persisted default model, switching one session to qwen3.8-max (efforts low/medium/xhigh — 'max' unsupported) via the config option deletes 'max' from settings forever while model.name/baseUrl/selectedType are deliberately not written; switching the session back restores nothing. Two explicitly non-persisting operations destroy a durable preference the unchanged default model still supports.
Witness:
probe (real Session.setModel, scratch tree):
INTACT: AssertionError: expected undefined to be 'max'
(stored 'max' deleted by setModel(..., {persistDefault:false}); setValue never called with 'model.name')
FIXED (persist: persistDefault): 'max' kept — and the PR's pinning test
'still deletes an incompatible reasoning preference when the model default is not persisted'
fails, isolating this hunk as load-bearing
Fix:
this.reconcileReasoningSelection(effectiveModelId, {
persist: persistDefault,
allowReasoning: !isRuntime && !rawModelId.startsWith(ACP_ROUTE_ID_PREFIX),
});Fix constraint: the live DEFAULT apply must remain even when the persist clear is skipped — applyReasoningSelection(this.config, REASONING_EFFORT_DEFAULT); in the mustClear branch (Session.ts:~9991) keeps the session-scoped switch from sending an unsupported effort; the model-default write gate if (persistDefault) at Session.ts:~9904 keeps its current semantics.
Fix witness: invert the new test still deletes an incompatible reasoning preference when the model default is not persisted (Session.test.ts:5195) — with persistDefault: false, assert state.user.settings.model.reasoningEffort is still 'max' and no model.reasoningEffort setValue occurred; then remove the persist: persistDefault gate and confirm the test reds.
中文说明
会话级（persistDefault: false）模型切换仍会永久删除已存的推理偏好，越过了该操作自身的持久化边界。setModel 向 reconcileReasoningSelection（下方三行）传入 persist: !this.requiresManagedConversationBinding，忽略了刚计算出的 persistDefault。唯一传 { persistDefault: false } 的生产调用方是 ACP setSessionConfigOption 的 'model' 分支（acpAgent.ts:5711-5718）——任意 ACP 宿主发起的会话级模型变更。用户已存 model.reasoningEffort: 'max'（对其持久化默认模型有效）时，把单个会话经 config option 切到 qwen3.8-max（efforts 为 low/medium/xhigh，不支持 'max'）会从设置中永久删除 'max'，而 model.name/baseUrl/selectedType 刻意不写；再切回也不恢复任何东西。两个明确不持久化的操作摧毁了一个未变更默认模型仍然支持的持久偏好。
见证（探针，真实 Session.setModel，临时树）：INTACT AssertionError: expected undefined to be 'max'（setModel(..., {persistDefault:false}) 删除已存 'max'，setValue 从未以 'model.name' 调用）；改为 persist: persistDefault 后 'max' 保留，且 PR 的钉住测试 'still deletes an incompatible reasoning preference when the model default is not persisted' 变红——证明该 hunk 是承重行为。
修复约束：跳过持久化清除时 live 的 DEFAULT 应用必须保留——mustClear 分支的 applyReasoningSelection(this.config, REASONING_EFFORT_DEFAULT);（Session.ts:~9991）保证会话级切换不会发送不支持的 effort；Session.ts:~9904 的模型默认写入门控 if (persistDefault) 保持现有语义。
修复见证：把新测试 still deletes an incompatible reasoning preference when the model default is not persisted（Session.test.ts:5195）反转——persistDefault: false 时断言 state.user.settings.model.reasoningEffort 仍为 'max' 且没有发生 model.reasoningEffort 的 setValue；随后移除 persist: persistDefault 门控并确认测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| (value: ReasoningSelection) => | ||
| actions | ||
| .setReasoningEffort(value) | ||
| .setReasoningEffort(value, { persist: true }) |
There was a problem hiding this comment.
[Critical] R3-3: [fails-closed] [regression] Reasoning controls are permanently broken for managed-conversation sessions. Every reasoning selection from the active-session UI now demands persistence (this call site and App.tsx handleReasoningEffort both send { persist: true }), but this PR's new daemon guard makes persistence impossible for managed-conversation (sourceType: 'standalone') sessions — Session.persistReasoningSelection throws Reasoning selection cannot be persisted for this session. The daemon handler live-applies the selection first (acpAgent.ts:~5780) and only then persists (~5804-5806), so the RPC fails AFTER the live mutation: the user sees reasoning.updateFailed, the chip keeps the stale state while the session actually generates with the changed setting, and every retry fails identically. Pre-PR the same controls applied live without persisting. Reachability: Web Shell standalone sessions created via DaemonSessionProvider with sessionContext.kind === 'standalone' (the daemon creates those with sourceType 'standalone' → requiresManagedConversationBinding); the bundled browser app uses workspace contexts, so the affected class is integrations of the exported DaemonSessionProvider/daemon-react-sdk API.
Witness:
probe (Session constructed with getSessionSourceType() === 'standalone', scratch tree):
persistReasoningSelection('medium') threw 'Reasoning selection cannot be persisted for this session'
ordering quoted from source: live apply at acpAgent.ts:~5780 precedes persist at ~5804-5806
Fix: don't make persistence a hard requirement of the live change for sessions that cannot persist — e.g. have the daemon apply live and return persisted: false with a _meta marker naming the managed-conversation refusal instead of throwing after applying, and relax actions.setReasoningEffort's opts?.persist && result.persisted !== true check to accept that marker; or expose a 'cannot persist reasoning' capability so ChatPane/App omit persist for such sessions. Either way the live apply must not be reported as a failure.
Fix constraint: managed-conversation sessions must never write model.reasoningEffort into settings scopes — the guard is deliberate (Session.ts:9941-9945), matching reconcileReasoningSelection's persist: !this.requiresManagedConversationBinding (Session.ts:9858).
Fix witness: add an actions.test.ts case whose setConfigOption reports the managed-conversation refusal (no persisted: true) — setReasoningEffort('medium', { persist: true }) must still resolve and update connection.reasoning (live apply surfaced) rather than rejecting; removing the relaxation reds it.
中文说明
managed-conversation 会话的推理控件被永久性破坏。活跃会话 UI 的每次推理选择现在都要求持久化（本调用点与 App.tsx handleReasoningEffort 均发送 { persist: true }），但本 PR 新增的 daemon 守卫使 managed-conversation（sourceType: 'standalone'）会话无法持久化——Session.persistReasoningSelection 抛出 Reasoning selection cannot be persisted for this session。daemon 处理器先 live 应用（acpAgent.ts:~5780）再持久化（~5804-5806），因此 RPC 在 live 变更之后才失败：用户看到 reasoning.updateFailed，界面保持旧状态而会话实际已按变更后的设置生成，且每次重试都以同样方式失败。PR 前同样的控件只做 live 应用、不持久化。可达性：经 DaemonSessionProvider 以 sessionContext.kind === 'standalone' 创建的 Web Shell standalone 会话（daemon 以 sourceType 'standalone' 创建 → requiresManagedConversationBinding）；内置浏览器应用使用 workspace 上下文，受影响的是导出组件 DaemonSessionProvider/daemon-react-sdk API 的集成方。
见证（探针，临时树）：以 getSessionSourceType() === 'standalone' 构造的 Session 调用 persistReasoningSelection('medium') 抛出 'Reasoning selection cannot be persisted for this session'；顺序引自源码：live 应用（acpAgent.ts:~5780）先于持久化（~5804-5806）。
修复：不要把持久化作为不可持久化会话 live 变更的硬性前提——例如 daemon 先 live 应用并返回 persisted: false 加一个说明 managed-conversation 拒绝的 _meta 标记，而不是应用后再抛错；同时放宽 actions.setReasoningEffort 的 opts?.persist && result.persisted !== true 检查以接受该标记；或暴露 'cannot persist reasoning' 能力，让 ChatPane/App 对此类会话省略 persist。无论如何，live 应用成功不得被报告为失败。
修复约束：managed-conversation 会话绝不能把 model.reasoningEffort 写入设置 scope——该守卫是刻意的（Session.ts:9941-9945），与 reconcileReasoningSelection 的 persist: !this.requiresManagedConversationBinding（Session.ts:9858）一致。
修复见证：新增 actions.test.ts 用例，setConfigOption 报告 managed-conversation 拒绝（无 persisted: true）——setReasoningEffort('medium', { persist: true }) 仍应 resolve 并更新 connection.reasoning（live 应用被呈现）而不是 reject；移除放宽逻辑后该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| activeRuntimeSnapshot || | ||
| currentModelId.startsWith(ACP_ROUTE_ID_PREFIX) || | ||
| !isReasoningSelectionSupported( | ||
| rawCurrentModelId, | ||
| REASONING_EFFORT_DEFAULT, | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R3-8: For unconfigured qwen-family models, this gate hides every reasoning control and rejects every value — including 'default', the one that would clear the override — while the CLI seed path and TUI /effort still apply and persist tiers for exactly those models, model-independently. A user who runs /effort high in the TUI while a generic qwen model (not in MODEL_CONFIGURATIONS) is active persists the tier (effort-command has no model gate); every later session seeds generationConfig.reasoning = { effort: 'high' } (modelConfigUtils.ts:447-451, no family check) and emits high-effort thinking on the wire — while WebShell offers no control to inspect or clear it. Pre-PR the generic ladder was shown for all models and 'default' cleared the override; the design doc's capability policy ('a model without reasoning controls removes every explicit selection') is enforced on model switch but not at session start.
Witness:
probe (same commit):
resolveCliGenerationConfig with settings { name: 'qwen-plus', reasoningEffort: 'high' }
→ generationConfig.reasoning = { effort: 'high' }
PR's own test 'does not guess reasoning capabilities for an unregistered Qwen model' passes
(option undefined + 'high' rejected) — both halves of the inconsistency observed together
Fix: align the two stances — either gate the seed (skip seeding the persisted tier and warn when !isReasoningSelectionSupported(modelId, selection, thinkingMandatory)), or keep offering 'default' in setSessionConfigOption as a reset for these models so the override is clearable.
中文说明
对未配置的 qwen 系模型，该门控隐藏所有推理控件并拒绝每一个值——包括唯一能清除覆盖的 'default'——而 CLI 种子路径与 TUI /effort 仍会为这些模型按模型无关的方式应用并持久化档位。用户在通用 qwen 模型（不在 MODEL_CONFIGURATIONS 中）处于激活状态时于 TUI 执行 /effort high，档位即被持久化（effort-command 无模型门控）；之后每个会话都会播种 generationConfig.reasoning = { effort: 'high' }（modelConfigUtils.ts:447-451，无家族检查）并在线上发出高强度思考——而 WebShell 没有任何控件可以查看或清除它。PR 前通用档位阶梯对所有模型展示，'default' 可清除覆盖；设计文档的能力策略（"无推理控件的模型移除所有显式选择"）只在模型切换时执行，会话启动时不执行。
见证（探针，同一提交）：resolveCliGenerationConfig 在设置 { name: 'qwen-plus', reasoningEffort: 'high' } 下产生 generationConfig.reasoning = { effort: 'high' }；同时 PR 自己的测试 'does not guess reasoning capabilities for an unregistered Qwen model' 通过（option 为 undefined 且 'high' 被拒绝）——不一致的两半在同一提交上同时观察到。
修复：统一两种立场——要么门控种子（当 !isReasoningSelectionSupported(modelId, selection, thinkingMandatory) 时跳过播种并告警），要么在 setSessionConfigOption 中为这些模型保留 'default' 作为复位，使覆盖可被清除。
— qwen3.8-max via Qwen Code /review (v0.22.3)
5 participants
What this PR does
This PR persists the WebShell model and reasoning preference across daemon sessions by reusing the existing
model.reasoningEffortsetting. An absent value means the model default,nonemeans thinking is disabled, and the five fixed effort tiers mean thinking is enabled at that tier.defaultis only a daemon command that clears the stored value and restores the model default; it is neither stored nor rendered as a UI option.Welcome remains lazy: changing the model, thinking switch, or effort only updates the local preview. The first prompt creates and attaches the session, sets the model, persists the reasoning selection, and then sends the prompt. Existing and running sessions apply and persist reasoning changes immediately, with WebShell updating only after the daemon confirms the resulting options and persistence result.
Every model switch validates the current selection against the target model. Compatible tiers are retained; incompatible tiers such as
max, unsupported reasoning, toggle-only capabilities, and mandatory-thinking constraints clear the persisted value and live override without downgrading or restoring an older dormant effort. Tiered models show only real effort choices, toggle-only models show only the thinking switch, unsupported models show no reasoning section, and all fixed effort labels are rendered from WebShell English and Chinese translations.The change adds only the optional
persistflag and strict reasoning selection/result types to the daemon HTTP boundary and TypeScript SDK. It does not addmodel.reasoningEnabled, change the public ACP protocol, alter provider clamping or the Core request pipeline, or modify the Java SDK.Why it's needed
Model and reasoning choices should behave as one coherent session preference instead of depending on whether the user is on Welcome, already has messages, or currently has a request running. Persisting a capability-incompatible tier is also unsafe because it can silently leak into a later model switch. This keeps the contract small and explicit while preserving lazy session creation.
Reviewer Test Plan
How to verify
maxon a supporting model, switch to a model that does not support it, and switch back. The stored setting and live override should be cleared, the target model default should be shown, andmaxshould not be restored or downgraded.nonevalue should be removed and the model default effort should be used instead of restoring the effort that was active before disabling.none.Evidence (Before & After)
Before: the current
mainbranch does not provide the complete Welcome-to-live persistence and capability-reconciliation contract described above.After: request-ledger and DOM Playwright coverage verifies all 10 reasoning scenarios, including Welcome lazy creation, live updates, mandatory thinking, incompatible model round trips, toggle-only rendering, unsupported/older-daemon behavior, and authoritative live capabilities. Unit regression suites cover persistence, strict validation, daemon confirmation, SDK request/response shape, model switching, and English/Chinese rendering.
Tested on
Environment (optional)
Local macOS checkout with the fake daemon/request ledger and Playwright Chromium. Verified with 730 CLI Session tests, 517 ACP agent tests, 1,700 TypeScript SDK tests, 645 WebUI tests, 4,572 WebShell tests, 10 focused Playwright tests, the root build and typecheck, relevant lint checks, Prettier, and two clean diff-review passes.
Risk & Scope
Linked Issues
Supersedes #10011.
中文说明
本 PR 做了什么
本 PR 复用现有的
model.reasoningEffort设置，使 WebShell 的模型和思考偏好能够跨 daemon 会话持久化。未设置表示使用模型默认值，none表示关闭思考，五个固定强度表示开启思考并使用对应档位。default仅是 daemon 用来清除持久化值并恢复模型默认的控制命令；它不会写入配置，也不会作为 UI 选项展示。Welcome 保持懒创建：切换模型、思考开关或强度时只更新本地预览。首次发送会依次创建并挂载会话、设置模型、持久化思考选择，然后发送提示词。已有消息和运行中的会话会立即应用并持久化思考修改，WebShell 只在 daemon 确认最终选项和持久化结果后更新界面。
每次模型切换都会根据目标模型能力校验当前选择。兼容档位会保留；不兼容档位（例如
max）、不支持思考、仅开关能力以及强制思考约束都会清除持久化值和 live override，不做降级映射，也不会恢复旧的 dormant effort。分档模型只显示真实强度选项，仅开关模型只显示思考开关，不支持模型完全不显示思考区域；所有固定强度文案都由 WebShell 的中英文翻译渲染。变更只在 daemon HTTP 边界和 TypeScript SDK 增加可选
persist参数以及严格的思考选择/结果类型。它不会新增model.reasoningEnabled，不会修改公共 ACP 协议、provider clamp、Core 请求管线或 Java SDK。为什么需要
模型和思考选择应该作为一组一致的会话偏好，而不应因用户位于 Welcome、已有消息或请求运行中而行为不同。持久化与目标能力不兼容的档位还可能在之后切换模型时错误泄漏。本实现保持契约小而明确，同时保留会话懒创建。
Reviewer 测试计划
验证方式
max，切换到不支持它的模型，再切回原模型。磁盘设置和 live override 应被清除，界面应显示目标模型默认值，且不会恢复或降级max。none应被删除，并使用模型默认强度，而不是恢复关闭前的档位。none。前后证据
之前：当前
main分支不具备上述从 Welcome 到 live session 的完整持久化和能力校验契约。之后：request-ledger 与 DOM Playwright 覆盖全部 10 个思考场景，包括 Welcome 懒创建、live 修改、强制思考、不兼容模型往返、仅开关渲染、不支持/旧 daemon 行为和 live capability 权威性。单元回归覆盖持久化、严格校验、daemon 确认、SDK 请求响应、模型切换以及中英文渲染。
测试平台
环境（可选）
本地 macOS checkout，使用 fake daemon/request ledger 和 Playwright Chromium。已通过 730 条 CLI Session 测试、517 条 ACP agent 测试、1,700 条 TypeScript SDK 测试、645 条 WebUI 测试、4,572 条 WebShell 测试、10 条 focused Playwright、根目录 build 与 typecheck、相关 lint、Prettier 以及两轮完整 diff 净审。
风险与范围
关联问题
替代 #10011。