Skip to content

fix(cli): probe microphone permission on recording start, not voice warmup - #8912

Open
Nas01010101 wants to merge 2 commits into
QwenLM:mainfrom
Nas01010101:fix/voice-mic-permission-on-record
Open

fix(cli): probe microphone permission on recording start, not voice warmup#8912
Nas01010101 wants to merge 2 commits into
QwenLM:mainfrom
Nas01010101:fix/voice-mic-permission-on-record

Conversation

@Nas01010101

@Nas01010101 Nas01010101 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Voice warmup no longer asks the operating system about microphone permission. Preloading the recorder backend and probing the permission were the same step, so a session that had voice dictation configured queried the microphone the moment the input prompt appeared. Those two jobs are now separate: warmup only preloads the backend, and the permission probe runs when a recording actually starts.

The state that keeps the notice from repeating also moves. It used to live inside the input prompt component, so it was reset whenever that component remounted and the same notice could be appended again. It now lives in the app container, which stays mounted across dialog swaps and layout switches, and reaches the input prompt through the UI state — the route mainControlsRef already uses — as an optional prop with a local fallback.

Why it's needed

On macOS an undetermined TCC status is reported as prompt, which is the normal state for anyone who has never dictated. Because the probe ran at warmup, every startup of a session with a voice model configured appended "Voice dictation needs microphone access" to the chat history — including for users who never press the voice key. A remount of the input prompt during startup reset the per-instance guard, so the notice frequently arrived twice.

After this change the notice reaches only someone who is trying to dictate, and it arrives once per run.

Reviewer Test Plan

How to verify

Configure a voice model so voice dictation is enabled, do not grant microphone access to the terminal (or reset it with tccutil reset Microphone <terminal bundle id>), and start the CLI on macOS.

  • Expected before: the chat history contains "Voice dictation needs microphone access …" immediately at startup, often twice, without any interaction.
  • Expected after: startup is quiet. The notice appears the first time you start a recording with the voice key, and only that first time — repeated recordings, and remounts of the input prompt, do not repeat it.

Automated coverage, run from the repo root:

npx vitest run --root packages/cli --coverage=false src/ui/components/InputPrompt.test.tsx src/ui/hooks/use-voice-input.test.ts src/ui/components/Composer.test.tsx src/ui/voice/voice-recorder.test.ts

New tests in packages/cli/src/ui/components/InputPrompt.test.tsx (voice microphone permission):

  • does not probe or warn about microphone permission during warmup
  • warns about a pending permission when a recording starts
  • reports a denied permission as an error when a recording starts
  • warns only once for repeated recordings with the same status
  • warns only once across remounts when a session ref is supplied
  • stays quiet when the recorder cannot report permission
  • stays quiet when the permission probe rejects
  • stays quiet when permission is already granted
  • warns again when the permission status changes between recordings

And in packages/cli/src/ui/hooks/use-voice-input.test.ts: does not check microphone permission until a recording starts. In packages/cli/src/ui/components/Composer.test.tsx: forwards the session voice permission ref across input-active toggles.

Evidence (Before & After)

The behavior change is a message that is absent rather than present, so the evidence is the same tests run against the old and the new source.

Before — new tests against the pre-change source (the three source files reverted to main, the tests kept):

 FAIL  src/ui/components/InputPrompt.test.tsx > InputPrompt > voice microphone permission > warns only once across remounts when a session ref is supplied
AssertionError: expected "spy" to be called 1 times, but got 0 times
 FAIL  src/ui/hooks/use-voice-input.test.ts > use-voice-input > does not check microphone permission until a recording starts
AssertionError: expected "spy" to be called 1 times, but got 0 times

 Test Files  2 failed (2)
      Tests  6 failed | 3 passed | 227 skipped (236)

After — the same command on this branch:

 ✓ src/ui/hooks/use-voice-input.test.ts (24 tests | 23 skipped) 15ms
 ✓ src/ui/components/InputPrompt.test.tsx (212 tests | 204 skipped) 1053ms

 Test Files  2 passed (2)
      Tests  9 passed | 227 skipped (236)

The suites for every file this change touches, unfiltered:

 Test Files  4 passed (4)
      Tests  264 passed (264)

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

macOS is where the reported behavior occurs (the issue is labelled scope/macos) and is where this was verified. Windows and Linux were not run locally; the change is platform-independent TypeScript and CI covers those legs.

Environment (optional)

macOS, Node 22, npm run test:ci in packages/cli plus the targeted suites above.

Risk & Scope

  • Main risk or tradeoff: someone whose microphone is already denied now learns about it at the moment they press the voice key rather than at startup. That is the intended trade — the notice is delivered when it is actionable instead of to everyone. The denied path still surfaces as an error item, unchanged.
  • Not validated / out of scope: the native permission probe itself (packages/audio-capture) is untouched, as is the Web Shell and desktop voice path. No change to when or how warmup preloads the backend.
  • Breaking changes / migration notes: none. The new checkMicrophonePermission hook argument and the voiceMicWarnedStatusRef prop are both optional, so callers that do not pass them keep working — with the notice falling back to once per component instance.

