fix(placement): a young VM's unconsumed memory was handed out twice

The capacity harness scenario, on its first full run, caught what it was
written to catch:

  capacity:   architect peaked at 6 of 6 slot(s)
  FAIL       capacity: 'morpheus' peaked at 3 concurrent VM(s) with only 2 slot(s)
  capacity:   tank peaked at 6 of 6 slot(s)
  PASS       capacity: the over-capacity missions QUEUED
  PASS       capacity: all 16 queued/placed missions completed

`capacity_of` inferred the host's own footprint by subtracting the VMs' FULL
8 GiB claim from observed usage — which assumes they have already consumed it.
A VM booted seconds ago holds about an eighth. On morpheus (31757 MiB total,
4314 MiB idle, 2 slots) with 2 young VMs at ~6314 MiB observed, the inference
6314 - 16384 goes negative, clamps to the 2048 floor, and invents 2266 MiB —
exactly enough for a third VM on a two-slot node.

The footprint is only honestly MEASURABLE when nothing is committed, so
remember it then: `nodes.mem_baseline_mib`, sampled by `survey` whenever it
observes an idle node with fresh health. When VMs are committed, take the
LARGER of the remembered reading and the old inference — a node that was once
idle at 4 GiB and is now running a 20 GiB build must not be scored as idle,
which would be the same over-commit arrived at from the other direction. Both
directions have a test; the second is the one that would otherwise rot.

Raising HOST_BASELINE_FLOOR_MIB would have made this one node's numbers pass
and drifted the moment the fleet changed shape.

Co-Authored-By: Claude Opus 5 <[email protected]>

crates/cm-api/src/vm_placement.rs

@@ -152,6 +152,34 @@ impl PlacementError {
     }
 }
 
+/// What the host itself costs, excluding its phase VMs.
+///
+/// With nothing committed the answer is simply what the node reports. With VMs
+/// committed it cannot be measured, only remembered or inferred — and inference
+/// is where this went wrong: subtracting the VMs' FULL 8 GiB claim from
+/// observed usage assumes they have already consumed it. A VM booted seconds
+/// ago holds about an eighth of that, so the subtraction goes negative, hits
+/// the floor, and hands back memory the host is really using.
+///
+/// Measured on morpheus (31757 MiB total, 4314 MiB idle, 2 slots) with 2 VMs
+/// committed and young: the inferred baseline collapsed to the 2048 floor,
+/// freeing 2266 MiB — exactly enough to admit a 3rd VM to a 2-slot node. The
+/// `capacity` harness scenario caught it on its first full run.
+///
+/// So prefer the remembered idle reading, and take the LARGER of it and the
+/// inference: a host that has genuinely started doing non-VM work must not be
+/// under-charged just because it was once idle at a lower number.
+fn host_baseline(mem_used_mib: i64, committed_vms: i64, baseline_mib: Option<i64>) -> i64 {
+    if committed_vms <= 0 {
+        // Directly observable, and the only moment it is.
+        return mem_used_mib.max(HOST_BASELINE_FLOOR_MIB);
+    }
+    let inferred = mem_used_mib - committed_vms * MEM_PER_VM_MIB as i64;
+    inferred
+        .max(baseline_mib.unwrap_or(0))
+        .max(HOST_BASELINE_FLOOR_MIB)
+}
+
 /// The whole capacity decision for one node, as pure arithmetic.
 ///
 /// Separated from every I/O concern so the numbers can be tested against measured
@@ -163,13 +191,12 @@ pub fn capacity_of(
     mem_used_mib: i64,
     disk_free_gib: i64,
     committed_vms: i64,
+    // What this node used the last time it was seen with nothing committed.
+    // `None` before it has ever been observed idle.
+    baseline_mib: Option<i64>,
     headroom: f64,
 ) -> Result<NodeCapacity, Unfit> {
-    // What the host itself costs, inferred by subtracting what the VMs claimed.
+    let host_baseline = host_baseline(mem_used_mib, committed_vms, baseline_mib);
-    // Floored because a node reporting less used than its VMs claim would
-    // otherwise produce a negative baseline and invent free memory.
-    let host_baseline =
-        (mem_used_mib - committed_vms * MEM_PER_VM_MIB as i64).max(HOST_BASELINE_FLOOR_MIB);
     let committed_use = committed_vms * MEM_PER_VM_MIB as i64 + host_baseline;
 
     // The worse of the two views. Observed alone under-counts a freshly booted
@@ -230,6 +257,7 @@ pub fn from_eval(
         used / MIB,
         row.disk_free_bytes.unwrap_or(0) / GIB,
         committed_vms,
+        row.mem_baseline_mib,
         row.headroom_fresh(MAX_METRICS_AGE_SECS),
     )
 }
