fix: route addon log fetches directly to supervisor on addon installs

Addresses homeassistant-ai#1116. The HA Core proxy at /api/hassio/addons/{slug}/logs
returns 403 against SUPERVISOR_TOKEN on current HA Core releases. Route
around HA Core entirely on add-on installs by hitting the Supervisor
REST API at http://supervisor/addons/{slug}/logs directly — same
pattern tools_bug_report.py already uses for self-logs.

The HA-Core-proxy path stays as the fallback for non-addon installs
(Docker/pip with admin LLA). Branching gate is the existing
is_running_in_addon() helper in _version.py — no fourth duplicate of
the SUPERVISOR_TOKEN env-var check.

Test coverage pins both branches plus the gate selection — see new
TestGetAddonLogsViaSupervisor and TestGetAddonLogsBranchSelection
classes in test_tools_utility_supervisor_logs.py.

Author: Patch76

src/ha_mcp/client/rest_client.py

@@ -5,11 +5,13 @@
 import asyncio
 import json
 import logging
+import os
 import ssl
 from typing import Any
 
 import httpx
 
+from .._version import is_running_in_addon
 from ..config import get_global_settings
 
 
@@ -434,28 +436,86 @@ async def get_error_log(self) -> str:
         return response if isinstance(response, str) else str(response)
 
     async def get_addon_logs(self, slug: str) -> str:
-        """Fetch an add-on's container logs via HA Core's Supervisor REST proxy.
+        """Fetch an add-on's container logs.
 
