fix(server): drop stale indexes before renaming the tenant key

Two defects found by running migration 0004 against a seeded legacy database.

A unique index on org_id treats a missing org_id as null. Renaming the field
strips it, so the second document collided and the whole update failed:

  E11000 duplicate key error collection: instance_oidc index: org_id_1
  dup key: { org_id: null }

The index cleanup therefore has to run BEFORE the field rename, not after.
rename-rollback needs the symmetric step for instance_id, or reverting hits
the same wall.

The detection also silently matched nothing: the driver decodes an index key
document as bson.D, not bson.M, so the type assertion always failed and no
index was ever dropped. IndexKeyedOn now handles both.

server/internal/services/migrate_instance.go

@@ -93,7 +93,22 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error {
 		log.Printf("0004: renamed collection %s to %s", r.from, r.to)
 	}
 
-	// Step 2: rename the field.
+	// Step 2: drop indexes keyed on the old field name, BEFORE renaming it.
+	//
+	// Order matters and is not obvious. A unique index on org_id treats a
+	// missing org_id as null, so as soon as $rename strips the field from the
+	// second document the index reports a duplicate null and the whole update
+	// fails. Dropping first avoids that entirely.
+	//
+	// Dropping an index touches no documents. The boot-time index builders
+	// recreate the current ones against the new field name.
+	for _, c := range ScopedCollections {
+		if err := DropIndexesKeyedOn(ctx, db, c, "org_id"); err != nil {
+			return err
+		}
+	}
+
+	// Step 3: rename the field.
 	for _, c := range ScopedCollections {
 		res, err := db.Collection(c).UpdateMany(ctx,
 			bson.M{"org_id": bson.M{"$exists": true}},
@@ -107,7 +122,7 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error {
 		}
 	}
 
-	// Step 3: verify before anyone records a marker. Any mismatch aborts, and
+	// Step 4: verify before anyone records a marker. Any mismatch aborts, and
 	// the migration is re-run rather than marked done.
 	for _, c := range ScopedCollections {
 		total, err := db.Collection(c).CountDocuments(ctx, bson.M{})
@@ -135,38 +150,62 @@ func MigrateOrgToInstance(ctx context.Context, db *mongo.Database) error {
 		}
 	}
 
-	// Step 4: indexes keyed on the old field name now point at a field that no
-	// longer exists. Drop them; the boot-time index builders recreate the
-	// current ones. Dropping an index touches no documents.
-	for _, c := range ScopedCollections {
-		cur, err := db.Collection(c).Indexes().List(ctx)
-		if err != nil {
-			return fmt.Errorf("list indexes on %s: %w", c, err)
-		}
-		var specs []bson.M
-		if err := cur.All(ctx, &specs); err != nil {
-			return fmt.Errorf("decode indexes on %s: %w", c, err)
-		}
-		for _, s := range specs {
-			name, _ := s["name"].(string)
-			if name == "_id_" {
-				continue
+	log.Printf("0004: verified %d collection(s)", len(ScopedCollections))
+	return nil
+}
+
+// IndexKeyedOn reports whether an index specification's key document mentions
+// field.
+//
+// The key is checked as both bson.D and bson.M because the driver's decoding of
+// a nested document depends on the target type, and getting this wrong is
+// silent: the index simply is not found, and the field rename then fails on a
+// duplicate null.
+func IndexKeyedOn(key any, field string) bool {
+	switch k := key.(type) {
+	case bson.M:
+		_, ok := k[field]
+		return ok
+	case bson.D:
+		for _, e := range k {
+			if e.Key == field {
+				return true
 			}
-			keys, ok := s["key"].(bson.M)
-			if !ok {
-				continue
-			}
-			if _, keyed := keys["org_id"]; !keyed {
-				continue
-			}
-			if err := db.Collection(c).Indexes().DropOne(ctx, name); err != nil {
-				return fmt.Errorf("drop index %s on %s: %w", name, c, err)
-			}
-			log.Printf("0004: dropped stale index %s on %s", name, c)
 		}
 	}
+	return false
+}
 
-	log.Printf("0004: verified %d collection(s)", len(ScopedCollections))
+// DropIndexesKeyedOn removes every index on coll whose key mentions field,
+// leaving _id_ alone.
+//
+// Both the migration and its rollback must do this BEFORE renaming the field. A
+// unique index treats a missing field as null, so as soon as $rename strips the
+// field from the second document the index reports a duplicate null and the
+// whole update fails. Dropping an index touches no documents; the boot-time
+// index builders recreate what is needed.
+func DropIndexesKeyedOn(ctx context.Context, db *mongo.Database, coll, field string) error {
+	cur, err := db.Collection(coll).Indexes().List(ctx)
+	if err != nil {
+		return fmt.Errorf("list indexes on %s: %w", coll, err)
+	}
+	var specs []bson.M
+	if err := cur.All(ctx, &specs); err != nil {
+		return fmt.Errorf("decode indexes on %s: %w", coll, err)
+	}
+	for _, s := range specs {
+		name, _ := s["name"].(string)
+		if name == "_id_" {
+			continue
+		}
+		if !IndexKeyedOn(s["key"], field) {
+			continue
+		}
+		if err := db.Collection(coll).Indexes().DropOne(ctx, name); err != nil {
+			return fmt.Errorf("drop index %s on %s: %w", name, coll, err)
+		}
+		log.Printf("dropped stale index %s on %s", name, coll)
+	}
 	return nil
 }