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:
@@ -66,6 +66,19 @@ impl Default for StopConfig {
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "kebab-case")]
|
||||
pub enum StdinMode {
|
||||
/// The child inherits the daemon's stdin. Today's behaviour.
|
||||
#[default]
|
||||
Inherit,
|
||||
/// The child gets `/dev/null`; a read sees EOF immediately.
|
||||
Null,
|
||||
/// The child gets a pipe that `xy` holds open and never writes to,
|
||||
/// so a read blocks instead of seeing EOF.
|
||||
KeepOpen,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct ServerConfig {
|
||||
pub name: String,
|
||||
@@ -83,6 +96,8 @@ pub struct ServerConfig {
|
||||
pub stop: StopConfig,
|
||||
#[serde(default)]
|
||||
pub wait_for: Option<WaitForConfig>,
|
||||
#[serde(default)]
|
||||
pub stdin: StdinMode,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
use crate::{
|
||||
ConfigError, RestartConfig, RestartPolicy, ServerConfig, StopConfig, WaitCondition,
|
||||
ConfigError, RestartConfig, RestartPolicy, ServerConfig, StdinMode, StopConfig, WaitCondition,
|
||||
WaitForConfig, default_wait_interval, default_wait_timeout,
|
||||
};
|
||||
use kdl::{KdlDocument, KdlNode};
|
||||
@@ -31,6 +31,7 @@ pub fn parse_server_config(
|
||||
let restart = parse_restart(&doc, source_path)?;
|
||||
let stop = parse_stop(&doc, source_path)?;
|
||||
let wait_for = parse_wait_for(&doc, source_path)?;
|
||||
let stdin = parse_stdin(&doc, source_path)?;
|
||||
|
||||
Ok(ServerConfig {
|
||||
name: name.to_string(),
|
||||
@@ -42,6 +43,7 @@ pub fn parse_server_config(
|
||||
restart,
|
||||
stop,
|
||||
wait_for,
|
||||
stdin,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -214,6 +216,27 @@ fn parse_restart(doc: &KdlDocument, path: &Path) -> Result<RestartConfig, Config
|
||||
Ok(out)
|
||||
}
|
||||
|
||||
fn parse_stdin(doc: &KdlDocument, path: &Path) -> Result<StdinMode, ConfigError> {
|
||||
let Some(node) = find_node(doc, "stdin") else {
|
||||
return Ok(StdinMode::default());
|
||||
};
|
||||
|
||||
single_arg(node, "stdin", path)?;
|
||||
|
||||
let s = string_arg(node, "stdin", path)?;
|
||||
|
||||
match s.as_str() {
|
||||
"inherit" => Ok(StdinMode::Inherit),
|
||||
"null" => Ok(StdinMode::Null),
|
||||
"keep-open" => Ok(StdinMode::KeepOpen),
|
||||
other => Err(ConfigError::InvalidValue {
|
||||
path: path.to_path_buf(),
|
||||
field: "stdin",
|
||||
message: format!("unknown stdin mode `{other}`"),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_stop(doc: &KdlDocument, path: &Path) -> Result<StopConfig, ConfigError> {
|
||||
let Some(node) = find_node(doc, "stop") else {
|
||||
return Ok(StopConfig::default());
|
||||
@@ -810,4 +833,58 @@ stop {
|
||||
WaitCondition::Path(PathBuf::from("/x"))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stdin_defaults_to_inherit() {
|
||||
let text = "command \"/bin/x\"\nport 1";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(cfg.stdin, StdinMode::Inherit);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_stdin_inherit() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstdin \"inherit\"";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(cfg.stdin, StdinMode::Inherit);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_stdin_null() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstdin \"null\"";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(cfg.stdin, StdinMode::Null);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_stdin_keep_open() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstdin \"keep-open\"";
|
||||
let cfg = parse_server_config("foo", text, p()).unwrap();
|
||||
|
||||
assert_eq!(cfg.stdin, StdinMode::KeepOpen);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_stdin_mode_fails() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstdin \"maybe\"";
|
||||
let err = parse_server_config("foo", text, p()).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
err,
|
||||
ConfigError::InvalidValue { field: "stdin", .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stdin_with_two_args_fails() {
|
||||
let text = "command \"/bin/x\"\nport 1\nstdin \"null\" \"keep-open\"";
|
||||
let err = parse_server_config("foo", text, p()).unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
err,
|
||||
ConfigError::InvalidValue { field: "stdin", .. }
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,8 +7,8 @@ pub mod rpc;
|
||||
pub mod state;
|
||||
|
||||
pub use config::{
|
||||
RestartConfig, RestartPolicy, ServerConfig, StopConfig, WaitCondition, WaitForConfig,
|
||||
default_wait_interval, default_wait_timeout,
|
||||
RestartConfig, RestartPolicy, ServerConfig, StdinMode, StopConfig, WaitCondition,
|
||||
WaitForConfig, default_wait_interval, default_wait_timeout,
|
||||
};
|
||||
pub use error::{ConfigError, RpcErrorCode};
|
||||
pub use kdl_parse::{load_all_configs, parse_server_config};
|
||||
|
||||
@@ -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