diff --git a/cortex-cli/src/agent_cmd.rs b/cortex-cli/src/agent_cmd.rs index a241bd4e..ff1b3c6a 100644 --- a/cortex-cli/src/agent_cmd.rs +++ b/cortex-cli/src/agent_cmd.rs @@ -1023,6 +1023,51 @@ async fn run_list(args: ListArgs) -> Result<()> { Ok(()) } +/// Compute the effective tools available to an agent. +/// Returns a summary of allowed, denied, and explicitly configured tools. +fn compute_effective_tools(agent: &AgentInfo) -> serde_json::Value { + // All available tools in the system + let all_tools = vec![ + "Read", + "Write", + "Edit", + "MultiEdit", + "Create", + "LS", + "Glob", + "Grep", + "Execute", + "WebSearch", + "FetchUrl", + "TodoRead", + "TodoWrite", + "AskUser", + "Skill", + "Task", + "Batch", + ]; + + let allowed = if let Some(ref allowed_list) = agent.allowed_tools { + allowed_list.clone() + } else { + // No allow list means all tools are potentially available (minus denied) + all_tools.iter().map(|s| s.to_string()).collect() + }; + + // Remove denied tools + let denied = &agent.denied_tools; + let effective: Vec = allowed + .into_iter() + .filter(|t| !denied.iter().any(|d| d.eq_ignore_ascii_case(t))) + .collect(); + + serde_json::json!({ + "available": effective, + "explicitly_denied": denied, + "has_restrictions": agent.allowed_tools.is_some() || !agent.denied_tools.is_empty() + }) +} + /// Show agent details command. async fn run_show(args: ShowArgs) -> Result<()> { let agents = load_all_agents()?; @@ -1043,6 +1088,7 @@ async fn run_show(args: ShowArgs) -> Result<()> { if args.json { // Create a custom JSON value to show "builtin" instead of null for native agents + // and include effective tools information let mut json_value = serde_json::to_value(agent)?; if let serde_json::Value::Object(ref mut map) = json_value { if agent.native && agent.path.is_none() { @@ -1051,6 +1097,12 @@ async fn run_show(args: ShowArgs) -> Result<()> { serde_json::Value::String("builtin".to_string()), ); } + // Add effective_tools field showing what tools this agent can use + let effective_tools = compute_effective_tools(agent); + map.insert( + "effective_tools".to_string(), + serde_json::to_value(&effective_tools).unwrap_or(serde_json::Value::Null), + ); } let json = serde_json::to_string_pretty(&json_value)?; println!("{json}"); diff --git a/cortex-cli/src/debug_cmd.rs b/cortex-cli/src/debug_cmd.rs index 2d8f0afc..f9084918 100644 --- a/cortex-cli/src/debug_cmd.rs +++ b/cortex-cli/src/debug_cmd.rs @@ -405,6 +405,7 @@ fn generate_unified_diff(old_content: &str, new_content: &str) -> String { #[derive(Debug, Parser)] pub struct FileArgs { /// Path to the file to inspect. + #[arg(value_parser = validate_non_empty_path)] pub path: PathBuf, /// Output as JSON. @@ -412,6 +413,14 @@ pub struct FileArgs { pub json: bool, } +/// Validate that a path is not empty. +fn validate_non_empty_path(s: &str) -> Result { + if s.trim().is_empty() { + return Err("Please provide a file path".to_string()); + } + Ok(PathBuf::from(s)) +} + /// File debug output. #[derive(Debug, Serialize)] struct FileDebugOutput { @@ -912,8 +921,8 @@ struct LspConnectionTest { } async fn run_lsp(args: LspArgs) -> Result<()> { - // Known LSP servers - let known_servers = vec![ + // Known LSP servers - using static array to avoid ownership issues + const KNOWN_SERVERS: &[(&str, &str, &str)] = &[ ("rust-analyzer", "Rust", "rust-analyzer"), ( "typescript-language-server", @@ -939,7 +948,7 @@ async fn run_lsp(args: LspArgs) -> Result<()> { let mut servers = Vec::new(); - for (name, language, command) in known_servers { + for (name, language, command) in KNOWN_SERVERS { let (installed, path, version) = check_command_installed(command).await; servers.push(LspServerInfo { name: name.to_string(), @@ -953,12 +962,37 @@ async fn run_lsp(args: LspArgs) -> Result<()> { // Filter if specific server requested if let Some(ref server_name) = args.server { + let original_count = servers.len(); servers.retain(|s| s.name.to_lowercase().contains(&server_name.to_lowercase())); + // Error if no servers match the filter + if servers.is_empty() && original_count > 0 { + let known_names: Vec<_> = KNOWN_SERVERS.iter().map(|(name, _, _)| *name).collect(); + bail!( + "Unknown LSP server: '{}'\n\nKnown servers: {}", + server_name, + known_names.join(", ") + ); + } } // Filter by language if specified if let Some(ref lang) = args.language { + let original_count = servers.len(); servers.retain(|s| s.language.to_lowercase().contains(&lang.to_lowercase())); + // Error if no servers match the language filter + if servers.is_empty() && original_count > 0 { + let known_languages: Vec<_> = KNOWN_SERVERS + .iter() + .map(|(_, language, _)| *language) + .collect::>() + .into_iter() + .collect(); + bail!( + "No LSP servers found for language: '{}'\n\nKnown languages: {}", + lang, + known_languages.join(", ") + ); + } } // Connection test placeholder (actual implementation would require LSP client) diff --git a/cortex-cli/src/exec_cmd.rs b/cortex-cli/src/exec_cmd.rs index 19b3672d..ad0b5281 100644 --- a/cortex-cli/src/exec_cmd.rs +++ b/cortex-cli/src/exec_cmd.rs @@ -396,7 +396,13 @@ impl ExecCli { let prompt = self.build_prompt().await?; if prompt.is_empty() { - bail!("No prompt provided. Use positional argument, --file, or pipe via stdin."); + bail!( + "Please provide a prompt.\n\n\ + Usage:\n \ + cortex exec \"your prompt here\"\n \ + cortex exec --file prompt.txt\n \ + echo \"prompt\" | cortex exec" + ); } // Echo prompt if requested (#2715) diff --git a/cortex-cli/src/import_cmd.rs b/cortex-cli/src/import_cmd.rs index 1d30cc0a..1adfe05e 100644 --- a/cortex-cli/src/import_cmd.rs +++ b/cortex-cli/src/import_cmd.rs @@ -49,18 +49,25 @@ impl ImportCommand { .map(|h| h.join(".cortex")) .ok_or_else(|| anyhow::anyhow!("Could not determine home directory"))?; - // Read the export data - let (json_content, is_from_url) = - if self.source.starts_with("http://") || self.source.starts_with("https://") { - // Fetch from URL - (fetch_url(&self.source).await?, true) - } else { - // Read from local file - let path = PathBuf::from(&self.source); - let content = std::fs::read_to_string(&path) - .with_context(|| format!("Failed to read file: {}", path.display()))?; - (content, false) - }; + // Validate and read the export data + let (json_content, is_from_url) = if self.source == "http" || self.source == "https" { + // Detect incomplete URL schemes + bail!( + "Error: '{}' looks like an incomplete URL.\n\n\ + Please provide a complete URL starting with 'http://' or 'https://'.\n\ + Example: cortex import https://example.com/session.json", + self.source + ); + } else if self.source.starts_with("http://") || self.source.starts_with("https://") { + // Fetch from URL + (fetch_url(&self.source).await?, true) + } else { + // Read from local file + let path = PathBuf::from(&self.source); + let content = std::fs::read_to_string(&path) + .with_context(|| format!("Failed to read file: {}", path.display()))?; + (content, false) + }; // Parse the export with helpful error messages let export: SessionExport = serde_json::from_str(&json_content).map_err(|e| { diff --git a/cortex-cli/src/mcp_cmd.rs b/cortex-cli/src/mcp_cmd.rs index 26ec37f0..b3e5dbe1 100644 --- a/cortex-cli/src/mcp_cmd.rs +++ b/cortex-cli/src/mcp_cmd.rs @@ -419,9 +419,16 @@ pub struct AuthListArgs { /// Arguments for logout command. #[derive(Debug, Parser)] +#[command( + about = "Remove OAuth credentials for an MCP server", + long_about = "Remove OAuth credentials for an MCP server.\n\n\ +You must specify either a server name or --all to remove all credentials.\n\n\ +Examples:\n cortex mcp logout my-server\n cortex mcp logout --all" +)] pub struct LogoutArgs { /// Name of the MCP server to remove credentials for. - #[arg(required_unless_present = "all")] + /// Required unless --all is specified. + #[arg(required_unless_present = "all", value_name = "SERVER_NAME")] pub name: Option, /// Remove OAuth credentials for all servers. diff --git a/cortex-cli/src/pr_cmd.rs b/cortex-cli/src/pr_cmd.rs index 0d976da1..d65b489c 100644 --- a/cortex-cli/src/pr_cmd.rs +++ b/cortex-cli/src/pr_cmd.rs @@ -109,9 +109,9 @@ async fn run_pr_checkout(args: PrCli) -> Result<()> { let repo_path = args.path.unwrap_or_else(|| PathBuf::from(".")); let pr_number = args.number; - // Validate PR number is positive + // Validate PR number is at least 1 if pr_number == 0 { - bail!("Error: PR number must be a positive integer"); + bail!("Error: PR number must be at least 1"); } // Change to repo directory diff --git a/cortex-cli/src/scrape_cmd.rs b/cortex-cli/src/scrape_cmd.rs index 272c39f5..358a9523 100644 --- a/cortex-cli/src/scrape_cmd.rs +++ b/cortex-cli/src/scrape_cmd.rs @@ -326,17 +326,36 @@ impl ScrapeCommand { eprintln!("Content-Type: {content_type}"); } - let html = response + let body = response .text() .await .context("Failed to read response body")?; if self.verbose { - eprintln!("Received {} bytes", html.len()); + eprintln!("Received {} bytes", body.len()); } + // Check if content is plain text (not HTML) + let is_plain_text = content_type.starts_with("text/plain") + || (!content_type.contains("html") && !body.trim_start().starts_with('<')); + // Parse and convert - let output = self.process_html(&html, format)?; + let output = if is_plain_text { + // For plain text content, return as-is without HTML processing + match format { + OutputFormat::Text | OutputFormat::Markdown => body, + OutputFormat::Html => { + // Escape HTML special characters + let escaped = body + .replace('&', "&") + .replace('<', "<") + .replace('>', ">"); + format!("
{}
", escaped) + } + } + } else { + self.process_html(&body, format)? + }; // Write output match &self.output { diff --git a/cortex-common/src/config_override.rs b/cortex-common/src/config_override.rs index ca2a1179..88db455d 100644 --- a/cortex-common/src/config_override.rs +++ b/cortex-common/src/config_override.rs @@ -18,8 +18,20 @@ pub struct CliConfigOverrides { impl CliConfigOverrides { /// Parse the raw overrides into key-value pairs. + /// Returns a tuple of (parsed_overrides, warnings). pub fn parse_overrides(&self) -> Result, String> { + self.parse_overrides_with_warnings() + .map(|(overrides, _)| overrides) + } + + /// Parse the raw overrides into key-value pairs, with optional warnings. + /// Returns a tuple of (parsed_overrides, warnings). + pub fn parse_overrides_with_warnings( + &self, + ) -> Result<(Vec<(String, toml::Value)>, Vec), String> { let mut result = Vec::new(); + let mut warnings = Vec::new(); + for raw in &self.raw_overrides { let parts: Vec<&str> = raw.splitn(2, '=').collect(); if parts.len() != 2 { @@ -43,10 +55,61 @@ impl CliConfigOverrides { toml::Value::String(value_str.to_string()) }; + // Validate model name if this is a model override + if key == "model" { + if let Some(warning) = validate_model_name(value_str) { + warnings.push(warning); + } + } + result.push((key, value)); } - Ok(result) + Ok((result, warnings)) + } +} + +/// Validate a model name and return a warning if it doesn't match known models. +fn validate_model_name(model: &str) -> Option { + use crate::model_presets::{MODEL_ALIASES, MODEL_PRESETS}; + + // Check if it's a known alias + if MODEL_ALIASES + .iter() + .any(|a| a.alias.eq_ignore_ascii_case(model)) + { + return None; + } + + // Check if it's a known model ID + if MODEL_PRESETS.iter().any(|p| p.id == model) { + return None; + } + + // Check if it looks like a provider/model format (e.g., "openai/gpt-4") + if model.contains('/') { + // Allow custom provider/model combinations without warning + return None; } + + // Unknown model - generate warning with suggestions + let model_lower = model.to_lowercase(); + let suggestions: Vec<&str> = MODEL_PRESETS + .iter() + .filter(|p| p.id.to_lowercase().contains(&model_lower)) + .take(3) + .map(|p| p.id) + .collect(); + + let suggestion_text = if !suggestions.is_empty() { + format!(" Did you mean: {}?", suggestions.join(", ")) + } else { + String::new() + }; + + Some(format!( + "Warning: Model '{}' is not a recognized model name.{}", + model, suggestion_text + )) } /// CLI configuration overrides.