Skip to content

iOS composer: on-device voice dictation - #6197

Merged
lawrencecchen merged 11 commits into
mainfrom
feat-ios-mic-dictation
Jun 16, 2026
Merged

lawrencecchen merged 11 commits into
mainfrom
feat-ios-mic-dictation

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

What

Adds a microphone button to the iOS composer that does on-device voice dictation. Tap the mic, speak, and the transcription fills the composer text field live. Tap again (or tap send, or leave the field) to stop. The text is then sent as a normal message. This dictates INTO the textbox; it does not send audio.

How to use

A mic button (MobileComposerMic) sits beside the paperclip attach button. Tap to start; it turns into a pulsing red mic.fill while listening. As you speak, partial transcriptions stream into the field, appended after whatever you already typed. Tap the mic again, tap send, move focus off the field, switch terminals, or leave the composer to stop.

Permissions

  • NSMicrophoneUsageDescription and NSSpeechRecognitionUsageDescription added to ios/Config/Info.plist.
  • Both keys added to ios/cmux/Resources/InfoPlist.xcstrings with English and Japanese values (minimal textual insertion, file stays valid JSON).
  • Authorization: SFSpeechRecognizer.requestAuthorization plus microphone permission (AVAudioApplication.requestRecordPermission on iOS 17+, AVAudioSession.requestRecordPermission fallback). Denied/restricted/unsupported lands in a terminal unavailable state that disables the button. No crash.

On-device vs server recognition

Prefers on-device recognition for privacy and offline use: sets requiresOnDeviceRecognition = true when supportsOnDeviceRecognition, falling back to server recognition only when the device cannot recognize locally.

Design

ComposerDictationController is an @MainActor ObservableObject wrapping SFSpeechRecognizer + SFSpeechAudioBufferRecognitionRequest driven by an AVAudioEngine tap. State machine: idle -> requestingPermission -> listening -> stopping -> idle, with a terminal unavailable. The SwiftUI view stays thin (a @StateObject and a toggle).

Text merge: on start the controller captures the composer's current text as the base. Every partial replaces the live tail, so the field always reads base + transcript and the pre-typed text is never clobbered. A single separating space is inserted between a non-whitespace base and the transcript; existing trailing whitespace is preserved (no doubling). Factored into a pure ComposerDictationTextMerge for host testing.

Lifecycle / teardown guarantees

stop() is idempotent and tears down fully: cancels the recognition task, ends and drops the request, removes the audio tap (removeTap(onBus:)), stops the engine, deactivates the audio session, and clears the callback. It is called from: a second mic tap, send, field focus loss, the view's .onDisappear, and .onChange(of: terminalID). A failed start (no input route, session error, recognizer offline) tears down and disables the mic rather than leaving it hot.

Swift 6 concurrency

The recognition result handler extracts only Sendable value snapshots (the transcript String and isFinal/error Bools) before hopping to the main actor, so no non-Sendable reference (SFSpeechRecognitionResult, Error) crosses the actor boundary. The result closure captures self weakly (no retain cycle through the task). The realtime audio-tap closure only appends buffers to the request and touches no main-actor state.

Tests

ComposerDictationTests covers the text-merge (empty base, separating space, trailing-whitespace preservation, leading-transcript trim, empty/growing partials, verbatim base) and the state machine's pure canStart/isListening transitions. The Speech/AVFoundation engine wiring is iOS-only and exercised by the simulator build, not host-compilable here (the package depends on the iOS-only GhosttyKit binary).

🤖 Generated with Claude Code


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Note

Medium Risk
Touches microphone/speech permissions and real-time audio session teardown; send uses hard-cancel to avoid late recognition callbacks mutating the draft after submit.

Overview
Adds on-device voice dictation to the iOS message composer: a mic control beside attach streams live transcription into the draft (text only, not audio).

New ComposerDictationController (Speech + AVAudioEngine) and host-testable ComposerDictationState / ComposerDictationTextMerge handle permissions, prefer on-device recognition, merge partials as base + transcript without clobbering existing text, and distinguish graceful stop (finalize + 2.5s watchdog) from hard cancel (send, disappear, terminal switch).

TerminalComposerView wires the mic UI, disables the field while dictation owns the text, ignores focus-loss from that lock, and tears down dictation on lifecycle events. iOS adds microphone and speech recognition usage strings (EN/JA) plus localized mic accessibility labels; ComposerDictationTests covers merge and state rules.

Reviewed by Cursor Bugbot for commit 5674550. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Adds on-device voice dictation to the iOS composer. Tap the mic to speak and see live transcription; send hard‑cancels to prevent late results from changing what gets sent.

  • New Features

    • Mic button MobileComposerMic beside attach; pulsing red mic.fill while listening.
    • Streams partials via base+transcript merge; locks the field while listening/stopping to avoid clobbering edits.
    • Start/stop: second tap cancels pending permission; graceful stop on mic tap and focus loss; hard-cancel on send, terminal switch, and composer disappear.
    • Prefers on-device recognition with server fallback; @MainActor @Observable controller with clean teardown and an “unavailable” state; adds NSMicrophoneUsageDescription and NSSpeechRecognitionUsageDescription (EN/JA) and localized “Start dictation”/“Stop dictation”; tests cover merge/state/lock.
  • Bug Fixes

    • Split graceful stop vs hard cancel with a 2.5s watchdog; ignore lock-driven focus loss so dictation doesn’t auto-stop after starting.
    • Fixed AVAudioSession start: use .record + .measurement without invalid options to prevent start failures.

Written for commit 5674550. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • New Features
    • Added iOS-only voice dictation for the message composer, including on-device transcription when available.
    • Added a microphone button to start/stop dictation with pulsing “listening” feedback, plus an unavailable/disabled state.
    • Automatically merges partial and final transcripts into the draft with consistent spacing.
  • Behavior & UX
    • Dictation stops gracefully when finalizing, and is hard-cancelled when the composer view changes, loses focus, or before sending.
  • Permissions & Localization
    • Added microphone and speech-recognition permission prompts and localized “Start dictation” / “Stop dictation” labels.
  • Tests
    • Added host-test coverage for dictation state transitions and transcript/base merge rules (including graceful vs hard cancel).

cmux-lawrence and others added 4 commits June 15, 2026 16:32
On-device speech-to-text for the composer, encapsulated in an @mainactor
ObservableObject with a state machine (idle, requestingPermission, listening,
stopping, unavailable). Prefers on-device recognition, falls back to server.
The pure text-merge (base + transcript) is split into a host-testable type.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a mic button (MobileComposerMic) beside the attach button that toggles
dictation, shows a red mic.fill while listening, and is disabled when the
recognizer is unavailable. Stops dictation on send, focus loss, onDisappear,
and terminal switch so the mic never stays hot. Refreshes the file-length
budget for the touched composer file.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
NSSpeechRecognitionUsageDescription and NSMicrophoneUsageDescription in
Info.plist, plus English and Japanese values in InfoPlist.xcstrings.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers the base + transcript merge (spacing, trailing whitespace, empty
partials, growing partials) and the state machine's pure canStart/isListening
transitions. The Speech/AVFoundation engine wiring is iOS-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 16, 2026 2:45am
cmux-staging Building Building Preview, Comment Jun 16, 2026 2:45am

