From 83d5adba66ddc6078461988f553a9ac773a04721 Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Wed, 13 May 2026 23:29:34 +0530 Subject: [PATCH] feat(local_ai): bind owned ollama serve lifecycle to openhuman (#1622) (#1638) Co-authored-by: Claude Opus 4.7 (1M context) --- src/core/jsonrpc.rs | 20 ++ src/openhuman/local_ai/core.rs | 9 + src/openhuman/local_ai/paths.rs | 11 + src/openhuman/local_ai/service/bootstrap.rs | 1 + src/openhuman/local_ai/service/mod.rs | 5 + .../local_ai/service/ollama_admin.rs | 197 ++++++++++++-- .../local_ai/service/ollama_admin_tests.rs | 170 ++++++++++++ .../local_ai/service/spawn_marker.rs | 243 ++++++++++++++++++ 8 files changed, 629 insertions(+), 27 deletions(-) create mode 100644 src/openhuman/local_ai/service/spawn_marker.rs diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 8278eb839..6f07b4925 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -994,6 +994,26 @@ async fn run_server_inner( .with_graceful_shutdown(crate::core::shutdown::signal()) .await?; } + + // Server has stopped accepting and in-flight requests drained. + // Kill any `ollama serve` openhuman itself spawned (no-op when the + // daemon was externally managed) and clear the spawn marker so the + // next launch doesn't try to reclaim a daemon that's already dead. + // Bounded so a wedged Ollama can't hold up app shutdown. + if let Some(svc) = crate::openhuman::local_ai::try_global() { + let cfg = crate::openhuman::config::Config::load_or_init() + .await + .unwrap_or_default(); + log::info!("[core] shutdown: cleaning up openhuman-owned ollama if any"); + let shutdown_fut = svc.shutdown_owned_ollama(&cfg); + if tokio::time::timeout(std::time::Duration::from_secs(2), shutdown_fut) + .await + .is_err() + { + log::warn!("[core] shutdown: ollama cleanup exceeded 2s budget; proceeding with exit"); + } + } + Ok(()) } diff --git a/src/openhuman/local_ai/core.rs b/src/openhuman/local_ai/core.rs index 6331f75f8..07bfb6415 100644 --- a/src/openhuman/local_ai/core.rs +++ b/src/openhuman/local_ai/core.rs @@ -14,6 +14,15 @@ pub fn global(config: &Config) -> Arc { .clone() } +/// Like [`global`] but returns `None` instead of initialising the singleton. +/// +/// Useful from shutdown paths where lazy-creating the service just to call a +/// no-op cleanup would be wasteful — if local AI was never used in this +/// process, there's nothing to clean up. +pub fn try_global() -> Option> { + LOCAL_AI.get().cloned() +} + pub fn model_artifact_path(config: &Config) -> PathBuf { let root = crate::openhuman::config::default_root_openhuman_dir().unwrap_or_else(|_| { config diff --git a/src/openhuman/local_ai/paths.rs b/src/openhuman/local_ai/paths.rs index 8a0dc3980..50639a10d 100644 --- a/src/openhuman/local_ai/paths.rs +++ b/src/openhuman/local_ai/paths.rs @@ -72,6 +72,17 @@ pub(crate) fn workspace_local_models_dir(config: &Config) -> PathBuf { shared_root_dir(config).join("models").join("local-ai") } +/// Spawn marker file recording the PID of any `ollama serve` openhuman +/// itself spawned. Read on next launch to recognise our own orphan when +/// openhuman crashed before its graceful-shutdown hook ran. Lives under +/// the shared root so it survives per-user config rewrites and sits next +/// to the workspace install dir. +pub(crate) fn ollama_spawn_marker_path(config: &Config) -> PathBuf { + shared_root_dir(config) + .join("local-ai") + .join("ollama.spawn") +} + pub(crate) fn resolve_whisper_binary() -> Option { if let Some(from_env) = std::env::var("WHISPER_BIN") .ok() diff --git a/src/openhuman/local_ai/service/bootstrap.rs b/src/openhuman/local_ai/service/bootstrap.rs index 7dab36cc8..90441ca90 100644 --- a/src/openhuman/local_ai/service/bootstrap.rs +++ b/src/openhuman/local_ai/service/bootstrap.rs @@ -47,6 +47,7 @@ impl LocalAiService { bootstrap_lock: tokio::sync::Mutex::new(()), whisper_load_lock: tokio::sync::Mutex::new(()), last_memory_summary_at: parking_lot::Mutex::new(None), + owned_ollama: parking_lot::Mutex::new(None), http: reqwest::Client::builder() // Local models can take >30s on cold start and first-token generation. // Keep the total timeout generous so inline autocomplete and local diff --git a/src/openhuman/local_ai/service/mod.rs b/src/openhuman/local_ai/service/mod.rs index b36cb13f7..0deff2fb4 100644 --- a/src/openhuman/local_ai/service/mod.rs +++ b/src/openhuman/local_ai/service/mod.rs @@ -4,6 +4,7 @@ mod assets; mod bootstrap; mod ollama_admin; mod public_infer; +pub(crate) mod spawn_marker; mod speech; mod vision_embed; pub(crate) mod whisper_engine; @@ -19,4 +20,8 @@ pub struct LocalAiService { pub(crate) http: reqwest::Client, /// In-process whisper.cpp context for low-latency STT. pub(crate) whisper: whisper_engine::WhisperEngineHandle, + /// Handle to any `ollama serve` openhuman itself spawned. `None` when + /// the daemon currently on `:11434` was started outside openhuman (and + /// adopted via the health probe) — those are never killed on exit. + pub(crate) owned_ollama: Mutex>, } diff --git a/src/openhuman/local_ai/service/ollama_admin.rs b/src/openhuman/local_ai/service/ollama_admin.rs index f8744238a..de7fe03d4 100644 --- a/src/openhuman/local_ai/service/ollama_admin.rs +++ b/src/openhuman/local_ai/service/ollama_admin.rs @@ -13,6 +13,7 @@ use crate::openhuman::local_ai::paths::{find_workspace_ollama_binary, workspace_ use crate::openhuman::local_ai::presets::{self, VisionMode}; use crate::openhuman::local_ai::process_util::apply_no_window; +use super::spawn_marker::{self, OllamaSpawnMarker}; use super::LocalAiService; impl LocalAiService { @@ -20,19 +21,33 @@ impl LocalAiService { &self, config: &Config, ) -> Result<(), String> { + // If openhuman crashed last session and left a daemon running, the + // spawn marker lets us recognise it and reclaim it (kill + respawn + // under owned-child tracking) instead of either leaking it forever + // or hitting an external daemon that just happens to be on :11434. + self.reclaim_orphan_if_ours(config).await; + if self.ollama_healthy().await { // Server is running — verify it can actually execute models by checking // if the runner works. A stale server with a missing binary will 500. if self.ollama_runner_ok().await { return Ok(()); } - // Runner is broken (e.g. binary moved). Kill stale server and restart. - log::warn!("[local_ai] Ollama server responds but runner is broken, restarting"); + // Runner is broken (e.g. binary moved). + log::warn!("[local_ai] Ollama server responds but runner is broken"); + // Only restart if we own it. Killing an external daemon's + // broken runner is the user's job, not ours — friendly-fire. self.kill_ollama_server().await; + if self.ollama_healthy().await { + // Our kill was a no-op (or didn't take effect) — daemon is external. + return Err("An external Ollama daemon on :11434 has a broken runner. \ + Restart it manually (or stop it so openhuman can take over)." + .to_string()); + } } let ollama_cmd = self.resolve_or_install_ollama_binary(config).await?; - self.start_and_wait_for_server(&ollama_cmd).await + self.start_and_wait_for_server(config, &ollama_cmd).await } /// Like `ensure_ollama_server`, but forces a fresh install of the Ollama binary @@ -49,17 +64,74 @@ impl LocalAiService { let system_bin = find_system_ollama_binary() .ok_or_else(|| "Ollama installed but binary not found on system".to_string())?; // Try to use the system binary directly. - return self.start_and_wait_for_server(&system_bin).await; + return self.start_and_wait_for_server(config, &system_bin).await; }; - self.start_and_wait_for_server(&ollama_cmd).await + self.start_and_wait_for_server(config, &ollama_cmd).await } - async fn start_and_wait_for_server(&self, ollama_cmd: &Path) -> Result<(), String> { + /// Check if a healthy daemon on `:11434` is actually openhuman's own + /// orphan from a prior session (i.e. we crashed before the graceful + /// shutdown hook fired). If so, kill it so the upcoming spawn can + /// resume owned-child tracking. External daemons are never touched. + async fn reclaim_orphan_if_ours(&self, config: &Config) { + let Some(marker) = spawn_marker::read_marker(config) else { + return; + }; + if !spawn_marker::pid_is_alive(marker.pid) { + log::debug!( + "[local_ai] stale ollama spawn marker (pid={} no longer alive); clearing", + marker.pid + ); + spawn_marker::clear_marker(config); + return; + } + if !self.ollama_healthy().await { + // PID is alive but :11434 isn't healthy — either Ollama is + // mid-boot or the recorded PID was reused for an unrelated + // process. Leave the marker; either the daemon will come up + // and the next call will reclaim it, or `start_and_wait_for_server` + // will overwrite it on a fresh spawn. + log::debug!( + "[local_ai] ollama spawn marker pid={} alive but :11434 not healthy yet; \ + deferring reclaim", + marker.pid + ); + return; + } + log::info!( + "[local_ai] reclaiming openhuman-owned ollama orphan from prior session \ + (pid={}, binary={})", + marker.pid, + marker.binary_path + ); + kill_pid_by_id(marker.pid); + spawn_marker::clear_marker(config); + // Brief settle so the listener releases :11434 before we respawn. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + } + + async fn start_and_wait_for_server( + &self, + config: &Config, + ollama_cmd: &Path, + ) -> Result<(), String> { if self.ollama_healthy().await { + // A daemon is already up — adopt it. We did NOT spawn it (or any + // prior spawn was already reclaimed in `reclaim_orphan_if_ours`), + // so `owned_ollama` stays `None` and the daemon survives openhuman + // exit. This is the contract: external/adopted daemons are never + // killed; only our own children die with us. return Ok(()); } + // Defensive: if a previous spawn attempt left a stale `Child` in + // `owned_ollama` (e.g. ensure_ollama_server_fresh after a failed + // first pass), clear it before respawning. Without this, the new + // child would replace the field and the old one would be leaked. + self.kill_ollama_server().await; + spawn_marker::clear_marker(config); + let mut version_cmd = tokio::process::Command::new(ollama_cmd); version_cmd .arg("--version") @@ -135,6 +207,27 @@ impl LocalAiService { for _ in 0..20 { if self.ollama_healthy().await { + // Daemon is up. Take ownership so we can kill it on exit and + // write the spawn marker so a crashed openhuman can reclaim + // this PID on next launch instead of orphaning it forever. + let pid = serve_child.id().unwrap_or(0); + if pid == 0 { + log::warn!( + "[local_ai] spawned ollama child has no PID — owned-child kill \ + will be a no-op but daemon is healthy, continuing" + ); + } else { + let marker = OllamaSpawnMarker::new(pid, ollama_cmd); + if let Err(e) = spawn_marker::write_marker(config, &marker) { + // Marker write failure is non-fatal — graceful shutdown + // still kills via the in-memory `Child` handle. Only + // crash-recovery on next launch is degraded. + log::warn!( + "[local_ai] failed to write ollama spawn marker (pid={pid}): {e}" + ); + } + } + *self.owned_ollama.lock() = Some(serve_child); return Ok(()); } tokio::time::sleep(std::time::Duration::from_millis(300)).await; @@ -1044,29 +1137,52 @@ impl LocalAiService { } /// Kill any running Ollama server process so we can restart with the correct binary. + /// Kill the `ollama serve` daemon openhuman itself spawned, if any. + /// + /// **No-op when openhuman never spawned a daemon** (i.e. it adopted an + /// externally-managed one via the `ollama_healthy()` fast-path, or no + /// daemon was started at all). This avoids the friendly-fire bug from + /// the previous blanket `taskkill /IM ollama.exe` / `pkill -f` which + /// would terminate any Ollama on the host — including ones started by + /// the user's CLI, tray app, or other tooling. + /// + /// External daemons can be replaced/restarted by the user; killing + /// them out from under their owner is never the right move from inside + /// a desktop app. async fn kill_ollama_server(&self) { - #[cfg(unix)] - { - let _ = tokio::process::Command::new("pkill") - .arg("-f") - .arg("ollama serve") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .await; - // Give it a moment to die. - tokio::time::sleep(std::time::Duration::from_millis(500)).await; - } - #[cfg(windows)] - { - let mut cmd = tokio::process::Command::new("taskkill"); - cmd.args(["/F", "/IM", "ollama.exe"]) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()); - apply_no_window(&mut cmd); - let _ = cmd.status().await; - tokio::time::sleep(std::time::Duration::from_millis(500)).await; + let maybe_child = self.owned_ollama.lock().take(); + let Some(mut child) = maybe_child else { + log::debug!( + "[local_ai] kill_ollama_server: no openhuman-owned daemon; \ + leaving any external Ollama on :11434 untouched" + ); + return; + }; + let pid = child.id().unwrap_or(0); + match child.kill().await { + Ok(()) => { + log::info!("[local_ai] killed openhuman-owned ollama serve (pid={pid})"); + // Reap so the OS doesn't keep the zombie around on Unix. + let _ = child.wait().await; + } + Err(err) => { + log::warn!("[local_ai] kill of owned ollama serve pid={pid} failed: {err}"); + } } + // Give the kernel a moment to release :11434 before any imminent + // respawn races for the same port. + tokio::time::sleep(std::time::Duration::from_millis(500)).await; + } + + /// Public shutdown hook for the Tauri exit lifecycle. + /// + /// Kills the openhuman-owned `ollama serve` (if any) and clears the + /// spawn marker so the next launch doesn't try to reclaim a daemon + /// that's already dead. Idempotent — safe to call from both + /// `RunEvent::ExitRequested` and window-close paths. + pub async fn shutdown_owned_ollama(&self, config: &Config) { + self.kill_ollama_server().await; + spawn_marker::clear_marker(config); } pub(in crate::openhuman::local_ai::service) async fn has_model( @@ -1128,6 +1244,33 @@ fn interrupted_pull_settle_window_secs(observed_bytes: bool, settle_window_secs: } } +/// Kill a process by PID using `sysinfo`'s cross-platform `Process::kill`. +/// +/// Used by `reclaim_orphan_if_ours` where we no longer have the original +/// `tokio::process::Child` handle (the spawning openhuman crashed) but +/// recorded the PID in the spawn marker. +fn kill_pid_by_id(pid: u32) { + use sysinfo::{Pid, ProcessesToUpdate, System}; + let target = Pid::from_u32(pid); + let mut sys = System::new(); + sys.refresh_processes(ProcessesToUpdate::Some(&[target]), true); + match sys.process(target) { + Some(proc) => { + if proc.kill() { + log::info!("[local_ai] killed reclaimed ollama orphan pid={pid}"); + } else { + // sysinfo's kill returns false if the platform refused + // (permissions, race with exit). The next ollama_healthy() + // check will reveal whether the daemon is actually gone. + log::warn!("[local_ai] sysinfo Process::kill returned false for pid={pid}"); + } + } + None => { + log::debug!("[local_ai] kill_pid_by_id: pid={pid} no longer present"); + } + } +} + #[cfg(test)] #[path = "ollama_admin_tests.rs"] mod tests; diff --git a/src/openhuman/local_ai/service/ollama_admin_tests.rs b/src/openhuman/local_ai/service/ollama_admin_tests.rs index b55d2f64c..9e34ad1dd 100644 --- a/src/openhuman/local_ai/service/ollama_admin_tests.rs +++ b/src/openhuman/local_ai/service/ollama_admin_tests.rs @@ -412,3 +412,173 @@ async fn list_models_errors_on_non_success() { std::env::remove_var("OPENHUMAN_OLLAMA_BASE_URL"); } } + +// ---- owned-PID lifecycle ------------------------------------------------ +// +// These tests pin the contract that `kill_ollama_server` only touches +// daemons openhuman spawned itself, and that the kill path actually +// reaches the child process (the previous `taskkill /F /IM ollama.exe` / +// `pkill -f` would terminate any Ollama on the host, including ones the +// user started outside openhuman — the issue #1622 friendly-fire bug). + +#[tokio::test] +async fn kill_ollama_server_with_no_owned_child_is_noop() { + let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX + .lock() + .expect("local ai mutex"); + + let config = Config::default(); + let service = LocalAiService::new(&config); + + // A fresh service has never spawned anything, so `owned_ollama` is `None`. + assert!( + service.owned_ollama.lock().is_none(), + "owned_ollama must start as None" + ); + + // Must complete without panicking and leave the field None — i.e. + // never reach for an external daemon when there's nothing to kill. + service.kill_ollama_server().await; + + assert!( + service.owned_ollama.lock().is_none(), + "owned_ollama must stay None after a no-op kill" + ); +} + +#[tokio::test] +async fn kill_ollama_server_kills_owned_child() { + let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX + .lock() + .expect("local ai mutex"); + + let config = Config::default(); + let service = LocalAiService::new(&config); + + // Spawn a long-lived child we fully control. We need something that + // sleeps for longer than the test's worst-case settle window so it + // can't exit on its own before our kill lands. + let mut cmd = if cfg!(windows) { + let mut c = tokio::process::Command::new("powershell"); + c.args(["-NoProfile", "-Command", "Start-Sleep -Seconds 30"]); + c + } else { + let mut c = tokio::process::Command::new("sleep"); + c.arg("30"); + c + }; + cmd.stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()); + let child = cmd.spawn().expect("spawn sleep/Start-Sleep child"); + let pid = child.id().expect("child pid available"); + *service.owned_ollama.lock() = Some(child); + + // Sanity: child should be alive immediately after spawn. + assert!( + crate::openhuman::local_ai::service::spawn_marker::pid_is_alive(pid), + "child pid {pid} should be alive right after spawn" + ); + + service.kill_ollama_server().await; + + // Owned slot is cleared — `take()` happened. + assert!( + service.owned_ollama.lock().is_none(), + "kill_ollama_server must take() the owned child" + ); + + // PID should no longer be alive. Allow a brief settle for the OS to + // update its process table — the kill is signalled but reap is async. + let mut still_alive = true; + for _ in 0..40 { + if !crate::openhuman::local_ai::service::spawn_marker::pid_is_alive(pid) { + still_alive = false; + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!( + !still_alive, + "child pid {pid} should be dead within 2s of kill_ollama_server" + ); +} + +#[tokio::test] +async fn shutdown_owned_ollama_clears_marker_and_kills_child() { + let _guard = crate::openhuman::local_ai::LOCAL_AI_TEST_MUTEX + .lock() + .expect("local ai mutex"); + + // Redirect the workspace root to a tempdir so the marker file doesn't + // touch the real `~/.openhuman/`. Per `paths::shared_root_dir`, when + // `default_root_openhuman_dir()` errors, it falls back to + // `config_root_dir(config)` — which is `config.config_path.parent()`. + let tmp = tempfile::tempdir().unwrap(); + let mut config = Config::default(); + config.workspace_dir = tmp.path().to_path_buf(); + config.config_path = tmp.path().join("config.toml"); + + let service = LocalAiService::new(&config); + + // Spawn the same long-running stub. + let mut cmd = if cfg!(windows) { + let mut c = tokio::process::Command::new("powershell"); + c.args(["-NoProfile", "-Command", "Start-Sleep -Seconds 30"]); + c + } else { + let mut c = tokio::process::Command::new("sleep"); + c.arg("30"); + c + }; + cmd.stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()); + let child = cmd.spawn().expect("spawn child"); + let pid = child.id().expect("pid"); + *service.owned_ollama.lock() = Some(child); + + // Write a marker (mimicking what `start_and_wait_for_server` would do + // on a successful spawn) so we can verify shutdown clears it. + // + // NOTE: This test only verifies the shutdown path itself; it does not + // assert the marker survives the `default_root_openhuman_dir()` + // resolution on every CI environment. On hosts where the fallback + // resolves to a writable temp path, the write is exercised. On hosts + // where `default_root_openhuman_dir()` succeeds against the real home + // dir, we skip the marker assertion to avoid touching `~/.openhuman/`. + let marker_path = crate::openhuman::local_ai::paths::ollama_spawn_marker_path(&config); + let marker_writable = marker_path.starts_with(tmp.path()); + if marker_writable { + crate::openhuman::local_ai::service::spawn_marker::write_marker_at( + &marker_path, + &crate::openhuman::local_ai::service::spawn_marker::OllamaSpawnMarker::new( + pid, + std::path::Path::new("test-stub"), + ), + ) + .expect("write marker"); + assert!(marker_path.exists(), "marker should exist before shutdown"); + } + + service.shutdown_owned_ollama(&config).await; + + // Owned handle is gone. + assert!(service.owned_ollama.lock().is_none()); + + if marker_writable { + assert!( + !marker_path.exists(), + "shutdown_owned_ollama must clear the spawn marker" + ); + } + + // And the spawned process is dead. + let mut still_alive = true; + for _ in 0..40 { + if !crate::openhuman::local_ai::service::spawn_marker::pid_is_alive(pid) { + still_alive = false; + break; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + assert!(!still_alive, "spawned stub pid {pid} should be dead"); +} diff --git a/src/openhuman/local_ai/service/spawn_marker.rs b/src/openhuman/local_ai/service/spawn_marker.rs new file mode 100644 index 000000000..e8f5dd59e --- /dev/null +++ b/src/openhuman/local_ai/service/spawn_marker.rs @@ -0,0 +1,243 @@ +//! Spawn marker for openhuman-owned `ollama serve` processes. +//! +//! Every time `start_and_wait_for_server` actually spawns an Ollama daemon +//! (i.e. didn't adopt a healthy external one), we write a small JSON file +//! recording the PID, the binary we launched, and the openhuman process +//! that owned it. On graceful shutdown the marker is cleared. If openhuman +//! crashes before its shutdown hook fires, the marker survives — and on +//! next launch we can reclaim the orphaned daemon (kill + respawn fresh) +//! instead of either leaking it forever or running blanket `taskkill /IM +//! ollama.exe` and hitting daemons we don't own. +//! +//! Liveness is checked via `sysinfo` (already a workspace dep) to match +//! the cross-platform pattern in `install::is_ollama_installer_running`. + +use std::path::Path; + +use serde::{Deserialize, Serialize}; + +use crate::openhuman::config::Config; +use crate::openhuman::local_ai::paths::ollama_spawn_marker_path; + +/// On-disk record of an openhuman-spawned `ollama serve` process. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub(crate) struct OllamaSpawnMarker { + pub pid: u32, + pub started_at_unix: u64, + pub binary_path: String, + pub openhuman_pid: u32, +} + +impl OllamaSpawnMarker { + pub(crate) fn new(pid: u32, binary_path: &Path) -> Self { + let started_at_unix = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + Self { + pid, + started_at_unix, + binary_path: binary_path.display().to_string(), + openhuman_pid: std::process::id(), + } + } +} + +// ---- Path-keyed helpers (testable without touching ~/.openhuman/) ------- + +/// Write `marker` to `path`, replacing any existing file. Creates the +/// parent directory if needed. Uses a tmp-and-rename so a crash mid-write +/// can't leave truncated JSON. +pub(crate) fn write_marker_at(path: &Path, marker: &OllamaSpawnMarker) -> Result<(), String> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .map_err(|e| format!("create marker dir {}: {e}", parent.display()))?; + } + let json = + serde_json::to_string_pretty(marker).map_err(|e| format!("serialize spawn marker: {e}"))?; + + let tmp = path.with_extension("spawn.tmp"); + std::fs::write(&tmp, json).map_err(|e| format!("write marker tmp {}: {e}", tmp.display()))?; + std::fs::rename(&tmp, path) + .map_err(|e| format!("rename marker {} -> {}: {e}", tmp.display(), path.display()))?; + Ok(()) +} + +pub(crate) fn read_marker_at(path: &Path) -> Option { + let content = std::fs::read_to_string(path).ok()?; + match serde_json::from_str::(&content) { + Ok(m) => Some(m), + Err(e) => { + log::warn!( + "[local_ai] ollama spawn marker at {} is unparseable, ignoring: {e}", + path.display() + ); + None + } + } +} + +pub(crate) fn clear_marker_at(path: &Path) { + match std::fs::remove_file(path) { + Ok(()) => log::debug!( + "[local_ai] cleared ollama spawn marker at {}", + path.display() + ), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => log::warn!( + "[local_ai] failed to clear ollama spawn marker {}: {e}", + path.display() + ), + } +} + +// ---- Config-keyed conveniences (used by production code) --------------- + +pub(crate) fn write_marker(config: &Config, marker: &OllamaSpawnMarker) -> Result<(), String> { + let path = ollama_spawn_marker_path(config); + write_marker_at(&path, marker)?; + log::debug!( + "[local_ai] wrote ollama spawn marker pid={} bin={} at {}", + marker.pid, + marker.binary_path, + path.display() + ); + Ok(()) +} + +pub(crate) fn read_marker(config: &Config) -> Option { + read_marker_at(&ollama_spawn_marker_path(config)) +} + +pub(crate) fn clear_marker(config: &Config) { + clear_marker_at(&ollama_spawn_marker_path(config)); +} + +/// True iff `pid` corresponds to a live process on this machine. +/// +/// Uses `sysinfo` rather than libc/Win32 directly to stay consistent with +/// the rest of the local_ai module (see `install::is_ollama_installer_running`). +pub(crate) fn pid_is_alive(pid: u32) -> bool { + use sysinfo::{Pid, ProcessesToUpdate, System}; + let mut sys = System::new(); + let target = Pid::from_u32(pid); + // Refresh just the one PID we care about; cheap on Windows where a full + // refresh can take ~tens of ms on a loaded machine. + sys.refresh_processes(ProcessesToUpdate::Some(&[target]), true); + sys.process(target).is_some() +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn tmp_marker_path() -> (TempDir, std::path::PathBuf) { + let tmp = TempDir::new().unwrap(); + let path = tmp.path().join("local-ai").join("ollama.spawn"); + (tmp, path) + } + + #[test] + fn marker_round_trips_through_disk() { + let (_tmp, path) = tmp_marker_path(); + let m = OllamaSpawnMarker { + pid: 4242, + started_at_unix: 1_700_000_000, + binary_path: "C:\\fake\\ollama.exe".to_string(), + openhuman_pid: 9001, + }; + + write_marker_at(&path, &m).expect("write marker"); + let loaded = read_marker_at(&path).expect("read marker"); + assert_eq!(loaded, m); + + clear_marker_at(&path); + assert!( + read_marker_at(&path).is_none(), + "marker must be gone after clear" + ); + } + + #[test] + fn read_marker_returns_none_when_file_missing() { + let (_tmp, path) = tmp_marker_path(); + assert!(read_marker_at(&path).is_none()); + } + + #[test] + fn read_marker_returns_none_on_corrupt_json() { + let (_tmp, path) = tmp_marker_path(); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, b"{ not valid json").unwrap(); + + assert!( + read_marker_at(&path).is_none(), + "corrupt marker must be treated as absent" + ); + } + + #[test] + fn clear_marker_is_idempotent() { + let (_tmp, path) = tmp_marker_path(); + clear_marker_at(&path); + clear_marker_at(&path); + } + + #[test] + fn write_marker_creates_missing_parent_dir() { + let (_tmp, path) = tmp_marker_path(); + // path.parent() does NOT exist yet — write should create it. + assert!(!path.parent().unwrap().exists()); + let m = OllamaSpawnMarker::new(1234, std::path::Path::new("ollama")); + write_marker_at(&path, &m).expect("write"); + assert!(path.exists()); + } + + #[test] + fn new_marker_captures_current_process_id() { + let m = OllamaSpawnMarker::new(4242, std::path::Path::new("ollama")); + assert_eq!(m.openhuman_pid, std::process::id()); + assert_eq!(m.pid, 4242); + assert_eq!(m.binary_path, "ollama"); + } + + #[test] + fn pid_is_alive_recognises_self() { + let me = std::process::id(); + assert!( + pid_is_alive(me), + "current process PID {me} should be reported alive" + ); + } + + #[test] + fn pid_is_alive_rejects_dead_pid() { + // Spawn a short child, wait for it to exit, then check that its + // recycled PID is no longer reported alive. Hardcoded sentinel PIDs + // (0, u32::MAX) are unreliable cross-platform — on Windows PID 0 is + // "System Idle Process" and registers as alive in sysinfo. + let child = if cfg!(windows) { + std::process::Command::new("cmd") + .args(["/C", "exit 0"]) + .spawn() + .expect("spawn cmd /C exit") + } else { + std::process::Command::new("true") + .spawn() + .expect("spawn /usr/bin/true") + }; + let pid = child.id(); + let mut child = child; + let _ = child.wait(); + + // Give the OS a moment to fully reap so sysinfo doesn't catch a + // lingering zombie entry. + std::thread::sleep(std::time::Duration::from_millis(200)); + + assert!( + !pid_is_alive(pid), + "exited child pid {pid} should not be reported alive" + ); + } +}