Feature/enhance item update logic

backend/checklists.go

@@ -49,10 +49,20 @@ func (s *scanner) str(col string) string {
 	if !ok || i >= len(s.row) || s.row[i] == nil {
 		return ""
 	}
-	if v, ok := s.row[i].(string); ok {
+	switch v := s.row[i].(type) {
+	case string:
 		return v
+	case *string:
+		// updateItem re-inserts nullable columns as *string (nil = SQL NULL);
+		// a real driver dereferences this on the way in, but the row may
+		// still hold the pointer verbatim in tests, so unwrap it here too.
+		if v == nil {
+			return ""
+		}
+		return *v
+	default:
+		return fmt.Sprintf("%v", s.row[i])
 	}
-	return fmt.Sprintf("%v", s.row[i])
 }
 
 func (s *scanner) strPtr(col string) *string {

backend/items.go

@@ -1,9 +1,40 @@
 package main
 
 import (
+	"encoding/json"
+
 	plugin "github.com/Paca-AI/plugin-sdk-go"
 )
 
+// optionalString distinguishes three states for a JSON PATCH field:
+//   - key absent    → Set=false (leave the stored value unchanged)
+//   - key = null    → Set=true, Value=nil (explicitly clear the stored value)
+//   - key = "value" → Set=true, Value=non-nil (set the stored value)
+//
+// A plain *string cannot represent this: encoding/json leaves it nil both
+// when the key is absent and when it is explicitly null, so a client asking
+// to clear the field silently gets ignored instead.
+type optionalString struct {
+	Set   bool
+	Value *string
+}
+
+// UnmarshalJSON implements json.Unmarshaler. It marks the field as Set and
+// decodes the value, treating JSON null as a nil pointer.
+func (o *optionalString) UnmarshalJSON(data []byte) error {
+	o.Set = true
+	if string(data) == "null" {
+		o.Value = nil
+		return nil
+	}
+	var s string
+	if err := json.Unmarshal(data, &s); err != nil {
+		return err
+	}
+	o.Value = &s
+	return nil
+}
+
 // ── Item route handlers ───────────────────────────────────────────────────────
 
 // createItem handles POST /tasks/:taskId/checklists/:checklistId/items
@@ -87,17 +118,17 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response)
 	}
 
 	type body struct {
-		Title      *string `json:"title"`
-		IsChecked  *bool   `json:"is_checked"`
-		AssigneeID *string `json:"assignee_id"`
+		Title      *string        `json:"title"`
+		IsChecked  *bool          `json:"is_checked"`
+		AssigneeID optionalString `json:"assignee_id"`
 	}
 	b, err := plugin.JSONBody[body](req)
 	if err != nil {
 		res.Error(400, "invalid request body")
 		return
 	}
 
-	if b.Title == nil && b.IsChecked == nil && b.AssigneeID == nil {
+	if b.Title == nil && b.IsChecked == nil && !b.AssigneeID.Set {
 		res.Error(400, "no fields to update")
 		return
 	}
@@ -133,8 +164,8 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response)
 	if b.IsChecked != nil {
 		updChecked = *b.IsChecked
 	}
-	if b.AssigneeID != nil {
-		updAssignee = b.AssigneeID
+	if b.AssigneeID.Set {
+		updAssignee = b.AssigneeID.Value
 	}
 
 	// Simulate UPDATE as DELETE + re-INSERT. task_checklist_items has no child
@@ -176,12 +207,20 @@ func (p *checklistPlugin) updateItem(req *plugin.Request, res *plugin.Response)
 		changes = append(changes, map[string]any{"field": "is_checked", "old": oldChecked, "new": *b.IsChecked})
 	}
 	oldAssignee := sc.strPtr("assignee_id")
