Merge pull request #25 from dwalleck/feat/catalyst-cli-phase6-update-command

feat: Phase 6 - Update Command with Comprehensive Error Handling

Author: dwalleck

catalyst-cli/src/bin/catalyst.rs

@@ -31,6 +31,7 @@
 use anyhow::{Context, Result};
 use catalyst_cli::init;
 use catalyst_cli::types::{InitConfig, AVAILABLE_SKILLS, AVAILABLE_SKILLS_WITH_DESC};
+use catalyst_cli::update;
 use catalyst_cli::validation::check_binaries_installed;
 use catalyst_core::settings::*;
 use clap::{Parser, Subcommand};
@@ -548,12 +549,97 @@ fn main() -> Result<()> {
                 path.unwrap_or_else(|| env::current_dir().unwrap_or_else(|_| PathBuf::from(".")));
 
             if use_color {
-                println!("{}", "⚠️  Not implemented yet".yellow().bold());
+                println!("{}", "🔄 Updating Catalyst...".cyan().bold());
             } else {
-                println!("⚠️  Not implemented yet");
+                println!("🔄 Updating Catalyst...");
+            }
+            println!();
+
+            // Run update
+            let report = update::update(&target_dir, force)?;
+
+            // Display results
+            if report.updated_skills.is_empty()
+                && report.updated_hooks.is_empty()
+                && report.skipped_skills.is_empty()
+            {
+                if use_color {
+                    println!("{}", "✅ Already up to date!".green().bold());
+                } else {
+                    println!("✅ Already up to date!");
+                }
+            } else {
+                // Show updated hooks
+                if !report.updated_hooks.is_empty() {
+                    if use_color {
+                        println!("{}", "Updated hooks:".green().bold());
+                    } else {
+                        println!("Updated hooks:");
+                    }
+                    for hook in &report.updated_hooks {
+                        println!("  ✓ {}", hook);
+                    }
+                    println!();
+                }
+
+                // Show updated skills
+                if !report.updated_skills.is_empty() {
+                    if use_color {
+                        println!("{}", "Updated skills:".green().bold());
+                    } else {
+                        println!("Updated skills:");
+                    }
+                    for skill in &report.updated_skills {
+                        println!("  ✓ {}", skill);
+                    }
+                    println!();
+                }
+
+                // Show skipped skills
+                if !report.skipped_skills.is_empty() {
+                    if use_color {
+                        println!("{}", "Skipped skills (modified locally):".yellow().bold());
+                    } else {
+                        println!("Skipped skills (modified locally):");
+                    }
+                    for skipped in &report.skipped_skills {
+                        println!("  ⚠️  {} - {}", skipped.name, skipped.reason);
+                    }
+                    println!();
+                    if use_color {
+                        println!("{}", "  Use --force to overwrite modified skills".yellow());
+                    } else {
+                        println!("  Use --force to overwrite modified skills");
+                    }
+                    println!();
+                }
+
+                // Show errors
+                if !report.errors.is_empty() {
+                    if use_color {
+                        println!("{}", "Errors:".red().bold());
+                    } else {
+                        println!("Errors:");
+                    }
+                    for error in &report.errors {
+                        println!("  ❌ {}", error);
+                    }
+                    println!();
+                }
+
+                // Final status
+                if report.success {
+                    if use_color {
+                        println!("{}", "✅ Update completed successfully!".green().bold());
+                    } else {
+                        println!("✅ Update completed successfully!");
+                    }
+                } else if use_color {
+                    println!("{}", "⚠️  Update completed with errors".yellow().bold());
+                } else {
+                    println!("⚠️  Update completed with errors");
+                }
             }
-            println!("Would update: {:?}", target_dir);
-            println!("  Force: {}", force);
         }
 
         Commands::Settings { command } => {

catalyst-cli/src/init.rs

@@ -5,7 +5,7 @@
 
 use crate::types::{
     CatalystError, InitConfig, InitReport, Platform, Result, AGENTS_DIR, AVAILABLE_SKILLS,
-    CLAUDE_DIR, COMMANDS_DIR, HOOKS_DIR, SKILLS_DIR,
+    CATALYST_VERSION, CLAUDE_DIR, COMMANDS_DIR, HOOKS_DIR, SKILLS_DIR, VERSION_FILE,
 };
 use include_dir::{include_dir, Dir};
 use indicatif::{ProgressBar, ProgressStyle};
@@ -899,6 +899,54 @@ fn collect_file_hashes(
 /// # Returns
 ///
 /// Returns an `InitReport` with details of what was created
+///
+/// Write .catalyst-version file to track installation version
+///
+/// # Arguments
+///
+/// * `target_dir` - Directory where .catalyst-version should be created
+///
+/// # Returns
+///
+/// Returns Ok(()) on success
+pub fn write_version_file(target_dir: &Path) -> Result<()> {
+    let version_path = target_dir.join(VERSION_FILE);
+    fs::write(&version_path, format!("{}\n", CATALYST_VERSION)).map_err(|e| {
+        CatalystError::FileWriteFailed {
+            path: version_path.clone(),
+            source: e,
+        }
+    })?;
+    Ok(())
+}
+
+/// Read .catalyst-version file
+///
+/// # Arguments
+///
+/// * `target_dir` - Directory where .catalyst-version exists
+///
+/// # Returns
+///
+/// Returns the version string on success, None if file doesn't exist
+///
+/// # Implementation Note
+///
+/// Avoids TOCTOU (Time-of-Check-Time-of-Use) race by directly attempting
+/// to read the file instead of checking existence first.
+pub fn read_version_file(target_dir: &Path) -> Result<Option<String>> {
+    let version_path = target_dir.join(VERSION_FILE);
+
+    match fs::read_to_string(&version_path) {
+        Ok(content) => Ok(Some(content.trim().to_string())),
+        Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None),
+        Err(e) => Err(CatalystError::FileReadFailed {
+            path: version_path,
+            source: e,
+        }),
+    }
+}
+
 pub fn initialize(config: &InitConfig) -> Result<InitReport> {
     // Acquire lock to prevent concurrent init
     let _lock = acquire_init_lock(&config.directory)?;
@@ -950,6 +998,15 @@ pub fn initialize(config: &InitConfig) -> Result<InitReport> {
         }
     }
 
+    // Phase 6.1: Write .catalyst-version file to track installation
+    if let Err(e) = write_version_file(&config.directory) {
+        let warning = format!("⚠️  Failed to write .catalyst-version: {}", e);
+        eprintln!("{}", warning);
+        report.warnings.push(warning);
+    } else {
+        report.version_file_created = true;
+    }
+
     Ok(report)
 }
 
