Fix Ubuntu CI race condition in skills_installer test

The test_install_and_uninstall_skills_lifecycle test was failing on
Ubuntu CI because codex tests temporarily replace HOME with a tempdir
path. With 8 parallel test threads, the lifecycle test could call
home_dir() while HOME pointed to a codex test's temp directory that
gets dropped mid-execution, causing ENOENT errors.

Add #[serial] to serialize the test with codex tests and wrap the
test body in with_temp_home for full HOME isolation, matching the
pattern already used by codex tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: jwiegley

src/mdm/skills_installer.rs

@@ -256,6 +256,40 @@ pub fn uninstall_skills(dry_run: bool, _verbose: bool) -> Result<SkillsInstallRe
 #[cfg(test)]
 mod tests {
     use super::*;
+    use serial_test::serial;
+
+    /// Temporarily override HOME (and USERPROFILE on Windows) to a temp directory
+    /// for the duration of the closure. Must only be called from `#[serial]` tests
+    /// to avoid racing with other tests that read HOME.
+    fn with_temp_home<F: FnOnce(&std::path::Path)>(f: F) {
+        let temp = tempfile::tempdir().unwrap();
+        let home = temp.path().to_path_buf();
+
+        let prev_home = std::env::var_os("HOME");
+        let prev_userprofile = std::env::var_os("USERPROFILE");
+
+        // SAFETY: tests using this helper are serialized via #[serial],
+        // so mutating the process environment is safe.
+        unsafe {
+            std::env::set_var("HOME", &home);
+            std::env::set_var("USERPROFILE", &home);
+        }
+
+        f(&home);
+
+        // SAFETY: tests using this helper are serialized via #[serial],
+        // so restoring the process environment is safe.
+        unsafe {
+            match prev_home {
+                Some(v) => std::env::set_var("HOME", v),
+                None => std::env::remove_var("HOME"),
+            }
+            match prev_userprofile {
+                Some(v) => std::env::set_var("USERPROFILE", v),
+                None => std::env::remove_var("USERPROFILE"),
+            }
+        }
+    }
 
     #[test]
     fn test_embedded_skills_are_loaded() {
@@ -399,46 +433,51 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn test_install_and_uninstall_skills_lifecycle() {
-        let skills_base = skills_dir_path().unwrap();
-
-        // Dry run should not create anything
-        let dry_result = install_skills(true, false).unwrap();
-        assert!(dry_result.changed);
-        assert_eq!(dry_result.installed_count, EMBEDDED_SKILLS.len());
-
-        // Install creates skill files with correct content
-        let result = install_skills(false, false).unwrap();
-        assert!(result.changed);
-        assert_eq!(result.installed_count, EMBEDDED_SKILLS.len());
-        assert!(skills_base.exists());
-        for skill in EMBEDDED_SKILLS {
-            let skill_md = skills_base.join(skill.name).join("SKILL.md");
-            assert!(skill_md.exists(), "SKILL.md missing for {}", skill.name);
-            let content = fs::read_to_string(&skill_md).unwrap();
-            assert_eq!(content, skill.skill_md);
-        }
+        // Use an isolated temp HOME so we don't pollute the real home directory
+        // and don't race with other tests that mutate HOME (e.g. codex tests).
+        with_temp_home(|_home| {
+            let skills_base = skills_dir_path().unwrap();
+
+            // Dry run should not create anything
+            let dry_result = install_skills(true, false).unwrap();
+            assert!(dry_result.changed);
+            assert_eq!(dry_result.installed_count, EMBEDDED_SKILLS.len());
+
+            // Install creates skill files with correct content
+            let result = install_skills(false, false).unwrap();
+            assert!(result.changed);
+            assert_eq!(result.installed_count, EMBEDDED_SKILLS.len());
+            assert!(skills_base.exists());
+            for skill in EMBEDDED_SKILLS {
+                let skill_md = skills_base.join(skill.name).join("SKILL.md");
+                assert!(skill_md.exists(), "SKILL.md missing for {}", skill.name);
+                let content = fs::read_to_string(&skill_md).unwrap();
+                assert_eq!(content, skill.skill_md);
+            }
 
-        // Install again is idempotent
-        let result2 = install_skills(false, false).unwrap();
-        assert!(result2.changed);
-        for skill in EMBEDDED_SKILLS {
-            let skill_md = skills_base.join(skill.name).join("SKILL.md");
-            assert!(
-                skill_md.exists(),
-                "SKILL.md missing after re-install for {}",
-                skill.name
-            );
-        }
+            // Install again is idempotent
+            let result2 = install_skills(false, false).unwrap();
+            assert!(result2.changed);
+            for skill in EMBEDDED_SKILLS {
+                let skill_md = skills_base.join(skill.name).join("SKILL.md");
+                assert!(
+                    skill_md.exists(),
+                    "SKILL.md missing after re-install for {}",
+                    skill.name
+                );
+            }
 
-        // Uninstall removes skills directory
-        let uninstall_result = uninstall_skills(false, false).unwrap();
-        assert!(uninstall_result.changed);
-        assert!(!skills_base.exists());
+            // Uninstall removes skills directory
+            let uninstall_result = uninstall_skills(false, false).unwrap();
+            assert!(uninstall_result.changed);
+            assert!(!skills_base.exists());
 
-        // Uninstall again is a no-op
-        let noop_result = uninstall_skills(false, false).unwrap();
-        assert!(!noop_result.changed);
-        assert_eq!(noop_result.installed_count, 0);
+            // Uninstall again is a no-op
+            let noop_result = uninstall_skills(false, false).unwrap();
+            assert!(!noop_result.changed);
+            assert_eq!(noop_result.installed_count, 0);
+        });
     }
 }