fix(fleet): let a deliberate re-claim outrank an older signed eviction

crates/allmystuff-protocol/src/app.rs

@@ -391,6 +391,24 @@ pub struct OwnedMember {
     /// label a gossip carries wins).
     #[serde(default)]
     pub label: String,
+    /// Unix seconds at which this owner first added the member, or `None` for a
+    /// record written before this field existed.
+    ///
+    /// Load-bearing for exactly one decision: whether a signed eviction still
+    /// applies. `ensure_fleet_network` prunes a locally listed device the signed
+    /// governance has removed, so one owner's admit loop can't resurrect a
+    /// device another owner evicted. Membership converges last-writer-wins on
+    /// the entry stamp, so an eviction only outranks a claim that came *before*
+    /// it — and without this the prune could not tell the two apart, which made
+    /// a deliberate re-claim of a previously evicted device impossible: the
+    /// prune ran before the admit loop every time, so the superseding grant the
+    /// governance layer would have honoured was never authored.
+    ///
+    /// `None` is treated as older than any eviction, so a legacy record keeps
+    /// the previous (always-prune) behaviour rather than being resurrected by
+    /// an upgrade. Re-claiming such a device stamps it and recovers it.
+    #[serde(default)]
+    pub claimed_at: Option<u64>,
 }
 
 /// A snapshot of an owner's fleet for the front-end: the shared key, the
@@ -1670,10 +1688,13 @@ mod tests {
                 OwnedMember {
                     device: "my-laptop".into(),
                     label: "My laptop".into(),
+                    claimed_at: Some(1_700_000_000),
                 },
                 OwnedMember {
                     device: "spare-nuc".into(),
                     label: "Spare NUC".into(),
+                    // Unstamped: a member whose record predates the field.
+                    claimed_at: None,
                 },
             ],
         };

node/src/mesh.rs

@@ -6408,6 +6408,11 @@ impl Mesh {
                                 members.push(OwnedMember {
                                     device: NodeId::from(pubkey_part(id)),
                                     label,
+                                    // A projection of the signed roster for the
+                                    // GUI, not this owner's local record — the
+                                    // claim stamp lives on the latter and is
+                                    // read only by the eviction prune.
+                                    claimed_at: None,
                                 });
                             }
                         }
@@ -6451,6 +6456,7 @@ impl Mesh {
                 members.push(OwnedMember {
                     device: NodeId::from(canon.as_str()),
                     label: m.label.clone(),
+                    claimed_at: m.claimed_at,
                 });
             }
             member_roles
@@ -6473,6 +6479,7 @@ impl Mesh {
                 members.push(OwnedMember {
                     device: NodeId::from(canon.as_str()),
                     label,
+                    claimed_at: None,
                 });
             }
             // Best-effort role for this device (it isn't in its own roster):
@@ -6506,6 +6513,7 @@ impl Mesh {
                     members.push(OwnedMember {
                         device: NodeId::from(canon.as_str()),
                         label: String::new(),
+                        claimed_at: None,
                     });
                 }
                 member_roles
