From 5f1e308341bfeab0beac711ab1e302bfb007f6cd Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 27 Jun 2026 00:32:37 -0400 Subject: [PATCH 1/4] fix(#5000): relay mirrored gateway approvals after stream loss --- api/route_approvals.py | 25 +++++ api/routes.py | 23 ++-- tests/test_gateway_approval_legacy_path.py | 121 +++++++++++++++++++++ 3 files changed, 154 insertions(+), 15 deletions(-) diff --git a/api/route_approvals.py b/api/route_approvals.py index e4987f7c..33e9dca7 100644 --- a/api/route_approvals.py +++ b/api/route_approvals.py @@ -184,6 +184,31 @@ def reconcile_gateway_pending_mirror_locked(session_key: str) -> tuple[dict | No return head, total, changed +def _gateway_mirrored_pending_run_id(session_key: str, approval_id: str) -> str | None: + """Return the mirrored gateway approval run_id for a matching pending card. + + Reconciles the mirror first so a live gateway head still survives a lost + `active_stream_id` pointer. + """ + approval_id = str(approval_id or "").strip() + if not approval_id: + return None + with _lock: + reconcile_gateway_pending_mirror_locked(session_key) + queue = _pending.get(session_key) + if isinstance(queue, list): + entries = queue + elif queue: + entries = [queue] + else: + return None + for entry in entries: + if isinstance(entry, dict) and entry.get("approval_id") == approval_id and entry.get(_GATEWAY_MIRROR_FLAG): + run_id = str(entry.get("run_id") or "").strip() + return run_id or None + return None + + def submit_gateway_pending_mirror(session_key: str, approval: dict) -> None: """Mirror the live gateway head into WebUI polling state under a typed tag.""" del approval # mirror from the live gateway head under `_lock`, not from callback input diff --git a/api/routes.py b/api/routes.py index e7f93fdc..f5ecc7ed 100644 --- a/api/routes.py +++ b/api/routes.py @@ -6673,6 +6673,7 @@ from api.route_approvals import ( # noqa: F401 — re-exports for backward comp _approval_sse_notify_locked, _approval_sse_notify, _GATEWAY_MIRROR_FLAG, + _gateway_mirrored_pending_run_id, reconcile_gateway_pending_mirror_locked, submit_gateway_pending_mirror, submit_pending, @@ -18553,7 +18554,9 @@ def _handle_approval_respond(handler, body): return bad(handler, f"Invalid choice: {choice}") approval_id = body.get("approval_id", "") - # Gateway relay: forward choice to the runs API when session has an active run. + # Gateway relay: forward choice to the runs API when session has an active run, + # or recover the run_id from the mirrored gateway approval entry if the + # stream pointer has already been cleared. try: from api.gateway_chat import ( _STREAM_RUN_IDS, @@ -18568,6 +18571,8 @@ def _handle_approval_respond(handler, body): active_sid = getattr(s, "active_stream_id", None) if active_sid: _run_id = _STREAM_RUN_IDS.get(active_sid) + if not _run_id and approval_id: + _run_id = _gateway_mirrored_pending_run_id(sid, approval_id) if _run_id: if not approval_id: return bad(handler, "approval_id is required for gateway approvals") @@ -18580,20 +18585,8 @@ 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}) - # #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) + # Only a still-mirrored gateway approval with a missing run should 409; + # stale or empty gateway clicks fall through to local resolution. if webui_gateway_chat_enabled(_get_config()) and _gateway_pending_approval_without_run_id( sid, approval_id ): diff --git a/tests/test_gateway_approval_legacy_path.py b/tests/test_gateway_approval_legacy_path.py index 526a3041..c6f1ed81 100644 --- a/tests/test_gateway_approval_legacy_path.py +++ b/tests/test_gateway_approval_legacy_path.py @@ -332,6 +332,127 @@ def test_legacy_approval_records_run_id_for_response_relay(): STREAMS.pop(stream_id, None) _STREAM_RUN_IDS.pop(stream_id, None) + +def test_mirrored_run_id_survives_active_stream_loss(): + """A mirrored gateway approval must still relay after active_stream_id is lost.""" + import io + import tools.approval as ta + from api import route_approvals as ra + from api import routes + + sid = "sess-legacy-stream-loss" + approval_id = "appr-legacy-stream-loss" + run_id = "run-legacy-stream-loss" + + with ta._lock: + ta._gateway_queues.pop(sid, None) + ta._pending.pop(sid, None) + + entry = ta._ApprovalEntry({ + "command": "rm -rf /tmp/test", + "description": "Delete temporary files", + "pattern_key": "dangerous_command", + "pattern_keys": ["dangerous_command"], + "approval_id": approval_id, + "run_id": run_id, + "choices": ["once", "session", "always", "deny"], + }) + with ta._lock: + ta._gateway_queues.setdefault(sid, []).append(entry) + ra.submit_gateway_pending_mirror(sid, entry.data) + + with ta._lock: + mirrored = ta._pending[sid][0] + assert mirrored["approval_id"] == approval_id + assert mirrored["run_id"] == run_id + assert mirrored.get(ra._GATEWAY_MIRROR_FLAG) is True + + relay_session = MagicMock() + relay_session.active_stream_id = None + captured = {} + + def fake_request_json(self, req): + captured["url"] = req.full_url + captured["body"] = json.loads(req.data) + return {"ok": True} + + handler = MagicMock() + handler.wfile = io.BytesIO() + body = {"session_id": sid, "choice": "once", "approval_id": approval_id} + + try: + with patch("api.routes.get_session", return_value=relay_session), \ + patch("api.gateway_chat.webui_gateway_chat_enabled", return_value=True), \ + patch("api.gateway_chat._gateway_base_url", return_value="http://gw:8642"), \ + patch("api.gateway_chat._gateway_api_key", return_value=""), \ + patch("api.config.get_config", return_value={}), \ + patch("api.runner_client.HttpRunnerClient._request_json", new=fake_request_json): + routes._handle_approval_respond(handler, body) + + assert captured.get("url", "") == f"http://gw:8642/v1/runs/{run_id}/approval", ( + f"approval respond must relay to the mirrored gateway run; got {captured.get('url')!r}" + ) + assert captured["body"] == {"choice": "once", "approval_id": approval_id} + handler.send_response.assert_called_with(200) + assert handler.wfile.getvalue() + assert json.loads(handler.wfile.getvalue().decode("utf-8")) == { + "ok": True, + "choice": "once", + "relayed": True, + } + finally: + with ta._lock: + ta._gateway_queues.pop(sid, None) + ta._pending.pop(sid, None) + + +def test_gateway_mode_no_pending_click_stays_non_409(): + """Gateway mode must still fall through when nothing is pending.""" + import tools.approval as ta + from api import routes + + sid = "sess-legacy-no-pending" + approval_id = "appr-legacy-no-pending" + + with ta._lock: + ta._gateway_queues.pop(sid, None) + ta._pending.pop(sid, None) + + mock_session = MagicMock() + mock_session.active_stream_id = None + 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 + + captured = {} + + def fake_j(handler, data, status=200, extra_headers=None): + captured["payload"] = data + captured["status"] = status + return data + + 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), \ + patch("api.runtime_adapter.runtime_adapter_enabled", return_value=False): + routes._handle_approval_respond( + object(), + {"session_id": sid, "choice": "once", "approval_id": approval_id}, + ) + + assert captured["status"] == 200 + assert captured["payload"]["ok"] is True + assert captured["payload"]["choice"] == "once" + assert captured["payload"]["stale_cleared"] is True + assert captured["payload"].get("code") != "gateway_run_unavailable" + + def test_legacy_approval_without_run_id_stays_actionable(): """Legacy approvals without a run_id must fail explicitly and keep the mirror live.""" from types import SimpleNamespace From 8f0cc7912bf82b0a8cae0c20574e40ebf6668146 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 27 Jun 2026 00:56:09 -0400 Subject: [PATCH 2/4] fix(#5000): settle mirrored gateway approval cleanup --- api/routes.py | 4 ++++ tests/test_gateway_approval_legacy_path.py | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/api/routes.py b/api/routes.py index f5ecc7ed..9396c9cc 100644 --- a/api/routes.py +++ b/api/routes.py @@ -18584,6 +18584,10 @@ def _handle_approval_respond(handler, body): HttpRunnerClient(base_url=_base, api_key=_key).respond_approval(_run_id, approval_id, choice) except (RunnerClientError, ValueError) as exc: return j(handler, {"ok": False, "choice": choice, "relayed": True, "error": str(exc)}, status=502) + # The outbound relay only resumes the remote run; the local mirror + # still needs the same cleanup path so the parked entry, mirrored + # card, and agent signal all settle here too. + _resolve_approval_legacy(sid, approval_id, choice) return j(handler, {"ok": True, "choice": choice, "relayed": True}) # Only a still-mirrored gateway approval with a missing run should 409; # stale or empty gateway clicks fall through to local resolution. diff --git a/tests/test_gateway_approval_legacy_path.py b/tests/test_gateway_approval_legacy_path.py index c6f1ed81..224b8a7e 100644 --- a/tests/test_gateway_approval_legacy_path.py +++ b/tests/test_gateway_approval_legacy_path.py @@ -394,6 +394,11 @@ def test_mirrored_run_id_survives_active_stream_loss(): ) assert captured["body"] == {"choice": "once", "approval_id": approval_id} handler.send_response.assert_called_with(200) + assert entry.event.is_set(), "mirrored gateway approval was not resolved" + assert entry.result == "once" + with ta._lock: + assert sid not in ta._pending, "mirrored pending card was not cleared" + assert sid not in ta._gateway_queues, "parked gateway entry was not drained" assert handler.wfile.getvalue() assert json.loads(handler.wfile.getvalue().decode("utf-8")) == { "ok": True, From e621d7360a92090f7f67958d851419d3edd0db1f Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Sat, 27 Jun 2026 01:17:49 -0400 Subject: [PATCH 3/4] test(#5000): keep gateway approval regressions CI-portable --- tests/test_gateway_approval_legacy_path.py | 73 ++++++++++++++-------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/tests/test_gateway_approval_legacy_path.py b/tests/test_gateway_approval_legacy_path.py index 224b8a7e..1ad5eed4 100644 --- a/tests/test_gateway_approval_legacy_path.py +++ b/tests/test_gateway_approval_legacy_path.py @@ -336,7 +336,8 @@ def test_legacy_approval_records_run_id_for_response_relay(): def test_mirrored_run_id_survives_active_stream_loss(): """A mirrored gateway approval must still relay after active_stream_id is lost.""" import io - import tools.approval as ta + import threading + from types import SimpleNamespace from api import route_approvals as ra from api import routes @@ -344,25 +345,29 @@ def test_mirrored_run_id_survives_active_stream_loss(): approval_id = "appr-legacy-stream-loss" run_id = "run-legacy-stream-loss" - with ta._lock: - ta._gateway_queues.pop(sid, None) - ta._pending.pop(sid, None) + with ra._lock: + ra._gateway_queues.pop(sid, None) + ra._pending.pop(sid, None) - entry = ta._ApprovalEntry({ - "command": "rm -rf /tmp/test", - "description": "Delete temporary files", - "pattern_key": "dangerous_command", - "pattern_keys": ["dangerous_command"], - "approval_id": approval_id, - "run_id": run_id, - "choices": ["once", "session", "always", "deny"], - }) - with ta._lock: - ta._gateway_queues.setdefault(sid, []).append(entry) + entry = SimpleNamespace( + data={ + "command": "rm -rf /tmp/test", + "description": "Delete temporary files", + "pattern_key": "dangerous_command", + "pattern_keys": ["dangerous_command"], + "approval_id": approval_id, + "run_id": run_id, + "choices": ["once", "session", "always", "deny"], + }, + event=threading.Event(), + result=None, + ) + with ra._lock: + ra._gateway_queues.setdefault(sid, []).append(entry) ra.submit_gateway_pending_mirror(sid, entry.data) - with ta._lock: - mirrored = ta._pending[sid][0] + with ra._lock: + mirrored = ra._pending[sid][0] assert mirrored["approval_id"] == approval_id assert mirrored["run_id"] == run_id assert mirrored.get(ra._GATEWAY_MIRROR_FLAG) is True @@ -376,6 +381,19 @@ def test_mirrored_run_id_survives_active_stream_loss(): captured["body"] = json.loads(req.data) return {"ok": True} + def fake_resolve_gateway_approval(session_key, choice, resolve_all=False): + del resolve_all + with ra._lock: + queue = ra._gateway_queues.get(session_key) or [] + if not queue: + return 0 + queued_entry = queue.pop(0) + queued_entry.result = choice + queued_entry.event.set() + if not queue: + ra._gateway_queues.pop(session_key, None) + return 1 + handler = MagicMock() handler.wfile = io.BytesIO() body = {"session_id": sid, "choice": "once", "approval_id": approval_id} @@ -386,6 +404,7 @@ def test_mirrored_run_id_survives_active_stream_loss(): patch("api.gateway_chat._gateway_base_url", return_value="http://gw:8642"), \ patch("api.gateway_chat._gateway_api_key", return_value=""), \ patch("api.config.get_config", return_value={}), \ + patch("api.routes.resolve_gateway_approval", new=fake_resolve_gateway_approval), \ patch("api.runner_client.HttpRunnerClient._request_json", new=fake_request_json): routes._handle_approval_respond(handler, body) @@ -396,9 +415,9 @@ def test_mirrored_run_id_survives_active_stream_loss(): handler.send_response.assert_called_with(200) assert entry.event.is_set(), "mirrored gateway approval was not resolved" assert entry.result == "once" - with ta._lock: - assert sid not in ta._pending, "mirrored pending card was not cleared" - assert sid not in ta._gateway_queues, "parked gateway entry was not drained" + with ra._lock: + assert sid not in ra._pending, "mirrored pending card was not cleared" + assert sid not in ra._gateway_queues, "parked gateway entry was not drained" assert handler.wfile.getvalue() assert json.loads(handler.wfile.getvalue().decode("utf-8")) == { "ok": True, @@ -406,22 +425,22 @@ def test_mirrored_run_id_survives_active_stream_loss(): "relayed": True, } finally: - with ta._lock: - ta._gateway_queues.pop(sid, None) - ta._pending.pop(sid, None) + with ra._lock: + ra._gateway_queues.pop(sid, None) + ra._pending.pop(sid, None) def test_gateway_mode_no_pending_click_stays_non_409(): """Gateway mode must still fall through when nothing is pending.""" - import tools.approval as ta + from api import route_approvals as ra from api import routes sid = "sess-legacy-no-pending" approval_id = "appr-legacy-no-pending" - with ta._lock: - ta._gateway_queues.pop(sid, None) - ta._pending.pop(sid, None) + with ra._lock: + ra._gateway_queues.pop(sid, None) + ra._pending.pop(sid, None) mock_session = MagicMock() mock_session.active_stream_id = None From e47c3369810950d93fdd4c4d61fc783114a6f96c Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 27 Jun 2026 06:14:26 +0000 Subject: [PATCH 4/4] Release: relay mirrored gateway approvals after stream loss (#5041, fixes #5000) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecce844..bcd41b65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Gateway approval cards can be answered even after the live stream pointer is lost.** When you approve/deny a tool-use request that came through the gateway, `/api/approval/respond` walked back through the session's `active_stream_id` to find the run — so if that pointer was gone (reconnect, background tab, stream ended), responding failed with `gateway_run_unavailable` even though the mirrored approval card still carried the originating gateway run info. The respond handler now relays a mirrored gateway approval using the approval's own carried origin (scoped to the same session + approval id), so the card stays actionable across stream loss. The normal stream-alive path is unchanged, auth/CSRF gates are untouched, and a genuinely origin-less approval still returns `gateway_run_unavailable`. Thanks @rodboev. (#5041, fixes #5000) + - **Installing an extension now shows its post-install next steps.** Some extensions need a follow-up step after install (run a setup command, restart, configure a key). The Settings → Extensions gallery now renders that guidance from the extension's own registry metadata — generic `post_install` text and lifecycle requirements, with an optional docs link — right on the card, plus a "see the card for next steps" install toast. The guidance is registry-driven (no vendor special-casing), every value is HTML-escaped, any docs URL is validated to safe http(s) before it's linked, and the note degrades cleanly when an extension declares no post-install steps. Thanks @franksong2702. (#4964, fixes #4959) - **The Settings → Extensions diagnostics panel now shows per-extension sidecar runtime status.** When an extension's loopback sidecar health response includes an optional top-level `runtime` object, the diagnostics panel parses it and renders only allowlisted scalar fields (sidecar/native-host/bridge status, last-seen time, and the loopback origin) so you can see at a glance whether an extension's helper process is running, waiting, or stale — without WebUI depending on any one extension's private payload shape. All values are status-enum-allowlisted, HTML-escaped, timestamp-validated, and origin-limited to `http://127.0.0.1`/`localhost`; a missing or malformed `runtime` object degrades cleanly, and a failed diagnostics refresh now renders the error instead of staying stuck on "Loading…". Thanks @franksong2702. (#4979)