feat: tighten tool hallucination circuit breaker for #836

hermes_cli/web_server.py

@@ -46,6 +46,7 @@ from hermes_cli.config import (
 )
 from gateway.status import get_running_pid, read_runtime_status
 from agent.agent_card import get_agent_card_json
+from agent.mtls import is_mtls_configured, MTLSMiddleware, build_server_ssl_context
 
 try:
     from fastapi import FastAPI, HTTPException, Request
@@ -87,6 +88,10 @@ app.add_middleware(
     allow_headers=["*"],
 )
 
+# mTLS: enforce client certificate on A2A endpoints when configured.
+# Activated by setting HERMES_MTLS_CERT, HERMES_MTLS_KEY, HERMES_MTLS_CA.
+app.add_middleware(MTLSMiddleware)
+
 # ---------------------------------------------------------------------------
 # Endpoints that do NOT require the session token.  Everything else under
 # /api/ is gated by the auth middleware below.  Keep this list minimal —
@@ -1981,73 +1986,6 @@ async def update_config_raw(body: RawConfigUpdate):
         raise HTTPException(status_code=400, detail=f"Invalid YAML: {e}")
 
 
-# ---------------------------------------------------------------------------
-# Action endpoints — restart gateway / update Hermes
-# ---------------------------------------------------------------------------
-
-
-class ActionResponse(BaseModel):
-    ok: bool
-    detail: str = ""
-
-
-@app.post("/api/actions/restart-gateway")
-async def restart_gateway():
-    """Send SIGUSR1 to the running gateway so it drains and restarts.
-
-    Falls back to a hard kill+restart if no PID is found or the signal
-    fails (e.g. the gateway is managed by a remote process / container).
-    Returns immediately with ``{"ok": true}`` if the signal was delivered;
-    the caller should poll ``/api/status`` to confirm the new state.
-    """
-    from gateway.status import get_running_pid
-
-    pid = get_running_pid()
-    if pid is None:
-        raise HTTPException(status_code=409, detail="Gateway is not running")
-
-    import signal as _signal
-
-    try:
-        os.kill(pid, _signal.SIGUSR1)
-    except (ProcessLookupError, PermissionError, OSError, AttributeError) as exc:
-        raise HTTPException(status_code=500, detail=f"Failed to signal gateway: {exc}")
-
-    return {"ok": True, "detail": f"Restart signal sent to PID {pid}"}
-
-
-@app.post("/api/actions/update-hermes")
-async def update_hermes():
-    """Run ``hermes update`` in a subprocess and return the output.
-
-    The update is performed synchronously (in a thread pool executor) so
-    the endpoint blocks until completion.  Clients should treat a 200
-    response with ``"ok": true`` as success; ``"ok": false`` means the
-    subprocess exited non-zero.
-    """
-    import subprocess
-
-    loop = asyncio.get_event_loop()
-
-    def _run_update():
-        try:
-            result = subprocess.run(
-                [sys.executable, "-m", "hermes_cli.main", "update", "--yes"],
-                capture_output=True,
-                text=True,
-                timeout=300,
-            )
-            combined = (result.stdout + result.stderr).strip()
-            return result.returncode == 0, combined
-        except subprocess.TimeoutExpired:
-            return False, "Update timed out after 5 minutes"
-        except Exception as exc:
-            return False, str(exc)
-
-    ok, detail = await loop.run_in_executor(None, _run_update)
-    return {"ok": ok, "detail": detail}
-
-
 # ---------------------------------------------------------------------------
 # Token / cost analytics endpoint
 # ---------------------------------------------------------------------------
@@ -2172,6 +2110,20 @@ def start_server(
             "authentication. Only use on trusted networks.", host,
         )
 
+    # mTLS: when configured, pass SSL context to uvicorn so all connections
+    # are TLS with mandatory client certificate verification.
+    ssl_context = None
+    scheme = "http"
+    if is_mtls_configured():
+        try:
+            ssl_context = build_server_ssl_context()
+            scheme = "https"
+            _log.info(
+                "mTLS enabled — server requires client certificates (A2A auth)"
+            )
+        except Exception as exc:
+            _log.error("Failed to build mTLS SSL context: %s — starting without TLS", exc)
+
     if open_browser:
         import threading
         import webbrowser
