fix: restore enhanced keyboard modes on PTY teardown

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Make server-side SSH compression configurable instead of hard-disabled** (#220). The #215 workaround forced the server to advertise only `none` compression for every deployment, with no way for operators to opt back in. A new `server.compression` option (YAML `server.compression: true`, env `BSSH_COMPRESSION`, builder `ServerConfigBuilder::compression`) now controls whether `build_russh_config` advertises russh's default compression list (`none`, `zlib`, `zlib@openssh.com`) or only `none`. The default stays off, so behavior is unchanged unless explicitly enabled, and enabling it logs a warning: russh's delayed-zlib (`zlib@openssh.com`) transport still desyncs a few packets after compression activates post-auth (reproduced on russh 0.61.1 and 0.62.1), dropping clients that negotiate it mid-session. Both settings are covered by unit tests, and the option plus the desync caveat are documented in the man page and config docs.
 
 ### Fixed
+- **Restore enhanced keyboard modes after interactive PTY disconnects** (#234). Remote applications can enable Kitty keyboard progressive enhancements or xterm `modifyOtherKeys`; when an SSH transport drops before the application sends its matching teardown, the local shell receives key presses as visible CSI-u fragments such as `;1:3u` or `:1C`. Both RAII and forced terminal cleanup now use one injectable best-effort reset path that drains Kitty mode stacks, clears active flags, disables `modifyOtherKeys`, and repeats keyboard cleanup after leaving the alternate screen because Kitty maintains separate main/alternate-screen stacks. Exact byte-order tests cover the shared writer, idempotency, and ignored write failures, and the manual Ghostty/Kitty-compatible terminal procedure is documented.
 - **Fix an SFTP session deadlock under paramiko's unbounded READ prefetch, and stop stalling sequential SFTP round trips on Nagle's algorithm** (#227). paramiko's `SFTPClient.get()` prefetches by firing READ requests for the whole file with no outstanding-request cap (2048 requests for a 64 MiB file), and any download to such a client froze at exactly the client's initial 2 MiB channel window, with the unread WINDOW_ADJUST messages sitting in the server socket's receive queue. The cycle: the SFTP processor blocks writing a response once the channel window is exhausted; the count-bounded (16) read-ahead queue fills; the reader task stops draining the channel stream; the russh channel rx buffer (100 messages) fills; and the russh session event loop blocks delivering channel data, so it stops reading the socket and never processes the WINDOW_ADJUST that would refill the window. OpenSSH's `sftp` caps outstanding requests at 64 and sshj uses bounded read-ahead, which is why only paramiko (the stack under Ansible and many Python tools) tripped it; the bug predates the #224 pipelining, whose serial predecessor coupled intake to response progress the same way. The SFTP intake queue is now unbounded in request count and bounded in bytes (`max_buffered_request_bytes`, default 8 MiB; a READ request is ~50 bytes, so legitimate pipelining stays far under it): the reader never stops draining while under budget, and a client that floods past it has its session terminated instead of deadlocked. Separately, `build_russh_config` now sets `nodelay: true` on accepted sockets (russh defaults to Nagle on; OpenSSH always sets TCP_NODELAY), removing a ~30 ms delayed-ACK stall per strictly sequential round trip. With both fixes the previously hanging paramiko `get()` completes with byte-for-byte integrity (64 MiB in 0.4 s, 2 GiB verified), and paramiko sequential reads (`prefetch=False`) reach parity with OpenSSH sshd on the same host (15.3 s vs 13.9 s for 64 MiB; the residual per-request latency is client-side, since paramiko itself never sets TCP_NODELAY). Client-initiated disconnects (`Connection reset by peer` and friends) are also no longer logged at ERROR, and regression tests cover the deadlock shape (verified to fail against the previous code), budget-overflow termination, and the disconnect classification.
 - **Make `sftp.root`/`scp.root` chroot usable by re-anchoring client paths under the chroot root** (#214). With a chroot configured, directory listing worked but every path resolution failed: `cd subdir`, `get file`, and even `get` of a file sitting directly at the chroot root were rejected as `path outside root` or `not found`, so only bare `/` and `readdir` functioned. Under chroot the client's coordinate space is rooted at `/` (this is what `realpath` reports and what the client sends back), but `resolve_chroot` treated a client absolute path such as `/subdir` as a *host* path and rejected anything not starting with the host root, so `/subdir` (meaning `<root>/subdir`) never matched. A prior change had introduced this to avoid "path doubling", but real clients never send the host path; they send chroot-relative absolute paths, so the guard broke the normal case. The SFTP and SCP resolvers now both interpret absolute and relative client paths relative to `root` (OpenSSH `ChrootDirectory` re-rooting), ignoring a leading `/`, dropping `.`, and clamping `..` so traversal still cannot escape. Containment is verified: `..` stays pinned at the root and a client `/etc/passwd` maps to `<root>/etc/passwd` inside the jail, never the host file. SCP previously kept the old "reject absolute outside root" behavior, so it is unified here to match `sftp.root`. Verified end-to-end with the OpenSSH `sftp` client (`cd`/`get`/root-level files/Unicode names all work; escape attempts stay confined) and covered by updated unit and integration tests.
 - **Advertise only `none` SSH compression so clients that negotiate `zlib@openssh.com` no longer drop mid-session** (#215). Cyberduck, OpenSSH `sftp -C`, and any client that prefers delayed zlib completed the SFTP handshake (`INIT` / `REALPATH` / `STAT` all succeeded) and then the connection died, with the server logging `SshEncoding: length invalid` on the next inbound packet. The root cause is russh's delayed-zlib (`zlib@openssh.com`) transport: the flate2 stream desyncs a few packets after compression activates post-auth, so russh decodes the following packet's length prefix out of corrupted plaintext. It reproduces on both russh 0.61.1 and 0.62.1, which rules out the channel-close path and pins it to compression rather than the SFTP layer (OpenSSH's default `sftp`, FileZilla, and paramiko all negotiate `none` and were unaffected). `build_russh_config` now sets `russh::Preferred { compression: Cow::Borrowed(&[compression::NONE]), ..DEFAULT }`, so the server advertises only `none` and every client falls back to the uncompressed transport, matching the Dropbear / OpenSSH `sftp-server` defaults used in Backend.AI kernel containers, where compression was never in play. Verified with `sftp -C` (now negotiates `compression: none` and lists/downloads cleanly) and a live Cyberduck 9.5 session. The underlying russh delayed-zlib desync is left to be fixed upstream.

docs/architecture/interactive-mode.md

@@ -182,6 +182,18 @@ All timeouts and buffer sizes have been carefully chosen based on empirical test
 - Multiple cleanup mechanisms ensure terminal restoration
 - `Drop` implementations provide failsafe cleanup
 - Force cleanup functions for emergency recovery
+- PTY teardown disables bracketed paste, mouse tracking, Kitty keyboard progressive enhancements, and xterm `modifyOtherKeys` on both the current and main screens before returning control to the local shell
+
+### Manual Enhanced-Keyboard Teardown Test
+
+Run this check in Ghostty and at least one other Kitty-keyboard-compatible terminal such as Kitty or WezTerm:
+
+1. Start an interactive session with `bssh user@host`.
+2. At the remote shell, run `printf '\033[>15u'; exit` to leave all supported Kitty keyboard enhancements enabled while the PTY exits.
+3. At the restored local prompt, verify that Space, Enter, arrow keys, and modified keys behave normally and do not print CSI-u fragments such as `;1:3u`, `:1C`, or `:3C`.
+4. Repeat with a Kitty-keyboard-aware full-screen application on the remote host and terminate the SSH transport before the application exits cleanly.
+
+The cleanup deliberately restores the ordinary shell baseline. Exact preservation of an enhanced keyboard mode owned by an outer local TUI is not currently available because crossterm exposes protocol support detection but not the active flag value, while issuing a separate `/dev/tty` query would race bssh's PTY input reader.
 
 ### Performance Characteristics
 

src/pty/terminal.rs

@@ -22,7 +22,7 @@ use std::sync::{
 
 use anyhow::{Context, Result};
 use crossterm::{
-    event::{DisableBracketedPaste, EnableBracketedPaste},
+    event::EnableBracketedPaste,
     execute,
     terminal::{disable_raw_mode, enable_raw_mode},
 };
@@ -32,6 +32,29 @@ use crossterm::{
 static TERMINAL_MUTEX: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
 static RAW_MODE_ACTIVE: AtomicBool = AtomicBool::new(false);
 
+/// Best-effort reset for terminal modes that a remote PTY application can leak.
+///
+/// Kitty keyboard mode stacks are independent for the main and alternate screens.
+/// Reset the current screen first, leave the alternate screen, and then reset the
+/// main screen. `CSI < 999 u` performs a large bounded pop of unbalanced pushes,
+/// `CSI = 0 u` also handles implementations without a stack, and
+/// `CSI > 4 ; 0 m` disables xterm's `modifyOtherKeys` fallback.
+///
+/// This intentionally restores the ordinary shell baseline. Crossterm does not
+/// expose the current enhancement flags, and querying `/dev/tty` here would race
+/// the PTY input reader. An outer local TUI that starts bssh with enhanced keyboard
+/// reporting enabled may therefore need to re-enable its mode after bssh exits.
+const TERMINAL_CLEANUP_SEQUENCE: &[u8] = b"\x1b[?2004l\x1b[<999u\x1b[=0u\x1b[>4;0m\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\x1b[?1049l\x1b[<999u\x1b[=0u\x1b[>4;0m\x1b[?25h";
+
+/// Emit the shared terminal reset sequence to an injectable writer.
+///
+/// Cleanup must never replace the original session error, so write and flush
+/// failures are deliberately ignored.
+fn write_terminal_cleanup(writer: &mut impl Write) {
+    let _ = writer.write_all(TERMINAL_CLEANUP_SEQUENCE);
+    let _ = writer.flush();
+}
+
 /// Terminal state information that needs to be preserved and restored
 #[derive(Debug, Clone)]
 pub struct TerminalState {
@@ -161,19 +184,9 @@ impl TerminalStateGuard {
         // Use global synchronization to prevent race conditions
         let _guard = TERMINAL_MUTEX.lock().unwrap();
 
-        // Disable bracketed paste mode
-        if let Err(e) = execute!(std::io::stdout(), DisableBracketedPaste) {
-            eprintln!("Warning: Failed to disable bracketed paste mode during cleanup: {e}");
-        }
-
-        // Best-effort: disable all mouse tracking modes that a remote program may have
-        // enabled. Each write is independent so one failure does not abort the rest.
-        // Modes: 1000 (X11), 1002 (button-event), 1003 (any-event), 1006 (SGR),
-        //        1015 (urxvt), plus restore cursor visibility and alternate screen.
-        let _ = std::io::stdout().write_all(
-            b"\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\x1b[?1049l\x1b[?25h",
-        );
-        let _ = std::io::stdout().flush();
+        // Reset every terminal mode owned by the remote PTY through the same path
+        // used by panic and last-resort cleanup.
+        write_terminal_cleanup(&mut std::io::stdout());
 
         // Exit raw mode if it's globally active
         if RAW_MODE_ACTIVE.load(Ordering::SeqCst) {
@@ -203,9 +216,9 @@ impl Drop for TerminalStateGuard {
 
 /// Force terminal cleanup - can be called from anywhere to ensure terminal is restored.
 ///
-/// This is a best-effort, infallible cleanup that disables mouse tracking, resets
-/// alternate screen and cursor visibility, and exits raw mode. Each operation is
-/// performed independently so a failure in one does not prevent the rest.
+/// This is a best-effort, infallible cleanup that disables bracketed paste, enhanced
+/// keyboard reporting, and mouse tracking, resets alternate screen and cursor
+/// visibility, and exits raw mode.
 ///
 /// # Panic-safety
 ///
@@ -228,13 +241,7 @@ pub fn force_terminal_cleanup() {
     // operations below are individually safe.
     let _guard = TERMINAL_MUTEX.try_lock().ok();
 
-    // Best-effort: disable all mouse tracking modes, restore cursor, and leave alternate
-    // screen. Written as a single atomic blob to minimize partial-state risk.
-    // Modes: 1000 (X11), 1002 (button-event), 1003 (any-event), 1006 (SGR),
-    //        1015 (urxvt); then restore cursor visibility and normal screen buffer.
-    let _ = std::io::stdout()
-        .write_all(b"\x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\x1b[?1049l\x1b[?25h");
-    let _ = std::io::stdout().flush();
+    write_terminal_cleanup(&mut std::io::stdout());
 
     if RAW_MODE_ACTIVE.load(Ordering::SeqCst) {
         let _ = disable_raw_mode();
@@ -343,19 +350,65 @@ mod tests {
     //     must be carefully ordered or marked #[serial].
     //
     // What *can* be reliably tested here:
-    //   1. Idempotency: calling force_terminal_cleanup() twice does not panic.
-    //   2. Poisoned-mutex resilience: the `try_lock().ok()` pattern correctly
+    //   1. Exact cleanup bytes and ordering through an injectable writer.
+    //   2. Idempotency: calling force_terminal_cleanup() twice does not panic.
+    //   3. Poisoned-mutex resilience: the `try_lock().ok()` pattern correctly
     //      yields None (rather than panicking) when the mutex is poisoned.
     //      Verified using a local Mutex so the global TERMINAL_MUTEX is never
     //      poisoned, keeping other tests unaffected.
-    //   3. Held-mutex resilience: `try_lock()` returns WouldBlock (not a
+    //   4. Held-mutex resilience: `try_lock()` returns WouldBlock (not a
     //      deadlock) when a lock is already held. Verified using a local Mutex
     //      for the same isolation reason.
-    //
-    // Manual reproduction of the actual terminal fix (mouse tracking escape
-    // sequences) requires a real TTY (vim/tmux) and cannot be automated here.
     // -----------------------------------------------------------------------
 
+    #[test]
+    fn test_terminal_cleanup_writes_exact_mode_reset_sequence() {
+        let mut output = Vec::new();
+
+        write_terminal_cleanup(&mut output);
+
+        assert_eq!(
+            output,
+            b"\x1b[?2004l\
+              \x1b[<999u\x1b[=0u\x1b[>4;0m\
+              \x1b[?1000l\x1b[?1002l\x1b[?1003l\x1b[?1006l\x1b[?1015l\
+              \x1b[?1049l\
+              \x1b[<999u\x1b[=0u\x1b[>4;0m\
+              \x1b[?25h"
+        );
+    }
+
+    #[test]
+    fn test_terminal_cleanup_writer_is_idempotent() {
+        let mut output = Vec::new();
+
+        write_terminal_cleanup(&mut output);
+        write_terminal_cleanup(&mut output);
+
+        assert_eq!(output.len(), TERMINAL_CLEANUP_SEQUENCE.len() * 2);
+        assert_eq!(
+            &output[..TERMINAL_CLEANUP_SEQUENCE.len()],
+            &output[TERMINAL_CLEANUP_SEQUENCE.len()..]
+        );
+    }
+
+    #[test]
+    fn test_terminal_cleanup_ignores_writer_failures() {
+        struct FailingWriter;
+
+        impl Write for FailingWriter {
+            fn write(&mut self, _buffer: &[u8]) -> std::io::Result<usize> {
+                Err(std::io::Error::other("intentional write failure"))
+            }
+
+            fn flush(&mut self) -> std::io::Result<()> {
+                Err(std::io::Error::other("intentional flush failure"))
+            }
+        }
+
+        write_terminal_cleanup(&mut FailingWriter);
+    }
+
     /// Calling force_terminal_cleanup() twice in succession must not panic.
     ///
     /// In a non-TTY test environment RAW_MODE_ACTIVE is false (no test calls