From 5abe45674dc7eaf72190d97785738ea2ea8b607b Mon Sep 17 00:00:00 2001 From: Bryan Bednarski Date: Sat, 6 Jun 2026 09:26:18 -0700 Subject: [PATCH] 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 --- docs/middleware/README.md | 9 +++++ hermes_cli/middleware.py | 10 +++-- tests/hermes_cli/test_plugins.py | 65 ++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 3 deletions(-) diff --git a/docs/middleware/README.md b/docs/middleware/README.md index b385b87eb29..4a5c06f8cbe 100644 --- a/docs/middleware/README.md +++ b/docs/middleware/README.md @@ -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. diff --git a/hermes_cli/middleware.py b/hermes_cli/middleware.py index 938bffcf172..277368dffb3 100644 --- a/hermes_cli/middleware.py +++ b/hermes_cli/middleware.py @@ -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]) diff --git a/tests/hermes_cli/test_plugins.py b/tests/hermes_cli/test_plugins.py index 6bff7b6d87d..ddd1dab56e4 100644 --- a/tests/hermes_cli/test_plugins.py +++ b/tests/hermes_cli/test_plugins.py @@ -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"