feat(#760): Wire UsbIdentifier into PortResolver (#753) by amiable-dev · Pull Request #776 · amiable-dev/conductor

amiable-dev · 2026-03-24T08:34:26Z

Summary

Wire the existing matches_with_usb() method into the actual runtime pipeline. UsbIdentifier matchers now resolve ports when USB metadata is provided.

Changes

Extend PortInfo with vendor_id: Option<u16>, product_id: Option<u16>
PortResolver::resolve() calls matches_with_usb() instead of matches()
All daemon PortInfo constructors pass None (platform USB metadata is a follow-up)
Name-based matchers (ExactName, NameContains, NameRegex) continue working unchanged

TDD

3 new resolver tests: USB match with metadata, no-match without metadata, name matcher backward compat
14 total resolver tests pass

Test plan

14 resolver tests pass
cargo clippy --workspace clean
cargo test --workspace

🤖 Generated with Claude Code

Extend PortInfo with vendor_id/product_id and update PortResolver to use matches_with_usb() instead of matches(). UsbIdentifier matchers now resolve when USB metadata is provided. - Add vendor_id: Option<u16>, product_id: Option<u16> to PortInfo - PortResolver::resolve() calls matches_with_usb() for all matchers - Name-based matchers continue working unchanged (fall through) - All daemon PortInfo constructors pass None (metadata from platform APIs later) - TDD: 3 new tests (USB match, no-match without metadata, name matcher compat) 14 resolver tests pass (11 existing + 3 new). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR wires the existing USB VID/PID matcher path into the runtime binding resolver by extending PortInfo with optional USB metadata and having PortResolver use matches_with_usb(...) during identity resolution.

Changes:

Extend conductor_core::resolver::PortInfo with vendor_id / product_id fields.
Update PortResolver::resolve() to use DeviceMatcher::matches_with_usb(...) so UsbIdentifier can match when metadata is present.
Add resolver tests covering USB matching with/without metadata and ensuring name-based matcher behavior remains unchanged; update daemon PortInfo construction sites shown in the diff to populate None.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
conductor-daemon/src/input_manager.rs	Updates `PortInfo` construction to include `vendor_id`/`product_id: None` (metadata plumbing deferred).
conductor-core/src/resolver.rs	Adds USB metadata to `PortInfo` and switches resolver matching to `matches_with_usb(...)`.
conductor-core/tests/resolver_test.rs	Updates existing tests for new `PortInfo` fields and adds 3 tests for USB matcher behavior and backward compatibility.

Copilot · 2026-03-24T08:37:02Z

conductor-core/src/resolver.rs

+    /// USB Product ID (populated when available from platform APIs)
+    pub product_id: Option<u16>,
 }



PortInfo gained required vendor_id/product_id fields, but there are still PortInfo { name, index } literals elsewhere in the workspace (e.g. conductor-daemon/tests/hot_plug_test.rs, conductor-daemon/tests/multi_device_test.rs, conductor-core/tests/proptest_multidevice.rs). This will fail to compile cargo test --workspace unless those call sites are updated (likely setting both fields to None, or adding a PortInfo::new(...)/Default helper to reduce churn).

Suggested change

impl PortInfo {

/// Construct a `PortInfo` when USB IDs are unknown or unavailable.

pub fn new(name: String, index: usize) -> Self {

Self {

name,

index,

vendor_id: None,

product_id: None,

}

}

}

impl Default for PortInfo {

fn default() -> Self {

Self {

name: String::new(),

index: 0,

vendor_id: None,

product_id: None,

}

}

}

Copilot · 2026-03-24T08:37:03Z

