refactor(web-shell): restructure chat UI#5775
Conversation
|
Thanks for the PR @ytahdn! Template looks good ✓ — all required sections present and filled in. On direction: web-shell chat UI restructuring is squarely within the project's scope. The move from terminal-era layout to a chat-first experience with dialog-based command surfaces, standalone On approach: 136 files / +14,992 / -10,267 is a large diff, but the scope is coherent — it's a single UI refactor with related daemon transcript/session fixes needed to support the new rendering (server timestamp propagation, One observation: Moving on to code review. 🔍 中文说明感谢 PR @ytahdn! 模板完整 ✓ — 所有必填部分均已填写。 方向:web-shell 聊天 UI 重构完全在项目范围内。从终端时代的布局转向以聊天为主的体验,使用弹窗式命令面板、独立的 方案:136 个文件 / +14,992 / -10,267 是一个大 diff,但范围连贯——是一个统一的 UI 重构,配合必要的 daemon transcript/session 修复来支持新渲染(服务端时间戳传播、 一个观察: 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewOverall the restructuring is architecturally sound. Key design choices — standalone Previously flagged issue (resolved): The No new actionable findings. The cross-package changes are consistent:
No security issues, no hardcoded secrets, no Test ResultsAll tests pass across 5 packages (run from worktree with shared Tmux Real-Scenario TestingThis PR refactors the web-shell browser UI, which requires a running daemon + browser environment to exercise interactively. Tmux-based CLI testing (designed for terminal TUI output) is not applicable here — the changed surface is entirely in the browser. The 967 unit tests above cover the transcript normalization, message rendering, composer behavior, session provider logic, and rewind/branch flows that underpin the UI. 中文说明代码审查整体重构架构合理。关键设计选择——独立的 之前标记的问题(已解决): 无新的可操作发现。 跨包改动一致:
无安全问题,无硬编码密钥,无 测试结果5 个包共 967 个测试全部通过: Tmux 真实场景测试本 PR 重构的是 web-shell 浏览器端 UI,需要运行中的 daemon + 浏览器环境才能交互测试。基于 tmux 的 CLI 测试(针对终端 TUI 输出)不适用于此——改动表面完全在浏览器中。以上 967 个单元测试覆盖了 transcript 规范化、消息渲染、composer 行为、session provider 逻辑以及 rewind/branch 流程。 — Qwen Code · qwen3.7-max |
ReflectionStepping back, this is a well-executed refactor that does what it sets out to do. The web-shell chat UI has been restructured around a proper chat editor, dialog-based command surfaces, standalone thinking messages, and collapse summaries — all architecturally coherent choices that make the chat mode the primary experience. The cross-package changes are genuinely needed to support the new UI: server timestamp propagation on The previously flagged duplication (
VerdictThis PR passes all triage stages. However, as a cross-repository fork PR with a Escalating to maintainers for final review and approval. The code is solid, tests pass, build artifacts are clean — ready for a maintainer to look at and ship. 中文说明回顾整体而言,这是一次执行良好的重构。web-shell 聊天 UI 围绕专用聊天编辑器、弹窗式命令面板、独立思考消息和折叠摘要进行了重构——所有架构选择都连贯一致,使聊天模式成为主要体验。 跨包改动确实是新 UI 所必需的: 之前标记的重复问题(
结论本 PR 通过了所有 triage 阶段。但作为来自外部 fork 的大范围 转交维护者进行最终审查和批准。 代码质量良好,测试通过,构建干净——维护者可以查看并合并。 — Qwen Code · qwen3.7-max |
|
@ytahdn This is a large UI refactor (+14.8k/-10.1k) that restructures the entire web-shell chat experience — new ChatEditor, dialog-based command surfaces, turn/tool/thinking collapse summaries, and standalone session handoff. The PR description explicitly notes that screenshots were not attached and validation relied on automated tests only. For a UI refactor of this scope, screenshots are essential review evidence — reviewers can't assess layout drift, popover clipping, compact summary readability, or narrow-screen behavior from test names alone. Could you attach a few key UI screenshots? Specifically:
中文说明这是一个大型 UI 重构 PR(+14.8k/-10.1k),全面重构了 web-shell chat 体验——新增 ChatEditor、命令迁移到弹窗、turn/tool/thinking 折叠摘要、standalone session 交接逻辑改动。PR 描述中明确写了没有附截图,仅以自动化测试作为验证依据。 对于这种规模的 UI 重构,截图是 reviewer 验证的核心证据——布局漂移、弹层裁剪、折叠摘要可读性、窄屏行为这些都无法从测试名称判断。能否补几张关键场景的 UI 截图?建议包括:
— Qwen Code |
c783e63 to
4131910
Compare
|
@pomelo-nwu thanks! Screenshots have been added. I also rebased this PR onto the latest main, resolved the conflict, and addressed the review bot's timestamp helper duplication by exporting/reusing the SDK timestamp extractor from the webui session provider. Local verification after the rebase:
|
chiga0
left a comment
There was a problem hiding this comment.
Overview
Final Verdict: Approve — Large but well-structured web-shell UI refactor with clean daemon/SDK backend changes. The transcript reducer improvements (event propagation, discrete message merging, rewind cleanup), new session actions (createSession, rewind), and chat-mode restructuring are all correctly implemented. Test coverage is comprehensive (964 tests passing across 5 packages).
Findings Summary
- Critical/Major: 0 items
- Minor: 2 items (non-blocking observations)
- Nit: 0 items
Key Observations
The PR touches three layers: daemon/session backend, SDK transcript reducer, and web-shell UI. The backend changes are focused and well-tested:
assistantDoneFromTurnEvent: Clean extraction that enrichesassistant.donewitheventId/serverTimestamp— propagates server timing to transcript blocks for accurate ordering.canMergeTextDeltagating: PreventsqwenDiscreteMessageblocks from merging into streaming text — essential for chat UI rendering where inserted messages must be standalone.- Rewind index cleanup:
truncateTranscriptBeforeBlocknow properly cleans staletoolProgressentries — prevents ghost progress indicators after rewind. promptStatusbroadened transition:idle → streamingwhen a generation signal arrives is correct for observer sessions that don't go through thewaitingstate.
Minor Observations (non-blocking)
-
createSession()vsnewSession():createSession()does NOT abort active prompts or clear the session, whilenewSession()does. This appears intentional for embedding scenarios where the host app owns navigation, but could surprise callers. The JSDoc onDaemonSessionActions.createSessioncould clarify this distinction. -
metafield openness:meta?: Record<string, unknown>onDaemonTextTranscriptBlockis deliberately open for renderer flexibility, but future consumers could accidentally rely on undocumented fields. Consider narrowing the type as usage patterns stabilize.
Additional Audit Coverage
- Bridge
rewindFilesparameter: Correctly respects caller'sreq.rewindFileswith!== falsedefault (true unless explicitly false). SDK only includes in body when explicitly provided — backward compatible. preferredCurrentModelin mappers: Ensures session-reported model overrides provider-reported model for context window lookup. Fix for context window display when model is set via session context.mid_turn_message_injectednormalization: Handles malformed data gracefully withfallbackDebug. Status rendering useslabels.midTurnInsertedfor i18n support.- Thinking block separation in
transcriptToMessages:currentThinkingIdxproperly tracks standalone thinking messages, preventing merge into assistant content.currentAssistantIdx = nullafter thinking block ensures next assistant text starts fresh. extractServerTimestampexport: Previously private, now exported for use inDaemonSessionProvider— enables server timestamp propagation to transcript blocks.- Large paste handling in
useComposerCore:LARGE_PASTE_CHAR_THRESHOLD(1000) andLARGE_PASTE_LINE_THRESHOLD(10) with batch insertion prevents editor freeze on large pastes.
This review was generated by QoderWork AI
|
Qwen Code review did not complete successfully: Qwen review timed out after 85 minutes. See workflow logs. |
|
@qwen-code /triage |
The mic was an absolute-positioned overlay anchored to the composer wrapper, so after the QwenLM#5775 chat-UI restructure it floated outside the input box (clipped at the bottom-right). Render it inside ChatEditor's `toolbarRight`, just before the send button, wiring the transcript insert to the composer core — it sits inline with the `/`, `@`, and send controls like the rest of the toolbar. Restyle to a Codex-style recording experience: an idle mic that matches the other toolbar icon buttons, and a recording pill with a live waveform (driven by the RMS meter), an elapsed `M:SS` timer, and a stop control. Also forward `/voice/stream` through the vite dev proxy (`ws: true`, scoped to the exact path so it doesn't shadow the client's own `client/voice/*` source modules) so the WebSocket reaches the daemon in dev instead of silently failing against vite. Co-authored-by: Qwen-Coder <[email protected]>
* feat(serve): voice dictation over the daemon for the Web Shell Bring voice input to the `qwen serve` Web Shell. The browser captures the microphone, streams raw 16 kHz mono PCM to a new `/voice/stream` WebSocket, and the daemon transcribes server-side by reusing the existing CLI voice pipeline (realtime streaming + on-stop batch) — provider credentials never reach the browser, and it works whether the daemon is local or remote. Daemon: - `/voice/stream` WebSocket handler (serve/voice) reusing openVoiceStream / openQwenAsrRealtimeStream / transcribeVoiceAudio; resolves the workspace voiceModel from a ModelsConfig built off settings. - Routed through the existing ACP upgrade listener so it shares the loopback / host-allowlist / CSRF / bearer checks; concurrency-capped. - Advertises the `voice_transcribe` capability (unconditional, like auth_device_flow; the WS errors when no voice model is configured). - Relax `Permissions-Policy` to `microphone=(self)` so the same-origin shell can request the mic — an empty `microphone=()` allowlist blocked the prompt entirely. - Allowlist `voiceModel` on `/workspace/settings` so the picker can read it. Web Shell: - Mic button in the composer: click to record, click to stop -> the transcript is inserted into the input for review before sending. - `/model --voice` picker (sourced from `/workspace/providers`, since voice models are hidden from the session model list) and `/model --voice <id>`, persisted via the prompt channel like `/model --fast`. The voiceModel resolver now accepts a structural model lookup so the daemon can resolve it without constructing a full CLI Config. Co-authored-by: Qwen-Coder <[email protected]> * fix(serve): harden web voice streaming * fix(serve): clean stale voice capture resources * fix(serve): harden voice websocket errors * fix(serve): hide voice capability when token auth is configured * fix(serve): address voice review blockers * fix(web-shell): keep voice cancel available when disabled * fix(web-shell): inline the voice mic in the composer toolbar The mic was an absolute-positioned overlay anchored to the composer wrapper, so after the #5775 chat-UI restructure it floated outside the input box (clipped at the bottom-right). Render it inside ChatEditor's `toolbarRight`, just before the send button, wiring the transcript insert to the composer core — it sits inline with the `/`, `@`, and send controls like the rest of the toolbar. Restyle to a Codex-style recording experience: an idle mic that matches the other toolbar icon buttons, and a recording pill with a live waveform (driven by the RMS meter), an elapsed `M:SS` timer, and a stop control. Also forward `/voice/stream` through the vite dev proxy (`ws: true`, scoped to the exact path so it doesn't shadow the client's own `client/voice/*` source modules) so the WebSocket reaches the daemon in dev instead of silently failing against vite. Co-authored-by: Qwen-Coder <[email protected]> * feat(serve): authenticate the voice WebSocket via a bearer subprotocol Voice was suppressed on any token-configured daemon because browsers cannot set an `Authorization` header on a WebSocket, so the bearer check on the shared ACP upgrade listener always failed for the browser mic. Let the browser carry the token in `Sec-WebSocket-Protocol` as `qwen-bearer.<base64url(token)>` (the only header a browser can set on a WS). The upgrade listener now accepts the token from either the `Authorization` header (non-browser clients, unchanged) or the subprotocol, hash-compared in constant time. `handleProtocols` selects a non-secret subprotocol (or none) so the token is never echoed back in the handshake response. This aligns the voice WS with how the ACP WS will authenticate browsers. With the WS now authenticated, drop the `!tokenConfigured` / `requireAuth` suppression of the `voice_transcribe` capability — it is advertised whenever the `/voice/stream` endpoint exists. Tests updated to assert voice is advertised under a token / `--require-auth`. Co-authored-by: Qwen-Coder <[email protected]> * fix(serve): make the voice WS subprotocol handshake complete + test it Review follow-up to the bearer-subprotocol auth. Fixes a real handshake bug and closes the test gap on the security-critical path. - Handshake bug: when the browser offered only the `qwen-bearer.*` subprotocol, the daemon's `handleProtocols` selected none, and a strict WS client (the `ws` library) aborts with "Server sent no subprotocol". The web-shell now offers a non-secret marker (`qwen-ws`) alongside the bearer one, so the daemon always selects the marker — never echoing the secret — and the handshake completes for strict and lenient clients alike. - Tests: add daemon-side coverage for the subprotocol auth path (accept valid token, reject wrong/malformed, no-token loopback) and assert the secret is never echoed back; add client coverage that the bearer token is offered as `[qwen-ws, qwen-bearer.<b64url>]` and round-trips. - Drop the unreachable try/catch around `Buffer.from(_, 'base64url')` (it never throws) and correct the comment. - a11y: mark the transcribing/interim regions as `role="status"` / `aria-live`, and add a `:focus-visible` ring to the idle mic. - Refresh the stale `useVoiceCapture` doc comment (token deployments are now supported in the browser) and fix the mock typing in VoiceButton.test.tsx. Co-authored-by: Qwen-Coder <[email protected]> * fix(web-shell): ignore stale voice socket callbacks * test(serve): sync voice capability integration expectation * fix(serve): harden voice websocket review gaps * fix(serve): sanitize voice websocket failures * fix(serve): address voice review suggestions * fix(serve): harden voice websocket lifecycle --------- Co-authored-by: Qwen-Coder <[email protected]>
The workspace permissions rules API (#5743) added workspacePermissions and the set/add/remove rule methods plus their types to the browser-facing DaemonClient (~718 bytes minified) without raising MAX_DAEMON_BROWSER_BUNDLE_BYTES. On top of the 125 KiB budget set in #5775, the daemon SDK bundle grew to 128651 bytes and exceeded the 128000-byte cap, breaking `npm run build` on main (E2E Tests fail at the bundle assertion). These are public class methods on DaemonClient and are not tree-shakeable, so the bytes ship regardless of consumers. Raise the cap one notch to 126 KiB (129024 bytes), following the established per-feature bump pattern. Co-authored-by: yiliang114 <[email protected]>
What this PR does
This PR restructures the web-shell chat experience around a dedicated chat editor, dialog-based command surfaces, turn/tool/thinking collapse summaries, and a cleaner standalone session handoff flow. It also includes related daemon transcript/session handling fixes needed by the new UI, including structured branch and mid-turn events, rewind support, better background task filtering, safer transcript merging, and non-destructive web-shell new-session behavior when an embedding app owns session navigation.
Why it's needed
The previous web-shell UI still carried terminal-era layout and inline command-panel assumptions, which made chat embedding fragile: footer sizing could drift, popovers could be clipped or covered, long messages and tool groups were hard to scan, and session/navigation ownership was unclear for host applications. This refactor makes the chat mode the primary web-shell experience, moves interactive command flows into consistent dialogs, improves compact summaries for long-running work, and fixes several bugs found while integrating the component into other pages.
Reviewer Test Plan
How to verify
Open the web-shell chat UI and verify the main chat flow: a new session starts centered, sending a prompt transitions to the bottom chat layout, tool/thinking/parallel-agent groups summarize and expand correctly, slash commands open the expected dialogs, quick actions work on narrow screens, and
/new,/clear, and/resetcreate a new session without cancelling the currently running prompt. For embedded usage, passonSessionIdChangeand confirm the callback receives the new session id while the host controls remount/navigation.Automated verification run locally:
cd packages/core && npx vitest run src/services/sessionService.test.ts— 66 passedcd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts src/acp-integration/session/Session.test.ts— 303 passedcd packages/sdk-typescript && npx vitest run test/daemon-ui-transcript.test.ts test/unit/daemonUi.test.ts— 246 passedcd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx— 89 passedcd packages/web-shell && npx vitest run client/build-artifact.test.ts client/completions/slashCompletion.test.ts client/adapters/transcriptAdapter.test.ts client/adapters/transcriptToMessages.test.ts client/components/MessageList.test.ts client/components/MessageList.dom.test.tsx client/components/messages/AssistantMessage.test.tsx client/components/messages/SettingsMessage.test.ts client/components/messages/ToolGroup.test.tsx client/components/messages/UserMessage.test.tsx client/components/messages/toolFormatting.test.ts client/hooks/useComposerCore.test.ts— 260 passednpm run build— passedEvidence (Before & After)
N/A for this PR description. The change is user-visible, but this submission is validated with the automated tests and build listed above rather than attached screenshots.
Tested on
Environment (optional)
Local macOS development checkout using the repository npm workspace scripts. The webui session provider test prints sandboxed
detach failed: EPERM 127.0.0.1:4170cleanup stderr in this environment, but all assertions pass.Risk & Scope
onSessionIdChangeshould own remount/navigation for newly created sessions.Linked Issues
N/A
接入方截图:








运行中:
运行结果(收起):
运行结果(展开):
相关弹窗(resume):
斜杠命令:
移动端适配:
底部快捷操作:
中文说明
What this PR does
这个 PR 围绕新的 web-shell chat 体验做了重构:新增独立的 chat editor,迁移斜杠命令和多种交互面板到统一弹窗,增加 turn/tool/thinking 的折叠摘要,并整理 standalone 场景下的 session 切换交接逻辑。同时补了新 UI 需要的 daemon transcript/session 相关修复,包括结构化 branch 和 mid-turn 事件、rewind 支持、后台任务消息过滤、transcript merge 修复,以及宿主接管 session 导航时
/new不主动取消当前运行的会话。Why it's needed
旧的 web-shell UI 还保留了较多 terminal 场景下的布局和 inline 面板假设,导致作为 chat 组件嵌入其他页面时比较脆弱:footer 高度容易漂移,弹层容易被裁剪或遮挡,长消息和工具组不易阅读,session/navigation 的归属也不够清晰。这次重构让 chat mode 成为 web-shell 的主体验,将有交互的命令流程迁移到一致的弹窗中,改善长运行任务的摘要展示,并修复了接入其他页面时发现的一批问题。
Reviewer Test Plan
How to verify
打开 web-shell chat UI,验证主流程:新会话默认居中,发送消息后切换到底部 chat 布局,工具/思考/并行智能体分组可以正确摘要和展开,斜杠命令能打开对应弹窗,窄屏更多操作可用,
/new、/clear、/reset会创建新 session 且不取消当前正在运行的 prompt。嵌入场景下传入onSessionIdChange,确认回调收到新的 session id,并由宿主控制重新挂载或导航。本地已运行自动化验证:
cd packages/core && npx vitest run src/services/sessionService.test.ts— 66 passedcd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts src/acp-integration/session/Session.test.ts— 303 passedcd packages/sdk-typescript && npx vitest run test/daemon-ui-transcript.test.ts test/unit/daemonUi.test.ts— 246 passedcd packages/webui && npx vitest run src/daemon/session/DaemonSessionProvider.test.tsx— 89 passedcd packages/web-shell && npx vitest run client/build-artifact.test.ts client/completions/slashCompletion.test.ts client/adapters/transcriptAdapter.test.ts client/adapters/transcriptToMessages.test.ts client/components/MessageList.test.ts client/components/MessageList.dom.test.tsx client/components/messages/AssistantMessage.test.tsx client/components/messages/SettingsMessage.test.ts client/components/messages/ToolGroup.test.tsx client/components/messages/UserMessage.test.tsx client/components/messages/toolFormatting.test.ts client/hooks/useComposerCore.test.ts— 260 passednpm run build— passedEvidence (Before & After)
N/A。本 PR 是用户可见改动,但本次提交说明中未附截图,主要以以上自动化测试和构建结果作为验证依据。
Tested on
Environment (optional)
本地 macOS 开发仓库,使用仓库 npm workspace 脚本验证。webui session provider 测试在当前沙箱环境中会输出
detach failed: EPERM 127.0.0.1:4170的 cleanup stderr,但所有断言均通过。Risk & Scope
onSessionIdChange的宿主需要自行处理新 session 的重新挂载或导航。Linked Issues
N/A