From 3087a1268d70757ae15feb7d7571388e19a34877 Mon Sep 17 00:00:00 2001
From: timonrieger <mail@timonrieger.de>
Date: Fri, 12 Jun 2026 18:41:49 +0200
Subject: [PATCH] fix(server): do not merge metadata when multiple duplicates
 are kept

---
 server/src/services/duplicate.service.spec.ts | 20 ++++++
 server/src/services/duplicate.service.ts      | 72 ++++++++++---------
 2 files changed, 57 insertions(+), 35 deletions(-)

diff --git a/server/src/services/duplicate.service.spec.ts b/server/src/services/duplicate.service.spec.ts
index 18e3e664b5..81786abee5 100644
--- a/server/src/services/duplicate.service.spec.ts
+++ b/server/src/services/duplicate.service.spec.ts
@@ -369,6 +369,26 @@ describe(DuplicateService.name, () => {
       expect(mocks.job.queueAll).toHaveBeenCalledWith([{ name: JobName.SidecarWrite, data: { id: asset1.id } }]);
     });
 
+    it('should not merge metadata when multiple assets are kept', async () => {
+      const asset1 = { ...AssetFactory.create(), isFavorite: true };
+      const asset2 = { ...AssetFactory.create(), isFavorite: false };
+      mocks.access.duplicate.checkOwnerAccess.mockResolvedValue(new Set(['group-1']));
+      mocks.duplicateRepository.get.mockResolvedValue({
+        duplicateId: 'group-1',
+        assets: [asset1 as unknown as MapAsset, asset2 as unknown as MapAsset],
+      });
+
+      const result = await sut.resolve(authStub.admin, {
+        groups: [{ duplicateId: 'group-1', keepAssetIds: [asset1.id, asset2.id], trashAssetIds: [] }],
+      });
+
+      expect(result[0].success).toBe(true);
+      expect(mocks.album.addAssetIdsToAlbums).not.toHaveBeenCalled();
+      expect(mocks.tag.replaceAssetTags).not.toHaveBeenCalled();
+      expect(mocks.asset.updateAllExif).not.toHaveBeenCalled();
+      expect(mocks.asset.updateAll).toHaveBeenCalledWith([asset1.id, asset2.id], { duplicateId: null });
+    });
+
     // NOTE: The following integration-style tests are covered by E2E tests instead
     // to avoid complex mock setup. The validation and error-handling logic above
     // is thoroughly unit tested.
diff --git a/server/src/services/duplicate.service.ts b/server/src/services/duplicate.service.ts
index 6e9e62ba0b..7ea300f88a 100644
--- a/server/src/services/duplicate.service.ts
+++ b/server/src/services/duplicate.service.ts
@@ -156,51 +156,51 @@ export class DuplicateService extends BaseService {
       }
     }
 
-    const assetAlbumMap = await this.albumRepository.getByAssetIds(auth.user.id, [...groupAssetIds]);
+    // Only merge metadata into the keeper when exactly one asset can absorb trashed duplicates.
+    if (idsToKeep.length === 1 && idsToTrash.length > 0) {
+      const assetAlbumMap = await this.albumRepository.getByAssetIds(auth.user.id, [...groupAssetIds]);
 
-    const { assetUpdate, exifUpdate, mergedAlbumIds, mergedTagIds, mergedTagValues } = this.getSyncMergeResult(
-      duplicateGroup.assets,
-      assetAlbumMap,
-    );
+      const { assetUpdate, exifUpdate, mergedAlbumIds, mergedTagIds, mergedTagValues } = this.getSyncMergeResult(
+        duplicateGroup.assets,
+        assetAlbumMap,
+      );
 
-    if (mergedAlbumIds.length > 0) {
-      const allowedAlbumIds = await this.checkAccess({
-        auth,
-        permission: Permission.AlbumAssetCreate,
-        ids: mergedAlbumIds,
-      });
+      if (mergedAlbumIds.length > 0) {
+        const allowedAlbumIds = await this.checkAccess({
+          auth,
+          permission: Permission.AlbumAssetCreate,
+          ids: mergedAlbumIds,
+        });
 
-      const allowedShareIds = await this.checkAccess({
-        auth,
-        permission: Permission.AssetShare,
-        ids: idsToKeep,
-      });
+        const allowedShareIds = await this.checkAccess({
+          auth,
+          permission: Permission.AssetShare,
+          ids: idsToKeep,
+        });
 
-      if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) {
-        await this.albumRepository.addAssetIdsToAlbums(
-          [...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))),
-        );
+        if (allowedAlbumIds.size > 0 && allowedShareIds.size > 0) {
+          await this.albumRepository.addAssetIdsToAlbums(
+            [...allowedAlbumIds].flatMap((albumId) => [...allowedShareIds].map((assetId) => ({ albumId, assetId }))),
+          );
+        }
       }
-    }
 
-    if (mergedTagIds.length > 0) {
-      const allowedTagIds = await this.checkAccess({
-        auth,
-        permission: Permission.TagAsset,
-        ids: mergedTagIds,
-      });
+      if (mergedTagIds.length > 0) {
+        const allowedTagIds = await this.checkAccess({
+          auth,
+          permission: Permission.TagAsset,
+          ids: mergedTagIds,
+        });
 
-      if (allowedTagIds.size > 0) {
-        // Replace tags for each keeper asset to ensure all merged tags are applied
-        await Promise.all(idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds])));
+        if (allowedTagIds.size > 0) {
+          await Promise.all(
+            idsToKeep.map((assetId) => this.tagRepository.replaceAssetTags(assetId, [...allowedTagIds])),
+          );
 
-        // Update asset_exif.tags so the subsequent SidecarWrite + MetadataExtraction
-        // cycle preserves the merged tags (updateAllExif locks the property automatically)
-        await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
+          await this.assetRepository.updateAllExif(idsToKeep, { tags: mergedTagValues });
+        }
       }
-    }
 
-    if (idsToKeep.length > 0) {
       const hasExifUpdate = Object.keys(exifUpdate).length > 0;
       const hasTagUpdate = mergedTagIds.length > 0;
 
@@ -213,6 +213,8 @@ export class DuplicateService extends BaseService {
       }
 
       await this.assetRepository.updateAll(idsToKeep, { duplicateId: null, ...assetUpdate });
+    } else if (idsToKeep.length > 0) {
+      await this.assetRepository.updateAll(idsToKeep, { duplicateId: null });
     }
 
     if (idsToTrash.length > 0) {