Skip to content

feat(web-shell): add Session Overview panel and in-window split view - #6400

Merged
wenshao merged 11 commits into
QwenLM:mainfrom
wenshao:feat/web-shell-session-overview-split
Jul 7, 2026
Merged

feat(web-shell): add Session Overview panel and in-window split view#6400
wenshao merged 11 commits into
QwenLM:mainfrom
wenshao:feat/web-shell-session-overview-split

Conversation

@wenshao

@wenshao wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Adds two related web-shell surfaces for working with several daemon sessions at once: a large-screen Session Overview panel (mission control) and an in-window Split View.

The Session Overview lists every workspace session as a live card, ranked needs-approval → running → idle. It merges the cheap session list with the detail=full status report to surface the per-session "needs approval" signal, current model, and client count. From the multi-select you can open the checked sessions as a split view either in the current tab (Open in split) or in a new browser tab (Open in new tab, which navigates to a ?split=a,b URL that the app consumes on load).

The Split View renders N independent chat panes side by side — one DaemonSessionProvider per pane under the single shared DaemonWorkspaceProvider. Each pane is a self-contained interactive chat (transcript, composer, streaming status, tool/AskUserQuestion approvals) scoped to its own session. Because each pane is its own provider subtree, the browser scopes keyboard focus to whichever pane you click into, so panes never contend over which session an approval or Enter belongs to. Entry points (the sidebar Session Overview / Split View buttons) are gated to large screens, and the split view's Back button returns to the Session Overview.

Why it's needed

Power users often drive multiple agent sessions in parallel (e.g. a refactor in one, tests in another, CI triage in a third). Today the web shell shows one session at a time, so keeping tabs on several means constant switching. The overview gives a single place to see which sessions are running or waiting for approval; the split view lets you read and steer 2+ sessions at once on one screen — or pop them into a dedicated tab.

Reviewer Test Plan

How to verify

  1. Build the web shell and start the daemon with it: npm run build --workspace=@qwen-code/web-shell then node packages/cli/dist/index.js serve --web (open in a wide browser window).
  2. Create a few sessions (send a prompt, click New chat, repeat).
  3. Sidebar → the grid icon opens Session Overview: one card per session, ranked with needs-approval / running first, showing model + client count + relative time.
  4. Select 2+ sessions → Open in split: the current tab shows those sessions side by side, each with its own transcript + composer; typing in one pane reaches only that session.
  5. Select 2+ → Open in new tab: a new browser tab opens at /?split=… showing the same split; the ?split= param is stripped after load.
  6. In the split, Back returns to the Session Overview.

Automated: npm run test --workspace=@qwen-code/web-shell — 1098 passing, including new suites for the overview (ranking + open-in-tab/split), split view (N panes + add/close), chat pane (send / approval / close wiring), the split URL helpers, and the large-screen hook.

Evidence (Before & After)

Before: N/A — these are new surfaces.

After — Session Overview (mission control): ranked live cards, multi-select, Open in new tab / Open in split.

Session Overview

After — Split View opened in a new tab: three sessions side by side, each its own interactive chat.

Split View

Both were captured against a real node packages/cli/dist/index.js serve --web daemon. An end-to-end run additionally confirmed per-pane routing (a prompt typed in one pane reaches only that session — verified via two distinct POST /session/<id>/prompt in the daemon log and no cross-contamination between panes) and that Open in new tab opens a ?split= tab that loads the split with the selected sessions.

Tested on

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

Environment (optional)

node packages/cli/dist/index.js serve --web (loopback) + Playwright (Chrome) for the real-UI capture; Vitest (jsdom) for unit/integration.

Risk & Scope

  • Main risk or tradeoff: each split pane opens its own SSE stream to its session. Panes reuse the existing per-session provider (all state is per-instance, no shared module singletons), so N panes = N independent streams; capped at 6 panes. When the current session is also shown as a pane it is attached twice (main view + pane) — harmless, just an extra stream for that one session.
  • Not validated / out of scope: touch / small-screen split (entry points are large-screen only by design); split panes intentionally use a minimal composer — advanced features (slash commands, @-mentions, model picker) remain in the single-session view.
  • Breaking changes / migration notes: none. Purely additive; the existing single-session behavior is unchanged.

Linked Issues

中文说明

这个 PR 做了什么

为 web-shell 新增两个协作管理多个 daemon 会话的界面:大屏专用的 会话总览(Session Overview) 面板(mission control)和窗口内 分屏(Split View)

会话总览把每个工作区会话渲染成一张 live 卡片,按 待审批 → 运行中 → 空闲 排序。它把便宜的会话列表与 detail=full 状态报告合并,得到每个会话的"待审批"信号、当前模型、客户端数。通过多选,可以把勾选的会话作为分屏打开:要么在当前标签页(Open in split)、要么在新浏览器标签页(Open in new tab,跳转到 ?split=a,b URL,应用加载时消费该参数)。

分屏把 N 个独立聊天窗格并排渲染——共享同一个 DaemonWorkspaceProvider,每个窗格挂一个自己的 DaemonSessionProvider。每个窗格是自包含的可交互聊天(消息、composer、流式状态、工具/AskUserQuestion 审批),作用域是它自己的会话。因为每个窗格是独立的 provider 子树,浏览器把键盘焦点作用到你点中的那个窗格,窗格之间不会争"回车/审批属于哪个会话"。入口(侧栏的会话总览 / 分屏按钮)仅在大屏显示;分屏的"返回"回到会话总览。

为什么需要

重度用户常常并行推进多个 agent 会话(比如一个重构、一个写测试、一个查 CI)。现在 web-shell 一次只显示一个会话,盯多个就得反复切换。总览给了一个统一的地方看哪些会话在跑、哪些在等审批;分屏让你在一块屏上同时读和驱动 2+ 个会话——或把它们弹到独立标签页。

复现步骤

  1. 构建 web-shell 并用它起 daemon:npm run build --workspace=@qwen-code/web-shell,然后 node packages/cli/dist/index.js serve --web(用宽的浏览器窗口打开)。
  2. 建几个会话(发一条 prompt、点 New chat、重复)。
  3. 侧栏 → 网格图标打开 会话总览:每个会话一张卡片,待审批/运行中排前,显示模型 + 客户端数 + 相对时间。
  4. 选 2+ 个会话 → Open in split:当前标签页把这些会话并排显示,各有自己的消息 + composer;在一个窗格打字只到那个会话。
  5. 选 2+ → Open in new tab:开一个新标签页 /?split=… 显示同样的分屏;?split= 参数加载后被清除。
  6. 分屏里 Back(返回) 回到会话总览。

自动化:npm run test --workspace=@qwen-code/web-shell — 1098 个通过,含总览(排序 + 开标签页/分屏)、分屏(N 窗格 + 加/关)、chat pane(发送/审批/关闭接线)、split URL 工具、大屏 hook 的新测试套件。

证据(Before & After)

Before:N/A —— 都是新界面。截图见上(英文部分):会话总览(排序卡片、多选、Open in new tab / Open in split);分屏在新标签页打开(三个会话并排,各自独立聊天)。均在真实 serve --web daemon 上截取。端到端跑还验证了 per-pane 路由(一个窗格打字只到那个会话——daemon 日志里两个不同 POST /session/<id>/prompt,窗格间不串扰),以及 Open in new tab 会开出一个 ?split= 标签页加载选中会话的分屏。

风险与范围

  • 主要风险/取舍:每个分屏窗格对自己的会话开一条 SSE。窗格复用既有的 per-session provider(全部状态每实例、无共享模块单例),所以 N 窗格 = N 条独立流;上限 6 个窗格。当前会话若同时作为窗格显示,会被附着两次(主视图 + 窗格)——无害,只是那一个会话多一条流。
  • 未验证/范围外:触屏/小屏分屏(入口设计上仅大屏);分屏窗格刻意用精简 composer——高级功能(斜杠命令、@ 提及、模型选择器)仍在单会话视图。
  • 破坏性变更/迁移:无。纯增量;既有单会话行为不变。

