Release XI (v0.51.653): keep gateway approval failures actionable (#4917, closes #4771)
This commit is contained in:
7 files changed
+640
-18
No files matched your search
@@ -3,6 +3,12 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.653] — 2026-06-25 — Release XI (gateway approval failures stay actionable instead of dead-ending)
|
||||
|
||||
### Fixed
|
||||
|
||||
- **In gateway-backed sessions, an approval card no longer disappears when the backend can't relay your approve/deny click.** Previously the WebUI cleared the approval card as soon as you clicked, before knowing whether the backend actually relayed the response to the gateway run — so if the relay failed, the card vanished and the run was left blocked with no way to respond. Now `/api/approval/respond` returns an explicit relay-failure error and preserves the pending state, and the card stays visible and actionable (with the error surfaced) until the backend confirms acceptance. Re-renders and Enter no longer re-enable or duplicate-submit an in-flight approval. Thanks @rodboev. (#4917, closes #4771)
|
||||
|
||||
## [v0.51.652] — 2026-06-25 — Release XH (background subagent results return to chat)
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
+57
-15
@@ -5361,6 +5361,7 @@ function hideApprovalCard(force=false) {
|
||||
let _approvalSessionId = null;
|
||||
let _approvalCurrentId = null; // approval_id of the card currently shown
|
||||
let _approvalPendingBySession = new Map();
|
||||
let _approvalResponding = null;
|
||||
|
||||
const _DISMISSED_APPROVALS_KEY = 'hermes_dismissed_approvals';
|
||||
|
||||
@@ -5449,6 +5450,27 @@ function _renderPendingApprovalForActiveSession() {
|
||||
if (entry) showApprovalCard(entry.pending, entry.pendingCount);
|
||||
}
|
||||
|
||||
function _approvalResponseMatches(sid, approvalId) {
|
||||
return !!(
|
||||
_approvalResponding &&
|
||||
_approvalResponding.sid === sid &&
|
||||
(_approvalResponding.approvalId || null) === (approvalId || null)
|
||||
);
|
||||
}
|
||||
|
||||
function _setApprovalControlsDisabled(choice, disabled) {
|
||||
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
|
||||
const b = $(id);
|
||||
if (!b) return;
|
||||
b.disabled = !!disabled;
|
||||
if (disabled && choice && b.id === "approvalBtn" + choice.charAt(0).toUpperCase() + choice.slice(1)) {
|
||||
b.classList.add("loading");
|
||||
} else {
|
||||
b.classList.remove("loading");
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
function showApprovalForSession(sid, pending, pendingCount) {
|
||||
if (!pending) return;
|
||||
pending._session_id = sid;
|
||||
@@ -5487,10 +5509,11 @@ function showApprovalCard(pending, pendingCount) {
|
||||
// approval's collapsed state, which would hide its command + action buttons. (#3515)
|
||||
card.classList.remove("collapsed");
|
||||
}
|
||||
// Re-enable buttons in case a previous approval disabled them
|
||||
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
|
||||
const b = $(id); if (b) { b.disabled = false; b.classList.remove("loading"); }
|
||||
});
|
||||
const responding = _approvalResponseMatches(sid, _approvalCurrentId);
|
||||
_setApprovalControlsDisabled(
|
||||
responding ? _approvalResponding.choice : null,
|
||||
responding,
|
||||
);
|
||||
card.classList.add("visible");
|
||||
_syncApprovalCollapseButton(card);
|
||||
_syncApprovalTranscriptSpace(card, {immediate: true});
|
||||
@@ -5559,6 +5582,14 @@ function _syncApprovalTranscriptSpace(card, opts) {
|
||||
setTimeout(measure, 420);
|
||||
}
|
||||
|
||||
function _restoreFailedApprovalResponse(sid, errMsg) {
|
||||
_approvalResponding = null;
|
||||
_setApprovalControlsDisabled(null, false);
|
||||
if (_approvalPromptBelongsToActiveSession(sid)) _renderPendingApprovalForActiveSession();
|
||||
if (typeof showToast === "function") showToast(errMsg, 5000);
|
||||
if (typeof setStatus === "function") setStatus(errMsg);
|
||||
}
|
||||
|
||||
function toggleApprovalCardCollapsed(forceCollapsed) {
|
||||
const card = $("approvalCard");
|
||||
if (!card) return;
|
||||
@@ -5572,22 +5603,33 @@ async function respondApproval(choice) {
|
||||
const sid = _approvalSessionId || (S.session && S.session.session_id);
|
||||
if (!sid) return;
|
||||
const approvalId = _approvalCurrentId;
|
||||
if (_approvalResponseMatches(sid, approvalId)) return;
|
||||
_unmarkApprovalDismissed(sid, approvalId);
|
||||
// Disable all buttons immediately to prevent double-submit
|
||||
["approvalBtnOnce","approvalBtnSession","approvalBtnAlways","approvalBtnDeny"].forEach(id => {
|
||||
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);
|
||||
_approvalResponding = {sid, approvalId: approvalId || null, choice};
|
||||
_setApprovalControlsDisabled(choice, 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) {
|
||||
_approvalResponding = null;
|
||||
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.";
|
||||
_restoreFailedApprovalResponse(sid, errMsg);
|
||||
} catch(e) {
|
||||
const errMsg = (e && e.message) || (t("approval_responding") + " failed");
|
||||
_restoreFailedApprovalResponse(sid, errMsg);
|
||||
}
|
||||
}
|
||||
|
||||
function startApprovalPolling(sid) {
|
||||
|
||||
@@ -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)
|
||||
@@ -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 types import SimpleNamespace
|
||||
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
|
||||
|
||||
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] = [SimpleNamespace(data=dict(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)
|
||||
@@ -0,0 +1,360 @@
|
||||
"""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 _run_failure_case(api_js: str) -> dict:
|
||||
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, "_approvalResponseMatches"),
|
||||
_extract_fn(MESSAGES_JS, "_setApprovalControlsDisabled"),
|
||||
_extract_fn(MESSAGES_JS, "showApprovalCard"),
|
||||
_extract_fn(MESSAGES_JS, "_restoreFailedApprovalResponse"),
|
||||
_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 _approvalResponding = null;
|
||||
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; }}
|
||||
{api_js}
|
||||
{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);
|
||||
const realRenderPendingApprovalForActiveSession = _renderPendingApprovalForActiveSession;
|
||||
renderCalls = 0;
|
||||
_renderPendingApprovalForActiveSession = function() {{
|
||||
renderCalls += 1;
|
||||
return realRenderPendingApprovalForActiveSession();
|
||||
}};
|
||||
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));
|
||||
}});
|
||||
"""
|
||||
return _run_node(script)
|
||||
|
||||
|
||||
def test_failed_gateway_approval_keeps_card_and_reenables_buttons():
|
||||
out = _run_failure_case(
|
||||
"""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);
|
||||
}"""
|
||||
)
|
||||
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"]
|
||||
|
||||
|
||||
def test_rejected_ok_false_approval_keeps_card_and_reenables_buttons():
|
||||
out = _run_failure_case(
|
||||
"""function api() {
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
error: 'Approval response not accepted for this session.',
|
||||
});
|
||||
}"""
|
||||
)
|
||||
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 "Approval response not accepted for this session." == out["toast"]
|
||||
assert "Approval response not accepted for this session." == out["status"]
|
||||
|
||||
|
||||
def test_poll_rerender_keeps_inflight_buttons_disabled_and_blocks_duplicates():
|
||||
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, "_approvalResponseMatches"),
|
||||
_extract_fn(MESSAGES_JS, "_setApprovalControlsDisabled"),
|
||||
_extract_fn(MESSAGES_JS, "showApprovalCard"),
|
||||
_extract_fn(MESSAGES_JS, "_restoreFailedApprovalResponse"),
|
||||
_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 _approvalSessionId = 'sess-1';
|
||||
let _approvalCurrentId = 'appr-1';
|
||||
let _approvalPendingBySession = new Map();
|
||||
let _approvalResponding = null;
|
||||
let _approvalSignature = '';
|
||||
let _approvalVisibleSince = 0;
|
||||
let _approvalHideTimer = null;
|
||||
const _clarifyPendingBySession = new Map();
|
||||
let resolveApi;
|
||||
let apiCalls = 0;
|
||||
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() {{}}
|
||||
function hideApprovalCard() {{ card.classList.remove('visible'); }}
|
||||
function _clearApprovalHideTimer() {{}}
|
||||
function _syncApprovalCollapseButton() {{}}
|
||||
function _syncApprovalTranscriptSpace() {{}}
|
||||
function applyLocaleToDOM() {{}}
|
||||
function setTimeout(fn) {{ return 1; }}
|
||||
function showToast() {{}}
|
||||
function setStatus() {{}}
|
||||
function t(key) {{ return key; }}
|
||||
function api() {{
|
||||
apiCalls += 1;
|
||||
return new Promise(resolve => {{ resolveApi = resolve; }});
|
||||
}}
|
||||
{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);
|
||||
const first = respondApproval('once');
|
||||
_renderPendingApprovalForActiveSession();
|
||||
const second = respondApproval('once');
|
||||
output.apiCallsDuringFlight = apiCalls;
|
||||
output.onceDisabledDuringFlight = buttons.get('approvalBtnOnce').disabled;
|
||||
output.onceLoadingDuringFlight = buttons.get('approvalBtnOnce').classList.contains('loading');
|
||||
resolveApi({{ ok: false }});
|
||||
Promise.all([first, second]).then(() => {{
|
||||
output.apiCallsFinal = apiCalls;
|
||||
output.onceDisabledAfter = buttons.get('approvalBtnOnce').disabled;
|
||||
output.onceLoadingAfter = buttons.get('approvalBtnOnce').classList.contains('loading');
|
||||
output.cardVisibleAfter = card.classList.contains('visible');
|
||||
process.stdout.write(JSON.stringify(output));
|
||||
}});
|
||||
"""
|
||||
out = _run_node(script)
|
||||
assert out["apiCallsDuringFlight"] == 1
|
||||
assert out["onceDisabledDuringFlight"] is True
|
||||
assert out["onceLoadingDuringFlight"] is True
|
||||
assert out["apiCallsFinal"] == 1
|
||||
assert out["onceDisabledAfter"] is False
|
||||
assert out["onceLoadingAfter"] is False
|
||||
assert out["cardVisibleAfter"] is True
|
||||
@@ -272,12 +272,16 @@ class TestApprovalMessagesJS:
|
||||
|
||||
def test_show_approval_card_re_enables_buttons(self):
|
||||
src = read(REPO / "static/messages.js")
|
||||
assert "b.disabled = false" in src and "loading" in src, \
|
||||
"showApprovalCard should re-enable buttons on each show"
|
||||
assert "_approvalResponseMatches" in src and "_setApprovalControlsDisabled(" in src, \
|
||||
"showApprovalCard should route button state through the shared approval helper"
|
||||
assert "responding ? _approvalResponding.choice : null" in src, \
|
||||
"showApprovalCard should preserve the active loading choice during rerenders"
|
||||
|
||||
def test_respond_disables_buttons_immediately(self):
|
||||
src = read(REPO / "static/messages.js")
|
||||
assert "b.disabled = true" in src, \
|
||||
assert "_approvalResponding = {sid, approvalId: approvalId || null, choice};" in src, \
|
||||
"respondApproval should record the in-flight approval before the API call"
|
||||
assert "_setApprovalControlsDisabled(choice, true);" in src, \
|
||||
"respondApproval should disable buttons immediately to prevent double-submit"
|
||||
|
||||
def test_respond_uses_i18n_for_error(self):
|
||||
|
||||
Reference in new issue
Block a user