fix(web): keep dictation in the current thread

[NoahHendrickson]

What Changed

Keep dictation attached to the current thread by reading composer callbacks from a ref refreshed after each committed render. Add regression tests using a memoized composer and register them in the fork manifest.

Why

After opening thread A and switching to thread B, confirming dictation could insert the transcript into A while B stayed empty and no error appeared. The effect-event callback retained A’s draft handlers inside the memoized composer. Both new tests fail before this fix and pass afterward; they cover repeated thread switches and a late transcript arriving after navigation.

This changes the desktop dictation hook used by both the microphone button and keyboard shortcut. Audio processing, mobile, browser-only clients, and server contracts are unchanged.

Validation

• All 451 fork guard tests pass using the web Vite configuration (cd apps/web && vp test run src/__fork_guards__).
• All 11 focused dictation and recorder tests pass.
• Web typecheck, focused lint, formatting, and git diff --check pass.
• Regression verified in React tests; the running desktop app and live drafts were left untouched.

Checklist

• This PR is small and focused
• I explained what changed and why

Model: GPT-6-Astra. Harness: Codex.

[cursor]

The diagnosis is right and the fix is in the right file. The implementation is not the simple version this repo already picked.

useLayoutEffect plus a stable useCallback is a third freshness mechanism next to the render-time closures on start / cancel. Mobile's useVoiceInputController — the hook this file says it mirrors — and BranchToolbar already solved this compiler / memo() / useEffectEvent failure with ref.current = input during render. Use that one ref for the session, the keydown handler, start, and cancel. The layout-effect also leaves a render-to-commit window that still points at the previous thread. Drop the extra hooks.

  

_{Sent by Cursor Automation: Thermo nuke 4.6}

[github-actions]

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ The exact PR base did not have a successful artifact. Baseline uses the latest successful main measurement shown below.

Provider	Metric	Main baseline	This PR	Impact	PR ceiling
Codex	Total thread wire	13.5 KiB	13.6 KiB	+104 B (+0.8%)	15.1 KiB	✅
Codex	Thread snapshot wire	7.0 KiB	7.0 KiB	−4 B (−0.1%)	7.3 KiB	✅
Codex	Live turn WebSocket wire	6.4 KiB	6.5 KiB	+108 B (+1.6%)	7.8 KiB	✅
Codex	Live turn WebSocket decoded	56.2 KiB	57.0 KiB	+822 B (+1.4%)	66.4 KiB	✅
Codex	Live turn messages	8	8	0 (0.0%)	21	✅
Claude	Total thread wire	13.6 KiB	13.6 KiB	+42 B (+0.3%)	15.1 KiB	✅
Claude	Thread snapshot wire	7.0 KiB	7.0 KiB	−4 B (−0.1%)	7.3 KiB	✅
Claude	Live turn WebSocket wire	6.5 KiB	6.6 KiB	+46 B (+0.7%)	7.8 KiB	✅
Claude	Live turn WebSocket decoded	57.8 KiB	57.9 KiB	+44 B (+0.1%)	66.4 KiB	✅
Claude	Live turn messages	9	10	+1 (+11.1%)	21	✅

Baseline: 3a68a82 · PR result: 2a23865 · Source CI: success

Scenario and decoded snapshot size

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

• Codex decoded thread snapshot: 113.9 KiB
• Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

[NoahHendrickson]

Review — max effort

The premise checks out, and the root cause is now pinned. Running this branch's guard against a copy of the pre-fix hook reproduces the bug exactly as described (all three threads' transcripts land in thread-a). The mechanism is in react-dom 19.2.6: commitBeforeMutationEffects applies updateQueue.events only for FunctionComponent fibers — case 11 (ForwardRef) and case 15 (SimpleMemoComponent) are explicit empty breaks. ChatComposer is memo(function ChatComposer …) → tag 15, and it carries no key, so it stays mounted across thread switches. The fix is sound.

Gates are green: vp fmt --check clean, vp lint --report-unused-disable-directives exits 0 on the changed files, all 451 fork guards pass, and the 11 focused dictation tests pass.

Inline comments below. Three things stand out.

1. One race is still open (inline at useForkDictationController.ts:200)

