Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 60 additions & 29 deletions src/ha_mcp/settings_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ToolStub(TypedDict):
destructiveHint: NotRequired[bool]
disabled_by: NotRequired[str]


_VALID_STATES = frozenset({"enabled", "disabled", "pinned"})

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}

Expand Down Expand Up @@ -362,7 +367,8 @@ def apply_tool_visibility(
return pinned_names


_SETTINGS_HTML = """\
_SETTINGS_HTML = (
"""\
<!DOCTYPE html>
<html lang="en">
<head>
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -763,6 +773,7 @@ def apply_tool_visibility(
</body>
</html>
"""
)


def register_settings_routes(
Expand Down Expand Up @@ -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")
Expand All @@ -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
Comment thread
Patch76 marked this conversation as resolved.
) 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:
Expand All @@ -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()
Expand Down Expand Up @@ -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
)
5 changes: 4 additions & 1 deletion src/ha_mcp/tools/tools_bug_report.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}"},
Expand Down
173 changes: 169 additions & 4 deletions tests/src/unit/test_settings_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
Loading