mirror of
https://github.com/codewhale-hq/Codewhale.git
synced 2026-11-11 10:01:30 -10:00
fix(tui): keep SIGPIPE from killing the session when a stdio peer exits first
The dispatcher resets SIGPIPE to SIG_DFL so `codewhale doctor | head` exits quietly (#4030). The interactive session inherited that, and it writes to pipes whose far end it does not own: stdio MCP servers, shell tools, hooks, LSP. A stdio MCP server that exits before `initialize` is written kills the whole TUI on that write, with the terminal left in raw mode, nothing on stderr, and an empty runtime log. That is the Linux "blank frames, exited 1" startup: the PTY harness folds a signal death into exit code 1. Reproduced on the launch-card fixture (`/usr/bin/false` as a required MCP server) with a raw pty and `waitpid`: 6 of 12 launches died of SIGPIPE (13) about 0.4 s in, before the first frame; under strace the child was slow enough that the write won and every launch drew. `run_tui` now sets SIGPIPE to SIG_IGN once the terminal checks pass, so the failed write returns `EPIPE` and the MCP client reports the server as failed like any other transport error. Children are unaffected: the standard library resets SIGPIPE to SIG_DFL before exec, so `| head` inside a shell tool still terminates the way a shell expects. Non-TUI subcommands keep SIG_DFL. The QA PTY harness now prints the killing signal beside the mapped exit code (`observed_exit=Some(1) signal=Some("Broken pipe")`), so the next signal death does not read as a deliberate exit. Validation: the raw-pty probe above is the reproduction (6 of 12 launches killed by SIGPIPE on the unfixed binary, sha db798d8b). The post-fix probe and the `launch_mcp_summary_*` / `workbench_*_visual_evidence` PTY runs on the rebuilt binary are recorded on the PR. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_0134iUMxmGuXiG1LzPgfZVnv
This commit is contained in:
3 files changed
+34
-2
No files matched your search
@@ -636,6 +636,24 @@ pub async fn run_tui(
|
||||
require_interactive_terminal(io::stdin().is_terminal(), io::stdout().is_terminal())?;
|
||||
require_foreground_terminal_owner()?;
|
||||
|
||||
// The dispatcher resets SIGPIPE to SIG_DFL so `codewhale doctor | head`
|
||||
// exits quietly (#4030). A full-screen session is the opposite case: it
|
||||
// writes to pipes whose far end it does not own — stdio MCP servers, shell
|
||||
// tools, hooks, LSP — and a peer that exits first must surface as an
|
||||
// `EPIPE` error on that one write, not kill the whole TUI with the terminal
|
||||
// left in raw mode and nothing in the runtime log. Reproduced with a stdio
|
||||
// MCP server that exits before `initialize` is written: the process died
|
||||
// of SIGPIPE before its first frame, and the PTY harness reported it as a
|
||||
// plain exit 1. Children are unaffected: the standard library resets
|
||||
// SIGPIPE to SIG_DFL before exec, so `| head` inside a shell tool still
|
||||
// terminates the way a shell expects. Non-TUI subcommands keep SIG_DFL.
|
||||
// SAFETY: a plain disposition change, no handler; it runs before this
|
||||
// session spawns anything that writes to a pipe.
|
||||
#[cfg(unix)]
|
||||
unsafe {
|
||||
libc::signal(libc::SIGPIPE, libc::SIG_IGN);
|
||||
}
|
||||
|
||||
// #6169: install the suspend/resume handshake here — after the
|
||||
// foreground-ownership check (the termios snapshot needs the still-cooked
|
||||
// tty) and before raw mode, so every mode enabled below has a handler that
|
||||
|
||||
@@ -329,6 +329,7 @@ impl Harness {
|
||||
let transcript = self.pty.transcript();
|
||||
let pid = self.pty.pid();
|
||||
let exit = self.pty.wait_until(Instant::now());
|
||||
let signal = self.pty.signal().map(str::to_owned);
|
||||
let program = self.program.clone();
|
||||
let diagnostic_root = self.diagnostic_root.clone();
|
||||
let sealed_home = self.sealed_home.clone();
|
||||
@@ -348,7 +349,7 @@ impl Harness {
|
||||
let destination = diagnostic_root
|
||||
.join(format!("{}-{nonce}", pid.unwrap_or_default()));
|
||||
let mut report = format!(
|
||||
"program={:?} host={}/{} pid={pid:?} observed_exit={exit:?} wait_budget={budget:?} parent_CI={} {}\nPTY bytes={}\n",
|
||||
"program={:?} host={}/{} pid={pid:?} observed_exit={exit:?} signal={signal:?} wait_budget={budget:?} parent_CI={} {}\nPTY bytes={}\n",
|
||||
program,
|
||||
std::env::consts::OS,
|
||||
std::env::consts::ARCH,
|
||||
|
||||
@@ -29,6 +29,10 @@ pub struct PtySession {
|
||||
/// no trace on the rendered screen at all.
|
||||
transcript: Arc<Mutex<Vec<u8>>>,
|
||||
reader_handle: Option<JoinHandle<()>>,
|
||||
/// The signal that killed the child, when `wait_until` reaped one.
|
||||
/// `portable_pty` folds a signal death into exit code 1, which reads as a
|
||||
/// deliberate `exit(1)`; a SIGPIPE death spent a debugging session that way.
|
||||
signal: Option<String>,
|
||||
}
|
||||
|
||||
pub struct PtySessionBuilder<'a> {
|
||||
@@ -158,6 +162,7 @@ impl<'a> PtySessionBuilder<'a> {
|
||||
buffer,
|
||||
transcript,
|
||||
reader_handle: Some(reader_handle),
|
||||
signal: None,
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -171,6 +176,11 @@ impl PtySession {
|
||||
self.child.process_id()
|
||||
}
|
||||
|
||||
/// The signal that killed the child, once it has been reaped.
|
||||
pub fn signal(&self) -> Option<&str> {
|
||||
self.signal.as_deref()
|
||||
}
|
||||
|
||||
pub fn write_bytes(&mut self, bytes: &[u8]) -> Result<()> {
|
||||
self.writer.write_all(bytes).context("pty write")?;
|
||||
self.writer.flush().context("pty flush")?;
|
||||
@@ -210,7 +220,10 @@ impl PtySession {
|
||||
pub fn wait_until(&mut self, deadline: Instant) -> Option<i32> {
|
||||
loop {
|
||||
match self.child.try_wait() {
|
||||
Ok(Some(status)) => return Some(status.exit_code() as i32),
|
||||
Ok(Some(status)) => {
|
||||
self.signal = status.signal().map(str::to_owned);
|
||||
return Some(status.exit_code() as i32);
|
||||
}
|
||||
Ok(None) => {}
|
||||
Err(_) => return None,
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user