diff --git a/crates/agent_ui/src/agent_ui.rs b/crates/agent_ui/src/agent_ui.rs index 9e5d6cbe0d5ac0b07ee4326a461ab91b5b2064b2..4a5382c9e4d67e483fbfd96f456187638829da87 100644 --- a/crates/agent_ui/src/agent_ui.rs +++ b/crates/agent_ui/src/agent_ui.rs @@ -371,26 +371,49 @@ fn update_active_language_model_from_settings(cx: &mut App) { } } - let default = settings.default_model.as_ref().map(to_selected_model); + // Filter out models from providers that are not authenticated + fn is_provider_authenticated( + selection: &LanguageModelSelection, + registry: &LanguageModelRegistry, + cx: &App, + ) -> bool { + let provider_id = LanguageModelProviderId::from(selection.provider.0.clone()); + registry + .provider(&provider_id) + .map_or(false, |provider| provider.is_authenticated(cx)) + } + + let registry = LanguageModelRegistry::global(cx); + let registry_ref = registry.read(cx); + + let default = settings + .default_model + .as_ref() + .filter(|s| is_provider_authenticated(s, registry_ref, cx)) + .map(to_selected_model); let inline_assistant = settings .inline_assistant_model .as_ref() + .filter(|s| is_provider_authenticated(s, registry_ref, cx)) .map(to_selected_model); let commit_message = settings .commit_message_model .as_ref() + .filter(|s| is_provider_authenticated(s, registry_ref, cx)) .map(to_selected_model); let thread_summary = settings .thread_summary_model .as_ref() + .filter(|s| is_provider_authenticated(s, registry_ref, cx)) .map(to_selected_model); let inline_alternatives = settings .inline_alternatives .iter() + .filter(|s| is_provider_authenticated(s, registry_ref, cx)) .map(to_selected_model) .collect::>(); - LanguageModelRegistry::global(cx).update(cx, |registry, cx| { + registry.update(cx, |registry, cx| { registry.select_default_model(default.as_ref(), cx); registry.select_inline_assistant_model(inline_assistant.as_ref(), cx); registry.select_commit_message_model(commit_message.as_ref(), cx); diff --git a/crates/extension_host/src/extension_host.rs b/crates/extension_host/src/extension_host.rs index 3b2f1586a62ea854464bc2e73618ebdf815807f6..ea6d52418fe693572107445272f7ea7b658e132d 100644 --- a/crates/extension_host/src/extension_host.rs +++ b/crates/extension_host/src/extension_host.rs @@ -82,7 +82,7 @@ const FS_WATCH_LATENCY: Duration = Duration::from_millis(100); /// Extension IDs that are being migrated from hardcoded LLM providers. /// For backwards compatibility, if the user has the corresponding env var set, -/// we automatically enable env var reading for these extensions. +/// we automatically enable env var reading for these extensions on first install. const LEGACY_LLM_EXTENSION_IDS: &[&str] = &[ "anthropic", "copilot_chat", @@ -93,6 +93,9 @@ const LEGACY_LLM_EXTENSION_IDS: &[&str] = &[ /// Migrates legacy LLM provider extensions by auto-enabling env var reading /// if the env var is currently present in the environment. +/// +/// This migration only runs once per provider - we track which providers have been +/// migrated in `migrated_llm_providers` to avoid overriding user preferences. fn migrate_legacy_llm_provider_env_var(manifest: &ExtensionManifest, cx: &mut App) { // Only apply migration to known legacy LLM extensions if !LEGACY_LLM_EXTENSION_IDS.contains(&manifest.id.as_ref()) { @@ -108,49 +111,64 @@ fn migrate_legacy_llm_provider_env_var(manifest: &ExtensionManifest, cx: &mut Ap continue; }; - // Check if the env var is present and non-empty - let env_var_is_set = std::env::var(env_var_name) - .map(|v| !v.is_empty()) - .unwrap_or(false); - - if !env_var_is_set { - continue; - } - let full_provider_id: Arc = format!("{}:{}", manifest.id, provider_id).into(); - // Check if already in settings - let already_allowed = ExtensionSettings::get_global(cx) - .allowed_env_var_providers + // Check if we've already run migration for this provider (regardless of outcome) + let already_migrated = ExtensionSettings::get_global(cx) + .migrated_llm_providers .contains(full_provider_id.as_ref()); - if already_allowed { + if already_migrated { continue; } - // Auto-enable env var reading for this provider - log::info!( - "Migrating legacy LLM provider {}: auto-enabling {} env var reading", - full_provider_id, - env_var_name - ); + // Check if the env var is present and non-empty + let env_var_is_set = std::env::var(env_var_name) + .map(|v| !v.is_empty()) + .unwrap_or(false); + // Mark as migrated regardless of whether we enable env var reading settings::update_settings_file(::global(cx), cx, { let full_provider_id = full_provider_id.clone(); + let env_var_is_set = env_var_is_set; move |settings, _| { - let providers = settings + // Always mark as migrated + let migrated = settings .extension - .allowed_env_var_providers + .migrated_llm_providers .get_or_insert_with(Vec::new); - if !providers + if !migrated .iter() .any(|id| id.as_ref() == full_provider_id.as_ref()) { - providers.push(full_provider_id); + migrated.push(full_provider_id.clone()); + } + + // Only enable env var reading if the env var is set + if env_var_is_set { + let providers = settings + .extension + .allowed_env_var_providers + .get_or_insert_with(Vec::new); + + if !providers + .iter() + .any(|id| id.as_ref() == full_provider_id.as_ref()) + { + providers.push(full_provider_id); + } } } }); + + if env_var_is_set { + log::info!( + "Migrating legacy LLM provider {}: auto-enabling {} env var reading", + full_provider_id, + env_var_name + ); + } } } diff --git a/crates/extension_host/src/extension_settings.rs b/crates/extension_host/src/extension_settings.rs index 3322ea4068cc08ef8f5257e670ad8cb7088b57b7..36777cb1727c95082b6a6e403ece1acb9e490729 100644 --- a/crates/extension_host/src/extension_settings.rs +++ b/crates/extension_host/src/extension_settings.rs @@ -20,6 +20,9 @@ pub struct ExtensionSettings { /// from environment variables. Each entry is a provider ID in the format /// "extension_id:provider_id". pub allowed_env_var_providers: HashSet>, + /// Tracks which legacy LLM providers have been migrated. + /// This prevents the migration from running multiple times and overriding user preferences. + pub migrated_llm_providers: HashSet>, } impl ExtensionSettings { @@ -71,6 +74,13 @@ impl Settings for ExtensionSettings { .unwrap_or_default() .into_iter() .collect(), + migrated_llm_providers: content + .extension + .migrated_llm_providers + .clone() + .unwrap_or_default() + .into_iter() + .collect(), } } } diff --git a/crates/extension_host/src/wasm_host.rs b/crates/extension_host/src/wasm_host.rs index cfd75f7a10fce04076350bf4d3f3ad7e42d21899..5194cafec2601dd2ef17a2e6744488c2326a5f15 100644 --- a/crates/extension_host/src/wasm_host.rs +++ b/crates/extension_host/src/wasm_host.rs @@ -5,7 +5,7 @@ use crate::capability_granter::CapabilityGranter; use crate::{ExtensionManifest, ExtensionSettings}; use anyhow::{Context as _, Result, anyhow, bail}; use async_trait::async_trait; -use collections::HashSet; + use dap::{DebugRequest, StartDebuggingRequestArgumentsRequest}; use extension::{ CodeLabel, Command, Completion, ContextServerConfiguration, DebugAdapterBinary, @@ -60,8 +60,6 @@ pub struct WasmHost { pub work_dir: PathBuf, /// The capabilities granted to extensions running on the host. pub(crate) granted_capabilities: Vec, - /// Extension LLM providers allowed to read API keys from environment variables. - pub(crate) allowed_env_var_providers: HashSet>, _main_thread_message_task: Task<()>, main_thread_message_tx: mpsc::UnboundedSender, } @@ -597,7 +595,6 @@ impl WasmHost { proxy, release_channel: ReleaseChannel::global(cx), granted_capabilities: extension_settings.granted_capabilities.clone(), - allowed_env_var_providers: extension_settings.allowed_env_var_providers.clone(), _main_thread_message_task: task, main_thread_message_tx: tx, }) diff --git a/crates/extension_host/src/wasm_host/llm_provider.rs b/crates/extension_host/src/wasm_host/llm_provider.rs index 3f16fb31cd11d5b12ec076e67dfbcfa60feff93e..b9650d5715909a53ee0de5673ceb3a84b7397bbc 100644 --- a/crates/extension_host/src/wasm_host/llm_provider.rs +++ b/crates/extension_host/src/wasm_host/llm_provider.rs @@ -495,6 +495,20 @@ impl ExtensionProviderConfigurationView { cx.notify(); }); + // If env var is being enabled, clear any stored keychain credentials + // so there's only one source of truth for the API key + if new_allowed { + let credential_key = self.credential_key.clone(); + let credentials_provider = ::global(cx); + cx.spawn(async move |_this, cx| { + credentials_provider + .delete_credentials(&credential_key, cx) + .await + .log_err(); + }) + .detach(); + } + // If env var is being disabled, reload credentials from keychain if !new_allowed { self.reload_keychain_credentials(cx); diff --git a/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs b/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs index 1b6d5c4d4ea9644b9a3d27f703c95874389e12fa..82a662db9a28d8583534818fb5836aee632516d5 100644 --- a/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs +++ b/crates/extension_host/src/wasm_host/wit/since_v0_8_0.rs @@ -1139,11 +1139,23 @@ impl llm_provider::Host for WasmState { if let Some(env_var_name) = env_var_name { let full_provider_id: Arc = format!("{}:{}", extension_id, provider_id).into(); - // Use cached settings from WasmHost instead of going to main thread + // Read settings dynamically to get current allowed_env_var_providers let is_allowed = self - .host - .allowed_env_var_providers - .contains(&full_provider_id); + .on_main_thread({ + let full_provider_id = full_provider_id.clone(); + move |cx| { + async move { + cx.update(|cx| { + crate::extension_settings::ExtensionSettings::get_global(cx) + .allowed_env_var_providers + .contains(&full_provider_id) + }) + } + .boxed_local() + } + }) + .await + .unwrap_or(false); if is_allowed { if let Ok(value) = env::var(&env_var_name) { @@ -1240,12 +1252,24 @@ impl llm_provider::Host for WasmState { }; // Check if the user has allowed this provider to read env vars - // Use cached settings from WasmHost instead of going to main thread + // Read settings dynamically to get current allowed_env_var_providers let full_provider_id: Arc = format!("{}:{}", extension_id, provider_id).into(); let is_allowed = self - .host - .allowed_env_var_providers - .contains(&full_provider_id); + .on_main_thread({ + let full_provider_id = full_provider_id.clone(); + move |cx| { + async move { + cx.update(|cx| { + crate::extension_settings::ExtensionSettings::get_global(cx) + .allowed_env_var_providers + .contains(&full_provider_id) + }) + } + .boxed_local() + } + }) + .await + .unwrap_or(false); if !is_allowed { log::debug!( diff --git a/crates/settings/src/settings_content/extension.rs b/crates/settings/src/settings_content/extension.rs index 64df163f4ec961cf6bfc469c18ac0f8884c39f0b..b405103e8c311dbd5c83a100002bae7689d11591 100644 --- a/crates/settings/src/settings_content/extension.rs +++ b/crates/settings/src/settings_content/extension.rs @@ -26,6 +26,9 @@ pub struct ExtensionSettingsContent { /// /// Default: [] pub allowed_env_var_providers: Option>>, + /// Tracks which legacy LLM providers have been migrated. This is an internal + /// setting used to prevent the migration from running multiple times. + pub migrated_llm_providers: Option>>, } /// A capability for an extension.