From 15454b6fec6ddd0c09eb8342b5871e71fcab7a8d Mon Sep 17 00:00:00 2001 From: Tianye Song <162393000+sontianye@users.noreply.github.com> Date: Sat, 4 Jul 2026 23:09:29 +0800 Subject: [PATCH] feat(skills): deferred skill discovery via describe_skill tool (#3775) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the full-metadata system-prompt block with a compact (names only) and an on-demand describe_skill tool when skills.deferred_discovery: true (default: false / backward compat). New modules: - skills/catalog.py — SkillCatalog (immutable, searchable; select: has no cap, keyword/prefix search caps at MAX_RESULTS=5) - skills/describe.py — build_describe_skill_tool(catalog) closure; build_skill_search_setup() wires SkillSearchSetup into both the LangGraph agent factory (agent.py) and DeerFlowClient (client.py) Changes: - Skill @dataclass(frozen=True); allowed_tools/required_secrets list→tuple - Skill First prompt line gated on skill_names (deferred vs legacy wording) - get_skills_prompt_section: short-circuit storage on deferred path; merge user_id (upstream) + skill_names (this PR) params - describe_skill tool parameter named "name" (matches prompt wording) - select: branch removes [:MAX_RESULTS] cap (exact request, not ranking) - AGENTS.md: document deferred_discovery config field + new modules Co-authored-by: Willem Jiang --- backend/AGENTS.md | 6 +- .../deerflow/agents/lead_agent/agent.py | 26 ++ .../deerflow/agents/lead_agent/prompt.py | 77 +++-- backend/packages/harness/deerflow/client.py | 17 +- .../harness/deerflow/config/skills_config.py | 4 + .../harness/deerflow/skills/__init__.py | 6 + .../harness/deerflow/skills/catalog.py | 102 +++++++ .../harness/deerflow/skills/describe.py | 180 +++++++++++ .../harness/deerflow/skills/parser.py | 16 +- .../deerflow/skills/storage/skill_storage.py | 4 +- .../storage/user_scoped_skill_storage.py | 13 +- .../packages/harness/deerflow/skills/types.py | 6 +- backend/tests/test_checkpointer.py | 10 + backend/tests/test_client.py | 86 ++++++ .../tests/test_deferred_tool_crosscontext.py | 2 +- backend/tests/test_lead_agent_prompt.py | 51 +++- backend/tests/test_lead_agent_skills.py | 73 ++++- backend/tests/test_skill_catalog.py | 193 ++++++++++++ backend/tests/test_skill_describe.py | 282 ++++++++++++++++++ .../test_skill_request_scoped_secrets.py | 2 +- backend/tests/test_skills_parser.py | 4 +- backend/tests/test_slash_skills.py | 10 +- backend/tests/test_subagent_executor.py | 2 +- config.example.yaml | 7 + 24 files changed, 1122 insertions(+), 57 deletions(-) create mode 100644 backend/packages/harness/deerflow/skills/catalog.py create mode 100644 backend/packages/harness/deerflow/skills/describe.py create mode 100644 backend/tests/test_skill_catalog.py create mode 100644 backend/tests/test_skill_describe.py diff --git a/backend/AGENTS.md b/backend/AGENTS.md index bf2d06fe5..dd954f743 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -404,7 +404,10 @@ Additional providers also live here (`brave`, `browserless`, `crawl4ai`, `ddg_se - **Location**: `deer-flow/skills/{public,custom}/` - **Format**: Directory with `SKILL.md` (YAML frontmatter: name, description, license, allowed-tools, required-secrets) - **Loading**: `load_skills()` recursively scans `skills/{public,custom}` for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json -- **Injection**: Enabled skills listed in agent system prompt with container paths +- **Injection (legacy / default)**: Enabled skills are listed in the agent system prompt with full metadata and container paths (`` block). Controlled by `skills.deferred_discovery: false` (default). +- **Deferred discovery** (`skills.deferred_discovery: true`): Skills are listed by name only in a compact `` block, keeping the system prompt prefix-cache friendly. The agent calls the `describe_skill` tool at runtime to fetch full metadata for skills it wants to use, then loads the SKILL.md via `read_file`. Two new modules support this path: + - `skills/catalog.py` — `SkillCatalog` (immutable, searchable; query forms: `select:a,b`, `+prefix`, free-text regex); `select:` returns all requested skills without a result cap; other modes cap at `MAX_RESULTS=5`. + - `skills/describe.py` — `build_describe_skill_tool(catalog)` builds the `describe_skill` tool as a closure; `build_skill_search_setup(skills, enabled, ...)` produces a `SkillSearchSetup(describe_skill_tool, skill_names)` that is wired into both the LangGraph agent factory (`agent.py`) and the embedded client (`client.py`). - **Slash activation**: `/skill-name task` loads that enabled skill's `SKILL.md` for the current model call only. The resolver rejects leading whitespace, missing separators, reserved channel commands (`/new`, `/help`, `/bootstrap`, `/status`, `/models`, `/memory`, `/goal`), disabled skills, and skills outside a custom agent's whitelist. - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory @@ -663,6 +666,7 @@ Returns `{}` when Langfuse is not in the enabled providers — LangSmith-only de - `tool_groups[]` - Logical groupings for tools - `sandbox.use` - Sandbox provider class path - `skills.path` / `skills.container_path` - Host and container paths to skills directory +- `skills.deferred_discovery` - When `true`, replaces the full-metadata `` prompt block with a compact `` (names only) and registers the `describe_skill` tool so the agent fetches metadata on demand. Defaults to `false` (legacy full-metadata injection) - `title` - Auto-title generation (enabled, max_words, max_chars, model_name; null model_name uses fast local fallback, explicit model_name uses the prompt_template LLM path) - `summarization` - Context summarization (enabled, trigger conditions, keep policy) - `subagents.enabled` - Master switch for subagent delegation diff --git a/backend/packages/harness/deerflow/agents/lead_agent/agent.py b/backend/packages/harness/deerflow/agents/lead_agent/agent.py index b8eab167b..de510088d 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/agent.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/agent.py @@ -520,15 +520,30 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig): skills_for_tool_policy = _load_enabled_skills_for_tool_policy(available_skills, app_config=resolved_app_config, user_id=resolved_user_id) + # Build skill search setup (deferred skill discovery). + # Controlled by skills.deferred_discovery — independent from tool_search.enabled. + from deerflow.skills.describe import build_skill_search_setup + + skill_search_enabled = resolved_app_config.skills.deferred_discovery + container_base_path = resolved_app_config.skills.container_path + if is_bootstrap: # Special bootstrap agent with minimal prompt for initial custom agent creation flow # Keep the bootstrap skill set intentionally narrow so agent creation # remains deterministic before the custom agent's own config exists. + bootstrap_skills = [s for s in skills_for_tool_policy if s.name in _BOOTSTRAP_SKILL_NAMES] + skill_setup = build_skill_search_setup( + bootstrap_skills, + enabled=skill_search_enabled, + container_base_path=container_base_path, + ) raw_tools = get_available_tools(model_name=model_name, subagent_enabled=subagent_enabled, app_config=resolved_app_config) + [setup_agent] filtered = filter_tools_by_skill_allowed_tools(raw_tools, skills_for_tool_policy, always_allowed_tool_names=SKILL_LOADING_TOOL_NAMES) if non_interactive: filtered = [tool for tool in filtered if tool.name not in _NON_INTERACTIVE_DISABLED_TOOL_NAMES] final_tools, setup = assemble_deferred_tools(filtered, enabled=resolved_app_config.tool_search.enabled) + if skill_setup.describe_skill_tool: + final_tools.append(skill_setup.describe_skill_tool) return create_agent( model=create_chat_model(name=model_name, thinking_enabled=thinking_enabled, app_config=resolved_app_config, attach_tracing=False), tools=final_tools, @@ -547,12 +562,20 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig): app_config=resolved_app_config, deferred_names=setup.deferred_names, user_id=resolved_user_id, + skill_names=skill_setup.skill_names or None, ), state_schema=ThreadState, ) # Custom agents can update their own SOUL.md / config via update_agent. # The default agent (no agent_name) does not see this tool. + # Build skill search setup from policy-filtered skills (same list used for + # tool-policy filtering), so describe_skill only exposes allowed skills. + skill_setup = build_skill_search_setup( + skills_for_tool_policy, + enabled=skill_search_enabled, + container_base_path=container_base_path, + ) # # Withhold ``update_agent`` from runs triggered by webhook channels # (currently only ``github``). Webhook prompts come from arbitrary @@ -575,6 +598,8 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig): if non_interactive: filtered = [tool for tool in filtered if tool.name not in _NON_INTERACTIVE_DISABLED_TOOL_NAMES] final_tools, setup = assemble_deferred_tools(filtered, enabled=resolved_app_config.tool_search.enabled) + if skill_setup.describe_skill_tool: + final_tools.append(skill_setup.describe_skill_tool) return create_agent( model=create_chat_model(name=model_name, thinking_enabled=thinking_enabled, reasoning_effort=reasoning_effort, app_config=resolved_app_config, attach_tracing=False), tools=final_tools, @@ -595,6 +620,7 @@ def _make_lead_agent(config: RunnableConfig, *, app_config: AppConfig): app_config=resolved_app_config, deferred_names=setup.deferred_names, user_id=resolved_user_id, + skill_names=skill_setup.skill_names or None, ), state_schema=ThreadState, ) diff --git a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py index 4a2c1b2b6..e87957210 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/prompt.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/prompt.py @@ -631,8 +631,8 @@ combined with a FastAPI gateway for REST API access [citation:FastAPI](https://f - **Clarification First**: ALWAYS clarify unclear/missing/ambiguous requirements BEFORE starting work - never assume or guess -{subagent_reminder}- Skill First: Always load the relevant skill before starting **complex** tasks. -- Progressive Loading: Load resources incrementally as referenced in skills +{subagent_reminder}{skill_first_reminder} +- Progressive Loading: Load skill resources incrementally as referenced - Output Files: Final deliverables must be in `/mnt/user-data/outputs` (⚠️ Skills are NOT deliverables — use `skill_manage` tool instead) - File Editing Workflow: When revising an existing file, prefer `str_replace` over `write_file` — it sends only the diff and avoids @@ -750,20 +750,20 @@ You have access to skills that provide optimized workflows for specific tasks. E """ -def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_config: AppConfig | None = None, user_id: str | None = None) -> str: - """Generate the skills prompt section with available skills list.""" - # Load ALL skills (enabled + disabled) to build the disabled-skills section - from deerflow.skills.storage import get_or_new_skill_storage, get_or_new_user_skill_storage - - if user_id: - storage = get_or_new_user_skill_storage(user_id, app_config=app_config) - else: - storage = get_or_new_skill_storage(app_config=app_config) - all_skills = storage.load_skills(enabled_only=False) - disabled_skills = [s for s in all_skills if not s.enabled] - - skills = get_enabled_skills_for_config(app_config, user_id=user_id) +def get_skills_prompt_section( + available_skills: set[str] | None = None, + *, + app_config: AppConfig | None = None, + user_id: str | None = None, + skill_names: frozenset[str] | None = None, +) -> str: + """Generate the skills prompt section. + When *skill_names* is provided, renders a compact ```` (names + only) so the LLM can discover skills via ``describe_skill``. When omitted, + falls back to the legacy full-metadata ```` rendering for + backward compatibility. + """ if app_config is None: try: from deerflow.config import get_app_config @@ -775,9 +775,30 @@ def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_c container_base_path = DEFAULT_SKILLS_CONTAINER_PATH skill_evolution_enabled = False else: - config = app_config - container_base_path = config.skills.container_path - skill_evolution_enabled = config.skill_evolution.enabled + container_base_path = app_config.skills.container_path + skill_evolution_enabled = app_config.skill_evolution.enabled + + skill_evolution_section = _build_skill_evolution_section(skill_evolution_enabled) + + # ── Deferred discovery path — storage not needed (caller supplies names) ─ + if skill_names is not None: + from deerflow.skills.describe import get_skill_index_prompt_section + + return get_skill_index_prompt_section( + skill_names=skill_names, + container_base_path=container_base_path, + skill_evolution_section=skill_evolution_section, + ) + + # ── Legacy full-metadata path — load ALL skills for disabled-skill section + if user_id: + storage = get_or_new_user_skill_storage(user_id, app_config=app_config) + else: + storage = get_or_new_skill_storage(app_config=app_config) + all_skills = storage.load_skills(enabled_only=False) + disabled_skills = [s for s in all_skills if not s.enabled] + + skills = get_enabled_skills_for_config(app_config, user_id=user_id) if not skills and not disabled_skills and not skill_evolution_enabled: return "" @@ -790,7 +811,6 @@ def get_skills_prompt_section(available_skills: set[str] | None = None, *, app_c available_key = tuple(sorted(available_skills)) if available_skills is not None else None if not skill_signature and not disabled_skill_signature and available_key is not None: return "" - skill_evolution_section = _build_skill_evolution_section(skill_evolution_enabled) return _get_cached_skills_prompt_section(skill_signature, disabled_skill_signature, available_key, container_base_path, skill_evolution_section) @@ -883,6 +903,7 @@ def apply_prompt_template( app_config: AppConfig | None = None, deferred_names: frozenset[str] = frozenset(), user_id: str | None = None, + skill_names: frozenset[str] | None = None, ) -> str: # Include subagent section only if enabled (from runtime parameter) n = max_concurrent_subagents @@ -906,8 +927,13 @@ def apply_prompt_template( else "" ) - # Get skills section - skills_section = get_skills_prompt_section(available_skills, app_config=app_config, user_id=user_id) + # Get skills section (deferred discovery when skill_names is provided) + skills_section = get_skills_prompt_section( + available_skills, + app_config=app_config, + user_id=user_id, + skill_names=skill_names, + ) # Get deferred tools section (tool_search) deferred_tools_section = get_deferred_tools_prompt_section(deferred_names=deferred_names) @@ -917,6 +943,14 @@ def apply_prompt_template( custom_mounts_section = _build_custom_mounts_section(app_config=app_config) acp_and_mounts_section = "\n".join(section for section in (acp_section, custom_mounts_section) if section) + # Gate the "Skill First" instruction on the deferred discovery path: + # legacy mode uses tool-agnostic wording; deferred mode references describe_skill. + skill_first_reminder = ( + "- Skill First: For complex tasks, call describe_skill(name) to check if a matching skill exists, then read_file to load it.\n" + if skill_names is not None + else "- Skill First: Always load the relevant skill before starting **complex** tasks.\n" + ) + # Build and return the fully static system prompt. # Memory and current date are injected per-turn via DynamicContextMiddleware # as a in the first HumanMessage, keeping this prompt @@ -929,6 +963,7 @@ def apply_prompt_template( deferred_tools_section=deferred_tools_section, subagent_section=subagent_section, subagent_reminder=subagent_reminder, + skill_first_reminder=skill_first_reminder, subagent_thinking=subagent_thinking, acp_section=acp_and_mounts_section, ) diff --git a/backend/packages/harness/deerflow/client.py b/backend/packages/harness/deerflow/client.py index 418df837d..70d704173 100644 --- a/backend/packages/harness/deerflow/client.py +++ b/backend/packages/harness/deerflow/client.py @@ -35,7 +35,7 @@ from langchain_core.messages import AIMessage, HumanMessage, SystemMessage, Tool from langchain_core.runnables import RunnableConfig from deerflow.agents.lead_agent.agent import build_middlewares -from deerflow.agents.lead_agent.prompt import apply_prompt_template +from deerflow.agents.lead_agent.prompt import apply_prompt_template, get_enabled_skills_for_config from deerflow.agents.thread_state import ThreadState from deerflow.config.agents_config import AGENT_NAME_PATTERN from deerflow.config.app_config import get_app_config, is_trace_correlation_enabled, reload_app_config @@ -44,6 +44,7 @@ from deerflow.config.paths import get_paths from deerflow.models import create_chat_model from deerflow.runtime.goal import DEFAULT_MAX_GOAL_CONTINUATIONS, build_goal_state, goal_thread_lock, read_thread_goal, write_thread_goal from deerflow.runtime.user_context import get_effective_user_id +from deerflow.skills.describe import build_skill_search_setup from deerflow.skills.storage import get_or_new_user_skill_storage from deerflow.tools.builtins.tool_search import assemble_deferred_tools from deerflow.trace_context import DEERFLOW_TRACE_METADATA_KEY, generate_trace_id, get_current_trace_id, reset_current_trace_id, set_current_trace_id @@ -256,6 +257,19 @@ class DeerFlowClient: tools = self._get_tools(model_name=model_name, subagent_enabled=subagent_enabled) final_tools, deferred_setup = assemble_deferred_tools(tools, enabled=self._app_config.tool_search.enabled) + + # Wire deferred skill discovery — mirrors agent.py so config flag works on both paths. + skills_list = get_enabled_skills_for_config(self._app_config) + if self._available_skills is not None: + skills_list = [s for s in skills_list if s.name in self._available_skills] + skill_setup = build_skill_search_setup( + skills_list, + enabled=self._app_config.skills.deferred_discovery, + container_base_path=self._app_config.skills.container_path, + ) + if skill_setup.describe_skill_tool: + final_tools.append(skill_setup.describe_skill_tool) + kwargs: dict[str, Any] = { # attach_tracing=False because ``stream()`` injects tracing # callbacks at the graph invocation root so a single embedded run @@ -281,6 +295,7 @@ class DeerFlowClient: app_config=self._app_config, deferred_names=deferred_setup.deferred_names, user_id=get_effective_user_id(), + skill_names=skill_setup.skill_names or None, ), "state_schema": ThreadState, } diff --git a/backend/packages/harness/deerflow/config/skills_config.py b/backend/packages/harness/deerflow/config/skills_config.py index 158ec8691..d18cca884 100644 --- a/backend/packages/harness/deerflow/config/skills_config.py +++ b/backend/packages/harness/deerflow/config/skills_config.py @@ -29,6 +29,10 @@ class SkillsConfig(BaseModel): default=DEFAULT_SKILLS_CONTAINER_PATH, description="Path where skills are mounted in the sandbox container", ) + deferred_discovery: bool = Field( + default=False, + description=("When enabled, skill metadata is not injected into the system prompt. Instead, only skill names appear in and the LLM discovers details on demand via the describe_skill tool."), + ) def get_skills_path(self) -> Path: """ diff --git a/backend/packages/harness/deerflow/skills/__init__.py b/backend/packages/harness/deerflow/skills/__init__.py index 77910d5c2..22975f617 100644 --- a/backend/packages/harness/deerflow/skills/__init__.py +++ b/backend/packages/harness/deerflow/skills/__init__.py @@ -1,5 +1,7 @@ from __future__ import annotations +from .catalog import SkillCatalog +from .describe import SkillSearchSetup, build_describe_skill_tool, build_skill_search_setup from .installer import SkillAlreadyExistsError, SkillSecurityScanError from .storage import LocalSkillStorage, SkillStorage, get_or_new_skill_storage from .types import Skill @@ -7,6 +9,10 @@ from .validation import ALLOWED_FRONTMATTER_PROPERTIES, _validate_skill_frontmat __all__ = [ "Skill", + "SkillCatalog", + "SkillSearchSetup", + "build_describe_skill_tool", + "build_skill_search_setup", "ALLOWED_FRONTMATTER_PROPERTIES", "_validate_skill_frontmatter", "SkillAlreadyExistsError", diff --git a/backend/packages/harness/deerflow/skills/catalog.py b/backend/packages/harness/deerflow/skills/catalog.py new file mode 100644 index 000000000..41b19a5fa --- /dev/null +++ b/backend/packages/harness/deerflow/skills/catalog.py @@ -0,0 +1,102 @@ +"""Skill catalog — deferred skill discovery at runtime. + +Mirrors ``DeferredToolCatalog`` from ``tool_search.py``: an immutable, searchable +catalog that lets the LLM discover skill metadata on demand rather than having +every skill's full description baked into the system prompt. + +The agent sees skill names in ```` but cannot read their metadata +until it calls ``describe_skill``. This keeps the system prompt compact and +prefix-cache friendly while still giving the model autonomous skill discovery. +""" + +from __future__ import annotations + +import logging +import re +from dataclasses import dataclass +from functools import cached_property + +from deerflow.skills.types import Skill + +logger = logging.getLogger(__name__) + +MAX_RESULTS = 5 + + +def _compile_catalog_regex(pattern: str) -> re.Pattern[str]: + """Compile ``pattern`` case-insensitively, falling back to literal match. + + Search queries come from the model, so an invalid regex (e.g. an unbalanced + paren) must degrade to a literal substring match rather than raise. + """ + try: + return re.compile(pattern, re.IGNORECASE) + except re.error: + return re.compile(re.escape(pattern), re.IGNORECASE) + + +# NOTE: frozen=True without slots=True keeps __dict__, which is what lets the +# @cached_property fields below cache (they write to instance.__dict__, bypassing +# the frozen __setattr__). Do NOT add slots=True or hash/names break at runtime. +@dataclass(frozen=True) +class SkillCatalog: + """Immutable catalog of skills. Pure search, no mutation. + + Query forms (mirror ``DeferredToolCatalog.search``): + + - ``"select:data-analysis,deep-research"`` — exact match by name. + - ``"+podcast gen"`` — require *podcast* in the name, rank by *gen*. + - ``"chart visualization"`` — regex match on name + description. + """ + + skills: tuple[Skill, ...] + + @cached_property + def names(self) -> frozenset[str]: + """All skill names in insertion order.""" + return frozenset(s.name for s in self.skills) + + def search(self, query: str) -> list[Skill]: + """Match *query* against skill names and descriptions. + + Returns at most ``MAX_RESULTS`` skills, ranked by relevance. + """ + query = query.strip() + if not query: + return [] + + # ── Exact selection ──────────────────────────────────────────── + if query.startswith("select:"): + wanted = {n.strip() for n in query[7:].split(",")} + return [s for s in self.skills if s.name in wanted] + + # ── Required-prefix search ───────────────────────────────────── + if query.startswith("+"): + parts = query[1:].split(None, 1) + if not parts: + return [] # bare "+" with no required token + required = parts[0].lower() + candidates = [s for s in self.skills if required in s.name.lower()] + if len(parts) > 1: + pattern = _compile_catalog_regex(parts[1]) + candidates.sort( + key=lambda s: _catalog_regex_score(pattern, s), + reverse=True, + ) + return candidates[:MAX_RESULTS] + + # ── Free-text regex search ───────────────────────────────────── + regex = _compile_catalog_regex(query) + scored: list[tuple[int, Skill]] = [] + for s in self.skills: + searchable = f"{s.name} {s.description or ''}" + if regex.search(searchable): + # Name match scores higher than description-only match. + scored.append((2 if regex.search(s.name) else 1, s)) + scored.sort(key=lambda x: x[0], reverse=True) + return [s for _, s in scored][:MAX_RESULTS] + + +def _catalog_regex_score(pattern: re.Pattern[str], s: Skill) -> int: + """Count regex hits across name + description for ranking.""" + return len(pattern.findall(f"{s.name} {s.description or ''}")) diff --git a/backend/packages/harness/deerflow/skills/describe.py b/backend/packages/harness/deerflow/skills/describe.py new file mode 100644 index 000000000..0546006aa --- /dev/null +++ b/backend/packages/harness/deerflow/skills/describe.py @@ -0,0 +1,180 @@ +"""describe_skill — deferred skill metadata retrieval at runtime. + +Builds the ``describe_skill`` tool as a closure over a :class:`SkillCatalog`. +The tool returns structured metadata (description, allowed tools, file location) +so the LLM can decide whether to ``read_file`` the full SKILL.md. + +Mirrors ``build_tool_search_tool`` from ``tool_search.py``: same query syntax, +same ``Command`` + ``ToolMessage`` return shape, same fail-safe degradation. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from typing import TYPE_CHECKING, Annotated + +from langchain_core.messages import ToolMessage +from langchain_core.tools import InjectedToolCallId, tool +from langgraph.types import Command + +if TYPE_CHECKING: + from langchain.tools import BaseTool + +from deerflow.constants import DEFAULT_SKILLS_CONTAINER_PATH +from deerflow.skills.catalog import SkillCatalog +from deerflow.skills.types import SkillCategory + +logger = logging.getLogger(__name__) + + +# ── Setup ──────────────────────────────────────────────────────────────────── + + +@dataclass(frozen=True) +class SkillSearchSetup: + """Result of assembling skill search for one agent build. + + Mirrors ``DeferredToolSetup`` from ``tool_search.py``. + + - **Empty** ``(None, frozenset())``: no skills available or skill search + disabled. The agent falls back to the legacy full-metadata prompt. + - **Populated**: ``describe_skill_tool`` is appended to the agent's tools, + ``skill_names`` are rendered in ```` instead of full metadata. + """ + + describe_skill_tool: BaseTool | None + skill_names: frozenset[str] + + +def build_describe_skill_tool( + catalog: SkillCatalog, + *, + container_base_path: str = DEFAULT_SKILLS_CONTAINER_PATH, +) -> BaseTool: + """Build the ``describe_skill`` tool as a closure over *catalog*. + + The returned tool is a plain ``@tool``-decorated function that searches the + catalog and returns a ``Command`` wrapping a ``ToolMessage``. No graph state + mutation is needed (unlike ``tool_search`` which promotes deferred tools). + """ + + @tool + def describe_skill( + name: str, + tool_call_id: Annotated[str, InjectedToolCallId], + ) -> Command: + """Fetch usage metadata for installed skills so you can decide whether to load them. + + Skills appear by name in in the system prompt. Until + fetched, only the name is known. This tool matches a query against + installed skills and returns their full metadata — description, allowed + tools, and file location — so you can decide whether to load the + SKILL.md via read_file. + + Query forms: + - "select:data-analysis,deep-research" -- fetch these exact skills (no cap) + - "chart visualization" -- keyword search, best matches (up to 5) + - "+podcast gen" -- require "podcast" in the name, rank by remaining terms (up to 5) + """ + matched = catalog.search(name) + if not matched: + content = f"No skills matched: {name}" + else: + content = _render_skill_metadata(matched, container_base_path) + + return Command( + update={ + "messages": [ + ToolMessage( + content=content, + tool_call_id=tool_call_id, + name="describe_skill", + ) + ], + } + ) + + return describe_skill + + +def build_skill_search_setup( + skills: list, + *, + enabled: bool, + container_base_path: str = DEFAULT_SKILLS_CONTAINER_PATH, +) -> SkillSearchSetup: + """Build the skill search setup from a filtered skill list. + + Mirrors ``build_deferred_tool_setup`` from ``tool_search.py``. + + Returns an empty setup when *enabled* is ``False`` or *skills* is empty. + """ + if not enabled or not skills: + return SkillSearchSetup(None, frozenset()) + + catalog = SkillCatalog(tuple(skills)) + return SkillSearchSetup( + describe_skill_tool=build_describe_skill_tool( + catalog, + container_base_path=container_base_path, + ), + skill_names=catalog.names, + ) + + +# ── Rendering ──────────────────────────────────────────────────────────────── + + +def _render_skill_metadata(skills: list, container_base_path: str) -> str: + """Render structured metadata for a list of matched skills.""" + blocks: list[str] = [] + for s in skills: + mutability = "[custom, editable]" if s.category == SkillCategory.CUSTOM else "[built-in]" + tools_line = ", ".join(s.allowed_tools) if s.allowed_tools else "(all)" + location = s.get_container_file_path(container_base_path) + blocks.append(f"## Skill: {s.name}\n- Description: {s.description} {mutability}\n- Allowed tools: {tools_line}\n- Location: {location}") + return "\n\n".join(blocks) + + +# ── Prompt rendering ───────────────────────────────────────────────────────── + + +def get_skill_index_prompt_section( + *, + skill_names: frozenset[str] = frozenset(), + container_base_path: str = DEFAULT_SKILLS_CONTAINER_PATH, + skill_evolution_section: str = "", +) -> str: + """Generate ```` with a name-only ````. + + Mirrors ``get_deferred_tools_prompt_section`` from ``tool_search.py``. + The agent knows what exists and can use ``describe_skill`` to load metadata. + + Returns empty string when there are no skills. + """ + if not skill_names: + return "" + + names = ", ".join(sorted(skill_names)) + evolution = f"\n{skill_evolution_section}" if skill_evolution_section else "" + + return f""" +You have access to skills that provide optimized workflows for specific tasks. + +**Skill Discovery:** +1. Check for a skill name that matches your task +2. Call describe_skill(name) to fetch its description and capabilities +3. If the skill matches, call read_file on the returned location to load full instructions +4. Follow the skill's instructions precisely + +**Explicit Slash Skill Activation:** +- If the user starts a request with `/`, that skill was explicitly requested. +- The runtime injects the activated skill content; do not call `read_file` for that SKILL.md again unless the injected skill references supporting resources you need. +{evolution} + +{names} + + +Skills are located at: {container_base_path} +""" diff --git a/backend/packages/harness/deerflow/skills/parser.py b/backend/packages/harness/deerflow/skills/parser.py index ddd4b638b..a02803f7e 100644 --- a/backend/packages/harness/deerflow/skills/parser.py +++ b/backend/packages/harness/deerflow/skills/parser.py @@ -43,11 +43,11 @@ def _format_yaml_error(skill_file: Path, exc: yaml.YAMLError, source: str) -> st return "\n".join(lines) -def parse_allowed_tools(raw: object, skill_file: Path) -> list[str] | None: +def parse_allowed_tools(raw: object, skill_file: Path) -> tuple[str, ...] | None: """Parse the optional allowed-tools frontmatter field. - Returns None when the field is omitted. Returns a list when the field is a - YAML sequence of strings, including an empty list for explicit no-tool + Returns None when the field is omitted. Returns a tuple when the field is a + YAML sequence of strings, including an empty tuple for explicit no-tool skills. Raises ValueError for malformed values. """ if raw is None: @@ -63,21 +63,21 @@ def parse_allowed_tools(raw: object, skill_file: Path) -> list[str] | None: if not tool_name: raise ValueError(f"allowed-tools in {skill_file} cannot contain empty tool names") allowed_tools.append(tool_name) - return allowed_tools + return tuple(allowed_tools) -def parse_required_secrets(raw: object, skill_file: Path) -> list[SecretRequirement]: +def parse_required_secrets(raw: object, skill_file: Path) -> tuple[SecretRequirement, ...]: """Parse the optional required-secrets frontmatter field (issue #3861). Accepts a YAML sequence whose items are either a string (the secret / env - variable name) or a mapping (``{name, optional}``). Returns an empty list + variable name) or a mapping (``{name, optional}``). Returns an empty tuple when the field is omitted. Entries whose name is missing or is not a valid environment-variable name are dropped with a warning, so one malformed declaration does not invalidate the whole skill. Raises ValueError only when the field is present but is not a list. """ if raw is None: - return [] + return () if not isinstance(raw, list): raise ValueError(f"required-secrets in {skill_file} must be a list") @@ -100,7 +100,7 @@ def parse_required_secrets(raw: object, skill_file: Path) -> list[SecretRequirem continue seen.add(name) secrets.append(SecretRequirement(name=name, optional=optional)) - return secrets + return tuple(secrets) def parse_skill_file(skill_file: Path, category: SkillCategory, relative_path: Path | None = None) -> Skill | None: diff --git a/backend/packages/harness/deerflow/skills/storage/skill_storage.py b/backend/packages/harness/deerflow/skills/storage/skill_storage.py index 08138e3ce..39e7e5d6e 100644 --- a/backend/packages/harness/deerflow/skills/storage/skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/skill_storage.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import logging import re from abc import ABC, abstractmethod @@ -267,8 +268,7 @@ class SkillStorage(ABC): from deerflow.config.extensions_config import ExtensionsConfig extensions_config = ExtensionsConfig.from_file() - for skill in skills: - skill.enabled = extensions_config.is_skill_enabled(skill.name, skill.category) + skills = [dataclasses.replace(s, enabled=extensions_config.is_skill_enabled(s.name, s.category)) for s in skills] except Exception as e: logger.warning("Failed to load extensions config: %s", e) diff --git a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py index de56d1f31..2630da8d0 100644 --- a/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py +++ b/backend/packages/harness/deerflow/skills/storage/user_scoped_skill_storage.py @@ -29,6 +29,7 @@ two users own same-named custom skills. from __future__ import annotations import asyncio +import dataclasses import json import logging import os @@ -205,12 +206,12 @@ class UserScopedSkillStorage(LocalSkillStorage): from deerflow.config.extensions_config import get_extensions_config extensions_config = get_extensions_config() - for skill in skills: - category = skill.category.value if hasattr(skill.category, "value") else skill.category - if category != SkillCategory.PUBLIC.value: - per_user_state = self.get_skill_enabled_state(skill.name) - global_state = extensions_config.is_skill_enabled(skill.name, category) - skill.enabled = per_user_state and global_state + skills = [ + dataclasses.replace(s, enabled=self.get_skill_enabled_state(s.name) and extensions_config.is_skill_enabled(s.name, s.category.value if hasattr(s.category, "value") else s.category)) + if dataclasses.is_dataclass(s) and not isinstance(s, type) and (s.category.value if hasattr(s.category, "value") else s.category) != SkillCategory.PUBLIC.value + else s + for s in skills + ] if enabled_only: skills = [s for s in skills if s.enabled] diff --git a/backend/packages/harness/deerflow/skills/types.py b/backend/packages/harness/deerflow/skills/types.py index 5d607bf9d..ce78c475f 100644 --- a/backend/packages/harness/deerflow/skills/types.py +++ b/backend/packages/harness/deerflow/skills/types.py @@ -35,7 +35,7 @@ class SecretRequirement: optional: bool = False -@dataclass +@dataclass(frozen=True) class Skill: """Represents a skill with its metadata and file path""" @@ -46,9 +46,9 @@ class Skill: skill_file: Path relative_path: Path # Relative path from category root to skill directory category: SkillCategory # 'public' or 'custom' - allowed_tools: list[str] | None = None + allowed_tools: tuple[str, ...] | None = None enabled: bool = False # Whether this skill is enabled - required_secrets: list[SecretRequirement] = field(default_factory=list) + required_secrets: tuple[SecretRequirement, ...] = field(default_factory=tuple) @property def skill_path(self) -> str: diff --git a/backend/tests/test_checkpointer.py b/backend/tests/test_checkpointer.py index c089ddc93..f305afc0a 100644 --- a/backend/tests/test_checkpointer.py +++ b/backend/tests/test_checkpointer.py @@ -896,12 +896,17 @@ class TestClientCheckpointerFallback: config_mock.get_model_config.return_value = MagicMock(supports_vision=False) config_mock.checkpointer = None + config_mock.skills.deferred_discovery = False + config_mock.skills.container_path = "/mnt/skills" + config_mock.tool_search.enabled = False + with ( patch("deerflow.client.get_app_config", return_value=config_mock), patch("deerflow.client.create_agent", side_effect=fake_create_agent), patch("deerflow.client.create_chat_model", return_value=MagicMock()), patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value=""), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch("deerflow.client.DeerFlowClient._get_tools", return_value=[]), ): client = DeerFlowClient(checkpointer=None) @@ -930,12 +935,17 @@ class TestClientCheckpointerFallback: config_mock.get_model_config.return_value = MagicMock(supports_vision=False) config_mock.checkpointer = None + config_mock.skills.deferred_discovery = False + config_mock.skills.container_path = "/mnt/skills" + config_mock.tool_search.enabled = False + with ( patch("deerflow.client.get_app_config", return_value=config_mock), patch("deerflow.client.create_agent", side_effect=fake_create_agent), patch("deerflow.client.create_chat_model", return_value=MagicMock()), patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value=""), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch("deerflow.client.DeerFlowClient._get_tools", return_value=[]), ): client = DeerFlowClient(checkpointer=explicit_cp) diff --git a/backend/tests/test_client.py b/backend/tests/test_client.py index e1255d7f7..0be07e78b 100644 --- a/backend/tests/test_client.py +++ b/backend/tests/test_client.py @@ -41,6 +41,9 @@ def mock_app_config(): config = MagicMock() config.models = [model] config.token_usage.enabled = False + config.skills.deferred_discovery = False + config.skills.container_path = "/mnt/skills" + config.tool_search.enabled = False return config @@ -916,6 +919,7 @@ class TestEnsureAgent: patch("deerflow.client.create_agent", return_value=mock_agent), patch("deerflow.client.build_middlewares", return_value=[]) as mock_build_middlewares, patch("deerflow.client.apply_prompt_template", return_value="prompt") as mock_apply_prompt, + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=MagicMock()), ): @@ -941,6 +945,7 @@ class TestEnsureAgent: patch("deerflow.client.create_agent", return_value=mock_agent) as mock_create_agent, patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=mock_checkpointer), ): @@ -966,6 +971,7 @@ class TestEnsureAgent: patch("deerflow.client.create_agent", return_value=mock_agent) as mock_create_agent, patch("deerflow.client.build_middlewares", side_effect=fake_build_middlewares), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=MagicMock()), ): @@ -985,6 +991,7 @@ class TestEnsureAgent: patch("deerflow.client.create_agent", return_value=mock_agent) as mock_create_agent, patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=None), ): @@ -1004,6 +1011,82 @@ class TestEnsureAgent: # Should still be the same mock — no recreation assert client._agent is mock_agent + def test_deferred_skill_discovery_wired_when_enabled(self, client, mock_app_config): + """When skills.deferred_discovery=True, skill_names reaches apply_prompt_template + (parity with agent.py — config flag must not be a silent no-op on the embedded path).""" + from pathlib import Path + + from deerflow.skills.types import Skill, SkillCategory + + fake_skill = Skill( + name="deep-research", + description="Multi-source research", + license=None, + skill_dir=Path("/mnt/skills/public/deep-research"), + skill_file=Path("/mnt/skills/public/deep-research/SKILL.md"), + relative_path=Path("deep-research"), + category=SkillCategory.PUBLIC, + enabled=True, + ) + + mock_app_config.skills.deferred_discovery = True + mock_app_config.skills.container_path = "/mnt/skills" + mock_app_config.tool_search.enabled = False + client._app_config = mock_app_config + config = client._get_runnable_config("t1") + + with ( + patch("deerflow.client.create_chat_model"), + patch("deerflow.client.create_agent", return_value=MagicMock()), + patch("deerflow.client.build_middlewares", return_value=[]), + patch("deerflow.client.apply_prompt_template", return_value="prompt") as mock_apply_prompt, + patch.object(client, "_get_tools", return_value=[]), + patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=None), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[fake_skill]), + ): + client._ensure_agent(config) + + skill_names_arg = mock_apply_prompt.call_args.kwargs.get("skill_names") + assert skill_names_arg is not None, "skill_names must be passed when deferred_discovery=True" + assert "deep-research" in skill_names_arg + + def test_deferred_skill_discovery_not_wired_when_disabled(self, client, mock_app_config): + """When skills.deferred_discovery=False, skill_names is None so the legacy prompt path runs.""" + from pathlib import Path + + from deerflow.skills.types import Skill, SkillCategory + + fake_skill = Skill( + name="deep-research", + description="Multi-source research", + license=None, + skill_dir=Path("/mnt/skills/public/deep-research"), + skill_file=Path("/mnt/skills/public/deep-research/SKILL.md"), + relative_path=Path("deep-research"), + category=SkillCategory.PUBLIC, + enabled=True, + ) + + mock_app_config.skills.deferred_discovery = False + mock_app_config.skills.container_path = "/mnt/skills" + mock_app_config.tool_search.enabled = False + client._app_config = mock_app_config + config = client._get_runnable_config("t1") + + with ( + patch("deerflow.client.create_chat_model"), + patch("deerflow.client.create_agent", return_value=MagicMock()), + patch("deerflow.client.build_middlewares", return_value=[]), + patch("deerflow.client.apply_prompt_template", return_value="prompt") as mock_apply_prompt, + patch.object(client, "_get_tools", return_value=[]), + patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=None), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[fake_skill]), + ): + client._ensure_agent(config) + + skill_names_arg = mock_apply_prompt.call_args.kwargs.get("skill_names") + assert skill_names_arg is None, "skill_names must be None when deferred_discovery=False" + # --------------------------------------------------------------------------- # get_model @@ -1997,6 +2080,7 @@ class TestScenarioAgentRecreation: patch("deerflow.client.create_agent", side_effect=fake_create_agent), patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=MagicMock()), ): @@ -2025,6 +2109,7 @@ class TestScenarioAgentRecreation: patch("deerflow.client.create_agent", side_effect=fake_create_agent), patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=MagicMock()), ): @@ -2050,6 +2135,7 @@ class TestScenarioAgentRecreation: patch("deerflow.client.create_agent", side_effect=fake_create_agent), patch("deerflow.client.build_middlewares", return_value=[]), patch("deerflow.client.apply_prompt_template", return_value="prompt"), + patch("deerflow.client.get_enabled_skills_for_config", return_value=[]), patch.object(client, "_get_tools", return_value=[]), patch("deerflow.runtime.checkpointer.get_checkpointer", return_value=MagicMock()), ): diff --git a/backend/tests/test_deferred_tool_crosscontext.py b/backend/tests/test_deferred_tool_crosscontext.py index 0933f1365..e4a1a26ae 100644 --- a/backend/tests/test_deferred_tool_crosscontext.py +++ b/backend/tests/test_deferred_tool_crosscontext.py @@ -137,7 +137,7 @@ def _make_skill(allowed_tools): skill_file=Path("/tmp/s/SKILL.md"), relative_path=Path("s"), category="public", - allowed_tools=allowed_tools, + allowed_tools=tuple(allowed_tools) if allowed_tools is not None else None, enabled=True, ) diff --git a/backend/tests/test_lead_agent_prompt.py b/backend/tests/test_lead_agent_prompt.py index 647e0406a..0d2ce8248 100644 --- a/backend/tests/test_lead_agent_prompt.py +++ b/backend/tests/test_lead_agent_prompt.py @@ -292,8 +292,8 @@ def test_explicit_config_enabled_skills_are_cached_by_config_identity(monkeypatc def load_skills(*, enabled_only): nonlocal load_count - load_count += 1 - assert enabled_only is True + if enabled_only: + load_count += 1 return [make_skill("cached-skill")] return SimpleNamespace(load_skills=load_skills) @@ -417,5 +417,52 @@ def test_system_prompt_template_preserves_placeholders(): "{subagent_section}", "{acp_section}", "{subagent_reminder}", + "{skill_first_reminder}", ): assert ph in template, f"placeholder {ph} accidentally removed" + + +def _make_minimal_app_config(): + return SimpleNamespace( + sandbox=SimpleNamespace(mounts=[]), + skills=SimpleNamespace(container_path="/mnt/skills"), + skill_evolution=SimpleNamespace(enabled=False), + tool_search=SimpleNamespace(enabled=False), + memory=SimpleNamespace(enabled=False, injection_enabled=True, max_injection_tokens=2000), + acp_agents={}, + ) + + +def test_apply_prompt_template_legacy_path_does_not_mention_describe_skill(monkeypatch): + """When skill_names is None (legacy path), critical_reminders must not + reference describe_skill (the tool is not registered in legacy mode).""" + config = _make_minimal_app_config() + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None: SimpleNamespace(load_skills=lambda enabled_only=True: [])) + monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "") + + prompt = prompt_module.apply_prompt_template(app_config=config) + + # Legacy wording — tool-agnostic + assert "Always load the relevant skill" in prompt + # Must NOT reference the deferred tool + assert "describe_skill(name)" not in prompt + + +def test_apply_prompt_template_deferred_path_mentions_describe_skill(monkeypatch): + """When skill_names is provided (deferred path), critical_reminders must + reference describe_skill so the LLM knows how to discover skills.""" + config = _make_minimal_app_config() + monkeypatch.setattr("deerflow.config.get_app_config", lambda: config) + monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None: SimpleNamespace(load_skills=lambda enabled_only=True: [])) + monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "") + + prompt = prompt_module.apply_prompt_template( + app_config=config, + skill_names=frozenset({"data-analysis"}), + ) + + # Deferred wording — references describe_skill + assert "describe_skill(name)" in prompt + # Must NOT contain the legacy wording + assert "Always load the relevant skill" not in prompt diff --git a/backend/tests/test_lead_agent_skills.py b/backend/tests/test_lead_agent_skills.py index 689ec3dae..40591c2c8 100644 --- a/backend/tests/test_lead_agent_skills.py +++ b/backend/tests/test_lead_agent_skills.py @@ -20,7 +20,7 @@ def _make_skill(name: str, allowed_tools: list[str] | None = None) -> Skill: skill_file=Path(f"/tmp/{name}/SKILL.md"), relative_path=Path(name), category="public", - allowed_tools=allowed_tools, + allowed_tools=tuple(allowed_tools) if allowed_tools is not None else None, enabled=True, ) @@ -170,6 +170,61 @@ def test_get_skills_prompt_section_uses_explicit_config_for_enabled_skills(monke assert "global-skill" not in result +def test_get_skills_prompt_section_deferred_path_uses_skill_index(monkeypatch): + """When skill_names is provided, renders instead of .""" + skills = [_make_skill("data-analysis"), _make_skill("deep-research")] + monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills) + monkeypatch.setattr( + "deerflow.config.get_app_config", + lambda: SimpleNamespace( + skills=SimpleNamespace(container_path="/mnt/skills"), + skill_evolution=SimpleNamespace(enabled=False), + ), + ) + # Deferred path never touches storage, but patch defensively in case of fallback. + _null_storage = SimpleNamespace(load_skills=lambda *, enabled_only: []) + monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kw: _null_storage) + monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda *a, **kw: _null_storage) + + # Deferred path: skill_names provided + result = get_skills_prompt_section( + available_skills=None, + skill_names=frozenset({"data-analysis", "deep-research"}), + ) + assert "" in result + assert "data-analysis" in result + assert "deep-research" in result + assert "describe_skill" in result + # Must NOT contain legacy full-metadata format + assert "" not in result + assert "Description for data-analysis" not in result # descriptions excluded from index + + +def test_get_skills_prompt_section_legacy_path_when_skill_names_none(monkeypatch): + """When skill_names is None, falls back to legacy rendering.""" + skills = [_make_skill("data-analysis")] + monkeypatch.setattr("deerflow.agents.lead_agent.prompt._get_enabled_skills", lambda: skills) + monkeypatch.setattr( + "deerflow.config.get_app_config", + lambda: SimpleNamespace( + skills=SimpleNamespace(container_path="/mnt/skills"), + skill_evolution=SimpleNamespace(enabled=False), + ), + ) + # Legacy path loads ALL skills (enabled + disabled) from storage for the disabled-skills section. + _storage = SimpleNamespace(load_skills=lambda *, enabled_only: skills) + monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_skill_storage", lambda **kw: _storage) + monkeypatch.setattr("deerflow.agents.lead_agent.prompt.get_or_new_user_skill_storage", lambda *a, **kw: _storage) + + # Legacy path: skill_names not provided + result = get_skills_prompt_section(available_skills=None) + assert "" in result + assert "data-analysis" in result + assert "Description for data-analysis" in result + assert "" not in result + assert "describe_skill" not in result + + def test_make_lead_agent_empty_skills_passed_correctly(monkeypatch): from unittest.mock import MagicMock @@ -231,11 +286,17 @@ def test_make_lead_agent_filters_tools_from_available_skills(monkeypatch): mock_app_config = MagicMock() mock_app_config.get_model_config.return_value = SimpleNamespace(supports_thinking=False, supports_vision=False) + mock_app_config.tool_search.enabled = True + mock_app_config.skills.container_path = "/mnt/skills" + mock_app_config.skills.deferred_discovery = True # describe_skill will be added monkeypatch.setattr(lead_agent_module, "get_app_config", lambda: mock_app_config) agent_kwargs = lead_agent_module.make_lead_agent({"configurable": {"agent_name": "test"}}) - assert [tool.name for tool in agent_kwargs["tools"]] == ["read_file", "web_search"] + # With skills.deferred_discovery=True, describe_skill is added to tools + tool_names = [tool.name for tool in agent_kwargs["tools"]] + assert "read_file" in tool_names + assert "describe_skill" in tool_names def test_skill_allowed_tools_default_does_not_preserve_read_file_for_subagents(): @@ -269,7 +330,9 @@ def test_make_lead_agent_all_legacy_skills_preserve_all_tools(monkeypatch): agent_kwargs = lead_agent_module.make_lead_agent({"configurable": {"agent_name": "test"}}) - assert [tool.name for tool in agent_kwargs["tools"]] == ["bash", "read_file", "update_agent"] + # describe_skill is appended after skill-allowed-tools filtering (it bypasses policy). + tool_names = [tool.name for tool in agent_kwargs["tools"]] + assert tool_names == ["bash", "read_file", "update_agent", "describe_skill"] def test_make_lead_agent_enforces_allowed_tools_when_skill_cache_is_cold(monkeypatch): @@ -298,7 +361,9 @@ def test_make_lead_agent_enforces_allowed_tools_when_skill_cache_is_cold(monkeyp agent_kwargs = lead_agent_module.make_lead_agent({"configurable": {"agent_name": "test"}}) - assert [tool.name for tool in agent_kwargs["tools"]] == ["read_file"] + # describe_skill is appended after skill-allowed-tools filtering (it bypasses policy). + tool_names = [tool.name for tool in agent_kwargs["tools"]] + assert tool_names == ["read_file", "describe_skill"] def test_make_lead_agent_fails_closed_when_skill_policy_load_fails(monkeypatch): diff --git a/backend/tests/test_skill_catalog.py b/backend/tests/test_skill_catalog.py new file mode 100644 index 000000000..4ea742d33 --- /dev/null +++ b/backend/tests/test_skill_catalog.py @@ -0,0 +1,193 @@ +"""Tests for SkillCatalog — deferred skill discovery search engine.""" + +from pathlib import Path + +import pytest + +from deerflow.skills.catalog import MAX_RESULTS, SkillCatalog +from deerflow.skills.types import Skill, SkillCategory + +# ── Fixtures ────────────────────────────────────────────────────────────────── + + +def _make_skill( + name: str, + description: str = "A skill", + category: SkillCategory = SkillCategory.PUBLIC, + allowed_tools: tuple[str, ...] | None = None, +) -> Skill: + """Create a minimal Skill for testing.""" + base = Path("/mnt/skills") / category.value / name + return Skill( + name=name, + description=description, + license=None, + skill_dir=base, + skill_file=base / "SKILL.md", + relative_path=Path(name), + category=category, + allowed_tools=allowed_tools, + enabled=True, + ) + + +@pytest.fixture +def sample_skills() -> list[Skill]: + return [ + _make_skill("data-analysis", "Analyze data with Python, pandas, jupyter"), + _make_skill("deep-research", "Conduct multi-source research with fact-checking"), + _make_skill("chart-visualization", "Visualize data with interactive charts"), + _make_skill("podcast-generation", "Generate podcast scripts and audio"), + _make_skill("music-generation", "Generate music compositions"), + _make_skill("video-generation", "Generate video from text prompts"), + _make_skill("image-generation", "Generate images from descriptions"), + _make_skill("ppt-generation", "Generate PowerPoint presentations"), + _make_skill("custom-analyzer", "Custom data analyzer", category=SkillCategory.CUSTOM), + ] + + +@pytest.fixture +def catalog(sample_skills: list[Skill]) -> SkillCatalog: + return SkillCatalog(tuple(sample_skills)) + + +# ── Name property ───────────────────────────────────────────────────────────── + + +def test_names_returns_frozenset(catalog: SkillCatalog): + assert isinstance(catalog.names, frozenset) + + +def test_names_contains_all_skills(catalog: SkillCatalog, sample_skills: list[Skill]): + expected = {s.name for s in sample_skills} + assert catalog.names == expected + + +def test_empty_catalog_names(): + catalog = SkillCatalog(()) + assert catalog.names == frozenset() + + +# ── Exact selection (select:) ───────────────────────────────────────────────── + + +def test_select_single(catalog: SkillCatalog): + result = catalog.search("select:data-analysis") + assert len(result) == 1 + assert result[0].name == "data-analysis" + + +def test_select_multiple(catalog: SkillCatalog): + result = catalog.search("select:data-analysis,deep-research") + names = {s.name for s in result} + assert names == {"data-analysis", "deep-research"} + + +def test_select_nonexistent(catalog: SkillCatalog): + result = catalog.search("select:nonexistent-skill") + assert result == [] + + +def test_select_partial_match(catalog: SkillCatalog): + """select: with one valid and one invalid name returns only the valid one.""" + result = catalog.search("select:data-analysis,nonexistent") + assert len(result) == 1 + assert result[0].name == "data-analysis" + + +def test_select_returns_all_requested(catalog: SkillCatalog, sample_skills: list[Skill]): + """select: returns all requested names without capping — exact selection, not ranked search.""" + all_names = ",".join(sorted(catalog.names)) + result = catalog.search(f"select:{all_names}") + assert len(result) == len(sample_skills) + + +# ── Required-prefix search (+) ──────────────────────────────────────────────── + + +def test_required_prefix_filters_by_name(catalog: SkillCatalog): + result = catalog.search("+podcast") + assert all("podcast" in s.name for s in result) + + +def test_required_prefix_with_ranking(catalog: SkillCatalog): + """'+gen generation' should require 'gen' in name, rank by 'generation'.""" + result = catalog.search("+gen generation") + assert all("gen" in s.name for s in result) + + +def test_required_prefix_bare_plus(catalog: SkillCatalog): + """Bare '+' with no token returns empty.""" + result = catalog.search("+") + assert result == [] + + +def test_required_prefix_no_match(catalog: SkillCatalog): + result = catalog.search("+zzz_nonexistent") + assert result == [] + + +# ── Free-text regex search ──────────────────────────────────────────────────── + + +def test_keyword_matches_name(catalog: SkillCatalog): + result = catalog.search("podcast") + assert any(s.name == "podcast-generation" for s in result) + + +def test_keyword_matches_description(catalog: SkillCatalog): + """Description match should also be returned.""" + result = catalog.search("pandas") + assert any(s.name == "data-analysis" for s in result) + + +def test_name_match_scores_higher_than_description(catalog: SkillCatalog): + """When both name and description match, name match should rank first.""" + # 'data-analysis' name matches 'data', description also matches 'data' + # 'deep-research' description matches 'data' (no, it doesn't) + # Let's use 'chart' — matches chart-visualization by name + result = catalog.search("chart") + assert result[0].name == "chart-visualization" + + +def test_regex_case_insensitive(catalog: SkillCatalog): + result_lower = catalog.search("data") + result_upper = catalog.search("DATA") + assert {s.name for s in result_lower} == {s.name for s in result_upper} + + +def test_invalid_regex_falls_back_to_literal(catalog: SkillCatalog): + """Unbalanced paren should degrade to literal match, not raise.""" + result = catalog.search("(invalid") + # Should not raise; may or may not match anything + assert isinstance(result, list) + + +def test_empty_query(catalog: SkillCatalog): + result = catalog.search("") + assert result == [] + + +def test_whitespace_only_query(catalog: SkillCatalog): + result = catalog.search(" ") + assert result == [] + + +def test_max_results_cap(catalog: SkillCatalog): + """Free-text search should cap results at MAX_RESULTS.""" + # 'generation' matches many descriptions + result = catalog.search("generation") + assert len(result) <= MAX_RESULTS + + +# ── Edge cases ──────────────────────────────────────────────────────────────── + + +def test_frozen_catalog_is_hashable(catalog: SkillCatalog): + """SkillCatalog with real skills must be hashable (frozen=True on both Skill and SkillCatalog).""" + assert hash(catalog) is not None + + +def test_names_cached_property_stable(catalog: SkillCatalog): + """Multiple accesses to .names should return the same frozenset.""" + assert catalog.names is catalog.names diff --git a/backend/tests/test_skill_describe.py b/backend/tests/test_skill_describe.py new file mode 100644 index 000000000..38baf0a4e --- /dev/null +++ b/backend/tests/test_skill_describe.py @@ -0,0 +1,282 @@ +"""Tests for describe_skill tool and skill index prompt rendering.""" + +from pathlib import Path + +import pytest + +from deerflow.skills.catalog import SkillCatalog +from deerflow.skills.describe import ( + _render_skill_metadata, + build_describe_skill_tool, + build_skill_search_setup, + get_skill_index_prompt_section, +) +from deerflow.skills.types import Skill, SkillCategory + +# ── Helpers ──────────────────────────────────────────────────────────────────── + + +def _make_skill( + name: str, + description: str = "A skill", + category: SkillCategory = SkillCategory.PUBLIC, + allowed_tools: tuple[str, ...] | None = None, +) -> Skill: + base = Path("/mnt/skills") / category.value / name + return Skill( + name=name, + description=description, + license=None, + skill_dir=base, + skill_file=base / "SKILL.md", + relative_path=Path(name), + category=category, + allowed_tools=allowed_tools, + enabled=True, + ) + + +@pytest.fixture +def sample_skills() -> list[Skill]: + return [ + _make_skill("data-analysis", "Analyze data with Python", allowed_tools=("execute_code", "read_file")), + _make_skill("deep-research", "Multi-source research"), + _make_skill("custom-analyzer", "Custom analyzer", category=SkillCategory.CUSTOM), + ] + + +@pytest.fixture +def catalog(sample_skills: list[Skill]) -> SkillCatalog: + return SkillCatalog(tuple(sample_skills)) + + +# ── _render_skill_metadata ──────────────────────────────────────────────────── + + +def test_render_metadata_format(sample_skills: list[Skill]): + rendered = _render_skill_metadata(sample_skills[:1], "/mnt/skills") + assert "## Skill: data-analysis" in rendered + assert "Description: Analyze data with Python" in rendered + assert "[built-in]" in rendered + assert "Allowed tools: execute_code, read_file" in rendered + assert "Location: /mnt/skills/public/data-analysis/SKILL.md" in rendered + + +def test_render_custom_skill_mutability(sample_skills: list[Skill]): + custom = [s for s in sample_skills if s.category == SkillCategory.CUSTOM] + rendered = _render_skill_metadata(custom, "/mnt/skills") + assert "[custom, editable]" in rendered + + +def test_render_no_allowed_tools_shows_all(sample_skills: list[Skill]): + """Skills without allowed_tools should show '(all)'.""" + no_tools = [s for s in sample_skills if s.allowed_tools is None] + rendered = _render_skill_metadata(no_tools[:1], "/mnt/skills") + assert "Allowed tools: (all)" in rendered + + +def test_render_multiple_skills(sample_skills: list[Skill]): + rendered = _render_skill_metadata(sample_skills, "/mnt/skills") + assert "## Skill: data-analysis" in rendered + assert "## Skill: deep-research" in rendered + assert "## Skill: custom-analyzer" in rendered + + +# ── build_describe_skill_tool ───────────────────────────────────────────────── + + +def test_describe_tool_is_invokable(catalog: SkillCatalog): + tool = build_describe_skill_tool(catalog) + assert tool.name == "describe_skill" + assert hasattr(tool, "invoke") + + +def test_describe_tool_docstring(catalog: SkillCatalog): + tool = build_describe_skill_tool(catalog) + assert "describe_skill" in tool.name + assert tool.description is not None + + +def test_describe_skill_parameter_name_matches_prompt(catalog: SkillCatalog): + """Regression: the tool parameter must be 'name', matching the prompt wording + 'describe_skill(name)'. A strict function-calling model submits exactly the + parameter name the prompt specifies — any drift silently breaks the flow. + """ + tool = build_describe_skill_tool(catalog) + schema = tool.get_input_schema().model_json_schema() + assert "name" in schema["properties"], "tool must accept 'name' (matching prompt wording)" + assert "query" not in schema["properties"], "old 'query' parameter must not exist" + + +# ── build_skill_search_setup ────────────────────────────────────────────────── + + +def test_setup_enabled_with_skills(sample_skills: list[Skill]): + setup = build_skill_search_setup(sample_skills, enabled=True) + assert setup.describe_skill_tool is not None + assert setup.skill_names == frozenset(s.name for s in sample_skills) + + +def test_setup_disabled(): + setup = build_skill_search_setup([_make_skill("a", "A")], enabled=False) + assert setup.describe_skill_tool is None + assert setup.skill_names == frozenset() + + +def test_setup_empty_skills(): + setup = build_skill_search_setup([], enabled=True) + assert setup.describe_skill_tool is None + assert setup.skill_names == frozenset() + + +def test_setup_frozen(): + """Empty SkillSearchSetup (describe_skill_tool=None) must be hashable. + + The populated setup contains a BaseTool, which is not hashable by design — + so only the disabled/empty path is required to hash. frozen=True still + prevents accidental mutation in both cases. + """ + setup = build_skill_search_setup([], enabled=True) + assert hash(setup) is not None + + +# ── get_skill_index_prompt_section ──────────────────────────────────────────── + + +def test_skill_index_contains_names(): + section = get_skill_index_prompt_section( + skill_names=frozenset({"data-analysis", "deep-research"}), + ) + assert "" in section + assert "data-analysis" in section + assert "deep-research" in section + + +def test_skill_index_no_description(): + """Index should NOT contain descriptions (that's the whole point).""" + section = get_skill_index_prompt_section( + skill_names=frozenset({"data-analysis"}), + ) + assert "Analyze data with Python" not in section + + +def test_skill_index_no_location(): + """Index should NOT contain file paths.""" + section = get_skill_index_prompt_section( + skill_names=frozenset({"data-analysis"}), + ) + assert "/mnt/skills/public/data-analysis/SKILL.md" not in section + + +def test_skill_index_contains_discovery_instructions(): + section = get_skill_index_prompt_section( + skill_names=frozenset({"data-analysis"}), + ) + assert "describe_skill" in section + assert "Skill Discovery" in section + + +def test_skill_index_empty_returns_empty(): + section = get_skill_index_prompt_section(skill_names=frozenset()) + assert section == "" + + +def test_skill_index_default_returns_empty(): + section = get_skill_index_prompt_section() + assert section == "" + + +def test_skill_index_with_evolution_section(): + section = get_skill_index_prompt_section( + skill_names=frozenset({"a"}), + skill_evolution_section="## Skill Self-Evolution\n...", + ) + assert "Skill Self-Evolution" in section + + +def test_skill_index_without_evolution_section(): + section = get_skill_index_prompt_section( + skill_names=frozenset({"a"}), + skill_evolution_section="", + ) + assert "Skill Self-Evolution" not in section + + +def test_skill_index_custom_container_path(): + section = get_skill_index_prompt_section( + skill_names=frozenset({"a"}), + container_base_path="/custom/skills", + ) + assert "/custom/skills" in section + + +def test_skill_index_names_are_sorted(): + """Names should be sorted for deterministic output.""" + section = get_skill_index_prompt_section( + skill_names=frozenset({"z-skill", "a-skill", "m-skill"}), + ) + # Extract just the block content + import re + + match = re.search(r"\n(.*?)\n", section, re.DOTALL) + assert match is not None + names_str = match.group(1).strip() + names = [n.strip() for n in names_str.split(",")] + assert names == sorted(names) + + +# ── Integration: describe_skill tool invocation ─────────────────────────────── + + +def test_describe_tool_returns_command_with_tool_message(catalog: SkillCatalog): + """describe_skill should return a Command with a ToolMessage.""" + tool = build_describe_skill_tool(catalog) + + # Tools with InjectedToolCallId must be invoked with a full ToolCall dict + result = tool.invoke( + {"args": {"name": "select:data-analysis"}, "name": "describe_skill", "type": "tool_call", "id": "test_call_123"}, + ) + + # Result is a Command wrapping a ToolMessage + messages = result.update["messages"] + assert len(messages) == 1 + msg = messages[0] + assert msg.name == "describe_skill" + assert msg.tool_call_id == "test_call_123" + assert "## Skill: data-analysis" in msg.content + + +def test_describe_tool_no_match(catalog: SkillCatalog): + tool = build_describe_skill_tool(catalog) + result = tool.invoke( + {"args": {"name": "xyz_nonexistent"}, "name": "describe_skill", "type": "tool_call", "id": "test_call_456"}, + ) + messages = result.update["messages"] + assert "No skills matched" in messages[0].content + + +def test_describe_tool_keyword_search(catalog: SkillCatalog): + tool = build_describe_skill_tool(catalog) + result = tool.invoke( + {"args": {"name": "research"}, "name": "describe_skill", "type": "tool_call", "id": "test_call_789"}, + ) + messages = result.update["messages"] + assert "deep-research" in messages[0].content + + +def test_describe_tool_select_uncapped(tmp_path): + """select: must return ALL requested skills, not capped at MAX_RESULTS.""" + from deerflow.skills.catalog import MAX_RESULTS + + # Build more skills than MAX_RESULTS so the cap would visibly truncate + many_skills = [_make_skill(f"skill-{i:02d}") for i in range(MAX_RESULTS + 2)] + big_catalog = SkillCatalog(tuple(many_skills)) + tool = build_describe_skill_tool(big_catalog) + + names_csv = ",".join(s.name for s in many_skills) + result = tool.invoke( + {"args": {"name": f"select:{names_csv}"}, "name": "describe_skill", "type": "tool_call", "id": "test_select_uncapped"}, + ) + content = result.update["messages"][0].content + for s in many_skills: + assert s.name in content, f"select: truncated — {s.name} missing from result" diff --git a/backend/tests/test_skill_request_scoped_secrets.py b/backend/tests/test_skill_request_scoped_secrets.py index 0df85aa37..4b0fa2c77 100644 --- a/backend/tests/test_skill_request_scoped_secrets.py +++ b/backend/tests/test_skill_request_scoped_secrets.py @@ -228,7 +228,7 @@ class TestRequiredSecretsParsing: skill_file = self._write_skill(tmp_path, "name: erp-report\ndescription: Pull an ERP report") skill = parse_skill_file(skill_file, SkillCategory.CUSTOM) assert skill is not None - assert skill.required_secrets == [] + assert skill.required_secrets == () def test_string_list_form(self, tmp_path): from deerflow.skills.parser import parse_skill_file diff --git a/backend/tests/test_skills_parser.py b/backend/tests/test_skills_parser.py index 52172125f..aecb36697 100644 --- a/backend/tests/test_skills_parser.py +++ b/backend/tests/test_skills_parser.py @@ -98,14 +98,14 @@ def test_parse_allowed_tools_list(tmp_path): skill_file = _write_skill(tmp_path, 'name: my-skill\ndescription: Test\nallowed-tools: ["bash", "read_file"]') skill = parse_skill_file(skill_file, category="custom") assert skill is not None - assert skill.allowed_tools == ["bash", "read_file"] + assert skill.allowed_tools == ("bash", "read_file") def test_parse_empty_allowed_tools_list(tmp_path): skill_file = _write_skill(tmp_path, "name: my-skill\ndescription: Test\nallowed-tools: []") skill = parse_skill_file(skill_file, category="custom") assert skill is not None - assert skill.allowed_tools == [] + assert skill.allowed_tools == () def test_parse_invalid_allowed_tools_returns_none(tmp_path): diff --git a/backend/tests/test_slash_skills.py b/backend/tests/test_slash_skills.py index 6afa749f3..9cb667780 100644 --- a/backend/tests/test_slash_skills.py +++ b/backend/tests/test_slash_skills.py @@ -106,8 +106,9 @@ def test_resolve_slash_skill_respects_available_skill_whitelist(tmp_path): def test_resolve_slash_skill_rejects_disabled_skills(tmp_path): - skill = _make_skill(tmp_path, "data-analysis") - skill.enabled = False + import dataclasses + + skill = dataclasses.replace(_make_skill(tmp_path, "data-analysis"), enabled=False) assert resolve_slash_skill("/data-analysis run", [skill]) is None @@ -466,8 +467,9 @@ def test_skill_activation_middleware_returns_clear_error_for_missing_skill(monke def test_skill_activation_middleware_returns_clear_error_for_disabled_skill(monkeypatch, tmp_path): - skill = _make_skill(tmp_path, "data-analysis") - skill.enabled = False + import dataclasses + + skill = dataclasses.replace(_make_skill(tmp_path, "data-analysis"), enabled=False) monkeypatch.setattr(middleware_module, "get_or_new_skill_storage", lambda **kwargs: _make_storage(tmp_path, [skill])) middleware = SkillActivationMiddleware() diff --git a/backend/tests/test_subagent_executor.py b/backend/tests/test_subagent_executor.py index 1473ff42e..b6da865bb 100644 --- a/backend/tests/test_subagent_executor.py +++ b/backend/tests/test_subagent_executor.py @@ -165,7 +165,7 @@ def _skill(name: str, allowed_tools: list[str] | None) -> Skill: skill_file=skill_dir / "SKILL.md", relative_path=Path(name), category="custom", - allowed_tools=allowed_tools, + allowed_tools=tuple(allowed_tools) if allowed_tools is not None else None, enabled=True, ) diff --git a/config.example.yaml b/config.example.yaml index 79ea3dafb..03db0f47b 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -1180,6 +1180,13 @@ skills: # Default: /mnt/skills container_path: /mnt/skills + # Deferred skill discovery (default: false) + # When enabled, only skill names appear in the system prompt (). + # The LLM discovers skill details on demand via the describe_skill tool. + # This keeps the system prompt compact and prefix-cache friendly when many + # skills are installed. + # deferred_discovery: true + # Note: To restrict which skills are loaded for a specific custom agent, # define a `skills` list in that agent's `config.yaml` (e.g. `agents/my-agent/config.yaml`): # - Omitted or null: load all globally enabled skills (default)