refactor(server): break the memories import cycle

immich -> unified -> photoResolver -> immich was a real cycle, every edge a
value import; journeyService only got past it with a dynamic import(). Nothing
in that cluster can become a provider while it stands, because Nest would need a
forwardRef through three modules.

It had exactly one cause: syncAlbumAssets and syncSynologyAlbumLink called
addTripPhotos, which lives in unifiedService. Both functions are two things
stapled together - fetch the album from the provider, then write it to the trip
- and only the first half is provider-specific.

So the providers keep the half they own (collectAlbumSelection,
collectSynologyAlbumSelection) and unifiedService keeps the half it owns
(syncImmichAlbum, syncSynologyAlbum). Every error string, status code and return
shape is the one the combined function produced, including the per-provider
wording the split had to preserve ('Could not reach Immich' stays in immich).

The plan called for pulling addTripPhotos and getOrCreateTrekPhoto into one
neutral module instead. Together they do not work as a pair: such a module still
needs immich and synology to fetch, so the cycle only moves.

There is a second cut that works - photoResolverService is really two files, and
its trek_photos CRUD half touches no provider, so splitting that out also opens
both loops with less churn. It is not the one taken here because it leaves
immich -> unified standing, i.e. a provider depending on the trip aggregate.
That direction is what tied the knot in the first place. The CRUD split is still
worth doing, but as the photos/ vs memories/ module boundary, not as the cycle
fix.

server/src/services/memories/ is a DAG now: helpers <- immich/synology <-
photoResolver <- unified.

server/src/nest/memories/memories.service.ts

@@ -6,6 +6,8 @@ import {
   createTripAlbumLink,
   removeAlbumLink,
   addTripPhotos,
+  syncImmichAlbum,
+  syncSynologyAlbum,
   removeTripPhoto,
   setTripPhotoSharing,
 } from '../../services/memories/unifiedService';
@@ -20,7 +22,6 @@ import {
   streamImmichAsset,
   listAlbums,
   getAlbumPhotos,
-  syncAlbumAssets,
   getAssetInfo,
   isValidAssetId,
 } from '../../services/memories/immichService';
