fix(pi): treat stale skill cleanup as post-commit state

src-tauri/src/services/skill_deployment.rs

@@ -84,7 +84,7 @@ impl PiSkillDeploymentService {
                 existing,
                 Some(true),
             )?;
-            cleanup_stale_deployments(db, skill, &destination_key)?;
+            cleanup_stale_deployments_after_commit(db, skill, &destination_key);
         } else {
             remove_all_recorded_deployments(db, skill, Some(false))?;
         }
@@ -124,7 +124,7 @@ impl PiSkillDeploymentService {
                     Some(true),
                 )?;
             }
-            cleanup_stale_deployments(db, skill, &destination_key)?;
+            cleanup_stale_deployments_after_commit(db, skill, &destination_key);
         } else {
             remove_all_recorded_deployments(db, skill, Some(false))?;
         }
@@ -411,7 +411,8 @@ fn reconcile_skill_unlocked(db: &Arc<Database>, skill: &InstalledSkill) -> Resul
             existing,
             None,
         )?;
-        cleanup_stale_deployments(db, skill, &destination_key)
+        cleanup_stale_deployments_after_commit(db, skill, &destination_key);
+        Ok(())
     } else {
         remove_all_recorded_deployments(db, skill, None)
     }
@@ -430,6 +431,25 @@ fn cleanup_stale_deployments(
     Ok(())
 }
 
+/// `deploy` atomically commits the new native destination, its ownership
+/// ledger, and (when requested) desired state before old-root cleanup begins.
+/// A drifted old root must remain visible as stale ownership evidence, but it
+/// must not turn that committed publication into an error: several callers
+/// compensate a returned error by deleting the Skill's SSOT/database row.
+fn cleanup_stale_deployments_after_commit(
+    db: &Arc<Database>,
+    skill: &InstalledSkill,
+    current_destination_key: &str,
+) {
+    if let Err(error) = cleanup_stale_deployments(db, skill, current_destination_key) {
+        log::warn!(
+            "Pi Skill '{}' was published at its current agent root, but stale deployment cleanup \
+             remains pending: {error}",
+            skill.id
+        );
+    }
+}
+
 fn remove_all_recorded_deployments(
     db: &Arc<Database>,
     skill: &InstalledSkill,
@@ -1238,4 +1258,96 @@ mod tests {
             destination_key(&new_destination)
         );
     }
+
+    #[test]
+    #[serial_test::serial]
+    fn drifted_old_root_is_post_commit_status_not_a_failed_new_publication() {
+        struct EnvGuard {
+            key: &'static str,
+            previous: Option<std::ffi::OsString>,
+        }
+        impl EnvGuard {
+            fn set(key: &'static str, value: &Path) -> Self {
+                let previous = std::env::var_os(key);
+                std::env::set_var(key, value);
+                Self { key, previous }
+            }
+        }
+        impl Drop for EnvGuard {
+            fn drop(&mut self) {
+                match self.previous.take() {
+                    Some(value) => std::env::set_var(self.key, value),
+                    None => std::env::remove_var(self.key),
+                }
+                if self.key == "CC_SWITCH_TEST_HOME" {
+                    let _ = crate::settings::reload_settings();
+                }
+            }
+        }
+
+        let temp = tempfile::tempdir().expect("tempdir");
+        let _home = EnvGuard::set("CC_SWITCH_TEST_HOME", temp.path());
+        crate::settings::reload_settings().expect("reload settings");
+        let old_root = temp.path().join("old-pi");
+        let new_root = temp.path().join("new-pi");
+        let _pi_root = EnvGuard::set("PI_CODING_AGENT_DIR", &old_root);
+        let source = SkillService::get_ssot_dir()
+            .expect("SSOT")
+            .join("drifted-relocation");
+        fs::create_dir_all(&source).expect("source");
+        fs::write(
+            source.join("SKILL.md"),
+            "---\nname: drifted-relocation\ndescription: relocation test\n---\n",
+        )
+        .expect("manifest");
+        let mut skill = InstalledSkill {
+            id: "local:drifted-relocation".to_string(),
+            name: "Drifted relocation".to_string(),
+            description: Some("relocation test".to_string()),
+            directory: "drifted-relocation".to_string(),
+            repo_owner: None,
+            repo_name: None,
+            repo_branch: None,
+            readme_url: None,
+            apps: SkillApps::only(&AppType::Pi),
+            installed_at: 1,
+            content_hash: None,
+            updated_at: 1,
+        };
+        let db = Arc::new(Database::memory().expect("database"));
+        db.save_skill(&skill).expect("save skill");
+        PiSkillDeploymentService::reconcile_skill(&db, &skill).expect("old deployment");
+        let old_destination = old_root.join("skills").join(&skill.directory);
+        remove_path(&old_destination).expect("replace old deployment");
+        fs::create_dir_all(&old_destination).expect("foreign old destination");
+        fs::write(old_destination.join("external.txt"), "external").expect("external drift");
+
+        std::env::set_var("PI_CODING_AGENT_DIR", &new_root);
+        PiSkillDeploymentService::toggle(&db, &mut skill, true)
+            .expect("the new publication is already committed");
+
+        let new_destination = new_root.join("skills").join(&skill.directory);
+        assert!(fs::symlink_metadata(&new_destination).is_ok());
+        assert_eq!(
+            fs::read_to_string(old_destination.join("external.txt")).expect("external survives"),
+            "external"
+        );
+        let stored = db
+            .get_installed_skill(&skill.id)
+            .expect("read desired state")
+            .expect("skill remains");
+        assert!(stored.apps.pi);
+        assert_eq!(
+            db.get_pi_skill_deployments(&skill.id)
+                .expect("ledger")
+                .len(),
+            2,
+            "old drift evidence and the committed new deployment must both remain"
+        );
+        let status =
+            PiSkillDeploymentService::inspect_all(&db).expect("inspect")[&skill.id].clone();
+        assert_eq!(status.ownership, PiSkillOwnership::Stale);
+        assert!(status.effectively_discovered);
+        assert!(status.issue.is_some_and(|issue| issue.contains("previous")));
+    }
 }