Skip to content

macOS: key gate — hold the user's keystrokes during a correction (#8) - #22

Merged
vstrelnikof merged 7 commits into
Just-Code-NET:mainfrom
shohart:macos-key-gate-upstream
Aug 6, 2026
Merged

macOS: key gate — hold the user's keystrokes during a correction (#8)#22
vstrelnikof merged 7 commits into
Just-Code-NET:mainfrom
shohart:macos-key-gate-upstream

Conversation

@shohart

@shohart shohart commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Implements #8 on real hardware (macOS 15.7, Intel Mac Pro).

What changes

The event tap moves from listen-only to active (when the gate is on), and the callback consults the gate on every KeyDown/KeyUp: a held keystroke is still forwarded to the engine — which replays it behind the correction — but the callback drops it for the window server. Engine side needed nothing: HeldKeys / the flush path were already platform-ready.

  • HoldState moves windows/hold.rscrate::hold. The swallow decision is identical on both platforms, and its tests ("the user's keyboard always comes back") keep running on every host.
  • MacosGate mirrors WindowsGate: on by default, POLTERTYPE_HOLD_KEYS=0 as the escape hatch (inverse default from Windows, which has never run on hardware — this one has). `available()) additionally requires the tap to be running: a gate that claims to hold when nothing is listening would make corrections skip their compensation path and lose text.
  • Own emissions bypass the gate via the EMITTER_TAG stamp (kCGEventSourceUserData), the same self-deadlock guard the evdev gate needed. FlagsChanged is never swallowed — a held modifier edge without its counterpart would stick the system modifier state.
  • Tap-timeout survival: the callback re-enables the tap on kCGEventTapDisabledByTimeout / ByUserInput instead of staying deaf.

The dependency bump is the load-bearing part

core-graphics 0.24's tap trampoline cannot swallow: cg_event_tap_callback_internal maps a callback's None back to the original event, so an active tap returned the event either way. The gate 'held' keys, macOS delivered them anyway, and the flush replayed them — the user's characters landed twice. 0.25's CallbackResult::Drop returns NULL, which is the actual swallow. Migrating the callback to the new API is most of the diff.

Reproduced and fixed on hardware: a 4-key racer burst fired mid-correction went from привет ффыывава (doubled, 0.24) to привет фыва (0.25) — held keys re-emitted exactly once, in order, in the freshly switched layout (typed-in-new-layout semantics, as on evdev).

Test plan

  • macOS 15.7 (Intel): corrections with keystrokes racing the burst land intact and in order; gate holds/releases visible in debug log; holds_keys=true at startup
  • cargo fmt --all --check clean
  • cargo clippy --workspace --all-targets --locked -- -D warnings clean
  • cargo test --workspace --locked — same result as main on a macOS host (112 passed; 5 pre-existing plugins::menu failures that need the main thread, unrelated — they fail identically on unmodified main)

Known bounds, same as the Linux gate: a mid-correction chord (shortcut) is unreproducible and documented as lost; Enter/Tab held mid-burst are dropped by design (submission keys are never re-emitted).

@vstrelnikof

Copy link
Copy Markdown
Member

Thank you — this is the piece of macOS support I expected to be hardest, and you brought it back with the one diagnosis nobody could have made from reading the docs: that core-graphics 0.24's trampoline maps a callback's None back to the original event, so an "active" tap swallows nothing. That explains a whole class of confusing behaviour, and the привет ффыывавапривет фыва reproduction is exactly the evidence I'd want. The shape of the change is right too:

  • Moving HoldState to crate::hold is the correct call — the swallow decision is genuinely platform-free, and keeping "the user's keyboard always comes back" under test on every host matters more than module tidiness.
  • available() requiring the tap to be running, not merely constructed, is the subtle one, and you got it right — and in the right order: set_tap_running(true) lands before ready_tx.send(Ok(())), so holds_keys at main.rs:385 reports the truth.
  • FlagsChanged never being swallowed, and our own emissions passing on EMITTER_TAG, are both load-bearing; I checked the emitter and every posted event is stamped, including the release_modifiers flags-changed posts.
  • Re-enabling on TapDisabledByTimeout / ByUserInput instead of going deaf is a nice touch.

CI is green on all three OSes and there are no conflicts. Three things I'd like sorted before it goes in, and only one of them is a real decision.

1. The default — this is the one I want your read on

The gate being on by default is the part I'm not ready to merge as-is, and the reason changed under you four days ago. docs/PERMISSIONS.md:164 now says Windows stays opt-in not out of fear, but because of latency: keys are withheld from the application for roughly 75–100 ms after every correction and then arrive together, which reads as the caret lagging behind your typing. Your rationale in the PR description ("inverse default from Windows, which has never run on hardware") answers the old reason for opt-in, not the current one.

And that cost is not Windows-specific — it comes from the engine, not the backend: HELD_FLUSH_QUIET_PROBES = 3 × POST_EMIT_LAG = 25 ms, ceiling HELD_FLUSH = 250 ms (crates/poltertype-core/src/engine/consts.rs:17,70,76). So macOS should be paying the same delay after every correction, and on-by-default makes that everyone's trade rather than the fast typist's.

So: did you feel it? You've been typing on this build on real hardware, which is more than we have for Windows. If the post-correction lag is not perceptible on your machine I want to know that, because it would mean the Windows measurement is not the whole story. If it is perceptible, I'd rather ship it opt-in on macOS too and revisit both platforms together — the mechanism stays exactly as you wrote it, only MacosGate::new()'s default flips (and the doc comment with it). Numbers, even rough ones, decide it.

2. A hole on our side that your PR makes live — worth closing here

Not your bug, but this PR is what makes it reachable. In crates/poltertype-core/src/engine/switcher/correction.rs:568-575, the final sweep after held.release() emits through key_emitter.send_keys(&last) directly:

if !last.is_empty() {
    debug!(count = last.len(), "typing out the last held keystrokes");
    let sent = self.key_emitter.send_keys(&last);

send_keys is the trait default on macOS and Windows — InputError::Unsupported — so those keystrokes are swallowed by the tap and then dropped with nothing but a warn!. This is the same failure that was found and fixed on Windows hardware on 2026-08-04; the fix was the text fallback inside emit_held_keys, and this second call site was missed. One line closes it:

if let Err(e) = self.emit_held_keys(&last, to) {
    warn!(?e, "flushing the last held keystrokes failed");
}

The window is narrow — keys arriving between the last drain and the release — but it opens exactly for the fast typist the gate exists to protect, and it loses characters outright rather than scrambling them. Since you're the only one who can test it on hardware, I'd rather it rode along with this PR.

3. Docs and changelog

The gate landing on macOS makes four places untrue, and none of them are in code:

  • CHANGELOG.md — no ## [Unreleased] entry at all (your other PR has one; this one is the larger change).
  • README.md:28 — the hold-back paragraph covers Linux and Windows only.
  • docs/PERMISSIONS.md (~:173) — "On macOS there is no implementation".
  • docs/PLAN.md:893 — "The key gate is Linux/evdev only; on macOS, as on Windows, a keystroke can still land inside a correction."

Docs tracking code is a release blocker here rather than a nicety, so it has to happen either way — but it's four short edits and you're the one who knows what's now true. If you'd rather not, say so and I'll take them.

Nits — no action needed unless you feel like it

  • TAP_PORT: OnceLock is process-global, and let _ = TAP_PORT.set(...) silently keeps the first value. Unreachable today (listener.start() is called once, main.rs:382), but a second tap would re-enable a stale port. A comment saying "one tap per process, by construction" would save the next reader the trace.
  • TAP_PORT.set() runs after tap.enable(). If the OS disabled the tap inside that gap the re-enable would no-op. It fails toward keys reaching the application, so it's the safe direction — just noting it's there.
  • to_key_event() returning None (unmapped keycode) short-circuits before the gate check, so such a key isn't swallowed and lands mid-correction. Narrow, but it's a hole in the hold.
  • No unit test for MacosGate itself — available() staying false until set_tap_running(true), and the POLTERTYPE_HOLD_KEYS=0 path. They'd run on the macOS CI job we already have.

And two things I looked at and decided are fine, so nobody re-opens them later: the per-keystroke debug! / trace! in the tap callback is not a hazard — the default filter is info and the file writer is tracing_appender::non_blocking, so there's no I/O in the callback, and trace!(scancode = …) already exists on main. Cargo.lock carrying both core-graphics 0.24 (via another dependency) and 0.25 is expected and harmless.

Housekeeping: the branch is one commit behind main, and that commit (d758b51) is what fixed the five plugins::menu failures you saw — they should be gone after a rebase, so a clean cargo test --workspace is now a fair expectation on macOS.

@shohart
shohart marked this pull request as draft August 6, 2026 12:37
shohart added 7 commits August 6, 2026 15:08
The tap moves from listen-only to active (when the gate is on), and
the callback consults the gate on every KeyDown/KeyUp: held keys are
still forwarded to the engine (it replays them behind the correction)
but return NULL to the window server. Our own emissions bypass the
gate via the EMITTER_TAG stamp; FlagsChanged is never swallowed
(a held modifier edge without its counterpart would stick the system
modifier state).

- HoldState moves from windows/ to a shared crate::hold — the swallow
  decision is identical on both platforms and its tests must keep
  running on every host.
- MacosGate is on by default with POLTERTYPE_HOLD_KEYS=0 as the escape
  hatch, and reports available() only while the tap is actually
  running — a gate that claims to hold when nothing is listening
  would make corrections skip compensation and lose text.
- The callback re-enables the tap on kCGEventTapDisabledByTimeout /
  ByUserInput instead of staying deaf.

Validated on macOS 15 (Intel): corrections no longer interleave with
fast typing.
0.24's cg_event_tap_callback_internal maps a callback's None back to
the ORIGINAL event, so an active tap returned the event either way
and the gate 'swallowed' nothing — held keys reached the app directly
*and* were replayed by the flush, doubling them. 0.25's
CallbackResult::Drop returns NULL, which is the actual swallow.
Migrates the callback to the new API (Keep/Drop, mach_port()).
- Default flips to opt-in (POLTERTYPE_HOLD_KEYS=1), same as Windows:
  the flush latency after every correction is an engine-side cost,
  not a backend one, so it is not a trade to make on everybody's
  behalf. Doc comment updated to match.
- correction.rs: the post-release sweep emitted held keystrokes via
  send_keys, which is Unsupported on macOS/Windows — they were
  swallowed from the app and then dropped. Now goes through
  emit_held_keys like the main flush path.
- Docs: CHANGELOG [Unreleased] entry, README hold-back paragraph,
  PERMISSIONS.md macOS gate section, PLAN.md status.
- TAP_PORT: document the one-tap-per-process construction and the
  enable-then-set ordering being the safe direction.
- Unit tests for MacosGate: unavailable until the tap runs, env=0
  path, opt-in default (run on the macOS CI job).
@shohart
shohart force-pushed the macos-key-gate-upstream branch from 0d01861 to 86756d5 Compare August 6, 2026 13:28
@shohart

shohart commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

All three sorted in the force-push (86756d5), rebased onto current main while at it.

1. The default. Flipped to opt-in, same knob and semantics as Windows. On the latency question — honest answer: my validation was scripted (a key poster racing a correction), not perceptual typing, so I can't give you a felt-sense verdict. What the trace shows from the hardware run: the four racer keys were flushed inside the burst window (last flush ~110 ms before correction applied, release immediately after), i.e. the machine paid single-digit-to-tens of ms there — but that's a quiet typist ending, exactly the cheap case; the expensive one is someone who keeps typing through the flush, and I have no measurement for that. Given the engine-side cost you cite (HELD_FLUSH_QUIET_PROBES × POST_EMIT_LAG, ceiling 250 ms) is identical on macOS, the conservative call is the one you made for Windows. MacosGate::new() and the doc comment now read opt-in, and there are unit tests pinning the default, the =0 path, and available() staying false until the tap reports running (they run on the macOS CI job).

2. The final sweep. Closed — emit_held_keys(&last, to) with a comment saying why send_keys can't stay there. Same one-liner shape you suggested; push_echoes is covered inside emit_held_keys.

3. Docs. All four: [Unreleased] changelog entry (gate + the 0.24-trampoline finding + the sweep fix), README hold-back paragraph now covers macOS with a link to #8, PERMISSIONS.md has a proper macOS gate section, PLAN.md re-stamped. Non-blocking note: upstream main moved to v0.12.0 while I was rebasing, so the entry sits as [Unreleased] above the 0.12.0 heading — fold it wherever it belongs when you cut the next one.

Nits: TAP_PORT now documents one-tap-per-process and the enable-then-set ordering being the safe direction. The to_key_event short-circuit I've left as-is deliberately: an untranslatable key swallowed and invisible to the engine would be lost with no replay possible, whereas passing it at worst scrambles — losing felt worse; happy to flip if you read it differently.

Verification on this push: cargo fmt --check clean; the full workspace fmt/clippy/test suite was green on the Mac before the rebase, and the diff since is the default flip, the sweep one-liner, comments, docs and the new gate tests — the Mac went offline mid-run (host unreachable), so I'll re-run the complete suite there and confirm on this thread as soon as it's back.

@shohart
shohart marked this pull request as ready for review August 6, 2026 13:51
@vstrelnikof
vstrelnikof merged commit 1aa9fa7 into Just-Code-NET:main Aug 6, 2026
4 checks passed
vstrelnikof added a commit that referenced this pull request Aug 6, 2026
audio: release the cached output stream when idle — HDMI output blocks
macOS sleep.

Merged here rather than on GitHub: #22 landed first and both PRs opened
a `## [Unreleased]` section, so the changelog conflicted. Both entries
are kept verbatim, split into `### Added — macOS: the key gate (opt-in)`
and `### Fixed — macOS` under the one heading; nothing else in the merge
needed a decision.
vstrelnikof added a commit that referenced this pull request Aug 6, 2026
The three `MacosGate` tests each drive the gate through
`POLTERTYPE_HOLD_KEYS`, a process-global, and the harness runs them on
separate threads. Nothing stopped `default_is_opt_in`'s `remove_var`
from landing between another test's `set_var` and its
`MacosGate::new()` — or its own gate from being built while `=1` was
briefly in force. Either way the wrong gate gets constructed and an
assertion fails on a machine nobody is watching: these are
`cfg(target_os = "macos")`, so the flake would only ever appear on the
macOS CI job, intermittently, in a PR that had nothing to do with it.

A mutex around the env-mutating section fixes it. Poisoning is stepped
over with `into_inner` on purpose: when one test fails while holding
the lock, the others should still report their own verdict instead of
turning one real failure into three panics.

Found while reviewing #22; the gate itself is unchanged.
vstrelnikof added a commit that referenced this pull request Aug 6, 2026
Step 2 of the release, and it caught what step 2 exists to catch: #22
updated the README's hold-back paragraph to cover macOS, then left a
sentence forty lines below saying "the keystroke hold-back remains
Linux-only there, as on Windows." Both were in the same file, and the
true one made the stale one look reviewed.

The same paragraph's other half was stale in the other direction: it
said the 0.7.0 macOS input changes "have still not been run on a Mac by
anyone". Validating the key gate ran the tap and the emitter on Intel
hardware, so that is no longer true — but the held-modifier case those
changes exist for still has nobody's report behind it, and Apple
Silicon none at all, so the warning is narrowed rather than deleted.

PLAN.md gets its `Last updated` stamp moved to v0.13.0; its body was
already updated in #22.
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.

2 participants