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

@@ -7293,3 +7293,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/store/store.go

@@ -2255,6 +2255,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, fmt.Errorf("observation title is required (non-empty after trimming whitespace)")
+	}
+
 	if len(content) > s.cfg.MaxObservationLength {
 		content = content[:s.cfg.MaxObservationLength] + "... [truncated]"
 	}

internal/store/store_test.go

@@ -8576,3 +8576,86 @@ func TestMostRecentActiveSessionScopedByProject(t *testing.T) {
 		t.Fatalf("expected no active session for engram when only 'other' has one, got ok=%v", ok)
 	}
 }
+
+func TestAddObservationRejectsEmptyTitle(t *testing.T) {
+	cases := []struct {
+		name  string
+		title string
+	}{
+		{"empty string", ""},
+		{"only spaces", "   "},
+		{"only tabs and newlines", "\t\n  \n"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			s := newTestStore(t)
+			_, err := s.AddObservation(AddObservationParams{
+				SessionID: "s1",
+				Type:      "decision",
+				Title:     tc.title,
+				Content:   "some content",
+				Project:   "engram",
+				Scope:     "project",
+			})
+			if err == nil {
+				t.Fatal("expected error for empty title, got nil")
+			}
+			if !strings.Contains(err.Error(), "title is required") {
+				t.Errorf("expected 'title is required' in error, got: %v", err)
+			}
+		})
+	}
+}
+
+func TestAddObservationAcceptsNonEmptyTitle(t *testing.T) {
+	s := newTestStore(t)
+	if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+		t.Fatalf("create session: %v", err)
+	}
+	id, err := s.AddObservation(AddObservationParams{
+		SessionID: "s1",
+		Type:      "decision",
+		Title:     "Valid title",
+		Content:   "some content",
+		Project:   "engram",
+		Scope:     "project",
+	})
+	if err != nil {
+		t.Fatalf("unexpected error for valid title: %v", err)
+	}
+	if id == 0 {
+		t.Fatal("expected a non-zero observation id")
+	}
+}
+
+// TestAddObservationValidatesPostStripTitle pins the contract that the
+// non-empty check runs on the value *after* stripPrivateTags, which is what
+// actually gets persisted and pushed to the cloud (see issue #459). A title
+// made only of private tags is NOT empty post-strip: stripPrivateTags replaces
+// the tags with "[REDACTED]", so the observation is accepted and persisted with
+// that title. This guards against a refactor that moves validation before the
+// strip and silently changes the persisted value.
+func TestAddObservationValidatesPostStripTitle(t *testing.T) {
+	s := newTestStore(t)
+	if err := s.CreateSession("s1", "engram", "/tmp/engram"); err != nil {
+		t.Fatalf("create session: %v", err)
+	}
+	id, err := s.AddObservation(AddObservationParams{
+		SessionID: "s1",
+		Type:      "decision",
+		Title:     "<private>secret</private>",
+		Content:   "some content",
+		Project:   "engram",
+		Scope:     "project",
+	})
+	if err != nil {
+		t.Fatalf("private-only title strips to a non-empty placeholder; expected accept, got: %v", err)
+	}
+	obs, err := s.GetObservation(id)
+	if err != nil {
+		t.Fatalf("get observation: %v", err)
+	}
+	if obs.Title != "[REDACTED]" {
+		t.Errorf("expected persisted title %q, got %q", "[REDACTED]", obs.Title)
+	}
+}