diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index cb809b6c04a..75961952206 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -5,7 +5,6 @@ use serde::Deserialize; 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)] @@ -18,12 +17,12 @@ use super::agent_update_rollback::{rollback_failed_agent_update, AgentUpdateRoll use crate::{ app_state::AppState, managed_agents::{ - current_instance_id, discovery_env_with_baked_floor, find_managed_agent_mut, - known_acp_runtime, load_global_agent_config, load_managed_agents, load_personas, - managed_agent_avatar_url, missing_command_message, normalize_agent_args, resolve_command, - save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo, - AgentModelsResponse, ManagedAgentRecord, UpdateManagedAgentRequest, - UpdateManagedAgentResponse, DEFAULT_ACP_COMMAND, + build_managed_agent_summary, current_instance_id, discovery_env_with_baked_floor, + find_managed_agent_mut, known_acp_runtime, load_global_agent_config, load_managed_agents, + load_personas, managed_agent_avatar_url, missing_command_message, normalize_agent_args, + resolve_command, save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, + AgentModelInfo, AgentModelsResponse, ManagedAgentRecord, UpdateManagedAgentRequest, + validate_provider_value, UpdateManagedAgentResponse, DEFAULT_ACP_COMMAND, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -202,8 +201,10 @@ pub async fn discover_agent_models( state: State<'_, AppState>, ) -> Result { crate::managed_agents::validate_user_env_keys(&input.env_vars)?; - // Also validate definition_env (caller-supplied, same trust level as env_vars). crate::managed_agents::validate_user_env_keys(&input.definition_env)?; + if let Some(provider) = input.provider.as_deref() { + crate::managed_agents::validate_provider_value(provider)?; + } let acp_command = input .acp_command @@ -346,13 +347,11 @@ use openrouter::{ }; fn is_openai_compatible_provider(provider: Option<&str>) -> bool { - matches!( - provider - .map(str::trim) - .map(str::to_ascii_lowercase) - .as_deref(), - Some("openai" | "openai-compat") - ) + let normalized = provider.map(str::trim).map(str::to_ascii_lowercase); + matches!(normalized.as_deref(), Some("openai" | "openai-compat")) + || normalized + .as_deref() + .is_some_and(crate::managed_agents::provider_is_http_base_url) } #[cfg(test)] diff --git a/desktop/src-tauri/src/commands/agent_models_discovery_config.rs b/desktop/src-tauri/src/commands/agent_models_discovery_config.rs index e43f09495bf..d2deac07190 100644 --- a/desktop/src-tauri/src/commands/agent_models_discovery_config.rs +++ b/desktop/src-tauri/src/commands/agent_models_discovery_config.rs @@ -89,8 +89,9 @@ pub(super) fn draft_agent_model_discovery_env( definition_env: &BTreeMap, env_vars: &BTreeMap, ) -> BTreeMap { + let runtime = known_acp_runtime(agent_command); let mut derived_env = BTreeMap::new(); - if let Some(meta) = known_acp_runtime(agent_command) { + if let Some(meta) = runtime { let provider = provider.map(str::trim).filter(|value| !value.is_empty()); if !meta.provider_locked { if let (Some(env_key), Some(provider)) = (meta.provider_env_var, provider) { @@ -98,15 +99,20 @@ pub(super) fn draft_agent_model_discovery_env( } } } - // Reserved keys are stripped from definition env, matching the same filter - // applied at spawn. - let mut filtered_definition_env = BTreeMap::new(); - for (key, value) in definition_env { - if !crate::managed_agents::is_reserved_env_key(key) { - filtered_definition_env.insert(key.clone(), value.clone()); - } + + let filtered_definition_env = + crate::managed_agents::merged_user_env(&BTreeMap::new(), definition_env); + let filtered_user_env = crate::managed_agents::merged_user_env(&BTreeMap::new(), env_vars); + if let Some(runtime) = runtime { + crate::managed_agents::merge_runtime_provider_env_layers( + runtime, + provider, + &mut derived_env, + [filtered_definition_env, filtered_user_env], + ); + } else { + derived_env.extend(filtered_definition_env); + derived_env.extend(filtered_user_env); } - let merged_with_def = - crate::managed_agents::merged_user_env(&derived_env, &filtered_definition_env); - crate::managed_agents::merged_user_env(&merged_with_def, env_vars) + derived_env } diff --git a/desktop/src-tauri/src/commands/agent_models_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs index df3849de4a4..f1f4168267b 100644 --- a/desktop/src-tauri/src/commands/agent_models_tests.rs +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -267,6 +267,102 @@ fn saved_agent_model_discovery_uses_record_snapshot_for_definition_less_agent() assert_eq!(discovery.provider_env_var, Some("GOOSE_PROVIDER")); } +#[test] +fn saved_agent_model_discovery_uses_descriptor_provider_mapping() { + let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "mapped1234", + "name": "mapped-agent", + "private_key_nsec": "", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "goose", + "runtime": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 1, + "system_prompt": null, + "model": "provider-model", + "provider": "openai-compat", + "env_vars": { + "OPENAI_COMPAT_API_KEY": "test-key", + "OPENAI_COMPAT_BASE_URL": "https://provider.example/v1" + }, + "created_at": "", + "updated_at": "", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("sample managed agent record"); + + let discovery = agent_model_discovery_config(&record, &[], &Default::default()) + .expect("saved discovery config"); + + assert_eq!(discovery.provider.as_deref(), Some("openai-compat")); + assert_eq!( + discovery.env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + assert_eq!( + discovery.env.get("OPENAI_HOST").map(String::as_str), + Some("https://provider.example/v1") + ); + assert_eq!( + discovery + .env + .get("GOOSE_PROVIDER__HOST") + .map(String::as_str), + Some("https://provider.example/v1") + ); +} + +#[test] +fn draft_agent_model_discovery_applies_aliases_with_scope_precedence() { + let definition_env = BTreeMap::from([ + ("OPENAI_API_KEY".to_string(), "definition-key".to_string()), + ( + "OPENAI_HOST".to_string(), + "https://definition.example/v1".to_string(), + ), + ]); + let user_env = BTreeMap::from([ + ("OPENAI_COMPAT_API_KEY".to_string(), "user-key".to_string()), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "https://user.example/v1".to_string(), + ), + ("GOOSE_PROVIDER".to_string(), "anthropic".to_string()), + ]); + + let env = + draft_agent_model_discovery_env("goose", Some("openai-compat"), &definition_env, &user_env); + + assert_eq!( + env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("user-key") + ); + assert_eq!( + env.get("GOOSE_PROVIDER__API_KEY").map(String::as_str), + Some("user-key") + ); + assert_eq!( + env.get("OPENAI_HOST").map(String::as_str), + Some("https://user.example/v1") + ); + assert_eq!( + env.get("GOOSE_PROVIDER__HOST").map(String::as_str), + Some("https://user.example/v1") + ); +} + // --------------------------------------------------------------------------- // Provider resolution for discovery // --------------------------------------------------------------------------- @@ -555,7 +651,7 @@ fn linked_instance_ignores_model_provider_prompt_writes() { Some(Some("explicit-prov".to_string())), Some(Some("explicit-prompt".to_string())), ) - .unwrap(); + .expect("linked provider write is ignored"); assert!( record.model.is_none(), @@ -607,7 +703,7 @@ fn definition_less_instance_accepts_model_provider_prompt_writes() { Some(Some("new-prov".to_string())), Some(Some("new-prompt".to_string())), ) - .unwrap(); + .expect("standalone provider write is valid"); assert_eq!(record.model.as_deref(), Some("new-model")); assert_eq!(record.provider.as_deref(), Some("new-prov")); @@ -624,12 +720,19 @@ fn is_databricks_provider_matches_both_variants() { } #[test] -fn databricks_interactive_auth_launches_only_without_a_static_token() { - // Phase 2: both surfaces launch the browser flow when the token is empty; - // the surface distinction is now cooldown-only (asserted separately). A - // configured static token still short-circuits interactive auth entirely. - assert!(should_start_interactive_auth("")); - assert!(!should_start_interactive_auth("static-token")); +fn databricks_interactive_auth_requires_explicit_intent_and_no_static_token() { + assert!(should_start_interactive_auth( + "", + DatabricksAuthIntent::InteractiveModelPicker + )); + assert!(!should_start_interactive_auth( + "", + DatabricksAuthIntent::PassiveDraftDiscovery + )); + assert!(!should_start_interactive_auth( + "static-token", + DatabricksAuthIntent::InteractiveModelPicker + )); } #[test] diff --git a/desktop/src-tauri/src/commands/agents.rs b/desktop/src-tauri/src/commands/agents.rs index 33b6ae44620..5506b989475 100644 --- a/desktop/src-tauri/src/commands/agents.rs +++ b/desktop/src-tauri/src/commands/agents.rs @@ -6,12 +6,13 @@ use super::managed_agent_definition::validate_create_definition; use crate::{ app_state::AppState, managed_agents::{ - build_managed_agent_summary, current_instance_id, ensure_persona_is_active, - find_managed_agent_mut, load_managed_agents, load_personas, load_teams, - managed_agent_avatar_url, normalize_agent_args, resolve_provider_binary, - save_managed_agents, start_managed_agent_process, stop_managed_agent_process, - stop_managed_agent_workspace_pair, sync_managed_agent_processes, try_regenerate_nest, - validate_provider_config, BackendKind, CreateManagedAgentRequest, + build_managed_agent_summary, current_instance_id, discover_provider_candidates, + ensure_persona_is_active, find_managed_agent_mut, load_managed_agents, load_personas, + load_teams, managed_agent_avatar_url, normalize_agent_args, provider_deploy, + resolve_provider_binary, save_managed_agents, start_managed_agent_process, + stop_managed_agent_process, stop_managed_agent_workspace_pair, + sync_managed_agent_processes, try_regenerate_nest, validate_provider_config, + validate_provider_value, BackendKind, CreateManagedAgentRequest, CreateManagedAgentResponse, ManagedAgentRecord, ManagedAgentSummary, RelayMeshConfig, DEFAULT_ACP_COMMAND, DEFAULT_AGENT_PARALLELISM, DEFAULT_AGENT_TURN_TIMEOUT_SECONDS, }, @@ -621,6 +622,7 @@ pub async fn create_managed_agent( let snapshot_source_version = persona_snapshot.as_ref().map(|s| s.source_version.clone()); let effective_provider = snapshot_provider .or_else(|| input.provider.as_deref().and_then(trim_to_optional_string)); + validate_provider_value(effective_provider.as_deref().unwrap_or_default())?; let mut effective_model = snapshot_model.or_else(|| input.model.as_deref().and_then(trim_to_optional_string)); if effective_provider.as_deref() == Some(crate::managed_agents::RELAY_MESH_PROVIDER_ID) @@ -628,7 +630,6 @@ pub async fn create_managed_agent( { effective_model = Some(crate::managed_agents::RELAY_MESH_AUTO_MODEL_ID.to_string()); } - // Mint-time behavioral quad: explicit input wins, then the linked // definition's NIP-AP defaults, then client defaults. The ONLY parse // point for definition behavioral strings — fails loudly on a bad diff --git a/desktop/src-tauri/src/commands/managed_agent_definition.rs b/desktop/src-tauri/src/commands/managed_agent_definition.rs index 32753807486..98169f562e3 100644 --- a/desktop/src-tauri/src/commands/managed_agent_definition.rs +++ b/desktop/src-tauri/src/commands/managed_agent_definition.rs @@ -32,6 +32,9 @@ pub(super) fn apply_model_provider_prompt_update( record.model = model_update; } if let Some(provider_update) = provider { + crate::managed_agents::validate_provider_value( + provider_update.as_deref().unwrap_or_default(), + )?; record.provider = provider_update; } if let Some(prompt_update) = system_prompt { diff --git a/desktop/src-tauri/src/commands/personas/create.rs b/desktop/src-tauri/src/commands/personas/create.rs index 944013029b8..aa263368586 100644 --- a/desktop/src-tauri/src/commands/personas/create.rs +++ b/desktop/src-tauri/src/commands/personas/create.rs @@ -33,6 +33,9 @@ pub async fn create_persona( let runtime = trim_optional(input.runtime); let model = trim_optional(input.model); let provider = trim_optional(input.provider); + if let Some(provider) = provider.as_deref() { + crate::managed_agents::validate_provider_value(provider)?; + } // Normalized before the store is touched: a coordinate that can't match // a publication is worse than no coordinate, because it silently // re-enables the duplicate add it exists to prevent. diff --git a/desktop/src-tauri/src/commands/personas/update.rs b/desktop/src-tauri/src/commands/personas/update.rs index b3830e62b52..fa515f3a45a 100644 --- a/desktop/src-tauri/src/commands/personas/update.rs +++ b/desktop/src-tauri/src/commands/personas/update.rs @@ -96,6 +96,9 @@ pub(super) async fn update_persona_with( let runtime = trim_optional(input.runtime); let model = trim_optional(input.model); let provider = trim_optional(input.provider); + if let Some(provider) = provider.as_deref() { + crate::managed_agents::validate_provider_value(provider)?; + } let _store_guard = state .managed_agents_store_lock diff --git a/desktop/src-tauri/src/huddle/playout.rs b/desktop/src-tauri/src/huddle/playout.rs index 5bfce3adad5..4371d8a1313 100644 --- a/desktop/src-tauri/src/huddle/playout.rs +++ b/desktop/src-tauri/src/huddle/playout.rs @@ -33,7 +33,7 @@ use tokio_util::sync::CancellationToken; use super::human_floor::HumanFloor; use super::jitter::{PeerJitterBuffer, SAMPLE_RATE_HZ}; use super::relay_api::{WsStream, REMOTE_SPEECH_THRESHOLD}; -use super::wire::{parse_relay_frame, FLAG_DTX}; +use super::wire::{FrameHeader, FLAG_DTX, V2_HEADER_LEN}; /// Speaker-tick window for emitting `huddle-active-speakers`. Active set is /// cleared each tick — peers that didn't send a frame in the last window are @@ -149,11 +149,18 @@ fn is_agent_peer( }) } -/// Whether `peer_idx` is currently occupied per the authoritative roster. -/// Protocol v2 media carries only the peer index, so roster presence is the -/// strongest routing boundary available until the relay supports v3 epochs. -fn is_current_occupant(peer_idx: u8, index_to_epoch: &std::collections::HashMap) -> bool { - index_to_epoch.contains_key(&peer_idx) +/// Whether `peer_idx` is currently occupied at exactly `epoch`, per the +/// authoritative roster. A frame is deliverable only when both match: an index +/// absent from the roster is stale, and a slot reused by a later occupant has +/// advanced its epoch, so a departed occupant's in-flight frame is fenced +/// rather than mis-attributed to the new occupant. A legacy relay omits the +/// epoch, which degrades to `0` on both sides, making the fence a no-op. +fn is_current_occupant( + peer_idx: u8, + epoch: u8, + index_to_epoch: &std::collections::HashMap, +) -> bool { + index_to_epoch.get(&peer_idx) == Some(&epoch) } fn same_occupancy( @@ -189,7 +196,7 @@ fn f32_samples_to_le_bytes(samples: &[f32]) -> Vec { /// One remote peer's slot: jitter buffer + dedicated rodio Player. /// /// Per-frame seq/timestamp come from the v2 wire header (sender-authored). -/// The relay forwards `peer_index | header | opus_bytes` opaquely; we +/// The relay forwards `peer_index | epoch | header | opus_bytes` opaquely; we /// parse the header here and pass the sender's own monotonic seq + 48 kHz media /// timestamp into NetEq. struct PeerSlot { @@ -415,19 +422,22 @@ pub(crate) async fn run_playout_recv_loop( msg = ws_rx.next() => { match msg { Some(Ok(WsMsg::Binary(data))) => { - // Wire shape (v2): [peer_index: u8][header: 8 bytes][opus payload...] - // The minimum size is 1 (peer index) + 8 (header) + ≥1 Opus byte. - let Some((peer_idx, header, opus_bytes)) = parse_relay_frame(&data) else { - eprintln!( - "buzz-desktop: dropping malformed v2 audio relay frame ({} bytes)", - data.len(), - ); + // Wire shape (v2): [peer_index: u8][epoch: u8][header: 8 bytes][opus payload...] + // The minimum size is 2 (peer_index + epoch) + 8 (header) + ≥1 Opus byte. + if data.len() <= 2 + V2_HEADER_LEN { continue; - }; - // Protocol v2 has no media epoch. Drop frames for slots - // absent from the control roster; delayed frames after - // an index is reassigned cannot be fenced until v3. - if !is_current_occupant(peer_idx, &index_to_epoch) { + } + let peer_idx = data[0]; + let epoch = data[1]; + // Fence the peer-index reuse race: a frame authored by a + // departed occupant that arrives after its index is + // reassigned carries the old epoch. Drop it rather than + // mis-attribute stale audio (and the new occupant's + // human/agent STT policy) to whoever grabbed the index. + // An index absent from the roster is also stale. A slot + // with no known epoch (legacy relay) degrades to 0 on + // both sides, so the fence is a no-op there. + if !is_current_occupant(peer_idx, epoch, &index_to_epoch) { continue; } // Suppress only an agent stream synthesized and @@ -436,6 +446,21 @@ pub(crate) async fn run_playout_recv_loop( if is_locally_synthesized_peer(peer_idx, &local_tts_publishers) { continue; } + let after_idx = &data[2..]; + let Some((header, opus_bytes)) = FrameHeader::parse(after_idx) + else { + // Malformed v2 frame: header parse only fails when + // the slice is too short, which `if data.len() <= ...` + // already guards. Defensive log + drop. + eprintln!( + "buzz-desktop: dropping malformed audio frame from peer {peer_idx} ({} bytes)", + data.len(), + ); + continue; + }; + if opus_bytes.is_empty() { + continue; + } let is_dtx = (header.flags & FLAG_DTX) != 0; // Only count non-DTX arrivals toward the UI's // active-speaker set. DTX/comfort packets are emitted @@ -746,16 +771,34 @@ mod tests { ); } + /// Causal regression for the peer-index reuse race (Jude's blocking + /// finding): a frame authored by a departed occupant that arrives after + /// its slot is reassigned to a new occupant carries the stale epoch and + /// must be fenced, never mis-attributed to the new occupant. #[test] - fn v2_media_is_routed_only_for_current_roster_indices() { + fn stale_epoch_frame_is_fenced_after_its_index_is_reused() { let mut index_to_epoch = std::collections::HashMap::new(); + // Slot 3 first occupied at epoch 0. index_to_epoch.insert(3_u8, 0_u8); assert!( - is_current_occupant(3, &index_to_epoch), + is_current_occupant(3, 0, &index_to_epoch), "current occupant's frame is delivered" ); + + // The occupant departs and a new peer reuses slot 3 at epoch 1. + index_to_epoch.insert(3, 1); + assert!( + !is_current_occupant(3, 0, &index_to_epoch), + "in-flight frame from the departed occupant (epoch 0) is fenced" + ); + assert!( + is_current_occupant(3, 1, &index_to_epoch), + "the new occupant's frame (epoch 1) is delivered" + ); + + // A frame for an index absent from the roster is stale. assert!( - !is_current_occupant(9, &index_to_epoch), + !is_current_occupant(9, 0, &index_to_epoch), "frame for an unoccupied index is dropped" ); } diff --git a/desktop/src-tauri/src/huddle/relay_api.rs b/desktop/src-tauri/src/huddle/relay_api.rs index 190397aa054..20a2be57652 100644 --- a/desktop/src-tauri/src/huddle/relay_api.rs +++ b/desktop/src-tauri/src/huddle/relay_api.rs @@ -114,9 +114,6 @@ async fn connect_authenticated_audio_socket( "type": "auth", "event": event_json, "parent_channel_id": parent_channel_id, - // Use the released v2 contract while deployed relays remain capped at - // v2. Relay-to-client media therefore has a one-byte peer-index prefix; - // see huddle::wire for the compatibility tradeoff. "protocol_version": super::wire::PROTOCOL_VERSION, }); ws_tx diff --git a/desktop/src-tauri/src/huddle/wire.rs b/desktop/src-tauri/src/huddle/wire.rs index bcf9c007c2d..518377a60b0 100644 --- a/desktop/src-tauri/src/huddle/wire.rs +++ b/desktop/src-tauri/src/huddle/wire.rs @@ -7,18 +7,25 @@ //! //! No per-frame metadata; receiver synthesizes sequence/timestamp on arrival. //! Kept for backward compatibility — relay still admits v1 clients into -//! v1-pinned rooms — but new clients speak v2 while deployed relays remain -//! capped at the released v2 contract. +//! v1-pinned rooms — but new clients always speak v3. //! -//! ## v2 (compatibility contract) +//! ## v2 (released) //! //! Client → relay: `` //! Relay → client: `` //! -//! Protocol v2 does not carry v3's occupancy epoch in media frames. The -//! control-plane roster still resets decoder and playout state when an index is -//! reassigned, but v2 cannot fence a delayed packet from the previous occupant -//! after that reassignment. +//! ## v3 (this commit) +//! +//! Client → relay: `` +//! Relay → client: `` +//! +//! The relay prefixes each forwarded frame with the sender's stable +//! `peer_index` and the current occupancy `epoch` of that index. The epoch +//! advances each time a slot is reused by a new occupant, so a client can +//! fence a frame authored by a departed occupant that arrives after its index +//! is reassigned — it carries the stale epoch and is dropped rather than +//! mis-attributed. The client's own send path is unaffected: it emits only +//! `
` and the relay stamps the prefix. //! //! Header layout (8 bytes, network byte order, big-endian): //! @@ -37,13 +44,13 @@ //! * `level_dbov` is client-authored telemetry. The relay parses it for //! logging/active-speaker hints, clamps invalid values into range, and //! **never** uses it for trust decisions (admission, moderation, etc.). -//! * Negotiation lives in the WS auth message (`protocol_version: 2`), not +//! * Negotiation lives in the WS auth message (`protocol_version: 3`), not //! in any bit of `flags`. Mixed-version rooms are rejected at the relay //! with `upgrade_required`. /// Wire protocol version this client speaks. Bumped only when the frame /// layout itself changes; the relay tracks pinned per-room. -pub const PROTOCOL_VERSION: u8 = 2; +pub const PROTOCOL_VERSION: u8 = 3; /// Length of the v2 per-frame header in bytes. pub const V2_HEADER_LEN: usize = 8; @@ -128,19 +135,6 @@ impl FrameHeader { } } -/// Parse a complete relay-to-client v2 frame. -/// -/// The released v2 contract has exactly one relay-authored prefix byte: the -/// sender's peer index. A non-empty Opus payload must follow the fixed header. -pub fn parse_relay_frame(bytes: &[u8]) -> Option<(u8, FrameHeader, &[u8])> { - let (&peer_index, framed_audio) = bytes.split_first()?; - let (header, opus_payload) = FrameHeader::parse(framed_audio)?; - if opus_payload.is_empty() { - return None; - } - Some((peer_index, header, opus_payload)) -} - /// Compute a dBov audio level for a normalized f32 PCM frame. /// /// "dBov" is RMS expressed in dB relative to full scale (where full scale = @@ -224,41 +218,6 @@ mod tests { assert_eq!(tail, b"opus-bytes"); } - #[test] - fn relay_frame_uses_the_v2_one_byte_peer_prefix() { - let header = FrameHeader { - seq: 0x0102, - ts_48k: 960, - level_dbov: -20, - flags: 0, - }; - let mut frame = vec![7]; - frame.extend_from_slice(&header.encode()); - frame.extend_from_slice(b"opus"); - - let (peer_index, parsed_header, opus_payload) = - parse_relay_frame(&frame).expect("valid v2 relay frame"); - assert_eq!(peer_index, 7); - assert_eq!(parsed_header, header); - assert_eq!(opus_payload, b"opus"); - } - - #[test] - fn relay_frame_rejects_a_missing_opus_payload() { - let mut frame = vec![7]; - frame.extend_from_slice( - &FrameHeader { - seq: 1, - ts_48k: 960, - level_dbov: -20, - flags: 0, - } - .encode(), - ); - - assert!(parse_relay_frame(&frame).is_none()); - } - /// Bytes in big-endian network order, matching Max's spec. This pins /// the byte layout against accidental endianness changes. #[test] diff --git a/desktop/src-tauri/src/managed_agents/agent_events.rs b/desktop/src-tauri/src/managed_agents/agent_events.rs index f0a4fabfed8..36702143aaf 100644 --- a/desktop/src-tauri/src/managed_agents/agent_events.rs +++ b/desktop/src-tauri/src/managed_agents/agent_events.rs @@ -117,7 +117,12 @@ pub fn build_agent_event(record: &ManagedAgentRecord) -> Result) -> Result<(), "total env var payload is {total} bytes; limit is {MAX_ENV_TOTAL_BYTES}" )); } + super::validate_provider_env_urls(env_vars)?; Ok(()) } diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index f3de11ad242..53b66de4681 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -206,6 +206,32 @@ fn validate_keys_accepts_normal_env() { assert!(validate_user_env_keys(&env).is_ok()); } +#[test] +fn validate_keys_rejects_credential_bearing_openai_compatible_base_url() { + let env = map(&[( + "OPENAI_COMPAT_BASE_URL", + "https://user:secret@example.com/v1", + )]); + let err = validate_user_env_keys(&env).expect_err("unsafe base URL must be rejected"); + assert!(err.contains("cannot include credentials"), "got: {err}"); + assert!( + !err.contains("secret"), + "error must not echo URL values: {err}" + ); +} + +#[test] +fn validate_keys_leaves_runtime_native_provider_urls_to_custom_harnesses() { + let env = map(&[ + ("GOOSE_PROVIDER__HOST", "localhost:11434"), + ( + "OPENAI_BASE_URL", + "https://example.com/v1?api-version=2025-01-01", + ), + ]); + assert!(validate_user_env_keys(&env).is_ok()); +} + #[test] fn validate_keys_rejects_reserved() { let env = map(&[("BUZZ_PRIVATE_KEY", "nsec1evil")]); diff --git a/desktop/src-tauri/src/managed_agents/global_config/mod.rs b/desktop/src-tauri/src/managed_agents/global_config/mod.rs index 162f447981b..2b4a6c330b9 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/mod.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/mod.rs @@ -139,6 +139,9 @@ pub fn validate_global_config(config: &GlobalAgentConfig) -> Result<(), String> // normalize_global_config_fields, called from save_global_agent_config. } } + if let Some(provider) = config.provider.as_deref() { + crate::managed_agents::validate_provider_value(provider)?; + } Ok(()) } diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 65cde47f26b..db0ce1518f2 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -153,6 +153,19 @@ fn validate_accepts_valid_provider_and_model() { assert!(validate_global_config(&config).is_ok()); } +#[test] +fn validate_rejects_provider_url_with_publishable_credentials() { + let config = GlobalAgentConfig { + provider: Some("https://user:secret@example.com/v1".to_string()), + ..Default::default() + }; + + let error = validate_global_config(&config).expect_err("unsafe provider URL must be rejected"); + + assert!(error.contains("cannot include credentials")); + assert!(!error.contains("secret")); +} + // ── normalize_global_config_fields ─────────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/mod.rs b/desktop/src-tauri/src/managed_agents/mod.rs index 272c03348b9..8e78101015a 100644 --- a/desktop/src-tauri/src/managed_agents/mod.rs +++ b/desktop/src-tauri/src/managed_agents/mod.rs @@ -82,6 +82,10 @@ pub use repos::{ write_persisted_repos_dir, }; pub use restore::*; +pub(crate) use runtime::provider_env::{ + merge_runtime_provider_env_layers, provider_is_http_base_url, validate_provider_env_urls, + validate_provider_value, +}; pub use runtime::*; pub use runtime_commands::*; pub use runtime_types::*; diff --git a/desktop/src-tauri/src/managed_agents/persona_events.rs b/desktop/src-tauri/src/managed_agents/persona_events.rs index 7a3ce35b036..e5eb229c570 100644 --- a/desktop/src-tauri/src/managed_agents/persona_events.rs +++ b/desktop/src-tauri/src/managed_agents/persona_events.rs @@ -130,6 +130,10 @@ pub fn monotonic_created_at(prior_head_created_at: Option) -> nostr::Timest /// /// Returns an unsigned `EventBuilder` — the caller signs and submits. pub fn build_persona_event(record: &AgentDefinition) -> Result { + if let Some(provider) = record.provider.as_deref() { + crate::managed_agents::validate_provider_value(provider)?; + } + // Single projection point — persona_event_content owns the field mapping // (and the hash-stability rules that come with it). let content = persona_event_content(record); diff --git a/desktop/src-tauri/src/managed_agents/readiness.rs b/desktop/src-tauri/src/managed_agents/readiness.rs index f7f5d5c5d0e..fbb0d6ab93a 100644 --- a/desktop/src-tauri/src/managed_agents/readiness.rs +++ b/desktop/src-tauri/src/managed_agents/readiness.rs @@ -241,33 +241,29 @@ fn resolve_effective_agent_env_with_def( } } - // Layer 2b: definition env — the harness author's defaults (e.g. CURSOR_ACP=1). - // Applied as a floor below global so user env always wins on collision. - // Reserved keys are stripped by the shared `is_reserved_env_key` predicate. - if let Some(ref def) = harness_def { - for (key, value) in &def.env { - if !super::env_vars::is_reserved_env_key(key) { - env.insert(key.clone(), value.clone()); - } - } - } - - // Layer 3a: global env vars — the lowest user-settable layer. - // Injected before persona/agent so per-agent values win on collision. - // `merged_user_env` with an empty "lower" map applies reserved/malformed-key - // filtering to the global map for free. + // Keep layers separate until runtime aliases are mirrored. + let definition_env = harness_def + .map(|def| merged_user_env(&BTreeMap::new(), &def.env)) + .unwrap_or_default(); let global_env = merged_user_env(&BTreeMap::new(), &global.env_vars); - env.extend(global_env); - - // Layer 3b: merged user env — live persona env under the record's own - // overrides (last-wins), after reserved/malformed-key filtering. Reading - // the persona live is what makes persona credential edits refresh on the - // next spawn instead of being frozen into the record. - let user_env = merged_user_env( + let persona_env = merged_user_env( + &BTreeMap::new(), &super::env_vars::live_persona_env(personas, record.persona_id.as_deref()), - &record.env_vars, ); - env.extend(user_env); + let agent_env = merged_user_env(&BTreeMap::new(), &record.env_vars); + + if let Some(rt) = runtime { + super::merge_runtime_provider_env_layers( + rt, + effective_provider.as_deref(), + &mut env, + [definition_env, global_env, persona_env, agent_env], + ); + } else { + [definition_env, global_env, persona_env, agent_env] + .into_iter() + .for_each(|layer| env.extend(layer)); + } // Buzz shared compute is a native Buzz provider. Translate it to buzz-agent's // OpenAI-compatible transport only in the effective runtime environment. diff --git a/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs b/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs index 46d0e4c7a75..ca9468b5b66 100644 --- a/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs +++ b/desktop/src-tauri/src/managed_agents/readiness_goose_file_config_tests.rs @@ -1,4 +1,4 @@ -//! Goose file-config-aware requirement tests. +//! Goose file-config and provider-environment readiness tests. //! //! These tests call `goose_requirements` directly, injecting a synthetic //! `RuntimeFileConfig` so there is no disk I/O and tests are deterministic. @@ -188,3 +188,117 @@ fn goose_goose_provider_databricks_flat_host_silences_databricks_host() { "DATABRICKS_HOST must be silenced when canonical key is in file extra" ); } + +#[test] +fn descriptor_maps_authoritative_openai_compatible_config_for_spawn_and_readiness() { + let record: crate::managed_agents::types::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "test-pubkey", + "name": "test-agent", + "private_key_nsec": "", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "goose", + "runtime": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 1, + "system_prompt": null, + "model": "provider-model", + "provider": "openai-compat", + "env_vars": { + "GOOSE_PROVIDER": "anthropic", + "OPENAI_COMPAT_API_KEY": "test-key", + "OPENAI_COMPAT_BASE_URL": "https://provider.example/v1" + }, + "created_at": "", + "updated_at": "", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("sample managed agent record"); + + let descriptor = resolve_effective_harness_descriptor(&record, &[], &Default::default()) + .expect("descriptor"); + + assert_eq!( + descriptor.env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai"), + "structured effective provider must beat arbitrary layered GOOSE_PROVIDER" + ); + assert_eq!( + descriptor.env.get("OPENAI_API_KEY").map(String::as_str), + Some("test-key") + ); + assert_eq!( + descriptor.env.get("OPENAI_HOST").map(String::as_str), + Some("https://provider.example/v1") + ); + assert_eq!( + descriptor + .env + .get("GOOSE_PROVIDER__HOST") + .map(String::as_str), + Some("https://provider.example/v1") + ); + + let effective = EffectiveAgentEnv { + env: descriptor.env, + config_file_path: Some("~/.config/goose/config.yaml"), + effective_command: descriptor.command, + }; + assert!(agent_readiness(&effective).is_ready()); +} + +#[test] +fn descriptor_keeps_legacy_raw_provider_url_records_runnable() { + let record: crate::managed_agents::types::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "legacy-pubkey", + "name": "legacy-agent", + "private_key_nsec": "", + "relay_url": "", + "acp_command": "buzz-acp", + "agent_command": "goose", + "runtime": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "parallelism": 1, + "system_prompt": null, + "model": "legacy-model", + "provider": "http://127.0.0.1:9337/v1", + "env_vars": {"OPENAI_COMPAT_API_KEY": "test-key"}, + "created_at": "", + "updated_at": "", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("legacy managed agent record"); + + let descriptor = resolve_effective_harness_descriptor(&record, &[], &Default::default()) + .expect("descriptor"); + + assert_eq!( + descriptor.env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + for key in [ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", + ] { + assert_eq!( + descriptor.env.get(key).map(String::as_str), + Some("http://127.0.0.1:9337/v1") + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 0ce5ca7b219..c580ed5b16d 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -26,7 +26,7 @@ pub(crate) use metadata::{ apply_agent_display_env, resolve_session_title, runtime_metadata_env_vars, DISPLAY_NAME_ENV_VAR, SESSION_TITLE_ENV_VAR, }; - +pub(crate) mod provider_env; mod stop; pub(crate) use stop::managed_agent_runtime_keys; pub use stop::{stop_managed_agent_process, stop_managed_agent_workspace_pair}; diff --git a/desktop/src-tauri/src/managed_agents/runtime/provider_env.rs b/desktop/src-tauri/src/managed_agents/runtime/provider_env.rs new file mode 100644 index 00000000000..080c674d486 --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/provider_env.rs @@ -0,0 +1,239 @@ +use std::collections::{BTreeMap, BTreeSet}; + +use crate::managed_agents::KnownAcpRuntime; + +#[derive(Debug, Clone, Copy)] +struct RuntimeProviderEnvMapping { + provider_id: &'static str, + runtime_provider_id: &'static str, + provider_url_env_var: Option<&'static str>, + env_aliases: &'static [(&'static str, &'static str)], +} + +const GOOSE_OPENAI_API_KEY_ALIASES: &[(&str, &str)] = &[ + ("OPENAI_COMPAT_API_KEY", "GOOSE_PROVIDER__API_KEY"), + ("OPENAI_COMPAT_API_KEY", "OPENAI_API_KEY"), +]; + +const GOOSE_OPENAI_COMPAT_ENV_ALIASES: &[(&str, &str)] = &[ + ("OPENAI_COMPAT_API_KEY", "GOOSE_PROVIDER__API_KEY"), + ("OPENAI_COMPAT_API_KEY", "OPENAI_API_KEY"), + ("OPENAI_COMPAT_BASE_URL", "GOOSE_PROVIDER__HOST"), + ("OPENAI_COMPAT_BASE_URL", "OPENAI_HOST"), + ("OPENAI_COMPAT_BASE_URL", "OPENAI_BASE_URL"), +]; + +const GOOSE_PROVIDER_ENV_MAPPINGS: &[RuntimeProviderEnvMapping] = &[ + RuntimeProviderEnvMapping { + provider_id: "openai", + runtime_provider_id: "openai", + provider_url_env_var: None, + env_aliases: GOOSE_OPENAI_API_KEY_ALIASES, + }, + RuntimeProviderEnvMapping { + provider_id: "openai-compat", + runtime_provider_id: "openai", + provider_url_env_var: Some("OPENAI_COMPAT_BASE_URL"), + env_aliases: GOOSE_OPENAI_COMPAT_ENV_ALIASES, + }, +]; + +fn provider_env_mappings(runtime: &KnownAcpRuntime) -> &'static [RuntimeProviderEnvMapping] { + match runtime.id { + "goose" => GOOSE_PROVIDER_ENV_MAPPINGS, + _ => &[], + } +} + +const OPENAI_COMPAT_BASE_URL_KEYS: &[&str] = &[ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", +]; + +fn runtime_provider_env_mapping( + runtime: &KnownAcpRuntime, + configured_provider: &str, +) -> Option<(&'static RuntimeProviderEnvMapping, bool)> { + let mappings = provider_env_mappings(runtime); + if let Some(mapping) = mappings + .iter() + .find(|mapping| mapping.provider_id == configured_provider) + { + return Some((mapping, false)); + } + + provider_is_http_base_url(configured_provider) + .then(|| { + mappings + .iter() + .find(|mapping| mapping.provider_id == "openai-compat") + .map(|mapping| (mapping, true)) + }) + .flatten() +} + +/// Mirror canonical/runtime-native aliases inside one precedence layer. +/// +/// Runtime-native values win only when both spellings occur in the same +/// layer. Normalizing before layers are merged makes a higher-layer value win +/// regardless of which spelling that layer uses. +fn normalize_runtime_provider_env_layer( + runtime: &KnownAcpRuntime, + configured_provider: Option<&str>, + env: &mut BTreeMap, +) { + let Some(configured_provider) = configured_provider else { + return; + }; + let Some((mapping, _)) = runtime_provider_env_mapping(runtime, configured_provider) else { + return; + }; + + let mut normalized_source_keys = BTreeSet::new(); + for (source_key, _) in mapping.env_aliases { + if !normalized_source_keys.insert(*source_key) { + continue; + } + let resolved_value = mapping + .env_aliases + .iter() + .filter(|(candidate_source_key, _)| candidate_source_key == source_key) + .find_map(|(_, runtime_key)| env.get(*runtime_key).cloned()) + .or_else(|| env.get(*source_key).cloned()); + if let Some(mut value) = resolved_value { + if mapping.provider_url_env_var == Some(*source_key) { + value = value.trim().to_string(); + } + env.insert((*source_key).to_string(), value.clone()); + for (_, runtime_key) in mapping + .env_aliases + .iter() + .filter(|(candidate_source_key, _)| candidate_source_key == source_key) + { + env.insert((*runtime_key).to_string(), value.clone()); + } + } + } +} + +/// Merge filtered env layers while preserving alias-aware precedence, then +/// adapt Buzz's canonical provider configuration to the runtime's names. +/// +/// `configured_provider` must come from the effective structured config. It is +/// deliberately not inferred from a layered `GOOSE_PROVIDER`/ +/// `BUZZ_AGENT_PROVIDER`, because linked definitions are authoritative. +pub(crate) fn merge_runtime_provider_env_layers( + runtime: &KnownAcpRuntime, + configured_provider: Option<&str>, + env: &mut BTreeMap, + layers: impl IntoIterator>, +) { + let configured_provider = configured_provider + .map(str::trim) + .filter(|provider| !provider.is_empty()); + normalize_runtime_provider_env_layer(runtime, configured_provider, env); + for mut layer in layers { + normalize_runtime_provider_env_layer(runtime, configured_provider, &mut layer); + env.extend(layer); + } + apply_runtime_provider_env_mapping(runtime, configured_provider, env); +} + +/// Apply the effective structured provider and its runtime-specific aliases. +pub(crate) fn apply_runtime_provider_env_mapping( + runtime: &KnownAcpRuntime, + configured_provider: Option<&str>, + env: &mut BTreeMap, +) { + let Some(provider_env_var) = runtime.provider_env_var else { + return; + }; + let Some(configured_provider) = configured_provider + .map(str::trim) + .filter(|provider| !provider.is_empty()) + else { + return; + }; + + let Some((mapping, provider_is_url)) = + runtime_provider_env_mapping(runtime, configured_provider) + else { + return; + }; + + if provider_is_url { + if let Some(base_url_env_var) = mapping.provider_url_env_var { + let provider_url = configured_provider.to_string(); + // Legacy v0.4.24 records stored the endpoint in `provider`. That + // selected endpoint must beat any stale inherited base URL. + env.insert(base_url_env_var.to_string(), provider_url.clone()); + for (source_key, runtime_key) in mapping.env_aliases { + if *source_key == base_url_env_var { + env.insert((*runtime_key).to_string(), provider_url.clone()); + } + } + } + } else if mapping.provider_id == "openai" { + // The ordinary OpenAI provider must not inherit a compatibility host + // from an earlier openai-compat selection. + for key in OPENAI_COMPAT_BASE_URL_KEYS { + env.remove(*key); + } + } + normalize_runtime_provider_env_layer(runtime, Some(configured_provider), env); + + env.insert( + provider_env_var.to_string(), + mapping.runtime_provider_id.to_string(), + ); +} + +pub(crate) fn validate_provider_value(provider: &str) -> Result<(), String> { + let trimmed = provider.trim(); + let normalized = trimmed.to_ascii_lowercase(); + if !normalized.starts_with("http://") && !normalized.starts_with("https://") { + return Ok(()); + } + validate_openai_compat_base_url(trimmed) +} + +pub(crate) fn validate_openai_compat_base_url(value: &str) -> Result<(), String> { + let trimmed = value.trim(); + let url = url::Url::parse(trimmed) + .map_err(|_| "OpenAI-compatible base URL must be a valid HTTP(S) URL".to_string())?; + if !matches!(url.scheme(), "http" | "https") || url.host_str().is_none() { + return Err("OpenAI-compatible base URL must include a valid HTTP(S) host".to_string()); + } + if !url.username().is_empty() + || url.password().is_some() + || url.query().is_some() + || url.fragment().is_some() + { + return Err( + "OpenAI-compatible base URL cannot include credentials, query parameters, or fragments" + .to_string(), + ); + } + Ok(()) +} + +pub(crate) fn validate_provider_env_urls( + env_vars: &BTreeMap, +) -> Result<(), String> { + if let Some(value) = env_vars + .get("OPENAI_COMPAT_BASE_URL") + .filter(|value| !value.trim().is_empty()) + { + validate_openai_compat_base_url(value)?; + } + Ok(()) +} + +pub(crate) fn provider_is_http_base_url(provider: &str) -> bool { + validate_openai_compat_base_url(provider).is_ok() +} + +#[cfg(test)] +mod tests; diff --git a/desktop/src-tauri/src/managed_agents/runtime/provider_env/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/provider_env/tests.rs new file mode 100644 index 00000000000..3661f3603eb --- /dev/null +++ b/desktop/src-tauri/src/managed_agents/runtime/provider_env/tests.rs @@ -0,0 +1,231 @@ +use std::collections::BTreeMap; + +use super::{ + apply_runtime_provider_env_mapping, merge_runtime_provider_env_layers, + provider_is_http_base_url, validate_openai_compat_base_url, +}; + +fn goose() -> &'static crate::managed_agents::KnownAcpRuntime { + crate::managed_agents::discovery::known_acp_runtime_exact("goose").expect("goose runtime") +} + +#[test] +fn goose_openai_compatible_mapping_uses_runtime_native_names() { + let mut env = BTreeMap::from([ + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "canonical-key".to_string(), + ), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://localhost:1234/v1".to_string(), + ), + ]); + + apply_runtime_provider_env_mapping(goose(), Some("openai-compat"), &mut env); + + assert_eq!( + env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("canonical-key") + ); + assert_eq!( + env.get("GOOSE_PROVIDER__API_KEY").map(String::as_str), + Some("canonical-key") + ); + for key in [ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", + ] { + assert_eq!( + env.get(key).map(String::as_str), + Some("http://localhost:1234/v1") + ); + } +} + +#[test] +fn ordinary_openai_clears_inherited_compatibility_hosts() { + let mut env = BTreeMap::from([ + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "canonical-key".to_string(), + ), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://stale.example/v1".to_string(), + ), + ( + "GOOSE_PROVIDER__HOST".to_string(), + "http://stale.example/v1".to_string(), + ), + ( + "OPENAI_HOST".to_string(), + "http://stale.example/v1".to_string(), + ), + ( + "OPENAI_BASE_URL".to_string(), + "http://stale.example/v1".to_string(), + ), + ]); + + apply_runtime_provider_env_mapping(goose(), Some("openai"), &mut env); + + assert_eq!( + env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("canonical-key") + ); + for key in [ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", + ] { + assert!(!env.contains_key(key), "{key} should be cleared"); + } +} + +#[test] +fn openai_compatible_mapping_trims_base_url_aliases() { + let mut env = BTreeMap::from([( + "OPENAI_COMPAT_BASE_URL".to_string(), + " http://localhost:1234/v1 ".to_string(), + )]); + + apply_runtime_provider_env_mapping(goose(), Some("openai-compat"), &mut env); + + for key in [ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", + ] { + assert_eq!( + env.get(key).map(String::as_str), + Some("http://localhost:1234/v1") + ); + } +} + +#[test] +fn effective_provider_overrides_arbitrary_layered_goose_provider() { + let mut env = BTreeMap::new(); + let agent = BTreeMap::from([("GOOSE_PROVIDER".to_string(), "anthropic".to_string())]); + + merge_runtime_provider_env_layers(goose(), Some("openai-compat"), &mut env, [agent]); + + assert_eq!( + env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); +} + +#[test] +fn higher_canonical_layer_overrides_lower_native_layer() { + let mut env = BTreeMap::new(); + let global = BTreeMap::from([ + ("OPENAI_API_KEY".to_string(), "global-key".to_string()), + ( + "OPENAI_BASE_URL".to_string(), + "http://global.example/v1".to_string(), + ), + ]); + let agent = BTreeMap::from([ + ("OPENAI_COMPAT_API_KEY".to_string(), "agent-key".to_string()), + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://agent.example/v1".to_string(), + ), + ]); + + merge_runtime_provider_env_layers(goose(), Some("openai-compat"), &mut env, [global, agent]); + + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("agent-key") + ); + assert_eq!( + env.get("OPENAI_COMPAT_API_KEY").map(String::as_str), + Some("agent-key") + ); + assert_eq!( + env.get("OPENAI_BASE_URL").map(String::as_str), + Some("http://agent.example/v1") + ); +} + +#[test] +fn padded_provider_preserves_alias_aware_layer_precedence() { + let mut env = BTreeMap::new(); + let global = BTreeMap::from([("OPENAI_API_KEY".to_string(), "global-key".to_string())]); + let agent = BTreeMap::from([("OPENAI_COMPAT_API_KEY".to_string(), "agent-key".to_string())]); + + merge_runtime_provider_env_layers(goose(), Some(" openai-compat "), &mut env, [global, agent]); + + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("agent-key") + ); +} + +#[test] +fn legacy_raw_provider_url_maps_to_openai_and_overrides_stale_base_url() { + let mut env = BTreeMap::from([ + ( + "OPENAI_COMPAT_BASE_URL".to_string(), + "http://stale.example/v1".to_string(), + ), + ( + "OPENAI_COMPAT_API_KEY".to_string(), + "canonical-key".to_string(), + ), + ]); + + apply_runtime_provider_env_mapping(goose(), Some("http://selected.example/v1"), &mut env); + + assert_eq!( + env.get("GOOSE_PROVIDER").map(String::as_str), + Some("openai") + ); + for key in [ + "OPENAI_COMPAT_BASE_URL", + "GOOSE_PROVIDER__HOST", + "OPENAI_HOST", + "OPENAI_BASE_URL", + ] { + assert_eq!( + env.get(key).map(String::as_str), + Some("http://selected.example/v1") + ); + } + assert_eq!( + env.get("OPENAI_API_KEY").map(String::as_str), + Some("canonical-key") + ); +} + +#[test] +fn provider_urls_reject_publishable_secret_material() { + for invalid in [ + "https://user:password@example.com/v1", + "https://example.com/v1?api_key=secret", + "https://example.com/v1#api-key", + "file:///tmp/provider", + ] { + assert!( + validate_openai_compat_base_url(invalid).is_err(), + "{invalid} must be rejected" + ); + assert!(!provider_is_http_base_url(invalid)); + } + assert!(provider_is_http_base_url("https://example.com/v1")); +} 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..f0bbe020d2a 100644 --- a/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs +++ b/desktop/src-tauri/src/managed_agents/spawn_snapshot/tests.rs @@ -134,6 +134,31 @@ fn snapshot_is_deterministic() { ); } +#[test] +fn openai_compatible_base_url_change_trips_spawn_snapshot() { + let mut before = record(); + before.provider = Some("openai-compat".into()); + before.model = Some("provider-model".into()); + before.env_vars.insert( + "OPENAI_COMPAT_BASE_URL".into(), + "https://provider-a.example/v1".into(), + ); + before + .env_vars + .insert("OPENAI_COMPAT_API_KEY".into(), "test-key".into()); + let mut after = before.clone(); + after.env_vars.insert( + "OPENAI_COMPAT_BASE_URL".into(), + "https://provider-b.example/v1".into(), + ); + + assert_ne!( + snapshot(&before, &[], &[], "wss://ws.example", &Default::default()), + snapshot(&after, &[], &[], "wss://ws.example", &Default::default()), + "the restart badge must cover the mapped endpoint spawn receives" + ); +} + #[test] fn materializing_runtime_keeps_snapshot_stable() { // Migration cutover invariant (Phase 1A): materializing the linked diff --git a/desktop/src/features/agents/AGENTS.md b/desktop/src/features/agents/AGENTS.md index 7822211541b..a5baa26ba3c 100644 --- a/desktop/src/features/agents/AGENTS.md +++ b/desktop/src/features/agents/AGENTS.md @@ -250,6 +250,12 @@ with a TypeScript lookup table or an id comparison in a component. refresh only local persona/team/managed-agent caches; they must never invalidate the remote relay directory. +15. **Provider translation belongs to runtime metadata.** When Buzz's canonical + provider ID or env names differ from what a harness reads, declare the + mapping on `KnownAcpRuntime` and apply it after effective config precedence + is resolved. Do not duplicate provider aliases in UI components or + individual readiness, discovery, and spawn call sites. + ## The tests that enforce this - `lib/agentConfigCore.test.mjs` — field model per harness × scope, clearing