From 2f0462047b19a36ed1d6f82e29fb2483ca8e59c3 Mon Sep 17 00:00:00 2001 From: snekxs <26660858+snekxs@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:25:07 -0600 Subject: [PATCH 1/4] fix(hid): open several paths when they share a verified serial 98294bc keyed each HID candidate by path and refused any candidate with more than one path ("refusing multi-path access", plus a matching "refusing report fan-out" guard). ed5bcd9 then made a non-empty serial the grouping identity, so every top-level collection of a device with a serial lands in one candidate again. On Windows each collection is its own path, so "list" failed for every serial-bearing multi-collection mouse. Allow several paths when every interface reports the same non-empty serial, and keep refusing otherwise. The report path already routes to the path that declares the report ID, so its guard now only rejects a device with no open path. Co-Authored-By: Claude Opus 5.5 --- src/hid.rs | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/hid.rs b/src/hid.rs index 7858658..08622d9 100644 --- a/src/hid.rs +++ b/src/hid.rs @@ -915,8 +915,11 @@ impl HidSession { .open_devices .get(key) .ok_or_else(|| "the HID interface is not open".to_owned())?; - if open.paths.len() != 1 { - return Err("HID physical identity is ambiguous; refusing report fan-out".into()); + // An open device has several paths only when `inspect_candidate` verified + // they share one serial; the loop below sends to the path that declares + // the report. + if open.paths.is_empty() { + return Err("the HID interface has no open path".into()); } let _access = crate::drivers::manual_report_access(open.vendor_id) .map_err(|error| error.to_string())?; @@ -1113,6 +1116,14 @@ fn device_group_key(info: &hidapi::DeviceInfo) -> Vec { ) } +/// True when every interface reports the same non-empty serial. +fn shares_verified_serial<'a>(mut serials: impl Iterator>) -> bool { + match serials.next().flatten().filter(|serial| !serial.is_empty()) { + Some(first) => serials.all(|serial| serial == Some(first)), + None => false, + } +} + fn inspect_candidate( api: &HidApi, key: String, @@ -1129,7 +1140,11 @@ fn inspect_candidate( } } - if unique_paths.len() != 1 { + // Several paths are the collections of one device only when they share a + // verified serial; that is the one case `grouped_device_key` merges them. + if unique_paths.len() > 1 + && !shares_verified_serial(infos.iter().map(hidapi::DeviceInfo::serial_number)) + { return Err("HID physical identity is ambiguous; refusing multi-path access".into()); } @@ -1655,6 +1670,22 @@ mod tests { ); } + #[test] + fn several_paths_open_only_under_one_verified_serial() { + assert!(shares_verified_serial( + [Some("receiver-123"), Some("receiver-123")].into_iter() + )); + assert!(!shares_verified_serial( + [Some("receiver-123"), Some("receiver-456")].into_iter() + )); + assert!(!shares_verified_serial( + [Some("receiver-123"), None].into_iter() + )); + assert!(!shares_verified_serial([None, None].into_iter())); + assert!(!shares_verified_serial([Some(""), Some("")].into_iter())); + assert!(!shares_verified_serial(std::iter::empty())); + } + #[test] fn report_payload_never_reads_past_the_buffer() { // A full unnumbered Razer read: hidapi on Windows reports 92 bytes. From 6b8e75ab0d3f824ec0b76f97937a8387c470868e Mon Sep 17 00:00:00 2001 From: snekxs <26660858+snekxs@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:25:07 -0600 Subject: [PATCH 2/4] feat(api): serve icons for saved local applications GET /v1/local-applications (and the PUT response) now gives each entry an `iconId`, computed with applications::icon_id(path). The field is response-only; saved lists are accepted with or without it. GET /v1/applications/{iconId}/icon serves a saved local application's icon whether or not it is running: on a cache miss it is extracted from the saved path, and prune_application_icons keeps saved applications' icons. It still 404s when extraction fails or off Windows. Co-Authored-By: Claude Opus 5.5 --- src/api.rs | 67 +++++++++++++++++++++++++++++++-- src/applications.rs | 6 +-- src/service.rs | 56 ++++++++++++++++++++++++--- tests/backend_local_profiles.rs | 15 ++++++-- 4 files changed, 126 insertions(+), 18 deletions(-) diff --git a/src/api.rs b/src/api.rs index cf70ed8..9b12a8b 100644 --- a/src/api.rs +++ b/src/api.rs @@ -307,8 +307,32 @@ async fn set_custom_game_image( Ok(Json(ApiResult { ok: true })) } -async fn local_applications(State(service): State) -> Json> { - Json(service.local_applications().await) +/// A saved local application as the API serves it, with the id its icon is +/// served under. The id is never read back from a saved list. +#[derive(Debug, Serialize)] +#[serde(rename_all = "camelCase")] +struct LocalApplicationEntry { + #[serde(flatten)] + application: LocalApplication, + icon_id: String, +} + +fn local_application_entries(applications: Vec) -> Vec { + applications + .into_iter() + .map(|application| LocalApplicationEntry { + icon_id: crate::applications::icon_id(&application.path), + application, + }) + .collect() +} + +async fn local_applications( + State(service): State, +) -> Json> { + Json(local_application_entries( + service.local_applications().await, + )) } async fn replace_local_applications( @@ -321,7 +345,7 @@ async fn replace_local_applications( .await .map_err(profile_error)?; Ok(Json( - serde_json::json!({"ok":true,"applications":applications}), + serde_json::json!({"ok":true,"applications":local_application_entries(applications)}), )) } @@ -510,6 +534,43 @@ mod tests { (status, bytes, kind) } + #[tokio::test] + async fn saved_local_applications_carry_their_icon_id() { + let (app, dir) = app(); + let path = r"C:\Apps\Editor.exe"; + let icon_id = crate::applications::icon_id(path); + let entry = serde_json::json!({"id":"local:editor","name":"Editor","executable":"Editor.exe","path":path,"kind":"app"}); + let (status, saved, _) = call( + &app, + Method::PUT, + "/v1/local-applications", + "application/json", + serde_json::to_vec(&serde_json::json!({"applications":[entry]})).unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); + let saved: serde_json::Value = serde_json::from_slice(&saved).unwrap(); + assert_eq!(saved["applications"][0]["iconId"], icon_id.as_str()); + + let (status, listed, _) = + call(&app, Method::GET, "/v1/local-applications", "", vec![]).await; + assert_eq!(status, StatusCode::OK); + let listed: serde_json::Value = serde_json::from_slice(&listed).unwrap(); + assert_eq!(listed[0]["iconId"], icon_id.as_str()); + + // A client may send back the list it was served, icon ids included. + let (status, ..) = call( + &app, + Method::PUT, + "/v1/local-applications", + "application/json", + serde_json::to_vec(&serde_json::json!({"applications":listed})).unwrap(), + ) + .await; + assert_eq!(status, StatusCode::OK); + std::fs::remove_dir_all(dir).expect("cleanup"); + } + #[tokio::test] async fn a_custom_game_keeps_the_image_saved_for_it() { let (app, dir) = app(); diff --git a/src/applications.rs b/src/applications.rs index 848697b..6e476b3 100644 --- a/src/applications.rs +++ b/src/applications.rs @@ -15,11 +15,7 @@ pub struct ApplicationInfo { pub icon_id: String, } -#[cfg_attr( - not(any(target_os = "windows", target_os = "macos", target_os = "linux")), - allow(dead_code) -)] -fn icon_id(path: &str) -> String { +pub fn icon_id(path: &str) -> String { let mut hasher = DefaultHasher::new(); crate::games::path_key(path).hash(&mut hasher); format!("{:016x}", hasher.finish()) diff --git a/src/service.rs b/src/service.rs index 6d356c5..0c3e2e1 100644 --- a/src/service.rs +++ b/src/service.rs @@ -80,14 +80,20 @@ fn profile_applications( /// Drops cached icons for applications that are no longer running, so the /// cache stays the size of the running list instead of every app ever seen. +/// Icons of saved local applications stay, since they are served whether or +/// not the application is running. fn prune_application_icons( icons: &mut HashMap>>, running: &[ApplicationInfo], + local_applications: &[LocalApplication], ) { icons.retain(|icon_id, _| { running .iter() .any(|application| &application.icon_id == icon_id) + || local_applications + .iter() + .any(|target| &applications::icon_id(&target.path) == icon_id) }); } @@ -882,14 +888,32 @@ impl BridgeService { self.inner.read().await.running_applications.clone() } + /// Serves a cached icon, extracting a saved local application's icon from + /// its path the first time it is asked for, even when it is not running. pub async fn application_icon(&self, icon_id: &str) -> Option> { + let path = { + let state = self.inner.read().await; + if let Some(icon) = state.application_icons.get(icon_id) { + return icon.clone(); + } + state + .config + .local_applications + .iter() + .find(|target| applications::icon_id(&target.path) == icon_id)? + .path + .clone() + }; + let icon = tokio::task::spawn_blocking(move || applications::application_icon(&path)) + .await + .ok() + .flatten(); self.inner - .read() + .write() .await .application_icons - .get(icon_id) - .cloned() - .flatten() + .insert(icon_id.to_owned(), icon.clone()); + icon } pub async fn profiles(&self) -> Vec { @@ -1336,7 +1360,12 @@ impl BridgeService { state.processes = processes; state.applications = applications; state.application_icons.extend(icons); - prune_application_icons(&mut state.application_icons, &running); + let inner = &mut *state; + prune_application_icons( + &mut inner.application_icons, + &running, + &inner.config.local_applications, + ); state.running_applications = running; // Detect a debounced active-profile switch, push the new @@ -2562,12 +2591,27 @@ mod tests { ("closed".to_string(), None), ]); - prune_application_icons(&mut icons, &running); + prune_application_icons(&mut icons, &running, &[]); assert_eq!(icons.len(), 1); assert!(icons.contains_key("running")); } + #[test] + fn a_saved_local_application_keeps_its_icon_while_closed() { + let saved: LocalApplication = serde_json::from_value(serde_json::json!({"id":"local:editor","name":"Editor","executable":"Editor.exe","path":"C:\\Apps\\Editor.exe","kind":"app"})).unwrap(); + let saved_icon = applications::icon_id(&saved.path); + let mut icons = HashMap::from([ + (saved_icon.clone(), Some(vec![1])), + ("closed".to_string(), Some(vec![2])), + ]); + + prune_application_icons(&mut icons, &[], &[saved]); + + assert_eq!(icons.len(), 1); + assert!(icons.contains_key(&saved_icon)); + } + #[tokio::test] async fn client_is_connected_only_after_a_heartbeat() { let bridge = service(BridgeConfig::default()); diff --git a/tests/backend_local_profiles.rs b/tests/backend_local_profiles.rs index 396956b..fc00a26 100644 --- a/tests/backend_local_profiles.rs +++ b/tests/backend_local_profiles.rs @@ -55,6 +55,13 @@ async fn server_with_config(storage_failure: bool, config: BridgeConfig) -> Test } } +/// A saved target as the local-applications endpoints serve it. +fn served(mut value: serde_json::Value) -> serde_json::Value { + let icon_id = openmouse_bridge::applications::icon_id(value["path"].as_str().unwrap()); + value["iconId"] = icon_id.into(); + value +} + fn target() -> serde_json::Value { serde_json::json!({"id":"local:editor","name":"Editor","executable":"editor.exe","path":"C:/Tools/editor.exe","kind":"app"}) } @@ -74,7 +81,7 @@ async fn activation_metadata_roundtrips_into_canonical_profiles() { assert_eq!(response.status(), 200); assert_eq!( response.json::().await.unwrap(), - serde_json::json!({"ok":true,"applications":[value.clone()]}) + serde_json::json!({"ok":true,"applications":[served(value.clone())]}) ); let stored = server.service.local_applications().await; assert_eq!(serde_json::to_value(&stored[0]).unwrap(), value); @@ -163,7 +170,7 @@ async fn saved_missing_canonical_bundle_survives_batch_crud_and_profile_readback .unwrap(); assert_eq!(response.status(), 200); let readback: serde_json::Value = response.json().await.unwrap(); - assert_eq!(readback["applications"][0], bundle); + assert_eq!(readback["applications"][0], served(bundle.clone())); let profiles = server.service.profiles().await; assert_eq!(profiles.len(), 1); assert_eq!( @@ -201,7 +208,7 @@ async fn local_http_roundtrip_reconciles_id_linked_profiles_and_restarts() { assert_eq!(response.status(), 200); assert_eq!( response.json::().await.unwrap(), - serde_json::json!({"ok":true,"applications":[target()]}) + serde_json::json!({"ok":true,"applications":[served(target())]}) ); let local = serde_json::json!({"application":{"localId":"local:editor","name":"Wrong","executable":"wrong.exe","path":""},"device":{"id":"d","name":"Mouse"},"settings":{"dpi":1600},"enabled":false}); let preset = serde_json::json!({"application":{"name":"Editor","executable":"editor.exe","path":""},"device":{"id":"d","name":"Mouse"},"settings":{"dpi":800}}); @@ -303,7 +310,7 @@ async fn local_http_validation_is_400_and_never_changes_state() { assert_eq!(std::fs::read(&server.path).unwrap(), original); let targets: serde_json::Value = client.get(&url).send().await.unwrap().json().await.unwrap(); - assert_eq!(targets, serde_json::json!([target()])); + assert_eq!(targets, serde_json::json!([served(target())])); } assert_eq!( client From eb346ff292fb406ba4df7599ab2358d8bccfab1d Mon Sep 17 00:00:00 2001 From: snekxs <26660858+snekxs@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:42:28 -0600 Subject: [PATCH 3/4] fix(hid): send a report to one path and stop readers when a spawn fails Review follow-up on the multi-path fix. A device opened with several paths now sends each report to exactly one of them: the paths that declare the report (or every path, for Razer, whose descriptors are not parsed) are tried in order and the first that takes it is the only one written, so one command never reaches the device twice. start_reader stores the stop flag before spawning, and a later spawn failure stops and joins the readers already running instead of leaving them to block session cleanup. Co-Authored-By: Claude Opus 5.5 --- src/hid.rs | 96 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 60 insertions(+), 36 deletions(-) diff --git a/src/hid.rs b/src/hid.rs index 08622d9..0e01322 100644 --- a/src/hid.rs +++ b/src/hid.rs @@ -843,6 +843,9 @@ impl HidSession { .get_mut(key) .ok_or_else(|| "the HID interface must be open before listening".to_owned())?; let stop = Arc::new(AtomicBool::new(false)); + // Stored before any reader starts, so a later spawn failure can stop and + // join the readers already running instead of leaving them to block cleanup. + open.stop = Some(Arc::clone(&stop)); for (index, (device, buffer_len, uses_report_ids)) in reader_specs.into_iter().enumerate() { let thread_stop = Arc::clone(&stop); let tx = self.input_tx.clone(); @@ -884,12 +887,16 @@ impl HidSession { } } } - }) - .map_err(|error| format!("could not start the HID input reader: {error}"))?; - open.readers.push(reader); + }); + match reader { + Ok(reader) => open.readers.push(reader), + Err(error) => { + open.stop_reader(); + return Err(format!("could not start the HID input reader: {error}")); + } + } } open.listening = true; - open.stop = Some(stop); Ok(()) } @@ -916,8 +923,7 @@ impl HidSession { .get(key) .ok_or_else(|| "the HID interface is not open".to_owned())?; // An open device has several paths only when `inspect_candidate` verified - // they share one serial; the loop below sends to the path that declares - // the report. + // they share one serial. A report still goes to exactly one of them. if open.paths.is_empty() { return Err("the HID interface has no open path".into()); } @@ -926,27 +932,10 @@ impl HidSession { let mut frame = Vec::with_capacity(data.len() + 1); frame.push(report_id); frame.extend_from_slice(&data); - let mut sent = false; let mut last_error = None; - let declared_path_exists = open.paths.iter().any(|path| { - let reports = if feature { - &path.layout.feature_bits - } else { - &path.layout.output_bits - }; - reports.contains_key(&report_id) - }); - for path in &open.paths { - if declared_path_exists { - let reports = if feature { - &path.layout.feature_bits - } else { - &path.layout.output_bits - }; - if !reports.contains_key(&report_id) { - continue; - } - } + let layouts: Vec<&ReportLayout> = open.paths.iter().map(|path| &path.layout).collect(); + for index in report_paths(&layouts, report_id, feature) { + let path = &open.paths[index]; let result = path .device .lock() @@ -959,20 +948,18 @@ impl HidSession { } .map_err(|error| error.to_string()) }); + // The first path that takes the report is the only one written, so a + // command never reaches the device twice. match result { - Ok(()) => sent = true, + Ok(()) => return Ok(()), Err(error) => last_error = Some(error), } } - if sent { - Ok(()) - } else { - let operation = if feature { "feature" } else { "output" }; - Err(format!( - "could not send {operation} report {report_id}: {}", - last_error.unwrap_or_else(|| "no HID path was open".to_owned()) - )) - } + let operation = if feature { "feature" } else { "output" }; + Err(format!( + "could not send {operation} report {report_id}: {}", + last_error.unwrap_or_else(|| "no HID path was open".to_owned()) + )) } fn receive_input_report(&self, key: &str, report_id: u8) -> Result, String> { @@ -1116,6 +1103,27 @@ fn device_group_key(info: &hidapi::DeviceInfo) -> Vec { ) } +/// The paths to try for a report, in order: those whose descriptor declares it, +/// or every path when none does (Razer descriptors are not parsed). The sender +/// stops at the first path that takes the report. +fn report_paths(layouts: &[&ReportLayout], report_id: u8, feature: bool) -> Vec { + let declares = |layout: &ReportLayout| { + if feature { + layout.feature_bits.contains_key(&report_id) + } else { + layout.output_bits.contains_key(&report_id) + } + }; + let declared: Vec = (0..layouts.len()) + .filter(|&index| declares(layouts[index])) + .collect(); + if declared.is_empty() { + (0..layouts.len()).collect() + } else { + declared + } +} + /// True when every interface reports the same non-empty serial. fn shares_verified_serial<'a>(mut serials: impl Iterator>) -> bool { match serials.next().flatten().filter(|serial| !serial.is_empty()) { @@ -1686,6 +1694,22 @@ mod tests { assert!(!shares_verified_serial(std::iter::empty())); } + #[test] + fn a_report_goes_to_the_paths_that_declare_it_first() { + let mut vendor = ReportLayout::default(); + vendor.feature_bits.insert(0x11, 160); + vendor.output_bits.insert(0x10, 48); + let mouse = ReportLayout::default(); + let mut second = ReportLayout::default(); + second.feature_bits.insert(0x11, 160); + let layouts = [&mouse, &vendor, &second]; + assert_eq!(report_paths(&layouts, 0x11, true), vec![1, 2]); + assert_eq!(report_paths(&layouts, 0x10, false), vec![1]); + // Undeclared (a Razer path without parsed descriptors): every path is a candidate, tried in order. + assert_eq!(report_paths(&layouts, 0x00, true), vec![0, 1, 2]); + assert!(report_paths(&[], 0x11, true).is_empty()); + } + #[test] fn report_payload_never_reads_past_the_buffer() { // A full unnumbered Razer read: hidapi on Windows reports 92 bytes. From 3d103f222eec2e46e58598016942c817542d69ff Mon Sep 17 00:00:00 2001 From: snekxs <26660858+snekxs@users.noreply.github.com> Date: Mon, 26 Oct 2026 14:42:28 -0600 Subject: [PATCH 4/4] fix(api): drop removed apps' icons and do not cache failed extractions Replacing the saved local applications prunes the icon cache, so a removed app's icon is no longer served. A failed extraction is not cached, so a path that comes back later gets its icon, and an icon read while the list changed is cached only if a saved app still owns its id. Co-Authored-By: Claude Opus 5.5 --- src/service.rs | 96 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 89 insertions(+), 7 deletions(-) diff --git a/src/service.rs b/src/service.rs index 0c3e2e1..d3718eb 100644 --- a/src/service.rs +++ b/src/service.rs @@ -893,8 +893,8 @@ impl BridgeService { pub async fn application_icon(&self, icon_id: &str) -> Option> { let path = { let state = self.inner.read().await; - if let Some(icon) = state.application_icons.get(icon_id) { - return icon.clone(); + if let Some(Some(icon)) = state.application_icons.get(icon_id) { + return Some(icon.clone()); } state .config @@ -908,11 +908,22 @@ impl BridgeService { .await .ok() .flatten(); - self.inner - .write() - .await - .application_icons - .insert(icon_id.to_owned(), icon.clone()); + // A failed extraction is not cached, so a path that comes back later gets + // its icon. The saved list may have changed while the icon was read, so it + // is cached only while a saved application still owns this id. + if let Some(icon) = &icon { + let mut state = self.inner.write().await; + let owned = state + .config + .local_applications + .iter() + .any(|target| applications::icon_id(&target.path) == icon_id); + if owned { + state + .application_icons + .insert(icon_id.to_owned(), Some(icon.clone())); + } + } icon } @@ -1041,6 +1052,14 @@ impl BridgeService { Ok(()) }) .await?; + // A removed application's icon is not served any more. + let mut guard = self.inner.write().await; + let state = &mut *guard; + prune_application_icons( + &mut state.application_icons, + &state.running_applications, + &state.config.local_applications, + ); Ok(canonical) } @@ -2612,6 +2631,69 @@ mod tests { assert!(icons.contains_key(&saved_icon)); } + fn saved_editor() -> LocalApplication { + serde_json::from_value(serde_json::json!({"id":"local:editor","name":"Editor","executable":"Editor.exe","path":"C:\\Missing\\Editor.exe","kind":"app"})).unwrap() + } + + #[tokio::test] + async fn removing_a_saved_application_drops_its_cached_icon() { + let saved = saved_editor(); + let icon_id = applications::icon_id(&saved.path); + let path = + std::env::temp_dir().join(format!("openmouse-icon-test-{}.json", std::process::id())); + let bridge = BridgeService::new( + BridgeConfig { + local_applications: vec![saved], + ..BridgeConfig::default() + }, + path.clone(), + ); + bridge + .inner + .write() + .await + .application_icons + .insert(icon_id.clone(), Some(vec![1])); + assert_eq!(bridge.application_icon(&icon_id).await, Some(vec![1])); + + bridge.replace_local_applications(Vec::new()).await.unwrap(); + let _ = std::fs::remove_file(&path); + + assert!( + !bridge + .inner + .read() + .await + .application_icons + .contains_key(&icon_id) + ); + assert_eq!(bridge.application_icon(&icon_id).await, None); + } + + #[tokio::test] + async fn a_failed_icon_extraction_is_not_cached() { + let saved = saved_editor(); + let icon_id = applications::icon_id(&saved.path); + let bridge = service(BridgeConfig { + local_applications: vec![saved], + ..BridgeConfig::default() + }); + + assert_eq!( + bridge.application_icon(&icon_id).await, + None, + "the path does not exist" + ); + assert!( + !bridge + .inner + .read() + .await + .application_icons + .contains_key(&icon_id) + ); + } + #[tokio::test] async fn client_is_connected_only_after_a_heartbeat() { let bridge = service(BridgeConfig::default());