A task's kind is correctable — the Kind select stops lying (#3129)

PR #132 was rebase-merged, so main carries rewritten copies of dev's
milestone-312 commits under new SHAs. The merge base stayed at 02c1e37, so
a dev->main merge tried to replay all eight already-landed commits and
collided in the two test files both sides had touched.

Resolved by taking dev's version of each: dev's copy is main's content plus
the #3129 additions, verified as a strict superset before resolving rather
than assumed. The merged tree is identical to dev's — asserted below, not
eyeballed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

# Conflicts:
#	tests/test_integration_task_kind_spike.py
#	tests/test_mcp_tool_tasks_kind.py

frontend/src/stores/tasks.ts

@@ -55,6 +55,9 @@ export const useTasksStore = defineStore("tasks", () => {
 
   async function updateTask(
     id: number,
+    // IssueFields carries `kind`, which the PATCH route now reads. It has
+    // always been SENT by the task editor; until #3129 the route dropped it
+    // and the save reported success while changing nothing.
     data: Partial<
       Pick<Task, "title" | "body" | "tags" | "status" | "priority" | "due_date" | "project_id" | "milestone_id" | "parent_id" | "recurrence_rule">
     > & IssueFields

src/scribe/mcp/tools/tasks.py

@@ -23,6 +23,11 @@ from scribe.mcp.tools import systems as systems_tools
 from scribe.services import access as access_svc
 from scribe.services import dedup as dedup_svc
 from scribe.services import notes as notes_svc
+# Imported by NAME, not reached through notes_svc: minted_kind is pure
+# validation, not a service call, and a test that stubs the service module to
+# avoid the database would otherwise stub the validation too — turning a
+# guard into a MagicMock that approves anything.
+from scribe.services.notes import minted_kind
 from scribe.services import planning as planning_svc
 from scribe.services import rulebooks as rulebooks_svc
 from scribe.services import systems as systems_svc
@@ -194,7 +199,7 @@ async def create_task(
         milestone_id=milestone_id or None,
         parent_id=parent_id or None,
         tags=tags,
-        task_kind=kind,
+        task_kind=minted_kind(kind),
         arose_from_id=arose_from_id or None,
     )
     if system_ids:
@@ -214,6 +219,7 @@ async def update_task(
     milestone_id: int = 0,
     system_ids: list[int] | None = None,
     arose_from_id: int = 0,
+    kind: str = "",
 ) -> dict:
     """Update an existing Scribe task. Only explicitly provided fields are changed.
 
@@ -233,6 +239,12 @@ async def update_task(
             (set-semantics). None = leave unchanged; [] = clear all.
         arose_from_id: Provenance (issue → originating task). 0 = leave unchanged,
             -1 = clear, positive = set.
+        kind: Re-file this task as 'work', 'issue' or 'spike'. Omit (empty) to
+            leave unchanged. Correcting a kind is ordinary — what a task turns
+            out to BE is often clear only once it is under way, and a piece of
+            work that becomes an investigation should say so. 'plan' is
+            refused: plans are milestones (start_planning), and the value
+            survives only so historical plan-tasks stay writable.
     """
     uid = current_user_id()
     fields: dict = {}
@@ -258,6 +270,8 @@ async def update_task(
         fields["arose_from_id"] = None
     elif arose_from_id:
         fields["arose_from_id"] = arose_from_id
+    if kind:
+        fields["task_kind"] = minted_kind(kind)
     note = await notes_svc.update_note(uid, task_id, **fields)
     if note is None:
         raise ValueError(f"task {task_id} not found")

src/scribe/models/note.py

@@ -23,6 +23,24 @@ class TaskPriority(str, enum.Enum):
     high = "high"
 
 
+class TaskKind(str, enum.Enum):
+    """What KIND of work a task is. Mirrors CHECK notes_task_kind_check.
+
+    Every value the COLUMN may hold, including `plan`. That is deliberate:
+    plans became milestones in 0066, but historical plan-tasks still carry
+    the value and must stay readable and writable. Refusing to MINT a new
+    plan is a door policy (see the create/update task tools), not a
+    statement about what the column accepts — conflating the two would make
+    old rows unwritable, which is how a retired value turns into corrupt
+    data.
+    """
+
+    work = "work"
+    issue = "issue"
+    spike = "spike"
+    plan = "plan"
+
+
 class Note(Base, TimestampMixin, SoftDeleteMixin):
     __tablename__ = "notes"
 

src/scribe/routes/tasks.py

@@ -13,6 +13,7 @@ from scribe.services.notes import (
     list_notes,
     update_note,
 )
+from scribe.services.notes import minted_kind as notes_minted_kind
 from scribe.services.note_usage import record_pulled
 from scribe.services.planning import start_planning as svc_start_planning
 from scribe.services.recurrence import calculate_next_due, validate_recurrence_rule
@@ -239,6 +240,16 @@ async def update_task_route(task_id: int):
     if "tags" in data:
         fields["tags"] = data["tags"]
 
+    # Re-filing a task's kind. The editor has always SENT this field and the
+    # route silently dropped it — reporting "Task saved" and reverting on the
+    # next load (#3129). Validated at the same layer as status and priority so
+    # an unrecognised value is a 400 rather than a database CHECK violation.
+    if "kind" in data:
+        try:
+            fields["task_kind"] = notes_minted_kind(data["kind"])
+        except ValueError as exc:
+            return jsonify({"error": str(exc)}), 400
+
     for key in ("project_id", "milestone_id", "parent_id", "arose_from_id"):
         if key in data:
             fields[key] = data[key]

src/scribe/services/notes.py

@@ -5,7 +5,7 @@ from datetime import date, datetime, timezone
 from sqlalchemy import func, or_, select, text
 
 from scribe.models import async_session
-from scribe.models.note import Note, TaskPriority, TaskStatus
+from scribe.models.note import Note, TaskKind, TaskPriority, TaskStatus
 
 logger = logging.getLogger(__name__)
 
@@ -366,6 +366,37 @@ async def get_or_create_note_by_title(user_id: int, title: str) -> Note:
     return await create_note(user_id, title=title)
 
 
+# Kinds a caller may MINT. Narrower than what the COLUMN holds: `plan` is a
+# valid stored value — historical plan-tasks carry it and must stay writable —
+# but plans became milestones in 0066, so no door hands out a new one. The
+# CHECK whitelist and this policy answer different questions, which is why
+# they are deliberately not the same list.
+MINTABLE_KINDS = ("work", "issue", "spike")
+
+
+def minted_kind(kind: str) -> str:
+    """Validate a kind a caller is asking to WRITE, or raise saying why.
+
+    Lives here rather than in either door so both share one copy: the REST
+    route cannot import an MCP tool module, and a second spelling of this
+    list is how the two doors would come to disagree.
+
+    Raises rather than falling back to 'work'. A silently-corrected kind is
+    the defect this exists to end (#3129: the editor's Kind select reported
+    success and changed nothing), and a caller naming a kind we do not know
+    has a wrong idea that an error corrects and a default hides.
+    """
+    if kind in MINTABLE_KINDS:
+        return kind
+    if kind == "plan":
+        raise ValueError(
+            "kind='plan' is retired — plans are milestones. Call "
+            "start_planning(project_id, title) to begin one. Existing "
+            "plan-tasks keep the value and stay editable."
+        )
+    raise ValueError(f"kind must be one of {MINTABLE_KINDS}, got {kind!r}")
+
+
 async def update_note(user_id: int, note_id: int, **fields: object) -> Note | None:
     async with async_session() as session:
         result = await session.execute(
@@ -391,6 +422,20 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No
                     value = TaskPriority(value).value
                 except ValueError:
                     raise ValueError(f"Invalid priority: {value!r}. Must be one of: {[p.value for p in TaskPriority]}")
+            elif key == "task_kind" and isinstance(value, str):
+                # Same shape as status/priority above, and for the same
+                # reason: a kind the column will refuse should fail here with
+                # a readable message, not as a CheckViolationError from the
+                # database. Before this, `task_kind` reached setattr through
+                # the hasattr guard with no validation at all — but no door
+                # ever offered it, so a task's kind was write-once (#3129).
+                try:
+                    value = TaskKind(value).value
+                except ValueError:
+                    raise ValueError(
+                        f"Invalid kind: {value!r}. Must be one of: "
+                        f"{[k.value for k in TaskKind]}"
+                    )
             elif key == "tags" and isinstance(value, list):
                 value = _normalize_tags(value)
             setattr(note, key, value)

tests/test_integration_task_kind_spike.py

@@ -72,3 +72,30 @@ async def test_an_unknown_kind_is_still_refused(owner_id):
     """
     with pytest.raises(IntegrityError):
         await _write(owner_id, "investigation")
+
+
+async def test_a_kind_can_be_corrected_after_the_fact(owner_id):
+    """The write-once bug, asserted against a real column (#3129).
+
+    The read-back is the whole test. A version that only asserted the update
+    call succeeded would have passed against the broken code — the route
+    returned 200 while dropping the field, which is exactly how this survived
+    long enough to be found by hand.
+    """
+    from scribe.services import notes as notes_svc
+
+    note_id = await _write(owner_id, "work")
+    await notes_svc.update_note(owner_id, note_id, task_kind="spike")
+    async with async_session() as s:
+        assert (await s.get(Note, note_id)).task_kind == "spike"
+
+
+async def test_an_invalid_kind_is_refused_before_the_database(owner_id):
+    """A readable ValueError, not a CheckViolationError surfacing as a 500."""
+    from scribe.services import notes as notes_svc
+
+    note_id = await _write(owner_id, "work")
+    with pytest.raises(ValueError, match="Invalid kind"):
+        await notes_svc.update_note(owner_id, note_id, task_kind="investigation")
+    async with async_session() as s:
+        assert (await s.get(Note, note_id)).task_kind == "work"

tests/test_mcp_tool_tasks_kind.py

@@ -47,3 +47,53 @@ async def test_create_task_passes_spike():
         from scribe.mcp.tools.tasks import create_task
         await create_task(title="Can the runner be given a bash shell?", kind="spike")
     assert mock.call_args.kwargs["task_kind"] == "spike"
+
+
+@pytest.mark.asyncio
+async def test_update_task_can_re_file_a_kind():
+    """A task's kind must be CORRECTABLE, not write-once (#3129).
+
+    What a piece of work turns out to be is often clear only once it is under
+    way. Before this, `update_task` had no `kind` parameter at all and the
+    REST route dropped the field the editor was already sending — the save
+    reported success and reverted on reload.
+    """
+    mock = AsyncMock(return_value=fake_note(task_kind="spike"))
+    with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock):
+        from scribe.mcp.tools.tasks import update_task
+        await update_task(task_id=1, kind="spike")
+    assert mock.call_args.kwargs["task_kind"] == "spike"
+
+
+@pytest.mark.asyncio
+async def test_update_task_leaves_kind_alone_when_not_given():
+    """"" means leave unchanged, as it does for every other field here."""
+    mock = AsyncMock(return_value=fake_note())
+    with patch("scribe.mcp.tools.tasks.notes_svc.update_note", mock):
+        from scribe.mcp.tools.tasks import update_task
+        await update_task(task_id=1, title="renamed")
+    assert "task_kind" not in mock.call_args.kwargs
+
+
+@pytest.mark.asyncio
+async def test_minting_a_plan_is_refused_at_both_doors():
+    """`plan` is a valid stored value and NOT a mintable one.
+
+    Historical plan-tasks carry it and must stay writable, so the column
+    keeps accepting it — but plans became milestones in 0066, so no door
+    hands out a new one. The error names start_planning rather than just
+    refusing, because a caller reaching for kind='plan' wants a plan.
+    """
+    from scribe.services.notes import minted_kind
+
+    with pytest.raises(ValueError, match="start_planning"):
+        minted_kind("plan")
+
+
+@pytest.mark.asyncio
+async def test_an_unknown_kind_raises_rather_than_defaulting():
+    """A silently-corrected kind is the defect this whole fix exists to end."""
+    from scribe.services.notes import minted_kind
+
+    with pytest.raises(ValueError, match="kind must be one of"):
+        minted_kind("investigation")