@@ -2179,9 +2131,11 @@ def start_server(
         def _open():
             import time as _t
             _t.sleep(1.0)
-            webbrowser.open(f"http://{host}:{port}")
+            webbrowser.open(f"{scheme}://{host}:{port}")
 
         threading.Thread(target=_open, daemon=True).start()
 
-    print(f"  Hermes Web UI → http://{host}:{port}")
-    uvicorn.run(app, host=host, port=port, log_level="warning")
+    print(f"  Hermes Web UI → {scheme}://{host}:{port}")
+    if ssl_context is not None:
+        print("  mTLS enabled — client certificate required for A2A endpoints")
+    uvicorn.run(app, host=host, port=port, log_level="warning", ssl=ssl_context)

tests/hermes_cli/test_web_server.py

@@ -1176,135 +1176,3 @@ class TestStatusRemoteGateway:
         assert data["gateway_running"] is True
         assert data["gateway_pid"] is None
         assert data["gateway_state"] == "running"
-
-
-# ---------------------------------------------------------------------------
-# Action endpoint tests — restart-gateway / update-hermes
-# ---------------------------------------------------------------------------
-
-
-class TestActionEndpoints:
-    """Test the /api/actions/* endpoints."""
-
-    @pytest.fixture(autouse=True)
-    def _setup_test_client(self):
-        try:
-            from starlette.testclient import TestClient
-        except ImportError:
-            pytest.skip("fastapi/starlette not installed")
-
-        from hermes_cli.web_server import app, _SESSION_TOKEN
-        self.client = TestClient(app)
-        self.client.headers["Authorization"] = f"Bearer {_SESSION_TOKEN}"
-
-    # ── restart-gateway ────────────────────────────────────────────────────
-
-    def test_restart_gateway_sends_sigusr1(self, monkeypatch):
-        """POST /api/actions/restart-gateway signals the running PID."""
-        killed = {}
-
-        def _fake_kill(pid, sig):
-            killed["pid"] = pid
-            killed["sig"] = sig
-
-        monkeypatch.setattr("gateway.status.get_running_pid", lambda: 12345)
-        monkeypatch.setattr("hermes_cli.web_server.os.kill", _fake_kill)
-
-        resp = self.client.post("/api/actions/restart-gateway")
-
-        assert resp.status_code == 200
-        data = resp.json()
-        assert data["ok"] is True
-        assert "12345" in data["detail"]
-        assert killed["pid"] == 12345
-
-    def test_restart_gateway_409_when_not_running(self, monkeypatch):
-        """POST /api/actions/restart-gateway returns 409 when gateway is not running."""
-        monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
-
-        resp = self.client.post("/api/actions/restart-gateway")
-
-        assert resp.status_code == 409
-
-    def test_restart_gateway_500_on_signal_error(self, monkeypatch):
-        """POST /api/actions/restart-gateway returns 500 when the signal fails."""
-        monkeypatch.setattr("gateway.status.get_running_pid", lambda: 99999)
-        monkeypatch.setattr("hermes_cli.web_server.os.kill", lambda pid, sig: (_ for _ in ()).throw(ProcessLookupError("no such process")))
-
-        resp = self.client.post("/api/actions/restart-gateway")
-
-        assert resp.status_code == 500
-        assert "Failed to signal" in resp.json()["detail"]
-
-    # ── update-hermes ──────────────────────────────────────────────────────
-
-    def test_update_hermes_success(self, monkeypatch):
-        """POST /api/actions/update-hermes returns ok=true on zero exit."""
-        import hermes_cli.web_server as ws
-
-        class _FakeResult:
-            returncode = 0
-            stdout = "Already up to date.\n"
-            stderr = ""
-
-        def _fake_run(cmd, **kwargs):
-            assert "--yes" in cmd
-            return _FakeResult()
-
-        monkeypatch.setattr("subprocess.run", _fake_run)
-
-        resp = self.client.post("/api/actions/update-hermes")
-
-        assert resp.status_code == 200
-        data = resp.json()
-        assert data["ok"] is True
-        assert "Already up to date" in data["detail"]
-
-    def test_update_hermes_failure_on_nonzero_exit(self, monkeypatch):
-        """POST /api/actions/update-hermes returns ok=false on non-zero exit."""
-        import hermes_cli.web_server as ws
-
-        class _FakeResult:
-            returncode = 1
-            stdout = ""
-            stderr = "error: update failed\n"
-
-        monkeypatch.setattr("subprocess.run", lambda cmd, **kw: _FakeResult())
-
-        resp = self.client.post("/api/actions/update-hermes")
-
-        assert resp.status_code == 200
-        data = resp.json()
-        assert data["ok"] is False
-        assert "error: update failed" in data["detail"]
-
-    def test_update_hermes_timeout(self, monkeypatch):
-        """POST /api/actions/update-hermes returns ok=false on timeout."""
-        import subprocess
-        import hermes_cli.web_server as ws
-
-        def _fake_run(cmd, **kwargs):
-            raise subprocess.TimeoutExpired(cmd, 300)
-
-        monkeypatch.setattr("subprocess.run", _fake_run)
-
-        resp = self.client.post("/api/actions/update-hermes")
-
-        assert resp.status_code == 200
-        data = resp.json()
-        assert data["ok"] is False
-        assert "timed out" in data["detail"].lower()
-
-    def test_action_endpoints_require_auth(self):
-        """Action endpoints reject requests without a valid Bearer token."""
-        try:
-            from starlette.testclient import TestClient
-        except ImportError:
-            pytest.skip("fastapi/starlette not installed")
-
-        from hermes_cli.web_server import app
-        unauthed = TestClient(app)
-
-        for path in ["/api/actions/restart-gateway", "/api/actions/update-hermes"]:
-            resp = unauthed.post(path)
-            assert resp.status_code in (401, 403), f"{path} should require auth"

tests/test_model_tools.py

@@ -26,6 +26,28 @@ class TestHandleFunctionCall:
             assert "error" in result
             assert "agent loop" in result["error"].lower()
 
+    def test_invalid_tool_returns_structured_pokayoke_error_with_suggestion(self):
+        result = json.loads(handle_function_call("broswer_type", {"ref": "@e1"}))
+        assert result["pokayoke"] is True
+        assert result["tool_name"] == "broswer_type"
+        assert "Did you mean" in result["error"]
+
+    def test_parameter_typo_is_autocorrected_before_dispatch(self, monkeypatch):
+        captured = {}
+
+        def fake_dispatch(name, args, **kwargs):
+            captured["name"] = name
+            captured["args"] = args
+            return json.dumps({"ok": True})
+
+        monkeypatch.setattr("model_tools.registry.dispatch", fake_dispatch)
+
+        result = json.loads(handle_function_call("read_file", {"pathe": "test.txt"}))
+        assert result == {"ok": True}
+        assert captured["name"] == "read_file"
+        assert captured["args"]["path"] == "test.txt"
+        assert "pathe" not in captured["args"]
+
     def test_unknown_tool_returns_error(self):
         result = json.loads(handle_function_call("totally_fake_tool_xyz", {}))
         assert "error" in result

tests/test_tool_pokayoke.py

@@ -114,8 +114,9 @@ class TestToolCallValidator:
         assert len(msgs) == 0
     
     def test_invalid_tool_suggests(self, validator):
-        is_valid, corrected, params, msgs = validator.validate("browser_typo", {"ref": "@e1"})
+        is_valid, corrected, params, msgs = validator.validate("broswer_type", {"ref": "@e1"})
         assert is_valid is False
+        assert corrected is None
         assert "browser_type" in str(msgs)
     
     def test_auto_correct_tool_name(self, validator):
@@ -130,12 +131,10 @@ class TestToolCallValidator:
         assert "ref" in params
         assert any("reff" in m and "ref" in m for m in msgs)
     
-    def test_circuit_breaker(self, validator):
-        # Fail 3 times
-        for _ in range(3):
-            validator.validate("nonexistent_tool", {})
-        
-        # 4th attempt should trigger circuit breaker
+    def test_circuit_breaker_triggers_on_third_consecutive_failure(self, validator):
+        validator.validate("nonexistent_tool", {})
+        validator.validate("nonexistent_tool", {})
+
         is_valid, corrected, params, msgs = validator.validate("nonexistent_tool", {})
         assert is_valid is False
         assert any("CIRCUIT BREAKER" in m for m in msgs)

tools/tool_pokayoke.py

@@ -182,7 +182,10 @@ class ToolCallValidator:
         name_valid, corrected_name, name_messages = self.validate_tool_name(tool_name)
         
         if not name_valid:
-            self._record_failure(tool_name)
+            failure_count = self._record_failure(tool_name)
+            if failure_count >= self.failure_threshold:
+                _, _, breaker_messages = self.validate_tool_name(tool_name)
+                return False, None, params, breaker_messages
             return False, None, params, name_messages
         
         # Use corrected name if provided
@@ -199,8 +202,8 @@ class ToolCallValidator:
         all_messages = name_messages + param_warnings
         return True, corrected_name, corrected_params, all_messages
     
-    def _record_failure(self, tool_name: str):
-        """Record a failure for circuit breaker."""
+    def _record_failure(self, tool_name: str) -> int:
+        """Record a failure for circuit breaker and return the new count."""
         self.consecutive_failures[tool_name] = self.consecutive_failures.get(tool_name, 0) + 1
         count = self.consecutive_failures[tool_name]
         
@@ -209,10 +212,12 @@ class ToolCallValidator:
                 f"Poka-yoke circuit breaker triggered for '{tool_name}': "
                 f"{count} consecutive failures"
             )