Linked Issues

Fixes #8877

中文说明

这个 PR 做了什么

语音预热(warmup)不再向操作系统查询麦克风权限。以前预加载录音后端和探测权限是同一个步骤,因此只要会话配置了语音模型,输入框一出现就会去查询麦克风。现在这两件事被拆开:预热只负责预加载后端,权限探测则在真正开始录音时才执行。

用于避免重复提示的状态也上移了。它原先保存在输入框组件内部,组件重新挂载时会被重置,同一条提示可能再次被追加。现在它保存在 app container 中——它在对话框切换和布局切换期间始终保持挂载——并经由 UI state 传递到输入框(与 mainControlsRef 相同的路径),仍然是带本地兜底的可选 prop。

为什么需要

在 macOS 上,未决定(undetermined)的 TCC 状态会被报告为 prompt,而这正是从未使用过听写的用户的正常状态。由于探测发生在预热阶段,配置了语音模型的会话每次启动都会向聊天记录追加“Voice dictation needs microphone access”——即使用户从不按语音键。启动过程中输入框的一次重新挂载会重置每个实例的去重标记,因此该提示经常出现两次。

改动之后,只有真正尝试听写的用户才会看到该提示,并且每次运行只出现一次。

审阅者验证步骤

配置语音模型以启用语音听写,不要授予终端麦克风权限(或用 tccutil reset Microphone <terminal bundle id> 重置),然后在 macOS 上启动 CLI。

  • 改动前的预期:无需任何交互,启动时聊天记录中就会出现“Voice dictation needs microphone access …”,而且常常出现两次。
  • 改动后的预期:启动时不会有任何提示。第一次用语音键开始录音时才出现该提示,且仅出现这一次——重复录音以及输入框重新挂载都不会再次出现。

自动化覆盖,在仓库根目录执行:

npx vitest run --root packages/cli --coverage=false src/ui/components/InputPrompt.test.tsx src/ui/hooks/use-voice-input.test.ts src/ui/components/Composer.test.tsx src/ui/voice/voice-recorder.test.ts

packages/cli/src/ui/components/InputPrompt.test.tsx 中新增的测试(voice microphone permission):

  • 预热期间不探测、也不提示麦克风权限
  • 开始录音时对待决权限给出提示
  • 开始录音时把已拒绝的权限报告为错误
  • 状态相同的重复录音只提示一次
  • 提供会话级 ref 时,跨重新挂载也只提示一次
  • 录音器无法报告权限时保持安静
  • 权限探测被 reject 时保持安静
  • 权限已授予时保持安静
  • 两次录音之间权限状态发生变化时再次提示

以及 packages/cli/src/ui/hooks/use-voice-input.test.ts 中的:在开始录音之前不检查麦克风权限。packages/cli/src/ui/components/Composer.test.tsx 中的:跨输入激活切换转发同一个会话级权限 ref。

证据(前后对比)

这次的行为变化是“某条消息不再出现”,因此证据是同一批测试分别对旧代码和新代码运行的结果。

改动前——把三个源文件回退到 main、保留新测试:

 Test Files  2 failed (2)
      Tests  6 failed | 3 passed | 227 skipped (236)

改动后——本分支上执行同一条命令:

 Test Files  2 passed (2)
      Tests  9 passed | 227 skipped (236)

本次改动涉及的所有文件的测试:

 Test Files  4 passed (4)
      Tests  264 passed (264)

测试平台

macOS 已验证(该问题标记为 scope/macos,也正是在 macOS 上复现)。Windows 与 Linux 未在本地运行;改动是与平台无关的 TypeScript,由 CI 覆盖这两条腿。

风险与范围

  • 主要权衡:麦克风已被拒绝的用户现在会在按下语音键时才得知,而不是在启动时。这是有意的取舍——提示在可操作的时刻送达,而不是发给所有人。denied 分支仍然作为错误项呈现,未作改动。
  • 未验证 / 不在范围内:原生权限探测本身(packages/audio-capture)未改动,Web Shell 与桌面端语音路径同样未改动。预热预加载后端的时机与方式没有变化。
  • 破坏性变更 / 迁移说明:无。新增的 checkMicrophonePermission hook 参数与 voiceMicWarnedStatusRef prop 都是可选的,不传的调用方行为不变——提示退化为每个组件实例一次。

关联 issue

Fixes #8877

…armup

Voice warmup called recorder.microphoneStatus() as soon as the input
prompt mounted with voice dictation configured. On macOS an undetermined
TCC status maps to 'prompt', so every startup appended a "Voice dictation
needs microphone access" notice to the chat history, including for users
who never record.

warmupVoice now only preloads the recorder backend. The permission probe
and its 'denied'/'prompt' notices move to a checkMicrophonePermission
callback that useVoiceInput invokes from startRecording, so the notice
reaches only users who are actually trying to dictate.

