Skip to content

fix(cli): decode arrow/nav escape sequences in remaining curses menus - #35786

Closed
kshitijk4poor wants to merge 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/curses-arrows-plugins-browser
Closed

fix(cli): decode arrow/nav escape sequences in remaining curses menus#35786
kshitijk4poor wants to merge 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/curses-arrows-plugins-browser

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

What

Follow-up to #35776. Two more interactive curses menus had their own raw stdscr.getch() loops with the same root cause: they matched only curses.KEY_* and treated a leading byte 27 (ESC) as cancel. On terminals that deliver cursor keys as raw CSI/SS3 byte sequences (Ghostty among them, where #35776's symptom was first reported), pressing ↑/↓ misfired:

  • hermes plugins manager (including the "Provider Plugins" group/category rows): arrows bailed out via the {27, q} save & exit branch — selecting a group was unusable.
  • Session browser (hermes browse): arrows cleared the filter / exited, and the trailing escape bytes ([A, [B, …) were injected into the type-to-filter search box as literal text.

Fix

Generalize the shared helper rather than duplicating decode logic across menus:

  • Add read_menu_key_ex() returning (action, raw_key), plus NAV_PAGE_UP / PAGE_DOWN / HOME / END / BACKSPACE. Decode the full CSI/SS3 vocabulary: arrows, PgUp/PgDn (5~/6~), Home/End (H/F and 1~/4~/7~/8~), modified arrows carrying parameters (e.g. [1;2B), and consume unhandled sequences (Delete 3~) up to their terminator so no bytes leak.
  • An ESC immediately followed by a non-introducer byte (Alt-combo, fast typing, paste) now registers as a lone-ESC cancel and pushes the trailing byte back via curses.ungetch instead of swallowing it.
  • letters_are_nav=False mode for the session browser: j/k/space/q stay typeable filter characters, while real arrows (translated and raw escape), Enter, Backspace and Esc still navigate.

Route plugins_cmd.py and main.py's session browser through the helper. read_menu_key() is now a thin wrapper over read_menu_key_ex().

Behavior is a strict superset

Verified case-by-case — nothing that worked before changed:

  • Plugins/setup menus (letters_are_nav=True): translated keys, vim j/k, space, q, PgUp/PgDn unchanged; raw CSI/SS3 arrows + paging + Home/End now decode instead of false-cancelling.
  • Session browser (letters_are_nav=False): real arrows navigate; j/k/q/space/letters remain typed filter text; escape-sequence bytes can no longer leak into the filter.

Validation

  • New tests/hermes_cli/test_curses_menu_nav.py: paging, Home/End in both letter and numeric forms, modified arrows, both letters_are_nav modes, and the ESC-then-key ungetch requeue.
  • Updated the Home/End assertion in test_curses_arrow_keys.py (now decoded to NAV_HOME/END — previously ignored) and taught the session-browse test harness to honor curses.ungetch.
  • pytest tests/hermes_cli/ -k 'curses or plugin or browse or session or setup or menu or radio or select'782 passed, 0 regressions (one pre-existing, unrelated test_discover_project_plugins_skipped_by_default failure reproduces on clean main and is excluded).

Follow-up to the setup-wizard arrow-key fix. Two more interactive curses
menus had their own raw stdscr.getch() loops that only matched
curses.KEY_* and treated a leading byte 27 (ESC) as cancel, so on
terminals that deliver cursor keys as raw CSI/SS3 byte sequences
(Ghostty among them) pressing up/down misfired:

- hermes plugins manager (incl. the 'Provider Plugins' group/category
  rows): arrows bailed out via the {27, q} 'save & exit' branch.
- session browser (hermes browse): arrows cleared the filter / exited,
  and the trailing sequence bytes ([A, [B, ...) were injected into the
  type-to-filter search box as literal text.

Generalize the shared helper instead of duplicating decode logic:

- Add read_menu_key_ex() returning (action, raw_key) plus NAV_PAGE_UP/
  PAGE_DOWN/HOME/END/BACKSPACE. Decode the full CSI/SS3 vocabulary:
  arrows, PgUp/PgDn (5~/6~), Home/End (H/F and 1~/4~/7~/8~), modified
  arrows with parameters (e.g. [1;2B), and consume unhandled sequences
  (Delete 3~) up to their terminator so no bytes leak.
- An ESC immediately followed by a non-introducer byte (Alt-combo, fast
  typing, paste) now registers as a lone-ESC cancel and pushes the
  trailing byte back via curses.ungetch instead of swallowing it.
- letters_are_nav=False mode for the session browser: j/k/space/q stay
  typeable filter characters while real arrows (translated + escape) and
  Enter/Backspace/Esc still navigate.

Route plugins_cmd.py and main.py's session browser through the helper.
read_menu_key() is now a thin wrapper over read_menu_key_ex().

Tests: new test_curses_menu_nav.py (paging, Home/End in letter+numeric
forms, modified arrows, letters_are_nav both modes, ESC-then-key
ungetch). Updated the Home/End assertion in test_curses_arrow_keys.py
(now decoded, was ignored) and taught the session-browse harness to
honor curses.ungetch. Full related suite: 782 passed, 0 regressions.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 31, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Three listed PRs address this bug class: #8335 broadly combines raw arrow decoding with a migration of several model-related menus, #35776 implements the shared decoder for the core curses menus, and #35786 extends that merged decoder to the demonstrated plugin-manager and session-browser loops while adding broader navigation-sequence handling.

Related pull requests

  • #8335 [closed] related — (+581/-132) — superseded in its core fix: this closed PR decodes raw CSI/SS3 arrows across several curses call sites, but also carries a substantially broader migration from simple_term_menu plus rendering, fallback, and model-selection changes; its root-cause fix overlaps the narrower implementation merged in #35776 and remains relevant as prior regression coverage and design exploration.
  • #35776 [merged] related — (+190/-16) — merged reference implementation: it fixes the reported leading-ESC cancellation and trailing-byte leakage in curses_checklist, curses_radiolist, and curses_single_select through the shared read_menu_key() decoder; the contributor discussion explicitly identifies it as the most comprehensive fix among the compared work at that time.
  • #35786 related — (+324/-63) — merge: this follow-up generalizes #35776's decoder and applies it to two additional raw-getch loops shown by the diff, in the plugin manager and session browser, while preserving type-to-filter characters and decoding paging/Home/End variants. The supplied evidence demonstrates those two affected paths, but does not establish that they are the only remaining raw-getch loops repository-wide.

Duplicates

#8335 and #35776 substantially duplicate the core raw CSI/SS3 arrow-decoding fix, although #8335 has much broader menu-migration and rendering scope; #35786 is a follow-up rather than a duplicate. The discussion also identifies open related PRs #16370 and #35126, but their diffs and current states are absent here, so their duplicate status cannot be determined from this evidence.

Suggested consolidation

Merge #35786 as the focused follow-up to merged #35776 for the two additional affected menu loops demonstrated in its diff. #8335 is already closed and its core decoding work is superseded by #35776, while no close recommendation should be made for #16370 or #35126 until their current diffs and states are compared.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 72 kB of PR diffs, 7 kB of issue/PR text, 8 kB of discussion (8 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@kshitijk4poor
kshitijk4poor deleted the fix/curses-arrows-plugins-browser branch August 5, 2026 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/tui Terminal UI (ui-tui/ + tui_gateway/) P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants