scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-10-10 03:30:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

fix(cli): keep offline validation catalog-free

Browse source
This commit is contained in:
Bryan Helmkamp 2026-07-26 09:25:47 -04:00
parent 2bcf94fed8
commit 0b24649e76
No known key found for this signature in database
20 changed files with 449 additions and 216 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() {

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

@ -101,6 +101,52 @@ 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 workflow_path = context.temp_dir.join("server-model.fabro");
context.write_temp(
"server-model.fabro",
r#"digraph ServerModel {
graph [goal="Use a server-owned model"]
start [shape=Mdiamond]
work [prompt="Do work", model="private-model", provider="server-only"]
exit [shape=Msquare]
start -> work -> exit
}"#,
);
let output = context
.create_cmd()
.args([
"--server",
&format!("{}/api/v1", server.base_url()),
"--dry-run",
workflow_path.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!();

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

@ -69,6 +69,39 @@ fn simple_does_not_connect_to_configured_server() {
);
}
#[test]
#[expect(
clippy::disallowed_methods,
reason = "sync CLI test writes one workflow fixture before spawning the subprocess"
)]
fn server_owned_provider_is_not_rejected_by_offline_validation() {
let cli = LightweightCli::new();
let workflow = cli.home().join("server-model.fabro");
std::fs::write(
&workflow,
r#"digraph ServerModel {
graph [goal="Use a server-owned model"]
start [shape=Mdiamond]
work [prompt="Do work", model="private-model", provider="server-only"]
exit [shape=Msquare]
start -> work -> exit
}"#,
)
.expect("workflow fixture should be written");
let mut cmd = cli.command();
cmd.env("FABRO_SERVER", "http://127.0.0.1:9")
.arg("validate")
.arg(&workflow);
let output = cmd.output().expect("validate should execute");
assert!(
output.status.success(),
"offline validation should leave provider availability to the server\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
);
}
#[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)
}

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

@ -12,21 +12,34 @@ use crate::run_manifest;
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,
)
}
pub fn validate_manifest_with_catalog(
manifest_run_defaults: &RunLayer,
manifest: &types::RunManifest,
catalog: &Arc<Catalog>,
) -> Result<types::ValidateResponse> {
let prepared = run_manifest::prepare_manifest_with_environment_defaults(
manifest_run_defaults,
&fabro_environment::seeded_catalog_layer(),
&HashMap::new(),
manifest,
)?;
let validated =
run_manifest::validate_prepared_manifest(&prepared, catalog).map_err(anyhow::Error::new)?;
Ok(run_manifest::validate_response(&prepared, &validated))
}
pub fn validate_manifest_with_environment_defaults(
manifest_run_defaults: &RunLayer,
manifest_environment_defaults: &MergeMap<EnvironmentLayer>,
manifest: &types::RunManifest,
catalog: Arc<Catalog>,
) -> Result<types::ValidateResponse> {
let prepared = run_manifest::prepare_manifest_with_environment_defaults(
manifest_run_defaults,
@ -34,8 +47,8 @@ pub fn validate_manifest_with_environment_defaults(
&HashMap::new(),
manifest,
)?;
let validated =
run_manifest::validate_prepared_manifest(&prepared, catalog).map_err(anyhow::Error::new)?;
let validated = run_manifest::validate_prepared_manifest_structural(&prepared)
.map_err(anyhow::Error::new)?;
Ok(run_manifest::validate_response(&prepared, &validated))
}

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)),

