From 6f5cc66625f2fd2f9641ac84db42d526948f62d8 Mon Sep 17 00:00:00 2001
From: Marc-Merino <35137847+Marc-Merino@users.noreply.github.com>
Date: Mon, 12 Oct 2026 16:33:36 -0500
Subject: [PATCH] Never repeat an object id within one process

Commit group, revert group, revert plan and ledger node ids took their six hex
characters from the low bits of the clock. macOS clocks tick in microseconds,
so ids requested in the same tick came out equal: parallel PR reverts then
cloned into the same .knit/revert-clones directory and all but one failed with
"destination path already exists".

Each process now starts from a random suffix and counts up, so its ids never
repeat while keeping the same <prefix>_<date>_<6 hex> shape.

Knit-Group: kg_20261012_59fdd7
Knit-Bundle: give-every-revert-clone-a-unique-directory
---
 src/ids.rs | 57 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 50 insertions(+), 7 deletions(-)

diff --git a/src/ids.rs b/src/ids.rs
index 17aa0329..05c74a5d 100644
--- a/src/ids.rs
+++ b/src/ids.rs
@@ -1,5 +1,9 @@
 use crate::model::ChangeGroup;
 use chrono::Utc;
+use std::collections::hash_map::RandomState;
+use std::hash::{BuildHasher, Hasher};
+use std::sync::atomic::{AtomicU64, Ordering};
+use std::sync::OnceLock;
 use std::time::{SystemTime, UNIX_EPOCH};
 
 /// Expand CLI repo selector flags. Each value may be one id or a comma-separated
@@ -74,17 +78,56 @@ pub fn node_id(prefix: &str) -> String {
     object_id(prefix)
 }
 
+/// Ids from one process start at a random suffix and count up from it, so two
+/// ids requested in the same clock tick (parallel revert clones, for example)
+/// never repeat.
 fn object_id(prefix: &str) -> String {
+    static START: OnceLock<u64> = OnceLock::new();
+    static NEXT: AtomicU64 = AtomicU64::new(0);
+    let start = *START.get_or_init(|| {
+        let nanos = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .map(|duration| duration.as_nanos())
+            .unwrap_or_default();
+        let mut hasher = RandomState::new().build_hasher();
+        hasher.write_u128(nanos);
+        hasher.write_u32(std::process::id());
+        hasher.finish()
+    });
     let date = Utc::now().format("%Y%m%d");
-    let nanos = SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .map(|duration| duration.as_nanos())
-        .unwrap_or_default();
-    let mixed = nanos ^ u128::from(std::process::id());
-    let suffix = format!("{:06x}", mixed & 0xFF_FFFF);
-    format!("{prefix}_{date}_{suffix}")
+    let suffix = start.wrapping_add(NEXT.fetch_add(1, Ordering::Relaxed)) & 0xFF_FFFF;
+    format!("{prefix}_{date}_{suffix:06x}")
 }
 
 pub fn short_sha(sha: &str) -> String {
     sha.trim().chars().take(7).collect()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashSet;
+
+    #[test]
+    fn ids_requested_at_once_from_many_threads_never_repeat() {
+        let ids: Vec<String> = std::thread::scope(|scope| {
+            let workers: Vec<_> = (0..8)
+                .map(|_| scope.spawn(|| (0..2_000).map(|_| revert_group_id()).collect::<Vec<_>>()))
+                .collect();
+            workers
+                .into_iter()
+                .flat_map(|worker| worker.join().unwrap())
+                .collect()
+        });
+        let unique: HashSet<_> = ids.iter().collect();
+        assert_eq!(unique.len(), ids.len());
+        assert!(ids.iter().all(|id| {
+            let parts: Vec<_> = id.split('_').collect();
+            parts.len() == 3
+                && parts[0] == "rv"
+                && parts[1].len() == 8
+                && parts[2].len() == 6
+                && parts[2].chars().all(|c| c.is_ascii_hexdigit())
+        }));
+    }
+}