fix: make one-shot enumerate() retry transport-agnostic so Unifying partial drains recover

[mvanhorn]

Summary

The one-shot enumerate() free fn (used by the CLI list / diag paths) retries through a transient probe miss, but its early-exit never tripped for a Unifying receiver, so a late-arriving device was silently dropped and the CLI returned a short device set.

The retry loop stopped on all_healthy, the AND of each probed node's probe.healthy. The two receiver kinds define healthy differently: a Bolt receiver sets healthy = paired.len() == pairing_count (a missing device makes it false and the retry fires), while a Unifying receiver sets healthy = pairing_count.is_some() because offline Unifying devices aren't enumerable yet. A Unifying device whose arrival event lands just after the ~1.5s drain window is dropped, yet pairing_count.is_some() stays true, so all_healthy stays true and the retry never fires. The continuous GUI/agent watcher is unaffected because it re-enumerates every ~2s.

Fixes #277.

Why this matters

openlogi list and openlogi diag are the CLI's primary read paths, and a one-shot caller builds a fresh Enumerator with an empty ledger, so it has no prior snapshot to replay a transient miss against. The retry loop was added precisely to recover from that, but for a Unifying receiver it was a no-op: a device whose arrival event lands after the drain window is dropped while pairing_count.is_some() keeps all_healthy true, so the loop returns a short device set on the first pass. Users on Unifying hardware intermittently see fewer devices than are actually paired, with no recovery, while Bolt users (whose healthy rides on the count) are unaffected.

Approach

Make the one-shot early-exit transport-agnostic by separating the retry signal from per-node ledger health:

