fix(rb-12): hotkey device filter consults configured HotkeyCombo
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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<Key>>,
|
||||
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::<Key>::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::<Key>::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::<Key>::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::<Key>::new();
|
||||
keys.insert(Key::KEY_R);
|
||||
assert!(!device_supports_combo(Some(&keys), &combo_for(KEY_D)));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user