fix(web-shell): scope artifact actions to owning workspace - #8510
Conversation
Verification reportVerified head: Failure-first evidenceThe original ownership probes failed 3 cases while 29 passed: an ownerless artifact tab read the primary sentinel, a secondary artifact download never called the qualified secondary client, and a secondary durable-task list omitted its The follow-up visual regression was independently reproduced with real Playwright Chromium. Both dark and light scenarios failed at Automated checks at this head
The package-wide format check still reports only five unchanged baseline files: One intermediate full-suite run produced 10 Production two-workspace validationThe built CLI/Web Shell was run with two registered local workspaces containing the same relative sentinel path but different contents.
Visual evidenceAfter the fix, Playwright generated genuine 1366×768 dark and light screenshots. I inspected both images: the right panel renders the full 中文验证报告验证报告验证 head： 失败优先证据最初的所有权探针有 3 个失败、29 个通过：缺少所有者的产物标签错误读取主工作区 sentinel；次工作区产物下载没有调用带工作区限定的次工作区客户端；次工作区持久化任务列表遗漏 后续视觉回归已使用真实 Playwright Chromium 独立复现。深色和浅色场景都在 当前 head 的自动化检查
包级完整格式检查仍只报告 5 个未修改的历史文件： 有一次中间的完整测试出现 10 个 生产双工作区验证构建后的 CLI/Web Shell 使用两个已注册本地工作区运行；两个工作区包含相同相对 sentinel 路径但内容不同。
视觉证据修复后，Playwright 生成了真实的 1366×768 深色和浅色截图。我已检查两张图片：右侧面板完整显示 |
🖼️ 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 8 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 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Hi @zjunothing — thanks for the fix, and for the verification work described in the body.
Before we can take this into code review, the PR body needs to follow the repository's pull request template. It currently uses free-form ## Summary / ## Verification headings, and the required sections are missing:
## What this PR doesand## Why it's needed— currently merged into## Summary## Reviewer Test Planwith### How to verify,### Evidence (Before & After)and### Tested on—## Verificationlists suites you ran, but doesn't give a reviewer steps to confirm the behavior themselves; this is a user-visible ownership change in Web Shell, so the before/after evidence slot is the one that matters here## Risk & Scope— no risk/tradeoff section at all; for a change that reroutes artifact actions through workspace-qualified clients with fail-closed ownership checks, this is the section maintainers most need## Linked Issues—Fixes #8494is present in the body but belongs in this section
Template: https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md
Please restructure the body around those headings; the existing 中文说明 details block is already there — keep it translated paragraph-for-paragraph. This request implies no code changes. Once the body follows the template, re-run triage with @qwen-code /triage and the gate will re-evaluate.
中文说明
感谢提交这个修复，也感谢正文中描述的验证工作。
在进入代码审查之前，PR 正文需要先遵循仓库的 pull request 模板。目前正文使用了自由格式的 ## Summary / ## Verification 标题，缺少必需的章节：
## What this PR does和## Why it's needed—— 目前合并在## Summary中## Reviewer Test Plan（含### How to verify、### Evidence (Before & After)、### Tested on）——## Verification列出了你跑过的测试，但没有给审查者自行确认行为的步骤；这是 Web Shell 中用户可见的所有权变更，before/after 证据一栏在这里尤其重要## Risk & Scope—— 完全没有风险/取舍说明；对于把产物操作改走带工作区限定的客户端、并引入 fail-closed 所有权校验的改动，这一节恰恰是维护者最需要的## Linked Issues——Fixes #8494写在正文里，但应放在这一节
模板地址：https://github.com/QwenLM/qwen-code/blob/main/.github/pull_request_template.md
请按这些标题重组正文；已有的 中文说明 details 块请保留并逐段翻译。这个要求不涉及任何代码改动。正文符合模板后，用 @qwen-code /triage 重新触发检查即可。
— Qwen Code · qwen3.8-max
|
@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: 68 passed · 0 failed · 68 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：68 通过 · 0 失败 · 68 总计 Verification reportPR 8510 verification — fix(web-shell): scope artifact actions to owning workspaceVerdict: 中文摘要
Central claim + A/BCentral claim: a turn output produced by a secondary workspace performs its file actions (preview/download) against the owning workspace, never the primary one — including when both workspaces contain the same relative path. Harness: real
Witnesses: Secondary claim (bundled fix isolation): the PR bundles (1) ownership scoping and (2) the StrictMode authority-deferral fix. Commit 1 (
Both builds hit the same wire route — the read starts; the mutant rejects it at the post-await authority check. Secondary claim (authority lifecycle): 14/14 scenarios through real provider + real HTTP ( CorrectionsNone — no earlier review round or bot comment described this code inaccurately within the material available to this round. FindingsNo blocking findings. Non-blocking observations:
Mutation matrix + vacuityFocused suite =
9/9 killed, 0 survivors. Witness: Reviewer Test Plan, per step
Not covered
MethodologyEnvironment: CI merge-ref checkout (depth 2) of Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
Re-run gate check at the new head. Three commits landed since my last pass (
Moving on to code review. 🔍 中文说明在新 head 上的重跑门禁检查。自上次审查后新增三个 commit（
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (delta pass)This re-run reviews what changed since my last pass at What I verified in the delta:
No critical blockers in the delta. Non-blocking notes for a follow-up, carried from the latest qwen-review round (0 criticals, 10 suggestions): TestingUnattended CI run — no PR code is built or executed here; the evidence below is the PR's own CI at the reviewed head, read through the API. The author's local results (macOS only; two-workspace sentinel validation, 2,794/2,794 Web Shell suite) are their claim, quoted from the body, not independently re-run. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The one reservation from my last pass is resolved: the Ubuntu unit suite that ran red on What CI still cannot settle: the web-shell suites run against a mock daemon, so daemon-side workspace-qualified routing is asserted against mocks, and the two-workspace sentinel numbers in the PR body are the author's report, not independently re-run. Sandboxed verification would settle it: 中文说明代码审查（增量轮）本次重跑审查自上次在 增量中已核实：
增量中无关键阻塞项。 留给后续的非阻塞备注（来自最新一轮 qwen-review：0 个 Critical、10 条 Suggestion）：artifact 标签页上的 测试无人值守 CI 运行——此处不构建或执行 PR 代码；以下证据是 PR 自身在受审 head 上的 CI，通过 API 读取。作者的本地结果（仅 macOS；双工作区 sentinel 验证、Web Shell 套件 2,794/2,794）是其声明，引自正文，未独立复跑。 上一轮唯一的保留意见已解决：在 CI 仍无法了结的部分：web-shell 套件跑在 mock daemon 上，daemon 侧的工作区限定路由是对 mock 断言的；正文中的双工作区 sentinel 数字是作者的报告，未独立复跑。沙箱验证可以了结它： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — everything visible from the diff, the tests, and CI is clean at this head; the remaining reservations are external to it (daemon-side routing is only mock-asserted, the author's two-workspace numbers are unverified, and ten non-critical review suggestions are open for a follow-up). This re-run was a delta pass, and the delta is exactly what the review rounds asked for — no more, no less. The R2-1/R2-6 retention fix restores the pre-PR tab-survival semantics under the new identity model instead of papering over them, and it does it with merge-order that keeps fresher rows in charge. The R1-1 counter split is the minimal correct fix — data writes stay guarded by the shared counter while only the loading-flag settlement moves to the load-only one — and the settle test would red under the old single counter. The R2-2 widening removes the last unsound assertion in the panel, and the fail-closed guard now exempts exactly one tested case. My independent proposal from the first pass still matches what landed; the additions since then are the kind I'd have made myself under the same review pressure. Pushover check, as always: I looked for reasons to hold this. The open suggestions are test-fidelity and hygiene, not behavior — on a re-run those don't defer. The retained-extra note extends a pre-existing bounded pattern. Every CI check that runs for fork PRs is green, including the one episode (red Ubuntu suite) that gated my last pass. The diff carries no scope beyond the named findings, and the fail-closed posture — which can reject a request mid-flight — is still the right tradeoff, because the alternative is the bug being fixed. Verdict: approve, pinned to the reviewed commit. wenshao's approval already stands on this same head; with this one the PR has the two approvals branch protection asks for. The 中文说明置信度：4/5 —— 从 diff、测试和 CI 能看到的一切在本 head 上都是干净的；剩余的保留意见在 diff 之外（daemon 侧路由只有 mock 断言、作者的双工作区数字未经验证、十条非关键审查建议留待后续处理）。 本次重跑是增量审查，而增量恰好是审查各轮所要求的内容——不多不少。R2-1/R2-6 的保留修复在新的身份模型下恢复了改动前的标签页存活语义，而不是掩盖它，并且通过合并顺序让更新的行始终占优。R1-1 的计数器拆分是最小正确修复——数据写入仍由共享计数器守卫，只有 loading 标志的落定移到仅 load 计数器——settle 测试在旧的单计数器下会变红。R2-2 的放宽移除了面板中最后一个不健全的断言，fail-closed 守卫现在恰好豁免一个经过测试的情况。我在第一轮的独立方案与落地形态仍然一致；此后的补充正是我在同样审查压力下会做的改动。 照例自查是否放水：我找过扣下它的理由。未处理的建议属于测试保真度与卫生问题，不是行为问题——重跑时这些不构成延迟理由。保留 extra 的备注扩展的是一个改动前就存在的有界模式。fork PR 会运行的每个 CI 检查都是绿的，包括上次挡住我的那个事件（变红的 Ubuntu 套件）。diff 不携带超出 named findings 的范围，fail-closed 姿态——可能在飞行中拒绝请求——仍是正确取舍，因为替代方案正是被修复的 bug 本身。 结论：批准，锚定到受审 commit。wenshao 的批准已在同一 head 上；加上本次批准，PR 满足分支保护要求的两票。若有人想在合并前了结 daemon 侧的主张，Stage 2 中的 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.7-max via Qwen Code /review (v0.21.4)
Review follow-up verification reportVerified head: Failure-first evidenceThe Critical loading bug was independently reproduced on the reviewed head for all three mutation paths: save, enable/disable, and delete. In each case, a mutation superseded an in-flight reload through the shared request counter and left the panel permanently rendering Review fixes
Automated verificationThe full App run emits existing test-harness 中文验证报告审查跟进验证报告验证 head： 失败优先证据在被审查的 head 上，已分别复现 Critical Loading 问题的 3 条变更路径：保存、启用/禁用、删除。每种情况下，变更操作都会通过共享 request 计数器使进行中的 reload 失效，并让面板永久显示 审查修复
自动化验证完整 App 测试会输出已有测试夹具的 |
| if (paneArtifacts.length > 0) { | ||
| const paneArtifactIds = new Set( | ||
| paneArtifacts.map((artifact) => artifact.id), | ||
| ); |
There was a problem hiding this comment.
[Critical] R2-1: Open artifact tabs are orphaned when the producing pane closes. The new extra-artifact purge runs before the unchanged early-return, and useSessionArtifacts re-reports on every turn-idle refresh — so the extra pushed on open is purged by the next report, leaving the pane snapshot as the only copy. Closing the pane (or exiting split view) deletes the snapshot, so the still-open tab renders "Artifact not found." even though the workspace is still registered/trusted, the tab carries valid owner identity, and the file still exists on disk. Pre-change, pane-artifact extras were never purged, so the tab survived pane close. — Failure scenario: split view with pane session S → open artifact A from S into the right panel → any turn finishing in S re-reports and purges the extra → close pane S (or exit split view) → onPaneArtifactsChange(S, []) deletes the snapshot → the still-open tab's lookup fails and ArtifactDetailTab renders "Artifact not found." until a pane for the same session reopens. The design doc ties tab invalidation to workspace removal/trust loss/runtime replacement only — pane lifecycle is not one of them.
Suggested fix: exempt extras referenced by open artifact tabs from the purge — in handlePaneArtifactsChange, when filtering extras by paneArtifactIds, keep an extra whose id matches an open artifact tab's artifactId (the merge into artifactPanelArtifacts already dedupes by id, so retained copies are inert while the snapshot exists).
中文说明
[Critical] R2-1：当产生该产物的 pane 关闭时，已打开的产物标签页会变成“孤儿”。新增的 extra-artifact 清除逻辑在 unchanged 提前返回之前执行，而 useSessionArtifacts 在每次 turn-idle 刷新时都会重新上报——因此打开时下推到 extras 的副本会被下一次上报清除，只剩 pane 快照这一份拷贝。关闭 pane（或退出分屏）会删除快照，于是仍然打开的标签页渲染出 "Artifact not found."——即使工作区仍然注册且受信任、标签页仍携带有效的所有者身份、文件也仍然存在。改动前，pane 产物的 extras 从不清除，因此标签页能在 pane 关闭后继续存活。— 失败场景：分屏中有 pane 会话 S → 从 S 打开产物 A 到右侧面板 → S 中任意 turn 结束触发重新上报并清除该 extra → 关闭 pane S（或退出分屏）→ onPaneArtifactsChange(S, []) 删除快照 → 仍然打开的标签页查找失败，ArtifactDetailTab 渲染 "Artifact not found."，直到同一会话的 pane 再次打开。设计文档将标签页失效限定为工作区移除/失去信任/运行时替换——pane 生命周期不在其中。
建议修复：将“被打开的产物标签页引用的 extra”排除在清除之外——在 handlePaneArtifactsChange 中按 paneArtifactIds 过滤 extras 时，保留 id 与某个已打开 artifact 标签页的 artifactId 匹配的 extra（合并进 artifactPanelArtifacts 时本就按 id 去重，因此快照存在期间保留的副本不会产生副作用）。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| <ScheduledTaskDetail | ||
| key={activeTab.id} | ||
| task={activeTab.task} | ||
| actions={activeWorkspaceActions} | ||
| actions={activeWorkspaceActions!} | ||
| /> |
There was a problem hiding this comment.
[Suggestion] R2-2: actions={activeWorkspaceActions!} passes undefined into the non-optional actions prop for a non-durable scheduled-task tab whose workspace fails resolution — the fail-closed alert guard deliberately exempts non-durable tasks, so this is the one ! assertion not backed by the guard. No crash today (every actions dereference inside ScheduledTaskDetail is gated on loadedTask, which is always null for non-durable tasks, or sits after the !task.durable early return), but the invariant is enforced ~200 lines away in the child component instead of at the call site. — Failure scenario: any future unconditional actions.* use in ScheduledTaskDetail (e.g. a retry button in the loadError state, a refresh action) typechecks cleanly and then throws TypeError: Cannot read properties of undefined — exclusively on non-durable task tabs whose owning workspace was removed, the exact state this PR's fail-closed path produces.
Suggested fix: widen the prop to actions: ArtifactWorkspaceActions | undefined in ScheduledTaskDetail (no internal change needed — non-durable paths never dereference it) and pass actions={activeWorkspaceActions} without the assertion.
中文说明
[Suggestion] R2-2：对于工作区解析失败的“非持久化”定时任务标签页，actions={activeWorkspaceActions!} 会把 undefined 传入非可选的 actions prop——fail-closed 告警守卫刻意豁免了非持久化任务，因此这是唯一一个没有守卫背书的 ! 断言。目前不会崩溃（ScheduledTaskDetail 内部对 actions 的每一次引用都被 loadedTask 门控——非持久化任务下它恒为 null——或位于 !task.durable 提前返回之后），但该不变量是在约 200 行外的子组件里强制执行的，而不是在调用点。— 失败场景：未来在 ScheduledTaskDetail 中任何无条件使用 actions.* 的改动（例如 loadError 状态下的重试按钮、刷新操作）都能通过类型检查，然后抛出 TypeError: Cannot read properties of undefined——且只发生在所有者工作区已被移除的非持久化任务标签页上，这正是本 PR 的 fail-closed 路径所产生的状态。
建议修复：将 ScheduledTaskDetail 的 prop 放宽为 actions: ArtifactWorkspaceActions | undefined（内部无需改动——非持久化路径从不引用它），并去掉断言直接传 actions={activeWorkspaceActions}。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| openScheduledTaskPanel( | ||
| request.task, | ||
| request.workspaceActions, | ||
| request.workspaceCwd, | ||
| request.workspaceId, | ||
| request.sourceSessionId, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-3: No test drives a scheduled_task (or review) open request through App's handleTurnOutputOpen; the workspace-identity forwarding into the tab is untested glue. ArtifactPanel.test.tsx renders scheduled-task tabs directly and TurnOutputs.dom.test.tsx only asserts request emission; only the artifact-kind path is covered end-to-end (the split-open-artifact mock asserts '10 B' after resolution). — Failure scenario: dropping request.workspaceId here (or transposing the positional tabWorkspaceCwd/tabWorkspaceId params, or omitting task: tabWorkspaceId ? { ...task, workspaceId: tabWorkspaceId } : task in openScheduledTaskPanel) leaves the tab with workspaceId: undefined while useArtifactWorkspaceTarget resolves 'secondary-id', so the strict id-equality fails and a live secondary workspace's durable scheduled-task panel shows "This workspace may have been removed" instead of the task. No test turns red; the regression ships.
Suggested fix: extend the App test's SplitView mock with a button that calls props.onRightPanelOpen with a scheduled_task request (task + workspaceCwd/workspaceId + sourceSessionId), and assert mockWorkspaceActions.listScheduledTasks is called with that workspace id; repeat for a review request asserting preview/download uses the stamped identity.
中文说明
[Suggestion] R2-3：没有任何测试通过 App 的 handleTurnOutputOpen 驱动 scheduled_task（或 review）类型的打开请求；工作区身份向标签页的转发是未被测试覆盖的胶水代码。ArtifactPanel.test.tsx 直接渲染定时任务标签页，TurnOutputs.dom.test.tsx 只断言请求的发出；只有 artifact 类型路径有端到端覆盖（split-open-artifact mock 在解析后断言 '10 B'）。— 失败场景：在这里丢弃 request.workspaceId（或调换位置参数 tabWorkspaceCwd/tabWorkspaceId，或在 openScheduledTaskPanel 中漏掉 task: tabWorkspaceId ? { ...task, workspaceId: tabWorkspaceId } : task）会使标签页的 workspaceId: undefined，而 useArtifactWorkspaceTarget 解析出 'secondary-id'，于是严格 id 相等判断失败，一个仍然存活的次工作区的持久化定时任务面板会显示 "This workspace may have been removed" 而不是任务内容。没有任何测试变红；回归就这样发布。
建议修复：在 App 测试的 SplitView mock 中增加一个按钮，用 scheduled_task 请求（task + workspaceCwd/workspaceId + sourceSessionId）调用 props.onRightPanelOpen，并断言 mockWorkspaceActions.listScheduledTasks 以该 workspace id 被调用；对 review 请求同样补一个，断言预览/下载使用所 stamp 的身份。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| const updated = await actions.updateScheduledTask( | ||
| loadedTask.id, | ||
| { | ||
| cron, | ||
| prompt: prompt.trim(), | ||
| name: name.trim() || null, | ||
| }, | ||
| workspaceId, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R2-4: handleSave's workspaceId argument is never asserted. All three existing updateScheduledTask assertions in ArtifactPanel.test.tsx are toggle payloads ({ enabled: false }); 'settles a pending reload after a save mutation' clicks Edit → Save but asserts only rendered text. — Failure scenario: dropping workspaceId from this call site sends edits to a secondary workspace's durable task (cron/prompt/name) to the unqualified/primary route, mutating or 404-ing against the wrong workspace's task store — the exact leak #8494 fixes for toggle and delete — and every test stays green.
Suggested fix: in the save branch of 'settles a pending reload after a %s mutation' (or the toggle/delete test), add expect(mockWorkspaceActions.updateScheduledTask).toHaveBeenCalledWith('cron-secondary', expect.objectContaining({ prompt: expect.any(String) }), 'secondary-id') after the Save click.
中文说明
[Suggestion] R2-4：handleSave 的 workspaceId 参数从未被断言。ArtifactPanel.test.tsx 中现有的三处 updateScheduledTask 断言全部是 toggle 载荷（{ enabled: false }）；'settles a pending reload after a save mutation' 点击了 Edit → Save，但只断言渲染文本。— 失败场景：在该调用点丢弃 workspaceId 会使对次工作区持久化任务的编辑（cron/prompt/name）发送到未限定/主工作区路由，从而在错误工作区的任务存储上执行修改或返回 404——这正是 #8494 为 toggle 和 delete 修复的那类泄漏——而所有测试仍然全绿。
建议修复：在 'settles a pending reload after a %s mutation' 的 save 分支（或 toggle/delete 测试）中，于点击 Save 之后补充 expect(mockWorkspaceActions.updateScheduledTask).toHaveBeenCalledWith('cron-secondary', expect.objectContaining({ prompt: expect.any(String) }), 'secondary-id')。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| onRightPanelOpen({ | ||
| ...request, | ||
| workspaceActions, | ||
| sourceSessionId: connection.sessionId, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-5: ChatPane.handleRightPanelOpen changed in this diff (subagent special case removed, workspaceActions stamp removed, sourceSessionId now stamped on every request) but has zero test coverage — ChatPane.test.tsx never references onRightPanelOpen, and no App test clicks a turn-output button in the main ChatPane transcript. SubagentDetail's analogous handler IS tested (its integration test asserts sourceSessionId and the stamped identity); ChatPane's — the main-pane producer of the same field — is not. — Failure scenario: sourceSessionId is what App's handleTurnOutputOpen keys on to route pane artifact opens into artifactPanelExtraArtifacts, and what the reconciliation filter keys on (tab.sourceSessionId !== undefined) to identify pane-owned tabs. A refactor that drops sourceSessionId: connection.sessionId here would make main-pane artifact opens bypass the extras/snapshot reconciliation this PR introduces (duplicate/stale right-panel entries for pane artifacts), with no test failing.
Suggested fix: add a ChatPane test mirroring the SubagentDetail integration assertion: render a transcript with a turn-output open button (or capture the prop), trigger the open, and assert the forwarded request equals the TurnOutputs request plus sourceSessionId: connection.sessionId and without any workspaceActions key.
中文说明
[Suggestion] R2-5：ChatPane.handleRightPanelOpen 在本 diff 中发生了变化（移除了 subagent 特例、移除了 workspaceActions stamp、现在每个请求都会 stamp sourceSessionId），但它的测试覆盖为零——ChatPane.test.tsx 从未引用 onRightPanelOpen，也没有任何 App 测试点击主 ChatPane 转录中的 turn-output 按钮。SubagentDetail 的同类处理器有测试（其集成测试断言了 sourceSessionId 和所 stamp 的身份）；而 ChatPane 的——同一字段的主 pane 生产者——没有。— 失败场景：sourceSessionId 是 App 的 handleTurnOutputOpen 用来把 pane 产物打开路由进 artifactPanelExtraArtifacts 的键，也是 reconciliation 过滤器（tab.sourceSessionId !== undefined）识别 pane 所有标签页的键。若有重构在这里丢弃 sourceSessionId: connection.sessionId，主 pane 的产物打开将绕过本 PR 引入的 extras/快照协调（pane 产物在右侧面板出现重复/过期条目），且没有任何测试失败。
建议修复：仿照 SubagentDetail 集成测试的断言，为 ChatPane 补一个测试：渲染带有 turn-output 打开按钮的转录（或捕获该 prop），触发打开，并断言转发的请求等于 TurnOutputs 请求加上 sourceSessionId: connection.sessionId，且不含任何 workspaceActions 键。
— qwen3.8-max via Qwen Code /review (v0.21.5)
| return; | ||
| } | ||
| if (!request.workspaceActions || request.sourceSessionId) { | ||
| if (request.sourceSessionId) { |
There was a problem hiding this comment.
[Suggestion] R2-6: Main-session artifact opens no longer cache the artifact row in extras, so an open artifact tab degrades to "Artifact not found." whenever the session's artifact list is temporarily emptied. The replaced condition was if (!request.workspaceActions || request.sourceSessionId); TurnOutputs never stamped workspaceActions, so pre-change every artifact open (including the main chat's, which bypasses ChatPane stamping) was cached. Post-change only pane opens (with sourceSessionId) are. — Failure scenario: useSessionArtifacts.refresh() sets artifacts to [] whenever connection.status !== 'connected' (with loading=false, error=null), so during any transient SSE disconnect/reconnect with an artifact tab open, artifactPanelArtifacts becomes empty and ArtifactDetailTab renders "Artifact not found." for an artifact that exists; pre-change the extras copy kept the row renderable through the gap. The panel self-heals on reconnect, but the tab shows a false "not found" state for the duration of the drop.
Suggested fix: restore the cache for main-session opens — push the extra unconditionally (the merge already dedupes by id, and the existing reconciliation removes extras once they duplicate the live list), instead of gating on request.sourceSessionId.
中文说明
[Suggestion] R2-6：主会话的产物打开不再把产物行缓存进 extras，因此只要会话的产物列表被临时清空，已打开的产物标签页就会退化为 "Artifact not found."。被替换的条件原来是 if (!request.workspaceActions || request.sourceSessionId)；由于 TurnOutputs 从不 stamp workspaceActions，改动前每一次产物打开（包括绕过 ChatPane stamping 的主聊天打开）都会被缓存。改动后只有 pane 打开（带 sourceSessionId）才会被缓存。— 失败场景：useSessionArtifacts.refresh() 在 connection.status !== 'connected' 时会把 artifacts 置为 []（且 loading=false、error=null），因此在产物标签页打开期间发生任何瞬时 SSE 断开/重连时，artifactPanelArtifacts 会变空，ArtifactDetailTab 会对一个实际存在的产物渲染 "Artifact not found."；改动前 extras 副本能让标签页在这段间隙中继续渲染。面板会在重连后自愈，但标签页在断开期间会显示虚假的 "not found" 状态。
建议修复：恢复对主会话打开的缓存——无条件下推该 extra（合并本就按 id 去重，且现有 reconciliation 会在 extra 与 live 列表重复时将其移除），而不是以 request.sourceSessionId 作为门控。
— qwen3.8-max via Qwen Code /review (v0.21.5)
|
@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. This is a fork PR, so the first round comes from the next scheduled scan (usually within minutes). Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本 PR 来自 fork，首轮处理将由下一次定时扫描执行（通常几分钟内）。移除 |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Address-review summary — PR #8510 (round 2)Commit: Triage gate: PR body template (stage 1a, CHANGES_REQUESTED)The triage review asks to restructure the PR body around the repository template ( Round-2 findings
Round-1 findings (re-verified, already fixed by earlier commits)All eleven round-1 inline findings were re-verified against this head and remain resolved by
VerificationCommands actually run this round (all at the final working tree, before the commit; the repository's pre-commit hook also passed during the commit):
中文说明审查处理总结 — PR #8510（第 2 轮）提交： Triage 门禁：PR 正文模板（stage 1a，CHANGES_REQUESTED）Triage 审查要求按仓库模板重组 PR 正文（ 第 2 轮发现
第 1 轮发现（重新验证，已由早前提交修复）全部 11 条第 1 轮行内发现已在当前 head 重新验证，确认仍由
验证本轮实际运行的命令（均在最终工作树上、提交前执行；提交时仓库 pre-commit 钩子也已通过）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code 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. |
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 2/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/30978469269 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix ran out of time before finishing (timeout (1080000ms)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/30992285682 🧠 Handled by Qwen Code · model/模型 |
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. A human should split or reduce the PR (or raise the agent time budget AND its step backstop together), then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/30992591926 🧠 Handled by Qwen Code · model/模型 |
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
已审查。 建议见行内评论。 未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| workspaceActions?: DaemonWorkspaceActions; | ||
| workspaceCwd?: string; | ||
| workspaceId?: string; | ||
| sourceSessionId?: string; |
There was a problem hiding this comment.
[Suggestion] R3-3: The newly added sourceSessionId on the artifact tab variant is written but never read — App.tsx stamps it in handleTurnOutputOpen, but no production code ever reads tab.sourceSessionId: both extra reconciles match artifact tabs by artifactId only, and the tab id already encodes the source session id. The same applies to ArtifactWorkspaceTarget.workspaceCwd, which the hook returns but no consumer reads. Per the repo review rule (grep the read sites of every added field), this is dead state riding along in persisted panel tabs that misleadingly implies artifact tabs are session-affined the way monitor/shell tabs are — nothing enforces it.
Failure scenario: a future change relies on tab.sourceSessionId for session scoping/invalidation of artifact tabs, assuming it is wired because it is persisted → it silently does nothing (no consumer reads it), and the dead field masks the missing wiring until a bug ships.
Suggested fix: consume the field (e.g. reconcile/drop artifact tabs when their source session no longer matches), or remove sourceSessionId from the tab type and the spread in handleTurnOutputOpen (and drop the unread workspaceCwd from ArtifactWorkspaceTarget).
中文说明
[Suggestion] R3-3：artifact 标签页变体上新增的 sourceSessionId 只写不读 —— App.tsx 在 handleTurnOutputOpen 中写入该字段，但没有任何生产代码读取 tab.sourceSessionId：两处 extra 协调逻辑都只按 artifactId 匹配 artifact 标签页，且标签页 id 本身已编码来源会话 id。同样，hook 返回的 ArtifactWorkspaceTarget.workspaceCwd 也没有任何消费方读取。按仓库评审规则（对每个新增字段 grep 其读取点），这是随持久化面板标签页一起保存的死状态，并且会误导后人以为 artifact 标签页像 monitor/shell 标签页那样具有会话亲和性 —— 实际上没有任何逻辑保证这一点。
失败场景：未来某个改动依赖 tab.sourceSessionId 做 artifact 标签页的会话作用域/失效处理，因为它被持久化而误以为已经接通 → 实际上不会有任何效果（没有消费方读取），死字段掩盖了缺失的接线，直到 bug 出现。
建议修复：消费该字段（例如在来源会话不再匹配时协调/移除 artifact 标签页），或从标签页类型和 handleTurnOutputOpen 的展开中移除 sourceSessionId（并移除 ArtifactWorkspaceTarget 中未被读取的 workspaceCwd）。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| ...(tabWorkspaceActions | ||
| ? { workspaceActions: tabWorkspaceActions } | ||
| : {}), | ||
| task: tabWorkspaceId ? { ...task, workspaceId: tabWorkspaceId } : task, |
There was a problem hiding this comment.
[Suggestion] R3-4: This task.workspaceId re-stamp in openScheduledTaskPanel has no distinguishing test. The only exercising test (routes a split pane scheduled task through its stamped workspace identity in App.test.tsx) supplies a fixture task that already carries workspaceId: 'pane-ws' identical to the request-level stamp, so deleting this re-stamp keeps the test green (verified by mutation: with the re-stamp removed, listScheduledTasks is still called with 'pane-ws').
Failure scenario: remove the { ...task, workspaceId: tabWorkspaceId } re-stamp → the existing test stays green because the fixture already carries the same id → a future request source that stamps request.workspaceId without task.workspaceId ships a listScheduledTasks(undefined) / requireScheduledTaskOwner-throw failure undetected.
Suggested fix: drop workspaceId from the paneScheduledTask fixture (keep it on the request) so the listScheduledTasks('pane-ws') assertion depends on the panel stamping the task.
中文说明
[Suggestion] R3-4：openScheduledTaskPanel 中对 task.workspaceId 的这次重新 stamp 没有任何能区分它的测试。唯一覆盖它的测试（App.test.tsx 中的 routes a split pane scheduled task through its stamped workspace identity）提供的 fixture 任务本身就带有与请求级 stamp 相同的 workspaceId: 'pane-ws'，因此删除这个重新 stamp 后测试依然为绿（已用变异验证：移除重新 stamp 后，listScheduledTasks 仍以 'pane-ws' 被调用）。
失败场景：移除 { ...task, workspaceId: tabWorkspaceId } 重新 stamp → 现有测试因 fixture 已携带相同 id 而保持绿色 → 未来某个只 stamp request.workspaceId 而不 stamp task.workspaceId 的请求来源会让 listScheduledTasks(undefined) / requireScheduledTaskOwner 抛错的故障在无人察觉的情况下上线。
建议修复：从 paneScheduledTask fixture 中移除 workspaceId（保留请求上的），使 listScheduledTasks('pane-ws') 断言依赖于面板对任务的重新 stamp。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| // The pane snapshot is gone, but the extra pushed on open keeps the | ||
| // still-open tab renderable instead of orphaning it. | ||
| expect(document.body.textContent).toContain('Pane artifact'); | ||
| expect(document.body.textContent).toContain('10 B'); |
There was a problem hiding this comment.
[Suggestion] RA1-A: The open-time artifact extra retained for open tabs is never refreshed by fresher pane/live reports, and this test pins that stale fallback as correct. handlePaneArtifactsChange only filters extras for open tabs — it never replaces them with the fresher pane row; the test's own sequence reports v1 (10 B, available) on open, then v2 (20 B), then v3 (changed), then clears the pane — and the tab falls back to the open-time v1 metadata after the user was just viewing v3. Cost: ArtifactDetail gates code-review rendering on artifact.status, so after the pane clears the stale open-time status decides availability (a last-known-changed review can render as available and vice versa), and the metadata grid shows stale size/status/title. Keeping the tab alive is the intended tradeoff; never refreshing the retained copy is the defect (the merge comment's "drop the copy once a fresher source covers the artifact again" never fires for open tabs).
Failure scenario: pane reports v1 → user opens it (v1 cached) → pane reports v2 (changed, 20 B) → pane closes → merged list falls back to the v1 extra → the still-open tab shows stale 10 B metadata and its availability gating reflects v1's status instead of the last known v2 status.
Suggested fix: in handlePaneArtifactsChange, when a non-empty pane report covers an id already present in artifactPanelExtraArtifacts whose tab is open, replace the retained extra with the fresher reported row instead of only filtering; then update this test to expect the fresher values after the pane clears.
中文说明
[Suggestion] RA1-A：为已打开标签页保留的“打开时 artifact extra”永远不会被更新的 pane/live 上报刷新，而本测试把这种陈旧回退固化为正确行为。handlePaneArtifactsChange 只对已打开标签页的 extras 做过滤 —— 从不用更新的 pane 行替换它们；测试自身的序列在打开时上报 v1（10 B、available），随后上报 v2（20 B）、v3（changed），再清除 pane —— 用户刚刚还在看 v3，标签页却回退到打开时的 v1 元数据。代价：ArtifactDetail 依据 artifact.status 决定代码评审是否渲染，因此 pane 清除后陈旧的打开时状态会决定可用性（最后已知为 changed 的评审可能被渲染为可用，反之亦然），元数据网格也会显示陈旧的大小/状态/标题。让标签页存活是有意的取舍；但从不刷新保留的副本是缺陷（合并注释所说的“一旦有更新的来源覆盖该 artifact 就丢弃副本”对已打开标签页从不生效）。
失败场景：pane 上报 v1 → 用户打开（缓存 v1）→ pane 上报 v2（changed，20 B）→ pane 关闭 → 合并列表回退到 v1 extra → 仍然打开的标签页显示陈旧的 10 B 元数据，其可用性判定反映 v1 的状态而非最后已知的 v2 状态。
建议修复：在 handlePaneArtifactsChange 中，当非空 pane 上报覆盖了 artifactPanelExtraArtifacts 中某个已打开标签页引用的 id 时，用更新的上报行替换保留的 extra 而不是只做过滤；随后更新本测试，使其断言 pane 清除后显示更新的值。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| // Cache the opened row so the tab keeps rendering through transient | ||
| // gaps in the live artifact lists (an SSE reconnect, or the source | ||
| // pane closing); the snapshot/live-list reconciles drop the copy once | ||
| // a fresher source covers the artifact again. |
There was a problem hiding this comment.
[Suggestion] RA1-B: Unconditional open-time caching plus the retention rules means extras for artifacts that permanently leave the live list are never dropped once their tab closes. The reconcile effect early-returns entirely when the live list is empty (e.g. right after /clear) and otherwise keeps any extra not in the live list regardless of tab references; handlePaneArtifactsChange prunes only extras covered by a pane report; only closing the LAST tab fully clears extras. So the comment's cleanup ("drop the copy once a fresher source covers the artifact again") never fires when no fresher source ever covers it again — the entry survives for the life of the session and round-trips through session-switch save/restore. This is pre-existing in shape for pane/session-scoped opens but is widened by this diff, since main-session opens now also cache.
Failure scenario: long-lived web-shell session — the user repeatedly opens artifacts whose sessions later get cleared (or whose artifacts are deleted), and closes each tab while another panel tab stays open → each extra entry stays in artifactPanelExtraArtifacts forever → per-session saved state grows monotonically and is restored on every session switch.
Suggested fix: prune on tab close — in closeArtifactPanelTab, when an artifact tab is removed, also drop its extra if no remaining artifact tab references that id.
中文说明
[Suggestion] RA1-B：无条件的打开时缓存加上现有保留规则，意味着那些永久离开 live 列表的 artifact 的 extra 在其标签页关闭后永远不会被移除。协调 effect 在 live 列表为空时（例如刚执行 /clear 之后）直接整体提前返回，其余情况下无论是否还有标签页引用都保留不在 live 列表中的 extra；handlePaneArtifactsChange 只修剪被 pane 上报覆盖的 extras；只有关闭最后一个标签页才会完整清空 extras。因此注释声称的清理（“一旦有更新的来源覆盖该 artifact 就丢弃副本”）在再也没有更新来源覆盖时从不生效 —— 条目在会话存活期内一直存在，并随会话切换的保存/恢复来回搬运。这种形态对 pane/会话作用域的打开而言是既有行为，但本 diff 使其扩大，因为主会话的打开现在也会缓存。
失败场景：长生命周期的 web-shell 会话 —— 用户反复打开一些之后会话被清空（或 artifact 被删除）的 artifact，并在还有其他面板标签页时逐一关闭这些标签页 → 每个 extra 条目永远留在 artifactPanelExtraArtifacts 中 → 每会话保存的状态单调增长，并在每次会话切换时被恢复。
建议修复：在关闭标签页时修剪 —— 在 closeArtifactPanelTab 中，当某个 artifact 标签页被移除时，如果没有其他 artifact 标签页再引用该 id，则同时移除其 extra。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| }); | ||
|
|
||
| it('hides Download when the artifact workspace cannot be resolved', () => { | ||
| workspaceByCwd.mockReturnValue(undefined); |
There was a problem hiding this comment.
[Suggestion] RA1-D: This workspaceByCwd.mockReturnValue(undefined) line is inert dead weight — the test renders with workspaceCwd="/unknown", which fails at capability-match time in resolveArtifactWorkspaceOwner before any client lookup, no download click occurs, and the hook only dereferences workspaceByCwd inside action closures built after an owner resolves. Verified by probe: workspaceByCwd is never called during this test, so the line can be deleted with zero effect. Meanwhile the client-level resolution-failure path is pinned by no test (an eager-resolution mutation of the hook survives all suites). Note the mitigation: the real DaemonClient.workspaceByCwd never returns undefined, so the raw-TypeError scenario is unreachable against the production SDK — the actionable core is that this line misrepresents what the test covers (it pins capability-level fail-closed, not client-level lookup failure).
Failure scenario: a maintainer changing the hook's call-time workspaceByCwd dereference sees this test and believes download-time workspace-resolution failure is pinned → the suite stays green → a client-level resolution-failure regression ships uncaught.
Suggested fix: delete the inert line, or add a distinct test for the client-level path — render with a cwd that DOES resolve via capabilities (e.g. /secondary), set workspaceByCwd.mockReturnValue(undefined), click Download, and assert the intended fail-closed outcome.
中文说明
[Suggestion] RA1-D：这行 workspaceByCwd.mockReturnValue(undefined) 是无效的死代码 —— 测试以 workspaceCwd="/unknown" 渲染，它在 resolveArtifactWorkspaceOwner 的 capability 匹配阶段就失败了，早于任何客户端查找；测试也没有点击下载，且 hook 只在所有者解析完成之后构建的 actions 闭包内才会解引用 workspaceByCwd。探针验证：本测试期间 workspaceByCwd 从未被调用，因此删除该行没有任何影响。同时，客户端层的解析失败路径没有任何测试覆盖（对 hook 的“提前解析”变异可以在全部测试套件中存活）。注意缓解因素：真实的 DaemonClient.workspaceByCwd 从不返回 undefined，因此裸 TypeError 场景在生产 SDK 下不可达 —— 可执行的核心问题是该行误导了测试的覆盖范围（它固化的是 capability 层的 fail-closed，而非客户端查找失败）。
失败场景：维护者修改 hook 在调用时对 workspaceByCwd 的解引用，看到这行代码便以为下载时的 workspace 解析失败已被固化 → 测试套件保持绿色 → 客户端层解析失败的回归在无人察觉的情况下上线。
建议修复：删除该无效行；或为客户端层路径新增一个独立测试 —— 用一个确实能通过 capabilities 解析的 cwd（如 /secondary）渲染，设置 workspaceByCwd.mockReturnValue(undefined)，点击 Download，并断言预期的 fail-closed 结果。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| // Reconnecting restores the live list and reconciles the cached copy. | ||
| mockConnection.status = 'connected'; |
There was a problem hiding this comment.
[Suggestion] RA2-A: The reconnect half of keeps a main-session artifact tab renderable across a live-list gap cannot distinguish "live list restored and cached copy reconciled" from "reconnect did nothing and the open-time cached copy keeps rendering". The open-time extra (mainArtifact) and the live row (mainArtifactRow) are field-identical, and the only post-reconnect assertion (toContain('Main artifact')) is satisfied by the extras cache alone; the trailing loadArtifacts.mockResolvedValue({ artifacts: [] }) shows the reconnect call was anticipated but never asserted. Mutation-proven: a useSessionArtifacts mutant that skips re-fetch after the first successful load keeps every shipped assertion green.
Failure scenario: a regression stops refresh() from re-running on reconnect (or the reconcile keeps the cached extra instead of the fresher live row) → the tab renders the open-time copy indefinitely (stale size/status) while this test stays green.
Suggested fix: before the reconnect act, point loadArtifacts at a changed row (e.g. sizeBytes: 20) and assert the panel shows the fresh value after reconnect — or at minimum assert loadArtifacts was called again once status returns to connected.
中文说明
[Suggestion] RA2-A：keeps a main-session artifact tab renderable across a live-list gap 的重连后半段无法区分“live 列表已恢复且缓存副本被协调”与“重连什么都没做、打开时缓存的副本继续渲染”。打开时的 extra（mainArtifact）与 live 行（mainArtifactRow）字段完全相同，而重连后唯一的断言（toContain('Main artifact')）仅靠 extras 缓存即可满足；结尾的 loadArtifacts.mockResolvedValue({ artifacts: [] }) 表明重连调用曾被预期，但从未被断言。变异验证：让 useSessionArtifacts 在首次成功加载后跳过重新获取的变异体，可以让所有已交付断言保持绿色。
失败场景：某个回归使重连时不再重新执行 refresh()（或协调时保留缓存副本而非更新的 live 行）→ 标签页无限期渲染打开时的副本（陈旧的大小/状态），而本测试保持绿色。
建议修复：在重连 act 之前，把 loadArtifacts 指向一个有变化的行（如 sizeBytes: 20），并断言重连后面板显示新值 —— 至少应断言 status 恢复为 connected 后 loadArtifacts 再次被调用。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| artifact, | ||
| workspaceActions: paneActions, | ||
| workspaceCwd: '/tmp/project', | ||
| workspaceId: 'primary', | ||
| sourceSessionId: 'pane-session', |
There was a problem hiding this comment.
[Suggestion] RA2-B: This split-open-artifact payload stamps the MAIN workspace identity ('/tmp/project'/'primary') for a pane-session artifact, but production TurnOutputs.openArtifact stamps the pane's own workspaceCwd and the workspaceId resolved from it (fed per-pane by SplitView via workspaceCwd={paneWorkspaceCwd}), with ChatPane adding sourceSessionId. It matches neither the sibling split-open-scheduled-task/split-open-review buttons ('/tmp/pane'/'pane-ws') nor production. Consequently no App-level test covers artifact-tab routing under a pane workspace identity. Mutation-proven: stamping App's artifact branch with the main-connection identity (the realistic copy-paste regression) leaves all 306 App tests green.
Failure scenario: a regression that drops or mis-resolves the stamp for kind: 'artifact' requests only (leaving scheduled_task/review stamping intact) → no App-level test exercises an artifact open with pane-workspace identity → all tests stay green while production artifact tabs from panes fail closed or mis-route.
Suggested fix: stamp the pane identity in this payload (workspaceCwd: '/tmp/pane', workspaceId: 'pane-ws') and add pane-ws to that test's capabilities, mirroring the scheduled-task/review buttons; assert the panel still renders through the pane snapshot and the post-clear fallback.
中文说明
[Suggestion] RA2-B：这个 split-open-artifact 载荷为一个 pane 会话的 artifact 盖上了主工作区身份（'/tmp/project'/'primary'），但生产环境中 TurnOutputs.openArtifact 盖的是 pane 自身的 workspaceCwd 及由其解析出的 workspaceId（SplitView 通过 workspaceCwd={paneWorkspaceCwd} 按 pane 传入），ChatPane 再附加 sourceSessionId。它既不同于同级的 split-open-scheduled-task/split-open-review 按钮（'/tmp/pane'/'pane-ws'），也不同于生产行为。因此没有任何 App 级测试覆盖 pane 工作区身份下的 artifact 标签页路由。变异验证：把 App 的 artifact 分支改为使用主连接身份（现实中的复制粘贴回归）后，全部 306 个 App 测试保持绿色。
失败场景：某个回归仅针对 kind: 'artifact' 请求丢弃或错误解析该 stamp（scheduled_task/review 的 stamp 保持完好）→ 没有任何 App 级测试用 pane 工作区身份打开过 artifact → 所有测试保持绿色，而生产中来自 pane 的 artifact 标签页会 fail-closed 或错误路由。
建议修复：在此载荷中盖上 pane 身份（workspaceCwd: '/tmp/pane'、workspaceId: 'pane-ws'），并在该测试的 capabilities 中加入 pane-ws，与 scheduled-task/review 按钮保持一致；断言面板仍能通过 pane 快照及清除后的回退渲染。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(document.body.textContent).toContain('10 B'); | ||
| expect(document.body.textContent).not.toContain( | ||
| 'This workspace may have been removed', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] RA4-A: clears split pane artifact snapshots when switching sessions cannot distinguish "pane snapshots were cleared on session switch" from "the whole panel was reset" — the reset branch closes the panel and clears tabs/extras, so these post-switch assertions pass regardless of snapshot state. Mutation-proven: removing setPaneArtifactSnapshots(new Map()) from the session-switch reset branch keeps all 306 App tests green. A flip probe shows the concrete harm: on that mutant, switching back to a session that re-opens the same artifact id renders session-1's stale snapshot row (10 B) shadowing session-2's fresh row (999 B), because pane snapshots merge ahead of open-time extras — cross-session bleed, pinned by no test.
Failure scenario: a future regression skips the snapshot clear (or moves pane snapshots into the persisted per-session panel state) → dead panes' artifact rows linger and keep merging into artifactPanelArtifacts for unrelated sessions → ships green.
Suggested fix: make the test observe the snapshots directly — after switching to session-2, switch back to session-1 (restoring the saved tab), have the SplitView mock re-report nothing, and assert the pane artifact does not reappear; or assert via a capturing hook that the snapshot map was emptied.
中文说明
[Suggestion] RA4-A：clears split pane artifact snapshots when switching sessions 无法区分“会话切换时 pane 快照被清除”与“整个面板被重置”—— 重置分支会关闭面板并清空标签页/extras，因此这些切换后的断言无论快照状态如何都能通过。变异验证：从会话切换重置分支中移除 setPaneArtifactSnapshots(new Map())，全部 306 个 App 测试保持绿色。翻转探针展示了具体危害：在该变异体上，切回重新打开相同 artifact id 的会话时，渲染的是 session-1 的陈旧快照行（10 B）遮蔽 session-2 的新行（999 B），因为 pane 快照在合并时优先于打开时 extra —— 跨会话串数据，且没有测试固化。
失败场景：未来某个回归跳过快照清除（或把 pane 快照移入持久化的每会话面板状态）→ 已关闭 pane 的 artifact 行滞留，并继续合并进不相关会话的 artifactPanelArtifacts → 带着问题保持绿色上线。
建议修复：让测试直接观察快照 —— 切换到 session-2 后再切回 session-1（恢复已保存的标签页），让 SplitView mock 不再重新上报，并断言 pane artifact 不会重新出现；或通过一个捕获钩子断言快照映射已被清空。
— qwen3.8-max via Qwen Code /review (v0.21.6)
| expect(mockWorkspaceActions.readFileBytes).not.toHaveBeenCalled(); | ||
| expect(mockWorkspaceActions.stat).not.toHaveBeenCalled(); |
There was a problem hiding this comment.
[Suggestion] RA4-B: These no-read assertions target methods the tab's load path never uses — a secret.txt preview resolves to previewKind 'source' → FileArtifactPreview → useWorkspaceFileContent → workspaceActions.readWorkspaceFile, while readFileBytes/stat are only exercised by image previews and downloads. The actually-used readWorkspaceFile is never asserted here, unlike the sibling artifact-tab fail-closed test, which arms a PRIMARY_WORKSPACE_SECRET sentinel on readWorkspaceFile and asserts it uncalled. Probe-proven: a mutant that leaks an owner-less file-tab preview onto primary actions calls readWorkspaceFile('secret.txt') and renders the sentinel while both of these assertions stay green; adding the readWorkspaceFile assertion is red on the mutant and green on correct code.
Failure scenario: a regression exempts file tabs from the fail-closed branch (or leaks them onto primary actions) → useWorkspaceFileContent calls readWorkspaceFile, which neither assertion watches → the primary-workspace read leaks with both read-blocking assertions still green.
| expect(mockWorkspaceActions.readFileBytes).not.toHaveBeenCalled(); | |
| expect(mockWorkspaceActions.stat).not.toHaveBeenCalled(); | |
| expect(mockWorkspaceActions.readWorkspaceFile).not.toHaveBeenCalled(); | |
| expect(mockWorkspaceActions.readFileBytes).not.toHaveBeenCalled(); | |
| expect(mockWorkspaceActions.stat).not.toHaveBeenCalled(); |
(ideally armed with a sentinel, mirroring the artifact-tab test; alternatively change the fixture to an image such as secret.png so readFileBytes/stat genuinely are the load path)
中文说明
[Suggestion] RA4-B：这些“无读取发生”的断言针对的方法并非该标签页加载路径所使用 —— secret.txt 预览解析出的 previewKind 为 'source' → FileArtifactPreview → useWorkspaceFileContent → workspaceActions.readWorkspaceFile，而 readFileBytes/stat 只在图片预览和下载中使用。实际被使用的 readWorkspaceFile 在此从未被断言，而同级的 artifact 标签页 fail-closed 测试却在 readWorkspaceFile 上布置了 PRIMARY_WORKSPACE_SECRET 哨兵并断言其未被调用。探针验证：让无所有者文件标签页预览泄漏到主工作区 actions 的变异体会调用 readWorkspaceFile('secret.txt') 并渲染哨兵内容，而这两条断言保持绿色；加上 readWorkspaceFile 断言后，变异体变红、正确代码变绿。
失败场景：某个回归把 file 标签页排除在 fail-closed 分支之外（或让其泄漏到主工作区 actions）→ useWorkspaceFileContent 调用 readWorkspaceFile，而两条断言都没有监视它 → 主工作区读取泄漏，两条读阻断断言却保持绿色。
（最好像 artifact 标签页测试那样布置哨兵；或者把 fixture 改为图片如 secret.png，使 readFileBytes/stat 确实是加载路径）
— qwen3.8-max via Qwen Code /review (v0.21.6)
| it('loads a durable task through its secondary workspace route', async () => { | ||
| mockWorkspaceActions.listScheduledTasks.mockResolvedValue([]); |
There was a problem hiding this comment.
[Suggestion] RA4-C: This is the only test that constructs the "durable task absent from the fetched list" state (mockResolvedValue([])), but it asserts nothing about rendering — so loadTask's list-miss branch (setLoadedTask(null) → deleted snapshot, which detects externally deleted tasks) is pinned by no test. Mutation-proven: changing setLoadedTask(match) to if (match) setLoadedTask(match) leaves all 40 tests green. A flip probe shows the concrete harm: on that mutant, re-running loadTask with the task no longer listed keeps the stale task displayed as alive with working Edit/Disable/Delete buttons on the ghost id, instead of the deleted snapshot. The sole has been deleted assertion in this file is reached only through the local handleDelete path, never through the list-miss branch.
Failure scenario: a durable task deleted elsewhere (another session/CLI) — or a scope change landing on a task id no longer in the list — re-runs loadTask → the panel keeps displaying the stale task as alive and Enable/Edit/Delete keep mutating the ghost task id → the external-deletion snapshot never appears, and no test catches it.
Suggested fix: assert the rendered outcome of the empty list here (expect(container.textContent).toContain('has been deleted')), ideally plus a second case where a scope change lands on a task id missing from a non-empty list.
中文说明
[Suggestion] RA4-C：这是唯一构造“持久化任务不在获取到的列表中”状态（mockResolvedValue([])）的测试，但它对渲染结果没有任何断言 —— 因此 loadTask 的列表未命中分支（setLoadedTask(null) → 已删除快照，用于检测外部删除的任务）没有任何测试固化。变异验证：把 setLoadedTask(match) 改为 if (match) setLoadedTask(match)，全部 40 个测试保持绿色。翻转探针展示了具体危害：在该变异体上，任务不再出现在列表中时重新执行 loadTask，面板仍把陈旧任务显示为存活状态，Edit/Disable/Delete 按钮对幽灵 id 可用，而不是显示已删除快照。本文件中唯一的 has been deleted 断言只经由本地 handleDelete 路径到达，从不经过列表未命中分支。
失败场景：任务在别处被删除（其他会话/CLI）—— 或作用域变更落在一个已不在列表中的任务 id 上 —— 触发 loadTask 重新执行 → 面板继续把陈旧任务显示为存活，Enable/Edit/Delete 继续变更幽灵任务 id → 外部删除快照永不出现，且没有测试能发现。
建议修复：在此断言空列表的渲染结果（expect(container.textContent).toContain('has been deleted')），最好再加一个用例：作用域变更落在非空列表中缺失的任务 id 上。
— qwen3.8-max via Qwen Code /review (v0.21.6)
|
@qwen-code /triage |
|
🔬 Sandboxed verification is running — watch live progress. The report will be posted here when the run completes. 🔬 沙箱验证正在运行 —— 查看实时进度。运行结束后验证报告会发布在这里。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Independent local verification (real two-workspace daemon + real browser)I rebuilt this scenario locally instead of re-reading the tests: a real Harness (click to expand)
1. The vulnerability is real, and the fix closes it
Note the card in both runs reads Control: on PR head, a primary-workspace session still uses the unqualified
2. The StrictMode follow-up is load-bearing (and effective)
3. Finding — fail-closed also removes preview/download in untrusted workspaces
This is not limited to multi-workspace setups: a single-workspace daemon also advertises the array, so an ordinary untrusted single-workspace deployment loses artifact/review/file previews entirely (verified: One more wrinkle: Suggestion (either is fine, but please pick one before merge):
Severity: medium —
4. Ownership lossWith the secondary artifact tab open on PR head, I restarted the daemon without the secondary workspace. No request was retried against the primary route (the recorded network log contains no unqualified 5. Gates I ran on PR head
6. Smaller notes
VerdictThe core security fix is real and verified end-to-end against a real daemon: opening or downloading a secondary-workspace artifact no longer reads the primary repository. Item 3 (untrusted workspaces losing preview/download, with a misleading message) is what I'd like resolved or explicitly accepted before merge; 6.1 is a cheap robustness win. Everything else looks good. Not verified here: the durable scheduled-task 中文版独立本地验证（真实双工作区 daemon + 真实浏览器）我没有只复核测试，而是在本地重建了这个场景：一个注册了两个工作区的真实 验证环境（点击展开）
1. 漏洞真实存在，修复确实闭合了它
注意两次运行卡片都显示 对照：在 PR head 上，主工作区会话仍走非限定的 2. StrictMode 跟进提交是必要且有效的
3. 发现 —— fail-closed 同时移除了未受信任工作区的预览/下载
这不限于多工作区场景：单工作区 daemon 同样会公告该数组，因此普通的未受信任单工作区部署会完全失去产物/审查/文件预览（已验证： 还有一点： 建议（二选一，但希望合并前明确其一）：
严重程度：中 —— 4. 所有权丢失在 PR head 上保持次工作区产物标签页打开，然后不带次工作区重启 daemon。没有任何请求回退重试主工作区路由（重启后的网络记录中没有非限定 5. 我在 PR head 上跑过的检查
6. 其他小问题
结论核心安全修复真实有效，并已针对真实 daemon 端到端验证：打开或下载次工作区产物不再读取主仓库文件。第 3 点（未受信任工作区失去预览/下载，且文案有误导）希望在合并前解决或明确接受；6.1 是低成本的健壮性改进。其余部分看起来没问题。 本次未验证：持久化定时任务的 |
|
Released in v0.21.7. |
5 participants
What this PR does
This PR binds artifact previews, downloads, code-review reports, file reviews, and durable scheduled-task actions to the registered workspace that produced each turn output. It carries immutable workspace identity (
workspaceCwdand, when advertised,workspaceId) through primary chat, split panes, nested subagents, and side tasks; secondary file access uses the workspace-qualified daemon client, and scheduled-task operations always use the owning workspace ID.The owner is resolved against the daemon's current capabilities and fails closed after removal, trust loss, duplicate identity, replacement, or remove-and-re-add. A stable authority token preserves valid in-flight work across semantically equivalent capability refreshes and React StrictMode effect replay without allowing an old request to regain access after its owner was revoked.
Why it's needed
Turn outputs from a secondary workspace previously retained or reused primary-workspace action objects. When two repositories had the same relative artifact path, opening or downloading the secondary output could read the primary repository's file, and durable task controls could target the wrong workspace. Passing live actions through nested panes also allowed stale authority to survive workspace removal or runtime replacement.
The first ownership fix exposed a React StrictMode edge case in the repository's real visual scenario: effect cleanup briefly revoked a still-valid owner, so the code-review artifact panel rendered “Workspace artifact owner is no longer available.” The authority-token lifecycle fixes that regression while retaining the original fail-closed security boundary.
Reviewer Test Plan
How to verify
npm test,npm run build,npm run typecheck,npm run lint, andnpm run test:e2e:visuals -- --grep "code review artifact"frompackages/web-shell.Evidence (Before & After)
Before the ownership change, the failure-first route probes produced 3 failures and 29 passes: secondary artifact/file reads and scheduled-task mutations reached primary actions. Before the StrictMode follow-up, the real Playwright visual scenario failed in both dark and light themes at
screenshots.spec.ts:848; the panel showed “Unable to display code review — Workspace artifact owner is no longer available.” The same failure was reproduced locally and in visual workflow run 30879548937.After the change, the focused ownership suites pass 413/413, the complete Web Shell suite passes 2,780/2,780, and the StrictMode visual scenario passes 2/2 with real Chromium screenshots in dark and light themes. The generated local screenshots were visually inspected and show the complete “Authoritative verdict” panel; this environment has no authenticated interactive browser available for uploading those local PNGs, so the repository's visual-preview workflow is the shareable image source for this head.
Production two-workspace validation used primary workspace ID
35f375e336aa5962and secondary workspace ID2162ccb7ea97c9ec. The same relative sentinel path returned distinct SHA-256 values (818d5f…b9127primary,0e9fac…4a12secondary), and file, byte-range, and scheduled-task CRUD requests all used the expected owner route.Tested on
Environment (optional)
macOS arm64; repository Node/npm toolchain; Playwright Chromium 149. Linux behavior is additionally covered by the repository's Ubuntu CI, but was not counted as a local test.
Risk & Scope
TurnOutputOpenRequestwith a retainedworkspaceActionsobject should passworkspaceCwdand, for registered workspaces,workspaceIdinstead. Normal Web Shell consumers require no migration.Linked Issues
Fixes #8494
中文说明
本 PR 做了什么
本 PR 将产物预览、下载、代码审查报告、文件审查和持久化定时任务操作绑定到实际生成对应 turn output 的已注册工作区。它在主聊天、分屏、嵌套子代理和 side task 之间传递不可变的工作区身份（
workspaceCwd，以及 daemon 已公告时的workspaceId）；次工作区文件访问使用带工作区限定的 daemon 客户端，定时任务操作始终使用所有者工作区 ID。所有者会依据 daemon 当前 capabilities 重新解析；工作区被移除、失去信任、出现重复身份、被替换，或被移除后重新加入时均会 fail closed。稳定的 authority token 允许语义等价的 capabilities 刷新和 React StrictMode effect 重放期间的合法进行中操作继续完成，同时禁止旧请求在其所有者被撤销后重新获得访问权。
为什么需要它
来自次工作区的 turn output 以前会保留或复用主工作区 action 对象。当两个仓库拥有相同的相对产物路径时，打开或下载次工作区输出可能读取主仓库文件，持久化任务控件也可能操作错误的工作区。在嵌套面板间传递实时 action 还会使过期权限在工作区移除或运行时替换后继续存活。
第一版所有权修复暴露了仓库真实视觉场景中的 React StrictMode 边界情况：effect cleanup 会短暂撤销仍然有效的所有者，使代码审查产物面板显示“Workspace artifact owner is no longer available”。authority-token 生命周期修复了该回归，同时保留原有的 fail-closed 安全边界。
审查者测试计划
如何验证
packages/web-shell目录运行npm test、npm run build、npm run typecheck、npm run lint和npm run test:e2e:visuals -- --grep "code review artifact"。证据（修复前后）
所有权修复前，failure-first 路由探针结果为 3 个失败、29 个通过：次工作区产物/文件读取和定时任务修改会命中主工作区 action。StrictMode 跟进修复前，真实 Playwright 视觉场景的深色和浅色主题都在
screenshots.spec.ts:848失败；面板显示“Unable to display code review — Workspace artifact owner is no longer available”。同一失败已在本地和视觉工作流运行 30879548937中复现。修复后，所有权聚焦测试 413/413 通过，Web Shell 完整测试 2,780/2,780 通过，StrictMode 视觉场景使用真实 Chromium 在深色和浅色主题下 2/2 通过。本地生成的截图已目视检查，完整显示“Authoritative verdict”面板；当前环境没有可用于上传本地 PNG 的已认证交互式浏览器，因此此 head 的可共享图片来源是仓库视觉预览工作流。
生产双工作区验证使用主工作区 ID
35f375e336aa5962和次工作区 ID2162ccb7ea97c9ec。同一相对 sentinel 路径返回不同的 SHA-256（主工作区818d5f…b9127，次工作区0e9fac…4a12），文件、字节区间和定时任务 CRUD 请求均使用预期的所有者路由。测试平台
环境（可选）
macOS arm64；仓库规定的 Node/npm 工具链；Playwright Chromium 149。仓库 Ubuntu CI 也覆盖 Linux 行为，但未计为本地测试。
风险与范围
TurnOutputOpenRequest且保留workspaceActions对象的宿主集成，应改为传递workspaceCwd，并为已注册工作区传递workspaceId。普通 Web Shell 使用方无需迁移。关联 Issue
Fixes #8494