From 0432c019ee3d3004866d9254d77ff2ef8e900e6b Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <bryan@brynary.com>
Date: Thu, 23 Jul 2026 13:38:55 -0400
Subject: [PATCH] chore: address review feedback on error mapping and
 test-support gating

- Return 400 (not 500) for WorkflowError::ModelReference from run
  creation, matching ModelSelection: an ambiguous model/provider token
  is user input, not a server fault.
- Gate fabro-workflow's test_support module behind
  cfg(any(test, feature = "test-support")) so the feature actually
  controls exposure, per the repo's test-support boundary guidance.
  Add the self dev-dependency so tests/it keeps compiling, and gate
  the pipeline helpers that only test_support consumed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 Cargo.lock                                         | 1 +
 lib/crates/fabro-server/src/server/handler/runs.rs | 2 +-
 lib/crates/fabro-workflow/Cargo.toml               | 1 +
 lib/crates/fabro-workflow/src/lib.rs               | 2 +-
 lib/crates/fabro-workflow/src/pipeline/finalize.rs | 1 +
 lib/crates/fabro-workflow/src/pipeline/mod.rs      | 6 +++---
 6 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 098d71cbf..c4bffa44c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -3368,6 +3368,7 @@ dependencies = [
  "fabro-util",
  "fabro-validate",
  "fabro-vault",
+ "fabro-workflow",
  "futures",
  "git2",
  "hex",
diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs
index e3e7f3dd2..57562fc4c 100644
--- a/lib/crates/fabro-server/src/server/handler/runs.rs
+++ b/lib/crates/fabro-server/src/server/handler/runs.rs
@@ -640,7 +640,7 @@ pub(crate) async fn create_run_from_manifest(
         Err(WorkflowError::ValidationFailed { .. } | WorkflowError::Parse(_)) => {
             return ApiError::bad_request("Validation failed").into_response();
         }
-        Err(err @ WorkflowError::ModelSelection(_)) => {
+        Err(err @ (WorkflowError::ModelSelection(_) | WorkflowError::ModelReference(_))) => {
             return ApiError::bad_request(err.to_string()).into_response();
         }
         Err(err) => {
diff --git a/lib/crates/fabro-workflow/Cargo.toml b/lib/crates/fabro-workflow/Cargo.toml
index fc7166d7a..e719de2fd 100644
--- a/lib/crates/fabro-workflow/Cargo.toml
+++ b/lib/crates/fabro-workflow/Cargo.toml
@@ -76,6 +76,7 @@ fabro-vault = { path = "../fabro-vault" }
 [dev-dependencies]
 base64.workspace = true
 fabro-acp = { path = "../fabro-acp", features = ["test-support"] }
+fabro-workflow = { path = ".", features = ["test-support"] }
 fabro-api = { path = "../fabro-api" }
 fabro-environment = { path = "../fabro-environment" }
 fabro-sandbox = { path = "../fabro-sandbox", features = ["daytona", "docker", "test-support"] }
diff --git a/lib/crates/fabro-workflow/src/lib.rs b/lib/crates/fabro-workflow/src/lib.rs
index d5efaaebb..b4d3cc727 100644
--- a/lib/crates/fabro-workflow/src/lib.rs
+++ b/lib/crates/fabro-workflow/src/lib.rs
@@ -332,7 +332,7 @@ pub mod services;
 mod stage_scope;
 pub mod static_reference;
 pub mod steering_hub;
-#[doc(hidden)]
+#[cfg(any(test, feature = "test-support"))]
 pub mod test_support;
 #[doc(hidden)]
 pub mod transforms;
diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs
index 6e90991dc..9fdcad717 100644
--- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs
+++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs
@@ -438,6 +438,7 @@ async fn compute_final_patch(
     (final_patch, diff_summary)
 }
 
+#[cfg(any(test, feature = "test-support"))]
 pub(crate) fn billing_from_projection(projection: &RunProjection) -> Option<BilledTokenCounts> {
     billing_rollup_from_projection(projection, None).billing_if_present()
 }
diff --git a/lib/crates/fabro-workflow/src/pipeline/mod.rs b/lib/crates/fabro-workflow/src/pipeline/mod.rs
index ef517e9cc..ac77cc992 100644
--- a/lib/crates/fabro-workflow/src/pipeline/mod.rs
+++ b/lib/crates/fabro-workflow/src/pipeline/mod.rs
@@ -9,9 +9,9 @@ pub(crate) mod types;
 mod validate;
 
 pub use execute::execute;
-pub(crate) use finalize::{
-    billing_from_projection, build_conclusion_from_store, build_terminal_event,
-};
+pub(crate) use finalize::build_conclusion_from_store;
+#[cfg(any(test, feature = "test-support"))]
+pub(crate) use finalize::{billing_from_projection, build_terminal_event};
 pub use finalize::{classify_engine_result, finalize, write_finalize_commit};
 pub use initialize::initialize;
 pub use parse::parse;