Skip to content

Commit 1b8c8da

Browse files
committed
feat(skills): enhance skill installation to support multiple top-level folders and add duplicate handling
closes: #6949
1 parent e4ce090 commit 1b8c8da

6 files changed

Lines changed: 160 additions & 31 deletions

File tree

astrbot/core/skills/skill_manager.py

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,16 @@
3030
_SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
3131

3232

33+
def _normalize_archive_skill_name(name: str) -> str:
34+
normalized = re.sub(r"[^A-Za-z0-9._-]", "-", name.strip())
35+
normalized = normalized.strip("._-")
36+
if not normalized:
37+
normalized = "skill"
38+
if not _SKILL_NAME_RE.match(normalized):
39+
raise ValueError("Invalid skill name.")
40+
return normalized
41+
42+
3343
def _default_sandbox_skill_path(name: str) -> str:
3444
return f"{SANDBOX_WORKSPACE_ROOT}/{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md"
3545

@@ -530,7 +540,13 @@ def delete_skill(self, name: str) -> None:
530540
config["skills"].pop(name, None)
531541
self._save_config(config)
532542

533-
def install_skill_from_zip(self, zip_path: str, *, overwrite: bool = True) -> str:
543+
def install_skill_from_zip(
544+
self,
545+
zip_path: str,
546+
*,
547+
overwrite: bool = True,
548+
skill_name_hint: str | None = None,
549+
) -> str:
534550
zip_path_obj = Path(zip_path)
535551
if not zip_path_obj.exists():
536552
raise FileNotFoundError(f"Zip file not found: {zip_path}")
@@ -547,15 +563,40 @@ def install_skill_from_zip(self, zip_path: str, *, overwrite: bool = True) -> st
547563
if not file_names:
548564
raise ValueError("Zip archive is empty.")
549565

550-
top_dirs = {
551-
PurePosixPath(name).parts[0] for name in file_names if name.strip()
552-
}
566+
has_root_skill_md = any(
567+
PurePosixPath(name).name == "SKILL.md" and len(PurePosixPath(name).parts) == 1
568+
for name in file_names
569+
)
570+
has_root_legacy_skill_md = any(
571+
PurePosixPath(name).name == "skill.md" and len(PurePosixPath(name).parts) == 1
572+
for name in file_names
573+
)
574+
root_mode = has_root_skill_md or has_root_legacy_skill_md
553575

554-
if len(top_dirs) != 1:
555-
raise ValueError("Zip archive must contain a single top-level folder.")
556-
skill_name = next(iter(top_dirs))
557-
if skill_name in {".", "..", ""} or not _SKILL_NAME_RE.match(skill_name):
558-
raise ValueError("Invalid skill folder name.")
576+
archive_skill_name = None
577+
if skill_name_hint:
578+
archive_skill_name = _normalize_archive_skill_name(skill_name_hint)
579+
580+
if root_mode:
581+
archive_hint = (archive_skill_name or zip_path_obj.stem).strip()
582+
skill_name = _normalize_archive_skill_name(archive_hint)
583+
else:
584+
top_dirs = {
585+
PurePosixPath(name).parts[0] for name in file_names if name.strip()
586+
}
587+
if len(top_dirs) != 1:
588+
raise ValueError(
589+
"Zip archive must contain a single top-level folder."
590+
)
591+
archive_root_name = next(iter(top_dirs))
592+
if archive_root_name in {".", "..", ""} or not _SKILL_NAME_RE.match(
593+
archive_root_name
594+
):
595+
raise ValueError("Invalid skill folder name.")
596+
if archive_skill_name:
597+
skill_name = archive_skill_name
598+
else:
599+
skill_name = archive_root_name
559600

560601
for name in names:
561602
if not name:
@@ -565,24 +606,31 @@ def install_skill_from_zip(self, zip_path: str, *, overwrite: bool = True) -> st
565606
parts = PurePosixPath(name).parts
566607
if ".." in parts:
567608
raise ValueError("Zip archive contains invalid relative paths.")
568-
if parts and parts[0] != skill_name:
609+
if (not root_mode) and parts and parts[0] != archive_root_name:
569610
raise ValueError(
570611
"Zip archive contains unexpected top-level entries."
571612
)
572613

573-
if (
574-
f"{skill_name}/SKILL.md" not in file_names
575-
and f"{skill_name}/skill.md" not in file_names
576-
):
577-
raise ValueError("SKILL.md not found in the skill folder.")
614+
if root_mode:
615+
if "SKILL.md" not in file_names and "skill.md" not in file_names:
616+
raise ValueError("SKILL.md not found in the skill folder.")
617+
else:
618+
if (
619+
f"{archive_root_name}/SKILL.md" not in file_names
620+
and f"{archive_root_name}/skill.md" not in file_names
621+
):
622+
raise ValueError("SKILL.md not found in the skill folder.")
578623

