diff --git a/tests/gateway/test_approve_deny_commands.py b/tests/gateway/test_approve_deny_commands.py index f5ac3b8c40ba..7976838ef05f 100644 --- a/tests/gateway/test_approve_deny_commands.py +++ b/tests/gateway/test_approve_deny_commands.py @@ -515,28 +515,42 @@ class TestBlockingApprovalE2E: assert "BLOCKED" in result_holder[0]["message"] unregister_gateway_notify(session_key) - def test_blocking_approval_timeout(self): - """check_all_command_guards returns BLOCKED on timeout.""" + @pytest.mark.parametrize( + "approval_config", + [ + {"mode": "manual", "timeout": 0}, + {"mode": "manual", "timeout": 0, "gateway_timeout": 300}, + ], + ids=["shared-timeout-only", "shared-timeout-is-canonical"], + ) + def test_blocking_approval_uses_canonical_timeout(self, approval_config, monkeypatch): + """Gateway waits use approvals.timeout, without a second timeout knob.""" + from tools import approval as approval_module from tools.approval import ( - register_gateway_notify, unregister_gateway_notify, check_all_command_guards, + register_gateway_notify, + reset_current_session_key, + resolve_gateway_approval, + set_current_session_key, + unregister_gateway_notify, ) + monkeypatch.setattr(approval_module, "_YOLO_MODE_FROZEN", False) session_key = "e2e-timeout" register_gateway_notify(session_key, lambda d: None) result_holder = [None] def agent_thread(): - from tools.approval import reset_current_session_key, set_current_session_key - token = set_current_session_key(session_key) os.environ["HERMES_GATEWAY_SESSION"] = "1" os.environ["HERMES_EXEC_ASK"] = "1" os.environ["HERMES_SESSION_KEY"] = session_key try: - with patch("tools.approval._get_approval_config", - return_value={"gateway_timeout": 1}): + with patch( + "tools.approval._get_approval_config", + return_value=approval_config, + ): result_holder[0] = check_all_command_guards( "rm -rf /important", "local" ) @@ -548,9 +562,13 @@ class TestBlockingApprovalE2E: t = threading.Thread(target=agent_thread) t.start() - t.join(timeout=10) + t.join(timeout=1) + if t.is_alive(): + resolve_gateway_approval(session_key, "deny") + t.join(timeout=5) assert result_holder[0]["approved"] is False + assert result_holder[0]["outcome"] == "timeout" assert "timed out" in result_holder[0]["message"] unregister_gateway_notify(session_key) diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index e65a73333434..253c3382049b 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -2142,7 +2142,7 @@ class TestApprovalTimeoutIsNotConsent: SESSION_KEY = "test-no-consent-session" def setup_method(self): - """Reset module state and force tight gateway_timeout for fast tests.""" + """Reset module state and force a tight approval timeout for fast tests.""" from tools import approval as mod mod._gateway_queues.clear() mod._gateway_notify_cbs.clear() @@ -2179,7 +2179,7 @@ class TestApprovalTimeoutIsNotConsent: from tools import approval as mod monkeypatch.setattr( mod, "_get_approval_config", - lambda: {"mode": "manual", "gateway_timeout": seconds, "timeout": seconds}, + lambda: {"mode": "manual", "timeout": seconds}, ) def test_timeout_returns_approved_false_with_no_consent(self, monkeypatch): diff --git a/tests/tools/test_approval_interrupt.py b/tests/tools/test_approval_interrupt.py index b991afd80883..2ef91f752c32 100644 --- a/tests/tools/test_approval_interrupt.py +++ b/tests/tools/test_approval_interrupt.py @@ -73,7 +73,7 @@ class TestApprovalInterrupt: # Force a long timeout so a *passing* test can only happen via the # interrupt path, never by the deadline elapsing. - mod._get_approval_config = lambda: {"gateway_timeout": 300} + mod._get_approval_config = lambda: {"timeout": 300} approval_data = { "command": "rm -rf /tmp/whatever", @@ -128,7 +128,7 @@ class TestApprovalInterrupt: # Short timeout so the test finishes fast via the deadline, proving the # foreign interrupt did not short-circuit the wait. - mod._get_approval_config = lambda: {"gateway_timeout": 1} + mod._get_approval_config = lambda: {"timeout": 1} approval_data = { "command": "rm -rf /tmp/whatever", diff --git a/tests/tools/test_execute_code_approval_cluster.py b/tests/tools/test_execute_code_approval_cluster.py index c5d7f3fb78c9..b51a1c130cc5 100644 --- a/tests/tools/test_execute_code_approval_cluster.py +++ b/tests/tools/test_execute_code_approval_cluster.py @@ -111,8 +111,10 @@ def gw_session(monkeypatch): monkeypatch.delenv("HERMES_INTERACTIVE", raising=False) monkeypatch.delenv("HERMES_CRON_SESSION", raising=False) monkeypatch.delenv("HERMES_EXEC_ASK", raising=False) - # Force manual mode regardless of host config. + # Force manual mode regardless of host config and disable any process-level + # yolo inherited from the developer's live environment. monkeypatch.setattr(A, "_get_approval_mode", lambda: "manual") + monkeypatch.setattr(A, "_YOLO_MODE_FROZEN", False) session_key = "cluster-test-session" token = A.set_current_session_key(session_key) @@ -231,11 +233,21 @@ def test_guard_gateway_user_denies_blocks(gw_session): assert res["user_consent"] is False -def test_guard_gateway_timeout_blocks(gw_session, monkeypatch): +@pytest.mark.parametrize( + "approval_config", + [ + {"timeout": 0}, + {"timeout": 0, "gateway_timeout": 300}, + ], + ids=["shared-timeout-only", "shared-timeout-is-canonical"], +) +def test_guard_gateway_wait_uses_canonical_timeout( + gw_session, monkeypatch, approval_config +): # Register a callback that never resolves; force an immediate timeout. with A._lock: A._gateway_notify_cbs[gw_session] = lambda _d: None - monkeypatch.setattr(A, "_get_approval_config", lambda: {"gateway_timeout": 0}) + monkeypatch.setattr(A, "_get_approval_config", lambda: approval_config) res = A.check_execute_code_guard("import os", "local") assert res["approved"] is False assert res["outcome"] == "timeout" diff --git a/tools/approval.py b/tools/approval.py index 836066eda982..1ab321cacc2c 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -2493,15 +2493,12 @@ def _await_gateway_decision(session_key: str, notify_cb, approval_data: dict, _drop_entry() return {"resolved": False, "choice": None, "notify_failed": True} - # Block until the user responds or timeout (default 5 min). Poll in short - # slices so we can fire activity heartbeats every ~10s to the agent's - # inactivity tracker — otherwise the gateway watchdog kills the agent - # while the user is still responding. Mirrors _wait_for_process() cadence. - timeout = _get_approval_config().get("gateway_timeout", 300) - try: - timeout = int(timeout) - except (ValueError, TypeError): - timeout = 300 + # Block until the user responds or the canonical approval timeout elapses + # (default 60s). Poll in short slices so we can fire activity heartbeats + # every ~10s to the agent's inactivity tracker — otherwise the gateway + # watchdog kills the agent while the user is still responding. Mirrors + # _wait_for_process() cadence. + timeout = _get_approval_timeout() try: from tools.environments.base import touch_activity_if_due