diff --git a/Cargo.lock b/Cargo.lock index 88ecaa0b..e2e22ceb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -662,8 +662,10 @@ dependencies = [ "schemars", "serde", "serde_json", + "tempfile", "tokio", "tracing", + "windows-sys 0.60.2", ] [[package]] diff --git a/crates/fff-mcp/Cargo.toml b/crates/fff-mcp/Cargo.toml index 636f94bf..00b1abc4 100644 --- a/crates/fff-mcp/Cargo.toml +++ b/crates/fff-mcp/Cargo.toml @@ -30,3 +30,13 @@ tokio = { version = "1", features = ["full"] } tracing = { workspace = true } git2 = { workspace = true } clap = { version = "4", features = ["derive", "env"] } + +[target.'cfg(windows)'.dependencies] +windows-sys = { version = "0.60", features = [ + "Win32_Foundation", + "Win32_System_Threading", + "Win32_System_Diagnostics_ToolHelp", +] } + +[dev-dependencies] +tempfile = "3.8" diff --git a/crates/fff-mcp/src/main.rs b/crates/fff-mcp/src/main.rs index 17772820..3635f211 100644 --- a/crates/fff-mcp/src/main.rs +++ b/crates/fff-mcp/src/main.rs @@ -1,9 +1,12 @@ mod cursor; mod healthcheck; mod output; +mod parent; mod server; mod update_check; +use std::time::{Duration, SystemTime}; + use clap::Parser; use fff::file_picker::FilePicker; use fff::frecency::FrecencyTracker; @@ -92,7 +95,7 @@ pub const MCP_INSTRUCTIONS: &str = concat!( " !generated/ - exclude generated code", ); -/// FFF MCP Server -- a high performance & accuracy file finder for AI code assistants. +/// FFF MCP Server - a high performance & accuracy file finder for AI code assistants. #[derive(Parser)] #[command(name = "fff-mcp", version = concat!(env!("CARGO_PKG_VERSION"), " (", env!("FFF_GIT_HASH"), ")"))] pub(crate) struct Args { @@ -131,6 +134,7 @@ pub(crate) struct Args { /// Disable the content index built after the initial scan. /// This makes grep calls slower but consumes less RAM (recommended to not turn off) + #[arg(long = "no-content-indexing")] no_content_indexing: bool, /// Explicitly enable content indexing even when `--no-warmup` is set. @@ -158,11 +162,12 @@ pub(crate) struct Args { #[arg(long = "healthcheck")] pub(crate) healthcheck: bool, - /// Exit after this many seconds of inactivity. 0 = never exit. + /// Timeout of inactivity after which fff mcp will be exited. Even if the parent process + /// is alive we don't want to occupy resources on index and file watches if fff is unused #[arg( long = "idle-timeout-secs", env = "FFF_MCP_IDLE_TIMEOUT_SECS", - default_value_t = 900 + default_value_t = 60 * 60 )] idle_timeout_secs: u64, } @@ -317,9 +322,20 @@ async fn main() -> Result<(), Box> { } }; - if idle_timeout_secs > 0 { + let parent_watcher = parent::ParentWatcher::new(); + match &parent_watcher { + Some(watcher) => tracing::info!( + "Watching parent process (pid {}); will exit when it dies", + watcher.parent_pid() + ), + None => tracing::warn!( + "Parent process liveness detection unavailable; idle timeout will exit unconditionally" + ), + } + + if idle_timeout_secs > 0 || parent_watcher.is_some() { last_activity.store( - std::time::SystemTime::now() + SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) .unwrap_or(0), @@ -328,9 +344,27 @@ async fn main() -> Result<(), Box> { let last_activity_for_watchdog = last_activity.clone(); tokio::spawn(async move { - let tick = std::time::Duration::from_secs(60); + let tick = watchdog_interval(); loop { tokio::time::sleep(tick).await; + + if let Some(ref watcher) = parent_watcher { + if !watcher.parent_alive() { + tracing::info!( + "Parent process (pid {}) exited, shutting down", + watcher.parent_pid() + ); + flush_logs_and_exit().await; + } + // Parent is alive: it owns our lifecycle, never exit on idle + // Clients like Codex do not restart MCP servers @see #703 + continue; + } + + if idle_timeout_secs == 0 { + continue; + } + let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map(|d| d.as_secs()) @@ -338,12 +372,8 @@ async fn main() -> Result<(), Box> { let last = last_activity_for_watchdog.load(std::sync::atomic::Ordering::Relaxed); if now.saturating_sub(last) >= idle_timeout_secs { - tracing::info!( - "Exiting after {}s of inactivity (idle_timeout_secs={})", - now.saturating_sub(last), - idle_timeout_secs - ); - std::process::exit(0); + tracing::info!(?idle_timeout_secs, "Exiting due to inactivity",); + flush_logs_and_exit().await; } } }); @@ -370,3 +400,20 @@ async fn main() -> Result<(), Box> { Ok(()) } + +// Tracing appender is non blocking, to get full log give it some time before hard exit +async fn flush_logs_and_exit() -> ! { + tokio::time::sleep(std::time::Duration::from_millis(250)).await; + std::process::exit(0); +} + +fn watchdog_interval() -> Duration { + if cfg!(debug_assertions) + && let Some(milliseconds) = std::env::var("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS") + .ok() + .and_then(|value| value.parse().ok()) + { + return Duration::from_millis(milliseconds); + } + Duration::from_secs(60) +} diff --git a/crates/fff-mcp/src/parent.rs b/crates/fff-mcp/src/parent.rs new file mode 100644 index 00000000..b05af787 --- /dev/null +++ b/crates/fff-mcp/src/parent.rs @@ -0,0 +1,99 @@ +#[cfg(unix)] +mod imp { + pub struct ParentWatcher { + ppid: u32, + } + + impl ParentWatcher { + pub fn new() -> Option { + let ppid = std::os::unix::process::parent_id(); + // ppid <= 1 means we were spawned by init and can't detect death + (ppid > 1).then_some(Self { ppid }) + } + + pub fn parent_pid(&self) -> u32 { + self.ppid + } + + // When the parent dies the kernel reparents us, so getppid() changes. + // Race-free and immune to PID reuse, unlike kill(ppid, 0). + pub fn parent_alive(&self) -> bool { + std::os::unix::process::parent_id() == self.ppid + } + } +} + +#[cfg(windows)] +mod imp { + use windows_sys::Win32::Foundation::{CloseHandle, HANDLE, INVALID_HANDLE_VALUE, WAIT_TIMEOUT}; + use windows_sys::Win32::System::Diagnostics::ToolHelp::{ + CreateToolhelp32Snapshot, PROCESSENTRY32, Process32First, Process32Next, TH32CS_SNAPPROCESS, + }; + use windows_sys::Win32::System::Threading::{ + GetCurrentProcessId, OpenProcess, PROCESS_SYNCHRONIZE, WaitForSingleObject, + }; + + pub struct ParentWatcher { + handle: HANDLE, + ppid: u32, + } + + // HANDLE is a raw pointer; it is only ever used via WaitForSingleObject + // which is thread-safe, so moving/sharing the watcher across threads is fine. + unsafe impl Send for ParentWatcher {} + unsafe impl Sync for ParentWatcher {} + + impl ParentWatcher { + pub fn new() -> Option { + let ppid = parent_pid_of_current()?; + let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE, 0, ppid) }; + if handle.is_null() { + return None; + } + // Holding the handle pins the PID, preventing reuse for the process lifetime + Some(Self { handle, ppid }) + } + + pub fn parent_pid(&self) -> u32 { + self.ppid + } + + pub fn parent_alive(&self) -> bool { + unsafe { WaitForSingleObject(self.handle, 0) == WAIT_TIMEOUT } + } + } + + impl Drop for ParentWatcher { + fn drop(&mut self) { + unsafe { CloseHandle(self.handle) }; + } + } + + fn parent_pid_of_current() -> Option { + unsafe { + let snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); + if snapshot == INVALID_HANDLE_VALUE { + return None; + } + let mut entry: PROCESSENTRY32 = std::mem::zeroed(); + entry.dwSize = std::mem::size_of::() as u32; + let current = GetCurrentProcessId(); + let mut found = None; + if Process32First(snapshot, &mut entry) != 0 { + loop { + if entry.th32ProcessID == current { + found = Some(entry.th32ParentProcessID); + break; + } + if Process32Next(snapshot, &mut entry) == 0 { + break; + } + } + } + CloseHandle(snapshot); + found + } + } +} + +pub use imp::ParentWatcher; diff --git a/crates/fff-mcp/tests/parent_liveness.rs b/crates/fff-mcp/tests/parent_liveness.rs new file mode 100644 index 00000000..305b2a12 --- /dev/null +++ b/crates/fff-mcp/tests/parent_liveness.rs @@ -0,0 +1,181 @@ +use std::io::{BufRead, BufReader, Write}; +use std::process::{Child, ChildStdin, Command, Stdio}; +use std::sync::mpsc; +use std::time::{Duration, Instant}; + +const BIN: &str = env!("CARGO_BIN_EXE_fff-mcp"); + +#[test] +fn stays_alive_while_parent_alive_despite_idle_timeout() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.txt"), "hello").unwrap(); + + let mut child = Command::new(BIN) + .arg(dir.path()) + .args([ + "--no-update-check", + "--no-warmup", + "--no-watch", + "--idle-timeout-secs", + "1", + ]) + .arg("--log-file") + .arg(dir.path().join("test.log")) + .env("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS", "100") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + + let mut stdin = child.stdin.take().unwrap(); + let stdout_lines = spawn_line_reader(child.stdout.take().unwrap()); + do_handshake(&mut stdin, &stdout_lines); + + // Wait past the idle timeout and several watchdog ticks. + std::thread::sleep(Duration::from_secs(2)); + assert!( + child.try_wait().unwrap().is_none(), + "fff-mcp exited on idle timeout even though its parent is alive" + ); + + // Closing stdin ends the transport; the server must still shut down cleanly. + drop(stdin); + wait_for_exit(&mut child, Duration::from_secs(15)); +} + +#[cfg(unix)] +#[test] +fn exits_when_parent_dies_even_without_idle_timeout() { + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("a.txt"), "hello").unwrap(); + let log_path = dir.path().join("test.log"); + let exit_signal = dir.path().join("exit-parent"); + + // Intermediary parent: sh backgrounds fff-mcp and waits until the handshake + // completes before dying and orphaning it. + let mut sh = Command::new("sh") + .arg("-c") + .arg( + // Preserve stdin before POSIX shells assign /dev/null to background jobs. + r#"exec 3<&0 + "$1" "$2" --no-update-check --no-warmup --no-watch \ + --idle-timeout-secs 0 --log-file "$3" <&3 & + while [ ! -e "$4" ]; do sleep 0.1; done"#, + ) + .arg("sh") + .arg(BIN) + .arg(dir.path()) + .arg(&log_path) + .arg(&exit_signal) + .env("FFF_MCP_TEST_WATCHDOG_INTERVAL_MS", "100") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::null()) + .spawn() + .unwrap(); + + let mut stdin = sh.stdin.take().unwrap(); + let stdout_lines = spawn_line_reader(sh.stdout.take().unwrap()); + do_handshake(&mut stdin, &stdout_lines); + + std::fs::write(exit_signal, "").unwrap(); + sh.wait().unwrap(); + + // We still hold the stdin write end, so the only exit path is the parent + // liveness check. EOF on stdout means fff-mcp closed it by exiting. + let deadline = Instant::now() + Duration::from_secs(5); + loop { + match stdout_lines.recv_timeout(deadline.saturating_duration_since(Instant::now())) { + Ok(_) => continue, + Err(mpsc::RecvTimeoutError::Disconnected) => break, + Err(mpsc::RecvTimeoutError::Timeout) => { + panic!("fff-mcp did not exit within 5s of its parent dying") + } + } + } + drop(stdin); + + let logs = read_session_logs(dir.path()); + assert!( + logs.contains("Parent process") && logs.contains("exited, shutting down"), + "expected parent-death exit reason in logs, got:\n{}", + logs + ); +} + +fn do_handshake(stdin: &mut ChildStdin, stdout_lines: &mpsc::Receiver) { + let initialize = serde_json::json!({ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": { + "protocolVersion": "2024-11-05", + "capabilities": {}, + "clientInfo": { "name": "parent-liveness-test", "version": "0.0.0" } + } + }); + writeln!(stdin, "{}", initialize).unwrap(); + stdin.flush().unwrap(); + + let response = stdout_lines + .recv_timeout(Duration::from_secs(30)) + .expect("no initialize response within 30s"); + assert!( + response.contains("\"serverInfo\""), + "unexpected initialize response: {}", + response + ); + + writeln!( + stdin, + "{}", + serde_json::json!({ "jsonrpc": "2.0", "method": "notifications/initialized" }) + ) + .unwrap(); + stdin.flush().unwrap(); +} + +fn spawn_line_reader(stdout: std::process::ChildStdout) -> mpsc::Receiver { + let (tx, rx) = mpsc::channel(); + std::thread::spawn(move || { + for line in BufReader::new(stdout).lines() { + match line { + Ok(line) => { + if tx.send(line).is_err() { + break; + } + } + Err(_) => break, + } + } + }); + rx +} + +fn wait_for_exit(child: &mut Child, timeout: Duration) { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if child.try_wait().unwrap().is_some() { + return; + } + std::thread::sleep(Duration::from_millis(100)); + } + child.kill().ok(); + panic!( + "fff-mcp did not exit within {:?} after stdin closed", + timeout + ); +} + +#[cfg(unix)] +fn read_session_logs(dir: &std::path::Path) -> String { + let mut combined = String::new(); + for entry in std::fs::read_dir(dir).unwrap().flatten() { + let name = entry.file_name().to_string_lossy().to_string(); + if name.starts_with("test") && name.ends_with(".log") { + combined.push_str(&std::fs::read_to_string(entry.path()).unwrap_or_default()); + } + } + combined +}