feat: agent-teams part B - expert-skill backend mechanism (#370)

* feat: add expert-skill schema and type filter to skill_manager

* feat: fold installed expert skills into main-agent subagent registry

* feat: add GET /api/teams listing expert skills for gallery

* chore: cache SKILL.md body on SkillInfo, cleanup expert-container comments

* fix: register skill_manager in expert-subagent tool_registry

* fix: catch UnicodeDecodeError in expert-skill body loader

* fix: guard expert subagent registration against name collisions

* fix: skip expert registration when SKILL.md body is empty

* fix: drop redundant str() guards on expert-skill frontmatter

* fix: harden SKILL.md parsing on expert-registration hot path
This commit is contained in:
jfilipiuk
2026-07-24 13:59:52 +02:00
committed by Xi Zhang
parent b5b01d50c2
commit 3a1dbf0a0f
8 changed files with 1532 additions and 26 deletions
+45 -2
View File
@@ -407,6 +407,39 @@ def _ensure_general_purpose_subagent(subs: list[dict]) -> None:
)
def _fold_expert_subagents(subs: list[dict], tool_registry: dict) -> None:
"""Append expert-skill sub-agent specs to ``subs``, guarding names.
Each installed expert skill becomes an in-process sub-agent entry so
the main agent's ``task`` tool (and the QuickJS ``task()`` global for
panel mode) can dispatch to it by name. Async-graph deploy of experts
is v2 territory — they live purely in the sync in-process registry.
Skips (with a warning) any expert whose ``name`` collides with a
subagent already in ``subs`` or with ``general-purpose``. The reserved
name matters because ``_ensure_general_purpose_subagent`` runs right
after this and early-returns when it sees the slot occupied — an expert
named ``general-purpose`` would silently take the slot and deepagents'
default subagent prompt would never reach the agent.
"""
from deepagents.middleware.subagents import GENERAL_PURPOSE_SUBAGENT
from .subagents.expert_container import build_expert_subagent_specs
logger = logging.getLogger(__name__)
taken = {s.get("name") for s in subs} | {GENERAL_PURPOSE_SUBAGENT["name"]}
for spec in build_expert_subagent_specs(tool_registry=tool_registry):
name = spec["name"]
if name in taken:
logger.warning(
"Expert skill %r collides with an existing sub-agent name; skipping.",
name,
)
continue
taken.add(name)
subs.append(spec)
def _maybe_swap_async_subagents(
subs: list, middleware: list | None = None, *, cfg=None
) -> list:
@@ -524,7 +557,12 @@ def _build_base_kwargs(
from .utils import load_subagents
cfg = cfg if cfg is not None else _ensure_config()
tool_registry = {"think_tool": think_tool}
# `skill_manager` is registered here in addition to `base_tools` because
# expert subagents resolve their default toolset from `tool_registry` (see
# `_DEFAULT_EXPERT_TOOLS` in expert_container.py). Without this entry the
# tool silently misses from every expert sub-agent — e.g. idea-brainstorm
# can't run its `paper-navigator` precondition check.
tool_registry = {"think_tool": think_tool, "skill_manager": skill_manager}
if os.environ.get("TAVILY_API_KEY"):
tool_registry["tavily_search"] = tavily_search
base_tools = [think_tool, skill_manager]
@@ -538,6 +576,7 @@ def _build_base_kwargs(
tool_registry=tool_registry,
async_swap_pending=True,
)
_fold_expert_subagents(subs, tool_registry)
_ensure_general_purpose_subagent(subs)
_inject_subagent_middleware(
subs, workspace_dir=workspace_dir, cfg=cfg, chat_model=chat_model
@@ -595,7 +634,10 @@ def load_mcp_and_build_kwargs(
workspace_dir=workspace_dir,
)
tool_registry = {"think_tool": think_tool}
# Match `_build_base_kwargs`: register `skill_manager` in the registry so
# expert subagents (which resolve tools via `_DEFAULT_EXPERT_TOOLS` from
# `expert_container.py`) actually get it.
tool_registry = {"think_tool": think_tool, "skill_manager": skill_manager}
if os.environ.get("TAVILY_API_KEY"):
tool_registry["tavily_search"] = tavily_search
base_tools = [think_tool, skill_manager]
@@ -615,6 +657,7 @@ def load_mcp_and_build_kwargs(
tool_registry=registry,
async_swap_pending=True,
)
_fold_expert_subagents(subs, registry)
_ensure_general_purpose_subagent(subs)
_inject_subagent_middleware(
+50
View File
@@ -75,8 +75,58 @@ async def get_models(_request: Request) -> JSONResponse:
)
async def get_teams(_request: Request) -> JSONResponse:
"""Return installed expert skills as ``{teams: [...]}`` for the WebUI gallery.
A "team" in the WebUI vocabulary is an installed expert skill —
``SKILL.md`` with ``type: expert`` frontmatter. The response is a
curated, gallery-safe projection: name + description, plus optional
``byline`` / ``capability_tags`` / ``avatar_hint`` when the skill
populates them.
Backend implementation details (SKILL.md body / system prompt, role
line, default_dispatch, tool list, source tier, filesystem path,
tags) are intentionally NOT projected. The gallery only needs
identity + descriptor fields to render the card; anything richer
belongs in a dedicated info endpoint.
Sourced from ``list_expert_skills(include_system=True)`` so
first-party experts shipped as builtin skills surface alongside
workspace/global installs.
Offloaded to a thread because the skill loader does synchronous
filesystem walking + yaml parsing, which langgraph-dev's
``blockbuster`` middleware refuses on the async event loop.
Response shape (each entry): ``{name, description, byline?,
capability_tags?, avatar_hint?}`` — the WebUI gallery consumes these.
"""
from EvoScientist.tools.skills_manager import list_expert_skills
experts = await asyncio.to_thread(list_expert_skills, True)
teams = []
for info in experts:
entry = {
"name": info.name,
"description": info.description,
}
# Optional gallery fields — omit when unpopulated so the WebUI
# card degrades gracefully (SkillInfo defaults `byline` /
# `avatar_hint` to "" and `capability_tags` to [], which we
# treat as "not declared").
if info.byline:
entry["byline"] = info.byline
if info.capability_tags:
entry["capability_tags"] = list(info.capability_tags)
if info.avatar_hint:
entry["avatar_hint"] = info.avatar_hint
teams.append(entry)
return JSONResponse({"teams": teams})
app = Starlette(
routes=[
Route("/api/models", get_models, methods=["GET"]),
Route("/api/teams", get_teams, methods=["GET"]),
]
)
+156
View File
@@ -0,0 +1,156 @@
"""Expert-subagent-spec factory for the v1 agent-teams feature.
Turns an installed **expert skill** (a `SkillInfo` with `type == "expert"`) into
a deepagents subagent spec dict compatible with `subagents=[...]` on
`create_deep_agent`. The main agent's `_build_base_kwargs` folds these specs
into its subagent list at construction time so the `task` tool can dispatch to
each installed expert for sync consult; the same registry is reused by the
QuickJS `task()` global for panel mode.
The generic-container principle from #361 lives in THIS FUNCTION — one
construction path for all experts, sourcing behaviour from the skill file
rather than a per-expert YAML. There's no deployed graph per expert in v1;
that's async-thread territory (v2) and blocked on the deepagents
`AsyncSubAgent` config-passthrough gap.
"""
from __future__ import annotations
import logging
from typing import Any
from ..tools.skills_manager import SkillInfo, _split_frontmatter_and_body
_logger = logging.getLogger(__name__)
# Default toolset for expert subagents. Kept minimal — most experts are
# "reason about the incoming description and produce structured output";
# they can reach installed utility skills via the `/skills/` mount.
#
# `skill_manager` is included so experts can inspect what utility skills are
# available at runtime (e.g. `idea-brainstorm` checks for `paper-navigator`
# before starting its literature-review phase). Widening beyond these two
# defaults should be a deliberate decision (e.g. adding `execute` only when
# we know experts need to run scripts — deepagents' built-in file/execute
# tools are already available regardless of this list).
_DEFAULT_EXPERT_TOOLS: tuple[str, ...] = ("think_tool", "skill_manager")
# Default skills mount — expert subagents get the same read-only skills view
# as any other subagent (matches `research.yaml` / `writing.yaml` shape).
_DEFAULT_EXPERT_SKILLS: tuple[str, ...] = ("/skills/",)
def _body_of(skill_info: SkillInfo) -> str:
"""Return the SKILL.md body (post-frontmatter content).
Prefers the body cached on ``SkillInfo`` by ``_parse_skill_md``. Falls
back to reading SKILL.md fresh if the cached body is empty — that
handles skills constructed by hand (external callers) without a body
field populated. Returns an empty string if the file can't be read.
"""
if skill_info.body:
return skill_info.body
skill_md = skill_info.path / "SKILL.md"
try:
content = skill_md.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as exc:
_logger.warning(
"Expert skill %r: could not read SKILL.md at %s (%s)",
skill_info.name,
skill_md,
exc,
)
return ""
_, body = _split_frontmatter_and_body(content)
return body
def _compose_system_prompt(skill_info: SkillInfo, body: str) -> str:
"""Compose the expert's system_prompt from its role + SKILL.md body.
The `role` frontmatter (one-line role summary) is prepended as an
orientation line; the body carries the persona voice, rubrics, and
output-style instructions (all written in second person addressing the
expert itself, per the expert-skill authoring convention).
"""
if skill_info.role:
return f"You are {skill_info.role}.\n\n{body}".rstrip() + "\n"
return body if body.endswith("\n") else body + "\n"
def build_expert_subagent_spec(
skill_info: SkillInfo,
tool_registry: dict[str, Any] | None = None,
) -> dict[str, Any]:
"""Build a deepagents subagent spec dict from an expert skill.
Args:
skill_info: An expert skill (``type == "expert"``). The caller is
responsible for filtering — passing a utility skill here builds a
spec anyway (utility skills just don't have persona content in
the body, so the result is nonsensical rather than broken).
tool_registry: Same registry `load_subagents` uses to resolve tool
names to callables (e.g. `{"think_tool": think_tool, ...}`).
Unresolved tools are skipped with a warning, matching
`_build_one` in `EvoScientist/utils.py`.
Returns:
A subagent spec dict with the same shape ``load_subagents`` produces:
``{name, description, system_prompt, tools, skills, _async}``. Ready
to append to the main agent's `subagents=[...]` list.
"""
tool_registry = tool_registry or {}
body = _body_of(skill_info)
system_prompt = _compose_system_prompt(skill_info, body)
resolved_tools: list[Any] = []
for tool_name in _DEFAULT_EXPERT_TOOLS:
if tool_name in tool_registry:
resolved_tools.append(tool_registry[tool_name])
else:
_logger.warning(
"Expert skill %r: default tool %r not in registry, skipping",
skill_info.name,
tool_name,
)
return {
"name": skill_info.name,
"description": skill_info.description,
"system_prompt": system_prompt,
"tools": resolved_tools,
"skills": list(_DEFAULT_EXPERT_SKILLS),
# v1 is sync-consult + panel only; both use the in-process subagent
# registry, not the async graph path. Async-thread mode = v2.
"_async": False,
}
def build_expert_subagent_specs(
tool_registry: dict[str, Any] | None = None,
*,
include_system: bool = True,
) -> list[dict[str, Any]]:
"""Build spec dicts for every installed expert skill.
Thin wrapper over ``list_expert_skills()`` + ``build_expert_subagent_spec``.
Called by the main-agent construction path (``_build_base_kwargs``) to
fold experts into the ``subagents=[...]`` list.
Skips (with a warning) any expert whose SKILL.md body is empty — a
personaless expert advertised in the ``task`` tool schema would let the
orchestrator dispatch to a blank system prompt, a worse failure mode
than the expert being absent.
"""
from ..tools.skills_manager import list_expert_skills
specs: list[dict[str, Any]] = []
for info in list_expert_skills(include_system=include_system):
if not _body_of(info).strip():
_logger.warning(
"Expert skill %r: SKILL.md body is empty; skipping registration.",
info.name,
)
continue
specs.append(build_expert_subagent_spec(info, tool_registry=tool_registry))
return specs
+38 -8
View File
@@ -13,6 +13,7 @@ def skill_manager(
name: str = "",
tag: str = "",
include_system: bool = False,
skill_type: Literal["all", "expert", "utility"] = "all",
) -> str:
"""Manage user-installable skills: install from GitHub or local path, list available skills, browse remote skills, get details, or uninstall.
@@ -28,6 +29,8 @@ def skill_manager(
action="list":
List installed skills. By default only shows user-installed skills.
Set include_system=True to also show built-in system skills.
Set skill_type="expert" to show only expert skills (agent teams);
set skill_type="utility" to exclude them; leave as "all" (default) for no filter.
Built-in skills evolve over time, so use action="list" to see the current set.
action="browse" (optional tag):
@@ -37,7 +40,8 @@ def skill_manager(
action="info" (requires name):
Get details (description, source, path, tags) about a specific skill by name.
Searches both user and system skills.
Expert skills also surface their role, byline, capability tags, and default
dispatch mode. Searches both user and system skills.
action="uninstall" (requires name):
Remove a user-installed skill by name. System skills cannot be uninstalled.
@@ -48,6 +52,7 @@ def skill_manager(
name: Required for info and uninstall — the skill name (for example, one returned by action="list")
tag: Optional for browse — filter by tag (e.g. "core", "writing", "experiments", "research")
include_system: Only for list — set True to include built-in system skills in the output
skill_type: Only for list — one of "all" (default; no filter), "expert" (agent-team skills), or "utility"
Returns:
Result message
@@ -112,7 +117,16 @@ def skill_manager(
elif action == "list":
skills = list_skills(include_system=include_system)
# `skill_type="all"` (default) is the no-filter case. A specific
# value ("expert" or "utility") narrows the list to that type.
# Empty string is NOT a legal input — Gemini's function-declaration
# schema rejects empty enum values (was the root cause of a live
# smoke failure); the `Literal` above defends against it.
if skill_type in ("expert", "utility"):
skills = [s for s in skills if s.type == skill_type]
if not skills:
if skill_type in ("expert", "utility"):
return f"No {skill_type} skills found."
if include_system:
return "No skills found."
return "No user skills installed. Use action='install' to add skills, or set include_system=True to see built-in skills."
@@ -180,17 +194,33 @@ def skill_manager(
f"Skill not found: {name}. "
f"Use action='list' with include_system=True to see all available skills."
)
tags_str = f"\nTags: {', '.join(info.tags)}" if info.tags else ""
lines = [
f"Name: {info.name}",
f"Description: {info.description}",
f"Source: {info.source}",
]
# ``Path`` reports the sandbox-visible virtual mount segment (the skill's
# directory name under ``/skills/``), not the host filesystem path.
# Surfacing the host path (e.g. ``/home/.../EvoScientist/skills/<name>``)
# invited ``cd <host-path> && …`` chains that fail in the sandbox.
return (
f"Name: {info.name}\n"
f"Description: {info.description}\n"
f"Source: {info.source}\n"
f"Path: /skills/{info.path.name}{tags_str}"
)
lines.append(f"Path: /skills/{info.path.name}")
if info.tags:
lines.append(f"Tags: {', '.join(info.tags)}")
# Expert-skill surface (agent-teams v1): only shown when the
# skill declared `type: expert` in its SKILL.md frontmatter.
if info.type == "expert":
lines.append("Type: expert")
if info.role:
lines.append(f"Role: {info.role}")
if info.byline:
lines.append(f"Byline: {info.byline}")
if info.capability_tags:
lines.append(f"Capability tags: {', '.join(info.capability_tags)}")
if info.avatar_hint:
lines.append(f"Avatar hint: {info.avatar_hint}")
if info.default_dispatch:
lines.append(f"Default dispatch: {info.default_dispatch}")
return "\n".join(lines)
else:
return f"Unknown action: {action}. Use 'install', 'list', 'browse', 'uninstall', or 'info'."
+161 -13
View File
@@ -57,6 +57,47 @@ class SkillInfo:
path: Path
source: str # "workspace", "global", or "builtin"
tags: list[str] = field(default_factory=list)
# Expert-skill fields for the v1 agent-teams feature. All default
# to utility-skill values so existing skills stay unchanged and their
# SKILL.md files need no edits.
type: str = "utility" # "utility" (default) or "expert"
role: str = "" # one-line role summary shown to the orchestrator LLM
byline: str = "" # WebUI gallery byline
capability_tags: list[str] = field(default_factory=list) # WebUI chips
avatar_hint: str = "" # WebUI icon hint
default_dispatch: str = "" # "sync" | "panel" (expert skills only)
# SKILL.md body (post-frontmatter). Populated by ``_parse_skill_md`` so
# the expert-container factory doesn't have to re-read the file on every
# main-agent construction. Empty for skills built by hand or when the
# source SKILL.md has no body content.
body: str = ""
# Shared frontmatter split regex. Captures three parts:
# 1. leading fence (``^---\s*\n``)
# 2. frontmatter YAML (``.*?``)
# 3. trailing fence + optional newline (``\n---\s*\n?``)
# Used by both ``_parse_skill_md`` (needs the YAML) and the expert-container
# factory (needs the body); a single source of truth for "what is the
# frontmatter block" so schema extensions don't drift across call sites.
_FRONTMATTER_SPLIT_RE = re.compile(r"^---\s*\n(.*?)\n---\s*\n?", re.DOTALL)
def _split_frontmatter_and_body(content: str) -> tuple[str, str]:
"""Return ``(frontmatter_yaml, body)`` for a SKILL.md text.
``frontmatter_yaml`` is the raw YAML block between the ``---`` fences
(still a string, not parsed). ``body`` is the post-fence content with
leading whitespace stripped. When there is no valid frontmatter block,
``frontmatter_yaml`` is ``""`` and the whole content becomes the body —
matches the legacy expert-container behaviour of passing plain-body
files through unchanged.
"""
match = _FRONTMATTER_SPLIT_RE.match(content)
if not match:
return "", content.lstrip()
body = content[match.end() :].lstrip()
return match.group(1), body
def _normalize_tags(raw: object) -> list[str]:
@@ -206,7 +247,9 @@ def installed_provenance() -> dict[str, dict[str, str | None]]:
def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
"""Parse SKILL.md frontmatter to extract name, description, and tags.
"""Parse SKILL.md frontmatter to extract name, description, tags, and
(for expert skills) role / byline / capability_tags / avatar_hint /
default_dispatch.
SKILL.md format:
---
@@ -215,6 +258,14 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
tags: [tag1, tag2]
metadata:
tags: [tag1, tag2] # fallback location
# Expert-skill fields (all optional; only meaningful when
# ``type: expert`` — see agent-teams-design.md):
type: expert
role: One-line role summary
byline: Short persona name for gallery
capability_tags: [Tag One, Tag Two]
avatar_hint: lightbulb
default_dispatch: sync
---
# Skill Title
...
@@ -227,40 +278,115 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
SkillInfo with path set to the skill's parent directory.
"""
parent = skill_md_path.parent
content = skill_md_path.read_text(encoding="utf-8")
try:
content = skill_md_path.read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as exc:
# ``_parse_skill_md`` runs during agent construction (via
# ``_fold_expert_subagents`` → ``list_skills``), so an unreadable
# SKILL.md in any tier must not abort the whole main-agent build or
# ``GET /api/teams``. Degrade to an "(unreadable)" placeholder so
# the entry stays visible in ``skill_manager list`` for debugging
# but never advances into expert registration (empty body →
# ``build_expert_subagent_specs`` skips it).
_logger.warning(
"Skill %r: could not read %s (%s); skipping.",
parent.name,
skill_md_path,
exc,
)
return SkillInfo(
name=parent.name,
description="(unreadable)",
path=parent,
source=source,
)
def _info(name: str, description: str, tags: list[str] | None = None) -> SkillInfo:
def _info(
name: str,
description: str,
tags: list[str] | None = None,
*,
type_: str = "utility",
role: str = "",
byline: str = "",
capability_tags: list[str] | None = None,
avatar_hint: str = "",
default_dispatch: str = "",
body: str = "",
) -> SkillInfo:
return SkillInfo(
name=name,
description=description,
path=parent,
source=source,
tags=tags or [],
type=type_,
role=role,
byline=byline,
capability_tags=capability_tags or [],
avatar_hint=avatar_hint,
default_dispatch=default_dispatch,
body=body,
)
# Extract YAML frontmatter
frontmatter_match = re.match(r"^---\s*\n(.*?)\n---", content, re.DOTALL)
if not frontmatter_match:
# No frontmatter, use directory name
return _info(parent.name, "(no description)")
frontmatter_yaml, body = _split_frontmatter_and_body(content)
if not frontmatter_yaml:
# No frontmatter, use directory name; still cache the body so
# the expert-container factory doesn't have to re-read.
return _info(parent.name, "(no description)", body=body)
try:
frontmatter = yaml.safe_load(frontmatter_match.group(1))
frontmatter = yaml.safe_load(frontmatter_yaml)
if not isinstance(frontmatter, dict):
return _info(parent.name, "(empty frontmatter)")
return _info(parent.name, "(empty frontmatter)", body=body)
# Tags: check top-level first, fall back to metadata.tags
tags = _normalize_tags(frontmatter.get("tags"))
if not tags:
metadata = frontmatter.get("metadata")
if isinstance(metadata, dict):
tags = _normalize_tags(metadata.get("tags"))
# Expert-skill fields. Only trusted when explicit; unknown
# ``type`` values fall back to "utility" so a typo doesn't
# accidentally register a skill as an expert. Log the fallback
# so authors can debug a "why isn't my expert appearing" case.
raw_type = frontmatter.get("type")
type_ = raw_type if raw_type in ("expert", "utility") else "utility"
if raw_type is not None and raw_type != type_:
_logger.warning(
"Skill %r: unrecognized type %r in frontmatter; treating as utility.",
frontmatter.get("name") or parent.name,
raw_type,
)
role = frontmatter.get("role") or ""
byline = frontmatter.get("byline") or ""
capability_tags = _normalize_tags(frontmatter.get("capability_tags"))
avatar_hint = frontmatter.get("avatar_hint") or ""
raw_dispatch = frontmatter.get("default_dispatch")
default_dispatch = raw_dispatch if raw_dispatch in ("sync", "panel") else ""
# ``.get("name", parent.name)`` only defaults on missing key; a
# present-but-empty ``name:`` yields None, which would flow into
# ``SkillInfo.name`` and slip past the ``_fold_expert_subagents``
# collision guard. Coerce to the parent dir name in both cases.
return _info(
frontmatter.get("name", parent.name),
frontmatter.get("name") or parent.name,
frontmatter.get("description", "(no description)"),
tags,
type_=type_,
role=str(role),
byline=str(byline),
capability_tags=capability_tags,
avatar_hint=str(avatar_hint),
default_dispatch=default_dispatch,
body=body,
)
except yaml.YAMLError:
return _info(parent.name, "(invalid frontmatter)")
except yaml.YAMLError as exc:
_logger.warning(
"Skill %r: invalid frontmatter YAML in %s (%s); using placeholder.",
parent.name,
skill_md_path,
exc,
)
return _info(parent.name, "(invalid frontmatter)", body=body)
def _parse_github_url(url: str) -> tuple[str, str | None, str | None]:
@@ -763,6 +889,28 @@ def list_skills(include_system: bool = False) -> list[SkillInfo]:
return skills
def list_expert_skills(include_system: bool = True) -> list[SkillInfo]:
"""List installed expert skills (agent-teams v1).
Filters ``list_skills()`` output to entries whose SKILL.md frontmatter
declares ``type: expert``. Defaults ``include_system=True`` because
first-party experts (`idea-brainstorm`, etc.) ship under the builtin
skills tier; user-installed experts still surface from the workspace
and global tiers alongside them.
Consumed by:
- ``GET /api/teams`` (WebUI gallery listing).
- Main-agent construction (folds expert skills into the
``subagents=[...]`` list so the ``task`` tool can dispatch to
them for sync consult).
- The ``skill_manager`` @tool's ``type=expert`` filter.
Returns:
List of ``SkillInfo`` for each expert skill.
"""
return [s for s in list_skills(include_system=include_system) if s.type == "expert"]
def uninstall_skill(name: str) -> dict:
"""Uninstall a skill from workspace or global tier.