b25a886df8
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 20s
CI / lint (pull_request) Successful in 24s
CI / build (pull_request) Successful in 27s
CI / typecheck (pull_request) Successful in 34s
CI / security (pull_request) Successful in 1m11s
CI / integration_tests (pull_request) Successful in 4m40s
CI / unit_tests (pull_request) Successful in 9m55s
CI / docker (pull_request) Successful in 1m0s
CI / benchmark-regression (pull_request) Successful in 25m30s
CI / coverage (pull_request) Successful in 1h11m59s
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
291 lines
8.3 KiB
Markdown
291 lines
8.3 KiB
Markdown
# Skill Refresh Hooks
|
||
|
||
This document covers the **M4 skill-registry refresh** feature: dynamic recomputation
|
||
of skill tool sets triggered by `notifications/tools/list_changed` events from MCP
|
||
servers.
|
||
|
||
**Module:** `cleveragents.skills.refresh`, `cleveragents.skills.registry`,
|
||
`cleveragents.mcp.refresh_hook`
|
||
|
||
---
|
||
|
||
## Overview
|
||
|
||
| Component | Purpose |
|
||
|-----------|---------|
|
||
| `SkillRefreshResult` | Immutable summary of a refresh operation |
|
||
| `SkillRegistry.refresh(name)` | Recompute a single skill's tool set |
|
||
| `SkillRegistry.refresh_all()` | Recompute all registered skills |
|
||
| `MCPRefreshHook` | Wire MCP notifications to `refresh_all()` with debouncing |
|
||
|
||
When an MCP server's tool list changes, the adapter broadcasts a
|
||
`notifications/tools/list_changed` notification. `MCPRefreshHook` listens for this
|
||
event and triggers `SkillRegistry.refresh_all()` after a configurable debounce
|
||
window, ensuring rapid successive notifications collapse into a single refresh.
|
||
|
||
---
|
||
|
||
## SkillRefreshResult
|
||
|
||
**Module:** `cleveragents.skills.refresh`
|
||
**Export:** `cleveragents.skills.SkillRefreshResult`
|
||
|
||
An immutable (`frozen=True`) dataclass summarising the outcome of one or more
|
||
refresh operations.
|
||
|
||
### Fields
|
||
|
||
| Field | Type | Description |
|
||
|-------|------|-------------|
|
||
| `refreshed` | `list[str]` | Skill names successfully recomputed |
|
||
| `failed` | `dict[str, str]` | Mapping of skill name → error message for each failure |
|
||
| `skipped` | `list[str]` | Skill names skipped (tool registry unavailable) |
|
||
|
||
### Properties
|
||
|
||
| Property | Type | Description |
|
||
|----------|------|-------------|
|
||
| `total_refreshed` | `int` | `len(refreshed)` |
|
||
| `total_failed` | `int` | `len(failed)` |
|
||
| `total_skipped` | `int` | `len(skipped)` |
|
||
|
||
### Methods
|
||
|
||
#### `to_summary() → str`
|
||
|
||
Returns a human-readable one-liner suitable for CLI output or log messages.
|
||
|
||
```
|
||
"refreshed: 3, failed: 1, skipped: 0"
|
||
```
|
||
|
||
#### `merge(other: SkillRefreshResult) → SkillRefreshResult`
|
||
|
||
Combines two results into one by concatenating the `refreshed` and `skipped`
|
||
lists and merging the `failed` dicts. Used internally by `refresh_all()` to
|
||
aggregate per-skill results.
|
||
|
||
### Example
|
||
|
||
```python
|
||
from cleveragents.skills import SkillRefreshResult
|
||
|
||
r1 = SkillRefreshResult(refreshed=["ns/skill-a"], failed={}, skipped=[])
|
||
r2 = SkillRefreshResult(refreshed=[], failed={"ns/skill-b": "tool missing"}, skipped=[])
|
||
merged = r1.merge(r2)
|
||
print(merged.to_summary()) # "refreshed: 1, failed: 1, skipped: 0"
|
||
```
|
||
|
||
---
|
||
|
||
## SkillRegistry Refresh Methods
|
||
|
||
**Module:** `cleveragents.skills.registry`
|
||
|
||
### `refresh(name: str) → SkillRefreshResult`
|
||
|
||
Recomputes the flattened tool set for a single registered skill and validates tool
|
||
references against the `ToolRegistry` (if one is configured on the registry).
|
||
|
||
**Parameters**
|
||
|
||
| Parameter | Type | Description |
|
||
|-----------|------|-------------|
|
||
| `name` | `str` | Namespaced skill name to refresh |
|
||
|
||
**Returns:** `SkillRefreshResult` — skill appears in `refreshed`, `failed`, or
|
||
`skipped` depending on the outcome.
|
||
|
||
**Raises:** `SkillExecutionError` with `SKILL_NOT_FOUND` if `name` is not registered.
|
||
|
||
**Behaviour**
|
||
|
||
| Condition | Outcome |
|
||
|-----------|---------|
|
||
| Resolution succeeds, tool registry present, all refs found | `refreshed = [name]` |
|
||
| Resolution succeeds, tool registry present, refs missing | `failed = {name: "<missing refs>"}` |
|
||
| Resolution succeeds, **no tool registry** | `refreshed = [name]`, warning logged |
|
||
| Resolution fails (cycle, missing include) | `failed = {name: "<error>"}` |
|
||
|
||
When no `ToolRegistry` is configured, a `WARNING`-level log message is emitted
|
||
with recovery instructions (pass a `ToolRegistry` to `SkillRegistry.__init__`).
|
||
The skill is still counted as refreshed so callers are not permanently blocked.
|
||
|
||
### `refresh_all() → SkillRefreshResult`
|
||
|
||
Iterates over all registered skills, calls `refresh(name)` for each, and returns
|
||
an aggregated `SkillRefreshResult`.
|
||
|
||
**Returns:** Aggregated `SkillRefreshResult` across all skills.
|
||
|
||
**Example**
|
||
|
||
```python
|
||
from cleveragents.skills.registry import SkillRegistry
|
||
from cleveragents.tool.registry import ToolRegistry
|
||
|
||
tool_reg = ToolRegistry()
|
||
# ... register tools ...
|
||
|
||
skill_reg = SkillRegistry(tool_registry=tool_reg)
|
||
# ... register skills ...
|
||
|
||
result = skill_reg.refresh_all()
|
||
print(result.to_summary()) # e.g. "refreshed: 5, failed: 0, skipped: 0"
|
||
```
|
||
|
||
---
|
||
|
||
## MCPRefreshHook
|
||
|
||
**Module:** `cleveragents.mcp.refresh_hook`
|
||
**Export:** `cleveragents.mcp.MCPRefreshHook`
|
||
|
||
Connects an `MCPToolAdapter` to a `SkillRegistry` via the adapter's notification
|
||
listener mechanism. On receiving `notifications/tools/list_changed`, it schedules
|
||
a debounced call to `skill_registry.refresh_all()`.
|
||
|
||
### Constructor
|
||
|
||
```python
|
||
MCPRefreshHook(
|
||
adapter: MCPToolAdapter,
|
||
skill_registry: SkillRegistry,
|
||
debounce_seconds: float = 0.5,
|
||
)
|
||
```
|
||
|
||
| Parameter | Type | Default | Description |
|
||
|-----------|------|---------|-------------|
|
||
| `adapter` | `MCPToolAdapter` | — | Adapter whose notifications trigger refresh |
|
||
| `skill_registry` | `SkillRegistry` | — | Registry to refresh on notification |
|
||
| `debounce_seconds` | `float` | `0.5` | Seconds to wait after the **last** notification before triggering refresh |
|
||
|
||
**Raises:** `ValueError` if `debounce_seconds < 0`.
|
||
|
||
### Debounce Behaviour
|
||
|
||
Rapid successive `notifications/tools/list_changed` events within the debounce
|
||
window are coalesced: each new notification resets the countdown timer, so only
|
||
one `refresh_all()` fires once the storm subsides.
|
||
|
||
```
|
||
notification → reset timer (0.5 s)
|
||
notification → reset timer (0.5 s) ← only this one fires
|
||
notification → reset timer (0.5 s)
|
||
0.5 s later → refresh_all() ×1
|
||
```
|
||
|
||
### Properties
|
||
|
||
| Property | Type | Description |
|
||
|----------|------|-------------|
|
||
| `refresh_count` | `int` | Number of times `refresh_all()` has been triggered (thread-safe) |
|
||
|
||
### Methods
|
||
|
||
#### `cancel() → None`
|
||
|
||
Cancels any pending debounced refresh timer. Safe to call multiple times and from
|
||
any thread. Does **not** un-register the notification listener from the adapter.
|
||
|
||
### Example
|
||
|
||
```python
|
||
from cleveragents.mcp import MCPRefreshHook
|
||
from cleveragents.mcp.adapter import MCPServerConfig, MCPToolAdapter
|
||
from cleveragents.skills.registry import SkillRegistry
|
||
|
||
adapter = MCPToolAdapter(config=MCPServerConfig(name="my-server", transport="stdio", command="my-mcp"))
|
||
skill_registry = SkillRegistry()
|
||
|
||
hook = MCPRefreshHook(
|
||
adapter=adapter,
|
||
skill_registry=skill_registry,
|
||
debounce_seconds=0.5,
|
||
)
|
||
|
||
# ... adapter receives notifications automatically during normal operation ...
|
||
|
||
# Clean up when done
|
||
hook.cancel()
|
||
```
|
||
|
||
### Thread Safety
|
||
|
||
`MCPRefreshHook` uses `threading.Lock` internally. The `_on_notification` callback,
|
||
`_do_refresh`, `refresh_count`, and `cancel` are all safe to call from multiple
|
||
threads concurrently.
|
||
|
||
---
|
||
|
||
## MCPToolAdapter Notification API
|
||
|
||
**Module:** `cleveragents.mcp.adapter`
|
||
|
||
`MCPToolAdapter` exposes a generic notification listener API used by `MCPRefreshHook`.
|
||
|
||
### `add_notification_listener(callback)`
|
||
|
||
Registers a callable to receive all notifications dispatched by this adapter.
|
||
|
||
```python
|
||
def callback(method: str, params: dict[str, Any]) -> None: ...
|
||
```
|
||
|
||
### `dispatch_notification(method, params=None)`
|
||
|
||
Dispatches a notification to all registered listeners. Used internally when the
|
||
underlying MCP server sends a notification, and available for testing.
|
||
|
||
---
|
||
|
||
## Notification Flow
|
||
|
||
```
|
||
MCP Server
|
||
│ notifications/tools/list_changed
|
||
▼
|
||
MCPToolAdapter.dispatch_notification()
|
||
│
|
||
▼ (fan-out to all listeners)
|
||
MCPRefreshHook._on_notification()
|
||
│ debounce_seconds timer
|
||
▼
|
||
SkillRegistry.refresh_all()
|
||
│ per-skill
|
||
▼
|
||
SkillRegistry.refresh(name)
|
||
│ validate via ToolRegistry
|
||
▼
|
||
SkillRefreshResult → logged at INFO
|
||
```
|
||
|
||
---
|
||
|
||
## Exports
|
||
|
||
### `cleveragents.skills`
|
||
|
||
```python
|
||
from cleveragents.skills import SkillRefreshResult
|
||
```
|
||
|
||
### `cleveragents.mcp`
|
||
|
||
```python
|
||
from cleveragents.mcp import MCPRefreshHook
|
||
```
|
||
|
||
---
|
||
|
||
## Error Reference
|
||
|
||
| Situation | Result field | Details |
|
||
|-----------|-------------|---------|
|
||
| Skill not registered | `SkillExecutionError` raised | `SKILL_NOT_FOUND` error type |
|
||
| Tool ref missing from registry | `failed[name]` | Lists missing ref names |
|
||
| Resolution cycle / missing include | `failed[name]` | Exception message |
|
||
| No `ToolRegistry` configured | `refreshed[name]` | `WARNING` log emitted |
|
||
| `debounce_seconds < 0` | `ValueError` raised | Constructor validation |
|