From c82853af5a1218197829aa87ec9581508eeb3bda Mon Sep 17 00:00:00 2001 From: atlas Date: Sun, 2 Aug 2026 13:53:51 +0200 Subject: [PATCH] jobq: make NodeGuid an actual guid Third time the operator asked for a guid and got a substitute: first an i64 index, then a {random job id, per-builder counter} pair. The pair was defensible in isolation -- a foreign handle misses rather than colliding, with no new dependency -- but "an equivalent that avoids a dep" is a counter-proposal, not an implementation. It is also simpler as a guid, which was the question asked: NodeGuid(Uuid) drops the `job` field, the `next_seq` counter, `fresh_job_id()` and the `Cell` import, and halves the type's doc. One random draw per node rather than one per builder -- noise next to what a node does when it runs. uuid 1.24 was already in Cargo.lock as a transitive dependency, so this adds an edge rather than a package. --- Cargo.lock | 1 + Cargo.toml | 1 + hive-jobq/Cargo.toml | 1 + hive-jobq/src/builder.rs | 52 ++++++++++------------------------------ 4 files changed, 15 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6223c7ac..8ac44a4c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1758,6 +1758,7 @@ dependencies = [ "serde", "serde_json", "thiserror 2.0.18", + "uuid", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 9111c620..eb79b132 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,7 @@ hive-sock-client = { path = "hive-sock-client" } hive-types = { path = "hive-types" } thiserror = "2" tower-http = { version = "0.7", features = ["fs"] } +uuid = { version = "1", features = ["v4"] } rmcp = { version = "2", default-features = false, features = [ "server", "macros", diff --git a/hive-jobq/Cargo.toml b/hive-jobq/Cargo.toml index 4fc70e0a..845aa46f 100644 --- a/hive-jobq/Cargo.toml +++ b/hive-jobq/Cargo.toml @@ -12,6 +12,7 @@ chrono = { workspace = true } enumflags2.workspace = true serde = { workspace = true } thiserror = { workspace = true } +uuid = { workspace = true } [dev-dependencies] serde_json = { workspace = true } diff --git a/hive-jobq/src/builder.rs b/hive-jobq/src/builder.rs index 8ae84cb0..c95f1673 100644 --- a/hive-jobq/src/builder.rs +++ b/hive-jobq/src/builder.rs @@ -26,7 +26,7 @@ //! silent reorder — a builder that sorted for you would quietly accept a shape //! the graph itself cannot express. -use std::cell::{Cell, RefCell}; +use std::cell::RefCell; use std::collections::HashMap; use crate::{Dep, DepWhen, Graph, GraphError, NodeId, TerminalState}; @@ -38,32 +38,14 @@ use crate::{Dep, DepWhen, Graph, GraphError, NodeId, TerminalState}; /// insert time and are meaningful system-wide, while this is a builder-local /// label that stops existing once the job is inserted. /// -/// The `job` half is what makes it safe to compare. A [`NodeRef`] converts into -/// a bare `NodeGuid`, dropping the borrow that ties it to its builder — so a -/// handle *can* be carried to a different job (captured by an inner closure, -/// say). Were the identity a plain per-builder counter, the two jobs' `0`s would -/// be equal and that handle would silently resolve to an unrelated node. With a -/// per-builder random half it simply isn't found, and the insert fails by name. +/// It is a v4 uuid because it has to survive leaving its builder. A [`NodeRef`] +/// converts into a bare `NodeGuid`, dropping the borrow that tied it to the +/// builder — so a handle *can* be carried into a different job (captured by an +/// inner closure, say). Under a per-builder counter the two jobs' `0`s would be +/// equal and that handle would silently address an unrelated node; drawn at +/// random per node it simply isn't found, and the insert fails by name. #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -pub struct NodeGuid { - /// Identifies the builder that issued this handle. Random per builder, so - /// two jobs' handles never compare equal. - job: u64, - /// Position within that builder — the cheap half, since `job` already - /// separates one builder's handles from another's. - seq: u32, -} - -/// A fresh identity for one builder. -/// -/// `RandomState` is seeded from the process's random state and advances per -/// instance, so each builder gets a distinct, unguessable value — enough to -/// make a foreign handle a miss rather than a collision. No `rand` dependency -/// for what is a collision-avoidance job, not a cryptographic one. -fn fresh_job_id() -> u64 { - use std::hash::BuildHasher as _; - std::collections::hash_map::RandomState::new().hash_one(0_u8) -} +pub struct NodeGuid(uuid::Uuid); /// Why a job could not be inserted. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] @@ -117,10 +99,6 @@ struct Pending { #[derive(Debug)] pub struct JobBuilder { nodes: RefCell>>, - /// Identifies this builder in every handle it issues. - job: u64, - /// Position within this builder — the `seq` half of a [`NodeGuid`]. - next_seq: Cell, } // Hand-written rather than derived: `#[derive(Default)]` would demand @@ -129,8 +107,6 @@ impl Default for JobBuilder { fn default() -> Self { Self { nodes: RefCell::new(Vec::new()), - job: fresh_job_id(), - next_seq: Cell::new(0), } } } @@ -160,11 +136,7 @@ impl JobBuilder { /// The returned handle is where those are declared; it is [`Copy`], so it /// can be named as a dependency as many times as needed. pub fn node(&self, payload: N) -> NodeRef<'_, N, R> { - let guid = NodeGuid { - job: self.job, - seq: self.next_seq.get(), - }; - self.next_seq.set(guid.seq + 1); + let guid = NodeGuid(uuid::Uuid::new_v4()); self.nodes.borrow_mut().push(Pending { guid, payload, @@ -558,9 +530,9 @@ mod tests { /// job — an inner closure capturing an outer handle, say — must not /// silently address whichever node happens to sit at the same position. /// - /// This is what the random half of a [`NodeGuid`] buys: with a plain - /// per-builder counter both jobs' first handles would be equal, and the - /// edge below would resolve, wrongly, to the second job's own node. + /// This is what a random [`NodeGuid`] buys: with a plain per-builder + /// counter both jobs' first handles would be equal, and the edge below + /// would resolve, wrongly, to the second job's own node. #[test] fn a_handle_from_another_job_is_not_silently_resolved() { let mut g = graph();