fix(gui): reject transient direct device identities

crates/openlogi-agent-core/src/device_order.rs

@@ -9,10 +9,17 @@
 
 use openlogi_hid::DeviceRoute;
 
-/// A stable, route-derived identity used to order devices deterministically.
-/// Distinct devices never share one (a Bolt receiver UID + slot, a direct
-/// vendor/product + serial/unit, or a slot + serial/unit are each unique), so
-/// it alone fixes the sort order regardless of secondary tiebreakers.
+/// A configuration key backed by enough information to identify one physical
+/// device across inventory snapshots and process restarts.
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct PhysicalDeviceKey(String);
+
+/// A route-derived identity used to order devices deterministically.
+///
+/// Receiver UID + slot and direct serial/non-zero unit identities are stable
+/// and unique. A direct all-zero unit identity is deliberately retained here
+/// only so a transient inventory record can still be ordered; it cannot become
+/// a [`PhysicalDeviceKey`].
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub enum DeviceStableId {
     Bolt {
@@ -41,9 +48,11 @@ impl DeviceIdentity {
     /// Prefer the serial number (case-folded) when present, else the unit id.
     #[must_use]
     pub fn from_parts(serial: Option<&str>, unit_id: [u8; 4]) -> Self {
-        serial.map_or(Self::Unit(unit_id), |s| {
-            Self::Serial(s.to_ascii_lowercase())
-        })
+        serial
+            .filter(|serial| !serial.is_empty())
+            .map_or(Self::Unit(unit_id), |serial| {
+                Self::Serial(serial.to_ascii_lowercase())
+            })
     }
 }
 
@@ -81,13 +90,18 @@ impl DeviceStableId {
         }
     }
 
-    /// Stable key for persisted per-physical-device configuration.
+    /// Route-derived key used while a device is present in a runtime snapshot.
+    ///
+    /// Unlike [`Self::physical_key`], this also represents a direct or
+    /// routeless device whose only reported identity is an all-zero unit id.
+    /// Such a key is suitable for short-lived UI bookkeeping only and must not
+    /// be written to configuration.
     ///
     /// This intentionally keys receiver-connected devices by receiver UID +
     /// pairing slot rather than model id, so two identical mice paired to the
     /// same receiver can carry different settings.
     #[must_use]
-    pub fn config_key(&self) -> String {
+    pub fn runtime_key(&self) -> String {
         match self {
             Self::Bolt { receiver_uid, slot } => format!("receiver:{receiver_uid}:slot:{slot}"),
             Self::Direct {
@@ -98,9 +112,32 @@ impl DeviceStableId {
             Self::Unknown { slot, identity } => format!("unknown:slot:{slot}:{}", identity.key()),
         }
     }
+
+    /// Stable key for persisted per-physical-device configuration.
+    ///
+    /// Receiver-connected devices are identified by receiver UID + pairing
+    /// slot. Direct and routeless devices require either a non-empty serial
+    /// number or a non-zero unit id; an all-zero unit id is a transient probe
+    /// result, not a physical identity.
+    #[must_use]
+    pub fn physical_key(&self) -> Option<PhysicalDeviceKey> {
+        match self {
+            Self::Bolt { .. } => Some(PhysicalDeviceKey(self.runtime_key())),
+            Self::Direct { identity, .. } | Self::Unknown { identity, .. } => identity
+                .is_physical()
+                .then(|| PhysicalDeviceKey(self.runtime_key())),
+        }
+    }
 }
 
 impl DeviceIdentity {
+    fn is_physical(&self) -> bool {
+        match self {
+            Self::Serial(serial) => !serial.is_empty(),
+            Self::Unit(unit) => *unit != [0; 4],
+        }
+    }
+
     fn key(&self) -> String {
         match self {
             Self::Serial(serial) => format!("serial:{serial}"),
@@ -109,6 +146,81 @@ impl DeviceIdentity {
     }
 }
 
+impl PhysicalDeviceKey {
+    /// Parse a key emitted by [`DeviceStableId::physical_key`].
+    ///
+    /// Legacy model-scoped configuration keys intentionally return `None`;
+    /// callers can use that distinction when applying compatibility behavior
+    /// without treating a model identifier as a physical identity.
+    #[must_use]
+    pub fn parse(value: &str) -> Option<Self> {
+        if receiver_key_is_valid(value)
+            || direct_identity_fragment(value).is_some_and(identity_fragment_is_physical)
+            || unknown_identity_fragment(value).is_some_and(identity_fragment_is_physical)
+        {
+            Some(Self(value.to_string()))
+        } else {
+            None
+        }
+    }
+
+    /// Whether `value` is a structurally valid runtime key whose only device
+    /// identity is the all-zero unit id.
+    #[must_use]
+    pub fn is_transient(value: &str) -> bool {
+        direct_identity_fragment(value)
+            .or_else(|| unknown_identity_fragment(value))
+            .is_some_and(|identity| identity == "unit:00000000")
+    }
+
+    /// Borrow the serialized configuration key.
+    #[must_use]
+    pub fn as_str(&self) -> &str {
+        &self.0
+    }
+
+    /// Consume the wrapper and return its serialized configuration key.
+    #[must_use]
+    pub fn into_string(self) -> String {
+        self.0
+    }
+}
+
+fn receiver_key_is_valid(value: &str) -> bool {
+    value
+        .strip_prefix("receiver:")
+        .and_then(|rest| rest.rsplit_once(":slot:"))
+        .is_some_and(|(receiver_uid, slot)| !receiver_uid.is_empty() && slot.parse::<u8>().is_ok())
+}
+
+fn direct_identity_fragment(value: &str) -> Option<&str> {
+    let mut parts = value.strip_prefix("direct:")?.splitn(3, ':');
+    let vendor_id = parts.next()?;
+    let product_id = parts.next()?;
+    let identity = parts.next()?;
+    (is_hex_word(vendor_id) && is_hex_word(product_id)).then_some(identity)
+}
+
+fn unknown_identity_fragment(value: &str) -> Option<&str> {
+    let (slot, identity) = value.strip_prefix("unknown:slot:")?.split_once(':')?;
+    slot.parse::<u8>().ok().map(|_| identity)
+}
+
+fn identity_fragment_is_physical(value: &str) -> bool {
+    value
+        .strip_prefix("serial:")
+        .is_some_and(|serial| !serial.is_empty())
+        || value.strip_prefix("unit:").is_some_and(|unit| {
+            unit.len() == 8
+                && unit.bytes().all(|byte| byte.is_ascii_hexdigit())
+                && unit != "00000000"
+        })
+}
+
+fn is_hex_word(value: &str) -> bool {
+    value.len() == 4 && value.bytes().all(|byte| byte.is_ascii_hexdigit())
+}
+
 fn hex_unit(unit: [u8; 4]) -> String {
     format!(
         "{:02x}{:02x}{:02x}{:02x}",
@@ -120,7 +232,7 @@ fn hex_unit(unit: [u8; 4]) -> String {
 mod tests {
     use openlogi_hid::DeviceRoute;
 
-    use super::DeviceStableId;
+    use super::{DeviceStableId, PhysicalDeviceKey};
 
     #[test]
     fn unifying_route_maps_to_bolt_stable_id() {
@@ -162,8 +274,57 @@ mod tests {
         };
 
         assert_eq!(
-            DeviceStableId::from_parts(Some(&route), 2, Some("SERIAL"), [1, 2, 3, 4]).config_key(),
-            "receiver:aabb:slot:2"
+            DeviceStableId::from_parts(Some(&route), 2, Some("SERIAL"), [1, 2, 3, 4])
+                .physical_key()
+                .map(PhysicalDeviceKey::into_string),
+            Some("receiver:aabb:slot:2".to_string())
         );
     }
+
+    #[test]
+    fn zero_unit_direct_identity_is_transient() {
+        let route = DeviceRoute::Direct {
+            vendor_id: 0x046d,
+            product_id: 0xb023,
+        };
+        let id = DeviceStableId::from_parts(Some(&route), 0xff, None, [0; 4]);
+
+        assert!(id.physical_key().is_none());
+        assert_eq!(id.runtime_key(), "direct:046d:b023:unit:00000000");
+        assert!(PhysicalDeviceKey::is_transient(&id.runtime_key()));
+        assert!(PhysicalDeviceKey::parse(&id.runtime_key()).is_none());
+    }
+
+    #[test]
+    fn serial_identity_is_physical_when_unit_is_zero() {
+        let route = DeviceRoute::Direct {
+            vendor_id: 0x046d,
+            product_id: 0xb023,
+        };
+        let key = DeviceStableId::from_parts(Some(&route), 0xff, Some("ABCDEF"), [0; 4])
+            .physical_key()
+            .map(PhysicalDeviceKey::into_string);
+
+        assert_eq!(key, Some("direct:046d:b023:serial:abcdef".to_string()));
+    }
+
+    #[test]
+    fn nonzero_unit_identity_is_physical_without_serial() {
+        let route = DeviceRoute::Direct {
+            vendor_id: 0x046d,
+            product_id: 0xb023,
+        };
+        let key = DeviceStableId::from_parts(Some(&route), 0xff, None, [0xa3, 0x93, 0xca, 0xe0])
+            .physical_key()
+            .map(PhysicalDeviceKey::into_string);
+
+        assert_eq!(key, Some("direct:046d:b023:unit:a393cae0".to_string()));
+    }
+
+    #[test]
+    fn parser_distinguishes_physical_keys_from_legacy_model_keys() {
+        assert!(PhysicalDeviceKey::parse("receiver:d0289db2:slot:1").is_some());
+        assert!(PhysicalDeviceKey::parse("direct:046d:b023:unit:a393cae0").is_some());
+        assert!(PhysicalDeviceKey::parse("2b034").is_none());
+    }
 }

crates/openlogi-agent-core/src/orchestrator.rs

@@ -401,8 +401,11 @@ fn build_devices(inventories: &[DeviceInventory]) -> Vec<AgentDevice> {
                 model.serial_number.as_deref(),
                 model.unit_id,
             );
+            let Some(config_key) = stable_id.physical_key() else {
+                continue;
+            };
             devices.push(AgentDevice {
-                config_key: stable_id.config_key(),
+                config_key: config_key.into_string(),
                 model_key: model.config_key(),
                 route,
                 slot: paired.slot,
@@ -518,12 +521,15 @@ fn write_value<T>(lock: &RwLock<T>, value: T, name: &str) {
 #[cfg(test)]
 mod tests {
     use super::{
-        AgentDevice, InventoryHealth, Orchestrator, configured_wheel_mode, plan_reapply,
-        reapply_targets,
+        AgentDevice, InventoryHealth, Orchestrator, build_devices, configured_wheel_mode,
+        plan_reapply, reapply_targets,
     };
     use openlogi_core::config::{Config, ScrollResolution};
-    use openlogi_core::device::Capabilities;
-    use openlogi_hid::DeviceRoute;
+    use openlogi_core::device::{
+        Capabilities, DeviceInventory, DeviceKind, DeviceModelInfo, DeviceTransports, PairedDevice,
+        ReceiverInfo,
+    };
+    use openlogi_hid::{DIRECT_DEVICE_INDEX, DeviceRoute};
 
     fn dev(key: &str, slot: u8, online: bool) -> AgentDevice {
         AgentDevice {
@@ -541,6 +547,43 @@ mod tests {
         }
     }
 
+    fn direct_inventory(serial_number: Option<&str>, unit_id: [u8; 4]) -> DeviceInventory {
+        DeviceInventory {
+            receiver: ReceiverInfo {
+                name: "MX Master 3S".to_string(),
+                vendor_id: 0x046d,
+                product_id: 0xb023,
+                unique_id: None,
+            },
+            paired: vec![PairedDevice {
+                slot: DIRECT_DEVICE_INDEX,
+                codename: Some("MX Master 3S".to_string()),
+                wpid: None,
+                kind: DeviceKind::Mouse,
+                online: true,
+                battery: None,
+                model_info: Some(DeviceModelInfo {
+                    entity_count: 1,
+                    serial_number: serial_number.map(str::to_string),
+                    unit_id,
+                    transports: DeviceTransports::default(),
+                    model_ids: [0xb034, 0, 0],
+                    extended_model_id: 2,
+                }),
+                capabilities: Some(Capabilities::presumed_from_kind(DeviceKind::Mouse)),
+            }],
+        }
+    }
+
+    #[test]
+    fn build_devices_skips_transient_zero_unit_direct_identity() {
+        assert!(build_devices(&[direct_inventory(None, [0; 4])]).is_empty());
+
+        let devices = build_devices(&[direct_inventory(Some("ABC123"), [0; 4])]);
+        assert_eq!(devices.len(), 1);
+        assert_eq!(devices[0].config_key, "direct:046d:b023:serial:abc123");
+    }
+
     #[test]
     fn configured_wheel_mode_gates_resolution_and_inversion_independently() {
         let mut config = Config::default();

crates/openlogi-gui/src/app.rs

@@ -554,6 +554,7 @@ mod tests {
     fn record(kind: DeviceKind, capabilities: Option<Capabilities>) -> DeviceRecord {
         DeviceRecord {
             config_key: "test".to_string(),
+            persistent: true,
             model_key: "test".to_string(),
             display_name: "Test".to_string(),
             asset: None,