@coderabbitai

coderabbitai Bot commented Jun 15, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds on-device voice dictation to the iOS terminal composer. A new ComposerDictationController drives SFSpeechRecognizer and AVAudioEngine, exposes a published five-state lifecycle, and merges streaming transcripts with existing composer text via a pure ComposerDictationTextMerge helper. TerminalComposerView owns the controller via @State, wires lifecycle stop events, and adds a mic toggle button. iOS permission keys are added to Info.plist and localized in English and Japanese.

Changes

Voice Dictation Feature

Layer / File(s) Summary
Dictation state machine and text-merge contracts
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift, Packages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/ComposerDictationTests.swift
ComposerDictationState defines five lifecycle states with isListening, canStart, canCancelPendingStart, canFinalize, and isStopping computed properties. ComposerDictationTextMerge.merged applies whitespace trimming and join rules to merge a stable base with a streaming partial transcript. Tests cover all merge and state-eligibility cases, including graceful stop vs hard cancel pathways.
ComposerDictationController: permissions, recognition, teardown
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift
@MainActor final @Observable controller storing speech recognizer, audio engine, recognition task/request, merge base, and latched callback. Implements toggle/start/stop/cancel, permission helpers for SFSpeechRecognizer and AVAudioSession (iOS 17+ and pre-17), audio session setup with input tap, streaming transcript merge via ComposerDictationTextMerge, and teardown/failStart/finishGraceful paths.
TerminalComposerView dictation wiring
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
Adds @State dictation controller; stops dictation on view disappear, terminalID change, and focus loss; inserts mic button into the composer toolbar with pulsing-while-listening and disabled-when-unavailable states; adds toggleDictation() to capture the existing text as a merge base and write results back to the store; calls dictation.cancel() before send.
iOS permission declarations
ios/Config/Info.plist, ios/cmux/Resources/InfoPlist.xcstrings, ios/cmux/Resources/Localizable.xcstrings
Adds NSMicrophoneUsageDescription and NSSpeechRecognitionUsageDescription to Info.plist with English and Japanese localizations in InfoPlist.xcstrings; adds mobile.composer.mic.start and mobile.composer.mic.stop UI control labels (English and Japanese) to Localizable.xcstrings.

Sequence Diagram

sequenceDiagram
    actor User
    participant TerminalComposerView
    participant ComposerDictationController
    participant SFSpeechRecognizer
    participant AVAudioEngine

    User->>TerminalComposerView: tap mic button
    TerminalComposerView->>ComposerDictationController: toggle(existingText:onText:)
    ComposerDictationController->>SFSpeechRecognizer: requestAuthorization()
    ComposerDictationController->>AVAudioEngine: requestMicrophonePermission()
    SFSpeechRecognizer-->>ComposerDictationController: authorized
    AVAudioEngine-->>ComposerDictationController: granted
    ComposerDictationController->>AVAudioEngine: prepare + start + installTap
    ComposerDictationController->>SFSpeechRecognizer: recognitionTask(with:resultHandler:)
    loop Streaming partials
        AVAudioEngine->>SFSpeechRecognizer: audio buffer
        SFSpeechRecognizer-->>ComposerDictationController: partial transcript
        ComposerDictationController->>ComposerDictationController: TextMerge.merged(base:transcript:)
        ComposerDictationController->>TerminalComposerView: onText(mergedText)
        TerminalComposerView->>TerminalComposerView: store.terminalInputText = mergedText
    end
    SFSpeechRecognizer-->>ComposerDictationController: isFinal=true
    ComposerDictationController->>ComposerDictationController: teardown (cancel task, stop engine, clear callbacks)
    ComposerDictationController->>TerminalComposerView: state → .idle
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • manaflow-ai/cmux#6102: Main PR's dictation feature calls dictation.cancel() before message submission in the send() workflow, which shares the same composer submission pathway as the retrieved PR that restructures image attachment staging.

Poem

