From 528facfab04b33b60f336b89f9cab131503e80d4 Mon Sep 17 00:00:00 2001 From: Jake Date: Wed, 22 Apr 2026 10:18:54 +0100 Subject: [PATCH] fix(rb-12): hotkey device filter consults configured HotkeyCombo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit try_attach_device was rejecting any device that did not report KEY_A or KEY_R — a leftover heuristic from the whisper-overlay seed. A user whose binding was anything else (Ctrl+Shift+D is a common default) would see no hotkey events from that device even though it supports the key. Replace the hard-coded check with device_supports_combo(supported, combo), a pure helper that reads the configured trigger key code from the HotkeyCombo snapshot. Snapshot is taken from hotkey_rx.borrow() before opening the device; an unconfigured or shutting-down listener short-circuits to a non-attach. Four regression tests in linux::tests cover: supported+D → attach, unsupported → reject, no reported keys → reject, and the explicit non-A/non-R case that demonstrates the bug. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/hotkey/src/linux.rs | 82 ++++++++++++++++++++--- docs/issues/README.md | 9 ++- docs/issues/hotkey-linux-device-filter.md | 18 +++++ 3 files changed, 98 insertions(+), 11 deletions(-) diff --git a/crates/hotkey/src/linux.rs b/crates/hotkey/src/linux.rs index 3a3c79b..cc133f1 100644 --- a/crates/hotkey/src/linux.rs +++ b/crates/hotkey/src/linux.rs @@ -13,7 +13,7 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; use std::sync::Arc; -use evdev::{Device, InputEventKind, Key}; +use evdev::{AttributeSetRef, Device, InputEventKind, Key}; use notify::{recommended_watcher, EventKind, RecursiveMode, Watcher}; use tokio::sync::{mpsc, watch, Mutex}; @@ -225,6 +225,11 @@ async fn try_attach_device( return true; } + let Some(combo) = hotkey_rx.borrow().clone() else { + // Listener is unconfigured or shutting down. + return false; + }; + let device = match Device::open(path) { Ok(d) => d, Err(e) => { @@ -233,14 +238,7 @@ async fn try_attach_device( } }; - // Check if this device has the keys we need - let supported = device.supported_keys(); - let has_keys = supported.map_or(false, |keys| { - // Must support at least some keyboard keys - keys.contains(Key::KEY_A) || keys.contains(Key::KEY_R) - }); - - if !has_keys { + if !device_supports_combo(device.supported_keys(), &combo) { return false; } @@ -347,3 +345,69 @@ fn is_event_device(path: &Path) -> bool { .and_then(|n| n.to_str()) .map_or(false, |n| n.starts_with("event")) } + +/// Return true when the device's reported key set includes the combo's +/// configured trigger key. A device that reports no keys at all (for +/// example a mouse whose `EV_KEY` capability is buttons only) is rejected. +fn device_supports_combo( + supported: Option<&AttributeSetRef>, + combo: &HotkeyCombo, +) -> bool { + supported.map_or(false, |keys| keys.contains(Key::new(combo.key_code))) +} + +#[cfg(test)] +mod tests { + use super::*; + use evdev::AttributeSet; + + fn combo_for(key_code: u16) -> HotkeyCombo { + HotkeyCombo { + ctrl: false, + shift: false, + alt: false, + super_key: false, + key_code, + label: "test".to_string(), + } + } + + const KEY_D: u16 = 32; + + #[test] + fn attaches_when_device_supports_configured_trigger() { + let mut keys = AttributeSet::::new(); + keys.insert(Key::KEY_D); + assert!(device_supports_combo(Some(&keys), &combo_for(KEY_D))); + } + + #[test] + fn rejects_when_device_lacks_configured_trigger() { + let mut keys = AttributeSet::::new(); + keys.insert(Key::KEY_A); + assert!(!device_supports_combo(Some(&keys), &combo_for(KEY_D))); + } + + #[test] + fn rejects_when_device_reports_no_keys() { + assert!(!device_supports_combo(None, &combo_for(KEY_D))); + } + + // Regression for RB-12: the original filter hard-coded KEY_A || KEY_R + // and would drop a keyboard bound to any other trigger — for example + // a user's Ctrl+Shift+D binding on a keyboard that (hypothetically) + // reports only KEY_D — even though the device clearly supports it. + #[test] + fn attaches_for_non_a_non_r_trigger() { + let mut keys = AttributeSet::::new(); + keys.insert(Key::KEY_D); + assert!(device_supports_combo(Some(&keys), &combo_for(KEY_D))); + + // And conversely, a device that only supports KEY_R is correctly + // rejected when the binding is KEY_D — the old implementation + // would have incorrectly attached. + let mut keys = AttributeSet::::new(); + keys.insert(Key::KEY_R); + assert!(!device_supports_combo(Some(&keys), &combo_for(KEY_D))); + } +} diff --git a/docs/issues/README.md b/docs/issues/README.md index 16532e4..a54a3b4 100644 --- a/docs/issues/README.md +++ b/docs/issues/README.md @@ -19,7 +19,7 @@ should be mirrored as real GitHub issues on `jakejars/kon`. | RB-02 | [c3-migrations-atomicity.md](c3-migrations-atomicity.md) | `crates/storage/migrations.rs` | medium | | RB-03 | [c4-transcript-profile-fk.md](c4-transcript-profile-fk.md) | `crates/storage/migrations.rs` + `database.rs` | large | -## MAJOR (9) +## MAJOR (8 open, 1 resolved) | # | File | Area | Fix scope | |---|---|---|---| @@ -31,7 +31,12 @@ should be mirrored as real GitHub issues on `jakejars/kon`. | RB-09 | [decoder-partial-audio-on-error.md](decoder-partial-audio-on-error.md) | `crates/audio/decode.rs` | medium | | RB-10 | [llm-prompt-preflight.md](llm-prompt-preflight.md) | `crates/llm/lib.rs` | medium | | RB-11 | [keystore-thread-safety.md](keystore-thread-safety.md) | `crates/cloud-providers/keystore.rs` | medium | -| RB-12 | [hotkey-linux-device-filter.md](hotkey-linux-device-filter.md) | `crates/hotkey/linux.rs` | small | + +## Resolved + +| # | File | Area | Resolution | +|---|---|---|---| +| RB-12 | [hotkey-linux-device-filter.md](hotkey-linux-device-filter.md) | `crates/hotkey/linux.rs` | Extracted `device_supports_combo` helper; `try_attach_device` now reads the configured `HotkeyCombo` from the watch channel and checks support for that trigger key. Four regression tests land in `linux::tests`. | ## Dependencies diff --git a/docs/issues/hotkey-linux-device-filter.md b/docs/issues/hotkey-linux-device-filter.md index 0168d85..cd09024 100644 --- a/docs/issues/hotkey-linux-device-filter.md +++ b/docs/issues/hotkey-linux-device-filter.md @@ -4,6 +4,24 @@ **Path:** `crates/hotkey/src/linux.rs:236-241` **Source:** [2026-04-22 code review](../code-review-2026-04-22.md) **Labels:** release-blocker, major, hotkey, correctness +**Status:** RESOLVED (2026-04-22) + +## Resolution + +Extracted `device_supports_combo(supported, combo) -> bool` as a pure helper. +`try_attach_device` now snapshots the current `HotkeyCombo` from `hotkey_rx` +(returning early with `false` if the listener is unconfigured) and uses the +helper to filter devices by the configured trigger key. + +Tests in `crates/hotkey/src/linux.rs` (`linux::tests`): + +- `attaches_when_device_supports_configured_trigger` +- `rejects_when_device_lacks_configured_trigger` +- `rejects_when_device_reports_no_keys` +- `attaches_for_non_a_non_r_trigger` (direct regression) + +Manual verification of the Ctrl+Shift+D binding in Settings remains on the +ship-gate checklist — code path is correct; runtime GUI check is deferred. ## Problem