From 06dc3914e0af448f0587c067792ab95da5816e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=8B=E6=9C=88=E7=99=BD?= <105275068+jmt059@users.noreply.github.com> Date: Sat, 28 Mar 2026 13:46:26 +0800 Subject: [PATCH 1/3] fix: allow multiple skills in a single zip archive --- astrbot/core/skills/skill_manager.py | 124 ++++++++++++++------------- 1 file changed, 64 insertions(+), 60 deletions(-) diff --git a/astrbot/core/skills/skill_manager.py b/astrbot/core/skills/skill_manager.py index 1b71e5371f..04a41a5258 100644 --- a/astrbot/core/skills/skill_manager.py +++ b/astrbot/core/skills/skill_manager.py @@ -548,6 +548,8 @@ def install_skill_from_zip( if not zipfile.is_zipfile(zip_path): raise ValueError("Uploaded file is not a valid zip archive.") + installed_skills = [] + with zipfile.ZipFile(zip_path) as zf: names = [ name @@ -573,34 +575,6 @@ def install_skill_from_zip( ): raise ValueError("Invalid skill name.") - if root_mode: - archive_hint = _normalize_skill_name( - archive_skill_name or zip_path_obj.stem - ) - if not archive_hint or not _SKILL_NAME_RE.fullmatch(archive_hint): - raise ValueError("Invalid skill name.") - skill_name = archive_hint - else: - top_dirs = { - PurePosixPath(name).parts[0] for name in file_names if name.strip() - } - if len(top_dirs) != 1: - raise ValueError( - "Zip archive must contain a single top-level folder." - ) - archive_root_name = next(iter(top_dirs)) - archive_root_name_normalized = _normalize_skill_name(archive_root_name) - if archive_root_name in {".", "..", ""} or not _SKILL_NAME_RE.fullmatch( - archive_root_name_normalized - ): - raise ValueError("Invalid skill folder name.") - if archive_skill_name: - if not _SKILL_NAME_RE.fullmatch(archive_skill_name): - raise ValueError("Invalid skill name.") - skill_name = archive_skill_name - else: - skill_name = archive_root_name_normalized - for name in names: if not name: continue @@ -609,20 +583,6 @@ def install_skill_from_zip( parts = PurePosixPath(name).parts if ".." in parts: raise ValueError("Zip archive contains invalid relative paths.") - if (not root_mode) and parts and parts[0] != archive_root_name: - raise ValueError( - "Zip archive contains unexpected top-level entries." - ) - - if root_mode: - if "SKILL.md" not in file_names and "skill.md" not in file_names: - raise ValueError("SKILL.md not found in the skill folder.") - else: - if ( - f"{archive_root_name}/SKILL.md" not in file_names - and f"{archive_root_name}/skill.md" not in file_names - ): - raise ValueError("SKILL.md not found in the skill folder.") with tempfile.TemporaryDirectory(dir=get_astrbot_temp_path()) as tmp_dir: for member in zf.infolist(): @@ -630,21 +590,65 @@ def install_skill_from_zip( if not member_name or _is_ignored_zip_entry(member_name): continue zf.extract(member, tmp_dir) - src_dir = ( - Path(tmp_dir) if root_mode else Path(tmp_dir) / archive_root_name - ) - normalized_path = _normalize_skill_markdown_path(src_dir) - if normalized_path is None: - raise ValueError("SKILL.md not found in the skill folder.") - _normalize_skill_markdown_path(src_dir) - if not src_dir.exists(): - raise ValueError("Skill folder not found after extraction.") - dest_dir = Path(self.skills_root) / skill_name - if dest_dir.exists(): - if not overwrite: - raise FileExistsError("Skill already exists.") - shutil.rmtree(dest_dir) - shutil.move(str(src_dir), str(dest_dir)) - - self.set_skill_active(skill_name, True) - return skill_name + + if root_mode: + archive_hint = _normalize_skill_name( + archive_skill_name or zip_path_obj.stem + ) + if not archive_hint or not _SKILL_NAME_RE.fullmatch(archive_hint): + raise ValueError("Invalid skill name.") + skill_name = archive_hint + + src_dir = Path(tmp_dir) + normalized_path = _normalize_skill_markdown_path(src_dir) + if normalized_path is None: + raise ValueError("SKILL.md not found in the root of the zip archive.") + + dest_dir = Path(self.skills_root) / skill_name + if dest_dir.exists(): + if not overwrite: + raise FileExistsError(f"Skill {skill_name} already exists.") + shutil.rmtree(dest_dir) + shutil.move(str(src_dir), str(dest_dir)) + self.set_skill_active(skill_name, True) + installed_skills.append(skill_name) + + else: + top_dirs = { + PurePosixPath(name).parts[0] for name in file_names if name.strip() + } + + for archive_root_name in top_dirs: + archive_root_name_normalized = _normalize_skill_name(archive_root_name) + + if ( + f"{archive_root_name}/SKILL.md" not in file_names + and f"{archive_root_name}/skill.md" not in file_names + ): + continue + + if archive_root_name in {".", "..", ""} or not _SKILL_NAME_RE.fullmatch( + archive_root_name_normalized + ): + continue + + skill_name = archive_skill_name if archive_skill_name and len(top_dirs) == 1 else archive_root_name_normalized + + src_dir = Path(tmp_dir) / archive_root_name + normalized_path = _normalize_skill_markdown_path(src_dir) + if normalized_path is None: + continue + + dest_dir = Path(self.skills_root) / skill_name + if dest_dir.exists(): + if not overwrite: + raise FileExistsError(f"Skill {skill_name} already exists.") + shutil.rmtree(dest_dir) + shutil.move(str(src_dir), str(dest_dir)) + self.set_skill_active(skill_name, True) + installed_skills.append(skill_name) + + if not installed_skills: + raise ValueError("No valid SKILL.md found in any folder of the zip archive.") + + return ", ".join(installed_skills) From a72ccb8a07de8a0111a36971c3bf69f5b63cf1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=8B=E6=9C=88=E7=99=BD?= <105275068+jmt059@users.noreply.github.com> Date: Sat, 28 Mar 2026 14:20:36 +0800 Subject: [PATCH 2/3] refactor: address bot review comments --- astrbot/core/skills/skill_manager.py | 54 ++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 11 deletions(-) diff --git a/astrbot/core/skills/skill_manager.py b/astrbot/core/skills/skill_manager.py index 04a41a5258..15af4149f0 100644 --- a/astrbot/core/skills/skill_manager.py +++ b/astrbot/core/skills/skill_manager.py @@ -541,7 +541,7 @@ def install_skill_from_zip( *, overwrite: bool = True, skill_name_hint: str | None = None, - ) -> str: + ) -> str: zip_path_obj = Path(zip_path) if not zip_path_obj.exists(): raise FileNotFoundError(f"Zip file not found: {zip_path}") @@ -584,6 +584,34 @@ def install_skill_from_zip( if ".." in parts: raise ValueError("Zip archive contains invalid relative paths.") + if not root_mode and not overwrite: + top_dirs = {PurePosixPath(n).parts[0] for n in file_names if n.strip()} + conflict_dirs: list[str] = [] + for src_dir_name in top_dirs: + if (f"{src_dir_name}/SKILL.md" not in file_names and f"{src_dir_name}/skill.md" not in file_names): + continue + + candidate_name = _normalize_skill_name(src_dir_name) + if not candidate_name or not _SKILL_NAME_RE.fullmatch(candidate_name): + continue + + if archive_skill_name and len(top_dirs) == 1: + target_name = archive_skill_name + else: + target_name = candidate_name + + dest_dir = Path(self.skills_root) / target_name + if dest_dir.exists(): + conflict_dirs.append(str(dest_dir)) + + if conflict_dirs: + raise FileExistsError( + "One or more skills from the archive already exist and " + "overwrite=False. No skills were installed. Conflicting " + f"paths: {', '.join(conflict_dirs)}" + ) + + with tempfile.TemporaryDirectory(dir=get_astrbot_temp_path()) as tmp_dir: for member in zf.infolist(): member_name = member.filename.replace("\\", "/") @@ -605,18 +633,17 @@ def install_skill_from_zip( raise ValueError("SKILL.md not found in the root of the zip archive.") dest_dir = Path(self.skills_root) / skill_name - if dest_dir.exists(): - if not overwrite: - raise FileExistsError(f"Skill {skill_name} already exists.") + if dest_dir.exists() and overwrite: shutil.rmtree(dest_dir) + elif dest_dir.exists() and not overwrite: + raise FileExistsError(f"Skill {skill_name} already exists.") + shutil.move(str(src_dir), str(dest_dir)) self.set_skill_active(skill_name, True) installed_skills.append(skill_name) else: - top_dirs = { - PurePosixPath(name).parts[0] for name in file_names if name.strip() - } + top_dirs = {PurePosixPath(n).parts[0] for n in file_names if n.strip()} for archive_root_name in top_dirs: archive_root_name_normalized = _normalize_skill_name(archive_root_name) @@ -632,7 +659,11 @@ def install_skill_from_zip( ): continue - skill_name = archive_skill_name if archive_skill_name and len(top_dirs) == 1 else archive_root_name_normalized + + if archive_skill_name and len(top_dirs) == 1: + skill_name = archive_skill_name + else: + skill_name = archive_root_name_normalized src_dir = Path(tmp_dir) / archive_root_name normalized_path = _normalize_skill_markdown_path(src_dir) @@ -642,13 +673,14 @@ def install_skill_from_zip( dest_dir = Path(self.skills_root) / skill_name if dest_dir.exists(): if not overwrite: - raise FileExistsError(f"Skill {skill_name} already exists.") + raise FileExistsError(f"Skill {skill_name} already exists.") shutil.rmtree(dest_dir) + shutil.move(str(src_dir), str(dest_dir)) self.set_skill_active(skill_name, True) installed_skills.append(skill_name) - + if not installed_skills: raise ValueError("No valid SKILL.md found in any folder of the zip archive.") - return ", ".join(installed_skills) + return installed_skills[0] From 8153b73bc9a1dcbc597431493e592d93e4359d7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=8B=E6=9C=88=E7=99=BD?= <105275068+jmt059@users.noreply.github.com> Date: Sat, 28 Mar 2026 21:09:42 +0800 Subject: [PATCH 3/3] fix: apply ruff format and fix return value --- astrbot/core/skills/skill_manager.py | 71 ++++++++++++++++------------ 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/astrbot/core/skills/skill_manager.py b/astrbot/core/skills/skill_manager.py index 15af4149f0..a8121c42a4 100644 --- a/astrbot/core/skills/skill_manager.py +++ b/astrbot/core/skills/skill_manager.py @@ -541,7 +541,7 @@ def install_skill_from_zip( *, overwrite: bool = True, skill_name_hint: str | None = None, - ) -> str: + ) -> str: zip_path_obj = Path(zip_path) if not zip_path_obj.exists(): raise FileNotFoundError(f"Zip file not found: {zip_path}") @@ -588,18 +588,23 @@ def install_skill_from_zip( top_dirs = {PurePosixPath(n).parts[0] for n in file_names if n.strip()} conflict_dirs: list[str] = [] for src_dir_name in top_dirs: - if (f"{src_dir_name}/SKILL.md" not in file_names and f"{src_dir_name}/skill.md" not in file_names): + if ( + f"{src_dir_name}/SKILL.md" not in file_names + and f"{src_dir_name}/skill.md" not in file_names + ): continue - + candidate_name = _normalize_skill_name(src_dir_name) - if not candidate_name or not _SKILL_NAME_RE.fullmatch(candidate_name): - continue - + if not candidate_name or not _SKILL_NAME_RE.fullmatch( + candidate_name + ): + continue + if archive_skill_name and len(top_dirs) == 1: target_name = archive_skill_name else: target_name = candidate_name - + dest_dir = Path(self.skills_root) / target_name if dest_dir.exists(): conflict_dirs.append(str(dest_dir)) @@ -611,7 +616,6 @@ def install_skill_from_zip( f"paths: {', '.join(conflict_dirs)}" ) - with tempfile.TemporaryDirectory(dir=get_astrbot_temp_path()) as tmp_dir: for member in zf.infolist(): member_name = member.filename.replace("\\", "/") @@ -626,61 +630,70 @@ def install_skill_from_zip( if not archive_hint or not _SKILL_NAME_RE.fullmatch(archive_hint): raise ValueError("Invalid skill name.") skill_name = archive_hint - + src_dir = Path(tmp_dir) normalized_path = _normalize_skill_markdown_path(src_dir) if normalized_path is None: - raise ValueError("SKILL.md not found in the root of the zip archive.") - + raise ValueError( + "SKILL.md not found in the root of the zip archive." + ) + dest_dir = Path(self.skills_root) / skill_name if dest_dir.exists() and overwrite: shutil.rmtree(dest_dir) elif dest_dir.exists() and not overwrite: - raise FileExistsError(f"Skill {skill_name} already exists.") - + raise FileExistsError(f"Skill {skill_name} already exists.") + shutil.move(str(src_dir), str(dest_dir)) self.set_skill_active(skill_name, True) installed_skills.append(skill_name) - + else: - top_dirs = {PurePosixPath(n).parts[0] for n in file_names if n.strip()} - + top_dirs = { + PurePosixPath(n).parts[0] for n in file_names if n.strip() + } + for archive_root_name in top_dirs: - archive_root_name_normalized = _normalize_skill_name(archive_root_name) - + archive_root_name_normalized = _normalize_skill_name( + archive_root_name + ) + if ( f"{archive_root_name}/SKILL.md" not in file_names and f"{archive_root_name}/skill.md" not in file_names ): continue - - if archive_root_name in {".", "..", ""} or not _SKILL_NAME_RE.fullmatch( - archive_root_name_normalized + + if archive_root_name in {".", "..", ""} or not ( + _SKILL_NAME_RE.fullmatch(archive_root_name_normalized) ): continue - - + if archive_skill_name and len(top_dirs) == 1: skill_name = archive_skill_name else: skill_name = archive_root_name_normalized - + src_dir = Path(tmp_dir) / archive_root_name normalized_path = _normalize_skill_markdown_path(src_dir) if normalized_path is None: continue - + dest_dir = Path(self.skills_root) / skill_name if dest_dir.exists(): if not overwrite: - raise FileExistsError(f"Skill {skill_name} already exists.") + raise FileExistsError( + f"Skill {skill_name} already exists." + ) shutil.rmtree(dest_dir) - + shutil.move(str(src_dir), str(dest_dir)) self.set_skill_active(skill_name, True) installed_skills.append(skill_name) if not installed_skills: - raise ValueError("No valid SKILL.md found in any folder of the zip archive.") + raise ValueError( + "No valid SKILL.md found in any folder of the zip archive." + ) - return installed_skills[0] + return ", ".join(installed_skills)