fix(addons): harden Supervisor REST failures

Author: kingpanther13

src/ha_mcp/tools/tools_addons.py

@@ -66,6 +66,10 @@
 # Maximum response size to return from app (add-on) API calls (50 KB)
 _MAX_RESPONSE_SIZE = 50 * 1024
 
+# Supervisor is local to the app network, so connection acquisition should
+# remain short even when an app operation needs a multi-minute response budget.
+_SUPERVISOR_ACQUIRE_TIMEOUT = 10.0
+
 # Hard safety cap on WebSocket messages collected per call. `message_limit`
 # can lower this but never raise it.
 _MAX_WS_MESSAGES = 1000
@@ -313,8 +317,11 @@ def _normalize_supervisor_rest_response(
 ) -> dict[str, Any]:
     """Normalize a direct Supervisor response and retain write ambiguity."""
     verb = method.upper()
-    if response.status_code >= 500 and verb not in {"GET", "HEAD"}:
-        response_body = response.text.strip()
+    write_outcome_unknown = verb not in {"GET", "HEAD"} and (
+        300 <= response.status_code < 400 or response.status_code >= 500
+    )
+    if write_outcome_unknown:
+        response_body = response.text.strip()[:_MAX_RESPONSE_SIZE]
         _raise_supervisor_write_outcome_unknown(
             ErrorCode.SERVICE_CALL_FAILED,
             f"Supervisor API {verb} {endpoint} returned HTTP "
@@ -479,9 +486,15 @@ async def _supervisor_api_call_once(
         # optional schema by parsing request.json(), so bodyless POST actions
         # must still carry an empty JSON object.
         request_kwargs["json"] = data or {}
+    acquire_timeout = min(wait_timeout, _SUPERVISOR_ACQUIRE_TIMEOUT)
+    transport_timeout = httpx.Timeout(
+        wait_timeout,
+        connect=acquire_timeout,
+        pool=acquire_timeout,
+    )
     try:
         async with make_supervisor_httpx_client(
-            timeout=wait_timeout,
+            timeout=transport_timeout,
             verify=client.verify_ssl,
         ) as supervisor_client:
             response = await supervisor_client.request(
@@ -492,7 +505,7 @@ async def _supervisor_api_call_once(
     except (httpx.ConnectTimeout, httpx.PoolTimeout) as exc:
         raise HomeAssistantConnectionError(
             f"Supervisor API {method.upper()} {endpoint} could not start before "
-            f"the {wait_timeout}s timeout: {exc}"
+            f"the {acquire_timeout}s connection-acquisition timeout: {exc}"
         ) from exc
     except httpx.TimeoutException as exc:
         verb = method.upper()
@@ -737,13 +750,19 @@ async def _supervisor_api_call(
         raise
     except Exception as e:
         logger.error(f"Error calling Supervisor API {endpoint}: {e}")
+        suggestions = None
+        if isinstance(e, HomeAssistantAPIError) and e.status_code == 404:
+            suggestions = [
+                "Check Home Assistant connection and Supervisor availability"
+            ]
         exception_to_structured_error(
             e,
             context={
                 "endpoint": endpoint,
                 "operation": f"Supervisor API {endpoint}",
                 "timeout_seconds": wait_timeout,
             },
+            suggestions=suggestions,
         )
         return None  # unreachable: exception_to_structured_error always raises
 

tests/src/unit/test_tools_addons.py

@@ -4126,7 +4126,13 @@ async def test_addon_mode_uses_direct_supervisor_rest(self, monkeypatch):
             "success": True,
             "result": {"addons": [{"slug": "core_mqtt"}]},
         }
-        factory.assert_called_once_with(timeout=30.0, verify=False)
+        supervisor_timeout = factory.call_args.kwargs["timeout"]
+        assert isinstance(supervisor_timeout, httpx.Timeout)
+        assert supervisor_timeout.connect == 10.0
+        assert supervisor_timeout.read == 30.0
+        assert supervisor_timeout.write == 30.0
+        assert supervisor_timeout.pool == 10.0
+        assert factory.call_args.kwargs["verify"] is False
         direct_client.request.assert_awaited_once_with("GET", "/addons")
         client.send_websocket_message.assert_not_awaited()
 
@@ -4278,6 +4284,11 @@ async def test_addon_mode_preserves_direct_http_status_classification(
             assert "refresh" in suggestions
             assert "hassio_api" in suggestions
             assert "hassio_role" in suggestions
+        if status_code == 404:
+            assert (
+                payload["error"]["suggestion"]
+                == "Check Home Assistant connection and Supervisor availability"
+            )
 
         client.send_websocket_message.assert_not_awaited()
 
@@ -4370,6 +4381,8 @@ async def test_addon_mode_pre_send_write_failure_is_not_ambiguous(
 
         payload = _parse_tool_error(exc_info)
         assert payload["error"]["code"] == expected_code
+        if isinstance(transport_error, (httpx.ConnectTimeout, httpx.PoolTimeout)):
+            assert "10.0s connection-acquisition timeout" in payload["error"]["message"]
         assert "outcome" not in payload
         direct_client.request.assert_awaited_once()
         client.send_websocket_message.assert_not_awaited()
@@ -4512,6 +4525,77 @@ async def test_addon_mode_write_server_error_reports_unknown_outcome(
         assert payload["status_code"] == response.status_code
         assert payload["response_body"] == response.text
 
+    @pytest.mark.asyncio
+    async def test_addon_mode_write_server_error_caps_response_body(self, monkeypatch):
+        """An unknown-outcome error never returns an unbounded response body."""
+        from ha_mcp.tools.tools_addons import _supervisor_api_call
+
+        monkeypatch.setenv("SUPERVISOR_TOKEN", "test-supervisor-token")
+        client = _make_mock_client()
+        client.send_websocket_message = AsyncMock()
+        direct_client = AsyncMock()
+        direct_client.request.return_value = httpx.Response(
+            503,
+            text="x" * (50 * 1024 + 1),
+        )
+        context = MagicMock()
+        context.__aenter__ = AsyncMock(return_value=direct_client)
+        context.__aexit__ = AsyncMock(return_value=False)
+
+        with (
+            patch(
+                "ha_mcp.tools.tools_addons.make_supervisor_httpx_client",
+                return_value=context,
+                create=True,
+            ),
+            pytest.raises(ToolError) as exc_info,
+        ):
+            await _supervisor_api_call(
+                client,
+                "/addons/core_mosquitto/restart",
+                method="POST",
+            )
+
+        payload = _parse_tool_error(exc_info)
+        assert payload["response_body"] == "x" * (50 * 1024)
+
+    @pytest.mark.asyncio
+    async def test_addon_mode_write_redirect_reports_unknown_outcome(self, monkeypatch):
+        """A redirect cannot prove whether Supervisor applied the received write."""
+        from ha_mcp.tools.tools_addons import _supervisor_api_call
+
+        monkeypatch.setenv("SUPERVISOR_TOKEN", "test-supervisor-token")
+        client = _make_mock_client()
+        client.send_websocket_message = AsyncMock()
+        direct_client = AsyncMock()
+        direct_client.request.return_value = httpx.Response(
+            307,
+            headers={"location": "/addons/core_mosquitto/restart"},
+        )
+        context = MagicMock()
+        context.__aenter__ = AsyncMock(return_value=direct_client)
+        context.__aexit__ = AsyncMock(return_value=False)
+
+        with (
+            patch(
+                "ha_mcp.tools.tools_addons.make_supervisor_httpx_client",
+                return_value=context,
+                create=True,
+            ),
+            pytest.raises(ToolError) as exc_info,
+        ):
+            await _supervisor_api_call(
+                client,
+                "/addons/core_mosquitto/restart",
+                method="POST",
+            )
+
+        payload = _parse_tool_error(exc_info)
+        assert payload["error"]["code"] == "SERVICE_CALL_FAILED"
+        assert payload["method"] == "POST"
+        assert payload["outcome"] == "unknown"
+        assert payload["status_code"] == 307
+
     @pytest.mark.asyncio
     @pytest.mark.parametrize(
         "response",
@@ -4826,7 +4910,13 @@ async def test_addon_long_timeout_extends_direct_rest_wait(self, monkeypatch):
                 timeout=1800,
             )
 
-        factory.assert_called_once_with(timeout=1815.0, verify=client.verify_ssl)
+        supervisor_timeout = factory.call_args.kwargs["timeout"]
+        assert isinstance(supervisor_timeout, httpx.Timeout)
+        assert supervisor_timeout.connect == 10.0
+        assert supervisor_timeout.read == 1815.0
+        assert supervisor_timeout.write == 1815.0
+        assert supervisor_timeout.pool == 10.0
+        assert factory.call_args.kwargs["verify"] is client.verify_ssl
         direct_client.request.assert_awaited_once_with(
             "POST", "/addons/core_mosquitto/update", json={}
         )