Refactor shell I/O handling with unified stdin reader#8
Merged
Conversation
Make the interactive shell behave like plain `ssh`. PTY input was reconstructed byte-by-byte from parsed crossterm key events, which mistranslated a lot of input: Ctrl+<letter> was off by one (Ctrl+C sent 0x02, so it never interrupted), non-ASCII/UTF-8 keys were truncated to a single byte, Backspace sent a BS-SP-BS erase sequence instead of DEL, and function keys used a bogus encoding. Instead, put the terminal in raw mode and forward stdin bytes verbatim to the channel, letting the remote PTY interpret them — control keys, UTF-8 and escape sequences now pass through correctly. Local terminal resizes are mirrored to the remote PTY, and raw mode is restored via an RAII guard on every exit path. Both shells previously spun in a 1ms sleep loop calling read_nonblocking. Replace that with a select! over the stdin channel and a 10ms ticker: the loop parks until there is input or it is time to drain output, so an idle session no longer burns CPU while input stays responsive. Output is now fully drained each wake, and the dumb shell reads both stdout and stderr every cycle instead of alternating between them. Also send the local $TERM when requesting the PTY and exit non-zero when the shell loop fails. https://claude.ai/code/session_017vAGzSSYNcgdQzJRLWfX47
Dumb mode has no remote pseudo-terminal, so there is no line discipline on the far end to translate carriage returns. On platforms whose console sends Enter as CR or CRLF (e.g. Windows), a typed `ls` arrives at the remote shell as `ls\r`, which fails with `ls\r: not found` — and the embedded CR rewinds the cursor so it shows up as just ": not found". Translate CR and CRLF in the forwarded input to LF, mirroring what a terminal line discipline would do, with carry-over state so a `\r\n` split across reads still collapses to a single `\n`. PTY mode is unaffected: it sends CR and lets the remote PTY do the translation. https://claude.ai/code/session_017vAGzSSYNcgdQzJRLWfX47
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the shell I/O handling in both PTY and dumb modes to use a unified, cleaner approach for reading stdin and managing the event loop. The changes extract common I/O patterns into a new
iomodule and simplify the main shell loops.Key Changes
New
iomodule: Extractedspawn_stdin_reader()function that reads stdin on a background thread and forwards bytes through a channel, replacing the previous per-mode event handling threadsSimplified PTY mode (
pty.rs):EventThreadstruct and crossterm event handlingRawModeRAII guard to ensure terminal is restored even on early returndrain()helper functionSimplified dumb mode (
dumb.rs):EventThreadstruct in favor of sharedspawn_stdin_reader()crlf_to_lf()function to normalize line endings (CR/CRLF → LF) since dumb mode lacks remote line disciplinedrain()helper functionUnified polling: Both modes now use
crossbeam_channel::tick()for periodic polling with a 10ms interval, replacing ad-hoc sleep callsError handling: Extracted
io_error()helper for consistent error conversion from libssh errorsTerminal handling: Improved TERM environment variable handling in main.rs
Implementation Details
select!to multiplex between stdin input and a periodic tickersend_eof()is called on the remote channel to signal completioncrlf_to_lf()function maintains state across chunk boundaries to handle CRLF pairs split across readshttps://claude.ai/code/session_017vAGzSSYNcgdQzJRLWfX47