Restore template Codex provider id when backfilling live config

Backfill writes the current Codex live config back to the previous
provider's stored template after a switch. Because the live file now
carries a normalized stable model_provider id, the previous provider's
template would lose its own provider-specific id (and any matching
[profiles.*] references) on every subsequent switch.

Reverse the normalization at backfill time by rewriting model_provider,
the active model_providers section, and matching profile references back
to the template's original id.
This commit is contained in:
Jason
2026-04-30 09:50:31 +08:00
parent 10c0ad49bc
commit e8bf2c4090
3 changed files with 264 additions and 17 deletions

View File

@@ -178,6 +178,29 @@ fn stable_codex_model_provider_id_from_config(config_text: &str) -> Option<Strin
}
}
fn codex_model_provider_id_with_table_from_config(
config_text: &str,
) -> Result<Option<String>, AppError> {
if config_text.trim().is_empty() {
return Ok(None);
}
let doc = config_text
.parse::<DocumentMut>()
.map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?;
let Some(provider_id) = active_codex_model_provider_id(&doc) else {
return Ok(None);
};
let has_provider_table = doc
.get("model_providers")
.and_then(|item| item.as_table())
.and_then(|table| table.get(provider_id.as_str()))
.is_some();
Ok(has_provider_table.then_some(provider_id))
}
fn normalize_codex_live_config_model_provider_with_anchors<'a>(
config_text: &str,
anchor_config_texts: impl IntoIterator<Item = &'a str>,
@@ -295,6 +318,77 @@ pub fn normalize_codex_settings_config_model_provider(
Ok(())
}
fn restore_codex_backfill_model_provider_id(
config_text: &str,
template_config_text: &str,
) -> Result<String, AppError> {
let Some(template_provider_id) =
codex_model_provider_id_with_table_from_config(template_config_text)?
else {
return Ok(config_text.to_string());
};
if config_text.trim().is_empty() {
return Ok(config_text.to_string());
}
let mut doc = config_text
.parse::<DocumentMut>()
.map_err(|e| AppError::Message(format!("Invalid Codex config.toml: {e}")))?;
let Some(live_provider_id) = active_codex_model_provider_id(&doc) else {
return Ok(config_text.to_string());
};
if live_provider_id == template_provider_id {
return Ok(config_text.to_string());
}
if let Some(model_providers) = doc
.get_mut("model_providers")
.and_then(|item| item.as_table_mut())
{
let Some(provider_table) = model_providers.remove(live_provider_id.as_str()) else {
return Ok(config_text.to_string());
};
model_providers[template_provider_id.as_str()] = provider_table;
} else {
return Ok(config_text.to_string());
}
rewrite_codex_profile_model_provider_refs(&mut doc, &live_provider_id, &template_provider_id);
doc["model_provider"] = toml_edit::value(template_provider_id.as_str());
Ok(doc.to_string())
}
/// Convert a Codex live config that was normalized for history stability back
/// to the provider-specific id used by the stored provider template.
pub fn restore_codex_settings_config_model_provider_for_backfill(
settings: &mut Value,
template_settings: &Value,
) -> Result<(), AppError> {
let Some(config_text) = settings
.get("config")
.and_then(|value| value.as_str())
.map(str::to_string)
else {
return Ok(());
};
let Some(template_config_text) = template_settings
.get("config")
.and_then(|value| value.as_str())
else {
return Ok(());
};
let restored = restore_codex_backfill_model_provider_id(&config_text, template_config_text)?;
if let Some(obj) = settings.as_object_mut() {
obj.insert("config".to_string(), Value::String(restored));
}
Ok(())
}
/// Atomically write Codex live config after normalizing provider-specific ids.
///
/// Use this for provider-driven live writes. Keep `write_codex_live_atomic` available

View File