The ref sync is a layout effect, but controller.ownerChanged() is still passive. A transcript settling in that window takes the resolveTranscriptCommit stale path and paints "The draft changed while voice input was running" — plus steals focus — on the thread the user just opened. Not reproducible under act(), which drains passive effects synchronously, so this harness structurally cannot see it.

2. The same bug is live one file away — apps/web/src/custom/useComposerPendingUserInputCard.ts:75

Not in this diff, so no inline anchor. handleOptionSelection is a useEffectEvent whose only caller is memo(function ComposerPendingUserInputCard(…)) (overrides/components/chat/ComposerPendingUserInputPanel.tsx:69), keyed by activePrompt.requestId — so the card stays mounted while questionIndex advances.

On a prompt whose question 1 is single-select and question 2 is multi-select, clicking an option on question 2 (panel:156) or pressing its number key (hook:115) runs question 1's frozen closure: activeQuestion?.multiSelect reads false, so it takes the single-select branch, sets optimisticSingleSelect and arms the 200 ms auto-advance — shoving the user off a multi-select question after one pick — and writes through the mount-time onToggleOption. Note the hook already hand-rolls onAdvanceRef for onAdvance, so the latest-ref need was recognised there.

An audit of all 18 useEffectEvent files found these two are the only memo/forwardRef hosts; everything else is tag 0 and unaffected. Worth a follow-up rather than growing this PR.

3. The change silently drops the hook out of the React Compiler

Measured with the repo's own babel-plugin-react-compiler@1.0.0: custom compiles to const $ = _c(41); this branch emits no _c( at all, with CompileError: Cannot access refs during render. Details inline at line 206. Bounded today, but worth a deliberate decision rather than a silent one.

---

Ruled out, so nobody re-investigates: the .tsx extension is picked up correctly by vitest (vite.config.ts:82), .fork/lint-owned.mjs, and forkLintCleanliness.test.ts, with no baseline growth; the oxlint-disable is load-bearing, not an unused directive; the folded-scalar manifest edit parses through detect-drift.mjs; docs/user/composer.md:96 already says "Switching threads cancels an active recording", so no doc update is owed; StrictMode's double-invoked useState initializer leaks nothing (BrowserVoiceRecorder's constructor is field-initializers only, and acquireSession() is reached only from start()); and listing the test under verify: but not files: matches the existing manifest convention.

Two candidates were refuted and dropped: the no-dep useLayoutEffect is not a scheduling cost (~60 ns/commit vs useEffectEvent, which sets the same Update flag), and there is no retention regression — the old code pinned the first render's closure; the ref holds the latest.

One convention nit not worth an inline: this guard has no Fork guard — see .fork/customizations.yaml#<id> header. 55 of the 57 guard files carry one — but the other exception is forkLocalDictation.test.ts, the existing guard for this same customization, so the new file matches its sibling. Add the header to both or neither.

🤖 Generated with Claude Code

[NoahHendrickson]

Review follow-ups — pushed in 68fd135

Addressed

1. Layout-vs-passive race. ownerChanged() is now a useLayoutEffect declared after the mirror sync, so the two run back-to-back in one uninterruptible commit and the window you described is gone.

Comment records the wrong mechanism. Rewritten to name it: react-dom 19.2.6 applies useEffectEvent impls only for FunctionComponent fibers, memo and forwardRef hosts hit case 11: case 15: break; — permanent, not a race. Ends with the audit query (grep useEffectEvent, check whether the host is memoized or a forwardRef).

useCallback + the dep existed only for each other. Both gone. readInput is gone entirely; the four call sites read inputRef.current directly and createSession gets an inline () => inputRef.current. Keydown deps are back to [controller].

The indirection was incomplete. start and cancel now read the mirror too, with a comment naming the regression shape (memoizing ForkDictationControl, hoisting start into a command action). New test 4 pins it: it retains the object from thread A's render, calls retained.start() / retained.cancel() after switching, and asserts focus lands on thread B. It fails pre-fix with ['thread-a'].

3. React Compiler bailout. Made deliberate with "use no memo" plus a note. Verified with the repo's babel-plugin-react-compiler@1.0.0 at panicThreshold: "all_errors": without the directive it throws, with it the file compiles clean and simply carries no memo slots. Same emitted output, but the loss is now declared rather than swallowed by the default panic threshold.

