From 7a58ab4e2b9542b58337cf4e6879c3250f472456 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 22 Apr 2026 16:07:52 -0400 Subject: [PATCH] refactor(server): unify daemon runtime metadata --- lib/crates/fabro-cli/src/args.rs | 4 - lib/crates/fabro-cli/src/commands/install.rs | 41 ++-- .../src/commands/server/foreground.rs | 22 +-- .../fabro-cli/src/commands/server/mod.rs | 3 - .../fabro-cli/src/commands/server/record.rs | 166 ---------------- .../fabro-cli/src/commands/server/start.rs | 72 +++---- .../fabro-cli/src/commands/server/status.rs | 21 +- .../fabro-cli/src/commands/server/stop.rs | 20 +- .../fabro-cli/src/commands/uninstall.rs | 9 +- lib/crates/fabro-cli/src/main.rs | 8 +- lib/crates/fabro-cli/tests/it/cmd/install.rs | 2 +- lib/crates/fabro-cli/tests/it/cmd/ps.rs | 2 +- .../fabro-cli/tests/it/cmd/server_start.rs | 86 +-------- .../fabro-cli/tests/it/cmd/server_status.rs | 86 +-------- lib/crates/fabro-cli/tests/it/cmd/support.rs | 25 +-- .../fabro-cli/tests/it/cmd/uninstall.rs | 96 +-------- .../tests/it/scenario/server_lifecycle.rs | 2 +- lib/crates/fabro-config/src/lib.rs | 2 +- lib/crates/fabro-config/src/storage.rs | 12 +- lib/crates/fabro-install/src/lib.rs | 4 +- lib/crates/fabro-server/src/daemon.rs | 182 ++++++++++++++++++ lib/crates/fabro-server/src/install.rs | 2 +- lib/crates/fabro-server/src/lib.rs | 1 + lib/crates/fabro-server/src/serve.rs | 2 +- lib/crates/fabro-server/src/server.rs | 41 ++-- .../fabro-server/tests/it/api/install.rs | 13 +- lib/crates/fabro-server/tests/it/api/tcp.rs | 4 +- lib/crates/fabro-test/src/lib.rs | 4 +- 28 files changed, 338 insertions(+), 594 deletions(-) delete mode 100644 lib/crates/fabro-cli/src/commands/server/record.rs create mode 100644 lib/crates/fabro-server/src/daemon.rs diff --git a/lib/crates/fabro-cli/src/args.rs b/lib/crates/fabro-cli/src/args.rs index 13bd10780..b8c92fddb 100644 --- a/lib/crates/fabro-cli/src/args.rs +++ b/lib/crates/fabro-cli/src/args.rs @@ -1301,10 +1301,6 @@ pub(crate) struct ServerServeArgs { #[command(flatten)] pub(crate) storage_dir: StorageDirArgs, - /// Path to the server record file - #[arg(long)] - pub(crate) record_path: PathBuf, - #[command(flatten)] pub(crate) serve_args: ServeArgs, } diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 4bc2dc8aa..4b0bc7e01 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -29,6 +29,7 @@ use fabro_install::{ }; use fabro_model::Provider; use fabro_server::bind::Bind; +use fabro_server::daemon::ServerDaemon; use fabro_server::serve; use fabro_store::ArtifactStore; use fabro_types::settings::cli::{CliLayer, OutputFormat}; @@ -51,7 +52,7 @@ use crate::args::{ DoctorArgs, InstallArgs, InstallCommand, InstallGitHubStrategyArg, InstallGithubArgs, InstallNonInteractiveArgs, ServerTargetArgs, }; -use crate::commands::server::{record, start, stop}; +use crate::commands::server::{start, stop}; use crate::gh::GhCli; use crate::shared::provider_auth::{ ApiKeySource, authenticate_provider, authenticate_provider_with_api_key_source, @@ -1152,7 +1153,7 @@ fn persist_server_env_secrets(storage_dir: &Path, secrets: &[(String, String)]) return Ok(()); } - let env_path = Storage::new(storage_dir).runtime_state().env_path(); + let env_path = Storage::new(storage_dir).runtime_directory().env_path(); envfile::merge_env_file(&env_path, secrets.iter().cloned()) .with_context(|| format!("merging server env secrets into {}", env_path.display()))?; Ok(()) @@ -1214,7 +1215,7 @@ fn persist_github_install_changes( writes: &PendingGitHubInstallWrite<'_>, ) -> Result<()> { let storage = Storage::new(storage_dir); - let server_env_path = storage.runtime_state().env_path(); + let server_env_path = storage.runtime_directory().env_path(); let vault_path = storage.secrets_path(); let previous_server_env = std::fs::read_to_string(&server_env_path).ok(); let previous_vault = std::fs::read_to_string(&vault_path).ok(); @@ -1490,7 +1491,8 @@ async fn run_install_github_inner( .clone_path() .unwrap_or_else(default_storage_dir) }); - let server_was_running = record::active_server_record(&storage_dir)?.is_some(); + let server_was_running = + ServerDaemon::load_running(&Storage::new(&storage_dir).runtime_directory())?.is_some(); let mut doc: toml::Value = toml::from_str(&existing_config_contents) .context("failed to parse existing settings.toml")?; @@ -1580,7 +1582,9 @@ async fn run_install_github_inner( .contains(&ServerAuthMethod::DevToken) .then(|| { dev_token::read_dev_token_file( - &Storage::new(&storage_dir).runtime_state().dev_token_path(), + &Storage::new(&storage_dir) + .runtime_directory() + .dev_token_path(), ) }) .flatten(); @@ -1638,7 +1642,8 @@ async fn run_install_inner( let emoji = console::Emoji("⚒️ ", ""); let cli_settings = user_config::load_settings_with_storage_dir(args.storage_dir.as_deref())?; let storage_dir = local_server::storage_dir(&cli_settings)?; - let server_was_running = record::active_server_record(&storage_dir)?.is_some(); + let server_was_running = + ServerDaemon::load_running(&Storage::new(&storage_dir).runtime_directory())?.is_some(); let fabro_dir = fabro_util::Home::from_env().root().to_path_buf(); let config_path = fabro_dir.join(SETTINGS_CONFIG_FILENAME); let existing_config_contents = std::fs::read_to_string(&config_path).ok(); @@ -1827,7 +1832,9 @@ async fn run_install_inner( &fabro_util::Home::from_env().dev_token_path(), )?; dev_token::write_dev_token( - &Storage::new(&storage_dir).runtime_state().dev_token_path(), + &Storage::new(&storage_dir) + .runtime_directory() + .dev_token_path(), &token, )?; fabro_util::printerr!( @@ -1878,7 +1885,7 @@ async fn run_install_inner( " {} Saved {} runtime secrets to {}", s.green.apply_to("✔"), server_env_pairs.len(), - path::contract_tilde(&Storage::new(&storage_dir).runtime_state().env_path()).display() + path::contract_tilde(&Storage::new(&storage_dir).runtime_directory().env_path()).display() ); fabro_util::printerr!( printer, @@ -1913,7 +1920,9 @@ async fn run_install_inner( .contains(&ServerAuthMethod::DevToken) .then(|| { dev_token::read_dev_token_file( - &Storage::new(&storage_dir).runtime_state().dev_token_path(), + &Storage::new(&storage_dir) + .runtime_directory() + .dev_token_path(), ) }) .flatten(); @@ -2577,7 +2586,8 @@ client_id = "client-id" .unwrap(); let server_env = - std::fs::read_to_string(Storage::new(dir.path()).runtime_state().env_path()).unwrap(); + std::fs::read_to_string(Storage::new(dir.path()).runtime_directory().env_path()) + .unwrap(); assert!(server_env.contains("SESSION_SECRET=session")); assert!(server_env.contains("FABRO_JWT_PUBLIC_KEY=public-key")); assert_eq!(created.calls_async().await, 2); @@ -2833,7 +2843,12 @@ client_id = "client-id" .await; assert!(result.is_err()); - assert!(Storage::new(dir.path()).runtime_state().env_path().exists()); + assert!( + Storage::new(dir.path()) + .runtime_directory() + .env_path() + .exists() + ); assert!(!settings_path.exists()); assert!(stop_called.load(Ordering::SeqCst)); } @@ -2877,7 +2892,7 @@ client_id = "client-id" fn persist_github_install_changes_replaces_app_env_keys_with_token_secret() { let dir = tempfile::tempdir().unwrap(); let storage = Storage::new(dir.path()); - let server_env_path = storage.runtime_state().env_path(); + let server_env_path = storage.runtime_directory().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([ @@ -2939,7 +2954,7 @@ client_id = "client-id" fn persist_github_install_changes_replaces_token_secret_with_app_env_keys() { let dir = tempfile::tempdir().unwrap(); let storage = Storage::new(dir.path()); - let server_env_path = storage.runtime_state().env_path(); + let server_env_path = storage.runtime_directory().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([("KEEP_ME".to_string(), "1".to_string())]), diff --git a/lib/crates/fabro-cli/src/commands/server/foreground.rs b/lib/crates/fabro-cli/src/commands/server/foreground.rs index beda408af..286409cb6 100644 --- a/lib/crates/fabro-cli/src/commands/server/foreground.rs +++ b/lib/crates/fabro-cli/src/commands/server/foreground.rs @@ -1,18 +1,15 @@ use std::path::PathBuf; use anyhow::Result; -use chrono::Utc; -use fabro_config::ServerRuntimeState; +use fabro_config::RuntimeDirectory; use fabro_server::bind::BindRequest; +use fabro_server::daemon::ServerDaemon; use fabro_server::serve; use fabro_server::serve::ServeArgs; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; -use super::record; - pub(crate) async fn execute( - record_path: PathBuf, mut serve_args: ServeArgs, bind: BindRequest, storage_dir: PathBuf, @@ -22,8 +19,9 @@ pub(crate) async fn execute( let _ = printer; serve_args.bind = Some(bind.to_string()); - let _record_guard = scopeguard::guard(record_path.clone(), |path| { - record::remove_server_record(&path); + let runtime_directory = RuntimeDirectory::new(&storage_dir); + let _record_guard = scopeguard::guard(runtime_directory.clone(), |dir| { + ServerDaemon::remove(&dir); }); let _socket_guard = if let BindRequest::Unix(ref path) = bind { @@ -35,20 +33,16 @@ pub(crate) async fn execute( None }; - let log_path = ServerRuntimeState::new(&storage_dir).log_path(); + let log_path = runtime_directory.log_path(); let pid = std::process::id(); + let daemon_dir = runtime_directory.clone(); Box::pin(serve::serve_command( serve_args, styles, Some(storage_dir), move |resolved_bind| { - record::write_server_record(&record_path, &record::ServerRecord { - pid, - bind: resolved_bind.clone(), - log_path: log_path.clone(), - started_at: Utc::now(), - }) + ServerDaemon::new(pid, resolved_bind.clone(), log_path.clone()).write(&daemon_dir) }, )) .await diff --git a/lib/crates/fabro-cli/src/commands/server/mod.rs b/lib/crates/fabro-cli/src/commands/server/mod.rs index a36d27333..88fdc7579 100644 --- a/lib/crates/fabro-cli/src/commands/server/mod.rs +++ b/lib/crates/fabro-cli/src/commands/server/mod.rs @@ -1,5 +1,4 @@ pub(crate) mod foreground; -pub(crate) mod record; pub(crate) mod start; pub(crate) mod status; pub(crate) mod stop; @@ -124,7 +123,6 @@ pub(crate) async fn dispatch( } ServerCommand::Serve(ServerServeArgs { storage_dir, - record_path, serve_args, }) => { let settings = user_config::load_settings_with_config_and_storage_dir( @@ -141,7 +139,6 @@ pub(crate) async fn dispatch( let bind_addr = local_server::bind_request(&settings, serve_args.bind.as_deref())?; let styles: &'static Styles = Box::leak(Box::new(Styles::detect_stderr())); Box::pin(foreground::execute( - record_path, ServeArgs { config: active_config_path, ..serve_args diff --git a/lib/crates/fabro-cli/src/commands/server/record.rs b/lib/crates/fabro-cli/src/commands/server/record.rs deleted file mode 100644 index 60ca26181..000000000 --- a/lib/crates/fabro-cli/src/commands/server/record.rs +++ /dev/null @@ -1,166 +0,0 @@ -#![expect( - clippy::disallowed_methods, - reason = "CLI server record helpers: sync read/write of local server record file" -)] - -use std::path::{Path, PathBuf}; - -use anyhow::{Context, Result, bail}; -use chrono::{DateTime, Utc}; -use fabro_config::ServerRuntimeState; -use fabro_config::user::default_storage_dir; -use fabro_server::bind::Bind; -use fabro_util::Home; -use serde::{Deserialize, Serialize}; - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub(crate) struct ServerRecord { - pub pid: u32, - pub bind: Bind, - pub log_path: PathBuf, - pub started_at: DateTime, -} - -#[derive(Debug, Clone)] -pub(crate) struct ActiveServerRecord { - pub record: ServerRecord, - pub record_path: PathBuf, -} - -pub(crate) fn write_server_record(path: &Path, record: &ServerRecord) -> Result<()> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("creating server record directory {}", parent.display()))?; - } - std::fs::write(path, serde_json::to_string_pretty(record)?) - .with_context(|| format!("Failed to write server metadata to {}", path.display())) -} - -pub(crate) fn read_server_record(path: &Path) -> Option { - let content = std::fs::read_to_string(path).ok()?; - serde_json::from_str(&content).ok() -} - -pub(crate) fn remove_server_record(path: &Path) { - let _ = std::fs::remove_file(path); -} - -pub(crate) fn server_record_is_running(record: &ServerRecord) -> bool { - fabro_proc::process_running(record.pid) && server_process_matches(record) -} - -fn server_record_path(storage_dir: &Path) -> PathBuf { - ServerRuntimeState::new(storage_dir).record_path() -} - -fn legacy_record_path(storage_dir: &Path) -> Option { - if storage_dir == default_storage_dir() { - Some(Home::from_env().root().join("server.json")) - } else { - None - } -} - -fn active_server_record_at_path(path: PathBuf) -> Option { - let record = read_server_record(&path)?; - if server_record_is_running(&record) { - Some(ActiveServerRecord { - record, - record_path: path, - }) - } else { - remove_server_record(&path); - None - } -} - -pub(crate) fn active_server_record_details( - storage_dir: &Path, -) -> Result> { - let primary_path = server_record_path(storage_dir); - if let Some(active) = active_server_record_at_path(primary_path.clone()) { - return Ok(Some(active)); - } - - if let Some(legacy_path) = legacy_record_path(storage_dir) { - if active_server_record_at_path(legacy_path.clone()).is_some() { - bail!( - "Legacy server record {} is still active while current storage record {} is missing.\nStop the old daemon with a legacy Fabro CLI or manually clear the stale daemon before retrying.", - legacy_path.display(), - primary_path.display() - ); - } - } - - Ok(None) -} - -pub(crate) fn active_server_record(storage_dir: &Path) -> Result> { - Ok(active_server_record_details(storage_dir)?.map(|active| active.record)) -} - -#[cfg(unix)] -#[expect( - clippy::disallowed_methods, - reason = "This synchronous process identity probe is shared by async server start and sync server status flows." -)] -fn server_process_matches(record: &ServerRecord) -> bool { - let output = match std::process::Command::new("ps") - .args(["-ww", "-o", "command=", "-p", &record.pid.to_string()]) - .output() - { - Ok(output) if output.status.success() => output, - _ => return false, - }; - let command = String::from_utf8_lossy(&output.stdout); - command.contains("fabro") && command.contains("server") -} - -#[cfg(not(unix))] -fn server_process_matches(_record: &ServerRecord) -> bool { - true -} - -#[cfg(test)] -mod tests { - use super::*; - - fn test_record(bind: Bind) -> ServerRecord { - ServerRecord { - pid: std::process::id(), - bind, - log_path: PathBuf::from("/tmp/storage/logs/server.log"), - started_at: Utc::now(), - } - } - - #[test] - fn write_and_read_round_trip() { - let dir = tempfile::tempdir().unwrap(); - let path = ServerRuntimeState::new(dir.path()).record_path(); - let record = test_record(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); - write_server_record(&path, &record).unwrap(); - - let loaded = read_server_record(&path).unwrap(); - assert_eq!(loaded.pid, record.pid); - assert_eq!(loaded.bind, record.bind); - } - - #[test] - fn active_server_record_returns_none_when_no_file() { - let dir = tempfile::tempdir().unwrap(); - assert!(active_server_record(dir.path()).unwrap().is_none()); - } - - #[test] - fn active_server_record_cleans_stale_dead_pid() { - let dir = tempfile::tempdir().unwrap(); - let path = ServerRuntimeState::new(dir.path()).record_path(); - let mut record = test_record(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); - record.pid = u32::MAX; // definitely not alive - write_server_record(&path, &record).unwrap(); - - assert!(active_server_record(dir.path()).unwrap().is_none()); - assert!(!path.exists()); - } -} diff --git a/lib/crates/fabro-cli/src/commands/server/start.rs b/lib/crates/fabro-cli/src/commands/server/start.rs index efbde1ee0..6c7e248b7 100644 --- a/lib/crates/fabro-cli/src/commands/server/start.rs +++ b/lib/crates/fabro-cli/src/commands/server/start.rs @@ -7,10 +7,10 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use anyhow::{Context, Result, anyhow, bail}; -use chrono::Utc; use fabro_config::user::{FABRO_CONFIG_ENV, default_settings_path, load_settings_config}; -use fabro_config::{ServerRuntimeState, envfile}; +use fabro_config::{RuntimeDirectory, envfile}; use fabro_server::bind::{Bind, BindRequest}; +use fabro_server::daemon::ServerDaemon; use fabro_server::jwt_auth::auth_method_name; use fabro_server::serve; use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs}; @@ -23,7 +23,6 @@ use tokio::process::Command as TokioCommand; use tokio::task::spawn_blocking; use tokio::time; -use super::record; use crate::local_server; pub(crate) struct ForegroundServerLogBootstrap { @@ -70,7 +69,8 @@ pub(crate) async fn prepare_foreground_server_log( storage_dir: &Path, ) -> Result { let lock_file = acquire_lock(storage_dir).await?; - if let Some(existing) = record::active_server_record(storage_dir)? { + let runtime_directory = RuntimeDirectory::new(storage_dir); + if let Some(existing) = ServerDaemon::load_running(&runtime_directory)? { bail!( "Server already running (pid {}) on {}", existing.pid, @@ -78,7 +78,7 @@ pub(crate) async fn prepare_foreground_server_log( ); } - let log_path = ServerRuntimeState::new(storage_dir).log_path(); + let log_path = runtime_directory.log_path(); if let Some(parent) = log_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating log directory {}", parent.display()))?; @@ -119,7 +119,8 @@ async fn ensure_server_running_with_bind( config_path: &Path, storage_dir: &Path, ) -> Result { - if let Some(existing) = record::active_server_record(storage_dir)? { + let runtime_directory = RuntimeDirectory::new(storage_dir); + if let Some(existing) = ServerDaemon::load_running(&runtime_directory)? { if bind_request .as_ref() .is_none_or(|requested| bind_matches_request(&existing.bind, requested)) @@ -163,7 +164,7 @@ async fn ensure_server_running_with_bind( ) .await { - Ok(()) => record::active_server_record(storage_dir)? + Ok(()) => ServerDaemon::load_running(&runtime_directory)? .map(|server| server.bind) .ok_or_else(|| { anyhow!( @@ -172,7 +173,7 @@ async fn ensure_server_running_with_bind( ) }), Err(err) => { - if let Some(existing) = record::active_server_record(storage_dir)? { + if let Some(existing) = ServerDaemon::load_running(&runtime_directory)? { Ok(existing.bind) } else { Err(err) @@ -235,7 +236,7 @@ fn load_or_create_local_session_secret(storage_dir: &Path) -> Result { return Ok(secret); } - let server_env_path = ServerRuntimeState::new(storage_dir).env_path(); + let server_env_path = RuntimeDirectory::new(storage_dir).env_path(); if let Some(secret) = envfile::read_env_file(&server_env_path) .ok() .and_then(|entries| entries.get("SESSION_SECRET").cloned()) @@ -274,13 +275,13 @@ async fn execute_foreground( }, ); - let runtime_state = ServerRuntimeState::new(&storage_dir); - let record_path = runtime_state.record_path(); - let log_path = runtime_state.log_path(); + let runtime_directory = RuntimeDirectory::new(&storage_dir); + let log_path = runtime_directory.log_path(); let pid = std::process::id(); + let daemon_dir = runtime_directory.clone(); - let _record_guard = scopeguard::guard(record_path.clone(), |path| { - record::remove_server_record(&path); + let _record_guard = scopeguard::guard(runtime_directory.clone(), |dir| { + ServerDaemon::remove(&dir); }); let _socket_guard = if let BindRequest::Unix(ref path) = bind { @@ -297,12 +298,7 @@ async fn execute_foreground( styles, Some(storage_dir), move |resolved_bind| { - record::write_server_record(&record_path, &record::ServerRecord { - pid, - bind: resolved_bind.clone(), - log_path: log_path.clone(), - started_at: Utc::now(), - }) + ServerDaemon::new(pid, resolved_bind.clone(), log_path.clone()).write(&daemon_dir) }, )) .await @@ -323,7 +319,8 @@ async fn execute_daemon( let lock_file = acquire_lock(storage_dir).await?; let _lock_file = lock_file; - if let Some(existing) = record::active_server_record(storage_dir)? { + let runtime_directory = RuntimeDirectory::new(storage_dir); + if let Some(existing) = ServerDaemon::load_running(&runtime_directory)? { if announce { bail!( "Server already running (pid {}) on {}", @@ -334,14 +331,12 @@ async fn execute_daemon( return Ok(()); } - let runtime_state = ServerRuntimeState::new(storage_dir); - let log_path = runtime_state.log_path(); + let log_path = runtime_directory.log_path(); if let Some(parent) = log_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating log directory {}", parent.display()))?; } - let record_path = runtime_state.record_path(); let log_file = std::fs::File::create(&log_path) .with_context(|| format!("creating server log file {}", log_path.display()))?; let stdout_log = log_file @@ -351,8 +346,6 @@ async fn execute_daemon( let mut cmd = TokioCommand::new(&exe); cmd.args(["server", "__serve"]) - .arg("--record-path") - .arg(&record_path) .arg("--bind") .arg(bind.to_string()); @@ -399,7 +392,7 @@ async fn execute_daemon( .with_context(|| format!("spawning fabro server subprocess {}", exe.display()))?; if let Ok(Some(status)) = child.try_wait() { - record::remove_server_record(&record_path); + ServerDaemon::remove(&runtime_directory); let tail = read_log_tail(&log_path, 20); if !tail.is_empty() { fabro_util::printerr!(printer, "{tail}"); @@ -412,18 +405,27 @@ async fn execute_daemon( let mut elapsed = Duration::ZERO; while elapsed < timeout { - if let Some(record) = record::read_server_record(&record_path) { - if try_connect(&record.bind).await { + let daemon = match ServerDaemon::read(&runtime_directory) { + Ok(daemon) => daemon, + Err(err) => { + ServerDaemon::remove(&runtime_directory); + let _ = child.kill().await; + let _ = child.wait().await; + return Err(err); + } + }; + if let Some(daemon) = daemon { + if try_connect(&daemon.bind).await { if announce { let pid = child.id().unwrap_or_default(); - maybe_warn_host_port_fallback(bind, &record.bind, printer); + maybe_warn_host_port_fallback(bind, &daemon.bind, printer); fabro_util::printerr!( printer, "Server started (pid {}) on {}", pid, - record.bind + daemon.bind ); - if let Bind::Tcp(addr) = &record.bind { + if let Bind::Tcp(addr) = &daemon.bind { let url = format!("http://{addr}"); let styled = match styles { Some(s) => format!("{}", s.cyan.apply_to(&url)), @@ -438,7 +440,7 @@ async fn execute_daemon( } if let Ok(Some(status)) = child.try_wait() { - record::remove_server_record(&record_path); + ServerDaemon::remove(&runtime_directory); let tail = read_log_tail(&log_path, 20); if !tail.is_empty() { fabro_util::printerr!(printer, "{tail}"); @@ -450,7 +452,7 @@ async fn execute_daemon( elapsed += poll_interval; } - record::remove_server_record(&record_path); + ServerDaemon::remove(&runtime_directory); let _ = child.kill().await; let _ = child.wait().await; let tail = read_log_tail(&log_path, 20); @@ -471,7 +473,7 @@ fn print_auth_methods(printer: Printer, serve_args: &ServeArgs) { // --------------------------------------------------------------------------- async fn acquire_lock(storage_dir: &Path) -> Result { - let lock_path = ServerRuntimeState::new(storage_dir).lock_path(); + let lock_path = RuntimeDirectory::new(storage_dir).lock_path(); if let Some(parent) = lock_path.parent() { std::fs::create_dir_all(parent) .with_context(|| format!("creating server lock directory {}", parent.display()))?; diff --git a/lib/crates/fabro-cli/src/commands/server/status.rs b/lib/crates/fabro-cli/src/commands/server/status.rs index 5c8536b3b..b93fc9a97 100644 --- a/lib/crates/fabro-cli/src/commands/server/status.rs +++ b/lib/crates/fabro-cli/src/commands/server/status.rs @@ -2,12 +2,13 @@ use std::path::Path; use anyhow::Result; use chrono::Utc; +use fabro_config::RuntimeDirectory; +use fabro_server::daemon::ServerDaemon; use fabro_util::printer::Printer; -use super::record; - pub(crate) fn execute(storage_dir: &Path, json: bool, printer: Printer) -> Result<()> { - let Some(record) = record::active_server_record(storage_dir)? else { + let runtime_directory = RuntimeDirectory::new(storage_dir); + let Some(daemon) = ServerDaemon::load_running(&runtime_directory)? else { if json { fabro_util::printout!(printer, r#"{{"status":"stopped"}}"#); } else { @@ -17,22 +18,22 @@ pub(crate) fn execute(storage_dir: &Path, json: bool, printer: Printer) -> Resul }; if json { - let uptime_seconds = (Utc::now() - record.started_at).num_seconds().max(0); + let uptime_seconds = (Utc::now() - daemon.started_at).num_seconds().max(0); let output = serde_json::json!({ "status": "running", - "pid": record.pid, - "bind": record.bind.to_string(), - "started_at": record.started_at.to_rfc3339(), + "pid": daemon.pid, + "bind": daemon.bind.to_string(), + "started_at": daemon.started_at.to_rfc3339(), "uptime_seconds": uptime_seconds, }); fabro_util::printout!(printer, "{}", serde_json::to_string_pretty(&output)?); } else { - let uptime = format_uptime(Utc::now() - record.started_at); + let uptime = format_uptime(Utc::now() - daemon.started_at); fabro_util::printerr!( printer, "Server running (pid {}) on {}, started {} ago", - record.pid, - record.bind, + daemon.pid, + daemon.bind, uptime ); } diff --git a/lib/crates/fabro-cli/src/commands/server/stop.rs b/lib/crates/fabro-cli/src/commands/server/stop.rs index 09811c6bf..66a9b248a 100644 --- a/lib/crates/fabro-cli/src/commands/server/stop.rs +++ b/lib/crates/fabro-cli/src/commands/server/stop.rs @@ -2,19 +2,19 @@ use std::path::Path; use std::time::Duration; use anyhow::Result; +use fabro_config::RuntimeDirectory; use fabro_server::bind::Bind; +use fabro_server::daemon::ServerDaemon; use fabro_util::printer::Printer; use tokio::time; -use super::record; - pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> Result { - let Some(active) = record::active_server_record_details(storage_dir)? else { + let runtime_directory = RuntimeDirectory::new(storage_dir); + let Some(daemon) = ServerDaemon::load_running(&runtime_directory)? else { return Ok(false); }; - let record = active.record; - fabro_proc::sigterm(record.pid); + fabro_proc::sigterm(daemon.pid); // Use the zombie-aware predicate here: this loop is commonly driven // against a child of the calling process (tests, install/uninstall @@ -26,21 +26,21 @@ pub(crate) async fn stop_server(storage_dir: &Path, timeout: Duration) -> Result let poll_interval = Duration::from_millis(100); let mut elapsed = Duration::ZERO; while elapsed < timeout { - if !fabro_proc::process_running_strict(record.pid) { + if !fabro_proc::process_running_strict(daemon.pid) { break; } time::sleep(poll_interval).await; elapsed += poll_interval; } - if fabro_proc::process_running_strict(record.pid) { - fabro_proc::sigkill(record.pid); + if fabro_proc::process_running_strict(daemon.pid) { + fabro_proc::sigkill(daemon.pid); time::sleep(Duration::from_millis(100)).await; } - record::remove_server_record(&active.record_path); + ServerDaemon::remove(&runtime_directory); - if let Bind::Unix(ref path) = record.bind { + if let Bind::Unix(ref path) = daemon.bind { let _ = std::fs::remove_file(path); } diff --git a/lib/crates/fabro-cli/src/commands/uninstall.rs b/lib/crates/fabro-cli/src/commands/uninstall.rs index 3a5be57d5..10992edf6 100644 --- a/lib/crates/fabro-cli/src/commands/uninstall.rs +++ b/lib/crates/fabro-cli/src/commands/uninstall.rs @@ -13,6 +13,8 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use anyhow::{Context, Result}; +use fabro_config::Storage; +use fabro_server::daemon::ServerDaemon; use fabro_types::settings::CliSettings; use fabro_types::settings::cli::OutputFormat; use fabro_util::Home; @@ -21,7 +23,7 @@ use serde::Serialize; use tracing::warn; use crate::args::UninstallArgs; -use crate::commands::server; +use crate::commands::server::stop; use crate::shared::{format_size, print_json_pretty, tilde_path}; use crate::{local_server, user_config}; @@ -87,7 +89,8 @@ pub(crate) async fn run_uninstall( fn build_inventory(home_root: &Path, storage_dir: &Path) -> Result { let home_size = dir_size(home_root); - let server_running = server::record::active_server_record_details(storage_dir)?.is_some(); + let server_running = + ServerDaemon::load_running(&Storage::new(storage_dir).runtime_directory())?.is_some(); let shell_configs = find_shell_configs_with_sentinel(); let (binary_path, binary_is_managed) = resolve_binary(home_root); @@ -264,7 +267,7 @@ async fn execute_uninstall(inventory: &Inventory, json: bool, printer: Printer) // Unit 3a: Server stop if inventory.server_running { - server::stop::execute(&inventory.storage_dir, Duration::from_secs(5), printer).await?; + stop::execute(&inventory.storage_dir, Duration::from_secs(5), printer).await?; result.server_stopped = true; } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 51538e197..64251cd51 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -495,7 +495,7 @@ async fn prepare_server_bootstrap( let settings = user_config::load_settings_with_config_and_storage_dir(config_path, storage_dir)?; let storage_dir = local_server::storage_dir(&settings)?; - let runtime_state = fabro_config::ServerRuntimeState::new(storage_dir.clone()); + let runtime_directory = fabro_config::RuntimeDirectory::new(storage_dir.clone()); let foreground_server_log_bootstrap = if foreground { Some(commands::server::start::prepare_foreground_server_log(&storage_dir).await?) } else { @@ -504,7 +504,7 @@ async fn prepare_server_bootstrap( Ok(PreTracingBootstrap { sink: logging::InternalLogSink::Server { - path: runtime_state.log_path(), + path: runtime_directory.log_path(), }, config_log_level: local_server::config_log_level(&settings), foreground_server_log_bootstrap, @@ -743,16 +743,12 @@ level = "warn" let config_dir = tempfile::tempdir().unwrap(); let config_path = config_dir.path().join("settings.toml"); write_test_settings(&config_path); - let record_path = storage_dir.path().join("server.json"); - let cli = Cli::try_parse_from([ "fabro", "server", "__serve", "--storage-dir", storage_dir.path().to_str().unwrap(), - "--record-path", - record_path.to_str().unwrap(), "--config", config_path.to_str().unwrap(), ]) diff --git a/lib/crates/fabro-cli/tests/it/cmd/install.rs b/lib/crates/fabro-cli/tests/it/cmd/install.rs index 595e9542e..509e4dce9 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/install.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/install.rs @@ -237,7 +237,7 @@ mode = "keep-me" ), ); - let server_env_path = Storage::new(&storage_dir).runtime_state().env_path(); + let server_env_path = Storage::new(&storage_dir).runtime_directory().env_path(); envfile::write_env_file( &server_env_path, &std::collections::HashMap::from([ diff --git a/lib/crates/fabro-cli/tests/it/cmd/ps.rs b/lib/crates/fabro-cli/tests/it/cmd/ps.rs index 268cdfa3f..79bc788b5 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/ps.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/ps.rs @@ -12,7 +12,7 @@ const TEST_DEV_TOKEN: &str = fn provision_local_server_auth(context: &fabro_test::TestContext, storage_dir: &std::path::Path) { context.ensure_home_server_auth_methods(); - let server_env_path = Storage::new(storage_dir).runtime_state().env_path(); + let server_env_path = Storage::new(storage_dir).runtime_directory().env_path(); envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]) .expect("merging FABRO_DEV_TOKEN into server.env"); dev_token::write_dev_token( diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs index 69bb12dcd..13996832b 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_start.rs @@ -14,8 +14,8 @@ use std::time::{Duration, Instant}; use fabro_config::{Storage, envfile}; use fabro_test::{ - apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files, stop_pid, - test_context, wait_for_log_line, wait_for_path, + apply_test_isolation, fabro_snapshot, isolated_storage_dir, server_log_files, test_context, + wait_for_log_line, wait_for_path, }; use fabro_util::dev_token; @@ -31,7 +31,7 @@ fn write_dev_token_server_settings(config_path: &std::path::Path, rest: &str) { } fn provision_dev_token_auth(home_dir: &std::path::Path, storage_dir: &std::path::Path) { - let server_env_path = Storage::new(storage_dir).runtime_state().env_path(); + let server_env_path = Storage::new(storage_dir).runtime_directory().env_path(); envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]) .expect("merging FABRO_DEV_TOKEN into server.env"); dev_token::write_dev_token(&home_dir.join(".fabro").join("dev-token"), TEST_DEV_TOKEN) @@ -367,86 +367,6 @@ fn daemon_start_writes_tracing_to_storage_server_log() { ); } -#[test] -#[expect( - clippy::disallowed_methods, - reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade." -)] -fn start_errors_when_only_a_legacy_running_server_record_exists() { - let home_dir = tempfile::tempdir_in("/tmp").unwrap(); - let fabro_home = home_dir.path().join(".fabro"); - let storage_dir = fabro_home.join("storage"); - let socket_path = home_dir.path().join("legacy.sock"); - let config_dir = tempfile::tempdir_in("/tmp").unwrap(); - let config_path = config_dir.path().join("settings.toml"); - write_dev_token_server_settings(&config_path, ""); - provision_dev_token_auth(home_dir.path(), &storage_dir); - - let start_output = { - let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - apply_test_isolation(&mut start, home_dir.path()); - start - .args(["server", "start", "--bind"]) - .arg(&socket_path) - .arg("--config") - .arg(&config_path) - .output() - .expect("server start should run") - }; - assert!( - start_output.status.success(), - "server start should succeed:\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&start_output.stdout), - String::from_utf8_lossy(&start_output.stderr) - ); - - let current_record = storage_dir.join("server.json"); - wait_for_path(¤t_record); - let legacy_record = fabro_home.join("server.json"); - std::fs::rename(¤t_record, &legacy_record).unwrap(); - - let retry_output = { - let mut retry = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - apply_test_isolation(&mut retry, home_dir.path()); - retry - .args(["server", "start", "--bind"]) - .arg(home_dir.path().join("new.sock")) - .arg("--config") - .arg(&config_path) - .output() - .expect("server start retry should run") - }; - - let pid_u64 = serde_json::from_str::( - &std::fs::read_to_string(&legacy_record).unwrap(), - ) - .unwrap()["pid"] - .as_u64() - .unwrap(); - let pid = u32::try_from(pid_u64).expect("pid fits in u32"); - stop_pid(pid); - let _ = std::fs::remove_file(&legacy_record); - let _ = std::fs::remove_file(&socket_path); - - assert!( - !retry_output.status.success(), - "server start should fail when only the legacy record exists" - ); - let stderr = String::from_utf8_lossy(&retry_output.stderr); - assert!( - stderr.contains(&legacy_record.display().to_string()), - "expected stderr to mention the legacy record path, got:\n{stderr}" - ); - assert!( - stderr.contains(¤t_record.display().to_string()), - "expected stderr to mention the current record path, got:\n{stderr}" - ); - assert!( - stderr.contains("legacy Fabro CLI"), - "expected stderr to instruct manual cleanup, got:\n{stderr}" - ); -} - #[test] #[expect( clippy::disallowed_methods, diff --git a/lib/crates/fabro-cli/tests/it/cmd/server_status.rs b/lib/crates/fabro-cli/tests/it/cmd/server_status.rs index c62fd79b0..9c648d1fb 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/server_status.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/server_status.rs @@ -1,4 +1,4 @@ -use fabro_test::{fabro_snapshot, isolated_storage_dir, stop_pid, test_context, wait_for_path}; +use fabro_test::{fabro_snapshot, isolated_storage_dir, test_context}; #[test] fn help() { @@ -41,87 +41,3 @@ fn status_when_not_running() { Server is not running "); } - -#[test] -#[expect( - clippy::disallowed_methods, - reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade." -)] -fn status_errors_when_only_a_legacy_running_server_record_exists() { - let home_dir = tempfile::tempdir_in("/tmp").unwrap(); - let fabro_home = home_dir.path().join(".fabro"); - let storage_dir = fabro_home.join("storage"); - let socket_path = home_dir.path().join("legacy.sock"); - let config_dir = tempfile::tempdir_in("/tmp").unwrap(); - let config_path = config_dir.path().join("settings.toml"); - std::fs::write( - &config_path, - "_version = 1\n\n[server.auth]\nmethods = [\"dev-token\"]\n", - ) - .unwrap(); - - let start_output = { - let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - fabro_test::apply_test_isolation(&mut start, home_dir.path()); - start.env( - "FABRO_DEV_TOKEN", - "fabro_dev_abababababababababababababababababababababababababababababababab", - ); - start - .args(["server", "start", "--bind"]) - .arg(&socket_path) - .arg("--config") - .arg(&config_path) - .output() - .expect("server start should run") - }; - assert!( - start_output.status.success(), - "server start should succeed:\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&start_output.stdout), - String::from_utf8_lossy(&start_output.stderr) - ); - - let current_record = storage_dir.join("server.json"); - wait_for_path(¤t_record); - let legacy_record = fabro_home.join("server.json"); - std::fs::rename(¤t_record, &legacy_record).unwrap(); - - let status_output = { - let mut status = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - fabro_test::apply_test_isolation(&mut status, home_dir.path()); - status - .args(["server", "status"]) - .output() - .expect("server status should run") - }; - - let pid_u64 = serde_json::from_str::( - &std::fs::read_to_string(&legacy_record).unwrap(), - ) - .unwrap()["pid"] - .as_u64() - .unwrap(); - let pid = u32::try_from(pid_u64).expect("pid fits in u32"); - stop_pid(pid); - let _ = std::fs::remove_file(&legacy_record); - let _ = std::fs::remove_file(&socket_path); - - assert!( - !status_output.status.success(), - "server status should fail when only the legacy record exists" - ); - let stderr = String::from_utf8_lossy(&status_output.stderr); - assert!( - stderr.contains(&legacy_record.display().to_string()), - "expected stderr to mention the legacy record path, got:\n{stderr}" - ); - assert!( - stderr.contains(¤t_record.display().to_string()), - "expected stderr to mention the current record path, got:\n{stderr}" - ); - assert!( - stderr.contains("legacy Fabro CLI"), - "expected stderr to instruct manual cleanup, got:\n{stderr}" - ); -} diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index f706d67bb..8eeb4299e 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -16,6 +16,7 @@ use std::time::{Duration, Instant}; use fabro_config::{Storage, envfile}; use fabro_server::bind::Bind; +use fabro_server::daemon::ServerDaemon; use fabro_store::EventEnvelope; use fabro_test::{TestContext, expect_reqwest_status}; use fabro_types::RunId; @@ -658,13 +659,8 @@ fn block_on(future: impl std::future::Future) -> T { .block_on(future) } -#[derive(Debug, serde::Deserialize)] -struct TestServerRecord { - bind: Bind, -} - pub(crate) fn local_dev_token(storage_dir: &Path) -> Option { - let server_state = Storage::new(storage_dir).runtime_state(); + let server_state = Storage::new(storage_dir).runtime_directory(); envfile::read_env_file(&server_state.env_path()) .ok() @@ -673,10 +669,8 @@ pub(crate) fn local_dev_token(storage_dir: &Path) -> Option { } pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpClient, String)> { - let record_path = Storage::new(storage_dir).runtime_state().record_path(); - let record = std::fs::read_to_string(record_path) - .ok() - .and_then(|content| serde_json::from_str::(&content).ok())?; + let runtime_directory = Storage::new(storage_dir).runtime_directory(); + let daemon = ServerDaemon::read(&runtime_directory).ok().flatten()?; let mut headers = fabro_http::HeaderMap::new(); if let Some(token) = local_dev_token(storage_dir) { headers.insert( @@ -685,7 +679,7 @@ pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpCli .expect("local dev token should build an authorization header"), ); } - match record.bind { + match daemon.bind { Bind::Unix(path) if path.exists() => Some(( fabro_http::HttpClientBuilder::new() .unix_socket(path) @@ -708,12 +702,11 @@ pub(crate) fn server_endpoint(storage_dir: &Path) -> Option<(fabro_http::HttpCli } pub(crate) fn server_target(storage_dir: &Path) -> String { - let record_path = Storage::new(storage_dir).runtime_state().record_path(); - let record = std::fs::read_to_string(record_path) - .ok() - .and_then(|content| serde_json::from_str::(&content).ok()) + let runtime_directory = Storage::new(storage_dir).runtime_directory(); + let daemon = ServerDaemon::read(&runtime_directory) + .expect("server record should parse") .expect("server record should exist"); - match record.bind { + match daemon.bind { Bind::Unix(path) => path.to_string_lossy().to_string(), Bind::Tcp(addr) => format!("http://{addr}"), } diff --git a/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs b/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs index 3cb57c96f..16bdc377f 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/uninstall.rs @@ -5,7 +5,7 @@ use std::fs; -use fabro_test::{fabro_snapshot, stop_pid, test_context, wait_for_path}; +use fabro_test::{fabro_snapshot, test_context}; use serde_json::Value; #[test] @@ -169,97 +169,3 @@ fn not_installed_json() { assert_eq!(value["status"].as_str(), Some("not_installed")); } - -#[test] -#[expect( - clippy::disallowed_methods, - reason = "This integration test moves a live daemon record on disk to simulate an unsupported legacy daemon upgrade." -)] -fn uninstall_yes_fails_when_only_a_legacy_running_server_record_exists() { - let home_dir = tempfile::tempdir_in("/tmp").unwrap(); - let fabro_home = home_dir.path().join(".fabro"); - let storage_dir = fabro_home.join("storage"); - let socket_path = home_dir.path().join("legacy.sock"); - let config_dir = tempfile::tempdir_in("/tmp").unwrap(); - let config_path = config_dir.path().join("settings.toml"); - std::fs::write( - &config_path, - "_version = 1\n\n[server.auth]\nmethods = [\"dev-token\"]\n", - ) - .unwrap(); - std::fs::create_dir_all(&fabro_home).unwrap(); - std::fs::write( - fabro_home.join("settings.toml"), - "_version = 1\n\n[server.auth]\nmethods = [\"dev-token\"]\n", - ) - .unwrap(); - - let start_output = { - let mut start = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - fabro_test::apply_test_isolation(&mut start, home_dir.path()); - start.env( - "FABRO_DEV_TOKEN", - "fabro_dev_abababababababababababababababababababababababababababababababab", - ); - start - .args(["server", "start", "--bind"]) - .arg(&socket_path) - .arg("--config") - .arg(&config_path) - .output() - .expect("server start should run") - }; - assert!( - start_output.status.success(), - "server start should succeed:\nstdout:\n{}\nstderr:\n{}", - String::from_utf8_lossy(&start_output.stdout), - String::from_utf8_lossy(&start_output.stderr) - ); - - let current_record = storage_dir.join("server.json"); - wait_for_path(¤t_record); - let legacy_record = fabro_home.join("server.json"); - std::fs::rename(¤t_record, &legacy_record).unwrap(); - let pid_u64 = serde_json::from_str::( - &std::fs::read_to_string(&legacy_record).unwrap(), - ) - .unwrap()["pid"] - .as_u64() - .unwrap(); - let pid = u32::try_from(pid_u64).expect("pid fits in u32"); - - let uninstall_output = { - let mut uninstall = std::process::Command::new(env!("CARGO_BIN_EXE_fabro")); - fabro_test::apply_test_isolation(&mut uninstall, home_dir.path()); - uninstall - .args(["uninstall", "--yes"]) - .output() - .expect("uninstall should run") - }; - - stop_pid(pid); - let _ = std::fs::remove_file(&legacy_record); - let _ = std::fs::remove_file(&socket_path); - - assert!( - !uninstall_output.status.success(), - "uninstall --yes should fail when only the legacy record exists" - ); - let stderr = String::from_utf8_lossy(&uninstall_output.stderr); - assert!( - stderr.contains(&legacy_record.display().to_string()), - "expected stderr to mention the legacy record path, got:\n{stderr}" - ); - assert!( - stderr.contains(¤t_record.display().to_string()), - "expected stderr to mention the current record path, got:\n{stderr}" - ); - assert!( - stderr.contains("legacy Fabro CLI"), - "expected stderr to instruct manual cleanup, got:\n{stderr}" - ); - assert!( - fabro_home.exists(), - "uninstall should not remove ~/.fabro when the legacy daemon detector fires" - ); -} diff --git a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs index a79691b1a..5abc94115 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/server_lifecycle.rs @@ -11,7 +11,7 @@ fn start_status_stop_lifecycle() { "[server.auth]\nmethods = [\"dev-token\"]\n", ); let server_env_path = fabro_config::Storage::new(&storage_dir) - .runtime_state() + .runtime_directory() .env_path(); fabro_config::envfile::merge_env_file(&server_env_path, [( "FABRO_DEV_TOKEN", diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 12dcb349b..6eb6d28ff 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -38,7 +38,7 @@ pub use resolve::{ resolve_storage_root, resolve_workflow, resolve_workflow_from_file, }; use serde::de::DeserializeOwned; -pub use storage::{RunScratch, ServerRuntimeState, Storage}; +pub use storage::{RunScratch, RuntimeDirectory, Storage}; pub fn load_and_resolve( layers: effective_settings::EffectiveSettingsLayers, diff --git a/lib/crates/fabro-config/src/storage.rs b/lib/crates/fabro-config/src/storage.rs index 84bf576cb..9bf0bedab 100644 --- a/lib/crates/fabro-config/src/storage.rs +++ b/lib/crates/fabro-config/src/storage.rs @@ -9,7 +9,7 @@ pub struct Storage { } #[derive(Clone, Debug, PartialEq, Eq)] -pub struct ServerRuntimeState { +pub struct RuntimeDirectory { root: PathBuf, } @@ -48,8 +48,8 @@ impl Storage { } #[must_use] - pub fn runtime_state(&self) -> ServerRuntimeState { - ServerRuntimeState::new(self.root.clone()) + pub fn runtime_directory(&self) -> RuntimeDirectory { + RuntimeDirectory::new(self.root.clone()) } #[must_use] @@ -78,7 +78,7 @@ impl Storage { } } -impl ServerRuntimeState { +impl RuntimeDirectory { #[must_use] pub fn new(root: impl Into) -> Self { Self { root: root.into() } @@ -163,12 +163,12 @@ mod tests { use chrono::Local; use fabro_types::RunId; - use super::{RunScratch, ServerRuntimeState, Storage}; + use super::{RunScratch, RuntimeDirectory, Storage}; #[test] fn storage_accessors_are_relative_to_root() { let storage = Storage::new("/tmp/fabro-data"); - let runtime = ServerRuntimeState::new("/tmp/fabro-data"); + let runtime = RuntimeDirectory::new("/tmp/fabro-data"); assert_eq!(storage.root(), std::path::Path::new("/tmp/fabro-data")); assert_eq!( diff --git a/lib/crates/fabro-install/src/lib.rs b/lib/crates/fabro-install/src/lib.rs index 31a68b838..f6421b82f 100644 --- a/lib/crates/fabro-install/src/lib.rs +++ b/lib/crates/fabro-install/src/lib.rs @@ -266,7 +266,7 @@ fn persist_server_env_secrets(storage_dir: &Path, secrets: &[(String, String)]) return Ok(()); } - let env_path = Storage::new(storage_dir).runtime_state().env_path(); + let env_path = Storage::new(storage_dir).runtime_directory().env_path(); envfile::merge_env_file(&env_path, secrets.iter().cloned()) .with_context(|| format!("merging server env secrets into {}", env_path.display()))?; Ok(()) @@ -500,7 +500,7 @@ name = "custom" assert_eq!(restored.get("EXISTING_SECRET"), Some("keep")); assert_eq!(restored.get("bad-secret-name"), None); - let server_env = envfile::read_env_file(&storage.runtime_state().env_path()).unwrap(); + let server_env = envfile::read_env_file(&storage.runtime_directory().env_path()).unwrap(); assert_eq!( server_env.get("SESSION_SECRET").map(String::as_str), Some("session") diff --git a/lib/crates/fabro-server/src/daemon.rs b/lib/crates/fabro-server/src/daemon.rs new file mode 100644 index 000000000..d811039fa --- /dev/null +++ b/lib/crates/fabro-server/src/daemon.rs @@ -0,0 +1,182 @@ +#![expect( + clippy::disallowed_methods, + reason = "Server daemon metadata uses synchronous local file I/O and process probes." +)] + +use std::io::ErrorKind; +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use chrono::{DateTime, Utc}; +use fabro_config::RuntimeDirectory; +use serde::{Deserialize, Serialize}; +use tempfile::NamedTempFile; + +use crate::bind::Bind; + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct ServerDaemon { + pub pid: u32, + pub bind: Bind, + pub log_path: PathBuf, + pub started_at: DateTime, +} + +impl ServerDaemon { + #[must_use] + pub fn new(pid: u32, bind: Bind, log_path: PathBuf) -> Self { + Self { + pid, + bind, + log_path, + started_at: Utc::now(), + } + } + + pub fn read(dir: &RuntimeDirectory) -> Result> { + let record_path = dir.record_path(); + let content = match std::fs::read_to_string(&record_path) { + Ok(content) => content, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(anyhow::Error::new(err) + .context(format!("reading server record {}", record_path.display()))); + } + }; + + serde_json::from_str(&content) + .map(Some) + .with_context(|| format!("parsing server record {}", record_path.display())) + } + + pub fn load_running(dir: &RuntimeDirectory) -> Result> { + let Some(daemon) = Self::read(dir)? else { + return Ok(None); + }; + + if daemon.is_running() { + Ok(Some(daemon)) + } else { + Self::remove(dir); + Ok(None) + } + } + + pub fn write(&self, dir: &RuntimeDirectory) -> Result<()> { + let record_path = dir.record_path(); + let record_dir = record_path + .parent() + .unwrap_or_else(|| std::path::Path::new(".")); + std::fs::create_dir_all(record_dir).with_context(|| { + format!("creating server record directory {}", record_dir.display()) + })?; + + let temp = NamedTempFile::new_in(record_dir).with_context(|| { + format!("creating temp server record for {}", record_path.display()) + })?; + std::fs::write(temp.path(), serde_json::to_string_pretty(self)?) + .with_context(|| format!("writing temp server record for {}", record_path.display()))?; + temp.persist(&record_path) + .map_err(|err| err.error) + .with_context(|| format!("persisting server record {}", record_path.display()))?; + Ok(()) + } + + pub fn remove(dir: &RuntimeDirectory) { + let _ = std::fs::remove_file(dir.record_path()); + } + + #[must_use] + pub fn is_running(&self) -> bool { + fabro_proc::process_running(self.pid) && server_process_matches(self.pid) + } +} + +#[cfg(unix)] +fn server_process_matches(pid: u32) -> bool { + let output = match std::process::Command::new("ps") + .args(["-ww", "-o", "command=", "-p", &pid.to_string()]) + .output() + { + Ok(output) if output.status.success() => output, + _ => return false, + }; + let command = String::from_utf8_lossy(&output.stdout); + command.contains("fabro") && command.contains("server") +} + +#[cfg(not(unix))] +fn server_process_matches(_pid: u32) -> bool { + true +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use fabro_config::RuntimeDirectory; + + use super::{Bind, ServerDaemon}; + + fn test_daemon(bind: Bind) -> ServerDaemon { + ServerDaemon { + pid: std::process::id(), + bind, + log_path: PathBuf::from("/tmp/storage/logs/server.log"), + started_at: chrono::Utc::now(), + } + } + + #[test] + fn write_and_read_round_trip() { + let dir = tempfile::tempdir().unwrap(); + let runtime_directory = RuntimeDirectory::new(dir.path()); + let daemon = test_daemon(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); + daemon.write(&runtime_directory).unwrap(); + + let loaded = ServerDaemon::read(&runtime_directory).unwrap().unwrap(); + assert_eq!(loaded, daemon); + } + + #[test] + fn active_server_record_returns_none_when_no_file() { + let dir = tempfile::tempdir().unwrap(); + let runtime_directory = RuntimeDirectory::new(dir.path()); + assert!( + ServerDaemon::load_running(&runtime_directory) + .unwrap() + .is_none() + ); + } + + #[test] + fn active_server_record_cleans_stale_dead_pid() { + let dir = tempfile::tempdir().unwrap(); + let runtime_directory = RuntimeDirectory::new(dir.path()); + let mut daemon = test_daemon(Bind::Tcp("127.0.0.1:3000".parse().unwrap())); + daemon.pid = u32::MAX; + daemon.write(&runtime_directory).unwrap(); + + assert!( + ServerDaemon::load_running(&runtime_directory) + .unwrap() + .is_none() + ); + assert!(!runtime_directory.record_path().exists()); + } + + #[test] + fn read_surfaces_parse_error_with_path() { + let dir = tempfile::tempdir().unwrap(); + let runtime_directory = RuntimeDirectory::new(dir.path()); + let record_path = runtime_directory.record_path(); + std::fs::create_dir_all(record_path.parent().unwrap()).unwrap(); + std::fs::write(&record_path, "not json").unwrap(); + + let err = ServerDaemon::read(&runtime_directory).unwrap_err(); + assert!( + err.to_string() + .contains(record_path.display().to_string().as_str()) + ); + } +} diff --git a/lib/crates/fabro-server/src/install.rs b/lib/crates/fabro-server/src/install.rs index a9c4e39fb..09075a07d 100644 --- a/lib/crates/fabro-server/src/install.rs +++ b/lib/crates/fabro-server/src/install.rs @@ -827,7 +827,7 @@ async fn post_install_finish( }; if let Err(err) = dev_token::write_dev_token( &Storage::new(state.storage_dir.as_ref()) - .runtime_state() + .runtime_directory() .dev_token_path(), &token, ) { diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index f1a76af8f..e686ee60a 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -12,6 +12,7 @@ pub mod auth; pub mod bind; mod canonical_origin; pub mod csp; +pub mod daemon; #[allow( clippy::wildcard_imports, clippy::absolute_paths, diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index 53829fca4..ab1685746 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -470,7 +470,7 @@ where }; let storage = Storage::new(&data_dir); let vault_path = storage.secrets_path(); - let server_env_path = storage.runtime_state().env_path(); + let server_env_path = storage.runtime_directory().env_path(); let server_secrets = ServerSecrets::load(server_env_path.clone())?; let webhook_secret_present = server_secrets.get(WEBHOOK_SECRET_ENV).is_some(); diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 9714a5d39..376571080 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -112,6 +112,7 @@ use ulid::Ulid; use crate::auth::{self, GithubEndpoints, auth_translation_middleware, demo_routing_middleware}; use crate::bind::Bind; use crate::canonical_origin::resolve_canonical_origin; +use crate::daemon::ServerDaemon; use crate::error::ApiError; use crate::github_webhooks::{ WEBHOOK_ROUTE, WEBHOOK_SECRET_ENV, parse_event_metadata, verify_signature, @@ -1569,7 +1570,7 @@ fn build_disk_usage_response( verbose: bool, ) -> anyhow::Result { let scratch_base_dir = scratch_base(storage_dir); - let logs_base_dir = Storage::new(storage_dir).runtime_state().logs_dir(); + let logs_base_dir = Storage::new(storage_dir).runtime_directory().logs_dir(); let runs = scan_runs_with_summaries(summaries, &scratch_base_dir)?; let mut active_count = 0u64; @@ -3760,28 +3761,16 @@ async fn append_worker_exit_failure( } } -#[derive(serde::Deserialize)] -struct WorkerServerRecord { - bind: Bind, -} - fn current_server_target(storage_dir: &std::path::Path) -> anyhow::Result { - let record_path = Storage::new(storage_dir).runtime_state().record_path(); - #[expect( - clippy::disallowed_methods, - reason = "sync helper invoked from worker_command (sync) via spawn_blocking at the async \ - boundary in execute_run_subprocess; see commit 9d1c0d98c" - )] - let content = std::fs::read_to_string(&record_path) - .map_err(|err| anyhow::anyhow!("failed to read {}: {err}", record_path.display()))?; - let record: WorkerServerRecord = serde_json::from_str(&content).map_err(|err| { - anyhow::anyhow!( - "failed to parse server record {}: {err}", - record_path.display() + let runtime_directory = Storage::new(storage_dir).runtime_directory(); + let daemon = ServerDaemon::read(&runtime_directory)?.with_context(|| { + format!( + "server record {} is missing", + runtime_directory.record_path().display() ) })?; - Ok(match record.bind { + Ok(match daemon.bind { Bind::Unix(path) => path.to_string_lossy().to_string(), Bind::Tcp(addr) => format!("http://{addr}"), }) @@ -7901,15 +7890,13 @@ allowed_usernames = ["octocat"] .join(", ") )) .unwrap(); - let record_path = Storage::new(storage_dir).runtime_state().record_path(); - std::fs::create_dir_all(record_path.parent().unwrap()).unwrap(); - std::fs::write( - &record_path, - serde_json::to_string(&json!({ - "bind": Bind::Tcp("127.0.0.1:32276".parse::().unwrap()), - })) - .unwrap(), + let runtime_directory = Storage::new(storage_dir).runtime_directory(); + ServerDaemon::new( + std::process::id(), + Bind::Tcp("127.0.0.1:32276".parse::().unwrap()), + runtime_directory.log_path(), ) + .write(&runtime_directory) .unwrap(); create_app_state_with_env_lookup(settings, 5, move |name| match name { diff --git a/lib/crates/fabro-server/tests/it/api/install.rs b/lib/crates/fabro-server/tests/it/api/install.rs index 52082d254..934c7a06b 100644 --- a/lib/crates/fabro-server/tests/it/api/install.rs +++ b/lib/crates/fabro-server/tests/it/api/install.rs @@ -395,7 +395,7 @@ async fn token_install_finish_persists_settings_env_and_vault() { let server_env = std::fs::read_to_string( fabro_config::Storage::new(temp_dir.path()) - .runtime_state() + .runtime_directory() .env_path(), ) .unwrap(); @@ -556,7 +556,8 @@ async fn app_install_finish_omits_dev_token_and_does_not_write_it() { ); let server_env = - std::fs::read_to_string(Storage::new(temp_dir.path()).runtime_state().env_path()).unwrap(); + std::fs::read_to_string(Storage::new(temp_dir.path()).runtime_directory().env_path()) + .unwrap(); assert!(!server_env.contains("FABRO_DEV_TOKEN=")); assert!( @@ -565,7 +566,7 @@ async fn app_install_finish_omits_dev_token_and_does_not_write_it() { ); assert!( !Storage::new(temp_dir.path()) - .runtime_state() + .runtime_directory() .dev_token_path() .exists(), "storage dev token file should not be created for App installs" @@ -1372,7 +1373,7 @@ async fn install_finish_failure_restores_settings_and_vault_but_leaves_env_keys( "{ not valid json" ); - let server_env = std::fs::read_to_string(storage.runtime_state().env_path()).unwrap(); + let server_env = std::fs::read_to_string(storage.runtime_directory().env_path()).unwrap(); assert!(server_env.contains("SESSION_SECRET=")); assert!(server_env.contains("FABRO_DEV_TOKEN=")); assert!(!callback_invoked.load(Ordering::Acquire)); @@ -1441,10 +1442,10 @@ async fn install_finish_failure_leaves_home_dev_token_mirror_written() { let home_dev_token = dev_token::read_dev_token_file(&home.dev_token_path()) .expect("home dev token should exist"); let storage_dev_token = - dev_token::read_dev_token_file(&storage.runtime_state().dev_token_path()) + dev_token::read_dev_token_file(&storage.runtime_directory().dev_token_path()) .expect("storage dev token should exist"); assert_eq!(home_dev_token, storage_dev_token); - let server_env = std::fs::read_to_string(storage.runtime_state().env_path()).unwrap(); + let server_env = std::fs::read_to_string(storage.runtime_directory().env_path()).unwrap(); assert!(server_env.contains(&format!("FABRO_DEV_TOKEN={home_dev_token}"))); } diff --git a/lib/crates/fabro-server/tests/it/api/tcp.rs b/lib/crates/fabro-server/tests/it/api/tcp.rs index 9ff9a8f36..ce31ad152 100644 --- a/lib/crates/fabro-server/tests/it/api/tcp.rs +++ b/lib/crates/fabro-server/tests/it/api/tcp.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use std::time::Duration; use axum::http::StatusCode; -use fabro_config::{ServerRuntimeState, parse_settings_layer, resolve_server_from_file}; +use fabro_config::{RuntimeDirectory, parse_settings_layer, resolve_server_from_file}; use fabro_server::bind::Bind; use fabro_server::ip_allowlist::{IpAllowlist, IpAllowlistConfig}; use fabro_server::jwt_auth::{AuthMode, resolve_auth_mode_with_lookup}; @@ -64,7 +64,7 @@ fn write_test_config(tempdir: &TempDir, settings: &str) -> PathBuf { let config_path = tempdir.path().join("settings.toml"); std::fs::write(&config_path, settings).expect("test settings should write"); std::fs::write( - ServerRuntimeState::new(tempdir.path()).env_path(), + RuntimeDirectory::new(tempdir.path()).env_path(), format!("FABRO_DEV_TOKEN={TEST_DEV_TOKEN}\nSESSION_SECRET={TEST_SESSION_SECRET}\n"), ) .expect("test env file should write"); diff --git a/lib/crates/fabro-test/src/lib.rs b/lib/crates/fabro-test/src/lib.rs index 92a112121..8a36b94e8 100644 --- a/lib/crates/fabro-test/src/lib.rs +++ b/lib/crates/fabro-test/src/lib.rs @@ -629,7 +629,7 @@ fn write_settings_file(path: &Path, storage_dir: &Path, rest: &str) { } fn write_test_server_dev_token(storage_dir: &Path) { - let server_env_path = Storage::new(storage_dir).runtime_state().env_path(); + let server_env_path = Storage::new(storage_dir).runtime_directory().env_path(); envfile::merge_env_file(&server_env_path, [("FABRO_DEV_TOKEN", TEST_DEV_TOKEN)]) .unwrap_or_else(|err| panic!("failed to write {}: {err}", server_env_path.display())); } @@ -876,7 +876,7 @@ fn clear_server_storage(table: &mut TomlMap) { } fn server_record_path(storage_dir: &Path) -> PathBuf { - storage_dir.join("server.json") + Storage::new(storage_dir).runtime_directory().record_path() } fn server_record_pid(storage_dir: &Path) -> Option {