feat(web-shell): improve sidebar session management - #9311
Merged
Conversation
…details' into codex/web-shell-sidebar-session-details
- sdk: route workspace session metadata PATCH through direct REST - collapsed switcher: cancel stale hover-close timers on reopen, keep the surface open while the group picker or keyboard focus is inside it, and emit the missing close signal when a tracked menu unmounts open - suppress the session menu's close focus restore when it started a rename - migrate the primary workspace expansion preference across the provisional-to-resolved cwd key change - honor a persisted workspace collapse over stale one-shot auto-expansion - drop the inert archived-row tab stop - align the constrained smoke test with the single-line details title
) - collapsed switcher: resolve pointer targets through composedPath and make the close timer's focus guard shadow-DOM aware so hover-open containment works in shadowDom portal mode - reset search state when the sidebar collapses so the autofocused search input no longer mounts inside the hover popover and steals keyboard focus - rename: propagate the daemon-resolved displayName (clamped to 256) instead of the locally typed string and cap both rename inputs at 256 characters - keep the session list scrollable clear of the fixed footer so rows stay hoverable, and close the details popover before each constrained re-hover in the smoke test - projects section: write the expansion preference outside the state updater, never lock hideProjectHeader consumers behind a stored collapse, and reset the one-shot show-all per session source and primary workspace - stop a double-click inside a mounted rename input from restarting the rename and discarding the typed text
…9122) - serve: reject empty/whitespace displayName on the workspace metadata route so archived sessions never persist an empty custom_title record - serve: advertise workspace_session_metadata in the integration capability baseline to match the registry and unit baselines - sidebar: end the session scroll port above the fixed footer so rows can never park under it and block hover (drops stale clearances) - sidebar: reset search state whenever the collapsed surface closes so a stale autofocused input cannot steal composer focus on hover-open - sidebar: keep keyboard-opened collapsed switcher in keyboard semantics; a pointer graze no longer suppresses focus restoration - sidebar: busy-guard the archived rename menu item, align the group-create icon with its siblings, and reset per-section show-all on session-source change to match the flat list Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…ar-session-details # Conflicts: # packages/web-shell/client/components/sidebar/WebShellSidebar.tsx
ytahdn
requested review from
LaZzyMan,
doudouOUC,
tanzhenxin and
yiliang114
as code owners
August 17, 2026 03:52
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "workspace_session_metadata" |
— Qwen Code · serve A/B
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
Collaborator
|
Thanks for the PR — this is a substantial sidebar rework, and the write-up makes the intent easy to follow. Template ✓ — all required sections present, bilingual, with a concrete reviewer test plan. Problem: observed, not theoretical. The collapsed sidebar genuinely removes the only session-switching surface, the cross-workspace race that can replace loaded content with a false "session not found" is a real defect, and rename/delete scoping inconsistencies across workspaces are visible in the current behavior. This also supersedes #9122 after reviewer discussion. For a UX feature of this kind, the design docs plus the superseded thread are reasonable evidence; no standalone reproduction is expected. Direction: aligned. Session management in narrow surfaces is an area the reference agent actively invests in (CHANGELOG shows sidebar session groups and a session picker gaining git branch + message count), and the Web Shell sidebar is core qwen-code product surface. Size: this is a Approach: the central call is right — reusing the same complete session browser for expanded and collapsed states instead of building a second switcher. The two design docs are concise, and the rename path goes through a new capability-gated workspace-qualified route rather than bending the session-scoped one. My one question is whether the tooltip/popover arrow redesign needed to ride along — it affects every web-shell tooltip, not just the sidebar — but it's small and test-covered, so not a blocker. Risk: no matches against the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献——这是一次相当规模的侧边栏重构，PR 描述把意图讲得很清楚。 模板 ✓ —— 所有必需章节齐全，中英双语，并附有具体的验证步骤。 问题： 已观测到的问题，不是理论性加固。侧边栏收起后确实失去了唯一的会话切换入口；跨工作区切换可能发出相互竞争的加载请求、把已加载内容替换成错误的"会话不存在"；重命名/删除在不同工作区之间的作用域不一致也是当前可见的行为。本 PR 同时是在 #9122 讨论之后重新提交的。此类 UX 改进以设计文档加前序讨论作为证据即可，不要求单独的复现。 方向： 对齐。参考产品的 CHANGELOG 显示其正在持续投入窄界面下的会话管理（侧边栏会话分组、会话选择器增加 Git 分支和消息数），Web Shell 侧边栏属于 qwen-code 的核心产品界面。 规模： 这是 方案： 核心选择是正确的——展开态与收起态复用同一个完整会话浏览器，而不是再造一个切换器。两份设计文档简洁，重命名走新的能力门控的按工作区路由，而不是改造会话级路由。唯一的疑问是 tooltip/popover 箭头重设计是否必须随行——它影响所有 web-shell tooltip 而不仅是侧边栏——但改动小且有测试覆盖，不构成阻塞。 风险： 未命中与 revert 相关的高风险路径。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewReviewed statically at the head commit (no PR code executed). The architecture holds together well:
Non-blocking observations for the maintainer:
sequenceDiagram
participant P1 as WebShellSidebar
participant P2 as WorkspaceDaemonClient
participant P3 as serve metadata route
participant P4 as Runtime bridge
participant P5 as SessionService
P1->>P2: updateSessionMetadata (sessionId, displayName)
P2->>P3: PATCH workspace session metadata
P3->>P4: live session rename via bridge
P3->>P5: persisted or archived renameSession
P5-->>P3: custom_title record appended
P3-->>P1: sessionId and effective displayName
Files changed (30 of 37 shown)
Test evidenceFrom the PR's own CI on the reviewed commit (no PR code was built or executed in this review). Every workflow run that actually ran is green; the macOS/Windows unit matrix and CLI integration jobs were skipped by workflow gating, which is not a failure signal. The serve A/B bot run on this commit independently reports the only serve-side difference is the new Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The two check-runs still open on this commit are bot orchestration jobs ( Sandboxed verification would settle the remaining behavioral claims: 中文说明代码审查在 head 提交上做了静态审查（未执行任何 PR 代码）。整体架构是自洽的：
供维护者参考的非阻塞观察：
测试证据来自该 PR 自身的 CI（本次审查未构建或执行任何 PR 代码）。所有实际运行的工作流均为绿色；macOS/Windows 单测矩阵和 CLI 集成任务被工作流门控跳过，不属于失败信号。serve A/B 机器人在本提交上的独立运行显示 serve 侧唯一差异是新增 本提交上仍未结束的两个 check-run 是机器人编排任务（ 沙箱验证可以落实剩余的行为性结论： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 3/5 — clean review across every stage, but this Stepping back: this is genuinely good work. My independent take before reading the diff would have been exactly this shape — reuse the one existing session browser for the collapsed state rather than building a flyout copy, and add a capability-gated workspace-qualified rename route rather than bending the session-scoped one. The PR matches that and then goes further (status dot on the collapsed icon, five-item previews, persisted expansion), and each addition carries its own tests. The parts that are actually hard — the pointer/focus state machine around the collapsed popover — are the parts with the deepest test coverage, including the ugly cases (stale timers, unmounting menus, StrictMode replay). CI is fully green on the reviewed commit, and the serve A/B shows no unexpected daemon-side drift. The reservations are small but real: the hover-only details panel drops the keyboard path to session-ID copying (a documented choice, still an a11y regression), the arrow redesign touches every tooltip in the package, and manual verification happened on macOS only. None of these block on their own. I am not approving because Stage 0 escalation policy requires maintainer awareness for a core-touching PR of this size — not because the review turned up a defect. A maintainer should make the final call on the scope and the a11y trade-off. ⏸️ Deferring to @wenshao — code owner for 中文说明置信度：3/5 —— 各阶段审查都很干净，但本 整体来看：这是一份质量很高的工作。在读 diff 之前，我的独立方案与之完全同形——收起态复用现有唯一的会话浏览器而不是再造一个弹出副本；新增能力门控的按工作区重命名路由而不是改造会话级路由。PR 与此一致并更进一步（收起图标上的状态点、五条预览、展开状态持久化），且每项都有各自的测试。真正难的部分——收起弹窗周围的指针/焦点状态机——恰恰是测试覆盖最深的部分，包括棘手的场景（过期定时器、菜单卸载、StrictMode 重放）。CI 在被审提交上全绿，serve A/B 未发现守护进程侧的意外漂移。 保留意见不大但真实存在：仅悬浮触发的详情面板失去了键盘可达的会话 ID 复制路径（设计文档有记录，但仍是无障碍回退）；箭头重设计影响包内所有 tooltip；手动验证仅在 macOS 上进行。这些单独都不构成阻塞。 不予批准的原因是 Stage 0 升级策略：触及核心且规模达到此量级的 PR 需要维护者知悉——而不是审查发现了缺陷。最终决定应由维护者作出，重点是范围与无障碍取舍。 ⏸️ 转交 @wenshao —— — Qwen Code · qwen3.8-max Reviewed at |
chiga0
approved these changes
Aug 17, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #9311 feat(web-shell): improve sidebar session management
Type: New Feature (UI) + Backend API + Bug Fix
Change size: +4765 / -1913 across 37 files
HEAD: c706aa5
Supersedes: #9122
Findings Summary
- Critical / Major: 0
- Minor: 1 item (accessibility — deliberate design tradeoff)
- Nit: 1 item
Verification of #9122 Standing Blockers
我对照 #9122 中六个 Critical standing blocker 逐一核实了 HEAD 代码，结论如下：
| #9122 Finding | 状态 | 验证依据 |
|---|---|---|
'unavailable' 分支缺失 (session.ts) |
✅ FIXED | if (liveOwner.kind === 'unavailable') { sendWorkspaceRuntimeUnavailable(res); return; } 已在锁内正确处理 |
| TOCTOU pre-lock probe (R12-1) | ✅ FIXED | 改为 runExclusiveMany（独占锁），live-owner 检查移入锁内，captureRuntimeGenerationAssertion + assertRuntimeGenerationOpen?.() 在操作全程多次校验 generation |
| 缺失 generation guard | ✅ FIXED | assertRuntimeGenerationOpen?.() 在 bridge.updateSessionMetadata、getSessionLocation、renameSession 后均有调用，与兄弟路径一致 |
| CancelOnUnmount 跨实例 stale cancel (R12-2) | ✅ FIXED | CancelOnUnmount 组件已完全删除（grep 确认 0 occurrences），提交信息 "remove unsafe rename unmount cleanup" |
| Same-session rename retry (R11-2) | ✅ FIXED | busySessionIdsRef.current.has(sessionIdentity) 守卫在 rename 飞行期间阻止同 session 重复确认；settle 回调中 identity 检查现在只会匹配 已完成 的操作，不会误取消后续编辑 |
| 无键盘路径访问 session 详情 (R11-16) | SessionDetailsSubmenu + keyboard test 仍被删除，SessionDetailsTooltip 仍为 pointer-only。见下方 Minor 分析 |
New Fix Verification
- ✅ DaemonSessionProvider 竞争 load 请求:
sessionEffectWorkspaceCwd = restoreWorkspaceCwd ?? workspaceCwd作为 session effect 依赖，跨工作区切换进行中时固定使用restoreWorkspaceCwd，防止workspaceCwd变化触发第二次 load 请求，避免已成功加载的内容被 "session not found" 覆盖。变更最小（2 行），逻辑正确。 - ✅ 收起侧边栏状态指示器:
collapsedSessionStatusDot+collapsedSessionStatus逻辑正确区分approval/question/running 状态优先级，与展开侧边栏颜色一致。
Additional Audit Coverage
- session.ts:
runExclusiveMany合理性: 独占锁比原来的runSharedMany更保守；对交互式 rename 操作，持锁期间不会有感知延迟，是正确的安全升级 ✓ - session.ts:
effective = { displayName: displayName || undefined }:displayName已经过非空校验（trim() !== ''），|| undefined是死代码，但不影响正确性（见 Nit） - sessionService.ts
archiveState参数: 默认值'active'保持向后兼容，所有现有调用者不受影响 ✓ - capability baseline 同步:
capabilities.ts、server.test.ts、DaemonClient.test.ts、qwen-serve-routes.test.ts均已同步添加workspace_session_metadata✓ - workspaceExpansion.ts: 与 #9122 一致，默认返回
true（展开）的行为变更已在 #9122 review 中记录，本 PR 保持该设计 ✓
Minor — Accessibility Design Tradeoff
SessionDetailsTooltip 仍为 pointer-only（onPointerEnter 触发，无 onFocus，copy button tabIndex={-1}）。SessionDetailsSubmenu.keyboard.test.tsx（176 行键盘访问测试）随旧组件一起删除。
这是一个已知的 WCAG 2.1.1 取舍，设计文档 docs/design/2026-08-13-web-shell-sidebar-session-details.md 已收录该设计。如果 maintainer 接受指针专用设计，建议在设计文档中明确标注 "keyboard path intentionally not provided"，以便后续贡献者了解该决策背景。
Final Verdict
APPROVE — #9122 的所有六个 Critical blocker 均已解决（3 个后端竞态、CancelOnUnmount、同 session rename 竞态、DaemonSessionProvider 双重 load），整体实现质量显著提升。唯一剩余的 accessibility concern 是已记录的产品设计决策，不构成合并阻塞。
This review was generated by QoderWork AI
yiliang114
approved these changes
Aug 17, 2026
yiliang114
approved these changes
Aug 17, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the full diff (37 files) with focus on the new serve surface and the cross-workspace load path.
Verified:
- New
PATCH /workspaces/:workspace/session/:id/metadataroute goes through the same runtime/client gates as its siblings; capability gating (workspaceSessionMetadataEnabled) is real and covered by tests; rename promise races are guarded viaeditingSessionIdentityRef. - Cross-workspace fix: the session-connect effect now depends on
restoreWorkspaceCwd ?? workspaceCwd, so a controlled workspace update after an imperative switch no longer triggers a second competing load. - Expansion persistence:
migrateWorkspaceExpansionPreferencere-syncs state (setProjectExpanded(readWorkspaceExpanded(...))) in the same effect, so provisional→registered id migration cannot desync UI from storage. - Security pass found no authorization gap, XSS surface, or data exposure; session titles render as text, not HTML.
- Checks green on this head: unit tests, web-shell E2E smoke, real daemon E2E, Serve A/B.
One non-blocking nit: rawDisplayName.slice(0, 256) operates on UTF-16 code units, so a supplementary-plane character straddling offset 256 leaves a lone surrogate that passes the control-char validation and renders as U+FFFD. Consider truncating by code points (Array.from(rawDisplayName).slice(0, 256).join('')) in a follow-up.
LGTM.
Collaborator
|
Released in v0.21.14. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR improves session navigation and management in the Web Shell sidebar. Session rows now expose consistent hover details, compact status indicators, predictable title scrolling, workspace-scoped actions, and five-item previews with explicit expansion. When the sidebar is collapsed, the project icon opens the same complete project and session browser used by the expanded sidebar, stays open while sessions and menus are operated, and closes after the pointer leaves or through normal outside dismissal. The collapsed icon also surfaces pending approval, question, and completion states with the same colors as the expanded sidebar.
It also makes workspace expansion persistent, restores session ID copying in the details panel, keeps rename and delete actions correctly scoped to their owning workspace, and prevents a controlled workspace update from issuing a second load after an imperative cross-workspace session switch.
Why it's needed
Previously, collapsing the sidebar removed the only way to switch sessions. Cross-workspace navigation could additionally issue competing load requests and replace successfully loaded content with a false “session not found” state. Session details and actions were inconsistent across primary and secondary workspaces, and transient interaction states could dismiss or visually collapse the browser while the user was still working in it.
This supersedes #9122 so the final implementation can be reviewed in a clean discussion thread.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: the collapsed sidebar had no session switcher; cross-workspace switching could emit duplicate load requests; session details and actions differed by workspace.
After: the collapsed project icon provides the complete session browser, interaction remains stable during session operations, status attention is visible while collapsed, and each session operation stays scoped to its owning workspace.
Tested on
Environment (optional)
Local Web Shell development runtime and browser verification. Relevant Web Shell tests passed (220/220), daemon session provider tests passed (216/216), and package type checks passed.
Risk & Scope
Linked Issues
Supersedes #9122.
中文说明
本 PR 做了什么
本 PR 优化了 Web Shell 侧边栏中的会话导航与管理。会话行现在提供一致的悬浮详情、紧凑的状态标识、可预测的标题滚动、按工作区归属执行的操作，以及最多五条的预览与明确的展开入口。侧边栏收起时，项目图标会打开与展开状态共用的完整项目和会话浏览器；切换会话和操作菜单时面板保持打开，鼠标移出后延迟关闭，也支持正常的外部点击关闭。收起图标还会使用与展开侧边栏一致的颜色提示待审批、提问和完成状态。
本 PR 还会持久化工作区展开状态，恢复详情面板中的会话 ID 复制能力，确保重命名和删除按会话所属工作区执行，并避免跨工作区主动切换会话后，受控工作区属性更新再次发起加载请求。
为什么需要
此前侧边栏收起后无法切换会话；跨工作区导航还可能发出相互竞争的加载请求，把已经成功加载的内容替换成错误的“会话不存在”状态。主工作区与非主工作区的会话详情和操作不一致，部分瞬时交互状态也会在用户仍在操作时关闭或压缩浏览面板。
本 PR 替代 #9122，以便在干净的讨论线程中审查最终实现。
Reviewer Test Plan
如何验证
证据（修改前后）
修改前：侧边栏收起后没有会话切换入口；跨工作区切换可能发出重复加载请求；不同工作区的会话详情和操作不一致。
修改后：收起状态的项目图标提供完整会话浏览器，切换和操作会话期间交互保持稳定，收起时也能看到需要注意的状态，并且每项会话操作都按其所属工作区执行。
测试环境
环境（可选）
本地 Web Shell 开发运行时及浏览器验证。相关 Web Shell 测试通过（220/220），守护进程会话 Provider 测试通过（216/216），包级类型检查通过。
风险与范围
关联问题
替代 #9122。