579624
with tempfile.TemporaryDirectory(dir=get_astrbot_temp_path()) as tmp_dir:
580625
for member in zf.infolist():
581626
member_name = member.filename.replace("\\", "/")
582627
if not member_name or _is_ignored_zip_entry(member_name):
583628
continue
584629
zf.extract(member, tmp_dir)
585-
src_dir = Path(tmp_dir) / skill_name
630+
src_dir = Path(tmp_dir) if root_mode else Path(tmp_dir) / archive_root_name
631+
normalized_path = _normalize_skill_markdown_path(src_dir)
632+
if normalized_path is None:
633+
raise ValueError("SKILL.md not found in the skill folder.")
586634
_normalize_skill_markdown_path(src_dir)
587635
if not src_dir.exists():
588636
raise ValueError("Skill folder not found after extraction.")

astrbot/dashboard/routes/skills.py

Lines changed: 84 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
import re
33
import shutil
44
import traceback
5-
import uuid
65
from collections.abc import Awaitable, Callable
76
from pathlib import Path
87
from typing import Any
@@ -44,6 +43,17 @@ def _to_bool(value: Any, default: bool = False) -> bool:
4443
_SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$")
4544

4645

46+
def _next_available_temp_path(temp_dir: str, filename: str) -> str:
47+
stem = Path(filename).stem
48+
suffix = Path(filename).suffix
49+
candidate = filename
50+
index = 1
51+
while os.path.exists(os.path.join(temp_dir, candidate)):
52+
candidate = f"{stem}_{index}{suffix}"
53+
index += 1
54+
return os.path.join(temp_dir, candidate)
55+
56+
4757
class SkillsRoute(Route):
4858
def __init__(self, context: RouteContext, core_lifecycle) -> None:
4959
super().__init__(context)
@@ -164,11 +174,24 @@ async def upload_skill(self):
164174

165175
temp_dir = get_astrbot_temp_path()
166176
os.makedirs(temp_dir, exist_ok=True)
167-
temp_path = os.path.join(temp_dir, filename)
177+
skill_mgr = SkillManager()
178+
temp_path = _next_available_temp_path(temp_dir, filename)
168179
await file.save(temp_path)
169180

170-
skill_mgr = SkillManager()
171-
skill_name = skill_mgr.install_skill_from_zip(temp_path, overwrite=True)
181+
try:
182+
try:
183+
skill_name = skill_mgr.install_skill_from_zip(
184+
temp_path, overwrite=False, skill_name_hint=Path(filename).stem
185+
)
186+
except TypeError:
187+
# Backward compatibility for callers that do not accept skill_name_hint
188+
skill_name = skill_mgr.install_skill_from_zip(
189+
temp_path, overwrite=False
190+
)
191+
except Exception:
192+
# Keep behavior consistent with previous implementation
193+
# and bubble up install errors (including duplicates).
194+
raise
172195

173196
try:
174197
await sync_skills_to_active_sandboxes()
@@ -208,6 +231,7 @@ async def batch_upload_skills(self):
208231

209232
succeeded = []
210233
failed = []
234+
skipped = []
211235
skill_mgr = SkillManager()
212236
temp_dir = get_astrbot_temp_path()
213237
os.makedirs(temp_dir, exist_ok=True)
@@ -226,14 +250,42 @@ async def batch_upload_skills(self):
226250
)
227251
continue
228252

229-
temp_path = os.path.join(
230-
temp_dir, f"batch_{uuid.uuid4().hex}_{filename}"
231-
)
253+
temp_path = _next_available_temp_path(temp_dir, filename)
232254
await file.save(temp_path)
233255

234-
skill_name = skill_mgr.install_skill_from_zip(
235-
temp_path, overwrite=True
236-
)
256+
try:
257+
skill_name = skill_mgr.install_skill_from_zip(
258+
temp_path,
259+
overwrite=False,
260+
skill_name_hint=Path(filename).stem,
261+
)
262+
except TypeError:
263+
# Backward compatibility for monkeypatched implementations in tests
264+
try:
265+
skill_name = skill_mgr.install_skill_from_zip(
266+
temp_path, overwrite=False
267+
)
268+
except FileExistsError:
269+
skipped.append(
270+
{
271+
"filename": filename,
272+
"name": Path(filename).stem,
273+
"error": "Skill already exists.",
274+
}
275+
)
276+
skill_name = None
277+
except FileExistsError:
278+
skipped.append(
279+
{
280+
"filename": filename,
281+
"name": Path(filename).stem,
282+
"error": "Skill already exists.",
283+
}
284+
)
285+
skill_name = None
286+
287+
if skill_name is None:
288+
continue
237289
succeeded.append({"filename": filename, "name": skill_name})
238290

239291
except Exception as e:
@@ -255,8 +307,10 @@ async def batch_upload_skills(self):
255307

