Files
turnstone/tests/test_skill_parse_api.py
Patrick Buckley 06e16de066 chore(skills): drop Anthropic attribution from SKILL.md spec references
Two related cleanups landed together because they touch the same surface
(skill-spec uplift PRs #569/#570/#571/#572):

  1. Wording: replace "Anthropic spec" / "Anthropic Claude Code skill spec"
     with "SKILL.md spec" across admin UI tooltips, code comments, test
     docstrings, migration 056's module docstring, and the user-facing
     `arguments` description in tools/skills.json.  Renames a parser test
     `test_anthropic_tags` -> `test_nested_metadata_tags` and consolidates
     a parse-API test of the same shape; fixture author renamed
     `Anthropic` -> `Acme` to keep the fixture neutral.  Legitimate
     provider/SDK/API references (provider name, api.anthropic.com,
     `_anthropic.py`, capability comments) are intentionally untouched.

  2. Admin UX: in the Create + Edit Skill modals, six fields per modal
     (Compatibility, Paths, Hide-from-skill-picker, Arguments, Argument
     hint, Activation) had long uppercase label-hint spans crammed into
     the visible label.  Migrated each to the existing
     `.settings-help-btn` + `.settings-help-popover` pattern already used
     in the Settings tab — short label + inline `?` button that opens a
     styled popover with proper `<code>` formatting for technical tokens.

     Pattern reuse required two small generalisations in admin.js:

       * `_toggleSettingsHelp` now looks up the popover via a new
         `data-help-target="<id>"` attribute first, falling back to the
         settings-tab `.settings-label-col` ancestor lookup.
       * `_closeAllSettingsHelp` mirrors the same dual-path lookup when
         resetting `aria-expanded`, so modal buttons don't get stuck on
         `aria-expanded="true"` after another popover opens.
       * Added a document-delegated click handler that fires only for
         buttons with `data-help-target`; existing per-button binding
         in the settings-tab render path is unchanged.

     CSS: `.settings-help-btn` now paints its `?` via `::after` with the
     button's own `font-size: 0`, so prettier-introduced whitespace
     inside the new HTML buttons can't off-center the glyph.  The same
     rule applies to existing admin.js-generated buttons (text content
     hidden, pseudo identical).  Small additions for
     `.settings-help-popover code` / `strong` styling so technical
     tokens render with the same monospace pill treatment used elsewhere
     in skill UI.

Known follow-ups (intentionally NOT in this PR):
  * Migrate the settings-tab `_renderSettingRow` button assembly to the
    empty-`<button>` + `data-help-target` form so the per-button
    addEventListener loop can be dropped in favour of pure document
    delegation, and the `font-size: 0` rule stops being a workaround for
    two markup styles.
  * The 12 new popover blocks are duplicated verbatim between the
    Create and Edit modals (same as the rest of the create/edit modal
    pair).  A small renderer that emits popovers from a shared data
    object would eliminate the drift risk but is unrelated cleanup.
2026-05-24 14:56:43 -07:00

262 lines
9.0 KiB
Python
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
"""Tests for the SKILL.md parse admin API endpoint.
The endpoint is a thin permission-checked wrapper around
``turnstone.core.skill_parser.parse_skill_md``. These tests cover the
routing, auth, and error-handling layers — parser semantics live in
``test_skill_parser.py``.
"""
from __future__ import annotations
from typing import TYPE_CHECKING, Any
import pytest
from starlette.applications import Starlette
from starlette.middleware import Middleware
from starlette.middleware.base import BaseHTTPMiddleware
from starlette.routing import Mount, Route
from starlette.testclient import TestClient
if TYPE_CHECKING:
from collections.abc import Iterator
from starlette.requests import Request
from starlette.responses import Response
from turnstone.console.server import admin_parse_skill
from turnstone.core.auth import AuthResult
class _InjectAuthMiddleware(BaseHTTPMiddleware):
async def dispatch(self, request: Request, call_next: Any) -> Response:
request.state.auth_result = AuthResult(
user_id="test-user",
scopes=frozenset({"approve"}),
token_source="config",
permissions=frozenset({"read", "write", "approve", "admin.skills"}),
)
return await call_next(request)
class _InjectAuthNoSkillsMiddleware(BaseHTTPMiddleware):
async def dispatch(self, request: Request, call_next: Any) -> Response:
request.state.auth_result = AuthResult(
user_id="test-user",
scopes=frozenset({"approve"}),
token_source="jwt",
permissions=frozenset({"read", "write", "approve"}),
)
return await call_next(request)
_ROUTES = [
Mount(
"/v1",
routes=[
Route("/api/admin/skills/parse", admin_parse_skill, methods=["POST"]),
],
),
]
@pytest.fixture
def client() -> TestClient:
app = Starlette(
routes=_ROUTES,
middleware=[Middleware(_InjectAuthMiddleware)],
)
return TestClient(app)
@pytest.fixture
def client_no_perm() -> TestClient:
app = Starlette(
routes=_ROUTES,
middleware=[Middleware(_InjectAuthNoSkillsMiddleware)],
)
return TestClient(app)
_FULL_SKILL = """\
---
name: code-review
description: Automated code review skill
author: Test Author
version: 2.0.0
tags: [python, review, quality]
allowed-tools: [read_file, list_directory]
paths: ["**/*.py", "src/api/**"]
when_to_use: when the user asks to review code
model: claude-opus-4-7
effort: high
disable-model-invocation: true
user-invocable: false
arguments: [pr_number, focus]
argument-hint: "[pr-number] [focus-area]"
license: MIT
compatibility: ">=0.7"
---
# Code Review
Review code for best practices.
"""
_MINIMAL_SKILL = """\
---
name: minimal
---
Just some content.
"""
class TestParseSkill:
def test_parses_full_frontmatter(self, client: TestClient) -> None:
resp = client.post("/v1/api/admin/skills/parse", json={"raw": _FULL_SKILL})
assert resp.status_code == 200
data = resp.json()
assert data["name"] == "code-review"
# ``when_to_use`` is concatenated into description by the parser;
# the separate ``when_to_use`` field below shows the raw source.
assert data["description"] == (
"Automated code review skill\n\nWhen to use: when the user asks to review code"
)
assert data["author"] == "Test Author"
assert data["version"] == "2.0.0"
assert data["tags"] == ["python", "review", "quality"]
assert data["allowed_tools"] == ["read_file", "list_directory"]
assert data["paths"] == ["**/*.py", "src/api/**"]
assert data["when_to_use"] == "when the user asks to review code"
assert data["model"] == "claude-opus-4-7"
assert data["effort"] == "high"
assert data["disable_model_invocation"] is True
assert data["user_invocable"] is False
assert data["arguments"] == ["pr_number", "focus"]
assert data["argument_hint"] == "[pr-number] [focus-area]"
assert data["license"] == "MIT"
assert data["compatibility"] == ">=0.7"
assert "# Code Review" in data["content"]
# Frontmatter should not leak into the body.
assert "name: code-review" not in data["content"]
# ParsedSkill carries raw_frontmatter (the full YAML dict) but the
# handler whitelists fields by hand to avoid leaking arbitrary keys.
# Pin that contract — a future refactor to dataclasses.asdict would
# silently break it without this assertion.
assert "raw_frontmatter" not in data
def test_parses_minimal_frontmatter(self, client: TestClient) -> None:
resp = client.post("/v1/api/admin/skills/parse", json={"raw": _MINIMAL_SKILL})
assert resp.status_code == 200
data = resp.json()
assert data["name"] == "minimal"
assert data["description"] == "Just some content."
assert data["version"] == "1.0.0"
assert data["tags"] == []
assert data["allowed_tools"] == []
assert data["paths"] == []
assert data["when_to_use"] == ""
assert data["model"] == ""
assert data["effort"] == ""
# Spec defaults: model can autoload, user can pick.
assert data["disable_model_invocation"] is False
assert data["user_invocable"] is True
assert data["arguments"] == []
assert data["argument_hint"] == ""
assert data["license"] == ""
def test_nested_metadata_tags(self, client: TestClient) -> None:
# Some SKILL.md authors put tags under metadata.tags rather than
# at the top level — the parser must handle both layouts.
raw = """\
---
name: nested-meta
description: A skill using nested metadata
metadata:
tags: [alpha, beta]
author: Acme
version: 3.1.4
---
Body.
"""
resp = client.post("/v1/api/admin/skills/parse", json={"raw": raw})
assert resp.status_code == 200
data = resp.json()
assert data["tags"] == ["alpha", "beta"]
assert data["author"] == "Acme"
assert data["version"] == "3.1.4"
def test_unquoted_colon_in_description(self, client: TestClient) -> None:
# Common cross-client mistake: ``description: Use when: the user...``
# The parser retries with the description value quoted.
raw = """\
---
name: colon-desc
description: Use when: the user asks for a review
---
Body.
"""
resp = client.post("/v1/api/admin/skills/parse", json={"raw": raw})
assert resp.status_code == 200
data = resp.json()
assert data["name"] == "colon-desc"
assert "Use when" in data["description"]
def test_missing_name_returns_400(self, client: TestClient) -> None:
raw = """\
---
description: No name field
---
Body.
"""
resp = client.post("/v1/api/admin/skills/parse", json={"raw": raw})
assert resp.status_code == 400
assert "name" in resp.json()["error"].lower()
def test_missing_raw_returns_400(self, client: TestClient) -> None:
resp = client.post("/v1/api/admin/skills/parse", json={})
assert resp.status_code == 400
assert "raw" in resp.json()["error"].lower()
def test_blank_raw_returns_400(self, client: TestClient) -> None:
resp = client.post("/v1/api/admin/skills/parse", json={"raw": " \n"})
assert resp.status_code == 400
def test_invalid_yaml_returns_400(self, client: TestClient) -> None:
# YAML that the malformed-description retry can't fix.
raw = "---\nname: [not, valid, here\n---\nBody.\n"
resp = client.post("/v1/api/admin/skills/parse", json={"raw": raw})
assert resp.status_code == 400
def test_requires_admin_skills_permission(self, client_no_perm: TestClient) -> None:
resp = client_no_perm.post("/v1/api/admin/skills/parse", json={"raw": _MINIMAL_SKILL})
assert resp.status_code == 403
def test_oversized_content_length_returns_413(self, client: TestClient) -> None:
# Content-Length pre-check rejects oversized bodies before they're
# buffered into memory. Caps worker memory against an admin-token
# holder spraying multi-GB JSON. The threshold is generous (~4×
# the per-string cap) so payload here must clearly exceed it.
oversized = "a" * 200_000
resp = client.post("/v1/api/admin/skills/parse", json={"raw": oversized})
assert resp.status_code == 413
def test_oversized_raw_chunked_returns_413(self, client: TestClient) -> None:
# When the client sends Transfer-Encoding: chunked there is no
# Content-Length header, so the pre-check is skipped and the
# application-layer cap is the only line of defence. httpx switches
# to chunked when the body is a generator.
def _gen() -> Iterator[bytes]:
yield b'{"raw":"' + b"a" * 33_000 + b'"}'
resp = client.post(
"/v1/api/admin/skills/parse",
content=_gen(),
headers={"Content-Type": "application/json"},
)
assert resp.status_code == 413
assert "raw" in resp.json()["error"].lower()