fix(addon): bound Supervisor error payloads reaching the model (#2280)

* fix(addon): bound Supervisor error payloads reaching the model

The direct Supervisor REST path builds its tool errors out of parsed
response payloads: the two invalid-payload branches embed the payload
repr, the failure branch forwards Supervisor's own message, and the
result mapping embeds a non-mapping result payload. None of those were
bounded, so one oversized Supervisor response could produce a tool
error many times larger than the 50 KiB bound the raw-text paths
already apply.

Route them through that same bound. The helper now takes a string, so
the response-body path and the payload-derived messages share one
implementation and one truncation marker.

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

* fix(addon): bound Core-relayed Supervisor failures too

The first bound sat in _supervisor_rest_failure, which only the direct
REST transport passes through. A standard install routes through
_supervisor_api_call_via_core, whose failed WebSocket result is returned
unchanged, so Supervisor text relayed by Core still reached the model
unbounded.

Move the bound to _raise_supervisor_api_failure, the single point every
non-retryable failure of either transport passes, and cover the Core
route with a regression test.

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

* fix(addon): bound the job-collision give-up message too

Moving the bound to _raise_supervisor_api_failure left one reporting
path uncovered: when a job-group collision exhausts the retry budget,
_supervisor_api_call raises its own error from the raw error_text it
computed for the marker test, so an oversized collision message was
still reported in full. That path was bounded before the move, on the
direct REST transport, so this closes a gap the move opened.

Bind the size after the marker classification is settled, and cover the
give-up path with a regression test.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: Patch76

src/ha_mcp/tools/tools_addons.py

@@ -316,13 +316,17 @@ def _supervisor_invalid_response(
     return _supervisor_rest_failure(response, error)
 
 
+def _bounded_supervisor_text(value: str) -> str:
+    """Return model-safe Supervisor text, marking any truncation visibly."""
+    if len(value) <= _MAX_RESPONSE_SIZE:
+        return value
+    prefix_size = _MAX_RESPONSE_SIZE - len(_SUPERVISOR_RESPONSE_TRUNCATION_SUFFIX)
+    return value[:prefix_size] + _SUPERVISOR_RESPONSE_TRUNCATION_SUFFIX
+
+
 def _bounded_supervisor_response_text(response: httpx.Response) -> str:
     """Return a model-safe Supervisor response body with visible truncation."""
-    body = response.text.strip()
-    if len(body) <= _MAX_RESPONSE_SIZE:
-        return body
-    prefix_size = _MAX_RESPONSE_SIZE - len(_SUPERVISOR_RESPONSE_TRUNCATION_SUFFIX)
-    return body[:prefix_size] + _SUPERVISOR_RESPONSE_TRUNCATION_SUFFIX
+    return _bounded_supervisor_text(response.text.strip())
 
 
 def _normalize_supervisor_rest_response(
@@ -558,7 +562,12 @@ def _raise_supervisor_api_failure(
     endpoint: str,
 ) -> NoReturn:
     """Raise the structured exception represented by a non-retryable result."""
-    error_text = str(result.get("error", f"Supervisor API call failed: {endpoint}"))
+    # Both transports land here, and both carry Supervisor's own text: the
+    # direct REST payload and the message Core relays over the WebSocket
+    # bridge. Bind the size once, where every failure passes.
+    error_text = _bounded_supervisor_text(
+        str(result.get("error", f"Supervisor API call failed: {endpoint}"))
+    )
     status_code = result.get("_status_code")
     response_data = result.get("_response_data")
     if status_code == 401:
@@ -613,7 +622,7 @@ def _supervisor_result_mapping(
         return payload
 
     verb = method.upper()
-    message = (
+    message = _bounded_supervisor_text(
         f"Supervisor API {verb} {endpoint} returned an invalid result payload: "
         f"{payload!r}"
     )
@@ -717,6 +726,10 @@ async def _supervisor_api_call(
             if _JOB_COLLISION_MARKER not in error_text.lower():
                 _raise_supervisor_api_failure(result, endpoint)
 
+            # The marker test runs on the raw text; everything below reports
+            # it, so bind the size once the classification is settled.
+            error_text = _bounded_supervisor_text(error_text)
+
             remaining = deadline - time.monotonic()
             if remaining <= 0:
                 # The retry budget is exhausted; the group may be stuck or

tests/src/unit/test_tools_addons.py

@@ -4623,6 +4623,165 @@ async def test_addon_mode_invalid_json_error_caps_response_body(self, monkeypatc
             "x" * (50 * 1024 - len(suffix)) + suffix
         )
 
+    @pytest.mark.asyncio
+    async def test_job_collision_giveup_message_is_capped(self, monkeypatch):
+        """An exhausted job-collision retry cannot report an unbounded error."""
+        from ha_mcp.tools import tools_addons
+        from ha_mcp.tools.tools_addons import _supervisor_api_call
+
+        monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
+        monkeypatch.setattr(tools_addons, "_JOB_COLLISION_RETRY_WINDOW", 0.0)
+        client = _make_mock_client()
+        client.send_websocket_message = AsyncMock(
+            return_value={
+                "success": False,
+                "error": (
+                    "Command failed: another job is running for job group "
+                    + "v" * (50 * 1024 + 1)
+                ),
+            }
+        )
+
+        with pytest.raises(ToolError) as exc_info:
+            await _supervisor_api_call(client, "/addons/x/restart", method="POST")
+
+        payload = _parse_tool_error(exc_info)
+        suffix = "\n[Supervisor response truncated to 50 KiB]"
+        message = payload["error"]["message"]
+        assert message.endswith(suffix)
+        assert len(message) == 50 * 1024
+
+    @pytest.mark.asyncio
+    async def test_core_routed_failure_message_is_capped(self, monkeypatch):
+        """A Core-relayed Supervisor failure cannot produce an unbounded error."""
+        from ha_mcp.tools.tools_addons import _supervisor_api_call
+
+        monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
+        client = _make_mock_client()
+        client.send_websocket_message = AsyncMock(
+            return_value={
+                "success": False,
+                "error": "Command failed: " + "w" * (50 * 1024 + 1),
+            }
+        )
+
+        with pytest.raises(ToolError) as exc_info:
+            await _supervisor_api_call(client, "/addons")
+
+        payload = _parse_tool_error(exc_info)
+        suffix = "\n[Supervisor response truncated to 50 KiB]"
+        message = payload["error"]["message"]
+        assert message.startswith("Command failed: w")
+        assert message.endswith(suffix)
+        assert len(message) == 50 * 1024
+
+    @pytest.mark.asyncio
+    async def test_addon_mode_non_object_json_caps_error_message(self, monkeypatch):
+        """A valid-JSON non-object body cannot produce an unbounded tool error."""
+        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(
+            200,
+            json=["x" * 1000] * 200,
+        )
+        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")
+
+        payload = _parse_tool_error(exc_info)
+        message = payload["error"]["message"]
+        prefix = "Command failed: Supervisor returned an invalid response: "
+        suffix = "\n[Supervisor response truncated to 50 KiB]"
+        assert message.startswith(prefix)
+        assert message.endswith(suffix)
+        assert len(message) == 50 * 1024
+
+    @pytest.mark.asyncio
+    async def test_addon_mode_error_payload_message_is_capped(self, monkeypatch):
+        """An oversized Supervisor error message is bounded before it is raised."""
+        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(
+            502,
+            json={"result": "error", "message": "y" * (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")
+
+        payload = _parse_tool_error(exc_info)
+        suffix = "\n[Supervisor response truncated to 50 KiB]"
+        message = payload["error"]["message"]
+        # The status-code path prefixes the bounded Supervisor text, so the
+        # constant framing sits outside the bound.
+        prefix = "Command failed: "
+        assert message.startswith(prefix + "y")
+        assert message.endswith(suffix)
+        assert len(message.removeprefix(prefix)) == 50 * 1024
+
+    @pytest.mark.asyncio
+    async def test_addon_mode_non_mapping_result_payload_is_capped(self, monkeypatch):
+        """An oversized non-mapping result payload is bounded before it is raised."""
+        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(
+            200,
+            json={"result": "ok", "data": "z" * (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")
+
+        payload = _parse_tool_error(exc_info)
+        message = payload["error"]["message"]
+        prefix = "Supervisor API GET /addons returned an invalid result payload: "
+        suffix = "\n[Supervisor response truncated to 50 KiB]"
+        assert message.startswith(prefix)
+        assert message.endswith(suffix)
+        assert len(message) == 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."""