agent_ui: Route MCP configuration to active workspace (#61882)
# Objective - Prevent MCP extension configuration dialogs from appearing when switching to background workspaces. - Closes FR-145. ## Solution - Route configuration requests through the active application window and workspace, while keeping uninstall settings cleanup global. ## Testing - Added a regression test covering active and retained background workspaces. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable --- Release Notes: - Agent: Fixed MCP extension configuration dialogs appearing in other workspaces.
This commit is contained in:
Ben Kunkle authored and GitHub committed
1 parent
0cf4dacd26
commit
4215da0a21
1 file changed
+125
-49
@@ -8,57 +8,47 @@ use language::LanguageRegistry;
|
||||
use settings::update_settings_file;
|
||||
use ui::prelude::*;
|
||||
use util::ResultExt;
|
||||
use workspace::Workspace;
|
||||
use workspace::{MultiWorkspace, Workspace};
|
||||
|
||||
use crate::agent_configuration::ConfigureContextServerModal;
|
||||
|
||||
pub(crate) fn init(language_registry: Arc<LanguageRegistry>, fs: Arc<dyn Fs>, cx: &mut App) {
|
||||
cx.observe_new(move |_: &mut Workspace, window, cx| {
|
||||
let Some(window) = window else {
|
||||
return;
|
||||
};
|
||||
let Some(extension_events) = extension::ExtensionEvents::try_global(cx) else {
|
||||
log::info!(
|
||||
"No extension events global found. Skipping context server configuration wizard"
|
||||
);
|
||||
return;
|
||||
};
|
||||
|
||||
if let Some(extension_events) = extension::ExtensionEvents::try_global(cx).as_ref() {
|
||||
cx.subscribe_in(extension_events, window, {
|
||||
let language_registry = language_registry.clone();
|
||||
let fs = fs.clone();
|
||||
move |_, _, event, window, cx| match event {
|
||||
extension::Event::ExtensionInstalled(manifest) => {
|
||||
show_configure_mcp_modal(
|
||||
language_registry.clone(),
|
||||
manifest,
|
||||
cx.weak_entity(),
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
}
|
||||
extension::Event::ExtensionUninstalled(manifest) => {
|
||||
remove_context_server_settings(
|
||||
manifest.context_servers.keys().cloned().collect(),
|
||||
fs.clone(),
|
||||
cx,
|
||||
);
|
||||
}
|
||||
extension::Event::ConfigureExtensionRequested(manifest) => {
|
||||
if !manifest.context_servers.is_empty() {
|
||||
show_configure_mcp_modal(
|
||||
language_registry.clone(),
|
||||
manifest,
|
||||
cx.weak_entity(),
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
})
|
||||
.detach();
|
||||
} else {
|
||||
log::info!(
|
||||
"No extension events global found. Skipping context server configuration wizard"
|
||||
cx.subscribe(&extension_events, move |_, event, cx| match event {
|
||||
extension::Event::ExtensionUninstalled(manifest) => {
|
||||
remove_context_server_settings(
|
||||
manifest.context_servers.keys().cloned().collect(),
|
||||
fs.clone(),
|
||||
cx,
|
||||
);
|
||||
}
|
||||
extension::Event::ExtensionInstalled(manifest)
|
||||
| extension::Event::ConfigureExtensionRequested(manifest) => {
|
||||
let Some(multi_workspace) = cx
|
||||
.active_window()
|
||||
.and_then(|window| window.downcast::<MultiWorkspace>())
|
||||
else {
|
||||
return;
|
||||
};
|
||||
multi_workspace
|
||||
.update(cx, |multi_workspace, window, cx| {
|
||||
show_configure_mcp_modal(
|
||||
language_registry.clone(),
|
||||
manifest,
|
||||
multi_workspace.workspace().downgrade(),
|
||||
window,
|
||||
cx,
|
||||
);
|
||||
})
|
||||
.log_err();
|
||||
}
|
||||
extension::Event::ExtensionsInstalledChanged => {}
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
@@ -81,12 +71,8 @@ fn show_configure_mcp_modal(
|
||||
manifest: &Arc<ExtensionManifest>,
|
||||
workspace: WeakEntity<Workspace>,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<'_, Workspace>,
|
||||
cx: &mut Context<'_, MultiWorkspace>,
|
||||
) {
|
||||
if !window.is_window_active() {
|
||||
return;
|
||||
}
|
||||
|
||||
let ids = manifest.context_servers.keys().cloned().collect::<Vec<_>>();
|
||||
if ids.is_empty() {
|
||||
return;
|
||||
@@ -114,3 +100,93 @@ fn show_configure_mcp_modal(
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use context_server::ContextServerCommand;
|
||||
use gpui::TestAppContext;
|
||||
use project::{
|
||||
FakeFs, Project,
|
||||
project_settings::{ContextServerSettings, ProjectSettings},
|
||||
};
|
||||
use settings::Settings as _;
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_configure_extension_only_opens_modal_in_active_workspace(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
crate::test_support::init_test(cx);
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let language_registry = Arc::new(LanguageRegistry::test(cx.executor()));
|
||||
cx.update(|cx| {
|
||||
extension::init(cx);
|
||||
let mut settings = ProjectSettings::get_global(cx).clone();
|
||||
settings.context_servers.insert(
|
||||
"slack".into(),
|
||||
ContextServerSettings::Stdio {
|
||||
enabled: true,
|
||||
remote: false,
|
||||
command: ContextServerCommand {
|
||||
path: "slack-mcp-server".into(),
|
||||
args: Vec::new(),
|
||||
env: None,
|
||||
timeout: None,
|
||||
},
|
||||
},
|
||||
);
|
||||
ProjectSettings::override_global(settings, cx);
|
||||
init(language_registry, fs.clone(), cx);
|
||||
});
|
||||
|
||||
let active_project = Project::test(fs.clone(), [], cx).await;
|
||||
let background_project = Project::test(fs, [], cx).await;
|
||||
let multi_workspace =
|
||||
cx.add_window(|window, cx| MultiWorkspace::test_new(active_project, window, cx));
|
||||
let (active_workspace, background_workspace) = multi_workspace
|
||||
.update(cx, |multi_workspace, window, cx| {
|
||||
let active_workspace = multi_workspace.workspace().clone();
|
||||
let background_workspace =
|
||||
cx.new(|cx| Workspace::test_new(background_project, window, cx));
|
||||
multi_workspace.add(background_workspace.clone(), window, cx);
|
||||
(active_workspace, background_workspace)
|
||||
})
|
||||
.expect("multi-workspace window should exist");
|
||||
cx.run_until_parked();
|
||||
|
||||
let manifest = Arc::new(
|
||||
serde_json::from_value::<ExtensionManifest>(serde_json::json!({
|
||||
"id": "slack",
|
||||
"name": "Slack",
|
||||
"version": "1.0.0",
|
||||
"schema_version": 1,
|
||||
"context_servers": {
|
||||
"slack": {}
|
||||
}
|
||||
}))
|
||||
.expect("test extension manifest should deserialize"),
|
||||
);
|
||||
cx.update(|cx| {
|
||||
let extension_events = extension::ExtensionEvents::try_global(cx)
|
||||
.expect("extension events should be initialized");
|
||||
extension_events.update(cx, |extension_events, cx| {
|
||||
extension_events.emit(extension::Event::ConfigureExtensionRequested(manifest), cx);
|
||||
});
|
||||
});
|
||||
cx.run_until_parked();
|
||||
|
||||
assert!(
|
||||
active_workspace.read_with(cx, |workspace, cx| workspace
|
||||
.active_modal::<ConfigureContextServerModal>(cx)
|
||||
.is_some()),
|
||||
"the active workspace should show the configuration modal"
|
||||
);
|
||||
assert!(
|
||||
background_workspace.read_with(cx, |workspace, cx| workspace
|
||||
.active_modal::<ConfigureContextServerModal>(cx)
|
||||
.is_none()),
|
||||
"a background workspace should not receive a hidden configuration modal"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user