From 7b8a7925c2b92af5cb11b55267925263ea302455 Mon Sep 17 00:00:00 2001 From: Guo-Yixin Date: Sat, 3 Oct 2026 16:38:00 +0800 Subject: [PATCH] fix(config): reject invalid tool output budgets (#6172) * fix(config): validate tool output budget limits * fix(config): align Helm config schema version --- README.md | 4 + .../harness/deerflow/config/AGENTS.md | 7 + .../deerflow/config/tool_output_config.py | 38 +++-- .../tests/test_tool_output_config_limits.py | 155 ++++++++++++++++++ deploy/helm/deer-flow/README.md | 2 +- 5 files changed, 187 insertions(+), 19 deletions(-) create mode 100644 backend/tests/test_tool_output_config_limits.py diff --git a/README.md b/README.md index 711a23d198..7c6ff73e78 100644 --- a/README.md +++ b/README.md @@ -1798,6 +1798,10 @@ For example, independent read-only research can run concurrently when the wall-c ### Sandbox & File System Host-externalized tool outputs use the Gateway's normal file-creation umask. +`tool_output` character/count budgets, including per-tool overrides, require +non-negative integers; YAML booleans are rejected rather than treated as 0 or 1. +An explicit zero per-tool override disables externalization while preserving +any positive global fallback limit. Mounted sandboxes running under another UID need read access through the shared storage permissions. An unclean shutdown can leave `.tool-output-*.tmp` files in `tool_output.storage_subdir` (default `.tool-results`) under thread outputs. diff --git a/backend/packages/harness/deerflow/config/AGENTS.md b/backend/packages/harness/deerflow/config/AGENTS.md index 2d42aed8c5..16c026a140 100644 --- a/backend/packages/harness/deerflow/config/AGENTS.md +++ b/backend/packages/harness/deerflow/config/AGENTS.md @@ -39,6 +39,13 @@ their tool-local settings unchanged. Top-level `recursion_limit` and `max_recursion_limit` are hot-reloaded per Gateway run. The former supplies the default when a request omits or provides an invalid value; the latter caps both configured and client-provided budgets. +`tool_output` character/count limits and every `tool_overrides` value share +non-negative integer validation. Reject booleans before Pydantic coercion; +preserve numeric strings from environment substitution and explicit zero. +Zero per-tool overrides disable externalization only; a positive global +fallback budget still truncates oversized output. Regression tests exercise +YAML loading and both middleware tool-call paths in `test_tool_output_config_limits.py`. + **Config Caching**: `get_app_config()` caches the parsed config, but automatically reloads it when the resolved config path or file content signature changes. The signature includes file metadata and a content digest, so Gateway and LangGraph reads stay aligned with `config.yaml` edits even on object-store or network mounts where mtime can remain stale. The loader reads the file once through `file_signature.read_config_with_signature` and parses those same bytes, so the recorded signature always describes the parsed content: a write that races the load can only cause one extra reload, never a cache that holds one revision under another revision's signature (which the comparison could never detect). **Config Hot-Reload Boundary**: Gateway dependencies route through `get_app_config()` on every request, so per-run fields like `models[*].max_tokens`, `summarization.*`, `title.*`, `memory.*`, `subagents.*`, `verification.*`, `tools[*]`, and the agent system prompt pick up `config.yaml` edits on the next message. `AppConfig` is intentionally **not** cached on `app.state` — `lifespan()` keeps a local `startup_config` variable for one-shot bootstrap work and passes it to `langgraph_runtime(app, startup_config)`. diff --git a/backend/packages/harness/deerflow/config/tool_output_config.py b/backend/packages/harness/deerflow/config/tool_output_config.py index a3e4d3bf42..6eafce1f7a 100644 --- a/backend/packages/harness/deerflow/config/tool_output_config.py +++ b/backend/packages/harness/deerflow/config/tool_output_config.py @@ -3,12 +3,22 @@ from __future__ import annotations import os +from typing import Annotated -from pydantic import BaseModel, Field, field_validator +from pydantic import BaseModel, BeforeValidator, Field, field_validator from deerflow.constants import TOOL_RESULTS_DIRNAME +def _reject_boolean_limit(value: object) -> object: + if isinstance(value, bool): + raise ValueError("must be an integer, not a boolean") + return value + + +_OutputLimit = Annotated[int, BeforeValidator(_reject_boolean_limit), Field(ge=0)] + + class ToolOutputConfig(BaseModel): """Config section for tool-result output budget enforcement. @@ -28,34 +38,28 @@ class ToolOutputConfig(BaseModel): default=True, description="Enable the tool output budget middleware.", ) - externalize_min_chars: int = Field( + externalize_min_chars: _OutputLimit = Field( default=12_000, - ge=0, description="Character threshold to trigger disk externalization. Outputs below this pass through unchanged. Set to 0 to disable externalization (fallback truncation still applies when output exceeds fallback_max_chars).", ) - preview_head_chars: int = Field( + preview_head_chars: _OutputLimit = Field( default=2_000, - ge=0, description="Sampling budget retained for compatibility. Typed previews use this with preview_tail_chars only for fallback samples inside the structured synopsis.", ) - preview_tail_chars: int = Field( + preview_tail_chars: _OutputLimit = Field( default=1_000, - ge=0, description="Sampling budget retained for compatibility. Typed previews use this with preview_head_chars only for fallback samples inside the structured synopsis.", ) - fallback_max_chars: int = Field( + fallback_max_chars: _OutputLimit = Field( default=30_000, - ge=0, description="Maximum characters when disk persistence is unavailable. 0 disables fallback truncation.", ) - fallback_head_chars: int = Field( + fallback_head_chars: _OutputLimit = Field( default=8_000, - ge=0, description="Head characters for fallback truncation.", ) - fallback_tail_chars: int = Field( + fallback_tail_chars: _OutputLimit = Field( default=3_000, - ge=0, description="Tail characters for fallback truncation.", ) storage_subdir: str = Field( @@ -88,7 +92,7 @@ class ToolOutputConfig(BaseModel): default_factory=lambda: ["read_file", "read_file_tool"], description="Tool names exempt from budget enforcement (prevents persist→read→persist loops).", ) - tool_overrides: dict[str, int] = Field( + tool_overrides: dict[str, _OutputLimit] = Field( default_factory=dict, description="Per-tool externalize_min_chars overrides. Keys are tool names, values are char thresholds. Use 0 to disable externalization for a specific tool.", ) @@ -102,9 +106,8 @@ class ToolOutputConfig(BaseModel): "changes: stored message history, receipts, and the run journal keep the original arguments." ), ) - superseded_write_min_chars: int = Field( + superseded_write_min_chars: _OutputLimit = Field( default=2000, - ge=0, description=( "Elide only write_file content at least this many characters long; shorter content stays visible. " "0 elides every non-empty content. This is a Python character count, not a token count: the same value " @@ -112,8 +115,7 @@ class ToolOutputConfig(BaseModel): "figure is the same character count." ), ) - keep_recent_writes: int = Field( + keep_recent_writes: _OutputLimit = Field( default=1, - ge=0, description="Never elide the content of the newest N successful write_file calls (counted across all paths), even when superseded, so the model can still say what it just wrote without a read. 0 keeps none.", ) diff --git a/backend/tests/test_tool_output_config_limits.py b/backend/tests/test_tool_output_config_limits.py new file mode 100644 index 0000000000..42000eb701 --- /dev/null +++ b/backend/tests/test_tool_output_config_limits.py @@ -0,0 +1,155 @@ +"""Reject ambiguous output budgets before they reach the runtime middleware.""" + +from types import SimpleNamespace + +import pytest +import yaml +from langchain_core.messages import ToolMessage +from pydantic import ValidationError + +from deerflow.agents.middlewares.tool_output_budget_middleware import ToolOutputBudgetMiddleware +from deerflow.config.app_config import AppConfig +from deerflow.config.tool_output_config import ToolOutputConfig + +LIMIT_FIELDS = ( + "externalize_min_chars", + "preview_head_chars", + "preview_tail_chars", + "fallback_max_chars", + "fallback_head_chars", + "fallback_tail_chars", + "superseded_write_min_chars", + "keep_recent_writes", +) + + +@pytest.mark.parametrize("field", LIMIT_FIELDS) +@pytest.mark.parametrize("value", [True, False]) +def test_boolean_limits_fail_at_the_field(field, value): + with pytest.raises(ValidationError) as exc: + ToolOutputConfig.model_validate({field: value}) + + assert exc.value.errors()[0]["loc"] == (field,) + assert "boolean" in exc.value.errors()[0]["msg"] + + +@pytest.mark.parametrize("value", [True, False, -1, "-1"]) +def test_invalid_override_identifies_the_tool(value): + with pytest.raises(ValidationError) as exc: + ToolOutputConfig(tool_overrides={"web_fetch": value, "bash": 500}) + + assert exc.value.errors()[0]["loc"] == ("tool_overrides", "web_fetch") + + +@pytest.mark.parametrize("field", LIMIT_FIELDS) +@pytest.mark.parametrize("value", [0, 120, "120", 120.0]) +def test_existing_numeric_inputs_remain_supported(field, value): + config = ToolOutputConfig.model_validate({field: value}) + + assert getattr(config, field) == int(value) + assert type(getattr(config, field)) is int + + +@pytest.mark.parametrize("value", [1.5, "not-a-number", None]) +def test_overrides_reject_other_invalid_integer_budgets(value): + with pytest.raises(ValidationError) as exc: + ToolOutputConfig(tool_overrides={"web_fetch": value}) + + assert exc.value.errors()[0]["loc"] == ("tool_overrides", "web_fetch") + + +@pytest.mark.parametrize("section", [{"fallback_max_chars": True}, {"tool_overrides": {"web_fetch": False}}]) +def test_yaml_loading_rejects_boolean_budgets(tmp_path, section): + path = tmp_path / "config.yaml" + path.write_text(yaml.safe_dump({"sandbox": {"use": "test"}, "tool_output": section}), encoding="utf-8") + + with pytest.raises(ValidationError) as exc: + AppConfig.from_file(path) + + assert exc.value.errors()[0]["loc"][0] == "tool_output" + assert "boolean" in exc.value.errors()[0]["msg"] + + +def test_environment_numeric_strings_and_switches_remain_supported(tmp_path, monkeypatch): + monkeypatch.setenv("OUTPUT_LIMIT", "180") + path = tmp_path / "config.yaml" + path.write_text( + yaml.safe_dump( + { + "sandbox": {"use": "test"}, + "tool_output": { + "enabled": True, + "elide_superseded_writes": False, + "externalize_min_chars": "$OUTPUT_LIMIT", + "tool_overrides": {"web_fetch": "$OUTPUT_LIMIT", "bash": 0}, + }, + } + ), + encoding="utf-8", + ) + + config = AppConfig.from_file(path).tool_output + + assert config.externalize_min_chars == 180 + assert config.tool_overrides == {"web_fetch": 180, "bash": 0} + assert config.enabled is True + assert config.elide_superseded_writes is False + + +@pytest.mark.parametrize("override", [0, "0", 100, "100"]) +@pytest.mark.parametrize("asynchronous", [False, True]) +@pytest.mark.asyncio +async def test_valid_override_controls_real_output_budgeting(tmp_path, override, asynchronous): + content = "evidence line\n" * 100 + message = ToolMessage(content=content, name="web_fetch", tool_call_id="call-1") + request = SimpleNamespace( + tool_call={"name": "web_fetch", "id": "call-1"}, + runtime=SimpleNamespace(state={"thread_data": {"outputs_path": str(tmp_path)}}), + ) + config = ToolOutputConfig( + externalize_min_chars=10_000, + fallback_max_chars=200, + fallback_head_chars=60, + fallback_tail_chars=40, + tool_overrides={"web_fetch": override}, + ) + middleware = ToolOutputBudgetMiddleware(config) + calls = [] + + def handler(actual_request): + calls.append(actual_request) + return message + + async def async_handler(actual_request): + return handler(actual_request) + + if asynchronous: + result = await middleware.awrap_tool_call(request, async_handler) + else: + result = middleware.wrap_tool_call(request, handler) + + assert calls == [request] + assert message.content == content + assert result.tool_call_id == message.tool_call_id + assert result.content != content + stored = [path for path in tmp_path.rglob("*") if path.is_file()] + if int(override) == 0: + assert stored == [] + assert "chars omitted" in result.content + else: + assert len(stored) == 1 + assert stored[0].read_text(encoding="utf-8") == content + + +def test_zero_override_and_zero_fallback_leave_output_unchanged(tmp_path): + message = ToolMessage(content="evidence" * 200, name="web_fetch", tool_call_id="call-1") + request = SimpleNamespace( + tool_call={"name": "web_fetch", "id": "call-1"}, + runtime=SimpleNamespace(state={"thread_data": {"outputs_path": str(tmp_path)}}), + ) + middleware = ToolOutputBudgetMiddleware(ToolOutputConfig(tool_overrides={"web_fetch": 0}, fallback_max_chars=0)) + + result = middleware.wrap_tool_call(request, lambda _: message) + + assert result is message + assert list(tmp_path.iterdir()) == [] diff --git a/deploy/helm/deer-flow/README.md b/deploy/helm/deer-flow/README.md index a03c9d9486..728b2150d2 100644 --- a/deploy/helm/deer-flow/README.md +++ b/deploy/helm/deer-flow/README.md @@ -135,7 +135,7 @@ they resolve from the `secrets` map): ```yaml config: | - config_version: 50 + config_version: 51 models: - name: gpt-4 use: langchain_openai:ChatOpenAI