fix(policy): drop approve_url, instruct LLM to send user to settings page (#966)

The relative-path approve_url doesn't resolve cleanly through cloudflared
or other reverse-proxy deployment modes — the LLM can't safely hand it
to the user. The user already knows where the Tool Security Policies
tab is (they set the rule from it), and that page lists all pending
approvals, so a per-request URL is unnecessary noise.

- Drop approve_url from USER_APPROVAL_REQUIRED context; keep `token`
  so a caller could correlate but the user doesn't need to act on it.
- Update message + progress text to instruct the LLM to tell the user
  to open the settings UI Tool Security Policies tab.
- Drop the now-unused approval_url_builder param + the
  _settings_secret_prefix plumbing in server.py / settings_ui.py.

Also fix the failing test_defaults (asserted dropped Policy.enabled field)
and the e2e test PUT body that still carried `"enabled": True`.

Author: kingpanther13

src/ha_mcp/policy/middleware.py

@@ -44,14 +44,10 @@ def __init__(
         *,
         policy_provider: Callable[[], Policy],
         queue: ApprovalQueue,
-        approval_url_builder: Callable[[str], str] | None = None,
         wait_seconds: int | None = None,
     ) -> None:
         self._policy_provider = policy_provider
         self._queue = queue
-        self._approval_url_builder = approval_url_builder or (
-            lambda token: f"/settings?tab=tool-security-policies&token={token}"
-        )
         self._wait_override = wait_seconds
 
     async def on_call_tool(
@@ -117,14 +113,13 @@ async def on_call_tool(
             args,
             ttl_minutes=policy.approval_ttl_minutes,
         )
-        approval_url = self._approval_url_builder(pending.token)
 
         wait = (
             self._wait_override
             if self._wait_override is not None
             else policy.wait_seconds
         )
-        await self._wait_for_decision(context, pending, approval_url, wait)
+        await self._wait_for_decision(context, pending, wait)
 
         if pending.decision == "approved":
             self._queue.consume_and_maybe_remember(
@@ -136,13 +131,12 @@ async def on_call_tool(
             self._queue.remove(pending.token)
             raise self._denied_error()
 
-        raise self._pending_error(pending, approval_url)
+        raise self._pending_error(pending)
 
     async def _wait_for_decision(
         self,
         context: MiddlewareContext,
         pending: PendingApproval,
-        approval_url: str,
         wait_seconds: int,
     ) -> None:
         deadline = anyio.current_time() + wait_seconds
@@ -153,8 +147,9 @@ async def _wait_for_decision(
                 progress=0,
                 total=0,
                 message=(
-                    f"Awaiting user approval — open this URL to review the call "
-                    f"in the Tool Security Policies tab and approve it: {approval_url}"
+                    "Awaiting user approval — open the ha-mcp settings UI, "
+                    "go to the Tool Security Policies tab, and approve or deny "
+                    "the pending request."
                 ),
             )
             remaining = deadline - anyio.current_time()
@@ -180,7 +175,7 @@ def _denied_error() -> ToolError:
             )
         )
 
-    def _pending_error(self, pending: PendingApproval, approval_url: str) -> ToolError:
+    def _pending_error(self, pending: PendingApproval) -> ToolError:
         # Time-remaining, not total TTL: an LLM that re-calls a minute
         # before expiry should see "~60s left", not the original 300s.
         remaining = max(
@@ -193,16 +188,17 @@ def _pending_error(self, pending: PendingApproval, approval_url: str) -> ToolErr
                     "error": {
                         "code": "USER_APPROVAL_REQUIRED",
                         "message": (
-                            f"User approval required. Open this URL to review the call "
-                            f"in the Tool Security Policies tab and approve it: {approval_url}. "
-                            "Re-call this tool with the same arguments after the user approves."
+                            "User approval required. Tell the user to open the "
+                            "ha-mcp settings UI, go to the Tool Security Policies "
+                            "tab, and approve or deny the pending request. Re-call "
+                            "this tool with the same arguments after the user approves."
                         ),
                         "context": {
-                            "approve_url": approval_url,
+                            "token": pending.token,
                             "expires_in_seconds": remaining,
                         },
                         "suggestions": [
-                            "Tell the user to click the approval link.",
+                            "Tell the user to open the Tool Security Policies tab in the ha-mcp settings UI and approve the pending request.",
                             "Re-call this tool with the same arguments after the user approves.",
                         ],
                     },

src/ha_mcp/server.py

@@ -95,11 +95,6 @@ def __init__(
         # Used by _apply_tool_search to remove default-pinned tools from
         # the always_visible set so users can unpin defaults (#966).
         self._user_enabled_tools: set[str] = set()
-        # Set by register_settings_routes (settings_ui.py) when the
-        # settings UI routes are mounted under the secret-prefixed path.
-        # Declared on __init__ so pyright doesn't flag the external
-        # assignment as reportAttributeAccessIssue.
-        self._settings_secret_prefix: str = ""
 
         # Get server name/version from settings if no client provided
         if not self._client_provided:
@@ -892,23 +887,11 @@ def _policy_provider() -> Policy:
             # roundtrip of a gated tool call.
             return load_policy(data_dir)
 
-        # The secret prefix (e.g. ``/private_xxx``) is wired in lazily
-        # by ``settings_ui.register_settings_routes`` after the HTTP
-        # entrypoint resolves ``MCP_SECRET_PATH``. Read at URL-build
-        # time so the closure picks up the value once it's set.
-        # Empty-string fallback (add-on mode + root-mount + stdio) keeps
-        # the existing relative-URL behavior, which already resolves
-        # correctly against the settings page the user navigates to.
-        def _approval_url(token: str) -> str:
-            prefix = getattr(self, "_settings_secret_prefix", "") or ""
-            return f"{prefix}/settings?tab=tool-security-policies&token={token}"
-
         try:
             self.mcp.add_middleware(
                 PolicyMiddleware(
                     policy_provider=_policy_provider,
                     queue=self.approval_queue,
-                    approval_url_builder=_approval_url,
                 )
             )
             logger.info(

src/ha_mcp/settings_ui.py

@@ -3939,17 +3939,6 @@ def register_settings_routes(
     secret_prefix = secret_path.rstrip("/") if secret_path else ""
     is_addon = is_running_in_addon()
 
-    # Expose the resolved prefix to the server so the tool security policies
-    # middleware can build absolute-looking approval URLs (#966). The
-    # middleware reads this lazily via ``getattr(self,
-    # "_settings_secret_prefix", "")`` so the closure picks up the value
-    # set here, even though ``_apply_tool_security_policies`` ran in __init__
-    # before this function was called. Skip when ``server is None`` (sidecar
-    # / unit-test shape) — the policy middleware isn't registered in that
-    # mode anyway, and a Mock-less ``None`` would raise AttributeError here.
-    if server is not None:
-        server._settings_secret_prefix = secret_prefix
-
     if not is_addon and not secret_prefix:
         logger.warning(
             "register_settings_routes: not in add-on mode and no secret_path "

tests/src/e2e/policy/test_approval_flow.py

@@ -144,7 +144,6 @@ async def test_blocked_call_then_approve_then_recall(policy_enabled_mcp):
     current_resp = await handlers["policy_get_config"](_make_request())
     current = json.loads(current_resp.body)
     new_policy = {
-        "enabled": True,
         "wait_seconds": 5,
         "approval_ttl_minutes": 5,
         "rules": [

tests/src/unit/policy/test_middleware.py

@@ -155,7 +155,8 @@ async def test_timeout_raises_pending_error_and_keeps_entry(queue):
         )
     body = json.loads(ei.value.args[0])
     assert body["error"]["code"] == "USER_APPROVAL_REQUIRED"
-    assert "approve_url" in body["error"]["context"]
+    assert body["error"]["context"]["token"]
+    assert "Tool Security Policies" in body["error"]["message"]
     call_next.assert_not_called()
     # entry survives for re-call
     assert queue.list_pending()

tests/src/unit/policy/test_model.py

@@ -86,15 +86,13 @@ def test_remember_minutes_non_negative(self):
 class TestPolicy:
     def test_defaults(self):
         p = Policy()
-        assert p.enabled is False
         assert p.wait_seconds == 60
         assert p.approval_ttl_minutes == 5
         assert p.rules == []
 
     def test_user_example_from_issue(self):
         """Example from the maintainer: ha_call_service approval when domain is lock or alarm_control_panel."""
         p = Policy(
-            enabled=True,
             rules=[
                 Rule(
                     tool_name="ha_call_service",

tests/src/unit/test_server_policy_wiring.py

@@ -20,12 +20,10 @@ def _make_server_stub(*, enable_policies: bool) -> MagicMock:
       * ``self.settings.enable_tool_security_policies`` (early return gate)
       * ``self.approval_queue = ApprovalQueue()`` (attribute write)
       * ``self.mcp.add_middleware(...)`` (the wiring side-effect)
-      * ``self._settings_secret_prefix`` (read by the URL-builder closure)
     """
     stub = MagicMock()
     stub.settings = MagicMock(enable_tool_security_policies=enable_policies)
     stub.mcp = MagicMock()
-    stub._settings_secret_prefix = ""
     # Explicitly start without the attribute the method is supposed to
     # set, so the disabled-case assertion is a real signal.
     del stub.approval_queue