diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 123e01554..3b79b75d5 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -20,7 +20,7 @@ DeerFlow is a LangGraph-based AI super agent system with a full-stack architectu - Scheduled-task executions must reuse that same Gateway run lifecycle. The scheduler may decide *when* work runs, but it must dispatch through the existing run path rather than introducing a parallel execution stack. - The background scheduler is single-instance by default. `scheduler.multi_instance=true` opts into lease-aware recovery across Gateway instances and requires shared Postgres, `run_ownership.heartbeat_enabled=true`, and `run_events.backend=db`; otherwise startup rejects the configuration. Live scheduled runs are preserved when a peer starts; expired leases are atomically taken over, stale post-launch writes are fenced by the dispatch lease owner, and the Postgres advisory-locked budget makes `max_concurrent_runs` a shared global cap (including pre-launch reservations). - Long-running MCP work uses a separate durable task runtime rather than keeping remote task IDs or status polling inside the Agent loop. Explicit `task_toolsets` bind raw submit/status/cancel names; only submit remains Agent-visible, and its wrapper persists the remote handle before returning a local ID. `McpTaskService` claims due rows with leases, resolves a protocol-specific `McpTaskDriver`, and writes normalized snapshots back to `mcp_tasks`; expired leases are the restart-recovery mechanism, and a result returned after expiry or after a cancel request must be discarded even when the owner token still matches. The first cancel request fences an in-flight poll lease, while repeats preserve an active cancellation lease so they cannot issue concurrent remote cancels; cancellation backoff starts when the remote attempt finishes, so a slow timeout cannot consume the retry delay. Cancellation, polling, and notification batches isolate per-task exceptions; an unexpected cancellation/poll failure leaves that record's lease to expire, while notification failures release only the affected lease for retry. Input-required and terminal event snapshots are delivered by idempotent Agent runs and marked delivered only after run success; the trusted notification instruction stays outside the input boundary while the serialized remote event is framed as untrusted data. A busy-thread conflict is normalized back to the service boundary so the queued snapshot coalesces to the latest task event. A missing dispatched run becomes a failed delivery attempt, while transient run-store hydration errors stay distinguishable and retry the same lookup. The database is the source of truth; `ThreadState` receives only a bounded current-thread projection, and display names are neutralized at that model-state boundary. The installed process-local submitter is the source of truth for management-tool exposure; hot `mcp_tasks` edits take effect only after restart, and active skills must explicitly declare the list/cancel business tools. -- MCP notification failures use a consecutive counter separate from the idempotency-key `dispatch_attempt`, capped exponential backoff, latest-event rebuilding before a run launches, and a five-attempt budget before `dead_letter`. A permanently missing/mismatched target thread is dead-lettered immediately instead of being recreated or reclaimed. HTTP and Agent cancellation requests return after the durable cancel fence; the background loop alone owns the potentially slow remote call and retry schedule. The bounded notification error/count/status join poll and cancellation diagnostics in the task detail API and expanded card. +- MCP notification failures use a consecutive counter separate from the idempotency-key `dispatch_attempt`, capped exponential backoff, latest-event rebuilding before a run launches, and a five-attempt budget before `dead_letter`. A permanently missing/mismatched target thread is dead-lettered immediately instead of being recreated or reclaimed. HTTP and Agent cancellation requests return after the durable cancel fence; the background loop alone owns the potentially slow remote call and retry schedule. The HTTP cancel endpoint rejects requests with 503 when the loop is not running (`mcp_tasks_available` false, e.g. `mcp_tasks.enabled=false` with SQL persistence), so a cancellation is never acknowledged without a worker to perform it. The bounded notification error/count/status join poll and cancellation diagnostics in the task detail API and expanded card. - `extensions_config.json` is written at runtime by the Gateway (`PUT`/`PATCH /api/mcp/config`, the MCP enable switch, skill updates), so the production compose mounts it read-write while `config.yaml` stays `:ro`; Helm copies its ConfigMap seed into a writable home-volume directory before Gateway starts. Every read-modify-write holds both `extensions_config_write_lock` and the sidecar advisory `extensions_config_file_lock`, because the process-local lock alone loses updates across workers. Docker mounts the compose file as its own mount point, and Linux refuses `rename()` over a mount point with `EBUSY` even when the mount is writable — so `atomic_write_extensions_config` keeps the temp-file-plus-rename path and falls back to an in-place overwrite only on `EBUSY`. That fallback is deliberately non-atomic (a crash mid-write truncates the file); it exists because the alternative is a write that can never succeed, and only its first occurrence per target is logged at warning level. Any other `errno` still propagates. Pinned by `tests/test_compose_extensions_config_writable.py`, `tests/test_extensions_config_atomic_write.py`, and `tests/test_helm_extensions_config_writable.py`. - Scheduled-task dispatch enforces "at most one active run per task when `overlap_policy=skip`" at the DB layer via the partial unique index `uq_scheduled_task_run_active` (`scheduled_task_runs.task_id WHERE status IN ('queued','running')`). `ScheduledTaskService.dispatch_task`'s `has_active_runs` check is a non-atomic fast path (its own session, separated from the `create()` insert by `await` points), so two concurrent dispatches — a manual `POST /scheduled-tasks/{id}/trigger` racing the poller, a double-click, or a client retry — can both pass it; the index is the atomic arbiter, and the losing `create` surfaces as `ActiveScheduledRunConflict` (translated from `IntegrityError` in the repository) and collapses to the same outcome as the fast path (manual → 409 conflict, scheduled → a `"skipped"` tombstone). The scheduled-skip tombstone is created directly as terminal `"skipped"` (not a transient `"queued"`) so it never occupies the active slot the pre-existing run still holds. Sibling of the `runs` table's `uq_runs_thread_active` (PR #4003), which keys on `thread_id` and so does not cover the default `fresh_thread_per_run` context where every dispatch gets a new thread. Index is status-only, not `overlap_policy`-conditional (the policy is fixed to `"skip"` in the MVP). diff --git a/backend/app/gateway/routers/mcp_tasks.py b/backend/app/gateway/routers/mcp_tasks.py index c9b358860..3d0e5a2eb 100644 --- a/backend/app/gateway/routers/mcp_tasks.py +++ b/backend/app/gateway/routers/mcp_tasks.py @@ -110,6 +110,12 @@ async def cancel_mcp_task( ) -> dict[str, Any]: service = get_mcp_task_service(request) user_id = await _current_user_id(request) + if not getattr(request.app.state, "mcp_tasks_available", False): + # The service exists whenever SQL persistence is configured, but the + # background loop that owns the remote cancel call only runs when + # mcp_tasks.enabled=true. Recording cancel_requested_at without a + # worker would acknowledge a cancellation nobody will ever perform. + raise HTTPException(status_code=503, detail="MCP task cancellation worker is not running") record = await service.cancel_task( task_id=task_id, thread_id=thread_id, diff --git a/backend/tests/test_mcp_tasks_router.py b/backend/tests/test_mcp_tasks_router.py index 7100ab899..a96c6cdff 100644 --- a/backend/tests/test_mcp_tasks_router.py +++ b/backend/tests/test_mcp_tasks_router.py @@ -161,6 +161,7 @@ async def test_cancel_uses_service_with_exact_user_and_thread_scope(monkeypatch) service.cancel_task.return_value = _record(status="working", cancel_requested_at="2026-08-05T00:00:06+00:00") request = _request(repo) request.app.state.mcp_task_service = service + request.app.state.mcp_tasks_available = True monkeypatch.setattr(mcp_tasks, "get_current_user", AsyncMock(return_value="user-1")) response = await mcp_tasks.cancel_mcp_task.__wrapped__( @@ -176,3 +177,44 @@ async def test_cancel_uses_service_with_exact_user_and_thread_scope(monkeypatch) ) assert response["status"] == "working" assert response["cancel_requested"] is True + + +@pytest.mark.asyncio +async def test_cancel_rejected_when_worker_not_running(monkeypatch) -> None: + repo = FakeRepository([_record()]) + service = AsyncMock() + service.tracking_degraded_after_errors = 3 + request = _request(repo) + request.app.state.mcp_task_service = service + request.app.state.mcp_tasks_available = False + monkeypatch.setattr(mcp_tasks, "get_current_user", AsyncMock(return_value="user-1")) + + with pytest.raises(HTTPException) as excinfo: + await mcp_tasks.cancel_mcp_task.__wrapped__( + thread_id="thread-1", + task_id="mcp-task-1", + request=request, + ) + + assert excinfo.value.status_code == 503 + service.cancel_task.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_cancel_rejected_when_availability_flag_missing(monkeypatch) -> None: + repo = FakeRepository([_record()]) + service = AsyncMock() + service.tracking_degraded_after_errors = 3 + request = _request(repo) + request.app.state.mcp_task_service = service + monkeypatch.setattr(mcp_tasks, "get_current_user", AsyncMock(return_value="user-1")) + + with pytest.raises(HTTPException) as excinfo: + await mcp_tasks.cancel_mcp_task.__wrapped__( + thread_id="thread-1", + task_id="mcp-task-1", + request=request, + ) + + assert excinfo.value.status_code == 503 + service.cancel_task.assert_not_awaited()