Detailed changes
@@ -1,11 +1,14 @@
use credentials_provider::CredentialsProvider;
use gpui::App;
-use util::ResultExt as _;
const ANTHROPIC_EXTENSION_ID: &str = "anthropic";
const ANTHROPIC_PROVIDER_ID: &str = "anthropic";
const ANTHROPIC_DEFAULT_API_URL: &str = "https://api.anthropic.com";
+/// Migrates Anthropic API credentials from the old built-in provider location
+/// to the new extension-based location.
+///
+/// This should only be called during auto-install of the extension.
pub fn migrate_anthropic_credentials_if_needed(extension_id: &str, cx: &mut App) {
if extension_id != ANTHROPIC_EXTENSION_ID {
return;
@@ -19,17 +22,7 @@ pub fn migrate_anthropic_credentials_if_needed(extension_id: &str, cx: &mut App)
let credentials_provider = <dyn CredentialsProvider>::global(cx);
cx.spawn(async move |cx| {
- let existing_credential = credentials_provider
- .read_credentials(&extension_credential_key, &cx)
- .await
- .ok()
- .flatten();
-
- if existing_credential.is_some() {
- log::debug!("Anthropic extension already has credentials, skipping migration");
- return;
- }
-
+ // Read from old location
let old_credential = credentials_provider
.read_credentials(ANTHROPIC_DEFAULT_API_URL, &cx)
.await
@@ -40,8 +33,8 @@ pub fn migrate_anthropic_credentials_if_needed(extension_id: &str, cx: &mut App)
Some((_, key_bytes)) => match String::from_utf8(key_bytes) {
Ok(key) if !key.is_empty() => key,
Ok(_) => {
- log::debug!("Existing Anthropic API key is empty, marking as migrated");
- String::new()
+ log::debug!("Existing Anthropic API key is empty, nothing to migrate");
+ return;
}
Err(_) => {
log::error!("Failed to decode Anthropic API key as UTF-8");
@@ -49,20 +42,11 @@ pub fn migrate_anthropic_credentials_if_needed(extension_id: &str, cx: &mut App)
}
},
None => {
- log::debug!("No existing Anthropic API key found, marking as migrated");
- String::new()
+ log::debug!("No existing Anthropic API key found to migrate");
+ return;
}
};
- if api_key.is_empty() {
- // Write empty credentials as a marker that migration was attempted
- credentials_provider
- .write_credentials(&extension_credential_key, "Bearer", b"", &cx)
- .await
- .log_err();
- return;
- }
-
log::info!("Migrating existing Anthropic API key to Anthropic extension");
match credentials_provider
@@ -105,41 +89,7 @@ mod tests {
}
#[gpui::test]
- async fn test_skips_migration_if_extension_already_has_credentials(cx: &mut TestAppContext) {
- let old_api_key = "sk-ant-old-key";
- let existing_key = "sk-ant-existing-key";
-
- cx.write_credentials(ANTHROPIC_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- cx.write_credentials(
- "extension-llm-anthropic:anthropic",
- "Bearer",
- existing_key.as_bytes(),
- );
-
- cx.update(|cx| {
- migrate_anthropic_credentials_if_needed(ANTHROPIC_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-anthropic:anthropic");
- let (_, password) = credentials.unwrap();
- assert_eq!(
- String::from_utf8(password).unwrap(),
- existing_key,
- "Should not overwrite existing credentials"
- );
- }
-
- #[gpui::test]
- async fn test_skips_migration_if_empty_marker_exists(cx: &mut TestAppContext) {
- let old_api_key = "sk-ant-old-key";
-
- // Old credentials exist
- cx.write_credentials(ANTHROPIC_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- // But empty marker already exists (from previous migration attempt)
- cx.write_credentials("extension-llm-anthropic:anthropic", "Bearer", b"");
-
+ async fn test_no_migration_if_no_old_credentials(cx: &mut TestAppContext) {
cx.update(|cx| {
migrate_anthropic_credentials_if_needed(ANTHROPIC_EXTENSION_ID, cx);
});
@@ -147,29 +97,10 @@ mod tests {
cx.run_until_parked();
let credentials = cx.read_credentials("extension-llm-anthropic:anthropic");
- let (_, password) = credentials.unwrap();
assert!(
- password.is_empty(),
- "Should not overwrite empty marker with old credentials"
- );
- }
-
- #[gpui::test]
- async fn test_writes_empty_marker_if_no_old_credentials(cx: &mut TestAppContext) {
- cx.update(|cx| {
- migrate_anthropic_credentials_if_needed(ANTHROPIC_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-anthropic:anthropic");
- assert!(
- credentials.is_some(),
- "Should write empty credentials as migration marker"
+ credentials.is_none(),
+ "Should not create credentials if none existed"
);
- let (username, password) = credentials.unwrap();
- assert_eq!(username, "Bearer");
- assert!(password.is_empty(), "Password should be empty marker");
}
#[gpui::test]
@@ -1,11 +1,14 @@
use credentials_provider::CredentialsProvider;
use gpui::App;
use std::path::PathBuf;
-use util::ResultExt as _;
const COPILOT_CHAT_EXTENSION_ID: &str = "copilot-chat";
const COPILOT_CHAT_PROVIDER_ID: &str = "copilot-chat";
+/// Migrates Copilot OAuth credentials from the GitHub Copilot config files
+/// to the new extension-based credential location.
+///
+/// This should only be called during auto-install of the extension.
pub fn migrate_copilot_credentials_if_needed(extension_id: &str, cx: &mut App) {
if extension_id != COPILOT_CHAT_EXTENSION_ID {
return;
@@ -18,27 +21,12 @@ pub fn migrate_copilot_credentials_if_needed(extension_id: &str, cx: &mut App) {
let credentials_provider = <dyn CredentialsProvider>::global(cx);
- cx.spawn(async move |cx| {
- let existing_credential = credentials_provider
- .read_credentials(&credential_key, &cx)
- .await
- .ok()
- .flatten();
-
- if existing_credential.is_some() {
- log::debug!("Copilot Chat extension already has credentials, skipping migration");
- return;
- }
-
+ cx.spawn(async move |_cx| {
+ // Read from copilot config files
let oauth_token = match read_copilot_oauth_token().await {
Some(token) if !token.is_empty() => token,
_ => {
- log::debug!("No existing Copilot OAuth token found, marking as migrated");
- // Write empty credentials as a marker that migration was attempted
- credentials_provider
- .write_credentials(&credential_key, "api_key", b"", &cx)
- .await
- .log_err();
+ log::debug!("No existing Copilot OAuth token found to migrate");
return;
}
};
@@ -46,7 +34,7 @@ pub fn migrate_copilot_credentials_if_needed(extension_id: &str, cx: &mut App) {
log::info!("Migrating existing Copilot OAuth token to Copilot Chat extension");
match credentials_provider
- .write_credentials(&credential_key, "api_key", oauth_token.as_bytes(), &cx)
+ .write_credentials(&credential_key, "api_key", oauth_token.as_bytes(), &_cx)
.await
{
Ok(()) => {
@@ -193,31 +181,6 @@ mod tests {
assert_eq!(token, Some("github_token".to_string()));
}
- #[gpui::test]
- async fn test_skips_migration_if_extension_already_has_credentials(cx: &mut TestAppContext) {
- let existing_token = "existing_oauth_token";
-
- cx.write_credentials(
- "extension-llm-copilot-chat:copilot-chat",
- "api_key",
- existing_token.as_bytes(),
- );
-
- cx.update(|cx| {
- migrate_copilot_credentials_if_needed(COPILOT_CHAT_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-copilot-chat:copilot-chat");
- let (_, password) = credentials.unwrap();
- assert_eq!(
- String::from_utf8(password).unwrap(),
- existing_token,
- "Should not overwrite existing credentials"
- );
- }
-
#[gpui::test]
async fn test_skips_migration_for_other_extensions(cx: &mut TestAppContext) {
cx.update(|cx| {
@@ -235,9 +198,7 @@ mod tests {
// Note: Unlike the other migrations, copilot migration reads from the filesystem
// (copilot config files), not from the credentials provider. In tests, these files
- // don't exist, and the smol async filesystem operations don't integrate well with
- // the GPUI test executor's run_until_parked(). So we test the original behavior:
- // no config files = no credentials written.
+ // don't exist, so no migration occurs.
#[gpui::test]
async fn test_no_credentials_when_no_copilot_config_exists(cx: &mut TestAppContext) {
cx.update(|cx| {
@@ -247,11 +208,9 @@ mod tests {
cx.run_until_parked();
let credentials = cx.read_credentials("extension-llm-copilot-chat:copilot-chat");
- // The async task that would write the marker doesn't complete in tests
- // because smol filesystem operations use a different executor
assert!(
credentials.is_none(),
- "No credentials should be written when copilot config doesn't exist (in test environment)"
+ "No credentials should be written when copilot config doesn't exist"
);
}
}
@@ -219,6 +219,8 @@ pub struct ExtensionStore {
pub enum ExtensionOperation {
Upgrade,
Install,
+ /// Auto-install from settings - triggers legacy LLM provider migrations
+ AutoInstall,
Remove,
}
@@ -755,7 +757,7 @@ impl ExtensionStore {
}
this.update(cx, |this, cx| {
- this.install_latest_extension(extension_id.clone(), cx);
+ this.auto_install_latest_extension(extension_id.clone(), cx);
})
.ok();
}
@@ -910,13 +912,11 @@ impl ExtensionStore {
this.update(cx, |this, cx| this.reload(Some(extension_id.clone()), cx))?
.await;
- if let ExtensionOperation::Install = operation {
+ if matches!(
+ operation,
+ ExtensionOperation::Install | ExtensionOperation::AutoInstall
+ ) {
this.update(cx, |this, cx| {
- // Check for legacy LLM provider migration
- if let Some(manifest) = this.extension_manifest_for_id(&extension_id) {
- migrate_legacy_llm_provider_env_var(&manifest, cx);
- }
-
cx.emit(Event::ExtensionInstalled(extension_id.clone()));
if let Some(events) = ExtensionEvents::try_global(cx)
&& let Some(manifest) = this.extension_manifest_for_id(&extension_id)
@@ -926,15 +926,26 @@ impl ExtensionStore {
});
}
- // Run extension-specific migrations
- copilot_migration::migrate_copilot_credentials_if_needed(&extension_id, cx);
- anthropic_migration::migrate_anthropic_credentials_if_needed(&extension_id, cx);
- google_ai_migration::migrate_google_ai_credentials_if_needed(&extension_id, cx);
- openai_migration::migrate_openai_credentials_if_needed(&extension_id, cx);
- open_router_migration::migrate_open_router_credentials_if_needed(
- &extension_id,
- cx,
- );
+ // Run legacy LLM provider migrations only for auto-installed extensions
+ if matches!(operation, ExtensionOperation::AutoInstall) {
+ if let Some(manifest) = this.extension_manifest_for_id(&extension_id) {
+ migrate_legacy_llm_provider_env_var(&manifest, cx);
+ }
+ copilot_migration::migrate_copilot_credentials_if_needed(&extension_id, cx);
+ anthropic_migration::migrate_anthropic_credentials_if_needed(
+ &extension_id,
+ cx,
+ );
+ google_ai_migration::migrate_google_ai_credentials_if_needed(
+ &extension_id,
+ cx,
+ );
+ openai_migration::migrate_openai_credentials_if_needed(&extension_id, cx);
+ open_router_migration::migrate_open_router_credentials_if_needed(
+ &extension_id,
+ cx,
+ );
+ }
})
.ok();
}
@@ -944,6 +955,24 @@ impl ExtensionStore {
}
pub fn install_latest_extension(&mut self, extension_id: Arc<str>, cx: &mut Context<Self>) {
+ self.install_latest_extension_with_operation(extension_id, ExtensionOperation::Install, cx);
+ }
+
+ /// Auto-install an extension, triggering legacy LLM provider migrations.
+ fn auto_install_latest_extension(&mut self, extension_id: Arc<str>, cx: &mut Context<Self>) {
+ self.install_latest_extension_with_operation(
+ extension_id,
+ ExtensionOperation::AutoInstall,
+ cx,
+ );
+ }
+
+ fn install_latest_extension_with_operation(
+ &mut self,
+ extension_id: Arc<str>,
+ operation: ExtensionOperation,
+ cx: &mut Context<Self>,
+ ) {
let schema_versions = schema_version_range();
let wasm_api_versions = wasm_api_version_range(ReleaseChannel::global(cx));
@@ -966,13 +995,8 @@ impl ExtensionStore {
return;
};
- self.install_or_upgrade_extension_at_endpoint(
- extension_id,
- url,
- ExtensionOperation::Install,
- cx,
- )
- .detach_and_log_err(cx);
+ self.install_or_upgrade_extension_at_endpoint(extension_id, url, operation, cx)
+ .detach_and_log_err(cx);
}
pub fn upgrade_extension(
@@ -1171,11 +1195,6 @@ impl ExtensionStore {
this.update(cx, |this, cx| this.reload(None, cx))?.await;
this.update(cx, |this, cx| {
- // Run migration for legacy LLM provider env vars
- if let Some(manifest) = this.extension_manifest_for_id(&extension_id) {
- migrate_legacy_llm_provider_env_var(&manifest, cx);
- }
-
cx.emit(Event::ExtensionInstalled(extension_id.clone()));
if let Some(events) = ExtensionEvents::try_global(cx)
&& let Some(manifest) = this.extension_manifest_for_id(&extension_id)
@@ -1,11 +1,14 @@
use credentials_provider::CredentialsProvider;
use gpui::App;
-use util::ResultExt as _;
const GOOGLE_AI_EXTENSION_ID: &str = "google-ai";
const GOOGLE_AI_PROVIDER_ID: &str = "google-ai";
const GOOGLE_AI_DEFAULT_API_URL: &str = "https://generativelanguage.googleapis.com";
+/// Migrates Google AI API credentials from the old built-in provider location
+/// to the new extension-based location.
+///
+/// This should only be called during auto-install of the extension.
pub fn migrate_google_ai_credentials_if_needed(extension_id: &str, cx: &mut App) {
if extension_id != GOOGLE_AI_EXTENSION_ID {
return;
@@ -19,17 +22,7 @@ pub fn migrate_google_ai_credentials_if_needed(extension_id: &str, cx: &mut App)
let credentials_provider = <dyn CredentialsProvider>::global(cx);
cx.spawn(async move |cx| {
- let existing_credential = credentials_provider
- .read_credentials(&extension_credential_key, &cx)
- .await
- .ok()
- .flatten();
-
- if existing_credential.is_some() {
- log::debug!("Google AI extension already has credentials, skipping migration");
- return;
- }
-
+ // Read from old location
let old_credential = credentials_provider
.read_credentials(GOOGLE_AI_DEFAULT_API_URL, &cx)
.await
@@ -40,8 +33,8 @@ pub fn migrate_google_ai_credentials_if_needed(extension_id: &str, cx: &mut App)
Some((_, key_bytes)) => match String::from_utf8(key_bytes) {
Ok(key) if !key.is_empty() => key,
Ok(_) => {
- log::debug!("Existing Google AI API key is empty, marking as migrated");
- String::new()
+ log::debug!("Existing Google AI API key is empty, nothing to migrate");
+ return;
}
Err(_) => {
log::error!("Failed to decode Google AI API key as UTF-8");
@@ -49,20 +42,11 @@ pub fn migrate_google_ai_credentials_if_needed(extension_id: &str, cx: &mut App)
}
},
None => {
- log::debug!("No existing Google AI API key found, marking as migrated");
- String::new()
+ log::debug!("No existing Google AI API key found to migrate");
+ return;
}
};
- if api_key.is_empty() {
- // Write empty credentials as a marker that migration was attempted
- credentials_provider
- .write_credentials(&extension_credential_key, "Bearer", b"", &cx)
- .await
- .log_err();
- return;
- }
-
log::info!("Migrating existing Google AI API key to Google AI extension");
match credentials_provider
@@ -105,41 +89,7 @@ mod tests {
}
#[gpui::test]
- async fn test_skips_migration_if_extension_already_has_credentials(cx: &mut TestAppContext) {
- let old_api_key = "AIzaSy-old-key";
- let existing_key = "AIzaSy-existing-key";
-
- cx.write_credentials(GOOGLE_AI_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- cx.write_credentials(
- "extension-llm-google-ai:google-ai",
- "Bearer",
- existing_key.as_bytes(),
- );
-
- cx.update(|cx| {
- migrate_google_ai_credentials_if_needed(GOOGLE_AI_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-google-ai:google-ai");
- let (_, password) = credentials.unwrap();
- assert_eq!(
- String::from_utf8(password).unwrap(),
- existing_key,
- "Should not overwrite existing credentials"
- );
- }
-
- #[gpui::test]
- async fn test_skips_migration_if_empty_marker_exists(cx: &mut TestAppContext) {
- let old_api_key = "AIzaSy-old-key";
-
- // Old credentials exist
- cx.write_credentials(GOOGLE_AI_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- // But empty marker already exists (from previous migration attempt)
- cx.write_credentials("extension-llm-google-ai:google-ai", "Bearer", b"");
-
+ async fn test_no_migration_if_no_old_credentials(cx: &mut TestAppContext) {
cx.update(|cx| {
migrate_google_ai_credentials_if_needed(GOOGLE_AI_EXTENSION_ID, cx);
});
@@ -147,29 +97,10 @@ mod tests {
cx.run_until_parked();
let credentials = cx.read_credentials("extension-llm-google-ai:google-ai");
- let (_, password) = credentials.unwrap();
assert!(
- password.is_empty(),
- "Should not overwrite empty marker with old credentials"
- );
- }
-
- #[gpui::test]
- async fn test_writes_empty_marker_if_no_old_credentials(cx: &mut TestAppContext) {
- cx.update(|cx| {
- migrate_google_ai_credentials_if_needed(GOOGLE_AI_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-google-ai:google-ai");
- assert!(
- credentials.is_some(),
- "Should write empty credentials as migration marker"
+ credentials.is_none(),
+ "Should not create credentials if none existed"
);
- let (username, password) = credentials.unwrap();
- assert_eq!(username, "Bearer");
- assert!(password.is_empty(), "Password should be empty marker");
}
#[gpui::test]
@@ -1,11 +1,14 @@
use credentials_provider::CredentialsProvider;
use gpui::App;
-use util::ResultExt as _;
const OPEN_ROUTER_EXTENSION_ID: &str = "openrouter";
const OPEN_ROUTER_PROVIDER_ID: &str = "openrouter";
const OPEN_ROUTER_DEFAULT_API_URL: &str = "https://openrouter.ai/api/v1";
+/// Migrates OpenRouter API credentials from the old built-in provider location
+/// to the new extension-based location.
+///
+/// This should only be called during auto-install of the extension.
pub fn migrate_open_router_credentials_if_needed(extension_id: &str, cx: &mut App) {
if extension_id != OPEN_ROUTER_EXTENSION_ID {
return;
@@ -19,17 +22,7 @@ pub fn migrate_open_router_credentials_if_needed(extension_id: &str, cx: &mut Ap
let credentials_provider = <dyn CredentialsProvider>::global(cx);
cx.spawn(async move |cx| {
- let existing_credential = credentials_provider
- .read_credentials(&extension_credential_key, &cx)
- .await
- .ok()
- .flatten();
-
- if existing_credential.is_some() {
- log::debug!("OpenRouter extension already has credentials, skipping migration");
- return;
- }
-
+ // Read from old location
let old_credential = credentials_provider
.read_credentials(OPEN_ROUTER_DEFAULT_API_URL, &cx)
.await
@@ -40,8 +33,8 @@ pub fn migrate_open_router_credentials_if_needed(extension_id: &str, cx: &mut Ap
Some((_, key_bytes)) => match String::from_utf8(key_bytes) {
Ok(key) if !key.is_empty() => key,
Ok(_) => {
- log::debug!("Existing OpenRouter API key is empty, marking as migrated");
- String::new()
+ log::debug!("Existing OpenRouter API key is empty, nothing to migrate");
+ return;
}
Err(_) => {
log::error!("Failed to decode OpenRouter API key as UTF-8");
@@ -49,20 +42,11 @@ pub fn migrate_open_router_credentials_if_needed(extension_id: &str, cx: &mut Ap
}
},
None => {
- log::debug!("No existing OpenRouter API key found, marking as migrated");
- String::new()
+ log::debug!("No existing OpenRouter API key found to migrate");
+ return;
}
};
- if api_key.is_empty() {
- // Write empty credentials as a marker that migration was attempted
- credentials_provider
- .write_credentials(&extension_credential_key, "Bearer", b"", &cx)
- .await
- .log_err();
- return;
- }
-
log::info!("Migrating existing OpenRouter API key to OpenRouter extension");
match credentials_provider
@@ -105,21 +89,7 @@ mod tests {
}
#[gpui::test]
- async fn test_skips_migration_if_extension_already_has_credentials(cx: &mut TestAppContext) {
- let old_api_key = "sk-or-old-key";
- let existing_key = "sk-or-existing-key";
-
- cx.write_credentials(
- OPEN_ROUTER_DEFAULT_API_URL,
- "Bearer",
- old_api_key.as_bytes(),
- );
- cx.write_credentials(
- "extension-llm-openrouter:openrouter",
- "Bearer",
- existing_key.as_bytes(),
- );
-
+ async fn test_no_migration_if_no_old_credentials(cx: &mut TestAppContext) {
cx.update(|cx| {
migrate_open_router_credentials_if_needed(OPEN_ROUTER_EXTENSION_ID, cx);
});
@@ -127,57 +97,10 @@ mod tests {
cx.run_until_parked();
let credentials = cx.read_credentials("extension-llm-openrouter:openrouter");
- let (_, password) = credentials.unwrap();
- assert_eq!(
- String::from_utf8(password).unwrap(),
- existing_key,
- "Should not overwrite existing credentials"
- );
- }
-
- #[gpui::test]
- async fn test_skips_migration_if_empty_marker_exists(cx: &mut TestAppContext) {
- let old_api_key = "sk-or-old-key";
-
- // Old credentials exist
- cx.write_credentials(
- OPEN_ROUTER_DEFAULT_API_URL,
- "Bearer",
- old_api_key.as_bytes(),
- );
- // But empty marker already exists (from previous migration attempt)
- cx.write_credentials("extension-llm-openrouter:openrouter", "Bearer", b"");
-
- cx.update(|cx| {
- migrate_open_router_credentials_if_needed(OPEN_ROUTER_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-openrouter:openrouter");
- let (_, password) = credentials.unwrap();
assert!(
- password.is_empty(),
- "Should not overwrite empty marker with old credentials"
- );
- }
-
- #[gpui::test]
- async fn test_writes_empty_marker_if_no_old_credentials(cx: &mut TestAppContext) {
- cx.update(|cx| {
- migrate_open_router_credentials_if_needed(OPEN_ROUTER_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-openrouter:openrouter");
- assert!(
- credentials.is_some(),
- "Should write empty credentials as migration marker"
+ credentials.is_none(),
+ "Should not create credentials if none existed"
);
- let (username, password) = credentials.unwrap();
- assert_eq!(username, "Bearer");
- assert!(password.is_empty(), "Password should be empty marker");
}
#[gpui::test]
@@ -1,11 +1,14 @@
use credentials_provider::CredentialsProvider;
use gpui::App;
-use util::ResultExt as _;
const OPENAI_EXTENSION_ID: &str = "openai";
const OPENAI_PROVIDER_ID: &str = "openai";
const OPENAI_DEFAULT_API_URL: &str = "https://api.openai.com/v1";
+/// Migrates OpenAI API credentials from the old built-in provider location
+/// to the new extension-based location.
+///
+/// This should only be called during auto-install of the extension.
pub fn migrate_openai_credentials_if_needed(extension_id: &str, cx: &mut App) {
if extension_id != OPENAI_EXTENSION_ID {
return;
@@ -19,17 +22,7 @@ pub fn migrate_openai_credentials_if_needed(extension_id: &str, cx: &mut App) {
let credentials_provider = <dyn CredentialsProvider>::global(cx);
cx.spawn(async move |cx| {
- let existing_credential = credentials_provider
- .read_credentials(&extension_credential_key, &cx)
- .await
- .ok()
- .flatten();
-
- if existing_credential.is_some() {
- log::debug!("OpenAI extension already has credentials, skipping migration");
- return;
- }
-
+ // Read from old location
let old_credential = credentials_provider
.read_credentials(OPENAI_DEFAULT_API_URL, &cx)
.await
@@ -40,8 +33,8 @@ pub fn migrate_openai_credentials_if_needed(extension_id: &str, cx: &mut App) {
Some((_, key_bytes)) => match String::from_utf8(key_bytes) {
Ok(key) if !key.is_empty() => key,
Ok(_) => {
- log::debug!("Existing OpenAI API key is empty, marking as migrated");
- String::new()
+ log::debug!("Existing OpenAI API key is empty, nothing to migrate");
+ return;
}
Err(_) => {
log::error!("Failed to decode OpenAI API key as UTF-8");
@@ -49,20 +42,11 @@ pub fn migrate_openai_credentials_if_needed(extension_id: &str, cx: &mut App) {
}
},
None => {
- log::debug!("No existing OpenAI API key found, marking as migrated");
- String::new()
+ log::debug!("No existing OpenAI API key found to migrate");
+ return;
}
};
- if api_key.is_empty() {
- // Write empty credentials as a marker that migration was attempted
- credentials_provider
- .write_credentials(&extension_credential_key, "Bearer", b"", &cx)
- .await
- .log_err();
- return;
- }
-
log::info!("Migrating existing OpenAI API key to OpenAI extension");
match credentials_provider
@@ -105,41 +89,7 @@ mod tests {
}
#[gpui::test]
- async fn test_skips_migration_if_extension_already_has_credentials(cx: &mut TestAppContext) {
- let old_api_key = "sk-old-key";
- let existing_key = "sk-existing-key";
-
- cx.write_credentials(OPENAI_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- cx.write_credentials(
- "extension-llm-openai:openai",
- "Bearer",
- existing_key.as_bytes(),
- );
-
- cx.update(|cx| {
- migrate_openai_credentials_if_needed(OPENAI_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-openai:openai");
- let (_, password) = credentials.unwrap();
- assert_eq!(
- String::from_utf8(password).unwrap(),
- existing_key,
- "Should not overwrite existing credentials"
- );
- }
-
- #[gpui::test]
- async fn test_skips_migration_if_empty_marker_exists(cx: &mut TestAppContext) {
- let old_api_key = "sk-old-key";
-
- // Old credentials exist
- cx.write_credentials(OPENAI_DEFAULT_API_URL, "Bearer", old_api_key.as_bytes());
- // But empty marker already exists (from previous migration attempt)
- cx.write_credentials("extension-llm-openai:openai", "Bearer", b"");
-
+ async fn test_no_migration_if_no_old_credentials(cx: &mut TestAppContext) {
cx.update(|cx| {
migrate_openai_credentials_if_needed(OPENAI_EXTENSION_ID, cx);
});
@@ -147,29 +97,10 @@ mod tests {
cx.run_until_parked();
let credentials = cx.read_credentials("extension-llm-openai:openai");
- let (_, password) = credentials.unwrap();
assert!(
- password.is_empty(),
- "Should not overwrite empty marker with old credentials"
- );
- }
-
- #[gpui::test]
- async fn test_writes_empty_marker_if_no_old_credentials(cx: &mut TestAppContext) {
- cx.update(|cx| {
- migrate_openai_credentials_if_needed(OPENAI_EXTENSION_ID, cx);
- });
-
- cx.run_until_parked();
-
- let credentials = cx.read_credentials("extension-llm-openai:openai");
- assert!(
- credentials.is_some(),
- "Should write empty credentials as migration marker"
+ credentials.is_none(),
+ "Should not create credentials if none existed"
);
- let (username, password) = credentials.unwrap();
- assert_eq!(username, "Bearer");
- assert!(password.is_empty(), "Password should be empty marker");
}
#[gpui::test]
@@ -428,10 +428,7 @@ impl ExtensionProviderConfigurationView {
.log_err()
.flatten();
- // Treat empty credentials as not authenticated (used as migration marker)
- let has_credentials = credentials
- .map(|(_, password)| !password.is_empty())
- .unwrap_or(false);
+ let has_credentials = credentials.is_some();
// Update authentication state based on stored credentials
cx.update(|cx| {
@@ -539,10 +536,7 @@ impl ExtensionProviderConfigurationView {
.log_err()
.flatten();
- // Treat empty credentials as not authenticated (used as migration marker)
- let has_credentials = credentials
- .map(|(_, password)| !password.is_empty())
- .unwrap_or(false);
+ let has_credentials = credentials.is_some();
cx.update(|cx| {
state.update(cx, |state, cx| {