fix(store): reject empty observation titles before persistence

internal/mcp/mcp.go

@@ -1176,6 +1176,14 @@ func handleSave(s *store.Store, cfg MCPConfig, activity *SessionActivity) server
 		if strings.TrimSpace(content) == "" {
 			return mcp.NewToolResultError("content is required for mem_save (use content, or observation for backward-compatible clients)"), nil
 		}
+		// Reject empty titles early with an agent-actionable message. The store
+		// enforces the same rule (single source of truth), but returning here
+		// lets us tell the model exactly what to provide on retry instead of
+		// surfacing a generic store error. An empty title would otherwise be
+		// accepted and silently block cloud sync downstream. See issue #459.
+		if strings.TrimSpace(title) == "" {
+			return mcp.NewToolResultError("title is required for mem_save — pass a short, searchable title (e.g. 'Fixed N+1 query in UserList', 'Decision: use OpenTofu over Terraform'). Empty titles silently block cloud sync (issue #459)."), nil
+		}
 		typ, _ := req.GetArguments()["type"].(string)
 		sessionID, _ := req.GetArguments()["session_id"].(string)
 		scope, _ := req.GetArguments()["scope"].(string)

internal/mcp/mcp_test.go

@@ -1478,6 +1478,62 @@ func TestHandleUpdateAcceptsAllOptionalFields(t *testing.T) {
 	}
 }
 
+// TestHandleUpdateRejectsEmptyTitle pins that mem_update surfaces an
+// agent-actionable error when the new title is empty or whitespace-only, and
+// that the original title is left untouched. This closes the same stuck-sync
+// class as the save path (issue #459) on the update surface.
+func TestHandleUpdateRejectsEmptyTitle(t *testing.T) {
+	cases := []struct {
+		name  string
+		title string
+	}{
+		{"empty string", ""},
+		{"only whitespace", "   \t\n"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			s := newMCPTestStore(t)
+			if err := s.CreateSession("s-empty-update", "engram", "/tmp/engram"); err != nil {
+				t.Fatalf("create session: %v", err)
+			}
+			id, err := s.AddObservation(store.AddObservationParams{
+				SessionID: "s-empty-update",
+				Type:      "decision",
+				Title:     "Original",
+				Content:   "Original content",
+				Project:   "engram",
+				Scope:     "project",
+			})
+			if err != nil {
+				t.Fatalf("add observation: %v", err)
+			}
+
+			res, err := handleUpdate(s)(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
+				"id":    float64(id),
+				"title": tc.title,
+			}}})
+			if err != nil {
+				t.Fatalf("update handler error: %v", err)
+			}
+			if !res.IsError {
+				t.Fatalf("expected an error result for empty title, got success")
+			}
+			if !strings.Contains(callResultText(t, res), "title is required") {
+				t.Fatalf("expected 'title is required' in error, got %q", callResultText(t, res))
+			}
+
+			// The original title must survive the rejected update.
+			obs, err := s.GetObservation(id)
+			if err != nil {
+				t.Fatalf("get observation: %v", err)
+			}
+			if obs.Title != "Original" {
+				t.Fatalf("expected title unchanged %q, got %q", "Original", obs.Title)
+			}
+		})
+	}
+}
+
 func TestHandleContextWithSessionOnlyUsesNoneProjects(t *testing.T) {
 	s := newMCPTestStore(t)
 	if err := s.CreateSession("s-context-none", "engram", "/tmp/engram"); err != nil {
@@ -7293,3 +7349,50 @@ func TestHandleSearchLegacyMixedCaseProject(t *testing.T) {
 		t.Fatalf("expected search results for legacy project, got: %s", text)
 	}
 }
+
+func TestHandleSaveRejectsEmptyTitle(t *testing.T) {
+	cases := []struct {
+		name  string
+		title any
+	}{
+		{"missing title", nil},
+		{"empty string", ""},
+		{"only whitespace", "   \t\n"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			s := newMCPTestStore(t)
+			h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
+
+			args := map[string]any{
+				"content": "Body with no usable title",
+				"type":    "bugfix",
+				"project": "engram",
+			}
+			if tc.title != nil {
+				args["title"] = tc.title
+			}
+
+			res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: args}})
+			if err != nil {
+				t.Fatalf("handler error: %v", err)
+			}
+			if !res.IsError {
+				t.Fatalf("expected an error result for empty title, got success")
+			}
+			text := callResultText(t, res)
+			if !strings.Contains(text, "title is required") {
+				t.Fatalf("expected 'title is required' in error, got %q", text)
+			}
+
+			// Nothing should have been persisted.
+			obs, err := s.RecentObservations("engram", "project", 5)
+			if err != nil {
+				t.Fatalf("recent observations: %v", err)
+			}
+			if len(obs) != 0 {
+				t.Fatalf("expected no persisted observations, got %d", len(obs))
+			}
+		})
+	}
+}

internal/server/server.go

@@ -337,6 +337,10 @@ func (s *Server) handleAddObservation(w http.ResponseWriter, r *http.Request) {
 
 	id, err := s.store.AddObservation(body)
 	if err != nil {
+		if errors.Is(err, store.ErrEmptyObservationTitle) {
+			jsonError(w, http.StatusBadRequest, err.Error())
+			return
+		}
 		jsonError(w, http.StatusInternalServerError, err.Error())
 		return
 	}
@@ -452,7 +456,14 @@ func (s *Server) handleUpdateObservation(w http.ResponseWriter, r *http.Request)
 
 	obs, err := s.store.UpdateObservation(id, body)
 	if err != nil {
-		jsonError(w, http.StatusNotFound, err.Error())
+		switch {
+		case errors.Is(err, store.ErrEmptyObservationTitle):
+			jsonError(w, http.StatusBadRequest, err.Error())
+		case errors.Is(err, store.ErrObservationNotFound), errors.Is(err, sql.ErrNoRows):
+			jsonError(w, http.StatusNotFound, err.Error())
+		default:
+			jsonError(w, http.StatusInternalServerError, err.Error())
+		}
 		return
 	}
 

internal/server/server_test.go

@@ -169,6 +169,121 @@ func TestClaudeSaveNudgeCompatibilityRoutes(t *testing.T) {
 	}
 }
 
+// TestUpdateObservationRejectsEmptyTitle covers the HTTP PATCH path used by the
+// gentle-engram (opencode) plugin, which forwards mem_update to
+// PATCH /observations/{id}. An empty or whitespace-only title must be rejected
+// with 400 (not persisted, not enqueued) and the original title preserved —
+// closing the stuck-sync class from issue #459 on the supported HTTP surface.
+func TestUpdateObservationRejectsEmptyTitle(t *testing.T) {
+	st := newServerTestStore(t)
+	srv := New(st, 0)
+	h := srv.Handler()
+
+	createReq := httptest.NewRequest(http.MethodPost, "/sessions", strings.NewReader(`{"id":"s-empty","project":"engram"}`))
+	createReq.Header.Set("Content-Type", "application/json")
+	createRec := httptest.NewRecorder()
+	h.ServeHTTP(createRec, createReq)
+	if createRec.Code != http.StatusCreated {
+		t.Fatalf("expected session create 201, got %d", createRec.Code)
+	}
+
+	obsReq := httptest.NewRequest(http.MethodPost, "/observations", strings.NewReader(`{"session_id":"s-empty","type":"note","title":"Original","content":"body","project":"engram"}`))
+	obsReq.Header.Set("Content-Type", "application/json")
+	obsRec := httptest.NewRecorder()
+	h.ServeHTTP(obsRec, obsReq)
+	if obsRec.Code != http.StatusCreated {
+		t.Fatalf("expected observation create 201, got %d body=%s", obsRec.Code, obsRec.Body.String())
+	}
+	var created map[string]any
+	if err := json.Unmarshal(obsRec.Body.Bytes(), &created); err != nil {
+		t.Fatalf("decode created observation: %v", err)
+	}
+	id := int64(created["id"].(float64))
+
+	for _, tc := range []struct {
+		name string
+		body string
+	}{
+		{"empty string", `{"title":""}`},
+		{"only whitespace", `{"title":"   \t\n"}`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/observations/%d", id), strings.NewReader(tc.body))
+			req.Header.Set("Content-Type", "application/json")
+			rec := httptest.NewRecorder()
+			h.ServeHTTP(rec, req)
+			if rec.Code != http.StatusBadRequest {
+				t.Fatalf("expected 400 for empty title, got %d body=%s", rec.Code, rec.Body.String())
+			}
+
+			getReq := httptest.NewRequest(http.MethodGet, fmt.Sprintf("/observations/%d", id), nil)
+			getRec := httptest.NewRecorder()
+			h.ServeHTTP(getRec, getReq)
+			if getRec.Code != http.StatusOK {
+				t.Fatalf("expected get 200, got %d", getRec.Code)
+			}
+			var got map[string]any
+			if err := json.Unmarshal(getRec.Body.Bytes(), &got); err != nil {
+				t.Fatalf("decode observation: %v", err)
+			}
+			if got["title"] != "Original" {
+				t.Fatalf("expected title unchanged %q, got %v", "Original", got["title"])
+			}
+		})
+	}
+}
+
+// TestAddObservationRejectsEmptyTitle covers the POST /observations route after
+// the store-level empty-title validation change: a whitespace-only title (which
+// passes the raw `title == ""` pre-check) must be rejected with 400 by the
+// store sentinel, and an empty string by the pre-check — neither may persist.
+func TestAddObservationRejectsEmptyTitle(t *testing.T) {
+	st := newServerTestStore(t)
+	srv := New(st, 0)
+	h := srv.Handler()
+
+	createReq := httptest.NewRequest(http.MethodPost, "/sessions", strings.NewReader(`{"id":"s-add-empty","project":"engram"}`))
+	createReq.Header.Set("Content-Type", "application/json")
+	createRec := httptest.NewRecorder()
+	h.ServeHTTP(createRec, createReq)
+	if createRec.Code != http.StatusCreated {
+		t.Fatalf("expected session create 201, got %d", createRec.Code)
+	}
+
+	for _, tc := range []struct {
+		name string
+		body string
+	}{
+		{"empty string", `{"session_id":"s-add-empty","type":"note","title":"","content":"body","project":"engram"}`},
+		{"only whitespace", `{"session_id":"s-add-empty","type":"note","title":"   \t\n","content":"body","project":"engram"}`},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req := httptest.NewRequest(http.MethodPost, "/observations", strings.NewReader(tc.body))
+			req.Header.Set("Content-Type", "application/json")
+			rec := httptest.NewRecorder()
+			h.ServeHTTP(rec, req)
+			if rec.Code != http.StatusBadRequest {
+				t.Fatalf("expected 400 for empty title, got %d body=%s", rec.Code, rec.Body.String())
+			}
+		})
+	}
+
+	// Nothing should have been persisted by the rejected requests.
+	listReq := httptest.NewRequest(http.MethodGet, "/observations?project=engram&limit=10", nil)
+	listRec := httptest.NewRecorder()
+	h.ServeHTTP(listRec, listReq)
+	if listRec.Code != http.StatusOK {
+		t.Fatalf("expected list 200, got %d", listRec.Code)
+	}
+	var obs []map[string]any
+	if err := json.Unmarshal(listRec.Body.Bytes(), &obs); err != nil {
+		t.Fatalf("decode observations: %v", err)
+	}
+	if len(obs) != 0 {
+		t.Fatalf("expected no persisted observations, got %d", len(obs))
+	}
+}
+
 func TestAdditionalServerErrorBranches(t *testing.T) {
 	st := newServerTestStore(t)
 	srv := New(st, 0)

internal/store/store.go

@@ -51,6 +51,14 @@ var (
 	ErrObservationNotFound    = errors.New("observation not found")
 	ErrPromptNotFound         = errors.New("prompt not found")
 	ErrProjectNotFound        = errors.New("project not found")
+
+	// ErrEmptyObservationTitle is returned by AddObservation and
+	// UpdateObservation when the post-stripPrivateTags title is empty after
+	// trimming whitespace. The check runs on the persisted value because that is
+	// exactly what gets pushed to the cloud, where ValidateSyncMutationPayload
+	// rejects empty titles and silently stalls the sync queue (issue #459).
+	// Callers use errors.Is to map this to a 400-class response.
+	ErrEmptyObservationTitle = errors.New("observation title is required (non-empty after trimming whitespace)")
 )
 
 // Sentinel errors for relation sync apply path (Phase 2).
@@ -2255,6 +2263,17 @@ func (s *Store) AddObservation(p AddObservationParams) (int64, error) {
 	title := stripPrivateTags(p.Title)
 	content := stripPrivateTags(p.Content)
 
+	// Reject empty titles before touching the DB. This mirrors the rule
+	// ValidateSyncMutationPayload enforces on the sync path (observation
+	// upserts require a non-empty title). We validate the post-strip value
+	// because that is exactly what gets persisted and later pushed to the
+	// cloud server. Without this, an empty title is accepted locally and the
+	// failure only surfaces later when sync rejects the mutation, silently
+	// blocking the queue with no feedback to the caller. See issue #459.
+	if strings.TrimSpace(title) == "" {
+		return 0, ErrEmptyObservationTitle
+	}
+
 	if len(content) > s.cfg.MaxObservationLength {
 		content = content[:s.cfg.MaxObservationLength] + "... [truncated]"
 	}
@@ -2897,6 +2916,16 @@ func (s *Store) UpdateObservation(id int64, p UpdateObservationParams) (*Observa
 			topicKey = normalizeTopicKey(*p.TopicKey)
 		}
 
+		// Reject empty titles before persisting. We validate the final post-strip
+		// value because that is what gets written and enqueued as a sync upsert.
+		// Without this, mem_update / PATCH /observations/{id} could persist an
+		// empty title and queue a mutation the cloud server rejects, silently
+		// stalling the sync queue — the same class AddObservation guards against
+		// (issue #459).
+		if strings.TrimSpace(title) == "" {
+			return ErrEmptyObservationTitle
+		}
+
 		if _, err := s.execHook(tx,
 			`UPDATE observations
 			 SET type = ?,