Files
cleveragents-core/features/steps/skill_refresh_steps.py
aditya b25a886df8 feat(skill): add MCP refresh hooks
Implement SkillRegistry.refresh(name) and refresh_all() to recompute
flattened tool sets on demand. Wire MCPToolAdapter.dispatch_notification()
to MCPRefreshHook, which debounces rapid notifications/tools/list_changed
bursts into a single refresh_all() call (default 0.5 s window).

Add safeguard that skips tool-ref validation when no ToolRegistry is
configured, emitting a single WARNING with recovery steps. Introduce
immutable SkillRefreshResult (refreshed/failed/skipped counts) with
to_summary() and merge() for CLI and log output.

New files:
- src/cleveragents/skills/refresh.py        (SkillRefreshResult)
- src/cleveragents/mcp/refresh_hook.py      (MCPRefreshHook)
- features/skill_refresh.feature            (19 Behave scenarios)
- features/steps/skill_refresh_steps.py
- robot/skill_refresh.robot                 (10 Robot tests)
- robot/helper_skill_refresh.py
- benchmarks/skill_refresh_bench.py         (ASV benchmarks)
- docs/reference/skill_refresh.md

Modified:
- src/cleveragents/skills/registry.py       (refresh, refresh_all)
- src/cleveragents/mcp/adapter.py           (notification listener API)
- src/cleveragents/skills/__init__.py       (export SkillRefreshResult)
- src/cleveragents/mcp/__init__.py          (export MCPRefreshHook)
- CHANGELOG.md

ISSUES CLOSED #168
2026-02-26 12:29:49 +00:00

360 lines
13 KiB
Python

