feat(P5.1-P5.6): task state-machine validation — enforce legal transitions in transition/batch/update endpoints

app/api/routers/tasks.py

@@ -19,6 +19,25 @@ from app.services.activity import log_activity
 
 router = APIRouter(tags=["Tasks"])
 
+# ---- State-machine: valid transitions (P5.1-P5.6) ----
+VALID_TRANSITIONS: dict[str, set[str]] = {
+    "pending":    {"open", "closed"},
+    "open":       {"undergoing", "closed"},
+    "undergoing": {"completed", "closed"},
+    "completed":  {"open"},       # reopen
+    "closed":     {"open"},       # reopen
+}
+
+def _check_transition(old_status: str, new_status: str) -> None:
+    """Raise 400 if the transition is not allowed by the state machine."""
+    allowed = VALID_TRANSITIONS.get(old_status, set())
+    if new_status not in allowed:
+        raise HTTPException(
+            status_code=400,
+            detail=f"Cannot transition from '{old_status}' to '{new_status}'. "
+                   f"Allowed targets from '{old_status}': {sorted(allowed) if allowed else 'none'}",
+        )
+
 # ---- Type / Subtype validation ----
 TASK_SUBTYPE_MAP = {
     'issue': {'infrastructure', 'performance', 'regression', 'security', 'user_experience', 'defect'},
@@ -167,6 +186,11 @@ def update_task(task_id: int, task_update: schemas.TaskUpdate, db: Session = Dep
     update_data = task_update.model_dump(exclude_unset=True)
     if "status" in update_data:
         new_status = update_data["status"]
+        old_status = task.status.value if hasattr(task.status, 'value') else task.status
+        # P5.1: enforce state-machine even through PATCH
+        _check_transition(old_status, new_status)
+        if new_status == "open" and old_status in ("completed", "closed"):
+            task.finished_on = None
         if new_status == "undergoing" and not task.started_on:
             task.started_on = datetime.utcnow()
         if new_status in ("closed", "completed") and not task.finished_on:
@@ -208,6 +232,31 @@ def transition_task(task_id: int, new_status: str, bg: BackgroundTasks, db: Sess
     if not task:
         raise HTTPException(status_code=404, detail="Task not found")
     old_status = task.status.value if hasattr(task.status, 'value') else task.status
+
+    # P5.1: enforce state-machine
+    _check_transition(old_status, new_status)
+
+    # P5.2: pending -> open requires milestone to be undergoing (dependencies checked later)
+    if old_status == "pending" and new_status == "open":
+        milestone = db.query(Milestone).filter(Milestone.id == task.milestone_id).first()
+        if milestone:
+            ms_status = milestone.status.value if hasattr(milestone.status, 'value') else milestone.status
+            if ms_status != "undergoing":
+                raise HTTPException(
+                    status_code=400,
+                    detail=f"Cannot open task: milestone is '{ms_status}', must be 'undergoing'",
+                )
+
+    # P5.3: open -> undergoing requires assignee
+    if old_status == "open" and new_status == "undergoing":
+        if not task.assignee_id:
+            raise HTTPException(status_code=400, detail="Cannot start task: assignee must be set first")
+
+    # P5.6: reopen from completed/closed -> open
+    if new_status == "open" and old_status in ("completed", "closed"):
+        # Clear finished_on on reopen so lifecycle timestamps are accurate
+        task.finished_on = None
+
     if new_status == "undergoing" and not task.started_on:
         task.started_on = datetime.utcnow()
     if new_status in ("closed", "completed") and not task.finished_on:
@@ -306,17 +355,32 @@ def batch_transition(data: BatchTransition, bg: BackgroundTasks, db: Session = D
     if data.new_status not in valid_statuses:
         raise HTTPException(status_code=400, detail="Invalid status")
     updated = []
+    skipped = []
     for task_id in data.task_ids:
         task = db.query(Task).filter(Task.id == task_id).first()
         if task:
             old_status = task.status.value if hasattr(task.status, 'value') else task.status
+            allowed = VALID_TRANSITIONS.get(old_status, set())
+            if data.new_status not in allowed:
+                skipped.append({"id": task.id, "title": task.title, "old": old_status,
+                                "reason": f"Cannot transition from '{old_status}' to '{data.new_status}'"})
+                continue
+            if data.new_status == "undergoing" and not task.started_on:
+                task.started_on = datetime.utcnow()
+            if data.new_status in ("closed", "completed") and not task.finished_on:
+                task.finished_on = datetime.utcnow()
+            if data.new_status == "open" and old_status in ("completed", "closed"):
+                task.finished_on = None
             task.status = data.new_status
             updated.append({"id": task.id, "title": task.title, "old": old_status, "new": data.new_status})
     db.commit()
     for u in updated:
         event = "task.closed" if data.new_status == "closed" else "task.updated"
         bg.add_task(fire_webhooks_sync, event, u, None, db)
-    return {"updated": len(updated), "tasks": updated}
+    result = {"updated": len(updated), "tasks": updated}
+    if skipped:
+        result["skipped"] = skipped
+    return result
 
 
 @router.post("/tasks/batch/assign")