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>
166 lines
6.5 KiB
Python
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}
|