-        Uses `/api/hassio/addons/{slug}/logs`, which HA Core proxies to
-        Supervisor and returns as `text/plain`. This avoids the
-        `supervisor/api` websocket path that tries to JSON-decode the text
-        body and always fails (see #950).
+        On add-on installs (``SUPERVISOR_TOKEN`` env present), goes directly to
+        the Supervisor REST API at ``http://supervisor/addons/{slug}/logs``
+        with the Supervisor token. The HA Core proxy at
+        ``/api/hassio/addons/{slug}/logs`` rejects this token+path combination
+        on current HA Core releases (see #1116) — the direct path bypasses
+        HA Core entirely and is the documented Supervisor contract.
+
+        On non-addon installs (Docker, pyinstaller, pip pointing at a normal
+        HA URL), falls back to the HA Core proxy path. That path requires an
+        admin LLA but works fine when not invoked from the add-on container.
+
+        Both branches return ``text/plain`` log content.
 
         Raises:
-            HomeAssistantAuthError: 401 from HA Core.
+            HomeAssistantAuthError: 401 response.
             HomeAssistantAPIError: Non-2xx response (e.g. 404 unknown slug,
-                400 addon not installed). `status_code` is set so callers
+                400 addon not installed). ``status_code`` is set so callers
                 can map to specific suggestions.
             HomeAssistantConnectionError: Network, timeout, or transport error.
         """
-        logger.debug(f"Fetching addon logs for slug={slug}")
+        if is_running_in_addon():
+            return await self._get_addon_logs_via_supervisor(slug)
+
+        logger.debug(f"Fetching addon logs for slug={slug} via HA Core proxy")
         response = await self._raw_request(
             "GET",
             f"/hassio/addons/{slug}/logs",
             headers={"Accept": "text/plain"},
         )
         return response.text
 
+    async def _get_addon_logs_via_supervisor(self, slug: str) -> str:
+        """Direct Supervisor REST API fetch for add-on installs.
+
+        Mirrors the access pattern used by ``tools_bug_report._fetch_addon_logs``:
+        a fresh ``httpx.AsyncClient`` against ``http://supervisor`` authed with
+        the Supervisor token. Bypasses ``HomeAssistantClient.httpx_client``
+        because the Supervisor endpoint takes a different base URL and a
+        different token than the HA Core REST API.
+        """
+        token = os.environ.get("SUPERVISOR_TOKEN", "")
+        url = f"http://supervisor/addons/{slug}/logs"
+        logger.debug(f"Fetching addon logs for slug={slug} via Supervisor direct")
+
+        try:
+            async with httpx.AsyncClient(
+                timeout=httpx.Timeout(self.timeout)
+            ) as client:
+                response = await client.get(
+                    url,
+                    headers={
+                        "Authorization": f"Bearer {token}",
+                        "Accept": "text/plain",
+                    },
+                )
+        except httpx.TimeoutException as e:
+            raise HomeAssistantConnectionError(
+                f"Request timeout fetching addon logs: {e}"
+            ) from e
+        except httpx.HTTPError as e:
+            raise HomeAssistantConnectionError(
+                f"HTTP error fetching addon logs: {e}"
+            ) from e
+
+        if response.status_code == 401:
+            raise HomeAssistantAuthError(
+                "Invalid Supervisor token for /addons/<slug>/logs"
+            )
+        if response.status_code >= 400:
+            text_body = response.text
+            message = text_body.strip() or response.reason_phrase or "<empty body>"
+            raise HomeAssistantAPIError(
+                f"API error: {response.status_code} - {message}",
+                status_code=response.status_code,
+                response_data={"message": text_body},
+            )
+        return response.text
+
     async def test_connection(self) -> tuple[bool, str | None]:
         """
         Test connection to Home Assistant.

tests/src/unit/test_tools_utility_supervisor_logs.py

@@ -43,6 +43,32 @@ def mock_client():
         return client
 
 
+@pytest.fixture
+def non_addon_install():
+    """Force `is_running_in_addon()` False so `get_addon_logs` takes the
+    HA-Core-proxy fallback path. Required for tests that mock
+    `httpx_client.request` — the Supervisor-direct branch uses a fresh
+    `httpx.AsyncClient` and would bypass that mock entirely.
+    """
+    with patch(
+        "ha_mcp.client.rest_client.is_running_in_addon", return_value=False
+    ):
+        yield
+
+
+@pytest.fixture
+def addon_install():
+    """Force `is_running_in_addon()` True and stub `SUPERVISOR_TOKEN` so
+    `get_addon_logs` takes the direct Supervisor REST API path."""
+    with (
+        patch(
+            "ha_mcp.client.rest_client.is_running_in_addon", return_value=True
+        ),
+        patch.dict("os.environ", {"SUPERVISOR_TOKEN": "supervisor-token-test"}),
+    ):
+        yield
+
+
 def _register_and_collect(client: Any) -> dict[str, Any]:
     """Register utility tools on a collector mcp and return the registered tools.
 
@@ -69,7 +95,19 @@ def _parse_tool_error(exc_info: pytest.ExceptionInfo[ToolError]) -> dict[str, An
 
 
 class TestGetAddonLogs:
-    """Tests for the REST-client `get_addon_logs` method (the core fix)."""
+    """Tests for the REST-client `get_addon_logs` method on non-addon installs.
+
+    These exercise the HA-Core-proxy fallback branch (`/hassio/addons/{slug}/logs`)
+    via `httpx_client.request`. The `non_addon_install` fixture forces
+    `is_running_in_addon()` False so `get_addon_logs` doesn't take the
+    Supervisor-direct branch — that path opens a fresh `httpx.AsyncClient`
+    and would bypass the `mock_client.httpx_client` mock entirely.
+    """
+
+    @pytest.fixture(autouse=True)
+    def _force_non_addon(self, non_addon_install):
+        """Apply `non_addon_install` to every test in this class."""
+        yield
 
     @pytest.mark.asyncio
     async def test_returns_text_on_200(self, mock_client):
@@ -167,6 +205,202 @@ async def test_does_not_parse_json(self, mock_client):
         mock_response.json.assert_not_called()
 
 
+class TestGetAddonLogsViaSupervisor:
+    """Tests for the Supervisor-direct branch of `get_addon_logs`.
+
+    Regression coverage for #1116: on add-on installs, the HA-Core-proxy path
+    `/api/hassio/addons/{slug}/logs` returns 403 for every slug because HA Core
+    rejects the Supervisor-token+route combination. The fix routes around HA
+    Core entirely on add-on installs by hitting the documented Supervisor REST
+    API at `http://supervisor/addons/{slug}/logs` with the Supervisor token.
+    """
+
+    @pytest.fixture
+    def mock_async_client_class(self):
+        """Patch `httpx.AsyncClient` (the class) inside `rest_client` so the
+        ``async with httpx.AsyncClient(...) as client:`` block returns a
+        controllable mock client. Yields the inner client mock so each test
+        can configure ``.get`` directly.
+        """
+        inner_client = MagicMock()
+        inner_client.get = AsyncMock()
+
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=inner_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        client_class = MagicMock(return_value=cm)
+        with patch(
+            "ha_mcp.client.rest_client.httpx.AsyncClient", client_class
+        ):
+            yield inner_client, client_class
+
+    @pytest.mark.asyncio
+    async def test_uses_direct_supervisor_url_and_supervisor_token(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        """The fix's primary contract: on add-on installs, the request must go
+        to `http://supervisor/addons/{slug}/logs` with `Authorization: Bearer
+        ${SUPERVISOR_TOKEN}` — NOT through HA Core's `/api/hassio/...` proxy
+        with the user's HA token.
+        """
+        inner_client, _ = mock_async_client_class
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        mock_response.text = "addon log line 1\nready\n"
+        inner_client.get.return_value = mock_response
+
+        result = await mock_client.get_addon_logs("81f33d0f_ha_mcp")
+
+        assert "addon log line 1" in result
+        inner_client.get.assert_awaited_once()
+        args, kwargs = inner_client.get.call_args
+        assert args[0] == "http://supervisor/addons/81f33d0f_ha_mcp/logs"
+        assert (
+            kwargs["headers"]["Authorization"] == "Bearer supervisor-token-test"
+        )
+        assert kwargs["headers"]["Accept"] == "text/plain"
+        # Critically: the HA-Core-proxy path must NOT have been touched.
+        mock_client.httpx_client.request.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_raises_auth_error_on_401(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        """401 from Supervisor (bad/expired token) maps to HomeAssistantAuthError
+        the same way the HA-Core-proxy path does — so error mapping in the
+        wrapper layer stays uniform regardless of which branch ran.
+        """
+        inner_client, _ = mock_async_client_class
+        mock_response = MagicMock()
+        mock_response.status_code = 401
+        mock_response.text = "unauthorized"
+        inner_client.get.return_value = mock_response
+
+        with pytest.raises(HomeAssistantAuthError):
+            await mock_client.get_addon_logs("core_mosquitto")
+
+    @pytest.mark.asyncio
+    async def test_raises_api_error_on_404(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        """404 (unknown slug) propagates as HomeAssistantAPIError with the
+        Supervisor's plain-text body in the message — preserves the same
+        error shape callers expect from the HA-Core-proxy branch."""
+        inner_client, _ = mock_async_client_class
+        mock_response = MagicMock()
+        mock_response.status_code = 404
+        mock_response.text = "Addon is not installed"
+        mock_response.reason_phrase = "Not Found"
+        inner_client.get.return_value = mock_response
+
+        with pytest.raises(HomeAssistantAPIError) as exc_info:
+            await mock_client.get_addon_logs("nonexistent_slug")
+
+        assert exc_info.value.status_code == 404
+        assert "Addon is not installed" in str(exc_info.value)
+
+    @pytest.mark.asyncio
+    async def test_empty_body_falls_back_to_reason_phrase(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        """If Supervisor returns a non-2xx with empty body, the error tail
+        falls back to the HTTP reason phrase rather than landing as
+        ``"API error: 5xx - "`` with a blank tail."""
+        inner_client, _ = mock_async_client_class
+        mock_response = MagicMock()
+        mock_response.status_code = 502
+        mock_response.text = ""
+        mock_response.reason_phrase = "Bad Gateway"
+        inner_client.get.return_value = mock_response
+
+        with pytest.raises(HomeAssistantAPIError) as exc_info:
+            await mock_client.get_addon_logs("core_mosquitto")
+
+        assert exc_info.value.status_code == 502
+        assert "Bad Gateway" in str(exc_info.value)
+        assert not str(exc_info.value).endswith(" - ")
+
+    @pytest.mark.asyncio
+    async def test_raises_connection_error_on_timeout(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        inner_client, _ = mock_async_client_class
+        inner_client.get.side_effect = httpx.TimeoutException("supervisor timeout")
+
+        with pytest.raises(HomeAssistantConnectionError):
+            await mock_client.get_addon_logs("core_mosquitto")
+
+    @pytest.mark.asyncio
+    async def test_raises_connection_error_on_network_failure(
+        self, mock_client, addon_install, mock_async_client_class
+    ):
+        inner_client, _ = mock_async_client_class
+        inner_client.get.side_effect = httpx.ConnectError("supervisor unreachable")
+
+        with pytest.raises(HomeAssistantConnectionError):
+            await mock_client.get_addon_logs("core_mosquitto")
+
+
+class TestGetAddonLogsBranchSelection:
+    """The branch decision is made via `is_running_in_addon()`. Pin both
+    directions so a future refactor of the gate (e.g. inlining the env-var
+    check) doesn't silently regress one branch.
+    """
+
+    @pytest.mark.asyncio
+    async def test_non_addon_install_uses_ha_core_proxy(self, mock_client):
+        """`is_running_in_addon()` False → HA-Core-proxy path, no Supervisor URL."""
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        mock_response.text = "via proxy\n"
+        mock_client.httpx_client.request = AsyncMock(return_value=mock_response)
+
+        with patch(
+            "ha_mcp.client.rest_client.is_running_in_addon", return_value=False
+        ):
+            result = await mock_client.get_addon_logs("core_mosquitto")
+
+        assert "via proxy" in result
+        mock_client.httpx_client.request.assert_called_once()
+        args, _ = mock_client.httpx_client.request.call_args
+        assert args[1] == "/hassio/addons/core_mosquitto/logs"
+
+    @pytest.mark.asyncio
+    async def test_addon_install_uses_supervisor_direct(self, mock_client):
+        """`is_running_in_addon()` True → Supervisor-direct path, no HA-Core call."""
+        inner_client = MagicMock()
+        inner_client.get = AsyncMock()
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        mock_response.text = "via supervisor\n"
+        inner_client.get.return_value = mock_response
+
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=inner_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        with (
+            patch(
+                "ha_mcp.client.rest_client.is_running_in_addon",
+                return_value=True,
+            ),
+            patch.dict(
+                "os.environ", {"SUPERVISOR_TOKEN": "supervisor-token-branch"}
+            ),
+            patch(
+                "ha_mcp.client.rest_client.httpx.AsyncClient",
+                return_value=cm,
+            ),
+        ):
+            result = await mock_client.get_addon_logs("core_mosquitto")
+
+        assert "via supervisor" in result
+        # HA-Core-proxy must NOT have been called.
+        mock_client.httpx_client.request.assert_not_called()
+        inner_client.get.assert_awaited_once()
+
+
 class TestRawRequestEmptyBodyFallback:
     """Error message must stay actionable even when the 4xx body is empty.