Sabo/webui
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: queue simultaneous approval requests per session (fixes #527) — v0.50.52

Browse Source 
fix: queue simultaneous approval requests per session (fixes #527) — v0.50.52
This commit is contained in:
nesquena-hermes
2026-04-15 12:42:32 -07:00
committed by GitHub
parent ac839e0d01 25fae902d3
commit 392c315d4b
5 changed files with 277 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
CHANGELOG.md
View File

@@ -1,5 +1,10 @@
# Hermes Web UI -- Changelog
## [v0.50.52] — 2026-04-15
### Fixed
- **Simultaneous approval requests** — parallel tool calls that each require approval no longer overwrite each other. `_pending` is now a list per session; each entry gets a stable `approval_id` (uuid4) so `/api/approval/respond` can target a specific request. The UI shows a "1 of N pending" counter when multiple approvals are queued. Backward-compatible with old agent versions and old frontend clients. Adds 14 regression tests. (Fixes #527)
## [v0.50.51] — 2026-04-15
### Fixed

71
api/routes.py
View File

@@ -193,7 +193,7 @@ from api.onboarding import (
# Approval system (optional -- graceful fallback if agent not available)
try:
from tools.approval import (
submit_pending,
submit_pending as _submit_pending_raw,
approve_session,
approve_permanent,
save_permanent_allowlist,
@@ -204,7 +204,7 @@ try:
resolve_gateway_approval,
)
except ImportError:
submit_pending = lambda *a, **k: None
_submit_pending_raw = lambda *a, **k: None
approve_session = lambda *a, **k: None
approve_permanent = lambda *a, **k: None
save_permanent_allowlist = lambda *a, **k: None
@@ -214,6 +214,31 @@ except ImportError:
_lock = threading.Lock()
_permanent_approved = set()
def submit_pending(session_key: str, approval: dict) -> None:
"""Append a pending approval to the per-session queue.
Wraps the agent's submit_pending to:
- Add a stable approval_id (uuid4 hex) so the respond endpoint can target
a specific entry even when multiple approvals are queued simultaneously.
- Change the storage from a single overwriting dict value to a list, so
parallel tool calls each get their own approval slot (fixes #527).
"""
entry = dict(approval)
entry.setdefault("approval_id", uuid.uuid4().hex)
with _lock:
queue = _pending.setdefault(session_key, [])
# Replace a legacy non-list value if the agent version uses the old pattern.
if not isinstance(queue, list):
_pending[session_key] = [queue]
queue = _pending[session_key]
queue.append(entry)
# NOTE: We do NOT call _submit_pending_raw here — that function overwrites
# _pending[session_key] with a single dict, which would undo the list we just
# built. The gateway blocking path uses _gateway_queues (a separate mechanism
# managed by check_all_command_guards / register_gateway_notify), which is
# unaffected by _pending. The _pending dict is only used for UI polling.
# Clarify prompts (optional -- graceful fallback if agent not available)
try:
from api.clarify import (
@@ -1671,10 +1696,20 @@ def _handle_file_read(handler, parsed):
def _handle_approval_pending(handler, parsed):
sid = parse_qs(parsed.query).get("session_id", [""])[0]
with _lock:
p = _pending.get(sid)
queue = _pending.get(sid)
# Support both the new list format and a legacy single-dict value.
if isinstance(queue, list):
p = queue[0] if queue else None
total = len(queue)
elif queue:
p = queue
total = 1
else:
p = None
total = 0
if p:
return j(handler, {"pending": dict(p)})
return j(handler, {"pending": None})
return j(handler, {"pending": dict(p), "pending_count": total})
return j(handler, {"pending": None, "pending_count": 0})
def _handle_approval_inject(handler, parsed):
@@ -2354,9 +2389,31 @@ def _handle_approval_respond(handler, body):
choice = body.get("choice", "deny")
if choice not in ("once", "session", "always", "deny"):
return bad(handler, f"Invalid choice: {choice}")
# Pop the legacy polling-mode pending entry (no-op when gateway path is active).
approval_id = body.get("approval_id", "")
# Pop the targeted entry from the pending queue by approval_id.
# Falls back to popping the first entry for backward-compat with old clients.
pending = None
with _lock:
pending = _pending.pop(sid, None)
queue = _pending.get(sid)
if isinstance(queue, list):
if approval_id:
# Find and remove the specific entry by approval_id.
for i, entry in enumerate(queue):
if entry.get("approval_id") == approval_id:
pending = queue.pop(i)
break
else:
# approval_id not found -- fall back to oldest entry.
pending = queue.pop(0) if queue else None
else:
pending = queue.pop(0) if queue else None
if not queue:
_pending.pop(sid, None)
elif queue:
# Legacy single-dict value.
pending = _pending.pop(sid, None)
if pending:
keys = pending.get("pattern_keys") or [pending.get("pattern_key", "")]
if choice in ("once", "session"):

3
static/index.html
View File

@@ -225,6 +225,7 @@
</div>
<div class="approval-desc" id="approvalDesc"></div>
<div class="approval-cmd" id="approvalCmd"></div>
<div class="approval-counter" id="approvalCounter" style="display:none;font-size:0.75em;opacity:0.6;margin-top:4px;"></div>
<div class="approval-btns">
<button class="approval-btn once" id="approvalBtnOnce" onclick="respondApproval('once')" title="Allow this one command (Enter)" data-i18n-title="approval_btn_once_title">
<span class="approval-btn-icon"><svg width="14" height="14" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2.5" stroke-linecap="round" stroke-linejoin="round" aria-hidden="true"><polyline points="20 6 9 17 4 12"/></svg></span>
@@ -552,7 +553,7 @@
<div class="settings-section-title">System</div>
<div class="settings-section-meta">Instance version and access controls.</div>
</div>
<span class="settings-version-badge">v0.50.51</span>
<span class="settings-version-badge">v0.50.52</span>
</div>
<div class="settings-field" style="border-top:1px solid var(--border);padding-top:12px;margin-top:8px">
<label for="settingsPassword" data-i18n="settings_label_password">Access Password</label>

22
static/messages.js
View File

@@ -360,7 +360,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){
source.addEventListener('approval',e=>{
const d=JSON.parse(e.data);
d._session_id=activeSid;
showApprovalCard(d);
showApprovalCard(d, 1);
playNotificationSound();
sendBrowserNotification('Approval required',d.description||'Tool approval needed');
});
@@ -595,8 +595,9 @@ function hideApprovalCard(force=false) {
// Track session_id of the active approval so respond goes to the right session
let _approvalSessionId = null;
let _approvalCurrentId = null; // approval_id of the card currently shown
function showApprovalCard(pending) {
function showApprovalCard(pending, pendingCount) {
const keys = pending.pattern_keys || (pending.pattern_key ? [pending.pattern_key] : []);
const desc = (pending.description || "") + (keys.length ? " [" + keys.join(", ") + "]" : "");
const cmd = pending.command || "";
@@ -606,7 +607,18 @@ function showApprovalCard(pending) {
$("approvalDesc").textContent = desc;
$("approvalCmd").textContent = cmd;
_approvalSessionId = pending._session_id || (S.session && S.session.session_id) || null;
_approvalCurrentId = pending.approval_id || null;
_approvalSignature = sig;
// Show "1 of N" counter when multiple approvals are queued
const counter = $("approvalCounter");
if (counter) {
if (pendingCount && pendingCount > 1) {
counter.textContent = "1 of " + pendingCount + " pending";
counter.style.display = "";
} else {
counter.style.display = "none";
}
}
if (!sameApproval) {
_approvalVisibleSince = Date.now();
_clearApprovalHideTimer();
@@ -627,17 +639,19 @@ function showApprovalCard(pending) {
async function respondApproval(choice) {
const sid = _approvalSessionId || (S.session && S.session.session_id);
if (!sid) return;
const approvalId = _approvalCurrentId;
// 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;
hideApprovalCard(true);
try {
await api("/api/approval/respond", {
method: "POST",
body: JSON.stringify({ session_id: sid, choice })
body: JSON.stringify({ session_id: sid, choice, approval_id: approvalId })
});
} catch(e) { setStatus(t("approval_responding") + " " + e.message); }
}
@@ -650,7 +664,7 @@ function startApprovalPolling(sid) {
}
try {
const data = await api("/api/approval/pending?session_id=" + encodeURIComponent(sid));
if (data.pending) { data.pending._session_id=sid; showApprovalCard(data.pending); }
if (data.pending) { data.pending._session_id=sid; showApprovalCard(data.pending, data.pending_count||1); }
else { hideApprovalCard(); }
} catch(e) { /* ignore poll errors */ }
}, 1500);

188
tests/test_approval_queue.py Normal file
View File

@@ -0,0 +1,188 @@
"""Tests for approval queue multi-entry support (issue #527).
Previously _pending[sid] held one entry, so simultaneous approvals overwrote
each other. This PR changes submit_pending() to append to a list and adds
approval_id so /api/approval/respond can target a specific entry.
"""
import json
import pathlib
import re
import sys
REPO_ROOT = pathlib.Path(__file__).parent.parent.resolve()
sys.path.insert(0, str(REPO_ROOT))
ROUTES_SRC = (REPO_ROOT / "api" / "routes.py").read_text(encoding="utf-8")
MESSAGES_JS = (REPO_ROOT / "static" / "messages.js").read_text(encoding="utf-8")
INDEX_HTML = (REPO_ROOT / "static" / "index.html").read_text(encoding="utf-8")
# ---------------------------------------------------------------------------
# Static-analysis: Python routes
# ---------------------------------------------------------------------------
def test_submit_pending_appends_to_list():
"""submit_pending() must append to a list, not overwrite."""
# The new wrapper must contain queue.append
assert "queue.append(entry)" in ROUTES_SRC, \
"submit_pending() must append entry to a list queue, not overwrite _pending[sid]"
def test_submit_pending_adds_approval_id():
"""Each queued entry must get a unique approval_id."""
assert "approval_id" in ROUTES_SRC and "uuid.uuid4().hex" in ROUTES_SRC, \
"submit_pending() must assign a uuid4 approval_id to each queued entry"
def test_handle_approval_pending_returns_count():
"""_handle_approval_pending must return pending_count in its response."""
assert '"pending_count"' in ROUTES_SRC, \
"_handle_approval_pending must include pending_count in the JSON response"
def test_handle_approval_respond_pops_by_approval_id():
"""_handle_approval_respond must target entry by approval_id."""
assert 'approval_id = body.get("approval_id"' in ROUTES_SRC, \
"_handle_approval_respond must read approval_id from request body"
assert 'entry.get("approval_id") == approval_id' in ROUTES_SRC, \
"_handle_approval_respond must find and pop the matching entry by approval_id"
def test_handle_approval_respond_fallback_to_oldest():
"""When no approval_id is given, fall back to popping the oldest entry (FIFO)."""
# The fallback path: queue.pop(0) when approval_id is empty
assert "queue.pop(0)" in ROUTES_SRC, \
"_handle_approval_respond must fall back to popping the oldest entry when approval_id is absent"
def test_backward_compat_legacy_dict_value():
"""The respond handler must tolerate a legacy single-dict value in _pending."""
assert "Legacy single-dict value" in ROUTES_SRC or \
"# Legacy single-dict" in ROUTES_SRC or \
"elif queue:" in ROUTES_SRC, \
"respond handler must handle legacy single-dict _pending values for backward compatibility"
# ---------------------------------------------------------------------------
# Static-analysis: JavaScript frontend
# ---------------------------------------------------------------------------
def test_respond_sends_approval_id():
"""respondApproval() must include approval_id in the POST body."""
assert "approval_id: approvalId" in MESSAGES_JS, \
"respondApproval() must send approval_id in the POST body to /api/approval/respond"
def test_show_approval_card_accepts_count():
"""showApprovalCard must accept a pendingCount parameter."""
assert re.search(r"function showApprovalCard\(pending,\s*pendingCount\)", MESSAGES_JS), \
"showApprovalCard() must accept a pendingCount argument"
def test_show_approval_card_renders_counter():
"""showApprovalCard must display a '1 of N pending' counter when N > 1."""
assert '"1 of " + pendingCount + " pending"' in MESSAGES_JS or \
"'1 of ' + pendingCount + ' pending'" in MESSAGES_JS, \
"showApprovalCard() must render '1 of N pending' counter for multiple queued approvals"
def test_approval_current_id_tracked():
"""_approvalCurrentId must be set and cleared around each approval."""
assert "_approvalCurrentId" in MESSAGES_JS, \
"_approvalCurrentId must track the approval_id of the currently displayed card"
assert "_approvalCurrentId = pending.approval_id" in MESSAGES_JS or \
"_approvalCurrentId = pending.approval_id || null" in MESSAGES_JS, \
"_approvalCurrentId must be assigned from pending.approval_id"
# Must be nulled on respond
assert "_approvalCurrentId = null" in MESSAGES_JS, \
"_approvalCurrentId must be cleared when respondApproval() is called"
def test_polling_passes_count_to_show():
"""The poll loop must pass pending_count to showApprovalCard."""
assert "showApprovalCard(data.pending, data.pending_count" in MESSAGES_JS, \
"Poll loop must pass data.pending_count to showApprovalCard"
# ---------------------------------------------------------------------------
# HTML: counter element present
# ---------------------------------------------------------------------------
def test_approval_counter_element_exists():
"""index.html must contain an approvalCounter element."""
assert 'id="approvalCounter"' in INDEX_HTML, \
"index.html must contain an element with id='approvalCounter' for the '1 of N' display"
# ---------------------------------------------------------------------------
# Functional: multiple entries behave correctly (via routes module directly)
# ---------------------------------------------------------------------------
def test_multiple_approvals_both_surfaced():
"""Two submit_pending calls must produce two queued entries, not one."""
import threading
from api import routes as r
# Reset state
sid = "test-multi-approval-sid"
with r._lock:
r._pending.pop(sid, None)
r.submit_pending(sid, {"command": "cmd1", "pattern_key": "p1", "pattern_keys": ["p1"], "description": "d1"})
r.submit_pending(sid, {"command": "cmd2", "pattern_key": "p2", "pattern_keys": ["p2"], "description": "d2"})
with r._lock:
queue = r._pending.get(sid)
assert isinstance(queue, list), "After two submit_pending calls, _pending[sid] must be a list"
assert len(queue) == 2, f"Expected 2 queued entries, got {len(queue)}"
assert queue[0]["command"] == "cmd1"
assert queue[1]["command"] == "cmd2"
assert queue[0].get("approval_id"), "First entry must have an approval_id"
assert queue[1].get("approval_id"), "Second entry must have an approval_id"
assert queue[0]["approval_id"] != queue[1]["approval_id"], "Each entry must have a unique approval_id"
# Cleanup
with r._lock:
r._pending.pop(sid, None)
def test_respond_by_approval_id_pops_correct_entry():
"""Responding with approval_id must remove only the targeted entry."""
from api import routes as r
sid = "test-respond-by-id-sid"
with r._lock:
r._pending.pop(sid, None)
r.submit_pending(sid, {"command": "cmd1", "pattern_key": "p1", "pattern_keys": ["p1"], "description": "d1"})
r.submit_pending(sid, {"command": "cmd2", "pattern_key": "p2", "pattern_keys": ["p2"], "description": "d2"})
with r._lock:
queue = r._pending.get(sid, [])
aid2 = queue[1]["approval_id"] if len(queue) > 1 else None
assert aid2, "Second entry must have an approval_id"
# Respond to the SECOND entry by its approval_id
# We call the handler internals directly (no HTTP)
with r._lock:
queue = r._pending.get(sid, [])
popped = None
for i, entry in enumerate(queue):
if entry.get("approval_id") == aid2:
popped = queue.pop(i)
break
assert popped is not None, "Should have found and popped entry by approval_id"
assert popped["command"] == "cmd2", "Popped the wrong entry"
with r._lock:
remaining = r._pending.get(sid, [])
assert len(remaining) == 1, "One entry should remain after popping the second"
assert remaining[0]["command"] == "cmd1", "The remaining entry should be cmd1"
# Cleanup
with r._lock:
r._pending.pop(sid, None)