This is phase 2 of the NeMo-Flow stacked change, depending on the middleware hooks from PR #29722. Supersedes the combined PR #29551. Related: draft #28611, unified telemetry tracking #6642. P3 per plugin-provider-demotion taste rule.

Blocked on #29722 review — left detailed notes on the observer/middleware split there: #29722 (comment). This PR is unblocked once the observer half of #29722 lands.

• Rebased the PR down to a single middleware/adaptive commit on current main, after the observer half landed in perf(observability): observer telemetry hooks + NeMo-Relay plugin, gated tool emit (salvage #38190/#29722) #38232
• Added has_middleware(kind) mirroring has_hook().
• Gated apply_* and run_* middleware entry points so the no-listener path returns by reference without deepcopy/callback setup.
• Changed request middleware changed tracking to bool(trace).
• Fixed the post-next_call middleware exception path so it does not re-run the downstream tool/LLM call.
• Added regression coverage for no-listener fast path, changed = bool(trace), and post-next_call exceptions.

Responses to individual comments

1. Middleware isn't gated on a registered consumer

Thanks for pointing this out - I had forgotten to propagate these changes into this PR. Super helpful.

Resolved and measured on the restacked branch.

I added has_middleware(kind) mirroring has_hook() and moved the middleware checks ahead of any deepcopy() or execution-chain setup. With no middleware registered, the request middleware helpers now return by reference:

• payload is request / original_payload is request
• payload is args / original_payload is args
• changed=False
• trace=[]

The execution middleware helpers also bypass chain setup and call the downstream callback directly when no callbacks are registered.

I also benchmarked the no-consumer path with chat-shaped payloads. I used many message/tool-result dicts rather than one giant string, because deepcopy() reuses immutable string objects and under-represents traversal cost.

Interpretation: the default install/no-consumer path is effectively just the registry presence check and is flat with payload size. If request middleware is actually registered, that remains an explicit opt-in behavior-changing path and still pays the defensive copy/traversal cost. I added regression coverage for the no-listener by-reference behavior so this does not regress.

2. Deep equality compare

I changed request middleware result tracking from structural deep equality to trace-based tracking:

changed=bool(trace)

That applies to both LLM request middleware and tool request middleware. This avoids walking the full request/args payload a third time after the defensive copies, and it also reflects the actual middleware contract: a request is considered changed when a middleware callback returned a valid replacement payload and produced a trace entry.

I added regression coverage for the edge case where middleware returns an equal/same-shaped payload. That now reports changed=True because middleware did run and supplied a replacement, without relying on expensive structural comparison.

3. Double-execution if a middleware raises after calling next_call

I updated _run_execution_chain to track whether next_call() was already invoked in the current middleware frame. If the middleware raises after a successful downstream call, the generic middleware-error handler now returns the already-computed downstream result instead of retrying call_at(index + 1).

I also added regression coverage for this exact case:

def test_execution_middleware_post_next_call_error_does_not_retry(...)

The test verifies the terminal callback is called exactly once and the original downstream result is preserved.

Reviewed the restacked commit 2e0c908 end-to-end (read the contract + all four hot-path call sites, ran the 4 targeted suites → 491 passed, plus an adversarial pass with a second reviewer + standalone repros). The three items from my earlier notes are genuinely resolved and each has a real regression test — nice. One new High-severity hole that the post-next_call fix opened up, with a small fix.

✅ Confirmed resolved

• Consumer gating — has_middleware(kind) mirrors has_hook(); no-listener path returns by reference (payload is request verified, locked by test_middleware_helpers_skip_no_listener_work). Benchmarks match.
• changed = bool(trace) — correct and intentional; can't produce a false changed=False. (Minor: .changed currently has no runtime consumer — only asserted in tests. Fine to leave, just flagging.)
• Post-next_call no automatic re-run — the terminal can't run twice on the automatic path; covered by test_execution_middleware_post_next_call_error_does_not_retry.
• Also nice: invoke_tool → handle_function_call passes skip_tool_request_middleware=True + forwards the trace, so request middleware doesn't double-apply on the agent-loop path.

🔴 High — downstream failure silently masked as None (hermes_cli/middleware.py:244, 265-266)

The post-next_call fix opened the mirror-image bug. next_called is set True at line 244 before the recursion at line 246 can fail, but next_result is only assigned after a successful recursion. So when:

1. middleware calls next_call(),
2. the downstream LLM/tool call genuinely raises → next_result stays None, next_called is already True,
3. middleware catches the _DownstreamExecutionError and raises its own error (normal error-translation pattern),
4. line 258 catches it, next_called is True → line 266 returns next_result = None.

A real provider/tool failure gets reported as a successful empty result. Reproduced standalone:

RESULT: None
terminal_runs: 1   # terminal ran once and failed; chain returned None instead of raising

This path isn't covered — test_execution_middleware_does_not_retry_downstream_failure only exercises a middleware that lets the error propagate; the catch-and-reraise case is the gap.

Minimal fix — track downstream success, not just invocation:

next_succeeded = False
def next_call(next_payload=None):
    nonlocal next_called, next_succeeded, next_result
    next_called = True
    next_result = call_at(index + 1, payload if next_payload is None else next_payload)
    next_succeeded = True
    return next_result
...
except Exception as exc:
    logger.warning(...)
    if next_succeeded:          # was: if next_called
        return next_result
    if next_called:             # downstream failed + middleware re-raised → propagate
        raise
    return call_at(index + 1, payload)

Worth a regression test where the downstream raises and the middleware catches-then-reraises (asserts the error propagates, not None).

🟡 Optional (non-blocking)

• next_call isn't single-use — a middleware that deliberately calls next_call() twice runs the terminal twice. Doesn't violate the "no automatic retry" claim (that holds), but if the intended contract is "terminal executes at most once," it needs a single-use guard.
• deepcopy of the LLM request (lines 75-76, 90) — unsafe if api_kwargs ever carries non-deepcopyable/stateful objects (clients, callbacks, file handles). Guarded by the caller try/except against hard failures, but a silent client-clone would slip through. Tool path is safe (plain JSON args). A deepcopy-safety comment, or shallow-copy of the known-mutated keys, would harden it.

Doesn't strictly block merge today — the High trigger needs a catch-and-reraise consumer, and the only in-repo consumer (NeMo-Relay) calls its own captured next_call rather than the chain wrapper. But it's a real latent hole in a brand-new control-plane contract and the fix is ~4 lines + a test, so I'd rather get it right now than paper over it later. Everything else looks good.

@kshitijk4poor Thanks for the follow-up. I addressed your comments and added some additional test cases and documentation.

What changed:

• _run_execution_chain now tracks next_succeeded separately from next_called.
• next_succeeded is set only after downstream execution returns successfully.
• If middleware calls next_call(), downstream fails, and the middleware catches/translates that failure, Hermes now propagates the translated error instead of returning None.
• If middleware calls next_call() successfully and later raises during post-processing, Hermes still preserves the downstream result and does not re-run the provider/tool.
• next_call() now wraps ordinary Exception downstream failures, not BaseException, so interrupts/cancellation-style base exceptions are not accidentally converted into normal middleware errors.

I added regression coverage for the edge cases around this contract:

• test_execution_middleware_translated_downstream_failure_is_not_masked
• test_execution_middleware_pre_next_call_error_fails_open_to_remaining_chain
• test_execution_middleware_downstream_base_exception_is_not_wrapped

I also updated docs/middleware/README.md to spell out the execution middleware error semantics for plugin authors:

• pre-next_call middleware failures fail open to the remaining chain/base execution;
• post-success middleware failures preserve the downstream result and do not re-run execution;
• downstream provider/tool failures can propagate or be deliberately translated, but are not converted into a successful None result.

Validation:

99 passed in 3.60s

Targeted command:

.venv/bin/python -m pytest tests/hermes_cli/test_plugins.py tests/plugins/test_nemo_relay_plugin.py -q

Thanks @kshitijk4poor