fix(config): preserve built-in reserved-slug cloud_providers across settings saves (#2457)

This commit is contained in:
YellowSnnowmann
2026-05-22 17:40:09 -07:00
committed by GitHub
parent 9b2eb90758
commit b0facb5e78
6 changed files with 252 additions and 18 deletions
+22
View File
@@ -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;
+37
View File
@@ -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();
+125
View File
@@ -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();
+26 -6
View File
@@ -910,8 +910,34 @@ fn handle_update_model_settings(params: Map<String, Value>) -> 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<String, Value>) -> 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()
+26 -6
View File
@@ -556,19 +556,39 @@ fn handle_inference_update_model_settings(params: Map<String, Value>) -> 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()
+16 -6
View File
@@ -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