From fae815adc21baba6d12024c94e4dc3a39ae4df5e Mon Sep 17 00:00:00 2001 From: simokiihamaki Date: Mon, 25 May 2026 03:50:03 -0700 Subject: [PATCH] fix(cli): prevent /reset and /new freeze on Windows by falling back to stdin prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows (PowerShell/Windows Terminal), the queue-based modal used for destructive slash command confirmations deadlocks because prompt_toolkit's input channel becomes unresponsive when entered from the process_loop daemon thread. Keystrokes never reach the key bindings, so response_queue.get() blocks until the 120-second timeout expires. Fix: fall back to _prompt_text_input (stdin-based) when: 1. sys.platform == 'win32' — Windows console doesn't support the modal reliably 2. Called from non-main thread — key bindings can't fire from daemon threads 3. self._app is not set — existing behavior for tests/non-interactive This mirrors the thread-aware guard from _prompt_text_input (PR #23454). 9 new regression tests covering Windows detection, non-main thread fallback, macOS/Linux modal preservation, and integration with _confirm_destructive_slash. Fixes #30768 Surgical reapply of PR #30773. Original branch was many months stale (911 files / 146k LOC of unrelated reverts); the substantive ~30 LOC change in cli.py plus the new test file were reapplied onto current main with the contributor's authorship preserved via --author. --- cli.py | 35 ++++ tests/cli/test_slash_confirm_windows.py | 259 ++++++++++++++++++++++++ 2 files changed, 294 insertions(+) create mode 100644 tests/cli/test_slash_confirm_windows.py diff --git a/cli.py b/cli.py index 76433fb9170..9bfe55da95b 100644 --- a/cli.py +++ b/cli.py @@ -7034,7 +7034,28 @@ class HermesCLI: could be interpreted as EOF/exit. A first-class modal state keeps the choices visible and lets the normal Enter key binding submit the typed or highlighted choice. + + **Platform note (Windows dead-lock — issue #30768):** + The queue-based modal relies on prompt_toolkit key bindings receiving + keyboard events and calling ``_submit_slash_confirm_response``. On + Windows (PowerShell / Windows Terminal) the prompt_toolkit input + channel can become unresponsive when the modal is entered from the + ``process_loop`` daemon thread, causing a dead-lock: the user sees the + confirmation panel but keystrokes never reach the key bindings and the + ``response_queue.get()`` blocks until the 120-second timeout expires. + + To avoid this, we fall back to ``_prompt_text_input`` (a simple + ``input()``-based prompt) when any of these conditions hold: + + * ``sys.platform == "win32"`` — native Windows console (ConPTY / + win32_input) does not support the modal reliably. + * Called from a non-main thread — the prompt_toolkit event loop only + runs on the main thread; key bindings can't fire from a daemon + thread (same rationale as the ``_prompt_text_input`` thread guard + in PR #23454). + * ``self._app`` is not set — unit tests / non-interactive contexts. """ + import threading import time as _time if not choices: @@ -7045,6 +7066,20 @@ class HermesCLI: if not getattr(self, "_app", None): return self._prompt_text_input("Choice [1/2/3]: ") + # On Windows the prompt_toolkit input channel can deadlock when the + # modal is entered from the process_loop daemon thread — keystrokes + # never reach the key bindings, so response_queue.get() blocks for + # the full timeout (issue #30768). Fall back to the simpler + # stdin-based prompt which works reliably on Windows. + if sys.platform == "win32": + return self._prompt_text_input("Choice [1/2/3]: ") + + # Mirror the thread-aware guard from _prompt_text_input (PR #23454): + # run_in_terminal and the modal queue both depend on the main-thread + # event loop. From a daemon thread the modal key bindings never fire. + if threading.current_thread() is not threading.main_thread(): + return self._prompt_text_input("Choice [1/2/3]: ") + response_queue = queue.Queue() self._capture_modal_input_snapshot() self._slash_confirm_state = { diff --git a/tests/cli/test_slash_confirm_windows.py b/tests/cli/test_slash_confirm_windows.py new file mode 100644 index 00000000000..2ec341f456d --- /dev/null +++ b/tests/cli/test_slash_confirm_windows.py @@ -0,0 +1,259 @@ +"""Regression tests for issue #30768: /reset and /new freeze on Windows. + +``_prompt_text_input_modal`` uses a queue-based modal that relies on +prompt_toolkit key bindings receiving keyboard events. On Windows the +prompt_toolkit input channel can deadlock when the modal is entered from +the ``process_loop`` daemon thread. The fix falls back to the simpler +``_prompt_text_input`` (stdin-based) prompt on Windows and non-main threads. + +These tests verify: +1. Windows detection triggers the stdin fallback +2. Non-main thread detection triggers the stdin fallback +3. macOS/Linux main-thread path still uses the modal (no regression) +4. No-app path still uses the stdin fallback (existing behavior) +5. Empty choices returns None (existing behavior) +""" + +import queue +import sys +import threading +import time +from unittest.mock import MagicMock, patch + +import pytest + + +def _make_cli(): + """Minimal HermesCLI shell exposing prompt/modal helpers.""" + import cli as cli_mod + + obj = object.__new__(cli_mod.HermesCLI) + obj._app = MagicMock() + obj._status_bar_visible = True + obj._last_invalidate = 0.0 + obj._modal_input_snapshot = None + obj._slash_confirm_state = None + obj._slash_confirm_deadline = 0 + return obj + + +# --------------------------------------------------------------------------- +# Sample choices used across tests +# --------------------------------------------------------------------------- +_SAMPLE_CHOICES = [ + ("once", "Approve Once", "proceed this time only"), + ("always", "Always Approve", "proceed and silence this prompt permanently"), + ("cancel", "Cancel", "keep current conversation"), +] + + +class TestModalWindowsFallback: + """Windows dead-lock regression tests for _prompt_text_input_modal.""" + + def test_windows_falls_back_to_stdin(self): + """On Windows, _prompt_text_input_modal should use _prompt_text_input.""" + cli = _make_cli() + + with patch.object(sys, "platform", "win32"), \ + patch.object(cli, "_prompt_text_input", return_value="1") as mock_stdin: + result = cli._prompt_text_input_modal( + title="⚠️ /new — destroys conversation state", + detail="This starts a fresh session.", + choices=_SAMPLE_CHOICES, + ) + + # The stdin-based fallback was used, not the modal queue path. + mock_stdin.assert_called_once_with("Choice [1/2/3]: ") + assert result == "1" + + def test_non_main_thread_falls_back_to_stdin(self): + """Off the main thread, _prompt_text_input_modal should use stdin fallback.""" + cli = _make_cli() + result_holder = {} + + def run_on_daemon(): + # Patch platform to "linux" so the Windows check doesn't short-circuit. + with patch.object(sys, "platform", "linux"), \ + patch.object(cli, "_prompt_text_input", return_value="2") as mock_stdin: + result_holder["result"] = cli._prompt_text_input_modal( + title="⚠️ /reset", + detail="This starts a fresh session.", + choices=_SAMPLE_CHOICES, + ) + result_holder["stdin_called"] = mock_stdin.called + + t = threading.Thread(target=run_on_daemon, daemon=True) + t.start() + t.join(timeout=2.0) + assert not t.is_alive(), "daemon thread hung — modal deadlocked" + assert result_holder["stdin_called"] is True + assert result_holder["result"] == "2" + + def test_main_thread_non_windows_uses_modal(self): + """On macOS/Linux main thread, the queue-based modal is still used.""" + cli = _make_cli() + + # We need to simulate the modal receiving a response. We'll patch + # the response_queue to immediately return a value. + with patch.object(sys, "platform", "darwin"), \ + patch.object(cli, "_capture_modal_input_snapshot"), \ + patch.object(cli, "_restore_modal_input_snapshot"), \ + patch.object(cli, "_invalidate"): + # Start the modal in a way that it will receive a response + # immediately via the queue. + original_queue = queue.Queue + original_time = time.monotonic + + def _fake_modal_flow(*args, **kwargs): + """Simulate the modal flow: set state, put response, return.""" + # We'll directly test that the modal path is entered by + # checking that _slash_confirm_state was set. + pass + + # Since we can't easily mock the internal queue, let's test + # that the modal path is entered by checking that + # _prompt_text_input was NOT called. + with patch.object(cli, "_prompt_text_input") as mock_stdin: + # Set up a response that will be put into the queue + # after the modal starts waiting. + def _submit_after_delay(): + time.sleep(0.2) + state = cli._slash_confirm_state + if state and "response_queue" in state: + state["response_queue"].put("once") + + submitter = threading.Thread(target=_submit_after_delay, daemon=True) + submitter.start() + + result = cli._prompt_text_input_modal( + title="⚠️ /new", + detail="This starts a fresh session.", + choices=_SAMPLE_CHOICES, + timeout=5, + ) + + submitter.join(timeout=2.0) + + # The stdin fallback should NOT have been called. + mock_stdin.assert_not_called() + # The result should be "once" from the simulated modal response. + assert result == "once" + + def test_no_app_falls_back_to_stdin(self): + """Without a prompt_toolkit app, always use stdin fallback.""" + cli = _make_cli() + cli._app = None + + with patch.object(cli, "_prompt_text_input", return_value="3") as mock_stdin: + result = cli._prompt_text_input_modal( + title="⚠️ /clear", + detail="This clears the screen.", + choices=_SAMPLE_CHOICES, + ) + + mock_stdin.assert_called_once_with("Choice [1/2/3]: ") + assert result == "3" + + def test_empty_choices_returns_none(self): + """Empty choices list should return None without prompting.""" + cli = _make_cli() + + with patch.object(cli, "_prompt_text_input") as mock_stdin: + result = cli._prompt_text_input_modal( + title="Test", + detail="Test", + choices=[], + ) + + mock_stdin.assert_not_called() + assert result is None + + def test_windows_fallback_does_not_set_modal_state(self): + """Verify Windows fallback doesn't leave _slash_confirm_state set.""" + cli = _make_cli() + + with patch.object(sys, "platform", "win32"), \ + patch.object(cli, "_prompt_text_input", return_value="1"): + cli._prompt_text_input_modal( + title="⚠️ /reset", + detail="This starts a fresh session.", + choices=_SAMPLE_CHOICES, + ) + + assert cli._slash_confirm_state is None + + def test_non_main_thread_fallback_does_not_set_modal_state(self): + """Verify daemon-thread fallback doesn't leave modal state set.""" + cli = _make_cli() + errors = [] + + def run_on_daemon(): + try: + with patch.object(sys, "platform", "linux"), \ + patch.object(cli, "_prompt_text_input", return_value="1"): + cli._prompt_text_input_modal( + title="⚠️ /new", + detail="This starts a fresh session.", + choices=_SAMPLE_CHOICES, + ) + if cli._slash_confirm_state is not None: + errors.append("_slash_confirm_state should be None") + except Exception as exc: + errors.append(str(exc)) + + t = threading.Thread(target=run_on_daemon, daemon=True) + t.start() + t.join(timeout=2.0) + assert not errors, f"unexpected errors: {errors}" + assert cli._slash_confirm_state is None + + +class TestConfirmDestructiveSlashWindows: + """Integration-level tests for _confirm_destructive_slash on Windows.""" + + def test_confirm_destructive_slash_bypasses_modal_on_windows(self): + """_confirm_destructive_slash should work on Windows via stdin fallback.""" + cli = _make_cli() + cli.model = "test-model" + cli._agent_running = False + cli._spinner_text = "" + cli._should_exit = False + cli._command_running = False + cli.session_id = "test-session" + cli._pending_tool_info = {} + cli._tool_start_time = 0.0 + cli._last_scrollback_tool = "" + + with patch.object(sys, "platform", "win32"), \ + patch.object(cli, "_prompt_text_input", return_value="1"), \ + patch("cli.load_cli_config", return_value={"approvals": {"destructive_slash_confirm": True}}): + result = cli._confirm_destructive_slash( + "new", + "This starts a fresh session.\nThe current conversation history will be discarded.", + ) + + assert result == "once" + + def test_confirm_destructive_slash_cancelled_on_windows(self): + """Cancellation via stdin fallback works on Windows.""" + cli = _make_cli() + cli.model = "test-model" + cli._agent_running = False + cli._spinner_text = "" + cli._should_exit = False + cli._command_running = False + cli.session_id = "test-session" + cli._pending_tool_info = {} + cli._tool_start_time = 0.0 + cli._last_scrollback_tool = "" + + with patch.object(sys, "platform", "win32"), \ + patch.object(cli, "_prompt_text_input", return_value="3"), \ + patch("cli.load_cli_config", return_value={"approvals": {"destructive_slash_confirm": True}}): + result = cli._confirm_destructive_slash( + "reset", + "This starts a fresh session.\nThe current conversation history will be discarded.", + ) + + # Choice "3" normalizes to "cancel", which returns None. + assert result is None