Fix Codex startup live import duplication (#2590)

* Fix Codex startup live import duplication

* Fix: Prevent duplicate Codex default provider on restart & add startup import tests
This commit is contained in:
Dhruv_S
2026-05-07 17:41:50 -07:00
committed by GitHub
parent f5fbcd0493
commit b05be92aa1
5 changed files with 190 additions and 4 deletions
+16
View File
@@ -535,6 +535,22 @@ impl Database {
Ok(ids)
}
/// 判断指定 app 下是否已存在任意 provider。
///
/// 启动阶段的 live import 需要使用这个更严格的判断:
/// 只要该 app 已经有任何 provider(包括官方 seed),就不应再自动导入 `default`。
pub fn has_any_provider_for_app(&self, app_type: &str) -> Result<bool, AppError> {
let conn = lock_conn!(self.conn);
let exists: bool = conn
.query_row(
"SELECT EXISTS(SELECT 1 FROM providers WHERE app_type = ?1)",
params![app_type],
|row| row.get(0),
)
.map_err(|e| AppError::Database(e.to_string()))?;
Ok(exists)
}
/// 判断指定 app 下是否存在非官方种子的供应商。
///
/// 比 `get_all_providers` 轻量得多:只读 id 列、无 endpoint 子查询、首条命中即返回。
+13
View File
@@ -494,6 +494,19 @@ pub fn run() {
for app_type in
crate::app_config::AppType::all().filter(|t| !t.is_additive_mode())
{
if !crate::services::provider::should_import_default_config_on_startup(
&app_state,
&app_type,
)
.unwrap_or(false)
{
log::debug!(
"○ {} already has providers; live import skipped",
app_type.as_str()
);
continue;
}
match crate::services::provider::import_default_config(
&app_state,
app_type.clone(),
+17
View File
@@ -1129,10 +1129,27 @@ pub fn import_default_config(state: &AppState, app_type: AppType) -> Result<bool
state
.db
.set_current_provider(app_type.as_str(), &provider.id)?;
crate::settings::set_current_provider(&app_type, Some(provider.id.as_str()))?;
Ok(true) // 真正导入了
}
/// Decide whether startup should auto-import the current live config as `default`.
///
/// This is intentionally stricter than the manual import path:
/// if the app already has any provider row at all (including official seeds),
/// startup must skip auto-import to avoid recreating `default` on each launch.
pub fn should_import_default_config_on_startup(
state: &AppState,
app_type: &AppType,
) -> Result<bool, AppError> {
if app_type.is_additive_mode() {
return Ok(false);
}
Ok(!state.db.has_any_provider_for_app(app_type.as_str())?)
}
/// Write Gemini live configuration with authentication handling
pub(crate) fn write_gemini_live(provider: &Provider) -> Result<(), AppError> {
use crate::gemini_config::{
+9 -1
View File
@@ -22,7 +22,8 @@ use crate::store::AppState;
// Re-export sub-module functions for external access
pub use live::{
import_default_config, import_hermes_providers_from_live, import_openclaw_providers_from_live,
import_opencode_providers_from_live, read_live_settings, sync_current_to_live,
import_opencode_providers_from_live, read_live_settings,
should_import_default_config_on_startup, sync_current_to_live,
};
// Internal re-exports (pub(crate))
@@ -1933,6 +1934,13 @@ impl ProviderService {
import_default_config(state, app_type)
}
pub fn should_import_default_config_on_startup(
state: &AppState,
app_type: &AppType,
) -> Result<bool, AppError> {
should_import_default_config_on_startup(state, app_type)
}
/// Read current live settings (re-export)
pub fn read_live_settings(app_type: AppType) -> Result<Value, AppError> {
read_live_settings(app_type)
+135 -3
View File
@@ -1,14 +1,146 @@
use serde_json::json;
use std::path::{Path, PathBuf};
use cc_switch_lib::{
get_codex_auth_path, get_codex_config_path, read_json_file, switch_provider_test_hook,
write_codex_live_atomic, AppError, AppType, McpApps, McpServer, MultiAppConfig, Provider,
get_codex_auth_path, get_codex_config_path, import_default_config_test_hook, read_json_file,
switch_provider_test_hook, write_codex_live_atomic, AppError, AppType, McpApps, McpServer,
MultiAppConfig, Provider, ProviderService,
};
#[path = "support.rs"]
mod support;
use std::collections::HashMap;
use support::{create_test_state_with_config, ensure_test_home, reset_test_fs, test_mutex};
use support::{
create_test_state, create_test_state_with_config, ensure_test_home, reset_test_fs, test_mutex,
};
fn settings_path(home: &Path) -> PathBuf {
home.join(".cc-switch").join("settings.json")
}
#[test]
fn codex_startup_import_fresh_install_imports_once_and_syncs_current_setting() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let home = ensure_test_home();
let auth = json!({"OPENAI_API_KEY": "fresh-key"});
let config = r#"model = "gpt-5"
"#;
write_codex_live_atomic(&auth, Some(config)).expect("seed codex live config");
let state = create_test_state().expect("create test state");
assert!(
ProviderService::should_import_default_config_on_startup(&state, &AppType::Codex)
.expect("check startup import eligibility"),
"empty Codex provider set should import on startup"
);
import_default_config_test_hook(&state, AppType::Codex).expect("import codex default");
let providers = state
.db
.get_all_providers(AppType::Codex.as_str())
.expect("get codex providers after import");
assert_eq!(
providers.len(),
1,
"fresh install import should create exactly one Codex provider before seeding"
);
assert!(
providers.contains_key("default"),
"fresh install import should create default provider"
);
let current_id = state
.db
.get_current_provider(AppType::Codex.as_str())
.expect("get codex current provider");
assert_eq!(current_id.as_deref(), Some("default"));
let settings: serde_json::Value = serde_json::from_str(
&std::fs::read_to_string(settings_path(home)).expect("read settings.json"),
)
.expect("parse settings.json");
assert_eq!(
settings
.get("currentProviderCodex")
.and_then(|value| value.as_str()),
Some("default"),
"live import should also sync device-local currentProviderCodex"
);
state
.db
.init_default_official_providers()
.expect("seed official providers");
let providers_after_seed = state
.db
.get_all_providers(AppType::Codex.as_str())
.expect("get codex providers after seed");
assert_eq!(
providers_after_seed.len(),
2,
"official seeding should add codex-official alongside imported default"
);
assert!(providers_after_seed.contains_key("codex-official"));
assert!(
!ProviderService::should_import_default_config_on_startup(&state, &AppType::Codex)
.expect("re-check startup import eligibility"),
"subsequent startup should skip once Codex already has providers"
);
}
#[test]
fn codex_startup_import_skips_when_only_official_seed_exists() {
let _guard = test_mutex().lock().expect("acquire test mutex");
reset_test_fs();
let _home = ensure_test_home();
let auth = json!({"OPENAI_API_KEY": "fresh-key"});
let config = r#"model = "gpt-5"
"#;
write_codex_live_atomic(&auth, Some(config)).expect("seed codex live config");
let state = create_test_state().expect("create test state");
state
.db
.init_default_official_providers()
.expect("seed official providers");
let providers_before = state
.db
.get_all_providers(AppType::Codex.as_str())
.expect("get codex providers before restart check");
assert_eq!(
providers_before.len(),
1,
"fixture should start with only codex-official present"
);
assert!(providers_before.contains_key("codex-official"));
assert!(
!ProviderService::should_import_default_config_on_startup(&state, &AppType::Codex)
.expect("check startup import eligibility"),
"startup should skip import when codex-official already exists"
);
let providers_after = state
.db
.get_all_providers(AppType::Codex.as_str())
.expect("get codex providers after restart check");
assert_eq!(
providers_after.len(),
providers_before.len(),
"skipping startup import should not grow the Codex provider set"
);
assert!(
!providers_after.contains_key("default"),
"restart path should not create a new default provider"
);
}
#[test]
fn switch_provider_updates_codex_live_and_state() {