fix(daemon): version+identity-keyed endpoint + version arbitration (#1009)

CLAUDE.md

@@ -130,7 +130,7 @@ The current inventory is auto-published to a stable tracking issue every Monday
 ## Key Constraints
 
 - **No file-based locks** — all locking through daemon's in-memory managers
-- **Dev mode isolation** — `FBUILD_DEV_MODE=1` → port 8865, `~/.fbuild/dev/`
+- **Dev mode isolation** — `FBUILD_DEV_MODE=1` → `~/.fbuild/dev/`. The daemon endpoint is no longer a fixed port: it's derived per (backend version + cache identity) in the IANA dynamic range 49152–65535 (`fbuild_paths::default_daemon_port` / `daemon_endpoint_key`), so different-version checkouts get isolated daemons and can't serve each other wrong-version builds (FastLED/fbuild#1009). Override with `FBUILD_DAEMON_PORT`.
 - **HTTP API compatibility** — same endpoints and JSON schemas as the Python daemon
 - **Windows USB-CDC** — 30 retries, aggressive buffer drain, DTR/RTS toggling after flash
 - **Emulator CLI convention** — prefer `fbuild test-emu` for CI; `fbuild deploy --to emu [--emulator <kind>]` for interactive use; keep `--target` and `--qemu` only as compatibility aliases

crates/CLAUDE.md

@@ -59,7 +59,7 @@ fbuild-test-support (test utilities) ──────────────
 
 - **fbuild-core** — `FbuildError`/`Result`, `BuildProfile`, `Platform`, `SizeInfo`, `DaemonState`. Also ships the `dump_usb_ids` example (`examples/dump_usb_ids.rs`) — **tier-1 source** for the nightly `online-data` USB-VID merge (FastLED/fbuild#720). Do not delete; the `usb-ids` workspace dep exists *only* for that example, so removing it drops the aggregator from 4 → 3 independent sources.
 - **fbuild-config** — `PlatformIOConfig` (INI parser with `extends` inheritance), `BoardConfig`, `McuSpec`
-- **fbuild-paths** — Dev/prod path isolation (`~/.fbuild/{dev|prod}/`), port mapping (8765/8865), cache dirs
+- **fbuild-paths** — Dev/prod path isolation (`~/.fbuild/{dev|prod}/`), version+identity-keyed daemon endpoint (`daemon_endpoint_key`/`default_daemon_port`, dynamic range 49152–65535; FastLED/fbuild#1009), cache dirs
 - **fbuild-packages** — URL-based package downloads, toolchain resolution, library manager, parallel pipeline
 - **fbuild-serial** — `SharedSerialManager` (centralized serial I/O), deploy preemption protocol, WebSocket messages, USB-CDC retry logic
 - **fbuild-build** — `BuildOrchestrator` trait, per-platform orchestrators (AVR, ESP32, ESP8266, RP2040, STM32, Teensy, WASM)

crates/fbuild-cli/src/daemon_client.rs

@@ -686,6 +686,38 @@ fn compute_daemon_binary_mtime() -> f64 {
     0.0
 }
 
+/// Decide whether the CLI should restart the running daemon it just probed.
+///
+/// FastLED/fbuild#1009 — arbitrate by version, not raw binary mtime:
+/// - CLI **newer** than the daemon → restart (legitimate upgrade).
+/// - CLI **older** than the daemon → **never** restart (an older CLI must not
+///   evict a newer daemon just because its freshly-built binary has a newer
+///   mtime — the exact bug #940/#1006 left open).
+/// - **Same** version → restart only if the CLI's sibling daemon binary is
+///   newer on disk than the running one (the dev rebuild-then-restart flow).
+///
+/// If either version string doesn't parse as semver, fall back to the legacy
+/// mtime heuristic.
+fn should_restart_daemon(
+    cli_version: &str,
+    daemon_version: &str,
+    cli_mtime: f64,
+    daemon_mtime: f64,
+) -> bool {
+    let newer_on_disk = cli_mtime > 0.0 && daemon_mtime > 0.0 && cli_mtime > daemon_mtime;
+    match (
+        semver::Version::parse(cli_version),
+        semver::Version::parse(daemon_version),
+    ) {
+        (Ok(cli), Ok(daemon)) => match cli.cmp(&daemon) {
+            std::cmp::Ordering::Greater => true,        // upgrade
+            std::cmp::Ordering::Less => false,          // never evict a newer daemon
+            std::cmp::Ordering::Equal => newer_on_disk, // dev rebuild
+        },
+        _ => newer_on_disk, // unparseable → legacy mtime behaviour
+    }
+}
+
 /// Ensure the daemon is running. Spawn it if not.
 /// If the daemon binary has been updated since the running daemon started,
 /// gracefully restart it (stale source detection, matching Python behavior).
@@ -784,29 +816,35 @@ async fn ensure_direct_daemon_running() -> fbuild_core::Result<()> {
 
     // Check if already running
     if client.health().await {
-        // Check if daemon binary is stale (updated since daemon started)
         if let Some(health) = client.health_full().await {
-            if health.source_mtime > 0.0 {
-                let current_mtime = compute_daemon_binary_mtime();
-                if current_mtime > 0.0 && current_mtime > health.source_mtime {
-                    tracing::info!(
-                        "daemon binary is stale (daemon={}, current={}), restarting...",
-                        health.source_mtime,
-                        current_mtime
-                    );
-                    eprintln!("daemon binary updated, restarting...");
-                    let _ = client.shutdown().await;
-                    // Wait for it to stop
-                    for _ in 0..50 {
-                        tokio::time::sleep(std::time::Duration::from_millis(100)).await;
-                        if !client.health().await {
-                            break;
-                        }
+            // FastLED/fbuild#1009: arbitrate by VERSION first, mtime second.
+            // Never let an older-version CLI evict a newer daemon (the old
+            // mtime-only check could, since a freshly-built older binary has a
+            // newer mtime). Restart only on an upgrade, or a same-version dev
+            // rebuild.
+            if should_restart_daemon(
+                env!("CARGO_PKG_VERSION"),
+                &health.version,
+                compute_daemon_binary_mtime(),
+                health.source_mtime,
+            ) {
+                tracing::info!(
+                    "daemon needs restart (daemon v{} mtime={}, cli v{} mtime={})",
+                    health.version,
+                    health.source_mtime,
+                    env!("CARGO_PKG_VERSION"),
+                    compute_daemon_binary_mtime(),
+                );
+                eprintln!("daemon binary updated, restarting...");
+                let _ = client.shutdown().await;
+                // Wait for it to stop
+                for _ in 0..50 {
+                    tokio::time::sleep(std::time::Duration::from_millis(100)).await;
+                    if !client.health().await {
+                        break;
                     }
-                    // Fall through to spawn a fresh daemon below
-                } else {
-                    return Ok(());
                 }
+                // Fall through to spawn a fresh daemon below
             } else {
                 return Ok(());
             }

crates/fbuild-cli/src/daemon_client/tests.rs

@@ -2,7 +2,8 @@
 //! parent file under the 1000-LOC gate (see ci.yml LOC Gate workflow).
 
 use super::{
-    broker_refusal_is_fatal, daemon_cache_identity_error, DaemonAcquisition, DaemonInfoResponse,
+    broker_refusal_is_fatal, daemon_cache_identity_error, should_restart_daemon, DaemonAcquisition,
+    DaemonInfoResponse,
 };
 use running_process::broker::client::RefusalKind::{VersionBlocked, VersionUnsupported};
 
@@ -97,3 +98,36 @@ fn daemon_cache_identity_rejects_wrong_schema() {
     let err = daemon_cache_identity_error(&info).expect("schema mismatch must fail closed");
     assert!(err.contains("cache schema"));
 }
+
+// FastLED/fbuild#1009 — version-based daemon arbitration.
+
+#[test]
+fn older_cli_never_evicts_newer_daemon_regardless_of_mtime() {
+    // The bug: a freshly-built OLDER binary (newer mtime) displacing a running
+    // NEWER daemon. Must not restart even though cli_mtime > daemon_mtime.
+    assert!(!should_restart_daemon("2.4.0", "2.5.0", 9999.0, 1.0));
+    assert!(!should_restart_daemon("2.4.0", "2.4.1", 9999.0, 1.0));
+}
+
+#[test]
+fn newer_cli_upgrades_the_daemon() {
+    // CLI strictly newer → restart regardless of mtime.
+    assert!(should_restart_daemon("2.5.0", "2.4.0", 1.0, 9999.0));
+    assert!(should_restart_daemon("2.4.1", "2.4.0", 0.0, 0.0));
+}
+
+#[test]
+fn same_version_restarts_only_on_newer_binary_mtime() {
+    // Dev rebuild of the same version: restart iff the on-disk binary is newer.
+    assert!(should_restart_daemon("2.4.0", "2.4.0", 200.0, 100.0));
+    assert!(!should_restart_daemon("2.4.0", "2.4.0", 100.0, 200.0));
+    assert!(!should_restart_daemon("2.4.0", "2.4.0", 100.0, 100.0));
+    // No usable mtimes → don't churn.
+    assert!(!should_restart_daemon("2.4.0", "2.4.0", 0.0, 0.0));
+}
+
+#[test]
+fn unparseable_versions_fall_back_to_mtime() {
+    assert!(should_restart_daemon("not-semver", "2.4.0", 200.0, 100.0));
+    assert!(!should_restart_daemon("2.4.0", "garbage", 100.0, 200.0));
+}

crates/fbuild-paths/src/lib.rs

@@ -43,9 +43,63 @@ pub fn get_daemon_pid_file() -> PathBuf {
     get_daemon_dir().join("fbuild_daemon.pid")
 }
 
+/// Short, stable hex key identifying this daemon *endpoint*: a hash of the
+/// backend version + the cache identity (mode + trust + cache-root + schema).
+///
+/// FastLED/fbuild#1009: the default endpoint used to be a fixed per-user port
+/// (8765/8865) shared by every checkout, so a daemon of a *different version*
+/// could silently serve another checkout's builds. Keying the endpoint on
+/// version+identity means daemons of different versions land on distinct ports
+/// and distinct port files — they can no longer serve each other. Two checkouts
+/// of the SAME version sharing the SAME cache still (correctly) share a daemon,
+/// which is not a wrong-version hazard.
+///
+/// `fbuild-paths` is workspace-versioned, so `env!("CARGO_PKG_VERSION")` here is
+/// identical to the value compiled into the CLI and the daemon — both sides
+/// derive the same key without any handshake.
+pub fn daemon_endpoint_key() -> String {
+    let identity = crate::running_process::DaemonCacheIdentity::discover();
+    let material = format!("{}|{}", env!("CARGO_PKG_VERSION"), identity.label_value());
+    endpoint_key_from_material(&material)
+}
+
+/// FNV-1a (64-bit) of `material`, formatted as 16 lowercase hex chars.
+/// Deterministic + dependency-free; pure (unit-tested).
+fn endpoint_key_from_material(material: &str) -> String {
+    let mut hash: u64 = 0xcbf2_9ce4_8422_2325;
+    for byte in material.as_bytes() {
+        hash ^= u64::from(*byte);
+        hash = hash.wrapping_mul(0x0000_0100_0000_01b3);
+    }
+    format!("{hash:016x}")
+}
+
+/// Deterministic default daemon port derived from [`daemon_endpoint_key`].
+///
+/// Lands in the IANA dynamic range (49152–65535) so it never collides with a
+/// well-known service, and is stable for a given version+identity. Distinct
+/// versions (and dev vs prod, since mode is part of the identity) get distinct
+/// ports. `FBUILD_DAEMON_PORT` still overrides this (see [`get_daemon_port`]).
+pub fn default_daemon_port() -> u16 {
+    port_from_endpoint_key(&daemon_endpoint_key())
+}
+
+/// Map a hex endpoint key into the IANA dynamic port window (49152–65535).
+/// Pure + deterministic (unit-tested).
+fn port_from_endpoint_key(key: &str) -> u16 {
+    const LOW: u32 = 49152;
+    const SPAN: u32 = 65536 - LOW; // 16384
+    let n = u64::from_str_radix(key, 16).unwrap_or(0);
+    (LOW + (n % u64::from(SPAN)) as u32) as u16
+}
+
 /// Daemon port file path (written by daemon so clients can discover the port).
+///
+/// Keyed by [`daemon_endpoint_key`] (FastLED/fbuild#1009) so daemons of
+/// different versions/identities write distinct files and never read each
+/// other's port.
 pub fn get_daemon_port_file() -> PathBuf {
-    get_daemon_dir().join("daemon.port")
+    get_daemon_dir().join(format!("daemon-{}.port", daemon_endpoint_key()))
 }
 
 /// Daemon log file path.
@@ -221,12 +275,18 @@ fn read_port_from_file(path: &Path) -> Option<u16> {
 ///
 /// Priority:
 /// 1. `FBUILD_DAEMON_PORT` environment variable (if set and valid 1–65535)
-/// 2. Port file in current mode's daemon dir (if exists and valid)
-/// 3. Port file in OTHER mode's daemon dir (cross-mode fallback —
-///    handles dev daemon running but client not in dev mode, or vice versa)
-/// 4. Mode-based default: 8865 (dev) or 8765 (prod)
+/// 2. Port file for this endpoint (if it exists and is valid)
+/// 3. [`default_daemon_port`] — a deterministic per-(version, cache-identity)
+///    port
+///
+/// FastLED/fbuild#1009: the endpoint is keyed by version+identity (via
+/// [`daemon_endpoint_key`]) rather than a fixed per-user port, so a daemon of a
+/// different version can no longer bind the same endpoint and serve another
+/// checkout's builds. The old cross-mode fallback (dev CLI adopting a prod
+/// daemon and vice-versa) was an *anti*-isolation bridge and is intentionally
+/// dropped — dev and prod already have separate roots and now separate ports.
 pub fn get_daemon_port() -> u16 {
-    // Priority 1: env var
+    // Priority 1: env var override.
     if let Ok(port_str) = std::env::var("FBUILD_DAEMON_PORT") {
         if let Ok(port) = port_str.parse::<u16>() {
             if port > 0 {
@@ -235,24 +295,13 @@ pub fn get_daemon_port() -> u16 {
         }
     }
 
-    // Priority 2: port file in current mode's daemon dir
-    let port_file = get_daemon_port_file();
-    if let Some(port) = read_port_from_file(&port_file) {
+    // Priority 2: this endpoint's port file.
+    if let Some(port) = read_port_from_file(&get_daemon_port_file()) {
         return port;
     }
 
-    // Priority 3: cross-mode fallback — check the OTHER mode's port file
-    let other_port_file = get_other_fbuild_root().join("daemon").join("daemon.port");
-    if let Some(port) = read_port_from_file(&other_port_file) {
-        return port;
-    }
-
-    // Priority 4: mode-based default
-    if is_dev_mode() {
-        8865
-    } else {
-        8765
-    }
+    // Priority 3: deterministic per-endpoint default.
+    default_daemon_port()
 }
 
 /// Daemon URL.
@@ -367,13 +416,75 @@ mod tests {
     #[test]
     fn dev_mode_port() {
         // Note: can't set env vars in parallel tests safely, and the
-        // function's own priority chain (env var > current-mode port file >
-        // other-mode port file > mode-default) legitimately returns any
-        // u16 > 0. Assert only the contract the function actually promises.
+        // function's own priority chain (env var > endpoint port file >
+        // per-endpoint default) legitimately returns any u16 > 0. Assert only
+        // the contract the function actually promises.
         let port = get_daemon_port();
         assert!(port > 0);
     }
 
+    #[test]
+    fn endpoint_key_is_deterministic_16_hex() {
+        // FastLED/fbuild#1009: the key must be deterministic per (version,
+        // identity) so the CLI and daemon derive the same endpoint. Tested on
+        // the pure hasher so it's immune to parallel env-var mutation.
+        let a = endpoint_key_from_material("2.4.0|mode=prod;trust=local;schema=1;cache=/x");
+        let b = endpoint_key_from_material("2.4.0|mode=prod;trust=local;schema=1;cache=/x");
+        assert_eq!(a, b, "same material must hash identically");
+        assert_eq!(a.len(), 16, "expected 16 hex chars, got {a:?}");
+        assert!(a.chars().all(|c| c.is_ascii_hexdigit()));
+        // Different version → different key (this is the #1009 isolation).
+        let other_version =
+            endpoint_key_from_material("2.5.0|mode=prod;trust=local;schema=1;cache=/x");
+        assert_ne!(a, other_version, "different version must key differently");
+        // Live key is well-formed too.
+        let live = daemon_endpoint_key();
+        assert_eq!(live.len(), 16);
+        assert!(live.chars().all(|c| c.is_ascii_hexdigit()));
+    }
+
+    #[test]
+    fn default_daemon_port_is_in_dynamic_range() {
+        let p = default_daemon_port();
+        assert!(
+            (49152..=65535).contains(&p),
+            "port {p} outside dynamic range"
+        );
+    }
+
+    #[test]
+    fn port_from_key_is_deterministic_and_ranged() {
+        // Same key → same port; keys differing (e.g. by version or checkout)
+        // map into the dynamic range and generally differ.
+        assert_eq!(
+            port_from_endpoint_key("0123456789abcdef"),
+            port_from_endpoint_key("0123456789abcdef")
+        );
+        for key in ["0000000000000000", "ffffffffffffffff", "deadbeefcafef00d"] {
+            let p = port_from_endpoint_key(key);
+            assert!((49152..=65535).contains(&p), "key {key} → {p} out of range");
+        }
+        // Two distinct version/identity keys should not collapse to one port
+        // for these representative values.
+        assert_ne!(
+            port_from_endpoint_key("1111111111111111"),
+            port_from_endpoint_key("2222222222222222")
+        );
+    }
+
+    #[test]
+    fn port_file_name_is_endpoint_keyed() {
+        // The port file must carry the endpoint key so different versions /
+        // identities never read each other's port. FastLED/fbuild#1009.
+        let file = get_daemon_port_file();
+        let name = file.file_name().unwrap().to_string_lossy();
+        assert!(
+            name.starts_with("daemon-") && name.ends_with(".port"),
+            "unexpected port file name: {name}"
+        );
+        assert!(name.contains(&daemon_endpoint_key()));
+    }
+
     #[test]
     fn other_fbuild_root_is_opposite_mode() {
         // get_other_fbuild_root should return the opposite mode's root