-	if b.AssigneeID != nil && (oldAssignee == nil || *oldAssignee != *b.AssigneeID) {
-		var oldAssigneeActivity any
-		if oldAssignee != nil {
-			oldAssigneeActivity = *oldAssignee
+	if b.AssigneeID.Set {
+		newAssignee := b.AssigneeID.Value
+		assigneeChanged := (oldAssignee == nil) != (newAssignee == nil) ||
+			(oldAssignee != nil && newAssignee != nil && *oldAssignee != *newAssignee)
+		if assigneeChanged {
+			var oldAssigneeActivity, newAssigneeActivity any
+			if oldAssignee != nil {
+				oldAssigneeActivity = *oldAssignee
+			}
+			if newAssignee != nil {
+				newAssigneeActivity = *newAssignee
+			}
+			changes = append(changes, map[string]any{"field": "assignee_id", "old": oldAssigneeActivity, "new": newAssigneeActivity})
 		}
-		changes = append(changes, map[string]any{"field": "assignee_id", "old": oldAssigneeActivity, "new": *b.AssigneeID})
 	}
 	plugin.RecordActivity(taskID, projectID, req.Caller.UserID, "task.checklist_item.updated",
 		map[string]any{"text": updTitle, "changes": changes, "_description": "updated checklist item: \"" + updTitle + "\""})

backend/plugin_test.go

@@ -306,6 +306,114 @@ func TestDeleteItem(t *testing.T) {
 	}
 }
 
+// TestUpdateItem_OmittedAssigneeUnchanged guards against regressing to
+// unconditionally overwriting assignee_id on every PATCH, even when the
+// request omits it.
+func TestUpdateItem_OmittedAssigneeUnchanged(t *testing.T) {
+	tc := setupPlugin(t)
+
+	createRes := tc.Call("POST", "/tasks/:taskId/checklists",
+		withPathParams(callerReq(), map[string]string{"taskId": testTaskID}).
+			WithJSONBody(map[string]string{"title": "CL"}))
+	var clEnv struct {
+		Data checklist `json:"data"`
+	}
+	_ = json.Unmarshal(createRes.Body, &clEnv)
+	clID := clEnv.Data.ID
+
+	itemRes := tc.Call("POST", "/tasks/:taskId/checklists/:checklistId/items",
+		withPathParams(callerReq(), map[string]string{
+			"taskId":      testTaskID,
+			"checklistId": clID,
+		}).WithJSONBody(map[string]string{"title": "Step"}))
+	var itemEnv struct {
+		Data checklistItem `json:"data"`
+	}
+	_ = json.Unmarshal(itemRes.Body, &itemEnv)
+	itemID := itemEnv.Data.ID
+
+	itemPath := map[string]string{"taskId": testTaskID, "checklistId": clID, "itemId": itemID}
+
+	assignRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId",
+		withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": "user-42"}))
+	if assignRes.StatusCode != 200 {
+		t.Fatalf("expected 200, got %d: %s", assignRes.StatusCode, assignRes.BodyString())
+	}
+	var assignEnv struct {
+		Data checklistItem `json:"data"`
+	}
+	_ = json.Unmarshal(assignRes.Body, &assignEnv)
+	if assignEnv.Data.AssigneeID == nil || *assignEnv.Data.AssigneeID != "user-42" {
+		t.Fatalf("expected assignee_id=user-42, got %+v", assignEnv.Data.AssigneeID)
+	}
+
+	// Patch only is_checked; assignee_id is absent from the body and must
+	// survive the update untouched.
+	patchRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId",
+		withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"is_checked": true}))
+	if patchRes.StatusCode != 200 {
+		t.Fatalf("expected 200, got %d: %s", patchRes.StatusCode, patchRes.BodyString())
+	}
+	var patchEnv struct {
+		Data checklistItem `json:"data"`
+	}
+	_ = json.Unmarshal(patchRes.Body, &patchEnv)
+	if !patchEnv.Data.IsChecked {
+		t.Fatal("expected is_checked=true")
+	}
+	if patchEnv.Data.AssigneeID == nil || *patchEnv.Data.AssigneeID != "user-42" {
+		t.Fatalf("expected assignee_id to remain user-42, got %+v", patchEnv.Data.AssigneeID)
+	}
+}
+
+// TestUpdateItem_ExplicitNullClearsAssignee verifies the other half of the
+// three-state contract: an explicit JSON null clears the assignee, matching
+// the checklist MCP tool's documented "pass null to unassign" behavior.
+func TestUpdateItem_ExplicitNullClearsAssignee(t *testing.T) {
+	tc := setupPlugin(t)
+
+	createRes := tc.Call("POST", "/tasks/:taskId/checklists",
+		withPathParams(callerReq(), map[string]string{"taskId": testTaskID}).
+			WithJSONBody(map[string]string{"title": "CL"}))
+	var clEnv struct {
+		Data checklist `json:"data"`
+	}
+	_ = json.Unmarshal(createRes.Body, &clEnv)
+	clID := clEnv.Data.ID
+
+	itemRes := tc.Call("POST", "/tasks/:taskId/checklists/:checklistId/items",
+		withPathParams(callerReq(), map[string]string{
+			"taskId":      testTaskID,
+			"checklistId": clID,
+		}).WithJSONBody(map[string]string{"title": "Step"}))
+	var itemEnv struct {
+		Data checklistItem `json:"data"`
+	}
+	_ = json.Unmarshal(itemRes.Body, &itemEnv)
+	itemID := itemEnv.Data.ID
+
+	itemPath := map[string]string{"taskId": testTaskID, "checklistId": clID, "itemId": itemID}
+
+	assignRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId",
+		withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": "user-42"}))
+	if assignRes.StatusCode != 200 {
+		t.Fatalf("expected 200, got %d: %s", assignRes.StatusCode, assignRes.BodyString())
+	}
+
+	clearRes := tc.Call("PATCH", "/tasks/:taskId/checklists/:checklistId/items/:itemId",
+		withPathParams(callerReq(), itemPath).WithJSONBody(map[string]any{"assignee_id": nil}))
+	if clearRes.StatusCode != 200 {
+		t.Fatalf("expected 200, got %d: %s", clearRes.StatusCode, clearRes.BodyString())
+	}
+	var clearEnv struct {
+		Data checklistItem `json:"data"`
+	}
+	_ = json.Unmarshal(clearRes.Body, &clearEnv)
+	if clearEnv.Data.AssigneeID != nil {
+		t.Fatalf("expected assignee_id to be cleared, got %q", *clearEnv.Data.AssigneeID)
+	}
+}
+
 // ── 404 guard tests ───────────────────────────────────────────────────────────
 
 func TestListChecklists_UnknownTask(t *testing.T) {

plugin.json

@@ -2,7 +2,7 @@
   "id": "com.paca.checklist",
   "displayName": "Checklist",
   "description": "Adds named checklists with checkable items to tasks.",
-  "version": "0.2.6",
+  "version": "0.2.7",
   "permissions": ["db.read", "db.write", "events.subscribe"],
   "backend": {
     "eventSubscriptions": ["task.deleted"],