fix(v8): preserve migrated config and correct provider requests
- Preserve provider credentials, aliases, headers, and network settings during migration - Normalize Claude cloak metadata and repair previously saved invalid types - Use streaming Gemini health probes and honor group and key proxy settings - Retain Codex customizations for temporarily unavailable models - Add regression tests for migration, metadata, proxy routing, and catalog persistence
This commit is contained in:
1 parent
2fd0f73f4c
commit
06fd58d3d8
15 files changed
+433
-35
No files matched your search
@@ -370,7 +370,17 @@ fn save_for_state(
|
||||
state: &mut CatalogState,
|
||||
) -> Result<CatalogEditorSnapshot, String> {
|
||||
let snapshot = snapshot_for_state(runtime_models, state)?;
|
||||
let customizations = customizations_from_request(&snapshot, request)?;
|
||||
let mut customizations = customizations_from_request(&snapshot, request)?;
|
||||
// The public catalog can temporarily omit disabled or unavailable models.
|
||||
// Editing visible models must not erase the saved settings for those IDs.
|
||||
let visible: HashSet<String> = snapshot.models.iter()
|
||||
.map(|model| normalize_id(&model.slug))
|
||||
.collect();
|
||||
for (slug, configuration) in &state.customizations {
|
||||
if !visible.contains(slug) {
|
||||
customizations.insert(slug.clone(), configuration.clone());
|
||||
}
|
||||
}
|
||||
let saved = SavedCustomizations {
|
||||
version: 1,
|
||||
models: customizations,
|
||||
@@ -423,6 +433,52 @@ mod tests {
|
||||
))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saving_visible_models_preserves_unavailable_models_and_allows_visible_reset() {
|
||||
let mut state = CatalogState {
|
||||
sources: parse_sources(MODEL_CATALOG_JSON).unwrap(),
|
||||
json: MODEL_CATALOG_JSON.to_string(),
|
||||
customizations: Default::default(),
|
||||
};
|
||||
let hidden = serde_json::json!({
|
||||
"base_instructions": "Saved prompt for temporarily unavailable A",
|
||||
"context_window": 131_072,
|
||||
"max_context_window": 262_144,
|
||||
}).as_object().unwrap().clone();
|
||||
state.customizations.insert("model-a".to_string(), hidden.clone());
|
||||
let runtime = vec![runtime_model("model-b")];
|
||||
let snapshot = snapshot_for_state(&runtime, &state).unwrap();
|
||||
let mut configuration = snapshot.models[0].configuration.clone();
|
||||
configuration.insert("display_name".to_string(), Value::String("Edited B".to_string()));
|
||||
let path = temporary_path();
|
||||
let saved = save_for_state(&path, &runtime, CatalogEditorRequest {
|
||||
revision: snapshot.revision,
|
||||
models: vec![CatalogEditorModelRequest { slug: "model-b".to_string(), configuration }],
|
||||
}, &mut state).unwrap();
|
||||
let persisted = decode_customizations(&std::fs::read(&path).unwrap()).unwrap();
|
||||
assert_eq!(persisted.get("model-a"), Some(&hidden));
|
||||
assert_eq!(persisted["model-b"]["display_name"], "Edited B");
|
||||
|
||||
save_for_state(&path, &runtime, CatalogEditorRequest {
|
||||
revision: saved.revision,
|
||||
models: vec![CatalogEditorModelRequest {
|
||||
slug: "model-b".to_string(), configuration: saved.models[0].defaults.clone(),
|
||||
}],
|
||||
}, &mut state).unwrap();
|
||||
assert!(!state.customizations.contains_key("model-b"));
|
||||
assert_eq!(state.customizations.get("model-a"), Some(&hidden));
|
||||
|
||||
let empty = snapshot_for_state(&[], &state).unwrap();
|
||||
save_for_state(&path, &[], CatalogEditorRequest {
|
||||
revision: empty.revision, models: vec![],
|
||||
}, &mut state).unwrap();
|
||||
assert_eq!(decode_customizations(&std::fs::read(&path).unwrap()).unwrap().get("model-a"), Some(&hidden));
|
||||
let restored = snapshot_for_state(&[runtime_model("model-a")], &state).unwrap();
|
||||
assert_eq!(restored.models[0].configuration["base_instructions"], hidden["base_instructions"]);
|
||||
assert_eq!(restored.models[0].configuration["context_window"], 131_072);
|
||||
std::fs::remove_file(path).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn customizations_persist_apply_and_restore_to_the_current_template() {
|
||||
let sources = parse_sources(MODEL_CATALOG_JSON).unwrap();
|
||||
|
||||
@@ -65,6 +65,8 @@ const PATHS: &[(&str, &str)] = &[
|
||||
("oauth/providers/xai", "xai"),
|
||||
("server/host", "host"),
|
||||
("server/port", "port"),
|
||||
("server/trusted-proxies", "trusted-proxies"),
|
||||
("server/discovery", "discovery"),
|
||||
("server/tls", "tls"),
|
||||
("server/commercial-mode", "commercial-mode"),
|
||||
("management", "remote-management"),
|
||||
|
||||
@@ -1276,14 +1276,12 @@ fn lift_legacy_core_config_fields(
|
||||
template: &serde_norway::Mapping,
|
||||
) -> Result<(), String> {
|
||||
let copy = |document: &mut serde_norway::Value, legacy_path: &[&str], v8_path: &[&str]| -> Result<(), String> {
|
||||
let Some(value) = nested_yaml_value(legacy, legacy_path).cloned() else {
|
||||
// Read the remaining source: narrower moves may already have consumed
|
||||
// children which now belong elsewhere in the v8 layout.
|
||||
let Some(value) = document.as_mapping().and_then(|root| nested_yaml_value(root, legacy_path)).cloned() else {
|
||||
return Ok(());
|
||||
};
|
||||
// A partially migrated file can contain both spellings. Prefer the
|
||||
// canonical v8 value in that case; otherwise move the legacy value.
|
||||
if nested_yaml_value(legacy, v8_path).is_none() {
|
||||
set_core_yaml_path_value(document, v8_path, value)?;
|
||||
}
|
||||
lift_legacy_config_value(document, legacy, v8_path, value)?;
|
||||
if nested_yaml_value(template, legacy_path).is_none() {
|
||||
remove_core_yaml_path_value(document, legacy_path);
|
||||
}
|
||||
@@ -1292,6 +1290,7 @@ fn lift_legacy_core_config_fields(
|
||||
for (legacy_path, v8_path) in [
|
||||
(&["host"][..], &["server", "host"][..]),
|
||||
(&["port"], &["server", "port"]),
|
||||
(&["tls"], &["server", "tls"]),
|
||||
(&["commercial-mode"], &["server", "commercial-mode"]),
|
||||
(&["auth-dir"], &["oauth", "auth-dir"]),
|
||||
(&["debug"], &["observability", "logs", "debug"]),
|
||||
@@ -1313,14 +1312,6 @@ fn lift_legacy_core_config_fields(
|
||||
] {
|
||||
copy(document, legacy_path, v8_path)?;
|
||||
}
|
||||
if let Some(tls) = yaml_mapping_value(legacy, "tls").filter(|value| value.is_mapping()).cloned() {
|
||||
if nested_yaml_value(legacy, &["server", "tls"]).is_none() {
|
||||
set_core_yaml_path_value(document, &["server", "tls"], tls)?;
|
||||
}
|
||||
if yaml_mapping_value(template, "tls").is_none() {
|
||||
remove_core_yaml_path_value(document, &["tls"]);
|
||||
}
|
||||
}
|
||||
if let Some(keys) = yaml_mapping_value(legacy, "api-keys").filter(|value| value.is_sequence()).cloned() {
|
||||
if nested_yaml_value(legacy, &["access", "api-keys"]).is_none() {
|
||||
set_core_yaml_path_value(document, &["access", "api-keys"], keys)?;
|
||||
@@ -1336,15 +1327,11 @@ fn lift_legacy_core_config_fields(
|
||||
remove_core_yaml_path_value(document, &["api-keys"]);
|
||||
}
|
||||
}
|
||||
let has_canonical_upstream_mapping = yaml_mapping_value(legacy, "api-keys")
|
||||
.is_some_and(serde_norway::Value::is_mapping);
|
||||
for (legacy_name, provider) in V8_PROVIDER_FAMILIES {
|
||||
let Some(records) = yaml_mapping_value(legacy, legacy_name) else {
|
||||
continue;
|
||||
};
|
||||
if !has_canonical_upstream_mapping
|
||||
&& nested_yaml_value(legacy, &["api-keys", provider]).is_none()
|
||||
{
|
||||
if nested_yaml_value(legacy, &["api-keys", provider]).is_none() {
|
||||
let grouped = group_legacy_provider_records(provider, records)?;
|
||||
set_core_yaml_path_value(document, &["api-keys", provider], grouped)?;
|
||||
}
|
||||
@@ -1358,6 +1345,49 @@ fn lift_legacy_core_config_fields(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Structs have independent leaf settings. User-owned maps (aliases, payload
|
||||
// params, headers, etc.) instead retain the canonical value as a whole.
|
||||
fn lift_legacy_config_value(
|
||||
document: &mut serde_norway::Value,
|
||||
original: &serde_norway::Mapping,
|
||||
path: &[&str],
|
||||
value: serde_norway::Value,
|
||||
) -> Result<(), String> {
|
||||
// Explicit canonical null/scalar parents must not be recreated by a
|
||||
// narrower legacy spelling such as claude-header-defaults.
|
||||
if (1..path.len()).any(|length| {
|
||||
nested_yaml_value(original, &path[..length]).is_some_and(|value| !value.is_mapping())
|
||||
}) {
|
||||
return Ok(());
|
||||
}
|
||||
let is_struct = matches!(path,
|
||||
["server", "tls"] | ["server", "discovery"]
|
||||
| ["server", "discovery", "interfaces"] | ["management"]
|
||||
| ["credentials", "concurrency"] | ["credentials", "in-flight"]
|
||||
| ["requests", "streaming"] | ["observability", "pprof"]
|
||||
| ["oauth", "providers", "codex"] | ["oauth", "providers", "claude"]
|
||||
| ["oauth", "providers", "antigravity"] | ["oauth", "providers", "devin"]
|
||||
| ["oauth", "providers", "xai"]
|
||||
| ["oauth", "providers", "codex", "live-media-relay"]
|
||||
| ["oauth", "providers", "codex", "header-defaults"]
|
||||
| ["oauth", "providers", "claude", "header-defaults"]
|
||||
| ["oauth", "providers", "claude", "claude-code"]
|
||||
| ["oauth", "providers", "antigravity", "connection-pool"]
|
||||
);
|
||||
let canonical = nested_yaml_value(original, path);
|
||||
if is_struct && value.is_mapping() && canonical.is_none_or(serde_norway::Value::is_mapping) {
|
||||
for (key, child) in value.as_mapping().unwrap() {
|
||||
let key = key.as_str().ok_or("Configuration field must be a string")?;
|
||||
let mut child_path = path.to_vec();
|
||||
child_path.push(key);
|
||||
lift_legacy_config_value(document, original, &child_path, child.clone())?;
|
||||
}
|
||||
} else if canonical.is_none() {
|
||||
set_core_yaml_path_value(document, path, value)?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn remove_core_yaml_path_value(document: &mut serde_norway::Value, path: &[&str]) -> bool {
|
||||
let Some((key, parents)) = path.split_last() else {
|
||||
return false;
|
||||
|
||||
@@ -26,6 +26,8 @@ pub(crate) struct ProviderHealthProbeRequest {
|
||||
protocol: String,
|
||||
#[serde(default)]
|
||||
source_provider: Option<String>,
|
||||
#[serde(default)]
|
||||
proxy_url: Option<String>,
|
||||
timeout_ms: Option<u64>,
|
||||
#[serde(default)]
|
||||
model: String,
|
||||
@@ -424,6 +426,7 @@ async fn probe_core_model(
|
||||
}).to_string(),
|
||||
protocol: "openai-chat".to_string(),
|
||||
source_provider: None,
|
||||
proxy_url: None,
|
||||
timeout_ms,
|
||||
model,
|
||||
source: String::new(),
|
||||
@@ -447,11 +450,7 @@ pub(crate) async fn provider_health_probe(
|
||||
let url = reqwest::Url::parse(request.url.trim())
|
||||
.map_err(|error| format!("Invalid health check URL: {error}"))?;
|
||||
let config = gui_config_state.snapshot()?;
|
||||
let proxy_url = url
|
||||
.host_str()
|
||||
.filter(|host| !is_loopback_host(host))
|
||||
.map(|_| config.proxy_url.as_str())
|
||||
.unwrap_or_default();
|
||||
let proxy_url = provider_health_proxy_url(&url, request.proxy_url.as_deref(), &config.proxy_url);
|
||||
let client = build_http_client_with_proxy(
|
||||
reqwest::Client::builder()
|
||||
.connect_timeout(Duration::from_secs(10))
|
||||
@@ -462,6 +461,14 @@ pub(crate) async fn provider_health_probe(
|
||||
execute_health_probe(Some(&app), client, request).await
|
||||
}
|
||||
|
||||
fn provider_health_proxy_url<'a>(url: &reqwest::Url, requested: Option<&'a str>, global: &'a str) -> &'a str {
|
||||
if url.host_str().is_some_and(is_loopback_host) {
|
||||
return "";
|
||||
}
|
||||
let proxy = requested.map(str::trim).filter(|value| !value.is_empty()).unwrap_or(global.trim());
|
||||
if proxy.eq_ignore_ascii_case("direct") { "" } else { proxy }
|
||||
}
|
||||
|
||||
async fn execute_health_probe(
|
||||
usage_app: Option<&tauri::AppHandle>,
|
||||
client: reqwest::Client,
|
||||
|
||||
@@ -2,6 +2,34 @@ use super::*;
|
||||
use crate::GuiApiKeyEntry;
|
||||
use tokio::io::{AsyncReadExt, AsyncWriteExt};
|
||||
|
||||
#[test]
|
||||
fn provider_proxy_override_preserves_direct_and_global_inheritance() {
|
||||
let upstream = reqwest::Url::parse("https://provider.invalid/v1/responses").unwrap();
|
||||
let global = "http://127.0.0.1:18080";
|
||||
assert_eq!(provider_health_proxy_url(&upstream, None, global), global);
|
||||
assert_eq!(provider_health_proxy_url(&upstream, Some(" "), global), global);
|
||||
assert_eq!(provider_health_proxy_url(&upstream, Some(" direct "), global), "");
|
||||
assert_eq!(provider_health_proxy_url(&upstream, Some("socks5://127.0.0.1:19080"), global), "socks5://127.0.0.1:19080");
|
||||
let local = reqwest::Url::parse("http://127.0.0.1:8317/v1/responses").unwrap();
|
||||
assert_eq!(provider_health_proxy_url(&local, None, global), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn provider_health_uses_request_proxy_instead_of_global_proxy() {
|
||||
let (port, server) = mock_core_response("200 OK", "text/event-stream", "data: {\"candidates\":[{\"content\":{\"parts\":[{\"text\":\"OK\"}]}}]}\n\n", "").await;
|
||||
let request: ProviderHealthProbeRequest = serde_json::from_value(serde_json::json!({
|
||||
"url": "http://provider.invalid/v1beta/models/test:streamGenerateContent?alt=sse",
|
||||
"header": {}, "data": "{}", "protocol": "gemini",
|
||||
"proxyUrl": format!("http://127.0.0.1:{port}"),
|
||||
})).unwrap();
|
||||
let url = reqwest::Url::parse(&request.url).unwrap();
|
||||
let selected = provider_health_proxy_url(&url, request.proxy_url.as_deref(), "http://127.0.0.1:1");
|
||||
let client = build_http_client_with_proxy(reqwest::Client::builder().timeout(Duration::from_secs(3)), selected, "proxy test").unwrap();
|
||||
assert!(execute_health_probe(None, client, request).await.unwrap().first_token_latency_ms.is_some());
|
||||
let wire_request = server.await.unwrap();
|
||||
assert!(wire_request.starts_with("POST http://provider.invalid/v1beta/models/test:streamGenerateContent?alt=sse HTTP/1.1"));
|
||||
}
|
||||
|
||||
async fn mock_core_response(
|
||||
status: &str,
|
||||
content_type: &str,
|
||||
|
||||
@@ -697,6 +697,85 @@ fn v7_config_startup_with_v8_template_writes_v8_version() {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_migration_preserves_each_legacy_provider_family_independently() {
|
||||
let template = "config-version: 8\napi-keys: {codex: [], claude: []}\n";
|
||||
for marker in ["", "config-version: 7\n"] {
|
||||
let old = format!("{marker}api-keys: {{codex: []}}\ncodex-api-key: [{{api-key: ignored}}]\nclaude-api-key: [{{api-key: fixture, models: [{{name: upstream, alias: claude-alias}}]}}]\n");
|
||||
let migrated = migrate_legacy_core_config_to_v8(template, &old).unwrap();
|
||||
let value: serde_norway::Value = serde_norway::from_str(&migrated).unwrap();
|
||||
assert_eq!(value["api-keys"]["codex"].as_sequence().unwrap().len(), 0);
|
||||
assert_eq!(value["api-keys"]["claude"][0]["keys"][0]["api-key"], "fixture");
|
||||
assert_eq!(value["api-keys"]["claude"][0]["models"][0]["alias"], "claude-alias");
|
||||
assert!(value.get("claude-api-key").is_none());
|
||||
assert!(value.get("codex-api-key").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_migration_preserves_narrow_fields_and_canonical_struct_leaves() {
|
||||
let template = "config-version: 8\nmanagement: {allow-remote: false}\noauth: {providers: {claude: {model-level-cooling: false}, codex: {response-steering: false}}}\n";
|
||||
let legacy = r#"
|
||||
remote-management: {allow-remote: true, secret-key: old-secret}
|
||||
management: {secret-key: canonical-secret}
|
||||
claude-header-defaults: {user-agent: custom-agent, timezone: custom-zone}
|
||||
claude-code: {disable-cloaking-model-list: true}
|
||||
disable-claude-cloak-mode: true
|
||||
claude: {model-level-cooling: true}
|
||||
codex-header-defaults: {user-agent: codex-agent}
|
||||
codex: {response-steering: true, optimize-multi-agent-v2: true}
|
||||
oauth:
|
||||
providers:
|
||||
claude:
|
||||
model-level-cooling: false
|
||||
header-defaults: {timezone: canonical-zone}
|
||||
codex: {response-steering: false}
|
||||
"#;
|
||||
let migrated = migrate_legacy_core_config_to_v8(template, legacy).unwrap();
|
||||
let value: serde_norway::Value = serde_norway::from_str(&migrated).unwrap();
|
||||
let claude = &value["oauth"]["providers"]["claude"];
|
||||
assert_eq!(claude["header-defaults"]["user-agent"], "custom-agent");
|
||||
assert_eq!(claude["header-defaults"]["timezone"], "canonical-zone");
|
||||
assert_eq!(claude["model-level-cooling"].as_bool(), Some(false));
|
||||
assert_eq!(claude["claude-code"]["disable-cloaking-model-list"].as_bool(), Some(true));
|
||||
assert_eq!(claude["disable-claude-cloak-mode"].as_bool(), Some(true));
|
||||
assert_eq!(value["management"]["allow-remote"].as_bool(), Some(true));
|
||||
assert_eq!(value["management"]["secret-key"], "canonical-secret");
|
||||
assert_eq!(value["oauth"]["providers"]["codex"]["header-defaults"]["user-agent"], "codex-agent");
|
||||
assert_eq!(value["oauth"]["providers"]["codex"]["response-steering"].as_bool(), Some(false));
|
||||
assert!(value["oauth"]["providers"]["codex"].get("optimize-multi-agent-v2").is_none());
|
||||
assert_eq!(value["client"]["codex"]["optimize-multi-agent-v2"].as_bool(), Some(true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_migration_preserves_network_settings_over_template_defaults() {
|
||||
let template = "config-version: 8\nserver: {trusted-proxies: [], discovery: {enabled: false}, tls: {enable: false, cert: '', key: ''}}\n";
|
||||
let legacy = "trusted-proxies: [127.0.0.1]\ndiscovery: {enabled: true, service-name: office}\ntls: {enable: true, cert: old-cert, key: old-key}\nserver: {tls: {cert: canonical-cert}}\n";
|
||||
let migrated = migrate_legacy_core_config_to_v8(template, legacy).unwrap();
|
||||
let value: serde_norway::Value = serde_norway::from_str(&migrated).unwrap();
|
||||
assert_eq!(value["server"]["trusted-proxies"][0], "127.0.0.1");
|
||||
assert_eq!(value["server"]["discovery"]["enabled"].as_bool(), Some(true));
|
||||
assert_eq!(value["server"]["discovery"]["service-name"], "office");
|
||||
assert_eq!(value["server"]["tls"]["enable"].as_bool(), Some(true));
|
||||
assert_eq!(value["server"]["tls"]["cert"], "canonical-cert");
|
||||
assert_eq!(value["server"]["tls"]["key"], "old-key");
|
||||
assert!(value.get("trusted-proxies").is_none());
|
||||
assert!(value.get("discovery").is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v8_migration_preserves_explicit_canonical_clears_and_existing_v8_files() {
|
||||
let template = "config-version: 8\nserver: {trusted-proxies: [192.0.2.1]}\n";
|
||||
let old = "trusted-proxies: [127.0.0.1]\nserver: {trusted-proxies: []}\nclaude-header-defaults: {user-agent: legacy}\nclaude: {model-level-cooling: true}\noauth: {providers: {claude: null}, model-alias: {}}\noauth-model-alias: {claude: [{name: original, alias: old-alias}]}\n";
|
||||
let migrated = migrate_legacy_core_config_to_v8(template, old).unwrap();
|
||||
let value: serde_norway::Value = serde_norway::from_str(&migrated).unwrap();
|
||||
assert!(value["server"]["trusted-proxies"].as_sequence().unwrap().is_empty());
|
||||
assert!(value["oauth"]["providers"]["claude"].is_null());
|
||||
assert!(value["oauth"]["model-alias"].as_mapping().unwrap().is_empty());
|
||||
let already_v8 = format!("config-version: 8\n{old}");
|
||||
assert_eq!(migrate_legacy_core_config_to_v8(template, &already_v8).unwrap(), already_v8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn replacing_a_core_rejects_invalid_config_without_overwriting_files() {
|
||||
let root = agent_test_home("core-config-migrate-invalid");
|
||||
|
||||
Reference in new issue
Block a user