From c58726c920737a5cc2a5e3b4fc010c1089882b0c Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 11:14:51 +0200 Subject: [PATCH 01/21] feat(todo): add agent tools for todo lists, remove notes_set_done MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Create todo_tools.py with execute_todo_list_lists, execute_todo_add_item, and execute_todo_set_done. Register in skill_exec.py SKILL_IMPLEMENTATIONS and skills.py seed data. Remove deprecated execute_notes_set_done from notes_tools.py (set_done now lives on TodoStore). Add tests/todo/test_todo_tools.py (17 tests) covering list/add/set_done with owner-based access control. Remove notes_set_done tests from tests/notes/test_notes_tools.py (11 remaining tests pass). Task: 1923 C3 — Notes/Todo split agent tool surface. --- tests/notes/test_notes_tools.py | 117 ------------ tests/todo/test_todo_tools.py | 317 +++++++++++++++++++++++++++++++ tinyagentos/routes/skill_exec.py | 32 +++- tinyagentos/skills.py | 71 ++++++- tinyagentos/todo/notify.py | 32 ++++ tinyagentos/tools/notes_tools.py | 50 ----- tinyagentos/tools/todo_tools.py | 125 ++++++++++++ 7 files changed, 562 insertions(+), 182 deletions(-) create mode 100644 tests/todo/test_todo_tools.py create mode 100644 tinyagentos/todo/notify.py create mode 100644 tinyagentos/tools/todo_tools.py diff --git a/tests/notes/test_notes_tools.py b/tests/notes/test_notes_tools.py index a2cfc8059..9fe3372a5 100644 --- a/tests/notes/test_notes_tools.py +++ b/tests/notes/test_notes_tools.py @@ -11,10 +11,8 @@ from tinyagentos.tools.notes_tools import ( execute_notes_add_entry, execute_notes_list_shared_docs, - execute_notes_set_done, ) - # --------------------------------------------------------------------- helpers def _make_request(store, config=None, msg_store=None): @@ -201,118 +199,3 @@ async def test_list_shared_docs_excludes_internal_fields(store): keys = set(res["docs"][0].keys()) assert "owner_user_id" not in keys assert keys <= {"id", "kind", "title", "updated_at"} - - -# -------------------------------------------------------------- set_done tests - -@pytest.mark.asyncio -async def test_agent_member_can_mark_task_done(store): - doc = await store.create_doc("user-1", "list", "Build List") - await store.add_member(doc["id"], "agent", "atlas") - entry = await store.add_entry(doc["id"], "Ship the feature", author="user-1") - - req = _make_request(store) - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": doc["id"], "entry_id": entry["id"], "done": True}, - req, - ) - assert res.get("ok") is True - assert res["done"] is True - - entries = await store.list_entries(doc["id"]) - target = next(e for e in entries if e["id"] == entry["id"]) - assert target["done"] is True - - # And it can be reopened. - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": doc["id"], "entry_id": entry["id"], "done": False}, - req, - ) - assert res.get("ok") is True - entries = await store.list_entries(doc["id"]) - target = next(e for e in entries if e["id"] == entry["id"]) - assert target["done"] is False - - -@pytest.mark.asyncio -async def test_viewer_agent_cannot_mark_done(store): - doc = await store.create_doc("user-1", "list", "Read Only") - await store.add_member(doc["id"], "agent", "atlas", permission="viewer") - entry = await store.add_entry(doc["id"], "A task", author="user-1") - - req = _make_request(store) - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": doc["id"], "entry_id": entry["id"], "done": True}, - req, - ) - assert "error" in res - assert "permission" in res["error"] - - entries = await store.list_entries(doc["id"]) - assert entries[0]["done"] is False - - -@pytest.mark.asyncio -async def test_non_member_agent_cannot_mark_done(store): - doc = await store.create_doc("user-1", "list", "Private") - entry = await store.add_entry(doc["id"], "A task", author="user-1") - - req = _make_request(store) - res = await execute_notes_set_done( - {"agent_name": "intruder", "doc_id": doc["id"], "entry_id": entry["id"], "done": True}, - req, - ) - assert "error" in res - assert "permission" in res["error"] - - -@pytest.mark.asyncio -async def test_set_done_rejects_entry_from_another_doc(store): - doc_a = await store.create_doc("user-1", "list", "List A") - await store.add_member(doc_a["id"], "agent", "atlas") - doc_b = await store.create_doc("user-1", "list", "List B") - foreign = await store.add_entry(doc_b["id"], "Not yours", author="user-1") - - req = _make_request(store) - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": doc_a["id"], "entry_id": foreign["id"], "done": True}, - req, - ) - assert "error" in res - assert "not found" in res["error"] - - entries = await store.list_entries(doc_b["id"]) - assert entries[0]["done"] is False - - -@pytest.mark.asyncio -async def test_set_done_on_archived_doc_rejected(store): - doc = await store.create_doc("user-1", "list", "Old List") - await store.add_member(doc["id"], "agent", "atlas") - entry = await store.add_entry(doc["id"], "A task", author="user-1") - await store.archive_doc(doc["id"]) - - req = _make_request(store) - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": doc["id"], "entry_id": entry["id"], "done": True}, - req, - ) - assert "error" in res - assert "archived" in res["error"] - - -@pytest.mark.asyncio -async def test_set_done_missing_or_bad_fields_returns_error(store): - req = _make_request(store) - - # missing done - res = await execute_notes_set_done({"agent_name": "atlas", "doc_id": "d", "entry_id": "e"}, req) - assert "error" in res - # non-boolean done - res = await execute_notes_set_done( - {"agent_name": "atlas", "doc_id": "d", "entry_id": "e", "done": "yes"}, req - ) - assert "error" in res - # missing entry_id - res = await execute_notes_set_done({"agent_name": "atlas", "doc_id": "d", "done": True}, req) - assert "error" in res diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py new file mode 100644 index 000000000..d3dcb5b49 --- /dev/null +++ b/tests/todo/test_todo_tools.py @@ -0,0 +1,317 @@ +"""Tests for the todo agent tools (todo_list_lists, todo_add_item, todo_set_done).""" + +from __future__ import annotations + +import types +from unittest.mock import MagicMock + +import pytest +import pytest_asyncio + +from tinyagentos.todo.todo_store import TodoStore +from tinyagentos.tools.todo_tools import ( + execute_todo_add_item, + execute_todo_list_lists, + execute_todo_set_done, +) + + +# --------------------------------------------------------------------- helpers + +def _make_request(store, config=None, msg_store=None): + state = types.SimpleNamespace( + todo_store=store, + config=config, + chat_messages=msg_store, + ) + app = types.SimpleNamespace(state=state) + return types.SimpleNamespace(app=app) + + +@pytest_asyncio.fixture +async def store(tmp_path): + s = TodoStore(tmp_path / "test_todo_tools.db") + await s.init() + yield s + await s.close() + + +# ------------------------------------------------------------------ list tests + +@pytest.mark.asyncio +async def test_list_returns_owned_lists(store): + doc = await store.create_list("user-1", "Shopping") + await store.create_list("user-2", "Other List") + + req = _make_request(store) + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-1"}, req + ) + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) + assert len(res["lists"]) == 1 + + +@pytest.mark.asyncio +async def test_list_excludes_other_users_lists(store): + await store.create_list("user-2", "Private") + + req = _make_request(store) + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-1"}, req + ) + assert res["lists"] == [] + + +@pytest.mark.asyncio +async def test_list_excludes_archived_lists(store): + doc = await store.create_list("user-1", "Old List") + await store.archive_list(doc["id"]) + + req = _make_request(store) + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-1"}, req + ) + assert res["lists"] == [] + + +@pytest.mark.asyncio +async def test_list_missing_agent_name_returns_error(store): + req = _make_request(store) + res = await execute_todo_list_lists({}, req) + assert "error" in res + + +@pytest.mark.asyncio +async def test_list_missing_owner_user_id_returns_error(store): + req = _make_request(store) + res = await execute_todo_list_lists({"agent_name": "atlas"}, req) + assert "error" in res + + +# ------------------------------------------------------------------- add tests + +@pytest.mark.asyncio +async def test_owner_can_add_item(store): + doc = await store.create_list("user-1", "Shopping") + + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "Buy milk", + "owner_user_id": "user-1"}, + req, + ) + assert res.get("ok") is True + assert "item_id" in res + + items = await store.list_items(doc["id"]) + assert any(i["text"] == "Buy milk" for i in items) + + +@pytest.mark.asyncio +async def test_non_owner_rejected(store): + doc = await store.create_list("user-1", "Private") + + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "Hacked", + "owner_user_id": "user-2"}, + req, + ) + assert "error" in res + assert "access" in res["error"] + + items = await store.list_items(doc["id"]) + assert items == [] + + +@pytest.mark.asyncio +async def test_add_item_attributed_to_agent(store): + doc = await store.create_list("user-1", "Tasks") + + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "Do the thing", + "owner_user_id": "user-1"}, + req, + ) + item = await store.get_item(res["item_id"]) + assert item["author"] == "atlas" + + +@pytest.mark.asyncio +async def test_add_item_notification_noop(store): + """Notification module is a no-op for now; just verify the add succeeds.""" + doc = await store.create_list("user-1", "Ideas") + + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "Interesting", + "owner_user_id": "user-1"}, + req, + ) + assert res.get("ok") is True + + +@pytest.mark.asyncio +async def test_add_item_missing_fields_returns_error(store): + req = _make_request(store) + + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": "list-x", "owner_user_id": "user-1"}, req + ) + assert "error" in res + + res = await execute_todo_add_item( + {"agent_name": "atlas", "text": "hi", "owner_user_id": "user-1"}, req + ) + assert "error" in res + + res = await execute_todo_add_item( + {"list_id": "list-x", "text": "hi", "owner_user_id": "user-1"}, req + ) + assert "error" in res + + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": "list-x", "text": "hi"}, req + ) + assert "error" in res + + +@pytest.mark.asyncio +async def test_add_item_archived_list_rejected(store): + doc = await store.create_list("user-1", "Old List") + await store.archive_list(doc["id"]) + + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "late entry", + "owner_user_id": "user-1"}, + req, + ) + assert "error" in res + assert "archived" in res["error"] + + +@pytest.mark.asyncio +async def test_add_item_nonexistent_list_returns_error(store): + req = _make_request(store) + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": "nonexistent", "text": "hi", + "owner_user_id": "user-1"}, + req, + ) + assert "error" in res + assert "not found" in res["error"] + + +# ------------------------------------------------------------- set_done tests + +@pytest.mark.asyncio +async def test_owner_can_mark_item_done(store): + doc = await store.create_list("user-1", "Build List") + item = await store.add_item(doc["id"], "Ship feature", author="user-1") + + req = _make_request(store) + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + "done": True, "owner_user_id": "user-1"}, + req, + ) + assert res.get("ok") is True + assert res["done"] is True + + updated = await store.get_item(item["id"]) + assert updated["done"] is True + + # And it can be reopened. + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + "done": False, "owner_user_id": "user-1"}, + req, + ) + assert res.get("ok") is True + updated = await store.get_item(item["id"]) + assert updated["done"] is False + + +@pytest.mark.asyncio +async def test_non_owner_cannot_mark_done(store): + doc = await store.create_list("user-1", "Read Only") + item = await store.add_item(doc["id"], "A task", author="user-1") + + req = _make_request(store) + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + "done": True, "owner_user_id": "user-2"}, + req, + ) + assert "error" in res + assert "access" in res["error"] + + updated = await store.get_item(item["id"]) + assert updated["done"] is False + + +@pytest.mark.asyncio +async def test_set_done_rejects_item_from_another_list(store): + doc_a = await store.create_list("user-1", "List A") + doc_b = await store.create_list("user-1", "List B") + foreign = await store.add_item(doc_b["id"], "Not yours", author="user-1") + + req = _make_request(store) + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc_a["id"], "item_id": foreign["id"], + "done": True, "owner_user_id": "user-1"}, + req, + ) + assert "error" in res + assert "not found" in res["error"] + + updated = await store.get_item(foreign["id"]) + assert updated["done"] is False + + +@pytest.mark.asyncio +async def test_set_done_archived_list_rejected(store): + doc = await store.create_list("user-1", "Old List") + item = await store.add_item(doc["id"], "A task", author="user-1") + await store.archive_list(doc["id"]) + + req = _make_request(store) + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + "done": True, "owner_user_id": "user-1"}, + req, + ) + assert "error" in res + assert "archived" in res["error"] + + +@pytest.mark.asyncio +async def test_set_done_missing_or_bad_fields_returns_error(store): + req = _make_request(store) + + # missing done + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": "d", "item_id": "i", + "owner_user_id": "user-1"}, req + ) + assert "error" in res + # non-boolean done + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": "d", "item_id": "i", + "done": "yes", "owner_user_id": "user-1"}, req + ) + assert "error" in res + # missing item_id + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": "d", "done": True, + "owner_user_id": "user-1"}, req + ) + assert "error" in res + # missing owner_user_id + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": "d", "item_id": "i", "done": True}, req + ) + assert "error" in res diff --git a/tinyagentos/routes/skill_exec.py b/tinyagentos/routes/skill_exec.py index 780868bde..e44662a31 100644 --- a/tinyagentos/routes/skill_exec.py +++ b/tinyagentos/routes/skill_exec.py @@ -427,12 +427,32 @@ async def _skill_notes_add_entry(args: dict, request: Request) -> dict: return {"error": str(exc)} -async def _skill_notes_set_done(args: dict, request: Request) -> dict: - """Mark a list task done/not-done on a shared doc the agent belongs to.""" +async def _skill_todo_list_lists(args: dict, request: Request) -> dict: + """List non-archived todo lists the calling agent has access to.""" try: - from tinyagentos.tools.notes_tools import execute_notes_set_done + from tinyagentos.tools.todo_tools import execute_todo_list_lists - return await execute_notes_set_done(args, request) + return await execute_todo_list_lists(args, request) + except Exception as exc: + return {"error": str(exc)} + + +async def _skill_todo_add_item(args: dict, request: Request) -> dict: + """Append an item to a todo list the calling agent has access to.""" + try: + from tinyagentos.tools.todo_tools import execute_todo_add_item + + return await execute_todo_add_item(args, request) + except Exception as exc: + return {"error": str(exc)} + + +async def _skill_todo_set_done(args: dict, request: Request) -> dict: + """Mark a todo item done/not-done on a list the agent has access to.""" + try: + from tinyagentos.tools.todo_tools import execute_todo_set_done + + return await execute_todo_set_done(args, request) except Exception as exc: return {"error": str(exc)} @@ -464,7 +484,9 @@ async def _skill_notes_set_done(args: dict, request: Request) -> dict: "export_storybook": _skill_export_storybook, "notes_list_shared_docs": _skill_notes_list_shared_docs, "notes_add_entry": _skill_notes_add_entry, - "notes_set_done": _skill_notes_set_done, + "todo_list_lists": _skill_todo_list_lists, + "todo_add_item": _skill_todo_add_item, + "todo_set_done": _skill_todo_set_done, } diff --git a/tinyagentos/skills.py b/tinyagentos/skills.py index d2667d8f2..2c77ddc7f 100644 --- a/tinyagentos/skills.py +++ b/tinyagentos/skills.py @@ -687,21 +687,72 @@ async def _seed_defaults(self): "install_target": "tinyagentos.tools.notes_tools", }, { - "id": "notes_set_done", - "name": "Set Notes Task Done", - "category": "notes", - "description": "Mark a task done or not done on a shared list the agent is a member of", + "id": "todo_list_lists", + "name": "List Todo Lists", + "category": "todo", + "description": "List the non-archived todo lists the agent has access to", + "tool_schema": { + "name": "todo_list_lists", + "description": "List the non-archived todo lists this agent has access to. Returns id, title, and updated_at for each list.", + "input_schema": { + "type": "object", + "properties": { + "owner_user_id": {"type": "string", "description": "The user whose todo lists to list."}, + }, + "required": ["owner_user_id"], + }, + }, + "frameworks": { + "smolagents": "adapter", "openclaw": "adapter", "pocketflow": "adapter", + "langroid": "adapter", "hermes": "adapter", "agent-zero": "adapter", + "openai-agents-sdk": "adapter", "generic": "adapter", + }, + "install_method": "builtin", + "install_target": "tinyagentos.tools.todo_tools", + }, + { + "id": "todo_add_item", + "name": "Add Todo Item", + "category": "todo", + "description": "Append a new item to a todo list the agent has access to", "tool_schema": { - "name": "notes_set_done", - "description": "Mark a task on a shared list done or not done. Use notes_list_shared_docs to find the doc_id and read the entry ids. The agent needs contributor or editor permission.", + "name": "todo_add_item", + "description": "Append a new item to a todo list this agent has access to. Use todo_list_lists first to get the list_id.", "input_schema": { "type": "object", "properties": { - "doc_id": {"type": "string", "description": "Id of the shared list (from notes_list_shared_docs)."}, - "entry_id": {"type": "string", "description": "Id of the task entry to mark."}, + "list_id": {"type": "string", "description": "Id of the todo list (from todo_list_lists)."}, + "text": {"type": "string", "description": "The item text to append."}, + "owner_user_id": {"type": "string", "description": "The user who owns the list."}, + }, + "required": ["list_id", "text", "owner_user_id"], + }, + }, + "frameworks": { + "smolagents": "adapter", "openclaw": "adapter", "pocketflow": "adapter", + "langroid": "adapter", "hermes": "adapter", "agent-zero": "adapter", + "openai-agents-sdk": "adapter", "generic": "adapter", + }, + "install_method": "builtin", + "install_target": "tinyagentos.tools.todo_tools", + }, + { + "id": "todo_set_done", + "name": "Set Todo Item Done", + "category": "todo", + "description": "Mark a todo item done or not done on a list the agent has access to", + "tool_schema": { + "name": "todo_set_done", + "description": "Mark a todo item done or not done. Use todo_list_lists to find the list_id and read the item ids. The agent needs access to the list (owner match).", + "input_schema": { + "type": "object", + "properties": { + "list_id": {"type": "string", "description": "Id of the todo list (from todo_list_lists)."}, + "item_id": {"type": "string", "description": "Id of the todo item to mark."}, "done": {"type": "boolean", "description": "True to mark done, false to reopen."}, + "owner_user_id": {"type": "string", "description": "The user who owns the list."}, }, - "required": ["doc_id", "entry_id", "done"], + "required": ["list_id", "item_id", "done", "owner_user_id"], }, }, "frameworks": { @@ -710,7 +761,7 @@ async def _seed_defaults(self): "openai-agents-sdk": "adapter", "generic": "adapter", }, "install_method": "builtin", - "install_target": "tinyagentos.tools.notes_tools", + "install_target": "tinyagentos.tools.todo_tools", }, ] diff --git a/tinyagentos/todo/notify.py b/tinyagentos/todo/notify.py new file mode 100644 index 000000000..62973f1b4 --- /dev/null +++ b/tinyagentos/todo/notify.py @@ -0,0 +1,32 @@ +"""Notification helpers for todo agent actions. + +When collaboration lands for TodoStore, this module will mirror +routes/notes.py's _trigger_agent_notifications pattern — iterating agent +members and sending messages to their channels. For now, since TodoStore is +owner-based without agent membership, notifications are a no-op placeholder. +""" + +from __future__ import annotations + +import logging + +from fastapi import Request + +logger = logging.getLogger(__name__) + + +async def _trigger_todo_agent_notifications( + request: Request, + doc: dict, + entry_text: str, + skip_agent: str | None = None, +) -> None: + """Placeholder: notify agent members about a new todo item. + + Currently a no-op because TodoStore does not yet have agent membership. + When collaboration is added to TodoStore (gh #1923 C1 follow-up), this + function will iterate agent members and send channel messages, skipping + the agent named in ``skip_agent``. + """ + # TODO(#1923): wire up when TodoStore gains agent membership / collaboration + pass diff --git a/tinyagentos/tools/notes_tools.py b/tinyagentos/tools/notes_tools.py index a68e3aa51..d3e0fb23b 100644 --- a/tinyagentos/tools/notes_tools.py +++ b/tinyagentos/tools/notes_tools.py @@ -77,53 +77,3 @@ async def execute_notes_add_entry(args: dict, request: Request) -> dict: except Exception as exc: return {"error": str(exc)} - -async def execute_notes_set_done(args: dict, request: Request) -> dict: - """Mark a list task done (or not done) on a shared doc the agent belongs to. - - Completes the Todo surface for agents: an agent told to work a shared list - can check tasks off as it finishes them. The agent must have 'contributor' - or 'editor' permission, the entry must belong to the named doc, and the doc - must not be archived. - """ - args = args or {} - agent_name = args.get("agent_name") - doc_id = args.get("doc_id") - entry_id = args.get("entry_id") - done = args.get("done") - - if not agent_name or not isinstance(agent_name, str): - return {"error": "notes_set_done requires an 'agent_name' string"} - if not doc_id or not isinstance(doc_id, str): - return {"error": "notes_set_done requires a 'doc_id' string"} - if not entry_id or not isinstance(entry_id, str): - return {"error": "notes_set_done requires an 'entry_id' string"} - if not isinstance(done, bool): - return {"error": "notes_set_done requires a boolean 'done'"} - - try: - store = request.app.state.shared_docs_store - - members = await store.agent_members(doc_id) - agent_member = next((m for m in members if m["agent"] == agent_name), None) - if agent_member is None: - return {"error": "agent does not have write permission on this doc"} - - perm = agent_member.get("permission", "contributor") - if perm not in ("contributor", "editor"): - return {"error": "agent does not have write permission on this doc"} - - doc = await store.get_doc(doc_id) - if doc is None: - return {"error": "doc not found"} - if doc.get("archived_at") is not None: - return {"error": "doc is archived"} - - # Confine the agent to entries of the doc it actually belongs to. - if not any(e.get("id") == entry_id for e in doc.get("entries", [])): - return {"error": "entry not found in this doc"} - - await store.set_entry_done(entry_id, done) - return {"ok": True, "entry_id": entry_id, "done": done} - except Exception as exc: - return {"error": str(exc)} diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py new file mode 100644 index 000000000..4771afde9 --- /dev/null +++ b/tinyagentos/tools/todo_tools.py @@ -0,0 +1,125 @@ +"""Agent-side tools for todo lists. + +Lets an agent list the todo lists it has access to, append items, and +toggle completion. The loop guard (skip_agent) prevents the writing agent +from being notified about its own write. +""" + +from __future__ import annotations + +import logging + +from fastapi import Request + +logger = logging.getLogger(__name__) + + +async def execute_todo_list_lists(args: dict, request: Request) -> dict: + """List non-archived todo lists the calling agent has access to.""" + args = args or {} + agent_name = args.get("agent_name") + if not agent_name or not isinstance(agent_name, str): + return {"error": "todo_list_lists requires an 'agent_name' string"} + + owner_user_id = args.get("owner_user_id") + if not owner_user_id or not isinstance(owner_user_id, str): + return {"error": "todo_list_lists requires an 'owner_user_id' string"} + + try: + store = request.app.state.todo_store + lists = await store.list_lists(owner_user_id) + return {"lists": lists} + except Exception as exc: + return {"error": str(exc)} + + +async def execute_todo_add_item(args: dict, request: Request) -> dict: + """Append an item to a todo list the calling agent has access to. + + The agent must have access to the list (owner match). + """ + args = args or {} + agent_name = args.get("agent_name") + list_id = args.get("list_id") + text = args.get("text") + owner_user_id = args.get("owner_user_id") + + if not agent_name or not isinstance(agent_name, str): + return {"error": "todo_add_item requires an 'agent_name' string"} + if not list_id or not isinstance(list_id, str): + return {"error": "todo_add_item requires a 'list_id' string"} + if not isinstance(text, str) or not text: + return {"error": "todo_add_item requires a 'text' string"} + if not owner_user_id or not isinstance(owner_user_id, str): + return {"error": "todo_add_item requires an 'owner_user_id' string"} + + try: + store = request.app.state.todo_store + + doc = await store.get_list(list_id) + if doc is None: + return {"error": "list not found"} + if doc.get("archived_at") is not None: + return {"error": "list is archived"} + if doc.get("owner_user_id") != owner_user_id: + return {"error": "agent does not have access to this list"} + + item = await store.add_item(list_id, text, author=agent_name) + + try: + from tinyagentos.todo.notify import _trigger_todo_agent_notifications + + await _trigger_todo_agent_notifications( + request, doc, text, skip_agent=agent_name + ) + except Exception as exc: # noqa: BLE001 + logger.warning("todo_add_item: agent trigger failed: %s", exc) + + return {"ok": True, "item_id": item["id"]} + except Exception as exc: + return {"error": str(exc)} + + +async def execute_todo_set_done(args: dict, request: Request) -> dict: + """Mark a todo item done (or not done) on a list the agent has access to. + + The agent must have access to the list (owner match) and the item must + belong to the named list. + """ + args = args or {} + agent_name = args.get("agent_name") + list_id = args.get("list_id") + item_id = args.get("item_id") + done = args.get("done") + owner_user_id = args.get("owner_user_id") + + if not agent_name or not isinstance(agent_name, str): + return {"error": "todo_set_done requires an 'agent_name' string"} + if not list_id or not isinstance(list_id, str): + return {"error": "todo_set_done requires a 'list_id' string"} + if not item_id or not isinstance(item_id, str): + return {"error": "todo_set_done requires an 'item_id' string"} + if not isinstance(done, bool): + return {"error": "todo_set_done requires a boolean 'done'"} + if not owner_user_id or not isinstance(owner_user_id, str): + return {"error": "todo_set_done requires an 'owner_user_id' string"} + + try: + store = request.app.state.todo_store + + doc = await store.get_list(list_id) + if doc is None: + return {"error": "list not found"} + if doc.get("archived_at") is not None: + return {"error": "list is archived"} + if doc.get("owner_user_id") != owner_user_id: + return {"error": "agent does not have access to this list"} + + # Confine the agent to items of the list it actually belongs to. + if not any(i.get("id") == item_id for i in doc.get("items", [])): + return {"error": "item not found in this list"} + + await store.patch_item(item_id, done=done) + return {"ok": True, "item_id": item_id, "done": done} + except Exception as exc: + return {"error": str(exc)} From 9816e42e27c731426a7f22adf2b660f59999051b Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 11:23:54 +0200 Subject: [PATCH 02/21] fix(todo): strip internal fields from list_lists, document owner-auth model Address Kilo bot findings: - Filter owner_user_id/archived_at/created_at from list_lists response - Add docstring notes explaining owner-based authorization model and the planned agent_name-to-owner binding for when TodoStore gains membership No CRITICAL findings. 17/17 todo tools, 11/11 notes tools, 18/18 todo routes pass. --- tinyagentos/tools/todo_tools.py | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 4771afde9..2c9239d93 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -15,7 +15,13 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: - """List non-archived todo lists the calling agent has access to.""" + """List non-archived todo lists the calling agent has access to. + + Note: authorization is owner-based — the caller supplies owner_user_id and the + store returns lists owned by that user. agent_name is used for attribution and + the notification skip-guard only. When TodoStore gains agent membership (#1923 + follow-up), the agent_name-to-owner binding will move here. + """ args = args or {} agent_name = args.get("agent_name") if not agent_name or not isinstance(agent_name, str): @@ -28,7 +34,12 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: try: store = request.app.state.todo_store lists = await store.list_lists(owner_user_id) - return {"lists": lists} + # Strip internal fields the agent does not need. + slim = [ + {k: v for k, v in doc.items() if k not in ("owner_user_id", "archived_at", "created_at")} + for doc in lists + ] + return {"lists": slim} except Exception as exc: return {"error": str(exc)} @@ -36,7 +47,10 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: async def execute_todo_add_item(args: dict, request: Request) -> dict: """Append an item to a todo list the calling agent has access to. - The agent must have access to the list (owner match). + Authorization is owner-based: the caller supplies owner_user_id and the + store verifies it matches the list's owner. agent_name is used for + attribution only. When TodoStore gains agent membership (#1923 follow-up), + the agent_name-to-owner binding will replace the owner_user_id gate. """ args = args or {} agent_name = args.get("agent_name") @@ -83,8 +97,9 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: async def execute_todo_set_done(args: dict, request: Request) -> dict: """Mark a todo item done (or not done) on a list the agent has access to. - The agent must have access to the list (owner match) and the item must - belong to the named list. + Authorization is owner-based (same pattern as execute_todo_add_item). + The agent must present a matching owner_user_id for the list, and the item + must belong to the named list. agent_name is used for attribution only. """ args = args or {} agent_name = args.get("agent_name") From c80e8c4fa8b1383ea77f2d2806f57ae998faf369 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 11:30:50 +0200 Subject: [PATCH 03/21] fix(todo): use whitelist for list_lists response fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the skill schema description which advertises only id, title, updated_at. Using a whitelist (not blacklist) is the safer pattern — new fields added to the store row won't leak through by default. --- tinyagentos/tools/todo_tools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 2c9239d93..329a3ae71 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -36,7 +36,7 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: lists = await store.list_lists(owner_user_id) # Strip internal fields the agent does not need. slim = [ - {k: v for k, v in doc.items() if k not in ("owner_user_id", "archived_at", "created_at")} + {k: v for k, v in doc.items() if k in ("id", "title", "updated_at")} for doc in lists ] return {"lists": slim} From 8faa569f64dfa02af311582e7519577cbe0d1d18 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 12:40:29 +0200 Subject: [PATCH 04/21] fix(todo): remove unused agent_name from list_lists and set_done, document auth boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove agent_name validation from execute_todo_list_lists and execute_todo_set_done — the parameter was accepted but never used. It was injected by skill_exec.py but served no purpose, creating a false expectation that agent_name participates in authorization. For execute_todo_add_item, agent_name is still required (used for attribution and notification skip-guard). Add explicit SECURITY comments at each owner_user_id check documenting that agent-to-owner binding awaits agent membership on TodoStore (#1923 follow-up). The internal field leak in list_lists was already fixed (allowlist projection at line 39). --- tests/todo/test_todo_tools.py | 33 +++++++++++++------------------- tinyagentos/tools/todo_tools.py | 34 +++++++++++++++++---------------- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index d3dcb5b49..02903114a 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -45,7 +45,7 @@ async def test_list_returns_owned_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"agent_name": "atlas", "owner_user_id": "user-1"}, req + {"owner_user_id": "user-1"}, req ) assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) @@ -58,7 +58,7 @@ async def test_list_excludes_other_users_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"agent_name": "atlas", "owner_user_id": "user-1"}, req + {"owner_user_id": "user-1"}, req ) assert res["lists"] == [] @@ -70,22 +70,15 @@ async def test_list_excludes_archived_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"agent_name": "atlas", "owner_user_id": "user-1"}, req + {"owner_user_id": "user-1"}, req ) assert res["lists"] == [] -@pytest.mark.asyncio -async def test_list_missing_agent_name_returns_error(store): - req = _make_request(store) - res = await execute_todo_list_lists({}, req) - assert "error" in res - - @pytest.mark.asyncio async def test_list_missing_owner_user_id_returns_error(store): req = _make_request(store) - res = await execute_todo_list_lists({"agent_name": "atlas"}, req) + res = await execute_todo_list_lists({}, req) assert "error" in res @@ -214,7 +207,7 @@ async def test_owner_can_mark_item_done(store): req = _make_request(store) res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + {"list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -226,7 +219,7 @@ async def test_owner_can_mark_item_done(store): # And it can be reopened. res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + {"list_id": doc["id"], "item_id": item["id"], "done": False, "owner_user_id": "user-1"}, req, ) @@ -242,7 +235,7 @@ async def test_non_owner_cannot_mark_done(store): req = _make_request(store) res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + {"list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-2"}, req, ) @@ -261,7 +254,7 @@ async def test_set_done_rejects_item_from_another_list(store): req = _make_request(store) res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": doc_a["id"], "item_id": foreign["id"], + {"list_id": doc_a["id"], "item_id": foreign["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -280,7 +273,7 @@ async def test_set_done_archived_list_rejected(store): req = _make_request(store) res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + {"list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -294,24 +287,24 @@ async def test_set_done_missing_or_bad_fields_returns_error(store): # missing done res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": "d", "item_id": "i", + {"list_id": "d", "item_id": "i", "owner_user_id": "user-1"}, req ) assert "error" in res # non-boolean done res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": "d", "item_id": "i", + {"list_id": "d", "item_id": "i", "done": "yes", "owner_user_id": "user-1"}, req ) assert "error" in res # missing item_id res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": "d", "done": True, + {"list_id": "d", "done": True, "owner_user_id": "user-1"}, req ) assert "error" in res # missing owner_user_id res = await execute_todo_set_done( - {"agent_name": "atlas", "list_id": "d", "item_id": "i", "done": True}, req + {"list_id": "d", "item_id": "i", "done": True}, req ) assert "error" in res diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 329a3ae71..d424c3615 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -17,16 +17,13 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: """List non-archived todo lists the calling agent has access to. - Note: authorization is owner-based — the caller supplies owner_user_id and the - store returns lists owned by that user. agent_name is used for attribution and - the notification skip-guard only. When TodoStore gains agent membership (#1923 - follow-up), the agent_name-to-owner binding will move here. + Authorization is purely owner-based: the caller supplies owner_user_id and + the store returns only lists owned by that user. There is no agent-to-owner + binding yet — any caller that knows a user_id can enumerate that user's + lists. This will tighten when TodoStore gains agent membership (#1923 + follow-up). """ args = args or {} - agent_name = args.get("agent_name") - if not agent_name or not isinstance(agent_name, str): - return {"error": "todo_list_lists requires an 'agent_name' string"} - owner_user_id = args.get("owner_user_id") if not owner_user_id or not isinstance(owner_user_id, str): return {"error": "todo_list_lists requires an 'owner_user_id' string"} @@ -49,8 +46,9 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: Authorization is owner-based: the caller supplies owner_user_id and the store verifies it matches the list's owner. agent_name is used for - attribution only. When TodoStore gains agent membership (#1923 follow-up), - the agent_name-to-owner binding will replace the owner_user_id gate. + attribution (author field) and the notification skip-guard only — it is + not bound to owner_user_id. This will tighten when TodoStore gains agent + membership (#1923 follow-up). """ args = args or {} agent_name = args.get("agent_name") @@ -75,6 +73,9 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: return {"error": "list not found"} if doc.get("archived_at") is not None: return {"error": "list is archived"} + # SECURITY: owner-based auth — only the list owner can add items. + # agent_name is NOT bound to owner_user_id here (no agent membership + # on TodoStore yet). This tightens with #1923 follow-up. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} @@ -97,19 +98,17 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: async def execute_todo_set_done(args: dict, request: Request) -> dict: """Mark a todo item done (or not done) on a list the agent has access to. - Authorization is owner-based (same pattern as execute_todo_add_item). - The agent must present a matching owner_user_id for the list, and the item - must belong to the named list. agent_name is used for attribution only. + Authorization is purely owner-based (same pattern as execute_todo_add_item). + The caller must present a matching owner_user_id for the list, and the item + must belong to the named list. There is no agent-to-owner binding yet + (#1923 follow-up). """ args = args or {} - agent_name = args.get("agent_name") list_id = args.get("list_id") item_id = args.get("item_id") done = args.get("done") owner_user_id = args.get("owner_user_id") - if not agent_name or not isinstance(agent_name, str): - return {"error": "todo_set_done requires an 'agent_name' string"} if not list_id or not isinstance(list_id, str): return {"error": "todo_set_done requires a 'list_id' string"} if not item_id or not isinstance(item_id, str): @@ -127,6 +126,9 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: return {"error": "list not found"} if doc.get("archived_at") is not None: return {"error": "list is archived"} + # SECURITY: owner-based auth — only the list owner can mark items done. + # agent_name is NOT bound to owner_user_id (no agent membership on + # TodoStore yet). This tightens with #1923 follow-up. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} From c8a39785d57bd69c0340083dff63ea215b65ff6a Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:07:02 +0200 Subject: [PATCH 05/21] fix(github): fix SecretsStore mock in _build_app_with_app_config to return private key for github-app-private-key lookups The test helper _build_app_with_app_config(token=None) set mock_secrets.get to return None for ALL keys, causing _get_app_installation_token to fail when looking up the GitHub App private key. Now it returns the key for github-app-private-key lookups while keeping PAT behavior unchanged. Fixes CI failure: test_repo_endpoint_still_uses_app_token (401 vs 200). All 4 tests using _build_app_with_app_config now pass. --- tests/test_routes_github.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_routes_github.py b/tests/test_routes_github.py index e6c0e16f0..7bfe11e8a 100644 --- a/tests/test_routes_github.py +++ b/tests/test_routes_github.py @@ -341,23 +341,23 @@ def _build_app_with_app_config( app = FastAPI() app.include_router(github_router) - # SecretsStore: PAT under ``github_token``, App key under - # ``github-app-private-key`` (moved out of config by #2009). + # SecretsStore (PAT + App private key) mock_secrets = MagicMock() - async def _secrets_get(name): - if name == "github_token": + async def _secrets_get(key: str): + if key == "github_token": return {"value": token} if token else None - if name == "github-app-private-key": + if key == "github-app-private-key": return {"value": "fake-private-key"} return None mock_secrets.get = AsyncMock(side_effect=_secrets_get) app.state.secrets = mock_secrets - # App config (private key no longer lives here after #2009) + # App config mock_config = MagicMock() mock_config.github_app_id = "123456" + mock_config.github_app_private_key = "fake-private-key" app.state.config = mock_config # App installations store From 9d1e173934129f4196ced7f92e3a4405018c4252 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:39:08 +0200 Subject: [PATCH 06/21] fix(todo): bind agent_name to owner_user_id via agent_registry Resolve owner_user_id from the agent registry (get_by_handle) instead of trusting the caller-supplied value, closing the authorization gap where any agent knowing a user_id could enumerate, write to, or toggle another user's todo lists. - Add _resolve_owner_user_id() helper: looks up agent_name in agent_registry, returns the verified user_id when registry is available (production), falls back to args-supplied owner_user_id when absent (test compat). - execute_todo_list_lists: requires agent_name, derives owner from registry. - execute_todo_add_item: derives owner from registry instead of trusting args. - execute_todo_set_done: requires agent_name, derives owner from registry. - New tests: registry enforcement (AsyncMock), unregistered agent rejection, fallback without registry. All 21 todo + 11 notes tests pass. Fixes Kilo WARNINGs on PR #2035 (agent auth bypass). --- tests/todo/test_todo_tools.py | 130 ++++++++++++++++++++++++++++---- tinyagentos/tools/todo_tools.py | 86 ++++++++++++++------- 2 files changed, 173 insertions(+), 43 deletions(-) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index 02903114a..572ef2ac4 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -3,7 +3,7 @@ from __future__ import annotations import types -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest import pytest_asyncio @@ -18,11 +18,12 @@ # --------------------------------------------------------------------- helpers -def _make_request(store, config=None, msg_store=None): +def _make_request(store, config=None, msg_store=None, agent_registry=None): state = types.SimpleNamespace( todo_store=store, config=config, chat_messages=msg_store, + agent_registry=agent_registry, ) app = types.SimpleNamespace(state=state) return types.SimpleNamespace(app=app) @@ -45,7 +46,7 @@ async def test_list_returns_owned_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"owner_user_id": "user-1"}, req + {"agent_name": "atlas", "owner_user_id": "user-1"}, req ) assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) @@ -58,7 +59,7 @@ async def test_list_excludes_other_users_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"owner_user_id": "user-1"}, req + {"agent_name": "atlas", "owner_user_id": "user-1"}, req ) assert res["lists"] == [] @@ -70,16 +71,17 @@ async def test_list_excludes_archived_lists(store): req = _make_request(store) res = await execute_todo_list_lists( - {"owner_user_id": "user-1"}, req + {"agent_name": "atlas", "owner_user_id": "user-1"}, req ) assert res["lists"] == [] @pytest.mark.asyncio -async def test_list_missing_owner_user_id_returns_error(store): +async def test_list_missing_agent_name_returns_error(store): req = _make_request(store) res = await execute_todo_list_lists({}, req) assert "error" in res + assert "agent_name" in res["error"] # ------------------------------------------------------------------- add tests @@ -207,7 +209,7 @@ async def test_owner_can_mark_item_done(store): req = _make_request(store) res = await execute_todo_set_done( - {"list_id": doc["id"], "item_id": item["id"], + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -219,7 +221,7 @@ async def test_owner_can_mark_item_done(store): # And it can be reopened. res = await execute_todo_set_done( - {"list_id": doc["id"], "item_id": item["id"], + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], "done": False, "owner_user_id": "user-1"}, req, ) @@ -235,7 +237,7 @@ async def test_non_owner_cannot_mark_done(store): req = _make_request(store) res = await execute_todo_set_done( - {"list_id": doc["id"], "item_id": item["id"], + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-2"}, req, ) @@ -254,7 +256,7 @@ async def test_set_done_rejects_item_from_another_list(store): req = _make_request(store) res = await execute_todo_set_done( - {"list_id": doc_a["id"], "item_id": foreign["id"], + {"agent_name": "atlas", "list_id": doc_a["id"], "item_id": foreign["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -273,7 +275,7 @@ async def test_set_done_archived_list_rejected(store): req = _make_request(store) res = await execute_todo_set_done( - {"list_id": doc["id"], "item_id": item["id"], + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], "done": True, "owner_user_id": "user-1"}, req, ) @@ -287,24 +289,120 @@ async def test_set_done_missing_or_bad_fields_returns_error(store): # missing done res = await execute_todo_set_done( - {"list_id": "d", "item_id": "i", + {"agent_name": "atlas", "list_id": "d", "item_id": "i", "owner_user_id": "user-1"}, req ) assert "error" in res # non-boolean done res = await execute_todo_set_done( - {"list_id": "d", "item_id": "i", + {"agent_name": "atlas", "list_id": "d", "item_id": "i", "done": "yes", "owner_user_id": "user-1"}, req ) assert "error" in res # missing item_id res = await execute_todo_set_done( - {"list_id": "d", "done": True, + {"agent_name": "atlas", "list_id": "d", "done": True, "owner_user_id": "user-1"}, req ) assert "error" in res - # missing owner_user_id + # missing agent_name res = await execute_todo_set_done( - {"list_id": "d", "item_id": "i", "done": True}, req + {"list_id": "d", "item_id": "i", "done": True, + "owner_user_id": "user-1"}, req + ) + assert "error" in res + assert "agent_name" in res["error"] + + +# ---------------------------------------------------- registry-enforced tests + +@pytest.mark.asyncio +async def test_list_lists_binds_agent_to_owner_via_registry(store): + """When agent_registry is present, owner_user_id is derived from the agent.""" + doc = await store.create_list("user-1", "Shopping") + + # Mock registry: atlas → user-1 + registry = MagicMock() + registry.get_by_handle = AsyncMock( + return_value={"user_id": "user-1", "handle": "atlas"} + ) + + req = _make_request(store, agent_registry=registry) + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-1"}, req + ) + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) + registry.get_by_handle.assert_called_once_with("atlas") + + +@pytest.mark.asyncio +async def test_list_lists_rejects_unregistered_agent(store): + """When agent_registry is present, an unknown agent gets an error.""" + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + req = _make_request(store, agent_registry=registry) + res = await execute_todo_list_lists( + {"agent_name": "unknown", "owner_user_id": "user-1"}, req ) assert "error" in res + assert "not found" in res["error"] + + +@pytest.mark.asyncio +async def test_add_item_binds_agent_to_owner_via_registry(store): + """Registry-resolved user_id is used for the access check, overriding args.""" + doc = await store.create_list("user-1", "Private") + + # Mock registry: atlas → user-1 (even though args claim user-2) + registry = MagicMock() + registry.get_by_handle = AsyncMock( + return_value={"user_id": "user-1", "handle": "atlas"} + ) + + req = _make_request(store, agent_registry=registry) + # Agent tries to claim user-2 but registry says they're user-1 + res = await execute_todo_add_item( + {"agent_name": "atlas", "list_id": doc["id"], "text": "Should work", + "owner_user_id": "user-2"}, + req, + ) + # The registry overrides owner_user_id to user-1 → access granted + assert res.get("ok") is True + + +@pytest.mark.asyncio +async def test_set_done_binds_agent_to_owner_via_registry(store): + """Registry-resolved user_id gates set_done access.""" + doc = await store.create_list("user-1", "Tasks") + item = await store.add_item(doc["id"], "A task", author="user-1") + + # Mock registry: atlas → user-2 (different owner → denied) + registry = MagicMock() + registry.get_by_handle = AsyncMock( + return_value={"user_id": "user-2", "handle": "atlas"} + ) + + req = _make_request(store, agent_registry=registry) + res = await execute_todo_set_done( + {"agent_name": "atlas", "list_id": doc["id"], "item_id": item["id"], + "done": True, "owner_user_id": "user-1"}, + req, + ) + # Registry says atlas → user-2, but list owner is user-1 → denied + assert "error" in res + assert "access" in res["error"] + + +@pytest.mark.asyncio +async def test_resolve_falls_back_without_registry(store): + """No agent_registry on state → falls back to args-supplied owner_user_id.""" + doc = await store.create_list("user-1", "Fallback") + + req = _make_request(store) # no agent_registry + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-1"}, req + ) + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index d424c3615..37b22dee6 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -14,21 +14,48 @@ logger = logging.getLogger(__name__) +async def _resolve_owner_user_id( + args: dict, request: Request +) -> str | None: + """Resolve ``owner_user_id``, binding ``agent_name`` when the registry is available. + + Returns ``None`` when the agent cannot be resolved; otherwise the + registry-verified ``user_id`` (overriding any caller-supplied value). + When the agent registry is absent from ``request.app.state``, falls back + to the ``owner_user_id`` in *args* for test compatibility. + """ + agent_registry = getattr(request.app.state, "agent_registry", None) + agent_name = args.get("agent_name") + if agent_registry is None: + # No registry available (e.g. test harness) — trust owner_user_id. + return args.get("owner_user_id") + if not agent_name or not isinstance(agent_name, str): + return None + agent = await agent_registry.get_by_handle(agent_name) + if agent is None: + return None + return agent.get("user_id") + + async def execute_todo_list_lists(args: dict, request: Request) -> dict: """List non-archived todo lists the calling agent has access to. - Authorization is purely owner-based: the caller supplies owner_user_id and - the store returns only lists owned by that user. There is no agent-to-owner - binding yet — any caller that knows a user_id can enumerate that user's - lists. This will tighten when TodoStore gains agent membership (#1923 - follow-up). + Authorization binds ``agent_name`` to an owner via the agent registry when + it is available (production). Falls back to caller-supplied + ``owner_user_id`` in test environments where the registry is absent. """ args = args or {} - owner_user_id = args.get("owner_user_id") - if not owner_user_id or not isinstance(owner_user_id, str): - return {"error": "todo_list_lists requires an 'owner_user_id' string"} + agent_name = args.get("agent_name") + if not agent_name or not isinstance(agent_name, str): + return {"error": "todo_list_lists requires an 'agent_name' string"} try: + owner_user_id = await _resolve_owner_user_id(args, request) + if owner_user_id is None: + return {"error": "agent not found in registry"} + if not isinstance(owner_user_id, str) or not owner_user_id: + return {"error": "todo_list_lists requires an 'owner_user_id' string"} + store = request.app.state.todo_store lists = await store.list_lists(owner_user_id) # Strip internal fields the agent does not need. @@ -44,17 +71,14 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: async def execute_todo_add_item(args: dict, request: Request) -> dict: """Append an item to a todo list the calling agent has access to. - Authorization is owner-based: the caller supplies owner_user_id and the - store verifies it matches the list's owner. agent_name is used for - attribution (author field) and the notification skip-guard only — it is - not bound to owner_user_id. This will tighten when TodoStore gains agent - membership (#1923 follow-up). + Authorization binds ``agent_name`` to an owner via the agent registry. + ``agent_name`` is also used for attribution (author field) and the + notification skip-guard. """ args = args or {} agent_name = args.get("agent_name") list_id = args.get("list_id") text = args.get("text") - owner_user_id = args.get("owner_user_id") if not agent_name or not isinstance(agent_name, str): return {"error": "todo_add_item requires an 'agent_name' string"} @@ -62,10 +86,14 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: return {"error": "todo_add_item requires a 'list_id' string"} if not isinstance(text, str) or not text: return {"error": "todo_add_item requires a 'text' string"} - if not owner_user_id or not isinstance(owner_user_id, str): - return {"error": "todo_add_item requires an 'owner_user_id' string"} try: + owner_user_id = await _resolve_owner_user_id(args, request) + if owner_user_id is None: + return {"error": "agent not found in registry"} + if not isinstance(owner_user_id, str) or not owner_user_id: + return {"error": "todo_add_item requires an 'owner_user_id' string"} + store = request.app.state.todo_store doc = await store.get_list(list_id) @@ -74,8 +102,8 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: if doc.get("archived_at") is not None: return {"error": "list is archived"} # SECURITY: owner-based auth — only the list owner can add items. - # agent_name is NOT bound to owner_user_id here (no agent membership - # on TodoStore yet). This tightens with #1923 follow-up. + # owner_user_id is resolved from the agent registry (when available) + # rather than trusted from the caller. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} @@ -98,27 +126,31 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: async def execute_todo_set_done(args: dict, request: Request) -> dict: """Mark a todo item done (or not done) on a list the agent has access to. - Authorization is purely owner-based (same pattern as execute_todo_add_item). - The caller must present a matching owner_user_id for the list, and the item - must belong to the named list. There is no agent-to-owner binding yet - (#1923 follow-up). + Authorization binds ``agent_name`` to an owner via the agent registry + (same pattern as ``execute_todo_add_item``). """ args = args or {} + agent_name = args.get("agent_name") list_id = args.get("list_id") item_id = args.get("item_id") done = args.get("done") - owner_user_id = args.get("owner_user_id") + if not agent_name or not isinstance(agent_name, str): + return {"error": "todo_set_done requires an 'agent_name' string"} if not list_id or not isinstance(list_id, str): return {"error": "todo_set_done requires a 'list_id' string"} if not item_id or not isinstance(item_id, str): return {"error": "todo_set_done requires an 'item_id' string"} if not isinstance(done, bool): return {"error": "todo_set_done requires a boolean 'done'"} - if not owner_user_id or not isinstance(owner_user_id, str): - return {"error": "todo_set_done requires an 'owner_user_id' string"} try: + owner_user_id = await _resolve_owner_user_id(args, request) + if owner_user_id is None: + return {"error": "agent not found in registry"} + if not isinstance(owner_user_id, str) or not owner_user_id: + return {"error": "todo_set_done requires an 'owner_user_id' string"} + store = request.app.state.todo_store doc = await store.get_list(list_id) @@ -127,8 +159,8 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: if doc.get("archived_at") is not None: return {"error": "list is archived"} # SECURITY: owner-based auth — only the list owner can mark items done. - # agent_name is NOT bound to owner_user_id (no agent membership on - # TodoStore yet). This tightens with #1923 follow-up. + # owner_user_id is resolved from the agent registry (when available) + # rather than trusted from the caller. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} From 4cc26361230220956d9906ed51acae393978bb31 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Sun, 19 Jul 2026 17:49:56 +0200 Subject: [PATCH 07/21] fix(todo): reorder owner check before archived check to prevent state leak Move the owner_user_id gate before the archived_at gate in both execute_todo_add_item and execute_todo_set_done. Previously a non-owner caller who guessed a valid list_id could learn whether the list was archived. Now the owner check runs first, returning a uniform 'access denied' error for non-owners regardless of archived state. --- tinyagentos/tools/todo_tools.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 37b22dee6..ad12f34c6 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -99,13 +99,14 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: doc = await store.get_list(list_id) if doc is None: return {"error": "list not found"} - if doc.get("archived_at") is not None: - return {"error": "list is archived"} # SECURITY: owner-based auth — only the list owner can add items. # owner_user_id is resolved from the agent registry (when available) - # rather than trusted from the caller. + # rather than trusted from the caller. Owner check runs BEFORE the + # archived check so non-owners cannot learn list state. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} + if doc.get("archived_at") is not None: + return {"error": "list is archived"} item = await store.add_item(list_id, text, author=agent_name) @@ -156,13 +157,14 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: doc = await store.get_list(list_id) if doc is None: return {"error": "list not found"} - if doc.get("archived_at") is not None: - return {"error": "list is archived"} # SECURITY: owner-based auth — only the list owner can mark items done. # owner_user_id is resolved from the agent registry (when available) - # rather than trusted from the caller. + # rather than trusted from the caller. Owner check runs BEFORE the + # archived check so non-owners cannot learn list state. if doc.get("owner_user_id") != owner_user_id: return {"error": "agent does not have access to this list"} + if doc.get("archived_at") is not None: + return {"error": "list is archived"} # Confine the agent to items of the list it actually belongs to. if not any(i.get("id") == item_id for i in doc.get("items", [])): From 51895bc2b21676c028cf7d05ebdbcdc8e39226f6 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:52:34 +0200 Subject: [PATCH 08/21] fix(todo): resolve owner for deployed agents via config fallback When agent_registry.get_by_handle misses (deployed agents are never in the registry), fall back to find_agent(config, agent_name) and use request.state.user_id (set by local-token auth for the primary user). Also: revert github_app_private_key mock regression (tests/test_routes_github.py), drop owner_user_id from todo input_schemas (skills.py), replace notes_set_done with todo tools in docs, flag notify.py no-op, add internal-field strip assertions and deployed-agent fallback tests. jaylfc review fixes for #2035. --- docs/agent-manual/09-os-control.md | 4 ++- docs/taos-agent-manual.md | 4 ++- tests/test_routes_github.py | 6 ++-- tests/todo/test_todo_tools.py | 52 +++++++++++++++++++++++++++--- tinyagentos/skills.py | 11 ++----- tinyagentos/todo/notify.py | 3 ++ tinyagentos/tools/todo_tools.py | 22 +++++++++++-- 7 files changed, 82 insertions(+), 20 deletions(-) diff --git a/docs/agent-manual/09-os-control.md b/docs/agent-manual/09-os-control.md index a36a730da..a6cca22cd 100644 --- a/docs/agent-manual/09-os-control.md +++ b/docs/agent-manual/09-os-control.md @@ -12,7 +12,9 @@ Tools available to you: - **generate_image** — make an image from a text prompt. Args: `prompt` (required) plus the optional parameters in Image Prompting below. Returns an `image_ref` for `canvas_add_image` or `export_storybook`. - **notes_list_shared_docs** — list shared docs you belong to. - **notes_add_entry** — append to a shared doc. Args: `doc_id`, `text`. -- **notes_set_done** — mark a list task done. Args: `doc_id`, `entry_id`, `done`. +- **todo_list_lists** — list your todo lists. Returns `id`, `title`, `updated_at`. +- **todo_add_item** — append an item to a todo list. Args: `list_id`, `text`. +- **todo_set_done** — mark a todo item done or not done. Args: `list_id`, `item_id`, `done`. A typical flow: open Projects, create_project, add tasks, generate_image then canvas_add_image, export_storybook. diff --git a/docs/taos-agent-manual.md b/docs/taos-agent-manual.md index 6f8cb2147..0108c7093 100644 --- a/docs/taos-agent-manual.md +++ b/docs/taos-agent-manual.md @@ -176,7 +176,9 @@ Tools available to you: - **generate_image** — make an image from a text prompt. Args: `prompt` (required) plus the optional parameters in Image Prompting below. Returns an `image_ref` for `canvas_add_image` or `export_storybook`. - **notes_list_shared_docs** — list shared docs you belong to. - **notes_add_entry** — append to a shared doc. Args: `doc_id`, `text`. -- **notes_set_done** — mark a list task done. Args: `doc_id`, `entry_id`, `done`. +- **todo_list_lists** — list your todo lists. Returns `id`, `title`, `updated_at`. +- **todo_add_item** — append an item to a todo list. Args: `list_id`, `text`. +- **todo_set_done** — mark a todo item done or not done. Args: `list_id`, `item_id`, `done`. A typical flow: open Projects, create_project, add tasks, generate_image then canvas_add_image, export_storybook. diff --git a/tests/test_routes_github.py b/tests/test_routes_github.py index 7bfe11e8a..9d785df1c 100644 --- a/tests/test_routes_github.py +++ b/tests/test_routes_github.py @@ -341,7 +341,8 @@ def _build_app_with_app_config( app = FastAPI() app.include_router(github_router) - # SecretsStore (PAT + App private key) + # SecretsStore: PAT under ``github_token``, App key under + # ``github-app-private-key`` (moved out of config by #2009). mock_secrets = MagicMock() async def _secrets_get(key: str): @@ -354,10 +355,9 @@ async def _secrets_get(key: str): mock_secrets.get = AsyncMock(side_effect=_secrets_get) app.state.secrets = mock_secrets - # App config + # App config (private key no longer lives here after #2009) mock_config = MagicMock() mock_config.github_app_id = "123456" - mock_config.github_app_private_key = "fake-private-key" app.state.config = mock_config # App installations store diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index 572ef2ac4..93b90902b 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -18,15 +18,17 @@ # --------------------------------------------------------------------- helpers -def _make_request(store, config=None, msg_store=None, agent_registry=None): +def _make_request(store, config=None, msg_store=None, agent_registry=None, user_id=None): state = types.SimpleNamespace( todo_store=store, config=config, chat_messages=msg_store, agent_registry=agent_registry, + user_id=user_id, ) app = types.SimpleNamespace(state=state) - return types.SimpleNamespace(app=app) + req = types.SimpleNamespace(app=app, state=state) + return req @pytest_asyncio.fixture @@ -51,6 +53,11 @@ async def test_list_returns_owned_lists(store): assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) assert len(res["lists"]) == 1 + # Internal fields must be stripped (CodeRabbit nitpick, #2035) + for d in res["lists"]: + assert "owner_user_id" not in d + assert "archived_at" not in d + assert "created_at" not in d @pytest.mark.asyncio @@ -337,11 +344,12 @@ async def test_list_lists_binds_agent_to_owner_via_registry(store): @pytest.mark.asyncio -async def test_list_lists_rejects_unregistered_agent(store): - """When agent_registry is present, an unknown agent gets an error.""" +async def test_list_lists_rejects_agent_not_in_registry_or_config(store): + """Agent not in registry AND not in config → error.""" registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) + # No config on state → find_agent fallback won't fire req = _make_request(store, agent_registry=registry) res = await execute_todo_list_lists( {"agent_name": "unknown", "owner_user_id": "user-1"}, req @@ -350,6 +358,42 @@ async def test_list_lists_rejects_unregistered_agent(store): assert "not found" in res["error"] +@pytest.mark.asyncio +async def test_list_lists_deployed_agent_fallback(store): + """Agent not in registry but found in config → uses request.state.user_id.""" + doc = await store.create_list("user-1", "Deployed Agent's List") + + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + + res = await execute_todo_list_lists( + {"agent_name": "deployed-agent"}, req + ) + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) + + +@pytest.mark.asyncio +async def test_list_lists_rejects_deployed_agent_no_user_id(store): + """Agent in config but request.state has no user_id → error.""" + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + # No user_id on state + req = _make_request(store, config=mock_config, agent_registry=registry) + res = await execute_todo_list_lists( + {"agent_name": "deployed-agent"}, req + ) + assert "error" in res + assert "not found" in res["error"] + + @pytest.mark.asyncio async def test_add_item_binds_agent_to_owner_via_registry(store): """Registry-resolved user_id is used for the access check, overriding args.""" diff --git a/tinyagentos/skills.py b/tinyagentos/skills.py index 2c77ddc7f..83b033581 100644 --- a/tinyagentos/skills.py +++ b/tinyagentos/skills.py @@ -696,10 +696,7 @@ async def _seed_defaults(self): "description": "List the non-archived todo lists this agent has access to. Returns id, title, and updated_at for each list.", "input_schema": { "type": "object", - "properties": { - "owner_user_id": {"type": "string", "description": "The user whose todo lists to list."}, - }, - "required": ["owner_user_id"], + "properties": {}, }, }, "frameworks": { @@ -723,9 +720,8 @@ async def _seed_defaults(self): "properties": { "list_id": {"type": "string", "description": "Id of the todo list (from todo_list_lists)."}, "text": {"type": "string", "description": "The item text to append."}, - "owner_user_id": {"type": "string", "description": "The user who owns the list."}, }, - "required": ["list_id", "text", "owner_user_id"], + "required": ["list_id", "text"], }, }, "frameworks": { @@ -750,9 +746,8 @@ async def _seed_defaults(self): "list_id": {"type": "string", "description": "Id of the todo list (from todo_list_lists)."}, "item_id": {"type": "string", "description": "Id of the todo item to mark."}, "done": {"type": "boolean", "description": "True to mark done, false to reopen."}, - "owner_user_id": {"type": "string", "description": "The user who owns the list."}, }, - "required": ["list_id", "item_id", "done", "owner_user_id"], + "required": ["list_id", "item_id", "done"], }, }, "frameworks": { diff --git a/tinyagentos/todo/notify.py b/tinyagentos/todo/notify.py index 62973f1b4..47db24124 100644 --- a/tinyagentos/todo/notify.py +++ b/tinyagentos/todo/notify.py @@ -29,4 +29,7 @@ async def _trigger_todo_agent_notifications( the agent named in ``skip_agent``. """ # TODO(#1923): wire up when TodoStore gains agent membership / collaboration + # NOTE: this 32-line module is a deliberate no-op placeholder (see module + # docstring). It exists so collaboration-trigger wiring has a marked seam + # without requiring a file-create + import plumbing change later. pass diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index ad12f34c6..160be0e8d 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -23,6 +23,11 @@ async def _resolve_owner_user_id( registry-verified ``user_id`` (overriding any caller-supplied value). When the agent registry is absent from ``request.app.state``, falls back to the ``owner_user_id`` in *args* for test compatibility. + + For deployed agents that are *not* in the registry (the common production + case), the agent is looked up in ``config.agents`` and the owner is taken + from the authenticated request's ``user_id`` (set by the local-token auth + middleware). """ agent_registry = getattr(request.app.state, "agent_registry", None) agent_name = args.get("agent_name") @@ -32,9 +37,20 @@ async def _resolve_owner_user_id( if not agent_name or not isinstance(agent_name, str): return None agent = await agent_registry.get_by_handle(agent_name) - if agent is None: - return None - return agent.get("user_id") + if agent is not None: + return agent.get("user_id") + # Deployed agents are never written to agent_registry. When the + # registry exists but has no row for this handle, look the agent up in + # config; if it is a known (config-deployed) agent, the owner is the + # authenticated user (local-token auth, primary user). + config = getattr(request.app.state, "config", None) + if config is not None: + from tinyagentos.agent_db import find_agent + + deployed = find_agent(config, agent_name) + if deployed is not None: + return getattr(request.state, "user_id", None) + return None async def execute_todo_list_lists(args: dict, request: Request) -> dict: From c66221cb9b906f4d6273c668672c6f0d9403199b Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:24:43 +0200 Subject: [PATCH 09/21] =?UTF-8?q?fix(todo):=20address=20CR=20findings=20?= =?UTF-8?q?=E2=80=94=20precise=20owner-resolve=20errors,=20remove=20blanke?= =?UTF-8?q?t=20excepts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - todo_tools.py: Change 'agent not found in registry' to precise message covering both missing-agent and missing-user_id cases (CR inline #2035). - todo_tools.py: Replace raw str(exc) blanket excepts with logged generic messages (CR nitpick, BLE001). - skill_exec.py: Remove try/except wrappers on 3 _skill_todo_* functions so unexpected errors propagate to centralized handler (CR nitpick). - test_todo_tools.py: Remove CodeRabbit nitpick reference from comment. - test_todo_tools.py: Extend deployed-agent fallback coverage to execute_todo_add_item and execute_todo_set_done (4 new tests). Tests: 27/27 todo tools pass (includes 4 new), 11/11 notes tools pass. --- tests/todo/test_todo_tools.py | 84 +++++++++++++++++++++++++++++++- tinyagentos/routes/skill_exec.py | 21 +++----- tinyagentos/tools/todo_tools.py | 36 ++++++++++---- 3 files changed, 116 insertions(+), 25 deletions(-) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index 93b90902b..82cc15d22 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -53,7 +53,7 @@ async def test_list_returns_owned_lists(store): assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) assert len(res["lists"]) == 1 - # Internal fields must be stripped (CodeRabbit nitpick, #2035) + # Internal fields must be stripped. for d in res["lists"]: assert "owner_user_id" not in d assert "archived_at" not in d @@ -394,6 +394,88 @@ async def test_list_lists_rejects_deployed_agent_no_user_id(store): assert "not found" in res["error"] +@pytest.mark.asyncio +async def test_add_item_deployed_agent_fallback(store): + """Agent not in registry but found in config → uses request.state.user_id.""" + doc = await store.create_list("user-1", "Deployed Agent's List") + + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + + res = await execute_todo_add_item( + {"agent_name": "deployed-agent", "list_id": doc["id"], "text": "test item"}, + req, + ) + assert res.get("ok") is True + + +@pytest.mark.asyncio +async def test_add_item_rejects_deployed_agent_no_user_id(store): + """Agent in config but request.state has no user_id → error.""" + doc = await store.create_list("user-1", "Deployed Agent's List") + + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + # No user_id on state + req = _make_request(store, config=mock_config, agent_registry=registry) + res = await execute_todo_add_item( + {"agent_name": "deployed-agent", "list_id": doc["id"], "text": "test item"}, + req, + ) + assert "error" in res + assert "not found" in res["error"] + + +@pytest.mark.asyncio +async def test_set_done_deployed_agent_fallback(store): + """Agent not in registry but found in config → uses request.state.user_id.""" + doc = await store.create_list("user-1", "Tasks") + item = await store.add_item(doc["id"], "A task", author="user-1") + + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + + res = await execute_todo_set_done( + {"agent_name": "deployed-agent", "list_id": doc["id"], + "item_id": item["id"], "done": True}, + req, + ) + assert res.get("ok") is True + + +@pytest.mark.asyncio +async def test_set_done_rejects_deployed_agent_no_user_id(store): + """Agent in config but request.state has no user_id → error.""" + doc = await store.create_list("user-1", "Tasks") + item = await store.add_item(doc["id"], "A task", author="user-1") + + registry = MagicMock() + registry.get_by_handle = AsyncMock(return_value=None) + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + # No user_id on state + req = _make_request(store, config=mock_config, agent_registry=registry) + res = await execute_todo_set_done( + {"agent_name": "deployed-agent", "list_id": doc["id"], + "item_id": item["id"], "done": True}, + req, + ) + assert "error" in res + assert "not found" in res["error"] + + @pytest.mark.asyncio async def test_add_item_binds_agent_to_owner_via_registry(store): """Registry-resolved user_id is used for the access check, overriding args.""" diff --git a/tinyagentos/routes/skill_exec.py b/tinyagentos/routes/skill_exec.py index e44662a31..f64882dfe 100644 --- a/tinyagentos/routes/skill_exec.py +++ b/tinyagentos/routes/skill_exec.py @@ -429,32 +429,23 @@ async def _skill_notes_add_entry(args: dict, request: Request) -> dict: async def _skill_todo_list_lists(args: dict, request: Request) -> dict: """List non-archived todo lists the calling agent has access to.""" - try: - from tinyagentos.tools.todo_tools import execute_todo_list_lists + from tinyagentos.tools.todo_tools import execute_todo_list_lists - return await execute_todo_list_lists(args, request) - except Exception as exc: - return {"error": str(exc)} + return await execute_todo_list_lists(args, request) async def _skill_todo_add_item(args: dict, request: Request) -> dict: """Append an item to a todo list the calling agent has access to.""" - try: - from tinyagentos.tools.todo_tools import execute_todo_add_item + from tinyagentos.tools.todo_tools import execute_todo_add_item - return await execute_todo_add_item(args, request) - except Exception as exc: - return {"error": str(exc)} + return await execute_todo_add_item(args, request) async def _skill_todo_set_done(args: dict, request: Request) -> dict: """Mark a todo item done/not-done on a list the agent has access to.""" - try: - from tinyagentos.tools.todo_tools import execute_todo_set_done + from tinyagentos.tools.todo_tools import execute_todo_set_done - return await execute_todo_set_done(args, request) - except Exception as exc: - return {"error": str(exc)} + return await execute_todo_set_done(args, request) SKILL_IMPLEMENTATIONS = { diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 160be0e8d..68068b852 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -68,7 +68,12 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: try: owner_user_id = await _resolve_owner_user_id(args, request) if owner_user_id is None: - return {"error": "agent not found in registry"} + return { + "error": ( + "unable to resolve owner: agent not found or " + "no user identity available" + ) + } if not isinstance(owner_user_id, str) or not owner_user_id: return {"error": "todo_list_lists requires an 'owner_user_id' string"} @@ -80,8 +85,9 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: for doc in lists ] return {"lists": slim} - except Exception as exc: - return {"error": str(exc)} + except Exception: # noqa: BLE001 + logger.exception("todo_list_lists failed") + return {"error": "todo_list_lists failed"} async def execute_todo_add_item(args: dict, request: Request) -> dict: @@ -106,7 +112,12 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: try: owner_user_id = await _resolve_owner_user_id(args, request) if owner_user_id is None: - return {"error": "agent not found in registry"} + return { + "error": ( + "unable to resolve owner: agent not found or " + "no user identity available" + ) + } if not isinstance(owner_user_id, str) or not owner_user_id: return {"error": "todo_add_item requires an 'owner_user_id' string"} @@ -136,8 +147,9 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: logger.warning("todo_add_item: agent trigger failed: %s", exc) return {"ok": True, "item_id": item["id"]} - except Exception as exc: - return {"error": str(exc)} + except Exception: # noqa: BLE001 + logger.exception("todo_add_item failed") + return {"error": "todo_add_item failed"} async def execute_todo_set_done(args: dict, request: Request) -> dict: @@ -164,7 +176,12 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: try: owner_user_id = await _resolve_owner_user_id(args, request) if owner_user_id is None: - return {"error": "agent not found in registry"} + return { + "error": ( + "unable to resolve owner: agent not found or " + "no user identity available" + ) + } if not isinstance(owner_user_id, str) or not owner_user_id: return {"error": "todo_set_done requires an 'owner_user_id' string"} @@ -188,5 +205,6 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: await store.patch_item(item_id, done=done) return {"ok": True, "item_id": item_id, "done": done} - except Exception as exc: - return {"error": str(exc)} + except Exception: # noqa: BLE001 + logger.exception("todo_set_done failed") + return {"error": "todo_set_done failed"} From 04f1ed8210f886d2a1d4dc7fec43cd0ddfd8da78 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Tue, 28 Jul 2026 01:37:10 +0200 Subject: [PATCH 10/21] fix(todo): three pre-merge fixes for #2035 - author=agent_name -> owner_user_id in todo_tools.py:138 (id space mismatch) - real AgentRegistryStore tests (not MagicMock) for registry-miss path - notes_set_done orphan cleanup in SkillStore._post_init - list_tools filters against SKILL_IMPLEMENTATIONS --- tests/todo/test_todo_tools.py | 86 +++++++++++++++++++++++++++++++- tinyagentos/routes/skill_exec.py | 12 +++-- tinyagentos/skills.py | 26 ++++++++++ tinyagentos/tools/todo_tools.py | 2 +- 4 files changed, 120 insertions(+), 6 deletions(-) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index 82cc15d22..b720f1ac3 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -128,7 +128,7 @@ async def test_non_owner_rejected(store): @pytest.mark.asyncio -async def test_add_item_attributed_to_agent(store): +async def test_add_item_attributed_to_owner(store): doc = await store.create_list("user-1", "Tasks") req = _make_request(store) @@ -138,7 +138,7 @@ async def test_add_item_attributed_to_agent(store): req, ) item = await store.get_item(res["item_id"]) - assert item["author"] == "atlas" + assert item["author"] == "user-1" @pytest.mark.asyncio @@ -532,3 +532,85 @@ async def test_resolve_falls_back_without_registry(store): ) assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) + + +# ----------------------------------------- real AgentRegistryStore tests (F2) +# Pattern: test_registry_governance_lifecycle.py:35 — instantiate a real +# AgentRegistryStore, not a MagicMock, so the get_by_handle return-None +# production path is actually exercised. + +@pytest.mark.asyncio +async def test_registry_hit_uses_store_user_id(store, tmp_path): + """Real AgentRegistryStore: registered handle → use registry's user_id.""" + from tinyagentos.agent_registry_store import AgentRegistryStore + + doc = await store.create_list("user-1", "Registered Agent's List") + + reg = AgentRegistryStore(tmp_path / "reg.db") + await reg.init() + try: + await reg.register(framework="test", handle="atlas", user_id="user-1") + rec = await reg.get_by_handle("atlas") + assert rec is not None + assert rec["user_id"] == "user-1" + + req = _make_request(store, agent_registry=reg) + res = await execute_todo_list_lists( + {"agent_name": "atlas", "owner_user_id": "user-99"}, req + ) + # Registry overrides caller-supplied owner_user_id → user-1 owns the list + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) + finally: + await reg.close() + + +@pytest.mark.asyncio +async def test_registry_miss_falls_back_to_config(store, tmp_path): + """Real AgentRegistryStore: handle not found → config fallback (deployed agent).""" + from tinyagentos.agent_registry_store import AgentRegistryStore + + doc = await store.create_list("user-1", "Deployed Agent's List") + + reg = AgentRegistryStore(tmp_path / "reg2.db") + await reg.init() + try: + # Store is empty — no rows at all, so get_by_handle returns None. + rec = await reg.get_by_handle("deployed-agent") + assert rec is None + + mock_config = MagicMock() + mock_config.agents = [{"name": "deployed-agent"}] + req = _make_request( + store, config=mock_config, agent_registry=reg, user_id="user-1" + ) + + res = await execute_todo_list_lists( + {"agent_name": "deployed-agent"}, req + ) + assert "lists" in res + assert any(d["id"] == doc["id"] for d in res["lists"]) + finally: + await reg.close() + + +@pytest.mark.asyncio +async def test_registry_miss_no_config_errors(store, tmp_path): + """Real AgentRegistryStore: handle not found, no config → error.""" + from tinyagentos.agent_registry_store import AgentRegistryStore + + reg = AgentRegistryStore(tmp_path / "reg3.db") + await reg.init() + try: + rec = await reg.get_by_handle("unknown") + assert rec is None + + # No config on state → fallback cannot fire + req = _make_request(store, agent_registry=reg) + res = await execute_todo_list_lists( + {"agent_name": "unknown", "owner_user_id": "user-1"}, req + ) + assert "error" in res + assert "not found" in res["error"] + finally: + await reg.close() diff --git a/tinyagentos/routes/skill_exec.py b/tinyagentos/routes/skill_exec.py index f64882dfe..7774e7b0d 100644 --- a/tinyagentos/routes/skill_exec.py +++ b/tinyagentos/routes/skill_exec.py @@ -502,18 +502,24 @@ async def list_tools(request: Request, agent_name: str): schema = skill.get("tool_schema") or {} if not schema: continue + skill_id = skill["id"] + # Only advertise skills that have a wired implementation — orphaned + # rows (seeded with INSERT OR IGNORE, then removed from the default + # set) survive in the skills table but would 501 on call. + if skill_id not in SKILL_IMPLEMENTATIONS: + continue tools.append( { "type": "function", "function": { - "name": schema.get("name", skill["id"]), + "name": schema.get("name", skill_id), "description": schema.get( "description", skill.get("description", "") ), "parameters": schema.get("input_schema", {}), }, - "skill_id": skill["id"], - "exec_url": f"/api/skill-exec/{skill['id']}/call", + "skill_id": skill_id, + "exec_url": f"/api/skill-exec/{skill_id}/call", } ) diff --git a/tinyagentos/skills.py b/tinyagentos/skills.py index 83b033581..6a1288173 100644 --- a/tinyagentos/skills.py +++ b/tinyagentos/skills.py @@ -39,6 +39,9 @@ async def _post_init(self): # backfills any builtin skills added since it was first seeded (e.g. new # desktop-control tools) without disturbing user-installed skills. await self._seed_defaults() + # Clean up skills removed from SKILL_IMPLEMENTATIONS — rows are seeded + # with INSERT OR IGNORE so they survive when the default-set is trimmed. + await self._remove_orphan_skills() async def _seed_defaults(self): """Seed the default skill set.""" @@ -794,6 +797,29 @@ async def _seed_defaults(self): ) await self._db.commit() + async def _remove_orphan_skills(self) -> None: + """Delete seeded skills that no longer have an implementation. + + Built-in skills are seeded with INSERT OR IGNORE on every startup, + so removing a skill from the default set does not delete its row + on existing installs. This method cleans up known-orphaned rows + so agents that had the skill assigned are no longer advertised a + tool that will 501 on call. + """ + if self._db is None: + return + # The list is deliberately explicit — each entry documents *when* + # the skill was removed and why (issue #1923 notes/todo split). + _ORPHAN_SKILL_IDS: list[str] = [ + "notes_set_done", # removed 2026-07 — replaced by todo_set_done (#1923) + ] + for skill_id in _ORPHAN_SKILL_IDS: + await self._db.execute("DELETE FROM skills WHERE id = ?", (skill_id,)) + await self._db.execute( + "DELETE FROM agent_skills WHERE skill_id = ?", (skill_id,) + ) + await self._db.commit() + async def list_skills(self, category: str | None = None) -> list[dict]: assert self._db is not None if category: diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 68068b852..60d3f5a13 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -135,7 +135,7 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: if doc.get("archived_at") is not None: return {"error": "list is archived"} - item = await store.add_item(list_id, text, author=agent_name) + item = await store.add_item(list_id, text, author=owner_user_id) try: from tinyagentos.todo.notify import _trigger_todo_agent_notifications From bbad64e8a1525eebc79761f45d9ff464484f1ddc Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Tue, 28 Jul 2026 17:58:53 +0200 Subject: [PATCH 11/21] test(todo): assert field whitelist in real-store registry-hit test --- tests/todo/test_todo_tools.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index b720f1ac3..502d86736 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -561,6 +561,11 @@ async def test_registry_hit_uses_store_user_id(store, tmp_path): # Registry overrides caller-supplied owner_user_id → user-1 owns the list assert "lists" in res assert any(d["id"] == doc["id"] for d in res["lists"]) + # Internal fields must be stripped from the real-store path too. + for d in res["lists"]: + assert "owner_user_id" not in d + assert "archived_at" not in d + assert "created_at" not in d finally: await reg.close() From f81af1910b1de1a95b337efd2623b0861d87c582 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Tue, 28 Jul 2026 18:11:18 +0200 Subject: [PATCH 12/21] fix(skills): migrate orphan skill assignments before deleting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _remove_orphan_skills previously deleted agent_skills rows for notes_set_done without migrating them to todo_set_done. Agents that had completion access via notes_set_done silently lost it after startup. Now the method uses an _ORPHAN_REPLACEMENTS dict that maps each orphaned skill id to its replacement. Before deleting, it copies agent assignments to the replacement via INSERT OR IGNORE — if the agent already has the replacement, the existing newer assignment is preserved; otherwise the legacy assignment is migrated so access is not lost. --- tinyagentos/skills.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/tinyagentos/skills.py b/tinyagentos/skills.py index 6a1288173..08c374e4d 100644 --- a/tinyagentos/skills.py +++ b/tinyagentos/skills.py @@ -805,18 +805,34 @@ async def _remove_orphan_skills(self) -> None: on existing installs. This method cleans up known-orphaned rows so agents that had the skill assigned are no longer advertised a tool that will 501 on call. + + Before deleting an orphan skill, any agent assignments are migrated + to the replacement skill (INSERT OR IGNORE) so agents do not + silently lose tool access after startup. """ if self._db is None: return - # The list is deliberately explicit — each entry documents *when* - # the skill was removed and why (issue #1923 notes/todo split). - _ORPHAN_SKILL_IDS: list[str] = [ - "notes_set_done", # removed 2026-07 — replaced by todo_set_done (#1923) - ] - for skill_id in _ORPHAN_SKILL_IDS: - await self._db.execute("DELETE FROM skills WHERE id = ?", (skill_id,)) + # Keys: old (orphaned) skill id → replacement skill id. + # Entries document *when* the skill was removed and why. + _ORPHAN_REPLACEMENTS: dict[str, str] = { + "notes_set_done": "todo_set_done", # removed 2026-07, #1923 notes/todo split + } + for old_id, new_id in _ORPHAN_REPLACEMENTS.items(): + # Migrate agents from the orphan skill to its replacement. + # INSERT OR IGNORE: if the agent already has the replacement, + # keep the existing (newer) assignment; only fill in for agents + # that would otherwise lose completion access. + await self._db.execute( + """INSERT OR IGNORE INTO agent_skills (agent_id, skill_id, enabled, config) + SELECT agent_id, ?, enabled, config + FROM agent_skills + WHERE skill_id = ?""", + (new_id, old_id), + ) + # Now safe to remove the orphan rows. + await self._db.execute("DELETE FROM skills WHERE id = ?", (old_id,)) await self._db.execute( - "DELETE FROM agent_skills WHERE skill_id = ?", (skill_id,) + "DELETE FROM agent_skills WHERE skill_id = ?", (old_id,) ) await self._db.commit() From 8d2c305c1f7f3fa86e4345091df8b45e324d9953 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Thu, 30 Jul 2026 00:18:24 +0200 Subject: [PATCH 13/21] refactor(todo): extract _resolve_and_validate_owner helper, deduplicate owner resolution Extract the triplicated owner resolution + validation pattern from execute_todo_list_lists, execute_todo_add_item, and execute_todo_set_done into a shared _resolve_and_validate_owner helper. Fixes stale error messages that referenced 'owner_user_id' (removed from input schemas in an earlier commit) and now uses tool-specific error text. --- tinyagentos/tools/todo_tools.py | 65 ++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 60d3f5a13..98585273e 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -53,6 +53,26 @@ async def _resolve_owner_user_id( return None +async def _resolve_and_validate_owner( + args: dict, request: Request, tool_name: str +) -> tuple[str | None, dict | None]: + """Resolve and type-validate the owner, returning (owner_id, None) on + success or (None, error_dict) on failure.""" + owner_user_id = await _resolve_owner_user_id(args, request) + if owner_user_id is None: + return None, { + "error": ( + "unable to resolve owner: agent not found or " + "no user identity available" + ) + } + if not isinstance(owner_user_id, str) or not owner_user_id: + return None, { + "error": f"{tool_name} could not resolve a valid owner identity" + } + return owner_user_id, None + + async def execute_todo_list_lists(args: dict, request: Request) -> dict: """List non-archived todo lists the calling agent has access to. @@ -66,16 +86,11 @@ async def execute_todo_list_lists(args: dict, request: Request) -> dict: return {"error": "todo_list_lists requires an 'agent_name' string"} try: - owner_user_id = await _resolve_owner_user_id(args, request) - if owner_user_id is None: - return { - "error": ( - "unable to resolve owner: agent not found or " - "no user identity available" - ) - } - if not isinstance(owner_user_id, str) or not owner_user_id: - return {"error": "todo_list_lists requires an 'owner_user_id' string"} + owner_user_id, err = await _resolve_and_validate_owner( + args, request, "todo_list_lists" + ) + if err: + return err store = request.app.state.todo_store lists = await store.list_lists(owner_user_id) @@ -110,16 +125,11 @@ async def execute_todo_add_item(args: dict, request: Request) -> dict: return {"error": "todo_add_item requires a 'text' string"} try: - owner_user_id = await _resolve_owner_user_id(args, request) - if owner_user_id is None: - return { - "error": ( - "unable to resolve owner: agent not found or " - "no user identity available" - ) - } - if not isinstance(owner_user_id, str) or not owner_user_id: - return {"error": "todo_add_item requires an 'owner_user_id' string"} + owner_user_id, err = await _resolve_and_validate_owner( + args, request, "todo_add_item" + ) + if err: + return err store = request.app.state.todo_store @@ -174,16 +184,11 @@ async def execute_todo_set_done(args: dict, request: Request) -> dict: return {"error": "todo_set_done requires a boolean 'done'"} try: - owner_user_id = await _resolve_owner_user_id(args, request) - if owner_user_id is None: - return { - "error": ( - "unable to resolve owner: agent not found or " - "no user identity available" - ) - } - if not isinstance(owner_user_id, str) or not owner_user_id: - return {"error": "todo_set_done requires an 'owner_user_id' string"} + owner_user_id, err = await _resolve_and_validate_owner( + args, request, "todo_set_done" + ) + if err: + return err store = request.app.state.todo_store From faf9eea5791eb0cc1a060ee6951918f966715a62 Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Thu, 30 Jul 2026 10:38:35 +0200 Subject: [PATCH 14/21] chore: retrigger CI (Kilo review was infra failure on prior runs) From 9d6b6df5165a2a51f2a733a5b358ac6078bfc3be Mon Sep 17 00:00:00 2001 From: Hogne <227774406+hognek@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:58:47 +0200 Subject: [PATCH 15/21] fix(todo): bind owner auth to agent identity in no-registry fallback --- tinyagentos/tools/todo_tools.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index 98585273e..db2200c21 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -32,7 +32,13 @@ async def _resolve_owner_user_id( agent_registry = getattr(request.app.state, "agent_registry", None) agent_name = args.get("agent_name") if agent_registry is None: - # No registry available (e.g. test harness) — trust owner_user_id. + # No registry available (e.g. test harness) — prefer the + # authenticated user_id from the request when available; + # fall back to caller-supplied owner_user_id only when + # neither registry nor authenticated identity is present. + user_id = getattr(request.state, "user_id", None) + if user_id: + return user_id return args.get("owner_user_id") if not agent_name or not isinstance(agent_name, str): return None From de5ed275b45aa9a63fe27e42bc773451c82d2031 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:11:52 +0200 Subject: [PATCH 16/21] fix(todo): use authenticated identity for deployed agents not in registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the config.agents lookup fallback with direct use of request.state.user_id when the agent_registry has no row. Deployed agents (config-deployed, calling /api/skill-exec/{id}/call) are never written to agent_registry — only internal driver handles get written. The agent IS already authenticated (via local-token or session auth enforced by the middleware), so request.state.user_id is the caller's identity. No config.agents check is needed; simpler and more robust. Closes jaylfc blocker on PR #2035. --- tests/todo/test_todo_tools.py | 45 ++++++++++++--------------------- tinyagentos/tools/todo_tools.py | 23 +++++++---------- 2 files changed, 25 insertions(+), 43 deletions(-) diff --git a/tests/todo/test_todo_tools.py b/tests/todo/test_todo_tools.py index 502d86736..751892ec0 100644 --- a/tests/todo/test_todo_tools.py +++ b/tests/todo/test_todo_tools.py @@ -344,15 +344,15 @@ async def test_list_lists_binds_agent_to_owner_via_registry(store): @pytest.mark.asyncio -async def test_list_lists_rejects_agent_not_in_registry_or_config(store): - """Agent not in registry AND not in config → error.""" +async def test_list_lists_rejects_agent_not_in_registry_no_user_id(store): + """Agent not in registry and no user_id → error.""" registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - # No config on state → find_agent fallback won't fire + # No user_id on state req = _make_request(store, agent_registry=registry) res = await execute_todo_list_lists( - {"agent_name": "unknown", "owner_user_id": "user-1"}, req + {"agent_name": "unknown"}, req ) assert "error" in res assert "not found" in res["error"] @@ -360,15 +360,12 @@ async def test_list_lists_rejects_agent_not_in_registry_or_config(store): @pytest.mark.asyncio async def test_list_lists_deployed_agent_fallback(store): - """Agent not in registry but found in config → uses request.state.user_id.""" + """Agent not in registry but authenticated → uses request.state.user_id.""" doc = await store.create_list("user-1", "Deployed Agent's List") registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] - req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + req = _make_request(store, agent_registry=registry, user_id="user-1") res = await execute_todo_list_lists( {"agent_name": "deployed-agent"}, req @@ -379,14 +376,12 @@ async def test_list_lists_deployed_agent_fallback(store): @pytest.mark.asyncio async def test_list_lists_rejects_deployed_agent_no_user_id(store): - """Agent in config but request.state has no user_id → error.""" + """Agent not in registry and no user_id → error.""" registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] # No user_id on state - req = _make_request(store, config=mock_config, agent_registry=registry) + req = _make_request(store, agent_registry=registry) res = await execute_todo_list_lists( {"agent_name": "deployed-agent"}, req ) @@ -396,15 +391,13 @@ async def test_list_lists_rejects_deployed_agent_no_user_id(store): @pytest.mark.asyncio async def test_add_item_deployed_agent_fallback(store): - """Agent not in registry but found in config → uses request.state.user_id.""" + """Agent not in registry but authenticated → uses request.state.user_id.""" doc = await store.create_list("user-1", "Deployed Agent's List") registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] - req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + req = _make_request(store, agent_registry=registry, user_id="user-1") res = await execute_todo_add_item( {"agent_name": "deployed-agent", "list_id": doc["id"], "text": "test item"}, @@ -415,16 +408,14 @@ async def test_add_item_deployed_agent_fallback(store): @pytest.mark.asyncio async def test_add_item_rejects_deployed_agent_no_user_id(store): - """Agent in config but request.state has no user_id → error.""" + """Agent not in registry and no user_id → error.""" doc = await store.create_list("user-1", "Deployed Agent's List") registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] # No user_id on state - req = _make_request(store, config=mock_config, agent_registry=registry) + req = _make_request(store, agent_registry=registry) res = await execute_todo_add_item( {"agent_name": "deployed-agent", "list_id": doc["id"], "text": "test item"}, req, @@ -435,16 +426,14 @@ async def test_add_item_rejects_deployed_agent_no_user_id(store): @pytest.mark.asyncio async def test_set_done_deployed_agent_fallback(store): - """Agent not in registry but found in config → uses request.state.user_id.""" + """Agent not in registry but authenticated → uses request.state.user_id.""" doc = await store.create_list("user-1", "Tasks") item = await store.add_item(doc["id"], "A task", author="user-1") registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] - req = _make_request(store, config=mock_config, agent_registry=registry, user_id="user-1") + req = _make_request(store, agent_registry=registry, user_id="user-1") res = await execute_todo_set_done( {"agent_name": "deployed-agent", "list_id": doc["id"], @@ -456,17 +445,15 @@ async def test_set_done_deployed_agent_fallback(store): @pytest.mark.asyncio async def test_set_done_rejects_deployed_agent_no_user_id(store): - """Agent in config but request.state has no user_id → error.""" + """Agent not in registry and no user_id → error.""" doc = await store.create_list("user-1", "Tasks") item = await store.add_item(doc["id"], "A task", author="user-1") registry = MagicMock() registry.get_by_handle = AsyncMock(return_value=None) - mock_config = MagicMock() - mock_config.agents = [{"name": "deployed-agent"}] # No user_id on state - req = _make_request(store, config=mock_config, agent_registry=registry) + req = _make_request(store, agent_registry=registry) res = await execute_todo_set_done( {"agent_name": "deployed-agent", "list_id": doc["id"], "item_id": item["id"], "done": True}, diff --git a/tinyagentos/tools/todo_tools.py b/tinyagentos/tools/todo_tools.py index db2200c21..4d8b777d8 100644 --- a/tinyagentos/tools/todo_tools.py +++ b/tinyagentos/tools/todo_tools.py @@ -24,10 +24,7 @@ async def _resolve_owner_user_id( When the agent registry is absent from ``request.app.state``, falls back to the ``owner_user_id`` in *args* for test compatibility. - For deployed agents that are *not* in the registry (the common production - case), the agent is looked up in ``config.agents`` and the owner is taken - from the authenticated request's ``user_id`` (set by the local-token auth - middleware). + For deployed agents that are *not* in the registry (the common production\n case), the authenticated ``user_id`` from the request (set by the\n local-token or session auth middleware) is used directly. The agent\n IS authenticated — it just does not have a registry row. """ agent_registry = getattr(request.app.state, "agent_registry", None) agent_name = args.get("agent_name") @@ -46,16 +43,14 @@ async def _resolve_owner_user_id( if agent is not None: return agent.get("user_id") # Deployed agents are never written to agent_registry. When the - # registry exists but has no row for this handle, look the agent up in - # config; if it is a known (config-deployed) agent, the owner is the - # authenticated user (local-token auth, primary user). - config = getattr(request.app.state, "config", None) - if config is not None: - from tinyagentos.agent_db import find_agent - - deployed = find_agent(config, agent_name) - if deployed is not None: - return getattr(request.state, "user_id", None) + # registry exists but has no row for this handle, fall back to the + # authenticated identity from the request. The agent IS authenticated + # (via local-token or session auth, enforced by the middleware) — it + # just does not have a registry row. No config.agents check is needed; + # the authenticated user_id is the caller's identity. + user_id = getattr(request.state, "user_id", None) + if user_id: + return user_id return None From a9b52aa27684ec8ba9a2472e38ea155466621d6b Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Tue, 4 Aug 2026 14:51:26 +0200 Subject: [PATCH 17/21] chore: retrigger CI (doc-gate clean locally, deleted-symbols check stale) --- changelog.d/2035-todo-agent-tools.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog.d/2035-todo-agent-tools.md diff --git a/changelog.d/2035-todo-agent-tools.md b/changelog.d/2035-todo-agent-tools.md new file mode 100644 index 000000000..6da3f21f9 --- /dev/null +++ b/changelog.d/2035-todo-agent-tools.md @@ -0,0 +1,7 @@ +### Added + +- Agent-accessible todo-list tools: `list_lists`, `list_list_items`, `add_todo`, `update_todo`, `delete_todo` (#2035). + +### Removed + +- `notes_set_done` agent tool superseded by the richer todo tools above (#2035). \ No newline at end of file From 70ea9cf6cf8a2ace1bd990558d58b2a0dc7de06e Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Mon, 17 Aug 2026 12:15:50 +0200 Subject: [PATCH 18/21] =?UTF-8?q?docs:=20doc-gate=20waiver=20=E2=80=94=20t?= =?UTF-8?q?odo=20tool=20split=20from=20notes=5Fset=5Fdone=20(#2035)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Docs-Reviewed: replaces the notes_set_done skill/tool with todo_list_lists / todo_add_item / todo_set_done (the done concept moves from shared docs to the Todo store). The agent-facing tool list is documented in docs/agent-manual/09-os-control.md and docs/taos-agent-manual.md, both updated in this PR; agent-coordination.md documents the coordination protocol, not the skill tool catalogue. From 8b54b2c1146d0fe0a8969c460df1d7e478aacd39 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Mon, 17 Aug 2026 13:42:31 +0200 Subject: [PATCH 19/21] docs: trim todo-tool prose to fit compiled-manual size budget (#2035) The three todo-tool entries pushed the compiled agent manual to 18139 chars, over the 18000 limit (test_compiled_size_under_limit). Tighten to terse one-liners matching the notes entries; full arg schemas live in skills.py. --- docs/agent-manual/09-os-control.md | 6 +++--- docs/taos-agent-manual.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/agent-manual/09-os-control.md b/docs/agent-manual/09-os-control.md index a6cca22cd..700c33c4e 100644 --- a/docs/agent-manual/09-os-control.md +++ b/docs/agent-manual/09-os-control.md @@ -12,9 +12,9 @@ Tools available to you: - **generate_image** — make an image from a text prompt. Args: `prompt` (required) plus the optional parameters in Image Prompting below. Returns an `image_ref` for `canvas_add_image` or `export_storybook`. - **notes_list_shared_docs** — list shared docs you belong to. - **notes_add_entry** — append to a shared doc. Args: `doc_id`, `text`. -- **todo_list_lists** — list your todo lists. Returns `id`, `title`, `updated_at`. -- **todo_add_item** — append an item to a todo list. Args: `list_id`, `text`. -- **todo_set_done** — mark a todo item done or not done. Args: `list_id`, `item_id`, `done`. +- **todo_list_lists** — list todo lists. +- **todo_add_item** — add an item. +- **todo_set_done** — mark done. A typical flow: open Projects, create_project, add tasks, generate_image then canvas_add_image, export_storybook. diff --git a/docs/taos-agent-manual.md b/docs/taos-agent-manual.md index 0108c7093..1703312fc 100644 --- a/docs/taos-agent-manual.md +++ b/docs/taos-agent-manual.md @@ -176,9 +176,9 @@ Tools available to you: - **generate_image** — make an image from a text prompt. Args: `prompt` (required) plus the optional parameters in Image Prompting below. Returns an `image_ref` for `canvas_add_image` or `export_storybook`. - **notes_list_shared_docs** — list shared docs you belong to. - **notes_add_entry** — append to a shared doc. Args: `doc_id`, `text`. -- **todo_list_lists** — list your todo lists. Returns `id`, `title`, `updated_at`. -- **todo_add_item** — append an item to a todo list. Args: `list_id`, `text`. -- **todo_set_done** — mark a todo item done or not done. Args: `list_id`, `item_id`, `done`. +- **todo_list_lists** — list todo lists. +- **todo_add_item** — add an item. +- **todo_set_done** — mark done. A typical flow: open Projects, create_project, add tasks, generate_image then canvas_add_image, export_storybook. From e63049cee1ac89f201f26772a340ef699ffb79f5 Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:02:39 +0200 Subject: [PATCH 20/21] docs: fix todo tool names in changelog fragment (#2035) --- changelog.d/2035-todo-agent-tools.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/changelog.d/2035-todo-agent-tools.md b/changelog.d/2035-todo-agent-tools.md index 6da3f21f9..b515da3ab 100644 --- a/changelog.d/2035-todo-agent-tools.md +++ b/changelog.d/2035-todo-agent-tools.md @@ -1,7 +1,7 @@ ### Added -- Agent-accessible todo-list tools: `list_lists`, `list_list_items`, `add_todo`, `update_todo`, `delete_todo` (#2035). +- Agent-accessible todo-list tools: `todo_list_lists`, `todo_add_item`, `todo_set_done` (#2035). ### Removed -- `notes_set_done` agent tool superseded by the richer todo tools above (#2035). \ No newline at end of file +- `notes_set_done` agent tool superseded by the richer todo tools above (#2035). From c96ed26e07f848f252a0922d0e15c127b655727b Mon Sep 17 00:00:00 2001 From: hognek <227774406+hognek@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:08:10 +0200 Subject: [PATCH 21/21] fix(skills): drop notes-scoped config when migrating notes_set_done -> todo_set_done (#2035) --- tinyagentos/skills.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tinyagentos/skills.py b/tinyagentos/skills.py index 08c374e4d..f03f633d0 100644 --- a/tinyagentos/skills.py +++ b/tinyagentos/skills.py @@ -824,7 +824,7 @@ async def _remove_orphan_skills(self) -> None: # that would otherwise lose completion access. await self._db.execute( """INSERT OR IGNORE INTO agent_skills (agent_id, skill_id, enabled, config) - SELECT agent_id, ?, enabled, config + SELECT agent_id, ?, enabled, '{}' FROM agent_skills WHERE skill_id = ?""", (new_id, old_id),