diff --git a/cli.py b/cli.py index fc426a28d..0a5f8118b 100644 --- a/cli.py +++ b/cli.py @@ -7411,7 +7411,15 @@ class HermesCLI: self._invalidate() def _get_approval_display_fragments(self): - """Render the dangerous-command approval panel for the prompt_toolkit UI.""" + """Render the dangerous-command approval panel for the prompt_toolkit UI. + + Layout priority: title + command + choices must always render, even if + the terminal is short or the description is long. Description is placed + at the bottom of the panel and gets truncated to fit the remaining row + budget. This prevents HSplit from clipping approve/deny off-screen when + tirith findings produce multi-paragraph descriptions or when the user + runs in a compact terminal pane. + """ state = self._approval_state if not state: return [] @@ -7470,22 +7478,89 @@ class HermesCLI: box_width = _panel_box_width(title, preview_lines) inner_text_width = max(8, box_width - 2) + # Pre-wrap the mandatory content — command + choices must always render. + cmd_wrapped = _wrap_panel_text(cmd_display, inner_text_width) + + # (choice_index, wrapped_line) so we can re-apply selected styling below + choice_wrapped: list[tuple[int, str]] = [] + for i, choice in enumerate(choices): + label = choice_labels.get(choice, choice) + prefix = '❯ ' if i == selected else ' ' + for wrapped in _wrap_panel_text(f"{prefix}{label}", inner_text_width, subsequent_indent=" "): + choice_wrapped.append((i, wrapped)) + + # Budget vertical space so HSplit never clips the command or choices. + # Panel chrome (full layout with separators): + # top border + title + blank_after_title + # + blank_between_cmd_choices + bottom border = 5 rows. + # In tight terminals we collapse to: + # top border + title + bottom border = 3 rows (no blanks). + # + # reserved_below: rows consumed below the approval panel by the + # spinner/tool-progress line, status bar, input area, separators, and + # prompt symbol. Measured at ~6 rows during live PTY approval prompts; + # budget 6 so we don't overestimate the panel's room. + term_rows = shutil.get_terminal_size((100, 24)).lines + chrome_full = 5 + chrome_tight = 3 + reserved_below = 6 + + available = max(0, term_rows - reserved_below) + mandatory_full = chrome_full + len(cmd_wrapped) + len(choice_wrapped) + + # If the full-chrome panel doesn't fit, drop the separator blanks. + # This keeps the command and every choice on-screen in compact terminals. + use_compact_chrome = mandatory_full > available + chrome_rows = chrome_tight if use_compact_chrome else chrome_full + + # If the command itself is too long to leave room for choices (e.g. user + # hit "view" on a multi-hundred-character command), truncate it so the + # approve/deny buttons still render. Keep at least 1 row of command. + max_cmd_rows = max(1, available - chrome_rows - len(choice_wrapped)) + if len(cmd_wrapped) > max_cmd_rows: + keep = max(1, max_cmd_rows - 1) if max_cmd_rows > 1 else 1 + cmd_wrapped = cmd_wrapped[:keep] + ["… (command truncated — use /logs or /debug for full text)"] + + # Allocate any remaining rows to description. The extra -1 in full mode + # accounts for the blank separator between choices and description. + mandatory_no_desc = chrome_rows + len(cmd_wrapped) + len(choice_wrapped) + desc_sep_cost = 0 if use_compact_chrome else 1 + available_for_desc = available - mandatory_no_desc - desc_sep_cost + # Even on huge terminals, cap description height so the panel stays compact. + available_for_desc = max(0, min(available_for_desc, 10)) + + desc_wrapped = _wrap_panel_text(description, inner_text_width) if description else [] + if available_for_desc < 1 or not desc_wrapped: + desc_wrapped = [] + elif len(desc_wrapped) > available_for_desc: + keep = max(1, available_for_desc - 1) + desc_wrapped = desc_wrapped[:keep] + ["… (description truncated)"] + + # Render: title → command → choices → description (description last so + # any remaining overflow clips from the bottom of the least-critical + # content, never from the command or choices). Use compact chrome (no + # blank separators) when the terminal is tight. lines = [] lines.append(('class:approval-border', '╭' + ('─' * box_width) + '╮\n')) _append_panel_line(lines, 'class:approval-border', 'class:approval-title', title, box_width) - _append_blank_panel_line(lines, 'class:approval-border', box_width) - for wrapped in _wrap_panel_text(description, inner_text_width): - _append_panel_line(lines, 'class:approval-border', 'class:approval-desc', wrapped, box_width) - for wrapped in _wrap_panel_text(cmd_display, inner_text_width): + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:approval-border', box_width) + + for wrapped in cmd_wrapped: _append_panel_line(lines, 'class:approval-border', 'class:approval-cmd', wrapped, box_width) - _append_blank_panel_line(lines, 'class:approval-border', box_width) - for i, choice in enumerate(choices): - label = choice_labels.get(choice, choice) + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:approval-border', box_width) + + for i, wrapped in choice_wrapped: style = 'class:approval-selected' if i == selected else 'class:approval-choice' - prefix = '❯ ' if i == selected else ' ' - for wrapped in _wrap_panel_text(f"{prefix}{label}", inner_text_width, subsequent_indent=" "): - _append_panel_line(lines, 'class:approval-border', style, wrapped, box_width) - _append_blank_panel_line(lines, 'class:approval-border', box_width) + _append_panel_line(lines, 'class:approval-border', style, wrapped, box_width) + + if desc_wrapped: + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:approval-border', box_width) + for wrapped in desc_wrapped: + _append_panel_line(lines, 'class:approval-border', 'class:approval-desc', wrapped, box_width) + lines.append(('class:approval-border', '╰' + ('─' * box_width) + '╯\n')) return lines @@ -9137,7 +9212,13 @@ class HermesCLI: lines.append((border_style, "│" + (" " * box_width) + "│\n")) def _get_clarify_display(): - """Build styled text for the clarify question/choices panel.""" + """Build styled text for the clarify question/choices panel. + + Layout priority: choices + Other option must always render even if + the question is very long. The question is budgeted to leave enough + rows for the choices and trailing chrome; anything over the budget + is truncated with a marker. + """ state = cli_ref._clarify_state if not state: return [] @@ -9158,48 +9239,97 @@ class HermesCLI: box_width = _panel_box_width("Hermes needs your input", preview_lines) inner_text_width = max(8, box_width - 2) + # Pre-wrap choices + Other option — these are mandatory. + choice_wrapped: list[tuple[int, str]] = [] + if choices: + for i, choice in enumerate(choices): + prefix = '❯ ' if i == selected and not cli_ref._clarify_freetext else ' ' + for wrapped in _wrap_panel_text(f"{prefix}{choice}", inner_text_width, subsequent_indent=" "): + choice_wrapped.append((i, wrapped)) + # Trailing Other row(s) + other_idx = len(choices) + if selected == other_idx and not cli_ref._clarify_freetext: + other_label_mand = '❯ Other (type your answer)' + elif cli_ref._clarify_freetext: + other_label_mand = '❯ Other (type below)' + else: + other_label_mand = ' Other (type your answer)' + other_wrapped = _wrap_panel_text(other_label_mand, inner_text_width, subsequent_indent=" ") + elif cli_ref._clarify_freetext: + # Freetext-only mode: the guidance line takes the place of choices. + other_wrapped = _wrap_panel_text( + "Type your answer in the prompt below, then press Enter.", + inner_text_width, + ) + else: + other_wrapped = [] + + # Budget the question so mandatory rows always render. + # Chrome layouts: + # full : top border + blank_after_title + blank_after_question + # + blank_before_bottom + bottom border = 5 rows + # tight: top border + bottom border = 2 rows (drop all blanks) + # + # reserved_below matches the approval-panel budget (~6 rows for + # spinner/tool-progress + status + input + separators + prompt). + term_rows = shutil.get_terminal_size((100, 24)).lines + chrome_full = 5 + chrome_tight = 2 + reserved_below = 6 + + available = max(0, term_rows - reserved_below) + mandatory_full = chrome_full + len(choice_wrapped) + len(other_wrapped) + + use_compact_chrome = mandatory_full > available + chrome_rows = chrome_tight if use_compact_chrome else chrome_full + + max_question_rows = max(1, available - chrome_rows - len(choice_wrapped) - len(other_wrapped)) + max_question_rows = min(max_question_rows, 12) # soft cap on huge terminals + + question_wrapped = _wrap_panel_text(question, inner_text_width) + if len(question_wrapped) > max_question_rows: + keep = max(1, max_question_rows - 1) + question_wrapped = question_wrapped[:keep] + ["… (question truncated)"] + lines = [] # Box top border lines.append(('class:clarify-border', '╭─ ')) lines.append(('class:clarify-title', 'Hermes needs your input')) lines.append(('class:clarify-border', ' ' + ('─' * max(0, box_width - len("Hermes needs your input") - 3)) + '╮\n')) - _append_blank_panel_line(lines, 'class:clarify-border', box_width) + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:clarify-border', box_width) - # Question text - for wrapped in _wrap_panel_text(question, inner_text_width): + # Question text (bounded) + for wrapped in question_wrapped: _append_panel_line(lines, 'class:clarify-border', 'class:clarify-question', wrapped, box_width) - _append_blank_panel_line(lines, 'class:clarify-border', box_width) + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:clarify-border', box_width) if cli_ref._clarify_freetext and not choices: - guidance = "Type your answer in the prompt below, then press Enter." - for wrapped in _wrap_panel_text(guidance, inner_text_width): + for wrapped in other_wrapped: _append_panel_line(lines, 'class:clarify-border', 'class:clarify-choice', wrapped, box_width) - _append_blank_panel_line(lines, 'class:clarify-border', box_width) + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:clarify-border', box_width) if choices: # Multiple-choice mode: show selectable options - for i, choice in enumerate(choices): + for i, wrapped in choice_wrapped: style = 'class:clarify-selected' if i == selected and not cli_ref._clarify_freetext else 'class:clarify-choice' - prefix = '❯ ' if i == selected and not cli_ref._clarify_freetext else ' ' - wrapped_lines = _wrap_panel_text(f"{prefix}{choice}", inner_text_width, subsequent_indent=" ") - for wrapped in wrapped_lines: - _append_panel_line(lines, 'class:clarify-border', style, wrapped, box_width) + _append_panel_line(lines, 'class:clarify-border', style, wrapped, box_width) - # "Other" option (5th line, only shown when choices exist) + # "Other" option (trailing row(s), only shown when choices exist) other_idx = len(choices) if selected == other_idx and not cli_ref._clarify_freetext: other_style = 'class:clarify-selected' - other_label = '❯ Other (type your answer)' elif cli_ref._clarify_freetext: other_style = 'class:clarify-active-other' - other_label = '❯ Other (type below)' else: other_style = 'class:clarify-choice' - other_label = ' Other (type your answer)' - for wrapped in _wrap_panel_text(other_label, inner_text_width, subsequent_indent=" "): + for wrapped in other_wrapped: _append_panel_line(lines, 'class:clarify-border', other_style, wrapped, box_width) - _append_blank_panel_line(lines, 'class:clarify-border', box_width) + if not use_compact_chrome: + _append_blank_panel_line(lines, 'class:clarify-border', box_width) lines.append(('class:clarify-border', '╰' + ('─' * box_width) + '╯\n')) return lines diff --git a/tests/cli/test_cli_approval_ui.py b/tests/cli/test_cli_approval_ui.py index 63e03b9ab..205e31608 100644 --- a/tests/cli/test_cli_approval_ui.py +++ b/tests/cli/test_cli_approval_ui.py @@ -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