From 45a8db4be9eae294f2a261dc816377363e6b9511 Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:58:13 -0700 Subject: [PATCH 1/2] fix(tasks): cascade dependency cleanup on delete (#724) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- codeframe/core/tasks.py | 31 ++++++++++++++++++++++------ tests/core/test_task_dependencies.py | 31 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index b59aa363..e682f98b 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -725,23 +725,42 @@ 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: cursor = conn.cursor() + # Strip this task from dependents' depends_on lists first, in the same + # transaction as the delete so we never leave a dangling reference. + 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), + ) + cursor.execute( """ DELETE FROM tasks diff --git a/tests/core/test_task_dependencies.py b/tests/core/test_task_dependencies.py index c4426c76..7ac7b628 100644 --- a/tests/core/test_task_dependencies.py +++ b/tests/core/test_task_dependencies.py @@ -192,3 +192,34 @@ 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 + + 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 From 6edf4adb9f20d3e4d81aa39c5ef648d4f5a0a1a1 Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Fri, 3 Jul 2026 00:04:35 -0700 Subject: [PATCH 2/2] fix(tasks): only cascade after a real delete; test missing-task + updated_at (#724 review) --- codeframe/core/tasks.py | 41 ++++++++++++++++------------ tests/core/test_task_dependencies.py | 14 ++++++++++ 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/codeframe/core/tasks.py b/codeframe/core/tasks.py index e682f98b..742e159c 100644 --- a/codeframe/core/tasks.py +++ b/codeframe/core/tasks.py @@ -744,23 +744,6 @@ def delete(workspace: Workspace, task_id: str) -> bool: try: cursor = conn.cursor() - # Strip this task from dependents' depends_on lists first, in the same - # transaction as the delete so we never leave a dangling reference. - 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), - ) - cursor.execute( """ DELETE FROM tasks @@ -769,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 7ac7b628..4baf9df8 100644 --- a/tests/core/test_task_dependencies.py +++ b/tests/core/test_task_dependencies.py @@ -208,6 +208,20 @@ def test_delete_strips_id_from_dependents(self, workspace): 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")