Address comments

Author: kkashilk

crates/chat-cli/src/cli/migrate.rs

@@ -18,7 +18,6 @@ use serde_json::{
     Value,
 };
 use tokio::fs;
-use tracing::debug;
 
 use crate::os::Os;
 use crate::util::paths::GlobalPaths;
@@ -39,35 +38,20 @@ pub struct MigrateArgs {
 }
 
 impl MigrateArgs {
-    pub async fn execute(self, _os: &mut Os) -> Result<ExitCode> {
+    pub async fn execute(self, os: &mut Os) -> Result<ExitCode> {
         // Try to acquire migration lock
         let _lock = match acquire_migration_lock()? {
             Some(lock) => lock,
             None => {
-                // Another process is migrating, skip silently
+                println!("Migration already in progress by another process");
                 return Ok(ExitCode::SUCCESS);
             },
         };
 
-        if !self.yes && !self.dry_run {
-            println!("This will migrate your database and settings from amazon-q to kiro-cli.");
-            println!("\nMigration details:");
-            println!("  • Database: {{data_local_dir}}/amazon-q → {{data_local_dir}}/kiro-cli");
-            println!("  • Settings: {{data_local_dir}}/amazon-q → ~/.aws/kiro-cli");
-            println!("\nContinue? (y/N)");
-
-            let mut input = String::new();
-            std::io::stdin().read_line(&mut input)?;
-            if !input.trim().eq_ignore_ascii_case("y") {
-                println!("Migration cancelled");
-                return Ok(ExitCode::SUCCESS);
-            }
-        }
-
-        let status = detect_migration()?;
+        let status = detect_migration(os).await?;
 
         if !self.force && matches!(status, MigrationStatus::Completed) {
-            debug!("✓ Migration already completed");
+            println!("✓ Migration already completed");
             return Ok(ExitCode::SUCCESS);
         }
 
@@ -78,26 +62,47 @@ impl MigrateArgs {
             new_settings,
         } = status
         else {
-            debug!("✓ No migration needed (fresh install)");
+            println!("✓ No migration needed (fresh install)");
             return Ok(ExitCode::SUCCESS);
         };
 
+        if !self.yes && !self.dry_run {
+            println!("This will migrate your database and settings from amazon-q to kiro-cli.");
+            println!("\nMigration details:");
+            println!("  • Database: {{data_local_dir}}/amazon-q → {{data_local_dir}}/kiro-cli");
+            println!("  • Settings: {{data_local_dir}}/amazon-q → ~/.aws/kiro-cli");
+            println!("\nContinue? (y/N)");
+
+            let mut input = String::new();
+            std::io::stdin().read_line(&mut input)?;
+            if !input.trim().eq_ignore_ascii_case("y") {
+                println!("Migration cancelled");
+                return Ok(ExitCode::SUCCESS);
+            }
+        }
+
         // Migrate database
         let db_result = migrate_database(&old_db, &new_db, self.dry_run).await?;
-        debug!("✓ Database: {}", db_result.message);
+        println!("✓ Database: {}", db_result.message);
+
+        // Reload database connection after copying the file
+        if !self.dry_run && db_result.bytes_copied > 0 {
+            os.database = crate::database::Database::new().await?;
+        }
 
         // Migrate settings
         let settings_result = migrate_settings(&old_settings, &new_settings, self.dry_run).await?;
-        debug!("✓ Settings: {}", settings_result.message);
+        println!("✓ Settings: {}", settings_result.message);
         if !settings_result.transformations.is_empty() {
-            debug!("  Transformations applied:");
+            println!("  Transformations applied:");
             for t in &settings_result.transformations {
-                debug!("    - {t}");
+                println!("    - {t}");
             }
         }
 
         if !self.dry_run {
-            debug!("\n✓ Migration completed successfully!");
+            os.database.set_kiro_migration_completed()?;
+            println!("\n✓ Migration completed successfully!");
         } else {
             println!("\n(Dry run - no changes made)");
         }
@@ -119,14 +124,14 @@ enum MigrationStatus {
     Completed,
 }
 
-fn detect_migration() -> Result<MigrationStatus> {
+async fn detect_migration(os: &mut Os) -> Result<MigrationStatus> {
     let old_db = GlobalPaths::old_database_path()?;
     let old_settings = GlobalPaths::old_settings_path()?;
-    let new_db = GlobalPaths::new_database_path()?;
-    let new_settings = GlobalPaths::new_settings_path()?;
+    let new_db = GlobalPaths::database_path()?;
+    let new_settings = GlobalPaths::settings_path()?;
 
     let old_exists = old_db.exists() || old_settings.exists();
-    let migration_completed = GlobalPaths::is_migration_completed_static().unwrap_or(false);
+    let migration_completed = os.database.is_kiro_migration_completed().unwrap_or(false);
 
     if migration_completed {
         Ok(MigrationStatus::Completed)
@@ -180,7 +185,7 @@ async fn migrate_database(old_path: &Path, new_path: &Path, dry_run: bool) -> Re
     if let Some(parent) = new_path.parent() {
         fs::create_dir_all(parent)
             .await
-            .context("Failed to create target directory")?;
+            .context(format!("Failed to create target directory: {}", parent.display()))?;
     }
 
     let bytes = fs::copy(old_path, new_path).await.context("Failed to copy database")?;
@@ -248,7 +253,7 @@ async fn migrate_settings(old_path: &Path, new_path: &Path, dry_run: bool) -> Re
     if let Some(parent) = new_path.parent() {
         fs::create_dir_all(parent)
             .await
-            .context("Failed to create target directory")?;
+            .context(format!("Failed to create target directory: {}", parent.display()))?;
     }
 
     let json = serde_json::to_string_pretty(&transformed).context("Failed to serialize settings")?;
@@ -257,14 +262,25 @@ async fn migrate_settings(old_path: &Path, new_path: &Path, dry_run: bool) -> Re
         .context("Failed to write settings file")?;
 
     Ok(SettingsMigrationResult {
-        message: format!("Settings migrated successfully ({} settings)", transformed.len()),
+        message: "Settings migrated successfully".to_string(),
         settings_count: transformed.len(),
         transformations,
     })
 }
 
 // File locking
-fn acquire_migration_lock() -> Result<Option<std::fs::File>> {
+struct MigrationLock {
+    _file: std::fs::File,
+    path: PathBuf,
+}
+
+impl Drop for MigrationLock {
+    fn drop(&mut self) {
+        let _ = std::fs::remove_file(&self.path);
+    }
+}
+
+fn acquire_migration_lock() -> Result<Option<MigrationLock>> {
     let lock_path = GlobalPaths::migration_lock_path()?;
 
     if let Some(parent) = lock_path.parent() {
@@ -275,10 +291,38 @@ fn acquire_migration_lock() -> Result<Option<std::fs::File>> {
         .create(true)
         .write(true)
         .truncate(false)
-        .open(lock_path)?;
+        .open(&lock_path)?;
 
     match flock(&file, FlockOperation::NonBlockingLockExclusive) {
-        Ok(()) => Ok(Some(file)),
-        Err(_) => Ok(None),
+        Ok(()) => Ok(Some(MigrationLock {
+            _file: file,
+            path: lock_path,
+        })),
+        Err(_) => {
+            // Check if lock is stale (older than 1 minute)
+            if let Ok(metadata) = std::fs::metadata(&lock_path) {
+                if let Ok(modified) = metadata.modified() {
+                    if let Ok(elapsed) = modified.elapsed() {
+                        if elapsed.as_secs() > 60 {
+                            // Stale lock - remove and retry
+                            std::fs::remove_file(&lock_path)?;
+                            let file = std::fs::OpenOptions::new()
+                                .create(true)
+                                .write(true)
+                                .truncate(false)
+                                .open(&lock_path)?;
+                            return match flock(&file, FlockOperation::NonBlockingLockExclusive) {
+                                Ok(()) => Ok(Some(MigrationLock {
+                                    _file: file,
+                                    path: lock_path,
+                                })),
+                                Err(_) => Ok(None),
+                            };
+                        }
+                    }
+                }
+            }
+            Ok(None)
+        },
     }
 }

crates/chat-cli/src/database/mod.rs

@@ -63,6 +63,7 @@ const IDC_REGION_KEY: &str = "auth.idc.region";
 const CUSTOMIZATION_STATE_KEY: &str = "api.selectedCustomization";
 const PROFILE_MIGRATION_KEY: &str = "profile.Migrated";
 const HEARTBEAT_DATE_KEY: &str = "telemetry.lastHeartbeatDate";
+const KIRO_MIGRATION_KEY: &str = "migration.kiro.completed";
 
 const MIGRATIONS: &[Migration] = migrations![
     "000_migration_table",
@@ -355,6 +356,19 @@ impl Database {
         Ok(())
     }
 
+    /// Check if kiro migration has been completed
+    pub fn is_kiro_migration_completed(&self) -> Result<bool, DatabaseError> {
+        Ok(self
+            .get_entry::<bool>(Table::State, KIRO_MIGRATION_KEY)?
+            .unwrap_or(false))
+    }
+
+    /// Mark kiro migration as completed
+    pub fn set_kiro_migration_completed(&self) -> Result<(), DatabaseError> {
+        self.set_entry(Table::State, KIRO_MIGRATION_KEY, true)?;
+        Ok(())
+    }
+
     // /// Get the model id used for last conversation state.
     // pub fn get_last_used_model_id(&self) -> Result<Option<String>, DatabaseError> {
     //     self.get_json_entry::<String>(Table::State, LAST_USED_MODEL_ID)

crates/chat-cli/src/util/paths.rs

@@ -313,17 +313,9 @@ impl<'a> GlobalPaths<'a> {
 
     /// Get settings path (new location with fallback to old)
     pub fn settings_path() -> Result<PathBuf> {
-        let new_path = Self::new_settings_path()?;
-        if new_path.exists() {
-            return Ok(new_path);
-        }
-
-        let old_path = Self::old_settings_path()?;
-        if old_path.exists() {
-            return Ok(old_path);
-        }
-
-        Ok(new_path)
+        Ok(dirs::home_dir()
+            .ok_or(DirectoryError::NoHomeDirectory)?
+            .join(global::SETTINGS_FILE))
     }
 
     pub fn mcp_auth_dir(&self) -> Result<PathBuf> {
@@ -332,32 +324,17 @@ impl<'a> GlobalPaths<'a> {
 
     /// Get database path (new location with fallback to old)
     pub fn database_path() -> Result<PathBuf> {
-        let new_path = Self::new_database_path()?;
-        if new_path.exists() {
-            return Ok(new_path);
-        }
-
-        let old_path = Self::old_database_path()?;
-        if old_path.exists() {
-            return Ok(old_path);
-        }
-
-        Ok(new_path)
-    }
-
-    /// Get old database path (amazon-q location)
-    pub fn old_database_path() -> Result<PathBuf> {
         Ok(dirs::data_local_dir()
             .ok_or(DirectoryError::NoHomeDirectory)?
-            .join("amazon-q")
+            .join("kiro-cli")
             .join("data.sqlite3"))
     }
 
-    /// Get new database path (kiro-cli location)
-    pub fn new_database_path() -> Result<PathBuf> {
+    /// Get old database path (amazon-q location)
+    pub fn old_database_path() -> Result<PathBuf> {
         Ok(dirs::data_local_dir()
             .ok_or(DirectoryError::NoHomeDirectory)?
-            .join("kiro-cli")
+            .join("amazon-q")
             .join("data.sqlite3"))
     }
 
@@ -369,31 +346,6 @@ impl<'a> GlobalPaths<'a> {
             .join("settings.json"))
     }
 
-    /// Get new settings path (kiro-cli location in home)
-    pub fn new_settings_path() -> Result<PathBuf> {
-        Ok(dirs::home_dir()
-            .ok_or(DirectoryError::NoHomeDirectory)?
-            .join(global::SETTINGS_FILE))
-    }
-
-    /// Check if migration completed by reading settings file
-    pub fn is_migration_completed_static() -> Result<bool> {
-        let new_settings = Self::new_settings_path()?;
-        if !new_settings.exists() {
-            return Ok(false);
-        }
-
-        let content = std::fs::read_to_string(&new_settings)?;
-        let Ok(settings) = serde_json::from_str::<serde_json::Value>(&content) else {
-            return Ok(false);
-        };
-
-        Ok(settings
-            .get("migration.kiro.completed")
-            .and_then(|c| c.as_bool())
-            .unwrap_or(false))
-    }
-
     /// Get migration lock file path
     pub fn migration_lock_path() -> Result<PathBuf> {
         Ok(dirs::home_dir()