Guard: no reset. vi.resetModules() + a per-test dynamic import of the hook, which resets both activeSession and activeTranscriptionOperation. I went this way instead of resetVoiceInputGlobalsForTests() because it is not exported from client-runtime's voice-input barrel, and adding it would be an inline edit to an upstream file needing a fence and a watch: entry — disproportionate for a test helper. (The import has to run before the global stubs: the graph reaches @base-ui/utils/detectBrowser, which sniffs the real navigator at load.)

Guard: unmount can leak stubs. try { … } finally { vi.unstubAllGlobals(); }.

Guard: reconciliation invariant unasserted. Test 1 collects dictation.subscribeLevel across the three renderThread calls and asserts sessions.size === 1. It comes out of the useState initializer, so a remount would show up as three distinct values.

Guard: drafts.size === 0 proves nothing. Test 2 now also asserts phase === "idle" and error === null after the late transcript, with a comment on why — exactly the error-path drop the race would have produced.

Guard: keyboard path untested. New test 3. Each thread gets its own composer element; after switching to B, a chord targeted at A's editor is ignored and one targeted at B's starts and stops a recording that lands in B's draft. Fails pre-fix with expected 'recording' to be 'idle' — the stale composer accepts A's editor.

Guard header. Added to both this file and forkLocalDictation.test.ts, per the "both or neither" note.

Manifest intent. Now "A new recording targets the thread the composer is currently showing, and an owner change cancels an in-flight one rather than re-targeting it." The React implementation fact moved into the code comment, and the orphaned Cancellation stops owned work… line is re-wrapped.

Declined, with reasons

@cursor — switch to a render-phase inputRef.current = input. Not taking this one, and I think the stated benefit is backwards. A render-phase write does not close the window; it opens a wider one, and against an uncommitted target. Sequence with render-sync: render for B writes the ref, React yields before commit, the DOM and every other consumer still show A, and a transcript settling in that gap commits to B — a thread the user cannot see yet. With the mirror in the layout phase and ownerChanged() immediately after it, there is no gap at all: both run inside one synchronous commit, and anything settling before that commit correctly targets A, which is still what is on screen. Render-phase writes also latch inputs from concurrent renders that never commit. The trade-off is now written down in the code comment, next to the pointers to BranchToolbar and mobile's useVoiceInputController — which is the part of your finding I agree with: three shapes, no shared answer. A useLatestRef helper would settle it once, but that is a repo-wide change and not this PR.

Worth noting the BranchToolbar comment blames the React Compiler; the cause is the fiber tag, and BranchToolbar is itself memo(...). Same defect, misattributed — another reason the reasoning needed to be recorded somewhere accurate.

Third copy of the fake-DOM harness. Left in place. The other two copies are components/chat/useComposerMenuState.test.tsx and useComposerFocusState.test.tsx, both upstream-owned; extracting to apps/web/src/test/ means editing them, which under .fork/AGENTS.md needs fences and watch: entries on two files this PR otherwise does not touch. A helper only the guard imports would leave three copies anyway. I did restore the dropped // The probe renders no host nodes, but ReactDOM still needs an event target. line, so the copy is no longer the unexplained one.

2. useComposerPendingUserInputCard.ts:75. Agreed it is real and agreed it is a follow-up — say the word and I will open one.

Gates

vp fmt --check clean; vp lint --report-unused-disable-directives exits 0 on all four changed files; web typecheck clean; all 453 fork guards pass; .fork/lint-owned.mjs reports no blocking warnings and .fork/detect-drift.mjs is clean on the changed set. All four guard tests still fail against a copy of the pre-fix hook.

Unrelated: Release Smoke is red

ERR_PNPM_UNUSED_PATCH: expo-audio@57.0.4 — registry drift, not this PR. apps/mobile/package.json carries "expo-audio": "~57.0.4" while pnpm-workspace.yaml patches the exact 57.0.4, and the smoke check regenerates the lockfile from scratch, so it resolves past the patched version. Same shape as expo-sharing in #109. Needs a one-line pin on custom, then gh pr update-branch here.

🤖 Generated with Claude Code