hive-jobq/src/builder.rs

@@ -29,7 +29,7 @@
 use std::cell::RefCell;
 use std::collections::HashMap;
 
-use crate::{Dep, DepWhen, NodeId, TerminalState};
+use crate::{Dep, DepWhen, GraphError, NodeId, TerminalState};
 
 /// An opaque identity for a node **within the job being built**.
 ///
@@ -108,6 +108,10 @@ pub enum BuildError {
         /// The target of the unsatisfiable edge.
         dep: NodeGuid,
     },
+    /// The graph rejected an otherwise well-formed node — an out-of-group edge,
+    /// an unsatisfiable [`DepWhen`], and so on.
+    #[error(transparent)]
+    Graph(#[from] GraphError),
 }
 
 /// Reject a job whose own declarations don't hold up — **before anything is
@@ -306,15 +310,14 @@ impl<N, R> JobBuilder<N, R> {
     ///
     /// # Errors
     ///
-    /// A [`BuildError`] if the job's own declarations don't hold up — see
-    /// [`check_job_shape`], which decides every one of them **before** the
-    /// first insert. The sink itself is infallible: by the time it runs, the
-    /// job is known-good, so no node can be rejected half-way through.
+    /// [`BuildError::ForwardEdge`] / [`BuildError::ForwardParent`] if a node
+    /// references one declared after it, or [`BuildError::Graph`] if the graph
+    /// rejects a node (see [`crate::Graph::insert`]).
     pub(crate) fn insert_with(
         self,
         root_parent: Option<NodeId>,
         wanted: &[NodeGuid],
-        mut insert: impl FnMut(N, Vec<Dep<R>>, Option<NodeId>) -> NodeId,
+        mut insert: impl FnMut(N, Vec<Dep<R>>, Option<NodeId>) -> Result<NodeId, GraphError>,
     ) -> Result<Vec<NodeId>, BuildError> {
         let pending = self.nodes.into_inner();
         check_job_shape(&pending, wanted, root_parent)?;
@@ -334,10 +337,7 @@ impl<N, R> JobBuilder<N, R> {
                     .into_iter()
                     .map(|(name, count)| Dep::Resource { name, count }),
             );
-            // Infallible by construction: `check_job_shape` above decided every
-            // rejection the graph could raise, so there is nothing left here to
-            // abandon a half-inserted job on.
-            let id = insert(node.payload, deps, parent);
+            let id = insert(node.payload, deps, parent)?;
             ids.insert(node.guid, id);
         }
         Ok(wanted.iter().map(|g| ids[g]).collect())

hive-jobq/src/lib.rs

@@ -419,18 +419,11 @@ impl<N, R> Graph<N, R> {
     /// graph that minted it, so a dangling reference is rejected here rather than
     /// surfacing as a broken edge later.
     ///
-    /// **Crate-private on purpose.** Nodes enter a graph only through
-    /// [`crate::builder::JobBuilder`] (via
-    /// [`crate::scheduler::Scheduler::insert_job`]) or
-    /// [`crate::scheduler::Scheduler::append`]. A second public way in would be
-    /// a second place validation has to agree with the builder's, and the two
-    /// would drift.
-    ///
     /// # Errors
     /// Returns [`GraphError::UnknownDep`] / [`GraphError::UnknownParent`] for a
     /// dangling dependency or parent id, or [`GraphError::DepOutsideParent`] if a
     /// `Dep::Node` edge points outside the node's own parent group.
-    pub(crate) fn insert(
+    pub fn insert(
         &mut self,
         payload: N,
         deps: Vec<Dep<R>>,
@@ -451,25 +444,6 @@ impl<N, R> Graph<N, R> {
                 }
             }
         }
-        Ok(self.insert_unchecked(payload, deps, parent))
-    }
-
-    /// Insert without re-validating — **only** for a node the builder has
-    /// already proved well-formed.
-    ///
-    /// [`crate::builder::check_job_shape`] decides every rejection
-    /// [`Graph::insert`] could raise, before the first node lands. Re-checking
-    /// here would not add safety: the insert loop mutates as it goes, so a
-    /// rejection at node `i` would leave `0..i` in the graph — a loud error
-    /// *after* the corruption rather than instead of it. Making the sink
-    /// infallible is what turns the builder's atomicity from an accident of the
-    /// pre-pass being exhaustive into a property of the types.
-    pub(crate) fn insert_unchecked(
-        &mut self,
-        payload: N,
-        deps: Vec<Dep<R>>,
-        parent: Option<NodeId>,
-    ) -> NodeId {
         let id = self.mint_id();
         self.nodes.push(Node {
             id,
@@ -481,7 +455,7 @@ impl<N, R> Graph<N, R> {
             finished_at: None,
             error: None,
         });
-        id
+        Ok(id)
     }
 
     /// Borrow a node by id.

hive-jobq/src/scheduler.rs

@@ -110,12 +110,10 @@ impl<N, R: Clone + Eq + Hash> Scheduler<N, R> {
     /// insert one itself, so there is no way to end up with a job-shaped value
     /// being passed around as a spec.
     ///
-    /// The one insertion entry point for a job. Nodes go straight into
-    /// [`Graph::insert_unchecked`]: [`crate::builder::check_job_shape`] has
-    /// already decided every rejection the graph could raise, so re-validating
-    /// per node could only report a problem *after* the earlier nodes were
-    /// inserted. Call [`Scheduler::settle`] afterwards to start whatever became
-    /// runnable.
+    /// The one insertion entry point: every node goes through
+    /// [`Scheduler::append`], so a caller never has to reach past the scheduler
+    /// at the graph underneath. Call [`Scheduler::settle`] afterwards to start
+    /// whatever became runnable.
     ///
     /// **Atomic in the job's own shape.** A forward edge, a forward parent, or
     /// a request for a handle this job never declared is rejected *before* the
@@ -132,9 +130,8 @@ impl<N, R: Clone + Eq + Hash> Scheduler<N, R> {
     ) -> Result<Vec<NodeId>, BuildError> {
         let job = JobBuilder::new();
         let wanted = declare(&job);
-        let graph = &mut self.graph;
         job.insert_with(root_parent, &wanted, |payload, deps, parent| {
-            graph.insert_unchecked(payload, deps, parent)
+            self.append(payload, deps, parent)
         })
     }