mimir mcp: exit when the client dies, not when stdin says so

CHANGELOG.md

@@ -3,6 +3,23 @@
 All notable changes are documented here. Versions follow semver; the CLI,
 the `mimir-mem` crate, and the on-disk schema move together.
 
+## [Unreleased]
+### Fixed
+- **`mimir mcp` no longer outlives the client that spawned it.** Fourteen
+  orphaned stdio servers had accumulated on one machine, the oldest fourteen
+  days old, ~2.9 GB resident between them, every one holding the SQLite
+  database open. The EOF handling was never at fault — rmcp returns `None` on
+  a zero-length read and the process exits in 0.3 s when stdin closes. The
+  EOF simply never arrived: MCP stdio is an `AF_UNIX` **socketpair**, not a
+  pipe, and a socketpair delivers EOF only once *every* descriptor for the
+  peer end is closed, so any unrelated process that inherited it pins the
+  server open forever. The trigger therefore cannot be a descriptor:
+  `PR_SET_PDEATHSIG` (Linux) plus a `getppid()` poll (macOS, and to close the
+  arming race) now end the process when its parent does, and SIGTERM/SIGHUP
+  route through the same path — cancel the service, drain in flight, drop the
+  engine and its database handle. `--http` is untouched: `mimir daemon` has
+  its own lifetime and arms none of this.
+
 ## [Unreleased]
 ### Added
 - **`mimir link --scan --all-projects`, and a `doctor` nudge to run it.**

crates/mimir-cli/src/main.rs

@@ -661,6 +661,12 @@ fn main() {
         libc::signal(libc::SIGPIPE, libc::SIG_DFL);
     }
 
+    // Before anything slow. The stdio MCP server uses this to notice that the
+    // client that spawned it has died; read any later and a client that exits
+    // during startup is already gone, leaving nothing to compare against.
+    #[cfg(unix)]
+    mcp::record_parent_pid();
+
     // Windows hands the main thread a 1 MiB stack (Linux/macOS give 8).
     // clap's derived builder for our ~50 subcommands needs more than that
     // in debug builds, so `mimir --version` aborted with a stack overflow

crates/mimir-cli/src/mcp.rs

@@ -1058,16 +1058,151 @@ pub fn run(
                 if !daemon_alive {
                     eager_load_reranker_if_auto(&mut engine);
                 }
+                // Armed BEFORE `serve`, which awaits the MCP `initialize`
+                // handshake and therefore blocks indefinitely against a client
+                // that spawns the server and dies without ever initializing —
+                // an orphan window that arming afterwards cannot see. Stdio
+                // only: the HTTP path is a daemon with its own lifetime.
+                #[cfg(unix)]
+                arm_shutdown_watchdog();
                 let service = MimirServer::new(engine, project_id)
                     .serve(rmcp::transport::stdio())
                     .await?;
+                #[cfg(unix)]
+                register_shutdown_token(service.cancellation_token());
                 service.waiting().await?;
                 Ok(())
             }
         }
     })
 }
 
