feat(web-shell): show sessions awaiting user action - #6956
Conversation
🖼️ web-shell visual previewAuto-rendered from this PR head Screenshots · light / dark
Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Thanks for the PR! Template looks good ✓ Problem: Observed UX gap — the session API already exposes Direction: Aligned. Distinguishing permission-blocked from input-blocked sessions is a clear usability improvement, and showing AskUserQuestion question counts is a natural companion change. No direct CHANGELOG reference, but the area is squarely within web-shell's scope. Size: Not applicable — all changes are in Approach: The scope feels right. Five files, +135/-18, and every line serves the stated goal. The attention badge logic is a simple ternary chain, the CSS uses existing design tokens, and the Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 UX 差距——会话 API 已提供 方向：对齐。区分权限阻塞和输入阻塞是明确的可用性改进，显示 AskUserQuestion 问题数量是自然的配套改动。CHANGELOG 无直接参考，但该领域完全在 web-shell 范围内。 规模：不适用——所有改动都在 方案：范围合理。5 个文件，+135/-18，每一行都服务于目标。attention badge 逻辑是简单的三元链，CSS 使用现有设计 token， 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Code ReviewRe-run after the author addressed both actionable review suggestions in Wording fix: Active AskUserQuestion summaries now show standalone "Provide information" / "补充信息" without the misleading "Running/正在执行" prefix. The fix is clean — Contrast fix: Badge text now mixes 95% of the status token with 5% theme foreground ( Everything else from the prior review still holds: the dual-flag priority logic correctly makes permission win when both flags are set, No correctness issues, security concerns, or AGENTS.md violations found. Real-Scenario TestingThis is a Web Shell UI change (React sidebar badges + message summaries). Visual verification of the sidebar badges requires browser rendering — not achievable via tmux terminal capture. Unit tests cover the new logic comprehensively (28 ToolGroup tests, 27 sidebar tests, 1627/1627 total across 103 files). Dev server from PR worktree: Dev server starts cleanly with no errors. The PR build serves from the worktree's built Unit tests: 1627/1627 passed across 103 test files. Typecheck and ESLint clean. 中文说明代码审查作者在 文案修复： 运行中的 AskUserQuestion 摘要现在独立显示"补充信息" / "Provide information"，不再带误导性的"正在执行/Running"前缀。修复方式简洁—— 对比度修复： badge 文字现在使用 95% 状态 token 与 5% 主题前景色混合（ 上一轮审查的其它结论仍然成立：双标志优先级逻辑正确地在两者都置位时让权限优先， 未发现正确性问题、安全隐患或 AGENTS.md 违规。 真实场景测试这是 Web Shell UI 改动（React 侧边栏 badge + 消息摘要）。侧边栏 badge 的视觉验证需要浏览器渲染——无法通过 tmux 终端捕获实现。单元测试全面覆盖了新逻辑（28 个 ToolGroup 测试，27 个侧边栏测试，103 个测试文件共 1627/1627 通过）。 开发服务器干净启动，无错误。PR 构建从 worktree 的 单元测试： 103 个测试文件中 1627/1627 通过。Typecheck 和 ESLint 均通过。 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 — clean across every stage; review feedback addressed, all tests green. The re-run confirms the author addressed both actionable suggestions from the prior review: active AskUserQuestion summaries now render standalone "Provide information" (no more "Running Provide information"), and badge text opacity was bumped to 95% for AA contrast compliance. The implementation is minimal (5 files, +135/-18), the CSS follows existing conventions, i18n is complete, and all 1627 web-shell tests pass with clean typecheck and lint. The two deferred suggestions — collapsed-sidebar dot indicator and sidebar render test — are correctly scoped out: the first is a separate feature, and the second was explicitly deferred per the resolved review thread. Neither blocks this PR. Ships it. ✅ 中文说明置信度：5/5 — 各阶段均通过；审查反馈已处理，所有测试通过。 重新运行确认作者处理了上一轮审查的两个可操作建议：运行中的 AskUserQuestion 摘要现在独立显示"补充信息"（不再出现"正在执行 补充信息"），badge 文字不透明度提升到 95% 以满足 AA 对比度要求。实现精简（5 个文件，+135/-18），CSS 遵循现有规范，i18n 完整，全部 1627 个 web-shell 测试通过，typecheck 和 lint 均干净。 两个延后的建议——折叠侧边栏圆点指示器和侧边栏渲染测试——范围划分正确：前者是独立功能，后者已在已解决的审查线程中明确延后。均不阻塞本 PR。 可以合入。✅ — Qwen Code · qwen3.7-max Reviewed at |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Downgraded from Approve: CI failing (route).
— qwen3.7-max via Qwen Code /review
Code Review — feat(web-shell): show sessions awaiting user actionReviewed the diff (5 files, +133/−14). Overall this is a clean, well-scoped change that reuses existing tokens/patterns and ships matching EN/ZH strings and unit tests. No blocking issues found; a few minor suggestions below. Overview
What's correct / done well
Suggestions (all non-blocking)
Security / performance
中文小结改动清晰、范围合理，复用了已有的设计 token 与样式模式，EN/ZH 文案与单测均已补齐，未发现阻塞性问题。
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
✅ Local runtime verification — PR #6956I built the branch and drove the real Before → After (real component, headless Chromium)The core win is visible immediately: on
Localization (English / 中文)
How it was verifiedBoth screenshots come from the actual
Detail 1 — badge state matrix (real component)Driving the real component with sessions in every attention state produced:
This matches the code: Detail 2 — mutation testing (proves the tests have teeth)To confirm the suites would actually catch a regression, I introduced deliberate bugs and re-ran:
All mutations were reverted; the working tree matches the PR head. Detail 3 — how to reproduce locally# unit + mutation surface
cd packages/web-shell
npx vitest run client/components/messages/ToolGroup.test.tsx # 28 pass
npx vitest run client/components/sidebar/ # existing sidebar suites pass
# real-browser render: mount WebShellSidebar with faked daemon hooks under
# Vite dev, drive with Playwright, screenshot ?v=before|after & ?lang=en|zh-CNThe browser harness fakes only the five daemon-SDK hooks the sidebar reads ( Notes / nits (non-blocking)
中文版本（点击展开）✅ 本地真实运行验证 —— PR #6956我在 Linux 上构建了该分支，并真实驱动了 改动前 → 改动后（真实组件，headless Chromium）核心价值一眼可见：在 （对比图见上方 本地化（英文 / 中文）（中英对照图见上方 验证方式两张截图均来自真实的
状态矩阵（真实组件）：
与代码一致： 变异测试（证明测试有效）：
说明 / 小建议（不阻塞合并）
Verified locally on Linux against the PR head merged with |
|
Addressed the actionable review suggestions in |
Add a `sidebar attention` scenario: four sessions in distinct states — waiting-on-permission, waiting-on-user-question, running, idle — so the sidebar renders #6956's "Waiting for approval" / "User input needed" attention pills. Renders in light and dark; asserts on session names (present with or without the pills) so the frame is the same shape on main and the PR, letting the before/after preview surface the pills.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: chunk 1 — no agent reported covering these; nobody read them.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Approve — clean, well-scoped web-shell UI change; no blocking issues.
Verified:
- Both
isWaitingForPermission/isWaitingForUserQuestioncan be set at once (bridgependingInteractions); giving permission priority is a sound choice, and the blue variant is correctly gated to the pure-question state. - Sidebar three-way render (badge / spinner / time) is correct; running spinner is retained alongside the badge as described.
getAskUserQuestionCountfallback is safe andargs.questionshas a real data source (merged from remembered permission info), so completed summaries show the true count rather than always falling back to 1.- Types are correct (
DaemonSessionSummarydeclares both fields); CSS tokens exist for light/dark; layout truncates the session name so the fixed-width badge can't overflow. - i18n complete in EN + ZH; old
askedUserkey fully removed with no remaining consumers. ToolGroup.test.tsxpasses locally (28/28), covering active / completed / multi-question / both fallback paths.
Non-blocking nits (fine to defer): the badge <span> carries a redundant aria-label identical to its visible text; and the sidebar DOM has no regression test (already acknowledged by the author as an intentional out-of-scope gap).
CI note: the only red check, review-pr, ran exactly 30m10s — a workflow timeout of the automated reviewer, not a code defect. All functional checks (unit tests, typecheck/lint, E2E smoke, visuals) are green.
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. ✅
…wenLM#6964) * test(web-shell): add a mermaid diagram visual scenario Add a `mermaid diagram` scenario to the visuals suite so the preview covers the Mermaid rendering surface — an assistant message with a mermaid fenced flowchart. It renders the real MermaidBlock (async mermaid import, injected <svg>) in light and dark, waiting on the rendered SVG so the capture is never the "rendering…" placeholder. This is the surface the diagram zoom/pan work (QwenLM#6881) enriches, so once the before/after preview lands it gives that PR a real before/after target instead of an unrelated canned screenshot. * test(web-shell): add a split-view (+ maximize) visual scenario Add a `split view` scenario: enter the two-pane split via the `?split=a,b` deep link, then maximize one pane (QwenLM#6951). Captures the tiled state (both panes, with the maximize controls) and the maximized state (one pane filling, restore control) in light and dark, driving the real SplitView against the mock daemon serving two sessions. * test(web-shell): add a sidebar attention-badge visual scenario Add a `sidebar attention` scenario: four sessions in distinct states — waiting-on-permission, waiting-on-user-question, running, idle — so the sidebar renders QwenLM#6956's "Waiting for approval" / "User input needed" attention pills. Renders in light and dark; asserts on session names (present with or without the pills) so the frame is the same shape on main and the PR, letting the before/after preview surface the pills. * test(web-shell): derive the split view's second session from the scenario list Addresses a review suggestion: the split view test hardcoded the 'previous-session' id, which only worked because it is in createWebShellDaemonScenario's default sessions list. Derive the second pane's session from the scenario's own list instead (and throw a clear error if absent), so a future rename/removal of that default surfaces as a self-explaining failure rather than a confusing SSE connection timeout. * test(web-shell): tidy split copy and mermaid width in visual scenarios Address review nits on the visual scenarios: - Split scenario: the mock replays the same events into both panes, so "Here is the first pane of the split." read wrong in the second pane. Use pane-neutral copy ("Here are the two sessions, side by side."). - Mermaid scenario: the flowchart's rightmost node clipped at the code-block edge at the 1280px capture viewport. Shorten the node labels (same nodes and flow) so the whole diagram fits with margin. Re-ran both scenarios (light + dark) locally: 4/4 pass, and confirmed in the captures that the diagram no longer clips and the neutral copy reads correctly in both panes. * test(web-shell): capture split-view restore and assert all sidebar sessions Address review nits on the visual scenarios: - Split view: after maximize, click "Restore pane" and capture the restored tiled layout, asserting the maximize control returns on both panes — so a regression in the restore path is caught, not just the tiled and maximized states. - Sidebar attention: assert all four session names render (not just the two waiting ones). The running session is also the loaded one, so its name also shows in the main view — scope the running/idle checks to the sidebar landmark so the match stays unambiguous. --------- Co-authored-by: wenshao <wenshao@example.com>
4 participants
What this PR does
Adds localized attention badges to Web Shell sidebar sessions that are waiting for permission approval or user input. Permission requests use a green “Waiting for approval” badge, while AskUserQuestion requests use a blue “User input needed” badge. AskUserQuestion message summaries now prompt for additional information while active and report the number of questions after completion.
Why it's needed
The session API already exposes whether a running session is blocked on permission approval or an AskUserQuestion response, but the sidebar previously showed the same spinner for both states. Users had to open sessions individually to discover which action was required, and completed AskUserQuestion summaries did not communicate how many questions were asked.
Reviewer Test Plan
How to verify
Start one Web Shell session that pauses for tool permission and another that pauses on AskUserQuestion. Confirm the sidebar shows “Waiting for approval” for the permission session and “User input needed” for the question session, with the existing running spinner retained. Resolve an AskUserQuestion request containing multiple questions and confirm its completed message summary reports the question count. Switch between English and Chinese and confirm the corresponding localized labels.
Evidence (Before & After)
Before: blocked sessions showed only the generic running spinner, and completed AskUserQuestion messages said “Asked user”.
After: blocked sessions show distinct approval/input badges, active AskUserQuestion messages request additional information, and completed messages report “Asked N questions”.
Tested on
Environment (optional)
Web Shell targeted unit test, typecheck, ESLint, and Prettier checks.
Risk & Scope
Linked Issues
N/A
Pictures
中文说明
本 PR 的改动
为 Web Shell 侧边栏中等待权限批准或用户输入的会话增加本地化状态标签。权限请求使用绿色“等待批准”标签，AskUserQuestion 请求使用蓝色“需要用户输入”标签。AskUserQuestion 消息在运行中提示补充信息，完成后显示询问的问题数量。
改动原因
会话接口已经提供运行中会话是在等待权限批准还是 AskUserQuestion 回复，但侧边栏此前对两种状态都只显示相同的 spinner。用户必须逐个打开会话才能知道需要执行什么操作，完成后的 AskUserQuestion 摘要也没有显示询问了多少个问题。
Reviewer 测试计划
验证方式
分别启动一个暂停等待工具权限的 Web Shell 会话和一个暂停等待 AskUserQuestion 回复的会话。确认侧边栏对权限会话显示“等待批准”，对提问会话显示“需要用户输入”，并保留已有的运行 spinner。完成一个包含多个问题的 AskUserQuestion 请求，确认完成消息显示问题数量。切换中英文，确认对应文案正确。
前后对比
改动前：阻塞中的会话只显示通用运行 spinner，完成后的 AskUserQuestion 消息显示“已询问用户”。
改动后：阻塞中的会话分别显示权限/输入标签，运行中的 AskUserQuestion 消息提示补充信息，完成消息显示“已询问 N 个问题”。
测试平台
环境
运行了 Web Shell 定向单元测试、typecheck、ESLint 和 Prettier 检查。
风险与范围
关联 Issue
无。