fix(terminal): address review feedback on send_keys special key handling

Author: Jathin Sreenivas

openhands-tools/openhands/tools/terminal/terminal/subprocess_terminal.py

@@ -37,6 +37,25 @@
 
 ENTER = b"\n"
 
+# Map normalized special key names to ANSI escape bytes for PTY.
+_SUBPROCESS_SPECIALS: dict[str, bytes] = {
+    "ENTER": ENTER,
+    "TAB": b"\t",
+    "BS": b"\x7f",  # Backspace (DEL)
+    "ESC": b"\x1b",
+    "UP": b"\x1b[A",
+    "DOWN": b"\x1b[B",
+    "RIGHT": b"\x1b[C",
+    "LEFT": b"\x1b[D",
+    "HOME": b"\x1b[H",
+    "END": b"\x1b[F",
+    "PGUP": b"\x1b[5~",
+    "PGDN": b"\x1b[6~",
+    "C-L": b"\x0c",  # Ctrl+L
+    "C-D": b"\x04",  # Ctrl+D (EOF)
+    "C-C": b"\x03",  # Ctrl+C (SIGINT)
+}
+
 
 def _normalize_eols(raw: bytes) -> bytes:
     # CRLF/LF/CR -> CR, so each logical line is terminated with \r for the TTY
@@ -351,36 +370,16 @@ def send_keys(self, text: str, enter: bool = True) -> None:
         if not self._initialized:
             raise RuntimeError("PTY terminal is not initialized")
 
-        specials = {
-            "ENTER": ENTER,
-            "TAB": b"\t",
-            "BS": b"\x7f",  # Backspace (DEL)
-            "ESC": b"\x1b",
-            "UP": b"\x1b[A",
-            "DOWN": b"\x1b[B",
-            "RIGHT": b"\x1b[C",
-            "LEFT": b"\x1b[D",
-            "HOME": b"\x1b[H",
-            "END": b"\x1b[F",
-            "PGUP": b"\x1b[5~",
-            "PGDN": b"\x1b[6~",
-            "C-L": b"\x0c",  # Ctrl+L
-            "C-D": b"\x04",  # Ctrl+D (EOF)
-            "C-C": b"\x03",  # Ctrl+C (SIGINT)
-        }
-
         upper = text.upper().strip()
         payload: bytes | None = None
 
         # Named specials
-        if upper in specials:
-            payload = specials[upper]
+        if upper in _SUBPROCESS_SPECIALS:
+            payload = _SUBPROCESS_SPECIALS[upper]
             # Do NOT auto-append another EOL; special already includes it when needed.
             append_eol = False
         # Generic Ctrl-<letter>
-        elif parse_ctrl_key(text) is not None:
-            ctrl = parse_ctrl_key(text)
-            assert ctrl is not None
+        elif (ctrl := parse_ctrl_key(text)) is not None:
             # ctrl is "C-x" — extract the letter
             key_char = ctrl[-1].upper()
             payload = bytes([ord(key_char) & 0x1F])

openhands-tools/openhands/tools/terminal/terminal/tmux_terminal.py

@@ -20,6 +20,25 @@
 
 logger = get_logger(__name__)
 
+# Map normalized special key names to tmux key names.
+_TMUX_SPECIALS: dict[str, str] = {
+    "ENTER": "Enter",
+    "TAB": "Tab",
+    "BS": "BSpace",
+    "ESC": "Escape",
+    "UP": "Up",
+    "DOWN": "Down",
+    "LEFT": "Left",
+    "RIGHT": "Right",
+    "HOME": "Home",
+    "END": "End",
+    "PGUP": "PPage",
+    "PGDN": "NPage",
+    "C-L": "C-l",
+    "C-D": "C-d",
+    "C-C": "C-c",
+}
+
 
 class TmuxTerminal(TerminalInterface):
     """Tmux-based terminal backend.
@@ -130,24 +149,6 @@ def send_keys(self, text: str, enter: bool = True) -> None:
             raise RuntimeError("Tmux terminal is not initialized")
 
         # Map normalized names to tmux key names
-        _TMUX_SPECIALS = {
-            "ENTER": "Enter",
-            "TAB": "Tab",
-            "BS": "BSpace",
-            "ESC": "Escape",
-            "UP": "Up",
-            "DOWN": "Down",
-            "LEFT": "Left",
-            "RIGHT": "Right",
-            "HOME": "Home",
-            "END": "End",
-            "PGUP": "PPage",
-            "PGDN": "NPage",
-            "C-L": "C-l",
-            "C-D": "C-d",
-            "C-C": "C-c",
-        }
-
         upper = text.strip().upper()
 
         # 1) Named specials

tests/tools/terminal/test_send_keys.py

@@ -54,6 +54,23 @@ def test_supported_special_keys_contains_essentials() -> None:
         assert key in SUPPORTED_SPECIAL_KEYS
 
 
+def test_subprocess_specials_match_contract() -> None:
+    """Backend specials dicts must stay in sync with SUPPORTED_SPECIAL_KEYS."""
+    from openhands.tools.terminal.terminal.subprocess_terminal import (
+        _SUBPROCESS_SPECIALS,
+    )
+
+    assert set(_SUBPROCESS_SPECIALS.keys()) == SUPPORTED_SPECIAL_KEYS
+
+
+def test_tmux_specials_match_contract() -> None:
+    from openhands.tools.terminal.terminal.tmux_terminal import (
+        _TMUX_SPECIALS,
+    )
+
+    assert set(_TMUX_SPECIALS.keys()) == SUPPORTED_SPECIAL_KEYS
+
+
 # ── SubprocessTerminal.send_keys ────────────────────────────────────
 
 
@@ -90,6 +107,14 @@ def test_subprocess_send_keys_ctrl_variants(subprocess_terminal) -> None:
     subprocess_terminal.send_keys("CTRL+e")
 
 
+def test_subprocess_send_keys_echo(subprocess_terminal) -> None:
+    """Verify data actually flows through the PTY dispatch path."""
+    subprocess_terminal.send_keys("echo hello_subprocess")
+    time.sleep(0.5)
+    screen = subprocess_terminal.read_screen()
+    assert "hello_subprocess" in screen
+
+
 # ── TmuxTerminal.send_keys ─────────────────────────────────────────