From 0d8c521a3cfa4957438b0f341078a0095d080860 Mon Sep 17 00:00:00 2001 From: yilsem Date: Wed, 7 Oct 2026 17:16:43 +0300 Subject: [PATCH] fix(approval): fail closed at once when no human can see the approval prompt A one-shot run (hermes chat --query-file ... -Q, stdin closed, no window) still installs the prompt_toolkit approval callback but never starts the prompt UI. The protected instruction-file gate (AGENTS.md / CLAUDE.md / SOUL.md ...) took the registered callback for a reachable human and waited out approvals.timeout on a panel painted nowhere. With the profile value of 300000 s the stage job hung for good. - Gate: in a one-shot run, or when only an automatic approver (subagent_auto_approve) is attached, return BLOCKED / APPROVAL REQUIRED immediately instead of prompting. Human consent is still required every time; yolo and approvals.single_query_mode still do not open this gate. - One-shot exit prints an "approval_required: {json}" line on stderr (session_id, tool_call_id, request_id, targets, reason, applied=false) so the launcher can tell "blocked, needs a human" from "still working". - execute_code: an approval that arrives after the calling script stopped waiting (300 s reply timeout, script run timeout, retired kernel cell) is not applied. - Same invisible wait removed from the computer_use and memory-write prompts in a one-shot run. The one-shot marker is no longer inherited by child processes. - BLOCKED text no longer breaks on a target path containing braces. Long-running tools are untouched: no general tool timeout was shortened. Kart: turkservis-20261007-1421-2 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EwSpve9HuXMy7dHf4RDMzX --- cli.py | 29 + tests/conftest.py | 1 + .../test_protected_instruction_unattended.py | 578 ++++++++++++++++++ tools/approval.py | 109 ++++ tools/code_execution_tool.py | 39 ++ tools/code_kernel.py | 16 + tools/computer_use/tool.py | 16 + tools/delegate_tool.py | 5 + tools/environments/local.py | 4 + tools/file_tools.py | 93 ++- tools/write_approval.py | 8 + 11 files changed, 884 insertions(+), 14 deletions(-) create mode 100644 tests/tools/test_protected_instruction_unattended.py diff --git a/cli.py b/cli.py index 19536ff..a3fd9d0 100644 --- a/cli.py +++ b/cli.py @@ -1470,6 +1470,34 @@ def _wait_for_oneshot_background_completions(cli) -> None: ) +def _emit_approval_required(session_id) -> None: + """Report approvals a one-shot run could not put in front of a human. + + One ``approval_required: `` line per blocked request on stderr, + next to ``session_id:``, so the process that launched this run can tell + "blocked, needs a human" from "still working" without parsing prose. + Nothing was applied for these requests; resuming the same session in an + interactive surface is the way to get them approved. + """ + try: + from tools.approval import drain_human_approval_required + records = drain_human_approval_required() + except Exception: + return + for record in records: + try: + if not record.get("session_id"): + record["session_id"] = session_id or "" + record["resume"] = "interactive" + print( + "approval_required: " + + json.dumps(record, ensure_ascii=True, sort_keys=True), + file=sys.stderr, + ) + except Exception: + continue + + def _finalize_single_query(cli) -> None: """Close one-shot CLI resources before releasing the active session lease.""" try: @@ -22360,6 +22388,7 @@ def main( # Session ID goes to stderr so piped stdout is clean. print(f"\nsession_id: {cli.session_id}", file=sys.stderr) + _emit_approval_required(cli.session_id) # Ensure proper exit code for automation wrappers. # diff --git a/tests/conftest.py b/tests/conftest.py index e4dfb0e..40454df 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -272,6 +272,7 @@ _HERMES_BEHAVIORAL_VARS = frozenset({ "HERMES_REAL_HOME", "TERMINAL_HOME_MODE", "HERMES_INTERACTIVE", + "HERMES_SINGLE_QUERY_SESSION", "HERMES_QUIET", "HERMES_TOOL_PROGRESS", "HERMES_TOOL_PROGRESS_MODE", diff --git a/tests/tools/test_protected_instruction_unattended.py b/tests/tools/test_protected_instruction_unattended.py new file mode 100644 index 0000000..766256f --- /dev/null +++ b/tests/tools/test_protected_instruction_unattended.py @@ -0,0 +1,578 @@ +"""Protected instruction-file gate when no human can be reached. + +A one-shot run (``chat -q`` / ``--query-file``) installs the CLI approval +callback but never starts the prompt UI. The gate used to treat the +registered callback as a reachable human and wait out ``approvals.timeout`` +on a panel nobody could see. These tests pin the fixed contract: + +* no reachable human -> BLOCKED at once, nothing written, the outcome is + recorded for the launching process; +* the gate itself is unchanged: still asks a real human every time, still + ignores yolo and ``approvals.single_query_mode``; +* an approval that arrives after the calling execute_code script stopped + waiting is not applied. +""" + +import json +import time + +import pytest + + +@pytest.fixture(autouse=True) +def _gate_on(monkeypatch): + import tools.approval as A + import tools.file_tools as ft + + monkeypatch.setattr(ft, "_protected_instruction_config", lambda: (True, [])) + monkeypatch.delenv("HERMES_SINGLE_QUERY_SESSION", raising=False) + A.drain_human_approval_required() + A.set_approval_reply_deadline(None) + yield + A.drain_human_approval_required() + A.set_approval_reply_deadline(None) + + +@pytest.fixture +def approvals(): + """CLI approval callback that records calls and gives a scripted answer.""" + from tools.terminal_tool import set_approval_callback + + state = {"calls": [], "answer": "deny"} + + def cb(command, description, **kwargs): + state["calls"].append({"command": command, **kwargs}) + answer = state["answer"] + if callable(answer): + return answer() + return answer + + set_approval_callback(cb) + yield state + set_approval_callback(None) + + +@pytest.fixture +def one_shot(monkeypatch): + monkeypatch.setenv("HERMES_SINGLE_QUERY_SESSION", "1") + + +def _write(path, content="injected"): + from tools.file_tools import write_file_tool + + return json.loads(write_file_tool(str(path), content)) + + +def _records(): + import tools.approval as A + + return A.drain_human_approval_required() + + +# ---- one-shot run: callback registered, nobody to answer ------------------ + + +class TestOneShotRun: + def test_blocks_at_once_without_prompting(self, tmp_path, approvals, one_shot): + target = tmp_path / "AGENTS.md" + target.write_text("rules\n", encoding="utf-8") + approvals["answer"] = "once" # would be honored if anyone were asked + + started = time.monotonic() + res = _write(target) + elapsed = time.monotonic() - started + + assert res.get("error") and "BLOCKED" in res["error"] + assert "APPROVAL REQUIRED" in res["error"] + assert target.read_text(encoding="utf-8") == "rules\n" + assert approvals["calls"] == [], "prompted on a surface nobody can see" + assert elapsed < 5 + + def test_outcome_is_recorded_for_the_launcher(self, tmp_path, approvals, one_shot): + import tools.approval as A + + target = tmp_path / "CLAUDE.md" + sid = A._approval_session_id.set("sess-123") + tcid = A._approval_tool_call_id.set("call-9") + try: + _write(target) + finally: + A._approval_tool_call_id.reset(tcid) + A._approval_session_id.reset(sid) + + records = _records() + assert len(records) == 1 + rec = records[0] + assert rec["kind"] == "protected_instruction_file" + assert rec["reason"] == "no_interactive_user" + assert rec["applied"] is False + assert rec["session_id"] == "sess-123" + assert rec["tool_call_id"] == "call-9" + assert rec["request_id"] + assert any("CLAUDE.md" in t for t in rec["targets"]) + assert _records() == [], "drain must clear" + + def test_yolo_does_not_open_the_gate(self, tmp_path, approvals, one_shot, monkeypatch): + import tools.approval as A + + monkeypatch.setattr(A, "_YOLO_MODE_FROZEN", True) + target = tmp_path / "AGENTS.md" + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + + def test_single_query_mode_approve_does_not_open_the_gate( + self, tmp_path, approvals, one_shot, monkeypatch + ): + import tools.approval as A + + real_cfg_get = A.cfg_get + + def fake_cfg_get(config, *keys, default=None): + if keys == ("approvals", "single_query_mode"): + return "approve" + return real_cfg_get(config, *keys, default=default) + + monkeypatch.setattr(A, "cfg_get", fake_cfg_get) + target = tmp_path / "SOUL.md" + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + + def test_regular_file_still_writes(self, tmp_path, approvals, one_shot): + target = tmp_path / "notes.md" + res = _write(target, "hello") + assert not res.get("error"), res + assert target.read_text(encoding="utf-8") == "hello" + assert _records() == [] + + def test_multi_file_patch_applies_nothing(self, tmp_path, approvals, one_shot): + from tools.file_tools import patch_tool + + agents = tmp_path / "AGENTS.md" + agents.write_text("rules\n", encoding="utf-8") + plain = tmp_path / "plain.txt" + plain.write_text("hello\n", encoding="utf-8") + patch = ( + "*** Begin Patch\n" + f"*** Update File: {plain}\n" + "@@\n" + "-hello\n" + "+world\n" + f"*** Update File: {agents}\n" + "@@\n" + "-rules\n" + "+injected\n" + "*** End Patch" + ) + res = json.loads(patch_tool(mode="patch", patch=patch)) + assert res.get("error") and "BLOCKED" in res["error"] + assert plain.read_text(encoding="utf-8") == "hello\n" + assert agents.read_text(encoding="utf-8") == "rules\n" + assert approvals["calls"] == [] + + def test_retry_in_same_run_blocks_again(self, tmp_path, approvals, one_shot): + target = tmp_path / "AGENTS.md" + _write(target) + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + assert len(_records()) == 2 + + +# ---- no channel at all ---------------------------------------------------- + + +def test_no_channel_fails_closed_and_is_recorded(tmp_path): + target = tmp_path / "AGENTS.md" + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + records = _records() + assert [r["reason"] for r in records] == ["no_approval_channel"] + + +def test_undeliverable_gateway_request_is_recorded(tmp_path): + import tools.approval as A + + session_key = "protected-unattended-undelivered" + token = A.set_current_session_key(session_key) + try: + def notify(approval_data): + raise RuntimeError("client went away") + + A.register_gateway_notify(session_key, notify) + try: + target = tmp_path / "AGENTS.md" + res = _write(target) + finally: + A.unregister_gateway_notify(session_key) + finally: + A.reset_current_session_key(token) + + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + assert [r["reason"] for r in _records()] == ["approval_not_delivered"] + + +# ---- a reachable human: the gate behaves exactly as before ----------------- + + +class TestReachableHuman: + def test_allow_writes(self, tmp_path, approvals): + target = tmp_path / "AGENTS.md" + approvals["answer"] = "once" + res = _write(target, "approved") + assert not res.get("error"), res + assert target.read_text(encoding="utf-8") == "approved" + assert len(approvals["calls"]) == 1 + assert _records() == [] + + @pytest.mark.parametrize("answer", ["deny", "timeout"]) + def test_deny_and_silence_block(self, tmp_path, approvals, answer): + target = tmp_path / "AGENTS.md" + approvals["answer"] = answer + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + assert len(approvals["calls"]) == 1 + # A human was asked and did not consent: not an "unattended" outcome. + assert _records() == [] + + def test_prompt_that_dies_blocks(self, tmp_path, approvals): + """Cancel / lost surface: the prompt raising is not consent.""" + def boom(): + raise RuntimeError("prompt surface lost") + + target = tmp_path / "AGENTS.md" + approvals["answer"] = boom + res = _write(target) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + + +# ---- approval that arrives after the caller stopped waiting ---------------- + + +class TestLateApproval: + def test_late_cli_approval_is_not_applied(self, tmp_path, approvals): + import tools.approval as A + + target = tmp_path / "AGENTS.md" + target.write_text("rules\n", encoding="utf-8") + + def answer_after_deadline(): + # The human says yes, but only once the execute_code script that + # asked for this write has already timed out. + A.set_approval_reply_deadline(time.monotonic() - 1) + return "once" + + A.set_approval_reply_deadline(time.monotonic() + 300) + approvals["answer"] = answer_after_deadline + res = _write(target) + + assert res.get("error") and "BLOCKED" in res["error"] + assert "NOT applied" in res["error"] + assert target.read_text(encoding="utf-8") == "rules\n" + + def test_late_approval_does_not_carry_to_the_next_request(self, tmp_path, approvals): + import tools.approval as A + + target = tmp_path / "AGENTS.md" + A.set_approval_reply_deadline(time.monotonic() - 1) + approvals["answer"] = "once" + assert "BLOCKED" in _write(target).get("error", "") + assert not target.exists() + + # A fresh call (new deadline) asks again; the old yes is not reused. + A.set_approval_reply_deadline(time.monotonic() + 300) + approvals["answer"] = "deny" + assert "BLOCKED" in _write(target).get("error", "") + assert not target.exists() + assert len(approvals["calls"]) == 2 + + def test_in_time_approval_is_applied(self, tmp_path, approvals): + import tools.approval as A + + target = tmp_path / "AGENTS.md" + A.set_approval_reply_deadline(time.monotonic() + 300) + approvals["answer"] = "once" + res = _write(target, "approved") + assert not res.get("error"), res + assert target.read_text(encoding="utf-8") == "approved" + + def test_late_gateway_approval_is_not_applied(self, tmp_path): + import tools.approval as A + + session_key = "protected-unattended-late" + token = A.set_current_session_key(session_key) + try: + def notify(approval_data): + A.resolve_gateway_approval(session_key, "once") + + A.register_gateway_notify(session_key, notify) + try: + A.set_approval_reply_deadline(time.monotonic() - 1) + target = tmp_path / "AGENTS.md" + res = _write(target) + finally: + A.unregister_gateway_notify(session_key) + finally: + A.reset_current_session_key(token) + + assert res.get("error") and "NOT applied" in res["error"] + assert not target.exists() + + def test_no_deadline_outside_execute_code(self): + import tools.approval as A + + assert A.approval_reply_window_remaining() is None + assert A.approval_arrived_too_late() is False + + def test_rpc_dispatch_binds_and_restores_the_deadline(self): + import tools.approval as A + import tools.code_execution_tool as cet + + start = time.monotonic() + previous = cet._bind_rpc_reply_deadline(start) + try: + remaining = A.approval_reply_window_remaining() + assert remaining is not None + assert 0 < remaining <= cet._RPC_REPLY_TIMEOUT_SECONDS + finally: + cet._restore_rpc_reply_deadline(previous) + assert A.approval_reply_window_remaining() is None + + def test_deadline_is_per_thread(self): + import threading + + import tools.approval as A + + A.set_approval_reply_deadline(time.monotonic() - 1) + seen = {} + + def other(): + seen["late"] = A.approval_arrived_too_late() + + worker = threading.Thread(target=other) + worker.start() + worker.join() + assert seen["late"] is False + assert A.approval_arrived_too_late() is True + + +# ---- what the launching process sees --------------------------------------- + + +def test_recorded_outcomes_are_bounded(): + import tools.approval as A + + for i in range(A._HUMAN_APPROVAL_REQUIRED_MAX + 20): + A.note_human_approval_required("protected_instruction_file", [f"f{i}"], "x") + records = A.drain_human_approval_required() + assert len(records) == A._HUMAN_APPROVAL_REQUIRED_MAX + assert records[-1]["targets"] == [f"f{A._HUMAN_APPROVAL_REQUIRED_MAX + 19}"] + + +def test_one_shot_exit_reports_approval_required_on_stderr(capsys): + import cli as cli_module + import tools.approval as A + + A.note_human_approval_required( + "protected_instruction_file", ["AGENTS.md"], "no_interactive_user" + ) + cli_module._emit_approval_required("sess-42") + + err = capsys.readouterr().err + lines = [l for l in err.splitlines() if l.startswith("approval_required: ")] + assert len(lines) == 1 + payload = json.loads(lines[0].split(": ", 1)[1]) + assert payload["session_id"] == "sess-42" + assert payload["applied"] is False + assert payload["reason"] == "no_interactive_user" + assert payload["resume"] == "interactive" + assert payload["targets"] == ["AGENTS.md"] + + cli_module._emit_approval_required("sess-42") + assert "approval_required" not in capsys.readouterr().err + + +# ---- execute_code RPC: the dispatcher really binds the window --------------- + + +def _drive_rpc_once(dispatch, stop_event=None): + """Send one tool call through the real RPC server loop; return its reply.""" + import socket + import threading + + import tools.code_execution_tool as cet + + server = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + server.bind(("127.0.0.1", 0)) + server.listen(1) + stop_event = stop_event or threading.Event() + worker = threading.Thread( + target=cet._rpc_server_loop, + args=(server, "task", [], [0], 5, frozenset({"patch"}), stop_event, "tok", dispatch), + daemon=True, + ) + worker.start() + client = socket.create_connection(server.getsockname(), timeout=10) + try: + client.sendall( + (json.dumps({"tool": "patch", "args": {}, "token": "tok"}) + "\n").encode() + ) + buf = b"" + while not buf.endswith(b"\n"): + chunk = client.recv(65536) + if not chunk: + break + buf += chunk + finally: + client.close() + stop_event.set() + worker.join(timeout=10) + server.close() + return buf.decode().strip() + + +def test_rpc_server_loop_binds_reply_window_around_dispatch(): + import tools.approval as A + import tools.code_execution_tool as cet + + seen = {} + + def dispatch(tool_name, tool_args): + seen["remaining"] = A.approval_reply_window_remaining() + seen["late"] = A.approval_arrived_too_late() + return json.dumps({"ok": True}) + + reply = _drive_rpc_once(dispatch) + assert json.loads(reply) == {"ok": True} + assert seen["late"] is False + assert 0 < seen["remaining"] <= cet._RPC_REPLY_TIMEOUT_SECONDS + + +def test_rpc_approval_after_script_ended_is_late(): + """Script killed by its own run timeout while the prompt is still open.""" + import threading + + import tools.approval as A + + stop_event = threading.Event() + seen = {} + + def dispatch(tool_name, tool_args): + seen["before"] = A.approval_arrived_too_late() + stop_event.set() # the script ended while a human was deciding + seen["after"] = A.approval_arrived_too_late() + return json.dumps({"ok": True}) + + _drive_rpc_once(dispatch, stop_event) + assert seen == {"before": False, "after": True} + + +def test_protected_write_through_rpc_is_dropped_when_script_ended(tmp_path, approvals): + """End to end through the RPC loop: late yes -> BLOCKED, file untouched.""" + import threading + + from tools.file_tools import write_file_tool + from tools.terminal_tool import _get_approval_callback, set_approval_callback + + target = tmp_path / "AGENTS.md" + target.write_text("rules\n", encoding="utf-8") + stop_event = threading.Event() + callback = _get_approval_callback() + + def answer_after_script_died(): + stop_event.set() + return "once" + + approvals["answer"] = answer_after_script_died + + def dispatch(tool_name, tool_args): + set_approval_callback(callback) # the RPC thread carries the cell's callback + try: + return write_file_tool(str(target), "injected") + finally: + set_approval_callback(None) + + reply = json.loads(_drive_rpc_once(dispatch, stop_event)) + assert "NOT applied" in reply.get("error", "") + assert target.read_text(encoding="utf-8") == "rules\n" + assert len(approvals["calls"]) == 1 + + +def test_retired_kernel_cell_counts_as_gone(): + import tools.approval as A + + cell = {"active": True} + previous = A.also_gone_when(lambda: not cell["active"]) + try: + assert A.approval_arrived_too_late() is False + cell["active"] = False + assert A.approval_arrived_too_late() is True + finally: + A.restore_approval_reply_deadline(previous) + assert A.approval_arrived_too_late() is False + + +# ---- an automatic approver is not a person ---------------------------------- + + +def test_subagent_auto_approve_cannot_open_the_gate(tmp_path): + from tools.delegate_tool import _subagent_auto_approve + from tools.terminal_tool import set_approval_callback + + target = tmp_path / "AGENTS.md" + set_approval_callback(_subagent_auto_approve) + try: + res = _write(target) + finally: + set_approval_callback(None) + assert res.get("error") and "BLOCKED" in res["error"] + assert not target.exists() + assert [r["reason"] for r in _records()] == ["no_interactive_user"] + + +def test_blocked_message_survives_braces_in_target(tmp_path): + import tools.file_tools as ft + + res = ft._request_protected_instruction_approval(["{be}/AGENTS.md"]) + assert res.startswith("BLOCKED") and "{be}/AGENTS.md" in res + + +# ---- sibling gates in a one-shot run ---------------------------------------- + + +def test_computer_use_does_not_wait_in_one_shot_run(one_shot, monkeypatch): + import tools.computer_use.tool as cu + + calls = [] + monkeypatch.setattr(cu, "_approval_callback", lambda *a, **k: calls.append(a) or "approve_once") + res = cu._request_approval("click", {}, session_id="unattended-test") + assert res is not None and "one-shot run" in json.loads(res)["error"] + assert calls == [] + + +def test_computer_use_still_asks_when_interactive(monkeypatch): + import tools.computer_use.tool as cu + + calls = [] + monkeypatch.setattr(cu, "_approval_callback", lambda *a, **k: calls.append(a) or "approve_once") + assert cu._request_approval("click", {}, session_id="attended-test") is None + assert len(calls) == 1 + + +def test_memory_write_prompt_unavailable_in_one_shot_run(approvals, monkeypatch): + import tools.write_approval as wa + + assert wa._interactive_approval_available() is True + monkeypatch.setenv("HERMES_SINGLE_QUERY_SESSION", "1") + assert wa._interactive_approval_available() is False + + +def test_one_shot_marker_is_not_inherited_by_child_processes(one_shot): + from tools.environments.local import _make_run_env + + assert "HERMES_SINGLE_QUERY_SESSION" not in _make_run_env({}) diff --git a/tools/approval.py b/tools/approval.py index 15867a1..24775f7 100644 --- a/tools/approval.py +++ b/tools/approval.py @@ -78,6 +78,115 @@ _hermes_interactive_ctx: contextvars.ContextVar[Optional[str]] = contextvars.Con ) +# Absolute ``time.monotonic()`` instant after which whoever issued the +# current tool call has stopped waiting for its result. Set by the +# execute_code RPC dispatchers, whose sandbox client gives up after a fixed +# reply timeout. An approval that resolves after this instant must NOT be +# acted on: the caller already reported the call as failed, so applying it +# would be a write the agent believes never happened (and may redo). +# The reply timeout is not the only way a caller goes away: the script can +# hit its own run timeout or be interrupted first. ``gone`` is a probe the +# dispatcher supplies for that ("has my script/cell already ended?"). +# Thread-local on purpose: the dispatcher and the tool handler it calls run +# on the same thread, whatever context the handler is re-bound to. +_approval_reply_deadline = threading.local() + + +def set_approval_reply_deadline(deadline: Optional[float], gone=None): + """Bind the caller's reply window for this thread; return the prior binding. + + ``deadline`` is monotonic seconds (None = unbounded); ``gone`` is an + optional zero-argument callable that turns true once the caller has + ended. Pass the returned value to :func:`restore_approval_reply_deadline`. + """ + previous = ( + getattr(_approval_reply_deadline, "value", None), + getattr(_approval_reply_deadline, "gone", None), + ) + _approval_reply_deadline.value = deadline + _approval_reply_deadline.gone = gone + return previous + + +def restore_approval_reply_deadline(previous) -> None: + """Undo :func:`set_approval_reply_deadline` / :func:`also_gone_when`.""" + deadline, gone = previous if previous else (None, None) + _approval_reply_deadline.value = deadline + _approval_reply_deadline.gone = gone + + +def also_gone_when(probe): + """Add another "caller has ended" probe, keeping the current window.""" + deadline = getattr(_approval_reply_deadline, "value", None) + current = getattr(_approval_reply_deadline, "gone", None) + + def gone() -> bool: + return bool(probe()) or bool(current and current()) + + return set_approval_reply_deadline(deadline, gone) + + +def approval_reply_window_remaining() -> Optional[float]: + """Seconds left before the caller stops waiting, or None when unbounded.""" + deadline = getattr(_approval_reply_deadline, "value", None) + if deadline is None: + return None + return deadline - time.monotonic() + + +def approval_arrived_too_late() -> bool: + """True when the caller of the current tool call has stopped waiting.""" + gone = getattr(_approval_reply_deadline, "gone", None) + if gone is not None: + try: + if gone(): + return True + except Exception: + pass + remaining = approval_reply_window_remaining() + return remaining is not None and remaining <= 0 + + +# Approvals that could not be put in front of a human because the run has +# no interactive channel (single-query ``chat -Q`` stage runs, scripts). +# The single-query CLI reports them on stderr as ``approval_required:`` +# lines so the coordinating process gets a machine-readable outcome +# instead of inferring one from prose. Bounded: long-lived processes that +# never drain it must not grow without limit. +_human_approval_required: list[dict] = [] +_HUMAN_APPROVAL_REQUIRED_MAX = 50 +_human_approval_required_lock = threading.Lock() + + +def note_human_approval_required(kind: str, targets: list[str], reason: str) -> dict: + """Record an approval the run could not present to a human. + + Captures the active session/tool-call identity so the coordinator can + tie the outcome to the exact job and resume the same session. + """ + record = { + "kind": kind, + "targets": list(dict.fromkeys(str(t) for t in targets)), + "reason": reason, + "request_id": uuid.uuid4().hex, + "tool_call_id": _approval_tool_call_id.get() or "", + "session_id": _approval_session_id.get() or os.getenv("HERMES_SESSION_ID", ""), + "applied": False, + } + with _human_approval_required_lock: + _human_approval_required.append(record) + del _human_approval_required[:-_HUMAN_APPROVAL_REQUIRED_MAX] + return dict(record) + + +def drain_human_approval_required() -> list[dict]: + """Return and clear the recorded approval-required outcomes.""" + with _human_approval_required_lock: + records = list(_human_approval_required) + _human_approval_required.clear() + return records + + def set_hermes_interactive_context(interactive: bool) -> contextvars.Token: """Bind interactive mode for the current context (thread or asyncio task). diff --git a/tools/code_execution_tool.py b/tools/code_execution_tool.py index f268ef8..b5adbd8 100644 --- a/tools/code_execution_tool.py +++ b/tools/code_execution_tool.py @@ -716,6 +716,36 @@ def _call(tool_name, args): # RPC server (runs in a thread inside the parent process) # --------------------------------------------------------------------------- +# How long the generated sandbox client waits for one tool-call reply +# (``_sock.settimeout(300)`` / the file transport's 300s poll deadline). +_RPC_REPLY_TIMEOUT_SECONDS = 300 + + +def _bind_rpc_reply_deadline(call_start: float, stop_event=None): + """Tell the approval gates when this call's requester stops waiting. + + The requester is gone at the reply timeout, or earlier when the script + itself ended (run timeout, interrupt): ``stop_event`` is set then. + """ + try: + from tools.approval import set_approval_reply_deadline + gone = stop_event.is_set if stop_event is not None else None + return (True, set_approval_reply_deadline( + call_start + _RPC_REPLY_TIMEOUT_SECONDS, gone)) + except Exception: + return (False, None) + + +def _restore_rpc_reply_deadline(previous) -> None: + if not previous or not previous[0]: + return + try: + from tools.approval import restore_approval_reply_deadline + restore_approval_reply_deadline(previous[1]) + except Exception: + pass + + # Terminal parameters that must not be used from ephemeral sandbox scripts _TERMINAL_BLOCKED_PARAMS = {"background", "pty", "notify", "notify_on_complete", "watch_patterns"} @@ -826,12 +856,18 @@ def _rpc_server_loop( # Dispatch through the standard tool handler. # Suppress stdout/stderr from internal tool handlers so # their status prints don't leak into the CLI spinner. + # The sandbox client stops waiting for this reply after + # _RPC_REPLY_TIMEOUT_SECONDS; an approval granted later + # must not apply a write the script already saw fail. + _prev_deadline = _bind_rpc_reply_deadline(call_start, stop_event) try: with thread_scoped_silence(): result = dispatch(tool_name, tool_args) except Exception as exc: logger.error("Tool call failed in sandbox: %s", exc, exc_info=True) result = tool_error(str(exc)) + finally: + _restore_rpc_reply_deadline(_prev_deadline) tool_call_counter[0] += 1 call_duration = time.monotonic() - call_start @@ -1101,6 +1137,7 @@ def _rpc_poll_loop( tool_args.pop(param, None) # Dispatch through the standard tool handler + _prev_deadline = _bind_rpc_reply_deadline(call_start, stop_event) try: with thread_scoped_silence(): tool_result = handle_function_call( @@ -1110,6 +1147,8 @@ def _rpc_poll_loop( logger.error("Tool call failed in remote sandbox: %s", exc, exc_info=True) tool_result = tool_error(str(exc)) + finally: + _restore_rpc_reply_deadline(_prev_deadline) tool_call_counter[0] += 1 call_duration = time.monotonic() - call_start diff --git a/tools/code_kernel.py b/tools/code_kernel.py index 166e32b..6b36bb9 100644 --- a/tools/code_kernel.py +++ b/tools/code_kernel.py @@ -322,9 +322,25 @@ class CellAuthority: set_sudo(self._sudo_cb) except Exception: previous = None + # A cell that settled (timeout, interrupt, error) no longer waits for + # this call: an approval granted afterwards must not be applied. + reply_window = None + try: + from tools.approval import also_gone_when + + reply_window = (also_gone_when(lambda: not self.active),) + except Exception: + reply_window = None try: return handle_function_call(tool_name, tool_args, task_id=self.task_id) finally: + if reply_window is not None: + try: + from tools.approval import restore_approval_reply_deadline + + restore_approval_reply_deadline(reply_window[0]) + except Exception: + pass if previous is not None and self._callback_setters is not None: set_approval, set_sudo = self._callback_setters try: diff --git a/tools/computer_use/tool.py b/tools/computer_use/tool.py index 0b3801b..084d60f 100644 --- a/tools/computer_use/tool.py +++ b/tools/computer_use/tool.py @@ -613,6 +613,22 @@ def _request_approval(action: str, args: Dict[str, Any], # No CLI approval wired — default allow. Gateway approval is handled # one layer out via the normal tool-approval infra. return None + try: + from tools.approval import _is_single_query_approval_context + unattended = _is_single_query_approval_context() + except Exception: + unattended = False + if unattended: + # One-shot run: the prompt callback is installed but its panel is + # never shown, so asking would only wait out approvals.timeout. + return json.dumps({ + "error": ( + "approval required but this is a one-shot run with no " + "interactive user to approve it. Silence is not consent; " + "do not retry without the user." + ), + "action": action, + }) summary = _summarize_action(action, args) try: verdict = cb(action, args, summary) diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 02a999c..c0213aa 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -102,6 +102,11 @@ def _subagent_auto_approve(command: str, description: str, **kwargs) -> str: return "once" +# Not a person: gates that require real human consent every time (protected +# instruction files) must not accept this callback's answer. +_subagent_auto_approve.unattended = True + + def _get_subagent_approval_callback(): """Return the callback to install into subagent worker threads. diff --git a/tools/environments/local.py b/tools/environments/local.py index f876753..6062687 100644 --- a/tools/environments/local.py +++ b/tools/environments/local.py @@ -1546,6 +1546,10 @@ def _make_run_env(env: dict) -> dict: run_env[real_key] = v elif _is_hermes_internal_secret(k): continue + elif k == "HERMES_SINGLE_QUERY_SESSION": + # Describes this process only. A child (an interactive hermes + # started from a one-shot run's terminal) sets its own. + continue else: first_party = _is_terminal_first_party_env(k) passthrough = _is_passthrough(k) diff --git a/tools/file_tools.py b/tools/file_tools.py index a68655d..06fc26a 100644 --- a/tools/file_tools.py +++ b/tools/file_tools.py @@ -861,12 +861,18 @@ def _request_protected_instruction_approval( "required (not bypassed by auto-approve)." ) display = f"" - blocked = ( - f"BLOCKED: write to protected agent-instruction file(s) ({targets}) " - "{why} The user has NOT consented to this write. Do NOT retry it or " - "attempt the same edit via another path (terminal, execute_code, " - "etc.)." - ) + class _Blocked: + # Not str.format: a target path may itself contain braces. + @staticmethod + def format(why: str) -> str: + return ( + "BLOCKED: write to protected agent-instruction file(s) " + f"({targets}) {why} The user has NOT consented to this " + "write. Do NOT retry it or attempt the same edit via " + "another path (terminal, execute_code, etc.)." + ) + + blocked = _Blocked try: import tools.approval as _approval @@ -874,6 +880,41 @@ def _request_protected_instruction_approval( return blocked.format(why="requires approval but the approval " "subsystem is unavailable.") + def _needs_human(reason: str, why: str) -> str: + # No human could be shown this request. Record it so a one-shot + # run can hand its coordinator a machine-readable outcome, and tell + # the agent where a human CAN approve it instead of waiting on a + # prompt nobody sees. + try: + _approval.note_human_approval_required( + "protected_instruction_file", list(reasons), reason) + except Exception: + pass + return blocked.format( + why=why + " APPROVAL REQUIRED: a human must approve this from " + "an interactive session (resume this same session in " + "the desktop app or an interactive terminal and " + "request the write again there).") + + def _too_late() -> str | None: + # The approval came back after whoever issued this call stopped + # waiting (execute_code RPC reply timeout). The caller already saw + # a failure, so applying the write now would be an edit the agent + # believes never happened. + try: + late = _approval.approval_arrived_too_late() + except Exception: + late = False + if not late: + return None + logger.warning( + "Approval for protected instruction file(s) %s arrived after " + "the caller stopped waiting; write not applied.", targets) + return blocked.format( + why="was approved only after the calling script had stopped " + "waiting for the result, so the write was NOT applied. " + "The approval is spent; a new request needs a new approval.") + # Gateway surface: block on the button round-trip when a notify callback # is registered for this session (Telegram/Discord/Slack). One-operation # only — no session/permanent buttons are offered. @@ -898,14 +939,15 @@ def _request_protected_instruction_approval( session_key, notify_cb, approval_data, surface="gateway", ) if decision.get("notify_failed"): - return blocked.format( - why="requires approval but the approval request could not " - "be delivered.") + return _needs_human( + "approval_not_delivered", + "requires approval but the approval request could not " + "be delivered.") choice = decision.get("choice") if decision.get("resolved") and choice in {"once", "session", "always"}: # One-operation grant regardless of the tapped scope — nothing # is persisted for this gate. - return None + return _too_late() if not decision.get("resolved"): return blocked.format( why="approval prompt timed out without a user response. " @@ -921,6 +963,28 @@ def _request_protected_instruction_approval( callback = None if callback is not None: + # A one-shot run (``chat -q`` / ``--query-file``) installs the + # prompt callback but never starts the prompt UI: the panel would + # be painted nowhere and the wait would last the whole + # approvals.timeout with nobody able to answer. A registered + # callback is not a reachable human. Fail closed NOW; + # approvals.single_query_mode does not apply to this gate. + try: + unattended = _approval._is_single_query_approval_context() + except Exception: + unattended = False + if unattended: + return _needs_human( + "no_interactive_user", + "requires approval but this is a one-shot run with no " + "interactive user to approve it.") + if getattr(callback, "unattended", False): + # An auto-answering callback (subagent auto-approve) is not a + # person; this gate takes consent from a human only. + return _needs_human( + "no_interactive_user", + "requires approval but only an automatic approver is " + "attached to this run, not a person.") choice = _approval.prompt_dangerous_approval( display, description, allow_permanent=False, @@ -929,7 +993,7 @@ def _request_protected_instruction_approval( ) if choice in {"once", "session", "always"}: # One-operation grant; never persisted (see docstring). - return None + return _too_late() if choice == "timeout": return blocked.format( why="approval prompt timed out without a user response. " @@ -938,9 +1002,10 @@ def _request_protected_instruction_approval( # No human channel at all (script, cron, background thread): fail # closed. Auto-approving here would recreate the persistence vector. - return blocked.format( - why="requires approval but no interactive user or gateway is " - "present to approve it.") + return _needs_human( + "no_approval_channel", + "requires approval but no interactive user or gateway is " + "present to approve it.") def _check_protected_instruction_write(paths: list[str], diff --git a/tools/write_approval.py b/tools/write_approval.py index b017299..9985fb6 100644 --- a/tools/write_approval.py +++ b/tools/write_approval.py @@ -327,6 +327,14 @@ def _interactive_approval_available() -> bool: review affordance (``/memory pending``) instead. * Scripts, cron, and background threads — no user present. """ + try: + from tools.approval import _is_single_query_approval_context + if _is_single_query_approval_context(): + # One-shot run: the callback exists but its panel is never + # shown, so an inline prompt would wait with nobody to answer. + return False + except Exception: + pass try: from tools.terminal_tool import _get_approval_callback return _get_approval_callback() is not None