Skip to content

fix(cli): stop Shift+Space CSI bytes leaking into buffer (#88071) - #88097

Open
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/88071-shiftspace-csi
Open

fix(cli): stop Shift+Space CSI bytes leaking into buffer (#88071)#88097
webtecnica wants to merge 1 commit into
NousResearch:mainfrom
webtecnica:fix/88071-shiftspace-csi

Conversation

@webtecnica

Copy link
Copy Markdown
Contributor

What Changed

Adds install_keypress_data_normalization() in hermes_cli/pt_input_extras.py, called from CLI startup alongside the other alias installers. It patches Vt100Parser._call_handler so that when an extended-key sequence maps to a single plain character (Shift+Space → ' ', Shift+letter → the uppercase letter, keypad digits/operators), the KeyPress.data field carries the mapped character instead of the raw CSI bytes.

The patch is idempotent (sentinel flag) and defensive (import failures degrade to a no-op), matching the existing Vt100Parser.feed bracketed-paste patch pattern.

Root Cause

Vt100Parser._call_handler builds KeyPress(key, match.group(0)) — the key was correctly remapped by the alias table (#86866 follow-up, #87511/#87630), but the data field still held the full raw prefix (e.g. \x1b[32;2u). prompt_toolkit's default character-insert binding (self-insert, basic.py) inserts event.data, so the raw CSI bytes landed in the buffer:

  • Shift+Space xterm'ab\x1b[27;2;32~cd'
  • Shift+Space kitty'ab\x1b[32;2ucd'

For a plain space both fields are ' ', which is why the bug was invisible there. The same leak class affected every string-valued alias: Shift+letter, keypad digits, keypad operators. The previous test only asserted on key_press.key == ' ' (parser output), so it passed while the buffer-level behavior leaked.

Verification

  • New parser-level tests assert KeyPress.data equals the mapped character for Shift+Space (both xterm modifyOtherKeys and kitty CSI-u formats), Shift+letter, keypad digits, and that plain space typing is unchanged.
  • New end-to-end test feeds the sequences through a real Application + Buffer (the repro from the issue) and asserts the buffer contains 'ab cd' with no raw CSI bytes.
  • tests/cli/test_modify_other_keys_aliases.py: 178 passed.
  • tests/cli/test_cli_shift_enter_newline.py, test_cli_cmd_backspace.py, test_cli_terminal_shortcuts.py: 20 passed.

Closes #88071

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists labels Aug 17, 2026
@ceotind

ceotind commented Aug 21, 2026

Copy link
Copy Markdown

need to merge this

@komarov-dc

Copy link
Copy Markdown

This PR fixes more than Shift+Space: as of 1a8fea3ce2 the same KeyPress.data bug makes every Shift+letter insert ^[[27;2;<cp>~ into the classic CLI prompt on Ghostty. Reproduced on current main (057dcdf236). Bisect and mechanism below — hopefully useful for prioritising the review.

Bisect

build sequence pushed to Ghostty Shift+L
5d3c15aaa7 (2026-08-19) \x1b[>1u\x1b[>4;2m L
057dcdf236 (current main) \x1b[>4;2m ^[[27;2;76~

1a8fea3ce2 "skip Kitty keyboard protocol push for Ghostty, use modifyOtherKeys only" is the only commit in that range that touches the pushed constant:

$ git log --format='%h %s' 5d3c15aaa7..057dcdf236 -S'_EXTENDED_ENTER_KEYS_SEQ' -- cli.py
45f11263bd fix(tui): skip the kitty protocol push for Ghostty in the Ink TUI too
1a8fea3ce2 fix(cli): skip Kitty keyboard protocol push for Ghostty, use modifyOtherKeys only

$ git merge-base --is-ancestor 1a8fea3ce2 5d3c15aaa7; echo $?
1   # absent from the good build
$ git merge-base --is-ancestor 1a8fea3ce2 057dcdf236; echo $?
0   # present in the bad build

One detail that makes this easy to misdate: author date is 2026-08-17, but it was committed to main on 2026-08-20 11:39 +0530, so installs that pulled between the 17th and the 20th were still fine.

Why the bug was dormant until then

The incomplete half of #87511 has been in the tree since 2026-08-16, but on Ghostty it was unreachable. While the kitty flag stack is active (CSI >1u), Ghostty never emits the ESC[27;2;<cp>~ form for Shift+letter — it sends the plain character — so the alias table was simply never exercised on that path. 1a8fea3ce2 removed the kitty push for Ghostty specifically, which dropped it onto the legacy modifyOtherKeys=2 encoding and straight onto the KeyPress.data defect this PR fixes.

Two individually reasonable fixes composed into a regression: #87511 shipped an alias table that only half works, and the #87630 Ghostty workaround is what turned that path on.

Mechanism (independently reproduced, matches this PR's analysis)

>>> install_modify_other_keys_aliases()
>>> out = []; Vt100Parser(out.append).feed('\x1b[27;2;76~')
>>> [(kp.key, kp.data) for kp in out]
[('L', '\x1b[27;2;76~')]

vt100_parser.py:173 _call_handler builds KeyPress(key, insert_text) from the raw matched prefix; named_commands.py:269 self_insert then runs insert_text(event.data). The key is remapped, the payload is not.

Why CI stayed green through all of this

tests/cli/test_modify_other_keys_aliases.py:52 — the shared _parse() helper ends with return [kp.key for kp in out], and kp.data is asserted zero times in that file across ~60 tests:

$ grep -c '\.data' tests/cli/test_modify_other_keys_aliases.py
0

That is how #87511 shipped, and why the +127 lines this same file gained between 2026-08-19 and 2026-08-24 still didn't catch it. The 122 test lines in this PR are what closes that hole — arguably the more valuable half of the change.

Scope note

This PR normalises data only for sequences present in ANSI_SEQUENCES, so two sibling leaks stay open — their codepoints are never mapped, so there is nothing to normalise:

It may also be worth reconsidering whether the push is right for Ghostty at all. Ghostty implements the kitty protocol natively, and anyone who already has keybind = shift+enter=text:\x1b\r in their Ghostty config gains nothing from CSI >4;2m — both CLI newline paths (@kb.add('escape','enter') at cli.py:18190, unconditional; c-j at cli.py:18201, kept via _preserve_ctrl_enter_newline()) already work without it.

Environment

  • Hermes Agent v0.20.5 (2026.8.19), git install, 057dcdf236 (= current main)
  • Python 3.11.15, prompt_toolkit 3.0.52
  • macOS (Darwin 25.5.0), Apple Silicon
  • Ghostty 1.3.1, default TERM=xterm-ghostty (no term override in config)

CI here is green and the branch is mergeable: clean — a merge would unbreak text input for every Ghostty user on main.

@ceotind

ceotind commented Aug 24, 2026

Copy link
Copy Markdown

have been seeing Shift + letter issue after recent update

@antrang

antrang commented Aug 25, 2026

Copy link
Copy Markdown

Confirming the fix works and adding scope data from a local port against macOS Terminal.app (not just Ghostty).

Local verification (Hermes v0.20.5 / git 0268c0b, Python 3.11.15, prompt_toolkit 3.0.52, macOS 26.6.2):

I ported this PR's install_keypress_data_normalization to a local pt_input_extras.py and wired it into cli_output.py so hermes --cli (the classic REPL, not just the TUI) picks it up at import. End-to-end through a real Application + Buffer:

Shift+Space xterm      payload='\x1b[27;2;32~'      -> buffer='ab cd'              OK
Shift+Space kitty      payload='\x1b[32;2u'         -> buffer='ab cd'              OK
Shift+M xterm          payload='\x1b[27;2;77~'      -> buffer='abMcd'              OK
Shift+M kitty          payload='\x1b[77;2u'         -> buffer='abMcd'              OK
plain space            payload=' '                  -> buffer='ab cd'              OK
plain letter           payload='M'                  -> buffer='abMcd'              OK

220 + 42 sibling tests all pass. Reproduction, root cause, and fix are exactly as you described in the PR — no surprises.

One scope observation worth flagging to reviewers: the bisect blames commit 1a8fea3ce2 (the Ghostty kitty-push skip), and most discussion has stayed focused on Ghostty. But on a stock macOS Terminal.app install I see the same Shift+letter leak and the same regression date. 1a8fea3ce2 doesn't mention Terminal.app, but the legacy CSI >4;2m push and the underlying KeyPress.data defect are universal — every classic CLI user who updated between 2026-08-20 and the next hermes release is affected, on every terminal that respects modifyOtherKeys=2. (For me it was reproducible on Terminal.app in under a second.)

So the fix in this PR is the right one and the merge would unblock a much wider blast radius than just Ghostty. +1 from me.

Small local addition in case the maintainer wants it as a follow-up: I also added a test_plain_letter_keypress_data_unchanged assertion to the same test file (in the spirit of the new .data assertions) to lock in that the normalisation predicate (insert_text.startswith('\x1b')) is the only trigger, so ordinary ASCII typing is provably unaffected. Happy to upstream as a follow-up commit on this PR or a separate one if the maintainer prefers.

@alt-glitch alt-glitch added P1 High — major feature broken, no workaround and removed P2 Medium — degraded but workaround exists labels Aug 25, 2026
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 P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Shift+Space still leaks raw CSI bytes into the buffer on main — follow-up to #86866

5 participants