From dc6443755fa90283f87785fb13385e4eb67a94c0 Mon Sep 17 00:00:00 2001 From: Wenjie Zhang Date: Wed, 3 Jun 2026 20:29:04 +0800 Subject: [PATCH] =?UTF-8?q?fix:=20=E5=85=BC=E5=AE=B9=20Skill=20=E5=B1=95?= =?UTF-8?q?=E7=A4=BA=E5=90=8D=E4=B8=8E=20slug=20=E5=88=86=E7=A6=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- backend/package/yuxi/agents/skills/service.py | 75 +++--- .../test/unit/services/test_skill_service.py | 237 +++++++++++++++++- docs/agents/skills-management.md | 13 +- docs/develop-guides/roadmap.md | 2 +- 4 files changed, 289 insertions(+), 38 deletions(-) diff --git a/backend/package/yuxi/agents/skills/service.py b/backend/package/yuxi/agents/skills/service.py index 827f2f50..fdddace4 100644 --- a/backend/package/yuxi/agents/skills/service.py +++ b/backend/package/yuxi/agents/skills/service.py @@ -531,14 +531,23 @@ async def update_skill_dependencies( ) -def _validate_skill_name(name: str) -> str: +def _validate_skill_slug_value(slug: str, *, field_name: str) -> str: + slug = slug.strip() + if not slug: + raise ValueError(f"SKILL.md frontmatter 缺少 {field_name}") + if len(slug) > 128: + raise ValueError(f"SKILL.md frontmatter.{field_name} 长度不能超过 128") + if not SKILL_NAME_PATTERN.match(slug): + raise ValueError(f"SKILL.md frontmatter.{field_name} 必须是小写字母/数字/短横线，且不能连续短横线") + return slug + + +def _validate_skill_display_name(name: str) -> str: name = name.strip() if not name: raise ValueError("SKILL.md frontmatter 缺少 name") if len(name) > 128: - raise ValueError("skill name 长度不能超过 128") - if not SKILL_NAME_PATTERN.match(name): - raise ValueError("skill name 必须是小写字母/数字/短横线，且不能连续短横线") + raise ValueError("SKILL.md frontmatter.name 长度不能超过 128") return name @@ -565,7 +574,7 @@ def _split_frontmatter(content: str) -> tuple[str, str]: return frontmatter_raw, body -def _parse_skill_markdown(content: str) -> tuple[str, str, dict[str, Any]]: +def _parse_skill_markdown(content: str) -> tuple[str, str, str, dict[str, Any]]: frontmatter_raw, _body = _split_frontmatter(content) try: data = yaml.safe_load(frontmatter_raw) @@ -575,20 +584,27 @@ def _parse_skill_markdown(content: str) -> tuple[str, str, dict[str, Any]]: if not isinstance(data, dict): raise ValueError("SKILL.md frontmatter 必须是对象") - name = _validate_skill_name(str(data.get("name", ""))) + name = _validate_skill_display_name(str(data.get("name", ""))) + raw_slug = str(data.get("slug", "")).strip() + slug = _validate_skill_slug_value(raw_slug, field_name="slug") if raw_slug else _validate_skill_slug_value( + name, field_name="name" + ) description = str(data.get("description", "")).strip() if not description: raise ValueError("SKILL.md frontmatter 缺少 description") - return name, description, data + return slug, name, description, data -def _rewrite_frontmatter_name(content: str, new_name: str) -> str: +def _rewrite_frontmatter_slug(content: str, new_slug: str) -> str: frontmatter_raw, body = _split_frontmatter(content) data = yaml.safe_load(frontmatter_raw) if not isinstance(data, dict): raise ValueError("SKILL.md frontmatter 必须是对象") - data["name"] = new_name + if data.get("slug"): + data["slug"] = new_slug + else: + data["name"] = new_slug dumped = yaml.safe_dump(data, sort_keys=False, allow_unicode=True).strip() return f"---\n{dumped}\n---\n{body}" @@ -621,8 +637,9 @@ def _parse_skill_dir_metadata(source_skill_dir: Path) -> dict[str, Any]: raise ValueError("技能目录缺少根级 SKILL.md") content = skill_md_path.read_text(encoding="utf-8") - parsed_name, parsed_desc, meta = _parse_skill_markdown(content) + parsed_slug, parsed_name, parsed_desc, meta = _parse_skill_markdown(content) return { + "slug": parsed_slug, "name": parsed_name, "description": parsed_desc, "tool_dependencies": normalize_string_list(meta.get("tool_dependencies")), @@ -641,19 +658,19 @@ async def _stage_skill_draft_item( item_dir = draft_items_dir / item_id shutil.copytree(source_skill_dir, item_dir, symlinks=False) parsed = _parse_skill_dir_metadata(item_dir) - final_slug = await _generate_available_slug(repo, parsed["name"]) + final_slug = await _generate_available_slug(repo, parsed["slug"]) return { "draft_item_id": item_id, "source_dir": f"items/{item_id}", "slug": final_slug, - "name": final_slug, - "original_name": parsed["name"], + "name": parsed["name"], + "original_name": parsed["slug"], "description": parsed["description"], "tool_dependencies": parsed["tool_dependencies"], "mcp_dependencies": parsed["mcp_dependencies"], "skill_dependencies": parsed["skill_dependencies"], - "warnings": [f"原始名称 {parsed['name']} 已存在，将安装为 {final_slug}"] - if final_slug != parsed["name"] + "warnings": [f"原始 slug {parsed['slug']} 已存在，将安装为 {final_slug}"] + if final_slug != parsed["slug"] else [], "success": True, } @@ -683,14 +700,14 @@ async def _import_skill_dir_impl( repo = SkillRepository(db) skills_root = get_skills_root_dir() parsed = _parse_skill_dir_metadata(source_skill_dir) - final_slug = await _generate_available_slug(repo, parsed["name"]) + final_slug = await _generate_available_slug(repo, parsed["slug"]) with tempfile.TemporaryDirectory(prefix=".skill-import-", dir=str(skills_root.parent)) as temp_root: stage_dir = Path(temp_root) / "stage" shutil.copytree(source_skill_dir, stage_dir) - if final_slug != parsed["name"]: + if final_slug != parsed["slug"]: content = (stage_dir / "SKILL.md").read_text(encoding="utf-8") - (stage_dir / "SKILL.md").write_text(_rewrite_frontmatter_name(content, final_slug), encoding="utf-8") + (stage_dir / "SKILL.md").write_text(_rewrite_frontmatter_slug(content, final_slug), encoding="utf-8") temp_target = skills_root / f".{final_slug}.tmp-{uuid.uuid4().hex[:8]}" if temp_target.exists(): @@ -706,7 +723,7 @@ async def _import_skill_dir_impl( try: item = await repo.create( slug=final_slug, - name=final_slug, + name=parsed["name"], description=parsed["description"], source_type=source_type, tool_dependencies=parsed["tool_dependencies"], @@ -942,9 +959,9 @@ async def confirm_skill_install_draft( with tempfile.TemporaryDirectory(prefix=".skill-confirm-", dir=str(skills_root.parent)) as temp_root: stage_dir = Path(temp_root) / "stage" shutil.copytree(source_dir, stage_dir) - if parsed["name"] != slug: + if parsed["slug"] != slug: content = (stage_dir / "SKILL.md").read_text(encoding="utf-8") - (stage_dir / "SKILL.md").write_text(_rewrite_frontmatter_name(content, slug), encoding="utf-8") + (stage_dir / "SKILL.md").write_text(_rewrite_frontmatter_slug(content, slug), encoding="utf-8") temp_target = skills_root / f".{slug}.tmp-{uuid.uuid4().hex[:8]}" shutil.move(str(stage_dir), str(temp_target)) @@ -958,7 +975,7 @@ async def confirm_skill_install_draft( try: item = await repo.create( slug=slug, - name=slug, + name=parsed["name"], description=parsed["description"], source_type=source_type, tool_dependencies=parsed["tool_dependencies"], @@ -1135,9 +1152,9 @@ async def _update_skill_metadata_if_skills_md( ) -> None: """如果目标文件是 SKILL.md，则解析并更新元数据""" if target.name == "SKILL.md" and target.parent == skill_dir: - parsed_name, parsed_desc, _ = _parse_skill_markdown(content) - if parsed_name != item.slug: - raise ValueError("SKILL.md frontmatter.name 必须与 skill slug 一致") + parsed_slug, parsed_name, parsed_desc, _ = _parse_skill_markdown(content) + if parsed_slug != item.slug: + raise ValueError("SKILL.md frontmatter.slug 必须与 skill slug 一致") repo = SkillRepository(db) await repo.update_metadata(item, name=parsed_name, description=parsed_desc, updated_by=updated_by) @@ -1267,14 +1284,14 @@ def list_builtin_skill_specs() -> list[dict[str, Any]]: raise ValueError(f"内置 skill 缺少 SKILL.md: {source_dir}") content = skill_md.read_text(encoding="utf-8") - parsed_name, parsed_desc, meta = _parse_skill_markdown(content) - if parsed_name != slug: - raise ValueError(f"内置 skill frontmatter.name 必须等于 slug: {slug}") + parsed_slug, parsed_name, parsed_desc, meta = _parse_skill_markdown(content) + if parsed_slug != slug: + raise ValueError(f"内置 skill frontmatter.slug 必须等于 slug: {slug}") specs.append( { "slug": slug, - "name": slug, + "name": parsed_name, "description": configured_description or parsed_desc, "version": version, "tool_dependencies": configured_tools or normalize_string_list(meta.get("tool_dependencies")), diff --git a/backend/test/unit/services/test_skill_service.py b/backend/test/unit/services/test_skill_service.py index 58f961c3..875c56e4 100644 --- a/backend/test/unit/services/test_skill_service.py +++ b/backend/test/unit/services/test_skill_service.py @@ -74,8 +74,14 @@ async def test_list_visible_skills_for_management_includes_owned_disabled_and_en @pytest.mark.parametrize( "skill,operator", [ - (Skill(slug="owned-disabled", name="owned-disabled", description="", created_by="root", enabled=False), _user("root", role="user")), - (Skill(slug="admin-disabled", name="admin-disabled", description="", created_by="other", enabled=False), _user("root", role="admin")), + ( + Skill(slug="owned-disabled", name="owned-disabled", description="", created_by="root", enabled=False), + _user("root", role="user"), + ), + ( + Skill(slug="admin-disabled", name="admin-disabled", description="", created_by="other", enabled=False), + _user("root", role="admin"), + ), ( Skill( slug="shared-enabled", @@ -233,12 +239,32 @@ async def test_normal_user_confirm_skill_draft_rejects_wider_share_scope( def test_parse_skill_markdown_ok(): content = "---\nname: demo-skill\ndescription: demo description\n---\n# Demo\n" - name, desc, meta = svc._parse_skill_markdown(content) + slug, name, desc, meta = svc._parse_skill_markdown(content) + assert slug == "demo-skill" assert name == "demo-skill" assert desc == "demo description" assert meta["name"] == "demo-skill" +def test_parse_skill_markdown_supports_display_name_with_slug(): + content = ( + "---\n" + "name: Word / DOCX\n" + "slug: word-docx\n" + "version: 1.0.2\n" + "homepage: https://clawic.com/skills/word-docx\n" + "description: Create, inspect, and edit Microsoft Word documents.\n" + 'metadata: {"clawdbot":{"emoji":"📘","os":["linux","darwin","win32"]}}\n' + "---\n" + "# Word / DOCX\n" + ) + slug, name, desc, meta = svc._parse_skill_markdown(content) + assert slug == "word-docx" + assert name == "Word / DOCX" + assert desc == "Create, inspect, and edit Microsoft Word documents." + assert meta["version"] == "1.0.2" + + def test_parse_skill_markdown_requires_frontmatter(): with pytest.raises(ValueError, match="frontmatter"): svc._parse_skill_markdown("# missing") @@ -385,6 +411,211 @@ async def test_skill_upload_prepare_confirm_rewrites_conflicting_name(tmp_path: assert "name: demo-v2" in skill_md +@pytest.mark.asyncio +async def test_skill_zip_import_uses_skill_md_name_not_zip_or_root_dir( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path)) + + class FakeRepo: + created_item: Skill | None = None + + def __init__(self, _db): + pass + + async def exists_slug(self, _slug: str) -> bool: + return False + + async def create(self, **kwargs) -> Skill: + item = Skill(**kwargs, updated_by=kwargs["created_by"]) + self.__class__.created_item = item + return item + + monkeypatch.setattr(svc, "SkillRepository", FakeRepo) + + zip_bytes = _build_zip( + { + "Bad--Archive-Name/SKILL.md": ( + "---\nname: valid-skill\ndescription: this is valid\n---\n# Valid\n" + ), + "Bad--Archive-Name/prompts/system.md": "Use valid skill metadata.", + } + ) + operator = _user("root") + + draft = await svc.prepare_skill_upload( + None, + filename="Bad--Archive-Name.zip", + file_bytes=zip_bytes, + operator=operator, + ) + results = await svc.confirm_skill_install_draft( + None, + draft_id=draft["draft_id"], + share_config=draft["default_share_config"], + operator=operator, + ) + + assert draft["items"][0]["original_name"] == "valid-skill" + assert draft["items"][0]["slug"] == "valid-skill" + assert results[0]["success"] is True + assert results[0]["slug"] == "valid-skill" + assert FakeRepo.created_item.slug == "valid-skill" + assert (tmp_path / "skills" / "valid-skill" / "SKILL.md").exists() + + +@pytest.mark.asyncio +async def test_skill_zip_import_validates_skill_md_name_not_zip_filename( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path)) + + class FakeRepo: + def __init__(self, _db): + pass + + async def exists_slug(self, _slug: str) -> bool: + return False + + monkeypatch.setattr(svc, "SkillRepository", FakeRepo) + + zip_bytes = _build_zip( + { + "valid-archive/SKILL.md": ( + "---\nname: invalid--skill\ndescription: invalid name\n---\n# Invalid\n" + ), + } + ) + + with pytest.raises(ValueError, match="SKILL.md frontmatter.name 必须是小写字母/数字/短横线"): + await svc.prepare_skill_upload( + None, + filename="valid-archive.zip", + file_bytes=zip_bytes, + operator=_user("root"), + ) + + +@pytest.mark.asyncio +async def test_skill_zip_import_uses_frontmatter_slug_and_keeps_display_name( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path)) + + class FakeRepo: + created_item: Skill | None = None + + def __init__(self, _db): + pass + + async def exists_slug(self, _slug: str) -> bool: + return False + + async def create(self, **kwargs) -> Skill: + item = Skill(**kwargs, updated_by=kwargs["created_by"]) + self.__class__.created_item = item + return item + + monkeypatch.setattr(svc, "SkillRepository", FakeRepo) + + zip_bytes = _build_zip( + { + "Word Skill/SKILL.md": ( + "---\n" + "name: Word / DOCX\n" + "slug: word-docx\n" + "version: 1.0.2\n" + "homepage: https://clawic.com/skills/word-docx\n" + "description: Create, inspect, and edit Microsoft Word documents.\n" + "changelog: Tightened review workflows.\n" + 'metadata: {"clawdbot":{"emoji":"📘","os":["linux","darwin","win32"]}}\n' + "---\n" + "# Word / DOCX\n" + ) + } + ) + operator = _user("root") + + draft = await svc.prepare_skill_upload( + None, + filename="Word Skill.zip", + file_bytes=zip_bytes, + operator=operator, + ) + results = await svc.confirm_skill_install_draft( + None, + draft_id=draft["draft_id"], + share_config=draft["default_share_config"], + operator=operator, + ) + + assert draft["items"][0]["slug"] == "word-docx" + assert draft["items"][0]["name"] == "Word / DOCX" + assert results[0]["success"] is True + assert results[0]["slug"] == "word-docx" + assert FakeRepo.created_item.slug == "word-docx" + assert FakeRepo.created_item.name == "Word / DOCX" + + +@pytest.mark.asyncio +async def test_skill_zip_import_rewrites_conflicting_slug_not_display_name( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +): + monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path)) + + class FakeRepo: + existing_slugs = {"word-docx"} + created_item: Skill | None = None + + def __init__(self, _db): + pass + + async def exists_slug(self, slug: str) -> bool: + return slug in self.__class__.existing_slugs + + async def create(self, **kwargs) -> Skill: + item = Skill(**kwargs, updated_by=kwargs["created_by"]) + self.__class__.existing_slugs.add(item.slug) + self.__class__.created_item = item + return item + + monkeypatch.setattr(svc, "SkillRepository", FakeRepo) + + zip_bytes = _build_zip( + { + "Word Skill/SKILL.md": ( + "---\n" + "name: Word / DOCX\n" + "slug: word-docx\n" + "description: Create, inspect, and edit Microsoft Word documents.\n" + "---\n" + "# Word / DOCX\n" + ) + } + ) + operator = _user("root") + + draft = await svc.prepare_skill_upload( + None, + filename="Word Skill.zip", + file_bytes=zip_bytes, + operator=operator, + ) + results = await svc.confirm_skill_install_draft( + None, + draft_id=draft["draft_id"], + share_config=draft["default_share_config"], + operator=operator, + ) + + assert results[0]["slug"] == "word-docx-v2" + assert results[0]["success"] is True + assert FakeRepo.created_item.name == "Word / DOCX" + skill_md = (tmp_path / "skills" / "word-docx-v2" / "SKILL.md").read_text(encoding="utf-8") + assert "name: Word / DOCX" in skill_md + assert "slug: word-docx-v2" in skill_md + + @pytest.mark.asyncio async def test_skill_md_prepare_confirm_creates_single_file_skill(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): monkeypatch.setattr(svc.sys_config, "save_dir", str(tmp_path)) diff --git a/docs/agents/skills-management.md b/docs/agents/skills-management.md index 07789825..5eaebae7 100644 --- a/docs/agents/skills-management.md +++ b/docs/agents/skills-management.md @@ -77,7 +77,8 @@ my-awesome-skill/ ```markdown --- -name: my-awesome-skill +name: My Awesome Skill +slug: my-awesome-skill description: 这是一个用于处理特定任务的技能 --- @@ -99,7 +100,8 @@ description: 这是一个用于处理特定任务的技能 | 字段 | 必填 | 说明 | |------|------|------| -| `name` | 是 | Skill 名称，必须是小写字母、数字、短横线的组合（如 `my-skill`） | +| `name` | 是 | Skill 展示名称，可使用更易读的名称（如 `Word / DOCX`） | +| `slug` | 否 | Skill 唯一标识，必须是小写字母、数字、短横线的组合，且不能连续短横线（如 `my-skill`）。未填写时兼容旧格式，系统会使用 `name` 作为 slug，此时 `name` 也必须满足 slug 规则 | | `description` | 是 | Skill 的功能描述，会在 Agent 配置时展示 | ### 导入 Skill @@ -237,9 +239,10 @@ Skills 管理采用基于角色的权限控制： ### Skill 命名规范 -- 使用小写字母、数字和短横线 -- 具有描述性，如 `weather-query`、`sql-reporter` -- 避免过长的名称 +- `slug` 使用小写字母、数字和短横线，不能连续短横线 +- `slug` 应具有描述性，如 `weather-query`、`sql-reporter` +- `name` 用于展示，可比 `slug` 更自然，例如 `Word / DOCX` +- 避免过长的 `name` 和 `slug` ### 依赖管理建议 diff --git a/docs/develop-guides/roadmap.md b/docs/develop-guides/roadmap.md index 1e1acf41..a9e6192b 100644 --- a/docs/develop-guides/roadmap.md +++ b/docs/develop-guides/roadmap.md @@ -49,7 +49,7 @@ - 新增 Milvus 图谱检索链路：Query 可召回图谱实体和三元组，结合 Chunk 命中实体构造 seed entity，读取 Neo4j 2-hop 子图后用 igraph 执行 PPR，最终以 Chunk 为产物并通过 RRF 与原 Chunk 召回融合；检索配置改为 dataclass 元数据生成，支持 `depend_on` 控制重排序和图检索参数展示。 - 收紧用户管理部门隔离：普通管理员创建用户时固定归属本部门，用户列表、访问选项、详情、更新和删除接口均限制在本部门范围内。 - 调整 Agent 资源默认选择与运行时上下文：未显式配置工具、知识库、MCP、Skills 时默认启用当前用户可访问/可用的全部资源，子智能体默认不启用，显式保存空列表仍表示不启用对应资源；Agent 创建前统一完成最终资源权限过滤、知识库 `db_id` 可见范围派生和 Skill prompt/readable 依赖闭包派生，聊天运行时与文件系统预览复用同一结果。 -- 重构 Skills 权限与安装流程：Skill 增加 `source_type/share_config/enabled`，内置 Skill 作为启动同步入库的全局资源，不再保留前端安装/更新状态，支持启停但不允许删除；上传和远程添加统一为解析草稿后确认生效范围，管理端支持编辑生效范围与启停；Agent 运行时按当前用户可访问 Skills 派生 prompt/readable 依赖闭包并限制挂载/激活，Skills prompt 改为模型请求级注入以避免污染 runtime context。 +- 重构 Skills 权限与安装流程：Skill 增加 `source_type/share_config/enabled`，内置 Skill 作为启动同步入库的全局资源，不再保留前端安装/更新状态，支持启停但不允许删除；上传和远程添加统一为解析草稿后确认生效范围，安装 slug 优先读取 `SKILL.md` 的 `slug` 字段并保留 `name` 展示名，压缩包名称不参与 slug 校验；管理端支持编辑生效范围与启停；Agent 运行时按当前用户可访问 Skills 派生 prompt/readable 依赖闭包并限制挂载/激活，Skills prompt 改为模型请求级注入以避免污染 runtime context。 - 精简历史兼容层：移除 sandbox provisioner `local` 后端别名、ask_user_question 单问题旧协议、JWT 历史默认密钥特殊判断、内置 Skill `SKILLS.md` 文件名回退、运行事件数字 seq 兼容和前端若干旧字段回退。 - 重构知识库共享权限：`share_config` 改为全局共享、部门共享、指定人可访问三档，部门共享必须包含当前用户部门，指定人可访问必须包含当前用户，并补充权限过滤测试。 - 移除知识库沙盒文件系统映射：不再通过 `/home/gem/kbs` 暴露知识库文件树，Agent 继续使用 `query_kb` 与 `open_kb_document` 访问知识库内容。