Files
hermes-webui/tests/test_issue4771_local_approval_regression.py
T
nesquena-hermes bac8108b38 fix(#4771): don't 409 local approvals as "gateway run unavailable"
#4771 added an explicit relay-failure 409 (code: gateway_run_unavailable)
to /api/approval/respond so a gateway approval whose run is gone keeps its
card actionable instead of silently failing. But the 409 was emitted
regardless of backend.

On the DEFAULT local in-process backend, every guarded command parks an
entry in tools.approval._gateway_queues (via _await_gateway_decision), and
the local streaming notify callback mirrors it into _pending tagged with
_GATEWAY_MIRROR_FLAG. 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, so the handler 409'd
("active run unavailable") and refused to resolve an approval that resolves
perfectly well locally. Local-backend users hit this on every approval
click (reported by b3nw on Discord; independently filed as #4948 by
claw-io, which also notes the local-runtime trigger).

Fix: gate the 409 on the WebUI actually running the gateway chat backend
(webui_gateway_chat_enabled). Local approvals fall through to the local
resolution path; the legitimate #4771 gateway behaviour is unchanged.

Tests:
- New tests/test_issue4771_local_approval_regression.py drives the real
  _handle_approval_respond handler and asserts: local backend -> 200 +
  agent thread unblocked; gateway backend -> 409 preserved. These fail
  before the fix (409) and pass after.
- The two existing #4771 tests that asserted the 409 are updated to pin
  HERMES_WEBUI_CHAT_BACKEND=gateway at respond time, since the 409 is now
  gateway-deployment behaviour.

Reported-by: b3nw
Reported-by: claw-io (#4948)
2026-06-25 22:21:56 +00:00

223 lines
8.3 KiB
Python

"""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)