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
9 changes: 9 additions & 0 deletions changelog.d/tsk-s5pif2-checklist-orphan-task-publish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
### Fixed
- Checklist item create and archive now refuse a task that no longer exists
instead of publishing their broker event under a topic no project subscriber
listens to. `task_checklist_items.task_id` declares a foreign key but the
task store never enables `PRAGMA foreign_keys = ON`, so a checklist item can
outlive its task; the previous fallback resolved the publish topic to an
empty string and the `checklist.item.created` / `checklist.item.archived`
event was silently lost. Both paths now resolve the parent task before they
mutate, so a refusal leaves no orphan row and no half-applied archive.
9 changes: 9 additions & 0 deletions docs/agent-coordination.md
Original file line number Diff line number Diff line change
Expand Up @@ -1174,6 +1174,15 @@ Route module `tinyagentos/routes/projects.py`.
another project is existence-hiding rather than merely forbidden.
- Creating an item logs `checklist.item.created` to the project activity feed
with the actor, task id, item id and text.
- Both `checklist.item.created` and `checklist.item.archived` are published on
the broker under the task's **`project_id`**, because project subscribers
subscribe at project scope. The store therefore resolves the parent task
first and raises `ValueError: task not found: <task_id>` when it is gone —
create refuses before inserting the row, archive refuses before flipping
`archived`. `task_checklist_items.task_id` declares a foreign key but the
store never sets `PRAGMA foreign_keys = ON`, so an item can outlive its task;
without the guard the event went to a topic nobody listens to and was
silently lost.
- Archiving is store-level only and refuses unless the item is both **verified**
and **reported**; there is no archive route.
- `DELETE` and per-item subpaths (`.../checklist-items/{item_id}`) stay
Expand Down
74 changes: 74 additions & 0 deletions tests/projects/test_task_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,80 @@ async def test_checklist_item_event_delivered_at_project_scope(store_with_broker
assert checklist_events[0].payload["task_id"] == t["id"]


async def _delete_task_row(store, task_id: str) -> None:
"""Drop a task row out from under its checklist items.

``task_checklist_items.task_id`` declares ``REFERENCES project_tasks(id)``
but ProjectTaskStore never issues ``PRAGMA foreign_keys = ON``, so SQLite
does not enforce it and a live checklist item can outlive its task.
"""
await store._db.execute("DELETE FROM project_tasks WHERE id = ?", (task_id,))
await store._db.commit()


@pytest.mark.asyncio
async def test_archive_checklist_item_refuses_when_task_is_gone(store_with_broker):
"""Archiving an orphaned item must raise, not publish off-topic.

The fallback resolved the publish topic to something that is not a
project_id, so ``checklist.item.archived`` landed on a channel no
project subscriber listens to and the mutation was silently lost.
"""
store, broker = store_with_broker
t = await store.create_task(project_id="proj-orphan", title="Objective", created_by="u")
item = await store.create_checklist_item(task_id=t["id"], text="step one", created_by="u")
await store.update_checklist_item(item_id=item["id"], verified=True, reported=True)
await _delete_task_row(store, t["id"])

# Captured rather than asserted with pytest.raises so the topic assertion
# below is the one that reports the defect, not an unreached line after it.
raised: ValueError | None = None
try:
await store.archive_checklist_item(item_id=item["id"])
except ValueError as exc:
raised = exc

# Nothing may reach a non-project channel: "" is the current fallback
# topic, the task_id is the pre-#2622 one. Both are dead letter boxes.
for dead_topic in ("", t["id"]):
queue = await broker.subscribe(dead_topic)
stray = []
while not queue.empty():
stray.append(queue.get_nowait().kind)
assert not stray, f"event published to dead topic {dead_topic!r}: {stray}"

assert raised is not None and "task not found" in str(raised), (
f"archive must refuse an item whose task is gone, raised: {raised!r}"
)
# The refused archive must not have mutated the row either.
again = await store.get_checklist_item(item["id"])
assert again["archived"] is False


@pytest.mark.asyncio
async def test_create_checklist_item_refuses_when_task_is_missing(store_with_broker):
"""Same defect on the sibling create path — it must refuse, not orphan a row."""
store, broker = store_with_broker

raised: ValueError | None = None
try:
await store.create_checklist_item(task_id="tsk-ghost", text="step one", created_by="u")
except ValueError as exc:
raised = exc

for dead_topic in ("", "tsk-ghost"):
queue = await broker.subscribe(dead_topic)
stray = []
while not queue.empty():
stray.append(queue.get_nowait().kind)
assert not stray, f"event published to dead topic {dead_topic!r}: {stray}"

assert raised is not None and "task not found" in str(raised), (
f"create must refuse a missing task, raised: {raised!r}"
)
assert await store.list_checklist_items(task_id="tsk-ghost", include_archived=True) == []


@pytest.mark.asyncio
async def test_checklist_item_created_by_persists(store):
"""Acceptance 1: Round-trip test: create_checklist_item(created_by="u") -> list_checklist_items returns created_by == "u". RED on origin/dev (column absent), green on the fix."""
Expand Down
26 changes: 19 additions & 7 deletions tinyagentos/projects/task_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,6 +917,16 @@ async def create_checklist_item(
text: str,
created_by: str,
) -> dict:
"""Create a checklist item on a task.

Raises ValueError if the task does not exist: the item's only route to
a project subscriber is the task's ``project_id``, so a missing task
leaves nothing to publish under. Resolved before the INSERT so a refusal
never leaves an orphan row behind.
"""
task = await self.get_task(task_id)
if task is None:
raise ValueError(f"task not found: {task_id}")
cid = new_id("cki")
now = time.time()
await self._db.execute(
Expand All @@ -932,9 +942,7 @@ async def create_checklist_item(
row = await cur.fetchone()
desc = cur.description
item = _row_to_checklist_item(row, desc)
task = await self.get_task(task_id)
project_id = task["project_id"] if task is not None else ""
await self._publish(project_id, "checklist.item.created", {"id": item["id"], "text": item["text"], "task_id": task_id})
await self._publish(task["project_id"], "checklist.item.created", {"id": item["id"], "text": item["text"], "task_id": task_id})
return item

async def list_checklist_items(
Expand Down Expand Up @@ -989,7 +997,10 @@ async def archive_checklist_item(self, item_id: str) -> dict:
"""Archive a checklist item. Only valid if verified=1 and reported=1.

Raises ValueError if the item cannot be archived because it lacks
verification or a report.
verification or a report, or because its task is gone — the task
carries the ``project_id`` that ``checklist.item.archived`` is
published under, and project subscribers listen at project scope only.
Resolved before the UPDATE so a refusal leaves the item untouched.
"""
item = await self.get_checklist_item(item_id)
if item is None:
Expand All @@ -998,15 +1009,16 @@ async def archive_checklist_item(self, item_id: str) -> dict:
raise ValueError("item cannot be archived: not verified")
if item["reported"] != 1:
raise ValueError("item cannot be archived: not reported")
task = await self.get_task(item["task_id"])
if task is None:
raise ValueError(f"task not found: {item['task_id']}")
now = time.time()
await self._db.execute(
"UPDATE task_checklist_items SET archived = 1, updated_at = ? WHERE id = ?",
(now, item_id),
)
await self._db.commit()
task = await self.get_task(item["task_id"])
project_id = task["project_id"] if task is not None else ""
await self._publish(project_id, "checklist.item.archived", {"id": item_id, "task_id": item["task_id"], "archived": True})
await self._publish(task["project_id"], "checklist.item.archived", {"id": item_id, "task_id": item["task_id"], "archived": True})
return await self.get_checklist_item(item_id)

async def get_checklist_item(self, item_id: str) -> dict | None:
Expand Down
Loading