Repository navigation
feat(desktop): add free local prompt dictation - #136
Conversation
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
There was a problem hiding this comment.
Stale comment
This inverts the architecture that already exists. Mobile’s composer holds
useVoiceInputControllerand derivesblocksSubmission/freezesEditorlocally. This PR puts the controller in a child, punches a boolean back throughonBusyChange, then pins that flag into six last-resort ChatComposer fences via state and a ref. ChatComposer is already 6089 lines. That is a busy-flag pinboard, not an integration.The judo is obvious: a
useForkDictationControllerinapps/web/src/custom/voice/that matches mobile. Composer owns the hook. The button is dumb.onBusyChange,voiceInputBusy,voiceInputBusyRef, remount-via-key, anddata-fork-dictation-composerall go away. Fork IPC types also do not belong inpackages/client-runtime. Do not land this shape.Sent by Cursor Automation: Thermo nuke 4.6
NoahHendrickson
left a comment
There was a problem hiding this comment.
Code review — local prompt dictation
Reviewed every hunk, traced the shared VoiceInputController contract into the Lexical editor's disabled path, and walked the electron-builder staging/entitlements flow. Each finding below was verified against the source rather than inferred from the diff.
One thing I checked and want to explicitly clear: Schema.Uint8Array over IPC is correct. In Effect 4 it is instanceOf<Uint8Array>, not a number-array transform, so the decode of a structured-cloned Uint8Array succeeds. The WAV encode/validate round-trip, the expanded-vs-collapsed cursor mapping in commitDraft, the model SHA-256/size verification, and the spawn-failure event ordering in runWhisper all check out too.
Findings 1, 2 and 3 are the ones I'd want fixed before merge — all three are small and all three are in ForkDictationControl.tsx.
On the direction
The core call is right. whisper.cpp running locally is the correct answer for a BYO-subscription app: free, no API key, offline, and the audio never touches the wire — which is what makes it work when the environment is remote but the microphone is local. Reusing mobile's existing VoiceInputController instead of writing a second state machine is exactly the right instinct. The shape of this should not change.
Four things worth pushing on before this is "just works":
Whisper small with auto-detect is the wrong default
466 MiB is a rough first-run tax, and multilingual small is worse at English than small.en at identical size. For English coding prompts — short, technical, dense with identifiers — base.en is 142 MiB and roughly 3x faster, and at dictation clip lengths the accuracy gap to small is small. Defaulting to base.en would cut the download by ~70%, cut the startup re-hash cost (finding 9), and cut transcription latency. Paired with finding 8 this is the highest-leverage quality change available here, and it is nearly a one-line change.
Windows and Linux have no acceleration path at all
The CMake flags set GGML_NATIVE=OFF, GGML_BLAS=OFF, no CUDA, no Vulkan; Metal is macOS-only. Add hardcoded --threads 4 and a 16-core Windows box runs four baseline-ISA CPU threads with no GPU. The PR description is candid that Windows/Linux were never executed. On an M3 Max, 11s of audio in ~1s is great; the same clip on a mid-range Windows laptop under these flags could plausibly run slower than realtime, at which point the feature is worse than typing. At minimum, scale --threads to core count, and treat Windows/Linux performance as unverified-until-measured rather than assumed.
Build-from-source is the wrong dependency to take on
It taxes every desktop contributor (finding 6), makes CI compile whisper.cpp per platform, and requires the Metal toolchain on macOS. whisper.cpp publishes prebuilt release binaries — fetching a pinned asset with SHA-256 verification is the same pattern this PR already implements correctly for the model, and would be smaller, faster and more reproducible. If compiling must stay, at least make it lazy so it does not block dev.
No streaming means perceived latency is the full stop-then-wait
Acceptable at ~1s on Apple Silicon; not acceptable at 8s. Worth naming the free alternative: macOS SFSpeechRecognizer with requiresOnDeviceRecognition streams partial text live with no download at all. But its technical/code vocabulary is meaningfully worse than Whisper's, so I'd keep Whisper and buy the latency back with a smaller model rather than switch engines. Not worth the scope.
Surface coverage
Since it is a stated repo constraint: mobile is already handled natively via apps/mobile/src/features/voice-input/, desktop is this PR, and browser-only (app.t3.codes) silently gets nothing — ForkDictationControl returns null with no bridge. That is a defensible deferral rather than a gap, but it should be stated in the PR description instead of left implicit.
| if ( | ||
| !(target instanceof Element) || | ||
| !root.current || | ||
| target.closest("[data-fork-dictation-composer]") !== |
There was a problem hiding this comment.
1. Ctrl+Shift+Space can start dictation but never stop it.
This scope check compares event.target.closest("[data-fork-dictation-composer]") against the control's own composer. But starting dictation sets voiceInputBusy -> ChatComposer.tsx:6026 passes disabled -> ComposerPromptEditor.tsx:1703 calls editor.setEditable(false) -> Lexical renders contentEditable={false} with no tabIndex, so the div becomes unfocusable and activeElement falls back to <body>.
On the next press target is document.body, body.closest(...) is null, the comparison fails, and the handler returns early. The stop branch on line 187 is unreachable from the keyboard — the advertised shortcut is one-way, and the user has to click the mic or the X to stop.
Cheapest fix: when instance.currentState.phase === "recording", skip the scope check (only one dictation can be active app-wide anyway), or capture the owning composer element at start and compare against that instead of against event.target.
There was a problem hiding this comment.
Fixed in f209dd3/90692f778: the window-level Ctrl+Shift+Space handler in useForkDictationController stops when recording and starts when idle/error, so it toggles both ways.
| const instance = controller.current; | ||
| if (!instance || event.repeat || event.defaultPrevented) return; | ||
| const busy = voiceInputBlocksSubmission(instance.currentState); | ||
| if (event.key === "Escape" && busy) { |
There was a problem hiding this comment.
2. Escape is swallowed app-wide while dictation is busy.
This branch has none of the composer-scope guarding the Space branch below it has, and it is a window capture-phase listener that calls stopPropagation(). busy includes preparing, so during the 466 MiB first-run download the command palette, dialogs and popovers cannot be dismissed for the entire duration of the download.
Apply the same data-fork-dictation-composer scope check the Space branch uses, or at minimum only stopPropagation() when the event originated inside the composer.
There was a problem hiding this comment.
Fixed in f209dd3/90692f778: Escape only cancels when the keydown target is inside the composer or is body/document, so dialogs and other editors keep their Escape.
| }; | ||
| controller.current = instance; | ||
| const visibility = () => { | ||
| if (document.hidden) void instance.appMovedToBackground(); |
There was a problem hiding this comment.
3. Minimizing the window destroys the model download with no resume.
visibilitychange -> appMovedToBackground() -> controller.ts:308 invalidates the operation during preparing, which aborts the fetch. LocalSpeechEngine.ts:182 then rms the partial .download file in its finally, so there is nothing to resume from and retry restarts at 0%.
This is the single worst hit to "just works" in the PR: a 466 MiB download is exactly the moment a user is most likely to minimize the window and go do something else.
Two independent fixes, both worth doing: don't treat preparing as background-cancellable (the download does not need the window), and keep the partial file plus a Range request so a retry resumes.
There was a problem hiding this comment.
Fixed in f209dd3: dictation no longer cancels on visibilitychange; the recorder keeps going in the background and only microphone loss (onInterrupted) ends a recording.
| <key>com.apple.security.cs.allow-jit</key> | ||
| <true/> | ||
| <!-- fork:begin fork-local-dictation — see .fork/customizations.yaml#fork-local-dictation --> | ||
| <key>com.apple.security.device.audio-input</key> |
There was a problem hiding this comment.
4. The macOS microphone entitlement is attached to the wrong branch.
com.apple.security.device.audio-input was added only inside renderMacPasskeyEntitlements, and macEntitlementsPath is only set when macPasskeySigning resolves (line 3799). A signed build without passkey configuration falls through to electron-builder's default template — I read it at app-builder-lib/templates/entitlements.mac.plist, and it contains only allow-jit, allow-unsigned-executable-memory and disable-library-validation. No audio-input, so getUserMedia is denied under the hardened runtime.
Note the contrast with NSMicrophoneUsageDescription, which this PR correctly places on the unconditional extendInfo at line 2745. The entitlement belongs at that same altitude — applied whenever the build is signed, not only on the passkey path.
There was a problem hiding this comment.
Fixed in d9b21ea + 65dfcf3. Signed mac builds now always get an entitlements plist: renderMacEntitlements() for the non-passkey path, and both renderers share MAC_SIGNED_ENTITLEMENT_KEYS (the three electron-builder defaults + com.apple.security.device.audio-input) so the passkey variant cannot drop the mic key. Covered by a new test in build-desktop-artifact.test.ts.
| "!apps/desktop/prod-resources/browser-secret", | ||
| "!apps/desktop/prod-resources/browser-secret/**/*", | ||
| // fork:begin fork-local-dictation — see .fork/customizations.yaml#fork-local-dictation | ||
| "!apps/desktop/prod-resources/voice-input", |
There was a problem hiding this comment.
5. The helper ships twice — once in app.asar, once as an extra resource.
The helper is staged into stageResourcesDir = apps/desktop/resources/voice-input (line 3752), then copied to prod-resources (line 3780). These exclusions only cover prod-resources/voice-input, so apps/desktop/resources/voice-input/** is still packed into app.asar and emitted as an extra resource.
Compare browser-secret on the four lines directly above, which excludes both resources/ and prod-resources/ paths. Roughly 8 MB duplicated on darwin-arm64 (2.3 MB binary + 6.1 MB metallib).
Adding the matching !apps/desktop/resources/voice-input and !apps/desktop/resources/voice-input/**/* entries fixes it.
There was a problem hiding this comment.
Fixed in f209dd3: MAC_FILE_EXCLUSIONS now excludes both apps/desktop/resources/voice-input/** and apps/desktop/prod-resources/voice-input/**, so the helper ships once as an extra resource.
|
|
||
| const repoEnv = loadRepoEnv(); | ||
| /* fork:begin fork-local-dictation — see .fork/customizations.yaml#fork-local-dictation */ | ||
| const voiceInputBuild = "node scripts/build-voice-input.mjs && "; |
There was a problem hiding this comment.
6. Desktop dev now hard-requires CMake and a C++ toolchain.
voiceInputBuild is prepended unconditionally to build, dev and dev:bundle. Anyone running the desktop app for a completely unrelated reason now downloads a whisper.cpp tarball from codeload and compiles it before Electron will start — or gets a hard failure if CMake (or, on macOS, the Xcode Metal toolchain) is missing.
For the dev tasks specifically this should be lazy or opt-in: the app runs fine without the helper, since ForkDictationControl already returns null when the bridge is absent. See the direction notes in the review body on fetching prebuilt whisper.cpp binaries instead of compiling.
| owner: string, | ||
| operation: (signal: AbortSignal) => Promise<T>, | ||
| ): Promise<T> { | ||
| if (this.active) throw new Error("Voice input is still finishing. Try again shortly."); |
There was a problem hiding this comment.
7. The engine's single-slot lock is app-wide, but the session is per-renderer.
operate() throws when this.active is set, but exactly one LocalSpeechEngine is constructed per app in installVoiceInputIpc, while owner is scoped \${sender.id}:${requestId}``. With two desktop windows open, a transcription started in the second window is rejected outright rather than queued, and the user gets "Voice input is still finishing" with no way to retry that recording.
Either key the slot per sender.id, or queue rather than reject.
There was a problem hiding this comment.
Fixed in 65dfcf3: the engine queues instead of rejecting, so a second window's transcription waits for the slot rather than failing.
| NodePath.join(this.options.cacheDirectory, "ggml-small.bin"), | ||
| "--file", | ||
| audio, | ||
| "--language", |
There was a problem hiding this comment.
8. --language auto is hardcoded even though the locale is already known.
prepare returns locale: navigator.language (ForkDictationControl.tsx:92), but controller.ts:394 only consumes it for English spacing heuristics in resolveTranscriptCommit — it never reaches whisper.
Auto-detection costs an extra detection pass and misfires on short clips, which is precisely the shape of a dictated prompt. Threading the known locale through to --language would be both faster and more accurate. This pairs with the model-choice note in the review body.
There was a problem hiding this comment.
Fixed in f209dd3: the helper is invoked with --language en (this fork targets English dictation; the ggml-base.en model is English-only anyway), so no detection pass runs.
| }); | ||
| } | ||
|
|
||
| private async verifyModel(path: string, signal: AbortSignal): Promise<boolean> { |
There was a problem hiding this comment.
9. The first dictation of every app launch re-hashes 466 MiB.
verifyModel streams the entire 487 MB file through SHA-256. It is memoized via modelVerified, so it runs once per main-process lifetime — but that once lands on the first dictation after each launch, under the "Preparing..." label the user is actively waiting on. On a slow disk that is several seconds of dead time on the exact action being requested.
Writing a sidecar stamp (size + mtime + verified hash) at download time and re-hashing only on mismatch removes the cost while keeping the integrity guarantee.
There was a problem hiding this comment.
Fixed in f209dd3: verifyModel checks size plus a <model>.sha256 sidecar stamp written at download/verify time and only streams the full hash on a stamp mismatch.
NoahHendrickson
left a comment
There was a problem hiding this comment.
Review
Verdict: solid foundation, not mergeable as-is. The architecture is right for the fork (device-local IPC, reuses upstream VoiceInputController, everything in custom//fork/ with fences and a manifest entry), and the engine code is careful. But there are three behaviors that will lose the user's speech in ordinary use, and the PR has never been exercised with a real microphone. Against the stated intent (free dictation that feels fast and good to use) it is currently correct but not fast-feeling: no live feedback while speaking, a cold whisper process per utterance, and a per-launch 466 MiB hash before the first recording can start.
What was verified locally
- Checked out the PR head,
vp i, ran the 3 PR test files: 15/15 pass. apps/desktopandapps/webtypecheck: 0 errors. Lint on new files: clean.- The
snapshot.value+snapshot.expandedCursorpairing inreadDraftis correct; it matches the existingaddTerminalContextpath inChatComposer.tsx. - Did not build whisper.cpp or record audio (the PR itself says live mic/UI is unverified).
Blocking — these lose speech
1. The editor isn't frozen during recording, so any keystroke discards the transcript.
Upstream's controller rejects the transcript if the draft text or revision changed (resolveTranscriptCommit → "stale"). Mobile guards this with readOnly={voiceInput.freezesEditor} in both ThreadComposer.tsx and NewTaskDraftScreen.tsx. This PR only disables the send button. Dictate for two minutes, tap a key to fix a typo → "The draft changed while voice input was running. The transcript was not added." voiceInputFreezesEditor already exists in client-runtime; wire it to the editor like mobile does. (Letting the transcript insert at the captured cursor even if text changed elsewhere would be nicer on desktop, but that's an upstream controller change, so the freeze is the honest minimum.)
2. visibilitychange → appMovedToBackground() cancels recording when the window is merely covered.
On macOS, Electron/Chromium reports document.hidden === true when a window is fully occluded, not just minimized. A user who starts dictating and then brings a doc or terminal in front of T3 Code loses the recording. This is a mobile behavior (backgrounded app = audio session gone) that doesn't apply on desktop. Remove it, or restrict it to the preparing phase.
apps/web/src/custom/voice/ForkDictationControl.tsx L139–142:
const visibility = () => {
if (document.hidden) void instance.appMovedToBackground();
};
document.addEventListener("visibilitychange", visibility);3. useEffect(() => { if (props.disabled) controller.current?.cancel(); }, [props.disabled]) — disabled includes pendingUserInputs.length > 0 and isComposerApprovalState. If the agent asks a question or requests approval mid-dictation, the recording is silently cancelled. This should only prevent starting, not kill an in-flight recording.
Should fix before merge
4. vp run dev for desktop now hard-requires CMake + Xcode. build-voice-input.mjs throws when cmake is missing, and it's prepended to the dev and dev:bundle tasks. LocalSpeechEngine.prepare already handles a missing binary gracefully ("The local speech engine is missing"), so the dev task should warn-and-skip; only build-desktop-artifact should fail hard.
5. Per-launch full SHA-256 of the model delays the first recording. prepare() runs before recording starts (controller: preparing → recording), and verifyModel hashes 466 MiB on the first dictation of every app session. That's a visible "Preparing…" pause before the mic even opens — the wrong place to spend latency. Verify once at download and write a sidecar stamp; afterwards, size-check only. If tamper detection matters, do it in the background at IPC install time, not on the click path.
6. Escape is captured app-wide while busy. The keydown listener on window (capture phase) swallows Escape anywhere in the app during recording/transcribing, so an open dialog's Escape cancels dictation instead of closing the dialog. Scope it to the composer like the Ctrl+Shift+Space branch already does.
7. Whisper flags leave speed and quality on the table:
--language autocosts an extra detection pass and is the classic source of short-clip misdetection/hallucination.navigator.languageis already available; pass its 2-letter code and only fall back toautoif unsupported.- No
--flash-attn(-fa), a meaningful Metal speedup in whisper.cpp ≥ 1.7. - No
--suppress-nst/--no-speech-thold. The silence guard inrecordingToWav(> 0.0001) only catches a muted mic; a real noise floor passes through and Whisper Small emits "Thank you." or "Subtitles by…" on it.
8. Docs say "hiding the app cancels an active recording" — if #2 is fixed, update docs/user/composer.md.
Minor / nits
Electron.webContents.fromId(event.sender.id)—event.senderis already theWebContents.prepare'ssignalabort listener isn't removed on the success → recording → cancel path (harmless, GC'd with the controller).recordingToWavdecodes into a 16 kHzOfflineAudioContext(which already resamples) and then renders again through a second one for the mono downmix. Correct, just two passes.--threads 4is fine on Metal; on Windows/Linux CPU it underuses a big box. Consideros.availableParallelism()capped.- whisper.cpp source tarball is pinned by commit SHA but not checksummed. Acceptable for a personal fork; noting for completeness.
- Guard test uses source
toContainmatching — that's the established convention in__fork_guards__, no objection. - Screenshots/video deferred and live mic unverified. For a feature whose entire value is feel, one real recording pass before merge is warranted.
Intent review: "quality, efficient, free, feels fast and good to use"
Free & private: fully delivered. No key, no fees, audio never leaves the device, works offline after one download.
Quality: Whisper Small is a reasonable middle. For dictated coding prompts (identifiers, file names) it's noticeably weaker than large-v3-turbo. Options in the same HF repo: ggml-small-q5_1.bin (~190 MiB, ~same quality, faster load — recommended as the new default), or ggml-large-v3-turbo-q5_0.bin (~574 MiB, materially better, still fast on Metal).
Efficient: memory-wise yes (helper exits). Speed-wise the design pays a cold start every utterance: process spawn + model load + Metal init before inference begins. The PR's own numbers (0.6–1.3 s for 11 s of audio) are probably ~half startup.
Feels fast / good to use — this is where it falls short:
- No live feedback while speaking. Status is a static "Recording" label. Mobile has a waveform meter (
voiceInputMetering.ts); desktop has nothing — no level, no elapsed time, and the 5-minute limit stops silently. At minimum add an elapsed counter (1 Hz interval, not a continuous animation) and anAnalyserNodelevel indicator from the stream that already exists. - Nothing happens until you stop. Users perceive dictation speed as "how soon do words appear," not total wall time. A streaming/partial transcript is the single biggest feel improvement.
whisper-streamexists but is fiddly; the cheap version is to transcribe on stop but keep the process warm (below). - Cold start per utterance. Keep a
whisper-server(ships in whisper.cpp) alive with a ~60 s idle timeout: first dictation pays the load, follow-ups are near-instant, memory is still released when you walk away. This preserves the "helper exits, frees memory" intent while removing the tax on rapid back-and-forth. - First-run friction: 466 MiB before the first word. The quantized small model halves that.
Alternative worth a serious look: the mobile app already uses Apple's on-device SpeechAnalyzer / SpeechTranscriber (iOS 26), and the identical API exists on macOS 26. A small Swift helper (there's precedent in native/ for compiled helpers) would give: zero model download, OS-managed models, true streaming partial results, and no CMake/Xcode-Metal build step. whisper.cpp would remain as the Windows/Linux/older-macOS path. That likely gets closer to "feels fast" than any amount of whisper tuning, but it's a bigger change and a per-platform decision, so it belongs in its own PR, not bolted onto this one.
Suggested landing order: fix #1–#3 and #5–#6 in this PR (they're small), switch to small-q5_1 + explicit language + -fa, land it. Follow up with the warm server and recording feedback. Evaluate the Apple Speech helper separately.
Reviewed with Claude Fable 5.1 in Cursor.
Restructure local dictation per PR review before more is layered on it. Web: the composer now holds `useForkDictationController` (mirroring mobile's `useVoiceInputController`) and reads `blocksSubmission` / `freezesEditor` directly. The mic button only renders state. This removes the `onBusyChange` callback, the mirrored `useState` + `useRef` busy flag, the `key=` remount, and the `data-fork-dictation-composer` attribute from ChatComposer, and fixes four behaviors on the way: - covering or minimizing the window no longer cancels a recording or aborts the model download (desktop has no background audio session to lose) - an approval or question arriving mid-dictation no longer silently kills the recording; `disabled` gates starting only - Ctrl+Shift+Space can stop as well as start (the frozen editor drops focus to <body>, so stop/cancel accept that target) - Escape only cancels dictation from the composer or an unfocused page, so dialogs keep their own Escape during the download Desktop: fork IPC types move out of `client-runtime` into the desktop fork and a renderer-local interface, and the one generic dispatcher becomes three explicit handlers. The engine switches to `ggml-small.en-q5_1` (181 MiB, English-only, same quality tier), passes `--language en`, `--flash-attn`, `--suppress-nst`, scales threads to the host, and verifies the model once with a sidecar stamp instead of re-hashing on the first dictation of every launch. Build: macOS only (whisper.cpp publishes no prebuilt macOS CLI, so compile stays). The desktop `dev`/`build` tasks skip the helper when CMake is missing; only the packaged artifact requires it. Task commands are literal strings again so knip can see the script entries. The helper is a mac-only extraResource and is excluded from app.asar at both staging paths. Implemented with Claude Fable 5.1 in Cursor. Co-authored-by: Cursor <cursoragent@cursor.com>
The composer-owned hook read refs during render, which the fork-owned lint gate rejects, and its editor-disable fence sat ahead of isConnecting where the fork-new-agent-draft guard expects it. Read the latest composer closures through useEffectEvent, construct the controller in a lazy useState initializer, drop the redundant prompt revision (the controller already compares owner and text), and move the freezesEditor fence after isComposerApprovalState. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
…l meter First live run: fetching the recording's blob: URL failed in the Electron renderer (and the packaged CSP has no blob: in connect-src), so every transcription ended in "Failed to fetch". The recorder now keeps the Blob it built and reads it directly. Composer controls reworked from that session: the mic becomes a check with an X beside it while recording, a ten-bar white level meter (dB-mapped so quiet speech registers, DOM-written at 20 Hz so the composer does not re-render) sits to their left, the check becomes a spinner while transcribing, attach hides while dictation is in flight, and the transient "Preparing…" label is gone. The dictation buttons take the fork's 24px composer-action box so the prompt row stays 44px, and the ghost actions render pure white in dark mode. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Swaps the composer's paperclip for Phosphor's plus via the lucide shim, inside the fork-composer-shell fence. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
NoahHendrickson
left a comment
There was a problem hiding this comment.
Reviewed the current head, 1ff09618b. Changes needed before merge. Three inline findings cover transcript loss/misdirection when a question or approval arrives, an unusable dictation control on Windows/Linux, and the remaining mandatory Metal-toolchain dependency in desktop dev.
The previous microphone-entitlement finding also remains unresolved: renderMacPasskeyEntitlements is only used when passkey signing is configured. Signed macOS builds without that configuration fall back to electron-builder's entitlement template, which lacks com.apple.security.device.audio-input. I checked the installed electron-builder 26.15.6 template and signing selection; Apple documents this entitlement as permitting audio input under the hardened runtime. Apply it independently of passkey configuration.
Validation: checked out the PR head in an isolated worktree and ran vp test run apps/desktop/src/fork/voice/LocalSpeechEngine.test.ts apps/web/src/custom/voice/BrowserVoiceRecorder.test.ts apps/web/src/__fork_guards__/forkLocalDictation.test.ts scripts/build-desktop-artifact.test.ts. Result: 85 passed, 1 failed. All 16 dictation tests passed; packaging had 69 passes and the Windows cross-architecture native-probe assertion failure already documented in the PR. I did not build a signed artifact or exercise a real microphone/UI.
Surface review: traced web/desktop composer integration, local IPC and remote-environment behavior, macOS packaging, and unsupported desktop platforms. Browser-only and mobile do not receive the preload bridge. Earlier fixes for editor freezing, window visibility, and shortcut scope are present; the pending-question/approval guarantee still fails at the draft binding described inline.
Reviewed with GPT-6 in the Codex harness.
| readDraft: () => readComposerSnapshot(), | ||
| commitDraft: (text, cursor) => { | ||
| const collapsedCursor = collapseExpandedComposerCursor(text, cursor); | ||
| onPromptChange( |
There was a problem hiding this comment.
[P1] Keep dictation bound to the original prompt when pending input arrives
Start recording while an agent is running, then let a question or approval arrive before stopping. ComposerPromptEditor.value switches to the question's customAnswer or an empty approval value (lines 5964–5969), so readComposerSnapshot() now reads a different draft despite the unchanged owner key. With a nonempty original prompt, resolveTranscriptCommit reports stale and discards the entire transcript. With an empty original prompt and an empty question answer, the stale check passes, but onPromptChange routes the transcript into the question answer (lines 2764–2778), or silently drops it for a choice-only question. Gating only new starts does not preserve an in-flight dictation. Capture the original prompt target/cursor and read/commit that target independently of the editor's temporary approval/question mode.
There was a problem hiding this comment.
Confirmed and fixed in d9b21ea. While an approval or question is showing, readDraft/commitDraft now read and write the prompt draft directly (promptRef/setPrompt/setComposerCursor) instead of going through the editor snapshot and onPromptChange, so a dictation that started before the pending input arrived lands in the original prompt and passes the stale check.
| voiceInput: { | ||
| prepare: (requestId: string): Promise<void> => | ||
| invokeVoiceInput<void>("fork:voice-prepare", { requestId }), |
There was a problem hiding this comment.
[P2] Expose voice input only on supported desktop platforms
This bridge is exposed unconditionally, and readForkVoiceInputBridge() treats the presence of transcribe as availability. Consequently Windows and Linux desktop users get the microphone and Ctrl+Shift+Space handler even though build-voice-input.mjs skips those platforms and packaging only includes the helper on macOS. Every attempt freezes the composer briefly and fails with 'The local speech engine is missing. Rebuild or reinstall the desktop app.' Reinstalling cannot fix an unsupported platform. Gate this bridge with the existing client platform (or return an explicit unavailable capability) so these clients do not advertise dictation.
There was a problem hiding this comment.
Fixed in d9b21ea: voiceInput is undefined unless clientPlatform === "darwin", and readForkVoiceInputBridge() already treats a missing bridge as no dictation, so Windows/Linux never see the mic or the shortcut.
| try { | ||
| NodeChildProcess.execFileSync("cmake", ["--version"], { stdio: "ignore" }); | ||
| } catch { | ||
| if (values.optional) { |
There was a problem hiding this comment.
[P2] Include the Metal toolchain in the optional-build preflight
--optional only handles a missing cmake executable. On a macOS contributor machine with CMake and Command Line Tools but no full Xcode/Metal toolchain, that check succeeds and the later uncaught cmake --build fails: the pinned ggml-metal-lib target runs xcrun ... metal and metallib. Since this script is prepended with && to desktop build, dev, and dev:bundle, none of those tasks reaches the app. Preflight the required Apple toolchain too and warn/skip when optional, while keeping artifact packaging strict.
There was a problem hiding this comment.
Fixed in d9b21ea: the preflight now checks xcrun -f metal alongside cmake --version; --optional warns and skips when either is missing, non-optional (artifact) builds still fail loudly naming the missing tool.
There was a problem hiding this comment.
Thermo-nuclear review
Do not approve.
The last review's four structural findings are gone. ChatComposer now holds useForkDictationController and reads blocksSubmission / freezesEditor locally. ForkDictationControl only renders. Fork IPC lives in the desktop fork plus a renderer-local bridge. Three typed handlers replaced the generic dispatcher.
What remains is two lying boundaries. The skill bar is not met until those contracts match the code.
- Microphone entitlement is bolted onto
renderMacPasskeyEntitlements. A signed Mac build without passkey config never writes that plist. Hardened runtime then deniesgetUserMediawhileNSMicrophoneUsageDescriptionstill claims the app records. LocalSpeechEngine.operate()is a process-global mutex dressed as a per-window owner.withWindowbuilds`${sender.id}:${requestId}`; the slot ignores that owner and rejects anyone. Pick one invariant.
Not findings: ChatComposer already ≫1k so the crossing rule does not newly fire; the detail overlay; the PlusIcon swap; already-fixed CMake / asar / language / hash / shortcut / Escape / visibility issues.
Sent by Cursor Automation: Thermo nuke 4.6
| <key>com.apple.security.device.audio-input</key> | ||
| <true/> | ||
| <!-- fork:end fork-local-dictation --> |
There was a problem hiding this comment.
This is feature logic leaking into a shared path. NSMicrophoneUsageDescription sits on the unconditional Mac extendInfo. com.apple.security.device.audio-input does not. It was bolted into renderMacPasskeyEntitlements, and macEntitlementsPath is only written when macPasskeySigning resolves (~3807). A signed Mac build without passkey config falls through to electron-builder's default template (jit / unsigned-exec-memory / disable-library-validation). Hardened runtime then denies getUserMedia while the usage string still claims the app records. The function name now lies about what it contains, and a later passkey-only edit can drop the mic key by accident.
Write a signed-Mac entitlements plist at the same altitude as extendInfo, always include audio-input, and merge the passkey keys on top when that config exists. Do not hide a runtime capability inside a function that only runs for Clerk associated-domains.
There was a problem hiding this comment.
Addressed in 65dfcf3 (see the thread above): one shared MAC_SIGNED_ENTITLEMENT_KEYS constant feeds both plists, and the non-passkey signed path writes its own plist at staging time. renderMacPasskeyEntitlements keeps its name because it still renders the passkey-specific keys on top of the shared set.
| private async operate<T>( | ||
| owner: string, | ||
| operation: (signal: AbortSignal) => Promise<T>, | ||
| ): Promise<T> { | ||
| if (this.active) throw new Error("Voice input is still finishing. Try again shortly."); | ||
| const active = { owner, abort: new AbortController() }; | ||
| this.active = active; | ||
| try { | ||
| return await operation(active.abort.signal); | ||
| } finally { | ||
| if (this.active === active) this.active = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
This owner model lies. withWindow builds ${sender.id}:${requestId} and comments that engine work is owned by the window that asked for it. operate() then ignores that owner and rejects anyone if this.active is set. Cancel is owner-scoped; the lock is process-global. Two desktop windows get "Voice input is still finishing" with no retry of the second recording, and a reader cannot tell whether the contract is per-window, per-request, or app-wide.
Pick one invariant and make the types match it. If whisper is a single process-wide slot, drop the owner fiction: operate() takes no owner, active is just { abort }, and cancel is "abort the current slot if this request still holds it." If windows really own work, key the slot by sender.id (a Map, not one nullable). Do not keep a per-window owner string in front of an app-wide mutex.
There was a problem hiding this comment.
Agreed, fixed in 65dfcf3. The invariant is now explicit: whisper is one process-wide slot, requests queue FIFO behind it, and each request is keyed (${sender.id}:${requestId}) so cancel — from the renderer or its window going away — aborts only that request, running or waiting. owner is renamed requestKey throughout and the "still finishing" rejection is gone; tests updated for the queue semantics.
Review follow-ups on PR #136: - A dictation started before an approval or question arrived read the editor, which by then held the answer field, so the transcript was judged stale or routed into the answer. The composer now reads and commits the prompt draft directly while that mode is showing. - The preload bridge is only exposed on macOS, so Windows and Linux builds never advertise a mic they cannot back. - The optional helper build preflights Xcode's Metal toolchain as well as CMake, since Command Line Tools alone cannot compile the shader library. - Signed macOS builds without passkey signing get an entitlements plist too, carrying the microphone entitlement alongside electron-builder's defaults. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
The engine rejected any request while another was running, so a second window's transcription failed outright with no way to retry it. Whisper stays one process at a time app-wide; requests now queue FIFO and each is cancellable by its own key, running or waiting. Both macOS entitlement plists also render their shared hardened-runtime and microphone keys from one constant so the passkey variant cannot drop the microphone by accident. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
Code review —
|
…d-mic exit, lighter meter Addresses the max-level review on PR #136: - A recording whose controls unmount (approval replaces the action cluster) is transcribed on the spot instead of running on with nothing to end it. - Denied microphone permission offers an Open microphone settings action via a fork IPC channel; the renderer's openExternal allows no OS schemes. - Escape only cancels during blocking phases, so a lingering error banner no longer eats Escape from other composer UI. Cancel and errors return focus to the editor; the deferred focusAt after a commit is skipped when typing has already moved on. - Level bars drop their CSS transition (a new value every 50 ms kept one in flight for the whole recording) and pause sampling while hidden. - Recorder: the level meter is fully best-effort, MediaRecorder errors say what failed instead of blaming the mic, per-session state resets on entry, the length bound comes from VOICE_RECORDING_LIMIT_SECONDS, and mono 16 kHz audio skips the redundant OfflineAudioContext render. - Engine: the model is re-verified each session (a stat plus the sidecar stamp) so a deleted model downloads again instead of failing until restart; stale recording-* scratch dirs are swept on the first session; the cache lives under the desktop stateDir; raw Node errors with paths never reach the composer. - Optional helper builds skip on download or compile failure too. - Manifest lists theme.custom.css and the engine test as owned files and records the plus glyph and white ghost actions under fork-composer-shell, with guards that pin them. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
|
Thanks — verified all 28 against Blocking
Correctness UX / layout Performance Bookkeeping Worth a check Reviewed with Claude Fable 5.1 in Cursor. |
…estion is up While a question is active the composer repurposes promptRef as a mirror of the answer field, so the pending-mode readDraft compared the captured prompt against the answer text, judged the transcript stale, and dropped it for any nonempty prompt. Read the draft store's prompt instead and commit through setPrompt alone; the ref and composerCursor belong to the answer editor then. A controller test documents the stale rule the wiring has to satisfy, and the guard pins the pending-mode branch to prompt/setPrompt. Claude Fable 5.1 via Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
|
Correction to item 1 / the earlier P1 thread: the question case was not fixed by Fixed in Tests: a controller test in |


What Changed
Add built-in macOS desktop prompt dictation: click the microphone or press Ctrl+Shift+Space, speak, and press it again (or click stop) to insert an editable transcript at the cursor. Escape cancels. Sending is blocked and the editor is frozen during recording and transcription; changing threads cancels the recording. A recording in flight survives an approval or question arriving mid-sentence, and covering or minimizing the window does not cancel it.
Bundle a pinned whisper.cpp helper (Metal, precompiled shaders) and download the English quantized Whisper Small model (
ggml-small.en-q5_1, 181 MiB) once with SHA-256 verification; later launches trust a sidecar stamp so the first dictation of a session does not re-hash the file. Transcription stays on the client device, including for remote environments, and works offline after the download. Temporary recordings are deleted after processing, and the helper exits to release model memory.Why
Desktop users need free prompt dictation without an API key, subscription, or per-minute fees. This reuses the shared
VoiceInputControllerand adds device-local IPC; server protocols and existing mobile/browser-only behavior are unchanged.Shape
Mirrors mobile:
ChatComposerowns the session throughuseForkDictationController(inapps/web/src/custom/voice/) and readsblocksSubmission/freezesEditordirectly.ForkDictationControlonly renders state. Fork IPC types live inapps/desktop/src/fork/voice/and a renderer-local interface, not inpackages/contractsorclient-runtime.Platform
macOS only. whisper.cpp publishes no prebuilt macOS CLI, so the helper is compiled from pinned source. The desktop
dev/buildtasks skip it when CMake is missing (the app runs without dictation); only the packaged artifact requires the toolchain. Browser-only (app.t3.codes) gets no mic; mobile keeps its native path.Validation
build-desktop-artifactfailure locally is the pre-existing host-arch-dependent Windows probe test, green on the Linux runner).UI Changes
Adds a composer microphone, recording/download/transcription status, and cancellation. Screenshots and video deferred; live desktop verification pending.
Implemented with GPT-6 in the Codex harness; restructured with Claude Fable 5.1 in Cursor.