feat(mcp): milestone_id=-1 clears a task's milestone (update_task)
Optional FKs on update_task previously had no way to express 'remove' — 0 meant leave-unchanged and any positive int meant set, so a milestone (or project) could only be cleared via the web UI. Now -1 clears the FK (NULL); clearing project_id also clears milestone_id since a milestone can't outlive its project. update_note already NULLs on None, so the change is confined to the tool wrapper. Closes scribe task #586. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -36,7 +36,8 @@ Mechanics:
|
||||
- Tags are plain strings (no `#` prefix). Empty list clears tags; omit to leave
|
||||
unchanged on updates.
|
||||
- For optional integer FKs (project_id, milestone_id, parent_id), use 0 to mean
|
||||
"not set".
|
||||
"not set". On update_task, -1 clears an existing FK (e.g. milestone_id=-1
|
||||
removes the task from its milestone); 0 leaves it unchanged.
|
||||
|
||||
Keep task state honest — this is what makes the project a trustworthy record:
|
||||
- When you begin working a task, set it to in_progress (update_task
|
||||
|
||||
@@ -14,7 +14,7 @@ Sentinels (preserved from existing fable-mcp):
|
||||
what makes a Note a Task)
|
||||
- priority="none" sets explicit no-priority; priority="" is "leave unchanged"
|
||||
- project_id=0 / milestone_id=0 / parent_id=0 → "no association" on create,
|
||||
"leave unchanged" on update
|
||||
"leave unchanged" on update; on update, -1 clears the FK (sets it NULL)
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
@@ -147,8 +147,10 @@ async def update_task(
|
||||
the lifecycle: set in_progress when you start, done when complete —
|
||||
don't leave finished work at todo.
|
||||
priority: New priority — one of: none, low, medium, high.
|
||||
project_id: New project. Omit (0) to leave unchanged.
|
||||
milestone_id: New milestone. Omit (0) to leave unchanged.
|
||||
project_id: New project. 0 = leave unchanged, -1 = clear (remove from
|
||||
its project; also clears the milestone), positive = set.
|
||||
milestone_id: New milestone. 0 = leave unchanged, -1 = clear (remove
|
||||
from its milestone), positive = set.
|
||||
"""
|
||||
uid = current_user_id()
|
||||
fields: dict = {}
|
||||
@@ -160,9 +162,15 @@ async def update_task(
|
||||
fields["status"] = status
|
||||
if priority:
|
||||
fields["priority"] = priority
|
||||
if project_id:
|
||||
# Optional FKs: 0 = leave unchanged, -1 = clear (set NULL), positive = set.
|
||||
if project_id == -1:
|
||||
fields["project_id"] = None
|
||||
fields["milestone_id"] = None # a milestone can't outlive its project
|
||||
elif project_id:
|
||||
fields["project_id"] = project_id
|
||||
if milestone_id:
|
||||
if milestone_id == -1:
|
||||
fields["milestone_id"] = None
|
||||
elif milestone_id:
|
||||
fields["milestone_id"] = milestone_id
|
||||
note = await notes_svc.update_note(uid, task_id, **fields)
|
||||
if note is None:
|
||||
|
||||
@@ -164,6 +164,46 @@ async def test_update_task_raises_when_not_found():
|
||||
await update_task(task_id=999, status="done")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_task_milestone_zero_is_omitted():
|
||||
"""milestone_id=0 is 'leave unchanged' — must not reach the service."""
|
||||
fake = _fake_task()
|
||||
mock = AsyncMock(return_value=fake)
|
||||
with patch("fabledassistant.mcp.tools.tasks.notes_svc.update_note", mock):
|
||||
await update_task(task_id=1, milestone_id=0)
|
||||
assert "milestone_id" not in mock.call_args.kwargs
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_task_milestone_positive_is_set():
|
||||
fake = _fake_task()
|
||||
mock = AsyncMock(return_value=fake)
|
||||
with patch("fabledassistant.mcp.tools.tasks.notes_svc.update_note", mock):
|
||||
await update_task(task_id=1, milestone_id=42)
|
||||
assert mock.call_args.kwargs["milestone_id"] == 42
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_task_milestone_negative_one_clears():
|
||||
"""milestone_id=-1 clears the milestone (sets the column NULL)."""
|
||||
fake = _fake_task()
|
||||
mock = AsyncMock(return_value=fake)
|
||||
with patch("fabledassistant.mcp.tools.tasks.notes_svc.update_note", mock):
|
||||
await update_task(task_id=1, milestone_id=-1)
|
||||
assert mock.call_args.kwargs["milestone_id"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_update_task_clearing_project_also_clears_milestone():
|
||||
"""project_id=-1 clears the project and, with it, the milestone."""
|
||||
fake = _fake_task()
|
||||
mock = AsyncMock(return_value=fake)
|
||||
with patch("fabledassistant.mcp.tools.tasks.notes_svc.update_note", mock):
|
||||
await update_task(task_id=1, project_id=-1)
|
||||
assert mock.call_args.kwargs["project_id"] is None
|
||||
assert mock.call_args.kwargs["milestone_id"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_add_task_log_returns_log_dict():
|
||||
log = MagicMock()
|
||||
|
||||
Reference in New Issue
Block a user