feat(#760): Wire SysExIdentity into PortResolver (#752) by amiable-dev · Pull Request #778 · amiable-dev/conductor

amiable-dev · 2026-03-24T15:24:21Z

Summary

Wire the existing matches_with_sysex() method into the port resolution pipeline. SysExIdentity matchers now resolve ports when identity data is provided.

Changes

Add sysex_identity: Option<SysExIdentity> to PortInfo
PortResolver::resolve() chains: matches_with_usb() || matches_with_sysex()
Constructors (new, new_with_usb) default sysex_identity to None

TDD

test_sysex_identity_matches_with_metadata: KORG identity resolves to correct binding
test_sysex_identity_no_match_without_metadata: no identity data → Unbound

Test plan

16 resolver tests pass (14 existing + 2 new)
cargo clippy --workspace clean
cargo test --workspace

Pipeline wiring plan complete

All 3 wiring PRs:

feat(#760): Wire UsbIdentifier into PortResolver (#753) #776: UsbIdentifier → PortResolver (merged)
feat(#760): Wire channel scope into rule matching (#751) #777: Channel scope → rule matching (merged)
This PR: SysExIdentity → PortResolver

🤖 Generated with Claude Code

Extend PortInfo with sysex_identity and update PortResolver to check matches_with_sysex() alongside matches_with_usb(). SysExIdentity matchers now resolve ports when identity data is provided. - Add sysex_identity: Option<SysExIdentity> to PortInfo - Resolver chains: matches_with_usb() || matches_with_sysex() - Constructors (new, new_with_usb) default sysex_identity to None - TDD: 2 new tests (SysEx match, no-match without metadata) 16 resolver tests pass (14 existing + 2 new). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Wires SysEx Identity metadata into the PortResolver pipeline so DeviceMatcher::SysExIdentity matchers can participate in runtime port binding when identity reply data is available.

Changes:

Extend PortInfo with sysex_identity: Option<SysExIdentity> (defaulting to None in constructors).
Update PortResolver::resolve() to treat a matcher as matched when matches_with_usb(...) || matches_with_sysex(...) is true.
Add resolver tests covering SysExIdentity match with metadata and no-match without metadata.

Reviewed changes

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

File	Description
conductor-core/src/resolver.rs	Adds SysEx identity metadata to `PortInfo` and uses it during resolution via `matches_with_sysex()`
conductor-core/tests/resolver_test.rs	Adds TDD coverage to validate SysExIdentity-based binding behavior

Copilot · 2026-03-24T15:28:38Z

conductor-core/src/resolver.rs

@@ -17,16 +18,19 @@ pub struct PortInfo {
    pub vendor_id: Option<u16>,
    /// USB Product ID (populated when available from platform APIs)
    pub product_id: Option<u16>,
+    /// SysEx Identity Reply data (populated by probing, when available)
+    pub sysex_identity: Option<SysExIdentity>,
 }


PortInfo is a public struct with public fields; adding sysex_identity is a semver-breaking change for any downstream crate that constructs PortInfo via a struct literal (it must now provide the new field). If this type is part of the public API, consider making PortInfo #[non_exhaustive] and/or making fields private and exposing constructor/builder APIs so future metadata additions don’t keep causing breaking changes.

Copilot · 2026-03-24T15:28:38Z

conductor-core/src/resolver.rs

                for matcher in &identity.matchers {
-                    if matcher.matches_with_usb(&port.name, port.vendor_id, port.product_id) {
+                    let matched =
+                        matcher.matches_with_usb(&port.name, port.vendor_id, port.product_id)
+                            || matcher.matches_with_sysex(&port.name, port.sysex_identity.as_ref());
+                    if matched {


Now that PortResolver::resolve() invokes matches_with_sysex(), any existing documentation that says SysEx identity metadata is “not yet passed to the resolver” becomes stale/misleading. Please update the relevant doc comments (e.g., on DeviceMatcher::matches_with_sysex) to reflect the new wiring so future readers don’t miss that SysExIdentity matchers are active in resolution.

- Doc: matches_with_sysex() is now called by PortResolver (not stale) - Add doc note recommending constructors over struct literals for PortInfo Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

Copilot · 2026-03-24T15:58:59Z

conductor-core/src/resolver.rs

+                    let matched =
+                        matcher.matches_with_usb(&port.name, port.vendor_id, port.product_id)
+                            || matcher.matches_with_sysex(&port.name, port.sysex_identity.as_ref());
+                    if matched {


resolve() now evaluates both matches_with_usb() and matches_with_sysex() for every matcher. Because matches_with_sysex() delegates to matches(port_name) for non-SysEx matchers, name-based matchers that don’t match will do the same name/regex check twice (and for regex this also means two mutex-lock + regex is_match paths). Consider only calling matches_with_sysex() when the matcher is DeviceMatcher::SysExIdentity { .. } (or otherwise avoid the fallback-to-name path) to prevent the extra work.

Copilot · 2026-03-24T15:58:59Z

conductor-core/tests/resolver_test.rs

+    let ports = vec![PortInfo {
+        name: "Generic MIDI Port".to_string(),
+        index: 0,
+        vendor_id: None,
+        product_id: None,
+        sysex_identity: Some(SysExIdentity {
+            manufacturer_id: vec![0x42], // KORG
+            family: 0x0034,
+            model: 0x0001,
+            version: [1, 0, 0, 0],
+        }),
+    }];


This test constructs PortInfo via a struct literal, even though PortInfo’s docs now recommend using constructors to reduce future breakage when metadata fields are added. To keep tests consistent with that intent (and avoid having to set unrelated fields like VID/PID), consider building with PortInfo::new(...) and then setting sysex_identity on the instance.

Suggested change

let ports = vec![PortInfo {

name: "Generic MIDI Port".to_string(),

index: 0,

vendor_id: None,

product_id: None,

sysex_identity: Some(SysExIdentity {

manufacturer_id: vec![0x42], // KORG

family: 0x0034,

model: 0x0001,

version: [1, 0, 0, 0],

}),

}];

let mut port = PortInfo::new("Generic MIDI Port", 0);

port.sysex_identity = Some(SysExIdentity {

manufacturer_id: vec![0x42], // KORG

family: 0x0034,

model: 0x0001,

version: [1, 0, 0, 0],

});

let ports = vec![port];

- Only call matches_with_sysex() for SysExIdentity matchers; other matchers use matches_with_usb() which already handles name matching - Use PortInfo::new() + field set instead of struct literal in test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

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

Copilot · 2026-03-24T16:31:41Z

conductor-core/src/identity.rs

+    /// Note: `PortResolver::resolve()` calls both `matches_with_usb()` and this method,
+    /// passing `port.sysex_identity` from `PortInfo`. SysExIdentity matchers are active
+    /// in binding resolution when identity data is available.


The doc comment says PortResolver::resolve() calls both matches_with_usb() and matches_with_sysex(), but the resolver currently selects one method based on matcher type (SysExIdentity -> matches_with_sysex, all others -> matches_with_usb). Please update this note to reflect the actual call pattern to avoid misleading future changes/debugging.

Suggested change

/// Note: `PortResolver::resolve()` calls both `matches_with_usb()` and this method,

/// passing `port.sysex_identity` from `PortInfo`. SysExIdentity matchers are active

/// in binding resolution when identity data is available.

/// Note: `PortResolver::resolve()` selects between `matches_with_usb()` and this

/// method based on matcher type: `SysExIdentity` matchers use this method with

/// `port.sysex_identity` from `PortInfo`, while all other matchers use

/// `matches_with_usb()`. SysExIdentity matchers are only active in binding

/// resolution when identity data is available.

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

Copilot

Pull request overview

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

Copilot AI review requested due to automatic review settings March 24, 2026 15:24

Copilot started reviewing on behalf of amiable-dev March 24, 2026 15:24 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

fix: Update matches_with_sysex doc, add PortInfo constructor guidance

7654c6b

- Doc: matches_with_sysex() is now called by PortResolver (not stale) - Add doc note recommending constructors over struct literals for PortInfo Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

amiable-dev requested a review from Copilot March 24, 2026 15:55

Copilot started reviewing on behalf of amiable-dev March 24, 2026 15:55 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

amiable-dev requested a review from Copilot March 24, 2026 16:29

Copilot started reviewing on behalf of amiable-dev March 24, 2026 16:30 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

fix: Clarify doc — resolver dispatches by matcher type, not calls bot…

9df590a

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

amiable-dev requested a review from Copilot March 24, 2026 16:45

Copilot started reviewing on behalf of amiable-dev March 24, 2026 16:46 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

amiable-dev merged commit 245d631 into main Mar 24, 2026
15 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#760): Wire SysExIdentity into PortResolver (#752)#778

feat(#760): Wire SysExIdentity into PortResolver (#752)#778
amiable-dev merged 4 commits intomainfrom
fix/760-pipeline-wiring-sysex

amiable-dev commented Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    /// Note: `PortResolver::resolve()` calls both `matches_with_usb()` and this method,
-    /// passing `port.sysex_identity` from `PortInfo`. SysExIdentity matchers are active
-    /// in binding resolution when identity data is available.
+    /// Note: `PortResolver::resolve()` selects between `matches_with_usb()` and this
+    /// method based on matcher type: `SysExIdentity` matchers use this method with
+    /// `port.sysex_identity` from `PortInfo`, while all other matchers use
+    /// `matches_with_usb()`. SysExIdentity matchers are only active in binding
+    /// resolution when identity data is available.

Conversation

amiable-dev commented Mar 24, 2026

Summary

Changes

TDD

Test plan

Pipeline wiring plan complete

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants