From 6a951652a70ad5b981083461b7ac9b82f3210b2e Mon Sep 17 00:00:00 2001
From: Scott Werner <stwerner@vt.edu>
Date: Fri, 11 Sep 2026 15:24:10 -0600
Subject: [PATCH] Report path collisions in a deterministic order

The refactored ancestor-collision loop iterated the HashMap of folded
keys, so when a version contained more than one ancestor collision the
reported pair depended on the hasher seed. The same request could
produce different 422 bodies from POST /workflow-versions on repeated
submissions.

Collect the input into a Vec and walk it in order for the ancestor
pass, matching the previous behavior, and add a test with two
collisions that runs the check repeatedly.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---
 .../fabro-types/src/workflow_version.rs       | 36 ++++++++++++++++---
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/lib/foundation/fabro-types/src/workflow_version.rs b/lib/foundation/fabro-types/src/workflow_version.rs
index 555be2814..297e485d6 100644
--- a/lib/foundation/fabro-types/src/workflow_version.rs
+++ b/lib/foundation/fabro-types/src/workflow_version.rs
@@ -189,16 +189,22 @@ fn validate_path_collisions<'a>(
     paths: impl IntoIterator<Item = &'a WorkflowPath>,
     key: impl Fn(&'a str) -> Cow<'a, str>,
 ) -> Result<(), WorkflowVersionShapeError> {
-    let mut by_text = HashMap::new();
-    for path in paths {
-        if let Some(existing) = by_text.insert(key(path.as_str()), path) {
+    let keyed: Vec<(Cow<'a, str>, &WorkflowPath)> = paths
+        .into_iter()
+        .map(|path| (key(path.as_str()), path))
+        .collect();
+    let mut by_text = HashMap::with_capacity(keyed.len());
+    for (text, path) in &keyed {
+        if let Some(existing) = by_text.insert(text.as_ref(), *path) {
             return Err(WorkflowVersionShapeError::PathCollision {
                 first:  existing.clone(),
-                second: path.clone(),
+                second: (*path).clone(),
             });
         }
     }
-    for (text, path) in &by_text {
+    // Walk the input order, not the map, so the reported pair is stable when
+    // more than one ancestor collision exists.
+    for (text, path) in &keyed {
         for (index, _) in text.match_indices('/') {
             if let Some(ancestor) = by_text.get(&text[..index]) {
                 return Err(WorkflowVersionShapeError::PathCollision {
@@ -560,6 +566,26 @@ mod tests {
 mod source_path_tests {
     use super::*;
 
+    #[test]
+    fn ancestor_collision_reports_the_first_pair_in_input_order() {
+        let paths = ["assets", "assets/item.txt", "libs", "libs/child.fabro"]
+            .map(|path| WorkflowPath::new(path).unwrap());
+        for _ in 0..32 {
+            let error = validate_workflow_source_paths(paths.iter()).unwrap_err();
+            assert_eq!(
+                error.to_string(),
+                "workflow paths collide: `assets` and `assets/item.txt`"
+            );
+            let version = WorkflowVersion::new(
+                paths[1].clone(),
+                paths.iter().map(|p| (p.clone(), String::new())).collect(),
+                BTreeMap::new(),
+            )
+            .unwrap_err();
+            assert_eq!(version.to_string(), error.to_string());
+        }
+    }
+
     #[test]
     fn workflow_source_collisions_are_portable_in_both_orders() {
         for pair in [