feat(skills): show YAML load errors for owned custom skills (#6623)
* feat(skills): show YAML load errors for owned custom skills * docs: capture skill diagnostics with real gateway validation * fix(skills): clarify YAML diagnostic scope * docs: remove PR evidence directory --------- Co-authored-by: kwsYeager <331533331+kwsYegar@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
15
README.md
@ -1291,6 +1291,21 @@ view. Built-in and user-created/imported skills are listed separately. The
|
|||||||
Community tab supports importing `.skill` archives into My skills.
|
Community tab supports importing `.skill` archives into My skills.
|
||||||
General preferences remain in Settings.
|
General preferences remain in Settings.
|
||||||
|
|
||||||
|
Administrators can see **Could not load** warnings in Capability Center > Skills
|
||||||
|
when their own custom skill's `SKILL.md` contains invalid YAML. Warnings show the
|
||||||
|
package-relative file and, when available, its line and column, with a quoting
|
||||||
|
hint for unquoted colon values. Fix the file on disk and select **Reload skills**
|
||||||
|
to refresh the list and invalidate skill prompt caches for subsequent runs;
|
||||||
|
active runs retain their existing snapshot. This covers caller-owned custom
|
||||||
|
skills only, excluding legacy/global, public, integration, and external linked
|
||||||
|
packages. These warnings cover YAML syntax errors only: missing frontmatter,
|
||||||
|
non-mapping metadata, missing/invalid name or description, invalid allowed-tools
|
||||||
|
or required-secrets declarations, non-UTF-8 files, and file-read failures
|
||||||
|
(including permission denied) are not reported. A missing warning does not
|
||||||
|
establish that a package is valid or readable. The scope note remains visible
|
||||||
|
even when there are no YAML warnings; check Gateway logs for other load failures.
|
||||||
|
Parsing remains strict and invalid skills stay unavailable to agents.
|
||||||
|
|
||||||
Skills are what make DeerFlow do *almost anything*.
|
Skills are what make DeerFlow do *almost anything*.
|
||||||
|
|
||||||
A standard Agent Skill is a structured capability module — a Markdown file that defines a workflow, best practices, and references to supporting resources. DeerFlow ships with built-in skills for research, report generation, slide creation, web pages, image and video generation, and more. But the real power is extensibility: add your own skills, replace the built-in ones, or combine them into compound workflows.
|
A standard Agent Skill is a structured capability module — a Markdown file that defines a workflow, best practices, and references to supporting resources. DeerFlow ships with built-in skills for research, report generation, slide creation, web pages, image and video generation, and more. But the real power is extensibility: add your own skills, replace the built-in ones, or combine them into compound workflows.
|
||||||
|
|||||||
@ -41,6 +41,7 @@ from deerflow.config.extensions_config import (
|
|||||||
)
|
)
|
||||||
from deerflow.runtime.user_context import get_effective_user_id
|
from deerflow.runtime.user_context import get_effective_user_id
|
||||||
from deerflow.skills import Skill
|
from deerflow.skills import Skill
|
||||||
|
from deerflow.skills.diagnostics import SkillLoadDiagnostic
|
||||||
from deerflow.skills.export import SkillExportError, build_skill_export, export_manifest
|
from deerflow.skills.export import SkillExportError, build_skill_export, export_manifest
|
||||||
from deerflow.skills.installer import SkillAlreadyExistsError, SkillSecurityScanError
|
from deerflow.skills.installer import SkillAlreadyExistsError, SkillSecurityScanError
|
||||||
from deerflow.skills.security_scanner import scan_skill_content
|
from deerflow.skills.security_scanner import scan_skill_content
|
||||||
@ -112,6 +113,10 @@ class SkillsListResponse(BaseModel):
|
|||||||
skills: list[SkillResponse]
|
skills: list[SkillResponse]
|
||||||
|
|
||||||
|
|
||||||
|
class SkillDiagnosticsResponse(BaseModel):
|
||||||
|
diagnostics: list[SkillLoadDiagnostic]
|
||||||
|
|
||||||
|
|
||||||
class SkillUpdateRequest(BaseModel):
|
class SkillUpdateRequest(BaseModel):
|
||||||
"""Request model for updating a skill."""
|
"""Request model for updating a skill."""
|
||||||
|
|
||||||
@ -652,6 +657,21 @@ async def list_custom_skills(request: Request, config: AppConfig = Depends(get_c
|
|||||||
raise HTTPException(status_code=500, detail=f"Failed to list custom skills: {str(e)}")
|
raise HTTPException(status_code=500, detail=f"Failed to list custom skills: {str(e)}")
|
||||||
|
|
||||||
|
|
||||||
|
@router.get("/skills/diagnostics/custom", response_model=SkillDiagnosticsResponse, summary="List Owned Custom Skill YAML Errors")
|
||||||
|
async def list_custom_skill_diagnostics(request: Request, response: Response, config: AppConfig = Depends(get_config)) -> SkillDiagnosticsResponse:
|
||||||
|
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
|
||||||
|
response.headers["Cache-Control"] = "no-store"
|
||||||
|
|
||||||
|
def scan() -> list[SkillLoadDiagnostic]:
|
||||||
|
return _get_user_skill_storage(config).load_custom_skill_diagnostics()
|
||||||
|
|
||||||
|
try:
|
||||||
|
return SkillDiagnosticsResponse(diagnostics=await asyncio.to_thread(scan))
|
||||||
|
except Exception as exc:
|
||||||
|
logger.error("Failed to load custom skill diagnostics (%s)", type(exc).__name__)
|
||||||
|
raise HTTPException(status_code=500, detail="Failed to load custom skill diagnostics.") from exc
|
||||||
|
|
||||||
|
|
||||||
@router.get("/skills/custom/{skill_name}/export-manifest", response_model=SkillExportManifestResponse, response_model_exclude_unset=True, summary="Preview Custom Skill Export")
|
@router.get("/skills/custom/{skill_name}/export-manifest", response_model=SkillExportManifestResponse, response_model_exclude_unset=True, summary="Preview Custom Skill Export")
|
||||||
async def preview_custom_skill_export(skill_name: str, request: Request, response: Response, config: AppConfig = Depends(get_config)) -> SkillExportManifestResponse | Response:
|
async def preview_custom_skill_export(skill_name: str, request: Request, response: Response, config: AppConfig = Depends(get_config)) -> SkillExportManifestResponse | Response:
|
||||||
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
|
await require_admin_user(request, detail=_ADMIN_REQUIRED_DETAIL)
|
||||||
|
|||||||
@ -1,5 +1,7 @@
|
|||||||
### Skills System (`packages/harness/deerflow/skills/`)
|
### Skills System (`packages/harness/deerflow/skills/`)
|
||||||
|
|
||||||
|
- **Management YAML diagnostics**: `parse_skill_file(..., diagnostics=...)` optionally collects `SkillLoadDiagnostic` values separately from valid `Skill` entries. Only `UserScopedSkillStorage.load_custom_skill_diagnostics()` supplies that sink, after validating root ancestors and each resolved file against the caller's custom root. Exclude legacy/shared locations and external symlinks; do not weaken runtime discovery's separate compatibility policy. `GET /api/skills/diagnostics/custom` is admin-only and returns stable codes plus package-relative identifiers and 1-based file positions. Never return `_format_yaml_error`, exceptions or source excerpts. The management UI posts the existing admin-only reload before refetching diagnostics and normal skills; active Agent snapshots are unchanged.
|
||||||
|
- **Diagnostic scope**: only `yaml.YAMLError` produces a management diagnostic. Missing frontmatter, non-mapping or invalid metadata, non-UTF-8 files and read errors (including permission denied) remain excluded; the UI and README state those limits. An empty diagnostic list is not a package-validity check. `tests/test_skill_load_diagnostics.py` pins these exclusions and permission-error redaction without changing runtime parsing.
|
||||||
- **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`.
|
- **History JSONL**: split records on LF, not Unicode text separators; preserve U+0085/U+2028/U+2029 in saved fields. Keep malformed JSON rejected. Tests: `tests/test_skill_history_jsonl.py`.
|
||||||
|
|
||||||
- **SkillScan API tokens**: `secret-cloud-token` accepts `-` and `_` in the `sk-` family, matching the character class used by PII redaction. Legacy, OpenAI project/service-account/admin, and Anthropic keys must trigger `CRITICAL` even in a Bearer header without a secret assignment. Skip placeholder matches until a real credential is found, retaining at most one cloud-token finding per file. Keep evidence fully redacted and preserve the minimum length and word boundaries; synthetic regressions live in `tests/test_skillscan_native.py`. Keep the mirrored `sk-` pattern in `app/scheduler/notification_text.py` aligned so scheduled-run summaries scrub the same key formats before IM delivery.
|
- **SkillScan API tokens**: `secret-cloud-token` accepts `-` and `_` in the `sk-` family, matching the character class used by PII redaction. Legacy, OpenAI project/service-account/admin, and Anthropic keys must trigger `CRITICAL` even in a Bearer header without a secret assignment. Skip placeholder matches until a real credential is found, retaining at most one cloud-token finding per file. Keep evidence fully redacted and preserve the minimum length and word boundaries; synthetic regressions live in `tests/test_skillscan_native.py`. Keep the mirrored `sk-` pattern in `app/scheduler/notification_text.py` aligned so scheduled-run summaries scrub the same key formats before IM delivery.
|
||||||
|
|||||||
35
backend/packages/harness/deerflow/skills/diagnostics.py
Normal file
@ -0,0 +1,35 @@
|
|||||||
|
"""Host-path-free diagnostics for skill management, never the agent catalog."""
|
||||||
|
|
||||||
|
from dataclasses import dataclass
|
||||||
|
from typing import Literal
|
||||||
|
|
||||||
|
import yaml
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class SkillLoadDiagnostic:
|
||||||
|
package: str
|
||||||
|
path: Literal["SKILL.md"] = "SKILL.md"
|
||||||
|
code: Literal["invalid_frontmatter"] = "invalid_frontmatter"
|
||||||
|
hint: Literal["quote_colon_value"] | None = None
|
||||||
|
line: int | None = None
|
||||||
|
column: int | None = None
|
||||||
|
|
||||||
|
|
||||||
|
def yaml_load_diagnostic(package: str, exc: yaml.YAMLError, source: str, line_offset: int) -> SkillLoadDiagnostic:
|
||||||
|
"""Project parser details into stable codes without serializing the exception."""
|
||||||
|
mark = getattr(exc, "problem_mark", None)
|
||||||
|
hint = None
|
||||||
|
lines = source.splitlines()
|
||||||
|
if mark is not None and 0 <= mark.line < len(lines) and getattr(exc, "problem", "") == "mapping values are not allowed here":
|
||||||
|
offending = lines[mark.line]
|
||||||
|
_, separator, value = offending.partition(":")
|
||||||
|
value = value.strip()
|
||||||
|
if separator and value and value[0] not in {'"', "'", "|", ">", "[", "{"} and mark.column > offending.index(":") and offending[mark.column : mark.column + 2] == ": ":
|
||||||
|
hint = "quote_colon_value"
|
||||||
|
return SkillLoadDiagnostic(
|
||||||
|
package=package,
|
||||||
|
hint=hint,
|
||||||
|
line=mark.line + line_offset + 1 if mark is not None else None,
|
||||||
|
column=mark.column + 1 if mark is not None else None,
|
||||||
|
)
|
||||||
@ -4,6 +4,7 @@ from pathlib import Path
|
|||||||
|
|
||||||
import yaml
|
import yaml
|
||||||
|
|
||||||
|
from .diagnostics import SkillLoadDiagnostic, yaml_load_diagnostic
|
||||||
from .frontmatter import _FRONTMATTER_RE
|
from .frontmatter import _FRONTMATTER_RE
|
||||||
from .types import SKILL_MD_FILE, SecretRequirement, Skill, SkillCategory
|
from .types import SKILL_MD_FILE, SecretRequirement, Skill, SkillCategory
|
||||||
|
|
||||||
@ -198,7 +199,7 @@ def parse_secrets_autonomous(raw: object, skill_file: Path) -> bool:
|
|||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: Path | None = None) -> Skill | None:
|
def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: Path | None = None, *, diagnostics: list[SkillLoadDiagnostic] | None = None) -> Skill | None:
|
||||||
"""Parse a SKILL.md file and extract metadata.
|
"""Parse a SKILL.md file and extract metadata.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
@ -206,6 +207,8 @@ def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: P
|
|||||||
category: Category of the skill.
|
category: Category of the skill.
|
||||||
relative_path: Relative path from the category root to the skill
|
relative_path: Relative path from the category root to the skill
|
||||||
directory. Defaults to the skill directory name when omitted.
|
directory. Defaults to the skill directory name when omitted.
|
||||||
|
diagnostics: Optional management-only sink for YAML failures. The caller
|
||||||
|
must authorize the file path before requesting diagnostics.
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Skill object if parsing succeeds, None otherwise.
|
Skill object if parsing succeeds, None otherwise.
|
||||||
@ -225,6 +228,8 @@ def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: P
|
|||||||
try:
|
try:
|
||||||
metadata = yaml.safe_load(front_matter_text)
|
metadata = yaml.safe_load(front_matter_text)
|
||||||
except yaml.YAMLError as exc:
|
except yaml.YAMLError as exc:
|
||||||
|
if diagnostics is not None:
|
||||||
|
diagnostics.append(yaml_load_diagnostic((relative_path or Path(skill_file.parent.name)).as_posix(), exc, front_matter_text, content.count("\n", 0, front_matter_match.start(1))))
|
||||||
logger.error("%s", _format_yaml_error(skill_file, exc, front_matter_text))
|
logger.error("%s", _format_yaml_error(skill_file, exc, front_matter_text))
|
||||||
return None
|
return None
|
||||||
if not isinstance(metadata, dict):
|
if not isinstance(metadata, dict):
|
||||||
|
|||||||
@ -11,6 +11,7 @@ from collections.abc import Iterable
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
|
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
|
||||||
|
from deerflow.skills.diagnostics import SkillLoadDiagnostic
|
||||||
from deerflow.skills.types import SKILL_MD_FILE, Skill, SkillCategory # noqa: F401
|
from deerflow.skills.types import SKILL_MD_FILE, Skill, SkillCategory # noqa: F401
|
||||||
from deerflow.utils.host_paths import windows_incompatible_segment
|
from deerflow.utils.host_paths import windows_incompatible_segment
|
||||||
|
|
||||||
@ -69,6 +70,10 @@ class SkillStorage(ABC):
|
|||||||
def __init__(self, container_path: str = DEFAULT_SKILLS_CONTAINER_PATH) -> None:
|
def __init__(self, container_path: str = DEFAULT_SKILLS_CONTAINER_PATH) -> None:
|
||||||
self._container_root = container_path
|
self._container_root = container_path
|
||||||
|
|
||||||
|
def load_custom_skill_diagnostics(self) -> list[SkillLoadDiagnostic]:
|
||||||
|
"""No diagnostics unless storage explicitly establishes caller ownership."""
|
||||||
|
return []
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
# Static protocol helpers (not storage-specific)
|
# Static protocol helpers (not storage-specific)
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
|
|||||||
@ -39,6 +39,8 @@ from collections.abc import Iterable
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
|
from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH
|
||||||
|
from deerflow.skills.diagnostics import SkillLoadDiagnostic
|
||||||
|
from deerflow.skills.parser import parse_skill_file
|
||||||
from deerflow.skills.permissions import make_skill_written_path_sandbox_readable
|
from deerflow.skills.permissions import make_skill_written_path_sandbox_readable
|
||||||
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
|
from deerflow.skills.storage.local_skill_storage import LocalSkillStorage
|
||||||
from deerflow.skills.storage.skill_storage import SKILL_MD_FILE, walk_skill_directories
|
from deerflow.skills.storage.skill_storage import SKILL_MD_FILE, walk_skill_directories
|
||||||
@ -270,6 +272,38 @@ class UserScopedSkillStorage(LocalSkillStorage):
|
|||||||
raise ValueError(f"'{name}' is a managed integration skill and cannot be edited. Create a custom skill with another name if you need a modified workflow.")
|
raise ValueError(f"'{name}' is a managed integration skill and cannot be edited. Create a custom skill with another name if you need a modified workflow.")
|
||||||
raise FileNotFoundError(f"Custom skill '{name}' not found.")
|
raise FileNotFoundError(f"Custom skill '{name}' not found.")
|
||||||
|
|
||||||
|
def load_custom_skill_diagnostics(self) -> list[SkillLoadDiagnostic]:
|
||||||
|
"""Scan only owned packages; legacy/shared and external links are excluded.
|
||||||
|
|
||||||
|
Unlike runtime discovery's external-package compatibility, management
|
||||||
|
diagnostics cannot establish ownership outside the caller's custom root.
|
||||||
|
Root ancestors must not redirect into another user's storage either.
|
||||||
|
"""
|
||||||
|
root = self._user_custom_root
|
||||||
|
expected = self._paths.base_dir.resolve() / "users" / self._user_id / "skills" / "custom"
|
||||||
|
if root.resolve() != expected or not root.is_dir():
|
||||||
|
return []
|
||||||
|
diagnostics: list[SkillLoadDiagnostic] = []
|
||||||
|
for current, dirs, files in walk_skill_directories(root):
|
||||||
|
package_dir = Path(current)
|
||||||
|
if not package_dir.resolve().is_relative_to(expected):
|
||||||
|
dirs.clear()
|
||||||
|
continue
|
||||||
|
dirs[:] = sorted(name for name in dirs if not name.startswith("."))
|
||||||
|
if SKILL_MD_FILE not in files:
|
||||||
|
continue
|
||||||
|
dirs.clear()
|
||||||
|
try:
|
||||||
|
skill_file = (package_dir / SKILL_MD_FILE).resolve(strict=True)
|
||||||
|
if not skill_file.is_relative_to(expected):
|
||||||
|
continue
|
||||||
|
# Preserve the logical SKILL.md name for owned internal links.
|
||||||
|
parse_skill_file(package_dir / SKILL_MD_FILE, SkillCategory.CUSTOM, package_dir.relative_to(root), diagnostics=diagnostics)
|
||||||
|
except (OSError, RuntimeError):
|
||||||
|
# A concurrently removed/retargeted package is not a YAML error.
|
||||||
|
continue
|
||||||
|
return diagnostics
|
||||||
|
|
||||||
def _iter_skill_files(self) -> Iterable[tuple[SkillCategory, Path, Path]]:
|
def _iter_skill_files(self) -> Iterable[tuple[SkillCategory, Path, Path]]:
|
||||||
# 1. Public skills: always from global root
|
# 1. Public skills: always from global root
|
||||||
public_path = self._host_root / SkillCategory.PUBLIC.value
|
public_path = self._host_root / SkillCategory.PUBLIC.value
|
||||||
|
|||||||
262
backend/tests/test_skill_load_diagnostics.py
Normal file
@ -0,0 +1,262 @@
|
|||||||
|
"""Management-only YAML diagnostics: scope, redaction and recovery."""
|
||||||
|
|
||||||
|
from dataclasses import asdict
|
||||||
|
from pathlib import Path
|
||||||
|
from types import SimpleNamespace
|
||||||
|
from uuid import UUID
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
from _router_auth_helpers import make_authed_test_app
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from support.symlinks import symlink_or_skip
|
||||||
|
|
||||||
|
from app.gateway.auth.models import User
|
||||||
|
from app.gateway.routers import skills as routes
|
||||||
|
from deerflow.config.paths import Paths
|
||||||
|
from deerflow.skills.storage import reset_skill_storage
|
||||||
|
from deerflow.skills.storage.user_scoped_skill_storage import UserScopedSkillStorage
|
||||||
|
|
||||||
|
BAD = "---\nname: broken\ndescription: private-value: detail\n---\n"
|
||||||
|
GOOD = '---\nname: broken\ndescription: "private-value: detail"\n---\n'
|
||||||
|
USER_ID = "00000000-0000-0000-0000-000000000001"
|
||||||
|
|
||||||
|
|
||||||
|
def write(root: Path, package: str, content: str = BAD) -> Path:
|
||||||
|
path = root / package / "SKILL.md"
|
||||||
|
path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
path.write_text(content, encoding="utf-8", newline="")
|
||||||
|
return path
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def scope(tmp_path, monkeypatch):
|
||||||
|
paths = Paths(tmp_path)
|
||||||
|
monkeypatch.setattr("deerflow.config.paths.get_paths", lambda: paths)
|
||||||
|
config = SimpleNamespace(skills=SimpleNamespace(get_skills_path=lambda: tmp_path / "global", container_path="/mnt/skills"))
|
||||||
|
storage = UserScopedSkillStorage(USER_ID, app_config=config)
|
||||||
|
reset_skill_storage()
|
||||||
|
yield paths, config, storage
|
||||||
|
reset_skill_storage()
|
||||||
|
|
||||||
|
|
||||||
|
def test_diagnostics_are_separate_and_disappear_after_correction(scope):
|
||||||
|
paths, _, storage = scope
|
||||||
|
root = paths.user_custom_skills_dir(USER_ID)
|
||||||
|
broken = write(root, "namespace/broken")
|
||||||
|
write(root, "valid", GOOD.replace("broken", "valid"))
|
||||||
|
write(root, ".hidden")
|
||||||
|
write(root, "valid/nested")
|
||||||
|
assert [s.name for s in storage.load_skills()] == ["valid"]
|
||||||
|
result = [asdict(d) for d in storage.load_custom_skill_diagnostics()]
|
||||||
|
assert result == [{"package": "namespace/broken", "path": "SKILL.md", "code": "invalid_frontmatter", "hint": "quote_colon_value", "line": 3, "column": 27}]
|
||||||
|
assert str(paths.base_dir) not in str(result)
|
||||||
|
assert "private-value" not in str(result)
|
||||||
|
broken.write_text(GOOD, encoding="utf-8")
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
assert {s.name for s in storage.load_skills()} == {"valid", "broken"}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("prefix", ["---\n", "\ufeff---\r\n\r\n"])
|
||||||
|
def test_locations_match_actual_file_and_other_yaml_errors_have_no_colon_hint(scope, prefix):
|
||||||
|
paths, _, storage = scope
|
||||||
|
write(paths.user_custom_skills_dir(USER_ID), "broken", prefix + "name: broken\ndescription: [one, two\n---\n")
|
||||||
|
result = storage.load_custom_skill_diagnostics()
|
||||||
|
assert len(result) == 1
|
||||||
|
assert result[0].code == "invalid_frontmatter"
|
||||||
|
assert result[0].hint is None
|
||||||
|
assert result[0].line == prefix.count("\n") + 2
|
||||||
|
assert result[0].column == 23
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"content",
|
||||||
|
[
|
||||||
|
"# No frontmatter\n",
|
||||||
|
"---\nname: broken\ndescription: normal\n", # Unclosed frontmatter.
|
||||||
|
"---\n- not\n- a mapping\n---\n",
|
||||||
|
"---\nscalar\n---\n",
|
||||||
|
"---\nnull\n---\n",
|
||||||
|
"---\nname: broken\n---\n",
|
||||||
|
"---\ndescription: normal\n---\n",
|
||||||
|
'---\nname: " "\ndescription: normal\n---\n',
|
||||||
|
'---\nname: broken\ndescription: " "\n---\n',
|
||||||
|
"---\nname: 42\ndescription: normal\n---\n",
|
||||||
|
"---\nname: broken\ndescription: []\n---\n",
|
||||||
|
GOOD.replace("\n---\n", "\nallowed-tools: 42\n---\n"),
|
||||||
|
GOOD.replace("\n---\n", "\nallowed-tools: [42]\n---\n"),
|
||||||
|
GOOD.replace("\n---\n", "\nrequired-secrets: private-value\n---\n"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_non_yaml_load_failures_remain_outside_diagnostics(scope, content):
|
||||||
|
"""The UI's YAML-only scope must not imply all omitted skills are reported."""
|
||||||
|
paths, config, storage = scope
|
||||||
|
root = paths.user_custom_skills_dir(USER_ID)
|
||||||
|
write(root, "broken", content)
|
||||||
|
write(root, "valid", GOOD.replace("broken", "valid"))
|
||||||
|
assert [s.name for s in storage.load_skills()] == ["valid"]
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
with client_for(config) as client:
|
||||||
|
response = client.get("/api/skills/diagnostics/custom")
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json() == {"diagnostics": []}
|
||||||
|
|
||||||
|
|
||||||
|
def test_non_utf8_file_is_not_a_yaml_diagnostic(scope):
|
||||||
|
paths, config, storage = scope
|
||||||
|
path = write(paths.user_custom_skills_dir(USER_ID), "broken")
|
||||||
|
path.write_bytes(GOOD.encode("utf-8") + b"\xff")
|
||||||
|
assert storage.load_skills() == []
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
with client_for(config) as client:
|
||||||
|
response = client.get("/api/skills/diagnostics/custom")
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json() == {"diagnostics": []}
|
||||||
|
|
||||||
|
|
||||||
|
def test_permission_denied_file_is_not_a_yaml_diagnostic(scope, monkeypatch):
|
||||||
|
paths, config, storage = scope
|
||||||
|
root = paths.user_custom_skills_dir(USER_ID)
|
||||||
|
unreadable = write(root, "unreadable", GOOD.replace("broken", "unreadable"))
|
||||||
|
write(root, "valid", GOOD.replace("broken", "valid"))
|
||||||
|
original = Path.read_text
|
||||||
|
denied_reads = []
|
||||||
|
|
||||||
|
def deny_owned_file(path, *args, **kwargs):
|
||||||
|
if path == unreadable:
|
||||||
|
denied_reads.append(path)
|
||||||
|
raise PermissionError(f"private-value: cannot read {path}")
|
||||||
|
return original(path, *args, **kwargs)
|
||||||
|
|
||||||
|
# Portable even when tests run as root or on hosts without POSIX chmod.
|
||||||
|
with monkeypatch.context() as patch:
|
||||||
|
patch.setattr(Path, "read_text", deny_owned_file)
|
||||||
|
assert [s.name for s in storage.load_skills()] == ["valid"]
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
with client_for(config) as client:
|
||||||
|
response = client.get("/api/skills/diagnostics/custom")
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.json() == {"diagnostics": []}
|
||||||
|
for private in ["private-value", "unreadable", str(paths.base_dir)]:
|
||||||
|
assert private not in response.text
|
||||||
|
assert len(denied_reads) == 3
|
||||||
|
assert {s.name for s in storage.load_skills()} == {"valid", "unreadable"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_other_owners_and_global_sources_are_excluded(scope):
|
||||||
|
paths, config, storage = scope
|
||||||
|
write(paths.user_custom_skills_dir("another-user"), "other-private")
|
||||||
|
for category in ["custom", "public"]:
|
||||||
|
write(config.skills.get_skills_path() / category, "global-private")
|
||||||
|
write(paths.integration_skills_dir(), "provider/integration-private")
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_comment_colon_does_not_turn_indentation_error_into_quoting_hint(scope):
|
||||||
|
paths, _, storage = scope
|
||||||
|
write(paths.user_custom_skills_dir(USER_ID), "broken", "---\nname: broken\ndescription: normal\n bad: value # comment: words\n---\n")
|
||||||
|
result = storage.load_custom_skill_diagnostics()
|
||||||
|
assert len(result) == 1
|
||||||
|
assert result[0].hint is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_owned_file_symlink_preserves_skill_filename(scope):
|
||||||
|
paths, _, storage = scope
|
||||||
|
root = paths.user_custom_skills_dir(USER_ID)
|
||||||
|
source = root / "linked" / "source.md"
|
||||||
|
source.parent.mkdir(parents=True)
|
||||||
|
source.write_text(BAD, encoding="utf-8")
|
||||||
|
symlink_or_skip(source.parent / "SKILL.md", source)
|
||||||
|
result = storage.load_custom_skill_diagnostics()
|
||||||
|
assert len(result) == 1
|
||||||
|
assert result[0].package == "linked"
|
||||||
|
assert result[0].path == "SKILL.md"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("kind", ["package", "file", "root"])
|
||||||
|
def test_symlink_escape_is_not_read(scope, monkeypatch, kind):
|
||||||
|
paths, _, storage = scope
|
||||||
|
root = paths.user_custom_skills_dir(USER_ID)
|
||||||
|
target = write(paths.user_custom_skills_dir("another-user"), "private")
|
||||||
|
if kind == "root":
|
||||||
|
root.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
symlink_or_skip(root, target.parent.parent, target_is_directory=True)
|
||||||
|
elif kind == "package":
|
||||||
|
root.mkdir(parents=True)
|
||||||
|
symlink_or_skip(root / "escape", target.parent, target_is_directory=True)
|
||||||
|
else:
|
||||||
|
(root / "escape").mkdir(parents=True)
|
||||||
|
symlink_or_skip(root / "escape" / "SKILL.md", target)
|
||||||
|
original = Path.read_text
|
||||||
|
|
||||||
|
def guarded_read(path, *args, **kwargs):
|
||||||
|
assert path.resolve() != target.resolve(), "must authorize before reading another owner's file"
|
||||||
|
return original(path, *args, **kwargs)
|
||||||
|
|
||||||
|
monkeypatch.setattr(Path, "read_text", guarded_read)
|
||||||
|
assert storage.load_custom_skill_diagnostics() == []
|
||||||
|
|
||||||
|
|
||||||
|
def client_for(config, role="admin"):
|
||||||
|
user = User(id=UUID(USER_ID), email="diagnostics@example.com", password_hash="x", system_role=role)
|
||||||
|
app = make_authed_test_app(user_factory=lambda: user, bind_current_user=True)
|
||||||
|
app.dependency_overrides[routes.get_config] = lambda: config
|
||||||
|
app.include_router(routes.router)
|
||||||
|
return TestClient(app)
|
||||||
|
|
||||||
|
|
||||||
|
def test_management_route_uses_caller_scope(scope):
|
||||||
|
paths, config, _ = scope
|
||||||
|
write(paths.user_custom_skills_dir(USER_ID), "mine")
|
||||||
|
write(paths.user_custom_skills_dir("another-user"), "other-private")
|
||||||
|
with client_for(config) as client:
|
||||||
|
response = client.get("/api/skills/diagnostics/custom")
|
||||||
|
assert response.status_code == 200
|
||||||
|
assert response.headers["cache-control"] == "no-store"
|
||||||
|
assert [d["package"] for d in response.json()["diagnostics"]] == ["mine"]
|
||||||
|
for private in ["private-value", "other-private", str(paths.base_dir)]:
|
||||||
|
assert private not in response.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_non_admin_cannot_scan_diagnostics(scope, monkeypatch):
|
||||||
|
_, config, _ = scope
|
||||||
|
monkeypatch.setattr(routes, "_get_user_skill_storage", lambda _: pytest.fail("unauthorized scan"))
|
||||||
|
with client_for(config, "user") as client:
|
||||||
|
assert client.get("/api/skills/diagnostics/custom").status_code == 403
|
||||||
|
|
||||||
|
|
||||||
|
def test_diagnostic_failure_does_not_expose_exception(scope, monkeypatch):
|
||||||
|
_, config, storage = scope
|
||||||
|
|
||||||
|
def fail():
|
||||||
|
raise OSError("private /srv/customer/secret SKILL.md source")
|
||||||
|
|
||||||
|
monkeypatch.setattr(storage, "load_custom_skill_diagnostics", fail)
|
||||||
|
monkeypatch.setattr(routes, "_get_user_skill_storage", lambda _: storage)
|
||||||
|
with client_for(config) as client:
|
||||||
|
response = client.get("/api/skills/diagnostics/custom")
|
||||||
|
assert response.status_code == 500
|
||||||
|
assert response.json() == {"detail": "Failed to load custom skill diagnostics."}
|
||||||
|
|
||||||
|
|
||||||
|
def test_correction_and_reload_restore_runtime_catalog(scope, monkeypatch):
|
||||||
|
from deerflow.agents.lead_agent import prompt
|
||||||
|
|
||||||
|
paths, config, storage = scope
|
||||||
|
broken = write(paths.user_custom_skills_dir(USER_ID), "broken")
|
||||||
|
monkeypatch.setattr(prompt, "get_or_new_user_skill_storage", lambda *_args, **_kwargs: storage)
|
||||||
|
monkeypatch.setattr(prompt, "get_or_new_skill_storage", lambda **_kwargs: storage)
|
||||||
|
monkeypatch.setattr(prompt, "resolve_shared_config_path", lambda: None)
|
||||||
|
prompt.clear_skills_system_prompt_cache()
|
||||||
|
try:
|
||||||
|
assert prompt.get_enabled_skills_for_config(config, user_id=USER_ID) == []
|
||||||
|
with client_for(config) as client:
|
||||||
|
assert len(client.get("/api/skills/diagnostics/custom").json()["diagnostics"]) == 1
|
||||||
|
broken.write_text(GOOD, encoding="utf-8")
|
||||||
|
# A management read alone does not update the runtime's cached list.
|
||||||
|
assert client.get("/api/skills/diagnostics/custom").json()["diagnostics"] == []
|
||||||
|
assert prompt.get_enabled_skills_for_config(config, user_id=USER_ID) == []
|
||||||
|
assert client.post("/api/skills/reload").status_code == 200
|
||||||
|
assert client.get("/api/skills/diagnostics/custom").json()["diagnostics"] == []
|
||||||
|
assert [s.name for s in prompt.get_enabled_skills_for_config(config, user_id=USER_ID)] == ["broken"]
|
||||||
|
finally:
|
||||||
|
prompt.clear_skills_system_prompt_cache()
|
||||||
|
Before Width: | Height: | Size: 88 KiB |
|
Before Width: | Height: | Size: 50 KiB |
|
Before Width: | Height: | Size: 256 KiB |
|
Before Width: | Height: | Size: 71 KiB |
|
Before Width: | Height: | Size: 45 KiB |
|
Before Width: | Height: | Size: 92 KiB |
|
Before Width: | Height: | Size: 224 KiB |
|
Before Width: | Height: | Size: 267 KiB |
@ -231,6 +231,15 @@ delete helper accepts remote 404 (not 403) before retrying local cleanup, and
|
|||||||
|
|
||||||
## Capability Center
|
## Capability Center
|
||||||
|
|
||||||
|
`skill-diagnostics.tsx` mounts only for administrators outside static demos. Its
|
||||||
|
user-keyed diagnostics query is separate from `useSkills()` and every picker.
|
||||||
|
Reload posts `/api/skills/reload` before invalidating both normal skills and
|
||||||
|
diagnostics; failures retain the warning and show localized retry feedback.
|
||||||
|
Never interpret raw parser exception wording in the UI.
|
||||||
|
Keep the scope note visible even when diagnostics are empty. It must describe
|
||||||
|
the YAML-syntax-only check and its exclusions (missing frontmatter, metadata
|
||||||
|
validation, unreadable and non-UTF-8 files); it is not a package-validity check.
|
||||||
|
|
||||||
`/workspace/capabilities` owns Plugins and Skills navigation. Plugins composes the
|
`/workspace/capabilities` owns Plugins and Skills navigation. Plugins composes the
|
||||||
MCP manager and a lazily loaded Lark configuration dialog; installation, OAuth,
|
MCP manager and a lazily loaded Lark configuration dialog; installation, OAuth,
|
||||||
mutation permissions, and cache ownership remain in the existing hooks. Skill display names/summaries are presentation
|
mutation permissions, and cache ownership remain in the existing hooks. Skill display names/summaries are presentation
|
||||||
|
|||||||
@ -0,0 +1,84 @@
|
|||||||
|
"use client";
|
||||||
|
|
||||||
|
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||||
|
import { AlertTriangleIcon, RefreshCwIcon } from "lucide-react";
|
||||||
|
|
||||||
|
import { Button } from "@/components/ui/button";
|
||||||
|
import { useI18n } from "@/core/i18n/hooks";
|
||||||
|
import { loadSkillDiagnostics, reloadSkills } from "@/core/skills/api";
|
||||||
|
|
||||||
|
export function SkillDiagnostics({ userId }: { userId: string }) {
|
||||||
|
const { t } = useI18n();
|
||||||
|
const text = t.settings.skills;
|
||||||
|
const client = useQueryClient();
|
||||||
|
const queryKey = ["skill-diagnostics", userId];
|
||||||
|
const diagnostics = useQuery({
|
||||||
|
queryKey,
|
||||||
|
queryFn: ({ signal }) => loadSkillDiagnostics(signal),
|
||||||
|
retry: false,
|
||||||
|
});
|
||||||
|
const reload = useMutation({
|
||||||
|
mutationFn: reloadSkills,
|
||||||
|
onSuccess: async () => {
|
||||||
|
await Promise.all([
|
||||||
|
client.invalidateQueries({ queryKey: ["skills"] }),
|
||||||
|
client.invalidateQueries({ queryKey }),
|
||||||
|
]);
|
||||||
|
},
|
||||||
|
});
|
||||||
|
const failures = diagnostics.data ?? [];
|
||||||
|
return (
|
||||||
|
<div className="mb-4 space-y-3">
|
||||||
|
<div className="flex justify-end">
|
||||||
|
<Button
|
||||||
|
size="sm"
|
||||||
|
variant="outline"
|
||||||
|
disabled={reload.isPending || diagnostics.isFetching}
|
||||||
|
onClick={() => reload.mutate()}
|
||||||
|
>
|
||||||
|
<RefreshCwIcon
|
||||||
|
className={reload.isPending ? "size-4 animate-spin" : "size-4"}
|
||||||
|
/>
|
||||||
|
{reload.isPending
|
||||||
|
? text.diagnosticsRefreshing
|
||||||
|
: text.diagnosticsRefresh}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
<p className="text-muted-foreground text-sm">{text.diagnosticsScope}</p>
|
||||||
|
{(diagnostics.isError || reload.isError) && (
|
||||||
|
<p role="alert" className="text-destructive text-sm">
|
||||||
|
{reload.isError
|
||||||
|
? text.diagnosticsReloadFailed
|
||||||
|
: text.diagnosticsFailed}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
{failures.length > 0 && (
|
||||||
|
<section
|
||||||
|
aria-label={text.diagnosticsTitle}
|
||||||
|
className="rounded-lg border border-amber-500/40 bg-amber-500/5 p-4"
|
||||||
|
>
|
||||||
|
<h3 className="flex items-center gap-2 text-sm font-semibold">
|
||||||
|
<AlertTriangleIcon className="size-4 text-amber-600" />
|
||||||
|
{text.diagnosticsTitle}
|
||||||
|
</h3>
|
||||||
|
<ul className="mt-3 space-y-3">
|
||||||
|
{failures.map((failure) => (
|
||||||
|
<li
|
||||||
|
key={`${failure.package}/${failure.path}`}
|
||||||
|
className="text-sm"
|
||||||
|
>
|
||||||
|
<code className="break-all">{`${failure.package}/${failure.path}${failure.line == null ? "" : `:${failure.line}${failure.column == null ? "" : `:${failure.column}`}`}`}</code>
|
||||||
|
<p className="mt-1">{text.diagnosticsInvalid}</p>
|
||||||
|
{failure.hint === "quote_colon_value" && (
|
||||||
|
<p className="text-muted-foreground">
|
||||||
|
{text.diagnosticsQuote}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
</li>
|
||||||
|
))}
|
||||||
|
</ul>
|
||||||
|
</section>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
@ -45,6 +45,7 @@ import type { Skill } from "@/core/skills/type";
|
|||||||
import { env } from "@/env";
|
import { env } from "@/env";
|
||||||
|
|
||||||
import { CapabilityCard, CapabilityIcon } from "./capability-card";
|
import { CapabilityCard, CapabilityIcon } from "./capability-card";
|
||||||
|
import { SkillDiagnostics } from "./skill-diagnostics";
|
||||||
import { presentSkill } from "./skill-presentation";
|
import { presentSkill } from "./skill-presentation";
|
||||||
|
|
||||||
const SkillExportDialog = dynamic(() => import("./skill-export-dialog"), {
|
const SkillExportDialog = dynamic(() => import("./skill-export-dialog"), {
|
||||||
@ -53,11 +54,16 @@ const SkillExportDialog = dynamic(() => import("./skill-export-dialog"), {
|
|||||||
|
|
||||||
export function SkillGallery({ query = "" }: { query?: string } = {}) {
|
export function SkillGallery({ query = "" }: { query?: string } = {}) {
|
||||||
const { t } = useI18n();
|
const { t } = useI18n();
|
||||||
|
const { user } = useAuth();
|
||||||
const { skills, isLoading, error } = useSkills();
|
const { skills, isLoading, error } = useSkills();
|
||||||
const adminRequired =
|
const adminRequired =
|
||||||
error instanceof SkillRequestError && error.isAdminRequired;
|
error instanceof SkillRequestError && error.isAdminRequired;
|
||||||
return (
|
return (
|
||||||
<div>
|
<div>
|
||||||
|
{user?.system_role === "admin" &&
|
||||||
|
env.NEXT_PUBLIC_STATIC_WEBSITE_ONLY !== "true" && (
|
||||||
|
<SkillDiagnostics key={user.id} userId={user.id} />
|
||||||
|
)}
|
||||||
{isLoading ? (
|
{isLoading ? (
|
||||||
<div className="text-muted-foreground text-sm">{t.common.loading}</div>
|
<div className="text-muted-foreground text-sm">{t.common.loading}</div>
|
||||||
) : adminRequired ? (
|
) : adminRequired ? (
|
||||||
|
|||||||
@ -2212,6 +2212,16 @@ export const enUS: Translations = {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
skills: {
|
skills: {
|
||||||
|
diagnosticsTitle: "Could not load",
|
||||||
|
diagnosticsScope:
|
||||||
|
"Only YAML syntax errors in your own custom skills are shown. After correcting a SKILL.md file, reload skills. Missing frontmatter, metadata validation errors, and unreadable or non-UTF-8 files are not reported. Shared and externally linked packages are excluded.",
|
||||||
|
diagnosticsInvalid: "Invalid YAML frontmatter.",
|
||||||
|
diagnosticsQuote: "Quote values containing a colon followed by a space.",
|
||||||
|
diagnosticsRefresh: "Reload skills",
|
||||||
|
diagnosticsRefreshing: "Reloading skills…",
|
||||||
|
diagnosticsFailed:
|
||||||
|
"Could not check custom skills for YAML errors. Reload to try again.",
|
||||||
|
diagnosticsReloadFailed: "Could not reload skills. Try again.",
|
||||||
exportPrevious: "Previous 50 files",
|
exportPrevious: "Previous 50 files",
|
||||||
exportNotices: {
|
exportNotices: {
|
||||||
skill_export_yaml_alias:
|
skill_export_yaml_alias:
|
||||||
|
|||||||
@ -1806,6 +1806,14 @@ export interface Translations {
|
|||||||
};
|
};
|
||||||
};
|
};
|
||||||
skills: {
|
skills: {
|
||||||
|
diagnosticsTitle: string;
|
||||||
|
diagnosticsScope: string;
|
||||||
|
diagnosticsInvalid: string;
|
||||||
|
diagnosticsQuote: string;
|
||||||
|
diagnosticsRefresh: string;
|
||||||
|
diagnosticsRefreshing: string;
|
||||||
|
diagnosticsFailed: string;
|
||||||
|
diagnosticsReloadFailed: string;
|
||||||
exportPrevious: string;
|
exportPrevious: string;
|
||||||
exportNotices: Record<string, string>;
|
exportNotices: Record<string, string>;
|
||||||
exportSkill: string;
|
exportSkill: string;
|
||||||
|
|||||||
@ -2082,6 +2082,15 @@ export const zhCN: Translations = {
|
|||||||
},
|
},
|
||||||
},
|
},
|
||||||
skills: {
|
skills: {
|
||||||
|
diagnosticsTitle: "无法加载",
|
||||||
|
diagnosticsScope:
|
||||||
|
"仅报告你自己的自定义技能的 YAML 语法错误。修正 SKILL.md 文件后请重新加载。不报告缺少头部、元数据校验错误、无法读取或非 UTF-8 编码的文件,不包含共享目录和外部链接包。",
|
||||||
|
diagnosticsInvalid: "YAML 头部格式错误。",
|
||||||
|
diagnosticsQuote: "包含“冒号加空格”的值需要用引号包裹。",
|
||||||
|
diagnosticsRefresh: "重新加载技能",
|
||||||
|
diagnosticsRefreshing: "正在重新加载…",
|
||||||
|
diagnosticsFailed: "无法检查自定义技能的 YAML 错误,请重新加载后重试。",
|
||||||
|
diagnosticsReloadFailed: "重新加载技能失败,请重试。",
|
||||||
exportPrevious: "上 50 项",
|
exportPrevious: "上 50 项",
|
||||||
exportNotices: {
|
exportNotices: {
|
||||||
skill_export_yaml_alias:
|
skill_export_yaml_alias:
|
||||||
|
|||||||
@ -1,7 +1,7 @@
|
|||||||
import { fetch } from "@/core/api/fetcher";
|
import { fetch } from "@/core/api/fetcher";
|
||||||
import { getBackendBaseURL } from "@/core/config";
|
import { getBackendBaseURL } from "@/core/config";
|
||||||
|
|
||||||
import type { Skill } from "./type";
|
import type { Skill, SkillLoadDiagnostic } from "./type";
|
||||||
|
|
||||||
// Keep this in lockstep with `_MAX_SKILL_ARCHIVE_UPLOAD_BYTES` in
|
// Keep this in lockstep with `_MAX_SKILL_ARCHIVE_UPLOAD_BYTES` in
|
||||||
// `backend/app/gateway/routers/skills.py`; nginx and Ingress allow 101 MiB so
|
// `backend/app/gateway/routers/skills.py`; nginx and Ingress allow 101 MiB so
|
||||||
@ -121,6 +121,33 @@ async function readErrorDetail(response: Response): Promise<SkillErrorDetail> {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export async function loadSkillDiagnostics(
|
||||||
|
signal?: AbortSignal,
|
||||||
|
): Promise<SkillLoadDiagnostic[]> {
|
||||||
|
const response = await fetch(
|
||||||
|
`${getBackendBaseURL()}/api/skills/diagnostics/custom`,
|
||||||
|
{ signal, cache: "no-store" },
|
||||||
|
);
|
||||||
|
if (!response.ok) {
|
||||||
|
const detail = await readErrorDetail(response);
|
||||||
|
throw new SkillRequestError(response.status, detail.message, detail);
|
||||||
|
}
|
||||||
|
const data = (await response.json()) as {
|
||||||
|
diagnostics: SkillLoadDiagnostic[];
|
||||||
|
};
|
||||||
|
return data.diagnostics;
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function reloadSkills(): Promise<void> {
|
||||||
|
const response = await fetch(`${getBackendBaseURL()}/api/skills/reload`, {
|
||||||
|
method: "POST",
|
||||||
|
});
|
||||||
|
if (!response.ok) {
|
||||||
|
const detail = await readErrorDetail(response);
|
||||||
|
throw new SkillRequestError(response.status, detail.message, detail);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export async function loadSkills() {
|
export async function loadSkills() {
|
||||||
const skills = await fetch(`${getBackendBaseURL()}/api/skills`);
|
const skills = await fetch(`${getBackendBaseURL()}/api/skills`);
|
||||||
if (!skills.ok) {
|
if (!skills.ok) {
|
||||||
|
|||||||
@ -6,3 +6,12 @@ export interface Skill {
|
|||||||
enabled: boolean;
|
enabled: boolean;
|
||||||
editable: boolean;
|
editable: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export interface SkillLoadDiagnostic {
|
||||||
|
package: string;
|
||||||
|
path: "SKILL.md";
|
||||||
|
code: "invalid_frontmatter";
|
||||||
|
hint?: "quote_colon_value" | null;
|
||||||
|
line?: number | null;
|
||||||
|
column?: number | null;
|
||||||
|
}
|
||||||
|
|||||||
132
frontend/tests/e2e/skill-diagnostics.spec.ts
Normal file
@ -0,0 +1,132 @@
|
|||||||
|
import { expect, test } from "@playwright/test";
|
||||||
|
|
||||||
|
import { mockLangGraphAPI } from "./utils/mock-api";
|
||||||
|
|
||||||
|
test("YAML diagnostics stay separate and reload restores a corrected skill", async ({
|
||||||
|
page,
|
||||||
|
}, testInfo) => {
|
||||||
|
mockLangGraphAPI(page, { skills: [] });
|
||||||
|
let corrected = false;
|
||||||
|
let reloads = 0;
|
||||||
|
await page.route("**/api/skills/diagnostics/custom", (route) =>
|
||||||
|
route.fulfill({
|
||||||
|
json: {
|
||||||
|
diagnostics: corrected
|
||||||
|
? []
|
||||||
|
: [
|
||||||
|
{
|
||||||
|
package: "collect-startrun",
|
||||||
|
path: "SKILL.md",
|
||||||
|
code: "invalid_frontmatter",
|
||||||
|
hint: "quote_colon_value",
|
||||||
|
line: 3,
|
||||||
|
column: 27,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await page.route("**/api/skills", (route) =>
|
||||||
|
route.fulfill({
|
||||||
|
json: {
|
||||||
|
skills: corrected
|
||||||
|
? [
|
||||||
|
{
|
||||||
|
name: "collect-startrun",
|
||||||
|
description: "Collect project updates",
|
||||||
|
category: "custom",
|
||||||
|
enabled: true,
|
||||||
|
editable: true,
|
||||||
|
},
|
||||||
|
]
|
||||||
|
: [],
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await page.route("**/api/skills/reload", (route) => {
|
||||||
|
reloads++;
|
||||||
|
expect(route.request().method()).toBe("POST");
|
||||||
|
if (reloads === 1)
|
||||||
|
return route.fulfill({ status: 500, json: { detail: "Reload failed" } });
|
||||||
|
corrected = true;
|
||||||
|
return route.fulfill({ json: { success: true, scope: "shared_config" } });
|
||||||
|
});
|
||||||
|
await page.goto("/workspace/capabilities?tab=skills");
|
||||||
|
await expect(
|
||||||
|
page.getByRole("heading", { name: "Could not load" }),
|
||||||
|
).toBeVisible();
|
||||||
|
await expect(page.getByText("collect-startrun/SKILL.md:3:27")).toBeVisible();
|
||||||
|
await expect(
|
||||||
|
page.getByText("Quote values containing a colon followed by a space."),
|
||||||
|
).toBeVisible();
|
||||||
|
await expect(
|
||||||
|
page.getByRole("button", { name: "View details collect-startrun" }),
|
||||||
|
).toHaveCount(0);
|
||||||
|
await page.screenshot({
|
||||||
|
path: testInfo.outputPath("skill-yaml-warning.png"),
|
||||||
|
fullPage: true,
|
||||||
|
});
|
||||||
|
await page
|
||||||
|
.getByRole("button", { name: "Reload skills", exact: true })
|
||||||
|
.click();
|
||||||
|
await expect(
|
||||||
|
page
|
||||||
|
.getByRole("alert")
|
||||||
|
.filter({ hasText: "Could not reload skills. Try again." }),
|
||||||
|
).toBeVisible();
|
||||||
|
await expect(page.getByText("collect-startrun/SKILL.md:3:27")).toBeVisible();
|
||||||
|
await page
|
||||||
|
.getByRole("button", { name: "Reload skills", exact: true })
|
||||||
|
.click();
|
||||||
|
await expect(
|
||||||
|
page.getByRole("heading", { name: "Could not load" }),
|
||||||
|
).toHaveCount(0);
|
||||||
|
await page.getByRole("tab", { name: "My skills", exact: true }).click();
|
||||||
|
await expect(
|
||||||
|
page.getByRole("button", {
|
||||||
|
name: "View details collect-startrun",
|
||||||
|
exact: true,
|
||||||
|
}),
|
||||||
|
).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
test("ordinary users never request management diagnostics", async ({
|
||||||
|
page,
|
||||||
|
}) => {
|
||||||
|
mockLangGraphAPI(page, { skills: [] });
|
||||||
|
let requests = 0;
|
||||||
|
await page.route("**/api/v1/auth/me", (route) =>
|
||||||
|
route.fulfill({
|
||||||
|
json: {
|
||||||
|
id: "ordinary",
|
||||||
|
email: "ordinary@example.com",
|
||||||
|
system_role: "user",
|
||||||
|
needs_setup: false,
|
||||||
|
permissions: [],
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
await page.route("**/api/skills/diagnostics/custom", (route) => {
|
||||||
|
requests++;
|
||||||
|
return route.fulfill({ json: { diagnostics: [] } });
|
||||||
|
});
|
||||||
|
// The local preview starts with an SSR admin. Refresh the real auth
|
||||||
|
// provider before opening the skill gallery as an ordinary user.
|
||||||
|
await page.goto("/workspace/capabilities");
|
||||||
|
await expect(
|
||||||
|
page.getByRole("textbox", { name: "Search plugins by name or purpose" }),
|
||||||
|
).toBeEnabled();
|
||||||
|
const refreshed = page.waitForResponse("**/api/v1/auth/me");
|
||||||
|
await page.evaluate(() =>
|
||||||
|
document.dispatchEvent(new Event("visibilitychange")),
|
||||||
|
);
|
||||||
|
await refreshed;
|
||||||
|
await page.getByRole("tab", { name: "Skills", exact: true }).click();
|
||||||
|
await expect(
|
||||||
|
page.getByRole("tab", { name: "My skills", exact: true }),
|
||||||
|
).toBeVisible();
|
||||||
|
await expect(
|
||||||
|
page.getByRole("button", { name: "Reload skills", exact: true }),
|
||||||
|
).toHaveCount(0);
|
||||||
|
expect(requests).toBe(0);
|
||||||
|
});
|
||||||
@ -0,0 +1,168 @@
|
|||||||
|
import { afterEach, expect, it, rs } from "@rstest/core";
|
||||||
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||||
|
import {
|
||||||
|
act,
|
||||||
|
cleanup,
|
||||||
|
fireEvent,
|
||||||
|
render,
|
||||||
|
screen,
|
||||||
|
waitFor,
|
||||||
|
} from "@testing-library/react";
|
||||||
|
|
||||||
|
const api = rs.hoisted(() => ({ load: rs.fn(), reload: rs.fn() }));
|
||||||
|
const i18n = rs.hoisted(() => ({ locale: "en-US" }));
|
||||||
|
rs.mock("@/core/skills/api", () => ({
|
||||||
|
loadSkillDiagnostics: api.load,
|
||||||
|
reloadSkills: api.reload,
|
||||||
|
}));
|
||||||
|
rs.mock("@/core/i18n/hooks", () => ({
|
||||||
|
useI18n: () => ({
|
||||||
|
t: {
|
||||||
|
common: { loading: "Loading" },
|
||||||
|
settings: {
|
||||||
|
skills: {
|
||||||
|
diagnosticsTitle: "Could not load",
|
||||||
|
diagnosticsScope:
|
||||||
|
i18n.locale === "zh-CN"
|
||||||
|
? zhCN.settings.skills.diagnosticsScope
|
||||||
|
: enUS.settings.skills.diagnosticsScope,
|
||||||
|
diagnosticsInvalid: "Invalid YAML",
|
||||||
|
diagnosticsQuote: "Quote colon values",
|
||||||
|
diagnosticsRefresh: "Reload skills",
|
||||||
|
diagnosticsRefreshing: "Reloading skills",
|
||||||
|
diagnosticsFailed: "Could not load diagnostics",
|
||||||
|
diagnosticsReloadFailed: "Could not reload skills",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { SkillDiagnostics } from "@/components/workspace/capabilities/skill-diagnostics";
|
||||||
|
import { enUS } from "@/core/i18n/locales/en-US";
|
||||||
|
import { zhCN } from "@/core/i18n/locales/zh-CN";
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
rs.clearAllMocks();
|
||||||
|
i18n.locale = "en-US";
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
[
|
||||||
|
"en-US",
|
||||||
|
/Only YAML syntax errors/,
|
||||||
|
/Missing frontmatter, metadata validation errors, and unreadable or non-UTF-8 files are not reported/,
|
||||||
|
[],
|
||||||
|
],
|
||||||
|
[
|
||||||
|
"en-US",
|
||||||
|
/Only YAML syntax errors/,
|
||||||
|
/Missing frontmatter, metadata validation errors, and unreadable or non-UTF-8 files are not reported/,
|
||||||
|
[{ package: "broken", path: "SKILL.md", code: "invalid_frontmatter" }],
|
||||||
|
],
|
||||||
|
[
|
||||||
|
"zh-CN",
|
||||||
|
/仅报告.*YAML 语法错误/,
|
||||||
|
/不报告缺少头部、元数据校验错误、无法读取或非 UTF-8 编码的文件/,
|
||||||
|
[],
|
||||||
|
],
|
||||||
|
[
|
||||||
|
"zh-CN",
|
||||||
|
/仅报告.*YAML 语法错误/,
|
||||||
|
/不报告缺少头部、元数据校验错误、无法读取或非 UTF-8 编码的文件/,
|
||||||
|
[{ package: "broken", path: "SKILL.md", code: "invalid_frontmatter" }],
|
||||||
|
],
|
||||||
|
])(
|
||||||
|
"explains YAML-only diagnostics and exclusions in %s",
|
||||||
|
async (locale, syntax, exclusions, failures) => {
|
||||||
|
i18n.locale = locale;
|
||||||
|
api.load.mockResolvedValue(failures);
|
||||||
|
const client = new QueryClient({
|
||||||
|
defaultOptions: { queries: { retry: false } },
|
||||||
|
});
|
||||||
|
render(
|
||||||
|
<QueryClientProvider client={client}>
|
||||||
|
<SkillDiagnostics userId="alice" />
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(screen.getByRole<HTMLButtonElement>("button").disabled).toBe(
|
||||||
|
false,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
expect(screen.getByText(syntax)).toBeTruthy();
|
||||||
|
expect(screen.getByText(exclusions)).toBeTruthy();
|
||||||
|
expect(screen.queryByText("broken/SKILL.md") === null).toBe(
|
||||||
|
failures.length === 0,
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
it("keeps warnings on reload failure, then refetches both lists after success", async () => {
|
||||||
|
const client = new QueryClient({
|
||||||
|
defaultOptions: { queries: { retry: false } },
|
||||||
|
});
|
||||||
|
client.setQueryData(["skills"], ["existing"]);
|
||||||
|
api.load.mockResolvedValue([
|
||||||
|
{
|
||||||
|
package: "broken",
|
||||||
|
path: "SKILL.md",
|
||||||
|
code: "invalid_frontmatter",
|
||||||
|
hint: "quote_colon_value",
|
||||||
|
line: 3,
|
||||||
|
column: 27,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
api.reload.mockRejectedValueOnce(new Error("offline"));
|
||||||
|
render(
|
||||||
|
<QueryClientProvider client={client}>
|
||||||
|
<SkillDiagnostics userId="alice" />
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
await screen.findByText("broken/SKILL.md:3:27");
|
||||||
|
expect(screen.getByText("Quote colon values")).toBeTruthy();
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: "Reload skills" }));
|
||||||
|
await screen.findByRole("alert");
|
||||||
|
expect(screen.getByText("broken/SKILL.md:3:27")).toBeTruthy();
|
||||||
|
expect(client.getQueryState(["skills"])?.isInvalidated).toBe(false);
|
||||||
|
let finish!: () => void;
|
||||||
|
api.reload.mockImplementationOnce(
|
||||||
|
() =>
|
||||||
|
new Promise<void>((resolve) => {
|
||||||
|
finish = resolve;
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
api.load.mockResolvedValue([]);
|
||||||
|
fireEvent.click(screen.getByRole("button", { name: "Reload skills" }));
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(
|
||||||
|
screen.getByRole<HTMLButtonElement>("button", {
|
||||||
|
name: "Reloading skills",
|
||||||
|
}).disabled,
|
||||||
|
).toBe(true),
|
||||||
|
);
|
||||||
|
expect(screen.getByText("broken/SKILL.md:3:27")).toBeTruthy();
|
||||||
|
await act(async () => finish());
|
||||||
|
await waitFor(() =>
|
||||||
|
expect(screen.queryByText("broken/SKILL.md:3:27")).toBeNull(),
|
||||||
|
);
|
||||||
|
expect(client.getQueryState(["skills"])?.isInvalidated).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not display a colon hint for generic YAML failures", async () => {
|
||||||
|
const client = new QueryClient({
|
||||||
|
defaultOptions: { queries: { retry: false } },
|
||||||
|
});
|
||||||
|
api.load.mockResolvedValue([
|
||||||
|
{ package: "broken", path: "SKILL.md", code: "invalid_frontmatter" },
|
||||||
|
]);
|
||||||
|
render(
|
||||||
|
<QueryClientProvider client={client}>
|
||||||
|
<SkillDiagnostics userId="bob" />
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
await screen.findByText("broken/SKILL.md");
|
||||||
|
expect(screen.getByText("Invalid YAML")).toBeTruthy();
|
||||||
|
expect(screen.queryByText("Quote colon values")).toBeNull();
|
||||||
|
});
|
||||||