scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-08-28 05:27:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #651 from fabro-sh/fix/offline-validation-catalog-free

Browse source 
fix(cli): keep offline validation catalog-free
This commit is contained in:
Bryan Helmkamp 2026-07-28 14:28:35 -04:00 • committed by GitHub
commit c5d3345144
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
21 changed files with 413 additions and 329 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
lib/apps/fabro-cli/src/commands/run/create.rs
View file

@ -61,11 +61,8 @@ pub(crate) async fn create_run(
None
};
let mut validation = manifest_validation::validate_manifest(
&RunLayer::default(),
&built.manifest,
ctx.catalog()?,
)?;
let mut validation =
manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)?;
manifest_validation::promote_template_undefined_variables_to_errors(&mut validation);
let diagnostics = api_diagnostics_to_local(&validation.workflow.diagnostics);
if !quiet {

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

@ -263,12 +263,7 @@ 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,
Arc::clone(&self.catalog),
)
run_tool_manifest::build_run_tool_manifest(spec, cwd, user_settings_path, &self.catalog)
}
}

6
lib/apps/fabro-cli/src/commands/validate.rs
View file

@ -23,11 +23,7 @@ pub(crate) fn run(
user_settings_path: Some(active_settings_path(None)),
..Default::default()
})?;
let response = manifest_validation::validate_manifest(
&RunLayer::default(),
&built.manifest,
base_ctx.catalog()?,
)?;
let response = manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest)?;
let diagnostics = api_diagnostics_to_local(&response.workflow.diagnostics);
if base_ctx.json_output() {

34
lib/apps/fabro-cli/tests/it/cmd/create.rs
View file

@ -101,6 +101,40 @@ fn create_uses_explicit_server_target_and_prints_remote_run_id() {
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_defers_provider_validation_to_the_server() {
let context = test_context!();
let server = MockServer::start();
let run_id = unique_run_id();
let mock = server.mock(|when, then| {
when.method("POST")
.path("/api/v1/runs")
.body_includes(r#"provider=\"server-only\""#);
then.status(201)
.header("Content-Type", "application/json")
.body(run_status_response(run_id.as_str(), "submitted").to_string());
});
let output = context
.create_cmd()
.args([
"--server",
&format!("{}/api/v1", server.base_url()),
"--dry-run",
fixture("server-model.fabro").to_str().unwrap(),
])
.output()
.expect("command should execute");
assert!(
output.status.success(),
"local validation should not reject a server-owned provider\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
mock.assert();
assert_eq!(output_stdout(&output).trim(), run_id.as_str());
}
#[test]
fn create_uses_configured_server_target_without_server_flag() {
let context = test_context!();

18
lib/apps/fabro-cli/tests/it/cmd/validate.rs
View file

@ -69,6 +69,24 @@ fn simple_does_not_connect_to_configured_server() {
);
}
/// Offline validation has no model catalog, so a model or provider the server
/// owns must pass rather than be reported as unknown.
#[test]
fn server_owned_provider_is_not_rejected_by_offline_validation() {
let context = test_context!();
let mut cmd = context.validate();
cmd.arg(fixture("server-model.fabro"));
fabro_snapshot!(context.filters(), cmd, @"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Workflow: ServerModel (3 nodes, 2 edges)
Graph: [FIXTURES]/server-model.fabro
Validation: OK
");
}
#[test]
fn branching() {
let context = test_context!();

2
lib/apps/fabro-mcp-server/src/manifest_builder.rs
View file

@ -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)
}

40
lib/apps/fabro-server/src/manifest_validation.rs
View file

@ -3,42 +3,48 @@ use std::sync::Arc;
use anyhow::Result;
use fabro_api::types;
use fabro_config::{EnvironmentLayer, MergeMap, RunLayer};
use fabro_config::RunLayer;
use fabro_model::Catalog;
use fabro_workflow::pipeline::TEMPLATE_UNDEFINED_VARIABLE_RULE;
use crate::run_manifest;
/// Validate a manifest without a model catalog. Model and provider
/// availability is deferred to the server, which owns the catalog.
pub fn validate_manifest(
manifest_run_defaults: &RunLayer,
manifest: &types::RunManifest,
catalog: Arc<Catalog>,
) -> Result<types::ValidateResponse> {
validate_manifest_with_environment_defaults(
manifest_run_defaults,
&fabro_environment::seeded_catalog_layer(),
manifest,
catalog,
)
let prepared = prepare(manifest_run_defaults, manifest)?;
let validated = run_manifest::validate_prepared_manifest_structural(&prepared)
.map_err(anyhow::Error::new)?;
Ok(run_manifest::validate_response(&prepared, &validated))
}
pub fn validate_manifest_with_environment_defaults(
/// Validate a manifest including the catalog-backed model and provider rules.
pub fn validate_manifest_with_catalog(
manifest_run_defaults: &RunLayer,
manifest_environment_defaults: &MergeMap<EnvironmentLayer>,
manifest: &types::RunManifest,
catalog: Arc<Catalog>,
catalog: &Arc<Catalog>,
) -> Result<types::ValidateResponse> {
let prepared = run_manifest::prepare_manifest_with_environment_defaults(
manifest_run_defaults,
manifest_environment_defaults,
&HashMap::new(),
manifest,
)?;
let prepared = prepare(manifest_run_defaults, manifest)?;
let validated =
run_manifest::validate_prepared_manifest(&prepared, catalog).map_err(anyhow::Error::new)?;
Ok(run_manifest::validate_response(&prepared, &validated))
}
fn prepare(
manifest_run_defaults: &RunLayer,
manifest: &types::RunManifest,
) -> Result<run_manifest::PreparedManifest> {
run_manifest::prepare_manifest_with_environment_defaults(
manifest_run_defaults,
&fabro_environment::seeded_catalog_layer(),
&HashMap::new(),
manifest,
)
}
pub fn promote_template_undefined_variables_to_errors(response: &mut types::ValidateResponse) {
let mut promoted = false;
for diagnostic in &mut response.workflow.diagnostics {

40
lib/apps/fabro-server/src/run_manifest.rs
View file

@ -35,7 +35,8 @@ use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSecti
use fabro_validate::Severity;
use fabro_workflow::Error as WorkflowError;
use fabro_workflow::operations::{
CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_ready_providers,
CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_catalog,
validate_with_ready_providers,
};
use fabro_workflow::pipeline::Validated;
use fabro_workflow::run_materialization::materialize_run_with_ready_providers;
@ -187,34 +188,40 @@ 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())
}
pub(crate) fn validate_prepared_manifest_structural(
prepared: &PreparedManifest,
) -> Result<Validated, WorkflowError> {
validate(manifest_validate_input(prepared, HashMap::new()))
}
pub(crate) fn validate_prepared_manifest_with_vars(
prepared: &PreparedManifest,
catalog: Arc<Catalog>,
catalog: &Arc<Catalog>,
vars: HashMap<String, String>,
) -> Result<Validated, WorkflowError> {
validate(manifest_validate_input(prepared, catalog, vars))
validate_with_catalog(manifest_validate_input(prepared, vars), catalog)
}
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> {
validate_with_ready_providers(
manifest_validate_input(prepared, catalog, vars),
manifest_validate_input(prepared, vars),
catalog,
ready_providers,
)
}
fn manifest_validate_input(
prepared: &PreparedManifest,
catalog: Arc<Catalog>,
vars: HashMap<String, String>,
) -> ValidateInput {
ValidateInput {
@ -223,7 +230,6 @@ fn manifest_validate_input(
vars,
cwd: prepared.cwd.clone(),
custom_transforms: Vec::new(),
catalog,
}
}
@ -1548,7 +1554,7 @@ digraph Demo {{
.unwrap();
let validated = validate_prepared_manifest_for_preflight(
&prepared,
state.catalog(),
&state.catalog(),
HashMap::new(),
&ready_providers,
)
@ -1633,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(),
@ -2193,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());
@ -2238,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)
@ -2288,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());
@ -2397,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
@ -2465,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
@ -2579,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");
};
@ -2646,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,
)

11
lib/apps/fabro-server/src/run_tool_manifest.rs
View file

@ -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),
@ -29,9 +29,12 @@ pub fn build_run_tool_manifest(
})
.map_err(|err| ToolError::from_anyhow(&err))?;
let mut validation =
manifest_validation::validate_manifest(&RunLayer::default(), &built.manifest, catalog)
.map_err(|err| ToolError::from_anyhow(&err))?;
let mut validation = manifest_validation::validate_manifest_with_catalog(
&RunLayer::default(),
&built.manifest,
catalog,
)
.map_err(|err| ToolError::from_anyhow(&err))?;
manifest_validation::promote_template_undefined_variables_to_errors(&mut validation);
if !validation.ok {
return Err(ToolError::message("workflow manifest validation failed"));

2
lib/apps/fabro-server/src/server/handler/graph.rs
View file

@ -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(),
};

22
lib/apps/fabro-server/src/server/handler/runs.rs
View file

@ -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,17 +879,15 @@ 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)),

