Files
hermes-agent/tests/tools/test_approval_interrupt.py
teknium1 6332216384 fix(approval): withdrawn gateway approval prompts no longer read as a user deny
When a gateway approval wait ends without anyone answering — the parent's
delegate_task finishing and tearing the child down, a /stop, or the turn's
notifier being unregistered at turn end — the tool result said
"BLOCKED: Command denied by user" (outcome="denied", user_summary "You denied
this command"). The user never saw or answered the prompt, so the parent agent
went on reasoning about a refusal that never happened (#112026, #22992).

The action stays fail-closed (the command does not run, the model still gets
the NOT-consented stop text), but the attribution is now truthful:

- tools/approval_gateway_wait.py: `_cancel_cause()` reads the existing
  per-thread interrupt-cause channel (`get_interrupt_reason()`, a trusted fixed
  category — no string matching) for the interrupted state and marks a
  notifier-unregister wake (event set, result None) as "the turn ended before
  the prompt was answered". Both the direct and the coalesced-follower wait
  return `cancelled=<cause>`; the post_approval_response hook fires
  choice="cancelled" instead of "deny"/"timeout".
- tools/approval.py: a cancelled decision renders
  "BLOCKED: Command approval was withdrawn before the user answered (<cause>)."
  with outcome="cancelled" and its own user_summary; an explicit /deny is
  untouched.
- tools/delegate_tool_child_run.py: `_signal_child_stop` publishes a fixed
  tool_reason ("parent delegation ended"; the late-child mirror forwards the
  parent's own category) so a child's pending approval can tell teardown from a
  user /stop — previously it rode the default "explicit stop requested".
- tools/file_tools_write_guards.py / tools/approval_prompt.py: the protected
  instruction-file gate and MCP elicitation consume the same key instead of
  reporting "denied by the user" / "decline".

Co-authored-by: zccyman <16263913+zccyman@users.noreply.github.com>
Co-authored-by: KoNit-K <124019182+KoNit-K@users.noreply.github.com>
2026-09-15 18:44:46 -07:00

166 lines
6.5 KiB
Python

"""Regression: a blocking gateway approval wait must honor an interrupt (#8697).
When an agent calls a dangerous command, the gateway approval flow blocks the
agent's execution thread inside ``_await_gateway_decision`` on
``threading.Event.wait()`` until the user responds or the 5-minute approval
timeout elapses. Before the fix, ``/stop`` (which calls
``AIAgent.interrupt()`` → per-thread interrupt flag) was silently ignored by
that wait loop, so the session stayed wedged until the timeout fired.
The fix checks ``is_interrupted()`` at the top of the poll loop. Because the
wait runs on the agent's execution thread — the exact thread
``AIAgent.interrupt()`` flags — the check sees the signal and resolves the
pending approval as ``deny`` so the agent loop unwinds cleanly.
"""
import os
import threading
import time
def _clear_approval_state():
"""Reset all module-level approval state between tests."""
from tools import approval as mod
mod._gateway_queues.clear()
mod._gateway_notify_cbs.clear()
mod._session_approved.clear()
mod._permanent_approved.clear()
mod._pending.clear()
class TestApprovalInterrupt:
SESSION_KEY = "interrupt-test-session"
def setup_method(self):
from tools.interrupt import set_interrupt
from tools import interrupt as _interrupt_mod
_clear_approval_state()
# Wipe ALL per-thread interrupt bits — thread idents are recycled by
# the OS, so a bit set on a now-dead thread in a prior test can leak
# onto a fresh worker that happens to reuse the ident.
with _interrupt_mod._lock:
_interrupt_mod._interrupted_threads.clear()
set_interrupt(False)
self._saved_env = {
k: os.environ.get(k)
for k in ("HERMES_GATEWAY_SESSION", "HERMES_YOLO_MODE",
"HERMES_SESSION_KEY")
}
os.environ.pop("HERMES_YOLO_MODE", None)
os.environ["HERMES_GATEWAY_SESSION"] = "1"
os.environ["HERMES_SESSION_KEY"] = self.SESSION_KEY
def teardown_method(self):
from tools.interrupt import set_interrupt
from tools import interrupt as _interrupt_mod
with _interrupt_mod._lock:
_interrupt_mod._interrupted_threads.clear()
set_interrupt(False)
for k, v in self._saved_env.items():
if v is None:
os.environ.pop(k, None)
else:
os.environ[k] = v
_clear_approval_state()
def test_interrupt_unblocks_pending_approval_quickly(self, monkeypatch):
"""An interrupt on the waiting thread must resolve the wait as deny
well before the (here, intentionally long) approval timeout."""
from tools import approval as mod
from tools import approval_context
from tools.interrupt import set_interrupt
# Force a long timeout so a *passing* test can only happen via the
# interrupt path, never by the deadline elapsing.
monkeypatch.setattr(approval_context, "_get_approval_config", lambda: {"timeout": 300})
approval_data = {
"command": "rm -rf /tmp/whatever",
"description": "recursive delete",
"pattern_key": "rm_rf",
"pattern_keys": ["rm_rf"],
}
result_holder = {}
notified = threading.Event()
def _notify_cb(_data):
# Mimic the gateway: a callback is registered and invoked once the
# approval is enqueued. We just record that the user *would* have
# been prompted.
notified.set()
def _worker():
result_holder["result"] = mod._await_gateway_decision(
self.SESSION_KEY, _notify_cb, approval_data
)
result_holder["thread_id"] = threading.get_ident()
t = threading.Thread(target=_worker, daemon=True)
start = time.monotonic()
t.start()
# Wait until the worker has enqueued + notified, proving it is actually
# blocked inside the poll loop.
assert notified.wait(timeout=5), "approval was never enqueued/notified"
# Simulate /stop: AIAgent.interrupt() flags the agent's execution
# thread. Here the worker thread *is* that execution thread.
set_interrupt(True, t.ident)
t.join(timeout=10)
elapsed = time.monotonic() - start
assert not t.is_alive(), "approval wait did not return after interrupt"
result = result_holder["result"]
assert (result["resolved"], result["choice"], result["reason"]) == (True, "deny", None)
# A bare interrupt bit (no cause published) is still reported as a withdrawn prompt.
assert result["cancelled"] == "turn interrupted"
# Must be far below the 300s timeout — the interrupt, not the deadline,
# is what released the wait.
assert elapsed < 10, f"interrupt path too slow ({elapsed:.1f}s)"
# Queue entry was cleaned up.
assert not mod.has_blocking_approval(self.SESSION_KEY)
def test_unrelated_thread_interrupt_does_not_unblock(self, monkeypatch):
"""An interrupt flagged on a *different* thread must NOT release this
session's approval wait — interrupts are thread-scoped."""
from tools import approval as mod
from tools import approval_context
from tools.interrupt import set_interrupt
# Short timeout so the test finishes fast via the deadline, proving the
# foreign interrupt did not short-circuit the wait.
monkeypatch.setattr(approval_context, "_get_approval_config", lambda: {"timeout": 1})
approval_data = {
"command": "rm -rf /tmp/whatever",
"description": "recursive delete",
"pattern_key": "rm_rf",
"pattern_keys": ["rm_rf"],
}
result_holder = {}
notified = threading.Event()
def _notify_cb(_data):
notified.set()
def _worker():
result_holder["result"] = mod._await_gateway_decision(
self.SESSION_KEY, _notify_cb, approval_data
)
t = threading.Thread(target=_worker, daemon=True)
t.start()
assert notified.wait(timeout=5)
# Flag an interrupt on a thread that is NOT the worker.
set_interrupt(True, threading.get_ident())
t.join(timeout=10)
assert not t.is_alive()
# Timed out (no resolution) because the foreign interrupt was ignored.
assert result_holder["result"] == {"resolved": False, "choice": None, "reason": None}