diff --git a/config.yaml.example b/config.yaml.example index 9d897392..0f786ba2 100644 --- a/config.yaml.example +++ b/config.yaml.example @@ -127,3 +127,18 @@ agents: - "final_answer_tool" - "my_custom_tool" - "my_other_tool" + + # Optional: Skills (Anthropic Agent Skills model) + # Each skill is a directory with a SKILL.md (frontmatter: name, description). + # Skill name + description are auto-registered into the system prompt so the + # agent can invoke a skill on its own via the `use_skill` tool; skills are + # also exposed as commands over ACP (available_commands). + # By default skills are read from ./.agent/skills (CWD) and ~/.agent/skills; + # missing/empty folders are fine. Use the block below to customize. + # skills: + # enabled: true + # paths: # overrides the default roots (relative to CWD) + # - "./.agent/skills" + # include: null # optional allowlist of skill names to activate + # exclude: null # optional denylist of skill names + # The prompt listing budget is global: execution.max_skill_desc_chars (500). diff --git a/docs/design/skills.md b/docs/design/skills.md new file mode 100644 index 00000000..b778f323 --- /dev/null +++ b/docs/design/skills.md @@ -0,0 +1,206 @@ +# Skills subsystem — design & specification + +Status: draft (feat/skills) +Author: feat/skills work +Layered per `.cursor/rules/architecture.mdc` (bottom-up, one class at a time, TDD). + +## 1. Motivation + +Add first-class **Agent Skills** to SGR Agent Core, modeled on the Anthropic +"Agent Skills" pattern (a `SKILL.md` file with YAML frontmatter + markdown body, +progressive disclosure). Two capabilities are required: + +1. **Autonomous invocation** — the agent automatically *registers* available + skills into its system prompt (name + description only, "progressive + disclosure" level 1) so the LLM can decide, on its own, to invoke a skill. + Invoking a skill loads its full body (level 2) into the conversation. +2. **Skills as commands** — skills are surfaced as user-visible **commands** + through the protocol layers: ACP `available_commands` (slash commands in ACP + clients such as Zed) and an optional MCP prompts server. This mirrors how + codex / Claude Code / Cursor expose reusable prompts/skills as commands. + +## 2. What a skill is (Anthropic model) + +A skill is a directory containing a `SKILL.md`: + +``` +skills/ + pdf-processing/ + SKILL.md # required: frontmatter + body + scripts/ # optional bundled resources (level 3) + references/ +``` + +`SKILL.md` frontmatter (YAML). The portable open standard requires exactly two +fields — `name` and `description`; the rest are optional extensions. + +| field | required | validation / notes | +|-----------------|----------|--------------------------------------------------------------------| +| `name` | yes | ≤ 64 chars; only lowercase letters, digits, hyphens; no XML tags; not "anthropic"/"claude" | +| `description` | yes | non-empty; ≤ 1024 chars; no XML tags; third person; *what it does* + *when to use* | +| `license` | no | SPDX id or text | +| `allowed-tools` | no | advisory allowlist of tool names the skill uses (soft, documented) | +| `metadata` | no | free-form dict (version, author, ...) | + +Validation is enforced at parse time (Anthropic spec rules). Body: markdown +instructions (level-2 content, keep under ~500 lines / ~5k tokens); push long +material into reference files loaded on demand. + +### Progressive disclosure (three levels) + +- **Level 1 — metadata**: `name` + `description` always injected into the system + prompt so the model knows the skill exists and when to reach for it. +- **Level 2 — body**: the `SKILL.md` markdown body, loaded into context only when + the skill is invoked. +- **Level 3 — bundled files**: scripts/resources under the skill dir, referenced + from the body and read/run only as needed. + +## 3. Discovery + +Skill roots are scanned (in order; later roots override same-named skills): + +1. Packaged/builtin skills dir (optional, shipped with an example). +2. User dir: `~/.sgr/skills` (personal). +3. Project dir: `./.sgr/skills` relative to `config_dir`. +4. Explicit dirs from config (`skills.paths`) and per-agent `skills:` selection. + +A skill dir is any immediate subdirectory that contains a `SKILL.md`. + +## 4. Architecture (layered) + +### Layer 1 — data model + loader (no deps) + +- `sgr_agent_core/skills/models.py` + - `SkillMetadata(BaseModel)`: `name`, `description`, `license: str | None`, + `allowed_tools: list[str]`, `metadata: dict`. + - `Skill(BaseModel)`: `metadata: SkillMetadata`, `body: str`, `path: Path`. + Convenience props: `name`, `description`. +- `sgr_agent_core/skills/loader.py` + - `SkillLoader.load_skill(dir: Path) -> Skill` — parse `SKILL.md` + (frontmatter via a tiny YAML front-matter splitter; no new dependency — + PyYAML already present). Validate required fields; raise `SkillError` + on malformed/missing. + - `SkillLoader.discover(root: Path) -> list[Skill]` — scan subdirs. + +### Layer 2 — registry + config + +- `sgr_agent_core/skills/registry.py` + - `SkillRegistry` — runtime catalog (dict name -> Skill). Unlike ToolRegistry + it is populated by scanning dirs (skills are data, not subclasses). + Methods: `register`, `get`, `list_items`, `load_from_paths(paths)`, + `clear`. +- Config: `SkillsConfig(BaseModel)` added to `AgentConfig` + (`skills: SkillsConfig | None`), with: + - `paths: list[str]` — extra skill roots. + - `enabled: bool = True`. + - `include: list[str] | None` / `exclude: list[str] | None` — filter by name. + Per-agent selection via `AgentDefinition.skills: list[str]` (names) — resolved + in a validator mirroring `agent_level_tools_validator`. + +### Layer 3 — prompt injection + invocation tool + +- `PromptLoader.get_system_prompt(..., available_skills=...)` — render a + `{available_skills}` block (numbered `name: description`). Default + `system_prompt.txt` gains an `` section. Always pass the + kwarg (empty string when no skills) so custom templates using the placeholder + never KeyError, and templates without it are unaffected (`str.format` ignores + extra kwargs). +- `sgr_agent_core/tools/skill_tool.py` + - `SkillTool(SystemBaseTool)` — `tool_name = "use_skill"`. Field: + `skill_name: str`. `__call__` looks up the skill (from + `context.available_skills` / SkillRegistry), returns the level-2 body as a + string (which the agent appends to conversation). Unknown skill -> helpful + error string listing available skills. This is the autonomous-invocation + vehicle and flows through both function-calling and SGR union paths via the + registry — same pattern as `SearchToolsTool`. + +### Layer 4 — agent + factory wiring + +- `AgentContext` gains `available_skills: list[Skill]` (default empty) so tools + can resolve skills without global state. +- `BaseAgent._prepare_context()` passes + `available_skills=self.available_skills` to `PromptLoader.get_system_prompt`. + `BaseAgent.__init__` accepts `skills: list[Skill] | None`. +- `AgentFactory.create` resolves skills (from `SkillRegistry` + + `agent_def.skills` + config paths), injects `SkillTool` into the toolkit when + skills are present, and threads the skill list into the agent + context. + +### Layer 5 — protocol command surfaces + +- **ACP**: `SGRACPBridge` sends an `AvailableCommandsUpdate` session update + after `new_session` (and on agent switch), advertising each skill as an + `AvailableCommand(name, description, input=UnstructuredCommandInput(...))`. + `/skill-name` references are NOT handled in the bridge; they are expanded + centrally in `AgentFactory.create` (via `inject_referenced_skills`) for every + mode (ACP/CLI/server), detecting a reference anywhere in the last user message. +- **MCP**: considered (skills as MCP prompts) but **dropped** — the ACP + `available_commands` surface already covers "skills as commands", and MCP + prompts added a parallel server for little extra value. +- **CLI**: `sgrsh --list-skills` prints discovered skills; `/skill` handling in + chat loop (nice-to-have). +- **Server**: `GET /v1/skills` lists discovered skills (OpenAI-adjacent). + +## 5. Testing plan (TDD, red first) + +- `tests/test_skill_loader.py` — frontmatter parsing, required-field validation, + discovery, malformed handling. +- `tests/test_skill_registry.py` — register/get/list, path loading, override. +- `tests/test_skills_config.py` — config parse, per-agent selection validator. +- `tests/test_prompts.py` (extend) — `{available_skills}` rendering. +- `tests/test_skill_tool.py` — `use_skill` returns body / errors. +- `tests/test_agent_factory.py` (extend) — factory injects SkillTool + skills. +- `tests/test_acp_bridge.py` (extend) — available_commands advertised; `/name` + routes to skill. +- `tests/test_skills_e2e.py` (`@pytest.mark.e2e`) — end-to-end autonomous + invocation with a mocked LLM choosing `use_skill`. +- Mode checks: ACP, CLI, OpenAI server. + +## 5a. Listing budget & invocation semantics + +- **Listing budget (level 1).** Injecting many skills must not blow the system + prompt. Each rendered entry is `name: description` with the description + truncated to a per-entry cap (`SkillsConfig.max_desc_chars`, default 500). + This mirrors Claude Code's skill-listing budget (~1% context, ~1536-char + per-entry cap). The name is always shown in full. +- **Invocation = prompt injection.** When `use_skill` runs, the level-2 body is + returned as the tool result and appended to the conversation, where it + persists for the rest of the run (matching Anthropic semantics). No isolated + subagent in v1. +- **ACP push mechanism.** The bridge advertises commands via + `client.session_update(session_id, AvailableCommandsUpdate(available_commands=[ + AvailableCommand(name, description, input=UnstructuredCommandInput(hint=...))]))` + — same `session_update` channel the streaming generator already uses. + +## 5b. Prior art (codex / Claude Code / Cursor / MCP) + +Autonomous invocation across all these tools = inject a rendered catalog block +of `name + description (+ when-to-use)` into the system/developer context, under +a budget, plus a "how to use" instruction telling the model to load the full +body on match. Concretely: + +- **Codex** `render_available_skills_body` emits a `## Skills` section: + `- {name}: {description} (file: {path})` followed by a "How to use skills" + block with trigger rules ("if the task matches a skill's description, use it"; + read `SKILL.md` fully first). Budget = 2% of context. +- **Claude Code** injects a skill listing (name + truncated description), + budget ~1% of context, per-entry cap ~1536 chars; loads the body on trigger. +- **Cursor** "Apply Intelligently" rules present the description to the agent to + decide relevance. +- **MCP `prompts`** primitive (`prompts/list` / `prompts/get` / + `notifications/prompts/list_changed`; Prompt = `name`/`title`/`description`/ + `arguments`) is **user-controlled by spec** → renders as slash commands. This + is the transport for the *command* surface; autonomous invocation still needs + the description in the model catalog (our system-prompt block + `use_skill`). + +Our `{available_skills}` block mirrors the codex format (name + description) and +adds an explicit instruction to call `use_skill` to load a skill's body, giving +the same autonomous-invocation behavior. `model_invocable` / `user_invocable` +flags (mirroring codex/Claude `disable-model-invocation` / `user-invocable`) +gate whether a skill appears in the model catalog vs the command menu. + +## 6. Non-goals / constraints + +- No new runtime dependency (reuse PyYAML, fastmcp, acp already present). +- Backward compatible: agents without skills behave exactly as before + (`{available_skills}` optional, SkillTool only added when skills exist). +- English comments; 120-char lines; ruff clean. diff --git a/docs/en/framework/skills.md b/docs/en/framework/skills.md new file mode 100644 index 00000000..70410ff3 --- /dev/null +++ b/docs/en/framework/skills.md @@ -0,0 +1,129 @@ +# Skills + +Skills are reusable, on-demand instruction packages (the Anthropic *Agent +Skills* model). Each skill is a directory with a `SKILL.md` file — YAML +frontmatter plus a markdown body. SGR Agent Core auto-registers skill +`name` + `description` into the agent system prompt so the agent can invoke a +skill autonomously, and also surfaces skills as **commands** over ACP, the CLI, +and the HTTP server. + +## What a skill looks like + +``` +skills/ + citation-style/ + SKILL.md + concise-answer/ + SKILL.md +``` + +`SKILL.md`: + +```markdown +--- +name: citation-style +description: Formats research citations in a consistent numbered style. Use when writing reports that reference web sources. +--- + +# Citation style + +1. Number every source in order: [1], [2], ... +2. Collect all sources under a `## Sources` heading. +``` + +### Frontmatter + +| field | required | notes | +|-----------------|----------|--------------------------------------------------------------------| +| `name` | yes | ≤ 64 chars; lowercase letters, digits, hyphens; no `anthropic`/`claude` | +| `description` | yes | non-empty, ≤ 1024 chars; write in third person, say *what* and *when* | +| `license` | no | SPDX id or text | +| `allowed-tools` | no | advisory list of tool names the skill uses | +| `metadata` | no | free-form mapping (version, author, ...) | +| `disable-model-invocation` | no | `true` hides the skill from the model catalog (user command only) | +| `user-invocable` | no | `false` hides the skill from command menus (model-only) | + +If `name` is omitted it defaults to the directory name. + +## Progressive disclosure + +1. **Level 1 — metadata**: `name` + `description` are always injected into the + system prompt (a compact catalog block), so the model knows the skill exists. +2. **Level 2 — body**: the `SKILL.md` body is loaded only when the skill is + invoked (the `use_skill` tool returns it into the conversation). +3. **Level 3 — resources**: any bundled files are read only when needed. + +## Enabling skills + +Skills work out of the box: **by default** the agent scans + +1. `./.agent/skills` (project, relative to the current working directory) +2. `~/.agent/skills` (personal) + +If those folders are missing or empty, nothing breaks — the agent runs normally +with no skills. To customize, add a `skills` block to an agent (or globally, at +the top level) in `config.yaml`: + +```yaml +agents: + sgr_agent: + base_class: SGRToolCallingAgent + tools: + - reasoningtool + skills: + enabled: true # default true; set false to disable skills + paths: # override the default roots (relative to CWD) + - ./my-skills + include: null # optional allowlist of skill names to activate + exclude: null # optional denylist of skill names +``` + +`skills.paths`, when set, **replaces** the default roots. Later roots override +earlier ones by name. The per-entry description budget in the prompt listing is +a global setting: `execution.max_skill_desc_chars` (default 500). + +When any skill is available, the agent's toolkit automatically gains the +`use_skill` tool. + +## Invoking a skill + +There are two ways a skill is invoked: + +1. **Autonomously (the model decides).** The system prompt gains an + `AVAILABLE_SKILLS` block listing each model-invocable skill and instructing + the agent to call `use_skill` when a task matches. Calling + `use_skill ` returns that skill's body into the conversation + (progressive disclosure level 2). +2. **Explicitly (the user references it).** If a user message mentions a skill + by name with a leading slash — `/citation-style` — that skill's body is + injected into the prompt. This works in **every mode** (ACP, CLI, and the + OpenAI-compatible server), because the reference is expanded centrally in + `AgentFactory.create`. The reference may appear anywhere in the message + (`please use /concise-answer`); slashes inside URLs/paths are ignored. + +## Discovering skills as commands + +- **ACP** (`sgracp`): user-invocable skills are advertised to the client as + `available_commands`, so they appear in the client's slash-command menu. +- **CLI**: `sgrsh --list-skills -c config.yaml` prints the available skills. +- **HTTP server**: `GET /v1/skills` (optional `?model=`) lists skills per + agent. + +## Security & trust + +Skills are **trusted content**, on the same level as `config.yaml` and custom +tool code: a skill body is injected verbatim into the model context when +invoked. The default roots `./.agent/skills` and `~/.agent/skills` are scanned +automatically, so opening a repository that ships a `.agent/skills/` directory +will auto-load its authors' instructions. Only run skills from sources you +trust, and review third-party `SKILL.md` files before use. The loader caps each +`SKILL.md` at 1 MiB and skips unreadable files. + +## Authoring tips + +- Write the `description` in third person; include both *what it does* and *when + to use it* — this is what the model matches against. +- Keep `SKILL.md` focused (under ~500 lines); move long material into separate + reference files. +- Use gerund or noun-phrase names (`processing-pdfs`, `citation-style`); avoid + vague names like `helper` or `utils`. diff --git a/docs/ru/framework/skills.md b/docs/ru/framework/skills.md new file mode 100644 index 00000000..919f5398 --- /dev/null +++ b/docs/ru/framework/skills.md @@ -0,0 +1,128 @@ +# Навыки (Skills) + +Навыки — это переиспользуемые пакеты инструкций, загружаемые по необходимости +(модель *Agent Skills* от Anthropic). Каждый навык — это директория с файлом +`SKILL.md`: YAML-фронтматтер плюс тело в markdown. SGR Agent Core автоматически +регистрирует `name` + `description` навыка в системном промпте, чтобы агент мог +вызвать навык самостоятельно, а также показывает навыки как **команды** через +ACP, CLI и HTTP-сервер. + +## Как выглядит навык + +``` +skills/ + citation-style/ + SKILL.md + concise-answer/ + SKILL.md +``` + +`SKILL.md`: + +```markdown +--- +name: citation-style +description: Formats research citations in a consistent numbered style. Use when writing reports that reference web sources. +--- + +# Citation style + +1. Number every source in order: [1], [2], ... +2. Collect all sources under a `## Sources` heading. +``` + +### Фронтматтер + +| поле | обязательное | примечания | +|-----------------|--------------|---------------------------------------------------------------------| +| `name` | да | ≤ 64 символов; строчные буквы, цифры, дефисы; без `anthropic`/`claude` | +| `description` | да | непустое, ≤ 1024 символов; от третьего лица, *что* и *когда* | +| `license` | нет | SPDX-идентификатор или текст | +| `allowed-tools` | нет | рекомендательный список инструментов навыка | +| `metadata` | нет | произвольный словарь (версия, автор, ...) | +| `disable-model-invocation` | нет | `true` скрывает навык из каталога модели (только команда пользователя) | +| `user-invocable` | нет | `false` скрывает навык из меню команд (только для модели) | + +Если `name` не указан, берётся имя директории. + +## Прогрессивное раскрытие + +1. **Уровень 1 — метаданные**: `name` + `description` всегда попадают в системный + промпт (компактный блок каталога), чтобы модель знала о существовании навыка. +2. **Уровень 2 — тело**: тело `SKILL.md` загружается только при вызове навыка + (инструмент `use_skill` возвращает его в диалог). +3. **Уровень 3 — ресурсы**: вложенные файлы читаются только при необходимости. + +## Включение навыков + +Навыки работают из коробки: **по умолчанию** агент сканирует + +1. `./.agent/skills` (проектные, относительно текущего каталога, CWD) +2. `~/.agent/skills` (личные) + +Если этих папок нет или они пусты — ничего не ломается, агент работает без +навыков. Чтобы настроить, добавьте блок `skills` в агента (или глобально, на +верхнем уровне) в `config.yaml`: + +```yaml +agents: + sgr_agent: + base_class: SGRToolCallingAgent + tools: + - reasoningtool + skills: + enabled: true # по умолчанию true; false — отключить навыки + paths: # переопределяет корни по умолчанию (относительно CWD) + - ./my-skills + include: null # необязательный список активируемых имён + exclude: null # необязательный список запрещённых имён +``` + +`skills.paths`, если задан, **заменяет** корни по умолчанию. Последующие корни +переопределяют предыдущие по имени. Бюджет описания на запись в промпте — +глобальная настройка `execution.max_skill_desc_chars` (по умолчанию 500). + +Когда доступен хотя бы один навык, в набор инструментов агента автоматически +добавляется инструмент `use_skill`. + +## Вызов навыка + +Навык вызывается двумя способами: + +1. **Самостоятельно (решает модель).** В системный промпт добавляется блок + `AVAILABLE_SKILLS` со списком навыков, доступных модели, и указанием вызвать + `use_skill`, когда задача подходит под навык. Вызов `use_skill ` + возвращает тело навыка в диалог (уровень 2 progressive disclosure). +2. **Явно (ссылается пользователь).** Если в сообщении пользователя упомянут + навык по имени со слешем — `/citation-style` — тело этого навыка добавляется + в промт. Это работает во **всех режимах** (ACP, CLI и OpenAI-совместимый + сервер), потому что ссылка разворачивается централизованно в + `AgentFactory.create`. Ссылка может быть где угодно в сообщении + (`используй /concise-answer`); слеши внутри URL/путей игнорируются. + +## Навыки как команды (обнаружение) + +- **ACP** (`sgracp`): навыки, доступные пользователю, объявляются клиенту как + `available_commands` — они попадают в меню слэш-команд клиента. +- **CLI**: `sgrsh --list-skills -c config.yaml` печатает доступные навыки. +- **HTTP-сервер**: `GET /v1/skills` (опционально `?model=`) перечисляет + навыки по агентам. + +## Безопасность и доверие + +Навыки — это **доверенный контент**, наравне с `config.yaml` и кодом +пользовательских инструментов: тело навыка дословно попадает в контекст модели +при вызове. Корни по умолчанию `./.agent/skills` и `~/.agent/skills` +сканируются автоматически, поэтому открытие репозитория с директорией +`.agent/skills/` загрузит инструкции его авторов. Запускайте навыки только из +доверенных источников и проверяйте сторонние `SKILL.md` перед использованием. +Загрузчик ограничивает `SKILL.md` размером 1 МиБ и пропускает нечитаемые файлы. + +## Советы по написанию + +- Пишите `description` от третьего лица; указывайте *что делает* и *когда + применять* — именно по этому модель выбирает навык. +- Держите `SKILL.md` компактным (до ~500 строк); длинный материал выносите в + отдельные файлы. +- Используйте имена-герундии или существительные (`processing-pdfs`, + `citation-style`); избегайте расплывчатых `helper` или `utils`. diff --git a/examples/.agent/skills/citation-style/SKILL.md b/examples/.agent/skills/citation-style/SKILL.md new file mode 100644 index 00000000..f396bbe1 --- /dev/null +++ b/examples/.agent/skills/citation-style/SKILL.md @@ -0,0 +1,26 @@ +--- +name: citation-style +description: Formats research citations in a consistent numbered style. Use when writing reports or answers that reference web sources, or when the user asks for citations, references, or a bibliography. +metadata: + version: '1.0' +--- + +# Citation style + +Apply this style whenever the answer cites external sources. + +## Rules + +1. Number every source in the order it is first referenced: `[1]`, `[2]`, ... +2. Place the marker immediately after the sentence it supports. +3. Collect all sources under a `## Sources` heading at the end. +4. Each source line: `[N] Title — URL`. +5. Never invent URLs; only cite sources actually retrieved. + +## Example + +The framework uses schema-guided reasoning \[1\]. + +## Sources + +\[1\] SGR Agent Core — https://github.com/vamplabAI/sgr-agent-core diff --git a/examples/.agent/skills/concise-answer/SKILL.md b/examples/.agent/skills/concise-answer/SKILL.md new file mode 100644 index 00000000..dcb78bc9 --- /dev/null +++ b/examples/.agent/skills/concise-answer/SKILL.md @@ -0,0 +1,20 @@ +--- +name: concise-answer +description: Produces a tight, structured final answer. Use when the user asks for a brief, summary, TL;DR, or bullet-point response, or when a long analysis should be condensed. +--- + +# Concise answer + +When this skill is active, shape the final answer as follows. + +## Structure + +1. One-sentence direct answer first. +2. Up to five supporting bullets, each a single line. +3. No filler, hedging, or restating the question. + +## Checklist + +- Lead with the conclusion. +- Prefer numbers and concrete nouns over adjectives. +- Stop when the question is answered. diff --git a/mkdocs.yml b/mkdocs.yml index df650c7c..680a1362 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -32,6 +32,7 @@ plugins: Configuration: Конфигурация Langfuse: Langfuse Build your agent: Собери своего агента + Skills: Навыки Workflow: Рабочий процесс Demonstration: Демонстрация Integration & Examples: Интеграция и примеры @@ -87,6 +88,7 @@ nav: - Langfuse: framework/langfuse.md - Build your agent: framework/agents.md - Tools: framework/tools.md + - Skills: framework/skills.md - QnA: framework/qna.md - SGR API Service: - API Server Quick Start: sgr-api/SGR-Quick-Start.md diff --git a/sgr_agent_core/__init__.py b/sgr_agent_core/__init__.py index 9f720f43..2c90def9 100644 --- a/sgr_agent_core/__init__.py +++ b/sgr_agent_core/__init__.py @@ -35,6 +35,14 @@ PromptLoader, ToolRegistry, ) +from sgr_agent_core.skills import ( + BaseSkill, + SkillError, + SkillLoader, + SkillMetadata, + SkillRegistry, + SkillsConfig, +) from sgr_agent_core.tools import * # noqa: F403 __all__ = [ @@ -69,4 +77,11 @@ "NextStepToolsBuilder", # Factory "AgentFactory", + # Skills + "BaseSkill", + "SkillMetadata", + "SkillLoader", + "SkillRegistry", + "SkillsConfig", + "SkillError", ] diff --git a/sgr_agent_core/acp/bridge.py b/sgr_agent_core/acp/bridge.py index a3f1088d..78ea747a 100644 --- a/sgr_agent_core/acp/bridge.py +++ b/sgr_agent_core/acp/bridge.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import logging import uuid from dataclasses import dataclass from typing import Any @@ -11,6 +12,9 @@ from acp.schema import ( AgentCapabilities, AuthenticateResponse, + AvailableCommand, + AvailableCommandInput, + AvailableCommandsUpdate, CloseSessionResponse, HttpMcpServer, Implementation, @@ -29,6 +33,7 @@ SetSessionModeResponse, SseMcpServer, TextContentBlock, + UnstructuredCommandInput, ) from sgr_agent_core import __version__ @@ -38,6 +43,9 @@ from sgr_agent_core.agent_factory import AgentFactory from sgr_agent_core.base_agent import BaseAgent from sgr_agent_core.models import AgentStatesEnum +from sgr_agent_core.skills.models import BaseSkill + +logger = logging.getLogger(__name__) def extract_prompt_text( @@ -110,6 +118,52 @@ def _agent_definition_for_session(self, sess: _ACPSession) -> AgentDefinition: agent_def.llm.model = sess.model return agent_def + def _skills_for_session(self, sess: _ACPSession) -> list[BaseSkill]: + """Resolve the skills available for the session's agent definition.""" + try: + return AgentFactory._resolve_skills(self._agent_definition_for_session(sess)) + except Exception: # noqa: BLE001 - never let skill resolution break a session + return [] + + def _build_available_commands(self, skills: list[BaseSkill]) -> list[AvailableCommand]: + """Map user-invocable skills to ACP available commands (slash + commands).""" + commands: list[AvailableCommand] = [] + for skill in skills: + if not skill.metadata.user_invocable: + continue + commands.append( + AvailableCommand( + name=skill.name, + description=skill.description, + input=AvailableCommandInput(root=UnstructuredCommandInput(hint="optional arguments")), + ) + ) + return commands + + async def _advertise_commands(self, sess: _ACPSession) -> None: + """Push the session's skill commands to the client (best effort). + + Advertising commands must never break session creation or an + agent switch, so client/transport errors are swallowed and + logged. + """ + if self._client is None: + return + commands = self._build_available_commands(self._skills_for_session(sess)) + if not commands: + return + try: + await self._client.session_update( + sess.session_id, + AvailableCommandsUpdate( + available_commands=commands, + session_update="available_commands_update", + ), + ) + except Exception as exc: # noqa: BLE001 - advertising is best effort + logger.warning("Failed to advertise skill commands for %s: %s", sess.session_id, exc) + def _agent_names(self) -> list[str]: return list(GlobalConfig().agents.keys()) @@ -207,6 +261,7 @@ async def new_session( model = GlobalConfig().agents[agent_name].llm.model sess = _ACPSession(session_id=session_id, cwd=cwd, agent_name=agent_name, model=model) self._sessions[session_id] = sess + await self._advertise_commands(sess) return NewSessionResponse(session_id=session_id, config_options=self._build_config_options(sess)) async def load_session( @@ -256,6 +311,8 @@ async def set_config_option( raise ValueError(f"Unknown agent config value: {value!r}") sess.agent_name = value self._reset_agent_if_idle(sess) + # Skills are per-agent, so refresh the advertised commands. + await self._advertise_commands(sess) elif config_id == self._MODEL_CONFIG_ID: if not isinstance(value, str) or value not in self._model_choices(): raise ValueError(f"Unknown model config value: {value!r}") @@ -301,6 +358,7 @@ async def prompt( sess.agent = None sess.execute_task = None + # "/skill-name" references in `text` are expanded centrally by AgentFactory. agent_def = self._agent_definition_for_session(sess) gen_cls = create_acp_streaming_generator_class(session_id, self._client) agent = await AgentFactory.create( diff --git a/sgr_agent_core/agent_definition.py b/sgr_agent_core/agent_definition.py index a3659a5b..20847925 100644 --- a/sgr_agent_core/agent_definition.py +++ b/sgr_agent_core/agent_definition.py @@ -10,6 +10,8 @@ from fastmcp.mcp_config import MCPConfig from pydantic import BaseModel, Field, FilePath, ImportString, computed_field, field_validator, model_validator +from sgr_agent_core.skills.config import SkillsConfig + logger = logging.getLogger(__name__) @@ -132,6 +134,9 @@ class ExecutionConfig(BaseModel, extra="allow"): max_clarifications: int = Field(default=3, ge=0, description="Maximum number of clarifications") max_iterations: int = Field(default=10, gt=0, description="Maximum number of iterations") mcp_context_limit: int = Field(default=15000, gt=0, description="Maximum context length from MCP server response") + max_skill_desc_chars: int = Field( + default=500, gt=0, description="Per-entry skill description cap in the system-prompt listing" + ) streaming_generator: Literal["openai", "open_webui"] = Field( default="openai", @@ -183,6 +188,7 @@ class AgentConfig(BaseModel, extra="allow"): execution: ExecutionConfig = Field(default_factory=ExecutionConfig, description="Execution settings") prompts: PromptsConfig = Field(default_factory=PromptsConfig, description="Prompts settings") mcp: MCPConfig = Field(default_factory=MCPConfig, description="MCP settings") + skills: SkillsConfig | None = Field(default=None, description="Skills discovery and rendering settings") @field_validator("langfuse", mode="before") @classmethod diff --git a/sgr_agent_core/agent_factory.py b/sgr_agent_core/agent_factory.py index 18060e9f..13784b4d 100644 --- a/sgr_agent_core/agent_factory.py +++ b/sgr_agent_core/agent_factory.py @@ -3,7 +3,8 @@ import importlib import logging from importlib import import_module -from typing import Any, Type, TypeVar +from pathlib import Path +from typing import TYPE_CHECKING, Any, Type, TypeVar import httpx from openai import AsyncOpenAI @@ -14,7 +15,12 @@ from sgr_agent_core.base_agent import BaseAgent from sgr_agent_core.base_tool import BaseTool from sgr_agent_core.services import AgentRegistry, MCP2ToolConverter, StreamingGeneratorRegistry +from sgr_agent_core.skills import SkillRegistry, SkillsConfig, inject_referenced_skills from sgr_agent_core.stream import BaseStreamingGenerator, OpenAIStreamingGenerator +from sgr_agent_core.tools.skill_tool import SkillTool + +if TYPE_CHECKING: + from sgr_agent_core.skills import BaseSkill logger = logging.getLogger(__name__) @@ -109,6 +115,40 @@ def _resolve_tools_with_configs( tool_configs[tool_class.tool_name] = tool_def.tool_kwargs() return toolkit, tool_configs + @classmethod + def _default_skill_roots(cls, skills_config: SkillsConfig) -> list[Path]: + """Resolve skill root directories to scan. + + When ``skills.paths`` is set it overrides the defaults (relative paths + resolved against the current working directory). Otherwise the defaults + are ``./.agent/skills`` (project, relative to CWD) and + ``~/.agent/skills`` (personal). Missing/empty roots are simply skipped. + """ + if skills_config.paths: + return [p if (p := Path(raw)).is_absolute() else Path.cwd() / p for raw in skills_config.paths] + return [Path.cwd() / ".agent" / "skills", Path.home() / ".agent" / "skills"] + + @classmethod + def _resolve_skills(cls, agent_def: AgentDefinition) -> list["BaseSkill"]: + """Discover and filter skills for an agent. + + Skills are scanned from the default roots even without a ``skills:`` + block; a ``skills:`` config overrides the roots and activation filters. + """ + skills_config = getattr(agent_def, "skills", None) or SkillsConfig() + if not skills_config.enabled: + return [] + registry = SkillRegistry() + registry.load_from_paths(cls._default_skill_roots(skills_config)) + skills = registry.list_items() + # An empty include list is treated as "no allowlist filter" (symmetric + # with exclude), so it does not silently drop every skill. + if skills_config.include: + skills = [s for s in skills if s.name in skills_config.include] + if skills_config.exclude: + skills = [s for s in skills if s.name not in skills_config.exclude] + return skills + @classmethod async def create( cls, @@ -172,12 +212,21 @@ async def create( tools, tool_configs = cls._resolve_tools_with_configs(agent_def.tools) tools.extend(mcp_tools) + # Resolve skills and expose the use_skill tool when any are available. + skills = cls._resolve_skills(agent_def) + if skills and SkillTool not in tools: + tools.append(SkillTool) + # Inject bodies of skills the user referenced as "/skill-name" in the prompt. + task_messages = inject_referenced_skills(task_messages, skills) + try: # Extract agent-specific parameters from agent_def (e.g., working_directory) # These are fields that are not part of standard AgentConfig but are allowed via extra="allow" agent_kwargs = {} for key, value in agent_def.model_dump().items(): agent_kwargs[key] = value + # 'skills' is resolved into BaseSkill objects below; drop the raw config dump. + agent_kwargs.pop("skills", None) gen_cls = streaming_generator or cls._resolve_streaming_generator(agent_def.execution.streaming_generator) agent = BaseClass( @@ -188,6 +237,7 @@ async def create( openai_client=cls._create_client(agent_def.llm), agent_config=agent_def, streaming_generator=gen_cls, + skills=skills, **agent_kwargs, ) logger.info( diff --git a/sgr_agent_core/base_agent.py b/sgr_agent_core/base_agent.py index dd79e6a4..6c3f0363 100644 --- a/sgr_agent_core/base_agent.py +++ b/sgr_agent_core/base_agent.py @@ -5,7 +5,7 @@ import traceback import uuid from datetime import datetime -from typing import Any, Type +from typing import TYPE_CHECKING, Any, Type from openai import AsyncOpenAI, pydantic_function_tool from openai.types.chat import ChatCompletionFunctionToolParam, ChatCompletionMessageParam @@ -22,6 +22,9 @@ ReasoningTool, ) +if TYPE_CHECKING: + from sgr_agent_core.skills import BaseSkill + class AgentRegistryMixin: def __init_subclass__(cls, **kwargs): @@ -44,6 +47,7 @@ def __init__( def_name: str | None = None, streaming_generator: type[BaseStreamingGenerator] = OpenAIStreamingGenerator, tool_configs: dict[str, ToolDefinition] | None = None, + skills: list["BaseSkill"] = (), **kwargs: dict, ): self.id = f"{def_name or self.name}_{uuid.uuid4()}" @@ -55,8 +59,9 @@ def __init__( self.task_messages = task_messages self.toolkit = toolkit self.tool_configs = tool_configs or {} + self.available_skills: list[BaseSkill] = list(skills or []) - self._context = AgentContext() + self._context = AgentContext(available_skills=self.available_skills) self.conversation = [] self.logger = logging.getLogger(f"sgr_agent_core.agents.{self.id}") self.log = [] @@ -182,7 +187,14 @@ async def _prepare_context(self) -> list[dict]: """ return [ - {"role": "system", "content": PromptLoader.get_system_prompt(self.toolkit, self.config.prompts)}, + { + "role": "system", + "content": PromptLoader.get_system_prompt( + self.toolkit, + self.config.prompts, + available_skills=self.available_skills, + ), + }, *self.task_messages, {"role": "user", "content": PromptLoader.get_initial_user_request(self.task_messages, self.config.prompts)}, *self.conversation, diff --git a/sgr_agent_core/cli/sgrsh.py b/sgr_agent_core/cli/sgrsh.py index 770b5b17..89dff0a7 100644 --- a/sgr_agent_core/cli/sgrsh.py +++ b/sgr_agent_core/cli/sgrsh.py @@ -21,10 +21,27 @@ if TYPE_CHECKING: from sgr_agent_core.base_agent import BaseAgent + from sgr_agent_core.skills import BaseSkill logger = logging.getLogger(__name__) +def format_skills_listing(skills: "list[BaseSkill]") -> str: + """Format a human-readable listing of skills for the CLI --list-skills + flag. + + Only user-invocable skills are shown as ``/commands`` because those are the + ones the CLI expands when typed. + """ + invocable = [s for s in skills if s.metadata.user_invocable] + if not invocable: + return "No skills available for this agent." + lines = ["Available skills:"] + for skill in invocable: + lines.append(f" /{skill.name} — {skill.description}") + return "\n".join(lines) + + def _read_user_input(prompt: str) -> str: """Read user input from buffer and decode as UTF-8 to avoid losing input on decode errors. @@ -149,6 +166,7 @@ async def chat_loop(agent_def_name: str, config: GlobalConfig): if not user_input: continue + # "/skill-name" references are expanded centrally by AgentFactory. conversation_history.append({"role": "user", "content": user_input}) agent = await AgentFactory.create(agent_def, task_messages=conversation_history) result = await run_agent(agent) @@ -192,6 +210,11 @@ async def main(): default=None, help="Agent name to use (default: first agent in config)", ) + parser.add_argument( + "--list-skills", + action="store_true", + help="List skills available to the selected agent and exit", + ) parser.add_argument( "query", nargs="*", @@ -235,6 +258,16 @@ async def main(): print(f"ℹ️ Using agent: {agent_name}") print(f" Available agents: {', '.join(config.agents.keys())}") + # List skills and exit + if args.list_skills: + agent_def = config.agents.get(agent_name) + if agent_def is None: + print(f"❌ Agent '{agent_name}' not found in config") + sys.exit(1) + skills = AgentFactory._resolve_skills(agent_def) + print(format_skills_listing(skills)) + return + # Check if query provided query = " ".join(args.query) if args.query else None @@ -246,7 +279,7 @@ async def main(): print(f"Available agents: {', '.join(config.agents.keys())}") sys.exit(1) - # Create agent + # "/skill-name" references are expanded centrally by AgentFactory. task_messages = [{"role": "user", "content": query}] agent = await AgentFactory.create(agent_def, task_messages) diff --git a/sgr_agent_core/models.py b/sgr_agent_core/models.py index 39539044..e2904353 100644 --- a/sgr_agent_core/models.py +++ b/sgr_agent_core/models.py @@ -5,6 +5,8 @@ from pydantic import BaseModel, Field +from sgr_agent_core.skills.models import BaseSkill + class SourceData(BaseModel): """Data about a research source.""" @@ -67,8 +69,12 @@ class AgentContext(BaseModel): default=None, description="Custom context for project-specific data" ) + available_skills: list[BaseSkill] = Field( + default_factory=list, description="Skills available to the agent this run" + ) + def agent_state(self) -> dict: - return self.model_dump(exclude={"searches", "sources", "clarification_received"}) + return self.model_dump(exclude={"searches", "sources", "clarification_received", "available_skills"}) class AgentStatistics(BaseModel): diff --git a/sgr_agent_core/prompts/research_system_prompt.txt b/sgr_agent_core/prompts/research_system_prompt.txt index ac98555e..0608abda 100644 --- a/sgr_agent_core/prompts/research_system_prompt.txt +++ b/sgr_agent_core/prompts/research_system_prompt.txt @@ -63,3 +63,5 @@ When working with specific dates, numbers, versions, names, or other precise inf {available_tools} + +{available_skills} diff --git a/sgr_agent_core/prompts/system_prompt.txt b/sgr_agent_core/prompts/system_prompt.txt index 035024b5..9df91fa7 100644 --- a/sgr_agent_core/prompts/system_prompt.txt +++ b/sgr_agent_core/prompts/system_prompt.txt @@ -43,3 +43,5 @@ When answering questions about specific dates, numbers, versions, or names: : {available_tools} + +{available_skills} diff --git a/sgr_agent_core/server/endpoints.py b/sgr_agent_core/server/endpoints.py index 170fb7c7..3ee2d546 100644 --- a/sgr_agent_core/server/endpoints.py +++ b/sgr_agent_core/server/endpoints.py @@ -41,7 +41,7 @@ async def get_agent_state(agent_id: str): agent_id=agent.id, task_messages=agent.task_messages, sources_count=len(agent._context.sources), - **agent._context.model_dump(), + **agent._context.model_dump(exclude={"available_skills"}), ) @@ -150,6 +150,26 @@ async def get_available_models(): return {"data": models_data, "object": "list"} +@router.get("/v1/skills") +async def get_available_skills(model: str | None = None): + """List skills available per agent model (optionally filtered by model).""" + definitions = AgentFactory.get_definitions_list() + if model is not None: + definitions = [d for d in definitions if d.name == model] + + data = [ + { + "model": agent_def.name, + "skills": [ + {"name": skill.name, "description": skill.description} + for skill in AgentFactory._resolve_skills(agent_def) + ], + } + for agent_def in definitions + ] + return {"data": data, "object": "list"} + + @router.post("/agents/{agent_id}/provide_clarification") async def provide_clarification( request: MessagesRequest, diff --git a/sgr_agent_core/services/overlayfs_manager.py b/sgr_agent_core/services/overlayfs_manager.py index be9af146..fedc6c65 100644 --- a/sgr_agent_core/services/overlayfs_manager.py +++ b/sgr_agent_core/services/overlayfs_manager.py @@ -188,7 +188,7 @@ async def initialize_from_config(cls) -> None: if not tool_config: if candidates: logger.warning( - "RunCommandTool is configured only with unsafe mode; " "OverlayFS will not be initialized.", + "RunCommandTool is configured only with unsafe mode; OverlayFS will not be initialized.", ) return diff --git a/sgr_agent_core/services/prompt_loader.py b/sgr_agent_core/services/prompt_loader.py index 1a4eb63b..c06544ae 100644 --- a/sgr_agent_core/services/prompt_loader.py +++ b/sgr_agent_core/services/prompt_loader.py @@ -3,25 +3,41 @@ from openai.types.chat import ChatCompletionMessageParam +from sgr_agent_core.skills.rendering import render_available_skills + if TYPE_CHECKING: from sgr_agent_core import BaseTool, PromptsConfig + from sgr_agent_core.skills import BaseSkill class PromptLoader: @classmethod - def get_system_prompt(cls, available_tools: list[type["BaseTool"]], prompts_config: "PromptsConfig") -> str: + def get_system_prompt( + cls, + available_tools: list[type["BaseTool"]], + prompts_config: "PromptsConfig", + available_skills: "list[BaseSkill]" = (), + ) -> str: template = prompts_config.system_prompt available_tools_str_list = [ f"{i}. {tool.tool_name}: {tool.description}" for i, tool in enumerate(available_tools, start=1) ] + skills_block = render_available_skills(available_skills or []) try: - return template.format( + rendered = template.format( available_tools="\n".join(available_tools_str_list), + available_skills=skills_block, ) except KeyError as e: raise KeyError(f"Missing placeholder in system prompt template: {e}") from e + # If a custom template omits {available_skills}, append the catalog so the + # agent still sees skills it can invoke via use_skill (autonomous discovery). + if skills_block and "{available_skills}" not in template: + rendered = f"{rendered}\n\n{skills_block}" + return rendered + @classmethod def get_initial_user_request( cls, diff --git a/sgr_agent_core/skills/__init__.py b/sgr_agent_core/skills/__init__.py new file mode 100644 index 00000000..68e9d26f --- /dev/null +++ b/sgr_agent_core/skills/__init__.py @@ -0,0 +1,32 @@ +"""Agent Skills subsystem for SGR Agent Core. + +Skills are directories containing a ``SKILL.md`` file (YAML frontmatter plus a +markdown body). They are auto-registered into the agent system prompt (name + +description) so the agent can invoke them autonomously, and can be surfaced as +user commands over ACP. +""" + +from sgr_agent_core.skills.commands import ( + find_referenced_skills, + inject_referenced_skills, + render_skill_body, +) +from sgr_agent_core.skills.config import SkillsConfig +from sgr_agent_core.skills.loader import SKILL_FILE, SkillLoader +from sgr_agent_core.skills.models import BaseSkill, SkillError, SkillMetadata +from sgr_agent_core.skills.registry import SkillRegistry +from sgr_agent_core.skills.rendering import render_available_skills + +__all__ = [ + "BaseSkill", + "SkillError", + "SkillMetadata", + "SkillLoader", + "SkillRegistry", + "SkillsConfig", + "render_available_skills", + "find_referenced_skills", + "inject_referenced_skills", + "render_skill_body", + "SKILL_FILE", +] diff --git a/sgr_agent_core/skills/commands.py b/sgr_agent_core/skills/commands.py new file mode 100644 index 00000000..b485e5e8 --- /dev/null +++ b/sgr_agent_core/skills/commands.py @@ -0,0 +1,86 @@ +"""Detect ``/skill-name`` references in user messages and inject skill bodies. + +This is the explicit, user-driven counterpart to autonomous invocation via the +``use_skill`` tool: if a user message references a skill by name with a leading +slash (e.g. ``/citation-style``), that skill's full body is injected into the +prompt so the agent follows it. Applied centrally in ``AgentFactory.create`` so +every entrypoint (ACP, CLI, OpenAI server) behaves the same. +""" + +from __future__ import annotations + +import re +from collections.abc import Sequence +from typing import Any + +from sgr_agent_core.skills.models import BaseSkill + +# A reference is a "/name" token preceded by start-of-string, whitespace, or a +# common opening delimiter (quote, backtick, paren, bracket, asterisk, brace), +# and NOT followed by another "/". The possessive quantifier + "(?!/)" reject +# filesystem paths like "/etc/hosts"; the lookbehind rejects URLs like +# "http://x/greet" and inline "a/greet". +SKILL_REF_RE = re.compile(r"""(? wrapper (skills are trusted content, but be defensive). +_CLOSING_TAG_RE = re.compile(r"", re.IGNORECASE) + + +def render_skill_body(skill: BaseSkill) -> str: + """Wrap a skill's body in a delimited block for injection into the + prompt.""" + body = skill.body.strip() or "(this skill has no additional instructions)" + body = _CLOSING_TAG_RE.sub("< /SKILL>", body) + return f'\n{body}\n' + + +def _message_text(content: Any) -> str: + """Extract user-visible text from a message ``content`` (str or parts + list).""" + if isinstance(content, str): + return content + if isinstance(content, list): + parts: list[str] = [] + for part in content: + if isinstance(part, str): + parts.append(part) + elif isinstance(part, dict) and part.get("type") == "text": + parts.append(str(part.get("text", ""))) + return " ".join(parts) + return "" + + +def find_referenced_skills(text: str, skills: Sequence[BaseSkill]) -> list[BaseSkill]: + """Return user-invocable skills referenced as ``/name`` in ``text`` (in + order).""" + invocable = {s.name: s for s in skills if s.metadata.user_invocable} + seen: set[str] = set() + found: list[BaseSkill] = [] + for match in SKILL_REF_RE.finditer(text or ""): + name = match.group(1) + if name in invocable and name not in seen: + seen.add(name) + found.append(invocable[name]) + return found + + +def inject_referenced_skills(task_messages: list[dict], skills: Sequence[BaseSkill]) -> list[dict]: + """Inject bodies of skills referenced in the last user message. + + Scans the last ``user`` message for ``/skill-name`` references and, for each + match, inserts a user message with the skill body right before it. Returns a + new list; the input is left unmodified. No references (or no skills) returns + the original list unchanged. + """ + if not skills or not task_messages: + return task_messages + idx = next((i for i in range(len(task_messages) - 1, -1, -1) if task_messages[i].get("role") == "user"), None) + if idx is None: + return task_messages + referenced = find_referenced_skills(_message_text(task_messages[idx].get("content")), skills) + if not referenced: + return task_messages + block = "\n\n".join(render_skill_body(s) for s in referenced) + new_messages = list(task_messages) + new_messages.insert(idx, {"role": "user", "content": block}) + return new_messages diff --git a/sgr_agent_core/skills/config.py b/sgr_agent_core/skills/config.py new file mode 100644 index 00000000..efa85fee --- /dev/null +++ b/sgr_agent_core/skills/config.py @@ -0,0 +1,22 @@ +"""Configuration model for the skills subsystem.""" + +from __future__ import annotations + +from pydantic import BaseModel, Field + + +class SkillsConfig(BaseModel): + """Agent/global configuration for skill discovery and activation. + + Attributes: + enabled: Master switch; when False no skills are loaded or injected. + paths: Skill root directories to scan. When set, these replace the + default roots; otherwise the default roots are used. + include: If set, only skills whose name is in this list are activated. + exclude: Skills whose name is in this list are dropped. + """ + + enabled: bool = True + paths: list[str] = Field(default_factory=list) + include: list[str] | None = None + exclude: list[str] | None = None diff --git a/sgr_agent_core/skills/loader.py b/sgr_agent_core/skills/loader.py new file mode 100644 index 00000000..811d02a6 --- /dev/null +++ b/sgr_agent_core/skills/loader.py @@ -0,0 +1,148 @@ +"""Loader that parses ``SKILL.md`` files and discovers skills on disk.""" + +from __future__ import annotations + +import logging +from pathlib import Path + +import yaml +from pydantic import ValidationError + +from sgr_agent_core.skills.models import BaseSkill, SkillError, SkillMetadata + +logger = logging.getLogger(__name__) + +SKILL_FILE = "SKILL.md" +# Guardrail: skip pathologically large SKILL.md files (they are injected into +# the model context on invocation). 1 MiB is far above any reasonable skill. +MAX_SKILL_FILE_BYTES = 1024 * 1024 + + +class SkillLoader: + """Parse and discover skills from the filesystem. + + A skill lives in its own directory that contains a ``SKILL.md`` file with + YAML frontmatter and a markdown body (progressive disclosure level 2). + """ + + @staticmethod + def _split_frontmatter(text: str) -> tuple[dict, str]: + """Split raw ``SKILL.md`` text into a frontmatter dict and a body. + + Args: + text: Full ``SKILL.md`` content. + + Returns: + Tuple of (frontmatter mapping, markdown body). + + Raises: + SkillError: If the leading YAML frontmatter block is missing or + unterminated, or is not a mapping. + """ + stripped = text.lstrip("") + if not stripped.startswith("---"): + raise SkillError("SKILL.md must start with a YAML frontmatter block delimited by '---'") + + lines = stripped.splitlines() + end_index: int | None = None + for i in range(1, len(lines)): + if lines[i].strip() == "---": + end_index = i + break + if end_index is None: + raise SkillError("SKILL.md frontmatter block is not terminated by '---'") + + frontmatter_text = "\n".join(lines[1:end_index]) + body = "\n".join(lines[end_index + 1 :]).strip() + try: + data = yaml.safe_load(frontmatter_text) or {} + except yaml.YAMLError as exc: + raise SkillError(f"Invalid YAML in SKILL.md frontmatter: {exc}") from exc + if not isinstance(data, dict): + raise SkillError("SKILL.md frontmatter must be a YAML mapping") + return data, body + + @classmethod + def parse(cls, text: str, *, path: Path | None = None, default_name: str | None = None) -> BaseSkill: + """Parse ``SKILL.md`` text into a validated :class:`BaseSkill`. + + Args: + text: Full ``SKILL.md`` content. + path: Optional source path stored on the skill. + default_name: Name to use when frontmatter omits ``name`` + (typically the skill directory name). + + Returns: + A validated :class:`BaseSkill`. + + Raises: + SkillError: If frontmatter is missing, malformed, or fails validation. + """ + data, body = cls._split_frontmatter(text) + if "name" not in data and default_name is not None: + data["name"] = default_name + try: + metadata = SkillMetadata.model_validate(data) + except ValidationError as exc: + raise SkillError(f"Invalid skill metadata: {exc}") from exc + return BaseSkill(metadata=metadata, body=body, path=path) + + @classmethod + def load_skill(cls, directory: Path) -> BaseSkill: + """Load a single skill from a directory containing ``SKILL.md``. + + Args: + directory: Directory holding the ``SKILL.md`` file. + + Returns: + The loaded :class:`BaseSkill`. + + Raises: + SkillError: If ``SKILL.md`` is missing or invalid. + """ + directory = Path(directory) + skill_file = directory / SKILL_FILE + if not skill_file.is_file(): + raise SkillError(f"No {SKILL_FILE} found in {directory}") + try: + if skill_file.stat().st_size > MAX_SKILL_FILE_BYTES: + raise SkillError(f"{skill_file} exceeds the maximum skill size of {MAX_SKILL_FILE_BYTES} bytes") + text = skill_file.read_text(encoding="utf-8") + except OSError as exc: + raise SkillError(f"Cannot read {skill_file}: {exc}") from exc + except UnicodeDecodeError as exc: + raise SkillError(f"{skill_file} is not valid UTF-8: {exc}") from exc + return cls.parse(text, path=directory, default_name=directory.name) + + @classmethod + def discover(cls, root: Path) -> list[BaseSkill]: + """Discover all skills in immediate subdirectories of ``root``. + + Malformed skills are logged and skipped so one bad skill never breaks + discovery of the rest. + + Args: + root: Directory whose immediate subdirectories may be skills. + + Returns: + List of successfully loaded skills, sorted by name. + """ + root = Path(root) + if not root.is_dir(): + return [] + skills: list[BaseSkill] = [] + try: + entries = sorted(root.iterdir()) + except OSError as exc: + logger.warning("Cannot list skill root %s: %s", root, exc) + return [] + for entry in entries: + if not entry.is_dir(): + continue + if not (entry / SKILL_FILE).is_file(): + continue + try: + skills.append(cls.load_skill(entry)) + except SkillError as exc: + logger.warning("Skipping malformed skill in %s: %s", entry, exc) + return skills diff --git a/sgr_agent_core/skills/models.py b/sgr_agent_core/skills/models.py new file mode 100644 index 00000000..df07ffd0 --- /dev/null +++ b/sgr_agent_core/skills/models.py @@ -0,0 +1,115 @@ +"""Data model for Agent Skills (Anthropic SKILL.md model). + +A skill is a directory containing a ``SKILL.md`` file with YAML frontmatter +(``name`` + ``description`` required) and a markdown body. Frontmatter is +validated against the Anthropic Agent Skills spec rules. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator + +NAME_MAX_LENGTH = 64 +DESCRIPTION_MAX_LENGTH = 1024 +NAME_PATTERN = re.compile(r"^[a-z0-9-]+$") +RESERVED_WORDS = ("anthropic", "claude") + + +class SkillError(Exception): + """Raised when a skill cannot be parsed, validated, or loaded.""" + + +class SkillMetadata(BaseModel): + """Validated frontmatter of a ``SKILL.md`` file. + + Enforces the Anthropic Agent Skills spec: ``name`` is lowercase letters, + digits and hyphens up to 64 chars (no XML tags, no reserved words) and + ``description`` is a non-empty string up to 1024 chars with no XML tags. + """ + + model_config = ConfigDict(populate_by_name=True) + + name: str + description: str + license: str | None = None + allowed_tools: list[str] = Field(default_factory=list, alias="allowed-tools") + metadata: dict = Field(default_factory=dict) + # Invocation gating (mirrors codex/Claude disable-model-invocation / user-invocable). + model_invocable: bool = True + user_invocable: bool = True + + @staticmethod + def _coerce_bool(value: object) -> bool: + """Coerce a YAML scalar to bool, honoring quoted strings like + "false".""" + if isinstance(value, str): + return value.strip().lower() in ("true", "1", "yes", "on") + return bool(value) + + @model_validator(mode="before") + @classmethod + def _map_invocation_aliases(cls, data: object) -> object: + """Translate Claude-style ``disable-model-invocation`` / ``user- + invocable`` frontmatter into the internal ``model_invocable`` / + ``user_invocable`` flags.""" + if isinstance(data, dict): + data = dict(data) + if "disable-model-invocation" in data: + data["model_invocable"] = not cls._coerce_bool(data.pop("disable-model-invocation")) + if "user-invocable" in data: + data["user_invocable"] = cls._coerce_bool(data.pop("user-invocable")) + return data + + @field_validator("name") + @classmethod + def _validate_name(cls, value: str) -> str: + if not value: + raise ValueError("Skill 'name' must be non-empty") + if len(value) > NAME_MAX_LENGTH: + raise ValueError(f"Skill 'name' must be at most {NAME_MAX_LENGTH} characters") + if "<" in value or ">" in value: + raise ValueError("Skill 'name' cannot contain XML tags") + if not NAME_PATTERN.match(value): + raise ValueError("Skill 'name' must contain only lowercase letters, digits and hyphens") + lowered = value.lower() + for word in RESERVED_WORDS: + if word in lowered: + raise ValueError(f"Skill 'name' cannot contain the reserved word '{word}'") + return value + + @field_validator("description") + @classmethod + def _validate_description(cls, value: str) -> str: + if not value or not value.strip(): + raise ValueError("Skill 'description' must be non-empty") + if len(value) > DESCRIPTION_MAX_LENGTH: + raise ValueError(f"Skill 'description' must be at most {DESCRIPTION_MAX_LENGTH} characters") + if "<" in value or ">" in value: + raise ValueError("Skill 'description' cannot contain XML tags") + return value + + +class BaseSkill(BaseModel): + """A loaded skill: validated metadata plus the markdown body and source path.""" + + metadata: SkillMetadata + body: str = "" + path: Path | None = None + + @property + def name(self) -> str: + """BaseSkill name (from metadata).""" + return self.metadata.name + + @property + def description(self) -> str: + """BaseSkill description (from metadata).""" + return self.metadata.description + + @property + def allowed_tools(self) -> list[str]: + """Advisory list of tool names the skill uses.""" + return self.metadata.allowed_tools diff --git a/sgr_agent_core/skills/registry.py b/sgr_agent_core/skills/registry.py new file mode 100644 index 00000000..c3b7aba1 --- /dev/null +++ b/sgr_agent_core/skills/registry.py @@ -0,0 +1,68 @@ +"""In-memory catalog of loaded skills. + +Unlike ``ToolRegistry`` (populated via ``__init_subclass__`` at import time), +skills are *data* loaded from the filesystem, so ``SkillRegistry`` is an +instance-based catalog populated by scanning skill roots. +""" + +from __future__ import annotations + +from collections.abc import Iterable +from pathlib import Path + +from sgr_agent_core.skills.loader import SkillLoader +from sgr_agent_core.skills.models import BaseSkill + + +class SkillRegistry: + """Ordered, name-keyed catalog of :class:`BaseSkill` objects. + + Registering a skill with an existing name overrides the previous + one, so later skill roots take precedence over earlier ones + (personal over builtin, project over personal, explicit paths last). + """ + + def __init__(self, skills: Iterable[BaseSkill] | None = None) -> None: + self._skills: dict[str, BaseSkill] = {} + if skills: + for skill in skills: + self.register(skill) + + def register(self, skill: BaseSkill) -> None: + """Add or replace a skill by name.""" + self._skills[skill.name] = skill + + def get(self, name: str) -> BaseSkill | None: + """Return the skill with ``name`` or ``None`` if not registered.""" + return self._skills.get(name) + + def names(self) -> list[str]: + """Return all registered skill names, sorted.""" + return sorted(self._skills.keys()) + + def list_items(self) -> list[BaseSkill]: + """Return all registered skills, sorted by name.""" + return [self._skills[name] for name in self.names()] + + def load_from_paths(self, paths: Iterable[Path | str]) -> None: + """Discover and register skills from each root directory in order. + + Missing roots are ignored. Skills discovered in later roots override + same-named skills from earlier roots. + + Args: + paths: BaseSkill root directories to scan. + """ + for path in paths: + for skill in SkillLoader.discover(Path(path)): + self.register(skill) + + def clear(self) -> None: + """Remove all registered skills.""" + self._skills.clear() + + def __contains__(self, name: object) -> bool: + return name in self._skills + + def __len__(self) -> int: + return len(self._skills) diff --git a/sgr_agent_core/skills/rendering.py b/sgr_agent_core/skills/rendering.py new file mode 100644 index 00000000..84331a91 --- /dev/null +++ b/sgr_agent_core/skills/rendering.py @@ -0,0 +1,76 @@ +"""Render the model-facing skills catalog (progressive disclosure level 1). + +Mirrors the codex ``render_available_skills_body`` pattern: a short block listing +each model-invocable skill's name + (budgeted) description, followed by a +"how to use" instruction telling the agent to load a skill's full body via the +``use_skill`` tool. This is what makes skills autonomously invocable. +""" + +from __future__ import annotations + +from collections.abc import Iterable + +from sgr_agent_core.skills.models import BaseSkill + +DEFAULT_MAX_DESC_CHARS = 500 + + +def _truncate(text: str, max_chars: int) -> str: + """Truncate ``text`` to ``max_chars`` with an ellipsis if it overflows.""" + text = " ".join(text.split()) + if max_chars > 0 and len(text) > max_chars: + return text[:max_chars].rstrip() + "…" + return text + + +def _global_max_desc_chars() -> int: + """Read the description budget from global execution settings (lazy).""" + try: + from sgr_agent_core.agent_config import GlobalConfig + + return GlobalConfig().execution.max_skill_desc_chars + except Exception: # noqa: BLE001 - fall back to the default outside a loaded config + return DEFAULT_MAX_DESC_CHARS + + +def render_available_skills(skills: Iterable[BaseSkill], max_desc_chars: int | None = None) -> str: + """Render the skills listing block injected into the system prompt. + + Only model-invocable skills are listed. Returns an empty string when there + are none, so the system prompt stays unchanged for skill-less agents. + + Args: + skills: Skills available to the agent. + max_desc_chars: Per-entry description cap (level-1 budget). Defaults to + the global ``execution.max_skill_desc_chars`` setting. + + Returns: + A markdown block (or empty string). + """ + if max_desc_chars is None: + max_desc_chars = _global_max_desc_chars() + listed = [s for s in skills if s.metadata.model_invocable] + if not listed: + return "" + + lines = [ + "", + "A skill is a set of instructions loaded on demand from a SKILL.md source.", + "The skills below are available this turn. Each entry shows a name and description.", + "", + "### Available Skills", + ] + for skill in listed: + lines.append(f"- {skill.name}: {_truncate(skill.description, max_desc_chars)}") + lines.extend( + [ + "", + "### How to use Skills", + "- If the user names a skill, or the task clearly matches a skill's description above,", + " call the `use_skill` tool with that skill's name to load its full instructions.", + "- Read the loaded instructions completely before acting on them.", + "- Only load a skill when it is relevant; do not load skills you do not need.", + "", + ] + ) + return "\n".join(lines) diff --git a/sgr_agent_core/tools/__init__.py b/sgr_agent_core/tools/__init__.py index 1ec918af..c6de9a74 100644 --- a/sgr_agent_core/tools/__init__.py +++ b/sgr_agent_core/tools/__init__.py @@ -13,6 +13,7 @@ from sgr_agent_core.tools.generate_plan_tool import GeneratePlanTool from sgr_agent_core.tools.reasoning_tool import ReasoningTool from sgr_agent_core.tools.run_command_tool import RunCommandTool +from sgr_agent_core.tools.skill_tool import SkillTool from sgr_agent_core.tools.web_search_tool import WebSearchConfig, WebSearchTool __all__ = [ @@ -35,6 +36,7 @@ "GeneratePlanTool", "ReasoningTool", "RunCommandTool", + "SkillTool", "WebSearchConfig", "WebSearchTool", ] diff --git a/sgr_agent_core/tools/skill_tool.py b/sgr_agent_core/tools/skill_tool.py new file mode 100644 index 00000000..8b5271ae --- /dev/null +++ b/sgr_agent_core/tools/skill_tool.py @@ -0,0 +1,49 @@ +"""The ``use_skill`` tool: progressive-disclosure level-2 skill expansion. + +When the agent decides (from the system-prompt skills catalog) that a skill is +relevant, it calls ``use_skill`` with the skill name. The tool returns the +skill's full ``SKILL.md`` body, which the agent appends to the conversation and +follows for the rest of the run. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, ClassVar + +from pydantic import Field + +from sgr_agent_core.base_tool import SystemBaseTool +from sgr_agent_core.skills.commands import render_skill_body + +if TYPE_CHECKING: + from sgr_agent_core.agent_definition import AgentConfig + from sgr_agent_core.models import AgentContext + + +class SkillTool(SystemBaseTool): + """Load the full instructions of an available skill by name.""" + + tool_name: ClassVar[str] = "use_skill" + description: ClassVar[str] = ( + "Load the full instructions of an available skill by name. " + "Use this when a task matches a skill listed in AVAILABLE_SKILLS. " + "Returns the skill's instructions to follow for the rest of the task." + ) + + skill_name: str = Field(description="Name of the skill to load (as shown in AVAILABLE_SKILLS)") + + async def __call__(self, context: "AgentContext", config: "AgentConfig", **kwargs) -> str: + """Return the requested skill's body, or a helpful error listing the + available skills. + + Only model-invocable skills can be loaded here, symmetric with the + user-driven ``/skill-name`` path (which gates on ``user_invocable``). + Skills authored with ``disable-model-invocation: true`` are therefore + invisible to the model even if their name is known. + """ + skills = [s for s in (getattr(context, "available_skills", []) or []) if s.metadata.model_invocable] + skill = next((s for s in skills if s.name == self.skill_name), None) + if skill is None: + available = ", ".join(sorted(s.name for s in skills)) or "(none)" + return f"Skill '{self.skill_name}' not found. Available skills: {available}" + return render_skill_body(skill) diff --git a/tests/test_api_endpoints.py b/tests/test_api_endpoints.py index 9d6bf13e..5d2ac4bb 100644 --- a/tests/test_api_endpoints.py +++ b/tests/test_api_endpoints.py @@ -18,6 +18,7 @@ delete_agent, get_agent_state, get_agents_list, + get_available_skills, provide_clarification, ) from sgr_agent_core.server.models import ChatCompletionRequest, MessagesList, MessagesRequest @@ -685,3 +686,37 @@ def test_agent_storage_isolation(self): """Test that different test methods have isolated storage.""" # This test verifies that setup_method clears storage properly assert len(agents_storage) == 0 + + +class TestSkillsEndpoint: + """Tests for GET /v1/skills.""" + + @patch("sgr_agent_core.server.endpoints.AgentFactory") + @pytest.mark.asyncio + async def test_lists_skills_per_model(self, mock_factory): + from sgr_agent_core.skills import BaseSkill, SkillMetadata + + mock_def = Mock() + mock_def.name = "sgr_agent" + mock_factory.get_definitions_list.return_value = [mock_def] + mock_factory._resolve_skills.return_value = [ + BaseSkill(metadata=SkillMetadata(name="greet", description="Greets people.")) + ] + + result = await get_available_skills() + assert result["object"] == "list" + assert result["data"][0]["model"] == "sgr_agent" + assert result["data"][0]["skills"] == [{"name": "greet", "description": "Greets people."}] + + @patch("sgr_agent_core.server.endpoints.AgentFactory") + @pytest.mark.asyncio + async def test_filter_by_model(self, mock_factory): + a = Mock() + a.name = "a" + b = Mock() + b.name = "b" + mock_factory.get_definitions_list.return_value = [a, b] + mock_factory._resolve_skills.return_value = [] + + result = await get_available_skills(model="b") + assert [d["model"] for d in result["data"]] == ["b"] diff --git a/tests/test_cli.py b/tests/test_cli.py index f0b8b0ee..d2bdc565 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,8 +6,31 @@ import pytest -from sgr_agent_core.cli.sgrsh import chat_loop, find_config_file, main, run_agent +from sgr_agent_core.cli.sgrsh import chat_loop, find_config_file, format_skills_listing, main, run_agent from sgr_agent_core.models import AgentStatesEnum +from sgr_agent_core.skills import BaseSkill, SkillMetadata + + +class TestFormatSkillsListing: + """Test the skills listing formatter used by --list-skills.""" + + def test_empty(self): + assert "No skills" in format_skills_listing([]) + + def test_lists_name_and_description(self): + skills = [BaseSkill(metadata=SkillMetadata(name="greet", description="Greets people."))] + out = format_skills_listing(skills) + assert "greet" in out + assert "Greets people." in out + + def test_excludes_non_user_invocable(self): + skills = [ + BaseSkill(metadata=SkillMetadata(name="shown", description="Shown.")), + BaseSkill(metadata=SkillMetadata(name="hidden", description="Hidden.", user_invocable=False)), + ] + out = format_skills_listing(skills) + assert "shown" in out + assert "hidden" not in out class TestFindConfigFile: diff --git a/tests/test_prompts.py b/tests/test_prompts.py index c2ca26b0..552c935b 100644 --- a/tests/test_prompts.py +++ b/tests/test_prompts.py @@ -164,6 +164,101 @@ def test_get_system_prompt_missing_placeholder(self): result = PromptLoader.get_system_prompt([], prompts_config) assert result == "This template has no placeholders." + def test_get_system_prompt_with_skills(self): + """Skills are auto-registered into the system prompt via + {available_skills}.""" + from sgr_agent_core.skills import BaseSkill, SkillMetadata + + skills = [BaseSkill(metadata=SkillMetadata(name="greet", description="Greets people warmly."))] + + with tempfile.TemporaryDirectory() as tmpdir: + template_file = os.path.join(tmpdir, "system_prompt.txt") + template = "Tools:\n{available_tools}\nSkills:\n{available_skills}\nEnd." + with open(template_file, "w", encoding="utf-8") as f: + f.write(template) + dummy_file = os.path.join(tmpdir, "dummy.txt") + with open(dummy_file, "w", encoding="utf-8") as f: + f.write("dummy") + + prompts_config = PromptsConfig( + system_prompt_file=template_file, + initial_user_request_file=dummy_file, + clarification_response_file=dummy_file, + ) + + result = PromptLoader.get_system_prompt([], prompts_config, available_skills=skills) + assert "greet: Greets people warmly." in result + assert "use_skill" in result + + def test_get_system_prompt_skills_default_empty(self): + """Without skills, the {available_skills} placeholder renders empty and + no error is raised (backward compatible).""" + with tempfile.TemporaryDirectory() as tmpdir: + template_file = os.path.join(tmpdir, "system_prompt.txt") + template = "Tools:\n{available_tools}\nSkills:\n{available_skills}\nEnd." + with open(template_file, "w", encoding="utf-8") as f: + f.write(template) + dummy_file = os.path.join(tmpdir, "dummy.txt") + with open(dummy_file, "w", encoding="utf-8") as f: + f.write("dummy") + + prompts_config = PromptsConfig( + system_prompt_file=template_file, + initial_user_request_file=dummy_file, + clarification_response_file=dummy_file, + ) + + result = PromptLoader.get_system_prompt([], prompts_config) + assert "Skills:\n\nEnd." in result + + def test_get_system_prompt_skills_appended_when_placeholder_absent(self): + """A custom template without {available_skills} still gets the skills + catalog appended, so autonomous discovery works regardless of + template.""" + from sgr_agent_core.skills import BaseSkill, SkillMetadata + + skills = [BaseSkill(metadata=SkillMetadata(name="greet", description="Greets."))] + with tempfile.TemporaryDirectory() as tmpdir: + template_file = os.path.join(tmpdir, "system_prompt.txt") + template = "Only tools:\n{available_tools}" + with open(template_file, "w", encoding="utf-8") as f: + f.write(template) + dummy_file = os.path.join(tmpdir, "dummy.txt") + with open(dummy_file, "w", encoding="utf-8") as f: + f.write("dummy") + + prompts_config = PromptsConfig( + system_prompt_file=template_file, + initial_user_request_file=dummy_file, + clarification_response_file=dummy_file, + ) + + result = PromptLoader.get_system_prompt([], prompts_config, available_skills=skills) + assert result.startswith("Only tools:\n") + assert "greet: Greets." in result + assert "use_skill" in result + + def test_get_system_prompt_no_skills_no_append(self): + """Without skills, nothing is appended to a placeholder-less + template.""" + with tempfile.TemporaryDirectory() as tmpdir: + template_file = os.path.join(tmpdir, "system_prompt.txt") + template = "Only tools:\n{available_tools}" + with open(template_file, "w", encoding="utf-8") as f: + f.write(template) + dummy_file = os.path.join(tmpdir, "dummy.txt") + with open(dummy_file, "w", encoding="utf-8") as f: + f.write("dummy") + + prompts_config = PromptsConfig( + system_prompt_file=template_file, + initial_user_request_file=dummy_file, + clarification_response_file=dummy_file, + ) + + result = PromptLoader.get_system_prompt([], prompts_config) + assert result == "Only tools:\n" + def test_get_initial_user_request(self): """Test get_initial_user_request formats date correctly.""" with tempfile.TemporaryDirectory() as tmpdir: diff --git a/tests/test_skill_loader.py b/tests/test_skill_loader.py new file mode 100644 index 00000000..6124f932 --- /dev/null +++ b/tests/test_skill_loader.py @@ -0,0 +1,174 @@ +"""Tests for the skills data model and loader (Layer 1).""" + +from pathlib import Path + +import pytest + +from sgr_agent_core.skills import BaseSkill, SkillError, SkillLoader, SkillMetadata + + +def _write_skill(root: Path, name: str, frontmatter: str, body: str = "Body text") -> Path: + """Create a skill directory with a SKILL.md and return the directory.""" + skill_dir = root / name + skill_dir.mkdir(parents=True, exist_ok=True) + (skill_dir / "SKILL.md").write_text(f"---\n{frontmatter}\n---\n\n{body}\n", encoding="utf-8") + return skill_dir + + +class TestSkillMetadata: + """Validation rules for skill frontmatter metadata.""" + + def test_valid_metadata(self): + meta = SkillMetadata(name="pdf-processing", description="Process PDF files. Use when working with PDFs.") + assert meta.name == "pdf-processing" + assert meta.description.startswith("Process PDF") + assert meta.allowed_tools == [] + assert meta.metadata == {} + + def test_name_too_long_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="a" * 65, description="ok") + + def test_name_invalid_chars_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="Bad_Name", description="ok") + + def test_name_reserved_word_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="claude-helper", description="ok") + + def test_name_xml_tag_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="foo", description="ok") + + def test_description_empty_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="foo", description="") + + def test_description_too_long_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="foo", description="x" * 1025) + + def test_description_xml_tag_rejected(self): + with pytest.raises((ValueError, SkillError)): + SkillMetadata(name="foo", description="uses