Skip to content

fix(cli): map kitty CSI-u lock-bit variants so key combos survive NumLock - #89676

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-89651
Closed

fix(cli): map kitty CSI-u lock-bit variants so key combos survive NumLock#89676
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-89651

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

When the CLI pushes the kitty keyboard protocol (disambiguate mode) to allowlisted terminals, kitty and ghostty encode the CapsLock/NumLock state as extra bits in the CSI-u modifier parameter: with NumLock on, Ctrl+C arrives as ESC[99;133u (5 + 128) instead of ESC[99;5u. The alias table installed by install_modify_other_keys_aliases() only mapped the exact base modifiers, so with a lock enabled every key combo was unmapped and leaked as literal text ([127;133u[127;133u...), breaking Ctrl+C, Ctrl+Backspace word-movement, and all other combos — while gnome terminal (no kitty protocol) kept working.

This installs every CSI-u alias with its lock-bit variants (base + 64 CapsLock, +128 NumLock, +192 both), mapped to the same Keys value as the base form. The xterm modifyOtherKeys encoding (ESC[27;N;CP~) never carries lock bits, so that form is deliberately left untouched. The Esc-key registration also now covers modifier 1 (a lone Esc with NumLock on arrives as ESC[27;129u).

Related Issue

Fixes #89651

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/pt_input_extras.py: _install_paired() now installs each CSI-u sequence with the four lock-bit variants (0/64/128/192); the modifyOtherKeys tilde form stays base-only. The Esc-key loop registers modifier 1..16 with the same variants. Docstring documents the new variants.

How to Test

  1. pytest tests/cli/test_modify_other_keys_aliases.py -q — 206 passed (34 new lock-bit tests).
  2. Fail-on-main verified: stashing the source change makes the 34 new tests fail on the unmodified table.
  3. Full pytest tests/cli/ -q on branch vs stashed-main: identical failure sets apart from the new tests (remaining failures are pre-existing environment-dependent ones in both runs).
  4. Manual equivalent of the report: _parse("\x1b[127;133u") now yields [Escape, ControlH] (Ctrl+Backspace) instead of leaking [, 1, 2, 7... as text.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (arm64)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

…Lock

kitty and ghostty OR the CapsLock (64) / NumLock (128) state into the
CSI-u modifier parameter. With NumLock on, Ctrl+C arrives as
ESC[99;133u (5 + 128) instead of ESC[99;5u; the alias table had no
entry for it, so every key combo leaked as literal text like
[127;133u (NousResearch#89651). Install every CSI-u alias with the lock-bit
variants (+64/+128/+192); the xterm modifyOtherKeys encoding never
carries lock bits, so the ESC[27;N;CP~ form is left untouched. The
Esc-key registration now covers modifier 1 as well (1+128=129 is a
lone Esc with NumLock on).
@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 sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 19, 2026
kshitijk4poor added a commit to kshitijk4poor/hermes-agent that referenced this pull request Aug 20, 2026
…s, and PUA functional keys

Follow-up to the salvaged NousResearch#89676 + NousResearch#90291 lock-bit fixes: extract a shared _lock_variants() helper and cover the sites both PRs missed - install_shift_enter_alias / install_ctrl_enter_alias / install_cmd_backspace_alias CSI-u spellings, legacy CSI-letter and CSI-tilde navigation twins derived from the existing table for ALL modifiers 1-16 (not just plain/shift), plain F1-F4 SS3 fallback, unmodified CSI-u keys (Tab/Enter/Space/Backspace), and kitty PUA functional keys (keypad, F13-F24, Ignore range) under lock bits. 8 new tests.
kshitijk4poor added a commit that referenced this pull request Aug 20, 2026
…s, and PUA functional keys

Follow-up to the salvaged #89676 + #90291 lock-bit fixes: extract a shared _lock_variants() helper and cover the sites both PRs missed - install_shift_enter_alias / install_ctrl_enter_alias / install_cmd_backspace_alias CSI-u spellings, legacy CSI-letter and CSI-tilde navigation twins derived from the existing table for ALL modifiers 1-16 (not just plain/shift), plain F1-F4 SS3 fallback, unmodified CSI-u keys (Tab/Enter/Space/Backspace), and kitty PUA functional keys (keypad, F13-F24, Ignore range) under lock bits. 8 new tests.
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Merged via #90561 with your commit cherry-picked onto current main — your authorship is preserved in the git history (git log shows your commit under your name on main). We combined it with #90291's legacy CSI-letter/tilde navigation slice and widened the coverage to the alias installers (Shift/Ctrl+Enter, Cmd+Backspace), modified navigation keys for all modifiers 1–16, and the kitty PUA functional keys under lock bits.

Your producer-contract insight — that the xterm modifyOtherKeys tilde form never carries lock bits, so only the CSI-u form needs the twins — shaped the final design and is negative-tested in the merged suite. Thanks!

@liuhao1024

Copy link
Copy Markdown
Contributor Author

Thank you for the merge path via #90561 and for preserving authorship in the git history — and for extending the coverage well beyond the original slice. Combining the lock-bit mapping with #90291's legacy CSI-letter/tilde navigation, widening to the alias installers (Shift/Ctrl+Enter, Cmd+Backspace), covering modified navigation keys for all modifiers 1–16, and adding the kitty PUA function keys turns two narrow fixes into a coherent terminal-input layer. That's the right shape for this family of bugs — glad the NumLock lock-bit diagnosis could serve as the seed.

lisajlau pushed a commit to lisajlau/hermes-agent that referenced this pull request Aug 20, 2026
…s, and PUA functional keys

Follow-up to the salvaged NousResearch#89676 + NousResearch#90291 lock-bit fixes: extract a shared _lock_variants() helper and cover the sites both PRs missed - install_shift_enter_alias / install_ctrl_enter_alias / install_cmd_backspace_alias CSI-u spellings, legacy CSI-letter and CSI-tilde navigation twins derived from the existing table for ALL modifiers 1-16 (not just plain/shift), plain F1-F4 SS3 fallback, unmodified CSI-u keys (Tab/Enter/Space/Backspace), and kitty PUA functional keys (keypad, F13-F24, Ignore range) under lock bits. 8 new tests.
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 P2 Medium — degraded but workaround exists 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.

[Bug]: Having NumLock enabled prevents key combos

3 participants