fix: HA Core proxy fallback for ha_get_logs(source=system_service) on non-addon installs (#1283)

* fix: HA Core proxy fallback for ha_get_logs(source=system_service) on non-addon installs

Closes #1260. Pre-fix `_get_system_service_logs` had only the Supervisor-direct
branch, so non-addon installs (the Docker image, uvx ha-mcp, pip-based deploys)
fell straight through to the SUPERVISOR_TOKEN-absent fail-fast in
`_supervisor_logs_get` for every system-service slug. Sibling `get_addon_logs`
and `get_error_log` already had the `is_running_in_addon()` gate plus HA Core
proxy fallback added in PR #1126; this PR closes the parallel gap.

The reproduced symptom matched the issue exactly on a non-addon install
(uvx ha-mcp pointed at a Supervisor-equipped HA): `source="supervisor"` worked
via the existing Core proxy fallback, while `source="system_service"` with
slug in {supervisor, host, core} returned AUTH_INVALID_TOKEN with the
misleading "addon-mode gate fired but SUPERVISOR_TOKEN env var not set"
message. Once the gate is added, that fail-fast string is accurate again
because the only callers reaching it are addon-mode-confirmed.

Verified the proxy path is reachable for all seven service slugs by reading
HA Core's `homeassistant/components/hassio/http.py` — PATHS_ADMIN whitelists
`{audio,cli,core,dns,host,multicast,observer,supervisor}/logs` plus
`addons/{slug}/logs`. Admin LLA is sufficient. (HA Core also proxies
`cli/logs`, which ha-mcp's SYSTEM_SERVICE_SLUGS doesn't include — leaving
that out as scope for a separate PR if anyone wants CLI logs surfaced.)

Tests: new TestGetSystemServiceLogsBranchSelection class mirrors the
existing TestGetAddonLogsBranchSelection / TestGetErrorLogBranchSelection
shape — non-addon branch parametrized over all seven slugs, plus an
addon-branch test that proxy is not called.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(rest_client): Accept header + 404 path on system_service non-addon branch

Two parity additions in TestGetSystemServiceLogsBranchSelection, per PR
review by pr-test-analyzer:

- Parametrized happy-path test now asserts `Accept: text/plain` on the
  proxy request. Without it the HA Core proxy negotiates application/json
  and the body stops being raw log text (same silent-failure signature
  #950 describes one layer up).
- New `test_non_addon_install_404_raises_api_error_with_service_context`
  anchors the live "observer returned 404 on hubs that don't run it"
  case from the #1260 end-to-end verification. Guards against a future
  refactor wrapping the proxy call in a swallow-and-return-empty
  try/except in `_get_system_service_logs`. Parallels
  `TestGetAddonLogs::test_raises_api_error_on_404_with_slug_context`.

64 tests pass in this file (was 63), ruff clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(get_logs): branch-aware error suggestions + add cli to system_service slugs

Two follow-ons from the PR #1283 review, both addressing review findings
that were initially deferred as "future improvements" but are real
behavior issues introduced or exposed by this PR's branch split:

1. Branch-aware wrapper suggestions in tools_utility.py:

   Pre-fix, the AuthError and 403 branches in `_get_supervisor_log` and
   `_get_system_service_log` always emitted SUPERVISOR_TOKEN/hassio_role
   hints — useless on non-addon installs that hit the new HA Core proxy
   path. With #1283's gate-and-fallback split, the wrapper now gates on
   `is_running_in_addon()` and picks branch-appropriate suggestions:

   - In-addon AuthError → "Verify SUPERVISOR_TOKEN..." (unchanged)
   - Non-addon AuthError → "Verify HOMEASSISTANT_TOKEN is a valid admin
     Long-Lived Access Token..."
   - In-addon 403 (system_service) → "Addon's hassio_role must be
     'manager'..." (unchanged)
   - Non-addon 403 (system_service) → "The LLA must belong to an admin..."

   Same shape applied to `_get_supervisor_log` so source="supervisor"
   doesn't have the same dead-end advice on Docker/uvx installs either.

2. Add `cli` to SYSTEM_SERVICE_SLUGS:

   HA Core's hassio HTTP proxy already whitelists `cli/logs` in
   PATHS_ADMIN. Adding the slug surfaces Supervisor CLI logs via the
   same routes #1116 set up for the other seven services. Eight slugs
   total now.

Tests:
- Parametrized non-addon proxy test extended to 8 slugs (cli added)
- `test_all_seven_allowed_services_dispatch` renamed → `_all_allowed_`,
  extended to 8 slugs
- Existing `test_403_role_hint_suggestion` renamed to clarify it's the
  in-addon case + explicit `is_running_in_addon` mock added
- 5 new tests pinning the branch-aware suggestion behavior (in-addon
  AuthError, non-addon AuthError, non-addon 403) on both wrappers
- Supervisor mock updated to recognize the 8-slug set
- Backtick consistency in TestGetAddonLogsBranchSelection +
  TestGetSystemServiceLogsBranchSelection class docstrings, per
  comment-analyzer S3

71 unit tests pass in test_tools_utility_supervisor_logs.py (was 63 pre-PR,
64 after first review-pass test additions). Ruff clean across all
touched files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: kingpanther13

src/ha_mcp/client/rest_client.py

@@ -527,7 +527,7 @@ async def _supervisor_logs_get(self, path: str) -> str:
 
         - ``"addons/<slug>"`` for add-on container logs
         - ``"<service>"`` (where service ∈ {supervisor, host, core, dns, audio,
-          multicast, observer}) for system-service logs
+          cli, multicast, observer}) for system-service logs
 
         Bypasses ``HomeAssistantClient.httpx_client`` because the Supervisor
         endpoint uses a different base URL (``http://supervisor``) and a
@@ -645,18 +645,42 @@ async def _get_addon_logs_via_supervisor(self, slug: str) -> str:
         return await self._supervisor_logs_get(f"addons/{slug}")
 
     async def _get_system_service_logs(self, service: str) -> str:
-        """Fetch HA system-service logs directly from Supervisor's REST API.
-
-        Hits ``http://supervisor/{service}/logs``. ``service`` must be one of
-        the seven Supervisor-managed services: ``supervisor``, ``host``,
-        ``core``, ``dns``, ``audio``, ``multicast``, ``observer``. Caller is
-        responsible for validating ``service`` against the allowed set; this
-        helper does no validation and will raise ``HomeAssistantAPIError`` on
-        any unknown path (404 from Supervisor).
-
-        Requires ``hassio_role: manager`` like the addon-logs path.
+        """Fetch HA system-service logs.
+
+        ``service`` must be one of the eight Supervisor-managed services:
+        ``supervisor``, ``host``, ``core``, ``dns``, ``audio``, ``cli``,
+        ``multicast``, ``observer``. Caller is responsible for validating
+        ``service`` against the allowed set; this helper does no validation
+        and will raise ``HomeAssistantAPIError`` on any unknown path (404).
+
+        Branch on ``is_running_in_addon()`` — mirror of ``get_addon_logs``:
+        inside the addon container goes directly to Supervisor at
+        ``http://supervisor/{service}/logs`` with the Supervisor token
+        (``hassio_role: manager`` required). On non-addon installs (Docker
+        without Supervisor, pyinstaller, pip pointing at a normal HA URL),
+        falls back to the HA Core proxy at ``/api/hassio/{service}/logs``.
+
+        All seven slugs are whitelisted in HA Core's hassio proxy
+        (``homeassistant/components/hassio/http.py`` — ``PATHS_ADMIN``), so
+        an admin LLA is sufficient to reach any of them from outside the
+        addon.
+
+        Closes #1260: pre-fix this method had only the addon-direct branch,
+        so non-addon installs (the Docker image, uvx ha-mcp, etc.) hit the
+        ``SUPERVISOR_TOKEN``-absent fail-fast in ``_supervisor_logs_get`` for
+        every service, while the sibling ``source="supervisor"`` (addon
+        logs) call kept working through its own Core-proxy fallback.
         """
-        return await self._supervisor_logs_get(service)
+        if is_running_in_addon():
+            return await self._supervisor_logs_get(service)
+
+        logger.debug(f"Fetching {service} logs via HA Core proxy")
+        response = await self._raw_request(
+            "GET",
+            f"/hassio/{service}/logs",
+            headers={"Accept": "text/plain"},
+        )
+        return response.text
 
     async def test_connection(self) -> tuple[bool, str | None]:
         """

src/ha_mcp/tools/tools_utility.py

@@ -12,6 +12,7 @@
 
 from fastmcp.exceptions import ToolError
 
+from .._version import is_running_in_addon
 from ..client.rest_client import (
     HomeAssistantAPIError,
     HomeAssistantAuthError,
@@ -42,11 +43,12 @@
 }
 
 
-# Supervisor-managed system services exposed via /<slug>/logs. Stable set
-# in HA Core; if Supervisor adds e.g. /cli/logs in a future release, extend
-# here. See #1116.
+# Supervisor-managed system services exposed via /<slug>/logs. Set mirrors
+# HA Core's hassio HTTP proxy ``PATHS_ADMIN`` whitelist in
+# ``homeassistant/components/hassio/http.py``. See #1116 (original 7-service
+# scope) and #1260 (cli added — proxy supported it the whole time).
 SYSTEM_SERVICE_SLUGS = frozenset(
-    {"supervisor", "host", "core", "dns", "audio", "multicast", "observer"}
+    {"supervisor", "host", "core", "dns", "audio", "cli", "multicast", "observer"}
 )
 
 
@@ -145,7 +147,7 @@ async def ha_get_logs(
         - "error_log": Raw home-assistant.log text
         - "supervisor": Add-on container logs (requires slug = add-on slug)
         - "system_service": HA-Supervisor-managed system service logs (requires
-          slug ∈ {supervisor, host, core, dns, audio, multicast, observer})
+          slug ∈ {supervisor, host, core, dns, audio, cli, multicast, observer})
         - "logger": Effective log level per integration via logger/log_info (confirms logger.set_level changes took effect)
 
         **Shared params:** limit, search (keyword filter on entries/lines; matches integration domain for source='logger')
@@ -762,15 +764,29 @@ async def _get_supervisor_log(
         except HomeAssistantAuthError as e:
             # Listed before HomeAssistantAPIError because AuthError is a sibling,
             # not a subclass — without this explicit clause the 401 from
-            # _supervisor_logs_get propagates raw to FastMCP and surfaces
-            # without a structured `code` field.
+            # _supervisor_logs_get / _raw_request propagates raw to FastMCP and
+            # surfaces without a structured `code` field.
+            #
+            # Suggestions branch on is_running_in_addon(): addon installs go
+            # direct to Supervisor (the failure mode is a missing/rotated
+            # SUPERVISOR_TOKEN), non-addon installs hit HA Core's hassio
+            # proxy with the user's LLA (the failure mode is a non-admin or
+            # expired LLA — SUPERVISOR_TOKEN doesn't even apply).
+            if is_running_in_addon():
+                suggestions = [
+                    "Verify SUPERVISOR_TOKEN is set correctly inside the add-on",
+                    "Reinstall the add-on if the token may have rotated",
+                ]
+            else:
+                suggestions = [
+                    "Verify HOMEASSISTANT_TOKEN is a valid admin Long-Lived "
+                    "Access Token (Settings → Profile → Long-Lived Access Tokens)",
+                    "Re-create the LLAT if it has expired or been revoked",
+                ]
             exception_to_structured_error(
                 e,
                 context={"source": "supervisor", "slug": slug},
-                suggestions=[
-                    "Verify SUPERVISOR_TOKEN is set correctly inside the add-on",
-                    "Reinstall the add-on if the token may have rotated",
-                ],
+                suggestions=suggestions,
             )
         except HomeAssistantAPIError as e:
             status = getattr(e, "status_code", None)
@@ -832,13 +848,15 @@ async def _get_system_service_log(
     ) -> dict[str, Any]:
         """Fetch HA system-service logs from Supervisor's per-service endpoint.
 
-        ``service`` ∈ {supervisor, host, core, dns, audio, multicast, observer}.
+        ``service`` ∈ ``SYSTEM_SERVICE_SLUGS`` (the eight Supervisor-managed
+        services: supervisor, host, core, dns, audio, cli, multicast, observer).
         Caller (``ha_get_logs(source='system_service')``) validates against
-        ``SYSTEM_SERVICE_SLUGS`` before dispatch. Hits
-        ``http://supervisor/<service>/logs`` directly via
-        ``HomeAssistantClient._get_system_service_logs`` — same direct-Supervisor
-        path #1116's add-on fix uses, just with a different URL prefix.
-        Requires ``hassio_role: manager`` in the addon manifest.
+        ``SYSTEM_SERVICE_SLUGS`` before dispatch. Routed through
+        ``HomeAssistantClient._get_system_service_logs`` which gates on
+        ``is_running_in_addon()``: addon installs hit Supervisor directly at
+        ``http://supervisor/<service>/logs`` (requires ``hassio_role: manager``
+        in the addon manifest), non-addon installs fall back to the HA Core
+        proxy at ``/api/hassio/<service>/logs`` (requires an admin LLA).
         """
         effective_limit = _coerce_limit(
             limit, default=DEFAULT_LOG_LIMIT, suggestion_example="100"
@@ -877,29 +895,53 @@ async def _get_system_service_log(
         except HomeAssistantAuthError as e:
             # Listed before HomeAssistantAPIError because AuthError is a sibling,
             # not a subclass — without this explicit clause the 401 from
-            # _supervisor_logs_get propagates raw to FastMCP and surfaces
-            # without a structured `code` field.
+            # _supervisor_logs_get / _raw_request propagates raw to FastMCP and
+            # surfaces without a structured `code` field.
+            #
+            # Suggestions branch on is_running_in_addon() (see _get_supervisor_log
+            # for the rationale): SUPERVISOR_TOKEN suggestions only make sense
+            # inside the addon container; non-addon installs need admin-LLA hints.
+            if is_running_in_addon():
+                suggestions = [
+                    "Verify SUPERVISOR_TOKEN is set correctly inside the add-on",
+                    "Reinstall the add-on if the token may have rotated",
+                ]
+            else:
+                suggestions = [
+                    "Verify HOMEASSISTANT_TOKEN is a valid admin Long-Lived "
+                    "Access Token (Settings → Profile → Long-Lived Access Tokens)",
+                    "Re-create the LLAT if it has expired or been revoked",
+                ]
             exception_to_structured_error(
                 e,
                 context={"source": "system_service", "slug": service},
-                suggestions=[
-                    "Verify SUPERVISOR_TOKEN is set correctly inside the add-on",
-                    "Reinstall the add-on if the token may have rotated",
-                ],
+                suggestions=suggestions,
             )
         except HomeAssistantAPIError as e:
             status = getattr(e, "status_code", None)
             if status == 403:
-                # Same role-too-low cause as the addon-logs branch.
-                exception_to_structured_error(
-                    e,
-                    context={"source": "system_service", "slug": service},
-                    suggestions=[
+                # In-addon: Supervisor returns 403 when the addon's hassio_role
+                # is below 'manager'. Non-addon: HA Core's hassio proxy returns
+                # 403 when the LLA's user lacks admin — completely different
+                # remediation. Branch on the gate accordingly.
+                if is_running_in_addon():
+                    suggestions = [
                         "Addon's hassio_role must be 'manager' or higher to "
                         "read /<service>/logs",
                         "Verify the addon was reinstalled after the role bump "
                         "took effect",
-                    ],
+                    ]
+                else:
+                    suggestions = [
+                        "The Long-Lived Access Token must belong to a user "
+                        "with admin privileges",
+                        "Generate a new LLAT under an admin account and set "
+                        "HOMEASSISTANT_TOKEN to it",
+                    ]
+                exception_to_structured_error(
+                    e,
+                    context={"source": "system_service", "slug": service},
+                    suggestions=suggestions,
                 )
             if status == 404:
                 exception_to_structured_error(

tests/src/e2e/utilities/supervisor_mock.py

@@ -20,7 +20,7 @@
 Endpoints implemented (only what the code actually calls):
 
 - ``GET /{service}/logs`` for service ∈ {supervisor, host, core, dns, audio,
-  multicast, observer} — the seven Supervisor-managed system services
+  cli, multicast, observer} — the eight Supervisor-managed system services
 - ``GET /addons/{slug}/logs`` and ``GET /addons/self/logs`` — addon container logs
 - ``POST /addons/self/restart`` — addon self-restart (Supervisor envelope reply)
 
@@ -47,10 +47,10 @@
 # need to exercise the role-mismatch branch added alongside #1116.
 MOCK_INSUFFICIENT_ROLE_TOKEN = "test-supervisor-token-low-role"
 
-# The seven Supervisor-managed system services exposed at /<service>/logs.
+# The eight Supervisor-managed system services exposed at /<service>/logs.
 # Mirrors SYSTEM_SERVICE_SLUGS in src/ha_mcp/tools/tools_utility.py.
 SYSTEM_SERVICES = frozenset(
-    {"supervisor", "host", "core", "dns", "audio", "multicast", "observer"}
+    {"supervisor", "host", "core", "dns", "audio", "cli", "multicast", "observer"}
 )
 
 _SERVICE_LOGS_RE = re.compile(r"^/([a-z]+)/logs$")