Add a large-screen "Session Overview" mission-control panel and an
in-window split view so users can monitor and drive multiple daemon
sessions at once.

- SessionOverviewPanel: ranked live cards (needs-approval -> running ->
  idle) merging the workspace session list with the detail=full status
  report. Multi-select opens the selected sessions as a split view in
  the current tab ("Open in split") or in a new browser tab ("Open in
  new tab", via a ?split=a,b URL).
- SplitView + ChatPane: one DaemonWorkspaceProvider hosting N
  DaemonSessionProvider panes, each a self-contained interactive chat
  (transcript, composer, streaming, tool/ask approvals). Browser focus
  scopes the keyboard per pane, so panes never contend over approvals.
- Sidebar entry points gated to large screens; the split view's Back
  returns to the Session Overview.
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual, with screenshots and a detailed reviewer test plan.

Problem: Real, well-defined UX gap. Power users driving multiple parallel daemon sessions have no way to see all sessions at once or interact with 2+ simultaneously. The current web-shell shows one session at a time, requiring constant switching. This is an observed workflow friction, not theoretical hardening.

Direction: Aligned. Multi-session management is a natural evolution for the daemon web-shell — the project already has a roadmap/session-management label, and competing tools (VS Code split editor, browser DevTools) offer similar surfaces. The two complementary views (overview as mission control, split for active steering) are well-motivated. No direct Claude Code CHANGELOG reference, but the area is clearly relevant.

Size: 1,537 production logic lines (additions + deletions) across 11 production files, plus 984 test lines across 7 test files. No core module paths touched — everything is within packages/web-shell/client/**. Not subject to the core module hard block. The 1000+ line advisory applies but is non-blocking; the feature is cohesive enough that splitting would be artificial.

Approach: The scope feels right for what's being delivered. The Session Overview (ranked cards with multi-select → open in split or new tab) and Split View (N independent ChatPane components, each under its own DaemonSessionProvider) are complementary surfaces sharing infrastructure. Good reuse of existing SDK hooks (useSessions, useStatusReport, DaemonSessionProvider) and UI components (MessageList, ChatEditor, ToolApproval, ErrorBoundary). Polling cadence is well-considered (cheap list at 3s, expensive status at 10s, both pausing when hidden). The 6-pane cap and large-screen-only gating are sensible constraints. No unrelated changes or drive-by refactors detected.

One question: the split view intentionally does NOT dismiss when an approval becomes pending on the outer main session (each pane owns its own approval). This is a deliberate design choice — worth confirming this matches user expectations when the main session also has a pending approval that isn't visible in any split pane.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必填段落齐全,中英双语,附带截图和详细的 reviewer test plan。

问题: 真实、明确的 UX 缺口。重度用户并行驱动多个 daemon 会话时,无法一眼看到所有会话或同时操作 2+ 个会话。当前 web-shell 一次只显示一个会话,需要频繁切换。这是已观测到的工作流痛点,不是理论性加固。

方向: 对齐。多会话管理是 daemon web-shell 的自然演进——项目已有 roadmap/session-management 标签,竞品(VS Code 分屏编辑器、浏览器 DevTools)也提供类似界面。两个互补视图(总览做 mission control,分屏做主动操控)动机充分。Claude Code CHANGELOG 中无直接参考,但该领域显然相关。

规模: 11 个生产文件共 1,537 行生产逻辑代码(增 + 删),7 个测试文件共 984 行测试代码。未触及核心模块路径——全部在 packages/web-shell/client/** 内。不受核心模块硬阻断约束。1000+ 行大 PR 建议适用但非阻断;该功能足够内聚,拆分反而不自然。

方案: 范围与交付内容匹配。会话总览(排序卡片 + 多选 → 分屏/新标签页打开)和分屏(N 个独立 ChatPane,各挂自己的 DaemonSessionProvider)是共享基础设施的互补界面。良好复用了既有 SDK hooks(useSessionsuseStatusReportDaemonSessionProvider)和 UI 组件(MessageListChatEditorToolApprovalErrorBoundary)。轮询节奏考虑周到(廉价列表 3s、昂贵状态 10s、标签页隐藏时暂停)。6 窗格上限和大屏专用入口是合理约束。未检测到无关改动或顺手重构。

一个疑问:分屏故意不在外层主会话出现待审批时关闭(每个窗格管自己的审批)。这是有意的设计选择——值得确认当主会话也有待审批、但在任何分屏窗格中不可见时,是否符合用户预期。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal (before reading diff): to add multi-session management to the web-shell, I'd create a session overview panel listing all sessions with status/ranking, and a split view rendering N independent chat panes each with their own session provider. I'd reuse DaemonSessionProvider for per-pane isolation, the existing useSessions / useStatusReport hooks for data, and gate entry points to large screens.

The PR's approach matches and exceeds this baseline. Specifically:

  • Architecture is clean. The three-component hierarchy (SessionOverviewPanelSplitViewChatPane) is well-decomposed. Each ChatPane is self-contained under its own DaemonSessionProvider subtree — keyboard focus, approvals, and streaming state are naturally scoped per-pane without cross-contamination. The clientId is set to split-pane:<id> to avoid colliding with the main view's attachment.
  • Good reuse. All SDK hooks (useSessions, useStatusReport, useActions, useConnection, useStreamingState, useTranscriptBlocks), UI components (MessageList, ChatEditor, ToolApproval, AskUserQuestion, StreamingStatus, ErrorBoundary), and utilities (formatRelativeTime, extractPendingPermission, isAskUserQuestionToolName) are existing exports confirmed in the codebase.
  • Polling is well-engineered. Cheap session list polls at 3s; expensive status report at 10s; both skip ticks when the tab is hidden or a request is in flight. This mirrors the sidebar/daemon-status patterns.
  • No critical blockers found. No correctness bugs, security holes, or regressions identified.
  • No AGENTS.md violations. No over-abstraction, no unnecessary duplication, code lives in the right package (web-shell/client).

One observation: the deriveSessionCards function is exported and tested directly (good). The cx and colorDotClass helpers are small, local, and appropriate — no premature abstraction.

Test Results

Unit tests (base branch, 71 files): All 1,054 tests pass. The PR claims 1,098 passing including the new test suites — the delta is the new test files (SessionOverviewPanel, SplitView, ChatPane, useIsLargeScreen, splitUrl, sidebar additions, App additions).

 Test Files  71 passed (71)
      Tests  1054 passed (1054)
   Duration  3.86s

Build: npm run build --workspace=@qwen-code/web-shell succeeds. Both client (Vite) and server (esbuild) bundles compile cleanly.

✓ built in 8.27s (client)
✓ built in 1.30s (server, 187 modules)

Type check: Pre-existing type errors in MessageList.test.ts, Markdown.test.ts, and WebShellSidebar.test.tsx exist on the base branch — none introduced by this PR.

Real-Scenario Testing (tmux)

Daemon boots and serves the web-shell successfully on the base build:

$ node packages/cli/dist/index.js serve --web --port 18924
qwen serve: Web Shell UI served from .../packages/web-shell/dist
qwen serve listening on http://127.0.0.1:18924 (mode=http-bridge)
qwen serve: bound to workspace "..."
qwen serve: startup timing: processToListenMs=108 runQwenServeToListenMs=30
qwen serve: bearer auth disabled (loopback default)
[DAEMON] deferred runtime: scheduling fallback start in 1000ms
[DAEMON] deferred runtime: fallback timer fired, starting
[DAEMON] workspace roots initialized
qwen serve: session reaper started (interval 60000ms, idle threshold 1800000ms)
qwen serve: /acp WebSocket transport enabled on /acp
$ curl -s -o webshell.html -w "HTTP %{http_code} %{size_download}bytes" http://127.0.0.1:18924/
HTTP 200 4511bytes

Limitation: This CI runner is headless Linux — no browser available for interactive UI testing. The Session Overview and Split View are browser-rendered React components that require a real browser to verify interactive behavior (card ranking, multi-select, split pane isolation, ?split= URL handling, Back navigation). The PR author provided Playwright-verified screenshots and described end-to-end validation in the PR body. The unit test coverage (ranking, open-in-tab/split, pane add/close, URL helpers, large-screen hook) covers the testable logic.

中文说明

代码审查

独立方案(读 diff 前):为 web-shell 添加多会话管理,我会创建会话总览面板列出所有会话并显示状态/排序,以及分屏视图渲染 N 个独立聊天窗格各挂自己的 session provider。我会复用 DaemonSessionProvider 做窗格隔离,用既有的 useSessions / useStatusReport hooks 取数据,入口限定大屏。

PR 方案与我的基线一致并有超越:

  • 架构干净。 三层组件(SessionOverviewPanelSplitViewChatPane)解耦良好。每个 ChatPane 在自己的 DaemonSessionProvider 子树下自包含——键盘焦点、审批、流式状态自然按窗格隔离。clientId 设为 split-pane:<id> 避免与主视图冲突。
  • 复用良好。 所有 SDK hooks、UI 组件和工具函数均为代码库中已确认的既有导出。
  • 轮询设计合理。 廉价列表 3s、昂贵状态 10s,标签页隐藏或请求未完成时跳过。与侧栏/daemon 状态轮询模式一致。
  • 未发现关键阻断问题。 无正确性 bug、安全漏洞或回归。
  • 无 AGENTS.md 违规。 无过度抽象、无不必要的重复、代码放在正确的包中。

测试结果

单元测试(base 分支,71 文件): 1,054 个全部通过。PR 声称含新测试共 1,098 个通过。

构建: npm run build --workspace=@qwen-code/web-shell 成功。

类型检查: 基础分支上已有的类型错误,非 PR 引入。

真实场景测试(tmux)

Daemon 成功启动并提供 web-shell 服务(HTTP 200)。

限制: 此 CI 环境是无头 Linux——无浏览器可用。会话总览和分屏是浏览器渲染的 React 组件,需要真实浏览器验证交互行为。PR 作者已提供 Playwright 验证的截图和端到端验证描述。单元测试覆盖了可测试的逻辑。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Stepping back to look at the whole picture.

This PR adds two well-motivated UI surfaces to the web-shell — a Session Overview (mission control) and a Split View (N independent chat panes). The motivation is genuine: power users running multiple daemon sessions in parallel currently have no way to see or steer them simultaneously. The screenshots in the PR body show a polished, production-quality result.

The code is clean and well-structured. The three-component decomposition (SessionOverviewPanelSplitViewChatPane) is the right shape for this feature — each layer has a clear responsibility, and the per-pane DaemonSessionProvider isolation is an elegant way to scope keyboard focus, approvals, and streaming state without cross-contamination. The reuse of existing SDK hooks, UI components, and utilities is thorough — no parallel implementations or reinvented wheels. The polling strategy (cheap list at 3s, expensive status at 10s, both respecting visibility and in-flight requests) shows attention to operational cost.

The test coverage is comprehensive: ranking logic, open-in-tab/split wiring, pane add/close, URL helpers, large-screen hook, sidebar entry points, approval interaction, and App-level integration tests. 1,054 existing tests pass on the base branch, and the build compiles cleanly.

My independent proposal (before reading the diff) was essentially the same architecture — session overview panel with status ranking, split view with per-pane providers, large-screen gating. The PR's implementation matches and exceeds this baseline in polish (color-coded cards, responsive grid, popup-blocked handling, ?split= URL deep linking with one-shot consumption).

One design question I raised in Stage 1 (split view not dismissing on main-session approval) is a deliberate choice: each pane owns its own session's approval, so the outer main session's approval is intentionally not surfaced in the split. This is consistent with the per-pane isolation model, though it means a main-session approval could be invisible when the user is in split view. Worth a maintainer's judgment on whether this needs a visual indicator.

Honest assessment: This is a well-executed, well-tested feature PR. The code quality is high, the design decisions are reasoned, and the test coverage is thorough. It ships a real capability that users of the web-shell will find valuable. No critical blockers, no security concerns, no code quality issues.

Reservations:

  • Could not verify interactive browser behavior (headless CI). The PR author's Playwright-verified screenshots and detailed E2E description provide evidence, but a maintainer should do a quick browser smoke test before merging.
  • The 1,537 production lines and 18 files is a meaningful addition to the web-shell. Not a concern about quality — more that a feature of this scope benefits from a maintainer's eye on product direction (is this the right priority vs. other web-shell work?).

Recommending maintainer review rather than auto-approval, given the feature's scope and the fork origin.

中文说明

退一步看整体。

这个 PR 为 web-shell 添加了两个动机充分的 UI 界面——会话总览(mission control)和分屏视图(N 个独立聊天窗格)。动机真实:并行驱动多个 daemon 会话的重度用户目前没有同时查看或操控它们的方式。PR 中的截图展示了精致的、生产质量的结果。

代码干净、结构良好。三层组件分解(SessionOverviewPanelSplitViewChatPane)是这个功能的正确形状——每层职责清晰,每窗格的 DaemonSessionProvider 隔离优雅地将键盘焦点、审批和流式状态限定作用域,无交叉污染。对既有 SDK hooks、UI 组件和工具函数的复用充分——无并行实现或重复造轮子。轮询策略(廉价列表 3s、昂贵状态 10s、均尊重可见性和在途请求)体现了对运行成本的关注。

测试覆盖全面:排序逻辑、开标签页/分屏接线、窗格加/关、URL 工具、大屏 hook、侧栏入口、审批交互、App 级集成测试。基础分支 1,054 个既有测试全部通过,构建编译干净。

我的独立方案(读 diff 前)基本是相同的架构——带状态排序的会话总览面板、每窗格独立 provider 的分屏视图、大屏限定入口。PR 的实现在精细度上超越了基线(颜色标记卡片、响应式网格、弹窗拦截处理、?split= URL 深链接一次性消费)。

Stage 1 提出的一个设计疑问(分屏不在主会话待审批时关闭)是有意选择:每个窗格管自己会话的审批,所以外层主会话的审批故意不在分屏中显示。这与按窗格隔离模型一致,但当用户在分屏视图中时,主会话的待审批可能不可见。值得维护者判断是否需要视觉提示。

坦诚评价: 这是一个执行良好、测试充分的功能 PR。代码质量高,设计决策有理,测试覆盖全面。它交付了 web-shell 用户会觉得有价值的真实能力。无关键阻断、无安全问题、无代码质量问题。

顾虑:

  • 无法验证交互式浏览器行为(无头 CI)。PR 作者的 Playwright 验证截图和详细 E2E 描述提供了证据,但维护者应在合并前做一次快速浏览器冒烟测试。
  • 1,537 行生产代码和 18 个文件是对 web-shell 的有意义的添加。不是质量担忧——更多是这个规模的功能受益于维护者对产品方向的把关(相对于其他 web-shell 工作,这是正确的优先级吗?)。

建议维护者审查而非自动批准,鉴于功能范围和 fork 来源。

Qwen Code · qwen3.7-max

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

DragonnZhang
DragonnZhang previously approved these changes Jul 6, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Code Review Summary

Overall: Approve — clean architecture, solid provider isolation, good test coverage. A few actionable items below.

🔴 Major (should fix before merge)

  1. Stale selected Set when sessions disappearSessionOverviewPanel.tsx
    When a session vanishes from the list, its ID stays in the selected Set. If it reappears, it's silently pre-selected. toggleSelectAll also misbehaves when stale IDs inflate prev.size. Fix: prune selected in a useEffect keyed on cards.

  2. No keyboard navigation in card gridSessionOverviewPanel.tsx
    The <ul> grid lacks role="grid" / arrow-key handling. With 10+ sessions, keyboard users must tab through 3 controls per card. Consider role="listbox" + aria-multiselectable or arrow-key nav.

  3. isAskUserPermission duplicatedChatPane.tsx:34-43 copies App.tsx:652-660 verbatim. Extract to a shared util to prevent drift.

🟡 Minor (worth addressing, can be follow-up)

  • Session picker lacks click-outside / Escape dismiss (SplitView.tsx)
  • StreamingStatus always receives startedAt={undefined} in split panes (ChatPane.tsx)
  • Approval overlays lack aria-live — screen readers won't announce approvals in non-focused panes
  • useIsLargeScreen creates two MQL instances for the same query (initializer + effect) — reuse via useRef
  • buildSplitUrl has no URL length cap protection
  • Hardcoded hex colors in SessionOverviewPanel.module.css bypass the CSS variable design system
  • Cards lack role="group" semantic grouping for screen readers
  • Approval effect uses exact match mainView === 'scheduledTasks' — fragile if new mainView values are added

🧪 Test gaps

  • No test for MAX_PANES (6) enforcement
  • No test for session disappearing mid-view (stale selection)
  • No test for popup-blocked path (window.open returns null)
  • No test for onOpenSession NOT firing on checkbox click

✅ Highlights

  • Provider isolation is excellent: each pane gets its own DaemonSessionProvider with unique clientId, no cross-pane state leaks
  • ?split= URL lifecycle is correct: consume → replaceState strip → no re-trigger
  • SSE cleanup relies correctly on React unmount semantics
  • Tiered polling (3s list / 10s status) with document.hidden and in-flight guards is well-designed
  • Fully backward compatible — single-session flow unchanged
  • No security concerns (React auto-escapes session names, no injection vector in ?split=)

…/ split view

- SessionOverviewPanel: prune the selection Set when a session leaves the list
  (so a reappearing session isn't silently reselected) and make select-all use
  the intersection rather than prev.size.
- Extract isAskUserPermission into a shared util so App.tsx and ChatPane.tsx no
  longer keep verbatim copies that can drift.
- SplitView: dismiss the "add session" picker on Escape or a click outside it.
- Tests: MAX_PANES cap, popup-blocked path, checkbox-selects-without-navigating,
  stale-selection pruning, and a direct test for the extracted util.
@wenshao
wenshao dismissed stale reviews from DragonnZhang and qwen-code-ci-bot via 0615384 July 6, 2026 15:04
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @qqqys for the thorough review — addressed the correctness items and the test gaps in 0615384.

Fixed

  • 🔴 Stale selected Set — the selection is now pruned in a useEffect keyed on the cards, so a session that leaves and reappears is no longer silently pre-selected. toggleSelectAll now checks the intersection (ids.every(id => prev.has(id))) instead of prev.size, so stale ids can't skew it.
  • 🔴 isAskUserPermission duplication — extracted to utils/askUserPermission.ts; App.tsx and ChatPane.tsx both import it now (no more verbatim copies).
  • 🟡 Picker dismiss — the split-view "add session" picker now closes on Escape or a click outside it.
  • 🧪 Test gaps — added: MAX_PANES (6) cap, popup-blocked path (window.open → null), checkbox-selects-without-navigating (onOpenSession not fired), stale-selection pruning, and a direct test for the extracted util.

Deferred (with reasons)

  • Keyboard grid nav / role=group / aria-live on approvals — real a11y gaps, but a proper role=grid + roving-tabindex pass (and live-region announcements) is a focused follow-up; today the cards are tabbable buttons.
  • Hardcoded hex color dots — these intentionally mirror the existing sidebar session-group palette (getGroupColorClass in WebShellSidebar.module.css uses the same values); changing them only here would diverge from the sidebar. Better as a repo-wide token migration.
  • buildSplitUrl length cap — bounded in practice: SplitView renders at most MAX_PANES (6) panes, and even "select all" of many sessions yields a short URL of UUIDs that browsers handle fine.
  • StreamingStatus startedAt — degrades gracefully (no elapsed timer); wiring a per-pane turn-start is minor polish.
  • useIsLargeScreen two matchMedia calls — cheap and transient (initializer + effect, not a leak), left as-is.
  • Approval effect exact === 'scheduledTasks' — deliberate: it was changed from !== 'chat' precisely so an approval on the outer session doesn't yank the user out of the split; a future full-pane view would make its own explicit choice here.

CI is green and the full suite is 1105 passing.

中文说明

感谢 @qqqys 的细致 review,已在 0615384 处理了正确性问题和测试缺口。

已修

  • 🔴 stale selected 反选:现在用一个以 cards 为 key 的 useEffect 剪枝选择集,会话离开又回来时不会被静默预选;toggleSelectAll 改用交集判断(ids.every(id => prev.has(id)))而非 prev.size,避免陈旧 id 干扰。
  • 🔴 isAskUserPermission 重复:抽到 utils/askUserPermission.ts,App.tsx 和 ChatPane.tsx 都改为 import(不再各留一份逐字副本)。
  • 🟡 picker 关闭:分屏"添加会话"下拉现在支持 Escape 或点击外部关闭。
  • 🧪 测试缺口:补了 MAX_PANES(6)封顶、弹窗被拦截路径(window.open 返回 null)、勾选框只选不跳转(不触发 onOpenSession)、stale 选择剪枝,以及抽出 util 的直接测试。

暂缓(附理由)

  • 键盘网格导航 / role=group / 审批 aria-live:确是 a11y 缺口,但完整的 role=grid + roving-tabindex(以及 live-region 播报)更适合单独一轮 a11y follow-up;目前卡片本身是可 tab 的按钮。
  • 写死的色值圆点:刻意与既有侧栏的会话分组调色板一致(WebShellSidebar.module.cssgetGroupColorClass 用同样的值),只在这里改会和侧栏不一致,更适合仓库级的 token 迁移一起做。
  • buildSplitUrl 长度上限:实际有界——SplitView 最多渲染 MAX_PANES(6)个窗格,即使全选很多会话,URL 也就是若干 UUID,浏览器可正常处理。
  • StreamingStatus startedAt:优雅降级(只是没有计时器);逐窗格追踪 turn 开始时间属于小打磨。
  • useIsLargeScreen 两次 matchMedia:开销小且短暂(initializer + effect,非泄漏),保持原样。
  • 审批 effect 的精确匹配 === 'scheduledTasks':有意为之——从 !== 'chat' 改成这个,正是为了主会话审批时不把用户从分屏踢出;未来新增全屏视图时再各自显式决定。

CI 已绿,全套测试 1105 通过。

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ Downgraded from Request changes to Comment: self-PR; CI failing: review-pr.

— GPT-5 via Qwen Code /review

Comment thread packages/web-shell/client/App.tsx
Comment thread packages/web-shell/client/components/ChatPane.tsx
Comment thread packages/web-shell/client/components/ChatPane.tsx
Comment thread packages/web-shell/client/components/SplitView.tsx Outdated
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx Outdated
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /resolve

…n-overview-split

# Conflicts:
#	packages/web-shell/client/App.tsx
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts, but the head branch changed while resolving, so the update was not pushed. Re-run /resolve. The resolved diff is attached as the qwen-resolve-pr-6400 artifact on the workflow run.

Merge Conflict Resolution Summary — PR #6400

Conflicted file

packages/web-shell/client/App.tsx

Conflict

At the import block (~line 75), the PR branch and origin/main diverged:

Side Imports
PR branch (HEAD) resolveShellOutputMaxLines from ./components/messages/ToolGroup and isAskUserPermission from ./utils/askUserPermission
origin/main isAskUserQuestionToolName from ./components/messages/toolFormatting

Resolution

  1. Kept isAskUserPermission from the PR branch — it is used at two call sites in App.tsx (lines ~1010 and ~1016). The PR extracted the previously-inline isAskUserPermission helper into utils/askUserPermission.ts, which internally delegates to isAskUserQuestionToolName.

  2. Dropped isAskUserQuestionToolName from origin/main — no longer directly used in App.tsx after the PR's extraction; it is consumed transitively via utils/askUserPermission.ts.

  3. Dropped resolveShellOutputMaxLines from the PR branch — origin/main refactored shellOutputMaxLines handling: removed the const shellOutputMaxLines = resolveShellOutputMaxLines(workspaceSettings) call and the shellOutputMaxLines prop on MessageList, moving the logic into ToolGroup itself. The import became dead code after merge.

Auto-merged files (no conflict)

  • packages/web-shell/client/i18n.tsx — auto-merged cleanly.

doudouOUC
doudouOUC previously approved these changes Jul 6, 2026

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review Summary

Well-designed, high-quality, purely additive PR. Clean component boundaries, correct provider isolation strategy, and thorough test coverage. No blocking issues found.

Architecture highlights

  • Provider isolation is correct: each split pane runs under its own DaemonSessionProvider with a unique clientId (split-pane:{sessionId}), ensuring SSE streams, transcripts, and approvals never interfere across panes.
  • Good separation of concerns: isAskUserPermission extracted to a shared util prevents the ChatPane and App approval logic from drifting apart.
  • URL-driven split entry is solid: ?split=a,b build/parse logic is concise and round-trip tested.
  • Responsive degradation is well-handled: large-screen gate + auto-close on viewport shrink protects against stranded-UI states.

Minor suggestions (non-blocking)

  1. Repeated constants: SESSION_PAGE_SIZE = 1000 and SESSION_ORGANIZATION_FEATURE are duplicated between SessionOverviewPanel.tsx and SplitView.tsx. Extracting to a shared constants file would prevent future drift.

  2. openSelectedInNewTab useCallback efficacy: selectedIds is a new array reference every render (derived from cards.filter(…)), so the useCallback dependency changes every render, defeating memoization. Consider using a ref for the latest ids, or simply dropping useCallback (since the consumer is a button, perf impact is negligible either way).

  3. StreamingStatus startedAt={undefined} in ChatPane: always passing undefined is intentional (no timer in split panes), but a brief comment explaining the design choice would help future readers.

  4. Panel aria-label ternary chain in App.tsx: currently settings ? … : status ? … : sessionsOverview. If more panel types are added, a Record<panel, label> map would scale better.

  5. aria-selected="false" on SplitView picker options: since items are click-to-select-and-dismiss (no persistent selected state), omitting aria-selected entirely is slightly more correct per WAI-ARIA listbox semantics.

Test coverage

Excellent coverage including: card ranking/priority, select-all/deselect/prune-on-leave, popup-blocked notice, split URL round-trip, useIsLargeScreen sync init + async change + graceful degradation, ChatPane send/cancel/approval/close, SplitView add/remove/cap/dedup/exit, and App-level panel switching + ?split= consumption + viewport-shrink auto-close.

LGTM — ready to merge.

- ToolApproval: add a `keyboardActive` prop; split panes pass false so global
  Enter/Escape/digit shortcuts can't confirm the wrong session's approval, and
  the outer session's approval overlay is no longer rendered behind the split
  (where it would keep its global shortcuts while hidden).
- ChatPane: defer the composer commit until sendPrompt resolves, so a rejected
  prompt (transcript loading / disconnected / turn active) preserves the draft
  instead of silently dropping it.
- SplitView: include a per-mount nonce in each pane's clientId so two tabs
  opening the same split don't share a client id — which suppressOwnUserEcho
  would treat as a self-echo and drop from the transcript.
- SessionOverviewPanel: cap the split selection to MAX_SPLIT_PANES before
  building the ?split= URL or opening the in-window split, with a hint when more
  are selected; also dismiss the split picker on Escape / click-outside.
- Tests covering each.
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Status update:

  • Rebase/merge: resolved the conflict with main (it had advanced past fix(web-shell): refine tool detail presentation #6399, which reshaped MessageListProps and the isAskUserPermission/resolveShellOutputMaxLines imports) by merging origin/main in.
  • /review findings: addressed all four [Critical] items and the [Suggestion] in 4fcec87 (replied inline + resolved each thread): outer approval no longer mounts behind the split, split-pane approvals are click-only (no cross-pane global keyboard), the composer commits only after the prompt is accepted, per-tab pane client ids, and the split selection is capped to the pane limit.

Full suite is 1120 passing, typecheck/eslint/build clean.

中文
  • rebase/合并: main 已领先(越过 fix(web-shell): refine tool detail presentation #6399,它改了 MessageListPropsisAskUserPermission/resolveShellOutputMaxLines 的导入),已 merge origin/main 解决冲突。
  • /review 意见: 四个 [Critical] 和一个 [Suggestion] 均已在 4fcec87 处理(逐条 inline 回复并 resolve):分屏下不再挂外层审批、分屏窗格审批改为纯点击(无跨窗格全局键盘)、prompt 被接受后才提交草稿、每标签页独立的窗格 client id、分屏选择按窗格上限封顶。

全套测试 1120 通过,typecheck/eslint/build 均干净。

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ Downgraded from Request Changes to Comment: self-PR; CI failing: review-pr.

[Critical] No Escape key exits split viewApp.tsx:3740: when mainView === 'split', activePanel is null, so the Escape handler skips the panel-dismiss branch. Escape falls through to decideEscapeIntent which receives blocked: true and returns { kind: 'ignore' }. Every other full-pane view (Settings, Status, Sessions) can be dismissed with Escape; split view is the only exception. Fix: add if (e.key === 'Escape' && live.mainView === 'split') { e.preventDefault(); live.exitSplitView(); return; } before the activePanel check.

3 additional Critical findings inline, plus Suggestion-level recommendations in the summary comment.

— qwen3.7-max via Qwen Code /review

Comment thread packages/web-shell/client/components/SplitView.tsx Outdated
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/utils/splitUrl.ts
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Suggestions — commit ab7e40cb

File Issue Suggested fix
SessionOverviewPanel.tsx:180 selected Set never pruned when sessions disappear — toggleSelectAll misfires when stale entries inflate selected.size Prune stale entries in useEffect keyed on cards
splitUrl.ts:35 parseSplitSessionIds accepts any string with no count cap or format validation Cap at MAX_PANES (6) and add lightweight format check
SessionOverviewPanel.tsx:35, SplitView.tsx:21, WebShellSidebar.tsx:34 SESSION_PAGE_SIZE/SESSION_ORGANIZATION_FEATURE duplicated across 3 files Extract to shared constants module
SplitView.tsx:118 Session picker dropdown has no click-outside or Escape dismiss Add document pointerdown listener or onBlur handler
App.tsx:4332 Sidebar "Split View" button opens single-pane split (confusing) Open overview first, or add placeholder pane
ChatPane.tsx:38 isAskUserPermission is a verbatim copy of App.tsx's local helper Extract to shared utility (e.g. toolFormatting.ts)
SessionOverviewPanel.tsx:228 popupBlocked notice set once, never cleared on retry Reset before retry or add dismiss button
SessionOverviewPanel.tsx:377 ErrorBoundary wrapper untested Test render throw → fallback UI
ChatPane.tsx:88 handleSubmit empty text guard untested Submit empty string, assert sendPrompt not called
SplitView.tsx:83 MAX_PANES=6 cap untested Add 7th pane, verify rejection

— qwen3.7-max via Qwen Code /review

- SplitView: wrap each pane in its own ErrorBoundary, so a render crash in one
  pane (malformed block, unexpected tool shape) shows an inline fallback with a
  close action instead of white-screening the whole split.
- splitUrl / overview: carry the daemon token into the new-tab split URL's
  fragment. The current tab has already stripped the token from its URL, so a
  token-auth (`serve --open`) deployment would otherwise open the split tab
  unauthenticated. The token rides the hash (never sent to the server / logs).
- Tests: per-pane error isolation, token-in-fragment (and none without a token),
  and the overview polling effects (interval fires, document.hidden skips, and
  the in-flight guard prevents overlapping polls).

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Suggestions — commit e49274c

File Issue Suggested fix
packages/web-shell/client/components/messages/ToolGroup.tsx (ExpandedBashOutput) All shell output truncation was removed — MAX_BASH_LINE_CHARS, maxLines, show-all/show-less toggle. The new component renders full unbounded output in <pre>. shellOutputMaxLines user setting is silently ignored. A verbose command (find /, cat large file) can freeze the browser tab. ExpandedReadContent retains MAX_MARKDOWN_READ_CHARS/MAX_MARKDOWN_READ_LINES guards; the shell path has none. Reintroduce a cap (e.g. 500 lines or 50k chars) with a "Show all" toggle, or add max-height + overflow: auto CSS containment. Restore shellOutputMaxLines from settings or document its removal.

Needs Human Review:

  • packages/cli/src/ui/hooks/useGeminiStream.ts:2768-2769 — The new hideAlwaysAllow filter in handleApprovalModeChange may over-capture pmForcedAsk tools. When switching to YOLO/AUTO_EDIT while a PM forced-ask tool is pending, it may be skipped from batch auto-approval and remain stuck in awaiting_approval.
  • packages/web-shell/client/utils/splitUrl.ts:33-42buildSplitUrl copies all query params from currentHref (via new URL(currentHref)) including any ?token= still present. It adds the token to the fragment but doesn't strip the query-string copy. In production removeDaemonTokenFromUrl() strips it first, but in DEV mode that function returns early. Consider adding url.searchParams.delete('token').

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

⚠️ Downgraded from Request changes to Comment: self-PR.

— GPT-5 via Qwen Code /review

Comment thread packages/web-shell/client/App.tsx
Comment thread packages/web-shell/client/App.tsx Outdated
…el contexts

- App: hide (display:none) + aria-hide the outer chat subtree whenever
  mainView !== 'chat', not only when a panel is open. Previously the outer
  chat/composer/toolbar stayed reachable by keyboard/AT behind the full-page
  split (it was only covered visually). State is preserved (node stays mounted).
- App: wrap SplitView in the app-level WebShellCustomizationProvider and
  CompactModeContext so split panes render markdown / tool-headers / thinking
  the same way the single-session chat does. Todo contexts stay chat-only —
  they belong to the outer session, not the panes.

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No blockers. One Suggestion-level recommendation is in the Suggestion summary comment below.

Comment thread packages/web-shell/client/components/messages/ToolApproval.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx Outdated
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
Comment thread packages/web-shell/client/components/SplitView.tsx
Comment thread packages/web-shell/client/components/ChatPane.tsx
…lit UX

- ToolApproval: add a dedicated test on the real component that the global
  keyboard shortcut is armed by default and NOT armed when keyboardActive=false
  (the cross-pane approval safety mechanism).
- SplitView: auto-exit to the Session Overview when the last pane is closed
  (guarded so an initial empty seed doesn't bounce straight back out).
- ChatPane: add tests for the cancel action, the empty/whitespace submit guard,
  and error routing to the onError prop.
- Extract the shared session-list page size + organization feature flag into
  constants/sessions.ts, used by the overview, split view, and sidebar, so the
  values can't drift between the three.
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /resolve

…n-overview-split

# Conflicts:
#	packages/web-shell/client/App.tsx
Comment thread packages/web-shell/client/components/SessionOverviewPanel.tsx Outdated
Comment thread packages/web-shell/client/App.test.tsx Outdated
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts, but the head branch changed while resolving, so the update was not pushed. Re-run /resolve. The resolved diff is attached as the qwen-resolve-pr-6400 artifact on the workflow run.

Merge Conflict Resolution Summary — PR #6400

Conflicted Files

Only one file had conflicts: packages/web-shell/client/App.tsx

Other auto-merged files (no conflicts):

  • packages/web-shell/client/App.test.tsx
  • packages/web-shell/client/i18n.tsx

Conflict Details

packages/web-shell/client/App.tsx — Single conflict at line ~4655

Root cause: Both branches modified the scroll-to-bottom button SVG area, but with different indentation levels:

  • origin/main added a chatViewWrap wrapper div around the chat subtree, which shifted all nested content by +2 spaces of indentation. This included the complete SVG block for the scroll-to-bottom button.
  • PR branch modified the same SVG area but at a shallower indentation level (no chatViewWrap wrapper).

Git could not match the corresponding lines because the indentation differed, creating a conflict region where both sides looked like completely different code.

Resolution Strategy

1. SVG block structure → Took origin/main's version

The HEAD (PR) side of the conflict had a broken SVG element — the opening <svg> tag was missing, and there were duplicate code fragments. origin/main's side contained the complete, well-formed SVG:

<svg
  className={styles.scrollToBottomIcon}
  viewBox="0 0 24 24"
  aria-hidden="true"
>
  <path
    d="M12 5v13M6.5 12.5 12 18l5.5-5.5"
    fill="none"
    stroke="currentColor"
    strokeLinecap="round"
    strokeLinejoin="round"
  />
</svg>

2. Closing tags → Took origin/main's indentation

The PR branch's closing tags (</button>, </div>, )}) were at a shallower indentation because it lacked the chatViewWrap wrapper. Since the merged file includes chatViewWrap, origin/main's deeper indentation is correct.

3. Approval overlay guards → Added from PR branch

Added mainView === 'chat' guards to both pendingToolApproval and pendingAskUserApproval render blocks:

{pendingToolApproval && mainView === 'chat

wenshao added 2 commits July 7, 2026 06:39
…plit

- Split view: when the outer (main) session is waiting on an approval
  that's hidden behind the split, show a non-blocking notice banner with
  a "Go to it" button that returns to the chat where the approval lives.
- Auto-close the split (like the overview panel) when the viewport shrinks
  below the large-screen breakpoint, so users aren't stranded.
- Session Overview: surface a failed refresh inline (keeping the last-good
  cards) instead of silently swallowing it once cards are on screen.
- Tests: status-report poll cadence, picker dismiss (Escape / outside /
  inside click), inline refresh-failure banner.
… test

- openSelectedInNewTab now clears win.opener (the split tab carries a
  daemon token in its URL fragment) to prevent reverse tabnabbing, matching
  the existing bug-report window.open path.
- Strengthen the split-view App test so a missing outer-chat subtree fails
  instead of passing vacuously through an optional chain.
Comment thread packages/web-shell/client/App.tsx
Comment thread packages/web-shell/client/App.tsx
Comment thread packages/web-shell/client/App.tsx Outdated
Comment thread packages/web-shell/client/components/ChatPane.tsx
Comment thread packages/web-shell/client/components/ChatPane.tsx Outdated
@wenshao

wenshao commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections filled in, including bilingual description, reviewer test plan with screenshots, and risk/scope analysis.

Problem: this is a feature addition (not a bug fix), so the "problem existence" check is about whether the use case is real. Managing multiple parallel daemon sessions is a genuine power-user workflow, and today's web-shell only shows one session at a time. The motivation is clear and well-articulated.

Direction: aligned with the web-shell's purpose as a management UI for the daemon. Adding session overview + split view extends the existing multi-session capabilities without changing any core logic. All changes are scoped to packages/web-shell/ — no core infrastructure is touched.

Size: not applicable for the core-module gate (all 24 files are in packages/web-shell/). For reference: ~1,881 production lines + ~1,672 test lines across 24 files. Well-tested — the PR author reports 1,098 passing tests.

Approach: the scope feels right for the stated goal. The component decomposition (SessionOverviewPanel, SplitView, ChatPane) is clean, each under its own provider subtree. Good refactoring touches: extracting isAskUserPermission to a shared util (prevents drift between App and ChatPane), and pulling SESSION_LIST_PAGE_SIZE/SESSION_ORGANIZATION_FEATURE into constants/sessions.ts so the sidebar, overview, and split picker stay in sync. The security details are handled well — win.opener = null on new tabs (reverse tabnabbing), daemon token in URL fragment (not query), and the outer approval overlay suppressed under the split (no ghost keyboard shortcuts confirming unseen approvals).

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必填部分均已填写,包括双语描述、带截图的审查测试计划和风险/范围分析。

问题:这是一个功能新增(非 bug 修复),所以"问题是否存在"检查的是用例是否真实。管理多个并行的 daemon 会话是真实的重度用户工作流,而今天的 web-shell 一次只显示一个会话。动机清晰且表述充分。

方向:与 web-shell 作为 daemon 管理 UI 的定位一致。添加会话总览 + 分屏扩展了既有多会话能力,未改动任何核心逻辑。所有改动都在 packages/web-shell/ 内——未触及核心基础设施。

规模:核心模块门控不适用(24 个文件全在 packages/web-shell/)。供参考:约 1,881 行生产代码 + 约 1,672 行测试代码,跨 24 个文件。测试充分——作者报告 1,098 个测试通过。

方案:范围与目标匹配。组件拆分(SessionOverviewPanel、SplitView、ChatPane)干净,各自挂在独立的 provider 子树下。合理的重构:把 isAskUserPermission 提取到共享工具(防止 App 和 ChatPane 漂移),把 SESSION_LIST_PAGE_SIZE/SESSION_ORGANIZATION_FEATURE 抽到 constants/sessions.ts(侧栏、总览、分屏选择器保持一致)。安全细节处理到位——新标签页设置 win.opener = null(防反向标签页劫持)、daemon token 放在 URL fragment 而非 query、分屏下隐藏外层审批覆盖层(避免幽灵键盘快捷键确认看不见的审批)。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code Review

I read the full diff (4,030 lines) and compared it against what I would independently build for "multi-session overview + split view in a web shell."

Independent baseline: I'd add a session list panel showing all sessions with status badges (needs-approval > running > idle), a split view rendering N chat panes each with their own session provider, URL-based routing for opening the split in a new tab, and a responsive gate to hide both on small screens. I'd extract shared session-list constants and the isAskUserPermission helper since it's now used in two places.

The PR matches or exceeds this baseline. The component decomposition is clean, the ranking logic is well-tested, and the security/UX details go beyond what I'd have thought of on a first pass (reverse tabnabbing guard, fragment-only token, per-pane error boundaries, approval overlay suppression under the split).

No critical blockers found:

  • No correctness bugs — state management (panel/mainView/split) is well-orchestrated, with proper cleanup on viewport shrink, last-pane-close auto-exit, and selection pruning when sessions leave the list.
  • No security holes — win.opener = null is set, daemon token stays in the URL fragment (never sent to the server), and the outer approval overlay is correctly suppressed under the split view to prevent ghost keyboard shortcuts.
  • No regressions — the single-session chat behavior is unchanged; new entry points are additive and gated to large screens.

No AGENTS.md violations:

  • No over-abstraction — deriveSessionCards, buildSplitUrl/parseSplitSessionIds, useIsLargeScreen, and isAskUserPermission are all shared across 2+ consumers. No speculative indirection.
  • No unnecessary duplication — ChatPane reuses MessageList, StreamingStatus, ChatEditor, ToolApproval, and AskUserQuestion from the existing component library. The session-interaction wiring (pending approval, submit, cancel) is necessarily duplicated from App.tsx because each pane is an independent provider subtree — extracting it further would require a custom hook that couples to the outer approval model.
  • Code is in the right package — everything lives in packages/web-shell/client/, following the existing CSS-module + hook patterns.
  • i18n is complete (both EN and ZH) for all new strings.

Reuse check: The PR correctly reuses existing components (ErrorBoundary, MessageList, ChatEditor, ToolApproval, AskUserQuestion, StreamingStatus, DialogShell patterns) and SDK hooks (useConnection, useSessions, useStatusReport, useActions). The formatRelativeTime and getDaemonToken utilities are reused from existing web-shell modules. No parallel implementation of anything that already exists.

Testing

Unit tests: CI passed on ubuntu-latest (24m42s). The PR author reports 1,098 tests passing, including new suites for:

  • SessionOverviewPanel (ranking, selection, new-tab URL, split, polling, popup-blocked, error states)
  • SplitView (N panes, dedup, add/close, auto-exit, max panes, per-pane error boundary)
  • ChatPane (send, approval, cancel, streaming, connection loss)
  • useIsLargeScreen (seed, react, degrade)
  • splitUrl (build, parse, round-trip, token-in-fragment)
  • askUserPermission (questions detection, tool-name fallback)
  • App.test.tsx (overview/split open/close, URL param consumption, viewport shrink, approval suppression)
  • WebShellSidebar.test.tsx (grid + columns entries, large-screen gating)

Real-scenario testing: This is a web UI feature (Session Overview panel and Split View in the browser-based web shell). The tmux-based E2E testing workflow is designed for CLI features and cannot drive a React-rendered browser UI. The PR author provided real screenshots captured against node packages/cli/dist/index.js serve --web with Playwright, and the reviewer test plan describes the manual verification steps. The unit test coverage is comprehensive for the logic layer.

CI checks: Test (ubuntu-latest, Node 22.x) ✅ pass, precheck-pr / precheck ✅ pass, review-pr ✅ pass, Classify PR ✅ pass.

中文说明

代码审查

阅读了完整 diff(4,030 行),并与我独立构建"多会话总览 + web-shell 内分屏"时会采用的方案进行了比较。

独立基线: 我会添加一个会话列表面板,显示所有会话及其状态徽章(待审批 > 运行中 > 空闲),一个分屏视图渲染 N 个各自拥有独立 session provider 的聊天窗格,基于 URL 路由在新标签页打开分屏,以及响应式门控在小屏上隐藏两者。我会提取共享的会话列表常量和 isAskUserPermission 工具函数,因为它现在有两处使用。

PR 匹配或超过了这个基线。 组件拆分干净,排序逻辑测试充分,安全/UX 细节超出了我首次会考虑的范围(反向标签页劫持防护、token 仅在 fragment 中、分屏下按窗格隔离的错误边界、分屏下外层审批覆盖层的抑制)。

未发现关键阻断问题:

  • 无正确性 bug — 状态管理(panel/mainView/split)编排良好,视口缩小时正确清理、最后一个窗格关闭时自动退出、会话离开列表时选择被修剪。
  • 无安全漏洞 — 设置了 win.opener = null,daemon token 仅在 URL fragment 中(不会发送到服务器),分屏下外层审批覆盖层被正确抑制以防止幽灵键盘快捷键。
  • 无回归 — 单会话聊天行为不变;新入口是增量的且仅限大屏。

无 AGENTS.md 违规:

  • 无过度抽象 — deriveSessionCardsbuildSplitUrl/parseSplitSessionIdsuseIsLargeScreenisAskUserPermission 都有 2+ 消费者。无投机性间接层。
  • 无不必要的重复 — ChatPane 复用了 MessageListStreamingStatusChatEditorToolApprovalAskUserQuestion。会话交互接线的必要重复是因为每个窗格是独立的 provider 子树。
  • 代码在正确的包中 — 全部在 packages/web-shell/client/ 内,遵循既有的 CSS-module + hook 模式。
  • i18n 完整(中英文)。

测试

单元测试: CI 在 ubuntu-latest 上通过(24m42s)。作者报告 1,098 个测试通过。

真实场景测试: 这是一个 web UI 功能(浏览器中的会话总览和分屏)。基于 tmux 的 E2E 测试工作流是为 CLI 功能设计的,无法驱动 React 渲染的浏览器 UI。作者提供了在真实 daemon 上用 Playwright 截取的截图,单元测试覆盖了逻辑层。

CI 检查: Test (ubuntu-latest, Node 22.x) ✅ 通过,precheck-pr ✅ 通过,review-pr ✅ 通过。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

This is a well-executed feature PR. After reading the full diff, running the numbers, and comparing against what I'd build independently, I'm confident in the implementation.

The two new surfaces — Session Overview (mission control) and Split View (side-by-side panes) — solve a real power-user need without touching any core infrastructure. The component decomposition is clean: each split pane is an independent DaemonSessionProvider subtree, so browser focus naturally scopes keyboard input to the right session. The security details are handled thoughtfully (reverse tabnabbing guard, fragment-only token, approval overlay suppression). Tests are comprehensive (1,098 passing on CI), i18n is complete in both languages, and the code follows existing web-shell conventions.

The shared refactors (isAskUserPermission util, constants/sessions.ts) are justified — they prevent drift between 3+ consumers and don't add speculative abstraction. The scope is right for the goal; I don't see anything that could be cut without losing functionality.

LGTM. ✅

中文说明

这是一个执行良好的功能 PR。在阅读完整 diff、统计数据并与我独立会构建的方案比较后,我对这个实现有信心。

两个新界面——会话总览(mission control)和分屏(并排窗格)——解决了真实的重度用户需求,且未触及任何核心基础设施。组件拆分干净:每个分屏窗格是独立的 DaemonSessionProvider 子树,浏览器焦点自然地将键盘输入限定到正确的会话。安全细节处理周到(反向标签页劫持防护、token 仅在 fragment 中、审批覆盖层抑制)。测试充分(CI 上 1,098 个通过),i18n 双语完整,代码遵循既有 web-shell 约定。

共享重构(isAskUserPermission 工具、constants/sessions.ts)是合理的——防止 3+ 消费者之间的漂移,且没有添加投机性抽象。范围与目标匹配;我看不到砍掉什么不会丢失功能的部分。

LGTM ✅

Qwen Code · qwen3.7-max

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@qwen-code-ci-bot qwen-code-ci-bot added category/ui User interface and display scope/web-shell type/enhancement Non-bug improvement or optimization labels Jul 7, 2026
- Refocus the composer after a shrink-driven split close so keyboard users
  aren't dropped onto <body> (skips when an approval or panel takes over).
- Stabilize SplitView onExit via useCallback so its last-pane-close effect
  doesn't re-fire on every App re-render.
- ChatPane: surface a per-pane connection-loss banner instead of silently
  showing stale messages when a pane's daemon connection drops.
- ChatPane: anchor the streaming timer to the active turn's start (last user
  message timestamp) so a pane opened mid-turn shows real elapsed time.
- Tests: split auto-close on shrink, outer-approval split notice + return-to-
  chat, connection banner, and streaming-timer anchoring.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No review findings. Downgraded from Approve to Comment: CI still running.

Reviewed across 9 parallel agents (correctness, security, code quality, performance, test coverage, 3 undirected personas, build/test) + deterministic analysis (tsc: 0 findings, eslint: 0 findings). All 924 unit tests pass. The code demonstrates careful attention to provider isolation, per-pane error boundaries, keyboard scoping, and polling guards.

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

qwen /review 6400 --comment failed to produce findings.

Error: [API Error: {"code":"Throttling.BurstRate","message":"Request rate increased too quickly. To ensure system stability, please adjust your client logic to scale requests more smoothly over time.","request_id":"3a8b5ed1-3d75-94fd-b536-e276dd3dd232"}]

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review (post-update): Approved

Summary

Re-reviewed the full diff (4,031 lines, 24 files) after the author's updates. All previously raised critical and suggestion-level items have been addressed. The code is clean, well-structured, and thoroughly tested.

Previously Raised Issues — All Resolved

Issue Resolution
[Critical] Invisible approval when main session not in pane Addressed: splitApprovalNotice banner now surfaces when the outer session has a pending approval under the split, with a "Go to it" button that exits split and shows the overlay
[Suggestion] Viewport shrink leaves user stranded in split Fixed: a useEffect now closes both the Session Overview panel AND the split view when the screen shrinks below 1024px; focus is restored to the composer via focusComposerAfterSplitCloseRef
[Suggestion] popupBlocked state never resets Fixed: setPopupBlocked(!win) on each attempt — success clears the notice
[Suggestion] Status-report polling untested Fixed: 4 new polling tests (interval firing, tab-hidden skip, in-flight guard, separate status cadence)
[Suggestion] Picker-dismiss untested Fixed: Escape and outside-click dismiss tests now in SplitView.test.tsx, plus a "keeps open on inner click" test

Architecture Highlights (unchanged assessment)

  • Provider isolation: each split pane is a DaemonSessionProvider subtree — own SSE, transcript, approvals, streaming state. No cross-pane global state.
  • Per-mount nonce (crypto.randomUUID()): prevents clientId collisions across tabs for the same session.
  • Security: window.opener = null on new-tab open; daemon token in URL fragment (never sent to server).
  • Accessibility: aria-hidden on the outer chat subtree when split is active; keyboardActive={false} on pane approvals prevents ghost keyboard confirmations; role="status" / role="alert" on notices.
  • Graceful degradation: per-pane ErrorBoundary, connection-loss banner per pane, refresh failure inline (last-good cards preserved).
  • Shared constants (constants/sessions.ts): eliminates drift between sidebar, overview, and split picker.
  • Draft-commit-after-acceptance: sendPrompt promise resolves → commit; rejects → preserves draft. Well-tested.

Non-Blocking Observations (no action required)

  1. Minor: The initial autoLoad inside useSessions and the first poll tick (3s) could theoretically overlap since the listInFlight ref only guards the interval handler. Harmless (one extra request at mount), but worth knowing.
  2. Future enhancement: The split view doesn't yet support Escape-to-exit (unlike the scheduled-tasks page); this is intentionally different (Escape in a pane would conflict with Escape-to-dismiss-picker), so the current Back-button-only exit is the correct design for now.

Test Coverage

Comprehensive: 1098+ tests passing. New suites cover deriveSessionCards ranking, overview rendering + interaction, polling behavior, split view lifecycle (add/close/cap/ErrorBoundary/picker-dismiss), chat pane (send/approval/cancel/connection-error/streaming-timer), ToolApproval keyboard gate, splitUrl helpers (build/parse/round-trip/token-in-fragment), useIsLargeScreen hook, sidebar entry points, App-level integration (split URL consumption, approval notice, viewport shrink).

No blockers. Ship it. 🚀

@wenshao
wenshao added this pull request to the merge queue Jul 7, 2026
Merged via the queue into QwenLM:main with commit f41e95a Jul 7, 2026
182 checks passed

@DragonnZhang DragonnZhang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.

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

Labels

category/ui User interface and display scope/web-shell type/enhancement Non-bug improvement or optimization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants