diff --git a/benchmarks/harbor-buzz-orchestra/testbed/endpoints/README.md b/benchmarks/harbor-buzz-orchestra/testbed/endpoints/README.md index f8d2a560bf2..0683a574deb 100644 --- a/benchmarks/harbor-buzz-orchestra/testbed/endpoints/README.md +++ b/benchmarks/harbor-buzz-orchestra/testbed/endpoints/README.md @@ -14,8 +14,8 @@ all entries as endpoint configs (no comment keys). M1 wiring proof: both placeholder endpoints resolve to one local llama-server (OpenAI-compatible, `http://127.0.0.1:8091/v1`, no cloud keys). -buzz-agent env contract (crates/buzz-agent/src/config.rs, pinned at the M1 -binary SHA): `provider=openai` reads `OPENAI_COMPAT_API_KEY` + +buzz-agent env contract (crates/buzz-agent/src/config.rs): +`provider=openai-compat` reads the optional `OPENAI_COMPAT_API_KEY` and required `OPENAI_COMPAT_BASE_URL`; the runtime sets `BUZZ_AGENT_MODEL` from the manifest endpoint name, which overrides `OPENAI_COMPAT_MODEL` — llama-server ignores the model name, so the placeholder value is harmless there. diff --git a/benchmarks/harbor-buzz-orchestra/testbed/endpoints/m1-local.json b/benchmarks/harbor-buzz-orchestra/testbed/endpoints/m1-local.json index dfd82f4e5fe..e89fcd1b5c0 100644 --- a/benchmarks/harbor-buzz-orchestra/testbed/endpoints/m1-local.json +++ b/benchmarks/harbor-buzz-orchestra/testbed/endpoints/m1-local.json @@ -1,13 +1,13 @@ { "local/placeholder-orchestrator": { - "provider": "openai", + "provider": "openai-compat", "api_key_env": "OPENAI_COMPAT_API_KEY", "env": { "OPENAI_COMPAT_BASE_URL": "http://127.0.0.1:8091/v1" } }, "local/placeholder-worker": { - "provider": "openai", + "provider": "openai-compat", "api_key_env": "OPENAI_COMPAT_API_KEY", "env": { "OPENAI_COMPAT_BASE_URL": "http://127.0.0.1:8091/v1" diff --git a/crates/buzz-acp/src/setup_mode.rs b/crates/buzz-acp/src/setup_mode.rs index b1a9372ea46..74eb0d6df42 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/crates/buzz-agent/README.md b/crates/buzz-agent/README.md index 0bc03db7813..329461268fc 100644 --- a/crates/buzz-agent/README.md +++ b/crates/buzz-agent/README.md @@ -43,11 +43,17 @@ ANTHROPIC_API_KEY=sk-ant-... \ ANTHROPIC_MODEL=claude-sonnet-4-5 \ ./target/release/buzz-agent -# Or any OpenAI-compatible endpoint +# Or OpenAI BUZZ_AGENT_PROVIDER=openai \ -OPENAI_COMPAT_API_KEY=sk-... \ +OPENAI_API_KEY=sk-... \ OPENAI_COMPAT_MODEL=gpt-5 \ -OPENAI_COMPAT_BASE_URL=https://api.openai.com/v1 \ + ./target/release/buzz-agent + +# Or an OpenAI-compatible endpoint (the key is optional) +BUZZ_AGENT_PROVIDER=openai-compat \ +OPENAI_COMPAT_API_KEY=local-secret \ +OPENAI_COMPAT_MODEL=llama3 \ +OPENAI_COMPAT_BASE_URL=http://localhost:11434/v1 \ ./target/release/buzz-agent # Or OpenRouter @@ -135,14 +141,15 @@ Everything is environment variables. No flags, no config files. (We are a subpro | Variable | Default | Notes | |---|---|---| -| `BUZZ_AGENT_PROVIDER` | — | Required. `anthropic`, `openai`, `openrouter`, `databricks`, or `databricks_v2`. No implicit fallback — the agent errors at startup when this is unset. | +| `BUZZ_AGENT_PROVIDER` | — | Required. `anthropic`, `openai`, `openai-compat`, `openrouter`, `databricks`, or `databricks_v2`. No implicit fallback — the agent errors at startup when this is unset. | | `ANTHROPIC_API_KEY` | — | Required when provider=anthropic. | | `ANTHROPIC_MODEL` | — | Required when provider=anthropic. | | `ANTHROPIC_BASE_URL` | `https://api.anthropic.com` | | | `ANTHROPIC_API_VERSION` | `2023-06-01` | | -| `OPENAI_COMPAT_API_KEY` | — | Required when provider=openai. | -| `OPENAI_COMPAT_MODEL` | — | Required when provider=openai. | -| `OPENAI_COMPAT_BASE_URL` | `https://api.openai.com/v1` | Point at vLLM, llama.cpp, Ollama, etc. | +| `OPENAI_API_KEY` | — | Required when provider=openai. Never used by provider=openai-compat. | +| `OPENAI_COMPAT_API_KEY` | — | Optional when provider=openai-compat. Never used by provider=openai. | +| `OPENAI_COMPAT_MODEL` | — | Required when provider=openai or provider=openai-compat. | +| `OPENAI_COMPAT_BASE_URL` | — | Required for provider=openai-compat. Custom values are rejected by provider=openai, which is pinned to `https://api.openai.com/v1`. | | `OPENAI_COMPAT_API` | `auto` | `auto` \| `chat` \| `responses`. `auto` picks Responses for `*.openai.com`, Chat Completions everywhere else. | | `OPENROUTER_API_KEY` | — | Required when provider=openrouter. | | `OPENROUTER_MODEL` | — | Required when provider=openrouter. Use OpenRouter's `vendor/model` id, e.g. `anthropic/claude-sonnet-4.5`. | @@ -235,17 +242,17 @@ lifecycle hook — see [MCP_DRIVEN_HOOKS.md](../../docs/MCP_DRIVEN_HOOKS.md). |---|---|---|---| | Anthropic | `anthropic` | `POST {base}/v1/messages` | claude-sonnet-4-5, claude-opus-4 | | OpenAI | `openai` | `POST {base}/responses` | gpt-5, gpt-5-mini, o4-mini, gpt-4o | -| vLLM | `openai` | `POST {base}/chat/completions` | any tool-calling model | -| llama.cpp | `openai` | `POST {base}/chat/completions` | any tool-calling GGUF | -| Ollama | `openai` | `POST {base}/chat/completions` | llama3.1, qwen2.5-coder | -| Block Gateway | `openai` | `POST {base}/chat/completions` | gpt-5, claude | +| vLLM | `openai-compat` | `POST {base}/chat/completions` | any tool-calling model | +| llama.cpp | `openai-compat` | `POST {base}/chat/completions` | any tool-calling GGUF | +| Ollama | `openai-compat` | `POST {base}/chat/completions` | llama3.1, qwen2.5-coder | +| Block Gateway | `openai-compat` | `POST {base}/chat/completions` | gpt-5, claude | | OpenRouter | `openrouter` | `POST {base}/chat/completions` | anything they route (extended-thinking replay, provider-agnostic tool calling) | | Databricks | `databricks` | `POST {host}/serving-endpoints/{model}/invocations` | goose-claude-4-6-sonnet | | Databricks AI Gateway v2 | `databricks_v2` | `POST {host}/ai-gateway/{provider}/v1/...` | databricks-gpt-5-5, databricks-claude-opus-4-7 | -If `BUZZ_AGENT_PROVIDER=anthropic` is selected without `ANTHROPIC_API_KEY`, `BUZZ_AGENT_PROVIDER=openai` is selected without `OPENAI_COMPAT_API_KEY`, or `BUZZ_AGENT_PROVIDER=openrouter` is selected without `OPENROUTER_API_KEY`, the agent returns an error — there is no implicit fallback to another provider. +If `BUZZ_AGENT_PROVIDER=anthropic` is selected without `ANTHROPIC_API_KEY`, `BUZZ_AGENT_PROVIDER=openai` is selected without `OPENAI_API_KEY`, or `BUZZ_AGENT_PROVIDER=openrouter` is selected without `OPENROUTER_API_KEY`, the agent returns an error — there is no implicit fallback to another provider. -`provider=openai` speaks two HTTP dialects: the [Responses API](https://platform.openai.com/docs/api-reference/responses) (`/v1/responses`, required for GPT-5 / o-series tool-calling on OpenAI's own service) and the [Chat Completions API](https://platform.openai.com/docs/api-reference/chat) (`/chat/completions`, the broadly-supported OpenAI-compatible wire format). +`provider=openai` and `provider=openai-compat` share the same OpenAI transport implementation, which speaks the [Responses API](https://platform.openai.com/docs/api-reference/responses) (`/v1/responses`, required for GPT-5 / o-series tool-calling on OpenAI's own service) and the [Chat Completions API](https://platform.openai.com/docs/api-reference/chat) (`/chat/completions`, the broadly-supported OpenAI-compatible wire format). They do not share credentials or origins: official OpenAI reads only `OPENAI_API_KEY` and is pinned to `https://api.openai.com/v1`; compatible endpoints read only the optional `OPENAI_COMPAT_API_KEY` and require an explicit `OPENAI_COMPAT_BASE_URL`. Selecting `provider=openai` with a custom compatible URL fails closed with an instruction to select `openai-compat`. By default (`OPENAI_COMPAT_API=auto`) the agent picks **Responses** when `OPENAI_COMPAT_BASE_URL` points at an `*.openai.com` host and **Chat Completions** everywhere else. Pin the choice explicitly with `OPENAI_COMPAT_API=chat` or `OPENAI_COMPAT_API=responses` for providers that diverge from the default (e.g. a Responses-compatible self-hosted gateway). diff --git a/crates/buzz-agent/src/config.rs b/crates/buzz-agent/src/config.rs index 67d7c593b56..3ec9e7b63f4 100644 --- a/crates/buzz-agent/src/config.rs +++ b/crates/buzz-agent/src/config.rs @@ -418,6 +418,9 @@ const DEFAULT_SYSTEM_PROMPT: &str = pub enum Provider { Anthropic, OpenAi, + /// A custom OpenAI-compatible endpoint. Unlike official OpenAI, the base + /// URL is explicit and bearer authentication is optional. + OpenAiCompat, /// Databricks model serving. Routes to `{base_url}/serving-endpoints/{model}/invocations` /// with a dynamically-acquired bearer (OAuth 2.0 PKCE, or static `DATABRICKS_TOKEN`). /// Wire format is OpenAI-chat-compatible — reuses the same body builder and parser. @@ -527,10 +530,22 @@ impl Config { pub fn from_env() -> Result { let databricks_host = env("DATABRICKS_HOST"); let databricks_model = env("DATABRICKS_MODEL"); + let requested_provider = env("BUZZ_AGENT_PROVIDER"); + let openai_key = env("OPENAI_API_KEY"); + let legacy_openai_key = env("OPENAI_COMPAT_API_KEY"); + let openai_base_url = env("OPENAI_COMPAT_BASE_URL"); + let selected_openai_key = + first_nonempty(openai_key.as_deref(), legacy_openai_key.as_deref()); let provider = resolve_provider( - env("BUZZ_AGENT_PROVIDER").as_deref(), + normalize_legacy_openai_provider( + requested_provider.as_deref(), + openai_base_url.as_deref(), + openai_key.as_deref(), + legacy_openai_key.as_deref(), + ) + .as_deref(), env("ANTHROPIC_API_KEY").as_deref(), - env("OPENAI_COMPAT_API_KEY").as_deref(), + selected_openai_key, env("OPENROUTER_API_KEY").as_deref(), )?; @@ -540,8 +555,8 @@ impl Config { // user intent; provider-specific vars serve as defaults for CLI/standalone use. let buzz_agent_model = env("BUZZ_AGENT_MODEL"); - // OPENAI_COMPAT_API is only read when provider=openai, so a stray - // bad value can't break an Anthropic-only deployment. + // OPENAI_COMPAT_API is only read for OpenAI-family providers, so a + // stray bad value can't break an Anthropic-only deployment. // // Databricks borrows api_key as the *optional* `DATABRICKS_TOKEN` escape // hatch — empty means "use OAuth PKCE." Legacy Databricks encodes the @@ -558,13 +573,27 @@ impl Config { OpenAiApi::Auto, // unused for Anthropic ), Provider::OpenAi => ( - req("OPENAI_COMPAT_API_KEY")?, + selected_openai_key + .map(str::to_string) + .ok_or_else(|| "config: OPENAI_API_KEY required".to_string())?, + resolve_model( + buzz_agent_model.as_deref(), + env("OPENAI_COMPAT_MODEL").as_deref(), + ) + .ok_or_else(|| "config: OPENAI_COMPAT_MODEL required".to_string())?, + official_openai_base_url(env("OPENAI_COMPAT_BASE_URL").as_deref())?, + parse_openai_api(env("OPENAI_COMPAT_API").as_deref())?, + ), + Provider::OpenAiCompat => ( + env("OPENAI_COMPAT_API_KEY") + .map(|value| value.trim().to_string()) + .unwrap_or_default(), resolve_model( buzz_agent_model.as_deref(), env("OPENAI_COMPAT_MODEL").as_deref(), ) .ok_or_else(|| "config: OPENAI_COMPAT_MODEL required".to_string())?, - env_or("OPENAI_COMPAT_BASE_URL", "https://api.openai.com/v1"), + parse_openai_compat_base_url(env("OPENAI_COMPAT_BASE_URL").as_deref())?, parse_openai_api(env("OPENAI_COMPAT_API").as_deref())?, ), Provider::Databricks | Provider::DatabricksV2 => ( @@ -788,10 +817,43 @@ fn resolve_model( explicit_override.or(provider_default).map(str::to_owned) } +fn first_nonempty<'a>(primary: Option<&'a str>, fallback: Option<&'a str>) -> Option<&'a str> { + primary + .filter(|value| !value.trim().is_empty()) + .or_else(|| fallback.filter(|value| !value.trim().is_empty())) +} + fn present_nonempty(v: Option<&str>) -> bool { v.map(str::trim).is_some_and(|s| !s.is_empty()) } +fn normalize_legacy_openai_provider( + requested: Option<&str>, + openai_base_url: Option<&str>, + openai_key: Option<&str>, + legacy_openai_key: Option<&str>, +) -> Option { + let requested = requested?.trim(); + if !requested.eq_ignore_ascii_case("openai") { + return Some(requested.to_string()); + } + // OPENAI_API_KEY is the post-split discriminator. Only configurations that + // still rely exclusively on the legacy key are eligible for URL-based + // normalization; an explicit official key must never be routed elsewhere. + if present_nonempty(openai_key) || !present_nonempty(legacy_openai_key) { + return Some("openai".to_string()); + } + let custom_origin = openai_base_url + .map(str::trim) + .filter(|value| !value.is_empty()) + .is_some_and(|value| value.trim_end_matches('/') != "https://api.openai.com/v1"); + Some(if custom_origin { + "openai-compat".to_string() + } else { + "openai".to_string() + }) +} + fn resolve_provider( requested: Option<&str>, anthropic_key: Option<&str>, @@ -806,10 +868,9 @@ fn resolve_provider( "anthropic" => Err( "config: ANTHROPIC_API_KEY required".into(), ), - "openai" | "openai-compat" if present_nonempty(openai_key) => Ok(Provider::OpenAi), - "openai" | "openai-compat" => Err( - "config: OPENAI_COMPAT_API_KEY required".into(), - ), + "openai" if present_nonempty(openai_key) => Ok(Provider::OpenAi), + "openai" => Err("config: OPENAI_API_KEY required".into()), + "openai-compat" => Ok(Provider::OpenAiCompat), "databricks" => Ok(Provider::Databricks), "databricks_v2" | "databricks-v2" => Ok(Provider::DatabricksV2), "openrouter" if present_nonempty(openrouter_key) => Ok(Provider::OpenRouter), @@ -825,6 +886,39 @@ fn resolve_provider( } } +fn official_openai_base_url(raw: Option<&str>) -> Result { + const BASE_URL: &str = "https://api.openai.com/v1"; + match raw.map(str::trim).filter(|value| !value.is_empty()) { + None => Ok(BASE_URL.to_string()), + Some(value) if value.trim_end_matches('/') == BASE_URL => Ok(BASE_URL.to_string()), + Some(_) => Err( + "config: OPENAI_COMPAT_BASE_URL is only supported by provider=openai-compat" + .to_string(), + ), + } +} + +fn parse_openai_compat_base_url(raw: Option<&str>) -> Result { + const INVALID_URL: &str = + "config: OPENAI_COMPAT_BASE_URL must be an HTTP(S) URL without credentials, query, or fragment"; + + let value = raw + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or_else(|| "config: OPENAI_COMPAT_BASE_URL required for openai-compat".to_string())?; + let parsed = url::Url::parse(value).map_err(|_| INVALID_URL.to_string())?; + if !matches!(parsed.scheme(), "http" | "https") + || parsed.host_str().is_none() + || !parsed.username().is_empty() + || parsed.password().is_some() + || parsed.query().is_some() + || parsed.fragment().is_some() + { + return Err(INVALID_URL.to_string()); + } + Ok(value.trim_end_matches('/').to_string()) +} + /// Parse `OPENAI_COMPAT_API`. Pure (env-free) for testability; the /// caller hands in the raw value. fn parse_openai_api(raw: Option<&str>) -> Result { @@ -1104,6 +1198,35 @@ mod tests { assert!(err.contains("OPENAI_COMPAT_API=nope"), "{err}"); } + #[test] + fn legacy_openai_provider_normalizes_custom_origins_only_without_new_key() { + assert_eq!( + normalize_legacy_openai_provider( + Some("openai"), + Some("http://localhost:11434/v1"), + None, + Some("legacy-key"), + ) + .as_deref(), + Some("openai-compat") + ); + assert_eq!( + normalize_legacy_openai_provider( + Some("openai"), + Some("http://localhost:11434/v1"), + Some("official-key"), + Some("legacy-key"), + ) + .as_deref(), + Some("openai") + ); + assert_eq!( + normalize_legacy_openai_provider(Some("openai"), None, None, Some("legacy-key"),) + .as_deref(), + Some("openai") + ); + } + #[test] fn resolve_provider_keeps_requested_provider_when_token_present() { assert_eq!( @@ -1117,13 +1240,61 @@ mod tests { } #[test] - fn resolve_provider_errors_when_requested_provider_key_missing() { - // No fallback — missing key returns an error regardless of Databricks availability. + fn empty_official_key_falls_back_to_populated_legacy_key() { + assert_eq!(first_nonempty(Some(""), Some("legacy")), Some("legacy")); + assert_eq!(first_nonempty(Some(" "), Some("legacy")), Some("legacy")); + } + + #[test] + fn resolve_provider_requires_only_official_openai_key() { let err = resolve_provider(Some("anthropic"), None, None, None).unwrap_err(); assert!(err.contains("ANTHROPIC_API_KEY required"), "{err}"); - let err = resolve_provider(Some("openai-compat"), None, Some(" "), None).unwrap_err(); - assert!(err.contains("OPENAI_COMPAT_API_KEY required"), "{err}"); + let err = resolve_provider(Some("openai"), None, Some(" "), None).unwrap_err(); + assert!(err.contains("OPENAI_API_KEY required"), "{err}"); + + assert_eq!( + resolve_provider(Some("openai-compat"), None, None, None).unwrap(), + Provider::OpenAiCompat + ); + } + + #[test] + fn official_openai_rejects_compat_routing_state() { + assert_eq!( + official_openai_base_url(None).unwrap(), + "https://api.openai.com/v1" + ); + assert_eq!( + official_openai_base_url(Some(" https://api.openai.com/v1/ ")).unwrap(), + "https://api.openai.com/v1" + ); + let error = official_openai_base_url(Some("https://gateway.example/v1")).unwrap_err(); + assert!(error.contains("provider=openai-compat"), "{error}"); + } + + #[test] + fn openai_compat_base_url_is_required_and_normalized() { + assert!(parse_openai_compat_base_url(None) + .unwrap_err() + .contains("required for openai-compat")); + for invalid in [ + "ftp://localhost/v1", + "http://user:secret@localhost/v1", + "http://localhost/v1?tenant=x", + "http://localhost/v1#fragment", + ] { + assert!( + parse_openai_compat_base_url(Some(invalid)) + .unwrap_err() + .contains("without credentials, query, or fragment"), + "invalid={invalid}" + ); + } + assert_eq!( + parse_openai_compat_base_url(Some(" http://localhost:11434/v1/// ")).unwrap(), + "http://localhost:11434/v1" + ); } #[test] @@ -1144,7 +1315,7 @@ mod tests { ); // Missing key for other providers still errors — no Databricks fallback. let err = resolve_provider(Some("openai"), None, None, None).unwrap_err(); - assert!(err.contains("OPENAI_COMPAT_API_KEY required"), "{err}"); + assert!(err.contains("OPENAI_API_KEY required"), "{err}"); let err = resolve_provider(None, None, None, None).unwrap_err(); assert!(err.contains("BUZZ_AGENT_PROVIDER is required"), "{err}"); } diff --git a/crates/buzz-agent/src/llm.rs b/crates/buzz-agent/src/llm.rs index 47df56a6d37..c415c0064bc 100644 --- a/crates/buzz-agent/src/llm.rs +++ b/crates/buzz-agent/src/llm.rs @@ -43,7 +43,7 @@ pub struct Llm { /// for the lifetime of the process. auto_upgraded: AtomicBool, /// Bearer-token source for OpenAI-family requests. Static for OpenAI - /// (the `OPENAI_COMPAT_API_KEY` env var) and Databricks-with-token + /// (the provider-specific OpenAI API key env var) and Databricks-with-token /// (the `DATABRICKS_TOKEN` env var); a refreshable PKCE engine for /// Databricks otherwise. Anthropic doesn't use this — it always /// reads `cfg.api_key` directly because the API expects `x-api-key`. @@ -114,9 +114,9 @@ impl Llm { .await .and_then(parse_openai_with_reasoning_details) } - Provider::OpenAi | Provider::Databricks => { + Provider::OpenAi | Provider::OpenAiCompat | Provider::Databricks => { let provider_str = match cfg.provider { - Provider::OpenAi => "openai", + Provider::OpenAi | Provider::OpenAiCompat => "openai", Provider::Databricks => "databricks", _ => unreachable!(), }; @@ -269,7 +269,7 @@ impl Llm { let v = self.post_openrouter(cfg, &body).await?; Ok(parse_openai(v)?.text) } - Provider::OpenAi | Provider::Databricks => { + Provider::OpenAi | Provider::OpenAiCompat | Provider::Databricks => { let r = self .openai_request(cfg, effective_model, |use_responses, request_model| { if use_responses { @@ -494,14 +494,19 @@ impl Llm { // statuses map to `LlmAuth` in `post`: a 403 is indistinguishable from // an expired-token 403 here, so we refresh once and let it propagate. let mut bearer = self.auth.bearer().await.map_err(PostError::from)?; + let use_bearer = cfg.provider != Provider::OpenAiCompat || !bearer.is_empty(); let mut refreshed = false; loop { - match post(&self.http, &url, body_ref, cfg.llm_timeout, |r| { - r.bearer_auth(&bearer) + match post(&self.http, &url, body_ref, cfg.llm_timeout, |request| { + if use_bearer { + request.bearer_auth(&bearer) + } else { + request + } }) .await { - Err(PostError::Agent(AgentError::LlmAuth(_))) if !refreshed => { + Err(PostError::Agent(AgentError::LlmAuth(_))) if use_bearer && !refreshed => { refreshed = true; bearer = self .auth @@ -2069,14 +2074,15 @@ pub(crate) fn databricks_pkce_config(host: &str) -> PkceOAuthConfig { /// - `Provider::Anthropic`: a static source seeded from `cfg.api_key`. It's /// never read for Anthropic requests (those go through `post_anthropic` with /// `x-api-key`), but Llm holds one to keep the field non-`Option`. -/// - `Provider::OpenAi`: a static source over `OPENAI_COMPAT_API_KEY`. +/// - `Provider::OpenAi`: a static source over `OPENAI_API_KEY`. +/// - `Provider::OpenAiCompat`: a static source over `OPENAI_COMPAT_API_KEY`. /// - `Provider::Databricks`: if `DATABRICKS_TOKEN` is set, a static source. /// Otherwise a `PkceOAuthTokenSource` pointed at the workspace's OIDC /// discovery URL. First request without a cached token triggers a browser /// flow; subsequent requests use the cache + refresh transparently. pub(crate) fn build_token_source(cfg: &Config) -> Result, AgentError> { match cfg.provider { - Provider::Anthropic | Provider::OpenAi | Provider::OpenRouter => { + Provider::Anthropic | Provider::OpenAi | Provider::OpenAiCompat | Provider::OpenRouter => { Ok(Arc::new(StaticTokenSource::new(cfg.api_key.clone()))) } Provider::Databricks | Provider::DatabricksV2 => { @@ -2102,9 +2108,11 @@ pub(crate) fn build_token_source(cfg: &Config) -> Result, A pub(crate) fn summary_completion_cap(provider: Provider, max_output_tokens: u32) -> u32 { match provider { Provider::OpenRouter => max_output_tokens.saturating_mul(2), - Provider::Anthropic | Provider::OpenAi | Provider::Databricks | Provider::DatabricksV2 => { - max_output_tokens - } + Provider::Anthropic + | Provider::OpenAi + | Provider::OpenAiCompat + | Provider::Databricks + | Provider::DatabricksV2 => max_output_tokens, } } @@ -5781,6 +5789,49 @@ mod tests { } } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn post_openai_compat_omits_authorization_when_key_is_empty() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + use tokio::net::TcpListener; + + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let captured = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut bytes = Vec::new(); + let mut buffer = [0u8; 4096]; + while !bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let count = socket.read(&mut buffer).await.unwrap(); + if count == 0 { + break; + } + bytes.extend_from_slice(&buffer[..count]); + } + let body = "{\"ok\":true}"; + socket + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), body + ) + .as_bytes(), + ) + .await + .unwrap(); + String::from_utf8_lossy(&bytes).to_ascii_lowercase() + }); + + let llm = llm_with(Arc::new(StaticTokenSource::new(""))); + let mut config = cfg(Provider::OpenAiCompat); + config.base_url = base; + llm.post_openai(&config, "/v1/x", &json!({}), "model") + .await + .unwrap(); + + let headers = captured.await.unwrap(); + assert!(!headers.contains("authorization:"), "{headers}"); + } + /// A single 401 forces exactly one refresh, the retry with the fresh /// token succeeds, and a *later* call gets its own refresh — proving the /// one-shot guard is per-call, not stored on the source. diff --git a/crates/buzz-agent/tests/fake_llm.rs b/crates/buzz-agent/tests/fake_llm.rs index 4253ef329c1..4d3f6a12e06 100644 --- a/crates/buzz-agent/tests/fake_llm.rs +++ b/crates/buzz-agent/tests/fake_llm.rs @@ -175,7 +175,7 @@ impl Harness { async fn spawn(base_url: &str) -> Self { let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("OPENAI_COMPAT_BASE_URL", base_url) @@ -572,7 +572,7 @@ async fn rejects_oversized_line() { let url = spawn_fake_llm(vec![]).await; let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("OPENAI_COMPAT_BASE_URL", &url) diff --git a/crates/buzz-agent/tests/golden_transcripts.rs b/crates/buzz-agent/tests/golden_transcripts.rs index 32c7e1034f9..29be2825e27 100644 --- a/crates/buzz-agent/tests/golden_transcripts.rs +++ b/crates/buzz-agent/tests/golden_transcripts.rs @@ -19,7 +19,7 @@ impl Harness { async fn spawn(extra: &[(&str, &str)]) -> Self { let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("BUZZ_AGENT_LLM_TIMEOUT_SECS", "5") @@ -472,7 +472,7 @@ async fn test_oversized_line_kills_agent() { let url = spawn_fake_llm(vec![]).await; let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("OPENAI_COMPAT_BASE_URL", &url) @@ -634,7 +634,7 @@ async fn test_thought_chunk_emitted_before_message_chunk_responses_api() { )]) .await; let mut h = Harness::spawn(&[ - ("BUZZ_AGENT_PROVIDER", "openai"), + ("BUZZ_AGENT_PROVIDER", "openai-compat"), ("OPENAI_COMPAT_API_KEY", "test"), ("OPENAI_COMPAT_MODEL", "fake-model"), ("OPENAI_COMPAT_API", "responses"), @@ -940,7 +940,7 @@ async fn test_acp_v2_chunks_carry_message_id() { ]) .await; let mut h = Harness::spawn(&[ - ("BUZZ_AGENT_PROVIDER", "openai"), + ("BUZZ_AGENT_PROVIDER", "openai-compat"), ("OPENAI_COMPAT_API_KEY", "test"), ("OPENAI_COMPAT_MODEL", "fake-model"), ("OPENAI_COMPAT_API", "responses"), diff --git a/crates/buzz-agent/tests/hints_integration.rs b/crates/buzz-agent/tests/hints_integration.rs index 63a55514dbe..fcb70c0596d 100644 --- a/crates/buzz-agent/tests/hints_integration.rs +++ b/crates/buzz-agent/tests/hints_integration.rs @@ -92,7 +92,7 @@ impl Harness { async fn spawn_with_env(base_url: &str, extra: &[(&str, &str)]) -> Self { let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("OPENAI_COMPAT_BASE_URL", base_url) diff --git a/crates/buzz-agent/tests/openai_auto_upgrade.rs b/crates/buzz-agent/tests/openai_auto_upgrade.rs index f85a52bfaa1..de2e7a517ba 100644 --- a/crates/buzz-agent/tests/openai_auto_upgrade.rs +++ b/crates/buzz-agent/tests/openai_auto_upgrade.rs @@ -129,7 +129,7 @@ async fn openai_auto_upgrades_chat_to_responses_on_databricks_signal() { let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "gpt-5.5") .env("OPENAI_COMPAT_BASE_URL", &base_url) diff --git a/crates/buzz-agent/tests/regressions.rs b/crates/buzz-agent/tests/regressions.rs index 6a4f347f6bb..a610d783873 100644 --- a/crates/buzz-agent/tests/regressions.rs +++ b/crates/buzz-agent/tests/regressions.rs @@ -108,7 +108,7 @@ impl Harness { async fn spawn_with_env(base_url: &str, extra: &[(&str, &str)]) -> Self { let bin = env!("CARGO_BIN_EXE_buzz-agent"); let mut cmd = tokio::process::Command::new(bin); - cmd.env("BUZZ_AGENT_PROVIDER", "openai") + cmd.env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("OPENAI_COMPAT_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("OPENAI_COMPAT_BASE_URL", base_url) @@ -2296,7 +2296,7 @@ async fn reply_guard_combines_with_stop_hook_objection() { fn reply_guard_rejects_unparseable_toggle() { let out = std::process::Command::new(env!("CARGO_BIN_EXE_buzz-agent")) .env("BUZZ_AGENT_PROVIDER", "openai") - .env("OPENAI_COMPAT_API_KEY", "test") + .env("OPENAI_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("BUZZ_AGENT_REQUIRE_REPLY", "true") .stdin(Stdio::null()) @@ -2318,7 +2318,7 @@ fn reply_guard_rejects_unparseable_toggle() { fn max_token_recoveries_rejects_unparseable_value() { let out = std::process::Command::new(env!("CARGO_BIN_EXE_buzz-agent")) .env("BUZZ_AGENT_PROVIDER", "openai") - .env("OPENAI_COMPAT_API_KEY", "test") + .env("OPENAI_API_KEY", "test") .env("OPENAI_COMPAT_MODEL", "fake-model") .env("BUZZ_AGENT_MAX_TOKEN_RECOVERIES", "unbounded") .stdin(Stdio::null()) diff --git a/crates/buzz-relay/examples/mesh_agent_e2e.rs b/crates/buzz-relay/examples/mesh_agent_e2e.rs index db0fc6090be..84a742bc86a 100644 --- a/crates/buzz-relay/examples/mesh_agent_e2e.rs +++ b/crates/buzz-relay/examples/mesh_agent_e2e.rs @@ -279,7 +279,7 @@ async fn agent_chat_in_isolated_home( // The transport subset of apply_relay_mesh_env(): provider, base URL, // model, key, and chat API. Not BUZZ_AGENT_REQUIRE_REPLY, which needs // Buzz's publish tools to mean anything. - .env("BUZZ_AGENT_PROVIDER", "openai") + .env("BUZZ_AGENT_PROVIDER", "openai-compat") .env("BUZZ_AGENT_MODEL", model) .env("OPENAI_COMPAT_BASE_URL", base) .env("OPENAI_COMPAT_MODEL", model) diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index cb809b6c04a..4d98d510539 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -6,13 +6,14 @@ use tauri::{AppHandle, State}; use super::agent_model_process::run_agent_models_command; use super::managed_agent_definition::apply_model_provider_prompt_update; -// The map-only lookup is reached solely from the base-URL helpers that exist for -// their unit tests; discovery itself always goes through the process-env variant. -#[cfg(test)] -use super::agent_models_env::env_value; +// The map-only lookup is reached solely from an Anthropic base-URL helper used +// by unit tests; discovery itself always goes through the process-env variant. use super::agent_models_env::{ - effective_discovery_provider, env_or_process_value, redaction_env_with_value, DiscoveryProvider, + effective_discovery_provider, env_or_process_value, openai_compatible_models_url_for_discovery, + redaction_env_with_value, DiscoveryProvider, }; +#[cfg(test)] +use super::agent_models_env::{env_or_process_override, env_value}; use super::agent_update_rollback::{rollback_failed_agent_update, AgentUpdateRollback}; use crate::{ @@ -95,11 +96,18 @@ pub async fn get_agent_models( command: _, } = discovery; - let merged_env = discovery_env_with_baked_floor(merged_env); + let mut merged_env = discovery_env_with_baked_floor(merged_env); // Resolve against the baked/process env when the record saved no provider, // so a build-provided provider still gets live discovery. - let effective_provider = + let mut effective_provider = effective_discovery_provider(saved_provider.as_deref(), provider_env_var, &merged_env); + if known_acp_runtime(&discovery.command).is_some_and(|runtime| runtime.id == "buzz-agent") { + effective_provider.value = + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut merged_env, + effective_provider.as_deref(), + ); + } if let Some(models) = discover_openrouter_models( &state.http_client, &effective_provider, @@ -230,14 +238,21 @@ pub async fn discover_agent_models( &input.definition_env, &input.env_vars, ); - let merged_env = discovery_env_with_baked_floor(merged_env); + let mut merged_env = discovery_env_with_baked_floor(merged_env); // Recover a build-provided provider when the form has none, so the create // dialog discovers live models instead of falling through to the subprocess. - let effective_provider = effective_discovery_provider( + let mut effective_provider = effective_discovery_provider( input.provider.as_deref(), runtime_meta.and_then(|meta| meta.provider_env_var), &merged_env, ); + if runtime_meta.is_some_and(|runtime| runtime.id == "buzz-agent") { + effective_provider.value = + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut merged_env, + effective_provider.as_deref(), + ); + } // Buzz shared compute discovery must not depend on the local OpenAI ingress: that // client endpoint is started only after a live target is selected. @@ -355,19 +370,6 @@ fn is_openai_compatible_provider(provider: Option<&str>) -> bool { ) } -#[cfg(test)] -fn openai_compatible_models_url(env: &BTreeMap) -> String { - let base_url = env_value(env, "OPENAI_COMPAT_BASE_URL") - .unwrap_or_else(|| "https://api.openai.com/v1".to_string()); - format!("{}/models", base_url.trim_end_matches('/')) -} - -fn openai_compatible_models_url_for_discovery(env: &BTreeMap) -> String { - let base_url = env_or_process_value(env, "OPENAI_COMPAT_BASE_URL") - .unwrap_or_else(|| "https://api.openai.com/v1".to_string()); - format!("{}/models", base_url.trim_end_matches('/')) -} - fn is_agent_text_model_id(id: &str) -> bool { let lower = id.to_ascii_lowercase(); if [ @@ -496,23 +498,39 @@ async fn discover_openai_compatible_models( return Ok(None); } + let is_compat = provider + .as_deref() + .is_some_and(|value| value.trim().eq_ignore_ascii_case("openai-compat")); + let api_key_env_var = if is_compat || relay_mesh { + "OPENAI_COMPAT_API_KEY" + } else { + "OPENAI_API_KEY" + }; let api_key = if relay_mesh { crate::managed_agents::RELAY_MESH_API_KEY_PLACEHOLDER.to_string() + } else if is_compat { + env.get(api_key_env_var) + .map(|value| value.trim().to_string()) + .unwrap_or_default() } else { - match provider.required_env(env, "OPENAI_COMPAT_API_KEY")? { + match provider.required_env(env, api_key_env_var)? { Some(api_key) => api_key, None => return Ok(None), } }; - let redaction_env = redaction_env_with_value(env, "OPENAI_COMPAT_API_KEY", &api_key); + let redaction_env = redaction_env_with_value(env, api_key_env_var, &api_key); let url = if relay_mesh { format!("{}/models", crate::managed_agents::RELAY_MESH_API_BASE_URL) } else { - openai_compatible_models_url_for_discovery(env) + openai_compatible_models_url_for_discovery(provider.as_deref(), env)? }; - let response = client - .get(&url) - .bearer_auth(&api_key) + let request = client.get(&url); + let request = if api_key.is_empty() { + request + } else { + request.bearer_auth(&api_key) + }; + let response = request .send() .await .map_err(|error| format!("OpenAI model discovery request failed: {error}"))?; @@ -673,7 +691,6 @@ async fn discover_anthropic_models( if models.is_empty() { return Err("Anthropic model discovery returned no models".to_string()); } - Ok(Some(AgentModelsResponse { agent_name: provider .as_deref() @@ -687,7 +704,6 @@ async fn discover_anthropic_models( supports_switching: true, })) } - #[path = "agent_models_databricks.rs"] mod databricks; #[cfg(test)] @@ -703,7 +719,6 @@ pub use update::update_managed_agent; pub(super) use update::{flush_managed_agent_policy, managed_agent_access_policy_changed}; // ── Model normalization ─────────────────────────────────────────────────────── - /// Normalize raw `buzz-acp models --json` output into a typed DTO for the frontend. /// /// Merges models from both ACP paths (stable configOptions + unstable SessionModelState), @@ -720,10 +735,8 @@ pub(super) fn normalize_agent_models( .as_str() .unwrap_or("unknown") .to_string(); - let mut models: Vec = Vec::new(); let mut seen_ids: HashSet = HashSet::new(); - // 1. Stable configOptions (preferred). Only entries with category "model" // are model options — the CLI pre-filters, but we're defensive here. if let Some(config_options) = raw["stable"]["configOptions"].as_array() { @@ -749,7 +762,6 @@ pub(super) fn normalize_agent_models( } } } - // 2. Unstable availableModels (fallback — skip duplicates from stable). let mut agent_default_model: Option = None; if let Some(unstable) = raw.get("unstable") { @@ -771,9 +783,7 @@ pub(super) fn normalize_agent_models( } } } - let supports_switching = !models.is_empty(); - AgentModelsResponse { agent_name, agent_version, @@ -783,7 +793,6 @@ pub(super) fn normalize_agent_models( supports_switching, } } - #[cfg(test)] #[path = "agent_models_tests.rs"] mod tests; diff --git a/desktop/src-tauri/src/commands/agent_models_env.rs b/desktop/src-tauri/src/commands/agent_models_env.rs index 0a40b6bd8ff..a2bbcf54b44 100644 --- a/desktop/src-tauri/src/commands/agent_models_env.rs +++ b/desktop/src-tauri/src/commands/agent_models_env.rs @@ -25,6 +25,67 @@ pub(super) fn env_or_process_value(env: &BTreeMap, key: &str) -> }) } +/// Read a trimmed mapped value even when it is blank, falling back to the +/// process only when the map has no override. Optional credentials use this so +/// an explicit blank means "send no authentication" rather than inheriting an +/// unrelated process secret. +pub(super) fn env_or_process_override(env: &BTreeMap, key: &str) -> Option { + env.get(key) + .map(|value| value.trim().to_string()) + .or_else(|| { + std::env::var(key) + .ok() + .map(|value| value.trim().to_string()) + }) +} + +pub(super) fn validate_openai_compat_base_url(value: &str) -> Result<(), String> { + const INVALID_URL: &str = + "OPENAI_COMPAT_BASE_URL must be an HTTP(S) URL without credentials, query, or fragment"; + let parsed = url::Url::parse(value.trim()).map_err(|_| INVALID_URL.to_string())?; + if !matches!(parsed.scheme(), "http" | "https") + || parsed.host_str().is_none() + || !parsed.username().is_empty() + || parsed.password().is_some() + || parsed.query().is_some() + || parsed.fragment().is_some() + { + return Err(INVALID_URL.to_string()); + } + Ok(()) +} + +pub(super) fn openai_compatible_models_url_for_discovery( + provider: Option<&str>, + env: &BTreeMap, +) -> Result { + let is_compat = + provider.is_some_and(|value| value.trim().eq_ignore_ascii_case("openai-compat")); + let configured_base_url = if is_compat { + env_or_process_override(env, "OPENAI_COMPAT_BASE_URL") + } else { + env_value(env, "OPENAI_COMPAT_BASE_URL") + } + .filter(|value| !value.is_empty()); + if !is_compat { + if configured_base_url + .as_deref() + .is_some_and(|value| value.trim_end_matches('/') != "https://api.openai.com/v1") + { + return Err( + "OPENAI_COMPAT_BASE_URL is only supported by provider=openai-compat".into(), + ); + } + return Ok("https://api.openai.com/v1/models".to_string()); + } + + let base_url = configured_base_url.ok_or_else(|| { + "OPENAI_COMPAT_BASE_URL required for OpenAI-compatible model discovery".to_string() + })?; + validate_openai_compat_base_url(&base_url)?; + Ok(format!("{}/models", base_url.trim().trim_end_matches('/'))) +} + /// Clone `env` with `key` set to the value a request actually used, so error /// redaction masks the inherited process value and not just the mapped one. pub(super) fn redaction_env_with_value( @@ -49,7 +110,7 @@ pub(super) fn redaction_env_with_value( /// turn the subprocess catalog into `config: ANTHROPIC_API_KEY required`. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct DiscoveryProvider { - value: Option, + pub(super) value: Option, inferred: bool, } @@ -68,7 +129,7 @@ impl DiscoveryProvider { env: &BTreeMap, key: &str, ) -> Result, String> { - match env_or_process_value(env, key) { + match env_or_process_override(env, key).filter(|value| !value.is_empty()) { Some(value) => Ok(Some(value)), None if self.inferred => Ok(None), None => Err(format!("config: {key} required")), @@ -112,3 +173,23 @@ pub(super) fn effective_discovery_provider( inferred: true, } } + +#[cfg(test)] +mod tests { + use super::validate_openai_compat_base_url; + + #[test] + fn rejects_ambiguous_openai_compat_url_components() { + for value in [ + "http://user:secret@localhost/v1", + "http://localhost/v1?tenant=x", + "http://localhost/v1#fragment", + ] { + let error = validate_openai_compat_base_url(value).unwrap_err(); + assert!( + error.contains("without credentials, query, or fragment"), + "value={value} error={error}" + ); + } + } +} diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index a9e3b677753..f2fa7ad737c 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -1,5 +1,8 @@ use super::*; +#[path = "agent_models_tests/openai_credentials.rs"] +mod openai_credentials; + #[test] fn access_policy_change_requires_runtime_refresh_for_effective_gate_changes() { use crate::managed_agents::RespondTo; @@ -142,10 +145,18 @@ fn openai_compat_model_normalization_preserves_provider_specific_ids() { } #[test] -fn openai_models_url_uses_openai_default_base_url() { +fn openai_compat_models_url_requires_custom_base_url() { + let err = openai_compatible_models_url_for_discovery(Some("openai-compat"), &BTreeMap::new()) + .unwrap_err(); + assert!(err.contains("OPENAI_COMPAT_BASE_URL required"), "{err}"); + + let env = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://localhost:11434/v1/".to_string(), + )]); assert_eq!( - openai_compatible_models_url(&BTreeMap::new()), - "https://api.openai.com/v1/models" + openai_compatible_models_url_for_discovery(Some("openai-compat"), &env).unwrap(), + "http://localhost:11434/v1/models" ); } @@ -310,7 +321,7 @@ fn effective_discovery_provider_recovers_baked_provider_when_record_has_none() { fn effective_discovery_provider_is_none_without_an_explicit_or_env_provider() { let env = BTreeMap::new(); assert_eq!( - effective_discovery_provider(None, Some("BUZZ_AGENT_PROVIDER"), &env).as_deref(), + effective_discovery_provider(None, Some("BUZZ_TEST_UNSET_PROVIDER"), &env).as_deref(), None ); // A runtime that takes no provider env var has nothing to recover from. diff --git a/desktop/src-tauri/src/commands/agent_models_tests/openai_credentials.rs b/desktop/src-tauri/src/commands/agent_models_tests/openai_credentials.rs new file mode 100644 index 00000000000..6f2b5cf8ecb --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_tests/openai_credentials.rs @@ -0,0 +1,105 @@ +use super::*; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::TcpListener; + +#[test] +fn official_openai_models_url_rejects_compat_routing_state() { + let env = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://arbitrary-compatible-host.example/v1".to_string(), + )]); + let error = openai_compatible_models_url_for_discovery(Some("openai"), &env).unwrap_err(); + assert!(error.contains("provider=openai-compat"), "{error}"); + + let legacy_canonical = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://api.openai.com/v1/".to_string(), + )]); + assert_eq!( + openai_compatible_models_url_for_discovery(Some("openai"), &legacy_canonical).unwrap(), + "https://api.openai.com/v1/models" + ); +} + +#[tokio::test] +async fn official_openai_discovery_does_not_accept_compat_key() { + let provider = effective_discovery_provider(Some("openai"), None, &BTreeMap::new()); + let env = BTreeMap::from([ + ("OPENAI_API_KEY".to_string(), " ".to_string()), + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "must-not-cross-provider-boundary".to_string(), + ), + ]); + + let error = discover_openai_compatible_models(&reqwest::Client::new(), &provider, &env, None) + .await + .unwrap_err(); + + assert!(error.contains("OPENAI_API_KEY"), "{error}"); + assert!(!error.contains("OPENAI_COMPAT_API_KEY"), "{error}"); +} + +#[tokio::test] +async fn openai_compat_discovery_omits_authorization_without_key() { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base_url = format!("http://{}/v1", listener.local_addr().unwrap()); + let captured = tokio::spawn(async move { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut bytes = Vec::new(); + let mut buffer = [0u8; 4096]; + while !bytes.windows(4).any(|window| window == b"\r\n\r\n") { + let count = socket.read(&mut buffer).await.unwrap(); + if count == 0 { + break; + } + bytes.extend_from_slice(&buffer[..count]); + } + let body = r#"{"data":[{"id":"llama3","created":1}]}"#; + socket + .write_all( + format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + body.len(), + body + ) + .as_bytes(), + ) + .await + .unwrap(); + String::from_utf8_lossy(&bytes).to_ascii_lowercase() + }); + + let provider = effective_discovery_provider(Some("openai-compat"), None, &BTreeMap::new()); + let env = BTreeMap::from([ + ("OPENAI_COMPAT_BASE_URL".to_string(), base_url), + ( + "OPENAI_API_KEY".to_string(), + "must-not-cross-provider-boundary".to_string(), + ), + ]); + let result = discover_openai_compatible_models(&reqwest::Client::new(), &provider, &env, None) + .await + .unwrap() + .unwrap(); + + assert_eq!(result.models[0].id, "llama3"); + let request = captured.await.unwrap(); + assert!(request.starts_with("get /v1/models "), "{request}"); + assert!(!request.contains("authorization:"), "{request}"); +} + +#[test] +fn optional_compat_key_allows_explicit_blank_to_shadow_process_env() { + let key = "BUZZ_TEST_OPENAI_COMPAT_API_KEY_OVERRIDE"; + let prior = std::env::var_os(key); + std::env::set_var(key, "process-secret"); + + let env = BTreeMap::from([(key.to_string(), " ".to_string())]); + assert_eq!(env_or_process_override(&env, key).as_deref(), Some("")); + + match prior { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } +} diff --git a/desktop/src-tauri/src/commands/agents_deploy.rs b/desktop/src-tauri/src/commands/agents_deploy.rs index da5bb3ba5c0..a004a396147 100644 --- a/desktop/src-tauri/src/commands/agents_deploy.rs +++ b/desktop/src-tauri/src/commands/agents_deploy.rs @@ -192,11 +192,21 @@ pub(crate) fn build_deploy_payload( ) .require_resolved()?; - ensure_remote_provider_supported(effective.provider.value.as_deref())?; - let descriptor = crate::managed_agents::resolve_effective_harness_descriptor(record, &personas, &global) .map_err(|error| crate::managed_agents::user_facing_harness_error(&error))?; + let mut effective_provider = effective.provider.value; + if crate::managed_agents::known_acp_runtime(&descriptor.command) + .is_some_and(|runtime| runtime.id == "buzz-agent") + { + let mut env = descriptor.env.clone(); + effective_provider = crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut env, + effective_provider.as_deref(), + ); + } + ensure_remote_provider_supported(effective_provider.as_deref())?; + let owner_pubkey = super::workspace_owner_hex(state)?; let launch = build_launch_block( record, @@ -218,7 +228,7 @@ pub(crate) fn build_deploy_payload( ), DeployProjections { effective_model: effective.model.value, - effective_provider: effective.provider.value, + effective_provider, effective_prompt: effective.system_prompt.value, effective_parallelism, owner_only_access: crate::managed_agents::owner_only_access_build(), diff --git a/desktop/src-tauri/src/commands/managed_agent_definition.rs b/desktop/src-tauri/src/commands/managed_agent_definition.rs index 32753807486..76a37f98df3 100644 --- a/desktop/src-tauri/src/commands/managed_agent_definition.rs +++ b/desktop/src-tauri/src/commands/managed_agent_definition.rs @@ -33,6 +33,15 @@ pub(super) fn apply_model_provider_prompt_update( } if let Some(provider_update) = provider { record.provider = provider_update; + if record + .env_vars + .get(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV) + != record.provider.as_ref() + { + record + .env_vars + .remove(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV); + } } if let Some(prompt_update) = system_prompt { record.system_prompt = prompt_update; diff --git a/desktop/src-tauri/src/commands/personas/inbound.rs b/desktop/src-tauri/src/commands/personas/inbound.rs index 5214dd5a27e..e1e2a4007ce 100644 --- a/desktop/src-tauri/src/commands/personas/inbound.rs +++ b/desktop/src-tauri/src/commands/personas/inbound.rs @@ -531,6 +531,7 @@ fn apply_inbound_persona(personas: &mut Vec, inbound: AgentDefi local.runtime = inbound.runtime; local.model = inbound.model; local.provider = inbound.provider; + crate::managed_agents::openai_env::project_buzz_agent_definition(local); local.name_pool = inbound.name_pool; local.respond_to = inbound.respond_to; local.respond_to_allowlist = inbound.respond_to_allowlist; diff --git a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs index fbfede35886..4567afa8409 100644 --- a/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs +++ b/desktop/src-tauri/src/commands/personas/inbound/inbound_tests.rs @@ -33,6 +33,50 @@ fn local_in_app() -> AgentDefinition { } } +#[test] +fn inbound_legacy_openai_provider_keeps_local_custom_origin_compatible() { + let mut local = local_in_app(); + local.env_vars = BTreeMap::from([ + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "compat-key".to_string(), + ), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://gateway.example/v1".to_string(), + ), + ]); + let mut personas = vec![local]; + let mut inbound = inbound_for(UUID, "Remote"); + inbound.provider = Some("openai".to_string()); + + apply_inbound_persona(&mut personas, inbound); + + assert_eq!(personas[0].provider.as_deref(), Some("openai")); +} + +#[test] +fn inbound_explicit_official_key_preserves_openai_provider() { + let mut local = local_in_app(); + local.env_vars = BTreeMap::from([ + ("OPENAI_API_KEY".to_string(), "official-key".to_string()), + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "compat-key".to_string(), + ), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://gateway.example/v1".to_string(), + ), + ]); + let mut personas = vec![local]; + let inbound = inbound_for(UUID, "Remote"); + + apply_inbound_persona(&mut personas, inbound); + + assert_eq!(personas[0].provider.as_deref(), Some("openai")); +} + /// An inbound persona as `persona_from_event` would produce it: id = d-tag, /// slug = Some(d-tag), empty env_vars, source_team None. fn inbound_for(d_tag: &str, display_name: &str) -> AgentDefinition { diff --git a/desktop/src-tauri/src/commands/personas/mod.rs b/desktop/src-tauri/src/commands/personas/mod.rs index 3be24d04131..a08b7cb8446 100644 --- a/desktop/src-tauri/src/commands/personas/mod.rs +++ b/desktop/src-tauri/src/commands/personas/mod.rs @@ -49,6 +49,12 @@ pub async fn list_personas(app: AppHandle) -> Result, Strin .lock() .map_err(|error| error.to_string())?; let mut personas = load_personas(&app)?; + for persona in &mut personas { + crate::managed_agents::openai_env::project_buzz_agent_definition(persona); + persona + .env_vars + .remove(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV); + } pending::project_active_persona_sharing(&app, &state, &mut personas); Ok(personas) }) diff --git a/desktop/src-tauri/src/commands/personas/update.rs b/desktop/src-tauri/src/commands/personas/update.rs index b3830e62b52..c9eb5660ae1 100644 --- a/desktop/src-tauri/src/commands/personas/update.rs +++ b/desktop/src-tauri/src/commands/personas/update.rs @@ -119,6 +119,15 @@ pub(super) async fn update_persona_with( persona.runtime = runtime; persona.model = model; persona.provider = provider; + if persona + .env_vars + .get(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV) + != persona.provider.as_ref() + { + persona + .env_vars + .remove(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV); + } persona.name_pool = input .name_pool .into_iter() diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 272c03348b9..fa20cdbb9bc 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -20,6 +20,7 @@ pub(crate) mod git_bash; pub(crate) mod global_config; mod managed_node_paths; mod nest; +pub(crate) mod openai_env; pub(crate) mod parallelism; mod persona_avatars; pub(crate) mod persona_events; diff --git a/desktop/src-tauri/src/managed_agents/openai_env.rs b/desktop/src-tauri/src/managed_agents/openai_env.rs new file mode 100644 index 00000000000..a73a91ddc87 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/openai_env.rs @@ -0,0 +1,63 @@ +use std::collections::BTreeMap; + +pub(crate) const MIGRATED_OPENAI_PROVIDER_ENV: &str = "OPENAI_COMPAT_MIGRATED_PROVIDER"; + +pub(crate) fn project_buzz_agent_definition( + definition: &mut crate::managed_agents::AgentDefinition, +) { + let is_buzz_agent = definition + .runtime + .as_deref() + .and_then(crate::managed_agents::known_acp_runtime_exact) + .is_some_and(|runtime| runtime.id == "buzz-agent"); + if !is_buzz_agent { + return; + } + if let Some(provider) = definition.env_vars.get(MIGRATED_OPENAI_PROVIDER_ENV) { + definition.provider = Some(provider.clone()); + } +} + +pub(crate) fn effective_runtime_provider( + command: &str, + configured_provider: Option, + env: &BTreeMap, +) -> Option { + if crate::managed_agents::known_acp_runtime(command) + .is_some_and(|runtime| runtime.id == "buzz-agent") + { + env.get("BUZZ_AGENT_PROVIDER") + .cloned() + .or(configured_provider) + } else { + configured_provider + } +} + +pub(crate) fn canonicalize_openai_provider_env( + env: &mut BTreeMap, + provider: Option<&str>, +) -> Option { + let migrated_provider = env.remove(MIGRATED_OPENAI_PROVIDER_ENV); + let provider = migrated_provider + .as_deref() + .map(str::trim) + .filter(|value| matches!(*value, "openai" | "openai-compat")) + .map(str::to_string) + .or_else(|| provider.map(str::trim).map(str::to_ascii_lowercase)); + match provider.as_deref() { + Some("openai") => { + env.remove("OPENAI_COMPAT_BASE_URL"); + env.remove("OPENAI_COMPAT_API_KEY"); + } + Some("openai-compat") => { + env.remove("OPENAI_API_KEY"); + env.entry("OPENAI_COMPAT_API_KEY".to_string()).or_default(); + } + _ => {} + } + if let (Some(value), Some(slot)) = (provider.as_ref(), env.get_mut("BUZZ_AGENT_PROVIDER")) { + *slot = value.clone(); + } + provider +} diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index f7f5d5c5d0e..beee99449ac 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -269,6 +269,15 @@ fn resolve_effective_agent_env_with_def( ); env.extend(user_env); + if crate::managed_agents::known_acp_runtime(&effective_command) + .is_some_and(|runtime| runtime.id == "buzz-agent") + { + let _ = crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut env, + effective_provider.as_deref(), + ); + } + // Buzz shared compute is a native Buzz provider. Translate it to buzz-agent's // OpenAI-compatible transport only in the effective runtime environment. #[cfg(feature = "mesh-llm")] @@ -286,7 +295,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 +307,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 { @@ -387,7 +396,7 @@ impl AgentReadiness { /// present in the effective env or as structured fields). Additionally, /// provider-specific credentials are required: /// - `anthropic` → `ANTHROPIC_API_KEY` -/// - `openai` → `OPENAI_COMPAT_API_KEY` +/// - `openai` → `OPENAI_API_KEY` /// - `databricks` / `databricks_v2` → `DATABRICKS_HOST` (token optional — /// OAuth PKCE is the fallback) /// * **claude**: a successful `claude auth status` probe. @@ -445,6 +454,8 @@ fn collect_missing_requirements( } } +mod openai_origin; + /// Requirements for buzz-agent (provider + model + provider-specific creds). fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { let mut missing = Vec::new(); @@ -454,28 +465,18 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { missing.push(Requirement::GitBash); } - // Provider is required — maps to BUZZ_AGENT_PROVIDER in the effective env. - // An empty string is treated as absent: a key set to "" is not a valid - // provider and must not pass the readiness gate. let provider = effective .env .get("BUZZ_AGENT_PROVIDER") - .filter(|v| !v.is_empty()) - .map(String::as_str); + .map(|value| value.trim().to_ascii_lowercase()) + .filter(|value| !value.is_empty()); if provider.is_none() { missing.push(Requirement::NormalizedField { field: "provider".to_string(), }); } - // Model is required — maps to BUZZ_AGENT_MODEL in the effective env. - // Same empty-string treatment as provider. - // Also accept provider-specific model fallback keys, matching buzz-agent's - // own config.rs `from_env()` resolution order (e.g. DATABRICKS_MODEL for - // databricks/databricks_v2, ANTHROPIC_MODEL for anthropic, etc.). The - // baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL, - // so without this fallback agents baked from releases appear "not ready". - let provider_model_key = match provider { + let provider_model_key = match provider.as_deref() { Some("databricks") | Some("databricks_v2") | Some("databricks-v2") => { Some("DATABRICKS_MODEL") } @@ -503,7 +504,7 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { // A key present with an empty value is treated as absent — matching the // dialog's (envVars[key] ?? "").length === 0 emptiness check. let env_key_missing = |key: &str| effective.env.get(key).is_none_or(|v| v.is_empty()); - match provider { + match provider.as_deref() { Some("anthropic") if env_key_missing("ANTHROPIC_API_KEY") => { missing.push(Requirement::EnvKey { @@ -511,9 +512,15 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { }); } Some("openai") - if env_key_missing("OPENAI_COMPAT_API_KEY") => { + if env_key_missing("OPENAI_API_KEY") => { missing.push(Requirement::EnvKey { - key: "OPENAI_COMPAT_API_KEY".to_string(), + key: "OPENAI_API_KEY".to_string(), + }); + } + Some("openai-compat") + if env_key_missing("OPENAI_COMPAT_BASE_URL") => { + missing.push(Requirement::EnvKey { + key: "OPENAI_COMPAT_BASE_URL".to_string(), }); } Some("databricks") | Some("databricks_v2") | Some("databricks-v2") @@ -536,6 +543,10 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec { } } + if provider.as_deref() == Some("openai") { + openai_origin::require_safe_official_origin(effective, &mut missing); + } + missing } @@ -560,16 +571,17 @@ fn goose_requirements( let provider = effective .env .get("GOOSE_PROVIDER") - .filter(|v| !v.is_empty()) - .map(String::as_str); + .map(|value| value.trim().to_ascii_lowercase()) + .filter(|value| !value.is_empty()); // Effective provider for credential checking: prefer env layer, then file. - let effective_provider = provider.or_else(|| { - file_cfg - .as_ref() - .and_then(|c| c.provider.as_deref()) - .filter(|v| !v.is_empty()) - }); + let file_provider = file_cfg + .as_ref() + .and_then(|config| config.provider.as_deref()) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_ascii_lowercase); + let effective_provider = provider.as_deref().or(file_provider.as_deref()); if provider.is_none() { // Silenced if the file config provides a provider. @@ -732,22 +744,6 @@ mod tests { })); } - #[test] - fn buzz_agent_missing_openai_key_returns_not_ready() { - let env = make_env( - "buzz-agent", - env_with(&[ - ("BUZZ_AGENT_PROVIDER", "openai"), - ("BUZZ_AGENT_MODEL", "gpt-4o"), - ]), - ); - let result = agent_readiness(&env); - assert!(!result.is_ready()); - assert!(result.requirements().contains(&Requirement::EnvKey { - key: "OPENAI_COMPAT_API_KEY".to_string() - })); - } - #[test] fn buzz_agent_anthropic_with_all_fields_is_ready() { let env = make_env( @@ -1203,9 +1199,7 @@ mod tests { ); } } - // ── codex readiness version gate ─────────────────────────────────────── - /// Build a minimal `KnownAcpRuntime` for testing the codex version gate. /// `adapter_commands` are the exact strings passed to `find_command` — use /// `&["codex-acp"]` when the binary is on PATH, or `&[]` @@ -1249,48 +1243,40 @@ mod tests { auth_probe_args: None, } } - /// Build a temp dir containing a `codex-acp` script with the given body, /// prepend it to PATH, and clear the resolve cache. Returns the temp dir /// and the original PATH string for restoration. #[cfg(unix)] fn setup_temp_codex_acp(script_body: &str) -> (tempfile::TempDir, String) { use std::os::unix::fs::PermissionsExt; - let dir = tempfile::tempdir().expect("create temp dir"); let bin = dir.path().join("codex-acp"); std::fs::write(&bin, script_body).expect("write script"); std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)) .expect("chmod script"); - let original_path = std::env::var("PATH").unwrap_or_default(); let new_path = format!("{}:{}", dir.path().display(), original_path); std::env::set_var("PATH", &new_path); crate::managed_agents::clear_resolve_cache(); - (dir, original_path) } - #[cfg(unix)] fn leaked_adapter_commands(bin: &std::path::Path) -> &'static [&'static str] { let command = Box::leak(bin.display().to_string().into_boxed_str()); Box::leak(vec![command as &'static str].into_boxed_slice()) } - /// Restore PATH and clear the resolve cache after a PATH-mutating test. #[cfg(unix)] fn restore_path(original: &str) { std::env::set_var("PATH", original); crate::managed_agents::clear_resolve_cache(); } - /// Codex readiness: outdated adapter (exits non-zero) → AdapterOutdated, /// login probe skipped. #[cfg(unix)] #[test] fn cli_login_requirements_codex_outdated_adapter_emits_adapter_outdated() { let _guard = crate::managed_agents::lock_path_mutex(); - let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\nexit 1\n"); let exe = present_binary_str(); // Use the fixture's absolute adapter path here. Bare `codex-acp` @@ -1305,10 +1291,8 @@ mod tests { "run `codex login`", &rt, ); - restore_path(&orig); drop(dir); - assert!( !reqs.is_empty(), "outdated codex adapter must produce a requirement; got {reqs:?}" @@ -1326,14 +1310,12 @@ mod tests { panic!("expected CliLogin requirement; got {:?}", reqs[0]); } } - /// Codex readiness: adapter exits 0 but output is not a parseable version /// → AdapterOutdated (garbage output treated as outdated, same as non-zero). #[cfg(unix)] #[test] fn cli_login_requirements_codex_garbage_version_output_emits_adapter_outdated() { let _guard = crate::managed_agents::lock_path_mutex(); - let (dir, orig) = setup_temp_codex_acp("#!/bin/sh\necho 'not a version string'\nexit 0\n"); let exe = present_binary_str(); let rt = make_codex_runtime( @@ -1345,10 +1327,8 @@ mod tests { "run `codex login`", &rt, ); - restore_path(&orig); drop(dir); - assert!( !reqs.is_empty(), "garbage version output must produce a requirement; got {reqs:?}" @@ -1366,9 +1346,7 @@ mod tests { panic!("expected CliLogin requirement; got {:?}", reqs[0]); } } - // ── custom/unknown command ───────────────────────────────────────────── - #[test] fn unknown_command_is_always_ready() { // Since Phase B-7 (readiness exec-check), unknown/custom commands that are @@ -1381,7 +1359,6 @@ mod tests { "unknown/custom command present in PATH should be Ready" ); } - #[test] fn unknown_command_missing_from_path_is_not_ready() { let env = make_env("my-custom-harness-that-does-not-exist", BTreeMap::new()); @@ -1397,14 +1374,11 @@ mod tests { "should surface MissingBinary requirement" ); } - // ── AgentReadiness helpers ───────────────────────────────────────────── - #[test] fn agent_readiness_ready_has_empty_requirements() { assert!(AgentReadiness::Ready.requirements().is_empty()); } - #[test] fn agent_readiness_not_ready_exposes_requirements() { let r = AgentReadiness::NotReady { @@ -1415,9 +1389,7 @@ mod tests { assert!(!r.is_ready()); assert_eq!(r.requirements().len(), 1); } - // ── Requirement serialization ───────────────────────────────────────── - #[test] fn requirement_serializes_with_surface_tag() { let r = Requirement::NormalizedField { @@ -1427,13 +1399,11 @@ mod tests { assert_eq!(json["surface"], "normalized_field"); assert_eq!(json["field"], "provider"); } - #[test] fn git_bash_requirement_serializes_correctly() { let json = serde_json::to_value(Requirement::GitBash).unwrap(); assert_eq!(json, serde_json::json!({ "surface": "git_bash" })); } - #[test] fn env_key_requirement_serializes_correctly() { let r = Requirement::EnvKey { @@ -1443,7 +1413,6 @@ mod tests { assert_eq!(json["surface"], "env_key"); assert_eq!(json["key"], "ANTHROPIC_API_KEY"); } - #[test] fn cli_login_requirement_serializes_correctly() { let r = Requirement::CliLogin { @@ -1460,9 +1429,7 @@ mod tests { assert!(json["probe_args"].is_array()); assert!(json["setup_copy"].as_str().unwrap().contains("codex login")); } - // ── resolve_effective_agent_env ───────────────────────────────────────── - #[test] fn resolve_effective_agent_env_user_env_wins_over_structured_fields() { // User env_vars must win over baked defaults; in OSS builds baked map is empty, @@ -1473,7 +1440,6 @@ mod tests { "BUZZ_AGENT_MODEL".to_string(), "claude-opus-4-5".to_string(), ); - // Minimal record: only the fields resolve_effective_agent_env reads. let record = crate::managed_agents::types::ManagedAgentRecord { pubkey: "test-pubkey".to_string(), @@ -1532,10 +1498,8 @@ mod tests { relay_mesh: None, effort_level: None, }; - let runtime = known_acp_runtime_exact("buzz-agent"); let effective = resolve_effective_agent_env(&record, &[], runtime, &Default::default()); - // User env_vars must be present in the output (last-write-wins). assert_eq!( effective.env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), @@ -1546,6 +1510,7 @@ mod tests { Some("claude-opus-4-5") ); } + // ── provider-specific model fallback tests ──────────────────────────── #[test] fn buzz_agent_databricks_v2_with_databricks_model_but_no_buzz_agent_model_is_ready() { @@ -1564,7 +1529,6 @@ mod tests { "DATABRICKS_MODEL must satisfy the model requirement for databricks_v2" ); } - #[test] fn buzz_agent_databricks_v2_hyphen_alias_with_databricks_model_is_ready() { // buzz-agent accepts both "databricks_v2" and "databricks-v2". The @@ -1582,7 +1546,6 @@ mod tests { "databricks-v2 alias with DATABRICKS_MODEL must be Ready" ); } - #[test] fn buzz_agent_databricks_hyphen_alias_missing_host_returns_not_ready() { // The hyphen alias "databricks-v2" requires DATABRICKS_HOST just like @@ -1607,7 +1570,6 @@ mod tests { "missing requirements must include DATABRICKS_HOST; got {reqs:?}" ); } - #[test] fn buzz_agent_databricks_v1_with_databricks_model_but_no_buzz_agent_model_is_ready() { // V1 (Model Serving) also resolves DATABRICKS_MODEL — same fallback applies. @@ -1624,7 +1586,6 @@ mod tests { "DATABRICKS_MODEL must satisfy the model requirement for databricks (V1)" ); } - #[test] fn buzz_agent_anthropic_with_anthropic_model_but_no_buzz_agent_model_is_ready() { let env = make_env( @@ -1640,7 +1601,6 @@ mod tests { "ANTHROPIC_MODEL must satisfy the model requirement for anthropic" ); } - #[test] fn buzz_agent_openai_with_openai_compat_model_but_no_buzz_agent_model_is_ready() { let env = make_env( @@ -1648,7 +1608,7 @@ mod tests { env_with(&[ ("BUZZ_AGENT_PROVIDER", "openai"), ("OPENAI_COMPAT_MODEL", "gpt-4o"), - ("OPENAI_COMPAT_API_KEY", "sk-test"), + ("OPENAI_API_KEY", "sk-test"), ]), ); assert!( @@ -1656,7 +1616,6 @@ mod tests { "OPENAI_COMPAT_MODEL must satisfy the model requirement for openai" ); } - #[test] fn buzz_agent_empty_provider_model_fallback_key_is_not_ready() { // An empty DATABRICKS_MODEL with no BUZZ_AGENT_MODEL must still be NotReady. @@ -1679,9 +1638,7 @@ mod tests { field: "model".to_string() })); } - // ── OpenRouter readiness ───────────────────────────────────────────── - #[test] fn buzz_agent_openrouter_with_all_fields_is_ready() { let env = make_env( @@ -1698,7 +1655,6 @@ mod tests { "openrouter with all fields should be ready" ); } - #[test] fn buzz_agent_openrouter_missing_key_returns_not_ready() { let env = make_env( @@ -1714,7 +1670,6 @@ mod tests { key: "OPENROUTER_API_KEY".to_string() })); } - #[test] fn buzz_agent_openrouter_with_provider_model_fallback_is_ready() { let env = make_env( @@ -1732,9 +1687,9 @@ mod tests { ); } } - -// Goose file-config-aware requirement tests live in a sibling file so this -// module stays under the desktop file-size ratchet. #[cfg(test)] #[path = "readiness_goose_file_config_tests.rs"] mod goose_file_config_tests; +#[cfg(test)] +#[path = "readiness_openai_tests.rs"] +mod openai_tests; diff --git a/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs b/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs new file mode 100644 index 00000000000..e785b343096 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness/openai_origin.rs @@ -0,0 +1,50 @@ +use super::{EffectiveAgentEnv, Requirement}; + +pub(super) fn require_safe_official_origin( + effective: &EffectiveAgentEnv, + missing: &mut Vec, +) { + let safe = effective + .env + .get("OPENAI_COMPAT_BASE_URL") + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .is_none_or(|value| value.trim_end_matches('/') == "https://api.openai.com/v1"); + if !safe { + missing.push(Requirement::ConfigInvalid { + message: "remove `OPENAI_COMPAT_BASE_URL` or switch the provider to `openai-compat`" + .to_string(), + }); + } +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + #[test] + fn custom_origin_is_unsafe_for_official_openai() { + let effective = EffectiveAgentEnv { + env: BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://gateway.example/v1".to_string(), + )]), + config_file_path: None, + effective_command: "buzz-agent".to_string(), + }; + let mut missing = Vec::new(); + + require_safe_official_origin(&effective, &mut missing); + + assert_eq!( + missing, + 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/readiness_openai_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs new file mode 100644 index 00000000000..33c706ecd7a --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/readiness_openai_tests.rs @@ -0,0 +1,154 @@ +use super::*; + +fn make_env(command: &str, pairs: &[(&str, &str)]) -> EffectiveAgentEnv { + EffectiveAgentEnv { + env: pairs + .iter() + .map(|(key, value)| (key.to_string(), value.to_string())) + .collect(), + config_file_path: None, + effective_command: command.to_string(), + } +} + +#[test] +fn provider_env_canonicalization_isolates_consumers() { + let shared = BTreeMap::from([ + ("OPENAI_API_KEY".into(), "official".into()), + ("OPENAI_COMPAT_API_KEY".into(), "compat".into()), + ( + "OPENAI_COMPAT_BASE_URL".into(), + "http://localhost:11434/v1".into(), + ), + ]); + let mut official = shared.clone(); + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut official, + Some("openai"), + ); + assert_eq!( + official.get("OPENAI_API_KEY").map(String::as_str), + Some("official") + ); + assert!(!official.contains_key("OPENAI_COMPAT_API_KEY")); + assert!(!official.contains_key("OPENAI_COMPAT_BASE_URL")); + + let mut compatible = shared; + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut compatible, + Some("openai-compat"), + ); + assert!(!compatible.contains_key("OPENAI_API_KEY")); + assert_eq!( + compatible.get("OPENAI_COMPAT_API_KEY").map(String::as_str), + Some("compat") + ); +} + +#[test] +fn migrated_identity_overrides_legacy_provider_without_secret_inference() { + let mut env = BTreeMap::from([ + ("BUZZ_AGENT_PROVIDER".into(), "openai".into()), + ( + crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV.into(), + "openai-compat".into(), + ), + ("OPENAI_COMPAT_API_KEY".into(), "legacy".into()), + ( + "OPENAI_COMPAT_BASE_URL".into(), + "http://localhost:11434/v1".into(), + ), + ]); + assert_eq!( + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut env, + Some("openai") + ) + .as_deref(), + Some("openai-compat") + ); + assert_eq!( + env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("openai-compat") + ); +} + +#[test] +fn equal_credentials_do_not_override_explicit_official_provider() { + let mut env = BTreeMap::from([ + ("BUZZ_AGENT_PROVIDER".into(), "openai".into()), + ("OPENAI_API_KEY".into(), "same-secret".into()), + ("OPENAI_COMPAT_API_KEY".into(), "same-secret".into()), + ( + "OPENAI_COMPAT_BASE_URL".into(), + "https://gateway.example/v1".into(), + ), + ]); + assert_eq!( + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut env, + Some("openai") + ) + .as_deref(), + Some("openai") + ); + assert!(!env.contains_key("OPENAI_COMPAT_API_KEY")); + assert!(!env.contains_key("OPENAI_COMPAT_BASE_URL")); +} + +#[test] +fn non_buzz_harness_keeps_its_configured_provider() { + let env = BTreeMap::from([("BUZZ_AGENT_PROVIDER".into(), "openai-compat".into())]); + assert_eq!( + crate::managed_agents::openai_env::effective_runtime_provider( + "goose", + Some("openai".into()), + &env, + ) + .as_deref(), + Some("openai") + ); +} + +#[test] +fn keyless_compatible_provider_gets_explicit_blank_credential() { + let mut env = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".into(), + "http://localhost:11434/v1".into(), + )]); + crate::managed_agents::openai_env::canonicalize_openai_provider_env( + &mut env, + Some("openai-compat"), + ); + assert_eq!( + env.get("OPENAI_COMPAT_API_KEY").map(String::as_str), + Some("") + ); +} + +#[test] +fn readiness_enforces_openai_provider_boundaries() { + let official = make_env( + "buzz-agent", + &[ + ("BUZZ_AGENT_PROVIDER", "openai"), + ("BUZZ_AGENT_MODEL", "gpt-4o"), + ("OPENAI_COMPAT_API_KEY", "must-not-cross-provider-boundary"), + ], + ); + assert!(agent_readiness(&official) + .requirements() + .contains(&Requirement::EnvKey { + key: "OPENAI_API_KEY".to_string(), + })); + + let keyless_compat = make_env( + "buzz-agent", + &[ + ("BUZZ_AGENT_PROVIDER", "OpenAI-Compat"), + ("BUZZ_AGENT_MODEL", "llama3"), + ("OPENAI_COMPAT_BASE_URL", "http://localhost:11434/v1"), + ], + ); + assert!(agent_readiness(&keyless_compat).is_ready()); +} diff --git a/desktop/src-tauri/src/managed_agents/relay_mesh.rs b/desktop/src-tauri/src/managed_agents/relay_mesh.rs index 3858212bbba..2a3934594c8 100644 --- a/desktop/src-tauri/src/managed_agents/relay_mesh.rs +++ b/desktop/src-tauri/src/managed_agents/relay_mesh.rs @@ -42,7 +42,10 @@ pub fn apply_relay_mesh_env( return; } let model = relay_mesh_wire_model(model.unwrap_or(RELAY_MESH_AUTO_MODEL_ID)).to_string(); - env.insert("BUZZ_AGENT_PROVIDER".to_string(), "openai".to_string()); + env.insert( + "BUZZ_AGENT_PROVIDER".to_string(), + "openai-compat".to_string(), + ); env.insert("BUZZ_AGENT_MODEL".to_string(), model.clone()); env.insert( "OPENAI_COMPAT_BASE_URL".to_string(), @@ -162,6 +165,10 @@ mod tests { env.get("OPENAI_COMPAT_MODEL").map(String::as_str), Some(RELAY_MESH_VIRTUAL_MODEL_ID) ); + assert_eq!( + env.get("BUZZ_AGENT_PROVIDER").map(String::as_str), + Some("openai-compat") + ); } /// A blank stored model is the legacy encoding of the same intent. diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0ce5ca7b219..2d17f8ca7e5 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use tauri::AppHandle; use super::agent_env::{build_buzz_agent_provider_defaults, idle_pool_sleep_env}; - +use super::openai_env::effective_runtime_provider; use crate::{ managed_agents::{ append_log_marker, known_acp_runtime, login_shell_path, managed_agent_log_path, @@ -24,7 +24,7 @@ pub(crate) use super::access_policy::{build_respond_to_env_with_policy, RespondT mod metadata; pub(crate) use metadata::{ apply_agent_display_env, resolve_session_title, runtime_metadata_env_vars, - DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, + scrub_ambient_openai_env, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, }; mod stop; @@ -204,7 +204,7 @@ pub fn build_managed_agent_summary( personas, global_config, ); - let (effective_model, effective_provider, effective_prompt, model_source) = match effective_cfg + let (effective_model, configured_provider, effective_prompt, model_source) = match effective_cfg { crate::managed_agents::effective_config::EffectiveConfigResult::Resolved(cfg) => { let source = cfg.model.source.clone(); @@ -293,6 +293,8 @@ pub fn build_managed_agent_summary( env: Default::default(), } }); + let effective_provider = + effective_runtime_provider(&descriptor.command, configured_provider, &descriptor.env); let effective_mcp_command = known_acp_runtime(&descriptor.command) .and_then(|r| r.mcp_command) .unwrap_or("") @@ -582,7 +584,6 @@ pub fn spawn_agent_child( config_file_path: runtime_meta.and_then(|r| r.config_file_path), effective_command: descriptor.command.clone(), }; - // Compute the optional payload before touching the command. let setup_payload_json = if let AgentReadiness::NotReady { requirements } = agent_readiness(&effective) { let reqs: Vec = requirements @@ -596,6 +597,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, @@ -709,7 +713,9 @@ pub fn spawn_agent_child( let mesh_model_id = effective_cfg.relay_mesh_model_id(); let effective_prompt = effective_cfg.system_prompt.value; let effective_model = effective_cfg.model.value; - let effective_provider = effective_cfg.provider.value; + let configured_provider = effective_cfg.provider.value; + let effective_provider = + effective_runtime_provider(&descriptor.command, configured_provider, &descriptor.env); if let Some(prompt) = &effective_prompt { command.env("BUZZ_ACP_SYSTEM_PROMPT", prompt); @@ -804,17 +810,14 @@ pub fn spawn_agent_child( ); } - // User env (descriptor.env): fully-layered floor→runtime→definition→global→persona→agent, - // reserved-key filtered. Written last so user-explicit values win over Buzz-set env. for (key, value) in &descriptor.env { command.env(key, value); } - // B5: carry persisted effort; harness resolves thought_level configId at first session. - // Written AFTER descriptor.env so the canonical persisted value wins over any - // user-supplied BUZZ_ACP_EFFORT_LEVEL entry, mirroring the A1 model-authority pattern + if runtime_meta.is_some_and(|runtime| runtime.id == "buzz-agent") { + scrub_ambient_openai_env(&mut command, effective_provider.as_deref(), &descriptor.env); + } // (ANTHROPIC_MODEL is applied post-loop for the same reason). When effort_level is - // None there is no canonical value to assert, so env passthrough stands — user env // legitimately seeds startup effort in that case. apply_effort_env(&mut command, record.effort_level.as_deref()); @@ -903,8 +906,6 @@ pub fn spawn_agent_child( None }; - // Receipt persistence belongs to the caller's atomic register transition. - // Windows: assign the harness to a Job Object so its whole tree dies with // the handle. The Unix process-group equivalent is set above. #[cfg(windows)] @@ -994,6 +995,5 @@ pub fn start_managed_agent_process( #[cfg(test)] mod test_fixtures; - #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs index 5aef424ea61..da21338716c 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/metadata.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/metadata.rs @@ -24,6 +24,26 @@ pub(crate) fn runtime_metadata_env_vars<'a>( vars } +pub(crate) fn scrub_ambient_openai_env( + command: &mut std::process::Command, + effective_provider: Option<&str>, + env: &std::collections::BTreeMap, +) { + match effective_provider.map(str::trim) { + Some(provider) if provider.eq_ignore_ascii_case("openai") => { + command.env_remove("OPENAI_COMPAT_API_KEY"); + command.env_remove("OPENAI_COMPAT_BASE_URL"); + } + Some(provider) if provider.eq_ignore_ascii_case("openai-compat") => { + command.env_remove("OPENAI_API_KEY"); + if !env.contains_key("OPENAI_COMPAT_API_KEY") { + command.env_remove("OPENAI_COMPAT_API_KEY"); + } + } + _ => {} + } +} + /// Env var carrying the session title to the harness. Shared with /// `spawn_snapshot` so the restart badge records the same key the spawn writes. pub(crate) const SESSION_TITLE_ENV_VAR: &str = "BUZZ_ACP_SESSION_TITLE"; @@ -76,7 +96,41 @@ pub(crate) fn resolve_session_title(display_name: Option<&str>, name: &str) -> O #[cfg(test)] mod tests { - use super::resolve_session_title; + use super::{resolve_session_title, scrub_ambient_openai_env}; + use std::collections::BTreeMap; + + #[test] + fn keyless_openai_compat_scrubs_parent_process_credential() { + let mut command = std::process::Command::new("echo"); + command.env("OPENAI_COMPAT_API_KEY", "ambient-secret"); + scrub_ambient_openai_env(&mut command, Some("openai-compat"), &BTreeMap::new()); + assert!(command + .get_envs() + .any(|(key, value)| key == "OPENAI_COMPAT_API_KEY" && value.is_none())); + } + + #[test] + fn configured_openai_compat_key_is_preserved() { + let mut command = std::process::Command::new("echo"); + let env = BTreeMap::from([("OPENAI_COMPAT_API_KEY".to_string(), "key".to_string())]); + scrub_ambient_openai_env(&mut command, Some("openai-compat"), &env); + assert!(!command + .get_envs() + .any(|(key, value)| key == "OPENAI_COMPAT_API_KEY" && value.is_none())); + } + + #[test] + fn official_openai_scrubs_ambient_compat_routing() { + let mut command = std::process::Command::new("echo"); + command.env("OPENAI_COMPAT_API_KEY", "ambient-secret"); + command.env("OPENAI_COMPAT_BASE_URL", "http://localhost:11434/v1"); + scrub_ambient_openai_env(&mut command, Some("openai"), &BTreeMap::new()); + for key in ["OPENAI_COMPAT_API_KEY", "OPENAI_COMPAT_BASE_URL"] { + assert!(command + .get_envs() + .any(|(name, value)| name == key && value.is_none())); + } + } #[test] fn resolve_session_title_prefers_display_name() { diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs index 8a6f68a693d..0e7d9fc0867 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot.rs @@ -291,12 +291,18 @@ pub(crate) fn prospective_spawn_config_snapshot( // definition) resolves as if all three were absent: `spawn_agent_child` // refuses to spawn an orphan regardless, and `eligible_restart_diff` // suppresses the badge for one. - let (prompt, model, provider) = match resolve_effective_config(record, personas, global) { - EffectiveConfigResult::Resolved(cfg) => { - (cfg.system_prompt.value, cfg.model.value, cfg.provider.value) - } - EffectiveConfigResult::OrphanedInstance { .. } => (None, None, None), - }; + let (prompt, model, configured_provider) = + match resolve_effective_config(record, personas, global) { + EffectiveConfigResult::Resolved(cfg) => { + (cfg.system_prompt.value, cfg.model.value, cfg.provider.value) + } + EffectiveConfigResult::OrphanedInstance { .. } => (None, None, None), + }; + let provider = crate::managed_agents::openai_env::effective_runtime_provider( + &descriptor.command, + configured_provider, + &descriptor.env, + ); SpawnConfigSnapshot::from_inputs(SpawnConfigInputs { record, diff --git a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs index b007e0b2ffa..b32d1d4423a 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs @@ -34,6 +34,33 @@ fn snapshot( snapshot_with_policy(record, personas, teams, workspace_relay, global, false) } +#[test] +fn legacy_custom_buzz_agent_snapshot_uses_runtime_provider_identity() { + let mut rec = record(); + rec.runtime = Some("buzz-agent".into()); + rec.provider = Some("openai".into()); + rec.env_vars.insert( + crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV.into(), + "openai-compat".into(), + ); + rec.env_vars + .insert("OPENAI_COMPAT_API_KEY".into(), "shared-key".into()); + rec.env_vars.insert( + "OPENAI_COMPAT_BASE_URL".into(), + "https://gateway.example/v1".into(), + ); + let global = GlobalAgentConfig { + env_vars: BTreeMap::from([("OPENAI_API_KEY".into(), "shared-key".into())]), + ..Default::default() + }; + + let value = snapshot(&rec, &[], &[], "wss://ws.example", &global); + assert_eq!( + value.get("provider").and_then(serde_json::Value::as_str), + Some("openai-compat") + ); +} + /// `snapshot` with the fixed no-persona/no-team/default-global shape the effort /// tests share, so their call sites read as `snap(&record)` instead of wrapping. fn snap(record: &ManagedAgentRecord) -> serde_json::Value { diff --git a/desktop/src-tauri/src/migration.rs b/desktop/src-tauri/src/migration.rs index 1e22d7aaeca..86dd2ebe589 100644 --- a/desktop/src-tauri/src/migration.rs +++ b/desktop/src-tauri/src/migration.rs @@ -189,6 +189,7 @@ fn run_boot_migrations_inner(app: &tauri::AppHandle, reset_completed: bool) { reconcile_provider_mcp_commands(app); reconcile_databricks_v1_to_v2(app); materialize_agent_runtimes(app); + openai_credentials::migrate_openai_credentials(app); } /// Copy one-time app state from the legacy app identifier directory to @@ -1366,6 +1367,7 @@ pub fn migrate_persona_provider_to_runtime(app: &tauri::AppHandle) { rename_provider_to_runtime_in_personas(&path); } mod materialize; +mod openai_credentials; pub use materialize::materialize_agent_runtimes; mod fold; pub use fold::fold_personas_into_agent_store; @@ -1379,18 +1381,16 @@ pub(crate) use pollen::*; mod team_suffix; pub use team_suffix::strip_baked_team_instructions; +#[cfg(test)] +#[path = "migration_avatar_tests.rs"] +mod avatar_tests; #[cfg(test)] #[path = "migration_test_support.rs"] mod test_support; - #[cfg(test)] #[path = "migration_tests.rs"] mod tests; -#[cfg(test)] -#[path = "migration_avatar_tests.rs"] -mod avatar_tests; - #[cfg(test)] #[path = "migration_command_tests.rs"] mod command_tests; diff --git a/desktop/src-tauri/src/migration/openai_credentials.rs b/desktop/src-tauri/src/migration/openai_credentials.rs new file mode 100644 index 00000000000..25669fd377e --- /dev/null +++ b/desktop/src-tauri/src/migration/openai_credentials.rs @@ -0,0 +1,857 @@ +use std::{ + collections::{BTreeMap, HashMap}, + path::{Path, PathBuf}, +}; + +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; + +use crate::managed_agents::{ + effective_config::{resolve_effective_config, ConfigSource, EffectiveConfigResult}, + AgentDefinition, GlobalAgentConfig, ManagedAgentRecord, +}; + +const OPENAI_API_KEY: &str = "OPENAI_API_KEY"; +const OPENAI_COMPAT_API_KEY: &str = "OPENAI_COMPAT_API_KEY"; +const OPENAI_COMPAT_BASE_URL: &str = "OPENAI_COMPAT_BASE_URL"; +const CANONICAL_OPENAI_ORIGIN: &str = "https://api.openai.com/v1"; +const MARKER_FILE: &str = ".openai-credentials-v1.migrated"; +const MANIFEST_FILE: &str = ".openai-credentials-v1.transaction.json"; +const BACKUP_SUFFIX: &str = "pre-openai-credentials-v1.bak"; + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +enum Owner { + Global, + Record(usize), + Baked, + Harness(String), + BuiltinDefinition(String), +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Identity { + Official, + Compatible, +} + +type EnvLayer = (Owner, BTreeMap); +type SerializedStore = (Option>, Option>); + +#[derive(Clone, Debug)] +struct ResolvedValue { + value: String, + owner: Owner, +} + +#[derive(Clone, Debug)] +struct Consumer { + label: String, + record_index: Option, + provider: String, + provider_owner: Owner, + identity: Identity, + endpoint: Option, + legacy_key: Option, + official_key: Option, +} + +#[derive(Clone, Debug)] +struct Store { + global: GlobalAgentConfig, + records: Vec, +} + +#[derive(Debug, Serialize, Deserialize)] +struct FileHashes { + source: Option, + target: Option, +} + +#[derive(Debug, Serialize, Deserialize)] +struct TransactionManifest { + global: FileHashes, + managed: FileHashes, +} + +#[derive(Clone)] +struct PlannedStore { + global: Option>, + managed: Option>, + consumers: Vec, +} + +fn hash(bytes: Option<&[u8]>) -> Option { + bytes.map(|bytes| hex::encode(Sha256::digest(bytes))) +} + +fn sibling_path(path: &Path, suffix: &str) -> PathBuf { + let name = path + .file_name() + .map(|n| n.to_string_lossy()) + .unwrap_or_default(); + crate::util::resolved_backup_path(path, &format!("{name}.{suffix}")) +} + +fn read_optional(path: &Path) -> Result>, String> { + if !path.exists() { + return Ok(None); + } + std::fs::read(path) + .map(Some) + .map_err(|e| format!("failed to read {}: {e}", path.display())) +} + +fn parse_store(global: Option<&[u8]>, managed: Option<&[u8]>) -> Result { + let global = match global { + Some(bytes) => serde_json::from_slice(bytes) + .map_err(|e| format!("failed to parse global agent config: {e}"))?, + None => GlobalAgentConfig::default(), + }; + let records = match managed { + Some(bytes) => serde_json::from_slice(bytes) + .map_err(|e| format!("failed to parse managed agent store: {e}"))?, + None => Vec::new(), + }; + Ok(Store { global, records }) +} + +fn definitions(store: &Store) -> Vec { + let mut defs: Vec<_> = store + .records + .iter() + .filter(|r| r.pubkey.is_empty()) + .filter_map(ManagedAgentRecord::to_definition_view) + .collect(); + for id in store.records.iter().filter_map(|r| r.persona_id.as_deref()) { + if !defs.iter().any(|d| d.id == id) { + if let Some(def) = crate::managed_agents::built_in_persona_definition(id, "") { + defs.push(def); + } + } + } + defs +} + +fn selected_definition<'a>( + record: &ManagedAgentRecord, + defs: &'a [AgentDefinition], +) -> Option<&'a AgentDefinition> { + record + .persona_id + .as_deref() + .and_then(|id| defs.iter().find(|d| d.id == id)) +} + +fn runtime_id<'a>( + record: &'a ManagedAgentRecord, + def: Option<&'a AgentDefinition>, +) -> Option<&'a str> { + record + .runtime + .as_deref() + .or_else(|| def.and_then(|d| d.runtime.as_deref())) + .map(str::trim) + .filter(|v| !v.is_empty()) +} + +fn layers( + store: &Store, + record: &ManagedAgentRecord, + def: Option<&AgentDefinition>, +) -> Result, String> { + let mut result = vec![(Owner::Baked, crate::managed_agents::baked_build_env())]; + if let Some(id) = runtime_id(record, def) { + if let Some(harness) = + crate::managed_agents::custom_harnesses::lookup_loaded_harness_by_id(id) + { + result.push((Owner::Harness(id.to_string()), harness.env.clone())); + } else if crate::managed_agents::known_acp_runtime_exact(id).is_none() + && record + .agent_command_override + .as_deref() + .map(str::trim) + .filter(|v| !v.is_empty()) + .is_none() + { + return Err(format!("unresolved custom harness {id:?}")); + } + } + result.push((Owner::Global, store.global.env_vars.clone())); + if let Some(def) = def { + let owner = store + .records + .iter() + .position(|r| r.pubkey.is_empty() && r.slug.as_deref() == Some(def.id.as_str())) + .map(Owner::Record) + .unwrap_or_else(|| Owner::BuiltinDefinition(def.id.clone())); + result.push((owner, def.env_vars.clone())); + } + result.push(( + Owner::Record( + store + .records + .iter() + .position(|candidate| std::ptr::eq(candidate, record)) + .ok_or("consumer record is not in store")?, + ), + record.env_vars.clone(), + )); + Ok(result) +} + +fn resolve_key(layers: &[(Owner, BTreeMap)], key: &str) -> Option { + layers.iter().rev().find_map(|(owner, env)| { + env.get(key).map(|value| ResolvedValue { + value: value.clone(), + owner: owner.clone(), + }) + }) +} + +fn classify_endpoint(endpoint: Option<&str>) -> Result { + let Some(value) = endpoint.map(str::trim).filter(|v| !v.is_empty()) else { + return Ok(Identity::Official); + }; + let url = url::Url::parse(value).map_err(|_| "invalid OpenAI endpoint".to_string())?; + if !matches!(url.scheme(), "http" | "https") + || url.host_str().is_none() + || !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + { + return Err("invalid OpenAI endpoint".to_string()); + } + let official = url.scheme() == "https" + && url + .host_str() + .is_some_and(|h| h.eq_ignore_ascii_case("api.openai.com")) + && url.port_or_known_default() == Some(443) + && matches!(url.path().trim_end_matches('/'), "" | "/v1"); + Ok(if official { + Identity::Official + } else { + Identity::Compatible + }) +} + +fn is_buzz_agent_consumer(record: &ManagedAgentRecord, defs: &[AgentDefinition]) -> bool { + crate::managed_agents::known_acp_runtime(&crate::managed_agents::record_agent_command( + record, defs, + )) + .is_some_and(|runtime| runtime.id == "buzz-agent") +} + +fn global_defaults_target_buzz_agent(store: &Store) -> bool { + match store.global.preferred_runtime.as_deref().map(str::trim) { + Some("") | None => true, + Some(runtime_id) => crate::managed_agents::known_acp_runtime_exact(runtime_id) + .is_some_and(|runtime| runtime.id == "buzz-agent"), + } +} + +fn collect_consumers(store: &Store) -> Result, String> { + let defs = definitions(store); + let mut consumers = Vec::new(); + if global_defaults_target_buzz_agent(store) { + 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() { + if record.pubkey.is_empty() || !is_buzz_agent_consumer(record, &defs) { + continue; + } + let def = selected_definition(record, &defs); + if record.persona_id.is_some() && def.is_none() { + return Err(format!( + "cannot resolve linked definition {:?}", + record.persona_id + )); + } + let effective = resolve_effective_config(record, &defs, &store.global); + let config = match effective { + EffectiveConfigResult::Resolved(config) => config, + EffectiveConfigResult::OrphanedInstance { + missing_persona_id, .. + } => { + return Err(format!( + "cannot resolve linked definition {missing_persona_id:?}" + )) + } + }; + let Some(provider) = config + .provider + .value + .as_deref() + .map(str::trim) + .filter(|v| !v.is_empty()) + else { + continue; + }; + if !matches!( + provider.to_ascii_lowercase().as_str(), + "openai" | "openai-compat" + ) { + continue; + } + let provider_owner = match config.provider.source { + ConfigSource::Global => Owner::Global, + ConfigSource::Definition => { + let id = record + .persona_id + .as_deref() + .or(record.slug.as_deref()) + .ok_or("definition provider has no owner")?; + store + .records + .iter() + .position(|r| r.pubkey.is_empty() && r.slug.as_deref() == Some(id)) + .map(Owner::Record) + .unwrap_or_else(|| Owner::BuiltinDefinition(id.to_string())) + } + ConfigSource::InstanceLegacy => Owner::Record(index), + }; + let resolved_layers = layers(store, record, def)?; + let endpoint = resolve_key(&resolved_layers, OPENAI_COMPAT_BASE_URL); + let identity = classify_endpoint(endpoint.as_ref().map(|v| v.value.as_str()))?; + consumers.push(Consumer { + label: record + .slug + .clone() + .or_else(|| (!record.pubkey.is_empty()).then(|| record.pubkey.clone())) + .unwrap_or_else(|| format!("record-{index}")), + record_index: Some(index), + provider: provider.to_ascii_lowercase(), + provider_owner, + identity, + endpoint, + legacy_key: resolve_key(&resolved_layers, OPENAI_COMPAT_API_KEY), + official_key: resolve_key(&resolved_layers, OPENAI_API_KEY), + }); + } + if consumers.is_empty() + && global_defaults_target_buzz_agent(store) + && [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) +} + +fn env_mut(store: &mut Store, owner: Owner) -> Result<&mut BTreeMap, String> { + match owner { + Owner::Global => Ok(&mut store.global.env_vars), + Owner::Record(i) => Ok(&mut store + .records + .get_mut(i) + .ok_or("record owner is missing")? + .env_vars), + Owner::Baked | Owner::Harness(_) | Owner::BuiltinDefinition(_) => { + Err("required mutation is owned by a read-only harness/build layer".into()) + } + } +} + +fn set_provider(store: &mut Store, owner: Owner, provider: &str) -> Result<(), String> { + match owner { + Owner::Global => { + store.global.provider = Some(provider.into()); + Ok(()) + } + Owner::Record(i) => { + store + .records + .get_mut(i) + .ok_or("record owner is missing")? + .provider = Some(provider.into()); + Ok(()) + } + Owner::Baked | Owner::Harness(_) | Owner::BuiltinDefinition(_) => { + Err("required provider mutation is read-only".into()) + } + } +} + +fn shared_with_non_buzz_consumer(store: &Store, owner: &Owner) -> bool { + if owner != &Owner::Global { + return false; + } + let defs = definitions(store); + store.records.iter().any(|record| { + !record.pubkey.is_empty() + && !is_buzz_agent_consumer(record, &defs) + && resolve_effective_config(record, &defs, &store.global) + .require_resolved() + .is_ok_and(|config| config.provider.source == ConfigSource::Global) + }) +} + +fn plan_store(mut store: Store) -> Result<(Store, Vec), String> { + let consumers = collect_consumers(&store)?; + let mut provider_targets: HashMap, bool)> = HashMap::new(); + for consumer in &consumers { + let expected = if consumer.identity == Identity::Official { + "openai" + } else { + "openai-compat" + }; + match provider_targets.entry(consumer.provider_owner.clone()) { + std::collections::hash_map::Entry::Vacant(entry) => { + entry.insert((Some(consumer.identity), consumer.provider != expected)); + } + std::collections::hash_map::Entry::Occupied(mut entry) => { + if entry.get().0 != Some(consumer.identity) { + entry.get_mut().0 = None; + } else { + entry.get_mut().1 |= consumer.provider != expected; + } + } + } + } + for (owner, (identity, needs_change)) in provider_targets { + if let (Some(identity), true) = (identity, needs_change) { + if !shared_with_non_buzz_consumer(&store, &owner) { + set_provider( + &mut store, + owner, + if identity == Identity::Official { + "openai" + } else { + "openai-compat" + }, + )?; + } + } + } + for consumer in &consumers { + let provider = if consumer.identity == Identity::Official { + "openai" + } else { + "openai-compat" + }; + let owner = consumer + .record_index + .map(Owner::Record) + .unwrap_or(Owner::Global); + env_mut(&mut store, owner)?.insert( + crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV.into(), + provider.into(), + ); + } + + let mut official_legacy_targets = Vec::new(); + for consumer in &consumers { + if consumer.identity != Identity::Official + || consumer + .official_key + .as_ref() + .is_some_and(|v| !v.value.is_empty()) + { + continue; + } + if let Some(legacy) = &consumer.legacy_key { + official_legacy_targets.push(legacy.owner.clone()); + } + } + official_legacy_targets.sort_by_key(|owner| format!("{owner:?}")); + official_legacy_targets.dedup(); + for owner in official_legacy_targets { + let env = env_mut(&mut store, owner)?; + if env.get(OPENAI_API_KEY).is_none_or(|value| value.is_empty()) { + if let Some(value) = env.get(OPENAI_COMPAT_API_KEY).cloned() { + env.insert(OPENAI_API_KEY.into(), value); + } + } + } + + // Official aliases and inherited custom origins must be shadowed with the one + // spelling accepted by both readiness and buzz-agent. Plan this by the + // shared endpoint owner: changing a lower alias is forbidden when it also + // routes a custom/non-OpenAI consumer. + let mut alias_owners = Vec::new(); + for consumer in &consumers { + if consumer.identity != Identity::Official { + continue; + } + if let Some(endpoint) = &consumer.endpoint { + if endpoint.value.trim().trim_end_matches('/') != CANONICAL_OPENAI_ORIGIN { + alias_owners.push(endpoint.owner.clone()); + } + } + } + alias_owners.sort_by_key(|owner| format!("{owner:?}")); + alias_owners.dedup(); + for owner in alias_owners { + env_mut(&mut store, owner)?.insert( + OPENAI_COMPAT_BASE_URL.into(), + CANONICAL_OPENAI_ORIGIN.into(), + ); + } + + // A canonical local endpoint may be the only thing hiding a lower custom + // route. Removing it would make official OpenAI fail closed after restart, + // so leave every already-canonical shadow intact. + Ok((store, consumers)) +} + +fn serialize_store( + store: &Store, + source_global: Option<&[u8]>, + source_managed: Option<&[u8]>, +) -> Result { + let global = if source_global.is_some() || store.global != GlobalAgentConfig::default() { + Some(serde_json::to_vec_pretty(&store.global).map_err(|e| e.to_string())?) + } else { + None + }; + let managed = if source_managed.is_some() || !store.records.is_empty() { + Some(serde_json::to_vec_pretty(&store.records).map_err(|e| e.to_string())?) + } else { + None + }; + Ok((global, managed)) +} + +fn verify_post_split(before: &[Consumer], after: &Store) -> Result<(), String> { + let post = collect_consumers(after)?; + let defs = definitions(after); + for old in before { + let new = post + .iter() + .find(|c| c.record_index == old.record_index) + .ok_or_else(|| format!("consumer {:?} disappeared", old.label))?; + if new.identity != old.identity { + return Err(format!( + "consumer {:?} changed endpoint identity", + old.label + )); + } + let expected_provider = if old.identity == Identity::Official { + "openai" + } else { + "openai-compat" + }; + 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 descriptor = crate::managed_agents::resolve_effective_harness_descriptor( + &after.records[record_index], + &defs, + &after.global, + ) + .map_err(|e| format!("consumer {:?} harness resolution failed: {e}", old.label))?; + let old_selected = if old.provider == "openai" { + old.official_key.as_ref().or(old.legacy_key.as_ref()) + } else { + old.legacy_key.as_ref() + }; + let effective = crate::managed_agents::resolve_effective_agent_env( + &after.records[record_index], + &defs, + crate::managed_agents::known_acp_runtime(&descriptor.command), + &after.global, + ); + let readiness = crate::managed_agents::agent_readiness(&effective); + let readiness_failure = match &readiness { + crate::managed_agents::AgentReadiness::Ready => false, + crate::managed_agents::AgentReadiness::NotReady { requirements } => { + requirements.iter().any(|requirement| match requirement { + crate::managed_agents::Requirement::ConfigInvalid { .. } => true, + crate::managed_agents::Requirement::EnvKey { key } => { + key == if old.identity == Identity::Official { + OPENAI_API_KEY + } else { + OPENAI_COMPAT_BASE_URL + } + } + _ => false, + }) + } + }; + if old_selected.is_some() && readiness_failure { + return Err(format!( + "consumer {:?} fails post-split readiness", + old.label + )); + } + let selected = if old.identity == Identity::Official { + descriptor.env.get(OPENAI_API_KEY) + } else { + descriptor.env.get(OPENAI_COMPAT_API_KEY) + }; + let expected = if old.identity == Identity::Official { + old.official_key + .as_ref() + .filter(|v| !v.value.is_empty()) + .or(old.legacy_key.as_ref()) + .map(|v| &v.value) + } else { + old.legacy_key.as_ref().map(|v| &v.value) + }; + if old_selected.is_some() && selected != expected { + return Err(format!( + "consumer {:?} changed selected credential", + old.label + )); + } + if old.identity == Identity::Official { + let origin = descriptor + .env + .get(OPENAI_COMPAT_BASE_URL) + .map(String::as_str); + if origin.is_some_and(|v| v.trim().trim_end_matches('/') != CANONICAL_OPENAI_ORIGIN) { + return Err(format!( + "consumer {:?} retains rejected official origin", + old.label + )); + } + } else if classify_endpoint( + descriptor + .env + .get(OPENAI_COMPAT_BASE_URL) + .map(String::as_str), + )? != Identity::Compatible + { + return Err(format!("consumer {:?} lost its custom origin", old.label)); + } + } + Ok(()) +} + +fn make_plan(global: Option<&[u8]>, managed: Option<&[u8]>) -> Result { + let source = parse_store(global, managed)?; + let (store, consumers) = plan_store(source)?; + verify_post_split(&consumers, &store)?; + let (global, managed) = serialize_store(&store, global, managed)?; + Ok(PlannedStore { + global, + managed, + consumers, + }) +} + +fn current_hash(path: &Path) -> Result, String> { + Ok(hash(read_optional(path)?.as_deref())) +} + +fn apply_target(path: &Path, target: Option<&[u8]>) -> Result<(), String> { + match target { + Some(bytes) => crate::managed_agents::atomic_write_json_restricted(path, bytes), + None if path.exists() => std::fs::remove_file(path) + .map_err(|e| format!("failed to remove {}: {e}", path.display())), + None => Ok(()), + } +} + +fn verify_written_targets( + expected: &TransactionManifest, + consumers: &[Consumer], + global: Option<&[u8]>, + managed: Option<&[u8]>, +) -> Result<(), String> { + if hash(global) != expected.global.target || hash(managed) != expected.managed.target { + return Err("migration target hash mismatch after write".into()); + } + let reread = parse_store(global, managed)?; + verify_post_split(consumers, &reread) +} + +fn migrate_pair(agents_dir: &Path) -> Result<(), String> { + let marker = agents_dir.join(MARKER_FILE); + if marker.is_file() + && std::fs::read(&marker) + .ok() + .as_deref() + .is_some_and(|bytes| bytes == b"1\n") + { + return Ok(()); + } + let global_path = agents_dir.join("global-agent-config.json"); + let managed_path = agents_dir.join("managed-agents.json"); + let manifest_path = agents_dir.join(MANIFEST_FILE); + + let (source_global, source_managed, manifest) = if manifest_path.exists() { + let manifest: TransactionManifest = + serde_json::from_slice(&std::fs::read(&manifest_path).map_err(|e| e.to_string())?) + .map_err(|e| format!("invalid migration manifest: {e}"))?; + for (path, hashes) in [ + (&global_path, &manifest.global), + (&managed_path, &manifest.managed), + ] { + let current = current_hash(path)?; + if current != hashes.source && current != hashes.target { + return Err(format!( + "external edit detected during OpenAI credential migration: {}", + path.display() + )); + } + } + let source_global = if manifest.global.source.is_some() { + read_optional(&sibling_path(&global_path, BACKUP_SUFFIX))? + } else { + None + }; + let source_managed = if manifest.managed.source.is_some() { + read_optional(&sibling_path(&managed_path, BACKUP_SUFFIX))? + } else { + None + }; + (source_global, source_managed, Some(manifest)) + } else { + ( + read_optional(&global_path)?, + read_optional(&managed_path)?, + None, + ) + }; + if source_global.is_none() && source_managed.is_none() { + return Ok(()); + } + let plan = make_plan(source_global.as_deref(), source_managed.as_deref())?; + let expected = TransactionManifest { + global: FileHashes { + source: hash(source_global.as_deref()), + target: hash(plan.global.as_deref()), + }, + managed: FileHashes { + source: hash(source_managed.as_deref()), + target: hash(plan.managed.as_deref()), + }, + }; + if let Some(existing) = manifest { + if existing.global.source != expected.global.source + || existing.global.target != expected.global.target + || existing.managed.source != expected.managed.source + || existing.managed.target != expected.managed.target + { + return Err("migration manifest does not match pristine backups".into()); + } + } else { + if let Some(bytes) = &source_global { + let backup_path = sibling_path(&global_path, BACKUP_SUFFIX); + crate::util::create_restricted_backup_once(&backup_path, bytes) + .map_err(|e| e.to_string())?; + if read_optional(&backup_path)?.as_deref() != Some(bytes.as_slice()) { + return Err(format!( + "existing migration backup does not match pristine source: {}", + backup_path.display() + )); + } + } + if let Some(bytes) = &source_managed { + let backup_path = sibling_path(&managed_path, BACKUP_SUFFIX); + crate::util::create_restricted_backup_once(&backup_path, bytes) + .map_err(|e| e.to_string())?; + if read_optional(&backup_path)?.as_deref() != Some(bytes.as_slice()) { + return Err(format!( + "existing migration backup does not match pristine source: {}", + backup_path.display() + )); + } + } + let bytes = serde_json::to_vec_pretty(&expected).map_err(|e| e.to_string())?; + crate::managed_agents::atomic_write_json_restricted(&manifest_path, &bytes)?; + } + if current_hash(&global_path)? == expected.global.source { + apply_target(&global_path, plan.global.as_deref())?; + } + if current_hash(&managed_path)? == expected.managed.source { + apply_target(&managed_path, plan.managed.as_deref())?; + } + let reread_global = read_optional(&global_path)?; + let reread_managed = read_optional(&managed_path)?; + verify_written_targets( + &expected, + &plan.consumers, + reread_global.as_deref(), + reread_managed.as_deref(), + )?; + crate::managed_agents::atomic_write_json_restricted(&marker, b"1\n") +} + +pub(super) fn migrate_openai_credentials(app: &tauri::AppHandle) { + let Ok(agents_dir) = crate::managed_agents::managed_agents_base_dir(app) else { + return; + }; + let custom_dir = agents_dir.parent().map(|p| p.join("custom_harnesses")); + crate::managed_agents::custom_harnesses::warm_harness_registry_from_dir(custom_dir.as_deref()); + if let Err(error) = migrate_pair(&agents_dir) { + eprintln!("buzz-desktop: openai-credential-migration: {error}"); + } +} + +#[cfg(test)] +#[path = "openai_credentials_tests.rs"] +mod tests; diff --git a/desktop/src-tauri/src/migration/openai_credentials_tests.rs b/desktop/src-tauri/src/migration/openai_credentials_tests.rs new file mode 100644 index 00000000000..c5f2ce9c64a --- /dev/null +++ b/desktop/src-tauri/src/migration/openai_credentials_tests.rs @@ -0,0 +1,714 @@ +use super::*; + +fn record(pubkey: &str, name: &str) -> ManagedAgentRecord { + serde_json::from_value(serde_json::json!({ + "pubkey": pubkey, + "name": name, + "relay_url": "wss://relay.example", + "acp_command": "buzz-acp", + "agent_command": "buzz-agent", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z" + })) + .unwrap() +} + +fn definition(id: &str) -> ManagedAgentRecord { + let mut value = record("", id); + value.slug = Some(id.to_string()); + value.display_name = Some(id.to_string()); + value.system_prompt = Some(String::new()); + value +} + +fn planned(global: GlobalAgentConfig, records: Vec) -> Result { + plan_store(Store { global, records }).map(|(store, _)| store) +} + +#[test] +fn whole_store_matrix_classifies_origins_and_moves_only_official_credentials() { + for (origin, expected_provider, expected_key) in [ + (None, "openai", OPENAI_API_KEY), + (Some("https://api.openai.com/v1"), "openai", OPENAI_API_KEY), + (Some("https://api.openai.com"), "openai", OPENAI_API_KEY), + ( + Some("http://localhost:11434/v1"), + "openai-compat", + OPENAI_COMPAT_API_KEY, + ), + ] { + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + if let Some(origin) = origin { + agent + .env_vars + .insert(OPENAI_COMPAT_BASE_URL.into(), origin.into()); + } + + let store = planned(GlobalAgentConfig::default(), vec![agent]).unwrap(); + assert_eq!( + store.records[0].provider.as_deref(), + Some(expected_provider) + ); + assert_eq!( + store.records[0] + .env_vars + .get(expected_key) + .map(String::as_str), + Some("secret") + ); + if expected_provider == "openai" { + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("secret"), + "migration copies the shared legacy key during compatibility" + ); + } + if expected_provider == "openai" && origin.is_some() { + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_COMPAT_BASE_URL) + .map(String::as_str), + Some(CANONICAL_OPENAI_ORIGIN) + ); + } + } +} + +#[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"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_BASE_URL.into(), "not a url".into()); + assert!(planned(GlobalAgentConfig::default(), vec![agent]) + .unwrap_err() + .contains("invalid OpenAI endpoint")); +} + +#[test] +fn existing_official_key_wins_without_copying_legacy_secret() { + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_API_KEY.into(), "official".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "compat".into()); + let store = planned(GlobalAgentConfig::default(), vec![agent]).unwrap(); + assert_eq!( + store.records[0].env_vars.get(OPENAI_API_KEY).unwrap(), + "official" + ); + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .unwrap(), + "compat" + ); +} + +#[test] +fn mixed_global_provider_owner_is_resolved_per_consumer() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + ..Default::default() + }; + let mut official = record("official", "official"); + official + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "a".into()); + let mut custom = record("custom", "custom"); + custom + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "b".into()); + custom.env_vars.insert( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ); + let store = planned(global, vec![official, custom]).unwrap(); + assert_eq!(store.global.provider.as_deref(), Some("openai")); + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("a") + ); + assert_eq!( + store.records[1] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("b") + ); +} + +#[test] +fn unrelated_consumer_does_not_claim_global_openai_credential() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + env_vars: BTreeMap::from([(OPENAI_COMPAT_API_KEY.into(), "shared".into())]), + ..Default::default() + }; + let official = record("official", "official"); + let mut unrelated = record("unrelated", "unrelated"); + unrelated.provider = Some("anthropic".into()); + let store = planned(global, vec![official, unrelated]).unwrap(); + assert_eq!( + store + .global + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("shared") + ); +} + +#[test] +fn shared_global_legacy_key_supports_official_and_custom_buzz_agents() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + env_vars: BTreeMap::from([(OPENAI_COMPAT_API_KEY.into(), "shared-key".into())]), + ..Default::default() + }; + let official = record("official", "official"); + let mut custom = record("custom", "custom"); + custom.env_vars.insert( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ); + + let (store, before) = plan_store(Store { + global, + records: vec![official, custom], + }) + .unwrap(); + assert_eq!( + store + .global + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("shared-key") + ); + verify_post_split(&before, &store).unwrap(); + + let defs = definitions(&store); + let custom_descriptor = crate::managed_agents::resolve_effective_harness_descriptor( + &store.records[1], + &defs, + &store.global, + ) + .unwrap(); + assert_eq!( + custom_descriptor + .env + .get("BUZZ_AGENT_PROVIDER") + .map(String::as_str), + Some("openai-compat") + ); + assert!(!custom_descriptor.env.contains_key(OPENAI_API_KEY)); +} + +#[test] +fn global_defaults_migrate_even_when_existing_buzz_agent_uses_anthropic() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + env_vars: BTreeMap::from([(OPENAI_COMPAT_API_KEY.into(), "future-default".into())]), + ..Default::default() + }; + let mut anthropic = record("anthropic", "anthropic"); + anthropic.provider = Some("anthropic".into()); + + let store = planned(global, vec![anthropic]).unwrap(); + assert_eq!(store.global.provider.as_deref(), Some("openai")); + assert_eq!( + store + .global + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("future-default") + ); +} + +#[test] +fn shared_global_provider_is_not_rewritten_under_goose() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + preferred_runtime: Some("buzz-agent".into()), + env_vars: BTreeMap::from([ + (OPENAI_COMPAT_API_KEY.into(), "shared".into()), + ( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ), + ]), + ..Default::default() + }; + let buzz = record("buzz", "buzz"); + let mut goose = record("goose", "goose"); + goose.runtime = Some("goose".into()); + + let store = planned(global, vec![buzz, goose]).unwrap(); + assert_eq!(store.global.provider.as_deref(), Some("openai")); + assert_eq!(store.records[0].provider, None); + assert_eq!( + store.records[0] + .env_vars + .get(crate::managed_agents::openai_env::MIGRATED_OPENAI_PROVIDER_ENV) + .map(String::as_str), + Some("openai-compat") + ); +} + +#[test] +fn goose_records_are_not_openai_migration_consumers() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + preferred_runtime: Some("goose".into()), + env_vars: BTreeMap::from([ + (OPENAI_COMPAT_API_KEY.into(), "goose-owned".into()), + ( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ), + ]), + ..Default::default() + }; + let mut goose = record("goose", "goose"); + goose.runtime = Some("goose".into()); + goose.provider = Some("openai".into()); + + let store = planned(global.clone(), vec![goose.clone()]).unwrap(); + assert_eq!(store.global, global); + assert_eq!(store.records[0].provider, goose.provider); +} + +#[test] +fn linked_provider_snapshot_is_ignored_and_definition_is_mutated() { + let mut def = definition("shared"); + def.provider = Some("openai".into()); + def.env_vars.insert( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ); + let mut instance = record("instance", "instance"); + instance.persona_id = Some("shared".into()); + instance.provider = Some("anthropic".into()); + instance + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "compat".into()); + let store = planned(GlobalAgentConfig::default(), vec![def, instance]).unwrap(); + assert_eq!(store.records[0].provider.as_deref(), Some("openai-compat")); + assert_eq!(store.records[1].provider.as_deref(), Some("anthropic")); +} + +#[test] +fn linked_local_identity_is_resolved_per_consumer() { + let mut def = definition("shared"); + def.provider = Some("openai".into()); + let mut official = record("official", "official"); + official.persona_id = Some("shared".into()); + official + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "a".into()); + let mut custom = record("custom", "custom"); + custom.persona_id = Some("shared".into()); + custom + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "b".into()); + custom.env_vars.insert( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + ); + let store = planned(GlobalAgentConfig::default(), vec![def, official, custom]).unwrap(); + assert_eq!(store.records[0].provider.as_deref(), Some("openai")); + assert_eq!( + store.records[1] + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("a") + ); + assert_eq!( + store.records[2] + .env_vars + .get(OPENAI_COMPAT_API_KEY) + .map(String::as_str), + Some("b") + ); +} + +#[test] +fn first_fold_builtin_link_does_not_block_unrelated_official_consumer() { + let mut builtin = record("builtin", "builtin"); + builtin.persona_id = Some("builtin:fizz".into()); + let mut official = record("official", "official"); + official.provider = Some("openai".into()); + official + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + let store = planned(GlobalAgentConfig::default(), vec![builtin, official]).unwrap(); + assert_eq!( + store.records[1] + .env_vars + .get(OPENAI_API_KEY) + .map(String::as_str), + Some("secret") + ); +} + +#[test] +fn unrelated_consumer_does_not_claim_global_openai_endpoint() { + let global = GlobalAgentConfig { + provider: Some("openai".into()), + env_vars: BTreeMap::from([( + OPENAI_COMPAT_BASE_URL.into(), + "https://api.openai.com".into(), + )]), + ..Default::default() + }; + let official = record("official", "official"); + let mut unrelated = record("unrelated", "unrelated"); + unrelated.provider = Some("anthropic".into()); + let store = planned(global, vec![official, unrelated]).unwrap(); + assert_eq!( + store + .global + .env_vars + .get(OPENAI_COMPAT_BASE_URL) + .map(String::as_str), + Some(CANONICAL_OPENAI_ORIGIN) + ); +} + +#[test] +fn builtin_read_only_provider_is_accepted_when_identity_already_matches() { + let mut builtin = record("builtin", "builtin"); + builtin.persona_id = Some("builtin:fizz".into()); + assert!(planned(GlobalAgentConfig::default(), vec![builtin]).is_ok()); +} + +#[test] +fn canonical_local_shadow_is_preserved_over_inherited_custom_origin() { + let global = GlobalAgentConfig { + env_vars: BTreeMap::from([( + OPENAI_COMPAT_BASE_URL.into(), + "https://gateway.example/v1".into(), + )]), + ..Default::default() + }; + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent.env_vars.insert( + OPENAI_COMPAT_BASE_URL.into(), + CANONICAL_OPENAI_ORIGIN.into(), + ); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + let store = planned(global, vec![agent]).unwrap(); + assert_eq!( + store.records[0] + .env_vars + .get(OPENAI_COMPAT_BASE_URL) + .map(String::as_str), + Some(CANONICAL_OPENAI_ORIGIN) + ); +} + +#[test] +fn dangling_runtime_requires_command_override() { + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent.runtime = Some("missing-custom-runtime".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + assert!(planned(GlobalAgentConfig::default(), vec![agent.clone()]) + .unwrap_err() + .contains("unresolved custom harness")); + agent.agent_command_override = Some("buzz-agent".into()); + assert!(planned(GlobalAgentConfig::default(), vec![agent]).is_ok()); +} + +#[test] +fn semantic_failure_creates_no_transaction_artifacts() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_BASE_URL.into(), "broken".into()); + std::fs::write( + agents.join("managed-agents.json"), + serde_json::to_vec(&vec![agent]).unwrap(), + ) + .unwrap(); + assert!(migrate_pair(agents).is_err()); + assert!(!agents.join(MANIFEST_FILE).exists()); + assert!(!agents.join(MARKER_FILE).exists()); + assert!(!sibling_path(&agents.join("managed-agents.json"), BACKUP_SUFFIX).exists()); +} + +#[test] +fn stale_backup_is_refused_before_manifest_or_config_write() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + let source = serde_json::to_vec(&vec![agent]).unwrap(); + let path = agents.join("managed-agents.json"); + std::fs::write(&path, &source).unwrap(); + std::fs::write(sibling_path(&path, BACKUP_SUFFIX), b"stale").unwrap(); + assert!(migrate_pair(agents) + .unwrap_err() + .contains("backup does not match pristine source")); + assert_eq!(std::fs::read(path).unwrap(), source); + assert!(!agents.join(MANIFEST_FILE).exists()); + assert!(!agents.join(MARKER_FILE).exists()); +} + +#[test] +fn transaction_writes_backups_manifest_targets_then_marker() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let global = GlobalAgentConfig { + provider: Some("openai".into()), + ..Default::default() + }; + let mut agent = record("agent", "agent"); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + std::fs::write( + agents.join("global-agent-config.json"), + serde_json::to_vec(&global).unwrap(), + ) + .unwrap(); + std::fs::write( + agents.join("managed-agents.json"), + serde_json::to_vec(&vec![agent]).unwrap(), + ) + .unwrap(); + migrate_pair(agents).unwrap(); + assert!(agents.join(MANIFEST_FILE).exists()); + assert!(agents.join(MARKER_FILE).exists()); + assert!(sibling_path(&agents.join("global-agent-config.json"), BACKUP_SUFFIX).exists()); + assert!(sibling_path(&agents.join("managed-agents.json"), BACKUP_SUFFIX).exists()); + let reread: Vec = + serde_json::from_slice(&std::fs::read(agents.join("managed-agents.json")).unwrap()) + .unwrap(); + assert_eq!( + reread[0].env_vars.get(OPENAI_API_KEY).map(String::as_str), + Some("secret") + ); +} + +#[test] +fn marker_write_failure_leaves_verified_targets_retryable() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + std::fs::write( + agents.join("managed-agents.json"), + serde_json::to_vec(&vec![agent]).unwrap(), + ) + .unwrap(); + std::fs::create_dir(agents.join(MARKER_FILE)).unwrap(); + + assert!(migrate_pair(agents).is_err()); + assert!(agents.join(MANIFEST_FILE).exists()); + let target: Vec = + serde_json::from_slice(&std::fs::read(agents.join("managed-agents.json")).unwrap()) + .unwrap(); + assert_eq!( + target[0].env_vars.get(OPENAI_API_KEY).map(String::as_str), + Some("secret") + ); + + std::fs::remove_dir(agents.join(MARKER_FILE)).unwrap(); + migrate_pair(agents).unwrap(); + assert_eq!(std::fs::read(agents.join(MARKER_FILE)).unwrap(), b"1\n"); +} + +#[test] +fn malformed_marker_does_not_suppress_retry() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + std::fs::write( + agents.join("managed-agents.json"), + serde_json::to_vec(&vec![agent]).unwrap(), + ) + .unwrap(); + std::fs::write(agents.join(MARKER_FILE), b"").unwrap(); + migrate_pair(agents).unwrap(); + assert_eq!(std::fs::read(agents.join(MARKER_FILE)).unwrap(), b"1\n"); +} + +#[test] +fn post_write_verification_rejects_hash_mismatch() { + let mut agent = record("agent", "agent"); + agent.provider = Some("openai".into()); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + let source = serde_json::to_vec(&vec![agent]).unwrap(); + let plan = make_plan(None, Some(&source)).unwrap(); + let expected = TransactionManifest { + global: FileHashes { + source: None, + target: hash(plan.global.as_deref()), + }, + managed: FileHashes { + source: hash(Some(&source)), + target: hash(plan.managed.as_deref()), + }, + }; + + let err = verify_written_targets(&expected, &plan.consumers, None, Some(b"[]")).unwrap_err(); + assert!(err.contains("target hash mismatch")); +} + +#[test] +fn partial_retry_finishes_expected_target_but_refuses_external_edit() { + let dir = tempfile::tempdir().unwrap(); + let agents = dir.path(); + let global_bytes = serde_json::to_vec(&GlobalAgentConfig { + provider: Some("openai".into()), + ..Default::default() + }) + .unwrap(); + let mut agent = record("agent", "agent"); + agent + .env_vars + .insert(OPENAI_COMPAT_API_KEY.into(), "secret".into()); + let managed_bytes = serde_json::to_vec(&vec![agent]).unwrap(); + let plan = make_plan(Some(&global_bytes), Some(&managed_bytes)).unwrap(); + std::fs::write( + agents.join("global-agent-config.json"), + plan.global.as_ref().unwrap(), + ) + .unwrap(); + std::fs::write(agents.join("managed-agents.json"), &managed_bytes).unwrap(); + std::fs::write( + sibling_path(&agents.join("global-agent-config.json"), BACKUP_SUFFIX), + &global_bytes, + ) + .unwrap(); + std::fs::write( + sibling_path(&agents.join("managed-agents.json"), BACKUP_SUFFIX), + &managed_bytes, + ) + .unwrap(); + let manifest = TransactionManifest { + global: FileHashes { + source: hash(Some(&global_bytes)), + target: hash(plan.global.as_deref()), + }, + managed: FileHashes { + source: hash(Some(&managed_bytes)), + target: hash(plan.managed.as_deref()), + }, + }; + std::fs::write( + agents.join(MANIFEST_FILE), + serde_json::to_vec(&manifest).unwrap(), + ) + .unwrap(); + migrate_pair(agents).unwrap(); + assert!(agents.join(MARKER_FILE).exists()); + + std::fs::remove_file(agents.join(MARKER_FILE)).unwrap(); + std::fs::write(agents.join("managed-agents.json"), b"[]").unwrap(); + assert!(migrate_pair(agents).unwrap_err().contains("external edit")); +} diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index 88f2a3c9821..35ba187a41f 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -105,7 +105,16 @@ with a TypeScript lookup table or an id comparison in a component. `onboarding-agent-defaults.spec.ts`. 9. **The defaults modal is progressively disclosed.** An unset global config starts on the Buzz Agent-first deployment fallback and carries that visible - harness into the next saved edit. The `progressive-defaults` disclosure + harness into the next saved edit. Provider credentials have distinct + identities: official OpenAI uses `OPENAI_API_KEY`, while arbitrary + OpenAI-compatible origins use `OPENAI_COMPAT_API_KEY`. Provider changes preserve the shared env map; provider-specific consumers + select only their own credential and routing values. Official OpenAI pins its + origin and ignores compatible routing state, so changing the default never + destroys credentials or endpoints still used by explicitly configured agents + or card minting. Persisted pre-split credentials are migrated once at + boot: official `api.openai.com` records move to `OPENAI_API_KEY`, while + custom origins remain `openai-compat` and retain `OPENAI_COMPAT_API_KEY`. + The `progressive-defaults` disclosure preset therefore begins at Provider for Buzz Agent, then reveals Model, Effort, and Advanced only after a provider is configured. Harnesses whose runtime metadata has no provider field skip that gate. Reveals animate their diff --git a/desktop/src/features/agents/ui/AgentConfigFields.tsx b/desktop/src/features/agents/ui/AgentConfigFields.tsx index 295c37f23c8..d97d2a7041c 100644 --- a/desktop/src/features/agents/ui/AgentConfigFields.tsx +++ b/desktop/src/features/agents/ui/AgentConfigFields.tsx @@ -37,7 +37,6 @@ import { CARD_MINT_KEY_ANNOTATIONS, CUSTOM_PROVIDER_DROPDOWN_VALUE, getPersonaProviderOptions, - getProviderApiKeyEnvVar, getProviderApiKeyLabel, runtimeSupportsLlmProviderSelection, } from "@/features/agents/ui/agentConfigOptions"; @@ -47,6 +46,11 @@ import { AgentModelField, } from "@/features/agents/ui/agentConfigControls"; import { PersonaProviderApiKeyField } from "@/features/agents/ui/PersonaProviderApiKeyField"; +import { + OPENAI_COMPAT_BASE_URL, + OpenAiCompatibleBaseUrlField, + openAiCompatibleBaseUrlError, +} from "@/features/agents/ui/OpenAiCompatibleBaseUrlField"; import { usePersonaModelDiscovery } from "@/features/agents/ui/usePersonaModelDiscovery"; import { resolveModelLabel } from "@/features/agents/lib/formatAgentModelLabel"; import { @@ -63,6 +67,7 @@ import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup" import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge"; import { CardMintKeyCue } from "./CardMintKeyCue"; import { getGlobalAgentCredentialState } from "./globalAgentCredentialState"; +import { envVarsPreservingProviderState } from "./providerEnvVarUpdates"; export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = { env_vars: {}, @@ -90,23 +95,22 @@ type AgentConfigDisclosure = // onboarding's values won every call and are now the only behavior: // - auto-select a valid model when the provider changes // - keep the model select usable during discovery -// - preserve credential env vars across provider switches (the abandoned -// provider's key stays in env_vars — visible/deletable under Advanced) +// - clear a managed provider credential when its identity changes; unrelated +// env vars remain visible/deletable under Advanced. +// - clear provider-owned endpoint metadata when leaving openai-compat. // - require a provider before model/effort are editable (no saveable // invalid state — design principle #4) const autoSelectModelOnProviderChange = true; const disableModelSelectDuringDiscovery = false; -const preserveCredentialEnvVarsOnProviderChange = true; +const clearManagedCredentialOnProviderChange = true; const requireProviderForModelAndEffort = true; - /** The canonical behavior contract, exported for the contract test. */ export const CANONICAL_CONFIG_BEHAVIORS = { autoSelectModelOnProviderChange, disableModelSelectDuringDiscovery, - preserveCredentialEnvVarsOnProviderChange, + clearManagedCredentialOnProviderChange, requireProviderForModelAndEffort, } as const; - /** Disclosure preset → the eight visibility decisions it owns. Exported for the contract test. */ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { const full = disclosure !== "onboarding-essential"; @@ -121,7 +125,6 @@ export function resolveDisclosure(disclosure: AgentConfigDisclosure) { showUnavailableEffortOptions: full, } as const; } - export function shouldRevealDependentConfigFields({ disclosure, providerFieldVisible, @@ -137,7 +140,6 @@ export function shouldRevealDependentConfigFields({ providerValue.trim().length > 0 ); } - /** Whether the status line under the Model field renders. Discovery warnings bypass onboarding-essential so first-run failures are never invisible. */ export function shouldShowModelStatusMessage( showDescriptions: boolean, @@ -145,7 +147,6 @@ export function shouldShowModelStatusMessage( ): boolean { return showDescriptions || status !== null; } - /** * Renders the Model control given discovery state. Optional-model harnesses omit it while * discovery is loading or after confirmed successful empty; failures keep it for the #2246 UI. @@ -174,7 +175,6 @@ export function shouldRenderModelControl({ // Omit only on confirmed successful empty — not on failure/unavailable. return !modelDiscoverySuccessfulEmpty; } - export type AgentConfigFieldsProps = { bakedEnv: BakedEnvEntry[]; selectedRuntime: AcpRuntimeCatalogEntry | undefined; @@ -210,7 +210,6 @@ export type AgentConfigFieldsProps = { useCustomSelect?: boolean; useChevronSelectIcon?: boolean; }; - export function AgentConfigFields({ bakedEnv, selectedRuntime, @@ -240,7 +239,6 @@ export function AgentConfigFields({ showRequiredIndicators, showUnavailableEffortOptions, } = resolveDisclosure(disclosure); - const fieldModel = React.useMemo( () => deriveAgentConfigFieldModel({ @@ -255,7 +253,6 @@ export function AgentConfigFields({ effortField?.currentPersistence.kind === "envVar" ? effortField.currentPersistence.key : null; - const numericDescriptors = fieldModel.fields.filter( (d): d is NumericDescriptor => (d.kind === "maxOutputTokens" || @@ -307,7 +304,6 @@ export function AgentConfigFields({ ]), [bakedEnv, allStructuredKeys], ); - const providerValue = providerFieldVisible ? (config.provider ?? "") : ""; const providerForDiscovery = providerFieldVisible && !isCustomProvider @@ -343,6 +339,7 @@ export function AgentConfigFields({ apiKeyEnvVar, apiKeyFileSatisfied, apiKeyInherited, + apiKeyRequired, apiKeyValue, credentialsValid, } = getGlobalAgentCredentialState({ @@ -352,12 +349,23 @@ export function AgentConfigFields({ runtimeFileConfig, runtimeId: credentialRuntimeId, }); + const compatibleBaseUrl = config.env_vars[OPENAI_COMPAT_BASE_URL] ?? ""; + const compatibleBaseUrlInherited = + effectiveProvider === "openai-compat" && + compatibleBaseUrl.trim().length === 0 && + credentialsValid; + const compatibleBaseUrlValid = + effectiveProvider !== "openai-compat" || + compatibleBaseUrlInherited || + openAiCompatibleBaseUrlError(compatibleBaseUrl) === null; const configIsValid = - selectedRuntimeId.length > 0 && modelIsValid && credentialsValid; + selectedRuntimeId.length > 0 && + modelIsValid && + credentialsValid && + compatibleBaseUrlValid; React.useEffect(() => { onValidityChange?.(configIsValid); }, [configIsValid, onValidityChange]); - const { discoveredModelOptions, modelDiscoveryLoading, @@ -383,7 +391,6 @@ export function AgentConfigFields({ modelIsOptional, showCustomModelOption, }); - // Mount-time healing policy: onboarding page 4 edits the root config during // first-run (no higher layers to inherit from), so acting on open is safe // and intentional there — it heals stale state and picks a valid model. @@ -401,7 +408,6 @@ export function AgentConfigFields({ const mayMutateDependentFieldsRef = React.useRef(false); mayMutateDependentFieldsRef.current = healOnMount || userEditedProviderRef.current; - const autoSelectedModelScopeRef = React.useRef(null); React.useEffect(() => { if (!autoSelectModelOnProviderChange) return; @@ -415,12 +421,10 @@ export function AgentConfigFields({ if (modelDiscoveryLoading || discoveredModelOptions === null) return; const selectionScope = `${selectedRuntimeId}:${trimmedProvider}`; if (autoSelectedModelScopeRef.current === selectionScope) return; - const firstModel = discoveredModelOptions.find( (option) => option.id.trim().length > 0, ); if (!firstModel) return; - autoSelectedModelScopeRef.current = selectionScope; onCustomModelEditingChange(false); onConfigChange({ ...config, model: firstModel.id }); @@ -434,11 +438,9 @@ export function AgentConfigFields({ providerForDiscovery, selectedRuntimeId, ]); - const currentEffortForAutoClear = effortPersistenceKey ? (config.env_vars[effortPersistenceKey] ?? "") : ""; - // When the selected harness changes outside this component (Back → setup // page → choose a different harness → Next), the saved model can belong to // the old harness. In onboarding, heal that stale value as soon as the new @@ -452,7 +454,6 @@ export function AgentConfigFields({ const currentModel = (config.model ?? "").trim(); if (currentModel.length === 0) return; if (modelDiscoveryLoading) return; - const catalogMiss = discoveredModelOptions !== null && !discoveredModelOptions.some( @@ -461,7 +462,6 @@ export function AgentConfigFields({ const omittedAfterSuccessfulEmpty = modelIsOptional && !modelControlVisible && modelDiscoverySuccessfulEmpty; if (!catalogMiss && !omittedAfterSuccessfulEmpty) return; - const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; onCustomModelEditingChange(false); @@ -478,7 +478,6 @@ export function AgentConfigFields({ healOnMount, effortPersistenceKey, ]); - // Orphan-model clearing follows the mount-time healing policy above: the // backend resolves provider and model independently across layers // (agent → definition → global), so a saved global model WITHOUT a global @@ -496,7 +495,6 @@ export function AgentConfigFields({ ) { return; } - const nextEnvVars = { ...config.env_vars }; if (effortPersistenceKey) delete nextEnvVars[effortPersistenceKey]; onCustomModelEditingChange(false); @@ -522,34 +520,26 @@ export function AgentConfigFields({ onConfigChange({ ...config, env_vars: nextEnvVars }); }, }); - function handleProviderChange(value: string) { userEditedProviderRef.current = true; - const previousApiKey = getProviderApiKeyEnvVar(effectiveProvider); if (value === CUSTOM_PROVIDER_DROPDOWN_VALUE) { - const nextEnvVars = { ...config.env_vars }; - if (!preserveCredentialEnvVarsOnProviderChange && previousApiKey) { - delete nextEnvVars[previousApiKey]; - } + const nextEnvVars = envVarsPreservingProviderState( + config.env_vars, + effectiveProvider, + "", + ); onIsCustomProviderChange(true); onConfigChange({ ...config, env_vars: nextEnvVars, provider: null }); return; } const nextProvider = value === AUTO_PROVIDER_DROPDOWN_VALUE || value === "" ? null : value; - const nextApiKey = getProviderApiKeyEnvVar( + const nextEnvVars = envVarsPreservingProviderState( + config.env_vars, + effectiveProvider, nextProvider ?? bakedProvider ?? "", ); - const nextEnvVars = { ...config.env_vars }; - if ( - !preserveCredentialEnvVarsOnProviderChange && - previousApiKey && - previousApiKey !== nextApiKey - ) { - delete nextEnvVars[previousApiKey]; - } const providerChanged = nextProvider !== (config.provider ?? null); - onIsCustomProviderChange(false); onConfigChange({ ...config, @@ -563,28 +553,23 @@ export function AgentConfigFields({ : config.model, }); } - function handleCustomProviderInput(value: string) { onConfigChange({ ...config, provider: value || null }); } - function handleModelChange(value: string) { onConfigChange({ ...config, model: config.provider === "relay-mesh" ? value || "auto" : value || null, }); } - function handleEnvVarsChange(next: Record) { onConfigChange({ ...config, env_vars: next }); } - const handleNumericEnvVarChange = (key: string, value: string) => { const next = { ...config.env_vars, [key]: value }; if (value === "") delete next[key]; onConfigChange({ ...config, env_vars: next }); }; - // On internal Block builds, BUZZ_AGENT_PROVIDER is baked in and a boot // migration rewrites v1→v2. Hide the legacy v1 option so it is not offered // for new selections; OSS builds show it. @@ -609,7 +594,6 @@ export function AgentConfigFields({ const providerSelectValue = isCustomProvider ? CUSTOM_PROVIDER_DROPDOWN_VALUE : providerValue || AUTO_PROVIDER_DROPDOWN_VALUE; - const providerZeroLabel = React.useMemo(() => { if (!bakedProvider) return null; return getBakedProviderInheritLabel(bakedProvider, providerOptions); @@ -623,7 +607,6 @@ export function AgentConfigFields({ } return "Select a provider"; }, [bakedProvider, providerOptions]); - const implicitEffortProvider = selectedRuntimeId === "claude" ? "anthropic" @@ -639,7 +622,6 @@ export function AgentConfigFields({ ? (config.env_vars[effortPersistenceKey] ?? "") : ""; const effortFieldVisible = showEffortField && effortField !== undefined; - const progressiveDefaults = disclosure === "progressive-defaults"; const fieldClassName = unstyled ? progressiveDefaults @@ -708,7 +690,6 @@ export function AgentConfigFields({ ))} ); - const providerContent = providerFieldVisible ? (