fix(cli): align mcp existing server advice

Author: OnlyYu1996

src/cortex-cli/src/agent_cmd/tests.rs

@@ -3,10 +3,9 @@
 #[cfg(test)]
 mod tests {
     use crate::agent_cmd::cli::{CopyArgs, ExportArgs};
-    use crate::agent_cmd::loader::{
-        load_builtin_agents, parse_frontmatter, read_file_with_encoding,
-    };
+    use crate::agent_cmd::loader::{load_builtin_agents, parse_frontmatter};
     use crate::agent_cmd::types::AgentMode;
+    use crate::utils::file::read_file_with_encoding;
 
     #[test]
     fn test_read_file_with_utf8() {

src/cortex-cli/src/mcp_cmd/handlers.rs

@@ -18,6 +18,13 @@ use super::validation::{
     validate_env_var_value, validate_server_name, validate_url, validate_url_internal,
 };
 
+fn existing_server_advice(name: &str) -> String {
+    format!(
+        "MCP server '{}' already exists. Use --force to overwrite, or 'cortex mcp remove {}' first.",
+        name, name
+    )
+}
+
 /// Run the list command.
 pub(crate) async fn run_list(args: ListArgs) -> Result<()> {
     let servers = get_mcp_servers()?;
@@ -253,11 +260,7 @@ pub(crate) async fn run_add(args: AddArgs) -> Result<()> {
     // Check if server already exists
     let server_exists = get_mcp_server(&name)?.is_some();
     if server_exists && !force {
-        bail!(
-            "MCP server '{}' already exists. Use --force to overwrite, or 'cortex mcp remove {}' first.",
-            name,
-            name
-        );
+        bail!("{}", existing_server_advice(&name));
     }
 
     // Check for conflicting transport types
@@ -722,11 +725,8 @@ pub(crate) async fn run_rename(args: RenameArgs) -> Result<()> {
     }
 
     // Check if new name already exists
-    if get_mcp_server(&args.new_name)?.is_some() {
-        bail!(
-            "MCP server '{}' already exists. Remove it first or choose a different name.",
-            args.new_name
-        );
+    if get_mcp_server(&args.new_name)?.is_some() && !args.force {
+        bail!("{}", existing_server_advice(&args.new_name));
     }
 
     let cortex_home =
@@ -746,6 +746,9 @@ pub(crate) async fn run_rename(args: RenameArgs) -> Result<()> {
     if let Some(mcp_servers) = config.get_mut("mcp_servers").and_then(|v| v.as_table_mut())
         && let Some(server_config) = mcp_servers.remove(&args.old_name)
     {
+        if args.force {
+            mcp_servers.remove(&args.new_name);
+        }
         mcp_servers.insert(args.new_name.clone(), server_config);
     }
 
@@ -889,20 +892,31 @@ mod tests {
         let args = RenameArgs {
             old_name: "old-name".to_string(),
             new_name: "new-name".to_string(),
+            force: false,
         };
         assert_eq!(args.old_name, "old-name");
         assert_eq!(args.new_name, "new-name");
+        assert!(!args.force);
     }
 
     #[test]
     fn test_rename_args_same_name() {
         let args = RenameArgs {
             old_name: "same".to_string(),
             new_name: "same".to_string(),
+            force: false,
         };
         assert_eq!(args.old_name, args.new_name);
     }
 
+    #[test]
+    fn test_existing_server_advice_is_shared_by_add_and_rename() {
+        assert_eq!(
+            existing_server_advice("my-server"),
+            "MCP server 'my-server' already exists. Use --force to overwrite, or 'cortex mcp remove my-server' first."
+        );
+    }
+
     // ========================================================================
     // AddArgs tests
     // ========================================================================

src/cortex-cli/src/mcp_cmd/types.rs

@@ -24,7 +24,6 @@ pub enum McpSubcommand {
     List(ListArgs),
 
     /// List configured MCP servers (alias for list).
-    #[command(visible_alias = "ls")]
     Ls(ListArgs),
 
     /// Show details for a configured MCP server.
@@ -116,7 +115,7 @@ pub struct AddArgs {
 #[command(
     group(
         ArgGroup::new("transport")
-            .args(["command", "url", "sse"])
+            .args(["command", "url", "sse_url"])
             .required(true)
             .multiple(false)
     )
@@ -174,7 +173,7 @@ pub struct AddMcpSseArgs {
     #[arg(
         long = "sse-bearer-token-env-var",
         value_name = "ENV_VAR",
-        requires = "sse"
+        requires = "sse_url"
     )]
     pub sse_bearer_token_env_var: Option<String>,
 }
@@ -213,6 +212,10 @@ pub struct RenameArgs {
 
     /// New name for the MCP server.
     pub new_name: String,
+
+    /// Overwrite an existing server with the new name.
+    #[arg(long, short = 'f')]
+    pub force: bool,
 }
 
 /// Auth command with subcommands.
@@ -293,6 +296,7 @@ pub struct DebugArgs {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use clap::CommandFactory;
 
     // =========================================================================
     // ListArgs tests
@@ -666,20 +670,33 @@ mod tests {
         let args = RenameArgs {
             old_name: "old-server".to_string(),
             new_name: "new-server".to_string(),
+            force: false,
         };
         assert_eq!(args.old_name, "old-server");
         assert_eq!(args.new_name, "new-server");
+        assert!(!args.force);
     }
 
     #[test]
     fn test_rename_args_same_name() {
         let args = RenameArgs {
             old_name: "server".to_string(),
             new_name: "server".to_string(),
+            force: false,
         };
         assert_eq!(args.old_name, args.new_name);
     }
 
+    #[test]
+    fn test_rename_args_force_flag() {
+        let args = RenameArgs {
+            old_name: "old-server".to_string(),
+            new_name: "existing-server".to_string(),
+            force: true,
+        };
+        assert!(args.force);
+    }
+
     // =========================================================================
     // AuthCommand tests
     // =========================================================================
@@ -897,6 +914,11 @@ mod tests {
         assert!(matches!(subcommand, McpSubcommand::Ls(_)));
     }
 
+    #[test]
+    fn test_mcp_cli_command_aliases_are_valid() {
+        McpCli::command().debug_assert();
+    }
+
     #[test]
     fn test_subcommand_get_variant() {
         let subcommand = McpSubcommand::Get(GetArgs {
@@ -951,6 +973,7 @@ mod tests {
         let subcommand = McpSubcommand::Rename(RenameArgs {
             old_name: "old".to_string(),
             new_name: "new".to_string(),
+            force: false,
         });
         assert!(matches!(subcommand, McpSubcommand::Rename(_)));
     }