fix(migrations): sync Migration indexes on boot to enforce unique name

lib/services/migrations.js

@@ -2,10 +2,10 @@
  * Module dependencies.
  */
 import chalk from 'chalk';
-import mongoose from 'mongoose';
 import path from 'path';
 import { glob } from 'glob';
 import logger from './logger.js';
+import migrationRepository from '../../modules/core/repositories/migration.repository.js';
 
 /**
  * Scan all modules for migration files matching `modules/*/migrations/*.js`.
@@ -28,8 +28,7 @@ const discoverMigrationFiles = async () => {
  * @returns {Promise<Set<string>>} set of executed migration names
  */
 const getExecutedMigrations = async () => {
-  const Migration = mongoose.model('Migration');
-  const records = await Migration.find({}, { name: 1, _id: 0 }).lean();
+  const records = await migrationRepository.listExecuted();
   return new Set(records.map((r) => r.name));
 };
 
@@ -38,10 +37,7 @@ const getExecutedMigrations = async () => {
  * @param {string} name - the migration filename used as unique key
  * @returns {Promise<object>} the created Migration document
  */
-const recordMigration = async (name) => {
-  const Migration = mongoose.model('Migration');
-  return Migration.create({ name, executedAt: new Date() });
-};
+const recordMigration = (name) => migrationRepository.create(name);
 
 /**
  * Atomically claim a migration by inserting a record before execution.
@@ -50,9 +46,8 @@ const recordMigration = async (name) => {
  * @returns {Promise<boolean>} true if claimed successfully, false if already claimed by another runner
  */
 const claimMigration = async (name) => {
-  const Migration = mongoose.model('Migration');
   try {
-    await Migration.create({ name, executedAt: new Date() });
+    await migrationRepository.create(name);
     return true;
   } catch (err) {
     // Duplicate key error means another runner already claimed it
@@ -66,10 +61,7 @@ const claimMigration = async (name) => {
  * @param {string} name - the migration filename
  * @returns {Promise<object>} the deletion result
  */
-const unclaimMigration = async (name) => {
-  const Migration = mongoose.model('Migration');
-  return Migration.deleteOne({ name });
-};
+const unclaimMigration = (name) => migrationRepository.deleteByName(name);
 
 /**
  * Run a single migration file's `up()` export.
@@ -120,6 +112,16 @@ const runMigration = async (filePath, executed) => {
   return true;
 };
 
+/**
+ * Ensure the Migration collection's indexes are in sync with the schema.
+ * The `name` field has a unique index that backs the atomic claim logic in
+ * {@link claimMigration}; without it, concurrent runners can execute the same
+ * migration twice. Delegates to the repository so the service layer never
+ * touches Mongoose directly.
+ * @returns {Promise<void>} Resolves once indexes have been synchronised.
+ */
+const ensureMigrationIndexes = () => migrationRepository.syncIndexes();
+
 /**
  * Run all pending migrations in order.
  * Scans `modules/&#42;/migrations/&#42;.js`, compares with the `migrations` MongoDB
@@ -131,6 +133,11 @@ const run = async () => {
   // Ensure the Migration model is registered
   await import(path.resolve('modules/core/models/migration.model.mongoose.js'));
 
+  // Ensure the unique index on `name` exists before any claim/record calls.
+  // Without this, older deployments whose `migrations` collection was created
+  // before the unique index was added would silently allow duplicate claims.
+  await ensureMigrationIndexes();
+
   const files = await discoverMigrationFiles();
 
   if (files.length === 0) {
@@ -162,4 +169,4 @@ const run = async () => {
   return { total: files.length, executed: executedCount };
 };
 
-export default { run, discoverMigrationFiles, getExecutedMigrations, recordMigration, runMigration };
+export default { run, discoverMigrationFiles, getExecutedMigrations, recordMigration, runMigration, ensureMigrationIndexes };

modules/core/repositories/migration.repository.js

@@ -0,0 +1,42 @@
+/**
+ * Module dependencies
+ */
+import mongoose from 'mongoose';
+
+/**
+ * @function syncIndexes
+ * @description Synchronise the Migration collection's indexes with the schema.
+ * Creates any missing indexes (notably the unique index on `name` that backs
+ * the atomic claim logic) and drops any indexes no longer declared on the
+ * schema. Idempotent and safe to call on every boot.
+ * @returns {Promise<Array<string>>} Names of indexes dropped during sync.
+ */
+const syncIndexes = () => mongoose.model('Migration').syncIndexes();
+
+/**
+ * @function listExecuted
+ * @description Fetch the name of every migration recorded as executed.
+ * @returns {Promise<Array<{name: string}>>} Lean records with only the `name` field.
+ */
+const listExecuted = () => mongoose.model('Migration').find({}, { name: 1, _id: 0 }).lean();
+
+/**
+ * @function create
+ * @description Insert a new Migration record. Relies on the unique index on
+ * `name` to reject duplicates — used both by the public `recordMigration()`
+ * flow and the atomic claim logic in `claimMigration()`.
+ * @param {string} name - Migration filename, unique key for the collection.
+ * @returns {Promise<object>} The created Migration document.
+ */
+const create = (name) => mongoose.model('Migration').create({ name, executedAt: new Date() });
+
+/**
+ * @function deleteByName
+ * @description Remove a Migration record by name. Used to unclaim a migration
+ * when its execution fails so it can be retried on the next boot.
+ * @param {string} name - Migration filename to delete.
+ * @returns {Promise<object>} Mongo deletion result.
+ */
+const deleteByName = (name) => mongoose.model('Migration').deleteOne({ name });
+
+export default { syncIndexes, listExecuted, create, deleteByName };

modules/core/tests/migrations.unit.tests.js

@@ -71,5 +71,13 @@ describe('Migrations unit tests:', () => {
       const json = doc.toJSON();
       expect(json.id).toBeDefined();
     });
+
+    it('should declare a unique index on the name field', () => {
+      const Migration = mongoose.model('Migration');
+      // schema.indexes() returns [[fields, options], ...] for compound/explicit indexes.
+      // For `unique: true` declared on the path, the index is carried on the schema path itself.
+      const namePath = Migration.schema.path('name');
+      expect(namePath.options.unique).toBe(true);
+    });
   });
 });