hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

fix(approval): machine-readable outcome parity on the gateway tails + sudo human-wait exclusion (#85125 2e)

Browse Source
This commit is contained in:
kshitijk4poor
2026-08-25 13:27:20 +05:30
parent 760d3af0d7
commit c8c3f4c448
3 changed files with 114 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

77
tests/tools/test_approval_outcome_parity.py Normal file
View File

@@ -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"])

37
tools/approval.py
View File

@@ -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"]

14
tools/terminal_tool.py
View File

@@ -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 ""