From 372baf762f69ca05a8bc6de843dea10252ca40f0 Mon Sep 17 00:00:00 2001 From: golemon Date: Sat, 7 Nov 2026 21:43:16 +0800 Subject: [PATCH] fix(agent): prevent todo reminders from resuming runs after hard stops (#7955) * fix(agent): stop todo reminders after run hard limits Respect loop_capped and token_capped before scheduling completion reminders, and include the behavior in the assembly policy fingerprint. Cover sync and async agent graphs, blocked post-stop tools, preserved Todo state, and continuation from the same thread checkpoint. Validation: 316 targeted tests passed. * docs(agent): explain todo behavior after hard stops Document that loop and token hard stops suppress Todo completion reminders while retaining unfinished tasks for later runs. * docs(changelog): record todo hard-stop fix Add English and Chinese Unreleased entries referencing issue #7953. * fix(agent): mark safety stops with partial responses * fix(agent): honor provider stops before todo retries * docs(agent): clarify todo behavior after provider stops * test(agent): isolate todo safety-chain runtime dependencies --------- Co-authored-by: Willem Jiang Co-authored-by: Totoro --- CHANGELOG.md | 6 + CHANGELOG_zh.md | 5 + README.md | 4 + .../deerflow/agents/middlewares/AGENTS.md | 4 +- .../safety_finish_reason_middleware.py | 38 +-- .../agents/middlewares/todo_middleware.py | 23 +- backend/tests/AGENTS.md | 5 + .../tests/test_middleware_release_policy.py | 22 ++ ..._safety_finish_reason_graph_integration.py | 8 +- .../test_safety_finish_reason_middleware.py | 42 ++- backend/tests/test_safety_responses.py | 15 +- backend/tests/test_todo_middleware.py | 255 ++++++++++++++++++ 12 files changed, 379 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bd39dece6..d59e01e144 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,11 @@ This section accumulates work toward the **2.3.0** milestone ### Fixed +- **agents:** Unfinished todos no longer trigger additional model calls or + tool execution after a loop-detection, run token-budget, provider-safety, or + model output-length stop. Todo state is preserved so users can continue in a + later run. ([#7953]) + - **channels:** Long Feishu/Lark answers are no longer lost. Card text was unbounded and ASCII-escaped, so an answer of about 4,400 CJK characters made the card request exceed Feishu's 30 KB limit (`230025`); the final patch and @@ -16065,3 +16070,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag. [#7831]: https://github.com/bytedance/deer-flow/pull/7831 [#7892]: https://github.com/bytedance/deer-flow/pull/7892 [#7907]: https://github.com/bytedance/deer-flow/pull/7907 +[#7953]: https://github.com/bytedance/deer-flow/issues/7953 diff --git a/CHANGELOG_zh.md b/CHANGELOG_zh.md index 910021ac80..e4ce499844 100644 --- a/CHANGELOG_zh.md +++ b/CHANGELOG_zh.md @@ -23,6 +23,10 @@ ### 修复 +- **智能体：** 循环检测、运行 token 预算、模型服务安全限制或输出长度限制触发停止后， + 未完成的 Todo 不再触发额外的模型调用或工具执行。Todo 状态仍会保留， + 用户可在后续运行中继续处理。([#7953]) + - **渠道：** 飞书/Lark 的长回答不再丢失。此前卡片文本没有长度上限，并以 ASCII 转义 序列化，约 4,400 个中日韩字符就会让卡片请求超过飞书 30 KB 的限制（`230025`）： 最终更新及其回复兜底都会被拒绝，运行中卡片停留在最后一次流式内容或只剩 `▉`。 @@ -12780,3 +12784,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写，核心包含子 [#7798]: https://github.com/bytedance/deer-flow/pull/7798 [#7831]: https://github.com/bytedance/deer-flow/pull/7831 [#7782]: https://github.com/bytedance/deer-flow/pull/7782 +[#7953]: https://github.com/bytedance/deer-flow/issues/7953 diff --git a/README.md b/README.md index 9dd91cb1e1..40e801fb55 100644 --- a/README.md +++ b/README.md @@ -2728,6 +2728,10 @@ call. In planning mode, if the original `write_todos` call is no longer visible, DeerFlow adds a reminder using the latest task statuses before the next model call. Skipped or failed compaction leaves the existing messages unchanged. +When loop detection, the run token budget, or a provider safety/output-length +limit stops a run, unfinished todos are retained for a later user run; todo +completion reminders do not restart model calls or tool execution after that stop. +Safety-stopped partial text is preserved; it does not trigger a Todo retry. Optional `pii_redaction.enabled` redacts detected identifiers in user messages, remote tool results, compaction input, reinjected summaries, configured diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index 265a2d3b1b..97c32f0daa 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -150,7 +150,7 @@ Before changing a later authorization phase, read the [authorization RFC](../../ 18. **SkillActivationMiddleware** - Detects strict `/skill-name task` syntax on the latest real user message, resolves only enabled and runtime-allowed skills, injects the `SKILL.md` body as hidden current-turn context, and records a `middleware:skill_activation` audit event; a retried activation call replays the recorded reminder without re-auditing. Skill `allowed-tools` is compatibility metadata only: activation never filters, grants, or denies tools — Agent assembly/execution authorization owns the permission ceiling. 19. **DurableContextMiddleware** - Before compaction, captures task dispatch/result summaries in `ThreadState.delegations` and loaded skill name/path/description (never bodies) in `ThreadState.skill_context`; projects them per request. Static authority uses `SystemMessage`; untrusted `summary_text`, delegation results and skill descriptions use a separate hidden `HumanMessage` data block, never persisted as messages or promoted to system instructions. Subagents attach it immediately before summarization: summary precedes preserved AI/tool history for strict providers. See [tool artifacts](TOOL_ARTIFACTS.md) for projection, retry limits and delegation scope. 20. **SummarizationMiddleware** - *(optional)* Compacts near token limits. Fraction trigger/keep profiles belong to the active run model; an explicit summary model only generates. Memory flush/manual compaction follow runtime/checkpoint agent policy. Preserve the latest user request and DynamicContext reminders; a backward cutoff may retain old AI/tool turns or make first-turn compaction a no-op. -21. **TodoListMiddleware** - *(optional, if `is_plan_mode`)* Task tracking with `write_todos`. Skips reminders on `model_length_termination` so capped turns end cleanly. A raising model call requeues its reminder, uncounted, for the retry. Both hooks read stored todos through `_usable_todos`: thread-state updates and direct integrations can Overwrite that channel, and a non-mapping entry would otherwise raise on every model call. +21. **TodoListMiddleware** - *(optional, if `is_plan_mode`)* Task tracking with `write_todos`. Before the parent hook, completion reminders respect loop/token/safety/model-length `stop_reason` values; provider termination markers remain fallbacks. Unfinished todos survive for later runs. A raising model call requeues its reminder, uncounted, for the retry. Both hooks read stored todos through `_usable_todos`: thread-state updates and direct integrations can Overwrite that channel, and a non-mapping entry would otherwise raise on every model call. 22. **TokenUsageMiddleware** - *(optional, if `token_usage.enabled`)* Records token usage metrics; subagent usage is read from terminal `ToolMessage.additional_kwargs` in the current run and merged back into the dispatching AIMessage by message position. The same state update marks the ToolMessage with `subagent_token_usage_attributed=true`, so checkpoint replay or middleware re-entry cannot add the cumulative snapshot twice; missing/malformed usage or a result with no matching dispatch remains unmarked and retryable. TODO attribution skips non-string statuses before membership; preserve args/TODO state for normal tool validation. Attribution normalizes the stored `todos` channel with `_normalize_todos` before reading entries, the same tolerance the todo middleware applies. 23. **TitleMiddleware** - Auto-generates the thread title after the first complete exchange and normalizes structured message content before prompting the title model. If a first-turn run is interrupted before this middleware can write a title, `runtime/runs/worker.py` keeps the run in a finalizing state, persists a local fallback title from the latest checkpoint or original run input, and then syncs it to `threads_meta.display_name`. Replacement runs admitted by `multitask_strategy="interrupt"` / `"rollback"` wait for older same-thread finalization before entering the graph; the interrupted run only skips the fallback title write once a later run has started and may have advanced the checkpoint. @@ -174,7 +174,7 @@ Before changing a later authorization phase, read the [authorization RFC](../../ 35. **Configured extension middlewares** - `extensions.middlewares` in `config.yaml` or `extensions_config.json` optionally accepts `module.path:ClassName` strings or `{class, kwargs, apply_to}` objects. `deerflow.reflection.resolve_class` loads `AgentMiddleware` classes; import, class, and constructor errors fail agent creation. `kwargs` must be JSON-compatible; YAML dates/timestamps become ISO strings. Order: built-ins/custom and loop/token guards → extensions → terminal-response/safety/clarification tail. Optional `apply_to` selects both chains (default), lead, or subagent before import/construction. Trusted operator config only: paths instantiate arbitrary code. Gateway skill/MCP toggles preserve it in raw JSON; adding an API write path requires explicit trust-boundary review. 36. **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 37. **ModelLengthFinishReasonMiddleware** - Responses: same-container `status=incomplete` + `incomplete_details.reason=max_output_tokens`. Suppress calls via shared cloning after clearing invalid calls; keep non-call blocks/metadata. Stamp every cap; visible text-only caps get no notice. Add a notice only for suppressed calls or no visible content. Also registered on subagent chains (before Safety), where `consume_stop_reason` surfaces `model_length_capped` to the executor. -38. **SafetyFinishReasonMiddleware** - `safety_finish_reason.enabled`: suppresses valid/invalid safety-stopped calls. Responses: `status=incomplete` + `incomplete_details.reason=content_filter`. Clear invalid calls before shared cloning. Place after terminal/custom/configured hooks; `after_model` runs in reverse. +38. **SafetyFinishReasonMiddleware** - `safety_finish_reason.enabled`: marks all safety stops, preserving partial text and suppressing valid/invalid calls. Responses: `status=incomplete` + `incomplete_details.reason=content_filter`. Clear invalid calls before shared cloning. Place after terminal/custom/configured hooks; `after_model` runs in reverse. 39. **ClarificationMiddleware** - Intercepts `ask_clarification`, writes a readable `ToolMessage.content` fallback plus a structured `ToolMessage.artifact.human_input` payload, and interrupts via `Command(goto=END)` (must be last). `after_model` drops same-turn sibling tool calls so they cannot run before the user answers; a malformed `ask_clarification` parked on `invalid_tool_calls` is the same stop signal. `disable_clarification` runs keep the siblings. Payloads are versioned — legacy `free_text`/`choice_with_other` stay `version: 1`; the v2 `form` mode (from `fields`) is `version: 2` so older frontends reject it and fall back to plain text. Field normalization is deterministic and lives in the middleware (it short-circuits before tool execution, so tool-arg typing gives no runtime validation), and it is atomic: any structurally broken entry — non-dict, bad/duplicate name, a name colliding with a JS `Object.prototype` member (`__proto__`/`constructor`), or exceeding the caps (16 fields / 24 options per field / 200 chars per text / `MAX_FORM_SERIALIZED_BYTES` = 16KB UTF-8, the per-item caps alone admitting forms whose IM text fallback overruns channel limits) — degrades the whole form to the legacy option/free-text modes, so a card never renders "complete" while missing a field. Benign issues degrade locally (unknown types — incl. unhashable JSON like `type: []` — and option-less selects become `text`); options are trimmed/deduped with blanks dropped (form- and top-level) since the frontend rejects blank labels. XML-to-dict option payloads are recursively flattened from dict/list containers in source order, scalar leaves kept, residual XML tags stripped before that trimming. Checkboxes are booleans defaulting to "no"; `required` on one means consent semantics. The response protocol is unchanged (v1 `text`/`option`): form cards submit a text summary as `response_kind: "text"`, so journal persistence needs no new allowlist entries. `RunJournal` reconciles visible, unpersisted `ToolMessage`s for current-run lead-agent calls at the next lead-agent `on_chat_model_start` after consumption, or at successful root `on_chain_end`. This preserves any middleware short-circuit, including blocked writes, across later model errors and checkpoint compaction (#4666). `_remember_current_run_tool_calls` excludes subagents; their results stay in `subagent.step`. Human Input Card replies are `hide_from_ui` `HumanMessage`s with `additional_kwargs.human_input_response`; `RunJournal` persists only allowlisted hidden sources (currently `ask_clarification`) as `llm.human.input`. Inline `skill_references` accepts 1–16 names; empty lists retain slash/plain flow. 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 65782bd977..807e2a6509 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 @@ -25,11 +25,10 @@ configured ``SafetyTerminationDetector`` fires it either ("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. +A safety-terminated message that carries visible text but no tool calls keeps +its content without an explanation. Every detected termination stamps +``runtime.context.stop_reason`` and ``additional_kwargs.safety_termination`` +so downstream hooks cannot mistake the partial answer for a premature exit. 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 @@ -124,6 +123,7 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): detectors.append(descriptor) return { "action": "suppress_tool_calls", + "mark_text_only_termination": True, "detectors": detectors, } @@ -192,13 +192,15 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): ) -> AIMessage: tool_calls = [*(message.tool_calls or []), *(message.invalid_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, - ) - new_content = self._append_user_message(message.content, explanation) + new_content = message.content + if tool_calls or not message_content_to_text(message.content or "").strip(): + 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, + ) + new_content = self._append_user_message(message.content, explanation) # clone_ai_message_with_tool_calls handles structured tool_calls, # raw additional_kwargs.tool_calls, function_call, and provider @@ -365,9 +367,8 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): if not isinstance(last, AIMessage): return None - # 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. + # All provider-safety terminations need a stop marker. Only two cases + # need a visible explanation; partial text otherwise stays unchanged. # 1. tool_calls present: they may be truncated/unsafe (#3028), so # suppress them. # 2. blank content and no tool_calls: an empty assistant message @@ -381,9 +382,6 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): # shared helper also handles None from validation-skipping rewrites. # The trailing strip() makes whitespace-only content blank as well. 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 @@ -395,6 +393,10 @@ class SafetyFinishReasonMiddleware(AgentMiddleware[AgentState]): ctx = getattr(runtime, "context", None) if isinstance(ctx, dict): ctx["stop_reason"] = "safety_capped" + # Re-observing a terminal message must not replace its original + # suppression record or emit another event. Still stamp this runtime. + if not tool_calls and not content_is_blank and (last.additional_kwargs or {}).get("safety_termination"): + return None patched = self._build_suppressed_message(last, termination) thread_id = None diff --git a/backend/packages/harness/deerflow/agents/middlewares/todo_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/todo_middleware.py index e5a5803f70..d2d17b0887 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/todo_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/todo_middleware.py @@ -15,6 +15,8 @@ of being persisted into graph state as a normal user-visible message. The completion guard defers to ``model_length_termination``: when a length-capped turn has already been terminalized by ``ModelLengthFinishReasonMiddleware``, re-engaging would only re-emit the same oversized tool call into the same cap. +Run-level loop, token, provider-safety and model-length stops also suppress +completion reminders, leaving unfinished todos available for a later user run. """ from __future__ import annotations @@ -31,6 +33,9 @@ from langgraph.runtime import Runtime from deerflow.agents.thread_state import ThreadState TODO_REMINDER_MESSAGE_NAME = "todo_reminder" +# Runtime Goal continuation has a narrower policy; keep this local to Todo. +# Source-inventory and graph tests pin these values to the actual guard stamps. +_COMPLETION_REMINDER_HARD_STOP_REASONS = ("loop_capped", "token_capped", "safety_capped", "model_length_capped") def _todos_in_messages(messages: list[Any]) -> bool: @@ -154,6 +159,8 @@ class TodoMiddleware(TodoListMiddleware): "tool_description_hash": canonical_hash(self.tool_description), "state_channel": "todos", "skip_completion_reminder_on_length_cap": True, + "skip_completion_reminder_on_safety_cap": True, + "completion_reminder_hard_stop_reasons": list(_COMPLETION_REMINDER_HARD_STOP_REASONS), } @override @@ -347,6 +354,13 @@ class TodoMiddleware(TodoListMiddleware): # no jump_to — the model cannot update todos through an unbound tool. return None + # Guards run before this reversed after_model hook. They strip tool + # calls and stamp loop/token/safety/model-length stops. Check before + # the parent hook so its parallel-write retry cannot override a stop. + context = getattr(runtime, "context", None) + if isinstance(context, dict) and context.get("stop_reason") in _COMPLETION_REMINDER_HARD_STOP_REASONS: + return None + # 1. Preserve base class logic (parallel write_todos detection). base_result = super().after_model(state, runtime) if base_result is not None: @@ -363,11 +377,10 @@ class TodoMiddleware(TodoListMiddleware): if (last_ai.additional_kwargs or {}).get("deerflow_error_fallback"): return None - # A length-capped turn was already terminalized by - # ModelLengthFinishReasonMiddleware (tool calls suppressed, notice - # appended); re-engaging would only re-emit the same oversized tool - # call into the same cap. - if (last_ai.additional_kwargs or {}).get("model_length_termination"): + # Provider caps also mark the message: preserve this fallback when + # runtime context is unavailable or another reason already occupies it. + termination_metadata = last_ai.additional_kwargs or {} + if termination_metadata.get("model_length_termination") or termination_metadata.get("safety_termination"): return None # 3. Allow exit when all todos are completed or there are no todos. diff --git a/backend/tests/AGENTS.md b/backend/tests/AGENTS.md index df71580db6..e6f8c9d196 100644 --- a/backend/tests/AGENTS.md +++ b/backend/tests/AGENTS.md @@ -26,6 +26,11 @@ ownership and capture cleanup. Retain the real XLSX conversion coverage. Backend tests must preserve the runtime invariants they exercise without changing production execution topology. +Todo production-chain regressions bind their explicit config through the runtime +AppConfig context, isolate sandbox/path singletons, and clean up the test provider. +Keep the missing ambient config path so a developer's `config.yaml` cannot mask +an invocation-time dependency; retain the real sandbox and middleware chain. + `test_threads_router.py::test_goal_mutations_reject_run_owned_by_another_worker` feeds real PUT/DELETE 409 errors into the IM classifier. Keep this contract check so Gateway wording changes cannot silently remove localized busy feedback. diff --git a/backend/tests/test_middleware_release_policy.py b/backend/tests/test_middleware_release_policy.py index 2c8e1e1075..b70e2363c3 100644 --- a/backend/tests/test_middleware_release_policy.py +++ b/backend/tests/test_middleware_release_policy.py @@ -303,6 +303,28 @@ def _middleware_fingerprint(middleware): ).fingerprint +def test_todo_hard_stop_policy_changes_assembly_identity(monkeypatch): + from deerflow.agents.middlewares.todo_middleware import TodoMiddleware + + current = TodoMiddleware() + previous = TodoMiddleware() + previous_params = previous.release_policy_parameters() + assert previous_params["completion_reminder_hard_stop_reasons"] == ["loop_capped", "token_capped", "safety_capped", "model_length_capped"] + previous_params["completion_reminder_hard_stop_reasons"] = ["loop_capped", "token_capped"] + assert previous_params.pop("skip_completion_reminder_on_safety_cap") is True + monkeypatch.setattr(previous, "release_policy_parameters", lambda: previous_params) + assert _middleware_fingerprint(current) != _middleware_fingerprint(previous) + + +def test_safety_text_only_stop_policy_changes_assembly_identity(monkeypatch): + current = _make_safety_finish_reason_middleware() + previous = _make_safety_finish_reason_middleware() + previous_params = previous.release_policy_parameters() + assert previous_params.pop("mark_text_only_termination") is True + monkeypatch.setattr(previous, "release_policy_parameters", lambda: previous_params) + assert _middleware_fingerprint(current) != _middleware_fingerprint(previous) + + def _continuity_summarizer(config): from deerflow.agents.middlewares.summarization_middleware import DeerFlowSummarizationMiddleware diff --git a/backend/tests/test_safety_finish_reason_graph_integration.py b/backend/tests/test_safety_finish_reason_graph_integration.py index d13b512302..7af41c822d 100644 --- a/backend/tests/test_safety_finish_reason_graph_integration.py +++ b/backend/tests/test_safety_finish_reason_graph_integration.py @@ -177,9 +177,8 @@ async def test_safety_termination_event_reaches_astream_events(): assert _TOOL_INVOCATIONS == [] -def test_content_filter_without_tool_calls_passes_through_unchanged(): - """No tool calls => issue scope says don't intervene; the partial - response should be delivered as-is so the user sees what they got.""" +def test_content_filter_without_tool_calls_preserves_text_and_marks_termination(): + """Keep the partial response visible while telling later hooks to stop.""" _TOOL_INVOCATIONS.clear() class _NoToolModel(BaseChatModel): @@ -210,8 +209,7 @@ def test_content_filter_without_tool_calls_passes_through_unchanged(): # Content untouched. assert final_ai.content == "Partial answer truncated by safety filter" - # No safety_termination stamp because we didn't intervene. - assert "safety_termination" not in final_ai.additional_kwargs + assert final_ai.additional_kwargs["safety_termination"]["suppressed_tool_call_count"] == 0 # tool node never ran (there were no tool calls in the first place). assert _TOOL_INVOCATIONS == [] diff --git a/backend/tests/test_safety_finish_reason_middleware.py b/backend/tests/test_safety_finish_reason_middleware.py index eb36e72643..c7f9335f90 100644 --- a/backend/tests/test_safety_finish_reason_middleware.py +++ b/backend/tests/test_safety_finish_reason_middleware.py @@ -1,5 +1,6 @@ """Unit tests for SafetyFinishReasonMiddleware.""" +import asyncio from unittest.mock import MagicMock import pytest @@ -100,19 +101,34 @@ class TestTriggerCriteria: patched = result["messages"][0] assert patched.tool_calls == [] - def test_content_filter_without_tool_calls_passes_through(self): - """issue scope: when there are no tool calls the partial text is a - legitimate final response and should not be rewritten.""" + @pytest.mark.parametrize("invoke_async", [False, True], ids=["sync", "async"]) + @pytest.mark.parametrize("content", ["partial response", [{"type": "text", "text": "partial response"}]]) + def test_content_filter_without_tool_calls_preserves_text_and_stamps_stop(self, invoke_async, content): + """Partial text stays visible, but downstream hooks must see the stop.""" mw = SafetyFinishReasonMiddleware() - state = { - "messages": [ - _ai( - content="partial response", - response_metadata={"finish_reason": "content_filter"}, - ) - ] - } - assert mw._apply(state, _runtime()) is None + message = _ai(content=content, response_metadata={"finish_reason": "content_filter"}, additional_kwargs={"reasoning": "original"}) + message.id = "partial-safety-response" + runtime = _runtime() + state = {"messages": [message]} + + result = asyncio.run(mw.aafter_model(state, runtime)) if invoke_async else mw.after_model(state, runtime) + + assert result is not None + patched = result["messages"][0] + assert patched.content == content + assert patched.id == message.id + assert patched.response_metadata == message.response_metadata + assert patched.additional_kwargs["reasoning"] == "original" + assert patched.additional_kwargs["safety_termination"]["suppressed_tool_call_count"] == 0 + assert runtime.context["stop_reason"] == "safety_capped" + assert "safety_termination" not in message.additional_kwargs + assert message.content == content + + next_runtime = _runtime() + second_state = {"messages": [patched]} + second = asyncio.run(mw.aafter_model(second_state, next_runtime)) if invoke_async else mw.after_model(second_state, next_runtime) + assert second is None + assert next_runtime.context["stop_reason"] == "safety_capped" def test_content_filter_blank_content_no_tool_calls_backfills(self): """#4393: an empty content_filter response with no tool calls would be @@ -445,7 +461,7 @@ class TestMessageRewrite: def test_idempotent_on_already_cleared_message(self): # Re-running the middleware on a message we already cleared must not - # re-trigger (tool_calls is now empty → fast passthrough). + # re-trigger or replace the original suppression record. mw = SafetyFinishReasonMiddleware() state = { "messages": [ diff --git a/backend/tests/test_safety_responses.py b/backend/tests/test_safety_responses.py index 47159c9643..8afedd590b 100644 --- a/backend/tests/test_safety_responses.py +++ b/backend/tests/test_safety_responses.py @@ -153,14 +153,19 @@ def test_responses_signal_not_combined_across_containers(): @pytest.mark.parametrize("content", ["", "Partial answer"]) -def test_filtered_text_keeps_existing_empty_and_visible_behavior(content): +def test_filtered_text_marks_stop_and_preserves_visible_content(content): message = AIMessage(content=content, response_metadata={"status": "incomplete", "incomplete_details": {"reason": "content_filter"}}) - update = SafetyFinishReasonMiddleware().after_model({"messages": [message]}, SimpleNamespace(context={})) + runtime = SimpleNamespace(context={}) + update = SafetyFinishReasonMiddleware().after_model({"messages": [message]}, runtime) + assert update is not None + patched = update["messages"][0] + assert runtime.context["stop_reason"] == "safety_capped" + assert patched.additional_kwargs["safety_termination"]["suppressed_tool_call_count"] == 0 + assert patched.response_metadata == message.response_metadata if content: - assert update is None + assert patched.content == content else: - assert update is not None - assert "returned no content" in update["messages"][0].content + assert "returned no content" in patched.content @pytest.mark.parametrize("native", [False, True]) diff --git a/backend/tests/test_todo_middleware.py b/backend/tests/test_todo_middleware.py index 4d4ab4c32b..d412dc4cd2 100644 --- a/backend/tests/test_todo_middleware.py +++ b/backend/tests/test_todo_middleware.py @@ -1,20 +1,26 @@ """Tests for TodoMiddleware context-loss detection.""" +import ast import asyncio +from pathlib import Path from types import SimpleNamespace from typing import Any from unittest.mock import MagicMock import pytest from langchain.agents import create_agent +from langchain.agents.middleware import TodoListMiddleware from langchain.agents.middleware.types import ModelRequest +from langchain.tools import tool from langchain_core.language_models.fake_chat_models import FakeMessagesListChatModel from langchain_core.messages import AIMessage, HumanMessage, SystemMessage +from langgraph.checkpoint.memory import InMemorySaver from pydantic import PrivateAttr from deerflow.agents.middlewares.model_length_finish_reason_middleware import ( ModelLengthFinishReasonMiddleware, ) +from deerflow.agents.middlewares.safety_finish_reason_middleware import SafetyFinishReasonMiddleware from deerflow.agents.middlewares.todo_middleware import ( TODO_REMINDER_MESSAGE_NAME, TodoMiddleware, @@ -306,6 +312,89 @@ class TestToolCallIntentOrError: class TestAfterModel: + def test_all_middleware_stop_reason_writes_have_an_explicit_todo_policy(self): + middleware_dir = Path(__file__).resolve().parents[1] / "packages/harness/deerflow/agents/middlewares" + stamped_reasons = set() + for source in sorted(middleware_dir.glob("*.py")): + tree = ast.parse(source.read_text(encoding="utf-8")) + constants = {target.id: node.value for node in tree.body if isinstance(node, ast.Assign) for target in node.targets if isinstance(target, ast.Name) and isinstance(node.value, ast.Constant)} + for node in ast.walk(tree): + if not isinstance(node, (ast.Assign, ast.AnnAssign)): + continue + targets = node.targets if isinstance(node, ast.Assign) else [node.target] + for target in targets: + if not isinstance(target, ast.Subscript) or not isinstance(target.slice, ast.Constant) or target.slice.value != "stop_reason": + continue + # This copies a child result into a ledger entry; it does + # not stamp the current run's runtime.context. + if source.name == "delegation_ledger.py" and isinstance(target.value, ast.Name) and target.value.id == "entry": + continue + value = constants.get(node.value.id) if isinstance(node.value, ast.Name) else node.value + assert isinstance(value, ast.Constant) and isinstance(value.value, str), f"Classify stop_reason writer at {source.name}:{node.lineno}" + stamped_reasons.add(value.value) + + # The subagent budget only removes task calls; the lead may still + # complete its work using other tools. Every other writer is terminal. + nonterminal_reasons = {"subagent_limit_capped"} + assert nonterminal_reasons <= stamped_reasons + policy = TodoMiddleware().release_policy_parameters() + assert set(policy["completion_reminder_hard_stop_reasons"]) == stamped_reasons - nonterminal_reasons + + @pytest.mark.parametrize("marker", ["safety_termination", "model_length_termination"]) + @pytest.mark.parametrize("context", [None, {}, {"stop_reason": "custom_stop"}]) + def test_termination_marker_suppresses_reminders_without_recognized_context(self, marker, context): + mw = TodoMiddleware() + runtime = _make_runtime() + runtime.context = context + state = {"messages": [AIMessage(content="partial response", additional_kwargs={marker: {"suppressed_tool_call_count": 0}})], "todos": _incomplete_todos()} + + assert mw.after_model(state, runtime) is None + assert mw._pending_completion_reminders == {} + + @pytest.mark.parametrize("invoke_async", [False, True], ids=["sync", "async"]) + @pytest.mark.parametrize("stop_reason", ["loop_capped", "token_capped", "safety_capped", "model_length_capped"]) + def test_recognized_stop_reason_suppresses_completion_reminders(self, invoke_async, stop_reason): + mw = TodoMiddleware() + runtime = _make_runtime() + runtime.context["stop_reason"] = stop_reason + state = {"messages": [_ai_no_tool_calls()], "todos": _incomplete_todos()} + + result = asyncio.run(mw.aafter_model(state, runtime)) if invoke_async else mw.after_model(state, runtime) + + assert result is None + assert mw._pending_completion_reminders == {} + assert mw._completion_reminder_counts == {} + assert state["todos"] == _incomplete_todos() + + @pytest.mark.parametrize("invoke_async", [False, True], ids=["sync", "async"]) + @pytest.mark.parametrize("stop_reason", ["loop_capped", "token_capped", "safety_capped", "model_length_capped"]) + def test_hard_stop_takes_precedence_over_base_jump(self, monkeypatch, invoke_async, stop_reason): + base_hook = MagicMock(return_value={"jump_to": "model"}) + monkeypatch.setattr(TodoListMiddleware, "after_model", base_hook) + mw = TodoMiddleware() + runtime = _make_runtime() + state = {"messages": [_ai_no_tool_calls()], "todos": _incomplete_todos()} + + # Preserve normal parent behavior, but never let it override a hard stop. + def after_model(): + return asyncio.run(mw.aafter_model(state, runtime)) if invoke_async else mw.after_model(state, runtime) + + assert after_model() == {"jump_to": "model"} + base_hook.assert_called_once() + base_hook.reset_mock() + runtime.context["stop_reason"] = stop_reason + assert after_model() is None + base_hook.assert_not_called() + + @pytest.mark.parametrize("stop_reason", [None, "custom_stop", "subagent_limit_capped", [], {}]) + def test_unrecognized_stop_reason_keeps_completion_reminders(self, stop_reason): + mw = TodoMiddleware() + runtime = _make_runtime() + runtime.context["stop_reason"] = stop_reason + state = {"messages": [_ai_no_tool_calls()], "todos": _incomplete_todos()} + + assert mw.after_model(state, runtime) == {"jump_to": "model"} + def test_returns_none_when_agent_still_using_tools(self): mw = TodoMiddleware() state = { @@ -608,7 +697,173 @@ class TestWrapModelCall: assert mw._completion_reminder_counts == {} +@pytest.fixture +def production_chain_config(tmp_path, monkeypatch): + from deerflow.config.app_config import AppConfig, pop_current_app_config, push_current_app_config + from deerflow.sandbox.sandbox_provider import shutdown_sandbox_provider + + monkeypatch.setenv("DEER_FLOW_HOME", str(tmp_path)) + # A missing ambient config must not be masked by a developer's checkout. + monkeypatch.setenv("DEER_FLOW_CONFIG_PATH", str(tmp_path / "missing-config.yaml")) + monkeypatch.setenv("LANGSMITH_TRACING", "false") + monkeypatch.setenv("LANGCHAIN_TRACING_V2", "false") + monkeypatch.setattr("deerflow.config.paths._paths", None) + monkeypatch.setattr("deerflow.sandbox.sandbox_provider._default_sandbox_provider", None) + monkeypatch.setattr("deerflow.sandbox.local.local_sandbox_provider._singleton", None) + app_config = AppConfig.model_validate( + { + "models": [{"name": "offline", "use": "langchain_openai:ChatOpenAI", "model": "offline"}], + "sandbox": {"use": "deerflow.sandbox.local:LocalSandboxProvider", "allow_host_bash": False}, + "skills": {"path": str(tmp_path / "skills")}, + "memory": {"enabled": False}, + "title": {"enabled": False}, + "summarization": {"enabled": False}, + } + ) + # Sandbox lifecycle hooks resolve get_app_config() at invocation time, + # independently of the explicit config passed to build_middlewares(). + # The runtime binding propagates through both graph and worker-thread calls. + push_current_app_config(app_config) + try: + yield app_config + finally: + try: + shutdown_sandbox_provider() + finally: + pop_current_app_config() + + class TestTodoMiddlewareAgentGraphIntegration: + @pytest.mark.parametrize("invoke_async", [False, True], ids=["sync", "async"]) + @pytest.mark.parametrize("response_kind", ["tool", "blank", "partial_text"]) + def test_production_chain_stops_after_safety_response(self, production_chain_config, invoke_async, response_kind): + from deerflow.agents.lead_agent.agent import build_middlewares + + executed = [] + + @tool + def record_action(marker: str) -> str: + """Record a harmless action in memory.""" + executed.append(marker) + return marker + + filtered = AIMessage( + id="filtered-response", + content="Partial report." if response_kind == "partial_text" else "", + response_metadata={"finish_reason": "content_filter"}, + tool_calls=[{"name": "record_action", "args": {"marker": "filtered"}, "id": "filtered-call"}] if response_kind == "tool" else [], + ) + model = _CapturingFakeMessagesListChatModel( + responses=[ + filtered, + AIMessage(content="", tool_calls=[{"name": "record_action", "args": {"marker": "after-stop"}, "id": "later-call"}]), + AIMessage(content="final"), + ] + ) + middlewares = build_middlewares({"configurable": {"is_plan_mode": True}}, model_name="offline", app_config=production_chain_config, memory_enabled=False, available_skills=set()) + graph = create_agent(model=model, tools=[record_action], middleware=middlewares, state_schema=ThreadState) + todos = _incomplete_todos() + context = {"thread_id": "production-safety-thread", "run_id": "production-safety-run"} + state = {"messages": [("user", "Finish the report.")], "todos": todos} + + result = asyncio.run(graph.ainvoke(state, context=context)) if invoke_async else graph.invoke(state, context=context) + + assert len(model.seen_messages) == 1 + assert executed == [] + assert context["stop_reason"] == "safety_capped" + final = result["messages"][-1] + assert final.additional_kwargs["safety_termination"] + assert final.content + assert not final.tool_calls + assert result["todos"] == todos + if response_kind == "partial_text": + assert final.content == filtered.content + todo_mw = next(mw for mw in middlewares if isinstance(mw, TodoMiddleware)) + assert todo_mw._pending_completion_reminders == {} + assert todo_mw._completion_reminder_counts == {} + + @pytest.mark.parametrize("invoke_async", [False, True], ids=["sync", "async"]) + @pytest.mark.parametrize("guard_kind", ["loop", "token", "safety", "model_length"]) + @pytest.mark.parametrize("pending", [True, False], ids=["unfinished", "completed"]) + def test_run_hard_stop_does_not_reengage_todos(self, invoke_async, guard_kind, pending): + from deerflow.agents.middlewares.loop_detection_middleware import LoopDetectionMiddleware + from deerflow.agents.middlewares.token_budget_middleware import TokenBudgetMiddleware + from deerflow.config.token_budget_config import TokenBudgetConfig + + executed = [] + + @tool + def record_action(marker: str) -> str: + """Record a harmless action in memory.""" + executed.append(marker) + return marker + + def action(marker, call_id, tokens=10): + return AIMessage( + content="", + tool_calls=[{"name": "record_action", "args": {"marker": marker}, "id": call_id}], + usage_metadata={"input_tokens": tokens, "output_tokens": 0, "total_tokens": tokens}, + ) + + todos = [{"content": "Record the requested actions", "status": "pending" if pending else "completed"}] + blocked_action = action("repeated", "blocked", tokens=1000 if guard_kind == "token" else 10) + if guard_kind in ("safety", "model_length"): + blocked_action.response_metadata = {"finish_reason": "content_filter" if guard_kind == "safety" else "length"} + model = _CapturingFakeMessagesListChatModel( + responses=[ + AIMessage(content="", tool_calls=[{"name": "write_todos", "args": {"todos": todos}, "id": "todos-start"}]), + action("repeated", "first"), + blocked_action, + # A different action makes unintended post-stop execution observable. + action("after-stop", "after-stop"), + AIMessage(content="final answer"), + AIMessage(content="final answer"), + AIMessage(content="final answer"), + ] + ) + todo_mw = TodoMiddleware() + guards = { + "loop": LoopDetectionMiddleware(warn_threshold=2, hard_limit=2), + "token": TokenBudgetMiddleware(TokenBudgetConfig(enabled=True, max_tokens=1000)), + "safety": SafetyFinishReasonMiddleware(), + "model_length": ModelLengthFinishReasonMiddleware(), + } + guard = guards[guard_kind] + # Production registers Todo before the guards; after_model runs in reverse. + graph = create_agent(model=model, tools=[record_action], middleware=[todo_mw, guard], state_schema=ThreadState, checkpointer=InMemorySaver()) + config = {"configurable": {"thread_id": "hard-stop-thread"}, "recursion_limit": 100} + context = {"thread_id": "hard-stop-thread", "run_id": "hard-stop-run"} + state = {"messages": [("user", "Record actions and track todos.")]} + + def invoke(state, context): + return asyncio.run(graph.ainvoke(state, context=context, config=config)) if invoke_async else graph.invoke(state, context=context, config=config) + + result = invoke(state, context) + + assert context["stop_reason"] == f"{guard_kind}_capped" + assert executed == ["repeated"] + assert len(model.seen_messages) == 3 + # Pin the real producers to Todo's declared policy, without importing + # runtime Goal policy (which currently covers only loop/token). + assert context["stop_reason"] in todo_mw.release_policy_parameters()["completion_reminder_hard_stop_reasons"] + assert result["todos"] == todos + assert not result["messages"][-1].tool_calls + assert not any(_todo_completion_reminders(messages) for messages in model.seen_messages) + assert todo_mw._pending_completion_reminders == {} + snapshot = graph.get_state(config) + assert snapshot.next == () + assert snapshot.values["todos"] == todos + + if pending: + # Restore the same thread from its checkpoint without resupplying Todo. + # The old cap must not disable the new run or recount historical usage. + next_context = {"thread_id": "hard-stop-thread", "run_id": "next-run"} + next_result = invoke({"messages": [("user", "Continue the unfinished task.")]}, next_context) + assert "stop_reason" not in next_context + assert executed == ["repeated", "after-stop"] + assert any(_todo_completion_reminders(messages) for messages in model.seen_messages[3:]) + assert next_result["todos"] == todos + def test_reuses_thread_state_todos_schema_in_real_agent_graph(self): mw = TodoMiddleware() model = _CapturingFakeMessagesListChatModel(