fix: judge conflicts by named probes first and polish diagnosis edges

Patch76 review round:

- A named compliance probe now outranks a synthesized successor: a
  verdict must never be decided by a version the probe invented while
  one the violated specifier names exists (mcp<=1.24.0 admits the
  1.24.0 floor and cannot hold the package below it, but rejecting the
  invented 1.24.0.0.1 got it blamed). Successors still stand in when no
  named seed survives the filter.
- The passthrough branch of _worker_startup_failure gains the test that
  goes red on regression: a worker-composed EmbeddedServerError must
  surface verbatim, kind intact, without the doubled prefix.
- The direct-reference audit branch requires a readable version, so a
  version-less installed dist no longer renders as "not installed".
- The conflict action's plural counts distinct domains, not pin
  entries: one integration pinning two violated packages is one
  integration to remove.
- _apply_embedded_only_skip carries the same annotations as the
  _apply_haos_tls_skip precedent it cites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QiuyTYBQ2opBVPp8xzMq3r

Author: kingpanther13

custom_components/ha_mcp_tools/dependency_diagnostics.py

@@ -160,7 +160,12 @@ def audit_dependency_graph(
             # present-but-different origin stays unjudged, since pip
             # normalizes URLs and a re-hosted identical artifact is fine).
             if requirement.url is not None:
-                unsatisfied = not _dist_has_direct_url(child_dist)
+                # version non-None on this branch too: a dist whose METADATA
+                # yields no version would otherwise render as "not installed"
+                # in the violation sentence (Patch76 review on #2245).
+                unsatisfied = version is not None and not _dist_has_direct_url(
+                    child_dist
+                )
             else:
                 unsatisfied = version is not None and not _specifier_allows(
                     requirement, version
@@ -313,23 +318,30 @@ def _compliance_probes(violated: str) -> list[str]:
     """Version literals a compliant install could sit at, read from ``violated``.
 
     The lower-bound-ish clauses (``>=``, ``==``, ``===``, ``~=``, ``>``) seed
-    the candidates — the version itself plus a nearby higher successor, so a
-    seed the specifier itself rejects (an exclusive ``>`` bound, a ``!=``
-    exclusion sitting on the floor) still leaves a compliant probe; upper
-    bounds only exclude. A wildcard pin's trailing ``.*`` is stripped so the
-    candidate parses as a version. Every candidate is then checked against
-    the FULL violated specifier before it may serve as a probe: a probe that
-    violates the specifier would acquit exactly the pin that preserves the
-    conflict, and an empty probe list flips the caller conservative, blaming
-    integrations that are compatible (both CodeRabbit on #2245: ``>1.24``
-    probed with ``1.24`` acquitted a ``==1.24`` pinner, and
-    ``>=1.24,!=1.24`` yielded no probe at all).
+    the candidates; upper bounds only exclude. A wildcard pin's trailing
+    ``.*`` is stripped so the candidate parses as a version, and every
+    candidate is checked against the FULL violated specifier before it may
+    serve as a probe: a probe that violates the specifier would acquit
+    exactly the pin that preserves the conflict, and an empty probe list
+    flips the caller conservative, blaming integrations that are compatible
+    (both CodeRabbit on #2245: ``>1.24`` probed with ``1.24`` acquitted a
+    ``==1.24`` pinner, and ``>=1.24,!=1.24`` yielded no probe at all).
+
+    NAMED versions outrank synthesized ones: when any seed the violated
+    specifier itself names survives the filter, only those serve as probes,
+    and the nearby higher successors stand in solely when none do (an
+    exclusive ``>`` bound, a floor exclusion). A verdict must never be
+    decided by a version the probe invented while a named one exists —
+    ``mcp<=1.24.0`` admits the named floor of ``mcp<2.0,>=1.24.0`` and
+    cannot hold the package below it, but rejecting the synthesized
+    ``1.24.0.0.1`` used to get it blamed (Patch76 review on #2245).
     """
     try:
         specifier = Requirement(violated).specifier
     except InvalidRequirement:
         return []
-    candidates: list[str] = []
+    named: list[str] = []
+    synthesized: list[str] = []
     for clause in specifier:
         if clause.operator not in (">=", "==", "===", "~=", ">"):
             continue
@@ -338,17 +350,22 @@ def _compliance_probes(violated: str) -> list[str]:
             parsed = Version(base)
         except InvalidVersion:
             continue
-        candidates += [base, _successor(parsed)]
-    probes: list[str] = []
-    for candidate in candidates:
-        if candidate in probes:
-            continue
+        named.append(base)
+        synthesized.append(_successor(parsed))
+
+    def _surviving(candidates: list[str]) -> list[str]:
         # contains() answers False for a candidate it cannot parse (the
         # same quirk _specifier_allows documents), so a successor shape
         # that fails to parse is filtered here, never raised.
-        if specifier.contains(candidate, prereleases=True):
-            probes.append(candidate)
-    return probes
+        probes: list[str] = []
+        for candidate in candidates:
+            if candidate not in probes and specifier.contains(
+                candidate, prereleases=True
+            ):
+                probes.append(candidate)
+        return probes
+
+    return _surviving(named) or _surviving(synthesized)
 
 
 def _successor(version: Version) -> str:
@@ -622,7 +639,11 @@ def _action_sentence(
 ) -> str:
     """The closing instruction, scaled to how much the diagnosis identified."""
     if pinners:
-        subject = "integration" if len(pinners) == 1 else "integrations"
+        # Distinct domains, not entries: one integration pinning two
+        # violated packages is still one integration to update or remove
+        # (Patch76 review on #2245).
+        domains = {pinner.domain for pinner in pinners}
+        subject = "integration" if len(domains) == 1 else "integrations"
         # The reinstall clause is load-bearing on the no-install fast path
         # (a pinned server spec, or auto-update off): removing the
         # integration deletes its pin but restores nothing, and the next

tests/src/e2e/conftest.py

@@ -259,7 +259,9 @@ def _apply_haos_tls_skip(item: Any, enabled: bool, skip_marker: Any) -> None:
         item.add_marker(skip_marker)
 
 
-def _apply_embedded_only_skip(item, embedded_selected, skip_marker):
+def _apply_embedded_only_skip(
+    item: Any, embedded_selected: bool, skip_marker: Any
+) -> None:
     """Skip an ``embedded_only`` item everywhere but the embedded lane.
 
     Split out of ``pytest_collection_modifyitems`` for the same reason as

tests/src/unit/test_dependency_diagnostics.py

@@ -381,6 +381,16 @@ def test_wildcard_floor_still_probes(self):
         assert not requirement_forces_conflict("mcp>=1", wildcard)
         assert requirement_forces_conflict("mcp==1.9.0", wildcard)
 
+    def test_upper_bound_admitting_the_named_floor_is_innocent(self):
+        """Patch76 review on #2245: a named probe outranks a synthesized one.
+
+        ``mcp<=1.24.0`` admits the violated spec's own floor, so enforcing
+        it can never hold the package below that floor — blaming it for
+        rejecting the invented successor 1.24.0.0.1 pointed the user at an
+        integration that cannot be the culprit.
+        """
+        assert not requirement_forces_conflict("mcp<=1.24.0", self._VIOLATION)
+
     def test_inactive_environment_marker_is_innocent(self):
         """CodeRabbit on #2245: HA never installs a marker-inactive requirement."""
         assert not requirement_forces_conflict(
@@ -668,6 +678,23 @@ def test_several_pinners_read_as_plural(self):
         assert "one" in message
         assert "two" in message
 
+    def test_one_domain_with_two_pins_reads_as_singular(self):
+        """Patch76 review on #2245: one integration pinning two violated
+        packages is still one integration to update or remove."""
+        message = describe_dependency_failure(
+            None,
+            [],
+            [
+                PinningIntegration(domain="one", name="One", requirement="mcp==1.0"),
+                PinningIntegration(
+                    domain="one", name="One", requirement="websockets==9.0"
+                ),
+            ],
+        )
+
+        assert "conflicting integration," in message
+        assert "conflicting integrations" not in message
+
     def test_root_exception_alone_still_yields_an_action(self):
         message = describe_dependency_failure(ImportError(_ICON_IMPORT_ERROR), [], [])
 

tests/src/unit/test_embedded_server.py

@@ -2185,6 +2185,27 @@ async def test_wait_ready_raises_on_early_thread_crash(self, tmp_path):
         with pytest.raises(es.EmbeddedServerError, match="worker thread crashed"):
             await mgr._async_wait_until_ready()
 
+    async def test_wait_ready_surfaces_worker_composed_error_verbatim(self, tmp_path):
+        """A worker-composed EmbeddedServerError passes through UNWRAPPED.
+
+        The passthrough branch of _worker_startup_failure carries the
+        dependency diagnosis and its failure kind; re-wrapping is what
+        produced the doubled "failed to start:" prefix (#2239, Patch76
+        review: this branch had no test that goes red on regression).
+        """
+        mgr, hass, _entry = _manager(tmp_path)
+        hass.loop.time = MagicMock(return_value=0.0)
+        composed = es.EmbeddedServerError(
+            "Installed mcp 1.14.1 does not satisfy 'mcp>=1.24.0'.",
+            kind="package",
+        )
+        mgr._thread_exc = composed
+        with pytest.raises(es.EmbeddedServerError) as excinfo:
+            await mgr._async_wait_until_ready()
+        assert excinfo.value is composed
+        assert excinfo.value.kind == "package"
+        assert "failed to start" not in str(excinfo.value)
+
     async def test_wait_ready_raises_when_thread_exited(self, tmp_path):
         mgr, hass, _entry = _manager(tmp_path)
         hass.loop.time = MagicMock(return_value=0.0)