fix(web-shell): keep archive out of the sidebar hover slot and block it on running sessions - #10302
Merged
Conversation
…it on running sessions The archive action was one of the two default inline buttons that appear over a session row's metadata slot on hover, right on top of the row's own click target, so users opening a session could archive it by accident. Archive now stays in the row's dropdown by default (inlineItems still accepts 'archive' for hosts that want it back), and it is disabled on any session with a running turn because the daemon closes the live session when it archives, which would end that turn.
Collaborator
Collaborator
|
Re-run after the autofix rounds pushed a new head — gate re-checked against Template looks good ✓ Problem: observed interaction bug, verified from base code rather than taken on faith. The hover slot renders absolutely positioned over the row's click target, Direction: aligned — accidental data loss in the daemon web-shell UI is exactly the kind of fix that belongs in-tree. The Size: not core paths (web-shell sidebar + docs only). Production logic is ~26 changed lines; tests carry the remaining ~180. No maintainer-awareness threshold in play. Approach: still minimal on re-review — one default change ( Risk: no elevated risk signals (no revert-correlated paths touched). Moving on to code review. 🔍 中文说明autofix 推送新 head 后的 re-run——已对 模板完整 ✓ 问题：已观测到的交互 bug，从 base 代码验证而非凭说法采信。悬停区以绝对定位压在行的点击热区上， 方向：对齐——daemon web-shell UI 里的误触数据丢失正是该进代码库的修复。 规模：非核心路径（仅 web-shell 侧边栏 + 文档）。生产逻辑约 26 行改动，其余约 180 行是测试。无维护者关注阈值。 方案：re-review 后依然是最小改动——一处默认值修改（ 风险：无升级风险信号（未触及回滚关联路径）。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewRe-reviewed at the new head. My independent proposal for this problem was exactly what the PR does: pull archive out of the default inline set, and block it at the handler level on running sessions so no entry point — current UI, stale render, or future one — can archive a live turn. The implementation matches:
No correctness, security, or convention issues found. One observation, non-blocking: that handler-pin test reaches into React's Testing evidence (the PR's own CI, read via API — PR code not executed)The previously red
Green CI proves the tests pass; the live-browser behavior claim (a real hover misclick on a running session) still rests on jsdom tests. Sandboxed verification would settle it: the 中文说明代码审查已在新 head 上重新审查。我的独立方案和 PR 做法一致：把归档移出默认 inline 集合，并在 handler 层挡住运行中的会话，让任何入口——当前 UI、过期渲染还是未来的入口——都无法归档活跃回合。实现要点：
未发现正确性、安全性或规范问题。一个非阻塞观察：那个 handler 钉子测试使用了 React 的 测试证据（通过 API 读取 PR 自身 CI——未执行 PR 代码）此前挡住延迟批准的红色 绿色 CI 只能证明测试通过；真实浏览器里的悬停误触行为仍由 jsdom 测试支撑。沙箱验证可以补上：随本次 triage 触发的 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, minimal fix re-verified at the new head; the two standing notes (client-side-only guard; live-browser behavior resting on jsdom until the verify run lands) are non-blocking. Stepping back: the approach still matches what I would have written — change one default, add one condition to the shared guard, make both entry points visibly inert with a reason. The autofix rounds didn't bloat it; the only addition since the first review is the test that pins the handler guard, which was exactly the gap the earlier round flagged. Verified from base code at every step: the hover slot really sits on the row's click target, the daemon really force-closes the running session on archive, and the guard really short-circuits every archive entry point in this component. CI on the reviewed commit is fully green now — including the Ubuntu unit suite that was red when the earlier deferred approval got downgraded. Reservations, none blocking:
All 中文说明置信度：4/5——在新 head 上重新验证的干净、最小修复；两条保留意见（守卫仅在客户端侧；浏览器端实际行为在 verify 运行落定前由 jsdom 支撑）均不阻塞。 退一步看：方案仍然与我会写的一致——改一个默认值、在共享守卫加一个条件、让两个入口可见地失效并给出原因。autofix 轮次没有扩大范围；首次审查后唯一的新增是钉住 handler 守卫的测试，正是上一轮指出的缺口。每一步都从 base 代码验证：悬停区确实压在行的点击热区上，daemon 归档时确实强制关闭运行中的会话，守卫确实在本组件的所有归档入口短路。被审提交上的 CI 现已全绿——包括延迟批准被降级时变红的 Ubuntu 单测。 保留意见，均不阻塞：
被审提交上的所有 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 6b": run WebShellSidebar.workspace-removal.test.tsx (contains both new running-turn tests) — the review worktree lacks the built @qwen-code/sdk dist and workspac…; "agent 6b": verify whether AcpSessionBridge.closeSession itself refuses to close a session with an in-flight prompt (which would close Finding 1's window daemon-side) — t….
Test Plan (not a blocker): client/components/sidebar/session-action-visibility.test.ts — no such file or directory; client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx — no such file or directory.
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent 6b"：run WebShellSidebar.workspace-removal.test.tsx (contains both new running-turn tests) — the review worktree lacks the built @qwen-code/sdk dist and workspac…；"agent 6b"：verify whether AcpSessionBridge.closeSession itself refuses to close a session with an in-flight prompt (which would close Finding 1's window daemon-side) — t…。
Test Plan（非阻断）：client/components/sidebar/session-action-visibility.test.ts — no such file or directory; client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
1803
to
1806
| sessionActionItems.has('archive') && | ||
| !isCurrentSession(session) && | ||
| !session.hasActivePrompt && | ||
| canMutateSessionArchive(session), |
Collaborator
There was a problem hiding this comment.
[Suggestion] The handler-level !session.hasActivePrompt guard added to canArchiveSession is exercised by no test. Both new running-session tests reach handleArchive only through controls already disabled at the DOM/Radix layer (a natively disabled <button> inline, a Radix item with data-disabled in the dropdown), so expect(active.archiveSession).not.toHaveBeenCalled() holds whether or not the guard exists. The guard itself is real defense-in-depth — it is the only layer covering the stale-render race (hasActivePrompt flips true after the last render, the button is still enabled, the click reaches the handler) — but nothing pins it: a regression deleting the clause would ship with every test green, silently re-opening the archive-a-running-session hole this PR closes.
Verified with mutation probes in an isolated scratch tree: removing !session.hasActivePrompt && keeps both new tests green (2 passed | 84 skipped, identical to baseline); a probe invoking the inline button's React onClick directly (bypassing DOM disabled suppression) flips — guard removed, archiveSession IS called; guard restored, it is not.
Consider adding a test that reaches handleArchive for a running session without depending on DOM disabled suppression — e.g. retrieve the inline archive button's React onClick from its fiber props and invoke it for a hasActivePrompt: true session, asserting archiveSession is not called. That new test must go red when !session.hasActivePrompt && is removed from canArchiveSession (the mutation above proves the two existing tests do not), so please confirm with that deletion mutation.
中文说明
添加到 canArchiveSession 的处理器层 !session.hasActivePrompt 守卫没有任何测试覆盖。两个新的「运行中会话」测试只通过 DOM/Radix 层已被禁用的控件触达 handleArchive（inline 是原生禁用的 <button>，下拉菜单是带 data-disabled 的 Radix 项），所以无论该守卫是否存在，expect(active.archiveSession).not.toHaveBeenCalled() 都会通过。该守卫本身是真实的纵深防御——它是唯一能覆盖渲染过期竞态（上次渲染后 hasActivePrompt 变为 true、按钮仍可点击、点击到达处理器）的层——但没有任何测试把它钉住：删除该子句的回归会在所有测试为绿的情况下合入，悄悄重新打开本 PR 要堵上的「归档运行中会话」的口子。
在隔离 scratch 树中的变异探针验证：从 canArchiveSession 移除 !session.hasActivePrompt && 后，两个新测试仍然全绿（2 passed | 84 skipped，与基线一致）；直接调用 inline 按钮 React onClick 的探针（绕过 DOM 禁用抑制）则翻转——移除守卫 → archiveSession 被调用；恢复守卫 → 不被调用。
建议补充一个不依赖 DOM 禁用抑制、直接触达运行中会话 handleArchive 的测试——例如从 fiber props 取到 inline 归档按钮的 React onClick，对 hasActivePrompt: true 的会话调用它，断言 archiveSession 未被调用。新测试必须在从 canArchiveSession 移除 !session.hasActivePrompt && 时变红（上面的变异证明现有两个测试不会），请删除该守卫跑一次该测试确认变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Comment on lines
+500
to
+503
| /** Opt back into the inline archive button; the default keeps it dropdown-only. */ | ||
| const INLINE_ARCHIVE_ACTIONS = { | ||
| inlineItems: ['pin', 'archive'], | ||
| } as const; |
Collaborator
There was a problem hiding this comment.
[Suggestion] The default flip of DEFAULT_INLINE_ACTION_ITEMS to ['pin'] vacates the archive assertion in 'hides archive UI when session_archive is absent' (~line 3301) — the one render in this file the diff did not update with INLINE_ARCHIVE_ACTIONS. Post-flip, archive is never inline by default, so archiveButtonFor('Secondary active')).toBeUndefined() passes for ANY implementation of the canMutateSessionArchive capability gate; a broken gate would instead surface Archive in the row's ⋮ dropdown (via showArchive), which nothing in that test checks — clicking it there would fire archiveSessionsData against a daemon that doesn't advertise the feature and surface as an archiveFailed toast.
Verified with mutation probes: dropping sessionArchiveEnabled && from the gate keeps the original test green, and a dropdown probe finds Archive leaking into the menu under the broken gate (the probe correctly fails under the intact gate). Rendering that test with sessionActions: INLINE_ARCHIVE_ACTIONS restores its bite — it fails under the broken gate and passes under the intact one.
Consider rendering that test with sessionActions: INLINE_ARCHIVE_ACTIONS (matching the pattern this diff applied to the eight other renders in this file), or additionally opening the row's dropdown and asserting no Archive menu item is present. After the fix, mutating canMutateSessionArchive to ignore the session_archive feature must turn this test red (today it stays green) — please confirm with that mutation.
中文说明
DEFAULT_INLINE_ACTION_ITEMS 默认值翻转为 ['pin'] 之后，'hides archive UI when session_archive is absent'（约第 3301 行）里的归档断言失效了——这是本文件中唯一没有改用 INLINE_ARCHIVE_ACTIONS 的渲染。翻转后归档默认不再出现在 inline 区，archiveButtonFor('Secondary active')).toBeUndefined() 对任何 canMutateSessionArchive 能力门实现都会通过；而坏掉的门会让归档出现在该行的 ⋮ 下拉菜单里（经由 showArchive），该测试并没有检查这一点——在那里点击会对未声明该特性的 daemon 触发 archiveSessionsData，表现为 archiveFailed 提示。
变异探针验证：把 sessionArchiveEnabled && 从门里去掉后原测试仍然为绿，下拉探针在坏门状态下发现 Archive 泄漏进菜单（在完整门状态下该探针正常失败）。让该测试改用 sessionActions: INLINE_ARCHIVE_ACTIONS 渲染即可恢复咬合力——坏门时失败、好门时通过。
建议将该测试改为以 sessionActions: INLINE_ARCHIVE_ACTIONS 渲染（与本 diff 对其余八处渲染的处理一致），或额外打开该行的下拉菜单断言不存在 Archive 菜单项。修复后，把 canMutateSessionArchive 变异为忽略 session_archive 特性必须让该测试变红（目前该变异下仍为绿），请跑一次该变异确认。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
Author
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-1 handler-level
!session.hasActivePromptguard incanArchiveSessionexercised by no test — already reported (comment 3872911315) - R1-2 default flip vacates the archive assertion in 'hides archive UI when session_archive is absent' — already reported (comment 3872911326)
Not explored to full depth (tool budget reached): "agent 6b": executing the changed test files (packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx and session-action-visibility.test.ts)…; "agent 2": none — no check was cut short.; "agent 1a": could not execute the web-shell test files ( WebShellSidebar.workspace-removal.test.tsx , session-action-visibility.test.ts ) — the worktree has no node_modul….
中文说明
已审查。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 6b"：executing the changed test files (packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx and session-action-visibility.test.ts)…；"agent 2"：none — no check was cut short.；"agent 1a"：could not execute the web-shell test files ( WebShellSidebar.workspace-removal.test.tsx , session-action-visibility.test.ts ) — the worktree has no node_modul…。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
… gate (#10302) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #10302Feedback and decisions
Changes
Mutation probes (per guard witnessed)
Verification
中文说明Autofix 轮次总结 — PR #10302反馈与决定
变更内容
变异探针（每个被见证的守卫）
验证
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-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not reviewed: build-and-test — web-shell E2E Smoke (ubuntu-latest, Node 22.x) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 1d": none — the walk completed within budget.; "agent 1a": run the web-shell test suite ( WebShellSidebar.workspace-removal.test.tsx ) — no node_modules in the review worktree or parent checkout, so vitest cannot exec….
Test Plan (not a blocker): client/components/sidebar/session-action-visibility.test.ts — no such file or directory; client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx — no such file or directory.
中文说明
未审查：build-and-test — web-shell E2E Smoke (ubuntu-latest, Node 22.x) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 1d"：none — the walk completed within budget.；"agent 1a"：run the web-shell test suite ( WebShellSidebar.workspace-removal.test.tsx ) — no node_modules in the review worktree or parent checkout, so vitest cannot exec…。
Test Plan（非阻断）：client/components/sidebar/session-action-visibility.test.ts — no such file or directory; client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 Autofix review-address round — no action taken The only new feedback this round was a review downgrade, not a code finding. After triage and local reproduction, no code change is warranted. Triage
Evidence
Disposition
Verification
中文说明🤖 Autofix 审查处理回合 —— 未采取任何操作 本轮唯一的新反馈是一次审查降级，而非代码问题。经过分类和本地复现，无需进行任何代码修改。 分类
证据
处理结论
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
Author
|
@qwen-code /triage |
Collaborator
|
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: 24 passed · 0 failed · 24 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：24 通过 · 0 失败 · 24 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10302 Deep Verification —
|
| Cell | Source | Oracle | Result |
|---|---|---|---|
| A (control) | base 1bd20d38 + HEAD test files |
exactly the 4 new guard/default tests fail | 90 passed / 4 failed — the predicted four, nothing else |
| B (PR build) | head e0952967 |
fully green | 94 passed / 0 failed |
4/4 hazard cells flip broken→fixed: the inline-disabled, direct-handler-invocation, dropdown-disabled, and default-inline-slot tests are all red on base and green on head. Witness: 01-ab-hazard-cells-flip-broken-to-fixed.png; raw JSON in logs/ab-base.json, logs/ab-head.json.
Control hygiene: the base tree ran with the root node_modules (PR touches no package.json/lockfile); the only cross-workspace imports of the unit under test are @qwen-code/sdk and @qwen-code/webui, which the symlinks resolve into the head tree — identical code in both cells because this PR modifies neither package (diff is confined to packages/web-shell/client + docs).
Reviewer Test Plan, per step
| # | Step | Result |
|---|---|---|
| 1 | Hover non-current row → only pin + ⋯ in the metadata slot, no archive icon | Covered by "keeps archive out of the inline slot by default" (inline archive buttons = 0, one dropdown menu item), executed green at head, red at base |
| 2 | Open ⋯ → "Archive" listed and works | Same test counts exactly 1 enabled Archive menu item; pre-existing archive-flow tests execute the action through the default layout |
| 3 | Running session → "Archive" disabled with tooltip; turn keeps running | Disabled + exact EN tooltip asserted by two tests (green head / red base). "Turn keeps running" is proven to the shape level: the archiveSession spy is never called, i.e. the client issues no request that would close the session. The daemon-side continuation itself was not exercised (jsdom level) |
| 4 | Host override inlineItems: ['pin','archive'] restores inline archive, disabled while running |
"disables the inline archive button while a session has a running turn" renders with exactly that override; green head / red base |
All four steps are executable at unit level; none was structurally unreachable. Gate witnesses for the runs in this section: 03-gates-named-and-full-suite-green.png (named files 94/94, package 201/4337).
Corrections
- The PR body's test counts (
Tests 93 passedfor the two named files,Tests 4334 passedfor the package) were written before the final bot test commite0952967("pin the running-session archive guard and capability gate") landed. Measured at the verified head: 94 and 4337. No discrepancy in behavior; correcting the numbers only. - The PR body's three mutation claims were each reproduced and are true (M1, M2/M3 rows below).
Mutation matrix (vacuity proof)
Harness 02-matrix.mjs; unmutated control green first (94/94), each mutant a single exact-string replacement applied once to WebShellSidebar.tsx and restored (sha-verified) after each run. Witness: 02-mutation-matrix-all-guards-killed.png; per-mutant vitest JSON in logs/matrix-*.json.
| Mutant | Change (revert of…) | Killed by (intended assertion, quoted failure) | Extra reds |
|---|---|---|---|
| M1 | DEFAULT_INLINE_ACTION_ITEMS back to ['pin','archive'] |
"keeps archive out of the inline slot by default" — expected […] to have a length of +0 but got 1 |
1 explained: with archive inline by default, the dropdown-running test finds no menu item to assert on |
| M2 | drop !session.hasActivePrompt from canArchiveSession (handler guard) |
"refuses to archive a running session when the handler is invoked directly" — expected "spy" to not be called … called 1 times ["primary-running"] |
none |
| M3 | const running = false (UI inert) |
both running-disabled tests — expected false to be true (inline), expected null not to be null (dropdown data-disabled) |
none — the direct-invocation test stays green because the handler guard still catches it |
| M4 | inline: drop || running from disabled |
inline-disabled test, expected false to be true |
none — the not-called assertion survives because the handler guard rejects |
| M5 | inline: drop running tooltip branch | inline-disabled test, expected 'Archive' to be 'A running session cannot be archived;…' |
none |
| M6 | dropdown: drop || running from disabled |
dropdown test, expected null not to be null |
none |
| M7 | dropdown: drop running tooltip branch | dropdown test, expected '' to be 'A running session cannot be archived;…' |
none |
| M8 | COMBO M2+M4+M6 reverted together (layered set) | all three running tests red — the hazard returns end-to-end | — |
| PC | positive control: drop isCurrent from inline disabled (pre-PR guard, same file) |
pre-existing "shows rename for current and non-current locked-secondary sessions" at line 1130, expected false to be true |
+2 pre-existing tests that also pin the current-session layer |
Layered-guard reading: M2 alone is caught only by the direct-invocation test (the DOM disabled layer still holds), and M4/M6 alone are caught only by the disabled-state assertions (the handler layer still holds). That is the defence-in-depth working as intended; the M8 combination row is where the full hazard reappears. No survivors; every revert failed the intended behavioural assertion, not an import or compile. The PC confirms the harness collects live failures from this file, so the kills above are meaningful.
Findings (non-blocking)
- Nice to have —
session-action-visibility.test.tsis a self-contained model, not wired to the component. ItsDEFAULT_INLINE_ITEMS = ['pin']constant is hand-declared, not imported (DEFAULT_INLINE_ACTION_ITEMSis not exported). M1's failure set came entirely from the workspace-removal render file — this matrix file would stay green against a component default revert. The actual default is pinned by the render test, so nothing is unprotected; but the matrix file's "defaults" case would silently keep passing if the constant and the component drifted. Importing the real default (requires exporting it) would make the model test load-bearing too. Pre-existing structure; the PR kept the constant in sync correctly. - Boundary note (not a defect): the guard is client-side. The daemon HTTP archive endpoints still accept running sessions by design (archiving intentionally closes the session), and the guard's freshness is bounded by the last catalog refresh of
hasActivePrompt— a turn started after the last refresh has an inherent staleness window. The PR positions the guard as misclick protection and the direct-invocation test covers the stale-DOM shape; a caller bypassing the component is out of its threat model. No action suggested.
Not covered
- Per-commit attribution: the checkout is depth 2;
git rev-list HEAD^1..HEAD^2yields only the tip of the snapshot's 3 commits (fixac83267, merge-of-mainfd0318b, bot testse0952967). The aggregateHEAD^1..HEADdiff was verified; individual commits were not exercised. - Real-browser/manual verification: the Reviewer Test Plan's steps were executed at jsdom unit level, not Playwright/
qwen serve; hover CSS reveal (.sessionMetaSlot:hover, intentionally kept, confirmed present at WebShellSidebar.module.css:1006–1009 and untouched by the diff) was verified by inspection only. Step 3's "turn keeps running" is shape-level (no archive call issued), not an end-to-end daemon reproduction. - Repo-wide gates: lint / typecheck / build were not re-run (the PR's own CI covers them); the targeted gate here is the affected workspace's suite.
- Base OIDs: the snapshot's
baseRefOid(1637aa96) differs from the merge-checkout base tip (1bd20d38) — main advanced between snapshot and checkout. Per the CI merge-ref contract the A/B usesHEAD^1; the diff under test is unaffected either way (PR rebased onto current main via its merge commit). - Docs changes were reviewed against code (accurate, including the new default and the running-session sentence) but not link/lint-checked.
Methodology
Environment: the CI verify container at merge commit a960d0f942 (base 1bd20d38, head e0952967), npm ci + npm run build pre-done; no network calls were needed. The A/B drove the compiled-from-source component through vitest+jsdom (01-ab.mjs): the base cell ran HEAD's test files in a scratch git worktree at the base tip (worktree removed after capture), with the cross-workspace import path audited (head-tree symlinks, PR-unmodified packages). The mutation matrix (02-matrix.mjs) mutated the single file under test with backup/sha-restore around each of 10 vitest runs. Gates (03-gates.mjs) parse the captured vitest logs and assert the summaries programmatically. Raw per-cell output: logs/ab-{base,head}.json, logs/matrix-*.json, logs/head-named-files.log, logs/full-suite.log, logs/0{1,2,3}-*-run.log. All counts in assertions.json (24 = 8 + 10 + 6) correspond to scripted checks that executed; vitest-level test counts (94 named / 4337 package) are reported in the tables above, not double-counted.
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx
file packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/session-action-visibility.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: PPPPP
packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: P (exit 0)
round 5 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/sidebar/session-action-visibility.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
approved these changes
Aug 28, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
wenshao
enabled auto-merge
August 28, 2026 04:57
ytahdn
reviewed
Aug 28, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
💬 Review comments (head e095296)
独立单遍静态审查，未发现正确性、安全性或契约问题。逐点核对如下：
Independent single-pass static review; no correctness, security, or contract issues found. Point-by-point verification:
-
守卫链路完整 / Guard chain is complete —
canArchiveSession新增!session.hasActivePrompt（WebShellSidebar.tsx:1801），memo deps 正确；handleArchive以if (!canArchiveSession(session)) return;（:3249）短路；全组件只有 inline onClick 与 dropdown onSelect 两个归档入口且都走handleArchive。即使陈旧渲染留下可用按钮，handler 层也能挡住——新增的直调__reactProps$测试钉住的正是这条路径。
(canArchiveSessiongains!session.hasActivePrompt(WebShellSidebar.tsx:1801) with correct memo deps;handleArchiveshort-circuits onif (!canArchiveSession(session)) return;(:3249); the inline onClick and the dropdown onSelect are the only archive entry points in the component and both route throughhandleArchive. Even a stale render leaving the button enabled cannot archive — the new direct-invocation test via__reactProps$pins exactly that path.) -
showArchive（:4007）是能力级判定、不含单会话运行状态，因此运行中会话的归档项呈现为"可见但禁用 + 说明性 tooltip"而非消失，与设计意图和三个新用例的断言一致。
(showArchive(:4007) is a capability-level check, so a running session renders its archive entry as visible-but-disabled with an explanatory tooltip instead of hiding it — consistent with the design intent and the three new tests.) -
hasActivePrompt在 daemon 会话类型中存在（必选与可选两种形态），Boolean()写法对可选形态安全；新 i18n keysidebar.archiveRunningDisabledEN/ZH 齐全，且本文件只有这两个语言表。
(hasActivePromptexists on the daemon session types (required and optional variants);Boolean()is safe for the optional ones; the newsidebar.archiveRunningDisabledkey is present in both EN and ZH, the only locales in the file.) -
默认值变更的传播一致 / The default change propagates consistently — 常量、可见性矩阵测试、文档三处同步；原先断言 inline 归档按钮的既有测试全部通过共享常量
INLINE_ARCHIVE_ACTIONS显式选回旧布局；破坏性说明已写入sidebar-customization.md。
(the constant, the visibility-matrix test, and the docs are all in sync; every pre-existing test that asserted the inline archive button now opts back in through the sharedINLINE_ARCHIVE_ACTIONSconstant; the breaking note is documented insidebar-customization.md.)
结论 / Verdict
代码侧无阻塞项。已知非阻塞观察（此前审查已提）：直调测试依赖 React 的 __reactProps$ 内部属性，注释已说明原因。
No code blockers. Known non-blocking observation (raised in prior review): the direct-invocation test reaches into React's __reactProps$ internals, with the rationale documented in a comment.
ytahdn
approved these changes
Aug 28, 2026
qwen-code-dev-bot
added a commit
that referenced
this pull request
Aug 28, 2026
…ault (#10302) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix review round — PR #10302One commit this round: Feedback triage[rv:5048101841] Review comment (@ytahdn) — no action needed. [ic:5448098070] Verification finding 1 ("Nice to have" — model test not wired to the component) — implemented. [ic:5448098070] Verification finding 2 (boundary note — client-side guard scope) — declined with evidence. No inline ( Verification
中文说明Autofix 评审回合 — PR #10302本回合一个提交： 反馈分诊[rv:5048101841] 评审评论（@ytahdn）— 无需处理。 [ic:5448098070] 验证发现 1（"Nice to have" — 模型测试未接入组件）— 已实现。 [ic:5448098070] 验证发现 2（边界说明 — 客户端守卫的作用域）— 有证据地拒绝。 本回合没有内联（ 验证
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/模型 |
Collaborator
|
Released in v0.22.3. |
wenshao
added a commit
to cactuser-Lu/qwen-code
that referenced
this pull request
Aug 30, 2026
Resolve the session-catalog write-through overlap: Main landed its own pin write-through (SessionCatalogStore.applySessionPinToggle plus the sidebar's optimistic storeToggle/rollback), which is what this branch's applySessionPin/pinnedSessions half was for. Keep main's pin path and drop the duplicate — store.applySessionPin, its controller entry, the second pinnedSessions memo and the two pin write-through tests, and restore main's post-unpin assertion in the locked-secondary test. Keep everything only this branch has: invalidateSessionLists (with the invalidateWorkspace -> invalidateEntries refactor it needs), removeSession, addSession, their controller entries, the archive/unarchive write-through in the sidebar, and their store tests. The branch's three archive tests now open the row's overflow menu: main moved Archive out of the inline hover slot (QwenLM#10302). web-shell sidebar + session-catalog + App suites: 900 passed; tsc reports nothing in the touched files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Moves the sidebar's archive action out of the default inline hover buttons and into the session row's dropdown menu, so the only default inline button left is pin. The
inlineItemsoption still accepts'archive'for hosts that want the old layout back.Disables archive for any session that currently has a running turn, both as an inline button and as a dropdown item, with a tooltip explaining why. The handler-level guard (
canArchiveSession) rejects running sessions too, so no entry point can archive one.Why it's needed
The inline actions render absolutely positioned over the row's metadata slot (timestamp / git icon / spinner) and fade in on hover, directly on top of the row's own click target that opens the session. Archive had no confirmation and no undo, and restoring means expanding the collapsed Archived section first. In practice, users aiming for the row hit the archive button by accident.
The consequence is worse than "moved to a folder": daemon-side
archiveDaemonSessionscallsbridge.closeSession(..., { requireAgentClose: true })before archiving, which ends whatever turn that session is running. The client only guarded the current session, so a stray click on a background session's spinner area killed its in-flight work.Reviewer Test Plan
How to verify
Unit tests (run from
packages/web-shell):Mutation check that the new tests actually discriminate: reverting
const running = Boolean(session.hasActivePrompt)tofalseand dropping!session.hasActivePromptfromcanArchiveSessionfails the two "running turn" tests; revertingDEFAULT_INLINE_ACTION_ITEMSto['pin', 'archive']fails "keeps archive out of the inline slot by default".Manual, in
qwen serveweb-shell with at least two sessions:sessionActions: { inlineItems: ['pin', 'archive'] }. Expected: the inline archive button is back, and it is disabled (same tooltip) while that session has a running turn.Evidence (Before & After)
Before: hovering a row swaps its timestamp for pin + archive buttons; clicking archive on a background session with a running turn archives it and the turn ends.
After: hovering a row shows pin + ⋯ only; archive lives in ⋯ and is disabled while the session is running. Covered by the three new unit tests and the mutation check above.
Tested on
Environment (optional)
Vitest (jsdom) unit tests only.
Risk & Scope
['pin', 'archive']) get archive one click further away; they can restore it viasessionActions.inlineItems. Existing tests that asserted on the inline archive button now opt in explicitly through that option..sessionMetaSlot:hoverCSS rule that reveals actions on running rows is intentionally kept, since the disabled state already makes it harmless.inlineItemschanges from['pin', 'archive']to['pin']; documented indocs/developers/daemon-ui/sidebar-customization.md.Linked Issues
None.
中文说明
这个 PR 做了什么
把侧边栏的归档操作从默认的 inline 悬停按钮移到会话行的下拉菜单里，默认 inline 按钮只剩置顶。
inlineItems选项仍接受'archive'，需要旧布局的宿主可以配回来。对任何正在运行回合的会话禁用归档，inline 按钮和下拉菜单项都禁用，并给出说明原因的提示。handler 层的
canArchiveSession同样拒绝运行中的会话，所以任何入口都归档不了。为什么需要
inline 操作以绝对定位盖在会话行的元数据区（时间戳 / git 图标 / 加载动画）上，悬停时淡入，正好压在用于打开会话的整行点击热区之上。归档没有确认也没有撤销，恢复还得先展开折叠的「已归档」区。实际使用中，用户想点行却点到了归档按钮。
后果比"收进抽屉"更重：daemon 侧
archiveDaemonSessions在归档前会调用bridge.closeSession(..., { requireAgentClose: true })，会终止该会话正在跑的回合。客户端此前只挡了当前会话，误点后台会话的加载动画区域就会把它进行中的工作杀掉。评审验证方案
如何验证
单元测试（在
packages/web-shell下运行）：变异校验，证明新测试有辨别力：把
const running = Boolean(session.hasActivePrompt)改回false并从canArchiveSession去掉!session.hasActivePrompt，两个「running turn」用例失败；把DEFAULT_INLINE_ACTION_ITEMS改回['pin', 'archive']，「keeps archive out of the inline slot by default」用例失败。手工验证，在
qwen serve的 web-shell 里准备至少两个会话：sessionActions: { inlineItems: ['pin', 'archive'] }渲染侧边栏。预期：inline 归档按钮回来，且在该会话运行回合期间被禁用（同样的提示）。证据（改动前后）
改动前：悬停某行时时间戳被替换成置顶 + 归档按钮；对正在运行回合的后台会话点归档，会话被归档且回合终止。
改动后：悬停某行只显示置顶 + ⋯；归档在 ⋯ 里，会话运行期间禁用。由三个新增单测和上面的变异校验覆盖。
测试平台
环境（可选）
仅 Vitest（jsdom）单元测试。
风险与范围
['pin', 'archive']）的宿主，归档会多一次点击；可通过sessionActions.inlineItems恢复。原先断言 inline 归档按钮的既有测试改为通过该选项显式开启。.sessionMetaSlot:hover在运行中行上露出操作的 CSS 规则有意保留，禁用态已经让它无害。inlineItems从['pin', 'archive']变为['pin']；已写入docs/developers/daemon-ui/sidebar-customization.md。关联 Issue
无。