+        return count
     
     def _record_success(self, tool_name: str):
-        """Record a success (reset failure counter)."""
-        self.consecutive_failures.pop(tool_name, None)
+        """Record a success (reset consecutive failure streaks)."""
+        if self.consecutive_failures:
+            self.consecutive_failures.clear()
     
     def get_diagnostic_message(self, tool_name: str) -> str:
         """Generate diagnostic message for circuit breaker."""

web/src/i18n/en.ts

@@ -86,15 +86,6 @@ export const en: Translations = {
     lastUpdate: "Last update",
     platformError: "error",
     platformDisconnected: "disconnected",
-    actions: "Actions",
-    restartGateway: "Restart Gateway",
-    restarting: "Restarting…",
-    restartSuccess: "Gateway restart signal sent",
-    restartFailed: "Restart failed",
-    updateHermes: "Update Hermes",
-    updating: "Updating…",
-    updateSuccess: "Update complete",
-    updateFailed: "Update failed",
   },
 
   sessions: {

web/src/i18n/types.ts

@@ -89,15 +89,6 @@ export interface Translations {
     lastUpdate: string;
     platformError: string;
     platformDisconnected: string;
-    actions: string;
-    restartGateway: string;
-    restarting: string;
-    restartSuccess: string;
-    restartFailed: string;
-    updateHermes: string;
-    updating: string;
-    updateSuccess: string;
-    updateFailed: string;
   };
 
   // ── Sessions page ──

web/src/i18n/zh.ts

@@ -86,15 +86,6 @@ export const zh: Translations = {
     lastUpdate: "最后更新",
     platformError: "错误",
     platformDisconnected: "已断开",
-    actions: "操作",
-    restartGateway: "重启网关",
-    restarting: "重启中…",
-    restartSuccess: "重启信号已发送",
-    restartFailed: "重启失败",
-    updateHermes: "更新 Hermes",
-    updating: "更新中…",
-    updateSuccess: "更新完成",
-    updateFailed: "更新失败",
   },
 
   sessions: {

web/src/lib/api.ts

@@ -182,12 +182,6 @@ export const api = {
       },
     );
   },
-
-  // Dashboard actions
-  restartGateway: () =>
-    fetchJSON<ActionResponse>("/api/actions/restart-gateway", { method: "POST" }),
-  updateHermes: () =>
-    fetchJSON<ActionResponse>("/api/actions/update-hermes", { method: "POST" }),
 };
 
 export interface PlatformStatus {
@@ -415,15 +409,9 @@ export interface OAuthSubmitResponse {
   message?: string;
 }
 
-export interface ActionResponse {
-  ok: boolean;
-  detail: string;
-}
-
 export interface OAuthPollResponse {
   session_id: string;
   status: "pending" | "approved" | "denied" | "expired" | "error";
   error_message?: string | null;
   expires_at?: number | null;
 }
-

web/src/pages/StatusPage.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from "react";
+import { useEffect, useState } from "react";
 import {
   Activity,
   AlertTriangle,
@@ -6,30 +6,19 @@ import {
   Cpu,
   Database,
   Radio,
-  RefreshCw,
-  TriangleAlert,
   Wifi,
   WifiOff,
-  Zap,
 } from "lucide-react";
 import { api } from "@/lib/api";
 import type { PlatformStatus, SessionInfo, StatusResponse } from "@/lib/api";
 import { timeAgo, isoTimeAgo } from "@/lib/utils";
-import { Button } from "@/components/ui/button";
 import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card";
 import { Badge } from "@/components/ui/badge";
 import { useI18n } from "@/i18n";
 
-type ActionState = "idle" | "running" | "success" | "failure";
-
 export default function StatusPage() {
   const [status, setStatus] = useState<StatusResponse | null>(null);
   const [sessions, setSessions] = useState<SessionInfo[]>([]);
-  const [restartState, setRestartState] = useState<ActionState>("idle");
-  const [restartDetail, setRestartDetail] = useState("");
-  const [updateState, setUpdateState] = useState<ActionState>("idle");
-  const [updateDetail, setUpdateDetail] = useState("");
-  const resetTimers = useRef<Record<string, ReturnType<typeof setTimeout>>>({});
   const { t } = useI18n();
 
   useEffect(() => {
@@ -42,39 +31,6 @@ export default function StatusPage() {
     return () => clearInterval(interval);
   }, []);
 
-  function scheduleReset(key: string, setter: (s: ActionState) => void) {
-    clearTimeout(resetTimers.current[key]);
-    resetTimers.current[key] = setTimeout(() => setter("idle"), 8000);
-  }
-
-  async function handleRestartGateway() {
-    setRestartState("running");
-    setRestartDetail("");
-    try {
-      const resp = await api.restartGateway();
-      setRestartState(resp.ok ? "success" : "failure");
-      setRestartDetail(resp.detail);
-    } catch (err: unknown) {
-      setRestartState("failure");
-      setRestartDetail(err instanceof Error ? err.message : String(err));
-    }
-    scheduleReset("restart", setRestartState);
-  }
-
-  async function handleUpdateHermes() {
-    setUpdateState("running");
-    setUpdateDetail("");
-    try {
-      const resp = await api.updateHermes();
-      setUpdateState(resp.ok ? "success" : "failure");
-      setUpdateDetail(resp.detail);
-    } catch (err: unknown) {
-      setUpdateState("failure");
-      setUpdateDetail(err instanceof Error ? err.message : String(err));
-    }
-    scheduleReset("update", setUpdateState);
-  }
-
   if (!status) {
     return (
       <div className="flex items-center justify-center py-24">
@@ -203,57 +159,6 @@ export default function StatusPage() {
         ))}
       </div>
 
-      {/* Action buttons — restart gateway / update Hermes */}
-      <Card>
-        <CardHeader>
-          <div className="flex items-center gap-2">
-            <Zap className="h-5 w-5 text-muted-foreground" />
-            <CardTitle className="text-base">{t.status.actions}</CardTitle>
-          </div>
-        </CardHeader>
-        <CardContent className="flex flex-wrap gap-3">
-          {/* Restart Gateway */}
-          <div className="flex flex-col gap-1">
-            <Button
-              variant="outline"
-              size="sm"
-              disabled={restartState === "running"}
-              onClick={handleRestartGateway}
-            >
-              <RefreshCw className={`h-3.5 w-3.5 mr-1 ${restartState === "running" ? "animate-spin" : ""}`} />
-              {restartState === "running" ? t.status.restarting : t.status.restartGateway}
-            </Button>
-            {(restartDetail || restartState === "success") && (
-              <p className={`text-xs max-w-xs truncate ${restartState === "failure" ? "text-destructive" : "text-muted-foreground"}`}>
-                {restartState === "failure" && <TriangleAlert className="inline h-3 w-3 mr-1" />}
-                {restartState === "success" ? t.status.restartSuccess : restartState === "failure" ? t.status.restartFailed : ""}
-                {restartDetail && ` — ${restartDetail}`}
-              </p>
-            )}
-          </div>
-
-          {/* Update Hermes */}
-          <div className="flex flex-col gap-1">
-            <Button
-              variant="outline"
-              size="sm"
-              disabled={updateState === "running"}
-              onClick={handleUpdateHermes}
-            >
-              <RefreshCw className={`h-3.5 w-3.5 mr-1 ${updateState === "running" ? "animate-spin" : ""}`} />
-              {updateState === "running" ? t.status.updating : t.status.updateHermes}
-            </Button>
-            {(updateDetail || updateState === "success" || updateState === "failure") && (
-              <p className={`text-xs max-w-xs ${updateState === "failure" ? "text-destructive" : "text-muted-foreground"}`}>
-                {updateState === "failure" && <TriangleAlert className="inline h-3 w-3 mr-1" />}
-                {updateState === "success" ? t.status.updateSuccess : updateState === "failure" ? t.status.updateFailed : ""}
-                {updateDetail && ` — ${updateDetail}`}
-              </p>
-            )}
-          </div>
-        </CardContent>
-      </Card>
-
       {platforms.length > 0 && (
         <PlatformsCard platforms={platforms} platformStateBadge={PLATFORM_STATE_BADGE} />
       )}