From 126fc9ea8173c7b07c0f03898f9983bae0b3bda2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=BB=84=E4=BA=91=E9=BE=99?= <76432572+nankingjing@users.noreply.github.com> Date: Fri, 24 Jul 2026 21:56:11 +0800 Subject: [PATCH] fix(subagents): clamp subagent limit consistently with MIN_SUBAGENT_LIMIT (#4081) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(subagents): align prompt and middleware subagent limit; allow min of 1 SubagentLimitMiddleware clamped max_concurrent to [2, 4] internally, but agent.py and client.py fed the raw config value into the system prompt, so a user-configured 1 (or 5) produced a prompt that disagreed with the enforced middleware limit. Lower MIN_SUBAGENT_LIMIT to 1 and clamp the raw config value with _clamp_subagent_limit() at both the agent factory and the embedded client so the prompt and middleware see the same value. * fix: remove unused imports MAX_CONCURRENT_SUBAGENT_CALLS, MIN_CONCURRENT_SUBAGENT_CALLS, clamp_subagent_concurrency * fix: harmonize clamp range [1,4] across middleware, config, and prompt path; fix lint - Changed MIN_CONCURRENT_SUBAGENT_CALLS from 2 to 1 so prompt.py's clamp_subagent_concurrency and the middleware's _clamp_subagent_limit both clamp to [1,4] — eliminating the divergence where the prompt told the model 'max 2 task calls' but the middleware enforced 1. - Applied _clamp_subagent_limit at build_middlewares (agent.py:360) so all 3 construction sites (agent.py:360, agent.py:450, client.py:259) consistently clamp the config-resolved limit. - Derived MIN_SUBAGENT_LIMIT / MAX_SUBAGENT_LIMIT from MIN_CONCURRENT_SUBAGENT_CALLS / MAX_CONCURRENT_SUBAGENT_CALLS so the two module-level definitions stay in sync. - Added TestConfigParity.test_prompt_path_and_middleware_clamp_agree regression test. - Fixed lint. * fix(lint): add missing imports for MIN_CONCURRENT_SUBAGENT_CALLS and MAX_CONCURRENT_SUBAGENT_CALLS * docs+test: update AGENTS.md clamp range to 1-4; add prompt/middleware parity regression test - backend/AGENTS.md still documented the old [2,4] clamp in two places; updated to [1,4] to match MIN_CONCURRENT_SUBAGENT_CALLS = 1. - Added test_apply_prompt_template_single_subagent_limit_matches_middleware: renders the real system prompt with max_concurrent_subagents=1 and asserts the advertised HARD LIMITS value equals SubagentLimitMiddleware's enforced max_concurrent — the end-to-end check that would have caught the [1,4] vs [2,4] prompt-path divergence flagged in review. * refactor: simplify per review — restore clamp delegation, drop redundant call-site clamps Per willem-bd's review, reduce the PR to the one behavioral change plus docs/tests: - _clamp_subagent_limit delegates to clamp_subagent_concurrency again instead of inlining a byte-identical copy; with a single source of truth the TestConfigParity sync-check class is unnecessary — dropped. - Revert the call-site clamps in agent.py (build_middlewares, _make_lead_agent) and client.py (_ensure_agent) to main: both downstream consumers (SubagentLimitMiddleware.__init__ and the prompt path) already clamp internally, and the cross-module private import of _clamp_subagent_limit goes away with them. - Keep MIN_CONCURRENT_SUBAGENT_CALLS = 1 (the fix), the [1, 4] docstring updates, the AGENTS.md range corrections, and the end-to-end prompt/middleware parity test for single-subagent mode (docstring reworded: on main a configured 1 was bumped to 2 by both paths — there was no divergence to fix, just a silently raised floor). * test: fix stale comment referencing reverted agent.py/client.py call-site clamps --------- Co-authored-by: nankingjing Co-authored-by: Willem Jiang --- backend/AGENTS.md | 4 +- .../middlewares/subagent_limit_middleware.py | 4 +- .../deerflow/config/subagents_config.py | 2 +- backend/tests/test_lead_agent_prompt.py | 42 +++++++++++++++++++ .../tests/test_subagent_limit_middleware.py | 20 +++++++-- 5 files changed, 63 insertions(+), 9 deletions(-) diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 424c4b16f..8179fe9ab 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -342,7 +342,7 @@ Before changing a later authorization phase, read the [authorization RFC](../doc 24. **McpRoutingMiddleware** - *(optional, if `tool_search.enabled` and PR1 MCP routing metadata produce a routing index)* Auto-promotes matching deferred MCP tool schemas before the model call by writing a minimal `promoted` state update. It matches only the latest real `HumanMessage`, uses the global `tool_search.auto_promote_top_k` limit (default 3, clamped to 1..5), never executes tools, and must be installed before `DeferredToolFilterMiddleware` 25. **DeferredToolFilterMiddleware** - *(optional, if `tool_search.enabled`)* Hides deferred (MCP) tool schemas from the bound model until `tool_search` or `McpRoutingMiddleware` promotes them (reads per-thread promotions from `ThreadState.promoted`, hash-scoped) 26. **SystemMessageCoalescingMiddleware** - Merges every SystemMessage into a single leading SystemMessage per request; provider-agnostic fix for strict backends (vLLM/SGLang/Qwen/Anthropic) that reject non-leading system messages. Touches the per-request payload only (checkpoint state unchanged); on midnight crossings only the latest `dynamic_context_reminder` SystemMessage survives -27. **SubagentLimitMiddleware** - *(optional, if `subagent_enabled`)* Truncates excess `task` tool calls to enforce both the per-response concurrency limit (`max_concurrent_subagents`, clamped to 2-4) and the per-run total delegation cap (`max_total_subagents` runtime override or `subagents.max_total_per_run`, default 6, clamped to 1-50). The total cap counts current-run entries in the durable delegation ledger (entries are tagged with `run_id` when captured), so repeated planning checkpoints in one run cannot keep launching legal-sized batches indefinitely, while later user turns in the same thread get a fresh run budget. If the cap is exhausted, the middleware strips remaining `task` calls, forces `finish_reason="stop"`, and appends a visible limit note so the run can synthesize existing results instead of ending with an empty tool-call response. +27. **SubagentLimitMiddleware** - *(optional, if `subagent_enabled`)* Truncates excess `task` tool calls to enforce both the per-response concurrency limit (`max_concurrent_subagents`, clamped to 1-4) and the per-run total delegation cap (`max_total_subagents` runtime override or `subagents.max_total_per_run`, default 6, clamped to 1-50). The total cap counts current-run entries in the durable delegation ledger (entries are tagged with `run_id` when captured), so repeated planning checkpoints in one run cannot keep launching legal-sized batches indefinitely, while later user turns in the same thread get a fresh run budget. If the cap is exhausted, the middleware strips remaining `task` calls, forces `finish_reason="stop"`, and appends a visible limit note so the run can synthesize existing results instead of ending with an empty tool-call response. 28. **LoopDetectionMiddleware** - *(optional, if `loop_detection.enabled`)* Detects repeated tool-call loops; hard-stop clears both structured `tool_calls` and raw provider tool-call metadata before forcing a final text answer; stamps `loop_capped` via `consume_stop_reason` (#3875 Phase 2), symmetric to `TokenBudgetMiddleware` 29. **TokenBudgetMiddleware** - *(optional, if `token_budget.enabled`)* Enforces per-run token limits 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 @@ -518,7 +518,7 @@ copying a raw checkpoint because delta state is not self-contained in one tuple. **Built-in Agents**: `general-purpose` (all tools except `task`) and `bash` (command specialist) **User-scoped Skills**: Subagents resolve their configured skills through `get_or_new_user_skill_storage(user_id)` using the parent runtime identity, with `DEFAULT_USER_ID` only when no identity is available. This keeps custom-skill shadowing and visibility aligned with the lead agent instead of reading the global-only catalog. **Execution**: Dual thread pool - `_scheduler_pool` (3 workers) + `_execution_pool` (3 workers) -**Concurrency and total delegation cap**: `MAX_CONCURRENT_SUBAGENTS = 3` is enforced by `SubagentLimitMiddleware` (truncates excess tool calls in `after_model`; runtime `max_concurrent_subagents` is clamped to 2-4). The same middleware also enforces `subagents.max_total_per_run` (default 6, config schema 1-50, runtime override `max_total_subagents` clamped to the same range) against current-run entries in the durable delegation ledger, so a long lead-agent run cannot bypass concurrency limits by launching repeated legal-sized batches at each planning checkpoint, but historical delegations from previous runs in the same thread do not consume the new run's budget. The lead-agent prompt uses the same clamped values, so model-visible limits match enforcement. Gateway `run_agent()` and embedded `DeerFlowClient.stream()` both provide a per-invocation `run_id` in runtime context; `DeerFlowClient.stream()` also tags its input `HumanMessage` with that same id so durable-context capture can identify the current request boundary. Gateway resume paths may not append a new `HumanMessage`, so the worker also exposes the pre-run checkpoint's message ids in runtime context; durable-context capture uses that as the current-run boundary and never re-tags older task calls as the resumed run. When no delegation slots remain, task calls are stripped, provider raw tool-call metadata is synced, `finish_reason` is forced to `stop`, and a visible "subagent delegation limit" note is appended so the agent can synthesize already-collected results. Default subagent timeout `subagents.timeout_seconds=1800` (30 min) and built-in `general-purpose` `max_turns=150` (raised from 100/15-min so deep-research subtasks stop hitting `GraphRecursionError` out of the box) +**Concurrency and total delegation cap**: `MAX_CONCURRENT_SUBAGENTS = 3` is enforced by `SubagentLimitMiddleware` (truncates excess tool calls in `after_model`; runtime `max_concurrent_subagents` is clamped to 1-4). The same middleware also enforces `subagents.max_total_per_run` (default 6, config schema 1-50, runtime override `max_total_subagents` clamped to the same range) against current-run entries in the durable delegation ledger, so a long lead-agent run cannot bypass concurrency limits by launching repeated legal-sized batches at each planning checkpoint, but historical delegations from previous runs in the same thread do not consume the new run's budget. The lead-agent prompt uses the same clamped values, so model-visible limits match enforcement. Gateway `run_agent()` and embedded `DeerFlowClient.stream()` both provide a per-invocation `run_id` in runtime context; `DeerFlowClient.stream()` also tags its input `HumanMessage` with that same id so durable-context capture can identify the current request boundary. Gateway resume paths may not append a new `HumanMessage`, so the worker also exposes the pre-run checkpoint's message ids in runtime context; durable-context capture uses that as the current-run boundary and never re-tags older task calls as the resumed run. When no delegation slots remain, task calls are stripped, provider raw tool-call metadata is synced, `finish_reason` is forced to `stop`, and a visible "subagent delegation limit" note is appended so the agent can synthesize already-collected results. Default subagent timeout `subagents.timeout_seconds=1800` (30 min) and built-in `general-purpose` `max_turns=150` (raised from 100/15-min so deep-research subtasks stop hitting `GraphRecursionError` out of the box) **Flow**: `task()` tool → `SubagentExecutor` → background thread → poll 5s → SSE events → result. `task_started` carries the resolved effective model name. The per-subagent `SubagentTokenCollector` publishes a cumulative usage snapshot to the shared `SubagentResult` after every completed LLM response; the next `task_running` event carries that snapshot, so collapsed workspace cards can update without re-accounting parent-run totals. Terminal ToolMessage metadata (`subagent_model_name`, `subagent_token_usage`) and the persisted `subagent.end` event retain the model/usage after reload; absent provider usage stays absent rather than being estimated as zero. **Events**: `task_started`, `task_running`, `task_completed`/`task_failed`/`task_timed_out` **Handled LLM failures**: `LLMErrorHandlingMiddleware` deliberately converts provider/model exceptions into an `AIMessage` so the graph can end cleanly, stamping `additional_kwargs.deerflow_error_fallback=true` plus error metadata. Clean graph termination does not imply subagent success: `SubagentExecutor` inspects the last assistant message at terminalization and maps a marked fallback to `SubagentStatus.FAILED`, which then emits `task_failed` and the existing structured `subagent_error`. Only the marker is authoritative — error-looking assistant prose without it remains a normal completed result, so neither the executor nor frontend parses display text as a status protocol. diff --git a/backend/packages/harness/deerflow/agents/middlewares/subagent_limit_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/subagent_limit_middleware.py index 39e8e87d2..62b615d15 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/subagent_limit_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/subagent_limit_middleware.py @@ -36,7 +36,7 @@ _TOTAL_LIMIT_STOP_MSG = ( def _clamp_subagent_limit(value: int) -> int: - """Clamp subagent limit to valid range [2, 4].""" + """Clamp subagent limit to valid range [1, 4].""" return clamp_subagent_concurrency(value) @@ -104,7 +104,7 @@ class SubagentLimitMiddleware(AgentMiddleware[AgentState]): Args: max_concurrent: Maximum number of concurrent subagent calls allowed. - Defaults to MAX_CONCURRENT_SUBAGENTS (3). Clamped to [2, 4]. + Defaults to MAX_CONCURRENT_SUBAGENTS (3). Clamped to [1, 4]. max_total: Maximum number of subagent calls allowed across the run. Defaults to 6. Clamped to [1, 50]. """ diff --git a/backend/packages/harness/deerflow/config/subagents_config.py b/backend/packages/harness/deerflow/config/subagents_config.py index 1fae844c0..699669e6c 100644 --- a/backend/packages/harness/deerflow/config/subagents_config.py +++ b/backend/packages/harness/deerflow/config/subagents_config.py @@ -11,7 +11,7 @@ logger = logging.getLogger(__name__) DEFAULT_MAX_TOTAL_SUBAGENTS_PER_RUN = 6 MIN_TOTAL_SUBAGENTS_PER_RUN = 1 MAX_TOTAL_SUBAGENTS_PER_RUN = 50 -MIN_CONCURRENT_SUBAGENT_CALLS = 2 +MIN_CONCURRENT_SUBAGENT_CALLS = 1 MAX_CONCURRENT_SUBAGENT_CALLS = 4 diff --git a/backend/tests/test_lead_agent_prompt.py b/backend/tests/test_lead_agent_prompt.py index dac928ecb..1391bf1d8 100644 --- a/backend/tests/test_lead_agent_prompt.py +++ b/backend/tests/test_lead_agent_prompt.py @@ -262,6 +262,48 @@ def test_apply_prompt_template_clamps_subagent_limits_to_enforced_bounds(monkeyp assert "MAXIMUM 50 `task` CALLS PER RUN" in prompt +def test_apply_prompt_template_single_subagent_limit_matches_middleware(monkeypatch): + """Regression test for single-subagent mode (MIN_CONCURRENT_SUBAGENT_CALLS = 1). + + Before the floor was lowered to 1, a user-configured limit of 1 was silently + bumped to 2 by both the prompt path and the middleware. This renders the real + system prompt with max_concurrent_subagents=1 and asserts the advertised + HARD LIMITS value equals the middleware-enforced max_concurrent, so the two + paths cannot drift apart on the newly-allowed value. + """ + from deerflow.agents.middlewares.subagent_limit_middleware import SubagentLimitMiddleware + + explicit_config = SimpleNamespace( + sandbox=SimpleNamespace( + use="deerflow.sandbox.local:LocalSandboxProvider", + allow_host_bash=False, + mounts=[], + ), + subagents=SubagentsAppConfig(), + skills=SimpleNamespace(container_path="/mnt/skills", use="deerflow.skills.storage.local_skill_storage:LocalSkillStorage", get_skills_path=lambda: Path("/tmp/skills")), + skill_evolution=SimpleNamespace(enabled=False), + tool_search=SimpleNamespace(enabled=False), + memory=SimpleNamespace(enabled=False, injection_enabled=True, max_injection_tokens=2000), + acp_agents={}, + ) + + monkeypatch.setattr(prompt_module, "get_or_new_skill_storage", lambda app_config=None: SimpleNamespace(load_skills=lambda enabled_only=True: [])) + monkeypatch.setattr(prompt_module, "get_agent_soul", lambda agent_name=None: "") + + enforced = SubagentLimitMiddleware(max_concurrent=1).max_concurrent + assert enforced == 1 # 1 must pass through, not be bumped to 2 + + prompt = prompt_module.apply_prompt_template( + subagent_enabled=True, + max_concurrent_subagents=1, + max_total_subagents=6, + app_config=explicit_config, + ) + + assert f"MAXIMUM {enforced} `task` CALLS PER RESPONSE" in prompt + assert f"HARD LIMITS: max {enforced} `task` calls per response" in prompt + + def test_build_acp_section_uses_explicit_app_config_without_global_config(monkeypatch): explicit_config = SimpleNamespace(acp_agents={"codex": object()}) diff --git a/backend/tests/test_subagent_limit_middleware.py b/backend/tests/test_subagent_limit_middleware.py index dee3d3c4f..0d336fc8f 100644 --- a/backend/tests/test_subagent_limit_middleware.py +++ b/backend/tests/test_subagent_limit_middleware.py @@ -52,11 +52,23 @@ def _raw_tool_call(call_id: str, name: str = "task") -> dict: class TestClampSubagentLimit: - def test_below_min_clamped_to_min(self): - assert _clamp_subagent_limit(0) == MIN_SUBAGENT_LIMIT - assert _clamp_subagent_limit(1) == MIN_SUBAGENT_LIMIT + def test_min_limit_is_one(self): + # MIN lowered from 2 to 1 so a user asking for a single subagent gets 1. + # Both consumers (SubagentLimitMiddleware.__init__ and the prompt path) + # share this floor via clamp_subagent_concurrency in subagents_config.py. + assert MIN_SUBAGENT_LIMIT == 1 + assert MAX_SUBAGENT_LIMIT == 4 - def test_above_max_clamped_to_max(self): + def test_below_min_clamped_to_one(self): + assert _clamp_subagent_limit(0) == 1 + assert _clamp_subagent_limit(-5) == 1 + + def test_one_is_allowed_not_bumped_to_two(self): + # Previously 1 clamped up to 2; it must now pass through as 1. + assert _clamp_subagent_limit(1) == 1 + + def test_above_max_clamped_to_four(self): + assert _clamp_subagent_limit(5) == 4 assert _clamp_subagent_limit(10) == MAX_SUBAGENT_LIMIT assert _clamp_subagent_limit(100) == MAX_SUBAGENT_LIMIT