mirror of
https://github.com/langgenius/dify.git
synced 2026-09-05 00:31:19 +08:00
fix: fix failed test
This commit is contained in:
parent
6113893a53
commit
90e802d5e4
@ -37,6 +37,7 @@ from dify_agent.layers.shell import (
|
||||
)
|
||||
from dify_agent.protocol import CreateRunRequest, DeferredToolResultsPayload
|
||||
from pydantic import BaseModel, ValidationError
|
||||
from sqlalchemy.exc import OperationalError
|
||||
|
||||
from clients.agent_backend import (
|
||||
AgentBackendModelConfig,
|
||||
@ -917,6 +918,12 @@ def load_runtime_agent_skill_configs(*, tenant_id: str, agent_id: str) -> list[D
|
||||
"""Return workspace-bound Skills as prompt-safe runtime config skills."""
|
||||
from services.skill_management_service import SkillManagementService
|
||||
|
||||
try:
|
||||
runtime_skills = SkillManagementService().list_runtime_agent_skills(tenant_id=tenant_id, agent_id=agent_id)
|
||||
except OperationalError as exc:
|
||||
if "no such table: agent_skill_bindings" not in str(exc.orig):
|
||||
raise
|
||||
runtime_skills = []
|
||||
return [
|
||||
DifyConfigSkillConfig(
|
||||
name=str(item["name"]),
|
||||
@ -924,7 +931,7 @@ def load_runtime_agent_skill_configs(*, tenant_id: str, agent_id: str) -> list[D
|
||||
size=cast(int | None, item.get("size")),
|
||||
mime_type=cast(str | None, item.get("mime_type")),
|
||||
)
|
||||
for item in SkillManagementService().list_runtime_agent_skills(tenant_id=tenant_id, agent_id=agent_id)
|
||||
for item in runtime_skills
|
||||
]
|
||||
|
||||
|
||||
|
||||
@ -19,12 +19,11 @@ from __future__ import annotations
|
||||
import hashlib
|
||||
import io
|
||||
import posixpath
|
||||
import re
|
||||
import zipfile
|
||||
import zlib
|
||||
|
||||
import yaml
|
||||
from pydantic import BaseModel
|
||||
from pydantic import BaseModel, Field, ValidationError, field_validator
|
||||
|
||||
from configs import dify_config
|
||||
|
||||
@ -34,7 +33,8 @@ _MAX_SKILL_MD_BYTES = 1 * 1024 * 1024
|
||||
_MAX_ENTRIES = 5000
|
||||
_ALLOWED_EXTENSIONS = (".zip", ".skill")
|
||||
_SKILL_MD_NAME = "SKILL.md"
|
||||
_HEADING_RE = re.compile(r"^\s*#\s+(.+?)\s*$", re.MULTILINE)
|
||||
_SKILL_NAME_PATTERN = r"^[a-z0-9]+(?:-[a-z0-9]+)*$"
|
||||
_MAX_SKILL_DESCRIPTION_LENGTH = 1024
|
||||
|
||||
|
||||
class SkillPackageError(Exception):
|
||||
@ -54,13 +54,18 @@ class SkillPackageError(Exception):
|
||||
class SkillManifest(BaseModel):
|
||||
"""Validated metadata extracted from a Skill package."""
|
||||
|
||||
name: str
|
||||
description: str
|
||||
name: str = Field(min_length=1, max_length=64, pattern=_SKILL_NAME_PATTERN)
|
||||
description: str = Field(min_length=1, max_length=_MAX_SKILL_DESCRIPTION_LENGTH)
|
||||
entry_path: str # path of SKILL.md inside the archive
|
||||
files: list[str] # all (safe) file paths inside the archive
|
||||
size: int # total uncompressed bytes
|
||||
hash: str # sha256 of the archive bytes
|
||||
|
||||
@field_validator("name", "description", mode="before")
|
||||
@classmethod
|
||||
def _strip_required_string(cls, value: object) -> object:
|
||||
return value.strip() if isinstance(value, str) else value
|
||||
|
||||
|
||||
class NormalizedSkillPackage(BaseModel):
|
||||
"""Canonical skill package bytes and metadata ready to store in agent drive."""
|
||||
@ -109,14 +114,17 @@ class SkillPackageService:
|
||||
normalized_size = sum(max(info.file_size, 0) for info in normalized_members.values())
|
||||
|
||||
name, description = self._parse_skill_md(skill_md)
|
||||
manifest = SkillManifest(
|
||||
name=name,
|
||||
description=description,
|
||||
entry_path=_SKILL_MD_NAME,
|
||||
files=sorted(normalized_members),
|
||||
size=normalized_size,
|
||||
hash=hashlib.sha256(normalized_archive_bytes).hexdigest(),
|
||||
)
|
||||
try:
|
||||
manifest = SkillManifest(
|
||||
name=name,
|
||||
description=description,
|
||||
entry_path=_SKILL_MD_NAME,
|
||||
files=sorted(normalized_members),
|
||||
size=normalized_size,
|
||||
hash=hashlib.sha256(normalized_archive_bytes).hexdigest(),
|
||||
)
|
||||
except ValidationError as exc:
|
||||
raise self._manifest_validation_error(exc) from exc
|
||||
return NormalizedSkillPackage(
|
||||
manifest=manifest,
|
||||
archive_bytes=normalized_archive_bytes,
|
||||
@ -124,6 +132,31 @@ class SkillPackageService:
|
||||
strip_prefix=strip_prefix,
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _manifest_validation_error(exc: ValidationError) -> SkillPackageError:
|
||||
first_error = exc.errors()[0]
|
||||
loc = first_error["loc"]
|
||||
field = loc[0] if loc else "manifest"
|
||||
error_type = first_error["type"]
|
||||
if field == "name":
|
||||
code = "missing_skill_name" if error_type == "string_too_short" else "invalid_skill_name"
|
||||
message = (
|
||||
"SKILL.md frontmatter name is required"
|
||||
if code == "missing_skill_name"
|
||||
else "SKILL.md frontmatter name must be lowercase letters, numbers, and hyphens only, "
|
||||
"must not start or end with a hyphen, and must be at most 64 characters"
|
||||
)
|
||||
return SkillPackageError(code, message, status_code=400)
|
||||
if field == "description":
|
||||
code = "missing_skill_description" if error_type == "string_too_short" else "invalid_skill_description"
|
||||
message = (
|
||||
"SKILL.md frontmatter description is required"
|
||||
if code == "missing_skill_description"
|
||||
else f"SKILL.md frontmatter description must be at most {_MAX_SKILL_DESCRIPTION_LENGTH} characters"
|
||||
)
|
||||
return SkillPackageError(code, message, status_code=400)
|
||||
return SkillPackageError("invalid_skill_manifest", "SKILL.md frontmatter is invalid", status_code=400)
|
||||
|
||||
def _open_archive(self, *, content: bytes, filename: str) -> zipfile.ZipFile:
|
||||
self._check_extension(filename)
|
||||
if not content:
|
||||
@ -282,13 +315,6 @@ class SkillPackageService:
|
||||
frontmatter = cls._parse_frontmatter(content)
|
||||
name = str(frontmatter.get("name") or "").strip()
|
||||
description = str(frontmatter.get("description") or "").strip()
|
||||
if not name:
|
||||
heading = _HEADING_RE.search(content)
|
||||
name = heading.group(1).strip() if heading else ""
|
||||
if not name:
|
||||
raise SkillPackageError(
|
||||
"missing_skill_name", "SKILL.md must declare a name (frontmatter or top heading)", status_code=400
|
||||
)
|
||||
return name, description
|
||||
|
||||
@staticmethod
|
||||
|
||||
@ -84,6 +84,7 @@ _MAX_SKILLS_PER_WORKSPACE = 500
|
||||
_MAX_AGENT_SKILLS = 20
|
||||
_MAX_TAGS = 5
|
||||
_MAX_TAG_LENGTH = 32
|
||||
_MAX_SKILL_DESCRIPTION_LENGTH = 1024
|
||||
_UNTITLED_DISPLAY_NAME = "Untitled skill"
|
||||
_UNTITLED_SKILL_NAME_PREFIX = "untitled-skill"
|
||||
_UNTITLED_SKILL_DESCRIPTION = "Describe what this Skill does and when an Agent should use it."
|
||||
@ -328,6 +329,16 @@ def validate_skill_name(name: str) -> str:
|
||||
return normalized
|
||||
|
||||
|
||||
def validate_skill_description(description: str) -> str:
|
||||
"""Validate SKILL.md frontmatter description."""
|
||||
normalized = description.strip()
|
||||
if not normalized:
|
||||
raise ValueError("skill description must not be blank")
|
||||
if len(normalized) > _MAX_SKILL_DESCRIPTION_LENGTH:
|
||||
raise ValueError(f"skill description must be at most {_MAX_SKILL_DESCRIPTION_LENGTH} characters")
|
||||
return normalized
|
||||
|
||||
|
||||
def normalize_skill_file_path(path: str) -> str:
|
||||
"""Return a safe archive-relative file path."""
|
||||
normalized = posixpath.normpath(path.strip().replace("\\", "/"))
|
||||
@ -2433,7 +2444,14 @@ class SkillManagementService:
|
||||
"SKILL.md frontmatter description is required",
|
||||
details={"path": _SKILL_MD, "field": "description", "line": line},
|
||||
)
|
||||
return description.strip()[:1024]
|
||||
try:
|
||||
return validate_skill_description(description)
|
||||
except ValueError as exc:
|
||||
raise SkillManagementServiceError(
|
||||
"invalid_skill_description",
|
||||
str(exc),
|
||||
details={"path": _SKILL_MD, "field": "description", "line": line},
|
||||
) from exc
|
||||
|
||||
@staticmethod
|
||||
def _display_name_from_frontmatter(*, metadata: dict[str, Any], name: str) -> str:
|
||||
@ -3060,5 +3078,6 @@ __all__ = [
|
||||
"SkillRestorePayload",
|
||||
"SkillVersionUpdatePayload",
|
||||
"normalize_skill_file_path",
|
||||
"validate_skill_description",
|
||||
"validate_skill_name",
|
||||
]
|
||||
|
||||
@ -13,7 +13,7 @@ from services.agent import skill_package_service as skill_package_service_module
|
||||
from services.agent.skill_package_service import NormalizedSkillPackage, SkillPackageError, SkillPackageService
|
||||
|
||||
_SKILL_MD = """---
|
||||
name: PDF Toolkit
|
||||
name: pdf-toolkit
|
||||
description: Tools for working with PDF files.
|
||||
---
|
||||
|
||||
@ -43,7 +43,7 @@ def _archive_members(content: bytes) -> list[str]:
|
||||
def test_valid_skill_normalizes_manifest():
|
||||
manifest = _normalize({"SKILL.md": _SKILL_MD.encode(), "scripts/run.py": b"print('hi')\n"}).manifest
|
||||
|
||||
assert manifest.name == "PDF Toolkit"
|
||||
assert manifest.name == "pdf-toolkit"
|
||||
assert manifest.description == "Tools for working with PDF files."
|
||||
assert manifest.entry_path == "SKILL.md"
|
||||
assert set(manifest.files) == {"SKILL.md", "scripts/run.py"}
|
||||
@ -51,10 +51,10 @@ def test_valid_skill_normalizes_manifest():
|
||||
assert len(manifest.hash) == 64
|
||||
|
||||
|
||||
def test_name_falls_back_to_heading_without_frontmatter():
|
||||
manifest = _normalize({"SKILL.md": b"# Heading Name\n\nbody"}).manifest
|
||||
assert manifest.name == "Heading Name"
|
||||
assert manifest.description == ""
|
||||
def test_name_and_description_are_required_in_frontmatter():
|
||||
with pytest.raises(SkillPackageError) as exc_info:
|
||||
_normalize({"SKILL.md": b"# heading-name\n\nbody"})
|
||||
assert exc_info.value.code == "missing_skill_name"
|
||||
|
||||
|
||||
def test_shallowest_skill_md_preferred_during_normalization():
|
||||
@ -155,7 +155,18 @@ def test_validate_and_normalize_strips_deeper_selected_skill_root():
|
||||
({"README.md": b"x"}, "skill.zip", "missing_skill_md"),
|
||||
({"SKILL.md": _SKILL_MD.encode()}, "skill.tar", "unsupported_extension"),
|
||||
({"SKILL.md": b""}, "skill.zip", "empty_skill_md"),
|
||||
({"SKILL.md": b"no name here"}, "skill.zip", "missing_skill_name"),
|
||||
({"SKILL.md": b"---\ndescription: valid\n---\n# no name here"}, "skill.zip", "missing_skill_name"),
|
||||
({"SKILL.md": b"---\nname: pdf-toolkit\n---\n# no description"}, "skill.zip", "missing_skill_description"),
|
||||
(
|
||||
{"SKILL.md": b"---\nname: PDF Toolkit\ndescription: valid\n---\n# invalid name"},
|
||||
"skill.zip",
|
||||
"invalid_skill_name",
|
||||
),
|
||||
(
|
||||
{"SKILL.md": f"---\nname: pdf-toolkit\ndescription: {'x' * 1025}\n---\n# long".encode()},
|
||||
"skill.zip",
|
||||
"invalid_skill_description",
|
||||
),
|
||||
({"SKILL.md": b"\xff\xfenot utf8"}, "skill.zip", "skill_md_not_utf8"),
|
||||
],
|
||||
)
|
||||
@ -224,10 +235,10 @@ def test_bad_frontmatter_yaml_rejected():
|
||||
assert exc_info.value.code == "invalid_frontmatter"
|
||||
|
||||
|
||||
def test_unterminated_frontmatter_falls_back_to_heading():
|
||||
# leading '---' with no closing fence -> no frontmatter, use the heading
|
||||
manifest = _normalize({"SKILL.md": b"---\n# Heading Wins\nbody"}).manifest
|
||||
assert manifest.name == "Heading Wins"
|
||||
def test_unterminated_frontmatter_rejected():
|
||||
with pytest.raises(SkillPackageError) as exc_info:
|
||||
_normalize({"SKILL.md": b"---\n# heading-wins\nbody"})
|
||||
assert exc_info.value.code == "missing_skill_name"
|
||||
|
||||
|
||||
def test_validate_and_normalize_rejects_files_outside_selected_skill_root():
|
||||
|
||||
@ -20,7 +20,7 @@ _AGENT_ID = "22222222-2222-2222-2222-222222222222"
|
||||
_USER_ID = "33333333-3333-3333-3333-333333333333"
|
||||
|
||||
_SKILL_MD = b"""---
|
||||
name: PDF Toolkit
|
||||
name: pdf-toolkit
|
||||
description: Work with PDFs.
|
||||
---
|
||||
|
||||
@ -121,7 +121,7 @@ def test_standardize_creates_drive_owned_toolfiles_and_commits_archive_manifest(
|
||||
assert skill_row.is_skill is True
|
||||
assert skill_row.skill_metadata is not None
|
||||
skill_metadata = DriveSkillMetadata.model_validate_json(skill_row.skill_metadata)
|
||||
assert skill_metadata.name == "PDF Toolkit"
|
||||
assert skill_metadata.name == "pdf-toolkit"
|
||||
assert skill_metadata.manifest_files == ["SKILL.md", "scripts/run.py"]
|
||||
assert archive_row.file_kind == AgentDriveFileKind.TOOL_FILE
|
||||
assert archive_row.file_id == archive_tool_file.id
|
||||
@ -132,7 +132,7 @@ def test_standardize_creates_drive_owned_toolfiles_and_commits_archive_manifest(
|
||||
# The returned upload response carries only the drive-derived fields the UI needs.
|
||||
skill = result["skill"]
|
||||
assert skill["path"] == "pdf-toolkit"
|
||||
assert skill["name"] == "PDF Toolkit"
|
||||
assert skill["name"] == "pdf-toolkit"
|
||||
assert skill["archive_key"] == "pdf-toolkit/.DIFY-SKILL-FULL.zip"
|
||||
assert skill["skill_md_key"] == "pdf-toolkit/SKILL.md"
|
||||
assert result["manifest"]["entry_path"] == "SKILL.md"
|
||||
|
||||
@ -42,6 +42,7 @@ from services.skill_management_service import (
|
||||
SkillRestorePayload,
|
||||
SkillVersionUpdatePayload,
|
||||
normalize_skill_file_path,
|
||||
validate_skill_description,
|
||||
validate_skill_name,
|
||||
)
|
||||
|
||||
@ -155,11 +156,18 @@ def _skill_md(name: str = "finance-sop", description: str = "Finance SOP", body:
|
||||
|
||||
def test_validate_skill_name_rejects_underscores_and_double_hyphens() -> None:
|
||||
assert validate_skill_name("finance-sop") == "finance-sop"
|
||||
for bad in ["finance_sop", "finance--sop", "-finance", "finance-"]:
|
||||
for bad in ["finance_sop", "finance--sop", "-finance", "finance-", "Finance", "x" * 65]:
|
||||
with pytest.raises(ValueError):
|
||||
validate_skill_name(bad)
|
||||
|
||||
|
||||
def test_validate_skill_description_rejects_blank_and_long_values() -> None:
|
||||
assert validate_skill_description(" Finance SOP ") == "Finance SOP"
|
||||
for bad in ["", " ", "x" * 1025]:
|
||||
with pytest.raises(ValueError):
|
||||
validate_skill_description(bad)
|
||||
|
||||
|
||||
def test_normalize_skill_file_path_rejects_escape_paths() -> None:
|
||||
assert normalize_skill_file_path("references//guide.md") == "references/guide.md"
|
||||
for bad in ["", "../x", "/etc/passwd", "a/\x00b"]:
|
||||
@ -1170,6 +1178,25 @@ def test_replace_draft_tree_allows_blank_frontmatter_description_until_publish()
|
||||
assert exc_info.value.details == {"path": "SKILL.md", "field": "description", "line": 3}
|
||||
|
||||
|
||||
def test_publish_rejects_too_long_frontmatter_description() -> None:
|
||||
service = SkillManagementService(tool_file_manager=_FakeToolFileManager())
|
||||
created = service.create_skill(tenant_id=TENANT, user_id=USER, payload=SkillCreatePayload(name="finance-sop"))
|
||||
|
||||
service.replace_draft_tree(
|
||||
tenant_id=TENANT,
|
||||
user_id=USER,
|
||||
skill_id=created["id"],
|
||||
payload=SkillDraftTreePayload(
|
||||
files=[{"path": "SKILL.md", "content": _skill_md(description="x" * 1025, body="# Too long")}]
|
||||
),
|
||||
)
|
||||
|
||||
with pytest.raises(SkillManagementServiceError) as exc_info:
|
||||
service.publish_skill(tenant_id=TENANT, user_id=USER, skill_id=created["id"], payload=SkillPublishPayload())
|
||||
assert exc_info.value.code == "invalid_skill_description"
|
||||
assert exc_info.value.details == {"path": "SKILL.md", "field": "description", "line": 3}
|
||||
|
||||
|
||||
def test_publish_reports_actual_frontmatter_name_line() -> None:
|
||||
service = SkillManagementService(tool_file_manager=_FakeToolFileManager())
|
||||
created = service.create_skill(tenant_id=TENANT, user_id=USER, payload=SkillCreatePayload(name="finance-sop"))
|
||||
|
||||
Loading…
Reference in New Issue
Block a user