fix(tasks): cascade dependency cleanup on delete (#724) (#796)

* fix(tasks): cascade dependency cleanup on delete (#724)

tasks.delete() left the deleted id in other tasks' depends_on and pointed users
to a delete_cascade() that never existed. A dependent kept a dangling depends_on
entry; readiness needs deps.issubset(completed), so it could never become ready
— deleting a task silently and permanently stranded its dependents.

delete() now strips the id from every dependent's depends_on in the same
transaction as the row delete; docstring reference to the nonexistent
delete_cascade() removed and the new behavior documented.

Closes #724

* fix(tasks): only cascade after a real delete; test missing-task + updated_at (#724 review)

Author: frankbria

codeframe/core/tasks.py

@@ -725,18 +725,20 @@ def get_dependents(workspace: Workspace, task_id: str) -> list[Task]:
 
 
 def delete(workspace: Workspace, task_id: str) -> bool:
-    """Delete a task by ID.
+    """Delete a task by ID, cascading the dependency cleanup.
+
+    The deleted id is also stripped from every other task's ``depends_on``
+    (#724). Without this a dependent keeps a ``depends_on`` entry pointing at a
+    now-missing task, and ``get_ready_tasks`` (which needs
+    ``deps.issubset(completed)``) can never mark it ready — deleting a task used
+    to silently and permanently strand its dependents.
 
     Args:
         workspace: Target workspace
         task_id: Task ID to delete
 
     Returns:
-        True if task was deleted, False if not found
-
-    Note:
-        This does NOT remove the task from other tasks' depends_on lists.
-        Use delete_cascade() if you need to clean up dependencies.
+        True if the task was deleted, False if not found
     """
     conn = get_db_connection(workspace)
     try:
@@ -750,6 +752,30 @@ def delete(workspace: Workspace, task_id: str) -> bool:
             (workspace.id, task_id),
         )
         deleted = cursor.rowcount > 0
+
+        # Only cascade if the task actually existed, and only after the DELETE
+        # so the deleted row can't appear in the scan below. Same transaction
+        # (single commit) so no dangling reference can survive a rollback.
+        # (This is the write-side counterpart to get_dependents()'s read: the
+        # tasks that carry task_id in their depends_on JSON.) The full-workspace
+        # scan is fine for typical task counts; add a LIKE pre-filter if a
+        # workspace ever holds thousands of tasks.
+        if deleted:
+            now = _utc_now().isoformat()
+            cursor.execute(
+                "SELECT id, depends_on FROM tasks WHERE workspace_id = ?",
+                (workspace.id,),
+            )
+            for dep_row_id, dep_json in cursor.fetchall():
+                deps = json.loads(dep_json or "[]")
+                if task_id in deps:
+                    deps = [d for d in deps if d != task_id]
+                    cursor.execute(
+                        "UPDATE tasks SET depends_on = ?, updated_at = ? "
+                        "WHERE workspace_id = ? AND id = ?",
+                        (json.dumps(deps), now, workspace.id, dep_row_id),
+                    )
+
         conn.commit()
     finally:
         conn.close()

tests/core/test_task_dependencies.py

@@ -192,3 +192,48 @@ def test_task_created_before_migration_has_empty_depends_on(self, workspace):
         task = tasks.create(workspace, title="Test task")
         retrieved = tasks.get(workspace, task.id)
         assert retrieved.depends_on == []
+
+
+class TestDeleteCascade:
+    """#724 / P0.13: deleting a task must strip its id from dependents'
+    depends_on, or they strand (a dangling dep can never be satisfied)."""
+
+    def test_delete_strips_id_from_dependents(self, workspace):
+        a = tasks.create(workspace, title="A")
+        b = tasks.create(workspace, title="B", depends_on=[a.id])
+        assert a.id in tasks.get(workspace, b.id).depends_on
+
+        assert tasks.delete(workspace, a.id) is True
+
+        b2 = tasks.get(workspace, b.id)
+        assert a.id not in b2.depends_on
+        assert b2.depends_on == []  # no dangling reference → not stranded
+        # updated_at is bumped so caches/UI detect the cascade edit.
+        assert b2.updated_at >= b.updated_at
+
+    def test_delete_missing_task_does_not_touch_dependents(self, workspace):
+        """A delete that finds nothing (returns False) must not mutate dependents."""
+        a = tasks.create(workspace, title="A")
+        b = tasks.create(workspace, title="B", depends_on=[a.id])
+        before = tasks.get(workspace, b.id).updated_at
+
+        assert tasks.delete(workspace, "does-not-exist") is False
+
+        b2 = tasks.get(workspace, b.id)
+        assert b2.depends_on == [a.id]  # untouched
+        assert b2.updated_at == before
+
+    def test_delete_keeps_other_dependencies(self, workspace):
+        a = tasks.create(workspace, title="A")
+        c = tasks.create(workspace, title="C")
+        b = tasks.create(workspace, title="B", depends_on=[a.id, c.id])
+
+        tasks.delete(workspace, a.id)
+
+        b2 = tasks.get(workspace, b.id)
+        assert b2.depends_on == [c.id]  # only the deleted dep is removed
+
+    def test_delete_task_without_dependents(self, workspace):
+        a = tasks.create(workspace, title="A")
+        assert tasks.delete(workspace, a.id) is True
+        assert tasks.get(workspace, a.id) is None