Skip to content

fix(cli): enable arrow-key navigation in sessions browser - #16370

Open
suryanshsrivastava wants to merge 1 commit into
NousResearch:mainfrom
suryanshsrivastava:fix/session-browse-arrow-keys
Open

fix(cli): enable arrow-key navigation in sessions browser#16370
suryanshsrivastava wants to merge 1 commit into
NousResearch:mainfrom
suryanshsrivastava:fix/session-browse-arrow-keys

Conversation

@suryanshsrivastava

Copy link
Copy Markdown

Summary

  • enable curses keypad mode in the interactive session browser
  • enable keypad mode in shared curses pickers used elsewhere in Hermes
  • add a regression test covering keypad initialization in tests/hermes_cli/test_session_browse.py

Problem

hermes sessions browse could not be navigated with arrow keys in some terminals. Pressing an arrow key would immediately exit the picker and leave raw escape-sequence fragments in the shell, which users reported as strings like 27D, 26B, or similar junk.

Root cause: the curses UI was reading from stdscr.getch() without first enabling stdscr.keypad(True), so arrow keys were delivered as raw escape sequences instead of curses.KEY_UP / curses.KEY_DOWN.

Exact reproduction steps

  1. Open a terminal session where Hermes is installed.
  2. Ensure you have at least two saved sessions.
  3. Run:
    hermes sessions browse
  4. Press the Down Arrow key.
  5. Observe the buggy behavior on the unpatched version:
    • the picker exits immediately instead of moving the selection
    • the terminal receives leftover escape-sequence bytes
    • junk such as 27D, 26B, ^[ sequences, or similar random-looking strings may appear
  6. Re-run the same command on this branch.
  7. Press Up Arrow / Down Arrow again.
  8. Observe the fixed behavior:
    • the selection moves correctly
    • the picker stays open
    • pressing Enter resumes the selected session normally

Test plan

  • source venv/bin/activate && pytest -o addopts='' tests/hermes_cli/test_session_browse.py -q

Notes

This patch intentionally keeps the numbered fallback path unchanged for environments where curses is unavailable.

- enable curses keypad mode in session browser and shared pickers
- add regression coverage for keypad initialization
- preserves existing fallback behavior for non-curses terminals
@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 labels Apr 27, 2026
@suryanshsrivastava suryanshsrivastava changed the title fix: enable arrow-key navigation in sessions browser fix(cli): enable arrow-key navigation in sessions browser Apr 28, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for isolating the session-browser failure mode. The issue remains on current main, but this patch needs a different fix path.

Problems

  • stdscr.keypad(True) at hermes_cli/main.py:403 does not handle the raw ESC [ B / ESC O B sequences described in the PR. Current hermes_cli/main.py:1083-1104 still reads a raw first byte and treats 27 as cancellation.
  • Merged commit 3463c97a3 established that this raw-input behavior can occur even with keypad mode enabled, and added read_menu_key() in hermes_cli/curses_ui.py:276-340 to decode it.
  • The added test asserts keypad initialization only; it does not reproduce raw arrow input or verify session selection.

Suggested changes

  • Route the session picker through the existing raw-sequence decoder before its ESC-cancel branch, preserving its type-to-filter behavior.
  • Add a regression case for ESC [ B followed by Enter selecting the second session.

The shared-picker changes also need rework because current main consolidated those loops into _run_curses_menu() at hermes_cli/curses_ui.py:364-523.

Automated hermes-sweeper review.

Comment thread hermes_cli/main.py
@@ -402,6 +402,7 @@ def _match(s, query):

def _curses_browse(stdscr):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

keypad(True) alone does not cover the raw CSI/SS3 input described in this PR: merged commit 3463c97a3 verified that terminals can still return 27, '[', 'B' with keypad enabled. The current session loop needs sequence decoding before its ESC-cancel branch.

def run_inner(func):
func(mock_stdscr)

mock_wrapper.side_effect = run_inner

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please test the reported behavior rather than only setup: feed raw ESC [ B followed by Enter and assert that the second session is selected. This would fail on current main's direct getch()/ESC-cancel path.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform area/sessions Session lifecycle, resume, persistence, history labels Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants