fix(mcp): normalize server names, fix UTF-8 truncation, skip auth when header set

crates/ironclaw_gateway/static/app.js

@@ -6520,12 +6520,24 @@ function installWasmExtension() {
   });
 }
 
+function normalizeMcpServerName(raw) {
+  return raw.toLowerCase()
+    .replace(/[^a-z0-9_-]/g, '_')
+    .replace(/_+/g, '_')
+    .replace(/^_|_$/g, '');
+}
+
 function addMcpServer() {
-  var name = document.getElementById('mcp-install-name').value.trim();
-  if (!name) {
+  var rawName = document.getElementById('mcp-install-name').value.trim();
+  if (!rawName) {
     showToast(I18n.t('mcp.serverNameRequired'), 'error');
     return;
   }
+  var name = normalizeMcpServerName(rawName);
+  if (!name) {
+    showToast('Server name contains no valid characters', 'error');
+    return;
+  }
   var url = document.getElementById('mcp-install-url').value.trim();
   if (!url) {
     showToast(I18n.t('mcp.urlRequired'), 'error');

crates/ironclaw_tui/src/widgets/conversation.rs

@@ -859,7 +859,14 @@ impl ConversationWidget {
         let mut items_str = items.join(", ");
         // Truncate if too long
         if items_str.len() > 60 {
-            items_str.truncate(57);
+            // Use char_indices to find a safe truncation point (UTF-8 safe)
+            let truncate_at = items_str
+                .char_indices()
+                .map(|(i, _)| i)
+                .take_while(|&i| i <= 57)
+                .last()
+                .unwrap_or(0);
+            items_str.truncate(truncate_at);
             items_str.push_str("...");
         }
 

src/agent/thread_ops.rs

@@ -405,7 +405,9 @@ impl Agent {
                                 .iter()
                                 .any(|rule| rule.action == ironclaw_safety::PolicyAction::Block)
                             {
-                                return Ok(SubmissionResult::error("Input rejected by safety policy."));
+                                return Ok(SubmissionResult::error(
+                                    "Input rejected by safety policy.",
+                                ));
                             }
                             if let Some(warning) = self.safety().scan_inbound_for_secrets(content) {
                                 tracing::warn!(
@@ -426,7 +428,8 @@ impl Agent {
                             // acknowledgment, not a completed LLM turn.
                             return Ok(SubmissionResult::Ok {
                                 message: Some(
-                                    "Message queued — will be processed after the current turn.".into(),
+                                    "Message queued — will be processed after the current turn."
+                                        .into(),
                                 ),
                             });
                         }

src/channels/web/handlers/extensions.rs

@@ -139,8 +139,19 @@ pub async fn extensions_install_handler(
         _ => None,
     });
 
+    let normalized_name = crate::tools::mcp::config::normalize_server_name(&req.name);
+    if normalized_name.is_empty() {
+        return Ok(Json(ActionResponse::fail(
+            "Server name contains no valid characters".to_string(),
+        )));
+    }
     match ext_mgr
-        .install(&req.name, req.url.as_deref(), kind_hint, &user.user_id)
+        .install(
+            &normalized_name,
+            req.url.as_deref(),
+            kind_hint,
+            &user.user_id,
+        )
         .await
     {
         Ok(result) => Ok(Json(ActionResponse::ok(result.message))),

src/cli/mcp.rs

@@ -14,7 +14,7 @@ use crate::secrets::SecretsStore;
 use crate::tools::mcp::{
     McpClient, McpProcessManager, McpServerConfig, McpSessionManager, OAuthConfig,
     auth::{authorize_mcp_server, is_authenticated},
-    config::{self, EffectiveTransport, McpServersFile},
+    config::{self, EffectiveTransport, McpServersFile, normalize_server_name},
     factory::create_client_from_config,
 };
 
@@ -166,7 +166,7 @@ pub async fn run_mcp_command(cmd: McpCommand) -> anyhow::Result<()> {
 /// Add a new MCP server.
 async fn add_server(args: McpAddArgs) -> anyhow::Result<()> {
     let McpAddArgs {
-        name,
+        name: raw_name,
         url,
         transport,
         command,
@@ -181,6 +181,16 @@ async fn add_server(args: McpAddArgs) -> anyhow::Result<()> {
         description,
     } = args;
 
+    // Normalize the server name: lowercase, replace special chars with
+    // underscores so descriptive names like "My Twitter Server" work (#2236).
+    let name = normalize_server_name(&raw_name);
+    if name.is_empty() {
+        anyhow::bail!("Server name '{}' contains no valid characters", raw_name);
+    }
+    if name != raw_name {
+        println!("  (normalized name: '{}' -> '{}')", raw_name, name);
+    }
+
     let transport_lower = transport.to_lowercase();
 
     let mut config = match transport_lower.as_str() {
@@ -290,6 +300,7 @@ async fn add_server(args: McpAddArgs) -> anyhow::Result<()> {
 
 /// Remove an MCP server.
 async fn remove_server(name: String) -> anyhow::Result<()> {
+    let name = normalize_server_name(&name);
     let (db, owner_id) = connect_db().await;
     let mut servers = load_servers(db.as_deref(), &owner_id).await?;
     if !servers.remove(&name) {
@@ -411,6 +422,7 @@ async fn list_servers(verbose: bool) -> anyhow::Result<()> {
 
 /// Authenticate with an MCP server.
 async fn auth_server(name: String, user_id: String) -> anyhow::Result<()> {
+    let name = normalize_server_name(&name);
     // Get server config
     let (db, owner_id) = connect_db().await;
     let servers = load_servers(db.as_deref(), &owner_id).await?;
@@ -495,6 +507,7 @@ async fn auth_server(name: String, user_id: String) -> anyhow::Result<()> {
 
 /// Test connection to an MCP server.
 async fn test_server(name: String, user_id: String) -> anyhow::Result<()> {
+    let name = normalize_server_name(&name);
     // Get server config
     let (db, owner_id) = connect_db().await;
     let servers = load_servers(db.as_deref(), &owner_id).await?;
@@ -568,9 +581,11 @@ async fn test_server(name: String, user_id: String) -> anyhow::Result<()> {
                         };
                         println!("    • {}{}", tool.name, approval);
                         if !tool.description.is_empty() {
-                            // Truncate long descriptions
-                            let desc = if tool.description.len() > 60 {
-                                format!("{}...", &tool.description[..57])
+                            // Truncate long descriptions (char-safe to avoid
+                            // panicking on multi-byte UTF-8 boundaries, #1947)
+                            let desc = if tool.description.chars().count() > 60 {
+                                let truncated: String = tool.description.chars().take(57).collect();
+                                format!("{truncated}...")
                             } else {
                                 tool.description.clone()
                             };
@@ -618,6 +633,7 @@ async fn test_server(name: String, user_id: String) -> anyhow::Result<()> {
 
 /// Toggle server enabled/disabled state.
 async fn toggle_server(name: String, enable: bool, disable: bool) -> anyhow::Result<()> {
+    let name = normalize_server_name(&name);
     let (db, owner_id) = connect_db().await;
     let mut servers = load_servers(db.as_deref(), &owner_id).await?;
 
@@ -741,4 +757,54 @@ mod tests {
         assert!(result.is_err());
         assert!(result.unwrap_err().contains("invalid env var format"));
     }
+
+    /// Regression test for #1947: byte-index slicing panics on multi-byte UTF-8.
+    ///
+    /// Reproduces the original bug: a description with multi-byte chars where
+    /// byte index 57 falls inside a character boundary would panic with
+    /// `&tool.description[..57]`.
+    #[test]
+    fn test_tool_description_truncation_multibyte_utf8() {
+        // Each CJK character is 3 bytes. 20 chars = 60 bytes.
+        // Byte index 57 falls inside the 20th character (bytes 57..59).
+        let description = "\u{4e00}".repeat(20); // 20 CJK chars, 60 bytes
+        assert_eq!(description.len(), 60);
+        assert_eq!(description.chars().count(), 20);
+
+        // Simulate the fixed truncation logic
+        let desc = if description.chars().count() > 60 {
+            let truncated: String = description.chars().take(57).collect();
+            format!("{truncated}...")
+        } else {
+            description.clone()
+        };
+        // 20 chars <= 60, so no truncation needed
+        assert_eq!(desc, description);
+
+        // Now test with a string that actually exceeds 60 chars
+        let long_desc = "\u{4e00}".repeat(61); // 61 CJK chars
+        assert_eq!(long_desc.chars().count(), 61);
+        let desc = if long_desc.chars().count() > 60 {
+            let truncated: String = long_desc.chars().take(57).collect();
+            format!("{truncated}...")
+        } else {
+            long_desc.clone()
+        };
+        assert_eq!(desc.chars().count(), 60); // 57 chars + "..."
+        assert!(desc.ends_with("..."));
+    }
+
+    /// Verify ASCII descriptions still truncate correctly.
+    #[test]
+    fn test_tool_description_truncation_ascii() {
+        let description = "a".repeat(80);
+        let desc = if description.chars().count() > 60 {
+            let truncated: String = description.chars().take(57).collect();
+            format!("{truncated}...")
+        } else {
+            description.clone()
+        };
+        assert_eq!(desc.len(), 60);
+        assert!(desc.ends_with("..."));
+    }
 }