diff --git a/src/fabledassistant/services/tools/notes.py b/src/fabledassistant/services/tools/notes.py index b9b3144..36b2400 100644 --- a/src/fabledassistant/services/tools/notes.py +++ b/src/fabledassistant/services/tools/notes.py @@ -27,7 +27,8 @@ logger = logging.getLogger(__name__) ), parameters={ "title": {"type": "string", "description": "The title"}, - "body": {"type": "string", "description": "Content in markdown"}, + "body": {"type": "string", "description": "Content in markdown. NOTE: when status is set (creating a task), body is ignored — task bodies are auto-maintained from work logs. Use `description` to provide the goal/context for tasks."}, + "description": {"type": "string", "description": "User-stated goal or initial context for a task. Read-only context for the auto-summary pipeline. Ignored when status is omitted (knowledge note)."}, "tags": {"type": "array", "items": {"type": "string"}, "description": 'Tags (without # prefix, hyphens for multi-word: ["science-fiction", "story/idea"]). Do NOT embed #tags in the body.'}, "project": {"type": "string", "description": "Optional project name. Only set this if the user explicitly named a project. Do NOT infer a project from the content or context."}, "status": {"type": "string", "enum": ["todo", "in_progress", "done", "cancelled"], "description": "Set to 'todo' to create a task. Omit entirely for a knowledge note."}, @@ -43,6 +44,7 @@ logger = logging.getLogger(__name__) async def create_note_tool(*, user_id, arguments, **_ctx): title = arguments.get("title", "Untitled") body = arguments.get("body", "") + description = arguments.get("description") tags = arguments.get("tags", []) if not isinstance(title, str): return {"success": False, "error": "title must be a string. Call create_note once per item."} @@ -54,6 +56,11 @@ async def create_note_tool(*, user_id, arguments, **_ctx): is_task = "status" in arguments and arguments["status"] is not None status = arguments.get("status", "todo") if is_task else None + # Task bodies are auto-maintained by the consolidation pipeline; drop any + # body argument arriving with a task creation so it never lands in the DB. + if is_task: + body = "" + project_name = arguments.get("project") milestone_name = arguments.get("milestone") parent_task_name = arguments.get("parent_task") @@ -80,6 +87,7 @@ async def create_note_tool(*, user_id, arguments, **_ctx): user_id=user_id, title=title, body=body, + description=description, tags=tags, status=status, priority=arguments.get("priority", "none") if is_task else None, @@ -126,7 +134,8 @@ async def create_note_tool(*, user_id, arguments, **_ctx): description="Update an existing note or task — content, title, status, priority, or due date. Use for edits, marking tasks done, changing priority. Never use create_note for existing notes.", parameters={ "query": {"type": "string", "description": "Title or keyword to find the note or task to update"}, - "body": {"type": "string", "description": "New note content in markdown (omit if only updating task fields)"}, + "body": {"type": "string", "description": "New note content in markdown (omit if only updating task fields). REJECTED on tasks — task bodies are auto-maintained from work logs; use `log_work` to record progress or `description` to revise the goal."}, + "description": {"type": "string", "description": "Update the user-stated goal/context (tasks). Distinct from `body` (machine-maintained on tasks)."}, "title": {"type": "string", "description": "Optional new title"}, "mode": {"type": "string", "enum": ["replace", "append"], "description": "How to apply the new body: 'replace' overwrites existing content (default), 'append' adds after existing content"}, "status": {"type": "string", "enum": ["todo", "in_progress", "done", "cancelled"], "description": "New task status. Use to mark a task done, start it, cancel it, etc."}, @@ -155,6 +164,19 @@ async def update_note_tool(*, user_id, arguments, **_ctx): return {"success": False, "error": f"No note found matching '{query}'."} note = candidates[0] + # Schema-level separation: task bodies are auto-maintained from work logs. + # Reject body writes here rather than silently dropping so the LLM gets + # nudged toward log_work / description. + if note.is_task and arguments.get("body"): + return { + "success": False, + "error": ( + "Cannot write to `body` on a task — the body is auto-maintained " + "from work logs. Use the `log_work` tool to record progress, or " + "update `description` to revise the goal." + ), + } + update_fields: dict = {} if new_title: update_fields["title"] = new_title @@ -163,6 +185,8 @@ async def update_note_tool(*, user_id, arguments, **_ctx): update_fields["body"] = note.body + "\n\n" + new_body else: update_fields["body"] = new_body + if "description" in arguments: + update_fields["description"] = arguments["description"] if "status" in arguments: update_fields["status"] = arguments["status"] if "priority" in arguments: diff --git a/tests/test_tools_notes.py b/tests/test_tools_notes.py new file mode 100644 index 0000000..7512d39 --- /dev/null +++ b/tests/test_tools_notes.py @@ -0,0 +1,196 @@ +"""Tests for the create_note / update_note LLM tools. + +Verifies the new task-as-durable-record contract: +- create_note drops `body` when a task is being created (status set). +- create_note forwards `description` to the service. +- update_note rejects `body` writes when the target is a task. +- update_note accepts `description` updates on tasks. +""" +from types import SimpleNamespace +from unittest.mock import AsyncMock, patch + + +async def test_create_note_tool_ignores_body_when_creating_task(): + """Status present → is_task=True → body must be dropped before the + create_note service call; description must be forwarded.""" + from fabledassistant.services.tools import notes as notes_tool + + captured: dict = {} + + async def fake_create_note(**kwargs): + captured.update(kwargs) + return SimpleNamespace( + id=1, + title=kwargs["title"], + status=kwargs.get("status"), + priority=kwargs.get("priority"), + due_date=None, + project_id=None, + milestone_id=None, + parent_id=None, + ) + + with patch( + "fabledassistant.services.tools.notes.create_note", new=fake_create_note, + ), patch( + "fabledassistant.services.tools.notes.check_duplicate", + new=AsyncMock(return_value=None), + ), patch( + "fabledassistant.services.tools.notes.suggest_tags", + new=AsyncMock(return_value=[]), + ), patch( + "fabledassistant.services.tools.notes.schedule_embedding", + ): + await notes_tool.create_note_tool( + user_id=1, + arguments={ + "title": "renew cert", + "status": "todo", + "description": "the goal text", + "body": "this should be dropped on tasks", + }, + ) + + assert captured.get("description") == "the goal text" + # Body dropped because is_task=True. consolidation owns the body field. + assert not captured.get("body") + + +async def test_create_note_tool_preserves_body_for_knowledge_notes(): + """No status → is_task=False → body is preserved as today.""" + from fabledassistant.services.tools import notes as notes_tool + + captured: dict = {} + + async def fake_create_note(**kwargs): + captured.update(kwargs) + return SimpleNamespace(id=1, title=kwargs["title"], status=None) + + with patch( + "fabledassistant.services.tools.notes.create_note", new=fake_create_note, + ), patch( + "fabledassistant.services.tools.notes.check_duplicate", + new=AsyncMock(return_value=None), + ), patch( + "fabledassistant.services.tools.notes.suggest_tags", + new=AsyncMock(return_value=[]), + ), patch( + "fabledassistant.services.tools.notes.schedule_embedding", + ): + await notes_tool.create_note_tool( + user_id=1, + arguments={ + "title": "runbook", + "body": "preserved markdown content", + }, + ) + + assert captured.get("body") == "preserved markdown content" + + +async def test_update_note_tool_rejects_body_on_tasks(): + """When the resolved note has is_task=True, providing body must error.""" + from fabledassistant.services.tools import notes as notes_tool + + # SimpleNamespace can't fake a @property; build is_task as a real attr. + fake_task = SimpleNamespace( + id=42, title="t", status="in_progress", + body="", tags=[], project_id=None, milestone_id=None, + ) + fake_task.is_task = True + + with patch( + "fabledassistant.services.tools.notes.get_note_by_title", + new=AsyncMock(return_value=fake_task), + ), patch( + "fabledassistant.services.tools.notes.update_note", new=AsyncMock(), + ) as mock_update: + result = await notes_tool.update_note_tool( + user_id=1, + arguments={"query": "t", "body": "trying to overwrite"}, + ) + + assert result["success"] is False + err = result.get("error", "").lower() + assert "body" in err or "log_work" in err + mock_update.assert_not_awaited() + + +async def test_update_note_tool_accepts_description_on_tasks(): + """description updates flow through to the service even on tasks.""" + from fabledassistant.services.tools import notes as notes_tool + + fake_task = SimpleNamespace( + id=42, title="t", status="in_progress", + body="", tags=[], project_id=None, milestone_id=None, + description=None, + ) + fake_task.is_task = True + + captured: dict = {} + + async def fake_update_note(uid, nid, **kwargs): + captured.update(kwargs) + # Apply the description so the post-update inspection makes sense. + for k, v in kwargs.items(): + setattr(fake_task, k, v) + return fake_task + + with patch( + "fabledassistant.services.tools.notes.get_note_by_title", + new=AsyncMock(return_value=fake_task), + ), patch( + "fabledassistant.services.tools.notes.update_note", + new=fake_update_note, + ), patch( + "fabledassistant.services.tools.notes.suggest_tags", + new=AsyncMock(return_value=[]), + ), patch( + "fabledassistant.services.tools.notes.schedule_embedding", + ): + result = await notes_tool.update_note_tool( + user_id=1, + arguments={"query": "t", "description": "updated goal"}, + ) + + assert result["success"] is True + assert captured.get("description") == "updated goal" + + +async def test_update_note_tool_accepts_body_on_knowledge_notes(): + """Body writes are still allowed on non-task notes.""" + from fabledassistant.services.tools import notes as notes_tool + + fake_note = SimpleNamespace( + id=10, title="n", status=None, + body="old", tags=[], project_id=None, milestone_id=None, + ) + fake_note.is_task = False + + captured: dict = {} + + async def fake_update_note(uid, nid, **kwargs): + captured.update(kwargs) + for k, v in kwargs.items(): + setattr(fake_note, k, v) + return fake_note + + with patch( + "fabledassistant.services.tools.notes.get_note_by_title", + new=AsyncMock(return_value=fake_note), + ), patch( + "fabledassistant.services.tools.notes.update_note", + new=fake_update_note, + ), patch( + "fabledassistant.services.tools.notes.suggest_tags", + new=AsyncMock(return_value=[]), + ), patch( + "fabledassistant.services.tools.notes.schedule_embedding", + ): + result = await notes_tool.update_note_tool( + user_id=1, + arguments={"query": "n", "body": "new body content"}, + ) + + assert result["success"] is True + assert captured.get("body") == "new body content"