mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-04-28 06:51:16 +08:00
fix: CLI/UX batch — ChatConsole errors, curses scroll, skin-aware banner, git state banner (#5974)
* fix(cli): route error messages through ChatConsole inside patch_stdout Cherry-pick of PR #5798 by @icn5381. Replace self.console.print() with ChatConsole().print() for 11 error/status messages reachable during the interactive session. Inside patch_stdout, self.console (plain Rich Console) writes raw ANSI escapes that StdoutProxy mangles into garbled text. ChatConsole uses prompt_toolkit's native print_formatted_text which renders correctly. Same class of bug as #2262 — that fix covered agent output but missed these error paths in _ensure_runtime_credentials, _init_agent, quick commands, skill loading, and plan mode. * fix(model-picker): add scrolling viewport to curses provider menu Cherry-pick of PR #5790 by @Lempkey. Fixes #5755. _curses_prompt_choice rendered items starting unconditionally from index 0 with no scroll offset. The 'More providers' submenu has 13 entries. On terminals shorter than ~16 rows, items past the fold were never drawn. When UP-arrow wrapped cursor from 0 to the last item (Cancel, index 12), the highlight rendered off-screen — appearing as if only Cancel existed. Adds scroll_offset tracking that adjusts each frame to keep the cursor inside the visible window. * feat(cli): skin-aware compact banner + git state in startup banner Combined salvage of PR #5922 by @ASRagab and PR #5877 by @xinbenlv. Compact banner changes (from #5922): - Read active skin colors and branding instead of hardcoding gold/NOUS HERMES - Default skin preserves backward-compatible legacy branding - Non-default skins use their own agent_name and colors Git state in banner (from #5877): - New format_banner_version_label() shows upstream/local git hashes - Full banner title now includes git state (upstream hash, carried commits) - Compact banner line2 shows the version label with git state - Widen compact banner max width from 64 to 88 to fit version info Both the full Rich banner and compact fallback are now skin-aware and show git state.
This commit is contained in:
63
tests/hermes_cli/test_banner_git_state.py
Normal file
63
tests/hermes_cli/test_banner_git_state.py
Normal file
@@ -0,0 +1,63 @@
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
|
||||
def test_format_banner_version_label_without_git_state():
|
||||
from hermes_cli import banner
|
||||
|
||||
with patch.object(banner, "get_git_banner_state", return_value=None):
|
||||
value = banner.format_banner_version_label()
|
||||
|
||||
assert value == f"Hermes Agent v{banner.VERSION} ({banner.RELEASE_DATE})"
|
||||
|
||||
|
||||
def test_format_banner_version_label_on_upstream_main():
|
||||
from hermes_cli import banner
|
||||
|
||||
with patch.object(
|
||||
banner,
|
||||
"get_git_banner_state",
|
||||
return_value={"upstream": "b2f477a3", "local": "b2f477a3", "ahead": 0},
|
||||
):
|
||||
value = banner.format_banner_version_label()
|
||||
|
||||
assert value.endswith("· upstream b2f477a3")
|
||||
assert "local" not in value
|
||||
|
||||
|
||||
def test_format_banner_version_label_with_carried_commits():
|
||||
from hermes_cli import banner
|
||||
|
||||
with patch.object(
|
||||
banner,
|
||||
"get_git_banner_state",
|
||||
return_value={"upstream": "b2f477a3", "local": "af8aad31", "ahead": 3},
|
||||
):
|
||||
value = banner.format_banner_version_label()
|
||||
|
||||
assert "upstream b2f477a3" in value
|
||||
assert "local af8aad31" in value
|
||||
assert "+3 carried commits" in value
|
||||
|
||||
|
||||
def test_get_git_banner_state_reads_origin_and_head(tmp_path):
|
||||
from hermes_cli import banner
|
||||
|
||||
repo_dir = tmp_path / "repo"
|
||||
(repo_dir / ".git").mkdir(parents=True)
|
||||
|
||||
results = {
|
||||
("git", "rev-parse", "--short=8", "origin/main"): MagicMock(returncode=0, stdout="b2f477a3\n"),
|
||||
("git", "rev-parse", "--short=8", "HEAD"): MagicMock(returncode=0, stdout="af8aad31\n"),
|
||||
("git", "rev-list", "--count", "origin/main..HEAD"): MagicMock(returncode=0, stdout="3\n"),
|
||||
}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
key = tuple(cmd)
|
||||
if key not in results:
|
||||
raise AssertionError(f"unexpected command: {cmd}")
|
||||
return results[key]
|
||||
|
||||
with patch("hermes_cli.banner.subprocess.run", side_effect=fake_run):
|
||||
state = banner.get_git_banner_state(repo_dir)
|
||||
|
||||
assert state == {"upstream": "b2f477a3", "local": "af8aad31", "ahead": 3}
|
||||
140
tests/test_cli_skin_integration.py
Normal file
140
tests/test_cli_skin_integration.py
Normal file
@@ -0,0 +1,140 @@
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from cli import HermesCLI, _build_compact_banner, _rich_text_from_ansi
|
||||
from hermes_cli.skin_engine import get_active_skin, set_active_skin
|
||||
|
||||
|
||||
def _make_cli_stub():
|
||||
cli = HermesCLI.__new__(HermesCLI)
|
||||
cli._sudo_state = None
|
||||
cli._secret_state = None
|
||||
cli._approval_state = None
|
||||
cli._clarify_state = None
|
||||
cli._clarify_freetext = False
|
||||
cli._command_running = False
|
||||
cli._agent_running = False
|
||||
cli._voice_recording = False
|
||||
cli._voice_processing = False
|
||||
cli._voice_mode = False
|
||||
cli._command_spinner_frame = lambda: "⟳"
|
||||
cli._tui_style_base = {
|
||||
"prompt": "#fff",
|
||||
"input-area": "#fff",
|
||||
"input-rule": "#aaa",
|
||||
"prompt-working": "#888 italic",
|
||||
}
|
||||
cli._app = SimpleNamespace(style=None)
|
||||
cli._invalidate = MagicMock()
|
||||
return cli
|
||||
|
||||
|
||||
class TestCliSkinPromptIntegration:
|
||||
def test_default_prompt_fragments_use_default_symbol(self):
|
||||
cli = _make_cli_stub()
|
||||
|
||||
set_active_skin("default")
|
||||
assert cli._get_tui_prompt_fragments() == [("class:prompt", "❯ ")]
|
||||
|
||||
def test_ares_prompt_fragments_use_skin_symbol(self):
|
||||
cli = _make_cli_stub()
|
||||
|
||||
set_active_skin("ares")
|
||||
assert cli._get_tui_prompt_fragments() == [("class:prompt", "⚔ ❯ ")]
|
||||
|
||||
def test_secret_prompt_fragments_preserve_secret_state(self):
|
||||
cli = _make_cli_stub()
|
||||
cli._secret_state = {"response_queue": object()}
|
||||
|
||||
set_active_skin("ares")
|
||||
assert cli._get_tui_prompt_fragments() == [("class:sudo-prompt", "🔑 ❯ ")]
|
||||
|
||||
def test_icon_only_skin_symbol_still_visible_in_special_states(self):
|
||||
cli = _make_cli_stub()
|
||||
cli._secret_state = {"response_queue": object()}
|
||||
|
||||
with patch("hermes_cli.skin_engine.get_active_prompt_symbol", return_value="⚔ "):
|
||||
assert cli._get_tui_prompt_fragments() == [("class:sudo-prompt", "🔑 ⚔ ")]
|
||||
|
||||
def test_build_tui_style_dict_uses_skin_overrides(self):
|
||||
cli = _make_cli_stub()
|
||||
|
||||
set_active_skin("ares")
|
||||
skin = get_active_skin()
|
||||
style_dict = cli._build_tui_style_dict()
|
||||
|
||||
assert style_dict["prompt"] == skin.get_color("prompt")
|
||||
assert style_dict["input-rule"] == skin.get_color("input_rule")
|
||||
assert style_dict["prompt-working"] == f"{skin.get_color('banner_dim')} italic"
|
||||
assert style_dict["approval-title"] == f"{skin.get_color('ui_warn')} bold"
|
||||
|
||||
def test_apply_tui_skin_style_updates_running_app(self):
|
||||
cli = _make_cli_stub()
|
||||
|
||||
set_active_skin("ares")
|
||||
assert cli._apply_tui_skin_style() is True
|
||||
assert cli._app.style is not None
|
||||
cli._invalidate.assert_called_once_with(min_interval=0.0)
|
||||
|
||||
def test_handle_skin_command_refreshes_live_tui(self, capsys):
|
||||
cli = _make_cli_stub()
|
||||
|
||||
with patch("cli.save_config_value", return_value=True):
|
||||
cli._handle_skin_command("/skin ares")
|
||||
|
||||
output = capsys.readouterr().out
|
||||
assert "Skin set to: ares (saved)" in output
|
||||
assert "Prompt + TUI colors updated." in output
|
||||
assert cli._app.style is not None
|
||||
|
||||
|
||||
class TestCompactBannerSkinIntegration:
|
||||
def test_default_compact_banner_keeps_legacy_nous_hermes_branding(self):
|
||||
set_active_skin("default")
|
||||
|
||||
with patch("cli.shutil.get_terminal_size", return_value=SimpleNamespace(columns=90)), \
|
||||
patch("cli.format_banner_version_label", return_value="Hermes Agent v0.1.0 (test)"):
|
||||
banner = _build_compact_banner()
|
||||
|
||||
assert "NOUS HERMES" in banner
|
||||
|
||||
def test_poseidon_compact_banner_uses_skin_branding_instead_of_nous_hermes(self):
|
||||
set_active_skin("poseidon")
|
||||
|
||||
with patch("cli.shutil.get_terminal_size", return_value=SimpleNamespace(columns=90)), \
|
||||
patch("cli.format_banner_version_label", return_value="Hermes Agent v0.1.0 (test)"):
|
||||
banner = _build_compact_banner()
|
||||
|
||||
assert "Poseidon Agent" in banner
|
||||
assert "NOUS HERMES" not in banner
|
||||
|
||||
def test_poseidon_compact_banner_uses_skin_colors(self):
|
||||
set_active_skin("poseidon")
|
||||
skin = get_active_skin()
|
||||
|
||||
with patch("cli.shutil.get_terminal_size", return_value=SimpleNamespace(columns=90)), \
|
||||
patch("cli.format_banner_version_label", return_value="Hermes Agent v0.1.0 (test)"):
|
||||
banner = _build_compact_banner()
|
||||
|
||||
assert skin.get_color("banner_border") in banner
|
||||
assert skin.get_color("banner_title") in banner
|
||||
assert skin.get_color("banner_dim") in banner
|
||||
|
||||
def test_compact_banner_shows_version_label(self):
|
||||
set_active_skin("default")
|
||||
|
||||
with patch("cli.shutil.get_terminal_size", return_value=SimpleNamespace(columns=90)), \
|
||||
patch("cli.format_banner_version_label", return_value="Hermes Agent v1.0 (test) · upstream abc12345"):
|
||||
banner = _build_compact_banner()
|
||||
|
||||
assert "upstream abc12345" in banner
|
||||
|
||||
|
||||
class TestAnsiRichTextHelper:
|
||||
def test_preserves_literal_brackets(self):
|
||||
text = _rich_text_from_ansi("[notatag] literal")
|
||||
assert text.plain == "[notatag] literal"
|
||||
|
||||
def test_strips_ansi_but_keeps_plain_text(self):
|
||||
text = _rich_text_from_ansi("\x1b[31mred\x1b[0m")
|
||||
assert text.plain == "red"
|
||||
118
tests/test_model_picker_scroll.py
Normal file
118
tests/test_model_picker_scroll.py
Normal file
@@ -0,0 +1,118 @@
|
||||
"""Tests for the scrolling viewport logic in _curses_prompt_choice (issue #5755).
|
||||
|
||||
The "More providers" submenu has 13 entries (11 extended + custom + cancel).
|
||||
Before the fix, _curses_prompt_choice rendered items starting unconditionally
|
||||
from index 0 with no scroll offset. On terminals shorter than ~16 rows, items
|
||||
near the bottom were never drawn. When the cursor wrapped from 0 to the last
|
||||
item (Cancel) via UP-arrow, the highlight rendered off-screen, leaving the menu
|
||||
looking like only "Cancel" existed.
|
||||
|
||||
The fix adds a scroll_offset that tracks the cursor so the highlighted item
|
||||
is always within the visible window. These tests exercise that logic in
|
||||
isolation without requiring a real TTY.
|
||||
"""
|
||||
|
||||
import sys
|
||||
import os
|
||||
import pytest
|
||||
|
||||
sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Pure scroll-offset logic extracted from _curses_menu for unit testing
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _compute_scroll_offset(cursor: int, scroll_offset: int, visible: int, n_choices: int) -> int:
|
||||
"""Mirror of the scroll adjustment block inside _curses_menu."""
|
||||
if cursor < scroll_offset:
|
||||
scroll_offset = cursor
|
||||
elif cursor >= scroll_offset + visible:
|
||||
scroll_offset = cursor - visible + 1
|
||||
scroll_offset = max(0, min(scroll_offset, max(0, n_choices - visible)))
|
||||
return scroll_offset
|
||||
|
||||
|
||||
def _visible_indices(cursor: int, scroll_offset: int, visible: int, n_choices: int):
|
||||
"""Return the list indices that would be rendered for the given state."""
|
||||
scroll_offset = _compute_scroll_offset(cursor, scroll_offset, visible, n_choices)
|
||||
return list(range(scroll_offset, min(scroll_offset + visible, n_choices)))
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Tests: scroll offset calculation
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestScrollOffsetLogic:
|
||||
N = 13 # typical extended-providers list length
|
||||
|
||||
def test_cursor_at_zero_no_scroll(self):
|
||||
"""Start position: offset stays 0, first items visible."""
|
||||
assert _compute_scroll_offset(0, 0, 8, self.N) == 0
|
||||
|
||||
def test_cursor_within_window_unchanged(self):
|
||||
"""Cursor inside the current window: offset unchanged."""
|
||||
assert _compute_scroll_offset(5, 0, 8, self.N) == 0
|
||||
|
||||
def test_cursor_at_last_item_scrolls_down(self):
|
||||
"""Cursor on Cancel (index 12) with 8-row window: offset = 12 - 8 + 1 = 5."""
|
||||
offset = _compute_scroll_offset(12, 0, 8, self.N)
|
||||
assert offset == 5
|
||||
assert 12 in _visible_indices(12, 0, 8, self.N)
|
||||
|
||||
def test_cursor_wraps_to_cancel_via_up(self):
|
||||
"""UP from index 0 wraps to last item; last item must be visible."""
|
||||
wrapped_cursor = (0 - 1) % self.N # == 12
|
||||
indices = _visible_indices(wrapped_cursor, 0, 8, self.N)
|
||||
assert wrapped_cursor in indices
|
||||
|
||||
def test_cursor_above_window_scrolls_up(self):
|
||||
"""Cursor above current window: offset tracks cursor."""
|
||||
# window currently shows [5..12], cursor moves to 3
|
||||
offset = _compute_scroll_offset(3, 5, 8, self.N)
|
||||
assert offset == 3
|
||||
assert 3 in _visible_indices(3, 5, 8, self.N)
|
||||
|
||||
def test_visible_window_never_exceeds_list(self):
|
||||
"""Offset is clamped so the window never starts past the list end."""
|
||||
offset = _compute_scroll_offset(12, 0, 20, self.N) # window larger than list
|
||||
assert offset == 0
|
||||
|
||||
def test_single_item_list(self):
|
||||
"""Edge case: one choice, cursor 0."""
|
||||
assert _compute_scroll_offset(0, 0, 8, 1) == 0
|
||||
|
||||
def test_list_fits_in_window_no_scroll_needed(self):
|
||||
"""If all choices fit in the visible window, offset is always 0."""
|
||||
for cursor in range(self.N):
|
||||
offset = _compute_scroll_offset(cursor, 0, 20, self.N)
|
||||
assert offset == 0, f"cursor={cursor} should not scroll when window > list"
|
||||
|
||||
def test_cursor_always_in_visible_range(self):
|
||||
"""Invariant: cursor is always within the rendered window after adjustment."""
|
||||
visible = 5
|
||||
for cursor in range(self.N):
|
||||
indices = _visible_indices(cursor, 0, visible, self.N)
|
||||
assert cursor in indices, f"cursor={cursor} not in visible={indices}"
|
||||
|
||||
def test_full_navigation_down_cursor_always_visible(self):
|
||||
"""Simulate pressing DOWN through all items; cursor always in view."""
|
||||
visible = 6
|
||||
scroll_offset = 0
|
||||
cursor = 0
|
||||
for _ in range(self.N + 2): # wrap around twice
|
||||
scroll_offset = _compute_scroll_offset(cursor, scroll_offset, visible, self.N)
|
||||
rendered = list(range(scroll_offset, min(scroll_offset + visible, self.N)))
|
||||
assert cursor in rendered, f"cursor={cursor} not in rendered={rendered}"
|
||||
cursor = (cursor + 1) % self.N
|
||||
|
||||
def test_full_navigation_up_cursor_always_visible(self):
|
||||
"""Simulate pressing UP through all items; cursor always in view."""
|
||||
visible = 6
|
||||
scroll_offset = 0
|
||||
cursor = 0
|
||||
for _ in range(self.N + 2):
|
||||
scroll_offset = _compute_scroll_offset(cursor, scroll_offset, visible, self.N)
|
||||
rendered = list(range(scroll_offset, min(scroll_offset + visible, self.N)))
|
||||
assert cursor in rendered, f"cursor={cursor} not in rendered={rendered}"
|
||||
cursor = (cursor - 1) % self.N
|
||||
Reference in New Issue
Block a user