feat(tools): tasks accept description; reject body writes via tools
create_note tool: - New 'description' parameter accepted and forwarded to the service. - When status is set (creating a task), 'body' is dropped before the service call. Task bodies are owned by the consolidation pipeline. update_note tool: - New 'description' parameter; routed through update_fields. - When the resolved target has is_task=True and 'body' is in the arguments, the call errors with a message nudging toward log_work or description. Knowledge notes are unaffected. HTTP routes (POST/PATCH/PUT /api/notes) accept body freely — the restriction is only at the LLM tool layer.
This commit is contained in:
@@ -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"
|
||||
Reference in New Issue
Block a user