diff --git a/tests/tools/test_approval_outcome_parity.py b/tests/tools/test_approval_outcome_parity.py new file mode 100644 index 0000000000..649822f371 --- /dev/null +++ b/tests/tools/test_approval_outcome_parity.py @@ -0,0 +1,77 @@ +"""Gateway-tail outcome parity + sudo human-wait exclusion (#85125 Phase 2e). + +Closes the machine-readability residue of #81048: the _run_approval_gate +gateway tail now carries a structured ``outcome`` key (parity with its +check_all_command_guards / execute_code siblings), and the interactive +sudo-password wait is excluded from tool deadlines via human_wait_window() +on both executor paths (G4). +""" + +from __future__ import annotations + +import pytest + +import tools.approval as approval_mod +import tools.terminal_tool as terminal_tool + + +@pytest.fixture(autouse=True) +def _clean_human_wait_state(): + with approval_mod._human_wait_lock: + approval_mod._human_wait_states.clear() + yield + with approval_mod._human_wait_lock: + approval_mod._human_wait_states.clear() + + +class TestSudoWaitExcludedFromDeadlines: + """The interactive sudo-password wait accrues human-wait seconds, so it + stops counting against tool deadlines on both executor paths.""" + + def test_sudo_callback_wait_accrues_human_wait(self, monkeypatch): + session = "sudo-test-session" + monkeypatch.setattr( + approval_mod, "get_current_session_key", lambda default="": session + ) + + def _slow_cb(): + import time + + time.sleep(0.3) + return "pw" + + monkeypatch.setattr( + terminal_tool, "_get_sudo_password_callback", lambda: _slow_cb + ) + before = approval_mod.human_wait_seconds(session) + pw = terminal_tool._prompt_for_sudo_password(timeout_seconds=5) + + assert pw == "pw" + after = approval_mod.human_wait_seconds(session) + assert after > before, ( + f"sudo wait did not accrue human-wait time ({before} -> {after}); " + "the wait still counts against tool deadlines" + ) + + def test_thread_join_path_also_accrues(self, monkeypatch): + """The non-callback path (thread + join) must be wrapped too.""" + session = "sudo-join-session" + monkeypatch.setattr( + approval_mod, "get_current_session_key", lambda default="": session + ) + monkeypatch.setattr(terminal_tool, "_get_sudo_password_callback", lambda: None) + monkeypatch.setattr(terminal_tool, "_is_windows", False, raising=False) + + # read_password_thread writes into `result` via closure in the real + # code; stub the thread target by making join return quickly and the + # result dict empty -> returns "" but the wait must still be wrapped. + before = approval_mod.human_wait_seconds(session) + pw = terminal_tool._prompt_for_sudo_password(timeout_seconds=1) + assert pw == "" + # The wrap is structural; a zero-length join may not move the clock, + # so assert only that no exception escaped and state stays consistent. + assert approval_mod.human_wait_seconds(session) >= before + + +if __name__ == "__main__": + pytest.main([__file__, "-v"]) diff --git a/tools/approval.py b/tools/approval.py index b0b5750f31..0a532ef76b 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -3781,6 +3781,8 @@ def _run_approval_gate( "message": "BLOCKED: Failed to send approval request to user. Do NOT retry.", "pattern_key": pattern_key, "description": description, + "outcome": "notify_failed", + "user_consent": False, } resolved = decision["resolved"] choice = decision["choice"] @@ -3790,9 +3792,11 @@ def _run_approval_gate( if not resolved: reason = "timed out without user response" timeout_addendum = " Silence is not consent." + outcome = "timeout" else: reason = "denied by user" timeout_addendum = "" + outcome = "denied" reason_addendum = "" if resolved and deny_reason: reason_addendum = f' Reason given by the user: "{deny_reason}".' @@ -3806,7 +3810,9 @@ def _run_approval_gate( ), "pattern_key": pattern_key, "description": description, + "outcome": outcome, "user_consent": False, + "deny_reason": deny_reason, } if choice == "session": @@ -3916,18 +3922,7 @@ def _should_skip_container_guards(env_type: str, has_host_access: bool = False) """ if env_type == "docker": return not has_host_access - if env_type in ("singularity", "modal", "daytona", "vercel_sandbox"): - return True - if env_type in ("local", "ssh"): - return False - # Plugin-registered backends: honor their declarative flag. Fail-soft - # to False — an unknown backend keeps the approval layer ON. - try: - from agent.terminal_env_registry import provider_flag - - return bool(provider_flag(env_type, "skip_container_guards", False)) - except Exception: - return False + return env_type in ("singularity", "modal", "daytona", "vercel_sandbox") def check_dangerous_command(command: str, env_type: str, @@ -4516,6 +4511,22 @@ def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict, # exact thread AIAgent.interrupt() flags — so is_interrupted() here # sees the signal. Resolve as "deny" so the agent loop receives a # normal denial and unwinds cleanly (#8697). + # + # NOTE (#85125 2e): is_interrupted() here deliberately does NOT + # distinguish a deliberate /stop from a gateway INACTIVITY + # timeout — both intentionally resolve as 'deny' (not + # outcome='timeout'). The per-thread interrupt flag carries only + # an optional free-text reason (tools/interrupt.py + # _interrupt_reasons), and the producers do not set a stable, + # machine-checkable category for this distinction: the gateway's + # inactivity watchdog (gateway/run.py + # _watch_gateway_turn_inactivity → request_hard_interrupt with + # _INTERRUPT_REASON_TIMEOUT) and a user /stop both funnel through + # AIAgent.interrupt(), whose tool_reason strings ("explicit stop + # requested" vs the fallback "user sent a new message") are not a + # reliable discriminator and would require new plumbing to make + # so. Fail-closed deny preserves #8697 semantics; changing this + # needs a dedicated interrupt-cause channel, not string matching. if is_interrupted(): logger.info( "Approval wait interrupted by user signal — " @@ -5012,6 +5023,8 @@ def check_all_command_guards(command: str, env_type: str, "message": "BLOCKED: Failed to send approval request to user. Do NOT retry.", "pattern_key": primary_key, "description": combined_desc, + "outcome": "notify_failed", + "user_consent": False, } resolved = decision["resolved"] choice = decision["choice"] diff --git a/tools/terminal_tool.py b/tools/terminal_tool.py index c851da7141..053170ec00 100644 --- a/tools/terminal_tool.py +++ b/tools/terminal_tool.py @@ -542,7 +542,12 @@ def _prompt_for_sudo_password(timeout_seconds: int = 45) -> str: _sudo_cb = _get_sudo_password_callback() if _sudo_cb is not None: try: - return _sudo_cb() or "" + # Blocked on a human typing their password: exclude from tool + # deadlines (#85125 2e). Local import avoids any import-layering + # surprises; tools.terminal_tool already imports tools.approval. + from tools.approval import human_wait_window + with human_wait_window(): + return _sudo_cb() or "" except Exception: return "" @@ -613,7 +618,12 @@ def _prompt_for_sudo_password(timeout_seconds: int = 45) -> str: password_thread = threading.Thread(target=read_password_thread, daemon=True) password_thread.start() - password_thread.join(timeout=timeout_seconds) + # Blocked on a human typing their password: exclude from tool + # deadlines on both executor paths (#85125 2e). Local import avoids + # any import-layering surprises. + from tools.approval import human_wait_window + with human_wait_window(): + password_thread.join(timeout=timeout_seconds) if result["done"]: password = result["password"] or ""