mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-25 00:51:20 +00:00
fix(cli): stop approval panel from clipping approve/deny off-screen (#11260)
* fix(cli): stop approval panel from clipping approve/deny off-screen
The dangerous-command approval panel had an unbounded Window height with
choices at the bottom. When tirith findings produced long descriptions or
the terminal was compact, HSplit clipped the bottom of the widget — which
is exactly where approve/session/always/deny live. Users were asked to
decide on commands without being able to see the choices (and sometimes
the command itself was hidden too).
Fix: reorder the panel so title → command → choices render first, with
description last. Budget vertical rows so the mandatory content (command
and every choice) always fits, and truncate the description to whatever
row budget is left. Handle three edge cases:
- Long description in a normal terminal: description gets truncated at
the bottom with a '… (description truncated)' marker. Command and
all four choices always visible.
- Compact terminal (≤ ~14 rows): description dropped entirely. Command
and choices are the only content, no overflow.
- /view on a giant command: command gets truncated with a marker so
choices still render. Keeps at least 2 rows of command.
Same row-budgeting pattern applied to the clarify widget, which had the
identical structural bug (long question would push choices off-screen).
Adds regression tests covering all three scenarios.
* fix(cli): add compact chrome mode for approval/clarify panels on short terminals
Live PTY test at 100x14 rows revealed reserved_below=4 was too optimistic
— the spinner/tool-progress line, status bar, input area, separators, and
prompt symbol actually consume ~6 rows below the panel. At 14 rows, the
panel still got 'Deny' clipped off the bottom.
Fix: bump reserved_below to 6 (measured from live PTY output) and add a
compact-chrome mode that drops the blank separators between title/command
and command/choices when the full-chrome panel wouldn't fit. Chrome goes
from 5 rows to 3 rows in tight mode, keeping command + all 4 choices on
screen in terminals as small as ~13 rows.
Same compact-chrome pattern applied to the clarify widget.
Verified live in PTY hermes chat sessions at 100x14 (compact chrome
triggered, all choices visible) and 100x30 (full chrome with blanks, nice
spacing) by asking the agent to run 'rm -rf /tmp/sandbox'.
---------
Co-authored-by: Teknium <teknium@nousresearch.com>
This commit is contained in:
parent
edefec4e68
commit
59a5ff9cb2
2 changed files with 274 additions and 31 deletions
|
|
@ -141,3 +141,116 @@ class TestCliApprovalUi:
|
|||
assert "archive-" in rendered
|
||||
assert "keyring.gpg" in rendered
|
||||
assert "status=progress" in rendered
|
||||
|
||||
def test_approval_display_preserves_command_and_choices_with_long_description(self):
|
||||
"""Regression: long tirith descriptions used to push approve/deny off-screen.
|
||||
|
||||
The panel must always render the command and every choice, even when
|
||||
the description would otherwise wrap into 10+ lines. The description
|
||||
gets truncated with a marker instead.
|
||||
"""
|
||||
cli = _make_cli_stub()
|
||||
long_desc = (
|
||||
"Security scan — [CRITICAL] Destructive shell command with wildcard expansion: "
|
||||
"The command performs a recursive deletion of log files which may contain "
|
||||
"audit information relevant to active incident investigations, running services "
|
||||
"that rely on log files for state, rotated archives, and other system artifacts. "
|
||||
"Review whether this is intended before approving. Consider whether a targeted "
|
||||
"deletion with more specific filters would better match the intent."
|
||||
)
|
||||
cli._approval_state = {
|
||||
"command": "rm -rf /var/log/apache2/*.log",
|
||||
"description": long_desc,
|
||||
"choices": ["once", "session", "always", "deny"],
|
||||
"selected": 0,
|
||||
"response_queue": queue.Queue(),
|
||||
}
|
||||
|
||||
# Simulate a compact terminal where the old unbounded panel would overflow.
|
||||
import shutil as _shutil
|
||||
|
||||
with patch("cli.shutil.get_terminal_size",
|
||||
return_value=_shutil.os.terminal_size((100, 20))):
|
||||
fragments = cli._get_approval_display_fragments()
|
||||
|
||||
rendered = "".join(text for _style, text in fragments)
|
||||
|
||||
# Command must be fully visible (rm -rf /var/log/apache2/*.log is short).
|
||||
assert "rm -rf /var/log/apache2/*.log" in rendered
|
||||
|
||||
# Every choice must render — this is the core bug: approve/deny were
|
||||
# getting clipped off the bottom of the panel.
|
||||
assert "Allow once" in rendered
|
||||
assert "Allow for this session" in rendered
|
||||
assert "Add to permanent allowlist" in rendered
|
||||
assert "Deny" in rendered
|
||||
|
||||
# The bottom border must render (i.e. the panel is self-contained).
|
||||
assert rendered.rstrip().endswith("╯")
|
||||
|
||||
# The description gets truncated — marker should appear.
|
||||
assert "(description truncated)" in rendered
|
||||
|
||||
def test_approval_display_skips_description_on_very_short_terminal(self):
|
||||
"""On a 12-row terminal, only the command and choices have room.
|
||||
|
||||
The description is dropped entirely rather than partially shown, so the
|
||||
choices never get clipped.
|
||||
"""
|
||||
cli = _make_cli_stub()
|
||||
cli._approval_state = {
|
||||
"command": "rm -rf /var/log/apache2/*.log",
|
||||
"description": "recursive delete",
|
||||
"choices": ["once", "session", "always", "deny"],
|
||||
"selected": 0,
|
||||
"response_queue": queue.Queue(),
|
||||
}
|
||||
|
||||
import shutil as _shutil
|
||||
|
||||
with patch("cli.shutil.get_terminal_size",
|
||||
return_value=_shutil.os.terminal_size((100, 12))):
|
||||
fragments = cli._get_approval_display_fragments()
|
||||
|
||||
rendered = "".join(text for _style, text in fragments)
|
||||
|
||||
# Command visible.
|
||||
assert "rm -rf /var/log/apache2/*.log" in rendered
|
||||
# All four choices visible.
|
||||
for label in ("Allow once", "Allow for this session",
|
||||
"Add to permanent allowlist", "Deny"):
|
||||
assert label in rendered, f"choice {label!r} missing"
|
||||
|
||||
def test_approval_display_truncates_giant_command_in_view_mode(self):
|
||||
"""If the user hits /view on a massive command, choices still render.
|
||||
|
||||
The command gets truncated with a marker; the description gets dropped
|
||||
if there's no remaining row budget.
|
||||
"""
|
||||
cli = _make_cli_stub()
|
||||
# 50 lines of command when wrapped at ~64 chars.
|
||||
giant_cmd = "bash -c 'echo " + ("x" * 3000) + "'"
|
||||
cli._approval_state = {
|
||||
"command": giant_cmd,
|
||||
"description": "shell command via -c/-lc flag",
|
||||
"choices": ["once", "session", "always", "deny"],
|
||||
"selected": 0,
|
||||
"show_full": True,
|
||||
"response_queue": queue.Queue(),
|
||||
}
|
||||
|
||||
import shutil as _shutil
|
||||
|
||||
with patch("cli.shutil.get_terminal_size",
|
||||
return_value=_shutil.os.terminal_size((100, 24))):
|
||||
fragments = cli._get_approval_display_fragments()
|
||||
|
||||
rendered = "".join(text for _style, text in fragments)
|
||||
|
||||
# All four choices visible even with a huge command.
|
||||
for label in ("Allow once", "Allow for this session",
|
||||
"Add to permanent allowlist", "Deny"):
|
||||
assert label in rendered, f"choice {label!r} missing"
|
||||
|
||||
# Command got truncated with a marker.
|
||||
assert "(command truncated" in rendered
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue