Try next mDNS address on Pair-Verify IncorrectPairingIdError

[bluetoothbot]

Summary

Some buggy HAP accessories (e.g. the BTicino Smarther 2 / XW8002) advertise other devices' IP addresses in their mDNS addresses list, with rotating order on each announcement. When the first address belongs to a different physical accessory, it connects fine at the TCP layer but returns its own pairing id during Pair-Verify step 3, raising IncorrectPairingIdError. The reconnect loop previously retried the same address forever and never reached the correct one. This makes the connection skip an address that fails Pair-Verify and try the next advertised address.

Closes #571

Changes

• SecureHomeKitConnection._connect_once() now loops over advertised addresses: on IncorrectPairingIdError it excludes the offending address and retries the next one, only propagating the error once all addresses are exhausted.
• HomeKitConnection._connect_once() gains an exclude_hosts parameter so the bad address is removed before the next TCP attempt.
• Pair-Verify negotiation extracted into _pair_verify() for the retry loop.

Test plan

• New tests/test_ip_connection.py covers: exclude_hosts filtering, address fallback on IncorrectPairingIdError, and error propagation once all addresses are exhausted.
• Full suite passes (220 passed).

---

Quality Report

Changes: 2 files changed, 177 insertions(+), 5 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: 1 issue(s)

• Non-conventional commit: Try next mDNS address on Pair-Verify IncorrectPairingIdError

Generated by Kōan

[bluetoothbot]

PR Review — Try next mDNS address on Pair-Verify IncorrectPairingIdError

Solid, well-scoped fix for the rotating-mDNS-address bug; one robustness gap around loop termination is worth addressing before merge.

Strengths:

• Clean extraction of _pair_verify() so the Pair-Verify exchange can be retried independently of the TCP connect, with the inline state-machine logic moved verbatim.
• The fix catches only IncorrectPairingIdError, so other Pair-Verify failures still bubble to _reconnect's backoff path — prior behavior preserved.
• Accumulating exclude_hosts correctly handles the case where more than one advertised address points at a wrong device, and the offending transport is torn down before each retry.
• exclude_hosts defaults to None (no mutable default), and _connect_once raises cleanly when all addresses are filtered out.
• New tests/test_ip_connection.py exercises exclusion filtering, address fallback, and exhaustion. I ran the full suite on the branch: 220 passed — the quality report's "Tests: failed" is a false positive.

Needs attention:

• Potential permanent hang / connection hammering if self.connected_host (from getpeername) doesn't string-match an entry in self.hosts (plausible for IPv6 zone-id/format mismatch): the loop has no backoff and holds the connect lock. Add a wrong_host not in self.hosts guard.
• Minor: owner.connection_made(False) re-fires on each discarded attempt — confirm it doesn't cause connect/disconnect churn upstream.
• Minor: the two give-up guard branches are untested.

---

🟡 Important

1. Loop can hammer the wrong device forever if connected_host isn't found in self.hosts
aiohomekit/controller/ip/connection.py:759-778

The termination guard relies on wrong_host (self.connected_host, set from sock.getpeername()[0]) string-matching an entry in self.hosts. If it doesn't match, remaining = [host for host in self.hosts if host not in exclude_hosts] never shrinks, if not remaining: raise never fires, and the next super()._connect_once(exclude_hosts=...) filters out nothing — so it reconnects to the same device, gets the same IncorrectPairingIdError, and repeats indefinitely.

Why it matters: this loop has no backoff/sleep (unlike _reconnect) and holds self._connect_lock the whole time, so a mismatch becomes a permanent hang for that accessory plus continuous TCP+Pair-Verify traffic against the wrong device. For plain IPv4 the canonical getpeername string matches the advertised address, but IPv6 with zone/scope ids or differing normalization (fe80::1%eth0 vs bare form) is a realistic way to hit the mismatch.

Suggested fix — refuse to loop when the offending host can't be excluded:

wrong_host = self.connected_host
if not wrong_host or wrong_host not in self.hosts:
    # Can't make progress by excluding it; give up rather than loop.
    raise

This also subsumes the existing if not wrong_host guard.

wrong_host = self.connected_host
if not wrong_host:
    raise
exclude_hosts.append(wrong_host)
remaining = [host for host in self.hosts if host not in exclude_hosts]

🟢 Suggestions

1. Coverage gap for the give-up guards
tests/test_ip_connection.py:63-95

The new tests cover the happy fallback and full-exhaustion paths well. Two branches added in this PR are untested:

• the if not wrong_host: raise guard (connected_host is None when the error surfaces), and
• the proposed wrong_host not in self.hosts guard (the infinite-loop edge in the other comment).

A test that drives IncorrectPairingIdError while connected_host is None/unknown and asserts the error propagates exactly once (rather than looping) would lock in the termination contract that protects against the hang described above.

---

Checklist

• Error handling: only the intended exception is caught and retried
• Loop has a guaranteed termination condition — warning ＃1
• Resource cleanup on retry/error paths (transport torn down)
• No mutable default arguments
• Backward-compatible signature change (exclude_hosts optional)
• Test coverage for new branches
• Diff matches PR description (no scope creep)

---

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.

---

Silent Failure Analysis

🟡 **MEDIUM** — non-terminating retry hides failure (silent hang)
aiohomekit/controller/ip/connection.py:746-783

Risk: The while True loop only terminates when remaining empties, which assumes self.connected_host is always a member of self.hosts; if it ever isn't, remaining never shrinks, the same wrong address is reconnected every iteration, and the IncorrectPairingIdError is never surfaced — turning a recoverable failure into an unbounded loop that silently burns connections.

exclude_hosts.append(wrong_host)
remaining = [host for host in self.hosts if host not in exclude_hosts]
...
if not remaining:
    raise

Fix: Add an explicit progress check (e.g. raise/break if wrong_host is already in exclude_hosts, or bound the loop to len(self.hosts) attempts) so the error propagates instead of spinning forever.

---

Automated review by Kōan (Claude) HEAD=11abf51 6 min 17s

[bluetoothbot]

Rebase with requested adjustments

Branch koan/fix-issue-571 was rebased onto main and review feedback was applied.

Changes applied

• Done.
• Hardened loop termination in SecureHomeKitConnection._connect_once(): changed guard from if not wrong_host: to if not wrong_host or wrong_host not in self.hosts:. Addresses the 🟡 silent-hang / non-terminating-loop warning — when the responding host can't be matched against advertised self.hosts (e.g. IPv6 zone-id normalization), remaining never shrank, so the loop reconnected to the same wrong device forever while holding _connect_lock. Now propagates IncorrectPairingIdError instead of spinning.
• Added test_pair_verify_raises_when_responding_host_unknown in tests/test_ip_connection.py: drives IncorrectPairingIdError with connected_host set to an unadvertised host, asserts the error surfaces after exactly one attempt rather than looping. Closes the 🟢 coverage gap for the give-up guard.

Stats

2 files changed, 207 insertions(+), 5 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=Recent main commits are only dependency bumps and CI changes, and connection.py contains no exclude_)
• Rebased koan/fix-issue-571 onto upstream/main
• Applied review feedback
• Pre-push CI check: no CI runs found
• Force-pushed koan/fix-issue-571 to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.45%. Comparing base (c1c0566) to head (bff6fe2).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #572      +/-   ##
==========================================
+ Coverage   75.21%   75.45%   +0.23%     
==========================================
  Files         100      101       +1     
  Lines        9580     9683     +103     
==========================================
+ Hits         7206     7306     +100     
- Misses       2374     2377       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.