From 12ab5f1a9345f8f96e8e00f41d89aefd55899279 Mon Sep 17 00:00:00 2001 From: Irwin Date: Wed, 18 Mar 2026 01:22:01 +1300 Subject: [PATCH] Normalize provider-backed model updates Resolve catalog display names and aliases to canonical model IDs when spawning or updating agents, keep provider-specific api_key_env hints in sync when switching providers, and route explicit provider model changes through kernel model normalization instead of directly mutating the registry. This fixes cases like xAI agents carrying stale OpenAI auth hints or UI labels such as 'Grok 4.20' being treated as raw model IDs. (cherry picked from commit 51ee6a5a927208ab0301a29fd2e40b29c9dfaf7d) --- crates/openfang-api/src/routes.rs | 20 +++---- crates/openfang-kernel/src/kernel.rs | 61 ++++++++++++++++---- crates/openfang-kernel/src/registry.rs | 21 +++++++ crates/openfang-runtime/src/model_catalog.rs | 18 +++++- 4 files changed, 96 insertions(+), 24 deletions(-) diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index 456d867f0..9ba169620 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -8813,20 +8813,16 @@ pub async fn patch_agent_config( if !new_model.is_empty() { if let Some(ref new_provider) = req.provider { if !new_provider.is_empty() { - // Explicit provider given — use it directly - if state - .kernel - .registry - .update_model_and_provider( - agent_id, - new_model.clone(), - new_provider.clone(), - ) - .is_err() + // Explicit provider given — still route through set_agent_model + // so provider-specific auth/env hints stay in sync. + if let Err(e) = + state + .kernel + .set_agent_model(agent_id, new_model, Some(new_provider)) { return ( - StatusCode::NOT_FOUND, - Json(serde_json::json!({"error": "Agent not found"})), + StatusCode::INTERNAL_SERVER_ERROR, + Json(serde_json::json!({"error": format!("{e}")})), ); } } else { diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index 816132fba..b6318c769 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -1311,6 +1311,30 @@ impl OpenFangKernel { } } + // Normalize catalog-backed model labels/aliases into canonical IDs and + // fill provider/auth hints when the manifest did not fully specify them. + if let Ok(catalog) = self.model_catalog.read() { + if let Some(entry) = catalog.find_model(&manifest.model.model) { + let provider_is_default = + manifest.model.provider.is_empty() || manifest.model.provider == "default"; + if provider_is_default || manifest.model.provider == entry.provider { + manifest.model.provider = entry.provider.clone(); + manifest.model.model = strip_provider_prefix(&entry.id, &entry.provider); + if manifest.model.api_key_env.is_none() { + manifest.model.api_key_env = + Some(self.config.resolve_api_key_env(&entry.provider)); + } + } + } + } + if manifest.model.api_key_env.is_none() + && !manifest.model.provider.is_empty() + && manifest.model.provider != "default" + { + manifest.model.api_key_env = + Some(self.config.resolve_api_key_env(&manifest.model.provider)); + } + // Normalize: strip provider prefix from model name if present let normalized = strip_provider_prefix(&manifest.model.model, &manifest.model.provider); if normalized != manifest.model.model { @@ -2831,6 +2855,11 @@ impl OpenFangKernel { model: &str, explicit_provider: Option<&str>, ) -> KernelResult<()> { + let catalog_entry = self + .model_catalog + .read() + .ok() + .and_then(|catalog| catalog.find_model(model).cloned()); let provider = if let Some(ep) = explicit_provider { // User explicitly set the provider — use it as-is Some(ep.to_string()) @@ -2849,25 +2878,35 @@ impl OpenFangKernel { None } else { // No custom base_url: safe to auto-detect from catalog / model name - let resolved_provider = self.model_catalog.read().ok().and_then(|catalog| { - catalog - .find_model(model) - .map(|entry| entry.provider.clone()) - }); + let resolved_provider = catalog_entry.as_ref().map(|entry| entry.provider.clone()); resolved_provider.or_else(|| infer_provider_from_model(model)) } }; // Strip the provider prefix from the model name (e.g. "openrouter/deepseek/deepseek-chat" → "deepseek/deepseek-chat") - let normalized_model = if let Some(ref prov) = provider { - strip_provider_prefix(model, prov) - } else { - model.to_string() - }; + let normalized_model = + if let (Some(entry), Some(prov)) = (catalog_entry.as_ref(), provider.as_ref()) { + if entry.provider == *prov { + strip_provider_prefix(&entry.id, prov) + } else { + strip_provider_prefix(model, prov) + } + } else if let Some(ref prov) = provider { + strip_provider_prefix(model, prov) + } else { + model.to_string() + }; if let Some(provider) = provider { + let api_key_env = Some(self.config.resolve_api_key_env(&provider)); self.registry - .update_model_and_provider(agent_id, normalized_model.clone(), provider.clone()) + .update_model_provider_config( + agent_id, + normalized_model.clone(), + provider.clone(), + api_key_env, + None, + ) .map_err(KernelError::OpenFang)?; info!(agent_id = %agent_id, model = %normalized_model, provider = %provider, "Agent model+provider updated"); } else { diff --git a/crates/openfang-kernel/src/registry.rs b/crates/openfang-kernel/src/registry.rs index b3c3a4962..aaf07d21f 100644 --- a/crates/openfang-kernel/src/registry.rs +++ b/crates/openfang-kernel/src/registry.rs @@ -177,6 +177,27 @@ impl AgentRegistry { Ok(()) } + /// Update an agent's model, provider, and connection hints together. + pub fn update_model_provider_config( + &self, + id: AgentId, + new_model: String, + new_provider: String, + api_key_env: Option, + base_url: Option, + ) -> OpenFangResult<()> { + let mut entry = self + .agents + .get_mut(&id) + .ok_or_else(|| OpenFangError::AgentNotFound(id.to_string()))?; + entry.manifest.model.model = new_model; + entry.manifest.model.provider = new_provider; + entry.manifest.model.api_key_env = api_key_env; + entry.manifest.model.base_url = base_url; + entry.last_active = chrono::Utc::now(); + Ok(()) + } + /// Update an agent's fallback model chain. pub fn update_fallback_models( &self, diff --git a/crates/openfang-runtime/src/model_catalog.rs b/crates/openfang-runtime/src/model_catalog.rs index 7ffe402a4..6bfa99141 100644 --- a/crates/openfang-runtime/src/model_catalog.rs +++ b/crates/openfang-runtime/src/model_catalog.rs @@ -107,13 +107,21 @@ impl ModelCatalog { &self.models } - /// Find a model by its canonical ID or by alias. + /// Find a model by its canonical ID, display name, or alias. pub fn find_model(&self, id_or_alias: &str) -> Option<&ModelCatalogEntry> { let lower = id_or_alias.to_lowercase(); // Direct ID match first if let Some(entry) = self.models.iter().find(|m| m.id.to_lowercase() == lower) { return Some(entry); } + // Display-name match for dashboard/UI payloads that send labels. + if let Some(entry) = self + .models + .iter() + .find(|m| m.display_name.to_lowercase() == lower) + { + return Some(entry); + } // Alias resolution if let Some(canonical) = self.aliases.get(&lower) { return self.models.iter().find(|m| m.id == *canonical); @@ -3938,6 +3946,14 @@ mod tests { assert_eq!(entry.provider, "xai"); } + #[test] + fn test_find_model_by_display_name() { + let catalog = ModelCatalog::new(); + let entry = catalog.find_model("Grok 4").unwrap(); + assert_eq!(entry.id, "grok-4-0709"); + assert_eq!(entry.provider, "xai"); + } + #[test] fn test_new_providers_in_catalog() { let catalog = ModelCatalog::new();