@@ -350,6 +378,22 @@ pub async fn survey(
             .collect();
         let committed = commitments(&live, &keys);
 
+        // An idle node is the ONLY time its own footprint is measurable rather
+        // than inferred, so take the reading whenever we get one. Cheap: an
+        // UPDATE per idle node per survey, and it is what stops a young VM's
+        // unconsumed memory from being handed out a second time.
+        if committed == 0 {
+            if let Some(used) = row.mem_used_bytes.filter(|_| {
+                row.health_age_secs
+                    .is_some_and(|a| a <= MAX_HEALTH_AGE_SECS)
+            }) {
+                let mib = used / (1024 * 1024);
+                if row.mem_baseline_mib != Some(mib) {
+                    let _ = cm_db::repo::nodes::set_mem_baseline(pool, node, mib).await;
+                }
+            }
+        }
+
         match from_eval(row, &name, committed) {
             Ok(c) => fit.push(c),
             Err(why) => unfit.push((node, name, why)),
@@ -446,17 +490,66 @@ mod tests {
     /// Observed-usage-only arithmetic says (61440-12000-4096)/8192 = 5 more VMs.
     /// The node has room for ONE. Booking those five is a node in swap, and every
     /// VM on it slows down together.
+    /// A node whose VMs have not yet consumed their claim must not hand the
+    /// difference out again.
+    ///
+    /// This is the bug the `capacity` harness scenario found on its first full
+    /// run — "morpheus peaked at 3 concurrent VM(s) with only 2 slot(s)" — and
+    /// the numbers here are that node's real ones. Idle it reports 4314 MiB of
+    /// 31757 and the survey correctly gives it 2 slots. Two VMs later, each
+    /// holding roughly 1 GiB of its 8 GiB, observed usage is ~6314 MiB;
+    /// inferring the baseline as 6314 - 16384 goes negative, clamps to the
+    /// 2048 floor, and invents 2266 MiB — exactly one more VM than exists.
+    #[test]
+    fn a_young_vms_unconsumed_memory_is_not_handed_out_twice() {
+        // Idle: the reading that gets remembered, and the slot count it implies.
+        let idle = capacity_of(nid(3), "morpheus", 31757, 4314, 312, 0, None, 90.0)
+            .expect("an idle morpheus fits VMs");
+        assert_eq!(idle.slots, 2, "idle capacity is the number we are defending");
+
+        // Two committed, both young. WITHOUT the remembered baseline this
+        // returned 1 slot and admitted a third VM.
+        let inferred = capacity_of(nid(3), "morpheus", 31757, 6314, 312, 2, None, 90.0);
+        assert!(
+            inferred.is_ok(),
+            "the old inference is preserved as the no-baseline fallback"
+        );
+
+        // WITH it, the node is correctly full.
+        let remembered = capacity_of(nid(3), "morpheus", 31757, 6314, 312, 2, Some(4314), 90.0);
+        assert!(
+            matches!(remembered, Err(Unfit::AtCapacity { committed: 2, .. })),
+            "a 2-slot node with 2 VMs committed is FULL, got {remembered:?}"
+        );
+    }
+
+    /// A host that starts doing real work outside its VMs is charged for it.
+    ///
+    /// The remembered baseline is a floor, not a substitute. If it replaced the
+    /// inference outright, a node that was idle at 4 GiB and is now running a
+    /// 20 GiB build would still be scored as if it were idle — the same
+    /// over-commit, arrived at from the opposite direction.
+    #[test]
+    fn a_remembered_baseline_never_under_charges_a_busy_host() {
+        // 1 VM committed and consumed (8192), plus 20 GiB of non-VM work.
+        let used = 8192 + 20480;
+        let c = capacity_of(nid(3), "busy", 61440, used, 800, 1, Some(4096), 90.0)
+            .expect("still has room");
+        // Inference says 20480; the stale 4096 baseline must not win.
+        assert_eq!(c.used_eff_mib, used, "observed usage is charged in full");
+    }
+
     #[test]
     fn a_sold_out_node_is_not_mistaken_for_an_idle_one() {
         let observed_only =
-            capacity_of(nid(1), "tank", 61440, 12000, 800, 0, 50.0).expect("fits");
+            capacity_of(nid(1), "tank", 61440, 12000, 800, 0, None, 50.0).expect("fits");
         assert_eq!(
             observed_only.slots, 5,
             "this is what utilisation alone claims — the bug being fixed"
         );
 
         let with_commitments =
-            capacity_of(nid(1), "tank", 61440, 12000, 800, 5, 50.0).expect("fits");
+            capacity_of(nid(1), "tank", 61440, 12000, 800, 5, None, 50.0).expect("fits");
         assert_eq!(
             with_commitments.slots, 1,
             "five 8 GiB claims are already spoken for, whatever the guests have touched"
@@ -468,33 +561,33 @@ mod tests {
     #[test]
     fn the_measured_fleet_gets_the_slots_it_actually_has() {
         // tank: 60 GiB, ~6 GiB used at idle.
-        let tank = capacity_of(nid(1), "tank", 61440, 6144, 869, 0, 90.0).unwrap();
+        let tank = capacity_of(nid(1), "tank", 61440, 6144, 869, 0, None, 90.0).unwrap();
         assert_eq!(tank.slots, 6);
         // architect: 60 GiB, ~7 GiB used.
-        let arch = capacity_of(nid(2), "architect", 61440, 7168, 388, 0, 90.0).unwrap();
+        let arch = capacity_of(nid(2), "architect", 61440, 7168, 388, 0, None, 90.0).unwrap();
         assert_eq!(arch.slots, 6);
         // morpheus: 31 GiB — deliberately the conservative 2, not 3. Three VMs
         // would leave under 2 GiB for the host, which is where the OOM killer
         // lives, and an OOM-killed VM looks like an agent that gave up.
-        let morph = capacity_of(nid(3), "morpheus", 31744, 5120, 312, 0, 90.0).unwrap();
+        let morph = capacity_of(nid(3), "morpheus", 31744, 5120, 312, 0, None, 90.0).unwrap();
         assert_eq!(morph.slots, 2);
     }
 
     /// Spread, don't stack; then real load; then determinism.
     #[test]
     fn ranking_prefers_free_slots_then_headroom_then_a_stable_order() {
-        let a = capacity_of(nid(1), "a", 61440, 6144, 800, 0, 40.0).unwrap(); // 6 slots
+        let a = capacity_of(nid(1), "a", 61440, 6144, 800, 0, None, 40.0).unwrap(); // 6 slots
-        let b = capacity_of(nid(2), "b", 61440, 6144, 800, 3, 90.0).unwrap(); // 3 slots
+        let b = capacity_of(nid(2), "b", 61440, 6144, 800, 3, None, 90.0).unwrap(); // 3 slots
         assert_eq!(rank(vec![b.clone(), a.clone()])[0].name, "a", "more slots wins");
 
         // Equal slots → the node under less real load.
-        let busy = capacity_of(nid(3), "busy", 61440, 6144, 800, 0, 10.0).unwrap();
+        let busy = capacity_of(nid(3), "busy", 61440, 6144, 800, 0, None, 10.0).unwrap();
-        let idle = capacity_of(nid(4), "idle", 61440, 6144, 800, 0, 95.0).unwrap();
+        let idle = capacity_of(nid(4), "idle", 61440, 6144, 800, 0, None, 95.0).unwrap();
         assert_eq!(rank(vec![busy.clone(), idle.clone()])[0].name, "idle");
 
         // Equal on both → same answer twice. `last_seen DESC` could not promise this.
-        let x = capacity_of(nid(9), "x", 61440, 6144, 800, 0, 50.0).unwrap();
+        let x = capacity_of(nid(9), "x", 61440, 6144, 800, 0, None, 50.0).unwrap();
-        let y = capacity_of(nid(8), "y", 61440, 6144, 800, 0, 50.0).unwrap();
+        let y = capacity_of(nid(8), "y", 61440, 6144, 800, 0, None, 50.0).unwrap();
         assert_eq!(rank(vec![x.clone(), y.clone()])[0].name, "y");
         assert_eq!(rank(vec![y, x])[0].name, "y");
     }
@@ -518,7 +611,7 @@ mod tests {
     /// names the real problem.
     #[test]
     fn a_node_short_of_disk_is_refused_even_with_memory_to_spare() {
-        let e = capacity_of(nid(1), "tank", 61440, 6144, 25, 0, 90.0).unwrap_err();
+        let e = capacity_of(nid(1), "tank", 61440, 6144, 25, 0, None, 90.0).unwrap_err();
         assert!(matches!(e, Unfit::NoDisk { free_gib: 25 }), "{e:?}");
         assert!(e.reason().contains("25 GiB"));
     }
@@ -577,6 +670,7 @@ mod tests {
             mem_total_bytes: Some(total),
             mem_used_bytes: Some(used),
             disk_free_bytes: Some(disk_free),
+            mem_baseline_mib: None,
             health_age_secs: Some(3.0),
             metrics_age_secs: Some(3.0),
         }

crates/cm-db/src/repo/node_metrics.rs

@@ -78,6 +78,10 @@ pub struct EvalRow {
     pub mem_total_bytes: Option<i64>,
     pub mem_used_bytes: Option<i64>,
     pub disk_free_bytes: Option<i64>,
+    /// Memory in use with no phase VMs committed, remembered from the last time
+    /// this node was observed idle. `None` until then, which makes placement
+    /// fall back to inferring it — the behaviour that over-committed morpheus.
+    pub mem_baseline_mib: Option<i64>,
     /// Age of each source. Placement is fail-closed on stale health (a node whose
     /// RAM we cannot read is one we are guessing at), and demotes rather than
     /// excludes on stale Beszel metrics, which only ever break ties.
@@ -133,6 +137,7 @@ pub async fn eval_all(pool: &PgPool) -> Result<Vec<EvalRow>, DbError> {
             h.mem_total AS mem_total_bytes,
             h.mem_used  AS mem_used_bytes,
             h.disk_free AS disk_free_bytes,
+            n.mem_baseline_mib,
             EXTRACT(EPOCH FROM now() - h.captured_at)::float8 AS health_age_secs,
             EXTRACT(EPOCH FROM now() - m.updated_at)::float8  AS metrics_age_secs
          FROM nodes n
@@ -156,6 +161,7 @@ pub async fn eval_all(pool: &PgPool) -> Result<Vec<EvalRow>, DbError> {
             mem_total_bytes: r.get("mem_total_bytes"),
             mem_used_bytes: r.get("mem_used_bytes"),
             disk_free_bytes: r.get("disk_free_bytes"),
+            mem_baseline_mib: r.get("mem_baseline_mib"),
             health_age_secs: r.get("health_age_secs"),
             metrics_age_secs: r.get("metrics_age_secs"),
         })

crates/cm-db/src/repo/nodes.rs

@@ -409,3 +409,17 @@ mod tests {
         assert_eq!(backend_key(Some("agent-terminal")), "agent-terminal");
     }
 }
+
+/// Remember a node's idle memory footprint.
+///
+/// Only ever called with a reading taken while the node had ZERO phase VMs
+/// committed — that is the one moment the number is honestly observable.
+/// Writing it at any other time would record the VMs as part of the host.
+pub async fn set_mem_baseline(pool: &PgPool, node_id: NodeId, mib: i64) -> Result<(), DbError> {
+    sqlx::query("UPDATE nodes SET mem_baseline_mib = $2 WHERE id = $1")
+        .bind(node_id.as_uuid())
+        .bind(mib)
+        .execute(pool)
+        .await?;
+    Ok(())
+}

migrations/0074_node_mem_baseline.sql

@@ -0,0 +1,20 @@
+-- What a node's memory looks like with NO phase VMs on it.
+--
+-- `vm_placement::capacity_of` used to infer the host's own footprint by
+-- subtracting the VMs' full claim from observed usage. That is right only when
+-- the VMs have actually consumed what they claimed. A VM booted seconds ago
+-- holds ~1 GiB of its 8 GiB, so the subtraction goes negative, hits the 2 GiB
+-- floor, and hands back memory the host is really using.
+--
+-- Measured on morpheus (31757 MiB total, 4314 MiB idle) with 2 VMs committed:
+-- the inferred baseline collapsed from 4314 to 2048, which freed 2266 MiB —
+-- just enough to fit a 3rd VM into a 2-slot node. The capacity harness caught
+-- it as "morpheus peaked at 3 concurrent VM(s) with only 2 slot(s)".
+--
+-- So remember the baseline instead of re-deriving it: it is only honestly
+-- observable when the node has nothing committed, and that is exactly when it
+-- is recorded.
+ALTER TABLE nodes ADD COLUMN IF NOT EXISTS mem_baseline_mib BIGINT;
+
+COMMENT ON COLUMN nodes.mem_baseline_mib IS
+  'Memory in use with zero phase VMs committed, sampled by vm_placement::survey. NULL until first observed idle.';