refactor: pass the catalog by value and correct the RenderMode docs

- Take `Arc<Catalog>` by value again through the validation entry points.
  `AppState::catalog()` returns an owned `Arc`, so `&state.catalog()` was
  cloning, borrowing the temporary, then cloning again at the leaf. Every
  consumer ends up owning the `Arc`, so by-value is the honest shape and it
  drops one clone per call. The one caller holding the catalog in a field
  now says `Arc::clone(&self.catalog)` explicitly.
- Correct the `RenderMode` doc comment. It claimed `Strict` is "used by
  run-create", but run-create renders `Structural` and promotes the
  resulting warnings to errors itself; `Strict` has no production caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

lib/apps/fabro-cli/src/commands/run/runner.rs

@@ -263,7 +263,12 @@ impl fabro_tool::RunManifestBuilder for WorkerRunManifestBuilder {
         cwd: &Path,
         user_settings_path: &Path,
     ) -> fabro_tool::ToolResult<RunManifest> {
-        run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path, &self.catalog)
+        run_tool_manifest::build_run_tool_manifest(
+            spec,
+            cwd,
+            user_settings_path,
+            Arc::clone(&self.catalog),
+        )
     }
 }
 

lib/apps/fabro-mcp-server/src/manifest_builder.rs

@@ -32,5 +32,5 @@ fn build_mcp_run_manifest(
         Catalog::from_builtin_with_overrides(&llm_catalog_settings)
             .map_err(|err| ToolError::message(err.to_string()))?,
     );
-    run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path, &catalog)
+    run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path, catalog)
 }

lib/apps/fabro-server/src/manifest_validation.rs

