Repository navigation
feat(serve): batch workspace session live-state snapshots - #12513
XIQIXIQIXIQI wants to merge 15 commits into
Conversation
github-actions
Bot
added
the
review/self-reported
E2E test report (macOS arm64, Node 22.23.1)The bundled CLI at
The globally installed |
Linux validation on
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Code review: no blocking findings in the implementation itself (see below), but CI Test (ubuntu-latest) is red on this head, and the failure belongs to this PR — so this needs a fix before approval.
The failure
Two existing contract tests fail at 8cc1567, both pointing at the same gap — the new route was added to the code and to qwen-serve-protocol.md, but its companion contract surfaces were not all updated:
-
telemetry-catalog.test.ts > matches the explicit Express route registrations in both directions—expected […(74)] to have a length of 74 but got 75. The test hard-codes the registered-route count at line 106 (expect(registered).toHaveLength(74)); the newPOST /sessions/live-stateregistration makes it 75. (The sibling hard-code inserver/telemetry.test.tswas updated to 75; this one was missed.) -
rest-integration-docs-contract.test.ts > indexes every operation with a dedicated protocol section—expected […(76)] to deeply equal […(77)].qwen-serve-protocol.mdgained the### \POST /sessions/live-state`section (77 protocol operations), butdocs/developers/daemon-rest-api-reference.md` has no link row for it (76 reference links).
Fixing #2 is not just adding a row to the reference table: the neighboring test keeps the published reference index in step with the OpenAPI document requires rows.length === operations.size and every row's capability/scope/sdk-method cells to equal the OpenAPI operation's extension fields. So the consistent fix is three surfaces in sync:
docs/developers/daemon-rest-api.openapi.json— add thePOST /sessions/live-stateoperation, withx-qwen-capability: workspace_session_live_state_batch, the matchingx-qwen-scope,x-qwen-sdk-method(getSessionsLiveState), andexternalDocspointing at the new protocol anchor.docs/developers/daemon-rest-api-reference.md— add the corresponding table row (link + the three cells).packages/cli/src/serve/server/telemetry-catalog.test.ts:106—74→75.
What I verified in the implementation (all clean)
- The batch handler is strict and well-isolated: zod
.strict()envelope (1–20 selectors, 4096 chars, extra keys rejected), per-member error isolation with the right status mapping, a TOCTOU re-check that fails closed to 503 when the generation is replaced mid-read (and a test that exercises exactly that), sequential reads withreq.aborted/res.destroyedbail-outs, 512 KiB member cap,Cache-Control: no-store. - The trust model is the strict one: untrusted primary and secondary get 403 — it does not inherit the permissive persisted-catalog policy, and unknown/internal selectors never fall back to primary. Internal-workspace exclusion has its own test asserting the bridge is never read.
- The extraction of
readWorkspaceLiveStatefrom the single-workspace handler is behavior-identical, and both routes sharinglastExposedCatalogVersionspreserves the catalog-reconciliation contract (the amended 7447-region test pins this). - Rate-limit classification (
readtier, anchored regex), telemetry attribution (handler_resolved, per-member workspace hash), capability registration, and the SDK method (one native REST request, no fan-out, signal/timeoutMs not serialized, older-daemon 404 surfaces as-is) all match the design doc and have tests. - The protocol and SDK docs, capabilities lists, and integration test were updated consistently.
Non-blocking notes for when you undraft: macOS intermittent failures you mentioned in the body are worth a second look if they reappear in CI, and there is no model-driven live session in the HTTP E2E (the in-memory read path is covered by the bridge mocks, so this is acceptable). The 512 KiB member check serializes twice (measure + res.json) — bounded and fine.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Thanks — this is in good shape structurally: the trust gate is strict per member (unknown/internal → 404, untrusted primary and secondary → 403, generation replaced mid-read → 503, never a fallback to primary), the shared projection/cache-invalidation contract with the single-workspace route is preserved and test-pinned, and the bounds (1–20 selectors, 4096 chars, 512 KiB per member with an explicit 413, no-store, sequential reads with disconnect bail-outs) all behave as the design doc says. The SDK method is a single native REST request with cancellation and no hidden fan-out. Two things before this can land:
- CI is red at
8cc1567and the failure belongs to this PR (already flagged in the previous review round, re-verified at head):telemetry-catalog.test.ts:106still expects 74 registered routes while the siblingserver/telemetry.test.tswas bumped to 75, and the docs contract test fails becausePOST /sessions/live-statehas a protocol section but no operation indaemon-rest-api.openapi.json(with itsx-qwen-capability/x-qwen-scope/x-qwen-sdk-methodcells) and no row indaemon-rest-api-reference.md. - The rebase changes the numbers. Since this branch's base, main landed
POST /session/:id/mcp-app/tools/call, so current main already counts 75 routes — after rebasing, the combined tree is 76, and both telemetry tests (telemetry-catalog.test.tsandtelemetry.test.ts) should assert 76 with the 74/2 attribution split. The capability baselines (server.test.ts,integration-tests/cli/qwen-serve-routes.test.ts) likewise need the union with the five capabilities main gained (daemon_update,hosted_harness_private_v1,session_branch_worktree,session_startup_config,workspace_git_worktrees). Resolving the conflict as a plain "keep both" without this adjustment will land red.
Non-blocking: the member size check serializes twice (measure + res.json) — bounded and fine as-is; and the macOS intermittent failures you noted in the PR body are worth a second look if they reappear in CI.
结构上没有大问题：信任门禁按成员严格生效（未知/内部 → 404，不可信的 primary 和 secondary 都是 403，读取途中 generation 被替换 → 503，任何情况下都不回退到 primary），与单 workspace 路由共用的投影与缓存失效契约保持且有测试钉住，资源边界（1–20 个 selector、4096 字符、单成员 512 KiB 显式 413、no-store、顺序读取 + 断连退出）与设计文档一致。SDK 方法是单次原生 REST 请求，支持取消，没有隐藏的扇出。合入前有两件事：
- CI 在当前 head（
8cc1567）是红的，且失败属于本 PR（上一轮评审已指出，我在 head 上重新核实）：telemetry-catalog.test.ts:106仍断言 74 条注册路由，而姊妹文件server/telemetry.test.ts已改为 75；文档契约测试失败是因为POST /sessions/live-state有协议章节，但daemon-rest-api.openapi.json中没有对应 operation（含x-qwen-capability/x-qwen-scope/x-qwen-sdk-method三个扩展字段），daemon-rest-api-reference.md中也没有对应行。 - rebase 会改变数字。 自本分支的 base 以来，main 已合入
POST /session/:id/mcp-app/tools/call，当前 main 的路由计数已是 75——rebase 之后合并树是 76 条，两个遥测测试（telemetry-catalog.test.ts与telemetry.test.ts）都应断言 76（74/2 归属比例）。能力基线（server.test.ts、integration-tests/cli/qwen-serve-routes.test.ts）同样需要与 main 新增的五个能力（daemon_update、hosted_harness_private_v1、session_branch_worktree、session_startup_config、workspace_git_worktrees）取并集。如果按简单的「两边都保留」解冲突而不做这些调整，合入后依然是红的。
非阻塞：成员大小检查会序列化两次（测量 + res.json）——有界，保持现状即可；另外你在 PR 描述里提到的 macOS 间歇性失败，如果 CI 复现值得再看一眼。
|
@qwen-code /resolve |
Both sides added one handler-resolved legacy session telemetry route: main added POST /session/:id/mcp-app/tools/call and this branch added POST /sessions/live-state. Each independently bumped the audited catalog from 74/72 to 75/73, so Git merged the identical assertion text cleanly while the merged catalog actually holds 76 routes at 74/2. Re-audit the counts and widen the explanatory comment to cover both routes.
|
Qwen Code resolved the merge conflicts and pushed the branch update. Root cause
Semantic, not textualThe only marker was #12258's new comment (HEAD side empty). Git auto-merged the numbers — both sides made the byte-identical edit it('contains 76 unique routes with the audited 74/2 attribution split', () => {
expect(keys).toHaveLength(76);
expect(new Set(keys).size).toBe(76);
/* handler_resolved */ ).toHaveLength(74);
/* pre_resolved */ ).toHaveLength(2); // unchangedLoad-bearing
Follow-up needed — not done here (file did not conflict)
No build, lint or tests run. 中文说明根因： 语义冲突，非文本冲突：唯一标记是 #12258 新增的注释（HEAD 侧为空）。数字被 Git 自动合并——两侧做了逐字节相同的 关键约束： 需后续修改——本次未处理（该文件未冲突）： 未运行构建或测试。 |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Resolve conflicts by keeping the extracted readWorkspaceLiveState helper and porting main's agent-host sourceType filter into it so both live-state routes share the behavior, and reconcile the telemetry route guards to the combined 80 routes with the 78/2 attribution split. Co-authored-by: Qwen-Coder <[email protected]>
…12513) Index POST /sessions/live-state in the REST reference's persisted-catalog row so the docs contract sees the protocol section, give the batch route the single-workspace route's server-side records (routing-failure warn on untrusted members, daemonLog error on unexpected bridge failures), make the mid-batch disconnect guard observable with an AbortController and a macrotask yield between members, and move the duplicated member-failure, generation-currency, and bound literals into workspace-route-runtime for both plural batch routes. Six tests pin the new witnesses. Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #12513 (batch workspace session live-state)Two commits this round: a merge of Base-conflict resolution (
|
…12513) Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #12513All six inline suggestions from the round-2 review are addressed in commit Findings and dispositionsR2-1 (rc:4152008465) — contradictory scope labels in the grouped catalog row — implemented
Verified against the code before editing: R2-2 (rc:4152008469) — grouped rows' capability/SDK cells unpinned — implemented
Probe: corrupting R2-3 (rc:4152008472) — envelope assertion only reachable via CI integration jobs — verified, no source changeRan the exact gate locally against a freshly built bundle: the full R2-4 (rc:4152008477) — post-loop abort guard unwitnessed — implementedNew test Probe: deleting the post-loop guard turns exactly this test red; R2-5 (rc:4152008481) —
|
|
🔀 Base updated: red check(s) [Lint & Static (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Lint & Static (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
…n shifts Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #12513Commit Feedback dispositionsR3-1 — grouped-row SDK check skipped bare method names (rc:4153835870, rc:4156815933) — resolvedReproduced first: on the pre-round test, renaming the bare doc token Fix, combining both reviewer prescriptions: the SDK regex's class prefix is now optional ( R3-2 — hardcoded column indices could silently vacate both loops (rc:4153835887, rc:4156816262) — resolvedReproduced first: replaying the reviewer's splice (an extra column before "Capability and scope") against the pre-round test left Fix: the two column indices are derived from the grouped table's own header row ( R2-3 — integration lane never executed the envelope assertion (rc:4153835899) — no code change; thread left open with a replyThe finding itself states no source change is needed. Verified locally that The reviewer-deferred probe ( Mutation probes (guard witnesses)All probes mutated
Changed files
Verification
中文说明Autofix 本轮处理 — PR #12513提交 发现处理结果R3-1 —— 分组行的 SDK 校验跳过裸方法名（rc:4153835870、rc:4156815933）—— 已解决先复现：在本轮之前的测试上，把 修复（综合评审者两轮给出的方案）：SDK 正则中的类名前缀改为可选（ R3-2 —— 硬编码列下标可能让两个循环静默空转（rc:4153835887、rc:4156816262）—— 已解决先复现：按评审者的做法重放（在 "Capability and scope" 之前插入一列），本轮之前的测试中 修复：两个列下标改为从分组表格自身的表头行（ R2-3 —— 集成通道从未执行过 envelope 断言（rc:4153835899）—— 无需改代码；线程保持开放并已回复该发现本身写明不需要修改源码。已在本地验证 评审者自行延后的探针（ 变异探针（防护见证）所有探针只改动
改动文件
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 2 selected review thread(s). · 已关闭全部选中的 2 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…lifiers Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #12513Commit rc:4159484825 (R5-1) — grouped-rows pin fails open at both granularities — IMPLEMENTED
rc:4159484840 (R3-1) —
|
|
🔀 Base updated: red check(s) [Lint & Static (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Lint & Static (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action requiredNo code change was made this round. Every item in this round's feedback was triaged and none is actionable, for the reasons below. What arrived since the last evaluation
Evidence checked
No build/test commands were run because this round makes no code change and the authoritative CI at the exact current head is green; the workflow's independent CI remains the final gate. 中文说明Autofix 审查轮次 —— 无需处理本轮未做任何代码修改。本轮反馈中的每一项都已分类处理，均不可执行，原因如下。 自上次评估以来收到的内容
已核查的证据
本轮未运行构建/测试命令，因为本轮不做任何代码修改，且针对当前确切 head 的权威 CI 已全部通过；工作流的独立 CI 仍是最终验证关口。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
🔬 Local verification round (maintainer, sandboxed worktrees)Verdict: merge-ready — 52/52 scripted assertions passed, 0 unexpected failures. Verified head 中文摘要结论:merge-ready —— 52/52 项脚本断言全部通过。
Central claim
A/B proof (real daemons, real loopback HTTP)Identical harness (
Totals: head 23/23, base 6/6 (base 404 cells are the expected-failure controls, encoded as assertions). Witness:
Wire-oracle harnessesSDK against the real daemon (
Trust gate (
Mutation matrix (vacuity check on the PR's new tests)Ran against
Adjudicated survivor: under M3, "fails closed if the selected runtime is replaced during its read" stays green. Verified cause, not assumed:
Gates
FindingsNone blocking. Two non-blocking observations:
Not covered
MethodologymacOS arm64, Node 24.18.1. Two worktrees at the resolved |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round — no change requiredNew feedback this round: one issue-level comment, the maintainer's sandboxed local verification round ([ic:5965045999], @wenshao). Verdict: merge-ready — 52/52 scripted assertions passed against head CI: all checks on the current head are green (SUCCESS or SKIPPED; no FAILURE). The "Failed checks" and "Still-red checks" sections are empty; the CI failures noted in the round-6 review were resolved by the merge with main in Disposition of the verification comment's two non-blocking observations:
No code changes, no commits this round. 中文说明Autofix 轮次 —— 无需改动本轮新增反馈: 一条 issue 级评论,即维护者的沙箱本地验证轮([ic:5965045999],@wenshao)。结论:merge-ready(可合并) —— 针对 head CI: 当前 head 上的所有检查均为绿色(SUCCESS 或 SKIPPED;无 FAILURE)。"Failed checks" 与 "Still-red checks" 两节均为空;第 6 轮评审中提到的 CI 失败已由 验证评论中两条非阻塞观察的处理:
本轮无代码改动、无提交。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
…-live-state # Conflicts: # packages/cli/src/serve/capabilities-docs-contract.test.ts
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #12513Feedback triage
Merge-conflict resolution (
|
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "workspace_session_live_state_batch" |
health-deep-with-session
| field | PR base (before) | this PR (after) |
|---|---|---|
activeWorkStaleMs |
5 |
6 |
— Qwen Code · serve A/B
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": did not execute the new batch workspace session live-state route tests myself; pass/fail and flake status rest on the prior round's measured build/test result….
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：did not execute the new batch workspace session live-state route tests myself; pass/fail and flake status rest on the prior round's measured build/test result…。
— qwen3.8-max via Qwen Code /review (v0.25.0)
|
🤖 AutoFix ran out of time before finishing (timeout (3600000ms)) (attempt 6/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (3600000ms)）（第 6/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/37609284075 🧠 Handled by Qwen Code · model/模型 |
…ift (#12513) Co-authored-by: Qwen-Coder <[email protected]>
The autofix verification gate runs the packages/cli suite with the host's real HOME on a machine shared with other jobs. A malformed $HOME/.qwen/settings.json there (observed content: "X") makes every operator-settings reader fail closed at once — readOperatorSettingsScopes is designed to refuse a corrupted policy file — and one such file failed 408 tests across 15 files in the measured gate rejection. Pin a private empty QWEN_HOME per test-file process in the suite setup when the environment did not select one, following the file's existing doctrine for operator-owned environment. Tests that exercise QWEN_HOME set, stub, or delete it themselves. settings.test.ts keys every user-settings and .env path off a mocked os.homedir, so it now deletes an ambient QWEN_HOME explicitly; with any ambient value set, 13 of its tests fail regardless of this change.
|
🤖 Addressed the latest review feedback (round 7/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 7/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #12513 (verification repair)Commit: This was a Same-run verification repair round: the previous commit Root cause (evidence, not a guess)
Fix (two test-infrastructure files, both inside the PR's packages/cli footprint)
No production code changed; no assertion was removed or weakened (one Mutation probe
Feedback dispositions
Conflict notes
VerificationAll run on this tree, in a gate-like environment (agent-specific
Note on scope: because this round touches 中文说明Autofix 轮次总结 — PR #12513（验证修复）提交： 本轮是同轮验证修复：上一个提交（ 根因（基于证据，而非猜测）
修复（两个测试基础设施文件，均在 PR 的 packages/cli 足迹内）
未改动任何生产代码；未删除或削弱任何断言（只新增了一条 变异探测
反馈处置
冲突说明
验证以下均在本树上、类线上环境中执行（清除 agent 特有的
范围说明：由于本轮改动了 🦷 Gate advisory — this round's changed tests all pass on the pre-round tree (machine-measured, not agent-authored). Expected for a refactor or coverage addition; if this round was meant to FIX a defect, that defect did not reproduce. · 本轮改动的测试在轮前树上全部通过（门自动测量，非 agent 文本）。对重构或补充覆盖属正常；若本轮意在修复缺陷，则该缺陷未能复现。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 2 selected review thread(s). · 已关闭全部选中的 2 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1c": full packages/cli suite run under the new QWEN_HOME pin — sampled the 21 highest-risk files (~1,070 tests) instead of all ~1,000 files.; "agent reverse-audit (round 1)": I did not empirically mutate-test the new guards (removing if (req.aborted || res.destroyed) abort() , the post-loop abort check, or the MAX_MEMBER_BYTES bra…; "agent 4": none — I stopped at ~16 tool calls with every candidate above resolved by inspection..
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session.ts:9785 — [review] The batch member's 500 path skips the shared bridge-error…
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 1c"：full packages/cli suite run under the new QWEN_HOME pin — sampled the 21 highest-risk files (~1,070 tests) instead of all ~1,000 files.；"agent reverse-audit (round 1)"：I did not empirically mutate-test the new guards (removing if (req.aborted || res.destroyed) abort() , the post-loop abort check, or the MAX_MEMBER_BYTES bra…；"agent 4"：none — I stopped at ~16 tool calls with every candidate above resolved by inspection.。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.25.0)
- Pin on truthiness so an ambient empty QWEN_HOME no longer falls through to $HOME/.qwen, matching every production reader. - Sweep the pinned home from afterAll instead of an 'exit' listener that vitest workers never fire (51 leaked dirs on src/config -> 0). - Restore QWEN_DEBUG_LOG_FILE around loadCliConfig tests so the debug logger's fire-and-forget writes stop racing the sweep. - Witness that the suite never resolves settings from the ambient $HOME/.qwen, and assert member shape in the 20-selector maxima half so twenty failed members cannot read as twenty snapshots.
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Autofix round — PR #12513 (verification repair)This round was a same-run verification repair: the previous commit ( No review feedback was actionable this round (Critical-only mode; the deferred section was empty; all four inline Suggestions were resolved in the previous round and are re-verified holding below). Root cause (probed, not guessed)Reproduced the gate failure locally with the pin active ( Instrumented probes (temporary, reverted) established the exact mechanism:
Any in-worker deletion of the pinned tree is therefore inherently racy for this file: the writer is fire-and-forget production behavior that cannot be observed or awaited from the setup file, and vitest workers are terminated without an Fix
Also added a witness test ( Mutation probes (each reverted after measurement):
Residual housekeeping: the file still leaves one Prior findings re-verified holding
Failed checks
Verification
No settings source changed, so 中文说明Autofix 本轮处理 — PR #12513（验证修复）本轮是一次同轮验证修复：上一个提交（ 本轮没有可执行的评审反馈（仅 Critical 模式；延后区段为空；4 条行内 Suggestion 已在上一轮解决，并在下方重新验证仍然成立）。 根因（探针实测，非猜测）在 pin 生效的条件下（ 通过（临时、已还原的）埋点探针确认了确切机制：
因此，对该文件而言，worker 内对被 pin 目录树的任何删除本质上都是竞态：写入方是 fire-and-forget 的生产行为，setup 文件无法观察也无法等待它；而且 vitest worker 被终止时不会触发 修复
同时新增了一个见证测试（ 变异探针（每个均在测量后还原）：
残留 housekeeping：该文件每次运行仍会留下一个 既有发现重新验证仍然成立
失败的检查
验证
未改动任何 settings 源文件，故无需运行 🦷 Gate advisory — this round's changed tests all pass on the pre-round tree (machine-measured, not agent-authored). Expected for a refactor or coverage addition; if this round was meant to FIX a defect, that defect did not reproduce. · 本轮改动的测试在轮前树上全部通过（门自动测量，非 agent 文本）。对重构或补充覆盖属正常；若本轮意在修复缺陷，则该缺陷未能复现。 Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 4 selected review thread(s). · 已关闭全部选中的 4 条评审线程。 🩸 Regression charged to round 7: that round pushed onto a head whose checks were all green and left them red. It no longer counts as progress for the consecutive-failure brake. · 已将回归记在第 7 轮：该轮在检查全绿的 head 上推送后检查转红，因此不再计入连续失败熔断的有进展判定。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": full packages/cli suite run to measure the new QWEN_HOME pin's blast radius beyond the ten files I targeted individually (a single whole-package run exceeds…; "agent 5": I did not execute packages/cli tests or any mutation (no run performed) — all mutation verdicts above are read-based..
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/multi-workspace-sessions.test.ts:7566 — [review] the invalid-envelope enumeration omits the empty-selector case, leaving the schema's element-level min(1) unpinned
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 1a"：full packages/cli suite run to measure the new QWEN_HOME pin's blast radius beyond the ten files I targeted individually (a single whole-package run exceeds…；"agent 5"：I did not execute packages/cli tests or any mutation (no run performed) — all mutation verdicts above are read-based.。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| it('keeps the resolved user settings dir off the ambient $HOME/.qwen', () => { | ||
| expect(getUserSettingsDir()).not.toBe(path.join(homedir(), '.qwen')); |
There was a problem hiding this comment.
[Suggestion] R2-2: (fix-induced) The witness this round added to answer R2-2 asserts more than the guard it witnesses. test-setup.ts pins a private home only when QWEN_HOME is falsy, but this assertion demands unconditionally that the resolved settings directory differ from path.join(homedir(), '.qwen'). When an environment legitimately selects a QWEN_HOME that resolves to that same default — ~/.qwen, $HOME/.qwen, or a trailing-slash spelling, all of which Storage.resolvePath normalizes to the identical string — the pin correctly stands aside, both sides compare equal, and the whole packages/cli suite goes red with a message that reads as an isolation breach while the guard is behaving exactly as designed. That is precisely the case the header comment above this test promises is safe: it claims the assertion holds whether the pin applied or the environment legitimately selected its own QWEN_HOME. The in-tree carrier today is .github/workflows/qwen-triage.yml:4416-4418, which runs the verify agent with HOME=$AGENT_HOME and QWEN_HOME=$AGENT_HOME/.qwen; no CI job that runs this suite carries the coincidence yet (ci.yml:2119-2123 does pair them, but only for typecheck:integration and test:integration:no-ak, and integration-tests has no setupFiles entry pointing at packages/cli/test-setup.ts), so this is a false alarm waiting for an operator shell profile or a new job rather than a live red. The same assertion is also vacuous in the other direction: with an empty HOME, os.homedir() returns '', the left side resolves to <tmpdir>/.qwen while the right side is '.qwen', so it passes whether or not the pin applied.
Witness:
ARM 1 HOME=/tmp/hh, QWEN_HOME unset:
Test Files 1 passed (1)
Tests 1 passed (1)
ARM 2 HOME=/tmp/hh QWEN_HOME=/tmp/hh/.qwen:
x test-setup QWEN_HOME pin > keeps the resolved user settings dir off the ambient $HOME/.qwen
AssertionError: expected '/tmp/hh/.qwen' not to be '/tmp/hh/.qwen' // Object.is equality
Test Files 1 failed (1)
Tests 1 failed (1) known-fail rc=1
Same environment, sibling suites unaffected:
src/config/path-freshness.test.ts (3 tests) and src/config/settings.test.ts (239 tests) pass
-> Tests 1 failed | 242 passed (243)
Empty-homedir arm (pin absent, real resolver):
homedir() = ""
LHS getGlobalQwenDir() = "<tmpdir>/.qwen"
RHS join(homedir,.qwen) = ".qwen"
witness .not.toBe(rhs) would PASS (vacuous)
control, HOME=/tmp/hh: LHS = "/tmp/hh/.qwen" RHS = "/tmp/hh/.qwen" -> FAIL (witness bites)
| it('keeps the resolved user settings dir off the ambient $HOME/.qwen', () => { | |
| expect(getUserSettingsDir()).not.toBe(path.join(homedir(), '.qwen')); | |
| it('keeps the resolved user settings dir off the ambient $HOME/.qwen', () => { | |
| const resolved = getUserSettingsDir(); | |
| if (resolved === path.join(homedir(), '.qwen')) { | |
| // Reaching the ambient default is legitimate only when the environment | |
| // selected it itself; the pin stands aside for a truthy QWEN_HOME. | |
| expect(process.env['QWEN_HOME']).toBeTruthy(); | |
| } |
Any guard here has to compare the resolved value using the same truthiness test the pin uses, never !== undefined: packages/core/src/config/storage.ts:194-196 is const envDir = process.env['QWEN_HOME']; if (envDir) { return Storage.resolvePath(envDir); }, and resolvePath expands a leading ~. packages/cli/src/config/path-freshness.test.ts:26-30 also deletes QWEN_HOME in beforeEach and asserts the default at :49, so the pin must stay a single setup-file assignment that a test can delete rather than something re-applied from a hook. Exporting the pinned directory from test-setup.ts and asserting against it would additionally close the empty-HOME vacuity and keep a positive assertion in the common case, at the cost of a little module surface.
Please confirm the mutation still bites after the change: with QWEN_HOME unset, delete the if (!process.env['QWEN_HOME']) { … } block from test-setup.ts:67-76 and check this test goes red, and check the QWEN_HOME=$HOME/.qwen run goes green.
中文说明
[Suggestion] R2-2：（由修复引入）本轮为回应 R2-2 而新增的见证测试，断言的范围超出了它所见证的那个守卫。test-setup.ts 只在 QWEN_HOME 为假值时才固定一个私有 home，但这条断言无条件要求解析出的 settings 目录不等于 path.join(homedir(), '.qwen')。当环境合法地把 QWEN_HOME 选为这个默认位置时——~/.qwen、$HOME/.qwen，或带尾斜杠的写法，Storage.resolvePath 会把它们全部规范化为同一个字符串——pin 会正确地不介入，两侧比较相等，于是整个 packages/cli 测试套件变红，报错信息读起来像是隔离被破坏，而守卫其实完全按设计工作。这正是本文件上方注释承诺安全的场景：注释声称该断言「无论 pin 是否生效、或环境是否合法地选择了自己的 QWEN_HOME，都成立」。目前仓库内的触发点是 .github/workflows/qwen-triage.yml:4416-4418，它以 HOME=$AGENT_HOME 和 QWEN_HOME=$AGENT_HOME/.qwen 运行 verify agent；但运行本套件的 CI 作业目前都不带这个巧合（ci.yml:2119-2123 确实成对设置了二者，但只用于 typecheck:integration 与 test:integration:no-ak，而 integration-tests 的 setupFiles 并不指向 packages/cli/test-setup.ts），所以这是一枚等待某个操作者 shell 配置或新作业触发的误报，而不是当前的红。同一条断言在另一个方向上也是空的：HOME 为空时 os.homedir() 返回 ''，左侧解析为 <tmpdir>/.qwen，右侧是 '.qwen'，因此无论 pin 是否生效都会通过。
修复必须比较解析后的值，并使用与 pin 相同的真值判断，而不是 !== undefined：packages/core/src/config/storage.ts:194-196 是 const envDir = process.env['QWEN_HOME']; if (envDir) { return Storage.resolvePath(envDir); }，且 resolvePath 会展开开头的 ~。packages/cli/src/config/path-freshness.test.ts:26-30 也会在 beforeEach 中删除 QWEN_HOME 并在 :49 断言默认值，所以 pin 必须保持为「setup 文件中一次性赋值、测试可以删除」的形式，而不是从钩子中反复施加。若从 test-setup.ts 导出被固定的目录并对其断言，还能同时消除空 HOME 下的空断言问题，并在常规场景保留一条正向断言，代价是增加一点模块导出面。
请在改动后确认变异仍然能被捕获：在不设置 QWEN_HOME 的情况下，删除 test-setup.ts:67-76 的 if (!process.env['QWEN_HOME']) { … } 代码块，本测试应变红；同时 QWEN_HOME=$HOME/.qwen 的运行应变绿。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| if (originalDebugLogFile === undefined) { | ||
| delete process.env['QWEN_DEBUG_LOG_FILE']; | ||
| } else { | ||
| process.env['QWEN_DEBUG_LOG_FILE'] = originalDebugLogFile; | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-1: This restore is the only guard the round added that no test can detect being removed. Every assertion on QWEN_DEBUG_LOG_FILE in this file establishes its own precondition first — :1851 deletes the variable and then asserts toBe('1'), :1921 assigns '0' and then asserts toBe('0'), :1931 deletes and then asserts toBeUndefined() — and a grep of the file turns up no other read of it, so deleting these five lines leaves all 509 tests green while the leak the comment above them describes comes straight back. config.ts:1721-1722 assigns '1' whenever debug mode is on and the variable is undefined, and a bare delete plus a bare assignment are both invisible to vi.unstubAllEnvs(), so the '1' written by the --debug test at :1850 survives for the roughly 330 tests that follow it. The leak is measurable rather than theoretical: with a fixed QWEN_HOME, removing the guard turns 0 session log files into 26 files totalling 4066 B under $QWEN_HOME/debug, and with the pin engaged it resurrects a pin directory that test-setup.ts's new afterAll sweep had already deleted, undoing the sibling cleanup added in the same round. It cannot fail the run, which is why this is a Suggestion and not a Critical: every debugLogger write path swallows its own rejection, and the mutated tree exited 0 on Linux where dangerouslyIgnoreUnhandledErrors is false.
Witness:
Eight-arm matrix, `vitest run src/config/config.test.ts --reporter=default`,
fresh TMPDIR and QWEN_HOME per row, on an out-of-tree copy of packages/cli.
ARM 0 intact, no witness -> Tests 509 passed (509), exit 0
ARM 0b intact, fixed QWEN_HOME -> debug/ + dangling `latest`, 0 log files, 40 B
ARM 1 MUTANT (1464-1468 deleted) -> Tests 509 passed (509), exit 0 <-- guard has no witness
ARM 1 same, fixed QWEN_HOME -> 26 session .txt files, 4066 B
ARM 1-pin MUTANT, pin engaged -> exit 0 on Linux (dangerouslyIgnoreUnhandledErrors:false);
1 resurrected pin dir <TMPDIR>/qwen-cli-test-97DR32/debug
ARM 2 MUTANT + proposed witness -> 1 failed | 509 passed (510)
x loadCliConfig > restores QWEN_DEBUG_LOG_FILE after a --debug run
AssertionError: expected '1' to be '0' // Object.is equality
ARM 3 intact + proposed witness -> Tests 510 passed (510); pin dirs left: 0
ARM 4 intact + witness, ambient QWEN_DEBUG_LOG_FILE=1 -> 1 failed | 509 passed (spurious)
ARM 6 MUTANT + toBe(originalDebugLogFile) -> 510 passed -- NO FLIP (tautological)
ARM 7 MUTANT + collection-time seeded const -> 1 failed | 509 passed, expected '1' to be '0'
ARM 8/8b intact + seeded const, ambient '1' / unset -> 510 passed both
A witness placed right after the --debug test at :1858, reading the variable rather than touching it:
// at describe('loadCliConfig') scope, evaluated at collection time
const seededDebugLogFile = process.env['QWEN_DEBUG_LOG_FILE'];
// immediately after the --debug test at :1858
it('restores QWEN_DEBUG_LOG_FILE after a --debug run', () => {
expect(process.env['QWEN_DEBUG_LOG_FILE']).toBe(seededDebugLogFile);
});test-setup.ts:19-21 seeds the variable to '0' only when it is undefined, so the baseline is '0' on any run through the CLI setup file and never undefined; and because config.ts:1721 fires only on undefined, the new test must read the variable and never delete it — deleting would reproduce the :1851 setup and make the assertion self-fulfilling. Do not write the obvious toBe(originalDebugLogFile) either: the witness's own beforeEach re-captures the leaked '1' and compares it to itself, which measured green against the mutant (arm 6). A plain toBe('0') also flips correctly and is fine on CI, but it fails spuriously on a machine that exports QWEN_DEBUG_LOG_FILE=1 — an opt-in test-setup.ts:17 explicitly invites — which is why the collection-time constant is the form that is correct in both.
Please confirm the mutation: remove these five lines and run npx vitest run src/config/config.test.ts — the new test must fail on expected '1' to be '0' while the other 509 stay green.
中文说明
[Suggestion] R3-1：本轮新增的守卫中，只有这一处没有任何测试能发现它被删除。文件中所有针对 QWEN_DEBUG_LOG_FILE 的断言都先自行设置前提——:1851 先删除该变量再断言 toBe('1')，:1921 先赋值 '0' 再断言 toBe('0')，:1931 先删除再断言 toBeUndefined()——而在整个文件中搜索也找不到其它读取点，因此删除这五行后 509 个测试依然全绿，而它们上方注释所描述的泄漏会原样回来。config.ts:1721-1722 在 debug 模式开启且变量为 undefined 时赋 '1'，而裸 delete 与裸赋值对 vi.unstubAllEnvs() 都不可见，所以 :1850 那个 --debug 测试写入的 '1' 会在其后约 330 个测试中一直存活。这个泄漏是可测量的，不是理论上的：在固定 QWEN_HOME 的情况下，去掉守卫会让 $QWEN_HOME/debug 下的会话日志从 0 个变成 26 个、共 4066 B；在 pin 生效时，它还会重建一个 test-setup.ts 新增的 afterAll 清扫已经删除的 pin 目录，等于抵消同一轮加入的姊妹清理。它不会让运行失败，这也是本条定为 Suggestion 而非 Critical 的原因：debugLogger 的每条写入路径都自行吞掉 rejection，且在 dangerouslyIgnoreUnhandledErrors 为 false 的 Linux 上，变异后的树仍以 0 退出。
test-setup.ts:19-21 只在变量为 undefined 时把它播种为 '0'，因此经过 CLI setup 文件的运行中基线一定是 '0' 而不会是 undefined；又因为 config.ts:1721 只在 undefined 时触发，新测试必须读取该变量而绝不能删除它——删除会重演 :1851 的前提，使断言自我实现。也不要写成显而易见的 toBe(originalDebugLogFile)：见证测试自己的 beforeEach 会重新捕获泄漏的 '1' 并与自身比较，实测在变异体上仍为绿（arm 6）。直接写 toBe('0') 同样能翻转，在 CI 上也没问题，但在导出了 QWEN_DEBUG_LOG_FILE=1 的机器上会误报——而 test-setup.ts:17 明确邀请这种显式覆盖——因此采集期常量是两种场景都正确的写法。
请确认变异效果：删除这五行后运行 npx vitest run src/config/config.test.ts，新测试必须以 expected '1' to be '0' 失败，而其余 509 个测试保持绿色。
— qwen3.8-max via Qwen Code /review (v0.25.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no action requiredNo code changes were made in this round. Why
The PR is left exactly as it was at the start of this round. The deferred non-Critical items remain open for maintainer follow-up (start a comment with [Critical], submit a Request changes review, or comment Verification
中文说明Autofix 本轮:无需处理本轮未做任何代码改动。 原因
PR 保持本轮开始时的原样。被延后的非 Critical 条目保持开放,等待维护者跟进(以 [Critical] 开头评论、提交 Request changes 审查,或评论 验证
Deferred non-Critical feedbackCritical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. (
中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
This branch has not been deployed
6 participants
What this PR does
Adds one read-only request for complete live-session snapshots from 1–20 explicitly selected registered workspaces. Each workspace keeps its own canonical identity, catalog version, and independent success or error result. The new request shares the existing single-workspace snapshot and catalog-cache reconciliation logic. A distinct capability and TypeScript SDK method let clients opt in with cancellation and timeout support.
Why it's needed
Clients showing several workspaces currently make one live-state HTTP request per workspace on every refresh. The recently added batch session catalog can read persisted history, so it is not a suitable high-frequency execution-state endpoint. This API lets clients fetch the same authoritative in-memory state in one request, while preserving the existing single-workspace route and per-session SSE.
Reviewer Test Plan
How to verify
Start a local daemon with a primary workspace, register a trusted secondary workspace, then send one batch request containing both IDs and an unknown selector. Expect two full snapshots and one explicit
workspace_not_foundmember in input order, withCache-Control: no-store. Confirm malformed bodies fail with HTTP 400, an untrusted or transitioning workspace fails only its own member, and the capability is advertised. Change a live session between running, waiting, and idle without advancing its catalog version; every batch snapshot should still reflect the current state. Confirm the SDK makes one authenticated native REST request and preserves abort and timeout behavior. Older daemons should continue to use the single-workspace route after callers preflight capabilities.Evidence (Before & After)
N/A (HTTP and SDK change; no TUI change). The local daemon returned two workspace snapshots and one explicit unknown-workspace error in a single POST.
Tested on
devx86_64: frozen install/build/bundle, typecheck, focused tests, and daemon HTTP E2E passedEnvironment (optional)
Node 22.23.1 on macOS arm64 and
devLinux x86_64, each with an isolated daemon on a dynamically assigned loopback port. The globalqwenbinary was unavailable for the pre-change CLI dry-run; the absence of this route was confirmed in the current base source.Risk & Scope
workspace_session_live_state_batchand retain the existing single-workspace path for older daemons.Design: English · 简体中文
Linked Issues
Closes #12511
中文说明
本 PR 的改动
新增一个只读请求，一次获取 1–20 个显式选定、已注册 workspace 的完整会话运行状态快照。每个 workspace 独立返回规范化身份、目录版本及成功或错误结果。新请求与单 workspace 接口共用快照生成和目录缓存校准逻辑。独立 capability 和 TypeScript SDK 方法支持客户端按能力接入，并支持取消与超时。
为什么需要
同时展示多个 workspace 的客户端目前每次刷新都要逐 workspace 发起运行状态 HTTP 请求。近期增加的批量会话目录可能读取持久历史，不适合高频查询执行状态。新接口让客户端通过一次请求获取同样的权威内存状态，保留原单 workspace 路由和逐会话 SSE。
评审验证计划
如何验证
启动包含 primary workspace 的本地 daemon，注册可信的 secondary workspace，再用一次批量请求查询两个 ID 和一个未知 selector。预期按输入顺序得到两个完整快照和一个明确的
workspace_not_found成员，响应包含Cache-Control: no-store。确认无效请求体返回 HTTP 400，不可信或过渡中的 workspace 只影响自身成员，且 capability 已公布。在目录版本不变时使会话在运行、等待和空闲之间切换，每次快照都应反映当前状态。确认 SDK 只发送一次带鉴权的原生 REST 请求，并保留取消与超时行为。旧 daemon 的调用方应预检能力，继续使用单 workspace 路由。前后证据
不适用（HTTP 与 SDK 改动，无 TUI 变化）。本地 daemon 实测一次 POST 返回两个 workspace 快照及一个未知 workspace 的明确错误。
测试平台
devx86_64：锁文件安装/构建/打包、类型检查、聚焦测试与 daemon HTTP E2E 均通过环境
在 macOS arm64 和
devLinux x86_64 上使用 Node 22.23.1，各自启动使用动态 loopback 端口的隔离 daemon。全局qwen命令不可用，无法进行改动前 CLI dry-run；已从当前基线源码确认该路由不存在。风险与范围
workspace_session_live_state_batch，旧 daemon 保留现有单 workspace 路径。设计文档：English · 简体中文
关联 Issue
Closes #12511