@@ -1517,4 +1574,99 @@ mod tests {
         assert!(hashes.is_object());
         assert!(!hashes.as_object().unwrap().is_empty());
     }
+
+    #[test]
+    fn test_read_version_file_success() {
+        let temp_dir = TempDir::new().unwrap();
+        let target = temp_dir.path();
+
+        // Write a version file
+        let version_path = target.join(VERSION_FILE);
+        fs::write(&version_path, "0.1.0\n").unwrap();
+
+        // Read it back
+        let result = read_version_file(target).unwrap();
+        assert_eq!(result, Some("0.1.0".to_string()));
+    }
+
+    #[test]
+    fn test_read_version_file_not_found() {
+        let temp_dir = TempDir::new().unwrap();
+        let target = temp_dir.path();
+
+        // No version file exists
+        let result = read_version_file(target).unwrap();
+        assert_eq!(result, None);
+    }
+
+    #[test]
+    #[cfg(unix)] // Only run on Unix systems that support file permissions
+    fn test_read_version_file_with_error_context() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let temp_dir = TempDir::new().unwrap();
+        let target = temp_dir.path();
+
+        // Create a version file
+        let version_path = target.join(VERSION_FILE);
+        fs::write(&version_path, "0.1.0\n").unwrap();
+
+        // Make it unreadable
+        fs::set_permissions(&version_path, fs::Permissions::from_mode(0o000)).unwrap();
+
+        // Try to read it - should fail with proper error context
+        let result = read_version_file(target);
+        assert!(result.is_err());
+        match result {
+            Err(CatalystError::FileReadFailed { path, source }) => {
+                assert_eq!(path, version_path);
+                assert_eq!(source.kind(), std::io::ErrorKind::PermissionDenied);
+            }
+            _ => panic!("Expected FileReadFailed with context"),
+        }
+
+        // Clean up - restore permissions so tempdir can be deleted
+        fs::set_permissions(&version_path, fs::Permissions::from_mode(0o644)).unwrap();
+    }
+
+    #[test]
+    fn test_write_version_file_success() {
+        let temp_dir = TempDir::new().unwrap();
+        let target = temp_dir.path();
+
+        // Write version file
+        write_version_file(target).unwrap();
+
+        // Verify it was written correctly
+        let version_path = target.join(VERSION_FILE);
+        assert!(version_path.exists());
+        let content = fs::read_to_string(&version_path).unwrap();
+        assert_eq!(content, format!("{}\n", CATALYST_VERSION));
+    }
+
+    #[test]
+    #[cfg(unix)] // Only run on Unix systems that support file permissions
+    fn test_write_version_file_with_error_context() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let temp_dir = TempDir::new().unwrap();
+        let target = temp_dir.path();
+
+        // Create a read-only directory
+        fs::set_permissions(target, fs::Permissions::from_mode(0o555)).unwrap();
+
+        // Try to write version file - should fail with proper error context
+        let result = write_version_file(target);
+        assert!(result.is_err());
+        match result {
+            Err(CatalystError::FileWriteFailed { path, source }) => {
+                assert_eq!(path, target.join(VERSION_FILE));
+                assert_eq!(source.kind(), std::io::ErrorKind::PermissionDenied);
+            }
+            _ => panic!("Expected FileWriteFailed with context"),
+        }
+
+        // Clean up - restore permissions so tempdir can be deleted
+        fs::set_permissions(target, fs::Permissions::from_mode(0o755)).unwrap();
+    }
 }

catalyst-cli/src/lib.rs

@@ -6,6 +6,7 @@
 pub mod init;
 pub mod status;
 pub mod types;
+pub mod update;
 pub mod validation;
 
 // Re-export commonly used types

catalyst-cli/src/types.rs

@@ -17,6 +17,27 @@ pub enum CatalystError {
     #[error("JSON serialization error: {0}")]
     Json(#[from] serde_json::Error),
 
+    #[error("Failed to read file {path}: {source}")]
+    FileReadFailed {
+        path: PathBuf,
+        #[source]
+        source: std::io::Error,
+    },
+
+    #[error("Failed to write file {path}: {source}")]
+    FileWriteFailed {
+        path: PathBuf,
+        #[source]
+        source: std::io::Error,
+    },
+
+    #[error("Failed to create directory {path}: {source}")]
+    DirectoryCreationFailed {
+        path: PathBuf,
+        #[source]
+        source: std::io::Error,
+    },
+
     #[error("Path not found: {0}")]
     PathNotFound(PathBuf),