Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 32 additions & 6 deletions codeframe/core/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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()
Expand Down
45 changes: 45 additions & 0 deletions tests/core/test_task_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading