fix(cli): retire a leftover daemon and retry once instead of failing after 3 spawns (#1627)

crates/fbuild-cli/src/cli/daemon_stop.rs

@@ -80,6 +80,72 @@ const GRACEFUL_STOP_BUDGET: std::time::Duration = std::time::Duration::from_secs
 /// is not going.
 const TERMINATION_BUDGET: std::time::Duration = std::time::Duration::from_secs(5);
 
+/// What a CLI that has just failed every spawn attempt should do next
+/// (FastLED/fbuild#1627).
+///
+/// Three failed attempts used to be terminal. Two of the three reported causes
+/// are recoverable without a human: the endpoint came up late (`Adopt` — the
+/// same adoption the spawn loop already performs before each attempt), or a
+/// leftover daemon from an earlier step still owns the cache root / endpoint
+/// and is no longer answering (`RetireAndRetry`). Only "nothing is alive and
+/// nothing answers" is genuinely hopeless at this point.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum SpawnRecovery {
+    /// The endpoint answers after all: attach to it, spawn nothing.
+    Adopt,
+    /// A PID verified to be an fbuild-daemon is alive while the endpoint is
+    /// silent. Retire it, then make one final spawn attempt.
+    RetireAndRetry(u32),
+    /// Nothing alive to retire and nothing answering: report the failure.
+    GiveUp,
+}
+
+/// Decide the post-failure step. `live_daemon_pid` comes from
+/// [`recorded_daemon_pid`], so it is already fail-closed on process identity.
+pub(crate) fn plan_spawn_recovery(
+    endpoint_healthy: bool,
+    live_daemon_pid: Option<u32>,
+) -> SpawnRecovery {
+    match (endpoint_healthy, live_daemon_pid) {
+        // Never shoot a daemon that answers: it is serving somebody, and
+        // adopting it is exactly what the failed spawn was trying to achieve.
+        (true, _) => SpawnRecovery::Adopt,
+        (false, Some(pid)) => SpawnRecovery::RetireAndRetry(pid),
+        (false, None) => SpawnRecovery::GiveUp,
+    }
+}
+
+/// Retire the unresponsive leftover daemon, if there is one, and report the
+/// PID that was retired.
+///
+/// Deliberately built on [`recorded_daemon_pid`]'s verified-identity rule and
+/// the same terminate-and-confirm escalation `daemon stop` uses, so this
+/// codebase still kills daemon processes in exactly one way. A retirement that
+/// fails is not fatal — the caller reports its original spawn failure either
+/// way, with the retirement noted in the message.
+pub(crate) async fn retire_unresponsive_for_recovery(client: &DaemonClient) -> Option<u32> {
+    match plan_spawn_recovery(client.health().await, recorded_daemon_pid()) {
+        SpawnRecovery::RetireAndRetry(pid) => {
+            tracing::warn!(
+                pid,
+                "every spawn attempt failed and PID {pid} is an unresponsive fbuild-daemon; \
+                 retiring it before one final attempt (FastLED/fbuild#1627)"
+            );
+            match terminate_and_confirm(pid).await {
+                Ok(()) => {
+                    clear_daemon_records();
+                    Some(pid)
+                }
+                Err(error) => {
+                    tracing::warn!(pid, %error, "could not retire the unresponsive daemon");
+                    None
+                }
+            }
+        }
+        SpawnRecovery::Adopt | SpawnRecovery::GiveUp => None,
+    }
+}
+
 /// `fbuild daemon stop` — stop the daemon for *this* endpoint and report what
 /// actually happened.
 ///

crates/fbuild-cli/src/cli/daemon_stop_tests.rs

@@ -62,3 +62,27 @@ fn a_live_non_daemon_pid_fails_the_identity_gate() {
         "the test binary is not an fbuild-daemon, so the gate must reject it"
     );
 }
+/// FastLED/fbuild#1627: after every spawn attempt fails, an unresponsive
+/// daemon that still owns this endpoint's cache root is retired and retried
+/// instead of failing the build outright.
+#[test]
+fn a_failed_spawn_retires_a_verified_unresponsive_daemon() {
+    assert_eq!(
+        plan_spawn_recovery(false, Some(4321)),
+        SpawnRecovery::RetireAndRetry(4321)
+    );
+}
+
+#[test]
+fn a_failed_spawn_with_nothing_alive_to_retire_gives_up() {
+    assert_eq!(plan_spawn_recovery(false, None), SpawnRecovery::GiveUp);
+}
+
+/// A daemon that answers is never retired, however the spawn attempts went:
+/// it may be serving a concurrent build, and adopting it is what the failed
+/// spawn was trying to do in the first place.
+#[test]
+fn a_healthy_endpoint_is_adopted_never_retired() {
+    assert_eq!(plan_spawn_recovery(true, Some(4321)), SpawnRecovery::Adopt);
+    assert_eq!(plan_spawn_recovery(true, None), SpawnRecovery::Adopt);
+}

crates/fbuild-cli/src/daemon_client.rs

@@ -1255,6 +1255,42 @@ async fn start_daemon(client: &DaemonClient) -> fbuild_core::Result<Option<u32>>
         );
     }
 
+    // FastLED/fbuild#1627: three failed attempts are not the end when the
+    // reason is a leftover daemon from an earlier step that still owns the
+    // cache root (`root-owner.lock busy`) or a wedged one that owns the
+    // endpoint (#1360). #1732 explained those failures and #1734 widened the
+    // readiness budget; this retires the verified dead weight and makes
+    // exactly ONE final attempt, so a CI runner self-heals instead of failing
+    // a release gate that then needs a manual rerun. The retirement is
+    // fail-closed on process identity and never touches a daemon that
+    // answers, so a parallel build on the same machine is safe.
+    let mut leftover_note = String::new();
+    if let Some(retired_pid) =
+        crate::cli::daemon_cmd::retire_unresponsive_for_recovery(client).await
+    {
+        leftover_note = format!(
+            "\n\nA leftover fbuild-daemon (PID {retired_pid}) was still alive while nothing \
+             answered at this endpoint, so it was terminated and one final spawn attempt was \
+             made (FastLED/fbuild#1627)."
+        );
+        match spawn_daemon_process().await {
+            Ok(pid) => {
+                if client
+                    .wait_until_healthy(std::time::Duration::from_secs(
+                        startup_log::readiness_timeout_secs(),
+                    ))
+                    .await?
+                {
+                    tracing::info!(
+                        "daemon started (pid {pid}) after retiring unresponsive PID {retired_pid}"
+                    );
+                    return Ok(Some(pid));
+                }
+            }
+            Err(e) => tracing::warn!("final daemon spawn attempt after retirement failed: {e}"),
+        }
+    }
+
     let readiness_secs = startup_log::readiness_timeout_secs();
     Err(fbuild_core::FbuildError::DaemonError(format!(
         "daemon did not become healthy after 3 spawn attempts \
@@ -1265,9 +1301,10 @@ async fn start_daemon(client: &DaemonClient) -> fbuild_core::Result<Option<u32>>
          version (FastLED/fbuild#1009), so a version-mismatched daemon listens on a different \
          port than this CLI polls and can never be reached. When running a locally-built CLI, \
          build the daemon too (`soldr cargo build -p fbuild-daemon`) so a sibling binary exists \
-         instead of resolving `fbuild-daemon` from PATH.{}",
+         instead of resolving `fbuild-daemon` from PATH.{}{}",
         runtime_diagnostic(),
         wedged_daemon_hint(),
+        leftover_note,
         startup_log::startup_problems_note(&spawn_started)
     )))
 }