diff --git a/backend/packages/harness/deerflow/mcp/AGENTS.md b/backend/packages/harness/deerflow/mcp/AGENTS.md index 648d43e85..76e2039f2 100644 --- a/backend/packages/harness/deerflow/mcp/AGENTS.md +++ b/backend/packages/harness/deerflow/mcp/AGENTS.md @@ -1,7 +1,7 @@ ### MCP System (`packages/harness/deerflow/mcp/`) - Uses `langchain-mcp-adapters` `MultiServerMCPClient` for multi-server management -- **Long-running task foundation**: `mcp/tasks/` defines the protocol-neutral `McpTaskDriver` contract and normalized `TaskSnapshot` states (`submitted`, `working`, `input_required`, `completed`, `failed`, `cancelled`). `persistence/mcp_tasks/` owns the durable remote-handle mapping, poll schedule, notification state, lease owner, and a consecutive poll-error counter (incremented on failed polls, reset on any applied snapshot — the total `poll_attempt_count` grows on every claim and cannot distinguish failure streaks) for a later driver-layer backoff/terminal-failure policy; `app/mcp_tasks/McpTaskService` performs status calls outside the Agent/LLM loop. A status result is applied only when the worker still owns an unexpired lease, so a stale result cannot be written after expiry even before another worker reclaims the row. Poll timestamps and retry schedules are based on the remote call's completion time rather than the scan start. If submission succeeds but persistence fails, the service best-effort cancels the remote task and preserves the original persistence error if that compensation also fails. The exact `uq_mcp_tasks_user_server_remote` conflict is different: an existing durable row already owns the remote handle, so the conflict surfaces without cancelling that tracked task. Unexpected per-task poll failures are isolated from sibling claims and remain recoverable through lease expiry; Gateway shutdown cancels the poller so a hung external status call cannot block process exit. `input_required` and terminal states stop polling and become `notification_status=pending` for later Agent/UI delivery. Durable recovery requires a SQL database backend (`sqlite` or `postgres`); the in-memory backend leaves the repository/service unavailable. The runtime is startup-configured by `mcp_tasks` and disabled by default until a concrete driver is registered; this foundation does not alter ordinary MCP tool behavior on its own. +- **Long-running task foundation**: `mcp/tasks/` defines the protocol-neutral `McpTaskDriver` contract and normalized `TaskSnapshot` states (`submitted`, `working`, `input_required`, `completed`, `failed`, `cancelled`). A driver-supplied `poll_after_seconds` must be a finite positive number, validated at the `TaskSnapshot` boundary so every driver is held to the same invariant rather than each one guarding the consumer that turns the interval into a `timedelta`. `persistence/mcp_tasks/` owns the durable remote-handle mapping, poll schedule, notification state, lease owner, and a consecutive poll-error counter (incremented on failed polls, reset on any applied snapshot — the total `poll_attempt_count` grows on every claim and cannot distinguish failure streaks) for a later driver-layer backoff/terminal-failure policy; `app/mcp_tasks/McpTaskService` performs status calls outside the Agent/LLM loop. A status result is applied only when the worker still owns an unexpired lease, so a stale result cannot be written after expiry even before another worker reclaims the row. Poll timestamps and retry schedules are based on the remote call's completion time rather than the scan start. If submission succeeds but persistence fails, the service best-effort cancels the remote task and preserves the original persistence error if that compensation also fails. The exact `uq_mcp_tasks_user_server_remote` conflict is different: an existing durable row already owns the remote handle, so the conflict surfaces without cancelling that tracked task. Unexpected per-task poll failures are isolated from sibling claims and remain recoverable through lease expiry; Gateway shutdown cancels the poller so a hung external status call cannot block process exit. `input_required` and terminal states stop polling and become `notification_status=pending` for later Agent/UI delivery. Durable recovery requires a SQL database backend (`sqlite` or `postgres`); the in-memory backend leaves the repository/service unavailable. The runtime is startup-configured by `mcp_tasks` and disabled by default until a concrete driver is registered; this foundation does not alter ordinary MCP tool behavior on its own. - **Lazy initialization**: Tools loaded on first use via `get_cached_mcp_tools()` - **Cache invalidation**: Detects extensions-config changes by comparing the resolved config path and a `(mtime, size, sha256)` content signature against the values recorded at initialization, not a strict mtime `>` comparison. This catches same-second edits, mtime that stays put or moves backward (`git checkout`, `cp -p` / backup restore, `tar` / `rsync`, object-store / network mounts), and a switch to a different config file with an equal-or-older mtime. The signature helper (`config/file_signature.py::get_config_signature`) is shared with `config/app_config.py::get_app_config()` for the sibling runtime-editable config file, rather than each maintaining its own copy. `ExtensionsConfig.resolve_config_path()` raises `FileNotFoundError` for an explicit `config_path`/`DEER_FLOW_EXTENSIONS_CONFIG_PATH` that points at a missing file — an operator-asserted path going missing is a real misconfiguration, so this is intentionally loud for callers that load the config for actual use (e.g. `from_file()` via `get_mcp_tools()`); only the fallback search mode returns `None`. The MCP cache's own path resolution (`mcp/cache.py::_resolve_config_path`) is narrower: it catches that specific `FileNotFoundError` locally and treats it the same as "unconfigured", so this staleness check degrades to "not stale" instead of propagating an exception when a previously-valid explicit/env-var config disappears mid-run - **Transports**: stdio (command-based), SSE, HTTP diff --git a/backend/packages/harness/deerflow/mcp/tasks/models.py b/backend/packages/harness/deerflow/mcp/tasks/models.py index 4aacbdd5f..5a49e6ddf 100644 --- a/backend/packages/harness/deerflow/mcp/tasks/models.py +++ b/backend/packages/harness/deerflow/mcp/tasks/models.py @@ -1,5 +1,6 @@ from __future__ import annotations +import math from dataclasses import dataclass, field from enum import StrEnum from typing import Any @@ -50,8 +51,10 @@ class TaskSnapshot: def __post_init__(self) -> None: if not isinstance(self.status, TaskStatus): object.__setattr__(self, "status", TaskStatus(self.status)) - if self.poll_after_seconds is not None and self.poll_after_seconds <= 0: - raise ValueError("poll_after_seconds must be positive") + if self.poll_after_seconds is not None and (not math.isfinite(self.poll_after_seconds) or self.poll_after_seconds <= 0): + # NaN and infinity survive a bare `<= 0` check but break the consumer, + # which turns this interval into a `timedelta` for the next poll. + raise ValueError("poll_after_seconds must be a finite positive number") if self.status == TaskStatus.INPUT_REQUIRED and self.input_required is None: raise ValueError("input_required status requires an input_required payload") diff --git a/backend/tests/test_mcp_task_models.py b/backend/tests/test_mcp_task_models.py index 46074e1d5..6538d369e 100644 --- a/backend/tests/test_mcp_task_models.py +++ b/backend/tests/test_mcp_task_models.py @@ -1,3 +1,5 @@ +from datetime import timedelta + import pytest from deerflow.mcp.tasks import McpTaskDriverRegistry, TaskSnapshot, TaskStatus, TaskSubmission @@ -14,6 +16,24 @@ def test_input_required_snapshot_requires_payload(): TaskSnapshot(status=TaskStatus.INPUT_REQUIRED) +@pytest.mark.parametrize("interval", [float("nan"), float("inf")]) +def test_snapshot_rejects_non_finite_poll_interval(interval): + with pytest.raises(ValueError, match="poll_after_seconds must be a finite positive number"): + TaskSnapshot(status=TaskStatus.WORKING, poll_after_seconds=interval) + + +@pytest.mark.parametrize("interval", [0, -1, float("-inf")]) +def test_snapshot_rejects_non_positive_poll_interval(interval): + with pytest.raises(ValueError, match="poll_after_seconds"): + TaskSnapshot(status=TaskStatus.WORKING, poll_after_seconds=interval) + + +def test_snapshot_keeps_valid_poll_interval_schedulable(): + snapshot = TaskSnapshot(status=TaskStatus.WORKING, poll_after_seconds=12.5) + assert snapshot.poll_after_seconds == 12.5 + assert timedelta(seconds=snapshot.poll_after_seconds) == timedelta(seconds=12.5) + + def test_submission_rejects_empty_remote_id(): with pytest.raises(ValueError, match="remote_task_id must not be empty"): TaskSubmission(remote_task_id=" ", snapshot=TaskSnapshot(status=TaskStatus.SUBMITTED))