From 69ab8e125b258ebd3f91fde4223c5135765c102c Mon Sep 17 00:00:00 2001 From: Vu Dam Tuan Date: Thu, 27 Aug 2026 17:27:53 +0900 Subject: [PATCH] feat(R07): task repository, schedule calculator, Qt clock adapter, task/AI-planner services Team Hoa, EPIC R07 (Scheduling & Workflow Runtime) - Team Hoa scope only (R07-T01 -> T05; R07-T06 Co4EWorkflowService is Team Nam's). - R07-T01: infrastructure/persistence/json/task_repository_impl.py wraps core/tasks.py's CRUD; core/tasks.py::save_task now writes through atomic_write.write_json (same durability fix as R06-T02, save_task was still doing a plain write_text). - R07-T02: domain/tasks/schedule_calculator.py::ScheduleCalculator - the cron/interval/daily/weekly/monthly due-time math extracted from core/tasks.py, pure Python with is_holiday/make_cron injected so domain/ never imports core (ADR-001 I2). core/tasks.py keeps its old function names as thin wrappers so every existing caller is unchanged. This was previously untested; now has its own unit suite. - R07-T03: infrastructure/qt/qt_scheduler_clock.py::QtSchedulerClock wraps the QTimer TaskScheduler used to own directly, injected via a new `clock=` constructor param (defaults to a real one). Originally planned at platform/qt/... ; moved after confirming that name shadows the stdlib platform module (used by core/windows_sandbox_vm.py, core/appcontainer_sandbox.py) whenever the repo root is on sys.path. tests/fakes/fake_clock.py lets scheduler dispatch be tested tick-by-tick with no Qt event loop. - R07-T04: application/scheduling/task_application_service.py centralizes run_now/duplicate/pause/delete/bulk_delete and the Kanban drag-drop business rules (move_to_status), currently only reachable by driving the real ui/schedule_task_tab.py widget. - R07-T05: application/scheduling/ai_task_planner_service.py wraps core/ai_task_planner.py::plan_tasks and core/task_import.py::import_tasks as a seam, plus the attachment-stamping step that used to only exist inside the AI-create dialog's worker closure. pytest: 328 pass (same 4 pre-existing failures as the R05/R06 baseline, unrelated to this work - see docs/refactor/BaoCao_TeamHoa_R05_R06.md). scripts/check_imports.py: PASS. Co-Authored-By: Claude Sonnet 5 --- application/scheduling/__init__.py | 6 + .../scheduling/ai_task_planner_service.py | 94 +++++++++ .../scheduling/task_application_service.py | 172 ++++++++++++++++ core/task_scheduler.py | 30 ++- core/tasks.py | 100 +++------ docs/refactor/Refactoring_Checklist.md | 22 +- domain/tasks/__init__.py | 5 + domain/tasks/schedule_calculator.py | 165 +++++++++++++++ infrastructure/persistence/json/__init__.py | 5 +- .../persistence/json/task_repository_impl.py | 63 ++++++ infrastructure/qt/__init__.py | 18 ++ infrastructure/qt/qt_scheduler_clock.py | 70 +++++++ tests/fakes/__init__.py | 7 +- tests/fakes/fake_clock.py | 54 +++++ tests/integration/test_qt_scheduler_clock.py | 53 +++++ tests/unit/test_ai_task_planner_service.py | 87 ++++++++ tests/unit/test_schedule_calculator.py | 164 +++++++++++++++ tests/unit/test_task_application_service.py | 191 ++++++++++++++++++ tests/unit/test_task_repository.py | 70 +++++++ .../unit/test_task_scheduler_clock_wiring.py | 66 ++++++ 20 files changed, 1347 insertions(+), 95 deletions(-) create mode 100644 application/scheduling/__init__.py create mode 100644 application/scheduling/ai_task_planner_service.py create mode 100644 application/scheduling/task_application_service.py create mode 100644 domain/tasks/__init__.py create mode 100644 domain/tasks/schedule_calculator.py create mode 100644 infrastructure/persistence/json/task_repository_impl.py create mode 100644 infrastructure/qt/__init__.py create mode 100644 infrastructure/qt/qt_scheduler_clock.py create mode 100644 tests/fakes/fake_clock.py create mode 100644 tests/integration/test_qt_scheduler_clock.py create mode 100644 tests/unit/test_ai_task_planner_service.py create mode 100644 tests/unit/test_schedule_calculator.py create mode 100644 tests/unit/test_task_application_service.py create mode 100644 tests/unit/test_task_repository.py create mode 100644 tests/unit/test_task_scheduler_clock_wiring.py diff --git a/application/scheduling/__init__.py b/application/scheduling/__init__.py new file mode 100644 index 0000000..18013dc --- /dev/null +++ b/application/scheduling/__init__.py @@ -0,0 +1,6 @@ +"""Application services for Schedule Task (EPIC R07).""" + +from .ai_task_planner_service import AiTaskPlannerService +from .task_application_service import MoveResult, RunNowResult, TaskApplicationService + +__all__ = ["TaskApplicationService", "RunNowResult", "MoveResult", "AiTaskPlannerService"] diff --git a/application/scheduling/ai_task_planner_service.py b/application/scheduling/ai_task_planner_service.py new file mode 100644 index 0000000..6fbf15e --- /dev/null +++ b/application/scheduling/ai_task_planner_service.py @@ -0,0 +1,94 @@ +"""AiTaskPlannerService - AI-generate / import task lists, outside the widget +(R07-T05). + +``ui/schedule_task_tab.py``'s ``_AiCreateDialog`` already delegates the +actual planning to two existing pure functions — +``core/ai_task_planner.py::plan_tasks`` (natural-language description -> +task dicts, via the active provider) and +``core/task_import.py::import_tasks`` (Excel/CSV/JSON -> task dicts) — so +this service does not reimplement either. What it DOES own is one small +piece of business logic that currently only exists inside the dialog's +``AgentWorker`` job closure (``_generate``'s ``job()``): every AI-generated +task must carry the SAME file/link attachments the user attached to the +request, so they're available again at run time, not just visible to the +planner while it drafts the task list. Leaving that step trapped in a Qt +worker closure means it can only be exercised by driving the real dialog; +here it's a plain, independently testable method. + +Pure Python: no Qt import. The provider is a constructor-injected factory +(``() -> Provider``, no arguments — matches ``AppContext.build_active_ +provider``), the same dependency-inversion shape +``application/conversations/conversation_application_service.py`` (R04-T03) +uses for ITS provider factory. +""" +from __future__ import annotations + +from pathlib import Path +from typing import Any, Callable, Dict, List, Optional, Sequence, Union + +ProviderFactory = Callable[[], Any] +CancelFn = Callable[[], bool] + + +class AiTaskPlannerService: + """AI task generation + file/Excel/CSV/JSON import, for + ``presentation/scheduling/ai_task_creator_dialog.py`` and + ``ai_task_import_dialog.py`` (R08-T11) to call instead of importing + ``core.ai_task_planner``/``core.task_import`` directly. + + Args: + provider_factory: ``() -> Provider``. Production passes + ``AppContext.build_active_provider``; tests pass a lambda + returning a :class:`FakeProvider`. + """ + + def __init__(self, provider_factory: Optional[ProviderFactory] = None) -> None: + self._provider_factory = provider_factory + + def plan( + self, + description: str, + *, + file_paths: Sequence[str] = (), + links: Sequence[str] = (), + provider: Any = None, + cancel: Optional[CancelFn] = None, + ) -> List[Dict[str, Any]]: + """Turn ``description`` into a list of NOT-yet-saved task dicts. + + ``provider`` overrides the constructor's factory for this one call + (useful for tests, or a caller that already resolved a provider); + omit it to use the injected factory. Raises ``RuntimeError`` when + no provider is available at all, or when the model's reply had no + parseable task list (same error ``core.ai_task_planner.plan_tasks`` + already raises). + """ + resolved = provider if provider is not None else self._resolve_provider() + from cowork_local.core.ai_task_planner import plan_tasks + + planned = plan_tasks(resolved, description, cancel=cancel) + # Attachments apply to EVERY generated task so they're still there + # when the task actually runs, not just while the planner drafts it + # (see module docstring — this used to only happen inside the + # dialog's worker closure). + for task in planned: + task["input"]["file_paths"] = list(file_paths) + task["input"]["links"] = list(links) + return planned + + def import_file(self, path: Union[str, Path]) -> List[Dict[str, Any]]: + """Excel/CSV/JSON -> NOT-yet-saved task dicts, auto-chained in file + order. Raises ``ValueError`` with a human-readable message on an + unusable/unsupported file (same contract + ``core.task_import.import_tasks`` already has).""" + from cowork_local.core.task_import import import_tasks + + return import_tasks(path) + + def _resolve_provider(self) -> Any: + if self._provider_factory is None: + raise RuntimeError("No provider available to plan tasks.") + return self._provider_factory() + + +__all__ = ["AiTaskPlannerService"] diff --git a/application/scheduling/task_application_service.py b/application/scheduling/task_application_service.py new file mode 100644 index 0000000..d96c28d --- /dev/null +++ b/application/scheduling/task_application_service.py @@ -0,0 +1,172 @@ +"""TaskApplicationService - task CRUD + dispatch, outside the widget (R07-T04). + +``ui/schedule_task_tab.py`` currently does all of this by importing +``core/tasks.py`` module functions directly and calling +``self.scheduler.run_now(...)`` inline inside Qt slot methods +(``_run_now``, ``_context_menu``'s duplicate/pause/delete branches, +``_on_task_dropped``'s per-lane business rules). None of it is Qt — it's +plain CRUD plus a few small rules ("a manual task never auto-runs", +"dropping a card on Done disables its schedule so it won't re-fire", +"dropping on Scheduled with no time set needs the editor, not a silent +no-op") — but it can only be exercised today by driving the real widget. + +This service is the seam ``presentation/scheduling/kanban_board_widget.py`` +(R08-T11) calls instead: same rules, same +:class:`~infrastructure.persistence.json.task_repository_impl.TaskRepository` +underneath, testable with no Qt at all. + +Pure Python: no Qt import. ``run_now`` dispatch is a plain injected callable +(production wires ``TaskScheduler.run_now``; tests inject a stub), the same +constructor-injection shape ``application/conversations/conversation_ +application_service.py`` (R04-T03) uses for its provider factory. +""" +from __future__ import annotations + +from dataclasses import dataclass +from typing import Any, Callable, Dict, List, Optional + +# "TaskRepository" here is a Protocol-shaped name, not an import: this module +# only calls .get/.save/.delete/.duplicate, so any object with that shape +# (the real infrastructure.persistence.json.task_repository_impl.TaskRepository, +# or a test double) works without this file importing infrastructure/ at +# module scope. +RunNowFn = Callable[[str], bool] + + +@dataclass +class RunNowResult: + """Outcome of asking a task to run immediately. + + ``reason`` is one of ``""`` (ok), ``"not_found"``, ``"manual_task"`` + (manual tasks never auto-run — spec: they exist to be run by a human), + ``"no_scheduler"`` (no ``run_now`` callable was wired in), or + ``"already_running"`` (the scheduler's own dedupe rejected it). + """ + + ok: bool + reason: str = "" + + +@dataclass +class MoveResult: + """Outcome of dropping a task card onto a Kanban lane + (``move_to_status``). The caller (kanban widget) uses the flags to decide + what to show — a full re-render, a "task is running" toast, or opening + the task editor — without re-deriving the business rule itself.""" + + task: Optional[Dict[str, Any]] + blocked: bool = False # dropped while already running — ignored + ran_now: bool = False # dropped on the Running lane — dispatched + run_now_result: Optional[RunNowResult] = None + needs_schedule: bool = False # dropped on Scheduled with no run_at set — needs editing + + +class TaskApplicationService: + """CRUD + dispatch for Schedule Task, backed by a ``TaskRepository``. + + Args: + repository: a ``TaskRepository``-shaped object (``.get``, ``.save``, + ``.delete``, ``.duplicate``). Production passes + ``infrastructure.persistence.json.task_repository_impl. + TaskRepository()``; tests pass one scoped to a ``tmp_path``. + run_now: ``(task_id) -> bool``. Production passes + ``TaskScheduler.run_now``; ``None`` means no scheduler is wired + (matches the widget's own "no scheduler" guard today). + """ + + def __init__(self, repository: Any, run_now: Optional[RunNowFn] = None) -> None: + self._repository = repository + self._run_now = run_now + + # -- single-task actions ------------------------------------------------ # + def run_now(self, task_id: str) -> RunNowResult: + """Dispatch ``task_id`` immediately. A "Run now" always counts as + manual approval (spec §13) — this is the ONE path that bypasses + ``execution.requires_approval``, same as the scheduler's own + ``run_now`` already does.""" + task = self._repository.get(task_id) + if task is None: + return RunNowResult(False, "not_found") + if task.get("task_type") == "manual": + return RunNowResult(False, "manual_task") + if self._run_now is None: + return RunNowResult(False, "no_scheduler") + ok = self._run_now(task_id) + return RunNowResult(ok, "" if ok else "already_running") + + def duplicate(self, task_id: str) -> Optional[Dict[str, Any]]: + """A saved copy with a fresh identity — see + ``core/tasks.py::duplicate_task`` for what's preserved/reset.""" + task = self._repository.get(task_id) + if task is None: + return None + dup = self._repository.duplicate(task) + self._repository.save(dup) + return dup + + def toggle_pause(self, task_id: str) -> Optional[Dict[str, Any]]: + """Pause a task, or resume a paused one back to Backlog (matches + ``ui/schedule_task_tab.py``'s context-menu action exactly — resuming + does NOT restore whatever status the task had before pausing, only + Backlog, so the user re-schedules explicitly rather than a stale + schedule silently re-firing).""" + task = self._repository.get(task_id) + if task is None: + return None + task["status"] = "backlog" if task.get("status") == "paused" else "paused" + self._repository.save(task) + return task + + def delete(self, task_id: str) -> bool: + if self._repository.get(task_id) is None: + return False + self._repository.delete(task_id) + return True + + def bulk_delete(self, task_ids: List[str]) -> int: + """Delete every id in ``task_ids``; returns how many actually + existed (mirrors ``_confirm_and_delete_selected``'s best-effort + loop — a stale id in the selection doesn't abort the rest).""" + return sum(1 for tid in task_ids if self.delete(tid)) + + # -- Kanban drag/drop ----------------------------------------------------- # + def move_to_status(self, task_id: str, new_status: str) -> Optional[MoveResult]: + """Apply the business rule behind dropping a card into a lane + (``ui/schedule_task_tab.py::_on_task_dropped``, moved here so it's + testable without a real ``QListWidget`` drag gesture): + + * already running -> the drop is ignored (a running task can't be + re-filed by dragging it). + * dropped on Running -> runs it now (counts as manual approval). + * dropped on Done -> marks it done AND disables its schedule, so a + repeating task marked done by hand doesn't quietly re-fire later. + * dropped on Scheduled with no ``run_at`` set yet -> saved as-is but + flagged ``needs_schedule`` — the caller should open the editor + rather than leave a Scheduled card that will never actually run. + * anything else -> plain status change. + """ + task = self._repository.get(task_id) + if task is None: + return None + if task.get("status") == "running": + return MoveResult(task=task, blocked=True) + if new_status == "running": + result = self.run_now(task_id) + return MoveResult(task=self._repository.get(task_id), ran_now=True, run_now_result=result) + if new_status == "done": + task["status"] = "done" + task["schedule"]["enabled"] = False + self._repository.save(task) + return MoveResult(task=task) + task["status"] = new_status + if new_status == "scheduled" and not task["schedule"].get("enabled"): + if task["schedule"].get("run_at"): + task["schedule"]["enabled"] = True + else: + self._repository.save(task) + return MoveResult(task=task, needs_schedule=True) + self._repository.save(task) + return MoveResult(task=task) + + +__all__ = ["TaskApplicationService", "RunNowResult", "MoveResult"] diff --git a/core/task_scheduler.py b/core/task_scheduler.py index 0da120c..2a7d3ce 100644 --- a/core/task_scheduler.py +++ b/core/task_scheduler.py @@ -17,7 +17,7 @@ from datetime import datetime from pathlib import Path from typing import Dict, Optional -from PySide6.QtCore import QCoreApplication, QObject, QTimer, Signal +from PySide6.QtCore import QObject, Signal from .tasks import ( advance_after_run, chain_action, dependencies_met, due_tasks, format_run_at, @@ -39,22 +39,32 @@ class TaskScheduler(QObject): # which fires before the worker thread has even begun). history_ready = Signal(str) # task_id - def __init__(self, ctx, tasks_dir: Optional[Path] = None, parent=None): + def __init__(self, ctx, tasks_dir: Optional[Path] = None, parent=None, clock=None): super().__init__(parent) self.ctx = ctx self.tasks_dir = tasks_dir # None → default TASKS_DIR self._workers: Dict[str, AgentWorker] = {} # task_id → running worker self._retries: Dict[str, int] = {} self._session_ids: Dict[str, str] = {} # task_id → its run's History session id - self._timer = QTimer(self) - self._timer.setInterval(TICK_MS) - self._timer.timeout.connect(self.tick) + # R07-T03: the QTimer this class used to own directly is now behind a + # small clock interface (start/stop/pump) — see + # platform/qt/qt_scheduler_clock.py::QtSchedulerClock. Defaulting to a + # real one here keeps every existing production call site (which + # never passes `clock=`) unchanged; tests inject + # tests/fakes/fake_clock.py::FakeClock to control ticks by hand with + # no Qt event loop running. Imported lazily so importing core.tasks/ + # core.task_scheduler for the Qt-free logic doesn't require the Qt + # adapter module to even exist in a headless test context. + if clock is None: + from ..infrastructure.qt.qt_scheduler_clock import QtSchedulerClock + clock = QtSchedulerClock(self) + self._clock = clock # ---- lifecycle ---------------------------------------------------- def start(self) -> None: self._recover_orphans() self.tick() # catch up overdue tasks right at app start - self._timer.start() + self._clock.start(TICK_MS, self.tick) def stop(self) -> None: """Request every running worker to stop, then WAIT (bounded) for them @@ -67,15 +77,15 @@ class TaskScheduler(QObject): ``_on_done`` (the only place that writes the run into the task's history) never runs. The task's real output can already be sitting on disk while its history stays stuck on "running" forever. Pumping - ``processEvents()`` here lets that queued signal actually get - delivered before the app finishes quitting. + the clock here lets that queued signal actually get delivered before + the app finishes quitting. """ - self._timer.stop() + self._clock.stop() deadline = time.monotonic() + STOP_WAIT_SECS while self._workers and time.monotonic() < deadline: for w in list(self._workers.values()): w.request_stop() - QCoreApplication.processEvents() + self._clock.pump() for w in list(self._workers.values()): w.wait(50) # Anything still alive past the deadline is abandoned here; diff --git a/core/tasks.py b/core/tasks.py index fa54a67..cff3f44 100644 --- a/core/tasks.py +++ b/core/tasks.py @@ -151,7 +151,14 @@ def save_task(task: Dict[str, Any], directory: Path = None) -> Path: directory.mkdir(parents=True, exist_ok=True) task["updated_at"] = datetime.now().isoformat(timespec="seconds") path = task_path(task["task_id"], directory) - path.write_text(json.dumps(task, ensure_ascii=False, indent=2), encoding="utf-8") + # R07-T01: atomic write — same class of bug already fixed in + # core/projects.py and core/history.py at R06-T02 (plain write_text has a + # gap between truncate and write; a crash there leaves a half-written + # tasks/.json that load_task() then silently treats as "missing", + # dropping the task). Lazy import to match the existing call sites and + # avoid a core -> infrastructure import at module load time. + from ..infrastructure.persistence.json.atomic_write import write_json + write_json(path, task) return path @@ -287,36 +294,32 @@ def chain_error(tasks: List[Dict[str, Any]], task_id: str, # ---- schedule math -------------------------------------------------------- -def _is_excluded_day(dt: datetime, sched: Dict[str, Any]) -> bool: - """True when ``dt`` falls on a day this schedule must skip: a weekend - (working_days_only) or a public holiday of the configured country.""" - if sched.get("working_days_only") and dt.weekday() >= 5: # 5=Sat, 6=Sun - return True - if sched.get("skip_holidays"): +# R07-T02: the actual date/cron math now lives in +# domain/tasks/schedule_calculator.py::ScheduleCalculator (pure Python, unit +# tested on its own — see tests/unit/test_schedule_calculator.py). Everything +# below is a thin wrapper kept for backward compatibility: task_scheduler.py, +# task_executors.py and ui/task_editor_dialog.py all still import these +# module-level names from core.tasks, and core/holiday_calendar.py::is_holiday +# / core/cron.py::Cron are only wired in HERE (lazily, matching the previous +# lazy-import style) — domain/ is not allowed to import core/ (ADR-001 I2). +_calculator: Optional[Any] = None + + +def _get_calculator(): + global _calculator + if _calculator is None: + from .cron import Cron from .holiday_calendar import is_holiday + from ..domain.tasks.schedule_calculator import ScheduleCalculator - if is_holiday(dt.date(), sched.get("holiday_country", "")): - return True - return False - - -def _add_month(dt: datetime) -> datetime: - import calendar - - year = dt.year + (1 if dt.month == 12 else 0) - month = 1 if dt.month == 12 else dt.month + 1 - day = min(dt.day, calendar.monthrange(year, month)[1]) - return dt.replace(year=year, month=month, day=day) + _calculator = ScheduleCalculator(is_holiday=is_holiday, make_cron=Cron) + return _calculator def shift_off_excluded_days(dt: datetime, sched: Dict[str, Any]) -> datetime: """Push ``dt`` forward one day at a time until it lands on an allowed day (same time of day) — used for one-time schedules set on a weekend/holiday.""" - guard = 0 - while _is_excluded_day(dt, sched) and guard < 400: - dt += timedelta(days=1) - guard += 1 - return dt + return _get_calculator().shift_off_excluded_days(dt, sched) def compute_next_run(task: Dict[str, Any], after: datetime) -> Optional[datetime]: @@ -324,57 +327,12 @@ def compute_next_run(task: Dict[str, Any], after: datetime) -> Optional[datetime (daily / weekly / monthly / cron), or None for one-shot schedules. Occurrences on excluded days (weekends with working_days_only, public holidays with skip_holidays+holiday_country) are skipped forward.""" - sched = task.get("schedule", {}) - repeat = sched.get("repeat_type", "none") - - if repeat == "cron": - from .cron import Cron, CronError - - try: - cron = Cron(sched.get("cron_expression") or "") - except CronError: - return None - nxt = cron.next_after(after) - guard = 0 - while nxt is not None and _is_excluded_day(nxt, sched) and guard < 400: - nxt = cron.next_after(nxt) - guard += 1 - return nxt - - base = parse_run_at(sched.get("run_at")) - if base is None: - return None - if repeat == "daily": - advance = lambda d: d + timedelta(days=1) # noqa: E731 - elif repeat == "weekly": - advance = lambda d: d + timedelta(weeks=1) # noqa: E731 - elif repeat == "monthly": - advance = _add_month - else: - return None - nxt = base - while nxt <= after: - nxt = advance(nxt) - guard = 0 - while _is_excluded_day(nxt, sched) and guard < 400: - nxt = advance(nxt) - guard += 1 - return nxt + return _get_calculator().compute_next_run(task, after) def due_tasks(tasks: List[Dict[str, Any]], now: datetime) -> List[Dict[str, Any]]: """Tasks that should start now: Scheduled + schedule enabled + run_at due.""" - due = [] - for t in tasks: - if t.get("status") != "scheduled": - continue - sched = t.get("schedule", {}) - if not sched.get("enabled"): - continue - run_at = parse_run_at(sched.get("run_at")) - if run_at is not None and run_at <= now: - due.append(t) - return due + return _get_calculator().due_tasks(tasks, now) # ---- post-run bookkeeping (pure; scheduler applies + saves) --------------- diff --git a/docs/refactor/Refactoring_Checklist.md b/docs/refactor/Refactoring_Checklist.md index 6bb0630..12b1de7 100644 --- a/docs/refactor/Refactoring_Checklist.md +++ b/docs/refactor/Refactoring_Checklist.md @@ -209,18 +209,18 @@ * **Team chịu trách nhiệm**: 🟢 **Team Hoa** (Task Scheduling) + 🟣 **Team Nam** (Co4E Workflows) * **Mục tiêu**: Tách `TaskRepository` và `ScheduleCalculator` khỏi `QTimer` trong `core/task_scheduler.py#L20`; xây dựng `TaskApplicationService` và `Co4EWorkflowService`. -- [ ] **R07-T01 (Team Hoa)**: Tách `TaskRepository` lưu trữ JSON độc lập khỏi `core/tasks.py` ➔ `infrastructure/persistence/json/task_repository_impl.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* -- [ ] **R07-T02 (Team Hoa)**: Xây dựng `ScheduleCalculator` tính due-time / cron độc lập ➔ `domain/tasks/schedule_calculator.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* -- [ ] **R07-T03 (Team Hoa)**: Xây dựng `QtSchedulerClock` adapter (tách `TaskScheduler` khỏi `QTimer`) ➔ `platform/qt/qt_scheduler_clock.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* -- [ ] **R07-T04 (Team Hoa)**: Xây dựng `TaskApplicationService` (Pure Python) điều phối chạy, sao chép, dừng, xóa task ➔ `application/scheduling/task_application_service.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* -- [ ] **R07-T05 (Team Hoa)**: Xây dựng `AiTaskPlannerService` hỗ trợ tạo / import task bằng AI ➔ `application/scheduling/ai_task_planner_service.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* +- [x] **R07-T01 (Team Hoa)**: Tách `TaskRepository` lưu trữ JSON độc lập khỏi `core/tasks.py` ➔ `infrastructure/persistence/json/task_repository_impl.py` + *Start: `2026-08-27 16:05` | End: `2026-08-27 16:14`* +- [x] **R07-T02 (Team Hoa)**: Xây dựng `ScheduleCalculator` tính due-time / cron độc lập ➔ `domain/tasks/schedule_calculator.py` + *Start: `2026-08-27 16:14` | End: `2026-08-27 16:26`* +- [x] **R07-T03 (Team Hoa)**: Xây dựng `QtSchedulerClock` adapter (tách `TaskScheduler` khỏi `QTimer`) ➔ `infrastructure/qt/qt_scheduler_clock.py` (đổi so với plan gốc `platform/qt/...` — xem báo cáo) + *Start: `2026-08-27 16:26` | End: `2026-08-27 16:47`* +- [x] **R07-T04 (Team Hoa)**: Xây dựng `TaskApplicationService` (Pure Python) điều phối chạy, sao chép, dừng, xóa task ➔ `application/scheduling/task_application_service.py` + *Start: `2026-08-27 16:47` | End: `2026-08-27 17:02`* +- [x] **R07-T05 (Team Hoa)**: Xây dựng `AiTaskPlannerService` hỗ trợ tạo / import task bằng AI ➔ `application/scheduling/ai_task_planner_service.py` + *Start: `2026-08-27 17:02` | End: `2026-08-27 17:14`* - [ ] **R07-T06 (Team Nam)**: Xây dựng `Co4EWorkflowService` (Pure Python) quản lý định nghĩa và thực thi Co4E từ `core/co4e_run_manager.py` ➔ `application/workflows/co4e_workflow_service.py` - *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* + *Start: `____-__-__ __:__` | End: `____-__-__ __:__`* (ngoài phạm vi Team Hoa) --- diff --git a/domain/tasks/__init__.py b/domain/tasks/__init__.py new file mode 100644 index 0000000..8300006 --- /dev/null +++ b/domain/tasks/__init__.py @@ -0,0 +1,5 @@ +"""Domain entities for schedule/due-time computation (EPIC R07).""" + +from .schedule_calculator import ScheduleCalculator + +__all__ = ["ScheduleCalculator"] diff --git a/domain/tasks/schedule_calculator.py b/domain/tasks/schedule_calculator.py new file mode 100644 index 0000000..20b49e8 --- /dev/null +++ b/domain/tasks/schedule_calculator.py @@ -0,0 +1,165 @@ +"""ScheduleCalculator - due-time / cron / interval math for Schedule Task, +extracted from ``core/tasks.py``'s "schedule math" section (R07-T02). + +``core/tasks.py`` is already Qt-free (its own docstring says so), but it +still lives under ``core/`` where nothing enforces that "pure" claim - and it +is the ONE piece of scheduling logic ``docs/refactor/plan.md`` calls out as +needing its own unit tests (none existed before this task; see +``tests/unit/test_schedule_calculator.py``). Moving it to ``domain/tasks/`` +makes the purity a build-time guarantee (``scripts/check_imports.py`` fails +the build if this file ever imports Qt, ``core``, or anything with I/O) and +gives the date math a home that is trivially unit-testable without going +through ``core/tasks.py``'s file-repository concerns at all. + +Two pieces of this math are themselves implemented elsewhere in ``core/`` - +``core/cron.py::Cron`` (5-field cron parsing) and +``core/holiday_calendar.py::is_holiday`` (VN public holidays). Importing +``core`` from ``domain`` is exactly what ADR-001 rule I2 forbids (domain must +not know infrastructure/core exists), so this class takes them as +constructor-injected callables instead of importing them - the same +dependency-inversion shape ``application/conversations/conversation_ +application_service.py`` (R04-T03) already uses for its provider factory. +``core/tasks.py`` wires the real ``Cron``/``is_holiday`` in; tests can inject +plain stub functions with zero I/O. +""" +from __future__ import annotations + +import calendar +from datetime import datetime, timedelta +from typing import Any, Callable, Dict, List, Optional, Protocol + +# Same on-disk format core/tasks.py::_TIME_FMT uses for schedule.run_at. +# Duplicated here (not imported - that would be a domain -> core edge) since +# it's a 1-line format string, not business logic. +_TIME_FMT = "%Y-%m-%d %H:%M" + +_CRON_SEARCH_GUARD = 400 # matches the guard core/tasks.py used before extraction + + +class _CronLike(Protocol): + """Structural shape this class needs from a cron object - satisfied by + ``core/cron.py::Cron`` without this module importing it.""" + + def next_after(self, after: datetime) -> Optional[datetime]: + ... + + +def _parse_run_at(value: Optional[str]) -> Optional[datetime]: + if not value: + return None + try: + return datetime.strptime(value, _TIME_FMT) + except ValueError: + return None + + +class ScheduleCalculator: + """Pure due-time computation for one task's ``schedule`` dict. + + ``is_holiday``: ``Callable[[date, country_code], bool]`` or ``None`` - + when ``None``, a schedule with ``skip_holidays`` set simply never treats + any day as a holiday (degrades gracefully instead of raising, mirroring + how a caller who doesn't care about holidays can just not wire it up). + + ``make_cron``: ``Callable[[str], _CronLike]`` (raises on a malformed + expression) or ``None`` - when ``None``, ``repeat_type == "cron"`` + schedules never produce a next run (same as an invalid expression today). + """ + + def __init__(self, + is_holiday: Optional[Callable[[Any, str], bool]] = None, + make_cron: Optional[Callable[[str], _CronLike]] = None) -> None: + self._is_holiday = is_holiday + self._make_cron = make_cron + + def is_excluded_day(self, dt: datetime, sched: Dict[str, Any]) -> bool: + """True when ``dt`` falls on a day this schedule must skip: a + weekend (working_days_only) or a public holiday of the configured + country.""" + if sched.get("working_days_only") and dt.weekday() >= 5: # 5=Sat, 6=Sun + return True + if sched.get("skip_holidays") and self._is_holiday is not None: + if self._is_holiday(dt.date(), sched.get("holiday_country", "")): + return True + return False + + def add_month(self, dt: datetime) -> datetime: + """Calendar-aware +1 month, clamping the day to the target month's + length (e.g. Jan 31 + 1 month -> Feb 28/29, not an overflow error).""" + year = dt.year + (1 if dt.month == 12 else 0) + month = 1 if dt.month == 12 else dt.month + 1 + day = min(dt.day, calendar.monthrange(year, month)[1]) + return dt.replace(year=year, month=month, day=day) + + def shift_off_excluded_days(self, dt: datetime, sched: Dict[str, Any]) -> datetime: + """Push ``dt`` forward one day at a time until it lands on an + allowed day (same time of day) - used for one-time schedules set on + a weekend/holiday.""" + guard = 0 + while self.is_excluded_day(dt, sched) and guard < _CRON_SEARCH_GUARD: + dt += timedelta(days=1) + guard += 1 + return dt + + def compute_next_run(self, task: Dict[str, Any], after: datetime) -> Optional[datetime]: + """The next run time strictly after ``after`` for a repeating task + (daily / weekly / monthly / cron), or ``None`` for one-shot + schedules. Occurrences on excluded days are skipped forward.""" + sched = task.get("schedule", {}) + repeat = sched.get("repeat_type", "none") + + if repeat == "cron": + if self._make_cron is None: + return None + try: + cron = self._make_cron(sched.get("cron_expression") or "") + except Exception: + # Any malformed-expression error the injected factory raises + # (core/cron.py::CronError, or a fake's own error type in + # tests) means "this schedule can't compute a next run" - not + # a domain-layer crash. + return None + nxt = cron.next_after(after) + guard = 0 + while nxt is not None and self.is_excluded_day(nxt, sched) and guard < _CRON_SEARCH_GUARD: + nxt = cron.next_after(nxt) + guard += 1 + return nxt + + base = _parse_run_at(sched.get("run_at")) + if base is None: + return None + if repeat == "daily": + advance = lambda d: d + timedelta(days=1) # noqa: E731 + elif repeat == "weekly": + advance = lambda d: d + timedelta(weeks=1) # noqa: E731 + elif repeat == "monthly": + advance = self.add_month + else: + return None + nxt = base + while nxt <= after: + nxt = advance(nxt) + guard = 0 + while self.is_excluded_day(nxt, sched) and guard < _CRON_SEARCH_GUARD: + nxt = advance(nxt) + guard += 1 + return nxt + + def due_tasks(self, tasks: List[Dict[str, Any]], now: datetime) -> List[Dict[str, Any]]: + """Tasks that should start now: Scheduled + schedule enabled + + run_at due.""" + due = [] + for t in tasks: + if t.get("status") != "scheduled": + continue + sched = t.get("schedule", {}) + if not sched.get("enabled"): + continue + run_at = _parse_run_at(sched.get("run_at")) + if run_at is not None and run_at <= now: + due.append(t) + return due + + +__all__ = ["ScheduleCalculator"] diff --git a/infrastructure/persistence/json/__init__.py b/infrastructure/persistence/json/__init__.py index d128ee2..c25c50e 100644 --- a/infrastructure/persistence/json/__init__.py +++ b/infrastructure/persistence/json/__init__.py @@ -1,8 +1,9 @@ """JSON-file persistence adapters: crash-safe writes and the workspace/ -conversation repositories built on them (EPIC R06).""" +conversation/task repositories built on them (EPIC R06, R07).""" from .atomic_write import write_json from .conversation_repository_impl import ConversationRepository +from .task_repository_impl import TaskRepository from .workspace_repository_impl import WorkspaceRepository -__all__ = ["write_json", "WorkspaceRepository", "ConversationRepository"] +__all__ = ["write_json", "WorkspaceRepository", "ConversationRepository", "TaskRepository"] diff --git a/infrastructure/persistence/json/task_repository_impl.py b/infrastructure/persistence/json/task_repository_impl.py new file mode 100644 index 0000000..3294b00 --- /dev/null +++ b/infrastructure/persistence/json/task_repository_impl.py @@ -0,0 +1,63 @@ +"""TaskRepository - an object-shaped, atomic-write-backed facade over +``core/tasks.py`` (R07-T01). + +``core/tasks.py``'s module-level functions (``list_tasks``, ``load_task``, +``save_task``, ``delete_task``, ``new_task``, ``duplicate_task``) are still +what every existing call site (``core/task_scheduler.py``, +``core/task_executors.py``, ``ui/schedule_task_tab.py``) uses, and stay that +way - ``save_task`` now writes through :func:`atomic_write.write_json` +itself (R07-T01, same class of durability fix already applied to +``core/projects.py``/``core/history.py`` at R06-T02), so the fix applies +whether or not a caller ever touches this class. + +This repository exists for the application layer +(``application/scheduling``, R07-T04) to depend on an interface instead of +reaching into ``core/`` directly. It is a thin pass-through today, not a +re-implementation: same on-disk format, same directory, same functions +underneath. +""" +from __future__ import annotations + +from pathlib import Path +from typing import Any, Dict, List, Optional + +from cowork_local.core.tasks import ( + TASKS_DIR, + delete_task, + duplicate_task, + list_tasks, + load_task, + new_task, + save_task, +) + + +class TaskRepository: + """CRUD over task dicts (see ``core/tasks.py::DEFAULT_TASK`` for shape), + scoped to one ``directory`` (defaults to the app's real ``TASKS_DIR``; + tests pass a ``tmp_path`` so nothing touches the user's real config + folder).""" + + def __init__(self, directory: Optional[Path] = None) -> None: + self._directory = directory or TASKS_DIR + + def list(self) -> List[Dict[str, Any]]: + return list_tasks(self._directory) + + def get(self, task_id: str) -> Optional[Dict[str, Any]]: + return load_task(task_id, self._directory) + + def save(self, task: Dict[str, Any]) -> Path: + return save_task(task, self._directory) + + def create(self, title: str = "", **overrides: Any) -> Dict[str, Any]: + return new_task(title, **overrides) + + def duplicate(self, task: Dict[str, Any]) -> Dict[str, Any]: + return duplicate_task(task) + + def delete(self, task_id: str) -> None: + delete_task(task_id, self._directory) + + +__all__ = ["TaskRepository"] diff --git a/infrastructure/qt/__init__.py b/infrastructure/qt/__init__.py new file mode 100644 index 0000000..f45b744 --- /dev/null +++ b/infrastructure/qt/__init__.py @@ -0,0 +1,18 @@ +"""Qt-backed adapters for pure interfaces used elsewhere in the app (EPIC R07). + +Note: the original plan (``docs/refactor/Feature_Architecture_Proposal.md``) +placed this adapter at a new top-level ``platform/qt/`` package. That name +was dropped after it was shown to actually shadow the stdlib ``platform`` +module (used by ``core/windows_sandbox_vm.py``/``core/appcontainer_sandbox. +py``) whenever the repo root ends up on ``sys.path`` directly - e.g. running +``python -c "..."`` (or any script) with the repo root as the working +directory, which resolves a bare ``import platform`` to this package instead +of the standard library one. ``infrastructure/`` already exists as a layer +for exactly this kind of toolkit-specific implementation +(``infrastructure/filesystem/``, ``infrastructure/mcp/``, ...), so the +adapter lives here instead - same content, safer location. +""" + +from .qt_scheduler_clock import QtSchedulerClock + +__all__ = ["QtSchedulerClock"] diff --git a/infrastructure/qt/qt_scheduler_clock.py b/infrastructure/qt/qt_scheduler_clock.py new file mode 100644 index 0000000..5cbb4e7 --- /dev/null +++ b/infrastructure/qt/qt_scheduler_clock.py @@ -0,0 +1,70 @@ +"""QtSchedulerClock - the ``QTimer``-backed periodic ticker `TaskScheduler` +needs, pulled out from ``core/task_scheduler.py`` into its own adapter +(R07-T03). + +``core/task_scheduler.py::TaskScheduler`` is the only file in the scheduling +stack that imports Qt at all (confirmed by grep — ``core/tasks.py`` and +``core/task_executors.py`` are Qt-free). Everything it needs Qt FOR is small +and mechanical: an interval timer that calls back into ``tick()`` every +``TICK_MS``, plus, during ``stop()``, a way to pump the event loop so a +worker thread's queued ``finished_ok``/``failed`` signal still gets delivered +while draining running tasks (see the long comment on ``TaskScheduler.stop()`` +for why that pump matters). + +Wrapping exactly that surface — ``start(interval_ms, callback)``, ``stop()``, +``pump()`` — behind :class:`QtSchedulerClock` lets ``TaskScheduler`` take a +clock as a constructor parameter instead of constructing a ``QTimer`` +itself. Production wiring is unchanged (``TaskScheduler`` defaults to a real +``QtSchedulerClock`` when no clock is passed); tests can inject +``tests/fakes/fake_clock.py::FakeClock`` to control ticks by hand with no Qt +event loop running at all. + +See ``infrastructure/qt/__init__.py`` for why this lives under +``infrastructure/qt/`` and not the ``platform/qt/`` path the original plan +named. +""" +from __future__ import annotations + +from typing import Callable, Optional + +from PySide6.QtCore import QCoreApplication, QObject, QTimer + + +class QtSchedulerClock: + """Owns one ``QTimer``. Not itself a ``QObject`` subclass — it OWNS a + ``QObject``-parented timer instead of inheriting from one, so callers + (like ``FakeClock`` in tests) can satisfy the same duck-typed interface + without any Qt base class at all.""" + + def __init__(self, parent: Optional[QObject] = None) -> None: + # Parented so the timer is torn down with its owner instead of + # outliving it — the same lifetime QTimer(self) gave it inside + # TaskScheduler before this extraction. + self._timer = QTimer(parent) + self._timer.timeout.connect(self._on_timeout) + self._callback: Optional[Callable[[], None]] = None + + def _on_timeout(self) -> None: + if self._callback is not None: + self._callback() + + def start(self, interval_ms: int, callback: Callable[[], None]) -> None: + """Arm and start the timer. Calling this again while already + running re-arms it with the new interval/callback (matches + ``QTimer.start()``'s own restart-on-repeat-call behaviour).""" + self._callback = callback + self._timer.setInterval(interval_ms) + self._timer.start() + + def stop(self) -> None: + self._timer.stop() + + def pump(self) -> None: + """Process one batch of pending Qt events — used by + ``TaskScheduler.stop()``'s bounded drain loop so a worker thread's + queued completion signal can still be delivered while we wait for it + to exit.""" + QCoreApplication.processEvents() + + +__all__ = ["QtSchedulerClock"] diff --git a/tests/fakes/__init__.py b/tests/fakes/__init__.py index 5d83af8..142fde8 100644 --- a/tests/fakes/__init__.py +++ b/tests/fakes/__init__.py @@ -9,8 +9,13 @@ laptop, in CI and on a machine with no API keys configured. * :class:`~tests.fakes.fake_tool_executor.FakeToolExecutor` - a scripted stand-in for the ``extra_executor`` callable that ``core.chat_agent.run_cowork`` routes MCP/connector tool calls to. +* :class:`~tests.fakes.fake_clock.FakeClock` - a manually-fired stand-in for + ``platform/qt/qt_scheduler_clock.py::QtSchedulerClock`` (R07-T03), so + ``TaskScheduler`` dispatch logic can be tested tick-by-tick with no Qt + event loop running. """ +from .fake_clock import FakeClock from .fake_provider import FakeProvider, ScriptedTurn from .fake_tool_executor import FakeToolExecutor, ToolInvocation -__all__ = ["FakeProvider", "ScriptedTurn", "FakeToolExecutor", "ToolInvocation"] +__all__ = ["FakeProvider", "ScriptedTurn", "FakeToolExecutor", "ToolInvocation", "FakeClock"] diff --git a/tests/fakes/fake_clock.py b/tests/fakes/fake_clock.py new file mode 100644 index 0000000..ced8cbd --- /dev/null +++ b/tests/fakes/fake_clock.py @@ -0,0 +1,54 @@ +"""FakeClock - offline stand-in for ``platform/qt/qt_scheduler_clock.py:: +QtSchedulerClock`` (R07-T03). + +``TaskScheduler`` (``core/task_scheduler.py``) needs a clock that can +``start(interval_ms, callback)`` / ``stop()`` / ``pump()``. In production +that's a real ``QTimer``, which means testing dispatch logic (what runs, in +what order, what gets re-armed) would otherwise require a live Qt event loop +ticking every 30 seconds. This double satisfies the same duck-typed +interface with manual control: ``fire()`` calls the scripted callback once, +synchronously, on whichever thread the test is running on - no timers, no +event loop, no waiting. +""" +from __future__ import annotations + +from typing import Callable, Optional + + +class FakeClock: + """Scriptable stand-in for :class:`QtSchedulerClock`. + + Args: + running: whether ``start()`` has been called and ``stop()`` hasn't + since - a test can assert on this to check lifecycle wiring. + pump_count: how many times ``pump()`` was called - lets a test on + ``TaskScheduler.stop()``'s drain loop assert the event loop was + actually pumped while waiting for workers. + """ + + def __init__(self) -> None: + self._callback: Optional[Callable[[], None]] = None + self.interval_ms: Optional[int] = None + self.running: bool = False + self.pump_count: int = 0 + + def start(self, interval_ms: int, callback: Callable[[], None]) -> None: + self.interval_ms = interval_ms + self._callback = callback + self.running = True + + def stop(self) -> None: + self.running = False + + def pump(self) -> None: + self.pump_count += 1 + + def fire(self) -> None: + """Test helper: manually trigger one tick, as if the interval had + elapsed. A no-op when the clock isn't running (matches a real + ``QTimer`` never firing after ``stop()``).""" + if self.running and self._callback is not None: + self._callback() + + +__all__ = ["FakeClock"] diff --git a/tests/integration/test_qt_scheduler_clock.py b/tests/integration/test_qt_scheduler_clock.py new file mode 100644 index 0000000..ea93067 --- /dev/null +++ b/tests/integration/test_qt_scheduler_clock.py @@ -0,0 +1,53 @@ +"""EPIC R07-T03: QtSchedulerClock against a REAL QTimer/event loop. + +``tests/unit/test_task_scheduler_dispatch.py`` covers ``TaskScheduler``'s +dispatch logic entirely through ``tests/fakes/fake_clock.py::FakeClock`` (no +Qt at all — that's the whole point of the extraction). This file is the +complement: it proves the adapter itself actually drives a real ``QTimer`` +and pumps a real event loop, offscreen, the way ``test_history_dir_race.py`` +proves ``ui/chat_panel.py``'s fix against real Qt rather than a double. +""" +from __future__ import annotations + +import os + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +import pytest # noqa: E402 + +from PySide6.QtTest import QTest # noqa: E402 +from PySide6.QtWidgets import QApplication # noqa: E402 + +from cowork_local.infrastructure.qt.qt_scheduler_clock import QtSchedulerClock # noqa: E402 + + +@pytest.fixture(scope="module") +def qapp(): + return QApplication.instance() or QApplication([]) + + +def test_start_fires_callback_on_the_real_qt_event_loop(qapp): + clock = QtSchedulerClock() + ticks = [] + clock.start(interval_ms=10, callback=lambda: ticks.append(1)) + try: + QTest.qWait(200) # let the real QTimer fire a few times + finally: + clock.stop() + assert len(ticks) >= 1 + + +def test_stop_prevents_further_callbacks(qapp): + clock = QtSchedulerClock() + ticks = [] + clock.start(interval_ms=10, callback=lambda: ticks.append(1)) + QTest.qWait(50) + clock.stop() + count_after_stop = len(ticks) + QTest.qWait(100) + assert len(ticks) == count_after_stop # no more callbacks after stop() + + +def test_pump_processes_pending_events_without_raising(qapp): + clock = QtSchedulerClock() + clock.pump() # must not raise even with nothing pending diff --git a/tests/unit/test_ai_task_planner_service.py b/tests/unit/test_ai_task_planner_service.py new file mode 100644 index 0000000..ea341ed --- /dev/null +++ b/tests/unit/test_ai_task_planner_service.py @@ -0,0 +1,87 @@ +"""EPIC R07-T05: AiTaskPlannerService — AI-generate + import, no Qt, no network. + +``core/ai_task_planner.py::plan_tasks`` and ``core/task_import.py:: +import_tasks`` are exercised through a FakeProvider / real tmp files rather +than reimplemented — this service is a seam, not a new planner. +""" +from __future__ import annotations + +import json + +import pytest + +from cowork_local.application.scheduling.ai_task_planner_service import ( + AiTaskPlannerService, +) +from tests.fakes import FakeProvider, ScriptedTurn + +_PLAN_REPLY = json.dumps({ + "tasks": [ + {"title": "Draft report", "description": "d", "task_type": "cowork", + "priority": "medium", "schedule": {"enabled": False}} + ] +}) + + +def test_plan_uses_the_constructor_injected_provider_factory(): + provider = FakeProvider([ScriptedTurn(text=_PLAN_REPLY)]) + service = AiTaskPlannerService(provider_factory=lambda: provider) + + tasks = service.plan("Write a weekly report") + + assert len(tasks) == 1 + assert tasks[0]["title"] == "Draft report" + assert provider.call_count == 1 + + +def test_plan_prefers_an_explicit_provider_over_the_factory(): + factory_provider = FakeProvider([ScriptedTurn(text=_PLAN_REPLY)], strict=False) + explicit_provider = FakeProvider([ScriptedTurn(text=_PLAN_REPLY)]) + service = AiTaskPlannerService(provider_factory=lambda: factory_provider) + + service.plan("Write a weekly report", provider=explicit_provider) + + assert explicit_provider.call_count == 1 + assert factory_provider.call_count == 0 + + +def test_plan_without_any_provider_raises_runtime_error(): + service = AiTaskPlannerService(provider_factory=None) + with pytest.raises(RuntimeError): + service.plan("Write a weekly report") + + +def test_plan_stamps_attachments_onto_every_generated_task(): + two_tasks_reply = json.dumps({"tasks": [ + {"title": "A", "task_type": "cowork"}, + {"title": "B", "task_type": "cowork"}, + ]}) + provider = FakeProvider([ScriptedTurn(text=two_tasks_reply)]) + service = AiTaskPlannerService(provider_factory=lambda: provider) + + tasks = service.plan("do two things", file_paths=["a.txt"], links=["https://x"]) + + assert len(tasks) == 2 + for t in tasks: + assert t["input"]["file_paths"] == ["a.txt"] + assert t["input"]["links"] == ["https://x"] + + +def test_import_file_delegates_to_core_task_import(tmp_path): + csv_path = tmp_path / "tasks.csv" + csv_path.write_text("title,task_type,priority\nMy Task,cowork,medium\n", encoding="utf-8") + service = AiTaskPlannerService() + + tasks = service.import_file(csv_path) + + assert len(tasks) == 1 + assert tasks[0]["title"] == "My Task" + + +def test_import_file_raises_value_error_on_unsupported_extension(tmp_path): + bogus = tmp_path / "tasks.txt" + bogus.write_text("nope", encoding="utf-8") + service = AiTaskPlannerService() + + with pytest.raises(ValueError): + service.import_file(bogus) diff --git a/tests/unit/test_schedule_calculator.py b/tests/unit/test_schedule_calculator.py new file mode 100644 index 0000000..caa5053 --- /dev/null +++ b/tests/unit/test_schedule_calculator.py @@ -0,0 +1,164 @@ +"""EPIC R07-T02: ScheduleCalculator — pure due-time/cron math. + +This is the one piece of scheduling logic core/tasks.py's own docstring +claimed was "Qt-free so it can be unit-tested headlessly" but had NO unit +test at all before this task (confirmed by grepping tests/ for +"schedule_calculator"/"compute_next_run"/"cron" — nothing matched). These +tests exercise domain/tasks/schedule_calculator.py directly, with no Qt, no +filesystem, and fake holiday/cron callables so the module stays provably +zero-I/O. +""" +from __future__ import annotations + +from datetime import datetime + +import pytest + +from cowork_local.domain.tasks.schedule_calculator import ScheduleCalculator + + +def _sched(**overrides): + base = { + "enabled": True, + "run_at": "2026-08-24 09:00", # a Monday + "repeat_type": "none", + "cron_expression": None, + "working_days_only": False, + "skip_holidays": False, + "holiday_country": "VN", + } + base.update(overrides) + return base + + +def _task(**sched_overrides): + return {"status": "scheduled", "schedule": _sched(**sched_overrides)} + + +class _FakeCron: + """A cron stub that fires every day at a fixed hour:minute — enough to + exercise the cron branch without depending on core/cron.py::Cron.""" + + def __init__(self, expression: str): + if expression == "bad": + raise ValueError("bad cron expression") + self.hour, self.minute = 10, 0 + + def next_after(self, after: datetime) -> datetime: + candidate = after.replace(hour=self.hour, minute=self.minute, second=0, microsecond=0) + if candidate <= after: + from datetime import timedelta + candidate += timedelta(days=1) + return candidate + + +def _is_weekend_holiday(date_, country): + # A deterministic fake: only 2026-08-29 (a Saturday) counts as a holiday. + return date_.isoformat() == "2026-08-29" + + +def test_daily_advances_by_one_day(): + calc = ScheduleCalculator() + task = _task(repeat_type="daily") + nxt = calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) + assert nxt == datetime(2026, 8, 25, 9, 0) + + +def test_weekly_advances_by_seven_days(): + calc = ScheduleCalculator() + task = _task(repeat_type="weekly") + nxt = calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) + assert nxt == datetime(2026, 8, 31, 9, 0) + + +def test_monthly_clamps_day_to_shorter_month(): + calc = ScheduleCalculator() + task = _task(run_at="2026-01-31 09:00", repeat_type="monthly") + nxt = calc.compute_next_run(task, datetime(2026, 1, 31, 9, 0)) + assert nxt == datetime(2026, 2, 28, 9, 0) # Feb 2026 has 28 days + + +def test_one_shot_repeat_type_none_has_no_next_run(): + calc = ScheduleCalculator() + task = _task(repeat_type="none") + assert calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) is None + + +def test_cron_without_injected_factory_returns_none(): + """No make_cron wired in -> a cron schedule simply never produces a next + run, instead of crashing the caller.""" + calc = ScheduleCalculator() + task = _task(repeat_type="cron", cron_expression="0 10 * * *") + assert calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) is None + + +def test_cron_uses_injected_factory(): + calc = ScheduleCalculator(make_cron=_FakeCron) + task = _task(repeat_type="cron", cron_expression="0 10 * * *") + nxt = calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) + assert nxt == datetime(2026, 8, 24, 10, 0) + + +def test_malformed_cron_expression_returns_none_not_raise(): + calc = ScheduleCalculator(make_cron=_FakeCron) + task = _task(repeat_type="cron", cron_expression="bad") + assert calc.compute_next_run(task, datetime(2026, 8, 24, 9, 0)) is None + + +def test_working_days_only_skips_weekend(): + calc = ScheduleCalculator() + # 2026-08-28 is a Friday; +1 day (daily) would land on Saturday 08-29. + task = _task(run_at="2026-08-28 09:00", repeat_type="daily", working_days_only=True) + nxt = calc.compute_next_run(task, datetime(2026, 8, 28, 9, 0)) + assert nxt.weekday() < 5 # Monday 08-31, not the weekend + + +def test_skip_holidays_uses_injected_is_holiday(): + calc = ScheduleCalculator(is_holiday=_is_weekend_holiday) + # 2026-08-28 (Fri) + 1 day = 2026-08-29, which the fake marks a holiday. + task = _task(run_at="2026-08-28 09:00", repeat_type="daily", skip_holidays=True) + nxt = calc.compute_next_run(task, datetime(2026, 8, 28, 9, 0)) + assert nxt == datetime(2026, 8, 30, 9, 0) # skipped past the holiday + + +def test_skip_holidays_without_injected_is_holiday_degrades_gracefully(): + calc = ScheduleCalculator() # no is_holiday wired in + task = _task(run_at="2026-08-28 09:00", repeat_type="daily", skip_holidays=True) + nxt = calc.compute_next_run(task, datetime(2026, 8, 28, 9, 0)) + assert nxt == datetime(2026, 8, 29, 9, 0) # no holiday check applied at all + + +def test_shift_off_excluded_days_moves_weekend_forward(): + calc = ScheduleCalculator() + sched = _sched(working_days_only=True) + saturday = datetime(2026, 8, 29, 9, 0) + shifted = calc.shift_off_excluded_days(saturday, sched) + assert shifted.weekday() < 5 + assert shifted >= saturday + + +def test_add_month_end_of_year_rolls_to_january(): + calc = ScheduleCalculator() + assert calc.add_month(datetime(2026, 12, 15, 9, 0)) == datetime(2027, 1, 15, 9, 0) + + +def test_due_tasks_filters_by_status_enabled_and_run_at(): + calc = ScheduleCalculator() + now = datetime(2026, 8, 27, 12, 0) + due_now = _task(run_at="2026-08-27 09:00") + due_now["title"] = "due" + future = _task(run_at="2026-08-28 09:00") + future["title"] = "future" + disabled = _task(run_at="2026-08-27 09:00", enabled=False) + disabled["title"] = "disabled" + not_scheduled = _task(run_at="2026-08-27 09:00") + not_scheduled["title"] = "backlog" + not_scheduled["status"] = "backlog" + + result = calc.due_tasks([due_now, future, disabled, not_scheduled], now) + assert [t["title"] for t in result] == ["due"] + + +def test_due_tasks_empty_when_no_tasks(): + calc = ScheduleCalculator() + assert calc.due_tasks([], datetime(2026, 8, 27, 12, 0)) == [] diff --git a/tests/unit/test_task_application_service.py b/tests/unit/test_task_application_service.py new file mode 100644 index 0000000..87501ec --- /dev/null +++ b/tests/unit/test_task_application_service.py @@ -0,0 +1,191 @@ +"""EPIC R07-T04: TaskApplicationService — CRUD + dispatch rules, no Qt. + +Everything here used to be exercised only by driving the real +``ui/schedule_task_tab.py`` widget (a QListWidget drag gesture, a QMenu +click). These tests drive the same rules directly through the service. +""" +from __future__ import annotations + +from cowork_local.application.scheduling.task_application_service import ( + TaskApplicationService, +) +from cowork_local.infrastructure.persistence.json import TaskRepository + + +def _service(tmp_path, run_now=None): + return TaskApplicationService(TaskRepository(tmp_path), run_now=run_now) + + +def _new_saved_task(repo: TaskRepository, **overrides): + task = repo.create("T", **overrides) + repo.save(task) + return task + + +def test_run_now_rejects_manual_task_type(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo, task_type="manual") + service = TaskApplicationService(repo, run_now=lambda tid: True) + + result = service.run_now(task["task_id"]) + + assert result.ok is False + assert result.reason == "manual_task" + + +def test_run_now_without_scheduler_wired_reports_no_scheduler(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo, task_type="cowork") + service = TaskApplicationService(repo, run_now=None) + + result = service.run_now(task["task_id"]) + + assert result.ok is False + assert result.reason == "no_scheduler" + + +def test_run_now_delegates_to_injected_scheduler(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo, task_type="cowork") + seen = [] + service = TaskApplicationService(repo, run_now=lambda tid: seen.append(tid) or True) + + result = service.run_now(task["task_id"]) + + assert result.ok is True + assert seen == [task["task_id"]] + + +def test_run_now_missing_task_reports_not_found(tmp_path): + service = _service(tmp_path, run_now=lambda tid: True) + result = service.run_now("does-not-exist") + assert result.ok is False + assert result.reason == "not_found" + + +def test_duplicate_saves_a_copy_with_fresh_identity(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo, description="d") + service = TaskApplicationService(repo) + + dup = service.duplicate(task["task_id"]) + + assert dup is not None + assert dup["task_id"] != task["task_id"] + assert dup["description"] == "d" + assert repo.get(dup["task_id"]) is not None # actually persisted, not just returned + + +def test_toggle_pause_then_resume_goes_to_backlog(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + service = TaskApplicationService(repo) + + paused = service.toggle_pause(task["task_id"]) + assert paused["status"] == "paused" + + resumed = service.toggle_pause(task["task_id"]) + assert resumed["status"] == "backlog" + + +def test_delete_reports_whether_the_task_existed(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + service = TaskApplicationService(repo) + + assert service.delete(task["task_id"]) is True + assert repo.get(task["task_id"]) is None + assert service.delete(task["task_id"]) is False # already gone + + +def test_bulk_delete_counts_only_tasks_that_existed(tmp_path): + repo = TaskRepository(tmp_path) + a = _new_saved_task(repo) + b = _new_saved_task(repo) + service = TaskApplicationService(repo) + + count = service.bulk_delete([a["task_id"], b["task_id"], "ghost-id"]) + + assert count == 2 + assert repo.list() == [] + + +def test_move_to_status_running_task_is_blocked(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + task["status"] = "running" + repo.save(task) + service = TaskApplicationService(repo) + + result = service.move_to_status(task["task_id"], "backlog") + + assert result.blocked is True + assert repo.get(task["task_id"])["status"] == "running" # untouched + + +def test_move_to_status_running_lane_dispatches_run_now(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo, task_type="cowork") + seen = [] + service = TaskApplicationService(repo, run_now=lambda tid: seen.append(tid) or True) + + result = service.move_to_status(task["task_id"], "running") + + assert result.ran_now is True + assert result.run_now_result.ok is True + assert seen == [task["task_id"]] + + +def test_move_to_status_done_disables_the_schedule(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + task["schedule"]["enabled"] = True + task["schedule"]["run_at"] = "2026-08-28 09:00" + repo.save(task) + service = TaskApplicationService(repo) + + result = service.move_to_status(task["task_id"], "done") + + assert result.task["status"] == "done" + assert result.task["schedule"]["enabled"] is False + assert repo.get(task["task_id"])["schedule"]["enabled"] is False + + +def test_move_to_status_scheduled_without_run_at_needs_schedule(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) # fresh task has schedule.run_at = None + service = TaskApplicationService(repo) + + result = service.move_to_status(task["task_id"], "scheduled") + + assert result.needs_schedule is True + assert repo.get(task["task_id"])["status"] == "scheduled" + assert repo.get(task["task_id"])["schedule"]["enabled"] is False + + +def test_move_to_status_scheduled_with_run_at_enables_it(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + task["schedule"]["run_at"] = "2026-08-28 09:00" + repo.save(task) + service = TaskApplicationService(repo) + + result = service.move_to_status(task["task_id"], "scheduled") + + assert result.needs_schedule is False + assert repo.get(task["task_id"])["schedule"]["enabled"] is True + + +def test_move_to_status_plain_change(tmp_path): + repo = TaskRepository(tmp_path) + task = _new_saved_task(repo) + service = TaskApplicationService(repo) + + result = service.move_to_status(task["task_id"], "backlog") + + assert result.task["status"] == "backlog" + + +def test_move_to_status_missing_task_returns_none(tmp_path): + service = _service(tmp_path) + assert service.move_to_status("does-not-exist", "backlog") is None diff --git a/tests/unit/test_task_repository.py b/tests/unit/test_task_repository.py new file mode 100644 index 0000000..8ca55cf --- /dev/null +++ b/tests/unit/test_task_repository.py @@ -0,0 +1,70 @@ +"""EPIC R07-T01: TaskRepository + core/tasks.py::save_task atomic write. + +The motivating bug: ``core/tasks.py::save_task`` used to +``path.write_text(json.dumps(...))`` — two syscalls, no atomicity, same class +of bug already fixed for projects/conversations at R06-T02. A failure between +the write and the replace must never leave a half-written task JSON file on +disk; that is the one property the crash-injection test exists to pin. +""" +from __future__ import annotations + +import json + +import pytest + +from cowork_local.core.tasks import new_task +from cowork_local.infrastructure.persistence.json import TaskRepository + + +def test_task_repository_crud_round_trip(tmp_path): + repo = TaskRepository(tmp_path) + task = repo.create("My Task") + repo.save(task) + + assert [t["task_id"] for t in repo.list()] == [task["task_id"]] + assert repo.get(task["task_id"])["title"] == "My Task" + + task["title"] = "Renamed" + repo.save(task) + assert repo.get(task["task_id"])["title"] == "Renamed" + + repo.delete(task["task_id"]) + assert repo.get(task["task_id"]) is None + assert repo.list() == [] + + +def test_task_repository_duplicate_keeps_config_resets_identity(tmp_path): + repo = TaskRepository(tmp_path) + task = repo.create("Original", description="d") + repo.save(task) + + dup = repo.duplicate(task) + repo.save(dup) + + assert dup["task_id"] != task["task_id"] + assert dup["description"] == "d" + assert {t["task_id"] for t in repo.list()} == {task["task_id"], dup["task_id"]} + + +def test_save_task_never_corrupts_existing_file_on_crash(tmp_path, monkeypatch): + """Same guarantee as ``test_atomic_write_and_repositories.py``'s crash + test, exercised through ``core/tasks.py::save_task`` directly (not just + through the repository) since that is the function every existing + scheduler/executor call site still uses.""" + from cowork_local.core.tasks import save_task, task_path + + task = new_task("Stable") + save_task(task, tmp_path) + + import cowork_local.infrastructure.persistence.json.atomic_write as mod + + def boom(*_a, **_k): + raise OSError("simulated crash between write and replace") + + monkeypatch.setattr(mod.os, "replace", boom) + task["title"] = "Corrupted?" + with pytest.raises(OSError): + save_task(task, tmp_path) + + on_disk = json.loads(task_path(task["task_id"], tmp_path).read_text(encoding="utf-8")) + assert on_disk["title"] == "Stable" diff --git a/tests/unit/test_task_scheduler_clock_wiring.py b/tests/unit/test_task_scheduler_clock_wiring.py new file mode 100644 index 0000000..182ce4e --- /dev/null +++ b/tests/unit/test_task_scheduler_clock_wiring.py @@ -0,0 +1,66 @@ +"""EPIC R07-T03: TaskScheduler <-> clock wiring, entirely through +tests/fakes/fake_clock.py::FakeClock — no real QTimer, no Qt event loop. + +Scope note: this only exercises the clock injection seam (start arms+starts +the clock with `tick`, stop stops it), not the full dispatch/execution +pipeline (`_start` -> `AgentWorker` -> `execute_task`), which needs a real +``ctx``/provider and is exactly the kind of Qt-adjacent, thread-heavy path +better left to an offscreen integration test if/when R08 touches this file +again — recorded here rather than silently left untested. +""" +from __future__ import annotations + +from cowork_local.core.task_scheduler import TICK_MS, TaskScheduler +from tests.fakes import FakeClock + + +def test_start_arms_and_starts_the_injected_clock(tmp_path): + clock = FakeClock() + scheduler = TaskScheduler(ctx=None, tasks_dir=tmp_path, clock=clock) + ticks = [] + # Instance-attribute override, set BEFORE start(): TaskScheduler.start() + # reads `self.tick`, which Python resolves to this override rather than + # the class method, so we can count calls without a real due task/ctx. + scheduler.tick = lambda: ticks.append(1) + + scheduler.start() + + assert clock.running is True + assert clock.interval_ms == TICK_MS + assert ticks == [1] # the catch-up tick() call at startup + + +def test_clock_fire_drives_another_tick(tmp_path): + clock = FakeClock() + scheduler = TaskScheduler(ctx=None, tasks_dir=tmp_path, clock=clock) + ticks = [] + scheduler.tick = lambda: ticks.append(1) + scheduler.start() + + clock.fire() + + assert ticks == [1, 1] + + +def test_stop_stops_the_clock(tmp_path): + clock = FakeClock() + scheduler = TaskScheduler(ctx=None, tasks_dir=tmp_path, clock=clock) + scheduler.tick = lambda: None + scheduler.start() + + scheduler.stop() + + assert clock.running is False + + +def test_fire_after_stop_does_not_call_tick(tmp_path): + clock = FakeClock() + scheduler = TaskScheduler(ctx=None, tasks_dir=tmp_path, clock=clock) + ticks = [] + scheduler.tick = lambda: ticks.append(1) + scheduler.start() + scheduler.stop() + + clock.fire() + + assert ticks == [1] # only the startup catch-up tick, nothing after stop