feat: agent-teams part C - expert selection UX (depends on part B) (#371)

* feat: bias main-agent delegation toward configurable.active_teams

* feat: add /experts and /expert TUI commands for expert-skill summoning

* feat: align expert-selection wording with WebUI (invite/dismiss)

* chore: clear active_teams on /new, cleanup active_team.py comment

* fix: use local append_to_system_message in ActiveTeamMiddleware

* fix: prevent configurable_extra from overriding thread_id

* fix: cache expert-skill lookup for /expert completions

* fix: suppress /expert completions past first arg and on exact match

* fix: invalidate /expert completion cache on skill install/uninstall

* fix: refuse /expert invites for non-dispatchable expert skills

* fix: fire /expert cache invalidation on every install_skill / uninstall_skill path

* fix: propagate active_teams to Rich CLI and serve dispatch surfaces

* fix: keep invited experts across channel shutdown

* fix: match /expert completions case-insensitively
This commit is contained in:
jfilipiuk
2026-07-27 14:34:06 +02:00
committed by Xi Zhang
parent 3a1dbf0a0f
commit 3cda9894c7
25 changed files with 1265 additions and 4 deletions
+4
View File
@@ -22,11 +22,14 @@ async def collect_events(
thread_id: str = "t1",
*,
events=None,
configurable_extra: dict[str, Any] | None = None,
):
"""Collect stream_agent_events output for tests.
``events`` is the frontend tool-selection sink to drive suppression /
selection rendering (defaults to the silent NoOpSink inside the stream).
``configurable_extra`` is forwarded verbatim to ``stream_agent_events``
for tests that assert plumbing into the LangGraph ``configurable`` dict.
"""
collected = []
async for ev in stream_agent_events(
@@ -34,6 +37,7 @@ async def collect_events(
message,
thread_id,
events=events,
configurable_extra=configurable_extra,
):
collected.append(ev)
return collected
+217
View File
@@ -0,0 +1,217 @@
"""Tests for EvoScientist.middleware.active_team."""
from __future__ import annotations
from types import SimpleNamespace
from unittest.mock import MagicMock, patch
from langchain_core.messages import SystemMessage
from EvoScientist.middleware.active_team import (
ActiveTeamMiddleware,
_read_active_teams,
create_active_team_middleware,
)
def _request():
"""A minimal ModelRequest stand-in supporting the fields the middleware
reads (`system_message`) and the `.override(**kwargs)` mutator."""
request = SimpleNamespace(
state={},
runtime=object(),
system_message=SystemMessage(content="base system"),
)
request.override = lambda **kwargs: SimpleNamespace(
**{
"state": request.state,
"runtime": request.runtime,
"system_message": kwargs.get("system_message", request.system_message),
}
)
return request
def _system_text(modified) -> str:
system_message = modified.system_message
assert system_message is not None
return str(system_message.content)
def _mock_config():
cfg = MagicMock()
cfg.enable_ask_user = False
cfg.auto_mode = False
cfg.auto_approve = False
cfg.model_fallbacks = None
cfg.auxiliary_model = ""
cfg.auxiliary_provider = ""
cfg.code_interpreter_timeout = 60
cfg.code_interpreter_max_result_chars = 6000
return cfg
# ---- unit tests: _read_active_teams behavior --------------------------------
@patch("langgraph.config.get_config")
def test_read_active_teams_returns_list_when_present(mock_get_config):
mock_get_config.return_value = {
"configurable": {"active_teams": ["idea-brainstorm"]},
}
assert _read_active_teams() == ["idea-brainstorm"]
@patch("langgraph.config.get_config")
def test_read_active_teams_returns_empty_when_configurable_missing(mock_get_config):
mock_get_config.return_value = {}
assert _read_active_teams() == []
@patch("langgraph.config.get_config")
def test_read_active_teams_returns_empty_when_active_teams_missing(mock_get_config):
mock_get_config.return_value = {"configurable": {"other_field": "x"}}
assert _read_active_teams() == []
@patch("langgraph.config.get_config")
def test_read_active_teams_returns_empty_when_value_not_list(mock_get_config):
"""WebUI mistakenly sends a scalar instead of a list; must not crash."""
mock_get_config.return_value = {
"configurable": {"active_teams": "idea-brainstorm"},
}
assert _read_active_teams() == []
@patch("langgraph.config.get_config")
def test_read_active_teams_filters_non_string_entries(mock_get_config):
mock_get_config.return_value = {
"configurable": {
"active_teams": ["idea-brainstorm", None, 42, "", "lit-review"]
},
}
assert _read_active_teams() == ["idea-brainstorm", "lit-review"]
@patch("langgraph.config.get_config", side_effect=RuntimeError("outside context"))
def test_read_active_teams_returns_empty_outside_runnable_context(mock_get_config):
assert _read_active_teams() == []
# ---- unit tests: middleware behavior ---------------------------------------
@patch("langgraph.config.get_config")
def test_middleware_no_op_when_active_teams_absent(mock_get_config):
mock_get_config.return_value = {"configurable": {}}
middleware = ActiveTeamMiddleware()
request = _request()
modified = middleware.modify_request(request)
# No override applied: original request returned as-is.
assert modified is request
@patch("langgraph.config.get_config")
def test_middleware_no_op_when_active_teams_empty_list(mock_get_config):
mock_get_config.return_value = {"configurable": {"active_teams": []}}
middleware = ActiveTeamMiddleware()
request = _request()
modified = middleware.modify_request(request)
assert modified is request
@patch("langgraph.config.get_config")
def test_middleware_appends_single_expert_cue(mock_get_config):
mock_get_config.return_value = {
"configurable": {"active_teams": ["idea-brainstorm"]},
}
middleware = ActiveTeamMiddleware()
modified = middleware.modify_request(_request())
text = _system_text(modified)
assert "<active_expert>" in text
assert "`idea-brainstorm`" in text
assert "Consult it via `task(" in text
assert "base system" in text # original preserved
@patch("langgraph.config.get_config")
def test_middleware_appends_multi_expert_cue(mock_get_config):
mock_get_config.return_value = {
"configurable": {"active_teams": ["idea-brainstorm", "literature-review"]},
}
middleware = ActiveTeamMiddleware()
modified = middleware.modify_request(_request())
text = _system_text(modified)
assert "<active_experts>" in text
assert "`idea-brainstorm`" in text
assert "`literature-review`" in text
assert "Consult any of them" in text
assert "base system" in text
@patch("langgraph.config.get_config")
def test_middleware_appends_cue_for_unknown_expert_names(mock_get_config):
"""Middleware doesn't validate names against the registry; main decides."""
mock_get_config.return_value = {
"configurable": {"active_teams": ["nonexistent-expert"]},
}
middleware = ActiveTeamMiddleware()
modified = middleware.modify_request(_request())
text = _system_text(modified)
assert "`nonexistent-expert`" in text
@patch("langgraph.config.get_config", side_effect=RuntimeError("outside context"))
def test_middleware_no_op_outside_runnable_context(mock_get_config):
middleware = ActiveTeamMiddleware()
request = _request()
modified = middleware.modify_request(request)
assert modified is request
# ---- composition tests: _get_default_middleware ----------------------------
@patch(
"EvoScientist.middleware.create_tool_selector_middleware",
return_value=[MagicMock(), MagicMock()],
)
@patch("EvoScientist.EvoScientist._ensure_chat_model")
@patch("EvoScientist.EvoScientist._ensure_config")
def test_default_middleware_includes_active_team_for_main_agent(
mock_config, mock_model, mock_tool_selector
):
mock_config.return_value = _mock_config()
mock_model.return_value = MagicMock(profile={"max_input_tokens": 200_000})
from EvoScientist.EvoScientist import _get_default_middleware
middleware = _get_default_middleware()
assert any(isinstance(m, ActiveTeamMiddleware) for m in middleware)
@patch(
"EvoScientist.middleware.create_tool_selector_middleware",
return_value=[MagicMock(), MagicMock()],
)
@patch("EvoScientist.EvoScientist._ensure_chat_model")
@patch("EvoScientist.EvoScientist._ensure_config")
def test_default_middleware_excludes_active_team_for_async_subagent(
mock_config, mock_model, mock_tool_selector
):
mock_config.return_value = _mock_config()
mock_model.return_value = MagicMock(profile={"max_input_tokens": 200_000})
from EvoScientist.EvoScientist import _get_default_middleware
middleware = _get_default_middleware(for_async_subagent=True)
assert not any(isinstance(m, ActiveTeamMiddleware) for m in middleware)
# ---- factory --------------------------------------------------------------
def test_factory_returns_middleware_instance():
assert isinstance(create_active_team_middleware(), ActiveTeamMiddleware)
+50
View File
@@ -0,0 +1,50 @@
"""Unit tests for helpers in ``EvoScientist.commands.base``."""
from __future__ import annotations
from EvoScientist.commands.base import ChannelRuntime, active_teams_configurable_extra
class TestActiveTeamsConfigurableExtra:
"""``active_teams_configurable_extra`` is used at every stream-call site
that needs to forward /expert invites into ``RunRequest.configurable_extra``.
"""
def test_none_runtime_returns_none(self):
assert active_teams_configurable_extra(None) is None
def test_runtime_without_invites_returns_none(self):
# Empty list must produce ``None`` so callers can pass the result
# unconditionally without polluting ``configurable`` with an empty
# ``active_teams: []`` (which ``ActiveTeamMiddleware`` would treat
# as no-op anyway, but the wire stays cleaner without it).
runtime = ChannelRuntime()
assert active_teams_configurable_extra(runtime) is None
def test_runtime_with_invites_returns_dict_copy(self):
runtime = ChannelRuntime()
runtime.active_teams = ["idea-brainstorm", "paper-review"]
result = active_teams_configurable_extra(runtime)
assert result == {"active_teams": ["idea-brainstorm", "paper-review"]}
# Must be a *copy* — mutating the returned list may not leak back
# to the runtime's session-scoped invite list.
result["active_teams"].append("mutated")
assert runtime.active_teams == ["idea-brainstorm", "paper-review"]
class TestChannelRuntimeClear:
"""``ChannelRuntime.clear`` runs on channel shutdown; it must leave the
session-scoped ``active_teams`` list intact so stopping a channel does
not silently dismiss the user's invited experts. ``/new`` and
``/expert clear`` handle invite reset explicitly.
"""
def test_clear_preserves_active_teams(self):
runtime = ChannelRuntime()
runtime.agent = object()
runtime.thread_id = "t-42"
runtime.active_teams = ["idea-brainstorm"]
runtime.clear()
assert runtime.agent is None
assert runtime.thread_id is None
assert runtime.active_teams == ["idea-brainstorm"]
+234
View File
@@ -0,0 +1,234 @@
"""Unit tests for /experts and /expert slash commands."""
from __future__ import annotations
from dataclasses import dataclass, field
from typing import Any
from unittest.mock import patch
import pytest
from EvoScientist.commands.base import ChannelRuntime, CommandContext
from EvoScientist.commands.implementation.experts import (
ExpertCommand,
ExpertsCommand,
invalidate_experts_cache,
)
@pytest.fixture(autouse=True)
def _bust_experts_cache_between_tests():
"""The dispatchable-experts cache in ``experts.py`` is module-level; without
resetting it, a test that patches ``list_expert_skills`` sees the previous
test's fakes.
"""
invalidate_experts_cache()
yield
invalidate_experts_cache()
class _FakeUI:
"""Minimal CommandUI capturing outputs for assertion."""
supports_interactive = False
def __init__(self) -> None:
self.lines: list[tuple[str, str]] = []
self.mounted: list[Any] = []
def append_system(self, text: str, style: str = "dim") -> None:
self.lines.append((text, style))
def mount_renderable(self, renderable: Any) -> None:
self.mounted.append(renderable)
@dataclass
class _FakeSkillInfo:
"""Enough of ``SkillInfo`` for the commands to render."""
name: str
description: str = ""
role: str = ""
default_dispatch: str = ""
type: str = "expert"
tags: list[str] = field(default_factory=list)
source: str = "builtin"
# Non-empty by default so the fake passes the empty-body filter in
# ``list_dispatchable_experts``. Tests that specifically want to
# exercise the empty-body reject path pass ``body=""``.
body: str = "persona"
def _make_ctx(active_teams: list[str] | None = None) -> tuple[CommandContext, _FakeUI]:
ui = _FakeUI()
runtime = ChannelRuntime()
if active_teams:
runtime.active_teams = list(active_teams)
ctx = CommandContext(
agent=None,
thread_id="t1",
ui=ui,
channel_runtime=runtime,
)
return ctx, ui
class TestExpertsList:
async def test_lists_installed_experts_in_table(self):
ctx, ui = _make_ctx()
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[
_FakeSkillInfo(
name="idea-brainstorm",
role="Research idea brainstormer",
default_dispatch="sync",
),
],
):
await ExpertsCommand().execute(ctx, args=[])
# A Rich Table was mounted, and the no-experts-invited hint appeared.
assert len(ui.mounted) == 1
assert any("No experts invited" in text for text, _ in ui.lines)
async def test_empty_list_prints_help_hint(self):
ctx, ui = _make_ctx()
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[],
):
await ExpertsCommand().execute(ctx, args=[])
assert any("No expert skills installed" in text for text, _ in ui.lines)
assert not ui.mounted
async def test_active_expert_marked_in_table(self):
ctx, ui = _make_ctx(active_teams=["idea-brainstorm"])
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[
_FakeSkillInfo(
name="idea-brainstorm",
role="Research idea brainstormer",
default_dispatch="sync",
),
],
):
await ExpertsCommand().execute(ctx, args=[])
assert any("Active: idea-brainstorm" in text for text, _ in ui.lines)
class TestExpertToggle:
async def test_missing_arg_prints_usage(self):
ctx, ui = _make_ctx()
await ExpertCommand().execute(ctx, args=[])
assert any("Usage:" in text for text, _ in ui.lines)
async def test_unknown_expert_errors(self):
ctx, ui = _make_ctx()
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[_FakeSkillInfo(name="idea-brainstorm")],
):
await ExpertCommand().execute(ctx, args=["not-an-expert"])
assert any(
"No expert skill named 'not-an-expert'" in text for text, _ in ui.lines
)
assert ctx.channel_runtime.active_teams == []
async def test_invite_adds_to_active_teams(self):
ctx, ui = _make_ctx()
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[_FakeSkillInfo(name="idea-brainstorm")],
):
await ExpertCommand().execute(ctx, args=["idea-brainstorm"])
assert ctx.channel_runtime.active_teams == ["idea-brainstorm"]
assert any("Invited expert: idea-brainstorm" in text for text, _ in ui.lines)
async def test_toggle_dismisses_when_already_invited(self):
ctx, ui = _make_ctx(active_teams=["idea-brainstorm"])
with patch(
"EvoScientist.tools.skills_manager.list_expert_skills",
return_value=[_FakeSkillInfo(name="idea-brainstorm")],
):
await ExpertCommand().execute(ctx, args=["idea-brainstorm"])
assert ctx.channel_runtime.active_teams == []
assert any("Dismissed expert: idea-brainstorm" in text for text, _ in ui.lines)
async def test_clear_dismisses_all(self):
ctx, ui = _make_ctx(active_teams=["idea-brainstorm", "second"])
await ExpertCommand().execute(ctx, args=["clear"])
assert ctx.channel_runtime.active_teams == []
assert any(
"Dismissed experts: idea-brainstorm, second" in text for text, _ in ui.lines
)
async def test_clear_on_empty_list_reports_nothing_to_do(self):
ctx, ui = _make_ctx()
await ExpertCommand().execute(ctx, args=["clear"])
assert ctx.channel_runtime.active_teams == []
assert any("No experts invited" in text for text, _ in ui.lines)
async def test_no_channel_runtime_prints_warning(self):
ui = _FakeUI()
ctx = CommandContext(agent=None, thread_id="t1", ui=ui, channel_runtime=None)
await ExpertCommand().execute(ctx, args=["idea-brainstorm"])
assert any("/expert requires a session runtime" in text for text, _ in ui.lines)
class TestExpertCompletions:
"""``ExpertCommand.get_completions`` mixes dynamic expert names with the
static ``clear`` subcommand. Regression coverage for the three fixes on
PR #371: exact-match suppression, past-first-arg guard, and
case-insensitive matching.
"""
def _patched_experts(self, *names: str):
# Patch ``list_dispatchable_experts`` directly (not the underlying
# ``list_expert_skills``) so the test does not depend on the shipped
# yaml sub-agent set — the reserved-name filter would otherwise
# silently reject a fake whose name collides with a future yaml
# sub-agent.
return patch(
"EvoScientist.subagents.expert_container.list_dispatchable_experts",
return_value=[_FakeSkillInfo(name=n) for n in names],
)
def test_lists_installed_experts_and_clear(self):
cmd = ExpertCommand()
with self._patched_experts("smoke-test-sync-expert", "smoke-test-alt-expert"):
completions = cmd.get_completions([""])
names = {name for name, _ in completions}
assert names == {"smoke-test-sync-expert", "smoke-test-alt-expert", "clear"}
def test_case_insensitive_prefix_match(self):
# Skill dir names sometimes have uppercase; completion typed
# lowercase must still surface them.
cmd = ExpertCommand()
with self._patched_experts("Smoke-Test-Case-Expert", "smoke-test-sync-expert"):
completions = cmd.get_completions(["smoke-test-c"])
names = {name for name, _ in completions}
assert names == {"Smoke-Test-Case-Expert"}
def test_exact_match_hides_popup_same_case(self):
cmd = ExpertCommand()
with self._patched_experts("smoke-test-sync-expert"):
completions = cmd.get_completions(["smoke-test-sync-expert"])
assert completions == []
def test_exact_match_hides_popup_different_case(self):
# Case-insensitive exact-match suppression: typing the name in a
# different case than the skill dir still fully completes it and
# hides the popup.
cmd = ExpertCommand()
with self._patched_experts("Smoke-Test-Case-Expert"):
completions = cmd.get_completions(["smoke-test-case-expert"])
assert completions == []
def test_past_first_arg_returns_empty(self):
# /expert takes a single positional. Trailing space -> tokens == ["n", ""].
cmd = ExpertCommand()
with self._patched_experts("smoke-test-sync-expert"):
assert cmd.get_completions(["smoke-test-sync-expert", ""]) == []
assert cmd.get_completions(["smoke-test-sync-expert", "foo"]) == []
+51
View File
@@ -34,3 +34,54 @@ class TestNewCommand:
ctx = CommandContext(agent=None, thread_id="tid", ui=ui)
# No AttributeError even though ctx.agent is None
await NewCommand().execute(ctx, [])
async def test_clears_invited_experts_and_announces(self):
"""/new dismisses invited experts uniformly with channel-shutdown clear."""
from EvoScientist.commands.base import ChannelRuntime, CommandContext
from EvoScientist.commands.implementation.session import NewCommand
ui = MagicMock()
ui.start_new_session = AsyncMock()
runtime = ChannelRuntime()
runtime.active_teams = ["idea-brainstorm"]
ctx = CommandContext(
agent=None,
thread_id="tid",
ui=ui,
channel_runtime=runtime,
)
await NewCommand().execute(ctx, [])
assert runtime.active_teams == []
messages = [call.args[0] for call in ui.append_system.call_args_list]
assert any(
"Dismissed experts on new session: idea-brainstorm" in msg
for msg in messages
)
async def test_no_announcement_when_no_experts_invited(self):
"""No noise on ``/new`` when the invite list is already empty."""
from EvoScientist.commands.base import ChannelRuntime, CommandContext
from EvoScientist.commands.implementation.session import NewCommand
ui = MagicMock()
ui.start_new_session = AsyncMock()
runtime = ChannelRuntime()
ctx = CommandContext(
agent=None,
thread_id="tid",
ui=ui,
channel_runtime=runtime,
)
await NewCommand().execute(ctx, [])
ui.append_system.assert_not_called()
async def test_no_announcement_without_channel_runtime(self):
"""Runs cleanly when no ChannelRuntime is attached."""
from EvoScientist.commands.base import CommandContext
from EvoScientist.commands.implementation.session import NewCommand
ui = MagicMock()
ui.start_new_session = AsyncMock()
ctx = CommandContext(agent=None, thread_id="tid", ui=ui, channel_runtime=None)
await NewCommand().execute(ctx, [])
ui.append_system.assert_not_called()
+67
View File
@@ -12,6 +12,7 @@ from EvoScientist.tools.skills_manager import (
_parse_github_url,
_parse_skill_md,
_record_install,
_reset_skills_changed_callbacks,
_validate_skill_dir,
fetch_remote_skill_index,
get_all_tags,
@@ -21,6 +22,7 @@ from EvoScientist.tools.skills_manager import (
list_expert_skills,
list_skills,
list_skills_by_tag,
register_skills_changed_callback,
resolve_remote_head,
uninstall_skill,
)
@@ -1589,3 +1591,68 @@ class TestSkillManagerInstall:
assert "Path: /skills/worked" in result
assert "broken" in result
assert "corrupt frontmatter" in result
# =============================================================================
# Tests for the skills-changed publish primitive
# =============================================================================
@pytest.fixture
def isolated_skills_changed_callbacks():
"""Clear the module-level callback list before and after each test so
subscribers registered elsewhere (e.g. by importing ``experts.py``) do
not leak in or out of these tests.
"""
_reset_skills_changed_callbacks()
yield
_reset_skills_changed_callbacks()
class TestSkillsChangedCallback:
"""Verifies install_skill / uninstall_skill fire subscribers on every
return path — success, early error return, and success-with-real-mutation.
"""
def test_install_skill_fires_callback_on_error_return(
self, isolated_skills_changed_callbacks, temp_skills_dir
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = install_skill("/nonexistent/path", str(temp_skills_dir))
assert result["success"] is False
assert fired == [True]
def test_install_skill_fires_callback_on_success(
self, isolated_skills_changed_callbacks, sample_skill_dir, temp_skills_dir
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = install_skill(str(sample_skill_dir), str(temp_skills_dir))
assert result["success"] is True
assert fired == [True]
def test_uninstall_skill_fires_callback_on_error_return(
self, isolated_skills_changed_callbacks
):
fired: list[bool] = []
register_skills_changed_callback(lambda: fired.append(True))
result = uninstall_skill("nonexistent-skill")
assert result["success"] is False
assert fired == [True]
def test_misbehaving_callback_does_not_break_return(
self, isolated_skills_changed_callbacks, temp_skills_dir
):
good: list[bool] = []
def bad_callback() -> None:
raise RuntimeError("subscriber intentionally raising")
register_skills_changed_callback(bad_callback)
register_skills_changed_callback(lambda: good.append(True))
# Bad callback runs first; the good one still fires; the install
# return value is unaffected.
result = install_skill("/nonexistent/path", str(temp_skills_dir))
assert result["success"] is False
assert good == [True]
+20
View File
@@ -158,6 +158,26 @@ class TestV3ProtocolStreaming:
assert "stream_mode" not in kwargs
assert "subgraphs" not in kwargs
async def test_configurable_extra_merged_into_config(self):
"""``configurable_extra`` from RunRequest lands next to thread_id."""
agent = FakeV3Agent([message_delta("hi")])
await collect_events(
agent,
thread_id="t1",
configurable_extra={"active_teams": ["idea-brainstorm"]},
)
_, kwargs = agent.astream_events.call_args
configurable = kwargs["config"]["configurable"]
assert configurable["thread_id"] == "t1"
assert configurable["active_teams"] == ["idea-brainstorm"]
async def test_configurable_extra_none_leaves_thread_id_only(self):
"""When no extras are passed, only ``thread_id`` sits under configurable."""
agent = FakeV3Agent([message_delta("hi")])
await collect_events(agent, thread_id="t1")
_, kwargs = agent.astream_events.call_args
assert kwargs["config"]["configurable"] == {"thread_id": "t1"}
async def test_streamed_non_selector_json_is_replayed(self):
"""Normal JSON answers are not swallowed by selector JSON buffering."""
agent = FakeV3Agent(