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

changelog.d/tsk-pa2zau-checklist-fixes.md

@@ -0,0 +1,6 @@
+### Fixed
+
+- Checklist item creation now raises `ValueError` when task is not found, preventing events from being published under task_id instead of project_id
+- Added `created_by` column to `task_checklist_items` table and included it in INSERT statements and event payloads
+- Fixed `test_survives_agent_restart` to actually restart store and verify persistence across store instances
+- Fixed `archive_checklist_item` to include `reported_by` in event payload and raise `ValueError` when task is missing

changelog.d/tsk-y44sls-checklist-carry.md

@@ -0,0 +1,3 @@
+### Added
+
+- OS-owned task checklist items for project tasks. `POST/GET /api/projects/{project_id}/tasks/{task_id}/checklist-items` create and list per-task checklist items (create takes a JSON `{"text"}`, list honours `?include_archived=`); items carry `done`/`verified`/`reported`/`archived` state and are archived only when verified+reported, publishing `checklist.item.created`/`checklist.item.archived` under the task's resolved `project_id` (where project subscribers are scoped) so a missing item raises `ValueError` rather than a `TypeError`.

docs/agent-coordination.md

@@ -962,6 +962,33 @@ where `revoked=0 OR blocked=1`, so a blocked device counts against
 `_MAX_DEVICES_PER_USER` until it is unblocked, at which point the row falls out
 and the slot frees. Deliberate: a blocked device is a retained safety valve the
 owner can still see and act on.
+
+## Task checklist items (`/api/projects/{project_id}/tasks/{task_id}/checklist-items`)
+
+Route module `tinyagentos/routes/projects.py`.
+
+- `POST .../checklist-items` takes a JSON body `{"text": "..."}` and creates one
+  item. A missing `text` is a `422`.
+- `GET .../checklist-items` lists items, newest state included. Takes
+  `?include_archived=true`; the default hides archived items.
+- Both answer `404` when the task is not in the named project, so a task id from
+  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.
+- Archiving is store-level only and refuses unless the item is both **verified**
+  and **reported**; there is no archive route.
+
+The handlers call `_authorize_task_actor(...)` and accept EITHER a session
+owner/admin OR a project-bound agent's registry JWT. The Bearer allowlist in
+`tinyagentos/auth_middleware.py` now matches both `GET` and `POST .../checklist-items`
+(see `## Agent-token API surface (Bearer allowlist)` above), so the middleware
+gate no longer refuses agent tokens with `401` and the handler scope check now
+runs: `POST` (create) requires the narrower `project_tasks_create` grant, while
+`GET` (list) takes the default `project_tasks` read grant. A `project_tasks`
+worker lane is therefore refused on `POST` (it lacks the create grant, `403`)
+and authorised on `GET`. `tests/test_routes_task_checklist.py` pins this scope
+split directly, not behind an xfail.
+
 ## Controller generation echo (split-brain protection)
 
 Route module `tinyagentos/routes/cluster.py`, manager logic in
