From ac958842c4a80e13f2479ff7461f1ad00ae74acb Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 14 Aug 2026 22:15:05 +0100 Subject: [PATCH 1/2] Give each dispatch test fixture a root no other can name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `fixture_toml` named its temporary root from the process id and a `SystemTime` nanosecond stamp. Both are shared across the test binary's threads, and the clock is not fine-grained enough to separate two fixtures built in the same instant, so two tests running in parallel could land on the same directory and the same scratch database. The symptom was an intermittent `UNIQUE constraint failed: projects.name`, seen once in the dispatch suite and passing on the retry — the kind of failure that teaches the operator to re-run rather than to read. The name now carries an atomic counter as well, which the test module increments per fixture. The clock still separates reruns of a recycled pid; the counter separates fixtures within one run, whatever the clock resolution. Two tests cover it. The first builds eight fixtures concurrently behind a barrier and asserts distinct roots. Eight is too few to catch a name that leans on the clock alone, so the second names four thousand roots across eight threads with nothing between them: with the counter replaced by a plain load, that test fails with 29 collisions, which is also the mechanism of the original flake. Test code only. `cargo test --workspace` passes five runs over, with clippy and fmt clean. --- crates/voro/src/dispatch.rs | 78 +++++++++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 8 deletions(-) diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index 3d8d45e..f13fd00 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -1918,8 +1918,13 @@ fn default_base_branch(repo_path: &str) -> String { #[cfg(test)] mod tests { use super::*; + use std::sync::atomic::{AtomicU64, Ordering}; use voro_core::{LivenessSource, NewTask, Priority}; + /// Distinguishes fixtures built within the same clock tick, which the + /// nanosecond stamp alone cannot separate. + static FIXTURE_SEQ: AtomicU64 = AtomicU64::new(0); + /// A scratch database, a freshly-`git init`ed clean project, and an /// `voro.toml` whose one agent is a stub command that just reads the /// prompt. Returns the store, the dispatch context, and the project path. @@ -1932,14 +1937,7 @@ mod tests { /// Like [`fixture`], but with the whole `voro.toml` supplied, for tests /// exercising the session verbs. fn fixture_toml(agents_toml: &str) -> (Store, DispatchCtx, PathBuf) { - let root = std::env::temp_dir().join(format!( - "voro-dispatch-{}-{}", - std::process::id(), - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let root = fixture_root(); let project = root.join("project"); std::fs::create_dir_all(&project).unwrap(); git(&project, &["init", "-q"]); @@ -1959,6 +1957,70 @@ mod tests { (store, ctx, project) } + /// A scratch directory no other fixture can name, however many are built + /// at once: the process id separates test binaries, the counter separates + /// fixtures within one, and the stamp keeps reruns of a recycled pid apart. + fn fixture_root() -> PathBuf { + std::env::temp_dir().join(format!( + "voro-dispatch-{}-{}-{}", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos(), + FIXTURE_SEQ.fetch_add(1, Ordering::Relaxed) + )) + } + + #[test] + fn fixtures_built_at_once_on_many_threads_get_distinct_roots() { + const THREADS: usize = 8; + let gate = std::sync::Barrier::new(THREADS); + + let roots: Vec = std::thread::scope(|scope| { + let handles: Vec<_> = (0..THREADS) + .map(|_| { + scope.spawn(|| { + gate.wait(); + let (_store, _ctx, project) = fixture("true"); + project.parent().unwrap().to_path_buf() + }) + }) + .collect(); + handles.into_iter().map(|h| h.join().unwrap()).collect() + }); + + let distinct: std::collections::HashSet<_> = roots.iter().collect(); + assert_eq!(distinct.len(), THREADS, "roots collided: {roots:?}"); + } + + /// Eight fixtures are too few to catch a root name that leans on the clock + /// alone, so name a great many with nothing else running between them. + #[test] + fn roots_named_back_to_back_on_many_threads_are_all_distinct() { + const THREADS: usize = 8; + const EACH: usize = 500; + let gate = std::sync::Barrier::new(THREADS); + + let roots: Vec = std::thread::scope(|scope| { + let handles: Vec<_> = (0..THREADS) + .map(|_| { + scope.spawn(|| { + gate.wait(); + (0..EACH).map(|_| fixture_root()).collect::>() + }) + }) + .collect(); + handles + .into_iter() + .flat_map(|h| h.join().unwrap()) + .collect() + }); + + let distinct: std::collections::HashSet<_> = roots.iter().collect(); + assert_eq!(distinct.len(), THREADS * EACH, "roots collided"); + } + fn git(dir: &Path, args: &[&str]) { let status = Command::new("git") .arg("-C") From 480b4da69a91f5dd55c3a7d5be6a02f66c4ac075 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 14 Aug 2026 22:36:40 +0100 Subject: [PATCH 2/2] Say in the fixture why a nanosecond stamp repeats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The counter added alongside the stamp reads as unnecessary — a nanosecond is short enough that nothing should share one — and the comment asserted the collision without saying what causes it, leaving the next reader to make the same objection. Measured on this workstation: 100000 consecutive `SystemTime::now()` reads on one thread never repeat, so the objection holds sequentially. The clock does not advance a nanosecond at a time, though; it steps every 20-30ns. On one thread each read is ordered after the last and so lands on a later step, but threads have no such ordering, and any two reading inside one step read the same number: 4000 reads across eight threads gave 1493 duplicates. A test binary runs its tests on threads, which is exactly that case. The comments now carry the mechanism and the measurement, and the note on the four-thousand-root test says what the eight-fixture test above it cannot catch: those eight sit in `git init` long enough to drift onto separate steps, so they pass even on a clock-only name. Comments only; no code changed. --- crates/voro/src/dispatch.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/voro/src/dispatch.rs b/crates/voro/src/dispatch.rs index f13fd00..e5297f6 100644 --- a/crates/voro/src/dispatch.rs +++ b/crates/voro/src/dispatch.rs @@ -1921,8 +1921,15 @@ mod tests { use std::sync::atomic::{AtomicU64, Ordering}; use voro_core::{LivenessSource, NewTask, Priority}; - /// Distinguishes fixtures built within the same clock tick, which the - /// nanosecond stamp alone cannot separate. + /// Distinguishes fixtures built within the same clock tick. + /// + /// A nanosecond stamp reads as though it could not repeat, and on one + /// thread it does not: consecutive `SystemTime::now()` calls always differ, + /// because each is ordered after the last. Across threads there is no such + /// ordering, and the clock does not advance a nanosecond at a time — it + /// steps roughly every 20-30ns, so any two threads reading inside one step + /// read the same number. Measured on this workstation: 4000 reads across + /// eight threads yielded 1493 duplicates. static FIXTURE_SEQ: AtomicU64 = AtomicU64::new(0); /// A scratch database, a freshly-`git init`ed clean project, and an @@ -1994,8 +2001,10 @@ mod tests { assert_eq!(distinct.len(), THREADS, "roots collided: {roots:?}"); } - /// Eight fixtures are too few to catch a root name that leans on the clock - /// alone, so name a great many with nothing else running between them. + /// Eight fixtures spend long enough in `git init` to drift apart on the + /// clock, so the test above can pass even on a name that has no counter in + /// it. This one names four thousand roots with nothing in between, where a + /// clock-only name collides tens of times over. #[test] fn roots_named_back_to_back_on_many_threads_are_all_distinct() { const THREADS: usize = 8;