fix: batch fix for cortex-cli bugs (exit codes, conflicting flags, JSON output, security) (#3)

* fix(cortex-cli): Apply batch of issue fixes and improvements

- Add validation for mutually exclusive flags across commands:
  - agent list: --primary vs --subagents
  - login: --token, --sso, --device-auth, --with-api-key
  - exec: --enabled-tools vs --disabled-tools
  - plugin list: --enabled vs --disabled
  - mcp add: --url vs --sse
  - mcp logout: --name vs --all

- Improve error handling with JSON support:
  - alias show: JSON error output
  - mcp get: JSON error output
  - plugin show: JSON error output
  - debug wait: JSON error output
  - run cmd: auth error JSON output

- Fix bug in autonomy.rs: is_read_only_command now uses command string
- Add session ID validation in lock_cmd
- Support 'last' as SESSION_ID in resume command
- Add duplicate task ID check in DAG helpers
- Add --jobs validation in DAG execute
- Add sort value validation in models list
- Add empty plugin name validation

- Update help text URLs to point to correct repo
- Replace emojis with text in PR checkout
- Move confirmation before backup in uninstall
- Convert GitHub URLs to raw URLs in upgrade changelog
- Support config.json in debug config command
- Add dry_run JSON output in compact and DAG commands

- Make run_whoami return Result<()> for proper error handling
- Change config get/unset to bail! on errors

Issues: #3646, #3651, #3682, #3696, #3700, #3716, #3722, #3815

* fix(cortex-cli): address validation feedback from PR review

- Fix clippy::bind_instead_of_map in debug_cmd/handlers/config.rs
  by replacing and_then() with map() since closure always returns Some()

- Fix duplicate error output in alias_cmd.rs and plugin_cmd.rs
  by using std::process::exit(1) after JSON error output instead of bail!()
  to avoid duplicating error message to stderr

- Replace raw .unwrap() calls with safer patterns in dag_cmd/helpers.rs
  using if-let guards and Option::and_then() instead of panicking

- Improve is_read_only_command() in exec_cmd/autonomy.rs to use
  exact word matching instead of prefix matching to prevent false
  positives (e.g., 'catfile' no longer matches 'cat')

* fix(cortex-cli): integrate allows_risk() for proper autonomy level validation

Address Greptile review feedback by actually calling the allows_risk() method
in the exec command approval flow. Previously, the security fix only added the
method but did not integrate it into the command execution path.

Changes:
- Replace simple AutonomyLevel::ReadOnly check with allows_risk(risk, command)
- Extract risk_level from sandbox_assessment if available
- Pass actual command string to validate read-only commands properly
- Provide clearer error messages including risk level and autonomy mode

Author: echobt

src/cortex-cli/src/agent_cmd/handlers/list.rs

@@ -1,6 +1,6 @@
 //! Handler for the `agent list` command.
 
-use anyhow::Result;
+use anyhow::{Result, bail};
 
 use crate::agent_cmd::cli::ListArgs;
 use crate::agent_cmd::loader::load_all_agents;
@@ -9,6 +9,13 @@ use crate::agent_cmd::utils::matches_pattern;
 
 /// List agents command.
 pub async fn run_list(args: ListArgs) -> Result<()> {
+    // Validate mutually exclusive flags
+    if args.primary && args.subagents {
+        bail!(
+            "Cannot specify both --primary and --subagents. Choose one filter or use neither for all agents."
+        );
+    }
+
     // Handle --remote flag
     if args.remote {
         println!("Remote agent registry:");

src/cortex-cli/src/alias_cmd.rs

@@ -241,10 +241,20 @@ async fn run_remove(args: AliasRemoveArgs) -> Result<()> {
 async fn run_show(args: AliasShowArgs) -> Result<()> {
     let config = load_aliases()?;
 
-    let alias = config
-        .aliases
-        .get(&args.name)
-        .ok_or_else(|| anyhow::anyhow!("Alias '{}' does not exist.", args.name))?;
+    let alias = match config.aliases.get(&args.name) {
+        Some(a) => a,
+        None => {
+            if args.json {
+                let error = serde_json::json!({
+                    "error": format!("Alias '{}' does not exist", args.name)
+                });
+                println!("{}", serde_json::to_string_pretty(&error)?);
+                // Exit with error code but don't duplicate error message via bail!()
+                std::process::exit(1);
+            }
+            bail!("Alias '{}' does not exist.", args.name);
+        }
+    };
 
     if args.json {
         println!("{}", serde_json::to_string_pretty(alias)?);

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

@@ -27,7 +27,7 @@ pub async fn dispatch_command(cli: Cli) -> Result<()> {
         Some(Commands::Login(login_cli)) => handle_login(login_cli).await,
         Some(Commands::Logout(logout_cli)) => handle_logout(logout_cli).await,
         Some(Commands::Whoami) => {
-            run_whoami().await;
+            run_whoami().await?;
             Ok(())
         }
         Some(Commands::Mcp(mcp_cli)) => mcp_cli.run().await,
@@ -182,6 +182,23 @@ async fn run_init(init_cli: InitCommand) -> Result<()> {
 
 /// Handle login command.
 async fn handle_login(login_cli: LoginCommand) -> Result<()> {
+    // Validate mutually exclusive authentication methods
+    let auth_methods_count = [
+        login_cli.token.is_some(),
+        login_cli.use_sso,
+        login_cli.use_device_code,
+        login_cli.with_api_key,
+    ]
+    .iter()
+    .filter(|&&x| x)
+    .count();
+
+    if auth_methods_count > 1 {
+        bail!(
+            "Cannot specify multiple authentication methods. Choose one: --token, --sso, --device-auth, or --with-api-key."
+        );
+    }
+
     match login_cli.action {
         Some(LoginSubcommand::Status) => run_login_status(login_cli.config_overrides).await,
         None => {
@@ -512,7 +529,7 @@ fn install_completions(shell: Shell) -> Result<()> {
 // ============================================================================
 
 /// Show current logged-in user.
-pub async fn run_whoami() {
+pub async fn run_whoami() -> Result<()> {
     use cortex_login::{AuthMode, load_auth_with_fallback, safe_format_key};
 
     let cortex_home = dirs::home_dir()
@@ -527,7 +544,7 @@ pub async fn run_whoami() {
             "Authenticated via CORTEX_AUTH_TOKEN: {}",
             safe_format_key(&token)
         );
-        return;
+        return Ok(());
     }
 
     if let Ok(token) = std::env::var("CORTEX_API_KEY")
@@ -537,7 +554,7 @@ pub async fn run_whoami() {
             "Authenticated via CORTEX_API_KEY: {}",
             safe_format_key(&token)
         );
-        return;
+        return Ok(());
     }
 
     // Load stored credentials
@@ -565,9 +582,11 @@ pub async fn run_whoami() {
             println!("Not logged in. Run 'cortex login' to authenticate.");
         }
         Err(e) => {
-            eprintln!("Error checking login status: {}", e);
+            return Err(anyhow::anyhow!("Error checking login status: {}", e));
         }
     }
+
+    Ok(())
 }
 
 /// Resume a previous session.
@@ -585,6 +604,16 @@ pub async fn run_resume(resume_cli: ResumeCommand) -> Result<()> {
     let config = cortex_engine::Config::default();
 
     let id_str = match (resume_cli.session_id, resume_cli.last, resume_cli.pick) {
+        // Support "last" as SESSION_ID as documented in help text (Issue #3646)
+        (Some(id), _, _) if id.to_lowercase() == "last" => {
+            let sessions = cortex_engine::list_sessions(&config.cortex_home)?;
+            if sessions.is_empty() {
+                print_info("No sessions found to resume.");
+                return Ok(());
+            }
+            print_info("Resuming most recent session...");
+            sessions[0].id.clone()
+        }
         (Some(id), _, _) => id,
         (None, true, _) => {
             let sessions = cortex_engine::list_sessions(&config.cortex_home)?;
@@ -810,10 +839,10 @@ pub async fn show_config(config_cli: ConfigCommand) -> Result<()> {
                 if let Some(value) = config.get(&args.key) {
                     println!("{}", value);
                 } else {
-                    println!("Key '{}' not found.", args.key);
+                    bail!("Key '{}' not found in configuration", args.key);
                 }
             } else {
-                println!("No configuration file found.");
+                bail!("No configuration file found");
             }
         }
         Some(ConfigSubcommand::Set(args)) => {
@@ -841,10 +870,10 @@ pub async fn show_config(config_cli: ConfigCommand) -> Result<()> {
                     std::fs::write(&config_path, content)?;
                     print_success(&format!("Removed key: {}", args.key));
                 } else {
-                    print_warning(&format!("Key '{}' not found.", args.key));
+                    bail!("Key '{}' not found in configuration", args.key);
                 }
             } else {
-                print_warning("No configuration file found.");
+                bail!("No configuration file found");
             }
         }
         None => {

src/cortex-cli/src/cli/styles.rs

@@ -49,8 +49,8 @@ pub const AFTER_HELP: &str = color_print::cstr!(
     <dim>Agents</>      ~/.cortex/agents/ (personal), .cortex/agents/ (project)
 
 <cyan,bold>🔗 LEARN MORE</>
-    <blue,underline>https://docs.cortex.foundation</>         Documentation
-    <blue,underline>https://github.com/CortexLM/cortex-cli</>  Source & Issues"#
+    <blue,underline>https://docs.cortex.foundation</>       Documentation
+    <blue,underline>https://github.com/CortexLM/cortex</>   Source & Issues"#
 );
 
 /// Before-help section with ASCII art banner.

src/cortex-cli/src/compact_cmd.rs

@@ -407,33 +407,32 @@ async fn run_compact(args: CompactRunArgs) -> Result<()> {
     }
 
     if args.dry_run {
-        println!("Dry run mode - no changes will be made.");
-        println!();
+        let log_files_count = dir_stats(&logs_dir).0;
+        let session_files_count = dir_stats(&sessions_dir).0;
+        let history_files_count = dir_stats(&history_dir).0;
+        let orphaned_history_count = count_orphaned_history(&sessions_dir, &history_dir);
 
-        let status = CompactionStatus {
-            data_dir: data_dir.clone(),
-            logs_dir: logs_dir.clone(),
-            sessions_dir: sessions_dir.clone(),
-            history_dir: history_dir.clone(),
-            log_files_count: dir_stats(&logs_dir).0,
-            log_files_size: dir_stats(&logs_dir).1,
-            log_files_size_human: format_size(dir_stats(&logs_dir).1),
-            session_files_count: dir_stats(&sessions_dir).0,
-            history_files_count: dir_stats(&history_dir).0,
-            orphaned_history_count: count_orphaned_history(&sessions_dir, &history_dir),
-            total_data_size: 0,
-            total_data_size_human: String::new(),
-            lock_held: false,
-        };
-
-        println!("Would process:");
-        println!("  Log files: {}", status.log_files_count);
-        println!("  Session files: {}", status.session_files_count);
-        println!("  History files: {}", status.history_files_count);
-        println!(
-            "  Orphaned files to clean: {}",
-            status.orphaned_history_count
-        );
+        if args.json {
+            let output = serde_json::json!({
+                "dry_run": true,
+                "would_process": {
+                    "log_files": log_files_count,
+                    "session_files": session_files_count,
+                    "history_files": history_files_count,
+                    "orphaned_files_to_clean": orphaned_history_count
+                },
+                "message": "Dry run completed - no changes made"
+            });
+            println!("{}", serde_json::to_string_pretty(&output)?);
+        } else {
+            println!("Dry run mode - no changes will be made.");
+            println!();
+            println!("Would process:");
+            println!("  Log files: {}", log_files_count);
+            println!("  Session files: {}", session_files_count);
+            println!("  History files: {}", history_files_count);
+            println!("  Orphaned files to clean: {}", orphaned_history_count);
+        }
         return Ok(());
     }
 

src/cortex-cli/src/dag_cmd/commands.rs

@@ -71,6 +71,11 @@ pub async fn run_create(args: DagCreateArgs) -> Result<()> {
 
 /// Execute a DAG.
 pub async fn run_execute(args: DagRunArgs) -> Result<()> {
+    // Validate --jobs argument (Issue #3716)
+    if args.max_concurrent == 0 {
+        bail!("--jobs must be at least 1. Use --jobs 1 for sequential execution.");
+    }
+
     let spec = load_spec(&args.file)?;
     let specs = convert_specs(&spec);
 
@@ -86,14 +91,33 @@ pub async fn run_execute(args: DagRunArgs) -> Result<()> {
 
     if matches!(args.strategy, ExecutionStrategy::DryRun) {
         // Dry run - just validate
-        dag.topological_sort()
+        let order = dag
+            .topological_sort()
             .map_err(|e| anyhow::anyhow!("DAG validation failed: {}", e))?;
-        print_success(&format!(
-            "✓ DAG is valid ({} tasks in execution order)",
-            dag.len()
-        ));
-        println!();
-        print_execution_order(&dag)?;
+
+        match args.format {
+            DagOutputFormat::Json => {
+                let task_names: Vec<String> = order
+                    .iter()
+                    .filter_map(|id| dag.get_task(*id).map(|t| t.name.clone()))
+                    .collect();
+                let output = serde_json::json!({
+                    "dry_run": true,
+                    "valid": true,
+                    "task_count": dag.len(),
+                    "execution_order": task_names
+                });
+                println!("{}", serde_json::to_string_pretty(&output)?);
+            }
+            _ => {
+                print_success(&format!(
+                    "✓ DAG is valid ({} tasks in execution order)",
+                    dag.len()
+                ));
+                println!();
+                print_execution_order(&dag)?;
+            }
+        }
         return Ok(());
     }
 
@@ -251,7 +275,14 @@ pub async fn run_list(args: DagListArgs) -> Result<()> {
     let ids = store.list().await?;
 
     if ids.is_empty() {
-        print_info("No DAGs found");
+        match args.format {
+            DagOutputFormat::Json => {
+                println!("[]");
+            }
+            _ => {
+                print_info("No DAGs found");
+            }
+        }
         return Ok(());
     }
 

src/cortex-cli/src/dag_cmd/helpers.rs

@@ -28,6 +28,17 @@ pub fn load_spec(path: &PathBuf) -> Result<DagSpecInput> {
         serde_yaml::from_str(&content).context("Failed to parse YAML")?
     };
 
+    // Check for duplicate task IDs (Issue #3815)
+    let mut seen_names = std::collections::HashSet::new();
+    for task in &spec.tasks {
+        if !seen_names.insert(&task.name) {
+            anyhow::bail!(
+                "Duplicate task ID '{}' found in DAG specification. Task IDs must be unique.",
+                task.name
+            );
+        }
+    }
+
     Ok(spec)
 }
 
@@ -67,8 +78,9 @@ pub fn convert_specs(input: &DagSpecInput) -> Vec<TaskSpec> {
 pub fn print_dag_summary(dag: &TaskDag) {
     println!("Tasks:");
     for task in dag.all_tasks() {
-        let deps = dag
-            .get_dependencies(task.id.unwrap())
+        let deps = task
+            .id
+            .and_then(|id| dag.get_dependencies(id))
             .map(|d| d.len())
             .unwrap_or(0);
         let dep_str = if deps > 0 {
@@ -119,7 +131,11 @@ pub fn print_execution_summary(stats: &DagExecutionStats, format: DagOutputForma
                 "skipped": stats.skipped_tasks,
                 "duration_secs": stats.total_duration.as_secs_f64()
             });
-            println!("{}", serde_json::to_string_pretty(&output).unwrap());
+            println!(
+                "{}",
+                serde_json::to_string_pretty(&output)
+                    .expect("JSON serialization should not fail for DagExecutionStats")
+            );
         }
         DagOutputFormat::Compact => {
             println!(
@@ -170,7 +186,7 @@ pub fn print_ascii_graph(dag: &TaskDag, spec: &DagSpecInput) {
         for task_id in tasks_at_depth {
             if let Some(task) = dag.get_task(*task_id) {
                 let deps = dag.get_dependencies(*task_id);
-                let arrow = if deps.map(|d| !d.is_empty()).unwrap_or(false) {
+                let arrow = if deps.is_some_and(|d| !d.is_empty()) {
                     "└─► "
                 } else {
                     "● "
@@ -195,7 +211,7 @@ pub fn print_dot_graph(dag: &TaskDag, spec: &DagSpecInput) {
     println!();
 
     for task in dag.all_tasks() {
-        let task_id = task.id.unwrap();
+        let Some(task_id) = task.id else { continue };
         let color = match task.status {
             TaskStatus::Completed => "green",
             TaskStatus::Failed => "red",
@@ -214,7 +230,7 @@ pub fn print_dot_graph(dag: &TaskDag, spec: &DagSpecInput) {
     println!();
 
     for task in dag.all_tasks() {
-        let task_id = task.id.unwrap();
+        let Some(task_id) = task.id else { continue };
         if let Some(deps) = dag.get_dependencies(task_id) {
             for dep_id in deps {
                 println!("  \"{}\" -> \"{}\";", dep_id.inner(), task_id.inner());
@@ -234,14 +250,14 @@ pub fn print_mermaid_graph(dag: &TaskDag, spec: &DagSpecInput) {
     // Create task ID to safe name mapping
     let mut id_to_name: HashMap<TaskId, String> = HashMap::new();
     for task in dag.all_tasks() {
-        let task_id = task.id.unwrap();
+        let Some(task_id) = task.id else { continue };
         let safe_name = task.name.replace([' ', '-'], "_");
         id_to_name.insert(task_id, safe_name.clone());
         println!("    {}[{}]", safe_name, task.name);
     }
 
     for task in dag.all_tasks() {
-        let task_id = task.id.unwrap();
+        let Some(task_id) = task.id else { continue };
         if let Some(deps) = dag.get_dependencies(task_id) {
             for dep_id in deps {
                 if let (Some(from), Some(to)) = (id_to_name.get(dep_id), id_to_name.get(&task_id)) {