P4.1: Extract reusable dependency check helper, deduplicate milestone_actions.py

- New app/services/dependency_check.py with check_milestone_deps()
- Replaces 3x duplicated JSON-parse + query + filter logic
- Supports both milestone and task dependency checking
- Returns structured DepCheckResult with ok/blockers/reason
- Refactored preflight and start endpoints to use shared helper

app/api/routers/milestone_actions.py

@@ -3,7 +3,6 @@
 Provides freeze / start / close actions on milestones.
 completed is triggered automatically when the sole release maintenance task finishes.
 """
-import json
 from datetime import datetime, timezone
 from typing import Optional
 
@@ -18,6 +17,7 @@ from app.models import models
 from app.models.milestone import Milestone, MilestoneStatus
 from app.models.task import Task, TaskStatus
 from app.services.activity import log_activity
+from app.services.dependency_check import check_milestone_deps
 
 router = APIRouter(
     prefix="/projects/{project_id}/milestones/{milestone_id}/actions",
@@ -101,38 +101,13 @@ def preflight_milestone_actions(
 
     # --- start pre-check (only meaningful when status == freeze) ---
     if ms_status == "freeze":
-        blockers: list[str] = []
-
-        # milestone dependencies
-        dep_ms_ids = []
-        if ms.depend_on_milestones:
-            try:
-                dep_ms_ids = json.loads(ms.depend_on_milestones)
-            except (json.JSONDecodeError, TypeError):
-                dep_ms_ids = []
-        if dep_ms_ids:
-            dep_milestones = db.query(Milestone).filter(Milestone.id.in_(dep_ms_ids)).all()
-            incomplete = [m.id for m in dep_milestones if _ms_status_value(m) != "completed"]
-            if incomplete:
-                blockers.append(f"Dependent milestones not completed: {incomplete}")
-
-        # task dependencies
-        dep_task_ids = []
-        if ms.depend_on_tasks:
-            try:
-                dep_task_ids = json.loads(ms.depend_on_tasks)
-            except (json.JSONDecodeError, TypeError):
-                dep_task_ids = []
-        if dep_task_ids:
-            dep_tasks = db.query(Task).filter(Task.id.in_(dep_task_ids)).all()
-            incomplete_tasks = [t.id for t in dep_tasks if (t.status.value if hasattr(t.status, "value") else t.status) != "completed"]
-            if incomplete_tasks:
-                blockers.append(f"Dependent tasks not completed: {incomplete_tasks}")
-
-        if blockers:
-            result["start"] = {"allowed": False, "reason": "; ".join(blockers)}
-        else:
+        dep_result = check_milestone_deps(
+            db, ms.depend_on_milestones, ms.depend_on_tasks,
+        )
+        if dep_result.ok:
             result["start"] = {"allowed": True, "reason": None}
+        else:
+            result["start"] = {"allowed": False, "reason": dep_result.reason}
 
     return result
 
@@ -232,50 +207,14 @@ def start_milestone(
             detail=f"Cannot start: milestone is '{_ms_status_value(ms)}', expected 'freeze'",
         )
 
-    # Dependency check — milestone dependencies
-    dep_ms_ids = []
-    if ms.depend_on_milestones:
-        try:
-            dep_ms_ids = json.loads(ms.depend_on_milestones)
-        except (json.JSONDecodeError, TypeError):
-            dep_ms_ids = []
-
-    if dep_ms_ids:
-        dep_milestones = (
-            db.query(Milestone)
-            .filter(Milestone.id.in_(dep_ms_ids))
-            .all()
+    # Dependency check (P4.1 — shared helper)
+    dep_result = check_milestone_deps(
+        db, ms.depend_on_milestones, ms.depend_on_tasks,
     )
-        incomplete = [
-            m.id
-            for m in dep_milestones
-            if _ms_status_value(m) != "completed"
-        ]
-        if incomplete:
+    if not dep_result.ok:
         raise HTTPException(
             status_code=400,
-                detail=f"Cannot start: dependent milestones not completed: {incomplete}",
-            )
-
-    # Dependency check — task dependencies
-    dep_task_ids = []
-    if ms.depend_on_tasks:
-        try:
-            dep_task_ids = json.loads(ms.depend_on_tasks)
-        except (json.JSONDecodeError, TypeError):
-            dep_task_ids = []
-
-    if dep_task_ids:
-        dep_tasks = db.query(Task).filter(Task.id.in_(dep_task_ids)).all()
-        incomplete_tasks = [
-            t.id
-            for t in dep_tasks
-            if (t.status.value if hasattr(t.status, "value") else t.status) != "completed"
-        ]
-        if incomplete_tasks:
-            raise HTTPException(
-                status_code=400,
-                detail=f"Cannot start: dependent tasks not completed: {incomplete_tasks}",
+            detail=f"Cannot start: {dep_result.reason}",
         )
 
     ms.status = MilestoneStatus.UNDERGOING

app/services/dependency_check.py

@@ -0,0 +1,113 @@
+"""P4.1 — Reusable dependency-check helpers.
+
+Used by milestone start, milestone preflight, and (future) task pending→open
+to verify that all declared dependencies are completed before allowing the
+entity to proceed.
+"""
+from __future__ import annotations
+
+import json
+from dataclasses import dataclass, field
+from typing import Sequence
+
+from sqlalchemy.orm import Session
+
+from app.models.milestone import Milestone
+from app.models.task import Task
+
+
+# ---------------------------------------------------------------------------
+# Result type
+# ---------------------------------------------------------------------------
+
+@dataclass
+class DepCheckResult:
+    """Outcome of a dependency check."""
+
+    ok: bool = True
+    blockers: list[str] = field(default_factory=list)
+
+    @property
+    def reason(self) -> str | None:
+        return "; ".join(self.blockers) if self.blockers else None
+
+
+# ---------------------------------------------------------------------------
+# Internal helpers
+# ---------------------------------------------------------------------------
+
+def _parse_json_ids(raw: str | None) -> list[int]:
+    """Safely parse a JSON-encoded list of integer IDs."""
+    if not raw:
+        return []
+    try:
+        ids = json.loads(raw)
+        if isinstance(ids, list):
+            return [int(i) for i in ids]
+    except (json.JSONDecodeError, TypeError, ValueError):
+        pass
+    return []
+
+
+def _ms_status(ms: Milestone) -> str:
+    return ms.status.value if hasattr(ms.status, "value") else ms.status
+
+
+def _task_status(t: Task) -> str:
+    return t.status.value if hasattr(t.status, "value") else t.status
+
+
+# ---------------------------------------------------------------------------
+# Public API
+# ---------------------------------------------------------------------------
+
+def check_milestone_deps(
+    db: Session,
+    depend_on_milestones: str | None,
+    depend_on_tasks: str | None,
+    *,
+    required_status: str = "completed",
+) -> DepCheckResult:
+    """Check whether all milestone + task dependencies are satisfied.
+
+    Parameters
+    ----------
+    db:
+        Active DB session.
+    depend_on_milestones:
+        JSON-encoded list of milestone IDs (from the entity's field).
+    depend_on_tasks:
+        JSON-encoded list of task IDs (from the entity's field).
+    required_status:
+        The status that dependees must have reached (default ``"completed"``).
+
+    Returns
+    -------
+    DepCheckResult with ``ok=True`` if all deps satisfied, else ``ok=False``
+    with human-readable ``blockers``.
+    """
+    result = DepCheckResult()
+
+    # --- milestone dependencies ---
+    ms_ids = _parse_json_ids(depend_on_milestones)
+    if ms_ids:
+        dep_milestones: Sequence[Milestone] = (
+            db.query(Milestone).filter(Milestone.id.in_(ms_ids)).all()
+        )
+        incomplete = [m.id for m in dep_milestones if _ms_status(m) != required_status]
+        if incomplete:
+            result.ok = False
+            result.blockers.append(f"Dependent milestones not {required_status}: {incomplete}")
+
+    # --- task dependencies ---
+    task_ids = _parse_json_ids(depend_on_tasks)
+    if task_ids:
+        dep_tasks: Sequence[Task] = (
+            db.query(Task).filter(Task.id.in_(task_ids)).all()
+        )
+        incomplete_tasks = [t.id for t in dep_tasks if _task_status(t) != required_status]
+        if incomplete_tasks:
+            result.ok = False
+            result.blockers.append(f"Dependent tasks not {required_status}: {incomplete_tasks}")
+
+    return result