mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(cli): use the confirm modal on native Windows instead of deadlocking input()
Native Windows bypassed the destructive-slash modal and fell back to a raw input() prompt. When the confirm was triggered from the process_loop daemon thread (the normal case), that input() deadlocked against prompt_toolkit's main-thread stdin ownership: bare /reset froze with Ctrl-C swallowed, while /reset now worked only because it skips the prompt. Route native Windows through the existing call_soon_threadsafe modal path (the same key-binding channel that already handles normal typing on Windows); keep the stdin fallback only for the safe no-app / scheduling-failure cases, and clean-cancel (None) off the main thread on win32 so a degraded path never re-deadlocks. Addresses #33961 Refs #30768
This commit is contained in:
parent
d66bac5a1a
commit
ab98818e5b
1 changed files with 33 additions and 35 deletions
68
cli.py
68
cli.py
|
|
@ -6218,27 +6218,20 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
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.
|
||||
**Platform note (Windows — issue #33961):**
|
||||
Earlier code bypassed the modal on ``sys.platform == "win32"`` and fell
|
||||
back to a raw ``input()`` prompt. When the confirm was triggered from the
|
||||
``process_loop`` daemon thread (the normal case) that ``input()`` ran off
|
||||
the main thread and deadlocked against prompt_toolkit's stdin ownership —
|
||||
the user saw a frozen cursor and Ctrl-C was swallowed (bare ``/reset``
|
||||
froze; ``/reset now`` worked only because it skips the prompt entirely).
|
||||
|
||||
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.
|
||||
* ``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.
|
||||
Native Windows now uses the same path as Linux/macOS: the modal is set up
|
||||
on ``self._app.loop`` via ``call_soon_threadsafe`` and answered by the
|
||||
normal prompt_toolkit key bindings (the same input channel that already
|
||||
handles ordinary typing on Windows). The raw ``input()`` fallback is kept
|
||||
only for the genuinely safe cases: no running app (unit tests /
|
||||
non-interactive), no resolvable event loop, or a scheduling failure.
|
||||
"""
|
||||
import threading
|
||||
import time as _time
|
||||
|
|
@ -6251,23 +6244,26 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
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]: ")
|
||||
|
||||
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:
|
||||
|
||||
def _stdin_fallback() -> str | None:
|
||||
# On native Windows a raw input() from a non-main thread deadlocks
|
||||
# against prompt_toolkit's stdin ownership (#33961). With an app
|
||||
# running we cannot safely prompt off the main thread, so cancel
|
||||
# cleanly (None) rather than hang the terminal.
|
||||
if sys.platform == "win32" and not in_main_thread:
|
||||
self._invalidate()
|
||||
return None
|
||||
return self._prompt_text_input("Choice [1/2/3]: ")
|
||||
|
||||
if not in_main_thread and app_loop is None:
|
||||
return _stdin_fallback()
|
||||
|
||||
response_queue = queue.Queue()
|
||||
|
||||
def _setup_modal() -> None:
|
||||
|
|
@ -6307,7 +6303,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
return ready.wait(timeout=5)
|
||||
|
||||
if not _run_on_app_loop(_setup_modal):
|
||||
return self._prompt_text_input("Choice [1/2/3]: ")
|
||||
return _stdin_fallback()
|
||||
|
||||
_last_countdown_refresh = _time.monotonic()
|
||||
try:
|
||||
|
|
@ -8223,9 +8219,10 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
print(" ⚠️ MCP reload timed out (30s). Some servers may not have reconnected.")
|
||||
|
||||
# Inline-skip tokens that bypass the destructive-slash confirmation modal.
|
||||
# Matches the escape-hatch pattern users on broken modal platforms
|
||||
# (currently native Windows PowerShell — issue #30768) need to self-serve
|
||||
# without having to flip approvals.destructive_slash_confirm in config.
|
||||
# A general escape hatch for non-interactive use (scripting/automation) and
|
||||
# for the degraded path where the modal can't be marshaled onto the app loop
|
||||
# — lets users self-serve without flipping approvals.destructive_slash_confirm
|
||||
# in config. (Native Windows now drives the modal normally — see #33961.)
|
||||
_DESTRUCTIVE_SKIP_TOKENS = frozenset({"now", "--yes", "-y"})
|
||||
|
||||
@classmethod
|
||||
|
|
@ -8283,8 +8280,9 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin):
|
|||
Inline-skip: if ``cmd_original`` contains ``now``, ``--yes``, or
|
||||
``-y`` as an argument (e.g. ``/reset now``, ``/new --yes My title``),
|
||||
the modal is bypassed and ``"once"`` is returned immediately. This is
|
||||
an escape hatch for platforms where the prompt_toolkit modal hangs
|
||||
(issue #30768 — native Windows PowerShell). Callers are responsible
|
||||
an escape hatch for non-interactive use and for the degraded path where
|
||||
the modal can't be marshaled onto the app loop (native Windows itself now
|
||||
drives the modal normally — see #33961). Callers are responsible
|
||||
for stripping the skip tokens from any remaining argument parsing
|
||||
(see :meth:`_split_destructive_skip`).
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue