Skip to content

refactor(ipc): acknowledge pairing commands#304

Open
AprilNEA wants to merge 3 commits into
masterfrom
refactor/pairing-command-result
Open

refactor(ipc): acknowledge pairing commands#304
AprilNEA wants to merge 3 commits into
masterfrom
refactor/pairing-command-result

Conversation

@AprilNEA

Copy link
Copy Markdown
Owner

Summary

  • make pairing command RPCs return typed command-acceptance errors instead of fire-and-forget unit results
  • keep session progress on the pairing event stream, while command rejection is acknowledged immediately
  • map command rejection to typed pairing failures in the GUI so Add Device never stays in an optimistic state after rejection
  • bump PROTOCOL_VERSION and pin new wire variants

Validation

  • cargo fmt --check
  • cargo test -p openlogi-agent pairing::tests
  • cargo test -p openlogi-agent-core --test wire_format
  • cargo check -p openlogi-agent -p openlogi-gui
  • cargo clippy -p openlogi-agent-core -p openlogi-agent -p openlogi-gui --all-targets -- -D warnings
  • cargo test -p openlogi-agent-core -p openlogi-agent

Base automatically changed from refactor/pairing-failure-wire to refactor/hid-receiver-lease June 21, 2026 08:51
@AprilNEA AprilNEA changed the base branch from refactor/hid-receiver-lease to master June 21, 2026 08:58
@greptile-apps

greptile-apps Bot commented Jun 21, 2026

Copy link
Copy Markdown

Greptile Summary

This PR converts pairing IPC commands from fire-and-forget unit returns to typed Result<(), PairingCommandError> responses, introduces a structured PairingFailure enum replacing the previous String error, and replaces the raw AtomicBool pair (pairing_active / capture_idle) with a proper mutex-based ReceiverAccess arbiter that coordinates exclusive HID node ownership between capture and pairing without polling.

  • New types: PairingFailure (typed terminal failure reason) and PairingCommandError (immediate command-acceptance rejection) are added to the IPC layer; the GUI maps both through pairing_failure_text to localized strings without matching on human-readable content.
  • ReceiverAccess arbiter (receiver_access.rs): replaces the old two-atomic-bool rendezvous with an OwnedMutexGuard-based lease; PairingRequest uses RAII drop semantics so a cancelled acquire_for_pairing future automatically withdraws the pairing intent.
  • Gesture watcher now acquires a CaptureReceiverLease when starting a capture session and holds it for the session's lifetime; the two-tick stop-then-start sequence (line 217–219) prevents the old and new sessions from racing for the lock.

Confidence Score: 5/5

Safe to merge — the previous race (cancel injecting a spurious failure after Paired) is correctly resolved, and the new ReceiverAccess arbiter is a clean replacement for the dual-AtomicBool rendezvous.

Both concerns from the prior review round are properly addressed: cancel() returns Ok(()) when sessions==0 (no-op, no spurious Failed update), and reply_disconnected now uses AgentRestarted for StartPairing/PairDevice. The ReceiverAccess design is cancellation-safe (PairingRequest RAII guard clears pairing_requested if the acquire_for_pairing future is dropped), the double-check in try_acquire_for_capture prevents TOCTOU between the flag read and the lock acquisition, and SessionAdmission ensures the sessions counter is always rolled back on any early-exit path in start(). Wire-format golden tests and targeted unit tests for ReceiverAccess cover the critical invariants.

No files require special attention.

Important Files Changed

