fix(approval): undelivered or unanswered CLI approval prompts are not user denials
When the CLI approval callback raises, when no callback is registered on the thread while prompt_toolkit owns the terminal, or when the input() read is interrupted, prompt_dangerous_approval returned "deny" and the command gate rendered "BLOCKED: User denied this command" — attributing a refusal to a user who was never asked (#22992). #112308 fixed the gateway half of the class (withdrawn prompts -> outcome "cancelled" with a cause); this closes the CLI residual on the same shape. - tools/approval_prompt.py: those three paths return an Unanswered("cancelled") sentinel carrying the cause; MCP elicitation consent maps it to "cancel". - tools/approval.py: the CLI gate renders "BLOCKED: <noun> was not approved: the approval prompt could not be delivered or was not answered (<cause>)" with outcome "cancelled" — still fail-closed, "Silence is not consent". - tools/file_tools_write_guards.py: the protected-instruction write gate reports the undelivered prompt instead of "was denied by the user". - Shared metrics: "cancelled" is a counted approval outcome (contract + v2 schema) instead of falling into "unknown". - Docs: hook `choice="cancelled"` now covers the CLI causes. Fixes #22992
This commit is contained in:
+8
-1
@@ -483,7 +483,7 @@ _USER_SUMMARIES = {
|
||||
"denied": "You denied this {noun} — it did not run.",
|
||||
"timeout": "No answer within {minutes} — the {noun} did not run.",
|
||||
"notify_failed": "The approval request could not be delivered — the {noun} did not run.",
|
||||
"cancelled": "The approval prompt was withdrawn before you answered — the {noun} did not run.",
|
||||
"cancelled": "The approval prompt was withdrawn or never reached you — the {noun} did not run.",
|
||||
"blocked": "This {noun} is not allowed in an unattended session — it did not run.",
|
||||
}
|
||||
|
||||
@@ -894,6 +894,13 @@ def _human_decision(spec: _GateSpec, *, command: str, description: str,
|
||||
approval_context._fire_approval_hook("post_approval_response", **hook_kwargs, choice=choice)
|
||||
if choice == "timeout":
|
||||
return deny(spec.cli_timeout, "timeout")
|
||||
if choice == "cancelled":
|
||||
# The prompt never reached a human (callback raised, no callback under prompt_toolkit, interrupted
|
||||
# read): fail closed, but do not attribute a refusal to the user (#22992).
|
||||
return deny(spec.gateway_refused, "cancelled",
|
||||
reason="was not approved: the approval prompt could not be delivered or was not answered "
|
||||
f"({getattr(choice, 'cause', 'no answer')})",
|
||||
reason_addendum="", timeout_addendum=" Silence is not consent.", deny_reason=None)
|
||||
if choice == "deny":
|
||||
# No _record_denial(): the breaker counts consecutive guardian LLM
|
||||
# DENY verdicts, not deliberate human denials.
|
||||
|
||||
+25
-10
@@ -35,9 +35,11 @@ def prompt_dangerous_approval(command: str, description: str, timeout_seconds: i
|
||||
allow_permanent=True, allow_session=True, smart_denied=False) -> str``; legacy
|
||||
signatures keep working while both keywords hold their defaults.
|
||||
|
||||
Returns 'once', 'session', 'always', 'deny', or 'timeout'. 'timeout' means no
|
||||
user response — still blocked (fail-closed), but callers report "no response"
|
||||
rather than an explicit denial.
|
||||
Returns 'once', 'session', 'always', 'deny', 'timeout', or 'cancelled'. 'timeout'
|
||||
means no user response — still blocked (fail-closed), but callers report "no
|
||||
response" rather than an explicit denial. 'cancelled' is an :class:`Unanswered`
|
||||
sentinel: the prompt never reached a human (callback raised, no callback under
|
||||
prompt_toolkit, interrupted read) and ``.cause`` says why (#22992).
|
||||
|
||||
See #81887.
|
||||
"""
|
||||
@@ -51,6 +53,18 @@ def prompt_dangerous_approval(command: str, description: str, timeout_seconds: i
|
||||
approval_callback, allow_session, smart_denied, title=title)
|
||||
|
||||
|
||||
class Unanswered(str):
|
||||
"""Choice ``"cancelled"`` carrying the reason nobody answered. Compares equal to the gateway's
|
||||
withdrawn-prompt choice so every consumer already handling ``cancelled`` fails closed without
|
||||
attributing a denial to the user."""
|
||||
__slots__ = ("cause",)
|
||||
|
||||
def __new__(cls, cause: str):
|
||||
self = super().__new__(cls, "cancelled")
|
||||
self.cause = cause
|
||||
return self
|
||||
|
||||
|
||||
_CLI_CHOICE_ALIASES = {
|
||||
"o": "once", "once": "once",
|
||||
"s": "session", "session": "session",
|
||||
@@ -112,14 +126,14 @@ def _ask_human(command: str, description: str, timeout_seconds: int, allow_perma
|
||||
return approval_callback(display_command, display_description, **callback_kwargs)
|
||||
except Exception as e:
|
||||
logger.error("Approval callback failed: %s", e, exc_info=True)
|
||||
return "deny"
|
||||
return Unanswered(f"the approval callback failed: {type(e).__name__}")
|
||||
|
||||
# Fail-closed guard: when prompt_toolkit owns the terminal and no callback is registered on this thread, the
|
||||
# input() fallback would spawn a daemon thread whose read never sees Enter (keystrokes go to prompt_toolkit) — an
|
||||
# invisible deadlock. Deny loudly instead; threads needing interactive approval must install a callback via
|
||||
# invisible deadlock. Fail closed loudly instead; threads needing interactive approval must install a callback via
|
||||
# tools.terminal_tool.set_approval_callback() first.
|
||||
try:
|
||||
# Deny fast and log loudly instead so the caller can surface a real error to the agent. Any thread
|
||||
# Fail fast and log loudly so the caller can surface a real error to the agent. Any thread
|
||||
# that needs interactive approval must install a callback via
|
||||
# tools.terminal_tool.set_approval_callback() before reaching this point (see delegate_tool.py,
|
||||
# run_agent.py _execute_tool_calls_concurrent / _spawn_background_review for the established
|
||||
@@ -127,9 +141,10 @@ def _ask_human(command: str, description: str, timeout_seconds: int, allow_perma
|
||||
from prompt_toolkit.application.current import get_app_or_none
|
||||
if get_app_or_none() is not None:
|
||||
logger.warning("Dangerous-command approval requested on a thread with no "
|
||||
"approval callback while prompt_toolkit is active; denying "
|
||||
"approval callback while prompt_toolkit is active; failing closed "
|
||||
"to avoid stdin deadlock. command=%r description=%r", command, description)
|
||||
return "deny"
|
||||
return Unanswered("no approval callback is registered on this thread while prompt_toolkit owns "
|
||||
"the terminal, so the prompt could not be shown")
|
||||
except Exception:
|
||||
pass # prompt_toolkit absent or detection failed: legacy input() path is safe
|
||||
|
||||
@@ -159,7 +174,7 @@ def _ask_human(command: str, description: str, timeout_seconds: int, allow_perma
|
||||
return decision
|
||||
except (EOFError, KeyboardInterrupt):
|
||||
print("\n" + t("approval.cancelled"))
|
||||
return "deny"
|
||||
return Unanswered("the prompt was interrupted before an answer was given")
|
||||
finally:
|
||||
os.environ.pop("HERMES_SPINNER_PAUSE", None)
|
||||
print()
|
||||
@@ -263,7 +278,7 @@ def _consent(choice, unresolved: str) -> str:
|
||||
"""Map an approval choice to an elicitation verdict; *unresolved* is the no-answer outcome."""
|
||||
if choice in ("once", "session", "always"):
|
||||
return "accept"
|
||||
return unresolved if choice == "timeout" else "decline"
|
||||
return unresolved if choice in ("timeout", "cancelled") else "decline"
|
||||
|
||||
|
||||
def request_elicitation_consent(message: str, description: str, *,
|
||||
|
||||
@@ -311,6 +311,9 @@ def _request_protected_instruction_approval(reasons: list[str], task_id: str = "
|
||||
return blocked.format(why=_NO_HUMAN)
|
||||
choice = prompt_dangerous_approval(
|
||||
display, description, allow_permanent=False, allow_session=False, approval_callback=callback)
|
||||
if choice == "cancelled":
|
||||
return blocked.format(why="approval prompt could not be delivered or was not answered "
|
||||
f"({getattr(choice, 'cause', 'no answer')}).")
|
||||
timed = choice == "timeout"
|
||||
# Any tapped scope is a one-operation grant; nothing is persisted.
|
||||
if not timed and choice in {"once", "session", "always"}:
|
||||
|
||||
Reference in New Issue
Block a user