From bbec3c988701f1240a73ce7010b9d21b07db63f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=20Houl=C3=A9?= Date: Tue, 31 Mar 2026 11:47:58 +0200 Subject: [PATCH] Fix reactivity and error state in the modal --- .../src/tools/context_server_registry.rs | 2 +- crates/agent_ui/src/agent_configuration.rs | 12 +- .../configure_context_server_modal.rs | 171 +++++++------- crates/context_server/src/oauth.rs | 59 +++-- crates/project/src/context_server_store.rs | 215 +++++++++--------- crates/project/src/project_settings.rs | 5 +- crates/settings_content/src/project.rs | 5 +- 7 files changed, 253 insertions(+), 216 deletions(-) diff --git a/crates/agent/src/tools/context_server_registry.rs b/crates/agent/src/tools/context_server_registry.rs index 02694ff3052a23f47e96e6d303a0f5f737ee76be..f257c89e99064256eece9b74f1adce52b30398a9 100644 --- a/crates/agent/src/tools/context_server_registry.rs +++ b/crates/agent/src/tools/context_server_registry.rs @@ -261,7 +261,7 @@ impl ContextServerRegistry { ContextServerStatus::Stopped | ContextServerStatus::Error(_) | ContextServerStatus::AuthRequired - | ContextServerStatus::ClientSecretRequired => { + | ContextServerStatus::ClientSecretRequired { .. } => { if let Some(registered_server) = self.registered_servers.remove(server_id) { if !registered_server.tools.is_empty() { cx.emit(ContextServerRegistryEvent::ToolsChanged); diff --git a/crates/agent_ui/src/agent_configuration.rs b/crates/agent_ui/src/agent_configuration.rs index e5d568310c5831da3d045f076b9753078592a3cd..5d0f6980a916a36d60a59227001e3806a8393251 100644 --- a/crates/agent_ui/src/agent_configuration.rs +++ b/crates/agent_ui/src/agent_configuration.rs @@ -664,8 +664,10 @@ impl AgentConfiguration { None }; let auth_required = matches!(server_status, ContextServerStatus::AuthRequired); - let client_secret_required = - matches!(server_status, ContextServerStatus::ClientSecretRequired); + let client_secret_required = matches!( + server_status, + ContextServerStatus::ClientSecretRequired { .. } + ); let authenticating = matches!(server_status, ContextServerStatus::Authenticating); let context_server_store = self.context_server_store.clone(); let workspace = self.workspace.clone(); @@ -689,7 +691,9 @@ impl AgentConfiguration { ContextServerStatus::Error(_) => AiSettingItemStatus::Error, ContextServerStatus::Stopped => AiSettingItemStatus::Stopped, ContextServerStatus::AuthRequired => AiSettingItemStatus::AuthRequired, - ContextServerStatus::ClientSecretRequired => AiSettingItemStatus::ClientSecretRequired, + ContextServerStatus::ClientSecretRequired { .. } => { + AiSettingItemStatus::ClientSecretRequired + } ContextServerStatus::Authenticating => AiSettingItemStatus::Authenticating, }; @@ -931,8 +935,6 @@ impl AgentConfiguration { .label_size(LabelSize::Small) .on_click({ let context_server_id = context_server_id.clone(); - let language_registry = language_registry.clone(); - let workspace = workspace.clone(); move |_event, window, cx| { ConfigureContextServerModal::show_modal_for_existing_server( context_server_id.clone(), diff --git a/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs b/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs index 528f7eeeb91ff01bf98add81c812526548c2621a..2eedfd0976f74aacf5464bd3b1fbc304ca99439a 100644 --- a/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs +++ b/crates/agent_ui/src/agent_configuration/configure_context_server_modal.rs @@ -406,9 +406,16 @@ fn resolve_context_server_extension( enum State { Idle, Waiting, - AuthRequired { server_id: ContextServerId }, - ClientSecretRequired { server_id: ContextServerId }, - Authenticating { _server_id: ContextServerId }, + AuthRequired { + server_id: ContextServerId, + }, + ClientSecretRequired { + server_id: ContextServerId, + error: Option, + }, + Authenticating { + server_id: ContextServerId, + }, Error(SharedString), } @@ -442,11 +449,14 @@ impl ConfigureContextServerModal { Some(ContextServerStatus::AuthRequired) => State::AuthRequired { server_id: server_id.clone(), }, - Some(ContextServerStatus::ClientSecretRequired) => State::ClientSecretRequired { - server_id: server_id.clone(), - }, + Some(ContextServerStatus::ClientSecretRequired { error }) => { + State::ClientSecretRequired { + server_id: server_id.clone(), + error: error.map(SharedString::from), + } + } Some(ContextServerStatus::Authenticating) => State::Authenticating { - _server_id: server_id.clone(), + server_id: server_id.clone(), }, Some(ContextServerStatus::Error(error)) => State::Error(error.into()), @@ -602,16 +612,12 @@ impl ConfigureContextServerModal { } fn confirm(&mut self, _: &menu::Confirm, cx: &mut Context) { - if matches!( - self.state, - State::Waiting - | State::AuthRequired { .. } - | State::ClientSecretRequired { .. } - | State::Authenticating { .. } - ) { + if matches!(self.state, State::Waiting | State::Authenticating { .. }) { return; } + self._auth_subscription = None; + self.state = State::Idle; let Some(workspace) = self.workspace.upgrade() else { return; @@ -627,7 +633,7 @@ impl ConfigureContextServerModal { self.state = State::Waiting; - let existing_server = self.context_server_store.read(cx).get_running_server(&id); + let existing_server = self.context_server_store.read(cx).get_server(&id); if existing_server.is_some() { self.context_server_store.update(cx, |store, cx| { store.stop_server(&id, cx).log_err(); @@ -650,8 +656,11 @@ impl ConfigureContextServerModal { this.state = State::AuthRequired { server_id: id }; cx.notify(); } - Ok(ContextServerStatus::ClientSecretRequired) => { - this.state = State::ClientSecretRequired { server_id: id }; + Ok(ContextServerStatus::ClientSecretRequired { error }) => { + this.state = State::ClientSecretRequired { + server_id: id, + error: error.map(SharedString::from), + }; cx.notify(); } Err(err) => { @@ -693,65 +702,33 @@ impl ConfigureContextServerModal { cx.emit(DismissEvent); } + fn cancel_authentication(&mut self, server_id: &ContextServerId, cx: &mut Context) { + self._auth_subscription = None; + self.context_server_store.update(cx, |store, cx| { + store.stop_server(server_id, cx).log_err(); + }); + self.state = State::Idle; + cx.notify(); + } + fn authenticate(&mut self, server_id: ContextServerId, cx: &mut Context) { self.context_server_store.update(cx, |store, cx| { store.authenticate_server(&server_id, cx).log_err(); }); - - self.state = State::Authenticating { - _server_id: server_id.clone(), - }; - - self._auth_subscription = Some(cx.subscribe( - &self.context_server_store, - move |this, _, event: &ServerStatusChangedEvent, cx| { - if event.server_id != server_id { - return; - } - match &event.status { - ContextServerStatus::Running => { - this._auth_subscription = None; - this.state = State::Idle; - this.show_configured_context_server_toast(event.server_id.clone(), cx); - cx.emit(DismissEvent); - } - ContextServerStatus::AuthRequired => { - this._auth_subscription = None; - this.state = State::AuthRequired { - server_id: event.server_id.clone(), - }; - cx.notify(); - } - ContextServerStatus::ClientSecretRequired => { - this._auth_subscription = None; - this.state = State::ClientSecretRequired { - server_id: event.server_id.clone(), - }; - cx.notify(); - } - ContextServerStatus::Error(error) => { - this._auth_subscription = None; - this.set_error(error.clone(), cx); - } - ContextServerStatus::Authenticating - | ContextServerStatus::Starting - | ContextServerStatus::Stopped => {} - } - }, - )); - - cx.notify(); + self.await_auth_outcome(server_id, cx); } fn submit_client_secret(&mut self, server_id: ContextServerId, cx: &mut Context) { let secret = self.secret_editor.read(cx).text(cx); - self.context_server_store.update(cx, |store, cx| { store.submit_client_secret(&server_id, secret, cx).log_err(); }); + self.await_auth_outcome(server_id, cx); + } + fn await_auth_outcome(&mut self, server_id: ContextServerId, cx: &mut Context) { self.state = State::Authenticating { - _server_id: server_id.clone(), + server_id: server_id.clone(), }; self._auth_subscription = Some(cx.subscribe( @@ -774,10 +751,11 @@ impl ConfigureContextServerModal { }; cx.notify(); } - ContextServerStatus::ClientSecretRequired => { + ContextServerStatus::ClientSecretRequired { error } => { this._auth_subscription = None; this.state = State::ClientSecretRequired { server_id: event.server_id.clone(), + error: error.clone().map(SharedString::from), }; cx.notify(); } @@ -981,13 +959,7 @@ impl ConfigureContextServerModal { fn render_modal_footer(&self, cx: &mut Context) -> ModalFooter { let focus_handle = self.focus_handle(cx); - let is_busy = matches!( - self.state, - State::Waiting - | State::AuthRequired { .. } - | State::ClientSecretRequired { .. } - | State::Authenticating { .. } - ); + let is_busy = matches!(self.state, State::Waiting | State::Authenticating { .. }); ModalFooter::new() .start_slot::