fix: pooled-WS connection-error residue and verify_ssl pool-key split (#1833)

* fix: pooled-WS connection-error residue and verify_ssl pool-key split

Follow-up to the two review notes on #1832:

- shared is_connection_error_message() helper (extracted from the
  history classifier); calendar's WS sites re-raise transport-shaped
  failures as HomeAssistantConnectionError and both calendar error
  handlers return connectivity guidance instead of rrule/uid hints;
  radio's _resolve_entity_device raises CONNECTION_FAILED for a
  transport drop instead of ENTITY_NOT_FOUND
- WebSocketManager.get_client keys on the EFFECTIVE verify_ssl
  (None resolves to the settings default), so callers passing the
  resolved default share the pooled connection with callers that omit
  the argument; only genuine overrides get an isolated connection

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

* fix: review-round hardening for the connection classifier and pool key

- is_connection_error_message stringifies its payload (HA error frames
  can carry a dict or None in the error slot)
- pool-key settings lookup falls back to verify_ssl=True on settings
  load failure, mirroring HomeAssistantWebSocketClient.__init__
  (extracted to _effective_verify_ssl to stay under the complexity cap)
- broaden the connection signatures with the pool's own close messages:
  'closed while waiting', 'connection dropped', 'not authenticated'

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

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: kingpanther13

src/ha_mcp/client/websocket_client.py

@@ -1083,6 +1083,23 @@ def _client_key(url: str, token: str) -> str:
         """Create a cache key from credentials."""
         return hashlib.sha256(f"{url.rstrip('/')}:{token}".encode()).hexdigest()
 
+    @staticmethod
+    def _effective_verify_ssl(verify_ssl: bool | None) -> bool:
+        """Resolve the effective TLS-verification mode for the pool key."""
+        if verify_ssl is not None:
+            return verify_ssl
+        try:
+            return bool(get_global_settings().verify_ssl)
+        except Exception as e:
+            # Mirror HomeAssistantWebSocketClient.__init__: a bad env var
+            # elsewhere should not crash pooling or silently flip TLS off.
+            logger.warning(
+                "Could not load settings while resolving the pool verify_ssl "
+                "key (%s); falling back to verify_ssl=True.",
+                e,
+            )
+            return True
+
     async def get_client(
         self,
         url: str | None = None,
@@ -1138,9 +1155,16 @@ async def get_client(
                 ws_url = settings.homeassistant_url
                 ws_token = settings.homeassistant_token
 
-            key = self._client_key(ws_url, ws_token)
-            if verify_ssl is not None:
-                key = f"{key}|verify_ssl={verify_ssl}"
+            # Key on the EFFECTIVE verification mode: a caller passing the
+            # resolved settings default (send_websocket_message) must share
+            # the pooled connection with callers that omit the argument
+            # (listener, HACS, installer) — only a genuine override such as
+            # verify_ssl=False gets its own isolated connection.
+            effective_verify_ssl = self._effective_verify_ssl(verify_ssl)
+            key = (
+                f"{self._client_key(ws_url, ws_token)}"
+                f"|verify_ssl={effective_verify_ssl}"
+            )
 
             # Return existing connected client for these credentials
             existing = self._clients.get(key)

src/ha_mcp/tools/tools_calendar.py

@@ -15,7 +15,10 @@
 from fastmcp.tools import tool
 from pydantic import Field
 
-from ..client.rest_client import HomeAssistantCommandError
+from ..client.rest_client import (
+    HomeAssistantCommandError,
+    HomeAssistantConnectionError,
+)
 from ..errors import ErrorCode, create_error_response
 from .auto_backup import with_auto_backup
 from .helpers import (
@@ -25,6 +28,7 @@
     register_tool_methods,
     validate_identifier_not_empty,
 )
+from .util_helpers import is_connection_error_message
 
 logger = logging.getLogger(__name__)
 
@@ -201,17 +205,20 @@ async def _create_recurring_calendar_event(
 
         # Route through the shared pooled WebSocket (issue #1813) instead of a
         # dedicated connect/auth handshake per call. The pooled path collapses a
-        # failed WS command into ``{"success": False, "error": ...}``; re-raise
-        # it as the same ``HomeAssistantCommandError`` the dedicated send_command
-        # used to raise so the caller's error handler still attaches the
-        # rrule-specific suggestions.
+        # failed WS command into ``{"success": False, "error": ...}``; a
+        # transport-shaped failure re-raises as ``HomeAssistantConnectionError``
+        # (the classifier type-matches it to connectivity guidance), anything
+        # else as the same ``HomeAssistantCommandError`` the dedicated
+        # send_command used to raise so the caller's error handler still
+        # attaches the rrule-specific suggestions.
         result = await self._client.send_websocket_message(
             {"type": "calendar/event/create", "entity_id": entity_id, "event": event}
         )
         if not result.get("success"):
-            raise HomeAssistantCommandError(
-                str(result.get("error", "calendar/event/create failed"))
-            )
+            error = str(result.get("error", "calendar/event/create failed"))
+            if is_connection_error_message(error):
+                raise HomeAssistantConnectionError(error)
+            raise HomeAssistantCommandError(error)
         return result
 
     async def _create_simple_calendar_event(
@@ -242,6 +249,13 @@ def _build_set_calendar_event_error_suggestions(
         self, entity_id: str, rrule: str | None, error: Exception
     ) -> list[str]:
         """Build suggestions for a failed ha_config_set_calendar_event call."""
+        if isinstance(error, HomeAssistantConnectionError):
+            # A transport drop is not a calendar problem — domain hints would
+            # send the agent chasing a non-issue during an HA restart.
+            return [
+                "Home Assistant may be restarting or unreachable — retry shortly",
+                "Check the connection to Home Assistant",
+            ]
         suggestions = [
             f"Verify calendar entity '{entity_id}' exists and supports event creation",
             "Check datetime format (ISO 8601)",
@@ -527,17 +541,20 @@ async def ha_config_remove_calendar_event(
                 ws_kwargs["recurrence_range"] = recurrence_range
 
             # Route through the shared pooled WebSocket (issue #1813) instead of a
-            # dedicated connect/auth handshake per call. Re-raise a failed WS
-            # command as the same ``HomeAssistantCommandError`` the dedicated
-            # send_command used to raise so the outer handler builds the
-            # delete-specific suggestions (404 / not-supported).
+            # dedicated connect/auth handshake per call. Transport-shaped
+            # failures raise ``HomeAssistantConnectionError`` (connectivity
+            # guidance); anything else re-raises as the same
+            # ``HomeAssistantCommandError`` the dedicated send_command used to
+            # raise so the outer handler builds the delete-specific
+            # suggestions (404 / not-supported).
             result = await self._client.send_websocket_message(
                 {"type": "calendar/event/delete", **ws_kwargs}
             )
             if not result.get("success"):
-                raise HomeAssistantCommandError(
-                    str(result.get("error", "calendar/event/delete failed"))
-                )
+                error = str(result.get("error", "calendar/event/delete failed"))
+                if is_connection_error_message(error):
+                    raise HomeAssistantConnectionError(error)
+                raise HomeAssistantCommandError(error)
 
             return {
                 "success": True,
@@ -554,28 +571,41 @@ async def ha_config_remove_calendar_event(
         except Exception as error:
             logger.error(f"Failed to delete calendar event from {entity_id}: {error}")
 
-            suggestions = [
-                f"Verify calendar entity '{entity_id}' exists",
-                f"Verify event with UID '{uid}' exists in the calendar",
-                "Use ha_config_get_calendar_events() to find the correct event UID",
-                "Some calendar integrations may not support event deletion",
-            ]
-
-            error_str = str(error)
-            if "404" in error_str or "not found" in error_str.lower():
-                suggestions.insert(
-                    0, f"Calendar entity '{entity_id}' or event '{uid}' not found"
-                )
-            if "not supported" in error_str.lower():
-                suggestions.insert(0, "This calendar does not support event deletion")
-
             exception_to_structured_error(
                 error,
                 context={"entity_id": entity_id, "uid": uid},
-                suggestions=suggestions,
+                suggestions=self._build_remove_calendar_event_error_suggestions(
+                    entity_id, uid, error
+                ),
             )
             return None  # unreachable: exception_to_structured_error always raises
 
+    def _build_remove_calendar_event_error_suggestions(
+        self, entity_id: str, uid: str, error: Exception
+    ) -> list[str]:
+        """Build suggestions for a failed ha_config_remove_calendar_event call."""
+        if isinstance(error, HomeAssistantConnectionError):
+            # A transport drop is not a calendar problem — domain hints would
+            # send the agent chasing a non-issue during an HA restart.
+            return [
+                "Home Assistant may be restarting or unreachable — retry shortly",
+                "Check the connection to Home Assistant",
+            ]
+        suggestions = [
+            f"Verify calendar entity '{entity_id}' exists",
+            f"Verify event with UID '{uid}' exists in the calendar",
+            "Use ha_config_get_calendar_events() to find the correct event UID",
+            "Some calendar integrations may not support event deletion",
+        ]
+        error_str = str(error)
+        if "404" in error_str or "not found" in error_str.lower():
+            suggestions.insert(
+                0, f"Calendar entity '{entity_id}' or event '{uid}' not found"
+            )
+        if "not supported" in error_str.lower():
+            suggestions.insert(0, "This calendar does not support event deletion")
+        return suggestions
+
 
 def register_calendar_tools(mcp: Any, client: Any, **kwargs: Any) -> None:
     """Register calendar management tools with the MCP server."""

src/ha_mcp/tools/tools_history.py

@@ -32,6 +32,7 @@
     JSON_STRING_COERCION,
     add_timezone_metadata,
     build_pagination_metadata,
+    is_connection_error_message,
     parse_string_list_param,
     project_fields,
 )
@@ -490,16 +491,6 @@ def _parse_time_range(
     return start_dt, end_dt
 
 
-_WS_CONNECTION_SIGNATURES = (
-    "failed to connect",
-    "timed out",
-    "timeout",
-    "connection closed",
-    "disconnected",
-    "not connected",
-)
-
-
 def _raise_recorder_ws_failure(
     kind: str,
     error_msg: str,
@@ -513,8 +504,7 @@ def _raise_recorder_ws_failure(
     as CONNECTION_FAILED (retry/connectivity guidance) instead of presenting
     recorder-retention suggestions during an HA restart or WS outage.
     """
-    lowered = error_msg.lower()
-    if any(sig in lowered for sig in _WS_CONNECTION_SIGNATURES):
+    if is_connection_error_message(error_msg):
         raise_tool_error(
             create_error_response(
                 ErrorCode.CONNECTION_FAILED,

src/ha_mcp/tools/tools_radio.py

@@ -31,7 +31,7 @@
 from .radio import zigbee as zigbee_handler
 from .radio import zwave as zwave_handler
 from .radio.base import confirm_required, require
-from .util_helpers import JSON_STRING_COERCION
+from .util_helpers import JSON_STRING_COERCION, is_connection_error_message
 
 logger = logging.getLogger(__name__)
 
@@ -62,6 +62,20 @@ async def _resolve_entity_device(self, entity_id: str) -> str:
             device_id = (result.get("result") or {}).get("device_id")
             if device_id:
                 return str(device_id)
+        else:
+            error = str(result.get("error", ""))
+            if is_connection_error_message(error):
+                # A transport drop is not evidence the entity is missing.
+                raise_tool_error(
+                    create_error_response(
+                        ErrorCode.CONNECTION_FAILED,
+                        f"Could not resolve entity '{entity_id}': {error}",
+                        context={"entity_id": entity_id},
+                        suggestions=[
+                            "Home Assistant may be restarting or unreachable — retry shortly",
+                        ],
+                    )
+                )
         raise_tool_error(
             create_error_response(
                 ErrorCode.ENTITY_NOT_FOUND,

src/ha_mcp/tools/util_helpers.py

@@ -2236,3 +2236,35 @@ def merge_visibility_warnings(
     if warnings:
         response.setdefault("warnings", []).extend(warnings)
     return response
+
+
+# Error strings produced by the pooled WebSocket path when the transport (not
+# the command) fails: the manager's connect raise, send timeouts, and socket
+# drops. ``send_websocket_message`` collapses these into
+# ``{"success": False, "error": ...}``, so callers that attach domain-specific
+# suggestions must first check the shape or an HA restart gets presented as a
+# domain problem (issue #1832 review).
+WS_CONNECTION_SIGNATURES = (
+    "failed to connect",
+    "timed out",
+    "timeout",
+    "connection closed",
+    # reset_connection: "WebSocket connection to Home Assistant closed while
+    # waiting for a response" — "connection closed" is not adjacent there.
+    "closed while waiting",
+    # listener close reason: "connection dropped without a close frame".
+    "connection dropped",
+    "disconnected",
+    "not connected",
+    "not authenticated",
+)
+
+
+def is_connection_error_message(error_msg: Any) -> bool:
+    """True when a pooled-WS failure payload is connection/transport-shaped.
+
+    Accepts any payload shape: HA error frames can carry a dict (or None)
+    in the error slot, so the value is stringified before matching.
+    """
+    lowered = str(error_msg).lower()
+    return any(sig in lowered for sig in WS_CONNECTION_SIGNATURES)

tests/src/unit/test_oauth.py

@@ -1650,3 +1650,70 @@ def test_hmac_secret_file_written_owner_only(self, tmp_data_dir):
         secret_file = tmp_data_dir / "oauth_hmac_secret"
         assert secret_file.exists()
         assert stat.S_IMODE(secret_file.stat().st_mode) == 0o600
+
+
+class TestWebSocketManagerVerifySslKey:
+    """Pool-key resolution for verify_ssl (issue #1832 review follow-up)."""
+
+    @pytest.fixture(autouse=True)
+    def reset_manager(self):
+        from ha_mcp.client.websocket_client import WebSocketManager
+
+        WebSocketManager._instance = None
+        yield
+        WebSocketManager._instance = None
+
+    @pytest.mark.asyncio
+    async def test_defaulted_verify_ssl_shares_connection_with_omitted(self):
+        """get_client(verify_ssl=<settings default>) and get_client() must key
+        identically — one pooled connection, not a split."""
+        from ha_mcp.client.websocket_client import WebSocketManager
+        from ha_mcp.config import get_global_settings
+
+        mock_client = MagicMock()
+        mock_client.is_connected = True
+        mock_client.connect = AsyncMock(return_value=True)
+
+        calls = []
+
+        def factory(url, token, **kwargs):
+            calls.append(kwargs)
+            return mock_client
+
+        manager = WebSocketManager()
+        manager.configure(client_factory=factory)
+
+        default = get_global_settings().verify_ssl
+        a = await manager.get_client(url="http://ha.local:8123", token="tok")
+        b = await manager.get_client(
+            url="http://ha.local:8123", token="tok", verify_ssl=default
+        )
+        assert a is b
+        assert len(calls) == 1
+
+    @pytest.mark.asyncio
+    async def test_verify_ssl_override_gets_isolated_connection(self):
+        """A genuine verify_ssl override must NOT share the default pool entry."""
+        from ha_mcp.client.websocket_client import WebSocketManager
+        from ha_mcp.config import get_global_settings
+
+        default = get_global_settings().verify_ssl
+
+        clients = []
+
+        def factory(url, token, **kwargs):
+            c = MagicMock()
+            c.is_connected = True
+            c.connect = AsyncMock(return_value=True)
+            clients.append(c)
+            return c
+
+        manager = WebSocketManager()
+        manager.configure(client_factory=factory)
+
+        a = await manager.get_client(url="http://ha.local:8123", token="tok")
+        b = await manager.get_client(
+            url="http://ha.local:8123", token="tok", verify_ssl=not default
+        )
+        assert a is not b
+        assert len(clients) == 2

tests/src/unit/test_radio_management.py

@@ -572,3 +572,23 @@ async def test_resolve_entity_device_not_found(self):
         # Uses the targeted get keyed by the requested entity_id.
         reg = [m for m in record if m["type"] == "config/entity_registry/get"]
         assert reg and reg[0]["entity_id"] == "light.ghost"
+
+
+@pytest.mark.asyncio
+async def test_resolve_entity_device_connection_failure_not_entity_not_found():
+    """A transport-shaped registry-get failure must surface as a connection
+    error, never ENTITY_NOT_FOUND (issue #1832 review)."""
+    client = _client(
+        {
+            "config/entity_registry/get": {
+                "success": False,
+                "error": "Failed to connect to Home Assistant WebSocket",
+            }
+        },
+    )
+    radio = _capture(register_radio_tools, client)["ha_manage_radio"]
+    with pytest.raises(ToolError) as exc:
+        await radio(radio="matter", action="diagnostics", entity_id="light.real")
+    msg = str(exc.value)
+    assert "CONNECTION_FAILED" in msg
+    assert "ENTITY_NOT_FOUND" not in msg