conductor-core/src/resolver.rs

                for matcher in &identity.matchers {
-                    if matcher.matches(&port.name) {
+                    if matcher.matches_with_usb(&port.name, port.vendor_id, port.product_id) {
                        let specificity = matcher.specificity();


Now that PortResolver::resolve() calls matches_with_usb(...), the doc comment on DeviceMatcher::matches_with_usb() (in conductor-core/src/identity.rs) is outdated where it says the resolver still calls matches(port_name) and needs updating for #753. Please update that comment to avoid misleading future changes.

…pdate doc - Add PortInfo::new(name, index) convenience constructor - Fix PortInfo constructors in hot_plug_test, multi_device_test, proptest - Update matches_with_usb() doc: PortResolver now calls this method Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-24T09:48:05Z

conductor-daemon/src/input_manager.rs

+            port_infos.push(PortInfo {
+                name,
+                index,
+                vendor_id: None,
+                product_id: None,
+            });


Now that PortInfo has vendor_id/product_id, this struct literal is duplicated in multiple places just to set those fields to None. Since PortInfo::new(name, index) was added, consider using it here to reduce boilerplate and avoid future churn if more fields are added.

Suggested change

port_infos.push(PortInfo {

name,

index,

vendor_id: None,

product_id: None,

});

port_infos.push(PortInfo::new(name, index));

Copilot · 2026-03-24T09:48:06Z

conductor-daemon/src/input_manager.rs

+            port_infos.push(PortInfo {
+                name,
+                index,
+                vendor_id: None,
+                product_id: None,
+            });


Same as above: prefer PortInfo::new(name, index) (or another constructor/builder) instead of repeating vendor_id: None, product_id: None at each call site.

Suggested change

port_infos.push(PortInfo {

name,

index,

vendor_id: None,

product_id: None,

});

port_infos.push(PortInfo::new(name, index));

Copilot · 2026-03-24T09:48:06Z

conductor-core/src/identity.rs

-    /// Note: The `PortResolver` currently calls `matches(port_name)`, not this method.
-    /// To use `UsbIdentifier` matchers in binding resolution, the resolver must be
-    /// updated to pass USB metadata from `PortInfo`. See #753.
+    /// Note: `PortResolver::resolve()` calls this method (since PR #776), passing


The doc comment references “since PR #776”, but this PR is wiring UsbIdentifier into the resolver now (and the PR metadata references #753/#760). Please update the reference to the correct PR/issue number or rephrase to avoid hardcoding a PR number that can become inaccurate.

Suggested change

/// Note: `PortResolver::resolve()` calls this method (since PR #776), passing

/// Note: `PortResolver::resolve()` calls this method, passing

…ound 2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-24T10:38:32Z

conductor-core/src/identity.rs

+    /// Note: `PortResolver::resolve()` calls this method, passing
+    /// `port.vendor_id` and `port.product_id` from `PortInfo`.
    pub fn matches_with_usb(


There’s now an inconsistency in the docs: matches_with_sysex() still says the binding resolver only calls matches(port_name), but PortResolver::resolve() was updated to call matches_with_usb(...). Please update the matches_with_sysex() note so it accurately reflects the current resolver behavior (it still doesn’t pass SysEx identity, but it no longer calls matches() directly).

…h_usb (round 3) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot · 2026-03-24T11:52:12Z

conductor-daemon/tests/multi_device_test.rs

        PortInfo {
            name: "IAC Driver Bus 1".to_string(),
            index: 0,
+            vendor_id: None,
+            product_id: None,
        },
        PortInfo {
            name: "Mikro MK3".to_string(),
            index: 1,
+            vendor_id: None,
+            product_id: None,
        },
        PortInfo {
            name: "Launchpad Mini".to_string(),
            index: 2,
+            vendor_id: None,
+            product_id: None,
        },
        PortInfo {
            name: "MIDI Through".to_string(),
            index: 3,
+            vendor_id: None,
+            product_id: None,
        },


This test file repeats vendor_id: None / product_id: None in many PortInfo literals. Consider switching these to PortInfo::new(name, index) (or struct update syntax on top of it) to reduce churn when USB metadata gets plumbed in later.

Suggested change

PortInfo {

name: "IAC Driver Bus 1".to_string(),

index: 0,

vendor_id: None,

product_id: None,

},

PortInfo {

name: "Mikro MK3".to_string(),

index: 1,

vendor_id: None,

product_id: None,

},

PortInfo {

name: "Launchpad Mini".to_string(),

index: 2,

vendor_id: None,

product_id: None,

},

PortInfo {

name: "MIDI Through".to_string(),

index: 3,

vendor_id: None,

product_id: None,

},

PortInfo::new("IAC Driver Bus 1".to_string(), 0),

PortInfo::new("Mikro MK3".to_string(), 1),

PortInfo::new("Launchpad Mini".to_string(), 2),

PortInfo::new("MIDI Through".to_string(), 3),

Copilot · 2026-03-24T11:52:13Z

conductor-core/tests/proptest_multidevice.rs

            .map(|i| PortInfo {
                name: format!("Port {}", i),
                index: i,
+                vendor_id: None,
+                product_id: None,
            })


Since PortInfo::new() now exists to represent the common "USB IDs unknown" case, consider using it here inside the generator to avoid duplicating the vendor_id/product_id: None defaults and keep the proptest setup aligned with the canonical constructor.

Suggested change

.map(|i| PortInfo {

name: format!("Port {}", i),

index: i,

vendor_id: None,

product_id: None,

})

.map(|i| PortInfo::new(format!("Port {}", i), i))

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-24T12:03:37Z

conductor-daemon/src/input_manager.rs

+            port_infos.push(PortInfo {
+                name,
+                index,
+                vendor_id: None,
+                product_id: None,
+            });


Consider using PortInfo::new(name, index) here instead of repeating a struct literal with vendor_id: None / product_id: None. This keeps the default construction in one place and makes later USB metadata wiring less error-prone.

Suggested change

port_infos.push(PortInfo {

name,

index,

vendor_id: None,

product_id: None,

});

port_infos.push(PortInfo::new(name, index));

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot · 2026-03-24T12:42:53Z

conductor-core/src/resolver.rs

+            vendor_id: None,
+            product_id: None,
+        }
+    }


Right now PortInfo::new() only supports the no-USB-metadata case, so tests that need USB IDs are forced to use full struct literals. Consider adding a PortInfo::new_with_usb(...) (or builder-style setters) so call sites can avoid repeating all fields and so future PortInfo expansions don’t cascade into many test updates.

Suggested change

}

}

/// Construct a PortInfo with known USB Vendor and Product IDs.

pub fn new_with_usb(

name: impl Into<String>,

index: usize,

vendor_id: u16,

product_id: u16,

) -> Self {

Self {

name: name.into(),

index,

vendor_id: Some(vendor_id),

product_id: Some(product_id),

}

}

Copilot · 2026-03-24T12:42:53Z

conductor-core/tests/resolver_test.rs

+    let ports = vec![PortInfo {
+        name: "Generic MIDI Port".to_string(),
+        index: 0,
+        vendor_id: Some(0x17CC),  // NI
+        product_id: Some(0x1620), // Mikro MK3
+    }];


These new tests construct PortInfo using full struct literals to set USB IDs. If a convenience constructor (e.g., PortInfo::new_with_usb) is added, prefer using it here to reduce duplication and avoid future breakage if PortInfo gains additional fields.

Use in USB-specific tests to reduce struct literal boilerplate. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot