mirror of
https://github.com/zed-industries/zed.git
synced 2026-06-01 03:14:56 +07:00
Add global and HTTP context server timeout settings (#45378)
### Closes - Maybe https://github.com/zed-industries/zed/issues/38252 (there might be something going on with NPX too) - Also addresses https://github.com/zed-industries/zed/pull/39021#issuecomment-3644818347 ### Why Some more involved MCP servers timeout often, especially on first setup. I got tired of having to set timeouts manually per server. I also noticed a timeout could not be set for http context servers, which causes Context7 or GitHub's remote servers to timeout at 60s sometimes. ### Overview MCP Timeout Configuration Feature This PR adds additional configurable timeout settings for Model Context Protocol servers including a global timeout and the addition of timeouts for http servers, addressing issues where servers were timing out after a fixed 60 seconds regardless of user needs. **Key Features:** - **Global timeout setting** (`context_server_timeout`) - default timeout for all MCP servers (default: 60s, max: 10min) - **Per-server timeout overrides** - individual servers can specify custom timeouts via `timeout` field - **Precedence hierarchy** - per-server timeout > global timeout > default (60s) - **Automatic bounds checking** - enforces 10-minute maximum to prevent resource exhaustion - **Support for both transports** - works with stdio and HTTP-based context servers - **Comprehensive test coverage** - 3 new tests validating global, override, and stdio timeout behavior - **Full backward compatibility** - existing configurations work unchanged with sensible defaults ### Release Notes: - Added the ability to configure timeouts for context server tool calls. The new global `context_server_timeout` setting controls the default timeout (default is 60s, max: 10min). Additionally, per-server timeouts can be configured using the `timeout` field within servers defined in the `"context_servers" setting --------- Co-authored-by: Ben Kunkle <ben@zed.dev>
This commit is contained in:
parent
beece469d6
commit
62ae7fb51b
10 changed files with 264 additions and 16 deletions
|
|
@ -2262,6 +2262,17 @@
|
|||
"ssh_connections": [],
|
||||
// Whether to read ~/.ssh/config for ssh connection sources.
|
||||
"read_ssh_config": true,
|
||||
// Default timeout in seconds for all context server tool calls.
|
||||
// Individual servers can override this in their configuration.
|
||||
// Examples:
|
||||
// "context_servers": {
|
||||
// "my-stdio-server": {
|
||||
// "command": "/path/to/server",
|
||||
// "timeout": 120 // Override: 2 minutes for this server
|
||||
// },
|
||||
// }
|
||||
// Default: 60
|
||||
"context_server_timeout": 60,
|
||||
// Configures context servers for use by the agent.
|
||||
"context_servers": {},
|
||||
// Configures agent servers available in the agent panel.
|
||||
|
|
|
|||
|
|
@ -306,6 +306,7 @@ impl AgentConnection for AcpConnection {
|
|||
project::context_server_store::ContextServerConfiguration::Http {
|
||||
url,
|
||||
headers,
|
||||
timeout: _,
|
||||
} => Some(acp::McpServer::Http(
|
||||
acp::McpServerHttp::new(id.0.to_string(), url.to_string()).headers(
|
||||
headers
|
||||
|
|
|
|||
|
|
@ -172,6 +172,7 @@ impl ConfigurationSource {
|
|||
enabled: true,
|
||||
url,
|
||||
headers: auth,
|
||||
timeout: None,
|
||||
},
|
||||
)
|
||||
})
|
||||
|
|
@ -411,6 +412,7 @@ impl ConfigureContextServerModal {
|
|||
enabled: _,
|
||||
url,
|
||||
headers,
|
||||
timeout: _,
|
||||
} => Some(ConfigurationTarget::ExistingHttp {
|
||||
id: server_id,
|
||||
url,
|
||||
|
|
|
|||
|
|
@ -176,7 +176,7 @@ impl Client {
|
|||
.map(|name| name.to_string_lossy().into_owned())
|
||||
.unwrap_or_else(String::new);
|
||||
|
||||
let timeout = binary.timeout.map(Duration::from_millis);
|
||||
let timeout = binary.timeout.map(Duration::from_secs);
|
||||
let transport = Arc::new(StdioTransport::new(binary, working_directory, &cx)?);
|
||||
Self::new(server_id, server_name.into(), transport, timeout, cx)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ use collections::HashMap;
|
|||
use http_client::HttpClient;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
use std::{fmt::Display, path::PathBuf};
|
||||
|
||||
use anyhow::Result;
|
||||
|
|
@ -39,6 +40,7 @@ pub struct ContextServer {
|
|||
id: ContextServerId,
|
||||
client: RwLock<Option<Arc<crate::protocol::InitializedContextServerProtocol>>>,
|
||||
configuration: ContextServerTransport,
|
||||
request_timeout: Option<Duration>,
|
||||
}
|
||||
|
||||
impl ContextServer {
|
||||
|
|
@ -54,6 +56,7 @@ impl ContextServer {
|
|||
command,
|
||||
working_directory.map(|directory| directory.to_path_buf()),
|
||||
),
|
||||
request_timeout: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -63,6 +66,7 @@ impl ContextServer {
|
|||
headers: HashMap<String, String>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
executor: gpui::BackgroundExecutor,
|
||||
request_timeout: Option<Duration>,
|
||||
) -> Result<Self> {
|
||||
let transport = match endpoint.scheme() {
|
||||
"http" | "https" => {
|
||||
|
|
@ -73,14 +77,23 @@ impl ContextServer {
|
|||
}
|
||||
_ => anyhow::bail!("unsupported MCP url scheme {}", endpoint.scheme()),
|
||||
};
|
||||
Ok(Self::new(id, transport))
|
||||
Ok(Self::new_with_timeout(id, transport, request_timeout))
|
||||
}
|
||||
|
||||
pub fn new(id: ContextServerId, transport: Arc<dyn crate::transport::Transport>) -> Self {
|
||||
Self::new_with_timeout(id, transport, None)
|
||||
}
|
||||
|
||||
pub fn new_with_timeout(
|
||||
id: ContextServerId,
|
||||
transport: Arc<dyn crate::transport::Transport>,
|
||||
request_timeout: Option<Duration>,
|
||||
) -> Self {
|
||||
Self {
|
||||
id,
|
||||
client: RwLock::new(None),
|
||||
configuration: ContextServerTransport::Custom(transport),
|
||||
request_timeout,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -113,7 +126,7 @@ impl ContextServer {
|
|||
client::ContextServerId(self.id.0.clone()),
|
||||
self.id().0,
|
||||
transport.clone(),
|
||||
None,
|
||||
self.request_timeout,
|
||||
cx.clone(),
|
||||
)?,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ pub mod extension;
|
|||
pub mod registry;
|
||||
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::{Context as _, Result};
|
||||
use collections::{HashMap, HashSet};
|
||||
|
|
@ -18,6 +19,10 @@ use crate::{
|
|||
worktree_store::WorktreeStore,
|
||||
};
|
||||
|
||||
/// Maximum timeout for context server requests
|
||||
/// Prevents extremely large timeout values from tying up resources indefinitely.
|
||||
const MAX_TIMEOUT_SECS: u64 = 600; // 10 minutes
|
||||
|
||||
pub fn init(cx: &mut App) {
|
||||
extension::init(cx);
|
||||
}
|
||||
|
|
@ -102,6 +107,7 @@ pub enum ContextServerConfiguration {
|
|||
Http {
|
||||
url: url::Url,
|
||||
headers: HashMap<String, String>,
|
||||
timeout: Option<u64>,
|
||||
},
|
||||
}
|
||||
|
||||
|
|
@ -151,9 +157,14 @@ impl ContextServerConfiguration {
|
|||
enabled: _,
|
||||
url,
|
||||
headers: auth,
|
||||
timeout,
|
||||
} => {
|
||||
let url = url::Url::parse(&url).log_err()?;
|
||||
Some(ContextServerConfiguration::Http { url, headers: auth })
|
||||
Some(ContextServerConfiguration::Http {
|
||||
url,
|
||||
headers: auth,
|
||||
timeout,
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -250,7 +261,8 @@ impl ContextServerStore {
|
|||
this.available_context_servers_changed(cx);
|
||||
}),
|
||||
cx.observe_global::<SettingsStore>(|this, cx| {
|
||||
let settings = Self::resolve_context_server_settings(&this.worktree_store, cx);
|
||||
let settings =
|
||||
&Self::resolve_project_settings(&this.worktree_store, cx).context_servers;
|
||||
if &this.context_server_settings == settings {
|
||||
return;
|
||||
}
|
||||
|
|
@ -264,7 +276,8 @@ impl ContextServerStore {
|
|||
|
||||
let mut this = Self {
|
||||
_subscriptions: subscriptions,
|
||||
context_server_settings: Self::resolve_context_server_settings(&worktree_store, cx)
|
||||
context_server_settings: Self::resolve_project_settings(&worktree_store, cx)
|
||||
.context_servers
|
||||
.clone(),
|
||||
worktree_store,
|
||||
project: weak_project,
|
||||
|
|
@ -482,17 +495,27 @@ impl ContextServerStore {
|
|||
configuration: Arc<ContextServerConfiguration>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Result<Arc<ContextServer>> {
|
||||
let global_timeout =
|
||||
Self::resolve_project_settings(&self.worktree_store, cx).context_server_timeout;
|
||||
|
||||
if let Some(factory) = self.context_server_factory.as_ref() {
|
||||
return Ok(factory(id, configuration));
|
||||
}
|
||||
|
||||
match configuration.as_ref() {
|
||||
ContextServerConfiguration::Http { url, headers } => Ok(Arc::new(ContextServer::http(
|
||||
ContextServerConfiguration::Http {
|
||||
url,
|
||||
headers,
|
||||
timeout,
|
||||
} => Ok(Arc::new(ContextServer::http(
|
||||
id,
|
||||
url,
|
||||
headers.clone(),
|
||||
cx.http_client(),
|
||||
cx.background_executor().clone(),
|
||||
Some(Duration::from_secs(
|
||||
timeout.unwrap_or(global_timeout).min(MAX_TIMEOUT_SECS),
|
||||
)),
|
||||
)?)),
|
||||
_ => {
|
||||
let root_path = self
|
||||
|
|
@ -511,19 +534,27 @@ impl ContextServerStore {
|
|||
})
|
||||
})
|
||||
});
|
||||
Ok(Arc::new(ContextServer::stdio(
|
||||
id,
|
||||
configuration.command().unwrap().clone(),
|
||||
root_path,
|
||||
)))
|
||||
|
||||
let mut command = configuration
|
||||
.command()
|
||||
.context("Missing command configuration for stdio context server")?
|
||||
.clone();
|
||||
command.timeout = Some(
|
||||
command
|
||||
.timeout
|
||||
.unwrap_or(global_timeout)
|
||||
.min(MAX_TIMEOUT_SECS),
|
||||
);
|
||||
|
||||
Ok(Arc::new(ContextServer::stdio(id, command, root_path)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_context_server_settings<'a>(
|
||||
fn resolve_project_settings<'a>(
|
||||
worktree_store: &'a Entity<WorktreeStore>,
|
||||
cx: &'a App,
|
||||
) -> &'a HashMap<Arc<str>, ContextServerSettings> {
|
||||
) -> &'a ProjectSettings {
|
||||
let location = worktree_store
|
||||
.read(cx)
|
||||
.visible_worktrees(cx)
|
||||
|
|
@ -532,7 +563,7 @@ impl ContextServerStore {
|
|||
worktree_id: worktree.read(cx).id(),
|
||||
path: RelPath::empty(),
|
||||
});
|
||||
&ProjectSettings::get(location, cx).context_servers
|
||||
ProjectSettings::get(location, cx)
|
||||
}
|
||||
|
||||
fn update_server_state(
|
||||
|
|
@ -1257,6 +1288,7 @@ mod tests {
|
|||
enabled: true,
|
||||
url: server_url.to_string(),
|
||||
headers: Default::default(),
|
||||
timeout: None,
|
||||
},
|
||||
)],
|
||||
)
|
||||
|
|
@ -1327,6 +1359,160 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_context_server_global_timeout(cx: &mut TestAppContext) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
SettingsStore::update_global(cx, |store, cx| {
|
||||
store
|
||||
.set_user_settings(r#"{"context_server_timeout": 90}"#, cx)
|
||||
.expect("Failed to set test user settings");
|
||||
});
|
||||
});
|
||||
|
||||
let (_fs, project) = setup_context_server_test(cx, json!({"code.rs": ""}), vec![]).await;
|
||||
|
||||
let registry = cx.new(|_| ContextServerDescriptorRegistry::new());
|
||||
let store = cx.new(|cx| {
|
||||
ContextServerStore::test(
|
||||
registry.clone(),
|
||||
project.read(cx).worktree_store(),
|
||||
project.downgrade(),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let result = store.update(cx, |store, cx| {
|
||||
store.create_context_server(
|
||||
ContextServerId("test-server".into()),
|
||||
Arc::new(ContextServerConfiguration::Http {
|
||||
url: url::Url::parse("http://localhost:8080")
|
||||
.expect("Failed to parse test URL"),
|
||||
headers: Default::default(),
|
||||
timeout: None,
|
||||
}),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"Server should be created successfully with global timeout"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_context_server_per_server_timeout_override(cx: &mut TestAppContext) {
|
||||
const SERVER_ID: &str = "test-server";
|
||||
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
SettingsStore::update_global(cx, |store, cx| {
|
||||
store
|
||||
.set_user_settings(r#"{"context_server_timeout": 60}"#, cx)
|
||||
.expect("Failed to set test user settings");
|
||||
});
|
||||
});
|
||||
|
||||
let (_fs, project) = setup_context_server_test(
|
||||
cx,
|
||||
json!({"code.rs": ""}),
|
||||
vec![(
|
||||
SERVER_ID.into(),
|
||||
ContextServerSettings::Http {
|
||||
enabled: true,
|
||||
url: "http://localhost:8080".to_string(),
|
||||
headers: Default::default(),
|
||||
timeout: Some(120),
|
||||
},
|
||||
)],
|
||||
)
|
||||
.await;
|
||||
|
||||
let registry = cx.new(|_| ContextServerDescriptorRegistry::new());
|
||||
let store = cx.new(|cx| {
|
||||
ContextServerStore::test(
|
||||
registry.clone(),
|
||||
project.read(cx).worktree_store(),
|
||||
project.downgrade(),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let result = store.update(cx, |store, cx| {
|
||||
store.create_context_server(
|
||||
ContextServerId("test-server".into()),
|
||||
Arc::new(ContextServerConfiguration::Http {
|
||||
url: url::Url::parse("http://localhost:8080")
|
||||
.expect("Failed to parse test URL"),
|
||||
headers: Default::default(),
|
||||
timeout: Some(120),
|
||||
}),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"Server should be created successfully with per-server timeout override"
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_context_server_stdio_timeout(cx: &mut TestAppContext) {
|
||||
const SERVER_ID: &str = "stdio-server";
|
||||
|
||||
let (_fs, project) = setup_context_server_test(
|
||||
cx,
|
||||
json!({"code.rs": ""}),
|
||||
vec![(
|
||||
SERVER_ID.into(),
|
||||
ContextServerSettings::Stdio {
|
||||
enabled: true,
|
||||
command: ContextServerCommand {
|
||||
path: "/usr/bin/node".into(),
|
||||
args: vec!["server.js".into()],
|
||||
env: None,
|
||||
timeout: Some(180000),
|
||||
},
|
||||
},
|
||||
)],
|
||||
)
|
||||
.await;
|
||||
|
||||
let registry = cx.new(|_| ContextServerDescriptorRegistry::new());
|
||||
let store = cx.new(|cx| {
|
||||
ContextServerStore::test(
|
||||
registry.clone(),
|
||||
project.read(cx).worktree_store(),
|
||||
project.downgrade(),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
let result = store.update(cx, |store, cx| {
|
||||
store.create_context_server(
|
||||
ContextServerId("stdio-server".into()),
|
||||
Arc::new(ContextServerConfiguration::Custom {
|
||||
command: ContextServerCommand {
|
||||
path: "/usr/bin/node".into(),
|
||||
args: vec!["server.js".into()],
|
||||
env: None,
|
||||
timeout: Some(180000),
|
||||
},
|
||||
}),
|
||||
cx,
|
||||
)
|
||||
});
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"Stdio server should be created successfully with timeout"
|
||||
);
|
||||
}
|
||||
|
||||
fn dummy_server_settings() -> ContextServerSettings {
|
||||
ContextServerSettings::Stdio {
|
||||
enabled: true,
|
||||
|
|
|
|||
|
|
@ -59,6 +59,9 @@ pub struct ProjectSettings {
|
|||
/// Settings for context servers used for AI-related features.
|
||||
pub context_servers: HashMap<Arc<str>, ContextServerSettings>,
|
||||
|
||||
/// Default timeout for context server requests in seconds.
|
||||
pub context_server_timeout: u64,
|
||||
|
||||
/// Configuration for Diagnostics-related features.
|
||||
pub diagnostics: DiagnosticsSettings,
|
||||
|
||||
|
|
@ -141,6 +144,8 @@ pub enum ContextServerSettings {
|
|||
/// Optional authentication configuration for the remote server.
|
||||
#[serde(skip_serializing_if = "HashMap::is_empty", default)]
|
||||
headers: HashMap<String, String>,
|
||||
/// Timeout for tool calls in milliseconds.
|
||||
timeout: Option<u64>,
|
||||
},
|
||||
Extension {
|
||||
/// Whether the context server is enabled.
|
||||
|
|
@ -167,10 +172,12 @@ impl From<settings::ContextServerSettingsContent> for ContextServerSettings {
|
|||
enabled,
|
||||
url,
|
||||
headers,
|
||||
timeout,
|
||||
} => ContextServerSettings::Http {
|
||||
enabled,
|
||||
url,
|
||||
headers,
|
||||
timeout,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -188,10 +195,12 @@ impl Into<settings::ContextServerSettingsContent> for ContextServerSettings {
|
|||
enabled,
|
||||
url,
|
||||
headers,
|
||||
timeout,
|
||||
} => settings::ContextServerSettingsContent::Http {
|
||||
enabled,
|
||||
url,
|
||||
headers,
|
||||
timeout,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
|
@ -560,6 +569,7 @@ impl Settings for ProjectSettings {
|
|||
.into_iter()
|
||||
.map(|(key, value)| (key, value.into()))
|
||||
.collect(),
|
||||
context_server_timeout: project.context_server_timeout.unwrap_or(60),
|
||||
lsp: project
|
||||
.lsp
|
||||
.clone()
|
||||
|
|
|
|||
|
|
@ -54,6 +54,12 @@ pub struct ProjectSettingsContent {
|
|||
#[serde(default)]
|
||||
pub context_servers: HashMap<Arc<str>, ContextServerSettingsContent>,
|
||||
|
||||
/// Default timeout in seconds for context server tool calls.
|
||||
/// Can be overridden per-server in context_servers configuration.
|
||||
///
|
||||
/// Default: 60
|
||||
pub context_server_timeout: Option<u64>,
|
||||
|
||||
/// Configuration for how direnv configuration should be loaded
|
||||
pub load_direnv: Option<DirenvSettings>,
|
||||
|
||||
|
|
@ -234,6 +240,8 @@ pub enum ContextServerSettingsContent {
|
|||
/// Optional headers to send.
|
||||
#[serde(skip_serializing_if = "HashMap::is_empty", default)]
|
||||
headers: HashMap<String, String>,
|
||||
/// Timeout for tool calls in seconds. Defaults to global context_server_timeout if not specified.
|
||||
timeout: Option<u64>,
|
||||
},
|
||||
Extension {
|
||||
/// Whether the context server is enabled.
|
||||
|
|
@ -275,7 +283,7 @@ pub struct ContextServerCommand {
|
|||
pub path: PathBuf,
|
||||
pub args: Vec<String>,
|
||||
pub env: Option<HashMap<String, String>>,
|
||||
/// Timeout for tool calls in milliseconds. Defaults to 60000 (60 seconds) if not specified.
|
||||
/// Timeout for tool calls in seconds. Defaults to 60 if not specified.
|
||||
pub timeout: Option<u64>,
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -403,6 +403,7 @@ impl VsCodeSettings {
|
|||
terminal: None,
|
||||
dap: Default::default(),
|
||||
context_servers: self.context_servers(),
|
||||
context_server_timeout: None,
|
||||
load_direnv: None,
|
||||
slash_commands: None,
|
||||
git_hosting_providers: None,
|
||||
|
|
|
|||
|
|
@ -6193,6 +6193,22 @@ pub(crate) fn settings_data(cx: &App) -> Vec<SettingsPage> {
|
|||
metadata: None,
|
||||
files: USER,
|
||||
}),
|
||||
SettingsPageItem::SectionHeader("Context Servers"),
|
||||
SettingsPageItem::SettingItem(SettingItem {
|
||||
title: "Context Server Timeout",
|
||||
description: "Default timeout in seconds for context server tool calls. Can be overridden per-server in context_servers configuration.",
|
||||
field: Box::new(SettingField {
|
||||
json_path: Some("context_server_timeout"),
|
||||
pick: |settings_content| {
|
||||
settings_content.project.context_server_timeout.as_ref()
|
||||
},
|
||||
write: |settings_content, value| {
|
||||
settings_content.project.context_server_timeout = value;
|
||||
},
|
||||
}),
|
||||
metadata: None,
|
||||
files: USER | PROJECT,
|
||||
}),
|
||||
];
|
||||
items.extend(edit_prediction_language_settings_section());
|
||||
items.extend(
|
||||
|
|
|
|||
Loading…
Reference in a new issue