The dedup ref moves up to Composer and reaches InputPrompt as an optional
prop, matching clipboardUnavailableShownRef. A per-instance ref reset on
every InputPrompt remount, which is what produced the duplicate notice.

Fixes QwenLM#8877
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on 94b6444 and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— 94b6444 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all sections filled in, bilingual, with a before/after test evidence section.

Problem: observed bug, not theoretical. The linked issue #8877 carries a screenshot of the warning appearing twice at startup, is triaged as type/bug / priority/P2 / scope/macos, and includes a root-cause analysis. The bug is real and well documented.

Direction: aligned. The issue itself prescribes exactly this fix — move the permission probe out of warmup into the recording-start path — and voice dictation UX is an actively maintained area (Claude Code's changelog shows a steady stream of voice-dictation fixes: mic-failure handling, misleading messages, spurious recordings), so polishing when the notice appears is squarely in scope.

Size: not applicable — no core-module paths are touched. All changes live in packages/cli/src/ui/**: 37 production lines (Composer.tsx +5, InputPrompt.tsx +18/−2, use-voice-input.ts +12) and 229 test lines.

Approach: the scope feels right. Splitting warmup (preload only) from the permission probe, and lifting the once-per-run dedup ref into Composer with a local fallback, is exactly the minimal shape of this fix — and the lifted-ref arrangement mirrors the existing clipboardUnavailableShownRef pattern already in Composer, so it adds no new idiom. No drive-by changes spotted.

Risk: no elevated risk signals — none of the changed files match the revert-prone paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 各部分齐全,中英双语,并附有前后对比的测试证据。

问题:已观测到的 bug,而非理论性问题。关联 issue #8877 附有启动时提示出现两次的截图,已标记为 type/bug / priority/P2 / scope/macos,并包含根因分析。问题真实存在且记录充分。

方向:对齐。issue 本身就给出了同样的修复思路——把权限探测从预热移到录音开始路径——并且语音听写 UX 是持续维护的领域(Claude Code 的 changelog 中有一系列语音听写修复:麦克风失败处理、误导性消息、误触发录音),因此打磨提示出现的时机完全在范围内。

规模:不适用 —— 未触及核心模块路径。所有改动都在 packages/cli/src/ui/**:生产代码 37 行(Composer.tsx +5、InputPrompt.tsx +18/−2、use-voice-input.ts +12),测试 229 行。

方案:范围合理。把预热(仅预加载)与权限探测拆开,并把"每次运行只提示一次"的去重 ref 上移到 Composer、保留本地兜底,正是这个修复的最小形态——而且上移 ref 的做法与 Composer 中已有的 clipboardUnavailableShownRef 模式一致,没有引入新的惯用法。未发现夹带的无关改动。

风险:无升级风险信号 —— 改动文件均未命中易回滚路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at 94b64446477e71facc51a24fe78fda10c308402a · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Code review

Read against the diff at the reviewed commit. The approach is the right one, and it's implemented cleanly — no blockers found.

The probe now fires exactly where it should. startRecording in use-voice-input.ts is the single entry point for every recording start — hold mode, tap mode, and the streaming path all go through it — so calling checkMicrophonePermission?.() at its top covers all of them. The probe is fire-and-forget (void Promise.resolve(...).catch(() => {})), so it never delays recorder.start().

The notice logic moved verbatim. The denied → error item / prompt → info item handling and the voiceMicWarnedStatusRef.current === status dedup are unchanged; only the trigger moved from warmup to recording start. The ref's type was tightened from string to MicrophonePermission along the way.

The lifted dedup ref is necessary, not decoration. Moving the probe to recording start fixes the startup spam, but a mid-session InputPrompt remount would reset a per-instance ref and let the notice repeat on the next recording. Composer persists across InputPrompt remounts, and the arrangement mirrors the existing clipboardUnavailableShownRef pattern already there, so it introduces no new idiom. The optional prop + local fallback keeps the single production call site backward compatible.

Small incidental improvement: warmupVoice's dependency list dropped uiState.historyManager, so the warmup effect no longer re-runs when historyManager identity churns — warmup only re-fires when enabled/voiceModel change, as intended.

Tests pin the behavior. The 8 new InputPrompt tests invoke the real checkVoiceMicPermission/warmup callbacks (captured from the mocked hook's args) across every branch: no probe during warmup, warn on prompt at record start, error on denied, dedup across repeated recordings, dedup across remounts with the session ref, and silence when the recorder can't report status, when the probe rejects, or when status is granted. The new hook test asserts the probe is not called until the voice key is pressed. Both new arguments are optional, so existing call sites are unaffected. Style matches house conventions (kebab-case files, type-only imports, colocated tests, why-comments).

Test evidence

This is an unattended CI run, so no PR code was built or executed here — the evidence below is the PR's own CI, read through the checks API at the reviewed commit (fetched once, no polling; the unit suite runs ~30 minutes). The Qwen Triage Finalize job will update the table below in place once CI settles.

Final CI results for 94b6444 (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The macOS/Windows/Integration legs are skipped on every PR by repo design (they run only in the merge queue — see the comment above test_macos in .github/workflows/ci.yml), not because of this change; ubuntu is the PR-time gate.

The PR body's before/after test output (new tests failing 6/9 against the pre-change source, passing on the branch) is the author's claim, not independently re-run here — this review cannot execute PR code. The test design itself does support the claim: the assertions describe post-change behavior ("does not probe during warmup", "not called until a recording starts"), which the old code cannot satisfy.

Sandboxed verification would settle the remaining gap: @qwen-code /verify — as a sponsored run (a maintainer's comment approves the head; fork authors can't trigger it), it would independently confirm the before/after claim by A/B-running the new tests against the base build. The macOS-specific TCC behavior itself (undetermined status surfacing as prompt at first record) is out of reach of this repo's Linux CI either way.

中文说明

代码审查

基于所审 commit 的 diff 阅读。方案正确,实现干净——未发现阻塞问题。

探测现在出现在正确的位置。use-voice-input.ts 中的 startRecording 是所有录音开始的唯一入口——按住模式、点按模式和流式路径都经过它——因此在其开头调用 checkMicrophonePermission?.() 覆盖了全部路径。探测是"发射后不管"(fire-and-forget),不会阻塞 recorder.start()

提示逻辑原样搬移。denied → 错误项 / prompt → 信息项的处理与 voiceMicWarnedStatusRef.current === status 去重均未改动,只是触发点从预热移到了录音开始。ref 类型顺带从 string 收紧为 MicrophonePermission

上移去重 ref 是必要的,不是装饰。把探测移到录音开始解决了启动刷屏,但会话中途 InputPrompt 重新挂载会重置实例内的 ref,使下一次录音再次出现提示。Composer 在 InputPrompt 重挂载时持续存在,且该做法与其中已有的 clipboardUnavailableShownRef 模式一致,没有引入新惯用法。可选 prop + 本地兜底保持了唯一生产调用点的向后兼容。

附带的小改进:warmupVoice 的依赖列表去掉了 uiState.historyManager,预热不再因 historyManager 标识变化而重复执行。

测试钉住了行为。8 个新的 InputPrompt 测试通过被 mock hook 的入参调用真实回调,覆盖所有分支;新的 hook 测试断言按下语音键前不会探测。两个新参数均为可选,现有调用点不受影响。风格符合仓库约定。

测试证据

这是无人值守的 CI 运行,此处未构建或执行任何 PR 代码——以下证据是通过 checks API 在审阅 commit 上读取的 PR 自身 CI(一次性获取,不轮询;单元套件约需 30 分钟)。CI 结束后 Qwen Triage Finalize 会就地更新下表。

macOS/Windows/集成腿在每个 PR 上都跳过是仓库设计使然(仅在合并队列运行),与本改动无关;ubuntu 是 PR 阶段的门禁。

PR 正文中的前后对比测试输出(新测试在旧代码上 6/9 失败、在分支上通过)是作者的声明,此处未独立复跑——本审查不能执行 PR 代码。测试设计本身支持该声明:断言描述的是改动后的行为,旧代码无法满足。

沙箱验证可以补上剩余缺口:@qwen-code /verify——以赞助运行方式(由维护者评论触发),通过对 base 构建做 A/B 独立证实前后对比的声明。macOS 特有的 TCC 行为(未决定状态在首次录音时以 prompt 呈现)无论如何都在本仓库 Linux CI 的能力范围之外。

Qwen Code · qwen3.8-max

Reviewed at 94b64446477e71facc51a24fe78fda10c308402a · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — a minimal, well-tested fix that does exactly what the linked issue asked for; the only reservations are that CI hasn't settled yet and the before/after test evidence is the author's (unreproducible here by design).

Stepping back: this is what a good bugfix PR looks like. The problem is observed and triaged (#8877, screenshot, P2), the fix is the exact shape the issue prescribed — warmup preloads the backend, the permission probe moves to recording start, and the once-per-run dedup state moves up to Composer following the pattern the clipboard notice already uses. 37 production lines, no drive-by edits, and every one of them is load-bearing: cutting the ref lift would leave the notice repeating across InputPrompt remounts mid-session, so the "80% cut" version would actually be incomplete.

The tests are the strongest part. They don't just assert the happy path — they walk every branch of the notice logic (prompt/denied/granted, probe rejection, recorder without microphoneStatus, dedup across recordings and across remounts) and assert post-change behavior the old code structurally cannot satisfy, which is what makes the author's claim that they fail on main credible even though this review can't re-run it.

My one honest reservation, beyond pending CI: the end-to-end macOS behavior (undetermined TCC status surfacing as prompt exactly when the first recording starts) can't be exercised by this repo's Linux CI, so final confidence rests on the unit-level contract plus a maintainer-triggered @qwen-code /verify sponsored run if anyone wants the before/after claim A/B-proven. Neither blocks.

Verdict: approve. CI for the unit suite is still running at review time, so approval is deferred until it lands green on the reviewed commit.

中文说明

这是一个规范的 bugfix PR 应有的样子:问题已观测并经过分诊(#8877,附截图,P2);修复正是 issue 所建议的形态——预热只预加载后端,权限探测移到录音开始时,"每次运行只提示一次"的去重状态上移到 Composer,沿用剪贴板提示已有的模式。生产代码 37 行,无夹带改动,且每一行都必要:若砍掉 ref 上移,会话中途 InputPrompt 重挂载时提示仍会重复,因此"砍掉 80%"的版本其实并不完整。

测试是最强的部分:覆盖了提示逻辑的所有分支,并断言旧代码在结构上无法满足的改动后行为——这使作者"新测试在 main 上失败"的声明可信(本审查按设计无法复跑)。

唯一保留意见(除 CI 未定之外):端到端的 macOS 行为(未决定的 TCC 状态在首次录音时以 prompt 呈现)无法在本仓库的 Linux CI 上运行,最终信心落在单元级契约上;如需独立证实前后对比,可由维护者触发 @qwen-code /verify 赞助运行。两者均不构成阻塞。

结论:批准。审阅时单元套件 CI 仍在运行,因此批准推迟到其在所审 commit 上变绿后自动执行。

Qwen Code · qwen3.8-max

Reviewed at 94b64446477e71facc51a24fe78fda10c308402a · re-run with @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 — CI landed green after the review. ✅

@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.

Reviewed. Suggestions are inline.

Not explored to full depth (tool budget reached): One-sentence change summary: This PR defers the microphon...: none — all checks above completed within budget.; One-sentence change summary: This PR defers the microphon...: could not execute InputPrompt.test.tsx / use-voice-input.test.ts to confirm they pass — the review worktree has no node_modules (root and packages/cli c…; One-sentence change summary: This PR defers the microphon...: could not run InputPrompt.test.tsx / use-voice-input.test.ts or npm run typecheck — the review worktree has no node_modules and a full monorepo install …; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget..

Test Plan (not a blocker): src/ui/components/InputPrompt.test.tsxno such file or directory; src/ui/hooks/use-voice-input.test.tsno such file or directory; Tests 9 passed — this review observed 18917 passed; Tests 264 passed — this review observed 18917 passed; 3 passed — this review observed 18917 passed.

中文说明

已审查。 建议见行内评论。

未探索到全部深度(达到工具调用预算):One-sentence change summary: This PR defers the microphon...:none — all checks above completed within budget.;One-sentence change summary: This PR defers the microphon...:could not execute InputPrompt.test.tsx / use-voice-input.test.ts to confirm they pass — the review worktree has no node_modules (root and packages/cli c…;One-sentence change summary: This PR defers the microphon...:could not run InputPrompt.test.tsx / use-voice-input.test.ts or npm run typecheck — the review worktree has no node_modules and a full monorepo install …;You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.

Test Plan(非阻断):src/ui/components/InputPrompt.test.tsxno such file or directory; src/ui/hooks/use-voice-input.test.tsno such file or directory; Tests 9 passed — this review observed 18917 passed; Tests 264 passed — this review observed 18917 passed; 3 passed — this review observed 18917 passed

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +66 to +68
// Held here rather than in InputPrompt so the microphone-permission notice
// is shown once per run instead of again on every InputPrompt remount.
const voiceMicWarnedStatusRef = useRef<MicrophonePermission | null>(null);

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.

[Suggestion] The warn-once ref now lives in Composer, but Composer itself is conditionally mounted, so the "once per run" guarantee this comment asserts is actually "once per Composer mount". DefaultAppLayout renders DialogManager instead of Composer whenever uiState.dialogsVisible is true — which includes every tool-approval confirmationRequest in default approval mode, plus auth, folder-trust, MCP-approval, settings and stats dialogs — and agent-tab / ScreenReader layout switches swap Composer out as well. Each such round trip discards the ref. — Failure scenario: on macOS with TCC status notDetermined, the user presses the voice key → the notice is appended once and the OS prompt is not approved → any later tool-approval dialog unmounts Composer and resets the ref → the next recording start re-appends the identical notice. This is the duplicate-warning shape the #8877 discussion asked the fix to avoid ("If a dedup ref is kept, it needs to live somewhere that survives remounts"). Suggested fix: hold the ref above the dialog/layout swap (e.g. create it in AppContainer, which owns dialogsVisible and never unmounts, and thread it down), or attach it to a singleton at that level — plus a test that unmounts/remounts Composer between recordings. If the Composer-level lifetime is deliberate, change the comment to "once per composer mount" so the next reader does not rely on a guarantee the code does not provide.

中文说明

[Suggestion] 去重 ref 现在保存在 Composer 中,但 Composer 本身是条件挂载的,因此该注释声称的“每次运行只提示一次”实际上是“每次 Composer 挂载只提示一次”。只要 uiState.dialogsVisible 为 true,DefaultAppLayout 就会渲染 DialogManager 而不是 Composer——这包括默认审批模式下每一次工具审批的 confirmationRequest,以及 auth、folder-trust、MCP 审批、settings、stats 等对话框——agent 标签页 / ScreenReader 布局切换同样会把 Composer 换掉。上述任何一次往返都会丢弃该 ref。—— 失败场景:在 macOS 上 TCC 状态为 notDetermined 时,用户按下语音键 → 提示追加一次且系统弹窗未被批准 → 之后任何一次工具审批对话框都会卸载 Composer 并重置 ref → 下一次开始录音时会再次追加相同的提示。这正是 #8877 讨论中要求避免的重复提示形态(“如果保留去重 ref,它需要放在能够在重新挂载后幸存的地方”)。建议修复:把 ref 放到对话框/布局切换之上(例如在拥有 dialogsVisible 且从不卸载的 AppContainer 中创建并向下传递),或挂到该层级的单例上,并补充一个在两次录音之间卸载/重新挂载 Composer 的测试。如果 Composer 层级的生命周期是有意为之,请把注释改为“每次 Composer 挂载一次”,避免后续读者依赖代码并未提供的保证。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +675 to +677
const setupRecorderWith = (
microphoneStatus: (() => Promise<MicrophonePermission>) | undefined,
) => {

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.

[Suggestion] setupRecorder and setupRecorderWith duplicate ~14 identical lines of mock setup — setupRecorder(status) is exactly setupRecorderWith(vi.fn().mockResolvedValue(status)) plus a wider return object. — Failure scenario: when the mocked useUIState or recorder shape gains a field (mock shapes in this file already diverge on which fields they include), both copies must be edited in lockstep; updating only one leaves half of this describe block silently testing a stale mock — easy to miss because the bodies look identical at a glance. Suggested fix — delegate:

const setupRecorder = (status: MicrophonePermission) => {
  const microphoneStatus = vi.fn().mockResolvedValue(status);
  const { addItem } = setupRecorderWith(microphoneStatus);
  return { addItem, microphoneStatus };
};
中文说明

[Suggestion] setupRecordersetupRecorderWith 重复了约 14 行完全相同的 mock 设置——setupRecorder(status) 恰好就是 setupRecorderWith(vi.fn().mockResolvedValue(status)) 外加更宽的返回对象。—— 失败场景:当被 mock 的 useUIState 或录音器形状需要新增字段时(本文件中各处的 mock 形状在所包含的字段上已经有分歧),两处副本必须同步修改;只改其中一处会让这个 describe 块的一半悄悄测试一个过期的 mock——由于两者乍看完全相同,这种漂移很容易被漏掉。建议修复——改为委托(见上方代码块)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +148 to +149
clipboardUnavailableShownRef={clipboardUnavailableShownRef}
voiceMicWarnedStatusRef={voiceMicWarnedStatusRef}

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.

[Suggestion] This Composer→InputPrompt voiceMicWarnedStatusRef forwarding — the central wiring this PR introduces — has no test: Composer.test.tsx stubs InputPrompt as () => <Text>InputPrompt</Text>, capturing no props, and nothing else asserts Composer passes the session ref. — Failure scenario: a future refactor drops this prop line; TypeScript compiles (the prop is optional) and every test stays green because InputPrompt silently falls back to localVoiceMicWarnedStatusRef. Composer renders InputPrompt conditionally on uiState.isInputActive, so each inactive→active transition remounts InputPrompt with a fresh local ref — the once-per-run guarantee degrades to once-per-mount and the exact #8877 symptom (repeated mic-permission notices) returns, while the InputPrompt-level tests still pass because they test the mechanism given a supplied ref, not that Composer supplies it. Suggested fix: make the Composer test stub prop-capturing (e.g. InputPrompt: vi.fn(() => <Text>InputPrompt</Text>)) and assert it receives a voiceMicWarnedStatusRef whose object identity is stable across an isInputActive off→on toggle (two renders, same ref object).

中文说明

[Suggestion] 这条 Composer→InputPrompt 的 voiceMicWarnedStatusRef 传递——本 PR 引入的核心接线——没有任何测试:Composer.test.tsxInputPrompt 打桩为 () => <Text>InputPrompt</Text>,不捕获任何 prop,也没有其他地方断言 Composer 传入了会话级 ref。—— 失败场景:未来某次重构删掉这行 prop;TypeScript 能编译(该 prop 是可选的),所有测试也依然通过,因为 InputPrompt 会悄悄回退到 localVoiceMicWarnedStatusRef。Composer 依据 uiState.isInputActive 条件渲染 InputPrompt,因此每次 inactive→active 切换都会用全新的本地 ref 重新挂载 InputPrompt——“每次运行一次”的保证退化为“每次挂载一次”,#8877 的症状(重复出现麦克风权限提示)会再次出现,而 InputPrompt 层的测试仍然通过,因为它们测试的是“给定一个 ref 时机制是否正确”,而不是“Composer 是否提供了 ref”。建议修复:让 Composer 测试的桩能够捕获 prop(例如 InputPrompt: vi.fn(() => <Text>InputPrompt</Text>)),并断言其收到的 voiceMicWarnedStatusRefisInputActive off→on 切换(两次渲染)中保持同一对象。

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +753 to +754
it('warns only once for repeated recordings with the same status', async () => {
const { addItem } = setupRecorder('prompt');

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.

[Suggestion] No test covers the warn-again-on-status-change branch of the dedup logic. The dedup ref stores the last warned status and short-circuits only on equality, so a transition ('prompt' → user denies the macOS dialog → 'denied') correctly re-warns — but every new test probes only from a fresh ref, same-status repeats, or remount persistence. A mutant probe confirmed the gap: changing the guard to if (voiceMicWarnedStatusRef.current !== null) return; leaves all 8 new permission tests and all 24 use-voice-input tests green; only a not-yet-written transition test detects it. — Failure scenario: if the dedup is ever "simplified" to a boolean flag, every test in this PR stays green, but a user who saw the "macOS will ask" info notice and then denied the dialog never sees the "Microphone access is denied…" error — every later recording silently produces empty transcripts with no explanation. Suggested fix: add a test that warns with 'prompt', then re-probes with a recorder whose microphoneStatus resolves 'denied' (e.g. a second mockResolvedValue on the same cached recorder mock), and asserts addItem is called twice with the second call type: 'error'.

中文说明

[Suggestion] 没有任何测试覆盖去重逻辑中“状态变化时重新提示”的分支。去重 ref 保存的是上一次被提示过的状态,且只在相等时短路,因此状态转变('prompt' → 用户在 macOS 弹窗中拒绝 → 'denied')能正确地再次提示——但所有新测试要么从全新 ref 探测、要么重复相同状态、要么测试跨重新挂载的持久性。一次变异探针确认了这个缺口:把守卫改成 if (voiceMicWarnedStatusRef.current !== null) return; 后,全部 8 个新的权限测试和全部 24 个 use-voice-input 测试依然通过;只有尚未编写的状态转变测试才能发现它。—— 失败场景:如果去重逻辑将来被“简化”为布尔标记,本 PR 的所有测试仍然通过,但看到过“macOS 将会询问”提示、随后拒绝了弹窗的用户永远看不到“麦克风访问被拒绝…”的错误——之后每次录音都会在没有解释的情况下悄悄产生空转写。建议修复:新增一个测试,先以 'prompt' 触发提示,再让录音器的 microphoneStatus 解析为 'denied' 重新探测(例如对同一个缓存的录音器 mock 第二次 mockResolvedValue),并断言 addItem 被调用两次且第二次调用的 type: 'error'

— qwen3.8-max via Qwen Code /review (v0.21.9)

Dialogs (tool approvals, auth, settings) swap Composer out of the layout,
so a ref held in Composer reset on every dialog round trip and the notice
could repeat on the next recording. The ref now lives in AppContainer,
which owns dialogsVisible and never unmounts, and reaches InputPrompt
through uiState like mainControlsRef.

Also from review: delegate setupRecorder to setupRecorderWith in the
InputPrompt tests, cover the prompt->denied status transition (re-warns
as an error), and assert Composer forwards the session ref with stable
identity across input-active toggles.
@Nas01010101

Copy link
Copy Markdown
Contributor Author

Addressed the review in ecaf956:

  • The dedup ref moved from Composer to AppContainer and reaches InputPrompt via uiState (the mainControlsRef route), so dialog swaps that unmount Composer no longer reset it.
  • setupRecorder now delegates to setupRecorderWith.
  • Added the prompt→denied transition test (re-warns as an error) and a Composer test asserting the session ref is forwarded with stable object identity across input-active toggles. Both fail against the mutants described in the review.

PR description updated to match the new ref location.

@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.

Reviewed. Suggestions are inline.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): This PR defers the microphone-permission probe from voice...: none — no checks left unfinished..

Test Plan (not a blocker): src/ui/components/InputPrompt.test.tsxno such file or directory; src/ui/hooks/use-voice-input.test.tsno such file or directory; Tests 9 passed — this review observed 18926 passed; Tests 264 passed — this review observed 18926 passed; 3 passed — this review observed 18926 passed.

中文说明

已审查。 建议见行内评论。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未探索到全部深度(达到工具调用预算):This PR defers the microphone-permission probe from voice...:none — no checks left unfinished.

Test Plan(非阻断):src/ui/components/InputPrompt.test.tsxno such file or directory; src/ui/hooks/use-voice-input.test.tsno such file or directory; Tests 9 passed — this review observed 18926 passed; Tests 264 passed — this review observed 18926 passed; 3 passed — this review observed 18926 passed

— qwen3.8-max via Qwen Code /review (v0.21.9)

Comment on lines +853 to +854
it('stays quiet when permission is already granted', async () => {
const { addItem } = setupRecorder('granted');

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.

[Suggestion] R2-3: The new permission-notice tests pin granted, denied, and prompt, but never 'unknown' — the status the real recorder resolves when no backend reports one (FallbackVoiceRecorder.microphoneStatus() ends in return 'unknown', and only the native recorder implements the method). The silent fall-through in checkVoiceMicPermission is therefore unpinned for the most common non-macOS value. — Failure scenario: a future edit that widens the warn branch — e.g. status !== 'granted' instead of the two explicit === 'denied' / === 'prompt' checks — emits the microphone notice to Linux / no-native-backend users on their first recording, re-introducing the unwanted-notice class this PR fixes (#8877). All 11 new tests stay green under that mutation because no test feeds 'unknown'. Add one case next to the 'granted' one:

it('stays quiet when the recorder reports unknown', async () => {
  const { addItem } = setupRecorder('unknown');
  const { unmount } = renderWithProviders(<InputPrompt {...props} />);

  await act(async () => {
    lastVoiceArgs().checkMicrophonePermission?.();
  });

  expect(addItem).not.toHaveBeenCalled();
  unmount();
});
中文说明

[Suggestion] 新增的权限提示测试固定了 granteddeniedprompt 三种状态,但从未固定 'unknown' —— 这是真实录音器在没有任何后端能报告权限时解析出的状态(FallbackVoiceRecorder.microphoneStatus()return 'unknown' 兜底,且只有 native 录音器实现了该方法)。因此 checkVoiceMicPermission 中最常见的非 macOS 取值所走的静默直通分支没有任何测试保护。—— 失败场景:未来某次编辑放宽提示分支 —— 例如用 status !== 'granted' 取代两个显式的 === 'denied' / === 'prompt' 判断 —— 会让 Linux / 无 native 后端的用户在第一次录音时收到麦克风提示,重新引入本 PR 要修复的多余提示问题(#8877);该变异下全部 11 个新测试仍然通过,因为没有任何测试注入 'unknown'。建议在 'granted' 用例旁补充一个用例(见上方代码块)。

— qwen3.8-max via Qwen Code /review (v0.21.9)

const recorder = getVoiceRecorder();
void Promise.resolve(recorder.microphoneStatus?.())
.then((status) => {
if (voiceMicWarnedStatusRef.current === status) {

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.

[Suggestion] R2-4: The dedup ref is only written in the denied/prompt branches, so a pass through granted leaves it stale — with this PR's per-recording probing plus the session-scoped ref, a later return to a previously warned status is then silently suppressed for the rest of the session, contradicting the "warn again when the permission status changes between recordings" semantics this PR's own tests pin. — Failure scenario: probe reports denied → error shown, ref='denied'; the user grants mic access in System Settings → the next probe returns granted, which matches no branch, so the ref stays 'denied'; the user later revokes access → the probe returns denied again → ref.current === status → no notice for the rest of the session. Verified by a live probe at this commit: denied → granted → denied produces one notice instead of two. (Pre-PR the probe ran once per mount and a remount reset the ref, so this session-long suppression is newly reachable.) Impact is bounded — a failed recorder.start() still surfaces via reportError — except for the 'empty audio' failure, which that guard swallows. Record every observed status, not just warned ones:

.then((status) => {
  if (voiceMicWarnedStatusRef.current === status) {
    return;
  }
  voiceMicWarnedStatusRef.current = status;
  if (status === 'denied') {
    uiState.historyManager?.addItem(/* error item */ ...);
  } else if (status === 'prompt') {
    uiState.historyManager?.addItem(/* info item */ ...);
  }
})

