fix(middleware): complete transactional request transform contract

docs/middleware/README.md

@@ -62,6 +62,20 @@ return {
 Hermes stores those trace entries in later observer hook payloads as
 `middleware_trace`.
 
+LLM request callbacks compose in registration order: each accepted complete
+`{"request": {...}}` replacement becomes the next callback's effective request,
+while `original_request` remains the pre-middleware snapshot. Each callback sees
+isolated deep copies. If a callback mutates its candidate and raises, that
+candidate is discarded and Hermes continues from the last committed request.
+There is no shallow-copy fallback: if the complete request cannot be deep-copied,
+Hermes logs the isolation failure, skips `llm_request` for that call, and sends
+the original request unchanged.
+
+`llm_request` delivery performs first-use plugin discovery itself, so a
+configured plugin runs even on surfaces that never import the model-tool layer
+(dashboards, TUI slash workers, query mode, cron) and is never mistaken for a
+missing listener in a fresh process.
+
 Execution middleware receives a `next_call` callback. Call it to continue the
 chain:
 

hermes_cli/middleware.py

@@ -78,35 +78,97 @@ def apply_llm_request_middleware(
     request: Dict[str, Any],
     **context: Any,
 ) -> RequestMiddlewareResult:
-    """Apply registered LLM request middleware.
+    """Apply registered LLM request middleware transactionally.
 
-    Middleware may return ``{"request": {...}}`` to replace the effective
-    provider kwargs before Hermes sends them.
+    Callbacks run in registration order on isolated copies of the complete
+    provider request. A replacement commits only after it can be deep-copied.
+    If the original request cannot be isolated, middleware is skipped and the
+    original request is returned unchanged.
     """
-    if not _has_middleware(LLM_REQUEST_MIDDLEWARE):
+    # Plugin discovery is normally triggered as an import side effect of
+    # model_tools, but this seam must not depend on which surface imported
+    # Hermes first (dashboards, TUI slash workers, query mode, cron). Ensure
+    # first-use discovery has run before snapshotting callbacks so a
+    # configured plugin in a fresh process is not mistaken for "no middleware".
+    # discover_plugins() is idempotent.
+    from hermes_cli.plugins import discover_plugins
+
+    discover_plugins()
+
+    callbacks = _get_middleware_callbacks(LLM_REQUEST_MIDDLEWARE)
+    if not callbacks:
         return RequestMiddlewareResult(
             payload=request,
             original_payload=request,
             changed=False,
             trace=[],
         )
 
-    original_request = _safe_copy(request)
-    current_request = _safe_copy(original_request)
+    try:
+        original_request = deepcopy(request)
+    except Exception as exc:
+        logger.warning(
+            "Middleware 'llm_request' skipped: request payload could not be "
+            "isolated transactionally: %s",
+            exc,
+        )
+        return RequestMiddlewareResult(
+            payload=request,
+            original_payload=request,
+            changed=False,
+            trace=[],
+        )
+
+    current_request = original_request
     trace: List[Dict[str, Any]] = []
 
-    for result in _invoke_middleware(
-        LLM_REQUEST_MIDDLEWARE,
-        request=current_request,
-        original_request=original_request,
-        **context,
-    ):
+    # ponytail: keep the transaction boundary inline until a second request
+    # chain actually needs the same abstraction.
+    for callback in callbacks:
+        try:
+            callback_request = deepcopy(current_request)
+            callback_original = deepcopy(original_request)
+        except Exception as exc:
+            logger.warning(
+                "Middleware 'llm_request' callback %s skipped: request payload "
+                "could not be isolated transactionally: %s",
+                getattr(callback, "__name__", repr(callback)),
+                exc,
+            )
+            continue
+
+        try:
+            result = callback(
+                **middleware_payload(
+                    **context,
+                    request=callback_request,
+                    original_request=callback_original,
+                )
+            )
+        except Exception as exc:
+            logger.warning(
+                "Middleware 'llm_request' callback %s raised: %s",
+                getattr(callback, "__name__", repr(callback)),
+                exc,
+            )
+            continue
+
         if not isinstance(result, dict):
             continue
         next_request = result.get("request")
         if not isinstance(next_request, dict):
             continue
-        current_request = _safe_copy(next_request)
+
+        try:
+            current_request = deepcopy(next_request)
+        except Exception as exc:
+            logger.warning(
+                "Middleware 'llm_request' callback %s returned a request that "
+                "could not be isolated transactionally: %s",
+                getattr(callback, "__name__", repr(callback)),
+                exc,
+            )
+            continue
         trace.append(_trace_entry(result))
 
     return RequestMiddlewareResult(

tests/hermes_cli/test_plugins.py

@@ -292,7 +292,13 @@ def test_plugin_can_register_and_invoke_middleware(self, tmp_path, monkeypatch):
 
 
     def test_middleware_helpers_skip_no_listener_work(self, monkeypatch):
-        manager = types.SimpleNamespace(_middleware={})
+        manager = types.SimpleNamespace(
+            _middleware={},
+            # apply_llm_request_middleware triggers first-use discovery before
+            # snapshotting callbacks; the no-op models an already-discovered
+            # manager with no registered listeners.
+            discover_and_load=lambda force=False: None,
+        )
         monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
 
         request = {"messages": []}

tests/hermes_cli/test_request_transform_contract.py

@@ -0,0 +1,191 @@
+"""Regression tests for the outbound LLM request transform boundary."""
+
+from __future__ import annotations
+
+import inspect
+import logging
+import types
+
+from agent import conversation_loop
+from hermes_cli.middleware import apply_llm_request_middleware
+
+
+class _UncopyableProviderObject:
+    def __deepcopy__(self, memo):
+        raise TypeError("opaque provider object cannot be deep-copied")
+
+
+def _install(monkeypatch, *callbacks):
+    manager = types.SimpleNamespace(
+        _middleware={"llm_request": list(callbacks)},
+        # The mocked manager models "already discovered"; discover_and_load is
+        # a no-op because the middleware is pre-registered.
+        discover_and_load=lambda force=False: None,
+    )
+    monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
+
+
+def test_fresh_process_discovers_configured_plugin_before_snapshot(monkeypatch):
+    """A configured llm_request plugin must run on first delivery even when
+    plugin discovery has not happened yet — this seam cannot depend on which
+    surface imported Hermes first. Without the discovery-aware snapshot a
+    fresh manager (empty registry, _discovered False) was mistaken for "no
+    middleware" and silently skipped the configured transform."""
+
+    manager = types.SimpleNamespace(
+        _middleware={},
+        _discovered=False,
+    )
+
+    def _discover_and_load(force=False):
+        # Simulate first-use discovery registering a configured plugin.
+        if "llm_request" not in manager._middleware:
+            manager._middleware["llm_request"] = [
+                lambda **kw: {
+                    "request": {**kw["request"], "fresh": True},
+                    "source": "fresh",
+                }
+            ]
+
+    manager.discover_and_load = _discover_and_load
+    monkeypatch.setattr("hermes_cli.plugins.get_plugin_manager", lambda: manager)
+
+    request = {"messages": [{"role": "user", "content": "hello"}]}
+    result = apply_llm_request_middleware(request)
+
+    assert result.payload == {**request, "fresh": True}
+    assert result.changed is True
+    assert result.trace == [{"source": "fresh"}]
+
+
+def test_missing_middleware_preserves_request_identity(monkeypatch):
+    _install(monkeypatch)
+    request = {"messages": [{"role": "user", "content": "hello"}]}
+
+    result = apply_llm_request_middleware(request)
+
+    assert result.payload is request
+    assert result.original_payload is request
+    assert result.changed is False
+    assert result.trace == []
+
+
+def test_request_replacements_chain_with_stable_original(monkeypatch):
+    seen = []
+
+    def first(**kwargs):
+        kwargs["original_request"]["tampered"] = True
+        return {"request": {**kwargs["request"], "first": True}, "source": "first"}
+
+    def second(**kwargs):
+        seen.append((kwargs["request"], kwargs["original_request"]))
+        return {"request": {**kwargs["request"], "second": True}, "source": "second"}
+
+    _install(monkeypatch, first, second)
+    request = {
+        "model": "test-model",
+        "messages": [{"role": "user", "content": "hello"}],
+        "tools": [{"type": "function", "function": {"name": "demo"}}],
+        "stream": True,
+    }
+
+    result = apply_llm_request_middleware(request)
+
+    assert seen == [({**request, "first": True}, request)]
+    assert result.payload == {**request, "first": True, "second": True}
+    assert result.original_payload == request
+    assert result.trace == [{"source": "first"}, {"source": "second"}]
+
+
+def test_callback_failure_discards_nested_mutation(monkeypatch, caplog):
+    seen = []
+
+    def first(**kwargs):
+        return {"request": {**kwargs["request"], "first": True}, "source": "first"}
+
+    def failing(**kwargs):
+        kwargs["request"]["messages"][0]["content"] = "corrupted"
+        kwargs["request"]["partial"] = True
+        raise RuntimeError("broken transformer")
+
+    def last(**kwargs):
+        seen.append(kwargs["request"])
+        return {"request": {**kwargs["request"], "last": True}, "source": "last"}
+
+    _install(monkeypatch, first, failing, last)
+    request = {"messages": [{"role": "user", "content": "original"}]}
+
+    with caplog.at_level(logging.WARNING):
+        result = apply_llm_request_middleware(request)
+
+    committed = {"messages": [{"role": "user", "content": "original"}], "first": True}
+    assert seen == [committed]
+    assert result.payload == {**committed, "last": True}
+    assert request == {"messages": [{"role": "user", "content": "original"}]}
+    assert "broken transformer" in caplog.text
+
+
+def test_uncopyable_original_skips_callbacks_without_shared_state(monkeypatch, caplog):
+    called = False
+
+    def callback(**kwargs):
+        nonlocal called
+        called = True
+        kwargs["request"]["messages"][0]["content"] = "changed"
+        raise RuntimeError("must not run")
+
+    _install(monkeypatch, callback)
+    request = {
+        "messages": [{"role": "user", "content": "original"}],
+        "provider_state": _UncopyableProviderObject(),
+    }
+
+    with caplog.at_level(logging.WARNING):
+        result = apply_llm_request_middleware(request)
+
+    assert called is False
+    assert result.payload is request
+    assert result.original_payload is request
+    assert request["messages"][0]["content"] == "original"
+    assert "could not be isolated transactionally" in caplog.text
+
+
+def test_uncopyable_replacement_is_not_committed(monkeypatch, caplog):
+    def callback(**kwargs):
+        kwargs["request"]["messages"][0]["content"] = "candidate-only"
+        return {
+            "request": {
+                **kwargs["request"],
+                "provider_state": _UncopyableProviderObject(),
+            },
+            "source": "uncopyable",
+        }
+
+    _install(monkeypatch, callback)
+    request = {"messages": [{"role": "user", "content": "original"}]}
+
+    with caplog.at_level(logging.WARNING):
+        result = apply_llm_request_middleware(request)
+
+    assert result.payload == request
+    assert result.changed is False
+    assert result.trace == []
+    assert request["messages"][0]["content"] == "original"
+    assert "returned a request that could not be isolated transactionally" in caplog.text
+
+
+def test_transform_runs_before_common_provider_dispatch():
+    source = inspect.getsource(conversation_loop.run_conversation)
+
+    transform_at = source.index("apply_llm_request_middleware(")
+    observer_at = source.index('"pre_api_request"', transform_at)
+    dispatch_at = source.index("def _perform_api_call", transform_at)
+    execution_at = source.index("run_llm_execution_middleware(", dispatch_at)
+
+    assert transform_at < observer_at < dispatch_at < execution_at
+
+    dispatch_block = source[dispatch_at:execution_at]
+    assert 'agent.api_mode == "codex_responses"' in dispatch_block
+    assert "agent._interruptible_streaming_api_call(" in dispatch_block
+    assert "relay_llm.execute(" in dispatch_block
+    assert "agent._interruptible_api_call" in dispatch_block