From 233328a3271c0a93d4d656621ff6acf9c44fc3ab Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sun, 25 Oct 2026 15:11:39 -0700 Subject: [PATCH 1/3] fix(cli): adopt the same-version daemon that owns the cache root; fail fast on a foreign owner Two fbuild builds that both report 2.5.44 derived different daemon ports (a wheel built before the #1700 port-window change: 60903; a dev build after it: 19142). The version-blind root-owner lock lets only one daemon run, so while the wheel's daemon was busy building, the dev CLI spawned three daemons that each exited with 'root-owner.lock busy' and then reported 'daemon did not become healthy after 3 spawn attempts' (~65 s). 'fbuild daemon status' already followed root-owner.json, so it showed the busy owner, while build/deploy did not. - get_daemon_port: after the endpoint port file, use the live owner claim's port when version, mode and cache root match. - ensure_direct_daemon_running: when a live owner of another version/identity holds the cache root on a different port, fail at once naming its pid/version/port/exe instead of doomed respawns. --- crates/fbuild-cli/src/daemon_client.rs | 39 +++++++ crates/fbuild-cli/src/daemon_client/tests.rs | 30 +++++- crates/fbuild-paths/src/lib.rs | 102 ++++++++++++++++++- 3 files changed, 167 insertions(+), 4 deletions(-) diff --git a/crates/fbuild-cli/src/daemon_client.rs b/crates/fbuild-cli/src/daemon_client.rs index 22d0467d..50b89dfc 100644 --- a/crates/fbuild-cli/src/daemon_client.rs +++ b/crates/fbuild-cli/src/daemon_client.rs @@ -1032,6 +1032,11 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { DaemonProbe::Restarted => true, DaemonProbe::NotRunning => false, }; + if !restarted { + if let Some(error) = foreign_root_owner_error() { + return Err(fbuild_core::FbuildError::DaemonError(error)); + } + } let spawned_pid = start_daemon(&client).await?; if restarted { // FastLED/fbuild#1476: whichever branch `start_daemon` took (spawned, @@ -1042,6 +1047,40 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> { Ok(()) } +/// A live daemon of another version or identity owns this cache root on a +/// port this CLI does not use. A daemon spawned now would only exit on +/// `root-owner.lock busy`, and three such spawns used to end in a misleading +/// "daemon did not become healthy" error while the owner was merely busy. +fn foreign_root_owner_error() -> Option { + let claim = fbuild_paths::daemon_ownership::read_owner_claim()?; + let own_port = fbuild_paths::get_daemon_port(); + let live = fbuild_core::platform::process::pid_is_alive(claim.pid) + && fbuild_core::platform::process::pid_exe_stem_matches( + claim.pid, + fbuild_paths::daemon_ownership::DAEMON_EXE_STEM, + ); + (live && claim.port != own_port).then(|| foreign_root_owner_message(&claim, own_port)) +} + +fn foreign_root_owner_message( + claim: &fbuild_paths::daemon_ownership::OwnerClaim, + own_port: u16, +) -> String { + format!( + "the fbuild cache root is owned by another fbuild-daemon (pid {}, version {}, mode {}, \ + port {}, exe {}), which this fbuild {} cannot share; it would use port {}. \ + No compilation was attempted. Wait for that daemon's work to finish and stop it \ + (`fbuild daemon stop`), or run the same fbuild build as the client that started it.", + claim.pid, + claim.version, + claim.mode, + claim.port, + claim.exe.display(), + env!("CARGO_PKG_VERSION"), + own_port + ) +} + enum DaemonProbe { /// A daemon is running and should be kept. Keep, diff --git a/crates/fbuild-cli/src/daemon_client/tests.rs b/crates/fbuild-cli/src/daemon_client/tests.rs index 2430bc59..e22b4e46 100644 --- a/crates/fbuild-cli/src/daemon_client/tests.rs +++ b/crates/fbuild-cli/src/daemon_client/tests.rs @@ -3,8 +3,9 @@ use super::{ DaemonAcquisition, DaemonClient, DaemonInfoResponse, HealthResponseFull, - broker_refusal_is_fatal, daemon_cache_identity_error, launcher_path, - restart_diag::SiblingDaemon, same_running_image, should_restart_daemon, wedged_daemon_note, + broker_refusal_is_fatal, daemon_cache_identity_error, foreign_root_owner_message, + launcher_path, restart_diag::SiblingDaemon, same_running_image, should_restart_daemon, + wedged_daemon_note, }; use running_process::broker::client::RefusalKind::{VersionBlocked, VersionUnsupported}; @@ -222,3 +223,28 @@ fn a_live_unresponsive_daemon_is_named_along_with_its_recovery() { fn no_live_daemon_produces_no_hint() { assert_eq!(wedged_daemon_note(None), ""); } + +/// A busy daemon of another build that owns the cache root must be named, +/// not reported as an unhealthy daemon after three doomed spawns. +#[test] +fn foreign_root_owner_message_names_the_owner() { + let claim = fbuild_paths::daemon_ownership::OwnerClaim { + pid: 98258, + exe: std::path::PathBuf::from("/venv/bin/fbuild-daemon").into(), + version: "2.5.44".to_string(), + mode: "prod".to_string(), + cache_root_key: "/c".to_string(), + port: 60903, + }; + let message = foreign_root_owner_message(&claim, 19142); + for needle in [ + "98258", + "2.5.44", + "60903", + "19142", + "/venv/bin/fbuild-daemon", + "No compilation", + ] { + assert!(message.contains(needle), "{needle} missing from: {message}"); + } +} diff --git a/crates/fbuild-paths/src/lib.rs b/crates/fbuild-paths/src/lib.rs index 4992ae19..294c909a 100644 --- a/crates/fbuild-paths/src/lib.rs +++ b/crates/fbuild-paths/src/lib.rs @@ -386,7 +386,10 @@ fn read_port_from_file(path: &Path) -> Option { /// Priority: /// 1. `FBUILD_DAEMON_PORT` environment variable (if set and valid 1–65535) /// 2. Port file for this endpoint (if it exists and is valid) -/// 3. [`default_daemon_port`] — a deterministic per-(version, cache-identity) +/// 3. The port of the live daemon that owns this cache root (its +/// `root-owner.json` claim), when it is the same version, mode and cache +/// root +/// 4. [`default_daemon_port`] — a deterministic per-(version, cache-identity) /// port /// /// FastLED/fbuild#1009: the endpoint is keyed by version+identity (via @@ -417,10 +420,57 @@ pub fn get_daemon_port() -> u16 { // `recorded_daemon_owner_is_dead`. } - // Priority 3: deterministic per-endpoint default. + // Priority 3: the live daemon that owns this cache root, when it is the + // same version and identity. FastLED/fbuild#1009 keys the endpoint on + // version+identity, but two builds that both report the same version can + // still derive different ports (e.g. a 2.5.44 wheel built before the + // #1700 port-window change and a 2.5.44 dev build after it). The + // version-blind root-ownership lock lets only one of them run, so a + // client that ignored the owner would spawn daemons that exit on + // `root-owner.lock busy` and then report the busy owner as unhealthy. + if let Some(port) = daemon_ownership::read_owner_claim().and_then(|claim| { + let identity = crate::running_process::DaemonCacheIdentity::discover(); + adoptable_owner_port( + &claim, + env!("CARGO_PKG_VERSION"), + identity.mode, + &identity.cache_root_key, + owner_claim_is_live, + ) + }) { + return port; + } + + // Priority 4: deterministic per-endpoint default. default_daemon_port() } +/// The owner claim's port, if the claim describes a live daemon this client +/// may share: same version, mode and cache root. Pure apart from `is_live`. +fn adoptable_owner_port( + claim: &daemon_ownership::OwnerClaim, + version: &str, + mode: &str, + cache_root_key: &str, + is_live: impl FnOnce(&daemon_ownership::OwnerClaim) -> bool, +) -> Option { + (claim.port > 0 + && claim.version == version + && claim.mode == mode + && claim.cache_root_key == cache_root_key + && is_live(claim)) + .then_some(claim.port) +} + +/// Positive evidence that the claim's pid is a running `fbuild-daemon`. +fn owner_claim_is_live(claim: &daemon_ownership::OwnerClaim) -> bool { + fbuild_core::platform::process::pid_is_alive(claim.pid) + && fbuild_core::platform::process::pid_exe_stem_matches( + claim.pid, + daemon_ownership::DAEMON_EXE_STEM, + ) +} + /// Can we *prove* the daemon that wrote this endpoint's records is gone? /// /// Fails safe: anything short of positive evidence of death returns `false`, @@ -611,6 +661,54 @@ mod tests { ); } + fn claim(version: &str, mode: &str, cache: &str, port: u16) -> daemon_ownership::OwnerClaim { + daemon_ownership::OwnerClaim { + pid: 42, + exe: std::path::PathBuf::from("/x/fbuild-daemon").into(), + version: version.to_string(), + mode: mode.to_string(), + cache_root_key: cache.to_string(), + port, + } + } + + /// Two same-version builds that derive different default ports must + /// still find the live daemon that owns their shared cache root. + #[test] + fn owner_claim_port_is_adopted_for_same_version_identity() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| true), + Some(60903) + ); + } + + #[test] + fn owner_claim_port_is_not_adopted_across_identity_or_when_dead() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| false), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.45", "prod", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "dev", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/d", |_| true), + None + ); + let zero = claim("2.5.44", "prod", "/c", 0); + assert_eq!( + adoptable_owner_port(&zero, "2.5.44", "prod", "/c", |_| true), + None + ); + } + #[test] fn endpoint_key_is_deterministic_16_hex() { // FastLED/fbuild#1009: the key must be deterministic per (version, From 27b9190327d56707222edf9f924dedcceea33d52 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sun, 25 Oct 2026 18:52:07 -0700 Subject: [PATCH 2/3] fix: split owner-port tests out of fbuild-paths lib.rs (1000-LOC gate); use NormalizedPath in OwnerClaim tests (ban_std_pathbuf) --- crates/fbuild-cli/src/daemon_client/tests.rs | 2 +- crates/fbuild-paths/src/lib.rs | 51 ++------------------ crates/fbuild-paths/src/owner_port_tests.rs | 51 ++++++++++++++++++++ 3 files changed, 55 insertions(+), 49 deletions(-) create mode 100644 crates/fbuild-paths/src/owner_port_tests.rs diff --git a/crates/fbuild-cli/src/daemon_client/tests.rs b/crates/fbuild-cli/src/daemon_client/tests.rs index e22b4e46..1f39fcb7 100644 --- a/crates/fbuild-cli/src/daemon_client/tests.rs +++ b/crates/fbuild-cli/src/daemon_client/tests.rs @@ -230,7 +230,7 @@ fn no_live_daemon_produces_no_hint() { fn foreign_root_owner_message_names_the_owner() { let claim = fbuild_paths::daemon_ownership::OwnerClaim { pid: 98258, - exe: std::path::PathBuf::from("/venv/bin/fbuild-daemon").into(), + exe: fbuild_core::path::NormalizedPath::new("/venv/bin/fbuild-daemon"), version: "2.5.44".to_string(), mode: "prod".to_string(), cache_root_key: "/c".to_string(), diff --git a/crates/fbuild-paths/src/lib.rs b/crates/fbuild-paths/src/lib.rs index 294c909a..4be3aaf2 100644 --- a/crates/fbuild-paths/src/lib.rs +++ b/crates/fbuild-paths/src/lib.rs @@ -13,6 +13,9 @@ pub mod executable_hash; pub mod long_paths; pub mod running_process; +#[cfg(test)] +mod owner_port_tests; + /// The project-local and home-local fbuild directory segment: `.fbuild`. /// /// This is the canonical spelling. Nothing outside `fbuild-paths` / @@ -661,54 +664,6 @@ mod tests { ); } - fn claim(version: &str, mode: &str, cache: &str, port: u16) -> daemon_ownership::OwnerClaim { - daemon_ownership::OwnerClaim { - pid: 42, - exe: std::path::PathBuf::from("/x/fbuild-daemon").into(), - version: version.to_string(), - mode: mode.to_string(), - cache_root_key: cache.to_string(), - port, - } - } - - /// Two same-version builds that derive different default ports must - /// still find the live daemon that owns their shared cache root. - #[test] - fn owner_claim_port_is_adopted_for_same_version_identity() { - let c = claim("2.5.44", "prod", "/c", 60903); - assert_eq!( - adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| true), - Some(60903) - ); - } - - #[test] - fn owner_claim_port_is_not_adopted_across_identity_or_when_dead() { - let c = claim("2.5.44", "prod", "/c", 60903); - assert_eq!( - adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| false), - None - ); - assert_eq!( - adoptable_owner_port(&c, "2.5.45", "prod", "/c", |_| true), - None - ); - assert_eq!( - adoptable_owner_port(&c, "2.5.44", "dev", "/c", |_| true), - None - ); - assert_eq!( - adoptable_owner_port(&c, "2.5.44", "prod", "/d", |_| true), - None - ); - let zero = claim("2.5.44", "prod", "/c", 0); - assert_eq!( - adoptable_owner_port(&zero, "2.5.44", "prod", "/c", |_| true), - None - ); - } - #[test] fn endpoint_key_is_deterministic_16_hex() { // FastLED/fbuild#1009: the key must be deterministic per (version, diff --git a/crates/fbuild-paths/src/owner_port_tests.rs b/crates/fbuild-paths/src/owner_port_tests.rs new file mode 100644 index 00000000..866fd1e3 --- /dev/null +++ b/crates/fbuild-paths/src/owner_port_tests.rs @@ -0,0 +1,51 @@ +//! Tests for `adoptable_owner_port` (split from lib.rs for the 1000-LOC gate). + +use super::{adoptable_owner_port, daemon_ownership}; + +fn claim(version: &str, mode: &str, cache: &str, port: u16) -> daemon_ownership::OwnerClaim { + daemon_ownership::OwnerClaim { + pid: 42, + exe: fbuild_core::path::NormalizedPath::new("/x/fbuild-daemon"), + version: version.to_string(), + mode: mode.to_string(), + cache_root_key: cache.to_string(), + port, + } +} + +/// Two same-version builds that derive different default ports must +/// still find the live daemon that owns their shared cache root. +#[test] +fn owner_claim_port_is_adopted_for_same_version_identity() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| true), + Some(60903) + ); +} + +#[test] +fn owner_claim_port_is_not_adopted_across_identity_or_when_dead() { + let c = claim("2.5.44", "prod", "/c", 60903); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/c", |_| false), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.45", "prod", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "dev", "/c", |_| true), + None + ); + assert_eq!( + adoptable_owner_port(&c, "2.5.44", "prod", "/d", |_| true), + None + ); + let zero = claim("2.5.44", "prod", "/c", 0); + assert_eq!( + adoptable_owner_port(&zero, "2.5.44", "prod", "/c", |_| true), + None + ); +} From e772a414f06e10ef9d54eb6d34c6f474572d3366 Mon Sep 17 00:00:00 2001 From: Zach Vorhies Date: Sun, 25 Oct 2026 19:03:56 -0700 Subject: [PATCH 3/3] test: avoid pid literal tripped by USB VID/PID source guard --- crates/fbuild-paths/src/owner_port_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fbuild-paths/src/owner_port_tests.rs b/crates/fbuild-paths/src/owner_port_tests.rs index 866fd1e3..3f0bb97e 100644 --- a/crates/fbuild-paths/src/owner_port_tests.rs +++ b/crates/fbuild-paths/src/owner_port_tests.rs @@ -4,7 +4,7 @@ use super::{adoptable_owner_port, daemon_ownership}; fn claim(version: &str, mode: &str, cache: &str, port: u16) -> daemon_ownership::OwnerClaim { daemon_ownership::OwnerClaim { - pid: 42, + pid: std::process::id(), exe: fbuild_core::path::NormalizedPath::new("/x/fbuild-daemon"), version: version.to_string(), mode: mode.to_string(),