• Add a complete flag to NodeProbe, reported alongside the existing healthy. For Unifying, complete requires paired.len() == pairing_count, so a partial drain now reports complete = false and the retry fires. healthy (which drives the ledger's per-node replay) is unchanged, so the watcher and the #218 replay behavior are preserved.
• The retry loop stops on all_complete, on an unchanged inventory between two consecutive healthy passes (the expected stable Unifying offline-drain case, so a chronically-short set stabilizes instead of burning every attempt), or on the attempt cap. The unchanged-inventory shortcut is gated on all_healthy and the previous snapshot is only retained from a healthy pass, so a failed/timed-out/open-failed probe keeps using the full retry budget to reopen its channel and recover.
• Add PartialEq / Eq derives to the inventory device structs (BatteryInfo, ReceiverInfo, DeviceModelInfo, DeviceTransports, PairedDevice, DeviceInventory) so successive attempts can be diffed. These are additive derives only; field order is untouched, so the bincode wire format is unchanged (openlogi-agent-core/tests/wire_format.rs still passes).

As a side benefit, this also stops a chronically-unhealthy node (e.g. an asleep BT-direct device that never answers) from forcing every one-shot call to burn all attempts.

Tests

Added unit tests in openlogi-core (structural equality across nested device fields) and openlogi-hid:

• Happy path: a complete inventory on attempt 1 returns after a single pass.
• Healthy partial drain stabilizes: the loop keeps retrying while the set grows, then stops once it stabilizes.
• Stabilization stop: a stable healthy-but-short set stops on "unchanged" rather than burning every attempt.
• Failed probe keeps retrying: an unchanged replay from an unhealthy pass does not stop early (regression guard).
• Bounded retries: a set that keeps changing still exits at the attempt cap.

cargo fmt, cargo clippy, and cargo test pass across the workspace.

[greptile-apps]

Greptile Summary

This PR fixes the one-shot enumerate() free function so its early-exit is transport-agnostic. Previously, the retry loop stopped on all_healthy, which for Unifying receivers was pairing_count.is_some() — a device whose arrival event lands after the ~1.5 s drain window was silently dropped while all_healthy stayed true, so openlogi list/openlogi diag intermittently returned a short device set with no recovery path.

• Adds a complete field to NodeProbe that, for Unifying, requires paired.len() == pairing_count (instead of just that the count register answered); Bolt keeps healthy == complete so no behavioral change there.
• Replaces the all_healthy early-exit with all_complete || (all_healthy && previous == current) || attempt >= cap, where the stable-unchanged shortcut lets a chronically-partial-but-settled Unifying drain avoid burning every retry, and the unhealthy-pass clears previous so a failed probe never masquerading as stable.
• Adds PartialEq/Eq derives to the five inventory structs needed to diff successive snapshots; field order is untouched so the bincode wire format is unchanged.

Confidence Score: 5/5

The change is safe to merge: it fixes a silent data-loss bug in the one-shot CLI path without touching the polling watcher, the IPC wire format, or per-node ledger replay.

The retry-loop logic is correct: all_complete is a strictly stronger condition than the old all_healthy for Bolt (behavior identical), and the new Unifying complete flag directly closes the gap the PR describes. The unhealthy-pass clear of previous_inventories prevents a ledger replay from ever counting as stable. All five new tests exercise the important boundaries of one_shot_should_stop. No floats in the Eq tree, no field reordering.

No files require special attention. inventory.rs and probe.rs are the core of the fix and both read cleanly.

Important Files Changed

Filename	Overview
crates/openlogi-hid/src/inventory.rs	Core fix: replaces the all_healthy early-exit with a three-condition one_shot_should_stop that correctly handles Unifying partial drains. Logic is sound — unhealthy passes clear previous, preventing a ledger replay from masquerading as stable.
crates/openlogi-hid/src/inventory/probe.rs	Adds complete to NodeProbe; Bolt keeps healthy == complete; Unifying separates them so a partial-but-responsive receiver reports complete = false without disturbing the ledger's healthy gate.
crates/openlogi-core/src/device.rs	Additive PartialEq/Eq derives on five inventory structs. All fields satisfy Eq, field order is unchanged, and a unit test verifies the derives propagate through nested fields.
crates/openlogi-hid/src/inventory/tests.rs	Five new unit tests cover the key one_shot_should_stop branches: happy path, growing-then-stable inventory, stable-short stops early, unhealthy pass does not allow unchanged-stop, and attempt cap.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[enumerate free fn] --> B[Fresh Enumerator\nprevious=None, attempt=1]
    B --> C[enumerate_reporting_completeness]
    C --> D{probe_one per node}
    D -->|Bolt| E["complete = healthy =\npaired.len == count"]
    D -->|Unifying| F["healthy = count.is_some\ncomplete = paired.len == count"]
    D -->|Direct| G["complete = healthy =\nwalk_succeeded"]
    D -->|Timeout or open failure| H["complete = false\nhealthy = false"]
    E --> I[AND-fold into all_complete / all_healthy]
    F --> I
    G --> I
    H --> I
    I --> J{one_shot_should_stop?}
    J -->|all_complete = true| K[Return inventories]
    J -->|all_healthy AND previous == current| K
    J -->|attempt >= ONESHOT_ATTEMPTS| K
    J -->|none of the above| L{all_healthy?}
    L -->|yes| M[previous = current]
    L -->|no| N[previous = None]
    M --> O[sleep 300ms, attempt++]
    N --> O
    O --> C

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[enumerate free fn] --> B[Fresh Enumerator\nprevious=None, attempt=1]
    B --> C[enumerate_reporting_completeness]
    C --> D{probe_one per node}
    D -->|Bolt| E["complete = healthy =\npaired.len == count"]
    D -->|Unifying| F["healthy = count.is_some\ncomplete = paired.len == count"]
    D -->|Direct| G["complete = healthy =\nwalk_succeeded"]
    D -->|Timeout or open failure| H["complete = false\nhealthy = false"]
    E --> I[AND-fold into all_complete / all_healthy]
    F --> I
    G --> I
    H --> I
    I --> J{one_shot_should_stop?}
    J -->|all_complete = true| K[Return inventories]
    J -->|all_healthy AND previous == current| K
    J -->|attempt >= ONESHOT_ATTEMPTS| K
    J -->|none of the above| L{all_healthy?}
    L -->|yes| M[previous = current]
    L -->|no| N[previous = None]
    M --> O[sleep 300ms, attempt++]
    N --> O
    O --> C

Loading

_{Reviews (2): Last reviewed commit: "fix(hid): make one-shot enumerate retry ..." | Re-trigger Greptile}

[greptile-apps]

Arrival-event ordering makes the stability check non-deterministic for Unifying

drain_device_arrival_unifying pushes events into out in the order they arrive over the HID channel; there is no sort. If a receiver sends events in a different slot order on two consecutive calls — e.g., slot 2 before slot 1 on the retry — the resulting paired vecs have the same devices but a different element order. Vec's PartialEq is element-wise, so previous == current returns false and the "stable healthy-but-short" early exit never fires even though the device set is identical. The loop then burns the full ONESHOT_ATTEMPTS budget instead of stopping after two passes, adding up to ~900 ms of unnecessary latency for every one-shot CLI call made against a Unifying receiver with offline paired devices. Sorting paired by slot in probe_unifying_receiver — or in enumerate_reporting_completeness before storing/comparing inventories — would make the comparison deterministic.