refactor(mcp-client): reuse http client to prevent file descriptor leaks (#3025)

Co-authored-by: Amit <amit@Tailcalls-MacBook-Pro.local>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: laststylebender <43403528+laststylebender14@users.noreply.github.com>

Author: 4 people

crates/forge_infra/src/mcp_client.rs

@@ -9,7 +9,8 @@ use forge_app::McpClientInfra;
 use forge_domain::{
     Environment, Image, McpHttpServer, McpServerConfig, ToolDefinition, ToolName, ToolOutput,
 };
-use http::{HeaderName, HeaderValue, header};
+use reqwest::Client;
+use reqwest::header::{HeaderName, HeaderValue};
 use rmcp::model::{CallToolRequestParam, ClientInfo, Implementation, InitializeRequestParam};
 use rmcp::service::RunningService;
 use rmcp::transport::sse_client::SseClientConfig;
@@ -33,20 +34,57 @@ type RmcpClient = RunningService<RoleClient, InitializeRequestParam>;
 #[derive(Clone)]
 pub struct ForgeMcpClient {
     client: Arc<RwLock<Option<Arc<RmcpClient>>>>,
+    http_client: Arc<Client>,
     config: McpServerConfig,
     env_vars: BTreeMap<String, String>,
     environment: Environment,
     resolved_config: Arc<OnceLock<anyhow::Result<McpServerConfig>>>,
 }
 
 impl ForgeMcpClient {
+    /// Build a reqwest client with default headers from the MCP server config.
+    fn build_http_client(http: &McpHttpServer) -> anyhow::Result<Client> {
+        let mut header_map = reqwest::header::HeaderMap::new();
+        for (key, value) in &http.headers {
+            if let Ok(name) = HeaderName::from_str(key)
+                && let Ok(val) = HeaderValue::from_str(value)
+            {
+                header_map.insert(name, val);
+            }
+        }
+
+        Ok(Client::builder().default_headers(header_map).build()?)
+    }
+
     pub fn new(
         config: McpServerConfig,
         env_vars: &BTreeMap<String, String>,
         environment: Environment,
     ) -> Self {
+        // Try to resolve config early so we can extract headers for the HTTP client.
+        // If resolution fails, fall back to a plain client (headers will be missing
+        // but the error will surface when create_connection is called).
+        let resolved = resolve_http_templates(
+            match &config {
+                McpServerConfig::Http(http) => http.clone(),
+                McpServerConfig::Stdio(_) => McpHttpServer {
+                    url: String::new(),
+                    headers: BTreeMap::new(),
+                    timeout: None,
+                    disable: false,
+                    oauth: forge_domain::McpOAuthSetting::default(),
+                },
+            },
+            env_vars,
+        );
+
+        let http_client = resolved
+            .and_then(|http| Self::build_http_client(&http))
+            .unwrap_or_default();
+
         Self {
             client: Default::default(),
+            http_client: Arc::new(http_client),
             config,
             env_vars: env_vars.clone(),
             environment,
@@ -181,16 +219,16 @@ impl ForgeMcpClient {
         http: &McpHttpServer,
     ) -> anyhow::Result<RmcpClient> {
         // Try HTTP first, fall back to SSE if it fails
-        let client = self.reqwest_client(http)?;
+        let client = self.reqwest_client();
         let transport = StreamableHttpClientTransport::with_client(
-            client.clone(),
+            client.as_ref().clone(),
             StreamableHttpClientTransportConfig::with_uri(http.url.clone()),
         );
         match self.client_info().serve(transport).await {
             Ok(client) => Ok(client),
             Err(_e) => {
                 let transport = SseClientTransport::start_with_client(
-                    client,
+                    client.as_ref().clone(),
                     SseClientConfig { sse_endpoint: http.url.clone().into(), ..Default::default() },
                 )
                 .await?;
@@ -358,9 +396,9 @@ impl ForgeMcpClient {
         http: &McpHttpServer,
         token: &str,
     ) -> anyhow::Result<Arc<RmcpClient>> {
-        let client = self.reqwest_client(http)?;
+        let client = self.reqwest_client();
         let transport = StreamableHttpClientTransport::with_client(
-            client,
+            client.as_ref().clone(),
             StreamableHttpClientTransportConfig::with_uri(http.url.clone()).auth_header(token),
         );
 
@@ -456,14 +494,12 @@ impl ForgeMcpClient {
         Ok((code, state))
     }
 
-    fn reqwest_client(&self, config: &McpHttpServer) -> anyhow::Result<reqwest::Client> {
-        let mut headers = header::HeaderMap::new();
-        for (key, value) in config.headers.iter() {
-            headers.insert(HeaderName::from_str(key)?, HeaderValue::from_str(value)?);
-        }
-
-        let client = reqwest::Client::builder().default_headers(headers);
-        Ok(client.build()?)
+    fn reqwest_client(&self) -> Arc<Client> {
+        // Reuse the cached HTTP client (with pre-configured default headers)
+        // to prevent file descriptor leaks. Each reqwest::Client manages its
+        // own connection pool, so creating new clients for each connection
+        // leads to "Too many open files" errors.
+        self.http_client.clone()
     }
 
     async fn list(&self) -> anyhow::Result<Vec<ToolDefinition>> {