42
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,17 +65,19 @@ 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 {
source: dot.to_string(),
base_dir: None,
let mut validated = validate_with_catalog(
ValidateInput {
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(),
},
settings: WorkflowSettings::default(),
vars: std::collections::HashMap::new(),
cwd: cwd.clone(),
custom_transforms: Vec::new(),
catalog: Arc::clone(&services.run.catalog),
})?;
&services.run.catalog,
)?;
validated.promote_template_undefined_variables_to_errors();
validated.raise_on_errors()?;
let (graph, _, _) = validated.into_parts();
@ -113,14 +115,16 @@ 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),
})?;
let mut validated = validate_with_catalog(
ValidateInput {
workflow,
settings: WorkflowSettings::default(),
vars: std::collections::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();

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

@ -24,7 +24,9 @@ use crate::error::Error;
use crate::event::{Event, append_event, to_run_event_at};
use crate::file_resolver::FileResolver;
use crate::pipeline::types::PersistOptions;
use crate::pipeline::{self, Persisted, TransformOptions, Validated};
use crate::pipeline::{
self, ModelResolutionOptions, Persisted, TransformOptions, Transformed, Validated,
};
use crate::records::RunSpec;
use crate::run_lookup::default_scratch_base;
use crate::run_materialization::materialize_run;
@ -340,6 +342,69 @@ pub(super) fn preprocess_and_validate(
catalog_fallback: bool,
catalog: &Arc<Catalog>,
) -> Result<Validated, Error> {
let model_resolution = ModelResolutionOptions {
catalog: Arc::clone(catalog),
default_provider,
eligible_providers: eligible_providers.iter().cloned().collect(),
catalog_fallback,
};
let transformed = preprocess(
dot_source,
source_name,
current_dir,
file_resolver,
custom_transforms,
template_context,
goal_override,
render_mode,
Some(model_resolution),
)?;
Ok(pipeline::validate_with_catalog(
transformed,
catalog.as_ref(),
&[],
))
}
pub(super) fn preprocess_and_validate_structural(
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,
) -> Result<Validated, Error> {
let transformed = preprocess(
dot_source,
source_name,
current_dir,
file_resolver,
custom_transforms,
template_context,
goal_override,
render_mode,
None,
)?;
Ok(pipeline::validate(transformed, &[]))
}
#[expect(
clippy::too_many_arguments,
reason = "pipeline stages have distinct source, rendering, and model-resolution inputs"
)]
fn preprocess(
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,
model_resolution: Option<ModelResolutionOptions>,
) -> Result<Transformed, Error> {
let mut parsed = pipeline::parse(dot_source)?;
apply_goal_override(&mut parsed.graph, goal_override);
@ -350,12 +415,9 @@ pub(super) fn preprocess_and_validate(
source_name,
render_mode,
custom_transforms,
catalog: Arc::clone(catalog),
default_provider,
eligible_providers: eligible_providers.iter().cloned().collect(),
catalog_fallback,
model_resolution,
})?;
Ok(pipeline::validate(transformed, catalog.as_ref(), &[]))
Ok(transformed)
}
pub(super) fn template_context(
@ -462,7 +524,7 @@ 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::workflow_bundle::BundledWorkflow;
fn memory_store() -> Arc<Database> {
@ -553,17 +615,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()
}
@ -852,7 +916,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
});
assert!(result.is_err());
@ -901,7 +964,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 +982,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
assert_eq!(
@ -944,7 +1005,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 +1023,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
assert_eq!(
@ -1055,7 +1114,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
});
assert!(result.is_err());
}
@ -1100,7 +1158,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 +1192,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 +1233,6 @@ reasoning = false
vars: HashMap::new(),
cwd: dir.path().to_path_buf(),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
@ -1227,7 +1282,6 @@ reasoning = false
vars: HashMap::new(),
cwd: PathBuf::from("."),
custom_transforms: Vec::new(),
catalog: test_catalog(),
})
.unwrap();
@ -1278,7 +1332,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;

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

@ -5,7 +5,9 @@ use std::sync::Arc;
use fabro_model::{Catalog, ProviderId};
use fabro_types::WorkflowSettings;
use super::create::{preprocess_and_validate, template_context};
use super::create::{
preprocess_and_validate, preprocess_and_validate_structural, template_context,
};
use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow};
use crate::error::Error;
use crate::operations::RenderMode;
@ -20,20 +22,50 @@ 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.
///
/// 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)
let ValidateInput {
workflow,
settings,
vars,
cwd,
custom_transforms,
} = input;
let resolved = resolve_workflow(ResolveWorkflowInput {
workflow,
settings,
cwd,
})
.map_err(|err| Error::Parse(err.to_string()))?;
preprocess_and_validate_structural(
&resolved.raw_source,
resolved
.dot_path
.as_ref()
.map(|path| path.display().to_string()),
resolved.current_dir,
resolved.file_resolver,
custom_transforms,
template_context(Some(&resolved.settings), vars),
resolved.goal_override.as_deref(),
RenderMode::Structural,
)
}
/// Parse, transform, and validate a DOT source string against `catalog`.
pub fn validate_with_catalog(
input: ValidateInput,
catalog: &Arc<Catalog>,
) -> Result<Validated, Error> {
let eligible_providers = catalog.all_provider_ids().into_iter().collect::<Vec<_>>();
validate_with_eligible_providers(input, catalog, &eligible_providers, false)
}
/// Parse, transform, and validate, resolving models against the ready
@ -41,20 +73,29 @@ 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_with_eligible_providers(input, catalog, ready_providers, true)
}
fn validate_with_eligible_providers(
input: ValidateInput,
catalog: &Arc<Catalog>,
eligible_providers: &[ProviderId],
catalog_fallback: bool,
) -> 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()))?;
@ -66,8 +107,8 @@ fn validate_with_eligible_providers(
.map(|path| path.display().to_string()),
resolved.current_dir,
resolved.file_resolver,
input.custom_transforms,
template_context(Some(&resolved.settings), input.vars),
custom_transforms,
template_context(Some(&resolved.settings), vars),
resolved.goal_override.as_deref(),
RenderMode::Structural,
resolved
@ -80,6 +121,6 @@ fn validate_with_eligible_providers(
.map(fabro_model::ProviderId::new),
eligible_providers,
catalog_fallback,
&input.catalog,
catalog,
)
}

8
lib/components/fabro-workflow/src/pipeline/mod.rs
View file

