feat(supervisor): let a server keep its stdin open
Stdio-first MCP servers exit the moment they see EOF on fd 0. Under the launchd agent the daemon's stdin is /dev/null, so a server that also speaks HTTP still shuts down seconds after binding its port, and there was no way to ask xy for anything else. Adds a per-server `stdin` mode: `inherit` (the default, unchanged), `null`, or `keep-open`. Under `keep-open` the child gets a pipe whose write end RealChild holds and never writes to, so a read blocks. The handle has to live on RealChild rather than on the TokioChild: `Child::wait()` opens with `drop(self.stdin.take())`, so leaving it where tokio put it reproduces the original bug the instant supervision starts. Refs #2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TWLFEoRaRafJm1SpdhWQ6F
This commit is contained in:
@@ -76,13 +76,18 @@ use nix::sys::signal::{Signal, kill};
|
||||
use nix::unistd::Pid;
|
||||
use std::process::Stdio;
|
||||
use tokio::io::{AsyncBufReadExt, BufReader};
|
||||
use tokio::process::{Child as TokioChild, Command};
|
||||
use xy_protocol::{ServerConfig, rpc::LogStream};
|
||||
use tokio::process::{Child as TokioChild, ChildStdin, Command};
|
||||
use xy_protocol::{ServerConfig, StdinMode, rpc::LogStream};
|
||||
|
||||
pub struct RealChild {
|
||||
pid: u32,
|
||||
pgid: Pid,
|
||||
child: Option<TokioChild>,
|
||||
/// Write end of the child's stdin pipe under [`StdinMode::KeepOpen`].
|
||||
/// Never written to; held so the child blocks on read rather than
|
||||
/// seeing EOF. `TokioChild::wait()` drops the handle it owns, so the
|
||||
/// pipe must live out here to outlast supervision.
|
||||
_stdin: Option<ChildStdin>,
|
||||
}
|
||||
|
||||
impl RealChild {
|
||||
@@ -132,6 +137,16 @@ pub fn spawn_with_logs(cfg: &ServerConfig, sink: LogSink) -> std::io::Result<Rea
|
||||
cmd.current_dir(dir);
|
||||
}
|
||||
|
||||
match cfg.stdin {
|
||||
StdinMode::Inherit => {}
|
||||
StdinMode::Null => {
|
||||
cmd.stdin(Stdio::null());
|
||||
}
|
||||
StdinMode::KeepOpen => {
|
||||
cmd.stdin(Stdio::piped());
|
||||
}
|
||||
}
|
||||
|
||||
cmd.stdout(Stdio::piped());
|
||||
cmd.stderr(Stdio::piped());
|
||||
cmd.kill_on_drop(true);
|
||||
@@ -149,6 +164,8 @@ pub fn spawn_with_logs(cfg: &ServerConfig, sink: LogSink) -> std::io::Result<Rea
|
||||
let pid = child.id().ok_or_else(|| std::io::Error::other("no pid"))?;
|
||||
let pgid = Pid::from_raw(pid as i32);
|
||||
|
||||
let stdin = child.stdin.take();
|
||||
|
||||
if let Some(out) = child.stdout.take() {
|
||||
spawn_pump(out, sink.clone(), LogStream::Stdout);
|
||||
}
|
||||
@@ -161,6 +178,7 @@ pub fn spawn_with_logs(cfg: &ServerConfig, sink: LogSink) -> std::io::Result<Rea
|
||||
pid,
|
||||
pgid,
|
||||
child: Some(child),
|
||||
_stdin: stdin,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -208,4 +226,65 @@ mod tests {
|
||||
child.terminate().unwrap();
|
||||
ctl.terminate_rx.try_recv().unwrap();
|
||||
}
|
||||
|
||||
use crate::logs::RotatingLogWriter;
|
||||
use std::time::Duration;
|
||||
use tempfile::tempdir;
|
||||
use xy_protocol::{RestartConfig, StdinMode, StopConfig};
|
||||
|
||||
fn reader_cfg(stdin: StdinMode, script: &str) -> ServerConfig {
|
||||
ServerConfig {
|
||||
name: "reader".to_string(),
|
||||
command: "/bin/sh".into(),
|
||||
args: vec!["-c".into(), script.to_string()],
|
||||
port: 1,
|
||||
env: Default::default(),
|
||||
working_dir: None,
|
||||
restart: RestartConfig::default(),
|
||||
stop: StopConfig::default(),
|
||||
wait_for: None,
|
||||
stdin,
|
||||
}
|
||||
}
|
||||
|
||||
fn test_sink() -> LogSink {
|
||||
let dir = tempdir().unwrap();
|
||||
let writer = RotatingLogWriter::open(&dir.path().join("s.log"), 1024, 3).unwrap();
|
||||
std::mem::forget(dir);
|
||||
LogSink::new("reader".to_string(), writer, 1024)
|
||||
}
|
||||
|
||||
/// Exits 9 straight away unless fd 0 is a FIFO, so the assertion below
|
||||
/// cannot be satisfied by whatever stdin the test runner happened to have.
|
||||
const REQUIRE_PIPE_THEN_READ: &str = "[ -p /dev/stdin ] || exit 9; read line; exit 7";
|
||||
|
||||
#[tokio::test]
|
||||
async fn keep_open_gives_the_child_a_pipe_that_survives_wait() {
|
||||
let cfg = reader_cfg(StdinMode::KeepOpen, REQUIRE_PIPE_THEN_READ);
|
||||
let mut child = spawn_with_logs(&cfg, test_sink()).unwrap();
|
||||
|
||||
// `TokioChild::wait()` drops its own stdin handle on entry, so this
|
||||
// blocks only if `RealChild` took the handle and is still holding it.
|
||||
let outcome = tokio::time::timeout(Duration::from_millis(500), child.wait()).await;
|
||||
|
||||
assert!(
|
||||
outcome.is_err(),
|
||||
"child should still be blocked on read, got {outcome:?}"
|
||||
);
|
||||
|
||||
child.kill().unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn null_stdin_child_sees_eof_immediately() {
|
||||
let cfg = reader_cfg(StdinMode::Null, "read line; exit 7");
|
||||
let mut child = spawn_with_logs(&cfg, test_sink()).unwrap();
|
||||
|
||||
let code = tokio::time::timeout(Duration::from_millis(2000), child.wait())
|
||||
.await
|
||||
.expect("child should exit promptly on EOF")
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(code, Some(7));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -536,6 +536,7 @@ mod tests {
|
||||
grace: Duration::from_millis(50),
|
||||
},
|
||||
wait_for: None,
|
||||
stdin: Default::default(),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user