diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 259e3d2b5..65cc6d36e 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -34,14 +34,15 @@ use fabro_types::{ use fabro_util::check_report::{CheckDetail, CheckReport, CheckResult, CheckSection, CheckStatus}; use fabro_validate::Severity; use fabro_workflow::Error as WorkflowError; -use fabro_workflow::operations::{CreateRunInput, ValidateInput, WorkflowInput, validate}; +use fabro_workflow::operations::{ + CreateRunInput, ValidateInput, WorkflowInput, validate, validate_with_ready_providers, +}; use fabro_workflow::pipeline::Validated; -use fabro_workflow::run_materialization::materialize_run; +use fabro_workflow::run_materialization::materialize_run_with_ready_providers; use fabro_workflow::workflow_bundle::{BundledWorkflow, ParsedWorkflowConfig, WorkflowBundle}; use futures_util::stream::{self, StreamExt}; use tokio::process::Command; use tokio::time; -use tracing::warn; use crate::interp::process_env_var; use crate::server::AppState; @@ -196,14 +197,34 @@ pub(crate) fn validate_prepared_manifest_with_vars( catalog: Arc, vars: HashMap, ) -> Result { - validate(ValidateInput { + validate(manifest_validate_input(prepared, catalog, vars)) +} + +pub(crate) fn validate_prepared_manifest_for_preflight( + prepared: &PreparedManifest, + catalog: Arc, + vars: HashMap, + ready_providers: &[ProviderId], +) -> Result { + validate_with_ready_providers( + manifest_validate_input(prepared, catalog, vars), + ready_providers, + ) +} + +fn manifest_validate_input( + prepared: &PreparedManifest, + catalog: Arc, + vars: HashMap, +) -> ValidateInput { + ValidateInput { workflow: WorkflowInput::Bundled(prepared.workflow_input.clone()), settings: prepared.settings.clone(), vars, cwd: prepared.cwd.clone(), custom_transforms: Vec::new(), catalog, - }) + } } pub(crate) fn create_run_input( @@ -238,8 +259,10 @@ pub(crate) async fn run_preflight( state: &AppState, prepared: &PreparedManifest, validated: &Validated, + llm_result: Result, ) -> Result<(types::PreflightResponse, bool)> { - let (report, checks_ok) = build_preflight_report(state, prepared, validated).await?; + let (report, checks_ok) = + build_preflight_report(state, prepared, validated, llm_result).await?; let preflight_ok = !validated.has_errors() && checks_ok; Ok(( preflight_response( @@ -458,6 +481,7 @@ async fn build_preflight_report( state: &AppState, prepared: &PreparedManifest, validated: &Validated, + llm_result: Result, ) -> Result<(CheckReport, bool)> { let graph = validated.graph(); let mut checks = base_preflight_checks(prepared, graph); @@ -475,22 +499,23 @@ async fn build_preflight_report( } let catalog = state.catalog(); - let llm_result = state.resolve_llm_client().await; - if let Err(err) = &llm_result { - warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); - } - // Preflight is credential-independent static validation. Materialize - // against every enabled catalog provider so aliases and defaults can be - // inspected even when the corresponding adapter is not currently ready; - // `run_llm_check` below reports actual credential/registration readiness. - let enabled_providers = catalog.all_provider_ids().into_iter().collect::>(); - let materialized = materialize_run( + let ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + let materialized = materialize_run_with_ready_providers( prepared.settings.clone(), graph, catalog.as_ref(), - &enabled_providers, + &ready_providers, )?; let resolved_run = materialized.run; + let (Some(run_model), Some(run_provider)) = ( + resolved_run.model.name.as_deref(), + resolved_run.model.provider.as_deref(), + ) else { + bail!("materialized run is missing a resolved model or provider"); + }; let server_settings = state.server_settings(); let github_integration = &server_settings.server.integrations.github; let sandbox_provider = effective_sandbox_provider(&resolved_run); @@ -553,7 +578,8 @@ async fn build_preflight_report( let llm_ok = run_llm_check( &mut checks, graph, - &resolved_run, + run_model, + run_provider, catalog.as_ref(), llm_result, ) @@ -1029,17 +1055,11 @@ struct PendingModelProbe { async fn run_llm_check( checks: &mut Vec, graph: &Graph, - settings: &RunNamespace, + model: &str, + default_provider: &str, catalog: &Catalog, llm_result: Result, ) -> bool { - let model = settings - .model - .name - .as_deref() - .unwrap_or_else(|| catalog.default_for_configured_ids(&[]).id.as_str()); - let provider = settings.model.provider.as_deref(); - let default_provider = provider.unwrap_or("anthropic"); let mut model_providers = std::collections::BTreeSet::new(); let mut has_llm_nodes = false; @@ -1050,24 +1070,7 @@ async fn run_llm_check( has_llm_nodes = true; let node_model = node.model().unwrap_or(model); let node_provider = node.provider().unwrap_or(default_provider); - let resolved = if node.provider().is_some() { - catalog.get_on_provider(&ProviderId::new(node_provider), node_model) - } else { - catalog - .select(node_model, None, &catalog.all_provider_ids()) - .ok() - }; - let (resolved_model, resolved_provider) = if let Some(info) = resolved { - (info.id.to_string(), info.provider.to_string()) - } else { - (node_model.to_string(), node_provider.to_string()) - }; - let final_provider = if node.provider().is_some() { - node_provider.to_string() - } else { - resolved_provider - }; - model_providers.insert((resolved_model, final_provider)); + model_providers.insert((node_model.to_string(), node_provider.to_string())); } if !has_llm_nodes { @@ -1375,6 +1378,7 @@ fn report_to_api(report: &CheckReport) -> types::PreflightCheckReport { mod tests { use fabro_model::ProviderId; use fabro_model::catalog::LlmCatalogSettings; + use fabro_workflow::run_materialization::materialize_run; use super::*; @@ -1470,6 +1474,100 @@ mod tests { Arc::new(Catalog::from_builtin().unwrap()) } + fn openai_compatible_completion(model: &str) -> serde_json::Value { + serde_json::json!({ + "id": "chatcmpl_preflight", + "object": "chat.completion", + "created": 1_700_000_000, + "model": model, + "choices": [{ + "index": 0, + "message": {"role": "assistant", "content": "OK"}, + "finish_reason": "stop" + }], + "usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2} + }) + } + + fn ready_kimi_and_openrouter_state( + server: &httpmock::MockServer, + ) -> Arc { + let kimi_url = server.url("/kimi/v1"); + let openrouter_url = server.url("/openrouter/v1"); + let llm_catalog_settings: LlmCatalogSettings = toml::from_str(&format!( + r#" +[providers.kimi] +base_url = "{kimi_url}" + +[providers.openrouter] +base_url = "{openrouter_url}" +enabled = true +"# + )) + .expect("catalog overrides should parse"); + + crate::test_support::TestAppStateBuilder::new() + .llm_catalog_settings(llm_catalog_settings) + .vault_entries([ + (EnvVars::KIMI_API_KEY, "test-kimi-key"), + (EnvVars::OPENROUTER_API_KEY, "test-openrouter-key"), + ]) + .build() + } + + async fn preflight_for_model( + state: &Arc, + model: &str, + ) -> (types::PreflightResponse, bool) { + let llm_result = state.resolve_llm_client().await; + let mut ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + ready_providers.sort(); + assert_eq!(ready_providers, vec![ + ProviderId::new("kimi"), + ProviderId::new("openrouter") + ]); + + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().source = format!( + r#" +digraph Demo {{ + start [shape=Mdiamond] + exit [shape=Msquare] + work [prompt="Do work", model="{model}"] + start -> work -> exit +}} +"# + ); + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + let validated = validate_prepared_manifest_for_preflight( + &prepared, + state.catalog(), + HashMap::new(), + &ready_providers, + ) + .unwrap(); + + run_preflight(state.as_ref(), &prepared, &validated, llm_result) + .await + .unwrap() + } + + async fn resolve_and_run_preflight( + state: &AppState, + prepared: &PreparedManifest, + validated: &Validated, + ) -> Result<(types::PreflightResponse, bool)> { + let llm_result = state.resolve_llm_client().await; + run_preflight(state, prepared, validated, llm_result).await + } + fn manifest_workflow() -> types::ManifestWorkflow { types::ManifestWorkflow { config: None, @@ -2099,7 +2197,7 @@ name = "Control Plane" assert!(validated.has_errors()); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); @@ -2143,7 +2241,7 @@ issues = "read" let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); assert!(!validated.has_errors()); - let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); @@ -2194,7 +2292,7 @@ id = "local" assert!(!validated.has_errors()); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); @@ -2301,7 +2399,7 @@ id = "daytona" .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, _ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, _ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); @@ -2369,7 +2467,7 @@ digraph Demo { .unwrap(); let validated = validate_prepared_manifest(&prepared, test_catalog()).unwrap(); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, ok) = resolve_and_run_preflight(state.as_ref(), &prepared, &validated) .await .unwrap(); @@ -2390,6 +2488,80 @@ digraph Demo { assert!(response_mock.calls_async().await >= 1); } + #[tokio::test] + async fn preflight_uses_ready_providers_for_known_shared_alias() { + let server = httpmock::MockServer::start_async().await; + let openrouter_probe = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/openrouter/v1/chat/completions") + .header("authorization", "Bearer test-openrouter-key") + .json_body_includes(r#"{"model":"anthropic/claude-fable-5"}"#); + then.status(200) + .header("content-type", "application/json") + .json_body(openai_compatible_completion("anthropic/claude-fable-5")); + }) + .await; + let state = ready_kimi_and_openrouter_state(&server); + + let (response, _ok) = preflight_for_model(&state, "claude-fable").await; + + let llm_check = response.checks.sections[0] + .checks + .iter() + .find(|check| check.name == "LLM" && check.summary == "claude-fable-5") + .expect("preflight should include Claude Fable"); + assert_eq!( + llm_check + .details + .iter() + .map(|detail| detail.text.as_str()) + .find(|detail| detail.starts_with("Provider: ")), + Some("Provider: openrouter") + ); + assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Pass); + openrouter_probe.assert_async().await; + } + + #[tokio::test] + async fn preflight_uses_ready_providers_for_unknown_unqualified_model() { + let server = httpmock::MockServer::start_async().await; + let kimi_probe = server + .mock_async(|when, then| { + when.method(httpmock::Method::POST) + .path("/kimi/v1/chat/completions") + .header("authorization", "Bearer test-kimi-key") + .json_body_includes(r#"{"model":"provider-private-preview"}"#); + then.status(200) + .header("content-type", "application/json") + .json_body(openai_compatible_completion("provider-private-preview")); + }) + .await; + let state = ready_kimi_and_openrouter_state(&server); + + let (response, _ok) = preflight_for_model(&state, "provider-private-preview").await; + + assert!(response.workflow.diagnostics.iter().any(|diagnostic| { + diagnostic.rule == "node_model_known" + && diagnostic.message.contains("provider-private-preview") + })); + let llm_check = response.checks.sections[0] + .checks + .iter() + .find(|check| check.name == "LLM" && check.summary == "provider-private-preview") + .expect("preflight should include the unknown passthrough model"); + assert_eq!( + llm_check + .details + .iter() + .map(|detail| detail.text.as_str()) + .find(|detail| detail.starts_with("Provider: ")), + Some("Provider: kimi") + ); + assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Pass); + kimi_probe.assert_async().await; + } + #[test] fn static_validation_rejects_unknown_llm_provider() { let mut manifest = minimal_manifest(); @@ -2466,9 +2638,21 @@ digraph Demo { &manifest, ) .unwrap(); - let validated = validate_prepared_manifest(&prepared, state.catalog()).unwrap(); + let llm_result = state.resolve_llm_client().await; + let ready_providers = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + assert!(ready_providers.is_empty()); + let validated = validate_prepared_manifest_for_preflight( + &prepared, + state.catalog(), + HashMap::new(), + &ready_providers, + ) + .unwrap(); - let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) + let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated, llm_result) .await .unwrap(); diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index fac2dd51a..47b57a462 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -1445,14 +1445,26 @@ impl AppState { self.llm_source.configured_providers(catalog.as_ref()).await } - pub(crate) async fn ready_llm_provider_ids(&self) -> Vec { - match self.resolve_llm_client().await { - Ok(result) => result.provider_ids(), - Err(err) => { - warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); - Vec::new() - } + /// Resolve the LLM client once and derive the ready provider IDs from it, + /// logging a warning when resolution fails. Callers that need both values + /// must use this instead of `ready_llm_provider_ids` so the client is not + /// resolved twice. + pub(crate) async fn resolve_llm_client_with_ready_ids( + &self, + ) -> (anyhow::Result, Vec) { + let llm_result = self.resolve_llm_client().await; + if let Err(err) = &llm_result { + warn!(error = ?err, "Failed to resolve LLM client while checking ready providers"); } + let ready_provider_ids = llm_result + .as_ref() + .map(LlmClientResult::provider_ids) + .unwrap_or_default(); + (llm_result, ready_provider_ids) + } + + pub(crate) async fn ready_llm_provider_ids(&self) -> Vec { + self.resolve_llm_client_with_ready_ids().await.1 } pub(crate) async fn decorate_run_summary(&self, run: fabro_types::Run) -> fabro_types::Run { diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 614f2d719..fdb9c1c3e 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -51,7 +51,6 @@ use crate::run_files::{list_run_commits, list_run_files}; use crate::run_manifest; use crate::run_selector::{ResolveRunError, resolve_run_by_selector}; use crate::run_title_generation::{self, GenerateTitleInput, TitlePromptInput, WorkflowSummary}; -use crate::server_secrets::LlmClientResult; #[cfg(any(test, feature = "test-support"))] use crate::test_support as server_test_support; @@ -591,17 +590,8 @@ pub(crate) async fn create_run_from_manifest( // and ask-fabro-readiness) and the LLM client itself (for the spawned // title-generation task). `ready_llm_provider_ids` would otherwise call // `resolve_llm_client` a second time and discard the client. - let llm_client_for_title = match state.resolve_llm_client().await { - Ok(result) => Some(result), - Err(err) => { - tracing::warn!(error = ?err, "Failed to resolve LLM client while creating run"); - None - } - }; - let ready_provider_ids = llm_client_for_title - .as_ref() - .map(LlmClientResult::provider_ids) - .unwrap_or_default(); + let (llm_result, ready_provider_ids) = state.resolve_llm_client_with_ready_ids().await; + let llm_client_for_title = llm_result.ok(); let run_materialization_provider_ids = { #[cfg(any(test, feature = "test-support"))] { @@ -835,10 +825,12 @@ async fn run_preflight( return ApiError::bad_request(format!("Run config variable interpolation failed: {err}")) .into_response(); } - let mut validated = match run_manifest::validate_prepared_manifest_with_vars( + 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(), vars, + &ready_providers, ) { Ok(validated) => validated, Err(WorkflowError::Parse(_)) => { @@ -847,13 +839,14 @@ async fn run_preflight( Err(err) => return ApiError::bad_request(err.to_string()).into_response(), }; validated.promote_template_undefined_variables_to_errors(); - let response = match run_manifest::run_preflight(&state, &prepared, &validated).await { - Ok((response, _ok)) => response, - Err(err) => { - return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) - .into_response(); - } - }; + let response = + match run_manifest::run_preflight(&state, &prepared, &validated, llm_result).await { + Ok((response, _ok)) => response, + Err(err) => { + return ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + .into_response(); + } + }; (StatusCode::OK, Json(response)).into_response() } diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 144cf5976..2ec40d443 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -312,6 +312,7 @@ fn create_from_source( .filter(|provider| !provider.is_empty()) .map(ProviderId::new), &options.configured_providers, + false, &options.catalog, )?; @@ -336,6 +337,7 @@ pub(super) fn preprocess_and_validate( render_mode: RenderMode, default_provider: Option, eligible_providers: &[ProviderId], + catalog_fallback: bool, catalog: &Arc, ) -> Result { let mut parsed = pipeline::parse(dot_source)?; @@ -351,6 +353,7 @@ pub(super) fn preprocess_and_validate( catalog: Arc::clone(catalog), default_provider, eligible_providers: eligible_providers.iter().cloned().collect(), + catalog_fallback, })?; Ok(pipeline::validate(transformed, catalog.as_ref(), &[])) } @@ -579,6 +582,7 @@ reasoning = false RenderMode::Structural, None, &test_provider_ids(), + false, &test_catalog(), ) .unwrap() @@ -749,6 +753,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), + false, &test_catalog(), ); let Err(err) = result else { @@ -787,6 +792,7 @@ reasoning = false RenderMode::Strict, None, &test_provider_ids(), + false, &test_catalog(), ); let Err(err) = result else { diff --git a/lib/components/fabro-workflow/src/operations/mod.rs b/lib/components/fabro-workflow/src/operations/mod.rs index 4710d97a9..38b7d9aaa 100644 --- a/lib/components/fabro-workflow/src/operations/mod.rs +++ b/lib/components/fabro-workflow/src/operations/mod.rs @@ -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}; +pub use validate::{ValidateInput, validate, validate_with_ready_providers}; pub use crate::pipeline::{LlmSpec, SandboxEnvSpec}; pub use crate::transforms::RenderMode; diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 9c2e3ed8d..ba720b11f 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -622,6 +622,7 @@ fn resolve_start_llm( &eligible, settings.model.name.as_deref(), settings.model.provider.as_deref(), + false, )?; let fallback_chain = resolve_fallback_chain(catalog, &provider_id, &model, &settings.model, &eligible)?; diff --git a/lib/components/fabro-workflow/src/operations/validate.rs b/lib/components/fabro-workflow/src/operations/validate.rs index a95b13920..c2b990f5c 100644 --- a/lib/components/fabro-workflow/src/operations/validate.rs +++ b/lib/components/fabro-workflow/src/operations/validate.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; -use fabro_model::Catalog; +use fabro_model::{Catalog, ProviderId}; use fabro_types::WorkflowSettings; use super::create::{preprocess_and_validate, template_context}; @@ -28,17 +28,35 @@ pub struct ValidateInput { /// 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 { + let eligible_providers = input + .catalog + .all_provider_ids() + .into_iter() + .collect::>(); + validate_with_eligible_providers(input, &eligible_providers, false) +} + +/// Parse, transform, and validate, resolving models against the ready +/// providers first and falling back to the full catalog only for +/// provider-readiness selection failures. +pub fn validate_with_ready_providers( + input: ValidateInput, + ready_providers: &[ProviderId], +) -> Result { + validate_with_eligible_providers(input, ready_providers, true) +} + +fn validate_with_eligible_providers( + input: ValidateInput, + eligible_providers: &[ProviderId], + catalog_fallback: bool, +) -> Result { let resolved = resolve_workflow(ResolveWorkflowInput { workflow: input.workflow, settings: input.settings, cwd: input.cwd, }) .map_err(|err| Error::Parse(err.to_string()))?; - let eligible_providers = input - .catalog - .all_provider_ids() - .into_iter() - .collect::>(); preprocess_and_validate( &resolved.raw_source, @@ -60,7 +78,8 @@ pub fn validate(input: ValidateInput) -> Result { .as_deref() .filter(|provider| !provider.is_empty()) .map(fabro_model::ProviderId::new), - &eligible_providers, + eligible_providers, + catalog_fallback, &input.catalog, ) } diff --git a/lib/components/fabro-workflow/src/pipeline/transform.rs b/lib/components/fabro-workflow/src/pipeline/transform.rs index 6b76380d0..b399d6637 100644 --- a/lib/components/fabro-workflow/src/pipeline/transform.rs +++ b/lib/components/fabro-workflow/src/pipeline/transform.rs @@ -68,6 +68,7 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result, pub default_provider: Option, pub eligible_providers: HashSet, + /// Fall back to the full catalog when the eligible providers cannot + /// supply a requested model, instead of erroring. + pub catalog_fallback: bool, } /// Options for the FINALIZE phase. diff --git a/lib/components/fabro-workflow/src/pipeline/validate.rs b/lib/components/fabro-workflow/src/pipeline/validate.rs index 06120e8ce..f0cd51dbd 100644 --- a/lib/components/fabro-workflow/src/pipeline/validate.rs +++ b/lib/components/fabro-workflow/src/pipeline/validate.rs @@ -51,6 +51,7 @@ mod tests { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), + catalog_fallback: false, }) .unwrap(); validate(transformed, catalog.as_ref(), &[]) diff --git a/lib/components/fabro-workflow/src/run_materialization.rs b/lib/components/fabro-workflow/src/run_materialization.rs index 7e82964ac..d1118b076 100644 --- a/lib/components/fabro-workflow/src/run_materialization.rs +++ b/lib/components/fabro-workflow/src/run_materialization.rs @@ -9,10 +9,32 @@ use fabro_types::settings::run::RunGoal; use crate::error::Error; pub fn materialize_run( - mut settings: WorkflowSettings, + settings: WorkflowSettings, graph: &Graph, catalog: &Catalog, configured_providers: &[ProviderId], +) -> Result { + materialize_run_with_eligible_providers(settings, graph, catalog, configured_providers, false) +} + +/// Materialize while resolving the run model against the ready providers +/// first, falling back to the full catalog only for provider-readiness +/// selection failures. +pub fn materialize_run_with_ready_providers( + settings: WorkflowSettings, + graph: &Graph, + catalog: &Catalog, + ready_providers: &[ProviderId], +) -> Result { + materialize_run_with_eligible_providers(settings, graph, catalog, ready_providers, true) +} + +fn materialize_run_with_eligible_providers( + mut settings: WorkflowSettings, + graph: &Graph, + catalog: &Catalog, + eligible_providers: &[ProviderId], + catalog_fallback: bool, ) -> Result { let configured_model = settings.run.model.name.take(); let configured_provider = settings.run.model.provider.take(); @@ -29,9 +51,14 @@ pub fn materialize_run( let provider = configured_provider.or(graph_provider); let model = configured_model.or(graph_model); - let eligible = configured_providers.iter().cloned().collect::>(); - let (resolved_model, resolved_provider) = - resolve_run_model(catalog, &eligible, model.as_deref(), provider.as_deref())?; + let eligible = eligible_providers.iter().cloned().collect::>(); + let (resolved_model, resolved_provider) = resolve_run_model( + catalog, + &eligible, + model.as_deref(), + provider.as_deref(), + catalog_fallback, + )?; settings.run.model.name = Some(resolved_model); settings.run.model.provider = Some(resolved_provider.into_inner()); @@ -60,10 +87,15 @@ pub(crate) fn resolve_run_model( eligible: &HashSet, model: Option<&str>, provider: Option<&str>, + catalog_fallback: bool, ) -> Result<(String, ProviderId), ModelSelectionError> { let provider = provider .filter(|provider| !provider.is_empty()) .map(ProviderId::new); - let selected = catalog.resolve_selection(model, provider.as_ref(), eligible)?; + let selected = if catalog_fallback { + catalog.resolve_selection_with_catalog_fallback(model, provider.as_ref(), eligible)? + } else { + catalog.resolve_selection(model, provider.as_ref(), eligible)? + }; Ok((selected.model, selected.provider)) } diff --git a/lib/components/fabro-workflow/src/transforms/model_resolution.rs b/lib/components/fabro-workflow/src/transforms/model_resolution.rs index 7eecbcfd1..12f29ac5a 100644 --- a/lib/components/fabro-workflow/src/transforms/model_resolution.rs +++ b/lib/components/fabro-workflow/src/transforms/model_resolution.rs @@ -13,6 +13,7 @@ pub struct ModelResolutionTransform { catalog: Arc, default_provider: Option, eligible_providers: HashSet, + catalog_fallback: bool, } impl ModelResolutionTransform { @@ -23,6 +24,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, + catalog_fallback: false, } } @@ -32,6 +34,7 @@ impl ModelResolutionTransform { catalog, default_provider: None, eligible_providers, + catalog_fallback: false, } } @@ -41,16 +44,29 @@ impl ModelResolutionTransform { self } + /// When enabled, provider-readiness selection failures fall back to the + /// full catalog instead of erroring. + #[must_use] + pub fn with_catalog_fallback(mut self, catalog_fallback: bool) -> Self { + self.catalog_fallback = catalog_fallback; + self + } + fn resolve_model( &self, model: &str, explicit_provider: Option<&ProviderId>, ) -> Result<(String, ProviderId), Error> { - let selected = self.catalog.resolve_selection( - Some(model), - explicit_provider, - &self.eligible_providers, - )?; + let selected = if self.catalog_fallback { + self.catalog.resolve_selection_with_catalog_fallback( + Some(model), + explicit_provider, + &self.eligible_providers, + ) + } else { + self.catalog + .resolve_selection(Some(model), explicit_provider, &self.eligible_providers) + }?; Ok((selected.model, selected.provider)) } } @@ -326,6 +342,50 @@ reasoning = false ); } + #[test] + fn fallback_resolution_keeps_ready_preference_for_unpinned_nodes() { + let overrides: LlmCatalogSettings = toml::from_str( + r" +[providers.openrouter] +enabled = true +", + ) + .unwrap(); + let catalog = Arc::new(Catalog::from_builtin_with_overrides(&overrides).unwrap()); + let mut graph = Graph::new("test"); + let mut portable = Node::new("portable"); + portable.attrs.insert( + "model".to_string(), + AttrValue::String("claude-fable".to_string()), + ); + graph.nodes.insert("portable".to_string(), portable); + let mut pinned = Node::new("pinned"); + pinned.attrs.insert( + "model".to_string(), + AttrValue::String("claude-fable".to_string()), + ); + pinned.attrs.insert( + "provider".to_string(), + AttrValue::String("anthropic".to_string()), + ); + graph.nodes.insert("pinned".to_string(), pinned); + + let graph = ModelResolutionTransform::for_eligible( + Arc::clone(&catalog), + HashSet::from([ProviderId::new("openrouter")]), + ) + .with_catalog_fallback(true) + .apply(graph) + .unwrap(); + + assert_eq!( + graph.nodes["portable"].provider(), + Some("openrouter"), + "the unrelated unavailable pin must not force catalog-wide routing" + ); + assert_eq!(graph.nodes["pinned"].provider(), Some("anthropic")); + } + #[test] fn graph_default_alias_materializes_to_canonical_offering() { let mut graph = Graph::new("test"); diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 5f3f75a55..8e415c81a 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -4908,6 +4908,7 @@ async fn import_e2e_through_engine() { catalog: std::sync::Arc::clone(&catalog), default_provider: None, eligible_providers: catalog.all_provider_ids(), + catalog_fallback: false, }) .unwrap(); let validated = validate(transformed, catalog.as_ref(), &[]); diff --git a/lib/foundation/fabro-model/src/catalog.rs b/lib/foundation/fabro-model/src/catalog.rs index c58035f8d..c3f4de596 100644 --- a/lib/foundation/fabro-model/src/catalog.rs +++ b/lib/foundation/fabro-model/src/catalog.rs @@ -1101,6 +1101,31 @@ impl Catalog { } } + /// Resolve a selection against a preferred provider snapshot, falling back + /// to every provider in the catalog only when the preferred set cannot + /// supply the requested provider or model. + /// + /// This is useful for readiness checks: ready providers remain preferred, + /// while a catalog-only offering can still be selected so the caller can + /// report why its provider is unavailable. Semantic failures such as an + /// unknown provider do not fall back. + pub fn resolve_selection_with_catalog_fallback( + &self, + selector: Option<&str>, + explicit_provider: Option<&ProviderId>, + preferred_providers: &HashSet, + ) -> Result { + match self.resolve_selection(selector, explicit_provider, preferred_providers) { + Ok(selected) => Ok(selected), + Err( + ModelSelectionError::ProviderUnavailable { .. } + | ModelSelectionError::NoEligibleOffering { .. } + | ModelSelectionError::NoDefaultModel { .. }, + ) => self.resolve_selection(selector, explicit_provider, &self.all_provider_ids()), + Err(error) => Err(error), + } + } + #[must_use] pub fn is_model_selector(&self, selector: &str) -> bool { self.candidate_indices(selector).is_some() @@ -4048,6 +4073,30 @@ adapter = "openai_compatible" )); } + #[test] + fn selection_fallback_preserves_ready_preference_per_request() { + let catalog = portable_model_catalog(); + let openai = ProviderId::openai(); + let openrouter = ProviderId::new("openrouter"); + let ready = HashSet::from([openrouter.clone()]); + + let shared = catalog + .resolve_selection_with_catalog_fallback(Some("portable"), None, &ready) + .unwrap(); + assert_eq!(shared.provider, openrouter); + + let pinned = catalog + .resolve_selection_with_catalog_fallback(Some("portable"), Some(&openai), &ready) + .unwrap(); + assert_eq!(pinned.provider, openai); + + let unknown = catalog + .resolve_selection_with_catalog_fallback(Some("provider-private-preview"), None, &ready) + .unwrap(); + assert_eq!(unknown.provider, ProviderId::new("openrouter")); + assert_eq!(unknown.model, "provider-private-preview"); + } + #[test] fn legacy_builtin_selector_uses_readiness_priority_and_explicit_pins() { let catalog = portable_model_catalog();