diff --git a/README.md b/README.md index e37de0f8..78c6d8ed 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ Turnstone includes a built-in governance layer for enterprise deployments — ma - **RBAC** — 15 granular permissions, 3 built-in roles (admin / operator / viewer), custom roles, privilege escalation prevention - **OIDC SSO** — single sign-on via any OpenID Connect provider (Okta, Azure AD, Google, Keycloak); Authorization Code Flow with PKCE, auto-provisioning, claim-based role mapping with demotion propagation; see [docs/oidc.md](docs/oidc.md) - **Tool policies** — glob-pattern rules (`allow` / `deny` / `ask`) with priority ordering; automate approvals or lock down dangerous tools -- **Skills** — reusable behavioral profiles with system prompts, `{{variable}}` substitution, session config (model, temperature, token budget), install-time security scanning, version history, external discovery (skills.sh / GitHub), and runtime `load_skill` tool for model-driven skill activation +- **Skills** — reusable behavioral profiles with system prompts, `{{variable}}` substitution, session config (model, temperature, token budget), install-time security scanning, version history, external discovery (skills.sh / GitHub), and runtime `skill` tool for model-driven skill activation - **Usage tracking** — per-request token and tool metrics, aggregation by day / model / user, automatic 90-day pruning - **Audit logging** — append-only event trail for all admin mutations, IP-aware, 365-day retention diff --git a/docs/diagrams/26-skills-discovery-architecture.puml b/docs/diagrams/26-skills-discovery-architecture.puml index 06143964..e3a67086 100644 --- a/docs/diagrams/26-skills-discovery-architecture.puml +++ b/docs/diagrams/26-skills-discovery-architecture.puml @@ -33,7 +33,7 @@ package "Core Modules" as core #181825 { } package "Session Runtime" as runtime #181825 { - rectangle "load_skill tool\nsession.py" as loadtool + rectangle "skill tool\nsession.py" as loadtool rectangle "set_skill()\nsession.py" as setskill rectangle "_load_skills()\nsession.py" as loadskills } @@ -80,6 +80,7 @@ importui --> install : POST (github source) ' Annotations note right of parser YAML frontmatter -> ParsedSkill + allowed-tools (standard) -> allowed_tools (internal) Anthropic + Hermes tag formats Name validation (lowercase+hyphens) end note diff --git a/docs/governance.md b/docs/governance.md index 104a1ce5..d7b3b077 100644 --- a/docs/governance.md +++ b/docs/governance.md @@ -70,7 +70,7 @@ etc.) since workstream templates were merged into the skills system in v0.8.0. `{{node_id}}` (server node ID). Unrecognized placeholders are kept as-is. - **Runtime switching**: `/template ` to switch, `/template clear` to revert to defaults, `/template` to show current. Persisted across resume. -- **Model-driven loading**: The `load_skill` built-in tool lets the model +- **Model-driven loading**: The `skill` built-in tool lets the model discover and activate skills mid-conversation. `search` action finds skills by query (auto-approved); `load` action activates by name (requires user approval since it changes session behavior). Main session only. @@ -83,11 +83,15 @@ etc.) since workstream templates were merged into the skills system in v0.8.0. precedence on name collision. MCP-synced content updates reset `is_default` to prevent compromised servers from injecting defaults. Admin UI shows origin badge and disables edit/delete for MCP-sourced skills. +- **Spec fields**: Skills support the full Agent Skills standard frontmatter: + `name`, `description`, `license`, `compatibility`, `metadata` (author, version), + `allowed-tools`. The `license` and `compatibility` fields are preserved on import + and editable in the admin UI. See https://agentskills.io/specification. - **Security scanning**: Skills are automatically scanned at creation and update time. The scanner evaluates four risk axes: content risk (command execution, data exfiltration), supply chain risk (pipe-to-shell, transitive installs), vulnerability risk (prompt injection, insecure credentials), and declared - capability risk (from `allowed_tools`). Results populate the `scan_status` + capability risk (from `allowed-tools` in SKILL.md). Results populate the `scan_status` (safe/low/medium/high/critical) and `scan_report` (JSON breakdown) columns. These fields are system-managed and cannot be overwritten via the admin API. - **Discovery**: External skills can be discovered and installed from registries: diff --git a/docs/judge.md b/docs/judge.md index 591982b5..15325e72 100644 --- a/docs/judge.md +++ b/docs/judge.md @@ -299,7 +299,7 @@ four independent risk axes: obfuscation, download-execute chains, executable URLs from untrusted domains 3. **Vulnerability risk** — prompt injection patterns, insecure credential handling, third-party content exposure (indirect prompt injection surface) -4. **Declared capability risk** — parsed from the skill's `allowed_tools` field. +4. **Declared capability risk** — parsed from `allowed-tools` in the skill's SKILL.md. `Bash(*)` (unrestricted shell) is high risk. `Bash(git:*)` is low. Read-only tools are safe. diff --git a/docs/tools.md b/docs/tools.md index 0fea58e1..faa9a22d 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -492,7 +492,7 @@ data.get("mergedAt") is not None --- -### load_skill +### skill Discover and activate skills at runtime during a conversation. The model can search for available skills and load one by name, replacing the current active @@ -543,7 +543,7 @@ pre-configure skills at workstream creation. | `watch` | Monitor | No (create) | No | No | `command` | | `read_resource`| MCP | No | Yes | Yes | `uri` | | `use_prompt` | MCP | No | Yes | Yes | `name` | -| `load_skill` | Skills | No (load) | No | No | `name` | +| `skill` | Skills | No (load) | No | No | `name` | | `tool_search`| Search | Yes | No | No | `query` | --- diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index 7f0df260..dc3060dc 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -187,6 +187,8 @@ export interface SkillInfo { notify_on_complete: string; enabled: boolean; allowed_tools: string; + license: string; + compatibility: string; resource_count: number; created: string; updated: string; @@ -214,6 +216,8 @@ export interface CreateSkillRequest { notify_on_complete?: string; enabled?: boolean; allowed_tools?: string; + license?: string; + compatibility?: string; } export interface UpdateSkillRequest { @@ -237,6 +241,8 @@ export interface UpdateSkillRequest { notify_on_complete?: string; enabled?: boolean; allowed_tools?: string; + license?: string; + compatibility?: string; } export interface ListSkillsResponse { diff --git a/tests/test_load_skill.py b/tests/test_load_skill.py index 8b8356e2..0ec328eb 100644 --- a/tests/test_load_skill.py +++ b/tests/test_load_skill.py @@ -1,4 +1,4 @@ -"""Tests for the load_skill built-in tool.""" +"""Tests for the skill built-in tool.""" from __future__ import annotations @@ -9,25 +9,25 @@ from turnstone.core.tools import BUILTIN_TOOL_NAMES, PRIMARY_KEY_MAP class TestToolRegistration: - """Verify load_skill is registered correctly.""" + """Verify skill is registered correctly.""" def test_in_builtin_tool_names(self) -> None: - assert "load_skill" in BUILTIN_TOOL_NAMES + assert "skill" in BUILTIN_TOOL_NAMES def test_not_agent_tool(self) -> None: from turnstone.core.tools import AGENT_TOOLS names = {t["function"]["name"] for t in AGENT_TOOLS} - assert "load_skill" not in names + assert "skill" not in names def test_not_task_agent_tool(self) -> None: from turnstone.core.tools import TASK_AGENT_TOOLS names = {t["function"]["name"] for t in TASK_AGENT_TOOLS} - assert "load_skill" not in names + assert "skill" not in names def test_has_primary_key(self) -> None: - assert PRIMARY_KEY_MAP.get("load_skill") == "name" + assert PRIMARY_KEY_MAP.get("skill") == "name" # --------------------------------------------------------------------------- @@ -82,12 +82,12 @@ def _make_session(skills: list[dict[str, Any]] | None = None): class TestPrepareLoadSkill: - """Test _prepare_load_skill validation and item dict shape.""" + """Test _prepare_skill validation and item dict shape.""" def test_load_valid(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "load", "name": "code-review"}) - assert item["func_name"] == "load_skill" + item = session._prepare_skill("call-1", {"action": "load", "name": "code-review"}) + assert item["func_name"] == "skill" assert item["action"] == "load" assert item["name"] == "code-review" assert item["needs_approval"] is True @@ -96,19 +96,19 @@ class TestPrepareLoadSkill: def test_load_missing_name(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "load"}) + item = session._prepare_skill("call-1", {"action": "load"}) assert "error" in item assert "name" in item["error"].lower() assert item["needs_approval"] is False def test_load_empty_name(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "load", "name": ""}) + item = session._prepare_skill("call-1", {"action": "load", "name": ""}) assert "error" in item def test_search_with_query(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "code review"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "code review"}) assert item["action"] == "search" assert item["query"] == "code review" assert item["needs_approval"] is False @@ -116,30 +116,30 @@ class TestPrepareLoadSkill: def test_search_without_query(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search"}) + item = session._prepare_skill("call-1", {"action": "search"}) assert item["action"] == "search" assert item["query"] == "" assert item["needs_approval"] is False def test_invalid_action(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "delete"}) + item = session._prepare_skill("call-1", {"action": "delete"}) assert "error" in item assert "delete" in item["error"] def test_empty_action(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": ""}) + item = session._prepare_skill("call-1", {"action": ""}) assert "error" in item def test_header_for_load(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "load", "name": "my-skill"}) + item = session._prepare_skill("call-1", {"action": "load", "name": "my-skill"}) assert "my-skill" in item["header"] def test_header_for_search(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "testing"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "testing"}) assert "testing" in item["header"] @@ -149,7 +149,7 @@ class TestPrepareLoadSkill: class TestExecLoadSkill: - """Test _exec_load_skill execution logic.""" + """Test _exec_skill execution logic.""" def test_load_existing_skill(self) -> None: skills = [ @@ -164,8 +164,8 @@ class TestExecLoadSkill: session, _, fake_get = _make_session(skills) with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get): - item = session._prepare_load_skill("call-1", {"action": "load", "name": "code-review"}) - call_id, result = session._exec_load_skill(item) + item = session._prepare_skill("call-1", {"action": "load", "name": "code-review"}) + call_id, result = session._exec_skill(item) assert call_id == "call-1" assert "code-review" in result @@ -177,8 +177,8 @@ class TestExecLoadSkill: session, _, fake_get = _make_session([]) with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get): - item = session._prepare_load_skill("call-1", {"action": "load", "name": "nope"}) - call_id, result = session._exec_load_skill(item) + item = session._prepare_skill("call-1", {"action": "load", "name": "nope"}) + call_id, result = session._exec_skill(item) assert "not found" in result.lower() assert session._set_skill_called == [] @@ -188,8 +188,8 @@ class TestExecLoadSkill: session, _, fake_get = _make_session(skills) with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get): - item = session._prepare_load_skill("call-1", {"action": "load", "name": "test"}) - session._exec_load_skill(item) + item = session._prepare_skill("call-1", {"action": "load", "name": "test"}) + session._exec_skill(item) session.ui.on_tool_result.assert_called_once() @@ -216,10 +216,10 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = skills session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "code"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "code"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "code-review" in result # docs-writer shouldn't match "code" query @@ -241,10 +241,10 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = skills session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search"}) + item = session._prepare_skill("call-1", {"action": "search"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) # Should be limited to 10 assert result.count("skill-") == 10 @@ -254,10 +254,10 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = [] session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "nonexistent"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "nonexistent"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "no skills found" in result.lower() @@ -276,21 +276,21 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = skills session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "risky"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "risky"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "high" in result def test_search_storage_failure_returns_empty(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "test"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "test"}) with patch( "turnstone.core.storage._registry.get_storage", side_effect=RuntimeError("no storage") ): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "no skills found" in result.lower() @@ -307,10 +307,8 @@ class TestExecLoadSkill: session, _, fake_get = _make_session(skills) with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get): - item = session._prepare_load_skill( - "call-1", {"action": "load", "name": "disabled-skill"} - ) - call_id, result = session._exec_load_skill(item) + item = session._prepare_skill("call-1", {"action": "load", "name": "disabled-skill"}) + call_id, result = session._exec_skill(item) assert "not found" in result.lower() assert session._set_skill_called == [] @@ -321,8 +319,8 @@ class TestExecLoadSkill: session._skill_name = "active" with patch("turnstone.core.session.get_skill_by_name", side_effect=fake_get): - item = session._prepare_load_skill("call-1", {"action": "load", "name": "active"}) - call_id, result = session._exec_load_skill(item) + item = session._prepare_skill("call-1", {"action": "load", "name": "active"}) + call_id, result = session._exec_skill(item) assert "already active" in result.lower() assert session._set_skill_called == [] @@ -352,10 +350,10 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = skills session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search"}) + item = session._prepare_skill("call-1", {"action": "search"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "enabled-skill" in result assert "disabled-skill" not in result @@ -375,14 +373,113 @@ class TestExecLoadSkill: mock_storage.list_prompt_templates.return_value = skills session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "search", "query": "code review"}) + item = session._prepare_skill("call-1", {"action": "search", "query": "code review"}) with patch("turnstone.core.storage._registry.get_storage", return_value=mock_storage): - call_id, result = session._exec_load_skill(item) + call_id, result = session._exec_skill(item) assert "code-review" in result def test_preparer_load_has_approval_label(self) -> None: session, _, _ = _make_session() - item = session._prepare_load_skill("call-1", {"action": "load", "name": "my-skill"}) - assert item["approval_label"] == "load_skill__my-skill" + item = session._prepare_skill("call-1", {"action": "load", "name": "my-skill"}) + assert item["approval_label"] == "skill__my-skill" + + +# --------------------------------------------------------------------------- +# Tests: Skill Catalog Disclosure (Agent Skills standard compliance) +# --------------------------------------------------------------------------- + + +class TestSkillCatalogDisclosure: + """Verify catalog appears in system messages.""" + + def _build_session_with_system_messages( + self, + search_skills: list[dict[str, Any]] | None = None, + ) -> Any: + """Build a session and call _init_system_messages to get dev_parts.""" + from turnstone.core.session import ChatSession + + session = ChatSession.__new__(ChatSession) + ui = MagicMock() + session.ui = ui + session.model = "test-model" + session._ws_id = "ws-test" + session._node_id = "node-1" + session._skill_name = None + session._skill_content = None + session._skill_resources = {} + session._applied_skill_content = None + session.context_window = 128000 + session.messages = [] + session._config = {} + session.creative_mode = False + session.instructions = "" + session.system_messages = [] + session._agent_system_messages = [] + session.reasoning_effort = "medium" + session._pending_nudge = [] + session._tool_search = None + session._mcp_client = None + session._notify_on_complete = "{}" + + # Memory stubs + session._memory_config = MagicMock() + session._memory_config.fetch_limit = 0 + session._user_id = "" + + with ( + patch( + "turnstone.core.session.list_skills_by_activation", + return_value=search_skills or [], + ), + patch.object(session, "_get_visible_memories", return_value=[]), + ): + session._init_system_messages() + + return session + + def test_catalog_present_with_search_skills(self) -> None: + skills = [ + {"name": "pdf-processing", "description": "Extract PDF text and forms."}, + {"name": "data-analysis", "description": "Analyze datasets."}, + ] + session = self._build_session_with_system_messages(search_skills=skills) + content = session.system_messages[0]["content"] + assert "" in content + assert "pdf-processing" in content + assert "data-analysis" in content + assert "" in content + + def test_catalog_omitted_when_no_search_skills(self) -> None: + session = self._build_session_with_system_messages(search_skills=[]) + content = session.system_messages[0]["content"] + assert "" not in content + + def test_catalog_capped_at_30(self) -> None: + skills = [{"name": f"skill-{i:03d}", "description": f"Desc {i}"} for i in range(50)] + session = self._build_session_with_system_messages(search_skills=skills) + content = session.system_messages[0]["content"] + # Should include first 30, not all 50 + assert "skill-029" in content + assert "skill-030" not in content + + def test_catalog_escapes_html(self) -> None: + skills = [ + {"name": "xss-test", "description": "Handle