fix(web-shell): respect voice enabled setting - #7345
Merged
Conversation
Collaborator
Author
E2E test reportEnvironment: macOS, Node.js 22 workspace install, Playwright mock-daemon harness. Scenarios verified
Results
The smoke run emitted the mock harness's existing best-effort proxy connection noise for |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: observed code gap. The voice button was gated only by the daemon Direction: aligned. Treating the workspace voice setting as the primary product gate is the right call — the daemon capability is a necessary-but-not-sufficient condition, and the workspace setting is the user-facing on/off switch. No CHANGELOG reference found for this specific gating, but the area is clearly relevant to the web-shell voice feature. Size: not applicable — all changes are in Approach: the scope feels right. One component change ( Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的代码缺陷。语音按钮此前仅受 daemon 方向：对齐。将工作区语音设置作为首要产品门槛是正确的——daemon capability 是必要但不充分条件，工作区设置才是用户侧的开关。CHANGELOG 中未找到此特定门控的引用，但该领域与 web-shell 语音功能明确相关。 规模：不适用——所有改动在 方案：范围合理。一个组件改动（ 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
Code ReviewIndependent proposal: given "voice button shows even when workspace voice is disabled," I would add a workspace voice status fetch to Comparison with the diff: the PR does exactly this, and does it cleanly. The No critical blockers found. No AGENTS.md violations. The implementation follows existing patterns (same hook usage as One observation (non-blocking): the TestingThis is a web-shell browser component — the voice button renders in the WebShell UI, not the terminal CLI, so CLI tmux before/after does not exercise this code path. Real-scenario verification via the project's unit and Playwright e2e suites instead: Unit tests (VoiceButton + ChatEditor): E2e smoke suite (Playwright, Chromium): Typecheck: clean ( 中文说明代码审查独立方案： 针对"语音按钮在工作区语音关闭时仍显示"的问题，我会在 与 diff 对比： PR 完全这样做了，且实现干净。 未发现关键阻塞问题。无 AGENTS.md 违规。实现遵循现有模式。 一个观察（非阻塞）： 测试这是 web-shell 浏览器组件——语音按钮在 WebShell UI 中渲染，不在终端 CLI 中，因此 CLI tmux before/after 无法覆盖此代码路径。改用项目的单元测试和 Playwright e2e 套件进行真实场景验证。 单元测试 27/27 通过，e2e smoke 16/16 通过，类型检查和 lint 均干净。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
|
Confidence: 5/5 — clean across every stage; would merge without hesitation. This is a textbook fix. The problem is real and verifiable in the code — the old My independent proposal matched the PR's approach almost exactly — I didn't find a simpler path it missed. The stale-response protection (client + settingsVersion binding) is the kind of detail that's easy to skip but matters for correctness when settings change rapidly. The test coverage is thorough: 8 unit tests covering enabled/disabled, pending/failed, no-capability, settings-change, and stale-response paths; a ChatEditor integration test for toolbar gating; and a Playwright e2e test that exercises the full disabled→enabled transition through the mock daemon. All 27 unit tests and 16 smoke tests pass. Typecheck and lint are clean. The design doc is a nice touch — concise, records the decision and boundaries without over-explaining. If I had to maintain this in six months, I'd thank the author. The code reads clearly, the fail-closed default is the safe choice, and the tests document the expected behavior at every boundary. 中文说明置信度：5/5 —— 每个阶段都干净，毫不犹豫地合并。 这是一个教科书式的修复。问题真实且可在代码中验证——旧的 我的独立方案与 PR 的方案几乎完全一致——没有找到更简路径。过期响应保护（client + settingsVersion 绑定）是容易跳过但对正确性很重要的细节。 测试覆盖全面：8 个单元测试覆盖启用/禁用、等待/失败、无 capability、设置变更和过期响应路径；ChatEditor 集成测试覆盖工具栏门控；Playwright e2e 测试通过 mock daemon 验证完整的禁用→启用转换。27 个单元测试和 16 个 smoke 测试全部通过。类型检查和 lint 干净。 设计文档简洁地记录了决策和边界。 — Qwen Code · qwen3.7-max Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
qwen-code-ci-bot
previously approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-8/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-8--work-qwen-code-qwen-code/subagents/09c6e904-c98b-4f5c-bd5b-3b38d4e0cb9a (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-8/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-8--work-qwen-code-qwen-code/subagents/09c6e904-c98b-4f5c-bd5b-3b38d4e0cb9a'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read. Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-8/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-8--work-qwen-code-qwen-code/subagents/09c6e904-c98b-4f5c-bd5b-3b38d4e0cb9a (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-8/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-8--work-qwen-code-qwen-code/subagents/09c6e904-c98b-4f5c-bd5b-3b38d4e0cb9a'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
— qwen3.7-max via Qwen Code /review
qwen-code-ci-bot
approved these changes
Jul 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
doudouOUC
reviewed
Jul 20, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Collaborator
Local verification (maintainer, for merge reference)I built and ran this PR locally against the PR head ( Behavior (E2E screenshot)Daemon advertises
Captured from a real Chromium run driving the WebShell dev server against the mock daemon. Results
Negative control (RED / GREEN)To confirm the new tests actually gate on the fix, I swapped
This is the behavior the PR intends: advertising I also confirmed the capability short-circuit (no Environment / notes
Looks good to me for merge from the WebShell side. ✅ 中文说明本地验证（维护者，供合并参考）我在本地基于 PR head（ 行为（E2E 截图）下面两种情况中，daemon 都声明了
截图来自真实 Chromium 运行，驱动 WebShell dev server 对接 mock daemon。 结果
反向对照（RED / GREEN）为确认新增测试确实依赖本次修复，我把
这正是本 PR 的目标行为：仅声明 我还通过单测确认了 capability 短路（无 环境 / 说明
从 WebShell 侧看，可以合并。✅ 🤖 Generated with Claude Code — Claude Opus 4.8 (1M context) |
wenshao
approved these changes
Jul 20, 2026
ytahdn
approved these changes
Jul 21, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
9 tasks
Collaborator
|
Released in v0.20.1. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This change makes the WebShell voice dictation button visible only when voice is explicitly enabled for the workspace, the embedding host allows the voice toolbar action, and the daemon advertises voice transcription capability. Workspace setting updates trigger a fail-closed refresh, and late responses from an older setting version are ignored.
Why it's needed
The button was previously gated only by daemon capability and host toolbar configuration, so a host could surface voice dictation even when
general.voice.enabledwas missing or disabled. Treating the workspace setting as the primary product gate keeps unsupported or disabled voice entry points hidden.Reviewer Test Plan
How to verify
voice_transcribe, then returnenabled: falsefrom the workspace voice status. Confirm that the voice button is absent.enabled: true, and confirm that the voice button appears.voice_transcribeand confirm that the button stays absent without requesting workspace voice status.voicefrom the host toolbar configuration and confirm that the voice component is not mounted.Evidence (Before & After)
Before: advertising
voice_transcribewas sufficient to display the voice button even when workspace voice status returnedenabled: false.After:
enabled: falsekeeps the button absent, whileenabled: truedisplays it only when the host and capability gates also pass. The focused unit suite passed 27/27 tests, and the WebShell smoke suite passed 16/16 scenarios including the disabled-to-enabled workspace voice transition.Tested on
Environment (optional)
Node.js 22 workspace install with the WebShell Vitest and Playwright mock-daemon harnesses.
Risk & Scope
general.voice.mode, and physical microphone capture are unchanged and out of scope.Linked Issues
N/A
中文说明
本 PR 做了什么
本次修改让 WebShell 语音听写按钮只在三个条件同时满足时显示：工作区显式启用语音、宿主允许语音工具栏动作、daemon 声明语音转写能力。工作区设置更新会触发失败关闭式刷新，旧设置版本的延迟响应会被忽略。
为什么需要
此前按钮只受 daemon capability 和宿主工具栏配置控制，因此即使
general.voice.enabled缺失或关闭，宿主仍可能展示语音听写入口。将工作区设置作为首要产品门槛，可以隐藏不受支持或已关闭的语音入口。Reviewer Test Plan
如何验证
voice_transcribe，然后让工作区语音状态返回enabled: false，确认语音按钮不存在。enabled: true，确认语音按钮出现。voice_transcribe，确认按钮保持隐藏且不会请求工作区语音状态。voice，确认语音组件不挂载。前后对比证据
修改前：只要声明
voice_transcribe，即使工作区语音状态返回enabled: false，语音按钮仍会显示。修改后：
enabled: false时按钮不存在；只有enabled: true且宿主与 capability 门槛同时通过时按钮才显示。聚焦单元测试 27/27 通过，WebShell smoke 测试 16/16 通过，其中包含工作区语音从关闭到开启的场景。测试平台
环境（可选）
Node.js 22 工作区安装环境，使用 WebShell Vitest 与 Playwright mock-daemon harness 验证。
风险与范围
general.voice.mode和真实物理麦克风采集均未修改且不在本次范围内。关联 Issue
无