refactor: pass verify_ssl to remaining direct-Supervisor httpx callers (#1128)

* refactor: pass verify_ssl to remaining direct-Supervisor httpx callers

Closes #1127. Mirrors the verify=self.verify_ssl propagation pattern
established in #1126 (rest_client.py:_get_addon_logs_via_supervisor) at
the two other direct-Supervisor httpx call sites:

- tools_bug_report.py:_fetch_addon_logs uses get_global_settings().verify_ssl
  (module-level helper, no self/closure context).
- settings_ui.py:_restart_addon uses server.client.verify_ssl (closure has
  access to server: HomeAssistantSmartMCPServer).

Both paths effectively propagate Settings.verify_ssl via the access route
appropriate to each call site's scope. The http://supervisor URL is plain
HTTP and TLS-irrelevant in practice — the parameter keeps all three
constructor sites consistent with the established HomeAssistantClient
pattern.

* refactor: read verify_ssl from server.settings instead of server.client

Per Gemini review on PR #1128: server.client is a lazy @Property
(server.py) — accessing it for a single config bool would instantiate
the full HomeAssistantClient (httpx pool, settings re-read, log line)
on first access. server.settings is eager-initialized in the
HomeAssistantSmartMCPServer constructor and is the canonical source of
truth for verify_ssl.

Additional benefit: in OAuth deployment mode (__main__.py:868),
HomeAssistantSmartMCPServer is constructed with an OAuthProxyClient
whose __getattr__ proxies to a per-request OAuth client requiring an
authenticated request context. _restart_addon is a plain admin POST
without that context, so server.client.verify_ssl could have surfaced
as an auth error in OAuth mode. server.settings.verify_ssl sidesteps it
without depending on OAuthProxyClient's attribute-forwarding semantics.

* test: pin _restart_addon untested branches per Boy-Scout

Adds unit-test coverage for the two previously-untested branches in
settings_ui.py:_restart_addon:

- Missing SUPERVISOR_TOKEN (settings_ui.py:780-789) — non-addon installs
  hit this when the user clicks Restart against a Docker/pyinstaller
  setup; the structured 400 must surface rather than ever reaching the
  Supervisor URL.
- Connection-drop-as-success (settings_ui.py:798-801) — the Supervisor
  kills our process mid-request during a restart, so a ReadError /
  RemoteProtocolError / ConnectError from the POST is the documented
  success signal.

Mirrors the _capture_handler pattern from TestSaveToolsValidation. The
fixture-level server.settings.verify_ssl = True is required by this
PR's post-G1 access path (httpx accepts only bool/SSLContext for verify=).

Boy-Scout fix while already touching _restart_addon for the
verify_ssl-propagation refactor — closes the test-coverage gap I'd
flagged in #960's approve-body but never followed up on at the time.

---------

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

Author: Patch76

src/ha_mcp/settings_ui.py

@@ -50,6 +50,7 @@ class ToolStub(TypedDict):
     destructiveHint: NotRequired[bool]
     disabled_by: NotRequired[str]
 
+
 _VALID_STATES = frozenset({"enabled", "disabled", "pinned"})
 
 logger = logging.getLogger(__name__)
@@ -261,7 +262,9 @@ def _render_stub(name: str, meta: ToolStub) -> dict[str, Any]:
     return rendered
 
 
-async def _get_tool_metadata(server: HomeAssistantSmartMCPServer) -> list[dict[str, Any]]:
+async def _get_tool_metadata(
+    server: HomeAssistantSmartMCPServer,
+) -> list[dict[str, Any]]:
     """Extract metadata for all registered tools from the server.
 
     Uses FastMCP's internal ``local_provider._list_tools()`` because the
@@ -291,14 +294,16 @@ async def _get_tool_metadata(server: HomeAssistantSmartMCPServer) -> list[dict[s
         title = getattr(tool, "title", None) or tool.name
         if tool.annotations and getattr(tool.annotations, "title", None):
             title = tool.annotations.title
-        tools.append({
-            "name": tool.name,
-            "title": title,
-            "description": (tool.description or "")[:200],
-            "tags": tags,
-            "primary_tag": primary,
-            "annotations": annotations,
-        })
+        tools.append(
+            {
+                "name": tool.name,
+                "title": title,
+                "description": (tool.description or "")[:200],
+                "tags": tags,
+                "primary_tag": primary,
+                "annotations": annotations,
+            }
+        )
 
     registered_names = {t["name"] for t in tools}
 
@@ -362,7 +367,8 @@ def apply_tool_visibility(
     return pinned_names
 
 
-_SETTINGS_HTML = """\
+_SETTINGS_HTML = (
+    """\
 <!DOCTYPE html>
 <html lang="en">
 <head>
@@ -525,8 +531,12 @@ def apply_tool_visibility(
   }
 }
 
-const DEFAULT_PINNED = """ + json.dumps(list(DEFAULT_PINNED_TOOLS)) + """;
-const MANDATORY = """ + json.dumps(list(MANDATORY_TOOLS)) + """;
+const DEFAULT_PINNED = """
+    + json.dumps(list(DEFAULT_PINNED_TOOLS))
+    + """;
+const MANDATORY = """
+    + json.dumps(list(MANDATORY_TOOLS))
+    + """;
 
 function getState(name) {
   if (toolStates[name]) return toolStates[name];
@@ -763,6 +773,7 @@ def apply_tool_visibility(
 </body>
 </html>
 """
+)
 
 
 def register_settings_routes(
@@ -868,15 +879,18 @@ async def _save_tools(request: Request) -> JSONResponse:
         pinned_count = sum(1 for s in states.values() if s == "pinned")
         logger.info(
             "Saved tool config (restart required to apply): %d disabled, %d pinned",
-            disabled_count, pinned_count,
+            disabled_count,
+            pinned_count,
         )
 
-        return JSONResponse({
-            "success": True,
-            "disabled": disabled_count,
-            "pinned": pinned_count,
-            "restart_required": True,
-        })
+        return JSONResponse(
+            {
+                "success": True,
+                "disabled": disabled_count,
+                "pinned": pinned_count,
+                "restart_required": True,
+            }
+        )
 
     async def _restart_addon(_: Request) -> JSONResponse:
         token = os.environ.get("SUPERVISOR_TOKEN")
@@ -892,13 +906,20 @@ async def _restart_addon(_: Request) -> JSONResponse:
         # Short timeout — the supervisor kills our process during restart so
         # the connection will drop. A connection drop is actually success.
         try:
-            async with httpx.AsyncClient(timeout=5.0) as client:
+            async with httpx.AsyncClient(
+                timeout=5.0, verify=server.settings.verify_ssl
+            ) as client:
                 resp = await client.post(
                     "http://supervisor/addons/self/restart",
                     headers={"Authorization": f"Bearer {token}"},
                 )
-        except (httpx.ReadError, httpx.RemoteProtocolError, httpx.ConnectError):
-            # Connection dropped mid-request — restart is happening
+        except (httpx.ReadError, httpx.RemoteProtocolError):
+            # Connection dropped mid-request — restart is happening.
+            # `ConnectError` is deliberately NOT in this tuple: it fires
+            # before a connection is established (DNS failure, TCP refused,
+            # Supervisor socket misconfigured) and means the restart was
+            # never initiated. Falls through to the `httpx.HTTPError`
+            # handler below, which returns 502 + CONNECTION_FAILED.
             logger.info("Restart request connection dropped (expected during restart)")
             return JSONResponse({"success": True, "message": "Restart initiated"})
         except httpx.HTTPError as e:
@@ -924,9 +945,11 @@ async def _restart_addon(_: Request) -> JSONResponse:
         return JSONResponse({"success": True, "message": "Restart initiated"})
 
     async def _settings_info(_: Request) -> JSONResponse:
-        return JSONResponse({
-            "is_addon": is_running_in_addon(),
-        })
+        return JSONResponse(
+            {
+                "is_addon": is_running_in_addon(),
+            }
+        )
 
     secret_prefix = secret_path.rstrip("/") if secret_path else ""
     is_addon = is_running_in_addon()
@@ -958,7 +981,15 @@ async def _settings_info(_: Request) -> JSONResponse:
         # endpoint. The frontend uses relative fetches (./api/settings/...)
         # so the JS works at either prefix unchanged.
         mcp.custom_route(f"{secret_prefix}/settings", methods=["GET"])(_settings_page)
-        mcp.custom_route(f"{secret_prefix}/api/settings/tools", methods=["GET"])(_get_tools)
-        mcp.custom_route(f"{secret_prefix}/api/settings/tools", methods=["POST"])(_save_tools)
-        mcp.custom_route(f"{secret_prefix}/api/settings/restart", methods=["POST"])(_restart_addon)
-        mcp.custom_route(f"{secret_prefix}/api/settings/info", methods=["GET"])(_settings_info)
+        mcp.custom_route(f"{secret_prefix}/api/settings/tools", methods=["GET"])(
+            _get_tools
+        )
+        mcp.custom_route(f"{secret_prefix}/api/settings/tools", methods=["POST"])(
+            _save_tools
+        )
+        mcp.custom_route(f"{secret_prefix}/api/settings/restart", methods=["POST"])(
+            _restart_addon
+        )
+        mcp.custom_route(f"{secret_prefix}/api/settings/info", methods=["GET"])(
+            _settings_info
+        )

src/ha_mcp/tools/tools_bug_report.py

@@ -19,6 +19,7 @@
 
 from ha_mcp import __version__
 
+from ..config import get_global_settings
 from ..utils.usage_logger import (
     AVG_LOG_ENTRIES_PER_TOOL,
     get_recent_logs,
@@ -181,7 +182,9 @@ async def _fetch_addon_logs() -> str:
         return ""
 
     try:
-        async with httpx.AsyncClient(timeout=10.0) as http_client:
+        async with httpx.AsyncClient(
+            timeout=10.0, verify=get_global_settings().verify_ssl
+        ) as http_client:
             resp = await http_client.get(
                 "http://supervisor/addons/self/logs",
                 headers={"Authorization": f"Bearer {token}"},

tests/src/unit/test_settings_ui.py

@@ -10,6 +10,7 @@
 from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import httpx
 import pytest
 from starlette.requests import Request
 from starlette.responses import JSONResponse
@@ -197,9 +198,7 @@ def fake_read_text(self: Path, *args, **kwargs):
         sys.platform == "win32",
         reason="chmod 0o000 doesn't model POSIX EACCES on Windows",
     )
-    def test_load_tool_config_handles_real_eacces_on_posix(
-        self, monkeypatch, tmp_path
-    ):
+    def test_load_tool_config_handles_real_eacces_on_posix(self, monkeypatch, tmp_path):
         """End-to-end variant of the EACCES regression: a real 0o000 dir.
 
         The mocked-``read_text`` test above pins the going-forward contract,
@@ -256,7 +255,9 @@ def test_ha_read_resource_is_advertised(self):
         assert "ha_read_resource" in TRANSFORM_GENERATED_TOOLS
 
     @pytest.mark.asyncio
-    async def test_metadata_includes_ha_resource_tools_when_local_provider_omits_them(self):
+    async def test_metadata_includes_ha_resource_tools_when_local_provider_omits_them(
+        self,
+    ):
         """Closes the gap from #1133: transform tools never reach
         local_provider, so _get_tool_metadata must inject stubs."""
         server = MagicMock()
@@ -433,3 +434,167 @@ async def test_returns_500_when_save_fails(self, monkeypatch, tmp_path):
         body = json.loads(resp.body)
         assert body["success"] is False
         assert "HA_MCP_CONFIG_DIR" in str(body)
+
+
+class TestRestartAddon:
+    """Tests for the `/api/settings/restart` handler — pins the previously
+    untested branches in `_restart_addon`. Boy-Scout pin landed alongside
+    the `verify_ssl` propagation in this PR. Symbol-based references below
+    rather than line numbers, since the kwarg-split here shifts them."""
+
+    def _capture_handler(self, monkeypatch, *, with_token: bool = True) -> SaveHandler:
+        """Capture the `_restart_addon` closure from `register_settings_routes`.
+
+        Mirrors `TestSaveToolsValidation._capture_handler`. `with_token`
+        toggles the env so the no-token branch and the happy-path branches
+        can both be exercised from the same fixture.
+        """
+        if with_token:
+            monkeypatch.setenv("SUPERVISOR_TOKEN", "fake-supervisor-token")
+        else:
+            monkeypatch.delenv("SUPERVISOR_TOKEN", raising=False)
+
+        captured: dict[str, Any] = {}
+
+        def custom_route_factory(path: str, methods: list[str]):
+            def decorator(fn: Any) -> Any:
+                if path.endswith("/api/settings/restart") and "POST" in methods:
+                    captured["restart"] = fn
+                return fn
+
+            return decorator
+
+        mcp = MagicMock()
+        mcp.custom_route = MagicMock(side_effect=custom_route_factory)
+        server = MagicMock()
+        # `_restart_addon` reads `server.settings.verify_ssl` — must resolve
+        # to a real bool, not a MagicMock, because httpx accepts only
+        # bool/SSLContext for `verify=`.
+        server.settings.verify_ssl = True
+        register_settings_routes(mcp, server, secret_path="/x")
+        return captured["restart"]
+
+    @pytest.mark.asyncio
+    async def test_returns_400_without_supervisor_token(self, monkeypatch):
+        """No-token branch (the `if not token:` guard at the top of
+        `_restart_addon`): when SUPERVISOR_TOKEN is unset (non-addon
+        install), the endpoint must surface a structured 400 rather than
+        ever reaching the Supervisor URL.
+        """
+        restart = self._capture_handler(monkeypatch, with_token=False)
+        request = MagicMock()
+
+        resp = await restart(request)
+
+        assert resp.status_code == 400
+        body = json.loads(resp.body)
+        assert body["success"] is False
+        assert body["error"]["code"] == "CONFIG_VALIDATION_FAILED"
+
+    @pytest.mark.asyncio
+    @pytest.mark.parametrize(
+        "exc_cls",
+        [httpx.ReadError, httpx.RemoteProtocolError],
+    )
+    async def test_treats_connection_drop_as_success(self, monkeypatch, exc_cls):
+        """Drop-as-success branch (the catch on
+        `(ReadError, RemoteProtocolError)` inside the `httpx.AsyncClient`
+        block): the Supervisor kills our process mid-request during a
+        restart, so the connection-drop is the documented success signal —
+        not a failure to surface. ConnectError is excluded because it fires
+        BEFORE a connection is established (DNS / TCP refused / socket
+        misconfigured) and means Supervisor was unreachable, not that a
+        restart was initiated.
+        """
+        restart = self._capture_handler(monkeypatch, with_token=True)
+        request = MagicMock()
+
+        # Patch the AsyncClient at the module level so the restart's
+        # `httpx.AsyncClient(...)` block resolves to a controllable mock.
+        mock_client = MagicMock()
+        mock_client.post = AsyncMock(side_effect=exc_cls("kill"))
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=mock_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        with patch("ha_mcp.settings_ui.httpx.AsyncClient", return_value=cm):
+            resp = await restart(request)
+
+        assert resp.status_code == 200
+        body = json.loads(resp.body)
+        assert body["success"] is True
+        assert "Restart initiated" in body["message"]
+
+    @pytest.mark.asyncio
+    async def test_connect_error_returns_502(self, monkeypatch):
+        """ConnectError fires before a connection is established and means
+        Supervisor was unreachable — must NOT be treated as a successful
+        restart. Falls through to the generic `httpx.HTTPError` handler
+        which returns 502 with `CONNECTION_FAILED`.
+        """
+        restart = self._capture_handler(monkeypatch, with_token=True)
+        request = MagicMock()
+
+        mock_client = MagicMock()
+        mock_client.post = AsyncMock(side_effect=httpx.ConnectError("no route"))
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=mock_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        with patch("ha_mcp.settings_ui.httpx.AsyncClient", return_value=cm):
+            resp = await restart(request)
+
+        assert resp.status_code == 502
+        body = json.loads(resp.body)
+        assert body["success"] is False
+        assert body["error"]["code"] == "CONNECTION_FAILED"
+
+    @pytest.mark.asyncio
+    async def test_generic_http_error_returns_502(self, monkeypatch):
+        """The generic `httpx.HTTPError` handler (catches anything not
+        already special-cased) maps to 502 + CONNECTION_FAILED. Pins the
+        last unconvered transport-error path in `_restart_addon`.
+        """
+        restart = self._capture_handler(monkeypatch, with_token=True)
+        request = MagicMock()
+
+        mock_client = MagicMock()
+        # PoolTimeout subclasses httpx.HTTPError but is NOT in the
+        # drop-as-success tuple — exercises the fall-through.
+        mock_client.post = AsyncMock(side_effect=httpx.PoolTimeout("pool full"))
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=mock_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        with patch("ha_mcp.settings_ui.httpx.AsyncClient", return_value=cm):
+            resp = await restart(request)
+
+        assert resp.status_code == 502
+        body = json.loads(resp.body)
+        assert body["success"] is False
+        assert body["error"]["code"] == "CONNECTION_FAILED"
+
+    @pytest.mark.asyncio
+    async def test_supervisor_4xx_returns_502(self, monkeypatch):
+        """When Supervisor returns a non-2xx status (e.g. 401 Unauthorized),
+        the handler must surface a 502 to the caller — the restart was not
+        initiated. Pins the `status_code >= 400` branch in `_restart_addon`.
+        """
+        restart = self._capture_handler(monkeypatch, with_token=True)
+        request = MagicMock()
+
+        response = MagicMock()
+        response.status_code = 401
+        response.text = "Unauthorized"
+        mock_client = MagicMock()
+        mock_client.post = AsyncMock(return_value=response)
+        cm = MagicMock()
+        cm.__aenter__ = AsyncMock(return_value=mock_client)
+        cm.__aexit__ = AsyncMock(return_value=None)
+
+        with patch("ha_mcp.settings_ui.httpx.AsyncClient", return_value=cm):
+            resp = await restart(request)
+
+        assert resp.status_code == 502
+        body = json.loads(resp.body)
+        assert body["success"] is False