From 6d66983115b1f0b35a1aebe21f59fcca4f5f9163 Mon Sep 17 00:00:00 2001 From: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Date: Wed, 19 Aug 2026 08:11:08 -0700 Subject: [PATCH] fix: migrate global-only openai credentials Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> --- .../src/migration/openai_credentials.rs | 109 +++++++++++++++--- .../src/migration/openai_credentials_tests.rs | 61 ++++++++++ 2 files changed, 156 insertions(+), 14 deletions(-) diff --git a/desktop/src-tauri/src/migration/openai_credentials.rs b/desktop/src-tauri/src/migration/openai_credentials.rs index 37648a186c..89b0601b7d 100644 --- a/desktop/src-tauri/src/migration/openai_credentials.rs +++ b/desktop/src-tauri/src/migration/openai_credentials.rs @@ -46,7 +46,7 @@ struct ResolvedValue { #[derive(Clone, Debug)] struct Consumer { label: String, - record_index: usize, + record_index: Option, provider: String, provider_owner: Owner, identity: Identity, @@ -238,6 +238,52 @@ fn classify_endpoint(endpoint: Option<&str>) -> Result { fn collect_consumers(store: &Store) -> Result, String> { let defs = definitions(store); let mut consumers = Vec::new(); + if let Some(provider) = store + .global + .provider + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .filter(|value| { + matches!( + value.to_ascii_lowercase().as_str(), + "openai" | "openai-compat" + ) + }) + { + let endpoint = store + .global + .env_vars + .get(OPENAI_COMPAT_BASE_URL) + .map(|value| ResolvedValue { + value: value.clone(), + owner: Owner::Global, + }); + consumers.push(Consumer { + label: "global defaults".into(), + record_index: None, + provider: provider.to_ascii_lowercase(), + provider_owner: Owner::Global, + identity: classify_endpoint(endpoint.as_ref().map(|value| value.value.as_str()))?, + endpoint, + legacy_key: store + .global + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(|value| ResolvedValue { + value: value.clone(), + owner: Owner::Global, + }), + official_key: store + .global + .env_vars + .get(OPENAI_API_KEY) + .map(|value| ResolvedValue { + value: value.clone(), + owner: Owner::Global, + }), + }); + } for (index, record) in store.records.iter().enumerate() { let def = selected_definition(record, &defs); if record.persona_id.is_some() && def.is_none() { @@ -298,7 +344,7 @@ fn collect_consumers(store: &Store) -> Result, String> { .clone() .or_else(|| (!record.pubkey.is_empty()).then(|| record.pubkey.clone())) .unwrap_or_else(|| format!("record-{index}")), - record_index: index, + record_index: Some(index), provider: provider.to_ascii_lowercase(), provider_owner, identity, @@ -307,6 +353,13 @@ fn collect_consumers(store: &Store) -> Result, String> { official_key: resolve_key(&resolved_layers, OPENAI_API_KEY), }); } + if consumers.is_empty() + && [OPENAI_COMPAT_API_KEY, OPENAI_COMPAT_BASE_URL] + .iter() + .any(|key| store.global.env_vars.contains_key(*key)) + { + return Err("global OpenAI-compatible state has no provider owner".into()); + } Ok(consumers) } @@ -427,7 +480,7 @@ fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { if legacy.as_ref().is_some_and(|value| value.owner == owner) { let relevant = consumers .iter() - .find(|consumer| consumer.record_index == *index); + .find(|consumer| consumer.record_index == Some(*index)); if relevant.is_none_or(|consumer| consumer.identity != Identity::Official) { return Err(format!( "credential owner {owner:?} is shared with a non-official consumer" @@ -474,7 +527,7 @@ fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { if endpoint.as_ref().is_some_and(|value| value.owner == owner) { let relevant = consumers .iter() - .find(|consumer| consumer.record_index == *index); + .find(|consumer| consumer.record_index == Some(*index)); if relevant.is_none_or(|consumer| consumer.identity != Identity::Official) { return Err(format!( "endpoint owner {owner:?} is shared with a non-official consumer" @@ -531,14 +584,42 @@ fn verify_post_split(before: &[Consumer], after: &Store) -> Result<(), String> { } else { "openai-compat" }; - let config = match resolve_effective_config( - &after.records[old.record_index], - &defs, - &after.global, - ) { - EffectiveConfigResult::Resolved(c) => c, - _ => return Err(format!("consumer {:?} became unresolved", old.label)), - }; + if old.record_index.is_none() { + if new.provider != expected_provider { + return Err(format!( + "consumer {:?} resolves provider {:?}, expected {expected_provider}", + old.label, new.provider + )); + } + let expected_key = if old.identity == Identity::Official { + old.official_key + .as_ref() + .filter(|value| !value.value.is_empty()) + .or(old.legacy_key.as_ref()) + } else { + old.legacy_key.as_ref() + }; + let selected_key = if old.identity == Identity::Official { + new.official_key.as_ref() + } else { + new.legacy_key.as_ref() + }; + if expected_key.map(|value| &value.value) != selected_key.map(|value| &value.value) { + return Err(format!( + "consumer {:?} changed selected credential", + old.label + )); + } + continue; + } + let record_index = old + .record_index + .ok_or("managed consumer has no record index")?; + let config = + match resolve_effective_config(&after.records[record_index], &defs, &after.global) { + EffectiveConfigResult::Resolved(c) => c, + _ => return Err(format!("consumer {:?} became unresolved", old.label)), + }; if config.provider.value.as_deref() != Some(expected_provider) { return Err(format!( "consumer {:?} resolves provider {:?}, expected {expected_provider}", @@ -546,7 +627,7 @@ fn verify_post_split(before: &[Consumer], after: &Store) -> Result<(), String> { )); } let descriptor = crate::managed_agents::resolve_effective_harness_descriptor( - &after.records[old.record_index], + &after.records[record_index], &defs, &after.global, ) @@ -557,7 +638,7 @@ fn verify_post_split(before: &[Consumer], after: &Store) -> Result<(), String> { old.legacy_key.as_ref() }; let effective = crate::managed_agents::resolve_effective_agent_env( - &after.records[old.record_index], + &after.records[record_index], &defs, crate::managed_agents::known_acp_runtime(&descriptor.command), &after.global, diff --git a/desktop/src-tauri/src/migration/openai_credentials_tests.rs b/desktop/src-tauri/src/migration/openai_credentials_tests.rs index 6054b70415..d6d4bc7e17 100644 --- a/desktop/src-tauri/src/migration/openai_credentials_tests.rs +++ b/desktop/src-tauri/src/migration/openai_credentials_tests.rs @@ -75,6 +75,67 @@ fn whole_store_matrix_classifies_origins_and_moves_only_official_credentials() { } } +#[test] +fn global_only_defaults_are_migrated_without_managed_consumers() { + for (origin, expected_provider, expected_key) in [ + (None, "openai", OPENAI_API_KEY), + ( + Some("https://gateway.example/v1"), + "openai-compat", + OPENAI_COMPAT_API_KEY, + ), + ] { + let mut global = GlobalAgentConfig { + provider: Some("openai".into()), + env_vars: BTreeMap::from([(OPENAI_COMPAT_API_KEY.into(), "global-secret".into())]), + ..Default::default() + }; + if let Some(origin) = origin { + global + .env_vars + .insert(OPENAI_COMPAT_BASE_URL.into(), origin.into()); + } + + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let global_path = agents.join("global-agent-config.json"); + std::fs::write(&global_path, serde_json::to_vec(&global).unwrap()).unwrap(); + migrate_pair(agents).unwrap(); + + let migrated: GlobalAgentConfig = + serde_json::from_slice(&std::fs::read(&global_path).unwrap()).unwrap(); + assert_eq!(migrated.provider.as_deref(), Some(expected_provider)); + assert_eq!( + migrated.env_vars.get(expected_key).map(String::as_str), + Some("global-secret") + ); + assert!(agents.join(MANIFEST_FILE).exists()); + assert_eq!(std::fs::read(agents.join(MARKER_FILE)).unwrap(), b"1\n"); + assert!(sibling_path(&global_path, BACKUP_SUFFIX).exists()); + } +} + +#[test] +fn global_only_unowned_legacy_state_creates_no_transaction_artifacts() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let global = GlobalAgentConfig { + env_vars: BTreeMap::from([(OPENAI_COMPAT_API_KEY.into(), "unowned-secret".into())]), + ..Default::default() + }; + let source = serde_json::to_vec(&global).unwrap(); + let global_path = agents.join("global-agent-config.json"); + std::fs::write(&global_path, &source).unwrap(); + + assert!(migrate_pair(agents) + .unwrap_err() + .contains("has no provider owner")); + assert_eq!(std::fs::read(&global_path).unwrap(), source); + assert!(!agents.join(MANIFEST_FILE).exists()); + assert!(!agents.join(MARKER_FILE).exists()); + assert!(!sibling_path(&global_path, BACKUP_SUFFIX).exists()); +} + #[test] fn malformed_origin_fails_before_mutation() { let mut agent = record("agent", "agent");