mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-18 21:50:55 +00:00
fix(#5139): distinguish offline gateway approval warnings
This commit is contained in:
+20
-3
@@ -7443,7 +7443,13 @@ def get_gateway_caps(base_url: str, api_key: str = "") -> dict:
|
||||
cached = _GATEWAY_CAPS_CACHE.get(cache_key)
|
||||
if cached and now - cached.get("fetched_at", 0) < _GATEWAY_CAPS_TTL_S:
|
||||
return cached
|
||||
caps = {"approval_events": False, "run_approval_response": False, "fetched_at": 0.0}
|
||||
caps = {
|
||||
"approval_events": False,
|
||||
"run_approval_response": False,
|
||||
"capabilities_reachable": False,
|
||||
"probe_error": None,
|
||||
"fetched_at": 0.0,
|
||||
}
|
||||
try:
|
||||
headers = {}
|
||||
if api_key:
|
||||
@@ -7451,13 +7457,14 @@ def get_gateway_caps(base_url: str, api_key: str = "") -> dict:
|
||||
req = urllib.request.Request(f"{base_url}/v1/capabilities", headers=headers, method="GET")
|
||||
with urllib.request.urlopen(req, timeout=3) as resp:
|
||||
body = json.loads(resp.read(65536))
|
||||
caps["capabilities_reachable"] = True
|
||||
features = body.get("features") if isinstance(body, dict) else {}
|
||||
if not isinstance(features, dict):
|
||||
features = {}
|
||||
caps["approval_events"] = bool(features.get("approval_events"))
|
||||
caps["run_approval_response"] = bool(features.get("run_approval_response"))
|
||||
except Exception:
|
||||
pass
|
||||
except Exception as exc:
|
||||
caps["probe_error"] = f"{type(exc).__name__}: {exc}"
|
||||
with _GATEWAY_CAPS_LOCK:
|
||||
current = _GATEWAY_CAPS_CACHE.get(cache_key)
|
||||
if current and current.get("fetched_at", 0) > probe_started_at:
|
||||
@@ -7467,6 +7474,16 @@ def get_gateway_caps(base_url: str, api_key: str = "") -> dict:
|
||||
return caps
|
||||
|
||||
|
||||
def gateway_approval_unavailable_reason(base_url: str, api_key: str = "") -> str | None:
|
||||
"""Return why approval support is unavailable, if it is unavailable."""
|
||||
caps = get_gateway_caps(base_url, api_key)
|
||||
if bool(caps.get("approval_events") and caps.get("run_approval_response")):
|
||||
return None
|
||||
if not caps.get("capabilities_reachable"):
|
||||
return "unreachable"
|
||||
return "unsupported"
|
||||
|
||||
|
||||
def gateway_supports_approval(base_url: str, api_key: str = "") -> bool:
|
||||
"""True only when the gateway advertises both approval_events and run_approval_response."""
|
||||
caps = get_gateway_caps(base_url, api_key)
|
||||
|
||||
+9
-2
@@ -20,6 +20,7 @@ from api.config import (
|
||||
STREAM_REASONING_TEXT,
|
||||
_get_session_agent_lock,
|
||||
coerce_reasoning_effort_for_model,
|
||||
gateway_approval_unavailable_reason,
|
||||
gateway_supports_approval,
|
||||
register_active_run,
|
||||
unregister_active_run,
|
||||
@@ -639,12 +640,18 @@ def _run_gateway_chat_streaming(
|
||||
# Legacy gateway path: emit unsupported approval notice once per session,
|
||||
# but only when the gateway genuinely lacks approval capability.
|
||||
if not gateway_supports_approval(base_url, api_key):
|
||||
approval_reason = gateway_approval_unavailable_reason(base_url, api_key)
|
||||
if not hasattr(s, "_approval_notice_emitted"):
|
||||
s._approval_notice_emitted = False
|
||||
if not s._approval_notice_emitted:
|
||||
approval_message = "Approvals require a newer gateway. Upgrade the connected Hermes gateway to enable this."
|
||||
approval_type = "approval_gateway_unsupported"
|
||||
if approval_reason == "unreachable":
|
||||
approval_type = "approval_gateway_offline"
|
||||
approval_message = "Gateway connection failed. Check that the connected Hermes gateway is running and reachable."
|
||||
put_gateway_event("warning", {
|
||||
"type": "approval_gateway_unsupported",
|
||||
"message": "Approvals require a newer gateway. Upgrade the connected Hermes gateway to enable this.",
|
||||
"type": approval_type,
|
||||
"message": approval_message,
|
||||
})
|
||||
s._approval_notice_emitted = True
|
||||
|
||||
|
||||
@@ -5447,6 +5447,10 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
|
||||
if(typeof showToast==='function') showToast(typeof t==='function'?t('approval_gateway_unsupported_label'):'Approvals not supported',4000,'warning');
|
||||
return;
|
||||
}
|
||||
if(d.type==='approval_gateway_offline'){
|
||||
if(typeof showToast==='function') showToast(d.message||'Gateway offline',4000,'warning');
|
||||
return;
|
||||
}
|
||||
// Show as a small inline notice, not a full error
|
||||
setComposerStatus(`${d.message||'Warning'}`);
|
||||
// If it's a fallback notice, show it briefly then clear
|
||||
|
||||
@@ -15,7 +15,9 @@ from unittest.mock import MagicMock, patch
|
||||
def test_gateway_capability_detection():
|
||||
"""get_gateway_caps / gateway_supports_approval correctly parse /v1/capabilities."""
|
||||
from api.config import (
|
||||
gateway_approval_unavailable_reason,
|
||||
gateway_supports_approval,
|
||||
get_gateway_caps,
|
||||
invalidate_gateway_caps,
|
||||
)
|
||||
|
||||
@@ -38,6 +40,10 @@ def test_gateway_capability_detection():
|
||||
return resp
|
||||
|
||||
with patch("urllib.request.urlopen", side_effect=_fake_urlopen_capable):
|
||||
caps = get_gateway_caps("http://fake:1234", "secret")
|
||||
assert caps["capabilities_reachable"] is True
|
||||
assert caps["probe_error"] is None
|
||||
assert gateway_approval_unavailable_reason("http://fake:1234", "secret") is None
|
||||
assert gateway_supports_approval("http://fake:1234", "secret") is True
|
||||
|
||||
invalidate_gateway_caps()
|
||||
@@ -52,11 +58,40 @@ def test_gateway_capability_detection():
|
||||
return resp
|
||||
|
||||
with patch("urllib.request.urlopen", side_effect=_fake_urlopen_incapable):
|
||||
caps = get_gateway_caps("http://fake:5678")
|
||||
assert caps["capabilities_reachable"] is True
|
||||
assert caps["probe_error"] is None
|
||||
assert gateway_approval_unavailable_reason("http://fake:5678") == "unsupported"
|
||||
assert gateway_supports_approval("http://fake:5678") is False
|
||||
|
||||
invalidate_gateway_caps()
|
||||
|
||||
|
||||
def test_gateway_capability_detection_marks_probe_failures_unreachable():
|
||||
"""Probe failures stay non-fatal but remain distinguishable from unsupported gateways."""
|
||||
from api.config import (
|
||||
gateway_approval_unavailable_reason,
|
||||
gateway_supports_approval,
|
||||
get_gateway_caps,
|
||||
invalidate_gateway_caps,
|
||||
)
|
||||
|
||||
invalidate_gateway_caps()
|
||||
|
||||
def _fake_urlopen_fail(req, *, timeout=None):
|
||||
assert req.full_url == "http://fake:9999/v1/capabilities"
|
||||
raise urllib.error.URLError("gateway unreachable")
|
||||
|
||||
with patch("urllib.request.urlopen", side_effect=_fake_urlopen_fail):
|
||||
caps = get_gateway_caps("http://fake:9999", "secret")
|
||||
assert caps["capabilities_reachable"] is False
|
||||
assert caps["probe_error"]
|
||||
assert gateway_approval_unavailable_reason("http://fake:9999", "secret") == "unreachable"
|
||||
assert gateway_supports_approval("http://fake:9999", "secret") is False
|
||||
|
||||
invalidate_gateway_caps()
|
||||
|
||||
|
||||
def test_gateway_capability_cache_keeps_fresher_success_on_probe_race():
|
||||
"""A slower failed probe must not overwrite a fresher successful capability result."""
|
||||
from api.config import gateway_supports_approval, invalidate_gateway_caps
|
||||
|
||||
@@ -17,7 +17,7 @@ def test_gateway_chat_has_approval_notice_emitted_attribute_check():
|
||||
def test_gateway_chat_emits_approval_gateway_unsupported_event():
|
||||
"""Verify put_gateway_event is called with approval_gateway_unsupported type on non-terminal channel."""
|
||||
assert "put_gateway_event(\"warning\"" in GATEWAY_CHAT
|
||||
assert "\"type\": \"approval_gateway_unsupported\"" in GATEWAY_CHAT
|
||||
assert "approval_type = \"approval_gateway_unsupported\"" in GATEWAY_CHAT
|
||||
|
||||
|
||||
def test_gateway_chat_once_per_session_guard_pattern():
|
||||
@@ -36,8 +36,8 @@ def test_gateway_chat_once_per_session_guard_pattern():
|
||||
|
||||
def test_gateway_chat_event_payload_contains_type_and_message():
|
||||
"""Verify the event payload has type and message fields."""
|
||||
assert "\"type\": \"approval_gateway_unsupported\"" in GATEWAY_CHAT
|
||||
assert "\"message\": \"Approvals require a newer gateway. Upgrade the connected Hermes gateway to enable this.\"" in GATEWAY_CHAT
|
||||
assert "approval_type = \"approval_gateway_unsupported\"" in GATEWAY_CHAT
|
||||
assert "approval_message = \"Approvals require a newer gateway. Upgrade the connected Hermes gateway to enable this.\"" in GATEWAY_CHAT
|
||||
|
||||
|
||||
def test_messages_js_handles_approval_gateway_unsupported_event():
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
"""Regression coverage for #5139 gateway approval offline notice."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from api.config import STREAMS, STREAMS_LOCK
|
||||
from api.gateway_chat import _run_gateway_chat_streaming
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
GATEWAY_CHAT = (REPO / "api" / "gateway_chat.py").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _run_gateway_warning_case(unavailable_reason: str) -> list:
|
||||
events = []
|
||||
q = MagicMock()
|
||||
q.put_nowait = lambda item: events.append(item)
|
||||
|
||||
stream_id = f"sid-{unavailable_reason}"
|
||||
with STREAMS_LOCK:
|
||||
STREAMS[stream_id] = q
|
||||
|
||||
mock_session = MagicMock()
|
||||
mock_session.active_stream_id = stream_id
|
||||
mock_session.workspace = "/tmp"
|
||||
mock_session.model = "test"
|
||||
mock_session.model_provider = None
|
||||
mock_session.profile = None
|
||||
mock_session.context_messages = []
|
||||
mock_session.messages = []
|
||||
mock_session.pending_user_message = None
|
||||
mock_session.pending_attachments = None
|
||||
mock_session.pending_started_at = None
|
||||
mock_session._approval_notice_emitted = False
|
||||
|
||||
def fake_urlopen(req, *, timeout=None):
|
||||
assert req.full_url == "http://127.0.0.1:8642/v1/chat/completions"
|
||||
assert req.get_method() == "POST"
|
||||
payload = req.data.decode("utf-8")
|
||||
assert json.loads(payload)["messages"][-1]["content"] == "hi"
|
||||
resp = MagicMock()
|
||||
resp.__iter__ = lambda s: iter(
|
||||
[
|
||||
b'data: {"choices":[{"delta":{"content":"Done"}}]}\n',
|
||||
b"\n",
|
||||
b"data: [DONE]\n",
|
||||
]
|
||||
)
|
||||
resp.__enter__ = lambda s: s
|
||||
resp.__exit__ = lambda s, *a: None
|
||||
return resp
|
||||
|
||||
try:
|
||||
with patch.dict("os.environ", {"HERMES_WEBUI_CHAT_BACKEND": "gateway"}):
|
||||
with patch("api.gateway_chat.gateway_supports_approval", return_value=False), \
|
||||
patch("api.gateway_chat.gateway_approval_unavailable_reason", return_value=unavailable_reason), \
|
||||
patch("urllib.request.urlopen", side_effect=fake_urlopen), \
|
||||
patch("api.gateway_chat.get_session", return_value=mock_session), \
|
||||
patch("api.gateway_chat._stream_writeback_is_current", return_value=True), \
|
||||
patch("api.gateway_chat.merge_session_messages_append_only", return_value=[]):
|
||||
_run_gateway_chat_streaming(
|
||||
session_id="sess-warning",
|
||||
msg_text="hi",
|
||||
model="test-model",
|
||||
workspace="/tmp",
|
||||
stream_id=stream_id,
|
||||
)
|
||||
finally:
|
||||
with STREAMS_LOCK:
|
||||
STREAMS.pop(stream_id, None)
|
||||
|
||||
return events
|
||||
|
||||
|
||||
def test_gateway_chat_emits_offline_warning_for_unreachable_probe():
|
||||
events = _run_gateway_warning_case("unreachable")
|
||||
warnings = [item for item in events if isinstance(item, tuple) and item[0] == "warning"]
|
||||
assert warnings
|
||||
assert warnings[0][1]["type"] == "approval_gateway_offline"
|
||||
assert warnings[0][1]["message"] == "Gateway connection failed. Check that the connected Hermes gateway is running and reachable."
|
||||
assert any(isinstance(item, tuple) and item[0] == "done" for item in events)
|
||||
assert not any(
|
||||
isinstance(item, tuple) and item[0] == "warning" and item[1].get("type") == "approval_gateway_unsupported"
|
||||
for item in events
|
||||
)
|
||||
|
||||
|
||||
def test_gateway_chat_keeps_unsupported_warning_for_reachable_older_gateway():
|
||||
events = _run_gateway_warning_case("unsupported")
|
||||
warnings = [item for item in events if isinstance(item, tuple) and item[0] == "warning"]
|
||||
assert warnings
|
||||
assert warnings[0][1]["type"] == "approval_gateway_unsupported"
|
||||
assert warnings[0][1]["message"] == "Approvals require a newer gateway. Upgrade the connected Hermes gateway to enable this."
|
||||
assert any(isinstance(item, tuple) and item[0] == "done" for item in events)
|
||||
assert not any(
|
||||
isinstance(item, tuple) and item[0] == "warning" and item[1].get("type") == "approval_gateway_offline"
|
||||
for item in events
|
||||
)
|
||||
|
||||
|
||||
def test_messages_js_handles_offline_warning_without_touching_unsupported_branch():
|
||||
assert "d.type==='approval_gateway_offline'" in MESSAGES_JS
|
||||
assert "Gateway offline" in MESSAGES_JS
|
||||
assert "d.type==='approval_gateway_unsupported'" in MESSAGES_JS
|
||||
assert "Approvals not supported" in MESSAGES_JS
|
||||
assert "setComposerStatus(`${d.message||'Warning'}`);" in MESSAGES_JS
|
||||
|
||||
|
||||
def test_gateway_chat_source_mentions_offline_warning_type():
|
||||
assert "approval_type = \"approval_gateway_offline\"" in GATEWAY_CHAT
|
||||
assert "approval_type = \"approval_gateway_unsupported\"" in GATEWAY_CHAT
|
||||
assert "approval_message = \"Gateway connection failed. Check that the connected Hermes gateway is running and reachable.\"" in GATEWAY_CHAT
|
||||
Reference in New Issue
Block a user