test(plugins): enforce behavior compatibility contract
This commit is contained in:
25
AGENTS.md
25
AGENTS.md
@@ -797,6 +797,31 @@ as a side effect of importing `model_tools.py`. Code paths that read plugin
|
||||
state without importing `model_tools.py` first must call `discover_plugins()`
|
||||
explicitly (it's idempotent).
|
||||
|
||||
#### Native plugin compatibility policy
|
||||
|
||||
The canonical contract and deprecation policy live in
|
||||
`website/docs/developer-guide/plugins/index.md#native-plugin-compatibility-contract`.
|
||||
Compatibility is enforced as a behavior contract, not through a monolithic
|
||||
`PLUGIN_API_VERSION`, a manifest-wide native `api:` match, or version literals
|
||||
on unrelated payloads. Keep documented plugin surfaces additive:
|
||||
|
||||
- add hook payload data as keyword fields; signature-inspect callbacks so old
|
||||
narrow signatures receive only fields they declare, while `**kwargs`
|
||||
callbacks receive the complete payload;
|
||||
- do not remove or rename `PluginContext` methods; make new parameters optional
|
||||
with defaults and keyword-only where possible;
|
||||
- ignore unknown native manifest fields;
|
||||
- give new provider methods default implementations, and signature-inspect
|
||||
optional callback kwargs rather than forwarding them unconditionally;
|
||||
- use a local schema version only for a capability with a wire or persisted
|
||||
contract, and preserve old state/config/session replay or ship a migration.
|
||||
|
||||
Deprecations require a once-per-process warning, a documented replacement and
|
||||
migration note, and at least two subsequent minor releases before removal.
|
||||
Compatibility tests must load frozen plugins through the real discovery path
|
||||
and assert outcomes. Do not replace these with exact registry/catalog counts,
|
||||
source-reading tests, or assertions that a global version literal changed.
|
||||
|
||||
### Memory-provider plugins (`plugins/memory/<name>/`)
|
||||
|
||||
Separate discovery system for pluggable memory backends. Current built-in
|
||||
|
||||
@@ -2162,11 +2162,42 @@ class PluginManager:
|
||||
# Hook invocation
|
||||
# -----------------------------------------------------------------------
|
||||
|
||||
@staticmethod
|
||||
def _invoke_hook_callback(callback: Callable, payload: Dict[str, Any]) -> Any:
|
||||
"""Invoke a hook while withholding additive fields from old callbacks."""
|
||||
try:
|
||||
parameters = inspect.signature(callback).parameters
|
||||
except (TypeError, ValueError):
|
||||
# Some extension/builtin callables do not expose a signature. Keep
|
||||
# the historical behavior for those callables rather than guessing.
|
||||
return callback(**payload)
|
||||
|
||||
if any(
|
||||
parameter.kind == inspect.Parameter.VAR_KEYWORD
|
||||
for parameter in parameters.values()
|
||||
):
|
||||
return callback(**payload)
|
||||
|
||||
accepted_payload = {
|
||||
name: value
|
||||
for name, value in payload.items()
|
||||
if name in parameters
|
||||
and parameters[name].kind
|
||||
in {
|
||||
inspect.Parameter.POSITIONAL_OR_KEYWORD,
|
||||
inspect.Parameter.KEYWORD_ONLY,
|
||||
}
|
||||
}
|
||||
return callback(**accepted_payload)
|
||||
|
||||
def invoke_hook(self, hook_name: str, **kwargs: Any) -> List[Any]:
|
||||
"""Call all registered callbacks for *hook_name*.
|
||||
|
||||
Each callback is wrapped in its own try/except so a misbehaving
|
||||
plugin cannot break the core agent loop.
|
||||
Hook payloads evolve additively. Callbacks that accept ``**kwargs``
|
||||
receive the complete payload; older callbacks with a narrow signature
|
||||
receive only the keyword arguments they declare. Each callback is
|
||||
wrapped in its own try/except so a misbehaving plugin cannot break the
|
||||
core agent loop.
|
||||
|
||||
Returns a list of non-``None`` return values from callbacks.
|
||||
|
||||
@@ -2187,7 +2218,7 @@ class PluginManager:
|
||||
results: List[Any] = []
|
||||
for cb in callbacks:
|
||||
try:
|
||||
ret = cb(**kwargs)
|
||||
ret = self._invoke_hook_callback(cb, kwargs)
|
||||
if ret is not None:
|
||||
results.append(ret)
|
||||
except Exception as exc:
|
||||
|
||||
@@ -216,6 +216,26 @@ class TestMemoryManager:
|
||||
# p1 failed but p2 still synced
|
||||
assert p2.synced_turns == [("user", "assistant")]
|
||||
|
||||
def test_sync_all_keeps_legacy_provider_working_when_messages_are_available(self):
|
||||
"""New optional turn context is not sent to an old provider signature."""
|
||||
mgr = MemoryManager()
|
||||
legacy_provider = FakeMemoryProvider("legacy")
|
||||
mgr.add_provider(legacy_provider)
|
||||
completed_messages = [
|
||||
{"role": "user", "content": "user"},
|
||||
{"role": "assistant", "content": "assistant"},
|
||||
]
|
||||
|
||||
mgr.sync_all(
|
||||
"user",
|
||||
"assistant",
|
||||
session_id="legacy-session",
|
||||
messages=completed_messages,
|
||||
)
|
||||
mgr.flush_pending(timeout=5)
|
||||
|
||||
assert legacy_provider.synced_turns == [("user", "assistant")]
|
||||
|
||||
# -- Tool routing -------------------------------------------------------
|
||||
|
||||
|
||||
|
||||
16
tests/hermes_cli/fixtures/plugin_compat_legacy/__init__.py
Normal file
16
tests/hermes_cli/fixtures/plugin_compat_legacy/__init__.py
Normal file
@@ -0,0 +1,16 @@
|
||||
"""Frozen legacy plugin used by the behavior compatibility suite.
|
||||
|
||||
Keep this callback in its old, narrow form: it intentionally predates additive
|
||||
hook payload fields and does not accept ``**kwargs``.
|
||||
"""
|
||||
|
||||
received_sessions = []
|
||||
|
||||
|
||||
def _on_session_start(*, session_id):
|
||||
received_sessions.append(session_id)
|
||||
return {"legacy_session_id": session_id}
|
||||
|
||||
|
||||
def register(ctx):
|
||||
ctx.register_hook("on_session_start", _on_session_start)
|
||||
@@ -0,0 +1,8 @@
|
||||
name: legacy-contract-fixture
|
||||
version: "0.1.0"
|
||||
description: Frozen plugin fixture for native behavior compatibility
|
||||
provides_hooks:
|
||||
- on_session_start
|
||||
future_manifest_field:
|
||||
schema: 2
|
||||
additive: true
|
||||
52
tests/hermes_cli/test_plugin_api_compat.py
Normal file
52
tests/hermes_cli/test_plugin_api_compat.py
Normal file
@@ -0,0 +1,52 @@
|
||||
"""Behavior-contract compatibility tests for native Hermes plugins."""
|
||||
|
||||
from pathlib import Path
|
||||
import shutil
|
||||
|
||||
import yaml
|
||||
|
||||
from hermes_cli.plugins import PluginManager
|
||||
|
||||
|
||||
LEGACY_PLUGIN = Path(__file__).parent / "fixtures" / "plugin_compat_legacy"
|
||||
|
||||
|
||||
def test_legacy_plugin_loads_and_ignores_additive_hook_and_manifest_fields(
|
||||
tmp_path, monkeypatch
|
||||
):
|
||||
"""A frozen plugin keeps working as manifests and hook payloads grow."""
|
||||
hermes_home = tmp_path / "hermes-home"
|
||||
plugins_dir = hermes_home / "plugins"
|
||||
plugins_dir.mkdir(parents=True)
|
||||
shutil.copytree(LEGACY_PLUGIN, plugins_dir / "legacy-contract-fixture")
|
||||
(hermes_home / "config.yaml").write_text(
|
||||
yaml.safe_dump(
|
||||
{"plugins": {"enabled": ["legacy-contract-fixture"]}}
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
empty_bundled = tmp_path / "bundled-plugins"
|
||||
empty_bundled.mkdir()
|
||||
monkeypatch.setenv("HOME", str(tmp_path / "os-home"))
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setenv("HERMES_BUNDLED_PLUGINS", str(empty_bundled))
|
||||
|
||||
manager = PluginManager()
|
||||
manager.discover_and_load()
|
||||
|
||||
loaded = manager._plugins["legacy-contract-fixture"]
|
||||
assert loaded.enabled is True
|
||||
assert loaded.error is None
|
||||
assert loaded.module is not None
|
||||
assert loaded.manifest.version == "0.1.0"
|
||||
assert "on_session_start" in loaded.hooks_registered
|
||||
|
||||
results = manager.invoke_hook(
|
||||
"on_session_start",
|
||||
session_id="legacy-session",
|
||||
resumed=True,
|
||||
future_additive_field={"nested": "value"},
|
||||
)
|
||||
|
||||
assert results == [{"legacy_session_id": "legacy-session"}]
|
||||
assert loaded.module.received_sessions == ["legacy-session"]
|
||||
@@ -103,6 +103,65 @@ records it as Published. Hermes keys behavior on the canonical v1.0.0 schema
|
||||
identifiers and normative text, not either mutable status label. This is an
|
||||
explicit supported subset, not a claim of full Agent Plugins conformance.
|
||||
|
||||
## Native plugin compatibility contract
|
||||
|
||||
Native `plugin.yaml` plus `register(ctx)` plugins are protected by behavior,
|
||||
not by one global plugin API number. Hermes does not expose a
|
||||
`PLUGIN_API_VERSION`, require a manifest-wide `api:` match, or attach an API
|
||||
version to unrelated values. A plugin that uses a documented behavior should
|
||||
continue to work after a normal Hermes upgrade.
|
||||
|
||||
The compatibility rules are:
|
||||
|
||||
- **Evolve additively.** Documented `PluginContext` methods are not removed or
|
||||
renamed. New parameters are optional, have defaults, and should be
|
||||
keyword-only. Existing return fields are not removed or silently retyped.
|
||||
- **Hook payloads are keyword payloads.** New hook data is added as keyword
|
||||
fields, never by changing the meaning or position of an existing field.
|
||||
Hermes inspects callback signatures: a legacy callback receives the fields it
|
||||
declares, while a callback with `**kwargs` receives the complete current
|
||||
payload. New plugins should accept `**kwargs` so they can opt into additive
|
||||
data without another signature change.
|
||||
- **Manifests are open to additions.** Unknown `plugin.yaml` fields are ignored.
|
||||
Older Hermes releases can therefore load a plugin whose manifest contains
|
||||
metadata introduced by a newer release, provided the plugin code itself uses
|
||||
supported runtime behavior.
|
||||
- **Provider interfaces grow through defaults.** New provider methods have a
|
||||
default implementation. New callback context is optional and forwarded only
|
||||
when signature inspection shows that a provider accepts it. Adding an
|
||||
abstract method or an unconditionally forwarded argument requires a
|
||||
migration window rather than a flag-day signature change.
|
||||
- **Version the contract that crosses a boundary.** A capability may carry its
|
||||
own schema version when it defines a wire payload or persisted format (for
|
||||
example, observer payloads or secret-source state). Keep fields additive
|
||||
within that local schema. Persisted plugin state and config must remain
|
||||
readable, or ship an explicit migration; resumed sessions written by the old
|
||||
format must still replay. Do not add version literals to unrelated callback
|
||||
or context values.
|
||||
|
||||
### Deprecation policy
|
||||
|
||||
A documented native plugin behavior may be deprecated only with all of the
|
||||
following:
|
||||
|
||||
1. a replacement and migration instructions in the plugin guide and release
|
||||
notes;
|
||||
2. a warning emitted at most once per process, naming the replacement and the
|
||||
earliest removal release;
|
||||
3. support for the old behavior through at least two subsequent minor
|
||||
releases; and
|
||||
4. behavior-based compatibility coverage for both the legacy path and the
|
||||
replacement throughout that window.
|
||||
|
||||
Removal after the window must include any migration needed for persisted data
|
||||
or resumable sessions. In practice, additive aliases and adapters are preferred
|
||||
to removal.
|
||||
|
||||
Hermes enforces this contract with frozen external-plugin fixtures discovered
|
||||
from an isolated `HERMES_HOME`. Those tests load and invoke the plugin through
|
||||
`PluginManager`; they assert real registration and callback outcomes rather
|
||||
than internal symbol lists or source-code shape.
|
||||
|
||||
## What you're building
|
||||
|
||||
A **calculator** plugin with two tools:
|
||||
|
||||
Reference in New Issue
Block a user