feat: dispatch newly installed experts in the background without /new (#420)
* refactor: rename AGENTS.md to EXPERT.md per EvoSkills convention * feat: resolve newly installed experts on start_async_task miss * chore: reword the /expert invite hint to state the dispatch boundary * docs: note the resolve-on-miss caller in build_expert_async_subagent_specs * fix: thread the construction cfg through resolve-on-miss and symmetric setdefault * fix: guard agent_map iteration against concurrent resolve-on-miss writes * fix: warn once per broken expert on repeated resolve-on-miss walks * fix: scope the /expert invite hint to newly installed experts * docs: note live uninstalls as a resolve-on-miss limitation * chore: isolate the warn-once collision test key from the route-specs suite
This commit is contained in:
@@ -575,10 +575,14 @@ def _route_async_specs_through_evo_middleware(
|
||||
route all async dispatch through our subclass, we strip AsyncSubAgent
|
||||
specs from ``subs`` here and hand them to our middleware.
|
||||
|
||||
Also folds in ``AsyncSubAgent`` specs for installed
|
||||
installed expert skills — all pointing at the shared
|
||||
``expert-container-async`` graph, marked ``is_expert=True`` so the
|
||||
middleware requires a payload with ``skill_name``.
|
||||
Also folds in ``AsyncSubAgent`` specs for installed expert skills —
|
||||
all pointing at the shared ``expert-container-async`` graph, marked
|
||||
``is_expert=True`` so the middleware requires a payload with
|
||||
``skill_name``.
|
||||
|
||||
The completion watcher (``AsyncWatcherMiddleware``) is found or created
|
||||
before the middleware so the middleware's resolve-on-miss start tool can
|
||||
hold the watcher's agent dict by reference — see the wiring block below.
|
||||
|
||||
Returns:
|
||||
``subs`` with ``graph_id``-carrying entries removed. Safe to pass
|
||||
@@ -596,6 +600,71 @@ def _route_async_specs_through_evo_middleware(
|
||||
expert_specs = build_expert_async_subagent_specs(cfg=cfg)
|
||||
async_specs.extend(expert_specs)
|
||||
|
||||
# Find or create the completion watcher BEFORE constructing
|
||||
# ``EvoAsyncSubAgentMiddleware``: the middleware's resolve-on-miss start
|
||||
# tool must hold the watcher's agent dict by reference, so an expert
|
||||
# discovered mid-session lands in the dispatch table and the watcher in
|
||||
# one step. Without the watcher update, dispatch succeeds but the
|
||||
# watcher's ``get_async(agent_name)`` raises KeyError inside its
|
||||
# ``try/except`` and the completion notification silently never fires.
|
||||
watcher_agents: dict | None = None
|
||||
watcher = next(
|
||||
(m for m in base_middleware if isinstance(m, AsyncWatcherMiddleware)),
|
||||
None,
|
||||
)
|
||||
if watcher is None:
|
||||
# No YAML async subagents were registered, so ``_maybe_swap`` did
|
||||
# not install the watcher. Install it now so experts still get
|
||||
# completion notifications. Appended before the middleware's
|
||||
# index-0 insert below, which yields the same final order as
|
||||
# append-after-insert: ``[EvoAsync..., ..., watcher]``.
|
||||
if expert_specs:
|
||||
from .cli import async_notifier
|
||||
|
||||
watcher = AsyncWatcherMiddleware(
|
||||
{s["name"]: s for s in expert_specs},
|
||||
notifier=async_notifier,
|
||||
)
|
||||
base_middleware.append(watcher)
|
||||
elif expert_specs:
|
||||
# Extend AsyncWatcherMiddleware's client cache with expert specs so
|
||||
# start_async_task launches for experts spawn a completion watcher —
|
||||
# otherwise the watcher's ``get_async(agent_name)`` KeyErrors on the
|
||||
# expert name, no notification is enqueued, and the main agent never
|
||||
# learns the task finished. ``_maybe_swap_async_subagents`` above only
|
||||
# populates the watcher with YAML-defined async subagents
|
||||
# (writing-agent, data-analysis-agent, scheduler); this hook folds in
|
||||
# the experts too.
|
||||
#
|
||||
# The mutation reaches through two layers of private state:
|
||||
# ``AsyncWatcherMiddleware._clients`` (our own) and
|
||||
# ``_ClientCache._agents`` (upstream deepagents). If upstream ever
|
||||
# renames ``_agents`` or wraps it in an immutable snapshot, the
|
||||
# ``.update(...)`` below silently lands on nothing — expert
|
||||
# completion nudges then stop firing without a diagnostic surface.
|
||||
# Convert that silent-drop into a grep-able error line and leave
|
||||
# ``watcher_agents`` unset; expert dispatches still work (without
|
||||
# completion notifications and without mid-session resolution into
|
||||
# the watcher) until upstream drift is fixed.
|
||||
if not hasattr(watcher._clients, "_agents"):
|
||||
logging.getLogger(__name__).error(
|
||||
"AsyncWatcherMiddleware._clients has no `_agents` slot — "
|
||||
"deepagents internal renamed; expert completion "
|
||||
"notifications will not fire until the extension hook is "
|
||||
"updated to the new attribute name."
|
||||
)
|
||||
watcher = None
|
||||
else:
|
||||
watcher._clients._agents.update({s["name"]: s for s in expert_specs})
|
||||
# The second ``hasattr`` is not redundant with the one in the ``elif``
|
||||
# above: that check only runs when ``expert_specs`` is non-empty. When a
|
||||
# pre-existing watcher has an empty expert set, this is the only guard
|
||||
# standing between an upstream rename of ``_ClientCache._agents`` and an
|
||||
# AttributeError that would kill agent construction — without it, the
|
||||
# drift degrades to "no completion nudges" instead of crashing.
|
||||
if watcher is not None and hasattr(watcher._clients, "_agents"):
|
||||
watcher_agents = watcher._clients._agents
|
||||
|
||||
if async_specs:
|
||||
# ``_maybe_swap_async_subagents`` installs the model-passthrough patch
|
||||
# only when the yaml-async spec list is non-empty. An expert-only setup
|
||||
@@ -612,52 +681,16 @@ def _route_async_specs_through_evo_middleware(
|
||||
# volatile memory tail, invalidating the cached prefix on every
|
||||
# memory change.
|
||||
base_middleware.insert(
|
||||
0, EvoAsyncSubAgentMiddleware(async_subagents=async_specs)
|
||||
0,
|
||||
EvoAsyncSubAgentMiddleware(
|
||||
async_subagents=async_specs,
|
||||
watcher_agents=watcher_agents,
|
||||
# The construction cfg, so resolve-on-miss specs the same
|
||||
# langgraph_dev_port the construction-time specs used instead
|
||||
# of re-reading config from disk at dispatch time.
|
||||
cfg=cfg,
|
||||
),
|
||||
)
|
||||
|
||||
# Extend AsyncWatcherMiddleware's client cache with expert specs so
|
||||
# start_async_task launches for experts spawn a completion watcher —
|
||||
# otherwise the watcher's ``get_async(agent_name)`` KeyErrors on the
|
||||
# expert name, no notification is enqueued, and the main agent never
|
||||
# learns the task finished. ``_maybe_swap_async_subagents`` above only
|
||||
# populates the watcher with YAML-defined async subagents (writing-agent,
|
||||
# data-analysis-agent, scheduler); this hook folds in the experts too.
|
||||
if expert_specs:
|
||||
watcher = next(
|
||||
(m for m in base_middleware if isinstance(m, AsyncWatcherMiddleware)),
|
||||
None,
|
||||
)
|
||||
if watcher is not None:
|
||||
# The mutation reaches through two layers of private state:
|
||||
# ``AsyncWatcherMiddleware._clients`` (our own) and
|
||||
# ``_ClientCache._agents`` (upstream deepagents). If upstream ever
|
||||
# renames ``_agents`` or wraps it in an immutable snapshot, the
|
||||
# ``.update(...)`` below silently lands on nothing — expert
|
||||
# completion nudges then stop firing without a diagnostic surface.
|
||||
# Convert that silent-drop into a grep-able error line and bail
|
||||
# out of the extension path; expert dispatches still work, just
|
||||
# without completion notifications until upstream drift is fixed.
|
||||
if not hasattr(watcher._clients, "_agents"):
|
||||
logging.getLogger(__name__).error(
|
||||
"AsyncWatcherMiddleware._clients has no `_agents` slot — "
|
||||
"deepagents internal renamed; expert completion "
|
||||
"notifications will not fire until the extension hook is "
|
||||
"updated to the new attribute name."
|
||||
)
|
||||
return sync_subs
|
||||
watcher._clients._agents.update({s["name"]: s for s in expert_specs})
|
||||
else:
|
||||
# No YAML async subagents were registered, so ``_maybe_swap`` did
|
||||
# not install the watcher. Install it now so experts still get
|
||||
# completion notifications.
|
||||
from .cli import async_notifier
|
||||
|
||||
base_middleware.append(
|
||||
AsyncWatcherMiddleware(
|
||||
{s["name"]: s for s in expert_specs},
|
||||
notifier=async_notifier,
|
||||
)
|
||||
)
|
||||
return sync_subs
|
||||
|
||||
|
||||
|
||||
@@ -229,11 +229,18 @@ class ExpertCommand(Command):
|
||||
else:
|
||||
runtime.active_teams = [*runtime.active_teams, canonical]
|
||||
ctx.ui.append_system(f"Invited expert: {canonical}", style="green")
|
||||
# Case (c): expert was installed after agent construction, so it
|
||||
# will not reach ``task()`` until the graph is rebuilt. Cheap
|
||||
# always-print hint mirrors the /install-skill success message.
|
||||
# An expert installed mid-session: the background reach
|
||||
# (``start_async_task``) resolves it on first dispatch, but the
|
||||
# in-turn ``task`` reach is frozen into the running agent, so it
|
||||
# needs a rebuilt agent. An expert installed before this session
|
||||
# started is already inside that frozen set — its in-turn reach
|
||||
# works without a rebuild — so the hint scopes the /new boundary
|
||||
# to newly installed experts instead of stating it
|
||||
# unconditionally.
|
||||
ctx.ui.append_system(
|
||||
"If just installed, run /new to activate it.", style="dim"
|
||||
"Newly installed experts: background dispatch is available "
|
||||
"immediately; in-turn task dispatch needs /new.",
|
||||
style="dim",
|
||||
)
|
||||
if runtime.active_teams:
|
||||
ctx.ui.append_system(
|
||||
|
||||
@@ -79,14 +79,14 @@ 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 — a skill
|
||||
directory carrying a sibling ``AGENTS.md`` (or, on the deprecated path,
|
||||
directory carrying a sibling ``EXPERT.md`` (or, on the deprecated path,
|
||||
``type: expert`` SKILL.md frontmatter). The response is a curated,
|
||||
gallery-safe projection: name + description, plus optional ``byline`` /
|
||||
``capability_tags`` / ``avatar_hint`` when the skill populates them.
|
||||
|
||||
Cards for experts on the current contract carry name + description only:
|
||||
the decoration fields were actor metadata in SKILL.md frontmatter, which
|
||||
that contract removes rather than relocates (``AGENTS.md`` has no
|
||||
that contract removes rather than relocates (``EXPERT.md`` has no
|
||||
frontmatter to hold them). The omit-when-unpopulated projection below is
|
||||
what makes those cards degrade rather than break; restoring richer cards
|
||||
means sourcing decoration from index metadata, not re-adding frontmatter
|
||||
|
||||
@@ -29,6 +29,19 @@ Design
|
||||
marker so the middleware knows when to add ``skill_name`` to the run
|
||||
input. Standard specs (``writing-agent`` / ``data-analysis-agent`` /
|
||||
``scheduler``) reach ``client.runs.create`` with the upstream shape.
|
||||
- Resolve-on-miss: when ``start_async_task`` is asked for a
|
||||
``subagent_type`` absent from ``agent_map`` — typically an expert
|
||||
installed after the agent was built — the tool runs one
|
||||
``build_expert_async_subagent_specs`` walk and merges every unknown
|
||||
expert into ``agent_map`` and the watcher's agent dict before
|
||||
re-validating (see ``_resolve_merge_validate``). New experts become
|
||||
background-dispatchable the first time they are named, with no agent
|
||||
rebuild, no registry watcher, and no restart; in-turn ``task`` reach
|
||||
for a new expert still requires a rebuilt agent (``/new``). The merge
|
||||
and the map-iterating validation serialize on one per-instance lock
|
||||
(``self._resolve_lock``); the event loop never touches it — the async
|
||||
variant miss-checks by keyed lookup and does all lock work on the
|
||||
``asyncio.to_thread`` worker.
|
||||
|
||||
If deepagents ever lands a skill-name-passthrough of its own, delete this
|
||||
file and rebind ``EvoAsyncSubAgentMiddleware`` → ``AsyncSubAgentMiddleware``
|
||||
@@ -43,7 +56,9 @@ check, and gets stripped from tool_input at parse time — the coroutine is
|
||||
then called without ``runtime`` and raises ``TypeError``.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import threading
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any, NotRequired
|
||||
|
||||
@@ -125,10 +140,111 @@ def _build_task_envelope(
|
||||
)
|
||||
|
||||
|
||||
def _resolve_merge_validate(
|
||||
agent_map: dict[str, AsyncSubAgent],
|
||||
watcher_agents: dict[str, AsyncSubAgent] | None,
|
||||
cfg: Any | None,
|
||||
subagent_type: str,
|
||||
lock: Any = None,
|
||||
) -> str | None:
|
||||
"""Resolve a start-tool miss, merge the walk's specs, re-validate.
|
||||
|
||||
Called from ``start_async_task`` only when ``subagent_type`` missed
|
||||
``agent_map`` — the resolve-on-miss path that makes an expert installed
|
||||
mid-session dispatchable without an agent rebuild. Returns the
|
||||
refreshed ``_validate_agent_type`` error for *subagent_type*: ``None``
|
||||
when the walk resolved it, upstream's unknown-type message (with the
|
||||
now-updated allowed-type list) for a genuine miss. Both dicts are
|
||||
mutated in place; the middleware and its tools hold them by reference,
|
||||
so the update is visible to every tool that resolves a name at call
|
||||
time (start / check / update / cancel all reach ``agent_map`` or
|
||||
``_ClientCache._agents``, which share the object).
|
||||
|
||||
One walk, every unknown expert: ``build_expert_async_subagent_specs``
|
||||
already walks the whole skills tree, so merging every not-yet-known
|
||||
spec costs nothing extra and N newly installed experts resolve on the
|
||||
first miss rather than one walk each.
|
||||
|
||||
``setdefault`` semantics on both dicts — an existing entry is never
|
||||
overwritten. The middleware's constructor already raised on duplicate
|
||||
names at build time, so an overwrite here could only smuggle in a spec
|
||||
the running agent was not validated against.
|
||||
|
||||
Known limitation — installs only, never uninstalls: the merge adds
|
||||
names, nothing removes them, so an expert uninstalled mid-session
|
||||
stays in the dispatch tables until the next agent rebuild (``/new``).
|
||||
Its runs fail late — the container graph reads the persona from disk
|
||||
at dispatch time and reports the unknown skill — rather than at this
|
||||
start-tool boundary.
|
||||
|
||||
*cfg* is the config the agent was constructed with, threaded through
|
||||
the middleware. The specs must point at the same ``langgraph_dev_port``
|
||||
the construction-time specs used — re-deriving config from disk here
|
||||
(the builder's ``get_effective_config()`` fallback) would let a
|
||||
mid-session port change spec a newly resolved expert onto a port the
|
||||
running dev subprocess is not on: dispatch accepts the name and only
|
||||
``runs.create`` fails, an advertise/provide split.
|
||||
|
||||
*lock* serializes the merge AND the re-validation — both run under one
|
||||
acquisition — against every other ``_validate_agent_type`` reader of
|
||||
``agent_map`` (the sync start tool's initial validation), which
|
||||
iterates the map to build its error string: an unsynchronized insert
|
||||
under that reader raises ``RuntimeError: dictionary changed size
|
||||
during iteration``. The skills-tree walk runs OUTSIDE the lock; only
|
||||
the ``setdefault`` loop and the validation — microseconds of pure
|
||||
dict operations — hold it. Keyed lookups (``_ClientCache.get_sync`` /
|
||||
``get_async``, the update tool) are single GIL-protected operations
|
||||
and need no lock.
|
||||
|
||||
``watcher_agents`` is ``AsyncWatcherMiddleware._clients._agents`` —
|
||||
a *separate* dict from ``agent_map`` (the watcher's cache was built
|
||||
from its own spec list). Without updating it, dispatch succeeds but
|
||||
the watcher's ``get_async(agent_name)`` raises KeyError inside its
|
||||
``try/except``, and the completion notification silently never fires.
|
||||
``None`` means no watcher is wired (yaml-async-less setup, or the
|
||||
upstream ``_agents`` drift guard tripped): dispatch still resolves,
|
||||
just without completion nudges — matching the pre-existing degradation.
|
||||
|
||||
Blocking (a skills-tree walk under ``list_expert_skills``); callers on
|
||||
an event loop must run it via ``asyncio.to_thread``.
|
||||
"""
|
||||
from ..subagents.expert_container_async import build_expert_async_subagent_specs
|
||||
|
||||
# The walk is the blocking part — never hold the lock over I/O.
|
||||
specs = build_expert_async_subagent_specs(cfg=cfg)
|
||||
if lock is not None:
|
||||
with lock:
|
||||
_merge_expert_specs(agent_map, watcher_agents, specs)
|
||||
return _validate_agent_type(agent_map, subagent_type)
|
||||
_merge_expert_specs(agent_map, watcher_agents, specs)
|
||||
return _validate_agent_type(agent_map, subagent_type)
|
||||
|
||||
|
||||
def _merge_expert_specs(
|
||||
agent_map: dict[str, AsyncSubAgent],
|
||||
watcher_agents: dict[str, AsyncSubAgent] | None,
|
||||
specs: list,
|
||||
) -> None:
|
||||
"""Merge built expert specs into both dispatch tables.
|
||||
|
||||
Split out of ``_resolve_merge_validate`` so the lock guards exactly
|
||||
this — microseconds of ``setdefault`` — and not the skills-tree walk
|
||||
that produced *specs*.
|
||||
"""
|
||||
for spec in specs:
|
||||
name = spec["name"]
|
||||
agent_map.setdefault(name, spec)
|
||||
if watcher_agents is not None:
|
||||
watcher_agents.setdefault(name, spec)
|
||||
|
||||
|
||||
def _build_expert_start_tool(
|
||||
agent_map: dict[str, AsyncSubAgent],
|
||||
clients: _ClientCache,
|
||||
tool_description: str,
|
||||
watcher_agents: dict[str, AsyncSubAgent] | None = None,
|
||||
cfg: Any | None = None,
|
||||
map_lock: Any = None,
|
||||
) -> StructuredTool:
|
||||
"""Build the skill-name-injecting ``start_async_task`` tool.
|
||||
|
||||
@@ -137,16 +253,55 @@ def _build_expert_start_tool(
|
||||
injects ``skill_name=subagent_type`` into the run input before
|
||||
dispatch, so the container graph resolves the right persona without
|
||||
the model contributing (or being able to corrupt) that value.
|
||||
|
||||
An unknown ``subagent_type`` triggers one resolve-on-miss pass before
|
||||
the error is returned (see ``_resolve_merge_validate``); a name that
|
||||
is still unknown after it is a genuine miss and gets upstream's error
|
||||
message, now with the refreshed allowed-type list.
|
||||
|
||||
``map_lock`` serializes every ``agent_map`` *iteration* against the
|
||||
resolver's merge: ``_validate_agent_type`` builds its error string by
|
||||
joining over the map, so an unsynchronized insert from the async
|
||||
resolver's worker thread (or a concurrent sync miss on another
|
||||
tool-executor thread) can raise ``RuntimeError: dictionary changed
|
||||
size during iteration`` under the reader. The two variants divide the
|
||||
work differently:
|
||||
|
||||
- the sync variant validates under the lock up front and delegates
|
||||
the miss to ``_resolve_merge_validate`` (merge and re-validation
|
||||
share one lock acquisition, on this tool-executor thread);
|
||||
- the async variant only does a keyed ``subagent_type not in
|
||||
agent_map`` check on the event loop — no iteration, and the loop
|
||||
never touches the lock; the miss path runs merge + re-validation
|
||||
inside one ``asyncio.to_thread`` acquisition on the worker thread
|
||||
and returns the refreshed error.
|
||||
"""
|
||||
|
||||
def _locked_validate(agent_type: str) -> str | None:
|
||||
"""``_validate_agent_type`` under ``map_lock`` when provided.
|
||||
|
||||
The validation error message iterates ``agent_map``; the resolver
|
||||
merges into it under the same lock. Used by the sync variant's
|
||||
initial validation only. ``None`` lock degrades to the unguarded
|
||||
read, matching pre-lock behavior.
|
||||
"""
|
||||
if map_lock is not None:
|
||||
with map_lock:
|
||||
return _validate_agent_type(agent_map, agent_type)
|
||||
return _validate_agent_type(agent_map, agent_type)
|
||||
|
||||
def start_async_task(
|
||||
description: str,
|
||||
subagent_type: str,
|
||||
runtime: ToolRuntime,
|
||||
) -> str | Command:
|
||||
error = _validate_agent_type(agent_map, subagent_type)
|
||||
error = _locked_validate(subagent_type)
|
||||
if error:
|
||||
return error
|
||||
error = _resolve_merge_validate(
|
||||
agent_map, watcher_agents, cfg, subagent_type, map_lock
|
||||
)
|
||||
if error:
|
||||
return error
|
||||
spec = agent_map[subagent_type]
|
||||
input_dict = _build_run_input(spec, subagent_type, description)
|
||||
try:
|
||||
@@ -171,9 +326,24 @@ def _build_expert_start_tool(
|
||||
subagent_type: str,
|
||||
runtime: ToolRuntime,
|
||||
) -> str | Command:
|
||||
error = _validate_agent_type(agent_map, subagent_type)
|
||||
if error:
|
||||
return error
|
||||
# Keyed miss check — no map iteration, and the event loop never
|
||||
# touches the lock: all lock work runs on the to_thread worker.
|
||||
# (The validation error message joins over ``agent_map``, so it
|
||||
# cannot run unlocked here; it runs inside the worker instead.)
|
||||
if subagent_type not in agent_map:
|
||||
# to_thread: the resolver walks the skills tree synchronously,
|
||||
# and this coroutine runs on the event loop where langgraph-dev's
|
||||
# blockbuster guard raises BlockingError on filesystem calls.
|
||||
error = await asyncio.to_thread(
|
||||
_resolve_merge_validate,
|
||||
agent_map,
|
||||
watcher_agents,
|
||||
cfg,
|
||||
subagent_type,
|
||||
map_lock,
|
||||
)
|
||||
if error:
|
||||
return error
|
||||
spec = agent_map[subagent_type]
|
||||
input_dict = _build_run_input(spec, subagent_type, description)
|
||||
try:
|
||||
@@ -223,6 +393,8 @@ class EvoAsyncSubAgentMiddleware(AsyncSubAgentMiddleware):
|
||||
*,
|
||||
async_subagents: list[AsyncSubAgent],
|
||||
system_prompt: str | None = None,
|
||||
watcher_agents: dict[str, AsyncSubAgent] | None = None,
|
||||
cfg: Any | None = None,
|
||||
) -> None:
|
||||
# Install the model-passthrough patch BEFORE ``super().__init__(...)``
|
||||
# so upstream's ``_build_async_subagent_tools`` sees the patched
|
||||
@@ -263,8 +435,24 @@ class EvoAsyncSubAgentMiddleware(AsyncSubAgentMiddleware):
|
||||
f"- {a['name']}: {a['description']}" for a in async_subagents
|
||||
)
|
||||
launch_desc = ASYNC_TASK_TOOL_DESCRIPTION.format(available_agents=agents_desc)
|
||||
# Serializes ``agent_map`` iteration (the sync start tool's
|
||||
# validation and the resolver's merge + re-validation, whose error
|
||||
# message joins over the map) against the resolve-on-miss merge,
|
||||
# which can run on a worker thread (``asyncio.to_thread`` in the
|
||||
# async variant) while the event loop keeps reading. Instance-
|
||||
# scoped: the map is per-middleware, so the lock is too. The async
|
||||
# variant's event loop never acquires it — the miss check there is
|
||||
# a keyed lookup and all lock work happens on the worker thread.
|
||||
self._resolve_lock = threading.Lock()
|
||||
self.tools = [
|
||||
_build_expert_start_tool(agent_map, clients, launch_desc),
|
||||
_build_expert_start_tool(
|
||||
agent_map,
|
||||
clients,
|
||||
launch_desc,
|
||||
watcher_agents,
|
||||
cfg,
|
||||
self._resolve_lock,
|
||||
),
|
||||
_build_check_tool(clients),
|
||||
_build_update_tool(agent_map, clients),
|
||||
_build_cancel_tool(clients),
|
||||
|
||||
@@ -7,7 +7,7 @@ into its subagent list at construction time so the `task` tool can dispatch to
|
||||
each installed expert in-turn; the same registry is reused by the QuickJS
|
||||
`task()` global for in-eval fan-out.
|
||||
|
||||
A skill declares itself an expert by carrying a sibling `AGENTS.md`, whose body
|
||||
A skill declares itself an expert by carrying a sibling `EXPERT.md`, whose body
|
||||
is the actor definition and therefore the runtime prompt — see
|
||||
`skills_manager._parse_skill_md`. THIS module builds the in-turn (`task()`) spec
|
||||
for such experts and for legacy `type: expert` frontmatter experts alike;
|
||||
@@ -55,7 +55,7 @@ def expert_prompt_body(skill_info: SkillInfo) -> str:
|
||||
it differently, and splitting that decision across call sites is how a
|
||||
skill ends up registered off one file and prompted off another:
|
||||
|
||||
- ``expert_source == "agents_md"`` — the sibling AGENTS.md body (persona
|
||||
- ``expert_source == "expert_md"`` — the sibling EXPERT.md body (persona
|
||||
+ result envelope). SKILL.md stays pure knowledge and stays readable
|
||||
in-turn off the ``/skills/`` mount, so it is deliberately NOT the
|
||||
prompt here.
|
||||
@@ -68,12 +68,12 @@ def expert_prompt_body(skill_info: SkillInfo) -> str:
|
||||
body populated. Returns an empty string if the file can't be read; the
|
||||
callers treat that as "refuse to register this expert".
|
||||
"""
|
||||
if skill_info.expert_source == "agents_md":
|
||||
if skill_info.agents_body:
|
||||
return skill_info.agents_body
|
||||
from ..tools.skills_manager import _read_agents_md
|
||||
if skill_info.expert_source == "expert_md":
|
||||
if skill_info.expert_body:
|
||||
return skill_info.expert_body
|
||||
from ..tools.skills_manager import _read_expert_md
|
||||
|
||||
return _read_agents_md(skill_info.path) or ""
|
||||
return _read_expert_md(skill_info.path) or ""
|
||||
|
||||
if skill_info.body:
|
||||
return skill_info.body
|
||||
@@ -95,13 +95,13 @@ def expert_prompt_body(skill_info: SkillInfo) -> str:
|
||||
def _compose_system_prompt(skill_info: SkillInfo, body: str) -> str:
|
||||
"""Compose the expert's system_prompt from its role + prompt body.
|
||||
|
||||
*body* is what :func:`expert_prompt_body` resolved — an AGENTS.md actor
|
||||
*body* is what :func:`expert_prompt_body` resolved — an EXPERT.md actor
|
||||
definition, or a legacy expert's SKILL.md body. Either way it carries
|
||||
the persona voice, rubrics, and output-style instructions, written in
|
||||
second person addressing the expert itself.
|
||||
|
||||
The legacy `role` frontmatter (one-line role summary) is prepended as an
|
||||
orientation line when set. AGENTS.md experts declare no `role` — their
|
||||
orientation line when set. EXPERT.md experts declare no `role` — their
|
||||
persona section opens with the same orientation in prose — so for those
|
||||
the body passes through untouched.
|
||||
"""
|
||||
@@ -264,7 +264,7 @@ def build_expert_subagent_specs(
|
||||
_logger.warning(
|
||||
"Expert skill %r: %s body is empty; skipping registration.",
|
||||
info.name,
|
||||
"AGENTS.md" if info.expert_source == "agents_md" else "SKILL.md",
|
||||
"EXPERT.md" if info.expert_source == "expert_md" else "SKILL.md",
|
||||
)
|
||||
continue
|
||||
specs.append(build_expert_subagent_spec(info, tool_registry=tool_registry))
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
One generic graph that reads ``skill_name`` from initial state and loads
|
||||
that expert skill's actor definition as the sub-agent's system prompt at
|
||||
invocation time — its ``AGENTS.md`` body under the current skill contract,
|
||||
invocation time — its ``EXPERT.md`` body under the current skill contract,
|
||||
or its ``SKILL.md`` body for legacy ``type: expert`` skills. Registered once
|
||||
in ``langgraph.json``; parameterised per run via the payload the main
|
||||
agent's ``start_async_task`` passes through
|
||||
@@ -81,7 +81,7 @@ class ExpertSkillLoaderMiddleware(AgentMiddleware[Any, Any, Any]):
|
||||
|
||||
Reads ``state.skill_name``, resolves the corresponding installed expert
|
||||
skill via ``list_expert_skills()``, composes the system message from
|
||||
``role`` + the skill's prompt body (AGENTS.md under the current contract,
|
||||
``role`` + the skill's prompt body (EXPERT.md under the current contract,
|
||||
SKILL.md for legacy frontmatter experts — resolved by
|
||||
``expert_container.expert_prompt_body``, mirroring the sync path in
|
||||
``expert_container._compose_system_prompt``), and overrides
|
||||
@@ -141,7 +141,7 @@ class ExpertSkillLoaderMiddleware(AgentMiddleware[Any, Any, Any]):
|
||||
# nonsense.
|
||||
#
|
||||
# Which file is checked follows the skill's contract:
|
||||
# ``expert_prompt_body`` reads AGENTS.md for experts declared that
|
||||
# ``expert_prompt_body`` reads EXPERT.md for experts declared that
|
||||
# way and SKILL.md for legacy frontmatter experts. Checking ``.body``
|
||||
# directly would clear a paper-review-shaped skill on the strength of
|
||||
# its knowledge file while its actor definition is empty.
|
||||
@@ -150,7 +150,7 @@ class ExpertSkillLoaderMiddleware(AgentMiddleware[Any, Any, Any]):
|
||||
body = expert_prompt_body(match)
|
||||
if not body.strip():
|
||||
source_file = (
|
||||
"AGENTS.md" if match.expert_source == "agents_md" else "SKILL.md"
|
||||
"EXPERT.md" if match.expert_source == "expert_md" else "SKILL.md"
|
||||
)
|
||||
return (
|
||||
f"ERROR: Expert skill '{skill_name}' has an empty {source_file} "
|
||||
@@ -221,6 +221,13 @@ class ExpertSkillLoaderMiddleware(AgentMiddleware[Any, Any, Any]):
|
||||
def build_expert_async_subagent_specs(cfg: Any | None = None) -> list[dict[str, Any]]:
|
||||
"""Build ``AsyncSubAgent``-shaped specs for every installed expert skill.
|
||||
|
||||
Called from two places: agent construction (fold-in via
|
||||
``EvoScientist.py::_route_async_specs_through_evo_middleware``) and the
|
||||
resolve-on-miss path in ``middleware/expert_async_subagent.py`` — an
|
||||
expert installed mid-session is spec'd here the first time
|
||||
``start_async_task`` names it, which is what makes background dispatch
|
||||
live without an agent rebuild.
|
||||
|
||||
Every expert gets a background reach here, and
|
||||
``build_expert_subagent_specs`` independently gives every expert an
|
||||
in-turn reach. Nothing classifies a skill into one or the other — the
|
||||
@@ -249,7 +256,7 @@ def build_expert_async_subagent_specs(cfg: Any | None = None) -> list[dict[str,
|
||||
if not is_async_subagents_available():
|
||||
return []
|
||||
|
||||
from ..tools.skills_manager import list_expert_skills
|
||||
from ..tools.skills_manager import _warn_once, list_expert_skills
|
||||
from .expert_container import _reserved_subagent_names, expert_prompt_body
|
||||
|
||||
port = int(getattr(cfg, "langgraph_dev_port", 6174))
|
||||
@@ -270,16 +277,24 @@ def build_expert_async_subagent_specs(cfg: Any | None = None) -> list[dict[str,
|
||||
# a body-less expert in ``start_async_task``'s tool schema, then
|
||||
# rejecting it at loader time, wastes a launch round-trip; filter
|
||||
# upstream so ``start_async_task`` never sees the broken skill.
|
||||
# The warnings go through ``_warn_once``: the sync fold-in runs
|
||||
# once per agent build, but the resolve-on-miss path in
|
||||
# ``middleware/expert_async_subagent.py`` re-runs this walk on
|
||||
# every ``start_async_task`` miss — hallucinated names included —
|
||||
# so a per-walk warning would fire once per miss for the rest
|
||||
# of the session while the broken skill stays broken.
|
||||
if not expert_prompt_body(skill).strip():
|
||||
_logger.warning(
|
||||
_warn_once(
|
||||
f"expert-async-empty-body:{skill.name}",
|
||||
"Expert skill %r: %s body is empty; skipping "
|
||||
"async-dispatch registration.",
|
||||
skill.name,
|
||||
"AGENTS.md" if skill.expert_source == "agents_md" else "SKILL.md",
|
||||
"EXPERT.md" if skill.expert_source == "expert_md" else "SKILL.md",
|
||||
)
|
||||
continue
|
||||
if skill.name in taken:
|
||||
_logger.warning(
|
||||
_warn_once(
|
||||
f"expert-async-name-collision:{skill.name}",
|
||||
"Expert skill %r collides with an existing async sub-agent "
|
||||
"name; skipping async-dispatch registration.",
|
||||
skill.name,
|
||||
|
||||
@@ -207,9 +207,9 @@ def skill_manager(
|
||||
if info.tags:
|
||||
lines.append(f"Tags: {', '.join(info.tags)}")
|
||||
# Expert surface: shown when the skill can also act as an expert —
|
||||
# declared by a sibling AGENTS.md, or by legacy `type: expert`
|
||||
# declared by a sibling EXPERT.md, or by legacy `type: expert`
|
||||
# frontmatter. The decoration lines below exist only on the legacy
|
||||
# path, so an AGENTS.md expert renders the type line and stops.
|
||||
# path, so an EXPERT.md expert renders the type line and stops.
|
||||
if info.type == "expert":
|
||||
lines.append("Type: expert")
|
||||
if info.role:
|
||||
|
||||
@@ -124,15 +124,15 @@ class SkillInfo:
|
||||
# source SKILL.md has no body content.
|
||||
body: str = ""
|
||||
# How this skill came to be classified as an expert:
|
||||
# "agents_md" — a sibling AGENTS.md is present (current contract)
|
||||
# "expert_md" — a sibling EXPERT.md is present (current contract)
|
||||
# "frontmatter" — legacy ``type: expert`` in SKILL.md (deprecated)
|
||||
# "" — not an expert
|
||||
expert_source: str = ""
|
||||
# AGENTS.md body — the actor definition (persona + result envelope) that
|
||||
# EXPERT.md body — the actor definition (persona + result envelope) that
|
||||
# becomes the expert's runtime prompt. Empty for non-experts and for
|
||||
# experts still on the legacy frontmatter path, whose runtime prompt is
|
||||
# still sourced from ``body``.
|
||||
agents_body: str = ""
|
||||
expert_body: str = ""
|
||||
|
||||
|
||||
# Shared frontmatter split regex. Captures three parts:
|
||||
@@ -168,7 +168,7 @@ def _split_frontmatter_and_body(content: str) -> tuple[str, str]:
|
||||
# rule the skill system already lives by, applied one level down: a directory
|
||||
# is a package, a contract file is a capability, presence is the declaration,
|
||||
# content is the contract. SKILL.md's presence makes a directory a skill;
|
||||
# AGENTS.md's presence makes it an actor.
|
||||
# EXPERT.md's presence makes it an actor.
|
||||
#
|
||||
# The file carries no frontmatter — the directory name is the expert's
|
||||
# identity, its presence is the declaration, and its body (persona + result
|
||||
@@ -180,7 +180,7 @@ def _split_frontmatter_and_body(content: str) -> tuple[str, str]:
|
||||
# index-facing consumers (the remote skill index, gallery chips) which can't
|
||||
# cheaply stat the directory; the runtime resolves presence directly, so
|
||||
# reading it would create a second classifier that can drift from the file.
|
||||
_AGENTS_FILENAME = "AGENTS.md"
|
||||
_EXPERT_FILENAME = "EXPERT.md"
|
||||
|
||||
# Legacy ``type: expert`` frontmatter is a deprecated fallback, so the warning
|
||||
# is one-shot per skill name — ``_parse_skill_md`` runs on every ``list_skills``
|
||||
@@ -197,32 +197,32 @@ def _warn_once(key: str, message: str, *args: object) -> None:
|
||||
_logger.warning(message, *args)
|
||||
|
||||
|
||||
def _read_agents_md(skill_dir: Path) -> str | None:
|
||||
"""Return the AGENTS.md body for *skill_dir*, or None when absent.
|
||||
def _read_expert_md(skill_dir: Path) -> str | None:
|
||||
"""Return the EXPERT.md body for *skill_dir*, or None when absent.
|
||||
|
||||
Returns a string (possibly empty) whenever the file exists, so callers
|
||||
can distinguish "not an expert" (None) from "declared expert with an
|
||||
empty actor definition" (""). The latter is a skill-authoring bug the
|
||||
container paths refuse loudly rather than papering over.
|
||||
|
||||
Frontmatter is stripped if present. The contract says AGENTS.md carries
|
||||
Frontmatter is stripped if present. The contract says EXPERT.md carries
|
||||
none, but an author who adds some should not have raw YAML leak into the
|
||||
expert's system prompt.
|
||||
"""
|
||||
agents_path = skill_dir / _AGENTS_FILENAME
|
||||
expert_path = skill_dir / _EXPERT_FILENAME
|
||||
try:
|
||||
if not agents_path.is_file():
|
||||
if not expert_path.is_file():
|
||||
return None
|
||||
content = agents_path.read_text(encoding="utf-8")
|
||||
content = expert_path.read_text(encoding="utf-8")
|
||||
except (OSError, UnicodeDecodeError) as exc:
|
||||
# Presence was the declaration, so an unreadable AGENTS.md is a
|
||||
# Presence was the declaration, so an unreadable EXPERT.md is a
|
||||
# broken expert, not a plain skill. Return "" to keep the expert
|
||||
# classification and let the empty-body guards in the container
|
||||
# factories refuse it with a skill-name-bearing warning.
|
||||
_logger.warning(
|
||||
"Skill %r: could not read %s (%s); treating actor definition as empty.",
|
||||
skill_dir.name,
|
||||
agents_path,
|
||||
expert_path,
|
||||
exc,
|
||||
)
|
||||
return ""
|
||||
@@ -383,7 +383,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
whether the skill is also an **expert** — an actor with a persona and a
|
||||
result-envelope contract, dispatchable as its own agent.
|
||||
|
||||
Expert classification is structural: a sibling ``AGENTS.md`` in the same
|
||||
Expert classification is structural: a sibling ``EXPERT.md`` in the same
|
||||
directory declares the skill an expert, and that file's body is the
|
||||
expert's runtime prompt. SKILL.md stays pure knowledge — portable to any
|
||||
harness, and readable in-turn off the ``/skills/`` mount whether or not
|
||||
@@ -391,7 +391,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
|
||||
my-expert/
|
||||
SKILL.md # knowledge: when to use, workflow, references
|
||||
AGENTS.md # actor: persona + result envelope <- the declaration
|
||||
EXPERT.md # actor: persona + result envelope <- the declaration
|
||||
scripts/
|
||||
references/
|
||||
|
||||
@@ -399,7 +399,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
frontmatter, alongside actor fields (``role`` / ``byline`` /
|
||||
``capability_tags`` / ``avatar_hint`` / ``default_dispatch``). That path
|
||||
still resolves, with a deprecation warning, and is still the source of
|
||||
the WebUI gallery's decoration fields. New experts declare via AGENTS.md
|
||||
the WebUI gallery's decoration fields. New experts declare via EXPERT.md
|
||||
and set none of those fields; their gallery cards carry name and
|
||||
description only.
|
||||
|
||||
@@ -407,7 +407,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
reachable both in-turn and in the background, and the orchestrator picks
|
||||
per task — a skill does not declare one dispatch shape for all time.
|
||||
|
||||
When both are present, AGENTS.md wins on every axis (prompt source,
|
||||
When both are present, EXPERT.md wins on every axis (prompt source,
|
||||
dispatch) and the frontmatter actor fields are reported as ignored — a
|
||||
skill mid-migration must not behave as a blend of the two contracts.
|
||||
|
||||
@@ -433,8 +433,8 @@ 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
|
||||
agents_body = _read_agents_md(parent)
|
||||
declares_actor = agents_body is not None
|
||||
expert_body = _read_expert_md(parent)
|
||||
declares_actor = expert_body is not None
|
||||
try:
|
||||
content = skill_md_path.read_text(encoding="utf-8")
|
||||
except (OSError, UnicodeDecodeError) as exc:
|
||||
@@ -451,7 +451,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
skill_md_path,
|
||||
exc,
|
||||
)
|
||||
# An unreadable SKILL.md doesn't undo an AGENTS.md declaration — the
|
||||
# An unreadable SKILL.md doesn't undo an EXPERT.md declaration — the
|
||||
# actor definition is a separate file and may be perfectly intact —
|
||||
# so the expert classification is applied here too.
|
||||
return SkillInfo(
|
||||
@@ -460,8 +460,8 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
path=parent,
|
||||
source=source,
|
||||
type="expert" if declares_actor else "utility",
|
||||
expert_source="agents_md" if declares_actor else "",
|
||||
agents_body=agents_body or "",
|
||||
expert_source="expert_md" if declares_actor else "",
|
||||
expert_body=expert_body or "",
|
||||
)
|
||||
|
||||
def _info(
|
||||
@@ -477,7 +477,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
legacy_dispatch: str = "",
|
||||
body: str = "",
|
||||
) -> SkillInfo:
|
||||
# AGENTS.md presence overrides whatever the frontmatter says about
|
||||
# EXPERT.md presence overrides whatever the frontmatter says about
|
||||
# actor-hood: the declaration is the file, and a skill mid-migration
|
||||
# (both present) must resolve to exactly one contract.
|
||||
if declares_actor:
|
||||
@@ -497,14 +497,14 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
"capability_tags / avatar_hint / default_dispatch) is "
|
||||
"ignored and should be removed.",
|
||||
name,
|
||||
_AGENTS_FILENAME,
|
||||
_EXPERT_FILENAME,
|
||||
)
|
||||
type_ = "expert"
|
||||
expert_source = "agents_md"
|
||||
# AGENTS.md is authoritative on every axis: the actor definition is
|
||||
expert_source = "expert_md"
|
||||
# EXPERT.md is authoritative on every axis: the actor definition is
|
||||
# that file, so legacy decoration from SKILL.md frontmatter must not
|
||||
# leak onto SkillInfo. ``role`` would otherwise be prepended to the
|
||||
# AGENTS.md prompt (``_compose_system_prompt`` / the async head), and
|
||||
# EXPERT.md prompt (``_compose_system_prompt`` / the async head), and
|
||||
# a mismatched frontmatter ``name`` would desync the registry
|
||||
# identity from the skill directory.
|
||||
name = parent.name
|
||||
@@ -520,7 +520,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
"result envelope instead, and drop the actor fields from "
|
||||
"frontmatter. The frontmatter path still works for now.",
|
||||
name,
|
||||
_AGENTS_FILENAME,
|
||||
_EXPERT_FILENAME,
|
||||
)
|
||||
expert_source = "frontmatter"
|
||||
else:
|
||||
@@ -539,7 +539,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
avatar_hint=avatar_hint,
|
||||
body=body,
|
||||
expert_source=expert_source,
|
||||
agents_body=agents_body or "",
|
||||
expert_body=expert_body or "",
|
||||
)
|
||||
|
||||
frontmatter_yaml, body = _split_frontmatter_and_body(content)
|
||||
@@ -578,7 +578,7 @@ def _parse_skill_md(skill_md_path: Path, *, source: str = "") -> SkillInfo:
|
||||
# in-turn and in the background, and the orchestrator picks per task
|
||||
# rather than the skill declaring one shape for all time. Still
|
||||
# detected here so the mixed-contract warning can name it among the
|
||||
# legacy actor fields an AGENTS.md skill should drop.
|
||||
# legacy actor fields an EXPERT.md skill should drop.
|
||||
legacy_dispatch = str(frontmatter.get("default_dispatch") or "")
|
||||
# ``.get("name", parent.name)`` only defaults on missing key; a
|
||||
# present-but-empty ``name:`` yields None, which would flow into
|
||||
@@ -1121,7 +1121,7 @@ def list_expert_skills(include_system: bool = True) -> list[SkillInfo]:
|
||||
"""List installed expert skills.
|
||||
|
||||
Filters ``list_skills()`` output to entries classified as experts by
|
||||
``_parse_skill_md`` — a sibling ``AGENTS.md`` (current contract), or
|
||||
``_parse_skill_md`` — a sibling ``EXPERT.md`` (current contract), or
|
||||
legacy ``type: expert`` SKILL.md frontmatter. Read ``expert_source`` on
|
||||
the returned entries to tell the two apart; use
|
||||
``expert_container.expert_prompt_body`` rather than ``.body`` to get an
|
||||
|
||||
Reference in New Issue
Block a user