From 46101e17843489a6903fb3971ed7a02a97a3b98b Mon Sep 17 00:00:00 2001 From: Alex Soffronow Pagonidis <237136924+alex-clickhouse@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:16:03 +0000 Subject: [PATCH] Add comments on tasks, with the person or agent that wrote each one People and the agent can now add comments to a task. Each comment records who wrote it and on whose behalf, like the other attributed writes: a person through the UI or REST is both, and the agent is the system actor writing for the cause of its session. The UI shows "Nerve for " for an agent comment. Storage: migration v061 adds the table task_comments and the column tasks.comment_count. Comments are not in the task's markdown file, because a file cannot hold a trustworthy author and an agent that rewrites the file would rewrite the comments too. Adding or deleting a comment also updates comment_count, updated_at and revision in the same transaction. Thus the count is on every task row, the task_updated broadcast refreshes open task views, and an agent that read the task before the comment gets a revision conflict on a guarded write. Agent: the new tool task_comment adds a comment. task_read shows the newest 50 comments in a marked block before the revision line, and task_write strips that block, so a read-edit-write round trip does not copy comments into the file. task_read indents each file line that starts with "` block, with the time, +the author and the text of each. `task_write` strips the block, so a +read-edit-write round trip does not copy comments into the file. In the +`task_read` output, a file line that starts with `[ \t]*(?:\n|\Z)", re.MULTILINE) +# task_read shows the task's comments between these two lines, and task_write +# strips the block for the same reason as the revision trailer. Each comment +# line is indented, and so is each file line that starts with ``" +_COMMENTS_END = "" +_COMMENTS_BLOCK_RE = re.compile( + rf"^{re.escape(_COMMENTS_START)}\n.*?^{re.escape(_COMMENTS_END)}[ \t]*(?:\n|\Z)", + re.MULTILINE | re.DOTALL, +) + +# How many of the newest comments task_read shows. +_READ_COMMENT_LIMIT = 50 + +# The longest comment body, in characters. +MAX_COMMENT_LENGTH = 10_000 + # Process-wide read-before-write guard. Tracks task IDs that have been # read (via task_read) or created (via task_create) in this process @@ -628,14 +648,21 @@ async def task_read_handler(ctx: ToolContext, args: dict) -> ToolResult: # Guarding here would refuse a read with a "Cannot write" message # the caller has no way to act on. if file_path.exists(): - content = await asyncio.to_thread( + content = _FILE_MARKER_LINE_RE.sub(" ", await asyncio.to_thread( file_path.read_text, encoding="utf-8", - ) + )) _tasks_read.add(task_id) + # Read after the row: a comment added in between shows here + # with the older revision, so a guarded write is refused and + # the agent reads again. The other order could hide it. + comments = ( + await _format_comments(ctx, task) + if task.get("comment_count") else "" + ) # The adapters drop ``structured``, so the text carries it too. sep = "" if content.endswith("\n") else "\n" return ToolResult.text( - f"{content}{sep}", structured={"revision": task["revision"]}, ) @@ -643,10 +670,83 @@ async def task_read_handler(ctx: ToolContext, args: dict) -> ToolResult: return ToolResult.text(f"Task file not found for: {task_id}") +async def _format_comments(ctx: ToolContext, task: dict) -> str: + """The comments block that task_read adds after the file content.""" + comments = await ctx.db.list_task_comments(task["id"], limit=_READ_COMMENT_LIMIT) + if not comments: + return "" + ids = {c[key] for c in comments for key in ("actor_id", "caused_by_actor_id")} - {None} + refs = {actor_id: await ctx.db.get_actor_ref(actor_id) for actor_id in ids} + + lines = [_COMMENTS_START, f"## Comments ({task['comment_count']})"] + older = task["comment_count"] - len(comments) + if older > 0: + lines.append(f"({older} older comments are not shown.)") + for comment in comments: + # "Alice", or "Nerve (agent) for Alice" when the agent wrote it in a + # session that Alice started. The web UI uses the same rule. + author = _actor_name(refs, comment["actor_id"]) + cause = refs.get(comment["caused_by_actor_id"]) + if cause and cause["id"] != comment["actor_id"] and cause["kind"] != "system": + author += f" for {_actor_name(refs, cause['id'])}" + when = datetime.fromisoformat(comment["created_at"]).strftime("%Y-%m-%d %H:%M UTC") + lines.append(f"- {when}, {author}:") + lines.extend(f" {line}" for line in comment["body"].splitlines()) + lines.append(_COMMENTS_END) + return "\n".join(lines) + "\n" + + +def _actor_name(refs: dict[str, dict | None], actor_id: str | None) -> str: + """A display name for an ``actor_refs`` ID, with the web UI's fallbacks.""" + if actor_id is None: + return "unknown" + ref = refs.get(actor_id) + if ref is None: + return actor_id[:8] + name = (ref["display_name"] or "").strip() or (ref.get("username") or "").strip() + if ref["kind"] == "system": + return f"{name or 'Nerve'} (agent)" + return name or actor_id[:8] + + +async def task_comment_handler(ctx: ToolContext, args: dict) -> ToolResult: + """Add a comment to a task as the caller (``ctx.attribution``). + + ``structured["reason"]`` names a refusal, so the REST route can choose a + status code without parsing the message. + """ + task_id = args["task_id"] + body = (args.get("body") or "").strip() + if not body: + return ToolResult.text( + "Cannot add an empty comment.", is_error=True, structured={"reason": "empty"}, + ) + if len(body) > MAX_COMMENT_LENGTH: + return ToolResult.text( + f"A comment can have at most {MAX_COMMENT_LENGTH} characters; " + f"this one has {len(body)}.", + is_error=True, structured={"reason": "too_long"}, + ) + if not ctx.db: + return ToolResult.text("Database not available.", is_error=True) + comment = await ctx.db.add_task_comment(task_id, body, attribution=ctx.attribution) + if comment is None: + return ToolResult.text( + f"Task not found: {task_id}", is_error=True, structured={"reason": "not_found"}, + ) + await _emit_task_event(ctx, task_id, "commented") + return ToolResult.text( + f"Comment {comment['id']} added to task {task_id}.", + structured={"comment": comment}, + ) + + @_one_change_per_task async def task_write_handler(ctx: ToolContext, args: dict) -> ToolResult: task_id = args["task_id"] - new_content = _REVISION_TRAILER_RE.sub("", args.get("content", "")) + new_content = _REVISION_TRAILER_RE.sub( + "", _COMMENTS_BLOCK_RE.sub("", args.get("content", "")), + ) if task_id not in _tasks_read: return ToolResult.text( @@ -969,6 +1069,18 @@ async def task_status_create_handler(ctx: ToolContext, args: dict) -> ToolResult handler=task_done_handler, ) +TASK_COMMENT_SPEC = ToolSpec( + name="task_comment", + description=( + "Add a comment to a task. The comment shows in the task's comment list " + "in the web UI, signed as you, and task_read shows it to agents. Use it " + "for discussion with people about the task; use task_update's note for " + "a progress line in the task file." + ), + input_schema=TASK_COMMENT_SCHEMA, + handler=task_comment_handler, +) + TASK_STATUS_LIST_SPEC = ToolSpec( name="task_status_list", description="List the configured task statuses with their colors and descriptions. Use this to discover valid status values for task_create/task_update.", @@ -999,6 +1111,7 @@ async def task_status_create_handler(ctx: ToolContext, args: dict) -> ToolResult TASK_READ_SPEC, TASK_WRITE_SPEC, TASK_DONE_SPEC, + TASK_COMMENT_SPEC, TASK_STATUS_LIST_SPEC, TASK_STATUS_CREATE_SPEC, ] diff --git a/nerve/agent/tools/schemas.py b/nerve/agent/tools/schemas.py index 8fcef3767..422054c53 100644 --- a/nerve/agent/tools/schemas.py +++ b/nerve/agent/tools/schemas.py @@ -176,6 +176,18 @@ "required": ["task_id"], } +TASK_COMMENT_SCHEMA = { + "type": "object", + "properties": { + "task_id": {"type": "string", "description": "Task ID"}, + "body": { + "type": "string", + "description": "Comment text, in markdown. At most 10000 characters.", + }, + }, + "required": ["task_id", "body"], +} + TASK_STATUS_LIST_SCHEMA = { "type": "object", "properties": {}, diff --git a/nerve/db/actor_columns.py b/nerve/db/actor_columns.py index 2d27a9fca..111435798 100644 --- a/nerve/db/actor_columns.py +++ b/nerve/db/actor_columns.py @@ -48,4 +48,6 @@ ("session_agents", "stopped_by_actor_id"), ("session_agents", "stopped_caused_by_actor_id"), ("onboarding", "actor_id"), + ("task_comments", "actor_id"), + ("task_comments", "caused_by_actor_id"), ) diff --git a/nerve/db/migrations/v061_task_comments.py b/nerve/db/migrations/v061_task_comments.py new file mode 100644 index 000000000..c1d3e7c6d --- /dev/null +++ b/nerve/db/migrations/v061_task_comments.py @@ -0,0 +1,63 @@ +"""V61: comments on tasks, with the person or agent that wrote each one. + +Adds the table ``task_comments``. One row is one comment: a markdown +``body``, the task it belongs to, and two nullable ``actor_refs`` IDs: + +- ``actor_id``: who wrote the comment. A person through the UI or REST, or + the system actor for an agent tool call. +- ``caused_by_actor_id``: on whose behalf it was written. For a person, the + person. For the agent, the cause of its session. + +Comments live here and not in the task's markdown file. A file can be edited +by anyone with access to the workspace, so it cannot hold a trustworthy +author, and an agent that rewrites the file would rewrite the comments too. + +Adds ``tasks.comment_count``. The comment store keeps it equal to the number +of rows for the task, in the same transaction as each insert and delete, so +every reader of a task row (the board, the list, the task broadcast) has the +count with no extra query. ``NOT NULL DEFAULT 0`` is correct for every +existing row, because no task has comments yet. + +The foreign keys follow v048: an ID that resolves to no actor, or to no +task, can never be stored. NULL is exempt from the check. ``CREATE ... IF NOT +EXISTS`` and the column check make a second run do nothing. +""" + +from __future__ import annotations + +import logging + +import aiosqlite + +logger = logging.getLogger(__name__) + +SQL = """ +CREATE TABLE IF NOT EXISTS task_comments ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + task_id TEXT NOT NULL REFERENCES tasks(id), + body TEXT NOT NULL, + actor_id TEXT REFERENCES actor_refs(id), + caused_by_actor_id TEXT REFERENCES actor_refs(id), + created_at TEXT NOT NULL +); + +-- The only read pattern: one task's comments in ``id`` order. An index on +-- ``task_id`` holds the rowid too, so it serves that order. +CREATE INDEX IF NOT EXISTS idx_task_comments_task + ON task_comments(task_id); +""" + + +async def _has_column(db: aiosqlite.Connection, table: str, column: str) -> bool: + async with db.execute(f"PRAGMA table_info({table})") as cursor: + rows = await cursor.fetchall() + return any(row[1] == column for row in rows) + + +async def up(db: aiosqlite.Connection) -> None: + await db.executescript(SQL) + if not await _has_column(db, "tasks", "comment_count"): + await db.execute( + "ALTER TABLE tasks ADD COLUMN comment_count INTEGER NOT NULL DEFAULT 0" + ) + logger.info("v061: task_comments ready, tasks.comment_count present") diff --git a/nerve/db/tasks.py b/nerve/db/tasks.py index 2e58fa379..e0bf47c2d 100644 --- a/nerve/db/tasks.py +++ b/nerve/db/tasks.py @@ -333,6 +333,83 @@ async def get_status_entry_times(self, task_ids: list[str]) -> dict[str, str]: if live.get(task_id) == to_status } + # ── Comments (task_comments) ───────────────────────────────────────── + # + # Adding or deleting a comment also changes the task row in the same + # transaction: ``comment_count`` so that each reader of the row has the + # count, ``updated_at`` so that the task broadcast tells an open task view + # and the board to show the change, and ``revision`` because ``task_read`` + # shows comments, so an agent that read the task before the change must + # read it again before a guarded write. + + async def add_task_comment( + self, task_id: str, body: str, *, attribution: Attribution = UNATTRIBUTED, + ) -> dict | None: + """Add a comment to a task and return its row. + + Returns ``None`` and writes nothing when the task does not exist. + """ + now = datetime.now(timezone.utc).isoformat() + async with self._atomic(): + cursor = await self.db.execute( + "UPDATE tasks SET comment_count = comment_count + 1, updated_at = ?, " + "revision = revision + 1 WHERE id = ?", + (now, task_id), + ) + found = (cursor.rowcount or 0) == 1 + await cursor.close() + if not found: + return None + cursor = await self.db.execute( + "INSERT INTO task_comments " + "(task_id, body, actor_id, caused_by_actor_id, created_at) " + "VALUES (?, ?, ?, ?, ?)", + (task_id, body, attribution.actor_id, attribution.caused_by_actor_id, now), + ) + comment_id = cursor.lastrowid + await cursor.close() + async with self.db.execute( + "SELECT * FROM task_comments WHERE id = ?", (comment_id,), + ) as cursor: + return dict(await cursor.fetchone()) + + async def get_task_comment(self, comment_id: int) -> dict | None: + async with self.db.execute( + "SELECT * FROM task_comments WHERE id = ?", (comment_id,), + ) as cursor: + row = await cursor.fetchone() + return dict(row) if row else None + + async def list_task_comments(self, task_id: str, limit: int = -1) -> list[dict]: + """The newest ``limit`` comments of a task, oldest first. + + The default ``-1`` is SQLite's "no limit": all comments. + """ + async with self.db.execute( + "SELECT * FROM (SELECT * FROM task_comments WHERE task_id = ? " + "ORDER BY id DESC LIMIT ?) ORDER BY id ASC", + (task_id, limit), + ) as cursor: + return [dict(row) async for row in cursor] + + async def delete_task_comment(self, task_id: str, comment_id: int) -> bool: + """Delete one comment of a task. ``False`` when the task has no such comment.""" + now = datetime.now(timezone.utc).isoformat() + async with self._atomic(): + cursor = await self.db.execute( + "DELETE FROM task_comments WHERE id = ? AND task_id = ?", + (comment_id, task_id), + ) + deleted = (cursor.rowcount or 0) == 1 + await cursor.close() + if deleted: + await self.db.execute( + "UPDATE tasks SET comment_count = comment_count - 1, updated_at = ?, " + "revision = revision + 1 WHERE id = ?", + (now, task_id), + ) + return deleted + # ── Board ordering (tasks.position) ────────────────────────────────── # # Lanes sort by ``position ASC``, so a *lower* rank is *higher* in the diff --git a/nerve/gateway/routes/tasks.py b/nerve/gateway/routes/tasks.py index f6780f380..2e0161546 100644 --- a/nerve/gateway/routes/tasks.py +++ b/nerve/gateway/routes/tasks.py @@ -71,6 +71,10 @@ class TaskMoveRequest(BaseModel): after_id: str | None = None +class TaskCommentRequest(BaseModel): + body: str + + class TaskStatusCreateRequest(BaseModel): name: str label: str = "" @@ -312,6 +316,56 @@ async def list_task_events(task_id: str, limit: int = 200): return {"events": await deps.db.list_task_events(task_id, limit=limit)} +@router.get("/api/tasks/{task_id}/comments") +async def list_task_comments(task_id: str): + """A task's comments, oldest first.""" + deps = get_deps() + if not await deps.db.get_task(task_id): + raise HTTPException(status_code=404, detail="Task not found") + return {"comments": await deps.db.list_task_comments(task_id)} + + +@router.post("/api/tasks/{task_id}/comments", status_code=201) +async def add_task_comment( + task_id: str, req: TaskCommentRequest, actor: Actor = Depends(require_auth), +): + """Add a comment as the person who sends the request.""" + result = await get_tool_registry().invoke( + "task_comment", + build_route_tool_context(_ROUTE_ORIGIN, actor=actor), + {"task_id": task_id, "body": req.body}, + ) + outcome = result.structured or {} + if result.is_error: + raise HTTPException( + status_code=404 if outcome.get("reason") == "not_found" else 422, + detail=result.text_content, + ) + return {"comment": outcome["comment"]} + + +@router.delete("/api/tasks/{task_id}/comments/{comment_id}") +async def delete_task_comment( + task_id: str, comment_id: int, actor: Actor = Depends(require_auth), +): + """Delete a comment. Only its author can. + + Accounts have no roles, so no person can delete the words of another. + """ + deps = get_deps() + comment = await deps.db.get_task_comment(comment_id) + if not comment or comment["task_id"] != task_id: + raise HTTPException(status_code=404, detail="Comment not found") + if comment["actor_id"] != actor.actor_id: + raise HTTPException( + status_code=403, detail="Only the author of a comment can delete it", + ) + if not await deps.db.delete_task_comment(task_id, comment_id): + raise HTTPException(status_code=404, detail="Comment not found") + await emit_task_event(await deps.db.get_task(task_id), "comment_deleted") + return {"id": comment_id, "deleted": True} + + @router.get("/api/tasks/{task_id}") async def get_task(task_id: str): deps = get_deps() diff --git a/nerve/templates/personal/TOOLS.md b/nerve/templates/personal/TOOLS.md index cc5a8b146..c86ca9f4e 100644 --- a/nerve/templates/personal/TOOLS.md +++ b/nerve/templates/personal/TOOLS.md @@ -20,6 +20,7 @@ Call tools as `mcp__nerve__`. The system prompt lists what is live; this i - `task_update`: status, deadline, tags, title, or a dated note - `task_read`/`task_write`: read or overwrite the markdown; `task_write` needs a prior `task_read` - `task_done`: moves the file to `done/` and the task's implementing plans to done +- `task_comment`: a comment on a task, signed as you; `task_read` shows the newest 50 - `task_status_list`/`task_status_create`: configured statuses; create only on request **Plans** (`pending` → `implementing` → `done`/`failed`, or `declined`/`superseded`; in the web UI, link a plan as `[plan-](/plans/plan-)`) diff --git a/nerve/templates/worker/TOOLS.md b/nerve/templates/worker/TOOLS.md index 3c98a5f08..e096f9343 100644 --- a/nerve/templates/worker/TOOLS.md +++ b/nerve/templates/worker/TOOLS.md @@ -20,6 +20,7 @@ Call tools as `mcp__nerve__`. The system prompt lists what is live; this i - `task_update`: status, deadline, tags, title, or a dated note - `task_read`/`task_write`: read or overwrite the markdown; `task_write` needs a prior `task_read` - `task_done`: moves the file to `done/` and the task's implementing plans to done +- `task_comment`: a comment on a task, signed as you; `task_read` shows the newest 50 - `task_status_list`/`task_status_create`: configured statuses; create only on request **Plans** (`pending` → `implementing` → `done`/`failed`, or `declined`/`superseded`; in the web UI, link a plan as `[plan-](/plans/plan-)`) diff --git a/tests/test_task_comments.py b/tests/test_task_comments.py new file mode 100644 index 000000000..f5fed82bd --- /dev/null +++ b/tests/test_task_comments.py @@ -0,0 +1,419 @@ +"""Task comments: storage, REST, the ``task_comment`` tool and ``task_read``. + +A comment records who wrote it (``actor_id``) and on whose behalf +(``caused_by_actor_id``), like every other attributed write. A person who +comments through REST is both. The agent is the system actor, writing for the +cause of its session. These tests drive the real routes, engine, tool registry +and database. +""" + +from __future__ import annotations + +import sqlite3 +from pathlib import Path + +import httpx +import pytest +import pytest_asyncio +from fastapi import FastAPI + +from nerve.agent.engine import AgentEngine +from nerve.config import NerveConfig, set_config +from nerve.db import Database +from nerve.db.migrations import v061_task_comments +from nerve.gateway.auth import create_session_token, hash_password, pin_jwt_secret +from nerve.gateway.routes import init_deps, register_all_routes +from nerve.identity import Attribution + +_SECRET = "test-secret-for-task-comments-pad-32-bytes" +_PASSWORD_HASH = hash_password("correct-horse-battery-staple") + + +class _Install: + def __init__(self, db: Database, engine: AgentEngine, config: NerveConfig): + self.db = db + self.engine = engine + self.config = config + self.people: dict[str, dict] = {} + + def actor_id(self, name: str) -> str: + return self.people[name]["actor_id"] + + def headers(self, name: str) -> dict: + token = create_session_token(_SECRET, self.people[name]["id"]) + return {"Authorization": f"Bearer {token}"} + + def client(self) -> httpx.AsyncClient: + app = FastAPI() + app.include_router(register_all_routes()) + return httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), base_url="http://nerve-test", + ) + + def task_file(self, task_id: str) -> Path: + return self.config.workspace / "memory" / "tasks" / "active" / f"{task_id}.md" + + async def new_task(self, client: httpx.AsyncClient, title: str = "Fix the login form") -> str: + response = await client.post( + "/api/tasks", headers=self.headers("alice"), json={"title": title}, + ) + assert response.status_code == 200, response.text + return response.json()["task"]["id"] + + async def agent_context(self, client: httpx.AsyncClient, person: str): + """The tool context of an agent turn in a session that ``person`` started.""" + response = await client.post("/api/sessions", headers=self.headers(person), json={}) + return self.engine._build_tool_context(response.json()["id"]) + + async def invoke(self, ctx, tool: str, args: dict): + return await self.engine.registry.invoke(tool, ctx, args) + + +@pytest_asyncio.fixture +async def install(tmp_path, open_identity_db, wire_identity_store): + config = NerveConfig.from_dict({ + "workspace": str(tmp_path / "ws"), + "codex": {"home_dir": str(tmp_path / "codex-home")}, + "auth": {"jwt_secret": _SECRET}, + }) + (config.workspace / "memory" / "tasks" / "active").mkdir(parents=True) + set_config(config) + pin_jwt_secret(_SECRET) + database, identity = await open_identity_db(tmp_path / "nerve.db") + wire_identity_store(database) + engine = AgentEngine(config, database) + init_deps(engine, database) + installed = _Install(database, engine, config) + await database.update_account_login( + identity.owner_account_id, username="alice", credential=_PASSWORD_HASH, + ) + installed.people["alice"] = { + "id": identity.owner_account_id, "actor_id": identity.owner_actor_id, + } + installed.people["bob"] = await database.create_managed_account( + username="bob", credential=_PASSWORD_HASH, display_name="Bob", + ) + try: + yield installed + finally: + await database.close() + set_config(NerveConfig()) + + +@pytest.fixture +def broadcasts(monkeypatch) -> list[tuple[str, str]]: + """Every task broadcast, as ``(task_id, event)``.""" + from nerve.agent.streaming import broadcaster + + sent: list[tuple[str, str]] = [] + + async def record(task, event): + sent.append((task["id"], event)) + + monkeypatch.setattr(broadcaster, "broadcast_task_event", record) + return sent + + +# --------------------------------------------------------------------------- # +# Schema and store # +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_existing_tasks_start_with_no_comments_and_the_migration_reruns(install): + async with install.client() as client: + task_id = await install.new_task(client) + assert (await install.db.get_task(task_id))["comment_count"] == 0 + await v061_task_comments.up(install.db.db) + assert await install.db.list_task_comments(task_id) == [] + + +@pytest.mark.asyncio +async def test_add_and_delete_change_the_count_updated_at_and_revision(install): + async with install.client() as client: + task_id = await install.new_task(client) + who = Attribution(install.actor_id("bob"), install.actor_id("bob")) + before = await install.db.get_task(task_id) + + first = await install.db.add_task_comment(task_id, "First", attribution=who) + second = await install.db.add_task_comment(task_id, "Second", attribution=who) + after_add = await install.db.get_task(task_id) + assert after_add["comment_count"] == 2 + assert after_add["revision"] == before["revision"] + 2 + assert after_add["updated_at"] > before["updated_at"] + assert [c["body"] for c in await install.db.list_task_comments(task_id)] == [ + "First", "Second", + ] + + assert await install.db.delete_task_comment(task_id, first["id"]) is True + after_delete = await install.db.get_task(task_id) + assert after_delete["comment_count"] == 1 + assert after_delete["revision"] == after_add["revision"] + 1 + assert [c["id"] for c in await install.db.list_task_comments(task_id)] == [second["id"]] + + # A second delete, or a delete through another task, changes nothing. + assert await install.db.delete_task_comment(task_id, first["id"]) is False + assert await install.db.delete_task_comment("no-such-task", second["id"]) is False + assert await install.db.get_task(task_id) == after_delete + + +@pytest.mark.asyncio +async def test_a_comment_on_a_missing_task_writes_nothing(install): + assert await install.db.add_task_comment("no-such-task", "Hello") is None + async with install.db.db.execute("SELECT COUNT(*) FROM task_comments") as cursor: + assert (await cursor.fetchone())[0] == 0 + + +@pytest.mark.asyncio +async def test_an_unknown_actor_id_is_refused_and_the_task_row_is_kept(install): + async with install.client() as client: + task_id = await install.new_task(client) + before = await install.db.get_task(task_id) + with pytest.raises(sqlite3.IntegrityError): + await install.db.add_task_comment( + task_id, "Hello", attribution=Attribution("not-an-actor", None), + ) + assert await install.db.get_task(task_id) == before + + +@pytest.mark.asyncio +async def test_a_limit_gives_the_newest_comments_oldest_first(install): + async with install.client() as client: + task_id = await install.new_task(client) + for n in range(5): + await install.db.add_task_comment(task_id, f"c{n}") + newest = await install.db.list_task_comments(task_id, limit=2) + assert [c["body"] for c in newest] == ["c3", "c4"] + + +# --------------------------------------------------------------------------- # +# REST # +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_a_rest_comment_is_attributed_to_the_person_who_sends_it(install, broadcasts): + async with install.client() as client: + task_id = await install.new_task(client) + response = await client.post( + f"/api/tasks/{task_id}/comments", + headers=install.headers("bob"), json={"body": " Looks good to me "}, + ) + assert response.status_code == 201, response.text + comment = response.json()["comment"] + listed = ( + await client.get(f"/api/tasks/{task_id}/comments", headers=install.headers("alice")) + ).json()["comments"] + + assert comment["body"] == "Looks good to me" + assert comment["actor_id"] == install.actor_id("bob") + assert comment["caused_by_actor_id"] == install.actor_id("bob") + assert listed == [comment] + assert (await install.db.get_task(task_id))["comment_count"] == 1 + assert (task_id, "commented") in broadcasts + + +@pytest.mark.asyncio +@pytest.mark.parametrize("body", ["", " ", "x" * 10_001]) +async def test_an_empty_or_too_long_comment_is_refused(install, body): + async with install.client() as client: + task_id = await install.new_task(client) + response = await client.post( + f"/api/tasks/{task_id}/comments", + headers=install.headers("bob"), json={"body": body}, + ) + assert response.status_code == 422 + assert await install.db.list_task_comments(task_id) == [] + + +@pytest.mark.asyncio +async def test_a_missing_task_is_404(install): + async with install.client() as client: + posted = await client.post( + "/api/tasks/no-such-task/comments", + headers=install.headers("bob"), json={"body": "Hello"}, + ) + listed = await client.get( + "/api/tasks/no-such-task/comments", headers=install.headers("bob"), + ) + assert posted.status_code == 404 + assert listed.status_code == 404 + + +@pytest.mark.asyncio +async def test_only_the_author_can_delete_a_comment(install, broadcasts): + async with install.client() as client: + task_id = await install.new_task(client) + other_task = await install.new_task(client, "Write the release notes") + comment = ( + await client.post( + f"/api/tasks/{task_id}/comments", + headers=install.headers("bob"), json={"body": "Mine"}, + ) + ).json()["comment"] + url = f"/api/tasks/{task_id}/comments/{comment['id']}" + + by_alice = await client.delete(url, headers=install.headers("alice")) + # Through another task the comment does not exist, for the author and + # for anybody else. + other_url = f"/api/tasks/{other_task}/comments/{comment['id']}" + through_other_task = [ + (await client.delete(other_url, headers=install.headers(name))).status_code + for name in ("bob", "alice") + ] + assert by_alice.status_code == 403 + assert through_other_task == [404, 404] + assert len(await install.db.list_task_comments(task_id)) == 1 + + by_bob = await client.delete(url, headers=install.headers("bob")) + again = await client.delete(url, headers=install.headers("bob")) + + assert by_bob.status_code == 200 + assert again.status_code == 404 + assert await install.db.list_task_comments(task_id) == [] + assert (await install.db.get_task(task_id))["comment_count"] == 0 + assert (task_id, "comment_deleted") in broadcasts + + +# --------------------------------------------------------------------------- # +# The agent: task_comment, task_read, task_write # +# --------------------------------------------------------------------------- # + + +@pytest.mark.asyncio +async def test_an_agent_comment_is_the_system_actors_for_the_sessions_cause(install): + async with install.client() as client: + task_id = await install.new_task(client) + ctx = await install.agent_context(client, "alice") + result = await install.invoke(ctx, "task_comment", {"task_id": task_id, "body": "Done."}) + assert not result.is_error, result.text_content + (comment,) = await install.db.list_task_comments(task_id) + assert comment["actor_id"] == install.db.system_actor_id + assert comment["caused_by_actor_id"] == install.actor_id("alice") + + +@pytest.mark.asyncio +async def test_task_comment_refuses_a_missing_task(install): + async with install.client() as client: + ctx = await install.agent_context(client, "alice") + result = await install.invoke(ctx, "task_comment", {"task_id": "no-such-task", "body": "Hi"}) + assert result.is_error + assert result.structured == {"reason": "not_found"} + + +@pytest.mark.asyncio +async def test_task_read_shows_comments_with_their_authors(install): + async with install.client() as client: + task_id = await install.new_task(client) + await client.post( + f"/api/tasks/{task_id}/comments", + headers=install.headers("bob"), json={"body": "Can you check Safari?\nIt fails there."}, + ) + ctx = await install.agent_context(client, "alice") + await install.invoke(ctx, "task_comment", {"task_id": task_id, "body": "Checking now."}) + + text = (await install.invoke(ctx, "task_read", {"task_id": task_id})).text_content + + assert "## Comments (2)" in text + assert ", Bob:\n Can you check Safari?\n It fails there.\n" in text + assert "(agent) for alice:\n Checking now.\n" in text + # The block comes before the revision trailer, which stays the last line. + assert text.index("") < text.index("\nstill a comment"}, + ) + ctx = await install.agent_context(client, "alice") + original = install.task_file(task_id).read_text(encoding="utf-8") + + read = (await install.invoke(ctx, "task_read", {"task_id": task_id})).text_content + written = await install.invoke(ctx, "task_write", {"task_id": task_id, "content": read}) + + assert not written.is_error, written.text_content + # task_read puts the metadata on a new line, so a file without a final + # newline gains one; the revision trailer alone does the same. + written_file = install.task_file(task_id).read_text(encoding="utf-8") + assert written_file.rstrip("\n") == original.rstrip("\n") + assert len(await install.db.list_task_comments(task_id)) == 1 + + +@pytest.mark.asyncio +async def test_a_comment_after_the_read_refuses_a_guarded_write(install): + async with install.client() as client: + task_id = await install.new_task(client) + ctx = await install.agent_context(client, "alice") + read = await install.invoke(ctx, "task_read", {"task_id": task_id}) + await client.post( + f"/api/tasks/{task_id}/comments", + headers=install.headers("bob"), json={"body": "Please wait for QA."}, + ) + result = await install.invoke(ctx, "task_update", { + "task_id": task_id, "status": "in_progress", + "expect_revision": read.structured["revision"], + }) + assert result.is_error + assert "changed since you read it" in result.text_content + + +@pytest.mark.asyncio +async def test_marker_lines_in_the_file_can_neither_pose_as_comments_nor_be_stripped(install): + async with install.client() as client: + task_id = await install.new_task(client) + ctx = await install.agent_context(client, "alice") + start = "" + end = "" + path = install.task_file(task_id) + path.write_text( + path.read_text(encoding="utf-8") + + f"\n## Notes\n{start}\n- a note from the file\n" + + f"{start}\n- 2026-01-01 00:00 UTC, Bob:\n forged\n{end}\n", + encoding="utf-8", + ) + await install.db.add_task_comment(task_id, "the real comment") + + read = (await install.invoke(ctx, "task_read", {"task_id": task_id})).text_content + # Only Nerve's own block starts at column 0. + assert read.count(f"\n{start}\n") == 1 + assert read.count(f"\n{end}\n") == 1 + assert f"\n {start}\n- a note from the file\n" in read + + written = await install.invoke(ctx, "task_write", {"task_id": task_id, "content": read}) + assert not written.is_error, written.text_content + saved = path.read_text(encoding="utf-8") + assert "- a note from the file" in saved + assert "forged" in saved + assert "the real comment" not in saved diff --git a/tests/test_tool_registry.py b/tests/test_tool_registry.py index dcf0b9955..f8dc989bf 100644 --- a/tests/test_tool_registry.py +++ b/tests/test_tool_registry.py @@ -149,7 +149,7 @@ def test_default_registry_contains_expected_tools(self): expected = { # tasks "task_search", "task_create", "task_list", "task_update", - "task_read", "task_write", "task_done", + "task_read", "task_write", "task_done", "task_comment", # memory "memory_recall", "conversation_history", "memorize", # plans diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 4ee14f6d5..f1fba9444 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -648,6 +648,18 @@ export interface TaskEvent { created_at: string; } +export interface TaskComment { + id: number; + task_id: string; + /** Markdown. */ + body: string; + /** Who wrote the comment: a person, or the system actor for the agent. */ + actor_id: string | null; + /** On whose behalf it was written. `null` when unknown. */ + caused_by_actor_id: string | null; + created_at: string; +} + export interface Task { id: string; title: string; @@ -660,6 +672,8 @@ export interface Task { position: number; created_at: string; updated_at: string; + /** Number of comments. Server-maintained. */ + comment_count?: number; content?: string; } @@ -1519,6 +1533,16 @@ export const api = { }), listTaskEvents: (id: string) => request<{ events: TaskEvent[] }>(`/tasks/${id}/events`), + listTaskComments: (id: string) => + request<{ comments: TaskComment[] }>(`/tasks/${id}/comments`), + addTaskComment: (id: string, body: string) => + request<{ comment: TaskComment }>(`/tasks/${id}/comments`, { + method: 'POST', body: JSON.stringify({ body }), + }), + deleteTaskComment: (id: string, commentId: number) => + request<{ id: number; deleted: boolean }>(`/tasks/${id}/comments/${commentId}`, { + method: 'DELETE', + }), getTask: (id: string) => request(`/tasks/${id}`), createTask: (data: { title: string; content?: string; deadline?: string; diff --git a/web/src/api/websocket.ts b/web/src/api/websocket.ts index 61d46f7c9..a043341c4 100644 --- a/web/src/api/websocket.ts +++ b/web/src/api/websocket.ts @@ -60,7 +60,11 @@ export type WSMessage = // Global (session_id is always null): a task row changed anywhere — the // API, another tab, or the agent in an unrelated session. Deliberately // NOT view-scoped; the board reflects all of them. - | { type: 'task_updated'; session_id: null; event: 'created' | 'updated' | 'moved' | 'done'; task: Task } + | { + type: 'task_updated'; session_id: null; + event: 'created' | 'updated' | 'moved' | 'done' | 'commented' | 'comment_deleted'; + task: Task; + } | { type: 'wakeup'; session_id: string } // `triggers` names the agents whose settling started the turn (`more` counts the rest). | { type: 'auto_turn'; session_id: string; triggers?: AutoTurnTrigger[]; more?: number } diff --git a/web/src/components/Tasks/Board/BoardCard.tsx b/web/src/components/Tasks/Board/BoardCard.tsx index 245c78c88..b96bc9fc0 100644 --- a/web/src/components/Tasks/Board/BoardCard.tsx +++ b/web/src/components/Tasks/Board/BoardCard.tsx @@ -4,7 +4,7 @@ import { CSS } from '@dnd-kit/utilities'; import type { Task } from '../../../api/client'; import { formatTimeAgo } from '../../../utils/dateGroups'; import { Badge } from '../../ui'; -import { Calendar, Clock, ExternalLink, Hourglass } from '../../ui/icons'; +import { Calendar, Clock, ExternalLink, Hourglass, MessageSquare } from '../../ui/icons'; /** * Deadline urgency, as a token class rather than a raw colour so it tracks @@ -121,6 +121,14 @@ function BoardCardInner({ task, statusSince, onOpen }: BoardCardProps) { {age.days}d )} + {!!task.comment_count && ( + + + )} {task.source && task.source !== 'manual' && ( {task.source} )} diff --git a/web/src/components/Tasks/TaskCard.tsx b/web/src/components/Tasks/TaskCard.tsx index ee4e3d792..cf4797be4 100644 --- a/web/src/components/Tasks/TaskCard.tsx +++ b/web/src/components/Tasks/TaskCard.tsx @@ -1,5 +1,5 @@ import { useNavigate } from 'react-router-dom'; -import { Calendar, Clock, ExternalLink } from '../ui/icons'; +import { Calendar, Clock, ExternalLink, MessageSquare } from '../ui/icons'; import type { Task } from '../../stores/taskStore'; import { formatTimeAgo } from '../../utils/dateGroups'; import { StatusBadge, StatusSelect } from './StatusControls'; @@ -46,6 +46,14 @@ export function TaskCard({ task, onStatusChange }: { {formatTimeAgo(task.updated_at)} )} + {!!task.comment_count && ( + + + )} {task.source && ( from {task.source} )} diff --git a/web/src/components/Tasks/TaskComments.test.tsx b/web/src/components/Tasks/TaskComments.test.tsx new file mode 100644 index 000000000..eb1f30eaf --- /dev/null +++ b/web/src/components/Tasks/TaskComments.test.tsx @@ -0,0 +1,171 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { ActorRef, TaskComment } from '../../api/client'; + +vi.mock('../../api/client', () => ({ + api: { + listTaskComments: vi.fn(), + addTaskComment: vi.fn(), + deleteTaskComment: vi.fn(), + listActors: vi.fn(), + }, + // The viewer comes from the auth store, which wires these on import. + setToken: vi.fn(), clearToken: vi.fn(), getToken: vi.fn(), + setUnauthorizedHandler: vi.fn(), +})); +vi.mock('../../stores/helpers/draftStorage', () => ({ clearAllDrafts: vi.fn() })); +vi.mock('../../stores/helpers/readStorage', () => ({ clearAllReads: vi.fn() })); + +const { api } = await import('../../api/client'); +const { useActorStore } = await import('../../stores/actorStore'); +const { useAuthStore } = await import('../../stores/authStore'); +const { TaskComments } = await import('./TaskComments'); + +const listTaskComments = vi.mocked(api.listTaskComments); +const addTaskComment = vi.mocked(api.addTaskComment); +const deleteTaskComment = vi.mocked(api.deleteTaskComment); +const listActors = vi.mocked(api.listActors); + +const ALICE = 'actor-alice'; +const BOB = 'actor-bob'; +const SYSTEM = 'actor-system'; + +function actorRef(id: string, overrides: Partial = {}): ActorRef { + return { id, kind: 'human', display_name: null, username: null, ...overrides }; +} + +let nextId = 1; +function comment(overrides: Partial): TaskComment { + return { + id: nextId++, + task_id: 't1', + body: 'A comment', + actor_id: ALICE, + caused_by_actor_id: ALICE, + created_at: '2026-01-01T00:00:00+00:00', + ...overrides, + }; +} + +const box = () => screen.getByRole('textbox', { name: 'New comment' }); +const commentButton = () => screen.getByRole('button', { name: 'Comment' }); + +beforeEach(() => { + vi.clearAllMocks(); + useActorStore.getState().reset(); + listActors.mockResolvedValue({ + actors: [ + actorRef(ALICE, { display_name: 'Alice' }), + actorRef(BOB, { display_name: 'Bob' }), + actorRef(SYSTEM, { kind: 'system' }), + ], + }); + useAuthStore.setState({ + viewer: actorRef(ALICE), + account: { id: 'account-alice', username: 'alice' }, + }); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('TaskComments', () => { + it('shows each comment with its author, and the agent for the person', async () => { + listTaskComments.mockResolvedValue({ + comments: [ + comment({ body: 'Can you check **Safari**?', actor_id: BOB, caused_by_actor_id: BOB }), + comment({ body: 'Checking now.', actor_id: SYSTEM, caused_by_actor_id: ALICE }), + ], + }); + render(); + + expect(await screen.findByText('Safari')).toBeInTheDocument(); + expect(screen.getByText('Checking now.')).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Comments (2)' })).toBeInTheDocument(); + await waitFor(() => { + const labels = [...document.querySelectorAll('[data-attribution="actor"]')] + .map((el) => el.textContent); + expect(labels).toEqual(['Bob', 'Nerve for Alice']); + }); + }); + + it('says so when there are no comments', async () => { + listTaskComments.mockResolvedValue({ comments: [] }); + render(); + expect(await screen.findByText('No comments yet.')).toBeInTheDocument(); + expect(commentButton()).toBeDisabled(); + }); + + it('adds a comment and clears the box', async () => { + listTaskComments.mockResolvedValue({ comments: [] }); + addTaskComment.mockResolvedValue({ comment: comment({ body: 'Looks good' }) }); + render(); + await screen.findByText('No comments yet.'); + + await userEvent.type(box(), ' Looks good '); + await userEvent.click(commentButton()); + + expect(addTaskComment).toHaveBeenCalledWith('t1', 'Looks good'); + expect(await screen.findByText('Looks good')).toBeInTheDocument(); + expect(box()).toHaveValue(''); + }); + + it('sends with Ctrl+Enter', async () => { + listTaskComments.mockResolvedValue({ comments: [] }); + addTaskComment.mockResolvedValue({ comment: comment({ body: 'Shipped' }) }); + render(); + await screen.findByText('No comments yet.'); + + await userEvent.type(box(), 'Shipped{Control>}{Enter}{/Control}'); + + expect(addTaskComment).toHaveBeenCalledWith('t1', 'Shipped'); + }); + + it('keeps the text and says so when the comment is not added', async () => { + listTaskComments.mockResolvedValue({ comments: [] }); + addTaskComment.mockRejectedValue(new Error('500')); + render(); + await screen.findByText('No comments yet.'); + + await userEvent.type(box(), 'Lost?'); + await userEvent.click(commentButton()); + + expect(await screen.findByRole('alert')).toHaveTextContent('not added'); + expect(box()).toHaveValue('Lost?'); + }); + + it('offers delete only on the viewer\'s own comments', async () => { + const mine = comment({ body: 'Mine', actor_id: ALICE }); + listTaskComments.mockResolvedValue({ + comments: [mine, comment({ body: 'Bob wrote this', actor_id: BOB, caused_by_actor_id: BOB })], + }); + deleteTaskComment.mockResolvedValue({ id: mine.id, deleted: true }); + vi.spyOn(window, 'confirm').mockReturnValue(true); + render(); + await screen.findByText('Mine'); + + const deletes = screen.getAllByRole('button', { name: 'Delete comment' }); + expect(deletes).toHaveLength(1); + await userEvent.click(deletes[0]); + + expect(deleteTaskComment).toHaveBeenCalledWith('t1', mine.id); + await waitFor(() => expect(screen.queryByText('Mine')).not.toBeInTheDocument()); + expect(screen.getByText('Bob wrote this')).toBeInTheDocument(); + }); + + it('reads the comments again when the refresh key changes', async () => { + listTaskComments.mockResolvedValueOnce({ comments: [] }); + const { rerender } = render(); + await screen.findByText('No comments yet.'); + + listTaskComments.mockResolvedValueOnce({ + comments: [comment({ body: 'From the agent', actor_id: SYSTEM, caused_by_actor_id: ALICE })], + }); + rerender(); + + expect(await screen.findByText('From the agent')).toBeInTheDocument(); + expect(listTaskComments).toHaveBeenCalledTimes(2); + }); +}); diff --git a/web/src/components/Tasks/TaskComments.tsx b/web/src/components/Tasks/TaskComments.tsx new file mode 100644 index 000000000..95f0ac0c0 --- /dev/null +++ b/web/src/components/Tasks/TaskComments.tsx @@ -0,0 +1,138 @@ +import { useCallback, useEffect, useState } from 'react'; +import { api, type TaskComment } from '../../api/client'; +import { useAuthStore } from '../../stores/authStore'; +import { formatTimeAgo } from '../../utils/dateGroups'; +import { ActorAttribution } from '../Chat/ActorLabel'; +import { MarkdownContent } from '../Chat/MarkdownContent'; +import { Button, IconButton, TextArea } from '../ui'; +import { Trash2 } from '../ui/icons'; + +/** The server refuses a longer body. */ +const MAX_COMMENT_LENGTH = 10_000; + +/** + * The comments of a task, oldest first, and the box to add one. + * + * `refreshKey` is the task's `updated_at`. Adding or deleting a comment + * changes it, and the task broadcast carries the new value, so a comment by + * another person or by the agent shows while the task is open. + * + * Only the author of a comment can delete it, so the delete button shows on + * the viewer's own comments only. + */ +export function TaskComments({ taskId, refreshKey }: { taskId: string; refreshKey?: string }) { + const viewerId = useAuthStore((s) => s.viewer?.id ?? null); + const [comments, setComments] = useState(null); + const [draft, setDraft] = useState(''); + const [posting, setPosting] = useState(false); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + api.listTaskComments(taskId) + .then(({ comments }) => { if (!cancelled) setComments(comments); }) + // Keep what is shown; an empty list only when nothing loaded yet. + .catch(() => { if (!cancelled) setComments((shown) => shown ?? []); }); + return () => { cancelled = true; }; + }, [taskId, refreshKey]); + + const submit = useCallback(async () => { + const body = draft.trim(); + if (!body || posting) return; + setPosting(true); + setError(null); + try { + const { comment } = await api.addTaskComment(taskId, body); + // The refetch that the broadcast starts can arrive first. + setComments((shown) => + shown?.some((c) => c.id === comment.id) ? shown : [...(shown ?? []), comment]); + setDraft(''); + } catch { + setError('The comment was not added. Try again.'); + } finally { + setPosting(false); + } + }, [draft, posting, taskId]); + + const remove = async (comment: TaskComment) => { + if (!window.confirm('Delete this comment?')) return; + setError(null); + try { + await api.deleteTaskComment(taskId, comment.id); + setComments((shown) => shown?.filter((c) => c.id !== comment.id) ?? null); + } catch { + setError('The comment was not deleted. Try again.'); + } + }; + + return ( +
+

+ Comments{comments?.length ? ` (${comments.length})` : ''} +

+ + {comments === null &&

Loading comments...

} + {comments?.length === 0 &&

No comments yet.

} + {!!comments?.length && ( +
    + {comments.map((comment) => ( +
  1. +
    + + {comment.actor_id ? ( + + ) : 'Unknown'} + + + {formatTimeAgo(comment.created_at)} + + {viewerId !== null && comment.actor_id === viewerId && ( + void remove(comment)} + > + + + )} +
    + +
    2. + ))} +
+ )} + +