test(adapter_base): signature snapshot — drift gate for adapter public surface (#2364 item 2)
Every workspace template (langgraph, claude-code, hermes, etc.) subclasses BaseAdapter. Renaming, removing, or re-typing a method on the base class silently breaks templates: the override stops being recognized as an override; the old method-name's caller silently invokes the default no-op; the new method-name is unimplemented in templates that haven't migrated. Recent #87 universal-runtime + #1957 recordResource refactor both renamed/added methods. Without a frozen snapshot, the next rename ships quietly and surfaces only when a template's CI catches the AttributeError days later — long after the merge window for an easy revert. This snapshot pins BaseAdapter's public method surface against a checked-in JSON file. Same-shape pattern as PR #2363's A2A protocol-compat replay gate, applied to a Python public-API surface instead of JSON message shapes. Both close drift classes by snapshotting the structural surface that consumers depend on. Two tests: 1. test_base_adapter_signature_matches_snapshot — full introspection diff against tests/snapshots/adapter_base_signature.json. Drift = test failure with both expected + actual JSON in the message so the reviewer sees what changed. 2. test_snapshot_has_required_methods — defense-in-depth: even if both the source AND snapshot are updated together (intentional API removal), this catches removal of the short list of methods that EVERY template depends on (name, display_name, description, capabilities, memory_filename). Removing one of these requires explicit edit to the `required` set with a justification. Verified the gate fires red on a deliberate rename (memory_filename → memory_filename_RENAMED) — failure message shows the full snapshot diff including parameter shapes and return annotations. Updating the snapshot is the explicit acknowledgment that a template-affecting API change is intentional. Reviewer of the introducing PR sees the snapshot diff and decides whether template repos need coordinated updates. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
97058d5392
commit
8488a188c2
311
workspace/tests/snapshots/adapter_base_signature.json
Normal file
311
workspace/tests/snapshots/adapter_base_signature.json
Normal file
@ -0,0 +1,311 @@
|
||||
{
|
||||
"class": "BaseAdapter",
|
||||
"methods": [
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "append_to_memory_hook",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "AdapterConfig",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "config"
|
||||
},
|
||||
{
|
||||
"annotation": "str",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "filename"
|
||||
},
|
||||
{
|
||||
"annotation": "str",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "content"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "capabilities",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
}
|
||||
],
|
||||
"return_annotation": "RuntimeCapabilities"
|
||||
},
|
||||
{
|
||||
"is_abstract": true,
|
||||
"is_async": true,
|
||||
"name": "create_executor",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "AdapterConfig",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "config"
|
||||
}
|
||||
],
|
||||
"return_annotation": "AgentExecutor"
|
||||
},
|
||||
{
|
||||
"is_abstract": true,
|
||||
"is_async": false,
|
||||
"name": "description",
|
||||
"parameters": [],
|
||||
"return_annotation": "str"
|
||||
},
|
||||
{
|
||||
"is_abstract": true,
|
||||
"is_async": false,
|
||||
"name": "display_name",
|
||||
"parameters": [],
|
||||
"return_annotation": "str"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "get_config_schema",
|
||||
"parameters": [],
|
||||
"return_annotation": "dict"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "idle_timeout_override",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
}
|
||||
],
|
||||
"return_annotation": "int | None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": true,
|
||||
"name": "inject_plugins",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "AdapterConfig",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "config"
|
||||
},
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "plugins"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": true,
|
||||
"name": "install_plugins_via_registry",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "AdapterConfig",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "config"
|
||||
},
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "plugins"
|
||||
}
|
||||
],
|
||||
"return_annotation": "list"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "memory_filename",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
}
|
||||
],
|
||||
"return_annotation": "str"
|
||||
},
|
||||
{
|
||||
"is_abstract": true,
|
||||
"is_async": false,
|
||||
"name": "name",
|
||||
"parameters": [],
|
||||
"return_annotation": "str"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "pre_stop_state",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
}
|
||||
],
|
||||
"return_annotation": "dict"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "register_subagent_hook",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "str",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "name"
|
||||
},
|
||||
{
|
||||
"annotation": "dict",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "spec"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "register_tool_hook",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "str",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "name"
|
||||
},
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "fn"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": false,
|
||||
"name": "restore_state",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "dict",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "snapshot"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": true,
|
||||
"is_async": true,
|
||||
"name": "setup",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "AdapterConfig",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "config"
|
||||
}
|
||||
],
|
||||
"return_annotation": "None"
|
||||
},
|
||||
{
|
||||
"is_abstract": false,
|
||||
"is_async": true,
|
||||
"name": "transcript_lines",
|
||||
"parameters": [
|
||||
{
|
||||
"annotation": "",
|
||||
"has_default": false,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "self"
|
||||
},
|
||||
{
|
||||
"annotation": "int",
|
||||
"has_default": true,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "since"
|
||||
},
|
||||
{
|
||||
"annotation": "int",
|
||||
"has_default": true,
|
||||
"kind": "POSITIONAL_OR_KEYWORD",
|
||||
"name": "limit"
|
||||
}
|
||||
],
|
||||
"return_annotation": "dict"
|
||||
}
|
||||
]
|
||||
}
|
||||
166
workspace/tests/test_adapter_base_signature.py
Normal file
166
workspace/tests/test_adapter_base_signature.py
Normal file
@ -0,0 +1,166 @@
|
||||
"""BaseAdapter public-API signature snapshot — drift gate (#2364 item 2).
|
||||
|
||||
Every workspace template subclasses ``BaseAdapter``. Renaming, removing,
|
||||
or re-typing a method on the base class silently breaks templates that
|
||||
override it — the override stops being recognized as an override, the
|
||||
old method-name's caller silently invokes the default no-op, etc.
|
||||
Recent #87 universal-runtime work + #1957 recordResource refactor both
|
||||
renamed/added methods; without a frozen snapshot, the next rename ships
|
||||
quietly and only surfaces when a template's CI catches the AttributeError
|
||||
days later.
|
||||
|
||||
This test pins the public surface. It walks ``BaseAdapter`` with
|
||||
``inspect`` and compares the result against a checked-in JSON snapshot.
|
||||
Any drift = test failure.
|
||||
|
||||
When the failure is intentional:
|
||||
|
||||
1. Make the API change in ``adapter_base.py``.
|
||||
2. Run the test once to see the diff in the failure message.
|
||||
3. Update ``tests/snapshots/adapter_base_signature.json`` to match
|
||||
the new shape — that update IS the explicit acknowledgment that
|
||||
templates need follow-up. Reviewer of the PR sees the snapshot
|
||||
diff in their review and decides whether template repos need
|
||||
coordinated updates.
|
||||
|
||||
Same-shape pattern as PR #2363's A2A protocol-compat replay gate
|
||||
(workspace-server/internal/handlers/testdata/a2a_corpus). Both close
|
||||
drift classes by snapshotting the structural surface that templates
|
||||
or callers depend on.
|
||||
"""
|
||||
|
||||
import inspect
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Resolve workspace/ as the import root so adapter_base imports clean.
|
||||
WORKSPACE_DIR = Path(__file__).parent.parent
|
||||
if str(WORKSPACE_DIR) not in sys.path:
|
||||
sys.path.insert(0, str(WORKSPACE_DIR))
|
||||
|
||||
SNAPSHOT_PATH = Path(__file__).parent / "snapshots" / "adapter_base_signature.json"
|
||||
|
||||
|
||||
def _annotation_repr(annotation: object) -> str:
|
||||
"""Stable string form of a type annotation. ``inspect`` returns the
|
||||
runtime objects which don't compare cleanly — repr is the boring
|
||||
correct answer for snapshotting."""
|
||||
if annotation is inspect.Parameter.empty:
|
||||
return ""
|
||||
if isinstance(annotation, type):
|
||||
return annotation.__name__
|
||||
# types.UnionType / typing.Union / forward refs — repr captures all
|
||||
return str(annotation)
|
||||
|
||||
|
||||
def _parameter_record(p: inspect.Parameter) -> dict:
|
||||
return {
|
||||
"name": p.name,
|
||||
"kind": p.kind.name,
|
||||
"annotation": _annotation_repr(p.annotation),
|
||||
"has_default": p.default is not inspect.Parameter.empty,
|
||||
}
|
||||
|
||||
|
||||
def _signature_record(name: str, fn: object) -> dict:
|
||||
sig = inspect.signature(fn)
|
||||
return {
|
||||
"name": name,
|
||||
"is_async": inspect.iscoroutinefunction(fn),
|
||||
"is_abstract": getattr(fn, "__isabstractmethod__", False),
|
||||
"parameters": [_parameter_record(p) for p in sig.parameters.values()],
|
||||
"return_annotation": _annotation_repr(sig.return_annotation),
|
||||
}
|
||||
|
||||
|
||||
def _build_signature_snapshot() -> dict:
|
||||
"""Walk BaseAdapter and produce a stable JSON-serializable snapshot."""
|
||||
from adapter_base import BaseAdapter # imported lazy so test discovery is fast
|
||||
|
||||
methods: list[dict] = []
|
||||
for attr_name in sorted(vars(BaseAdapter)):
|
||||
if attr_name.startswith("_"):
|
||||
continue
|
||||
attr = vars(BaseAdapter)[attr_name]
|
||||
# Only callables — skip data attributes (none today, but
|
||||
# forward-defensive). staticmethod / classmethod are unwrapped
|
||||
# via __func__; abstractmethod wraps the underlying fn.
|
||||
if isinstance(attr, staticmethod):
|
||||
fn = attr.__func__
|
||||
elif isinstance(attr, classmethod):
|
||||
fn = attr.__func__
|
||||
elif callable(attr):
|
||||
fn = attr
|
||||
else:
|
||||
continue
|
||||
methods.append(_signature_record(attr_name, fn))
|
||||
return {"class": "BaseAdapter", "methods": methods}
|
||||
|
||||
|
||||
def test_base_adapter_signature_matches_snapshot():
|
||||
"""Pin BaseAdapter's public API surface against a frozen snapshot.
|
||||
|
||||
On failure, the test prints both the expected and actual snapshot
|
||||
JSON so the diff is human-readable. Updating the snapshot is the
|
||||
explicit ack that a template-affecting API change is intentional.
|
||||
"""
|
||||
actual = _build_signature_snapshot()
|
||||
if not SNAPSHOT_PATH.exists():
|
||||
# First-run convenience: write the snapshot if missing. A reviewer
|
||||
# of the introducing PR sees the new file in the diff.
|
||||
SNAPSHOT_PATH.parent.mkdir(parents=True, exist_ok=True)
|
||||
SNAPSHOT_PATH.write_text(json.dumps(actual, indent=2, sort_keys=True) + "\n")
|
||||
pytest.skip(
|
||||
f"snapshot did not exist; wrote {SNAPSHOT_PATH.name} — "
|
||||
"re-run the test to verify it now passes"
|
||||
)
|
||||
|
||||
expected = json.loads(SNAPSHOT_PATH.read_text())
|
||||
if actual != expected:
|
||||
# Pretty-print both for the failure message so reviewer sees what
|
||||
# changed without rerunning anything.
|
||||
actual_str = json.dumps(actual, indent=2, sort_keys=True)
|
||||
expected_str = json.dumps(expected, indent=2, sort_keys=True)
|
||||
pytest.fail(
|
||||
"BaseAdapter signature drifted from snapshot.\n\n"
|
||||
f"To update intentionally:\n cp <(python -c 'from tests.test_adapter_base_signature import _build_signature_snapshot; import json; print(json.dumps(_build_signature_snapshot(), indent=2, sort_keys=True))') {SNAPSHOT_PATH}\n"
|
||||
"Or rerun with the snapshot deleted to regenerate.\n\n"
|
||||
f"=== EXPECTED ({SNAPSHOT_PATH.name}) ===\n{expected_str}\n\n"
|
||||
f"=== ACTUAL (current adapter_base.py) ===\n{actual_str}\n"
|
||||
)
|
||||
|
||||
|
||||
def test_snapshot_has_required_methods():
|
||||
"""Defense-in-depth: the snapshot must include the methods every
|
||||
template overrides. If a future refactor accidentally drops one of
|
||||
these from BaseAdapter (e.g., moves it to a mixin), the equality
|
||||
test above passes if the snapshot file is also updated — but THIS
|
||||
test catches the structural regression.
|
||||
|
||||
Add a method to ``required`` ONLY when removing it would break a
|
||||
deployed template. The list is intentionally short.
|
||||
"""
|
||||
if not SNAPSHOT_PATH.exists():
|
||||
pytest.skip(f"{SNAPSHOT_PATH.name} not generated yet")
|
||||
|
||||
snapshot = json.loads(SNAPSHOT_PATH.read_text())
|
||||
method_names = {m["name"] for m in snapshot["methods"]}
|
||||
|
||||
required = {
|
||||
"name", # runtime identifier — every template MUST implement
|
||||
"display_name", # UI-facing label
|
||||
"description", # short description
|
||||
"capabilities", # native vs platform-fallback declaration (#117)
|
||||
"memory_filename", # plugin-pipeline hook
|
||||
}
|
||||
missing = required - method_names
|
||||
if missing:
|
||||
pytest.fail(
|
||||
f"BaseAdapter snapshot is missing required methods: {sorted(missing)}.\n"
|
||||
"Either restore them on adapter_base.py, OR coordinate template "
|
||||
"updates AND remove the entry from `required` in this test with "
|
||||
"a justification."
|
||||
)
|
||||
Loading…
Reference in New Issue
Block a user