+/// The parent pid as it was at process start.
+///
+/// Captured in `main`, microseconds after exec, and deliberately not where
+/// it is used. The stdio server spends a second or two loading models before
+/// it starts serving, and a client that exits during that window is already
+/// gone by then — a ppid read taken at that point returns the *reaper*, which
+/// then compares equal to itself forever and the watchdog never fires. That
+/// is not hypothetical: it is what the first version of this did, and the
+/// regression test caught it.
+#[cfg(unix)]
+static PARENT_AT_START: std::sync::atomic::AtomicI32 = std::sync::atomic::AtomicI32::new(0);
+
+/// Record the parent pid. Call once, as early in `main` as possible.
+#[cfg(unix)]
+pub fn record_parent_pid() {
+    PARENT_AT_START.store(
+        unsafe { libc::getppid() },
+        std::sync::atomic::Ordering::Relaxed,
+    );
+}
+
+/// Set by the SIGTERM/SIGHUP handler. A relaxed store is the whole handler
+/// body on purpose — it is the only thing here that is async-signal-safe.
+#[cfg(unix)]
+static SHUTDOWN_REQUESTED: std::sync::atomic::AtomicBool =
+    std::sync::atomic::AtomicBool::new(false);
+
+#[cfg(unix)]
+extern "C" fn note_shutdown(_sig: libc::c_int) {
+    SHUTDOWN_REQUESTED.store(true, std::sync::atomic::Ordering::Relaxed);
+}
+
+/// Exit when the client goes away, for the cases where stdin EOF never comes.
+///
+/// The stdio server's EOF handling is correct and needs no help: rmcp's
+/// transport returns `None` on a zero-length read and the service task ends.
+/// The problem is that the EOF can simply never arrive. Claude Code wires MCP
+/// stdio over an `AF_UNIX` **socketpair**, not a pipe, and a socketpair
+/// delivers EOF only once *every* descriptor for the peer end is closed. Any
+/// unrelated process that inherited that descriptor — a background command,
+/// another server spawned by the same client — pins it open, so when the
+/// client exits the kernel delivers nothing at all and the reader blocks
+/// forever. Measured before this existed: fourteen orphans, the oldest
+/// fourteen days old, ~2.9 GB resident between them, each holding the SQLite
+/// database open.
+///
+/// So the trigger cannot be a descriptor. It has to be the parent dying:
+///
+/// * `PR_SET_PDEATHSIG` (Linux) has the kernel send us SIGTERM the moment the
+///   parent does, regardless of who else holds the socket.
+/// * A `getppid()` poll covers macOS, which has no equivalent, and closes the
+///   race where the parent died in the window *before* `prctl` ran — that
+///   signal is never sent, so nothing but a check would catch it.
+///
+/// The poll compares against the ppid captured at startup rather than testing
+/// for `1`, because a server legitimately started by pid 1 would otherwise
+/// shut itself down immediately.
+///
+/// Cancelling the token ends the service task, which resolves the `waiting()`
+/// the caller is already parked on, so shutdown runs through exactly the same
+/// path as a clean EOF: in-flight work finishes, the transport closes, the
+/// engine drops and with it the database handle.
+/// Where `arm_shutdown_watchdog` finds the token once there is one. Empty
+/// until the handshake completes.
+#[cfg(unix)]
+static SHUTDOWN_TOKEN: std::sync::Mutex<Option<rmcp::service::RunningServiceCancellationToken>> =
+    std::sync::Mutex::new(None);
+
+/// Hand the watchdog a way to shut the service down gracefully. Before this,
+/// it can only exit the process — which is correct, because before the
+/// handshake there is no service to drain.
+#[cfg(unix)]
+fn register_shutdown_token(token: rmcp::service::RunningServiceCancellationToken) {
+    *SHUTDOWN_TOKEN.lock().unwrap() = Some(token);
+}
+
+#[cfg(unix)]
+fn arm_shutdown_watchdog() {
+    use std::sync::atomic::Ordering;
+
+    #[cfg(target_os = "linux")]
+    // SAFETY: prctl with PR_SET_PDEATHSIG only sets this process's
+    // parent-death signal; it touches no memory we own.
+    unsafe {
+        libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM);
+    }
+
+    let handler = note_shutdown as extern "C" fn(libc::c_int);
+    // SAFETY: installing a handler whose body is a single relaxed atomic
+    // store, which is async-signal-safe.
+    unsafe {
+        libc::signal(libc::SIGTERM, handler as libc::sighandler_t);
+        libc::signal(libc::SIGHUP, handler as libc::sighandler_t);
+    }
+
+    // Recorded in `main`, not here — see PARENT_AT_START. A zero means the
+    // recorder was never called; treat that as "cannot tell" and rely on the
+    // signal paths rather than shutting down on a bogus comparison.
+    let parent_at_start = PARENT_AT_START.load(Ordering::Relaxed);
+
+    std::thread::spawn(move || {
+        loop {
+            if SHUTDOWN_REQUESTED.load(Ordering::Relaxed) {
+                break;
+            }
+            if parent_at_start != 0 && unsafe { libc::getppid() } != parent_at_start {
+                break;
+            }
+            std::thread::sleep(std::time::Duration::from_millis(250));
+        }
+        // A token exists only after the handshake. Before that there is no
+        // service, no in-flight request and nothing to drain, so exiting is
+        // the whole of a correct shutdown.
+        match SHUTDOWN_TOKEN.lock().unwrap().take() {
+            Some(token) => token.cancel(),
+            None => std::process::exit(0),
+        }
+        // Backstop. Installing a SIGTERM handler means the default "die now"
+        // action is gone, so a wedged shutdown would leave a process that
+        // ignores SIGTERM — strictly worse than the orphan this fixes. Give
+        // the graceful path time to finish, then leave anyway.
+        std::thread::sleep(std::time::Duration::from_secs(10));
+        std::process::exit(0);
+    });
+}
+
 /// Serve the same MCP tools over Streamable-HTTP for remote clients reached
 /// through a tunnel / reverse proxy. Each session gets its own engine (its own
 /// SQLite connection; WAL handles the concurrency). The loopback bind is the

crates/mimir-cli/tests/e2e.rs

@@ -1236,3 +1236,139 @@ fn session_brief_fires_suppresses_and_respects_kill_switch() {
     let preview = h.ok(&["brief"]);
     assert!(preview.contains("candidates (score desc"), "{preview}");
 }
