fix(#4771): keep gateway approval failures actionable

api/routes.py

@@ -6842,6 +6842,7 @@ from api.route_approvals import (  # noqa: F401 — re-exports for backward comp
     _approval_sse_unsubscribe,
     _approval_sse_notify_locked,
     _approval_sse_notify,
+    _GATEWAY_MIRROR_FLAG,
     reconcile_gateway_pending_mirror_locked,
     submit_gateway_pending_mirror,
     submit_pending,
@@ -18645,6 +18646,32 @@ def _resolve_approval_legacy(sid: str, approval_id: str, choice: str) -> bool:
     return resolved
 
 
+_GATEWAY_APPROVAL_RELAY_UNAVAILABLE = (
+    "Gateway approval could not be relayed because the active run is unavailable. "
+    "Reopen the session or retry after it reconnects."
+)
+
+
+def _gateway_pending_approval_without_run_id(sid: str, approval_id: str) -> bool:
+    with _lock:
+        reconcile_gateway_pending_mirror_locked(sid)
+        queue = _pending.get(sid)
+        if isinstance(queue, list):
+            entries = queue
+        elif queue:
+            entries = [queue]
+        else:
+            entries = []
+        if approval_id:
+            for entry in entries:
+                if isinstance(entry, dict) and entry.get("approval_id") == approval_id:
+                    return bool(entry.get(_GATEWAY_MIRROR_FLAG))
+            return False
+        if not entries or not isinstance(entries[0], dict):
+            return False
+        return bool(entries[0].get(_GATEWAY_MIRROR_FLAG))
+
+
 def _handle_approval_respond(handler, body):
     sid = body.get("session_id", "")
     if not sid:
@@ -18676,6 +18703,18 @@ 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):
+            return j(
+                handler,
+                {
+                    "ok": False,
+                    "choice": choice,
+                    "relayed": False,
+                    "code": "gateway_run_unavailable",
+                    "error": _GATEWAY_APPROVAL_RELAY_UNAVAILABLE,
+                },
+                status=409,
+            )
     except Exception:
         pass  # fall through to local approval path
 

static/messages.js

@@ -5578,16 +5578,38 @@ async function respondApproval(choice) {
     const b = $(id);
     if (b) { b.disabled = true; if (b.id === "approvalBtn" + choice.charAt(0).toUpperCase() + choice.slice(1)) b.classList.add("loading"); }
   });
