diff --git a/backend/app/adapters/schedule/__init__.py b/backend/app/adapters/schedule/__init__.py new file mode 100644 index 000000000..bc75798ac --- /dev/null +++ b/backend/app/adapters/schedule/__init__.py @@ -0,0 +1,20 @@ +"""Adapters of the schedule context. + +Mostly secondary (driven) adapters -- implementations of the output ports the +domain declares, which the service calls out to: + + scheduled_task_repository.py owned persistence + scheduled_run_repository.py owned persistence + run_launcher.py anti-corruption layer over the run runtime + thread_lookup.py anti-corruption layer over the thread store + +One exception, and the name says so: + + run_completion.py PRIMARY (inbound) -- the run runtime calls it + +It lives here so the context stays in one place rather than beside the other +two primary adapters, which sit next to whatever drives them (the router under +`gateway/routers/schedule/`, the poller under `scheduler/`). Direction is +stated by each module's own first line; a file added here without one is a +file whose direction nobody decided. +""" diff --git a/backend/app/adapters/schedule/run_completion.py b/backend/app/adapters/schedule/run_completion.py new file mode 100644 index 000000000..83007371e --- /dev/null +++ b/backend/app/adapters/schedule/run_completion.py @@ -0,0 +1,102 @@ +"""Primary adapter (inbound) -- the run runtime's completion callback. + +The one file in this package that drives the domain rather than serving it. +Its siblings are secondary adapters the service calls out to; this one is +called *by* the run runtime, the same way the router is called by HTTP and the +poller by its clock. Kept here rather than beside those two so the context +stays in one place, with the direction stated by the name and by this line. + +Its job is the filtering the legacy completion hook did inline. Every run in +the process reaches that callback, so most of them are none of this context's +business, and producing no call at all says exactly that -- not an error, just +nothing to write back. That is why `ScheduleService.handle_run_completion` +carries no guard clauses and never imports ``RunRecord``. + +TODO(hexagonal): this depends on ``RunRecord``, a run-runtime type, rather +than on a contract published by the run context -- that context has not been +through a hexagonal slice yet. When it publishes one (a DTO, not its aggregate +and not its repository), replace ``_to_outcome``. Nothing else moves. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import TYPE_CHECKING + +from deerflow.domain.schedule.model import RunStatus +from deerflow.domain.schedule.ports import RunOutcome + +if TYPE_CHECKING: + from deerflow.domain.schedule.service import ScheduleService + from deerflow.runtime import RunRecord + +# The runtime reports four terminal states; the domain has three, because +# `timeout` and `error` are the same fact to a scheduled task while +# `interrupted` is deliberately not -- a cancel or same-thread takeover ends +# the task CANCELLED, not FAILED. +_TERMINAL_STATUSES = { + "success": RunStatus.SUCCESS, + "error": RunStatus.FAILED, + "timeout": RunStatus.FAILED, + "interrupted": RunStatus.INTERRUPTED, +} + +_INTERRUPTED_WITHOUT_ERROR = "run was interrupted before completion" + + +class ScheduleRunCompletionListener: + """Turns a finished run into the write-back use case, or into nothing. + + Deciding whether a run is ours and invoking the use case are one + responsibility, not two: "ignore this run" is only meaningful as "do not + call the service", so splitting them left the second half living in the + composition root, where behaviour is not asserted. + """ + + def __init__(self, service: ScheduleService) -> None: + self._service = service + + async def __call__(self, record: RunRecord) -> None: + outcome = self._to_outcome(record) + if outcome is None: + return + await self._service.handle_run_completion(outcome, now=datetime.now(UTC)) + + @staticmethod + def _to_outcome(record: RunRecord) -> RunOutcome | None: + """Translate into domain vocabulary, or `None` to ignore the run. + + `None` when the run is not a scheduled execution (no usable task + metadata, no owner) or has not reached a terminal state yet. + """ + metadata = record.metadata or {} + task_id = metadata.get("scheduled_task_id") + record_id = metadata.get("scheduled_task_run_id") + user_id = record.user_id + # `metadata` is a free-form dict a caller can influence, so the ids are + # type-checked rather than assumed; `user_id` is required because every + # task read is scoped by it. + if not isinstance(task_id, str) or not isinstance(record_id, str) or not user_id: + return None + + status = _TERMINAL_STATUSES.get(str(record.status.value)) + if status is None: + return None + + if status is RunStatus.SUCCESS: + # A stale error left on a successful record must not be written back + # as the task's last_error. + error = None + elif status is RunStatus.INTERRUPTED: + error = record.error or _INTERRUPTED_WITHOUT_ERROR + else: + error = record.error + + return RunOutcome( + task_id=task_id, + record_id=record_id, + run_id=record.run_id, + user_id=user_id, + status=status, + error=error, + ) diff --git a/backend/app/adapters/schedule/run_launcher.py b/backend/app/adapters/schedule/run_launcher.py new file mode 100644 index 000000000..d3c96b6af --- /dev/null +++ b/backend/app/adapters/schedule/run_launcher.py @@ -0,0 +1,89 @@ +"""Secondary adapter (anti-corruption layer) -- starting a run through Gateway. + +Implements ``RunLauncher`` from ``deerflow.domain.schedule.ports``. This context +owns no part of the run lifecycle: it asks the Gateway to start one and +translates whatever comes back into the two outcomes the domain distinguishes. + +That translation is the reason this file exists. The Gateway signals a busy +thread two different ways -- ``ConflictError`` from the run manager, or an +``HTTPException(409)`` from the route-level path -- and the legacy scheduler +service therefore imported ``fastapi`` to tell them apart. Both are the same +domain fact, and saying so here is what keeps the web framework and the run +runtime out of the inner ring. + +TODO(hexagonal): this depends on ``launch_scheduled_thread_run``, a Gateway +service function returning an untyped dict, rather than on a contract published +by the run context -- that context has not been through a hexagonal slice yet. +When it publishes one (a DTO, not its aggregate and not its repository), +replace the body of this class. The ``RunLauncher`` port does not move. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable, Mapping +from typing import Any + +from fastapi import HTTPException + +from deerflow.domain.schedule.exceptions import LaunchFailedError, ThreadBusyError +from deerflow.domain.schedule.ports import LaunchedRun, RunLauncher +from deerflow.runtime import ConflictError + +LaunchRun = Callable[..., Awaitable[Mapping[str, Any]]] + + +class GatewayRunLauncher(RunLauncher): + """Adapts the Gateway's scheduled-run launch path to the ``RunLauncher`` port. + + Takes the launch callable rather than importing it, because the production + one is bound to the FastAPI app (``launch_scheduled_thread_run(app=app, + ...)``) and that binding belongs to the composition root. + + Explicit inheritance is a readability aid only: a misspelled method would + still instantiate fine and silently inherit the Protocol's ``...`` body, + so the contract tests must call every port method and assert on what it + returns. + """ + + def __init__(self, launch_run: LaunchRun) -> None: + self._launch_run = launch_run + + async def launch( + self, + *, + thread_id: str, + assistant_id: str | None, + prompt: str, + owner_user_id: str | None, + metadata: dict[str, str], + ) -> LaunchedRun: + try: + result = await self._launch_run( + thread_id=thread_id, + assistant_id=assistant_id, + prompt=prompt, + owner_user_id=owner_user_id, + metadata=metadata, + ) + except ConflictError as exc: + raise ThreadBusyError(str(exc)) from exc + except HTTPException as exc: + if exc.status_code == 409: + raise ThreadBusyError(str(exc.detail)) from exc + raise LaunchFailedError(str(exc.detail)) from exc + except Exception as exc: + # Deliberately broad: the port promises the domain that nothing but + # its two errors escapes, so an unclassifiable failure has to become + # the "genuine failure" branch rather than unwinding the poll loop. + # `CancelledError` derives from BaseException and is not caught -- + # shutdown is control flow, not a launch outcome. + raise LaunchFailedError(str(exc)) from exc + + run_id = result.get("run_id") + launched_thread_id = result.get("thread_id") + if not isinstance(run_id, str) or not isinstance(launched_thread_id, str): + # The run path broke its own contract. Reporting it as a failure + # keeps the task's bookkeeping honest instead of recording a launch + # whose run can never be traced. + raise LaunchFailedError(f"run launch returned no usable identity: {result!r}") + return LaunchedRun(run_id=run_id, thread_id=launched_thread_id) diff --git a/backend/app/adapters/schedule/scheduled_run_repository.py b/backend/app/adapters/schedule/scheduled_run_repository.py new file mode 100644 index 000000000..7c55aa427 --- /dev/null +++ b/backend/app/adapters/schedule/scheduled_run_repository.py @@ -0,0 +1,176 @@ +"""Secondary adapter (owned persistence) -- the scheduled_task_runs table in SQL. + +Implements `ScheduledRunRepository` from `deerflow.domain.schedule.ports`. This +context owns the `scheduled_task_runs` table and writes its own queries. +Queries are migrated unchanged from the legacy repository. + +The load-bearing piece is the `IntegrityError` translation in `add`: the +partial unique index `uq_scheduled_task_run_active` is the atomic arbiter of +"at most one active run per task", and turning its rejection into +`ActiveRunConflictError` is what lets the service collapse a lost race into +exactly the same outcome as its own non-atomic fast path. +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from sqlalchemy import func, select +from sqlalchemy.exc import IntegrityError +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from deerflow.domain.schedule.exceptions import ActiveRunConflictError +from deerflow.domain.schedule.model import ACTIVE_RUN_STATUSES, TERMINAL_RUN_STATUSES, RunStatus, ScheduledRun, TriggerKind +from deerflow.domain.schedule.ports import ScheduledRunRepository + +# Transitional: the ORM row stays in the harness until engine, models, and +# migrations move into app/adapters together. +from deerflow.persistence.scheduled_task_runs.model import ScheduledTaskRunRow + +_ACTIVE_STATUS_VALUES = tuple(str(status) for status in ACTIVE_RUN_STATUSES) + + +def _tz_aware(value: datetime | None) -> datetime | None: + """SQLite drops tzinfo on read; stored values are always UTC.""" + return value if value is None or value.tzinfo is not None else value.replace(tzinfo=UTC) + + +class SqlScheduledRunRepository(ScheduledRunRepository): + """SQL implementation of the `ScheduledRunRepository` port.""" + + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._sf = session_factory + + @staticmethod + def _to_domain(row: ScheduledTaskRunRow) -> ScheduledRun: + return ScheduledRun( + record_id=row.id, + task_id=row.task_id, + thread_id=row.thread_id, + scheduled_for=_tz_aware(row.scheduled_for), + trigger=TriggerKind(row.trigger), + status=RunStatus(row.status), + run_id=row.run_id, + error=row.error, + started_at=_tz_aware(row.started_at), + finished_at=_tz_aware(row.finished_at), + created_at=_tz_aware(row.created_at), + ) + + async def add(self, run: ScheduledRun) -> ScheduledRun: + row = ScheduledTaskRunRow( + id=run.record_id, + task_id=run.task_id, + thread_id=run.thread_id, + run_id=run.run_id, + scheduled_for=run.scheduled_for, + trigger=str(run.trigger), + status=str(run.status), + error=run.error, + started_at=run.started_at, + finished_at=run.finished_at, + created_at=run.created_at, + ) + async with self._sf() as session: + session.add(row) + try: + await session.commit() + except IntegrityError: + await session.rollback() + # Only an active-status insert can trip the partial unique + # index; a terminal row (e.g. a skip tombstone) is outside its + # predicate and cannot conflict, so an IntegrityError there is + # a genuine fault and is re-raised untranslated. + if run.is_active: + raise ActiveRunConflictError(f"scheduled task {run.task_id!r} already has an active run") from None + raise + await session.refresh(row) + return self._to_domain(row) + + async def list_by_task(self, task_id: str, *, limit: int = 50, offset: int = 0) -> list[ScheduledRun]: + stmt = ( + select(ScheduledTaskRunRow) + .where(ScheduledTaskRunRow.task_id == task_id) + .order_by( + ScheduledTaskRunRow.created_at.desc(), + ScheduledTaskRunRow.id.desc(), + ) + .limit(limit) + .offset(offset) + ) + async with self._sf() as session: + result = await session.execute(stmt) + return [self._to_domain(row) for row in result.scalars()] + + async def count_active(self) -> int: + """Global count of active rows, used to bound cross-task concurrency.""" + stmt = select(func.count()).select_from(ScheduledTaskRunRow).where(ScheduledTaskRunRow.status.in_(_ACTIVE_STATUS_VALUES)) + async with self._sf() as session: + result = await session.execute(stmt) + return int(result.scalar() or 0) + + async def has_active(self, task_id: str) -> bool: + stmt = ( + select(ScheduledTaskRunRow.id) + .where( + ScheduledTaskRunRow.task_id == task_id, + ScheduledTaskRunRow.status.in_(_ACTIVE_STATUS_VALUES), + ) + .limit(1) + ) + async with self._sf() as session: + result = await session.execute(stmt) + return result.scalars().first() is not None + + async def update_status( + self, + record_id: str, + *, + status: RunStatus, + run_id: str | None = None, + error: str | None = None, + started_at: datetime | None = None, + finished_at: datetime | None = None, + protect_terminal: bool = False, + ) -> None: + async with self._sf() as session: + row = await session.get(ScheduledTaskRunRow, record_id) + if row is None: + return + if protect_terminal and RunStatus(row.status) in TERMINAL_RUN_STATUSES: + # The launch-path "running" write lost the race against the + # completion hook; keep the terminal status/error and only + # backfill bookkeeping the completion write could not know. + if row.run_id is None and run_id is not None: + row.run_id = run_id + if row.started_at is None and started_at is not None: + row.started_at = started_at + await session.commit() + return + row.status = str(status) + row.run_id = run_id + row.error = error + if started_at is not None: + row.started_at = started_at + if finished_at is not None: + row.finished_at = finished_at + await session.commit() + + async def mark_stale_active(self, *, error: str) -> int: + """Fail-fast bookkeeping for runs orphaned by a process crash. + + Agent runs execute in-process, so any active row found at scheduler + startup belongs to a run whose process is gone. Only valid under the + single-scheduler-instance assumption. + """ + stmt = select(ScheduledTaskRunRow).where(ScheduledTaskRunRow.status.in_(_ACTIVE_STATUS_VALUES)) + now = datetime.now(UTC) + async with self._sf() as session: + result = await session.execute(stmt) + rows = list(result.scalars()) + for row in rows: + row.status = str(RunStatus.INTERRUPTED) + row.error = error + row.finished_at = now + await session.commit() + return len(rows) diff --git a/backend/app/adapters/schedule/scheduled_task_repository.py b/backend/app/adapters/schedule/scheduled_task_repository.py new file mode 100644 index 000000000..6741bc573 --- /dev/null +++ b/backend/app/adapters/schedule/scheduled_task_repository.py @@ -0,0 +1,341 @@ +"""Secondary adapter (owned persistence) -- the scheduled_tasks table in SQL. + +Implements `ScheduledTaskRepository` from `deerflow.domain.schedule.ports`. +This context owns the `scheduled_tasks` table and writes its own queries, so +SQL/ORM vocabulary stops at this file: methods exchange domain objects and +normalize SQLite's tz-naive reads. + +Sibling of `scheduled_run_repository.py`. Translating the row is this class's +own private business, `_to_domain` / `_apply` and the two `schedule_spec` +helpers beside them -- the HTTP shape has its own translation next to the +router, and neither side imports the other. + +**The queries are migrated unchanged from the legacy repository.** The claim +statement's `FOR UPDATE SKIP LOCKED` and the `protect_terminal` conditional +write are the module's concurrency contract, not style choices -- rewriting +them is how the invariants silently break. +""" + +from __future__ import annotations + +import logging +import socket +import uuid +from datetime import UTC, datetime, timedelta + +from sqlalchemy import and_, or_, select +from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + +from deerflow.domain.schedule.exceptions import CorruptStoredScheduleError, ScheduleError +from deerflow.domain.schedule.model import TERMINAL_TASK_STATUSES, ContextMode, ScheduledTask, ScheduleSpec, ScheduleType, TaskStatus +from deerflow.domain.schedule.ports import ScheduledTaskRepository + +# Transitional: the ORM row stays in the harness until engine, models, and +# migrations move into app/adapters together. +from deerflow.persistence.scheduled_tasks.model import ScheduledTaskRow + +logger = logging.getLogger(__name__) + + +def _tz_aware(value: datetime | None) -> datetime | None: + """SQLite drops tzinfo on read; stored values are always UTC.""" + return value if value is None or value.tzinfo is not None else value.replace(tzinfo=UTC) + + +class SqlScheduledTaskRepository(ScheduledTaskRepository): + """SQL implementation of the `ScheduledTaskRepository` port. + + Explicit inheritance is a readability aid only: a missing method would + still instantiate fine (Protocol bodies are inherited), so the contract + test suite must cover every port method. + """ + + def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: + self._sf = session_factory + # Diagnostics only. The domain deliberately does not carry this -- who + # claimed a task is an identity, not a rule, and nothing reads it back. + self._lease_owner = f"{socket.gethostname()}:{uuid.uuid4().hex}" + + # ------------------------------------------------------------ conversion + + @staticmethod + def _spec_from_row(row: ScheduledTaskRow) -> ScheduleSpec: + """The stored triple -> the value object. + + All this side owns is which two keys this table's JSON uses; the rule + for turning them into a schedule belongs to the domain, which is why + the router's own mapping can state the same thing without either + importing the other. + """ + stored = row.schedule_spec or {} + return ScheduleSpec.from_primitives( + row.schedule_type, + cron=stored.get("cron"), + run_at=stored.get("run_at"), + timezone=row.timezone, + ) + + @staticmethod + def _spec_to_column(spec: ScheduleSpec) -> dict[str, str]: + """The value object -> the stored JSON. + + Emits the normalized value rather than echoing the caller's bytes: a + trailing-Z input is stored as "+00:00". Both forms parse back, so this + is deliberate -- preferable to carrying the raw dict on the value + object just to preserve the exact input spelling. + """ + if spec.schedule_type is ScheduleType.CRON: + return {"cron": spec.cron or ""} + return {"run_at": spec.run_at.isoformat() if spec.run_at else ""} + + @staticmethod + def _to_domain(row: ScheduledTaskRow) -> ScheduledTask: + """ORM row -> aggregate. + + Raises CorruptStoredScheduleError for a row that can no longer be + rebuilt -- unparseable schedule, unknown enum value. The rebuild goes + through the aggregate's own validation, so a hand-damaged row blows up + here rather than flowing on; the translation to the dedicated error is + what keeps it off ``InvalidScheduleError``'s client-facing 422 mapping + (a stored fault is the server's problem, and PATCH -- which reads + before writing -- must not become unusable for the one row it could + repair). Single-row reads let it propagate; list reads skip and log, + so one bad row cannot take down an entire listing. + """ + try: + return ScheduledTask( + task_id=row.id, + user_id=row.user_id, + title=row.title, + prompt=row.prompt, + schedule=SqlScheduledTaskRepository._spec_from_row(row), + context_mode=ContextMode(row.context_mode), + thread_id=row.thread_id, + assistant_id=row.assistant_id, + status=TaskStatus(row.status), + overlap_policy=row.overlap_policy, + next_run_at=_tz_aware(row.next_run_at), + last_run_at=_tz_aware(row.last_run_at), + last_run_id=row.last_run_id, + last_thread_id=row.last_thread_id, + last_error=row.last_error, + run_count=row.run_count, + created_at=_tz_aware(row.created_at), + updated_at=_tz_aware(row.updated_at), + ) + except (ScheduleError, ValueError) as exc: + raise CorruptStoredScheduleError(f"stored scheduled task {row.id} cannot be rebuilt: {exc}") from exc + + @classmethod + def _to_domain_or_skip(cls, row: ScheduledTaskRow) -> ScheduledTask | None: + try: + return cls._to_domain(row) + except CorruptStoredScheduleError: + logger.warning("Skipping scheduled task %s: stored row cannot be rebuilt", row.id, exc_info=True) + return None + + @staticmethod + def _apply(row: ScheduledTaskRow, task: ScheduledTask) -> None: + """Aggregate -> ORM row. Explicit field list: a new column stays + invisible here until it is deliberately mapped.""" + row.user_id = task.user_id + row.title = task.title + row.prompt = task.prompt + row.schedule_type = str(task.schedule.schedule_type) + row.schedule_spec = SqlScheduledTaskRepository._spec_to_column(task.schedule) + row.timezone = task.schedule.timezone + row.context_mode = str(task.context_mode) + row.thread_id = task.thread_id + row.assistant_id = task.assistant_id + row.status = str(task.status) + row.overlap_policy = task.overlap_policy + row.next_run_at = task.next_run_at + row.last_run_at = task.last_run_at + row.last_run_id = task.last_run_id + row.last_thread_id = task.last_thread_id + row.last_error = task.last_error + row.run_count = task.run_count + + # ------------------------------------------------------------------ port + + async def add(self, task: ScheduledTask) -> ScheduledTask: + # The aggregate's construction instant is the one truth for both + # bookkeeping stamps on insert; the later write paths own updated_at. + row = ScheduledTaskRow(id=task.task_id, created_at=task.created_at, updated_at=task.updated_at) + self._apply(row, task) + async with self._sf() as session: + session.add(row) + await session.commit() + await session.refresh(row) + return self._to_domain(row) + + async def get(self, task_id: str, *, user_id: str) -> ScheduledTask | None: + async with self._sf() as session: + row = await session.get(ScheduledTaskRow, task_id) + if row is None or row.user_id != user_id: + return None + return self._to_domain(row) + + async def list_by_user(self, user_id: str) -> list[ScheduledTask]: + stmt = select(ScheduledTaskRow).where(ScheduledTaskRow.user_id == user_id).order_by(ScheduledTaskRow.created_at.desc(), ScheduledTaskRow.id.desc()) + async with self._sf() as session: + result = await session.execute(stmt) + return [task for task in (self._to_domain_or_skip(row) for row in result.scalars()) if task is not None] + + async def list_by_user_and_thread(self, user_id: str, thread_id: str) -> list[ScheduledTask]: + stmt = ( + select(ScheduledTaskRow) + .where( + ScheduledTaskRow.user_id == user_id, + ScheduledTaskRow.thread_id == thread_id, + ) + .order_by(ScheduledTaskRow.created_at.desc(), ScheduledTaskRow.id.desc()) + ) + async with self._sf() as session: + result = await session.execute(stmt) + return [task for task in (self._to_domain_or_skip(row) for row in result.scalars()) if task is not None] + + async def save(self, task: ScheduledTask) -> ScheduledTask | None: + async with self._sf() as session: + row = await session.get(ScheduledTaskRow, task.task_id) + if row is None or row.user_id != task.user_id: + return None + self._apply(row, task) + row.updated_at = datetime.now(UTC) + await session.commit() + await session.refresh(row) + return self._to_domain(row) + + async def delete(self, task_id: str, *, user_id: str) -> bool: + async with self._sf() as session: + row = await session.get(ScheduledTaskRow, task_id) + if row is None or row.user_id != user_id: + return False + await session.delete(row) + await session.commit() + return True + + async def claim_due(self, *, now: datetime, lease_seconds: int, limit: int) -> list[ScheduledTask]: + lease_expires_at = now + timedelta(seconds=lease_seconds) + stmt = ( + select(ScheduledTaskRow) + .where( + ScheduledTaskRow.next_run_at.is_not(None), + ScheduledTaskRow.next_run_at <= now, + or_( + and_( + ScheduledTaskRow.status == "enabled", + or_( + ScheduledTaskRow.lease_expires_at.is_(None), + ScheduledTaskRow.lease_expires_at < now, + ), + ), + # A task stuck in "running" with an expired lease means the + # claiming process died between claim and dispatch; it must + # stay reclaimable or the task is dead forever. + and_( + ScheduledTaskRow.status == "running", + ScheduledTaskRow.lease_expires_at.is_not(None), + ScheduledTaskRow.lease_expires_at < now, + ), + ), + ) + .order_by(ScheduledTaskRow.next_run_at.asc(), ScheduledTaskRow.id.asc()) + .limit(limit) + .with_for_update(skip_locked=True) + ) + async with self._sf() as session: + result = await session.execute(stmt) + rows = list(result.scalars()) + for row in rows: + row.lease_owner = self._lease_owner + row.lease_expires_at = lease_expires_at + row.status = "running" + row.updated_at = datetime.now(UTC) + await session.commit() + return [task for task in (self._to_domain_or_skip(row) for row in rows) if task is not None] + + async def record_launch( + self, + task_id: str, + *, + status: TaskStatus, + next_run_at: datetime | None, + last_run_at: datetime | None, + last_run_id: str | None, + last_thread_id: str | None, + last_error: str | None, + increment_run_count: bool, + protect_terminal: bool = False, + ) -> None: + async with self._sf() as session: + row = await session.get(ScheduledTaskRow, task_id) + if row is None: + return + if protect_terminal and TaskStatus(row.status) in TERMINAL_TASK_STATUSES: + # A fast-failing run can reach the completion hook (which + # finalizes a `once` task) before this launch-path write + # commits; keep the hook's status/error and only record the + # launch bookkeeping. + pass + else: + row.status = str(status) + row.last_error = last_error + row.next_run_at = next_run_at + row.last_run_at = last_run_at + row.last_run_id = last_run_id + row.last_thread_id = last_thread_id + if increment_run_count: + row.run_count += 1 + row.lease_owner = None + row.lease_expires_at = None + row.updated_at = datetime.now(UTC) + await session.commit() + + async def record_completion( + self, + task_id: str, + *, + user_id: str, + status: TaskStatus | None, + error: str | None, + ) -> None: + async with self._sf() as session: + row = await session.get(ScheduledTaskRow, task_id) + if row is None or row.user_id != user_id: + return + # Field-level on purpose: `record_launch` may commit on either side + # of this write, and the two must not undo each other. Assigning + # anything below `last_error` here is what reintroduces the race -- + # see the port docstring. + row.last_error = error + if status is not None: + row.status = str(status) + row.updated_at = datetime.now(UTC) + await session.commit() + + async def cancel_stuck_once_tasks(self, *, error: str) -> int: + """Reconcile `once` tasks orphaned in `running` by a process crash. + + A launched `once` task stays `running` until the in-process completion + hook moves it to a terminal status; its lease was cleared at launch, so + the claim query's expired-lease branch never sees it. After a crash the + hook is gone and the task would be stuck forever. Tasks still holding a + lease are left alone -- they were claimed but not launched, and + expired-lease reclaim recovers them safely. + """ + stmt = select(ScheduledTaskRow).where( + ScheduledTaskRow.schedule_type == "once", + ScheduledTaskRow.status == "running", + ScheduledTaskRow.lease_expires_at.is_(None), + ) + async with self._sf() as session: + result = await session.execute(stmt) + rows = list(result.scalars()) + now = datetime.now(UTC) + for row in rows: + row.status = "cancelled" + row.last_error = error + row.updated_at = now + await session.commit() + return len(rows) diff --git a/backend/app/adapters/schedule/thread_lookup.py b/backend/app/adapters/schedule/thread_lookup.py new file mode 100644 index 000000000..9192f6f40 --- /dev/null +++ b/backend/app/adapters/schedule/thread_lookup.py @@ -0,0 +1,46 @@ +"""Secondary adapter (anti-corruption layer) -- narrowing ThreadMetaStore to ThreadLookup. + +Implements ``ThreadLookup`` from ``deerflow.domain.schedule.ports``. This +context does not own the ``threads_meta`` table and writes no SQL against it: it +asks its one question through the store the thread context already provides. + +``require_existing=True`` is the load-bearing argument. The store's default +treats an absent row as accessible -- reasonable for a thread that has not been +written yet, wrong for binding a task to it, since the task would then reference +a thread that never existed. + +TODO(hexagonal): this depends on ``ThreadMetaStore``, an infrastructure +component, rather than on a contract published by the thread context -- that +context has not been through a hexagonal slice yet. When it publishes one (a +DTO, not its aggregate and not its repository), replace the body of this class. +The ``ThreadLookup`` port does not move. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from deerflow.domain.schedule.ports import ThreadLookup + +if TYPE_CHECKING: + from deerflow.persistence.thread_meta.base import ThreadMetaStore + + +class ThreadStoreThreadLookup(ThreadLookup): + """Adapts the wide ``ThreadMetaStore`` to the one question this context asks. + + Both halves of that question -- does the thread exist, and may this user use + it -- collapse into a single bool on purpose: reporting them separately + would let a caller probe for the existence of threads they cannot see. + + Explicit inheritance is a readability aid only: a misspelled method would + still instantiate fine and silently inherit the Protocol's ``...`` body, + so the contract tests must call every port method and assert on what it + returns. + """ + + def __init__(self, thread_store: ThreadMetaStore) -> None: + self._thread_store = thread_store + + async def exists_for_user(self, thread_id: str, user_id: str) -> bool: + return bool(await self._thread_store.check_access(thread_id, user_id, require_existing=True)) diff --git a/backend/app/channels/manager.py b/backend/app/channels/manager.py index 72be91337..fd50727ad 100644 --- a/backend/app/channels/manager.py +++ b/backend/app/channels/manager.py @@ -930,7 +930,7 @@ class ChannelManager: self._require_bound_identity = require_bound_identity # Zero-arg accessor for the FastAPI app's StreamBridge singleton, # threaded in from app.py's lifespan via start_channel_service() -> - # ChannelService.__init__ (mirrors how ScheduledTaskService gets a + # ChannelService.__init__ (mirrors how the schedule composition root gets a # launch_run closure over `app` in the same lifespan function). None # when not wired (e.g. a ChannelManager constructed directly in # tests) — follow-up buffering still works, but no watcher is diff --git a/backend/app/channels/service.py b/backend/app/channels/service.py index f46724beb..bab30d9cc 100644 --- a/backend/app/channels/service.py +++ b/backend/app/channels/service.py @@ -441,7 +441,7 @@ async def start_channel_service( ``ChannelRunPolicy.buffer_followups_on_busy`` (currently GitHub) can watch a run's completion and auto-drain buffered follow-ups. ``app.py``'s lifespan passes a closure over ``app.state.stream_bridge`` here, the same - pattern it already uses for ``ScheduledTaskService``'s ``launch_run``. + pattern it already uses for the schedule composition root's ``launch_run``. """ global _channel_service if _channel_service is not None: diff --git a/backend/app/composition.py b/backend/app/composition.py new file mode 100644 index 000000000..0f38cecee --- /dev/null +++ b/backend/app/composition.py @@ -0,0 +1,119 @@ +"""Composition root -- the one place adapters are instantiated and wired. + +Every application service is assembled here and nowhere else. Ports are +declared by the domain, implemented under ``app/adapters/``, and the two are +introduced to each other in this file; no router, middleware, or lifespan hook +constructs an adapter of its own. + +**Why this is a pure function rather than part of the lifespan.** Wiring used +to live inside ``deps.py::langgraph_runtime``, tangled with engine startup, +orphan recovery, and graceful shutdown -- so the one rule that actually +governs it ("no SQL backend means no service, and the routes answer 503") +could not be tested without driving a full application startup, and was held +up by a single comment. ``build_domain_services`` takes what it needs and +returns what it built, so that rule is an assertion in +``tests/test_composition.py`` instead. + +**What "no SQL backend" means.** ``session_factory is None`` is how a +``database.backend: memory`` deployment presents itself. The context owns +tables, so it cannot run on it; the service is ``None`` and the dependency +providers translate that into 503. This is deliberately not a silent +degradation to an in-memory implementation -- scheduled work that vanishes on +restart is worse than scheduled work that is refused. +""" + +from __future__ import annotations + +from collections.abc import Awaitable, Callable, Mapping +from dataclasses import dataclass +from typing import TYPE_CHECKING, Any + +from deerflow.domain.schedule.model import SchedulePolicy +from deerflow.domain.schedule.service import ScheduleService + +if TYPE_CHECKING: + from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker + + from deerflow.config.scheduler_config import SchedulerConfig + from deerflow.persistence.thread_meta.base import ThreadMetaStore + + +@dataclass(frozen=True) +class DomainServices: + """Every application service the Gateway serves, or ``None`` where the + configured backend cannot support one.""" + + schedule: ScheduleService | None + + +def build_domain_services( + *, + session_factory: async_sessionmaker[AsyncSession] | None, + thread_store: ThreadMetaStore, + launch_run: Callable[..., Awaitable[Mapping[str, Any]]], + scheduler_config: SchedulerConfig, +) -> DomainServices: + """Wire the domain services from already-built infrastructure. + + Takes infrastructure rather than building it: engines, stores and the run + launcher have lifecycles (startup, recovery, shutdown) that belong to the + lifespan, while deciding *what is assembled from them* is this function's + only job. That split is what makes it callable from a test. + """ + if session_factory is None: + return DomainServices(schedule=None) + + # Imported here, not at module scope: these pull in SQLAlchemy and the + # Gateway run path, and the composition root is imported by tests that + # only want the memory-backend branch. + from app.adapters.schedule.run_launcher import GatewayRunLauncher + from app.adapters.schedule.scheduled_run_repository import SqlScheduledRunRepository + from app.adapters.schedule.scheduled_task_repository import SqlScheduledTaskRepository + from app.adapters.schedule.thread_lookup import ThreadStoreThreadLookup + + return DomainServices( + schedule=ScheduleService( + tasks=SqlScheduledTaskRepository(session_factory), + runs=SqlScheduledRunRepository(session_factory), + launcher=GatewayRunLauncher(launch_run), + threads=ThreadStoreThreadLookup(thread_store), + policy=build_schedule_policy(scheduler_config), + ), + ) + + +def build_run_completion_hook( + schedule_service: ScheduleService | None, +) -> Callable[[Any], Awaitable[None]] | None: + """Install the schedule context on the run runtime's completion callback. + + The inbound half of the wiring, and nothing more: which runs the context + cares about and what it does with them belongs to the adapter, not here. + This function's only decision is the same one the rest of this module + makes -- what to assemble, and what to leave unassembled. + + Returns ``None`` when there is no service, so the runtime installs no hook + at all rather than one that always declines. + """ + if schedule_service is None: + return None + + from app.adapters.schedule.run_completion import ScheduleRunCompletionListener + + return ScheduleRunCompletionListener(schedule_service) + + +def build_schedule_policy(scheduler_config: SchedulerConfig) -> SchedulePolicy: + """Project the operator's scheduler config onto the domain's policy. + + Separate from the wiring above because it is the whole of the + configuration-to-domain translation: the domain declares which thresholds + it needs, and this names where each one comes from. `poll_interval_seconds` + is deliberately absent -- how often to look is the poller's business, not a + rule any task is subject to. + """ + return SchedulePolicy( + min_once_delay_seconds=scheduler_config.min_once_delay_seconds, + max_concurrent_runs=scheduler_config.max_concurrent_runs, + lease_seconds=scheduler_config.lease_seconds, + ) diff --git a/backend/app/gateway/app.py b/backend/app/gateway/app.py index 705f9be7d..08f0d0246 100644 --- a/backend/app/gateway/app.py +++ b/backend/app/gateway/app.py @@ -30,13 +30,13 @@ from app.gateway.routers import ( memory, models, runs, - scheduled_tasks, skills, suggestions, thread_runs, threads, uploads, ) +from app.gateway.routers.schedule import router as schedule_router from app.gateway.trace_middleware import TraceMiddleware, resolve_trace_enabled from deerflow.config import app_config as deerflow_app_config from deerflow.logging_config import DEFAULT_LOG_DATE_FORMAT, DEFAULT_LOG_FORMAT, configure_logging @@ -287,7 +287,7 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: try: from app.channels.service import start_channel_service - # Closure over `app` (mirrors ScheduledTaskService's `launch_run` + # Closure over `app` (mirrors the schedule composition root's `launch_run` # below) rather than resolving `app.state.stream_bridge` here # directly: `stream_bridge` is a STARTUP_ONLY_FIELDS singleton set # once, above, by `langgraph_runtime(app, startup_config)`, so @@ -305,23 +305,23 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: logger.exception("No IM channels configured or channel service failed to start") try: - from app.gateway.services import launch_scheduled_thread_run - from app.scheduler import ScheduledTaskService + # The service itself was assembled by the composition root inside + # `langgraph_runtime`; all that is left here is the clock that + # drives it. It is None when the configured backend cannot support + # scheduling, in which case there is nothing to poll. + from app.scheduler.poller import SchedulePoller - if getattr(app.state, "scheduled_task_repo", None) is not None and getattr(app.state, "scheduled_task_run_repo", None) is not None: - scheduled_task_service = ScheduledTaskService( - task_repo=app.state.scheduled_task_repo, - task_run_repo=app.state.scheduled_task_run_repo, - launch_run=lambda **kwargs: launch_scheduled_thread_run(app=app, **kwargs), + schedule_service = getattr(app.state, "schedule_service", None) + if schedule_service is not None: + schedule_poller = SchedulePoller( + schedule_service, poll_interval_seconds=startup_config.scheduler.poll_interval_seconds, - lease_seconds=startup_config.scheduler.lease_seconds, - max_concurrent_runs=startup_config.scheduler.max_concurrent_runs, ) - app.state.scheduled_task_service = scheduled_task_service + app.state.schedule_poller = schedule_poller if startup_config.scheduler.enabled: - await scheduled_task_service.start() + await schedule_poller.start() except Exception: - logger.exception("Failed to initialize scheduled task service") + logger.exception("Failed to start the scheduled task poller") yield @@ -346,11 +346,11 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: except Exception: logger.exception("Failed to stop channel service") - if getattr(app.state, "scheduled_task_service", None) is not None: + if getattr(app.state, "schedule_poller", None) is not None: try: - await app.state.scheduled_task_service.stop() + await app.state.schedule_poller.stop() except Exception: - logger.exception("Failed to stop scheduled task service") + logger.exception("Failed to stop the scheduled task poller") try: from deerflow.community.browser_automation import get_browser_session_manager @@ -598,7 +598,7 @@ This gateway provides runtime endpoints for agent runs plus custom endpoints for app.include_router(threads.router) # Scheduled tasks API is mounted at /api/scheduled-tasks - app.include_router(scheduled_tasks.router) + app.include_router(schedule_router.router) # Agents API is mounted at /api/agents app.include_router(agents.router) diff --git a/backend/app/gateway/deps.py b/backend/app/gateway/deps.py index 9c5540b80..b293ee863 100644 --- a/backend/app/gateway/deps.py +++ b/backend/app/gateway/deps.py @@ -22,13 +22,14 @@ import logging import os from collections.abc import AsyncGenerator, Callable from contextlib import AsyncExitStack, asynccontextmanager -from typing import TYPE_CHECKING, TypeVar, cast +from typing import TYPE_CHECKING, Annotated, TypeVar, cast -from fastapi import FastAPI, HTTPException, Request +from fastapi import Depends, FastAPI, HTTPException, Request from langgraph.types import Checkpointer from deerflow.community.browser_automation.session import browser_multi_worker_error from deerflow.config.app_config import AppConfig, get_app_config +from deerflow.domain.schedule.service import ScheduleService from deerflow.persistence.feedback import FeedbackRepository from deerflow.runtime import ORPHAN_RECOVERY_STOP_REASON, STARTUP_ORPHAN_RECOVERY_ERROR, RunContext, RunManager, StreamBridge from deerflow.runtime.events.store.base import RunEventStore @@ -415,17 +416,25 @@ async def langgraph_runtime(app: FastAPI, startup_config: AppConfig) -> AsyncGen from deerflow.persistence.thread_meta import make_thread_store app.state.thread_store = make_thread_store(sf, app.state.store) - if sf is not None: - from deerflow.persistence.scheduled_task_runs import ( - ScheduledTaskRunRepository, - ) - from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository - app.state.scheduled_task_repo = ScheduledTaskRepository(sf) - app.state.scheduled_task_run_repo = ScheduledTaskRunRepository(sf) - else: - app.state.scheduled_task_repo = None - app.state.scheduled_task_run_repo = None + # Hexagonal slices: every adapter is instantiated by the composition + # root and nowhere else. It is a pure function of the infrastructure + # built above, so what it decides — including "no SQL backend means no + # service, and the routes answer 503" — is covered by + # tests/test_composition.py rather than only by this call site. + from app.composition import build_domain_services, build_run_completion_hook + from app.gateway.services import launch_scheduled_thread_run + + domain_services = build_domain_services( + session_factory=sf, + thread_store=app.state.thread_store, + launch_run=lambda **kwargs: launch_scheduled_thread_run(app=app, **kwargs), + scheduler_config=config.scheduler, + ) + app.state.schedule_service = domain_services.schedule + # Built once here rather than per request: it closes over the service, + # and every RunContext needs the same one. + app.state.run_completion_hook = build_run_completion_hook(domain_services.schedule) # Run event store. The store and the matching ``run_events_config`` are # both frozen at startup so ``get_run_context`` does not combine a @@ -556,25 +565,23 @@ def get_thread_store(request: Request) -> ThreadMetaStore: return val -def get_scheduled_task_repo(request: Request): - val = getattr(request.app.state, "scheduled_task_repo", None) +def get_schedule_service(request: Request) -> ScheduleService: + """The schedule context's input port. + + ``None`` here means the composition root refused to build it, which today + means a non-SQL backend -- see ``app/composition.py`` for why that is a + 503 rather than a degraded in-memory implementation. + """ + val = getattr(request.app.state, "schedule_service", None) if val is None: - raise HTTPException(status_code=503, detail="Scheduled task repo not available") + raise HTTPException(status_code=503, detail="Scheduled tasks require a SQL database backend") return val -def get_scheduled_task_run_repo(request: Request): - val = getattr(request.app.state, "scheduled_task_run_repo", None) - if val is None: - raise HTTPException(status_code=503, detail="Scheduled task run repo not available") - return val - - -def get_scheduled_task_service(request: Request): - val = getattr(request.app.state, "scheduled_task_service", None) - if val is None: - raise HTTPException(status_code=503, detail="Scheduled task service not available") - return val +# Declared as an alias so routes take the service without a default argument +# (leaving parameter order unconstrained) and so swapping the provider is a +# one-line change here rather than an edit to every handler signature. +ScheduleServiceDep = Annotated[ScheduleService, Depends(get_schedule_service)] def get_run_context(request: Request) -> RunContext: @@ -596,7 +603,7 @@ def get_run_context(request: Request) -> RunContext: checkpoint_snapshot_frequency=getattr(request.app.state, "checkpoint_snapshot_frequency", None), thread_store=get_thread_store(request), app_config=get_config(), - on_run_completed=getattr(request.app.state, "scheduled_task_service", None).handle_run_completion if getattr(request.app.state, "scheduled_task_service", None) is not None else None, + on_run_completed=getattr(request.app.state, "run_completion_hook", None), ) diff --git a/backend/app/gateway/routers/__init__.py b/backend/app/gateway/routers/__init__.py index a948c522b..27afb4341 100644 --- a/backend/app/gateway/routers/__init__.py +++ b/backend/app/gateway/routers/__init__.py @@ -5,7 +5,6 @@ from . import ( input_polish, mcp, models, - scheduled_tasks, skills, suggestions, thread_runs, @@ -20,7 +19,6 @@ __all__ = [ "input_polish", "mcp", "models", - "scheduled_tasks", "skills", "suggestions", "threads", diff --git a/backend/app/gateway/routers/schedule/__init__.py b/backend/app/gateway/routers/schedule/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/backend/app/gateway/routers/schedule/models.py b/backend/app/gateway/routers/schedule/models.py new file mode 100644 index 000000000..e2cb833a7 --- /dev/null +++ b/backend/app/gateway/routers/schedule/models.py @@ -0,0 +1,263 @@ +"""The HTTP shapes of the scheduled-task API. + +The primary adapter's own model of what a client sends and receives -- the +counterpart to the domain aggregates, not a view of them. Two things follow +from that: + +**Responses are an allowlist, not a dump.** The pre-migration router returned +the ORM row's ``to_dict()``, which leaked ``user_id``, ``lease_owner``, +``lease_expires_at``, ``overlap_policy`` and ``assistant_id`` -- lease fields +are scheduler-internal bookkeeping, and the other three are server-owned. None +appear in the frontend's ``ScheduledTask`` type or anywhere in its code, so +naming the fields explicitly here closes the leak without a client change. A +field added to the aggregate from now on stays invisible until it is +deliberately published. + +**Timestamps keep the legacy spelling.** The legacy path emitted +``coerce_iso`` -> ``astimezone(UTC).isoformat()``, i.e. +``2026-08-01T09:00:00+00:00``. Pydantic v2 would serialize the same instant as +``...T09:00:00Z``, which is a silent wire change for every client parsing +these, so ``UtcTimestamp`` pins ``isoformat()`` explicitly. Both spellings are +valid ISO 8601 and JS ``Date`` accepts either -- the point is that changing it +is a decision, not a side effect of adopting a model. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import Annotated, Any + +from pydantic import BaseModel, Field, PlainSerializer + +from deerflow.domain.schedule.commands import UNSET, ContextChange, CreateScheduledTask, UpdateScheduledTask +from deerflow.domain.schedule.model import ScheduledRun, ScheduledTask, ScheduleSpec, ScheduleType + +UtcTimestamp = Annotated[datetime, PlainSerializer(lambda value: value.isoformat(), return_type=str)] + + +def _utc(value: datetime | None) -> datetime | None: + """Match the legacy `coerce_iso` normalisation exactly.""" + if value is None: + return None + return value.astimezone(UTC) if value.tzinfo is not None else value.replace(tzinfo=UTC) + + +def _spec_to_wire(spec: ScheduleSpec) -> dict[str, str]: + """The value object -> the `schedule_spec` body field. + + Emits the normalized value rather than echoing the caller's bytes: the + frontend submits an already-UTC-aware ISO value (`zonedLocalToUtcIso`), so + a trailing-Z input comes back as "+00:00". Both forms parse on either side, + so the normalization is deliberate. + + All this side owns is which two keys the body uses. Its counterpart is + `SqlScheduledTaskRepository._spec_to_column`; the two are near-identical + today by coincidence, and are kept apart because a primary adapter must not + import a secondary one and because the day the API grows a field the column + does not have, they diverge without either having to be untangled. + """ + if spec.schedule_type is ScheduleType.CRON: + return {"cron": spec.cron or ""} + return {"run_at": spec.run_at.isoformat() if spec.run_at else ""} + + +class CreateScheduledTaskRequest(BaseModel): + """Body of ``POST /scheduled-tasks``. + + No identity field: ``user_id`` is resolved server-side and injected + through ``to_command``, never read off the wire. + """ + + thread_id: str | None = None + context_mode: str = "fresh_thread_per_run" + title: str = Field(min_length=1) + prompt: str = Field(min_length=1) + schedule_type: str + schedule_spec: dict[str, Any] + timezone: str + + def to_command(self, user_id: str) -> CreateScheduledTask: + """Wire shape -> command (transformation ① of the chain).""" + return CreateScheduledTask( + user_id=user_id, + title=self.title, + prompt=self.prompt, + schedule=self.to_schedule(), + context_mode=self.context_mode, + thread_id=self.thread_id, + ) + + def to_schedule(self) -> ScheduleSpec: + """Parse the submitted triple into the value object. + + Raises `InvalidScheduleError` -- a *domain* error out of a primary + adapter, on purpose: it is the vocabulary the outer ring uses to say + "this violates a domain rule", and the router maps that one family onto + 422. Structural problems (key missing, wrong type) and value problems + (5-field cron, resolvable timezone) both arrive as it. + """ + return ScheduleSpec.from_primitives( + self.schedule_type, + cron=self.schedule_spec.get("cron"), + run_at=self.schedule_spec.get("run_at"), + timezone=self.timezone, + ) + + +class UpdateScheduledTaskRequest(BaseModel): + """Body of ``PATCH /scheduled-tasks/{task_id}``. + + On the wire, ``None`` means "not supplied" -- an explicit ``null`` has + always meant that on this endpoint, and unbinding a thread is expressed + by switching ``context_mode``, not by nulling ``thread_id``. The + ``None``-to-``UNSET`` translation in ``to_command`` is where that wire + convention meets the command's unambiguous three-state fields. + """ + + context_mode: str | None = None + thread_id: str | None = None + title: str | None = Field(default=None, min_length=1) + prompt: str | None = Field(default=None, min_length=1) + schedule_spec: dict[str, Any] | None = None + timezone: str | None = None + + def changes_schedule(self) -> bool: + return self.schedule_spec is not None or self.timezone is not None + + def changes_context(self) -> bool: + return self.context_mode is not None or self.thread_id is not None + + def to_command(self, task_id: str, user_id: str, current: ScheduledTask | None) -> UpdateScheduledTask: + """Wire shape -> command (transformation ① of the chain). + + A schedule or context change needs the parts the client omitted, so + the router reads the current task once and passes it in -- this model + stays IO-free and only translates. ``current`` may be ``None`` when + neither composite field is being changed. + """ + schedule = UNSET + if current is not None and self.changes_schedule(): + schedule = self.to_schedule(current.schedule) + context = UNSET + if current is not None and self.changes_context(): + context = ContextChange( + context_mode=self.context_mode if self.context_mode is not None else str(current.context_mode), + thread_id=self.thread_id if self.thread_id is not None else current.thread_id, + ) + return UpdateScheduledTask( + task_id=task_id, + user_id=user_id, + title=self.title if self.title is not None else UNSET, + prompt=self.prompt if self.prompt is not None else UNSET, + schedule=schedule, + context=context, + ) + + def to_schedule(self, current: ScheduleSpec) -> ScheduleSpec: + """Build the replacement spec, taking what was omitted from `current`. + + The schedule *type* is not patchable; only its spec and its zone are. + Omitted parts are read straight off the current value object rather + than round-tripped through the wire shape and back. + """ + if self.schedule_spec is not None: + cron = self.schedule_spec.get("cron") + run_at = self.schedule_spec.get("run_at") + else: + cron = current.cron + run_at = current.run_at.isoformat() if current.run_at else None + return ScheduleSpec.from_primitives( + str(current.schedule_type), + cron=cron, + run_at=run_at, + timezone=self.timezone if self.timezone is not None else current.timezone, + ) + + +class ScheduledTaskResponse(BaseModel): + """One scheduled task as the client sees it. + + Mirrors the frontend's `ScheduledTask` type field for field. + """ + + id: str + thread_id: str | None + context_mode: str + title: str + prompt: str + schedule_type: str + schedule_spec: dict[str, str] + timezone: str + status: str + next_run_at: UtcTimestamp | None + last_run_at: UtcTimestamp | None + last_run_id: str | None + last_thread_id: str | None + last_error: str | None + run_count: int + created_at: UtcTimestamp + updated_at: UtcTimestamp + + @classmethod + def from_domain(cls, task: ScheduledTask) -> ScheduledTaskResponse: + return cls( + id=task.task_id, + thread_id=task.thread_id, + context_mode=str(task.context_mode), + title=task.title, + prompt=task.prompt, + schedule_type=str(task.schedule.schedule_type), + schedule_spec=_spec_to_wire(task.schedule), + timezone=task.schedule.timezone, + status=str(task.status), + next_run_at=_utc(task.next_run_at), + last_run_at=_utc(task.last_run_at), + last_run_id=task.last_run_id, + last_thread_id=task.last_thread_id, + last_error=task.last_error, + run_count=task.run_count, + created_at=_utc(task.created_at), + updated_at=_utc(task.updated_at), + ) + + +class ScheduledRunResponse(BaseModel): + """One execution record. Mirrors the frontend's `ScheduledTaskRun` type.""" + + id: str + task_id: str + thread_id: str + run_id: str | None + scheduled_for: UtcTimestamp + trigger: str + status: str + error: str | None + started_at: UtcTimestamp | None + finished_at: UtcTimestamp | None + created_at: UtcTimestamp + + @classmethod + def from_domain(cls, run: ScheduledRun) -> ScheduledRunResponse: + return cls( + id=run.record_id, + task_id=run.task_id, + thread_id=run.thread_id, + run_id=run.run_id, + scheduled_for=_utc(run.scheduled_for), + trigger=str(run.trigger), + status=str(run.status), + error=run.error, + started_at=_utc(run.started_at), + finished_at=_utc(run.finished_at), + created_at=_utc(run.created_at), + ) + + +class TriggerResponse(BaseModel): + id: str + triggered: bool + + +class DeleteResponse(BaseModel): + id: str + deleted: bool diff --git a/backend/app/gateway/routers/schedule/router.py b/backend/app/gateway/routers/schedule/router.py new file mode 100644 index 000000000..5443712c1 --- /dev/null +++ b/backend/app/gateway/routers/schedule/router.py @@ -0,0 +1,195 @@ +"""Primary adapter -- the scheduled-task HTTP API. + +Everything here is protocol translation. Parse the body into domain values, +call one service method, render the result, and map domain errors onto status +codes. What is deliberately *absent* is the giveaway: no cron normalisation, +no `next_run_at` arithmetic, no re-arm rule, no ownership checks written out +by hand. Those moved into the aggregate, where they are covered by domain +tests that never construct an HTTP request. + +The error mapping is one table rather than a `raise HTTPException` per branch, +so a new domain error surfaces as a 500 that has to be classified, instead of +being silently swallowed by whichever `except` happened to be nearest. +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from functools import wraps +from typing import Annotated + +from fastapi import APIRouter, HTTPException, Query, Request + +from app.gateway.authz import require_permission +from app.gateway.deps import ScheduleServiceDep, get_optional_user_from_request +from app.gateway.routers.schedule.models import ( + CreateScheduledTaskRequest, + DeleteResponse, + ScheduledRunResponse, + ScheduledTaskResponse, + TriggerResponse, + UpdateScheduledTaskRequest, +) +from deerflow.domain.schedule.commands import DeleteTask, PauseTask, ResumeTask, TriggerTask +from deerflow.domain.schedule.exceptions import ( + InvalidContextModeError, + InvalidScheduleError, + ScheduleError, + TaskNotFoundError, + TaskNotMutableError, + ThreadNotFoundError, +) +from deerflow.domain.schedule.model import DispatchOutcome + +router = APIRouter(prefix="/api", tags=["scheduled-tasks"]) + +# One family of domain errors, one place they become a protocol. +_STATUS_BY_ERROR: dict[type[ScheduleError], int] = { + # "Not yours" and "does not exist" are both 404 by design -- the service + # already refuses to distinguish them, and so must the status code. + TaskNotFoundError: 404, + ThreadNotFoundError: 404, + InvalidScheduleError: 422, + InvalidContextModeError: 422, + TaskNotMutableError: 409, +} + + +def _map_domain_errors(handler): + """Translate domain errors raised by the service into HTTP responses. + + An unclassified `ScheduleError` becomes a 500 on purpose: a new domain + error is a new protocol decision, and defaulting it to 4xx would let it + ship as a client error nobody chose. + """ + + @wraps(handler) + async def wrapper(*args, **kwargs): + try: + return await handler(*args, **kwargs) + except ScheduleError as exc: + status = _STATUS_BY_ERROR.get(type(exc)) + if status is None: + raise + raise HTTPException(status_code=status, detail=str(exc)) from exc + + return wrapper + + +async def _require_user_id(request: Request) -> str: + user = await get_optional_user_from_request(request) + if user is None: + raise HTTPException(status_code=401, detail="Authentication required") + return str(user.id) + + +@router.get("/scheduled-tasks", response_model=list[ScheduledTaskResponse]) +@require_permission("threads", "read") +@_map_domain_errors +async def list_scheduled_tasks(request: Request, service: ScheduleServiceDep): + user = await get_optional_user_from_request(request) + if user is None: + # Not 401: an unauthenticated listing has always been an empty one. + return [] + return [ScheduledTaskResponse.from_domain(task) for task in await service.list_tasks(str(user.id))] + + +@router.post("/scheduled-tasks", response_model=ScheduledTaskResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def create_scheduled_task(request: Request, body: CreateScheduledTaskRequest, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + task = await service.create_scheduled_task(body.to_command(user_id), now=datetime.now(UTC)) + return ScheduledTaskResponse.from_domain(task) + + +@router.get("/scheduled-tasks/{task_id}", response_model=ScheduledTaskResponse) +@require_permission("threads", "read") +@_map_domain_errors +async def get_scheduled_task(task_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + return ScheduledTaskResponse.from_domain(await service.get_task(task_id, user_id=user_id)) + + +@router.patch("/scheduled-tasks/{task_id}", response_model=ScheduledTaskResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def update_scheduled_task( + task_id: str, + request: Request, + body: UpdateScheduledTaskRequest, + service: ScheduleServiceDep, +): + user_id = await _require_user_id(request) + # A schedule or context change needs the parts the client omitted, so the + # current task is read once and handed to `to_command`. That one extra + # fetch is what lets the command carry whole value objects instead of a + # patch of loose fields. + current = await service.get_task(task_id, user_id=user_id) if body.changes_schedule() or body.changes_context() else None + task = await service.update_scheduled_task(body.to_command(task_id, user_id, current), now=datetime.now(UTC)) + return ScheduledTaskResponse.from_domain(task) + + +@router.post("/scheduled-tasks/{task_id}/pause", response_model=ScheduledTaskResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def pause_scheduled_task(task_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + # No body, so no request model: the command is built right here (spec §2.1 ①). + return ScheduledTaskResponse.from_domain(await service.pause_task(PauseTask(task_id=task_id, user_id=user_id))) + + +@router.post("/scheduled-tasks/{task_id}/resume", response_model=ScheduledTaskResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def resume_scheduled_task(task_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + return ScheduledTaskResponse.from_domain(await service.resume_task(ResumeTask(task_id=task_id, user_id=user_id))) + + +@router.post("/scheduled-tasks/{task_id}/trigger", response_model=TriggerResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def trigger_scheduled_task(task_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + result = await service.trigger_task(TriggerTask(task_id=task_id, user_id=user_id), now=datetime.now(UTC)) + if result.outcome is DispatchOutcome.CONFLICT: + raise HTTPException(status_code=409, detail=result.error or "Scheduled task trigger conflicted with an active run") + if result.outcome is DispatchOutcome.FAILED: + # 502, not 500: the failure is downstream of this API -- the run could + # not be started -- and the task itself is intact. + raise HTTPException(status_code=502, detail=result.error or "Scheduled task trigger failed") + return TriggerResponse(id=task_id, triggered=True) + + +@router.delete("/scheduled-tasks/{task_id}", response_model=DeleteResponse) +@require_permission("threads", "write") +@_map_domain_errors +async def delete_scheduled_task(task_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + await service.delete_task(DeleteTask(task_id=task_id, user_id=user_id)) + return DeleteResponse(id=task_id, deleted=True) + + +@router.get("/scheduled-tasks/{task_id}/runs", response_model=list[ScheduledRunResponse]) +@require_permission("threads", "read") +@_map_domain_errors +async def list_scheduled_task_runs( + task_id: str, + request: Request, + service: ScheduleServiceDep, + limit: Annotated[int, Query(ge=1, le=200)] = 50, + offset: Annotated[int, Query(ge=0)] = 0, +): + user_id = await _require_user_id(request) + runs = await service.list_task_runs(task_id, user_id=user_id, limit=limit, offset=offset) + return [ScheduledRunResponse.from_domain(run) for run in runs] + + +@router.get("/threads/{thread_id}/scheduled-tasks", response_model=list[ScheduledTaskResponse]) +@require_permission("threads", "read", owner_check=True) +@_map_domain_errors +async def list_thread_scheduled_tasks(thread_id: str, request: Request, service: ScheduleServiceDep): + user_id = await _require_user_id(request) + tasks = await service.list_tasks_by_thread(user_id, thread_id) + return [ScheduledTaskResponse.from_domain(task) for task in tasks] diff --git a/backend/app/gateway/routers/scheduled_tasks.py b/backend/app/gateway/routers/scheduled_tasks.py deleted file mode 100644 index 35cd74bf1..000000000 --- a/backend/app/gateway/routers/scheduled_tasks.py +++ /dev/null @@ -1,310 +0,0 @@ -from __future__ import annotations - -import uuid -from datetime import UTC, datetime -from typing import Any - -from fastapi import APIRouter, HTTPException, Query, Request -from pydantic import BaseModel, Field - -from app.gateway.authz import require_permission -from app.gateway.deps import ( - get_config, - get_optional_user_from_request, - get_scheduled_task_repo, - get_scheduled_task_run_repo, - get_scheduled_task_service, - get_thread_store, -) -from deerflow.scheduler.schedules import ( - next_run_at as compute_next_run_at, -) -from deerflow.scheduler.schedules import ( - normalize_cron_expression, - validate_timezone, -) - -router = APIRouter(prefix="/api", tags=["scheduled-tasks"]) - - -def _ensure_task_mutable(task: dict[str, Any]) -> None: - if task.get("status") == "running": - raise HTTPException( - status_code=409, - detail="Scheduled task is currently running; retry after the active execution finishes", - ) - - -class ScheduledTaskCreateRequest(BaseModel): - thread_id: str | None = None - context_mode: str = "fresh_thread_per_run" - title: str = Field(min_length=1) - prompt: str = Field(min_length=1) - schedule_type: str - schedule_spec: dict[str, Any] - timezone: str - - -class ScheduledTaskUpdateRequest(BaseModel): - context_mode: str | None = None - thread_id: str | None = None - title: str | None = Field(default=None, min_length=1) - prompt: str | None = Field(default=None, min_length=1) - schedule_spec: dict[str, Any] | None = None - timezone: str | None = None - - -@router.get("/scheduled-tasks") -@require_permission("threads", "read") -async def list_scheduled_tasks(request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - return [] - return await repo.list_by_user(str(user.id)) - - -@router.post("/scheduled-tasks") -@require_permission("threads", "write") -async def create_scheduled_task(request: Request, body: ScheduledTaskCreateRequest): - config = get_config() - repo = get_scheduled_task_repo(request) - thread_store = get_thread_store(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - if body.context_mode not in {"fresh_thread_per_run", "reuse_thread"}: - raise HTTPException(status_code=422, detail="Unsupported context_mode") - if body.context_mode == "reuse_thread": - if not body.thread_id: - raise HTTPException(status_code=422, detail="reuse_thread requires thread_id") - if not await thread_store.check_access(body.thread_id, str(user.id), require_existing=True): - raise HTTPException(status_code=404, detail="Thread not found") - if body.schedule_type not in {"once", "cron"}: - raise HTTPException(status_code=422, detail="Unsupported schedule_type") - - schedule_spec = dict(body.schedule_spec) - try: - validate_timezone(body.timezone) - if body.schedule_type == "cron": - raw_cron = schedule_spec.get("cron") - if not isinstance(raw_cron, str): - raise HTTPException(status_code=422, detail="cron schedule requires schedule_spec.cron") - schedule_spec["cron"] = normalize_cron_expression(raw_cron) - next_run_at = compute_next_run_at( - body.schedule_type, - schedule_spec, - body.timezone, - now=datetime.now(UTC), - ) - except ValueError as exc: - raise HTTPException(status_code=422, detail=str(exc)) from exc - - if body.schedule_type == "once" and next_run_at is None: - raise HTTPException(status_code=422, detail="once schedule must be in the future") - if body.schedule_type == "once" and next_run_at is not None and (next_run_at - datetime.now(UTC)).total_seconds() < config.scheduler.min_once_delay_seconds: - raise HTTPException( - status_code=422, - detail=(f"once schedule must be at least {config.scheduler.min_once_delay_seconds} seconds in the future"), - ) - - return await repo.create( - task_id=f"task-{uuid.uuid4().hex}", - user_id=str(user.id), - thread_id=body.thread_id, - context_mode=body.context_mode, - assistant_id="lead_agent", - title=body.title, - prompt=body.prompt, - schedule_type=body.schedule_type, - schedule_spec=schedule_spec, - timezone=body.timezone, - next_run_at=next_run_at, - ) - - -@router.get("/scheduled-tasks/{task_id}") -@require_permission("threads", "read") -async def get_scheduled_task(task_id: str, request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - task = await repo.get(task_id, user_id=str(user.id)) - if task is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - return task - - -@router.patch("/scheduled-tasks/{task_id}") -@require_permission("threads", "write") -async def update_scheduled_task(task_id: str, request: Request, body: ScheduledTaskUpdateRequest): - config = get_config() - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - existing = await repo.get(task_id, user_id=str(user.id)) - if existing is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - _ensure_task_mutable(existing) - - updates = body.model_dump(exclude_none=True) - if "context_mode" in updates: - if updates["context_mode"] not in {"fresh_thread_per_run", "reuse_thread"}: - raise HTTPException(status_code=422, detail="Unsupported context_mode") - effective_context_mode = str(updates.get("context_mode", existing["context_mode"])) - effective_thread_id = updates.get("thread_id", existing.get("thread_id")) - if effective_context_mode == "reuse_thread": - if not effective_thread_id: - raise HTTPException(status_code=422, detail="reuse_thread requires thread_id") - thread_store = get_thread_store(request) - if not await thread_store.check_access(str(effective_thread_id), str(user.id), require_existing=True): - raise HTTPException(status_code=404, detail="Thread not found") - elif effective_context_mode == "fresh_thread_per_run": - updates["thread_id"] = None - if "timezone" in updates: - try: - validate_timezone(str(updates["timezone"])) - except ValueError as exc: - raise HTTPException(status_code=422, detail=str(exc)) from exc - - if "schedule_spec" in updates or "timezone" in updates: - schedule_spec = dict(existing["schedule_spec"]) - if "schedule_spec" in updates and isinstance(updates["schedule_spec"], dict): - schedule_spec = dict(updates["schedule_spec"]) - timezone = str(updates.get("timezone", existing["timezone"])) - try: - if existing["schedule_type"] == "cron": - raw_cron = schedule_spec.get("cron") - if not isinstance(raw_cron, str): - raise HTTPException( - status_code=422, - detail="cron schedule requires schedule_spec.cron", - ) - schedule_spec["cron"] = normalize_cron_expression(raw_cron) - next_run_at = compute_next_run_at( - existing["schedule_type"], - schedule_spec, - timezone, - now=datetime.now(UTC), - ) - except ValueError as exc: - raise HTTPException(status_code=422, detail=str(exc)) from exc - if existing["schedule_type"] == "once" and next_run_at is None: - raise HTTPException(status_code=422, detail="once schedule must be in the future") - if existing["schedule_type"] == "once" and next_run_at is not None and (next_run_at - datetime.now(UTC)).total_seconds() < config.scheduler.min_once_delay_seconds: - raise HTTPException( - status_code=422, - detail=(f"once schedule must be at least {config.scheduler.min_once_delay_seconds} seconds in the future"), - ) - updates["schedule_spec"] = schedule_spec - updates["next_run_at"] = next_run_at - # A terminal task (completed/failed/cancelled) whose schedule was just - # pushed into the future must be re-armed: claim_due_tasks only admits - # "enabled" rows, so leaving the terminal status would return 200 with - # a next_run_at that silently never fires. - if next_run_at is not None and existing["status"] in {"completed", "failed", "cancelled"}: - updates["status"] = "enabled" - - updated = await repo.update( - task_id, - user_id=str(user.id), - updates=updates, - ) - return updated - - -@router.post("/scheduled-tasks/{task_id}/pause") -@require_permission("threads", "write") -async def pause_scheduled_task(task_id: str, request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - existing = await repo.get(task_id, user_id=str(user.id)) - if existing is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - _ensure_task_mutable(existing) - updated = await repo.update(task_id, user_id=str(user.id), updates={"status": "paused"}) - if updated is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - return updated - - -@router.post("/scheduled-tasks/{task_id}/resume") -@require_permission("threads", "write") -async def resume_scheduled_task(task_id: str, request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - existing = await repo.get(task_id, user_id=str(user.id)) - if existing is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - _ensure_task_mutable(existing) - updated = await repo.update(task_id, user_id=str(user.id), updates={"status": "enabled"}) - if updated is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - return updated - - -@router.post("/scheduled-tasks/{task_id}/trigger") -@require_permission("threads", "write") -async def trigger_scheduled_task(task_id: str, request: Request): - repo = get_scheduled_task_repo(request) - service = get_scheduled_task_service(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - task = await repo.get(task_id, user_id=str(user.id)) - if task is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - result = await service.dispatch_task(task, now=datetime.now(UTC), trigger="manual") - if result["outcome"] == "conflict": - raise HTTPException(status_code=409, detail=result["error"] or "Scheduled task trigger conflicted with an active run") - if result["outcome"] == "failed": - raise HTTPException(status_code=502, detail=result["error"] or "Scheduled task trigger failed") - return {"id": task_id, "triggered": True} - - -@router.delete("/scheduled-tasks/{task_id}") -@require_permission("threads", "write") -async def delete_scheduled_task(task_id: str, request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - deleted = await repo.delete(task_id, user_id=str(user.id)) - if not deleted: - raise HTTPException(status_code=404, detail="Scheduled task not found") - return {"id": task_id, "deleted": deleted} - - -@router.get("/scheduled-tasks/{task_id}/runs") -@require_permission("threads", "read") -async def list_scheduled_task_runs( - task_id: str, - request: Request, - limit: int = Query(default=50, ge=1, le=200), - offset: int = Query(default=0, ge=0), -): - task_repo = get_scheduled_task_repo(request) - run_repo = get_scheduled_task_run_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - task = await task_repo.get(task_id, user_id=str(user.id)) - if task is None: - raise HTTPException(status_code=404, detail="Scheduled task not found") - return await run_repo.list_by_task(task_id, limit=limit, offset=offset) - - -@router.get("/threads/{thread_id}/scheduled-tasks") -@require_permission("threads", "read", owner_check=True) -async def list_thread_scheduled_tasks(thread_id: str, request: Request): - repo = get_scheduled_task_repo(request) - user = await get_optional_user_from_request(request) - if user is None: - raise HTTPException(status_code=401, detail="Authentication required") - return await repo.list_by_user_and_thread(str(user.id), thread_id) diff --git a/backend/app/scheduler/__init__.py b/backend/app/scheduler/__init__.py index a3c331940..2bec3007a 100644 --- a/backend/app/scheduler/__init__.py +++ b/backend/app/scheduler/__init__.py @@ -1,3 +1 @@ -from .service import ScheduledTaskService - -__all__ = ["ScheduledTaskService"] +"""Clock-driven entry point of the schedule context (the poller).""" diff --git a/backend/app/scheduler/poller.py b/backend/app/scheduler/poller.py new file mode 100644 index 000000000..0a1998029 --- /dev/null +++ b/backend/app/scheduler/poller.py @@ -0,0 +1,84 @@ +"""Driving adapter -- the clock that asks the schedule service to work. + +This is the only part of the scheduler that knows about time passing. It owns +*when* `ScheduleService.run_once` is called and what happens when a poll fails; +it owns nothing about what a poll means. Everything the old +`app/scheduler/service.py` mixed into its loop -- overlap policy, lease +semantics, budget accounting -- now lives in the domain and reaches this file +only as one awaited call. + +Two behaviours here are load-bearing: + + - **A failing poll must not end the loop.** A transient error (SQLite's + "database is locked" is the realistic one) would otherwise stop every + scheduled task for the rest of the process life, silently. + - **Startup reconciliation must not block startup.** The service lets + reconcile failures propagate on purpose -- whether they are fatal is the + caller's policy -- and this caller's policy is to log and keep scheduling. + A gateway refusing to start over leftover rows is worse than one running + with them. +""" + +from __future__ import annotations + +import asyncio +import logging +from datetime import UTC, datetime +from typing import TYPE_CHECKING + +if TYPE_CHECKING: + from deerflow.domain.schedule.service import ScheduleService + +logger = logging.getLogger(__name__) + +RESTART_ERROR = "interrupted: gateway restarted before the run reached a terminal state" + + +class SchedulePoller: + """Runs `ScheduleService.run_once` on an interval until stopped.""" + + def __init__(self, service: ScheduleService, *, poll_interval_seconds: float) -> None: + self._service = service + self._poll_interval_seconds = poll_interval_seconds + self._task: asyncio.Task | None = None + self._stop = asyncio.Event() + + async def start(self) -> None: + """Reconcile what a crash left behind, then begin polling. + + Idempotent: a second call while running is a no-op, so a caller cannot + end up with two loops claiming the same tasks. + """ + if self._task is not None: + return + try: + stale_runs, stuck_tasks = await self._service.reconcile_on_startup(error=RESTART_ERROR) + if stale_runs: + logger.warning("Marked %d stale scheduled task run(s) as interrupted after restart", stale_runs) + if stuck_tasks: + logger.warning("Cancelled %d stuck once task(s) after restart", stuck_tasks) + except Exception: + logger.exception("Failed to reconcile scheduled tasks at startup; scheduling anyway") + self._stop.clear() + self._task = asyncio.create_task(self._run_loop()) + + async def stop(self) -> None: + """Signal the loop and wait for the in-flight poll to finish.""" + if self._task is None: + return + self._stop.set() + await self._task + self._task = None + + async def _run_loop(self) -> None: + while not self._stop.is_set(): + try: + await self._service.run_once(now=datetime.now(UTC)) + except Exception: + logger.exception("Scheduled task poll failed; retrying next interval") + try: + # Waiting on the stop event rather than sleeping keeps shutdown + # prompt at a production-sized interval. + await asyncio.wait_for(self._stop.wait(), timeout=self._poll_interval_seconds) + except TimeoutError: + continue diff --git a/backend/app/scheduler/service.py b/backend/app/scheduler/service.py deleted file mode 100644 index 9bcfc268a..000000000 --- a/backend/app/scheduler/service.py +++ /dev/null @@ -1,496 +0,0 @@ -from __future__ import annotations - -import asyncio -import logging -import socket -import uuid -from datetime import UTC, datetime -from typing import Any, Literal - -from fastapi import HTTPException - -from deerflow.persistence.scheduled_task_runs import ActiveScheduledRunConflict -from deerflow.runtime import ConflictError, RunRecord -from deerflow.scheduler.schedules import next_run_at - -logger = logging.getLogger(__name__) - -# Shared so the has_active_runs fast path and the unique-index race path return -# byte-identical outcomes for the same "task already has an active run" condition. -_ACTIVE_RUN_CONFLICT_ERROR = "task already has an active run" -_SKIP_ACTIVE_RUN_ERROR = "skipped: a previous run of this task is still active" - - -class ScheduledTaskService: - def __init__( - self, - *, - task_repo, - task_run_repo, - launch_run, - poll_interval_seconds: int, - lease_seconds: int, - max_concurrent_runs: int, - ) -> None: - self._task_repo = task_repo - self._task_run_repo = task_run_repo - self._launch_run = launch_run - self._poll_interval_seconds = poll_interval_seconds - self._lease_seconds = lease_seconds - self._max_concurrent_runs = max_concurrent_runs - self._lease_owner = f"{socket.gethostname()}:{uuid.uuid4().hex}" - self._task: asyncio.Task | None = None - self._stop = asyncio.Event() - - async def run_once(self, *, now: datetime) -> None: - # ``max_concurrent_runs`` is a global cap on active scheduled runs, not - # just a per-poll claim batch: long runs accumulate across poll cycles, - # so each cycle only claims into the remaining budget. - active = await self._task_run_repo.count_active_runs() - budget = self._max_concurrent_runs - active - if budget <= 0: - return - claimed = await self._task_repo.claim_due_tasks( - now=now, - lease_owner=self._lease_owner, - lease_seconds=self._lease_seconds, - limit=budget, - ) - for task in claimed: - await self.dispatch_task(task, now=now, trigger="scheduled") - - @staticmethod - def _is_overlap_conflict(exc: Exception) -> bool: - if isinstance(exc, ConflictError): - return True - return isinstance(exc, HTTPException) and exc.status_code == 409 - - @staticmethod - def _task_status_for_failure(task: dict[str, Any], *, trigger: str) -> str: - if trigger == "manual": - # A failed manual trigger must not consume the task's scheduled - # future: a `once` task with run_at still ahead would otherwise be - # flipped to "failed" and never claimed again. - return task.get("status") or "enabled" - if task["schedule_type"] == "once": - return "failed" - return "enabled" - - @staticmethod - def _task_status_for_launch(task: dict[str, Any], *, trigger: str) -> str: - # The task-level status to write once _launch_run has produced a live - # run. A `once` task stays "running" until handle_run_completion - # observes the real terminal outcome; declaring "completed" at launch - # would stick if the run fails or the process dies (startup - # reconciliation is cancel_stuck_once_tasks). - if task["schedule_type"] == "once": - return "running" - if trigger == "manual" and task.get("status") == "paused": - return "paused" - return "enabled" - - @staticmethod - def _task_status_for_skip(task: dict[str, Any]) -> str: - if task["schedule_type"] == "once": - # The single occurrence was lost to an overlapping run; "completed" - # would claim an execution that never happened. - return "failed" - return "enabled" - - async def dispatch_task( - self, - task: dict[str, Any], - *, - now: datetime, - trigger: str, - ) -> dict[str, Any]: - execution_thread_id = task.get("thread_id") - if task.get("context_mode") == "fresh_thread_per_run" or not execution_thread_id: - execution_thread_id = str(uuid.uuid4()) - # "skip" must hold for fresh-thread runs too, where every run gets a new - # thread and the same-thread multitask ConflictError below can never - # fire. Checked before creating this dispatch's own run row so the row - # does not count itself as the active run. A manual trigger against an - # active run is rejected outright (409 at the router) instead of being - # recorded as a skipped occurrence — nothing was scheduled to happen. - # - # This has_active_runs check is a non-atomic fast path: it runs in its - # own session and is separated from the create() below by await points, - # so two concurrent dispatches (double-click / client retry / a manual - # trigger racing the poller) can both observe no active run. The DB is - # the atomic arbiter — the partial unique index uq_scheduled_task_run_active - # rejects the second active insert, surfaced as ActiveScheduledRunConflict - # and collapsed to the SAME outcome as this fast path just below. - overlap_skip = task.get("overlap_policy", "skip") == "skip" - if overlap_skip and await self._task_run_repo.has_active_runs(task["id"]): - if trigger == "manual": - return self._active_run_conflict_result(execution_thread_id) - return await self._record_scheduled_skip(task, thread_id=execution_thread_id, now=now, trigger=trigger) - - task_run_id = f"task-run-{uuid.uuid4().hex}" - try: - await self._task_run_repo.create( - run_record_id=task_run_id, - task_id=task["id"], - thread_id=execution_thread_id, - scheduled_for=now, - trigger=trigger, - status="queued", - ) - except ActiveScheduledRunConflict: - # Lost the create race for the task's single active slot: a - # concurrent dispatch passed the same fast-path check and inserted - # its active row first. Identical outcome to the fast path above. - if trigger == "manual": - return self._active_run_conflict_result(execution_thread_id) - return await self._record_scheduled_skip(task, thread_id=execution_thread_id, now=now, trigger=trigger) - # Track whether _launch_run has produced a live run. A bookkeeping - # failure AFTER launch (the queued->running write, or the parent task - # update) must NOT be recorded as "failed": "failed" is outside the - # partial unique index uq_scheduled_task_run_active, so it would release - # the task's single active slot and the next dispatch would launch a - # duplicate run. Once launch succeeds we keep the row "running" and - # retain the launched run_id regardless of bookkeeping errors. - launched_run_id: str | None = None - launched_thread_id: str | None = None - # Flip immediately after _launch_run returns, before any further code - # that can raise (e.g. result["run_id"] on a malformed result). The - # retention branch keys off this flag, not `launched_run_id is not - # None`, so a launch that succeeded but whose result-unpacking raised - # still takes the retention path instead of the release-the-slot path. - launch_succeeded = False - try: - result = await self._launch_run( - thread_id=execution_thread_id, - assistant_id=task.get("assistant_id"), - prompt=task["prompt"], - owner_user_id=task.get("user_id"), - metadata={ - "scheduled_task_id": task["id"], - "scheduled_task_run_id": task_run_id, - "scheduled_trigger": trigger, - }, - ) - launch_succeeded = True - launched_run_id = result["run_id"] - launched_thread_id = result["thread_id"] - next_at = next_run_at( - task["schedule_type"], - task["schedule_spec"], - task["timezone"], - now=now, - ) - task_status = self._task_status_for_launch(task, trigger=trigger) - await self._task_run_repo.update_status( - task_run_id, - status="running", - run_id=launched_run_id, - started_at=now, - # A fast-failing run can reach handle_run_completion before this - # write resumes; never clobber its terminal status. - protect_terminal=True, - ) - await self._task_repo.update_after_launch( - task["id"], - status=task_status, - next_run_at=next_at, - last_run_at=now, - last_run_id=launched_run_id, - last_thread_id=launched_thread_id, - last_error=None, - increment_run_count=True, - # Same race as the run-row write above: a fast-failing run's - # completion hook may have already finalized a `once` task. - protect_terminal=True, - ) - return { - "outcome": "launched", - "task_run_id": task_run_id, - "run_id": launched_run_id, - "thread_id": launched_thread_id, - "error": None, - } - except Exception as exc: - if not launch_succeeded and self._is_overlap_conflict(exc) and trigger == "scheduled" and task.get("overlap_policy", "skip") == "skip": - # Pre-launch overlap conflict (e.g. same-thread multitask): no - # run was started, so recording a skip and releasing the slot is - # safe. Guarded by ``not launch_succeeded`` because a run that - # already launched must never be reclassified as a skip / failed. - return await self._finalize_skip( - task, - task_run_id=task_run_id, - thread_id=execution_thread_id, - now=now, - error=str(exc), - ) - - next_at = next_run_at( - task["schedule_type"], - task["schedule_spec"], - task["timezone"], - now=now, - ) - - if launch_succeeded: - # _launch_run succeeded, so a run is live even though - # post-launch bookkeeping raised. Keep the task-run row - # "running" so it keeps holding the task's single active slot - # (preventing a duplicate launch on the next dispatch) and - # persist the run_id on the parent task for recovery / - # reconciliation / cancellation. These writes are best-effort: - # if the DB is still down the row stays "queued" -- still - # active, still holding the slot -- so we log and still report - # the run as launched so callers know a run is in flight. - task_status = self._task_status_for_launch(task, trigger=trigger) - try: - await self._task_run_repo.update_status( - task_run_id, - status="running", - run_id=launched_run_id, - started_at=now, - protect_terminal=True, - ) - except Exception: - logger.exception( - "Scheduled task-run %s: post-launch bookkeeping failed; run %s is still live (task %s)", - task_run_id, - launched_run_id, - task["id"], - ) - try: - await self._task_repo.update_after_launch( - task["id"], - status=task_status, - next_run_at=next_at, - last_run_at=now, - last_run_id=launched_run_id, - last_thread_id=launched_thread_id, - # The bookkeeping exception is an infrastructure-level - # transient, not a run-level failure: the run launched - # and is still in flight. Clear last_error like the - # success path so the task list does not show an error - # on a task whose run is actively running; the real - # terminal outcome is written by handle_run_completion. - # The transient itself is logged above. - last_error=None, - increment_run_count=True, - protect_terminal=True, - ) - except Exception: - logger.exception( - "Scheduled task %s: post-launch update failed; run %s is still live", - task["id"], - launched_run_id, - ) - return { - "outcome": "launched", - "task_run_id": task_run_id, - "run_id": launched_run_id, - "thread_id": launched_thread_id, - "error": str(exc), - } - - # _launch_run itself failed (or a step before it did): no live run - # was created, so it is safe to release the active slot. - task_status = self._task_status_for_failure(task, trigger=trigger) - await self._task_run_repo.update_status( - task_run_id, - status="failed", - error=str(exc), - started_at=now, - finished_at=now, - ) - await self._task_repo.update_after_launch( - task["id"], - status=task_status, - next_run_at=next_at, - last_run_at=now, - last_run_id=None, - last_thread_id=execution_thread_id, - last_error=str(exc), - increment_run_count=False, - ) - return { - "outcome": "conflict" if self._is_overlap_conflict(exc) else "failed", - "task_run_id": task_run_id, - "run_id": None, - "thread_id": execution_thread_id, - "error": str(exc), - } - - def _active_run_conflict_result(self, thread_id: str) -> dict[str, Any]: - """Manual-trigger response when the task already has an active run. - - Nothing was scheduled to happen, so no run-history row is recorded; the - router maps this to a 409. - """ - return { - "outcome": "conflict", - "task_run_id": None, - "run_id": None, - "thread_id": thread_id, - "error": _ACTIVE_RUN_CONFLICT_ERROR, - } - - async def _record_scheduled_skip( - self, - task: dict[str, Any], - *, - thread_id: str, - now: datetime, - trigger: str, - ) -> dict[str, Any]: - """Record a skipped occurrence for a scheduled dispatch that overlapped an active run. - - The tombstone is created directly as terminal ``"skipped"`` rather than - the transient ``"queued"`` the launch path uses: a queued row is active - and would itself trip ``uq_scheduled_task_run_active`` against the - pre-existing run that is still holding the task's single active slot. - ``"skipped"`` is outside the index predicate, so it never conflicts. - """ - task_run_id = f"task-run-{uuid.uuid4().hex}" - await self._task_run_repo.create( - run_record_id=task_run_id, - task_id=task["id"], - thread_id=thread_id, - scheduled_for=now, - trigger=trigger, - status="skipped", - ) - return await self._finalize_skip(task, task_run_id=task_run_id, thread_id=thread_id, now=now, error=_SKIP_ACTIVE_RUN_ERROR) - - async def _finalize_skip( - self, - task: dict[str, Any], - *, - task_run_id: str, - thread_id: str, - now: datetime, - error: str, - ) -> dict[str, Any]: - next_at = next_run_at( - task["schedule_type"], - task["schedule_spec"], - task["timezone"], - now=now, - ) - await self._task_run_repo.update_status( - task_run_id, - status="skipped", - error=error, - started_at=now, - finished_at=now, - ) - await self._task_repo.update_after_launch( - task["id"], - status=self._task_status_for_skip(task), - next_run_at=next_at, - last_run_at=task.get("last_run_at"), - last_run_id=task.get("last_run_id"), - last_thread_id=task.get("last_thread_id"), - last_error=error if task["schedule_type"] == "once" else None, - increment_run_count=False, - ) - return { - "outcome": "skipped", - "task_run_id": task_run_id, - "run_id": None, - "thread_id": thread_id, - "error": error, - } - - async def handle_run_completion(self, record: RunRecord) -> None: - metadata = record.metadata or {} - task_id = metadata.get("scheduled_task_id") - task_run_id = metadata.get("scheduled_task_run_id") - user_id = record.user_id - if not isinstance(task_id, str) or not isinstance(task_run_id, str) or not user_id: - return - - terminal_status: Literal["success", "failed", "interrupted"] | None - if record.status.value == "success": - terminal_status = "success" - error = None - elif record.status.value == "interrupted": - # Distinct from "failed": an interrupt (user cancel, same-thread - # takeover) carries no error and is not an execution failure. - terminal_status = "interrupted" - error = record.error or "run was interrupted before completion" - elif record.status.value in {"error", "timeout"}: - terminal_status = "failed" - error = record.error - else: - terminal_status = None - error = record.error - if terminal_status is None: - return - - await self._task_run_repo.update_status( - task_run_id, - status=terminal_status, - run_id=record.run_id, - error=error, - finished_at=datetime.now(UTC), - ) - - task = await self._task_repo.get(task_id, user_id=user_id) - if task is None: - return - - updates: dict[str, Any] = {"last_error": error} - if task["schedule_type"] == "once": - # The single occurrence is consumed either way (the run did launch, - # so re-arming risks duplicate side effects), but an interrupt ends - # as "cancelled", not "failed". - if terminal_status == "success": - updates["status"] = "completed" - elif terminal_status == "interrupted": - updates["status"] = "cancelled" - else: - updates["status"] = "failed" - await self._task_repo.update(task_id, user_id=user_id, updates=updates) - - async def start(self) -> None: - if self._task is not None: - return - restart_error = "interrupted: gateway restarted before the run reached a terminal state" - try: - stale = await self._task_run_repo.mark_stale_active_runs(error=restart_error) - if stale: - logger.warning("Marked %d stale scheduled task run(s) as interrupted after restart", stale) - except Exception: - logger.exception("Failed to sweep stale scheduled task runs at startup") - try: - # The run rows above are only half the story: a launched `once` - # task is parked in "running" until the (now dead) completion hook - # would have finalized it, so reconcile the parent rows too. - stuck = await self._task_repo.cancel_stuck_once_tasks(error=restart_error) - if stuck: - logger.warning("Cancelled %d stuck once task(s) after restart", stuck) - except Exception: - logger.exception("Failed to reconcile stuck once tasks at startup") - self._stop.clear() - self._task = asyncio.create_task(self._run_loop()) - - async def stop(self) -> None: - if self._task is None: - return - self._stop.set() - await self._task - self._task = None - - async def _run_loop(self) -> None: - while not self._stop.is_set(): - try: - await self.run_once(now=datetime.now(UTC)) - except Exception: - # A transient DB error (e.g. SQLite "database is locked") must - # not kill the poller task for the rest of the process life. - logger.exception("Scheduled task poll failed; retrying next interval") - try: - await asyncio.wait_for( - self._stop.wait(), - timeout=self._poll_interval_seconds, - ) - except TimeoutError: - continue diff --git a/backend/packages/harness/deerflow/config/reload_boundary.py b/backend/packages/harness/deerflow/config/reload_boundary.py index 6f6f87d62..52e4a04f6 100644 --- a/backend/packages/harness/deerflow/config/reload_boundary.py +++ b/backend/packages/harness/deerflow/config/reload_boundary.py @@ -66,8 +66,8 @@ STARTUP_ONLY_FIELDS: dict[str, str] = { "start_channel_service() wires the connection repository and channel workers once at startup, and the channel-connections router caches the merged provider config on app.state; channel_connections.* edits need a restart." ), "scheduler": ( - "ScheduledTaskService is constructed and started once during Gateway lifespan startup; enabled, poll_interval_seconds, lease_seconds, " - "and max_concurrent_runs are captured into the service instance and the background poller task is not rebuilt on config.yaml edits." + "ScheduleService and SchedulePoller are constructed and started once during Gateway lifespan startup; enabled, poll_interval_seconds, lease_seconds, " + "and max_concurrent_runs are captured into the assembled service/poller and the background poller task is not rebuilt on config.yaml edits." ), "run_ownership": ( "RunOwnershipConfig is captured once into RunManager at langgraph_runtime() startup; the lease heartbeat background task is created and " diff --git a/backend/packages/harness/deerflow/persistence/scheduled_task_runs/__init__.py b/backend/packages/harness/deerflow/persistence/scheduled_task_runs/__init__.py index 8c014cf90..6669b4054 100644 --- a/backend/packages/harness/deerflow/persistence/scheduled_task_runs/__init__.py +++ b/backend/packages/harness/deerflow/persistence/scheduled_task_runs/__init__.py @@ -1,4 +1,12 @@ -from .model import ScheduledTaskRunRow -from .sql import ActiveScheduledRunConflict, ScheduledTaskRunRepository +"""ORM row of the scheduled_task_runs table. -__all__ = ["ActiveScheduledRunConflict", "ScheduledTaskRunRow", "ScheduledTaskRunRepository"] +The table definition -- including the partial unique index +``uq_scheduled_task_run_active`` that arbitrates the single active slot -- +lives here with the shared engine/alembic infrastructure; its only reader and +writer is the schedule context's secondary adapter +(``app/adapters/schedule/scheduled_run_repository.py``). +""" + +from .model import ScheduledTaskRunRow + +__all__ = ["ScheduledTaskRunRow"] diff --git a/backend/packages/harness/deerflow/persistence/scheduled_task_runs/sql.py b/backend/packages/harness/deerflow/persistence/scheduled_task_runs/sql.py deleted file mode 100644 index afedb4b4f..000000000 --- a/backend/packages/harness/deerflow/persistence/scheduled_task_runs/sql.py +++ /dev/null @@ -1,171 +0,0 @@ -from __future__ import annotations - -from datetime import UTC, datetime -from typing import Any - -from sqlalchemy import func, select -from sqlalchemy.exc import IntegrityError -from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker - -from deerflow.persistence.scheduled_task_runs.model import ScheduledTaskRunRow -from deerflow.utils.time import coerce_iso - -TERMINAL_RUN_STATUSES: frozenset[str] = frozenset({"success", "failed", "skipped", "interrupted"}) -ACTIVE_RUN_STATUSES: tuple[str, ...] = ("queued", "running") - - -class ActiveScheduledRunConflict(Exception): - """A concurrent dispatch already holds the task's single active-run slot. - - Raised by :meth:`ScheduledTaskRunRepository.create` when inserting an - active (queued/running) run row would violate the partial unique index - ``uq_scheduled_task_run_active`` (at most one active run per ``task_id``). - This is the atomic counterpart to the non-atomic ``has_active_runs`` check - in ``ScheduledTaskService.dispatch_task``: two dispatches can both pass that - check, but only one can insert the active row — the loser lands here. - - Translating the SQLAlchemy ``IntegrityError`` into a domain exception at - the repository boundary keeps the service layer free of ``sqlalchemy.exc`` - coupling (mirrors ``deerflow.runtime.ConflictError`` for the runs table). - """ - - def __init__(self, task_id: str) -> None: - self.task_id = task_id - super().__init__(f"scheduled task {task_id!r} already has an active run") - - -class ScheduledTaskRunRepository: - def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: - self._sf = session_factory - - @staticmethod - def _row_to_dict(row: ScheduledTaskRunRow) -> dict[str, Any]: - data = row.to_dict() - for key in ("scheduled_for", "started_at", "finished_at", "created_at"): - if data.get(key) is not None: - data[key] = coerce_iso(data[key]) - return data - - async def create( - self, - *, - run_record_id: str, - task_id: str, - thread_id: str, - scheduled_for: datetime, - trigger: str, - status: str, - ) -> dict[str, Any]: - row = ScheduledTaskRunRow( - id=run_record_id, - task_id=task_id, - thread_id=thread_id, - scheduled_for=scheduled_for, - trigger=trigger, - status=status, - created_at=datetime.now(UTC), - ) - async with self._sf() as session: - session.add(row) - try: - await session.commit() - except IntegrityError: - await session.rollback() - # Only active-status inserts can trip the partial unique index - # ``uq_scheduled_task_run_active``; a terminal-status row (e.g. - # a "skipped" tombstone) is outside its predicate and cannot - # conflict, so any IntegrityError there is a genuine fault and - # is re-raised untranslated. - if status in ACTIVE_RUN_STATUSES: - raise ActiveScheduledRunConflict(task_id) from None - raise - await session.refresh(row) - return self._row_to_dict(row) - - async def list_by_task(self, task_id: str, *, limit: int = 50, offset: int = 0) -> list[dict[str, Any]]: - stmt = ( - select(ScheduledTaskRunRow) - .where(ScheduledTaskRunRow.task_id == task_id) - .order_by( - ScheduledTaskRunRow.created_at.desc(), - ScheduledTaskRunRow.id.desc(), - ) - .limit(limit) - .offset(offset) - ) - async with self._sf() as session: - result = await session.execute(stmt) - return [self._row_to_dict(row) for row in result.scalars()] - - async def count_active_runs(self) -> int: - """Global count of queued/running rows, used to bound cross-task concurrency.""" - stmt = select(func.count()).select_from(ScheduledTaskRunRow).where(ScheduledTaskRunRow.status.in_(ACTIVE_RUN_STATUSES)) - async with self._sf() as session: - result = await session.execute(stmt) - return int(result.scalar() or 0) - - async def update_status( - self, - run_record_id: str, - *, - status: str, - run_id: str | None = None, - error: str | None = None, - started_at: datetime | None = None, - finished_at: datetime | None = None, - protect_terminal: bool = False, - ) -> None: - async with self._sf() as session: - row = await session.get(ScheduledTaskRunRow, run_record_id) - if row is None: - return - if protect_terminal and row.status in TERMINAL_RUN_STATUSES: - # The launch-path "running" write lost the race against the - # completion hook; keep the terminal status/error and only - # backfill bookkeeping the completion write could not know. - if row.run_id is None and run_id is not None: - row.run_id = run_id - if row.started_at is None and started_at is not None: - row.started_at = started_at - await session.commit() - return - row.status = status - row.run_id = run_id - row.error = error - if started_at is not None: - row.started_at = started_at - if finished_at is not None: - row.finished_at = finished_at - await session.commit() - - async def has_active_runs(self, task_id: str) -> bool: - stmt = ( - select(ScheduledTaskRunRow.id) - .where( - ScheduledTaskRunRow.task_id == task_id, - ScheduledTaskRunRow.status.in_(ACTIVE_RUN_STATUSES), - ) - .limit(1) - ) - async with self._sf() as session: - result = await session.execute(stmt) - return result.scalars().first() is not None - - async def mark_stale_active_runs(self, *, error: str) -> int: - """Fail-fast bookkeeping for runs orphaned by a process crash. - - Agent runs execute in-process, so any ``queued``/``running`` row found - at scheduler startup belongs to a run whose process is gone. Only valid - under the MVP's single-scheduler-instance assumption. - """ - stmt = select(ScheduledTaskRunRow).where(ScheduledTaskRunRow.status.in_(ACTIVE_RUN_STATUSES)) - now = datetime.now(UTC) - async with self._sf() as session: - result = await session.execute(stmt) - rows = list(result.scalars()) - for row in rows: - row.status = "interrupted" - row.error = error - row.finished_at = now - await session.commit() - return len(rows) diff --git a/backend/packages/harness/deerflow/persistence/scheduled_tasks/__init__.py b/backend/packages/harness/deerflow/persistence/scheduled_tasks/__init__.py index 18b6ae7fe..8cd1e94f6 100644 --- a/backend/packages/harness/deerflow/persistence/scheduled_tasks/__init__.py +++ b/backend/packages/harness/deerflow/persistence/scheduled_tasks/__init__.py @@ -1,4 +1,10 @@ -from .model import ScheduledTaskRow -from .sql import ScheduledTaskRepository +"""ORM row of the scheduled_tasks table. -__all__ = ["ScheduledTaskRow", "ScheduledTaskRepository"] +The table definition lives here with the shared engine/alembic +infrastructure; its only reader and writer is the schedule context's +secondary adapter (``app/adapters/schedule/scheduled_task_repository.py``). +""" + +from .model import ScheduledTaskRow + +__all__ = ["ScheduledTaskRow"] diff --git a/backend/packages/harness/deerflow/persistence/scheduled_tasks/sql.py b/backend/packages/harness/deerflow/persistence/scheduled_tasks/sql.py deleted file mode 100644 index 7f05f12bd..000000000 --- a/backend/packages/harness/deerflow/persistence/scheduled_tasks/sql.py +++ /dev/null @@ -1,232 +0,0 @@ -from __future__ import annotations - -from datetime import UTC, datetime, timedelta -from typing import Any - -from sqlalchemy import and_, or_, select -from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker - -from deerflow.persistence.scheduled_tasks.model import ScheduledTaskRow -from deerflow.utils.time import coerce_iso - -TERMINAL_TASK_STATUSES: frozenset[str] = frozenset({"completed", "failed", "cancelled"}) - - -class ScheduledTaskRepository: - def __init__(self, session_factory: async_sessionmaker[AsyncSession]) -> None: - self._sf = session_factory - - @staticmethod - def _row_to_dict(row: ScheduledTaskRow) -> dict[str, Any]: - data = row.to_dict() - for key in ( - "created_at", - "updated_at", - "next_run_at", - "last_run_at", - "lease_expires_at", - ): - if data.get(key) is not None: - data[key] = coerce_iso(data[key]) - return data - - async def create( - self, - *, - task_id: str, - user_id: str, - thread_id: str | None, - context_mode: str, - assistant_id: str | None, - title: str, - prompt: str, - schedule_type: str, - schedule_spec: dict[str, Any], - timezone: str, - next_run_at: datetime | None, - ) -> dict[str, Any]: - now = datetime.now(UTC) - row = ScheduledTaskRow( - id=task_id, - user_id=user_id, - thread_id=thread_id, - context_mode=context_mode, - assistant_id=assistant_id, - title=title, - prompt=prompt, - schedule_type=schedule_type, - schedule_spec=schedule_spec, - timezone=timezone, - next_run_at=next_run_at, - created_at=now, - updated_at=now, - ) - async with self._sf() as session: - session.add(row) - await session.commit() - await session.refresh(row) - return self._row_to_dict(row) - - async def get(self, task_id: str, *, user_id: str) -> dict[str, Any] | None: - async with self._sf() as session: - row = await session.get(ScheduledTaskRow, task_id) - if row is None or row.user_id != user_id: - return None - return self._row_to_dict(row) - - async def list_by_user(self, user_id: str) -> list[dict[str, Any]]: - stmt = select(ScheduledTaskRow).where(ScheduledTaskRow.user_id == user_id).order_by(ScheduledTaskRow.created_at.desc(), ScheduledTaskRow.id.desc()) - async with self._sf() as session: - result = await session.execute(stmt) - return [self._row_to_dict(row) for row in result.scalars()] - - async def update( - self, - task_id: str, - *, - user_id: str, - updates: dict[str, Any], - ) -> dict[str, Any] | None: - async with self._sf() as session: - row = await session.get(ScheduledTaskRow, task_id) - if row is None or row.user_id != user_id: - return None - for key, value in updates.items(): - if hasattr(row, key): - setattr(row, key, value) - row.updated_at = datetime.now(UTC) - await session.commit() - await session.refresh(row) - return self._row_to_dict(row) - - async def delete(self, task_id: str, *, user_id: str) -> bool: - async with self._sf() as session: - row = await session.get(ScheduledTaskRow, task_id) - if row is None or row.user_id != user_id: - return False - await session.delete(row) - await session.commit() - return True - - async def claim_due_tasks( - self, - *, - now: datetime, - lease_owner: str, - lease_seconds: int, - limit: int, - ) -> list[dict[str, Any]]: - lease_expires_at = now + timedelta(seconds=lease_seconds) - stmt = ( - select(ScheduledTaskRow) - .where( - ScheduledTaskRow.next_run_at.is_not(None), - ScheduledTaskRow.next_run_at <= now, - or_( - and_( - ScheduledTaskRow.status == "enabled", - or_( - ScheduledTaskRow.lease_expires_at.is_(None), - ScheduledTaskRow.lease_expires_at < now, - ), - ), - # A task stuck in "running" with an expired lease means the - # claiming process died between claim and dispatch; it must - # stay reclaimable or the task is dead forever. - and_( - ScheduledTaskRow.status == "running", - ScheduledTaskRow.lease_expires_at.is_not(None), - ScheduledTaskRow.lease_expires_at < now, - ), - ), - ) - .order_by(ScheduledTaskRow.next_run_at.asc(), ScheduledTaskRow.id.asc()) - .limit(limit) - .with_for_update(skip_locked=True) - ) - async with self._sf() as session: - result = await session.execute(stmt) - rows = list(result.scalars()) - for row in rows: - row.lease_owner = lease_owner - row.lease_expires_at = lease_expires_at - row.status = "running" - row.updated_at = datetime.now(UTC) - await session.commit() - return [self._row_to_dict(row) for row in rows] - - async def update_after_launch( - self, - task_id: str, - *, - status: str, - next_run_at: datetime | None, - last_run_at: datetime | None, - last_run_id: str | None, - last_thread_id: str | None, - last_error: str | None, - increment_run_count: bool, - protect_terminal: bool = False, - ) -> None: - async with self._sf() as session: - row = await session.get(ScheduledTaskRow, task_id) - if row is None: - return - if protect_terminal and row.status in TERMINAL_TASK_STATUSES: - # A fast-failing run can reach handle_run_completion (which - # finalizes a `once` task) before this launch-path write - # commits; keep the hook's status/error and only record the - # launch bookkeeping. - pass - else: - row.status = status - row.last_error = last_error - row.next_run_at = next_run_at - row.last_run_at = last_run_at - row.last_run_id = last_run_id - row.last_thread_id = last_thread_id - if increment_run_count: - row.run_count += 1 - row.lease_owner = None - row.lease_expires_at = None - row.updated_at = datetime.now(UTC) - await session.commit() - - async def list_by_user_and_thread(self, user_id: str, thread_id: str) -> list[dict[str, Any]]: - stmt = ( - select(ScheduledTaskRow) - .where( - ScheduledTaskRow.user_id == user_id, - ScheduledTaskRow.thread_id == thread_id, - ) - .order_by(ScheduledTaskRow.created_at.desc(), ScheduledTaskRow.id.desc()) - ) - async with self._sf() as session: - result = await session.execute(stmt) - return [self._row_to_dict(row) for row in result.scalars()] - - async def cancel_stuck_once_tasks(self, *, error: str) -> int: - """Reconcile ``once`` tasks orphaned in ``running`` by a process crash. - - A launched ``once`` task stays ``running`` until the in-process - completion hook moves it to a terminal status; its lease was cleared at - launch, so the claim query's expired-lease reclaim branch never sees - it. After a crash the hook is gone and the task would be stuck forever. - Tasks still holding a lease are left alone — they were claimed but not - launched, and expired-lease reclaim recovers them safely. - """ - stmt = select(ScheduledTaskRow).where( - ScheduledTaskRow.schedule_type == "once", - ScheduledTaskRow.status == "running", - ScheduledTaskRow.lease_expires_at.is_(None), - ) - async with self._sf() as session: - result = await session.execute(stmt) - rows = list(result.scalars()) - now = datetime.now(UTC) - for row in rows: - row.status = "cancelled" - row.last_error = error - row.updated_at = now - await session.commit() - return len(rows) diff --git a/backend/packages/harness/deerflow/scheduler/__init__.py b/backend/packages/harness/deerflow/scheduler/__init__.py deleted file mode 100644 index b2262f3e8..000000000 --- a/backend/packages/harness/deerflow/scheduler/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -from .schedules import next_run_at, normalize_cron_expression, validate_timezone - -__all__ = ["next_run_at", "normalize_cron_expression", "validate_timezone"] diff --git a/backend/packages/harness/deerflow/scheduler/schedules.py b/backend/packages/harness/deerflow/scheduler/schedules.py deleted file mode 100644 index 8509c3523..000000000 --- a/backend/packages/harness/deerflow/scheduler/schedules.py +++ /dev/null @@ -1,55 +0,0 @@ -from __future__ import annotations - -from datetime import UTC, datetime -from zoneinfo import ZoneInfo, ZoneInfoNotFoundError - -from croniter import croniter - - -def validate_timezone(timezone_name: str) -> str: - try: - ZoneInfo(timezone_name) - except ZoneInfoNotFoundError as exc: - raise ValueError(f"Unknown timezone: {timezone_name}") from exc - return timezone_name - - -def normalize_cron_expression(expr: str) -> str: - parts = [part for part in expr.split() if part] - if len(parts) != 5: - raise ValueError("Cron expression must contain exactly 5 fields") - return " ".join(parts) - - -def next_run_at( - schedule_type: str, - schedule_spec: dict[str, object], - timezone_name: str, - *, - now: datetime, -) -> datetime | None: - validate_timezone(timezone_name) - if now.tzinfo is None: - now = now.replace(tzinfo=UTC) - - if schedule_type == "once": - run_at_raw = schedule_spec.get("run_at") - if not isinstance(run_at_raw, str): - raise ValueError("once schedule requires run_at") - run_at = datetime.fromisoformat(run_at_raw) - if run_at.tzinfo is None: - # A naive run_at means "wall-clock time in the task's declared - # timezone", matching how cron schedules interpret it. - run_at = run_at.replace(tzinfo=ZoneInfo(timezone_name)) - return run_at if run_at > now else None - - if schedule_type == "cron": - cron_expr = normalize_cron_expression(str(schedule_spec.get("cron", ""))) - zone = ZoneInfo(timezone_name) - local_now = now.astimezone(zone) - next_local = croniter(cron_expr, local_now).get_next(datetime) - if next_local.tzinfo is None: - next_local = next_local.replace(tzinfo=zone) - return next_local.astimezone(UTC) - - raise ValueError(f"Unsupported schedule_type: {schedule_type}") diff --git a/backend/tests/test_composition.py b/backend/tests/test_composition.py new file mode 100644 index 000000000..f26df2f11 --- /dev/null +++ b/backend/tests/test_composition.py @@ -0,0 +1,146 @@ +"""Tests for the composition root. + +The point of extracting `build_domain_services` from the lifespan is that the +rules below become assertions. Before, "a memory backend means no service, +and the routes answer 503" was a comment inside a 180-line startup function +that no test could reach without booting the whole application. +""" + +from __future__ import annotations + +import pytest + +from app.composition import DomainServices, build_domain_services, build_run_completion_hook, build_schedule_policy +from deerflow.config.scheduler_config import SchedulerConfig +from deerflow.domain.schedule.model import SchedulePolicy + + +class _StubThreadStore: + async def check_access(self, thread_id: str, user_id: str, *, require_existing: bool = False) -> bool: + return True + + +async def _launch_run(**kwargs): + return {"run_id": "run-1", "thread_id": kwargs.get("thread_id", "thread-1")} + + +def _build(session_factory, scheduler_config=None) -> DomainServices: + return build_domain_services( + session_factory=session_factory, + thread_store=_StubThreadStore(), + launch_run=_launch_run, + scheduler_config=scheduler_config or SchedulerConfig(), + ) + + +class TestMemoryBackend: + """`session_factory is None` is how `database.backend: memory` presents.""" + + def test_no_session_factory_yields_no_services(self): + assert _build(None).schedule is None + + def test_it_does_not_degrade_to_an_in_memory_implementation(self): + """Refusing is the intended behaviour: a scheduled task that silently + vanishes on restart is worse than one the API declines to accept.""" + assert _build(None).schedule is None + + +class TestSqlBackend: + def test_a_session_factory_yields_the_service(self): + """A fake sessionmaker is enough -- wiring must not touch the database, + which is what makes this assertable without a live engine.""" + assert _build(object()).schedule is not None + + +class TestSchedulePolicy: + def test_every_threshold_comes_from_the_operator_config(self): + policy = build_schedule_policy( + SchedulerConfig( + min_once_delay_seconds=30, + max_concurrent_runs=7, + lease_seconds=90, + ) + ) + assert policy == SchedulePolicy( + min_once_delay_seconds=30, + max_concurrent_runs=7, + lease_seconds=90, + ) + + def test_the_domain_defaults_are_not_what_production_gets(self): + """The domain's defaults are permissive so that "nobody configured a + policy" invents no business constraint. Production must not inherit + them by accident -- the config's own defaults are the real values.""" + from_config = build_schedule_policy(SchedulerConfig()) + assert from_config != SchedulePolicy() + assert from_config.min_once_delay_seconds == SchedulerConfig().min_once_delay_seconds + + @pytest.mark.parametrize( + ("field", "value"), + [ + ("min_once_delay_seconds", 45), + ("max_concurrent_runs", 5), + ("lease_seconds", 300), + ], + ) + def test_each_field_is_mapped_from_its_own_config_key(self, field, value): + """Guards against two thresholds being wired from one key -- a + transposition the type checker cannot see, since all three are ints.""" + policy = build_schedule_policy(SchedulerConfig(**{field: value})) + assert getattr(policy, field) == value + + def test_poll_interval_is_not_part_of_the_policy(self): + """How often to look is the poller's business, not a rule any task is + subject to.""" + assert not hasattr(SchedulePolicy(), "poll_interval_seconds") + + +class TestEveryPortGetsTheRightAdapter: + """Deliberately white-box: verifying which adapter landed in which slot is + the entire job of a composition root, and all four are keyword arguments + of compatible shape, so a transposition type-checks cleanly and would only + surface as a production error. + """ + + def test_each_schedule_port_is_filled_with_its_own_adapter(self): + from app.adapters.schedule.run_launcher import GatewayRunLauncher + from app.adapters.schedule.scheduled_run_repository import SqlScheduledRunRepository + from app.adapters.schedule.scheduled_task_repository import SqlScheduledTaskRepository + from app.adapters.schedule.thread_lookup import ThreadStoreThreadLookup + + service = _build(object()).schedule + assert isinstance(service._tasks, SqlScheduledTaskRepository) + assert isinstance(service._runs, SqlScheduledRunRepository) + assert isinstance(service._launcher, GatewayRunLauncher) + assert isinstance(service._threads, ThreadStoreThreadLookup) + + def test_the_policy_reaches_the_service(self): + service = _build(object(), SchedulerConfig(max_concurrent_runs=9)).schedule + assert service._policy.max_concurrent_runs == 9 + + +class TestRunCompletionHook: + """The inbound half of the wiring. + + What the hook *does* with a run is asserted in + `test_schedule_run_completion.py`; all that is left here is the assembly + decision, which is this module's whole job. + """ + + def test_no_service_installs_no_hook(self): + """`None` rather than a hook that always declines: with no service + there is nothing for the runtime to call, and saying so lets it skip + the callback entirely.""" + assert build_run_completion_hook(None) is None + + def test_a_service_is_wrapped_in_the_inbound_adapter(self): + from app.adapters.schedule.run_completion import ScheduleRunCompletionListener + + hook = build_run_completion_hook(_build(object()).schedule) + assert isinstance(hook, ScheduleRunCompletionListener) + + def test_the_hook_is_bound_to_the_service_it_was_given(self): + """Deliberately white-box, for the same reason as the port assertions + above: a hook wired to the wrong service type-checks cleanly.""" + service = _build(object()).schedule + assert build_run_completion_hook(service)._service is service diff --git a/backend/tests/test_gateway_run_drain_shutdown.py b/backend/tests/test_gateway_run_drain_shutdown.py index 528997069..f4b2f8959 100644 --- a/backend/tests/test_gateway_run_drain_shutdown.py +++ b/backend/tests/test_gateway_run_drain_shutdown.py @@ -30,6 +30,7 @@ from typing import Annotated, TypedDict import pytest from langgraph.checkpoint.memory import InMemorySaver +from deerflow.config.scheduler_config import SchedulerConfig from deerflow.runtime import RunManager, RunStatus @@ -196,7 +197,7 @@ async def test_langgraph_runtime_drains_runs_before_closing_checkpointer(monkeyp monkeypatch.setattr(RunManager, "shutdown", spy_shutdown, raising=False) app = FastAPI() - startup_config = SimpleNamespace(database=SimpleNamespace(backend="memory", checkpoint_channel_mode="full", checkpoint_delta=SimpleNamespace(snapshot_frequency=10)), run_events=None) + startup_config = SimpleNamespace(database=SimpleNamespace(backend="memory", checkpoint_channel_mode="full", checkpoint_delta=SimpleNamespace(snapshot_frequency=10)), run_events=None, scheduler=SchedulerConfig()) async with langgraph_runtime(app, startup_config): pass diff --git a/backend/tests/test_gateway_run_recovery.py b/backend/tests/test_gateway_run_recovery.py index 9f6b05c7e..6374b69c4 100644 --- a/backend/tests/test_gateway_run_recovery.py +++ b/backend/tests/test_gateway_run_recovery.py @@ -14,6 +14,7 @@ from fastapi import FastAPI import deerflow.runtime as runtime_module from app.gateway import deps as gateway_deps from deerflow.config.run_ownership_config import RunOwnershipConfig +from deerflow.config.scheduler_config import SchedulerConfig from deerflow.persistence import engine as engine_module from deerflow.persistence import thread_meta as thread_meta_module from deerflow.runtime import END_SENTINEL, MemoryStreamBridge, RunManager @@ -224,6 +225,7 @@ async def test_sqlite_runtime_reconciles_orphaned_runs_on_startup(monkeypatch): database=SimpleNamespace(backend="sqlite", checkpoint_channel_mode="full", checkpoint_delta=SimpleNamespace(snapshot_frequency=10)), run_events=SimpleNamespace(backend="memory"), stream_bridge=SimpleNamespace(recovered_stream_cleanup_delay_seconds=60.0), + scheduler=SchedulerConfig(), ) thread_store = _FakeThreadStore() stream_bridge = _FakeStreamBridge(existing_streams={"run-1"}) @@ -269,6 +271,7 @@ async def test_sqlite_runtime_does_not_mark_thread_error_when_newer_run_is_succe database=SimpleNamespace(backend="sqlite", checkpoint_channel_mode="full", checkpoint_delta=SimpleNamespace(snapshot_frequency=10)), run_events=SimpleNamespace(backend="memory"), stream_bridge=SimpleNamespace(recovered_stream_cleanup_delay_seconds=60.0), + scheduler=SchedulerConfig(), ) thread_store = _FakeThreadStore() stream_bridge = _FakeStreamBridge(existing_streams={"old-running"}) diff --git a/backend/tests/test_gateway_services.py b/backend/tests/test_gateway_services.py index 842546991..bcca4bf73 100644 --- a/backend/tests/test_gateway_services.py +++ b/backend/tests/test_gateway_services.py @@ -1407,7 +1407,7 @@ def _make_start_run_persistence_context(): run_events_config=None, thread_store=thread_store, checkpoint_channel_mode="full", - scheduled_task_service=None, + run_completion_hook=None, ) request = SimpleNamespace( headers={}, diff --git a/backend/tests/test_schedule_corrupt_rows.py b/backend/tests/test_schedule_corrupt_rows.py new file mode 100644 index 000000000..d3335982f --- /dev/null +++ b/backend/tests/test_schedule_corrupt_rows.py @@ -0,0 +1,126 @@ +"""A corrupt stored schedule is an operator problem, not a client error. + +``SqlScheduledTaskRepository._to_domain`` rebuilds the aggregate on every +read, so a row whose stored schedule no longer parses surfaces at read time. +It used to surface as ``InvalidScheduleError`` -- the same error the aggregate +raises for a *client-submitted* schedule, which the router maps to 422. That +double duty told the client "your request is wrong" about a request that was +perfectly fine, and made the row unrepairable over HTTP: PATCH reads the task +before writing, so the fix path 422'd too. + +``CorruptStoredScheduleError`` splits the vocabulary: it is raised only by the +persistence adapter, and it is deliberately absent from the router's status +table so it falls through to the unclassified-500 branch -- a server-side +fault reported as one. + +SQL-only: the in-memory double stores whole aggregates and cannot hold a +corrupt row, which is exactly why this file exists beside the contract suite +rather than inside it. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from datetime import UTC, datetime + +import pytest +import pytest_asyncio + +from app.adapters.schedule.scheduled_task_repository import SqlScheduledTaskRepository +from app.gateway.routers.schedule.router import _STATUS_BY_ERROR +from deerflow.config.database_config import DatabaseConfig +from deerflow.domain.schedule.exceptions import CorruptStoredScheduleError, InvalidScheduleError +from deerflow.domain.schedule.model import ScheduledTask, SchedulePolicy, ScheduleSpec +from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config +from deerflow.persistence.scheduled_tasks.model import ScheduledTaskRow + +pytestmark = pytest.mark.asyncio + +NOW = datetime(2026, 7, 27, 12, 0, tzinfo=UTC) + + +@pytest_asyncio.fixture +async def repo(tmp_path) -> AsyncIterator[SqlScheduledTaskRepository]: + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + yield SqlScheduledTaskRepository(sf) + finally: + await close_engine() + + +async def _add_task(repo: SqlScheduledTaskRepository, *, title: str = "healthy") -> ScheduledTask: + return await repo.add( + ScheduledTask.create( + user_id="user-1", + title=title, + prompt="p", + schedule=ScheduleSpec.cron_schedule("0 9 * * *", "UTC"), + context_mode="fresh_thread_per_run", + thread_id=None, + now=NOW, + policy=SchedulePolicy(), + ) + ) + + +async def _corrupt(task_id: str, **columns) -> None: + """Damage a stored row directly -- the exact shape a bug or a manual edit + would leave behind, unreachable through the port.""" + sf = get_session_factory() + assert sf is not None + async with sf() as session: + row = await session.get(ScheduledTaskRow, task_id) + assert row is not None + for name, value in columns.items(): + setattr(row, name, value) + await session.commit() + + +class TestSingleRowReads: + async def test_a_corrupt_schedule_raises_the_dedicated_error(self, repo): + task = await _add_task(repo) + await _corrupt(task.task_id, schedule_spec={}) + + with pytest.raises(CorruptStoredScheduleError): + await repo.get(task.task_id, user_id="user-1") + + async def test_the_dedicated_error_is_not_the_client_facing_one(self, repo): + """The router maps InvalidScheduleError to 422; a corrupt row must not + ride that mapping.""" + task = await _add_task(repo) + await _corrupt(task.task_id, schedule_spec={}) + + with pytest.raises(CorruptStoredScheduleError) as exc_info: + await repo.get(task.task_id, user_id="user-1") + assert not isinstance(exc_info.value, InvalidScheduleError) + + async def test_a_corrupt_context_mode_is_reported_the_same_way(self, repo): + """Enum rebuild failures are the same fault as spec parse failures: a + raw ValueError out of the adapter would be a technical exception + crossing the boundary untranslated.""" + task = await _add_task(repo) + await _corrupt(task.task_id, context_mode="not-a-mode") + + with pytest.raises(CorruptStoredScheduleError): + await repo.get(task.task_id, user_id="user-1") + + +class TestListReads: + async def test_a_corrupt_row_does_not_take_down_the_listing(self, repo): + healthy = await _add_task(repo, title="healthy") + broken = await _add_task(repo, title="broken") + await _corrupt(broken.task_id, schedule_spec={}) + + listed = await repo.list_by_user("user-1") + + assert [t.task_id for t in listed] == [healthy.task_id] + + +class TestRouterMapping: + def test_the_corrupt_row_error_is_unclassified_on_purpose(self): + """Absent from the status table means the router's fallthrough turns + it into a 500 -- a new protocol decision would have to add it here + deliberately.""" + assert CorruptStoredScheduleError not in _STATUS_BY_ERROR diff --git a/backend/tests/test_schedule_dispatch_race.py b/backend/tests/test_schedule_dispatch_race.py new file mode 100644 index 000000000..d502b3978 --- /dev/null +++ b/backend/tests/test_schedule_dispatch_race.py @@ -0,0 +1,223 @@ +"""Concurrency regression tests for the scheduled-task dispatch TOCTOU. + +``ScheduleService.dispatch_task`` guards "at most one active run per task +when overlap_policy=skip" with a non-atomic ``has_active`` fast path followed +by a separate queued-record insert. Two concurrent dispatches (double-click, +client retry, or a manual trigger racing the poller) can both pass the check +and both launch. The database is the atomic arbiter via the partial unique +index ``uq_scheduled_task_run_active`` (``task_id WHERE status IN +('queued','running')``); the losing insert is translated to +``ActiveRunConflictError`` and collapsed to the same outcome as the fast +path. + +These tests drive the REAL ``SqlScheduledRunRepository`` + +``SqlScheduledTaskRepository`` + ``ScheduleService`` against a real +file-backed sqlite database (so the index is actually enforced), with a fake +launcher that only records launches. The contract suite deliberately does not +own this: its doubles provide no atomicity, so a green contract run says +nothing about two dispatchers racing -- this file is where that is proven. +""" + +from __future__ import annotations + +import asyncio +from datetime import UTC, datetime + +import pytest +from schedule_fakes import FakeThreadLookup + +from app.adapters.schedule.scheduled_run_repository import SqlScheduledRunRepository +from app.adapters.schedule.scheduled_task_repository import SqlScheduledTaskRepository +from deerflow.config.database_config import DatabaseConfig +from deerflow.domain.schedule.exceptions import ActiveRunConflictError +from deerflow.domain.schedule.model import DispatchOutcome, RunStatus, ScheduledRun, ScheduledTask, SchedulePolicy, ScheduleSpec, TriggerKind +from deerflow.domain.schedule.ports import LaunchedRun +from deerflow.domain.schedule.service import ScheduleService +from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config + +pytestmark = pytest.mark.asyncio + +NOW = datetime(2026, 7, 27, 12, 0, tzinfo=UTC) + + +class _BarrierRunRepo(SqlScheduledRunRepository): + """Real repository that only releases both dispatchers past ``has_active`` + once both have read it, so their ``add()`` calls genuinely race for the + task's single active slot -- a deterministic reproduction of the + check-then-insert TOCTOU.""" + + def __init__(self, session_factory, barrier: asyncio.Barrier | None) -> None: + super().__init__(session_factory) + self._barrier = barrier + + async def has_active(self, task_id: str) -> bool: + result = await super().has_active(task_id) + if self._barrier is not None: + await self._barrier.wait() + return result + + +class _RecordingLauncher: + """Launch double that yields first, so a truly-concurrent sibling can + interleave before the launch is recorded.""" + + def __init__(self) -> None: + self.calls: list[dict] = [] + + async def launch(self, *, thread_id, assistant_id, prompt, owner_user_id, metadata) -> LaunchedRun: + await asyncio.sleep(0) + self.calls.append({"thread_id": thread_id, "metadata": metadata}) + return LaunchedRun(run_id=f"run-{len(self.calls)}", thread_id=thread_id) + + +def _make_service(tasks, runs, launcher) -> ScheduleService: + return ScheduleService( + tasks=tasks, + runs=runs, + launcher=launcher, + threads=FakeThreadLookup(), + policy=SchedulePolicy(min_once_delay_seconds=60, max_concurrent_runs=10, lease_seconds=120), + ) + + +async def _seed_task(tasks: SqlScheduledTaskRepository, title: str) -> ScheduledTask: + # fresh_thread_per_run: every dispatch gets a NEW thread_id, so #4003's + # per-thread uq_runs_thread_active can never fire for two dispatches of the + # same task -- this is precisely the gap the per-task index closes. + return await tasks.add( + ScheduledTask.create( + user_id="user-1", + title=title, + prompt="do the thing", + schedule=ScheduleSpec.cron_schedule("*/5 * * * *", "UTC"), + context_mode="fresh_thread_per_run", + thread_id=None, + now=NOW, + policy=SchedulePolicy(), + ) + ) + + +async def _active_run_count(runs: SqlScheduledRunRepository, task_id: str) -> int: + rows = await runs.list_by_task(task_id, limit=100) + return sum(1 for row in rows if row.is_active) + + +async def test_two_concurrent_manual_dispatches_launch_exactly_once(tmp_path): + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + tasks = SqlScheduledTaskRepository(sf) + runs = _BarrierRunRepo(sf, asyncio.Barrier(2)) + launcher = _RecordingLauncher() + service = _make_service(tasks, runs, launcher) + task = await _seed_task(tasks, "task-race-manual") + + results = await asyncio.gather( + service.dispatch_task(task, now=NOW, trigger=TriggerKind.MANUAL), + service.dispatch_task(task, now=NOW, trigger=TriggerKind.MANUAL), + ) + + outcomes = sorted((result.outcome for result in results), key=str) + # Exactly one wins the active slot; the loser is a 409-style conflict. + assert outcomes == [DispatchOutcome.CONFLICT, DispatchOutcome.LAUNCHED], outcomes + assert len(launcher.calls) == 1, launcher.calls + assert await _active_run_count(runs, task.task_id) == 1 + # The manual loser records no run-history row (nothing was scheduled). + conflict = next(r for r in results if r.outcome is DispatchOutcome.CONFLICT) + assert conflict.record_id is None + finally: + await close_engine() + + +async def test_scheduled_and_manual_dispatch_launch_exactly_once(tmp_path): + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + tasks = SqlScheduledTaskRepository(sf) + runs = _BarrierRunRepo(sf, asyncio.Barrier(2)) + launcher = _RecordingLauncher() + service = _make_service(tasks, runs, launcher) + task = await _seed_task(tasks, "task-race-mixed") + + results = await asyncio.gather( + service.dispatch_task(task, now=NOW, trigger=TriggerKind.SCHEDULED), + service.dispatch_task(task, now=NOW, trigger=TriggerKind.MANUAL), + ) + + outcomes = [result.outcome for result in results] + # Whichever won launched; the loser is conflict (manual) or skipped + # (scheduled). Which one wins is timing-dependent, but exactly one runs. + assert outcomes.count(DispatchOutcome.LAUNCHED) == 1, outcomes + assert set(outcomes) <= {DispatchOutcome.LAUNCHED, DispatchOutcome.CONFLICT, DispatchOutcome.SKIPPED}, outcomes + assert len(launcher.calls) == 1, launcher.calls + assert await _active_run_count(runs, task.task_id) == 1 + finally: + await close_engine() + + +async def test_natural_timing_concurrent_dispatch_launches_exactly_once(tmp_path): + # No barrier: exercise the fix under the same natural interleaving that + # reproduced the bug (5/5 both-launch before the index). The fix must hold + # whether the second dispatch is caught by the has_active fast path or by + # the index-violation path. + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + tasks = SqlScheduledTaskRepository(sf) + runs = SqlScheduledRunRepository(sf) + for i in range(5): + launcher = _RecordingLauncher() + service = _make_service(tasks, runs, launcher) + task = await _seed_task(tasks, f"task-natural-{i}") + + results = await asyncio.gather( + service.dispatch_task(task, now=NOW, trigger=TriggerKind.MANUAL), + service.dispatch_task(task, now=NOW, trigger=TriggerKind.MANUAL), + ) + + outcomes = [result.outcome for result in results] + assert outcomes.count(DispatchOutcome.LAUNCHED) == 1, (i, outcomes) + assert len(launcher.calls) == 1, (i, launcher.calls) + assert await _active_run_count(runs, task.task_id) == 1, i + finally: + await close_engine() + + +async def test_partial_unique_index_enforces_one_active_run_per_task(tmp_path): + # Focused repository-level test of the index semantics + the typed conflict. + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + runs = SqlScheduledRunRepository(sf) + now = datetime(2026, 7, 2, 1, 0, tzinfo=UTC) + + first = ScheduledRun.queued(task_id="t1", thread_id="th1", scheduled_for=now, trigger=TriggerKind.SCHEDULED) + await runs.add(first) + + # queued -> running is a same-row UPDATE: keeps the one active slot, no + # violation (this is the normal launch transition). + await runs.update_status(first.record_id, status=RunStatus.RUNNING, run_id="run-1", started_at=now) + assert await runs.has_active("t1") is True + + # A second active insert for the same task is a domain conflict. + with pytest.raises(ActiveRunConflictError): + await runs.add(ScheduledRun.queued(task_id="t1", thread_id="th2", scheduled_for=now, trigger=TriggerKind.MANUAL)) + + # Terminal-status rows for the same task are outside the index predicate. + await runs.add(ScheduledRun.skipped_tombstone(task_id="t1", thread_id="th3", scheduled_for=now, trigger=TriggerKind.SCHEDULED)) + + # A different task's active row is independent. + await runs.add(ScheduledRun.queued(task_id="t2", thread_id="th4", scheduled_for=now, trigger=TriggerKind.SCHEDULED)) + + # Finishing the active run frees the slot; a fresh active row is allowed. + await runs.update_status(first.record_id, status=RunStatus.SUCCESS, run_id="run-1", finished_at=now) + assert await runs.has_active("t1") is False + await runs.add(ScheduledRun.queued(task_id="t1", thread_id="th5", scheduled_for=now, trigger=TriggerKind.SCHEDULED)) + assert await runs.has_active("t1") is True + finally: + await close_engine() diff --git a/backend/tests/test_schedule_fakes.py b/backend/tests/test_schedule_fakes.py new file mode 100644 index 000000000..abed843ff --- /dev/null +++ b/backend/tests/test_schedule_fakes.py @@ -0,0 +1,565 @@ +"""Contract suite for the schedule repository ports. + +Every case here runs **twice**: once against the in-memory doubles and once +against the SQL adapters on a real file-backed sqlite database. That is the +point -- a rule stated in a port docstring has to hold for both, and a +divergence becomes a failure rather than a surprise in production. + +What the contract owns is single-threaded semantics: which rows `claim_due` +selects, that `add` refuses a second active record, what `protect_terminal` +preserves. What it deliberately does **not** own is atomicity -- the doubles +provide none, and a green run here says nothing about two dispatchers racing. +That is covered against a real database in +``test_schedule_dispatch_race.py``. Do not read a passing contract suite +as licence to run more than one scheduler. +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from datetime import UTC, datetime, timedelta + +import pytest +import pytest_asyncio +from schedule_fakes import ( + FakeRunLauncher, + FakeThreadLookup, + InMemoryScheduledRunRepository, + InMemoryScheduledTaskRepository, +) + +from app.adapters.schedule.scheduled_run_repository import SqlScheduledRunRepository +from app.adapters.schedule.scheduled_task_repository import SqlScheduledTaskRepository +from deerflow.config.database_config import DatabaseConfig +from deerflow.domain.schedule.exceptions import ActiveRunConflictError +from deerflow.domain.schedule.model import ContextMode, RunStatus, ScheduledRun, ScheduledTask, ScheduleSpec, TaskStatus, TriggerKind +from deerflow.domain.schedule.ports import ScheduledRunRepository, ScheduledTaskRepository +from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config + +pytestmark = pytest.mark.asyncio + +NOW = datetime(2026, 7, 27, 12, 0, tzinfo=UTC) +CRON = ScheduleSpec.cron_schedule("0 9 * * *", "UTC") +ONCE = ScheduleSpec.once_at(datetime(2026, 8, 1, 9, 0, tzinfo=UTC), "UTC") + + +@pytest_asyncio.fixture(params=["memory", "sql"]) +async def repos(request, tmp_path) -> AsyncIterator[tuple[ScheduledTaskRepository, ScheduledRunRepository]]: + """One parametrized fixture, two implementations of the same ports.""" + if request.param == "memory": + yield InMemoryScheduledTaskRepository(), InMemoryScheduledRunRepository() + return + await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) + try: + sf = get_session_factory() + assert sf is not None + yield SqlScheduledTaskRepository(sf), SqlScheduledRunRepository(sf) + finally: + await close_engine() + + +@pytest_asyncio.fixture +async def tasks(repos) -> ScheduledTaskRepository: + return repos[0] + + +@pytest_asyncio.fixture +async def runs(repos) -> ScheduledRunRepository: + return repos[1] + + +async def seed( + repo: ScheduledTaskRepository, + task: ScheduledTask, + *, + claimed_until: datetime | None = None, +) -> ScheduledTask: + """Install a task, optionally already carrying a claim. + + A claim cannot be installed through the port -- `claim_due` only stamps + tasks that are actually due, and these cases need shapes that claiming + would never produce (running with an *expired* claim, running with a live + one). So each implementation is set up directly: the fake exposes a seed + helper, and the SQL side writes the row. That is the one place this suite + reaches past the port, and it is why the assertions that follow go back + through it. + """ + stored = await repo.add(task) + if claimed_until is None: + return stored + + if isinstance(repo, InMemoryScheduledTaskRepository): + repo.seed(task, lease_owner="prior-worker", lease_expires_at=claimed_until) + return task + + from deerflow.persistence.scheduled_tasks.model import ScheduledTaskRow + + factory = get_session_factory() + assert factory is not None + async with factory() as session: + row = await session.get(ScheduledTaskRow, task.task_id) + row.lease_owner = "prior-worker" + row.lease_expires_at = claimed_until + await session.commit() + return task + + +def make_task( + task_id: str = "task-1", + *, + user_id: str = "user-1", + status: TaskStatus = TaskStatus.ENABLED, + next_run_at: datetime | None = None, + schedule: ScheduleSpec = CRON, + thread_id: str | None = None, + context_mode: ContextMode = ContextMode.FRESH_THREAD_PER_RUN, +) -> ScheduledTask: + return ScheduledTask( + task_id=task_id, + user_id=user_id, + title=task_id, + prompt="do the thing", + schedule=schedule, + status=status, + next_run_at=next_run_at, + thread_id=thread_id, + context_mode=context_mode, + ) + + +class TestProtocolConformance: + """`runtime_checkable` only checks that the methods exist, not their + signatures -- enough to catch a rename that updates one side only.""" + + async def test_repositories_satisfy_their_ports(self, tasks, runs): + assert isinstance(tasks, ScheduledTaskRepository) + assert isinstance(runs, ScheduledRunRepository) + + +class TestTaskOwnershipIsolation: + """Another user's task must read as absent, never as forbidden.""" + + async def test_get_hides_another_users_task(self, tasks): + await tasks.add(make_task(user_id="owner")) + assert await tasks.get("task-1", user_id="owner") is not None + assert await tasks.get("task-1", user_id="intruder") is None + + async def test_save_refuses_another_users_task(self, tasks): + await tasks.add(make_task(user_id="owner")) + assert await tasks.save(make_task(user_id="intruder")) is None + stored = await tasks.get("task-1", user_id="owner") + assert stored.user_id == "owner" + + async def test_delete_refuses_another_users_task(self, tasks): + await tasks.add(make_task(user_id="owner")) + assert await tasks.delete("task-1", user_id="intruder") is False + assert await tasks.get("task-1", user_id="owner") is not None + + async def test_list_by_user_excludes_other_owners(self, tasks): + await tasks.add(make_task("mine", user_id="owner")) + await tasks.add(make_task("theirs", user_id="someone-else")) + assert [t.task_id for t in await tasks.list_by_user("owner")] == ["mine"] + + async def test_list_by_thread_only_matches_bound_tasks(self, tasks): + await tasks.add(make_task("bound", context_mode=ContextMode.REUSE_THREAD, thread_id="thread-1")) + await tasks.add(make_task("unbound")) + listed = await tasks.list_by_user_and_thread("user-1", "thread-1") + assert [task.task_id for task in listed] == ["bound"] + + +class TestRoundTrip: + """What goes in comes back out -- including the value object, which the + SQL side has to rebuild from three separate columns.""" + + async def test_a_cron_task_round_trips(self, tasks): + original = await tasks.add(make_task(schedule=ScheduleSpec.cron_schedule("*/5 * * * *", "Asia/Shanghai"))) + stored = await tasks.get(original.task_id, user_id="user-1") + assert stored.schedule == original.schedule + assert stored.schedule.cron == "*/5 * * * *" + assert stored.schedule.timezone == "Asia/Shanghai" + + async def test_a_once_task_round_trips_to_the_same_instant(self, tasks): + original = await tasks.add(make_task(schedule=ONCE)) + stored = await tasks.get(original.task_id, user_id="user-1") + assert stored.schedule == original.schedule + + async def test_timestamps_come_back_timezone_aware(self, tasks): + """SQLite drops tzinfo on read; a naive datetime downstream would + compare wrong against an aware `now`.""" + await tasks.add(make_task(next_run_at=NOW)) + stored = await tasks.get("task-1", user_id="user-1") + assert stored.next_run_at.tzinfo is not None + assert stored.created_at.tzinfo is not None + + async def test_add_persists_the_aggregates_construction_instant(self, tasks): + """`created_at` has one source of truth: the aggregate's construction + instant. An adapter that mints its own timestamp on insert gives the + same fact two values -- domain tests and listings would disagree with + the stored row by however long the insert took.""" + task = make_task() + await tasks.add(task) + stored = await tasks.get(task.task_id, user_id="user-1") + assert stored.created_at == task.created_at + assert stored.updated_at == task.updated_at + + async def test_save_replaces_the_whole_aggregate(self, tasks): + from dataclasses import replace + + await tasks.add(make_task()) + stored = await tasks.get("task-1", user_id="user-1") + + await tasks.save(replace(stored, title="renamed", status=TaskStatus.PAUSED, run_count=7)) + + reloaded = await tasks.get("task-1", user_id="user-1") + assert reloaded.title == "renamed" + assert reloaded.status is TaskStatus.PAUSED + assert reloaded.run_count == 7 + + +class TestClaimDue: + async def test_claims_an_enabled_due_task(self, tasks): + await tasks.add(make_task(next_run_at=NOW - timedelta(minutes=1))) + + claimed = await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) + + assert [task.task_id for task in claimed] == ["task-1"] + assert claimed[0].status is TaskStatus.RUNNING + + async def test_a_claimed_task_is_not_claimed_again(self, tasks): + """The lease is not readable through the port, so the rule is asserted + the way it actually matters: a second claimer comes back empty.""" + await tasks.add(make_task(next_run_at=NOW - timedelta(minutes=1))) + assert len(await tasks.claim_due(now=NOW, lease_seconds=120, limit=10)) == 1 + + assert await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) == [] + + async def test_the_claim_expires(self, tasks): + await tasks.add(make_task(next_run_at=NOW - timedelta(minutes=1))) + await tasks.claim_due(now=NOW, lease_seconds=60, limit=10) + + later = NOW + timedelta(seconds=61) + reclaimed = await tasks.claim_due(now=later, lease_seconds=60, limit=10) + + assert [task.task_id for task in reclaimed] == ["task-1"] + + @pytest.mark.parametrize( + ("label", "task_kwargs"), + [ + ("not yet due", {"next_run_at": NOW + timedelta(minutes=1)}), + ("never scheduled", {"next_run_at": None}), + ("paused", {"status": TaskStatus.PAUSED, "next_run_at": NOW - timedelta(minutes=1)}), + ("completed", {"status": TaskStatus.COMPLETED, "next_run_at": NOW - timedelta(minutes=1)}), + ], + ) + async def test_does_not_claim(self, tasks, label, task_kwargs): + await tasks.add(make_task(**task_kwargs)) + assert await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) == [], label + + async def test_reclaims_a_task_stuck_mid_dispatch(self, tasks): + """The claimer died between claiming and launching: status is running, + the claim has expired, and the task must not stay unreachable.""" + await seed( + tasks, + make_task(status=TaskStatus.RUNNING, next_run_at=NOW - timedelta(minutes=1)), + claimed_until=NOW - timedelta(seconds=1), + ) + + claimed = await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) + + assert [task.task_id for task in claimed] == ["task-1"] + + async def test_claims_the_most_overdue_first_and_honours_the_limit(self, tasks): + await tasks.add(make_task("late", next_run_at=NOW - timedelta(hours=2))) + await tasks.add(make_task("later", next_run_at=NOW - timedelta(hours=1))) + + claimed = await tasks.claim_due(now=NOW, lease_seconds=120, limit=1) + + assert [task.task_id for task in claimed] == ["late"] + + +class TestRecordLaunch: + async def test_writes_bookkeeping_and_frees_the_task_for_the_next_round(self, tasks): + await tasks.add(make_task(next_run_at=NOW - timedelta(minutes=1))) + await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) + + await tasks.record_launch( + "task-1", + status=TaskStatus.ENABLED, + next_run_at=NOW - timedelta(seconds=1), + last_run_at=NOW, + last_run_id="run-1", + last_thread_id="thread-1", + last_error=None, + increment_run_count=True, + ) + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.ENABLED + assert task.last_run_id == "run-1" + assert task.run_count == 1 + # The claim was released, so the next round can take it again. + assert len(await tasks.claim_due(now=NOW, lease_seconds=120, limit=10)) == 1 + + async def test_protect_terminal_keeps_a_concurrently_finalized_verdict(self, tasks): + """A fast-failing run's completion hook lands before the launch path's + own write; the completion is authoritative.""" + await tasks.add(make_task(status=TaskStatus.COMPLETED, schedule=ONCE)) + + await tasks.record_launch( + "task-1", + status=TaskStatus.RUNNING, + next_run_at=None, + last_run_at=NOW, + last_run_id="run-1", + last_thread_id="thread-1", + last_error="stale", + increment_run_count=True, + protect_terminal=True, + ) + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.COMPLETED, "the terminal status must survive" + assert task.last_error is None, "the terminal error must survive" + assert task.last_run_id == "run-1", "bookkeeping is still recorded" + assert task.run_count == 1 + + async def test_without_protect_terminal_the_write_wins(self, tasks): + await tasks.add(make_task(status=TaskStatus.COMPLETED, schedule=ONCE)) + + await tasks.record_launch( + "task-1", + status=TaskStatus.FAILED, + next_run_at=None, + last_run_at=NOW, + last_run_id=None, + last_thread_id=None, + last_error="boom", + increment_run_count=False, + ) + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.FAILED + assert task.last_error == "boom" + + async def test_unknown_task_is_ignored(self, tasks): + await tasks.record_launch( + "nope", + status=TaskStatus.ENABLED, + next_run_at=None, + last_run_at=None, + last_run_id=None, + last_thread_id=None, + last_error=None, + increment_run_count=False, + ) + + +class TestRecordCompletion: + """The completion hook's write. + + Deliberately as narrow as `record_launch` is, and for the same reason: the + two race, so neither may write through the whole aggregate. This one owns + the terminal verdict and nothing else -- every scheduling field belongs to + the launch path, which may commit at any point around it. + """ + + async def test_records_the_terminal_status_and_error(self, tasks): + await tasks.add(make_task(status=TaskStatus.RUNNING, schedule=ONCE)) + + await tasks.record_completion("task-1", user_id="user-1", status=TaskStatus.FAILED, error="boom") + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.FAILED + assert task.last_error == "boom" + + async def test_a_none_status_records_the_error_and_leaves_the_status(self, tasks): + """A cron task's schedule outlives any single run, so only what went + wrong is recorded.""" + await tasks.add(make_task(status=TaskStatus.ENABLED, schedule=CRON)) + + await tasks.record_completion("task-1", user_id="user-1", status=None, error="boom") + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.ENABLED + assert task.last_error == "boom" + + async def test_never_rolls_back_a_concurrent_launch_write(self, tasks): + """The regression this method exists to prevent. + + A fast-failing run reaches the completion hook while the dispatch path + is still writing its bookkeeping. Whichever lands second must not undo + the other: the launch owns the schedule, the completion owns the + verdict. + """ + await tasks.add(make_task(next_run_at=NOW - timedelta(minutes=1))) + await tasks.claim_due(now=NOW, lease_seconds=120, limit=10) + + next_at = NOW + timedelta(days=1) + await tasks.record_launch( + "task-1", + status=TaskStatus.ENABLED, + next_run_at=next_at, + last_run_at=NOW, + last_run_id="run-1", + last_thread_id="thread-1", + last_error=None, + increment_run_count=True, + protect_terminal=True, + ) + + await tasks.record_completion("task-1", user_id="user-1", status=None, error="boom") + + task = await tasks.get("task-1", user_id="user-1") + assert task.next_run_at == next_at, "the launch path's next fire time must survive" + assert task.run_count == 1, "the launch path's run count must survive" + assert task.last_run_id == "run-1" + assert task.last_thread_id == "thread-1" + assert task.last_error == "boom", "the completion still records its verdict" + # The whole point: the task is still reachable by the next poll. + claimed = await tasks.claim_due(now=NOW + timedelta(days=2), lease_seconds=120, limit=10) + assert [t.task_id for t in claimed] == ["task-1"] + + async def test_another_users_task_is_untouched(self, tasks): + await tasks.add(make_task(status=TaskStatus.ENABLED)) + + await tasks.record_completion("task-1", user_id="someone-else", status=TaskStatus.FAILED, error="boom") + + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.ENABLED + assert task.last_error is None + + async def test_unknown_task_is_ignored(self, tasks): + await tasks.record_completion("nope", user_id="user-1", status=TaskStatus.FAILED, error="boom") + + +class TestCancelStuckOnceTasks: + async def test_cancels_a_launched_once_task_with_no_claim(self, tasks): + """Launched, so the claim was released; the completion hook then died + with the process. Expired-claim reclaim can never see this one.""" + await tasks.add(make_task(status=TaskStatus.RUNNING, schedule=ONCE)) + + assert await tasks.cancel_stuck_once_tasks(error="restarted") == 1 + task = await tasks.get("task-1", user_id="user-1") + assert task.status is TaskStatus.CANCELLED + assert task.last_error == "restarted" + + async def test_leaves_a_claimed_task_to_claim_expiry(self, tasks): + """Claimed but not launched -- expired-claim reclaim recovers it, and + cancelling here would throw away a dispatch that never happened.""" + await seed( + tasks, + make_task(status=TaskStatus.RUNNING, schedule=ONCE, next_run_at=NOW - timedelta(minutes=1)), + claimed_until=NOW + timedelta(seconds=60), + ) + assert await tasks.cancel_stuck_once_tasks(error="restarted") == 0 + + async def test_leaves_cron_tasks_alone(self, tasks): + await tasks.add(make_task(status=TaskStatus.RUNNING, schedule=CRON)) + assert await tasks.cancel_stuck_once_tasks(error="restarted") == 0 + + +class TestActiveSlot: + def _queued(self, task_id: str = "task-1") -> ScheduledRun: + return ScheduledRun.queued(task_id=task_id, thread_id="thread-1", scheduled_for=NOW, trigger=TriggerKind.SCHEDULED) + + def _tombstone(self, task_id: str = "task-1") -> ScheduledRun: + return ScheduledRun.skipped_tombstone(task_id=task_id, thread_id="thread-1", scheduled_for=NOW, trigger=TriggerKind.SCHEDULED) + + async def test_second_active_record_is_refused(self, runs): + await runs.add(self._queued()) + with pytest.raises(ActiveRunConflictError): + await runs.add(self._queued()) + + async def test_a_tombstone_never_conflicts(self, runs): + """Terminal from birth, so it sits outside the active-slot rule -- this + is why the skip path cannot reuse the queued factory.""" + await runs.add(self._queued()) + await runs.add(self._tombstone()) + assert await runs.count_active() == 1 + + async def test_another_task_is_unaffected(self, runs): + await runs.add(self._queued("task-1")) + await runs.add(self._queued("task-2")) + assert await runs.count_active() == 2 + + async def test_slot_frees_up_once_the_record_terminalizes(self, runs): + first = await runs.add(self._queued()) + await runs.update_status(first.record_id, status=RunStatus.SUCCESS, finished_at=NOW) + await runs.add(self._queued()) + assert await runs.count_active() == 1 + + async def test_has_active_is_scoped_to_one_task_while_count_is_global(self, runs): + await runs.add(self._queued("task-1")) + await runs.add(self._queued("task-2")) + assert await runs.has_active("task-1") is True + assert await runs.has_active("task-3") is False + assert await runs.count_active() == 2 + + async def test_a_run_round_trips(self, runs): + stored = await runs.add(self._queued()) + listed = await runs.list_by_task("task-1", limit=10, offset=0) + assert [r.record_id for r in listed] == [stored.record_id] + assert listed[0].trigger is TriggerKind.SCHEDULED + assert listed[0].scheduled_for.tzinfo is not None + + +class TestRunStatusWrites: + async def _one_queued(self, runs) -> ScheduledRun: + return await runs.add(ScheduledRun.queued(task_id="t", thread_id="th", scheduled_for=NOW, trigger=TriggerKind.SCHEDULED)) + + async def test_protect_terminal_backfills_without_overwriting(self, runs): + run = await self._one_queued(runs) + await runs.update_status(run.record_id, status=RunStatus.FAILED, error="boom", finished_at=NOW) + + # The launch path's write arrives late. + await runs.update_status(run.record_id, status=RunStatus.RUNNING, run_id="run-1", started_at=NOW, protect_terminal=True) + + stored = (await runs.list_by_task("t", limit=10, offset=0))[0] + assert stored.status is RunStatus.FAILED + assert stored.error == "boom" + assert stored.run_id == "run-1", "the id the completion could not know is backfilled" + assert stored.started_at == NOW + + async def test_unknown_record_is_ignored(self, runs): + await runs.update_status("nope", status=RunStatus.SUCCESS) + + async def test_mark_stale_active_terminalizes_orphans(self, runs): + active = await self._one_queued(runs) + done = await runs.add(ScheduledRun.skipped_tombstone(task_id="t", thread_id="th", scheduled_for=NOW, trigger=TriggerKind.SCHEDULED)) + + assert await runs.mark_stale_active(error="gateway restarted") == 1 + + by_id = {run.record_id: run for run in await runs.list_by_task("t", limit=10, offset=0)} + assert by_id[active.record_id].status is RunStatus.INTERRUPTED + assert by_id[active.record_id].error == "gateway restarted" + assert by_id[done.record_id].status is RunStatus.SKIPPED + + +class TestLauncherAndThreadLookup: + """Fake-only: these two ports have no SQL implementation -- one starts a + run and the other asks the thread store, so both land in later adapters.""" + + async def test_launcher_records_the_call_and_echoes_the_thread(self): + launcher = FakeRunLauncher() + launched = await launcher.launch( + thread_id="thread-1", + assistant_id="lead_agent", + prompt="go", + owner_user_id="user-1", + metadata={"scheduled_task_id": "task-1"}, + ) + assert launched.thread_id == "thread-1" + assert launcher.calls[0]["metadata"] == {"scheduled_task_id": "task-1"} + + async def test_launcher_can_be_driven_into_either_failure_branch(self): + boom = RuntimeError("nope") + launcher = FakeRunLauncher(fail_with=boom) + with pytest.raises(RuntimeError): + await launcher.launch(thread_id="t", assistant_id=None, prompt="p", owner_user_id=None, metadata={}) + assert len(launcher.calls) == 1, "the attempt is still recorded" + + async def test_thread_lookup_requires_both_existence_and_ownership(self): + lookup = FakeThreadLookup({"thread-1": "user-1"}) + assert await lookup.exists_for_user("thread-1", "user-1") is True + assert await lookup.exists_for_user("thread-1", "user-2") is False + assert await lookup.exists_for_user("missing", "user-1") is False diff --git a/backend/tests/test_schedule_poller.py b/backend/tests/test_schedule_poller.py new file mode 100644 index 000000000..2db99354d --- /dev/null +++ b/backend/tests/test_schedule_poller.py @@ -0,0 +1,149 @@ +"""Tests for the scheduler poller. + +The poller owns everything about *when* the service is asked to work, and +nothing about what the work means. Two behaviours here are load-bearing and +were carried over deliberately from `app/scheduler/service.py`: + + - a failing poll must not kill the loop for the rest of the process life + (a transient "database is locked" would otherwise silently stop every + scheduled task until the next restart), and + - startup reconciliation must not block startup. +""" + +from __future__ import annotations + +import asyncio +from datetime import datetime + +import pytest + +from app.scheduler.poller import SchedulePoller + + +class _SpyService: + """Stands in for `ScheduleService`, recording how the poller drove it.""" + + def __init__(self, *, run_once_error: Exception | None = None, reconcile_error: Exception | None = None) -> None: + self.run_once_error = run_once_error + self.reconcile_error = reconcile_error + self.run_once_calls: list[datetime] = [] + self.reconcile_calls: list[str] = [] + self.polled = asyncio.Event() + + async def run_once(self, *, now: datetime) -> list: + self.run_once_calls.append(now) + self.polled.set() + if self.run_once_error is not None: + raise self.run_once_error + return [] + + async def reconcile_on_startup(self, *, error: str) -> tuple[int, int]: + self.reconcile_calls.append(error) + if self.reconcile_error is not None: + raise self.reconcile_error + return 2, 1 + + +async def _drain(poller: SchedulePoller, service: _SpyService, *, polls: int = 1) -> None: + """Start the poller, wait for it to poll, then stop it.""" + await poller.start() + try: + for _ in range(polls): + service.polled.clear() + await asyncio.wait_for(service.polled.wait(), timeout=2) + finally: + await poller.stop() + + +class TestStartupReconciliation: + @pytest.mark.asyncio + async def test_start_reconciles_before_polling(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service) + assert len(service.reconcile_calls) == 1 + assert "restart" in service.reconcile_calls[0] + + @pytest.mark.asyncio + async def test_a_failed_reconcile_does_not_block_the_loop(self): + """Whether a partial reconcile blocks startup is the caller's policy, + and this caller's policy is: log it and keep scheduling. A gateway that + refuses to start because of leftover rows is worse than one that runs + with them.""" + service = _SpyService(reconcile_error=RuntimeError("db down")) + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service) + assert service.run_once_calls + + +class TestPolling: + @pytest.mark.asyncio + async def test_polls_repeatedly(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service, polls=3) + assert len(service.run_once_calls) >= 3 + + @pytest.mark.asyncio + async def test_each_poll_passes_a_tz_aware_now(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service) + assert all(now.tzinfo is not None for now in service.run_once_calls) + + @pytest.mark.asyncio + async def test_a_failing_poll_does_not_kill_the_loop(self): + """The regression this guards: one transient DB error used to end + scheduling for the rest of the process life.""" + service = _SpyService(run_once_error=RuntimeError("database is locked")) + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service, polls=3) + assert len(service.run_once_calls) >= 3 + + +class TestLifecycle: + @pytest.mark.asyncio + async def test_stop_is_awaited_to_completion(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await poller.start() + await asyncio.wait_for(service.polled.wait(), timeout=2) + await poller.stop() + before = len(service.run_once_calls) + await asyncio.sleep(0.05) + assert len(service.run_once_calls) == before + + @pytest.mark.asyncio + async def test_stop_does_not_wait_out_the_poll_interval(self): + """The loop waits on a stop event rather than sleeping, so shutdown is + prompt even with a production-sized interval.""" + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=300) + await poller.start() + await asyncio.wait_for(service.polled.wait(), timeout=2) + await asyncio.wait_for(poller.stop(), timeout=2) + + @pytest.mark.asyncio + async def test_start_is_idempotent(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await poller.start() + await poller.start() + try: + await asyncio.wait_for(service.polled.wait(), timeout=2) + finally: + await poller.stop() + assert len(service.reconcile_calls) == 1 + + @pytest.mark.asyncio + async def test_stop_without_start_is_a_no_op(self): + poller = SchedulePoller(_SpyService(), poll_interval_seconds=0.01) + await poller.stop() + + @pytest.mark.asyncio + async def test_it_can_be_restarted(self): + service = _SpyService() + poller = SchedulePoller(service, poll_interval_seconds=0.01) + await _drain(poller, service) + await _drain(poller, service) + assert len(service.reconcile_calls) == 2 diff --git a/backend/tests/test_schedule_response_models.py b/backend/tests/test_schedule_response_models.py new file mode 100644 index 000000000..1866c9f5d --- /dev/null +++ b/backend/tests/test_schedule_response_models.py @@ -0,0 +1,176 @@ +"""Tests for the scheduled-task HTTP response models. + +Two things are being pinned, and they pull in opposite directions: + + - **the leak is closed** -- server-owned and scheduler-internal fields that + the pre-migration router dumped from the ORM row must not appear, and + - **nothing the client already reads changed** -- the field set and the + timestamp spelling have to stay byte-compatible with what the legacy + `to_dict()` + `coerce_iso` path emitted, or the frontend breaks. +""" + +from __future__ import annotations + +import json +from datetime import UTC, datetime, timedelta, timezone + +import pytest + +from app.gateway.routers.schedule.models import ScheduledRunResponse, ScheduledTaskResponse +from deerflow.domain.schedule.model import ContextMode, RunStatus, ScheduledRun, ScheduledTask, ScheduleSpec, TaskStatus, TriggerKind + +# Exactly the frontend's `ScheduledTask` type (frontend/src/core/scheduled-tasks/types.ts). +FRONTEND_TASK_FIELDS = { + "id", + "thread_id", + "context_mode", + "title", + "prompt", + "schedule_type", + "schedule_spec", + "timezone", + "status", + "next_run_at", + "last_run_at", + "last_run_id", + "last_thread_id", + "last_error", + "run_count", + "created_at", + "updated_at", +} + +# Exactly the frontend's `ScheduledTaskRun` type. +FRONTEND_RUN_FIELDS = { + "id", + "task_id", + "thread_id", + "run_id", + "scheduled_for", + "trigger", + "status", + "error", + "started_at", + "finished_at", + "created_at", +} + +# What the ORM dump used to expose. Lease fields never reached the domain, so +# they cannot leak now; the other three can, and must not. +LEAKED_FIELDS = {"user_id", "assistant_id", "overlap_policy", "lease_owner", "lease_expires_at"} + + +def _task(**overrides) -> ScheduledTask: + defaults = dict( + task_id="task-1", + user_id="user-1", + title="Morning digest", + prompt="summarize", + schedule=ScheduleSpec.cron_schedule("0 9 * * *", "Asia/Shanghai"), + context_mode=ContextMode.REUSE_THREAD, + thread_id="thread-1", + status=TaskStatus.ENABLED, + next_run_at=datetime(2026, 8, 1, 1, 0, tzinfo=UTC), + last_run_at=datetime(2026, 7, 31, 1, 0, tzinfo=UTC), + last_run_id="run-1", + last_thread_id="thread-1", + last_error=None, + run_count=3, + created_at=datetime(2026, 7, 1, 0, 0, tzinfo=UTC), + updated_at=datetime(2026, 7, 31, 1, 0, tzinfo=UTC), + ) + return ScheduledTask(**{**defaults, **overrides}) + + +def _run(**overrides) -> ScheduledRun: + defaults = dict( + record_id="task-run-1", + task_id="task-1", + thread_id="thread-1", + scheduled_for=datetime(2026, 8, 1, 1, 0, tzinfo=UTC), + trigger=TriggerKind.SCHEDULED, + status=RunStatus.SUCCESS, + run_id="run-1", + error=None, + started_at=datetime(2026, 8, 1, 1, 0, 1, tzinfo=UTC), + finished_at=datetime(2026, 8, 1, 1, 0, 9, tzinfo=UTC), + created_at=datetime(2026, 8, 1, 1, 0, tzinfo=UTC), + ) + return ScheduledRun(**{**defaults, **overrides}) + + +class TestTheLeakIsClosed: + def test_no_server_owned_field_is_published(self): + emitted = set(ScheduledTaskResponse.from_domain(_task()).model_dump()) + assert emitted & LEAKED_FIELDS == set() + + def test_the_field_set_is_exactly_what_the_frontend_declares(self): + """Both directions matter: an extra field is a leak, a missing one + breaks a client that reads it.""" + assert set(ScheduledTaskResponse.from_domain(_task()).model_dump()) == FRONTEND_TASK_FIELDS + + def test_run_records_publish_exactly_the_declared_fields(self): + assert set(ScheduledRunResponse.from_domain(_run()).model_dump()) == FRONTEND_RUN_FIELDS + + +class TestFieldMapping: + def test_the_aggregate_id_is_published_as_id(self): + assert ScheduledTaskResponse.from_domain(_task()).id == "task-1" + + def test_the_record_id_is_published_as_id(self): + assert ScheduledRunResponse.from_domain(_run()).id == "task-run-1" + + def test_the_schedule_value_object_is_flattened_into_three_fields(self): + """The client's shape predates the value object and keeps the three + columns separate; the flattening is this model's job, not the + domain's.""" + response = ScheduledTaskResponse.from_domain(_task()) + assert response.schedule_type == "cron" + assert response.schedule_spec == {"cron": "0 9 * * *"} + assert response.timezone == "Asia/Shanghai" + + def test_a_once_schedule_flattens_to_run_at(self): + task = _task(schedule=ScheduleSpec.once_at(datetime(2026, 8, 1, 9, 0, tzinfo=UTC), "UTC")) + response = ScheduledTaskResponse.from_domain(task) + assert response.schedule_type == "once" + assert response.schedule_spec == {"run_at": "2026-08-01T09:00:00+00:00"} + + def test_enums_are_emitted_as_their_string_values(self): + """`StrEnum` would serialize acceptably either way, but the client + compares against string literals, so the model declares `str`.""" + payload = json.loads(ScheduledTaskResponse.from_domain(_task()).model_dump_json()) + assert payload["status"] == "enabled" + assert payload["context_mode"] == "reuse_thread" + + +class TestTimestampCompatibility: + def test_utc_timestamps_keep_the_legacy_spelling(self): + """`coerce_iso` emitted `astimezone(UTC).isoformat()`; anything else + is a silent wire change for every client parsing these.""" + payload = json.loads(ScheduledTaskResponse.from_domain(_task()).model_dump_json()) + assert payload["next_run_at"] == "2026-08-01T01:00:00+00:00" + assert payload["created_at"] == "2026-07-01T00:00:00+00:00" + + def test_a_non_utc_timestamp_is_converted_not_echoed(self): + """The legacy path normalised the offset away. A value that reached + the aggregate in another zone must not start emitting `+08:00`.""" + shanghai = timezone(timedelta(hours=8)) + task = _task(next_run_at=datetime(2026, 8, 1, 9, 0, tzinfo=shanghai)) + payload = json.loads(ScheduledTaskResponse.from_domain(task).model_dump_json()) + assert payload["next_run_at"] == "2026-08-01T01:00:00+00:00" + + def test_a_naive_timestamp_is_assumed_utc(self): + """Same assumption the repository's `_tz_aware` makes on read.""" + task = _task(next_run_at=datetime(2026, 8, 1, 1, 0)) + payload = json.loads(ScheduledTaskResponse.from_domain(task).model_dump_json()) + assert payload["next_run_at"] == "2026-08-01T01:00:00+00:00" + + @pytest.mark.parametrize("field", ["next_run_at", "last_run_at"]) + def test_absent_timestamps_stay_null(self, field): + payload = json.loads(ScheduledTaskResponse.from_domain(_task(**{field: None})).model_dump_json()) + assert payload[field] is None + + def test_run_timestamps_use_the_same_spelling(self): + payload = json.loads(ScheduledRunResponse.from_domain(_run()).model_dump_json()) + assert payload["scheduled_for"] == "2026-08-01T01:00:00+00:00" + assert payload["finished_at"] == "2026-08-01T01:00:09+00:00" diff --git a/backend/tests/test_schedule_router.py b/backend/tests/test_schedule_router.py new file mode 100644 index 000000000..2a02eb7ac --- /dev/null +++ b/backend/tests/test_schedule_router.py @@ -0,0 +1,599 @@ +"""Behaviour tests for the scheduled-task HTTP adapter. + +Driven through the handlers with a **real** `ScheduleService` over in-memory +port fakes, not a mocked service. The router's whole remaining job is protocol +translation, and half of that is turning domain errors into status codes -- a +mocked service would let those assertions pass without a domain error ever +being raised. + +`__wrapped__` unwraps `@require_permission` (authorization is covered by its +own suite) while keeping `_map_domain_errors` in the call path, which is the +layer under test here. + +Every scenario the deleted legacy suite (`test_scheduled_task_router_behavior.py`) +pinned has a counterpart below, plus the response-shape and error-mapping +cases the old dict-returning router could not express. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +import pytest +from fastapi import HTTPException +from schedule_fakes import ( + FakeRunLauncher, + FakeThreadLookup, + InMemoryScheduledRunRepository, + InMemoryScheduledTaskRepository, +) + +from app.gateway.routers.schedule import router as router_module +from deerflow.domain.schedule.commands import UNSET +from deerflow.domain.schedule.exceptions import LaunchFailedError, ThreadBusyError +from deerflow.domain.schedule.model import RunStatus, ScheduledRun, ScheduledTask, SchedulePolicy, ScheduleSpec, TaskStatus, TriggerKind +from deerflow.domain.schedule.service import ScheduleService + +USER = "user-1" +OTHER_USER = "user-2" +THREAD = "thread-1" + + +class _User: + def __init__(self, user_id: str | None) -> None: + self.id = user_id + + +@pytest.fixture +def tasks(): + return InMemoryScheduledTaskRepository() + + +@pytest.fixture +def runs(): + return InMemoryScheduledRunRepository() + + +@pytest.fixture +def launcher(): + return FakeRunLauncher() + + +@pytest.fixture +def service(tasks, runs, launcher): + return ScheduleService( + tasks=tasks, + runs=runs, + launcher=launcher, + threads=FakeThreadLookup({THREAD: USER}), + policy=SchedulePolicy(min_once_delay_seconds=60, max_concurrent_runs=3, lease_seconds=120), + ) + + +@pytest.fixture +def as_user(monkeypatch): + """Authenticate the handlers as a given user id (None = anonymous).""" + + def _apply(user_id: str | None = USER): + async def _resolve(_request): + return None if user_id is None else _User(user_id) + + monkeypatch.setattr(router_module, "get_optional_user_from_request", _resolve) + + _apply() + return _apply + + +def _call(handler, **kwargs): + """Invoke a route handler with authorization unwrapped.""" + return handler.__wrapped__(request=object(), **kwargs) + + +def _create_body(**overrides): + defaults = dict( + thread_id=None, + context_mode="fresh_thread_per_run", + title="Daily summary", + prompt="Summarize the thread", + schedule_type="cron", + schedule_spec={"cron": "0 9 * * *"}, + timezone="UTC", + ) + return router_module.CreateScheduledTaskRequest(**{**defaults, **overrides}) + + +def _update_body(**fields): + return router_module.UpdateScheduledTaskRequest(**fields) + + +async def _create(service, **overrides): + return await _call(router_module.create_scheduled_task, body=_create_body(**overrides), service=service) + + +class TestCreate: + @pytest.mark.asyncio + async def test_a_cron_task_is_created_and_rendered(self, service, as_user): + created = await _create(service) + assert created.title == "Daily summary" + assert created.schedule_type == "cron" + assert created.schedule_spec == {"cron": "0 9 * * *"} + assert created.status == "enabled" + assert created.next_run_at is not None + + @pytest.mark.asyncio + async def test_the_response_carries_no_server_owned_fields(self, service, as_user): + """The pre-migration router returned the ORM row, leaking `user_id`, + `overlap_policy` and `assistant_id` to every client.""" + rendered = (await _create(service)).model_dump() + assert {"user_id", "assistant_id", "overlap_policy", "lease_owner", "lease_expires_at"} & set(rendered) == set() + + @pytest.mark.asyncio + async def test_a_fresh_thread_task_needs_no_thread_id(self, service, as_user): + created = await _create(service, context_mode="fresh_thread_per_run", thread_id=None) + assert created.thread_id is None + + @pytest.mark.asyncio + async def test_reuse_thread_binds_the_thread(self, service, as_user): + created = await _create(service, context_mode="reuse_thread", thread_id=THREAD) + assert created.thread_id == THREAD + + @pytest.mark.asyncio + async def test_reuse_thread_without_a_thread_id_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, context_mode="reuse_thread", thread_id=None) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_reuse_of_an_unknown_thread_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, context_mode="reuse_thread", thread_id="thread-nope") + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_reuse_of_someone_elses_thread_is_also_404(self, service, as_user): + """Indistinguishable from "does not exist" on purpose: telling them + apart would let a caller probe for threads they cannot see.""" + as_user(OTHER_USER) + with pytest.raises(HTTPException) as caught: + await _create(service, context_mode="reuse_thread", thread_id=THREAD) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_an_unknown_schedule_type_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, schedule_type="teleport") + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_a_cron_without_its_key_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, schedule_type="cron", schedule_spec={}) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_a_malformed_cron_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, schedule_type="cron", schedule_spec={"cron": "0 9 * *"}) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_an_unknown_timezone_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, timezone="Mars/Olympus_Mons") + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_a_once_schedule_too_close_to_now_is_422(self, service, as_user): + """`min_once_delay_seconds` is operator policy, and it reaches the + aggregate through SchedulePolicy rather than being re-checked here.""" + soon = (datetime.now(UTC) + timedelta(seconds=5)).isoformat() + with pytest.raises(HTTPException) as caught: + await _create(service, schedule_type="once", schedule_spec={"run_at": soon}) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_a_once_schedule_in_the_past_is_422(self, service, as_user): + past = (datetime.now(UTC) - timedelta(days=1)).isoformat() + with pytest.raises(HTTPException) as caught: + await _create(service, schedule_type="once", schedule_spec={"run_at": past}) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_an_unknown_context_mode_is_422(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _create(service, context_mode="telepathy") + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_an_anonymous_caller_is_401(self, service, as_user): + as_user(None) + with pytest.raises(HTTPException) as caught: + await _create(service) + assert caught.value.status_code == 401 + + +class TestRead: + @pytest.mark.asyncio + async def test_a_task_is_fetched_by_id(self, service, as_user): + created = await _create(service) + fetched = await _call(router_module.get_scheduled_task, task_id=created.id, service=service) + assert fetched.id == created.id + + @pytest.mark.asyncio + async def test_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call(router_module.get_scheduled_task, task_id="task-nope", service=service) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_another_users_task_is_404(self, service, as_user): + created = await _create(service) + as_user(OTHER_USER) + with pytest.raises(HTTPException) as caught: + await _call(router_module.get_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_listing_returns_the_users_tasks(self, service, as_user): + await _create(service, title="One") + await _create(service, title="Two") + listed = await _call(router_module.list_scheduled_tasks, service=service) + assert {task.title for task in listed} == {"One", "Two"} + + @pytest.mark.asyncio + async def test_an_anonymous_listing_is_empty_not_401(self, service, as_user): + """Unchanged from the pre-migration router: this endpoint has always + answered an unauthenticated GET with an empty list.""" + as_user(None) + assert await _call(router_module.list_scheduled_tasks, service=service) == [] + + @pytest.mark.asyncio + async def test_thread_listing_filters_by_thread(self, service, as_user): + await _create(service, context_mode="reuse_thread", thread_id=THREAD, title="Bound") + await _create(service, title="Unbound") + listed = await _call(router_module.list_thread_scheduled_tasks, thread_id=THREAD, service=service) + assert [task.title for task in listed] == ["Bound"] + + +class TestToCommand: + """Transformation ① of the chain: wire shape -> command, owned by the + request models. Identity is injected as a parameter, never read off the + wire; the wire's `None` = "not supplied" convention becomes the command's + unambiguous `UNSET` here and nowhere else.""" + + def test_identity_is_injected_not_read_off_the_wire(self): + assert "user_id" not in router_module.CreateScheduledTaskRequest.model_fields + assert "user_id" not in router_module.UpdateScheduledTaskRequest.model_fields + cmd = _create_body().to_command(USER) + assert cmd.user_id == USER + + def test_create_carries_the_parsed_value_object(self): + cmd = _create_body().to_command(USER) + assert cmd.schedule.cron == "0 9 * * *" + assert cmd.title == "Daily summary" + + def test_update_translates_wire_none_to_unset(self): + cmd = _update_body(title="Renamed").to_command("task-1", USER, None) + assert cmd.title == "Renamed" + assert cmd.prompt is UNSET + assert cmd.schedule is UNSET + assert cmd.context is UNSET + + def test_update_completes_composite_fields_from_the_current_task(self): + current = ScheduledTask.create( + user_id=USER, + title="t", + prompt="p", + schedule=ScheduleSpec.cron_schedule("0 9 * * *", "Asia/Shanghai"), + context_mode="fresh_thread_per_run", + thread_id=None, + now=datetime(2026, 7, 27, tzinfo=UTC), + policy=SchedulePolicy(), + ) + cmd = _update_body(schedule_spec={"cron": "30 2 * * *"}).to_command(current.task_id, USER, current) + assert cmd.schedule.cron == "30 2 * * *" + assert cmd.schedule.timezone == "Asia/Shanghai", "the omitted zone comes from the current value object" + + cmd = _update_body(thread_id=THREAD).to_command(current.task_id, USER, current) + assert str(cmd.context.context_mode) == "fresh_thread_per_run", "the omitted mode comes from the current task" + assert cmd.context.thread_id == THREAD + + +class TestUpdate: + @pytest.mark.asyncio + async def test_a_title_is_updated_in_place(self, service, as_user): + created = await _create(service) + updated = await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(title="Renamed"), + service=service, + ) + assert updated.title == "Renamed" + assert updated.schedule_spec == created.schedule_spec + + @pytest.mark.asyncio + async def test_a_schedule_spec_change_keeps_the_existing_timezone(self, service, as_user): + """Only one of the three schedule parts was supplied; the router reads + the task to complete the value object rather than the service being + handed a partial one.""" + created = await _create(service, timezone="Asia/Shanghai") + updated = await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(schedule_spec={"cron": "30 2 * * *"}), + service=service, + ) + assert updated.schedule_spec == {"cron": "30 2 * * *"} + assert updated.timezone == "Asia/Shanghai" + + @pytest.mark.asyncio + async def test_a_timezone_change_keeps_the_existing_spec(self, service, as_user): + created = await _create(service) + updated = await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(timezone="Asia/Shanghai"), + service=service, + ) + assert updated.schedule_spec == created.schedule_spec + assert updated.timezone == "Asia/Shanghai" + + @pytest.mark.asyncio + async def test_a_timezone_change_on_a_once_task_keeps_the_same_instant(self, service, as_user): + """The `once` half of the fallback above. + + The omitted `run_at` is read straight off the current value object, and + the stored instant is already offset-aware, so re-zoning the schedule + must relabel it without moving it. + """ + created = await _create( + service, + schedule_type="once", + schedule_spec={"run_at": "2026-08-01T09:00:00+00:00"}, + timezone="UTC", + ) + updated = await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(timezone="Asia/Shanghai"), + service=service, + ) + assert updated.schedule_spec == created.schedule_spec + assert updated.timezone == "Asia/Shanghai" + assert updated.next_run_at == created.next_run_at + + @pytest.mark.asyncio + async def test_a_running_task_cannot_be_updated(self, service, tasks, as_user): + """Red line: the mutability gate lives in the aggregate now, and the + router only maps it onto 409.""" + created = await _create(service) + stored = await tasks.get(created.id, user_id=USER) + await tasks.save(stored.__class__(**{**stored.__dict__, "status": TaskStatus.RUNNING})) + with pytest.raises(HTTPException) as caught: + await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(title="Renamed"), + service=service, + ) + assert caught.value.status_code == 409 + + @pytest.mark.asyncio + async def test_updating_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call( + router_module.update_scheduled_task, + task_id="task-nope", + body=_update_body(title="Renamed"), + service=service, + ) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_a_malformed_new_schedule_is_422(self, service, as_user): + created = await _create(service) + with pytest.raises(HTTPException) as caught: + await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(schedule_spec={"cron": "0 9 * *"}), + service=service, + ) + assert caught.value.status_code == 422 + + @pytest.mark.asyncio + async def test_a_terminal_once_task_pushed_into_the_future_is_rearmed(self, service, tasks, as_user): + """Red line: without re-arming, the API answers 200 with a + `next_run_at` that can never fire, because claiming only admits + `enabled` rows.""" + future = (datetime.now(UTC) + timedelta(days=1)).isoformat() + created = await _create(service, schedule_type="once", schedule_spec={"run_at": future}) + stored = await tasks.get(created.id, user_id=USER) + await tasks.save(stored.__class__(**{**stored.__dict__, "status": TaskStatus.FAILED})) + + later = (datetime.now(UTC) + timedelta(days=2)).isoformat() + updated = await _call( + router_module.update_scheduled_task, + task_id=created.id, + body=_update_body(schedule_spec={"run_at": later}), + service=service, + ) + assert updated.status == "enabled" + assert updated.next_run_at is not None + + +class TestPauseResume: + @pytest.mark.asyncio + async def test_pause_then_resume(self, service, as_user): + created = await _create(service) + paused = await _call(router_module.pause_scheduled_task, task_id=created.id, service=service) + assert paused.status == "paused" + resumed = await _call(router_module.resume_scheduled_task, task_id=created.id, service=service) + assert resumed.status == "enabled" + + @pytest.mark.asyncio + async def test_pausing_a_running_task_is_409(self, service, tasks, as_user): + created = await _create(service) + stored = await tasks.get(created.id, user_id=USER) + await tasks.save(stored.__class__(**{**stored.__dict__, "status": TaskStatus.RUNNING})) + with pytest.raises(HTTPException) as caught: + await _call(router_module.pause_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 409 + + @pytest.mark.asyncio + async def test_pausing_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call(router_module.pause_scheduled_task, task_id="task-nope", service=service) + assert caught.value.status_code == 404 + + +class TestTrigger: + """Red line: this endpoint answers exactly 409 / 502 / 200, and the success + body is `{"id": ..., "triggered": true}`.""" + + @pytest.mark.asyncio + async def test_a_launched_trigger_is_200_with_the_documented_body(self, service, as_user): + created = await _create(service) + response = await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + assert response.model_dump() == {"id": created.id, "triggered": True} + + @pytest.mark.asyncio + async def test_a_busy_thread_is_409(self, service, launcher, as_user): + created = await _create(service) + launcher.fail_with = ThreadBusyError("thread already has an active run") + with pytest.raises(HTTPException) as caught: + await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 409 + + @pytest.mark.asyncio + async def test_an_active_run_for_the_same_task_is_409(self, service, runs, as_user): + """The other route to a conflict: the task's single active slot is + already taken, which the service reports without calling the launcher + at all.""" + created = await _create(service) + await runs.add( + ScheduledRun( + record_id="task-run-existing", + task_id=created.id, + thread_id="thread-x", + scheduled_for=datetime.now(UTC), + trigger=TriggerKind.SCHEDULED, + status=RunStatus.RUNNING, + ) + ) + with pytest.raises(HTTPException) as caught: + await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 409 + + @pytest.mark.asyncio + async def test_a_launch_failure_is_502(self, service, launcher, as_user): + """502, not 500: the failure is downstream of this API and the task + itself is intact.""" + created = await _create(service) + launcher.fail_with = LaunchFailedError("run backend unavailable") + with pytest.raises(HTTPException) as caught: + await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 502 + + @pytest.mark.asyncio + async def test_triggering_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call(router_module.trigger_scheduled_task, task_id="task-nope", service=service) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_a_paused_task_can_still_be_triggered(self, service, as_user): + """Manual dispatch is deliberately allowed while paused, and leaves the + task paused.""" + created = await _create(service) + await _call(router_module.pause_scheduled_task, task_id=created.id, service=service) + response = await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + assert response.triggered is True + after = await _call(router_module.get_scheduled_task, task_id=created.id, service=service) + assert after.status == "paused" + + +class TestDelete: + @pytest.mark.asyncio + async def test_a_task_is_deleted(self, service, as_user): + created = await _create(service) + response = await _call(router_module.delete_scheduled_task, task_id=created.id, service=service) + assert response.model_dump() == {"id": created.id, "deleted": True} + + @pytest.mark.asyncio + async def test_deleting_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call(router_module.delete_scheduled_task, task_id="task-nope", service=service) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_deleting_another_users_task_is_404(self, service, as_user): + created = await _create(service) + as_user(OTHER_USER) + with pytest.raises(HTTPException) as caught: + await _call(router_module.delete_scheduled_task, task_id=created.id, service=service) + assert caught.value.status_code == 404 + + +class TestRunHistory: + @pytest.mark.asyncio + async def test_runs_are_listed_for_an_owned_task(self, service, as_user): + created = await _create(service) + await _call(router_module.trigger_scheduled_task, task_id=created.id, service=service) + listed = await _call(router_module.list_scheduled_task_runs, task_id=created.id, service=service) + assert len(listed) == 1 + assert listed[0].task_id == created.id + assert listed[0].trigger == "manual" + + @pytest.mark.asyncio + async def test_history_of_an_unknown_task_is_404(self, service, as_user): + with pytest.raises(HTTPException) as caught: + await _call(router_module.list_scheduled_task_runs, task_id="task-nope", service=service) + assert caught.value.status_code == 404 + + @pytest.mark.asyncio + async def test_history_of_another_users_task_is_404(self, service, as_user): + """Ownership is checked on the parent task, so run history cannot be + read sideways.""" + created = await _create(service) + as_user(OTHER_USER) + with pytest.raises(HTTPException) as caught: + await _call(router_module.list_scheduled_task_runs, task_id=created.id, service=service) + assert caught.value.status_code == 404 + + +class TestErrorMapping: + def test_every_mapped_error_is_a_schedule_error(self): + from deerflow.domain.schedule.exceptions import ScheduleError + + assert all(issubclass(error, ScheduleError) for error in router_module._STATUS_BY_ERROR) + + @pytest.mark.asyncio + async def test_an_unclassified_domain_error_is_not_swallowed(self): + """A new domain error is a new protocol decision. Letting it through + surfaces as a 500 that has to be classified, rather than shipping as + whatever 4xx happened to be nearest.""" + from deerflow.domain.schedule.exceptions import ScheduleError + + class NewDomainError(ScheduleError): + pass + + @router_module._map_domain_errors + async def handler(): + raise NewDomainError("something new") + + with pytest.raises(NewDomainError): + await handler() + + @pytest.mark.asyncio + async def test_a_mapped_error_keeps_its_message_as_the_detail(self): + from deerflow.domain.schedule.exceptions import TaskNotFoundError + + @router_module._map_domain_errors + async def handler(): + raise TaskNotFoundError("Scheduled task not found") + + with pytest.raises(HTTPException) as caught: + await handler() + assert caught.value.detail == "Scheduled task not found" diff --git a/backend/tests/test_schedule_run_completion.py b/backend/tests/test_schedule_run_completion.py new file mode 100644 index 000000000..c029801b5 --- /dev/null +++ b/backend/tests/test_schedule_run_completion.py @@ -0,0 +1,176 @@ +"""Tests for the run-completion driving adapter. + +Every run in the process reaches the run runtime's completion callback, so +this adapter's first job is deciding which of them the schedule context has +any business with -- the filtering the legacy hook did inline, with four guard +clauses and an `if/elif` chain over runtime strings. A run that is not a +scheduled execution produces no call at all, which is why the service carries +no guard clauses of its own and never imports the run runtime. + +Driven through `__call__` rather than a conversion function: "was the use case +invoked, and with what" is the behaviour, and the previous split -- a pure +converter here plus the invoking closure in the composition root -- left that +second half untested, since the composition root is not where behaviour is +asserted. +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from app.adapters.schedule.run_completion import ScheduleRunCompletionListener +from deerflow.domain.schedule.model import RunStatus +from deerflow.domain.schedule.ports import RunOutcome +from deerflow.runtime.runs.manager import RunRecord +from deerflow.runtime.runs.schemas import DisconnectMode +from deerflow.runtime.runs.schemas import RunStatus as RuntimeRunStatus + +pytestmark = pytest.mark.asyncio + +TASK_METADATA = { + "scheduled_task_id": "task-1", + "scheduled_task_run_id": "rec-1", + "scheduled_trigger": "scheduled", +} + +_DEFAULT = object() + + +def _record(*, status, metadata=_DEFAULT, user_id="user-1", error=None, run_id="run-1"): + return RunRecord( + run_id=run_id, + thread_id="thread-1", + assistant_id=None, + status=status, + on_disconnect=DisconnectMode.continue_, + metadata=TASK_METADATA if metadata is _DEFAULT else metadata, + user_id=user_id, + error=error, + ) + + +class _RecordingService: + """Stands in for `ScheduleService`, capturing what the use case received.""" + + def __init__(self) -> None: + self.calls: list[tuple[RunOutcome, datetime]] = [] + + async def handle_run_completion(self, outcome: RunOutcome, *, now: datetime) -> None: + self.calls.append((outcome, now)) + + +async def _delivered(record) -> RunOutcome | None: + """The outcome the service was called with, or None when it was not.""" + service = _RecordingService() + await ScheduleRunCompletionListener(service)(record) + return service.calls[0][0] if service.calls else None + + +class TestTerminalStatusMapping: + async def test_success(self): + assert await _delivered(_record(status=RuntimeRunStatus.success)) == RunOutcome( + task_id="task-1", + record_id="rec-1", + run_id="run-1", + user_id="user-1", + status=RunStatus.SUCCESS, + error=None, + ) + + async def test_a_successful_run_carries_no_error_even_if_the_record_has_one(self): + """A stale `error` on a successful record must not be written back as + the task's `last_error`; success is success.""" + outcome = await _delivered(_record(status=RuntimeRunStatus.success, error="stale")) + assert outcome is not None and outcome.error is None + + @pytest.mark.parametrize("status", [RuntimeRunStatus.error, RuntimeRunStatus.timeout]) + async def test_error_and_timeout_both_become_failed(self, status): + outcome = await _delivered(_record(status=status, error="boom")) + assert outcome is not None + assert outcome.status is RunStatus.FAILED + assert outcome.error == "boom" + + async def test_interrupted_is_distinct_from_failed(self): + """Red line: a cancel or same-thread takeover is not an execution + failure -- the task must end CANCELLED, and only INTERRUPTED gets it + there.""" + outcome = await _delivered(_record(status=RuntimeRunStatus.interrupted)) + assert outcome is not None and outcome.status is RunStatus.INTERRUPTED + + async def test_an_interrupt_without_an_error_gets_an_explanatory_one(self): + outcome = await _delivered(_record(status=RuntimeRunStatus.interrupted, error=None)) + assert outcome is not None and outcome.error == "run was interrupted before completion" + + async def test_an_interrupts_own_error_is_preserved(self): + outcome = await _delivered(_record(status=RuntimeRunStatus.interrupted, error="cancelled by user")) + assert outcome is not None and outcome.error == "cancelled by user" + + +class TestFilteredOut: + """None of these reach the service at all -- not as an error, not as a + no-op call it has to recognise.""" + + @pytest.mark.parametrize("status", [RuntimeRunStatus.pending, RuntimeRunStatus.running]) + async def test_a_non_terminal_run_is_ignored(self, status): + assert await _delivered(_record(status=status)) is None + + async def test_an_ordinary_chat_run_is_ignored(self): + """Every run in the process reaches this callback; only scheduled + executions carry the metadata that makes one ours.""" + assert await _delivered(_record(status=RuntimeRunStatus.success, metadata={})) is None + + @pytest.mark.parametrize( + "metadata", + [ + {"scheduled_task_run_id": "rec-1"}, + {"scheduled_task_id": "task-1"}, + {"scheduled_task_id": "task-1", "scheduled_task_run_id": None}, + {"scheduled_task_id": 7, "scheduled_task_run_id": "rec-1"}, + {"scheduled_task_id": "task-1", "scheduled_task_run_id": 7}, + ], + ) + async def test_half_or_mistyped_metadata_is_ignored(self, metadata): + """Both ids are required and both must be strings -- `metadata` is a + free-form dict a client can influence, so its shape is not assumed.""" + assert await _delivered(_record(status=RuntimeRunStatus.success, metadata=metadata)) is None + + @pytest.mark.parametrize("user_id", [None, ""]) + async def test_a_run_without_an_owner_is_ignored(self, user_id): + """Every task read is scoped by user_id; without one there is no + authorized way to look the task up.""" + assert await _delivered(_record(status=RuntimeRunStatus.success, user_id=user_id)) is None + + async def test_none_metadata_is_ignored(self): + """`metadata` is typed as a dict but the legacy hook defended against + `None`; keep that defence rather than rediscovering it in production.""" + assert await _delivered(_record(status=RuntimeRunStatus.success, metadata=None)) is None + + async def test_the_service_is_left_entirely_alone(self): + """Explicitly: not called, rather than called with something falsy. + + This half had no coverage while the invoking closure lived in the + composition root. + """ + service = _RecordingService() + await ScheduleRunCompletionListener(service)(_record(status=RuntimeRunStatus.running)) + assert service.calls == [] + + +class TestUseCaseInvocation: + async def test_the_completion_is_stamped_with_the_current_instant(self): + service = _RecordingService() + before = datetime.now(UTC) + await ScheduleRunCompletionListener(service)(_record(status=RuntimeRunStatus.success)) + after = datetime.now(UTC) + + assert len(service.calls) == 1 + _, now = service.calls[0] + assert now.tzinfo is not None, "the domain is tz-aware throughout" + assert before <= now <= after + + async def test_one_finished_run_produces_exactly_one_call(self): + service = _RecordingService() + await ScheduleRunCompletionListener(service)(_record(status=RuntimeRunStatus.success)) + assert len(service.calls) == 1 diff --git a/backend/tests/test_schedule_run_launcher.py b/backend/tests/test_schedule_run_launcher.py new file mode 100644 index 000000000..90768427e --- /dev/null +++ b/backend/tests/test_schedule_run_launcher.py @@ -0,0 +1,149 @@ +"""Contract tests for the run-launcher anti-corruption layer. + +The `RunLauncher` port allows exactly two exceptions to escape -- `ThreadBusyError` +and `LaunchFailedError` -- and the domain branches on the difference: a busy +thread on a scheduled dispatch is a *skipped* occurrence, a genuine failure is a +*recorded* one. Everything the Gateway can raise is therefore classified here, +and this file is what pins that classification. + +That translation is the whole point of the adapter: it is what lets +`app/scheduler/service.py`'s `from fastapi import HTTPException` disappear +without the busy/failed distinction disappearing with it. + +There is deliberately no `isinstance(launcher, RunLauncher)` assertion. The +adapter inherits the port explicitly, which makes that check trivially true -- +and worse than useless: inheritance is exactly what turns a misspelled method +into a silent inherited `...` body returning `None`. Calling every port method +and asserting on what it returns, as this file does, is what actually catches +that. +""" + +from __future__ import annotations + +import asyncio + +import pytest +from fastapi import HTTPException + +from app.adapters.schedule.run_launcher import GatewayRunLauncher +from deerflow.domain.schedule.exceptions import LaunchFailedError, ThreadBusyError +from deerflow.domain.schedule.ports import LaunchedRun +from deerflow.runtime import ConflictError + +LAUNCH_KWARGS = { + "thread_id": "thread-1", + "assistant_id": "assistant-1", + "prompt": "do the thing", + "owner_user_id": "user-1", + "metadata": {"scheduled_task_id": "task-1", "scheduled_task_run_id": "rec-1", "scheduled_trigger": "scheduled"}, +} + + +def _launcher_returning(payload): + async def launch_run(**kwargs): + launch_run.calls.append(kwargs) + return payload + + launch_run.calls = [] + return GatewayRunLauncher(launch_run), launch_run + + +def _launcher_raising(exc): + async def launch_run(**kwargs): + raise exc + + return GatewayRunLauncher(launch_run) + + +class TestSuccessfulLaunch: + @pytest.mark.asyncio + async def test_returns_what_the_gateway_reported(self): + launcher, _ = _launcher_returning({"run_id": "run-9", "thread_id": "thread-other"}) + result = await launcher.launch(**LAUNCH_KWARGS) + assert result == LaunchedRun(run_id="run-9", thread_id="thread-other") + + @pytest.mark.asyncio + async def test_echoes_the_gateways_thread_not_the_requested_one(self): + """`LaunchedRun.thread_id` is documented as what actually ran, so the + adapter must not substitute the thread it asked for.""" + launcher, _ = _launcher_returning({"run_id": "run-9", "thread_id": "thread-substituted"}) + result = await launcher.launch(**LAUNCH_KWARGS) + assert result.thread_id == "thread-substituted" + + @pytest.mark.asyncio + async def test_carries_every_argument_through_untouched(self): + launcher, spy = _launcher_returning({"run_id": "r", "thread_id": "t"}) + await launcher.launch(**LAUNCH_KWARGS) + assert spy.calls == [LAUNCH_KWARGS] + + @pytest.mark.asyncio + async def test_a_malformed_gateway_payload_is_a_launch_failure(self): + """A missing id is not a busy thread -- it is the run path breaking its + own contract, which the domain records as a failure.""" + launcher, _ = _launcher_returning({"thread_id": "t"}) + with pytest.raises(LaunchFailedError): + await launcher.launch(**LAUNCH_KWARGS) + + +class TestBusyThreadTranslation: + @pytest.mark.asyncio + async def test_conflict_error_becomes_thread_busy(self): + launcher = _launcher_raising(ConflictError("thread already has an active run")) + with pytest.raises(ThreadBusyError): + await launcher.launch(**LAUNCH_KWARGS) + + @pytest.mark.asyncio + async def test_http_409_becomes_thread_busy(self): + """`start_run` rejects a busy thread as an HTTP 409 rather than a + ConflictError on some paths; both mean the same thing here.""" + launcher = _launcher_raising(HTTPException(status_code=409, detail="thread is busy")) + with pytest.raises(ThreadBusyError): + await launcher.launch(**LAUNCH_KWARGS) + + @pytest.mark.asyncio + async def test_the_cause_survives_in_the_message(self): + launcher = _launcher_raising(ConflictError("thread already has an active run")) + with pytest.raises(ThreadBusyError, match="thread already has an active run"): + await launcher.launch(**LAUNCH_KWARGS) + + @pytest.mark.asyncio + async def test_http_409_message_is_the_detail_not_the_repr(self): + launcher = _launcher_raising(HTTPException(status_code=409, detail="thread is busy")) + with pytest.raises(ThreadBusyError, match="^thread is busy$"): + await launcher.launch(**LAUNCH_KWARGS) + + +class TestFailureTranslation: + @pytest.mark.asyncio + @pytest.mark.parametrize("status_code", [400, 404, 422, 500, 502]) + async def test_any_other_http_error_is_a_launch_failure(self, status_code): + launcher = _launcher_raising(HTTPException(status_code=status_code, detail="nope")) + with pytest.raises(LaunchFailedError): + await launcher.launch(**LAUNCH_KWARGS) + + @pytest.mark.asyncio + async def test_an_arbitrary_exception_is_a_launch_failure(self): + launcher = _launcher_raising(RuntimeError("database is on fire")) + with pytest.raises(LaunchFailedError, match="database is on fire"): + await launcher.launch(**LAUNCH_KWARGS) + + @pytest.mark.asyncio + async def test_the_original_exception_is_chained(self): + """The domain only needs the two categories, but an operator reading a + log needs the real traceback.""" + original = RuntimeError("database is on fire") + launcher = _launcher_raising(original) + with pytest.raises(LaunchFailedError) as caught: + await launcher.launch(**LAUNCH_KWARGS) + assert caught.value.__cause__ is original + + +class TestCancellationIsNotSwallowed: + @pytest.mark.asyncio + async def test_cancelled_error_propagates(self): + """`CancelledError` is shutdown control flow, not a launch outcome. + Translating it to LaunchFailedError would record a spurious failure and + break cooperative cancellation of the poll loop.""" + launcher = _launcher_raising(asyncio.CancelledError()) + with pytest.raises(asyncio.CancelledError): + await launcher.launch(**LAUNCH_KWARGS) diff --git a/backend/tests/test_schedule_thread_lookup.py b/backend/tests/test_schedule_thread_lookup.py new file mode 100644 index 000000000..acb71e607 --- /dev/null +++ b/backend/tests/test_schedule_thread_lookup.py @@ -0,0 +1,71 @@ +"""Contract tests for the thread-lookup anti-corruption layer. + +The port asks one question -- "does this thread exist AND may this user use +it?" -- and deliberately answers both halves with a single bool, so a caller +cannot probe for the existence of threads they cannot see. These tests pin that +the adapter does not accidentally widen it back into two answers. +""" + +from __future__ import annotations + +import pytest + +from app.adapters.schedule.thread_lookup import ThreadStoreThreadLookup + + +class _RecordingThreadStore: + """Stands in for `ThreadMetaStore`, recording how it was asked.""" + + def __init__(self, owners: dict[str, str] | None = None) -> None: + self._owners = dict(owners or {}) + self.calls: list[tuple[str, str, bool]] = [] + + async def check_access(self, thread_id: str, user_id: str, *, require_existing: bool = False) -> bool: + self.calls.append((thread_id, user_id, require_existing)) + owner = self._owners.get(thread_id) + if owner is None: + # Mirrors the real store: absent rows pass a non-strict check. + return not require_existing + return owner == user_id + + +class TestTheOneQuestion: + @pytest.mark.asyncio + async def test_an_owned_thread_exists_for_its_user(self): + lookup = ThreadStoreThreadLookup(_RecordingThreadStore({"thread-1": "user-1"})) + assert await lookup.exists_for_user("thread-1", "user-1") is True + + @pytest.mark.asyncio + async def test_someone_elses_thread_does_not(self): + lookup = ThreadStoreThreadLookup(_RecordingThreadStore({"thread-1": "user-1"})) + assert await lookup.exists_for_user("thread-1", "user-2") is False + + @pytest.mark.asyncio + async def test_a_missing_thread_does_not(self): + """This is the half `require_existing` buys: without it the store + treats an absent row as accessible, and a task could be bound to a + thread that does not exist.""" + lookup = ThreadStoreThreadLookup(_RecordingThreadStore()) + assert await lookup.exists_for_user("thread-nope", "user-1") is False + + @pytest.mark.asyncio + async def test_missing_and_forbidden_are_indistinguishable(self): + lookup = ThreadStoreThreadLookup(_RecordingThreadStore({"thread-1": "user-1"})) + missing = await lookup.exists_for_user("thread-nope", "user-2") + forbidden = await lookup.exists_for_user("thread-1", "user-2") + assert missing == forbidden is False + + +class TestHowTheStoreIsAsked: + @pytest.mark.asyncio + async def test_require_existing_is_always_set(self): + store = _RecordingThreadStore({"thread-1": "user-1"}) + await ThreadStoreThreadLookup(store).exists_for_user("thread-1", "user-1") + assert store.calls == [("thread-1", "user-1", True)] + + @pytest.mark.asyncio + async def test_the_result_is_a_real_bool(self): + """The port is typed `bool`; a truthy row object leaking through would + satisfy the domain's `if` and still be the wrong contract.""" + store = _RecordingThreadStore({"thread-1": "user-1"}) + assert await ThreadStoreThreadLookup(store).exists_for_user("thread-1", "user-1") is True diff --git a/backend/tests/test_scheduled_task_claims.py b/backend/tests/test_scheduled_task_claims.py deleted file mode 100644 index 83db44628..000000000 --- a/backend/tests/test_scheduled_task_claims.py +++ /dev/null @@ -1,152 +0,0 @@ -from datetime import UTC, datetime, timedelta - -import pytest - -from deerflow.config.database_config import DatabaseConfig -from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config -from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository - - -@pytest.mark.asyncio -async def test_claim_due_tasks_claims_only_due_rows(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - repo = ScheduledTaskRepository(sf) - - due = datetime.now(UTC) - timedelta(minutes=1) - future = datetime.now(UTC) + timedelta(hours=1) - - await repo.create( - task_id="due-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Due", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=due, - ) - await repo.create( - task_id="future-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Future", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=future, - ) - - claimed = await repo.claim_due_tasks( - now=datetime.now(UTC), - lease_owner="worker-1", - lease_seconds=120, - limit=10, - ) - assert [task["id"] for task in claimed] == ["due-1"] - - await close_engine() - - -@pytest.mark.asyncio -async def test_claim_reclaims_task_stuck_in_running_with_expired_lease(tmp_path): - """A task whose claiming process died mid-dispatch must stay reclaimable. - - Regression for the lease dead-end bug: claim flips status to ``running``, - and the old claim query only selected ``status == 'enabled'``, so a crash - between claim and dispatch left the task permanently un-triggerable. - """ - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - repo = ScheduledTaskRepository(sf) - - now = datetime.now(UTC) - due = now - timedelta(minutes=5) - - await repo.create( - task_id="stuck-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Stuck", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=due, - ) - - first_claim = await repo.claim_due_tasks( - now=now, - lease_owner="dead-worker", - lease_seconds=60, - limit=10, - ) - assert first_claim[0]["id"] == "stuck-1" - assert first_claim[0]["status"] == "running" - - # Simulate the claiming process dying: lease expires, status stays "running". - expired_now = now + timedelta(seconds=120) - reclaimed = await repo.claim_due_tasks( - now=expired_now, - lease_owner="new-worker", - lease_seconds=60, - limit=10, - ) - assert [task["id"] for task in reclaimed] == ["stuck-1"] - assert reclaimed[0]["lease_owner"] == "new-worker" - - await close_engine() - - -@pytest.mark.asyncio -async def test_claim_skips_task_with_active_lease(tmp_path): - """A task whose lease has not expired must not be reclaimed.""" - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - repo = ScheduledTaskRepository(sf) - - now = datetime.now(UTC) - due = now - timedelta(minutes=5) - - await repo.create( - task_id="active-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Active", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=due, - ) - - await repo.claim_due_tasks( - now=now, - lease_owner="worker-1", - lease_seconds=300, - limit=10, - ) - - # Lease still valid — second claim within the same process must not re-grab it. - reclaimed = await repo.claim_due_tasks( - now=now + timedelta(seconds=10), - lease_owner="worker-2", - lease_seconds=300, - limit=10, - ) - assert reclaimed == [] - - await close_engine() diff --git a/backend/tests/test_scheduled_task_dispatch_race.py b/backend/tests/test_scheduled_task_dispatch_race.py deleted file mode 100644 index 6dcc01dc0..000000000 --- a/backend/tests/test_scheduled_task_dispatch_race.py +++ /dev/null @@ -1,219 +0,0 @@ -"""Concurrency regression tests for the scheduled-task dispatch TOCTOU. - -``ScheduledTaskService.dispatch_task`` guards the "at most one active run per -task when overlap_policy=skip" invariant with a non-atomic -``has_active_runs`` check followed by a separate ``create(status="queued")`` -insert. Two concurrent dispatches (double-click, client retry, or a manual -trigger racing the poller) can both pass the check and both launch. The fix -makes the database the atomic arbiter via the partial unique index -``uq_scheduled_task_run_active`` (``task_id WHERE status IN -('queued','running')``); the losing insert is translated to the typed -``ActiveScheduledRunConflict`` and collapsed to the same outcome as the -fast-path check. - -These tests drive the REAL ``ScheduledTaskRunRepository`` + ``ScheduledTaskService`` -against a real file-backed ``sqlite+aiosqlite`` DB (so the index is actually -enforced), with a fake ``launch_run`` that only records launches. -""" - -from __future__ import annotations - -import asyncio -from datetime import UTC, datetime - -import pytest - -from app.scheduler.service import ScheduledTaskService -from deerflow.config.database_config import DatabaseConfig -from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config -from deerflow.persistence.scheduled_task_runs import ActiveScheduledRunConflict, ScheduledTaskRunRepository -from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository - -pytestmark = pytest.mark.asyncio - -_ACTIVE_STATUSES = {"queued", "running"} - - -class _BarrierRunRepo(ScheduledTaskRunRepository): - """Real repository that only releases both dispatchers past - ``has_active_runs`` once both have read it, so their ``create()`` calls - genuinely race for the task's single active slot — a deterministic - reproduction of the check-then-insert TOCTOU.""" - - def __init__(self, session_factory, barrier: asyncio.Barrier | None) -> None: - super().__init__(session_factory) - self._barrier = barrier - - async def has_active_runs(self, task_id: str) -> bool: - result = await super().has_active_runs(task_id) - if self._barrier is not None: - await self._barrier.wait() - return result - - -def _make_service(task_repo, run_repo, launched: list) -> ScheduledTaskService: - async def fake_launch(**kwargs): - # Yield so a truly-concurrent sibling can interleave, then record. - await asyncio.sleep(0) - launched.append(kwargs) - return {"run_id": f"run-{len(launched)}", "thread_id": kwargs["thread_id"]} - - return ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=10, - ) - - -async def _seed_task(task_repo: ScheduledTaskRepository, task_id: str) -> dict: - # fresh_thread_per_run: every dispatch gets a NEW thread_id, so #4003's - # per-thread uq_runs_thread_active can never fire for two dispatches of the - # same task — this is precisely the gap the per-task index closes. - await task_repo.create( - task_id=task_id, - user_id="user-1", - thread_id=None, - context_mode="fresh_thread_per_run", - assistant_id="lead_agent", - title=task_id, - prompt="do the thing", - schedule_type="cron", - schedule_spec={"cron": "*/5 * * * *"}, - timezone="UTC", - next_run_at=None, - ) - task = await task_repo.get(task_id, user_id="user-1") - assert task is not None - assert task["overlap_policy"] == "skip" - return task - - -async def _active_run_count(run_repo: ScheduledTaskRunRepository, task_id: str) -> int: - rows = await run_repo.list_by_task(task_id, limit=100) - return sum(1 for row in rows if row["status"] in _ACTIVE_STATUSES) - - -async def test_two_concurrent_manual_dispatches_launch_exactly_once(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - try: - sf = get_session_factory() - assert sf is not None - task_repo = ScheduledTaskRepository(sf) - run_repo = _BarrierRunRepo(sf, asyncio.Barrier(2)) - launched: list = [] - service = _make_service(task_repo, run_repo, launched) - task = await _seed_task(task_repo, "task-race-manual") - now = datetime.now(UTC) - - results = await asyncio.gather( - service.dispatch_task(dict(task), now=now, trigger="manual"), - service.dispatch_task(dict(task), now=now, trigger="manual"), - ) - - outcomes = sorted(result["outcome"] for result in results) - # Exactly one wins the active slot; the loser is a 409-style conflict. - assert outcomes == ["conflict", "launched"], outcomes - assert len(launched) == 1, launched - assert await _active_run_count(run_repo, "task-race-manual") == 1 - # The manual loser records no run-history row (nothing was scheduled). - conflict = next(r for r in results if r["outcome"] == "conflict") - assert conflict["task_run_id"] is None - finally: - await close_engine() - - -async def test_scheduled_and_manual_dispatch_launch_exactly_once(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - try: - sf = get_session_factory() - assert sf is not None - task_repo = ScheduledTaskRepository(sf) - run_repo = _BarrierRunRepo(sf, asyncio.Barrier(2)) - launched: list = [] - service = _make_service(task_repo, run_repo, launched) - task = await _seed_task(task_repo, "task-race-mixed") - now = datetime.now(UTC) - - results = await asyncio.gather( - service.dispatch_task(dict(task), now=now, trigger="scheduled"), - service.dispatch_task(dict(task), now=now, trigger="manual"), - ) - - outcomes = sorted(result["outcome"] for result in results) - # Whichever won launched; the loser is conflict (manual) or skipped - # (scheduled). Which one wins is timing-dependent, but exactly one runs. - assert outcomes.count("launched") == 1, outcomes - assert set(outcomes) <= {"launched", "conflict", "skipped"}, outcomes - assert len(launched) == 1, launched - assert await _active_run_count(run_repo, "task-race-mixed") == 1 - finally: - await close_engine() - - -async def test_natural_timing_concurrent_dispatch_launches_exactly_once(tmp_path): - # No barrier: exercise the fix under the same natural interleaving that - # reproduced the bug (5/5 both-launch on main). The fix must hold whether - # the second dispatch is caught by the has_active_runs fast path or by the - # index-violation path. - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - try: - sf = get_session_factory() - assert sf is not None - task_repo = ScheduledTaskRepository(sf) - run_repo = ScheduledTaskRunRepository(sf) - for i in range(5): - launched: list = [] - service = _make_service(task_repo, run_repo, launched) - task_id = f"task-natural-{i}" - task = await _seed_task(task_repo, task_id) - now = datetime.now(UTC) - - results = await asyncio.gather( - service.dispatch_task(dict(task), now=now, trigger="manual"), - service.dispatch_task(dict(task), now=now, trigger="manual"), - ) - - outcomes = sorted(result["outcome"] for result in results) - assert outcomes.count("launched") == 1, (i, outcomes) - assert len(launched) == 1, (i, launched) - assert await _active_run_count(run_repo, task_id) == 1, i - finally: - await close_engine() - - -async def test_partial_unique_index_enforces_one_active_run_per_task(tmp_path): - # Focused repository-level test of the index semantics + the typed conflict. - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - try: - sf = get_session_factory() - assert sf is not None - run_repo = ScheduledTaskRunRepository(sf) - now = datetime(2026, 7, 2, 1, 0, tzinfo=UTC) - - await run_repo.create(run_record_id="r1", task_id="t1", thread_id="th1", scheduled_for=now, trigger="scheduled", status="queued") - - # queued -> running is a same-row UPDATE: keeps the one active slot, no - # violation (this is the normal launch transition). - await run_repo.update_status("r1", status="running", run_id="run-1", started_at=now) - assert await run_repo.has_active_runs("t1") is True - - # A second active insert for the same task is a domain conflict. - with pytest.raises(ActiveScheduledRunConflict): - await run_repo.create(run_record_id="r2", task_id="t1", thread_id="th2", scheduled_for=now, trigger="manual", status="queued") - - # Terminal-status rows for the same task are outside the index predicate. - await run_repo.create(run_record_id="r3", task_id="t1", thread_id="th3", scheduled_for=now, trigger="scheduled", status="skipped") - - # A different task's active row is independent. - await run_repo.create(run_record_id="r4", task_id="t2", thread_id="th4", scheduled_for=now, trigger="scheduled", status="queued") - - # Finishing the active run frees the slot; a fresh active row is allowed. - await run_repo.update_status("r1", status="success", run_id="run-1", finished_at=now) - assert await run_repo.has_active_runs("t1") is False - await run_repo.create(run_record_id="r5", task_id="t1", thread_id="th5", scheduled_for=now, trigger="scheduled", status="queued") - assert await run_repo.has_active_runs("t1") is True - finally: - await close_engine() diff --git a/backend/tests/test_scheduled_task_models.py b/backend/tests/test_scheduled_task_models.py index c4f7b1f4b..47cfbc94f 100644 --- a/backend/tests/test_scheduled_task_models.py +++ b/backend/tests/test_scheduled_task_models.py @@ -1,4 +1,9 @@ +import re + +from sqlalchemy import Index + from deerflow.config.app_config import AppConfig +from deerflow.domain.schedule.model import ACTIVE_RUN_STATUSES from deerflow.persistence.models import ScheduledTaskRow, ScheduledTaskRunRow @@ -17,3 +22,37 @@ def test_app_config_exposes_scheduler_section(): def test_scheduled_task_models_registered(): assert ScheduledTaskRow.__tablename__ == "scheduled_tasks" assert ScheduledTaskRunRow.__tablename__ == "scheduled_task_runs" + + +def _active_run_index() -> Index: + return next(arg for arg in ScheduledTaskRunRow.__table_args__ if isinstance(arg, Index) and arg.name == "uq_scheduled_task_run_active") + + +def test_active_run_index_arbitrates_one_active_run_per_task(): + """The index is the atomic arbiter of the overlap rule, so its shape is a + contract, not a detail: unique, keyed on `task_id` alone.""" + index = _active_run_index() + assert index.unique is True + assert [column.name for column in index.expressions] == ["task_id"] + + +def test_active_run_index_predicate_matches_the_domain_constant(): + """`ACTIVE_RUN_STATUSES` and this predicate must stay in lockstep. + + The domain's fast path (`has_active`) and the index disagree the moment + they drift, which silently decouples the overlap check from its arbiter. + The domain tests cannot assert this -- they are deliberately + dependency-free and cannot import an ORM model -- so the assertion the + `ACTIVE_RUN_STATUSES` docstring promises lives here. + + Both dialect predicates are checked: `create_all` renders the SQLite one + and production renders the Postgres one, so a drift in either is real. + """ + index = _active_run_index() + expected = {str(status) for status in ACTIVE_RUN_STATUSES} + assert expected == {"queued", "running"}, "domain constant changed -- update the ORM predicates below" + + predicates = {key: str(value) for key, value in index.dialect_kwargs.items() if key.endswith("_where")} + assert set(predicates) == {"sqlite_where", "postgresql_where"}, "a dialect lost its partial-index predicate" + for dialect, predicate in predicates.items(): + assert set(re.findall(r"'([^']+)'", predicate)) == expected, f"{dialect} predicate drifted from ACTIVE_RUN_STATUSES" diff --git a/backend/tests/test_scheduled_task_repository.py b/backend/tests/test_scheduled_task_repository.py deleted file mode 100644 index a9101f440..000000000 --- a/backend/tests/test_scheduled_task_repository.py +++ /dev/null @@ -1,324 +0,0 @@ -from datetime import UTC, datetime - -import pytest - -from deerflow.config.database_config import DatabaseConfig -from deerflow.persistence.engine import close_engine, get_session_factory, init_engine_from_config -from deerflow.persistence.scheduled_task_runs import ScheduledTaskRunRepository -from deerflow.persistence.scheduled_tasks import ScheduledTaskRepository - - -@pytest.mark.asyncio -async def test_scheduled_task_repository_create_and_list(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRepository(sf) - created = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize this thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="Asia/Shanghai", - next_run_at=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - ) - - assert created["id"] == "task-1" - listed = await repo.list_by_user("user-1") - assert [task["id"] for task in listed] == ["task-1"] - - await close_engine() - - -@pytest.mark.asyncio -async def test_scheduled_task_run_repository_records_history(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRunRepository(sf) - row = await repo.create( - run_record_id="task-run-1", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="manual", - status="queued", - ) - - assert row["id"] == "task-run-1" - history = await repo.list_by_task("task-1") - assert [entry["id"] for entry in history] == ["task-run-1"] - - await close_engine() - - -@pytest.mark.asyncio -async def test_mark_stale_active_runs_fails_orphaned_runs(tmp_path): - """Runs stuck in queued/running after a process crash are swept to interrupted.""" - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRunRepository(sf) - # The queued and running rows live on different tasks: mark_stale_active_runs - # is a global sweep (no task filter), and the uq_scheduled_task_run_active - # partial unique index forbids two active rows on one task_id, so the pair - # that proves both active statuses get swept must be spread across tasks. - await repo.create( - run_record_id="task-run-queued", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="scheduled", - status="queued", - ) - await repo.create( - run_record_id="task-run-running", - task_id="task-2", - thread_id="thread-2", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="scheduled", - status="running", - ) - # A terminal row on task-1 (outside the index predicate) coexists with the - # active queued row and must be left untouched by the sweep. - await repo.create( - run_record_id="task-run-success", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="scheduled", - status="success", - ) - - swept = await repo.mark_stale_active_runs(error="interrupted: gateway restarted") - assert swept == 2 - - by_id = {entry["id"]: entry for entry in await repo.list_by_task("task-1")} - by_id.update({entry["id"]: entry for entry in await repo.list_by_task("task-2")}) - assert by_id["task-run-queued"]["status"] == "interrupted" - assert by_id["task-run-running"]["status"] == "interrupted" - assert by_id["task-run-success"]["status"] == "success" - - await close_engine() - - -@pytest.mark.asyncio -async def test_update_status_protect_terminal_keeps_completion_result(tmp_path): - """The launch-path "running" write must not clobber a terminal status - already committed by the completion hook (launch/completion race).""" - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRunRepository(sf) - await repo.create( - run_record_id="task-run-race", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="scheduled", - status="queued", - ) - # Completion hook wins the race and commits the terminal state first. - await repo.update_status("task-run-race", status="failed", run_id="run-1", error="boom", finished_at=datetime(2026, 7, 2, 1, 1, tzinfo=UTC)) - # Late launch-path write: keeps terminal status/error, backfills started_at. - await repo.update_status("task-run-race", status="running", run_id="run-1", started_at=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), protect_terminal=True) - - entry = (await repo.list_by_task("task-1"))[0] - assert entry["status"] == "failed" - assert entry["error"] == "boom" - assert entry["started_at"] is not None - - await close_engine() - - -@pytest.mark.asyncio -async def test_has_active_runs_sees_only_queued_and_running(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRunRepository(sf) - assert await repo.has_active_runs("task-1") is False - await repo.create( - run_record_id="task-run-active", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - trigger="scheduled", - status="running", - ) - assert await repo.has_active_runs("task-1") is True - await repo.update_status("task-run-active", status="success", run_id="run-1") - assert await repo.has_active_runs("task-1") is False - - await close_engine() - - -@pytest.mark.asyncio -async def test_cancel_stuck_once_tasks_reconciles_orphaned_running(tmp_path): - """Launched (lease cleared) once tasks stuck in running are cancelled at - startup; leased ones are left for expired-lease reclaim.""" - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRepository(sf) - for task_id, schedule_type, status in ( - ("task-once-stuck", "once", "running"), - ("task-once-done", "once", "completed"), - ("task-cron-running", "cron", "running"), - ): - await repo.create( - task_id=task_id, - user_id="user-1", - thread_id=None, - context_mode="fresh_thread_per_run", - assistant_id="lead_agent", - title=task_id, - prompt="p", - schedule_type=schedule_type, - schedule_spec={"run_at": "2026-07-02T01:00:00+00:00"} if schedule_type == "once" else {"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - await repo.update(task_id, user_id="user-1", updates={"status": status}) - # A claimed-but-not-launched once task still holds its lease: keep it. - await repo.create( - task_id="task-once-leased", - user_id="user-1", - thread_id=None, - context_mode="fresh_thread_per_run", - assistant_id="lead_agent", - title="task-once-leased", - prompt="p", - schedule_type="once", - schedule_spec={"run_at": "2026-07-02T01:00:00+00:00"}, - timezone="UTC", - next_run_at=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - ) - await repo.update("task-once-leased", user_id="user-1", updates={"status": "running", "lease_expires_at": datetime(2026, 7, 2, 1, 2, tzinfo=UTC)}) - - cancelled = await repo.cancel_stuck_once_tasks(error="interrupted: gateway restarted") - assert cancelled == 1 - - by_id = {t["id"]: t for t in await repo.list_by_user("user-1")} - assert by_id["task-once-stuck"]["status"] == "cancelled" - assert by_id["task-once-stuck"]["last_error"] == "interrupted: gateway restarted" - assert by_id["task-once-done"]["status"] == "completed" - assert by_id["task-cron-running"]["status"] == "running" - assert by_id["task-once-leased"]["status"] == "running" - - await close_engine() - - -@pytest.mark.asyncio -async def test_update_after_launch_protect_terminal_keeps_hook_result(tmp_path): - """The launch-path bookkeeping write must not clobber a terminal task - status committed first by the completion hook (fast-failing run).""" - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRepository(sf) - await repo.create( - task_id="task-race", - user_id="user-1", - thread_id=None, - context_mode="fresh_thread_per_run", - assistant_id="lead_agent", - title="task-race", - prompt="p", - schedule_type="once", - schedule_spec={"run_at": "2026-07-02T01:00:00+00:00"}, - timezone="UTC", - next_run_at=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - ) - # Completion hook wins the race: task finalized as failed. - await repo.update("task-race", user_id="user-1", updates={"status": "failed", "last_error": "boom"}) - # Late launch-path write with protection keeps the hook's outcome. - await repo.update_after_launch( - "task-race", - status="running", - next_run_at=None, - last_run_at=datetime(2026, 7, 2, 1, 0, tzinfo=UTC), - last_run_id="run-1", - last_thread_id="thread-1", - last_error=None, - increment_run_count=True, - protect_terminal=True, - ) - - task = await repo.get("task-race", user_id="user-1") - assert task is not None - assert task["status"] == "failed" - assert task["last_error"] == "boom" - # Launch bookkeeping still recorded. - assert task["last_run_id"] == "run-1" - assert task["run_count"] == 1 - - await close_engine() - - -@pytest.mark.asyncio -async def test_list_by_task_paginates(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRunRepository(sf) - for i in range(5): - await repo.create( - run_record_id=f"task-run-{i}", - task_id="task-1", - thread_id="thread-1", - scheduled_for=datetime(2026, 7, 2, 1, i, tzinfo=UTC), - trigger="scheduled", - status="success", - ) - - assert await repo.count_active_runs() == 0 - page1 = await repo.list_by_task("task-1", limit=2) - page2 = await repo.list_by_task("task-1", limit=2, offset=2) - assert len(page1) == 2 - assert len(page2) == 2 - assert {e["id"] for e in page1}.isdisjoint({e["id"] for e in page2}) - - await close_engine() - - -@pytest.mark.asyncio -async def test_list_by_user_and_thread_filters_in_sql(tmp_path): - await init_engine_from_config(DatabaseConfig(backend="sqlite", sqlite_dir=str(tmp_path))) - sf = get_session_factory() - assert sf is not None - - repo = ScheduledTaskRepository(sf) - for task_id, thread_id in (("task-a", "thread-1"), ("task-b", "thread-2"), ("task-c", "thread-1")): - await repo.create( - task_id=task_id, - user_id="user-1", - thread_id=thread_id, - context_mode="reuse_thread", - assistant_id="lead_agent", - title=task_id, - prompt="p", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - - listed = await repo.list_by_user_and_thread("user-1", "thread-1") - assert sorted(t["id"] for t in listed) == ["task-a", "task-c"] - assert await repo.list_by_user_and_thread("user-2", "thread-1") == [] - - await close_engine() diff --git a/backend/tests/test_scheduled_task_router.py b/backend/tests/test_scheduled_task_router.py deleted file mode 100644 index 4c28161c1..000000000 --- a/backend/tests/test_scheduled_task_router.py +++ /dev/null @@ -1,38 +0,0 @@ -from fastapi import FastAPI -from fastapi.testclient import TestClient - -from app.gateway.routers import scheduled_tasks - - -def test_router_registers_list_endpoint(): - app = FastAPI() - app.include_router(scheduled_tasks.router) - client = TestClient(app) - response = client.get("/api/scheduled-tasks") - assert response.status_code != 404 - - -def test_router_registers_trigger_route(): - app = FastAPI() - app.include_router(scheduled_tasks.router) - client = TestClient(app) - response = client.post("/api/scheduled-tasks/task-1/trigger") - assert response.status_code != 404 - - -def test_router_registers_create_route(): - app = FastAPI() - app.include_router(scheduled_tasks.router) - client = TestClient(app) - response = client.post( - "/api/scheduled-tasks", - json={ - "thread_id": "thread-1", - "title": "Daily summary", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - }, - ) - assert response.status_code != 404 diff --git a/backend/tests/test_scheduled_task_router_behavior.py b/backend/tests/test_scheduled_task_router_behavior.py deleted file mode 100644 index 70e516752..000000000 --- a/backend/tests/test_scheduled_task_router_behavior.py +++ /dev/null @@ -1,652 +0,0 @@ -from datetime import UTC, datetime, timedelta -from types import SimpleNamespace -from unittest.mock import AsyncMock - -import pytest - -from app.gateway.routers import scheduled_tasks - - -class _Repo: - def __init__(self) -> None: - self.created = [] - self.items = {} - - async def list_by_user(self, user_id: str): - return [item for item in self.items.values() if item["user_id"] == user_id] - - async def list_by_user_and_thread(self, user_id: str, thread_id: str): - return [item for item in self.items.values() if item["user_id"] == user_id and item["thread_id"] == thread_id] - - async def create(self, **kwargs): - item = { - "id": kwargs["task_id"], - "user_id": kwargs["user_id"], - "thread_id": kwargs["thread_id"], - "context_mode": kwargs["context_mode"], - "title": kwargs["title"], - "prompt": kwargs["prompt"], - "schedule_type": kwargs["schedule_type"], - "schedule_spec": kwargs["schedule_spec"], - "timezone": kwargs["timezone"], - "status": "enabled", - "next_run_at": kwargs["next_run_at"], - } - self.items[item["id"]] = item - self.created.append(item) - return item - - async def get(self, task_id: str, *, user_id: str): - item = self.items.get(task_id) - if item is None or item["user_id"] != user_id: - return None - return item - - async def update(self, task_id: str, *, user_id: str, updates): - item = await self.get(task_id, user_id=user_id) - if item is None: - return None - item.update(updates) - return item - - async def delete(self, task_id: str, *, user_id: str): - item = await self.get(task_id, user_id=user_id) - if item is None: - return False - self.items.pop(task_id, None) - return True - - async def list_by_task(self, task_id: str): - return [] - - -class _Service: - def __init__(self) -> None: - self.calls = [] - self.result = {"outcome": "launched"} - - async def dispatch_task(self, task, *, now, trigger): - self.calls.append((task, now, trigger)) - return self.result - - -class _RunStore: - def __init__(self, runs): - self.runs = runs - - async def get(self, run_id: str, *, user_id: str): - run = self.runs.get(run_id) - if run is None or run.get("user_id") != user_id: - return None - return run - - -class _Config: - def __init__(self, min_once_delay_seconds: int = 60) -> None: - self.scheduler = SimpleNamespace(min_once_delay_seconds=min_once_delay_seconds) - - -@pytest.mark.asyncio -async def test_create_scheduled_task_uses_repo(): - repo = _Repo() - request = SimpleNamespace() - body = scheduled_tasks.ScheduledTaskCreateRequest( - thread_id="thread-1", - title="Daily summary", - prompt="Summarize thread", - schedule_type="once", - schedule_spec={"run_at": "2027-01-01T01:00:00+00:00"}, - timezone="UTC", - ) - - user = SimpleNamespace(id="user-1") - thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) - config = _Config() - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_thread_store = scheduled_tasks.get_thread_store - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_thread_store = lambda _request: thread_store - scheduled_tasks.get_config = lambda: config - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - created = await scheduled_tasks.create_scheduled_task.__wrapped__( - request=request, - body=body, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_thread_store = old_thread_store - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert created["title"] == "Daily summary" - assert created["user_id"] == "user-1" - assert created["next_run_at"] == datetime(2027, 1, 1, 1, 0, tzinfo=UTC) - - -@pytest.mark.asyncio -async def test_create_fresh_thread_task_does_not_require_thread_id(): - repo = _Repo() - request = SimpleNamespace() - body = scheduled_tasks.ScheduledTaskCreateRequest( - context_mode="fresh_thread_per_run", - thread_id=None, - title="Fresh task", - prompt="Run in fresh thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - ) - - user = SimpleNamespace(id="user-1") - thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) - config = _Config() - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_thread_store = scheduled_tasks.get_thread_store - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_thread_store = lambda _request: thread_store - scheduled_tasks.get_config = lambda: config - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - created = await scheduled_tasks.create_scheduled_task.__wrapped__( - request=request, - body=body, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_thread_store = old_thread_store - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert created["context_mode"] == "fresh_thread_per_run" - assert created["thread_id"] is None - - -@pytest.mark.asyncio -async def test_trigger_scheduled_task_dispatches_manual_run(): - repo = _Repo() - service = _Service() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_service = scheduled_tasks.get_scheduled_task_service - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_scheduled_task_service = lambda _request: service - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.trigger_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_scheduled_task_service = old_service - scheduled_tasks.get_optional_user_from_request = old_user - - assert result == {"id": "task-1", "triggered": True} - assert len(service.calls) == 1 - assert service.calls[0][2] == "manual" - - -@pytest.mark.asyncio -async def test_trigger_scheduled_task_returns_conflict_when_dispatch_conflicts(): - repo = _Repo() - service = _Service() - service.result = {"outcome": "conflict", "error": "Thread thread-1 already has an active run"} - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_service = scheduled_tasks.get_scheduled_task_service - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_scheduled_task_service = lambda _request: service - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - with pytest.raises(Exception) as exc_info: - await scheduled_tasks.trigger_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_scheduled_task_service = old_service - scheduled_tasks.get_optional_user_from_request = old_user - - assert "already has an active run" in str(exc_info.value) - - -@pytest.mark.asyncio -async def test_update_scheduled_task_writes_repo(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - config = _Config() - thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_thread_store = scheduled_tasks.get_thread_store - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_thread_store = lambda _request: thread_store - scheduled_tasks.get_config = lambda: config - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.update_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - body=scheduled_tasks.ScheduledTaskUpdateRequest(title="Updated title"), - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_thread_store = old_thread_store - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert result["title"] == "Updated title" - - -@pytest.mark.asyncio -async def test_delete_scheduled_task_deletes_repo_row(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.delete_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_optional_user_from_request = old_user - - assert result == {"id": "task-1", "deleted": True} - assert repo.items == {} - - -@pytest.mark.asyncio -async def test_pause_and_resume_scheduled_task_update_status(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - paused = await scheduled_tasks.pause_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - paused_status = paused["status"] - resumed = await scheduled_tasks.resume_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_optional_user_from_request = old_user - - assert paused_status == "paused" - assert resumed["status"] == "enabled" - - -@pytest.mark.asyncio -async def test_pause_rejects_running_task(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - task["status"] = "running" - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - with pytest.raises(Exception) as exc_info: - await scheduled_tasks.pause_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_optional_user_from_request = old_user - - assert "currently running" in str(exc_info.value) - - -@pytest.mark.asyncio -async def test_update_rejects_running_task(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Daily summary", - prompt="Summarize thread", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - task["status"] = "running" - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - config = _Config() - thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_thread_store = scheduled_tasks.get_thread_store - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_thread_store = lambda _request: thread_store - scheduled_tasks.get_config = lambda: config - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - with pytest.raises(Exception) as exc_info: - await scheduled_tasks.update_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - body=scheduled_tasks.ScheduledTaskUpdateRequest(title="Updated title"), - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_thread_store = old_thread_store - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert "currently running" in str(exc_info.value) - - -@pytest.mark.asyncio -async def test_list_thread_scheduled_tasks_filters_by_thread_id(): - repo = _Repo() - await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Thread one task", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - await repo.create( - task_id="task-2", - user_id="user-1", - thread_id="thread-2", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Thread two task", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.list_thread_scheduled_tasks.__wrapped__( - thread_id="thread-1", - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_optional_user_from_request = old_user - - assert [task["id"] for task in result] == ["task-1"] - - -@pytest.mark.asyncio -async def test_list_scheduled_task_runs_returns_persisted_rows_without_side_effects(): - repo = _Repo() - task = await repo.create( - task_id="task-1", - user_id="user-1", - thread_id="thread-1", - context_mode="reuse_thread", - assistant_id="lead_agent", - title="Task", - prompt="Prompt", - schedule_type="cron", - schedule_spec={"cron": "0 9 * * *"}, - timezone="UTC", - next_run_at=None, - ) - run_repo = SimpleNamespace( - list_by_task=AsyncMock( - return_value=[ - { - "id": "task-run-1", - "task_id": "task-1", - "thread_id": "thread-1", - "run_id": "run-1", - "status": "running", - "error": None, - } - ] - ), - ) - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_task_repo = scheduled_tasks.get_scheduled_task_repo - old_run_repo = scheduled_tasks.get_scheduled_task_run_repo - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_scheduled_task_run_repo = lambda _request: run_repo - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.list_scheduled_task_runs.__wrapped__( - task_id=task["id"], - request=request, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_task_repo - scheduled_tasks.get_scheduled_task_run_repo = old_run_repo - scheduled_tasks.get_optional_user_from_request = old_user - - assert result[0]["status"] == "running" - - -@pytest.mark.asyncio -async def test_create_once_task_enforces_minimum_delay(): - repo = _Repo() - request = SimpleNamespace() - body = scheduled_tasks.ScheduledTaskCreateRequest( - thread_id="thread-1", - title="Soon task", - prompt="Run soon", - schedule_type="once", - schedule_spec={"run_at": (datetime.now(UTC) + timedelta(seconds=30)).isoformat()}, - timezone="UTC", - ) - user = SimpleNamespace(id="user-1") - thread_store = SimpleNamespace(check_access=AsyncMock(return_value=True)) - config = _Config(min_once_delay_seconds=60) - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_thread_store = scheduled_tasks.get_thread_store - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_thread_store = lambda _request: thread_store - scheduled_tasks.get_config = lambda: config - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - with pytest.raises(Exception) as exc_info: - await scheduled_tasks.create_scheduled_task.__wrapped__( - request=request, - body=body, - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_thread_store = old_thread_store - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert "once schedule must be at least" in str(exc_info.value) - - -@pytest.mark.asyncio -async def test_update_terminal_once_task_with_future_run_at_rearms_it(): - """PATCHing a fresh future run_at onto a completed/failed/cancelled once - task must reset status to enabled — claim_due_tasks only admits enabled - rows, so keeping the terminal status returns a next_run_at that never fires.""" - repo = _Repo() - task = await repo.create( - task_id="task-terminal", - user_id="user-1", - thread_id=None, - context_mode="fresh_thread_per_run", - assistant_id="lead_agent", - title="Once done", - prompt="p", - schedule_type="once", - schedule_spec={"run_at": "2026-07-01T00:00:00+00:00"}, - timezone="UTC", - next_run_at=None, - ) - task["status"] = "completed" - future_run_at = (datetime.now(UTC) + timedelta(hours=1)).isoformat() - request = SimpleNamespace() - user = SimpleNamespace(id="user-1") - - old_repo = scheduled_tasks.get_scheduled_task_repo - old_config = scheduled_tasks.get_config - old_user = scheduled_tasks.get_optional_user_from_request - try: - scheduled_tasks.get_scheduled_task_repo = lambda _request: repo - scheduled_tasks.get_config = lambda: _Config() - scheduled_tasks.get_optional_user_from_request = AsyncMock(return_value=user) - - result = await scheduled_tasks.update_scheduled_task.__wrapped__( - task_id=task["id"], - request=request, - body=scheduled_tasks.ScheduledTaskUpdateRequest(schedule_spec={"run_at": future_run_at}), - ) - finally: - scheduled_tasks.get_scheduled_task_repo = old_repo - scheduled_tasks.get_config = old_config - scheduled_tasks.get_optional_user_from_request = old_user - - assert result["status"] == "enabled" - assert result["next_run_at"] is not None diff --git a/backend/tests/test_scheduled_task_schedules.py b/backend/tests/test_scheduled_task_schedules.py deleted file mode 100644 index fcaac8676..000000000 --- a/backend/tests/test_scheduled_task_schedules.py +++ /dev/null @@ -1,49 +0,0 @@ -from datetime import UTC, datetime - -import pytest - -from deerflow.scheduler.schedules import ( - next_run_at, - normalize_cron_expression, - validate_timezone, -) - - -def test_validate_timezone_accepts_iana_name(): - assert validate_timezone("Asia/Shanghai") == "Asia/Shanghai" - - -def test_validate_timezone_rejects_unknown_name(): - with pytest.raises(ValueError): - validate_timezone("Mars/Base") - - -def test_normalize_cron_accepts_five_fields(): - assert normalize_cron_expression("0 9 * * 1") == "0 9 * * 1" - - -def test_normalize_cron_rejects_seconds_field(): - with pytest.raises(ValueError): - normalize_cron_expression("0 0 9 * * 1") - - -def test_next_run_at_for_once_returns_none_after_fire_time(): - now = datetime(2026, 7, 2, 2, 0, tzinfo=UTC) - result = next_run_at( - "once", - {"run_at": "2026-07-02T01:00:00+00:00"}, - "UTC", - now=now, - ) - assert result is None - - -def test_next_run_at_for_cron_uses_timezone(): - now = datetime(2026, 7, 1, 0, 30, tzinfo=UTC) - result = next_run_at( - "cron", - {"cron": "0 9 * * *"}, - "Asia/Shanghai", - now=now, - ) - assert result == datetime(2026, 7, 1, 1, 0, tzinfo=UTC) diff --git a/backend/tests/test_scheduled_task_service.py b/backend/tests/test_scheduled_task_service.py deleted file mode 100644 index a071ad298..000000000 --- a/backend/tests/test_scheduled_task_service.py +++ /dev/null @@ -1,795 +0,0 @@ -from datetime import UTC, datetime, timedelta - -import pytest - -from app.scheduler.service import ScheduledTaskService -from deerflow.runtime import ConflictError, RunStatus -from deerflow.runtime.runs.manager import RunRecord -from deerflow.runtime.runs.schemas import DisconnectMode - - -class DummyTaskRepo: - def __init__(self, rows): - self.rows = rows - self.claimed = False - self.updated = None - self.cancelled_stuck_once = None - - async def cancel_stuck_once_tasks(self, *, error): - self.cancelled_stuck_once = error - return 0 - - async def claim_due_tasks(self, **_kwargs): - if self.claimed: - return [] - self.claimed = True - return self.rows - - async def update_after_launch(self, *args, **kwargs): - self.updated = (args, kwargs) - - async def get(self, task_id: str, *, user_id: str): - row = next((item for item in self.rows if item["id"] == task_id and item["user_id"] == user_id), None) - return dict(row) if row is not None else None - - async def update(self, task_id: str, *, user_id: str, updates): - row = next((item for item in self.rows if item["id"] == task_id and item["user_id"] == user_id), None) - if row is None: - return None - row.update(updates) - return dict(row) - - -class DummyRunRepo: - def __init__(self, *, active=False, active_count=0): - self.created = None - self.updated = [] - self.active = active - self.active_count = active_count - self.stale_marked = None - - async def count_active_runs(self): - return self.active_count - - async def create(self, **kwargs): - self.created = kwargs - return {"id": kwargs["run_record_id"]} - - async def update_status(self, run_record_id, **kwargs): - self.updated.append((run_record_id, kwargs)) - - async def has_active_runs(self, task_id): - return self.active - - async def mark_stale_active_runs(self, *, error): - self.stale_marked = error - return 0 - - -@pytest.mark.asyncio -async def test_service_claims_and_dispatches_due_task(): - async def fake_launch(**kwargs): - assert kwargs["owner_user_id"] == "user-1" - assert kwargs["metadata"]["scheduled_task_id"] == "task-1" - assert kwargs["metadata"]["scheduled_trigger"] == "scheduled" - return {"run_id": "run-1", "thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo( - [ - { - "id": "task-1", - "user_id": "user-1", - "thread_id": "thread-1", - "context_mode": "reuse_thread", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "once", - "schedule_spec": {"run_at": "2026-07-02T01:00:00+00:00"}, - "timezone": "UTC", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - await service.run_once(now=datetime.now(UTC) + timedelta(days=1)) - - assert run_repo.created["task_id"] == "task-1" - assert run_repo.updated[0][1]["status"] == "running" - assert run_repo.updated[0][1]["protect_terminal"] is True - # `once` terminal status is owned by handle_run_completion, not the launch. - assert task_repo.updated[1]["status"] == "running" - - -@pytest.mark.asyncio -async def test_manual_trigger_keeps_paused_cron_task_paused(): - async def fake_launch(**kwargs): - return {"run_id": "run-2", "thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo( - [ - { - "id": "task-2", - "user_id": "user-1", - "thread_id": "thread-1", - "context_mode": "reuse_thread", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - "status": "paused", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - await service.dispatch_task( - task_repo.rows[0], - now=datetime.now(UTC), - trigger="manual", - ) - - assert task_repo.updated[1]["status"] == "paused" - - -@pytest.mark.asyncio -async def test_fresh_thread_per_run_creates_new_execution_thread(): - async def fake_launch(**kwargs): - assert kwargs["thread_id"] != "thread-template" - return {"run_id": "run-3", "thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo( - [ - { - "id": "task-3", - "user_id": "user-1", - "thread_id": "thread-template", - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - "status": "enabled", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - await service.dispatch_task( - task_repo.rows[0], - now=datetime.now(UTC), - trigger="scheduled", - ) - - assert run_repo.created["thread_id"] != "thread-template" - assert task_repo.updated[1]["last_thread_id"] == run_repo.created["thread_id"] - - -@pytest.mark.asyncio -async def test_scheduled_overlap_conflict_is_recorded_as_skip(): - async def fake_launch(**_kwargs): - raise ConflictError("Thread thread-1 already has an active run") - - task_repo = DummyTaskRepo( - [ - { - "id": "task-4", - "user_id": "user-1", - "thread_id": "thread-1", - "context_mode": "reuse_thread", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - "status": "running", - "overlap_policy": "skip", - "last_run_id": "run-old", - "last_thread_id": "thread-1", - "last_run_at": "2026-07-01T00:00:00+00:00", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - result = await service.dispatch_task( - task_repo.rows[0], - now=datetime.now(UTC), - trigger="scheduled", - ) - - assert result["outcome"] == "skipped" - assert run_repo.updated[-1][1]["status"] == "skipped" - assert task_repo.updated[1]["status"] == "enabled" - - -@pytest.mark.asyncio -async def test_manual_overlap_conflict_returns_conflict(): - async def fake_launch(**_kwargs): - raise ConflictError("Thread thread-1 already has an active run") - - task_repo = DummyTaskRepo( - [ - { - "id": "task-5", - "user_id": "user-1", - "thread_id": "thread-1", - "context_mode": "reuse_thread", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - "status": "enabled", - "overlap_policy": "skip", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - result = await service.dispatch_task( - task_repo.rows[0], - now=datetime.now(UTC), - trigger="manual", - ) - - assert result["outcome"] == "conflict" - assert run_repo.updated[-1][1]["status"] == "failed" - - -@pytest.mark.asyncio -async def test_handle_run_completion_persists_success(): - task_repo = DummyTaskRepo( - [ - { - "id": "task-6", - "user_id": "user-1", - "thread_id": None, - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "cron", - "schedule_spec": {"cron": "0 9 * * *"}, - "timezone": "UTC", - "status": "enabled", - } - ] - ) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=lambda **_kwargs: None, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - record = RunRecord( - run_id="run-6", - thread_id="thread-6", - assistant_id="lead_agent", - status=RunStatus.success, - on_disconnect=DisconnectMode.continue_, - metadata={ - "scheduled_task_id": "task-6", - "scheduled_task_run_id": "task-run-6", - }, - user_id="user-1", - ) - - await service.handle_run_completion(record) - - assert run_repo.updated[-1][0] == "task-run-6" - assert run_repo.updated[-1][1]["status"] == "success" - assert task_repo.rows[0]["last_error"] is None - - -def _make_service(task_repo, run_repo): - return ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=lambda **_kwargs: None, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - -def _once_task_row(task_id="task-once", status="running"): - return { - "id": task_id, - "user_id": "user-1", - "thread_id": None, - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "Summarize thread", - "schedule_type": "once", - "schedule_spec": {"run_at": "2026-07-02T01:00:00+00:00"}, - "timezone": "UTC", - "status": status, - } - - -def _completion_record(status, *, task_id="task-once", error=None): - return RunRecord( - run_id="run-x", - thread_id="thread-x", - assistant_id="lead_agent", - status=status, - on_disconnect=DisconnectMode.continue_, - metadata={ - "scheduled_task_id": task_id, - "scheduled_task_run_id": "task-run-x", - }, - user_id="user-1", - error=error, - ) - - -@pytest.mark.asyncio -async def test_once_task_completes_only_via_completion_hook(): - task_repo = DummyTaskRepo([_once_task_row()]) - run_repo = DummyRunRepo() - service = _make_service(task_repo, run_repo) - - await service.handle_run_completion(_completion_record(RunStatus.success)) - - assert run_repo.updated[-1][1]["status"] == "success" - assert task_repo.rows[0]["status"] == "completed" - - -@pytest.mark.asyncio -async def test_once_task_failed_run_marks_task_failed(): - task_repo = DummyTaskRepo([_once_task_row()]) - run_repo = DummyRunRepo() - service = _make_service(task_repo, run_repo) - - await service.handle_run_completion(_completion_record(RunStatus.error, error="boom")) - - assert run_repo.updated[-1][1]["status"] == "failed" - assert run_repo.updated[-1][1]["error"] == "boom" - assert task_repo.rows[0]["status"] == "failed" - assert task_repo.rows[0]["last_error"] == "boom" - - -@pytest.mark.asyncio -async def test_interrupted_run_is_distinct_and_cancels_once_task(): - task_repo = DummyTaskRepo([_once_task_row()]) - run_repo = DummyRunRepo() - service = _make_service(task_repo, run_repo) - - await service.handle_run_completion(_completion_record(RunStatus.interrupted)) - - run_update = run_repo.updated[-1][1] - assert run_update["status"] == "interrupted" - assert run_update["error"] == "run was interrupted before completion" - assert task_repo.rows[0]["status"] == "cancelled" - - -@pytest.mark.asyncio -async def test_interrupted_cron_run_keeps_task_enabled(): - row = _once_task_row(task_id="task-cron") - row.update({"schedule_type": "cron", "schedule_spec": {"cron": "0 9 * * *"}, "status": "enabled"}) - task_repo = DummyTaskRepo([row]) - run_repo = DummyRunRepo() - service = _make_service(task_repo, run_repo) - - await service.handle_run_completion(_completion_record(RunStatus.interrupted, task_id="task-cron")) - - assert run_repo.updated[-1][1]["status"] == "interrupted" - assert task_repo.rows[0]["status"] == "enabled" - - -@pytest.mark.asyncio -async def test_skip_policy_applies_to_fresh_thread_runs(): - launched = [] - - async def fake_launch(**kwargs): - launched.append(kwargs) - return {"run_id": "run-9", "thread_id": kwargs["thread_id"]} - - row = _once_task_row(task_id="task-9") - row.update({"schedule_type": "cron", "schedule_spec": {"cron": "* * * * *"}, "status": "running", "overlap_policy": "skip"}) - task_repo = DummyTaskRepo([row]) - run_repo = DummyRunRepo(active=True) - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - result = await service.dispatch_task(row, now=datetime.now(UTC), trigger="scheduled") - - assert result["outcome"] == "skipped" - assert launched == [] - # The skip tombstone is created directly as terminal "skipped" (not the - # transient "queued" the launch path uses): a queued row is active and would - # itself trip the uq_scheduled_task_run_active partial unique index against - # the pre-existing run still holding the task's single active slot. - assert run_repo.created["status"] == "skipped" - assert run_repo.updated[-1][1]["status"] == "skipped" - assert task_repo.updated[1]["status"] == "enabled" - assert task_repo.updated[1]["increment_run_count"] is False - - -@pytest.mark.asyncio -async def test_startup_sweep_reconciles_stale_runs_and_stuck_once_tasks(): - task_repo = DummyTaskRepo([]) - run_repo = DummyRunRepo() - service = _make_service(task_repo, run_repo) - - await service.start() - await service.stop() - - assert run_repo.stale_marked is not None - assert task_repo.cancelled_stuck_once == run_repo.stale_marked - - -@pytest.mark.asyncio -async def test_manual_trigger_with_active_run_returns_conflict_without_launching(): - launched = [] - - async def fake_launch(**kwargs): - launched.append(kwargs) - return {"run_id": "run-x", "thread_id": kwargs["thread_id"]} - - row = _once_task_row(task_id="task-manual-busy") - row.update({"schedule_type": "cron", "schedule_spec": {"cron": "* * * * *"}, "status": "enabled", "overlap_policy": "skip"}) - task_repo = DummyTaskRepo([row]) - run_repo = DummyRunRepo(active=True) - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - result = await service.dispatch_task(row, now=datetime.now(UTC), trigger="manual") - - assert result["outcome"] == "conflict" - assert launched == [] - # Nothing was scheduled to happen, so no run-history row is recorded. - assert run_repo.created is None - assert result["task_run_id"] is None - - -@pytest.mark.asyncio -async def test_run_once_claims_only_into_remaining_global_budget(): - claim_limits = [] - - class BudgetTaskRepo(DummyTaskRepo): - async def claim_due_tasks(self, **kwargs): - claim_limits.append(kwargs["limit"]) - return [] - - task_repo = BudgetTaskRepo([]) - run_repo = DummyRunRepo(active_count=2) - service = _make_service(task_repo, run_repo) - - await service.run_once(now=datetime.now(UTC)) - assert claim_limits == [1] - - run_repo.active_count = 3 - await service.run_once(now=datetime.now(UTC)) - # Budget exhausted: no claim at all this cycle. - assert claim_limits == [1] - - -@pytest.mark.asyncio -async def test_launch_bookkeeping_passes_protect_terminal(): - async def fake_launch(**kwargs): - return {"run_id": "run-pt", "thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo([_once_task_row(task_id="task-pt", status="enabled")]) - run_repo = DummyRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - await service.dispatch_task(task_repo.rows[0], now=datetime.now(UTC), trigger="scheduled") - - assert task_repo.updated[1]["protect_terminal"] is True - - -class _StatefulRunRepo: - """Stateful fake ``ScheduledTaskRunRepository`` for the #4452 tests. - - Mirrors just enough of the real repository to let a second dispatch - observe the active slot held by the first: - - * ``create()`` tracks each row by id, carrying its ``status`` and - ``run_id``; - * with ``fail_first_update=True`` the very FIRST ``update_status()`` - call raises, simulating a transient DB failure on the - ``queued -> running`` write that fires right after ``_launch_run`` - returns a live ``run_id``; every later ``update_status()`` applies; - * ``has_active_runs()`` reflects whether any tracked row for the task - is still in an active status (``queued``/``running``), exactly like - the partial unique index ``uq_scheduled_task_run_active``. - """ - - _ACTIVE = {"queued", "running"} - - def __init__(self, *, fail_first_update: bool = False) -> None: - self.created: list[dict] = [] - self.updates: list[tuple[str, dict]] = [] - self.rows: dict[str, dict] = {} - self._fail_first_update = fail_first_update - self._first_update_raised = False - - async def count_active_runs(self) -> int: - return sum(1 for row in self.rows.values() if row["status"] in self._ACTIVE) - - async def create(self, **kwargs) -> dict: - self.created.append(kwargs) - self.rows[kwargs["run_record_id"]] = { - "task_id": kwargs["task_id"], - "status": kwargs["status"], - "run_id": None, - } - return {"id": kwargs["run_record_id"]} - - async def update_status(self, run_record_id: str, **kwargs) -> None: - self.updates.append((run_record_id, kwargs)) - if self._fail_first_update and not self._first_update_raised: - # The launch-path queued->running write fails once, AFTER - # _launch_run has already returned a live run_id. - self._first_update_raised = True - raise RuntimeError("simulated transient DB error on queued->running write") - row = self.rows.get(run_record_id) - if row is None: - return - if "status" in kwargs: - row["status"] = kwargs["status"] - if kwargs.get("run_id") is not None: - row["run_id"] = kwargs["run_id"] - - async def has_active_runs(self, task_id: str) -> bool: - return any(row["task_id"] == task_id and row["status"] in self._ACTIVE for row in self.rows.values()) - - async def mark_stale_active_runs(self, *, error: str) -> int: - return 0 - - -@pytest.mark.asyncio -async def test_post_launch_bookkeeping_failure_does_not_release_active_slot(): - """Regression for issue #4452. - - A transient failure in the ``queued -> running`` bookkeeping write - (after ``_launch_run`` has already returned a live ``run_id``) must NOT - flip the task-run row to ``failed``: ``failed`` is outside the partial - unique index ``uq_scheduled_task_run_active``, so releasing the slot - would let the next dispatch launch a DUPLICATE run. The fix keeps the - row ``running`` with the launched ``run_id`` retained for recovery, - reconciliation, and cancellation. - """ - launched: list[dict] = [] - - async def fake_launch(**kwargs): - launched.append(kwargs) - return {"run_id": f"run-{len(launched)}", "thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo( - [ - { - "id": "task-4452", - "user_id": "user-1", - "thread_id": None, - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "do the thing", - "schedule_type": "cron", - "schedule_spec": {"cron": "*/5 * * * *"}, - "timezone": "UTC", - "status": "enabled", - "overlap_policy": "skip", - } - ] - ) - run_repo = _StatefulRunRepo(fail_first_update=True) - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - now = datetime.now(UTC) - task = dict(task_repo.rows[0]) - - first = await service.dispatch_task(task, now=now, trigger="scheduled") - # The run launched despite the post-launch bookkeeping error; the - # outcome and run_id reflect that a live run is in flight. - assert first["outcome"] == "launched" - assert first["run_id"] == "run-1" - assert first["error"] is not None # the bookkeeping error is surfaced, not hidden - - # Second dispatch must observe the active slot held by run-1 and NOT - # launch a duplicate. On main (bug) this would launch run-2 here. - second = await service.dispatch_task(task, now=now, trigger="scheduled") - assert len(launched) == 1, launched - assert second["outcome"] in {"skipped", "conflict"}, second - - # The launched run_id is retained on the task-run row (status "running", - # not "failed") so reconciliation / cancellation can still reach it. - first_row_id = run_repo.created[0]["run_record_id"] - assert run_repo.rows[first_row_id]["status"] == "running" - assert run_repo.rows[first_row_id]["run_id"] == "run-1" - - # The bookkeeping transient is NOT surfaced as the parent task's - # last_error: the run launched and is still in flight, so the task list - # must not show an error on an actively running task (matching the - # success path's clear-on-launch model). The real terminal outcome is - # written by handle_run_completion. - assert task_repo.updated[1]["last_error"] is None - - -@pytest.mark.asyncio -async def test_pre_launch_failure_still_releases_active_slot(): - """Complement to the #4452 fix: when ``_launch_run`` itself fails (no run - was ever started), the task-run row is marked ``failed`` and the active - slot is released as before -- the post-launch retention path does not - apply because there is no live run to protect. - """ - launched: list[dict] = [] - - async def fake_launch(**kwargs): - launched.append(kwargs) - raise RuntimeError("runtime refused to start the run") - - task_repo = DummyTaskRepo( - [ - { - "id": "task-4452-pre", - "user_id": "user-1", - "thread_id": None, - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "do the thing", - "schedule_type": "cron", - "schedule_spec": {"cron": "*/5 * * * *"}, - "timezone": "UTC", - "status": "enabled", - "overlap_policy": "skip", - } - ] - ) - run_repo = _StatefulRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - result = await service.dispatch_task(dict(task_repo.rows[0]), now=datetime.now(UTC), trigger="scheduled") - - assert result["outcome"] == "failed" - assert result["run_id"] is None - # launch was attempted (and raised), so exactly one launch attempt, and - # the row is terminal -> the slot is released for the next dispatch. - assert len(launched) == 1 - first_row_id = run_repo.created[0]["run_record_id"] - assert run_repo.rows[first_row_id]["status"] == "failed" - assert run_repo.rows[first_row_id]["run_id"] is None - - -@pytest.mark.asyncio -async def test_malformed_launch_result_still_retains_active_slot(): - """Defense-in-depth for the #4452 invariant. - - If ``_launch_run`` returns a malformed result (e.g. missing ``run_id``), - the unpacking line raises AFTER a live run was already created. The - dispatch must still take the retention path (keep the row active so the - slot stays held and no duplicate launches) rather than the pre-launch - generic-failure path, which would mark the row ``failed`` and release - the slot while a run is in flight. Keyed off ``launch_succeeded``, not - ``launched_run_id is not None``. - """ - launched: list[dict] = [] - - async def fake_launch(**kwargs): - launched.append(kwargs) - # Live run started, but the result payload is malformed. - return {"thread_id": kwargs["thread_id"]} - - task_repo = DummyTaskRepo( - [ - { - "id": "task-4452-malformed", - "user_id": "user-1", - "thread_id": None, - "context_mode": "fresh_thread_per_run", - "assistant_id": "lead_agent", - "prompt": "do the thing", - "schedule_type": "cron", - "schedule_spec": {"cron": "*/5 * * * *"}, - "timezone": "UTC", - "status": "enabled", - "overlap_policy": "skip", - } - ] - ) - run_repo = _StatefulRunRepo() - service = ScheduledTaskService( - task_repo=task_repo, - task_run_repo=run_repo, - launch_run=fake_launch, - poll_interval_seconds=5, - lease_seconds=120, - max_concurrent_runs=3, - ) - - now = datetime.now(UTC) - task = dict(task_repo.rows[0]) - - first = await service.dispatch_task(task, now=now, trigger="scheduled") - # Launch succeeded, so the outcome is "launched" (a run is in flight) - # even though the result unpacking raised; run_id is unknown. - assert first["outcome"] == "launched" - assert first["run_id"] is None - - # Second dispatch must observe the active slot still held (row stays in - # an active status, NOT "failed") and NOT launch a duplicate. - second = await service.dispatch_task(task, now=now, trigger="scheduled") - assert len(launched) == 1, launched - assert second["outcome"] in {"skipped", "conflict"}, second - - first_row_id = run_repo.created[0]["run_record_id"] - assert run_repo.rows[first_row_id]["status"] == "running"