Skip to content

feat(web-shell): improve session overview navigation and details - #11238

Merged
wenshao merged 14 commits into
mainfrom
codex/web-shell-session-overview
Sep 9, 2026
Merged

feat(web-shell): improve session overview navigation and details#11238
wenshao merged 14 commits into
mainfrom
codex/web-shell-session-overview

Conversation

@wenshao

@wenshao wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Make the session overview easier to scan and navigate. Each session now shows its workspace, branch and PR below the title, with distinct approval, question, running and idle states. Status filters and branch/PR search help locate sessions that need attention. Compact status icons remain beside pinned titles when narrow layouts hide the middle columns.

Reuse the sidebar's full details popover on title hover and provide a details button for keyboard and touch access. The popover shows the complete title, workspace path and session ID, supports copying the ID and opening linked PRs/issues, and keeps these actions separate from row navigation. Long details scroll within the available viewport height so every field and the copy action remain accessible. Opening another details popover replaces the previous one, and keyboard focus stays visible even when the details overflow. If hover replaces focused details, focus moves to the incoming title so Tab continues in the table; ordinary hover preserves a focused search field, including when details portals live in a ShadowRoot. Clicking a row opens its session; checkboxes control selection, and batch actions appear when a selection exists.

Why it's needed

Previously, approval requests, questions and running sessions shared a spinner, making blocked work hard to distinguish. Separate metadata columns competed with session titles, and clicking different parts of a row alternated between opening and selecting. The overview also exposed only the Git portion of the sidebar details, so users could not inspect the same information from both places.

Reviewer Test Plan

