fix: queue simultaneous approval requests per session (fixes #527)

[nesquena-hermes]

Summary

Fixes #527 — simultaneous approval requests overwrote each other.

When an agentic session spawned parallel tool calls that each needed approval, only the last one was ever visible in the UI. Earlier approvals were silently dropped.

Changes

api/routes.py

• New submit_pending() wrapper appends to a list and assigns a stable approval_id (uuid4) to each entry — instead of overwriting _pending[sid] with a single dict
• _handle_approval_pending() returns {pending: <first entry>, pending_count: N} so the UI can show "1 of N"
• _handle_approval_respond() pops by approval_id (falls back to oldest entry for backward-compat with clients that don't send it)
• Legacy single-dict values handled without crashing

static/messages.js

• respondApproval() sends approval_id in the POST body
• showApprovalCard(pending, pendingCount) shows "1 of N pending" counter when multiple are queued
• _approvalCurrentId tracks which approval is displayed so respond targets the right one
• Poll loop passes pending_count through to showApprovalCard

static/index.html

• Adds <div id="approvalCounter"> inside the approval card for the "1 of N" display

tests/test_approval_queue.py

• 14 tests: static-analysis (Python + JS + HTML) + functional tests that inject two simultaneous approvals and verify both are queued and independently resolvable by approval_id

Test results

1302/1302 passing (+14 new). No regressions.

Closes #527

[nesquena]

Independent Review: PR #546 — approval queue multi-entry support

Security Audit

Clean. The submit_pending wrapper copies the approval dict and adds a uuid4 approval_id under _lock. The approval_id from the frontend is used only for dict key comparison (string equality), never for filesystem ops or command execution. The choice parameter is validated against a fixed set (once, session, always, deny). No injection vectors.

Code Review

Problem: _pending[sid] held a single dict. When subagents spawned multiple parallel tool calls requiring approval, each submit_pending overwrote the previous one. Only the last approval was visible.

Fix: Three-layer solution that's clean and backward-compatible:

1. submit_pending() wrapper — Appends to a list instead of overwriting. Each entry gets a uuid4 approval_id. Correctly handles the legacy case where _pending[sid] is a dict (wraps it in a list). The comment about NOT calling _submit_pending_raw is important and correct — the raw agent function would undo the list.

2. _handle_approval_pending() — Returns the first entry from the queue (FIFO) plus pending_count so the frontend knows how many are queued. Handles both list and legacy dict formats.

3. _handle_approval_respond() — Targets a specific entry by approval_id. Falls back to FIFO pop when no approval_id is given (backward compat with old clients). Cleans up the _pending entry when the queue is empty.

Frontend changes (messages.js):

• showApprovalCard() now accepts pendingCount and shows "1 of N pending" counter
• _approvalCurrentId tracks which approval is displayed, sent back in the respond POST
• Polling passes pending_count through to showApprovalCard
• respondApproval() includes approval_id in the POST body

One design note: The current UI shows only the first queued approval and reveals the next one after the user responds. This is the right UX — showing all N simultaneously would overwhelm users. The "1 of N pending" counter gives enough context.

One thing to verify (can't test without a browser): After responding to approval 1 of N, does the next approval card appear immediately? The poll interval is 1.5s, so there could be a brief gap. If the SSE approval event fires for each queued entry, this is handled. If not, the user waits up to 1.5s for the next poll to pick up entry #2. Either way it works — just a UX smoothness question.

Tests

14 new tests — all pass. Good mix:

• 6 static Python tests (queue append, approval_id, pending_count, respond-by-id, FIFO fallback, legacy compat)
• 5 static JS tests (approval_id in POST, showApprovalCard signature, counter render, tracking, poll passthrough)
• 1 HTML test (counter element exists)
• 2 functional tests (multiple entries both surfaced, respond-by-id pops correct entry)

Full suite: 1244 passed, 0 regressions, 58 skipped. The 2 test_onboarding_existing_config failures are pre-existing on master.

Verdict

Approved. Clean, well-tested fix for a real concurrency bug. Backward-compatible with both old agent versions (legacy dict) and old frontend clients (no approval_id). Ready to merge.