fix(serve): Hot-reload runtime model providers - #10269
Conversation
Issue 10184 runtime model-provider syncBaseline
Verification
ResultsExecuted on 2026-08-27 with a disposable Baseline
Post-fix
Primary raw evidence was retained under The real-daemon run covered HTTP, ACP, Session registry, SSE, active-turn, |
🖼️ 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 |
|
Re-run gate on head Template looks good ✓ Problem: an observed bug with strong evidence. Linked issue #10184 remains open, and @wenshao reproduced the exact defect on the merge base in a live Linux environment (real daemon, real ACP children, mock providers, Web Shell in Chromium) before confirming this PR's path fixes it end-to-end. This is the opposite of theoretical hardening. Direction: aligned. A model installed at runtime being unusable until Size: 905 production logic lines (additions + deletions), 2,204 test lines, and 122 lines of design doc, across cli / acp-bridge / core / sdk-typescript / web-shell. The Stage 0 two-tier gate protects core from external contributions and exempts maintainer-authored PRs — this pass re-confirmed @doudouOUC's write access through the collaborator API, so the size cap does not bind this run. The size stays noted for awareness; it is not a blocker. Approach: the scope matches what the problem actually requires — four copies of state must agree after a mutation (persisted settings, future-child spawn env, the live ACP child, every Session registry), and the diff addresses each with a persist-first, fail-closed ordering. The final commit is a tightly focused fix of the two round-8 findings plus their pinning tests. No drive-by changes spotted. Risk: Stage 1e matches high-risk paths — Moving on to code review. 🔍 中文说明针对 head 模板完整 ✓ 问题：已观测到的 bug，证据充分。关联 issue #10184 仍处于 open 状态；@wenshao 已在真实 Linux 环境中（真实 daemon、真实 ACP child、mock provider、Chromium 中的 Web Shell）于 merge base 上精确复现该缺陷，并确认本 PR 的路径端到端修复了它。这不是理论性加固。 方向：对齐。运行时安装的模型必须重启 规模：约 905 行生产逻辑（增删合计）、2,204 行测试、122 行设计文档，横跨 cli / acp-bridge / core / sdk-typescript / web-shell。Stage 0 双层门槛用于保护核心模块免受外部贡献的冲击，维护者本人发起的 PR 豁免——本轮已通过 collaborator API 重新确认 @doudouOUC 具有 write 权限，因此规模封顶在本轮不适用。规模仅作知悉记录，不构成阻断。 方案：范围与问题所需相符——一次变更后必须保持一致的状态有四份（持久化 settings、未来 child 的 spawn 环境、当前 live ACP child、所有 Session 注册表），diff 以 persist-first、失败关闭的顺序逐一覆盖。最后一次提交是针对第 8 轮两项发现的聚焦修复及其锁定测试。未发现顺手改动。 风险：Stage 1e 命中高风险路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
Code review of Round-8 findings — both confirmed fixed on this head, with pinning tests.
Fresh pass over the whole production diff — no new blockers found. Highlights of what I checked directly, hunk by hunk:
Standing deferrals (recorded in round 8, not requested again): the pre-publish reload loop has no attempt bound; the DELETE sync-rejection and aggregation-rejected branches are unpinned by tests; the AuthMessage StrictMode probe (render-phase ref reset); and provider mutations run with no client deadline (timeout 0), so a wedged daemon keeps the UI busy. Follow-up-issue material, not blockers. The sync flow, for orientation: sequenceDiagram
participant P1 as Web Shell
participant P2 as serve route
participant P3 as workspace service
participant P4 as daemon env
participant P5 as ACP child
P1->>P2: install provider or delete model
P2->>P2: persist settings first
P2->>P3: syncModelProvidersRuntime
P3->>P4: rebuild parent spawn env, fail-closed
P3->>P5: model-providers reload control call
P5->>P5: reload settings, refresh Session registries, clear caches
P5-->>P3: configsRefreshed and configsFailed
P3-->>P2: applied, deferred, or failed
P2-->>P1: HTTP 200 plus runtimeSync
Files changed (30 of 43 shown)
Test evidence — the PR's own CI on the reviewed commit, fetched once via the API (this run never builds or executes PR code). No movement since the last pass thirty minutes ago: CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The behavioural claim is already substantiated beyond the unit suite — @wenshao's live A/B verification in a real Linux daemon/ACP/Web Shell environment (his evidence, earlier head) and the sandboxed verify on this exact head (161/161, merge-ready). The author's own runs were macOS-only, so the two in-flight Linux jobs plus the in-flight verify re-run are the remaining signal: Real-scenario testing: N/A for this pass — unattended CI re-run; live behaviour was exercised in @wenshao's Linux environment (his evidence, linked in the thread), not by this review. 中文说明对 第 8 轮两项发现——均已在本 head 上确认修复，且有锁定测试。
对全部生产 diff 的重新审查——未发现新的阻断项。 逐 hunk 直接核对的要点：
仍在延后清单中的事项（第 8 轮已记录，本轮不再要求）：发布前 reload 循环没有尝试次数上限；DELETE 同步拒绝分支与聚合拒绝分支没有测试锁定；AuthMessage 的 StrictMode 探针（render 阶段重置 ref）；provider 变更没有客户端超时（timeout 0），daemon 卡死时 UI 会一直忙碌。适合作为后续 issue 跟进，不构成本 PR 阻断。 测试证据——通过 API 一次性拉取的本提交 CI 结果（本审查不构建、不执行 PR 代码）。 与三十分钟前上一轮相比无变化： 行为层面的主张已经在单测之外获得证实——@wenshao 在真实 Linux daemon/ACP/Web Shell 环境的实机 A/B 验证（他的证据，针对较早 head）与针对本 head 的沙箱 verify（161/161，可合并）。作者本人的运行仅在 macOS 上，因此正在执行的两个 Linux job 与在途的 verify 复跑是剩余信号：若单测套件像 真实场景测试：本次不适用——非值守 CI re-run；实机行为由 @wenshao 在其 Linux 环境中执行（他的证据，见本线程），并非本次审查所驱动。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the two round-8 Criticals are re-verified fixed on this head with pinning tests, a full hunk-level pass over all ~905 production lines found no new blockers, and the verdict is approve; the withheld point is the unit suite that has never completed green on this head (a repo-wide 1-hour-cap problem also present on Stepping back. My independent proposal for #10184 would have been exactly the shape this PR landed on: persist the mutation, notify the live child to reload over a control call, rebuild the parent spawn environment for future children, and report sync status on the response. The PR matches that and goes further where the problem genuinely demands it — four copies of state that can disagree, sessions mid-initialization while a mutation lands (hence the revision fence and pre-publish recheck), and a multi-workspace daemon needing user-scoped fan-out with workspace-scoped isolation. That machinery is the actual state space of the daemon, not over-abstraction, and it reuses what already exists ( What changed since the last pass, and why the verdict holds:
Housekeeping for whoever merges this:
✅ Verdict: approve — deferred until CI lands green on this commit. 中文说明置信度：4/5 —— 第 8 轮的两项 Critical 已在本 head 上复核确认修复且有锁定测试，对全部约 905 行生产改动的逐 hunk 重读未发现新的阻断项，结论为批准；扣掉的一分是单测套件在本 head 上从未绿色跑完（ 退一步看。如果让我独立解决 #10184，方案形状与本 PR 一致：先持久化变更，通过控制调用通知 live child 重载，为后续 child 重建父进程 spawn 环境，并在响应中上报同步状态。PR 与此一致，并在问题确实要求的地方走得更远——可能不一致的状态有四份、变更落地时可能有 session 正在初始化（因此需要 revision 栅栏与发布前重查）、多 workspace daemon 需要 user scope 扇出与 workspace scope 隔离。这些机制是 daemon 真实的状态空间，不是过度抽象，且复用既有实现（ 上一轮之后发生了什么、结论为何保持：
给合并者的事项清单：
✅ 结论：批准——延迟到本提交 CI 全绿后执行。 — Qwen Code · qwen3.8-max Reviewed at |
🩺 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 |
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: 38 passed · 0 failed · 38 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：38 通过 · 0 失败 · 38 总计 抖动门： Verification reportPR #10269 — fix(serve): Hot-reload runtime model providersVerdict: 中文摘要
Central claim + A/BCentral claim: a provider installed (or model deleted) after a session was created becomes usable in that live session without restarting Harness (
Witnesses: The base cell reproduces #10184 exactly (route visible in status, switch 500, restart was the only remedy); the head cell closes it, and the wire oracle proves the installed credential — not the boot env key — served the post-switch turn. Secondary probes (both arms, symmetric)
Mutation matrix (
|
| row | mutation | result |
|---|---|---|
| 0 | control, unmutated | green (13 passed, sync filter) |
| 1 | server.ts: auth-provider route sync wiring removed |
killed — 3 auth-route sync tests red with behavioral assertions (expected undefined to deeply equal { status: 'deferred' }, object mismatch, expected 200 to be 503); DELETE-route sync test stayed green (precise attribution) |
| 2 | workspace-service/index.ts: configsFailed no longer marks failed |
killed — expected { status: 'applied' } to deeply equal { status: 'failed' } |
| 3 | restored tree | green again (13 passed) |
The A/B itself is the vacuity witness for the child-side path: the head switch succeeded only because the ACP child executed workspaceModelProvidersReload and rebuilt its registries; acp-integration suite (1233/1233) additionally pins beforeSessionPublish/reloadModelProvidersFromDisk.
Targeted gates (exact counts)
| suite | result |
|---|---|
cli server.test.ts |
1104/1104 |
cli run-qwen-serve.test.ts |
329/329 |
cli acpAgent.test.ts + session/Session.test.ts |
1233/1233 |
cli facade.test.ts + workspace-models.test.ts + settingsWatcher.test.ts |
green |
cli settings.test.ts |
161/174 — see Finding 1 |
acp-bridge bridge.test.ts |
804/804 |
core config.test.ts |
580/580 |
sdk DaemonClient.test.ts |
354/354 |
web-shell App.test.tsx / AuthMessage.dom.test.tsx |
549/549 / 4/4 |
Gate liveness: the mutation rows prove the runner and collection are live (mutants turned the selected suites red with the intended assertions).
Findings (non-blocking)
settings.test.tsenvironmental cluster; the PR's one new test lands in it. 13 tests fail in this sandbox: 12 fail identically on base (byte-identical failing names, sameexpected undefined to be …signature), and the 1 head-only failure is the PR's newshould keep workspace settings empty when reloading the home directory, whose failing assertion (getModelProvidersOwnerScope→'User') is downstream of the PR's change — the PR'ssettings.tsdiff is confined toreloadScopeFromDisk, and the pre-existing sibling fails atsettings.user.settings.uiright afterloadSettings(), beforereloadScopeFromDiskis ever called, on base. The four modified reload tests that pin the new boolean return pass. Not a PR regression; recommend confirming the suite is green in project CI (author tested macOS only).- Pre-existing
[object Object]sibling in daemon logs (both arms). On env-file read failure the daemon warnsfailedFiles="[object Object]"— the same stringification disease this PR fixes for model-switch notices, in the env-file warning path. Present on base; out of this PR's scope; noted for a follow-up. - Other
err instanceof Error ? err.message : String(err)sites remain inacp-bridge(10 sites on non-model_switch_failedpaths). The PR fixed exactly the two emitters named by the bug; scope matches. Informational.
Not covered
- Active-turn generator retention E2E (Reviewer plan step 3): pinned only by the core unit test (
getContentGenerator()unchanged acrossreloadModelProvidersConfig, green) — holding a turn open during install was not driven. - ACP channel recreation with updated credential E2E (step 5): the env-rebuild path was exercised via the S2c failure injection and the applied path, but a child respawn was not driven.
- Browser-level Web Shell E2E (component-level dom tests green instead).
- Per-commit attribution: single-commit PR, N/A.
- Windows/Linux author-side runs (author-declared untested; this round ran on Linux).
Methodology
Environment: CI merge-ref checkout (HEAD merge, HEAD^1 base, HEAD^2 head, depth 2), node:22 container; head dist prebuilt by the lane. Base control: scratch worktree tmp/base-tree at HEAD^1, core/acp-bridge/cli rebuilt there with tsc --build; root node_modules exposed via a symlink farm with @qwen-code/{qwen-code-core,acp-bridge,qwen-code} re-pointed into the base tree (realpath asserted; sdk and channel/web packages link head dists — unchanged by the PR, identical on both arms). Harnesses (ab-harness.mjs, probes.mjs, s2c-failed.mjs) drive real daemons over real HTTP with a loopback OpenAI stub that records every request (model + Authorization) as the wire oracle; raw logs in logs/. Assertions aggregated by check-assertions.mjs from the executed runs only.
Flakiness gate log
rounds=5 files=13 skipped=0
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
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/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/config/settings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/settings.test.ts
file packages/cli/src/config/settingsWatcher.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/settingsWatcher.test.ts
file packages/cli/src/serve/routes/workspace-models.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/workspace-models.test.ts
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.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-service/__tests__/facade.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/workspace-service/__tests__/facade.test.ts
file packages/core/src/config/config.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config.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/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/messages/AuthMessage.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/AuthMessage.dom.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: PPP
packages/cli/src/acp-integration/acpAgent.test.ts: PPP
packages/cli/src/acp-integration/session/Session.test.ts: PP
packages/cli/src/config/settings.test.ts: PP
packages/cli/src/config/settingsWatcher.test.ts: PP
packages/cli/src/serve/routes/workspace-models.test.ts: PP
packages/cli/src/serve/run-qwen-serve.test.ts: PP
packages/cli/src/serve/server.test.ts: PP
packages/cli/src/serve/workspace-service/__tests__/facade.test.ts: PP
packages/core/src/config/config.test.ts: PP
packages/sdk-typescript/test/unit/DaemonClient.test.ts: PP
packages/web-shell/client/App.test.tsx: PP
packages/web-shell/client/components/messages/AuthMessage.dom.test.tsx: PP
verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.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/settings.test.ts: P (exit 0)
round 1 · packages/cli/src/config/settingsWatcher.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/workspace-models.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/workspace-service/__tests__/facade.test.ts: P (exit 0)
round 1 · packages/core/src/config/config.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/messages/AuthMessage.dom.test.tsx: P (exit 0)
round 2 · packages/acp-bridge/src/bridge.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/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/config/settings.test.ts: P (exit 0)
round 2 · packages/cli/src/config/settingsWatcher.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/routes/workspace-models.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/workspace-service/__tests__/facade.test.ts: P (exit 0)
round 2 · packages/core/src/config/config.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/messages/AuthMessage.dom.test.tsx: P (exit 0)
round 3 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/acpAgent.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
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 (Agent 7 ran the unit suites of the changed workspaces only); the Test (macos-latest / windows-latest) matrix jobs were also skipped.
Not explored to full depth (tool budget reached): chunk 7: executing the two new tests (App.test.tsx filtered run, AuthMessage.dom.test.tsx) — the worktree has no node_modules and a full monorepo npm ci + workspace bu….
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (Agent 7 ran the unit suites of the changed workspaces only); the Test (macos-latest / windows-latest) matrix jobs were also skipped。
未探索到全部深度（达到工具调用预算）：chunk 7：executing the two new tests (App.test.tsx filtered run, AuthMessage.dom.test.tsx) — the worktree has no node_modules and a full monorepo npm ci + workspace bu…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
Addressed review feedback in 27257df.
Verification: focused affected-package tests passed; pre-commit formatting/lint passed; full repository build and typecheck passed. |
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. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (Agent 7 ran the unit suites of the changed workspaces only); the Test (macos-latest / windows-latest) matrix jobs were also skipped.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/workspace-service/__tests__/facade.test.ts:3584 — [probe] SessionNotFoundError + failed-env branch of reloadModelProviders has no test (mutant survives)packages/cli/src/serve/routes/workspace-models.ts:305 — [probe] DELETE route generation-close sync guards have no test (POST route has one)packages/cli/src/acp-integration/acpAgent.ts:12009 — [probe] reloadEnvironment(merged, cwd) in the reload handler is never asserted
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (Agent 7 ran the unit suites of the changed workspaces only); the Test (macos-latest / windows-latest) matrix jobs were also skipped。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
Follow-up review items addressed in 4802a7f.
Verification: full acpAgent.test.ts (515), Session.test.ts (721), and DaemonClient.test.ts (356) passed; npm run build and npm run typecheck passed. Independent mutation probes confirmed each new assertion turns red for its corresponding regression. |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Review spanning all three commits. No blocking findings at this head.
Round-1 Criticals — both resolved in 27257dfb:
- R1-1 (
App.tsxunreachableruntimeSync: 'failed'warning): Fixed — the SDK delete call now has a 45s budget and the Web UI wraps with a 50s client timeout, making thefailedbranch reachable before the client request times out. Author confirmed. - R1-2 (
AuthMessage.tsxinstall-path same race): Fixed with the same 45s SDK / 50s WebUI timeout layering.
Round-1 Suggestions — resolved in 27257dfb:
- R1-3/R1-4/R1-10: Reload mock cleared before test scope boundaries; reload call arguments pinned with
toHaveBeenNthCalledWith. - R1-5: Test added for
newSessionrejecting and disposing when final provider reload fails (both User and Workspace scope failure in4802a7f7). - R1-11: Persisted settings writes now broadcast before runtime sync rather than after.
- R1-12:
reloadDaemonEnvrejection path covered by facade regression test. - R1-13: Non-object JSON emits a shape-only warning, not treated as the object itself.
- R1-14: Ordering assertion now outside the sync mock, not inside it.
- R1-15: Unused optional
assertGenerationOpenparameter removed from both routes. - R1-16: Recovery guidance leads with restart over retry for the delete-provider flow.
Round-1 Suggestions — intentionally declined (R1-6/R1-7/R1-8/R1-9): Error details deliberately excluded from runtime-sync log output; raw child exceptions excluded from the structured error string per the approved design contract. Author's rationale documented in inline comments.
Round-2 Suggestions — resolved in 4802a7f7:
- R2-1/R2-3/R2-4: Failed-creation test now parametrized over User and Workspace scope; pre-publish reload guard failure path for
newSessionandlanguagePropagationpaths both covered. - R2-2:
DaemonClient.test.tsfetch mock now wires the request's AbortSignal to rejection so the 31s SDK timeout test can actually abort. - R2-5:
Session.test.tsnow covers workspace scope failure inreloadModelProvidersFromDisk.
CI: build-cli SKIPPED; Real daemon E2E / Java 11, web-shell E2E Smoke, Java matrix (ubuntu/windows/macos), and Post Coverage Comment all pass.
Reviewed with AI assistance.
|
@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: 72 passed · 0 failed · 72 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：72 通过 · 0 失败 · 72 总计 抖动门： Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) - follow-up round at new head Ran the PR in an isolated, token-free container: A/B against the base build at the new base tip, mock-free harness assertions, mutation A/B across the test-strengthening delta, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 72 passed · 0 failed · 72 total 中文 — 判定：✅ 通过 · 可合入（agent 判定，follow-up 轮）
Verification reportPR #10269 — fix(serve): Hot-reload runtime model providers (follow-up round)Verdict: Topology note: the snapshot lists 3 commits; Previous-finding status table (re-measured at new head, never diffed)
Additionally observed this round (new sibling of the F2/F3 family, both arms, pre-existing): the daemon's generic route-error log line Central claim + A/B (re-measured at new head)Central claim (carried): a provider installed (or model deleted) after a session was created becomes usable in that live session without restarting Harness (
Witnesses: The base cell reproduces #10184 exactly at the NEW base tip; the head cell closes it on top of 294 base-advance files, i.e. against what would actually merge. Secondary probes (both arms, symmetric, re-measured)
Delta since the previous roundCommit
Commit
No mutant regressed killed→survived. Witness: Note on M-D1/M-D2 old-arm survival (why it is the finding, not noise): with the 45s arg removed the client falls back to its 30s default and aborts at t=30s — but the OLD fake fetch ignores the abort signal, so the client's awaited promise still resolves at 31s and the old assertions pass. The old tests therefore never exercised the timeout at the wire; commit 3's abort-listener fake is what makes the 45s headroom observable. This is the "scenario never reached the code under test" failure mode, fixed by the strengthening commit; the new tests kill both mutants with the intended assertions. Carry-over mutations (re-measured at new head):
Reviewer Test Plan walk-through (per step, at new head)
Targeted gates (exact counts, at new head)
Gate liveness: the mutation rows prove the runners and collections live (mutants turned the selected suites red with the intended assertions); the settings.test.ts cluster is proven environmental by the base-side byte-identical comparison. Findings (non-blocking)
Not covered
MethodologyEnvironment: CI merge-ref checkout ( Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/sdk-typescript/test/unit/DaemonClient.test.ts:829 — [probe] installAuthProvider headroom test nested inside describe('deleteModel')packages/cli/src/acp-integration/session/Session.test.ts:961 — [probe] reloadModelProvidersFromDisk happy path cannot discriminate an apply-before-reload mutantpackages/sdk-typescript/test/unit/DaemonClient.test.ts:792 — [probe] 31s headroom tests pin only the 30s lower bound, not the 45s mutation-timeout cappackages/cli/src/acp-integration/session/Session.test.ts:960 (+1 locations) — [probe] providerProtocol ?? {} reload default unpinned at both reload call sitespackages/cli/src/acp-integration/acpAgent.test.ts:4567 — [probe] initializing-session reload test pins call counts only, never the applied provider argumentspackages/sdk-typescript/test/unit/DaemonClient.test.ts:853 — [probe] the only installAuthProvider test never inspects the recorded request
Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: packages/cli/src/acp-integration/acpAgent.test.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. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 5 条（其中 5 条首次提出）。发现反复回到同一批文件：packages/cli/src/acp-integration/acpAgent.test.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.2)
wenshao
left a comment
wenshao
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.
Not reviewed: build-and-test — packages/cli unit tests timed out after 540 seconds.
Not reviewed: build-and-test — local verification ran on Node 24.18.1 rather than the required Node 22.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/workspace-auth.ts:354 — [review] Provider installation lacks a test that pins persistence before runtime synchronization.packages/cli/src/serve/routes/workspace-models.ts:304 — [review] The deletion route does not directly test generation closure thrown by runtime synchronization.packages/sdk-typescript/src/daemon/DaemonClient.ts:360 — [review] The timeout tests prove 31 seconds succeeds but do not pin the intended 45-second deadline.packages/web-shell/client/App.tsx:11308 — [review] Delete-model UI tests do not prove applied, deferred, and absent statuses remain warning-free.
Convergence: round 3 posted 8 inline comment(s), 8 of them reported for the first time. Findings keep coming back to the same files: packages/cli/src/acp-integration/session/Session.ts (findings in round 2; 1 more now). (Evidence: the previous round was recovered from a marker this account did not post, so those rounds may not be this account's own.) 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.)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：build-and-test — packages/cli unit tests timed out after 540 seconds。
未审查：build-and-test — local verification ran on Node 24.18.1 rather than the required Node 22。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 4 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 8 条行内评论，其中 8 条是首次提出。发现反复回到同一批文件：packages/cli/src/acp-integration/session/Session.ts（第 2 轮已出过发现，本轮又有 1 条）。（证据说明：上一轮的数据来自并非本账号发布的标记，上述轮次可能不属于本账号。）一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
— gpt-5.6-sol via Qwen Code /review (v0.22.2)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
98b789a to
f9abe3f
Compare
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.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D8-6 publish-loop re-iteration branch unpinned — already reported as R4-2 (comment 3879560855, acpAgent.ts:13042) and deferred to follow-up by the author
- D8-7 DELETE sync-rejection branches untested — already reported in the round-2 deferred list (review 5042583992, workspace-models.ts:305) and round-3 deferred list (review 5044922045, workspace-models.ts:304)
- D8-8 publish-loop change-detection disjuncts exercised only in their false state — already reported (comment 3882343587, acpAgent.ts:13037) and deferred to follow-up by the author
- D8-10 partial-persist path discards the runtimeSync result — already reported in the round-5 deferred list (review 5053125741, workspace-models.ts:280) and discussed as R6-8 (comment 3883342901, workspace-models.ts:287)
- D8-11 dynamic-workspace fail-closed branches uncovered — already reported as R4-8 (comment 3879560897, run-qwen-serve.ts) and deferred to follow-up by the author
- D8-14 unconditional revision bump forces a redundant re-auth — already reported (comment 3882343609, acpAgent.ts:12100) and deferred to follow-up by the author
- D8-15 envFileReadFailed discarded without diagnostics — same family as the sync-failure stderr error detail already discussed and declined by the author with maintainer acceptance of the design's secret-safety contract (threads 3871829201, …
Not reviewed: build-and-test — install + core/acp-bridge builds green (one infrastructure timeout), but zero test suites ran within the per-call budget and test-efficacy was unmeasurable (all 18 probes inconclusive on build state); Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Test (macos-latest / windows-latest, Node 22.x) matrix jobs were also skipped.
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
4 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/web-shell/client/components/messages/AuthMessage.tsx:197 — [probe] Critical [fails-closed] [new-surface] StrictMode wedges the install dialog in "Saving…" — owner-change reset effect keyed on the transient ownerChanged boolean neve…packages/cli/src/serve/server.test.ts:33364 — [review] D8-2: The runtime-sync aggregation's rejected-secondary clause ( result.status === 'rejected' in syncModelProvidersRuntime ) has no test pin — every test in this block resolves the se…packages/web-shell/client/App.tsx:11534 — [review] D8-5: A combined delete response {removed: true, requiresRestart: true, runtimeSync: {status:'failed'}} — reachable when the deleted model id also appears in modelFallbacks (the route r…packages/cli/src/acp-integration/acpAgent.ts:13037 — [review] D8-9: This network-bound ensureAuthenticated runs after the last options.signal?.throwIfAborted() (line 12991) with no re-check, so a session can publish past its initializat…packages/web-shell/client/components/messages/AuthMessage.tsx:199 — [review] D8-13: The owner-change reset clears the load error without re-triggering the catalog fetch. The dialog is App-level state and survives session switches, while the…packages/web-shell/client/daemon/workspace/actions.ts:1036 — [review] provider mutations lost their client deadline — timeout 0 disables it; a wedged daemon leaves the UI permanently busypackages/cli/src/serve/run-qwen-serve.test.ts:7658 — [review] overlay env-reload failure branches unexercised — 0 of 10 reloadModelProviders call sites drive thempackages/cli/src/serve/run-qwen-serve.ts:5513 — [review] four drifted copies of the fail-closed env-reload algorithm; the primary lacks the apply-step catch its twins have
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 7 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — install + core/acp-bridge builds green (one infrastructure timeout), but zero test suites ran within the per-call budget and test-efficacy was unmeasurable (all 18 probes inconclusive on build state); Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; Test (macos-latest / windows-latest, Node 22.x) matrix jobs were also skipped。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
4 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 8 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Addressed the two round-8 Critical findings in eca0424.
Verification: acpAgent 533/533, workspace facade 124/124, run-qwen-serve 357/357, npm run build, and npm run typecheck. Suggestion-level and explicitly deferred findings were not expanded in this round to preserve convergence. |
|
@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: 161 passed · 0 failed · 161 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：161 通过 · 0 失败 · 161 总计 抖动门： Verification reportPR #10269 — fix(serve): Hot-reload runtime model providers (follow-up round 7)Verdict: 中文 — 判定：✅ 通过 · 可合入（agent 判定，follow-up 第 7 轮）
Previous-finding status table (re-measured at new head + new base, never diffed)
Central claim + A/B (re-measured fresh at the new merge tree and the new base tip)Central claim (carried): a provider installed (or model deleted) after a session exists becomes usable in that live session without restarting Harness: real
Env-failure contract ( Active-turn generator retention ( Delta verification: the two commits since round 6Round-6 head (
Mutation matrix over the delta guards (scratch worktree at the merge commit)Every mutant is a single-point revert applied by exact-match replacement (anchor uniqueness asserted: D1 ×1, D2 ×1, D3 two 1-occurrence anchors, D4 ×1 per file, D5 ×1), restored via
4/4 load-bearing delta guards killed by their intended assertions; the 1 survivor is classified (coverage gap D2); suite control green; no mutant regressed. Read-site audit of the added field (AGENTS.md rule): Targeted gates (exact counts, merged tree, serialized vitest; witness
|
| suite | result | note |
|---|---|---|
| cli acpAgent.test.ts | 535/535 | round 6: 526 (+9 from delta/main) |
| cli acpAgent.worktree.test.ts | 3/3 | |
| cli Session.test.ts | 738/738 | round 6: 724 |
| cli server.test.ts | 1120/1120 | round 6: 1109; includes fan-out unit gates |
| cli run-qwen-serve.test.ts | 357/357 | identical count on the base arm too (A/A ledger) |
| cli workspace-service facade.test.ts | 124/124 | round 6: 123 (+1 delta test) |
| cli environment.test.ts | 31/31 | |
| cli settingsWatcher.test.ts | 46/46 | |
| cli routes/workspace-models.test.ts | 24/24 | |
| core config.test.ts + modelRegistry.test.ts | 665/665 | round 6: 659 |
| acp-bridge bridge.test.ts | 827/827 | round 6: 804 |
| sdk DaemonClient.test.ts | 377/377 | round 6: 367 |
| web-shell App.test.tsx + AuthMessage.dom.test.tsx + main-boot.test.tsx | 591/591 | includes the runtimeSync.status=failed toast/message tests |
| web-shell daemon/workspace/actions.test.ts | 18/18 | moved from webui by main's cutover; webui's old path no longer exists |
Gate liveness: the mutation matrix's 4 killed mutants turned these same runners red with the intended assertions in the same tree. settings.test.ts is deliberately not gated — it is Finding F1. The old packages/webui/src/daemon/workspace/actions.test.ts from round 6's table no longer exists at this base (deleted by main's cutover, not by this PR — the PR touches zero webui files at this merge).
Findings (non-blocking)
settings.test.tsenvironmental cluster persists at the new base tip — base413b6d15d3: 162/174 passing; merged head: 163/176. The 12 base failure names re-appear byte-identically on head (scripted set comparison, witness00); the single head-only failure is the PR's ownshould keep workspace settings empty when reloading the home directory, inside the same home-dir/env cluster. Not a PR regression; reproduces on both arms in this Linux container (author tested macOS only). Repro:cd packages/cli && npx vitest run src/config/settings.test.ts.- D2 coverage gap (test to write):
reload()'s catch arm (runtimeEnvironmentApplied = falsewhenreloadDaemonEnvrejects) survived mutation — no test drives a non-fatal rejection through that path; the generation-closed test rethrows before the response exists. The guard itself is fail-closed and consistent withreloadModelProviders's catch; recommend a facade test with a rejectingreloadDaemonEnvassertingruntimeEnvironmentApplied: falsein both the response and thesettings_reloadedevent. runtimeEnvironmentAppliedhas no client reader yet (informational): set by all producers, consumed server-side by the provider-sync fail-closed check, exposed additively in the reload response/event — but web-shell surfaces onlyruntimeSync.status. Matches the design doc's stated intent (observability); noted per the added-field read-site audit so the asymmetry is a conscious choice.- The daemon's own error log still renders
[object Object]on the fixed failure path (sibling of the bug(serve): runtime-added model cannot be set as current in Web Shell until daemon restart #10184 class, pre-existing, out of the PR's touched files). Head-run evidence:logs/head/head-main-2.daemon.log:43—[ERROR] [DAEMON] route=POST /session/:id/model sessionId=22a02988-… [object Object]for the intentionally-failed B4 switch. Cause: the generic bridge-error pathsendBridgeError(packages/cli/src/serve/server/error-response.ts:905–942) logs via threeerr instanceof Error ? … : String(err)sites (thedaemonLog.errorline, theemitDaemonLogerror.messagetelemetry field, and the legacy stderr fallback); a JSON-RPC-shaped object error ({code: -32603, message: 'Internal error', data: …}— the exact shape the HTTP body carried) stringifies to[object Object]. Census: head daemon log = 1 occurrence (B4 only; A6 succeeded), base = 2 (A6 + B4) — identical behaviour both arms, so the PR neither caused nor changed it; the PR fixed the client-visible surfaces (SSE + readable HTTP details) while the operator-facing log/telemetry record of the same failure loses its message. The file is not in the PR's diff; an extractor already exists in the codebase (extractErrorMessage, used by the PR's SSE fix), so the follow-up is mechanical. Not blocking. failedFiles="[object Object]"in the daemon env-file warning — stands on both arms (re-driven this round viaf2-probe.mjs). Pre-existing, out of scope, follow-up candidate.- 37 remaining
err instanceof Error ? … : String(err)siblings inbridge.ts(39 at the new base tip; PR converts exactly the twomodel_switch_failedemitters, diff −2/+2). Informational; scope matches the bug report, and the A/B proves the reported symptom is fixed. Note the round-6 census was scoped tobridge.ts; Finding 4 shows the same class also clusters (×3) inerror-response.ts. - Harness infra disclosure (clean round): all six daemon runs (2 head main, 1 head envfail, 1 head activeturn, 1 base main, 1 base envfail) booted first-try with zero infra failures — no load-tainted replacements were needed this round (round 6 needed three). Runtime env: node 22 container, 64 CPUs; vitest serialized per package tree.
Not covered
- Reviewer Test Plan step 5 (ACP channel recreation E2E) — pinned by the acpAgent/Session/bridge suites (all green at the merged tree), not driven end-to-end. Steps 1–4 and 6 were driven E2E (A/B + envfail + activeturn).
- Workspace-scoped fan-out E2E (user-scoped fan-out verified in round 6 with a two-runtime daemon; this round's delta does not touch fan-out) — pinned by
server.test.tsfan-out unit tests (green) rather than a second E2E daemon. - Round 6's full 14-entry mutation matrix was not re-run — the delta matrix above covers the changed guards; round-6 guards' pins are re-asserted by the green gates that contain their killing tests (server/acpAgent/environment/DaemonClient/AuthMessage suites). The round-6 N4 mutant (secondary provider-reload envfail) specifically was not re-mutated.
- Per-commit verification of the PR's 12 commits: depth-2 checkout; only the last three (
3fcb3aee,f9abe3f7,eca0424f56) are individually reachable and were individually attributed; the earlier nine are verified as the aggregateHEAD^1..HEADdiff. - Main's advance itself (
fe34a5cf..413b6d15d3, fix(cli): deliver teammate messages at tool-round boundaries, not whole-task end #9638 atop refactor(vscode-ide-companion): migrate chat to WebShell and qwen serve #9811's cutover, 498 files): verified only insofar as it interacts with this PR (merge integrity + the 11 auto-merged files + overlapping gates); its own commits are not individually verified. The metadata snapshot'sbaseRefOid(fe34a5cf) is older than the merge-ref base (413b6d15d3) — established by content marker (dynamicCommandTranslationpresent infe34a5cf, removed by fix(cli): deliver teammate messages at tool-round boundaries, not whole-task end #9638); the lane builds the merge ref against the newer tip, which is what this round verified. - Browser-level Web Shell E2E — component/dom tests green instead (
App.test.tsx,AuthMessage.dom.test.tsx,main-boot.test.tsx). - Full-repo
npm run lint— the PR's own CI covers it; this round rebuilt the base tree and ran the affected suites instead. - Windows/Linux author-side runs (author-declared untested).
- The lane's flakiness gate — separate advisory lane; this round's ledger entries are above (F4 row).
Methodology
Environment: CI merge-ref checkout (HEAD merge 95c31f5a11, HEAD^1 base 413b6d15d3, HEAD^2 head eca0424f56, depth 2; delta commits 3fcb3aee/f9abe3f7 reachable as objects), node:22-bookworm container, 64 CPUs. Merge integrity: per-file blob membership (43 PR files: 32 adopt the branch blob, 11 auto-merged where main's #9638 also touched), full-tree census that every non-PR file is byte-identical to base, no conflict markers, and no PR-marker line removed by the auto-merge (round 6's 3-way replay was not possible — the current branch point is not reachable as an object). Base control: worktree at HEAD^1, node_modules wired as a symlink farm with all 26 @qwen-code/* links re-pointed into the base tree (realpath-asserted; nested per-package node_modules mirrored, external deps shared — lockfile untouched by this PR, verified), fully rebuilt with the repo's own build scripts; base dist proven free of all six PR symbols with head dist as control-on-the-control. Harnesses (harness/) drive real daemons over real HTTP with a loopback OpenAI stub recording every request (model + Authorization) as the wire oracle; SSE streams censused live; raw logs in logs/{head,base}/. Mutation runs used a third scratch worktree at the merge commit (tmp/mut-tree) with the same farm plus head-tree dist/ symlinks, mutating source by exact-match single-point replacement and restoring via git checkout (tracked-dirty=0 asserted after each); all vitest invocations serialized per package tree. Evidence captures in evidence/ produced with scripts/verify-capture.mjs from the recorded run summaries and scripted re-prints. Assertion accounting: merge integrity 56 + A/B harnesses 61 (head main 2×13, base main 13, envfail 2×7, activeturn 8) + F2 probes 6 (3/arm) + mutation matrix 6 (4 kills + 1 classified survivor + suite control) + targeted gates 14 + F1 set comparison 3 + F4 ledger 2 + symbol census 9 (6 base-clean + 3 head-present) + delta attribution 2 + daemon-log [object Object] census 2 (Finding 4) = 161, all pass; fail counts only unexpected outcomes, of which there were none.
Flakiness gate log
rounds=5 files=18 skipped=0
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
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/acpAgent.worktree.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.worktree.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/environment.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/environment.test.ts
file packages/cli/src/config/settings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/settings.test.ts
file packages/cli/src/config/settingsWatcher.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/settingsWatcher.test.ts
file packages/cli/src/serve/routes/workspace-models.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/workspace-models.test.ts
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.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-service/__tests__/facade.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/workspace-service/__tests__/facade.test.ts
file packages/core/src/config/config.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config.test.ts
file packages/core/src/models/modelRegistry.test.ts: (cd packages/core) npx --no-install vitest run ./src/models/modelRegistry.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/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/messages/AuthMessage.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/AuthMessage.dom.test.tsx
file packages/web-shell/client/daemon/workspace/actions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/workspace/actions.test.ts
file packages/web-shell/client/main-boot.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/main-boot.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: P
packages/cli/src/acp-integration/acpAgent.test.ts: P
packages/cli/src/acp-integration/acpAgent.worktree.test.ts: P
packages/cli/src/acp-integration/session/Session.test.ts: P
packages/cli/src/config/environment.test.ts: P
packages/cli/src/config/settings.test.ts: P
packages/cli/src/config/settingsWatcher.test.ts: P
packages/cli/src/serve/routes/workspace-models.test.ts: P
packages/cli/src/serve/run-qwen-serve.test.ts: P
packages/cli/src/serve/server.test.ts: P
packages/cli/src/serve/workspace-service/__tests__/facade.test.ts:
packages/core/src/config/config.test.ts:
packages/core/src/models/modelRegistry.test.ts:
packages/sdk-typescript/test/unit/DaemonClient.test.ts:
packages/web-shell/client/App.test.tsx:
packages/web-shell/client/components/messages/AuthMessage.dom.test.tsx:
packages/web-shell/client/daemon/workspace/actions.test.ts:
packages/web-shell/client/main-boot.test.tsx:
verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (0 done) — no flakiness signal either way
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.worktree.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/environment.test.ts: P (exit 0)
round 1 · packages/cli/src/config/settings.test.ts: P (exit 0)
round 1 · packages/cli/src/config/settingsWatcher.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/workspace-models.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.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
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Runtime verification report — PR #10269I built this PR into a real Verdict: the fix is real, the whole reviewer test plan reproduces, and I found no regression. One test-coverage gap is confirmed by mutation (it matches a finding already reported and deferred on this PR), and one Web Shell refresh observation is worth a sentence from the author. Notably, the review bot reported it could not run a single test suite across the last rounds; the suites do pass here — 5,594 of them.
1. Issue #10184 reproduces on the base, and the PR fixes itOne session is created before the provider install. Nothing is restarted between the install and the model switch.
On the base the model is visible in The same thing in the browser, against the Web Shell the daemon serves. Only the page was refreshed; the daemon process and its ACP child ran continuously:
2. The
|
| Step | Result |
|---|---|
1–2 · install after session creation, applied, select the route live |
✅ HTTP 200, applied, switch 200, turn served by MOCKB |
| 3 · install during an active turn | ✅ install returned in 19 ms; the in-flight turn finished on its original generator (MOCKA:SLOW-DONE); the new model was selectable afterwards |
| 4 · delete a model | ✅ removed:true, clearedActiveModel:true, requiresRestart:false, runtimeSync applied; the session was not switched (its next turn still reached MOCKB); the stale route returned a readable 500; switching back to alpha worked |
| 5 · rebuilt ACP child | ✅ after SIGKILL of the child, the replacement selected the new model and presented the updated credential on the wire |
| 6 · injected failures | ✅ degraded parent env → HTTP 200 with runtimeSync failed, persistence not rolled back, provider still listed; no live child → deferred |
| — · concurrency (not in the plan) | ✅ 8 installs interleaved with 8 session creations: every session created during the storm could select the last-installed model |
4. The multi-workspace gap I raised earlier (F1) is closed
With the daemon bound to two workspaces and a live session in each, a user-scope install through the unqualified route now reaches the secondary workspace's pre-existing session as well, and its next turn really lands on MOCKB with the new key. Workspace-scoped providers correctly stay primary-only, which matches the contract the author documented.
5. Both round-8 Critical fixes verified on the wire
- R8-2 —
POST /workspace/reloadnow reportsruntimeEnvironmentApplied: trueon a healthy reload andfalsewhen the parent-environment refresh fails closed, and thesettings_reloadedSSE event carries the same flag (alongsideenvFileReadFailed: true). Verified against a live daemon with an unreadable workspace.env. - R8-1 — the deferred-activation settings reload is pinned by the suite: forcing
settingsReloaded = true(i.e. applying the environment without the atomic reload) fails 3 tests.
6. Tests and mutation testing
All affected suites pass locally:
| Package | Result |
|---|---|
packages/cli (9 touched files incl. run-qwen-serve, server, acpAgent, facade) |
3125 / 3125 |
packages/acp-bridge |
822 / 822 |
packages/core (modelRegistry, config) |
665 / 665 |
packages/sdk-typescript (DaemonClient) |
373 / 373 |
packages/web-shell (App, AuthMessage.dom, workspace actions, main-boot) |
609 / 609 |
| Total | 5594 passed, 0 failed |
Nine mutants against the PR's own suites — eight killed:
| # | Mutation | Result |
|---|---|---|
| M1 | parent-env failure no longer marks the sync failed | killed (2) |
| M2 | closed channel reports deferred even when the parent env was degraded (R8-1 posture) |
killed (1) |
| M3 | child-reported per-Config failures ignored | killed (1) |
| M4 | F1 fix reverted — user-scope install stops fanning out to secondary runtimes | killed (2) |
| M5 | revert the structured error extractor (reintroduces [object Object]) |
killed (2) |
| M6 | deferred activation applies the environment without the atomic settings reload | killed (3) |
| M7 | fail-closed env read reports runtimeEnvironmentApplied: true |
killed (1) |
| M8 | DELETE route: a thrown runtime sync is reported as applied instead of failed |
survived |
| M9 | Web Shell warns on deferred/absent runtimeSync, not only failed |
killed (2) |
7. Findings
F-1 (Suggestion, already known). M8 survives — not just the workspace-models suite but all four serve suites. If syncModelProvidersRuntime() throws inside DELETE /workspace/models, the route sets runtimeSync = { status: 'failed' } (workspace-models.ts:331) and nothing asserts it; flipping that literal to 'applied' is invisible to the tests. The consequence would be a silently wrong applied for a sync that really failed, and therefore no Web Shell warning. This is the same gap already reported in the round-2/round-3 deferred lists and accepted as follow-up by the author — I am confirming it is real rather than asking for it now. The branch is hard to reach in practice (generation-closed throws are handled earlier by sendGenerationClosedError), so I am comfortable with it as follow-up.
F-2 (question, likely out of scope). On the PR arm I measured the Web Shell model picker of a live conversation:
picker BEFORE install -> alpha-model
POST /workspace/auth/provider -> 200 runtimeSync applied
picker AFTER install, NO page reload -> alpha-model <- unchanged
picker AFTER a page reload -> beta-model | alpha-model
The daemon side is correct throughout — the REST switch to beta-model(openai) succeeds immediately in that same session. What does not happen is a client-side refetch of the model list when the install arrives from outside this browser tab (another client, the SDK, or curl), even though a settings_changed event is delivered on the session stream. The in-UI install path presumably refreshes the app's own state, which is what the PR description describes, so I read this as out of scope rather than a defect — main behaves the same way. Worth one line from the author confirming that reading; a follow-up that refetches the model list on settings_changed would close the loop for multi-client setups.
No blocking issues found. I did not reproduce any of the failure modes the earlier rounds worried about: no lost persistence, no HTTP error on a degraded sync, no partially-initialized session escaping with a stale registry under concurrency, and no implicit model switch of a live session.
Summary
| Fixes #10184 | ✅ reproduced on the base, fixed on the PR, verified over REST and in a real browser |
| Reviewer test plan | ✅ all six steps reproduce, plus a concurrency case the plan does not cover |
| Earlier F1 finding | ✅ closed — verified at the wire level in a second workspace |
| Round-8 Critical fixes | ✅ both verified (R8-2 end-to-end, R8-1 by mutation) |
| Regressions found | none |
| Test strength | 5594 tests pass; 8 of 9 mutants killed |
| Open items | one confirmed coverage gap (already deferred), one Web Shell refresh question |
| Recommendation | merge-ready from my side; F-1 and F-2 as follow-ups |
Harness details
- Both arms are real
npm run bundleartifacts run asnode dist/cli.js serve, not unit-test doubles. BASE =fe34a5cf22(merge base), AFTER =eca0424f56(current head).packages/acp-bridgeneeded anpm run buildfirst in both arms or esbuild cannot resolveisScheduledTaskRunSource. - Daemon: isolated
HOME,--token,--workspace ws-main --workspace ws-second,--channel-idle-timeout-ms 600000(otherwise the channel is torn down between requests and per-child state never survives),--allow-private-auth-base-url(required for a127.0.0.1providerbaseUrl— without it the install route returns 400invalid_base_url). - Provider fixture:
alpha-modelpre-installed at user scope pointing at MOCKA:4501;beta-modelinstalled at runtime pointing at MOCKB:4502. Each mock logs{label, model, auth}per call and prefixes its reply with its own label, soMOCKB:...in a transcript is proof of routing, andBearer beta-secret-v1in the log is proof of credential propagation. - Fault injection is real, not mocked: the workspace
.envis replaced by a directory so the parent-environment refresh genuinely fails closed. - The ACP child is killed with a real
SIGKILL(ps --ppid <daemon>→ the--acpchild) to test the replacement-child path. - Browser: headless Chromium against the Web Shell the daemon itself serves, authenticated with
#token=. - Every scenario runs against a freshly reset daemon, home and workspaces.
中文版报告
PR #10269 运行时验证报告
我把这个 PR 构建成真实的 qwen serve bundle，驱动了实际运行的 daemon、它的 ACP child，以及由它托管的 Web Shell，并与 merge base 构建的 bundle 做 A/B。两个 mock OpenAI 兼容服务分别扮演两个 provider，各自记录被调用时的 model 与 Authorization 头，并在回复中带上自己的标签——因此每一次 turn 都能证明究竟是哪个 provider、用哪个凭据真正提供了服务，而不只是「请求返回了 200」。
结论：修复是真实有效的，Reviewer 测试计划六步全部可复现，未发现回归。 变异测试确认了一处测试覆盖缺口（与本 PR 上已报告并被 defer 的发现一致），另有一个 Web Shell 刷新行为值得作者一句说明。值得一提的是，评审 bot 在最近几轮都报告「零个测试套件得以运行」；而本地这些套件是通过的——共 5594 个。
| 臂 | 构建 |
|---|---|
| BEFORE | fe34a5cf22 —— 本 PR 与 main 的 merge base |
| AFTER | eca0424f56 —— 当前 PR head，含 round-8 修复 |
1. #10184 在 base 上可复现，PR 修复了它
Session 在安装 provider 之前创建；安装与模型切换之间不重启任何进程。
base 上模型在 GET /workspace/providers 中可见却不可用——正是 #10184 所报告的现象。PR 上同一个 live session 收到 runtimeSync.status = applied，不会被隐式切换，可以选中新 route，其下一次 turn 由新 provider 用新凭据（Bearer beta-secret-v1）作答。
浏览器中同样如此（截图见英文部分）：只刷新了页面，daemon 进程及其 ACP child 全程未重启。
2. [object Object] 通知在真实 UI 中已修复
两臂的 daemon 响应完全相同，差别只在消息提取逻辑。
3. 失败、生命周期与并发行为符合文档
对照 Reviewer 测试计划逐条核对：
| 步骤 | 结果 |
|---|---|
1–2 · 创建 session 后安装、applied、live 选中 route |
✅ HTTP 200、applied、切换 200、turn 由 MOCKB 提供 |
| 3 · 在 active turn 期间安装 | ✅ 安装 19 ms 返回；进行中的 turn 用原 generator 完成（MOCKA:SLOW-DONE）；结束后新模型可选 |
| 4 · 删除模型 | ✅ removed:true, clearedActiveModel:true, requiresRestart:false, runtimeSync applied；session 未被切换（下一次 turn 仍到 MOCKB）；旧 route 返回可读的 500；切回 alpha 正常 |
| 5 · 重建 ACP child | ✅ 对 child SIGKILL 后，替换 child 能选中新模型，并在链路上出示更新后的凭据 |
| 6 · 注入失败 | ✅ 父进程环境降级 → HTTP 200 且 runtimeSync failed，持久化未回滚，provider 仍在列表；无 live child → deferred |
| — · 并发（计划外补充） | ✅ 8 次安装与 8 次 session 创建交错：风暴期间创建的每个 session 都能选中最后安装的模型 |
4. 我此前提出的多 workspace 缺口（F1）已闭环
daemon 绑定两个 workspace、各有一个 live session 时，经非限定路由安装的 user-scope provider 现在也能到达次要 workspace 中已存在的 session，其下一次 turn 确实落到 MOCKB 并使用新 key。workspace 作用域的 provider 仍正确保持 primary-only，与作者记录的契约一致。
5. 两个 round-8 Critical 修复均在链路上得到验证
- R8-2 ——
POST /workspace/reload现在在健康重载时报告runtimeEnvironmentApplied: true，在父进程环境刷新 fail-closed 时报告false，且settings_reloadedSSE 事件携带同一字段（并带envFileReadFailed: true）。已用不可读的 workspace.env对真实 daemon 验证。 - R8-1 —— deferred activation 的 settings 重载已被测试钉住：强制
settingsReloaded = true（即跳过原子重载直接应用环境）会导致 3 个测试失败。
6. 测试与变异测试
受影响的套件本地全部通过：packages/cli 3125/3125、packages/acp-bridge 822/822、packages/core 665/665、packages/sdk-typescript 373/373、packages/web-shell 609/609，合计 5594 通过、0 失败。
针对 PR 自带套件构造了 9 个变异体，杀死 8 个（完整表格见英文部分）。唯一存活的是 M8。
7. 发现
F-1（Suggestion，已知项）。 M8 存活——不仅对 workspace-models 套件存活，对全部四个 serve 套件都存活。若 syncModelProvidersRuntime() 在 DELETE /workspace/models 内抛出，路由会设置 runtimeSync = { status: 'failed' }（workspace-models.ts:331），但没有任何测试断言它；把该字面量改成 'applied' 对测试完全不可见。后果是：真正失败的同步会被静默报成 applied，Web Shell 因此不会给出警告。这与 round-2/round-3 deferred 列表中已报告、作者已接受作为后续项的缺口是同一个——我这里只是确认它是真实存在的，而非在本轮提出要求。该分支在实践中难以触达（generation-closed 抛出会更早被 sendGenerationClosedError 处理），因此作为后续项我认为可以接受。
F-2（问题，很可能在范围外）。 在 PR 臂上我测量了一个 live 会话的 Web Shell 模型选择器：
安装前的 picker -> alpha-model
POST /workspace/auth/provider -> 200 runtimeSync applied
安装后、未刷新页面的 picker -> alpha-model <- 未变化
刷新页面后的 picker -> beta-model | alpha-model
daemon 侧全程正确——同一个 session 通过 REST 切换到 beta-model(openai) 立即成功。没有发生的是：当安装来自当前浏览器标签页之外（其他客户端、SDK 或 curl）时，客户端不会重新拉取模型列表，尽管 session 流上确实投递了 settings_changed 事件。UI 内的安装路径应当会刷新应用自身状态，这也正是 PR 描述所说的，因此我倾向认为这属于范围外而非缺陷——main 的行为是一样的。希望作者用一句话确认这一理解；后续可以考虑在收到 settings_changed 时重新拉取模型列表，以覆盖多客户端场景。
未发现阻断性问题。 此前几轮担心的失败模式我都没有复现：没有持久化丢失、降级同步没有变成 HTTP 错误、并发下没有初始化到一半且带着旧 registry 的 session 逃逸、live session 也没有被隐式切换模型。
总结
| 是否修复 #10184 | ✅ base 上复现、PR 上修复，REST 与真实浏览器双重验证 |
| Reviewer 测试计划 | ✅ 六步全部复现，另补充了计划外的并发场景 |
| 此前的 F1 发现 | ✅ 已闭环——在第二个 workspace 中于链路层面验证 |
| Round-8 Critical 修复 | ✅ 两项均验证（R8-2 端到端，R8-1 经变异测试） |
| 发现的回归 | 无 |
| 测试强度 | 5594 个测试通过；9 个变异体杀死 8 个 |
| 待办 | 一处已确认的覆盖缺口（已 defer）、一个 Web Shell 刷新问题 |
| 建议 | 从我这边看可以合并；F-1 与 F-2 作为后续项 |
|
Thanks for the thorough runtime verification. Confirming F-2 is out of scope for this PR: the supported mutation path refreshes the initiating Web Shell state and synchronizes daemon/ACP/session registries, while passive cross-client model-list refetch on |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
@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: 218 passed · 0 failed · 218 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：218 通过 · 0 失败 · 218 总计 抖动门： Verification reportPR #10269 — fix(serve): Hot-reload runtime model providers (follow-up round 8)Verdict: 中文 — 判定：✅ 通过 · 可合入（agent 判定，follow-up 第 8 轮）
Previous-finding status table (re-measured at this head, never diffed)Tree identity first: round 7 verified merge
Central claim + A/B (re-measured fresh; harness rebuilt from source)Central claim (carried): a provider installed (or model deleted) after a session exists becomes usable in that live session without restarting Base control: worktree at Official runs: head main ×3, base main ×2, envfail ×2 per arm, activeturn ×2 (head) — all cell-for-cell stable, zero infra taint. Witnesses:
Env-failure contract ( Active-turn generator retention ( CorrectionsNone this round — no prior description errors were found to stand. Findings (non-blocking; all carried from prior rounds, all re-measured)
Not covered
MethodologyEnvironment: CI merge-ref checkout ( Flakiness gate logEvidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
|
@qwen-code /triage |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Checked:
- Contract asymmetry:
reloadModelProvidersConfignow uses new-instance-then-assign (new ModelRegistry(...)→ assignmodelsByAuthType+providerProtocolConfig) — no partial-replacement possible if constructor throws. - Atomicity:
reloadScopesFromDiskAtomicallysnapshots both scopes before reload, rolls both back if eitherreloadScopeFromDiskreturns false — correctly prevents a one-scope-fails-the-other-stale split. - Env fail-closed:
reloadPrimaryDaemonEnvandreloadRuntimeOverlaySnapshotForModelProvidersboth return{runtimeEnvironmentApplied: false}early whenenvFileReadFailed— no partial env snapshot applied. - Secondary workspace coverage:
syncModelProvidersRuntimeinserver.tsfans out tosecondaryRuntimeswhenscope === SettingScope.User— user-scoped provider mutations now reach all active workspaces. - Client timeout:
DEFAULT_PROVIDER_MUTATION_TIMEOUT_MS = 0(no deadline) is applied only when!hasExplicitFetchTimeout— explicit client timeouts preserved. - Stale-completion race:
handleDeleteModelandAuthMessage.saveboth carrysessionOwnerGuard.capture()+isCurrent()guards before dispatching status — stale completions silently drop. - Partial-persistence path:
workspace-models.tscatch forWorkspaceSettingsPartialPersistErrornow callssyncModelProvidersRuntimewhenmodelProvidersis amongcommittedWrites— tested inworkspace-models.test.ts. - Broadcast-before-sync ordering:
broadcastWriteloop executes beforesyncModelProvidersRuntime— confirmed by the invocation-order assertion in the new test. - Pre-publication refresh loop:
createAndStoreSessionwhile-loop breaks only whenproviderReloadRevision === this.modelProviderReloadRevision—beforeSessionPublish(and the response built therein) sees the final consistent registry. reloadModelProvidersfacade: seven test cases infacade.test.tscover applied/deferred/failed/rejected parent-env/generation-closed paths, plus thereloadModelProvidersDaemonEnv-preferred-over-reloadDaemonEnvpath.
Round 3 criticals cross-checked — all resolved at this head: R3-1 (partial Config), R3-2 (pre-publication race), R3-3 (one-scope rollback), R3-4 (partial-persist sync), R3-5 (env fail-closed), R3-6 (secondary workspaces), R3-7 (explicit timeout), R3-8 (stale completion).
Not covered (CI gaps — not a reason to withhold):
build-clicheck is SKIPPED in CI — no CI build/typecheck/lint evidence. Author reportsnpm run build,npm run typecheck,npm run lintclean locally on macOS (Node 22.22.3).- Integration Tests (CLI, No Sandbox) SKIPPED.
- Windows and Linux not tested.
Reviewed with AI assistance.
|
Sandboxed verification: Skipped because the PR is not open for verification (state=MERGED, draft=false). 中文 — 判定：
|
5 participants
What this PR does
This PR adds a dedicated runtime synchronization path for model-provider mutations. After a provider is installed or a model is deleted, the daemon rebuilds the parent environment used by future ACP children, asks the active workspace child to reload persisted settings and environment, refreshes bootstrap, initializing, and live Session model registries, and clears per-model generator caches without switching any Session's current model or disturbing a generator already held by an active turn.
Mutation responses now expose an optional
runtimeSyncresult withapplied,deferred, orfailedstatus. A failed runtime refresh does not roll back persisted settings or turn the successful mutation into an HTTP error; the Web Shell instead reports that the change was saved and that running sessions may require a retry or daemon restart. JSON-RPC model-switch failures also use the existing structured error-message extractor so object errors no longer render as[object Object].The synchronization contract, state ownership, ordering, Session semantics, and failure behavior are documented as part of the change.
Why it's needed
The Web Shell currently persists and immediately lists newly installed models, but a Session created before the mutation keeps a stale in-memory model registry. Selecting the visible route therefore fails until
qwen serveis restarted. The same stale-state gap affects deletion and child replacement, and structured JSON-RPC errors make the failure difficult to understand. This change keeps persisted settings, future-child spawn state, the live ACP child, and every Session registry consistent without broadening the work into a general settings watcher or a full workspace reload.Reviewer Test Plan
How to verify
qwen servewith a disposable workspace, then create a Session before installing the stub provider.runtimeSync.status = applied; read the route from workspace provider status and select it in the pre-existing Session without restarting the daemon.runtimeSync.status = failed; confirm the Web Shell warns only forfailed, notapplied,deferred, or a missing field from an older daemon.Evidence (Before & After)
Before: a provider installed after Session creation appeared in workspace provider status, but selecting its route returned HTTP 500 (
Model ... not found in the registry) and the matching SSE notice displayed[object Object]; restarting the daemon made the route work.After: the same live Session received
runtimeSync.status = appliedand selected the new route immediately. An active-turn run retained its original generator until completion, deletion removed future route selection without switching the Session, and a rebuilt ACP child used the updated credential. Parent environment degradation preserved the successful mutation and returned HTTP 200 withruntimeSync.status = failed.Tested on
Environment (optional)
Node.js 22.22.3, npm 10.9.8, macOS, local HTTP
qwen serve, disposableQWEN_HOMEand workspace, and a local OpenAI-compatible stub. Verified with the affected CLI, ACP bridge, core, SDK, and Web Shell Vitest suites, plusnpm run build,npm run typecheck, andnpm run lint.Risk & Scope
runtimeSyncis additive and optional, so new SDK clients remain compatible with older daemons.Linked Issues
Closes #10184
中文说明
本 PR 做了什么
本 PR 为模型 Provider 变更新增专用运行时同步链路。安装 Provider 或删除模型后，daemon 会重新构建供后续 ACP child 使用的父进程环境，通知当前 workspace child 从已持久化 settings 重载配置与环境，刷新 bootstrap、初始化中以及所有 live Session 的模型注册表，并清理 per-model generator cache；整个过程不会切换任何 Session 的当前模型，也不会影响 active turn 已持有的 generator。
变更接口现在返回可选的
runtimeSync结果，状态为applied、deferred或failed。运行时刷新失败不会回滚已持久化的 settings，也不会把成功的变更误报为 HTTP 错误；Web Shell 会提示配置已保存，但运行中的 Session 可能需要重试或重启 daemon。JSON-RPC 模型切换失败也改为复用现有结构化错误消息提取逻辑，不再把对象错误展示成[object Object]。本次变更同时记录了同步契约、状态所有权、执行顺序、Session 语义和失败行为的设计文档。
为什么需要
当前 Web Shell 在安装模型后会立即持久化并展示新模型，但变更前已创建的 Session 仍持有旧的内存模型注册表，因此选择已经可见的 route 会失败，直到重启
qwen serve。删除模型和 child 重建也存在同类状态不一致，同时结构化 JSON-RPC 错误让问题难以理解。本次修改在不扩展为通用 settings watcher、也不触发完整 workspace reload 的前提下，使持久化 settings、后续 child 的 spawn 状态、当前 ACP child 以及所有 Session registry 保持一致。Reviewer 测试计划
如何验证
qwen serve；在安装 stub Provider 前先创建一个 Session。runtimeSync.status = applied；从 workspace Provider 状态读取 route，并在不重启 daemon 的情况下让已有 Session 选择该 route。runtimeSync.status = failed；确认 Web Shell 只对failed警告，对applied、deferred和旧 daemon 缺失字段均不警告。证据（变更前后）
变更前：Session 创建后再安装的 Provider 会出现在 workspace Provider 状态中，但选择其 route 会返回 HTTP 500（
Model ... not found in the registry），对应 SSE notice 显示[object Object]；重启 daemon 后同一路由才可使用。变更后：同一个 live Session 收到
runtimeSync.status = applied后可立即选择新 route。active-turn 验证中当前 turn 一直保留原 generator，删除只禁止后续 route 选择而不切换 Session，ACP child 重建后也使用了更新后的凭据。父进程环境降级时，变更仍成功持久化，并以 HTTP 200 返回runtimeSync.status = failed。测试平台
环境（可选）
Node.js 22.22.3、npm 10.9.8、macOS、本地 HTTP
qwen serve、临时QWEN_HOME与 workspace，以及本地 OpenAI-compatible stub。已验证受影响的 CLI、ACP bridge、core、SDK 和 Web Shell Vitest 测试，并执行npm run build、npm run typecheck与npm run lint。风险与范围
runtimeSync是 additive optional 字段，因此新 SDK 客户端仍可兼容旧 daemon。关联 Issue
Closes #10184