Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/cortex-cli/src/agent_cmd/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
34 changes: 24 additions & 10 deletions src/cortex-cli/src/mcp_cmd/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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);
}

Expand Down Expand Up @@ -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
// ========================================================================
Expand Down
29 changes: 26 additions & 3 deletions src/cortex-cli/src/mcp_cmd/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
)
Expand Down Expand Up @@ -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>,
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -293,6 +296,7 @@ pub struct DebugArgs {
#[cfg(test)]
mod tests {
use super::*;
use clap::CommandFactory;

// =========================================================================
// ListArgs tests
Expand Down Expand Up @@ -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
// =========================================================================
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(_)));
}
Expand Down