diff --git a/cli.py b/cli.py index e4901c57288..6c77afc07a4 100644 --- a/cli.py +++ b/cli.py @@ -7154,11 +7154,13 @@ class HermesCLI: * ``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. + + On non-Windows platforms the modal itself is still safe from the + ``process_loop`` daemon thread as long as the main-thread event loop + owns the prompt_toolkit buffer mutations. When we are off the main + thread, schedule the modal snapshot / restore work on ``self._app.loop`` + via ``call_soon_threadsafe`` and keep the queue-based response path. """ import threading import time as _time @@ -7179,33 +7181,62 @@ class HermesCLI: 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(): + try: + app_loop = self._app.loop + except Exception: + app_loop = None + + in_main_thread = threading.current_thread() is threading.main_thread() + if not in_main_thread and app_loop is None: return self._prompt_text_input("Choice [1/2/3]: ") response_queue = queue.Queue() - self._capture_modal_input_snapshot() - self._slash_confirm_state = { - "title": title, - "detail": detail, - "choices": choices, - "selected": 0, - "response_queue": response_queue, - } - self._slash_confirm_deadline = _time.monotonic() + timeout - self._invalidate() + + def _setup_modal() -> None: + self._capture_modal_input_snapshot() + self._slash_confirm_state = { + "title": title, + "detail": detail, + "choices": choices, + "selected": 0, + "response_queue": response_queue, + } + self._slash_confirm_deadline = _time.monotonic() + timeout + self._invalidate() + + def _teardown_modal() -> None: + self._slash_confirm_state = None + self._slash_confirm_deadline = 0 + self._restore_modal_input_snapshot() + self._invalidate() + + def _run_on_app_loop(fn) -> bool: + if in_main_thread or app_loop is None: + fn() + return True + ready = threading.Event() + + def _wrapped() -> None: + try: + fn() + finally: + ready.set() + + try: + app_loop.call_soon_threadsafe(_wrapped) + except Exception: + return False + return ready.wait(timeout=5) + + if not _run_on_app_loop(_setup_modal): + return self._prompt_text_input("Choice [1/2/3]: ") _last_countdown_refresh = _time.monotonic() try: while True: try: result = response_queue.get(timeout=1) - self._slash_confirm_state = None - self._slash_confirm_deadline = 0 - self._restore_modal_input_snapshot() - self._invalidate() + _run_on_app_loop(_teardown_modal) return result except queue.Empty: remaining = self._slash_confirm_deadline - _time.monotonic() @@ -7217,10 +7248,7 @@ class HermesCLI: self._invalidate() finally: if self._slash_confirm_state is not None: - self._slash_confirm_state = None - self._slash_confirm_deadline = 0 - self._restore_modal_input_snapshot() - self._invalidate() + _run_on_app_loop(_teardown_modal) return None def _submit_slash_confirm_response(self, value: str | None) -> None: diff --git a/tests/cli/test_slash_confirm_windows.py b/tests/cli/test_slash_confirm_windows.py index 2ec341f456d..980bae32d26 100644 --- a/tests/cli/test_slash_confirm_windows.py +++ b/tests/cli/test_slash_confirm_windows.py @@ -1,14 +1,14 @@ -"""Regression tests for issue #30768: /reset and /new freeze on Windows. +"""Regression tests for issue #30768 and #32383. ``_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. +``_prompt_text_input`` (stdin-based) prompt on Windows. These tests verify: 1. Windows detection triggers the stdin fallback -2. Non-main thread detection triggers the stdin fallback +2. Non-Windows daemon threads still use the modal via the app loop 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) @@ -29,6 +29,7 @@ def _make_cli(): obj = object.__new__(cli_mod.HermesCLI) obj._app = MagicMock() + obj._app.loop = MagicMock() obj._status_bar_visible = True obj._last_invalidate = 0.0 obj._modal_input_snapshot = None @@ -66,28 +67,47 @@ class TestModalWindowsFallback: 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.""" + def test_non_main_thread_uses_modal_via_app_loop(self): + """Off the main thread on Linux, keep the modal path via app-loop setup.""" cli = _make_cli() result_holder = {} + setup_calls = [] + teardown_calls = [] + + def _call_soon_threadsafe(callback): + callback() 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: + patch.object(cli._app.loop, "call_soon_threadsafe", side_effect=_call_soon_threadsafe), \ + patch.object(cli, "_prompt_text_input") as mock_stdin, \ + patch.object(cli, "_capture_modal_input_snapshot", side_effect=lambda: setup_calls.append("capture")), \ + patch.object(cli, "_restore_modal_input_snapshot", side_effect=lambda: teardown_calls.append("restore")): result_holder["result"] = cli._prompt_text_input_modal( title="⚠️ /reset", detail="This starts a fresh session.", choices=_SAMPLE_CHOICES, + timeout=5, ) result_holder["stdin_called"] = mock_stdin.called + 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) t = threading.Thread(target=run_on_daemon, daemon=True) + submitter.start() t.start() t.join(timeout=2.0) + submitter.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" + assert result_holder["stdin_called"] is False + assert result_holder["result"] == "once" + assert setup_calls == ["capture"] + assert teardown_calls == ["restore"] def test_main_thread_non_windows_uses_modal(self): """On macOS/Linux main thread, the queue-based modal is still used.""" @@ -182,20 +202,33 @@ class TestModalWindowsFallback: 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.""" + def test_non_main_thread_modal_clears_state(self): + """Verify daemon-thread modal teardown does not leave state behind.""" cli = _make_cli() errors = [] + def _call_soon_threadsafe(callback): + callback() + def run_on_daemon(): try: with patch.object(sys, "platform", "linux"), \ - patch.object(cli, "_prompt_text_input", return_value="1"): + patch.object(cli._app.loop, "call_soon_threadsafe", side_effect=_call_soon_threadsafe): + def _submit_after_delay(): + time.sleep(0.2) + state = cli._slash_confirm_state + if state and "response_queue" in state: + state["response_queue"].put("cancel") + + submitter = threading.Thread(target=_submit_after_delay, daemon=True) + submitter.start() cli._prompt_text_input_modal( title="⚠️ /new", detail="This starts a fresh session.", choices=_SAMPLE_CHOICES, + timeout=5, ) + submitter.join(timeout=2.0) if cli._slash_confirm_state is not None: errors.append("_slash_confirm_state should be None") except Exception as exc: