CLI approval prompt freezes, deadlocks, and input fixes

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #13697 fix: approval prompt freeze — CLI thread-local + TUI blind-keystroke

  merged to main

  files: cli.py, tests/cli/test_cli_approval_ui.py, ui-tui/src/app/useInputHandlers.ts

  Show the diff

  • cli.py

    @@ -8370,6 +8370,17 @@ def stream_callback(delta: str):
     
                 def run_agent():
                     nonlocal result
    +                # Set callbacks inside the agent thread so thread-local storage
    +                # in terminal_tool is populated for this thread.  The main thread
    +                # registration (run() line ~9046) is invisible here because
    +                # _callback_tls is threading.local().  Matches the pattern used
    +                # by acp_adapter/server.py for ACP sessions.
    +                set_sudo_password_callback(self._sudo_password_callback)
    +                set_approval_callback(self._approval_callback)
    +                try:
    +                    set_secret_capture_callback(self._secret_capture_callback)
    +                except Exception:
    +                    pass
                     agent_message = _voice_prefix + message if _voice_prefix else message
                     # Prepend pending model switch note so the model knows about the switch
                     _msn = getattr(self, '_pending_model_switch_note', None)
    @@ -8395,6 +8406,15 @@ def run_agent():
                             "failed": True,
                             "error": _summary,
                         }
    +                finally:
    +                    # Clear thread-local callbacks so a reused thread doesn't
    +                    # hold stale references to a disposed CLI instance.
    +                    try:
    +                        set_sudo_password_callback(None)
    +                        set_approval_callback(None)
    +                        set_secret_capture_callback(None)
    +                    except Exception:
    +                        pass
     
                 # Start agent in background thread (daemon so it cannot keep the
                 # process alive when the user closes the terminal tab — SIGHUP

  • tests/cli/test_cli_approval_ui.py

    @@ -254,3 +254,88 @@ def test_approval_display_truncates_giant_command_in_view_mode(self):
     
             # Command got truncated with a marker.
             assert "(command truncated" in rendered
    +
    +
    +class TestApprovalCallbackThreadLocalWiring:
    +    """Regression guard for the thread-local callback freeze (#13617 / #13618).
    +
    +    After 62348cff made _approval_callback / _sudo_password_callback thread-local
    +    (ACP GHSA-qg5c-hvr5-hjgr), the CLI agent thread could no longer see callbacks
    +    registered in the main thread — the dangerous-command prompt silently fell
    +    back to stdin input() and deadlocked against prompt_toolkit. The fix is to
    +    register the callbacks INSIDE the agent worker thread (matching the ACP
    +    pattern). These tests lock in that invariant.
    +    """
    +
    +    def test_main_thread_registration_is_invisible_to_child_thread(self):
    +        """Confirms the underlying threading.local semantics that drove the bug.
    +
    +        If this ever starts passing as "visible", the thread-local isolation
    +        is gone and the ACP race GHSA-qg5c-hvr5-hjgr may be back.
    +        """
    +        from tools.terminal_tool import (
    +            set_approval_callback,
    +            _get_approval_callback,
    +        )
    +
    +        def main_cb(_cmd, _desc):
    +            return "once"
    +
    +        set_approval_callback(main_cb)
    +        try:
    +            seen = {}
    +
    +            def _child():
    +                seen["value"] = _get_approval_callback()
    +
    +            t = threading.Thread(target=_child, daemon=True)
    +            t.start()
    +            t.join(timeout=2)
    +            assert seen["value"] is None
    +        finally:
    +            set_approval_callback(None)
    +
    +    def test_child_thread_registration_is_visible_and_cleared_in_finally(self):
    +        """The fix pattern: register INSIDE the worker thread, clear in finally.
    +
    +        This is exactly what cli.py's run_agent() closure does. If this test
    +        fails, the CLI approval prompt freeze (#13617) has regressed.
    +        """
    +        from tools.terminal_tool import (
    +            set_approval_callback,
    +            set_sudo_password_callback,
    +            _get_approval_callback,
    +            _get_sudo_password_callback,
    +        )
    +
    +        def approval_cb(_cmd, _desc):
    +            return "once"
    +
    +        def sudo_cb():
    +            return "hunter2"
    +
    +        seen = {}
    +
    +        def _worker():
    +            # Mimic cli.py's run_agent() thread target.
    +            set_approval_callback(approval_cb)
    +            set_sudo_password_callback(sudo_cb)
    +            try:
    +                seen["approval"] = _get_approval_callback()
    +                seen["sudo"] = _get_sudo_password_callback()
    +            finally:
    +                set_approval_callback(None)
    +                set_sudo_password_callback(None)
    +                seen["approval_after"] = _get_approval_callback()
    +                seen["sudo_after"] = _get_sudo_password_callback()
    +
    +        t = threading.Thread(target=_worker, daemon=True)
    +        t.start()
    +        t.join(timeout=2)
    +
    +        assert seen["approval"] is approval_cb
    +        assert seen["sudo"] is sudo_cb
    +        # Finally block must clear both slots — otherwise a reused thread
    +        # would hold a stale reference to a disposed CLI instance.
    +        assert seen["approval_after"] is None
    +        assert seen["sudo_after"] is None

  • ui-tui/src/app/useInputHandlers.ts

    @@ -172,6 +172,17 @@ export function useInputHandlers(ctx: InputHandlerContext): InputHandlerResult {
         const live = getUiState()
     
         if (isBlocked) {
    +      // When approval/clarify/confirm overlays are active, their own useInput
    +      // handlers must receive keystrokes (arrow keys, numbers, Enter).  Only
    +      // intercept Ctrl+C here so the user can deny/dismiss — all other keys
    +      // fall through to the component-level handlers.
    +      if (overlay.approval || overlay.clarify || overlay.confirm) {
    +        if (isCtrl(key, ch, 'c')) {
    +          cancelOverlayFromCtrlC()
    +        }
    +        return
    +      }
    +
           if (overlay.pager) {
             if (key.escape || isCtrl(key, ch, 'c') || ch === 'q') {
               return patchOverlayState({ pager: null })

• #16574 fix(approval): close prompt_toolkit deadlock vectors (#15216)

  merged to main

  files: run_agent.py, scripts/release.py, tests/run_agent/test_background_review.py, tests/tools/test_approval.py, tools/approval.py

  Show the diff

  • run_agent.py

    @@ -74,6 +74,12 @@
         check_toolset_requirements,
     )
     from tools.terminal_tool import cleanup_vm, get_active_env, is_persistent_env
    +from tools.terminal_tool import (
    +    set_approval_callback as _set_approval_callback,
    +    set_sudo_password_callback as _set_sudo_password_callback,
    +    _get_approval_callback,
    +    _get_sudo_password_callback,
    +)
     from tools.tool_result_storage import maybe_persist_tool_result, enforce_turn_budget
     from tools.interrupt import set_interrupt as _set_interrupt
     from tools.browser_tool import cleanup_browser
    @@ -3241,6 +3247,21 @@ def _spawn_background_review(
     
             def _run_review():
                 import contextlib
    +            # Install a non-interactive approval callback on this worker
    +            # thread so any dangerous-command guard the review agent trips
    +            # resolves to "deny" instead of falling back to input() -- which
    +            # deadlocks against the parent's prompt_toolkit TUI (#15216).
    +            # Same pattern as _subagent_auto_deny in tools/delegate_tool.py.
    +            def _bg_review_auto_deny(command, description, **kwargs):
    +                logger.warning(
    +                    "Background review auto-denied dangerous command: %s (%s)",
    +                    command, description,
    +                )
    +                return "deny"
    +            try:
    +                _set_approval_callback(_bg_review_auto_deny)
    +            except Exception:
    +                pass
                 review_agent = None
                 try:
                     with open(os.devnull, "w") as _devnull, \
    @@ -3322,6 +3343,12 @@ def _run_review():
                             review_agent.close()
                         except Exception:
                             pass
    +                # Clear the approval callback on this bg-review thread so a
    +                # recycled thread-id doesn't inherit a stale reference.
    +                try:
    +                    _set_approval_callback(None)
    +                except Exception:
    +                    pass
     
             t = threading.Thread(target=_run_review, daemon=True, name="bg-review")
             t.start()
    @@ -8469,6 +8496,14 @@ def _execute_tool_calls_concurrent(self, assistant_message, messages: list, effe
             self._current_tool = tool_names_str
             self._touch_activity(f"executing {num_tools} tools concurrently: {tool_names_str}")
     
    +        # Capture CLI callbacks from the agent thread so worker threads can
    +        # register them locally.  Without this, _get_approval_callback() in
    +        # terminal_tool returns None in ThreadPoolExecutor workers, causing
    +        # the dangerous-command prompt to fall back to input() — which
    +        # deadlocks against prompt_toolkit's raw terminal mode (#13617).
    +        _parent_approval_cb = _get_approval_callback()
    +        _parent_sudo_cb = _get_sudo_password_callback()
    +
             def _run_tool(index, tool_call, function_name, function_args):
                 """Worker function executed in a thread."""
                 # Register this worker tid so the agent can fan out an interrupt
    @@ -8495,6 +8530,18 @@ def _run_tool(index, tool_call, function_name, function_args):
                     set_activity_callback(self._touch_activity)
                 except Exception:
                     pass
    +            # Propagate approval/sudo callbacks to this worker thread.
    +            # Mirrors cli.py run_agent() pattern (GHSA-qg5c-hvr5-hjgr).
    +            if _parent_approval_cb is not None:
    +                try:
    +                    _set_approval_callback(_parent_approval_cb)
    +                except Exception:
    +                    pass
    +            if _parent_sudo_cb is not None:
    +                try:
    +                    _set_sudo_password_callback(_parent_sudo_cb)
    +                except Exception:
    +                    pass
                 start = time.time()
                 try:
                     result = self._invoke_tool(function_name, function_args, effective_task_id, tool_call.id, messages=messages)
    @@ -8517,6 +8564,13 @@ def _run_tool(index, tool_call, function_name, function_args):
                     _set_interrupt(False, _worker_tid)
                 except Exception:
                     pass
    +            # Clear thread-local callbacks so a recycled worker thread
    +            # doesn't hold stale references to a disposed CLI instance.
    +            try:
    +                _set_approval_callback(None)
    +                _set_sudo_password_callback(None)
    +            except Exception:
    +                pass
     
             # Start spinner for CLI mode (skip when TUI handles tool progress)
             spinner = None

  • scripts/release.py

    @@ -553,6 +553,7 @@
         "[redacted:email]": "chen1749144759",
         "[redacted:email]": "MorAlekss",
         "[redacted:email]": "ash",
    +    "[redacted:email]": "andrewhosf",
     }
     
     

  • tests/run_agent/test_background_review.py

    @@ -71,3 +71,59 @@ def close(self):
             "shutdown_memory_provider",
             "close",
         ]
    +
    +
    +def test_background_review_installs_auto_deny_approval_callback(monkeypatch):
    +    """Regression guard for #15216.
    +
    +    The background review thread must install a non-interactive approval
    +    callback. If it doesn't, any dangerous-command guard the review agent
    +    trips falls back to input() on a daemon thread, which deadlocks against
    +    the parent's prompt_toolkit TUI.
    +    """
    +    import tools.terminal_tool as tt
    +
    +    observed: dict = {"during_run": "<unread>", "after_finally": "<unread>"}
    +
    +    class FakeReviewAgent:
    +        def __init__(self, **kwargs):
    +            self._session_messages = []
    +
    +        def run_conversation(self, **kwargs):
    +            # Capture what the callback looks like mid-run. It must be
    +            # a callable (the auto-deny) -- not None.
    +            observed["during_run"] = tt._get_approval_callback()
    +
    +        def shutdown_memory_provider(self):
    +            pass
    +
    +        def close(self):
    +            pass
    +
    +    monkeypatch.setattr(run_agent_module, "AIAgent", FakeReviewAgent)
    +    monkeypatch.setattr(run_agent_module.threading, "Thread", ImmediateThread)
    +
    +    # Start from a clean slot.
    +    tt.set_approval_callback(None)
    +    agent = _bare_agent()
    +
    +    AIAgent._spawn_background_review(
    +        agent,
    +        messages_snapshot=[{"role": "user", "content": "hello"}],
    +        review_memory=True,
    +    )
    +
    +    observed["after_finally"] = tt._get_approval_callback()
    +
    +    assert callable(observed["during_run"]), (
    +        "Background review did not install an approval callback on its "
    +        "worker thread; dangerous-command prompts will deadlock against "
    +        "the parent TUI (#15216)."
    +    )
    +    # The installed callback must deny (it's a safety gate, not a prompt).
    +    assert observed["during_run"]("rm -rf /", "test") == "deny"
    +
    +    assert observed["after_finally"] is None, (
    +        "Background review leaked its approval callback into the worker "
    +        "thread's TLS slot; a recycled thread-id could reuse it."
    +    )

  • tests/tools/test_approval.py

    @@ -906,3 +906,62 @@ def test_safe_chmod_without_execute_not_flagged(self):
             cmd = "chmod +x script.sh"
             dangerous, _, _ = detect_dangerous_command(cmd)
             assert dangerous is False
    +
    +
    +class TestFailClosedUnderPromptToolkit:
    +    """Regression guard for #15216.
    +
    +    When prompt_toolkit owns the terminal and no approval callback is
    +    registered on the calling thread, prompt_dangerous_approval() must
    +    deny fast instead of falling through to the input() fallback -- which
    +    deadlocks because the user's keystrokes go to prompt_toolkit's raw-mode
    +    stdin capture, not to input().
    +    """
    +
    +    def test_denies_when_prompt_toolkit_active_and_no_callback(self):
    +        import threading
    +        import prompt_toolkit.application.current as ptc
    +
    +        orig = ptc.get_app_or_none
    +        ptc.get_app_or_none = lambda: object()  # pretend a pt app is running
    +        result = []
    +        try:
    +            def run():
    +                result.append(
    +                    prompt_dangerous_approval(
    +                        "rm -rf /",
    +                        "test danger",
    +                        timeout_seconds=30,
    +                        approval_callback=None,
    +                    )
    +                )
    +
    +            t = threading.Thread(target=run, daemon=True)
    +            t.start()
    +            t.join(timeout=3)
    +            assert not t.is_alive(), (
    +                "prompt_dangerous_approval deadlocked under prompt_toolkit "
    +                "with no callback -- fail-closed guard is broken"
    +            )
    +            assert result == ["deny"]
    +        finally:
    +            ptc.get_app_or_none = orig
    +
    +    def test_callback_path_still_wins_over_guard(self):
    +        """Guard must not short-circuit a valid callback."""
    +        import prompt_toolkit.application.current as ptc
    +
    +        orig = ptc.get_app_or_none
    +        ptc.get_app_or_none = lambda: object()
    +        try:
    +            def cb(command, description, **kwargs):
    +                return "once"
    +
    +            result = prompt_dangerous_approval(
    +                "rm -rf /",
    +                "test danger",
    +                approval_callback=cb,
    +            )
    +            assert result == "once"
    +        finally:
    +            ptc.get_app_or_none = orig

  • tools/approval.py

    @@ -536,6 +536,33 @@ def prompt_dangerous_approval(command: str, description: str,
                 logger.error("Approval callback failed: %s", e, exc_info=True)
                 return "deny"
     
    +    # Fail-closed guard: if prompt_toolkit owns the terminal (interactive
    +    # CLI session) and no approval callback is registered on this thread,
    +    # the input() fallback below would spawn a daemon thread whose read
    +    # can never see Enter -- the user's keystrokes go to prompt_toolkit,
    +    # not input(), producing an invisible 60s deadlock (issue #15216).
    +    # Deny fast and log loudly instead 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 pattern).
    +    try:
    +        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 "
    +                "to avoid stdin deadlock. command=%r description=%r",
    +                command, description,
    +            )
    +            return "deny"
    +    except Exception:
    +        # prompt_toolkit not installed, or detection failed -- fall through
    +        # to the legacy input() path (safe in non-TUI contexts: scripts,
    +        # tests, sshd, etc.).
    +        pass
    +
         os.environ["HERMES_SPINNER_PAUSE"] = "1"
         try:
             while True: