mirrors/hermes-agent
1
0
Fork 0
mirror of https://github.com/NousResearch/hermes-agent.git synced 2026-06-09 08:21:50 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 10

Merge pull request #41155 from kshitijk4poor/fix/cli-modal-direct-invalidate-41098

Browse source 
fix(cli): paint approval/clarify/sudo/secret modal prompts directly, not via the throttle (#41098)
This commit is contained in:
kshitij 2026-06-08 01:01:51 -07:00 • committed by GitHub
commit 4107076128
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 181 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

93
cli.py
View file

@ -3479,7 +3479,22 @@ class HermesCLI:
self._background_task_counter = 0
def _invalidate(self, min_interval: float = 0.25) -> None:
"""Throttled UI repaint — prevents terminal blinking on slow/SSH connections."""
"""Throttled UI repaint for high-frequency background updates.
Use this for spinner frames, streaming token flushes, and other
repaints that can fire many times per second — the throttle prevents
terminal blinking on slow/SSH connections, and the resize-recovery
guard avoids stamping footer/status-bar chrome into scrollback while a
SIGWINCH reflow is in flight.
Do NOT use this for user-blocking modal prompts (approval / clarify /
sudo). Those are rare, one-shot, user-blocking events that must paint
immediately; route them through ``self._app.invalidate()`` directly, the
same way the modal key-binding handlers already do. Sending a modal's
entry paint through this throttle lets an unrelated background repaint
within the 250ms window — or an in-flight resize — silently drop it, so
the prompt never renders and times out unseen (#41098).
"""
if getattr(self, "_resize_recovery_pending", False):
return
now = time.monotonic()
@ -3487,6 +3502,24 @@ class HermesCLI:
self._last_invalidate = now
self._app.invalidate()
def _paint_now(self) -> None:
"""Immediate, unthrottled repaint for user-blocking modal prompts.
Background-thread callbacks (approval / clarify / sudo) set their modal
state then call this to make the panel visible at once. It deliberately
bypasses the ``_invalidate`` throttle and resize-recovery guard — a
modal the user is actively waiting on must never be dropped — mirroring
the direct ``event.app.invalidate()`` the modal key-binding handlers
already use. See ``_invalidate`` for why the throttle must not gate
these paints (#41098).
"""
app = getattr(self, "_app", None)
if app is not None:
try:
app.invalidate()
except Exception:
pass
def _force_full_redraw(self) -> None:
"""Force a clean full-screen repaint of the prompt_toolkit UI.
@ -11801,18 +11834,15 @@ class HermesCLI:
# Open-ended questions skip straight to freetext input
self._clarify_freetext = is_open_ended
# Trigger prompt_toolkit repaint from this (non-main) thread
self._invalidate()
# Trigger an immediate prompt_toolkit repaint from this (non-main)
# thread. Modal prompts must paint at once and must not be gated by the
# _invalidate throttle / resize guard — see _paint_now / _invalidate (#41098).
self._paint_now()
# Poll for the user's response. The countdown in the hint line
# updates on each invalidate — but frequent repaints cause visible
# flicker in some terminals (Kitty, ghostty). We only refresh the
# countdown every 5 s; selection changes (↑/↓) trigger instant
# Poll for the user's response. The countdown in the hint line
# updates on each invalidate — but frequent repaints cause visible
# flicker in some terminals (Kitty, ghostty). We only refresh the
# countdown every 5 s; selection changes (↑/↓) trigger instant
# repaints via the key bindings.
# Poll for the user's response. The countdown in the hint line updates
# on each repaint; refresh it once a second so the timer stays visible
# while we wait. Selection changes (↑/↓) trigger instant repaints via
# the key bindings.
_last_countdown_refresh = _time.monotonic()
while True:
try:
@ -11823,20 +11853,16 @@ class HermesCLI:
remaining = self._clarify_deadline - _time.monotonic()
if remaining <= 0:
break
# Only repaint every 5 s for the countdown — avoids flicker
now = _time.monotonic()
if now - _last_countdown_refresh >= 5.0:
if now - _last_countdown_refresh >= 1.0:
_last_countdown_refresh = now
self._invalidate()
if now - _last_countdown_refresh >= 5.0:
_last_countdown_refresh = now
self._invalidate()
self._paint_now()
# Timed out — tear down the UI and let the agent decide
self._clarify_state = None
self._clarify_freetext = False
self._clarify_deadline = 0
self._invalidate()
self._paint_now()
_cprint(f"\n{_DIM}(clarify timed out after {timeout}s — agent will decide){_RST}")
return (
"The user did not provide a response within the time limit. "
@ -11862,7 +11888,9 @@ class HermesCLI:
}
self._sudo_deadline = _time.monotonic() + timeout
self._invalidate()
# Modal prompt — paint immediately, bypassing the throttle/resize guard
# so the prompt can't be dropped and time out unseen (#41098).
self._paint_now()
while True:
try:
@ -11870,7 +11898,7 @@ class HermesCLI:
self._sudo_state = None
self._sudo_deadline = 0
self._restore_modal_input_snapshot()
self._invalidate()
self._paint_now()
if result:
_cprint(f"\n{_DIM} ✓ Password received (cached for session){_RST}")
else:
@ -11880,12 +11908,12 @@ class HermesCLI:
remaining = self._sudo_deadline - _time.monotonic()
if remaining <= 0:
break
self._invalidate()
self._paint_now()
self._sudo_state = None
self._sudo_deadline = 0
self._restore_modal_input_snapshot()
self._invalidate()
self._paint_now()
_cprint(f"\n{_DIM} ⏱ Timeout — continuing without sudo{_RST}")
return ""
@ -11919,7 +11947,12 @@ class HermesCLI:
}
self._approval_deadline = _time.monotonic() + timeout
self._invalidate()
# Modal prompt — paint immediately, bypassing the throttle/resize
# guard. A throttled paint here can be silently dropped (250ms
# window collision or in-flight resize), leaving the panel unseen so
# the command is denied on timeout without the user ever seeing it
# (#41098). The countdown refreshes below paint the same way.
self._paint_now()
_last_countdown_refresh = _time.monotonic()
while True:
@ -11927,20 +11960,20 @@ class HermesCLI:
result = response_queue.get(timeout=1)
self._approval_state = None
self._approval_deadline = 0
self._invalidate()
self._paint_now()
return result
except queue.Empty:
remaining = self._approval_deadline - _time.monotonic()
if remaining <= 0:
break
now = _time.monotonic()
if now - _last_countdown_refresh >= 5.0:
if now - _last_countdown_refresh >= 1.0:
_last_countdown_refresh = now
self._invalidate()
self._paint_now()
self._approval_state = None
self._approval_deadline = 0
self._invalidate()
self._paint_now()
_cprint(f"\n{_DIM} ⏱ Timeout — denying command{_RST}")
return "deny"
@ -12198,7 +12231,9 @@ class HermesCLI:
self._secret_state["response_queue"].put(value)
self._secret_state = None
self._secret_deadline = 0
self._invalidate()
# Modal teardown — paint directly so the secret panel clears at once and
# isn't held by the _invalidate throttle/resize guard (#41098).
self._paint_now()
def _cancel_secret_capture(self) -> None:
self._submit_secret_response("")

117
tests/cli/test_cli_approval_ui.py
View file

@ -339,6 +339,123 @@ class TestCliApprovalUi:
assert not cli._background_tasks
def _make_real_paint_cli_stub():
"""A stub whose modal repaint path runs the REAL _paint_now / _invalidate.
Both gates are set adversarially: _resize_recovery_pending=True and a recent
_last_invalidate inside the throttle window. A throttled _invalidate() would
be dropped under these conditions — _paint_now must paint regardless.
"""
cli = HermesCLI.__new__(HermesCLI)
cli._approval_state = None
cli._approval_deadline = 0
cli._approval_lock = threading.Lock()
cli._sudo_state = None
cli._sudo_deadline = 0
cli._clarify_state = None
cli._clarify_freetext = False
cli._clarify_deadline = 0
cli._modal_input_snapshot = None
# Real methods, not mocks.
cli._paint_now = HermesCLI._paint_now.__get__(cli, HermesCLI)
cli._invalidate = HermesCLI._invalidate.__get__(cli, HermesCLI)
cli._resize_recovery_pending = True # gate 1: resize in flight
cli._last_invalidate = time.monotonic() # gate 2: inside throttle window
cli._app = SimpleNamespace(invalidate=MagicMock(), current_buffer=_FakeBuffer())
return cli
class TestModalPaintNow:
"""Regression for #41098 — modal prompts must paint immediately.
The dangerous-command approval, clarify, and sudo prompts run their wait
loop on a background thread, set modal state a ConditionalContainer reads,
then must repaint so the panel becomes visible. They used the throttled
_invalidate(), whose paint is silently dropped on a 250ms window collision
or while a resize is pending — so the prompt timed out unseen. They now use
_paint_now(), which paints directly like the modal key-binding handlers.
"""
def test_paint_now_bypasses_throttle_and_resize_guard(self):
cli = _make_real_paint_cli_stub()
# A bare _invalidate() is suppressed under both gates...
cli._invalidate()
assert not cli._app.invalidate.called
# ...but _paint_now() always paints.
cli._paint_now()
assert cli._app.invalidate.called
def test_paint_now_no_app_is_safe(self):
cli = HermesCLI.__new__(HermesCLI)
cli._app = None
cli._paint_now() # must not raise
def _drive(self, cli, target, state_attr):
result = {}
def _run():
result["value"] = target()
with patch.object(cli_module, "_cprint"):
thread = threading.Thread(target=_run, daemon=True)
thread.start()
deadline = time.time() + 2
while getattr(cli, state_attr) is None and time.time() < deadline:
time.sleep(0.01)
assert getattr(cli, state_attr) is not None
assert cli._app.invalidate.called, (
f"{state_attr} panel was not painted despite throttle + resize gates"
)
# Reset so we can prove the response-received teardown also repaints
# (the panel must clear at once, not be held by the throttle).
cli._app.invalidate.reset_mock()
getattr(cli, state_attr)["response_queue"].put(
"deny" if state_attr == "_approval_state" else
("a" if state_attr == "_clarify_state" else "pw")
)
thread.join(timeout=2)
# clarify returns immediately on a response (no teardown repaint);
# approval and sudo repaint to tear the panel down.
if state_attr != "_clarify_state":
assert cli._app.invalidate.called, (
f"{state_attr} panel was not repainted on teardown"
)
assert not thread.is_alive()
return result["value"]
def test_approval_prompt_paints_under_both_gates(self):
cli = _make_real_paint_cli_stub()
value = self._drive(
cli, lambda: cli._approval_callback("rm -rf /tmp/scratch", "danger"),
"_approval_state",
)
assert value == "deny"
def test_clarify_prompt_paints_under_both_gates(self):
cli = _make_real_paint_cli_stub()
value = self._drive(
cli, lambda: cli._clarify_callback("Pick one", ["a", "b"]),
"_clarify_state",
)
assert value == "a"
def test_sudo_prompt_paints_under_both_gates(self):
cli = _make_real_paint_cli_stub()
value = self._drive(cli, cli._sudo_password_callback, "_sudo_state")
assert value == "pw"
def test_secret_response_teardown_paints(self):
"""_submit_secret_response tears the secret panel down via _paint_now,
so the panel clears immediately rather than being held by the throttle."""
cli = _make_real_paint_cli_stub()
cli._secret_state = {"response_queue": queue.Queue()}
cli._secret_deadline = 0
cli._submit_secret_response("hunter2")
assert cli._secret_state is None
assert cli._app.invalidate.called
assert cli._secret_state is None # cleared
class TestApprovalCallbackThreadLocalWiring:
"""Regression guard for the thread-local callback freeze (#13617 / #13618).