@@ -25,7 +25,7 @@ pub fn validate_manifest(
 pub fn validate_manifest_with_catalog(
     manifest_run_defaults: &RunLayer,
     manifest: &types::RunManifest,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
 ) -> Result<types::ValidateResponse> {
     let prepared = prepare(manifest_run_defaults, manifest)?;
     let validated =

lib/apps/fabro-server/src/run_manifest.rs

@@ -188,7 +188,7 @@ pub(crate) fn prepare_manifest_with_environment_defaults(
 
 pub(crate) fn validate_prepared_manifest(
     prepared: &PreparedManifest,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
 ) -> Result<Validated, WorkflowError> {
     validate_prepared_manifest_with_vars(prepared, catalog, HashMap::new())
 }
@@ -201,7 +201,7 @@ pub(crate) fn validate_prepared_manifest_structural(
 
 pub(crate) fn validate_prepared_manifest_with_vars(
     prepared: &PreparedManifest,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
     vars: HashMap<String, String>,
 ) -> Result<Validated, WorkflowError> {
     validate_with_catalog(manifest_validate_input(prepared, vars), catalog)
@@ -209,7 +209,7 @@ pub(crate) fn validate_prepared_manifest_with_vars(
 
 pub(crate) fn validate_prepared_manifest_for_preflight(
     prepared: &PreparedManifest,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
     vars: HashMap<String, String>,
     ready_providers: &[ProviderId],
 ) -> Result<Validated, WorkflowError> {
@@ -1554,7 +1554,7 @@ digraph Demo {{
         .unwrap();
         let validated = validate_prepared_manifest_for_preflight(
             &prepared,
-            &state.catalog(),
+            state.catalog(),
             HashMap::new(),
             &ready_providers,
         )
@@ -1639,7 +1639,7 @@ enabled = {clone_enabled}
             &manifest,
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
         let resolved = materialize_run(
             prepared.settings.clone(),
             validated.graph(),
@@ -2199,7 +2199,7 @@ name = "Control Plane"
             &invalid_manifest(),
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
 
         assert!(validated.has_errors());
 
@@ -2244,7 +2244,7 @@ issues = "read"
             &manifest,
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
         assert!(!validated.has_errors());
 
         let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
@@ -2294,7 +2294,7 @@ id = "local"
             &manifest,
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
 
         assert!(!validated.has_errors());
 
@@ -2403,7 +2403,7 @@ id = "daytona"
             &manifest,
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
 
         let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
             .await
@@ -2471,7 +2471,7 @@ digraph Demo {
             &manifest,
         )
         .unwrap();
-        let validated = validate_prepared_manifest(&prepared, &test_catalog()).unwrap();
+        let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap();
 
         let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated)
             .await
@@ -2585,7 +2585,7 @@ digraph Demo {
             &manifest,
         )
         .unwrap();
-        let Err(error) = validate_prepared_manifest(&prepared, &test_catalog()) else {
+        let Err(error) = validate_prepared_manifest(&prepared, test_catalog()) else {
             panic!("unknown provider should fail static validation");
         };
 
@@ -2652,7 +2652,7 @@ digraph Demo {
         assert!(ready_providers.is_empty());
         let validated = validate_prepared_manifest_for_preflight(
             &prepared,
-            &state.catalog(),
+            state.catalog(),
             HashMap::new(),
             &ready_providers,
         )

lib/apps/fabro-server/src/run_tool_manifest.rs

@@ -14,7 +14,7 @@ pub fn build_run_tool_manifest(
     spec: &ValidatedCreateRunSpec,
     cwd: &Path,
     user_settings_path: &Path,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
 ) -> ToolResult<types::RunManifest> {
     let built = fabro_manifest::build_run_manifest(ManifestBuildInput {
         workflow:             PathBuf::from(&spec.workflow),

lib/apps/fabro-server/src/server/handler/graph.rs

@@ -52,7 +52,7 @@ async fn render_graph_from_manifest(
         Ok(prepared) => prepared,
         Err(err) => return ApiError::bad_request(err.to_string()).into_response(),
     };
-    let validated = match run_manifest::validate_prepared_manifest(&prepared, &state.catalog()) {
+    let validated = match run_manifest::validate_prepared_manifest(&prepared, state.catalog()) {
         Ok(validated) => validated,
         Err(err) => return ApiError::bad_request(err.to_string()).into_response(),
     };

lib/apps/fabro-server/src/server/handler/runs.rs

@@ -829,7 +829,7 @@ async fn run_preflight(
     let (llm_result, ready_providers) = state.resolve_llm_client_with_ready_ids().await;
     let mut validated = match run_manifest::validate_prepared_manifest_for_preflight(
         &prepared,
-        &state.catalog(),
+        state.catalog(),
         vars,
         &ready_providers,
     ) {
@@ -879,15 +879,17 @@ async fn validate_run_manifest(
         return ApiError::bad_request(format!("Run config variable interpolation failed: {err}"))
             .into_response();
     }
-    let validated =
-        match run_manifest::validate_prepared_manifest_with_vars(&prepared, &state.catalog(), vars)
-        {
-            Ok(validated) => validated,
-            Err(WorkflowError::Parse(_)) => {
-                return ApiError::bad_request("Validation failed").into_response();
-            }
-            Err(err) => return ApiError::bad_request(err.to_string()).into_response(),
-        };
+    let validated = match run_manifest::validate_prepared_manifest_with_vars(
+        &prepared,
+        state.catalog(),
+        vars,
+    ) {
+        Ok(validated) => validated,
+        Err(WorkflowError::Parse(_)) => {
+            return ApiError::bad_request("Validation failed").into_response();
+        }
+        Err(err) => return ApiError::bad_request(err.to_string()).into_response(),
+    };
     (
         StatusCode::OK,
         Json(run_manifest::validate_response(&prepared, &validated)),

lib/components/fabro-workflow/src/handler/manager_loop.rs

@@ -131,7 +131,7 @@ fn validate_child_workflow(
             cwd,
             custom_transforms: Vec::new(),
         },
-        &services.run.catalog,
+        Arc::clone(&services.run.catalog),
     )?;
     validated.promote_template_undefined_variables_to_errors();
     validated.raise_on_errors()?;

lib/components/fabro-workflow/src/operations/create.rs

@@ -559,7 +559,7 @@ reasoning = false
                 cwd: PathBuf::from("."),
                 custom_transforms: Vec::new(),
             },
-            &test_catalog(),
+            test_catalog(),
         )
         .unwrap()
     }

lib/components/fabro-workflow/src/operations/validate.rs

@@ -35,12 +35,9 @@ pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
 /// Parse, transform, and validate a DOT source string against `catalog`.
 pub fn validate_with_catalog(
     input: ValidateInput,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
 ) -> Result<Validated, Error> {
-    validate_resolving_models(
-        input,
-        Some(ModelResolutionTransform::new(Arc::clone(catalog))),
-    )
+    validate_resolving_models(input, Some(ModelResolutionTransform::new(catalog)))
 }
 
 /// Parse, transform, and validate, resolving models against the ready
@@ -48,14 +45,14 @@ pub fn validate_with_catalog(
 /// provider-readiness selection failures.
 pub fn validate_with_ready_providers(
     input: ValidateInput,
-    catalog: &Arc<Catalog>,
+    catalog: Arc<Catalog>,
     ready_providers: &[ProviderId],
 ) -> Result<Validated, Error> {
     validate_resolving_models(
         input,
         Some(
             ModelResolutionTransform::for_eligible(
-                Arc::clone(catalog),
+                catalog,
                 ready_providers.iter().cloned().collect(),
             )
             .with_catalog_fallback(true),

lib/components/fabro-workflow/src/transforms/variable_expansion.rs

@@ -19,17 +19,19 @@ use crate::static_reference::{
 
 /// How the template-expansion pass should treat undefined input variables.
 ///
-/// Validate is structural — it should not fail just because the user has not
-/// bound `{{ inputs.* }}` yet. Run-start is strict — missing inputs are real
-/// errors. Splitting the two lets validate work on a bare `.fabro` while
-/// run-start preserves its current hard-fail behavior.
+/// Neither validate nor run-create should fail just because the user has not
+/// bound `{{ inputs.* }}` yet, so both render structurally. Run-create then
+/// promotes the resulting warnings to errors itself, which keeps its hard-fail
+/// behavior while still reporting every undefined variable in one pass rather
+/// than aborting on the first.
 #[derive(Clone, Copy, Debug)]
 pub enum RenderMode {
-    /// Undefined inputs are hard errors. Used by run-create.
+    /// Undefined inputs abort the pass with a hard error. No production caller
+    /// uses this today; run-create promotes structural warnings instead.
     Strict,
     /// Undefined inputs render as empty and become warning diagnostics on the
     /// returned `Validated`, so structural lints still run. Used by
-    /// `fabro validate`.
+    /// `fabro validate` and by run-create.
     Structural,
 }