+
+/// Shutdown of the stdio server (`mimir mcp`).
+///
+/// These exist because fourteen orphaned servers accumulated on a real
+/// machine, the oldest fourteen days old, each holding the SQLite database
+/// open. The EOF path was never the bug — it works, and the first test here
+/// pins that — so a fix that only hardened EOF handling would have changed
+/// nothing. The other two cover the ways the client can vanish *without*
+/// producing an EOF.
+#[cfg(unix)]
+mod shutdown {
+    use super::*;
+    use std::process::Stdio;
+    use std::time::{Duration, Instant};
+
+    /// Poll for exit rather than `wait()`, so a hang fails the test instead
+    /// of hanging the suite.
+    fn exits_within(child: &mut std::process::Child, secs: u64) -> bool {
+        let deadline = Instant::now() + Duration::from_secs(secs);
+        while Instant::now() < deadline {
+            if child.try_wait().unwrap().is_some() {
+                return true;
+            }
+            std::thread::sleep(Duration::from_millis(100));
+        }
+        let _ = child.kill();
+        let _ = child.wait();
+        false
+    }
+
+    fn spawn_server(h: &Harness) -> std::process::Child {
+        Command::new(env!("CARGO_BIN_EXE_mimir"))
+            .arg("mcp")
+            .env("MIMIR_HOME", h.home.path())
+            .env("HOME", h.home.path())
+            .current_dir(h.cwd.path())
+            .stdin(Stdio::piped())
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .spawn()
+            .expect("spawns")
+    }
+
+    #[test]
+    fn exits_when_stdin_closes() {
+        let h = Harness::new();
+        h.ok(&["init", "--no-model"]);
+        let mut child = spawn_server(&h);
+        drop(child.stdin.take()); // client closed the pipe
+        assert!(
+            exits_within(&mut child, 10),
+            "server outlived its stdin closing"
+        );
+    }
+
+    /// The client is gone but something else still holds the descriptor, so
+    /// no EOF is ever delivered. Before the parent-death watchdog this ran
+    /// forever.
+    ///
+    /// `sh` spawns the server and exits immediately, which reparents it
+    /// exactly as a departing client does — after staying alive for a few
+    /// seconds first, which is the real shape of the bug: a client serves a
+    /// session and *then* exits. (A parent that dies during the server's own
+    /// startup is a genuine residual gap, documented on `watch_for_shutdown`;
+    /// it is a millisecond-wide window and not what produced the orphans.)
+    /// The server's stdin stays the pipe
+    /// whose write end this test holds for the whole run, so EOF cannot be
+    /// what ends it; the pid goes through a file rather than stdout, because
+    /// the server inherits `sh`'s stdout and reading that to EOF would block
+    /// until the server had already exited — which silently made an earlier
+    /// version of this test pass against the unfixed binary.
+    #[test]
+    fn exits_when_the_parent_dies_without_eof() {
+        let h = Harness::new();
+        h.ok(&["init", "--no-model"]);
+        let pidfile = h.cwd.path().join("server.pid");
+        let mut launcher = Command::new("sh")
+            .arg("-c")
+            // `exec 3<&0` then `<&3`: a non-interactive shell redirects a
+            // background job's stdin from /dev/null, which would hand the
+            // server an instant EOF and make this test pass against an
+            // unfixed binary. Duplicating the descriptor defeats that.
+            .arg(format!(
+                "exec 3<&0; {} mcp <&3 >/dev/null 2>&1 & echo $! > {}; sleep 4",
+                env!("CARGO_BIN_EXE_mimir"),
+                pidfile.display()
+            ))
+            .env("MIMIR_HOME", h.home.path())
+            .env("HOME", h.home.path())
+            .current_dir(h.cwd.path())
+            .stdin(Stdio::piped())
+            .stdout(Stdio::null())
+            .stderr(Stdio::null())
+            .spawn()
+            .expect("spawns");
+        let stdin_holder = launcher.stdin.take().expect("piped"); // held open
+        launcher.wait().unwrap(); // sh exits; the server is reparented
+
+        let pid: i32 = std::fs::read_to_string(&pidfile)
+            .expect("pidfile")
+            .trim()
+            .parse()
+            .expect("pid");
+
+        let deadline = Instant::now() + Duration::from_secs(15);
+        let mut gone = false;
+        while Instant::now() < deadline {
+            // signal 0 probes liveness without delivering anything
+            if unsafe { libc::kill(pid, 0) } != 0 {
+                gone = true;
+                break;
+            }
+            std::thread::sleep(Duration::from_millis(100));
+        }
+        drop(stdin_holder);
+        if !gone {
+            unsafe { libc::kill(pid, libc::SIGKILL) };
+        }
+        assert!(gone, "server survived its parent with stdin still open");
+    }
+
+    #[test]
+    fn exits_on_sigterm() {
+        let h = Harness::new();
+        h.ok(&["init", "--no-model"]);
+        let mut child = spawn_server(&h);
+        let _stdin = child.stdin.take(); // keep it open: no EOF
+        std::thread::sleep(Duration::from_millis(500));
+        unsafe { libc::kill(child.id() as i32, libc::SIGTERM) };
+        assert!(
+            exits_within(&mut child, 10),
+            "server ignored SIGTERM — installing a handler removed the \
+             default action, so this must work"
+        );
+    }
+}