Release: actionable ids for raw id-less pending approvals (#7510) (#7551)

* fix(approvals): mint actionable ids for raw id-less local pending entries

A raw id-less approval entry in _pending (agent-side _pending_result drops
{command, pattern_key, pattern_keys, description} verbatim when no gateway
notifier is registered for the session) is unactionable by contract: the
frontend owner-capture requires an approval_id, so the approval card can
neither be approved nor dismissed - the 1.5s poll re-serves it forever.

reconcile_gateway_pending_mirror_locked already mints gwlocal:<token> ids
for id-less _gateway_queues producers; extend the same contract to raw
non-mirror _pending entries with a stable gwlocal-mirrorless:<uuid> id
minted once onto the stored entry dict, so the id persists across polls
and frontend dismiss markers stick.

The exact-id respond path needs no further change: once the entry carries
an id, _resolve_approval_legacy pops it (also clearing the stale
stream-pointer 409 guard), stale explicit ids still fail closed (#527),
and the yolo Skip-all release drains it.

* docs(changelog): stamp #7510 (actionable ids for raw id-less pending approvals)

---------

Co-authored-by: CharlesMcquade <6466275+CharlesMcquade@users.noreply.github.com>
Co-authored-by: n <a@n>
Co-authored-by: nesquena-hermes <nesquena-hermes@users.noreply.github.com>

CHANGELOG.md

@@ -5,6 +5,8 @@
 
 ### Fixed
 
+- **A timed-out command-approval card can be dismissed again instead of getting stuck on screen forever.** When the agent raised a local approval with no gateway notifier registered, the raw pending entry was stored without an `approval_id`; the frontend needs that id to own the card, so a card that timed out (or otherwise went unanswered) could be neither approved nor dismissed and the poll re-served it indefinitely. Reconciliation now mints a stable `gwlocal-mirrorless:` id on the raw local entry itself — the same tokenization contract already used for gateway producers — so the id survives repeated polls and the client's dismiss marker sticks. Gateway/run-bearing entries are untouched, and the synthetic prefix can't be mistaken for gateway authority (routing keys on exact ids, `run_id`, and mirror tokens, never the prefix). Thanks @CharlesMcquade. (#7510)
+
 - **Manual "Regenerate title" now works for conversations that open with several queued user messages.** For a session whose transcript begins with consecutive user turns (no assistant reply before the second user message), the first-exchange walker stopped at the second user row with no assistant text, so "Regenerate title" skipped the LLM call, silently persisted the local fallback (a truncated first message), and returned `200` with that same wrong title on every retry — leaving the session permanently stuck. Manual regeneration now scans past the opening user rows to the first complete user+assistant pair so it reaches the aux model. The automatic in-stream title path is deliberately unchanged. Thanks @Raylands. (#7545, closes #7543)
 
 - **Custom providers that Hermes auto-discovered now show their full live `/v1/models` catalog again instead of just the saved models.** When a provider entry carries `models_discovered: true` alongside a `models:` mapping of per-model metadata (vision, context length), that mapping is discovery metadata — not a hand-curated allowlist — but the WebUI treated it as one and suppressed the live catalog, collapsing the model picker to only the already-saved IDs. Such a provider now defers to its live `/v1/models` catalog (matching the Hermes Agent's own `_models_config_is_allowlist` semantics), while an explicit `discover_models: false` opt-out (including the Agent-compatible string forms `false`/`no`/`0`) still pins the configured list, and a transient empty live probe falls back to the configured models rather than dropping the provider from the picker. Thanks @evgenyponomarev. (#7409, closes #7404)

api/route_approvals.py

@@ -265,6 +265,23 @@ def reconcile_gateway_pending_mirror_locked(session_key: str) -> tuple[dict | No
             live_local_tokens.add(live_entry_token)
             if not str(live_data.get("approval_id") or "").strip():
                 live_data["approval_id"] = f"gwlocal:{live_entry_token}"
+
+    # A raw id-less LOCAL entry in `_pending` (agent-side _pending_result drops
+    # `{command, pattern_key, pattern_keys, description}` verbatim when no
+    # gateway notifier is registered) is unactionable by contract: the frontend
+    # owner-capture requires an approval_id, so its card can neither be
+    # approved nor dismissed — the poll re-serves it forever. Mint a stable id
+    # on the entry itself (the same contract the producer-tokenization loop
+    # above applies to _gateway_queues producers). Minting on the stored dict
+    # keeps the id stable across polls, so frontend dismiss markers persist.
+    for entry in queue_list:
+        if not isinstance(entry, dict) or _is_gateway_mirror_entry(entry):
+            continue
+        if str(entry.get("approval_id") or "").strip():
+            continue
+        if str(entry.get("run_id") or "").strip():
+            continue
+        entry["approval_id"] = f"gwlocal-mirrorless:{uuid.uuid4().hex}"
     live_token = (
         _gateway_mirror_entry_token(live_head_entry)
         if live_head_entry and live_head_data

tests/test_issue_raw_pending_approval_id.py

@@ -0,0 +1,249 @@
+"""Raw id-less local approval entries in ``_pending`` must be actionable.
+
+Regression for the Sep 2026 live incident. When a guarded tool needs approval
+and no gateway notifier is registered for the session (``unregister_gateway_
+notify`` fires at turn end; a queued wakeup/continuation then hits a guard),
+the agent-side fallback ``tools/approval.py::_pending_result`` writes a raw
+dict — ``{command, description, pattern_key, pattern_keys}`` and NOTHING else,
+no ``approval_id``, no ``request_id`` — directly into the shared
+``tools.approval._pending[session_key]`` dict that the WebUI imports.
+
+Before the fix:
+- ``GET /api/approval/pending`` served that raw dict verbatim (reconcile passes
+  non-mirror entries through untouched) with NO ``approval_id``, so the
+  frontend owner-capture (``_captureApprovalResponseOwner``) bailed and every
+  button on the approval card was a no-op;
+- the frontend dismiss (X) only hid the card locally and the 1.5s poll
+  re-rendered it forever — the card could neither be approved nor dismissed;
+- there is no waiter thread behind this shape (``_pending_result`` returns the
+  STOP message immediately), so draining the entry is purely UI hygiene — but
+  leaving it forever means the session shows a permanent phantom card.
+
+After the fix (mint at the reconcile chokepoint):
+- id-less, run-less, non-mirror ``_pending`` entries get a stable minted
+  ``approval_id`` (persisted in the stored entry dict, so it is stable across
+  polls and dismissals persist);
+- an exact-id response pops the entry and reports ``ok``;
+- a stale/different explicit id still fails closed (#527 guard preserved);
+- Skip-all (yolo release) drains the entry instead of dead-ending.
+
+The sibling shape — an orphaned ``_gateway_queues`` producer entry — is already
+covered upstream (minting ``gwlocal:<token>`` in the producer-tokenization loop,
+commit 5826d76d); the tests in ``test_gateway_approval_legacy_path.py`` and this
+file's history document it. This file covers the raw-``_pending`` shape, which
+no existing test exercised.
+"""
+
+from __future__ import annotations
+
+import json
+import re
+import uuid
+from unittest.mock import patch
+
+import pytest
+
+from api import models
+from api import route_approvals as ra
+from api import routes
+
+try:
+    import tools.approval as ta
+    APPROVAL_AVAILABLE = True
+except ImportError:
+    ta = None
+    APPROVAL_AVAILABLE = False
+
+pytestmark = pytest.mark.skipif(
+    not APPROVAL_AVAILABLE,
+    reason="tools.approval not available in this environment",
+)
+
+
+class _FakeHandler:
+    def __init__(self):
+        self.status = None
+        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):
+        pass
+
+    def end_headers(self):
+        pass
+
+    def json(self):
+        return json.loads(self._body.decode("utf-8"))
+
+
+def _register_session(sid: str, active_stream_id: str | None = None):
+    s = models.Session(session_id=sid, title="approval-raw-pending-orphan")
+    s.active_stream_id = active_stream_id
+    with models.LOCK:
+        models.SESSIONS[sid] = s
+    return s
+
+
+def _seed_raw_pending(sid: str, command: str = "rm -rf /tmp/qa-probe-home",
+                      key: str = "recursive delete") -> dict:
+    """Seed ``_pending[sid]`` with the exact ``_pending_result`` shape.
+
+    Mirrors agent-side tools/approval.py::_pending_result when no gateway
+    notifier is registered: a raw dict with no approval_id, no request_id,
+    no run_id, written directly into the shared _pending dict.
+    """
+    pending = {
+        "command": command,
+        "pattern_key": key,
+        "pattern_keys": [key],
+        "description": "recursive delete",
+    }
+    with ta._lock:
+        ta._pending[sid] = pending
+        ta._gateway_queues.pop(sid, None)
+    return pending
+
+
+def _cleanup(sid: str):
+    with ta._lock:
+        ta._pending.pop(sid, None)
+        ta._gateway_queues.pop(sid, None)
+    with models.LOCK:
+        models.SESSIONS.pop(sid, None)
+    try:
+        ta.disable_session_yolo(sid)
+    except Exception:
+        pass
+
+
+def _poll_pending(sid: str) -> dict:
+    h = _FakeHandler()
+    routes._handle_approval_pending(h, type("P", (), {"query": f"session_id={sid}"})())
+    assert h.status == 200, f"pending poll failed: {h.status} {h.json()!r}"
+    return h.json()
+
+
+def _respond(sid: str, body: dict):
+    h = _FakeHandler()
+    with patch("api.gateway_chat.webui_gateway_chat_enabled", return_value=True):
+        routes._handle_approval_respond(h, body)
+    return h
+
+
+MINTED_ID_RE = re.compile(r"^gwlocal-mirrorless:[0-9a-f]{32}$")
+
+
+def test_raw_pending_entry_surfaces_with_stable_actionable_id():
+    """The id-less raw entry must be served with a minted id that is stable
+    across polls (dismiss persistence keys on sid+approval_id)."""
+    sid = f"raw-id-{uuid.uuid4().hex[:8]}"
+    entry = _seed_raw_pending(sid)
+    try:
+        data = _poll_pending(sid)
+        assert data["pending_count"] >= 1
+        pending = data["pending"]
+        assert pending is not None
+        approval_id = pending.get("approval_id")
+        assert approval_id, (
+            "pending payload served a raw id-less local entry with no "
+            "approval_id; the frontend cannot act on it (buttons no-op, "
+            "dismiss cannot stick)"
+        )
+        assert MINTED_ID_RE.match(approval_id), approval_id
+
+        # Stability: the second poll must return the SAME id — the mint must
+        # persist in the stored entry, not re-mint per poll.
+        data2 = _poll_pending(sid)
+        assert data2["pending"]["approval_id"] == approval_id
+        assert entry["approval_id"] == approval_id
+    finally:
+        _cleanup(sid)
+
+
+def test_raw_pending_entry_respond_with_minted_id_pops_entry():
+    """An exact-id response must pop the raw entry and report ok (no waiter
+    exists behind this shape — draining is the correct resolution)."""
+    sid = f"raw-resp-{uuid.uuid4().hex[:8]}"
+    entry = _seed_raw_pending(sid)
+    # The incident session had a LIVE run pointer; the stale stream-pointer
+    # guard must not 409 the click before the legacy resolver pops the entry.
+    _register_session(sid, active_stream_id="stream-raw-resp-live")
+    from api.gateway_chat import _STREAM_RUN_IDS
+
+    _STREAM_RUN_IDS["stream-raw-resp-live"] = "run-live-1"
+    try:
+        approval_id = _poll_pending(sid)["pending"]["approval_id"]
+        assert approval_id
+
+        h = _respond(sid, {"session_id": sid, "choice": "deny",
+                           "approval_id": approval_id})
+        body = h.json()
+        assert h.status == 200, f"respond failed: {h.status} {body!r}"
+        assert body.get("ok") is True, f"respond not accepted: {body!r}"
+
+        # The entry must actually be gone — no more phantom card.
+        data = _poll_pending(sid)
+        assert data["pending"] is None
+        assert data["pending_count"] == 0
+    finally:
+        _STREAM_RUN_IDS.pop("stream-raw-resp-live", None)
+        _cleanup(sid)
+
+
+def test_raw_pending_entry_stale_id_fails_closed():
+    """#527 guard preserved: a different explicit id must not pop the live
+    entry and must keep it surfaced with its stable id."""
+    sid = f"raw-stale-{uuid.uuid4().hex[:8]}"
+    entry = _seed_raw_pending(sid)
+    _register_session(sid)
+    try:
+        approval_id = _poll_pending(sid)["pending"]["approval_id"]
+        assert approval_id
+
+        h = _respond(sid, {"session_id": sid, "choice": "once",
+                           "approval_id": "stale-different-id"})
+        body = h.json()
+        assert body.get("ok") is not True, f"stale id must fail closed: {body!r}"
+
+        # The live entry remains pending, unchanged, with its stable id.
+        data = _poll_pending(sid)
+        assert data["pending"] is not None
+        assert data["pending"]["approval_id"] == approval_id
+        assert entry["approval_id"] == approval_id
+    finally:
+        _cleanup(sid)
+
+
+def test_raw_pending_entry_yolo_skip_all_drains():
+    """Skip-all (yolo release) must drain the raw entry instead of leaving the
+    phantom card, and report the yolo state truthfully."""
+    sid = f"raw-yolo-{uuid.uuid4().hex[:8]}"
+    entry = _seed_raw_pending(sid)
+    _register_session(sid)
+    try:
+        approval_id = _poll_pending(sid)["pending"]["approval_id"]
+        assert approval_id
+
+        h = _respond(sid, {"session_id": sid, "choice": "once",
+                           "approval_id": approval_id, "yolo": True})
+        body = h.json()
+        assert h.status == 200, f"yolo respond failed: {h.status} {body!r}"
+        assert body.get("ok") is True, body
+        assert body.get("yolo_enabled") is True
+        assert _poll_pending(sid)["pending"] is None
+    finally:
+        # Restore: the yolo release enables session YOLO as a side effect.
+        _cleanup(sid)