Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 78 additions & 0 deletions tests/tools/test_approval_cron_block.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
"""Tests for approval cron-context blocking (#1542, #1554).

Verifies that _is_unattended_context() and _cron_blocked_result() correctly
convert pending_approval into non-retryable blocked results in cron/subagent
contexts, preventing the retry spirals that cause 44% of terminal failures.

#1554: _is_unattended_context() now requires a *positive* signal
(HERMES_CRON_SESSION or HERMES_SUBAGENT) instead of defaulting to True
when nobody is "home" — the catch-all broke CI test environments.
"""

from __future__ import annotations

from unittest.mock import patch

import pytest

from tools.approval import _cron_blocked_result, _is_unattended_context


class TestIsUnattendedContext:
"""_is_unattended_context() detects cron/subagent via positive signal."""

def test_cron_session_is_unattended(self):
"""HERMES_CRON_SESSION=1 → always unattended."""
with patch(
"tools.approval.env_var_enabled",
side_effect=lambda v: v == "HERMES_CRON_SESSION",
):
assert _is_unattended_context() is True

def test_subagent_is_unattended(self):
"""HERMES_SUBAGENT=1 → unattended (#1554 positive signal)."""
with patch(
"tools.approval.env_var_enabled",
side_effect=lambda v: v == "HERMES_SUBAGENT",
):
assert _is_unattended_context() is True

def test_interactive_cli_not_unattended(self):
"""Interactive CLI with no cron → not unattended."""
with patch("tools.approval.env_var_enabled", return_value=False):
assert _is_unattended_context() is False

def test_gateway_not_unattended(self):
"""Gateway session → not unattended."""
with patch("tools.approval.env_var_enabled", return_value=False):
assert _is_unattended_context() is False

def test_no_context_not_unattended(self):
"""No positive signal → NOT unattended (#1554 fix).

Previously defaulted to True (catch-all "nobody's home"), which broke
CI test environments. Now requires HERMES_CRON_SESSION or
HERMES_SUBAGENT.
"""
with patch("tools.approval.env_var_enabled", return_value=False):
assert _is_unattended_context() is False


class TestCronBlockedResult:
"""_cron_blocked_result() builds a non-retryable blocked response."""

def test_returns_blocked_status(self):
result = _cron_blocked_result("test desc", "rm -rf /")
assert result["status"] == "blocked"
assert result["approved"] is False
assert result["approval_pending"] is False

def test_message_says_do_not_retry(self):
result = _cron_blocked_result("desc", "cmd")
assert "Do NOT retry" in result["message"]

def test_preserves_command_and_desc(self):
result = _cron_blocked_result("my desc", "my cmd", pattern_key="pk")
assert result["command"] == "my cmd"
assert result["description"] == "my desc"
assert result["pattern_key"] == "pk"
47 changes: 47 additions & 0 deletions tools/approval.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,45 @@ def _is_gateway_approval_context() -> bool:
return True
return bool(_get_session_platform())


def _is_unattended_context() -> bool:
"""True when no human or gateway is available to approve commands (#1542).

In cron/subagent contexts, returning ``pending_approval`` causes the agent
to retry the blocked command 3-4x (44% of terminal failures). Callers
should return a non-retryable ``blocked`` status instead.

Requires a **positive signal** (``HERMES_CRON_SESSION`` or
``HERMES_SUBAGENT``) rather than defaulting to True when nobody is
"home" — the catch-all default was too aggressive and broke CI test
environments that are non-interactive, non-gateway, and non-cron
(#1554).
"""
if env_var_enabled("HERMES_CRON_SESSION"):
return True
if env_var_enabled("HERMES_SUBAGENT"):
return True
return False


def _cron_blocked_result(desc, command, pattern_key=""):
"""Build a non-retryable ``blocked`` result for cron/subagent (#1542)."""
return {
"approved": False,
"status": "blocked",
"approval_pending": False,
"command": command,
"description": desc,
"pattern_key": pattern_key,
"message": (
f"BLOCKED: {desc} — no user or gateway present to approve in this "
f"context. Do NOT retry; find an alternative approach using "
f"file/search tools instead of terminal, or set "
f"approvals.cron_mode: approve in config.yaml."
),
}


# Sensitive write targets that should trigger approval even when referenced
# via shell expansions like $HOME or $HERMES_HOME, or by the resolved absolute
# active profile home path such as /home/hermes/.hermes/config.yaml. The
Expand Down Expand Up @@ -3536,6 +3575,10 @@ def check_all_command_guards(command: str, env_type: str,
# Return approval_required for backward compat. Redact secrets in the
# user-facing copy — the raw `command` is preserved for execution and
# the allowlist keys off pattern_key, so redaction is display-only.
#
# #1542: In unattended contexts, return non-retryable blocked.
if _is_unattended_context():
return _cron_blocked_result(combined_desc, command, pattern_key=primary_key)
from agent.redact import redact_sensitive_text
_disp_command = redact_sensitive_text(command)
_disp_combined_desc = redact_sensitive_text(combined_desc)
Expand Down Expand Up @@ -3762,6 +3805,10 @@ def check_execute_code_guard(code: str, env_type: str,
if notify_cb is None:
# No gateway callback registered (e.g. ask-mode without a notifier):
# surface a pending approval for backward compatibility.
#
# #1542: In unattended contexts, return non-retryable blocked.
if _is_unattended_context():
return _cron_blocked_result(display_description, display_command, pattern_key=pattern_key)
pending_data = {
"command": display_command,
"pattern_key": pattern_key,
Expand Down
Loading