From 2454a1ea6a009d71d58a6af668fd6d6d3e9ddfc1 Mon Sep 17 00:00:00 2001
From: atlas <atlas@pr1ma.darkest.space>
Date: Sun, 2 Aug 2026 16:30:13 +0200
Subject: [PATCH] jobq: drop JobBuilder's Default impl so it is really
 unconstructible outside
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The builder's module doc claimed a builder "cannot be constructed, held
or inserted from outside this crate". Two of those three were false:
`new()` is `pub(crate)`, but a hand-written `impl Default for
JobBuilder` is a trait impl on a `pub` type, so it is public regardless
— `JobBuilder::default()` compiled downstream.

Nothing was unsound (`insert_with` stayed `pub(crate)`, so an
outside-built builder could not reach a graph), but the sentence claimed
more than the visibility enforced, which is the bug this crate's docs
have hit before.

Delete the impl; `new()` constructs directly. The doc now says only what
is enforced, and records why there is no `Default` — so the next person
reaching for one finds the reason instead of adding it back.
---
 hive-jobq/src/builder.rs | 27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/hive-jobq/src/builder.rs b/hive-jobq/src/builder.rs
index fa1e9807..a2e207e9 100644
--- a/hive-jobq/src/builder.rs
+++ b/hive-jobq/src/builder.rs
@@ -8,8 +8,10 @@
 //! **An insertion API, not a spec factory.** A builder is only ever handed to a
 //! closure by the single insertion entry point
 //! ([`crate::scheduler::Scheduler::insert_job`]), which inserts the declared
-//! nodes and returns the ids the job asked for. It cannot be constructed, held
-//! or inserted from outside this crate, and there is no intermediate
+//! nodes and returns the ids the job asked for. It cannot be constructed or
+//! inserted from outside this crate — `new()` and `insert_with` are both
+//! `pub(crate)`, and there is deliberately no `Default` impl, since a trait impl
+//! on a `pub` type is public regardless. There is no intermediate
 //! node-description type to keep in sync with [`crate::Graph::insert`]'s signature —
 //! so a job has no representation that can be passed around instead of being
 //! inserted.
@@ -242,16 +244,6 @@ pub struct JobBuilder<N, R> {
     nodes: RefCell<Vec<Pending<N, R>>>,
 }
 
-// Hand-written rather than derived: `#[derive(Default)]` would demand
-// `N: Default, R: Default`, which has nothing to do with an empty builder.
-impl<N, R> Default for JobBuilder<N, R> {
-    fn default() -> Self {
-        Self {
-            nodes: RefCell::new(Vec::new()),
-        }
-    }
-}
-
 impl<N, R> JobBuilder<N, R> {
     /// A fresh, empty builder.
     ///
@@ -261,8 +253,17 @@ impl<N, R> JobBuilder<N, R> {
     /// nodes and returns the ids. Nothing job-shaped is constructible or
     /// carryable outside this crate — otherwise it is a spec factory again,
     /// just with a builder's name on it.
+    ///
+    /// Deliberately **not** a `Default` impl. A trait impl on a `pub` type is
+    /// public no matter how private its inherent constructors are, so
+    /// `JobBuilder::default()` would hand every downstream crate the builder
+    /// this fn is `pub(crate)` to withhold. The body is what `#[derive(Default)]`
+    /// could not be anyway — deriving would demand `N: Default, R: Default`,
+    /// which has nothing to do with an empty builder.
     pub(crate) fn new() -> Self {
-        Self::default()
+        Self {
+            nodes: RefCell::new(Vec::new()),
+        }
     }
 
     /// Whether nothing has been declared yet — for a caller deciding whether an