Fix circuit breaker wedging after a non-retriable half-open probe (#3991)

* Fix circuit breaker wedging after a non-retriable half-open probe

When the circuit breaker is half-open it admits a single probe call by
setting `_circuit_probe_in_flight = True`. If that probe raised a
*non-retriable* error (e.g. quota/auth), the except block skipped both
`_record_failure()` (correct - business errors must not trip the breaker)
and any probe reset, so the circuit stayed `half_open` with
`_circuit_probe_in_flight = True` permanently. Every later call then
fast-failed in `_check_circuit()` forever, because no call could run the
handler to reach `_record_success` / `_record_failure`.

Release the probe on the non-retriable path (mirroring the existing
GraphBubbleUp handler) so the next call admits a fresh probe. The breaker
still never trips on non-retriable errors. Applied to both the sync and
async paths.

Adds sync + async regression tests asserting the probe is released and the
next `_check_circuit()` re-admits a probe.

* Address review: extract _release_half_open_probe helper

backend/packages/harness/deerflow/agents/middlewares/llm_error_handling_middleware.py

@@ -182,6 +182,17 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]):
                         self.circuit_recovery_timeout_sec,
                     )
 
+    def _release_half_open_probe(self) -> None:
+        """Release the in-flight half-open probe without recording a failure.
+
+        Used when something other than a classified success/failure consumes the probe (a
+        GraphBubbleUp control-flow signal, or a non-retriable error), so the circuit can admit
+        the next probe instead of fast-failing forever.
+        """
+        with self._circuit_lock:
+            if self._circuit_state == "half_open":
+                self._circuit_probe_in_flight = False
+
     def _classify_error(self, exc: BaseException) -> tuple[bool, str]:
         detail = _extract_error_detail(exc)
         lowered = detail.lower()
@@ -326,9 +337,7 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]):
                 return response
             except GraphBubbleUp:
                 # Preserve LangGraph control-flow signals (interrupt/pause/resume).
-                with self._circuit_lock:
-                    if self._circuit_state == "half_open":
-                        self._circuit_probe_in_flight = False
+                self._release_half_open_probe()
                 raise
             except Exception as exc:
                 retriable, reason = self._classify_error(exc)
@@ -354,6 +363,9 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]):
                 )
                 if retriable:
                     self._record_failure()
+                else:
+                    # Non-retriable: release the probe without recording a failure.
+                    self._release_half_open_probe()
                 return self._build_user_fallback_message(exc, reason)
 
     @override
@@ -378,9 +390,7 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]):
                 return response
             except GraphBubbleUp:
                 # Preserve LangGraph control-flow signals (interrupt/pause/resume).
-                with self._circuit_lock:
-                    if self._circuit_state == "half_open":
-                        self._circuit_probe_in_flight = False
+                self._release_half_open_probe()
                 raise
             except Exception as exc:
                 retriable, reason = self._classify_error(exc)
@@ -406,6 +416,9 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]):
                 )
                 if retriable:
                     self._record_failure()
+                else:
+                    # Non-retriable: release the probe without recording a failure.
+                    self._release_half_open_probe()
                 return self._build_user_fallback_message(exc, reason)
 
 

backend/tests/test_llm_error_handling_middleware.py

@@ -224,6 +224,73 @@ async def test_async_circuit_half_open_graph_bubble_up_resets_probe() -> None:
     assert middleware._circuit_state == "half_open"
 
 
+def test_circuit_half_open_non_retriable_error_resets_probe() -> None:
+    """A non-retriable error during a half-open probe must release the probe.
+
+    Regression: the non-retriable branch neither recorded a failure (correct —
+    business errors like quota/auth must not trip the breaker) nor reset
+    ``_circuit_probe_in_flight``. So one non-retriable probe left the circuit
+    stuck at half_open with probe_in_flight=True, and every subsequent call
+    fast-failed forever because no later call could ever run the handler to
+    reach ``_record_success`` / ``_record_failure``.
+    """
+    import unittest.mock
+
+    middleware = _build_middleware()
+
+    # Enter half_open and let one probe through (probe_in_flight -> True).
+    middleware._circuit_state = "half_open"
+    middleware._circuit_probe_in_flight = False
+    assert middleware._check_circuit() is False
+    assert middleware._circuit_probe_in_flight is True
+
+    def handler(_request) -> AIMessage:
+        raise FakeError("insufficient_quota", status_code=429, code="insufficient_quota")
+
+    # _check_circuit already admitted the probe above; keep it False here so the
+    # top-of-call gate does not fast-fail before the handler runs. Force the
+    # error to classify as non-retriable regardless of heuristics.
+    with unittest.mock.patch.object(middleware, "_check_circuit", return_value=False):
+        with unittest.mock.patch.object(middleware, "_classify_error", return_value=(False, "quota")):
+            result = middleware.wrap_model_call(SimpleNamespace(), handler)
+
+    # Non-retriable errors still surface a graceful fallback (not a raise) and
+    # must NOT trip the breaker.
+    assert isinstance(result, AIMessage)
+    assert middleware._circuit_state == "half_open"
+    # The probe was released, so the real gate re-admits the next probe instead
+    # of fast-failing forever.
+    assert middleware._circuit_probe_in_flight is False
+    assert middleware._check_circuit() is False
+    assert middleware._circuit_probe_in_flight is True
+
+
+@pytest.mark.anyio
+async def test_async_circuit_half_open_non_retriable_error_resets_probe() -> None:
+    """Async mirror: a non-retriable error during a half-open probe releases it."""
+    import unittest.mock
+
+    middleware = _build_middleware()
+
+    middleware._circuit_state = "half_open"
+    middleware._circuit_probe_in_flight = False
+    assert middleware._check_circuit() is False
+    assert middleware._circuit_probe_in_flight is True
+
+    async def handler(_request) -> AIMessage:
+        raise FakeError("insufficient_quota", status_code=429, code="insufficient_quota")
+
+    with unittest.mock.patch.object(middleware, "_check_circuit", return_value=False):
+        with unittest.mock.patch.object(middleware, "_classify_error", return_value=(False, "quota")):
+            result = await middleware.awrap_model_call(SimpleNamespace(), handler)
+
+    assert isinstance(result, AIMessage)
+    assert middleware._circuit_state == "half_open"
+    assert middleware._circuit_probe_in_flight is False
+    assert middleware._check_circuit() is False
+    assert middleware._circuit_probe_in_flight is True
+
+
 # ---------- Circuit Breaker Tests ----------