@@ -530,29 +530,55 @@ pub(crate) fn strip_common_config_from_live_settings(
app_type.as_str(),
provider.id
);
return live_settings;
return restore_live_settings_for_provider_backfill(app_type, provider, live_settings);
}
};
if !provider_uses_common_config(app_type, provider, snippet.as_deref()) {
return live_settings;
}
let Some(snippet_text) = snippet.as_deref() else {
return live_settings;
let backfill_settings = if provider_uses_common_config(app_type, provider, snippet.as_deref()) {
match snippet.as_deref() {
Some(snippet_text) => {
match remove_common_config_from_settings(app_type, &live_settings, snippet_text) {
Ok(settings) => settings,
Err(err) => {
log::warn!(
"Failed to strip common config for {} provider '{}': {err}",
app_type.as_str(),
provider.id
);
live_settings
}
}
}
None => live_settings,
}
} else {
live_settings
};
match remove_common_config_from_settings(app_type, &live_settings, snippet_text) {
Ok(settings) => settings,
Err(err) => {
log::warn!(
"Failed to strip common config for {} provider '{}': {err}",
app_type.as_str(),
provider.id
);
live_settings
}
restore_live_settings_for_provider_backfill(app_type, provider, backfill_settings)
}
fn restore_live_settings_for_provider_backfill(
app_type: &AppType,
provider: &Provider,
live_settings: Value,
) -> Value {
if !matches!(app_type, AppType::Codex) {
return live_settings;
}
let mut settings = live_settings;
if let Err(err) = crate::codex_config::restore_codex_settings_config_model_provider_for_backfill(
&mut settings,
&provider.settings_config,
) {
log::warn!(
"Failed to restore Codex provider id while backfilling '{}': {err}",
provider.id
);
}
settings
}
pub(crate) fn normalize_provider_common_config_for_storage(

View File

@@ -347,6 +347,133 @@ requires_openai_auth = true
);
}
#[test]
fn provider_service_switch_codex_backfill_keeps_provider_specific_model_provider_id() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let _home = ensure_test_home();
let legacy_auth = json!({ "OPENAI_API_KEY": "rightcode-key" });
let provider_a_config = r#"model_provider = "rightcode"
model = "gpt-5.4"
[model_providers.rightcode]
name = "RightCode"
base_url = "https://rightcode.example/v1"
wire_api = "responses"
requires_openai_auth = true
"#;
write_codex_live_atomic(&legacy_auth, Some(provider_a_config))
.expect("seed existing codex live config");
let mut initial_config = MultiAppConfig::default();
{
let manager = initial_config
.get_manager_mut(&AppType::Codex)
.expect("codex manager");
manager.current = "provider-a".to_string();
manager.providers.insert(
"provider-a".to_string(),
Provider::with_id(
"provider-a".to_string(),
"RightCode".to_string(),
json!({
"auth": {"OPENAI_API_KEY": "rightcode-key"},
"config": provider_a_config
}),
None,
),
);
manager.providers.insert(
"provider-b".to_string(),
Provider::with_id(
"provider-b".to_string(),
"AiHubMix".to_string(),
json!({
"auth": {"OPENAI_API_KEY": "aihubmix-key"},
"config": r#"model_provider = "aihubmix"
model = "gpt-5.4"
profile = "work"
[model_providers.aihubmix]
name = "AiHubMix"
base_url = "https://aihubmix.example/v1"
wire_api = "responses"
requires_openai_auth = true
[profiles.work]
model_provider = "aihubmix"
model = "gpt-5.4"
"#
}),
None,
),
);
manager.providers.insert(
"provider-c".to_string(),
Provider::with_id(
"provider-c".to_string(),
"Vendor C".to_string(),
json!({
"auth": {"OPENAI_API_KEY": "vendor-c-key"},
"config": r#"model_provider = "vendor_c"
model = "gpt-5.4"
[model_providers.vendor_c]
name = "Vendor C"
base_url = "https://vendor-c.example/v1"
wire_api = "responses"
requires_openai_auth = true
"#
}),
None,
),
);
}
let state = create_test_state_with_config(&initial_config).expect("create test state");
ProviderService::switch(&state, AppType::Codex, "provider-b")
.expect("switch to provider b should succeed");
ProviderService::switch(&state, AppType::Codex, "provider-c")
.expect("switch to provider c should succeed");
let providers = state
.db
.get_all_providers(AppType::Codex.as_str())
.expect("read providers after switches");
let provider_b_config = providers
.get("provider-b")
.expect("provider b exists")
.settings_config
.get("config")
.and_then(|v| v.as_str())
.expect("provider b config");
let parsed: toml::Value = toml::from_str(provider_b_config).expect("parse provider b config");
assert_eq!(
parsed.get("model_provider").and_then(|v| v.as_str()),
Some("aihubmix"),
"backfill should restore provider b's storage-specific model_provider id"
);
assert!(
parsed
.get("model_providers")
.and_then(|v| v.get("aihubmix"))
.is_some(),
"provider b should keep its own model_providers table after backfill"
);
assert_eq!(
parsed
.get("profiles")
.and_then(|v| v.get("work"))
.and_then(|v| v.get("model_provider"))
.and_then(|v| v.as_str()),
Some("aihubmix"),
"profile overrides should be restored to provider b's storage-specific id"
);
}
#[test]
fn sync_current_provider_for_app_keeps_live_takeover_and_updates_restore_backup() {
let _guard = test_mutex().lock().expect("acquire test mutex");