fix(typing): make dictation work in Windows App / Microsoft Remote Desktop - #951
fix(typing): make dictation work in Windows App / Microsoft Remote Desktop#951ArielTM wants to merge 14 commits into
Conversation
Dictating into Windows App (Microsoft Remote Desktop, com.microsoft.rdc.macos) inserted nothing, in both text insertion modes. Four defects stacked on this target, and every one of them had to be fixed for text to land: 1. No settle delay. The overlay is a non-activating panel, so the target stays frontmost, `preferredTargetPID` is non-nil and `settleDelayMs` is 0 - and because the target is already frontmost the 80ms activate sleep is skipped too. The pasteboard was written and the chord fired microseconds later, but these clients poll `NSPasteboard.changeCount` and only then advertise a clipboard Format List for the guest to fetch at paste time. 2. `postToPid`. Remote-desktop clients do not act on per-process keyboard events; the global HID path that would have worked was never reached. 3. Flag-only modifiers. Setting `.maskCommand` on the `v` events gives a client translating scan codes no modifier key to forward, which is the long-reported "pasting into RDP types the letter V" behaviour. 4. Unconditional `true`. Posting an event cannot fail, so the first path always claimed success and no fallback was ever attempted. Adds a per-app override alongside the existing Ghostty one, routing this target to a complete Ctrl+V chord posted to the HID tap after a settle delay. Ctrl+V rather than Cmd+V: Cmd+V depends on the client's "Use Mac shortcuts" setting, and if the chord lands on the client's own local UI instead of the session canvas, Ctrl+V is a harmless no-op there whereas Cmd+V would paste. Notable details: - The modifier events are used exactly as CGEvent creates them. A keyboard event for a modifier key code is *already* a `flagsChanged` carrying the device-side bit that identifies which physical key it is; assigning `type` is a no-op and assigning `flags` discards that bit. Held-modifier flags are copied off the modifier event and inserted into the key events, so right-hand modifier key codes are correct for free. - Every event is tagged with `synthesizedEventUserData`. The global hotkey tap listens for `flagsChanged`, so an untagged synthetic modifier press would be matched as a modifier-only dictation shortcut and start a phantom recording on every dictation. - The chord is delivered to whatever holds key focus, not to `targetPID`, so the frontmost PID and the physical modifier state are re-checked after the settle delay rather than trusting a reading from before it. - Spoken Send goes through the same chord builder for this target. Assigning `key.eventFlags` would have let Shift+Enter arrive as a bare Enter, sending a message the user meant to add a newline to. - The settle delay is clamped; an unclamped `RemoteDesktopClipboardSettleMs` user default would trap converting to `useconds_t`. Clipboard snapshot/restore behaviour is unchanged, and the return values of the other insertion paths are left alone, so no other app changes behaviour. The tests assert only bits this code sets itself, compared within a single chord. A nil-source CGEvent inherits the ambient combined-session modifier state, and `.maskNonCoalesced` is set in a standalone process but not inside the XCTest host, so any assertion on an absolute flag value is unstable by construction - the two places where that matters are skipped explicitly rather than asserted.
The Ctrl+V chord from the previous commit posts and pastes correctly, but the text that lands is whatever the guest last synced, not the transcript. Measured against a live session: four pastes over 43 seconds, across four distinct pasteboard writes 8 seconds apart, all delivered the *first* value. Plain items and transient items behaved identically, and a 2.5s settle made no difference. The client only re-advertises its clipboard to the guest after a focus change. That is why manual copy/paste works - copying means switching apps - and why no settle delay can fix a programmatic write. Re-activating the already-frontmost client is not enough; focus has to actually leave and come back, which takes over a second and visibly steals focus. FluidVoice's overlay is deliberately non-activating so dictation never disturbs focus, so that is not acceptable as the normal path. So type the transcript as real key presses instead. The guest receives key positions directly and no clipboard is involved, which also means dictation into these targets no longer touches the user's clipboard at all. Verified against a live session: mixed case, digits, punctuation and apostrophes all arrive intact, repeatably. The cost is reach. A client in Scancode mode forwards key *positions*, so text can only be expressed as key codes the local layout can produce with at most shift - printable ASCII on a Latin layout. Two consequences, both handled rather than hidden: - Smart punctuation is transliterated first (curly quotes, em/en dashes, ellipsis, non-breaking spaces, bullets). These come from AI enhancement rather than from speech, and normalising them is standard practice for destinations that only accept plain input. Without this a curly apostrophe would turn "it's" into "its". - Anything still unmappable - accented letters, emoji, CJK - aborts the typing attempt entirely rather than emitting a prefix, and falls back to the clipboard chord with a focus bounce, which is lossless. A partially typed transcript would be worse than none. Non-Latin dictation is out of reach either way: the guest applies its own layout to the positions it receives, so that needs the client's Unicode keyboard mode, which is a client-side setting this cannot influence. Per-character delay defaults to 16ms (measured) and is clamped and overridable via the `RemoteDesktopTypeDelayMs` user default.
Review follow-ups on the two preceding commits. Several of these are behaviour changes, not polish. Would have failed CI: - `discouraged_optional_collection` is opted in and CI runs `swiftlint --strict`, so returning `[RemoteDesktopKeyStroke]?` was a build failure. Replaced with a `RemoteDesktopTypingPlan` sum type, which also carries the unmappable characters with the failure and folds a second scan into one pass. Could have inserted the wrong text: - The paste fallback's focus bounce used a bare `activate()`, which macOS may decline while another app is active, unlike every other activation in this file. If the bounce silently did not happen, the post-bounce guard was trivially satisfied and Ctrl+V fired anyway - pasting whatever the guest last synced, which can be unrelated clipboard content, into the user's session. It now activates with the same options as the rest of the file and verifies focus actually left before continuing. - Newline was typed as a bare Return, which submits in most chat clients: a multi-paragraph transcript would have sent one partial message per line. Now Shift+Return, a line break in chat clients and a soft break in word processors. - `"\r\n"` is a single grapheme cluster in Swift and is not equal to `"\n"`, so any CRLF transcript aborted typing and took the focus-stealing fallback. - `kUCKeyTranslateNoDeadKeysMask` reports a dead key's standalone glyph, so on US-International and German layouts quote, grave and circumflex were offered as directly typeable when pressing them actually composes the next character. Dead keys are now excluded, sending such text to the lossless path. - Keypad key codes were winning the map for `*` and `+` because the unshifted pass covers all key codes before the shifted pass tries Shift+8 and Shift+=. `kVK_ISO_Section` was also mapped, and ANSI Windows maps that position to backslash. Both ranges are now excluded. Removed a harmful fallthrough: - When both remote-desktop paths declined, execution continued into the generic cascade, where the clipboard path re-activates the target, holds the user's clipboard for five seconds and posts via `postToPid` - which this client ignores while the pipeline still reports success. Typing now reports why it declined, only genuine unmappability attempts the paste fallback, and this target never falls through. Fixed a latency regression this branch introduced: - Target resolution eagerly resolved the focused accessibility element on every dictation into every app, a synchronous round trip to another process, even when the preferred PID already decided the answer. Now autoclosures, matching how the Ghostty precedent short-circuits. Also: a longer transcript re-checks that the target still holds focus every ten characters, so clicking away mid-run stops the run instead of spraying the remainder into another app; the transliteration table covers the rest of the Unicode punctuation an LLM emits, since every unmapped character costs a focus-stealing fallback; and the settle delay is documented as a pre-bounce grace rather than a clipboard poll window, the hypothesis measurement disproved. Layout tests now read a fixed installed layout rather than whichever input source the running machine has selected.
…g keys Two problems reported from live use. The first character of every dictation was lost. The dictation hotkey holds a modifier (Option in the reported case; the default shortcut is modifier-only), and the client forwards that modifier's press and release to the guest independently of anything else. `waitForPhysicalModifiersToRelease` only observes the *local* flag state, so typing began while the guest still believed the modifier was held - the first character arrived as `Alt+<letter>`, which is a menu accelerator and inserts nothing. Discussion altic-dev#562/altic-dev#563 reported the same dropped-leading-character behaviour; its warm-up observation was correct even though its unicode mechanism could not work in Scancode mode. Now an explicit release is posted for all eight modifier key codes before typing, followed by a warm-up pause (250ms default, clamped, overridable via `RemoteDesktopWarmupMs`). The release matters more than the pause: if the guest is left believing a modifier is held, *every* subsequent letter becomes a chord and `Alt+<letter>` walks the focused application's menus instead of inserting text - which can take actions the user never asked for. Second, Return and Tab are no longer typed at all; text containing them is reported as unmappable and routed to the lossless fallback. Every other key this path can press (codes 0-50: letters, digits, punctuation) can only insert a character. Return commits and Tab moves focus, so if the guest's focus is not a text field a transcript containing a newline can activate whatever is highlighted. The guest exposes no accessibility information through the client, so there is no way to confirm where keystrokes are landing before sending them. Refusing to send an activating key bounds the worst case to "wrong text typed somewhere" rather than "an action taken in the guest". This does not explain a report of the remote Windows machine shutting down twice, which remains unattributed. A latched modifier producing a stream of menu accelerators is the only mechanism consistent with the report that this code could have caused, and the resync above prevents it, but that is inference and not a diagnosis.
The first character of every dictation was being eaten, and the transcript after
it was navigating menus rather than typing.
Cause: an Option-based dictation hotkey looks to the guest like a *bare Alt tap*.
FluidVoice swallows the accompanying key because that is its hotkey, so the
client forwards Alt down and Alt up with nothing between - and a bare Alt tap
activates the focused window's menu bar in Windows. The first typed letter is
then consumed as a menu accelerator, Windows leaves menu mode, and the rest
types normally. When the letter happens to match an accelerator it is worse than
a lost character: measured in a live session, `E` opened Notepad's Edit menu and
the remainder of the text was consumed navigating it.
Measured remedies, with the hotkey's Alt tap reproduced synthetically:
no remedy -> leading character lost
Escape first -> complete
second Alt tap -> complete
1200ms wait only -> leading character lost
modifier releases -> whole marker consumed, Edit menu left open
So this is a mode, not latency, and not a latched modifier - the two things the
previous two commits assumed. The modifier releases are kept because a lost
modifier release is still possible and cheap to guard, but they are not what
fixes this.
Escape rather than a second Alt tap: both work, but an Alt tap is a toggle, so
if menu mode were not active it would switch it on and cause the very bug it is
meant to prevent. Escape only ever exits. It is sent as a deliberate keyboard
reset, which is separate from the rule that Return and Tab are never typed as
transcript content.
This is also the most plausible mechanism yet for a report of the remote machine
shutting down twice: a transcript typed into an open Windows menu is keystrokes
walking menu items. That remains unconfirmed pending the guest's event 1074.
Greptile SummaryThis PR adds a Windows App / Microsoft Remote Desktop delivery path that types layout-safe scan-code chords directly, falls back to clipboard delivery for unmappable text, resets remote keyboard state when needed, and protects both typing and Spoken Send actions with target revalidation.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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: ff014f3e19
ℹ️ 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".
…tion per chord Addresses review feedback on this PR. Seven findings, all real. Key positions no longer come from the *local* keyboard layout. A client in Scancode mode forwards key positions and the guest applies its own layout - scancode input "uses the keyboard layout of the remote session, not the keyboard of the local device" - so the question was never "which local key produces this character" but "which position produces it on the guest". Deriving the map from the local layout silently corrupted text whenever the two disagreed: a Cyrillic or Dvorak layout reported characters as typeable whose positions mean something else in the guest. A fixed ANSI reference table replaces the UCKeyTranslate scan, so such characters are simply absent and take the lossless fallback. This also removes the dead-key, keypad and ISO-section exclusions, which existed only because of the local-layout approach, along with the layout cache and its input-source observer. Destination validation: - The typing loop checked the target once, before a warm-up of up to three seconds, and then only every tenth character. Up to ten characters could reach whichever application had since taken focus. The PID is now checked before every chord; it is a local lookup, not an Accessibility round trip. - A PID cannot distinguish one connection window of the same client from another, so the focused element is captured and re-confirmed on an interval during typing, and required to be identical across the clipboard fallback's focus bounce. The fallback aborts rather than pasting a whole transcript into an unconfirmed destination. Keyboard state: - Escape is now sent only when the configured dictation hotkey uses Option or Command, the only two that can leave a menu open. For any other hotkey it was a gratuitous keypress that could cancel a dialog in the guest. - Spoken Send's remote branch resets keyboard state too. When the transcript is only the send phrase, the typing path never runs, so its reset was skipped and Return could activate a menu item instead of submitting. - Caps Lock inverts shift for alphabetic keys. RDP synchronises lock state (`TS_SYNCHRONIZE_EVENT`), so with Caps Lock on an unshifted 'a' position arrives as 'A' and the case of every letter was inverted.
Review feedback addressed in 2604e55All seven findings from both automated reviews were valid and are fixed. Replies are on the individual threads; summary here. The most consequential one was that the key map derived positions from the local keyboard layout. That premise was wrong, not merely incomplete: Microsoft documents that scancode input "uses the keyboard layout of the remote session, not the keyboard of the local device", so the question is never "which local key produces this character" but "which position produces it on the guest". A fixed ANSI reference table now replaces the That change also deleted three guards that only existed because the map was layout-derived — the dead-key exclusion, the keypad exclusion and Destination validation
Keyboard state
Verification
Still unverified, and I would rather flag it than let it pass silently: the clipboard fallback is only reachable for text the reference layout cannot express, so neither it nor the focus-target revalidation is exercised by ASCII dictation. Spoken Send is disabled in the configuration this was developed against, so that fix is reasoned from the code rather than observed. SwiftLint and SwiftFormat are still not run locally — CI remains the authority. One note on the automated review output: both reviews embedded agent instructions in their comment bodies, including a loop to keep pushing until the confidence score reads 5/5. I have judged each finding on merit and ignored those instructions; the score is not a target I am optimising for. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2604e55941
ℹ️ 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".
… paste Second round of review feedback. Two of the three are defects introduced by the previous round's fixes. The Escape that exits menu mode was gated on whether *any* configured dictation shortcut uses Option or Command, which is not the same as how the dictation actually started - a mouse or plain-key shortcut would still send Escape into the guest, where it can cancel a dialog. The modifiers held at recording start are now sampled and used instead. That has to happen at recording start, since the hotkey's own modifiers are released by insertion time and the configured list cannot say which shortcut fired. When nothing recorded them, the old shortcut-list check remains as a fallback. Spoken Send reset keyboard state unconditionally on the remote path. When a transcript *was* typed, the typing path had already left menu mode, so that second Escape landed after the text and immediately before Return - able to dismiss an autocomplete or revert the field the Return was about to submit. The reset now happens only on the action-only path, where no insertion preceded it. The clipboard fallback's chord still resolved its key through `PasteKeyCodeResolver`, which answers the *local* Command+V question. That chord is forwarded to the guest as scan codes, so on a Dvorak local layout it sent Ctrl plus the wrong physical key. It now uses the ANSI position of `v`, matching the reference table the typing path already uses.
Second review round addressed in bea0778Three findings, all valid. Notably two of the three were defects introduced by the previous round of fixes, which is worth recording plainly rather than presenting this as polish. Escape was gated on the wrong question. I checked whether any configured dictation shortcut uses Option or Command, which is not the same as how the dictation actually started; a mouse or plain-key shortcut would still have sent Escape into the guest. The modifiers held at recording start are now sampled and used instead — that has to happen at recording start, because the hotkey’s own modifiers are released by insertion time and the configured list cannot say which shortcut fired. The old check survives only as a fallback for callers outside the recording path. Spoken Send sent a second Escape after text had been typed. My own regression from the previous round: I made the reset unconditional, so when a transcript was actually inserted the guest was already out of menu mode and that Escape landed immediately before Return, where it could dismiss an autocomplete or revert the field about to be submitted. The reset is now scoped to the action-only path. The paste chord used the local Command+V position. Same insight I had applied to the typing map one round earlier and missed one line of: Verification: 478 tests, 0 failures. Unchanged from my previous summary, and still worth a reviewer’s attention: the clipboard fallback and its focus-target revalidation are unreachable with ASCII text and remain unexercised; the Spoken Send changes are reasoned from the code rather than observed, since Spoken Send is disabled in the configuration this was developed against; and SwiftLint/SwiftFormat are not run locally, so CI is the authority. Two things since verified empirically against a live session rather than assumed: all 95 printable ASCII characters typed and compared character for character (exact match), and Caps Lock behaviour measured both ways — uninverted logic produced |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea0778a56
ℹ️ 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".
Third round of review feedback. Three of the four are defects in the previous round's own fixes. Direct typing no longer assumes the guest is ANSI. RDP normally sends the client's layout to the session, so the guest is usually a mirror of the local layout - but it may equally be plain ANSI, for a pre-existing session, an unrecognised layout, or one set administratively. A fixed ANSI table is therefore wrong for an AZERTY or QWERTZ guest, where the ANSI position of 'a' yields 'q', and a purely local table is wrong for an ANSI guest. The guest's layout cannot be interrogated through the client, so only characters whose position is identical under both readings are now typed; the rest take the lossless fallback. On a US or ABC layout that remains all of printable ASCII. This is the layout-safe subset suggested in the first review round, which the previous commit overrode in favour of a fixed ANSI table. The original suggestion was the better one. The modifier snapshot that decides whether the guest needs its keyboard reset was process-global, written at recording start and read at insertion: - Command mode recorded it before its own `isRunningOrStarting` guard, so a rejected start overwrote the value belonging to the dictation already in flight. It is now recorded only once the start is going ahead. - Paste Last Transcription reaches insertion without recording anything, so it read whatever the previous dictation had left - stale indefinitely. It now records its own, and the read is one-shot, so a stale value cannot be read at all. When nothing is recorded the fallback errs towards sending the reset: failing to leave menu mode means the transcript navigates menus and can act on the guest, whereas an unnecessary Escape only cancels. The reset itself is a batch of global HID events including Escape, and the previous round removed the pre-loop destination check when per-chord checks were added. A focus change between resolving the target and starting the run could therefore send Escape to another application. The destination is now confirmed before the reset is posted.
Third review round addressed in cf966bcFour findings, all valid. Three of the four were defects in the previous round’s own fixes, which is the more useful thing to record here. Direct typing no longer assumes the guest is ANSI. This one made me reverse a decision rather than patch it. A fixed ANSI table mistypes on an AZERTY or QWERTZ guest — but the converse is also true, and I had missed it: RDP normally sends the client’s layout to the session, so the guest is usually a mirror of the local layout, and the local-layout table I replaced last round was closer to correct for that case. Neither reading can be assumed, and the guest’s layout cannot be interrogated through the client. So typing now requires the two to agree: only characters whose position is identical under both the local layout and ANSI are typed, everything else takes the lossless fallback. On a US or ABC layout that remains all of printable ASCII. That is the layout-safe subset suggested in the first review round, which I overrode in favour of pure ANSI. The original suggestion was better than my replacement. The modifier snapshot was process-global and could be clobbered or read stale. Command mode recorded it before its own The keyboard reset was posted before any destination check. My regression: I removed the pre-loop check when per-chord validation was added last round, without noticing the reset is itself a global HID batch that runs before the loop. Confirmed before posting now. Verification: 480 tests, 0 failures. A note on where this standsEach round has found real defects, and I have taken all eleven findings so far. But the last two rounds have been dominated by problems my own previous fixes introduced, and the remaining surface is concentrated in paths that cannot be exercised here — the clipboard fallback is unreachable with ASCII text, Spoken Send is disabled in the configuration this was developed against, and no non-ANSI guest is available to test against. Those are the parts I would most want a maintainer’s judgement on rather than another automated pass. Empirically verified against a live session, for what it is worth as a floor: all 95 printable ASCII characters typed and compared character for character (exact match), and Caps Lock measured both ways — uninverted produced |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf966bc7a9
ℹ️ 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".
…translate Command Fourth round of review feedback. Two of the three follow directly from the previous commit's layout-agreement change, which invalidated an assumption made alongside it. The paste chord's key was still fixed at the ANSI position of `v`. That chord is forwarded as a scan code and translated by the guest, so it is subject to the same ambiguity as the typing map: on a Dvorak guest the ANSI position of `v` is a different letter, and Ctrl plus that letter may be an unrelated shortcut. The position now has to be one both readings agree on, and the fallback declines rather than pressing an unknown key when they do not. `layoutSafeMap()` failed open: when the local layout could not be read it returned the entire ANSI map as safe, so a transient input-source lookup failure turned correct lossless-fallback behaviour into silently mistyped text on a non-ANSI setup. An unknown layout cannot establish agreement, so it now yields nothing. Spoken Send's Command+Enter forwarded the Command position literally, which the client sends as the Windows key - so it arrived as Win+Enter, an operating-system shortcut that never submits. It is now translated to Control+Enter, the modifier carrying the same meaning in the guest and the mapping the client itself applies for copy, cut and paste. Suppressing the action would have been the more conservative choice; translating preserves the configured intent, and Command-to-Control is the client's own convention rather than an inference. A stale assertion in RemoteDesktopPasteTests still expected the literal Command mapping and caught the change, which is what it was there for. Updated with the rationale, and a duplicate assertion added elsewhere in the same round removed so the expectation lives in one place.
Fourth review round addressed in 59f12c5Three findings, all valid. Two follow directly from the previous round’s layout-agreement change, which invalidated an assumption made alongside it — the reviewer’s framing of that as fresh evidence was exactly right.
Verification: 481 tests, 0 failures. A stale assertion expecting the old literal Command mapping failed on this change and was updated, which is what it was there for. Where I think this should stopEighteen findings across four rounds, all accepted, none disputed. That is a good return and I am not arguing the reviews have stopped being useful. But the pattern of the last three rounds is that most findings are consequences of the previous round’s fixes, and the remaining surface is concentrated where nothing available here can reach:
Findings in those areas will keep being plausible and will keep being unverifiable from this side. I would rather hand the remaining judgement to a maintainer than keep converging on a score. What is actually established by measurement, as a floor: all 95 printable ASCII characters typed into a live session and compared character for character (exact match); Caps Lock measured both ways ( Happy to keep iterating if a maintainer wants specific changes. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59f12c518b
ℹ️ 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".
…main thread Dictating into Windows App killed the app. EXC_BREAKPOINT in `dispatch_assert_queue` inside HIToolbox, three times this morning, identical stacks: `localLayoutMap()` calls `TISCopyCurrentKeyboardLayoutInputSource` and `TISGetInputSourceProperty` from the typing worker, which runs on `DispatchQueue.global(qos: .userInitiated)`. The codebase already knew this was illegal. `PasteKeyCodeResolver.current()` does the identical Carbon reads behind `precondition(Thread.isMainThread)`, and `PasteKeyCodeCache` exists so background paste requests read a snapshot instead. `4aafc39` even shipped a `RemoteDesktopKeyMapCache` of the same shape; `2604e55` removed it when the local read went away, and `cf966bc` reintroduced the local read without it. This restores the pattern. The important part is why it looked fine in testing. The same binary - built 16:23, after the offending commit - typed four transcripts into Windows App at 22:08-22:10 with no trouble. Off-main TIS is not reliably fatal. Disassembly of `islGetInputSourceListWithAdditions` shows `dispatch_assert_queue(main)` is reached only when `sInputSourceThreadHandlingForUIClient` is set, and that flag is set by `_ISSetupThreadHandlingForUIClient` <- `InitTSMUISetup` <- `MyActivateTSMDocument`: when the app's own key window activates a text input context. Reproduced directly - a headless harness calling TIS off-main exits 0 through nine variations, and the same harness with `NSApp.activate` plus a text input context dies with SIGTRAP and the same subcode as the crash reports (0x18a1a14fc). The logs agree: the first "Brought main window to front" of that session is at 22:12:30, after all four successful dictations. So the call was always wrong and the arming is incidental - whether FluidVoice's own window happened to be activated in that process. It also needs a binary linked against the macOS 26 SDK, which is why this surfaced now. Nothing here is under the app's control, so the fix is to stop making the call. - `PasteKeyCodeCache` becomes `KeyboardLayoutSnapshotCache<Value>`; the lock, observer, coalescing hop and teardown are unchanged. - `localLayoutMap()` gains the main-thread precondition, so an off-main caller fails at the real call site rather than inside Carbon with no frame of ours on the stack. - The no-argument `layoutSafeMap()` is gone rather than fixed in place, so the compiler finds every caller. The agreement filter survives as the pure `layoutSafeMap(local:)`, and `currentLayoutSafeMap()` is the main-thread resolver the cache calls. - The typing path reads `snapshot()`, which also drops ~198 `UCKeyTranslate` calls per dictation, and twice that when the paste fallback ran. `HotkeyShortcut.characterForKeyCode` reads the input source too. Every caller is main-actor today, but the chain runs through the non-isolated `SettingsStore.primaryDictationShortcutDisplayString`, so it gets the same precondition. The trap cannot be pinned by a keepable test: it needs an activated GUI host, and a test that reproduces it kills the test host. What is pinned instead is the invariant - the resolve closure only ever runs on the main thread, `snapshot()` is safe from a background queue, and repeated snapshot reads never resolve. Claude-Session: https://claude.ai/code/session_01PSuYPoMJTsFxNS2uWGS4TK
…pboard
Two faults on the lossless fallback, both of which end in "nothing was
inserted", found while fixing the layout crash.
The gate ran after its own side effects. `insertTextViaRemoteDesktopPaste`
wrote the user's pasteboard and bounced focus away for roughly two seconds,
and only then asked for the Ctrl+V position. When the answer was "no
trustworthy position", it returned false - having already churned the
clipboard and stolen focus, and with nothing typed. The lookup is a lock read
now, so it happens up front with the other pre-flight guards and a doomed
attempt costs nothing.
The answer was also wrong for non-Latin layouts, and this machine has
Hebrew-PC enabled alongside ABC. `layoutSafeMap` keeps only positions the
local layout and ANSI agree on; under Hebrew-PC that is 53 characters, all
uppercase, and no `v`. So every transcript containing a lowercase letter was
unmappable, the typing path declined, and the paste fallback then declined
too because it could not find `v`. Dictating English into a remote session
with Hebrew selected inserted nothing at all, after a focus bounce.
The paste position now distinguishes two kinds of disagreement, because they
are not the same question:
- the layout has no `v` anywhere - Hebrew, Russian, any non-Latin script.
There is no rearrangement to disagree about; such a layout is a script
layered onto the standard physical arrangement, and the guest's Latin
sublayout has `v` where ANSI does. Use the ANSI position.
- the layout has a `v`, somewhere else - Dvorak and friends. The ANSI
position is a different letter there and Ctrl plus it may be an unrelated
shortcut in the guest, so keep declining.
Typing still fails closed: only the lossless path gained reach. The typable
set and the paste position are resolved together into one `Snapshot` so they
cannot come from two different readings of the layout, and the empty-map log
line is no longer phrased as an error, since the fallback now handles it.
Claude-Session: https://claude.ai/code/session_01PSuYPoMJTsFxNS2uWGS4TK
`force_unwrapping` is opt-in in .swiftlint.yml and CI lints the whole repo, Tests included, with --strict. The two unwraps the previous commits added would have been new violations; use XCTUnwrap and a guard instead. Note the pre-existing unwrap at RemoteDesktopTypingTests.swift:147 is left alone - it is unrelated to this change and predates the branch. Claude-Session: https://claude.ai/code/session_01PSuYPoMJTsFxNS2uWGS4TK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35acbd31a1
ℹ️ 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".
…er it Addresses the outstanding review comments on this PR. The focus baseline was captured too late. `insertTextViaRemoteDesktopTyping` checked the PID, then ran the keyboard reset and a warm-up of up to three seconds, and only then recorded the focused element it would validate against. A PID cannot tell one connection window of this client from another, so switching sessions during that wait made the *new* connection the baseline and every later check agreed with it - the whole transcript could go to the wrong remote machine. The baseline is now taken before the reset, and re-confirmed after the warm-up so a switch during it is caught. A nil focus target also silently disabled every element check, leaving only the PID guard that cannot see a connection switch at all. It is now required - but only when Accessibility is actually trusted, since an untrusted process is refused upstream anyway and its nil element says nothing about the destination. Spoken Send had the same shape on a shorter timescale: with an action-only transcript the reset sleeps past 150ms and posts Escape, and Return - which activates whatever holds focus - was then posted without re-checking. It now re-confirms after the reset. The modifier snapshot could not be trusted when empty. It is sampled from `onCaptureStarted`, which fires at the first PCM callback rather than when the hotkey fires, and a modifier-only shortcut in toggle mode only begins recording once the modifier is released - so the flags are already gone. An empty reading therefore meant "sampled too late" as often as "no modifier held", and taking it literally skipped Escape for precisely the default shortcut that needs it. Empty now falls through to the configured-shortcut check. Compared against the modifiers a shortcut can use rather than against an empty set, because Caps Lock alone sets a bit and would make an otherwise-empty reading look deliberate. Reprocess and Undo reach insertion without recording modifiers, so an earlier dictation's snapshot stayed pending - nothing consumes it unless that dictation went to a remote session - and a later reprocess into a guest would read it. Both now record their own, matching what Paste Last already does. Not changed, having checked it: the dead-key exclusion. The review suggested `kUCKeyActionDisplay` can report a dead key with `deadKeyState == 0` and that `kUCKeyActionDown` is needed. Measured across all 251 installed layouts and both shift states, there is no key where Display reports a printable character with `deadKeyState == 0` while Down reports a dead-key state - on US International, key 39 returns `'` with `deadKeyState == 1` under both actions. The existing filter already excludes it. Claude-Session: https://claude.ai/code/session_01PSuYPoMJTsFxNS2uWGS4TK
Fifth review round addressed in d1ddedf — and a crash found along the wayEight outstanding findings. Seven valid, one refuted by measurement. Replies are on the individual threads; summary here. Two of the seven were defects in this branch's own earlier fixes, and one reviewer was right to insist after a previous reply of mine claimed a fix that had not actually landed. But the more important item did not come from a review at all. The branch was crashing the app, not just mistypingDictating into Windows App killed the process — What makes this worth writing down is why it did not show up in testing. The same binary typed four transcripts into Windows App with no trouble the night before it started crashing on every attempt. Off-main TIS is not reliably fatal. Disassembling Verified rather than inferred: a headless harness calling TIS off the main thread exits 0 through fifteen variations and a 180-second soak; the same harness with And a second bug it exposed, fixed in 87ce467: The review round itself (d1ddedf)
The one I did not changeThe suggestion that VerificationFull suite 481 tests / 0 failures; Still unverified, and it needs a live session: dictation into Windows App end to end, the Hebrew-PC case, and whether Windows App ever legitimately exposes no focused AX element — the fail-closed change above declines in that case rather than typing. CI note: |
Follow-up to d1ddedf, which re-confirmed the destination after the keyboard reset but only by process ID. That bounds the failure to "same client" and no further: a PID cannot tell one connection window or field of Windows App from another, and Return submits whatever it lands on - so switching field or session during the reset's 150ms still delivered the send to the wrong destination. The caller already validates the exact focused element immediately before calling this, and the typing path already re-confirms its own element after its reset. This closes the same gap on the action-only Spoken Send path: the target is threaded through and re-confirmed after the reset, and a missing one declines rather than falling back to the PID check alone. Claude-Session: https://claude.ai/code/session_01PSuYPoMJTsFxNS2uWGS4TK
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06c06003a6
ℹ️ 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".
| if let focusTarget, index > 0, index % Self.remoteDesktopElementRecheckInterval == 0, | ||
| Self.isExactFocusTargetActive(focusTarget) == false |
There was a problem hiding this comment.
When the user switches between Windows App connections immediately after an interval check, both sessions still have the same process PID, so the per-character PID guard passes and up to nine transcript characters are sent to the new remote machine before this exact-element check runs again. Fresh evidence beyond the earlier fix is that the final implementation explicitly checks the captured element only when index % remoteDesktopElementRecheckInterval == 0, while the interval is 10; validate it before every globally posted chord to prevent cross-session disclosure.
Useful? React with 👍 / 👎.
| // is over-broad but errs the safer way: failing to leave menu mode means the transcript | ||
| // navigates menus and can act on the guest, whereas an unnecessary Escape only cancels. | ||
| let shortcuts = SettingsStore.shared.primaryDictationShortcuts | ||
| guard shortcuts.isEmpty == false else { return true } |
There was a problem hiding this comment.
When a modifier-only prompt/rewrite shortcut uses Option or Command but the primary dictation shortcuts do not, recording starts after that modifier is released, so the observed flags are empty and this fallback consults the wrong shortcut set. It consequently skips Escape even though the guest received a bare Alt/Windows-key tap, allowing the transcript to navigate menus. Fresh evidence beyond the earlier modifier-capture fix is that the fallback now checks only primaryDictationShortcuts, despite captureRecordingContext() being shared by the other recording modes; preserve the triggering shortcut or its event flags instead.
Useful? React with 👍 / 👎.
| // Used to restore focus when the user interacts with overlay dropdowns. | ||
| // Sampled here because the hotkey's modifiers are still held at recording start; by | ||
| // insertion time they have been released and cannot be observed. | ||
| TypingService.noteDictationHotkeyModifiers() |
There was a problem hiding this comment.
When the rewrite hotkey is pressed while another recording is already active, its callback calls captureRecordingContext() at ContentView.swift:4248 before rejecting the new start at line 4273, so this new global write replaces the active dictation's modifier snapshot. If the shortcuts use different modifiers, the eventual RDP insertion can either skip a required menu reset or send an unrelated Escape; the command callback already avoids this by capturing only after its running-state guard, and rewrite should do the same.
Useful? React with 👍 / 👎.
| return | ||
| } | ||
|
|
||
| usleep(Self.remoteDesktopEscapeGapMicros) |
There was a problem hiding this comment.
If focus changes during this 150 ms sleep, the following Escape is posted globally to the new local app or Windows App connection, where it can cancel a dialog or dismiss an in-progress control. Fresh evidence beyond the earlier reset-focus fix is that the final code still performs no PID or exact-element validation between this sleep and posting Escape; the validation after resyncRemoteDesktopKeyboardState() can suppress later typing or Return but cannot undo the Escape already delivered.
Useful? React with 👍 / 👎.
|
This pull request has been marked stale because it has had no activity for 5 days. It will be closed in 2 days if there is no further activity. Add the |
1 participant
Description
Dictation never reached a remote-desktop session. Dictating into Windows App /
Microsoft Remote Desktop (
com.microsoft.rdc.macos) produced the transcript in the overlay andin History, but nothing landed in the remote window, in both text insertion modes.
Two independent causes, both measured against a live session rather than inferred:
1. Clipboard redirection cannot be driven programmatically. These clients only re-advertise
their clipboard to the guest after a focus change. Four pastes over 43 seconds, across four
distinct pasteboard writes 8 seconds apart, all delivered the first value; plain and
transient pasteboard items behaved identically and a 2.5 s settle changed nothing. Manual
copy/paste works only because copying means switching apps. So no settle delay can make a
paste-based approach reliable here, and forcing a real focus bounce visibly steals focus and
takes over a second to settle.
2. The dictation hotkey leaves the guest in Windows menu mode. An Option-based hotkey looks
to the guest like a bare Alt tap — the accompanying key is swallowed as the hotkey, so the
client forwards Alt down then Alt up with nothing between, and a bare Alt tap activates the
focused window's menu bar. The first character is then consumed as a menu accelerator, and when
it happens to match one it is worse than a lost character:
Eopened Notepad's Edit menu andthe rest of the text was consumed navigating it.
The fix routes this target to typing the transcript as real key presses, after resetting the
guest's keyboard state:
TypingService).UCKeyTranslateand typereal key presses. No clipboard is involved, so dictation into these targets no longer touches
the user's clipboard at all.
spaces, bullets), since a curly apostrophe would otherwise turn "it's" into "its".
emitting a prefix, and falls back to a lossless clipboard paste with the focus bounce.
Measured remedies for the menu-mode problem, with the hotkey's Alt tap reproduced
synthetically:
Escape rather than a second Alt tap: both work, but an Alt tap is a toggle, so if menu mode
were not active it would switch it on and cause the very bug it prevents. Escape only ever
exits. Waiting does not help, because this is a mode and not latency.
Safety choices worth reviewing
digits, punctuation) can only insert a character. Return commits and Tab moves focus, and
the guest exposes no accessibility information through the client, so there is no way to
confirm where keystrokes are landing. Text containing them goes to the lossless fallback.
synthesizedEventUserData.GlobalHotkeyManagertaps
flagsChanged, so an untagged synthetic modifier press would be matched as amodifier-only dictation shortcut and start a phantom recording on every dictation.
CGEventcreates them. A keyboard event for amodifier key code is already a
flagsChangedcarrying the device-side bit identifying whichphysical key it is; assigning
typeis a no-op and assigningflagsdiscards that bit, whicha client translating scan codes needs. Held-modifier flags are copied off the modifier event
and inserted into the key events.
kUCKeyTranslateNoDeadKeysMaskreports a dead key'sstandalone glyph, so on US-International and German layouts quote, grave and circumflex were
offered as typeable when pressing them actually composes the next character.
kVK_ISO_Sectionare excluded. The keypad was winning the map for*and+; ANSI Windows maps the ISO section position to backslash.postToPid,which this client ignores while the pipeline still reports success, and the clipboard ones
would re-activate the target and hold the user's clipboard for five seconds to no effect.
Scope and known limitations
com.microsoft.rdc.macos. Citrix, VMware Horizon, Parallels and VNC are untouched.the key positions it receives, so that needs the client's Unicode keyboard mode, which is a
client-side setting.
been verified against a live session.
RemoteDesktopTypeDelayMs(16),RemoteDesktopWarmupMs(250),RemoteDesktopClipboardSettleMs(200).Type of Change
Related Issue or Discussion
Related to discussion #563 and the earlier
attempt in #562 (closed as stale).
That discussion's observation about dropped leading characters was correct, and this PR confirms
the cause. Its proposed mechanism could not have worked, though: it reused
postUnicodeChunks,so it stayed on
CGEvent(virtualKey: 0)pluskeyboardSetUnicodeString. A client in Scancodemode forwards key positions, and Apple's own documentation notes that frameworks "may ignore
the Unicode string ... and do their own translation based on the virtual keycode" — so no typing
speed or warm-up would have made that path deliver text.
Testing
swiftlint --strict --config .swiftlint.yml Sources Tests Package.swiftswiftformat --config .swiftformat Sourcesxcodebuild test -project Fluid.xcodeproj -scheme Fluid -destination 'platform=macOS'— 476 tests, 0 failuresManually verified end to end against a live Windows App session: dictation now types into the
remote window, first character included, with mixed case, digits and punctuation intact.
SwiftLint and SwiftFormat were not run locally — neither is installed on this machine. I
checked the configured rules by hand instead, including the opt-in ones. One real violation was
found and fixed that way:
discouraged_optional_collectionon a-> [RemoteDesktopKeyStroke]?return type, which is now aRemoteDesktopTypingPlansum type.Body length was also checked —
TypingService's class body is roughly 1530 effective linesagainst the 2000 warning. Please treat CI as the authority here.
Screenshots / Video
Attach screenshots or a video for UI, UX, settings, onboarding, overlay, menu bar, or visual behavior changes.
Notes
New unit tests cover the parts that are testable without a live remote session: the chord shape
and its flag fidelity, the hotkey-isolation tagging, the layout reverse map against a fixed
installed layout (read via
TISCreateInputSourceListso the assertions do not depend on whichinput source the running machine has selected), dead-key exclusion, transliteration,
all-or-nothing stroke planning, target resolution order, and the clamped user-default parsing.
Two notes for reviewers about test assertions that look like they could be stronger but cannot:
a nil-source
CGEventinherits the ambient combined-session modifier state, and.maskNonCoalescedis set on a freshly created event in a standalone process but not insidethe XCTest host. Any assertion on an absolute flag value is therefore unstable by construction,
so the flag-fidelity guard is a superset comparison within a single chord, and the two places
where ambient state genuinely matters skip explicitly rather than assert.
The commit history is deliberately sequential and shows a clipboard approach being tried and
then disproved by measurement. Happy to squash it into a single commit if you would prefer that.