53
lib/components/fabro-workflow/src/handler/manager_loop.rs
View file

@ -16,7 +16,7 @@ use crate::artifact_upload::ArtifactSink;
use crate::condition::evaluate_condition;
use crate::context::{Context, WorkflowContext, context_diff_public, keys};
use crate::error::Error;
use crate::operations::{ValidateInput, WorkflowInput, validate};
use crate::operations::{ValidateInput, WorkflowInput, validate_with_catalog};
use crate::outcome::{Outcome, OutcomeExt, StageOutcome};
use crate::pipeline::types::Initialized;
use crate::run_options::RunOptions;
@ -65,20 +65,14 @@ fn parse_child_graph(node: &Node, services: &EngineServices) -> Result<ParsedChi
.get("stack.child_dot_source")
.and_then(|v| v.as_str())
{
let mut validated = validate(ValidateInput {
workflow: WorkflowInput::DotSource {
let graph = validate_child_workflow(
WorkflowInput::DotSource {
source: dot.to_string(),
base_dir: None,
},
settings: WorkflowSettings::default(),
vars: std::collections::HashMap::new(),
cwd: cwd.clone(),
custom_transforms: Vec::new(),
catalog: Arc::clone(&services.run.catalog),
})?;
validated.promote_template_undefined_variables_to_errors();
validated.raise_on_errors()?;
let (graph, _, _) = validated.into_parts();
cwd,
services,
)?;
return Ok(ParsedChildWorkflow {
graph,
workflow_path: None,
@ -113,17 +107,7 @@ fn parse_child_graph(node: &Node, services: &EngineServices) -> Result<ParsedChi
WorkflowInput::Bundled(workflow) => Some(workflow.path.clone()),
WorkflowInput::Path(_) | WorkflowInput::DotSource { .. } => None,
};
let mut validated = validate(ValidateInput {
workflow,
settings: WorkflowSettings::default(),
vars: std::collections::HashMap::new(),
cwd,
custom_transforms: Vec::new(),
catalog: Arc::clone(&services.run.catalog),
})?;
validated.promote_template_undefined_variables_to_errors();
validated.raise_on_errors()?;
let (graph, _, _) = validated.into_parts();
let graph = validate_child_workflow(workflow, cwd, services)?;
return Ok(ParsedChildWorkflow {
graph,
workflow_path,
@ -132,6 +116,29 @@ fn parse_child_graph(node: &Node, services: &EngineServices) -> Result<ParsedChi
Err(Error::handler("No child workflow source".to_string()))
}
/// Validate a child workflow against the run's catalog, failing on any error
/// diagnostic (undefined template variables included).
fn validate_child_workflow(
workflow: WorkflowInput,
cwd: PathBuf,
services: &EngineServices,
) -> Result<Graph, Error> {
let mut validated = validate_with_catalog(
ValidateInput {
workflow,
settings: WorkflowSettings::default(),
vars: HashMap::new(),
cwd,
custom_transforms: Vec::new(),
},
&services.run.catalog,
)?;
validated.promote_template_undefined_variables_to_errors();
validated.raise_on_errors()?;
let (graph, _, _) = validated.into_parts();
Ok(graph)
}
#[async_trait]
impl Handler for SubWorkflowHandler {
async fn execute(

188
lib/components/fabro-workflow/src/operations/create.rs
View file

@ -28,7 +28,7 @@ use crate::pipeline::{self, Persisted, TransformOptions, Validated};
use crate::records::RunSpec;
use crate::run_lookup::default_scratch_base;
use crate::run_materialization::materialize_run;
use crate::transforms::{RenderMode, Transform};
use crate::transforms::{ModelResolutionTransform, RenderMode};
use crate::workflow_bundle::{RunDefinition, WorkflowBundle};
#[derive(Clone, Debug)]
@ -293,28 +293,21 @@ fn create_from_source(
file_resolver: Option<Arc<dyn FileResolver>>,
goal_override: Option<&str>,
) -> Result<Persisted, Error> {
let template_context = template_context(Some(&options.settings), vars);
let mut validated = preprocess_and_validate(
dot_source,
options.source_name.clone(),
let mut validated = preprocess_and_validate(dot_source, goal_override, &TransformOptions {
current_dir,
file_resolver,
Vec::new(),
template_context,
goal_override,
RenderMode::Structural,
options
.settings
.run
.model
.provider
.as_deref()
.filter(|provider| !provider.is_empty())
.map(ProviderId::new),
&options.configured_providers,
false,
&options.catalog,
)?;
template_context: template_context(Some(&options.settings), vars),
source_name: options.source_name.clone(),
render_mode: RenderMode::Structural,
custom_transforms: Vec::new(),
model_resolution: Some(
ModelResolutionTransform::for_eligible(
Arc::clone(&options.catalog),
options.configured_providers.iter().cloned().collect(),
)
.with_default_provider(configured_default_provider(&options.settings)),
),
})?;
validated.promote_template_undefined_variables_to_errors();
if validated.has_errors() {
@ -326,36 +319,37 @@ fn create_from_source(
persist_validated(validated, options)
}
/// Parse, transform, and validate `dot_source`.
///
/// `options.model_resolution` drives both halves of catalog awareness: it
/// selects concrete models during TRANSFORM and enables the catalog-backed
/// lint rules during VALIDATE. `None` leaves authored model and provider
/// selectors untouched for offline structural validation.
pub(super) fn preprocess_and_validate(
dot_source: &str,
source_name: Option<String>,
current_dir: Option<PathBuf>,
file_resolver: Option<Arc<dyn FileResolver>>,
custom_transforms: Vec<Box<dyn Transform>>,
template_context: TemplateContext,
goal_override: Option<&str>,
render_mode: RenderMode,
default_provider: Option<ProviderId>,
eligible_providers: &[ProviderId],
catalog_fallback: bool,
catalog: &Arc<Catalog>,
options: &TransformOptions,
) -> Result<Validated, Error> {
let mut parsed = pipeline::parse(dot_source)?;
apply_goal_override(&mut parsed.graph, goal_override);
let transformed = pipeline::transform(parsed, &TransformOptions {
current_dir,
file_resolver,
template_context,
source_name,
render_mode,
custom_transforms,
catalog: Arc::clone(catalog),
default_provider,
eligible_providers: eligible_providers.iter().cloned().collect(),
catalog_fallback,
})?;
Ok(pipeline::validate(transformed, catalog.as_ref(), &[]))
let transformed = pipeline::transform(parsed, options)?;
let catalog = options
.model_resolution
.as_ref()
.map(ModelResolutionTransform::catalog);
Ok(pipeline::validate(transformed, catalog, &[]))
}
/// The workflow-level default provider, treating an empty setting as unset.
pub(super) fn configured_default_provider(settings: &WorkflowSettings) -> Option<ProviderId> {
settings
.run
.model
.provider
.as_deref()
.filter(|provider| !provider.is_empty())
.map(ProviderId::new)
}
pub(super) fn template_context(
@ -462,8 +456,9 @@ mod tests {
use object_store::memory::InMemory;
use super::*;
use crate::operations::{ValidateInput, validate};
use crate::operations::{ValidateInput, validate, validate_with_catalog};
use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE};
use crate::transforms::Transform;
use crate::workflow_bundle::BundledWorkflow;
fn memory_store() -> Arc<Database> {
Arc::new(Database::new(
@ -553,17 +548,19 @@ reasoning = false
}
fn validate_dot(dot_source: &str, settings: WorkflowSettings) -> Validated {
validate(ValidateInput {
workflow: WorkflowInput::DotSource {
source: dot_source.to_string(),
base_dir: None,
validate_with_catalog(
ValidateInput {
workflow: WorkflowInput::DotSource {
source: dot_source.to_string(),
base_dir: None,
},
settings,
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
},
settings,
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
&test_catalog(),
)
.unwrap()
}
@ -573,21 +570,35 @@ reasoning = false
fn validate_dot_with_vars(dot_source: &str, vars: HashMap<String, String>) -> Validated {
preprocess_and_validate(
dot_source,
Some("workflow.fabro".to_string()),
Some(PathBuf::from(".")),
None,
Vec::new(),
template_context(Some(&WorkflowSettings::default()), vars),
None,
RenderMode::Structural,
None,
&test_provider_ids(),
false,
&test_catalog(),
&test_transform_options(
PathBuf::from("."),
None,
RenderMode::Structural,
template_context(Some(&WorkflowSettings::default()), vars),
),
)
.unwrap()
}
/// Catalog-backed TRANSFORM options for the built-in test catalog.
fn test_transform_options(
current_dir: PathBuf,
file_resolver: Option<Arc<dyn FileResolver>>,
render_mode: RenderMode,
template_context: TemplateContext,
) -> TransformOptions {
TransformOptions {
current_dir: Some(current_dir),
file_resolver,
template_context,
source_name: Some("workflow.fabro".to_string()),
render_mode,
custom_transforms: Vec::new(),
model_resolution: Some(ModelResolutionTransform::new(test_catalog())),
}
}
const MINIMAL_DOT: &str = r#"digraph Test {
graph [goal="Build feature"]
start [shape=Mdiamond]
@ -744,17 +755,13 @@ reasoning = false
let result = preprocess_and_validate(
dot,
Some("workflow.fabro".to_string()),
Some(PathBuf::from(".")),
None,
Vec::new(),
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
None,
RenderMode::Strict,
None,
&test_provider_ids(),
false,
&test_catalog(),
&test_transform_options(
PathBuf::from("."),
None,
RenderMode::Strict,
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
),
);
let Err(err) = result else {
panic!("expected strict mode to hard-fail on unbound inline prompt");
@ -781,19 +788,15 @@ reasoning = false
let result = preprocess_and_validate(
dot,
Some("workflow.fabro".to_string()),
Some(dir.path().to_path_buf()),
Some(Arc::new(crate::file_resolver::FilesystemFileResolver::new(
None,
))),
Vec::new(),
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
None,
RenderMode::Strict,
None,
&test_provider_ids(),
false,
&test_catalog(),
&test_transform_options(
dir.path().to_path_buf(),
Some(Arc::new(crate::file_resolver::FilesystemFileResolver::new(
None,
))),
RenderMode::Strict,
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
),
);
let Err(err) = result else {
panic!("expected strict mode to hard-fail on unbound imported prompt");
@ -852,7 +855,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
});
assert!(result.is_err());
@ -901,7 +903,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
let file_missing = validate(ValidateInput {
@ -920,7 +921,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
assert_eq!(
@ -944,7 +944,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
let file_goal = validate(ValidateInput {
@ -963,7 +962,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
assert_eq!(
@ -1055,7 +1053,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
});
assert!(result.is_err());
}
@ -1100,7 +1097,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: vec![Box::new(TagTransform)],
catalog: test_catalog(),
})
.unwrap();
validated.raise_on_errors().unwrap();
@ -1135,7 +1131,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
validated.raise_on_errors().unwrap();
@ -1177,7 +1172,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
@ -1227,7 +1221,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
@ -1278,7 +1271,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();

2
lib/components/fabro-workflow/src/operations/mod.rs
View file

@ -22,7 +22,7 @@ pub use rewind::{RewindInput, RewindOutcome, rewind};
pub use source::WorkflowInput;
pub use start::{StartServices, Started, start};
pub use timeline::{ForkTarget, RunTimeline, TimelineEntry, build_timeline, timeline};
pub use validate::{ValidateInput, validate, validate_with_ready_providers};
pub use validate::{ValidateInput, validate, validate_with_catalog, validate_with_ready_providers};
pub use crate::pipeline::{LlmSpec, SandboxEnvSpec};
pub use crate::transforms::RenderMode;

98
lib/components/fabro-workflow/src/operations/validate.rs
View file

@ -5,12 +5,12 @@ use std::sync::Arc;
use fabro_model::{Catalog, ProviderId};
use fabro_types::WorkflowSettings;
use super::create::{preprocess_and_validate, template_context};
use super::create::{configured_default_provider, preprocess_and_validate, template_context};
use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow};
use crate::error::Error;
use crate::operations::RenderMode;
use crate::pipeline::Validated;
use crate::transforms::Transform;
use crate::pipeline::{TransformOptions, Validated};
use crate::transforms::{ModelResolutionTransform, Transform};
pub struct ValidateInput {
pub workflow: WorkflowInput,
@ -20,20 +20,27 @@ pub struct ValidateInput {
pub vars: HashMap<String, String>,
pub cwd: PathBuf,
pub custom_transforms: Vec<Box<dyn Transform>>,
pub catalog: Arc<Catalog>,
}
/// Parse, transform, and validate a DOT source string.
/// Parse, transform, and structurally validate a DOT source string without a
/// model catalog. Model and provider availability is left to the caller that
/// owns a catalog — typically the server.
///
/// Returns `Validated` even when validation produced errors. Call
/// `validated.raise_on_errors()` if the caller wants to fail fast.
pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
let eligible_providers = input
.catalog
.all_provider_ids()
.into_iter()
.collect::<Vec<_>>();
validate_with_eligible_providers(input, &eligible_providers, false)
validate_resolving_models(input, None)
}
/// Parse, transform, and validate a DOT source string against `catalog`.
pub fn validate_with_catalog(
input: ValidateInput,
catalog: &Arc<Catalog>,
) -> Result<Validated, Error> {
validate_resolving_models(
input,
Some(ModelResolutionTransform::new(Arc::clone(catalog))),
)
}
/// Parse, transform, and validate, resolving models against the ready
@ -41,45 +48,60 @@ pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
/// provider-readiness selection failures.
pub fn validate_with_ready_providers(
input: ValidateInput,
catalog: &Arc<Catalog>,
ready_providers: &[ProviderId],
) -> Result<Validated, Error> {
validate_with_eligible_providers(input, ready_providers, true)
validate_resolving_models(
input,
Some(
ModelResolutionTransform::for_eligible(
Arc::clone(catalog),
ready_providers.iter().cloned().collect(),
)
.with_catalog_fallback(true),
),
)
}
fn validate_with_eligible_providers(
/// The workflow's own default provider is only known once the workflow is
/// resolved, so callers hand in a partially built transform and it is
/// completed here.
fn validate_resolving_models(
input: ValidateInput,
eligible_providers: &[ProviderId],
catalog_fallback: bool,
model_resolution: Option<ModelResolutionTransform>,
) -> Result<Validated, Error> {
let ValidateInput {
workflow,
settings,
vars,
cwd,
custom_transforms,
} = input;
let resolved = resolve_workflow(ResolveWorkflowInput {
workflow: input.workflow,
settings: input.settings,
cwd: input.cwd,
workflow,
settings,
cwd,
})
.map_err(|err| Error::Parse(err.to_string()))?;
let model_resolution = model_resolution.map(|resolution| {
resolution.with_default_provider(configured_default_provider(&resolved.settings))
});
preprocess_and_validate(
&resolved.raw_source,
resolved
.dot_path
.as_ref()
.map(|path| path.display().to_string()),
resolved.current_dir,
resolved.file_resolver,
input.custom_transforms,
template_context(Some(&resolved.settings), input.vars),
resolved.goal_override.as_deref(),
RenderMode::Structural,
resolved
.settings
.run
.model
.provider
.as_deref()
.filter(|provider| !provider.is_empty())
.map(fabro_model::ProviderId::new),
eligible_providers,
catalog_fallback,
&input.catalog,
&TransformOptions {
current_dir: resolved.current_dir,
file_resolver: resolved.file_resolver,
template_context: template_context(Some(&resolved.settings), vars),
source_name: resolved
.dot_path
.as_ref()
.map(|path| path.display().to_string()),
render_mode: RenderMode::Structural,
custom_transforms,
model_resolution,
},
)
}

107
lib/components/fabro-workflow/src/pipeline/transform.rs
View file

@ -3,8 +3,8 @@ use std::sync::Arc;
use super::types::{Parsed, TransformOptions, Transformed};
use crate::error::Error;
use crate::transforms::{
FileInliningTransform, ImportTransform, ModelResolutionTransform,
StylesheetApplicationTransform, TemplateTransform, Transform,
FileInliningTransform, ImportTransform, StylesheetApplicationTransform, TemplateTransform,
Transform,
};
/// TRANSFORM phase: apply built-in and custom transforms to a parsed graph.
@ -63,13 +63,10 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result<Transform
.apply_with_diagnostics(graph)?;
diagnostics.extend(transform_diagnostics);
let graph = StylesheetApplicationTransform.apply(graph)?;
let graph = ModelResolutionTransform::for_eligible(
Arc::clone(&options.catalog),
options.eligible_providers.clone(),
)
.with_default_provider(options.default_provider.clone())
.with_catalog_fallback(options.catalog_fallback)
.apply(graph)?;
let graph = match &options.model_resolution {
Some(model_resolution) => model_resolution.apply(graph)?,
None => graph,
};
// Custom transforms
let graph = options
@ -98,6 +95,7 @@ mod tests {
use crate::file_resolver::FilesystemFileResolver;
use crate::pipeline::parse::parse;
use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE};
use crate::transforms::ModelResolutionTransform;
fn write_file(path: &Path, contents: &str) {
if let Some(parent) = path.parent() {
@ -112,16 +110,13 @@ mod tests {
fn transform_options() -> TransformOptions {
TransformOptions {
current_dir: None,
file_resolver: None,
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
catalog: test_catalog(),
default_provider: None,
eligible_providers: Catalog::builtin().all_provider_ids(),
catalog_fallback: false,
current_dir: None,
file_resolver: None,
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
model_resolution: Some(ModelResolutionTransform::new(test_catalog())),
}
}
@ -177,16 +172,9 @@ mod tests {
)
.unwrap();
let transformed = transform(parsed, &TransformOptions {
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
catalog: test_catalog(),
default_provider: None,
eligible_providers: Catalog::builtin().all_provider_ids(),
catalog_fallback: false,
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
..transform_options()
})
.unwrap();
@ -227,21 +215,15 @@ mod tests {
)
.unwrap();
let transformed = transform(parsed, &TransformOptions {
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from(
[(
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([
(
"task".to_string(),
toml::Value::String("Launch".to_string()),
)],
)),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
catalog: test_catalog(),
default_provider: None,
eligible_providers: Catalog::builtin().all_provider_ids(),
catalog_fallback: false,
),
])),
..transform_options()
})
.unwrap();
@ -349,6 +331,33 @@ mod tests {
);
}
#[test]
fn structural_transform_preserves_catalog_owned_model_selection() {
let dot = r#"digraph Test {
graph [goal="Test"]
start [shape=Mdiamond]
work [prompt="Do work", model="private-model", provider="server-only"]
exit [shape=Msquare]
start -> work -> exit
}"#;
let parsed = parse(dot).unwrap();
let transformed = transform(parsed, &TransformOptions {
model_resolution: None,
..transform_options()
})
.unwrap();
let work = &transformed.graph.nodes["work"];
assert_eq!(
work.attrs.get("model").and_then(AttrValue::as_str),
Some("private-model")
);
assert_eq!(
work.attrs.get("provider").and_then(AttrValue::as_str),
Some("server-only")
);
}
#[test]
fn transform_reports_goal_self_reference_once_across_passes() {
// FileInlining renders the goal for prompt context, but TemplateTransform
@ -365,16 +374,10 @@ mod tests {
)
.unwrap();
let transformed = transform(parsed, &TransformOptions {
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Structural,
custom_transforms: vec![],
catalog: test_catalog(),
default_provider: None,
eligible_providers: Catalog::builtin().all_provider_ids(),
catalog_fallback: false,
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
render_mode: crate::operations::RenderMode::Structural,
..transform_options()
})
.unwrap();

25
lib/components/fabro-workflow/src/pipeline/types.rs
View file

@ -1,4 +1,4 @@
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::path::{Path, PathBuf};
use std::sync::Arc;
@ -28,7 +28,7 @@ use crate::runtime_store::RunStoreHandle;
use crate::services::{EngineServices, FabroRunToolServices, RunServices};
use crate::stage_execution::StageExecutionSeed;
use crate::steering_hub::SteeringHub;
use crate::transforms::{RenderMode, Transform};
use crate::transforms::{ModelResolutionTransform, RenderMode, Transform};
use crate::workflow_bundle::WorkflowBundle;
/// Output of the PARSE phase.
@ -359,18 +359,15 @@ pub struct Finalized {
/// Options for the TRANSFORM phase.
pub struct TransformOptions {
pub current_dir: Option<PathBuf>,
pub file_resolver: Option<Arc<dyn FileResolver>>,
pub template_context: TemplateContext,
pub source_name: Option<String>,
pub render_mode: RenderMode,
pub custom_transforms: Vec<Box<dyn Transform>>,
pub catalog: Arc<fabro_model::Catalog>,
pub default_provider: Option<ProviderId>,
pub eligible_providers: HashSet<ProviderId>,
/// Fall back to the full catalog when the eligible providers cannot
/// supply a requested model, instead of erroring.
pub catalog_fallback: bool,
pub current_dir: Option<PathBuf>,
pub file_resolver: Option<Arc<dyn FileResolver>>,
pub template_context: TemplateContext,
pub source_name: Option<String>,
pub render_mode: RenderMode,
pub custom_transforms: Vec<Box<dyn Transform>>,
/// Catalog-backed model resolution to perform. `None` preserves authored
/// model and provider selectors for catalog-free structural validation.
pub model_resolution: Option<ModelResolutionTransform>,
}
/// Options for the FINALIZE phase.

41
lib/components/fabro-workflow/src/pipeline/validate.rs
View file

@ -5,11 +5,15 @@ use super::types::{Transformed, Validated};
/// VALIDATE phase: run lint rules against the transformed graph.
///
/// Catalog-backed rules (model and provider availability) run only when
/// `catalog` is `Some`. Offline callers pass `None` so a workflow naming a
/// server-owned model is left for the server to judge.
///
/// **Infallible.** Always returns `Validated` with diagnostics. Caller decides
/// whether to fail via `validated.raise_on_errors()`.
pub fn validate(
transformed: Transformed,
catalog: &Catalog,
catalog: Option<&Catalog>,
extra_rules: &[&dyn LintRule],
) -> Validated {
let Transformed {
@ -17,44 +21,33 @@ pub fn validate(
source,
mut diagnostics,
} = transformed;
diagnostics.extend(fabro_validate::validate_with_catalog(
&graph,
catalog,
extra_rules,
));
diagnostics.extend(match catalog {
Some(catalog) => fabro_validate::validate_with_catalog(&graph, catalog, extra_rules),
None => fabro_validate::validate(&graph, extra_rules),
});
Validated::new(graph, source, diagnostics)
}
#[cfg(test)]
mod tests {
use fabro_model::Catalog;
use super::*;
use crate::pipeline::parse::parse;
use crate::pipeline::transform;
use crate::pipeline::types::TransformOptions;
fn test_catalog() -> std::sync::Arc<Catalog> {
std::sync::Arc::new(Catalog::from_builtin().unwrap())
}
fn run_pipeline(dot: &str) -> Validated {
let catalog = test_catalog();
let parsed = parse(dot).unwrap();
let transformed = transform::transform(parsed, &TransformOptions {
current_dir: None,
file_resolver: None,
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
catalog: std::sync::Arc::clone(&catalog),
default_provider: None,
eligible_providers: catalog.all_provider_ids(),
catalog_fallback: false,
current_dir: None,
file_resolver: None,
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
model_resolution: None,
})
.unwrap();
validate(transformed, catalog.as_ref(), &[])
validate(transformed, None, &[])
}
#[test]

7
lib/components/fabro-workflow/src/transforms/model_resolution.rs
View file

@ -52,6 +52,13 @@ impl ModelResolutionTransform {
self
}
/// The catalog this transform resolves against, so callers can run the
/// matching catalog-backed lint rules.
#[must_use]
pub fn catalog(&self) -> &Catalog {
&self.catalog
}
fn resolve_model(
&self,
model: &str,

22
lib/components/fabro-workflow/tests/it/integration.rs
View file

@ -4857,6 +4857,7 @@ async fn manager_loop_child_workflow_e2e() {
#[tokio::test]
async fn import_e2e_through_engine() {
use fabro_workflow::pipeline::{TransformOptions, transform, validate};
use fabro_workflow::transforms::ModelResolutionTransform;
let dir = tempfile::tempdir().unwrap();
let catalog = std::sync::Arc::new(
@ -4900,21 +4901,20 @@ async fn import_e2e_through_engine() {
)
.expect("parse should succeed");
let transformed = transform(parsed, &TransformOptions {
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(std::sync::Arc::new(
current_dir: Some(dir.path().to_path_buf()),
file_resolver: Some(std::sync::Arc::new(
fabro_workflow::file_resolver::FilesystemFileResolver::new(None),
)),
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: fabro_workflow::operations::RenderMode::Strict,
custom_transforms: vec![],
catalog: std::sync::Arc::clone(&catalog),
default_provider: None,
eligible_providers: catalog.all_provider_ids(),
catalog_fallback: false,
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: fabro_workflow::operations::RenderMode::Strict,
custom_transforms: vec![],
model_resolution: Some(ModelResolutionTransform::new(std::sync::Arc::clone(
&catalog,
))),
})
.unwrap();
let validated = validate(transformed, catalog.as_ref(), &[]);
let validated = validate(transformed, Some(catalog.as_ref()), &[]);
validated
.raise_on_errors()
.expect("validation should pass after imports expand");

10
test/server-model.fabro Normal file
View file

@ -0,0 +1,10 @@
digraph ServerModel {
graph [goal="Use a server-owned model"]
start [shape=Mdiamond, label="Start"]
exit [shape=Msquare, label="Exit"]
work [label="Work", prompt="Do work", model="private-model", provider="server-only"]
start -> work -> exit
}