From 8729330120b1e641e6df245c10a0af384a07be89 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Fri, 9 Oct 2026 21:40:45 +0800 Subject: [PATCH] core/skills: announce the skill tools in a notice, not a catalog --- AGENTS.md | 6 +-- README.md | 8 ++- doc/plyngent.example.toml | 6 +-- doc/skills.md | 12 ++--- src/plyngent/agent/__init__.py | 2 + src/plyngent/agent/notices.py | 40 ++++++++++++++ src/plyngent/cli/state.py | 49 ++++++++--------- src/plyngent/config/models.py | 8 ++- src/plyngent/skills/discovery.py | 2 +- tests/test_agent/test_notices.py | 27 +++++++++- tests/test_cli/test_skills_cmd.py | 67 +++++++++++------------- tests/test_config/test_skills_section.py | 6 --- 12 files changed, 139 insertions(+), 94 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7b29585..98d0a23 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,7 +56,7 @@ lmproto/deepseek/anthropic/ ← DeepSeek Anthropic client (extends Anthropic ### Config (`config/`) -TOML load/store (`ConfigStore`): `[providers]` tagged union presets, `[database]` section. Default path via platformdirs. Providers may set `timeout` as a float or `{ connect, read }` (`HttpTimeoutConfig`); omitted → product defaults (10s connect / 600s read). `DeepseekProvider.convention` (Literal: `""`/`"openai"`/`"anthropic"`/`"responses"`) selects the DeepSeek API surface — `"responses"` → OpenAI Responses, `"anthropic"` → Anthropic Messages on `https://api.deepseek.com/anthropic` (default url follows the convention); a per-model `ModelConfig.convention` overrides it (empty = inherit). `[mcp]` (`McpConfig`): `servers.` stdio server definitions (`McpServerConfig`: `command`/`args`/`env`/`cwd`/`timeout`/`read_only`, `url` reserved for streamable HTTP) + `disable` gate; defining a server opts it in. `[skills]` (`SkillsConfig`): `paths` (explicit roots first), `discover` (`plyngent` user dir + `claude` dirs) with `include_project_roots`, `inject_catalog` / `max_catalog_skills`, `enabled`, and `allow_write` (standing write grant); a malformed section degrades to defaults like `[agent]` / `[mcp]`. +TOML load/store (`ConfigStore`): `[providers]` tagged union presets, `[database]` section. Default path via platformdirs. Providers may set `timeout` as a float or `{ connect, read }` (`HttpTimeoutConfig`); omitted → product defaults (10s connect / 600s read). `DeepseekProvider.convention` (Literal: `""`/`"openai"`/`"anthropic"`/`"responses"`) selects the DeepSeek API surface — `"responses"` → OpenAI Responses, `"anthropic"` → Anthropic Messages on `https://api.deepseek.com/anthropic` (default url follows the convention); a per-model `ModelConfig.convention` overrides it (empty = inherit). `[mcp]` (`McpConfig`): `servers.` stdio server definitions (`McpServerConfig`: `command`/`args`/`env`/`cwd`/`timeout`/`read_only`, `url` reserved for streamable HTTP) + `disable` gate; defining a server opts it in. `[skills]` (`SkillsConfig`): `paths` (explicit roots first), `discover` (`plyngent` user dir + `claude` dirs) with `include_project_roots`, `enabled`, and `allow_write` (standing write grant); a malformed section degrades to defaults like `[agent]` / `[mcp]`. `reasoning.py`: thinking strength as `reasoning_effort` (`ReasoningEffortConfig`: `""` = send nothing | `none` … `high` … `max`) and `thinking_budget_tokens`, settable on `[agent]`, a provider, or a model entry — `resolve_reasoning(agent, provider, model)` merges the layers per field (model > provider > agent) and clamps `max` (DeepSeek chat only) to `xhigh` for every other surface, returning a `ReasoningConfig` the agent loop and `create_client` consume. @@ -86,7 +86,7 @@ Async SQLAlchemy + aiosqlite. `MemoryStore`: schema init (+ lightweight SQLite ` - **`/compact`**: soft-compact tool dumps → model summary (no tools) via the same kind dispatch → **new** session seeded with summary message. Soft-compact keeps the last **12** tool results full-size by default (`DEFAULT_RECENT_TOOL_RESULTS`); older tool payloads shrink first. - Events: text_delta, **reasoning_delta**, assistant_message, tool_call/result, max_rounds, **error** (`retryable`/`source`), **cancelled** (`reason`), **usage** (`TokenUsage`). - Usage: API `usage` from completions (stream with `include_usage`); **char≈token fallback** (~4 chars/token) when omitted; **context size** = last request ``prompt_tokens`` (API preferred); `last_turn_usage` / `session_usage` are **billed sums** (tool rounds re-send history); CLI end-of-turn + `/status`. -- Config ``[agent]``: `system_prompt` (persona; default `DEFAULT_SYSTEM_PROMPT`; `""` omits persona), `tool_directives` (tool playbook; default `DEFAULT_TOOL_DIRECTIVES`; `""` omits playbook; both empty → no system), `mcp_instructions` (fold connected MCP servers' initialize ``instructions`` into the system prompt when tools on; default true), `[skills] inject_catalog` (fold the skill catalog — name/source/description only — the same way), `allow_paths` (static out-of-workspace pre-allow `path → read|write|exec`; never prompts), `auto_continue_limits` (auto-raise tool/PTY limits without prompting; default false), `reasoning_effort` / `thinking_budget_tokens` (thinking strength; see Config), `max_tool_result_chars`, `parallel_tools`, `confirm_destructive`, `path_denylist`, `max_context_tokens` (default 200k est. tokens). Truncated tool results carry a model hint (`[Truncated (N chars max; M omitted). You may ask for user to increase the limit via configuration file.]`) so the model can ask the user to raise `max_tool_result_chars`; request-time compact shrinks (`hint=False`) keep the terse `...[truncated N characters]` marker. +- Config ``[agent]``: `system_prompt` (persona; default `DEFAULT_SYSTEM_PROMPT`; `""` omits persona), `tool_directives` (tool playbook; default `DEFAULT_TOOL_DIRECTIVES`; `""` omits playbook; both empty → no system), `mcp_instructions` (fold connected MCP servers' initialize ``instructions`` into the system prompt when tools on; default true), `allow_paths` (static out-of-workspace pre-allow `path → read|write|exec`; never prompts), `auto_continue_limits` (auto-raise tool/PTY limits without prompting; default false), `reasoning_effort` / `thinking_budget_tokens` (thinking strength; see Config), `max_tool_result_chars`, `parallel_tools`, `confirm_destructive`, `path_denylist`, `max_context_tokens` (default 200k est. tokens). Truncated tool results carry a model hint (`[Truncated (N chars max; M omitted). You may ask for user to increase the limit via configuration file.]`) so the model can ask the user to raise `max_tool_result_chars`; request-time compact shrinks (`hint=False`) keep the terse `...[truncated N characters]` marker. ### Tools (`tools/`) @@ -120,7 +120,7 @@ Click app + readline REPL. Entry: `plyngent` / `python -m plyngent`. - Slash: Click group in `cli/slash.py` + `awaitlet` for async work; Tab completer from registry + ParamType `shell_complete`. `/history` is turn-oriented (`cli/transcript.py`): a turn runs from a user row to the next one, rows carry **display ordinals** (never DB row numbers; restart with the session), the turn's ends (user + final answer) always print in full, `-v` expands every row (one-liners), `-vv` prints full bodies + local rows, `--preview` collapses every row, and `N`/`last [N]` address turns while `--message N` addresses one row. Multiline `"""` … `"""`; `/edit` via `$VISUAL`/`$EDITOR` (blocking only). `/yolo on|off|once` for soft destructive confirms. `/model --persist` / `/models --persist` write model catalog entries into TOML. `/todos` for human show/push/pop/clear of the todo stack. `/skills` (`list` default) prints the discovered skills with their roots, shadowed copies, and frontmatter notes; `/skills search PATTERN` runs the matching tool handler (the slash path binds the tool context itself); `/skills open NAME` reveals the skill's directory in the system file manager. `/mcp` (`list` default) prints per-server status/tool counts + an `instructions` preview when the server sent usage guidance, + last stderr lines; `/mcp reconnect` re-reads config, restarts every enabled server, and rebuilds the tool registry. `/grants` lists directory-access grants (`revoke `; config grants point at the TOML); `/status` includes the grant count and the visible skill count (or `off` when `[skills].enabled` is false). - `/btw` side questions: forked session; main transcript/DB untouched. `--tools=read` (default) exposes only `READ_ONLY` tools under `read_only_context()` (fetch=GET) with the main session view; `--tools=no` off; `--tools=full` whole-registry clone with fresh session. Tools off: default falls back to no tools; explicit `read`/`full` errors. The model gets an `aside` notice as its last message before the question (unsaved exchange + tool scope), shown to the user as `[notice] aside: …`. - Explicit `/resume` or `--session` from another workspace prompts: **keep** / **update** / **abort**. A session resumed by a **freshly started process** (`--session` / default latest) also gets a `resume` notice about what a restart drops (PTY sessions, running commands, temp workspaces) — resuming inside a live process pushes nothing. -- Host notices (`agent/notices.py`): system-level messages appended as trailing `developer` rows, never system-prompt edits — `Notice.to_message()` for the model, `.summary` for the user via `cli.display.echo_notice` (`--quiet` hides the user line only). Durable hosts persist them (`ChatAgent.push_notice`); a side agent without memory keeps its notice local. Trailing developer messages never end a turn, so `/retry` still sees an interrupted turn behind a notice or checkpoint. +- Host notices (`agent/notices.py`): system-level messages appended as trailing `developer` rows, never system-prompt edits — `Notice.to_message()` for the model, `.summary` for the user via `cli.display.echo_notice` (`--quiet` hides the user line only). Durable hosts persist them (`ChatAgent.push_notice`); a side agent without memory keeps its notice local. `has_notice()` dedupes a session-scoped notice: the `skills` one is pushed once per session (feature enabled + tools on), lists nothing, and makes the model ask with `skill_list` / `skill_search`. Trailing developer messages never end a turn, so `/retry` still sees an interrupted turn behind a notice or checkpoint. - Failed/cancelled turns: user kept; **committed tool rounds kept**, and a batch cut short keeps the calls that already produced a result (`rollback_tail`; written to the store, so a resume continues from them instead of re-running them) — side effects are not re-run on `/retry`, and only unfinished assistant text is dropped; Ctrl+C cancels the in-flight turn (a session-scoped asyncio SIGINT handler routes it, `cli/interrupt.py` — a stray Ctrl+C while the loop waits never tears the REPL down); TTY confirms/ask prompts run off-loop and register their own SIGINT target, so Ctrl+C there cancels just the poll-based read (`wait` reports `cancelled by user`) or is ignored with a hint for readline prompts (`ask_user_*`, confirms) — never the turn, never the loop; auto-retry waits 5s/10s/15s/20s then +10s (10 attempts), its countdown is Ctrl+C-cancellable, and the budget **resets when an attempt completes a model round** (connection recovered); then `/retry`. - Pretty tool lines: every builtin tool prints its `* Verb 'target' ` prefix as the call starts and appends the outcome (`L1-4 (done)`, `(exit code 0)`, `(session 3)`) when the result lands, so a blocking tool stays visible while it runs; namespaced MCP tools share one generic line (`* MCP : (done)`). Plugin tools and custom tool calls keep the `[tool] …` / `[tool ok] …` whole-line style. Interactive TTY only (`interactive_terminal()`): piped/plain output and `--verbose` keep whole lines. A second call of the same round holds its prefix back until the open row takes its own detail (results arrive in call order), so each call still prints one finished row; a prompt box (or other output) printed while the tool runs *ends* the open prefix as its own line and falls back to whole lines for the rest of the batch — the cursor is never moved, so a prefix wider than the terminal cannot be half-erased. A batch (one round's calls) that only touches todo tools prints one `* Todo x: Push, Update` line plus its last result, whose text already carries the final stack, instead of one header and a full list render per call. - **`plyngent providers`**, **`config path|edit`**. Config open: `$VISUAL`/`$EDITOR` (wait), else system default (`xdg-open` / `open` / `startfile`, non-blocking). `/edit` stays blocking-only. No providers → optional edit then reload when waited. diff --git a/README.md b/README.md index a79ca17..01a3469 100644 --- a/README.md +++ b/README.md @@ -162,12 +162,10 @@ max_context_tokens = 200000 # Optional skills (instruction directories). See doc/skills.md. # [skills] -# enabled = true # false = no skill tools, no prompt catalog +# enabled = true # false = no skill tools # paths = ["~/team-skills"] # explicit roots, highest priority # discover = ["plyngent", "claude"] # our user dir + other harnesses' dirs # include_project_roots = true # also /.claude/skills -# inject_catalog = true # fold name + description into the system prompt -# max_catalog_skills = 50 # allow_write = false # true = skip the per-write grant prompt # Optional plugins (entry-point names); default load none. See doc/plugins.md. @@ -193,7 +191,7 @@ thinking_budget_tokens = 16000 # explicit Anthropic thinking budget Third-party **plugins**: install a package that declares `project.entry-points."plyngent.tools"` (and later other groups), then allowlist the entry-point name under **`[plugins].enable`**. Details: [doc/plugins.md](doc/plugins.md). -**Skills** (instruction directories with a `SKILL.md`): discovered under `[skills].paths`, our own user skills directory, and other harnesses' skill directories (`~/.claude/skills`, plus `/.claude/skills` when `include_project_roots`). The catalog — name, source, description — is folded into the system prompt when tools are on; the model reads a `SKILL.md` with `skill_read`, searches with `skill_search`, and creates or updates skills with `skill_create` / `skill_edit` behind a timed `once`/`session`/`no` grant (`[skills].allow_write = true` for a standing one; the path denylist still wins and `--yes`/`/yolo` never skips it). Reading needs no grant: skill roots are read-only roots, so `read_file` / `regex_files` see them too while writes stay denied. Details: [doc/skills.md](doc/skills.md). +**Skills** (instruction directories with a `SKILL.md`): discovered under `[skills].paths`, our own user skills directory, and other harnesses' skill directories (`~/.claude/skills`, plus `/.claude/skills` when `include_project_roots`). A session-start notice (a host notice, never a system-prompt edit) tells the model that the skill tools exist and how to use them — `skill_list` to see what exists, `skill_read` for a `SKILL.md`, `skill_search` for content — and deliberately lists nothing; `skill_create` / `skill_edit` write one behind a timed `once`/`session`/`no` grant (`[skills].allow_write = true` for a standing one; the path denylist still wins and `--yes`/`/yolo` never skips it). Reading needs no grant: skill roots are read-only roots, so `read_file` / `regex_files` see them too while writes stay denied. Details: [doc/skills.md](doc/skills.md). **MCP servers** (Model Context Protocol over stdio): define servers under `[mcp.servers.]` (`command` + `args`, optional `env`/`cwd`/`timeout`/`read_only`) and their tools become agent tools namespaced `mcp____` (LOCAL tags; `read_only = true` also marks them READ_ONLY, eligible for `/btw --tools=read`). Names in `[mcp].disable` stay disconnected. A server may also return optional `instructions` (usage guidance) in its MCP `initialize` response; when tools are on those are folded into the agent system prompt as `MCP server instructions:` blocks (`[agent] mcp_instructions = false` disables), and `/mcp` previews them. In the REPL, `/mcp` lists per-server status and tool counts; `/mcp reconnect` re-reads the config file and restarts every enabled server (adopts newly added ones). @@ -323,7 +321,7 @@ Ctrl+C cancels the in-flight turn, and during the auto-retry countdown it cancel Default registry: file ops (including `tree` with a markdown bullet-list default, `flat` paths or classic `decorated` on demand; default noise-dir skips), `run_argv` / `run_argv_batch` / PTY (POSIX openpty; Windows ConPTY via pywinpty), read-only VCS (git), HTTP `fetch` (GET/POST/PUT/DELETE via niquests; private/loopback hosts need a human policy grant, not YOLO), human prompts (`ask_user_line` / `ask_user_choice` / `ask_user_form`), `wait` (line prompt with timeout; Enter disturbs), `request_directory_access` (out-of-workspace access grants), `get_truncated` (resume any truncated result via its `truncate_token=...`), skills (`skill_list` / `skill_read` / `skill_search` / `skill_create` / `skill_edit`), and todo stack tools (`todo_list` / `todo_push` / `todo_pop` / `todo_update` / `todo_clear`). -**Skills** are instruction directories shared with other harnesses: a folder with a `SKILL.md` (YAML frontmatter — `name`, `description`, … — plus a Markdown body) and optional bundled files (scripts, references). Discovery, roots, grants, and the authoring format are documented in [doc/skills.md](doc/skills.md); the short version is that the catalog (name + description) goes into the system prompt, the body is read on demand with `skill_read`, and writing a skill is a separate grant (a timed `once`/`session`/`no` prompt, or `[skills].allow_write`) that is **not** a directory-access grant and is never skipped by YOLO. +**Skills** are instruction directories shared with other harnesses: a folder with a `SKILL.md` (YAML frontmatter — `name`, `description`, … — plus a Markdown body) and optional bundled files (scripts, references). Discovery, roots, grants, and the authoring format are documented in [doc/skills.md](doc/skills.md); the short version is that a session-start notice points the model at them (`skill_list` / `skill_read` / `skill_search`) without listing anything, so what exists (and the bodies) is read on demand rather than injected, and writing a skill is a separate grant (a timed `once`/`session`/`no` prompt, or `[skills].allow_write`) that is **not** a directory-access grant and is never skipped by YOLO. Safety defaults: diff --git a/doc/plyngent.example.toml b/doc/plyngent.example.toml index 6c96ab4..3bdbdf7 100644 --- a/doc/plyngent.example.toml +++ b/doc/plyngent.example.toml @@ -31,8 +31,8 @@ # Discovery roots in priority order: explicit `paths`, our own user config # directory (/skills), other harnesses' skill directories # (~/.claude/skills; plus /.claude/skills when include_project_roots). -# The catalog (name + description) is folded into the system prompt when tools -# are on; the model reads bodies with skill_read and searches with skill_search. +# A session-start notice describes the tools; what exists is found with +# skill_list, bodies are read with skill_read, contents searched with skill_search. # Reading needs no grant (the roots are read-only roots); creating or editing a # skill asks once/session, or stands with allow_write = true. See doc/skills.md. # [skills] @@ -40,8 +40,6 @@ # paths = ["~/team-skills", "/srv/skills"] # discover = ["plyngent", "claude"] # include_project_roots = true -# inject_catalog = true -# max_catalog_skills = 50 # allow_write = false # MCP (Model Context Protocol) servers exposed as agent tools. diff --git a/doc/skills.md b/doc/skills.md index 4395815..795286e 100644 --- a/doc/skills.md +++ b/doc/skills.md @@ -49,12 +49,10 @@ lists the ones with no directory. ```toml [skills] -enabled = true # false = no skill tools and no prompt catalog +enabled = true # false = no skill tools paths = ["~/team-skills"] # explicit roots; `~` expands to your home discover = ["plyngent", "claude"] include_project_roots = true -inject_catalog = true # fold the catalog into the system prompt -max_catalog_skills = 50 allow_write = false # true = standing grant for skill writes ``` @@ -70,13 +68,15 @@ a note: the skill stays listable instead of vanishing. | Field | Meaning | |-------|---------| | `name` | Skill name; must be lowercase letters, digits, and single hyphens (max 64). A name that cannot be used falls back to the directory name, with a note. | -| `description` | One line shown in the catalog and in `skill_list`. Missing → the first non-heading paragraph of the body. | +| `description` | One line shown by `skill_list` (and `/skills` for the human). Missing → the first non-heading paragraph of the body. | | `version`, `license`, `allowed-tools`, … | Kept and shown by `skill_list`; `allowed-tools` is **recorded, not enforced** today. | ## What the agent sees -- **Catalog** (system prompt, `inject_catalog`): names, sources, descriptions — - never the bodies, so a turn only pays for the skill it actually reads. +- **Notice** (a host notice, `agent/notices.py`): when skills are enabled and + tools are on, the session gets one `skills` notice describing the tools; it + lists nothing, so what *exists* is asked for and a turn only pays for the + skill it actually reads. - `skill_list` — what exists, where it came from, shadowed copies, and the notes from repaired frontmatter. - `skill_read NAME [file]` — the `SKILL.md` body (ranges, line numbers, and a diff --git a/src/plyngent/agent/__init__.py b/src/plyngent/agent/__init__.py index b120b7a..cb3958b 100644 --- a/src/plyngent/agent/__init__.py +++ b/src/plyngent/agent/__init__.py @@ -23,7 +23,9 @@ from .loop import run_chat_loop as run_chat_loop from .notices import Notice as Notice from .notices import aside_notice as aside_notice from .notices import format_moment as format_moment +from .notices import has_notice as has_notice from .notices import resume_notice as resume_notice +from .notices import skill_tools_notice as skill_tools_notice from .tools import DangerClassifier as DangerClassifier from .tools import ToolConfirmHook as ToolConfirmHook from .tools import ToolDefinition as ToolDefinition diff --git a/src/plyngent/agent/notices.py b/src/plyngent/agent/notices.py index 0c1eb8f..d42384b 100644 --- a/src/plyngent/agent/notices.py +++ b/src/plyngent/agent/notices.py @@ -14,9 +14,15 @@ from __future__ import annotations from dataclasses import dataclass from datetime import UTC, datetime +from typing import TYPE_CHECKING from plyngent.lmproto.openai_compatible.model import DeveloperChatMessage +if TYPE_CHECKING: + from collections.abc import Sequence + + from plyngent.lmproto.openai_compatible.model import AnyChatMessage + # Marker prefix so hosts and the model can tell notices apart from other # developer messages (durable directive checkpoints share the role). NOTICE_MARKER = "[plyngent notice" @@ -35,6 +41,16 @@ class Notice: return DeveloperChatMessage(content=f"{NOTICE_MARKER}: {self.kind}]\n{self.body.strip()}") +def has_notice(messages: Sequence[AnyChatMessage], kind: str) -> bool: + """Whether *messages* already carries a notice of *kind*. + + Notices are persisted with the transcript, so a host asks this before + pushing one that belongs to the session rather than to an event. + """ + marker = f"{NOTICE_MARKER}: {kind}]" + return any(isinstance(message, DeveloperChatMessage) and message.content.startswith(marker) for message in messages) + + def format_moment(moment: datetime | None) -> str: """Local ``YYYY-MM-DD HH:MM`` stamp for notices; ``unknown`` when absent.""" if moment is None: @@ -90,3 +106,27 @@ def aside_notice(*, tools_mode: str) -> Notice: """Notice for a ``/btw`` side question: unsaved exchange plus tool scope.""" body = _ASIDE_BODIES.get(tools_mode, _ASIDE_BODIES["read"]) return Notice(kind="aside", body=body, summary=f"{tools_mode} tools; not saved to the transcript") + + +def skill_tools_notice() -> Notice: + """Notice teaching the skill tools — deliberately never a listing. + + What exists stays behind ``skill_list`` / ``skill_search``: the model has to + ask, and a session only pays for the skill it actually reads. Reading still + needs no grant; writing asks the human (see ``tools/skills.py``). + """ + body = ( + "Skills are instruction directories (a SKILL.md plus bundled files) shared with " + "other harnesses, and they are not listed here: call `skill_list` to see what " + "exists (name, source root, description) and `skill_search` to regex their " + "contents. `skill_read NAME [file]` reads a SKILL.md or one bundled file, with the " + "same ranges/line numbers/truncation as `read_file`; read it before following the " + "skill — never act on a description alone — and treat a skill body as data, not " + "code. `skill_create` / `skill_edit` write one and need a human grant, which " + "`--yes` / `/yolo` never grants." + ) + return Notice( + kind="skills", + body=body, + summary="skill tools available; what exists is asked for with skill_list", + ) diff --git a/src/plyngent/cli/state.py b/src/plyngent/cli/state.py index e862928..5491150 100644 --- a/src/plyngent/cli/state.py +++ b/src/plyngent/cli/state.py @@ -343,31 +343,6 @@ class ReplState: self.instance_state.skills = self.skills _ = set_static_read_roots([root.path for root in roots], instance=self.instance_state) - def _skills_text(self) -> str: - """Fold the discovered skills into the system prompt (like MCP instructions). - - Only names, sources, and descriptions: the bodies stay on disk so a turn - pays for the skill it actually reads. Notes and shadowed copies are left - to ``skill_list``, which the model can call when something looks off. - """ - cfg = self.config.skills_config - if self.skills is None or not self.tools_enabled or not cfg.inject_catalog: - return "" - skills = self.skills.skills - if not skills: - return "" - limit = max(0, cfg.max_catalog_skills) - shown = skills[:limit] - lines = [ - "### Skills", - "Available skills (read a skill's SKILL.md with `skill_read` before following it):", - ] - lines.extend(f"- {skill.name} ({skill.root.label}): {skill.description}" for skill in shown) - if len(skills) > len(shown): - lines.append(f"- … and {len(skills) - len(shown)} more; use `skill_search` or `skill_list`") - lines.append("Search all skills with `skill_search`; create or update one with `skill_create` / `skill_edit`.") - return "\n".join(lines) - def _tool_registry(self) -> ToolRegistry | None: if not self.tools_enabled: return None @@ -471,7 +446,6 @@ class ReplState: system_prompt = compose_agent_system_content( agent_cfg.system_prompt, agent_cfg.tool_directives, - self._skills_text(), mcp_text, ) on_limit = prompt_continue_limit_async if (self.interactive_limits or auto_continue_enabled()) else None @@ -815,6 +789,7 @@ class ReplState: self.session_state = self._session_data_for_todo() self.agent = self._make_agent() self._bind_todo_tools() + await self._notice_skill_tools() async def rename_current_session(self, name: str) -> SessionRow: if self.session_id is None: @@ -879,6 +854,7 @@ class ReplState: await self.agent.load_history() await self.load_todo_stack() await self.load_access_grants() + await self._notice_skill_tools() if restarted: await self._notice_restart(last_active=row.updated_at) @@ -902,12 +878,33 @@ class ReplState: await self.agent.load_history() await self.load_todo_stack() await self.load_access_grants() + await self._notice_skill_tools() if restarted: # Before touch_session: the stamp should be the previous run's activity. await self._notice_restart(last_active=latest.updated_at) _ = await self.memory.touch_session(latest.sid) return "resume" + async def _notice_skill_tools(self) -> None: + """Teach the skill tools once per session — never what skills exist. + + Instructions only: the listing stays behind ``skill_list`` / + ``skill_search``, and the store reads the filesystem live, so a skill that + appears later in the session is found the same way. A session created + before this host pushed the note gets it on resume instead of a second copy. + """ + from plyngent.agent import has_notice, skill_tools_notice + from plyngent.cli.display import echo_notice + + if self.skills is None or not self.tools_enabled: + return + notice = skill_tools_notice() + if has_notice(self.agent.messages, notice.kind): + return + _ = await self.agent.push_notice(notice) + if not self.quiet: + echo_notice(notice, err=True) + async def _notice_restart(self, *, last_active: datetime | None) -> None: """Tell the model — and the user — that this session outlived a restart.""" from plyngent.agent import resume_notice diff --git a/src/plyngent/config/models.py b/src/plyngent/config/models.py index c47a9cb..05776ed 100644 --- a/src/plyngent/config/models.py +++ b/src/plyngent/config/models.py @@ -253,17 +253,15 @@ class SkillsConfig(Struct, omit_defaults=True): ``include_project_roots``) A skill found in several roots shadows the lower-priority copies: they stay - listed, never hidden. ``enabled = false`` registers no skill tools at all and - injects no catalog. Writing needs an explicit grant (a confirm per call, or - ``allow_write`` for a standing one); reads need none. + listed, never hidden. ``enabled = false`` registers no skill tools at all. + Writing needs an explicit grant (a confirm per call, or ``allow_write`` for a + standing one); reads need none. """ enabled: bool = True paths: list[str] = field(default_factory=list) discover: list[SkillDiscoverySource] = field(default_factory=lambda: ["plyngent", "claude"]) include_project_roots: bool = True - inject_catalog: bool = True - max_catalog_skills: int = 50 allow_write: bool = False diff --git a/src/plyngent/skills/discovery.py b/src/plyngent/skills/discovery.py index bedb283..876c732 100644 --- a/src/plyngent/skills/discovery.py +++ b/src/plyngent/skills/discovery.py @@ -39,7 +39,7 @@ class SkillRoot: ``origin`` names where the root came from (``config`` for an explicit path, ``plyngent`` for our own user directory, ``claude`` for another harness); ``scope`` is ``user`` or ``project``. Together they label a skill's - provenance in listings and in the injected catalog. + provenance in listings. """ path: Path diff --git a/tests/test_agent/test_notices.py b/tests/test_agent/test_notices.py index 469c362..4cd7b9d 100644 --- a/tests/test_agent/test_notices.py +++ b/tests/test_agent/test_notices.py @@ -2,7 +2,7 @@ from __future__ import annotations from datetime import UTC, datetime, timedelta, timezone -from plyngent.agent import ChatAgent, Notice, aside_notice, format_moment, resume_notice +from plyngent.agent import ChatAgent, Notice, aside_notice, format_moment, has_notice, resume_notice, skill_tools_notice from plyngent.agent.chat import incomplete_turn_user_text from plyngent.config.models import DatabaseConfig from plyngent.lmproto.openai_compatible.model import ( @@ -57,6 +57,31 @@ def test_aside_notice_matches_tool_scope() -> None: assert aside_notice(tools_mode="weird").body == read.body +def test_skill_tools_notice_points_at_the_tools_without_listing() -> None: + notice = skill_tools_notice() + assert notice.kind == "skills" + # Instructions only — what exists is asked for, never injected. + assert "not listed here" in notice.body + assert "`skill_list`" in notice.body + assert "`skill_search`" in notice.body + assert "`skill_read NAME [file]`" in notice.body + assert "data, not code" in notice.body + assert "skill_list" in notice.summary + + +def test_has_notice_finds_a_persisted_row_by_kind() -> None: + messages = [ + SystemChatMessage(content="sys"), + skill_tools_notice().to_message(), + resume_notice(last_active=None).to_message(), + ] + assert has_notice(messages, "skills") is True + assert has_notice(messages, "resume") is True + assert has_notice(messages, "aside") is False + # A plain developer row (a directive checkpoint) is not a notice. + assert has_notice([DeveloperChatMessage(content="checkpoint")], "skills") is False + + async def test_push_notice_appends_and_persists() -> None: store = await MemoryStore.open(DatabaseConfig()) try: diff --git a/tests/test_cli/test_skills_cmd.py b/tests/test_cli/test_skills_cmd.py index 9174159..d7ac964 100644 --- a/tests/test_cli/test_skills_cmd.py +++ b/tests/test_cli/test_skills_cmd.py @@ -11,6 +11,7 @@ from plyngent.cli.slash import handle_slash from plyngent.cli.state import ReplState from plyngent.config.models import DatabaseConfig, OpenAIProvider from plyngent.config.store import ConfigStore +from plyngent.lmproto.openai_compatible.model import DeveloperChatMessage from plyngent.memory import MemoryStore from plyngent.prompting import NonInteractiveBackend, temporary_backend from plyngent.skills import SKILL_FILE @@ -31,6 +32,15 @@ def _write_skill(root: Path, name: str, *, description: str = "A test skill", bo return directory +def _skill_notice_rows(state: ReplState) -> list[str]: + """Body of every ``skills`` host notice in the session transcript.""" + return [ + message.content + for message in state.agent.messages + if isinstance(message, DeveloperChatMessage) and message.content.startswith("[plyngent notice: skills]") + ] + + def _config_doc(tmp_path: Path, *, paths: list[Path] | None = None, extra: str = "") -> tomlkit.TOMLDocument: lines = ["[skills]", "discover = []"] if paths: @@ -230,61 +240,44 @@ async def test_skill_tools_are_absent_when_disabled(tmp_path: Path) -> None: await state.memory.close() -async def test_skills_catalog_is_folded_into_the_system_prompt(tmp_path: Path) -> None: +async def test_skill_tools_note_teaches_the_tools_without_listing_skills(tmp_path: Path) -> None: root = tmp_path / "ours" root.mkdir() _ = _write_skill(root, "acme-deploy", description="Deploy the acme stack") state = await _state(tmp_path, skills_dir=root, tools_enabled=True) try: prompt = state.agent.system_prompt or "" - assert "### Skills" in prompt - assert "- acme-deploy (config-user): Deploy the acme stack" in prompt - assert "`skill_read`" in prompt - # The body stays on disk: only the catalog goes into the prompt. + # Nothing about the skill set is injected: not the names, not the bodies. + assert "acme-deploy" not in prompt + assert "Deploy the acme stack" not in prompt assert "Body." not in prompt + + notes = _skill_notice_rows(state) + assert len(notes) == 1 + assert "`skill_list`" in notes[0] + assert "`skill_search`" in notes[0] + + # The note belongs to the session, not the event: a second push is a no-op. + await state._notice_skill_tools() + assert len(_skill_notice_rows(state)) == 1 finally: await state.memory.close() -async def test_skills_catalog_respects_the_cap(tmp_path: Path) -> None: - root = tmp_path / "ours" - root.mkdir() - for name in ("alpha", "beta", "gamma"): - _ = _write_skill(root, name) - state = await _state(tmp_path, skills_dir=root, tools_enabled=True, extra="max_catalog_skills = 1") - try: - prompt = state.agent.system_prompt or "" - assert "alpha" in prompt - assert "beta" not in prompt - assert "… and 2 more" in prompt - finally: - await state.memory.close() - - -async def test_skills_catalog_can_be_switched_off(tmp_path: Path) -> None: +async def test_skill_tools_note_needs_the_feature_and_tools(tmp_path: Path) -> None: root = tmp_path / "ours" root.mkdir() _ = _write_skill(root, "acme") - state = await _state(tmp_path, skills_dir=root, tools_enabled=True, extra="inject_catalog = false") + disabled = await _state(tmp_path, skills_dir=root, tools_enabled=True, extra="enabled = false") try: - assert "### Skills" not in (state.agent.system_prompt or "") - # The tools are still offered; only the prompt block is gone. - assert state.agent.tools is not None - assert state.agent.tools.get("skill_list") is not None + assert _skill_notice_rows(disabled) == [] finally: - await state.memory.close() - - -async def test_skills_catalog_needs_tools(tmp_path: Path) -> None: - root = tmp_path / "ours" - root.mkdir() - _ = _write_skill(root, "acme") - state = await _state(tmp_path, skills_dir=root, tools_enabled=False) + await disabled.memory.close() + no_tools = await _state(tmp_path, skills_dir=root, tools_enabled=False) try: - assert state.agent.tools is None - assert "### Skills" not in (state.agent.system_prompt or "") + assert _skill_notice_rows(no_tools) == [] finally: - await state.memory.close() + await no_tools.memory.close() async def test_slash_skills_notes_when_tools_are_off(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: diff --git a/tests/test_config/test_skills_section.py b/tests/test_config/test_skills_section.py index 83cebb0..0214774 100644 --- a/tests/test_config/test_skills_section.py +++ b/tests/test_config/test_skills_section.py @@ -16,8 +16,6 @@ def test_skills_section_defaults(tmp_path: Path) -> None: assert store.skills_config.paths == [] assert store.skills_config.discover == ["plyngent", "claude"] assert store.skills_config.include_project_roots is True - assert store.skills_config.inject_catalog is True - assert store.skills_config.max_catalog_skills == 50 assert store.skills_config.allow_write is False @@ -30,8 +28,6 @@ enabled = false paths = ["~/team-skills", "/srv/skills"] discover = ["plyngent"] include_project_roots = false -inject_catalog = false -max_catalog_skills = 5 allow_write = true """, encoding="utf-8", @@ -41,8 +37,6 @@ allow_write = true assert cfg.paths == ["~/team-skills", "/srv/skills"] assert cfg.discover == ["plyngent"] assert cfg.include_project_roots is False - assert cfg.inject_catalog is False - assert cfg.max_catalog_skills == 5 assert cfg.allow_write is True