-  _approvalSessionId = null;
-  _approvalCurrentId = null;
-  _clearApprovalPendingForSession(sid);
-  hideApprovalCard(true);
   try {
-    await api("/api/approval/respond", {
+    const result = await api("/api/approval/respond", {
       method: "POST",
       body: JSON.stringify({ session_id: sid, choice, approval_id: approvalId })
     });
-  } catch(e) { setStatus(t("approval_responding") + " " + e.message); }
+    if (result && result.ok) {
+      const pendingEntry = _approvalPendingBySession.get(sid);
+      const samePending = !!(pendingEntry && pendingEntry.pending && (pendingEntry.pending.approval_id || null) === (approvalId || null));
+      if (_approvalSessionId === sid && _approvalCurrentId === approvalId) {
+        _approvalSessionId = null;
+        _approvalCurrentId = null;
+        hideApprovalCard(true);
+      }
+      if (samePending) _clearApprovalPendingForSession(sid);
+      return;
+    }
+    const errMsg = (result && result.error) || "Approval response not accepted.";
+    ["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
+      const b = $(id); if (b) { b.disabled = false; b.classList.remove("loading"); }
+    });
+    if (_approvalPromptBelongsToActiveSession(sid)) _renderPendingApprovalForActiveSession();
+    if (typeof showToast === "function") showToast(errMsg, 5000);
+    if (typeof setStatus === "function") setStatus(errMsg);
+  } catch(e) {
+    const errMsg = (e && e.message) || (t("approval_responding") + " failed");
+    ["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
+      const b = $(id); if (b) { b.disabled = false; b.classList.remove("loading"); }
+    });
+    if (_approvalPromptBelongsToActiveSession(sid)) _renderPendingApprovalForActiveSession();
+    if (typeof showToast === "function") showToast(errMsg, 5000);
+    if (typeof setStatus === "function") setStatus(errMsg);
+  }
 }
 
 function startApprovalPolling(sid) {

tests/test_approval_unblock.py

@@ -12,6 +12,7 @@ import urllib.request
 import urllib.error
 import urllib.parse
 from pathlib import Path
+from types import SimpleNamespace
 
 import pytest
 
@@ -479,3 +480,63 @@ class TestApprovalHTTPEndpoints:
                 r._pending.pop(sid, None)
                 r._gateway_queues.pop(sid, None)
                 pass  # no external token state to clean
+
+    def test_gateway_mirror_without_run_id_returns_explicit_conflict(self, monkeypatch):
+        """Gateway-mirrored approvals without relayable run state must stay actionable."""
+        from api import routes as r
+        from api import route_approvals as ra
+        from api.gateway_chat import _STREAM_RUN_IDS
+
+        sid = f"http-gateway-no-run-{uuid.uuid4().hex[:8]}"
+        stream_id = f"stream-no-run-{uuid.uuid4().hex[:8]}"
+        approval = {
+            "command": "rm -rf /tmp/no-run",
+            "pattern_key": "recursive delete",
+            "pattern_keys": ["recursive delete"],
+            "description": "recursive delete",
+        }
+        captured = {}
+
+        def fake_j(handler, data, status=200, extra_headers=None):
+            captured["payload"] = data
+            captured["status"] = status
+            return data
+
+        monkeypatch.setattr(r, "j", fake_j)
+        monkeypatch.setattr(r, "get_session", lambda _sid: SimpleNamespace(active_stream_id=stream_id))
+
+        with _lock:
+            r._pending.pop(sid, None)
+            r._gateway_queues.pop(sid, None)
+            _STREAM_RUN_IDS.pop(stream_id, None)
+            r._gateway_queues[sid] = [_ApprovalEntry(approval)]
+        try:
+            ra.submit_gateway_pending_mirror(sid, approval)
+            with _lock:
+                approval_id = r._pending[sid][0]["approval_id"]
+
+            r._handle_approval_respond(
+                object(),
+                {"session_id": sid, "choice": "once", "approval_id": approval_id},
+            )
+
+            assert captured["status"] == 409
+            assert captured["payload"] == {
+                "ok": False,
+                "choice": "once",
+                "relayed": False,
+                "code": "gateway_run_unavailable",
+                "error": r._GATEWAY_APPROVAL_RELAY_UNAVAILABLE,
+            }
+            with _lock:
+                pending_queue = r._pending.get(sid)
+                assert isinstance(pending_queue, list)
+                assert len(pending_queue) == 1
+                assert pending_queue[0]["approval_id"] == approval_id
+                assert sid in r._gateway_queues
+                assert not r._gateway_queues[sid][0].event.is_set()
+        finally:
+            with _lock:
+                r._pending.pop(sid, None)
+                r._gateway_queues.pop(sid, None)
+                _STREAM_RUN_IDS.pop(stream_id, None)

tests/test_gateway_approval_legacy_path.py

@@ -331,3 +331,113 @@ def test_legacy_approval_records_run_id_for_response_relay():
         with STREAMS_LOCK:
             STREAMS.pop(stream_id, None)
         _STREAM_RUN_IDS.pop(stream_id, None)
+
+def test_legacy_approval_without_run_id_stays_actionable():
+    """Legacy approvals without a run_id must fail explicitly and keep the mirror live."""
+    from api import routes as r
+    from api import route_approvals as ra
+    from api.config import STREAMS, STREAMS_LOCK
+    from api.gateway_chat import _STREAM_RUN_IDS, _run_gateway_chat_streaming
+    from tools.approval import _ApprovalEntry
+
+    stream_id = "sid-legacy-no-run"
+    session_id = "sess-legacy-no-run"
+    events = []
+    q = MagicMock()
+    q.put_nowait = lambda item: events.append(item)
+
+    with STREAMS_LOCK:
+        STREAMS[stream_id] = q
+    _STREAM_RUN_IDS.pop(stream_id, None)
+
+    approval_payload = json.dumps({
+        "command": "rm -rf /tmp/test",
+        "description": "Delete temporary files",
+        "pattern_key": "dangerous_command",
+        "pattern_keys": ["dangerous_command"],
+        "approval_id": "appr-legacy-no-run",
+        "choices": ["once", "session", "always", "deny"],
+    })
+    sse_body = (
+        f"event: approval.request\ndata: {approval_payload}\n\n"
+        'data: {"choices":[{"delta":{"content":"Done"}}]}\n\n'
+        "data: [DONE]\n\n"
+    ).encode()
+
+    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
+
+    def fake_urlopen(req, *, timeout=None):
+        resp = MagicMock()
+        resp.__iter__ = lambda s: iter(sse_body.split(b"\n"))
+        resp.__enter__ = lambda s: s
+        resp.__exit__ = lambda s, *a: None
+        return resp
+
+    captured = {}
+
+    def fake_j(handler, data, status=200, extra_headers=None):
+        captured["payload"] = data
+        captured["status"] = status
+        return data
+
+    try:
+        with patch.dict("os.environ", {"HERMES_WEBUI_CHAT_BACKEND": "gateway"}):
+            with patch("api.gateway_chat.gateway_supports_approval", return_value=False), \
+                 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=session_id,
+                    msg_text="do something risky",
+                    model="test",
+                    workspace="/tmp",
+                    stream_id=stream_id,
+                )
+
+        assert _STREAM_RUN_IDS.get(stream_id) is None
+        approval_events = [
+            item for item in events
+            if isinstance(item, tuple) and item[0] == "approval"
+        ]
+        assert approval_events
+        approval_data = approval_events[0][1]
+        with ra._lock:
+            r._gateway_queues[session_id] = [_ApprovalEntry(approval_data)]
+        ra.submit_gateway_pending_mirror(session_id, approval_data)
+        with ra._lock:
+            pending_queue = r._pending.get(session_id)
+            assert isinstance(pending_queue, list)
+            approval_id = pending_queue[0]["approval_id"]
+
+        with patch("api.routes.get_session", return_value=mock_session), \
+             patch("api.routes.j", new=fake_j):
+            r._handle_approval_respond(
+                object(),
+                {"session_id": session_id, "choice": "once", "approval_id": approval_id},
+            )
+
+        assert captured["status"] == 409
+        assert captured["payload"]["code"] == "gateway_run_unavailable"
+        assert captured["payload"]["error"] == r._GATEWAY_APPROVAL_RELAY_UNAVAILABLE
+        with ra._lock:
+            pending_queue = r._pending.get(session_id)
+            assert isinstance(pending_queue, list)
+            assert pending_queue[0]["approval_id"] == approval_id
+    finally:
+        with STREAMS_LOCK:
+            STREAMS.pop(stream_id, None)
+        with ra._lock:
+            r._pending.pop(session_id, None)
+            r._gateway_queues.pop(session_id, None)
+        _STREAM_RUN_IDS.pop(stream_id, None)

