From 71c8436e9059db13fbea570dcd0c2b59efc0f93e Mon Sep 17 00:00:00 2001
From: zhayujie <yjzha1996@163.com>
Date: Mon, 9 Mar 2026 18:43:28 +0800
Subject: [PATCH] fix: skill download to temp dir

---
 agent/skills/manager.py       |  4 +++-
 agent/skills/service.py       | 34 +++++++++++++++++++++-------------
 skills/skill-creator/SKILL.md |  1 +
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/agent/skills/manager.py b/agent/skills/manager.py
index 69e3b34..a70daae 100644
--- a/agent/skills/manager.py
+++ b/agent/skills/manager.py
@@ -95,12 +95,14 @@ class SkillManager:
         for name, entry in self.skills.items():
             skill = entry.skill
             prev = saved.get(name, {})
+            # category priority: persisted config (set by cloud) > default "skill"
+            category = prev.get("category", "skill")
             merged[name] = {
                 "name": name,
                 "description": skill.description,
                 "source": skill.source,
                 "enabled": prev.get("enabled", True),
-                **( {"category": prev["category"]} if "category" in prev else {} ),
+                "category": category,
             }
 
         self.skills_config = merged
diff --git a/agent/skills/service.py b/agent/skills/service.py
index 5ff97e6..ed24fb1 100644
--- a/agent/skills/service.py
+++ b/agent/skills/service.py
@@ -82,21 +82,29 @@ class SkillService:
 
         skill_dir = os.path.join(self.manager.custom_dir, name)
 
-        # Remove existing skill directory to ensure a clean overwrite
+        # Download to a temp directory first, then swap to avoid data loss on failure
+        tmp_dir = skill_dir + ".tmp"
+        if os.path.exists(tmp_dir):
+            shutil.rmtree(tmp_dir)
+        os.makedirs(tmp_dir, exist_ok=True)
+
+        try:
+            for file_info in files:
+                url = file_info.get("url")
+                rel_path = file_info.get("path")
+                if not url or not rel_path:
+                    logger.warning(f"[SkillService] add: skip invalid file entry {file_info}")
+                    continue
+                dest = os.path.join(tmp_dir, rel_path)
+                self._download_file(url, dest)
+        except Exception:
+            shutil.rmtree(tmp_dir, ignore_errors=True)
+            raise
+
+        # All files downloaded successfully, replace the old directory
         if os.path.exists(skill_dir):
             shutil.rmtree(skill_dir)
-            logger.info(f"[SkillService] add: removed existing skill directory for overwrite: {skill_dir}")
-
-        os.makedirs(skill_dir, exist_ok=True)
-
-        for file_info in files:
-            url = file_info.get("url")
-            rel_path = file_info.get("path")
-            if not url or not rel_path:
-                logger.warning(f"[SkillService] add: skip invalid file entry {file_info}")
-                continue
-            dest = os.path.join(skill_dir, rel_path)
-            self._download_file(url, dest)
+        os.rename(tmp_dir, skill_dir)
 
         # Reload to pick up the new skill and sync config
         self.manager.refresh_skills()
diff --git a/skills/skill-creator/SKILL.md b/skills/skill-creator/SKILL.md
index 28ff57e..84b4bf2 100644
--- a/skills/skill-creator/SKILL.md
+++ b/skills/skill-creator/SKILL.md
@@ -275,6 +275,7 @@ Write the YAML frontmatter with `name`, `description`, and optional `metadata`:
   - `requires.anyBins`: Alternative binaries — at least one must be present
   - `always`: Set to `true` to always load regardless of requirements
   - `emoji`: Skill icon (optional)
+  - Do NOT set `category` — it defaults to `skill` and is managed by the system
 
 **API Key Requirements**: