From 264cac3c64a55873d79c42a38951d9c79b1079b4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp <19+brynary@users.noreply.github.com> Date: Tue, 12 May 2026 14:55:28 -0700 Subject: [PATCH] feat(server): inject settings-backed model catalog (#247) ## Summary This PR moves the server-facing model catalog paths onto a resolved catalog stored in `AppState`, using configured `[llm]` provider/model overrides layered on top of the built-in catalog. The server now uses the injected catalog for: - `/models` listing and model test lookup - `/completions` default model and provider inference - manifest preflight materialization and LLM model alias resolution - diagnostics LLM probes - pull request default model selection - runtime settings refresh via `replace_settings` It also adds catalog overlay helpers in `fabro-model` and converts resolved server runtime `[llm]` settings into the catalog shape in `fabro-config`. This branch also includes the earlier `chore: update dockerfile` commit, which updates the Daytona snapshot to `fabro-v10` and installs Chromium through the xtradeb PPA with an XFCE/browser wrapper. Related: #210 ## Tests - `cargo +nightly-2026-04-14 fmt --check --all` - `cargo nextest run -p fabro-config -p fabro-model -p fabro-server` (844 tests passed) - `cargo +nightly-2026-04-14 clippy -p fabro-config -p fabro-model -p fabro-server --all-targets -- -D warnings` --- .fabro/project.toml | 35 ++- lib/crates/fabro-config/src/builders.rs | 178 ++++++++++++- lib/crates/fabro-model/src/catalog.rs | 235 +++++++++++++++++- lib/crates/fabro-server/src/diagnostics.rs | 30 ++- lib/crates/fabro-server/src/run_manifest.rs | 96 ++++++- lib/crates/fabro-server/src/serve.rs | 3 + lib/crates/fabro-server/src/server.rs | 18 ++ .../src/server/handler/completions.rs | 14 +- .../fabro-server/src/server/handler/models.rs | 6 +- .../src/server/handler/pull_requests.rs | 7 +- lib/crates/fabro-server/src/server/tests.rs | 113 +++++++++ lib/crates/fabro-server/src/test_support.rs | 11 + 12 files changed, 706 insertions(+), 40 deletions(-) diff --git a/.fabro/project.toml b/.fabro/project.toml index 3650b5985..35667daaf 100644 --- a/.fabro/project.toml +++ b/.fabro/project.toml @@ -11,7 +11,7 @@ auto_stop_interval = 30 repo = "fabro-sh/fabro" [run.sandbox.daytona.snapshot] -name = "fabro-v9" +name = "fabro-v10" cpu = 8 memory = "16GB" disk = "20GB" @@ -24,6 +24,39 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libx11-6 libxrandr2 libxext6 libxrender1 libxfixes3 libxss1 libxtst6 libxi6 \ && rm -rf /var/lib/apt/lists/* +# Install real Chromium (not the snap stub) via xtradeb PPA +RUN apt-get update && apt-get install -y --no-install-recommends \ + software-properties-common curl gnupg \ + && add-apt-repository -y ppa:xtradeb/apps \ + && apt-get update \ + && apt-get install -y --no-install-recommends chromium \ + && rm -rf /var/lib/apt/lists/* + +# Wrapper: Chromium needs --no-sandbox when running as root in a container, +# and --disable-dev-shm-usage avoids crashes from small /dev/shm +RUN printf '#!/bin/bash\nexec /usr/bin/chromium --no-sandbox --disable-dev-shm-usage "$@"\n' \ + > /usr/local/bin/chromium-wrapper \ + && chmod +x /usr/local/bin/chromium-wrapper + +# Make the wrapper the default in the system .desktop file and via alternatives +RUN sed -i 's|^Exec=.*|Exec=/usr/local/bin/chromium-wrapper %U|' \ + /usr/share/applications/chromium.desktop \ + && update-alternatives --install /usr/bin/x-www-browser x-www-browser \ + /usr/local/bin/chromium-wrapper 100 + +# Tell XFCE's exo-open that Chromium is the WebBrowser helper (system-wide) +RUN mkdir -p /etc/xdg/xfce4 /usr/share/xfce4/helpers \ + && printf 'WebBrowser=custom-WebBrowser\n' > /etc/xdg/xfce4/helpers.rc \ + && printf '[Desktop Entry]\n\ +Version=1.0\n\ +Type=X-XFCE-Helper\n\ +Name=Chromium\n\ +Icon=chromium\n\ +X-XFCE-Category=WebBrowser\n\ +X-XFCE-CommandsWithParameter=/usr/local/bin/chromium-wrapper "%%s"\n\ +X-XFCE-Commands=/usr/local/bin/chromium-wrapper\n' \ + > /usr/share/xfce4/helpers/custom-WebBrowser.desktop + # GitHub CLI RUN curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg \ | dd of=/usr/share/keyrings/githubcli-archive-keyring.gpg \ diff --git a/lib/crates/fabro-config/src/builders.rs b/lib/crates/fabro-config/src/builders.rs index c31577318..f32bdb9b3 100644 --- a/lib/crates/fabro-config/src/builders.rs +++ b/lib/crates/fabro-config/src/builders.rs @@ -1,6 +1,8 @@ +use std::collections::BTreeMap; use std::fmt; use std::path::Path; +use fabro_model::catalog as model_catalog; use fabro_types::settings::{RunNamespace, WorkflowNamespace}; use fabro_types::{ServerSettings, UserSettings, WorkflowSettings}; use fabro_util::error::SharedError; @@ -12,7 +14,11 @@ use crate::resolve::{ resolve_workflow, }; use crate::user::load_settings_config; -use crate::{CliLayer, Combine, Error, Result, RunLayer, ServerLayer, SettingsLayer, run}; +use crate::{ + CliLayer, Combine, CostRates, Error, LlmLayer, LlmModelFeatures, LlmModelLimits, ModelControls, + ModelCostTable, ModelSettings, ProviderSettings, Result, RunLayer, ServerLayer, SettingsLayer, + run, +}; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolveErrors(pub Vec); @@ -204,6 +210,7 @@ pub struct ServerRuntimeSettings { pub server_settings: ServerSettings, pub manifest_run_defaults: RunLayer, pub manifest_run_settings: std::result::Result, + pub llm_catalog_settings: model_catalog::LlmCatalogSettings, } pub fn load_server_runtime_settings( @@ -251,14 +258,134 @@ fn resolve_server_runtime_settings( } let manifest_run_defaults = layer.run.clone().unwrap_or_default(); + let llm_catalog_settings = llm_catalog_settings_from_layer(&layer); Ok(ServerRuntimeSettings { server_settings: ServerSettingsBuilder::from_layer(&layer)?, manifest_run_settings: RunSettingsBuilder::from_run_layer(&manifest_run_defaults) .map_err(|err| SharedError::new(anyhow::Error::new(err))), manifest_run_defaults, + llm_catalog_settings, }) } +fn llm_catalog_settings_from_layer(layer: &SettingsLayer) -> model_catalog::LlmCatalogSettings { + let layer = layer.clone().combine(DEFAULTS_LAYER.clone()); + layer + .llm + .map(llm_layer_to_catalog_settings) + .unwrap_or_default() +} + +fn llm_layer_to_catalog_settings(llm: LlmLayer) -> model_catalog::LlmCatalogSettings { + model_catalog::LlmCatalogSettings { + providers: llm + .providers + .into_inner() + .into_iter() + .map(|(id, settings)| (id, provider_settings_to_catalog(settings))) + .collect(), + models: llm + .models + .into_inner() + .into_iter() + .map(|(id, settings)| (id, model_settings_to_catalog(settings))) + .collect(), + } +} + +fn provider_settings_to_catalog( + settings: ProviderSettings, +) -> model_catalog::ProviderCatalogSettings { + model_catalog::ProviderCatalogSettings { + display_name: settings.display_name, + adapter: settings.adapter, + base_url: settings.base_url, + credentials: settings.credentials, + extra_headers: settings.extra_headers, + priority: settings.priority, + enabled: settings.enabled, + aliases: settings.aliases, + } +} + +fn model_settings_to_catalog(settings: ModelSettings) -> model_catalog::ModelCatalogSettings { + let ModelSettings { + provider, + api_id, + display_name, + family, + training, + knowledge_cutoff, + default, + enabled, + aliases, + estimated_output_tps, + limits, + features, + controls, + costs, + } = settings; + model_catalog::ModelCatalogSettings { + provider, + api_id, + display_name, + family, + training, + knowledge_cutoff, + default, + enabled, + aliases, + estimated_output_tps, + limits: limits.as_ref().map(model_limits_to_catalog), + features: features.as_ref().map(model_features_to_catalog), + controls: controls.map(model_controls_to_catalog), + costs: costs.as_ref().map(model_cost_table_to_catalog), + } +} + +fn model_limits_to_catalog(limits: &LlmModelLimits) -> model_catalog::SettingsModelLimits { + model_catalog::SettingsModelLimits { + context_window: limits.context_window, + max_output: limits.max_output, + } +} + +fn model_features_to_catalog(features: &LlmModelFeatures) -> model_catalog::SettingsModelFeatures { + model_catalog::SettingsModelFeatures { + tools: features.tools, + vision: features.vision, + reasoning: features.reasoning, + effort: features.effort, + } +} + +fn model_controls_to_catalog(controls: ModelControls) -> model_catalog::SettingsModelControls { + model_catalog::SettingsModelControls { + reasoning_effort: controls.reasoning_effort, + speed: controls.speed, + } +} + +fn model_cost_table_to_catalog(costs: &ModelCostTable) -> model_catalog::SettingsModelCostTable { + model_catalog::SettingsModelCostTable { + base: cost_rates_to_catalog(&costs.base), + speed: costs.speed.as_ref().map(|speed| { + speed + .iter() + .map(|(key, rates)| (key.clone(), cost_rates_to_catalog(rates))) + .collect::>() + }), + } +} + +fn cost_rates_to_catalog(rates: &CostRates) -> model_catalog::CostRates { + model_catalog::CostRates { + input_cost_per_mtok: rates.input_cost_per_mtok, + output_cost_per_mtok: rates.output_cost_per_mtok, + cache_input_cost_per_mtok: rates.cache_input_cost_per_mtok, + } +} + #[derive(Clone, Debug, Default)] pub struct WorkflowSettingsBuilder { args: SettingsLayer, @@ -456,7 +583,7 @@ mod tests { use fabro_types::settings::cli::OutputVerbosity; use fabro_types::settings::run::{ApprovalMode, RunMode}; - use super::{RunSettingsBuilder, WorkflowSettingsBuilder}; + use super::{RunSettingsBuilder, WorkflowSettingsBuilder, server_runtime_settings_from_toml}; use crate::{CliLayer, CliOutputLayer, ReplaceMap, RunExecutionLayer, RunLayer, RunModelLayer}; #[test] @@ -531,4 +658,51 @@ command = ["demo-mcp"] assert_eq!(settings.run.execution.mode, RunMode::DryRun); assert_eq!(settings.run.execution.approval, ApprovalMode::Auto); } + + #[test] + fn server_runtime_settings_preserves_llm_catalog_overrides() { + let settings = server_runtime_settings_from_toml( + r#" +_version = 1 + +[server.auth] +methods = ["dev-token"] + +[llm.providers.venice] +display_name = "Venice" +adapter = "openai_compatible" +base_url = "https://api.venice.ai/api/v1" +credentials = ["env:VENICE_API_KEY"] + +[llm.models."venice-large"] +provider = "venice" +display_name = "Venice Large" +family = "venice" +default = true + +[llm.models."venice-large".limits] +context_window = 128000 + +[llm.models."venice-large".features] +tools = true +vision = false +reasoning = false +effort = false +"#, + None, + None, + ) + .expect("server runtime settings should resolve"); + + let catalog = + fabro_model::Catalog::from_builtin_with_overrides(&settings.llm_catalog_settings) + .expect("catalog overrides should build"); + + assert_eq!( + catalog + .get("venice-large") + .map(|model| model.provider.clone()), + Some(fabro_model::ProviderId::new("venice")) + ); + } } diff --git a/lib/crates/fabro-model/src/catalog.rs b/lib/crates/fabro-model/src/catalog.rs index 9c35c03c1..b0658e89f 100644 --- a/lib/crates/fabro-model/src/catalog.rs +++ b/lib/crates/fabro-model/src/catalog.rs @@ -610,6 +610,14 @@ impl Catalog { }) } + pub fn from_builtin_with_overrides( + overrides: &LlmCatalogSettings, + ) -> Result { + let builtins = Self::builtin_settings()?; + let settings = merge_catalog_settings(overrides.clone(), builtins); + Self::from_settings(&settings) + } + /// Create a catalog from a custom set of models (useful for testing). #[must_use] pub fn from_models(models: Vec) -> Self { @@ -658,7 +666,7 @@ impl Catalog { } } - fn from_builtin_toml() -> Result { + fn builtin_settings() -> Result { let mut layer = LlmCatalogSettings::default(); let mut paths = BuiltinCatalogToml::iter() .filter(|path| path.ends_with(".toml")) @@ -688,7 +696,11 @@ impl Catalog { layer.models.extend(fragment.models); } - Self::from_settings(&layer) + Ok(layer) + } + + fn from_builtin_toml() -> Result { + Self::from_settings(&Self::builtin_settings()?) } /// Look up a model by ID or alias. @@ -899,6 +911,146 @@ fn build_model_index(models: &[Model]) -> HashMap { index } +fn merge_catalog_settings( + higher: LlmCatalogSettings, + mut fallback: LlmCatalogSettings, +) -> LlmCatalogSettings { + for (id, provider) in higher.providers { + let provider = match fallback.providers.remove(&id) { + Some(fallback_provider) => merge_provider_settings(provider, fallback_provider), + None => provider, + }; + fallback.providers.insert(id, provider); + } + + for (id, model) in higher.models { + let model = match fallback.models.remove(&id) { + Some(fallback_model) => merge_model_settings(model, fallback_model), + None => model, + }; + fallback.models.insert(id, model); + } + + fallback +} + +fn merge_provider_settings( + higher: ProviderCatalogSettings, + fallback: ProviderCatalogSettings, +) -> ProviderCatalogSettings { + ProviderCatalogSettings { + display_name: higher.display_name.or(fallback.display_name), + adapter: higher.adapter.or(fallback.adapter), + base_url: higher.base_url.or(fallback.base_url), + credentials: higher.credentials.or(fallback.credentials), + extra_headers: higher.extra_headers.or(fallback.extra_headers), + priority: higher.priority.or(fallback.priority), + enabled: higher.enabled.or(fallback.enabled), + aliases: higher.aliases.or(fallback.aliases), + } +} + +fn merge_model_settings( + higher: ModelCatalogSettings, + fallback: ModelCatalogSettings, +) -> ModelCatalogSettings { + ModelCatalogSettings { + provider: higher.provider.or(fallback.provider), + api_id: higher.api_id.or(fallback.api_id), + display_name: higher.display_name.or(fallback.display_name), + family: higher.family.or(fallback.family), + training: higher.training.or(fallback.training), + knowledge_cutoff: higher.knowledge_cutoff.or(fallback.knowledge_cutoff), + default: higher.default.or(fallback.default), + enabled: higher.enabled.or(fallback.enabled), + aliases: higher.aliases.or(fallback.aliases), + estimated_output_tps: higher + .estimated_output_tps + .or(fallback.estimated_output_tps), + limits: merge_optional( + higher.limits, + fallback.limits, + merge_model_limits_settings, + ), + features: merge_optional( + higher.features, + fallback.features, + merge_model_features_settings, + ), + controls: merge_optional( + higher.controls, + fallback.controls, + merge_model_controls_settings, + ), + costs: merge_optional(higher.costs, fallback.costs, merge_model_cost_table), + } +} + +fn merge_optional(higher: Option, fallback: Option, merge: fn(&T, &T) -> T) -> Option { + match (higher, fallback) { + (Some(higher), Some(fallback)) => Some(merge(&higher, &fallback)), + (Some(higher), None) => Some(higher), + (None, fallback) => fallback, + } +} + +fn merge_model_limits_settings( + higher: &SettingsModelLimits, + fallback: &SettingsModelLimits, +) -> SettingsModelLimits { + SettingsModelLimits { + context_window: higher.context_window.or(fallback.context_window), + max_output: higher.max_output.or(fallback.max_output), + } +} + +fn merge_model_features_settings( + higher: &SettingsModelFeatures, + fallback: &SettingsModelFeatures, +) -> SettingsModelFeatures { + SettingsModelFeatures { + tools: higher.tools.or(fallback.tools), + vision: higher.vision.or(fallback.vision), + reasoning: higher.reasoning.or(fallback.reasoning), + effort: higher.effort.or(fallback.effort), + } +} + +fn merge_model_controls_settings( + higher: &SettingsModelControls, + fallback: &SettingsModelControls, +) -> SettingsModelControls { + SettingsModelControls { + reasoning_effort: higher + .reasoning_effort + .clone() + .or_else(|| fallback.reasoning_effort.clone()), + speed: higher.speed.clone().or_else(|| fallback.speed.clone()), + } +} + +fn merge_model_cost_table( + higher: &SettingsModelCostTable, + fallback: &SettingsModelCostTable, +) -> SettingsModelCostTable { + SettingsModelCostTable { + base: merge_cost_rates(&higher.base, &fallback.base), + speed: higher.speed.clone().or_else(|| fallback.speed.clone()), + } +} + +fn merge_cost_rates(higher: &CostRates, fallback: &CostRates) -> CostRates { + CostRates { + input_cost_per_mtok: higher.input_cost_per_mtok.or(fallback.input_cost_per_mtok), + output_cost_per_mtok: higher + .output_cost_per_mtok + .or(fallback.output_cost_per_mtok), + cache_input_cost_per_mtok: higher + .cache_input_cost_per_mtok + .or(fallback.cache_input_cost_per_mtok), + } +} + fn build_providers( settings: &LlmCatalogSettings, ) -> Result, CatalogBuildError> { @@ -1354,6 +1506,85 @@ mod tests { // ---- Catalog struct tests ---- + #[test] + fn builtin_with_empty_overrides_matches_builtin_catalog() { + let catalog = Catalog::from_builtin_with_overrides(&LlmCatalogSettings::default()) + .expect("empty overrides should build"); + + assert_eq!( + catalog.get("sonnet").map(|model| model.id.as_str()), + Catalog::builtin() + .get("sonnet") + .map(|model| model.id.as_str()) + ); + assert_eq!( + catalog.default_model().id, + Catalog::builtin().default_model().id + ); + } + + #[test] + fn builtin_overrides_sparse_provider_fields() { + let catalog = Catalog::from_builtin_with_overrides(&minimal_settings( + r" +[providers.anthropic] +enabled = false +", + )) + .expect("sparse built-in provider override should build"); + + assert!(catalog.provider(&ProviderId::anthropic()).is_none()); + assert!(catalog.get("claude-sonnet-4-5").is_none()); + assert!( + catalog + .providers() + .iter() + .any(|provider| provider.id == ProviderId::openai()) + ); + } + + #[test] + fn builtin_overrides_add_custom_openai_compatible_provider_and_model() { + let catalog = Catalog::from_builtin_with_overrides(&minimal_settings( + r#" +[providers.venice] +display_name = "Venice" +adapter = "openai_compatible" +base_url = "https://api.venice.ai/api/v1" +credentials = ["env:VENICE_API_KEY"] +priority = 120 +aliases = ["venice-ai"] + +[models."venice-large"] +provider = "venice" +display_name = "Venice Large" +family = "venice" +default = true +aliases = ["vl"] + +[models."venice-large".limits] +context_window = 128000 + +[models."venice-large".features] +tools = true +vision = false +reasoning = false +effort = false +"#, + )) + .expect("custom provider overlay should build"); + + let provider = catalog + .provider(&ProviderId::new("venice-ai")) + .expect("provider alias should resolve"); + assert_eq!(provider.id, ProviderId::new("venice")); + assert_eq!(provider.adapter, "openai_compatible"); + + let model = catalog.get("vl").expect("model alias should resolve"); + assert_eq!(model.id, "venice-large"); + assert_eq!(model.provider, ProviderId::new("venice")); + } + #[test] fn builtin_get_by_id() { let m = Catalog::builtin().get("claude-opus-4-6").unwrap(); diff --git a/lib/crates/fabro-server/src/diagnostics.rs b/lib/crates/fabro-server/src/diagnostics.rs index 374d2919a..ea1aead56 100644 --- a/lib/crates/fabro-server/src/diagnostics.rs +++ b/lib/crates/fabro-server/src/diagnostics.rs @@ -119,13 +119,17 @@ async fn check_llm_providers(state: &AppState) -> CheckResult { .filter_map(|name| name.parse::().ok()) .collect(); let client = &result.client; - let probe_outcomes = join_all(providers.iter().map(|&provider| async move { - let outcome = timeout( - Duration::from_secs(30), - probe_llm_provider(client, provider), - ) - .await; - (provider, outcome) + let catalog = state.catalog(); + let probe_outcomes = join_all(providers.iter().map(|&provider| { + let catalog = catalog.clone(); + async move { + let outcome = timeout( + Duration::from_secs(30), + probe_llm_provider(client, provider, catalog.as_ref()), + ) + .await; + (provider, outcome) + } })) .await; for (provider, probe_result) in probe_outcomes { @@ -200,15 +204,19 @@ fn short_error_line(rendered: &str) -> String { } } -fn probe_model(provider: Provider) -> String { - Catalog::builtin() +fn probe_model(provider: Provider, catalog: &Catalog) -> String { + catalog .probe_for_provider(provider) .map_or_else(|| format!("unknown-{provider}"), |m| m.id.clone()) } -async fn probe_llm_provider(client: &LlmClient, provider: Provider) -> fabro_llm::Result<()> { +async fn probe_llm_provider( + client: &LlmClient, + provider: Provider, + catalog: &Catalog, +) -> fabro_llm::Result<()> { let request = Request { - model: probe_model(provider), + model: probe_model(provider, catalog), messages: vec![Message::user("hi")], provider: Some(provider.to_string()), tools: None, diff --git a/lib/crates/fabro-server/src/run_manifest.rs b/lib/crates/fabro-server/src/run_manifest.rs index a212010ea..3d47379f5 100644 --- a/lib/crates/fabro-server/src/run_manifest.rs +++ b/lib/crates/fabro-server/src/run_manifest.rs @@ -436,10 +436,11 @@ async fn build_preflight_report( } let configured_providers = state.llm_source.configured_providers().await; + let catalog = state.catalog(); let materialized = materialize_run( prepared.settings.clone(), graph, - Catalog::builtin(), + catalog.as_ref(), &configured_providers, ); let resolved_run = materialized.run; @@ -486,6 +487,7 @@ async fn build_preflight_report( graph, &resolved_run, &configured_providers, + catalog.as_ref(), ) .await; run_github_token_check(&mut checks, prepared, &resolved_run, github_app).await; @@ -887,8 +889,9 @@ async fn run_llm_check( graph: &Graph, settings: &RunNamespace, configured_providers: &[ProviderId], + catalog: &Catalog, ) -> bool { - let (model, provider) = resolve_model_provider(settings, graph, configured_providers); + let (model, provider) = resolve_model_provider(settings, graph, configured_providers, catalog); let default_provider = provider.as_deref().unwrap_or("anthropic"); match state.resolve_llm_client().await { @@ -912,7 +915,7 @@ async fn run_llm_check( let node_model = node.model().unwrap_or(&model); let node_provider = node.provider().unwrap_or(default_provider); let (resolved_model, resolved_provider) = - if let Some(info) = Catalog::builtin().get(node_model) { + if let Some(info) = catalog.get(node_model) { (info.id.clone(), info.provider.to_string()) } else { (node_model.to_string(), node_provider.to_string()) @@ -930,12 +933,11 @@ async fn run_llm_check( } if model_providers.is_empty() { - let (resolved_model, resolved_provider) = - if let Some(info) = Catalog::builtin().get(&model) { - (info.id.clone(), info.provider.to_string()) - } else { - (model.clone(), default_provider.to_string()) - }; + let (resolved_model, resolved_provider) = if let Some(info) = catalog.get(&model) { + (info.id.clone(), info.provider.to_string()) + } else { + (model.clone(), default_provider.to_string()) + }; model_providers.insert((resolved_model, resolved_provider)); } @@ -1040,6 +1042,7 @@ fn resolve_model_provider( settings: &RunNamespace, _graph: &Graph, configured_providers: &[ProviderId], + catalog: &Catalog, ) -> (String, Option) { let provider = settings .model @@ -1048,7 +1051,7 @@ fn resolve_model_provider( .map(InterpString::as_source); let model = settings.model.name.as_ref().map_or_else( || { - Catalog::builtin() + catalog .default_for_configured_ids(configured_providers) .id .clone() @@ -1056,7 +1059,7 @@ fn resolve_model_provider( InterpString::as_source, ); - match Catalog::builtin().get(&model) { + match catalog.get(&model) { Some(info) => ( info.id.clone(), provider.or(Some(info.provider.to_string())), @@ -2039,6 +2042,77 @@ digraph Demo { ); } + #[tokio::test] + async fn preflight_resolves_model_aliases_from_app_state_catalog() { + let llm_catalog_settings: fabro_model::catalog::LlmCatalogSettings = toml::from_str( + r#" +[providers.venice] +display_name = "Venice" +adapter = "openai_compatible" +base_url = "https://api.venice.ai/api/v1" +credentials = ["env:VENICE_API_KEY"] + +[models."venice-large"] +provider = "venice" +display_name = "Venice Large" +family = "venice" +default = true +aliases = ["vl"] + +[models."venice-large".limits] +context_window = 128000 + +[models."venice-large".features] +tools = true +vision = false +reasoning = false +effort = false +"#, + ) + .expect("catalog fixture should parse"); + let state = crate::test_support::TestAppStateBuilder::new() + .llm_catalog_settings(llm_catalog_settings) + .build(); + let mut manifest = minimal_manifest(); + manifest.workflows.get_mut("workflow.fabro").unwrap().source = r#" +digraph Demo { + start [shape=Mdiamond] + exit [shape=Msquare] + work [prompt="Do work", model="vl"] + start -> work -> exit +} +"# + .to_string(); + let prepared = prepare_manifest( + &manifest_run_defaults(Some(&default_settings_fixture())), + &manifest, + ) + .unwrap(); + let validated = validate_prepared_manifest(&prepared, RenderMode::Strict).unwrap(); + + let (response, ok) = run_preflight(state.as_ref(), &prepared, &validated) + .await + .unwrap(); + + assert!(!ok); + let llm_check = response.checks.sections[0] + .checks + .iter() + .find(|check| check.name == "LLM" && check.summary == "venice-large") + .expect("preflight should resolve the catalog alias"); + assert_eq!(llm_check.status, types::PreflightCheckResultStatus::Warning); + assert_eq!( + llm_check.remediation.as_deref(), + Some("Provider \"venice\" is not configured") + ); + assert!( + llm_check + .details + .iter() + .any(|detail| detail.text == "Provider: venice") + ); + } + mod root_workflow_run_layer_tests { //! `root_workflow_run_layer` parses bundled workflow.toml through //! the strict `SettingsLayer` schema, so unknown fields anywhere in diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index eb234c7db..dc1075868 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -713,6 +713,7 @@ where server_settings: runtime_settings.server_settings, manifest_run_defaults: runtime_settings.manifest_run_defaults, manifest_run_settings: runtime_settings.manifest_run_settings, + llm_catalog_settings: runtime_settings.llm_catalog_settings, }; let resolved_server_settings = resolved_app_settings.server_settings.server.clone(); let (auth_mode, server_secrets) = resolve_startup( @@ -882,6 +883,7 @@ where server_settings: resolved.server_settings, manifest_run_defaults: resolved.manifest_run_defaults, manifest_run_settings: resolved.manifest_run_settings, + llm_catalog_settings: resolved.llm_catalog_settings, } }); match resolved { @@ -1262,6 +1264,7 @@ mod tests { .map_err(|err| fabro_util::error::SharedError::new(anyhow::Error::new(err))), manifest_run_defaults, server_settings: server_settings(source), + llm_catalog_settings: fabro_model::catalog::LlmCatalogSettings::default(), } } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index c8f513efa..caed79573 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -56,6 +56,7 @@ use fabro_llm::types::{ ContentPart, FinishReason, Message as LlmMessage, Request as LlmRequest, Role, ToolChoice, ToolDefinition, }; +use fabro_model::catalog::LlmCatalogSettings; use fabro_model::{BilledTokenCounts, Catalog, ModelTestMode, ProviderId}; use fabro_redact::redact_jsonl_line; use fabro_sandbox::daytona::{self, DaytonaSandbox}; @@ -527,6 +528,7 @@ pub struct AppState { manifest_run_defaults: RwLock>, manifest_run_settings: RwLock>, pub(crate) server_settings: RwLock>, + catalog: RwLock>, pub(crate) env_lookup: EnvLookup, pub(crate) github_api_base_url: String, http_client: Option, @@ -602,6 +604,7 @@ pub(crate) struct ResolvedAppStateSettings { pub(crate) server_settings: ServerSettings, pub(crate) manifest_run_defaults: RunLayer, pub(crate) manifest_run_settings: std::result::Result, + pub(crate) llm_catalog_settings: LlmCatalogSettings, } fn accumulate_billing_rollup( @@ -660,6 +663,10 @@ impl AppState { ) } + pub(crate) fn catalog(&self) -> Arc { + Arc::clone(&self.catalog.read().expect("catalog lock poisoned")) + } + pub(crate) fn manifest_run_settings(&self) -> std::result::Result { self.manifest_run_settings .read() @@ -826,9 +833,14 @@ impl AppState { server_settings, manifest_run_defaults, manifest_run_settings, + llm_catalog_settings, } = resolved_settings; let server_settings = Arc::new(server_settings); let manifest_run_defaults = Arc::new(manifest_run_defaults); + let catalog = Arc::new( + Catalog::from_builtin_with_overrides(&llm_catalog_settings) + .context("building LLM model catalog")?, + ); resolve_canonical_origin(&server_settings.server, &self.env_lookup) .map_err(anyhow::Error::msg)?; @@ -844,6 +856,7 @@ impl AppState { .server_settings .write() .expect("server settings lock poisoned") = server_settings; + *self.catalog.write().expect("catalog lock poisoned") = catalog; Ok(()) } } @@ -1519,6 +1532,10 @@ pub(crate) fn build_app_state(config: AppStateConfig) -> anyhow::Result anyhow::Result, ) -> Response { // Resolve model - let model_id = req.model.unwrap_or_else(|| { - fabro_model::Catalog::builtin() - .list(None) - .first() - .map_or_else(|| "claude-sonnet-4-5".to_string(), |m| m.id.clone()) - }); + let catalog = state.catalog(); + let model_id = req + .model + .unwrap_or_else(|| catalog.default_model().id.clone()); - let catalog_info = fabro_model::Catalog::builtin().get(&model_id); + let catalog_info = catalog.get(&model_id); // Resolve provider: explicit request > catalog > None let explicit_provider = req.provider; @@ -131,7 +129,7 @@ async fn create_completion( let request = LlmRequest { model: model_id.clone(), messages, - provider: provider_name, + provider: provider_name.clone(), tools, tool_choice, response_format: None, diff --git a/lib/crates/fabro-server/src/server/handler/models.rs b/lib/crates/fabro-server/src/server/handler/models.rs index 1681c1f04..ceb39d558 100644 --- a/lib/crates/fabro-server/src/server/handler/models.rs +++ b/lib/crates/fabro-server/src/server/handler/models.rs @@ -47,7 +47,8 @@ async fn list_models( .into_iter() .collect(); - let mut models = fabro_model::Catalog::builtin() + let catalog = state.catalog(); + let mut models = catalog .list(provider_id.as_ref()) .into_iter() .filter(|model| match &query { @@ -100,7 +101,8 @@ async fn test_model( }, None => ModelTestMode::Basic, }; - let Some(info) = fabro_model::Catalog::builtin().get(&id) else { + let catalog = state.catalog(); + let Some(info) = catalog.get(&id) else { return ApiError::not_found(format!("Model not found: {id}")).into_response(); }; diff --git a/lib/crates/fabro-server/src/server/handler/pull_requests.rs b/lib/crates/fabro-server/src/server/handler/pull_requests.rs index 4c0a3d0ea..bc65f4f35 100644 --- a/lib/crates/fabro-server/src/server/handler/pull_requests.rs +++ b/lib/crates/fabro-server/src/server/handler/pull_requests.rs @@ -1,8 +1,8 @@ use std::sync::Arc; use super::super::{ - ApiError, AppState, Catalog, CloseRunPullRequestResponse, CreateRunPullRequestRequest, - IntoResponse, Json, MergeRunPullRequestRequest, MergeRunPullRequestResponse, PullRequestRecord, + ApiError, AppState, CloseRunPullRequestResponse, CreateRunPullRequestRequest, IntoResponse, + Json, MergeRunPullRequestRequest, MergeRunPullRequestResponse, PullRequestRecord, RequireRunScoped, Response, Router, RunId, State, StatusCode, get, lock_pull_request_create, post, pull_request, warn, workflow_event, }; @@ -248,7 +248,8 @@ async fn create_run_pull_request( model } else { let configured = state.llm_source.configured_providers().await; - Catalog::builtin() + state + .catalog() .default_for_configured_ids(&configured) .id .clone() diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 2dc425aae..b072e7509 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -16,6 +16,7 @@ use fabro_interview::{ AnswerValue, ControlInterviewer, Interviewer, Question, WorkerControlMessage, }; use fabro_llm::types::{Message as LlmMessage, Request as LlmRequest}; +use fabro_model::catalog::LlmCatalogSettings; use fabro_model::{ModelRef, Provider}; use fabro_types::settings::ServerAuthMethod; use fabro_types::{ @@ -72,6 +73,7 @@ fn resolved_runtime_settings_from_toml(source: &str) -> ResolvedAppStateSettings resolved_runtime_settings_for_tests( server_settings_from_toml(source), manifest_run_defaults_from_toml(source), + LlmCatalogSettings::default(), ) } @@ -1756,6 +1758,7 @@ methods = ["dev-token"] resolved_settings: resolved_runtime_settings_for_tests( server_settings, RunLayer::default(), + LlmCatalogSettings::default(), ), registry_factory_override: None, max_concurrent_runs: 5, @@ -3455,6 +3458,7 @@ fn create_github_token_app_state_with_env_lookup( resolved_settings: resolved_runtime_settings_for_tests( github_token_settings(), RunLayer::default(), + LlmCatalogSettings::default(), ), registry_factory_override: None, max_concurrent_runs: 5, @@ -3837,6 +3841,53 @@ async fn list_models_unknown_provider_returns_empty_page() { assert_eq!(body["meta"]["has_more"].as_bool(), Some(false)); } +#[tokio::test] +async fn list_models_uses_app_state_catalog_overrides() { + let llm_catalog_settings: LlmCatalogSettings = toml::from_str( + r#" +[providers.venice] +display_name = "Venice" +adapter = "openai_compatible" +base_url = "https://api.venice.ai/api/v1" +credentials = ["env:VENICE_API_KEY"] +priority = 120 + +[models."venice-large"] +provider = "venice" +display_name = "Venice Large" +family = "venice" +default = true + +[models."venice-large".limits] +context_window = 128000 + +[models."venice-large".features] +tools = true +vision = false +reasoning = false +effort = false +"#, + ) + .expect("catalog fixture should parse"); + let state = TestAppStateBuilder::new() + .llm_catalog_settings(llm_catalog_settings) + .build(); + let app = crate::test_support::build_test_router(state); + + let req = Request::builder() + .method("GET") + .uri(api("/models?provider=venice")) + .body(Body::empty()) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::OK).await; + let models = body["data"].as_array().unwrap(); + assert_eq!(models.len(), 1); + assert_eq!(models[0]["id"], "venice-large"); + assert_eq!(models[0]["provider"], "venice"); +} + #[tokio::test] async fn auth_login_github_redirects_to_github() { let source = r#" @@ -9125,6 +9176,68 @@ async fn create_completion_unknown_provider_returns_clear_error() { ); } +#[tokio::test] +async fn create_completion_default_model_uses_app_state_catalog() { + let llm_catalog_settings: LlmCatalogSettings = toml::from_str( + r#" +[providers.venice] +display_name = "Venice" +adapter = "openai_compatible" +base_url = "https://api.venice.ai/api/v1" +credentials = ["env:VENICE_API_KEY"] +priority = 120 + +[models."venice-large"] +provider = "venice" +display_name = "Venice Large" +family = "venice" +default = true + +[models."venice-large".limits] +context_window = 128000 + +[models."venice-large".features] +tools = true +vision = false +reasoning = false +effort = false +"#, + ) + .expect("catalog fixture should parse"); + let state = TestAppStateBuilder::new() + .llm_catalog_settings(llm_catalog_settings) + .build(); + let app = crate::test_support::build_test_router(state); + + let req = Request::builder() + .method("POST") + .uri(api("/completions")) + .header("content-type", "application/json") + .body(Body::from( + serde_json::json!({ + "stream": false, + "messages": [ + { + "role": "user", + "content": [{"kind": "text", "data": "hi"}] + } + ] + }) + .to_string(), + )) + .unwrap(); + + let response = app.oneshot(req).await.unwrap(); + let body = response_json!(response, StatusCode::BAD_GATEWAY).await; + assert!( + body["errors"][0]["detail"] + .as_str() + .unwrap() + .contains("Provider 'venice' not registered"), + "unexpected error body: {body:?}" + ); +} + #[tokio::test] async fn demo_boards_runs_returns_run_list_items() { let state = test_app_state(); diff --git a/lib/crates/fabro-server/src/test_support.rs b/lib/crates/fabro-server/src/test_support.rs index a080f03f4..04d17f386 100644 --- a/lib/crates/fabro-server/src/test_support.rs +++ b/lib/crates/fabro-server/src/test_support.rs @@ -15,6 +15,7 @@ use axum::{Router, middleware}; use chrono::Duration as ChronoDuration; use fabro_config::{RunLayer, RunSettingsBuilder, ServerSettingsBuilder, envfile}; use fabro_interview::Interviewer; +use fabro_model::catalog::LlmCatalogSettings; use fabro_static::EnvVars; use fabro_store::{ArtifactStore, Database}; use fabro_types::settings::ServerAuthMethod; @@ -64,6 +65,7 @@ pub struct TestAppStateBuilder { server_env_path: Option, server_secret_env: HashMap, env_lookup: EnvLookup, + llm_catalog_settings: LlmCatalogSettings, } impl Default for TestAppStateBuilder { @@ -78,6 +80,7 @@ impl Default for TestAppStateBuilder { server_env_path: None, server_secret_env: HashMap::new(), env_lookup: default_env_lookup(), + llm_catalog_settings: LlmCatalogSettings::default(), } } } @@ -121,6 +124,11 @@ impl TestAppStateBuilder { self } + pub fn llm_catalog_settings(mut self, settings: LlmCatalogSettings) -> Self { + self.llm_catalog_settings = settings; + self + } + pub fn server_secret_env(mut self, server_secret_env: HashMap) -> Self { self.server_secret_env = server_secret_env; self @@ -151,6 +159,7 @@ impl TestAppStateBuilder { resolved_settings: resolved_runtime_settings_for_tests( self.server_settings, self.manifest_run_defaults, + self.llm_catalog_settings, ), registry_factory_override: self.registry_factory_override, max_concurrent_runs: self.max_concurrent_runs, @@ -219,12 +228,14 @@ pub fn test_app_state_with_options( pub(crate) fn resolved_runtime_settings_for_tests( server_settings: ServerSettings, manifest_run_defaults: RunLayer, + llm_catalog_settings: LlmCatalogSettings, ) -> ResolvedAppStateSettings { ResolvedAppStateSettings { manifest_run_settings: RunSettingsBuilder::from_run_layer(&manifest_run_defaults) .map_err(|err| SharedError::new(anyhow::Error::new(err))), manifest_run_defaults, server_settings, + llm_catalog_settings, } }