🐇 Hop, hop, I speak and it types for me,
A mic button blooms where silence used to be.
The engine starts, transcripts stream in a flow,
merged(base:transcript:) tidies the show.
On final result the engine goes still —
My voice fills the composer, what a thrill! 🎙️


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
Cmux Swift Actor Isolation ❌ Error ComposerDictationState and ComposerDictationTextMerge are pure value-only enums lacking explicit nonisolated markers in a Swift 6 strict-concurrency package, creating unnecessary MainActor coupling. Mark both enums with nonisolated: nonisolated enum ComposerDictationState and nonisolated enum ComposerDictationTextMerge. These pure helpers should not be MainActor-bound.
Cmux Swift Blocking Runtime ❌ Error PR introduces Task.sleep in production code (ComposerDictationController.swift) for a 2.5-second watchdog timeout, violating swift-blocking-runtime.md rule against timing-based sync in non-test code. Replace Task.sleep watchdog with a proper timer abstraction, async sequence with timeout, or explicit task completion callback to avoid blocking synchronization in production.
Docstring Coverage ⚠️ Warning Docstring coverage is 55.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (18 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main feature addition: on-device voice dictation for the iOS composer, accurately reflecting the primary change in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Expensive Synchronous Load ✅ Passed PR adds voice dictation with no expensive synchronous loaders on main actor or interactive paths. ComposerDictationController.init() only creates lightweight SFSpeechRecognizer/AVAudioEngine withou...
Cmux Cache Substitution Correctness ✅ Passed baseText is transient and private to the controller, used only for merge math during active dictation; all partial results write directly back to authoritative store.terminalInputText, not persiste...
Cmux No Hacky Sleeps ✅ Passed PR modifies only Swift code, plist configuration, and localization files. Check scope is TypeScript/JavaScript/shell/build-runtime scripts only; Swift timing is covered separately by swift-blocking...
Cmux Algorithmic Complexity ✅ Passed New dictation code avoids all algorithmic complexity violations: no nested collection scans, no batch rescans, no filtering/sorting in hot paths, no problematic in-memory joins. Text merge is O(n)...
Cmux Swift Concurrency ✅ Passed ComposerDictationController uses @Observable macro instead of Combine, has no Dispatch patterns, and all Task usage is compliant: watchdog timer is stored with managed lifecycle, Task hops to @Main...
Cmux Swift @Concurrent ✅ Passed All async patterns in the new ComposerDictationController correctly hop to @MainActor, extract Sendable snapshots before crossing actor boundaries, and avoid heavy work in detached tasks. No missin...
Cmux Swift File And Package Boundaries ✅ Passed ComposerDictationController.swift (381 lines) stays under 400-line new-file threshold with cohesive speech-dictation responsibility; ComposerDictationTextMerge (90 lines) is pure logic factored for...
Cmux Swift Logging ✅ Passed All new/modified Swift files in this PR contain no print, debugPrint, dump, or NSLog statements; no file-scoped Logger declarations; and no exposure of sensitive data. Test files properly a...
Cmux User-Facing Error Privacy ✅ Passed All user-facing text (permission strings, button labels, accessibility text) uses generic, product-appropriate language. Error handling gracefully disables controls without exposing upstream vendor...
Cmux Full Internationalization ✅ Passed All user-facing Swift text uses L10n.string() with localization keys and defaultValues; all new xcstrings entries include complete translations for all supported locales (en, ja).
Cmux Swiftui State Layout ✅ Passed ComposerDictationController correctly uses @Observable with Observation import; TerminalComposerView holds it with @State, not legacy @StateObject. No @Published, ObservableObject, GeometryReader l...
Cmux Architecture Rethink ✅ Passed PR adds voice dictation with clear single-owner architecture: ComposerDictationController (@MainActor @Observable) owned by TerminalComposerView (@State), clean state machine (ComposerDictationStat...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PR adds iOS voice dictation as embedded view controls (mic button, callbacks) in TerminalComposerView; no NSWindow, NSPanel, NSWindowController, SwiftUI Window/WindowGroup, or standalone auxiliary...
Cmux Source Artifacts ✅ Passed All 7 changed files are intentional: Swift source (ComposerDictationController, ComposerDictationTextMerge, TerminalComposerView), Swift tests (ComposerDictationTests), configuration (Info.plist),...
Description check ✅ Passed The pull request description comprehensively covers all required sections: a clear What/How with user-facing feature overview, permission details, design rationale, lifecycle guarantees, concurrency safety, and testing coverage.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat-ios-mic-dictation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Jun 15, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

Adds on-device voice dictation to the iOS composer via a new ComposerDictationController (@MainActor @Observable) that drives an AVAudioEngine + SFSpeechRecognizer state machine, streaming partial transcriptions into the composer draft through a pure ComposerDictationTextMerge utility. Permissions, localized strings (EN/JA), and host tests are included.

  • ComposerDictationController: idle → requestingPermission → listening → stopping → idle state machine with clean hard-cancel and graceful-stop paths; stop() arms a Task.sleep-based 2.5 s watchdog to force-finish if no final result arrives, which conflicts with the cmux blocking-runtime rule — SFSpeechRecognitionTask.finish() or a structured async timeout would be the conforming replacement.
  • TerminalComposerView grows to 825 lines (from 744), crossing the 800-line budget threshold; the controller logic is correctly extracted, but the remaining mic-button glue could be pulled into a separate MobileComposerMic struct to bring the view file back under budget.
  • Localizable.xcstrings entries for mobile.composer.mic.start/.stop are present but their English values ("Start dictation" / "Stop dictation") differ from the defaultValue strings in the Swift call-sites ("Dictate Message" / "Stop Dictation"), which should be aligned.

Confidence Score: 4/5

Safe to merge with one blocking-runtime concern to address: the 2.5 s Task.sleep watchdog in stop() should be replaced with a signal-driven timeout before landing.

The controller design is solid — @MainActor @Observable, correct actor hopping for Speech callbacks, idempotent teardown, and the hard-cancel-before-send guard all prevent the most dangerous dictation-after-send races. The one concrete rule violation is the Task.sleep watchdog in stop(): it leaves the field locked and the controller stuck in .stopping for up to 2.5 s if recognition stalls, and the cmux blocking-runtime rule does not allow Task.sleep in production code even in teardown paths. Everything else (localization, state machine, lifecycle hooks) is well-handled.

ComposerDictationController.swift — the stop() watchdog needs a signal-driven replacement before merge.

Important Files Changed

Filename Overview
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift New @MainActor @Observable dictation controller; state machine and teardown are well-structured, but stop() uses Task.sleep as a 2.5 s watchdog timeout for teardown synchronization, violating the cmux blocking-runtime rule.
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift Pure state enum and text-merge utility, correctly separated for host-testability; no issues.
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift Adds mic button, dictation lifecycle hooks, and field-lock; raises file to 825 lines (past the 800-line budget threshold), and the accessibility label defaultValues don't match the catalog strings.
Packages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/ComposerDictationTests.swift Comprehensive host-testable coverage of the text-merge rules and state machine transitions; no issues.
ios/cmux/Resources/Localizable.xcstrings Adds mobile.composer.mic.start and mobile.composer.mic.stop with EN/JA; catalog values ("Start dictation" / "Stop dictation") diverge from the defaultValue strings in the Swift call-sites.
.github/swift-file-length-budget.tsv Budget raised from 744 → 825 for TerminalComposerView.swift, crossing the 800-line threshold the rule tracks; the controller logic is extracted but the view glue itself is not.
ios/Config/Info.plist Adds NSMicrophoneUsageDescription and NSSpeechRecognitionUsageDescription keys; localized via InfoPlist.xcstrings.
ios/cmux/Resources/InfoPlist.xcstrings Adds EN and JA translations for both new Info.plist permission keys; complete.
ios/cmuxPackage/Package.resolved Bumps swift-asn1 from 1.7.0 → 1.7.1; routine patch-version update.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([idle]) -->|tap mic| B([requestingPermission])
    B -->|second tap| A
    B -->|denied / restricted| E([unavailable])
    B -->|granted| C([listening\naudio engine running])
    C -->|tap mic| D([stopping\nwatchdog armed])
    C -->|focus loss not lock-driven| D
    C -->|send / onDisappear / terminal switch| A
    C -->|isFinal or error callback| A
    D -->|isFinal callback| A
    D -->|watchdog fires 2.5 s| A
    D -->|hard cancel| A
    C -->|recognizer unavailable / channel=0 / engine error| E
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([idle]) -->|tap mic| B([requestingPermission])
    B -->|second tap| A
    B -->|denied / restricted| E([unavailable])
    B -->|granted| C([listening\naudio engine running])
    C -->|tap mic| D([stopping\nwatchdog armed])
    C -->|focus loss not lock-driven| D
    C -->|send / onDisappear / terminal switch| A
    C -->|isFinal or error callback| A
    D -->|isFinal callback| A
    D -->|watchdog fires 2.5 s| A
    D -->|hard cancel| A
    C -->|recognizer unavailable / channel=0 / engine error| E
Loading

Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

Comment on lines +356 to +363
/// tinted mic. Disabled when the recognizer is unavailable or permission was
/// denied so the user is never left tapping a dead control.
private var micButton: some View {
let listening = dictation.state.isListening
return Button {
toggleDictation()
} label: {
Image(systemName: listening ? "mic.fill" : "mic")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Missing Localizable.xcstrings entries for new mic L10n keys

mobile.composer.mic.start and mobile.composer.mic.stop are consumed via L10n.string but are absent from ios/cmux/Resources/Localizable.xcstrings. Every other mobile.composer.* key in that file has both en and ja entries. Without catalog entries, the accessibility label falls back to the hardcoded English defaultValue for Japanese users — the exact localization debt the cmux rule exists to prevent. Both keys need en+ja stringUnit blocks in Localizable.xcstrings.

Rule Used: Flag production user-facing text that is not fully... (source)

Comment on lines +155 to +165
}
}

// MARK: - Recognition

/// Configure the audio session, install the engine tap, and start the
/// recognition task. On any setup failure this tears down and lands in
/// `unavailable` so the mic does not appear hot after a failed start.
private func beginRecognition() {
guard let recognizer, recognizer.isAvailable else {
failStart()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Transient setup failures permanently disable the mic button

failStart() sets state to .unavailable, which the isAvailable doc describes as a terminal state for "permanently unavailable (unsupported locale, denied, or restricted)" scenarios. However, three paths that call failStart() are transient: recognizer.isAvailable == false (server recognition temporarily offline or on-device model not yet downloaded), format.channelCount == 0 (audio input route absent — can change when the user connects headphones), and audioEngine.start() throwing (transient system resource contention). After any of these, isAvailable returns false and there is no code path back from .unavailable to .idle, so the button stays greyed out until the user restarts the app. These three paths should return to .idle (not .unavailable) so the user can retry after the transient condition clears.

Comment on lines +66 to +67
/// On-device voice dictation for the field. Owned here so its lifecycle is
/// the composer's: it is torn down on send, focus loss, `onDisappear`, and a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 New ObservableObject/@StateObject where @Observable/@State is the required cmux shape

ComposerDictationController is a new cmux-owned ObservableObject with @Published state, stored with @StateObject. The cmux SwiftUI state rule flags exactly this pattern for new code: @Observable + @State (or value snapshots) is the required shape. Using ObservableObject causes the whole TerminalComposerView body to invalidate on every state transition — even state changes like .requestingPermission → .listening that affect only the mic button. Converting to @Observable would scope invalidation to the views that actually read the changed property.

The same pattern appears in ComposerDictationController.swift at the class declaration.

Rule Used: Flag SwiftUI changes that can cause stale state, b... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

…on, l10n

FINDING 1: ComposerDictationController used ObservableObject/@published without
importing Combine (file-scoped imports), failing iOS type-check. Migrate to the
@observable macro to match the codebase convention (import Observation, drop the
protocol and @published); hold it in TerminalComposerView with @State so SwiftUI
tracks the observed state automatically.

FINDING 2: a second tap during .requestingPermission fell through to start(),
which canStart rejected, so the pending permission callback later started the
mic anyway. Make .requestingPermission cancellable: toggle now aborts the
pending start (back to idle, callback dropped), and the permission callback
short-circuits unless still .requestingPermission for both the granted and
denied paths, so a cancel never starts the engine or clobbers idle. A later tap
starts normally. Modeled as canCancelPendingStart on the pure state enum.

FINDING 3: add mobile.composer.mic.start / mobile.composer.mic.stop to
Localizable.xcstrings with en + ja values.

Extend host-testable state-machine tests for the cancel transition. Bump the
TerminalComposerView file-length budget for two doc-comment lines.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Addressed the three autoreview findings (215050b):

F1 (compile failure): Migrated ComposerDictationController to the @Observable macro (matching the codebase convention) rather than adding import Combine: import Observation, dropped : ObservableObject and @Published. In TerminalComposerView the controller is now held with @State instead of @StateObject; SwiftUI tracks the state reads in micButton automatically.

F2 (cancel race): .requestingPermission is now cancellable. A second tap there calls cancelPendingStart() (drops the callback, returns to idle) instead of falling through to a start that canStart rejected. The permission callback now short-circuits unless still .requestingPermission for both the granted and denied paths, so a cancel during authorization never starts the engine and never overwrites idle with unavailable. A later tap starts normally. Modeled as canCancelPendingStart on the pure state enum.

F3 (localization): Added mobile.composer.mic.start / mobile.composer.mic.stop to Localizable.xcstrings with en ("Start/Stop dictation") and ja ("音声入力を開始/停止"). Minimal insertion in key order; JSON validated.

Tests: extended the host-testable state-machine tests for the cancel transition. The package cannot host-build here (iOS-only GhosttyKit binary dep), so the iOS compile + tests rely on CI ios-simulator. Budget guard green (bumped TerminalComposerView budget by 2 for the @State doc comment).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`:
- Around line 63-67: The mic button should allow canceling a pending permission
request when tapped a second time while in the `.requestingPermission` state,
consistent with the documented behavior in the isAvailable property. Currently,
the toggle logic (around lines 75-80) only calls stop() when the state is
`.listening`, but it should also call stop() when the state is
`.requestingPermission` so that a second tap properly cancels the in-flight
permission request instead of routing back to start() which no-ops at line 87.
Update the condition that determines when to call stop() to include both the
`.listening` and `.requestingPermission` states.
- Around line 163-166: The failStart() method unconditionally sets state to
.unavailable, which prevents any retry attempts. Only permanent failures
(unsupported locale and denied/restricted authorization) should set state to
.unavailable; transient failures should allow retry by returning to .idle.
Modify the callers of failStart() at lines 165 (recognizer unavailability in
beginRecognition), 183 (audio session error), 192 (missing input route), and 205
(engine startup failure) to set state = .idle instead of calling failStart(),
reserving the permanent .unavailable state only for permanent conditions like
unsupported locale and authorization denials. Alternatively, refactor
failStart() to accept a parameter indicating whether the failure is permanent or
transient, then conditionally set either .unavailable or .idle accordingly.
🪄 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: 5ae3e37c-12b3-4410-9718-7e2a5e86b1fd

📥 Commits

Reviewing files that changed from the base of the PR and between 50ae246 and a649499.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (6)
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
  • Packages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/ComposerDictationTests.swift
  • ios/Config/Info.plist
  • ios/cmux/Resources/InfoPlist.xcstrings

Comment on lines +163 to +166
private func beginRecognition() {
guard let recognizer, recognizer.isAvailable else {
failStart()
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 "failStart\\(|state = \\.unavailable|recognizer\\.isAvailable|channelCount > 0|setCategory\\(|audioEngine\\.start\\(" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 1906


🏁 Script executed:

#!/bin/bash
# Get the full state enum definition and key transitions
rg -n -B5 -A10 "enum.*State|case idle|case unavailable" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift | head -60

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Get context around lines 57-60, 87-90, and 99-102 to understand the permanent vs transient distinction
sed -n '50,105p' Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 2442


🏁 Script executed:

#!/bin/bash
# Check if there's any retry mechanism or state reset elsewhere in the file
rg -n "state = \\.idle|\.idle" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 163


🏁 Script executed:

#!/bin/bash
# Get the full State enum definition and canStart/isListening logic
rg -n -B3 -A15 "enum ComposerDictationState|var canStart|var isListening" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Get the stop() method and any other state resets to understand lifecycle
sed -n '115,130p' Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 784


🏁 Script executed:

#!/bin/bash
# Check what recognizer.isAvailable represents (line 164 guard)
rg -n "isAvailable" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 179


🏁 Script executed:

#!/bin/bash
# Find the ComposerDictationState enum definition
rg -n "enum ComposerDictationState" -A 10 Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Get the full file size and structure to understand it better
wc -l Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 151


🏁 Script executed:

#!/bin/bash
# Get the complete initialization and state enum near the top
sed -n '1,50p' Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 2562


🏁 Script executed:

#!/bin/bash
# Find where ComposerDictationState is defined
find Packages/CmuxMobileShellUI -name "*.swift" -exec grep -l "enum ComposerDictationState" {} \;

Repository: manaflow-ai/cmux

Length of output: 146


🏁 Script executed:

#!/bin/bash
# Check the end of the file to see if enum is defined there
tail -100 Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 4144


🏁 Script executed:

#!/antml:thinking
Let me also look for what the canStart property is, since that's referenced in line 87 `guard state.canStart else`
</thinking>
# Find canStart definition
rg -n "var canStart" Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift

Repository: manaflow-ai/cmux

Length of output: 129


🏁 Script executed:

#!/bin/bash
# Check the ComposerDictationState file
cat Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift

Repository: manaflow-ai/cmux

Length of output: 3463


🏁 Script executed:

#!/bin/bash
# Search for ComposerDictationState definition
fd ComposerDictationState Packages/CmuxMobileShellUI/Sources/

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

#!/bin/bash
# Find all Swift files in the directory
ls -la Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/

Repository: manaflow-ai/cmux

Length of output: 5804


Do not permanently disable dictation after transient startup failures.

failStart() at line 240 unconditionally sets state = .unavailable, but its callers include transient conditions: audio session setup errors (line 183), missing input route (line 192), engine startup failure (line 205), and recognizer unavailability (line 165). Once .unavailable, canStart returns false indefinitely, blocking any retry.

The state machine design already distinguishes permanent failures—stop() explicitly preserves .unavailable as terminal while returning to .idle for other cases. Permanent conditions (unsupported locale at line 59, denied/restricted authorization at line 101) should set .unavailable; transient setup failures should return to .idle to allow retry.

🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`
around lines 163 - 166, The failStart() method unconditionally sets state to
.unavailable, which prevents any retry attempts. Only permanent failures
(unsupported locale and denied/restricted authorization) should set state to
.unavailable; transient failures should allow retry by returning to .idle.
Modify the callers of failStart() at lines 165 (recognizer unavailability in
beginRecognition), 183 (audio session error), 192 (missing input route), and 205
(engine startup failure) to set state = .idle instead of calling failStart(),
reserving the permanent .unavailable state only for permanent conditions like
unsupported locale and authorization denials. Alternatively, refactor
failStart() to accept a parameter indicating whether the failure is permanent or
transient, then conditionally set either .unavailable or .idle accordingly.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift (1)

263-266: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Treat setup-time start failures as retryable, not terminal.

Line 265 unconditionally sets .unavailable, but callers include transient start failures from Line 187, Line 206, Line 215, and Line 228. That makes one temporary audio/session/route failure permanently disable dictation for this controller instance. Keep .unavailable only for permanent conditions (unsupported recognizer / denied permission), and return to .idle for transient start failures.

Suggested fix
-    /// Tear down after a setup failure and disable the mic. Distinct from a clean
-    /// stop because a failed start indicates the recognizer cannot be used right
-    /// now (no input route, session error, recognizer offline).
+    /// Tear down after a setup failure and return to idle so the user can retry.
+    /// Permanent unavailability (unsupported locale / denied permission) is set
+    /// explicitly at those decision points.
     private func failStart() {
         teardown()
-        state = .unavailable
+        state = .idle
     }
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`
around lines 263 - 266, The failStart() method unconditionally sets state to
.unavailable, but it is called for transient start failures that should be
retryable rather than permanently disabling dictation. Modify the failStart()
method to set state to .idle instead of .unavailable so that temporary audio,
session, or route failures do not permanently disable the controller. Keep
.unavailable only for permanent conditions like unsupported recognizer or denied
permission, which should be handled separately and not through the failStart()
method.
🤖 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.

Duplicate comments:
In
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`:
- Around line 263-266: The failStart() method unconditionally sets state to
.unavailable, but it is called for transient start failures that should be
retryable rather than permanently disabling dictation. Modify the failStart()
method to set state to .idle instead of .unavailable so that temporary audio,
session, or route failures do not permanently disable the controller. Keep
.unavailable only for permanent conditions like unsupported recognizer or denied
permission, which should be handled separately and not through the failStart()
method.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5be89681-b786-4e53-b42b-2b342161e766

📥 Commits

Reviewing files that changed from the base of the PR and between a649499 and 215050b.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (5)
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
  • Packages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/ComposerDictationTests.swift
  • ios/cmux/Resources/Localizable.xcstrings

…te-away

The shared stop path treated an intentional Stop/send the same as a cancel:
teardown() cancelled the SFSpeechRecognitionTask before endAudio() and cleared
onText, discarding buffered audio and the late FINAL result, so the last spoken
words could be lost and a stale partial submitted.

Split the two intents:
- stop() now finalizes gracefully (Stop button, pre-send, focus loss): move to
  .stopping, flush buffered audio via endAudio(), stop the engine + remove the
  tap + deactivate the session, but keep the task and onText alive so the final
  result refines the committed text, then finishGraceful() cleans up. A 2.5s
  watchdog force-finishes so the controller cannot hang in .stopping.
- cancel() keeps the immediate hard teardown for onDisappear and terminal switch,
  where losing the unrecognized tail is acceptable.

Send preserves text because every partial already wrote into terminalInputText,
so the latest spoken words are committed before submitComposer() reads them; the
async final result only refines text already sent.

Call sites: Stop button + send + focus loss -> stop(); onDisappear + terminal
switch -> cancel(). Cancellable .requestingPermission behavior preserved (a
graceful stop from a non-listening state falls back to cancel).

Add pure-state coverage (canFinalize, isStopping) and bump the length budget for
the touched composer view.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fixed the P2 (dropped final words) finding.

Split the shared stop path into two intents:

  • stop() is now a graceful finalize (Stop button, pre-send, focus loss): moves to .stopping, flushes buffered audio with endAudio(), stops the engine + removes the tap + deactivates the session, but keeps the recognition task and onText alive so the FINAL result can refine the committed text. finishGraceful() then cleans up on the final result or error. A 2.5s watchdog force-finishes so the controller cannot hang in .stopping.
  • cancel() keeps the immediate hard teardown for onDisappear and terminal switch, where losing the unrecognized tail is acceptable.

Send preserves the text because every partial already wrote into terminalInputText, so the latest words are committed before submitComposer() reads them; the async final result only refines text already sent.

Call sites: Stop/send/focus-loss -> stop(); onDisappear/terminal-switch -> cancel(). The cancellable .requestingPermission behavior is preserved (a graceful stop from a non-listening state falls back to cancel()).

Extended the pure-state tests (canFinalize, isStopping, graceful-vs-hard partition) and bumped the Swift length budget for the touched composer view. iOS-only Speech/AVFoundation paths are not host-compilable; CI ios-simulator is the gate.

Comment thread Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift Outdated
The send path called the async graceful stop() then immediately ran
submitComposer(), which snapshots terminalInputText synchronously before
any await. A late final speech result could land after the snapshot,
dropping the finalized tail from the sent message and writing it back as
a new draft in the just-cleared field.

Switch send to cancel(): it immediately tears down the recognition task
and drops onText, so the snapshot captures the current field text and no
late callback can fire. Every partial already wrote the latest spoken
words into terminalInputText, so nothing is lost. cancel() on an idle
controller is a no-op, leaving the no-dictation send unchanged. The
graceful stop() stays on the Stop button and focus-loss paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fixed P1: send now hard-cancels dictation instead of the graceful async stop.

send() called dictation.stop() (graceful, async) then immediately ran submitComposer(), which snapshots terminalInputText synchronously before any await. A late final speech result could land after that snapshot, so the sent message dropped the finalized tail and the tail got written back into the just-cleared field as a stray draft.

Now send() calls dictation.cancel(): it immediately tears down the recognition task and drops onText, so (a) the synchronous snapshot captures exactly the current field text, and (b) no late onText can fire after the field is cleared. Every partial already wrote the latest spoken words into terminalInputText, so nothing is lost. cancel() from an idle controller is a no-op, so a send without active dictation is unchanged; cancel() from .stopping also fully drops onText and the finalize watchdog via teardown().

Graceful stop() stays on the Stop Dictation button and field-focus-loss paths (non-submitting, so async refinement is harmless). cancel() remains on onDisappear and terminal switch.

Budget: bumped the TSV for TerminalComposerView (+8 lines, comment), guard green. Host tests not runnable locally (package depends on the GhosttyKit xcframework, no host binary artifact); the new state-machine cases are exercised on CI ios-simulator. The fix is a call-site wiring change not observable through the pure ComposerDictationState enum, so no new pure-state test applies.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift`:
- Around line 425-428: The TerminalComposerView file exceeds 800 lines and
violates the Swift size guardrail policy. Extract a cohesive slice of
functionality—such as the dictation wiring (including the dictation.stop() call
and related dictation management logic) or attachment staging—into a separate
type or file. This will reduce the file size below 800 lines while improving
separation of concerns and maintainability. Ensure all dictation-related
methods, properties, and state are moved together to maintain cohesion in the
new extracted type.
🪄 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: 316f4ec2-ef73-4a66-bd3e-225d76560b95

📥 Commits

Reviewing files that changed from the base of the PR and between 215050b and 0051b2a.

⛔ Files ignored due to path filters (1)
  • .github/swift-file-length-budget.tsv is excluded by !**/*.tsv
📒 Files selected for processing (4)
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationTextMerge.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift
  • Packages/CmuxMobileShellUI/Tests/CmuxMobileShellUITests/ComposerDictationTests.swift

Comment on lines +425 to +428
// Stop dictation gracefully before sending. Every partial already wrote
// into `terminalInputText`, so the latest spoken words are committed and
// read by `submitComposer()` below.
dictation.stop()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Split TerminalComposerView to satisfy the production Swift size guardrail.

This file is already over 800 lines and continues to absorb behavior. Please extract a cohesive slice (for example, dictation wiring or attachment staging) into separate types/files so this view drops below the policy threshold and remains reviewable.

As per coding guidelines, “Flag Swift production files that exceed 400 lines without a clear single responsibility, or exceed 800 lines even with mostly coherent responsibility.”

🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift`
around lines 425 - 428, The TerminalComposerView file exceeds 800 lines and
violates the Swift size guardrail policy. Extract a cohesive slice of
functionality—such as the dictation wiring (including the dictation.stop() call
and related dictation management logic) or attachment staging—into a separate
type or file. This will reduce the file size below 800 lines while improving
separation of concerns and maintainability. Ensure all dictation-related
methods, properties, and state are moved together to maintain cohesion in the
new extracted type.

Source: Coding guidelines

cmux-lawrence and others added 2 commits June 15, 2026 17:07
`.duckOthers` is not a valid option for the `.record` category (only
Ambient/PlayAndRecord/Playback/MultiRoute), and
`.notifyOthersOnDeactivation` is only valid on deactivation. On OSes that
enforce these documented restrictions, setCategory/setActive threw, the
start path hit failStart(), and the mic was permanently disabled after
the first tap.

Use `.record` with `.measurement` mode and no options, and activate with
a plain setActive(true). The deactivation call keeps
`.notifyOthersOnDeactivation`, which is its correct use.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fixed P1 (invalid AVAudioSession options disabling dictation).

The start path set .record with .duckOthers and passed .notifyOthersOnDeactivation on setActive(true, ...). Per Apple's docs .duckOthers is invalid for .record and .notifyOthersOnDeactivation is only valid on deactivation. On OSes enforcing those restrictions, setCategory/setActive threw, the start hit failStart(), and the mic stayed permanently disabled after the first tap.

Now:

  • start: setCategory(.record, mode: .measurement) (no options) + setActive(true)
  • stop/teardown: deactivation keeps setActive(false, options: .notifyOthersOnDeactivation) (its correct use)

No other invalid category/option combos remain. State machine, graceful-stop vs hard-cancel split, @observable, cancellable requestingPermission, and send-path hard-cancel are unchanged. This is an AVFoundation-config change with no host-testable seam; the ios-simulator CI is the compile gate. Swift file-length budget green.

Commit 87184da.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (2)
Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift (1)

1-810: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

File exceeds 800-line threshold.

The file is 810 lines and continues to grow with new features. The past review correctly identified that a cohesive slice (e.g., dictation wiring or attachment staging) should be extracted.

🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift`
around lines 1 - 810, The TerminalComposerView file has grown to 810 lines and
exceeds the recommended threshold for maintainability. Extract the image
attachment handling logic into a separate file to reduce complexity and improve
modularity. Specifically, move the attachment-related functions and types:
`stagePickedItems`, `prepare`, `boundedSendPayload`, `downsampledImageData`, the
`PreparedAttachment` struct, the `ImportedImageFile` struct, the
`StagingTaskBox` class, the `AttachmentThumbnailCache` class, and the
`AttachmentChip` view into a new file (e.g., ComposerAttachmentView.swift or
similar). Keep the core `TerminalComposerView` and its main composition logic in
the original file, and import the extracted types where needed to maintain the
existing public interface and behavior.

Source: Coding guidelines

Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift (1)

326-329: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Transient startup failures should not permanently disable dictation.

failStart() unconditionally sets state = .unavailable, but several callers represent transient conditions: audio session setup errors (line 262), missing input route (line 271), engine startup failure (line 284), and recognizer temporarily unavailable (line 239). Once .unavailable, canStart returns false indefinitely with no retry path.

Permanent conditions (nil recognizer at init, denied/restricted authorization) correctly land in .unavailable. Transient setup failures should return to .idle so the user can retry.

Suggested approach
+    /// Tear down after a transient setup failure and allow retry.
+    private func failStartTransient() {
+        teardown()
+        state = .idle
+    }
+
     /// Tear down after a setup failure and disable the mic. Distinct from a clean
-    /// stop because a failed start indicates the recognizer cannot be used right
-    /// now (no input route, session error, recognizer offline).
+    /// stop because a failed start indicates the recognizer cannot be used
+    /// (unsupported locale, denied/restricted authorization).
     private func failStart() {
         teardown()
         state = .unavailable
     }

Then update callers:

  • Line 239 (recognizer.isAvailable): failStartTransient() — recognizer can become available again
  • Line 262 (session error): failStartTransient() — another app may release audio
  • Line 271 (no input route): failStartTransient() — headphones may reconnect
  • Line 284 (engine start): failStartTransient() — resource contention may resolve
🤖 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/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`
around lines 326 - 329, The failStart() method unconditionally sets state to
.unavailable, but several callers represent transient failures that should allow
retry. Create a new method failStartTransient() that calls teardown() and sets
state to .idle instead of .unavailable. Then update the four callers to use
failStartTransient() instead of failStart(): the recognizer.isAvailable check,
the audio session error handler, the missing input route detection, and the
engine startup failure handler. This allows transient conditions to be retried
while keeping permanent failures (nil recognizer, denied authorization) in the
.unavailable state via the original failStart() method.
🤖 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.

Duplicate comments:
In
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift`:
- Around line 326-329: The failStart() method unconditionally sets state to
.unavailable, but several callers represent transient failures that should allow
retry. Create a new method failStartTransient() that calls teardown() and sets
state to .idle instead of .unavailable. Then update the four callers to use
failStartTransient() instead of failStart(): the recognizer.isAvailable check,
the audio session error handler, the missing input route detection, and the
engine startup failure handler. This allows transient conditions to be retried
while keeping permanent failures (nil recognizer, denied authorization) in the
.unavailable state via the original failStart() method.

In
`@Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift`:
- Around line 1-810: The TerminalComposerView file has grown to 810 lines and
exceeds the recommended threshold for maintainability. Extract the image
attachment handling logic into a separate file to reduce complexity and improve
modularity. Specifically, move the attachment-related functions and types:
`stagePickedItems`, `prepare`, `boundedSendPayload`, `downsampledImageData`, the
`PreparedAttachment` struct, the `ImportedImageFile` struct, the
`StagingTaskBox` class, the `AttachmentThumbnailCache` class, and the
`AttachmentChip` view into a new file (e.g., ComposerAttachmentView.swift or
similar). Keep the core `TerminalComposerView` and its main composition logic in
the original file, and import the extracted types where needed to maintain the
existing public interface and behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e694f6d5-e63b-4517-9186-fda4859be1c6

📥 Commits

Reviewing files that changed from the base of the PR and between 1085ead and 87184da.

📒 Files selected for processing (2)
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/ComposerDictationController.swift
  • Packages/CmuxMobileShellUI/Sources/CmuxMobileShellUI/TerminalComposerView.swift

…not clobbered

Every recognition callback rewrote the composer text as base + transcript,
but the field stayed editable while listening/stopping, so any edit the
user made after dictation started (a correction, a typed suffix) was
silently discarded by the next partial or final callback.

Lock the field while dictation owns the text: add
ComposerDictationState.locksComposerField (.listening or .stopping),
surface it on the controller, and bind the TextField's .disabled to it.
The user cannot make an edit that would be clobbered; the field stays
visible showing the live transcript and re-enables once dictation settles
to idle. The mic toggle and send buttons sit outside the field and stay
live; send hard-cancels dictation back to idle, re-enabling editing and
refocusing.

Guard the focus-loss finalize: disabling a focused field makes SwiftUI
resign first responder, so skip the focus-loss stop() when the field is
locked, otherwise dictation would stop itself the instant it started.

Host-tested coverage for the lock predicate. Budget bumped for the
TerminalComposerView growth.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fixed the P2 (dictation overwrites user edits) in 7c589c4.

Approach: lock, not merge. While dictation owns the text (.listening or .stopping) the composer TextField is now .disabled, so the user cannot make an edit that a later partial/final callback would silently discard. The field stays visible showing the live transcript and re-enables once dictation settles to idle. This is simpler and more robust than tracking a dictation-owned range and reconciling it on every callback.

  • New ComposerDictationState.locksComposerField (host-testable), surfaced on the controller and bound to the field's .disabled.
  • mic toggle and send sit outside the field and stay live; send hard-cancels dictation -> idle, re-enabling + refocusing the field.
  • Guarded the focus-loss finalize: disabling a focused field makes SwiftUI resign first responder, so the focus-loss stop() is skipped while the field is locked (otherwise dictation would stop itself the instant it started).

Preserved from prior rounds: @observable controller, graceful-stop vs hard-cancel, cancellable .requestingPermission, send-path hard-cancel, valid AVAudioSession config.

Tests: added 4 host tests for the lock predicate (ComposerDictationTests); full suite runs on the ios-simulator gate. Budget green (TerminalComposerView 810 -> 825 bumped in the tsv).

# Conflicts:
#	.github/swift-file-length-budget.tsv
@lawrencecchen
lawrencecchen merged commit a7a892b into main Jun 16, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5674550. Configure here.

// Denied or restricted: a terminal rest state that disables the
// mic. The captured callback is dropped.
self.onText = nil
self.state = .unavailable

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unavailable mic never retries

High Severity

After a denied permission or a failed start, ComposerDictationController moves to unavailable and the mic button is disabled via isAvailable. Nothing transitions back to idle when the user later grants speech/mic access in Settings or when a transient setup error clears, so dictation can stay permanently off for that composer instance.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5674550. Configure here.

// started. Only a focus loss while the field is NOT locked is the user
// moving on, and only that should finalize.
if !focused, !dictation.locksComposerField {
dictation.stop()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Focus loss won't stop dictation

Medium Severity

Focus loss is supposed to call dictation.stop(), but while listening the field is disabled and locksComposerField is true, so the initial resign-first-responder focus change is ignored. After that the field usually stays unfocused, so tapping away no longer changes focus and dictation keeps capturing until the mic or send path runs.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5674550. Configure here.

austinywang added a commit that referenced this pull request Jun 16, 2026
package-conventions-lint scans the whole iOS package tree and flagged this
pre-existing caseless-enum namespace (added in #6197, whose lint run was
skipped). It is a deliberate pure, stateless text-merge factored out for
host-testing; mark it as a sanctioned lint:allow exception so the iOS lint
gate passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Jun 16, 2026
ComposerDictationTextMerge (landed in #6197) is a caseless static-member
enum with no lint:allow marker, so package-conventions-lint (a required
check) has been red on main and on every PR branched off it. It is a
genuine stateless pure-function namespace with no instance state to own,
so the sanctioned inline lint:allow escape hatch is the correct fix rather
than reshaping it into an instantiated type.

Unblocks this consolidation PR's required CI; the consolidation diff
itself is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Jun 16, 2026
…tring

The caseless enum ComposerDictationTextMerge (added by #6197, on main) trips
the namespace-enum convention lint, which scans the whole iOS tree and fails
package-conventions-lint on every PR. Convert the pure base+transcript merge
into a receiver-natural String extension method, mergingDictation(transcript:),
per the linter's recommended pattern, and update the controller call site and
host tests. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Jun 16, 2026
…s) (#6222)

* Consolidate workspace surface-list extraction into CmuxWorkspaces (no new package)

Folds the per-workspace surface-list derivation from
feat-workspace-surface-list-model into the EXISTING CmuxWorkspaces domain
package instead of standing up a new top-level CmuxWorkspaceSurfaceList
package. The owner rejected the per-sliver micro-packages; the extraction is
good, so it lives in the workspace domain package.

What moved (byte-identical logic):
- WorkspaceSurfaceListModel (the @mainactor @observable derivation model:
  orderedPanelIds, focusedPanelId, representativePanelIdForWorkspaceManualUnread,
  effectiveSelectedPanelId, the tabIdsTo* pane queries, and the paneLayoutVersion
  reorder bump) and its WorkspaceSurfaceTreeReading seam protocol now live in
  Packages/CmuxWorkspaces/Sources/CmuxWorkspaces/SurfaceList/. Both files are
  byte-identical to the member branch; they only import Foundation/Observation,
  so CmuxWorkspaces's Package.swift needs no new dependency.
- The 12 behavior tests move into CmuxWorkspacesTests (only the @testable
  import target changed from CmuxWorkspaceSurfaceList to CmuxWorkspaces).

App-side seam unchanged in shape: Workspace+WorkspaceSurfaceTreeReading.swift
conforms Workspace to the seam and Workspace holds the model (weak back-ref via
the seam to avoid a retain cycle), with the legacy accessors as one-line
forwards. Every `import CmuxWorkspaceSurfaceList` became `import CmuxWorkspaces`
(removed in Workspace.swift, which already imports CmuxWorkspaces).

No new top-level package: Packages/CmuxWorkspaceSurfaceList/ is deleted and its
6 pbxproj package-reference entries are removed. The pbxproj only gains the
4 source-file wiring entries for Workspace+WorkspaceSurfaceTreeReading.swift.

Verified: scripts/lint-ios-package-conventions.sh adds zero new violations (the
one pre-existing namespace-enum ERROR in CmuxMobileShellUI is untouched debt on
main); swift build + swift test green in Packages/CmuxWorkspaces (32 tests, 4
suites). Budget for Sources/Workspace.swift ratcheted to 12978.

Supersedes the standalone CmuxWorkspaceSurfaceList package PR from
feat-workspace-surface-list-model.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix pre-existing package-conventions-lint break on main

ComposerDictationTextMerge (landed in #6197) is a caseless static-member
enum with no lint:allow marker, so package-conventions-lint (a required
check) has been red on main and on every PR branched off it. It is a
genuine stateless pure-function namespace with no instance state to own,
so the sanctioned inline lint:allow escape hatch is the correct fix rather
than reshaping it into an instantiated type.

Unblocks this consolidation PR's required CI; the consolidation diff
itself is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Jun 17, 2026
)

* Consolidate debug extractions into CmuxFeedback + CmuxAppKitSupportUI (no new packages)

Folds two per-sliver refactor branches into existing domain packages so the
debug-group extractions land without creating any new top-level package.

1. feat-mobile-host-rpc-router extracted the privileged Mac<->phone dogfood
   feedback sink into a new CmuxDogfoodFeedbackSink package. That domain folds
   into the existing CmuxFeedback package under DogfoodSink/: DogfoodFeedbackLimits,
   DogfoodFeedbackOutcome, DogfoodFeedbackSubmission (Sendable value types) and the
   nonisolated Sendable DogfoodFeedbackService. Byte-identical logic (same caps,
   base64-char-cap-before-decode then byte-cap ordering, Task.detached(.utility)
   off-main write, ISO8601-colons-to-dash bundle naming, 0700/0600 perms,
   bundle.json schema/sorted-keys/pretty, lexicographic prune keeping newest 50,
   and the same RPC error codes/messages). TerminalController.v2MobileDogfoodFeedbackSubmit
   is now a thin forward that resolves the authenticated email via the main-actor
   MobileHostService and calls service.submit(...); the service re-enforces the
   @manaflow.ai gate at the trust boundary. CmuxFeedback is already imported by
   TerminalController and already linked to the app target, so no import or pbxproj
   change was needed.

2. feat-debug-windows-extraction extracted the self-contained About-titlebar debug
   cluster into a new CmuxDebugWindowsUI package. That UI folds into the existing
   CmuxAppKitSupportUI package under AboutTitlebarDebug/: the AboutWindowKind /
   TitlebarVisibilityOption / TitlebarToolbarStyleOption value enums,
   AboutTitlebarDebugOptions value type, AboutTitlebarDebugStore
   (@mainactor @observable, single writer), AboutTitlebarDebugWindowController,
   AboutTitlebarDebugView, plus the DebugWindowsCoordinator and the WindowDecorating
   protocol seam. AppDelegate conforms to WindowDecorating and owns the coordinator
   (held weakly by the coordinator/store to avoid a retain cycle); cmuxApp.swift and
   the About/Acknowledgments controllers forward into the app-owned
   coordinator/store. Byte-identical window identifiers, titles, style-mask bits,
   toolbar identifiers, sizes, and copy-config payload. CmuxAppKitSupportUI already
   exists and is already linked to the app target.

Both member branches branched off an older main (5321bec), before CmuxFeedback and
CmuxAppKitSupportUI existed, which is why they created standalone packages. Neither
new package is created here; zero new top-level packages and zero pbxproj entries
were added.

Verification: scripts/lint-ios-package-conventions.sh introduces no new violation
and zero lint:allow (the single pre-existing ERROR is ComposerDictationTextMerge in
CmuxMobileShellUI, unrelated to this change). swift build + swift test green in both
packages (CmuxFeedback 12 tests, CmuxAppKitSupportUI 8 tests). File-length budget
reconciled: TerminalController 14829->14681, cmuxApp 4921->4516, AppDelegate
17894->17905 (+11 for composition-root wiring).

Supersedes the per-sliver package PRs from feat-mobile-host-rpc-router and
feat-debug-windows-extraction.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix package-conventions-lint: scope ComposerDictationTextMerge onto String

The caseless enum ComposerDictationTextMerge (added by #6197, on main) trips
the namespace-enum convention lint, which scans the whole iOS tree and fails
package-conventions-lint on every PR. Convert the pure base+transcript merge
into a receiver-natural String extension method, mergingDictation(transcript:),
per the linter's recommended pattern, and update the controller call site and
host tests. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* AboutTitlebarDebugStore: split config snapshot from pasteboard write

The extracted store is now public package code with a unit test. Splitting the
pure configSnapshot() from copyConfigToPasteboard() lets the test assert the
payload without clearing the real NSPasteboard.general on the dev/CI process.
No behavior change to the menu action.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — 56745509 Deployed Jun 16, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant