diff --git a/changelog.d/tsk-gzwv3x-task-checklist-items.md b/changelog.d/tsk-gzwv3x-task-checklist-items.md new file mode 100644 index 000000000..fed1dbb50 --- /dev/null +++ b/changelog.d/tsk-gzwv3x-task-checklist-items.md @@ -0,0 +1,2 @@ +### Added +- Tasks gain an OS-owned checklist: `POST` and `GET /api/projects/{project_id}/tasks/{task_id}/checklist-items` let a project owner or a project-bound agent add and read checklist items, so an objective's steps are tracked by the OS instead of living in an agent's head where they can be silently dropped. The list/create routes are agent-Bearer-reachable (the middleware allowlist already matches them, per #2430) and are handler-gated by `project_tasks` (read, list) and `project_tasks_create` (write, create) in `_authorize_task_actor`: a `project_tasks` lane is refused on create (`403`) and allowed on read. Creating an item logs `checklist.item.created` to the project activity feed; archived items are hidden unless `include_archived=true`. Supersedes #2415. diff --git a/docs/agent-coordination.md b/docs/agent-coordination.md index adf7de63a..db95d4e8f 100644 --- a/docs/agent-coordination.md +++ b/docs/agent-coordination.md @@ -834,6 +834,31 @@ 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. ## Answering a select decision with free text (`other_value`) Route module `tinyagentos/routes/decisions.py`. Applies to BOTH answer paths: @@ -887,10 +912,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. diff --git a/tests/projects/test_task_store.py b/tests/projects/test_task_store.py index d08cdd6f4..4d509a455 100644 --- a/tests/projects/test_task_store.py +++ b/tests/projects/test_task_store.py @@ -371,3 +371,86 @@ async def test_get_task_context_project_falls_back_without_project_store(store): t = await store.create_task(project_id="p", title="T", created_by="u") ctx = await store.get_task_context(t["id"]) assert ctx["project"]["id"] == "p" + + +# --------------------------------------------------------------------------- +# Checklist item tests +# --------------------------------------------------------------------------- + +@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"] + + # Non-archived-only listing excludes nothing (all are new) + 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") + + # Attempting to archive without verification should fail + 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") + # Mark as verified but not reported + 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") + # Mark as verified and reported + 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 + # Item should still be listable (archived items are still visible) + 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 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") + + # Verify item is queryable + items = await store.list_checklist_items(task_id=t["id"]) + assert len(items) == 1 + assert items[0]["text"] == "Persistent item" + assert items[0]["archived"] is False + + # Item persists when listed again - simulates agent restart reading + # from the OS-owned store rather than relying on model memory + all_items = await store.list_checklist_items(task_id=t["id"], include_archived=True) + assert len(all_items) == 1 diff --git a/tests/test_routes_task_checklist.py b/tests/test_routes_task_checklist.py new file mode 100644 index 000000000..dba71e312 --- /dev/null +++ b/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"} diff --git a/tinyagentos/projects/ids.py b/tinyagentos/projects/ids.py index 30465b96e..ac15f0385 100644 --- a/tinyagentos/projects/ids.py +++ b/tinyagentos/projects/ids.py @@ -1,7 +1,7 @@ from __future__ import annotations import secrets -ID_PREFIXES = ("prj", "tsk", "cmt", "rel", "cve", "dec", "doc", "ent", "lst", "rev", "cs", "rtn", "elm", "note", "str") +ID_PREFIXES = ("prj", "tsk", "cmt", "rel", "cve", "dec", "doc", "ent", "lst", "rev", "cs", "rtn", "elm", "note", "str", "cki") _ALPHABET = "abcdefghijklmnopqrstuvwxyz234567" diff --git a/tinyagentos/projects/task_store.py b/tinyagentos/projects/task_store.py index cc9c6c277..c60a7cd1e 100644 --- a/tinyagentos/projects/task_store.py +++ b/tinyagentos/projects/task_store.py @@ -80,6 +80,20 @@ AND r.kind = 'blocks' AND bt.status NOT IN ('closed', 'cancelled') ); + +CREATE TABLE IF NOT EXISTS task_checklist_items ( + id TEXT PRIMARY KEY, + task_id TEXT NOT NULL REFERENCES project_tasks(id), + text TEXT NOT NULL DEFAULT '', + done INTEGER NOT NULL DEFAULT 0, + verified INTEGER NOT NULL DEFAULT 0, + reported INTEGER NOT NULL DEFAULT 0, + archived INTEGER NOT NULL DEFAULT 0, + created_at REAL NOT NULL, + updated_at REAL NOT NULL +); +CREATE INDEX IF NOT EXISTS idx_checklist_task + ON task_checklist_items(task_id, archived, done); """ _TASK_JSON_FIELDS = ("labels",) @@ -94,6 +108,16 @@ def _row_to_task(row, description) -> dict: return t +def _row_to_checklist_item(row, description) -> dict: + keys = [d[0] for d in description] + c = dict(zip(keys, row)) + c["done"] = bool(c.get("done", 0)) + c["verified"] = bool(c.get("verified", 0)) + c["reported"] = bool(c.get("reported", 0)) + c["archived"] = bool(c.get("archived", 0)) + return c + + # Sentinels for update_task's element_id: # _ELEMENT_UNCHANGED -> leave the task's element tag untouched (PATCH omitted) # _ELEMENT_CLEAR -> explicitly clear the tag to NULL ("none" sentinel) @@ -698,3 +722,107 @@ async def list_comments(self, task_id: str) -> list[dict]: rows = await cur.fetchall() keys = [d[0] for d in cur.description] return [dict(zip(keys, r)) for r in rows] + + # ------------------------------------------------------------------ checklist items + + async def create_checklist_item( + self, + task_id: str, + text: str, + created_by: str, + ) -> dict: + cid = new_id("cki") + now = time.time() + await self._db.execute( + """INSERT INTO task_checklist_items + (id, task_id, text, done, verified, reported, archived, created_at, updated_at) + VALUES (?, ?, ?, 0, 0, 0, 0, ?, ?)""", + (cid, task_id, text, now, now), + ) + await self._db.commit() + cur = await self._db.execute( + "SELECT * FROM task_checklist_items WHERE id = ?", (cid,) + ) + row = await cur.fetchone() + desc = cur.description + item = _row_to_checklist_item(row, desc) + await self._publish(task_id, "checklist.item.created", {"id": item["id"], "text": item["text"], "task_id": task_id}) + return item + + async def list_checklist_items( + self, + task_id: str, + *, + include_archived: bool = False, + ) -> list[dict]: + conds = ["task_id = ?"] + params: list = [task_id] + if not include_archived: + conds.append("archived = 0") + sql = f"SELECT * FROM task_checklist_items WHERE {' AND '.join(conds)} ORDER BY created_at ASC" + async with self._db.execute(sql, params) as cur: + rows = await cur.fetchall() + desc = cur.description + return [_row_to_checklist_item(r, desc) for r in rows] + + async def update_checklist_item( + self, + item_id: str, + *, + done: bool | None = None, + verified: bool | None = None, + reported: bool | None = None, + ) -> dict: + now = time.time() + candidates: list[tuple[str, object]] = [] + if done is not None: + candidates.append(("done", 1 if done else 0)) + if verified is not None: + candidates.append(("verified", 1 if verified else 0)) + if reported is not None: + candidates.append(("reported", 1 if reported else 0)) + if not candidates: + return await self.get_checklist_item(item_id) + sets: list[str] = [] + params: list = [] + for col, val in candidates: + sets.append(f"{col} = ?") + params.append(val) + sets.append("updated_at = ?") + params.append(now) + params.append(item_id) + await self._db.execute( + f"UPDATE task_checklist_items SET {', '.join(sets)} WHERE id = ?", params + ) + await self._db.commit() + return await self.get_checklist_item(item_id) + + async def archive_checklist_item(self, item_id: str, reported_by: 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. + """ + item = await self.get_checklist_item(item_id) + if item["verified"] != 1: + raise ValueError("item cannot be archived: not verified") + if item["reported"] != 1: + raise ValueError("item cannot be archived: not reported") + 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() + await self._publish(item["task_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: + async with self._db.execute( + "SELECT * FROM task_checklist_items WHERE id = ?", (item_id,) + ) as cur: + row = await cur.fetchone() + if row is None: + return None + desc = cur.description + return _row_to_checklist_item(row, desc) diff --git a/tinyagentos/routes/projects.py b/tinyagentos/routes/projects.py index 7f3b4bdb0..a9b6439e9 100644 --- a/tinyagentos/routes/projects.py +++ b/tinyagentos/routes/projects.py @@ -1247,6 +1247,79 @@ async def list_comments( return {"items": await store.list_comments(task_id)} +# --------------------------------------------------------------------------- +# Checklist routes +# --------------------------------------------------------------------------- + +class CreateChecklistItemIn(BaseModel): + text: str + + +@router.post("/api/projects/{project_id}/tasks/{task_id}/checklist-items") +async def create_checklist_item( + project_id: str, + task_id: str, + payload: CreateChecklistItemIn, + request: Request, +): + """Create a checklist item for a task. + + Authorized as session owner/admin or an agent holding + ``project_tasks_create`` on this project. + """ + pstore = request.app.state.project_store + auth = await _authorize_task_actor( + request, pstore, project_id, scope="project_tasks_create" + ) + if isinstance(auth, JSONResponse): + return auth + actor_id, _is_agent, _project = auth + store = request.app.state.project_task_store + guard = await _require_task_in_project(store, project_id, task_id) + if isinstance(guard, JSONResponse): + return guard + t = await store.get_task(task_id) + if t is None or t["project_id"] != project_id: + return JSONResponse({"error": "not found"}, status_code=404) + item = await store.create_checklist_item( + task_id=task_id, + text=payload.text, + created_by=actor_id, + ) + _beads_mark_dirty(request, project_id) + await pstore.log_activity( + project_id, actor_id, "checklist.item.created", {"task_id": task_id, "item_id": item["id"], "text": item["text"]} + ) + return item + + +@router.get("/api/projects/{project_id}/tasks/{task_id}/checklist-items") +async def list_checklist_items( + project_id: str, + task_id: str, + request: Request, + include_archived: bool = False, +): + """List checklist items for a task. + + By default shows only non-archived items. Set ``include_archived=true`` + to see all items including archived ones. + """ + pstore = request.app.state.project_store + auth = await _authorize_task_actor(request, pstore, project_id) + if isinstance(auth, JSONResponse): + return auth + store = request.app.state.project_task_store + guard = await _require_task_in_project(store, project_id, task_id) + if isinstance(guard, JSONResponse): + return guard + items = await store.list_checklist_items( + task_id=task_id, + include_archived=include_archived, + ) + return {"items": items} + + @router.get("/api/projects/{project_id}/tasks/{task_id}/relationships") async def list_relationships( project_id: str,