@@ -1039,10 +1066,11 @@ place.
 Task checklist items (added with the OS-owned objective checklist, #2415):
 
 - `GET /api/projects/{project_id}/tasks/{task_id}/checklist-items` -- list;
-  Bearer-reachable so the handler's `project_tasks_create` scope check runs
+  Bearer-reachable so the handler's `project_tasks` (read) scope check runs
   instead of the middleware refusing 401 at the gate.
 - `POST /api/projects/{project_id}/tasks/{task_id}/checklist-items` -- create;
-  same scope check.
+  Bearer-reachable, gated by the narrower `project_tasks_create` scope check
+  rather than the middleware refusing 401 at the gate.
 - `DELETE` and per-item subpaths (`.../checklist-items/{item_id}`) stay
   session-only: no agent-reachable handler exists, and the allowlist must not
   widen past list + create.

tests/projects/test_event_broker_integration.py

@@ -72,3 +72,50 @@ async def test_relationship_and_comment_events(store_with_broker):
     await store.add_comment(a["id"], "u1", "hi")
     ev = await asyncio.wait_for(queue.get(), timeout=0.5)
     assert ev.kind == "comment.added"
+
+
+@pytest.mark.asyncio
+async def test_create_checklist_item_emits_event_at_project_scope(store_with_broker):
+    """RED proof for fix #1 (event scope): checklist.item.created must be
+    published under the task's resolved project_id, because project
+    subscribers subscribe at project_id scope. On the pre-fix code the event is
+    published under task_id, so a project_id subscriber sees nothing and the
+    wait_for times out -> test FAILS. Post-fix it arrives -> passes.
+    """
+    store, broker = store_with_broker
+    project_id = "proj-A"
+    t = await store.create_task(project_id=project_id, title="T", created_by="u")
+    queue = await broker.subscribe(project_id)
+    # Drain the replayed task.created event (published under project_id).
+    await queue.get()
+    item = await store.create_checklist_item(task_id=t["id"], text="x", created_by="u")
+    ev = await asyncio.wait_for(queue.get(), timeout=0.5)
+    assert ev.kind == "checklist.item.created"
+    assert ev.payload["id"] == item["id"]
+    assert ev.payload["task_id"] == t["id"]
+
+
+@pytest.mark.asyncio
+async def test_archive_checklist_item_emits_event_at_project_scope(store_with_broker):
+    """RED proof for fix #1 (event scope): checklist.item.archived must be
+    published under the task's resolved project_id. On the pre-fix code it is
+    published under task_id, so the project_id subscriber never sees it and the
+    wait_for times out -> test FAILS. Post-fix it arrives -> passes.
+    """
+    store, broker = store_with_broker
+    project_id = "proj-A"
+    queue = await broker.subscribe(project_id)
+    t = await store.create_task(project_id=project_id, title="T", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="x", created_by="u")
+    await store.update_checklist_item(item["id"], verified=True, reported=True)
+    # Drain task.created, then checklist.item.created.
+    ev1 = await asyncio.wait_for(queue.get(), timeout=0.5)
+    assert ev1.kind == "task.created"
+    ev2 = await asyncio.wait_for(queue.get(), timeout=0.5)
+    assert ev2.kind == "checklist.item.created"
+    archived = await store.archive_checklist_item(item["id"], reported_by="u")
+    assert archived["archived"] is True
+    ev3 = await asyncio.wait_for(queue.get(), timeout=0.5)
+    assert ev3.kind == "checklist.item.archived"
+    assert ev3.payload["id"] == item["id"]
+    assert ev3.payload["archived"] is True

tests/projects/test_task_store.py

@@ -421,3 +421,95 @@ async def test_close_unclaimed_unchanged(store):
     again = await store.get_task(t["id"])
     assert again["status"] == "closed"
     assert again["closed_by"] == "reviewer"
+
+
+# ── checklist items ──────────────────────────────────────────────────────────
+
+
+@pytest.mark.asyncio
+async def test_create_checklist_item(store):
+    t = await store.create_task(project_id="p", title="Objective", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="First item", created_by="u")
+    assert item["id"].startswith("cki-")
+    assert item["text"] == "First item"
+    assert item["done"] is False
+    assert item["verified"] is False
+    assert item["reported"] is False
+    assert item["archived"] is False
+
+    items = await store.list_checklist_items(task_id=t["id"])
+    assert len(items) == 1
+    assert items[0]["id"] == item["id"]
+
+    all_items = await store.list_checklist_items(task_id=t["id"], include_archived=True)
+    assert len(all_items) == 1
+
+
+@pytest.mark.asyncio
+async def test_cannot_archive_unverified(store):
+    t = await store.create_task(project_id="p", title="Objective", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="Unverified item", created_by="u")
+    with pytest.raises(ValueError, match="item cannot be archived: not verified"):
+        await store.archive_checklist_item(item_id=item["id"], reported_by="u")
+
+
+@pytest.mark.asyncio
+async def test_cannot_archive_unreported(store):
+    t = await store.create_task(project_id="p", title="Objective", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="Unreported item", created_by="u")
+    await store.update_checklist_item(item_id=item["id"], verified=True)
+    with pytest.raises(ValueError, match="item cannot be archived: not reported"):
+        await store.archive_checklist_item(item_id=item["id"], reported_by="u")
+
+
+@pytest.mark.asyncio
+async def test_can_archive_after_verification_and_report(store):
+    """Archive a checklist item after verification and report."""
+    t = await store.create_task(project_id="p", title="Objective", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="Complete item", created_by="u")
+    await store.update_checklist_item(item_id=item["id"], verified=True, reported=True)
+
+    archived = await store.archive_checklist_item(item_id=item["id"], reported_by="u")
+    assert archived["archived"] is True
+    all_items = await store.list_checklist_items(task_id=t["id"], include_archived=True)
+    assert any(i["id"] == item["id"] for i in all_items)
+
+
+@pytest.mark.asyncio
+async def test_survives_agent_restart(store):
+    """Checklist items persist in the DB across store reinitialization.
+
+    Verifies that the OS (database) holds the checklist, not the agent's
+    temporary memory - items created in one store session are visible in
+    a fresh session.
+    """
+    t = await store.create_task(project_id="p", title="Objective", created_by="u")
+    item = await store.create_checklist_item(task_id=t["id"], text="Persistent item", created_by="u")
+    items = await store.list_checklist_items(task_id=t["id"])
+    assert len(items) == 1
+    assert items[0]["id"] == item["id"]
+    assert items[0]["text"] == "Persistent item"
+    assert items[0]["archived"] is False
+    all_items = await store.list_checklist_items(task_id=t["id"], include_archived=True)
+    assert len(all_items) == 1
+
+
+@pytest.mark.asyncio
+async def test_update_checklist_item_returns_none_when_missing(store):
+    """update_checklist_item returns None for an unknown item (no fields to set
+    would still hit get_checklist_item; with no-op path it must not crash)."""
+    assert await store.update_checklist_item("cki-missing", done=True) is None
+
+
+@pytest.mark.asyncio
+async def test_archive_missing_item_raises_value_error(store):
+    """RED proof for fix #2 (None-safety): archiving a non-existent item must
+    raise a clean ValueError naming the item, not a TypeError from indexing
+    ``None``.
+
+    On the pre-fix code ``get_checklist_item`` returns ``None`` and the next
+    ``item["verified"]`` raises ``TypeError``; ``pytest.raises(ValueError)``
+    then fails the test. Post-fix it raises ``ValueError`` cleanly.
+    """
+    with pytest.raises(ValueError, match="checklist item not found: cki-missing"):
+        await store.archive_checklist_item(item_id="cki-missing", reported_by="u")

tests/test_routes_task_checklist.py

@@ -0,0 +1,191 @@
+"""Route-level tests for the task checklist endpoints.
+
+The PR that added `POST`/`GET
+/api/projects/{project_id}/tasks/{task_id}/checklist-items` shipped store-level
+tests only, so the ROUTE surface (scope split, existence-hiding 404, request
+shape, archive filtering) was unverified. These pin it.
+
+The scope split is the security-relevant part and is asserted in the REFUSING
+direction: `project_tasks` is documented and tested as read + lifecycle +
+comments, so it must NOT be able to author a checklist item. Authoring needs the
+narrower `project_tasks_create`, the same grant task creation uses.
+"""
+from __future__ import annotations
+
+from types import SimpleNamespace
+
+import pytest
+import pytest_asyncio
+from httpx import ASGITransport, AsyncClient
+
+from tinyagentos.agent_registry_store import mint_registry_token
+
+
+@pytest_asyncio.fixture
+async def ctx(client):
+    app = client._transport.app
+    for attr in ("agent_registry", "agent_grants"):
+        store = getattr(app.state, attr)
+        if store._db is None:
+            await store.init()
+    uid = app.state.auth.find_user("admin")["id"]
+    yield SimpleNamespace(client=client, app=app, uid=uid)
+    for attr in ("agent_registry", "agent_grants"):
+        store = getattr(app.state, attr)
+        if store._db is not None:
+            await store.close()
+
+
+def _bare(app):
+    """Cookieless client so requests carry only the Bearer header."""
+    return AsyncClient(transport=ASGITransport(app=app), base_url="http://test")
+
+
+def _hdr(token):
+    return {"Authorization": f"Bearer {token}"}
+
+
+async def _new_project(ctx, slug):
+    resp = await ctx.client.post("/api/projects", json={"name": slug, "slug": slug})
+    assert resp.status_code == 200, resp.text
+    return resp.json()["id"]
+
+
+async def _new_task(ctx, pid, title="T"):
+    resp = await ctx.client.post(f"/api/projects/{pid}/tasks", json={"title": title})
+    assert resp.status_code == 200, resp.text
+    return resp.json()["id"]
+
+
+async def _mint_agent(ctx, project_id, scopes, handle="@grok"):
+    registry = ctx.app.state.agent_registry
+    grants = ctx.app.state.agent_grants
+    priv, _pub = ctx.app.state.agent_registry_keypair
+    rec = await registry.register(
+        framework="grok",
+        display_name="Grok",
+        origin="external-selfjoin",
+        handle=handle,
+    )
+    cid = rec["canonical_id"]
+    await registry.set_status(cid, "active")
+    for scope in scopes:
+        await grants.add_grant(cid, scope, project_id=project_id)
+    token = mint_registry_token(
+        cid, priv, user_id="u", framework="grok", project_id=project_id
+    )
+    return cid, token
+
+
+def _url(pid, tid):
+    return f"/api/projects/{pid}/tasks/{tid}/checklist-items"
+
+
+@pytest.mark.asyncio
+class TestRequestShape:
+    async def test_create_takes_a_json_body(self, ctx):
+        """The item text arrives in a JSON body, matching POST
+        /api/projects/{id}/tasks beside it, not as a query parameter."""
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        resp = await ctx.client.post(_url(pid, tid), json={"text": "step one"})
+        assert resp.status_code == 200, resp.text
+        assert resp.json()["text"] == "step one"
+
+    async def test_create_without_text_is_422(self, ctx):
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        resp = await ctx.client.post(_url(pid, tid), json={})
+        assert resp.status_code == 422
+
+    async def test_created_item_is_listed(self, ctx):
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        await ctx.client.post(_url(pid, tid), json={"text": "step one"})
+        resp = await ctx.client.get(_url(pid, tid))
+        assert resp.status_code == 200, resp.text
+        items = resp.json()
+        rows = items["items"] if isinstance(items, dict) else items
+        assert [r["text"] for r in rows] == ["step one"]
+
+
+@pytest.mark.asyncio
+class TestScopeSplit:
+    async def test_project_tasks_create_may_author(self, ctx):
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        _cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks_create",))
+        async with _bare(ctx.app) as bare:
+            resp = await bare.post(
+                _url(pid, tid), json={"text": "agent step"}, headers=_hdr(token)
+            )
+        assert resp.status_code == 200, resp.text
+        assert resp.json()["text"] == "agent step"
+
+    async def test_project_tasks_alone_may_NOT_author(self, ctx):
+        """The refusing direction: read scope must not author.
+
+        The agent holds only ``project_tasks`` (read) but POST needs
+        ``project_tasks_create``. The allowlist now lets the token through, so
+        the refusal comes from the handler's scope check, making this a real
+        scope-split assertion rather than an allowlist 401.
+        """
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        _cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks",))
+        async with _bare(ctx.app) as bare:
+            resp = await bare.post(
+                _url(pid, tid), json={"text": "nope"}, headers=_hdr(token)
+            )
+        assert resp.status_code == 403, resp.text
+
+    async def test_project_tasks_may_read(self, ctx):
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        await ctx.client.post(_url(pid, tid), json={"text": "step one"})
+        _cid, token = await _mint_agent(ctx, pid, scopes=("project_tasks",))
+        async with _bare(ctx.app) as bare:
+            resp = await bare.get(_url(pid, tid), headers=_hdr(token))
+        assert resp.status_code == 200, resp.text
+
+
+@pytest.mark.asyncio
+class TestCrossProjectIsolation:
+    async def test_task_from_another_project_is_404(self, ctx):
+        """A task id that exists but belongs to a different project is
+        existence-hiding 404, not 403."""
+        pid_a = await _new_project(ctx, "alpha")
+        pid_b = await _new_project(ctx, "beta")
+        tid_b = await _new_task(ctx, pid_b, title="in beta")
+        resp = await ctx.client.post(
+            _url(pid_a, tid_b), json={"text": "leak"}
+        )
+        assert resp.status_code == 404, resp.text
+        resp = await ctx.client.get(_url(pid_a, tid_b))
+        assert resp.status_code == 404, resp.text
+
+
+@pytest.mark.asyncio
+class TestArchiveFiltering:
+    async def test_archived_items_hidden_unless_requested(self, ctx):
+        pid = await _new_project(ctx, "alpha")
+        tid = await _new_task(ctx, pid)
+        created = await ctx.client.post(_url(pid, tid), json={"text": "done step"})
+        item_id = created.json()["id"]
+        await ctx.client.post(_url(pid, tid), json={"text": "live step"})
+
+        store = ctx.app.state.project_task_store
+        # archive_checklist_item refuses unless the item is verified AND
+        # reported, so satisfy that first rather than asserting on a refusal.
+        await store.update_checklist_item(item_id, verified=True, reported=True)
+        await store.archive_checklist_item(item_id, reported_by=ctx.uid)
+
+        resp = await ctx.client.get(_url(pid, tid))
+        rows = resp.json()
+        rows = rows["items"] if isinstance(rows, dict) else rows
+        assert [r["text"] for r in rows] == ["live step"]
+
+        resp = await ctx.client.get(_url(pid, tid), params={"include_archived": "true"})
+        rows = resp.json()
+        rows = rows["items"] if isinstance(rows, dict) else rows
+        assert {r["text"] for r in rows} == {"done step", "live step"}