mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-20 05:01:30 +00:00
fix: use TUI modal for slash confirmations
This commit is contained in:
parent
68d081f570
commit
ca1d4375ab
3 changed files with 391 additions and 67 deletions
|
|
@ -6,6 +6,7 @@ don't have to construct a full HermesCLI (which requires extensive setup).
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import queue
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -17,10 +18,17 @@ def _bound(fn, instance):
|
|||
|
||||
def _make_self(prompt_response):
|
||||
"""Build a minimal stand-in 'self' for _confirm_destructive_slash."""
|
||||
return SimpleNamespace(
|
||||
from cli import HermesCLI
|
||||
|
||||
self_ = SimpleNamespace(
|
||||
_app=None,
|
||||
_prompt_text_input=lambda _prompt: prompt_response,
|
||||
_prompt_text_input_modal=lambda **_kw: prompt_response,
|
||||
)
|
||||
self_._normalize_slash_confirm_choice = _bound(
|
||||
HermesCLI._normalize_slash_confirm_choice, self_,
|
||||
)
|
||||
return self_
|
||||
|
||||
|
||||
def test_gate_off_returns_once_without_prompting():
|
||||
|
|
@ -117,7 +125,6 @@ def test_gate_on_choice_always_persists_and_returns_always():
|
|||
self_ = _make_self(prompt_response="2")
|
||||
|
||||
saves = []
|
||||
|
||||
def _fake_save(key, value):
|
||||
saves.append((key, value))
|
||||
return True
|
||||
|
|
@ -150,3 +157,55 @@ def test_gate_default_true_when_config_missing():
|
|||
# treated as on despite the config error. If the gate had been off
|
||||
# this would have returned 'once' without consulting the prompt.
|
||||
assert result is None
|
||||
|
||||
|
||||
def test_slash_confirm_modal_number_selection_submits_without_raw_input():
|
||||
"""Pressing 2 in the TUI modal should resolve to Always Approve directly."""
|
||||
from cli import HermesCLI
|
||||
|
||||
q = queue.Queue()
|
||||
self_ = SimpleNamespace(
|
||||
_slash_confirm_state={
|
||||
"choices": [
|
||||
("once", "Approve Once", "proceed once"),
|
||||
("always", "Always Approve", "persist opt-out"),
|
||||
("cancel", "Cancel", "abort"),
|
||||
],
|
||||
"selected": 0,
|
||||
"response_queue": q,
|
||||
},
|
||||
_slash_confirm_deadline=123,
|
||||
_invalidate=lambda: None,
|
||||
)
|
||||
|
||||
_bound(HermesCLI._submit_slash_confirm_response, self_)("always")
|
||||
|
||||
assert q.get_nowait() == "always"
|
||||
assert self_._slash_confirm_state is None
|
||||
assert self_._slash_confirm_deadline == 0
|
||||
|
||||
|
||||
def test_slash_confirm_display_fragments_include_choice_mapping():
|
||||
"""The modal itself must show what 1/2/3 mean, not only 'Choice [1/2/3]'."""
|
||||
from cli import HermesCLI
|
||||
|
||||
self_ = SimpleNamespace(
|
||||
_slash_confirm_state={
|
||||
"title": "⚠️ /new — destroys conversation state",
|
||||
"detail": "This starts a fresh session.",
|
||||
"choices": [
|
||||
("once", "Approve Once", "proceed once"),
|
||||
("always", "Always Approve", "persist opt-out"),
|
||||
("cancel", "Cancel", "abort"),
|
||||
],
|
||||
"selected": 1,
|
||||
},
|
||||
)
|
||||
|
||||
fragments = _bound(HermesCLI._get_slash_confirm_display_fragments, self_)()
|
||||
rendered = "".join(fragment for _style, fragment in fragments)
|
||||
|
||||
assert "[1] Approve Once" in rendered
|
||||
assert "[2] Always Approve" in rendered
|
||||
assert "[3] Cancel" in rendered
|
||||
assert "Type 1/2/3" in rendered
|
||||
|
|
|
|||
|
|
@ -1,15 +1,9 @@
|
|||
"""Tests for ``HermesCLI._prompt_text_input`` thread-safe input dispatch.
|
||||
|
||||
Slash commands (``/clear``, ``/new``, ``/undo``, ``/reload-mcp``) are dispatched
|
||||
from the ``process_loop`` daemon thread. ``prompt_toolkit.run_in_terminal``
|
||||
returns a coroutine that only the main-thread event loop can drive; calling it
|
||||
from a daemon thread orphans the coroutine, ``_ask`` never runs, and user
|
||||
keystrokes leak into the composer instead of the confirmation prompt
|
||||
(see issue #23185).
|
||||
|
||||
The fix mirrors ``_run_curses_picker``: when off the main thread, fall back to
|
||||
a direct ``input()`` call so the prompt actually renders and consumes
|
||||
keystrokes.
|
||||
Raw ``input()`` prompts can race with prompt_toolkit when called from the TUI.
|
||||
The normal slash confirmations now use a prompt_toolkit-native modal, but
|
||||
``_prompt_text_input`` remains as a fallback for non-interactive calls and edge
|
||||
cases.
|
||||
"""
|
||||
|
||||
import threading
|
||||
|
|
@ -17,7 +11,7 @@ from unittest.mock import MagicMock, patch
|
|||
|
||||
|
||||
def _make_cli():
|
||||
"""Minimal HermesCLI shell exposing ``_prompt_text_input``."""
|
||||
"""Minimal HermesCLI shell exposing prompt fallback helpers."""
|
||||
import cli as cli_mod
|
||||
|
||||
obj = object.__new__(cli_mod.HermesCLI)
|
||||
|
|
@ -33,7 +27,7 @@ class TestPromptTextInputThreadSafety:
|
|||
|
||||
with patch("prompt_toolkit.application.run_in_terminal") as mock_rit, \
|
||||
patch("builtins.input", return_value="2"):
|
||||
result = cli._prompt_text_input("Choice: ")
|
||||
cli._prompt_text_input("Choice: ")
|
||||
|
||||
# run_in_terminal was invoked; the _ask closure passed to it would
|
||||
# call input() when driven by the event loop. We assert dispatch path,
|
||||
|
|
@ -43,10 +37,8 @@ class TestPromptTextInputThreadSafety:
|
|||
def test_background_thread_falls_back_to_direct_input(self):
|
||||
"""On a daemon thread, skip run_in_terminal and call input() directly.
|
||||
|
||||
This is the bug from issue #23185: process_loop dispatches slash
|
||||
commands on a daemon thread, so run_in_terminal's coroutine is
|
||||
orphaned. The fallback must drive input() itself so user keystrokes
|
||||
don't leak into the agent buffer.
|
||||
This preserves the fallback for any prompt that still runs off the main
|
||||
UI thread: run_in_terminal's coroutine would otherwise be orphaned.
|
||||
"""
|
||||
cli = _make_cli()
|
||||
captured = {}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue