fix(#4771): local approvals no longer 409 as gateway run unavailable

CHANGELOG.md

@@ -3,6 +3,10 @@
 
 ## [Unreleased]
 
+### Fixed
+
+- **Command approvals work again on the default local backend (#4771 regression).** On the in-process WebUI chat runtime, clicking Approve/Deny on a guarded-command card failed with `Gateway approval could not be relayed because the active run is unavailable` (HTTP 409, `gateway_run_unavailable`) on every click — a regression from #4771 (v0.51.653), which surfaced that 409 for gateway approvals whose run had gone but emitted it regardless of backend. Local approvals have no gateway run by design, so the check always tripped. The 409 is now gated on the WebUI actually running the gateway chat backend; local approvals resolve in-process as before, and the gateway behavior is unchanged. Reported by b3nw and claw-io (#4948).
+
 ## [v0.51.665] — 2026-06-25 — Release XU (sidebar stops re-querying every poll during streaming)
 
 ### Fixed

api/routes.py

@@ -18701,7 +18701,12 @@ def _handle_approval_respond(handler, body):
 
     # Gateway relay: forward choice to the runs API when session has an active run.
     try:
-        from api.gateway_chat import _STREAM_RUN_IDS, _gateway_base_url, _gateway_api_key
+        from api.gateway_chat import (
+            _STREAM_RUN_IDS,
+            _gateway_base_url,
+            _gateway_api_key,
+            webui_gateway_chat_enabled,
+        )
         from api.config import get_config as _get_config
         s = get_session(sid)
         _run_id = None
@@ -18721,7 +18726,23 @@ def _handle_approval_respond(handler, body):
             except (RunnerClientError, ValueError) as exc:
                 return j(handler, {"ok": False, "choice": choice, "relayed": True, "error": str(exc)}, status=502)
             return j(handler, {"ok": True, "choice": choice, "relayed": True})
-        if _gateway_pending_approval_without_run_id(sid, approval_id):
+        # #4771 surfaces an explicit relay-failure 409 when a gateway approval
+        # is pending but its run is gone (so the card stays actionable instead
+        # of silently failing). That signal is ONLY meaningful on a
+        # gateway-backed deployment. On the default local in-process backend,
+        # every guarded command parks an entry in tools.approval._gateway_queues
+        # (via _await_gateway_decision), which the WebUI mirrors into _pending
+        # with _GATEWAY_MIRROR_FLAG set — but there is no gateway run and no
+        # _STREAM_RUN_IDS entry by design. Without the backend-mode gate below,
+        # _gateway_pending_approval_without_run_id() returns True for that purely
+        # local approval and the handler 409s ("active run unavailable"),
+        # refusing to resolve an approval that resolves perfectly well locally.
+        # Gate on gateway mode so local approvals fall through to the local
+        # resolution path; gateway behaviour is unchanged. (#4771 regression;
+        # also reported as #4948)
+        if webui_gateway_chat_enabled(_get_config()) and _gateway_pending_approval_without_run_id(
+            sid, approval_id
+        ):
             return j(
                 handler,
                 {

tests/test_approval_unblock.py

@@ -504,6 +504,12 @@ def fake_j(handler, data, status=200, extra_headers=None):
 
         monkeypatch.setattr(r, "j", fake_j)
         monkeypatch.setattr(r, "get_session", lambda _sid: SimpleNamespace(active_stream_id=stream_id))
+        # The relay-unavailable 409 is gateway-deployment behaviour: it only
+        # fires when the WebUI actually runs the gateway chat backend. On the
+        # default local backend a mirrored approval is resolved locally
+        # instead (see test_issue4771_local_approval_regression.py). Pin the
+        # gateway backend so this test exercises the intended 409 path.
+        monkeypatch.setenv("HERMES_WEBUI_CHAT_BACKEND", "gateway")
 
         with _lock:
             r._pending.pop(sid, None)

tests/test_gateway_approval_legacy_path.py

@@ -420,7 +420,16 @@ def fake_j(handler, data, status=200, extra_headers=None):
             assert isinstance(pending_queue, list)
             approval_id = pending_queue[0]["approval_id"]
 
-        with patch("api.routes.get_session", return_value=mock_session), \
+        # The relay-unavailable 409 is only meaningful when the WebUI is
+        # actually running the gateway chat backend. On a gateway deployment
+        # the backend env is process-wide (not just during the stream), so
+        # assert the 409 with HERMES_WEBUI_CHAT_BACKEND=gateway active at
+        # respond time. Without this scope the handler now (correctly) treats
+        # a mirrored approval on the default LOCAL backend as locally
+        # resolvable and falls through instead of 409ing — see
+        # test_issue4771_local_approval_regression.py (#4771 follow-up).
+        with patch.dict("os.environ", {"HERMES_WEBUI_CHAT_BACKEND": "gateway"}), \
+             patch("api.routes.get_session", return_value=mock_session), \
              patch("api.routes.j", new=fake_j):
             r._handle_approval_respond(
                 object(),

tests/test_issue4771_local_approval_regression.py

@@ -0,0 +1,222 @@
+"""Regression tests for the #4771 follow-up: a LOCAL (in-process agent)
+approval must NOT be refused with the gateway "active run is unavailable" 409.
+
+Background
+----------
+#4771 ("keep gateway approval failures actionable") added an explicit 409
+relay-failure response in ``_handle_approval_respond`` for the case where a
+*gateway* approval is mirrored into WebUI polling state but its gateway run is
+gone (no ``_STREAM_RUN_IDS`` entry). That keeps the approval card actionable on
+a gateway-backed deployment instead of silently failing.
+
+The regression: on the DEFAULT local in-process backend, every guarded command
+parks an ``_ApprovalEntry`` in ``tools.approval._gateway_queues`` (via
+``_await_gateway_decision``), and the local streaming path mirrors it into
+``_pending`` with ``_GATEWAY_MIRROR_FLAG`` set — but there is no gateway run and
+no ``_STREAM_RUN_IDS`` entry, by design. ``_gateway_pending_approval_without_run_id``
+therefore returned True for that purely local approval, and the handler 409'd
+("Gateway approval could not be relayed because the active run is unavailable"),
+refusing to resolve an approval that resolves perfectly well locally. Users on
+the local backend hit this on *every* approval click.
+
+The fix gates the 409 on the WebUI actually running the gateway chat backend
+(``webui_gateway_chat_enabled``), so local approvals fall through to the local
+resolution path while the legitimate #4771 gateway behaviour is preserved.
+
+These tests drive the real ``_handle_approval_respond`` handler and assert:
+  - LOCAL backend  -> 200, approval resolved, agent thread unblocked (the fix)
+  - GATEWAY backend -> 409 preserved when a mirrored approval has no run (#4771)
+"""
+from __future__ import annotations
+
+import json
+import uuid
+from unittest.mock import patch
+
+import pytest
+
+# Importing an api module first injects the hermes-agent dir onto sys.path
+# (api.config._AGENT_DIR), which is what makes `tools.approval` importable.
+# Import order matters: tools.* will not resolve until api.config has run.
+from api import routes
+from api import models
+
+try:
+    import tools.approval as ta
+    from api import route_approvals as ra
+    APPROVAL_AVAILABLE = True
+except ImportError:
+    ta = None
+    ra = None
+    APPROVAL_AVAILABLE = False
+
+pytestmark = pytest.mark.skipif(
+    not APPROVAL_AVAILABLE,
+    reason="tools.approval not available in this environment",
+)
+
+
+class _FakeHandler:
+    """Minimal stand-in for BaseHTTPRequestHandler that captures the JSON body."""
+
+    def __init__(self):
+        self.status = None
+        self.headers_sent = {}
+        self._body = b""
+        self.client_address = ("127.0.0.1", 0)
+        self.headers = {}
+
+        class _W:
+            def __init__(self, outer):
+                self.outer = outer
+
+            def write(self, b):
+                self.outer._body += b
+
+        self.wfile = _W(self)
+
+    def send_response(self, code):
+        self.status = code
+
+    def send_header(self, k, v):
+        self.headers_sent[k] = v
+
+    def end_headers(self):
+        pass
+
+    def json(self):
+        return json.loads(self._body.decode("utf-8"))
+
+
+def _register_session(sid: str):
+    """Put a real Session (no active stream / no gateway run) into the cache."""
+    s = models.Session(session_id=sid, title="approval-regression")
+    s.active_stream_id = None
+    with models.LOCK:
+        models.SESSIONS[sid] = s
+    return s
+
+
+def _seed_local_pending_approval(sid: str):
+    """Reproduce what the local in-process agent path does on a guarded command.
+
+    The agent parks an ``_ApprovalEntry`` in ``_gateway_queues`` and the local
+    streaming notify callback mirrors it into ``_pending`` via
+    ``submit_gateway_pending_mirror`` (tagging it with ``_GATEWAY_MIRROR_FLAG``).
+    Returns (entry, approval_id).
+    """
+    with ta._lock:
+        ta._gateway_queues.pop(sid, None)
+        ta._pending.pop(sid, None)
+    entry = ta._ApprovalEntry({
+        "command": "rm -rf /tmp/x",
+        "description": "Dangerous command",
+        "pattern_key": "dangerous_command",
+        "pattern_keys": ["dangerous_command"],
+    })
+    with ta._lock:
+        ta._gateway_queues.setdefault(sid, []).append(entry)
+    ra.submit_gateway_pending_mirror(sid, entry.data)
+    with ta._lock:
+        q = ta._pending.get(sid)
+    head = q[0] if isinstance(q, list) and q else q
+    assert isinstance(head, dict)
+    # Sanity: the mirror is tagged exactly as the regression requires.
+    assert head.get(ra._GATEWAY_MIRROR_FLAG) is True
+    return entry, head.get("approval_id")
+
+
+def _cleanup(sid: str):
+    with ta._lock:
+        ta._gateway_queues.pop(sid, None)
+        ta._pending.pop(sid, None)
+    with models.LOCK:
+        models.SESSIONS.pop(sid, None)
+
+
+def test_local_mirrored_approval_resolves_not_409():
+    """The b3nw bug: a local approval must resolve (200), not 409.
+
+    Drives the real handler with the local backend (gateway chat disabled) and
+    asserts the agent thread is unblocked with the chosen result.
+    """
+    sid = f"local-approval-{uuid.uuid4().hex[:8]}"
+    _register_session(sid)
+    try:
+        entry, approval_id = _seed_local_pending_approval(sid)
+        handler = _FakeHandler()
+        with patch("api.gateway_chat.webui_gateway_chat_enabled", return_value=False):
+            routes._handle_approval_respond(
+                handler,
+                {"session_id": sid, "choice": "once", "approval_id": approval_id},
+            )
+        resp = handler.json()
+        assert handler.status == 200, f"expected 200, got {handler.status}: {resp}"
+        assert resp.get("ok") is True
+        assert resp.get("code") != "gateway_run_unavailable"
+        # The parked agent thread must be released with the user's choice.
+        assert entry.event.is_set(), "agent approval entry was not unblocked"
+        assert entry.result == "once"
+    finally:
+        _cleanup(sid)
+
+
+def test_local_mirrored_approval_deny_resolves():
+    """Deny on a local approval also resolves locally (no 409)."""
+    sid = f"local-approval-deny-{uuid.uuid4().hex[:8]}"
+    _register_session(sid)
+    try:
+        entry, approval_id = _seed_local_pending_approval(sid)
+        handler = _FakeHandler()
+        with patch("api.gateway_chat.webui_gateway_chat_enabled", return_value=False):
+            routes._handle_approval_respond(
+                handler,
+                {"session_id": sid, "choice": "deny", "approval_id": approval_id},
+            )
+        resp = handler.json()
+        assert handler.status == 200, f"expected 200, got {handler.status}: {resp}"
+        assert resp.get("ok") is True
+        assert entry.event.is_set()
+        assert entry.result == "deny"
+    finally:
+        _cleanup(sid)
+
+
+def test_gateway_mirrored_approval_without_run_still_409s():
+    """#4771 behaviour preserved: on the gateway backend, a mirrored approval
+    whose run is gone must still surface the actionable 409 so the card stays
+    visible instead of silently failing."""
+    sid = f"gw-approval-{uuid.uuid4().hex[:8]}"
+    _register_session(sid)  # no active_stream_id -> no _STREAM_RUN_IDS mapping
+    try:
+        entry, approval_id = _seed_local_pending_approval(sid)
+        handler = _FakeHandler()
+        with patch("api.gateway_chat.webui_gateway_chat_enabled", return_value=True):
+            routes._handle_approval_respond(
+                handler,
+                {"session_id": sid, "choice": "once", "approval_id": approval_id},
+            )
+        resp = handler.json()
+        assert handler.status == 409, f"expected 409, got {handler.status}: {resp}"
+        assert resp.get("ok") is False
+        assert resp.get("relayed") is False
+        assert resp.get("code") == "gateway_run_unavailable"
+    finally:
+        _cleanup(sid)
+
+
+def test_gateway_pending_without_run_id_flag_detection():
+    """Unit-level: the helper detects a mirrored (flagged) head correctly.
+
+    This is the predicate the 409 hinges on; it must report True for a
+    gateway-mirror entry regardless of backend (the backend gate lives in the
+    handler, not the predicate).
+    """
+    sid = f"flag-detect-{uuid.uuid4().hex[:8]}"
+    try:
+        entry, approval_id = _seed_local_pending_approval(sid)
+        assert routes._gateway_pending_approval_without_run_id(sid, approval_id) is True
+        # A non-existent approval_id must not match.
+        assert routes._gateway_pending_approval_without_run_id(sid, "no-such-id") is False
+    finally:
+        _cleanup(sid)