From 33f05c6447678e1084c7d7cd0f87ddb2811b9a4d Mon Sep 17 00:00:00 2001 From: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Date: Tue, 18 Aug 2026 15:09:22 -0700 Subject: [PATCH] fix: fail closed on ambiguous openai credential migration Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz> --- crates/buzz-acp/src/setup_mode.rs | 22 +++++ .../src-tauri/src/managed_agents/readiness.rs | 4 +- .../managed_agents/readiness/openai_origin.rs | 11 ++- .../src-tauri/src/managed_agents/runtime.rs | 3 + .../src/migration/openai_credentials.rs | 97 +++++++++++++++---- desktop/src/shared/lib/configNudge.ts | 3 + .../src/shared/ui/config-nudge-attachment.tsx | 17 ++++ 7 files changed, 133 insertions(+), 24 deletions(-) diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea4..74eb0d6df4 100644 --- a/crates/buzz-acp/src/setup_mode.rs +++ b/crates/buzz-acp/src/setup_mode.rs @@ -95,6 +95,8 @@ pub(crate) enum RequirementPayload { NormalizedField { field: String }, /// An env-backed credential that is absent. EnvKey { key: String }, + /// Invalid or conflicting configuration with actionable remediation copy. + ConfigInvalid { message: String }, /// A CLI authentication step that must be completed interactively. CliLogin { probe_args: Vec, @@ -127,6 +129,7 @@ impl RequirementPayload { RequirementPayload::EnvKey { key } => { format!("set `{}` in Edit Agent → Environment variables", key) } + RequirementPayload::ConfigInvalid { message } => message.clone(), RequirementPayload::CliLogin { setup_copy, availability, @@ -725,6 +728,25 @@ mod tests { assert!(body.contains("Fizz"), "nudge body should name the agent"); } + #[test] + fn nudge_body_explains_conflicting_openai_origin() { + let payload = SetupPayload { + agent_name: "OpenAI Agent".to_string(), + agent_pubkey: "test".to_string(), + requirements: vec![RequirementPayload::ConfigInvalid { + message: + "remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`" + .to_string(), + }], + }; + + let body = payload.nudge_body(); + + assert!(body.contains("remove `OPENAI_COMPAT_BASE_URL`")); + assert!(body.contains("switch the provider to `openai-compat`")); + assert!(body.contains(r#""surface":"config_invalid""#)); + } + #[test] fn nudge_body_codex_copy_does_not_mention_openai_api_key() { let payload = SetupPayload { diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index cb25193ea1..91ba4d2881 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -286,7 +286,6 @@ fn resolve_effective_agent_env_with_def( } // ── Requirement types ───────────────────────────────────────────────────────── - /// A single missing piece of configuration, tagged with the UI surface that /// owns it so the UI can route each gap to the right affordance. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -299,11 +298,12 @@ pub enum Requirement { field: String, }, /// An env-backed credential that is absent from the effective env. - /// Routes to the env-var row editor in the Edit Agent dialog. EnvKey { /// The env var key name (e.g. `"ANTHROPIC_API_KEY"`). key: String, }, + /// Invalid configuration with actionable remediation copy. + ConfigInvalid { message: String }, /// A CLI authentication step that must be completed interactively. /// Routes to a setup instruction panel in the Edit Agent dialog. CliLogin { diff --git a/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs b/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs index 0b9004b840..e785b34309 100644 --- a/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs +++ b/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs @@ -11,8 +11,9 @@ pub(super) fn require_safe_official_origin( .filter(|value| !value.is_empty()) .is_none_or(|value| value.trim_end_matches('/') == "https://api.openai.com/v1"); if !safe { - missing.push(Requirement::EnvKey { - key: "OPENAI_COMPAT_BASE_URL".to_string(), + missing.push(Requirement::ConfigInvalid { + message: "remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`" + .to_string(), }); } } @@ -39,8 +40,10 @@ mod tests { assert_eq!( missing, - vec![Requirement::EnvKey { - key: "OPENAI_COMPAT_BASE_URL".to_string() + vec![Requirement::ConfigInvalid { + message: + "remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`" + .to_string() }] ); } diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0ce5ca7b21..23d877e2d5 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -596,6 +596,9 @@ pub fn spawn_agent_child( "surface": "env_key", "key": key, }), + Requirement::ConfigInvalid { message } => serde_json::json!({ + "surface": "config_invalid", "message": message, + }), Requirement::CliLogin { probe_args, setup_copy, diff --git a/desktop/src-tauri/src/migration/openai_credentials.rs b/desktop/src-tauri/src/migration/openai_credentials.rs index ea75ae5485..cfedfe29b3 100644 --- a/desktop/src-tauri/src/migration/openai_credentials.rs +++ b/desktop/src-tauri/src/migration/openai_credentials.rs @@ -95,6 +95,22 @@ struct DefinitionContext { env: Map, } +fn harness_env_for_runtime( + runtime: Option<&str>, + harness_env_by_runtime: &std::collections::HashMap>, +) -> Result, String> { + let Some(runtime) = runtime.map(str::trim).filter(|value| !value.is_empty()) else { + return Ok(Map::new()); + }; + if let Some(env) = harness_env_by_runtime.get(runtime) { + return Ok(env.clone()); + } + if crate::managed_agents::custom_harnesses::check_id_collision(runtime).is_ok() { + return Err(format!("unresolved custom harness {runtime:?}")); + } + Ok(Map::new()) +} + fn sibling_path(path: &Path, suffix: &str) -> PathBuf { let file_name = path .file_name() @@ -201,10 +217,8 @@ fn migrate_openai_credentials_in_file( .filter(|runtime| !runtime.is_empty()) .or(effective_runtime); let merged_inherited_env = { - let mut merged = selected_runtime - .and_then(|runtime| harness_env_by_runtime.get(runtime)) - .cloned() - .unwrap_or_default(); + let mut merged = + harness_env_for_runtime(selected_runtime, harness_env_by_runtime)?; if let Some(inherited) = inherited_env_vars { merged.extend(inherited.clone()); } @@ -222,20 +236,7 @@ fn migrate_openai_credentials_in_file( changed } Value::Object(record) => { - let selected_runtime = record - .get("preferred_runtime") - .and_then(Value::as_str) - .map(str::trim) - .filter(|runtime| !runtime.is_empty()) - .or(effective_runtime); - let merged_inherited_env = selected_runtime - .and_then(|runtime| harness_env_by_runtime.get(runtime)) - .cloned(); - migrate_openai_credential_record( - record, - effective_provider, - merged_inherited_env.as_ref().or(inherited_env_vars), - ) + migrate_openai_credential_record(record, effective_provider, inherited_env_vars) } _ => { return Err(format!( @@ -562,6 +563,66 @@ mod tests { assert!(records[0]["env_vars"].get(OPENAI_API_KEY).is_none()); } + #[test] + fn unresolved_custom_harness_leaves_file_unmarked() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("managed-agents.json"); + let original = serde_json::to_vec(&serde_json::json!([{ + "pubkey": "", + "slug": "custom-definition", + "runtime": "missing-custom-harness", + "provider": "openai", + "env_vars": { "OPENAI_COMPAT_API_KEY": "compat-secret" } + }])) + .unwrap(); + std::fs::write(&path, &original).unwrap(); + + let error = migrate_openai_credentials_in_file( + &path, + None, + None, + None, + &std::collections::HashMap::new(), + ) + .unwrap_err(); + + assert!(error.contains("unresolved custom harness"), "{error}"); + assert_eq!(std::fs::read(&path).unwrap(), original); + assert!(!sibling_path(&path, MIGRATION_SUFFIX).exists()); + } + + #[test] + fn global_credentials_ignore_preferred_runtime_harness_env() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("global-agent-config.json"); + std::fs::write( + &path, + serde_json::to_vec(&serde_json::json!({ + "preferred_runtime": "custom-gateway", + "provider": "openai", + "env_vars": { "OPENAI_COMPAT_API_KEY": "official-secret" } + })) + .unwrap(), + ) + .unwrap(); + let harness_env = std::collections::HashMap::from([( + "custom-gateway".to_string(), + serde_json::json!({ + "OPENAI_COMPAT_BASE_URL": "https://gateway.example/v1" + }) + .as_object() + .unwrap() + .clone(), + )]); + + migrate_openai_credentials_in_file(&path, None, None, None, &harness_env).unwrap(); + let global: Value = serde_json::from_str(&std::fs::read_to_string(&path).unwrap()).unwrap(); + + assert_eq!(global["provider"], "openai"); + assert_eq!(global["env_vars"][OPENAI_API_KEY], "official-secret"); + assert!(global["env_vars"].get(OPENAI_COMPAT_API_KEY).is_none()); + } + #[test] fn linked_instance_uses_definition_provider_and_layered_endpoint() { let dir = tempfile::tempdir().unwrap(); diff --git a/desktop/src/shared/lib/configNudge.ts b/desktop/src/shared/lib/configNudge.ts index 86c0e16c13..bb1902caeb 100644 --- a/desktop/src/shared/lib/configNudge.ts +++ b/desktop/src/shared/lib/configNudge.ts @@ -23,6 +23,7 @@ import type { AcpAvailabilityStatus } from "@/shared/api/types"; export type ConfigNudgeRequirement = | { surface: "normalized_field"; field: string } | { surface: "env_key"; key: string } + | { surface: "config_invalid"; message: string } | { surface: "cli_login"; probe_args: string[]; @@ -145,6 +146,8 @@ function isConfigNudgeRequirement(v: unknown): v is ConfigNudgeRequirement { return typeof r.field === "string"; case "env_key": return typeof r.key === "string"; + case "config_invalid": + return typeof r.message === "string"; case "cli_login": return ( Array.isArray(r.probe_args) && diff --git a/desktop/src/shared/ui/config-nudge-attachment.tsx b/desktop/src/shared/ui/config-nudge-attachment.tsx index cc5931e23e..5767309d12 100644 --- a/desktop/src/shared/ui/config-nudge-attachment.tsx +++ b/desktop/src/shared/ui/config-nudge-attachment.tsx @@ -29,6 +29,8 @@ function requirementKey( switch (req.surface) { case "env_key": return `env_key:${req.key}:${index}`; + case "config_invalid": + return `config_invalid:${req.message}:${index}`; case "normalized_field": return `normalized_field:${req.field}:${index}`; case "cli_login": @@ -330,6 +332,21 @@ function RequirementRow({ )} ); + case "config_invalid": + return ( +
+ + {requirement.message} + + +
+ ); case "normalized_field": return (