mirror of
https://github.com/AstrBotDevs/AstrBot
synced 2026-07-15 17:30:13 +08:00
feat(skills): enhance skill installation to support multiple top-level folders and add duplicate handling, and Chinese skill name support (#6952)
* feat(skills): enhance skill installation to support multiple top-level folders and add duplicate handling closes: #6949 * refactor(skill_manager): streamline skill name normalization and validation logic * fix(skill_manager): update skill name regex to allow underscores in skill names * fix(skill_manager): improve skill name normalization and validation logic
This commit is contained in:
@@ -27,7 +27,12 @@ SANDBOX_SKILLS_ROOT = "skills"
|
||||
SANDBOX_WORKSPACE_ROOT = "/workspace"
|
||||
_SANDBOX_SKILLS_CACHE_VERSION = 1
|
||||
|
||||
_SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
|
||||
_SKILL_NAME_RE = re.compile(r"^[\w.-]+$")
|
||||
|
||||
|
||||
def _normalize_skill_name(name: str | None) -> str:
|
||||
raw = str(name or "")
|
||||
return re.sub(r"\s+", "_", raw.strip())
|
||||
|
||||
|
||||
def _default_sandbox_skill_path(name: str) -> str:
|
||||
@@ -530,7 +535,13 @@ class SkillManager:
|
||||
config["skills"].pop(name, None)
|
||||
self._save_config(config)
|
||||
|
||||
def install_skill_from_zip(self, zip_path: str, *, overwrite: bool = True) -> str:
|
||||
def install_skill_from_zip(
|
||||
self,
|
||||
zip_path: str,
|
||||
*,
|
||||
overwrite: bool = True,
|
||||
skill_name_hint: str | None = None,
|
||||
) -> str:
|
||||
zip_path_obj = Path(zip_path)
|
||||
if not zip_path_obj.exists():
|
||||
raise FileNotFoundError(f"Zip file not found: {zip_path}")
|
||||
@@ -547,15 +558,48 @@ class SkillManager:
|
||||
if not file_names:
|
||||
raise ValueError("Zip archive is empty.")
|
||||
|
||||
top_dirs = {
|
||||
PurePosixPath(name).parts[0] for name in file_names if name.strip()
|
||||
}
|
||||
has_root_skill_md = any(
|
||||
len(parts := PurePosixPath(name).parts) == 1
|
||||
and parts[0] in {"SKILL.md", "skill.md"}
|
||||
for name in file_names
|
||||
)
|
||||
root_mode = has_root_skill_md
|
||||
|
||||
if len(top_dirs) != 1:
|
||||
raise ValueError("Zip archive must contain a single top-level folder.")
|
||||
skill_name = next(iter(top_dirs))
|
||||
if skill_name in {".", "..", ""} or not _SKILL_NAME_RE.match(skill_name):
|
||||
raise ValueError("Invalid skill folder name.")
|
||||
archive_skill_name = None
|
||||
if skill_name_hint is not None:
|
||||
archive_skill_name = _normalize_skill_name(skill_name_hint)
|
||||
if archive_skill_name and not _SKILL_NAME_RE.fullmatch(
|
||||
archive_skill_name
|
||||
):
|
||||
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:
|
||||
@@ -565,16 +609,20 @@ class SkillManager:
|
||||
parts = PurePosixPath(name).parts
|
||||
if ".." in parts:
|
||||
raise ValueError("Zip archive contains invalid relative paths.")
|
||||
if parts and parts[0] != skill_name:
|
||||
if (not root_mode) and parts and parts[0] != archive_root_name:
|
||||
raise ValueError(
|
||||
"Zip archive contains unexpected top-level entries."
|
||||
)
|
||||
|
||||
if (
|
||||
f"{skill_name}/SKILL.md" not in file_names
|
||||
and f"{skill_name}/skill.md" not in file_names
|
||||
):
|
||||
raise ValueError("SKILL.md not found in the skill folder.")
|
||||
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():
|
||||
@@ -582,7 +630,12 @@ class SkillManager:
|
||||
if not member_name or _is_ignored_zip_entry(member_name):
|
||||
continue
|
||||
zf.extract(member, tmp_dir)
|
||||
src_dir = Path(tmp_dir) / skill_name
|
||||
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.")
|
||||
|
||||
@@ -2,7 +2,6 @@ import os
|
||||
import re
|
||||
import shutil
|
||||
import traceback
|
||||
import uuid
|
||||
from collections.abc import Awaitable, Callable
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
@@ -44,6 +43,17 @@ def _to_bool(value: Any, default: bool = False) -> bool:
|
||||
_SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
|
||||
|
||||
|
||||
def _next_available_temp_path(temp_dir: str, filename: str) -> str:
|
||||
stem = Path(filename).stem
|
||||
suffix = Path(filename).suffix
|
||||
candidate = filename
|
||||
index = 1
|
||||
while os.path.exists(os.path.join(temp_dir, candidate)):
|
||||
candidate = f"{stem}_{index}{suffix}"
|
||||
index += 1
|
||||
return os.path.join(temp_dir, candidate)
|
||||
|
||||
|
||||
class SkillsRoute(Route):
|
||||
def __init__(self, context: RouteContext, core_lifecycle) -> None:
|
||||
super().__init__(context)
|
||||
@@ -164,11 +174,24 @@ class SkillsRoute(Route):
|
||||
|
||||
temp_dir = get_astrbot_temp_path()
|
||||
os.makedirs(temp_dir, exist_ok=True)
|
||||
temp_path = os.path.join(temp_dir, filename)
|
||||
skill_mgr = SkillManager()
|
||||
temp_path = _next_available_temp_path(temp_dir, filename)
|
||||
await file.save(temp_path)
|
||||
|
||||
skill_mgr = SkillManager()
|
||||
skill_name = skill_mgr.install_skill_from_zip(temp_path, overwrite=True)
|
||||
try:
|
||||
try:
|
||||
skill_name = skill_mgr.install_skill_from_zip(
|
||||
temp_path, overwrite=False, skill_name_hint=Path(filename).stem
|
||||
)
|
||||
except TypeError:
|
||||
# Backward compatibility for callers that do not accept skill_name_hint
|
||||
skill_name = skill_mgr.install_skill_from_zip(
|
||||
temp_path, overwrite=False
|
||||
)
|
||||
except Exception:
|
||||
# Keep behavior consistent with previous implementation
|
||||
# and bubble up install errors (including duplicates).
|
||||
raise
|
||||
|
||||
try:
|
||||
await sync_skills_to_active_sandboxes()
|
||||
@@ -208,6 +231,7 @@ class SkillsRoute(Route):
|
||||
|
||||
succeeded = []
|
||||
failed = []
|
||||
skipped = []
|
||||
skill_mgr = SkillManager()
|
||||
temp_dir = get_astrbot_temp_path()
|
||||
os.makedirs(temp_dir, exist_ok=True)
|
||||
@@ -226,14 +250,42 @@ class SkillsRoute(Route):
|
||||
)
|
||||
continue
|
||||
|
||||
temp_path = os.path.join(
|
||||
temp_dir, f"batch_{uuid.uuid4().hex}_{filename}"
|
||||
)
|
||||
temp_path = _next_available_temp_path(temp_dir, filename)
|
||||
await file.save(temp_path)
|
||||
|
||||
skill_name = skill_mgr.install_skill_from_zip(
|
||||
temp_path, overwrite=True
|
||||
)
|
||||
try:
|
||||
skill_name = skill_mgr.install_skill_from_zip(
|
||||
temp_path,
|
||||
overwrite=False,
|
||||
skill_name_hint=Path(filename).stem,
|
||||
)
|
||||
except TypeError:
|
||||
# Backward compatibility for monkeypatched implementations in tests
|
||||
try:
|
||||
skill_name = skill_mgr.install_skill_from_zip(
|
||||
temp_path, overwrite=False
|
||||
)
|
||||
except FileExistsError:
|
||||
skipped.append(
|
||||
{
|
||||
"filename": filename,
|
||||
"name": Path(filename).stem,
|
||||
"error": "Skill already exists.",
|
||||
}
|
||||
)
|
||||
skill_name = None
|
||||
except FileExistsError:
|
||||
skipped.append(
|
||||
{
|
||||
"filename": filename,
|
||||
"name": Path(filename).stem,
|
||||
"error": "Skill already exists.",
|
||||
}
|
||||
)
|
||||
skill_name = None
|
||||
|
||||
if skill_name is None:
|
||||
continue
|
||||
succeeded.append({"filename": filename, "name": skill_name})
|
||||
|
||||
except Exception as e:
|
||||
@@ -255,8 +307,10 @@ class SkillsRoute(Route):
|
||||
|
||||
total = len(file_list)
|
||||
success_count = len(succeeded)
|
||||
skipped_count = len(skipped)
|
||||
failed_count = len(failed)
|
||||
|
||||
if success_count == total:
|
||||
if failed_count == 0 and success_count == total:
|
||||
message = f"All {total} skill(s) uploaded successfully."
|
||||
return (
|
||||
Response()
|
||||
@@ -265,18 +319,35 @@ class SkillsRoute(Route):
|
||||
"total": total,
|
||||
"succeeded": succeeded,
|
||||
"failed": failed,
|
||||
"skipped": skipped,
|
||||
},
|
||||
message,
|
||||
)
|
||||
.__dict__
|
||||
)
|
||||
if success_count == 0:
|
||||
if failed_count == 0 and success_count == 0:
|
||||
message = f"All {total} file(s) were skipped."
|
||||
return (
|
||||
Response()
|
||||
.ok(
|
||||
{
|
||||
"total": total,
|
||||
"succeeded": succeeded,
|
||||
"failed": failed,
|
||||
"skipped": skipped,
|
||||
},
|
||||
message,
|
||||
)
|
||||
.__dict__
|
||||
)
|
||||
if success_count == 0 and skipped_count == 0:
|
||||
message = f"Upload failed for all {total} file(s)."
|
||||
resp = Response().error(message)
|
||||
resp.data = {
|
||||
"total": total,
|
||||
"succeeded": succeeded,
|
||||
"failed": failed,
|
||||
"skipped": skipped,
|
||||
}
|
||||
return resp.__dict__
|
||||
|
||||
@@ -288,6 +359,7 @@ class SkillsRoute(Route):
|
||||
"total": total,
|
||||
"succeeded": succeeded,
|
||||
"failed": failed,
|
||||
"skipped": skipped,
|
||||
},
|
||||
message,
|
||||
)
|
||||
|
||||
@@ -900,6 +900,7 @@ export default {
|
||||
const applyUploadResults = (attemptedItems, payload) => {
|
||||
const succeededMap = buildResultMap(payload?.succeeded);
|
||||
const failedMap = buildResultMap(payload?.failed);
|
||||
const skippedMap = buildResultMap(payload?.skipped);
|
||||
|
||||
for (const item of attemptedItems) {
|
||||
const successEntry = takeFirstMatch(succeededMap, item.filenameKey);
|
||||
@@ -911,6 +912,14 @@ export default {
|
||||
continue;
|
||||
}
|
||||
|
||||
const skippedEntry = takeFirstMatch(skippedMap, item.filenameKey);
|
||||
if (skippedEntry) {
|
||||
item.status = STATUS_SKIPPED;
|
||||
item.validationMessage =
|
||||
skippedEntry.error || tm("skills.validationDuplicate");
|
||||
continue;
|
||||
}
|
||||
|
||||
const failedEntry = takeFirstMatch(failedMap, item.filenameKey);
|
||||
if (failedEntry) {
|
||||
item.status = STATUS_ERROR;
|
||||
|
||||
@@ -242,7 +242,7 @@
|
||||
"emptyHint": "Upload a Skills zip to get started",
|
||||
"uploadDialogTitle": "Upload Skills",
|
||||
"uploadHint": "Upload multiple zip skill packages or drag them in. The system validates the structure automatically and shows a result for each file.",
|
||||
"structureRequirement": "The most common failure is an invalid archive structure. Each zip must contain exactly one top-level folder such as `skillname/`, and that folder must include `SKILL.md`.",
|
||||
"structureRequirement": "The archive supports multiple skills folders.",
|
||||
"abilityMultiple": "Upload multiple zip files at once",
|
||||
"abilityValidate": "Validate `SKILL.md` automatically",
|
||||
"abilitySkip": "Automatically skip duplicate files.",
|
||||
|
||||
@@ -241,7 +241,7 @@
|
||||
"emptyHint": "Пожалуйста, загрузите архив с навыками",
|
||||
"uploadDialogTitle": "Загрузка навыков",
|
||||
"uploadHint": "Поддерживается массовая загрузка zip-архивов. Вы также можете перетащить файлы в это окно. Система автоматически проверит структуру каждого архива.",
|
||||
"structureRequirement": "Архив должен содержать одну корневую папку (например, `skillname/`), внутри которой обязательно должен находиться файл `SKILL.md`.",
|
||||
"structureRequirement": "Поддерживаются архивы с несколькими папками skills.",
|
||||
"abilityMultiple": "Поддержка массовой загрузки",
|
||||
"abilityValidate": "Автопроверка `SKILL.md`",
|
||||
"abilitySkip": "Пропуск дубликатов",
|
||||
|
||||
@@ -245,7 +245,7 @@
|
||||
"emptyHint": "请上传 Skills 压缩包",
|
||||
"uploadDialogTitle": "上传 Skills",
|
||||
"uploadHint": "支持批量上传 zip 技能包,也支持拖拽批量上传 zip 技能包。系统会自动校验目录结构,并给出逐个文件的结果。",
|
||||
"structureRequirement": "常见失败原因是压缩包结构不正确。每个 zip 必须只包含一个顶层目录,例如 `skillname/`,且该目录下必须存在 `SKILL.md`。",
|
||||
"structureRequirement": "支持压缩包内含多个 skills 文件夹。",
|
||||
"abilityMultiple": "支持一次上传多个zip文件",
|
||||
"abilityValidate": "自动校验 `SKILL.md`",
|
||||
"abilitySkip": "自动跳过重复文件",
|
||||
|
||||
Reference in New Issue
Block a user