@ -22,8 +22,8 @@ pub use pull_request::{
};
pub use transform::transform;
pub use types::{
Concluded, Executed, FinalizeOptions, Finalized, InitOptions, Initialized, LlmSpec, Parsed,
Persisted, PullRequestOptions, ResumeState, SandboxEnvSpec, TEMPLATE_UNDEFINED_VARIABLE_RULE,
TransformOptions, Transformed, Validated,
Concluded, Executed, FinalizeOptions, Finalized, InitOptions, Initialized, LlmSpec,
ModelResolutionOptions, Parsed, Persisted, PullRequestOptions, ResumeState, SandboxEnvSpec,
TEMPLATE_UNDEFINED_VARIABLE_RULE, TransformOptions, Transformed, Validated,
};
pub use validate::validate;
pub use validate::{validate, validate_with_catalog};

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

@ -63,13 +63,17 @@ 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 = if let Some(model_resolution) = &options.model_resolution {
ModelResolutionTransform::for_eligible(
Arc::clone(&model_resolution.catalog),
model_resolution.eligible_providers.clone(),
)
.with_default_provider(model_resolution.default_provider.clone())
.with_catalog_fallback(model_resolution.catalog_fallback)
.apply(graph)?
} else {
graph
};
// Custom transforms
let graph = options
@ -97,7 +101,9 @@ mod tests {
use super::*;
use crate::file_resolver::FilesystemFileResolver;
use crate::pipeline::parse::parse;
use crate::pipeline::types::{GOAL_SELF_REFERENCE_RULE, TEMPLATE_UNDEFINED_VARIABLE_RULE};
use crate::pipeline::types::{
GOAL_SELF_REFERENCE_RULE, ModelResolutionOptions, TEMPLATE_UNDEFINED_VARIABLE_RULE,
};
fn write_file(path: &Path, contents: &str) {
if let Some(parent) = path.parent() {
@ -112,16 +118,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(ModelResolutionOptions::new(test_catalog())),
}
}
@ -177,16 +180,13 @@ 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))),
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
model_resolution: Some(ModelResolutionOptions::new(test_catalog())),
})
.unwrap();
@ -227,21 +227,18 @@ 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,
),
])),
source_name: None,
render_mode: crate::operations::RenderMode::Strict,
custom_transforms: vec![],
model_resolution: Some(ModelResolutionOptions::new(test_catalog())),
})
.unwrap();
@ -349,6 +346,38 @@ 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 {
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();
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 +394,13 @@ 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))),
template_context: fabro_template::TemplateContext::new(),
source_name: None,
render_mode: crate::operations::RenderMode::Structural,
custom_transforms: vec![],
model_resolution: Some(ModelResolutionOptions::new(test_catalog())),
})
.unwrap();

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

@ -359,13 +359,20 @@ 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 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<ModelResolutionOptions>,
}
/// Catalog-backed model resolution options for the TRANSFORM phase.
pub struct ModelResolutionOptions {
pub catalog: Arc<Catalog>,
pub default_provider: Option<ProviderId>,
pub eligible_providers: HashSet<ProviderId>,
/// Fall back to the full catalog when the eligible providers cannot
@ -373,6 +380,19 @@ pub struct TransformOptions {
pub catalog_fallback: bool,
}
impl ModelResolutionOptions {
#[must_use]
pub fn new(catalog: Arc<Catalog>) -> Self {
let eligible_providers = catalog.all_provider_ids();
Self {
catalog,
default_provider: None,
eligible_providers,
catalog_fallback: false,
}
}
}
/// Options for the FINALIZE phase.
pub struct FinalizeOptions {
pub run_dir: PathBuf,

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

@ -1,15 +1,28 @@
use fabro_model::Catalog;
use fabro_validate::LintRule;
use super::types::{Transformed, Validated};
/// VALIDATE phase: run lint rules against the transformed graph.
/// VALIDATE phase: run catalog-free lint rules against the transformed graph.
///
/// **Infallible.** Always returns `Validated` with diagnostics. Caller decides
/// whether to fail via `validated.raise_on_errors()`.
pub fn validate(
pub fn validate(transformed: Transformed, extra_rules: &[&dyn LintRule]) -> Validated {
let Transformed {
graph,
source,
mut diagnostics,
} = transformed;
diagnostics.extend(fabro_validate::validate(&graph, extra_rules));
Validated::new(graph, source, diagnostics)
}
/// VALIDATE phase: run catalog-free and catalog-backed lint rules.
///
/// **Infallible.** Always returns `Validated` with diagnostics. Caller decides
/// whether to fail via `validated.raise_on_errors()`.
pub fn validate_with_catalog(
transformed: Transformed,
catalog: &Catalog,
catalog: &fabro_model::Catalog,
extra_rules: &[&dyn LintRule],
) -> Validated {
let Transformed {
@ -27,34 +40,24 @@ pub fn validate(
#[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, &[])
}
#[test]

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

@ -4853,7 +4853,9 @@ 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::pipeline::{
ModelResolutionOptions, TransformOptions, transform, validate_with_catalog,
};
let dir = tempfile::tempdir().unwrap();
let catalog = std::sync::Arc::new(
@ -4897,21 +4899,18 @@ 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(ModelResolutionOptions::new(std::sync::Arc::clone(&catalog))),
})
.unwrap();
let validated = validate(transformed, catalog.as_ref(), &[]);
let validated = validate_with_catalog(transformed, catalog.as_ref(), &[]);
validated
.raise_on_errors()
.expect("validation should pass after imports expand");