256308
total = len(file_list)
257309
success_count = len(succeeded)
310+
skipped_count = len(skipped)
311+
failed_count = len(failed)
258312

259-
if success_count == total:
313+
if failed_count == 0 and success_count == total:
260314
message = f"All {total} skill(s) uploaded successfully."
261315
return (
262316
Response()
@@ -265,18 +319,35 @@ async def batch_upload_skills(self):
265319
"total": total,
266320
"succeeded": succeeded,
267321
"failed": failed,
322+
"skipped": skipped,
323+
},
324+
message,
325+
)
326+
.__dict__
327+
)
328+
if failed_count == 0 and success_count == 0:
329+
message = f"All {total} file(s) were skipped."
330+
return (
331+
Response()
332+
.ok(
333+
{
334+
"total": total,
335+
"succeeded": succeeded,
336+
"failed": failed,
337+
"skipped": skipped,
268338
},
269339
message,
270340
)
271341
.__dict__
272342
)
273-
if success_count == 0:
343+
if success_count == 0 and skipped_count == 0:
274344
message = f"Upload failed for all {total} file(s)."
275345
resp = Response().error(message)
276346
resp.data = {
277347
"total": total,
278348
"succeeded": succeeded,
279349
"failed": failed,
350+
"skipped": skipped,
280351
}
281352
return resp.__dict__
282353

@@ -288,6 +359,7 @@ async def batch_upload_skills(self):
288359
"total": total,
289360
"succeeded": succeeded,
290361
"failed": failed,
362+
"skipped": skipped,
291363
},
292364
message,
293365
)

dashboard/src/components/extension/SkillsSection.vue

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -900,6 +900,7 @@ export default {
900900
const applyUploadResults = (attemptedItems, payload) => {
901901
const succeededMap = buildResultMap(payload?.succeeded);
902902
const failedMap = buildResultMap(payload?.failed);
903+
const skippedMap = buildResultMap(payload?.skipped);
903904
904905
for (const item of attemptedItems) {
905906
const successEntry = takeFirstMatch(succeededMap, item.filenameKey);
@@ -911,6 +912,14 @@ export default {
911912
continue;
912913
}
913914
915+
const skippedEntry = takeFirstMatch(skippedMap, item.filenameKey);
916+
if (skippedEntry) {
917+
item.status = STATUS_SKIPPED;
918+
item.validationMessage =
919+
skippedEntry.error || tm("skills.validationDuplicate");
920+
continue;
921+
}
922+
914923
const failedEntry = takeFirstMatch(failedMap, item.filenameKey);
915924
if (failedEntry) {
916925
item.status = STATUS_ERROR;

dashboard/src/i18n/locales/en-US/features/extension.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,7 @@
242242
"emptyHint": "Upload a Skills zip to get started",
243243
"uploadDialogTitle": "Upload Skills",
244244
"uploadHint": "Upload multiple zip skill packages or drag them in. The system validates the structure automatically and shows a result for each file.",
245-
"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`.",
245+
"structureRequirement": "The archive supports multiple skills folders.",
246246
"abilityMultiple": "Upload multiple zip files at once",
247247
"abilityValidate": "Validate `SKILL.md` automatically",
248248
"abilitySkip": "Automatically skip duplicate files.",

dashboard/src/i18n/locales/ru-RU/features/extension.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@
241241
"emptyHint": "Пожалуйста, загрузите архив с навыками",
242242
"uploadDialogTitle": "Загрузка навыков",
243243
"uploadHint": "Поддерживается массовая загрузка zip-архивов. Вы также можете перетащить файлы в это окно. Система автоматически проверит структуру каждого архива.",
244-
"structureRequirement": "Архив должен содержать одну корневую папку (например, `skillname/`), внутри которой обязательно должен находиться файл `SKILL.md`.",
244+
"structureRequirement": "Поддерживаются архивы с несколькими папками skills.",
245245
"abilityMultiple": "Поддержка массовой загрузки",
246246
"abilityValidate": "Автопроверка `SKILL.md`",
247247
"abilitySkip": "Пропуск дубликатов",

dashboard/src/i18n/locales/zh-CN/features/extension.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@
245245
"emptyHint": "请上传 Skills 压缩包",
246246
"uploadDialogTitle": "上传 Skills",
247247
"uploadHint": "支持批量上传 zip 技能包,也支持拖拽批量上传 zip 技能包。系统会自动校验目录结构,并给出逐个文件的结果。",
248-
"structureRequirement": "常见失败原因是压缩包结构不正确。每个 zip 必须只包含一个顶层目录,例如 `skillname/`,且该目录下必须存在 `SKILL.md`",
248+
"structureRequirement": "支持压缩包内含多个 skills 文件夹",
249249
"abilityMultiple": "支持一次上传多个zip文件",
250250
"abilityValidate": "自动校验 `SKILL.md`",
251251
"abilitySkip": "自动跳过重复文件",

0 commit comments

Comments
 (0)