Repository navigation
iOS voice: GPT Realtime across paired Macs - #7314
lawrencecchen wants to merge 35 commits into
Conversation
Restructure iOS settings into nested pages (Terminal, Browser, Voice, Notifications, About, Privacy, Troubleshooting) with privacy policy, terms, and support links to cmux.com, license acknowledgements, and a new /support page on the website. Add a CmuxVoice package with a voice-engine abstraction: Apple speech recognition stays the zero-download default, and NVIDIA Parakeet v3 (FluidAudio 0.15.4, ~480 MB CoreML, on-device, 25 languages) is user- downloadable from Settings > Voice with progress, cancel, and delete. Composer dictation gains a recognition-backend seam so the engine choice applies everywhere; the Apple path's iOS 26 crash workarounds are preserved verbatim. Add Voice Mode: the iPhone acts as a microphone for the paired Mac. The Mac streams focus.updated events (new MobileFocusObserver, active- window gated, 50ms throttle) so the phone shows in real time which workspace/pane the mouse selected; finalized utterances insert into that pane via the new mobile.voice.input RPC (no_focused_terminal when a non-terminal pane is focused), with an auto-submit toggle. New capabilities focus.events.v1 and voice.input.v1; scoped tickets are denied. Focus subscriptions resubscribe per connection generation and unsubscribe on exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Voice Mode transcription, Parakeet model management, mobile focus/voice-input RPCs, browser search settings, a route-based iOS settings flow, and a localized web support page. ChangesiPhone Voice Mode and Dictation Backend
Mobile Browser Search Engine Settings
iOS Settings Navigation Restructure
Web Support Page
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (8 errors, 2 warnings)
✅ Passed checks (15 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 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac77d5d5f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| guard !isFinished else { return } | ||
| isFinished = true | ||
| audioContinuation.finish() | ||
| Task { [manager, continuation, updateTask, audioTask] in |
There was a problem hiding this comment.
When the user stops or dismisses a Parakeet session while AsrModels.downloadAndLoad is still in progress, finish() closes audio and later cancels the manager, but it never cancels the startupTask created in init (unlike cancel()). That startup task can still complete and call loadModels/startStreaming after this cleanup, resurrecting the CoreML/ASR session or racing the next mic session. Please cancel/await the startup task as part of the graceful finish path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8351ea6 and 9f35786: finish() now cancels the startup task and the finalize task awaits it; a cancelled/failed startup skips manager.finish() and cleans up. The audio task also waits for startup success before forwarding, so a resurrected pipeline cannot race the next session.
— Claude Code
| self.task = recognizer?.recognitionTask( | ||
| with: request, | ||
| resultHandler: Self.makeRecognitionResultHandler(continuation: continuation) | ||
| ) |
There was a problem hiding this comment.
On devices/locales where SFSpeechRecognizer() returns nil, this optional chain leaves task nil while the AsyncStream remains open. Voice Mode still starts the microphone and flips to listening after the audio engine starts, so users get an active mic with no transcript or error, and finish() cannot ever finish the stream because there is no recognition task. Please reject unsupported recognizers up front or yield a failure and close the stream here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8351ea6: a nil SFSpeechRecognizer now yields a localized .failed and finishes the stream in init, so Voice Mode can never sit listening with no recognition task behind it.
— Claude Code
Greptile SummaryThis PR ships three features together: nested iOS Settings pages, a downloadable NVIDIA Parakeet on-device ASR engine (via FluidAudio), and a full-screen Voice Mode that streams Mac focus state in real time and inserts transcribed speech into the focused terminal. The implementation is substantial (~9 k net lines) and the core correctness mechanisms—generation-aware focus subscriptions, fail-closed
Confidence Score: 5/5Safe to merge. The fail-closed targeting guards (expected workspace/surface IDs checked server-side before any text is inserted) and generation-aware subscription loop are the highest-risk paths and both look correct. All correctness-critical paths—text insertion into the focused terminal, focus subscription lifecycle, download state transitions, and error handling—have explicit guards or are covered by tests. The one notable gap (silent filesystem error on model delete in settings) is a UX inconvenience rather than a data-integrity issue. Localization is complete for all new user-facing strings. Package.resolved lockfiles are fully updated. MobileVoiceSettingsPage.swift — the deleteModel(for:) error path swallows failures silently; worth a follow-up to surface a localised error in the UI. Important Files Changed
Sequence DiagramsequenceDiagram
participant iOS as VoiceModeView (iPhone)
participant Shell as MobileShellComposite
participant Mac as TerminalController (Mac)
participant Obs as MobileFocusObserver
iOS->>Shell: startVoiceFocusUpdates()
Shell->>Mac: "mobile.events.subscribe {focus.updated}"
Shell->>Mac: mobile.focus.get
Mac-->>Shell: MobileFocusSnapshot (seed)
Shell-->>iOS: voiceFocusSnapshot updated
Obs->>Mac: focus changes detected (50ms throttle, hash-diff)
Mac->>Shell: focus.updated event
Shell-->>iOS: voiceFocusSnapshot updated
iOS->>iOS: ASR → .final(text)
iOS->>Shell: sendVoiceInput(text, expectedWorkspaceID, expectedSurfaceID)
Shell->>Mac: mobile.voice.input
alt IDs match focused terminal
Mac-->>Shell: workspace_id, surface_id, surface_title
Shell-->>iOS: MobileVoiceInputResponse
else IDs stale
Mac-->>Shell: error target_changed
Shell-->>iOS: rpcError target_changed
end
iOS->>Shell: dismiss (task cancelled)
Shell->>Mac: mobile.events.unsubscribe
Reviews (19): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /// Factory consulted for every dictation start. The app composition root | ||
| /// swaps this to Parakeet when the user selected it and its model is installed. | ||
| public static var backendFactory: @MainActor () -> any ComposerDictationRecognitionBackend = { | ||
| AppleComposerDictationRecognitionBackend() |
There was a problem hiding this comment.
public static var backendFactory
backendFactory is a mutable global closure stored at class scope and written by the composition root after construction (ComposerDictationController.backendFactory = { … }). This violates the no-ambient-global-state rule: the canonical shape is a backendFactory parameter in ComposerDictationController.init(), captured as a stored property and called on every start(). As written, any future second instance of ComposerDictationController silently inherits the application-wide factory, and tests that swap the factory leave it mutated for subsequent test cases unless they restore it.
Rule Used: Flag new ambient global state in production Swift:... (source)
There was a problem hiding this comment.
Consciously kept as a composition-root seam, with the rationale now documented on the property (8351ea6): the two controllers are constructed where no DI path exists today (UIKit-hosted TerminalComposerView built by GhosttySurfaceRepresentable, and CmuxAgentChatUI which has no CmuxVoice dependency). AppCompositionRoot installs it once at process start before any composer view exists. Constructor injection lands when composer hosting is unified.
— Claude Code
| MobileHostService.shared.emitEvent(topic: "focus.updated", payload: snapshot.jsonObject()) | ||
| } | ||
|
|
||
| #if DEBUG | ||
| static func summaryHashForTesting(tabManager: TabManager) -> Int { |
There was a problem hiding this comment.
Sources/
summaryHashForTesting is a #if DEBUG-guarded static method whose name explicitly declares its test-only purpose. Adding it to Sources/Mobile/MobileFocusObserver.swift (a production Sources/ path) is exactly the pattern the cmux-no-test-debug-seam-in-production-source rule prohibits. The canonical fix — per cmux PR 6452 — is to widen summaryHash from private to internal on MobileFocusSnapshotPayload and read it from the test target via @testable import, removing this accessor from the production type entirely.
Rule Used: Flag Swift files under a production Sources path (... (source)
| defaultValue: "Model files were not found after download." | ||
| )) | ||
| } catch { |
There was a problem hiding this comment.
error.localizedDescription surfaces raw implementation details in user-facing UI
self.state = .failed(error.localizedDescription) passes a raw system or SDK error string directly into ParakeetDownloadState.failed(String), which MobileVoiceSettingsPage renders verbatim as a red Text(message). URLErrors, CoreML compilation errors, and FluidAudio SDK errors produce strings like "The operation couldn't be completed. (NSURLErrorDomain error -1009.)" — exactly the implementation-detail exposure the cmux-user-facing-errors rule forbids. The same pattern appears in ParakeetTranscriptionSession.finish() (line 68 — continuation.yield(.failed(error.localizedDescription))) and AppleVoiceTranscriptionSession.makeRecognitionResultHandler (line 64), both of which feed the VoiceModeView errorMessage label. All three sites should translate the error to cmux-domain copy — e.g. "Couldn't download voice model. Check your connection and try again." — and reserve localizedDescription for sanitised logs only.
There was a problem hiding this comment.
Fixed in 8351ea6: all three sites (ParakeetModelStore download failure, ParakeetTranscriptionSession startup/finalize, AppleVoiceTranscriptionSession recognition errors) now show localized cmux-domain copy (en+ja) and log the underlying error to OSLog.
— Claude Code
MobileShellComposite (+156: connection-generation-aware focus subscription must live beside the private connection internals it guards), TerminalAndGhosttyTests (+85: mobile.voice.input tests), WorkspaceListView (+20: voice mode entry point), TerminalController (+9), AppDelegate (+6), MobileHostService (+2): dispatch cases and observer wiring at existing switch points. Also clear the stale partial transcript when a Voice Mode session ends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Re-evaluate the selected dictation backend after denial
When the active backend is Apple and Speech authorization is denied, this puts the long-lived ComposerDictationController into .unavailable; the composer views keep that controller in @State, and start() will no longer reevaluate backendFactory because state.canStart fails. Since Parakeet only needs microphone permission, a user who denied Speech and then installs/selects Parakeet still has a disabled mic until the view/app is recreated. Please make the unavailable state backend-specific or reset it when the selected engine/model changes.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let engine = voiceSettings.effectiveEngine(modelInstalled: parakeetModelStore.isInstalled) | ||
| let permitted = await VoicePermissionRequester().requestPermissions(for: engine) | ||
| guard permitted else { | ||
| isStarting = false | ||
| errorMessage = L10n.string("mobile.voiceMode.permissionDenied", defaultValue: "Microphone or speech recognition permission is not available.") | ||
| return | ||
| } | ||
| guard isStarting else { return } |
There was a problem hiding this comment.
On first use, the permission prompt can suspend here while the Mac focus moves away from a terminal, the host is switched, or the connection/capabilities change. After the await, only isStarting is checked, so Voice Mode can still start the microphone even though canStartListening has become false, leaving the user recording against no valid target and only failing later when sending the final transcript. Re-check canStartListening after permissions before creating the session.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9f35786: after the permission await, startListening re-checks the voice target (hasVoiceTarget: capabilities + focused-terminal snapshot) in addition to the generation/isStarting guards, and aborts the start when the target went away.
— Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileAboutSettingsPage.swift`:
- Around line 21-38: The legal/support links in MobileAboutSettingsPage are
hardcoded and duplicated, so centralize them into shared URL constants or a
helper used by settingsLink instead of repeating the literal cmux.com URLs here
and in MobilePrivacySettingsPage. Update the existing MobileAboutSettingsPage
link calls to reference the shared symbols for privacy policy, terms of service,
and support so future URL changes only need one edit.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileNotificationsSettingsPage.swift`:
- Around line 12-29: The MobileNotificationsSettingsPage toggle button can start
overlapping enable()/disable() calls because repeated taps are allowed while the
Task is still running. Add simple in-flight state in the button action around
the existing pushCoordinator enable/disable flow, and use that state to disable
the Button until the current Task completes; locate the change in
MobileNotificationsSettingsPage’s Button and the notificationsEnabled toggle
logic.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileVoiceSettingsPage.swift`:
- Around line 128-130: The progressText(_:) helper in MobileVoiceSettingsPage
currently builds the percentage string with a fixed format, which ignores
locale-specific percent rendering. Update progressText(_:) to use locale-aware
percent formatting via the value’s formatted percent style so it follows the
user’s locale, and keep the change localized to that helper.
- Around line 94-110: The delete action in MobileVoiceSettingsPage’s .installed
branch resets voiceSettings.selectedEngine even when
ParakeetModelStore.deleteModel() fails because the error is swallowed by try?.
Change the Button action to only update the engine after a successful delete,
and handle the thrown error from deleteModel() explicitly so the selection stays
in sync with the actual model state. Use the existing
ParakeetModelStore.deleteModel, voiceSettings.selectedEngine, and .parakeetV3
handling to locate the fix.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/VoiceModeView.swift`:
- Around line 262-265: In VoiceModeView, replace the `String(format:)` usage for
`sendConfirmation` with `String.localizedStringWithFormat` so the localized
template from `L10n.string("mobile.voiceMode.sentToFormat", ...)` can handle
locale-aware substituted arguments correctly. Update the formatting at the
`sendConfirmation` assignment to use the localized string as the format source
and pass `title` through `String.localizedStringWithFormat`, keeping the
`mobile.voiceMode.sentToFormat` key and related localization behavior intact.
- Around line 267-269: The catch block in VoiceModeView is surfacing raw
upstream error text to the UI via error.localizedDescription. Replace that
assignment with a friendly, user-safe message and, if needed, log the detailed
error separately for diagnostics. Update the error handling around
store.sendVoiceInput so MobileVoiceModeError only receives sanitized copy, not
protocol/socket/internal details.
In
`@Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationController.swift`:
- Around line 37-41: `ComposerDictationController` is using a process-wide
mutable `backendFactory` static instead of an injected dependency. Move backend
selection into `ComposerDictationController.init(backendFactory:textMerger:)`
with a default factory closure that creates
`AppleComposerDictationRecognitionBackend`, and update the composition root to
pass the desired backend when constructing the controller rather than mutating
shared static state. Keep the factory as an instance-owned dependency so tests
and per-instance overrides don’t rely on global mutation.
In
`@Packages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationRecognitionBackend.swift`:
- Around line 16-18: The
`ComposerDictationRecognitionBackend.requestAuthorization` API still uses a
completion handler for a single boolean result; update the protocol to return
`async -> Bool` instead. Adjust
`ParakeetComposerDictationRecognitionBackend.requestAuthorization` to bridge the
existing permission callbacks with `withCheckedContinuation`, and update all
callers to `await` the result rather than passing a closure.
In
`@Packages/iOS/CmuxVoice/Sources/CmuxVoice/AppleVoiceTranscriptionSession.swift`:
- Around line 56-69: The callback in AppleVoiceTranscriptionSession should not
forward Speech framework errors directly to the UI through
.failed(error.localizedDescription). Update the error handling inside the
closure returned by the transcription session to emit a generic, localized
user-facing failure string instead, and keep the existing result/finish flow in
place for the transcript handling.
- Around line 15-29: In AppleVoiceTranscriptionSession.init(recognizer:), fail
fast when speech recognition cannot start instead of leaving the AsyncStream
open. Before creating the recognitionTask, check the recognizer for nil and
availability (including isAvailable, and keep the existing
supportsOnDeviceRecognition behavior), then immediately finish the continuation
with a failed VoiceTranscriptionUpdate or equivalent error state so
VoiceModeView’s updates loop does not hang. Use the existing recognizer,
request, continuation, and task setup in AppleVoiceTranscriptionSession to route
unavailable-recognizer cases into the same error handling path as
recognitionTask failures.
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetDownloadProgress.swift`:
- Around line 4-18: `ParakeetDownloadProgress.phaseDescription` is currently a
raw display string, which makes
`FluidAudioParakeetModelDownloader.progress(from:)` pass through hardcoded
English UI text. Change `ParakeetDownloadProgress` to use a `Sendable` enum for
the phase (for example the cases used by `progress(from:)`), then have the
UI/localization layer map each case to localized text with
`String(localized:defaultValue:)` instead of storing user-facing strings in the
model.
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetModelStore.swift`:
- Around line 82-89: The catch block in ParakeetModelStore.downloadModel is
passing error.localizedDescription directly into state.failed, which can expose
raw upstream/vendor messages in MobileVoiceSettingsPage. Replace that
user-facing text with a generic localized failure message, and log the original
error through the app’s structured logger for diagnostics; keep the cancellation
path unchanged and continue guarding with downloadAttemptID and isCancellation.
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift`:
- Around line 35-38: The catch blocks in ParakeetTranscriptionSession are
surfacing raw upstream error text via error.localizedDescription, which can leak
internal model/provider details into VoiceModeView.handle(_) through the .failed
case. Update the failure path to yield a generic, localized user-facing message
instead of the upstream description, and keep the detailed error only in logs or
internal diagnostics. Apply the same mapping in both transcription/loading
failure paths referenced by the session’s error handling so all .failed
emissions are sanitized consistently.
- Around line 6-14: The ParakeetTranscriptionSession lifecycle is missing a
deallocation safety net, so startupTask, updateTask, and audioTask can keep
running if finish() or cancel() is never called. Add a deinit on
ParakeetTranscriptionSession that invokes cancel() so cleanup always happens
even when the session is dropped unexpectedly, and keep the cleanup logic
centralized in the existing cancel/finish path.
- Around line 76-86: `ParakeetTranscriptionSession.cancel()` is not idempotent
when `finish()` has already run or is running, so add the same `isFinished`
guard pattern used by `finish()` before doing any cleanup. Update `cancel()` to
return early if the session is already finished, and only then cancel
`startupTask`, `audioTask`, `updateTask`, finish `audioContinuation`, and spawn
the `Task` that calls `manager.cancel()` and `continuation.finish()`.
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/VoicePermissionRequester.swift`:
- Around line 1-43: The permission request flow in VoicePermissionRequester
duplicates the mic/Speech authorization logic already handled by
AppleComposerDictationRecognitionBackend. Extract the shared
permission-negotiation behavior into one helper (for example in CmuxVoice or
CmuxMobileSupport) and have both
VoicePermissionRequester.requestPermissions(for:) and the backend’s
resolvedAuthorization()/requestAuthorization() paths call that shared API. Keep
the iOS version-specific microphone handling and any TCC workaround in the
shared helper so both callers stay in sync.
In `@Sources/Mobile/MobileFocusObserver.swift`:
- Line 5: The file-scoped Logger constant mobileFocusObserverLog is implicitly
MainActor-isolated here and should be marked nonisolated to avoid unintended
main-actor coupling. Update the declaration in MobileFocusObserver.swift to use
nonisolated private let for mobileFocusObserverLog, keeping the logger available
without inheriting the file’s default MainActor isolation.
- Around line 60-64: Remove the unnecessary DEBUG-only test seam from
MobileFocusObserver by deleting summaryHashForTesting(tabManager:), since tests
already access MobileFocusSnapshotPayload.snapshot(tabManager:).summaryHash
directly. Keep the production source focused on the existing snapshot logic and
do not add any replacement accessor or wrapper.
In `@Sources/TerminalController`+MobileVoiceInput.swift:
- Around line 6-18: Extract the nil-field “unresolved” payload into a shared
factory on MobileFocusSnapshotPayload and use it from v2MobileFocusGet instead
of constructing the literal inline. Update
MobileFocusSnapshotPayload.snapshot(tabManager:) to call that shared helper for
the unresolved path so both call sites stay in sync. Keep the shared symbol easy
to locate by naming it alongside MobileFocusSnapshotPayload and
v2MobileFocusGet.
In `@web/app/`[locale]/(legal)/support/page.tsx:
- Around line 13-17: The Support page metadata is hard-coding the canonical URL,
which breaks locale-aware SEO for localized routes. Update the metadata returned
from the support page’s generateMetadata logic to use the locale-aware
buildAlternates(locale, "/support") helper instead of a fixed
alternates.canonical value, matching the pattern used by the other localized
pages and keeping the locale-specific canonical in sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b84612b-aca8-4aff-be04-d8956bc8248b
📒 Files selected for processing (66)
Packages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/MobileBrowserPane.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/MobileBrowserSearchEngine.swiftPackages/iOS/CmuxMobileBrowser/Sources/CmuxMobileBrowser/MobileBrowserSettings.swiftPackages/iOS/CmuxMobileBrowser/Tests/CmuxMobileBrowserTests/MobileBrowserSettingsTests.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileFocusSnapshot.swiftPackages/iOS/CmuxMobileRPC/Sources/CmuxMobileRPC/MobileVoiceInputResponse.swiftPackages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swiftPackages/iOS/CmuxMobileShellUI/Package.resolvedPackages/iOS/CmuxMobileShellUI/Package.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileAboutSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileBrowserSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileNotificationsSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobilePrivacySettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileTerminalSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileTroubleshootingSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileVoiceSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalShortcutsSettingsView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/VoiceModeView.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView+Toolbar.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/AppleComposerDictationRecognitionBackend.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationAudioEngine.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationController.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationRecognitionBackend.swiftPackages/iOS/CmuxMobileSupport/Sources/CmuxMobileSupport/ComposerDictationRecognitionUpdate.swiftPackages/iOS/CmuxVoice/Package.resolvedPackages/iOS/CmuxVoice/Package.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/AppleVoiceTranscriptionSession.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/FluidAudioParakeetModelDownloader.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetComposerDictationRecognitionBackend.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetDownloadProgress.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetDownloadState.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetModelDownloading.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetModelStore.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/VoiceEngineID.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/VoicePermissionRequester.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/VoiceSettingsStore.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/VoiceTranscriptionSession.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/VoiceTranscriptionUpdate.swiftPackages/iOS/CmuxVoice/Tests/CmuxVoiceTests/ParakeetModelStoreTests.swiftPackages/iOS/CmuxVoice/Tests/CmuxVoiceTests/VoiceSettingsStoreTests.swiftResources/Localizable.xcstringsSources/AppDelegate.swiftSources/Mobile/MobileFocusObserver.swiftSources/Mobile/MobileFocusSnapshotPayload.swiftSources/Mobile/MobileHostService+Capabilities.swiftSources/Mobile/MobileHostService.swiftSources/TerminalController+MobileVoiceInput.swiftSources/TerminalController.swiftcmux.xcodeproj/project.pbxprojcmux.xcworkspace/contents.xcworkspacedatacmuxTests/MobileWorkspaceListFidelityTests.swiftcmuxTests/TerminalAndGhosttyTests.swiftios/CHANGELOG.mdios/cmux/AppCompositionRoot.swiftios/cmux/Resources/Localizable.xcstringsios/cmux/cmuxApp.swiftios/cmuxPackage/Package.resolvedios/cmuxPackage/Package.swiftios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swiftweb/app/[locale]/(legal)/support/page.tsxweb/app/[locale]/components/site-footer.tsxweb/messages/en.jsonweb/messages/ja.json
| settingsLink( | ||
| title: L10n.string("mobile.settings.privacyPolicy", defaultValue: "Privacy Policy"), | ||
| systemImage: "hand.raised", | ||
| url: URL(string: "https://cmux.com/privacy-policy")!, | ||
| identifier: "MobileSettingsAboutPrivacyPolicy" | ||
| ) | ||
| settingsLink( | ||
| title: L10n.string("mobile.settings.termsOfService", defaultValue: "Terms of Service"), | ||
| systemImage: "doc.text", | ||
| url: URL(string: "https://cmux.com/terms-of-service")!, | ||
| identifier: "MobileSettingsAboutTerms" | ||
| ) | ||
| settingsLink( | ||
| title: L10n.string("mobile.settings.support", defaultValue: "Support"), | ||
| systemImage: "questionmark.circle", | ||
| url: URL(string: "https://cmux.com/support")!, | ||
| identifier: "MobileSettingsAboutSupport" | ||
| ) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
De-duplicate the hardcoded cmux.com legal/support URLs.
https://cmux.com/privacy-policy is hardcoded here and again in MobilePrivacySettingsPage.swift (and terms-of-service/support only appear here). If these URLs ever change, every occurrence must be updated in lockstep or they'll silently drift.
♻️ Proposed fix: shared URL constants
+private enum CmuxLegalURL {
+ static let privacyPolicy = URL(string: "https://cmux.com/privacy-policy")!
+ static let termsOfService = URL(string: "https://cmux.com/terms-of-service")!
+ static let support = URL(string: "https://cmux.com/support")!
+}
+
struct MobileAboutSettingsPage: View {
var body: some View {
Form {
...
settingsLink(
title: L10n.string("mobile.settings.privacyPolicy", defaultValue: "Privacy Policy"),
systemImage: "hand.raised",
- url: URL(string: "https://cmux.com/privacy-policy")!,
+ url: CmuxLegalURL.privacyPolicy,
identifier: "MobileSettingsAboutPrivacyPolicy"
)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileAboutSettingsPage.swift`
around lines 21 - 38, The legal/support links in MobileAboutSettingsPage are
hardcoded and duplicated, so centralize them into shared URL constants or a
helper used by settingsLink instead of repeating the literal cmux.com URLs here
and in MobilePrivacySettingsPage. Update the existing MobileAboutSettingsPage
link calls to reference the shared symbols for privacy policy, terms of service,
and support so future URL changes only need one edit.
| Button { | ||
| Task { | ||
| if notificationsEnabled { | ||
| await pushCoordinator.disable() | ||
| notificationsEnabled = false | ||
| } else { | ||
| notificationsEnabled = await pushCoordinator.enable() | ||
| } | ||
| } | ||
| } label: { | ||
| Label( | ||
| notificationsEnabled | ||
| ? L10n.string("mobile.notifications.disable", defaultValue: "Turn Off Agent Notifications") | ||
| : L10n.string("mobile.notifications.enable", defaultValue: "Notify Me About Agents"), | ||
| systemImage: notificationsEnabled ? "bell.slash" : "bell" | ||
| ) | ||
| } | ||
| .accessibilityIdentifier("MobileSettingsNotifications") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
No guard against overlapping toggle taps.
Rapidly tapping the button before the in-flight Task completes can fire a second enable()/disable() call while the first is still awaiting, since the button isn't disabled and there's no in-flight tracking. This is a minor UX edge case (likely self-recovering once both calls settle) rather than a functional blocker.
🔒 Optional guard against re-entrant taps
struct MobileNotificationsSettingsPage: View {
`@Environment`(MobilePushCoordinator.self) private var pushCoordinator
`@State` private var notificationsEnabled = false
+ `@State` private var isUpdating = false
var body: some View {
Form {
Section(L10n.string("mobile.settings.notifications", defaultValue: "Notifications")) {
Button {
+ guard !isUpdating else { return }
+ isUpdating = true
Task {
+ defer { isUpdating = false }
if notificationsEnabled {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Button { | |
| Task { | |
| if notificationsEnabled { | |
| await pushCoordinator.disable() | |
| notificationsEnabled = false | |
| } else { | |
| notificationsEnabled = await pushCoordinator.enable() | |
| } | |
| } | |
| } label: { | |
| Label( | |
| notificationsEnabled | |
| ? L10n.string("mobile.notifications.disable", defaultValue: "Turn Off Agent Notifications") | |
| : L10n.string("mobile.notifications.enable", defaultValue: "Notify Me About Agents"), | |
| systemImage: notificationsEnabled ? "bell.slash" : "bell" | |
| ) | |
| } | |
| .accessibilityIdentifier("MobileSettingsNotifications") | |
| struct MobileNotificationsSettingsPage: View { | |
| `@Environment`(MobilePushCoordinator.self) private var pushCoordinator | |
| `@State` private var notificationsEnabled = false | |
| `@State` private var isUpdating = false | |
| var body: some View { | |
| Form { | |
| Section(L10n.string("mobile.settings.notifications", defaultValue: "Notifications")) { | |
| Button { | |
| guard !isUpdating else { return } | |
| isUpdating = true | |
| Task { | |
| defer { isUpdating = false } | |
| if notificationsEnabled { | |
| await pushCoordinator.disable() | |
| notificationsEnabled = false | |
| } else { | |
| notificationsEnabled = await pushCoordinator.enable() | |
| } | |
| } | |
| } label: { | |
| Label( | |
| notificationsEnabled | |
| ? L10n.string("mobile.notifications.disable", defaultValue: "Turn Off Agent Notifications") | |
| : L10n.string("mobile.notifications.enable", defaultValue: "Notify Me About Agents"), | |
| systemImage: notificationsEnabled ? "bell.slash" : "bell" | |
| ) | |
| } | |
| .accessibilityIdentifier("MobileSettingsNotifications") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileNotificationsSettingsPage.swift`
around lines 12 - 29, The MobileNotificationsSettingsPage toggle button can
start overlapping enable()/disable() calls because repeated taps are allowed
while the Task is still running. Add simple in-flight state in the button action
around the existing pushCoordinator enable/disable flow, and use that state to
disable the Button until the current Task completes; locate the change in
MobileNotificationsSettingsPage’s Button and the notificationsEnabled toggle
logic.
| private func progressText(_ value: Double) -> String { | ||
| String(format: "%.0f%%", value * 100) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Percent text isn't locale-aware.
String(format: "%.0f%%", ...) hardcodes an English-style percent layout (e.g. French puts a space before %). Prefer .formatted(.percent...) for locale-correct rendering.
♻️ Proposed fix
private func progressText(_ value: Double) -> String {
- String(format: "%.0f%%", value * 100)
+ value.formatted(.percent.precision(.fractionLength(0)))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private func progressText(_ value: Double) -> String { | |
| String(format: "%.0f%%", value * 100) | |
| } | |
| private func progressText(_ value: Double) -> String { | |
| value.formatted(.percent.precision(.fractionLength(0))) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileVoiceSettingsPage.swift`
around lines 128 - 130, The progressText(_:) helper in MobileVoiceSettingsPage
currently builds the percentage string with a fixed format, which ignores
locale-specific percent rendering. Update progressText(_:) to use locale-aware
percent formatting via the value’s formatted percent style so it follows the
user’s locale, and keep the change localized to that helper.
| #if os(iOS) | ||
| import AVFoundation | ||
| import Foundation | ||
| import Speech | ||
|
|
||
| /// Requests microphone and Speech permissions for voice capture. | ||
| public struct VoicePermissionRequester { | ||
| /// Creates a permission requester. | ||
| public init() {} | ||
|
|
||
| /// Requests permissions needed by the selected voice engine. | ||
| /// - Parameter engine: The engine that will run. | ||
| /// - Returns: `true` when the required permissions were granted. | ||
| public func requestPermissions(for engine: VoiceEngineID) async -> Bool { | ||
| let micGranted = await Self.requestMicrophonePermission() | ||
| guard micGranted else { return false } | ||
| guard engine == .apple else { return true } | ||
| return await Self.requestSpeechPermission() | ||
| } | ||
|
|
||
| private nonisolated static func requestSpeechPermission() async -> Bool { | ||
| await withCheckedContinuation { continuation in | ||
| SFSpeechRecognizer.requestAuthorization { status in | ||
| continuation.resume(returning: status == .authorized) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private nonisolated static func requestMicrophonePermission() async -> Bool { | ||
| await withCheckedContinuation { continuation in | ||
| if #available(iOS 17.0, *) { | ||
| AVAudioApplication.requestRecordPermission { granted in | ||
| continuation.resume(returning: granted) | ||
| } | ||
| } else { | ||
| AVAudioSession.sharedInstance().requestRecordPermission { granted in | ||
| continuation.resume(returning: granted) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Duplicate permission-negotiation logic vs. AppleComposerDictationRecognitionBackend.
This file re-implements mic + Speech authorization requesting that already exists in AppleComposerDictationRecognitionBackend.resolvedAuthorization()/requestAuthorization(). The two flows can drift — a future iOS-version workaround applied to one (like the existing TCC-callback crash fix) won't automatically apply to the other.
Consider extracting a single shared permission-negotiation helper in CmuxVoice/CmuxMobileSupport that both the composer dictation backend and Voice Mode's VoicePermissionRequester call into.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/VoicePermissionRequester.swift`
around lines 1 - 43, The permission request flow in VoicePermissionRequester
duplicates the mic/Speech authorization logic already handled by
AppleComposerDictationRecognitionBackend. Extract the shared
permission-negotiation behavior into one helper (for example in CmuxVoice or
CmuxMobileSupport) and have both
VoicePermissionRequester.requestPermissions(for:) and the backend’s
resolvedAuthorization()/requestAuthorization() paths call that shared API. Keep
the iOS version-specific microphone handling and any TCC workaround in the
shared helper so both callers stay in sync.
…cle, sanitized error copy Codex structured review + PR bot findings on the voice feature: - ParakeetTranscriptionSession: bound the mic-tap AsyncStream (bufferingNewest 1024) so a stalled/slow ASR consumer can no longer grow memory unboundedly while the user talks (P1). finish() now cancels and awaits the startup task so an early stop during CoreML load cannot resurrect the ASR session or race the next mic session, and startup/finalize failures surface localized cmux copy instead of raw error strings (detail goes to OSLog). - AppleVoiceTranscriptionSession: a nil SFSpeechRecognizer now fails the stream up front instead of leaving Voice Mode listening forever with no transcript; recognition errors show localized copy. - ParakeetComposerDictationRecognitionBackend: emit .finished when the update stream ends without a final result so a graceful stop settles immediately instead of waiting for the finalize watchdog. - VoiceModeView: generation token + hard-cancel of the previous session on restart, so quick stop-then-start cannot leave two transcription sessions/taps live, and a stale engine-ready callback cannot flip isListening back on after a stop. - ParakeetModelStore: download failures show localized copy; raw error detail moves to OSLog. Test updated to assert sanitized message. - MobileFocusObserver: drop the unused #if DEBUG summaryHashForTesting seam (tests read MobileFocusSnapshotPayload internals directly). - ComposerDictationController.backendFactory: document why the composition-root seam is static (consumers are constructed in UIKit-hosted and non-CmuxVoice packages with no DI path today). - Localizable.xcstrings (iOS): en+ja entries for the five new user-facing voice error strings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8351ea60d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| private func emitIfNeeded(force: Bool) { | ||
| guard let tabManager else { return } | ||
| guard tabManager === TerminalController.shared.activeTabManagerForCallerNotification() else { return } |
There was a problem hiding this comment.
When the active TabManager changes to nil (for example activateMainWindowContext(nil) calls setActiveTabManager(nil)), the active-manager notification wakes every MobileFocusObserver, but this guard makes each observer return before emitting anything. A phone already in Voice Mode then keeps showing the last terminal target instead of switching to “No terminal focused”, so the user can continue recording against a stale target until the final send fails. Emit a null focus snapshot on the nil-active transition rather than dropping the update.
Useful? React with 👍 / 👎.
| if !Self.backendFactory().isSupported { | ||
| state = .unavailable | ||
| } |
There was a problem hiding this comment.
When this controller is constructed before Parakeet is installed/selected on a locale where SFSpeechRecognizer() is nil, this permanently sets the state to .unavailable. The new backendFactory is dynamic, but start is still blocked by state.canStart, so after the user downloads and selects Parakeet the existing composer mic remains disabled until the composer view is recreated. Re-check the current backend before keeping dictation unavailable instead of treating the initial Apple recognizer result as terminal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
…arget re-check - ParakeetTranscriptionSession: gate audio forwarding on startup completion so pre-load buffers wait in this session's BOUNDED stream instead of moving the unbounded backlog into FluidAudio's internal input stream. - VoiceModeView: re-check the Voice Mode target after the permission prompt (focus/host/capabilities can change while it is up); keep the mic button tappable during isStarting so the coded cancel path is reachable when a permission prompt or engine spin-up hangs; sanitize send errors (Mac-authored RPC/auth messages pass through, transport errors map to localized copy); use String.localizedStringWithFormat for the send confirmation template. - MobileVoiceSettingsPage: only flip the engine back to Apple when model deletion actually succeeded, so selection cannot disagree with the installed model. - support page: use buildAlternates(locale, "/support") so localized pages do not self-canonicalize to the English URL. - Localizable.xcstrings (iOS): en+ja for mobile.voiceMode.sendFailed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Main merged growth without a budget bump; PR CI runs on the merge commit and inherited the violation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…deletion - VoiceModeView: dismissing the screen (Done / onDisappear) now hard-cancels the transcription session and update task instead of a graceful stop, so a recognizer that never delivers a final result cannot keep the unstructured task and the ASR/CoreML session retained past the view lifetime. - ParakeetModelStore.deleteModel(): the ~480 MB compiled model tree is renamed aside synchronously (O(1) same-volume rename, so isInstalled flips immediately and failures still throw) and the byte removal runs on a detached utility task, so the @mainactor store no longer blocks the settings UI. Orphaned rename targets from a mid-delete crash are swept in the background at store init. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift (2)
61-61: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRedact upstream error details in production logs.
error.localizedDescriptioncan include local paths/CoreML/provider details; logging it as.publicmakes those details visible in unified logs. Keep the user-facing message generic and log the diagnostic as private.🛡️ Proposed fix
- parakeetSessionLog.error("Parakeet startup failed: \(error.localizedDescription, privacy: .public)") + parakeetSessionLog.error("Parakeet startup failed: \(error.localizedDescription, privacy: .private)") ... - parakeetSessionLog.error("Parakeet finalize failed: \(error.localizedDescription, privacy: .public)") + parakeetSessionLog.error("Parakeet finalize failed: \(error.localizedDescription, privacy: .private)")As per coding guidelines, production Swift logs should keep dynamic sensitive/internal values redacted or private.
Also applies to: 114-114
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift` at line 61, The Parakeet startup error logging in ParakeetTranscriptionSession should not expose upstream details as public data. Update the parakeetSessionLog.error call in the startup failure handling to keep the user-facing message generic and log error.localizedDescription as private instead of public, and apply the same redaction approach to the other matching log site referenced in the review.Source: Coding guidelines
200-205: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winCopy the tap buffer before enqueueing. Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift:80-81, 201-205 —
AudioBufferBoxsuppresses Sendable checks while carrying a mutableAVAudioPCMBufferreference across an async boundary. Copy the PCM data into owned storage first, or document a concrete guarantee that the engine never reuses or mutates that buffer afteryield.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift` around lines 200 - 205, The AudioBufferBox path is passing a mutable AVAudioPCMBuffer across an async boundary via unchecked Sendable, which should be fixed in ParakeetTranscriptionSession. Update the audio enqueue flow in the transcription session to copy the tap buffer’s PCM data into owned storage before yielding, or otherwise make the ownership guarantee explicit and verifiable so the buffer cannot be reused or mutated after handoff. Focus on the tap-thread ingestion and the AudioBufferBox wrapper used by ParakeetTranscriptionSession.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Packages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swift`:
- Line 61: The Parakeet startup error logging in ParakeetTranscriptionSession
should not expose upstream details as public data. Update the
parakeetSessionLog.error call in the startup failure handling to keep the
user-facing message generic and log error.localizedDescription as private
instead of public, and apply the same redaction approach to the other matching
log site referenced in the review.
- Around line 200-205: The AudioBufferBox path is passing a mutable
AVAudioPCMBuffer across an async boundary via unchecked Sendable, which should
be fixed in ParakeetTranscriptionSession. Update the audio enqueue flow in the
transcription session to copy the tap buffer’s PCM data into owned storage
before yielding, or otherwise make the ownership guarantee explicit and
verifiable so the buffer cannot be reused or mutated after handoff. Focus on the
tap-thread ingestion and the AudioBufferBox wrapper used by
ParakeetTranscriptionSession.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 751c456a-0bdc-4332-af18-6f13d33a6fed
📒 Files selected for processing (5)
Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileVoiceSettingsPage.swiftPackages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/VoiceModeView.swiftPackages/iOS/CmuxVoice/Sources/CmuxVoice/ParakeetTranscriptionSession.swiftios/cmux/Resources/Localizable.xcstringsweb/app/[locale]/(legal)/support/page.tsx
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05caff9d8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await self?.unsubscribeVoiceFocus(client: client, streamID: streamID) | ||
| } | ||
| } | ||
| await unsubscribeVoiceFocus(client: client, streamID: streamID) |
There was a problem hiding this comment.
When the focus stream ends because the underlying RPC session is torn down (for example a disconnect or Mac switch), this unconditional unsubscribe sends a new request on the old MobileCoreRPCClient; MobileCoreRPCSession.tearDown finishes listeners and clears the transport, and a later sendRequest goes through ensureConnected, so this cleanup can reconnect a stale Mac just to unsubscribe. Only send the unsubscribe while the current connection is still alive; the cancellation handler already covers view dismissal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 80aa4b3: unsubscribeVoiceFocus is now generation-guarded (isCurrentRemoteOperation) so it is skipped after a disconnect or Mac switch and can no longer redial a stale Mac through ensureConnected.
— Claude Code
| var body: some View { | ||
| Form { | ||
| Section { | ||
| Text(L10n.string("mobile.settings.privacy.voice", defaultValue: "Voice audio is transcribed on this iPhone.")) |
There was a problem hiding this comment.
When the selected engine is Apple on a locale/device where SFSpeechRecognizer.supportsOnDeviceRecognition is false, the Apple voice sessions do not set requiresOnDeviceRecognition, so Speech may use server recognition. This privacy page promises all voice audio is transcribed on the iPhone, which is inaccurate for that supported path; either require on-device recognition or soften the copy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Already fixed in 8dc0167 (before this review posted): the privacy row now reads 'Parakeet transcription always runs on this iPhone. The Apple engine prefers on-device recognition; when your language does not support it, Apple's servers may process the audio.' (en+ja).
— Claude Code
…t history The nonisolated(unsafe) FileManager bridges compile locally but the fleet builder's toolchain still rejects the sending-closure captures; the detached delete/sweep now use FileManager.default inside the closure (tests only ever inject .default). Privacy page no longer claims unconditional on-device transcription: Apple's engine can use Apple servers when the locale lacks on-device support (Parakeet is always local). Voice Mode keeps at most 50 on-screen utterances. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dc0167f3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| continue | ||
| } | ||
| let generation = connectionGeneration | ||
| _ = await fetchVoiceFocus() |
There was a problem hiding this comment.
Because the initial mobile.focus.get runs before the mobile.events.subscribe request is installed on the Mac, a focus change that happens after the fetch response but before the subscription ack is dropped by the host and voiceFocusSnapshot stays stale until another focus change. In that window Voice Mode can keep the mic enabled for the old terminal, or show one target while the final send goes to a different/no terminal; subscribe before the first fetch or refetch immediately after the subscription is acknowledged.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 80aa4b3: the seed mobile.focus.get now runs after the mobile.events.subscribe ack (with a guarded unsubscribe on the immediate-supersede path), so a focus change during the subscribe round-trip is no longer lost.
— Claude Code
| } header: { | ||
| Text(L10n.string("mobile.settings.voice.parakeet", defaultValue: "Parakeet v3")) | ||
| } footer: { | ||
| Text(L10n.string("mobile.settings.voice.footer", defaultValue: "Voice transcription is processed on this iPhone. Apple uses the built-in recognizer; Parakeet runs from a downloaded CoreML model.")) |
There was a problem hiding this comment.
Fresh evidence: although the Privacy page was qualified, this Voice page footer still says all voice transcription is processed on the iPhone. When the selected Apple engine runs on a locale/device where SFSpeechRecognizer.supportsOnDeviceRecognition is false, the Apple backends do not set requiresOnDeviceRecognition, so Speech may use Apple's servers; users choosing an engine from this page still see an inaccurate privacy claim. Please match the qualified copy here too.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 80aa4b3: the Voice settings footer now carries the same qualified wording as the Privacy page (Parakeet always on-device; Apple prefers on-device and may use Apple's servers when the locale lacks support), en+ja.
— Claude Code
…cus emit Voice settings now renders a model-driven engine list where each row carries its own download state (Get 483 MB / progress + cancel / retry / installed + delete), so adding future models is one array entry; the separate Parakeet section is gone and the size reflects the verified HuggingFace tree (483 MB int8). Troubleshooting gains Run Diagnostics (account, paired/active computer, connection route, Voice Mode capability, notification and mic/speech permissions, model install state, Copy Report; pure tested report builder) and eight collapsible Common Issues entries with inline links into iOS Settings and diagnostics. The Mac now emits one empty focus.updated when the active TabManager goes away so Voice Mode cannot keep a stale target after the last window closes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
…oller budget Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50c5e2b4fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| utteranceHistory.markFailed( | ||
| id: id, | ||
| message: Self.sendErrorMessage(error), | ||
| isTargetChanged: Self.isTargetChanged(error) | ||
| ) |
There was a problem hiding this comment.
When sendVoiceInput throws because the Mac focus changed, no terminal is focused, the connection drops, or the terminal input queue rejects the text, this only records a .failed status in utteranceHistory; the visible transcript currently receives only utteranceHistory.utterances.map(\.text) and errorMessage is left unchanged, so the user sees the spoken text as if it was sent successfully. Please render the failed status or set the send error message here so Voice Mode has an explicit failure state.
AGENTS.md reference: AGENTS.md:L149-L149
Useful? React with 👍 / 👎.
# Conflicts: # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/MobileSettingsView.swift
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
# Conflicts: # Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift # Packages/iOS/CmuxMobileShellUI/Sources/CmuxMobileShellUI/WorkspaceListView.swift # Resources/Localizable.xcstrings # cmux.xcodeproj/project.pbxproj # ios/cmux/Resources/Localizable.xcstrings
|
Too many files changed for review. ( Bypass the limit by tagging |
lawrencecchen
changed the title
# Conflicts: # Packages/iOS/CmuxMobileShell/Sources/CmuxMobileShell/MobileShellComposite.swift # cmux.xcodeproj/project.pbxproj # ios/cmux.xcworkspace/xcshareddata/swiftpm/Package.resolved # ios/cmux/AppCompositionRoot.swift # ios/cmux/cmuxApp.swift # ios/cmuxPackage/Package.resolved # ios/cmuxPackage/Sources/cmuxFeature/CMUXMobileRootScene.swift # web/app/env.ts
Bugbot is paused — on-demand spend limit reachedBugbot uses usage-based billing for this team and has hit its on-demand spend limit. A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue. |
lawrencecchen
added
the
stale-revisit
3 participants
Three features in one branch, per the voiced-feature spec.
Nested settings. The iOS settings sheet is now a top-level list pushing dedicated pages: Terminal (shortcuts + workspace-list display), Browser (search engine picker, threaded into
BrowserURLResolver), Voice, Notifications, About, Privacy, Troubleshooting. About links to https://cmux.com/privacy-policy, https://cmux.com/terms-of-service, and the new https://cmux.com/support page (added underweb/app/[locale]/(legal)/supportwith en+ja messages and a footer link), plus acknowledgements for FluidAudio (Apache-2.0) and Parakeet TDT 0.6B v3 (CC-BY-4.0).Voice engines. New
Packages/iOS/CmuxVoicepackage. Apple speech recognition remains the zero-download default; Settings > Voice can download NVIDIA Parakeet v3 (FluidAudio pinnedexact: 0.15.4, ~480 MB CoreML int8, on-device, 25 languages, auto language detection) with progress, cancel, delete, and backup exclusion.ComposerDictationControllergained a recognition-backend seam so the engine choice also drives composer dictation; the Apple backend preserves the prior behavior and iOS 26 TCC-callback crash workarounds verbatim, and the Parakeet path requests only microphone permission.Voice Mode. Mic icon on the workspace list opens a full-screen mode where the iPhone is a microphone for the connected Mac. A new Mac-side
MobileFocusObserveremitsfocus.updatedevents (payload inline, 50 ms throttle, hash-diffed, gated to the active window's TabManager) whenever the mouse selects a different pane or workspace, so the phone's target card updates in real time. Finalized utterances insert into the focused terminal via the newmobile.voice.inputRPC (no_focused_terminalwhen a browser or nothing is focused), with an auto-submit toggle;mobile.focus.getseeds the initial state. Capabilitiesfocus.events.v1/voice.input.v1; scoped attach tickets are denied both methods. Phone-side focus subscriptions are connection-generation aware: they resubscribe after reconnect/Mac switch and sendmobile.events.unsubscribeon exit.Parakeet streaming details that matter for review: FluidAudio's
isConfirmedchunk flag is NOT utterance finality; the session accumulates confirmed+volatile text, emits cumulative partials, and emits exactly one final fromfinish()(prevents double-typing into the terminal). Audio buffers flow through a single ordered consumer (no per-buffer Tasks), andfinish()/cancel()tear down the SDK actor so loaded CoreML models are released per mic session.Tests: CmuxVoice download state machine (incl.
URLError(.cancelled)→ idle and superseded-attempt guard), browser search-engine resolver behavior, Mac-side focus snapshot hashing andmobile.voice.inputrouting (focused terminal receives text; non-terminal focus errs). All new user-facing strings have en+ja entries.Plan/code/judge loop: planned and orchestrated by Fable, implemented by GPT 5.5 (xhigh) via codex exec in an isolated worktree, adversarially reviewed by a read-only Fable judge (round 1: 6 confirmed issues, all fixed and re-verified in round 2).
🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
High Risk
Introduces remote text insertion into Mac terminals with focus-mismatch checks and a large new voice/RPC surface plus a third-party ML dependency; mis-routing or capability skew between phone and Mac would be user-visible.
Overview
iOS settings are reorganized into a compact root list with navigation to dedicated pages (Terminal, Browser, Voice, Notifications, About, Privacy, Troubleshooting), including diagnostics, pairing help, and a full-screen Voice Mode entry when the Mac advertises the new capabilities.
Mobile browser gains persisted default search engine settings (DuckDuckGo / Google / Bing) and wires address-bar submission through
BrowserURLResolverwith the selected template.Voice stack: new
CmuxVoice(FluidAudio) powers downloadable on-device Parakeet models, vocabulary/boost settings, andComposerDictationControlleris refactored behind a recognition backend seam (Apple backend preserved; factory can swap to Parakeet).MobileShellCompositeadds capability gating,mobile.focus.get,focus.updatedsubscription (generation-aware reconnect), andmobile.voice.inputwith expected workspace/surface IDs for fail-closed targeting.VoiceModeViewstreams Mac focus (including pane layout preview), transcribes via Apple or Parakeet, and sends typed or spoken utterances with optional auto-submit and resend history.RPC models (
MobileFocusSnapshot,MobileVoiceInputResponse) and unit tests cover layout decoding, browser settings, and utterance history.Reviewed by Cursor Bugbot for commit 1e5814a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds iOS Voice Mode with two paths: dictation into the focused Mac terminal and a GPT Realtime mode that can target terminals across paired Macs. Also ships downloadable on‑device Parakeet models, vocabulary boosting, a default browser search picker, nested settings with diagnostics, and removes a stale mobile titlebar source reference.
New Features
focus.updated(includes a pane‑layout wireframe and emits an empty snapshot when no window is active), fail‑closedmobile.voice.inputtargeting using expected workspace/surface IDs, auto‑submit, a resendable utterance history (cap 50), a bottom type‑or‑dictate field, and watchdog‑protected sessions that hard‑cancel on dismiss. Dictation recovers when a supported engine becomes available.mobile.voice.inputand is gated byvoice.targets.v1.CmuxVoicewith Apple (default) and NVIDIA Parakeet viaFluidAudio0.15.4— v3 int8 (~483 MB), v3 Compact int4 (~336 MB), v2 (~464 MB). Per‑engine rows support download/resume/cancel/retry/delete with real byte progress and off‑main deletion.VoiceSettingsStorepersists engine, auto‑submit, and a GPT Voice toggle; custom vocabulary biases both engines with an optional CTC add‑on (~103 MB) for Parakeet; Voice Mode can auto‑bias visible screen terms.BrowserURLResolverviaMobileBrowserSettings. Troubleshooting adds Run Diagnostics with Copy Report and common‑issues help. Adds a site Support page and footer link.MobileFocusSnapshotandMobileVoiceInputResponse; unit tests cover layout decoding, browser settings, voice routing/tool execution, Parakeet downloads, and session/runtime behavior.Migration
focus.events.v1andvoice.input.v1; GPT mode and multi‑Mac targeting are gated byvoice.targets.v1. Scoped attach tickets are denied formobile.focus.getandmobile.voice.input.Written for commit 047f193. Summary will update on new commits.
Summary by CodeRabbit