Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions crates/terminal-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ base64.workspace = true
chrono.workspace = true
portable-pty = "0.9.0"

# For `tcgetattr`, which is the only way to ask whether the child has taken the
# terminal out of canonical mode. That is the moment input stops being discarded.
[target.'cfg(unix)'.dependencies]
libc = "0.2"

[dev-dependencies]
criterion = "0.8.2"
tempfile = "3.27.0"
Expand Down
103 changes: 100 additions & 3 deletions crates/terminal-core/src/pty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,38 @@ const FLUSH_BYTES: usize = 32 * 1024;
/// Size of each blocking PTY read.
const READ_BUF: usize = 64 * 1024;

/// How often to ask whether the child has taken over the terminal.
const INPUT_GATE_POLL: Duration = Duration::from_millis(5);

/// Longest to hold input waiting for a child that may never take over.
///
/// Bounded because plenty of programs read in canonical mode for their whole life
/// and are perfectly able to receive input. They discard nothing, so gating them
/// would delay real keystrokes for no benefit.
const INPUT_GATE_MAX: Duration = Duration::from_millis(1500);

/// Whether the child has put the terminal into a mode it reads input from itself.
///
/// A line editor — zsh's ZLE, bash's readline — clears `ICANON` as it takes over,
/// and it does so with a `tcsetattr` that discards input already queued. Once the
/// flag is clear that call has happened, so anything written from here on survives.
///
/// The master's termios reports the pty's line discipline, which makes this the
/// child's own answer rather than a guess from elapsed time or from output, both of
/// which say only that a prompt was *printed* — a different and earlier moment.
#[cfg(unix)]
fn accepts_input(fd: std::os::unix::io::RawFd) -> bool {
// SAFETY: `fd` is the pty master, owned by this session and open for as long as
// the watcher runs. `tcgetattr` only reads, and only into the local struct.
unsafe {
let mut settings: libc::termios = std::mem::zeroed();
if libc::tcgetattr(fd, &mut settings) != 0 {
return false;
}
settings.c_lflag & libc::ICANON == 0
}
}

/// Longest a synchronized-output frame may hold back a flush.
///
/// An application that sets DEC 2026 and then blocks — or crashes before
Expand Down Expand Up @@ -207,10 +239,16 @@ pub struct PtySession {
/// `MasterPty` is `Send` but not `Sync`, and the session is shared across
/// threads by the pane registry and every IPC command that touches it.
master: Mutex<Box<dyn MasterPty + Send>>,
writer: Mutex<Box<dyn Write + Send>>,
/// Shared with the readiness watcher, which flushes any gated input.
writer: Arc<Mutex<Box<dyn Write + Send>>>,
child: Arc<Mutex<Box<dyn Child + Send + Sync>>>,
alive: Arc<AtomicBool>,
size: Mutex<(u16, u16)>,
/// Input written before the child could receive it.
///
/// `Some` while the child is still taking over the terminal, `None` once
/// anything written goes straight through. See [`PtySession::write`].
pending_input: Arc<Mutex<Option<Vec<u8>>>>,
}

impl PtySession {
Expand Down Expand Up @@ -462,13 +500,53 @@ impl PtySession {
.map_err(PtyError::Io)?;
}

let pending_input = Arc::new(Mutex::new(Some(Vec::new())));
let master = Mutex::new(pair.master);
let writer = Arc::new(Mutex::new(writer));

// Release input as soon as the child can actually receive it.
#[cfg(unix)]
{
let fd = master.lock().as_raw_fd();
let pending = pending_input.clone();
let writer = writer.clone();
let alive = alive.clone();
std::thread::Builder::new()
.name(format!("tervin-pty-ready-{}", config.pane_id))
.spawn(move || {
let deadline = Instant::now() + INPUT_GATE_MAX;
while Instant::now() < deadline
&& alive.load(Ordering::SeqCst)
&& !fd.is_some_and(accepts_input)
{
std::thread::sleep(INPUT_GATE_POLL);
}

// Open the gate either way, and under the same lock `write`
// takes, so nothing written afterwards can overtake what was
// held. A child that never leaves canonical mode — `cat`, a
// pager reading lines — discards nothing and must not be gated.
let mut pending = pending.lock();
if let Some(queued) = pending.take().filter(|q| !q.is_empty()) {
let mut w = writer.lock();
let _ = w.write_all(&queued).and_then(|()| w.flush());
}
})
.map_err(PtyError::Io)?;
}
#[cfg(not(unix))]
{
*pending_input.lock() = None;
}

Ok(Self {
pane_id: config.pane_id,
master: Mutex::new(pair.master),
writer: Mutex::new(writer),
master,
writer,
child,
alive,
size: Mutex::new((config.cols, config.rows)),
pending_input,
})
}

