feat(web-shell): declutter the workspace sidebar and add loopback open actions - #10606
Conversation
|
E2E test report (mock-daemon Playwright harness,
Daemon-side: 42 launcher unit tests ( Not covered by automation: actual window spawning on Windows/Linux (spawn calls are mocked); manual verification done on macOS only. |
|
Thanks for the PR! Template looks good ✓ Problem: observed, not theoretical — unchanged from prior passes. The sidebar really does truncate workspace names today, the codebase itself marked opening a folder/terminal from the Web Shell as unsupported, and the before/after captures back the pain. Direction: aligned — unchanged. Diagnostics on demand in the sidebar, and loopback open actions following the existing Size: at this head the PR is ≈2,180 production lines vs ≈2,690 test lines (0 generated/schema), 39 files — up ~210 production lines since the last pass, all in the three fix commits reviewed below. As a Approach: unchanged and still the right structure — registry + trust gate + mutation gate + capability advertisement on the daemon, capability tags + loopback double-gate on the client. The new delta since the last pass is three fix commits: a spawned-but-stalled wt.exe no longer falls back (dedicated Risk: no elevated-risk paths matched the revert-history signal. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实存在，不是理论问题——与前几轮判断一致。侧栏确实会截断工作区名，代码里也明确标注过 Web Shell 无法打开文件夹/终端，前后对比截图支撑这个痛点。 方向：对齐——判断不变。诊断信息按需展示，loopback 打开操作沿用现有 规模：当前 head 上生产代码约 2,180 行，测试约 2,690 行（生成/schema 0 行），共 39 个文件——比上一轮多约 210 行生产代码，全部来自下面审查的三个修复提交。作为 方案：结构不变且仍然正确——daemon 侧 registry + trust gate + mutation gate + capability 宣告，客户端 capability 标签 + loopback 双重门控。上一轮之后的新增量是三个修复提交：已启动但卡住的 wt.exe 不再触发回退（引入专用 风险：未命中回滚历史高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run scope: everything since the last pass at Both round-2 blockers are fixed at this head — verified by reading the head tree, not the fix-commit message:
The bootstrap-envelope commit mirrors the five runtime-only capability tags ( The merges disturbed nothing: the effective diff against the merged main parent is exactly the same 39-file set, and the one conflict resolution follows main's explicit Testing evidenceUnattended CI run — no PR code is built or executed here; the evidence is the PR's own CI fetched via the API. On this head the main
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification was triggered alongside this re-run and is still in flight in this thread ( Real-scenario testing: N/A on the CI path — no local product run; the CI evidence above is what this comment carries. 中文说明代码审查（本轮增量：上一轮 第二轮的两个阻塞项均已在当前 head 核实修复——读的是 head 代码，不是修复提交说明：wt.exe 挂起不再双开（挂起守卫改抛专用 bootstrap 包络提交补齐五个 runtime-only 能力标签，消除启动窗口的少报。已在 head 核验： 合并未扰动任何东西：对合并进来的 main 父提交的有效 diff 仍是同一 39 文件集，唯一的冲突解决跟随 main 的显式 测试证据：无人值守 CI 运行，不构建、不执行任何 PR 代码；证据来自 API 拉取的 PR 自身 CI。当前 head 上主单测通道 沙箱验证已与本次重跑一同触发、仍在线程内运行（ 真实场景测试：CI 路径不适用——无本地产品运行；本评论携带的证据即上述 CI 信号。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — four rounds of review have converged on a clean head: every historical blocker is verified fixed at this exact commit, the three-commit delta adds no new findings, and the unit lane is finally green; the remaining items are non-blocking nits, named below. Stepping back: this run re-reviewed the delta since The non-blocking nits, for the record: the cancelled web-shell E2E Smoke lane (classified environmental in Stage 2 — same code green ~40 minutes earlier; a re-run clears it); the lockfile stripping npm's Approval posts pinned to the reviewed commit and supersedes the round-2 request-changes review, which was gating on findings now verified fixed. The sandboxed 中文说明置信度：4/5 —— 四轮审查已在干净的 head 上收敛：全部历史阻塞项均已在当前这个提交上核实修复，三个提交的增量没有新发现，主单测通道也终于转绿；剩余事项都是非阻塞的小项，列在下面。 退一步看：本轮重新审查了 非阻塞小项，记录在案：被取消的 web-shell E2E Smoke 通道（Stage 2 已判定为环境性——同一代码约 40 分钟前全绿，重跑即可清除）；lockfile 剥掉 npm 批准锚定在已审查的提交上发布，并取代第二轮的 request-changes 审查——当时作为门禁的发现均已核实修复。与本次重跑一同触发的沙箱 — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "standalone_sessions_v1" |
features[] |
— | "workspace_trust_hot_reload" |
features[] |
— | "dynamic_workspace_registration" |
features[] |
— | "scratch_workspace_registration" |
features[] |
— | "web_terminal" |
— Qwen Code · serve A/B
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Static review — feat(web-shell): declutter the workspace sidebar and add loopback open actions
Reviewed statically against head ad677a3 (the full feature at dbe608f plus the one-commit env-guard registration delta). All claims below were verified by reading the head tree; no tests were executed.
CI at posting: on head ad677a3, Test (ubuntu) is still running (its pre-test lint/schema steps are all green; only the test step is pending), and every other substantive lane is green — Serve A/B, Desktop Shell (both OSes), Integration (no-AK), Real daemon E2E, security scans, and the web-shell visual capture (whose lane carries continue-on-error, so the artifact's render-status.txt = success was checked directly). The earlier Test red on the previous head was the env-guard registration, fixed by the delta commit.
What holds up well
- The daemon route fails closed in the right order: workspace_mismatch (400) → runtime unavailable (503 + Retry-After) → untrusted (403), all before anything is launched; the opened path is always the registry-resolved
runtime.workspaceCwd, and the request body is whitelist-mapped tofolder/terminalviasafeBody— there is no client-supplied path surface. - All launchers are argv-array spawns with no shell interpolation; the xterm fallback passes the path positionally (
-e sh -c 'cd "$1" && exec …' sh <path>); the availability probes fail closed across SSH markers, uid 0, missing display variables, and the "xdg-open is a directory" trap. - Capability wiring mirrors
native_directory_pickerend to end: the probes are evaluated once at startup, pinned by deps overrides in tests, advertised through both bootstrap envelopes, and the client double-gates on the advertised tags plusisLocalDaemon()(which can only under-report, never over-report). - The name-width cap math is precise (
headerActionCountonly overestimates, and the three CSS tiers leave a constant 3px margin), and the deletion audit is clean: the removedWorkspaceOverviewleaves no orphaned references,formatOverviewValuewas relocated rather than duplicated, and the facet-poll semantics (poll only while expanded, retain snapshots when collapsed) are preserved. - The follow-up delta commit registering the env-guard allowance is exact:
local-path-open.tsreads the wholeprocess.envexactly three times (two probe default arguments plus the Linux terminal spawn), matchingaccesses: { whole: 3 }. This also explains the earlier Test (ubuntu) red on the previous head — it was the env-guard test failing, not a product bug.
Findings
🟡 Important — diagnostic info moved behind a pointer-only surface (keyboard/screen-reader regression)
Locations: WorkspaceDetailsTooltip.tsx (anchor wiring at :194-217 binds onPointerEnter/onPointerLeave only — no onFocus/onBlur), WorkspaceSection.tsx :726-749, WebShellSidebar.tsx :5776-5779.
Before this change the full path, branch, session counts and facet chips were permanent inline DOM text on every expanded workspace. They now live exclusively in a popover that opens after 300ms of pointer hover. The one keyboard-reachable surface that still shows facet counts is the "Manage workspace" section of WorkspaceMenu, but that section is gated by canManage = ws.primary && ws.trusted && … — so on any non-primary workspace, keyboard-only and assistive-tech users have no remaining access to the session breakdown (e.g. how many sessions need attention) or the facet counts, including the failed-MCP highlight. Concrete cost: information that was accessible before this PR is now pointer-exclusive for a real user population.
Suggested fix: mirror the pointer handlers with onFocus={openAfterDelay} and a delayed onBlur close on the tooltip anchor (reusing the existing timers), or surface the counts in the per-workspace menu regardless of canManage.
🟢 Minor — win32 folder open on a deleted directory reports opened: true
local-path-open.ts :128-130 with :223-225 (explorer exit codes are intentionally ignored) and routes/workspace-local-open.ts :48-52. If a registered workspace directory is renamed or deleted externally while its runtime stays active, explorer.exe opens Quick Access/This PC and the route still answers 200 opened: true. darwin/linux fail the same case with 501 (open/xdg-open reject missing paths), so the platforms disagree. Suggested fix: an fs.statSync existence pre-check on the win32 branch that throws LocalPathOpenUnavailableError (→ 501).
🟢 Minor — formatDateTime cross-midnight ambiguity
utils/formatDateTime.ts :11-15. The branch decision uses absolute elapsed milliseconds while the display is wall-clock: at 08:00 today, a session from yesterday 08:01 renders 08:01:00, visually indistinguishable from a session one minute old. Suggested fix: compare calendar days instead of elapsed time (or prefix a date for non-today timestamps).
🟢 Minor — orphaned i18n key sidebar.overview.label
Defined at i18n.tsx :1408 (EN) and :4613 (ZH) but its only consumer (the removed chip list) is gone in this PR; a full-tree grep finds no remaining reference. Delete both definitions.
💡 Suggestion — two tautological assertions in WorkspaceDetailsTooltip.test.tsx
The success case asserts querySelector('svg') != null (:191), but both FolderOpenIcon and CheckIcon are svgs, so the icon swap is not actually pinned; the failure case asserts disabled === false (:218), but the implementation never sets disabled. If either behavior regresses the tests stay green. Consider asserting the concrete icon or markup change instead.
Cross-check of the earlier triage thread's two suggestions (independently verified)
- Windows
cmd.exefallback quoting: confirmed real, with one addition — because Node's CreateProcess backslash-escaping and cmd.exe's quote-toggling disagree, paths containing merely spaces also mis-parse on the fallback, silently opening the terminal in the wrong directory while the route reports success; names with quotes/ampersands are worse. Exposure stays bounded (fallback only when wt.exe is absent; the path is a trusted, server-resolved workspace cwd), and the fix directions suggested in the thread (PowerShell Start-Process, or rejecting shell-active characters, or verbatim arguments) remain the right options. - Protocol doc table pipe: confirmed — the
workspace_local_openrow contains an unescaped|inside its description, so that row renders three columns and the separator row grew an extra segment; the capabilities-docs contract test extracts tags by line-start regex only, so it cannot catch this.
Verdict
💬 Comment. No correctness, security, or data-integrity blockers were found; the daemon side fails closed everywhere probed and the deletion audit is clean. The one 🟡 (accessibility of the moved diagnostics) is what keeps this from an approve; the three 🟢 are cheap follow-ups.
中文摘要
针对 head ad677a3（dbe608ff2 的完整功能 + 一个 env-guard 注册增量提交）完成纯静态审查，所有结论均通过在 head tree 读码核实，未运行任何测试。
发布时 CI：head ad677a3 上 Test (ubuntu) 仍在运行（其测试前的 lint/schema 步骤已全部为绿，仅剩测试本体一步），其余实质车道全部为绿——Serve A/B、Desktop Shell（双平台）、集成（no-AK）、真实 daemon E2E、安全扫描，以及 web-shell 视觉捕获（该车道带 continue-on-error，已直接核对 artifact 内的 render-status.txt = success）。上一个 head 的 Test 红灯是 env-guard 注册问题，已由增量提交修复。
做得好的地方
- 守护进程路由的失败关闭顺序正确：workspace_mismatch(400) → runtime 不可用(503+Retry-After) → 未信任(403)，全部先于任何启动动作；打开的路径恒为注册表解析的
runtime.workspaceCwd，请求体经safeBody白名单映射为folder/terminal，无客户端可控路径面。 - 所有启动器均为 argv 数组 spawn、无 shell 插值；xterm 回退以位置参数传路径；可用性探测在 SSH 标记、uid 0、缺失显示变量、"xdg-open 是目录"等场景全部失败关闭。
- 能力接线与
native_directory_picker端到端同构：探测仅启动期求值一次、测试以 deps 覆盖锁定、两个引导包络都宣告、客户端在能力标签之外叠加isLocalDaemon()双门槛（只会漏报不会误报）。 - 名字宽度封顶计算精确（
headerActionCount只会高估，CSS 三档恒留 3px），删除审计干净：被移除的WorkspaceOverview无孤儿引用，formatOverviewValue是迁移而非复制，facet 轮询语义（仅展开时轮询、折叠保留快照）保持不变。 - 增量提交注册的 env-guard 允许项精确：
local-path-open.ts恰好三处读取整个process.env（两处探测默认参数 + Linux 终端 spawn），与accesses: { whole: 3 }吻合。这也解释了上一个 head 的 Test (ubuntu) 红灯——是 env-guard 测试失败，不是产品 bug。
发现
🟡 Important —— 诊断信息移入纯指针触发的弹层（键盘/读屏可达性回退）：改动前完整路径、分支、会话计数、facet 芯片是常驻行内文本；现在只存在于 hover 300ms 打开的弹层，锚点只有 onPointerEnter/onPointerLeave，无 onFocus 路径。唯一能展示 facet 计数的键盘可达面是工作区菜单的"管理"区，但它仅主工作区可见（canManage 要求 ws.primary）。非主工作区的会话明细与 facet 计数（含 MCP 失败高亮）对纯键盘用户彻底不可达。建议：锚点补 onFocus/延迟 onBlur（复用现有定时器），或让菜单对所有工作区展示计数。
🟢 Minor —— win32 下目录已删除时打开文件夹仍返回 opened: true（local-path-open.ts :128-130/:223-225 + 路由 :48-52）：explorer 退出码被有意忽略，注册目录被外部删除/改名后资源管理器打开"此电脑"而路由报成功；darwin/linux 同场景返回 501，平台不一致。建议 win32 分支加存在性预检。
🟢 Minor —— formatDateTime 跨午夜歧义（formatDateTime.ts :11-15）：判定用绝对毫秒差、显示用墙钟——今天 08:00 时昨天 08:01 的会话显示 08:01:00，与一分钟前的会话无法区分。建议按日历日判定。
🟢 Minor —— 孤儿 i18n 键 sidebar.overview.label（i18n.tsx :1408/:4613）：唯一消费者已随本 PR 移除，全树无引用，建议两处定义一并删除。
💡 建议 —— WorkspaceDetailsTooltip.test.tsx 两处恒真断言：成功用例只断言存在任意 svg（两个图标都是 svg，切换未被锁定）；失败用例断言 disabled === false 但实现从不设置 disabled。建议改为断言具体图标或标记变化。
对早前分诊线程两条建议的独立复核
- Windows
cmd.exe回退引号问题：属实且补充一点——Node CreateProcess 的反斜杠转义与 cmd.exe 的引号翻转解析不一致，仅含空格的路径在回退路径上也会断词，终端静默开在错误目录而路由报成功；含引号/&的目录名更糟。暴露面仍然受限（仅无 wt.exe 时走回退、路径为受信的服务端解析工作区路径），线程中给出的修复方向（PowerShell Start-Process / 拒绝 shell 活性字符 / verbatim 参数）依然成立。 - 协议文档表格竖线：属实——该行描述内未转义的
|使其渲染为三列、分隔行多出一段；能力文档契约测试只按行首正则提取标签名，无法发现此损坏。
结论
💬 评论。未发现正确性、安全或数据完整性阻塞项；守护进程侧处处失败关闭，删除审计干净。唯一的 🟡（诊断信息可达性）是不给批准的原因，三个 🟢 均为低成本跟进项。
…ce-hover-details # Conflicts: # docs/developers/qwen-serve-protocol.md # integration-tests/cli/qwen-serve-routes.test.ts # packages/cli/src/serve/capabilities.ts # packages/cli/src/serve/run-qwen-serve.ts # packages/cli/src/serve/server.test.ts # packages/cli/src/serve/server.ts # packages/cli/src/serve/server/serve-features.ts
|
All findings addressed in 🟡 Accessibility of the moved diagnostics — the tooltip anchor now mirrors the pointer handlers with 🟢 win32 folder open on a deleted directory — both win32 branches now 🟢 🟢 Orphaned i18n key — 💡 Tautological test assertions — the success case now pins the concrete icon swap ( Windows Protocol doc table pipe — fixed during the main-merge conflict resolution: the row now says Verification: 324 web-shell sidebar/util unit tests, 46 daemon local-open/guard tests, the filtered serve capability suites, the 4-spec |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Re-review — fix commit for the accessibility and platform edge-case findings
Re-reviewed statically against head c98701a (the author fix commit on top of the origin/main merge). The merge brought ~2,900 lines of main-forwarding; the substantive author delta is the single fix commit (440 lines across 8 files), whose scope maps exactly onto my round-1 findings. Every resolution below was verified by reading the new head tree, not taken from the commit message. No tests were executed.
CI at posting: on head c98701a only the automated labeling/review lanes have reported so far; the primary test suite (Test ubuntu, Serve A/B, Desktop Shell, Integration, E2E) has not yet reported at the time of this review. The merge queue still requires those green before merge regardless.
Round-1 findings — resolution verified
🟡→✅ Accessibility (the only round-1 Important). The pointer-only popover is now keyboard-reachable. WorkspaceDetailsTooltip.tsx :207-243 wires onFocus/onBlur on the anchor (open after the same 300 ms delay as hover) and onFocus={cancelClose}/onBlur on the portaled content, with a containsFocusTarget helper (:170-173) that spans both the anchor and the portal tree via anchorRef/contentRef so focus moving between the header button and the popover does not close it. onOpenAutoFocus is still preventDefault, so focus is not stolen off the header. A new test (WorkspaceDetailsTooltip.test.tsx :296-322) pins keyboard-focus open via a bubbling focusin + 300 ms, asserting the [role="dialog"] renders the cwd; the file already restores real timers in afterEach. Resolved.
🟢→✅ win32 deleted-directory honesty. local-path-open.ts :133 and :173 call a new assertPathExists (:153-159, statSync → throw LocalPathOpenUnavailableError) before launching explorer.exe / wt.exe. The route (routes/workspace-local-open.ts :53-61) maps that error to 501 and only answers opened: true when no throw, so a deleted/renamed workspace dir now fails on win32 exactly like darwin/linux instead of silently opening Quick Access. New test pins rejection-without-spawn. Resolved.
🟢→✅ formatDateTime cross-midnight. formatDateTime.ts :10 now branches on calendar-day equality (date.toDateString() === new Date(now).toDateString()) instead of elapsed milliseconds, so yesterday 08:01 can no longer masquerade as today 08:01. Tests updated to pin the across-midnight case. Resolved.
🟢→✅ orphaned i18n key. sidebar.overview.label removed from both EN and ZH; a tree-wide grep finds no remaining reference. Resolved.
💡→✅ tautological assertions. The icon-swap tests now assert the concrete svg.lucide-check / svg.lucide-folder-open selectors (WorkspaceDetailsTooltip.test.tsx :190-191, :217-219), matching the lucide-* class convention already used elsewhere (e.g. ArtifactPanel.test.tsx), so an icon regression can no longer stay green. Resolved.
Cross-check item from round 1 — also addressed
Windows cmd.exe start fallback quoting. The fallback in spawnWindowsTerminal (:194-215) no longer threads the path through a re-parsed cmd.exe start command line. It now spawns powershell.exe -NoProfile -STA -Command 'Start-Process cmd.exe -ArgumentList "/k cd /d $env:QWEN_LOCAL_OPEN_DIR""' and hands the directory via a new QWEN_LOCAL_OPEN_DIR env var (spawnAndIgnoreExitCode :246-255 spreads process.env only when extraEnv is passed). This is one of the fix directions from the earlier triage thread, and space-containing paths no longer mis-parse. The added whole-process.env read is reflected in the env-guard allowance bump whole: 3 → 4 (process-env-guard.test.ts :225-235), which matches the actual four accesses in local-path-open.ts (:75, :88, :218, :254).
Separately, the protocol-doc capability table row for workspace_local_open that round 1 flagged for an unescaped pipe is now clean at this head (each row is a well-formed 2-column row), so no doc follow-up remains.
Merge-drift check (not re-reviewed as author content)
The origin/main merge touched several files this PR also changes (run-qwen-serve.ts, server.ts, serve-features.ts, capabilities.ts, DaemonClient.ts). I did not re-review main's own evolution, but I confirmed the merge preserved this PR's wiring rather than clobbering it: registerWorkspaceLocalOpenRoutes is still registered (server.ts :2319), the startup capability probes are intact (run-qwen-serve.ts :2572-2573, server.ts :1077-1079), the workspace_local_open / workspace_local_terminal capability tags are present, and DaemonClient.openLocally/openTerminalLocally still target POST /workspaces/:workspace/open.
Verdict
✅ Approve. All round-1 findings — the one accessibility Important and the three Minors plus the tautological-assertion suggestion — are resolved and verified at this head, the cmd.exe fallback cross-check is fixed via the environment-variable route, and the merge did not disturb the feature wiring. The only remaining note is that the primary CI suite hasn't reported yet on this head; the merge queue will gate on that.
中文摘要
针对 head c98701a（基于 origin/main 合并之上的作者修复提交）完成复审。合并带入约 2900 行 main 前移内容，实质作者增量是单个修复提交（440 行、8 个文件），其范围与我上一轮的 findings 一一对应。以下每条结论均通过在新 head tree 读码核实，未采信提交信息，也未运行任何测试。
发布时 CI：head c98701a 上目前只有自动打标签/审查车道有结果；主测试套件（Test ubuntu、Serve A/B、Desktop Shell、集成、E2E）在本复审发布时尚未出结果。无论如何，合并队列在合并前仍要求这些为绿。
上一轮 findings 复核
🟡→✅ 可达性（上一轮唯一 Important）：纯指针触发的弹层现在键盘可达。WorkspaceDetailsTooltip.tsx :207-243 为锚点接上 onFocus/onBlur（与 hover 相同的 300ms 延迟打开），并为门户化内容接 onFocus={cancelClose}/onBlur；containsFocusTarget（:170-173）借助 anchorRef/contentRef 横跨锚点与门户两棵树，焦点在表头按钮与弹层间移动不会误关。onOpenAutoFocus 仍 preventDefault，不会把焦点从表头夺走。新增测试用冒泡 focusin + 300ms 锁定键盘打开并断言 [role="dialog"] 渲染出 cwd；该文件已在 afterEach 恢复真实计时器。已解决。
🟢→✅ win32 删除目录误报成功：local-path-open.ts :133/:173 在启动 explorer.exe/wt.exe 前调用新增 assertPathExists（:153-159，statSync→抛 LocalPathOpenUnavailableError）；路由（:53-61）将该错误映射为 501、仅在不抛错时返回 opened: true，因此目录被删/改名时 win32 与 darwin/linux 一致失败，而不再静默打开"快速访问"。新测试锁定"不 spawn 即拒绝"。已解决。
🟢→✅ formatDateTime 跨午夜歧义：formatDateTime.ts :10 改为按日历日相等判定而非流逝毫秒，昨天 08:01 不会再伪装成今天 08:01；测试已更新锁定跨午夜场景。已解决。
🟢→✅ 孤儿 i18n 键：sidebar.overview.label 已从 EN/ZH 两处移除，全树无残留引用。已解决。
💡→✅ 恒真断言：图标切换测试改断言具体 svg.lucide-check/svg.lucide-folder-open（:190-191/:217-219），与代码库既有 lucide-* 约定一致（如 ArtifactPanel.test.tsx），图标回归不再能保持绿灯。已解决。
上一轮交叉核验项——也已处理
Windows cmd.exe start 回退引号：spawnWindowsTerminal（:194-215）不再把路径塞进被重新解析的 cmd.exe start 命令行，改为 powershell.exe ... Start-Process cmd.exe 并经新增环境变量 QWEN_LOCAL_OPEN_DIR 传目录（spawnAndIgnoreExitCode :246-255 仅在传入 extraEnv 时展开 process.env）。这是早前分诊线程给出的修复方向之一，含空格路径不再断词。新增的一次整 process.env 读取已在 env-guard 允许项 whole: 3→4（:225-235）中体现，且与 local-path-open.ts 实际四处访问（:75/:88/:218/:254）吻合。
另外，上一轮指出的协议文档能力表 workspace_local_open 行未转义竖线，在当前 head 已是规范的 2 列行，无文档遗留项。
合并漂移核查（不作为作者内容复审）
origin/main 合并触及了本 PR 同样修改的多个文件（run-qwen-serve.ts、server.ts、serve-features.ts、capabilities.ts、DaemonClient.ts）。我未复审 main 自身的演进，但确认合并保留了本 PR 接线而非覆盖：registerWorkspaceLocalOpenRoutes 仍被注册（server.ts :2319），启动期能力探测完好（run-qwen-serve.ts :2572-2573、server.ts :1077-1079），workspace_local_open/workspace_local_terminal 能力标签在场，DaemonClient.openLocally/openTerminalLocally 仍指向 POST /workspaces/:workspace/open。
结论
✅ 批准。上一轮全部 findings——唯一的可达性 Important、三个 Minor 及恒真断言建议——均在本 head 解决并经核实；cmd.exe 回退交叉项以环境变量方式修复；合并未扰动功能接线。唯一遗留提示是该 head 的主测试套件尚未出结果，合并队列会据此把关。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — nothing was cut short; the only discarded candidates were duplicates of R1-1/R1-8 already on the list.; chunk 5: running the new vitest cases (worktree has no node_modules or built dist; install+build exceeds the tool budget) — static verification of the wiring was complet….
Test Plan (not a blocker): routes/workspace-local-open.test.ts — no such file or directory.
中文说明
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — nothing was cut short; the only discarded candidates were duplicates of R1-1/R1-8 already on the list.；chunk 5：running the new vitest cases (worktree has no node_modules or built dist; install+build exceeds the tool budget) — static verification of the wiring was complet…。
Test Plan（非阻断）：routes/workspace-local-open.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (process.platform === 'linux') { | ||
| await spawnLinuxTerminal(path); | ||
| return; | ||
| } |
There was a problem hiding this comment.
[Critical] R1-17: [certifies-falsely] [new-surface] The Linux branch of openTerminalLocally skips the assertPathExists check that the win32 branches apply (added by this PR's own deleted-directory fix round) and that darwin gets via the launcher's non-zero exit. If a registered workspace directory is deleted (or made unreadable) while the daemon runs, Open-terminal still answers 200 opened: true: spawnLongLived resolves the moment the OS accepts the emulator's fork/exec, before the emulator ever looks at the directory — with the xterm fallback sh -c 'cd "$1" && exec …' sh <path> the cd fails, && short-circuits, and the window exits in a flash; gnome-terminal/konsole open in a fallback directory. The client shows the success check for an open that never happened, while win32 answers 501 for the same state.
Witness:
[probe] intact: openTerminalLocally(missingDir) → resolved (spawn called)
with assertPathExists added to the Linux branch: rejected with LocalPathOpenUnavailableError, no spawn
Add assertPathExists(path); at the top of the Linux branch (mirroring win32), so a missing target throws LocalPathOpenUnavailableError and the route answers 501. A Linux mirror of the existing "rejects a deleted directory on Windows without spawning" test — setPlatform('linux'), stub PATH, expect rejection with LocalPathOpenUnavailableError and no spawn — must go red when the added guard is removed.
中文说明
openTerminalLocally 的 Linux 分支跳过了 assertPathExists 检查——win32 分支有该检查（本 PR 自己的修复轮次为"目录已删除"场景添加），darwin 则通过启动器非零退出码兜底。若已注册的工作区目录在 daemon 运行期间被删除（或变为不可读），"打开终端"仍会返回 200 opened: true：spawnLongLived 在操作系统接受模拟器 fork/exec 的瞬间即 resolve，此时模拟器尚未查看目录——xterm 回退执行 sh -c 'cd "$1" && exec …' sh <path> 时 cd 失败、&& 短路，窗口一闪即退；gnome-terminal/konsole 则打开在回退目录。客户端为一次从未发生的打开显示成功勾选，而 win32 在相同状态下返回 501。
修复：在 Linux 分支开头加 assertPathExists(path);（与 win32 保持一致），使缺失目标抛出 LocalPathOpenUnavailableError、路由返回 501。请补一个与现有 "rejects a deleted directory on Windows without spawning" 对称的 Linux 测试——setPlatform('linux')、stub PATH、断言抛出 LocalPathOpenUnavailableError 且未 spawn——并确认移除新增守卫后该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| async function spawnLinuxTerminal(path: string): Promise<void> { | ||
| const terminal = findExecutableOnPath(process.env, LINUX_TERMINAL_NAMES); |
There was a problem hiding this comment.
[Critical] R1-27: [certifies-falsely] [new-surface] The Linux terminal-open path applies no DISPLAY/WAYLAND_DISPLAY precondition at spawn time — the very predicate isLocalTerminalAvailable() checks — and the capability is probed once at startup, so the tag stays advertised after the display goes away. On a host whose GUI session ended (an SSH-forwarded display that died, or a direct integrator POST on a headless host with a terminal binary on PATH — the protocol doc documents this route for integrators), spawnLinuxTerminal finds the binary, spawnLongLived resolves on exec acceptance, and the route answers 200 opened: true while xterm/gnome-terminal exits instantly with "cannot open display" and the daemon never observes it. Probed against the production route: isLocalTerminalAvailable=false at that same instant, yet POST /workspaces/:workspace/open returned {"opened":true,"target":"terminal"}.
Witness:
[probe] intact: isLocalTerminalAvailable=false; openTerminalLocally → RESOLVED;
route POST → status=200 body={"kind":"workspace-local-open","opened":true,"target":"terminal"}
with display gate: REJECTED(LocalPathOpenUnavailableError); route → 501 local_path_open_unavailable
Reject the same state the availability probe rejects, at the top of spawnLinuxTerminal: if (!process.env['DISPLAY'] && !process.env['WAYLAND_DISPLAY']) throw new LocalPathOpenUnavailableError('No display server available'); — the route already maps that error to 501. Note the existing Linux success tests stub only PATH, never DISPLAY (local-path-open.test.ts:320+), so this gate turns them red unless they also stub DISPLAY/WAYLAND_DISPLAY. Add a case that stubs PATH with a terminal dir and unsets both display variables, expecting LocalPathOpenUnavailableError and no spawn; removing the gate must turn it red.
中文说明
Linux 的终端打开路径在 spawn 时没有 DISPLAY/WAYLAND_DISPLAY 前置检查——而这正是 isLocalTerminalAvailable() 所用的判定条件——且能力只在启动时探测一次，显示器消失后标签仍会持续宣告。在 GUI 会话已结束的主机上（SSH 转发的显示断开，或无头主机上装有终端二进制时收到集成方的直接 POST——协议文档明确该路由面向集成方），spawnLinuxTerminal 能找到二进制，spawnLongLived 在 exec 被接受时即 resolve，路由返回 200 opened: true，而 xterm/gnome-terminal 立刻以 "cannot open display" 退出，daemon 完全观察不到。已对生产路由实测：同一时刻 isLocalTerminalAvailable=false，但 POST /workspaces/:workspace/open 返回了 {"opened":true,"target":"terminal"}。
修复：在 spawnLinuxTerminal 开头拒绝与可用性探测相同的状态：if (!process.env['DISPLAY'] && !process.env['WAYLAND_DISPLAY']) throw new LocalPathOpenUnavailableError('No display server available');——路由已将该错误映射为 501。注意现有 Linux 成功用例只 stub 了 PATH、从未 stub DISPLAY（local-path-open.test.ts:320+），该守卫会使它们变红，需同步补 stub DISPLAY/WAYLAND_DISPLAY。另加一个用例：stub PATH 指向终端目录、清空两个显示变量，断言抛出 LocalPathOpenUnavailableError 且未 spawn；移除守卫后该用例必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect( | ||
| isLocalTerminalAvailable({ | ||
| WAYLAND_DISPLAY: 'wayland-0', | ||
| PATH: xtermDir, | ||
| }), | ||
| ).toBe(true); |
There was a problem hiding this comment.
[Critical] R1-30: [certifies-falsely] [new-surface] The Linux availability probe accepts WAYLAND_DISPLAY as sufficient evidence for xterm, but xterm is X11-only (Xt/Xaw, no Wayland backend). On a pure-Wayland compositor with XWayland disabled (sway xwayland disable, river, cage) where xterm is the only installed emulator of the supported set, isLocalTerminalAvailable() returns true, the capability is advertised, and Open-terminal answers opened: true while xterm exits immediately with "cannot open display" — the exact failure the probe's own comment says it exists to filter out, and this new assertion pins the false positive. Note that re-checking the env at spawn time (the R1-27 direction) does not close this trigger: WAYLAND_DISPLAY is set, so such a check passes while xterm still cannot run.
Witness:
[probe] intact: availability(WAYLAND-only, xterm-only)=true; openTerminalLocally resolved=true;
terminal exited immediately=true
with xterm filtered under Wayland-only: availability=false; availability(DISPLAY+xterm-only)=true (control unchanged)
Drop the X11-only emulator when only a Wayland display is present: const names = env['DISPLAY'] ? LINUX_TERMINAL_NAMES : LINUX_TERMINAL_NAMES.filter((n) => n !== 'xterm'); in isLocalTerminalAvailable, and mirror the same filter at the spawn site — spawnLinuxTerminal picks from the same list against live env (local-path-open.ts:218), so probe and spawner must not diverge or a direct POST re-creates the false success. Flip this assertion to .toBe(false) (removing the filter must turn it red) and stub DISPLAY in the existing xterm spawn test, which currently stubs only PATH.
中文说明
Linux 可用性探测把 WAYLAND_DISPLAY 当作 xterm 的充分证据，但 xterm 是纯 X11 程序（Xt/Xaw，无 Wayland 后端）。在禁用 XWayland 的纯 Wayland 合成器上（sway xwayland disable、river、cage），若受支持集合中只安装了 xterm，isLocalTerminalAvailable() 返回 true、能力被宣告，"打开终端"返回 opened: true，而 xterm 立即以 "cannot open display" 退出——这正是探测自身注释声称要过滤掉的失败，且这条新断言把该误报固化了下来。注意：spawn 时重查环境变量（R1-27 的方向）无法覆盖此触发条件——WAYLAND_DISPLAY 存在，该检查会通过，但 xterm 依然无法运行。
修复：仅有 Wayland 显示器时剔除 X11-only 模拟器：在 isLocalTerminalAvailable 中用 const names = env['DISPLAY'] ? LINUX_TERMINAL_NAMES : LINUX_TERMINAL_NAMES.filter((n) => n !== 'xterm');，并在 spawn 端镜像同样的过滤——spawnLinuxTerminal 用同一份列表对实时环境取值（local-path-open.ts:218），探测与启动器不能分歧，否则直接 POST 会重新造成假成功。把此断言翻转为 .toBe(false)（移除过滤后必须变红），并给现有 xterm spawn 测试补 stub DISPLAY（目前只 stub 了 PATH）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| '-NoProfile', | ||
| '-STA', | ||
| '-Command', | ||
| 'Start-Process cmd.exe -ArgumentList "/k cd /d `"$env:QWEN_LOCAL_OPEN_DIR`""', |
There was a problem hiding this comment.
[Suggestion] R1-1: The win32 cmd.exe fallback embeds the PowerShell-expanded directory in cmd's own command line, where cmd performs its immediate %VAR% expansion: a workspace directory containing a defined variable name between percent signs (e.g. C:\dev\old%TEMP%files — % is NTFS-legal) expands to garbage, cd /d targets a nonsense path, and the window stays in its default directory while the route still answers opened: true. This is the residual hole after the quoting fix moved the path into an environment variable — PowerShell expands $env:QWEN_LOCAL_OPEN_DIR into the ArgumentList before cmd starts, so the value re-enters a parsed command line.
Witness:
[probe] mocked win32 spawn: cmd.exe command line = /k cd /d "/tmp/probe/old%TEMP%files"
— %TEMP% reaches cmd's command line verbatim (cmd.exe %VAR% expansion is documented Windows behavior)
Hand the directory to cmd as the process working directory instead of embedding it in the command line: Start-Process cmd.exe -WorkingDirectory "$env:QWEN_LOCAL_OPEN_DIR". Keep the existing assertPathExists pre-check (local-path-open.ts:172-173) — Start-Process -WorkingDirectory throws on a missing directory, so without the pre-check a deleted directory would surface a raw PowerShell error instead of LocalPathOpenUnavailableError (501). Extend the fallback test to assert -WorkingDirectory (and no cd /d embedding); reverting to the cd /d form must turn it red.
中文说明
win32 的 cmd.exe 回退把 PowerShell 展开后的目录嵌入了 cmd 自己的命令行，而 cmd 会对其执行即时的 %VAR% 展开：目录名含百分号包裹的已定义变量名时（如 C:\dev\old%TEMP%files——% 在 NTFS 中合法），会被展开成无意义的路径，cd /d 指向错误位置，窗口停在默认目录，而路由仍返回 opened: true。这是把路径移入环境变量之后的残留漏洞——PowerShell 在 cmd 启动前就把 $env:QWEN_LOCAL_OPEN_DIR 展开进了 ArgumentList，该值重新进入了被解析的命令行。
修复：改为把目录作为进程工作目录交给 cmd：Start-Process cmd.exe -WorkingDirectory "$env:QWEN_LOCAL_OPEN_DIR"。保留现有 assertPathExists 前置检查（local-path-open.ts:172-173）——Start-Process -WorkingDirectory 对缺失目录会抛错，若没有该检查，已删除的目录会暴露原始 PowerShell 错误而非 LocalPathOpenUnavailableError（501）。请扩展回退测试断言 -WorkingDirectory（且不再嵌入 cd /d）；回退到 cd /d 形式必须使测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| ).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('shows the active/total split only when they differ', () => { |
There was a problem hiding this comment.
[Suggestion] R1-2: The hooks facet's display path lost its only tests in the chip→popover swap. The migrated formatOverviewValue tests cover mcp/skills/extensions/channels/context but not the hooks case, overviewDetail has no direct test anywhere, and WorkspaceDetailsTooltip.test.tsx never renders a hooks row — while hooks stays in WORKSPACE_OVERVIEW_ITEMS (workspaceOverviewModel.ts:31) and is fetched by useWorkspaceOverview, so both arms render in production for embedders opting in.
Witness:
[probe] hooks arms mutated → 112 passed, 0 failed (suite blind)
control: same mutation on the mcp arm → 3 tests failed in the same files
Restore the dropped pins next to the migrated cases: hooks cases for formatOverviewValue (initialized count, uninitialized → undefined) and overviewDetail tests for the hooks disabled/enabled branches plus at least one non-MCP facet. Removing the case 'hooks' arm or swapping hooksDisabled/hooksDetail must turn the new tests red.
中文说明
芯片→悬浮弹层的迁移让 hooks facet 的展示路径失去了唯一测试。迁移后的 formatOverviewValue 测试覆盖 mcp/skills/extensions/channels/context，唯独没有 hooks；overviewDetail 在任何地方都没有直接测试；WorkspaceDetailsTooltip.test.tsx 也从不渲染 hooks 行——而 hooks 仍在 WORKSPACE_OVERVIEW_ITEMS（workspaceOverviewModel.ts:31）中、useWorkspaceOverview 会拉取它，对选择启用该项的嵌入方来说这两条分支在生产中真实渲染。
修复：在迁移后的用例旁补回缺失的钉子：formatOverviewValue 的 hooks 用例（已初始化的计数、未初始化 → undefined），以及 overviewDetail 的 hooks 禁用/启用分支和至少一个非 MCP facet。移除 case 'hooks' 分支或互换 hooksDisabled/hooksDetail 必须使新测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| it('spawns wt.exe at the path on Windows and ignores its exit code', async () => { | ||
| setPlatform('win32'); | ||
| spawnMock.mockReturnValue(fakeChild('close')); |
There was a problem hiding this comment.
[Suggestion] R1-25: The "ignores its exit code" contract — the stated reason spawnAndIgnoreExitCode exists ("explorer.exe / wt.exe commonly exit 1 even when they did open the folder") — is pinned by no test: fakeChild('close') invokes the close callback with undefined and can never deliver an exit code. The explorer.exe twin test (~line 111) has the identical hole. A maintainer "tightening" the helper to reject on a truthy close code keeps every test green (undefined is falsy), but on a real Windows host where wt.exe exits 1 after opening the window, openTerminalLocally rejects, the route wraps it in LocalPathOpenUnavailableError, and the client gets a failure toast for an open that succeeded.
Witness:
[probe] mutant rejecting on truthy close code → 43/43 green
probe delivering close code 1: mutant REJECTED('explorer.exe exited with code 1') /
REJECTED('powershell.exe exited with code 1'); intact RESOLVED both
Make the fake deliver a nonzero close code (emit close with code 1) and assert openTerminalLocally/openPathLocally still resolve; the updated tests must go red if the close handler is mutated to reject on a truthy exit code.
中文说明
"忽略退出码" 这一契约——spawnAndIgnoreExitCode 存在的明示理由（"explorer.exe / wt.exe 即使成功打开了文件夹也常以 1 退出"）——没有任何测试钉住：fakeChild('close') 以 undefined 调用 close 回调，永远无法送出退出码。explorer.exe 的姊妹测试（约 111 行）有同样的漏洞。若有人 "收紧" 该辅助函数、在退出码为真值时 reject，所有测试照绿（undefined 为假值），但在真实 Windows 主机上，wt.exe 打开窗口后以 1 退出时 openTerminalLocally 会 reject，路由将其包成 LocalPathOpenUnavailableError，客户端为一次成功的打开弹出失败提示。
修复：让 fake 送出非零退出码（以码 1 触发 close），断言 openTerminalLocally/openPathLocally 仍然 resolve；把 close 处理器突变为真值退出码即 reject 时，更新后的测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| '-e', | ||
| 'sh', | ||
| '-c', | ||
| 'cd "$1" && exec "${SHELL:-/bin/sh}"', |
There was a problem hiding this comment.
[Suggestion] R1-26: spawnLongLived's child.unref() (local-path-open.ts:293) — the only line keeping a long-lived terminal window from holding the daemon's event loop, per its own comment — is pinned by no test. The fixture even exposes unref: vi.fn() (line 76) but nothing inspects it. Dropping the unref() (e.g. unifying spawnLongLived with spawnAndIgnoreExitCode, which does not unref) keeps the whole suite green; afterwards every opened terminal window remains a referenced child handle, and once the HTTP server closes on graceful shutdown the daemon process cannot exit until every user-opened terminal window is closed — qwen serve hangs indefinitely. The repo already pins this exact property elsewhere (external-tool-guard-provider.test.ts:399).
Witness:
[probe] unref removed → 43/43 green
expect(child.unref).toHaveBeenCalled(): FAILED on the mutant, PASSED intact
In the gnome-terminal/konsole/xterm tests capture the spawned child (spawnMock.mock.results[0].value) and assert expect(child.unref).toHaveBeenCalled(); removing child.unref() from spawnLongLived must turn it red.
中文说明
spawnLongLived 的 child.unref()（local-path-open.ts:293）——按其自身注释，是让长驻终端窗口不挂住 daemon 事件循环的唯一一行——没有任何测试钉住。fixture 甚至暴露了 unref: vi.fn()（76 行）却无人检查。删除 unref()（例如把 spawnLongLived 与不 unref 的 spawnAndIgnoreExitCode 统一）时整个套件全绿；此后每个打开的终端窗口都是被引用的子进程句柄，优雅关停时 HTTP 服务器关闭后，daemon 进程要等所有用户打开的终端窗口都关闭才能退出——qwen serve 无限挂起。仓库其他地方已钉过同样的属性（external-tool-guard-provider.test.ts:399）。
修复：在 gnome-terminal/konsole/xterm 测试中捕获 spawn 出的子进程（spawnMock.mock.results[0].value），断言 expect(child.unref).toHaveBeenCalled()；移除 spawnLongLived 中的 child.unref() 必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await act(async () => { | ||
| trigger.dispatchEvent(new Event('pointerover', { bubbles: true })); | ||
| vi.advanceTimersByTime(300); |
There was a problem hiding this comment.
[Suggestion] R1-28: The hover-intent cancel — closeAfterDelay clearing the pending 300 ms open timer when the pointer leaves the anchor before the popover opens (window.clearTimeout(openTimerRef.current) at WorkspaceDetailsTooltip.tsx:164) — is pinned by no test at any level: this file dispatches only open-side events, the only pointerout dispatches in the client tree target a different component, and every leave in the e2e happens after the dialog is visible. Dropping the clearTimeout line ships green (measured: 192/192 across the four suites); the verifier-corrected harm is a bounded ≤100 ms popover flash for hovers of 200–300 ms (the scheduled close() then clears the open timer and closes), not a stuck popover.
Witness:
[probe] mutant (clearTimeout dropped): 192/192 green; leave@250ms → t=300:OPEN, auto-closed ~t=350
intact: leave@100ms/leave@250ms → never opened
Add a unit test: dispatch pointerover, advance 100 ms (inside the 300 ms window), dispatch pointerout, advance ≥500 ms, and assert [role="dialog"] is null; deleting window.clearTimeout(openTimerRef.current) from closeAfterDelay must turn it red.
中文说明
悬浮意图取消——指针在弹层打开前离开锚点时，closeAfterDelay 清除待执行的 300ms 打开定时器（WorkspaceDetailsTooltip.tsx:164 的 window.clearTimeout(openTimerRef.current)）——在任何层级都没有测试钉住：本文件只派发打开侧事件，客户端代码树中仅有的 pointerout 派发指向另一个组件，e2e 中所有离开都发生在 dialog 可见之后。删除该 clearTimeout 行可以全绿上线（实测四个套件 192/192）；经验证者修正后的影响是 200–300ms 悬浮时一个不超过 100ms 的弹层闪现（随后预定的 close() 会清掉打开定时器并关闭），而不是弹层卡住。
修复：补一个单测：派发 pointerover，推进 100ms（在 300ms 窗口内），派发 pointerout，推进 ≥500ms，断言 [role="dialog"] 为 null；删除 closeAfterDelay 中的 window.clearTimeout(openTimerRef.current) 必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| onOpenPathLocally, | ||
| onOpenTerminalLocally, |
There was a problem hiding this comment.
[Suggestion] R1-29: The section-level && gitPollCwd && workspace.trusted gate on the two new local-open props is verified by no test: this file's tests never wire onOpenPathLocally/onOpenTerminalLocally (the renderSection overrides don't accept them), and the e2e never renders an untrusted workspace (13/13 fixtures are trusted: true) — so dropping the conjuncts ships green (measured: 18/18 test files). The hover popover would then offer Open folder/Open terminal on untrusted workspaces and on synthetic workspaces with a relative cwd; clicking fires POST /open, which resolveTrustedRuntime rejects server-side — the user is handed an affordance that can never succeed (a failure toast on click). The server-side trust check is what keeps this at Suggestion rather than a host-side exposure.
Witness:
[probe] gate-dropped mutant: full sidebar directory 18/18 files green
untrusted workspace probe: folderButton=PRESENT terminalButton=PRESENT; intact: both absent
Wire the two handlers through renderSection and assert: untrusted workspace → both buttons absent in the portaled dialog; trusted with a relative cwd → absent; trusted + absolute cwd → present. Removing && gitPollCwd && workspace.trusted from the wiring (lines ~734-742) must turn the new tests red. The server route enforces trust (routes/workspace-local-open.ts:38), so these tests assert button absence as UI policy mirroring the server rejection.
中文说明
两个新的本地打开属性上的节级 && gitPollCwd && workspace.trusted 门没有任何测试验证：本文件的测试从不接线 onOpenPathLocally/onOpenTerminalLocally（renderSection 的覆盖参数不接受它们），e2e 也从不渲染不可信工作区（13/13 的 fixture 都是 trusted: true）——因此删除这两个条件可以全绿上线（实测 18/18 个测试文件）。届时悬浮弹层会在不可信工作区和相对 cwd 的合成工作区上提供打开文件夹/终端；点击会触发 POST /open，被 resolveTrustedRuntime 在服务端拒绝——用户拿到一个永远不会成功的入口（点击即失败提示）。正是服务端信任检查把此问题留在建议级而非主机侧暴露。
修复：把两个处理器接入 renderSection 并断言：不可信工作区 → portal dialog 中两个按钮都不存在；可信但相对 cwd → 不存在；可信 + 绝对 cwd → 存在。移除接线处（约 734-742 行）的 && gitPollCwd && workspace.trusted 必须使新测试变红。服务端路由强制信任（routes/workspace-local-open.ts:38），这些测试断言的是与服务端拒绝保持一致的 UI 策略。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| title={title} | ||
| aria-label={title} | ||
| data-web-shell-workspace-overview={item} |
There was a problem hiding this comment.
[Suggestion] R1-32: The facet row's accessible name (aria-label={title}) lost its only pin when WorkspaceOverview.test.tsx was deleted — that file asserted the chip aria-label 'MCP: 3 of 4 connected, 1 failed'. No remaining unit or e2e test asserts a facet row's aria-label (this file asserts only the mcp row's title and textContent; the sessions row is the only row whose aria-label is pinned; the e2e asserts none). Dropping the attribute ships green (measured: 200/200 across the five tooltip-rendering suites), and screen-reader users then get the row's bare text content ("MCP1/2", icon aria-hidden) instead of the full detail "MCP: 1 of 3 connected, 1 failed, 1 disabled" — the failure count that makes the row actionable is silently lost from the accessible name.
Witness:
[probe] aria-label dropped: 200/200 green across the five suites
control (title mutated): FAILED — Expected "MCP: 1 of 3 connected, 1 failed, 1 disabled", Received "…disabled!"
Add one aria-label assertion to an existing popover test, e.g. expect(facetRow(details, 'mcp')?.getAttribute('aria-label')).toBe('MCP: 1 of 3 connected, 1 failed, 1 disabled'); — removing aria-label={title} from the facet row must turn it red.
中文说明
facet 行的可读名称（aria-label={title}）在 WorkspaceOverview.test.tsx 被删除时失去了唯一的钉子——该文件曾断言芯片的 aria-label 'MCP: 3 of 4 connected, 1 failed'。现存的单测和 e2e 都不再断言 facet 行的 aria-label（本文件只断言 mcp 行的 title 和 textContent；sessions 行是唯一钉了 aria-label 的行；e2e 不断言任何 aria-label）。删除该属性可以全绿上线（实测五个渲染 tooltip 的套件 200/200），屏幕阅读器用户此后只能听到行的裸文本内容（"MCP1/2"，图标 aria-hidden），而不是完整明细 "MCP: 1 of 3 connected, 1 failed, 1 disabled"——让该行可行动的失败计数从可读名称中静默消失。
修复：在现有弹层测试中补一条 aria-label 断言，例如 expect(facetRow(details, 'mcp')?.getAttribute('aria-label')).toBe('MCP: 1 of 3 connected, 1 failed, 1 disabled');——移除 facet 行的 aria-label={title} 必须使其变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…ce-hover-details # Conflicts: # packages/sdk-typescript/scripts/build.js
…rking-directory handoff, a11y and test pins
|
Round-2 review addressed in Criticals
Notable suggestions
Deferred to a follow-up (recording per review-round policy; none blocks behavior):
Verification: 332 web-shell sidebar/util tests, 55 daemon local-open/guard tests, the 4-spec workspace-overview e2e, filtered serve capability suites, and a root |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 vacuous synthetic-workspace negative assertions (WorkspaceSection.test.tsx) — already reported (comment 3896577987)
- R1-7 isLocalDaemon zero test coverage (config/daemon.ts) — already reported (comment 3896578001)
- R1-10 running-dot exclusion conditions unwitnessed (WebShellSidebar.tsx) — already reported (comment 3896578020)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 3: executing local-path-open.test.ts under vitest (worktree has no node_modules ; install + build exceeds remaining budget), so assertions were verified by cros…; "agent reverse-audit (round 4)": (none — no check was cut short); chunk 9: none — all checks I started completed; the only unperformed check (running the suite) was infeasible due to the missing dependency install, reported above..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Test Plan (not a blocker): routes/workspace-local-open.test.ts — no such file or directory.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/local-path-open.ts:139 — [probe] timeoutMs test hook ignored on the three execFileAsync branches (darwin/linux folder, darwin terminal hard-code OPEN_TIMEOUT_MS)packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx:271 — [probe] terminal-button announcement/label unpinned; wiring the terminal to the folder's strings ships greenpackages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx:221 — [probe] OpenLocallyButton re-enable on failure (.finally setPending(false)) unpinned; moving it into the success branch wedges the buttonpackages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.tsx:293 — [probe] popover Open folder/Open terminal buttons unreachable by keyboard (portal tab order + 100 ms blur-close); menu path is the keyboard alternativepackages/web-shell/client/e2e/web-shell.workspace-overview.spec.ts:438 — [probe] e2e menu path visibility-only; swapping the two menu handlers ships greenpackages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx:192 — [probe] terminal-button absence unasserted in folder-only configs; dropping the guard yields a phantom button with onOpen=undefinedpackages/cli/src/serve/local-path-open.test.ts:180 — [probe] darwin GUI-evidence guards have only an overdetermined negative fixture {0,501}; removing either conjunct ships greenpackages/cli/src/serve/local-path-open.test.ts:40 — [probe] isExecutableFile's accessSync(X_OK) has no negative fixture; removing the probe ships greenpackages/cli/src/serve/routes/workspace-local-open.ts:61 — [probe] early return after the 501 branch unwitnessed; deletion ships green and causes ERR_HTTP_HEADERS_SENT + misleading stderr per 501packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx:317 — [probe] keyboard focus-open test pins only an upper bound; instant-open mutation ships greenpackages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx:299 — [probe] keyboard blur-close half has zero coverage; removing the anchor onBlur ships greenpackages/web-shell/client/e2e/web-shell.workspace-overview.spec.ts:195 — [probe] popover capability gate has no negative pin; dropping the gate ships green and phantom buttons still actpackages/web-shell/client/e2e/web-shell.workspace-overview.spec.ts:416 — [probe] folder button's request body never pinned in e2e; folder→openTerminalLocally mix-up ships green
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 3：executing local-path-open.test.ts under vitest (worktree has no node_modules ; install + build exceeds remaining budget), so assertions were verified by cros…；"agent reverse-audit (round 4)"：(none — no check was cut short)；chunk 9：none — all checks I started completed; the only unperformed check (running the suite) was infeasible due to the missing dependency install, reported above.。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
Test Plan（非阻断）：routes/workspace-local-open.test.ts — no such file or directory。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 13 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| } | ||
|
|
||
| /** The long form of a facet's value, used as the tooltip row's title. */ | ||
| export function overviewDetail( |
There was a problem hiding this comment.
[Suggestion] R1-2: overviewDetail's per-facet tooltip strings lost their witnesses in the chip→popover swap — only the mcp branch's title is pinned by any test. Swapping the total/enabled variables in the skills branch, or renaming any sidebar.overview.*Detail i18n key (the translator falls back to the raw key), ships green across the unit and e2e suites and renders wrong user-visible tooltip text.
Witness:
mutant: total/enabled swapped in the skills branch → sidebar suites `Tests 189 passed`
flip probe asserting the skills title → AssertionError: expected 'Skills: 2 of 1 enabled' to be 'Skills: 1 of 2 enabled'
Extend WorkspaceDetailsTooltip.test.tsx (or workspaceOverviewModel.test.ts) to assert the detail title string for each facet branch, including the hooks disabled/enabled variants. The new per-facet title assertions must turn red if a variable pair is swapped in any overviewDetail branch — please prove it by running that mutation.
中文说明
芯片→弹层改造后，overviewDetail 的各 facet 详情字符串失去了测试钉子——目前只有 mcp 分支的 title 被断言。把 skills 分支里的 total/enabled 变量对调，或者重命名任意 sidebar.overview.*Detail i18n 键（翻译器会回退为原始键名），单测与 e2e 全部绿灯，但用户看到的悬浮提示文案是错的。
修复：在 WorkspaceDetailsTooltip.test.tsx（或 workspaceOverviewModel.test.ts）中为每个 facet 分支（含 hooks 的禁用/启用变体）断言详情 title。新增断言必须在对调任一 overviewDetail 分支的变量对后变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| it('keeps plain folder headers when the overview is switched off', () => { | ||
| renderSidebar({ workspaceOverview: false }); |
There was a problem hiding this comment.
[Suggestion] R1-3: Still standing from round 1. The negative assertions in 'keeps plain folder headers when the overview is switched off' can never fail: the path/overview/sessions rows now render only inside a Radix-ported popover that mounts while open, and this test never opens it — the assertions run against a DOM that cannot contain the rows under any wiring. Force-mounting the tooltip with a bogus path row for a workspace that should have none ships green.
Witness:
mutant force-mounting the tooltip when the overview is off → test stays green (9/9)
flip: forcing the popover open under the same mutant → expected <span data-web-shell-workspace-path="true" title="/tmp/project">/tmp/project</span> to be null
Open the workspace details popover (hover/focus plus the 300 ms delay) before asserting the rows are absent, so the assertions reach the portal content. The updated test must go red if the popover is wired to render a path row while the overview is off.
中文说明
第一轮遗留未解决。'keeps plain folder headers when the overview is switched off' 中的否定断言永远不会失败：path/overview/sessions 行现在只渲染在打开时才挂载的 Radix portal 弹层里，而该测试从未打开弹层——断言面对的 DOM 在任何接线下都不可能出现这些行。把弹层强制挂载并为不该显示路径的工作区渲染假路径行，测试照样绿灯。
修复：在断言这些行不存在之前先打开工作区详情弹层（hover/focus + 300ms 延迟），让断言能到达 portal 内容。若弹层被接线为在 overview 关闭时仍渲染路径行，更新后的测试必须变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| gitState: workspaceGitState, | ||
| sendBridgeError, | ||
| }); | ||
| registerWorkspaceLocalOpenRoutes(app, { |
There was a problem hiding this comment.
[Suggestion] R1-8: Still standing from round 1. The production mount of POST /workspaces/:workspace/open in createServeApp is verified by no test — every existing POST to the route runs against the route test's own express app. Capability advertising is wired independently, so dropping this registration call in a future refactor keeps the whole suite green while clients still see the buttons and get 404 on click.
Witness:
witness: not run — the flip is a supertest POST against createServeApp, i.e. the missing test itself; all 13 POSTs to /workspaces/.../open live in routes/workspace-local-open.test.ts's own app, zero in server.test.ts, and no route-inventory guard exists.
Add a wiring test in server.test.ts that POSTs /workspaces/:workspace/open against createServeApp (mock ../local-path-open.js, or assert the headless 501 arm). Deleting the registration call must turn the new test red — please prove it by running that mutation.
中文说明
第一轮遗留未解决。createServeApp 中 POST /workspaces/:workspace/open 的生产挂载没有任何测试覆盖——现有对该路由的 13 处 POST 全部跑在路由测试自建的 express app 上。能力宣告（capability）的接线是独立的，因此未来重构删掉这行挂载，整个测试套件仍全绿，而客户端依旧显示按钮、点击却得到 404。
修复：在 server.test.ts 中新增接线测试，对 createServeApp 发起 POST /workspaces/:workspace/open（mock ../local-path-open.js，或断言 headless 501 分支）。删除该挂载调用必须使新测试变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const child = { once: vi.fn(), kill: vi.fn(), unref: vi.fn() }; | ||
| spawnMock.mockReturnValue(child); | ||
|
|
||
| await expect(openPathLocally(existingDir, 5)).rejects.toBeInstanceOf( |
There was a problem hiding this comment.
[Suggestion] R1-9: (fix-induced) The round-1 fix that pinned the hang guards rewrote both witnesses to pass an explicit timeoutMs=5, deleting the only instrument that observed the spawn-side OPEN_TIMEOUT_MS default bindings (local-path-open.ts:283 destructure and :310 parameter default). Removing both defaults ships the whole suite green — success paths resolve via queueMicrotask before any timer, and the hang tests pass 5 explicitly. In production the route calls openLocally(runtime.workspaceCwd) with no second argument, so setTimeout(..., undefined) arms a ~1 ms timer: explorer.exe/wt.exe/the terminal are killed before they can launch and every win32/linux local-open request answers 501.
Witness:
baseline `Tests 41 passed (41)` → mutant (both defaults removed) `Tests 52 passed (52)`
setTimeout(cb, undefined) measured firing after 3 ms (node -e)
Keep the explicit-timeout tests and add one default-path witness per spawn helper: call openPathLocally(existingDir) / openTerminalLocally(existingDir) with no timeoutMs against a never-firing child and assert the arm delay is 10 000 (e.g. a setTimeout spy) and that kill+reject follow on expiry. Note const OPEN_TIMEOUT_MS = 10_000; (local-path-open.ts:16) is the single source of the default, and the deferred round-2 finding on the three execFileAsync branches still requires timeoutMs threaded there — a fix must not re-hardcode the constant in one branch only. The new default-delay assertion must go red under the default-removal mutant, which the entire current suite ships green — please prove it by running that mutation.
中文说明
（修复引入）第一轮为挂起守卫补测试的修复把两个见证用例都改写为显式传入 timeoutMs=5，删掉了唯一能观测 spawn 侧 OPEN_TIMEOUT_MS 默认绑定（local-path-open.ts:283 解构与 :310 参数默认值）的仪器。移除这两个默认值后整个套件依旧全绿——成功路径在任何计时器之前经 queueMicrotask resolve，挂起测试又显式传 5。而生产环境里路由以 openLocally(runtime.workspaceCwd) 调用、不带第二个参数，于是 setTimeout(..., undefined) 会布下一个约 1ms 的计时器：explorer.exe/wt.exe/终端在来得及启动前就被 kill，win32/linux 的每次本地打开请求都返回 501。
修复：保留显式超时测试，并为每个 spawn 辅助各补一个默认路径见证——不带 timeoutMs 调用 openPathLocally(existingDir) / openTerminalLocally(existingDir)，对永不触发事件的 child 断言布防延迟为 10 000（如用 setTimeout spy），且到期后 kill+reject。注意 const OPEN_TIMEOUT_MS = 10_000;（local-path-open.ts:16）是默认值的唯一来源，且第二轮被推迟的关于三个 execFileAsync 分支的发现仍要求把 timeoutMs 穿透到那里——修复时不要在某个分支重新硬编码该常量。新增的默认延迟断言必须在移除默认值的变异下变红（当前整个套件对该变异全绿）——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| trigger.dispatchEvent(new Event('pointerover', { bubbles: true })); | ||
| vi.advanceTimersByTime(300); |
There was a problem hiding this comment.
[Suggestion] R1-28: Still standing from round 1. The hover-intent cancel — closeAfterDelay clearing the pending 300 ms open timer when the pointer leaves before the popover opens — remains effectively unpinned: removing the window.clearTimeout(openTimerRef.current) line ships green across all three sidebar suites. Under fake timers the stale open callback and the close callback expire in the same tick and React batches setOpen(true)/setOpen(false), masking it; in real time the two are separate tasks, so after a pre-delay pointer-exit the popover mounts momentarily — a one-flash open/close — before the delayed close lands.
Witness:
mutant removing clearTimeout(openTimerRef.current) from closeAfterDelay → Test Files 3 passed (3) / Tests 166 passed (166)
Add a test that can see the flash: pointerover then pointerout before 300 ms under real timers (or tick-separated fake timers), verifying no dialog commit occurs at any point (e.g. MutationObserver or per-tick polling), not just absence at the end. It must go red if the clearTimeout(openTimerRef.current) line is removed — please prove it by running that mutation.
中文说明
第一轮遗留未解决。hover 意图取消——指针在弹层打开前离开时 closeAfterDelay 清除待执行的 300ms 打开计时器——实际上仍未被钉住：删掉 window.clearTimeout(openTimerRef.current) 这一行，三个侧栏套件全绿。在 fake timers 下，过期的打开回调与关闭回调同一 tick 到期、React 批处理 setOpen(true)/setOpen(false) 而掩盖了问题；真实时间下二者是独立任务，指针在延迟前离开后弹层会瞬间挂载又关闭——一闪而过。
修复：补一个能捕获该闪烁的测试——真实计时器（或逐 tick 分离的 fake timers）下先 pointerover 再在 300ms 前 pointerout，验证全程任何时刻都无 dialog 提交（如 MutationObserver 或逐 tick 轮询），而不只是最终不存在。移除 clearTimeout(openTimerRef.current) 行后该测试必须变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| await spawnAndIgnoreExitCode('wt.exe', ['-d', path], { timeoutMs }); | ||
| } catch { |
There was a problem hiding this comment.
[Suggestion] R2-1: spawnWindowsTerminal's blanket catch also falls back when the wt.exe hang guard fires — not only on a spawn-level error — conflating 'wt.exe absent' with 'wt.exe slow', and no test pins which behavior is intended on a hang. On a host where wt.exe spawns but does not exit within the 10 s window (a PATH shim, a stalled first-run launch), spawnAndIgnoreExitCode kills it and rejects; the catch then launches the PowerShell/cmd fallback anyway — one click opens two terminal windows after >=10 s of request latency. The fallback test triggers via fakeChild('error') only, so narrowing or widening the catch ships green.
Witness:
probe, hung wt.exe child + 5 ms timeout:
intact: outcome=resolved, spawn count=2 (wt.exe killed, then powershell.exe)
candidate fix (fall back only on spawn-level error): outcome=rejected:Unavailable, spawn count=1
Decide the hang semantics explicitly: if a hang should not double-open, make spawnAndIgnoreExitCode reject with a distinguishable timeout error and re-throw it from spawnWindowsTerminal without falling back; keep the fallback for spawn-level error events. If the double-open on hang is intended, pin it with a test instead. The new 'does not fall back when wt.exe hangs' test (never-firing child, openTerminalLocally(existingDir, 5) rejects, spawn called exactly once) must go red when the blanket catch is restored — please prove it by running that mutation.
中文说明
spawnWindowsTerminal 的兜底 catch 在 wt.exe 挂起守卫触发时也会回退——而不仅是 spawn 级错误——把「wt.exe 不存在」与「wt.exe 慢」混为一谈，且没有测试钉住挂起时应是哪种行为。在 wt.exe 能 spawn 但 10 秒窗口内不退出（PATH 上的 shim、首次启动卡住）的主机上，spawnAndIgnoreExitCode 会将其 kill 并 reject，catch 随后仍然启动 PowerShell/cmd 回退——一次点击打开两个终端窗口，且请求延迟 >=10 秒。回退测试只用 fakeChild('error') 触发，因此收窄或放宽这个 catch 都照样全绿。
修复：显式决定挂起语义：若挂起不应双开，让 spawnAndIgnoreExitCode 以可区分的超时错误 reject，并在 spawnWindowsTerminal 中直接重抛、不回退；回退只保留给 spawn 级 error 事件。若确实希望挂起时双开，则用测试钉住。新增的「wt.exe 挂起时不回退」测试（永不触发事件的 child、openTerminalLocally(existingDir, 5) reject、spawn 恰好调用一次）必须在恢复兜底 catch 后变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| onPointerLeave={() => { | ||
| if (!containsFocusTarget(document.activeElement)) { | ||
| closeAfterDelay(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
[Suggestion] R2-2: After any WorkspaceMenu action, Radix restores focus to the menu trigger — which renders inside this popover's anchor — and the anchor onFocus then reopens the details popover, which latches open with no path left to close it. Mouse user: hover the row (popover opens), click the ⋯ menu, pick Open folder; on menu unmount Radix's FocusScope restores focus to the trigger (handleSessionMenuCloseAutoFocus only suppresses the rename/pointer-dismiss paths, and an item selection is neither). The focusin bubbles to the anchor → openAfterDelay → the popover reopens 300 ms later. Focus stays on the trigger (no onBlur) and the pointer sits where the portaled menu item was (no onPointerLeave transition), so the tooltip stays latched; hovering another workspace row stacks a second popover.
Witness:
jsdom probe, real Radix, production-shaped wiring:
default arm: focusins=["BUTTON"] dialogAfter=true (popover reopened after selecting Open folder)
candidate fix (onCloseAutoFocus prevented): focusins=[] dialogAfter=false
The fix belongs at the menu content — pass onCloseAutoFocus={(event) => event.preventDefault()} on the workspace DropdownMenuContent (or suppress the focus-open path via a ref set in onPointerDownCapture, cleared on anchor blur/pointer-enter). Keyboard parity must survive: genuine keyboard focus must still open the popover after the same 300 ms delay (the lower-bound pin from R1-18's fix must not regress), which is the constraint this fix must not violate. A new test — open via pointer-enter, pointerdown+click an anchor child, .focus() it to simulate Radix's restore, advance past 300 ms, assert the popover stays closed — must go red when the suppression is removed — please prove it by running that mutation.
中文说明
在 WorkspaceMenu 执行任意操作后，Radix 会把焦点还给菜单触发按钮——它就渲染在本弹层的 anchor 内部——随后 anchor 的 onFocus 会重新打开详情弹层，且弹层卡死在打开状态、没有任何路径能关闭它。鼠标用户：悬停行（弹层打开）→ 点 ⋯ 菜单 → 选「打开文件夹」；菜单卸载时 Radix FocusScope 把焦点还给触发按钮（handleSessionMenuCloseAutoFocus 只抑制改名/指针关闭两条路径，菜单项选择两者都不是），focusin 冒泡到 anchor → openAfterDelay → 300ms 后弹层重开。焦点停在触发按钮上（无 onBlur），指针停在已卸载的菜单项原处（无 onPointerLeave 过渡），于是弹层一直挂着；再悬停另一个工作区会叠出第二个弹层。
修复：在菜单内容上处理——给工作区 DropdownMenuContent 传 onCloseAutoFocus={(event) => event.preventDefault()}（或用 onPointerDownCapture 中设置、anchor blur/pointer-enter 时清除的 ref 来抑制 focus 打开路径）。键盘对等必须保留：真正的键盘聚焦仍应在同样的 300ms 延迟后打开弹层（R1-18 修复钉住的下界不能回退），这是本修复不得违反的约束。新增测试——指针进入打开、对 anchor 子元素 pointerdown+click、再 .focus() 模拟 Radix 焦点还原、推进超过 300ms、断言弹层保持关闭——必须在移除抑制逻辑后变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| folderButton!.dispatchEvent(new MouseEvent('click', { bubbles: true })); | ||
| await Promise.resolve(); | ||
| }); | ||
| expect(onOpenPathLocally).toHaveBeenCalledWith('/tmp/project'); |
There was a problem hiding this comment.
[Suggestion] R2-3: This gates test pins the folder lambda's argument but only pins the terminal button's existence — the adjacent onOpenTerminalLocally lambda's workspace.cwd argument is asserted by no test anywhere. The two wiring lambdas in WorkspaceSection.tsx are copy-paste-shaped; corrupting the terminal one — e.g. swapping in workspaceLabel(workspace), which appears two lines up — ships green: this unit test never clicks the terminal button, the tooltip test takes an argument-less prop, and the e2e filters the request path with a regex matching any selector while the mock daemon answers any POST /workspaces/*/open unconditionally. 'Open terminal' would then silently spawn at a nonexistent or wrong path (daemon 400 workspace_mismatch, or a wrong workspace on a basename collision).
Witness:
mutant onOpenTerminalLocally(workspaceLabel(workspace)) → Test Files 2 passed (2), Tests 63 passed (63) — identical to baseline
| expect(onOpenPathLocally).toHaveBeenCalledWith('/tmp/project'); | |
| expect(onOpenPathLocally).toHaveBeenCalledWith('/tmp/project'); | |
| await act(async () => { | |
| details | |
| .querySelector('[data-web-shell-open-workspace-terminal]')! | |
| .dispatchEvent(new MouseEvent('click', { bubbles: true })); | |
| await Promise.resolve(); | |
| }); | |
| expect(onOpenTerminalLocally).toHaveBeenCalledWith('/tmp/project'); |
Click the terminal button in the same test and pin the argument, mirroring the folder arm. The added assertion must go red when the terminal lambda's argument is mutated — please prove it by running that mutation.
中文说明
该 gates 测试钉住了 folder lambda 的参数，却只钉住终端按钮的存在性——相邻 onOpenTerminalLocally lambda 的 workspace.cwd 参数在任何测试中都未被断言。WorkspaceSection.tsx 中两个接线 lambda 是复制粘贴形态；破坏终端那个——例如换成上方两行就出现的 workspaceLabel(workspace)——全套件依旧绿灯：本单测从未点击终端按钮，tooltip 测试接收无参 prop，e2e 用匹配任意选择器的正则过滤请求路径且 mock daemon 对任何 POST /workspaces/*/open 无条件应答。「打开终端」将静默在不存在或错误的路径上 spawn（daemon 400 workspace_mismatch，或 basename 碰撞时打开错误的工作区）。
修复：在同一测试中点击终端按钮并钉住参数（如上建议块），与 folder 分支对称。新增断言必须在终端 lambda 参数被变异后变红——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // xterm is X11-only: a pure-Wayland session must not count it. | ||
| expect( | ||
| isLocalTerminalAvailable({ | ||
| WAYLAND_DISPLAY: 'wayland-0', | ||
| PATH: xtermDir, | ||
| }), | ||
| ).toBe(false); |
There was a problem hiding this comment.
[Suggestion] R2-4: linuxTerminalCandidates' pure-Wayland arm is only witnessed in the negative — xterm excluded under WAYLAND_DISPLAY. The positive half, gnome-terminal/konsole still counting when only WAYLAND_DISPLAY is set, has no witness (every Wayland fixture uses xtermDir; every positive emulator fixture sets DISPLAY). Simplifying the helper to env['DISPLAY'] ? LINUX_TERMINAL_NAMES : [] ships green — the Wayland+xterm arm still returns false either way and the integration boot oracle recomputes from the same function, flipping along. Because probe and spawner share the helper, the local-terminal capability would then silently disappear from /capabilities on pure-Wayland sessions (the GNOME/KDE default on current distros) and the client hides the button.
Witness:
mutant `return env['DISPLAY'] ? LINUX_TERMINAL_NAMES : [];` → Tests 41 passed (41)
Add positive arms: expect(isLocalTerminalAvailable({ WAYLAND_DISPLAY: 'wayland-0', PATH: gnomeTerminalDir })).toBe(true) (and the konsoleDir variant), plus a spawner arm asserting openTerminalLocally under WAYLAND_DISPLAY-only spawns gnome-terminal. Both consumers share the helper — isLocalTerminalAvailable (local-path-open.ts:106) and spawnLinuxTerminal (local-path-open.ts:253) — so pin both the probe and the spawn path; a future split of the helper must not regress one side only. The new arms must go red under the empty-set mutant, which the whole current suite ships green — please prove it by running that mutation.
中文说明
linuxTerminalCandidates 的纯 Wayland 分支只有负向见证——仅有 WAYLAND_DISPLAY 时排除 xterm。正向的一半（只有 WAYLAND_DISPLAY 时 gnome-terminal/konsole 仍应计入）没有任何见证（所有 Wayland 用例都用 xtermDir，所有正向模拟器用例都设了 DISPLAY）。把辅助函数简化为 env['DISPLAY'] ? LINUX_TERMINAL_NAMES : [] 全套件全绿——Wayland+xterm 分支两种写法都返回 false，集成启动预言又从同一函数重算、随之一起翻转。由于探测器与启动器共用该辅助函数，本地终端能力会在纯 Wayland 会话（当前发行版 GNOME/KDE 的默认形态）上从 /capabilities 静默消失，客户端随之隐藏按钮。
修复：补正向分支：expect(isLocalTerminalAvailable({ WAYLAND_DISPLAY: 'wayland-0', PATH: gnomeTerminalDir })).toBe(true)（及 konsoleDir 变体），再补一个启动器分支，断言仅 WAYLAND_DISPLAY 下 openTerminalLocally 会 spawn gnome-terminal。两个消费者共用辅助函数——isLocalTerminalAvailable（local-path-open.ts:106）与 spawnLinuxTerminal（local-path-open.ts:253）——请同时钉住探测与 spawn 两条路径，避免未来拆分辅助函数时只回退一侧。新增分支必须在空集合变异下变红（当前整个套件对该变异全绿）——请以该变异验证。
— qwen3.8-max via Qwen Code /review (v0.22.3)
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
What this PR does / PR 主旨
EN: This PR declutters the web-shell workspace sidebar (session counts, path, git branch and facet stats move from inline chips into a hover details popover with keyboard parity) and adds loopback-only local-open actions: two daemon capabilities (workspace_local_open, workspace_local_terminal) probed once at boot from host GUI-session evidence, served by POST /workspaces/:workspace/open, handing off to the OS and answering 501 honestly when the directory is gone or no display/terminal exists.
中文：本 PR 整理 web-shell 工作区侧栏（计数、路径、分支与 facet 统计从内联芯片移入支持键盘对等的悬浮详情弹层），并新增仅回环可见的本地打开能力：两个启动时按宿主 GUI 会话证据探测一次的 daemon 能力，经 POST /workspaces/:workspace/open 交接给操作系统，目录缺失或无显示器/终端时诚实返回 501。
Round-4 scope / 第四轮范围
EN: Purely static re-review at head ab48b2106b5710c50cd4938970db5570408e0bd7 (no tests/builds executed). The increment since the round-3 verification (650d2f3) is a single commit — "make the spawn hang guards testable without fake timers" — plus the bot's newest review at this head. My round-3 APPROVE was auto-dismissed by the push, exactly as this branch's earlier approvals were; nothing in the new commit invalidated the previously verified state (checked below rather than assumed).
中文：本轮为纯静态复审，基线 ab48b2106。相对第三轮验证点（650d2f31d1）的增量只有一个提交——"让 spawn 挂起守卫不依赖 fake timers 也可测"——外加机器人在本 head 的最新审查。我第三轮的 APPROVE 因新推送被自动 dismiss（与本分支此前所有批准同款）；新提交并未使已验证状态失效，以下逐项核实而非想当然。
The increment, verified / 增量逐项核实
The change adds an optional timeoutMs parameter to openPathLocally/openTerminalLocally ("exists for tests; production callers use the default") and threads it through the spawn helpers; the private spawnAndIgnoreExitCode moves from a positional extraEnv to an options object { extraEnv, timeoutMs = OPEN_TIMEOUT_MS }, and the two rejection messages interpolate the actual timeout instead of hard-coding "10s". I checked: production behavior is byte-for-byte the same for every real path (the route at routes/workspace-local-open.ts:31-32 calls both with one argument, so the default applies; the win32 PowerShell fallback correctly migrates to { extraEnv: { QWEN_LOCAL_OPEN_DIR: path }, timeoutMs } — the env-carrying directory never enters a parsed command line, preserving the round-2 hardening); spawnLinuxTerminal's two shared invariants are alive (DISPLAY/WAYLAND_DISPLAY gate at :250-252, and probe/spawner both consuming linuxTerminalCandidates(env) — the R1-27/R1-30 fixes are untouched); the deleted-directory assertPathExists pre-checks on both win32 branches (:149, :192) and the Linux mirror (:197) are all intact; the process-env guard still holds — exactly 4 process.env sites in the file (:86/:99/:247/:287) against the declared whole: 4. The test rewrite replaces fake-timer advancement with a real 5 ms timeout against a stubbed child, asserting the same contract (kill + LocalPathOpenUnavailableError) with fewer moving parts — a strict improvement. One known trade-off stays: the three execFileAsync branches still hard-code OPEN_TIMEOUT_MS (the bot recorded this as a deferred probe) — test-hook inconsistency only, no production impact.
The three Criticals from the earlier CHANGES_REQUESTED (Linux deleted-directory false success, spawn-time display gate, pure-Wayland xterm) were verified fixed at 650d2f3 in round 3 and I re-confirmed each anchor at this head; the review bot's own newest pass at ab48b2106 posted zero Criticals — nine inline Suggestions, thirteen probes deferred as non-blockers, three duplicates of already-reported test-gap notes. Nothing in this round's new surface changes my read.
CI / 持续集成
EN: At publish time on ab48b2106: 21 checks pass, Test (ubuntu-latest) is still pending, remaining lanes skipped. Stated as-is; I do not wait on CI.
中文：发布时刻 21 项通过，Test (ubuntu-latest) 仍在运行，其余为 skip。如实记录，不等待。
Verdict / 结论
EN: APPROVE. Fourth pass over this PR, second over its current shape; the increment is a testability refactor that provably preserves every hardened behavior, the three previously filed Criticals remain fixed at this head, and the only open observations on both sides are Suggestion-level test-gap notes (legitimate follow-ups, none blocking). The stale CHANGES_REQUESTED state on this PR reflects commits that no longer exist on the branch — at ab48b2106 I find no Critical or Important finding, verified statically.
中文：批准（APPROVE）。 这是对该 PR 的第四轮、对其当前形态的第二轮审查；增量是一次可证不破坏任何加固行为的可测性重构，三条历史 Critical 在本 head 仍为修复态，双方剩余意见均为建议级测试缺口备忘（合理跟进项，不阻塞）。本 PR 上挂着的 CHANGES_REQUESTED 状态反映的是已被 rebase 移出分支的提交——在 ab48b2106 上，我未发现任何 Critical 或 Important 级发现。本轮为纯静态审查，未运行任何测试。
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Independent re-review at ab48b2106 — Critical-only / 独立复审
Re-verified statically against the exact head ab48b2106b5710c50cd4938970db5570408e0bd7 (head tree + CI API reads; no tests executed here). Line numbers below are at this head. / 以下为针对该精确 head 的读码复核结论,行号均以本 head 为准。
1. 历史问题复核 / Historical issues — resolution verified
- Bot Criticals R1-17 / R1-27 / R1-30 — 已修复 (verified fixed).
packages/cli/src/serve/local-path-open.ts: the Linux terminal branch runsassertPathExists(path)before spawning (:199);spawnLinuxTerminalthrowsLocalPathOpenUnavailableError('No display server available')when neitherDISPLAYnorWAYLAND_DISPLAYis set (:250-252); probe (:106) and spawner (:253) sharelinuxTerminalCandidates(env), which drops the X11-onlyxtermon pure-Wayland sessions. The route maps the error to 501 and answersopened: trueonly on no-throw. - Round-1 human findings — 已修复. Keyboard parity on the details popover (anchor
onFocus/onBlurwith the same 300 ms delay,WorkspaceDetailsTooltip.tsx:235-241; focus-aware pointer-leave viacontainsFocusTargeton anchor and portaled content); win32assertPathExistspre-checks on both branches (:148, :192);formatDateTimecompares calendar days (toDateString);sidebar.overview.label/…unknown/…unavailableremoved from both locales; icon-swap tests pin concretesvg.lucide-*selectors; the cmd fallback is%VAR%-free (Start-Process cmd.exe -WorkingDirectory "$env:QWEN_LOCAL_OPEN_DIR", :236); theworkspace_local_openprotocol-doc row is a well-formed 2-column row (qwen-serve-protocol.md:543, exactly 3 pipes);OpenLocallyButtonhas the sr-onlyaria-livesuccess announcement + in-flightpendingguard released via.finally;headerActionCountexcludes the menu trigger when locked (WebShellSidebar.tsx:5859-5862). - Daemon surface re-audited at this head: the route fails closed in order (mutate gate → 400/503/403 by
resolveTrustedRuntime→ whitelist-mappedfolder/terminal→ registry-resolvedruntime.workspaceCwdis the only path ever opened); env-guard allowancewhole: 4matches exactly fourprocess.envreads (:86/:99/:247/:287); docs count 149 registered tags matches the registry; capability probes flow through both bootstrap envelopes;registerWorkspaceLocalOpenRoutesis mounted (server.ts:2320). 本轮扫描未发现新的 Critical。
2. 仍存在的实质性历史问题 / Substantive items still standing at this head
- R2-1 —
spawnWindowsTerminal's blanketcatchalso falls back when the wt.exe hang guard fires (local-path-open.ts:217-241).spawnAndIgnoreExitCoderejects both on a spawn-levelerrorand on the hang timeout (${command} did not exit within ${timeoutMs}ms, :291); the untypedcatch {at :223 launches the PowerShell/cmd fallback for either. A wt.exe that spawns but stalls ≥10 s is killed, then double-opened one click later with ≥10 s request latency. Actionable: reject the hang with a distinguishable error type and re-throw it fromspawnWindowsTerminal(fallback only on spawn-level errors), or pin the intended double-open with a test that goes red when the catch is narrowed/widened. - R2-2 — selecting a workspace-menu item reopens and latches the details popover (
WorkspaceDetailsTooltip.tsxanchoronFocus→openAfterDelay:172/:233-237;WebShellSidebar.tsxhandleSessionMenuCloseAutoFocus:1362-1371 wired at :5962). TheWorkspaceMenutrigger renders inside the popover's anchor; after an item selection Radix restores focus to the trigger and the handler onlypreventDefault()s the rename and pointer-dismiss arms — an item selection is neither — so thefocusinbubbles to the anchor and the popover reopens 300 ms later with no blur/pointer-leave to close it; hovering another row stacks a second popover. Actionable: suppress the focus-restore for the workspace menu's item-selection close (onCloseAutoFocuspreventDefault) or gate the focus-open path behind a pointer-driven flag — without regressing the keyboard-parity open (the 300 ms-delayed genuine focus must still open it).
(记录但不作为本次发现门禁 / recorded, not re-derived as new findings: the remaining Suggestion-level test-pinning gaps on the round-2 ledger — R1-2/R1-3/R1-8/R1-9/R1-28/R2-3/R2-4, the 13 deferred probes, and the author-deferred isLocalDaemon() unit tests and createServeApp mount assertion. Each was re-read at head: the production logic behind them is sound.)
3. CI 阻塞 / CI blockers on this exact head
Test (ubuntu-latest, Node 22.x)has no green completion atab48b21and nothing is currently in flight (run 33461820573): attempt 4 ❌ — single failurepackages/core src/utils/shellAstParser.test.ts > classifies adversarial rule inputs within the CPU budget(1630.8 ms against a 1000 ms budget with vitest collect at 1735 s — a load-sensitive timing guard in a package whose only PR touch is a Prettier reflow of an unrelated test file, so not PR-caused); attempt 5 cancelled; attempt 6 ❌ "The self-hosted runner lost communication with the server" (infra); attempt 7 cancelled by @wenshao at 08:54Z. A main unit lane that ends cancelled is not a healthy completion — please re-run the suite to green on this head.web-shell E2E Smokewas cancelled with attempt 7 (needs the same re-run).- All other reported lanes are green at this head: Serve A/B (no response changes), Desktop Shell (ubuntu/windows), Integration (no-AK), Real daemon E2E, Dependency CVE audit, Secret scan, web-shell visuals, Classify PR, Java matrix.
结论 / Verdict
💬 COMMENT — 当前 head 无新的 Critical;不批准的原因是 §2 两项仍存在的实质性历史问题(R2-1、R2-2)与 §3 主测试车道在本 head 上没有绿色完成。修掉或钉住这两项、且 Test (ubuntu-latest) 在本 head 重跑转绿后,以本 head 的形态可以直接批准。/ No new Critical defects at this head; approval is held back by the two still-standing substantive items (R2-1, R2-2) and the absence of a green completion of the main unit lane on this head. With those resolved (fix or explicit test pin) and CI green on this head, this is approvable as-is.
— 衍星 · read-only PR review (posted as qqqys)
…ce-hover-details # Conflicts: # packages/web-shell/client/components/sidebar/WebShellSidebar.tsx
…close focus latch
|
Re-review items addressed in R2-1 (wt.exe hang also triggered the fallback) — the spawn hang guards now reject with a dedicated R2-2 (menu selection re-latched the details popover) — the workspace menu's Conflict-resolution correction: the merge briefly kept the pre-existing Verification: 345 web-shell sidebar tests, 58 daemon local-open/guard/docs tests, the 4-spec workspace-overview e2e, root On CI: the last ubuntu Test failures on the previous head were all infrastructure (serve-test daemon-boot hangs, the shellAstParser CPU budget under fleet load, one runner communication loss); #10667's concurrency tuning is in this merge, so this head should have a materially better shot. |
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
Re-review at bea3818e21 — 上轮阻塞项全部核实修复,唯一未完成门禁为运行中的主测试车道
本轮针对新 head bea3818e21c0673d2d29afe792c59d4b653c0311(原评审锚点 ab48b2106 之后的变化 = 合入 main 的 0120f704 + 修复提交 bea3818e21)。
上轮 COMMENT 两项阻塞问题的处置 — 均已修复并钉住
- wt.exe 挂起回退双开 — 已修复。新增
SpawnHangError把「spawn 成功但超时被 kill」与「spawn 级失败」分开:spawnWindowsTerminal对前者直接重抛,回退只在 wt.exe 缺失(ENOENT)时触发;openTerminalLocally的 catch 将其包装为LocalPathOpenUnavailableError,路由仍诚实返回 501(local-path-open.ts :34-37/:225-232/:297-302)。测试does not fall back when a spawned wt.exe stalls and is killed by the hang guard钉住恰一次 spawn、不启动 powershell、5ms 注入超时——恢复兜底 catch 会打红它,符合上轮要求的变异见证。 - 菜单项选择后弹层焦点卡死 — 已修复。工作区
DropdownMenuContent现无条件onCloseAutoFocus={(event) => event.preventDefault()}(WebShellSidebar.tsx :6016-6027),Radix 不再把焦点还原到位于 hover-open 锚点内的触发器,focusin → openAfterDelay的重开路径被切断。真实计时器测试does not reopen the details popover after a workspace menu selection复现「hover 开 → 菜单选择 → 450ms 后仍关」,抑制被移除即打红;键盘聚焦仍按同一 300ms 延迟打开(锚点onFocus未受影响,R1-18 的下界钉定不回退)。
其余门禁复核 / Remaining gates at this head
- 历史 Critical 与行为修复锚点在本 head 逐条在场:Linux/win32 三分支
assertPathExists(:153/:197/:204)、spawn 时显示门控与共享候选表(:260/:262/:111)、日历日时间戳、键盘对等/焦点感知关闭、aria-live播报与 in-flight 守卫;route 仍按 400/503/403 → 白名单 target → 注册表 cwd 失败封闭。 - 合入 main 未扰动本 PR 接线;对本
upstream/main的有效 diff 仍为同一 38 文件集。 - 本轮修复范围内未发现新的阻塞性正确性/安全/回归问题。
CI — 唯一未完成门禁 / The one outstanding gate
Test (ubuntu-latest, Node 22.x)在本 head 上自 11:23:40Z 起 仍在运行(run 33502227645 / job 99838034697),review-pr元检查同样在跑;其余 19 项 success、9 项路径性 skip。按流程 Pending 不计健康、也不等待轮询。
结论 / Verdict
💬 COMMENT — 代码门禁全部通过:两项上轮阻塞问题已在 bea3818 修复且各有会因回退而打红的测试;历史全部问题在本 head 复核为已修复态。唯一缺的是本 head 上 Test (ubuntu-latest) 的一次绿色完成(该车道正是此前连续多轮未能跑绿的主因,本轮重跑正在途中)。该车道在本 head 转绿后即可批准,无其他保留意见。 / All code gates pass at bea3818 — both previously-standing blockers are fixed with mutation-pinned tests, and every historical item re-verified in place. The only outstanding gate is one green completion of Test (ubuntu-latest) on this head (currently running); approve-ready as soon as it lands, no other reservations.
— 衍星 · read-only PR review (posted as qqqys)
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
APPROVE —— 已核对到最新 head bea3818。
What this PR does / PR 主旨
EN: This PR declutters the web-shell workspace sidebar (session counts, path, git branch and facet stats move from inline chips into a hover details popover with keyboard parity) and adds loopback-only local-open actions: two daemon capabilities (workspace_local_open, workspace_local_terminal) probed once at boot from host GUI-session evidence, served by POST /workspaces/:workspace/open, handing off to the OS and answering 501 honestly when the directory is gone or no display/terminal exists.
中文：本 PR 整理 web-shell 工作区侧栏（计数、路径、分支与 facet 统计从内联芯片移入支持键盘对等的悬浮详情弹层），并新增仅回环可见的本地打开能力：两个启动时按宿主 GUI 会话证据探测一次的 daemon 能力，经 POST /workspaces/:workspace/open 交接给操作系统，目录缺失或无显示器/终端时诚实返回 501。
Round-5 scope / 第五轮范围
EN: Purely static re-review at head bea3818 (no tests/builds executed). My prior analysis head ab48b21 is a strict ancestor of this head — no rebase — so the earlier verified state carries. The increment is (1) a merge of main and (2) one author commit, "address re-review — wt hang fallback guard, menu-close focus latch". Attribution is isolated to the author's own commit (a two-dot diff across the merge would miscount main's churn as PR changes).
中文：本轮为纯静态复审，基线 bea3818。我上轮分析点 ab48b21 是本 head 的严格祖先（无 rebase），先前已验证状态沿用。增量由 (1) 合并 main 与 (2) 作者一个提交（处理复审：wt 挂起回退守卫、菜单关闭焦点卡死）组成。归属按作者自有提交隔离（跨 merge 的两点 diff 会把 main 的改动误算成本 PR 变更）。
The increment, verified / 增量逐项核实
Merge drift: the effective three-dot diff is still exactly the same 38-file set; the two capability strings remain registered in the capabilities table and consumed by DaemonClient and the sidebar, and the route is still imported and mounted (server.ts:282 / :2323). The merge did not disturb this PR's wiring. / 合并未扰动本 PR 接线：有效 diff 仍是同一 38 文件集，两个能力字符串仍在注册表并被 DaemonClient 与侧栏消费，路由仍被导入并挂载（server.ts:282 / :2323）。
Author commit bea3818 — three production edits, each re-verified in the head tree with a mutation-pinned test: / 作者提交三处生产改动，均在 head tree 复核并各有回退即打红的测试：
-
wt.exe hang no longer double-opens. A new SpawnHangError separates "spawned but never exited within the timeout (killed by the hang guard)" from a spawn-level failure (wt.exe absent / ENOENT). spawnWindowsTerminal re-throws a hang instead of falling through, so the PowerShell/cmd fallback only runs when wt.exe is genuinely missing — a stalled-but-running terminal no longer opens a second window; it propagates to openTerminalLocally and still returns an honest 501 (local-path-open.ts :34-37 / :228-232 / :296-306; test "does not fall back when a spawned wt.exe stalls"). This is exactly the fix requested last round. / 新增 SpawnHangError 区分「起了但超时被 kill」与「wt.exe 缺失」，回退只在后者触发，杜绝卡死终端的双开；挂起仍诚实返回 501。
-
Primary-row New session targets the right workspace after the standalone merge. wsCwd changed from ws.primary ? undefined : ws.cwd to always ws.cwd. Once PR 10514 landed (undefined cwd now means a standalone/global session), passing undefined for the primary row would have opened a standalone chat instead of a session in the bound workspace; always using ws.cwd keeps the primary row consistent with secondary rows (WebShellSidebar.tsx:5813 → handleNewSession / handleNewWorktreeSession). / 合入 10514 后 undefined cwd 语义变为 standalone，primary 行原来传 undefined 会误开独立会话；恒取 ws.cwd 修正了这一跨 PR 语义漂移。
-
Menu selection no longer re-opens the details popover. The workspace-details dropdown's onCloseAutoFocus is now an unconditional preventDefault, because its trigger always lives inside the hover-open popover anchor — Radix restoring focus there re-triggers the 300 ms focus-open and the popover never closes. The shared conditional handler is still used by the two session menus, so nothing went dead, and keyboard focus-open is unaffected. / 工作区详情弹层内的菜单 trigger 恒在 hover-open 锚点内，Radix 还原焦点会重触发 300ms 打开致弹层永不关闭，故改为无条件抑制；两个 session 菜单仍用原条件处理器，无死码，键盘打开不受影响。
No Critical and no Important finding at this head. The stale CHANGES_REQUESTED on the PR reflects a bot review pinned to c98701a, a commit that no longer exists on the branch. / 本 head 无 Critical 亦无 Important；PR 上挂着的 CHANGES_REQUESTED 是钉在 c98701a（已不在分支上）的陈旧机器人评审。
CI / 持续集成
EN: At publish time on bea3818, Web-shell Visuals is green; Qwen Code CI (which runs Test (ubuntu-latest)) and the review-bot pass are still in_progress — I state this as observed and do not poll. Approving on maintainer judgment, consistent with my prior pass at ab48b21 which was on the same footing. This agrees with the independent re-review already on this head, which found both prior blockers fixed with mutation-pinned tests and no other reservations. / 发布时刻 Web-shell Visuals 已绿，Qwen Code CI（含 Test ubuntu）与机器人审查仍 in_progress；如实记录、不轮询。凭 maintainer 判断放行，与上轮同基准；与本 head 上已有的独立复审一致。
Verdict / 结论
EN: APPROVE. Fifth pass overall, third over the current shape. The increment is a correct merge plus three well-tested fixes that each close a previously-raised concern; every historical Critical remains fixed at this head and the effective diff is unchanged. No blocking findings remain from my side. / 批准。总体第五轮、当前形态第三轮；增量是一次正确合并加三处有测试钉住的修复，各闭合一个此前提出的问题，历史 Critical 在本 head 仍为修复态，有效 diff 未变。我方无阻塞项。
中文：APPROVE。纯静态审查，未运行任何测试；建议在 Qwen Code CI 于本 head 转绿后合并。
…ce-hover-details # Conflicts: # packages/web-shell/client/components/sidebar/WebShellSidebar.tsx # packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx
E2E verification report @ head
|
|
Delta review addendum — head moved Verification (static, at head
Verdict: no Critical in the delta. ci-bot's standing CHANGES_REQUESTED (local-path-open Linux terminal findings) is on the old head 增量复审补充 — head 从
结论：增量无 Critical。 ci-bot 在旧 head 的 CHANGES_REQUESTED 仍是活跃门禁；当前 head 无任何 approval。 |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 44 passed · 0 failed · 44 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：44 通过 · 0 失败 · 44 总计 抖动门： Verification reportPR 10606 — feat(web-shell): declutter the workspace sidebar and add loopback open actions (follow-up round)Verdict: 中文摘要
Previous-finding status (follow-up round)
Carried-over measurements were re-run at the new head, not diffed against the old report: the previous head Delta since the previous roundCommit messages after the previously verified head:
Central claim + A/B table (real daemon builds, wire-level)Base arm = full
Base harness 8/8, head harness 25/25 (raw logs: Mutation matrix (witness
|
| # | Mutation | Suite | Result |
|---|---|---|---|
| ctrl | unmutated head | launcher (42) + route (11) + server probe test (1) + bootstrap mirror tests (4) | all green |
| M0 | drop path arg from xdg-open (positive control) |
launcher | killed: opens the path with xdg-open on Linux red with expected-vs-actual argv |
| M1 | resolveTrustedRuntime → resolveWorkspaceRuntimeFromParam |
route | killed: rejects an untrusted workspace without opening anything |
| M2 | default target flipped folder→terminal | route | killed: 5 red |
| M3 | remove assertPathExists from linux terminal path |
launcher | killed: rejects a deleted directory on Linux without spawning |
| M4 | remove workspace_local_open conditional capability entry |
server.test.ts | killed: forwards the local path open probe result to capabilities |
| M5 | remove SpawnHangError rethrow in wt.exe fallback (delta bea3818e) |
launcher | killed: does not fall back when a spawned wt.exe stalls and is killed by the hang guard |
| M6 | drop local-open toggles from the bootstrap envelope (delta b2fe0ab4) |
run-qwen-serve.test.ts | killed: mirrors the local path open probe on the bootstrap envelopes (available: true) (2 red) |
No survivors. M0 proves the harness can make the suites fail; M6's available: false parameterizations staying green under the mutation is the expected direction. Worktree verified clean after the replay.
Targeted gates
| Gate | Result |
|---|---|
packages/cli server.test.ts |
1168/1168 |
packages/cli serve suites (local-path-open, workspace-local-open, process-env-guard, run-qwen-serve) |
433/433 |
packages/web-shell sidebar + formatDateTime (21 files) |
382/382 |
packages/web-shell ChatEditor.test.tsx |
107/107 |
packages/sdk-typescript DaemonClient.test.ts |
406/406 |
integration-tests/cli/qwen-serve-routes.test.ts (real daemon, capability baseline incl. new tags) |
37/37 |
Playwright web-shell.workspace-overview.spec.ts (chromium) |
4/4 — including the polling-cadence test that failed environmentally in the previous round; it passes at the new head |
tsc --noEmit packages/cli + packages/web-shell |
clean |
| Docs counts vs registry | 149 registered / 43 conditional — exact match, re-measured from the compiled registry |
Findings (non-blocking)
No new findings this round. F1 and F2 from the previous round stand as non-blocking (status table above); F3 is fixed. No correction to the corrections: C1 (calendar-day vs "last 24 hours" prose) remains a description staleness only.
Not covered
- Per-commit attribution: the checkout is depth-2 (merge, base tip, PR head only) and the previously verified head
ab48b210is not present locally, so the delta commits could not be diffed or exercised individually; the aggregateHEAD^1..HEADdiff was verified and each delta area was probed by behavior (section above). - macOS/Windows launcher branches: container is Linux; those branches are covered only by the PR's mocked-spawn unit tests (the new wt-hang guard test is proven load-bearing by M5, but the win32/macOS spawn paths were not executed end-to-end).
- Playwright mobile/visuals projects not run; only chromium desktop for the workspace-overview spec.
- Repo-wide lint/format gates rely on the PR's own CI; targeted
tsc --noEmiton both affected packages is clean. - One harness-side repair during the round, for transparency: the first base-arm run produced 503-then-connection-refused instead of 404 because the base rebuild was missing
src/generated/git-commit.ts(produced bynpm run generate, which a per-package build skips). After running the base tree's own generator and an incremental rebuild, the base cells behaved as designed. This was a control-build issue, not PR or base behavior.
Methodology
Environment: node:22-bookworm CI verify container, merge-ref checkout (HEAD merge 6262a577, HEAD^1 base 18046830, HEAD^2 head e5398dce). Harness harness-local-open.mjs boots real daemon processes (node dist/index.js serve --port 0 --hostname 127.0.0.1 --token … --workspace …) with a temp HOME, fake xdg-open/gnome-terminal binaries that reproduce the real no-display exit semantics and log argv, and stripped session env (DISPLAY/SSH_*/SESSIONNAME) so the GUI-vs-headless probe is deterministic; the bootstrap-vs-runtime envelope phases are deterministic because qwen serve defers runtime startup until the first /health (fast-path.ts). Base arm: full packages/cli compile at HEAD^1 in tmp/base-tree (worktree removed after capture). Control purity: readlink -f from the base worktree shows the internal @qwen-code/* links resolve into the head tree; this is clean because git diff HEAD^1..HEAD -- packages/core is empty (byte-identical source), the serve code imports no SDK, and the lockfile delta is metadata-only churn (libc arrays, a moved qwen-live workspace entry) with no third-party version change; the base dist was additionally confirmed to lack both new modules. Mutation matrix: mutation-matrix.sh replays seven single-point source mutations with per-mutation vitest runs and git checkout restores, NO_COLOR=1 for parseable output. Raw logs in logs/ (gate-*.log, tsc-*.log, base-build outputs were superseded by the successful rebuild), harness logs harness-{head,base}-final.log, matrix log mutation-matrix.log. Assertion totals (44) = head wire 25 + base wire 8 + matrix mutants 7 + matrix controls 4.
Flakiness gate log
integration test, out of gate scope: integration-tests/cli/qwen-serve-routes.test.ts
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.workspace-overview.spec.ts
rounds=5 files=13 skipped=2
file packages/cli/src/serve/local-path-open.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/local-path-open.test.ts
file packages/cli/src/serve/process-env-guard.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/process-env-guard.test.ts
file packages/cli/src/serve/routes/workspace-local-open.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/workspace-local-open.test.ts
file packages/cli/src/serve/run-qwen-serve.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/run-qwen-serve.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/sdk-typescript/test/unit/DaemonClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClient.test.ts
file packages/web-shell/client/components/ChatEditor.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatEditor.test.tsx
file packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx
file packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WorkspaceDetailsTooltip.test.tsx
file packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WorkspaceMenu.test.tsx
file packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WorkspaceSection.test.tsx
file packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/workspaceOverviewModel.test.ts
file packages/web-shell/client/utils/formatDateTime.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/formatDateTime.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/local-path-open.test.ts: PPPPP
packages/cli/src/serve/process-env-guard.test.ts: PPPPP
packages/cli/src/serve/routes/workspace-local-open.test.ts: PPPPP
packages/cli/src/serve/run-qwen-serve.test.ts: PPPPP
packages/cli/src/serve/server.test.ts: PPPPP
packages/sdk-typescript/test/unit/DaemonClient.test.ts: PPPPP
packages/web-shell/client/components/ChatEditor.test.tsx: PPPPP
packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: PPPP
packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: PPPP
packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: PPPP
packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: PPPP
packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: PPPP
packages/web-shell/client/utils/formatDateTime.test.ts: PPPP
verdict: timeout
summary: only 4 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/local-path-open.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/workspace-local-open.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/formatDateTime.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/local-path-open.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/routes/workspace-local-open.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 2 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: P (exit 0)
round 2 · packages/web-shell/client/utils/formatDateTime.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/local-path-open.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/routes/workspace-local-open.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 3 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: P (exit 0)
round 3 · packages/web-shell/client/utils/formatDateTime.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/local-path-open.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/routes/workspace-local-open.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 4 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WorkspaceDetailsTooltip.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WorkspaceMenu.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WorkspaceSection.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/workspaceOverviewModel.test.ts: P (exit 0)
round 4 · packages/web-shell/client/utils/formatDateTime.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/local-path-open.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/process-env-guard.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/routes/workspace-local-open.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/run-qwen-serve.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 5 · packages/web-shell/client/components/ChatEditor.test.tsx: P (exit 0)
Evidence images
1 additional image(s) did not pass the hosting checks (PNG magic, unique sanitized name, ≤2 MB, max 8) and remain in the run artifacts.
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
APPROVE — third pass at the new head e5398dce
Re-reviewed from scratch against e5398dcec84af3c243176fb0fecc0a0f113c3d28(上次评审 head 为 bea3818e;其间两次合入 main + 两个分支自有提交 b2fe0ab4/7e7e2508)。
历史阻塞项复核(在当前精确 head 逐锚读码,不采信线程状态)
- 上轮两修复仍在场:
SpawnHangError挂起类区分与instanceof SpawnHangError重抛(wt.exe 挂起不再双开回退)、三处assertPathExists、spawn 时DISPLAY/WAYLAND_DISPLAY门与共享linuxTerminalCandidates、菜单焦点卡死修复(WebShellSidebar.tsx:6132 工作区菜单onCloseAutoFocuspreventDefault)及formatDateTime日历日判定——与bea3818e上逐行验证过的实现一致,两次 main 合并未扰动接线(有效 diff 仍是同一文件集)。 - 更早的三个 Critical(Linux 删目录假成功、显示门、纯 Wayland xterm)与各行为修复锚点全部复验在场。
新面扫描(本 head 独有增量)
b2fe0ab4:bootstrap 包络补齐五个 runtime-only 标签(acpHttpEnabled走resolveAcpHttpEnabled(env)与运行时同源,其余四项与生产接线"始终注册"的事实对齐),并有集成断言(+5)。启动期多报的标签在运行时必然服务,消除的是 under-report 分歧;未发现可证明的阻塞缺陷。7e7e2508:仅测试文件内重复声明清理。
CI(一句话,不构成门禁):本 head 21 success / 8 path-skip / 1 in-progress(review-pr)/ 1 cancelled(web-shell E2E Smoke,无失败证据指向本 PR 引入的阻塞性缺陷)。
✅ APPROVE — 三轮评审至此:全部历史阻塞项已在当前 head 核实修复,当前扫描无新 Critical,批准不再新增门禁。
— 衍星 · read-only PR review (posted as qqqys)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.23.0. |
5 participants
What this PR does
Reworks the sidebar's workspace area in the Web Shell and adds host-local open actions for loopback deployments.
The expanded workspace row no longer spends its width on a path line and a block of facet chips (MCP / Skills / Extensions / Channels / Context) that were usually all zeros. The row now shows just the workspace name with the git chip directly after it, and the session list gains a subtle tree indent guide. Everything that was removed — full path, git branch, session counts, and the facet counts — moved into a hover popover on the workspace header, where known facets show even when their count is zero and facets with an issue (e.g. a failed MCP server) are highlighted. The hover action buttons overlay the row instead of reserving permanent space, and the name is capped by the rendered action count so the interactive git chip is never covered. Session rows pick up a pulsing green dot for running sessions and an accent bar marking the current session; the precise session date (
yyyy-MM-dd) lives in the session's own hover popover instead of crowding the row.When the browser reaches the daemon over loopback, the popover's path row and the workspace menu offer "Open folder" and "Open terminal". The daemon opens the resolved, registered workspace path — never a client-supplied one — in the host's file manager (Finder / Explorer / xdg-open) or a new terminal window (Terminal.app / Windows Terminal with cmd fallback / gnome-terminal / konsole / xterm). Availability is advertised through two new capability tags so headless hosts and remote (LAN-paired) clients never see the buttons.
Why it's needed
Workspace names in the sidebar were truncated to a handful of characters because hidden hover actions reserved space and five usually-zero facet chips consumed three rows of height. The diagnostic information is useful, but it belongs on demand, not permanently in the layout — and in the popover it can finally be complete, zeros included. Separately, opening the project folder or a terminal at a workspace was impossible from the web UI even when browser and daemon sit on the same machine (the codebase explicitly called this unsupported); on loopback it is the most natural action in the world.
Reviewer Test Plan
How to verify
Open the Web Shell against a local daemon with at least one git workspace that has sessions. Confirm: the workspace row shows the full name with the git chip right after it and no counts; session rows stay text-only with a pulsing green dot for running sessions, and hovering a session shows its timestamp in the popover (HH:mm:ss within the last 24 hours, yyyy-MM-dd beyond); hovering the workspace row opens a popover with the full path, branch, a Sessions row (running/attention/total), and every known facet including zeros; the path row's folder and terminal buttons open Finder/Terminal at the workspace path; the same two entries appear in the workspace menu after "Copy path". Accessing the same daemon over a LAN address (or running it headless) must hide both buttons. Facet fetching is unchanged: workspaces are polled only while expanded, and collapsed rows retain their last snapshot.
Automated coverage: daemon unit tests for the launcher and route (
packages/cli/src/serve/local-path-open.test.ts,routes/workspace-local-open.test.ts), capability toggle tests, an SDK client test, 319 web-shell sidebar unit tests including the new popover suite, and theweb-shell.workspace-overviewe2e spec which now drives the hover popover and the loopback open actions end to end.Evidence (Before & After)
Before — the workspace name is cut to
qwen-we...under five always-zero facet chips, and hidden hover actions still reserve space:After — full workspace name with the git chip right after it, no counts in the row, and session rows back to text-only; on hover the action buttons overlay the row without ever covering the chip:
Workspace hover popover — full path with the folder and terminal buttons, branch, Sessions row, and every known facet including zeros:
Session hover popover — the precise timestamp lives here (
HH:mm:sswithin the last 24 hours,yyyy-MM-ddbeyond) instead of in the row:Tested on
Windows and Linux launch commands are unit-tested with mocked spawns but were not manually verified on those platforms.
Environment (optional)
Local dev server with the Playwright mock daemon; daemon-side launch verified on macOS (Finder + Terminal.app).
Risk & Scope
Linked Issues
中文说明
本 PR 做了什么
重做 Web Shell 侧栏的工作区区域，并为 loopback 部署场景添加本地打开操作。
展开的工作区行不再把宽度浪费在路径行和一排通常为 0 的 facet 芯片（MCP / Skills / Extensions / Channels / Context）上。现在该行只显示工作区名，git 图标紧跟其后，会话列表增加了细长的树形缩进引导线。被移除的所有信息——完整路径、git 分支、会话计数和 facet 计数——都移入了工作区标题的悬浮弹层：已知的 facet 即使计数为 0 也会展示，有异常的 facet（如 MCP 服务器失败）会以警告色标出。hover 操作按钮改为浮层覆盖而非预留常驻空间，并按实际渲染的按钮数量限制名字宽度，保证可点击的 git 图标永不被遮挡。会话行新增运行中会话的绿色脉冲圆点和当前会话的左侧高亮条；精确的会话时间戳（24 小时内为
HH:mm:ss，更早为yyyy-MM-dd）不再占用行内空间，改为在会话自身的悬浮弹层中展示。当浏览器通过 loopback 访问 daemon 时，弹层的路径行和工作区菜单会提供"打开文件夹"和"打开终端"。daemon 打开的是解析后的、已注册的工作区路径——绝不接受客户端任意路径——在主机的文件管理器（Finder / Explorer / xdg-open）或新的终端窗口（Terminal.app / Windows Terminal（带 cmd 回退）/ gnome-terminal / konsole / xterm）中打开。可用性通过两个新的 capability 标签宣告，因此 headless 主机和远程（LAN 配对）客户端永远不会看到这些按钮。
为什么需要
侧栏中的工作区名之前被截断到只剩几个字符，因为隐藏的 hover 操作按钮预留了空间，而五个通常为 0 的 facet 芯片又占了三行高度。这些诊断信息很有用，但应该按需出现而不是常驻布局——而且在弹层里它终于可以完整展示（包括零值）。另外，即使浏览器和 daemon 在同一台机器上，之前也无法从 Web 界面打开项目文件夹或终端（代码中明确标注为不支持）；而在 loopback 场景下，这是最自然不过的操作。
评审测试计划
如何验证
对着一个本地 daemon 打开 Web Shell，至少有一个带会话的 git 工作区。确认：工作区行显示完整名称，git 图标紧跟其后，没有计数；会话行保持纯文本，运行中的会话有绿色脉冲点，悬浮会话行可在弹层中看到时间戳（24 小时内为 HH:mm:ss，更早为 yyyy-MM-dd）；悬浮工作区行会弹出包含完整路径、分支、Sessions 行（运行中/待处理/总数）以及全部已知 facet（含零值）的弹层；路径行的文件夹和终端按钮会在工作区路径打开 Finder/终端；工作区菜单中"复制路径"之后也有这两项。通过 LAN 地址访问同一个 daemon（或 headless 运行）时两个按钮必须隐藏。facet 拉取语义不变：仅在展开时轮询，折叠的行保留最近快照。
自动化覆盖：启动器与路由的 daemon 单测（
packages/cli/src/serve/local-path-open.test.ts、routes/workspace-local-open.test.ts）、capability 开关测试、SDK 客户端测试、319 个 web-shell 侧栏单测（含新弹层套件），以及端到端驱动悬浮弹层和 loopback 打开操作的web-shell.workspace-overviewe2e 用例。证据（前后对比）
修改前——工作区名被五个恒为 0 的 facet 芯片挤成
qwen-we...，隐藏的 hover 操作按钮仍预留空间：修改后——工作区名完整显示，git 图标紧跟其后，行内没有计数，会话行恢复纯文本；hover 时操作按钮浮于行上且永不遮挡图标：
工作区悬浮弹层——完整路径（带文件夹和终端按钮）、分支、Sessions 行、以及全部已知 facet（含零值）：
会话悬浮弹层——精确时间戳在这里展示（24 小时内为
HH:mm:ss，更早为yyyy-MM-dd），不再占用行内空间：测试平台
macOS ✅（单测 + e2e）；Windows / Linux⚠️ 未实测（启动命令以 mock spawn 的单测覆盖）。
风险与范围