fix: Community packages update check (no-changelog) (#15684)

Co-authored-by: कारतोफ्फेलस्क्रिप्ट™ <aditya@netroy.in>

packages/cli/src/commands/base-command.ts

@@ -160,7 +160,7 @@ export abstract class BaseCommand extends Command {
 		const { communityPackages } = this.globalConfig.nodes;
 		if (communityPackages.enabled && this.needsCommunityPackages) {
 			const { CommunityPackagesService } = await import('@/services/community-packages.service');
-			await Container.get(CommunityPackagesService).checkForMissingPackages();
+			await Container.get(CommunityPackagesService).init();
 		}
 
 		if (this.needsTaskRunner && this.globalConfig.taskRunners.enabled) {

packages/cli/src/services/__tests__/community-packages.service.test.ts

@@ -6,11 +6,12 @@ import { InstalledNodesRepository } from '@n8n/db';
 import { InstalledPackagesRepository } from '@n8n/db';
 import axios from 'axios';
 import { exec } from 'child_process';
-import { mkdir as fsMkdir, readFile, writeFile, rm } from 'fs/promises';
+import { mkdir, readFile, writeFile, rm, access, constants } from 'fs/promises';
 import { mocked } from 'jest-mock';
 import { mock } from 'jest-mock-extended';
 import type { Logger, InstanceSettings, PackageDirectoryLoader } from 'n8n-core';
 import type { PublicInstalledPackage } from 'n8n-workflow';
+import { join } from 'node:path';
 
 import {
 	NODE_PACKAGE_PREFIX,
@@ -53,29 +54,11 @@ describe('CommunityPackagesService', () => {
 		},
 	});
 	const loadNodesAndCredentials = mock<LoadNodesAndCredentials>();
-
-	const nodeName = randomName();
 	const installedNodesRepository = mockInstance(InstalledNodesRepository);
-	installedNodesRepository.create.mockImplementation(() => {
-		return Object.assign(new InstalledNodes(), {
-			name: nodeName,
-			type: nodeName,
-			latestVersion: COMMUNITY_NODE_VERSION.CURRENT.toString(),
-			packageName: 'test',
-		});
-	});
-
 	const installedPackageRepository = mockInstance(InstalledPackagesRepository);
-	installedPackageRepository.create.mockImplementation(() => {
-		return Object.assign(new InstalledPackages(), {
-			packageName: mockPackageName(),
-			installedVersion: COMMUNITY_PACKAGE_VERSION.CURRENT,
-		});
-	});
 
-	const instanceSettings = mock<InstanceSettings>({
+	const nodesDownloadDir = '/tmp/n8n-jest-global-downloads';
-		nodesDownloadDir: '/tmp/n8n-jest-global-downloads',
+	const instanceSettings = mock<InstanceSettings>({ nodesDownloadDir });
-	});
 
 	const logger = mock<Logger>();
 	const publisher = mock<Publisher>();
@@ -90,6 +73,26 @@ describe('CommunityPackagesService', () => {
 		globalConfig,
 	);
 
+	beforeEach(() => {
+		jest.resetAllMocks();
+		loadNodesAndCredentials.postProcessLoaders.mockResolvedValue(undefined);
+
+		const nodeName = randomName();
+		installedNodesRepository.create.mockImplementation(() => {
+			return Object.assign(new InstalledNodes(), {
+				name: nodeName,
+				type: nodeName,
+				latestVersion: COMMUNITY_NODE_VERSION.CURRENT,
+			});
+		});
+		installedPackageRepository.create.mockImplementation(() => {
+			return Object.assign(new InstalledPackages(), {
+				packageName: mockPackageName(),
+				installedVersion: COMMUNITY_PACKAGE_VERSION.CURRENT,
+			});
+		});
+	});
+
 	describe('parseNpmPackageName()', () => {
 		test('should fail with empty package name', () => {
 			expect(() => communityPackagesService.parseNpmPackageName('')).toThrowError();
@@ -139,9 +142,6 @@ describe('CommunityPackagesService', () => {
 
 	describe('executeCommand()', () => {
 		beforeEach(() => {
-			mocked(fsMkdir).mockReset();
-			mocked(fsMkdir).mockResolvedValue(undefined);
-			mocked(exec).mockReset();
 			mocked(exec).mockImplementation(execMock);
 		});
 
@@ -159,7 +159,6 @@ describe('CommunityPackagesService', () => {
 
 			await communityPackagesService.executeNpmCommand('ls');
 
-			expect(fsMkdir).toHaveBeenCalled();
 			expect(exec).toHaveBeenCalled();
 		});
 
@@ -168,7 +167,6 @@ describe('CommunityPackagesService', () => {
 
 			await communityPackagesService.executeNpmCommand('ls');
 
-			expect(fsMkdir).toHaveBeenCalled();
 			expect(exec).toHaveBeenCalled();
 		});
 
@@ -185,7 +183,6 @@ describe('CommunityPackagesService', () => {
 
 			await expect(call).rejects.toThrowError(RESPONSE_ERROR_MESSAGES.PACKAGE_NOT_FOUND);
 
-			expect(fsMkdir).toHaveBeenCalled();
 			expect(exec).toHaveBeenCalled();
 		});
 	});
@@ -410,7 +407,6 @@ describe('CommunityPackagesService', () => {
 
 			mocked(exec).mockImplementation(execMockForThisBlock as typeof exec);
 
-			mocked(fsMkdir).mockResolvedValue(undefined);
 			mocked(readFile).mockResolvedValue(
 				JSON.stringify({
 					name: PACKAGE_NAME,
@@ -446,10 +442,11 @@ describe('CommunityPackagesService', () => {
 			);
 
 			// ASSERT:
-			expect(exec).toHaveBeenCalledTimes(4);
+			expect(rm).toHaveBeenCalledTimes(2);
-
+			expect(rm).toHaveBeenNthCalledWith(1, testBlockPackageDir, { recursive: true, force: true });
-			expect(rm).toHaveBeenCalledWith(testBlockPackageDir, { recursive: true, force: true });
+			expect(rm).toHaveBeenNthCalledWith(2, `${nodesDownloadDir}/n8n-nodes-test-latest.tgz`);
 
+			expect(exec).toHaveBeenCalledTimes(3);
 			expect(exec).toHaveBeenNthCalledWith(
 				1,
 				`npm pack ${PACKAGE_NAME}@latest --registry=${testBlockRegistry} --quiet`,
@@ -471,14 +468,7 @@ describe('CommunityPackagesService', () => {
 				expect.any(Function),
 			);
 
-			expect(exec).toHaveBeenNthCalledWith(
+			expect(mkdir).toHaveBeenCalledWith(testBlockPackageDir, { recursive: true });
-				4,
-				`rm ${testBlockTarballName}`,
-				{ cwd: testBlockDownloadDir },
-				expect.any(Function),
-			);
-
-			expect(fsMkdir).toHaveBeenCalledWith(testBlockPackageDir, { recursive: true });
 			expect(readFile).toHaveBeenCalledWith(`${testBlockPackageDir}/package.json`, 'utf-8');
 			expect(writeFile).toHaveBeenCalledWith(
 				`${testBlockPackageDir}/package.json`,
@@ -534,4 +524,171 @@ describe('CommunityPackagesService', () => {
 			);
 		});
 	});
+
+	describe('ensurePackageJson', () => {
+		const packageJsonPath = join(nodesDownloadDir, 'package.json');
+
+		test('should not create package.json if it already exists', async () => {
+			mocked(access).mockResolvedValue(undefined);
+
+			await communityPackagesService.ensurePackageJson();
+
+			expect(access).toHaveBeenCalledWith(packageJsonPath, constants.F_OK);
+			expect(mkdir).not.toHaveBeenCalled();
+			expect(writeFile).not.toHaveBeenCalled();
+		});
+
+		test('should create package.json if it does not exist', async () => {
+			mocked(access).mockRejectedValue(new Error('ENOENT'));
+
+			await communityPackagesService.ensurePackageJson();
+
+			expect(access).toHaveBeenCalledWith(packageJsonPath, constants.F_OK);
+			expect(mkdir).toHaveBeenCalledWith(nodesDownloadDir, { recursive: true });
+			expect(writeFile).toHaveBeenCalledWith(
+				packageJsonPath,
+				JSON.stringify(
+					{
+						name: 'installed-nodes',
+						private: true,
+						dependencies: {},
+					},
+					null,
+					2,
+				),
+				'utf-8',
+			);
+		});
+	});
+
+	describe('checkForMissingPackages', () => {
+		const installedPackage1 = mock<InstalledPackages>({
+			packageName: 'package-1',
+			installedVersion: '1.0.0',
+			installedNodes: [{ type: 'node-type-1' }],
+		});
+		const installedPackage2 = mock<InstalledPackages>({
+			packageName: 'package-2',
+			installedVersion: '2.0.0',
+			installedNodes: [{ type: 'node-type-2' }],
+		});
+
+		beforeEach(() => {
+			jest
+				.spyOn(communityPackagesService, 'installPackage')
+				.mockResolvedValue({} as InstalledPackages);
+		});
+
+		test('should set missingPackages to empty array when no packages are missing', async () => {
+			const installedPackages = [installedPackage1];
+
+			installedPackageRepository.find.mockResolvedValue(installedPackages);
+			loadNodesAndCredentials.isKnownNode.mockReturnValue(true);
+
+			await communityPackagesService.checkForMissingPackages();
+
+			expect(communityPackagesService.missingPackages).toEqual([]);
+			expect(communityPackagesService.installPackage).not.toHaveBeenCalled();
+			expect(loadNodesAndCredentials.postProcessLoaders).not.toHaveBeenCalled();
+		});
+
+		test('should identify missing packages without reinstalling when reinstallMissing is false', async () => {
+			const installedPackages = [installedPackage1, installedPackage2];
+
+			installedPackageRepository.find.mockResolvedValue(installedPackages);
+			loadNodesAndCredentials.isKnownNode.mockImplementation(
+				(nodeType) => nodeType === 'node-type-2',
+			);
+			globalConfig.nodes.communityPackages.reinstallMissing = false;
+
+			await communityPackagesService.checkForMissingPackages();
+
+			expect(communityPackagesService.missingPackages).toEqual(['package-1@1.0.0']);
+			expect(communityPackagesService.installPackage).not.toHaveBeenCalled();
+			expect(loadNodesAndCredentials.postProcessLoaders).not.toHaveBeenCalled();
+			expect(logger.warn).toHaveBeenCalled();
+		});
+
+		test('should reinstall missing packages when reinstallMissing is true', async () => {
+			const installedPackages = [installedPackage1];
+
+			installedPackageRepository.find.mockResolvedValue(installedPackages);
+			loadNodesAndCredentials.isKnownNode.mockReturnValue(false);
+			globalConfig.nodes.communityPackages.reinstallMissing = true;
+
+			await communityPackagesService.checkForMissingPackages();
+
+			expect(communityPackagesService.installPackage).toHaveBeenCalledWith('package-1', '1.0.0');
+			expect(loadNodesAndCredentials.postProcessLoaders).toHaveBeenCalled();
+			expect(communityPackagesService.missingPackages).toEqual([]);
+			expect(logger.info).toHaveBeenCalledWith(
+				'Packages reinstalled successfully. Resuming regular initialization.',
+			);
+		});
+
+		test('should handle failed reinstallations and record missing packages', async () => {
+			const installedPackages = [installedPackage1];
+
+			installedPackageRepository.find.mockResolvedValue(installedPackages);
+			loadNodesAndCredentials.isKnownNode.mockReturnValue(false);
+			globalConfig.nodes.communityPackages.reinstallMissing = true;
+			communityPackagesService.installPackage = jest
+				.fn()
+				.mockRejectedValue(new Error('Installation failed'));
+
+			await communityPackagesService.checkForMissingPackages();
+
+			expect(communityPackagesService.installPackage).toHaveBeenCalledWith('package-1', '1.0.0');
+			expect(logger.error).toHaveBeenCalledWith('n8n was unable to install the missing packages.');
+			expect(communityPackagesService.missingPackages).toEqual(['package-1@1.0.0']);
+		});
+
+		test('should handle multiple missing packages and stop reinstalling after first failure', async () => {
+			const installedPackages = [installedPackage1, installedPackage2];
+
+			installedPackageRepository.find.mockResolvedValue(installedPackages);
+			loadNodesAndCredentials.isKnownNode.mockReturnValue(false);
+			globalConfig.nodes.communityPackages.reinstallMissing = true;
+
+			// First installation succeeds, second fails
+			communityPackagesService.installPackage = jest
+				.fn()
+				.mockResolvedValueOnce({} as InstalledPackages)
+				.mockRejectedValueOnce(new Error('Installation failed'));
+
+			await communityPackagesService.checkForMissingPackages();
+
+			expect(communityPackagesService.installPackage).toHaveBeenCalledWith('package-1', '1.0.0');
+			expect(communityPackagesService.installPackage).toHaveBeenCalledWith('package-2', '2.0.0');
+			expect(logger.error).toHaveBeenCalledWith('n8n was unable to install the missing packages.');
+			expect(communityPackagesService.missingPackages).toEqual(['package-2@2.0.0']);
+		});
+	});
+
+	describe('updatePackageJsonDependency', () => {
+		beforeEach(() => {
+			jest.clearAllMocks();
+			mocked(readFile).mockResolvedValue(JSON.stringify({ dependencies: {} }));
+		});
+
+		test('should update package dependencies', async () => {
+			await communityPackagesService.updatePackageJsonDependency('test-package', '1.0.0');
+
+			expect(writeFile).toHaveBeenCalledWith(
+				`${nodesDownloadDir}/package.json`,
+				JSON.stringify({ dependencies: { 'test-package': '1.0.0' } }, null, 2),
+				'utf-8',
+			);
+		});
+
+		test('should create file and update package dependencies', async () => {
+			await communityPackagesService.updatePackageJsonDependency('test-package', '1.0.0');
+
+			expect(writeFile).toHaveBeenCalledWith(
+				`${nodesDownloadDir}/package.json`,
+				JSON.stringify({ dependencies: { 'test-package': '1.0.0' } }, null, 2),
+				'utf-8',
+			);
+		});
+	});
 });

packages/cli/src/services/community-packages.service.ts

@@ -5,10 +5,11 @@ import { InstalledPackagesRepository } from '@n8n/db';
 import { Service } from '@n8n/di';
 import axios from 'axios';
 import { exec } from 'child_process';
-import { mkdir, readFile, writeFile, rm } from 'fs/promises';
+import { access, constants, mkdir, readFile, rm, writeFile } from 'fs/promises';
 import type { PackageDirectoryLoader } from 'n8n-core';
 import { InstanceSettings, Logger } from 'n8n-core';
-import { UnexpectedError, UserError, type PublicInstalledPackage } from 'n8n-workflow';
+import { jsonParse, UnexpectedError, UserError, type PublicInstalledPackage } from 'n8n-workflow';
+import { join } from 'path';
 import { promisify } from 'util';
 
 import {
@@ -56,12 +57,22 @@ const asyncExec = promisify(exec);
 
 const INVALID_OR_SUSPICIOUS_PACKAGE_NAME = /[^0-9a-z@\-./]/;
 
+type PackageJson = {
+	name: 'installed-nodes';
+	private: true;
+	dependencies: Record<string, string>;
+};
+
 @Service()
 export class CommunityPackagesService {
 	reinstallMissingPackages = false;
 
 	missingPackages: string[] = [];
 
+	private readonly downloadFolder = this.instanceSettings.nodesDownloadDir;
+
+	private readonly packageJsonPath = join(this.downloadFolder, 'package.json');
+
 	constructor(
 		private readonly instanceSettings: InstanceSettings,
 		private readonly logger: Logger,
@@ -72,6 +83,11 @@ export class CommunityPackagesService {
 		private readonly globalConfig: GlobalConfig,
 	) {}
 
+	async init() {
+		await this.ensurePackageJson();
+		await this.checkForMissingPackages();
+	}
+
 	get hasMissingPackages() {
 		return this.missingPackages.length > 0;
 	}
@@ -136,10 +152,8 @@ export class CommunityPackagesService {
 
 	/** @deprecated */
 	async executeNpmCommand(command: string, options?: { doNotHandleError?: boolean }) {
-		const downloadFolder = this.instanceSettings.nodesDownloadDir;
-
 		const execOptions = {
-			cwd: downloadFolder,
+			cwd: this.downloadFolder,
 			env: {
 				NODE_PATH: process.env.NODE_PATH,
 				PATH: process.env.PATH,
@@ -148,8 +162,6 @@ export class CommunityPackagesService {
 			},
 		};
 
-		await mkdir(downloadFolder, { recursive: true });
-
 		try {
 			const commandResult = await asyncExec(command, execOptions);
 
@@ -264,6 +276,20 @@ export class CommunityPackagesService {
 		}
 	}
 
+	async ensurePackageJson() {
+		try {
+			await access(this.packageJsonPath, constants.F_OK);
+		} catch {
+			await mkdir(this.downloadFolder, { recursive: true });
+			const packageJson: PackageJson = {
+				name: 'installed-nodes',
+				private: true,
+				dependencies: {},
+			};
+			await writeFile(this.packageJsonPath, JSON.stringify(packageJson, null, 2), 'utf-8');
+		}
+	}
+
 	async checkForMissingPackages() {
 		const installedPackages = await this.getAllInstalledPackages();
 		const missingPackages = new Set<{ packageName: string; version: string }>();
@@ -435,13 +461,11 @@ export class CommunityPackagesService {
 	}
 
 	private resolvePackageDirectory(packageName: string) {
-		const downloadFolder = this.instanceSettings.nodesDownloadDir;
+		return `${this.downloadFolder}/node_modules/${packageName}`;
-		return `${downloadFolder}/node_modules/${packageName}`;
 	}
 
 	private async downloadPackage(packageName: string, packageVersion: string): Promise<string> {
 		const registry = this.getNpmRegistry();
-		const downloadFolder = this.instanceSettings.nodesDownloadDir;
 		const packageDirectory = this.resolvePackageDirectory(packageName);
 
 		// (Re)create the packageDir
@@ -453,27 +477,37 @@ export class CommunityPackagesService {
 
 		const { stdout: tarOutput } = await asyncExec(
 			`npm pack ${packageName}@${packageVersion} --registry=${registry} --quiet`,
-			{ cwd: downloadFolder },
+			{ cwd: this.downloadFolder },
 		);
 
 		const tarballName = tarOutput?.trim();
 
 		try {
 			await asyncExec(`tar -xzf ${tarballName} -C ${packageDirectory} --strip-components=1`, {
-				cwd: downloadFolder,
+				cwd: this.downloadFolder,
 			});
 
 			// Strip dev, optional, and peer dependencies before running `npm install`
 			const packageJsonPath = `${packageDirectory}/package.json`;
 			const packageJsonContent = await readFile(packageJsonPath, 'utf-8');
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
-			const { devDependencies, peerDependencies, optionalDependencies, ...packageJson } =
+			const {
-				JSON.parse(packageJsonContent);
+				devDependencies,
+				peerDependencies,
+				optionalDependencies,
+				...packageJson
+			}: {
+				version: string;
+				devDependencies: Record<string, string>;
+				peerDependencies: Record<string, string>;
+				optionalDependencies: Record<string, string>;
+			} = JSON.parse(packageJsonContent);
 			await writeFile(packageJsonPath, JSON.stringify(packageJson, null, 2), 'utf-8');
 
 			await asyncExec(`npm install ${this.getNpmInstallArgs()}`, { cwd: packageDirectory });
+			await this.updatePackageJsonDependency(packageName, packageJson.version);
 		} finally {
-			await asyncExec(`rm ${tarballName}`, { cwd: downloadFolder });
+			await rm(join(this.downloadFolder, tarballName));
 		}
 
 		return packageDirectory;
@@ -483,4 +517,11 @@ export class CommunityPackagesService {
 		const packageDirectory = this.resolvePackageDirectory(packageName);
 		await rm(packageDirectory, { recursive: true, force: true });
 	}
+
+	async updatePackageJsonDependency(packageName: string, version: string) {
+		const existingContent = await readFile(this.packageJsonPath, 'utf-8');
+		const packageJson = jsonParse<PackageJson>(existingContent);
+		packageJson.dependencies[packageName] = version;
+		await writeFile(this.packageJsonPath, JSON.stringify(packageJson, null, 2), 'utf-8');
+	}
 }

packages/cli/test/integration/commands/worker.cmd.test.ts

@@ -16,6 +16,7 @@ import { Push } from '@/push';
 import { Publisher } from '@/scaling/pubsub/publisher.service';
 import { Subscriber } from '@/scaling/pubsub/subscriber.service';
 import { ScalingService } from '@/scaling/scaling.service';
+import { CommunityPackagesService } from '@/services/community-packages.service';
 import { TaskBrokerServer } from '@/task-runners/task-broker/task-broker-server';
 import { TaskRunnerProcess } from '@/task-runners/task-runner-process';
 import { Telemetry } from '@/telemetry';
@@ -28,6 +29,7 @@ config.set('binaryDataManager.availableModes', 'filesystem');
 Container.get(TaskRunnersConfig).enabled = true;
 mockInstance(LoadNodesAndCredentials);
 const binaryDataService = mockInstance(BinaryDataService);
+const communityPackagesService = mockInstance(CommunityPackagesService);
 const externalHooks = mockInstance(ExternalHooks);
 const externalSecretsManager = mockInstance(ExternalSecretsManager);
 const license = mockInstance(License, { loadCertStr: async () => '' });
@@ -50,6 +52,7 @@ test('worker initializes all its components', async () => {
 
 	expect(license.init).toHaveBeenCalledTimes(1);
 	expect(binaryDataService.init).toHaveBeenCalledTimes(1);
+	expect(communityPackagesService.init).toHaveBeenCalledTimes(1);
 	expect(externalHooks.init).toHaveBeenCalledTimes(1);
 	expect(externalSecretsManager.init).toHaveBeenCalledTimes(1);
 	expect(messageEventBus.initialize).toHaveBeenCalledTimes(1);