Repository navigation
iOS dictation: fix conventions-lint namespace-enum (main-wide red) + harden mic-start crash - #6238
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthrough
ChangesComposerDictationController async/await and audio safety
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes two follow-up issues from the #6197 on-device voice dictation merge: converts the caseless namespace-enum
Confidence Score: 5/5Safe to merge. Happy-path behavior is unchanged; the only runtime differences are graceful-failure on bad audio input routes (previously an uncatchable crash) and elimination of a main-actor executor trap on iOS 26 repeat taps. The actor-isolation model is correct throughout: nonisolated factory methods return closures with no main-actor isolation, a single Task { @mainactor } hop owns all state mutations, and weak-self captures prevent retain cycles. The synchronous fast-path for already-resolved permissions is safe because everything runs on the main actor and the state transitions are atomic. The sampleRate guard is a conservative addition that only changes behavior on devices that previously would have thrown an uncatchable exception. No new blocking primitives, no user-facing strings, and no source-control artifacts are introduced. No files require special attention. Important Files Changed
Reviews (4): Last reviewed commit: "iOS dictation: fix mic-tap crash, text l..." | Re-trigger Greptile |
…ic crash) cmux Beta crashed when using the mic. beginRecognition() guarded only channelCount > 0 before installTap, but installTap raises an UNCATCHABLE Obj-C exception (IsFormatSampleRateAndChannelCountValid) on an invalid input format (zero sample rate), which do/catch can't trap. Also require sampleRate > 0 and failStart() gracefully. Addresses issue #6217; will confirm the exact frame against the device crash report and add an Obj-C exception shim if the crash is a format MISMATCH rather than an invalid format. (The conventions-lint fix this branch originally carried is superseded by main's ComposerDictationTextMerger refactor; reset to main and kept only the mic guard.)
5187de9 to
c3bd4ab
Compare
Real root cause (device crash log, build 1.0.3 20260616105751): EXC_BREAKPOINT in swift_task_isCurrentExecutor -> dispatch_assert_queue_fail, inside the TCC permission callback. Tapping the mic requests speech+mic permission; SFSpeechRecognizer / AVAudioApplication invoke their completion handlers on their own (non-main) queues. The closures were inferred main-actor (written in this @mainactor controller), so under Swift 6 / iOS 26 the runtime asserts executor isolation when the system calls them off-main and traps. Fix (modern Swift 6 isolation): requestAuthorization + requestMicrophone- Permission are now nonisolated with Sendable completions, so no main-actor closure is invoked off-main; start() hops to the main actor exactly once via a Task @mainactor enqueue (not a synchronous executor assertion). Also keeps the installTap sample-rate guard (separate latent crash on an invalid input format). Compile verified on CI; runtime confirmed by iOS 26 device dogfood. Closes #6217.
…prompt The mic-tap crash on iOS 26 was a Swift-concurrency executor trap: closures the compiler inferred as @mainactor (the permission completions, the AVAudioEngine tap block, and the SFSpeechRecognitionTask result handler) were invoked off the main thread by TCC / the realtime audio thread / the recognition queue, tripping swift_task_isCurrentExecutor -> dispatch_assert_queue_fail (EXC_BREAKPOINT). Fixes: - Build the tap block and the result handler in `nonisolated` factory methods so they are genuinely non-isolated and safe to invoke off-main. - Resolve speech+mic authorization synchronously when already determined (`resolvedAuthorization()`), so a repeat tap never re-invokes the crashing async TCC callback; only a never-determined permission uses the async request path. This also keeps the permission prompt gated to the first mic tap (status reads never prompt). - Make the authorization completions nonisolated/@sendable. - Ignore an empty final transcript on stop so it cannot wipe the words the partial results already committed to the composer. Verified on-device (iPhone, iOS 26) through the full start -> listen -> stop cycle with no crash and text preserved. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hor (#6299) * ios: sustain hold-to-repeat Backspace via a virtual delete-repeat anchor Hold-to-repeat Backspace on the iOS soft keyboard never repeated. Device debugging confirmed the cause: with an EMPTY virtual document, UIKit no-ops the keyboard's software delete and never calls deleteBackward() while the key is held, so backspace did not reach the Mac. Forcing hasText == true alone (the earlier #6288 attempt) does not change this. Fix: convert TerminalInputTextView from a UITextView subclass to a bare UIView conforming to UIKeyInput + UITextInput that exposes a one-character virtual document. When not composing it shows a hidden zero-width "delete-repeat anchor" (toggling \u{200B}/\u{2060}); each empty-buffer deleteBackward() forwards a real backspace via onBackspace and brackets the anchor toggle in inputDelegate.textWillChange/textDidChange so UIKit re-arms its document-driven key-repeat timer. IME composition (markedText) suppresses the anchor; a delete during composition cancels the composition instead of forwarding a stray backspace. Supersedes the hasText-only approach in #6288 (can be closed in favor of this); related to #6238. Single commit (not red/green): the fix is a whole-view rewrite from UITextView to UIView/UIKeyInput/UITextInput, so the failing test and the view it tests are inseparable. The new TerminalInputBackspaceRepeatTests asserts the observable invariants that sustain the repeat (non-empty 1-char document when idle, N deletes => N backspaces with the document re-armed non-empty after each, the textWillChange/textDidChange re-arm firing per delete, the anchor char alternating, and composing suppressing both the anchor and any stray backspace), so reverting to UITextView, dropping the anchor, or removing the re-arm all fail it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Refresh Swift file-length budget for documentless input rewrite TerminalInputTextView grew with the hand-rolled UIKeyInput/UITextInput conformance (the proven delete-repeat fix). Accepting as known debt; splitting the conformance into an extension file is a follow-up (file-org). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * ios: shorten IME composition on composing-delete instead of cancelling While composing CJK/Japanese/Korean/pinyin text the documentless input view cancelled the entire marked composition on the first Backspace and emitted nothing, so mid-composition correction was impossible (en+ja are supported locales). Restore the prior UITextView behavior: deleteBackward() during composition now drops the last grapheme (Character, not a UTF-16 unit, so multi-scalar glyphs are never split) and re-presents the shortened candidate via setMarkedText, which brackets the change in textWillChange/textDidChange and derives markedTextRange/selectedTextRange from the new string so UIKit and the IME stay in sync. Removing the last unit clears/unmarks the composition. Still emits zero bytes to the Mac while composing; the non-composing forward-DEL + anchor re-arm path is unchanged. Update the regression test: a multi-char composition shortens by one per delete (still composing, zero onBackspace), clears on the last unit, and a subsequent non-composing delete forwards a backspace again. Adds a grapheme-boundary case (flag emoji = one grapheme, multiple UTF-16 units). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Refresh Swift file-length budget after IME composing-delete fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: conform mock UITextInputDelegate to the iOS 18.4 SDK (conversationContext) main's bump to the iOS 26 SDK made conversationContext(_:didChange:) a required UITextInputDelegate method; the backspace-repeat test's mock delegate didn't implement it, failing the cmuxFeatureTests build. Add the unused stub. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hor (#6299)
* ios: sustain hold-to-repeat Backspace via a virtual delete-repeat anchor
Hold-to-repeat Backspace on the iOS soft keyboard never repeated. Device
debugging confirmed the cause: with an EMPTY virtual document, UIKit no-ops
the keyboard's software delete and never calls deleteBackward() while the key
is held, so backspace did not reach the Mac. Forcing hasText == true alone
(the earlier #6288 attempt) does not change this.
Fix: convert TerminalInputTextView from a UITextView subclass to a bare
UIView conforming to UIKeyInput + UITextInput that exposes a one-character
virtual document. When not composing it shows a hidden zero-width
"delete-repeat anchor" (toggling \u{200B}/\u{2060}); each empty-buffer
deleteBackward() forwards a real backspace via onBackspace and brackets the
anchor toggle in inputDelegate.textWillChange/textDidChange so UIKit re-arms
its document-driven key-repeat timer. IME composition (markedText) suppresses
the anchor; a delete during composition cancels the composition instead of
forwarding a stray backspace.
Supersedes the hasText-only approach in
manaflow-ai/cmux#6288 (can be closed in favor of
this); related to manaflow-ai/cmux#6238.
Single commit (not red/green): the fix is a whole-view rewrite from
UITextView to UIView/UIKeyInput/UITextInput, so the failing test and the view
it tests are inseparable. The new TerminalInputBackspaceRepeatTests asserts
the observable invariants that sustain the repeat (non-empty 1-char document
when idle, N deletes => N backspaces with the document re-armed non-empty
after each, the textWillChange/textDidChange re-arm firing per delete, the
anchor char alternating, and composing suppressing both the anchor and any
stray backspace), so reverting to UITextView, dropping the anchor, or removing
the re-arm all fail it.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Refresh Swift file-length budget for documentless input rewrite
TerminalInputTextView grew with the hand-rolled UIKeyInput/UITextInput
conformance (the proven delete-repeat fix). Accepting as known debt; splitting the
conformance into an extension file is a follow-up (file-org).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* ios: shorten IME composition on composing-delete instead of cancelling
While composing CJK/Japanese/Korean/pinyin text the documentless input
view cancelled the entire marked composition on the first Backspace and
emitted nothing, so mid-composition correction was impossible (en+ja are
supported locales). Restore the prior UITextView behavior: deleteBackward()
during composition now drops the last grapheme (Character, not a UTF-16
unit, so multi-scalar glyphs are never split) and re-presents the shortened
candidate via setMarkedText, which brackets the change in
textWillChange/textDidChange and derives markedTextRange/selectedTextRange
from the new string so UIKit and the IME stay in sync. Removing the last
unit clears/unmarks the composition. Still emits zero bytes to the Mac while
composing; the non-composing forward-DEL + anchor re-arm path is unchanged.
Update the regression test: a multi-char composition shortens by one per
delete (still composing, zero onBackspace), clears on the last unit, and a
subsequent non-composing delete forwards a backspace again. Adds a
grapheme-boundary case (flag emoji = one grapheme, multiple UTF-16 units).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Refresh Swift file-length budget after IME composing-delete fix
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: conform mock UITextInputDelegate to the iOS 18.4 SDK (conversationContext)
main's bump to the iOS 26 SDK made conversationContext(_:didChange:) a required
UITextInputDelegate method; the backspace-repeat test's mock delegate didn't
implement it, failing the cmuxFeatureTests build. Add the unused stub.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two #6197 (on-device voice dictation) follow-ups
#6197 merged with two latent issues because the iOS checks that catch them are non-required (the iOS false-green gap):
1.
package-conventions-lint— main-wide red (priority)ComposerDictationTextMergewas a caseless namespace-enum with astatic func, which the namespace-type rule bans:Since the lint isn't a required check, #6197 merged red and every PR against current main now fails
package-conventions-lint. Converted it to an instantiablestruct(base/transcriptstored,mergedcomputed) — keeps the dictation-scoped name + host-testability, satisfies the lint. Updated the controller call site and the merge tests../scripts/lint-ios-package-conventions.shnow passes.2. mic-start crash hardening (issue #6217)
beginRecognition()guarded onlychannelCount > 0beforeinstallTap, butinstallTapraises an uncatchable Obj-C exception on an invalid input format (zero sample rate) thatdo/catchcan't trap. Now also requiresampleRate > 0andfailStart()gracefully. This addresses the reported "cmux Beta crashes when using the microphone." I'll confirm the exact frame against the device crash report and add an Obj-C exception shim if it turns out to be a format mismatch rather than an invalid format.No behavior change to the happy path. Unblocks the fleet's conventions-lint immediately.
🤖 Generated with Claude Code
Note
Medium Risk
Changes the live microphone/dictation startup path and Swift 6 actor isolation around system callbacks; behavior is mostly crash prevention and edge-case guards, but regressions in permission or transcript merging would be user-visible.
Overview
Fixes the mic-tap crash and related dictation failures by avoiding main-actor-isolated closures on Speech/AVFoundation and audio realtime threads, and by tightening startup guards.
When speech and mic permissions are already resolved,
startnow uses synchronousresolvedAuthorization()and goes straight to recognition (orunavailableif denied), skipping async TCC callbacks that were trapping on repeat taps. First-time prompts still use async authorization, but completions arenonisolated/@Sendablewith a singleTask { @MainActor in }before mutating controller state.Recognition setup moves the engine tap and recognition result handler into
nonisolatedbuilders so callbacks are not main-actor-isolated off the render/arbitrary queues.beginRecognitionalso requiressampleRate > 0beforeinstallTap, and empty final/partial transcripts are ignored so a graceful stop does not overwrite text already committed from partials.Reviewed by Cursor Bugbot for commit 94bf8c6. Bugbot is set up for automated code reviews on this repo. Configure here.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes the iOS dictation mic-tap crash and prevents text loss. Authorization and audio callbacks are now Swift 6–safe, permission prompts are gated to the first tap, and input formats are validated before the tap.
requestAuthorizationandrequestMicrophonePermissionuse@Sendablecompletions;startreadsresolvedAuthorization()and hops once to@MainActor.nonisolatedfactories, then hop to main before touching state.installTap: requireformat.channelCount > 0andformat.sampleRate > 0; otherwise callfailStart().Written for commit 94bf8c6. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Improvements