fix-forward #2622: archive_checklist_item still publishes under task_id in its fallback, contradicting its own docstring (tsk-s5pif2)

[jaylfc]

CARD TITLE (intent, not commit subject): fix-forward #2622: archive_checklist_item still publishes under task_id in its fallback, contradicting its own docstring
Autonomous build of board card tsk-s5pif2.

BASE: exec/tsk-pa2zau
Supersedes #2622

Base correction (please read first)

The card's BASE: exec/tsk-pa2zau is dead. PR #2622 was closed unmerged
(mergedAt: null), and its head 237dcd37f is not an ancestor of dev.
The checklist feature reached dev through a different carry (#2674, 79a5f12e5),
so the create-path fix #2622 was cut to make never landed. Building on
exec/tsk-pa2zau would have replayed a stale tree against a base that has moved
147 commits on. This branch is therefore cut from origin/dev (b8f7726ea).

That changes what the defect looks like, but not that it is real:

	card / exec/tsk-pa2zau	current dev
create_checklist_item fallback	raises (the #2622 fix)	project_id = ... else ""
archive_checklist_item fallback	else item["task_id"]	else ""

So on dev both siblings still resolve the publish topic to something that
is not a project_id. The card's premise ("create was fixed to raise, archive
kept the fallback") does not hold on dev — neither was fixed — and the card's
FIX ("make the archive path raise, exactly as create does") cannot be satisfied
by copying create, because create does not raise either. Both are fixed here.

The reachability the card asserts is confirmed verbatim on dev:
task_checklist_items.task_id declares REFERENCES project_tasks(id), and
ProjectTaskStore never issues PRAGMA foreign_keys = ON (grep for the pragma
hits mcp/registry.py, shared_folders.py, relationships.py,
library_store.py, secrets.py — never projects/task_store.py). A checklist
item can outlive its task, and ProjectEventBroker keys one channel per
project_id, so the event lands where nobody is subscribed and is silently lost.

What changed

tinyagentos/projects/task_store.py

• create_checklist_item resolves the parent task before the INSERT and
  raises ValueError: task not found: <task_id> when it is missing, so a
  refusal cannot leave an orphan row behind. The publish now uses
  task["project_id"] unconditionally — the else "" fallback is gone.
• archive_checklist_item resolves the parent task before the UPDATE, with
  the same raise, so a refusal leaves the item un-archived rather than
  half-applying the mutation and then losing its event. Same fallback removal.
• Both docstrings now state why the task is required (it carries the only
  project_id a project subscriber can be reached at) instead of asserting a
  guarantee the code did not have.

tests/projects/test_task_store.py

• Two mutating acceptance tests. The archive one deletes the parent row out from
  under a live, verified+reported checklist item (_delete_task_row, which is
  only possible because the FK is unenforced), then asserts no event reached
  either dead topic — "" (the dev fallback) or the task_id (the
  pre-fix-forward #2606: checklist create publishes under task_id in its fallback branch - the defect the PR closes, kept behind a comment saying it cannot happen #2622 one) — that the call raised, and that archived stayed False.
  The create one does the same against a task id that never existed and asserts
  no row was inserted.
• The raise is captured into a variable rather than wrapped in pytest.raises
  on purpose: with pytest.raises first, the topic assertion is unreachable on
  a red run and the failure reads "DID NOT RAISE", which is exactly the
  "asserting archive returns 200 cannot fail" shape the card rules out. In this
  order the assertion that goes red is the one naming the wrong topic and the
  event that went to it.

RED FIRST (pasted)

Run at the base source (git checkout origin/dev -- tinyagentos/projects/task_store.py,
new tests in place):

$ .venv/bin/python -m pytest tests/projects/test_task_store.py -q -p no:cacheprovider -k "refuses_when_task"
FF                                                                       [100%]
=================================== FAILURES ===================================
____________ test_archive_checklist_item_refuses_when_task_is_gone _____________

    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}"
E           AssertionError: event published to dead topic '': ['checklist.item.archived']
E           assert not ['checklist.item.archived']

tests/projects/test_task_store.py:507: AssertionError
___________ test_create_checklist_item_refuses_when_task_is_missing ____________

    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}"
E           AssertionError: event published to dead topic '': ['checklist.item.created']
E           assert not ['checklist.item.created']

tests/projects/test_task_store.py:533: AssertionError
=========================== short test summary info ============================
FAILED tests/projects/test_task_store.py::test_archive_checklist_item_refuses_when_task_is_gone
FAILED tests/projects/test_task_store.py::test_create_checklist_item_refuses_when_task_is_missing
2 failed, 44 deselected in 1.99s

The failing assertion is the topic one, and it names the event and the channel
it was misrouted to — the mutation is what makes it fail, per the card's
"ACCEPTANCE MUST MUTATE".

GREEN

$ .venv/bin/python -m pytest tests/projects/test_task_store.py tests/test_routes_task_checklist.py -q -p no:cacheprovider
......................................................                   [100%]
54 passed in 102.01s (0:01:42)

Also run on the fix:

$ .venv/bin/python -m pytest tests/projects/ -q -p no:cacheprovider
412 passed in 400.61s (0:06:40)

$ .venv/bin/python -m pytest tests/test_project_task_store.py tests/test_project_events.py \
    tests/test_routes_project_tasks_aggregate.py tests/test_routes_task_checklist.py \
    tests/test_auth_middleware.py -q -p no:cacheprovider
153 passed in 204.70s (0:03:24)

Docs

• docs/agent-coordination.md — the "Task checklist items" section under the
  Bearer-allowlist surface documented the routes and the verified+reported
  archive precondition but said nothing about the publish topic. It now records
  that both checklist.item.created and checklist.item.archived go out under
  the task's project_id, that the store raises task not found when the task
  is gone, and why the unenforced foreign key makes that state reachable.
• changelog.d/tsk-s5pif2-checklist-orphan-task-publish.md — new fragment.

Summary by CodeRabbit

• Bug Fixes

  • Checklist items can no longer be created for or archived under tasks that no longer exist.
  • Invalid operations now return a clear “task not found” error without creating orphaned records, changing existing items, or publishing unusable events.
  • Checklist events are now routed using the parent task’s project information.

• Documentation

  • Updated task checklist documentation to describe validation behavior and event routing.

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: 420eb6d8-4fe6-4d61-bd41-52d858f44513

📥 Commits

Reviewing files that changed from the base of the PR and between b8f7726 and fd72b48.

📒 Files selected for processing (4)

• changelog.d/tsk-s5pif2-checklist-orphan-task-publish.md
• docs/agent-coordination.md
• tests/projects/test_task_store.py
• tinyagentos/projects/task_store.py

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

Checklist item creation and archiving now verify that the parent task exists before changing storage or publishing events. Tests cover missing tasks, unchanged archive state, absent orphan rows, and suppressed invalid-topic events. Documentation and changelog entries describe the behavior.

Changes

Checklist orphan-task guards

Layer / File(s)	Summary
Parent-task validation and regression coverage tinyagentos/projects/task_store.py, tests/projects/test_task_store.py, docs/agent-coordination.md, changelog.d/tsk-s5pif2-checklist-orphan-task-publish.md	Checklist item creation and archiving raise ValueError when the parent task is missing. Creation inserts only after task resolution. Events use the resolved task project_id. Tests verify no invalid events, mutations, or orphan rows occur.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to fd72b

Checklist creation and archiving now reject missing parent tasks before changing data and publish successful events under the task project. The covered missing-task behavior prevents orphaned changes and lost events, with no remaining merge-blocking risk identified.

Suggested reviewers: hognek

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (2 skipped: 2…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies the checklist event-topic defect and references issue #2622. It is related to the primary change, although it does not mention the corresponding create-path correction.
Linked Issues check	✅ Passed	The PR resolves missing-task handling for checklist creation and archiving, publishes successful events under the task project topic, adds regression tests, and updates documentation and the changelog…
Out of Scope Changes check	✅ Passed	The code, tests, documentation, and changelog changes directly support the checklist missing-task and event-topic fixes. No unrelated changes are present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (2 skipped: 2 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch exec/tsk-s5pif2

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gitar-bot]

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

_Gitar

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Reviewed all 4 changed files. The fix is well-scoped: parent task resolution is moved before the mutation in both create_checklist_item and archive_checklist_item, the dead-letter fallback topic is removed, the docstrings explain the constraint, the two new tests mutate the database to reach the orphan state (using the unenforced FK as the test description explicitly calls out) and assert that neither dead topic ("" nor the task id) receives the event, and the changelog fragment matches the project convention.

Files Reviewed (4 files)

• tinyagentos/projects/task_store.py
• tests/projects/test_task_store.py
• docs/agent-coordination.md
• changelog.d/tsk-s5pif2-checklist-orphan-task-publish.md

---

_{Reviewed by minimax-m3:free · Input: 26.7K · Output: 4.4K · Cached: 325.5K}

[jaylfc]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.