@@ -31,7 +32,6 @@ import {
   testSynologyConnection,
   listSynologyAlbums,
   getSynologyAlbumPhotos,
-  syncSynologyAlbumLink,
   searchSynologyPhotos,
   getSynologyAssetInfo,
   streamSynologyAsset,
@@ -140,7 +140,7 @@ export class MemoriesService {
   }
 
   immichSyncAlbumAssets(tripId: string, linkId: string, userId: number, sid: string) {
-    return syncAlbumAssets(tripId, linkId, userId, sid);
+    return syncImmichAlbum(tripId, linkId, userId, sid);
   }
 
   // ── Synology ────────────────────────────────────────────────────────────────
@@ -169,7 +169,7 @@ export class MemoriesService {
   }
 
   synologySyncAlbumLink(userId: number, tripId: string, linkId: string, sid: string) {
-    return syncSynologyAlbumLink(userId, tripId, linkId, sid);
+    return syncSynologyAlbum(userId, tripId, linkId, sid);
   }
 
   synologySearchPhotos(userId: number, from: string | undefined, to: string | undefined, offset: number, limit: number) {

server/src/services/memories/immichService.ts

@@ -3,7 +3,6 @@ import { db } from '../../db/database';
 import { maybe_encrypt_api_key, decrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto';
 import { checkSsrf, safeFetch } from '../../utils/ssrfGuard';
 import { writeAudit } from '../../nest/audit/audit.bridge';
-import { addTripPhotos} from './unifiedService';
 import { getAlbumIdFromLink, updateSyncTimeForAlbumLink, Selection, pipeAsset } from './helpersService';
 
 // ── Credentials ────────────────────────────────────────────────────────────
@@ -489,12 +488,21 @@ export async function getAlbumPhotos(
   }
 }
 
-export async function syncAlbumAssets(
+/**
+ * The Immich half of an album sync: resolve the link, fetch the album, and hand
+ * back the picked asset ids.
+ *
+ * Adding them to the trip stays with the unified service, which owns
+ * addTripPhotos. Splitting it there is what breaks the import cycle this module
+ * used to close (immich -> unified -> photoResolver -> immich): the provider now
+ * only ever points downhill. Every error string and status below is the one the
+ * combined function returned.
+ */
+export async function collectAlbumSelection(
   tripId: string,
   linkId: string,
   userId: number,
-  sid: string,
-): Promise<{ success?: boolean; added?: number; total?: number; error?: string; status?: number }> {
+): Promise<{ selection: Selection; total: number } | { error: string; status: number }> {
   const response = getAlbumIdFromLink(tripId, linkId, userId);
   if (!response.success) return { error: 'Album link not found', status: 404 };
 
@@ -508,17 +516,10 @@ export async function syncAlbumAssets(
     // visibility, so a stored hidden asset is a permanently broken tile (#1474).
     const assets = albumResult.assets.filter((a: any) => a.type === 'IMAGE' && isVisibleAsset(a));
 
-    const selection: Selection = {
-      provider: 'immich',
-      asset_ids: assets.map((a: any) => a.id),
+    return {
+      selection: { provider: 'immich', asset_ids: assets.map((a: any) => a.id) },
+      total: assets.length,
     };
-
-    const result = await addTripPhotos(tripId, userId, true, [selection], sid, linkId);
-    if ('error' in result) return { error: result.error.message, status: result.error.status };
-
-    updateSyncTimeForAlbumLink(linkId);
-
-    return { success: true, added: result.data.added, total: assets.length };
   } catch {
     return { error: 'Could not reach Immich', status: 502 };
   }

server/src/services/memories/synologyService.ts

@@ -3,7 +3,6 @@ import { Response } from 'express';
 import { db } from '../../db/database';
 import { decrypt_api_key, encrypt_api_key, maybe_encrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto';
 import { safeFetch, SsrfBlockedError, checkSsrf } from '../../utils/ssrfGuard';
-import { addTripPhotos } from './unifiedService';
 import {
     getAlbumLinkForSync,
     updateSyncTimeForAlbumLink,
@@ -510,9 +509,13 @@ export async function getSynologyAlbumPhotos(userId: number, albumId: string, pa
     return success({ assets, total: assets.length, hasMore: false });
 }
 
-export async function syncSynologyAlbumLink(userId: number, tripId: string, linkId: string, sid: string): Promise<ServiceResult<SyncAlbumResult>> {
+/**
+ * The Synology half of an album sync — see collectAlbumSelection in
+ * immichService for why the trip-side write lives in the unified service now.
+ */
+export async function collectSynologyAlbumSelection(userId: number, tripId: string, linkId: string): Promise<ServiceResult<{ selection: Selection; total: number }>> {
     const response = getAlbumLinkForSync(tripId, linkId, userId);
-    if (!response.success) return response as ServiceResult<SyncAlbumResult>;
+    if (!response.success) return response as ServiceResult<{ selection: Selection; total: number }>;
 
     const { albumId, passphrase } = response.data;
 
@@ -527,7 +530,7 @@ export async function syncSynologyAlbumLink(userId: number, tripId: string, link
 
         const result = await _requestSynologyApi<{ list: SynologyPhotoItem[] }>(userId, itemParams);
 
-        if (!result.success) return result as ServiceResult<SyncAlbumResult>;
+        if (!result.success) return result as ServiceResult<{ selection: Selection; total: number }>;
 
         const items = result.data.list || [];
         allItems.push(...items);
@@ -541,12 +544,7 @@ export async function syncSynologyAlbumLink(userId: number, tripId: string, link
         passphrase,
     };
 
-    const result = await addTripPhotos(tripId, userId, true, [selection], sid, linkId);
-    if (!result.success) return result as ServiceResult<SyncAlbumResult>;
-
-    updateSyncTimeForAlbumLink(linkId);
-
-    return success({ added: result.data.added, total: allItems.length });
+    return success({ selection, total: allItems.length });
 }
 
 export async function searchSynologyPhotos(userId: number, from?: string, to?: string, offset = 0, limit = 300): Promise<ServiceResult<AssetsList>> {

server/src/services/memories/unifiedService.ts

@@ -3,12 +3,16 @@ import { send } from '../../nest/notifications/notifications.bridge';
 import { broadcast } from '../../websocket';
 import {
   ServiceResult,
+  SyncAlbumResult,
   fail,
   success,
   mapDbError,
   Selection,
+  updateSyncTimeForAlbumLink,
 } from './helpersService';
 import { getOrCreateTrekPhoto, deleteTrekPhotoIfOrphan } from './photoResolverService';
+import { collectAlbumSelection } from './immichService';
+import { collectSynologyAlbumSelection } from './synologyService';
 import { encrypt_api_key } from '../../nest/common/crypto/apiKeyCrypto';
 
 
@@ -313,3 +317,45 @@ async function _notifySharedTripPhotos(
     return fail('Failed to send notifications', 500);
   }
 }
+
+// ── Album sync (orchestration) ────────────────────────────────────────────
+//
+// The provider services collect the asset ids; adding them to the trip and
+// stamping the link is this module's job, because addTripPhotos lives here.
+// Keeping the write on this side is what lets immich/synology stop importing
+// this module — the cycle immich -> unified -> photoResolver -> immich is gone.
+// Both functions return exactly what the combined provider-side ones returned.
+
+export async function syncImmichAlbum(
+  tripId: string,
+  linkId: string,
+  userId: number,
+  sid: string,
+): Promise<{ success?: boolean; added?: number; total?: number; error?: string; status?: number }> {
+  const collected = await collectAlbumSelection(tripId, linkId, userId);
+  if ('error' in collected) return { error: collected.error, status: collected.status };
+
+  const result = await addTripPhotos(tripId, userId, true, [collected.selection], sid, linkId);
+  if ('error' in result) return { error: result.error.message, status: result.error.status };
+
+  updateSyncTimeForAlbumLink(linkId);
+
+  return { success: true, added: result.data.added, total: collected.total };
+}
+
+export async function syncSynologyAlbum(
+  userId: number,
+  tripId: string,
+  linkId: string,
+  sid: string,
+): Promise<ServiceResult<SyncAlbumResult>> {
+  const collected = await collectSynologyAlbumSelection(userId, tripId, linkId);
+  if (!collected.success) return collected as ServiceResult<SyncAlbumResult>;
+
+  const result = await addTripPhotos(tripId, userId, true, [collected.data.selection], sid, linkId);
+  if (!result.success) return result as ServiceResult<SyncAlbumResult>;
+
+  updateSyncTimeForAlbumLink(linkId);
+
+  return success({ added: result.data.added, total: collected.data.total });
+}

server/tests/e2e/memories.e2e.test.ts

@@ -35,17 +35,20 @@ const { unified, immich, synology } = vi.hoisted(() => ({
   unified: {
     listTripPhotos: vi.fn(), addTripPhotos: vi.fn(), setTripPhotoSharing: vi.fn(),
     removeTripPhoto: vi.fn(), listTripAlbumLinks: vi.fn(), createTripAlbumLink: vi.fn(), removeAlbumLink: vi.fn(),
+    // The album-sync orchestration moved here from the provider modules when the
+    // immich -> unified import cycle was broken.
+    syncImmichAlbum: vi.fn(), syncSynologyAlbum: vi.fn(),
   },
   immich: {
     getConnectionSettings: vi.fn(), saveImmichSettings: vi.fn(), setImmichAutoUpload: vi.fn(),
     testConnection: vi.fn(), getConnectionStatus: vi.fn(), browseTimeline: vi.fn(), searchPhotos: vi.fn(),
-    streamImmichAsset: vi.fn(), listAlbums: vi.fn(), getAlbumPhotos: vi.fn(), syncAlbumAssets: vi.fn(),
+    streamImmichAsset: vi.fn(), listAlbums: vi.fn(), getAlbumPhotos: vi.fn(), collectAlbumSelection: vi.fn(),
     getAssetInfo: vi.fn(), isValidAssetId: vi.fn(),
   },
   synology: {
     getSynologySettings: vi.fn(), updateSynologySettings: vi.fn(), getSynologyStatus: vi.fn(),
     testSynologyConnection: vi.fn(), listSynologyAlbums: vi.fn(), getSynologyAlbumPhotos: vi.fn(),
-    syncSynologyAlbumLink: vi.fn(), searchSynologyPhotos: vi.fn(), getSynologyAssetInfo: vi.fn(),
+    collectSynologyAlbumSelection: vi.fn(), searchSynologyPhotos: vi.fn(), getSynologyAssetInfo: vi.fn(),
     streamSynologyAsset: vi.fn(),
   },
 }));
@@ -290,12 +293,12 @@ describe('Memories e2e (real auth guard + temp SQLite)', () => {
     });
 
     it('200 album sync (POST stays 200) / 404 envelope', async () => {
-      immich.syncAlbumAssets.mockResolvedValue({ success: true, added: 3, total: 10 });
+      unified.syncImmichAlbum.mockResolvedValue({ success: true, added: 3, total: 10 });
       const ok = await request(server).post(`${IMMICH}/trips/5/album-links/7/sync`).set('Cookie', sessionCookie(1));
       expect(ok.status).toBe(200);
       expect(ok.body).toEqual({ success: true, added: 3, total: 10 });
 
-      immich.syncAlbumAssets.mockResolvedValue({ error: 'Album link not found', status: 404 });
+      unified.syncImmichAlbum.mockResolvedValue({ error: 'Album link not found', status: 404 });
       const bad = await request(server).post(`${IMMICH}/trips/5/album-links/9/sync`).set('Cookie', sessionCookie(1));
       expect(bad.status).toBe(404);
       expect(bad.body).toEqual({ error: 'Album link not found' });
@@ -369,7 +372,7 @@ describe('Memories e2e (real auth guard + temp SQLite)', () => {
     });
 
     it('200 album sync (POST stays 200)', async () => {
-      synology.syncSynologyAlbumLink.mockResolvedValue({ success: true, data: { added: 2, total: 5 } });
+      unified.syncSynologyAlbum.mockResolvedValue({ success: true, data: { added: 2, total: 5 } });
       const res = await request(server).post(`${SYNO}/trips/5/album-links/7/sync`).set('Cookie', sessionCookie(1));
       expect(res.status).toBe(200);
       expect(res.body).toEqual({ added: 2, total: 5 });

server/tests/unit/nest/memories.service.test.ts

@@ -10,6 +10,8 @@ const unified = vi.hoisted(() => ({
   createTripAlbumLink: vi.fn(() => ({ data: {} })),
   removeAlbumLink: vi.fn(() => ({ data: {} })),
   addTripPhotos: vi.fn(async () => ({ data: { added: 0 } })),
+  syncImmichAlbum: vi.fn(async () => ({ success: true, added: 0, total: 0 })),
+  syncSynologyAlbum: vi.fn(async () => ({ success: true, data: { added: 0, total: 0 } })),
   removeTripPhoto: vi.fn(() => ({ data: {} })),
   setTripPhotoSharing: vi.fn(async () => ({ data: {} })),
 }));
@@ -26,7 +28,7 @@ const immich = vi.hoisted(() => ({
   streamImmichAsset: vi.fn(async () => undefined),
   listAlbums: vi.fn(async () => ({ albums: [] })),
   getAlbumPhotos: vi.fn(async () => ({ assets: [] })),
-  syncAlbumAssets: vi.fn(async () => ({ added: 0, total: 0 })),
+  collectAlbumSelection: vi.fn(async () => ({ selection: { provider: 'immich', asset_ids: [] }, total: 0 })),
   getAssetInfo: vi.fn(async () => ({ data: {} })),
   isValidAssetId: vi.fn(() => true),
 }));
@@ -39,7 +41,7 @@ const synology = vi.hoisted(() => ({
   testSynologyConnection: vi.fn(async () => ({ success: true, data: {} })),
   listSynologyAlbums: vi.fn(async () => ({ success: true, data: {} })),
   getSynologyAlbumPhotos: vi.fn(async () => ({ success: true, data: {} })),
-  syncSynologyAlbumLink: vi.fn(async () => ({ success: true, data: {} })),
+  collectSynologyAlbumSelection: vi.fn(async () => ({ success: true, data: { selection: { provider: 'synologyphotos', asset_ids: [] }, total: 0 } })),
   searchSynologyPhotos: vi.fn(async () => ({ success: true, data: {} })),
   getSynologyAssetInfo: vi.fn(async () => ({ success: true, data: {} })),
   streamSynologyAsset: vi.fn(async () => undefined),
@@ -146,7 +148,7 @@ describe('MemoriesService (delegation wrapper over services/memories/*)', () =>
     expect(immich.getAlbumPhotos).toHaveBeenCalledWith(7, 'al1');
 
     await svc.immichSyncAlbumAssets('5', 'l1', 7, 'sock');
-    expect(immich.syncAlbumAssets).toHaveBeenCalledWith('5', 'l1', 7, 'sock');
+    expect(unified.syncImmichAlbum).toHaveBeenCalledWith('5', 'l1', 7, 'sock');
   });
 
   it('synology methods delegate', async () => {
@@ -166,7 +168,7 @@ describe('MemoriesService (delegation wrapper over services/memories/*)', () =>
     expect(synology.listSynologyAlbums).toHaveBeenCalledWith(7);
 
     await svc.synologySyncAlbumLink(7, '5', 'l1', 'sock');
-    expect(synology.syncSynologyAlbumLink).toHaveBeenCalledWith(7, '5', 'l1', 'sock');
+    expect(unified.syncSynologyAlbum).toHaveBeenCalledWith(7, '5', 'l1', 'sock');
 
     await svc.synologySearchPhotos(7, 'f', 't', 0, 100);
     expect(synology.searchSynologyPhotos).toHaveBeenCalledWith(7, 'f', 't', 0, 100);