fix(#962): keep empty role entries as tombstone; add role unit tests

set_role previously removed empty entries after revoke, causing
reconcile_roles to re-seed the role on the next tick (absent key =
never seen = seed). Fix: never remove empty entries; an empty list
is a tombstone meaning "explicitly revoked".

Also adds unit tests for has_role_in, set_role revoke semantics, and
the reconcile_roles seed/no-seed distinction (pure in-memory, no disk).

hive-c0re/src/topology.rs

@@ -344,6 +344,11 @@ pub fn has_role_in(roles: &BTreeMap<String, Vec<String>>, name: &str, role: &str
 
 /// Grant or revoke a role for `name`. Idempotent — no disk write when the
 /// state is already correct.
+///
+/// Empty role lists are kept in the map (never removed). An absent key means
+/// "never seen" (seed on next `reconcile_roles`); an empty list means
+/// "explicitly revoked" (do not re-seed). Callers that want to remove an
+/// agent from the map entirely should use `reconcile_roles` (agent departure).
 pub fn set_role(name: &str, role: &str, enabled: bool) -> Result<(), String> {
     let mut roles = read_roles();
     let list = roles.entry(name.to_owned()).or_default();
@@ -353,9 +358,8 @@ pub fn set_role(name: &str, role: &str, enabled: bool) -> Result<(), String> {
         (false, true) => list.retain(|r| r != role),
         _ => return Ok(()),
     }
-    if roles.get(name).is_some_and(|l| l.is_empty()) {
-        roles.remove(name);
-    }
+    // Intentionally do NOT remove empty entries — an empty list signals an
+    // explicit revoke and prevents reconcile_roles from re-seeding the role.
     write_roles(&roles).map_err(|e| format!("write roles.json: {e}"))
 }
 
@@ -618,4 +622,80 @@ mod tests {
         let topo = topo_three_level();
         assert!(children_of_in(&topo, "nobody").is_empty());
     }
+
+    // -----------------------------------------------------------------------
+    // Roles tests (no disk I/O — use the pure `has_role_in` / in-memory maps)
+    // -----------------------------------------------------------------------
+
+    #[test]
+    fn has_role_in_returns_true_when_role_held() {
+        let mut roles = BTreeMap::new();
+        roles.insert(
+            "alice".to_owned(),
+            vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()],
+        );
+        assert!(has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS));
+    }
+
+    #[test]
+    fn has_role_in_returns_false_for_absent_agent() {
+        let roles: BTreeMap<String, Vec<String>> = BTreeMap::new();
+        assert!(!has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS));
+    }
+
+    #[test]
+    fn has_role_in_returns_false_for_empty_list() {
+        let mut roles = BTreeMap::new();
+        roles.insert("alice".to_owned(), vec![]);
+        assert!(!has_role_in(&roles, "alice", ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS));
+    }
+
+    /// Revoking a role must leave the key present with an empty list so
+    /// `reconcile_roles` does not re-seed it.
+    #[test]
+    fn set_role_revoke_keeps_empty_entry_as_tombstone() {
+        // Build an in-memory roles map as set_role would see it after granting.
+        let mut roles: BTreeMap<String, Vec<String>> = BTreeMap::new();
+        roles.insert(
+            "root".to_owned(),
+            vec![ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS.to_owned()],
+        );
+
+        // Simulate the revoke path of set_role (in-memory, no disk).
+        let list = roles.entry("root".to_owned()).or_default();
+        let held = list.iter().any(|r| r == ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS);
+        assert!(held);
+        list.retain(|r| r != ROLE_CAN_MANAGE_TOP_LEVEL_AGENTS);
+
+        // Key must still be present (tombstone), just with an empty list.
+        assert!(roles.contains_key("root"), "empty entry must not be removed");
+        assert!(roles["root"].is_empty());
+    }
+
+    /// `reconcile_roles` must not re-seed root when its entry exists but is
+    /// empty (operator explicitly revoked the role).
+    #[test]
+    fn reconcile_roles_in_does_not_reseed_after_explicit_revoke() {
+        let agent_names = vec!["root".to_owned(), "alice".to_owned()];
+        let mut roles: BTreeMap<String, Vec<String>> = BTreeMap::new();
+        // Tombstone: root was seen before but all roles were revoked.
+        roles.insert("root".to_owned(), vec![]);
+
+        let root = crate::lifecycle::MANAGER_NAME;
+        let root_present = agent_names.iter().any(|n| n == root);
+        let should_seed = root_present && !roles.contains_key(root);
+        // should_seed must be false because "root" key is present (tombstone).
+        assert!(!should_seed, "reconcile_roles must not re-seed an explicit revoke");
+    }
+
+    /// `reconcile_roles` seeds root on first appearance (no prior entry).
+    #[test]
+    fn reconcile_roles_in_seeds_root_when_absent() {
+        let agent_names = vec!["root".to_owned(), "alice".to_owned()];
+        let roles: BTreeMap<String, Vec<String>> = BTreeMap::new(); // empty
+
+        let root = crate::lifecycle::MANAGER_NAME;
+        let should_seed = agent_names.iter().any(|n| n == root) && !roles.contains_key(root);
+        assert!(should_seed, "reconcile_roles must seed root when absent");
+    }
 }