How to verify

  1. Open Session Overview with approval, question, running and idle sessions. Confirm distinct labels/icons, with a spinner only for running sessions. Combine status and workspace filters with title, branch, PR number (including #123) or ID search; changing a filter should clear selection and return to the first page.

  2. Hover a title to inspect full details. Open the explicit details button using Enter, copy the session ID, and follow a PR or issue link. These actions should keep the overview open and leave selection unchanged. Escape should close explicit details and return focus to its button. In a small viewport, long titles should wrap completely, the popover should stay within Web Shell, and scrolling should reach the copy action.

  3. Click a row to open its owning session. Dragging to select metadata should keep the overview open; clicking a plain cell while renaming should preserve the draft, with Enter to save and Escape to cancel. Select checkboxes to reveal batch actions and clear selection to hide them. Pagination should remain available, and existing restrictions on active sessions and cross-workspace operations should remain in force.

  4. While renaming, click Time once. Sorting should cancel the unsaved rename, focus Time, and let Tab continue to the first row checkbox, whether the edited session stays visible or moves off the page. In a Shadow DOM host, open Details and hover another title: the incoming title should retain focus after the old popover closes, and Tab should continue within that row. Hovering from a focused search field or from a different popover owner should preserve that focus.

  5. Start renaming immediately after hovering a title, then cancel: details should stay closed. While renaming, a live update that filters or reorders the row off the current page should discard the hidden draft and keep other rows usable. A catalog shrink that moves the current session onto the last page should preserve its visible details or draft. At 499px width, scroll the table to both ends and confirm each active/attention row keeps its title icon.

Evidence (Before & After)

Screenshots use synthetic sessions from the browser regression tests. The before image includes a selected row; the after image shows the overview with selection cleared.

Before After
Previous session overview Updated session overview with distinct states and compact metadata

The shared details popover with the complete workspace path, PR/issue links, status and copyable session ID:

Full session details

Light theme and narrow-screen details

Light theme at 1440px:

Session overview in the light theme

At 499px, pinned title cues keep each active/attention row identifiable:

Narrow overview with visible status cues

Details at 499px, with the complete branch wrapping and the copy action accessible:

Narrow-screen session details

Observed against the globally installed qwen baseline and the locally built Web Shell, using deterministic daemon responses for four session states:

Interaction Before After
Approval/question/running Shared loading indicator Distinct status text; only running spins
Title hover Full title only Full title, workspace path, branch, PR/issue links, status and copyable ID
Session ID Separate table column Available in details
Row click Selects row Opens session
Checkbox click Selects row Selects row without navigation
No selection Disabled batch controls shown Batch controls hidden; pagination remains

Validation: repository build, typecheck and bundle passed; 220 focused unit tests, all 67 Chromium smoke cases and both dark/light visual cases passed. Fourteen affected regressions passed against the actual bundled CLI, with four additional independent cases covering light/shadow portal ownership and search focus, and sorting during rename with 4/11 sessions. The shadow host adapter only enables the public Shadow DOM option in the standalone host props; product logic remains identical to the bundle. Two targeted deliberate regressions failed at the intended focus assertions, and sources were restored byte-for-byte. ESLint, Prettier and two consecutive clean reverse-audit passes completed before commit. E2E details are posted in a separate comment.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Node.js 22.22.2; isolated local daemon launched with node dist/cli.js serve --web --no-open; Chromium with mocked daemon responses for reproducible UI state. Baseline used global qwen 0.23.0.

Risk & Scope

  • Main risk or tradeoff: row clicks now navigate, so selection intentionally requires checkboxes. Full path/title/branch wrapping and clearer blocked status also apply to the shared sidebar popover.
  • Not validated / out of scope: real model execution, daemon lifecycle changes, archived-session browsing, and non-Chromium browsers.
  • Breaking changes / migration notes: no API, persistence, dependency or migration changes.

Linked Issues

Follow-up coverage, the existing footer Archive edge-click issue, and the rename draft retention design are tracked in #11390.

中文说明

此 PR 的作用

让会话总览更便于浏览和导航。每个会话的工作区、分支和 PR 显示在标题下方,并明确区分待审批、待回答、运行中和空闲状态。状态筛选以及分支、PR 搜索帮助定位需要处理的会话。窄屏中间列被遮挡时,固定标题旁的紧凑状态图标仍可见。

在标题悬浮时复用侧栏的完整详情浮窗,并提供支持键盘和触屏的详情按钮。浮窗展示完整标题、工作区路径和会话 ID,支持复制 ID 和打开关联 PR、Issue,且这些操作不会触发行导航。详情过长时在视口可用高度内滚动,所有字段与复制操作仍可访问。打开另一个详情时替换已有浮窗,即使内容溢出也保持键盘焦点可见。悬浮替换正在使用的详情时,焦点移到新标题,Tab 从该行继续;普通悬浮保留搜索框的焦点,包括详情门户位于 ShadowRoot 的嵌入场景。点击行打开对应会话,复选框控制选择,有选择时才显示批量操作。

为什么需要

此前,审批请求、提问和运行中的会话使用相同的加载动画,难以分辨等待用户处理的工作。独立元数据列挤占标题空间,点击一行的不同位置分别执行打开或选择操作。总览也仅展示侧栏详情中的 Git 信息,无法在两处查看相同信息。

审阅者测试计划

如何验证

  1. 在会话总览中准备待审批、待回答、运行中和空闲会话,确认状态文字和图标可区分,仅运行中显示加载动画。组合状态、工作区筛选与标题、分支、PR 编号(包括 #123)或 ID 搜索;修改筛选时应清空选择并返回第一页。

  2. 悬浮标题查看完整详情。用 Enter 打开详情按钮,复制会话 ID,并访问 PR 或 Issue 链接。这些操作应保留总览页面且不改变选择。Escape 应关闭显式打开的详情,并将焦点返回详情按钮。在小视口中,长标题应完整换行,浮窗应保持在 Web Shell 内,并可通过滚动到达复制按钮。

  3. 点击行打开其所属会话。拖选元信息应保留总览;重命名期间点击普通单元格应保留草稿,Enter 保存、Escape 取消。勾选复选框时显示批量操作,清空选择时隐藏。分页保持可用,运行中会话和跨工作区操作的原有限制继续生效。

  4. 重命名期间点击一次“时间”:应执行一次排序、取消未保存草稿并聚焦“时间”,Tab 继续到首行复选框;编辑行仍可见和被排到下一页两种情况都应成立。在 Shadow DOM 宿主中打开详情再悬浮另一标题:旧浮窗关闭后,新标题仍应保持焦点,Tab 从该行继续。搜索框已有焦点或不同浮窗实例间悬浮时,原焦点应保持。

  5. 悬浮标题后立即开始重命名再取消,详情应保持关闭。实时更新通过筛选或重排将编辑行移出当前页时,应丢弃隐藏草稿且其他行仍可操作;目录收缩后若该会话仍在最终页,则保留其详情或草稿。在 499 像素宽度下横向滚到两端,运行中和待处理行的标题图标应始终可见。

修改前后证据

截图使用浏览器回归测试中的模拟会话。修改前截图包含一个已选中行;修改后截图展示清空选择后的总览。

修改前 修改后
原有会话总览 具有明确状态和紧凑元数据的新会话总览

共享详情浮窗包含完整工作区路径、PR/Issue 链接、状态和可复制的会话 ID:

完整会话详情

浅色主题与窄屏详情

1440 像素宽度下的浅色主题:

浅色主题会话总览

499 像素宽度下,固定标题旁的图标仍可辨认每个运行中和待处理会话:

窄屏中的状态图标

499 像素宽度下的详情,完整分支名换行且复制按钮可访问:

窄屏会话详情

使用全局安装的 qwen 作为基线,与本地构建的 Web Shell 对比;通过确定的 daemon 响应构造四种会话状态:

交互 修改前 修改后
待审批、待回答、运行中 共用加载动画 明确的状态文字,仅运行中转圈
标题悬浮 仅完整标题 完整标题、工作区路径、分支、PR/Issue 链接、状态和可复制 ID
会话 ID 独立表格列 在详情中查看
点击行 选择行 打开会话
点击复选框 选择行 选择行且不跳转
没有选择 展示禁用的批量按钮 隐藏批量按钮,保留分页

验证结果:整库 build、typecheck、bundle 通过;220 项定向单元测试、全部 67 项 Chromium smoke 及两项深浅色视觉用例通过。14 项受影响回归在实际打包 CLI 上通过,另有 4 项独立用例验证普通/Shadow DOM 浮窗的归属和搜索保焦,以及 4/11 个会话下重命名期间的排序。Shadow DOM 宿主适配仅启用公开的宿主选项,产品逻辑与构建产物相同。两项刻意回退均在预期焦点断言失败,源码随后逐字节恢复。ESLint、Prettier 及提交前连续两轮无新问题的反向审计均完成。E2E 详情另发评论。

已测试平台

操作系统 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境

Node.js 22.22.2;通过 node dist/cli.js serve --web --no-open 启动隔离的本地 daemon;Chromium 使用模拟 daemon 响应复现 UI 状态。基线为全局 qwen 0.23.0。

风险与范围

  • 主要风险或取舍:点击行现在会导航,选择操作明确要求使用复选框。完整路径、标题和分支换行及明确的等待处理状态也会应用于共享的侧栏浮窗。
  • 未验证或范围外:真实模型执行、daemon 生命周期修改、归档会话浏览、Chromium 以外的浏览器。
  • 破坏性变更或迁移说明:没有 API、持久化、依赖或迁移变更。

关联 Issue

测试补强、既有的页脚 Archive 底缘漏点击问题及重命名草稿保留规则,记录于 #11390

CI follow-up

Align the split-pane details regression with the complete workspace path shown by the shared popover. The supervisor tests now wait for startup and shut down their live supervisor before removing its temporary directory, preventing a timed-out attempt from leaving an asynchronous write behind during retries. Production behavior is unchanged by this follow-up.

Validation: build, typecheck and bundle; 55 related CLI unit tests; all 70 Chromium smoke cases; all 5 split-pane cases against the actual bundled CLI. A first-start delay beyond the readiness deadline now retries successfully without an unhandled rejection, while a real filesystem write failure still fails the test. Cleanup also retains the authenticated token of the runner it started, so replacing metadata before an assertion failure cannot leave the old runner alive. Two consecutive reverse-audit passes completed before commit. The detailed CI reproduction and verification report is posted separately.

CI 修复:分屏详情测试改为验证完整工作区路径;supervisor 测试在清理临时目录前关闭并等待实例退出,避免超时重试留下未完成的写入。此次修改仅涉及测试。build、typecheck、bundle、55 项 CLI 单测、70 项 smoke 及实际打包 CLI 上的 5 项分屏用例通过;延迟启动重试不再产生未处理拒绝,真实写入错误仍使测试失败。清理保留已验证的原实例 token,元数据替换后的提前失败也能关闭原实例。提交前完成连续两轮反向审计。

@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

E2E verification report

Verified on macOS with Chromium, Node.js 22.22.2 and an isolated daemon serving the actual locally built dist/web-shell assets. Daemon API responses were mocked to supply reproducible approval, question, running and idle sessions; no real model execution was exercised.

  • Baseline: global qwen 0.23.0, one browser scenario passed and confirmed the previous title-only hover, dedicated ID column and row-selection behavior.
  • Local build: 4 browser tests passed (9.3s) using the committed regression spec and repository Playwright configuration.
  • Confirmed distinct states, status/search filtering and checkbox-only selection; full hover metadata, real browser clipboard copying and PR popup navigation without leaving the overview; Enter activation, copy-button focus, Escape dismissal and trigger-focus restoration; row navigation to its session.
  • Visual checks passed in dark and light themes at 1440, 760 and 390 pixels. Long titles, workspace paths and IDs wrap inside details. The narrow table scrolls horizontally; the 390px details popover remains inside the viewport. Long branches retain the existing ellipsis/native-title behavior.
  • Build, typecheck and bundle passed. 146 focused unit tests passed, including the React 18 ref compatibility suite. Focused ESLint and formatting checks passed; Web Shell typecheck passed again after adding the browser spec.
  • Both isolated daemons were stopped after validation.

Reproduction after building, with a local Web Shell server running:

cd packages/web-shell
PLAYWRIGHT_BASE_URL=http://127.0.0.1:<port> npx playwright test --config playwright.config.ts client/e2e/web-shell.session-overview.spec.ts --project=chromium

Windows, Linux, other browsers and real model execution were not validated.

中文测试报告

在 macOS、Chromium、Node.js 22.22.2 上验证,隔离 daemon 提供本地实际构建的 dist/web-shell 资源。通过模拟 daemon API 响应稳定构造待审批、待回答、运行中和空闲会话,没有执行真实模型请求。

  • 基线:全局 qwen 0.23.0 的一个浏览器场景通过,确认原有的仅标题悬浮、独立 ID 列和点击行选择行为。
  • 本地构建:使用已提交的回归用例和仓库 Playwright 配置,4 项浏览器测试通过(9.3 秒)
  • 确认不同状态、状态与搜索筛选、仅复选框选择;完整悬浮详情、真实浏览器剪贴板复制、打开 PR 后总览不跳转;Enter 打开、复制按钮获焦、Escape 关闭和焦点返回;点击行进入对应会话。
  • 深浅色主题在 1440、760、390 像素宽度下完成视觉检查。长标题、工作区路径和 ID 在详情中换行;窄表格支持横向滚动,390 像素下浮窗保持在视口内。长分支保留原有省略和原生 title 行为。
  • Build、typecheck、bundle 通过,146 项定向单元测试通过,包括 React 18 ref 兼容测试。定向 ESLint 和格式检查通过;新增浏览器用例后再次通过 Web Shell typecheck。
  • 验证结束后已停止两个隔离 daemon。

构建并启动本地 Web Shell 后,可用上方命令复现测试。未验证 Windows、Linux、其他浏览器和真实模型执行。

@wenshao
wenshao marked this pull request as ready for review September 7, 2026 02:44
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Qwen Triage ended earlyview run. It stopped before finishing; check the run log.

⚠️ Qwen Triage 提前结束 —— 查看运行。未跑完,请查看运行日志。

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 8bee8cc. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

session-overview-dark before/after

session-overview-details-dark before/after

session-overview-details-light before/after

session-overview-light before/after

terminal-turn-error-copy-narrow-dark before/after

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — and the design doc under docs/design/ is exactly what AGENTS.md asks for on a multi-file change.

Problem: real, and I checked it against main rather than taking the description's word for it. Every non-idle state renders the same styles.loading span (card.status !== 'idle'), so "needs approval", "needs input" and "running" are visually identical — the only thing separating them today is a hover title and an aria-label, which is not how you scan a list of twenty sessions. Row click calls row.toggleSelected() while the title button calls onOpenSession(...), so the same row has two different outcomes depending on where you land. The ID column also spends 136px of fixed width next to a 224px title. All three claims hold up in the code.

Direction: aligned. Session-list state visibility and filter menus are somewhere the reference agents keep investing — Claude Code's CHANGELOG carries "Added Open and Closed to the session list's status filter menu", "Added a hollow ring in the session list for sessions open in a terminal … so they no longer look closed", and a fix for "a tab opened from the session list jumping back to that session". Status filters, distinct state indicators, and unambiguous row-click semantics are the same direction.

Size: not applicable — no core paths touched (packages/web-shell/client/** plus one design doc). For reference: ~894 production lines against ~852 test lines, under the 1000-line advisory.

Approach: close to what I'd have proposed independently — reuse the sidebar's SessionDetailsTooltip rather than grow a second popover in the overview, two-line session cell, checkbox-only selection, extend the existing search instead of adding a parallel one. Deriving the column from the SessionCardStatus that already exists (with its needsApproval > askUserQuestion > running > idle ranking) instead of inventing a second state machine is the right call, and feeding it back through isWaitingForPermission / isWaitingForUserQuestion — fields DaemonSessionSummary already defines — means no SDK surface change. Two things I'd push on:

  • Dropping the Session ID column is a capability reduction, not just a layout change. The ID is still in the popover, but scanning or copying one now costs two interactions per row instead of being visible at a glance. Was a narrower ID column considered, or is the position that nobody scans IDs?
  • The shared popover now shows the full workspace path instead of the basename and wraps long titles instead of ellipsizing them. That is disclosed under Risk & Scope and I agree the overview wants it — but it changes the sidebar too, and there is an existing @smoke browser test that pins the old single-line truncation. I've written that up in the code review comment.

Risk: no elevated risk signals — none of the revert-correlated paths are touched. One thing worth knowing up front: the new client/e2e/web-shell.session-overview.spec.ts is not tagged @smoke, and the E2E Tests workflow only runs post-merge, nightly, or on demand, so those four browser cases will not execute on this PR's CI.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— docs/design/ 下的设计文档正是 AGENTS.md 对多文件改动的要求。

问题: 真实存在,我对照 main 验证过,没有只采信 PR 描述。所有非 idle 状态渲染的是同一个 styles.loadingcard.status !== 'idle'),因此"待审批""待输入""运行中"在视觉上完全相同——目前只能靠 hover titlearia-label 区分,而这并不是浏览二十个会话的方式。行点击调用 row.toggleSelected(),标题按钮调用 onOpenSession(...),同一行按落点不同有两种结果。ID 列还在 224px 的标题旁边占掉 136px 固定宽度。三点在代码里都成立。

方向: 对齐。会话列表的状态可见性和筛选菜单是参考 agent 持续投入的方向——Claude Code 的 CHANGELOG 里有"Added Open and Closed to the session list's status filter menu"、"Added a hollow ring in the session list for sessions open in a terminal … so they no longer look closed",以及"a tab opened from the session list jumping back to that session"的修复。状态筛选、可区分的状态标识、明确的行点击语义都是同一方向。

规模: 不适用——未触及核心路径(packages/web-shell/client/** 加一份设计文档)。供参考:生产代码约 894 行,测试约 852 行,低于 1000 行的大 PR 建议阈值。

方案: 与我自己独立想到的做法很接近——复用侧栏的 SessionDetailsTooltip 而不是在总览里再长出一个浮窗、双行会话单元格、仅由复选框控制选择、扩展已有搜索而不是并列新增一个。基于已有的 SessionCardStatus(及其 needsApproval > askUserQuestion > running > idle 排序)派生状态列,而不是另造一套状态机,是正确的选择;再通过 DaemonSessionSummary 本来就定义的 isWaitingForPermission / isWaitingForUserQuestion 回传,也意味着不需要改动 SDK 接口。有两点想追问:

  • 去掉 Session ID 是能力收缩,不只是布局调整。ID 仍在浮窗里,但扫读或复制一个 ID 现在每行要两次交互,而不是一眼可见。是否考虑过保留一个更窄的 ID 列,还是判断为没人需要扫读 ID?
  • 共享浮窗现在显示完整工作区路径而不是目录名,长标题也改为换行而不是省略号。这一点在 Risk & Scope 里已披露,我也认同总览需要它——但它同样改变了侧栏,而现有一个 @smoke 浏览器测试锁定的正是旧的单行截断行为。这一点我写在代码审查评论里。

风险: 无升级风险信号——未触及与回滚相关的文件路径。有一点先说明:新增的 client/e2e/web-shell.session-overview.spec.ts 没有打 @smoke 标签,而 E2E Tests workflow 只在合并后、每日定时或手动触发时运行,所以这四个浏览器用例不会在本 PR 的 CI 上执行。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 965c72bd4d71550c71afc8a045c5ddd52f5fba96 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Code review

I wrote down my own approach before opening the diff — reuse the sidebar popover rather than grow a second one in the overview, two-line session cell, checkbox-only selection, extend the existing search. The PR landed on essentially the same shape, and in two places it is better than what I had in mind: it reuses the SessionCardStatus ranking that already exists instead of inventing a second state machine, and it feeds status back through isWaitingForPermission / isWaitingForUserQuestion, which DaemonSessionSummary already defines, so there is no SDK surface change. One blocker and two cleanups.

1. Blocker — the shared popover change breaks an existing @smoke browser test this PR doesn't touch

packages/web-shell/client/e2e/web-shell.collapsed-groups-persist.spec.ts, test keeps long session details inside a constrained WebShell @smoke, hovers a sidebar session whose display name is 'longunbrokensessiontitle'.repeat(24) and asserts:

const title = details.getByTitle(longTitle);
await expect(title).toHaveAttribute('title', longTitle);
// …
expect(titleMetrics.clientHeight).toBeLessThanOrEqual(titleMetrics.lineHeight + 1);
expect(titleMetrics.scrollWidth).toBeGreaterThan(titleMetrics.clientWidth);

This PR drops title={label} from .sessionDetailsTitle and overrides that class's white-space: nowrap / text-overflow: ellipsis (WebShellSidebar.module.css:799) with !whitespace-normal break-words. Both assertions then fail:

  • getByTitle(longTitle) resolves to zero elements. I checked the fixture — createOrganizedScenario gives those sessions no prs, branch or worktree, so the only remaining title attributes inside that dialog are the folder path and the session ID, neither of which is the long title.
  • The metrics pair pins single-line ellipsized truncation. A wrapped title is multi-line (clientHeight ≈ N × lineHeight) and has no horizontal overflow (scrollWidth == clientWidth).

This is not a stale test to wave off — it runs in this PR's own CI. The ci.yml job web-shell E2E Smoke (ubuntu-latest, Node 22.x) runs npm run test:e2e:smoke (playwright test --grep @smoke) and is gated on needs: test, which is still in progress, so the check has not been created yet. Expect it red.

The way out is a design decision, not a test edit. Either keep ellipsis truncation in the sidebar and wrap only in the overview — that is what a presentation prop on the shared popover is for, and worktreeOnly (below) is already the seam — or decide wrapping is right everywhere and update the smoke spec in this PR to pin the new behaviour deliberately. What shouldn't happen is merging with a gating check that the diff silently invalidated.

2. worktreeOnly becomes a dead switch

Its only caller was the overview's Git column, which this PR deletes; the prop, its default and all three read sites stay in SessionDetailsTooltip.tsx. After this PR align is always 'start' and both !worktreeOnly && blocks always render — precisely the unpopulated-optional-flag shape the review rules in AGENTS.md say to grep for. Drop it, or repurpose that seam for the sidebar/overview split in finding 1.

3. Three i18n keys are orphaned

sessionsOverview.sessionId, sessionsOverview.folder and sessionsOverview.worktree had exactly three call sites, all in the deleted column headers. The new e2e spec matches the literal string 'Session ID', not the key, so nothing reads them now — six entries across EN and ZH.

What I checked and found sound

Worth recording, so it's clear where the review actually went:

  • The synthesized session object typechecks. DaemonSessionSummary requires only sessionId and workspaceCwd; every other field, including the two waiting flags, is optional — so spreading a possibly-undefined map lookup and overriding four fields is safe.
  • The collision-boundary change is narrower than it looks. resolveSessionDetailsCollisionBoundary already returned sidebar.closest('[data-web-shell-root]') ?? sidebar, so the sidebar's resolved boundary is unchanged; passing closest('[data-web-shell-root]') instead of closest('aside') only gives the overview — which is not inside an aside and previously resolved to null — a boundary it lacked. That is what the design doc means by keeping popovers inside the Web Shell boundary.
  • onClickstopPropagation on PopoverContent is load-bearing, not decorative. The popover is portaled, so its clicks bubble through the React tree to the row's onClick and would otherwise navigate away while the user is copying an ID.
  • Checkbox clicks cannot navigate: the select column already carries stopRowClick: true, and the data-table stops propagation at the TableCell.
  • Filter and search changes reset both page index and selection — the effect's dependency array was updated to include statusFilter, which is the easy thing to miss.
  • The unit tests pin the behaviour instead of restating it: onOpenSession called exactly once with ('s-run', '/w') while the checkbox stays unchecked, then a checkbox click selects without navigating; the status test asserts the exact label array, that only running carries the spinner, and that the two attention icons differ; the hover test asserts the delay boundary at 299 ms vs 300 ms.
sequenceDiagram
    participant P1 as User
    participant P2 as TableRow onClick
    participant P3 as Checkbox cell (stopRowClick)
    participant P4 as Details button
    participant P5 as PopoverContent (portal)
    participant P6 as onOpenSession
    P1->>P2: click anywhere else in the row
    P2->>P6: open that session
    P1->>P3: click the checkbox
    Note over P3: stopped at the cell, selection only
    P1->>P4: click or press Enter
    P4->>P5: open details, focus moves in
    P1->>P5: copy the ID or follow a PR link
    Note over P5: portal clicks bubble through the React tree, so stopPropagation
    P5-->>P1: overview stays open, selection unchanged
Loading

Small and non-blocking: the per-filter counts call searchedCards.filter(...) inline in JSX, so four full passes per render — fine at 50 rows, worth a useMemo if the catalogue grows. And the tooltip's idle status concatenates two separately translated strings (Idle · 3 clients), which cannot be reordered per locale.

Files changed (8)
File What changed
docs/design/web-shell-session-overview.md New design doc; states the problem, the reuse-the-sidebar-popover decision, and that no daemon or SDK surface changes.
packages/web-shell/client/components/SessionOverviewPanel.tsx The bulk of it. Deletes the SessionIdCell and the git/sessionId/workspace columns, folds workspace, branch and PR under the title, adds a status column and a four-way status filter, moves the workspace filter into the toolbar, switches row click from select to open, and hides batch actions when nothing is selected.
packages/web-shell/client/components/SessionOverviewPanel.module.css One new .attention rule colouring the two blocked states with the warning token.
packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Shared component: new openOnClick mode using PopoverTrigger instead of PopoverAnchor, full path instead of basename, wrapping instead of truncation, status derived from the waiting flags, wider collision boundary, and stopPropagation on portaled clicks.
packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx Adds the ref-forwarding, keyboard-dismissal and host-portal case for the new click trigger.
packages/web-shell/client/components/SessionOverviewPanel.test.tsx Rewrites the tests that asserted the old columns and row-click semantics; adds status priority, filter and search reset, details entry, ID copy without navigation, and batch-action visibility.
packages/web-shell/client/e2e/web-shell.session-overview.spec.ts New four-case Playwright spec over a mocked daemon. Not tagged @smoke, so it does not run on PR CI.
packages/web-shell/client/i18n.tsx Twelve new keys per locale for the status column, filters, details label and session count; widens the search placeholder. Does not remove the three keys finding 3 orphans.

Testing evidence

This is an unattended CI run, so I did not build or execute anything from this PR — the evidence below is the PR's own CI, read through the API for commit 965c72bd4d71550c71afc8a045c5ddd52f5fba96. Nothing has failed yet, but the two checks that matter most for this diff have not finished: Test (ubuntu-latest, Node 22.x) and Lint & Static (ubuntu-latest, Node 22.x) are both still in progress, and web-shell E2E Smoke has not been created because it is queued behind Test. That smoke job is the one finding 1 predicts will go red, so the CI signal for this PR is genuinely incomplete right now rather than green.

Capture web-shell visuals passed, which is mild reassurance that nothing throws at render, but that workflow captures screenshots and does not assert the truncation behaviour the smoke spec pins. Test (macos-latest) and Test (windows-latest) are skipped by the CI profile, so the unit suite is Linux-only on this PR.

Not verified, and why: the browser-level claims — popover placement inside the Web Shell boundary at constrained viewport sizes, focus returning to the details button on Escape, external PR and issue links opening a popup without closing the overview — cannot be settled from the diff, and the spec that would settle them is not tagged @smoke, so this PR's CI never runs it. The PR description's "four Chromium browser regression cases passed" and "146 focused unit tests passed" are the author's own local results; I have not re-run them and they are not evidence in this section.

Final CI results for 965c72b (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Lint & Static (ubuntu-latest, Node 22.x) ❌ failure
Test (ubuntu-latest, Node 22.x) 🚫 cancelled
web-shell E2E Smoke (ubuntu-latest, Node 22.x) 🚫 cancelled
Capture web-shell visuals (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle this: @qwen-code /verify — that row click navigates while checkbox click only selects, that the status priority holds against a real daemon rather than a mocked one, and that the details popover stays inside the Web Shell boundary at constrained sizes, are none of them observable from the diff, and the new Playwright spec that would demonstrate them never runs on this PR's CI because it is untagged.

中文说明

代码审查

我在看 diff 之前先写下了自己的方案——复用侧栏浮窗而不是在总览里再造一个、双行会话单元格、仅由复选框控制选择、扩展已有搜索。这个 PR 基本落在同一形状上,有两处比我想的更好:复用了已有的 SessionCardStatus 排序而不是另造状态机;并通过 DaemonSessionSummary 本来就定义的 isWaitingForPermission / isWaitingForUserQuestion 回传状态,因此不需要改动 SDK 接口。一个阻塞项和两个清理项。

1. 阻塞项——共享浮窗的改动破坏了本 PR 未修改的现有 @smoke 浏览器测试

web-shell.collapsed-groups-persist.spec.ts 里的 keeps long session details inside a constrained WebShell @smoke 会悬浮一个侧栏会话,其名称是 'longunbrokensessiontitle'.repeat(24),并断言 details.getByTitle(longTitle) 存在且带 title 属性,同时断言标题是单行截断(clientHeight <= lineHeight + 1scrollWidth > clientWidth)。

本 PR 去掉了 .sessionDetailsTitle 上的 title={label},并用 !whitespace-normal break-words 覆盖了该类的 white-space: nowrap / text-overflow: ellipsis。两条断言都会失败:getByTitle(longTitle) 匹配到 0 个元素(我核对过 fixture,createOrganizedScenario 的这些会话没有 prsbranchworktree,所以该浮窗内剩下的 title 只有目录路径和会话 ID);换行后的标题是多行且没有横向溢出。

这不是可以忽略的过期测试——它跑在本 PR 自己的 CI 上。ci.ymlweb-shell E2E Smoke (ubuntu-latest, Node 22.x) 执行 npm run test:e2e:smoke--grep @smoke),它 needs: test,而 test 仍在进行中,所以这个 check 还没被创建。预计会红。

解法是一个设计决定,而不是改测试。要么侧栏保留省略号截断、只在总览换行——这正是共享浮窗上表现层 prop 的用途,而 worktreeOnly(见下)已经是这个接缝;要么认定换行在两处都正确,并在本 PR 内更新该 smoke spec,明确锁定新行为。不应该发生的是:合并时让一个被 diff 悄悄失效的 gating check 留在那里。

2. worktreeOnly 变成死开关

它唯一的调用方是本 PR 删掉的总览 Git 列;prop、默认值和三处读取点都还留在 SessionDetailsTooltip.tsx。改动之后 align 永远是 'start',两个 !worktreeOnly && 分支永远渲染——正是 AGENTS.md 审查规则要求 grep 的"声明并读取但没有任何调用方设置"的可选标志。删掉它,或者把这个接缝用于第 1 点里的侧栏/总览分化。

3. 三个 i18n key 成为孤儿

sessionsOverview.sessionIdsessionsOverview.foldersessionsOverview.worktree 只有三处调用,全在被删除的列头里。新增的 e2e spec 匹配的是字面量 'Session ID' 而不是 key,所以现在没有读取方——中英文共 6 条。

已核对且没问题的部分:合成的 session 对象能通过类型检查(DaemonSessionSummary 只要求 sessionIdworkspaceCwd,其余含两个 waiting 标志均为可选);碰撞边界改动比看起来窄(helper 本来就返回 closest('[data-web-shell-root]') ?? sidebar,侧栏结果不变,只是让不在 aside 内的总览第一次有了边界);PopoverContent 上的 stopPropagation 是必需的而非装饰(浮窗走 portal,点击会沿 React 树冒泡到行的 onClick);复选框不会触发导航(select 列本来就有 stopRowClick: true);筛选与搜索变更会同时重置页码和选择(effect 依赖数组已补上 statusFilter);单测确实在锁定行为而非复述实现。

非阻塞小点:各筛选按钮的计数在 JSX 里内联调用 searchedCards.filter(...),每次渲染跑四遍——50 行时无所谓,目录变大后值得 useMemo;浮窗的 idle 状态把两个独立翻译的字符串拼在一起(Idle · 3 clients),无法按语言调整语序。

测试证据

这是无人值守的 CI 运行,因此我没有构建或执行本 PR 的任何代码——以上证据来自 API 读取的本 PR 自身 CI(commit 965c72bd4d71550c71afc8a045c5ddd52f5fba96)。目前没有失败项,但对这份 diff 最关键的 check 还没跑完,因此本 PR 的 CI 信号现在是"不完整"而不是"绿"。

未验证的部分及原因:浏览器层面的断言(受限视口下浮窗是否留在 Web Shell 边界内、Escape 后焦点是否回到详情按钮、外部 PR/Issue 链接是否在打开弹窗的同时保留总览)无法从 diff 判定,而能判定它们的 spec 没有 @smoke 标签,本 PR 的 CI 不会运行它。PR 描述里的"四个 Chromium 浏览器回归用例通过""146 个聚焦单测通过"是作者本地的结果,我没有重跑,也不作为本节的证据。

沙箱验证可以定这件事:@qwen-code /verify——行点击导航而复选框只选择、状态优先级对真实 daemon(而非 mock)成立、详情浮窗在受限尺寸下留在 Web Shell 边界内,这三点都无法从 diff 看出,而本可证明它们的新 Playwright spec 因为没打标签,在本 PR 的 CI 上从不运行。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 965c72bd4d71550c71afc8a045c5ddd52f5fba96 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 2/5 — the design and the tests are good and I'd merge the overview half today, but the diff silently invalidates an existing browser test that runs on this PR's own CI, and that needs resolving first.

@wenshao Requesting changes on one item only — the rest of this is genuinely well put together.

I went looking for whether the problem is real rather than accepting the framing, and it is: on main all three blocked states render the same spinner, and a single row has two different click outcomes depending on where you land. Those are observable in the code, not asserted in the description.

The approach matched what I'd have proposed independently, and beat it in two places — deriving the column from the SessionCardStatus ranking that already exists instead of adding a parallel state machine, and feeding it back through isWaitingForPermission / isWaitingForUserQuestion, which DaemonSessionSummary already defines, so no SDK surface moves. The panel also ends up smaller rather than larger: SessionIdCell and three columns are gone. Six months from now I'd thank whoever wrote this, not curse them.

The blocker reads as an oversight at a seam, not as carelessness, which is why this is request-changes rather than a rethink. The diff reaches beyond the overview into a popover the sidebar also renders, and the shared-component work is otherwise careful — the collision-boundary edit is narrower than it looks and is correct, stopPropagation on the portaled content is load-bearing rather than decorative, and React 18 ref forwarding got the regression test AGENTS.md asks for. The one thing missing is an audit of the other consumer's tests: keeps long session details inside a constrained WebShell @smoke pins the exact single-line truncation this PR deliberately replaces, and it isn't in the diff.

One correction to how I described that in the review comment, because it changes what a maintainer should do about it. I called the smoke job a gating check. main's ruleset actually requires one approving review plus code-owner review and carries no required-status-check rule, so a red web-shell E2E Smoke will not mechanically stop this merge. That makes resolving it in the PR more important, not less — CI won't be the thing that catches it.

Not blocking, and not the reason for the verdict: the now-dead worktreeOnly prop and the three orphaned i18n keys are cleanups that can ride along with the fix. The untagged Playwright spec is a coverage gap rather than a defect, though tagging at least one of those four cases @smoke would mean the overview is exercised on PRs at all — right now it is only reachable post-merge or nightly, so the interaction changes this PR is built around have no browser coverage in PR CI.

I'm not requesting changes because the scope felt ambitious or because the PR is large. It is neither — the scope is right for the stated goal and the size is mostly tests. It is one specific, verified, deterministic breakage with two clean ways out, both named in the review comment.

中文说明

Confidence: 2/5 —— 设计和测试都不错,总览这部分我今天就可以合;但这份 diff 悄悄让一个跑在本 PR 自己 CI 上的现有浏览器测试失效,需要先解决。

@wenshao 只为一项请求修改——其余部分确实做得很好。

我去核实了问题是否真实存在,而不是采信 PR 的表述:在 main 上,三种被阻塞的状态渲染的是同一个加载动画,同一行按落点不同会有两种点击结果。这些在代码里可以直接观察到,不是描述里的断言。

方案与我自己独立想到的做法一致,有两处还更好——基于已有的 SessionCardStatus 排序派生状态列而不是新增一套平行状态机;通过 DaemonSessionSummary 本来就定义的 isWaitingForPermission / isWaitingForUserQuestion 回传,因此 SDK 接口不动。总览面板也变小而不是变大:SessionIdCell 和三个列都被删掉了。半年后回看,我会感谢写这段代码的人。

这个阻塞项看起来是接缝处的疏漏,而不是草率,所以是"请求修改"而不是"重新设计"。diff 越过总览改到了侧栏同样渲染的浮窗,而共享组件这部分其余工作都很细致——碰撞边界的改动比看起来窄而且是正确的,portal 内容上的 stopPropagation 是必需的而非装饰,React 18 的 ref 转发也按 AGENTS.md 要求补了回归测试。唯一缺的是对另一个使用方的测试做审计:keeps long session details inside a constrained WebShell @smoke 锁定的正是本 PR 有意替换掉的单行截断,而它不在 diff 里。

对我在审查评论里的一个说法做更正,因为它会改变维护者该采取的行动。我把那个 smoke job 称作 gating check。实际上 main 的 ruleset 只要求一个 approving review 加 code-owner review,没有 required-status-check 规则,所以 web-shell E2E Smoke 变红并不会机械地阻止本次合并。这反而让"在 PR 内解决它"更重要——CI 不会是抓住这件事的那一环。

不阻塞、也不是本次结论的原因:已成死开关的 worktreeOnly 和三个孤儿 i18n key 属于可以随修复一起带上的清理项。未打标签的 Playwright spec 是覆盖缺口而非缺陷,但给这四个用例中至少一个打上 @smoke,总览才会在 PR 上被真正执行——目前它只能在合并后或每日定时跑到,也就是说本 PR 核心的交互改动在 PR CI 里没有任何浏览器覆盖。

我请求修改,不是因为范围显得过大或 PR 太大。两者都不是——范围与所述目标相称,体量主要在测试。原因是一个具体、已验证、确定性的破坏,且在审查评论里给出了两条干净的解法。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 965c72bd4d71550c71afc8a045c5ddd52f5fba96 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blocker before this can merge.

The shared session-details popover now wraps long titles and drops its title attribute. That breaks the existing @smoke browser test keeps long session details inside a constrained WebShell @smoke in packages/web-shell/client/e2e/web-shell.collapsed-groups-persist.spec.ts, which pins single-line ellipsized truncation and matches the title via getByTitle. That spec is not in this diff, and it does run in this PR's CI through the web-shell E2E Smoke job (queued behind Test, so it hasn't reported yet).

Two clean ways out, both written up in my review comment: keep truncation in the sidebar and wrap only in the overview — worktreeOnly is already that seam — or decide wrapping is right everywhere and update the smoke spec in this PR. Worth knowing that main's ruleset has no required-status-check rule, so CI won't mechanically stop the merge if this is left as is.

Two cleanups can ride along with the fix: worktreeOnly is now a dead switch (its only caller was the deleted Git column), and sessionsOverview.sessionId / .folder / .worktree are orphaned in both locales.

Everything else looks good — the design doc, the reuse of the existing status ranking and DaemonSessionSummary fields, and unit tests that genuinely pin the row-click and status-priority changes. 🙏

Qwen Code · qwen3.8-max-2026-09-02

@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

CI fix verification

Fixed the constrained session-details regression reported by CI. The smoke test now checks complete wrapped titles instead of the removed native title attribute; the popover stays within the available height and scrolls to keep metadata and the copy action accessible. The four overview regression cases are now included in PR smoke runs.

  • Repository build, typecheck and bundle: passed.
  • Focused unit tests: 146 passed, including React 18 ref compatibility.
  • Chromium smoke suite: all 54 passed locally.
  • Bundled CLI browser verification: 5 passed, covering long-title wrapping without clipping at 700×500, 620×400 and 520×320, scrolling to copy the session ID, overview filtering, hover details, keyboard/Escape focus, and row navigation.
  • Focused ESLint, Prettier and diff checks: passed.

The bundled-page run used an isolated local daemon with deterministic session responses. Remote CI is rerunning for this fix; these results are local verification.


已修复 CI 报出的会话详情回归:测试改为验证完整标题换行,浮窗限制在可用高度内,超长内容可滚动查看并复制会话 ID。四项总览回归已加入 PR 的 smoke 测试范围。

整库 build、typecheck、bundle、146 项定向单元测试和本地全部 54 项 Chromium smoke 均通过。另通过本地打包 CLI 运行了五项浏览器测试,确认受限视口中的标题完整性、浮窗边界、滚动复制,以及总览筛选、悬浮详情、键盘焦点和行导航行为。ESLint、Prettier、diff 检查也通过。远端 CI 将重新验证该修复。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • D1-1 packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:34 worktreeOnly dead switch — already reported (comment 5564498785; review 5127758460)
  • D1-2 packages/web-shell/client/i18n.tsx:3096/3098/3100 and 6477/6479/6481 orphaned sessionsOverview.sessionId / .folder / .worktree keys — already reported (comment 5564498785; review 5127758460)

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Outdated
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Outdated
Comment thread packages/web-shell/client/components/SessionOverviewPanel.test.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Outdated
Comment thread packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Outdated
@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

Review follow-up and E2E report

Addressed the 15 inline review threads in 279dffd52f and resolved the merge conflict while preserving the standalone “No workspace” label. The old worktreeOnly switch and three unused locale entries are removed. The previously reported smoke blocker was fixed in the earlier CI follow-up; this revision adds the further interaction and regression coverage requested in review.

Details now replace one another without stealing focus, close when their row leaves the visible filter/page, keep the focused copy action visible, and wrap long branches as well as titles, paths and IDs. Dragging text does not navigate; a plain-cell click during rename preserves the draft. Compact metadata and status have recoverable full text, and the workspace filter exposes its visible state in its accessible name.

Before committing, reverse audits found and fixed keyboard-selection, anchor-remount, stale-open-state and focus-restoration issues. The final unchanged code then completed two consecutive clean reverse-audit passes, including an independent review and verification against the actual bundled Web Shell. The commit hook made no changes to that audited tree.

Validation on macOS with Chromium and deterministic daemon responses:

  • Repository build, typecheck and bundle passed.
  • 208 focused unit tests passed, including React 18 ref compatibility, the workspace/session collision consumers and sidebar collapse behavior.
  • 56 smoke cases passed.
  • 8 browser regression cases passed against node dist/cli.js; additional independent observations confirmed same-row popover replacement without focus theft and closure after live paging removal.
  • 2 dark/light screenshot cases passed and were visually inspected. The overview is now included in the automatic visual preview scenarios, addressing the earlier coverage-gap comment. Existing PR images hosted on the wenshao fork asset branch are preserved.
  • ESLint, Prettier and diff checks passed; task daemons were stopped.

GitHub reports the PR as mergeable with respect to conflicts. Fresh CI is running for this commit; these results describe completed local verification. Real model execution and non-Chromium browsers remain outside this UI verification.

中文说明

已在 279dffd52f 处理 15 条行内评论并解决合并冲突,保留主分支“无工作区”的显示语义。清除了未使用的 worktreeOnly 开关及三个翻译条目,也补上了总览深浅色截图场景。

提交前多轮反向审计发现并修复了文字选择影响键盘激活、悬浮锚点重新挂载、详情状态残留和旧浮窗抢回焦点等问题;最终代码连续两轮没有发现新问题,且通过实际打包页面的独立验证后才提交。提交钩子没有改变审计过的代码。

整库 build、typecheck、bundle,208 项定向单元测试,56 项 smoke,打包页面的八项浏览器回归,以及深浅色两项截图测试均通过。PR 原有 wenshao fork 图床配图保留。GitHub 已确认没有合并冲突,新一轮 CI 正在运行。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

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.

@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 7, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
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/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator Author

CI follow-up and reverse-audit report

Fixed the remaining session-details replacement smoke failure in f21e69c57e. CI traces and independent browser probes reproduced a test race: the count check accepted the old dialog, then the global single-element assertion failed while the old dialog was animating out beside the newly opened one.

The test now verifies the clicked session, waits for the intended hover session and outgoing dialog removal, then confirms the new dialog remains open before checking dismissal. Real animations and existing timeouts are preserved. Only this browser test changed. The remote branch advanced with another main merge during verification; this commit preserves that merge, and the build, typecheck, unit, smoke and bundled-page checks below were rerun on the merged version before committing.

Reverse auditing found an additional false-positive window: uniqueness alone could accept a target already closing after focus theft. The final open-state assertion rejects that case. After this correction, the unchanged final diff completed two consecutive clean reverse-audit passes before commit. Commit hooks preserved the exact audited tree.

Validation:

  • Repository build, typecheck and bundle passed; 208 focused unit tests passed.
  • All 59 Chromium smoke tests passed with retries disabled in CI mode.
  • 8 tracked browser cases passed against the actual bundled Web Shell.
  • The final replacement case passed with 100ms and 600ms real animations. Injected missing-incoming, retained-outgoing and focus-theft failures were rejected; removing only the final open-state assertion made the focus-theft control falsely pass.
  • Focused ESLint, Prettier and diff checks passed.

All 15 previous inline threads remain resolved, with no new unresolved thread. The latest bot comment reports its review workflow timeout and contains no new code finding. Existing PR screenshots on the wenshao fork asset branch are preserved. These are completed local results; fresh remote CI is verifying this commit.

中文说明

已在 f21e69c57e 修复会话详情切换测试的时序错误:旧浮窗退出动画与新浮窗短暂共存时,全局单元素断言会误报。测试改为依次验证指定会话打开、旧浮窗退出、新浮窗仍然打开以及移出关闭,保留真实动画和原有超时,本轮只修改这一项浏览器测试。

反向审计进一步发现并补上了焦点回抢可能导致测试错误通过的窗口,最终代码在提交前连续两轮未发现新问题,提交钩子未改变审计过的代码。整库 build、typecheck、bundle、208 项单元测试、关闭重试的 CI 模式下全部 59 项 smoke,以及实际打包页面的 8 项回归均通过。正常和延长动画通过;新浮窗未打开、旧浮窗未退出、旧浮窗抢回焦点三类故障均被识别。

15 条原有行内评论仍全部解决,没有新增未解决评论;最新机器人评论仅报告其审查流程超时。PR 中 wenshao fork 资产分支上的配图保留。以上是本地验证结果,远端 CI 正在验证新提交。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not explored to full depth (tool budget reached): chunk 10: none — no check was cut short. (I did not run the repo-wide npm run typecheck ; packages/web-shell/tsconfig.json excludes client/e2e/** and *.test.* , so …; "agent reverse-audit (round 1)": did not execute the new Playwright spec (it needs a web-shell build plus the dev-server webServer boot); every assertion above was validated statically agai…; "agent reverse-audit (round 1)": the new Playwright visual case in client/e2e/visuals/screenshots.spec.ts was walked statically only — this review worktree has no built bundle or browser, so ….

Not reviewed: reverse audit — stopped before round 5 by the review time budget.

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/web-shell/client/components/SessionOverviewPanel.tsx:74 — [review] Inert TooltipProvider and an unreachable, untested meta.tooltip branch in the shared data-table
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:582 — [probe] The filter/search reset test survives reducing the reset effect's dependency list to []
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:563 — [probe] A live status change silently shrinks a batch selection under any non-all filter, and no test can see the prune
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:1140 — [probe] The rewritten footer test drops the base's data-variant assertion, leaving batch-button variants unpinned
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:520 — [probe] Substring search is unpinned on the title, session-ID and branch arms (a startsWith refactor ships green)
中文说明

仅完成部分审查,审查缺口已披露。

未探索到全部深度(达到工具调用预算):chunk 10:none — no check was cut short. (I did not run the repo-wide npm run typecheck ; packages/web-shell/tsconfig.json excludes client/e2e/** and *.test.* , so …"agent reverse-audit (round 1)"did not execute the new Playwright spec (it needs a web-shell build plus the dev-server webServer boot); every assertion above was validated statically agai…"agent reverse-audit (round 1)"the new Playwright visual case in client/e2e/visuals/screenshots.spec.ts was walked statically only — this review worktree has no built bundle or browser, so …

未审查:反向审计——评审时间预算不足,未能开始第 5 轮。

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.test.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.test.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx Outdated
Comment thread packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx Outdated
Comment thread packages/web-shell/client/e2e/visuals/screenshots.spec.ts
Comment thread packages/web-shell/client/e2e/web-shell.collapsed-groups-persist.spec.ts Outdated
@qwen-code-dev-bot

qwen-code-dev-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

AutoFix round 4 finishedview run. See this round's report below.

中文说明

AutoFix 第 4 轮已完成 —— 查看运行。本轮报告见下方。

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 2/100) — it will retry on the next scan.

⚠️ This change was NOT pushed — any commit referenced below was made only in the runner workspace and has been discarded. What the agent reported:
Qwen failed during address-review: timeout (7200000ms).

See the Qwen Autofix agent step logs for model/tool output.

中文说明

🤖 AutoFix 在完成前耗尽了时间(timeout (7200000ms))(第 2/100 次尝试)—— 将在下次扫描时重试。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/34231179206


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.0

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • overview visual fixture row-action coverage (e2e/visuals/screenshots.spec.ts:57) — already reported (round-3 deferral list, review 5137868769)
  • batch-button data-variant coverage (SessionOverviewPanel.test.tsx:1397) — already reported (round-2 deferral list, review 5135733031)
  • filter/search reset coverage (SessionOverviewPanel.test.tsx:587) — already reported (round-2 deferral list, review 5135733031)
  • details-slot click-term witness (SessionOverviewPanel.test.tsx:810) — already reported (round-4 deferral list, review 5141881786)
  • overview PR-number search matching (SessionOverviewPanel.tsx:535) — already reported (round-4 deferral list, review 5141881786)
  • workspace span max-w cap coverage (SessionOverviewPanel.tsx:1184) — already reported (round-4 deferral list, review 5141881786)
  • R3-4 status/cue parity loop coverage (SessionOverviewPanel.test.tsx:663) — still stands, already reported (round-3 inline thread)
  • R4-2 rename draft retention on in-table controls (SessionOverviewPanel.tsx:1637) — still stands, already reported (round-4 inline thread; author reproduced and deferred to issue 11390)
  • R4-3 restoreFocusRef reset witness (sidebar/SessionDetailsTooltip.tsx:155) — still stands, already reported (round-4 inline thread)

Not reviewed: test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": did not read the bodies of canArchiveCard / canRenameCard in SessionOverviewPanel.tsx , so I could not decide whether the absence of any positive (enabled-…; "agent reverse-audit (round 1)": whether the pre-PR search filter also matched card.workspaceCwd — the new placeholder/aria-label enumerates only title, branch, PR and ID, and git show 1a73f…; "agent reverse-audit (round 2)": verifying per-browser document.getSelection() behaviour for a selection inside an open shadow root (needs a Playwright browser run against session-overview-s…; "agent 1c": npm run typecheck / focused vitest runs for SessionOverviewPanel.test.tsx and SessionDetailsTooltip.test.tsx were not executed; type-level and runtime c…; "agent reverse-audit (round 1)": executing the two e2e specs in my chunk ( web-shell.collapsed-groups-persist.spec.ts , visuals/screenshots.spec.ts ) under Playwright — every assertion was ver…, and 3 more.

Not reviewed: reverse audit — stopped before round 4 by the review time budget.

Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:

  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:874 — [probe] nothing pins that blur cancels a rename draft rather than saving it
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:1239 — [probe] rewritten title-cell test dropped the load-bearing min-w-0 assertion
  • packages/web-shell/client/e2e/web-shell.session-overview.spec.ts:281 — [probe] blank-row-space click addressed by positional cell index plus a hardcoded pixel offset
  • packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:119 — [probe] new reopen guard is unwitnessed; deleting it leaves 135/135 green
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:2031 — [probe] deleted contrast loop leaves the Time cell's text-current and the new idle dimming unpinned
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1152 — [probe] title button lost the only focus-openable disclosure of a truncated label
  • packages/web-shell/client/e2e/web-shell.collapsed-groups-persist.spec.ts:145 — [probe] overscroll check identifies the scroller by child position inside a shared primitive
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:1270 — [probe] popover align changed from center to start with no pin left anywhere
  • packages/web-shell/client/e2e/web-shell.session-overview.spec.ts:199 — [probe] link-keeps-popover-open promise asserted only after a re-hover that re-opens it

Convergence: round 5 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/web-shell/client/components/SessionOverviewPanel.tsx (findings in round 4; 2 more now); packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx (findings in round 4; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 9 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查(原文为英文):test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)"did not read the bodies of canArchiveCard / canRenameCard in SessionOverviewPanel.tsx , so I could not decide whether the absence of any positive (enabled-…"agent reverse-audit (round 1)"whether the pre-PR search filter also matched card.workspaceCwd — the new placeholder/aria-label enumerates only title, branch, PR and ID, and git show 1a73f…"agent reverse-audit (round 2)"verifying per-browser document.getSelection() behaviour for a selection inside an open shadow root (needs a Playwright browser run against session-overview-s…"agent 1c"npm run typecheck / focused vitest runs for SessionOverviewPanel.test.tsx and SessionDetailsTooltip.test.tsx were not executed; type-level and runtime c…"agent reverse-audit (round 1)"executing the two e2e specs in my chunk ( web-shell.collapsed-groups-persist.spec.ts , visuals/screenshots.spec.ts ) under Playwright — every assertion was ver…,另有 3 条。

未审查:反向审计——评审时间预算不足,未能开始第 4 轮。

收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 9 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 5 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 4 条(其中 4 条首次提出)。发现反复回到同一批文件:packages/web-shell/client/components/SessionOverviewPanel.tsx(第 4 轮已出过发现,本轮又有 2 条);packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx(第 4 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.23.0)

Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
… session overview (#11238)

Merge main and resolve the SessionDetailsTooltip conflict by combining the
overview's controlled-open/ownerToken props with main's side prop and
split-pane collision boundary.

- deriveSessionCards now counts activeWorkState 'active' as running, so the
  status column/filter no longer labels sessions with live background work
  "Idle" (which also left Archive/Delete enabled for them) while the details
  popover and sidebar reported active work.
- A click-pinned details popover whose row leaves the page mid-poll now
  hands focus to the panel root from a layout-effect cleanup (before React
  removes the row's DOM) instead of dropping it to <body>.
- The row Details button is disabled while an inline rename draft is open,
  so its popover can no longer steal focus and blur-cancel the draft.
- Repair main's split-view App test: it referenced the undeclared
  mockUseDaemonActivePromptBridge (a ChatPane hook App never calls); the
  render witness is mockUseDaemonSessionActivityBridge.

Each new guard is pinned by a witness verified red without it: removing the
activeWorkState term, the focus handoff, or the disabled gate turns the
respective new test red.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 3/100 轮)。改动内容与我反驳保留之处如下:

Review round — PR #11238 (commit 85c5e61)

Merged origin/main (requested base-conflict resolution) and addressed all
three round-5 Critical findings with minimal, mutation-verified fixes.

Merge conflict resolution

SessionDetailsTooltip.tsx and its test conflicted with main's split-view
work (#11250). Resolution keeps both sides: the overview's
openOnClick/ownerToken/open/onOpenChange controlled-mode props plus
main's side prop and its [data-pane-session-id] collision boundary for
side="bottom"; the default branch keeps the PR's anchor-based
resolveSessionDetailsCollisionBoundary(anchor), which subsumes main's
sidebar call (root → aside fallback order is identical for anchors inside a
sidebar). worktreeOnly stays removed — its only base caller was the old
overview git column this PR rewrote. The test-file conflict was the React
import line; resolved as the union (act, createRef, forwardRef,
ComponentProps) so both the PR's createRef case and main's
popoverContentProps observer compile.

Findings

  • [Critical] R5-1 (rc:3961475077) — "Idle" misclassification of background
    work: fixed.
    deriveSessionCards now classifies running as
    (session.hasActivePrompt ?? status?.hasActivePrompt) || session.activeWorkState === 'active',
    mirroring the sidebar (WebShellSidebar.tsx counts exactly
    activeWorkState === 'active'; 'unknown' stays out). The Status cell,
    the Running/Idle filter counts, and canArchiveCard/canDeleteCard
    (gated on status === 'idle', unchanged) now agree with the details
    popover and sidebar. New witness counts background work as running even without an active prompt asserts the Running label, the loading marker,
    both filter counts, and Archive/Delete disabled.
  • [Critical] R5-2 (rc:3961475090) — focus dropped to <body> when a
    pinned popover's row leaves the page: fixed.
    The handoff cannot live in
    the panel's passive reconcile effect (the row's DOM is gone by then), so
    SessionDetailsTooltip now relocates focus in a useLayoutEffect
    unmount cleanup — React runs a function component's layout cleanups before
    detaching its children's refs/DOM, so the content node and the focused
    element are still observable there. When the unmounted content held focus
    (shadow-aware active-element descent, same pattern as the round-4 hover
    handoff), focus moves to the panel root, which now carries tabIndex={-1}.
    The existing closes details when live state removes its anchor through a %s test now asserts focus sits inside the dialog before the flip and
    lands on [data-web-shell-session-panel] after it, in both filter and
    page modes.
  • [Critical] R5-3 (rc:3961475101) — Details button destroying the rename
    draft: fixed.
    The actions-cell Details button is now
    disabled={Boolean(editingIdentity)}, the panel-side option the finding
    offered. The in-row mousedown guard is untouched (the click is not
    re-swallowed; the disabled button simply never activates), and
    startRename's existing setDetailsOpen(null) keeps the other direction
    mutually exclusive. New witness keeps an inline rename draft when clicking the details button asserts the button is disabled, that a native
    click fires nothing (jsdom honors disabled activation behavior), and that
    the draft, its focus, and the absence of a dialog all survive.
  • [Critical] R4-1 (rc:3958063025) — shadow-DOM focus handoff: verified
    still resolved.
    The round-4 fix in 17ae281 survived the merge intact
    (the shadow descent in openAfterDelay is unchanged); listing the thread
    as resolved.
  • Round-4 Suggestions (R3-4 parity-loop coverage, R4-2 remaining
    title/details/footer arms, R4-3 shared-owner witness) remain deferred to
    Web Shell session overview: follow-up coverage and rename interactions #11390 with their on-thread replies from the previous round; nothing new
    was claimed for them here.

Merge fallout repaired

Main's #11250 added does not rerender App for other split sessions to
App.test.tsx referencing mockUseDaemonActivePromptBridge, which is
declared nowhere and mocks a hook App never calls (it is a ChatPane hook) —
the test throws ReferenceError on main. The merged test file was
byte-identical to main's, so this was inherited, not a conflict
mis-resolution. Repaired minimally: the render witness is now
mockUseDaemonSessionActivityBridge, the declared mock of the hook App
actually invokes per render; both parametrized cases pass.

Mutation probes (each guard proven load-bearing)

  • R5-1: removing || session.activeWorkState === 'active' turns the new
    witness red (expected 'Idle' to be 'Running'); restored green.
  • R5-2: removing the layout-effect handoff turns both closes details …
    arms red (document.activeElement is <body>); restored green.
  • R5-3: removing the disabled gate turns the new witness red
    (expected false to be true on details.disabled); restored green.

Verification

  • npm run build — passed (after all edits)
  • npm run typecheck — passed (after all edits)
  • npm run lint — passed, 0 problems
  • cd packages/web-shell && npx tsc -p tsconfig.json --noEmit — passed
  • cd packages/web-shell && npx vitest run (full suite) — 288 files, 6745
    tests passed
  • npx vitest run client/components/SessionOverviewPanel.test.tsx client/components/sidebar/SessionDetailsTooltip.test.tsx client/App.test.tsx on the committed tree — 933 passed
  • Playwright e2e not run here (no browser binaries in this runner); all
    three findings requested jsdom witnesses, which are in place.
中文说明

评审轮次 — PR #11238(提交 85c5e61)

合并了 origin/main(应要求解决与基线的冲突),并以最小且经过变异验证的修复处理了本轮全部三条 Critical 发现。

合并冲突解决

SessionDetailsTooltip.tsx 及其测试与 main 的分屏改动(#11250)冲突。解决方案同时保留双方:总览的 openOnClick/ownerToken/open/onOpenChange 受控模式属性,加上 main 的 side 属性及其针对 side="bottom"[data-pane-session-id] 碰撞边界;默认分支保留本 PR 基于锚点的 resolveSessionDetailsCollisionBoundary(anchor),它涵盖了 main 的侧栏调用(对于侧栏内的锚点,root → aside 的回退顺序完全一致)。worktreeOnly 保持移除——它在基线上的唯一调用方是本 PR 重写的旧总览 git 列。测试文件的冲突在 React 导入行,取并集(actcreateRefforwardRefComponentProps),使本 PR 的 createRef 用例与 main 的 popoverContentProps 观察器都能编译。

发现处理

  • [Critical] R5-1(rc:3961475077)——后台工作被误标为「Idle」:已修复。 deriveSessionCards 现在以 (session.hasActivePrompt ?? status?.hasActivePrompt) || session.activeWorkState === 'active' 判定 running,与侧栏一致(WebShellSidebar.tsx 只把 activeWorkState === 'active' 计为工作;'unknown' 不并入)。状态单元格、Running/Idle 筛选计数与 canArchiveCard/canDeleteCard(仍以 status === 'idle' 为条件,未改动)现在与详情浮窗和侧栏一致。新增见证 counts background work as running even without an active prompt,断言 Running 标签、加载标记、两个筛选计数以及 Archive/Delete 均为禁用。
  • [Critical] R5-2(rc:3961475090)——已固定浮窗所在行离开页面时焦点掉到 <body>:已修复。 焦点交接无法放在面板的被动对账 effect 里(那时行的 DOM 已被移除),因此 SessionDetailsTooltip 改为在 useLayoutEffect 卸载清理中转移焦点——React 在卸载函数组件时先运行其 layout 清理,再分离子级的 ref/DOM,所以此时内容节点与焦点元素仍可观测。当被卸载的内容持有焦点时(使用与第 4 轮悬浮交接相同的 shadow 感知活动元素穿透),焦点移到面板根节点,面板根节点现在带 tabIndex={-1}。既有用例 closes details when live state removes its anchor through a %s 现在断言:翻转前焦点在对话框内,翻转后落在 [data-web-shell-session-panel] 上;filter 与 page 两种模式都覆盖。
  • [Critical] R5-3(rc:3961475101)——详情按钮销毁重命名草稿:已修复。 操作列的详情按钮现在为 disabled={Boolean(editingIdentity)},即发现中提供的面板侧方案。行内 mousedown 守卫未动(点击没有被重新吞掉——禁用的按钮根本不会激活),startRename 既有的 setDetailsOpen(null) 保持另一方向的互斥。新增见证 keeps an inline rename draft when clicking the details button,断言按钮被禁用、原生 click 不会触发任何事件(jsdom 遵守 disabled 的激活行为),且草稿、焦点与「无对话框」状态全部保持。
  • [Critical] R4-1(rc:3958063025)——shadow DOM 焦点交接:已确认仍然修复。 第 4 轮在 17ae281 中的修复在合并后完整保留(openAfterDelay 中的 shadow 穿透未变),该线程列为已解决。
  • 第 4 轮的 Suggestion(R3-4 图标一致性覆盖、R4-2 剩余的标题/详情/页脚分支、R4-3 共享归属见证)仍按上一轮线程中的回复延期至 Web Shell session overview: follow-up coverage and rename interactions #11390;本轮未对它们声称任何新进展。

合并引入问题的修复

main 的 #11250App.test.tsx 新增了 does not rerender App for other split sessions,引用了从未声明的 mockUseDaemonActivePromptBridge,而且该 mock 对应的 hook 根本不被 App 调用(它是 ChatPane 的 hook)——该测试在 main 上即抛 ReferenceError。合并后的测试文件与 main 逐字节一致,因此这是继承自 main 的问题,而非冲突解决错误。已做最小修复:渲染见证改用 mockUseDaemonSessionActivityBridge(已声明的、App 每次渲染确实调用的 hook 的 mock);两个参数化用例均通过。

变异探针(每个守卫都验证为关键路径)

  • R5-1:移除 || session.activeWorkState === 'active' 后新见证变红(expected 'Idle' to be 'Running');恢复后转绿。
  • R5-2:移除 layout-effect 焦点交接后,closes details … 两条分支都变红(document.activeElement<body>);恢复后转绿。
  • R5-3:移除 disabled 门后新见证变红(details.disabledexpected false to be true);恢复后转绿。

验证

  • npm run build —— 通过(全部修改之后)
  • npm run typecheck —— 通过(全部修改之后)
  • npm run lint —— 通过,0 个问题
  • cd packages/web-shell && npx tsc -p tsconfig.json --noEmit —— 通过
  • cd packages/web-shell && npx vitest run(完整套件)—— 288 个文件、6745 个测试通过
  • 在提交后的代码树上运行 npx vitest run client/components/SessionOverviewPanel.test.tsx client/components/sidebar/SessionDetailsTooltip.test.tsx client/App.test.tsx —— 933 个测试通过
  • 未在此运行 Playwright e2e(本运行器无浏览器二进制);三条发现要求的都是 jsdom 见证,均已就位。

Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。

🧵 Resolved all 4 selected review thread(s). · 已关闭全部选中的 4 条评审线程。

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/模型 kimi-k3 · CLI 0.23.1

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • filter/search reset coverage (SessionOverviewPanel.test.tsx:587) — already reported (round-2 deferral list, review 5135733031; re-recorded in the round-4 and round-5 bodies)

Not reviewed: test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

Not reviewed: issue-fidelity — linked-issue evidence could not be fetched (closing-issue discovery failed: gh >= 2.72.0 required), so the closing-issue set is unknown rather than empty.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": I did not execute packages/web-shell/client/components/SessionOverviewPanel.test.tsx in this worktree, so the finding rests on reading the code paths rather t…; "agent reverse-audit (round 3)": executing the two changed Playwright specs ( web-shell.collapsed-groups-persist.spec.ts , visuals/screenshots.spec.ts ) and the new shadow-DOM harness page in …; chunk 10: the two Playwright specs were reasoned against the component/CSS source but never executed here (no dev server or browser run), so the wrap/no-clip metric asser….

Not reviewed: reverse audit — stopped before round 4 by the review time budget.

Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:

  • packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:143 — [probe] Critical [fails-closed] [new-surface] R5-2: (fix-induced) the new unmount focus handoff excludes…
  • packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:255 — [probe] Critical [fails-closed] [new-surface] A parent-driven close of a click-pinned popover drops…
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:593 — [probe] No test asserts a status-filter count while a workspace is…
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:1299 — [probe] The new PR-badge click test pins only the isolation half,…
  • packages/web-shell/client/components/SessionOverviewPanel.test.tsx:1837 — [probe] The only witness for the new N/M workspaces label cannot…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:225 — [probe] activeWorkState 'unknown' reads Idle in the new Status…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:782 — [probe] Clicking a row's own Rename button while its draft is open…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1313 — [review] The new Details button is the only action button with no…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1443 — [probe] The clamp-window early return preserves hover-triggered…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1448 — [probe] After the reconcile effect discards a rename draft,…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1678 — [probe] The status-filter button renders its label and count with…
  • packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:145 — [probe] The new focus rescue resolves its target through an…
  • packages/web-shell/client/e2e/web-shell.session-overview.spec.ts:142 — [probe] The e2e step meant to witness 'a filter change clears the…

Convergence: round 6 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx (findings in round 5; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查(原文为英文):test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

未审查(原文为英文):issue-fidelity — linked-issue evidence could not be fetched (closing-issue discovery failed: gh >= 2.72.0 required), so the closing-issue set is unknown rather than empty.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)"I did not execute packages/web-shell/client/components/SessionOverviewPanel.test.tsx in this worktree, so the finding rests on reading the code paths rather t…"agent reverse-audit (round 3)"executing the two changed Playwright specs ( web-shell.collapsed-groups-persist.spec.ts , visuals/screenshots.spec.ts ) and the new shadow-DOM harness page in …;chunk 10:the two Playwright specs were reasoned against the component/CSS source but never executed here (no dev server or browser run), so the wrap/no-clip metric asser…

未审查:反向审计——评审时间预算不足,未能开始第 4 轮。

收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改;其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 13 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 6 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx(第 5 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.23.0)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [web-shell E2E Smoke (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@wenshao

wenshao commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

CI repair for 8bee8cc

The failed run at 1044b1c had two independent causes:

  • The newly merged split-pane regression still expected only a workspace basename, while shared session details intentionally display the full path. The assertion now checks the exact fixture path and keeps the existing focus, draft, layout and keyboard checks.
  • A supervisor test gave startup roughly half a second, then could remove its temporary directory while the first startup write was still running. With CI retries the test could eventually pass while the old write produced an unhandled ENOENT. Real test supervisors are now registered for teardown before startup is awaited; teardown closes and awaits them before directory removal. The token used in the successful readiness IPC is retained so replacement metadata cannot redirect cleanup to the wrong credentials.

Validation on macOS, Node.js 22.22.2, Chromium:

  • Full repository build, typecheck and bundle passed; 55 related CLI unit tests and all 70 Web Shell smoke cases passed without test retries.
  • All 5 split-pane tests passed against actual node dist/cli.js assets with deterministic mock-daemon responses and no product asset edits.
  • Original failure reproduced with the unchanged supervisor test body and cleanup: paused real chmod → readiness timeout → directory deletion → ENOENT. With retries, tests showed passed but the job failed for an unhandled rejection.
  • On the final fix, a first-start delay of 6.2 seconds crosses the 5-second readiness deadline; retry succeeds and deletion occurs only after writes and runner shutdown complete, with no unhandled rejection.
  • Deliberately replacing metadata and failing the first attempt now closes the original supervisor: its socket is unreachable by 200ms and both attempts have a recorded server close before the independent observation cleanup. The earlier proposed fixture failed this audit check and was corrected before commit.
  • A genuine native chmod ENOENT still fails the test and reports the original atomic-write stack. The immediate settlement observer does not hide the error.
  • ESLint, Prettier and two consecutive final reverse-audit passes completed on the final code before commit. No production source or UI behavior changed in this CI follow-up; the PR's existing fork-hosted images are preserved.

本轮修复了完整路径断言过时,以及测试超时重试时未等待 supervisor 退出便清理目录的问题。55 项相关单测、70 项 smoke 和实际打包 CLI 上的 5 项分屏测试通过;延迟、元数据替换后提前失败、真实写入错误三条故障路径均已验证。审计发现的原 token 清理问题在提交前修正,随后重新完成连续两轮反向审计。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

11 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • activeWorkState 'unknown' status-column versus popover disagreement (SessionOverviewPanel.tsx:566) — already reported (round-6 deferral list, review 5148947616)
  • workspace funnel open flag outliving its unmounted popover (SessionOverviewPanel.tsx:1541) — already reported (round-4 deferral list, review 5141881786)
  • inert TooltipProvider left by the column removals (SessionOverviewPanel.tsx:1724) — already reported (round-2 deferral list, review 5135733031)
  • overview visual fixture row-action and PR-badge coverage (e2e/visuals/screenshots.spec.ts:73) — already reported (round-3 deferral list, review 5137868769; re-recorded in the round-5 body)
  • details reopen-guard and copy-badge witness (sidebar/SessionDetailsTooltip.tsx:155) — already reported (round-5 deferral list, review 5146099714; round-4 inline thread R4-3 still stands)
  • overview git search matching versus the shared exact-match rule (SessionOverviewPanel.test.tsx:526) — already reported (round-4 deferral list, review 5141881786; re-recorded in the round-5 body)
  • filter-clears-selection e2e witness (e2e/web-shell.session-overview.spec.ts:142) — already reported (round-6 deferral list, review 5148947616)
  • workspace and branch programmatic field labels (i18n.tsx:3133) — already reported (round-3 deferral list, review 5137868769; re-recorded in the round-4 and round-5 bodies)
  • footer pagination coverage in the selected state (SessionOverviewPanel.test.tsx:1447) — already reported (round-4 deferral list, review 5141881786)
  • batch-button data-variant coverage (SessionOverviewPanel.test.tsx:1449) — already reported (round-2 deferral list, review 5135733031; re-recorded in the round-4 and round-5 bodies)
  • rename-draft discard without focus restore (SessionOverviewPanel.tsx:1443) — already reported (round-6 deferral list, review 5148947616)

Not reviewed: test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": confirming by reading the session-resolution bootstrap ( WorkspaceSessionProvider / connection setup) that page.goto('/') leaves no session loaded, rather th…; "agent reverse-audit (round 2)": did not execute either Playwright spec — the new layout assertions in web-shell.collapsed-groups-persist.spec.ts ( clientHeight > lineHeight , scrollHeight <…; "agent reverse-audit (round 2)": did not verify that the { name, baseBranch } literals added to createOrganizedScenario and to the new visuals/screenshots.spec.ts fixture satisfy every re…; "agent reverse-audit (round 2)": did not confirm that vite dev actually serves and transforms the new harness's inline module script (bare react / react-dom/client specifiers plus ../ind….

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

2 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).

Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:

  • packages/web-shell/client/components/sidebar/SessionDetailsTooltip.tsx:143 — [probe] Critical [fails-closed] [new-surface] R5-2: (fix-induced) the new unmount focus handoff excludes the anchor-parked focus, so a hover-replaced row that then…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1159 — [probe] Critical [fails-closed] [new-surface] the drag-select guard's document-global point-in-time Selection predicate swallows a deliberate title click after any earlier…
  • packages/cli/src/agent-view/supervisor-runner.test.ts:427 — [review] The teardown path this CI-repair commit exists to add — registering a live runner for shutdown *before* readiness is awaited, and shutting it down with the retained token …
  • packages/cli/src/agent-view/supervisor-runner.test.ts:38 — [review] Making the supervisor cleanup the first await in this hook, with Promise.all (which rejects), means a rejecting supervisor short-circuits afterEach before the fake se…
  • packages/web-shell/client/components/SessionOverviewPanel.tsx:1307 — [review] the two new session-details entry points ignore the host's sessionActions.items embed config that every other surface honours
  • docs/design/web-shell-session-overview.md:1 — [review] new design doc ships English-only, with no zh-CN sibling and no reciprocal language links
  • docs/design/web-shell-session-overview.md:66 — [review] the two Critical focus-loss findings round 6 deferred are in neither issue 11390 nor this doc's out-of-scope list, so no owned record carries them
  • packages/cli/src/agent-view/supervisor-runner.test.ts:436 — [review] the teardown's vi.waitFor budget (10s) equals the resolved afterEach hookTimeout on non-ECS runners, so vitest kills the hook first
  • packages/web-shell/client/e2e/session-overview-shadow-dom.html:16 — [review] the shadow-DOM harness is the only app-level render entry point without React.StrictMode, so its smoke test never sees the effect replay

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 11 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查(原文为英文):test-efficacy probe — the harness control produced no verdict (harnessValidated null), so revert/mutant/hunk efficacy for this delta is unmeasured.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)"confirming by reading the session-resolution bootstrap ( WorkspaceSessionProvider / connection setup) that page.goto('/') leaves no session loaded, rather th…"agent reverse-audit (round 2)"did not execute either Playwright spec — the new layout assertions in web-shell.collapsed-groups-persist.spec.ts ( clientHeight > lineHeight , scrollHeight <…"agent reverse-audit (round 2)"did not verify that the { name, baseBranch } literals added to createOrganizedScenario and to the new visuals/screenshots.spec.ts fixture satisfy every re…"agent reverse-audit (round 2)"did not confirm that vite dev actually serves and transforms the new harness's inline module script (bare react / react-dom/client specifiers plus ../ind…

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

2 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论;CLI 已将其移入下方延后清单(下限强制执行)。

收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改;其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 9 条(原文未翻译,列表见上方英文部分)。

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff,除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上;并会一直如此,直到某一轮的标记重新带上锚点,或落地的嫁接能被运行该轮的评审使用。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

— qwen3.8-max via Qwen Code /review (v0.23.1)

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix round — no change required

Actionable feedback this round

Only one actionable item was newer than the last evaluation: the inline
Critical rc:3963949409 (R6-1) — the shared session-details popover's
workspace fallback switched from workspaceLabel ?? workspaceBasename(folderPath)
to workspaceLabel ?? folderPath, which falsified the exact-text assertion in
packages/web-shell/client/e2e/web-shell.split-persist.spec.ts:171-173 (a spec
this PR does not otherwise touch) and turned web-shell E2E Smoke red.

Everything else in this round's feedback requests no work:

  • The round-6 and round-7 review bodies carry deferral lists explicitly marked
    "recorded, not requested in this round" — an audit record, not work.
  • 12 Suggestion-level findings were confirmed as already reported on this PR and
    were not repeated.
  • @wenshao's issue-level comment describes the CI repair that landed as
    8bee8ccad2; it requests nothing further.
  • The failed-checks sections are empty: CI is green at HEAD.

Why no commit was made

The defect was already repaired at HEAD by 8bee8ccad2 ("test: Fix CI details
assertion and supervisor teardown"), and this round re-verified that the fix
holds rather than editing again:

  • SessionDetailsTooltip.tsx keeps the full-path contract in both places the
    finding said must not regress: visible text {workspaceLabel ?? folderPath}
    and title={workspaceLabel ?? folderPath} (line ~274).
  • web-shell.split-persist.spec.ts:171 now asserts
    details.getByText(WORKSPACE_CWD, { exact: true }) where
    WORKSPACE_CWD = '/tmp/qwen-web-shell-e2e' is the fixture's full cwd — the
    "update the untouched spec to the contract it now has" direction the finding
    offered, matching the full path the chat-pane popover renders when
    workspaceLabel is undefined.
  • web-shell.session-overview.spec.ts:181 (toContainText(workspaceCwd)) is
    intact, so the overview's full-path assertion stays green.
  • web-shell.collapsed-groups-persist.spec.ts:119 locates the same cell via
    getByTitle(scenario.workspaceCwd, { exact: true }); because title was not
    shortened, that contract and the wrap assertion at :132 remain satisfiable.
  • The same commit also fixed the second, independent CI cause: test supervisors
    are registered for teardown before startup is awaited, teardown closes and
    awaits them before directory removal, and the readiness-IPC token is retained
    so replacement metadata cannot redirect cleanup.
  • The round-7 review ran against this exact head and posted zero findings
    (COMMENTED, empty ledger).

The thread rc:3963949409 is recorded in resolved-comments.txt: the finding
is resolved in the code at HEAD, and leaving a fixed Critical open would read
as an unaddressed Critical.

Verification

Commands actually run in this round, with results:

  • npm run build (repo root) — passed
  • cd packages/cli && npx vitest run src/agent-view/supervisor-runner.test.ts
    10 passed
  • cd packages/web-shell && npx vitest run client/components/sidebar/SessionDetailsTooltip.test.tsx client/components/sidebar/sessionDetailsCollisionBoundary.test.ts
    21 passed
  • cd packages/web-shell && npx vitest run client/components/SessionOverviewPanel.test.tsx
    120 passed
  • Playwright witness (web-shell.split-persist.spec.ts:140 +
    web-shell.session-overview.spec.ts:162, chromium) — not runnable in this
    sandbox
    : Chromium cannot launch (libglib-2.0.so.0 missing), the default
    browser-cache path is EACCES, and no sudo/root exists for
    playwright install --with-deps. The spec↔component contract was instead
    verified statically line-by-line as itemized above, and CI at HEAD (empty
    failed-checks list, per the prepared feedback) plus the maintainer's reported
    70/70 green smoke run are the executing witnesses for the browser assertion.

No source, test, or spec file was modified this round; the working tree is
clean at 8bee8ccad2.

中文说明

Autofix 本轮——无需改动

本轮可执行的反馈

比上次评估更新的反馈中只有一条可执行项:行内 Critical rc:3963949409(R6-1)——共享会话详情浮窗的工作区兜底从
workspaceLabel ?? workspaceBasename(folderPath) 改成了
workspaceLabel ?? folderPath,导致本 PR 未修改的
packages/web-shell/client/e2e/web-shell.split-persist.spec.ts:171-173
中的精确文本断言失效,并使 web-shell E2E Smoke 变红。

本轮反馈中的其余内容都不要求改动:

  • 第 6、7 轮评审正文携带的延后清单明确标注「已记录,本轮不要求修改」——属于审计记录,不是工作项。
  • 12 条 Suggestion 级发现被确认为已在 PR 上报告过,未重复发布。
  • @wenshao 的 issue 级评论是在描述以 8bee8ccad2 落地的 CI 修复,没有进一步要求。
  • 失败检查列表为空:CI 在 HEAD 上是绿的。

为什么本轮没有提交

该缺陷已在 HEAD 由 8bee8ccad2("test: Fix CI details assertion and
supervisor teardown")修复,本轮做的是重新验证修复成立,而不是再次修改:

  • SessionDetailsTooltip.tsx 在该发现要求不得回退的两处都保持了完整路径契约:可见文本 {workspaceLabel ?? folderPath}title={workspaceLabel ?? folderPath}(约第 274 行)。
  • web-shell.split-persist.spec.ts:171 现在断言 details.getByText(WORKSPACE_CWD, { exact: true }),其中 WORKSPACE_CWD = '/tmp/qwen-web-shell-e2e' 是 fixture 的完整 cwd——正是该发现给出的「把未修改的 spec 更新为当前契约」方向,与聊天面板浮窗在 workspaceLabel 为 undefined 时渲染的完整路径一致。
  • web-shell.session-overview.spec.ts:181toContainText(workspaceCwd))保持不变,总览的完整路径断言保持绿色。
  • web-shell.collapsed-groups-persist.spec.ts:119 通过 getByTitle(scenario.workspaceCwd, { exact: true }) 定位同一单元格;由于 title 未被缩短,该契约以及 :132 的换行断言仍然可满足。
  • 同一提交还修复了第二个独立的 CI 原因:测试 supervisor 在 await 启动就绪之前就注册进 teardown;teardown 在删除目录之前关闭并等待它们;并保留就绪 IPC 所用的 token,使替换元数据无法把清理导向错误的凭据。
  • 第 7 轮评审正是针对这个 head 运行的,发布了零条发现(COMMENTED,ledger 为空)。

线程 rc:3963949409 已记录在 resolved-comments.txt 中:该发现在 HEAD 的代码中已解决,让一个已修复的 Critical 保持开放会被误读为未处理的 Critical。

验证

本轮实际运行的命令及结果:

  • npm run build(仓库根目录)——通过
  • cd packages/cli && npx vitest run src/agent-view/supervisor-runner.test.ts——10 通过
  • cd packages/web-shell && npx vitest run client/components/sidebar/SessionDetailsTooltip.test.tsx client/components/sidebar/sessionDetailsCollisionBoundary.test.ts——21 通过
  • cd packages/web-shell && npx vitest run client/components/SessionOverviewPanel.test.tsx——120 通过
  • Playwright 见证(web-shell.split-persist.spec.ts:140 + web-shell.session-overview.spec.ts:162,chromium)——在本沙箱中无法运行:Chromium 无法启动(缺少 libglib-2.0.so.0),默认浏览器缓存路径报 EACCES,且没有 sudo/root 权限执行 playwright install --with-deps。作为替代,已按上文逐行静态核验了 spec 与组件的契约一致性;HEAD 上的 CI(准备好的反馈中失败检查列表为空)以及维护者报告的 70/70 全绿 smoke 运行是该浏览器断言的执行见证。

本轮未修改任何源码、测试或 spec 文件;工作树在 8bee8ccad2 上保持干净。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

🧵 Resolved all 1 selected review thread(s). · 已关闭全部选中的 1 条评审线程。


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.1

@wenshao

wenshao commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@wenshao
wenshao enabled auto-merge September 9, 2026 11:47
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ❌ not passed — findings reported (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: 58 passed · 0 failed · 58 total

Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:58 通过 · 0 失败 · 58 总计

抖动门:✅ 4 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR 11238 deep verification — feat(web-shell): improve session overview navigation and details

Verdict: findings — 58 scripted assertions executed, 58 pass / 0 fail. The central
claim is proven load-bearing by a two-arm A/B against the base build, and every gate I ran is
green. Two concrete items are worth a reviewer's attention before merge (Findings 1 and 2);
neither is a regression against base.

  • Verified head OID: 8bee8ccad287b7ccc8cd66dd1e8f98f0030cbb68 (git rev-parse HEAD^2)
  • Base arm: cf987cdc59997b9ea935226f592bc5be1e121a06 (HEAD^1, the merge-ref base tip)
  • Round: first (no previous-report.md beside $QWEN_VERIFY_CONTEXT)
中文摘要

结论:findings —— 共执行 58 条脚本化断言,58 通过 / 0 失败。中心论点已通过
base/head 双构建 A/B 证明为真实生效,所有执行的门禁均为绿色。合并前有两项值得审阅者
关注(见 Findings 1、2),二者均非相对 base 的回归

  • A/B 结论:见 Part APart B 两张表。activeWorkState === 'active' 的会话在
    base 上被派生为 idle、在 head 上被派生为 running;对应渲染结果是该行的
    Archive / Delete 按钮由可点击变为禁用。其余 8 个 fixture(含权限、提问、
    状态报告优先级、真空闲)两臂完全一致,无附带影响。证据图:
    01-ab-status-and-mutation-gate.png
  • 变异矩阵:见 Mutation matrix 表。PR 声称的 4 个"见证测试"全部被重新测量并确认
    有效(移除任一守卫都会让对应那一条断言以预期值变红),另有一条工具级阳性对照
    命中 8 个测试。证据图:02-mutation-matrix.png
  • findings
    1. activeWorkState === 'unknown' 仍被派生为 idle,总览"状态"列显示 Idle
      Archive/Delete 可用,而同一行的详情浮窗显示 Background activity unknown
      —— 两个界面自相矛盾,且与 acp-bridge 自身"对未知一律按繁忙处理(fail-closed)"
      的书面策略相反。用户主动删除路径在服务端没有任何兜底。
    2. 全部 26 个 e2e 工件中没有任何一个出现 activeWorkState;把该守卫删掉后
      12/12 浏览器用例仍然全绿 —— 即这个 PR 自称重点的修复在浏览器层完全没有被钉住。
    3. 设计文档缺少 .zh-CN.md 配对与标题下的语言链接;但 docs/design 下 485 篇非
      .zh-CN 设计文档中仅 1 篇有中文配对,属仓库级既有惯例缺口,非本 PR 首创。
    4. 更正:PR 描述中"深浅色视觉用例通过"的表述需要限定 —— 视觉套件不做基线比对,
      通过仅意味着"渲染成功并写出了截图"。
  • 未覆盖范围:逐 commit 归因(浅克隆,本地仅 1 个 commit 可达,快照记录 14 个)、
    非 Chromium 浏览器与 mobile-chromium project、真实 daemon/模型执行(因此 Finding 1
    的最终后果未做端到端实证)、全仓库测试套件、npm run build/bundle 未由本轮重跑、
    Reviewer Test Plan 未做人工浏览器逐步操作(改为运行编码了各步骤的自动化用例)。

Scope

The diff is 2,580 insertions / 809 deletions over 15 files: one new design doc, one CLI
test-only file, and the rest inside packages/web-shell/client (the overview panel, the
shared details popover, the collision-boundary helper, i18n messages, three e2e specs, one
visual spec, one shadow-DOM fixture). No package.json, no lockfile, no daemon route, no
cross-package production change — so the base worktree could share the root node_modules
as a clean control (verified below).

Central claim (the one behaviour the PR exists to change, and the only one with a
data-loss consequence): the overview's derived session status is truthful about live
background work, so a session doing background work is no longer labelled Idle — which is
also what gates Archive and Delete.

Secondary claims (verified by the specs I ran, not independently A/B'd):
row click navigates while checkboxes exclusively select; the sidebar's full details popover
is reused in the overview (session ID, workspace path, PR/issue links, copy) with
owner-token-scoped focus rescue; distinct approval/question/running/idle states with a
spinner only for running.

Explicitly out of scope for this round: everything listed under Not covered.

Central claim — A/B against the base build

Both arms ran the identical harness file (verify-ab.test.tsx, kept in this artifact
dir), dropped unmodified into tmp/base-tree (at HEAD^1) and into the head tree. Cells
were captured live as 01-ab-status-and-mutation-gate.png.

Part A — deriveSessionCards status derivation (pure exported function)

fixture base (HEAD^1) head flip
A1 activeWorkState:'active', no prompt signal idle running FLIP
A2 activeWorkState:'active', hasActivePrompt:false idle running FLIP
A3 'active' + status report hasActivePrompt:true running running
A4 sibling activeWorkState:'unknown' idle idle
A5 sibling activeWorkState:'unsupported' idle idle
A6 activeWorkState:'idle' idle idle
A7 activeWorkState absent (older daemon) idle idle
A8 'active' + pending permission via status report needsApproval needsApproval
A9 'active' + isWaitingForUserQuestion askUserQuestion askUserQuestion
A10 hasActivePrompt:true, activeWorkState absent running running

Oracle: the status field of the derived card. 2 of 10 cells flip; the other 8 are
byte-identical, so the change is confined to exactly the value it names and does not disturb
the approval/question precedence it sits below.

Part B — the rendered consequence (jsdom, real component, no stub of the unit under test)

fixture base Archive/Delete disabled head Archive/Delete disabled flip
B1 activeWorkState:'active' only false / false (both enabled) true / true (both disabled) FLIP
B2 sibling 'unknown' only false / false false / false
B3 sibling 'unsupported' only false / false false / false
B4 truly idle ('idle') false / false false / false

Oracle: the disabled property of the buttons carrying aria-label="Archive" /
"Delete". This is why the status term matters — canArchiveCard and canDeleteCard are
byte-identical on both arms (both require card.status === 'idle'), so the PR's one-term
change to the status derivation is the only thing standing between a session doing background
work and a destructive mutation. B4 proves the fixture isolates that term: a genuinely idle
row keeps both mutations enabled on head.

Part C — the two status surfaces for one row

The PR deletes the popover's worktreeOnly prop, so head is the first version where the
overview renders both a Status column and a popover status line for the same row. In base
both were absent from the overview (measured: no [data-web-shell-session-status] element, no
[data-web-shell-session-details] button), because base's overview popover was the git-only
variant whose status and session-ID rows sat inside {!worktreeOnly && …}.

activeWorkState column attr column text popover text agree Archive/Delete disabled narrow cue
'active' running Running Running yes true / true running
'unknown' idle Idle Background activity unknown NO false / false null
'unsupported' idle Idle Idle · 0 client(s) yes false / false null
'idle' idle Idle Idle · 0 client(s) yes false / false null
absent idle Idle Idle · 0 client(s) yes false / false null

Real browser renders of the head build are in 03-overview-head-chromium-dark.png (overview,
four distinct states) and 04-details-popover-head-chromium-light.png (the reused details
popover).

Control validation

The monorepo symlink trap was checked rather than assumed. The diff touches no
package.json/lockfile (git diff --name-only HEAD^1..HEAD | grep -E 'package(-lock)?\.json'
→ none), so sharing the root node_modules is dependency-clean. Each arm's harness recorded
its own import.meta.url and the driver asserted it resolved inside the expected tree
(base: component resolved inside the base tree → PASS; same for head). ./SessionOverviewPanel
is a relative specifier, so Vite resolves it against the harness's own path; the one internal
package that does cross into the head tree (@qwen-code/sdk) is reached only through
import type, which esbuild strips, and @qwen-code/web-shell/daemon-react-sdk is both
aliased to each tree's own client/daemon-react-sdk.ts and mocked outright by the harness.

Mutation matrix — the PR's own claimed witnesses, re-measured

The merge commit states: "Each new guard is pinned by a witness verified red without it:
removing the activeWorkState term, the focus handoff, or the disabled gate turns the
respective new test red."
That is a falsifiable claim, so it was re-measured rather than
accepted. Cells captured live as 02-mutation-matrix.png. Sources were restored after every
mutant and the restore verified by sha256 (client/components/SessionOverviewPanel.tsx restored byte-for-byte → PASS, likewise the tooltip).

mutant guard suite result killed first failing test assertion quoted
M0 control none (unmutated) 141 passed green as required
M1 deriveSessionCards: drop || session.activeWorkState === 'active' 1 failed yes counts background work as running even without an active prompt expected 'Idle' to be 'Running'
M2 details button: disabled={Boolean(editingIdentity)}disabled={false} 1 failed yes keeps an inline rename draft when clicking the details button expected false to be true
M3 tooltip: drop the layout-effect focus handoff to [data-web-shell-session-panel] 2 failed yes closes details when live state removes its anchor through a filter / …through a page expected <body>…</body> to be <div tabindex="-1" …>
M4 tooltip: focus rescue no longer scoped to the shared ownerToken 1 failed yes leaves another owner's pinned details open and focused on hover expected 'closed' to be 'open'
M5 harness control STATUS_PRIORITY inverted (idle first) 8 failed yes ranks needs-approval above user-input above running above idle, then by recency +2 expected ['s-idle','s-run','s-q','s-appr'] to deeply equal ['s-appr','s-q','s-run','s-idle']

Every mutant died on a behavioural assertion naming expected-versus-actual values; none
died on a compile or import break (compileError=false on all rows). Attribution is correct:
each witness test is named after the guard it pins, not an unrelated bystander. M5 is the
harness-level positive control landed in the same file as M1/M2, so "the suite does not cover
this" and "my harness never ran your suite" are distinguishable. No survivor — all four
guards the PR claims are genuinely pinned.

Corrections to the PR description

These are corrections to the description, not requests to change code.

"both dark/light visual cases passed" reads as a visual-regression result. It is not. The
visuals suite contains zero toHaveScreenshot assertions anywhere under client/e2e/, and
captureScreenshot (client/e2e/visuals/harness.ts:164) only calls page.screenshot({ path })
after freezing animations. I ran the two new cases and they did pass — 2 passed (9.3s),
producing session-overview-{dark,light}.png and session-overview-details-{dark,light}.png
but "passed" here means the page rendered, the expected text and dialog were present, and the
PNG was written
, not that pixels were compared against a committed baseline. The before/after
threshold the harness comments mention lives in separate tooling, not in these tests. The
substantive assertions in those two cases (the overview contains the fixture title; the details
dialog is visible with the accessible name Review release approval) did execute and did pass.

Everything else in the description's validation block that I could check held up exactly:
"all 70 Chromium smoke cases" → 70/70; "220 focused unit tests" is a superset count I did not
reproduce, but the three changed test files give 141 and the whole package gives 6,769;
"repository build, typecheck and bundle passed" → typecheck re-run here and green.

Findings

1. activeWorkState: 'unknown' is labelled Idle with Archive/Delete enabled — the overview fails open where its own data source documents fail-closed (Suggestion)

Measured, from Part A / Part B / Part C above: a session whose only live-work signal is
activeWorkState: 'unknown' derives status: 'idle', renders "Idle" in the new Status
column, renders "Background activity unknown" in the details popover for the same row,
shows no narrow-layout cue, and offers Archive and Delete enabled.

Why 'unknown' is not a theoretical value — the producer's own documentation
(packages/acp-bridge/src/bridge.ts):

  • entryActiveWorkState returns 'unknown' when childWorkIsUnknown(entry), i.e. "the
    channel negotiated reporting, but no snapshot recent enough to grade has arrived.
    Never-reported and gone-quiet are the same state on purpose — a snapshot from ten minutes
    ago says nothing about whether a background agent started since"
    — or when the channel's
    capability does not cover all five ACTIVE_WORK_HOLD_CATEGORIES (agent, notification,
    shell, session, workflow).
  • entryHasActiveWork, the bridge's health surface, is documented as "Fails closed on
    ignorance: unknown reads the same as busy
    , because a controller must not be able to
    mistake 'nobody told me' for 'nothing is running'."
  • The bridge's automatic teardown acts on that: it routes through closeIfChildUnheld
    confirmChildUnheld, asking the child before destroying.

The overview is a controller, and it does the thing the comment says a controller must not: it
reads "nobody told me" as "nothing is running" and enables destructive mutations on that
basis. The user-initiated path has no server-side backstop — I traced it rather than assumed
it: deleteSessions (routes/session.ts) → deleteDaemonSessions (server/session-archive.ts,
which contains no hasActivePrompt / activeWork / busy refusal) → bridge.closeSession
closeSessionImpl directly; closeSession does not call closeIfChildUnheld (its only
two call sites, bridge.ts:3419 and :4097, are the automatic-teardown paths). So
canDeleteCard's card.status === 'idle' is the only active-work gate on that path.

Pre-existing cause vs this PR's contribution, named separately:

  • Pre-existing (not this PR's fault): base derived 'unknown'idle identically, and
    canArchiveCard/canDeleteCard are byte-identical on both arms. The permissive gate for
    'unknown' predates this PR.
  • This PR's contribution: (a) it took on making the derived status truthful and closed
    'active' only, leaving the sibling of the same bug class open; (b) by deleting
    worktreeOnly it made the overview render a Status column and a popover status for the same
    row for the first time, so the two now visibly contradict each other ("Idle" vs "Background
    activity unknown") in one surface — measured as the agree = NO row of Part C. That
    contradicts the design doc's own stated goal, "Make the popover status agree with the
    overview's derived live state."

What this finding is NOT (bounded, per the evidence):

  • It is not a regression: base behaved identically for 'unknown', and the PR strictly
    improved the 'active' case (proven by the B1 flip).
  • 'unsupported' is not part of this finding. The bridge explicitly scopes it out: "A
    channel that never negotiated is not 'unknown', it is out of scope: treating it as unknown
    would make every legacy Session unreapable."
    Treating 'unsupported' as deletable is a
    documented, deliberate choice, and Part C shows its two surfaces agree ("Idle" / "Idle · 0
    client(s)"). I am not asking for it to change.
  • A confirmation dialog (sessionsOverview.confirmDeleteTitle, "Delete session?") stands
    between the button and the call, and Archive is reversible.
  • I did not demonstrate end-to-end loss of a genuinely held background task: that needs a
    live runtime with a stale-holds child, which this container does not have. The chain above is
    proven by reading the four call sites plus measuring the UI gate, not by observing a deletion.

No measured patch is offered, deliberately. Either fix direction changes a contract the
author owns: relabelling needs a new SessionCardStatus value (which ripples into
STATUS_PRIORITY, matchesStatus, the filter enum, and four new i18n keys in two locales),
while gating the mutations needs activeWorkState carried onto SessionCard (which today
holds only the derived status). Both are product decisions — blocking Delete on 'unknown'
could over-block whenever a runtime's hold reporting goes stale. What I did measure is the
constraint that applies to whichever is chosen: see Finding 2 — nothing in the repo pins this
axis today, so the fix must ship with its own fixture.

Reproduce:

node tmp/pr11238-verify-20260909-120107/ab-driver.mjs tmp/pr11238-verify-20260909-120107
# Part A row "A4 SIBLING activeWorkState=unknown": base=idle head=idle
# Part C row "unknown": column "Idle" vs popover "Background activity unknown", deleteDisabled=false

2. activeWorkState appears in no browser-level fixture, so the guard the merge commit calls its point is unpinned above unit level (Suggestion)

The merge commit frames the activeWorkState term as the fix that stops Archive/Delete being
enabled for sessions with live background work. It is pinned at unit level (M1 above kills
counts background work as running even without an active prompt). It is pinned at no
other level:

  • Removing the term leaves the new 12-test browser spec fully green12 passed (39.1s),
    log gate-e2e-m1-mutant.log.
  • The reason is a fixture gap, proven by census rather than by reading: 0 of 26 e2e
    artifacts (*.spec.ts and *.html under client/e2e/) contain the string
    activeWorkState. The overview spec drives its four states purely through
    hasActivePrompt / isWaitingForPermission / isWaitingForUserQuestion; so does the new
    visual-capture scenario. The PR's own screenshot evidence therefore never contains a session
    in the state the fix exists for.

The fixture that would go red: add a fifth row to the web-shell.session-overview.spec.ts
state matrix — displayName: 'Background work fixture', hasActivePrompt: false,
activeWorkState: 'active' — and assert [data-web-shell-session-status="running"] plus
Archive/Delete toBeDisabled(), mirroring the loop the spec already runs for the other four
states. That single row is what turns M1 from a unit-only witness into a browser-level one.

Reproduce:

node tmp/pr11238-verify-20260909-120107/coverage-crosscheck.mjs tmp/pr11238-verify-20260909-120107
# "census: NO e2e artifact anywhere mentions activeWorkState" -> PASS (26 scanned, 0 matches)

3. The sibling axis is pinned in neither direction — a reverse-mutation probe (completeness reporting, not a merge condition)

Per the rule that a suite unable to tell head from head-plus-fix has its coverage gap exactly
where the next regression will land: I applied a candidate change in a scratch copy (treat
'unknown' as 'running' in deriveSessionCards) and re-ran both layers.

layer head head + probe
unit (3 changed test files) 141 passed 141 passed
e2e (web-shell.session-overview.spec.ts) 12 passed 12 passed

Green on both sides is not reassurance — it is proof that no test in the repo asserts anything
about how the overview treats activeWorkState: 'unknown', in either direction. The tooltip's
label is pinned (SessionDetailsTooltip.test.tsx:76'Background activity unknown'); the
overview's contradicting treatment of the same session is not. Whichever way Finding 1 is
resolved, it needs a new fixture, because today a regression here is invisible. Source restored
byte-for-byte (f30b806804c01355 before and after) and git status --porcelain is empty.

4. Design doc has no .zh-CN.md pair and no language links (Suggestion — repo-wide gap, not this PR's)

docs/design/README.md requires "Maintain two complete versions in the same directory:
<name>.md for English and <name>.zh-CN.md for Simplified Chinese"
and "Add relative links
to both versions immediately below each document title."
The PR adds
docs/design/web-shell-session-overview.md with neither: no sibling .zh-CN.md, and the title
is followed directly by ## Problem.

Context that should decide how this is handled, measured rather than assumed: docs/design
holds 485 non-.zh-CN Markdown documents (recursively, across 37 subdirectories) and
exactly 2 .zh-CN.md files — README.zh-CN.md, and
customize-banner-area/customize-banner-area.zh-CN.md. So one design document in 485 has
the required Chinese counterpart. And even that single complying pair does not carry the
reciprocal language links below its title: grep -rl 简体中文 docs/design matches only
README.md and README.zh-CN.md. Neither do the six most recently touched design docs. This
PR's doc follows the same shape as essentially every neighbour. Per the project's own review
rule a translation gap is a Suggestion, not a Critical; recording it here is recording the rule,
not singling out this author. The real decision is whether to start enforcing the convention on
new docs or to treat it as a repo-wide hygiene item.

Reviewer Test Plan — walked step by step

I ran the automated specs that encode each step rather than driving a browser by hand; the
mapping below says which evidence covers which step. No step turned out to be structurally
unreachable
— every one maps onto a spec that exists and passes.

step claim evidence result
1 four distinct states, spinner only for running; filters combine with title/branch/PR (#123)/ID search; changing a filter clears selection and returns to page 1 e2e compact overview distinguishes states… asserts all four data-web-shell-session-status labels + title attrs and Session ID column count 0; the reset effect (SessionOverviewPanel.tsx:1026) zeroes pageIndex, clears rowSelection, closes details, keyed on exactly [excludedWorkspaceCwds, searchQuery, statusFilter]; search covers label/sessionId/gitBranch/#<pr.number> covered, passed
2 hover for full details; Enter on the details button; copy ID; PR/issue links; Escape returns focus; small-viewport wrap and scroll to the copy action e2e title hover exposes full metadata, permits copy, and links keep overview open, details button supports keyboard and Escape restores focus without navigating, overview details stay inside an embedded shell; my Part C read the popover's [data-web-shell-session-id] and status row directly covered, passed
3 row click opens the session; drag-select keeps the overview open; plain-cell click during rename preserves the draft; checkboxes reveal batch actions; restrictions on active sessions and cross-workspace ops remain e2e row opens a session without selecting it, keeps text selection and rename drafts in the overview; M2 mutant killed the draft-preservation witness; canUseSessionMutation gates cross-workspace; Part B/Part C measured the active-session restriction and found its 'unknown' residual (Finding 1) covered, passed, with Finding 1
4 sorting during rename cancels the draft and keeps focus; Shadow DOM hover replacement keeps focus on the incoming title; focus preserved from a search field or a different popover owner e2e sorting during rename preserves keyboard navigation with N sessions (2 params), shadow portal details keep focus in the overview after hover replacement, hover details preserve a focused search field; M4 mutant killed the owner-token witness covered, passed
5 rename-then-cancel leaves details closed; a live update moving the row off-page discards the hidden draft; a catalog shrink preserves visible details/draft; at 499px each active/attention row keeps its title icon e2e attention cues stay visible in the pinned title at narrow widths; M3 mutant killed closes details when live state removes its anchor through a filter and …through a page covered, passed

Gates

gate command result
web-shell unit (whole package) npx vitest run in packages/web-shell 290 files / 6,769 tests passed, 0 failed (89.8s)
the three changed test files npx vitest run client/components/SessionOverviewPanel.test.tsx client/components/sidebar/SessionDetailsTooltip.test.tsx client/components/sidebar/sessionDetailsCollisionBoundary.test.ts 141 passed (120 + 17 + 4)
CLI test touched by the follow-up commit npx vitest run src/agent-view/supervisor-runner.test.ts in packages/cli 10 passed (817 ms)
Chromium smoke npx playwright test --grep @smoke --project=chromium 70 passed (1.7 m)
new overview spec npx playwright test client/e2e/web-shell.session-overview.spec.ts 12 passed (39.1 s)
other changed specs … web-shell.collapsed-groups-persist.spec.ts web-shell.split-persist.spec.ts 7 passed (24.8 s)
new visual captures npx playwright test --config playwright.visuals.config.ts --grep "session overview" 2 passed, 4 PNGs written (see Correction above)
typecheck npm run typecheck (repo-wide) pass
eslint npx eslint on all 12 changed .ts/.tsx files exit 0, zero output bytes
prettier npx prettier --experimental-cli --check packages/web-shell docs/design/web-shell-session-overview.md packages/cli/src/agent-view/supervisor-runner.test.ts All matched files use Prettier code style!

Both lint gates were proven live before being cited. Appending const plantedUnusedVariable = 42;
to sessionDetailsCollisionBoundary.ts made eslint report
11:7 error 'plantedUnusedVariable' is assigned a value but never used @typescript-eslint/no-unused-vars,
and appending const spaced = 1; made prettier report
[warn] Code style issues found in 1 file. The file was restored to sha 1efc2137830aaf6a,
matching its pre-plant value. An unproven green gate would have been an assumption here, since
eslint exits 0 both when it passes and when it matches nothing.

Not covered

  • Per-commit attribution. The checkout is depth 2: git rev-parse --is-shallow-repository
    true, and git rev-list HEAD^1..HEAD^2 returns 1 commit while
    $QWEN_VERIFY_CONTEXT lists 14. Only the aggregate HEAD^1..HEAD diff was verified. The
    four bot/authored fix commits described in the snapshot (rename-time control clicks, details
    focus ownership, shadow-portal focus, the status/focus/rename merge) were verified only as
    they survive in the aggregate head, not individually.
  • Base OID drift. The snapshot's baseRefOid is eb383fd3bbd4b55f1c5ba8d7da7048193f018230,
    which is not present locally (git cat-file -t fails); main advanced between the
    snapshot fetch and the merge-ref computation. The A/B used HEAD^1
    (cf987cdc59997b9ea935226f592bc5be1e121a06), the base tip GitHub actually merged into. No
    trial merge into a newer main was performed — with only three commits reachable there is no
    newer main locally to merge into.
  • End-to-end demonstration of Finding 1. No live runtime, model, or stale-holds child was
    available, so no actual deletion of a session holding background work was observed. The chain
    is a code-path proof plus a measured UI gate.
  • Non-Chromium browsers and the mobile-chromium project. Only --project=chromium was
    run; Firefox/WebKit are not installed. The PR itself scopes these out.
  • Repo-wide test suite. Only packages/web-shell in full and one CLI test file. packages/core,
    the rest of packages/cli, and the integration suites were not run.
  • npm run build / npm run bundle were not re-run by this round; the workflow built at
    HEAD before the clock started, and I ran npm run typecheck myself instead. The e2e specs ran
    against the vite dev server (source transformed live), not the bundled dist/cli.js, so the
    PR's claim of "against the actual bundled CLI" was not reproduced at that level.
  • The PR's separate E2E comment and the six external screenshot URLs in the description were
    not fetched — this job has no GitHub token and treats the local tree plus the snapshot as the
    whole world.
  • Real daemon responses. Every browser and jsdom run used the specs' own intercepted fake
    daemon. The [WebServer] connect ECONNREFUSED 127.0.0.1:4170 lines in the e2e logs are the
    vite dev proxy finding no real daemon; they are expected noise, and all specs passed anyway.
  • Interactive manual walkthrough. The Reviewer Test Plan was verified through the specs that
    encode it (table above), not by hand-driving a browser.
  • The sessionDetailsCollisionBoundary fallback change was traced but not A/B'd. Its
    behaviour for every caller configuration was reasoned through rather than measured: the
    WorkspaceDetailsTooltip call site is byte-identical between arms, and for a SessionDetailsTooltip
    anchor the new closest('[data-web-shell-root]') ?? closest('aside') ?? null chain matches
    base's closest('aside') ?? null result in the sidebar cases (with and without a shell root)
    and differs only for the overview anchor, where it now constrains to the Web Shell instead of
    returning null — which is the intended change and is pinned by the new test resolves an
    overview anchor inside a shell without an aside
    . Flagging it as reasoned, not measured.

Methodology

Everything ran in the CI verify container (node:22-bookworm, node v22.23.2, npm 10.9.8) on
the merge-ref checkout at 3582b7787a, with npm ci and npm run build already completed by
the workflow. The base arm was a scratch git worktree at tmp/base-tree (HEAD^1), nested
under the repo so it resolved node_modules by walking up to the root — no per-tree install —
and removed with git worktree remove --force once the A/B cells were captured; the tree is
currently clean (git status --porcelain empty, asserted by the coverage cross-check).

Three drivers, all kept in this artifact dir so a maintainer can rerun them verbatim:
ab-driver.mjs copies the same verify-ab.test.tsx into both trees and runs it under each
tree's own packages/web-shell/vitest.config.ts, so the arms differ only by the code under
test; the harness mocks only the daemon-facing seams (daemon-react-sdk, useScopedSessions,
the live-state hook, the catalog controller) and never the unit under test — deriveSessionCards
and SessionOverviewPanel are the real compiled components, rendered through react-dom/client
in jsdom, with the real SessionDetailsTooltip portalling into the document.
mutation-driver.mjs applies each mutant as an exact single-occurrence string replacement
(throwing if the anchor is not unique), runs the relevant suites, classifies the red as
behavioural versus compile breakage, and restores with sha256 verification.
coverage-crosscheck.mjs asserts over the recorded run logs plus a filesystem census.

Assertion bookkeeping follows the skill contract: an A/B control cell asserts that the base
arm shows the pre-fix behaviour, so base going red as predicted counts as a pass; fail
counts only unexpected outcomes. Two early failures were harness defects, not PR defects, and
were fixed and re-run rather than shipped: Part B initially executed at module-collection time
before beforeEach installed the mocks, so the ErrorBoundary swallowed the render and the test
still passed while reporting found: 0 (now guarded by a positive control requiring exactly one
Archive and one Delete button per row), and the control-validation step compared a file:// URL
against a filesystem path. Repo suite counts are reported separately as gates and are not
folded into assertions.json, so every number there is one of my own scripted checks.

Raw logs: ab-base.json, ab-head.json, ab-base.log, ab-head.log, ab-driver-stdout.log,
ab-tally.json, mutation-M*.log, mutation-tally.json, mutation-stdout.log,
probe-run.log, coverage-stdout.log, coverage-tally.json, gate-web-shell-unit.log,
gate-e2e-session-overview.log, gate-e2e-smoke-head.log, gate-e2e-other-specs.log,
gate-e2e-m1-mutant.log, gate-visuals.log, plus the two component diffs.

Images: 01-ab-status-and-mutation-gate.png (the A/B cells as they printed),
02-mutation-matrix.png (the matrix as it printed),
03-overview-head-chromium-dark.png and 04-details-popover-head-chromium-light.png (real
Chromium renders of the head build, produced by the PR's own visual spec),
05-coverage-crosscheck.png (the coverage census and probe results).

Flakiness gate log

e2e suite, out of gate scope: packages/web-shell/client/e2e/visuals/screenshots.spec.ts
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.collapsed-groups-persist.spec.ts
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.session-overview.spec.ts
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.split-persist.spec.ts
rounds=5 files=4 skipped=4
file packages/cli/src/agent-view/supervisor-runner.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-runner.test.ts
file packages/web-shell/client/components/SessionOverviewPanel.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/SessionOverviewPanel.test.tsx
file packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/SessionDetailsTooltip.test.tsx
file packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/sessionDetailsCollisionBoundary.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/agent-view/supervisor-runner.test.ts: PPPPP
  packages/web-shell/client/components/SessionOverviewPanel.test.tsx: PPPPP
  packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: PPPPP
  packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: PPPPP

verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/agent-view/supervisor-runner.test.ts: P (exit 0)
round 1 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-runner.test.ts: P (exit 0)
round 2 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-runner.test.ts: P (exit 0)
round 3 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-runner.test.ts: P (exit 0)
round 4 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-runner.test.ts: P (exit 0)
round 5 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/sidebar/SessionDetailsTooltip.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/sidebar/sessionDetailsCollisionBoundary.test.ts: P (exit 0)

Evidence images

01-ab-status-and-mutation-gate

02-mutation-matrix

03-overview-head-chromium-dark

04-details-popover-head-chromium-light

05-coverage-crosscheck

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical-only review at head 8bee8cca.

Previously blocking findings — all verified fixed in the code at this head

Finding Verification at 8bee8cca
R6-1 (full workspace path falsified the untouched split-persist assertion; smoke red) SessionDetailsTooltip.tsx:274-276 renders and titles workspaceLabel ?? folderPath; web-shell.split-persist.spec.ts now asserts details.getByText(WORKSPACE_CWD, { exact: true }), so both specs are satisfied by the same contract. web-shell.collapsed-groups-persist.spec.ts still locates the cell by title, which the popover keeps. Status check rollup at this head: SUCCESS, no failing or pending run.
R5-1 (background work classified as Idle, enabling Archive/Delete on a working session) deriveSessionCards (SessionOverviewPanel.tsx:224-226) now reads (session.hasActivePrompt ?? status?.hasActivePrompt) || session.activeWorkState === 'active' behind needsApproval/askUserQuestion, and does not fold 'unknown' in. sessionDetailsProps:566-568 derives the popover flags from card.status, so the column, the filter counts, the popover ladder and the card.status === 'idle' gates on canArchiveCard/canDeleteCard now consult one source.
R5-2 (pinned popover unmounted with focus, dropping activeElement to <body>) Relocated into the tooltip's own layout cleanup as the finding required: SessionDetailsTooltip.tsx:132-149 runs on unmount, pierces shadow roots, and hands focus to [data-web-shell-session-panel], which exists with tabIndex={-1} at SessionOverviewPanel.tsx:1644-1648. The panel effect no longer has to win a race against the row's removal.
R5-3 (Details button destroyed an in-progress rename via Radix autofocus) The actions-column trigger is disabled={Boolean(editingIdentity)} (SessionOverviewPanel.tsx:1312), so rename and click-pinned details are mutually exclusive in both directions (startRename still clears detailsOpen at :782). The mousedown guard's pinned contract is untouched.
R4-1 (focus handoff resolved through ownerDocument.activeElement, retargeting to the shadow host) Both the handoff (:167-177) and the unmount cleanup (:139-146) now walk active.shadowRoot.activeElement before matching, and the rescue matches only popovers carrying this instance's data-web-shell-session-details-content owner token, which also closes the fix-induced R2-5.
R3-1 (first in-row control click swallowed while a draft is open) The capture guard exempts only input (SessionOverviewPanel.tsx:1650-1659), so a pressed button/link no longer blurs the draft and remounts before mouseup; onRowClick still bails out while editingCard is set or a selection is active (:1717-1721).
R1-8 / R2-1 (hoisted detailsOpen slot not reconciled) startRename clears the slot, and the reconcile effect is gated on clampPending so a shrinking catalog cannot clear state for a row that is still about to be visible (:1441-1455).

Critical-only scan of the current diff

Read the full diff plus the current contents of SessionOverviewPanel.tsx, SessionDetailsTooltip.tsx, sessionDetailsCollisionBoundary.ts, i18n.tsx and the touched Playwright specs. No blocking correctness, security, data-loss, regression or compatibility defect is provable at this head:

  • Row navigation is bounded: onRowClick ignores clicks while renaming or while a text selection is active, checkbox and control clicks stay selection/action-only, and archive/delete/split keep their existing per-card and cross-workspace gates.
  • Every locale key the new UI reads (sessionsOverview.filter.*, sessionsOverview.status*, sessionsOverview.details, sidebar.activeWork, sidebar.activityUnknown, sidebar.clients) exists in both the English and Chinese tables.
  • The follow-up commit is test-only (supervisor-runner.test.ts teardown, split-pane details assertion); production behavior is unchanged and the check rollup is green.

All 38 review threads on this PR are marked resolved, and each previously blocking one was re-verified against the code above rather than against the flag. The remaining reported items are Suggestion-level (truncation recovery, extra coverage, visual capture of the popover, overscroll containment) and are tracked in #11390; they do not block merge.

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下:

Autofix review round — no action needed

No actionable feedback was found for this round, so no changes were made.

What was checked:

  • Reviews and inline comments — no new reviews or inline review comments from trusted maintainers or the automated reviewer since the last evaluation. All previously raised review threads are already resolved.
  • Issue-level comments — the only new comment is the CI bot's "Sandboxed verification is running" status notification, which is informational and requires no code action.
  • Failed checks — none. There are no new failures and no still-red checks persisting from before the last evaluation.
  • Base-branch conflicts — none; the branch merges cleanly, so no merge was performed.

The PR remains as it stands, awaiting the in-flight sandboxed verification report and further human review.

中文说明

Autofix 审查轮次 —— 无需处理

本轮未发现需要处理的反馈,因此未做任何改动。

已检查的内容:

  • 审查与行内评论 —— 自上次评估以来,没有来自可信维护者或自动审查器的新审查或行内评论。此前提出的所有审查讨论串均已解决。
  • Issue 级评论 —— 唯一的新评论是 CI 机器人发布的“沙箱验证正在运行”状态通知,该通知仅为信息提示,无需代码改动。
  • 失败的检查 —— 无。没有新的失败,也没有从上次评估之前就持续失败的检查。
  • 与基线分支的冲突 —— 无;分支可以干净地合并,因此未执行合并操作。

该 PR 保持现状,等待正在进行中的沙箱验证报告以及进一步的人工审查。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.1

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as the second vote. qqqys's Critical-only re-verification at this head covers the seven previously-blocking findings (R1-8/R2-1 detailsOpen reconciliation, R3-1 draft-swallow guard, R4-1 shadow-host focus handoff, R5-1 idle classification of active work, R5-2 unmount focus drop, R5-3 rename/Details mutual exclusion, R6-1 workspace-path assertion), each re-checked in the code rather than by thread flag, and I concur.

My independent scan of the current diff found no blocking defect: row navigation is bounded (rename/selection/checkbox all short-circuit onRowClick), archive/delete keep their per-card and cross-workspace gates, and every locale key the new UI reads exists in both en and zh tables. The follow-up commit is test-only. All 38 threads are resolved; the remaining Suggestion-level items are tracked in #11390 and don't block.

CI is fully green (Lint, Test, web-shell E2E Smoke, Capture visuals, no-AK integration, OpenTUI no-flicker, TUI parity, review-pr).

@yiliang114
yiliang114 dismissed stale reviews from qwen-code-ci-bot, qwen-code-ci-bot, qwen-code-ci-bot, qwen-code-ci-bot, qwen-code-ci-bot, and qwen-code-ci-bot September 9, 2026 12:09

Stale — filed against a prior head. All blocking findings are fixed at 8bee8cc (qqqys Critical-only verification) and two approvals are on the current head.

@wenshao
wenshao added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit 50b942a Sep 9, 2026
90 checks passed

@ytahdn ytahdn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

主旨总结 / PR Summary

中文 — 本 PR 重构 Web Shell 的「会话总览」面板,让它更易扫读和导航:

  • 每个会话在标题下方直接展示工作区、分支和 PR,并把「待审批 / 需用户输入 / 运行中 / 空闲」拆成互不相同的状态(此前审批、提问、运行共用一个 spinner,无法区分谁被卡住);新增状态筛选按钮与「标题/分支/PR 号(含 #123)/会话 ID」搜索。
  • 标题悬停复用侧栏那套完整详情浮层(完整标题、工作区路径、会话 ID、状态、PR/issue 链接、可复制 ID),并新增一个显式「详情」按钮供键盘/触摸访问;窄屏时在标题旁保留紧凑状态图标。
  • 交互契约改为:点击行=打开会话,勾选框=选择,有选择时才出现批量操作(此前点击行是切换选择)。
  • 覆盖模块:SessionOverviewPanel.tsxsidebar/SessionDetailsTooltip.tsxsessionDetailsCollisionBoundary.tsi18n.tsx 及一处 CLI 测试基建修复(supervisor-runner.test.ts),并配套大量单测/E2E/视觉用例。

English — This PR reworks the Web Shell Session Overview panel for scannability and navigation:

  • Each session now shows workspace, branch and PR under its title, with distinct approval / question / running / idle states (previously approval, question and running shared one spinner, so blocked work was indistinguishable). Adds status filter buttons plus title/branch/PR-number(incl. #123)/session-ID search.
  • Title hover reuses the sidebar's full details popover (full title, workspace path, session ID, status, PR/issue links, copyable ID), plus an explicit details button for keyboard/touch; compact status cues stay beside titles on narrow layouts.
  • The interaction contract changes to: click a row = open the session, checkboxes = selection, batch actions appear only when a selection exists (previously a row click toggled selection).
  • Modules touched: SessionOverviewPanel.tsx, sidebar/SessionDetailsTooltip.tsx, sessionDetailsCollisionBoundary.ts, i18n.tsx, plus one CLI test-infra fix (supervisor-runner.test.ts) and extensive unit/E2E/visual coverage.

结论 / Verdict: APPROVE

中文 — 我按纯静态方式审查了 head(8bee8ccad)的生产代码,并对每处跨文件影响做了 head tree 复核,未发现 Critical 或 Important 问题。以下是我特意核实并排除的几处疑点,以及两条非阻塞 Nit。

已核实、确认非缺陷:

  • resolveSessionDetailsCollisionBoundary 签名语义变更:参数由 sidebar 改为 anchor,回退逻辑由 ?? sidebar 改为 ?? closest('[data-web-shell-root]') ?? closest('aside') ?? null。全仓唯一另一个调用方 WorkspaceDetailsTooltip.tsx(本 PR 未改)传入的正是 anchorRef.current?.closest('aside');对 <aside> 元素而言新旧回退结果完全一致(aside.closest('aside') 命中自身),故其行为未变。SessionDetailsTooltip.tsx 自身调用点也已同步更新。
  • 删除的 i18n keysessionsOverview.folder / worktree / sessionId):head tree 中已无任何引用。
  • 新增 i18n keystatus.idle/running/needsApproval/askUserQuestionfilter.*detailssessionCountworkspaceAllworkspacesSelectedstatusColumnstatusFilter):EN 与 ZH 均已齐备;模板字面量 key sessionsOverview.status.${status} 的四种取值全部存在(这类 key tsc 静态查不到,专门核对过)。
  • 删除的 worktreeOnly prop 与 SessionIdCell:head 无残留引用。
  • 选择/打开职责分离select 列与 actions 列均带 stopRowClick: true,勾选框/详情按钮不会误触发「打开会话」;onRowClick 在重命名中或存在文本选区时提前 return,避免拖选元数据时误开。
  • .attention { color: var(--warning-color) }--warning-color 是既有主题变量,与仓库多处 module.css 用法一致。
  • columns useMemo 依赖数组:与 memo 体内实际引用一致,无遗漏。
  • 焦点管理(受控 open + ownerToken 单例替换、useLayoutEffect 卸载兜底把焦点交还 [data-web-shell-session-panel](本 PR 已给该根加 tabIndex={-1})、restoreFocusRefonCloseAutoFocus 的配合、shadowRoot activeElement 递归下钻)逻辑自洽,且有针对性单测/E2E(含 shadow-dom 用例)覆盖。

Nit(非阻塞,可选):

  1. PR 号搜索用 `#${pr.number}`.includes(query),短数字查询会宽泛命中(如 1 命中 #123)。这与既有的标题/ID 子串搜索行为一致,可接受;若希望精确按 PR 号定位,可考虑数值相等或前缀匹配。
  2. detailsOpenRef.current = detailsOpen(渲染期写 ref 镜像 state)是社区常见的 React 反模式;此处因在同一次渲染体内、子单元格读取之前同步赋值而安全,但仍可考虑用 effect 同步或直接读 state 以更显式。

CI:发布时全绿 —— Test (ubuntu, Node 22)、Lint & Static、web-shell E2E Smoke、Integration Tests (no-AK)、Capture web-shell visuals、Desktop Shell 均 pass;其余为 skipping 的自动化 review/authorize 作业。


English — I reviewed the head (8bee8ccad) production code statically and cross-checked every cross-file impact against the head tree. No Critical or Important issues. Below are the specific suspicions I verified and cleared, plus two non-blocking nits.

Verified as non-defects:

  • resolveSessionDetailsCollisionBoundary signature/semantics change: the param went from sidebar to anchor, and the fallback from ?? sidebar to ?? closest('[data-web-shell-root]') ?? closest('aside') ?? null. The only other caller repo-wide, WorkspaceDetailsTooltip.tsx (untouched by this PR), passes anchorRef.current?.closest('aside'); for an <aside> element the old and new fallbacks resolve identically (aside.closest('aside') matches itself), so its behavior is unchanged. SessionDetailsTooltip.tsx's own call site was updated accordingly.
  • Removed i18n keys (sessionsOverview.folder / worktree / sessionId): no remaining references in the head tree.
  • New i18n keys (status.idle/running/needsApproval/askUserQuestion, filter.*, details, sessionCount, workspaceAll, workspacesSelected, statusColumn, statusFilter): present in both EN and ZH; all four values of the template-literal key sessionsOverview.status.${status} exist (tsc can't statically check these, so I verified by hand).
  • Removed worktreeOnly prop and SessionIdCell: no dangling references at head.
  • Selection vs. open separation: both the select and actions columns carry stopRowClick: true, so checkbox/details clicks don't trigger "open session"; onRowClick early-returns while renaming or when a text selection exists, avoiding accidental opens during metadata drag-select.
  • .attention { color: var(--warning-color) }: --warning-color is an existing theme variable, consistent with its use across many module.css files.
  • columns useMemo dependency array: matches the values actually referenced inside; nothing missing.
  • Focus management (controlled open + single-owner-token replacement, the useLayoutEffect unmount rescue handing focus back to [data-web-shell-session-panel] — which this PR gives tabIndex={-1} — the restoreFocusRef / onCloseAutoFocus interplay, and recursive shadowRoot activeElement descent) is internally consistent and covered by targeted unit/E2E tests (including a shadow-dom case).

Nits (non-blocking, optional):

  1. PR-number search uses `#${pr.number}`.includes(query), so short numeric queries match broadly (e.g. 1 hits #123). This is consistent with the existing title/ID substring behavior and acceptable; if precise PR lookup is desired, consider numeric-equality or prefix matching.
  2. detailsOpenRef.current = detailsOpen (writing a ref to mirror state during render) is a commonly-cited React anti-pattern; it's safe here because it's assigned synchronously in the same render body before child cells read it, but an effect-synced ref or reading state directly would be more explicit.

CI: green at publish time — Test (ubuntu, Node 22), Lint & Static, web-shell E2E Smoke, Integration Tests (no-AK), Capture web-shell visuals, and Desktop Shell all pass; the rest are skipping automation review/authorize jobs.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants