feat(mcp): support env vars in the mcp config file (#2241)

Author: konippi

crates/chat-cli/src/cli/chat/tool_manager.rs

@@ -233,7 +233,7 @@ impl ToolManagerBuilder {
                     );
                     None
                 } else {
-                    let custom_tool_client = CustomToolClient::from_config(server_name.clone(), server_config);
+                    let custom_tool_client = CustomToolClient::from_config(server_name.clone(), server_config, os);
                     Some((server_name, custom_tool_client))
                 }
             })

crates/chat-cli/src/cli/chat/tools/custom_tool.rs

@@ -8,6 +8,7 @@ use crossterm::{
     style,
 };
 use eyre::Result;
+use regex::Regex;
 use schemars::JsonSchema;
 use serde::{
     Deserialize,
@@ -23,6 +24,7 @@ use crate::cli::agent::{
 };
 use crate::cli::chat::CONTINUATION_LINE;
 use crate::cli::chat::token_counter::TokenCounter;
+use crate::os::Os;
 use crate::mcp_client::{
     Client as McpClient,
     ClientConfig as McpClientConfig,
@@ -35,7 +37,6 @@ use crate::mcp_client::{
     StdioTransport,
     ToolCallResult,
 };
-use crate::os::Os;
 
 // TODO: support http transport type
 #[derive(Clone, Serialize, Deserialize, Debug, Eq, PartialEq, JsonSchema)]
@@ -63,6 +64,26 @@ pub fn default_timeout() -> u64 {
     120 * 1000
 }
 
+/// Substitutes environment variables in the format ${env:VAR_NAME} with their actual values
+fn substitute_env_vars(input: &str, env: &crate::os::Env) -> String {
+    // Create a regex to match ${env:VAR_NAME} pattern
+    let re = Regex::new(r"\$\{env:([^}]+)\}").unwrap();
+
+    re.replace_all(input, |caps: &regex::Captures<'_>| {
+        let var_name = &caps[1];
+        env.get(var_name).unwrap_or_else(|_| format!("${{{}}}", var_name))
+    })
+    .to_string()
+}
+
+/// Process a HashMap of environment variables, substituting any ${env:VAR_NAME} patterns
+/// with their actual values from the environment
+fn process_env_vars(env_vars: &mut HashMap<String, String>, env: &crate::os::Env) {
+    for (_, value) in env_vars.iter_mut() {
+        *value = substitute_env_vars(value, env);
+    }
+}
+
 #[derive(Debug)]
 pub enum CustomToolClient {
     Stdio {
@@ -75,7 +96,7 @@ pub enum CustomToolClient {
 
 impl CustomToolClient {
     // TODO: add support for http transport
-    pub fn from_config(server_name: String, config: CustomToolConfig) -> Result<Self> {
+    pub fn from_config(server_name: String, config: CustomToolConfig, os: &crate::os::Os) -> Result<Self> {
         let CustomToolConfig {
             command,
             args,
@@ -84,6 +105,13 @@ impl CustomToolClient {
             disabled: _,
             ..
         } = config;
+
+        // Process environment variables if present
+        let processed_env = env.map(|mut env_vars| {
+            process_env_vars(&mut env_vars, &os.env);
+            env_vars
+        });
+
         let mcp_client_config = McpClientConfig {
             server_name: server_name.clone(),
             bin_path: command.clone(),
@@ -93,7 +121,7 @@ impl CustomToolClient {
                "name": "Q CLI Chat",
                "version": "1.0.0"
             }),
-            env,
+            env: processed_env,
         };
         let client = McpClient::<JsonRpcStdioTransport>::from_config(mcp_client_config)?;
         Ok(CustomToolClient::Stdio {
@@ -279,3 +307,52 @@ impl CustomTool {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[tokio::test]
+    async fn test_substitute_env_vars() {
+        // Set a test environment variable
+        let os = Os::new().await.unwrap();
+        unsafe {
+            os.env.set_var("TEST_VAR", "test_value");
+        }
+
+        // Test basic substitution
+        assert_eq!(substitute_env_vars("Value is ${env:TEST_VAR}", &os.env), "Value is test_value");
+
+        // Test multiple substitutions
+        assert_eq!(
+            substitute_env_vars("${env:TEST_VAR} and ${env:TEST_VAR}", &os.env),
+            "test_value and test_value"
+        );
+
+        // Test non-existent variable
+        assert_eq!(substitute_env_vars("${env:NON_EXISTENT_VAR}", &os.env), "${NON_EXISTENT_VAR}");
+
+        // Test mixed content
+        assert_eq!(
+            substitute_env_vars("Prefix ${env:TEST_VAR} suffix", &os.env),
+            "Prefix test_value suffix"
+        );
+    }
+
+    #[tokio::test]
+    async fn test_process_env_vars() {
+        let os = Os::new().await.unwrap();
+        unsafe {
+            os.env.set_var("TEST_VAR", "test_value");
+        }
+
+        let mut env_vars = HashMap::new();
+        env_vars.insert("KEY1".to_string(), "Value is ${env:TEST_VAR}".to_string());
+        env_vars.insert("KEY2".to_string(), "No substitution".to_string());
+
+        process_env_vars(&mut env_vars, &os.env);
+
+        assert_eq!(env_vars.get("KEY1").unwrap(), "Value is test_value");
+        assert_eq!(env_vars.get("KEY2").unwrap(), "No substitution");
+    }
+}