From b0facb5e7828345951cddcb92319ee14e3ae09db Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Sat, 23 May 2026 06:10:09 +0530 Subject: [PATCH] fix(config): preserve built-in reserved-slug cloud_providers across settings saves (#2457) --- app/src/lib/i18n/chunks/de-5.ts | 22 +++++ src/openhuman/config/ops.rs | 37 +++++++++ src/openhuman/config/ops_tests.rs | 125 +++++++++++++++++++++++++++++ src/openhuman/config/schemas.rs | 32 ++++++-- src/openhuman/inference/schemas.rs | 32 ++++++-- tests/json_rpc_e2e.rs | 22 +++-- 6 files changed, 252 insertions(+), 18 deletions(-) diff --git a/app/src/lib/i18n/chunks/de-5.ts b/app/src/lib/i18n/chunks/de-5.ts index c8a26af5f..79f041cc1 100644 --- a/app/src/lib/i18n/chunks/de-5.ts +++ b/app/src/lib/i18n/chunks/de-5.ts @@ -523,6 +523,28 @@ const de5: TranslationMap = { 'settings.mascot.colorYellow': 'Gelb', 'settings.mascot.libraryUnavailable': 'OpenHuman Bibliothek nicht verfügbar', 'settings.mascot.title': 'OpenHuman', + 'settings.developerMenu.mcpServer.title': 'MCP-Server', + 'settings.developerMenu.mcpServer.desc': + 'Externe MCP-Clients zur Verbindung mit OpenHuman konfigurieren', + 'settings.mcpServer.title': 'MCP-Server', + 'settings.mcpServer.toolsSectionTitle': 'Verfügbare Tools', + 'settings.mcpServer.toolsSectionDesc': + 'Tools, die über den MCP-Stdio-Server bereitgestellt werden, wenn openhuman-core mcp ausgeführt wird', + 'settings.mcpServer.configSectionTitle': 'Client-Konfiguration', + 'settings.mcpServer.configSectionDesc': + 'Wählen Sie Ihren MCP-Client aus, um den passenden Konfigurations-Schnipsel zu erzeugen', + 'settings.mcpServer.copySnippet': 'In Zwischenablage kopieren', + 'settings.mcpServer.copied': 'Kopiert!', + 'settings.mcpServer.openConfigFile': 'Konfigurationsdatei öffnen', + 'settings.mcpServer.binaryPathNotFound': + 'OpenHuman-Binary nicht gefunden. Wenn Sie aus dem Quellcode arbeiten, bauen Sie mit: cargo build --bin openhuman-core', + 'settings.mcpServer.openConfigError': 'Konfigurationsdatei konnte nicht geöffnet werden', + 'settings.mcpServer.clientClaudeDesktop': 'Claude Desktop', + 'settings.mcpServer.clientCursor': 'Cursor', + 'settings.mcpServer.clientCodex': 'Codex', + 'settings.mcpServer.clientZed': 'Zed', + 'settings.mcpServer.configFilePath': 'Konfigurationsdatei', + 'settings.mcpServer.clientSelectorAriaLabel': 'MCP-Client-Auswahl', }; export default de5; diff --git a/src/openhuman/config/ops.rs b/src/openhuman/config/ops.rs index 2bb2b727d..23194da35 100644 --- a/src/openhuman/config/ops.rs +++ b/src/openhuman/config/ops.rs @@ -487,7 +487,44 @@ pub async fn apply_model_settings( config.model_routes = routes; } if let Some(providers) = update.cloud_providers { + // The schema handlers strip reserved-slug entries (e.g. the built-in + // "openhuman" provider seeded by `migrations::unify_ai_provider_settings`) + // from the user's payload. Preserve any reserved-slug entries that + // already live in the stored config so a routine settings save + // doesn't accidentally delete them — `primary_cloud` and the + // per-workload routing fields can reference these built-ins, and + // losing them would break inference routing. + use crate::openhuman::config::schema::cloud_providers::is_slug_reserved; + let preserved: Vec<_> = config + .cloud_providers + .iter() + .filter(|e| is_slug_reserved(e.slug.trim())) + .cloned() + .collect(); + log::debug!( + "[config] apply_model_settings: preserving {} reserved cloud provider(s) before overwrite", + preserved.len() + ); config.cloud_providers = providers; + let before_reinject = config.cloud_providers.len(); + for entry in preserved { + // Defensive: don't double-add if the payload (somehow) already + // contained an entry with this reserved slug — the schema-handler + // filter is the canonical guard, but apply_model_settings is also + // reachable from tests and CLI paths that bypass that filter. + let preserved_slug = entry.slug.trim(); + if !config + .cloud_providers + .iter() + .any(|e| e.slug.trim() == preserved_slug) + { + config.cloud_providers.push(entry); + } + } + log::debug!( + "[config] apply_model_settings: reinjected {} reserved cloud provider(s)", + config.cloud_providers.len() - before_reinject + ); } if let Some(primary) = update.primary_cloud { let trimmed = primary.trim(); diff --git a/src/openhuman/config/ops_tests.rs b/src/openhuman/config/ops_tests.rs index 2d1bdc069..6b1f332b2 100644 --- a/src/openhuman/config/ops_tests.rs +++ b/src/openhuman/config/ops_tests.rs @@ -471,6 +471,131 @@ async fn apply_model_settings_empty_strings_clear_optional_fields() { assert!(cfg.default_model.is_none()); } +#[tokio::test] +async fn apply_model_settings_preserves_existing_reserved_slug_cloud_providers() { + // Sentry TAURI-RUST-5 regression. The migration + // `unify_ai_provider_settings` seeds an "openhuman"-slug entry into + // `cloud_providers`. The frontend echoes the full cloud_providers + // list back on every settings save, but the schema handlers filter + // out reserved-slug entries before passing them through. Without + // this preservation step the filtered patch would silently delete + // the built-in entry — losing the `primary_cloud` referent and + // breaking inference routing. + use crate::openhuman::config::schema::cloud_providers::{AuthStyle, CloudProviderCreds}; + + let tmp = tempdir().unwrap(); + let mut cfg = tmp_config(&tmp); + // Simulate the post-migration state: a built-in "openhuman" entry plus + // a user-added custom provider. + cfg.cloud_providers = vec![ + CloudProviderCreds { + id: "openhuman-builtin".into(), + slug: "openhuman".into(), + label: "OpenHuman".into(), + endpoint: "https://api.tinyhumans.ai".into(), + auth_style: AuthStyle::OpenhumanJwt, + default_model: Some("reasoning-v1".into()), + ..Default::default() + }, + CloudProviderCreds { + id: "myopenai-1".into(), + slug: "myopenai".into(), + label: "My OpenAI".into(), + endpoint: "https://api.openai.com".into(), + auth_style: AuthStyle::Bearer, + default_model: Some("gpt-4o".into()), + ..Default::default() + }, + ]; + + // The patch arrives from the schema handler with the "openhuman" + // entry already filtered out (the schema handler drops reserved + // slugs silently). Only the user's custom provider is present, with + // the user's edit applied. + let patch = ModelSettingsPatch { + cloud_providers: Some(vec![CloudProviderCreds { + id: "myopenai-1".into(), + slug: "myopenai".into(), + label: "My OpenAI (edited)".into(), + endpoint: "https://api.openai.com/v1".into(), + auth_style: AuthStyle::Bearer, + default_model: Some("gpt-4o-mini".into()), + ..Default::default() + }]), + ..Default::default() + }; + + let _ = apply_model_settings(&mut cfg, patch).await.expect("apply"); + + // The user's edit is applied. + let myopenai = cfg + .cloud_providers + .iter() + .find(|e| e.slug == "myopenai") + .expect("myopenai entry survives"); + assert_eq!(myopenai.label, "My OpenAI (edited)"); + assert_eq!(myopenai.default_model.as_deref(), Some("gpt-4o-mini")); + + // And the built-in "openhuman" entry is still there. + let openhuman = cfg + .cloud_providers + .iter() + .find(|e| e.slug == "openhuman") + .expect("openhuman built-in must be preserved across saves"); + assert_eq!(openhuman.id, "openhuman-builtin"); + assert_eq!(openhuman.endpoint, "https://api.tinyhumans.ai"); +} + +#[tokio::test] +async fn apply_model_settings_does_not_double_add_reserved_entries() { + // Defensive: if a caller bypasses the schema handler (CLI / tests) and + // includes a reserved-slug entry in the patch, the preservation logic + // must not double-add it. + use crate::openhuman::config::schema::cloud_providers::{AuthStyle, CloudProviderCreds}; + + let tmp = tempdir().unwrap(); + let mut cfg = tmp_config(&tmp); + cfg.cloud_providers = vec![CloudProviderCreds { + id: "openhuman-stored".into(), + slug: "openhuman".into(), + label: "OpenHuman (stored)".into(), + endpoint: "https://api.tinyhumans.ai".into(), + auth_style: AuthStyle::OpenhumanJwt, + default_model: Some("reasoning-v1".into()), + ..Default::default() + }]; + + let patch = ModelSettingsPatch { + cloud_providers: Some(vec![CloudProviderCreds { + id: "openhuman-from-patch".into(), + slug: "openhuman".into(), + label: "OpenHuman (from patch)".into(), + endpoint: "https://api.tinyhumans.ai".into(), + auth_style: AuthStyle::OpenhumanJwt, + default_model: Some("reasoning-v1".into()), + ..Default::default() + }]), + ..Default::default() + }; + + let _ = apply_model_settings(&mut cfg, patch).await.expect("apply"); + + // Exactly one "openhuman" entry survives; the patch's version wins + // (since it was already in `providers` before preservation ran). + let count = cfg + .cloud_providers + .iter() + .filter(|e| e.slug == "openhuman") + .count(); + assert_eq!(count, 1, "no duplicate reserved-slug entries"); + let entry = cfg + .cloud_providers + .iter() + .find(|e| e.slug == "openhuman") + .unwrap(); + assert_eq!(entry.id, "openhuman-from-patch"); +} + #[tokio::test] async fn apply_memory_settings_updates_all_provided_fields() { let tmp = tempdir().unwrap(); diff --git a/src/openhuman/config/schemas.rs b/src/openhuman/config/schemas.rs index d8aa47085..51a61c538 100644 --- a/src/openhuman/config/schemas.rs +++ b/src/openhuman/config/schemas.rs @@ -910,8 +910,34 @@ fn handle_update_model_settings(params: Map) -> ControllerFuture generate_provider_id, is_slug_reserved, migrate_legacy_fields, AuthStyle, CloudProviderCreds, }; + let reserved_count = entries + .iter() + .filter(|e| { + let t = e.slug.trim(); + !t.is_empty() && is_slug_reserved(t) + }) + .count(); + if reserved_count > 0 { + log::debug!( + "[config] update_model_settings: dropping {} reserved cloud provider slug(s)", + reserved_count + ); + } entries .into_iter() + // Silently drop entries whose (non-empty) slug is reserved — + // typically the migration-seeded "openhuman" / "cloud" / + // "pid" built-ins that the frontend echoes back on every + // save (see `migrations::unify_ai_provider_settings`). + // Empty slugs still fall through so the explicit + // validation error below fires for actual frontend + // bugs. `apply_model_settings` re-injects the existing + // reserved entries from the stored config so they + // aren't dropped on save. + .filter(|e| { + let trimmed = e.slug.trim(); + trimmed.is_empty() || !is_slug_reserved(trimmed) + }) .map(|e| { let slug = e.slug.trim().to_string(); if slug.is_empty() { @@ -919,12 +945,6 @@ fn handle_update_model_settings(params: Map) -> ControllerFuture "cloud provider slug must not be empty".to_string() ); } - if is_slug_reserved(&slug) { - return Err(format!( - "slug '{}' is reserved and cannot be used for a custom provider", - slug - )); - } let auth_style = match e .auth_style .as_deref() diff --git a/src/openhuman/inference/schemas.rs b/src/openhuman/inference/schemas.rs index 70f70b9f6..c4f792c5a 100644 --- a/src/openhuman/inference/schemas.rs +++ b/src/openhuman/inference/schemas.rs @@ -556,19 +556,39 @@ fn handle_inference_update_model_settings(params: Map) -> Control generate_provider_id, is_slug_reserved, migrate_legacy_fields, AuthStyle, CloudProviderCreds, }; + let reserved_count = entries + .iter() + .filter(|e| { + let t = e.slug.trim(); + !t.is_empty() && is_slug_reserved(t) + }) + .count(); + if reserved_count > 0 { + log::debug!( + "[inference] update_model_settings: dropping {} reserved cloud provider slug(s)", + reserved_count + ); + } entries .into_iter() + // Silently drop entries whose (non-empty) slug is reserved — + // typically the migration-seeded "openhuman" / "cloud" / + // "pid" built-ins that the frontend echoes back on every + // save (see `migrations::unify_ai_provider_settings`). + // Empty slugs still fall through so the explicit + // validation error below fires for actual frontend + // bugs. `apply_model_settings` re-injects the existing + // reserved entries from the stored config so they + // aren't dropped on save. + .filter(|entry| { + let trimmed = entry.slug.trim(); + trimmed.is_empty() || !is_slug_reserved(trimmed) + }) .map(|entry| { let slug = entry.slug.trim().to_string(); if slug.is_empty() { return Err("cloud provider slug must not be empty".to_string()); } - if is_slug_reserved(&slug) { - return Err(format!( - "slug '{}' is reserved and cannot be used for a custom provider", - slug - )); - } let auth_style = match entry .auth_style .as_deref() diff --git a/tests/json_rpc_e2e.rs b/tests/json_rpc_e2e.rs index fa1534fa1..c4d432363 100644 --- a/tests/json_rpc_e2e.rs +++ b/tests/json_rpc_e2e.rs @@ -2329,12 +2329,22 @@ async fn json_rpc_web_chat_custom_chat_provider_with_auth_none_omits_auth_header config.get("chat_provider").and_then(Value::as_str), Some("proxy:gpt-oss") ); - assert_eq!( - config - .get("cloud_providers") - .and_then(Value::as_array) - .map(Vec::len), - Some(1) + // The user's "proxy" entry must survive the update. We intentionally + // assert by slug rather than by raw length — the + // `unify_ai_provider_settings` migration seeds a built-in "openhuman" + // cloud_providers entry on fresh configs, and `apply_model_settings` + // preserves reserved-slug built-ins across updates (Sentry TAURI-RUST-5 + // fix). A raw length assertion would couple this test to the count of + // built-ins, which is a separate concern. + let providers = config + .get("cloud_providers") + .and_then(Value::as_array) + .expect("cloud_providers should be present in config snapshot"); + assert!( + providers + .iter() + .any(|e| e.get("slug").and_then(Value::as_str) == Some("proxy")), + "user's auth-none 'proxy' entry must survive the update: {providers:?}" ); let loaded_config = openhuman_core::openhuman::config::load_config_with_timeout() .await