diff --git a/acp_adapter/permissions.py b/acp_adapter/permissions.py index 5f29a96725..b10b2a169e 100644 --- a/acp_adapter/permissions.py +++ b/acp_adapter/permissions.py @@ -158,9 +158,16 @@ def make_approval_callback( try: response = future.result(timeout=timeout) - except (FutureTimeout, Exception) as exc: + except FutureTimeout: future.cancel() - logger.warning("Permission request timed out or failed: %s", exc) + logger.warning("Permission request timed out after %ss", timeout) + # Distinct from an explicit deny: the client never answered. + # tools.approval callers report this as "timed out without user + # response" instead of a user denial. + return "timeout" + except Exception as exc: + future.cancel() + logger.warning("Permission request failed: %s", exc) return "deny" if response is None: diff --git a/agent/transports/codex_app_server_session.py b/agent/transports/codex_app_server_session.py index e2ace753bc..384a7de8ab 100644 --- a/agent/transports/codex_app_server_session.py +++ b/agent/transports/codex_app_server_session.py @@ -1259,6 +1259,8 @@ def _approval_choice_to_codex_decision(choice: str) -> str: return "accept" if choice in {"session", "always"}: return "acceptForSession" + # "deny" and "timeout" both map to decline — codex has no wire value for + # "prompt expired"; the Hermes-side messaging already distinguishes them. return "decline" diff --git a/cli.py b/cli.py index 95ce2b18f8..55b94c146a 100644 --- a/cli.py +++ b/cli.py @@ -13229,7 +13229,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): self._approval_deadline = 0 self._paint_now() _cprint(f"\n{_DIM} ⏱ Timeout — denying command{_RST}") - return "deny" + self._persist_prompt_summary( + "⚠", "Approval", command, "timed out (no response)", + ) + return "timeout" def _approval_choices(self, command: str, *, allow_permanent: bool = True, smart_denied: bool = False) -> list[str]: @@ -13261,6 +13264,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): "session": "approve_session", "always": "always_approve", "deny": "deny", + "timeout": "timeout", }.get(verdict, "deny") def _handle_approval_selection(self) -> None: diff --git a/hermes_cli/callbacks.py b/hermes_cli/callbacks.py index 69f4b6d405..aad0542d28 100644 --- a/hermes_cli/callbacks.py +++ b/hermes_cli/callbacks.py @@ -250,4 +250,4 @@ def approval_callback(cli, command: str, description: str) -> str: if hasattr(cli, "_app") and cli._app: cli._app.invalidate() cprint(f"\n{_DIM} ⏱ Timeout — denying command{_RST}") - return "deny" + return "timeout" diff --git a/tests/acp/test_permissions.py b/tests/acp/test_permissions.py index 649a388f01..3d8576d3e2 100644 --- a/tests/acp/test_permissions.py +++ b/tests/acp/test_permissions.py @@ -119,7 +119,7 @@ class TestApprovalBridge: assert result == "always" - def test_timeout_returns_deny_and_cancels_future(self): + def test_timeout_returns_timeout_and_cancels_future(self): loop = MagicMock(spec=asyncio.AbstractEventLoop) request_permission = AsyncMock(name="request_permission") future = MagicMock(spec=Future) @@ -138,7 +138,9 @@ class TestApprovalBridge: scheduled["coro"].close() - assert result == "deny" + # A no-response expiry is classified as "timeout" (still blocked, + # fail-closed) so the agent isn't told the user explicitly refused. + assert result == "timeout" assert scheduled["loop"] is loop assert future.cancel.call_count == 1 diff --git a/tests/tools/test_approval.py b/tests/tools/test_approval.py index 61525aec0d..9c4b0cbc68 100644 --- a/tests/tools/test_approval.py +++ b/tests/tools/test_approval.py @@ -1397,3 +1397,109 @@ class TestApprovalPromptRedaction: # The script's credential must not appear in the user-facing message. assert "sk-proj-abc123xyz4567890abcdef" not in result["message"] assert "sk-proj-abc123xyz4567890abcdef" not in result["command"] + + +class TestCliApprovalTimeoutClassifiedSeparately: + """CLI-path parity for the timeout-vs-deny distinction. + + The gateway wait already reported "timed out without user response"; + the CLI/TUI callback path collapsed a prompt timeout into "deny", so + the agent was told the user *refused* when the user simply never + answered. The prompt now returns a distinct "timeout" choice and both + guard tails classify it with outcome="timeout" + a "Silence is not + consent." message. + """ + + def _interactive_env(self): + return mock_patch.dict( + "os.environ", + {"HERMES_INTERACTIVE": "1"}, + clear=False, + ) + + def test_prompt_returns_timeout_when_input_never_arrives(self): + """The raw input() path returns 'timeout', not 'deny', on expiry.""" + import builtins + from unittest.mock import patch as _patch + + def _hang(_prompt=""): + time.sleep(10) + return "" + + with _patch.object(builtins, "input", _hang): + result = prompt_dangerous_approval( + "rm -rf /var/data", "recursive delete", + timeout_seconds=0.05, + ) + assert result == "timeout" + + def test_guard_classifies_callback_timeout_as_timeout(self, monkeypatch): + """check_all_command_guards: a 'timeout' choice from the CLI callback + yields outcome='timeout' and a no-response message, not 'denied by + user'.""" + from unittest.mock import patch as _patch + from tools import approval as mod + + mod._session_approved.clear() + mod._permanent_approved.clear() + + cfg = {"approvals": {"mode": "manual"}} + with self._interactive_env(): + with _patch("hermes_cli.config.load_config_readonly", return_value=cfg): + result = mod.check_all_command_guards( + "rm -rf /var/data", "local", + approval_callback=lambda *a, **kw: "timeout", + ) + + assert result["approved"] is False + assert result.get("outcome") == "timeout" + assert result.get("user_consent") is False + msg = result["message"] + assert "timed out without user response" in msg + assert "Silence is not consent" in msg + assert "denied" not in msg.lower() + + def test_guard_still_classifies_explicit_deny_as_denied(self): + """Explicit CLI deny keeps outcome='denied' and the denial wording.""" + from unittest.mock import patch as _patch + from tools import approval as mod + + mod._session_approved.clear() + mod._permanent_approved.clear() + + cfg = {"approvals": {"mode": "manual"}} + with self._interactive_env(): + with _patch("hermes_cli.config.load_config_readonly", return_value=cfg): + result = mod.check_all_command_guards( + "rm -rf /var/data", "local", + approval_callback=lambda *a, **kw: "deny", + ) + + assert result["approved"] is False + assert result.get("outcome") == "denied" + assert "denied" in result["message"].lower() + assert "Silence is not consent" not in result["message"] + + def test_run_approval_gate_cli_timeout_is_not_a_denial(self): + """The shared plugin-escalation gate (_run_approval_gate) also + distinguishes a prompt timeout from an explicit deny on the CLI + path.""" + from unittest.mock import patch as _patch + from tools import approval as mod + + mod._session_approved.clear() + mod._permanent_approved.clear() + + cfg = {"approvals": {"mode": "manual"}} + with self._interactive_env(): + with _patch("hermes_cli.config.load_config_readonly", return_value=cfg): + result = mod.request_tool_approval( + "write_file", "plugin flagged this write", + approval_callback=lambda *a, **kw: "timeout", + ) + + assert result["approved"] is False + assert result.get("outcome") == "timeout" + assert result.get("user_consent") is False + assert "timed out without user response" in result["message"] + assert "Silence is not consent" in result["message"] diff --git a/tools/approval.py b/tools/approval.py index f0771b8727..48edfa2f3a 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -2523,7 +2523,10 @@ def prompt_dangerous_approval(command: str, description: str, smart_denied=False) -> str. Legacy callback signatures remain supported when ``smart_denied`` is false. - Returns: 'once', 'session', 'always', or 'deny' + Returns: 'once', 'session', 'always', 'deny', or 'timeout'. + 'timeout' means the prompt expired without a user response — the + action must still be blocked (fail-closed), but callers should + report it as "no response" rather than an explicit user denial. """ if timeout_seconds is None: timeout_seconds = _get_approval_timeout() @@ -2612,7 +2615,10 @@ def prompt_dangerous_approval(command: str, description: str, if thread.is_alive(): print("\n" + t("approval.timeout")) - return "deny" + # Distinct from an explicit deny: the user never answered. + # Callers still block (fail-closed) but tell the agent the + # prompt timed out instead of claiming the user refused. + return "timeout" choice = result["choice"] if smart_denied: @@ -3113,6 +3119,21 @@ def _run_approval_gate( choice = prompt_dangerous_approval(display_target, description, approval_callback=approval_callback) + if choice == "timeout": + return { + "approved": False, + "message": ( + f"BLOCKED: Action timed out without user response. The user " + f"has NOT consented to this action. Do NOT retry it, do NOT " + f"rephrase it, and do NOT attempt the same outcome via a " + f"different path. Silence is not consent." + ), + "pattern_key": pattern_key, + "description": description, + "outcome": "timeout", + "user_consent": False, + } + if choice == "deny": return { "approved": False, @@ -3123,6 +3144,8 @@ def _run_approval_gate( ), "pattern_key": pattern_key, "description": description, + "outcome": "denied", + "user_consent": False, } if choice == "session": @@ -3887,6 +3910,25 @@ def check_all_command_guards(command: str, env_type: str, choice=choice, ) + if choice == "timeout": + breaker_addendum = _denial_breaker_addendum(session_key) + return { + "approved": False, + "message": ( + "BLOCKED: Command timed out without user response. The user " + "has NOT consented to this action. Do NOT retry this " + "command, do NOT rephrase it, and do NOT attempt the same " + "outcome via a different command. Stop the current workflow " + "and wait for the user to respond before taking any further " + "destructive or irreversible action. Silence is not " + f"consent.{breaker_addendum}" + ), + "pattern_key": primary_key, + "description": combined_desc, + "outcome": "timeout", + "user_consent": False, + } + if choice == "deny": breaker_addendum = _denial_breaker_addendum(session_key) return { @@ -4244,6 +4286,10 @@ def request_elicitation_consent( if choice in ("once", "session", "always"): return "accept" + if choice == "timeout": + # Prompt expired without a user response — mirror the gateway's + # unresolved outcome ("cancel") rather than an explicit decline. + return "cancel" return "decline" diff --git a/tools/computer_use/tool.py b/tools/computer_use/tool.py index dbe6fc1558..1422ff67e4 100644 --- a/tools/computer_use/tool.py +++ b/tools/computer_use/tool.py @@ -550,6 +550,14 @@ def _request_approval(action: str, args: Dict[str, Any], if verdict == "always_approve": _session_auto_approve[session_id] = True return None + if verdict == "timeout": + return json.dumps({ + "error": ( + "approval prompt timed out — the user did not respond. " + "Silence is not consent; do not retry without the user." + ), + "action": action, + }) return json.dumps({"error": "denied by user", "action": action})