fix(mcp): drain stderr from child processes to prevent buffer overflow (#2598)

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

Author: J-I-B

crates/forge_domain/src/mcp.rs

@@ -33,13 +33,19 @@ impl McpServerConfig {
             command: command.into(),
             args,
             env: env.unwrap_or_default(),
+            timeout: None,
             disable: false,
         })
     }
 
     /// Create a new HTTP-based MCP server (auto-detects transport type)
     pub fn new_http(url: impl Into<String>) -> Self {
-        Self::Http(McpHttpServer { url: url.into(), headers: BTreeMap::new(), disable: false })
+        Self::Http(McpHttpServer {
+            url: url.into(),
+            headers: BTreeMap::new(),
+            timeout: None,
+            disable: false,
+        })
     }
 
     pub fn is_disabled(&self) -> bool {
@@ -73,6 +79,11 @@ pub struct McpStdioServer {
     #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
     pub env: BTreeMap<String, String>,
 
+    /// Timeout in seconds for tool calls to this MCP server
+    /// If not specified, uses the default FORGE_MCP_TIMEOUT or 300 seconds
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub timeout: Option<u64>,
+
     /// Disable it temporarily without having to
     /// remove it from the config.
     #[serde(default)]
@@ -90,6 +101,11 @@ pub struct McpHttpServer {
     #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
     pub headers: BTreeMap<String, String>,
 
+    /// Timeout in seconds for HTTP requests to this MCP server
+    /// If not specified, uses the default FORGE_MCP_TIMEOUT or 300 seconds
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub timeout: Option<u64>,
+
     /// Disable it temporarily without having to
     /// remove it from the config.
     #[serde(default)]
@@ -337,6 +353,53 @@ mod tests {
         }
     }
 
+    #[test]
+    fn test_http_server_with_timeout() {
+        use pretty_assertions::assert_eq;
+
+        let json = r#"{
+            "mcpServers": {
+                "slow-server": {
+                    "url": "https://api.example.com/mcp/",
+                    "timeout": 600
+                }
+            }
+        }"#;
+
+        let actual: McpConfig = serde_json::from_str(json).unwrap();
+
+        match actual.mcp_servers.get(&"slow-server".to_string().into()) {
+            Some(McpServerConfig::Http(server)) => {
+                assert_eq!(server.url, "https://api.example.com/mcp/");
+                assert_eq!(server.timeout, Some(600));
+            }
+            _ => panic!("Expected Http variant"),
+        }
+    }
+
+    #[test]
+    fn test_http_server_without_timeout() {
+        use pretty_assertions::assert_eq;
+
+        let json = r#"{
+            "mcpServers": {
+                "fast-server": {
+                    "url": "https://api.example.com/mcp/"
+                }
+            }
+        }"#;
+
+        let actual: McpConfig = serde_json::from_str(json).unwrap();
+
+        match actual.mcp_servers.get(&"fast-server".to_string().into()) {
+            Some(McpServerConfig::Http(server)) => {
+                assert_eq!(server.url, "https://api.example.com/mcp/");
+                assert_eq!(server.timeout, None);
+            }
+            _ => panic!("Expected Http variant"),
+        }
+    }
+
     #[test]
     fn test_server_type() {
         use fake::{Fake, Faker};
@@ -354,4 +417,60 @@ mod tests {
         let expected = "HTTP";
         assert_eq!(actual, expected);
     }
+
+    #[test]
+    fn test_stdio_server_with_timeout() {
+        use pretty_assertions::assert_eq;
+
+        let json = r#"{
+            "mcpServers": {
+                "slow-stdio-server": {
+                    "command": "node",
+                    "args": ["server.js"],
+                    "timeout": 600
+                }
+            }
+        }"#;
+
+        let actual: McpConfig = serde_json::from_str(json).unwrap();
+
+        match actual
+            .mcp_servers
+            .get(&"slow-stdio-server".to_string().into())
+        {
+            Some(McpServerConfig::Stdio(server)) => {
+                assert_eq!(server.command, "node");
+                assert_eq!(server.args, vec!["server.js"]);
+                assert_eq!(server.timeout, Some(600));
+            }
+            _ => panic!("Expected Stdio variant"),
+        }
+    }
+
+    #[test]
+    fn test_stdio_server_without_timeout() {
+        use pretty_assertions::assert_eq;
+
+        let json = r#"{
+            "mcpServers": {
+                "fast-stdio-server": {
+                    "command": "node",
+                    "args": ["server.js"]
+                }
+            }
+        }"#;
+
+        let actual: McpConfig = serde_json::from_str(json).unwrap();
+
+        match actual
+            .mcp_servers
+            .get(&"fast-stdio-server".to_string().into())
+        {
+            Some(McpServerConfig::Stdio(server)) => {
+                assert_eq!(server.command, "node");
+                assert_eq!(server.timeout, None);
+            }
+            _ => panic!("Expected Stdio variant"),
+        }
+    }
 }

crates/forge_infra/src/mcp_client.rs

@@ -16,6 +16,7 @@ use rmcp::transport::{SseClientTransport, StreamableHttpClientTransport, TokioCh
 use rmcp::{RoleClient, ServiceExt};
 use schemars::Schema;
 use serde_json::Value;
+use tokio::io::{AsyncBufReadExt, BufReader};
 use tokio::process::Command;
 
 use crate::error::Error;
@@ -106,11 +107,21 @@ impl ForgeMcpClient {
 
                 cmd.args(&stdio.args).kill_on_drop(true);
 
-                // Use builder pattern to capture and ignore stderr to silence MCP logs
-                let (transport, _stderr) = TokioChildProcess::builder(cmd)
+                // Use builder pattern to capture stderr
+                let (transport, stderr) = TokioChildProcess::builder(cmd)
                     .stderr(std::process::Stdio::piped())
                     .spawn()?;
 
+                // Spawn a task to drain stderr to prevent buffer overflow
+                // If stderr fills up, the child process will block
+                if let Some(stderr) = stderr {
+                    tokio::spawn(async move {
+                        let mut reader = BufReader::new(stderr).lines();
+                        while let Ok(Some(line)) = reader.next_line().await {
+                            tracing::warn!("MCP server stderr: {}", line);
+                        }
+                    });
+                }
                 self.client_info().serve(transport).await?
             }
             McpServerConfig::Http(http) => {
@@ -297,6 +308,7 @@ mod tests {
                 ("X-API-Key".to_string(), "{{env.API_KEY}}".to_string()),
                 ("Content-Type".to_string(), "application/json".to_string()),
             ]),
+            timeout: None,
             disable: false,
         };
 
@@ -326,6 +338,7 @@ mod tests {
                 "Authorization".to_string(),
                 "Bearer {{env.MISSING_VAR}}".to_string(),
             )]),
+            timeout: None,
             disable: false,
         };
 
@@ -345,6 +358,7 @@ mod tests {
         let http = McpHttpServer {
             url: "https://test.example.com".to_string(),
             headers: BTreeMap::from([("Auth".to_string(), "{{env.TOKEN}}".to_string())]),
+            timeout: None,
             disable: true,
         };