language_models: Clear cached credentials when OpenAI and OpenAI Compatible provider `api_url` change (#37610)

Umesh Yadav created 2 months ago

Closes #37093

Also check this: #37099.

So currently in zed for both OpenAI and OpenAI Compatible provider when
the url is changed from settings the api_key stored in the provider
state is not cleared and it is still used. But if you restart zed the
api_key is cleared. Currently zed uses the api_url to store and fetch
the api key from credential provider. The behaviour is not changed
overall, it's just that we have made it consistent it with the zed
restart logic where it re-authenticates and fetches the api_key again. I
have attached the video below to show case before and after of this.

So all in all the problem was we were not re-authenticating the in case
api_url change while zed is still running. Now we trigger a
re-authentication and clear the state in case authentication fails.
 
OpenAI Compatible Provider:

| Before | After |
|--------|--------|
| <video
src="https://github.com/user-attachments/assets/324d2707-ea72-4119-8981-6b596a9f40a3"
/> | <video
src="https://github.com/user-attachments/assets/cc7fdb73-8975-4aaf-a642-809bb03ce319"
/> |

OpenAI Provider:

| Before | After |
|--------|--------|
| <video
src="https://github.com/user-attachments/assets/a1c07d1b-1909-4b49-b33c-fc05123e92e7"
/> | <video
src="https://github.com/user-attachments/assets/d78aeccd-5cd3-4d0c-8b9f-6f98e499d7c8"
/> |

Release Notes:

- Fixed OpenAI and OpenAI Compatible provide API keys being persisted
when changing the API URL setting. Authentication is now properly
revalidated when settings change.

---------

Signed-off-by: Umesh Yadav <[e-mail]>

Change summary

crates/language_models/src/provider/open_ai.rs            | 48 +++++++-
crates/language_models/src/provider/open_ai_compatible.rs | 36 +++++-
2 files changed, 69 insertions(+), 15 deletions(-)

Detailed changes

crates/language_models/src/provider/open_ai.rs

@@ -56,13 +56,13 @@ pub struct OpenAiLanguageModelProvider {
 pub struct State {
     api_key: Option<String>,
     api_key_from_env: bool,
+    last_api_url: String,
     _subscription: Subscription,
 }
 
 const OPENAI_API_KEY_VAR: &str = "OPENAI_API_KEY";
 
 impl State {
-    //
     fn is_authenticated(&self) -> bool {
         self.api_key.is_some()
     }
@@ -104,11 +104,7 @@ impl State {
         })
     }
 
-    fn authenticate(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
-        if self.is_authenticated() {
-            return Task::ready(Ok(()));
-        }
-
+    fn get_api_key(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
         let credentials_provider = <dyn CredentialsProvider>::global(cx);
         let api_url = AllLanguageModelSettings::get_global(cx)
             .openai
@@ -136,14 +132,52 @@ impl State {
             Ok(())
         })
     }
+
+    fn authenticate(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
+        if self.is_authenticated() {
+            return Task::ready(Ok(()));
+        }
+
+        self.get_api_key(cx)
+    }
 }
 
 impl OpenAiLanguageModelProvider {
     pub fn new(http_client: Arc<dyn HttpClient>, cx: &mut App) -> Self {
+        let initial_api_url = AllLanguageModelSettings::get_global(cx)
+            .openai
+            .api_url
+            .clone();
+
         let state = cx.new(|cx| State {
             api_key: None,
             api_key_from_env: false,
-            _subscription: cx.observe_global::<SettingsStore>(|_this: &mut State, cx| {
+            last_api_url: initial_api_url.clone(),
+            _subscription: cx.observe_global::<SettingsStore>(|this: &mut State, cx| {
+                let current_api_url = AllLanguageModelSettings::get_global(cx)
+                    .openai
+                    .api_url
+                    .clone();
+
+                if this.last_api_url != current_api_url {
+                    this.last_api_url = current_api_url;
+                    if !this.api_key_from_env {
+                        this.api_key = None;
+                        let spawn_task = cx.spawn(async move |handle, cx| {
+                            if let Ok(task) = handle.update(cx, |this, cx| this.get_api_key(cx)) {
+                                if let Err(_) = task.await {
+                                    handle
+                                        .update(cx, |this, _| {
+                                            this.api_key = None;
+                                            this.api_key_from_env = false;
+                                        })
+                                        .ok();
+                                }
+                            }
+                        });
+                        spawn_task.detach();
+                    }
+                }
                 cx.notify();
             }),
         });

crates/language_models/src/provider/open_ai_compatible.rs

@@ -113,11 +113,7 @@ impl State {
         })
     }
 
-    fn authenticate(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
-        if self.is_authenticated() {
-            return Task::ready(Ok(()));
-        }
-
+    fn get_api_key(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
         let credentials_provider = <dyn CredentialsProvider>::global(cx);
         let env_var_name = self.env_var_name.clone();
         let api_url = self.settings.api_url.clone();
@@ -143,6 +139,14 @@ impl State {
             Ok(())
         })
     }
+
+    fn authenticate(&self, cx: &mut Context<Self>) -> Task<Result<(), AuthenticateError>> {
+        if self.is_authenticated() {
+            return Task::ready(Ok(()));
+        }
+
+        self.get_api_key(cx)
+    }
 }
 
 impl OpenAiCompatibleLanguageModelProvider {
@@ -160,11 +164,27 @@ impl OpenAiCompatibleLanguageModelProvider {
             api_key: None,
             api_key_from_env: false,
             _subscription: cx.observe_global::<SettingsStore>(|this: &mut State, cx| {
-                let Some(settings) = resolve_settings(&this.id, cx) else {
+                let Some(settings) = resolve_settings(&this.id, cx).cloned() else {
                     return;
                 };
-                if &this.settings != settings {
-                    this.settings = settings.clone();
+                if &this.settings != &settings {
+                    if settings.api_url != this.settings.api_url && !this.api_key_from_env {
+                        let spawn_task = cx.spawn(async move |handle, cx| {
+                            if let Ok(task) = handle.update(cx, |this, cx| this.get_api_key(cx)) {
+                                if let Err(_) = task.await {
+                                    handle
+                                        .update(cx, |this, _| {
+                                            this.api_key = None;
+                                            this.api_key_from_env = false;
+                                        })
+                                        .ok();
+                                }
+                            }
+                        });
+                        spawn_task.detach();
+                    }
+
+                    this.settings = settings;
                     cx.notify();
                 }
             }),