From b7933d18e43ee1de3a88e65e7c8661cfc0ddaee2 Mon Sep 17 00:00:00 2001 From: Aari Date: Thu, 23 Jul 2026 16:59:34 +0800 Subject: [PATCH] fix(safety): backfill empty content-filter responses so they don't poison the thread (#4394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An empty assistant message from a provider safety filter (content_filter with no content, no tool calls) was persisted into thread history and replayed to strict OpenAI-compatible providers, which reject it with HTTP 400 ("message ... with role 'assistant' must not be empty") — breaking every later turn until a new chat is started. SafetyFinishReasonMiddleware only handled the tool-call case (#3028) and TerminalResponseMiddleware only the post-tool case (#4027), so a plain empty content-filter response fell through both. Extend the safety middleware to backfill a user-facing explanation when a safety-terminated message is otherwise blank, so the persisted turn is non-empty (and the user sees why it was blocked). Fixes #4393 --- backend/AGENTS.md | 2 +- .../safety_finish_reason_middleware.py | 73 ++++++++++++---- ..._safety_finish_reason_graph_integration.py | 50 +++++++++++ .../test_safety_finish_reason_middleware.py | 87 +++++++++++++++++++ 4 files changed, 195 insertions(+), 17 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 146871502..ec7f9f985 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -346,7 +346,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc 30. **Custom middlewares** - *(optional)* Any `custom_middlewares` passed to `build_middlewares` are injected here, before config-declared extensions and the terminal-response/safety/clarification tail 31. **Configured extension middlewares** - *(optional, if `extensions.middlewares` is set in `config.yaml` or `extensions_config.json`)* Zero-argument `AgentMiddleware` classes loaded from `module.path:ClassName` entries via `deerflow.reflection.resolve_class`. Missing packages, invalid classes, and broken modules fail loudly at agent creation. These run after built-ins/programmatic custom middleware and after the lead/subagent loop/token guards, but before the terminal-response/safety/clarification tail; subagents receive the same configured extension middleware class list before their safety tail. Treat these files as trusted operator config because middleware paths instantiate arbitrary code. Gateway skill/MCP toggle endpoints preserve this field through `to_file_dict()` but must not add a write path for `extensions.middlewares` without an explicit trust-boundary review. Lead-only vs subagent-only middleware lists and per-context constructor parameters are not expressible in this MVP. 32. **TerminalResponseMiddleware** - When a provider returns an empty terminal `AIMessage` after tool execution, injects a hidden recovery prompt and retries the model once; a second empty response is replaced in checkpoint state by a visible error fallback marked for the run worker, so the run finishes as an error instead of a silent success -33. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Suppresses tool execution when the provider safety-terminated the response (e.g. `finish_reason=content_filter`); registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first +33. **SafetyFinishReasonMiddleware** - *(optional, if `safety_finish_reason.enabled`)* Repairs AIMessages the provider safety-terminated (e.g. `finish_reason=content_filter`): strips truncated tool calls so they are not executed (#3028), and — when the response is otherwise blank (no tool calls, no visible content) — backfills a user-facing explanation so the empty message is not persisted and then rejected by strict OpenAI-compatible providers on the next request (`message ... with role 'assistant' must not be empty`), which would otherwise strand the whole thread (#4393). A safety-terminated response that still carries visible text is left untouched. Registered after terminal-response/custom/configured middlewares so LangChain's reverse-order `after_model` dispatch runs it first 34. **ClarificationMiddleware** - Intercepts `ask_clarification` tool calls, writes a readable `ToolMessage.content` fallback plus structured `ToolMessage.artifact.human_input` request payload, and interrupts via `Command(goto=END)` (must be last). Because this middleware can short-circuit tool execution before LangChain emits `on_tool_end`, `RunJournal` performs a root-run final reconciliation for allowlisted clarification `ToolMessage`s whose `tool_call_id` was produced by the current run, so human-input request cards remain recoverable from `run_events` after checkpoint compaction. Human Input Card replies are submitted as `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden response sources (currently `ask_clarification`) as `llm.human.input`, which preserves answered-card state after compaction without exposing generic internal hidden context. ### Configuration System diff --git a/backend/packages/harness/deerflow/agents/middlewares/safety_finish_reason_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/safety_finish_reason_middleware.py index b215b9712..e3a3d00ef 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/safety_finish_reason_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/safety_finish_reason_middleware.py @@ -1,6 +1,8 @@ -"""Suppress tool execution when the provider safety-terminated the response. +"""Repair AIMessages the provider safety-terminated so they are neither +executed nor persisted empty. -Background — see issue bytedance/deer-flow#3028. +Background — see issues bytedance/deer-flow#3028 (truncated tool calls) and +#4393 (empty response poisons the thread). Some providers (OpenAI ``finish_reason='content_filter'``, Anthropic ``stop_reason='refusal'``, Gemini ``finish_reason='SAFETY'`` ...) can stop @@ -12,11 +14,22 @@ they were complete. The agent then sees the truncated file, tries to fix it, gets filtered again, and loops. This middleware sits at ``after_model`` and gates that behaviour: when a -configured ``SafetyTerminationDetector`` fires *and* the AIMessage carries -tool calls, we strip the tool calls (both structured and raw provider -payloads), append a user-facing explanation, and stash observability fields -in ``additional_kwargs.safety_termination`` so logs, traces, and SSE -consumers can see what happened. +configured ``SafetyTerminationDetector`` fires it either + +* strips the AIMessage's tool calls (both structured and raw provider + payloads) when it carries any — the truncated-tool-call case (#3028), or +* backfills a user-facing explanation when the message is otherwise blank + (no tool calls, no visible content) — the empty-response case (#4393), + where the empty assistant message would otherwise be persisted and then + rejected by strict OpenAI-compatible providers on every following request + ("message ... with role 'assistant' must not be empty"), stranding the + whole thread until a new chat is started. + +A safety-terminated message that carries visible text but no tool calls is +left untouched so its partial answer still reaches the user. Either way we +append the explanation and stash observability fields in +``additional_kwargs.safety_termination`` so logs, traces, and SSE consumers +can see what happened. Hook choice: ``after_model`` (not ``wrap_model_call``) because the response is a *normal* return — not an exception — and we want to participate in the @@ -48,6 +61,7 @@ from deerflow.agents.middlewares.safety_termination_detectors import ( default_detectors, ) from deerflow.agents.middlewares.tool_call_metadata import clone_ai_message_with_tool_calls +from deerflow.utils.messages import message_content_to_text if TYPE_CHECKING: from deerflow.config.safety_finish_reason_config import SafetyFinishReasonConfig @@ -63,9 +77,15 @@ _USER_FACING_MESSAGE = ( "or ask for a narrower output." ) +# Used when the safety termination produced no tool calls *and* no content: +# the message is rewritten only so it is not persisted empty (see #4393), so +# it must not claim any tool calls were suppressed. +_USER_FACING_EMPTY_MESSAGE = "The model provider stopped this response with a safety-related signal ({reason_field}={reason_value!r}, detector={detector!r}) and returned no content. Please rephrase your request or start a new conversation." + class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): - """Strip tool_calls from AIMessages flagged by a SafetyTerminationDetector.""" + """Repair AIMessages flagged by a SafetyTerminationDetector: strip tool + calls, or backfill an explanation when the message is otherwise empty.""" def __init__(self, detectors: list[SafetyTerminationDetector] | None = None) -> None: super().__init__() @@ -135,8 +155,10 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): message: AIMessage, termination: SafetyTermination, ) -> AIMessage: - suppressed_names = [tc.get("name") or "unknown" for tc in (message.tool_calls or [])] - explanation = _USER_FACING_MESSAGE.format( + tool_calls = message.tool_calls or [] + suppressed_names = [tc.get("name") or "unknown" for tc in tool_calls] + template = _USER_FACING_MESSAGE if tool_calls else _USER_FACING_EMPTY_MESSAGE + explanation = template.format( reason_field=termination.reason_field, reason_value=termination.reason_value, detector=termination.detector, @@ -272,17 +294,34 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): if not isinstance(last, AIMessage): return None - # Issue scope: only intervene when there's something to suppress. - # ``content_filter`` without tool_calls is allowed through unchanged - # so the partial text response (if any) reaches the user naturally. - tool_calls = last.tool_calls - if not tool_calls: + # Two provider-safety failure modes are worth rewriting; a safety + # termination that produced visible text with no tool calls is left + # untouched so the partial answer still reaches the user naturally. + # 1. tool_calls present: they may be truncated/unsafe (#3028), so + # suppress them. + # 2. blank content and no tool_calls: an empty assistant message + # that strict OpenAI-compatible providers (Moonshot/Kimi, ...) + # reject on the *next* request ("message ... with role + # 'assistant' must not be empty", #4393), which poisons the whole + # thread until a new chat is started. Backfill an explanation so + # the persisted message is non-empty. + tool_calls = list(last.tool_calls or []) + # ``or ""`` normalizes every "no visible content" shape to blank: + # None, "", [] and whitespace all count. None is reachable via + # ``model_copy(update={"content": None})`` (a rewrite path that skips + # validation); without the guard message_content_to_text stringifies + # it to "None" and the backfill would be skipped, re-poisoning the + # thread this fix is meant to protect. + content_is_blank = not message_content_to_text(last.content or "").strip() + if not tool_calls and not content_is_blank: return None termination = self._detect(last) if termination is None: return None + backfilled_empty = content_is_blank and not tool_calls + # Stamp stop_reason so the worker can surface this capped completion # alongside loop_capped / token_capped (#4176). ctx = getattr(runtime, "context", None) @@ -295,14 +334,16 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): thread_id = runtime.context.get("thread_id") if isinstance(runtime.context, dict) else None logger.warning( - "Provider safety termination detected — suppressed %d tool call(s)", + "Provider safety termination detected — suppressed %d tool call(s), backfilled_empty_content=%s", len(tool_calls), + backfilled_empty, extra={ "thread_id": thread_id, "detector": termination.detector, "reason_field": termination.reason_field, "reason_value": termination.reason_value, "suppressed_tool_call_names": [tc.get("name") for tc in tool_calls], + "backfilled_empty_content": backfilled_empty, }, ) diff --git a/backend/tests/test_safety_finish_reason_graph_integration.py b/backend/tests/test_safety_finish_reason_graph_integration.py index f26a7be90..5695084c8 100644 --- a/backend/tests/test_safety_finish_reason_graph_integration.py +++ b/backend/tests/test_safety_finish_reason_graph_integration.py @@ -179,6 +179,56 @@ def test_content_filter_without_tool_calls_passes_through_unchanged(): assert _TOOL_INVOCATIONS == [] +def test_content_filter_empty_no_tool_calls_is_backfilled_not_persisted_empty(): + """#4393: a content_filter response with empty content and no tool calls + must not survive in graph state as an empty AIMessage — strict + OpenAI-compatible providers reject an empty assistant message on the next + request, poisoning the whole thread. The middleware backfills a + user-facing explanation so the persisted message is non-empty.""" + _TOOL_INVOCATIONS.clear() + + class _EmptyContentFilterModel(BaseChatModel): + """Mimics Kimi/Moonshot refusing a sensitive question: an empty + assistant message flagged finish_reason='content_filter'.""" + + @property + def _llm_type(self) -> str: + return "fake-empty-content-filter" + + def bind_tools(self, tools, **kwargs): + return self + + def _generate(self, messages, stop=None, run_manager=None, **kwargs): + msg = AIMessage(content="", response_metadata={"finish_reason": "content_filter"}) + return ChatResult(generations=[ChatGeneration(message=msg)]) + + async def _agenerate(self, messages, stop=None, run_manager=None, **kwargs): + return self._generate(messages, stop=stop, run_manager=run_manager, **kwargs) + + agent = create_agent( + model=_EmptyContentFilterModel(), + tools=[write_file], + middleware=[SafetyFinishReasonMiddleware()], + ) + result = agent.invoke({"messages": [HumanMessage(content="a sensitive question")]}) + final_ai = next(m for m in reversed(result["messages"]) if isinstance(m, AIMessage)) + + # The poison condition: the persisted assistant message must not be empty. + assert isinstance(final_ai.content, str) + assert final_ai.content.strip(), "empty assistant message would be rejected by strict providers on the next turn" + assert "safety-related signal" in final_ai.content + assert "returned no content" in final_ai.content + + # Observability stamp present with zero suppressed tool calls. + record = final_ai.additional_kwargs.get("safety_termination") + assert record is not None + assert record["suppressed_tool_call_count"] == 0 + + # Real provider reason preserved for downstream SSE / converters. + assert final_ai.response_metadata.get("finish_reason") == "content_filter" + assert _TOOL_INVOCATIONS == [] + + def test_normal_tool_call_round_trip_is_not_affected(): """Regression: a healthy finish_reason='tool_calls' response must still execute the tool. The middleware must not over-fire.""" diff --git a/backend/tests/test_safety_finish_reason_middleware.py b/backend/tests/test_safety_finish_reason_middleware.py index 14c6226dd..cbffccca2 100644 --- a/backend/tests/test_safety_finish_reason_middleware.py +++ b/backend/tests/test_safety_finish_reason_middleware.py @@ -113,6 +113,93 @@ class TestTriggerCriteria: } assert mw._apply(state, _runtime()) is None + def test_content_filter_blank_content_no_tool_calls_backfills(self): + """#4393: an empty content_filter response with no tool calls would be + persisted empty and rejected by strict providers on the next request. + Backfill an explanation so the persisted message is non-empty.""" + mw = SafetyFinishReasonMiddleware() + state = { + "messages": [ + _ai( + content="", + response_metadata={"finish_reason": "content_filter"}, + ) + ] + } + result = mw._apply(state, _runtime()) + assert result is not None + patched = result["messages"][0] + assert patched.tool_calls == [] + assert isinstance(patched.content, str) + assert patched.content.strip() # never persisted empty + assert "safety-related signal" in patched.content + assert "returned no content" in patched.content + # It must not claim tool calls were suppressed — none existed. + assert "were suppressed" not in patched.content + record = patched.additional_kwargs["safety_termination"] + assert record["suppressed_tool_call_count"] == 0 + assert record["suppressed_tool_call_names"] == [] + + def test_content_filter_whitespace_content_no_tool_calls_backfills(self): + """Whitespace-only content is still blank to a strict provider.""" + mw = SafetyFinishReasonMiddleware() + state = { + "messages": [ + _ai( + content=" \n ", + response_metadata={"finish_reason": "content_filter"}, + ) + ] + } + result = mw._apply(state, _runtime()) + assert result is not None + patched = result["messages"][0] + assert patched.tool_calls == [] + assert "returned no content" in patched.content + + def test_content_filter_none_content_no_tool_calls_backfills(self): + """content=None is reachable via model_copy rewrites (which skip + validation) and must be treated as blank, not stringified to 'None'.""" + mw = SafetyFinishReasonMiddleware() + none_content = _ai(response_metadata={"finish_reason": "content_filter"}).model_copy(update={"content": None}) + assert none_content.content is None # precondition for the regression + result = mw._apply({"messages": [none_content]}, _runtime()) + assert result is not None + patched = result["messages"][0] + assert patched.tool_calls == [] + assert isinstance(patched.content, str) + assert patched.content.strip() + assert "returned no content" in patched.content + + def test_anthropic_refusal_blank_content_no_tool_calls_backfills(self): + """The empty-content backfill is detector-agnostic (#4393).""" + mw = SafetyFinishReasonMiddleware() + state = { + "messages": [ + _ai( + content="", + response_metadata={"stop_reason": "refusal"}, + ) + ] + } + result = mw._apply(state, _runtime()) + assert result is not None + assert result["messages"][0].content.strip() + + def test_blank_content_no_tool_calls_without_safety_signal_passes_through(self): + """A blank response with no safety signal is out of scope: only a + detected safety termination triggers the backfill.""" + mw = SafetyFinishReasonMiddleware() + state = { + "messages": [ + _ai( + content="", + response_metadata={"finish_reason": "stop"}, + ) + ] + } + assert mw._apply(state, _runtime()) is None + def test_normal_tool_calls_pass_through(self): mw = SafetyFinishReasonMiddleware() state = {