diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index b59aa363..742e159c 100644 --- a/codeframe/core/tasks.py +++ b/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() diff --git a/tests/core/test_task_dependencies.py b/tests/core/test_task_dependencies.py index c4426c76..4baf9df8 100644 --- a/tests/core/test_task_dependencies.py +++ b/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