fix(plugins): trim request-middleware chaining to the minimal shape
Keep the contributors' fix (each llm_request / tool_request callback sees the previous accepted rewrite; each gets its own copy of the payload and of the original_* snapshot) in PluginManager.invoke_middleware, which stays the single plugin-failure boundary, and drop the rest: - the ~90-line iterative container walker in _safe_copy. It only runs when deepcopy fails (an opaque leaf in the request); main's shallow-copy fallback stays. - the original_* freeze/copy in _run_execution_chain. Execution middleware already composes via next_call; that was defensive isolation, not chaining. - the second copy of an accepted return inside invoke_middleware (_apply_request_chain already copies it). - tests/hermes_cli/test_middleware_isolation.py and the test_plugins.py additions; tests/hermes_cli/test_plugins_middleware_chain.py holds two invariant tests (composition, in-place-mutation isolation), each over both request kinds. test_plugins.py is over its size cap, so it gains nothing.
This commit is contained in:
Teknium
@@ -7,7 +7,7 @@ wrapping the actual execution callback. Agent-loop call sites and plugins share
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from copy import copy, deepcopy
|
||||
from copy import deepcopy
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any, Callable, Dict, List
|
||||
|
||||
@@ -48,107 +48,16 @@ def middleware_payload(**kwargs: Any) -> Dict[str, Any]:
|
||||
|
||||
|
||||
def _safe_copy(payload: Any) -> Any:
|
||||
"""Copy a request graph, tolerating non-deepcopyable members.
|
||||
"""Deep-copy a request payload, tolerating non-deepcopyable members.
|
||||
|
||||
An LLM request can carry clients/callbacks/file handles; a hard ``deepcopy`` failure would
|
||||
otherwise abort the whole request-middleware pass. The fallback isolates surrounding
|
||||
containers and preserves their aliases and cycles; opaque leaves retain their identity.
|
||||
otherwise abort the whole request-middleware pass.
|
||||
"""
|
||||
try:
|
||||
return deepcopy(payload)
|
||||
except Exception as exc:
|
||||
logger.debug("deepcopy failed for request payload (%s); retaining opaque leaves", exc)
|
||||
|
||||
# Walk containers without Python recursion: an opaque leaf or a deeply nested
|
||||
# request must not share surrounding containers or bypass the middleware pass. Shallow
|
||||
# copies retain container subclasses (including defaultdict's factory).
|
||||
memo: Dict[int, Any] = {}
|
||||
pending: Dict[int, List[Callable[[Any], None]]] = {}
|
||||
tasks: List[Callable[[], None]] = []
|
||||
root: List[Any] = []
|
||||
|
||||
def copy_state(value: Any, result: Any) -> None:
|
||||
state = getattr(value, "__dict__", None)
|
||||
if state is not None and hasattr(result, "__dict__"):
|
||||
tasks.append(lambda: copy_value(state, lambda copied: setattr(result, "__dict__", copied)))
|
||||
|
||||
def copy_value(value: Any, assign: Callable[[Any], None]) -> None:
|
||||
if type(value) in (str, bytes, int, float, bool, complex, type(None)):
|
||||
assign(value)
|
||||
return
|
||||
identity = id(value)
|
||||
if identity in memo:
|
||||
assign(memo[identity])
|
||||
return
|
||||
if identity in pending:
|
||||
pending[identity].append(assign)
|
||||
return
|
||||
result: Any
|
||||
if isinstance(value, (dict, list, set)):
|
||||
factory = dict if isinstance(value, dict) else list if isinstance(value, list) else set
|
||||
try:
|
||||
result = copy(value)
|
||||
if result is value or not isinstance(result, factory):
|
||||
result = factory()
|
||||
result.clear()
|
||||
except Exception:
|
||||
result = factory()
|
||||
memo[identity] = result
|
||||
assign(result)
|
||||
copy_state(value, result)
|
||||
if isinstance(result, dict):
|
||||
for key, item in reversed(list(value.items())):
|
||||
pair: List[Any] = [None, None]
|
||||
tasks.append(lambda pair=pair, target=result: target.__setitem__(pair[0], pair[1]))
|
||||
tasks.append(lambda item=item, pair=pair: copy_value(
|
||||
item, lambda copied: pair.__setitem__(1, copied)))
|
||||
tasks.append(lambda key=key, pair=pair: copy_value(
|
||||
key, lambda copied: pair.__setitem__(0, copied)))
|
||||
elif isinstance(result, list):
|
||||
list.extend(result, [None] * len(value))
|
||||
for index, item in reversed(list(enumerate(value))):
|
||||
tasks.append(lambda index=index, item=item, target=result: copy_value(
|
||||
item, lambda copied: list.__setitem__(target, index, copied)))
|
||||
else:
|
||||
for item in value:
|
||||
tasks.append(lambda item=item, target=result: copy_value(
|
||||
item, lambda copied: set.add(target, copied)))
|
||||
return
|
||||
if isinstance(value, (tuple, frozenset)):
|
||||
pending[identity] = [assign]
|
||||
items: List[Any] = [None] * len(value)
|
||||
|
||||
def finish() -> None:
|
||||
factory = tuple if isinstance(value, tuple) else frozenset
|
||||
result = factory.__new__(type(value), items)
|
||||
memo[identity] = result
|
||||
for setter in pending.pop(identity):
|
||||
setter(result)
|
||||
copy_state(value, result)
|
||||
|
||||
# Mutable children are allocated before their contents, so tuple/list
|
||||
# cycles can be completed after the tuple itself becomes available.
|
||||
tasks.append(finish)
|
||||
for index, item in reversed(list(enumerate(value))):
|
||||
tasks.append(lambda index=index, item=item: copy_value(
|
||||
item, lambda copied: items.__setitem__(index, copied)))
|
||||
return
|
||||
# A failing __deepcopy__ may leave partial containers in its memo.
|
||||
# Publish that memo only on success, never after an opaque leaf fails.
|
||||
leaf_memo = memo.copy()
|
||||
try:
|
||||
result = deepcopy(value, leaf_memo)
|
||||
except Exception:
|
||||
result = value
|
||||
else:
|
||||
memo.update(leaf_memo)
|
||||
memo[identity] = result
|
||||
assign(result)
|
||||
|
||||
tasks.append(lambda: copy_value(payload, root.append))
|
||||
while tasks:
|
||||
tasks.pop()()
|
||||
return root[0]
|
||||
except Exception as exc: # pragma: no cover - exercised via fallback test
|
||||
logger.debug("deepcopy failed for request payload (%s); using shallow copy", exc)
|
||||
return dict(payload) if isinstance(payload, dict) else payload
|
||||
|
||||
|
||||
def _apply_request_chain(
|
||||
@@ -258,11 +167,6 @@ def _run_execution_chain(kind: str, terminal_call: Callable[[Any], Any], **kwarg
|
||||
if not callbacks:
|
||||
return terminal_call(kwargs[payload_key])
|
||||
|
||||
# Execution-only plugins may rewrite the caller's payload in place, so freeze
|
||||
# its original before entering the first frame, then isolate each frame's view.
|
||||
original_key = "original_" + payload_key
|
||||
kwargs[original_key] = _safe_copy(kwargs[original_key])
|
||||
|
||||
def call_at(index: int, payload: Any) -> Any:
|
||||
if index >= len(callbacks):
|
||||
return terminal_call(payload)
|
||||
@@ -292,7 +196,6 @@ def _run_execution_chain(kind: str, terminal_call: Callable[[Any], Any], **kwarg
|
||||
|
||||
call_kwargs = middleware_payload(**kwargs)
|
||||
call_kwargs[payload_key] = payload
|
||||
call_kwargs[original_key] = _safe_copy(kwargs[original_key])
|
||||
call_kwargs["next_call"] = next_call
|
||||
try:
|
||||
return callback(**call_kwargs)
|
||||
|
||||
@@ -591,31 +591,28 @@ class PluginDispatchMixin:
|
||||
def invoke_middleware(
|
||||
self, kind: str, *, _payload_key: Optional[str] = None, **kwargs: Any
|
||||
) -> List[Any]:
|
||||
"""Dispatch middleware; request rewrites compose only after a valid return.
|
||||
"""Call middleware callbacks for *kind* (each isolated); return non-``None`` results.
|
||||
|
||||
Request callbacks receive a copy of the last accepted payload, so a
|
||||
failing/observer callback cannot mutate an earlier accepted rewrite.
|
||||
Each callback also receives its own pre-middleware snapshot.
|
||||
Request middleware passes ``_payload_key``: a dict returned under it becomes the payload the
|
||||
next callback sees, so rewrites compose, and each callback gets its own copy of the payload and
|
||||
of ``original_<key>`` so an in-place edit cannot leak.
|
||||
"""
|
||||
from hermes_cli.middleware import _safe_copy
|
||||
|
||||
copy_keys = {"original_request", "original_args"}
|
||||
if _payload_key is not None:
|
||||
copy_keys.add(_payload_key)
|
||||
results: List[Any] = []
|
||||
for cb in self._middleware.get(kind, []):
|
||||
call_kwargs = kwargs
|
||||
if _payload_key:
|
||||
original_key = "original_" + _payload_key
|
||||
call_kwargs = {**kwargs, _payload_key: _safe_copy(kwargs[_payload_key]),
|
||||
original_key: _safe_copy(kwargs[original_key])}
|
||||
try:
|
||||
callback_kwargs = {
|
||||
key: _safe_copy(value) if key in copy_keys else value
|
||||
for key, value in kwargs.items()
|
||||
}
|
||||
ret = cb(**callback_kwargs)
|
||||
ret = cb(**call_kwargs)
|
||||
if ret is not None:
|
||||
if (_payload_key is not None and isinstance(ret, dict)
|
||||
and isinstance(ret.get(_payload_key), dict)):
|
||||
ret = {**ret, _payload_key: _safe_copy(ret[_payload_key])}
|
||||
kwargs[_payload_key] = ret[_payload_key]
|
||||
results.append(ret)
|
||||
if _payload_key and isinstance(ret, dict) and isinstance(ret.get(_payload_key), dict):
|
||||
kwargs[_payload_key] = ret[_payload_key]
|
||||
except (Exception, SystemExit) as exc:
|
||||
# Runs once per tool call like a hook, so a mis-declared callback floods identically.
|
||||
self._report_hook_failure(kind, cb, kwargs, exc, surface="Middleware")
|
||||
return results
|
||||
|
||||
@@ -1,264 +0,0 @@
|
||||
"""Request rewrites and original snapshots stay isolated across discovered middleware."""
|
||||
|
||||
from collections import OrderedDict, defaultdict
|
||||
from typing import Any, NamedTuple, cast
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import plugins
|
||||
from hermes_cli.config import atomic_config_write
|
||||
from hermes_cli.middleware import (
|
||||
apply_llm_request_middleware,
|
||||
apply_tool_request_middleware,
|
||||
run_llm_execution_middleware,
|
||||
run_tool_execution_middleware,
|
||||
)
|
||||
|
||||
|
||||
class RequestDict(dict):
|
||||
pass
|
||||
|
||||
|
||||
class RequestList(list):
|
||||
pass
|
||||
|
||||
|
||||
class RequestTuple(NamedTuple):
|
||||
data: Any
|
||||
file: Any
|
||||
|
||||
|
||||
class OpaqueMember:
|
||||
def __init__(self, related):
|
||||
self.related = related
|
||||
|
||||
def __deepcopy__(self, memo):
|
||||
memo[id(self.related)] = {"partial": True}
|
||||
raise TypeError("opaque member cannot be copied")
|
||||
|
||||
|
||||
def _load_plugin(tmp_path, monkeypatch, source):
|
||||
home = tmp_path / "home"
|
||||
plugin = home / "plugins" / "copy_probe"
|
||||
plugin.mkdir(parents=True)
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
plugin.joinpath("plugin.yaml").write_text(
|
||||
"name: copy_probe\nversion: 0.1.0\ndescription: Middleware isolation probe\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
plugin.joinpath("__init__.py").write_text(source, encoding="utf-8")
|
||||
atomic_config_write(home / "config.yaml", {"plugins": {"enabled": ["copy_probe"]}})
|
||||
manager = plugins.PluginManager()
|
||||
manager.discover_and_load()
|
||||
monkeypatch.setattr(plugins, "get_plugin_manager", lambda: manager)
|
||||
assert any(manager.has_middleware(kind) for kind in (
|
||||
"llm_request", "tool_request", "llm_execution", "tool_execution",
|
||||
))
|
||||
|
||||
|
||||
def _apply(kind, payload):
|
||||
if kind == "llm":
|
||||
return apply_llm_request_middleware(payload)
|
||||
return apply_tool_request_middleware("probe", payload)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind", ["llm", "tool"])
|
||||
@pytest.mark.parametrize("stage", ["request", "execution", "execution_only"])
|
||||
@pytest.mark.parametrize("outcome", ["observe", "raise", "rewrite"])
|
||||
@pytest.mark.parametrize("opaque", [False, True])
|
||||
def test_original_snapshot_survives_each_middleware_callback(
|
||||
tmp_path, monkeypatch, kind, stage, outcome, opaque,
|
||||
):
|
||||
key = "request" if kind == "llm" else "args"
|
||||
request_listener = stage != "execution_only"
|
||||
if not request_listener:
|
||||
stage = "execution"
|
||||
_load_plugin(tmp_path, monkeypatch, f"""
|
||||
KIND = {kind!r}
|
||||
KEY = {key!r}
|
||||
STAGE = {stage!r}
|
||||
OUTCOME = {outcome!r}
|
||||
REQUEST_LISTENER = {request_listener!r}
|
||||
|
||||
def mutate(**kw):
|
||||
original = kw['original_' + KEY]
|
||||
original['input'].append('snapshot-write')
|
||||
original['injected'] = True
|
||||
if not REQUEST_LISTENER:
|
||||
kw[KEY]['input'].append('execution-write')
|
||||
if OUTCOME == 'raise':
|
||||
raise RuntimeError('snapshot probe failure')
|
||||
if OUTCOME == 'rewrite':
|
||||
payload = {{**kw[KEY], 'rewritten': True}}
|
||||
if STAGE == 'request':
|
||||
return {{KEY: payload, 'source': 'mutate'}}
|
||||
return kw['next_call'](payload)
|
||||
if STAGE == 'execution':
|
||||
return kw['next_call']()
|
||||
|
||||
def verify(**kw):
|
||||
original = kw['original_' + KEY]
|
||||
stage = 'execution' if 'next_call' in kw else 'request'
|
||||
payload = {{**kw[KEY], stage + '_snapshot': list(original['input'])}}
|
||||
payload[stage + '_injected'] = 'injected' in original
|
||||
if 'next_call' in kw:
|
||||
return kw['next_call'](payload)
|
||||
return {{KEY: payload, 'source': 'verify'}}
|
||||
|
||||
def register(ctx):
|
||||
ctx.register_middleware(KIND + '_' + STAGE, mutate)
|
||||
if REQUEST_LISTENER:
|
||||
ctx.register_middleware(KIND + '_request', verify)
|
||||
ctx.register_middleware(KIND + '_execution', verify)
|
||||
""")
|
||||
with (tmp_path / "upload.txt").open("w+", encoding="utf-8") as handle:
|
||||
original = {"input": ["original"], **({"file": handle} if opaque else {})}
|
||||
result = _apply(kind, original) if request_listener else None
|
||||
payload = result.payload if result else original
|
||||
context = {"original_" + key: result.original_payload} if result else {}
|
||||
calls = []
|
||||
|
||||
def terminal(payload):
|
||||
calls.append(payload)
|
||||
return payload
|
||||
|
||||
if kind == "llm":
|
||||
executed = run_llm_execution_middleware(
|
||||
payload, terminal, **context,
|
||||
)
|
||||
else:
|
||||
executed = run_tool_execution_middleware(
|
||||
"probe", payload, terminal, **context,
|
||||
)
|
||||
|
||||
expected_input = ["original"] if request_listener else ["original", "execution-write"]
|
||||
assert original["input"] == expected_input
|
||||
assert "injected" not in original
|
||||
if result:
|
||||
assert result.original_payload["input"] == ["original"]
|
||||
assert "injected" not in result.original_payload
|
||||
for phase in ("request", "execution") if request_listener else ("execution",):
|
||||
assert executed[phase + "_snapshot"] == ["original"]
|
||||
assert executed[phase + "_injected"] is False
|
||||
assert executed["input"] == expected_input
|
||||
assert bool(executed.get("rewritten")) == (outcome == "rewrite")
|
||||
assert calls == [executed]
|
||||
expected_sources = ["verify"]
|
||||
if stage == "request" and outcome == "rewrite":
|
||||
expected_sources.insert(0, "mutate")
|
||||
if result:
|
||||
assert [entry["source"] for entry in result.trace] == expected_sources
|
||||
if opaque:
|
||||
assert executed["file"] is original["file"] is handle
|
||||
if result:
|
||||
assert result.original_payload["file"] is handle
|
||||
assert not handle.closed
|
||||
|
||||
|
||||
def _leaf(node):
|
||||
while isinstance(node, dict) and "child" in node:
|
||||
node = node["child"]
|
||||
while isinstance(node, (list, tuple)):
|
||||
node = node[0]
|
||||
return node
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind", ["llm", "tool"])
|
||||
@pytest.mark.parametrize("container", [
|
||||
dict, OrderedDict, defaultdict, RequestDict, list, RequestList, tuple, RequestTuple,
|
||||
])
|
||||
@pytest.mark.parametrize("depth,wrapper", [
|
||||
(0, dict), (1500, dict), (6000, dict), (6000, list), (6000, tuple),
|
||||
])
|
||||
def test_opaque_request_graph_preserves_isolation_aliases_and_cycles(
|
||||
tmp_path, monkeypatch, kind, container, depth, wrapper,
|
||||
):
|
||||
key = "request" if kind == "llm" else "args"
|
||||
_load_plugin(tmp_path, monkeypatch, f"""
|
||||
KIND = {kind!r}
|
||||
KEY = {key!r}
|
||||
|
||||
def leaf(node):
|
||||
while isinstance(node, dict) and 'child' in node:
|
||||
node = node['child']
|
||||
while isinstance(node, (list, tuple)):
|
||||
node = node[0]
|
||||
return node
|
||||
|
||||
def first(**kw):
|
||||
return {{KEY: {{**kw[KEY], 'accepted': True}}, 'source': 'first'}}
|
||||
|
||||
def broken(**kw):
|
||||
leaf(kw[KEY]['node'])['steps'].append('broken')
|
||||
raise RuntimeError('graph probe failure')
|
||||
|
||||
def observer(**kw):
|
||||
leaf(kw[KEY]['node'])['steps'].append('observer')
|
||||
|
||||
def second(**kw):
|
||||
leaf(kw[KEY]['node'])['steps'].append('second')
|
||||
return {{KEY: kw[KEY], 'source': 'second'}}
|
||||
|
||||
def register(ctx):
|
||||
for callback in (first, broken, observer, second):
|
||||
ctx.register_middleware(KIND + '_request', callback)
|
||||
""")
|
||||
with (tmp_path / "upload.txt").open("w+", encoding="utf-8") as handle:
|
||||
leaf = {"steps": ["original"], "file": handle}
|
||||
if issubclass(container, dict):
|
||||
node = container(list, leaf) if container is defaultdict else container(leaf)
|
||||
elif container is RequestTuple:
|
||||
node = container(leaf, handle)
|
||||
else:
|
||||
node = container([leaf, handle])
|
||||
if container in (RequestDict, RequestList):
|
||||
node.metadata = ["original"]
|
||||
for _ in range(depth):
|
||||
node = {"child": node} if wrapper is dict else wrapper([node])
|
||||
cycle = []
|
||||
pair = (cycle, handle)
|
||||
cycle.append(pair)
|
||||
opaque = OpaqueMember(node)
|
||||
original: dict[str, Any] = {
|
||||
"opaque": opaque, "node": node, "alias": node, "cycle": pair,
|
||||
"cycle_alias": pair, "tags": {"original"},
|
||||
"frozen": frozenset({RequestTuple("original", handle)}),
|
||||
}
|
||||
try:
|
||||
result = _apply(kind, original)
|
||||
except RecursionError:
|
||||
# Keep pytest from comparing deeply nested frame locals while locating
|
||||
# recursion: that can hang its traceback formatter on this graph.
|
||||
raise AssertionError("Deep requests must still run middleware") from None
|
||||
|
||||
assert result.payload["accepted"] is True
|
||||
assert _leaf(result.payload["node"])["steps"] == ["original", "second"]
|
||||
assert _leaf(result.original_payload["node"])["steps"] == ["original"]
|
||||
assert _leaf(original["node"])["steps"] == ["original"]
|
||||
assert result.payload["node"] is result.payload["alias"]
|
||||
assert result.original_payload["node"] is result.original_payload["alias"]
|
||||
assert result.payload["cycle"][0][0] is result.payload["cycle"]
|
||||
assert result.original_payload["cycle"][0][0] is result.original_payload["cycle"]
|
||||
assert result.payload["cycle_alias"] is result.payload["cycle"]
|
||||
assert result.original_payload["cycle_alias"] is result.original_payload["cycle"]
|
||||
assert result.payload["opaque"] is result.original_payload["opaque"] is opaque
|
||||
assert result.payload["frozen"] == result.original_payload["frozen"] == original["frozen"]
|
||||
assert _leaf(result.payload["node"])["file"] is handle
|
||||
assert not handle.closed
|
||||
assert [entry["source"] for entry in result.trace] == ["first", "second"]
|
||||
copied_node = result.payload["node"]
|
||||
snapshot_node = result.original_payload["node"]
|
||||
source_node = cast(Any, original["node"])
|
||||
for _ in range(depth):
|
||||
index = "child" if wrapper is dict else 0
|
||||
copied_node = copied_node[index]
|
||||
snapshot_node = snapshot_node[index]
|
||||
source_node = source_node[index]
|
||||
assert type(copied_node) is type(snapshot_node) is type(source_node) is container
|
||||
if container is defaultdict:
|
||||
assert copied_node.default_factory is snapshot_node.default_factory is list
|
||||
if container in (RequestDict, RequestList):
|
||||
copied_node.metadata.append("accepted-only")
|
||||
assert source_node.metadata == snapshot_node.metadata == ["original"]
|
||||
result.payload["tags"].add("accepted-only")
|
||||
assert original["tags"] == result.original_payload["tags"] == {"original"}
|
||||
@@ -2718,76 +2718,3 @@ class TestAsyncHookOnCallerLoop:
|
||||
results = asyncio.run(mgr.ainvoke_hook("pre_gateway_dispatch", event="e", gateway="g"))
|
||||
assert results == [{"seen": "e"}, {"seen_async": "e"}]
|
||||
assert "async plugin blew up" in caplog.text
|
||||
|
||||
|
||||
def _load_request_chain_plugin(tmp_path, monkeypatch, failing=False):
|
||||
from hermes_cli import plugins
|
||||
home = tmp_path / "home"
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
plugin = _make_plugin_dir(home / "plugins", "request_chain")
|
||||
plugin.joinpath("__init__.py").write_text("""
|
||||
def register(ctx):
|
||||
for kind, key in [('llm_request', 'request'), ('tool_request', 'args')]:
|
||||
def first(_key=key, **kw):
|
||||
return {_key: {**kw[_key], 'steps': ['first']}, 'source': 'first'}
|
||||
def broken(_key=key, **kw):
|
||||
kw[_key]['steps'].append('broken')
|
||||
kw[_key]['input'].append('broken')
|
||||
raise RuntimeError('fixture failure')
|
||||
def observer(_key=key, **kw):
|
||||
kw[_key]['steps'].append('unaccepted')
|
||||
def second(_key=key, **kw):
|
||||
return {_key: {**kw[_key], 'second': True}, 'source': 'second'}
|
||||
ctx.register_middleware(kind, first)
|
||||
if FAILING:
|
||||
ctx.register_middleware(kind, broken)
|
||||
ctx.register_middleware(kind, observer)
|
||||
ctx.register_middleware(kind, second)
|
||||
""".replace("FAILING", str(failing)))
|
||||
manager = PluginManager()
|
||||
manager.discover_and_load()
|
||||
monkeypatch.setattr(plugins, "get_plugin_manager", lambda: manager)
|
||||
assert manager.has_middleware("llm_request")
|
||||
return manager
|
||||
|
||||
|
||||
def test_request_middleware_chains_discovered_plugins(tmp_path, monkeypatch):
|
||||
_load_request_chain_plugin(tmp_path, monkeypatch)
|
||||
for apply in (apply_llm_request_middleware,
|
||||
lambda payload: apply_tool_request_middleware("fixture", payload)):
|
||||
original = {"input": ["original"]}
|
||||
result = apply(original)
|
||||
assert result.payload == {**original, "steps": ["first"], "second": True}
|
||||
assert result.original_payload == original == {"input": ["original"]}
|
||||
assert [entry["source"] for entry in result.trace] == ["first", "second"]
|
||||
|
||||
|
||||
def test_failed_or_observer_middleware_cannot_mutate_accepted_rewrites(tmp_path, monkeypatch):
|
||||
_load_request_chain_plugin(tmp_path, monkeypatch, failing=True)
|
||||
# Real provider payloads can contain opaque handles. One such leaf must not
|
||||
# turn every nested request container into a shared, mutable reference.
|
||||
with (tmp_path / "upload.txt").open("w+", encoding="utf-8") as handle:
|
||||
for opaque in ({}, {"file": handle}):
|
||||
for apply in (apply_llm_request_middleware,
|
||||
lambda payload: apply_tool_request_middleware("fixture", payload)):
|
||||
original = {"input": ["original"], **opaque}
|
||||
shared = {"tags": {"original"}}
|
||||
original["metadata"] = (shared, shared)
|
||||
if opaque:
|
||||
cycle = []
|
||||
original["cycle"] = (cycle, handle)
|
||||
cycle.append(original["cycle"])
|
||||
result = apply(original)
|
||||
assert result.payload["steps"] == ["first"]
|
||||
assert result.payload["second"] is True
|
||||
assert original["input"] == result.original_payload["input"] == ["original"]
|
||||
assert result.payload["input"] == ["original"]
|
||||
assert [entry["source"] for entry in result.trace] == ["first", "second"]
|
||||
assert result.payload["metadata"][0] is result.payload["metadata"][1]
|
||||
result.payload["metadata"][0]["tags"].add("accepted-only")
|
||||
assert shared["tags"] == {"original"}
|
||||
assert result.original_payload["metadata"][0]["tags"] == {"original"}
|
||||
if opaque:
|
||||
assert result.payload["file"] is handle
|
||||
assert not handle.closed
|
||||
assert result.payload["cycle"][0][0] is result.payload["cycle"]
|
||||
|
||||
67
tests/hermes_cli/test_plugins_middleware_chain.py
Normal file
67
tests/hermes_cli/test_plugins_middleware_chain.py
Normal file
@@ -0,0 +1,67 @@
|
||||
"""Request middleware chaining (#128638): rewrites compose in registration order."""
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import plugins
|
||||
from hermes_cli.middleware import apply_llm_request_middleware, apply_tool_request_middleware
|
||||
from hermes_cli.plugins import PluginManager
|
||||
|
||||
_REQUEST_KINDS = [
|
||||
("llm_request", apply_llm_request_middleware),
|
||||
("tool_request", lambda payload: apply_tool_request_middleware("fixture", payload)),
|
||||
]
|
||||
|
||||
# Per request kind: ``first`` rewrites, ``meddler`` mutates its views in place and returns None,
|
||||
# ``second`` records what it saw and rewrites.
|
||||
_PLUGIN = """
|
||||
SEEN = {}
|
||||
|
||||
def register(ctx):
|
||||
for kind, key in [('llm_request', 'request'), ('tool_request', 'args')]:
|
||||
def first(_key=key, **kw):
|
||||
return {_key: {**kw[_key], 'steps': ['first']}, 'source': 'first'}
|
||||
def meddler(_key=key, **kw):
|
||||
kw[_key].setdefault('steps', []).append('meddler')
|
||||
kw['original_' + _key]['input'].append('meddler')
|
||||
def second(_key=key, _kind=kind, **kw):
|
||||
SEEN[_kind] = (kw[_key], kw['original_' + _key])
|
||||
return {_key: {**kw[_key], 'second': True}, 'source': 'second'}
|
||||
for callback in (first, meddler, second):
|
||||
ctx.register_middleware(kind, callback)
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def seen(tmp_path, monkeypatch):
|
||||
home = tmp_path / "home"
|
||||
plugin = home / "plugins" / "request_chain"
|
||||
plugin.mkdir(parents=True)
|
||||
(plugin / "plugin.yaml").write_text("name: request_chain\nversion: 0.1.0\n")
|
||||
(plugin / "__init__.py").write_text(_PLUGIN)
|
||||
(home / "config.yaml").write_text("plugins:\n enabled: [request_chain]\n")
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
manager = PluginManager()
|
||||
manager.discover_and_load()
|
||||
monkeypatch.setattr(plugins, "get_plugin_manager", lambda: manager)
|
||||
return manager._plugins["request_chain"].module.SEEN
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind,apply", _REQUEST_KINDS, ids=[k for k, _ in _REQUEST_KINDS])
|
||||
def test_request_middleware_rewrites_compose(seen, kind, apply):
|
||||
result = apply({"input": ["original"]})
|
||||
|
||||
# The second callback sees the first one's rewrite, not the original request.
|
||||
assert seen[kind][0] == {"input": ["original"], "steps": ["first"]}
|
||||
assert result.payload == {"input": ["original"], "steps": ["first"], "second": True}
|
||||
assert [entry["source"] for entry in result.trace] == ["first", "second"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kind,apply", _REQUEST_KINDS, ids=[k for k, _ in _REQUEST_KINDS])
|
||||
def test_request_middleware_in_place_mutation_does_not_leak(seen, kind, apply):
|
||||
original = {"input": ["original"]}
|
||||
|
||||
result = apply(original)
|
||||
|
||||
# Each callback gets its own copy of the payload and of the pre-middleware snapshot.
|
||||
assert "meddler" not in result.payload["steps"]
|
||||
assert seen[kind][1] == result.original_payload == original == {"input": ["original"]}
|
||||
Reference in New Issue
Block a user