Expand All @@ -485,10 +563,29 @@ impl PtySession {
}

/// Write user input to the PTY.
///
/// Input written before the child has taken over the terminal is held rather
/// than sent. A shell's line editor calls `tcsetattr` with `TCSAFLUSH` when it
/// starts reading, and that **discards whatever is already queued** — so input
/// sent a moment too early is not delayed, it is destroyed. Tervin writes to a
/// pane programmatically as well as on keystrokes, and a restored session or an
/// agent-issued command lands squarely in that window: the symptom is a command
/// that runs with its first character missing.
///
/// The gate is held under the same lock the watcher releases it with, so a write
/// arriving after the gate opens can never overtake one that was held. It costs
/// one uncontended lock once the pane is running.
pub fn write(&self, data: &[u8]) -> Result<(), PtyError> {
if !self.is_alive() {
return Err(PtyError::NotRunning(self.pane_id.clone()));
}
{
let mut pending = self.pending_input.lock();
if let Some(queue) = pending.as_mut() {
queue.extend_from_slice(data);
return Ok(());
}
}
let mut w = self.writer.lock();
w.write_all(data)?;
w.flush()?;
Expand Down
92 changes: 70 additions & 22 deletions crates/terminal-core/tests/pty_roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ fn run(program: &str, args: &[&str], input: &[&str], done: impl Fn(&str) -> bool

let session = PtySession::spawn(config, sink).expect("could not open a pty");

// Give the shell a moment to reach its first prompt before typing, so input
// is not swallowed by a shell still setting up its line editor.
std::thread::sleep(Duration::from_millis(600));
// No sleep before typing. `PtySession` holds input until the child has taken
// the terminal over, which is the only thing a sleep here ever approximated.
for line in input {
session
.write(line.as_bytes())
Expand Down Expand Up @@ -87,6 +86,22 @@ fn run(program: &str, args: &[&str], input: &[&str], done: impl Fn(&str) -> bool
collected
}

/// Write a word so the terminal's echo of it cannot pass for the shell's output.
///
/// A PTY echoes what is typed. A test that types `echo FOO` and then waits for
/// `FOO` is therefore satisfied by its own echo, before the shell has run at all,
/// and every assertion after it is reading the input back. Splitting the word with
/// an empty quote leaves the echo reading `F''OO` while the shell still prints
/// `FOO`, so only real output can match.
///
/// This is not hypothetical. Every marker in this file was written the plain way,
/// which is why the burst test stopped collecting after 81 bytes and then reported
/// the missing lines as though the pump had dropped them.
fn only_in_output(word: &str) -> String {
let (head, tail) = word.split_at(1);
format!("{head}''{tail}")
}

/// Strip escape sequences so assertions match what a user would read.
fn plain(text: &str) -> String {
let mut out = String::with_capacity(text.len());
Expand Down Expand Up @@ -135,7 +150,8 @@ fn a_real_shell_receives_input_and_returns_output() {
// The input path: keystrokes written to the PTY must reach the shell and its
// output must come back through the pump. A screenshot of a prompt proves
// only the output half.
let collected = run("/bin/sh", &[], &["echo tervin-roundtrip-ok\n"], |text| {
let line = format!("echo {}\n", only_in_output("tervin-roundtrip-ok"));
let collected = run("/bin/sh", &[], &[&line], |text| {
text.contains("tervin-roundtrip-ok")
});
let text = plain(&collected.text);
Expand All @@ -148,12 +164,12 @@ fn a_real_shell_receives_input_and_returns_output() {
#[test]
fn output_arrives_in_order_across_several_commands() {
// The coalescer batches reads; batching must never reorder them.
let collected = run(
"/bin/sh",
&[],
&["echo one\n", "echo two\n", "echo three\n"],
|text| text.contains("three"),
);
let lines: Vec<String> = ["one", "two", "three"]
.iter()
.map(|w| format!("echo {}\n", only_in_output(w)))
.collect();
let refs: Vec<&str> = lines.iter().map(String::as_str).collect();
let collected = run("/bin/sh", &[], &refs, |text| text.contains("three"));
let text = plain(&collected.text);
let one = text.find("one");
let two = text.find("two");
Expand All @@ -168,16 +184,45 @@ fn output_arrives_in_order_across_several_commands() {
);
}

#[test]
fn input_written_the_instant_a_pane_opens_is_not_eaten() {
// The bug the input gate exists for, and the one a sleep only hid. A shell's
// line editor calls `tcsetattr` with `TCSAFLUSH` as it starts, which discards
// input already queued — so writing a moment too early does not arrive late, it
// does not arrive at all, and the command runs a character short.
//
// Tervin writes to panes programmatically as well as on keystrokes, so this is
// a restored session or an agent-issued command losing its leading byte, not a
// hypothetical. Written with no delay whatsoever, which is the whole point.
let marker = only_in_output("tervin-first-byte-intact");
let collected = run("zsh", &[], &[&format!("echo {marker}\n")], |text| {
text.contains("tervin-first-byte-intact")
});
let text = plain(&collected.text);

assert!(
text.contains("tervin-first-byte-intact"),
"the command never ran:\n{text}"
);
// `cho` is the exact signature of the loss, and it is not a failure the shell
// reports usefully: it is simply a command that does not exist.
assert!(
!text.contains("cho tervin-first-byte-intact"),
"the leading character was eaten before the shell could read it:\n{text}"
);
}

#[test]
fn a_large_burst_of_output_arrives_intact() {
// The pump flushes early at a size threshold; nothing may be dropped at the
// boundary. 5000 lines crosses it many times over.
let collected = run(
"/bin/sh",
&[],
&["i=0; while [ $i -lt 5000 ]; do echo line-$i; i=$((i+1)); done; echo BURST-DONE\n"],
|text| text.contains("BURST-DONE"),
let script = format!(
"i=0; while [ $i -lt 5000 ]; do echo line-$i; i=$((i+1)); done; echo {}\n",
only_in_output("BURST-DONE"),
);
let collected = run("/bin/sh", &[], &[&script], |text| {
text.contains("BURST-DONE")
});
let text = plain(&collected.text);
assert!(text.contains("BURST-DONE"), "burst never completed");
for probe in ["line-0", "line-2500", "line-4999"] {
Expand All @@ -192,14 +237,17 @@ fn a_large_burst_of_output_arrives_intact() {
fn shell_integration_markers_survive_the_round_trip() {
// Emitted by the shell, extracted by the tap, delivered on the chunk. If this
// breaks, Blocks silently stop forming.
let script = concat!(
r#"printf '\033]7373;cmd=ZWNobyBoaQ==\007';"#,
r#"printf '\033]133;C\007';"#,
"echo hi;",
r#"printf '\033]133;D;0\007';"#,
"echo MARKERS-DONE\n",
let script = format!(
concat!(
r#"printf '\033]7373;cmd=ZWNobyBoaQ==\007';"#,
r#"printf '\033]133;C\007';"#,
"echo hi;",
r#"printf '\033]133;D;0\007';"#,
"echo {}\n",
),
only_in_output("MARKERS-DONE"),
);
let collected = run("/bin/sh", &[], &[script], |text| {
let collected = run("/bin/sh", &[], &[&script], |text| {
text.contains("MARKERS-DONE")
});

Expand Down
Loading