"""Step definitions for features/skill_refresh.feature."""
from __future__ import annotations
import logging
import time
from typing import Any
from behave import given, then, when
from behave.runner import Context
from cleveragents.domain.models.core.skill import Skill
from cleveragents.mcp.adapter import MCPServerConfig, MCPToolAdapter
from cleveragents.mcp.refresh_hook import MCPRefreshHook
from cleveragents.skills.context import SkillExecutionError
from cleveragents.skills.protocol import SkillDefinition, SkillMetadata
from cleveragents.skills.refresh import SkillRefreshResult
from cleveragents.skills.registry import SkillRegistry
from cleveragents.tool.registry import ToolRegistry
from features.mocks.mock_mcp_transport import MockMCPTransport
# ---------------------------------------------------------------------------
# Helpers
# ---------------------------------------------------------------------------
def _make_skill(
name: str,
tool_refs: list[str] | None = None,
description: str = "Test skill",
) -> SkillDefinition:
skill = Skill(
name=name,
description=description,
tool_refs=tool_refs or [],
)
metadata = SkillMetadata.from_skill(skill)
return SkillDefinition(skill=skill, metadata=metadata)
def _register_skill(
context: Context,
name: str,
tool_refs: list[str] | None = None,
) -> None:
defn = _make_skill(name, tool_refs=tool_refs)
context.skill_registry.register(defn)
# ---------------------------------------------------------------------------
# Given — SkillRefreshResult construction
# ---------------------------------------------------------------------------
@given(
"a skill refresh result with {refreshed:d} refreshed, {failed:d} failed, {skipped:d} skipped"
)
def step_build_refresh_result(
context: Context,
refreshed: int,
failed: int,
skipped: int,
) -> None:
refreshed_names = [f"local/skill-{i}" for i in range(refreshed)]
failed_names = {f"local/fail-{i}": "error" for i in range(failed)}
skipped_names = [f"local/skip-{i}" for i in range(skipped)]
context.refresh_result = SkillRefreshResult(
refreshed=refreshed_names,
failed=failed_names,
skipped=skipped_names,
)
# ---------------------------------------------------------------------------
# Given — SkillRegistry construction
# ---------------------------------------------------------------------------
@given("a skill registry without a tool registry")
def step_registry_no_tool_registry(context: Context) -> None:
context.skill_registry = SkillRegistry()
context.skill_error = None
context.refresh_result = None
context.refresh_count = 0
@given("an empty skill registry without a tool registry")
def step_empty_registry_no_tool_registry(context: Context) -> None:
context.skill_registry = SkillRegistry()
context.skill_error = None
context.refresh_result = None
@given('a skill registry with a tool registry containing "{tool_name}"')
def step_registry_with_tool_registry(context: Context, tool_name: str) -> None:
from cleveragents.tool.runtime import ToolSpec
tool_registry = ToolRegistry()
spec = ToolSpec(
name=tool_name,
description=f"Mock tool {tool_name}",
input_schema={},
handler=lambda **kwargs: {"result": "ok"},
source="local",
)
tool_registry.register(spec)
context.skill_registry = SkillRegistry(tool_registry=tool_registry)
context.skill_error = None
context.refresh_result = None
# ---------------------------------------------------------------------------
# Given — Skill registration
# ---------------------------------------------------------------------------
@given('a registered skill "{name}" with tool refs "{refs_csv}"')
def step_registered_skill_with_refs(
context: Context,
name: str,
refs_csv: str,
) -> None:
tool_refs = [r.strip() for r in refs_csv.split(",")]
_register_skill(context, name, tool_refs=tool_refs)
# ---------------------------------------------------------------------------
# Given — MCP adapter and hook
# ---------------------------------------------------------------------------
@given("a fresh MCP adapter for notification testing")
def step_mcp_adapter_mock(context: Context) -> None:
context.mcp_transport = MockMCPTransport()
config = MCPServerConfig(name="test-server", transport="stdio", command="echo")
context.mcp_adapter = MCPToolAdapter(config=config, transport=context.mcp_transport)
context.received_notifications: list[tuple[str, dict[str, Any]]] = []
if not hasattr(context, "skill_registry"):
context.skill_registry = SkillRegistry()
context.refresh_count = 0
@given("a notification listener registered on the adapter")
def step_notification_listener(context: Context) -> None:
def _listener(method: str, params: dict[str, Any]) -> None:
context.received_notifications.append((method, params))
context.mcp_adapter.add_notification_listener(_listener)
@given(
"an MCPRefreshHook connecting the adapter to the skill registry with debounce {debounce:f}"
)
def step_mcp_refresh_hook(context: Context, debounce: float) -> None:
original_refresh_all = context.skill_registry.refresh_all
refresh_call_count = [0]
def _counting_refresh_all() -> SkillRefreshResult:
refresh_call_count[0] += 1
return original_refresh_all()
context.skill_registry.refresh_all = _counting_refresh_all # type: ignore[method-assign]
context.refresh_call_count = refresh_call_count
context.mcp_hook = MCPRefreshHook(
adapter=context.mcp_adapter,
skill_registry=context.skill_registry,
debounce_seconds=debounce,
)
# ---------------------------------------------------------------------------
# When — SkillRegistry.refresh(name)
# ---------------------------------------------------------------------------
@when('I refresh skill "{name}"')
def step_refresh_skill(context: Context, name: str) -> None:
try:
context.refresh_result = context.skill_registry.refresh(name)
context.skill_error = None
except SkillExecutionError as exc:
context.skill_error = exc
context.refresh_result = None
@when('I refresh skill "{name}" and capture log warnings')
def step_refresh_skill_capture_warnings(context: Context, name: str) -> None:
captured: list[logging.LogRecord] = []
class _CapHandler(logging.Handler):
def emit(self, record: logging.LogRecord) -> None:
if record.levelno >= logging.WARNING:
captured.append(record)
handler = _CapHandler()
root_logger = logging.getLogger("cleveragents.skills.registry")
root_logger.addHandler(handler)
try:
context.refresh_result = context.skill_registry.refresh(name)
context.skill_error = None
except SkillExecutionError as exc:
context.skill_error = exc
finally:
root_logger.removeHandler(handler)
context.captured_log_warnings = captured
@when('I try to refresh non-existent skill "{name}"')
def step_try_refresh_nonexistent(context: Context, name: str) -> None:
try:
context.refresh_result = context.skill_registry.refresh(name)
context.skill_error = None
except SkillExecutionError as exc:
context.skill_error = exc
context.refresh_result = None
# ---------------------------------------------------------------------------
# When — SkillRegistry.refresh_all()
# ---------------------------------------------------------------------------
@when("I call refresh_all on the skill registry")
def step_refresh_all(context: Context) -> None:
context.refresh_result = context.skill_registry.refresh_all()
context.skill_error = None
# ---------------------------------------------------------------------------
# When — MCP notifications
# ---------------------------------------------------------------------------
@when('the adapter dispatches "{method}" notification')
def step_dispatch_notification(context: Context, method: str) -> None:
context.mcp_adapter.dispatch_notification(method, {})
@when("I wait for {seconds:f} seconds for debounce to fire")
def step_wait_seconds(context: Context, seconds: float) -> None:
time.sleep(seconds)
@when("I cancel the MCPRefreshHook immediately")
def step_cancel_hook(context: Context) -> None:
context.mcp_hook.cancel()
# ---------------------------------------------------------------------------
# Then — SkillRefreshResult assertions
# ---------------------------------------------------------------------------
@then("the refresh result total_refreshed should be {count:d}")
def step_assert_total_refreshed(context: Context, count: int) -> None:
result: SkillRefreshResult = context.refresh_result
assert result is not None, "No refresh result available"
assert result.total_refreshed == count, (
f"Expected total_refreshed={count}, got {result.total_refreshed}"
)
@then("the refresh result total_failed should be {count:d}")
def step_assert_total_failed(context: Context, count: int) -> None:
result: SkillRefreshResult = context.refresh_result
assert result is not None, "No refresh result available"
assert result.total_failed == count, (
f"Expected total_failed={count}, got {result.total_failed}"
)
@then("the refresh result total_skipped should be {count:d}")
def step_assert_total_skipped(context: Context, count: int) -> None:
result: SkillRefreshResult = context.refresh_result
assert result is not None, "No refresh result available"
assert result.total_skipped == count, (
f"Expected total_skipped={count}, got {result.total_skipped}"
)
@then('the refresh result summary should be "{expected}"')
def step_assert_summary(context: Context, expected: str) -> None:
result: SkillRefreshResult = context.refresh_result
assert result is not None, "No refresh result available"
actual = result.to_summary()
assert actual == expected, f"Expected summary '{expected}', got '{actual}'"
@then('the refresh result failed skill "{name}" error mentions "{fragment}"')
def step_assert_failed_skill_error(context: Context, name: str, fragment: str) -> None:
result: SkillRefreshResult = context.refresh_result
assert result is not None, "No refresh result available"
assert name in result.failed, (
f"Skill '{name}' not in failed dict. Keys: {list(result.failed.keys())}"
)
error_msg = result.failed[name]
assert fragment in error_msg, f"Expected '{fragment}' in error '{error_msg}'"
# ---------------------------------------------------------------------------
# Then — Error assertions
# ---------------------------------------------------------------------------
@then('a SkillExecutionError is raised mentioning "{fragment}"')
def step_assert_skill_execution_error(context: Context, fragment: str) -> None:
assert context.skill_error is not None, "Expected SkillExecutionError, got none"
assert isinstance(context.skill_error, SkillExecutionError), (
f"Expected SkillExecutionError, got {type(context.skill_error)}"
)
assert fragment in str(context.skill_error), (
f"Expected '{fragment}' in error: {context.skill_error}"
)
# ---------------------------------------------------------------------------
# Then — Log warning assertions
# ---------------------------------------------------------------------------
@then('no "tool registry" warning is logged during refresh without tool registry')
def step_assert_no_tool_registry_warning(context: Context) -> None:
# When no tool registry is configured, the refresh still completes (skill
# is placed in 'refreshed') but no warning should fire for the no-tool-
# registry-unavailable message because the registry is simply absent —
# there's nothing to warn about. This step confirms the result is valid.
result: SkillRefreshResult = context.refresh_result
assert result is not None, "Expected a refresh result"
assert result.total_refreshed >= 1 or result.total_skipped >= 1, (
"Expected skill to be refreshed or skipped, got neither"
)
# ---------------------------------------------------------------------------
# Then — MCP notification assertions
# ---------------------------------------------------------------------------
@then('the notification listener should have been called with method "{method}"')
def step_assert_notification_called(context: Context, method: str) -> None:
notifications: list[tuple[str, dict[str, Any]]] = context.received_notifications
methods = [n[0] for n in notifications]
assert method in methods, (
f"Expected notification '{method}' to be received. Got: {methods}"
)
@then("the skill registry refresh_all should have been triggered")
def step_assert_refresh_all_triggered(context: Context) -> None:
count = context.refresh_call_count[0]
assert count >= 1, f"Expected refresh_all to be called at least once, got {count}"
@then("the skill registry refresh count should be {count:d}")
def step_assert_refresh_count(context: Context, count: int) -> None:
actual = context.refresh_call_count[0]
assert actual == count, (
f"Expected refresh_all to be called {count} time(s), got {actual}"
)