fix(web-shell): Load models for fresh standalone sessions - #10719
fix(web-shell): Load models for fresh standalone sessions#10719doudouOUC wants to merge 4 commits into
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E test reportEnvironment: macOS, Node.js 22.22.3, isolated
An additional smoke test against the current local configuration showed |
|
Thanks for the PR — the writeup is thorough and the committed design doc is appreciated.
Moving on to code review. 🔍 中文说明感谢贡献!PR 描述详尽,还附带了已提交的设计文档。
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review — one blocking finding: the new capability is registered in code but missing from the docs its contract test pins, and that test is what just turned CI red. The implementation review from the previous pass still stands at this commit — the endpoint hygiene, SDK validation, mapper reuse, atomic create, and failure semantics all checked out, and I still don't see a materially simpler path. One update and one addition below. First, withdrawing the earlier nit: the author's pushback is right — the keyboard-handler site reads the raw Blocking —
This test is green on The deferred-hydration + atomic-create flow (unchanged from last pass): sequenceDiagram
participant P1 as Web Shell session provider
participant P2 as SDK DaemonClient
participant P3 as Daemon standalone session-options route
participant P4 as Internal Conversations runtime
Note over P1: page opens, session creation deferred
P1->>P2: getStandaloneSessionOptions()
P2->>P3: capability-gated GET, no query input accepted
P3->>P4: activity lease, exact-root and currency checks
P4-->>P3: provider status with workspace path and ACP state redacted
P3-->>P2: options v 1
P2-->>P1: validated options
Note over P1: hydrate models, picker becomes visible
P1->>P3: first prompt creates session carrying selected modelServiceId
Note over P1: post-attach model switch skipped
Files changed (24)
Test evidence — the PR's own CI, read via the API (per gate rules this review never builds or executes PR-derived code). CI has now settled on the reviewed commit: one red check,
Everything else is green: integration tests (no-AK, no sandbox), Serve A/B (12 scenarios, no response changes vs base), web-shell E2E smoke, web-shell visuals, desktop/Live Host builds, the Java compatibility matrix, dependency CVE audit, and the secret scan. The windows/macos unit-matrix entries report skipped. CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle this: 中文说明代码审查——发现一个阻塞问题:新能力已在代码中注册,但其契约测试所钉住的两份文档未同步更新,该测试正是 CI 变红的原因。 上一轮的代码审查结论在该提交上依然成立——接口卫生、SDK 校验、mapper 复用、原子创建与失败语义均核实无误,也没有找到更简路径。两点更新如下。 先撤回之前的小建议:作者的反驳是对的——键盘处理处读取的是原始 阻塞——
该测试在 测试证据来自 PR 自身的 CI(通过 API 读取;按门禁规则,本审查从不构建或执行 PR 代码)。该提交的 CI 已落定:一个红色检查
其余全部为绿:集成测试(no-AK、无沙箱)、Serve A/B(12 场景、与 base 无响应差异)、web-shell E2E smoke、web-shell visuals、桌面壳/Live Host 构建、Java 兼容矩阵、依赖 CVE 审计与密钥扫描。windows/macos 单元测试矩阵项为 skipped。 沙箱验证可以一锤定音: — Qwen Code · qwen3.8-max Reviewed at |
🖼️ 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 3 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 |
|
Confidence: 2/5 — the design and implementation held up again under this re-run, but CI settled red on a failure this PR introduces, so it cannot merge as-is. Stepping back: nothing changed my read of the code — the problem is real, the approach is the minimal shape for each half of it, the failure semantics match the design doc, and the tests pin what matters. The previous pass deferred approval only on CI; CI has now landed, and it says no. The blocker is mechanical, and it is this PR's own: One caveat so the next CI run doesn't cause confusion: even after the docs fix, the web-shell suite will still fail on @doudouOUC requesting changes for the docs sync — everything else from the earlier review stands. 中文说明回顾整体:代码层面的结论在本次复查中没有变化——问题真实、方案是每一半所需的最小形态、失败语义与设计文档一致、测试钉住了关键行为。上一轮只是等 CI,现在 CI 落定了,结果是红。 阻塞点是机械性的,且由本 PR 自身引入: 一个提醒,避免下一轮 CI 造成困惑:即便修复文档, @doudouOUC 因文档同步问题请求修改——此前评审的其他结论保持不变。 — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "standalone_session_options_v1" |
— Qwen Code · serve A/B
|
Thanks — won't take the nit. The standalone/no-session/no-models guard appears three times, but the keyboard-handler site reads |
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. |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 660 passed · 0 failed · 660 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:660 通过 · 0 失败 · 660 总计 抖动门: Verification reportVerification report — PR 10719
|
| cell | environment | oracle | result |
|---|---|---|---|
| head capability | daemon @ head | GET /capabilities features |
standalone_session_options_v1 present |
| head options | daemon @ head | GET /standalone/session-options |
200, v=1, initialized=true, providers populated, current.modelId set |
| head redaction | daemon @ head | response body keys/text | no workspaceCwd, no acpChannelLive, no Conversations path leak |
| head no-side-effect | daemon @ head, fresh | GET /standalone/sessions before/after options read |
0 → 0 |
| head input guard | daemon @ head | ?cwd=, ?workspaceCwd=, ?bogus= |
400 each |
| head atomic create | daemon @ head | POST /standalone/sessions with modelServiceId |
200, sessionId echoed, list 0 → 1 |
| base capability | daemon @ base | GET /capabilities |
feature absent |
| base options | daemon @ base | GET /standalone/session-options |
404 (the A/B flip) |
| base parity | daemon @ base | GET /standalone/sessions, create with modelServiceId |
intact (200; create works) |
Parity corroboration: on the head daemon, /standalone/session-options returned the exact GET /workspace/providers payload minus precisely workspaceCwd and acpChannelLive — the endpoint reads the runtime's provider status and redacts only the internals, as designed.
Secondary claim 2, wire oracle (03-sdk-wire-oracle.png, logs/sdk-wire.log): the real built DaemonClient against the real base daemon throws DaemonCapabilityMissingError for standalone_session_options_v1; against a decoy peer that advertises no capability but happily serves the route, the SDK throws with 0 route hits (gate checked before the call). 10 malformed-payload probes (leaked workspaceCwd/acpChannelLive, v:2, null providers, wrong kind, string isCurrent, zero contextLimit, bogus approvalMode, non-array errors, numeric initialized) each reject with the specific DaemonStandaloneProtocolError; the valid payload round-trips. 15/15.
Reviewer Test Plan walk (browser E2E, logs/e2e-webshell.log, screenshots 04–08):
| plan step | result |
|---|---|
fresh /?context=standalone shows current model + list before any session |
PASS — 1× GET /standalone/session-options, 0× POST /standalone/sessions; toolbar chip "Select Model: Qwen 3.8 Max · Extra High"; picker lists the model (04, 05) |
open + close picker, no POST /standalone/sessions |
PASS — 0 creates through the cycle |
first prompt: single create with stable id, selected modelServiceId, approval mode, no cwd/workspaceCwd, no follow-up switch |
PASS — exactly 1 create {"sessionId":"9f6b048f-…","modelServiceId":"qwen3.8-max(openai)","approvalMode":"auto"}; 0× POST /session/:id/model (06) |
| older daemon: picker hidden, create uses daemon default | PASS — head UI vs base daemon: 0 option calls, picker absent (07), create body without modelServiceId (08) |
Bundle-size claim: measured head dist/daemon/index.js = 224,830 B; base rebuilt with the identical esbuild step = 222,324 B; Δ = 2,506 B — all three numbers exactly as stated in the PR body (warning threshold 221,184 B is pre-existing and warn-only).
Findings (non-blocking observations)
- Environment, not a defect: the container's default catalog is composer-hidden. With no
modelProviderssettings, the daemon's only listed model iscoder-model(qwen-oauth), which the pre-existingHIDDEN_COMPOSER_MODEL_IDSfilter removes from the picker — so a fresh page correctly hid the picker (availableModels.length === 0), same as the attached-session flow would. After configuring a visiblemodelProviders.openaientry, every picker cell passed. The PR's hydration and the pre-existing visibility rule compose correctly; no action needed. - Prompt completion not verified (upstream quota). The shared OpenAI-compatible upstream returned
insufficient_quotafor chat completions, so the first prompt's response could not complete in E2E. The PR's changed surface — create orchestration, model-at-create, no post-attach switch — was fully observed before any model call; the quota failure is environmental and would affect base and head identically. - SDK browser bundle exceeds the soft warning by design. +2,506 B over the pre-existing 221,184 B warning threshold (warn-only, author-disclosed). Informational; the hard caps (transports 48 KiB, transcript 192 KiB) are untouched and pass.
Mutation matrix (vacuity)
Each guard the PR introduces was reverted in place; its suite must go red with the intended assertion. All 10 killed; 0 survivors; each kill doubles as the positive control proving its suite can fail (09-mutation-matrix.png).
| # | guard | suite | result |
|---|---|---|---|
| M1 | runtime-ownership check in getOptions() |
standalone-session-service.test.ts | KILLED — "promise resolved … instead of rejecting" |
| M2 | route query-param rejection | standalone-sessions.test.ts | KILLED — "expected 200 to be 400" |
| M3 | SDK reject of leaked workspaceCwd/acpChannelLive |
DaemonClientStandalone.test.ts | KILLED — only the 2 leak cases red, 3 sibling cases green (exact attribution) |
| M5 | models hydration from options |
DaemonSessionProvider.test.tsx | KILLED — "expected undefined to deeply equal […]" |
| M6 | skip setModel for standalone |
sessionPreparation.test.ts | KILLED — "expected spy to not be called at all" |
| M7 | persist:false reasoning for standalone |
sessionPreparation.test.ts | KILLED — - persist: false / + persist: true |
| M8 | reject per-call modelServiceId for workspace create |
actions.test.ts | KILLED — guard-message assertion fails |
| M9 | hide composer model action when no models | App.test.tsx | KILLED — "expected [ Array(7) ] to not include 'model'" |
| M10 | stale-options guard after context switch | DaemonSessionProvider.test.tsx | KILLED — stale publish matched-object failure |
| M11 | capability gate before fetching options | DaemonSessionProvider.test.tsx (pre-existing test) | KILLED — "expected [] to be undefined" |
Redaction is defended in depth (service allowlist M-return shape + SDK reject M3); reverting either end alone is caught at its own layer, so no combination row is needed to prove the set load-bearing.
Targeted gates (PR's own CI covers the full suites)
| suite | result |
|---|---|
cli standalone-session-service.test.ts + routes/standalone-sessions.test.ts |
116/116 |
cli server.test.ts (feature-list order, CONDITIONAL_SERVE_FEATURES drift, live-runtime capability advertisement) |
3/3 (1163 skipped by filter) |
sdk DaemonClientStandalone + daemon-public-surface |
44/44 |
web-shell DaemonSessionProvider + actions + sessionPreparation |
441/441 |
web-shell App.test.tsx (picker-hide + standalone-draft create) |
2/2 (655 skipped by filter) |
Not covered
- Full
server.test.ts(1166) andApp.test.tsx(657) runs — targeted-tselections only; the PR's CI runs the complete suites. - First-prompt response content (upstream quota, see Finding 2) — this E2E reproduces the PR's orchestration end to end but not a successful model completion.
- Windows/macOS browsers; workspace, Live, Recents, uploads, channel flows (author-declared out of scope).
- Base worktree typecheck: a fresh worktree cannot resolve per-package nested
node_modules/gitignored generated files the CI install provides; the base control was emitted withtsc --build --noCheck(TS emit is independent of type errors) plus the repo's owngenerate-git-commit-info.js, and verified pre-PR by string checks. Environmental, proven by the identical failure mode being unrelated to any PR-touched file. - Per-commit attribution: single commit, reachable locally; not applicable.
Methodology
Environment: CI verify container (node:22-bookworm), builds pre-installed at HEAD; scratch worktree under tmp/ (removed after capture). Head daemon: node packages/cli/dist/index.js serve --bare --safe-mode --port 0 with isolated HOME and loopback OpenAI-compatible proxy env; base daemon: same launcher against the base worktree's rebuilt CLI. Harnesses (harness/*.mjs) are mock-free: real HTTP against real daemons, real built SDK DaemonClient, real Chromium (Playwright, repo-pinned browsers) driving the Web Shell served by each daemon; malformed-payload cells use a loopback fake peer. Every assertion is a scripted comparison in the harnesses; raw per-run logs in logs/, captures in evidence/. Assertion totals count only executed checks; expected reds (mutation kills, base-arm 404) are encoded as passes.
Flakiness gate log
rounds=5 files=9 skipped=0
file packages/cli/src/serve/conversations/standalone-session-service.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/standalone-session-service.test.ts
file packages/cli/src/serve/routes/standalone-sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/standalone-sessions.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/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClientStandalone.test.ts
file packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemon-public-surface.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/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/actions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/actions.test.ts
file packages/web-shell/client/utils/sessionPreparation.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/sessionPreparation.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/conversations/standalone-session-service.test.ts: PP
packages/cli/src/serve/routes/standalone-sessions.test.ts: PP
packages/cli/src/serve/server.test.ts: PP
packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: PP
packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: PP
packages/web-shell/client/App.test.tsx: PP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P
packages/web-shell/client/daemon/session/actions.test.ts: P
packages/web-shell/client/utils/sessionPreparation.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/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
Evidence images
1 additional image(s) did not pass the hosting checks (PNG magic, unique sanitized name, ≤2 MB, max 8) and remain in the run artifacts.
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
CI failure breakdown + follow-ups (self-review notes) The red
Non-blocking nits from a full read-through, for the record:
中文说明
非阻塞记录项见上方英文列表:三处内联条件的漂移风险(其中键盘处理器用了未过滤的模型列表)、 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs the docs sync before this can merge — the new standalone_session_options_v1 capability is registered in code but missing from the two doc surfaces capabilities-docs-contract.test.ts pins (the conditional-serve-features table in docs/developers/qwen-serve-protocol.md and the tag counts in docs/developers/daemon/00-index.md), which is what turned CI red. Details and the exact fix are in my Stage 2 comment. Everything else still reads well — this is the only thing standing between this PR and green.
中文说明
合并前需要补齐文档同步——新能力 standalone_session_options_v1 已在代码中注册,但 capabilities-docs-contract.test.ts 钉住的两份文档(docs/developers/qwen-serve-protocol.md 的 conditional-serve-features 表格与 docs/developers/daemon/00-index.md 的计数)未更新,这正是 CI 变红的原因。具体修复方式见我的 Stage 2 评论。其余部分依然没有问题——这是唯一的阻塞项。
— Qwen Code · qwen3.8-max
|
CI follow-up on the
|
|
@qwen-code /triage |
chiga0
left a comment
There was a problem hiding this comment.
LGTM — no blockers.
Reviewed at 41d74d80.
What I checked
Route & service layer (standalone-session-service.ts, standalone-sessions.ts):
GET /standalone/session-options correctly calls ensureRuntime → runRuntimeActivity → assertExactRoot → getWorkspaceProvidersStatus and then strips workspaceCwd/acpChannelLive from the returned object. The ownership sanity check (status.workspaceCwd !== runtime.workspaceCwd) guards against a race where the runtime switches between calls. Query-param rejection (400 on unknown fields) is clean.
Contract symmetry (class 1):
Server emits Omit<ServeWorkspaceProvidersStatus, "workspaceCwd"|"acpChannelLive"> → SDK parseStandaloneSessionOptions accepts exactly Pick<..., "v"|"initialized"|"current"|"approvalMode"|"providers"|"errors"> and throws if workspaceCwd or acpChannelLive appear. Both ends strip the same two fields. Verified in DaemonClientStandalone.test.ts ("rejects malformed standalone options: workspace internals").
Capability gating (class 2):
STANDALONE_SESSION_OPTIONS_CAPABILITY is exported from both barrel files (src/daemon/index.ts and src/index.ts). getStandaloneSessionOptions() calls requireCapability before the HTTP call. The test ("gates standalone options with their dedicated capability") confirms a missing capability produces DaemonCapabilityMissingError without hitting the route. server.test.ts pins the feature under session_export and verifies standaloneSessionsAvailable toggling.
Error handling (class 3):
getStandaloneSessionOptions() failure in the deferred-connect path is caught, logged (console.warn), and standaloneOptions stays undefined; mapProviderStatus(undefined) returns { models: [] }, so models is set to undefined in the connection state — matching the pre-PR behavior for that path. Test "keeps a standalone draft usable when its options request fails" confirms the connection reaches status: connected even on failure.
State race (class 7):
if (disposed || abort.signal.aborted) return is placed immediately after the await getStandaloneSessionOptions() call, before setConnection. Test "does not publish stale standalone options after switching contexts" exercises this guard: options that resolve after the context switches to workspace do not clobber the new connection state.
modelServiceId threading (class 8):
sessionPreparation.ts seeds modelServiceId in the standalone create options and skips the setModel follow-up (if (modelId && sessionContext?.kind !== "standalone")). reasoning effort persist: false for standalone is also correct — ephemeral sessions should not persist effort. actions.ts guards: passing modelServiceId for a non-standalone context throws immediately before any RPC. The App.tsx test at line 25332 confirms setModel is never called after a standalone create with a model.
UX guards (App.tsx):
Model toolbar action is filtered out when !sessionId && kind === "standalone" && availableModels.length === 0. The keyboard shortcut and footer button also show a toast instead of opening an empty picker. Test "hides the empty model picker until standalone options are available" confirms the picker reappears after models is populated.
Bot review cross-check:
qwen-code-ci-bot CHANGES_REQUESTED the missing docs (qwen-serve-protocol.md conditional-serve-features table and 00-index.md tag counts). Both are updated in this head: standalone_session_options_v1 added to the feature block, counts updated 147→148 / 41→42. Issue resolved.
Coverage gaps:
None material. All applicable taxonomy classes were either verified clean or covered by existing tests.
|
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: 2537 passed · 0 failed · 2537 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:2537 通过 · 0 失败 · 2537 总计 抖动门: Verification reportVerdict: 中文摘要
Follow-up round: previous-finding status
Delta since round 1: one docs-only commit ( ScopeCentral claim: a fresh standalone Web Shell page loads the model catalog before any session exists, via the capability-gated read-only Secondary claims: (1) the endpoint is read-only, rejects any query input, redacts the internal Conversations path/ACP state, and fails closed on runtime-ownership mismatch; (2) old daemons and failed option reads keep the old behavior (picker empty, daemon default at create) because the SDK gates on Central claim — A/B against the base build (re-run at new base
|
| cell | environment | oracle | result |
|---|---|---|---|
| head capability | daemon @ head | GET /capabilities features |
standalone_session_options_v1 present |
| head options | daemon @ head | GET /standalone/session-options |
200, v=1, initialized=true, providers array |
| head catalog | daemon @ head, configured modelProviders.openai |
provider cell models | verify-model-a(openai), verify-model-b(openai) listed |
| head redaction | daemon @ head | body keys + serialized text | no workspaceCwd, no acpChannelLive, no absolute path |
| head no-side-effect | daemon @ head, fresh | HOME tree snapshot + GET /standalone/sessions before/after |
identical tree; 0 → 0 sessions |
| head input guard | daemon @ head | ?cwd=, ?workspaceCwd=, ?bogus= |
400 each |
| head parity | daemon @ head | options vs GET /workspace/providers |
equal minus exactly {workspaceCwd, acpChannelLive} |
| head atomic create | daemon @ head | POST /standalone/sessions with modelServiceId |
200, sessionId echoed, list 0 → 1 |
| base capability | daemon @ base | GET /capabilities |
feature absent |
| base options | daemon @ base | GET /standalone/session-options |
404 (the A/B flip) |
| capability delta | both | features set diff | exactly {standalone_session_options_v1} added, nothing removed |
| base parity | daemon @ base | list + create with modelServiceId |
intact (200; 0 → 1) |
32/32 (run twice, plus the captured run).
Wire oracle — real SDK against real daemons and decoy peers (02-sdk-wire-oracle.png)
Real built DaemonClient (head SDK dist): vs the real base daemon it rejects with DaemonCapabilityMissingError naming standalone_session_options_v1; vs the real head daemon it resolves with v=1 + providers. Against a decoy peer that advertises no capability but happily serves the route, the SDK rejects with 0 route hits (gate checked before the call). Against a decoy advertising the capability, 10 malformed payloads (leaked workspaceCwd/acpChannelLive, v:2, null providers, wrong kind, string isCurrent, zero contextLimit, bogus approvalMode, non-array errors, numeric initialized) each reject with the specific DaemonStandaloneProtocolError, and the valid payload round-trips byte-identical. 18/18.
Reviewer Test Plan walk — browser E2E (06–10)
Real Chromium (lane-pinned Playwright) against real daemons serving the built Web Shell; request/response traffic captured per page.
| plan step | result |
|---|---|
fresh /?context=standalone shows current model + list before any session |
PASS — exactly 1× GET /standalone/session-options (200), 0× POST /standalone/sessions; chip shows qwen3.8-max (06-e2e-head-fresh-standalone.png) |
| open picker: both configured models listed; selecting creates no session | PASS — picker lists Verify Model A/B; selection updates; 0 creates (07-e2e-head-picker-open.png) |
first prompt: single create with stable id, selected modelServiceId, approval mode, no cwd/workspaceCwd, no follow-up switch |
PASS — exactly 1 create {"sessionId":"<uuid>","modelServiceId":"verify-model-b(openai)","approvalMode":"auto"} → 200; 0× POST /session/:id/model; turn completed with a real assistant response (08, 10) |
| older daemon: picker stays empty, create uses daemon default | PASS — head UI @ base daemon: 0 option calls, no model chip (09-e2e-base-picker-empty.png), create body without modelServiceId |
17/17.
Mutation matrix (vacuity) at new head (03-mutation-matrix.png)
Each guard reverted in place; its suite must go red with the intended assertion. Controls (unmutated) green first. 17/17 (7 controls + 10 kills), 0 survivors.
| # | guard | suite | result |
|---|---|---|---|
| M1 | runtime-ownership check in getOptions() |
standalone-session-service.test.ts | KILLED — "fails closed when provider status belongs to another runtime" |
| M2 | route query-param rejection | routes/standalone-sessions.test.ts | KILLED — "returns session options without accepting workspace inputs" |
| M3 | SDK reject of leaked workspace internals | DaemonClientStandalone.test.ts | KILLED — "rejects malformed standalone options" (1 red of 24) |
| M4 | models hydration from options | DaemonSessionProvider.test.tsx | KILLED — "hydrates models for a deferred standalone session without creating it" |
| M5 | stale/abort guard after options fetch | DaemonSessionProvider.test.tsx | KILLED — "does not publish stale standalone options after switching contexts" |
| M6 | capability gate before fetching options | DaemonSessionProvider.test.tsx | KILLED — "updates an empty controlled provider when its session context changes" |
| M7 | skip post-attach setModel for standalone |
sessionPreparation.test.ts | KILLED — "creates standalone sessions without workspace-only fields" |
| M8 | persist:false reasoning for standalone |
sessionPreparation.test.ts | KILLED — same test |
| M9 | reject per-call modelServiceId for workspace create |
actions.test.ts | KILLED — "rejects a per-call model for workspace creation" |
| M10 | hide composer model action when no models | App.test.tsx | KILLED — "hides the empty model picker until standalone options are available" |
Targeted gates (full files; PR's CI covers the rest)
| suite | result |
|---|---|
cli standalone-session-service.test.ts |
91/91 |
cli routes/standalone-sessions.test.ts |
25/25 |
cli server.test.ts (full) |
1166/1166 |
sdk DaemonClientStandalone.test.ts |
24/24 |
sdk daemon-public-surface.test.ts |
20/20 |
web-shell DaemonSessionProvider.test.tsx |
276/276 |
web-shell sessionPreparation.test.ts |
21/21 |
web-shell actions.test.ts |
144/144 |
web-shell App.test.tsx (full) |
657/657 |
Docs delta vs code (04-docs-delta-vs-code.png, 05-bundle-size-head-vs-base.png)
00-index.mdclaims 148 registered / 42 conditional tags andSERVE_PROTOCOL_VERSION='v1'; compiled registry has exactly 148/42 and base dist has 147 (since: 'v1'count) — the +1/+1 increment is exact.- The protocol doc's feature-list block is a pre-existing curated list (106 of 147 base tags; 41 omissions identical on both arms). The PR's delta to it is exact: adds
standalone_session_options_v1immediately afterstandalone_sessions_v1, removes nothing, every listed tag is a real registry key. - Conditional predicate equivalence holds:
standalone_session_options_v1andstandalone_sessions_v1agree on all toggle sets (docs say "same condition asstandalone_sessions_v1"). - Route table row matches observed behavior: any query → 400; response
{v, initialized, current?, approvalMode?, providers, errors?}with internals omitted (the A/B cells). - Design-doc boundary statements (read-only, no cwd/workspaceCwd, no session/dir creation, fail closed, missing capability doesn't block prompt) all present and each behaviorally proven above. 25/25.
- Bundle: head
dist/daemon/index.js= 224,830 B; base rebuilt = 222,324 B; Δ = 2,506 B — all three numbers exactly as stated in the PR body; the 221,184 B warning threshold is pre-existing (base build warns). 4/4.
Findings (non-blocking)
- Pre-existing config behavior, not a PR defect:
modelProvidersentries need explicit credentials. A settings entry withoutenvKey(e.g.{id:'verify-model-b'}) makes the spawned session fail at generation with the actionable errorMissing credentials for modelProviders model 'verify-model-b'. Configure modelProviders.openai[].envKey and set that environment variable.andPOST /standalone/sessionsreturns 500standalone_creation_outcome_unknown. Reproduced identically on head and base (symmetric), so it predates the PR and the error message already tells the user the fix. Noted because the new options endpoint makes configured-but-credential-less models more visible in the picker. - Docs feature-list block remains curated, not exhaustive — 41 of 148 registry tags are not listed (identical at base). The PR's one-line addition is correct; the pre-existing gap is a docs-maintenance observation, not this PR's debt.
- Reproduction note:
qwen serve --bareskipssettings.jsonentirely (createMinimalSettings), so any harness verifying the catalog must boot with--safe-modeonly and pinQWEN_HOME(the lane exports aQWEN_HOMEthat otherwise overrides~/.qwen).
Not covered
- Per-commit attribution: the shallow checkout reaches only the merge commit,
HEAD^1,HEAD^2;git rev-list HEAD^1..HEAD^2returns 1 while the snapshot lists 2 commits (99736acunreachable). Verified the aggregateHEAD^1..HEADdiff; per-commit attribution out of reach. - Prompt completion with a proxy-unknown model (
verify-model-b): the create orchestration and atomic model-at-create were fully observed, but the lane proxy only serves its own model, so a turn completing on a settings-only model id is not reproducible here; the completing turn ran on the env-declared model. The failure mode for an unknown model is the pre-existing actionable credential/model error (Finding 1), not a silent fallback. - Windows/macOS browsers; workspace, Live, Recents, uploads, channel flows (author-declared out of scope).
- Full repo-wide test/lint/typecheck suites (PR's own CI covers them; targeted suites above all green).
Methodology
Environment: CI verify container (node:22-bookworm), builds pre-installed at HEAD; scratch worktree under tmp/ removed after capture; daemons booted with isolated HOME/QWEN_HOME and the lane's loopback OpenAI-compatible backend for session auth. Harnesses (harness/*.mjs) are mock-free: real HTTP against real head/base daemons, real built SDK DaemonClient, real Chromium driving the Web Shell each daemon serves, loopback decoy peers for gate/validation cells. Every assertion is a scripted comparison; expected reds (mutation kills, base-arm 404) are encoded as passes; intermediate harness-locator fixes during this round were harness bugs, not product behavior, and final runs are what the counts report. Raw logs in logs/, captures in evidence/. Assertion totals: harness checks 113 (A/B 32, wire 18, matrix 17, docs 25, bundle 4, E2E 17) + vitest tests 2,424 = 2,537.
Flakiness gate log
rounds=5 files=9 skipped=0
file packages/cli/src/serve/conversations/standalone-session-service.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/standalone-session-service.test.ts
file packages/cli/src/serve/routes/standalone-sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/standalone-sessions.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/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClientStandalone.test.ts
file packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemon-public-surface.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/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/DaemonSessionProvider.test.tsx
file packages/web-shell/client/daemon/session/actions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/actions.test.ts
file packages/web-shell/client/utils/sessionPreparation.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/sessionPreparation.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/conversations/standalone-session-service.test.ts: PPPPP
packages/cli/src/serve/routes/standalone-sessions.test.ts: PPPP
packages/cli/src/serve/server.test.ts: PPPP
packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: PPPP
packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: PPPP
packages/web-shell/client/App.test.tsx: PPPP
packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: PPPP
packages/web-shell/client/daemon/session/actions.test.ts: PPPP
packages/web-shell/client/utils/sessionPreparation.test.ts: PPPP
verdict: timeout
summary: only 4 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 2 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 3 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/routes/standalone-sessions.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/DaemonClientStandalone.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 4 · packages/web-shell/client/utils/sessionPreparation.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/conversations/standalone-session-service.test.ts: P (exit 0)
Evidence images
2 additional image(s) did not pass the hosting checks (PNG magic, unique sanitized name, ≤2 MB, max 8) and remain in the run artifacts.
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…l-model-options # Conflicts: # docs/developers/daemon/00-index.md
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — packages/cli unit suite did not complete (infrastructure time budget on this machine); packages/web-shell full-suite run failed under an environment load storm with attribution unresolved (175 timeouts across 26 files; the diff-touched DaemonSessionProvider.test.tsx passed 276/276 in isolation; sdk-typescript, qwen-live and webui suites all pass).
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — packages/cli unit suite did not complete (infrastructure time budget on this machine); packages/web-shell full-suite run failed under an environment load storm with attribution unresolved (175 timeouts across 26 files; the diff-touched DaemonSessionProvider.test.tsx passed 276/276 in isolation; sdk-typescript, qwen-live and webui suites all pass)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (modelId && sessionContext?.kind !== 'standalone') { | ||
| preparationStep = 'set model for new session'; |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [new-surface] The first standalone prompt can run on a model the user did not select. The daemon applies the create-time modelServiceId best-effort — applyModelServiceId(...).catch(() => {}) in packages/acp-bridge/src/bridge.ts ("session stays operational on the agent's default model"), while the approvalMode block directly below IS fail-closed — and this diff removes the post-attach setModel for standalone, which was the only step that could observe that failure. A user who picks a model plus a model-bound reasoning effort on a fresh standalone page and submits the first prompt: if the spawn-time switch fails (a setSessionModel timeout, the transport closing mid-switch, or the agent rejecting the id after the catalog drifted), the create still succeeds and carries no model confirmation, setReasoningEffort(..., { persist: false }) is applied on top, and the prompt runs on the daemon default while the composer shows the selection — exactly the outcome this PR's description says it eliminates. The workspace path fails closed in the identical case.
Witness:
npx vitest run src/bridge.test.ts -t "keeps the session alive on model-switch failure" -> 1 passed
(spawn with a rejected modelServiceId resolves successfully; session.attached === false, sessionCount === 1;
sole signal: model_switch_failed bus event)
sessionPreparation.test.ts -> 21 passed, incl. expect(actions.setModel).not.toHaveBeenCalled()
BASE (merge base 431ff3a11f): standalone ran the post-attach setModel; its catch rethrew when a reasoning
effort was bound (release + clear) — the observation point this diff removes
Suggested fix: surface the model-apply outcome to the client (e.g. a modelApplied flag or the effective model in the create response / DaemonStandaloneSession) and treat a not-applied model like the workspace setModel failure — throw when a reasoning effort is bound (releasing the session), warn-and-continue otherwise; do not make the daemon tear the session down on a model failure. Also update the new comment above, which asserts a guarantee the create route does not provide.
Note the swallow is deliberate (bridge.ts: "Already published model_switch_failed; session stays operational on the agent's default model") — the fix must keep the session alive and the event on the bus rather than failing the create, and the design doc requires reasoning applied post-attach with persist: false for standalone. Acceptance: add a sessionPreparation.test.ts case where the standalone create succeeds but the session lands on a different model, asserting the prepare rejects and the session is released/cleared — deleting that verification must turn it red.
中文说明
首次 standalone prompt 可能运行在用户未选择的模型上。守护进程对创建请求中的 modelServiceId 只做尽力而为的应用 —— packages/acp-bridge/src/bridge.ts 中的 applyModelServiceId(...).catch(() => {})("会话保持可用,运行在 agent 默认模型上"),而紧随其后的 approvalMode 应用却是失败即关闭的(fail-closed);同时本 diff 移除了 standalone 路径 attach 后的 setModel —— 那是唯一能观察到上述切换失败的步骤。用户在新的空白 standalone 页面选好模型和与模型绑定的思考强度后提交首条 prompt:若 spawn 阶段的切换失败(setSessionModel 超时、切换过程中传输关闭、或目录漂移后 agent 拒绝该模型 id),创建仍然成功且响应中不含模型确认,setReasoningEffort(..., { persist: false }) 照常应用,首条 prompt 运行在守护进程默认模型上,而编辑器仍显示所选模型 —— 这正是本 PR 描述声称要消除的结果。workspace 路径在相同情形下是失败即关闭的。
证据:仓库契约测试 keeps the session alive on model-switch failure 在本提交通过(携带被拒绝 modelServiceId 的 spawn 成功返回,唯一信号是 model_switch_failed 事件);sessionPreparation.test.ts 21 项通过,其中包含 expect(actions.setModel).not.toHaveBeenCalled();merge base 处的旧代码中,standalone 路径在 attach 后会执行 setModel,且绑定了思考强度时其 catch 会重新抛出(释放并清理会话)—— 即本 diff 移除的观察点。
建议:把模型应用结果暴露给客户端(如创建响应中增加 modelApplied 标志或实际生效的模型),对未应用的模型按 workspace 路径 setModel 失败处理 —— 绑定了思考强度时抛出(释放会话),否则警告并继续;不要让守护进程因模型失败而销毁会话。同时更新上方新增注释,它断言了创建路由并不提供的保证。注意该吞错是有意为之(见 bridge.ts 注释),修复应保持会话存活与事件发布,并保留设计文档要求的 persist: false。验收:新增 standalone 创建成功但会话落在不同模型的测试用例,断言准备阶段拒绝并释放/清理会话;移除该验证应使测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| !connectionRef.current.sessionId && | ||
| effectiveSessionContext?.kind === 'standalone' && | ||
| (connectionRef.current.models?.length ?? 0) === 0 |
There was a problem hiding this comment.
[Suggestion] R1-2: The three new "no selectable models" gates use divergent predicates. This /model slash-command guard counts raw connectionRef.current.models, while the composer-toolbar filter (~line 13332) and the StatusBar onSelectModel gate (~line 15876) count availableModels, which is connection.models filtered through isVisibleComposerModel. A deferred standalone draft whose hydrated catalog contains only hidden entries — coder-model(qwen-oauth) (HIDDEN_COMPOSER_MODEL_IDS); the default qwen-oauth-only daemon catalog is exactly that one entry — then hides the model action in the toolbar and toasts model.unavailable from the StatusBar, but /model sees raw length 1 > 0 and opens ModelDialog, which reads connection.models UNFILTERED and lists the hidden internal model; selecting it goes through selectWelcomeModel, so the hidden "must not be user-selectable" entry becomes the modelServiceId of the next standalone create.
Witness:
PROBE-G2 hidden-only {"rawModels":["coder-model(qwen-oauth)"],"toolbarHasModel":false,"slashModelDialogOpen":true}
with the suggested predicate patched in: {"slashModelDialogOpen":false} — flips
Suggested fix: use one predicate at all three gates — e.g. !(connectionRef.current.models ?? []).some(isVisibleComposerModel) here, or extract a single shared helper used by all three sites.
The visibility decision must reuse isVisibleComposerModel (packages/web-shell/client/utils/composerModels.ts:15), the shared predicate already used by the composer pickers — not a second hand-copied ID list. Acceptance: an App.test.tsx case with mockConnection.models = [{ id: 'coder-model(qwen-oauth)' }], no sessionId, standalone context — submitting /model must toast and must not open the model dialog; reverting to the raw-length check makes it red.
中文说明
三个新的"无可选择模型"门禁使用了不一致的判定条件。这个 /model 斜杠命令门禁统计的是原始 connectionRef.current.models,而工具栏过滤器(约 13332 行)与 StatusBar 的 onSelectModel 门禁(约 15876 行)统计的是经过 isVisibleComposerModel 过滤后的 availableModels。当一个延迟创建的 standalone 草稿水合出的模型目录只包含被隐藏的条目 —— coder-model(qwen-oauth)(HIDDEN_COMPOSER_MODEL_IDS);默认仅 qwen-oauth 的守护进程目录恰好只有这一个条目 —— 工具栏会隐藏模型入口、StatusBar 会弹出 model.unavailable 提示,但 /model 看到原始列表长度为 1 > 0,会打开未经过滤的 ModelDialog,列出这个被隐藏的内部模型;选择它会经由 selectWelcomeModel 使这个"不可被用户选择"的条目成为下一次 standalone 创建的 modelServiceId。
证据:探针(仅隐藏模型的目录)显示工具栏隐藏了模型入口而 /model 打开了对话框;打上建议的判定后翻转为不打开。
建议:三处门禁使用同一判定 —— 例如这里改用 !(connectionRef.current.models ?? []).some(isVisibleComposerModel),或抽取一个三处共用的辅助函数。可见性判定必须复用 isVisibleComposerModel(packages/web-shell/client/utils/composerModels.ts:15),而不是再复制一份隐藏 ID 列表。验收:新增 App.test.tsx 用例(standalone、无 sessionId、models 仅含隐藏条目),提交 /model 应弹出提示且不打开模型对话框;改回原始长度判断应使测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const modelServiceId = | ||
| overrides?.modelServiceId ?? | ||
| createSessionRequestRef.current?.modelServiceId; |
There was a problem hiding this comment.
[Suggestion] R1-3: The new precedence "per-call override wins over the createSessionRequest default" is not pinned by any test — no existing test sets both operands, so inverting the ?? operands leaves the whole suite green. A host that embeds a default model via the createSessionRequest prop (the hydration path this PR exists for) while the user picks a different model for the first prompt: a future edit inverting the operands silently calls createStandalone with the host default instead of the user's pick — the exact regression this PR fixes.
Witness:
Mutation (operands inverted): DaemonSessionProvider.test.tsx -> 276 passed (276) — ships green
Suggested test as probe: FAILED against the mutant (createStandalone received 'host-default'),
PASSED against intact code
Suggested fix: add a provider-level test — render with createSessionRequest={{ modelServiceId: 'host-default' }}, call actions.createSession({ sessionContext: { kind: 'standalone' }, modelServiceId: 'picked' }), and assert createStandalone received { modelServiceId: 'picked' } (optionally a second assertion that omitting the override sends 'host-default', which is also currently unpinned).
The fallback operand must keep sending the prop default when no override is given (createSessionRequestRef.current = createSessionRequest at line ~1084 tracks the host prop, and the pre-change code sent it whenever defined). Acceptance: the new test must go RED when the ?? operands are inverted.
中文说明
新的优先级"单次调用覆盖优先于 createSessionRequest 默认值"没有任何测试钉住 —— 现有测试没有同时设置两个操作数,因此把 ?? 的两个操作数颠倒,整个测试套件依然是绿的。宿主通过 createSessionRequest 属性内嵌默认模型(正是本 PR 存在的水合路径),而用户为首条 prompt 选择了不同模型时:未来某次编辑若颠倒操作数,会悄悄用宿主默认值而非用户所选调用 createStandalone —— 正是本 PR 要修复的回归。
证据:突变(颠倒操作数)下 DaemonSessionProvider.test.tsx 276/276 全绿;建议的测试作为探针在该突变下失败(createStandalone 收到 'host-default'),在完整代码下通过。
建议:新增 provider 级测试 —— 以 createSessionRequest={{ modelServiceId: 'host-default' }} 渲染,调用 actions.createSession({ sessionContext: { kind: 'standalone' }, modelServiceId: 'picked' }),断言 createStandalone 收到 { modelServiceId: 'picked' }(可再断言不提供覆盖时发送 'host-default',该分支目前同样未被钉住)。回退操作数必须保留"无覆盖时发送属性默认值"的行为。验收:颠倒 ?? 操作数时新测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| async getOptions(): Promise<StandaloneSessionOptions> { | ||
| const runtime = await this.options.ensureRuntime(); | ||
| return this.options.runRuntimeActivity(runtime, async () => { |
There was a problem hiding this comment.
[Suggestion] R1-4: getOptions()'s fail-closed guards for runtime quarantine/replacement — the three assertRuntimeCurrent(runtime) checks and the runRuntimeActivity lease — have no test; the design doc's "Runtime replacement, quarantine, or generation changes fail closed" is unpinned for this operation. The harness's assertRuntimeCurrent already throws conversation_runtime_unavailable once quarantineRuntime has run, but no test quarantines and then calls getOptions(); runRuntimeActivity is an inline pass-through in the harness. Deleting all three assertions (or unwrapping the lease) ships green, so a later edit could serve the model catalog from a quarantined or replaced runtime — returning provider status from the wrong runtime generation instead of failing closed. Every other operation in this file has quarantine coverage, so the pattern is established.
Witness:
Mutation (all three assertRuntimeCurrent calls deleted + lease unwrapped):
standalone-session-service.test.ts -> 91 passed (91) — ships green
Suggested quarantine test as probe: FAILED against the mutant, PASSED against intact code
Suggested fix: mirror the existing create-path quarantine test (pattern at standalone-session-service.test.ts:520): await harness.quarantineRuntime(harness.runtime), then await expect(harness.service.getOptions()).rejects.toMatchObject({ code: 'conversation_runtime_unavailable' }).
The harness throws with code 'conversation_runtime_unavailable' when quarantined (test lines 236-240) — the new test must expect that code, not the ownership code. Acceptance: the new test must go RED if the assertRuntimeCurrent calls are removed from getOptions().
中文说明
getOptions() 针对运行时隔离/替换的失败即关闭守卫 —— 三处 assertRuntimeCurrent(runtime) 检查与 runRuntimeActivity 租约 —— 没有测试;设计文档中"运行时替换、隔离或代际变化须失败即关闭"对该操作未被钉住。测试夹具的 assertRuntimeCurrent 在 quarantineRuntime 之后已会抛出 conversation_runtime_unavailable,但没有测试先隔离再调用 getOptions();runRuntimeActivity 在夹具中只是直通。删除全部三处断言(或解开租约)后套件仍然全绿,因此后续编辑可能从被隔离或已替换的运行时提供模型目录 —— 返回错误代际的 provider 状态而不是失败即关闭。该文件中其他所有操作都有隔离覆盖,模式是现成的。
证据:突变(删除三处断言并解开租约)下 91/91 全绿;建议的隔离测试作为探针在该突变下失败、在完整代码下通过。
建议:仿照现有创建路径的隔离测试(standalone-session-service.test.ts:520 处的模式):先 await harness.quarantineRuntime(harness.runtime),再 await expect(harness.service.getOptions()).rejects.toMatchObject({ code: 'conversation_runtime_unavailable' })。夹具有隔离时抛出的错误码是 conversation_runtime_unavailable(而非所有权错误码),新测试须断言该错误码。验收:从 getOptions() 移除这些断言时新测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| createDetachedStandaloneSession({ | ||
| ...(options?.modelServiceId !== undefined | ||
| ? { modelServiceId: options.modelServiceId } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] R1-5: Of the two createDetachedStandaloneSession call sites that forward the new modelServiceId override, only the no-active-session branch (~line 1613) is tested with a model; this active-session branch is exercised only with approvalMode ('returns a detached standalone result without replacing an active session'). Removing the forwarding here would keep the whole suite green. No production caller passes modelServiceId through this branch today (sessionPreparation is the sole setter and runs only without an active session), so the cost is latent: the first caller that does — e.g. a menu-driven "new standalone session" carrying the selected model — would silently drop the selection and run on the daemon default, with no test going red.
Witness:
Mutation (modelServiceId spread dropped from the active-session branch only):
actions.test.ts -> 144 passed (144)
Extending the active-session test with modelServiceId: FAILED against the mutant, PASSED intact
Suggested fix: extend the active-session test to pass modelServiceId and assert expect(createDetachedStandaloneSession).toHaveBeenCalledWith({ approvalMode: 'yolo', modelServiceId: 'qwen3.8-max(USE_OPENAI)' }).
Acceptance: the extended assertion goes RED if this branch stops forwarding options.modelServiceId while the detached-branch test stays green — proving the two sites are independently pinned.
中文说明
转发新增 modelServiceId 覆盖的两个 createDetachedStandaloneSession 调用点中,只有无活动会话分支(约 1613 行)用模型测试过;这个活动会话分支只用 approvalMode 测试('returns a detached standalone result without replacing an active session')。删除这里的转发,整个套件依然全绿。目前没有生产调用方通过该分支传 modelServiceId(sessionPreparation 是唯一设置者且只在无活动会话时运行),所以代价是潜在的:第一个这样做的调用方 —— 例如携带所选模型的菜单式"新建 standalone 会话" —— 会悄悄丢弃选择、运行在守护进程默认模型上,且没有任何测试变红。
证据:突变(仅删除活动会话分支的展开)下 actions.test.ts 144/144 全绿;为活动会话测试补上 modelServiceId 后,在突变下失败、在完整代码下通过。
建议:扩展活动会话测试传入 modelServiceId,断言 createDetachedStandaloneSession 收到 { approvalMode: 'yolo', modelServiceId: 'qwen3.8-max(USE_OPENAI)' }。验收:该分支停止转发时扩展断言变红,而无活动会话分支的测试保持绿色 —— 证明两个调用点被独立钉住。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| ).toHaveBeenCalledWith(expect.anything(), { | ||
| modelServiceId: 'qwen3.8-max(USE_OPENAI)', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R1-7: 'creates a fresh standalone session without the generic route' previously asserted createStandalone toHaveBeenCalledOnce(); this diff replaced that with an args-only assertion, dropping the call-count pin without needing to (the old count assertion passes equally for the new call shape). A regression that issues createStandalone twice with identical arguments on the direct-create path — e.g. a reintroduced automatic retry, which the design doc explicitly rules out ("no-automatic-retry behavior") — would have failed this test and now passes, because toHaveBeenCalledWith is satisfied by any matching call. Sibling tests (lines 1646, 1685, 1798, 1877, 1911) still assert toHaveBeenCalledOnce(), so this is a one-off loss, not a deliberate rebalancing.
Witness:
Faithful double-create mutation (both calls succeed, identical args):
PROBE-G7 {"createStandaloneCalls":2} — the PR's args-only assertion PASSES
PROBE-G7-OLD-ASSERTION-WOULD-FAIL: called 2 times
Suggested fix: keep the new args assertion and additionally restore expect(sdkMocks.MockDaemonSessionClient.createStandalone).toHaveBeenCalledOnce(). Acceptance: the restored count assertion goes red under a double-create mutation that the current assertion tolerates.
中文说明
'creates a fresh standalone session without the generic route' 原先断言 createStandalone toHaveBeenCalledOnce();本 diff 将其替换为仅断言参数的调用,不必要地丢掉了调用次数钉住(旧的次数断言对新的调用形态同样成立)。若直接创建路径上出现以相同参数发出两次 createStandalone 的回归 —— 例如重新引入自动重试(设计文档明确排除:"no-automatic-retry behavior")—— 旧测试会失败,而现在会通过,因为 toHaveBeenCalledWith 对任何匹配的调用都成立。同组测试(1646、1685、1798、1877、1911 行)仍然断言 toHaveBeenCalledOnce(),因此这是个别丢失,而非有意的再平衡。
证据:忠实的双次创建突变(两次调用都成功、参数相同)下,PR 的参数断言通过,而旧的次数断言失败(called 2 times)。
建议:保留新的参数断言,同时恢复 expect(...createStandalone).toHaveBeenCalledOnce()。验收:当前断言容忍的双次创建突变下,恢复的次数断言变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| model['contextLimit'] <= 0) | ||
| ) { | ||
| throw new DaemonStandaloneProtocolError(route, 'invalid contextLimit'); |
There was a problem hiding this comment.
[Suggestion] R1-8: The validator rejects the ENTIRE session-options payload when any single model has a non-positive contextLimit, but the daemon's own producer can emit exactly that, so one misconfigured model silently disables the whole pre-session model catalog. The producer computes contextLimit: model.contextWindowSize ?? tokenLimit(effectiveModelId) (packages/cli/src/serve/workspace-providers-status.ts:190) — ?? does not catch 0, and settingsSchema.ts admits contextWindowSize as a plain number with no positivity constraint (core treats non-positive values as reachable: provider-config.ts requires > 0, context-usage sites guard <= 0). A user with one model configured at contextWindowSize: 0: the options response carries contextLimit: 0, parseStandaloneSessionOptions throws, getStandaloneSessionOptions() rejects, and Web Shell's failure-tolerant fetch leaves the model fields empty — the picker stays hidden until the first prompt creates a session. The identical payload is tolerated in workspace mode, where workspaceProviders() casts without validation, so behavior diverges by session kind for identical config.
Witness:
Producer probe (generationConfig { contextWindowSize: 0 }): PROBE-G8-PRODUCER {"contextLimits":[1000000,0]}
SDK probe: zero/negative -> threw "malformed standalone-session response (invalid contextLimit)"; valid -> parsed
With the <= 0 clause removed: zero/negative -> parsed (flip)
Suggested fix: treat a non-positive contextLimit as absent instead of fatal (normalize non-positive values to undefined after validation, or drop the <= 0 clause), so one misconfigured model cannot sink the entire catalog.
The producer passes a user-configured value through verbatim and the SDK type declares contextLimit?: number — the fix must not assume the daemon always sends a positive number. Acceptance: a DaemonClientStandalone.test.ts case asserting getStandaloneSessionOptions() resolves when a model has contextLimit: 0 (it currently rejects).
中文说明
校验器在任一模型的 contextLimit 非正数时拒绝整个 session-options 载荷,但守护进程自己的生产者完全可能产出这种值,因此一个配置错误的模型会悄悄禁掉整个会话前模型目录。生产者计算 contextLimit: model.contextWindowSize ?? tokenLimit(effectiveModelId)(packages/cli/src/serve/workspace-providers-status.ts:190)—— ?? 不会捕获 0,而 settingsSchema.ts 允许 contextWindowSize 为任意数字、无正数约束(core 也认为非正数可达:provider-config.ts 要求 > 0、若干上下文用量处守卫 <= 0)。用户把一个模型配置为 contextWindowSize: 0 时:options 响应携带 contextLimit: 0,parseStandaloneSessionOptions 抛错,getStandaloneSessionOptions() 拒绝,Web Shell 的容错抓取把模型字段留空 —— 选择器一直隐藏到首条 prompt 创建会话。同样的载荷在 workspace 模式下被容忍(workspaceProviders() 不做校验直接断言),因此相同配置下行为按会话类型分叉。
证据:生产者探针(contextWindowSize: 0)输出 contextLimits:[1000000,0];SDK 探针对 0/负数抛出 'invalid contextLimit'、合法值正常解析;移除 <= 0 子句后 0/负数也能解析(翻转成立)。
建议:把非正 contextLimit 视为缺省而非致命错误(校验后归一化为 undefined,或去掉 <= 0 子句),使单个配置错误的模型不会拖垮整个目录。生产者原样透传用户配置值、SDK 类型声明 contextLimit?: number —— 修复不得假设守护进程总是发送正数。验收:新增 DaemonClientStandalone.test.ts 用例,断言模型 contextLimit: 0 时 getStandaloneSessionOptions() 正常解析(当前会拒绝)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| options['workspaceCwd'] !== undefined || | ||
| options['acpChannelLive'] !== undefined | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-9: The SDK-side rejection of an acpChannelLive leak is untested: the malformed-option it.each cases in DaemonClientStandalone.test.ts include a workspaceCwd leak but no acpChannelLive case, and both disjuncts throw the identical message, so the workspaceCwd row passes even without the acpChannelLive check. A daemon that omits workspaceCwd but still returns acpChannelLive in the session-options response would pass SDK validation and be handed to Web Shell connection state, even though the protocol table for GET /standalone/session-options declares both internals omitted. The daemon-side redaction IS pinned by this PR's service tests, but the client-side enforcement of the same contract is only half-pinned.
Witness:
Baseline: 24 passed (24)
MUTANT (only the acpChannelLive disjunct deleted): 24 passed (24) — ships green
MUTANT + suggested row: 1 failed | 24 passed (25) — "promise resolved ... instead of rejecting"
Intact + suggested row: 25 passed (25)
Suggested fix: add one it.each row: [{ ...standaloneOptions(), acpChannelLive: false }, 'workspace internals']. Acceptance: that case goes RED if the options['acpChannelLive'] !== undefined disjunct is removed from parseStandaloneSessionOptions.
中文说明
SDK 侧对 acpChannelLive 泄漏的拒绝没有测试:DaemonClientStandalone.test.ts 的畸形载荷 it.each 用例包含 workspaceCwd 泄漏但没有 acpChannelLive 用例,且两个析取条件抛出的错误信息相同,因此即使没有 acpChannelLive 检查,workspaceCwd 用例行也能通过。一个省略了 workspaceCwd 却仍返回 acpChannelLive 的守护进程会通过 SDK 校验、进入 Web Shell 连接状态 —— 尽管 GET /standalone/session-options 的协议表声明两个内部字段都被省略。守护进程侧的脱敏由本 PR 的服务测试钉住,但同一契约在客户端侧的执行只被钉住了一半。
证据:基线 24/24 通过;突变(仅删除 acpChannelLive 析取)24/24 通过(可带病上线);突变 + 建议用例行 → 1 失败;完整代码 + 建议用例行 → 25/25 通过。
建议:新增一行 it.each:[{ ...standaloneOptions(), acpChannelLive: false }, 'workspace internals']。验收:从 parseStandaloneSessionOptions 移除 options['acpChannelLive'] !== undefined 析取时该用例变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (options['errors'] !== undefined) { | ||
| if (!Array.isArray(options['errors'])) { | ||
| throw new DaemonStandaloneProtocolError(route, 'expected errors[]'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R1-10: The validator's errors[] path and enum gates (status whitelist, approvalMode, initialized) have zero test coverage — distinct from the acpChannelLive gap. The happy-path fixture standaloneOptions() carries no errors key and no invalid enum value, so a mutant deleting the errors[] loop, flipping the status whitelist, or dropping the approvalMode check ships green. The branch matters exactly in the rare case: the daemon emits errors[] only on degraded paths — config warnings (workspace-providers-status.ts:225, status: 'warning' cells) and the settings-load catch branch (:241, initialized: false) — so a regression there means a misconfigured user's payload is either accepted unvalidated or wholly rejected: Web Shell hides the model picker AND swallows the diagnostic cell that would explain why.
Witness:
MUTANT-A (errors[] block deleted): 24 passed (24) — ships green
MUTANT-B ('warning' dropped from status whitelist): 24 passed (24) — ships green
MUTANT-A + suggested cases: 1 failed | 26 passed (27)
MUTANT-B + suggested cases: 1 failed | 26 passed (27)
Intact + suggested cases: 27 passed (27)
Suggested fix: extend the it.each with one resolve case whose payload includes errors: [{ kind: 'providers', status: 'warning', error: 'boom' }] plus reject cases for invalid cell status and invalid approvalMode (and optionally unknown errorKind / non-boolean initialized).
The resolve-case fixture must match the producer's actual cell shape { kind: 'providers', status: 'warning' | 'error', error: string } (workspace-providers-status.ts:225, :241). Acceptance: removing the errors[] loop or any enum gate must turn the corresponding case red.
中文说明
校验器的 errors[] 路径与枚举门禁(status 白名单、approvalMode、initialized)测试覆盖为零 —— 与 acpChannelLive 缺口相互独立。正常路径夹具 standaloneOptions() 不携带 errors 键、也没有非法枚举值,因此删除 errors[] 循环、翻转 status 白名单或去掉 approvalMode 检查的突变都能带病上线。该分支恰好在少见情形才有意义:守护进程只在降级路径发出 errors[] —— 配置告警(workspace-providers-status.ts:225,status: 'warning')与设置加载的 catch 分支(:241,initialized: false)—— 一旦回归,配置有误的用户的载荷要么未经校验被接受、要么整体被拒绝:Web Shell 隐藏模型选择器,同时吞掉本可解释原因的诊断单元。
证据:突变 A(删除 errors[] 块)与突变 B(白名单去掉 'warning')均 24/24 通过;分别加上建议用例后各有 1 例失败;完整代码加建议用例 27/27 通过。
建议:为 it.each 增加一个解析成功用例(载荷含 errors: [{ kind: 'providers', status: 'warning', error: 'boom' }])以及非法单元 status、非法 approvalMode 的拒绝用例(可选未知 errorKind / 非布尔 initialized)。解析成功用例的夹具须匹配生产者的实际单元形态 { kind: 'providers', status: 'warning' | 'error', error: string }。验收:移除 errors[] 循环或任一枚举门禁时,对应用例变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| for (const field of [ | ||
| 'authType', | ||
| 'modelId', | ||
| 'baseUrl', | ||
| 'fastModelId', | ||
| 'visionModelId', | ||
| ]) { |
There was a problem hiding this comment.
[Suggestion] R1-11: The new parseStandaloneSessionOptions validates current.visionModelId, and the daemon genuinely emits it (buildCurrent in workspace-providers-status.ts; ServeWorkspaceProviderCurrent declares visionModelId?: string at packages/acp-bridge/src/status.ts:533) — but the published type backing the newly exported DaemonStandaloneSessionOptions, DaemonWorkspaceProviderCurrent (types.ts:1979-1984), declares only authType, modelId, baseUrl, fastModelId. A wire field the SDK vouches for is invisible to every typed consumer of the type this PR adds to the public surface: with visionModel set in workspace settings, an SDK consumer hydrating a standalone draft that wants the configured vision model reads options.current?.visionModelId and gets a TypeScript error — the field exists at runtime but not on the type — and must cast through unknown/any (defeating the validation the SDK just performed) or silently drop the value.
Witness:
tsc probe asserting 'visionModelId' is readable on DaemonStandaloneSessionOptions['current']:
intact PR -> error TS2322 (field absent from DaemonWorkspaceProviderCurrent)
with `visionModelId?: string;` added -> probe compiles clean; tsconfig.public-surface.json fence stays clean
Suggested fix: add visionModelId?: string; to DaemonWorkspaceProviderCurrent in packages/sdk-typescript/src/daemon/types.ts, plus a type-level assertion (expectTypeOf) in daemon-public-surface.test.ts.
The field must match the wire shape acp-bridge already declares (visionModelId?: string at status.ts:533). Acceptance: the type-level assertion fails to compile while the field is absent from the interface.
中文说明
新的 parseStandaloneSessionOptions 校验 current.visionModelId,守护进程也确实会发出该字段(workspace-providers-status.ts 的 buildCurrent;packages/acp-bridge/src/status.ts:533 的 ServeWorkspaceProviderCurrent 声明 visionModelId?: string)—— 但新导出的 DaemonStandaloneSessionOptions 所依赖的公开类型 DaemonWorkspaceProviderCurrent(types.ts:1979-1984)只声明了 authType、modelId、baseUrl、fastModelId。SDK 已为之背书的一个线上字段,对本 PR 加入公开表面的这个类型的每个类型化消费者都不可见:工作区设置中配置了 visionModel 时,水合 standalone 草稿并想读取所配置视觉模型的 SDK 使用者读 options.current?.visionModelId 会得到 TypeScript 错误 —— 字段在运行时存在但类型上没有 —— 只能通过 unknown/any 强转(架空了 SDK 刚做的校验)或悄悄丢弃该值。
证据:tsc 探针断言 DaemonStandaloneSessionOptions['current'] 可读 'visionModelId':完整 PR 下报 TS2322;补上 visionModelId?: string; 后探针编译通过、public-surface 栅栏也保持干净。
建议:在 packages/sdk-typescript/src/daemon/types.ts 的 DaemonWorkspaceProviderCurrent 中加入 visionModelId?: string;,并在 daemon-public-surface.test.ts 增加类型级断言(expectTypeOf)。字段形态须与 acp-bridge 已声明的线上一致(status.ts:533)。验收:接口缺少该字段时,类型级断言无法编译。
— qwen3.8-max via Qwen Code /review (v0.22.3)
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.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/App.test.tsx:11001 — [probe] R2-3 new test assigns undefined to mockConnection.models but MockConnection declares it required (TS2322 under strict; typecheck excludes test files)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // must never be applied after a failed switch to an unknown model. | ||
| if (modelId) { | ||
| if (modelId && sessionContext?.kind !== 'standalone') { | ||
| preparationStep = 'set model for new session'; |
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [new-surface] Still stands at 2ddb0d39 (round 2, re-verified by probe): the first standalone prompt can run on a model the user did not select. The daemon applies the create-time modelServiceId best-effort (applyModelServiceId(...).catch(() => {}) in bridge.ts:5504-5514), and this diff removes the post-attach setModel that was the only step able to observe that failure — so a failed switch silently falls back to the daemon default while the composer shows the selection, and the reasoning effort is applied on top of the wrong model. The workspace path fails closed in the identical case.
Witness (round-2 probe, failing unstable_setSessionModel):
intact PR: spawnStandaloneSession({ modelServiceId: 'probe-model' }) RESOLVED; first replayed event model_switch_failed
mutant (catch removed): StandaloneSessionSpawnError at bridge.ts:9228 — the probe flips
Suggested fix: surface the model-apply outcome to the client (e.g. a modelApplied flag or the effective model in the create response) and treat a not-applied model like the workspace setModel failure — throw when a reasoning effort is bound (releasing the session), warn-and-continue otherwise; also update the new comment above, which asserts a guarantee the create route does not provide. The swallow is a deliberate shared-session contract pinned by bridge.test.ts:21361 (#3889 A05Ym) — the fix must keep the session alive and the event on the bus rather than failing the create. Acceptance: a sessionPreparation.test.ts case where the standalone create succeeds but the session lands on a different model must reject and release the session, and deleting that verification must turn it red.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):首条 standalone prompt 可能运行在用户未选择的模型上。守护进程对创建请求中的 modelServiceId 只做尽力而为的应用(bridge.ts:5504-5514 中的 applyModelServiceId(...).catch(() => {})),而本 diff 移除了 attach 后的 setModel——那是唯一能观察到切换失败的步骤,因此切换失败时会静默回退到守护进程默认模型,编辑器仍显示所选模型,思考强度还会应用到错误的模型上。workspace 路径在相同情形下是失败即关闭的。建议:把模型应用结果暴露给客户端(如创建响应中的 modelApplied 标志或实际生效模型),对未应用的模型按 workspace setModel 失败处理——绑定思考强度时抛出(释放会话),否则警告并继续;同时更新上方新注释,它断言了创建路由并不提供的保证。注意该吞错是共享会话的有意契约(bridge.test.ts:21361 钉住,#3889 A05Ym)——修复应保持会话存活与事件发布。验收:新增"创建成功但落在不同模型"的用例,断言准备阶段拒绝并释放会话;删除该验证应变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| !connectionRef.current.sessionId && | ||
| effectiveSessionContext?.kind === 'standalone' && | ||
| (connectionRef.current.models?.length ?? 0) === 0 |
There was a problem hiding this comment.
[Suggestion] R1-2: Still stands at 2ddb0d39 (round 2, re-verified by probe): the three "no selectable models" gates remain divergent — this /model guard counts raw connectionRef.current.models, while the composer-toolbar filter (~13332) and the StatusBar onSelectModel gate (~15872) count the hidden-id-filtered availableModels. With a hidden-id-only catalog, /model opens the dialog while the other two surfaces show "unavailable". A single memoized predicate on the filtered list fixes it — the round-2 probe reproduces the divergence on the intact PR and flips when the guard is unified.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):三处"无可选择模型"门禁仍然不一致——此 /model 守卫统计原始 connectionRef.current.models,而工具栏过滤(~13332)与 StatusBar onSelectModel 门禁(~15872)统计经隐藏 id 过滤的 availableModels。仅含隐藏 id 的模型列表下,/model 会打开对话框,而另外两处显示"不可用"。用基于过滤列表的单一 memo 谓词即可修复——第 2 轮探针在原始代码上复现了该分歧,统一门禁后翻转。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| ); | ||
| const modelServiceId = | ||
| overrides?.modelServiceId ?? | ||
| createSessionRequestRef.current?.modelServiceId; |
There was a problem hiding this comment.
[Suggestion] R1-3: Still stands at 2ddb0d39 (round 2, re-verified by probe): the override-precedence merge (overrides?.modelServiceId ?? createSessionRequestRef.current?.modelServiceId) still has no test with both sources populated — the operand-swap mutant leaves the whole suite green while the host's model silently wins over the user's selection. The fix is a provider test mirroring the approvalMode harness at :2638 with both operands set; it must go red on operand swap and on ref-only revert.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):覆盖优先级合并(overrides?.modelServiceId ?? createSessionRequestRef.current?.modelServiceId)仍没有任何同时填充两个来源的测试——交换 ?? 操作数的突变体下整套测试仍为绿,而宿主模型会静默压过用户的选择。修复方式:仿照 :2638 的 approvalMode 用例新增同时设置两个操作数的 provider 测试;该测试在操作数互换或回退到仅读 ref 时应变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
|
|
||
| async getOptions(): Promise<StandaloneSessionOptions> { | ||
| const runtime = await this.options.ensureRuntime(); | ||
| return this.options.runRuntimeActivity(runtime, async () => { |
There was a problem hiding this comment.
[Suggestion] R1-4: Still stands at 2ddb0d39 (round 2, re-verified by probe): getOptions()'s fail-closed guards for runtime quarantine/replacement — the three assertRuntimeCurrent(runtime) checks — still have no test. Deleting all three leaves the suite green (91/91); a quarantine probe flips both ways (intact rejects with conversation_runtime_unavailable, mutant resolves with the provider status). The harness already wires the quarantine path (test:235-241) — one test closes this.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):getOptions() 针对运行时隔离/替换的失败即关闭守卫——三处 assertRuntimeCurrent(runtime)——仍然没有测试。删除全部三处后测试套件仍为绿(91/91);隔离探针双向翻转(原始代码以 conversation_runtime_unavailable 拒绝,突变体直接返回 provider status)。测试桩已接好隔离路径(test:235-241)——补一个用例即可闭合。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| createDetachedStandaloneSession({ | ||
| ...(options?.modelServiceId !== undefined | ||
| ? { modelServiceId: options.modelServiceId } | ||
| : {}), |
There was a problem hiding this comment.
[Suggestion] R1-5: Still stands at 2ddb0d39 (round 2, re-verified by probe): of the two createDetachedStandaloneSession call sites forwarding modelServiceId, this active-session branch still has no arg assertion — removing the spread stays green (144/144), and a standalone create made while a session is attached would silently land on the daemon default model. The probe flips with an attached-session harness asserting the received args.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):转发 modelServiceId 的两处 createDetachedStandaloneSession 调用点中,此活动会话分支仍然没有参数断言——删除该展开后测试仍为绿(144/144),且在已有会话附着时发起的 standalone 创建会静默落到守护进程默认模型。用带附着会话的测试桩断言收到的参数,探针即可翻转。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if ( | ||
| options['workspaceCwd'] !== undefined || | ||
| options['acpChannelLive'] !== undefined | ||
| ) { |
There was a problem hiding this comment.
[Suggestion] R1-9: Still stands at 2ddb0d39 (round 2, re-verified by inspection): the SDK-side rejection of an acpChannelLive leak remains untested — the malformed-options it.each table covers a workspaceCwd leak but has no acpChannelLive case, so that disjunct of the guard (standalone-sessions.ts:331-334) can regress unseen. One row fixes it: [{ ...standaloneOptions(), acpChannelLive: true }, 'workspace internals'].
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,检视复验):SDK 侧对 acpChannelLive 泄漏的拒绝仍然没有测试——畸形载荷 it.each 表覆盖了 workspaceCwd 泄漏,但没有 acpChannelLive 用例,因此该守卫(standalone-sessions.ts:331-334)的这一分支回归时不会被发现。补一行即可:[{ ...standaloneOptions(), acpChannelLive: true }, 'workspace internals']。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (options['errors'] !== undefined) { | ||
| if (!Array.isArray(options['errors'])) { | ||
| throw new DaemonStandaloneProtocolError(route, 'expected errors[]'); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R1-10: Still stands at 2ddb0d39 (round 2, re-verified by probe): the validator's errors[] path and enum gates (initialized boolean, approvalMode) still have zero test coverage — deleting any of these branches leaves the SDK suite green (mutants: 24/24 each; errors-branch deletion: full suite 1731 green). Note 'auto' IS a valid approval mode (PERMISSION_MODES = plan/default/auto-edit/auto/yolo) — the malformed fixture must use a value like 'not-a-mode'.
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):校验器的 errors[] 路径与枚举门禁(initialized 布尔、approvalMode)仍然零测试覆盖——删除任一分支后 SDK 套件仍为绿(各突变体 24/24;删除 errors 分支时全套 1731 为绿)。注意 'auto' 是合法的 approval mode(PERMISSION_MODES = plan/default/auto-edit/auto/yolo)——畸形载荷用例需使用如 'not-a-mode' 的值。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| 'baseUrl', | ||
| 'fastModelId', | ||
| 'visionModelId', | ||
| ]) { |
There was a problem hiding this comment.
[Suggestion] R1-11: Still stands at 2ddb0d39 (round 2, re-verified by probe): parseStandaloneSessionOptions validates current.visionModelId and the daemon genuinely emits it when settings.visionModel is configured, but DaemonWorkspaceProviderCurrent still omits the field — integrators reading it through the public type hit TS2339 (probe reproduced; clean after adding visionModelId?: string).
中文说明
在 2ddb0d39 上仍然成立(第 2 轮,探针复验):parseStandaloneSessionOptions 校验 current.visionModelId,守护进程在配置了 settings.visionModel 时也确实会发出该字段,但 DaemonWorkspaceProviderCurrent 仍未声明它——集成方通过公共类型读取会得到 TS2339(探针已复现;补上 visionModelId?: string 后消除)。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (disposed || abort.signal.aborted) return; | ||
| const providerModelStatus = | ||
| mapProviderStatus(standaloneOptions); |
There was a problem hiding this comment.
[Critical] R2-1: [certifies-falsely] [new-surface] A session attached while the standalone-options request is in flight is wiped from connection state by this late draft publish. The guard checks only disposed || abort.signal.aborted, never whether a session landed meanwhile.
Standalone draft on a cold runtime: getStandaloneSessionOptions() blocks in the daemon's ensureRuntime() for seconds while status is connecting, and the composer stays usable (shouldBlockComposerSubmit blocks only error/disconnected, composerInputState.ts:50-52). The user submits the first prompt; createSession publishes sessionId (actions.ts:1660-1688). If the options response lands after that publish but before this effect's cleanup aborts, the continuation passes both guards and setConnection spreads clearNonWorkspaceSessionState(current) — which routes a standalone context through getConnectionAfterSessionClear and deletes sessionId/clientId/standaloneSession/context/reasoning (actions.ts:283-297) over the live session. The UI reverts to an empty draft mid-stream and session-scoped consumers drop queued input (e.g. the shell-command drain, App.tsx:8727-8755). The nonce-bumped re-run re-adopts the session afterwards, so the damage is transient but user-visible. Pre-PR this branch had no await between the capabilities guard and setConnection — this diff widens the race window from one capabilities RTT to a runtime-boot-scale options RTT.
Witness:
Probe (DaemonSessionProvider.test.tsx): deferred options promise; createSession publishes sessionId 'standalone-race'; resolve options.
intact PR: AssertionError: expected undefined to be 'standalone-race' // live sessionId wiped
with session-present guard: Tests 2 passed // probe flips
Suggested fix — make the draft publish a no-op when a session landed meanwhile, mirroring the ACP-preheat closure pattern at :1688:
if (disposed || abort.signal.aborted || sessionRef.current) return;and/or keep current in the functional update when current.sessionId is set (current.status === 'error' || current.sessionId ? current : {...}). Note clearNonWorkspaceSessionState (:728-735) routes every non-workspace context through that delete-set — the fix must preserve the context-switch abort semantics pinned by the existing 'does not publish stale standalone options after switching contexts' test. Please add a test that holds the options promise deferred, creates/attaches a session, then resolves the options and asserts connection.sessionId survives — and confirm it goes red when the session-present check is removed.
中文说明
standalone options 请求在途期间若有会话完成附着,这次迟到的草稿发布会话把该会话从连接状态中抹掉。守卫只检查 disposed || abort.signal.aborted,从不检查期间是否已有会话落地。冷启动场景:getStandaloneSessionOptions() 在守护进程 ensureRuntime() 中阻塞数秒,状态停留在 connecting,而编辑器可用(shouldBlockComposerSubmit 仅拦截 error/disconnected)。用户提交首条 prompt,createSession 发布 sessionId;若 options 响应在该发布之后、本 effect 清理中止之前到达,续体通过两道守卫,setConnection 展开 clearNonWorkspaceSessionState(current)——standalone 上下文会经由 getConnectionAfterSessionClear 删除活跃会话的 sessionId/clientId/standaloneSession/context/reasoning。UI 在流式过程中退回空白草稿,按会话维度消费的逻辑(如 shell 命令队列排空)会丢弃排队输入。nonce 触发的重跑随后会重新收养该会话,因此损害是瞬时但用户可见的。改动前此分支在能力守卫与 setConnection 之间没有 await——本 diff 把竞态窗口从一次能力 RTT 拉宽到一次运行时启动量级的 options RTT。修复建议:会话已落地时让草稿发布成为空操作(if (disposed || abort.signal.aborted || sessionRef.current) return;),或参照 :1688 的 ACP 预热闭包模式在函数式更新中保留 current。请补充测试:挂起 options promise → 创建并附着会话 → 解析 options,断言 connection.sessionId 存活;移除会话存在检查时该测试应变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| currentModel: providerModelStatus.currentModel, | ||
| currentMode: providerModelStatus.currentMode, | ||
| contextWindow: providerModelStatus.contextWindow, |
There was a problem hiding this comment.
[Critical] R2-2: [certifies-falsely] [regression] A late standalone-options hydration overwrites the user's pending model/mode selection, so the first prompt creates the session on the daemon's previous model/mode.
During the connecting window the composer is fully usable, and the /model <name> branch (App.tsx:11542-11546) has no availability gate. The user runs /model qwen3-max and /plan; when the options response lands, this publish writes connection.currentModel/currentMode from the daemon's own values, and the pre-existing sync effects (App.tsx:8957-8963) fire on the undefined→value change, silently clobbering both selections. createSession then carries the daemon's previous model/mode. Pre-PR this publish wrote undefined for both fields, so the effects never re-fired during the draft and the pending selections survived to create — this diff introduces the regression.
Witness:
Probe (App.test.tsx harness): dispatch /model qwen3-max then /plan; late publish currentModel 'qwen-default' / currentMode 'default'; submit first prompt.
intact PR: createSession received { approvalMode: 'default', modelServiceId: 'qwen-default' } // both selections lost
with prev-preserving guards on both sync effects: received { approvalMode: 'plan', modelServiceId: 'qwen3-max' } — probe flips;
the PR's own hydration and first-prompt tests still pass
Suggested fix — preserve the pending session-less selection when syncing:
setCurrentModel((prev) =>
!connection.sessionId && prev && prev !== (connection.currentModel ?? '')
? prev
: (connection.currentModel ?? ''),
);(and the analogous guard for currentMode), or suppress the hydration's currentModel/currentMode overwrite while a pending selection exists. Initial hydration with no pending selection must still populate the pickers — pinned by 'hydrates models for a deferred standalone session without creating it' and the first-prompt test asserting modelServiceId: 'qwen'; the empty-prev fallthrough keeps both green. Please add an App.test.tsx case that holds the options deferred, selects /model qwen3-max, resolves options with a different current.modelId, and asserts the first prompt carries the user's selection — it is red on the current code and must go red again if the guard is removed.
中文说明
迟到的 standalone options 水合会覆盖用户待定的模型/模式选择,导致首条 prompt 以守护进程先前的模型/模式创建会话。connecting 窗口内编辑器完全可用,/model <name> 分支(App.tsx:11542-11546)也没有可用性门禁。用户执行 /model qwen3-max 与 /plan 后,若 options 响应到达,本次发布会把 connection.currentModel/currentMode 写成守护进程自身的值,既有的同步 effect(App.tsx:8957-8963)因 undefined→有值的变化触发,静默覆盖这两项选择;createSession 随后携带守护进程先前的模型/模式。改动前该发布对两个字段写入 undefined,effect 不会在草稿期间重新触发,待定选择可以保留到创建——本 diff 引入了该回归。修复建议:同步时保留无会话状态下的待定选择(如上 setCurrentModel 函数式守卫,currentMode 同理),或在存在待定选择时抑制水合对这两个字段的覆盖。无待定选择的初始水合仍须填充选择器(现有两个测试钉住);空 prev 兜底可保持两者为绿。请补充用例:挂起 options → /model qwen3-max → 以不同 current.modelId 解析 options,断言首条 prompt 携带用户所选模型;该用例在当前代码上为红,移除守卫后应再次变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
















What this PR does
This PR lets Web Shell load the configured model catalog before a standalone session exists. It adds a capability-gated, read-only standalone options endpoint backed by the exact internal Conversations runtime, validates the response in the TypeScript SDK, and hydrates the existing Web Shell model and reasoning controls during deferred standalone startup. The endpoint accepts no workspace input and omits the internal Conversations path and ACP state.
When the first standalone prompt creates its session, Web Shell now sends the selected model in the existing standalone create request and skips the later best-effort model switch. A selected reasoning effort is still applied after attachment without persisting it into the internal Conversations workspace. Older daemons or failed option reads continue to create with the daemon default.
Why it's needed
Web Shell intentionally defers standalone session creation until the first prompt, but model data previously arrived only after a session attached. A freshly opened standalone page therefore showed an empty model picker. Applying a preselected model after creation also allowed the first prompt to run with the daemon default if the follow-up switch failed. Loading the model catalog without creating a session and making model selection part of creation fixes both behaviors while preserving standalone isolation and recovery semantics.
Reviewer Test Plan
How to verify
Start the daemon and Web Shell, open
/?context=standalonewithout a session id, and confirm the current model and configured model list are visible before any standalone session is created. Open and close the picker and confirm noPOST /standalone/sessionsoccurs. Select a non-default model and submit the first prompt; the single create request should contain the stable session id, selectedmodelServiceId, and approval mode, with nocwdorworkspaceCwd, and no follow-up model-switch request. The attached session should keep the selected model. Against an older daemon or a failed options response, the empty picker should remain hidden and first-prompt creation should still use the daemon default.Evidence (Before & After)
Before: a fresh standalone page showed
Select Model:with no selected model, and opening the picker renderedNo model matching ""; the model catalog appeared only after the first session attached.After: a fresh standalone page showed
moonshot/kimi-k3immediately and listed 10 configured models whileGET /standalone/sessionsstill returned zero sessions. Isolated browser E2E also selectedqwen3.8-coder-plus, observed exactly onePOST /standalone/sessionscontaining thatmodelServiceId, received the prompt response, and observed no model/config switch request.Tested on
Environment (optional)
Node.js 22.22.3 on macOS; local
qwen serve --bare --safe-mode; Web Shell exercised in Chromium through the in-app browser. Unit tests, package builds, the repository build, full typecheck, SDK public-surface check, formatting, ESLint, bundle generation, and isolated browser E2E were run locally. The SDK browser bundle is 224,830 bytes versus an exact-main baseline of 222,324 bytes; both exceed the existing 221,184-byte warning threshold, and this PR adds 2,506 bytes for runtime response validation.Risk & Scope
standalone_session_options_v1, so older daemons retain their existing behavior.Linked Issues
N/A
中文说明
本 PR 做了什么
本 PR 让 Web Shell 能够在 standalone 会话尚未创建时加载已配置的模型列表。它新增了一个由 capability 控制、只读的 standalone options 接口;该接口只从精确的内部 Conversations runtime 读取数据。TypeScript SDK 会对响应做运行时校验,Web Shell 则在延迟创建 standalone 会话的启动阶段复用现有模型和思考选项状态。接口不接受 workspace 输入,也不会返回内部 Conversations 路径或 ACP 状态。
首次 standalone prompt 创建会话时,Web Shell 现在会把用户选择的模型放进现有 standalone 创建请求,并跳过后续的 best-effort 模型切换。用户选择的思考强度仍会在 attach 后应用,但不会持久化到内部 Conversations workspace。旧 daemon 或 options 请求失败时,仍会使用 daemon 默认模型完成创建。
为什么需要
Web Shell 会刻意将 standalone 会话创建延迟到首次 prompt,但之前模型数据只能在会话 attach 后获得,因此首次打开 standalone 页面会显示空模型选择器。创建后再应用预选模型还存在另一个问题:如果后续切换失败,首次 prompt 可能已经使用 daemon 默认模型运行。现在通过在不创建会话的情况下读取模型列表,并将模型选择原子地放入创建请求,同时修复了这两个问题,并保留 standalone 的隔离和恢复语义。
Reviewer 测试计划
如何验证
启动 daemon 和 Web Shell,不带 session id 打开
/?context=standalone,确认在创建任何 standalone 会话前就能看到当前模型和已配置模型列表。打开并关闭模型选择器,确认不会发送POST /standalone/sessions。选择一个非默认模型并提交首次 prompt;唯一一次创建请求应包含稳定 session id、所选modelServiceId和 approval mode,不包含cwd或workspaceCwd,并且没有后续模型切换请求。attach 后的会话应保持所选模型。连接旧 daemon 或 options 请求失败时,空模型选择器应保持隐藏,首次 prompt 仍应使用 daemon 默认模型创建会话。证据(修复前与修复后)
修复前:新打开的 standalone 页面显示
Select Model:,没有选中模型,打开选择器会显示No model matching "";只有首次会话 attach 后才会出现模型列表。修复后:新打开的 standalone 页面立即显示
moonshot/kimi-k3并列出 10 个已配置模型,同时GET /standalone/sessions仍返回零个会话。隔离浏览器 E2E 还选择了qwen3.8-coder-plus,观察到唯一一次POST /standalone/sessions携带对应modelServiceId,成功收到 prompt 响应,并确认没有模型或配置切换请求。已测试平台
环境(可选)
macOS,Node.js 22.22.3;本地
qwen serve --bare --safe-mode;通过应用内浏览器中的 Chromium 验证 Web Shell。已在本地运行单元测试、各 package 构建、仓库构建、完整 typecheck、SDK public-surface 检查、格式化检查、ESLint、bundle 生成以及隔离浏览器 E2E。SDK 浏览器 bundle 为 224,830 字节,精确 main 基线为 222,324 字节;两者都超过现有 221,184 字节 warning 阈值,本 PR 为运行时响应校验增加了 2,506 字节。风险与范围
standalone_session_options_v1控制,因此旧 daemon 保持原有行为。关联问题
N/A