fix(plugins): route execution middleware through lazy discovery (#105827)
`_run_execution_chain` read `get_plugin_manager()._middleware` directly, bypassing the lazy discovery every other delivery entry point gained in the #64178 parity work, so `tool_execution`/`llm_execution` middleware registered by user plugins silently failed to fire (fail-open) on surfaces that never run discovery at startup: query mode `chat -q`/`-z`, cron delivery, dashboard, TUI slash workers. Route it through `_delivery_manager()` like invoke_hook/invoke_middleware/has_middleware. (cherry picked from commit 06b9d44095733f8e46f40f0331b3f6dff352e98e) Fixes #105827 Salvages #105832
This commit is contained in:
@@ -159,10 +159,10 @@ class _DownstreamExecutionError(Exception):
|
||||
|
||||
|
||||
def _run_execution_chain(kind: str, terminal_call: Callable[[Any], Any], **kwargs: Any) -> Any:
|
||||
from hermes_cli.plugins import get_plugin_manager
|
||||
from hermes_cli.plugins import _delivery_manager
|
||||
|
||||
payload_key = "request" if "request" in kwargs else "args"
|
||||
manager = get_plugin_manager()
|
||||
manager = _delivery_manager()
|
||||
callbacks = list(manager._middleware.get(kind, []))
|
||||
if not callbacks:
|
||||
return terminal_call(kwargs[payload_key])
|
||||
|
||||
@@ -31,6 +31,7 @@ from hermes_cli.middleware import (
|
||||
VALID_MIDDLEWARE,
|
||||
apply_llm_request_middleware,
|
||||
apply_tool_request_middleware,
|
||||
run_llm_execution_middleware,
|
||||
run_tool_execution_middleware,
|
||||
)
|
||||
|
||||
@@ -1018,6 +1019,48 @@ class TestDeliveryParity:
|
||||
|
||||
assert plugins_mod.invoke_hook("anything") == ["stubbed"]
|
||||
|
||||
def test_execution_chain_lazily_discovers(self, monkeypatch):
|
||||
"""Execution middleware must fire on cold surfaces too (#105827).
|
||||
|
||||
``run_tool_execution_middleware`` / ``run_llm_execution_middleware`` deliver via
|
||||
``_run_execution_chain``, which used to read ``get_plugin_manager()._middleware``
|
||||
directly — no lazy discovery — so a registered fail-closed policy gate was silently
|
||||
skipped (fail-open) on surfaces that never ran discovery at startup (query mode
|
||||
``chat -q``, cron delivery, dashboard, TUI slash workers). The chain must route
|
||||
through ``_delivery_manager()`` like every other delivery entry point (#64178).
|
||||
"""
|
||||
fired = []
|
||||
terminal_calls = []
|
||||
|
||||
def _tool_gate(**kw):
|
||||
fired.append(kw.get("tool_name"))
|
||||
return {"denied": True} # fail-closed: returns without calling next_call
|
||||
|
||||
def _llm_gate(**kw):
|
||||
fired.append("llm")
|
||||
return {"denied": True}
|
||||
|
||||
def _register(m):
|
||||
m._middleware.setdefault("tool_execution", []).append(_tool_gate)
|
||||
m._middleware.setdefault("llm_execution", []).append(_llm_gate)
|
||||
|
||||
mgr = self._fresh_manager(monkeypatch, _register)
|
||||
|
||||
def _terminal(payload):
|
||||
terminal_calls.append(payload)
|
||||
return "terminal-ran"
|
||||
|
||||
tool_result = run_tool_execution_middleware("terminal", {"path": "x"}, _terminal)
|
||||
assert mgr._discovered is True, "execution chain must lazily discover on cold surfaces"
|
||||
assert fired == ["terminal"]
|
||||
assert tool_result == {"denied": True}
|
||||
assert terminal_calls == [], "a fail-closed gate must not be bypassed by terminal execution"
|
||||
|
||||
llm_result = run_llm_execution_middleware({"messages": []}, _terminal)
|
||||
assert fired == ["terminal", "llm"]
|
||||
assert llm_result == {"denied": True}
|
||||
assert terminal_calls == []
|
||||
|
||||
|
||||
class TestAsyncHookCallbacks:
|
||||
"""``async def`` hook callbacks run and their values land in the results (#12449)."""
|
||||
|
||||
Reference in New Issue
Block a user