Filename Overview
crates/openlogi-agent-core/src/ipc.rs Adds PairingFailure and PairingCommandError enums with From impls; changes three pairing trait methods from () to Result<(), PairingCommandError>; bumps PROTOCOL_VERSION 4→6.
crates/openlogi-agent-core/src/receiver_access.rs New file: mutex-based ReceiverAccess arbiter with RAII PairingRequest (cancellation-safe) and double-check pattern in try_acquire_for_capture to prevent TOCTOU; well-tested with two targeted async unit tests.
crates/openlogi-agent-core/src/watchers/gesture.rs Replaces pairing_active/capture_idle AtomicBool pair with ReceiverAccess; adds CaptureReceiverLease acquisition before spawning a session; two-tick stop-then-start sequence prevents concurrent lease holders; want==current short-circuit preserved.
crates/openlogi-agent/src/pairing.rs start/pair/cancel return Result; SessionAdmission RAII guard rolls back sessions counter on uncommitted starts; cancel returns Ok(()) when sessions==0 (resolves previously flagged race); receiver_lease stored via StdMutex and released in translate on terminal events.
crates/openlogi-gui/src/ipc_client.rs pairing_command_result helper converts Ok(Err(cmd)) to PairingUpdate::Failed; reply_disconnected now sends AgentRestarted for StartPairing/PairDevice and skips CancelPairing; pairing_poll AgentRestarted injection converted from String to typed variant.
crates/openlogi-gui/src/windows/add_device.rs Adds pairing_failure_text mapping all PairingFailure variants to localized strings; exhaustive match ensures new variants are caught at compile time.
crates/openlogi-agent-core/tests/wire_format.rs Pins PROTOCOL_VERSION to 6 and adds golden wire encodings for PairingFailure variants and PairingCommandError; correctly removes old string-based Failed golden.
crates/openlogi-agent/src/server.rs Thin delegation layer updated to propagate Result types from PairingManager through the tarpc trait impl; no logic changes.
crates/openlogi-agent/src/main.rs Call site updated to pass shared.receiver_access.clone() instead of the two separate AtomicBool args; one-line change.
crates/openlogi-agent-core/src/orchestrator.rs SharedRuntime replaces pairing_active/capture_idle AtomicBools with a single ReceiverAccess field; initialization changed to ReceiverAccess::default().
crates/openlogi-agent-core/src/lib.rs Exposes the new receiver_access module as pub; one-line addition.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant GUI as GUI add_device
    participant IPC as ipc_client poll_loop
    participant Agent as AgentServer tarpc
    participant PM as PairingManager
    participant RA as ReceiverAccess
    participant GW as GestureWatcher ticker

    GUI->>IPC: Command::StartPairing
    IPC->>Agent: start_pairing(selector)
    Agent->>PM: start(selector)
    PM->>PM: compare_exchange 0 to 1
    PM->>RA: acquire_for_pairing timeout 5s
    RA->>RA: "pairing_requested = true"
    GW-->>RA: sees pairing_requested stops capture drops lease
    RA-->>PM: PairingReceiverLease mutex held
    PM->>PM: store lease send Control::Start
    PM-->>Agent: Ok(())
    Agent-->>IPC: Ok(Ok(()))
    IPC->>IPC: pairing_command_result no update sent

    Note over PM: terminal event in translate
    PM->>PM: sessions fetch_sub to 0 drop PairingReceiverLease
    RA->>RA: "pairing_requested = false"
    GW-->>RA: acquires CaptureReceiverLease resumes

    GUI->>IPC: Command::StartPairing while session active
    IPC->>Agent: start_pairing
    Agent->>PM: start
    PM-->>Agent: Err(AlreadyActive)
    Agent-->>IPC: Ok(Err(AlreadyActive))
    IPC->>GUI: PairingUpdate::Failed AlreadyActive
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"}}}%%
sequenceDiagram
    participant GUI as GUI add_device
    participant IPC as ipc_client poll_loop
    participant Agent as AgentServer tarpc
    participant PM as PairingManager
    participant RA as ReceiverAccess
    participant GW as GestureWatcher ticker

    GUI->>IPC: Command::StartPairing
    IPC->>Agent: start_pairing(selector)
    Agent->>PM: start(selector)
    PM->>PM: compare_exchange 0 to 1
    PM->>RA: acquire_for_pairing timeout 5s
    RA->>RA: "pairing_requested = true"
    GW-->>RA: sees pairing_requested stops capture drops lease
    RA-->>PM: PairingReceiverLease mutex held
    PM->>PM: store lease send Control::Start
    PM-->>Agent: Ok(())
    Agent-->>IPC: Ok(Ok(()))
    IPC->>IPC: pairing_command_result no update sent

    Note over PM: terminal event in translate
    PM->>PM: sessions fetch_sub to 0 drop PairingReceiverLease
    RA->>RA: "pairing_requested = false"
    GW-->>RA: acquires CaptureReceiverLease resumes

    GUI->>IPC: Command::StartPairing while session active
    IPC->>Agent: start_pairing
    Agent->>PM: start
    PM-->>Agent: Err(AlreadyActive)
    Agent-->>IPC: Ok(Err(AlreadyActive))
    IPC->>GUI: PairingUpdate::Failed AlreadyActive
Loading

Reviews (2): Last reviewed commit: "refactor(ipc): acknowledge pairing comma..." | Re-trigger Greptile

Comment thread crates/openlogi-agent/src/pairing.rs
Comment thread crates/openlogi-gui/src/ipc_client.rs Outdated
@AprilNEA AprilNEA force-pushed the refactor/pairing-command-result branch from 8ceb181 to ca71e57 Compare June 21, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant