mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-09 08:21:50 +00:00
fix(middleware): preserve translated downstream failures
Track successful next_call completion separately from invocation so execution middleware that catches and translates a downstream provider/tool failure does not accidentally convert that failure into a successful None result. Also avoid wrapping BaseException from downstream execution, and document the execution middleware error semantics. Tests cover: - pre-next_call middleware failures fail open to the remaining chain - post-next_call middleware failures preserve the downstream result - translated downstream failures propagate instead of returning None - downstream BaseException is not wrapped Signed-off-by: Bryan Bednarski <bbednarski@nvidia.com>
This commit is contained in:
parent
2e0c9083db
commit
5abe45674d
3 changed files with 81 additions and 3 deletions
|
|
@ -244,6 +244,15 @@ For NeMo Relay adaptive execution middleware, see
|
|||
patches.
|
||||
- Execution middleware should call `next_call(...)` exactly once unless it is
|
||||
intentionally short-circuiting execution.
|
||||
- If execution middleware raises before calling `next_call(...)`, Hermes treats
|
||||
that as middleware failure and continues with the remaining middleware chain
|
||||
and base execution.
|
||||
- If execution middleware calls `next_call(...)` successfully and then raises
|
||||
during post-processing, Hermes preserves the downstream result and does not
|
||||
run the provider or tool a second time.
|
||||
- If downstream provider or tool execution fails, middleware may let that error
|
||||
propagate or translate it deliberately. Hermes does not convert downstream
|
||||
failure into a successful `None` result.
|
||||
- Tool request middleware runs before approvals. If it mutates file paths,
|
||||
commands, URLs, or arguments, the mutated values are what guardrails and
|
||||
approvals evaluate.
|
||||
|
|
|
|||
|
|
@ -237,15 +237,17 @@ def _run_execution_chain(
|
|||
|
||||
callback = callbacks[index]
|
||||
next_called = False
|
||||
next_succeeded = False
|
||||
next_result: Any = None
|
||||
|
||||
def next_call(next_payload: Any = None) -> Any:
|
||||
nonlocal next_called, next_result
|
||||
nonlocal next_called, next_succeeded, next_result
|
||||
next_called = True
|
||||
try:
|
||||
next_result = call_at(index + 1, payload if next_payload is None else next_payload)
|
||||
next_succeeded = True
|
||||
return next_result
|
||||
except BaseException as exc:
|
||||
except Exception as exc:
|
||||
raise _DownstreamExecutionError(exc) from exc
|
||||
|
||||
call_kwargs = middleware_payload(**kwargs)
|
||||
|
|
@ -262,8 +264,10 @@ def _run_execution_chain(
|
|||
getattr(callback, "__name__", repr(callback)),
|
||||
exc,
|
||||
)
|
||||
if next_called:
|
||||
if next_succeeded:
|
||||
return next_result
|
||||
if next_called:
|
||||
raise
|
||||
return call_at(index + 1, payload)
|
||||
|
||||
return call_at(0, kwargs[payload_key])
|
||||
|
|
|
|||
|
|
@ -207,6 +207,71 @@ class TestPluginDiscovery:
|
|||
assert result == "terminal-result"
|
||||
assert calls == [{"command": "printf ok"}]
|
||||
|
||||
def test_execution_middleware_pre_next_call_error_fails_open_to_remaining_chain(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def failing_middleware(**kwargs):
|
||||
calls.append("failing")
|
||||
raise RuntimeError("middleware setup failed")
|
||||
|
||||
def downstream_middleware(**kwargs):
|
||||
calls.append("downstream")
|
||||
return kwargs["next_call"]({**kwargs["args"], "rewritten": True})
|
||||
|
||||
manager = types.SimpleNamespace(_middleware={"tool_execution": [failing_middleware, downstream_middleware]})
|
||||
monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
|
||||
|
||||
def terminal(args):
|
||||
calls.append(("terminal", args))
|
||||
return args
|
||||
|
||||
result = run_tool_execution_middleware("terminal", {"command": "printf ok"}, terminal)
|
||||
|
||||
assert result == {"command": "printf ok", "rewritten": True}
|
||||
assert calls == ["failing", "downstream", ("terminal", {"command": "printf ok", "rewritten": True})]
|
||||
|
||||
def test_execution_middleware_translated_downstream_failure_is_not_masked(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def middleware(**kwargs):
|
||||
try:
|
||||
return kwargs["next_call"](kwargs["args"])
|
||||
except Exception as exc:
|
||||
raise RuntimeError(f"translated downstream failure: {exc}") from exc
|
||||
|
||||
manager = types.SimpleNamespace(_middleware={"tool_execution": [middleware]})
|
||||
monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
|
||||
|
||||
def terminal(args):
|
||||
calls.append(args)
|
||||
raise RuntimeError("terminal failed")
|
||||
|
||||
with pytest.raises(RuntimeError, match="translated downstream failure: terminal failed"):
|
||||
run_tool_execution_middleware("terminal", {"command": "false"}, terminal)
|
||||
|
||||
assert calls == [{"command": "false"}]
|
||||
|
||||
def test_execution_middleware_downstream_base_exception_is_not_wrapped(self, monkeypatch):
|
||||
calls = []
|
||||
|
||||
def middleware(**kwargs):
|
||||
try:
|
||||
return kwargs["next_call"](kwargs["args"])
|
||||
except Exception as exc:
|
||||
raise RuntimeError(f"middleware should not catch base exception: {exc}") from exc
|
||||
|
||||
manager = types.SimpleNamespace(_middleware={"tool_execution": [middleware]})
|
||||
monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
|
||||
|
||||
def terminal(args):
|
||||
calls.append(args)
|
||||
raise KeyboardInterrupt()
|
||||
|
||||
with pytest.raises(KeyboardInterrupt):
|
||||
run_tool_execution_middleware("terminal", {"command": "interrupt"}, terminal)
|
||||
|
||||
assert calls == [{"command": "interrupt"}]
|
||||
|
||||
def test_discover_project_plugins(self, tmp_path, monkeypatch):
|
||||
"""Plugins in ./.hermes/plugins/ are discovered."""
|
||||
project_dir = tmp_path / "project"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue