diff --git a/tests/gateway/test_session_boundary_security_state.py b/tests/gateway/test_session_boundary_security_state.py index e1cf181e02..ce7afd7806 100644 --- a/tests/gateway/test_session_boundary_security_state.py +++ b/tests/gateway/test_session_boundary_security_state.py @@ -208,8 +208,11 @@ def test_clear_session_boundary_security_state_wakes_blocked_approvals(): runner._clear_session_boundary_security_state(session_key) assert target_entry.event.is_set() - assert target_entry.result == "deny" + # Withdrawn, not denied: the waiter renders outcome="cancelled" rather than a user deny. + assert target_entry.result is None + assert target_entry.cancelled assert other_entry.event.is_set() is False assert other_entry.result is None + assert other_entry.cancelled is None assert session_key not in approval_mod._gateway_queues assert other_key in approval_mod._gateway_queues diff --git a/tests/tools/test_approval_cancelled_attribution.py b/tests/tools/test_approval_cancelled_attribution.py index 608bdc5b80..92d8415504 100644 --- a/tests/tools/test_approval_cancelled_attribution.py +++ b/tests/tools/test_approval_cancelled_attribution.py @@ -5,6 +5,7 @@ end, a ``/stop``) end the wait fail-closed, but the tool result carries ``outcom and the cause instead of "denied by user" (#112026, #22992). """ import threading +import time import pytest @@ -94,3 +95,35 @@ def test_turn_end_unregister_reports_withdrawn_prompt(gateway_session): denied = holder["result"] assert denied["outcome"] == "denied" assert "denied by user" in denied["message"] + + +def test_session_boundary_teardown_reports_withdrawn_prompt(gateway_session): + """``clear_session`` (/new, /reset, auto-reset) wakes the wait with no decision: the result + is a withdrawn prompt, not a user deny.""" + thread, holder = _run_gate_until_pending() + mod.clear_session(SESSION_KEY) + thread.join(timeout=10) + assert not thread.is_alive() + _assert_withdrawn(holder["result"], "the session ended before the prompt was answered") + + +def test_coalesced_follower_inherits_the_leaders_cancellation(gateway_session): + """A follower coalesced onto an interrupted leader wakes with the leader's cause, not a deny.""" + leader_thread, leader = _run_gate_until_pending() + follower = {} + follower_thread = threading.Thread( + target=lambda: follower.__setitem__("result", mod.check_all_command_guards("rm -rf .git", "local"))) + follower_thread.start() # identical command → coalesces onto the leader, no second prompt + deadline = time.monotonic() + 10 + while not any(n == "pre_approval_request" and kw.get("coalesced") for n, kw in gateway_session): + assert time.monotonic() < deadline, "follower never coalesced onto the leader" + time.sleep(0.05) + set_interrupt(True, leader["tid"], reason="parent delegation ended") + try: + leader_thread.join(timeout=10) + follower_thread.join(timeout=10) + finally: + set_interrupt(False, leader["tid"]) + assert not leader_thread.is_alive() and not follower_thread.is_alive() + _assert_withdrawn(leader["result"], "parent delegation ended") + _assert_withdrawn(follower["result"], "parent delegation ended") diff --git a/tools/approval.py b/tools/approval.py index 29db8b4f74..899386b3e3 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -268,8 +268,9 @@ def clear_session(session_key: str) -> None: _pending.pop(session_key, None) entries = _gateway_queues.pop(session_key, []) for entry in entries: - # Cancel blocked waits now so the old run unwinds instead of idling until timeout. - entry.result = "deny" + # Cancel blocked waits now so the old run unwinds instead of idling until timeout; + # the prompt was withdrawn, nobody denied it. + entry.cancelled = "the session ended before the prompt was answered" entry.event.set() _release_permission_mode_dependents(session_key) # Session-persistent code kernels (local and remote) share this owner key and die at the same boundary so a diff --git a/tools/approval_gateway_wait.py b/tools/approval_gateway_wait.py index 5f454b8e1d..5a3f8f2cb0 100644 --- a/tools/approval_gateway_wait.py +++ b/tools/approval_gateway_wait.py @@ -24,7 +24,7 @@ logger = logging.getLogger("tools.approval") class _ApprovalEntry: """One pending dangerous-command approval inside a gateway session.""" - __slots__ = ("event", "data", "result", "reason", "acknowledged", "settle") + __slots__ = ("event", "data", "result", "reason", "acknowledged", "settle", "cancelled") def __init__(self, data: dict): self.event = threading.Event() @@ -37,6 +37,9 @@ class _ApprovalEntry: self.result: str | None = None # "once"|"session"|"always"|"deny" # Free-text reason from ``/deny `` so the agent can adapt, not just hear "denied". self.reason: str | None = None + # Why the prompt was withdrawn with nobody answering (interrupt cause, session teardown); + # followers and teardown read it so a withdrawn prompt never renders as a user deny. + self.cancelled: str | None = None def _poll_event(event: threading.Event, session_key: str, *, interrupt_log: str) -> str: @@ -70,15 +73,16 @@ def _poll_event(event: threading.Event, session_key: str, *, interrupt_log: str) heartbeat() -def _cancel_cause(state: str, result: str | None) -> str | None: +def _cancel_cause(state: str, entry) -> str | None: """Why the wait ended with nobody answering: the turn was interrupted (cause from the - per-thread channel — a user /stop or a parent's delegation teardown) or the turn ended + per-thread channel — a user /stop or a parent's delegation teardown), the entry was + withdrawn with a stamped cause (leader interrupted, session torn down), or the turn ended under the prompt (notifier unregistered, result never set). ``None`` for a real answer or a plain timeout.""" if state == "interrupted": return get_interrupt_reason() or "turn interrupted" - if state == "set" and result is None: - return "the turn ended before the prompt was answered" + if state == "set" and entry.result is None: + return entry.cancelled or "the turn ended before the prompt was answered" return None @@ -107,7 +111,7 @@ def _await_coalesced_leader(session_key: str, leader, payload: dict): state = _poll_event(leader.event, session_key, interrupt_log="Coalesced approval wait interrupted — " "returning deny for session %s") - cancelled = _cancel_cause(state, leader.result) + cancelled = _cancel_cause(state, leader) if state == "interrupted": # Deny only OUR follower; the leader thread handles its own signal. choice, resolved = "deny", True @@ -187,10 +191,13 @@ def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict, *, state = _poll_event(entry.event, session_key, interrupt_log="Approval wait interrupted — returning deny for session %s") - cancelled = _cancel_cause(state, entry.result) + cancelled = _cancel_cause(state, entry) + choice = entry.result if state == "interrupted": - entry.result = "deny" + # Our own decision stays a fail-closed deny; coalesced followers wake with the + # cause instead of a deny nobody issued. + choice, entry.cancelled = "deny", cancelled entry.event.set() _drop_entry("answered" if state == "set" else state) extra = {"cancelled": cancelled} if cancelled else {} - return _finish(payload, state != "timeout", entry.result, entry.reason, **extra) + return _finish(payload, state != "timeout", choice, entry.reason, **extra)