diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ecce844af8..bcd41b653cb 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) diff --git a/api/route_approvals.py b/api/route_approvals.py index e4987f7cf6b..33e9dca75d7 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 e7f93fdc1d7..9396c9cc9da 100644 --- a/api/routes.py +++ b/api/routes.py @@ -6673,6 +6673,7 @@ def _resolve_from_rows(rows: list) -> str | None: _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") @@ -18579,21 +18584,13 @@ 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}) - # #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 526a3041275..1ad5eed4bcc 100644 --- a/tests/test_gateway_approval_legacy_path.py +++ b/tests/test_gateway_approval_legacy_path.py @@ -332,6 +332,151 @@ def fake_request_json(self, req): 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 threading + from types import SimpleNamespace + 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 ra._lock: + ra._gateway_queues.pop(sid, None) + ra._pending.pop(sid, None) + + 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 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 + + 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} + + 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} + + 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.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) + + 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 entry.event.is_set(), "mirrored gateway approval was not resolved" + assert entry.result == "once" + 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, + "choice": "once", + "relayed": True, + } + finally: + 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.""" + from api import route_approvals as ra + from api import routes + + sid = "sess-legacy-no-pending" + approval_id = "appr-legacy-no-pending" + + with ra._lock: + ra._gateway_queues.pop(sid, None) + ra._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