diff --git a/desktop/scripts/check-file-sizes.mjs b/desktop/scripts/check-file-sizes.mjs index 6f0281e9b..24727be06 100644 --- a/desktop/scripts/check-file-sizes.mjs +++ b/desktop/scripts/check-file-sizes.mjs @@ -67,9 +67,11 @@ const overrides = new Map([ // harness-persona-sync feature growth, queued to split in the resolver-unify // refactor followup. discovery.rs is dominated by the new test module // (the effective_agent_command / divergent / create-time override matrix); + // alias-preservation coverage extends that matrix so create-time persona + // agents keep an installed runtime alias when the primary command is absent. // types.rs adds the persona/instance harness fields. Load-bearing, not // generic debt. - ["src-tauri/src/managed_agents/discovery.rs", 1043], + ["src-tauri/src/managed_agents/discovery.rs", 1085], ["src-tauri/src/managed_agents/types.rs", 1037], // migration_tests.rs carries the harness-sync migration coverage plus the // patch_json_records owner-only writeback regression test (SECURITY.md:90 diff --git a/desktop/src-tauri/src/commands/agent_models.rs b/desktop/src-tauri/src/commands/agent_models.rs index d0261714e..afde620d5 100644 --- a/desktop/src-tauri/src/commands/agent_models.rs +++ b/desktop/src-tauri/src/commands/agent_models.rs @@ -1,6 +1,10 @@ -use std::collections::HashSet; +use std::{ + collections::{BTreeMap, HashSet}, + path::PathBuf, +}; use nostr::Keys; +use serde::Deserialize; use tauri::{AppHandle, State}; use crate::{ @@ -9,9 +13,9 @@ use crate::{ build_managed_agent_summary, current_instance_id, default_agent_workdir, find_managed_agent_mut, known_acp_runtime, load_managed_agents, load_personas, managed_agent_avatar_url, missing_command_message, normalize_agent_args, resolve_command, - resolve_effective_prompt_model_provider, save_managed_agents, sync_managed_agent_processes, - try_regenerate_nest, AgentModelInfo, AgentModelsResponse, UpdateManagedAgentRequest, - UpdateManagedAgentResponse, + save_managed_agents, sync_managed_agent_processes, try_regenerate_nest, AgentModelInfo, + AgentModelsResponse, UpdateManagedAgentRequest, UpdateManagedAgentResponse, + DEFAULT_ACP_COMMAND, }, relay::{relay_ws_url_with_override, sync_managed_agent_profile}, util::now_iso, @@ -27,7 +31,7 @@ pub async fn get_agent_models( app: AppHandle, state: State<'_, AppState>, ) -> Result { - let (resolved_acp, agent_command, agent_args, persisted_model, merged_env) = { + let (resolved_acp, agent_command, agent_args, persisted_model, effective_provider, merged_env) = { let _store_guard = state .managed_agents_store_lock .lock() @@ -65,26 +69,542 @@ pub async fn get_agent_models( .map(|p| p.display().to_string()) .unwrap_or_else(|| effective_command.clone()); - // Same env layering as runtime spawn: persona env < agent env. - // Model discovery needs the user's credentials. Fail closed on - // persona-resolution errors so a corrupt personas.json doesn't - // produce a model list as if the persona had no credentials. - let persona_env = - crate::managed_agents::resolve_persona_env(&app, record.persona_id.as_deref())?; - let env = crate::managed_agents::merged_user_env(&persona_env, &record.env_vars); - - // Resolve the effective model from the linked persona so the ModelPicker - // dropdown shows the current persona model as selected. - let (_prompt, effective_model, _provider) = resolve_effective_prompt_model_provider( - record.persona_id.as_deref(), - &personas, - record.system_prompt.clone(), - record.model.clone(), - ); - - (resolved, resolved_agent, args, effective_model, env) + // ModelPicker can persist a selected model but not rewrite the saved + // provider/env snapshot, and runtime spawn reads that same snapshot. + // Discover models against the record snapshot so an out-of-date persona + // cannot offer models for a provider this agent will not launch with. + let discovery = saved_agent_model_discovery_config(record, &effective_command); + + ( + resolved, + resolved_agent, + args, + discovery.model, + discovery.provider, + discovery.env, + ) }; // store lock released — subprocess runs without holding the lock + if let Some(models) = discover_openai_compatible_models( + &state.http_client, + effective_provider.as_deref(), + &merged_env, + persisted_model.clone(), + ) + .await? + { + return Ok(models); + } + + if let Some(models) = discover_anthropic_models( + &state.http_client, + effective_provider.as_deref(), + &merged_env, + persisted_model.clone(), + ) + .await? + { + return Ok(models); + } + + run_agent_models_command( + resolved_acp, + agent_command, + agent_args, + persisted_model, + merged_env, + ) + .await +} + +#[derive(Debug, PartialEq, Eq)] +struct SavedAgentModelDiscoveryConfig { + model: Option, + provider: Option, + env: BTreeMap, +} + +fn saved_agent_model_discovery_config( + record: &crate::managed_agents::ManagedAgentRecord, + agent_command: &str, +) -> SavedAgentModelDiscoveryConfig { + let mut derived_env = BTreeMap::new(); + if let Some(meta) = known_acp_runtime(agent_command) { + for (key, value) in crate::managed_agents::runtime_metadata_env_vars( + meta.model_env_var, + meta.provider_env_var, + meta.provider_locked, + record.model.as_deref(), + record.provider.as_deref(), + ) { + derived_env.insert(key.to_string(), value.to_string()); + } + } + + SavedAgentModelDiscoveryConfig { + model: record.model.clone(), + provider: record.provider.clone(), + env: crate::managed_agents::merged_user_env(&derived_env, &record.env_vars), + } +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct DiscoverAgentModelsInput { + #[serde(default)] + pub acp_command: Option, + pub agent_command: String, + #[serde(default)] + pub agent_args: Vec, + #[serde(default)] + pub provider: Option, + #[serde(default)] + pub env_vars: BTreeMap, +} + +/// Query available models from an unsaved agent configuration. +/// +/// This powers the new-agent dialog before a persona/agent record exists. It +/// mirrors the saved-agent discovery command, but derives runtime/provider/env +/// from the current form state instead of loading a persisted record. +#[tauri::command] +pub async fn discover_agent_models( + input: DiscoverAgentModelsInput, + state: State<'_, AppState>, +) -> Result { + crate::managed_agents::validate_user_env_keys(&input.env_vars)?; + + let acp_command = input + .acp_command + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or(DEFAULT_ACP_COMMAND); + let resolved_acp = resolve_command(acp_command) + .ok_or_else(|| missing_command_message(acp_command, "ACP harness command"))?; + + let agent_command = input.agent_command.trim(); + if agent_command.is_empty() { + return Err("agent command is required for model discovery".to_string()); + } + let agent_args = normalize_agent_args(agent_command, input.agent_args); + let resolved_agent = resolve_command(agent_command) + .map(|p| p.display().to_string()) + .unwrap_or_else(|| agent_command.to_string()); + + let mut derived_env = BTreeMap::new(); + if let Some(meta) = known_acp_runtime(agent_command) { + let provider = input + .provider + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()); + if !meta.provider_locked { + if let (Some(env_key), Some(provider)) = (meta.provider_env_var, provider) { + derived_env.insert(env_key.to_string(), provider.to_string()); + } + } + } + let merged_env = crate::managed_agents::merged_user_env(&derived_env, &input.env_vars); + + if let Some(models) = discover_openai_compatible_models( + &state.http_client, + input.provider.as_deref(), + &merged_env, + None, + ) + .await? + { + return Ok(models); + } + + if let Some(models) = discover_anthropic_models( + &state.http_client, + input.provider.as_deref(), + &merged_env, + None, + ) + .await? + { + return Ok(models); + } + + run_agent_models_command(resolved_acp, resolved_agent, agent_args, None, merged_env).await +} + +#[derive(Debug, Deserialize)] +struct OpenAiModelListResponse { + data: Vec, +} + +#[derive(Debug, Deserialize)] +struct OpenAiModelListItem { + id: String, + #[serde(default)] + created: Option, +} + +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") + ) +} + +#[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 env_value(env: &BTreeMap, key: &str) -> Option { + env.get(key) + .map(String::as_str) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) +} + +fn env_or_process_value(env: &BTreeMap, key: &str) -> Option { + env_value(env, key).or_else(|| { + std::env::var(key) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + }) +} + +fn redaction_env_with_value( + env: &BTreeMap, + key: &str, + value: &str, +) -> BTreeMap { + let mut redaction_env = env.clone(); + redaction_env.insert(key.to_string(), value.to_string()); + redaction_env +} + +fn is_agent_text_model_id(id: &str) -> bool { + let lower = id.to_ascii_lowercase(); + if [ + "audio", + "dall-e", + "embedding", + "image", + "moderation", + "realtime", + "speech", + "transcribe", + "tts", + "whisper", + ] + .iter() + .any(|needle| lower.contains(needle)) + { + return false; + } + + lower.starts_with("gpt-") || lower.starts_with('o') || lower.starts_with("chatgpt-") +} + +fn openai_dated_snapshot_alias(id: &str) -> Option { + let (base, date) = id.rsplit_once('-')?; + if date.len() != 2 || !date.chars().all(|character| character.is_ascii_digit()) { + return None; + } + let (base, month) = base.rsplit_once('-')?; + if month.len() != 2 || !month.chars().all(|character| character.is_ascii_digit()) { + return None; + } + let (base, year) = base.rsplit_once('-')?; + if year.len() != 4 || !year.chars().all(|character| character.is_ascii_digit()) { + return None; + } + + Some(base.to_string()) +} + +fn openai_model_display_name(id: &str) -> String { + let canonical = openai_dated_snapshot_alias(id).unwrap_or_else(|| id.to_string()); + if let Some(rest) = canonical.strip_prefix("chatgpt-") { + return format!("ChatGPT {}", title_case_model_suffix(rest, false)); + } + if let Some(rest) = canonical.strip_prefix("gpt-") { + return format!("GPT-{}", title_case_model_suffix(rest, true)); + } + + canonical +} + +fn title_case_model_suffix(value: &str, preserve_first_separator: bool) -> String { + value + .split('-') + .enumerate() + .map(|(index, part)| { + let part = if part.eq_ignore_ascii_case("pro") { + "Pro".to_string() + } else if part.eq_ignore_ascii_case("mini") { + "mini".to_string() + } else if part.eq_ignore_ascii_case("nano") { + "nano".to_string() + } else { + part.to_string() + }; + + if preserve_first_separator && index == 0 { + part + } else if index == 0 { + part + } else { + format!(" {part}") + } + }) + .collect::() +} + +fn normalize_openai_compatible_models( + response: OpenAiModelListResponse, + provider: Option<&str>, +) -> Vec { + let mut seen = HashSet::new(); + let mut items = response.data; + let filter_to_openai_text_models = matches!( + provider + .map(str::trim) + .map(str::to_ascii_lowercase) + .as_deref(), + Some("openai") + ); + let all_ids = items + .iter() + .map(|item| item.id.clone()) + .collect::>(); + items.sort_by(|left, right| { + right + .created + .cmp(&left.created) + .then_with(|| left.id.cmp(&right.id)) + }); + + items + .into_iter() + .filter(|item| !filter_to_openai_text_models || is_agent_text_model_id(&item.id)) + .filter(|item| match openai_dated_snapshot_alias(&item.id) { + Some(alias) if filter_to_openai_text_models => !all_ids.contains(&alias), + Some(_) | None => true, + }) + .filter(|item| seen.insert(item.id.clone())) + .map(|item| AgentModelInfo { + name: Some(openai_model_display_name(&item.id)), + id: item.id, + description: None, + }) + .collect() +} + +async fn discover_openai_compatible_models( + client: &reqwest::Client, + provider: Option<&str>, + env: &BTreeMap, + selected_model: Option, +) -> Result, String> { + if !is_openai_compatible_provider(provider) { + return Ok(None); + } + + let api_key = env_or_process_value(env, "OPENAI_COMPAT_API_KEY") + .ok_or_else(|| "config: OPENAI_COMPAT_API_KEY required".to_string())?; + let redaction_env = redaction_env_with_value(env, "OPENAI_COMPAT_API_KEY", &api_key); + let url = openai_compatible_models_url_for_discovery(env); + let response = client + .get(&url) + .bearer_auth(&api_key) + .send() + .await + .map_err(|error| format!("OpenAI model discovery request failed: {error}"))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + let body = crate::managed_agents::redact_env_values_in(&body, &redaction_env); + return Err(format!("OpenAI model discovery HTTP {status}: {body}")); + } + + let response = response + .json::() + .await + .map_err(|error| format!("OpenAI model discovery response parse failed: {error}"))?; + let models = normalize_openai_compatible_models(response, provider); + if models.is_empty() { + return Err("OpenAI model discovery returned no compatible text models".to_string()); + } + + Ok(Some(AgentModelsResponse { + agent_name: provider.unwrap_or("openai").trim().to_string(), + agent_version: "models-api".to_string(), + models, + agent_default_model: None, + selected_model, + supports_switching: true, + })) +} + +#[derive(Debug, Deserialize)] +struct AnthropicModelListResponse { + data: Vec, + #[serde(default)] + has_more: bool, + #[serde(default)] + last_id: Option, +} + +#[derive(Debug, Deserialize)] +struct AnthropicModelListItem { + id: String, + #[serde(default)] + display_name: Option, +} + +fn is_anthropic_provider(provider: Option<&str>) -> bool { + matches!( + provider + .map(str::trim) + .map(str::to_ascii_lowercase) + .as_deref(), + Some("anthropic") + ) +} + +#[cfg(test)] +fn anthropic_models_url(env: &BTreeMap) -> String { + let base_url = env_value(env, "ANTHROPIC_BASE_URL") + .unwrap_or_else(|| "https://api.anthropic.com".to_string()); + anthropic_models_url_from_base(&base_url) +} + +fn anthropic_models_url_for_discovery(env: &BTreeMap) -> String { + let base_url = env_or_process_value(env, "ANTHROPIC_BASE_URL") + .unwrap_or_else(|| "https://api.anthropic.com".to_string()); + anthropic_models_url_from_base(&base_url) +} + +fn anthropic_models_url_from_base(base_url: &str) -> String { + let base_url = base_url.trim_end_matches('/'); + if base_url.ends_with("/v1") { + format!("{base_url}/models") + } else { + format!("{base_url}/v1/models") + } +} + +fn normalize_anthropic_models(response: AnthropicModelListResponse) -> Vec { + let mut seen = HashSet::new(); + response + .data + .into_iter() + .filter(|item| seen.insert(item.id.clone())) + .map(|item| AgentModelInfo { + id: item.id, + name: item.display_name, + description: None, + }) + .collect() +} + +async fn fetch_anthropic_model_page( + client: &reqwest::Client, + url: &str, + api_key: &str, + after_id: Option<&str>, + env: &BTreeMap, +) -> Result { + let mut request = client + .get(url) + .header("x-api-key", api_key) + .header("anthropic-version", "2023-06-01"); + if let Some(after_id) = after_id { + request = request.query(&[("after_id", after_id)]); + } + + let response = request + .send() + .await + .map_err(|error| format!("Anthropic model discovery request failed: {error}"))?; + let status = response.status(); + if !status.is_success() { + let body = response.text().await.unwrap_or_default(); + let body = crate::managed_agents::redact_env_values_in(&body, env); + return Err(format!("Anthropic model discovery HTTP {status}: {body}")); + } + + response + .json::() + .await + .map_err(|error| format!("Anthropic model discovery response parse failed: {error}")) +} + +async fn discover_anthropic_models( + client: &reqwest::Client, + provider: Option<&str>, + env: &BTreeMap, + selected_model: Option, +) -> Result, String> { + if !is_anthropic_provider(provider) { + return Ok(None); + } + + let api_key = env_or_process_value(env, "ANTHROPIC_API_KEY") + .ok_or_else(|| "config: ANTHROPIC_API_KEY required".to_string())?; + let redaction_env = redaction_env_with_value(env, "ANTHROPIC_API_KEY", &api_key); + let url = anthropic_models_url_for_discovery(env); + let mut models = Vec::new(); + let mut after_id: Option = None; + for _ in 0..20 { + let response = + fetch_anthropic_model_page(client, &url, &api_key, after_id.as_deref(), &redaction_env) + .await?; + let has_more = response.has_more; + after_id = response.last_id.clone(); + models.extend(normalize_anthropic_models(response)); + if !has_more { + break; + } + if after_id.as_deref().unwrap_or_default().is_empty() { + return Err("Anthropic model discovery pagination did not return last_id".to_string()); + } + } + let mut seen = HashSet::new(); + models.retain(|model| seen.insert(model.id.clone())); + if models.is_empty() { + return Err("Anthropic model discovery returned no models".to_string()); + } + + Ok(Some(AgentModelsResponse { + agent_name: provider.unwrap_or("anthropic").trim().to_string(), + agent_version: "models-api".to_string(), + models, + agent_default_model: None, + selected_model, + supports_switching: true, + })) +} + +async fn run_agent_models_command( + resolved_acp: PathBuf, + agent_command: String, + agent_args: Vec, + persisted_model: Option, + merged_env: BTreeMap, +) -> Result { // Clone the env map for redaction below — `merged_env` is moved // into the spawn_blocking closure and we still need the values to // scrub any user-supplied secrets that the child surfaces in stderr. @@ -112,6 +632,11 @@ pub async fn get_agent_models( } } } + // Mirror runtime spawn: internal builds may bake Databricks host/model + // defaults. User-provided env below still wins. + for (key, value) in crate::managed_agents::build_databricks_defaults() { + cmd.env(key, value); + } // User env layering — written LAST so it overrides any Buzz-set env above. for (k, v) in &merged_env { cmd.env(k, v); @@ -415,3 +940,7 @@ 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_tests.rs b/desktop/src-tauri/src/commands/agent_models_tests.rs new file mode 100644 index 000000000..13f781659 --- /dev/null +++ b/desktop/src-tauri/src/commands/agent_models_tests.rs @@ -0,0 +1,213 @@ +use super::*; + +#[test] +fn openai_model_normalization_keeps_agent_text_models() { + let models = normalize_openai_compatible_models( + OpenAiModelListResponse { + data: vec![ + OpenAiModelListItem { + id: "text-embedding-3-large".to_string(), + created: Some(4), + }, + OpenAiModelListItem { + id: "gpt-image-2".to_string(), + created: Some(5), + }, + OpenAiModelListItem { + id: "chatgpt-5.5-pro-2026-04-23".to_string(), + created: Some(7), + }, + OpenAiModelListItem { + id: "chatgpt-5.5-pro".to_string(), + created: Some(6), + }, + OpenAiModelListItem { + id: "gpt-5.4-mini".to_string(), + created: Some(2), + }, + OpenAiModelListItem { + id: "o4-mini".to_string(), + created: Some(3), + }, + OpenAiModelListItem { + id: "gpt-5.4-mini".to_string(), + created: Some(1), + }, + ], + }, + Some("openai"), + ); + + let ids_and_names = models + .into_iter() + .map(|model| (model.id, model.name)) + .collect::>(); + assert_eq!( + ids_and_names, + vec![ + ( + "chatgpt-5.5-pro".to_string(), + Some("ChatGPT 5.5 Pro".to_string()), + ), + ("o4-mini".to_string(), Some("o4-mini".to_string())), + ("gpt-5.4-mini".to_string(), Some("GPT-5.4 mini".to_string()),), + ] + ); +} + +#[test] +fn openai_compat_model_normalization_preserves_provider_specific_ids() { + let models = normalize_openai_compatible_models( + OpenAiModelListResponse { + data: vec![ + OpenAiModelListItem { + id: "meta-llama/Llama-3.3-70B-Instruct".to_string(), + created: Some(5), + }, + OpenAiModelListItem { + id: "mistral-large-latest".to_string(), + created: Some(4), + }, + OpenAiModelListItem { + id: "anthropic/claude-sonnet-4-6".to_string(), + created: Some(3), + }, + OpenAiModelListItem { + id: "text-embedding-compatible".to_string(), + created: Some(2), + }, + OpenAiModelListItem { + id: "meta-llama/Llama-3.3-70B-Instruct".to_string(), + created: Some(1), + }, + ], + }, + Some("openai-compat"), + ); + + let ids = models.into_iter().map(|model| model.id).collect::>(); + assert_eq!( + ids, + vec![ + "meta-llama/Llama-3.3-70B-Instruct".to_string(), + "mistral-large-latest".to_string(), + "anthropic/claude-sonnet-4-6".to_string(), + "text-embedding-compatible".to_string(), + ] + ); +} + +#[test] +fn openai_models_url_uses_openai_default_base_url() { + assert_eq!( + openai_compatible_models_url(&BTreeMap::new()), + "https://api.openai.com/v1/models" + ); +} + +#[test] +fn anthropic_models_url_uses_anthropic_default_base_url() { + assert_eq!( + anthropic_models_url(&BTreeMap::new()), + "https://api.anthropic.com/v1/models" + ); +} + +#[test] +fn anthropic_models_url_accepts_versioned_base_url() { + let env = BTreeMap::from([( + "ANTHROPIC_BASE_URL".to_string(), + "https://proxy.example/v1/".to_string(), + )]); + + assert_eq!( + anthropic_models_url(&env), + "https://proxy.example/v1/models" + ); +} + +#[test] +fn anthropic_model_normalization_uses_display_names() { + let models = normalize_anthropic_models(AnthropicModelListResponse { + data: vec![ + AnthropicModelListItem { + id: "claude-opus-4-6".to_string(), + display_name: Some("Claude Opus 4.6".to_string()), + }, + AnthropicModelListItem { + id: "claude-opus-4-6".to_string(), + display_name: Some("Duplicate".to_string()), + }, + ], + has_more: false, + last_id: None, + }); + + assert_eq!(models.len(), 1); + assert_eq!(models[0].id, "claude-opus-4-6"); + assert_eq!(models[0].name.as_deref(), Some("Claude Opus 4.6")); +} + +#[test] +fn redaction_env_records_value_used_for_request() { + let env = BTreeMap::from([("OPENAI_COMPAT_API_KEY".to_string(), " ".to_string())]); + + let redaction_env = + redaction_env_with_value(&env, "OPENAI_COMPAT_API_KEY", "inherited-process-key"); + + assert_eq!( + redaction_env + .get("OPENAI_COMPAT_API_KEY") + .map(String::as_str), + Some("inherited-process-key") + ); +} + +#[test] +fn saved_agent_model_discovery_uses_record_snapshot() { + let record: crate::managed_agents::ManagedAgentRecord = serde_json::from_str( + r#"{ + "pubkey": "abcd1234", + "name": "test-agent", + "private_key_nsec": "nsec1fake", + "relay_url": "wss://localhost:3000", + "acp_command": "buzz-acp", + "agent_command": "goose", + "agent_args": [], + "mcp_command": "", + "turn_timeout_seconds": 320, + "system_prompt": null, + "model": "record-model", + "provider": "databricks", + "env_vars": { + "OPENAI_API_KEY": "record-key", + "BUZZ_PRIVATE_KEY": "must-not-leak" + }, + "created_at": "2026-01-01T00:00:00Z", + "updated_at": "2026-01-01T00:00:00Z", + "last_started_at": null, + "last_stopped_at": null, + "last_exit_code": null, + "last_error": null + }"#, + ) + .expect("sample managed agent record"); + + let config = saved_agent_model_discovery_config(&record, "goose"); + + assert_eq!(config.model.as_deref(), Some("record-model")); + assert_eq!(config.provider.as_deref(), Some("databricks")); + assert_eq!( + config.env.get("GOOSE_MODEL").map(String::as_str), + Some("record-model") + ); + assert_eq!( + config.env.get("GOOSE_PROVIDER").map(String::as_str), + Some("databricks") + ); + assert_eq!( + config.env.get("OPENAI_API_KEY").map(String::as_str), + Some("record-key") + ); + assert!(!config.env.contains_key("BUZZ_PRIVATE_KEY")); +} diff --git a/desktop/src-tauri/src/lib.rs b/desktop/src-tauri/src/lib.rs index 82b5caa0c..33a8860dc 100644 --- a/desktop/src-tauri/src/lib.rs +++ b/desktop/src-tauri/src/lib.rs @@ -508,6 +508,7 @@ pub fn run() { delete_managed_agent, get_managed_agent_log, get_agent_models, + discover_agent_models, mesh_availability, mesh_start_node, mesh_ensure_client_node, diff --git a/desktop/src-tauri/src/managed_agents/discovery.rs b/desktop/src-tauri/src/managed_agents/discovery.rs index 2e3518b50..042b71f8f 100644 --- a/desktop/src-tauri/src/managed_agents/discovery.rs +++ b/desktop/src-tauri/src/managed_agents/discovery.rs @@ -243,11 +243,9 @@ pub fn default_agent_command() -> String { .to_string() } -/// Resolve the agent command (harness) for a spawn/deploy/summary. Mirrors the -/// model resolution in `resolve_effective_prompt_model_provider`: the linked +/// Resolve the agent command (harness) for a spawn/deploy/summary. The linked /// persona wins so persona harness edits propagate on the next spawn. An -/// explicit per-instance override (`agent_command_override`) takes precedence, -/// matching the opt-in `record.model` override pattern. +/// explicit per-instance override (`agent_command_override`) takes precedence. /// /// Resolution order: /// 1. explicit override (non-empty) — a deliberate per-instance pin; @@ -315,9 +313,9 @@ pub fn divergent_agent_command_override( /// distinct cases that the backend MUST tell apart: /// /// - DELIBERATE OVERRIDE (`harness_override` true): the user explicitly picked a -/// non-persona runtime in a deploy dialog that exposes a runtime selector (e.g. -/// `AddChannelBotDialog`, "overriding persona preferences"). This is a real pin -/// and is preserved via `divergent_agent_command_override`. +/// runtime command in UI that exposes a runtime selector. This is a real pin +/// and is preserved when it differs from the command inheritance would spawn, +/// including installed aliases such as `claude-code-acp`. /// - MISSING-RUNTIME FALLBACK (`harness_override` false): the persona's runtime /// isn't installed locally, so `resolvePersonaRuntime` substitutes a fallback /// default. This is NOT a pin — baking it would freeze the agent on the fallback @@ -341,6 +339,15 @@ pub fn create_time_agent_command_override( if persona_id.is_some() && !harness_override { return None; } + + if persona_id.is_some() && harness_override { + let picked = picked_command + .map(str::trim) + .filter(|value| !value.is_empty())?; + let inherited_command = effective_agent_command(persona_id, personas, None); + return (picked != inherited_command).then(|| picked.to_string()); + } + divergent_agent_command_override(persona_id, personas, picked_command) } @@ -1027,6 +1034,38 @@ mod tests { ); } + #[test] + fn create_time_override_preserves_selected_runtime_alias() { + // A `claude` persona inherits the primary command `claude-agent-acp`, + // but discovery may select an installed alias such as `claude-code-acp`. + // When UI marks that create-time selection as explicit, preserve the + // alias so the first spawn uses a command known to be installed. + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override( + Some("p1"), + &personas, + Some("claude-code-acp"), + true + ), + Some("claude-code-acp".to_string()) + ); + } + + #[test] + fn create_time_override_inherits_exact_persona_command() { + let personas = vec![persona_with_runtime("p1", Some("claude"))]; + assert_eq!( + create_time_agent_command_override( + Some("p1"), + &personas, + Some("claude-agent-acp"), + true + ), + None + ); + } + #[test] fn create_time_override_preserves_pin_for_persona_less_create() { // The standalone CreateAgentDialog creates persona-LESS agents. With no diff --git a/desktop/src-tauri/src/managed_agents/env_vars.rs b/desktop/src-tauri/src/managed_agents/env_vars.rs index 979bbcde1..5ea746512 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars.rs @@ -296,36 +296,5 @@ pub(crate) fn merged_user_env( merged } -/// Resolve a managed-agent record's persona env_vars, failing closed. -/// -/// Three outcomes: -/// - `persona_id` is `None` → returns `Ok({})` (agent has no persona, no env to inherit). -/// - `persona_id = Some(id)` and the persona exists → returns `Ok(persona.env_vars)`. -/// - `persona_id = Some(id)` and either personas.json fails to load OR no persona -/// with that id exists → returns `Err(...)`. -/// -/// Why fail closed: persona env_vars frequently carry API credentials -/// (ANTHROPIC_API_KEY, OPENAI_API_KEY, …). If we swallow load errors and -/// quietly spawn with an empty env, the agent comes up unauthenticated and -/// the user sees a downstream "401 from provider" with no obvious cause. -/// Better to surface the persona load failure at spawn/deploy/discovery -/// time. -pub(crate) fn resolve_persona_env( - app: &tauri::AppHandle, - persona_id: Option<&str>, -) -> Result, String> { - let Some(pid) = persona_id else { - return Ok(BTreeMap::new()); - }; - let personas = super::load_personas(app).map_err(|e| { - format!("failed to load personas while resolving env for persona `{pid}`: {e}") - })?; - let persona = personas - .into_iter() - .find(|p| p.id == pid) - .ok_or_else(|| format!("persona `{pid}` not found while resolving env"))?; - Ok(persona.env_vars) -} - #[cfg(test)] mod tests; diff --git a/desktop/src-tauri/src/managed_agents/persona_card.rs b/desktop/src-tauri/src/managed_agents/persona_card.rs index 779adb21f..4668a3636 100644 --- a/desktop/src-tauri/src/managed_agents/persona_card.rs +++ b/desktop/src-tauri/src/managed_agents/persona_card.rs @@ -9,6 +9,7 @@ pub struct ParsedPersonaPreview { pub display_name: String, pub system_prompt: String, pub avatar_data_url: Option, + pub avatar_ref: Option, pub runtime: Option, pub model: Option, pub provider: Option, @@ -69,6 +70,7 @@ pub fn parse_png_persona(png_bytes: &[u8]) -> Result Result Result display_name: config.display_name, system_prompt: config.prompt, avatar_data_url: None, // .persona.md avatars are paths, not data URIs + avatar_ref: config.avatar, runtime: config.runtime, model, provider: None, // .persona.md format does not carry llmProvider @@ -385,6 +389,7 @@ pub fn parse_zip_pack(zip_bytes: &[u8]) -> Result, - personas: &[crate::managed_agents::types::PersonaRecord], - record_prompt: Option, - record_model: Option, -) -> (Option, Option, Option) { - match persona_id.and_then(|pid| personas.iter().find(|p| p.id == pid)) { - Some(p) => ( - Some(p.system_prompt.clone()), - p.model.clone(), - p.provider.clone(), - ), - None => (record_prompt, record_model, None), - } -} - /// Spawn an agent process without holding any locks on records or runtimes. /// Returns the child process and log path on success. The caller is responsible /// for updating `ManagedAgentRecord` fields and inserting into the runtimes map. @@ -1872,7 +1850,7 @@ fn child_rust_log_filter() -> String { /// Databricks host/model baked in at compile time for internal builds. Empty /// in OSS builds, where the `BUZZ_BUILD_DATABRICKS_*` env is unset. -fn build_databricks_defaults() -> Vec<(&'static str, &'static str)> { +pub(crate) fn build_databricks_defaults() -> Vec<(&'static str, &'static str)> { let mut defaults = Vec::new(); if let Some(host) = option_env!("BUZZ_DESKTOP_BUILD_DATABRICKS_HOST") { if !host.is_empty() { @@ -2007,7 +1985,7 @@ pub fn stop_managed_agent_process( /// switching need the initial bootstrap value. Provider injection is skipped /// when `provider_locked` is true (e.g. Claude runtimes that only work with /// Anthropic). -fn runtime_metadata_env_vars<'a>( +pub(crate) fn runtime_metadata_env_vars<'a>( model_env_var: Option<&'a str>, provider_env_var: Option<&'a str>, provider_locked: bool, diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 0060b9e35..ce0d130bc 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -253,11 +253,7 @@ fn build_env_rejects_empty_allowlist_in_allowlist_mode() { assert!(err.contains("at least one pubkey")); } -// ── resolve_effective_prompt_model_provider tests ─────────────────── - -fn persona(id: &str, prompt: &str, model: Option<&str>) -> crate::managed_agents::PersonaRecord { - persona_with_provider(id, prompt, model, None) -} +// ── persona fixture helpers ───────────────────────────────────────── fn persona_with_provider( id: &str, @@ -284,66 +280,6 @@ fn persona_with_provider( } } -#[test] -fn linked_persona_wins_over_record_snapshot() { - let personas = vec![persona_with_provider( - "p1", - "fresh", - Some("m-fresh"), - Some("anthropic"), - )]; - let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( - Some("p1"), - &personas, - Some("stale".into()), - Some("m-stale".into()), - ); - assert_eq!(prompt.as_deref(), Some("fresh")); - assert_eq!(model.as_deref(), Some("m-fresh")); - assert_eq!(provider.as_deref(), Some("anthropic")); -} - -#[test] -fn no_persona_id_falls_back_to_record() { - let personas = vec![persona("p1", "fresh", Some("m-fresh"))]; - let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( - None, - &personas, - Some("record".into()), - Some("m-record".into()), - ); - assert_eq!(prompt.as_deref(), Some("record")); - assert_eq!(model.as_deref(), Some("m-record")); - assert_eq!(provider, None); -} - -#[test] -fn deleted_persona_falls_back_to_record() { - let personas = vec![persona("p1", "fresh", None)]; - let (prompt, model, provider) = super::resolve_effective_prompt_model_provider( - Some("gone"), - &personas, - Some("record".into()), - Some("m-record".into()), - ); - assert_eq!(prompt.as_deref(), Some("record")); - assert_eq!(model.as_deref(), Some("m-record")); - assert_eq!(provider, None); -} - -#[test] -fn persona_with_no_model_clears_stale_record_model() { - let personas = vec![persona("p1", "fresh", None)]; - let (prompt, model, _provider) = super::resolve_effective_prompt_model_provider( - Some("p1"), - &personas, - Some("stale".into()), - Some("m-stale".into()), - ); - assert_eq!(prompt.as_deref(), Some("fresh")); - assert_eq!(model, None); -} - // ── persona pin/refresh acceptance (Phase 4) ──────────────────────────── // // The full lifecycle Will specified: create from P0, edit P0→P1 (env_vars diff --git a/desktop/src-tauri/src/managed_agents/types.rs b/desktop/src-tauri/src/managed_agents/types.rs index 0e777ec0d..3ff56fcc4 100644 --- a/desktop/src-tauri/src/managed_agents/types.rs +++ b/desktop/src-tauri/src/managed_agents/types.rs @@ -310,11 +310,10 @@ pub struct CreateManagedAgentRequest { pub relay_url: Option, pub acp_command: Option, pub agent_command: Option, - /// True when `agent_command` is a runtime the user deliberately picked to - /// override the linked persona (a deploy-dialog runtime selector). Distinguishes - /// a real pin from a missing-runtime fallback so a persona-backed create only - /// stores an `agent_command_override` for the former. Defaults `false`: callers - /// that don't set it (persona-less creates, fallback divergence) inherit. + /// True when `agent_command` is a runtime command the user deliberately + /// picked for a linked persona. Distinguishes a real selection, including an + /// installed alias, from a missing-runtime fallback so a persona-backed + /// create only stores an `agent_command_override` for the former. #[serde(default)] pub harness_override: bool, #[serde(default)] diff --git a/desktop/src/features/agents/ui/AgentCreationPreview.tsx b/desktop/src/features/agents/ui/AgentCreationPreview.tsx new file mode 100644 index 000000000..6446a18a4 --- /dev/null +++ b/desktop/src/features/agents/ui/AgentCreationPreview.tsx @@ -0,0 +1,358 @@ +import * as React from "react"; +import { Pencil, Plus } from "lucide-react"; +import { AnimatePresence, motion, useReducedMotion } from "motion/react"; + +import { ProfileAvatar } from "@/features/profile/ui/ProfileAvatar"; +import { useAvatarUpload } from "@/features/profile/useAvatarUpload"; +import { cn } from "@/shared/lib/cn"; +import { Button } from "@/shared/ui/button"; +import { Input } from "@/shared/ui/input"; +import { Popover, PopoverContent, PopoverTrigger } from "@/shared/ui/popover"; +import { Spinner } from "@/shared/ui/spinner"; + +function isAvatarFileDrag(event: React.DragEvent) { + return Array.from(event.dataTransfer.types).includes("Files"); +} + +const AVATAR_APPLY_MOTION_TRANSITION = { + duration: 0.14, + ease: [0.23, 1, 0.32, 1], +} as const; + +export function AgentCreationPreview({ + avatarUrl, + disabled = false, + label, + onClearAvatar, + onUploadPendingChange, + onSelectAvatar, +}: { + avatarUrl: string | null; + disabled?: boolean; + label: string; + onClearAvatar?: () => void; + onUploadPendingChange?: (isPending: boolean) => void; + onSelectAvatar: (avatarUrl: string) => void; +}) { + const avatarEditClipId = React.useId().replace(/:/g, ""); + const [isDragOverAvatar, setIsDragOverAvatar] = React.useState(false); + const [isAvatarMenuOpen, setIsAvatarMenuOpen] = React.useState(false); + const [avatarUrlDraft, setAvatarUrlDraft] = React.useState(""); + const [isAvatarUrlInputFocused, setIsAvatarUrlInputFocused] = + React.useState(false); + const avatarDragDepthRef = React.useRef(0); + const shouldReduceMotion = useReducedMotion(); + const { + inputRef: avatarUploadInputRef, + isUploading, + errorMessage: uploadErrorMessage, + clearError: clearUploadError, + openPicker: openUploadPicker, + uploadFile: uploadAvatarFile, + handleFileChange: handleAvatarUploadFileChange, + } = useAvatarUpload({ + onUploadSuccess: onSelectAvatar, + }); + + React.useEffect(() => { + onUploadPendingChange?.(isUploading); + return () => { + onUploadPendingChange?.(false); + }; + }, [isUploading, onUploadPendingChange]); + + React.useEffect(() => { + if (isAvatarMenuOpen) { + setAvatarUrlDraft(""); + setIsAvatarUrlInputFocused(false); + } + }, [isAvatarMenuOpen]); + + function applyAvatarUrl() { + const nextUrl = avatarUrlDraft.trim(); + if (nextUrl.length === 0) { + return; + } + clearUploadError(); + onSelectAvatar(nextUrl); + setIsAvatarMenuOpen(false); + } + + const avatarClipStyle = React.useMemo( + () => ({ + clipPath: `url(#${avatarEditClipId})`, + transform: "translateZ(0)", + }), + [avatarEditClipId], + ); + const hasAvatarUrlDraft = avatarUrlDraft.trim().length > 0; + const hasAvatar = (avatarUrl?.trim().length ?? 0) > 0; + const applyButtonTransition = shouldReduceMotion + ? { duration: 0 } + : AVATAR_APPLY_MOTION_TRANSITION; + + const handleAvatarDragEnter = React.useCallback( + (event: React.DragEvent) => { + if (disabled || !isAvatarFileDrag(event)) { + return; + } + event.preventDefault(); + event.stopPropagation(); + avatarDragDepthRef.current += 1; + event.dataTransfer.dropEffect = "copy"; + setIsDragOverAvatar(true); + }, + [disabled], + ); + + const handleAvatarDragOver = React.useCallback( + (event: React.DragEvent) => { + if (disabled || !isAvatarFileDrag(event)) { + return; + } + event.preventDefault(); + event.stopPropagation(); + event.dataTransfer.dropEffect = "copy"; + setIsDragOverAvatar(true); + }, + [disabled], + ); + + const handleAvatarDragLeave = React.useCallback( + (event: React.DragEvent) => { + if (!isAvatarFileDrag(event)) { + return; + } + event.preventDefault(); + event.stopPropagation(); + avatarDragDepthRef.current = Math.max(0, avatarDragDepthRef.current - 1); + if (avatarDragDepthRef.current === 0) { + setIsDragOverAvatar(false); + } + }, + [], + ); + + const handleAvatarDrop = React.useCallback( + (event: React.DragEvent) => { + if (!isAvatarFileDrag(event)) { + return; + } + event.preventDefault(); + event.stopPropagation(); + avatarDragDepthRef.current = 0; + setIsDragOverAvatar(false); + + const file = event.dataTransfer.files[0]; + if (!file || disabled || isUploading) { + return; + } + + clearUploadError(); + void uploadAvatarFile(file); + }, + [clearUploadError, disabled, isUploading, uploadAvatarFile], + ); + + const avatarMenuContent = ( + + +
{ + event.preventDefault(); + event.stopPropagation(); + applyAvatarUrl(); + }} + > + + setIsAvatarUrlInputFocused(false)} + onChange={(event) => setAvatarUrlDraft(event.target.value)} + onFocus={() => setIsAvatarUrlInputFocused(true)} + placeholder={isAvatarUrlInputFocused ? "https://..." : "Use a URL"} + spellCheck={false} + value={avatarUrlDraft} + /> + + {hasAvatarUrlDraft ? ( + + + + ) : null} + +
+ {hasAvatar && onClearAvatar ? ( + + ) : null} + + ); + + return ( +
+
+ + +
+ {hasAvatar ? ( + <> + + +
+ +
+ +
+ + + + + {avatarMenuContent} + +
+ + ) : ( + + + + + {avatarMenuContent} + + )} +
+ + {uploadErrorMessage ? ( +

+ {uploadErrorMessage} +

+ ) : null} +
+
+ ); +} diff --git a/desktop/src/features/agents/ui/AgentsView.tsx b/desktop/src/features/agents/ui/AgentsView.tsx index 7305a9987..5f17957dc 100644 --- a/desktop/src/features/agents/ui/AgentsView.tsx +++ b/desktop/src/features/agents/ui/AgentsView.tsx @@ -192,6 +192,16 @@ export function AgentsView() { }} /> ) : null} + {personas.createdAgent ? ( + { + if (!open) { + personas.setCreatedAgent(null); + } + }} + /> + ) : null} {personas.personaDialogState ? ( { @@ -143,6 +147,7 @@ export function BatchImportDialog({ .trim() .split("\n") .find((line) => line.trim().length > 0); + const avatarUrl = importedAvatarUrl(persona); return (
e.stopPropagation()} /> diff --git a/desktop/src/features/agents/ui/EnvVarsEditor.tsx b/desktop/src/features/agents/ui/EnvVarsEditor.tsx index 2bfeeb649..fa4cc543e 100644 --- a/desktop/src/features/agents/ui/EnvVarsEditor.tsx +++ b/desktop/src/features/agents/ui/EnvVarsEditor.tsx @@ -3,6 +3,11 @@ import * as React from "react"; import { Button } from "@/shared/ui/button"; import { Input } from "@/shared/ui/input"; +import { cn } from "@/shared/lib/cn"; +import { + PERSONA_FIELD_CONTROL_CLASS, + PERSONA_FIELD_SHELL_CLASS, +} from "./personaDialogPickers"; export type EnvVarsValue = Record; @@ -98,28 +103,48 @@ export function EnvVarsEditor({ return (
- - updateRow(row.id, { key: event.target.value }) - } - placeholder="ANTHROPIC_API_KEY" - value={row.key} - /> - - updateRow(row.id, { value: event.target.value }) - } - placeholder="sk-ant-..." - value={row.value} - /> +
+ + updateRow(row.id, { key: event.target.value }) + } + placeholder="VARIABLE_NAME" + value={row.key} + /> +
+
+ + updateRow(row.id, { value: event.target.value }) + } + placeholder="value" + value={row.value} + /> +