(move the ref write above the branches and drop the two per-branch assignments; the fix is compatible with every existing pinned test)

中文说明

[Suggestion] 去重 ref 只在 denied/prompt 分支中被写入,因此经过一次 granted 之后它就过期了 —— 在本 PR 改为每次录音都探测、且 ref 为会话级之后,权限状态随后回到一个已经提示过的值时,会在整个会话剩余时间内被静默抑制,这与本 PR 自己的测试所固定的“两次录音之间权限状态发生变化时再次提示”的语义相矛盾。—— 失败场景:探测返回 denied → 显示错误,ref='denied';用户在系统设置中授予麦克风权限 → 下一次探测返回 granted,不匹配任何分支,ref 保持 'denied';用户之后再次撤销权限 → 探测再次返回 deniedref.current === status → 会话剩余时间内不再有任何提示。已在当前 commit 上用实时探针验证:denied → granted → denied 只产生一条提示而不是两条。(改动前探测在每次挂载时运行一次,重新挂载会重置 ref,因此这条贯穿整个会话的抑制路径是本次改动新引入的。)影响有限 —— recorder.start() 失败仍会经由 reportError 呈现 —— 但 'empty audio' 失败会被该守卫吞掉,属于例外。建议记录每一次观察到的状态,而不仅是被提示过的状态(见上方代码块):把 ref 写入上移到分支之前,并删除两个分支内各自的赋值;该修复与现有全部已固定的测试兼容。

— qwen3.8-max via Qwen Code /review (v0.21.9)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Voice dictation microphone permission warning shows on every startup instead of only when user tries to record

2 participants