8000 fix(minimald): fix detach-chord detection under zellij by twitchyliquid64 · Pull Request #440 · gominimal/minimal · GitHub
[go: up one dir, main page]
Skip to content

fix(minimald): fix detach-chord detection under zellij #440

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
twitchyliquid64 merged 1 commit into from
Jun 17, 2026
Merged

fix(minimald): fix detach-chord detection under zellij #440

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion crates/minimald/src/session_host.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ use crate::RequestedPty;
#[cfg(not(test))]
use crate::session::SessionHandle;

/// Command sequence for the ctrl-w key chord, when the kitty keyboard protocol
/// is negotiated.
///
/// Corresponds to: Kitty: CSI 119 ; 5 u
const CTRL_W_CSI_U: &[u8] = b"\x1b[119;5u";
/// Command sequence for the ctrl-w key chord, when the modifyOtherKeys key
/// sequences are used by the outer terminal.
///
/// Corrsponds to: modifyOtherKeys: CSI 27 ; 5 ; 119 ~
const CTRL_W_CSI_27: &[u8] = b"\x1b[27;5;119~";

/// The dimensions of a terminal.
///
/// This is the libc-facing view of a terminal size, mirroring `libc::winsize`.
Expand Down Expand Up @@ -912,7 +923,13 @@ impl<P: SessionProcess, G: Send + 'static> Host<P, G> {
match msg {
Either::Left(b) => {
self.attrs.stdin_last = Some(SystemTime::now());
if b.len() == 1 && b[0] == 0x17 { // ctrl-w

// ctrl-w
let is_detach = b.len() == 1 && b[0] == 0x17 ||
b == CTRL_W_CSI_U ||
b == CTRL_W_CSI_27;
Comment on lines +927 to +930
@coderabbitai coderabbitai Bot Jun 17, 2026
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does russh::ChannelMsg::Data guarantee input message boundaries for terminal keystroke streams, or can CSI escape sequences be split/coalesced across messages?

💡 Result:

The russh::ChannelMsg::Data variant does not guarantee input message boundaries for terminal keystroke streams or any other application-level data [1][2]. SSH channel data is treated as a continuous, unstructured byte stream [3][4]. Because the SSH connection protocol (RFC 4254) multiplexes data over channels without preserving application-layer framing, CSI escape sequences and other terminal control codes can be split, coalesced, or fragmented across multiple ChannelMsg::Data messages [3][4]. Key technical considerations: 1. Stream Orientation: The russh library exposes SSH channels as stream-oriented abstractions, such as ChannelStream, which implements AsyncRead and AsyncWrite [1][5]. These abstractions behave like standard TCP-like streams where the receiver must handle partial reads and potentially reassemble data fragments [1][5]. 2. Protocol Nature: SSH SSH_MSG_CHANNEL_DATA packets are designed to carry arbitrary byte streams [4][6]. There is no inherent protocol-level requirement for an SSH message to contain a "complete" sequence of terminal data, and implementations (both client and server) are free to segment data based on window sizes, buffer limits, or network packet boundaries [4]. 3. Handling Requirements: Applications consuming these streams must implement their own buffering and parsing logic to reliably identify and process complete CSI sequences (e.g., waiting until a full ESC [ sequence is received) [3]. You cannot rely on individual ChannelMsg::Data messages to contain atomically complete terminal escape codes [3][4].

Citations:

🏁 Script executed:

# Get context around lines 927-930 in the file
head -n 950 crates/minimald/src/session_host.rs | tail -n 50 | cat -n

Repository: gominimal/minimal

Length of output: 2838

🏁 Script executed:

# Get more context to understand the data flow and any existing buffering
sed -n '900,960p' crates/minimald/src/session_host.rs | cat -n

Repository: gominimal/minimal

Length of output: 3498

🏁 Script executed:

# Search for existing buffering logic or state machines in the file
rg -n "buffer|Buffer|state|State" crates/minimald/src/session_host.rs | head -20

Repository: gominimal/minimal

Length of output: 715

🏁 Script executed:

# Check the function/method containing this code to understand the full context
sed -n '850,950p' crates/minimald/src/session_host.rs | cat -n

Repository: gominimal/minimal

Length of output: 5439

🏁 Script executed:

# Find where remote_rx is defined and initialized
rg -n "remote_rx" crates/minimald/src/session_host.rs | head -20

Repository: gominimal/minimal

Length of output: 302

🏁 Script executed:

# Search for where remote_rx gets its data (who sends to it)
rg -n "remote_rx\|Binding::run" crates/minimald/src/ -A 5 | grep -E "(fn run|send|remote_rx)" | head -30

Repository: gominimal/minimal

Length of output: 43

🏁 Script executed:

# Find the Binding struct and run method to understand data flow
fd -t f "*.rs" crates/minimald/src/ | xargs rg -l "Binding" | head -5

Repository: gominimal/minimal

Length of output: 399

🏁 Script executed:

# Search for where remote_tx is used (who sends data)
rg -n "remote_tx" crates/minimald/src/ -A 2 -B 2

Repository: gominimal/minimal

Length of output: 1974

🏁 Script executed:

# Find Binding::run to understand data flow from russh
rg -n "impl.*Binding|fn run" crates/minimald/src/session_host.rs | head -20

Repository: gominimal/minimal

Length of output: 113

🏁 Script executed:

# Look for the actual file that contains Binding::run or the connection handler
find crates/minimald/src -name "*.rs" -exec grep -l "Binding.*run\|remote_tx.send" {} \;

Repository: gominimal/minimal

Length of output: 43

🏁 Script executed:

# Read the Binding::run method to understand data flow from russh
sed -n '236,350p' crates/minimald/src/session_host.rs | cat -n

Repository: gominimal/minimal

Length of output: 6213

🏁 Script executed:

# Also find the Binding struct definition
sed -n '218,240p' crates/minimald/src/session_host.rs | cat -n

Repository: gominimal/minimal

Length of output: 941

🏁 Script executed:

# Search for where data is sent via remote_tx in Binding::run
sed -n '236,400p' crates/minimald/src/session_host.rs | grep -n "remote_tx\|send\|Channel"

Repository: gominimal/minimal

Length of output: 516

Detach detection logic depends on chunk boundaries and can miss Ctrl-W sequences.

At lines 928–930, is_detach checks exact equality against incoming bytes chunks. Since russh::ChannelMsg::Data does not guarantee message boundaries, CSI escape sequences (like CTRL_W_CSI_U) can be split across multiple messages. If a detach sequence fragments, neither chunk will match and detach will fail to trigger.

Implement stream-aware matching using a small rolling bu 7098 ffer or state machine instead of per-chunk equality checks.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/minimald/src/session_host.rs` around lines 927 - 930, The is_detach
logic at lines 928-930 performs exact byte equality checks on individual message
chunks, but multi-byte CSI escape sequences like CTRL_W_CSI_U and CTRL_W_CSI_27
can be fragmented across multiple russh::ChannelMsg::Data messages, causing
detach detection to fail. Replace the per-chunk equality checks with a rolling
buffer or state machine that accumulates incoming bytes across multiple messages
and matches against the complete sequences. This ensures that even if a Ctrl-W
sequence arrives fragmented, it will still be properly detected.


if is_detach {
let uc = self.unwind_codes();
if let Some((tx, _hnd)) = self.remote.as_mut() {
match tx.send(BindingMsg::TeardownDueToDetach(uc)).await {
Expand Down
Loading
0