tests/test_issue4771_gateway_approval_failure_ui.py

@@ -0,0 +1,193 @@
+"""Tests for #4771: failed gateway approval clicks must keep the card actionable."""
+
+from __future__ import annotations
+
+import json
+import shutil
+import subprocess
+import tempfile
+from pathlib import Path
+
+import pytest
+
+ROOT = Path(__file__).resolve().parent.parent
+MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8")
+NODE = shutil.which("node")
+
+pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH")
+
+
+def _extract_fn(src: str, name: str, prefix: str = "function ") -> str:
+    start = src.index(f"{prefix}{name}(")
+    brace = src.index("{", start)
+    depth = 0
+    for i in range(brace, len(src)):
+        if src[i] == "{":
+            depth += 1
+        elif src[i] == "}":
+            depth -= 1
+            if depth == 0:
+                return src[start:i + 1]
+    raise AssertionError(f"{name} body not closed")
+
+
+def _run_node(script: str) -> dict:
+    with tempfile.NamedTemporaryFile("w", suffix=".js", delete=False, encoding="utf-8") as tf:
+        tf.write(script)
+        script_path = tf.name
+    try:
+        result = subprocess.run(
+            [NODE, script_path],
+            check=True,
+            capture_output=True,
+            text=True,
+            timeout=15,
+        )
+        return json.loads(result.stdout)
+    finally:
+        Path(script_path).unlink(missing_ok=True)
+
+
+def test_failed_gateway_approval_keeps_card_and_reenables_buttons():
+    helpers = "\n".join(
+        [
+            _extract_fn(MESSAGES_JS, "_approvalDismissKey"),
+            _extract_fn(MESSAGES_JS, "_getDismissedApprovals"),
+            _extract_fn(MESSAGES_JS, "_isApprovalDismissed"),
+            _extract_fn(MESSAGES_JS, "_unmarkApprovalDismissed"),
+            _extract_fn(MESSAGES_JS, "_promptActiveSessionId"),
+            _extract_fn(MESSAGES_JS, "_approvalPromptBelongsToActiveSession"),
+            _extract_fn(MESSAGES_JS, "_rememberApprovalPending"),
+            _extract_fn(MESSAGES_JS, "_clearApprovalPendingForSession"),
+            _extract_fn(MESSAGES_JS, "_renderPendingApprovalForActiveSession"),
+            _extract_fn(MESSAGES_JS, "showApprovalCard"),
+            _extract_fn(MESSAGES_JS, "respondApproval", prefix="async function "),
+        ]
+    )
+    script = f"""
+const output = {{}};
+const _store = {{}};
+const localStorage = {{
+  getItem: key => Object.prototype.hasOwnProperty.call(_store, key) ? _store[key] : null,
+  setItem: (key, value) => {{ _store[key] = String(value); }},
+}};
+const S = {{ session: {{ session_id: 'sess-1' }} }};
+const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals';
+let syncTopbarCalls = 0;
+let showToastCalls = [];
+let statusCalls = [];
+let hideCalls = 0;
+let renderCalls = 0;
+let _approvalSessionId = 'sess-1';
+let _approvalCurrentId = 'appr-1';
+let _approvalPendingBySession = new Map();
+let _approvalSignature = '';
+let _approvalVisibleSince = 0;
+let _approvalHideTimer = null;
+const _clarifyPendingBySession = new Map();
+const buttons = new Map();
+function makeButton(id) {{
+  return {{
+    id,
+    disabled: false,
+    classList: {{
+      values: new Set(),
+      add(value) {{ this.values.add(value); }},
+      remove(value) {{ this.values.delete(value); }},
+      contains(value) {{ return this.values.has(value); }},
+    }},
+  }};
+}}
+['approvalBtnOnce','approvalBtnSession','approvalBtnAlways','approvalBtnDeny'].forEach(id => buttons.set(id, makeButton(id)));
+const card = {{
+  classList: {{
+    values: new Set(['visible']),
+    add(value) {{ this.values.add(value); }},
+    remove(value) {{ this.values.delete(value); }},
+    contains(value) {{ return this.values.has(value); }},
+    toggle(value, force) {{
+      if (force === undefined) {{
+        if (this.values.has(value)) this.values.delete(value);
+        else this.values.add(value);
+      }} else if (force) this.values.add(value);
+      else this.values.delete(value);
+    }},
+  }},
+  querySelector() {{ return null; }},
+}};
+const approvalDesc = {{ textContent: '' }};
+const approvalCmd = {{ textContent: '' }};
+const approvalCounter = {{ textContent: '', style: {{ display: '' }} }};
+const msg = {{}};
+const document = {{ activeElement: null }};
+const elements = {{
+  approvalCard: card,
+  approvalDesc,
+  approvalCmd,
+  approvalCounter,
+  approvalBtnOnce: buttons.get('approvalBtnOnce'),
+  approvalBtnSession: buttons.get('approvalBtnSession'),
+  approvalBtnAlways: buttons.get('approvalBtnAlways'),
+  approvalBtnDeny: buttons.get('approvalBtnDeny'),
+  msg,
+}};
+function $(id) {{ return elements[id] || null; }}
+function syncTopbar() {{ syncTopbarCalls += 1; }}
+function hideApprovalCard() {{ hideCalls += 1; card.classList.remove('visible'); }}
+function _clearApprovalHideTimer() {{}}
+function _syncApprovalCollapseButton() {{}}
+function _syncApprovalTranscriptSpace() {{}}
+function applyLocaleToDOM() {{}}
+function setTimeout(fn) {{ return 1; }}
+function showToast(msg) {{ showToastCalls.push(msg); }}
+function setStatus(msg) {{ statusCalls.push(msg); }}
+function t(key) {{ return key; }}
+function api() {{
+  const err = new Error('Gateway approval could not be relayed because the active run is unavailable. Reopen the session or retry after it reconnects.');
+  err.status = 409;
+  return Promise.reject(err);
+}}
+{helpers}
+const pending = {{
+  approval_id: 'appr-1',
+  _session_id: 'sess-1',
+  command: 'rm -rf /tmp/test',
+  description: 'Dangerous command approval',
+  pattern_key: 'dangerous_command',
+  pattern_keys: ['dangerous_command'],
+}};
+_approvalPendingBySession.set('sess-1', {{ pending, pendingCount: 1 }});
+showApprovalCard(pending, 1);
+renderCalls = 0;
+_renderPendingApprovalForActiveSession = function() {{
+  renderCalls += 1;
+  const entry = _approvalPendingBySession.get('sess-1');
+  if (entry) showApprovalCard(entry.pending, entry.pendingCount);
+}};
+respondApproval('once').then(() => {{
+  output.sessionId = _approvalSessionId;
+  output.approvalId = _approvalCurrentId;
+  output.pendingApprovalId = _approvalPendingBySession.get('sess-1').pending.approval_id;
+  output.hideCalls = hideCalls;
+  output.renderCalls = renderCalls;
+  output.toast = showToastCalls[0] || null;
+  output.status = statusCalls[0] || null;
+  output.onceDisabled = buttons.get('approvalBtnOnce').disabled;
+  output.onceLoading = buttons.get('approvalBtnOnce').classList.contains('loading');
+  output.denyDisabled = buttons.get('approvalBtnDeny').disabled;
+  output.cardVisible = card.classList.contains('visible');
+  process.stdout.write(JSON.stringify(output));
+}});
+"""
+    out = _run_node(script)
+    assert out["sessionId"] == "sess-1"
+    assert out["approvalId"] == "appr-1"
+    assert out["pendingApprovalId"] == "appr-1"
+    assert out["hideCalls"] == 0
+    assert out["renderCalls"] == 1
+    assert out["onceDisabled"] is False
+    assert out["onceLoading"] is False
+    assert out["denyDisabled"] is False
+    assert out["cardVisible"] is True
+    assert "active run is unavailable" in out["toast"]
+    assert "active run is unavailable" in out["status"]