@@ -7306,15 +7314,36 @@ impl Mesh {
         let signed_evicted = self.signed_evicted(&network).await;
         if !signed_evicted.is_empty() {
             let mut pruned = false;
-            for member in self.ownership.fleet_member_ids() {
-                if signed_evicted.contains(pubkey_part(&member)) {
+            for m in self.ownership.fleet_members() {
+                let member = m.device.to_string();
+                let Some(&evicted_at) = signed_evicted.get(pubkey_part(&member)) else {
+                    continue;
+                };
+                // Compare stamps rather than mere set membership. Membership
+                // converges last-writer-wins, so an eviction only outranks a
+                // claim that came before it. A device this owner deliberately
+                // re-claimed AFTER the eviction is the later intent, and the
+                // admit loop below authors the superseding grant the governance
+                // layer already honours (`re_admitting_an_evicted_member_
+                // supersedes_the_tombstone`). Pruning on set membership alone
+                // dropped it here first, every time, so that grant was never
+                // written and unclaim → re-claim was a one-way door.
+                //
+                // An unstamped (pre-upgrade) record sorts as older, so it keeps
+                // the previous always-prune behaviour; re-claiming stamps it.
+                if m.claimed_at.is_some_and(|at| at > evicted_at) {
                     tracing::info!(
-                        "pruning {} from the local fleet list — the signed governance evicted it",
+                        "keeping {} — re-claimed after the signed eviction; re-admitting it",
                         short_id(&member)
                     );
-                    let _ = self.ownership.kick_member(&member);
-                    pruned = true;
+                    continue;
                 }
+                tracing::info!(
+                    "pruning {} from the local fleet list — the signed governance evicted it",
+                    short_id(&member)
+                );
+                let _ = self.ownership.kick_member(&member);
+                pruned = true;
             }
             if pruned {
                 // Reflect the removal now: the authorised-controller cache and the
@@ -7654,7 +7683,10 @@ impl Mesh {
     /// daemon that doesn't report the field), so a transient read failure or a
     /// version skew never prunes a live member — it just falls back to the old
     /// (re-asserting) behaviour.
-    async fn signed_evicted(self: &Arc<Self>, network: &str) -> std::collections::HashSet<String> {
+    async fn signed_evicted(
+        self: &Arc<Self>,
+        network: &str,
+    ) -> std::collections::HashMap<String, u64> {
         let data = match self
             .client
             .request(&Request::GovernanceState {
@@ -7663,17 +7695,9 @@ impl Mesh {
             .await
         {
             Ok(r) if r.ok => r.data.unwrap_or(Value::Null),
-            _ => return std::collections::HashSet::new(),
+            _ => return std::collections::HashMap::new(),
         };
-        data.get("evicted")
-            .and_then(|v| v.as_array())
-            .map(|arr| {
-                arr.iter()
-                    .filter_map(|v| v.as_str())
-                    .map(|k| pubkey_part(k).to_string())
-                    .collect()
-            })
-            .unwrap_or_default()
+        eviction_stamps(&data)
     }
 
     /// Refresh the authorised-controller cache ([`Mesh::fleet_authorized`])
@@ -12529,6 +12553,71 @@ fn unique_path(dir: &std::path::Path, name: &str) -> std::path::PathBuf {
 /// so a device id in display form (`pubkey-SUFFIX`, what `IdentityShow` and
 /// presence use) and bare form (`pubkey`, what the daemon delivers as a
 /// channel `from`) compare equal.
+/// Date each device the fleet's signed governance has removed: canonical pubkey
+/// → the unix second its removal was authored.
+///
+/// Takes the raw `GovernanceState` reply. `evicted` is the authoritative,
+/// already-verified answer to *whether* a device is removed; `state.member_log`
+/// is read only for **when**, so nothing here re-decides membership or trusts an
+/// unverified entry. The latest entry targeting a removed device IS its removal
+/// — that is what made the verdict come out removed under the member log's
+/// last-writer-wins fold — so the max stamp over that device's entries is the
+/// tombstone's stamp without having to match variant kinds.
+///
+/// A removed device the log can't date is recorded at `u64::MAX`, which outranks
+/// every claim: an undatable eviction keeps the old always-prune behaviour
+/// rather than being quietly waived.
+fn eviction_stamps(data: &Value) -> std::collections::HashMap<String, u64> {
+    let removed: std::collections::HashSet<String> = data
+        .get("evicted")
+        .and_then(|v| v.as_array())
+        .map(|arr| {
+            arr.iter()
+                .filter_map(|v| v.as_str())
+                .map(|k| pubkey_part(k).to_string())
+                .collect()
+        })
+        .unwrap_or_default();
+    if removed.is_empty() {
+        return std::collections::HashMap::new();
+    }
+
+    let mut at: std::collections::HashMap<String, u64> = std::collections::HashMap::new();
+    if let Some(log) = data
+        .get("state")
+        .and_then(|s| s.get("member_log"))
+        .and_then(|v| v.as_array())
+    {
+        for entry in log {
+            let Some(stamp) = entry.get("at").and_then(|v| v.as_u64()) else {
+                continue;
+            };
+            let Some(target) = entry
+                .get("variant")
+                .and_then(|v| v.get("target"))
+                .and_then(|v| v.as_str())
+            else {
+                continue;
+            };
+            let target = pubkey_part(target).to_string();
+            if !removed.contains(&target) {
+                continue;
+            }
+            at.entry(target)
+                .and_modify(|s| *s = (*s).max(stamp))
+                .or_insert(stamp);
+        }
+    }
+
+    removed
+        .into_iter()
+        .map(|k| {
+            let stamp = at.get(&k).copied().unwrap_or(u64::MAX);
+            (k, stamp)
+        })
+        .collect()
+}
+
 fn pubkey_part(id: &str) -> &str {
     if let Some((body, suffix)) = id.rsplit_once('-') {
         if suffix.len() == 5 && suffix.chars().all(|c| c.is_ascii_alphanumeric()) {
@@ -12830,6 +12919,72 @@ fn parse_media(s: &str) -> MediaKind {
 
 #[cfg(test)]
 mod tests {
+    // ---- eviction stamps -------------------------------------------------
+    //
+    // The JSON below matches what `GovernanceState` actually emits — verified
+    // against myownmesh-core's `Transition`, which serialises the variant as a
+    // NESTED object ({"at":..,"variant":{"kind":"evict","target":".."},..}), not
+    // flattened. If that ever changes, `eviction_stamps` silently dates nothing,
+    // every eviction sorts as u64::MAX and the prune goes back to being
+    // unconditional — so these pin the shape as much as the logic.
+
+    #[test]
+    fn eviction_stamps_dates_each_removal_from_the_member_log() {
+        let data = serde_json::json!({
+            "evicted": ["kvmkey"],
+            "state": {"member_log": [
+                {"at": 100, "variant": {"kind": "role_grant", "target": "kvmkey", "role": "member"},
+                 "signers": ["owner"], "signatures": ["s"]},
+                {"at": 200, "variant": {"kind": "evict", "target": "kvmkey"},
+                 "signers": ["owner"], "signatures": ["s"]},
+                // A different device's entries must not leak into the stamp.
+                {"at": 900, "variant": {"kind": "evict", "target": "otherkey"},
+                 "signers": ["owner"], "signatures": ["s"]},
+            ]}
+        });
+
+        let stamps = super::eviction_stamps(&data);
+
+        assert_eq!(stamps.len(), 1, "only devices in `evicted` are dated");
+        assert_eq!(stamps.get("kvmkey"), Some(&200));
+    }
+
+    #[test]
+    fn an_undatable_eviction_outranks_every_claim() {
+        // No member log to read (an older daemon, a read that lost the field).
+        // The device is still removed, so it must stay pruned rather than being
+        // waived by a stamp we couldn't find.
+        let data = serde_json::json!({"evicted": ["kvmkey"]});
+
+        let stamps = super::eviction_stamps(&data);
+
+        assert_eq!(stamps.get("kvmkey"), Some(&u64::MAX));
+    }
+
+    #[test]
+    fn no_evictions_dates_nothing() {
+        let data = serde_json::json!({"evicted": [], "state": {"member_log": []}});
+        assert!(super::eviction_stamps(&data).is_empty());
+    }
+
+    #[test]
+    fn eviction_stamps_canonicalises_the_display_suffix() {
+        // The log and the `evicted` list may carry either the bare pubkey or a
+        // display id; the local member list is canonical, so both sides must
+        // collapse or the lookup misses and the prune runs anyway.
+        let data = serde_json::json!({
+            "evicted": ["kvmkey-AB12C"],
+            "state": {"member_log": [
+                {"at": 500, "variant": {"kind": "evict", "target": "kvmkey"},
+                 "signers": ["owner"], "signatures": ["s"]},
+            ]}
+        });
+
+        let stamps = super::eviction_stamps(&data);
+
+        assert_eq!(stamps.get("kvmkey"), Some(&500));
+    }
+
     use super::*;
 
     #[test]

node/src/ownership.rs

@@ -529,6 +529,15 @@ fn upsert_member_into(members: &mut Vec<OwnedMember>, device: &str, label: &str)
             members.push(OwnedMember {
                 device: NodeId::from(canon),
                 label: label.to_string(),
+                // Stamped on first add only. A later upsert is a label refresh,
+                // not a fresh claim, so it must not move this — the stamp is
+                // what tells `ensure_fleet_network` whether a signed eviction
+                // predates the claim, and a gossip-driven label touch would
+                // otherwise keep pushing it past every tombstone.
+                // An unset clock stamps this low, which reads as older than any
+                // eviction — the conservative direction (prune), not the one
+                // that resurrects a device.
+                claimed_at: Some(crate::cec::now_secs()),
             });
             true
         }
@@ -873,6 +882,36 @@ mod tests {
         assert!(!resolved_claimed);
     }
 
+    #[test]
+    fn upsert_stamps_the_first_add_and_a_label_refresh_never_moves_it() {
+        // The stamp dates this owner's CLAIM of the device, and the eviction
+        // prune in `ensure_fleet_network` compares it against the tombstone. A
+        // later upsert is a label refresh driven by roster gossip, not a fresh
+        // claim — if it moved the stamp, a device would drift past every
+        // eviction just by being talked about.
+        let mut members = Vec::new();
+        assert!(upsert_member_into(&mut members, "k1", "Laptop"));
+        let stamped = members[0].claimed_at.expect("first add must be stamped");
+
+        assert!(upsert_member_into(&mut members, "k1", "My laptop"));
+        assert_eq!(
+            members[0].claimed_at,
+            Some(stamped),
+            "a label refresh moved the claim stamp"
+        );
+    }
+
+    #[test]
+    fn a_record_without_a_claim_stamp_still_loads() {
+        // Records written before the field existed must keep loading: this is
+        // the owner's fleet list, and a parse failure would drop real members.
+        // `None` reads as older than any eviction, so such a device keeps the
+        // previous always-prune behaviour rather than being resurrected by an
+        // upgrade.
+        let m: OwnedMember = serde_json::from_str(r#"{"device":"k1","label":"Laptop"}"#).unwrap();
+        assert_eq!(m.claimed_at, None);
+    }
+
     #[test]
     fn upsert_dedups_by_canonical_pubkey_and_refreshes_labels() {
         let mut members = Vec::new();
@@ -922,6 +961,7 @@ mod tests {
         dev.inner.lock().fleet_members.push(OwnedMember {
             device: "old-owner".into(),
             label: "Old".into(),
+            claimed_at: None,
         });
 
         assert!(dev.try_accept_claim("new-owner"));