From c61c4a2346744c3fe34fd730b25d9da6eab83715 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 8 Oct 2025 09:02:18 -0300 Subject: [PATCH 01/13] feat: stop persisting status changes on all transitions --- packages/apps-engine/src/server/AppManager.ts | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 173dfc18fbd76..85888448b6266 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -459,13 +459,6 @@ export class AppManager { const isSetup = await this.runStartUpProcess(storageItem, rl, true, false); - if (isSetup) { - storageItem.status = await rl.getStatus(); - // This is async, but we don't care since it only updates in the database - // and it should not mutate any properties we care about - await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {}); - } - return isSetup; } @@ -497,11 +490,6 @@ export class AppManager { app.getStorageItem().marketplaceInfo = storageItem.marketplaceInfo; await app.validateLicense().catch(); - storageItem.status = await app.getStatus(); - // This is async, but we don't care since it only updates in the database - // and it should not mutate any properties we care about - await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {}); - return true; } @@ -730,7 +718,6 @@ export class AppManager { createdAt: old.createdAt, id: result.info.id, info: result.info, - status: (await this.apps.get(old.id)?.getStatus()) || old.status, languageContent: result.languageContent, settings: old.settings, implemented: result.implemented.getValues(), @@ -1072,17 +1059,6 @@ export class AppManager { result = false; await app.setStatus(status, silenceStatus); - - // If some error has happened in initialization, like license or installations invalidation - // we need to store this on the DB regardless of what the parameter requests - saveToDb = true; - } - - if (saveToDb) { - // This is async, but we don't care since it only updates in the database - // and it should not mutate any properties we care about - storageItem.status = await app.getStatus(); - await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {}); } return result; @@ -1167,10 +1143,6 @@ export class AppManager { } console.error(e); - - // If some error has happened during enabling, like license or installations invalidation - // we need to store this on the DB regardless of what the parameter requests - saveToDb = true; } if (enable) { @@ -1188,13 +1160,6 @@ export class AppManager { }); } - if (saveToDb) { - storageItem.status = status; - // This is async, but we don't care since it only updates in the database - // and it should not mutate any properties we care about - await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {}); - } - await app.setStatus(status, silenceStatus); return enable; From 16573fd0891f9ba2d519aad3b856b39801c4f26a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 8 Oct 2025 09:07:33 -0300 Subject: [PATCH 02/13] feat: persist manual status transitions --- packages/apps-engine/src/server/AppManager.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 85888448b6266..743f89cc14c6e 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -857,6 +857,8 @@ export class AppManager { throw new Error('Can not change the status of an App which does not currently exist.'); } + const storageItem = await rl.getStorageItem(); + if (AppStatusUtils.isEnabled(status)) { // Then enable it if (AppStatusUtils.isEnabled(await rl.getStatus())) { @@ -864,12 +866,18 @@ export class AppManager { } await this.enable(rl.getID()); + + storageItem.status = AppStatus.MANUALLY_ENABLED; + await this.appMetadataStorage.updateStatus(storageItem._id, AppStatus.MANUALLY_ENABLED); } else { if (!AppStatusUtils.isEnabled(await rl.getStatus())) { throw new Error('Can not disable an App which is not enabled.'); } await this.disable(rl.getID(), AppStatus.MANUALLY_DISABLED); + + storageItem.status = AppStatus.MANUALLY_DISABLED; + await this.appMetadataStorage.updateStatus(storageItem._id, AppStatus.MANUALLY_DISABLED); } return rl; From 10ad3c1f2d92930385338c787d874b2a71dda46b Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 8 Oct 2025 13:42:33 -0300 Subject: [PATCH 03/13] chore: remove unnecessary fields in update descriptor --- packages/apps-engine/src/server/AppManager.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 743f89cc14c6e..cfe17e037823c 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -715,14 +715,10 @@ export class AppManager { const descriptor: IAppStorageItem = { ...old, - createdAt: old.createdAt, id: result.info.id, info: result.info, languageContent: result.languageContent, - settings: old.settings, implemented: result.implemented.getValues(), - ...(old.marketplaceInfo && { marketplaceInfo: old.marketplaceInfo }), - ...(old.sourcePath && { sourcePath: old.sourcePath }), }; if (!permissionsGranted) { From 4f559076916c24e277944eb24a98e8ff715725c6 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 8 Oct 2025 13:44:56 -0300 Subject: [PATCH 04/13] chore: capture possible license validation error during disable --- packages/apps-engine/src/server/AppManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index cfe17e037823c..76f404b508453 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -488,7 +488,7 @@ export class AppManager { const storageItem = await this.appMetadataStorage.retrieveOne(id); app.getStorageItem().marketplaceInfo = storageItem.marketplaceInfo; - await app.validateLicense().catch(); + await app.validateLicense().catch(() => {}); return true; } From 35dfd01e90b578b0e03e9532e4a08d4f396b1823 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 9 Oct 2025 17:18:01 -0300 Subject: [PATCH 05/13] feat: target status on install is manually_enabled --- packages/apps-engine/src/server/AppManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 76f404b508453..d64ff08b9aa4a 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -558,7 +558,7 @@ export class AppManager { const descriptor: IAppStorageItem = { id: result.info.id, info: result.info, - status: AppStatus.UNKNOWN, + status: AppStatus.MANUALLY_ENABLED, settings: {}, implemented: result.implemented.getValues(), installationSource: marketplaceInfo ? AppInstallationSource.MARKETPLACE : AppInstallationSource.PRIVATE, From a5e8587231182b67e301f13fe0834e0bd968cc59 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 13:55:29 -0300 Subject: [PATCH 06/13] refactor: initializeApp calls --- packages/apps-engine/src/server/AppManager.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index d64ff08b9aa4a..9966012d6f3f4 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -310,7 +310,7 @@ export class AppManager { continue; } - await this.initializeApp(rl.getStorageItem(), rl, false, true).catch(console.error); + await this.initializeApp(rl, true).catch(console.error); } // Let's ensure the required settings are all set @@ -641,7 +641,7 @@ export class AppManager { // Start up the app await this.runStartUpProcess(created, app, false, false); } else { - await this.initializeApp(created, app, true); + await this.initializeApp(app); } return aff; @@ -821,7 +821,7 @@ export class AppManager { public async updateAndInitializeLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) { const app = await this.updateLocal(stored, appPackageOrInstance); - await this.initializeApp(stored, app, true, true); + await this.initializeApp(app, true); } public getLanguageContent(): { [key: string]: object } { @@ -964,7 +964,7 @@ export class AppManager { const item = rl.getStorageItem(); - await this.initializeApp(item, rl, false, silenceStatus); + await this.initializeApp(rl, silenceStatus); if (!this.areRequiredSettingsSet(item)) { await rl.setStatus(AppStatus.INVALID_SETTINGS_DISABLED); @@ -984,7 +984,7 @@ export class AppManager { silenceStatus: boolean, ): Promise { if ((await app.getStatus()) !== AppStatus.INITIALIZED) { - const isInitialized = await this.initializeApp(storageItem, app, true, silenceStatus); + const isInitialized = await this.initializeApp(app, silenceStatus); if (!isInitialized) { return false; } @@ -1035,7 +1035,7 @@ export class AppManager { return result; } - private async initializeApp(storageItem: IAppStorageItem, app: ProxiedApp, saveToDb = true, silenceStatus = false): Promise { + private async initializeApp(app: ProxiedApp, silenceStatus = false): Promise { let result: boolean; try { From 1fb56a509ff7bfaa774726055541a0b688ea1d9b Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 14:27:40 -0300 Subject: [PATCH 07/13] refactor: installApp calls --- packages/apps-engine/src/server/AppManager.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 9966012d6f3f4..7384e0fb693e2 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -633,7 +633,7 @@ export class AppManager { // If an error occurs during this, oh well. }); - await this.installApp(created, app, user); + await this.installApp(app, user); // Should enable === true, then we go through the entire start up process // Otherwise, we only initialize it. @@ -998,7 +998,7 @@ export class AppManager { return this.enableApp(storageItem, app, true, isManual, silenceStatus); } - private async installApp(_storageItem: IAppStorageItem, app: ProxiedApp, user: IUser): Promise { + private async installApp(app: ProxiedApp, user: IUser): Promise { let result: boolean; const context = { user }; From 13e17fae91f50d7e98838a320e110ce0e2284609 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 14:28:20 -0300 Subject: [PATCH 08/13] refactor: enableApp calls --- packages/apps-engine/src/server/AppManager.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 7384e0fb693e2..acc2ab88f6563 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -329,7 +329,7 @@ export class AppManager { for (const app of this.apps.values()) { const status = await app.getStatus(); if (!AppStatusUtils.isDisabled(status) && AppStatusUtils.isEnabled(app.getPreviousStatus())) { - await this.enableApp(app.getStorageItem(), app, true, app.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error); + await this.enableApp(app, app.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error); } else if (!AppStatusUtils.isError(status)) { this.listenerManager.lockEssentialEvents(app); this.uiActionButtonManager.clearAppActionButtons(app.getID()); @@ -971,7 +971,7 @@ export class AppManager { } if (!AppStatusUtils.isDisabled(await rl.getStatus()) && AppStatusUtils.isEnabled(rl.getPreviousStatus())) { - await this.enableApp(item, rl, false, rl.getPreviousStatus() === AppStatus.MANUALLY_ENABLED, silenceStatus); + await this.enableApp(rl, rl.getPreviousStatus() === AppStatus.MANUALLY_ENABLED, silenceStatus); } return this.apps.get(item.id); @@ -995,7 +995,7 @@ export class AppManager { return false; } - return this.enableApp(storageItem, app, true, isManual, silenceStatus); + return this.enableApp(app, isManual, silenceStatus); } private async installApp(app: ProxiedApp, user: IUser): Promise { @@ -1113,13 +1113,7 @@ export class AppManager { return result; } - private async enableApp( - storageItem: IAppStorageItem, - app: ProxiedApp, - saveToDb = true, - isManual: boolean, - silenceStatus = false, - ): Promise { + private async enableApp(app: ProxiedApp, isManual: boolean, silenceStatus = false): Promise { let enable: boolean; let status = AppStatus.ERROR_DISABLED; From 703d2e3e00bf89102bf7886d9a3a12b517d371ae Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 15:15:53 -0300 Subject: [PATCH 09/13] fix: descriptor instantiation --- packages/apps-engine/src/server/AppManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index acc2ab88f6563..649225c0aa4af 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -558,7 +558,7 @@ export class AppManager { const descriptor: IAppStorageItem = { id: result.info.id, info: result.info, - status: AppStatus.MANUALLY_ENABLED, + status: enable ? AppStatus.MANUALLY_ENABLED : AppStatus.MANUALLY_DISABLED, settings: {}, implemented: result.implemented.getValues(), installationSource: marketplaceInfo ? AppInstallationSource.MARKETPLACE : AppInstallationSource.PRIVATE, From 3668e8107f80c7ada39f1ddb2c3d46defc12b3e5 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 17:42:18 -0300 Subject: [PATCH 10/13] fix: make install more consistent --- apps/meteor/ee/server/apps/communication/rest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/ee/server/apps/communication/rest.ts b/apps/meteor/ee/server/apps/communication/rest.ts index 152a43a93a464..3f001a6805d40 100644 --- a/apps/meteor/ee/server/apps/communication/rest.ts +++ b/apps/meteor/ee/server/apps/communication/rest.ts @@ -474,8 +474,8 @@ export class AppsRestApi { try { await canEnableApp(aff.getApp().getStorageItem()); - const success = await manager.enable(info.id); - info.status = success ? AppStatus.AUTO_ENABLED : info.status; + const success = await manager.changeStatus(info.id, AppStatus.MANUALLY_ENABLED); + info.status = await success.getStatus(); } catch (error) { orchestrator.getRocketChatLogger().warn(`App "${info.id}" was installed but could not be enabled: `, error); } From 3978c936e821dba16ed5df6683d6a758901d9cf5 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 27 Oct 2025 19:13:46 -0300 Subject: [PATCH 11/13] chore: adapt tests to new behavior --- apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts | 2 +- apps/meteor/tests/end-to-end/apps/installation.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts b/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts index 4bb7d383ed7ff..34c8367544bda 100644 --- a/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts +++ b/apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts @@ -101,7 +101,7 @@ describe('LIVECHAT - rooms', () => { expect(res.body).to.have.a.property('app'); expect(res.body.app).to.have.a.property('id'); expect(res.body.app).to.have.a.property('version'); - expect(res.body.app).to.have.a.property('status').and.to.be.equal('auto_enabled'); + expect(res.body.app).to.have.a.property('status').and.to.be.equal('manually_enabled'); appId = res.body.app.id; }); diff --git a/apps/meteor/tests/end-to-end/apps/installation.ts b/apps/meteor/tests/end-to-end/apps/installation.ts index 4fac049bcae93..9f814c6089f26 100644 --- a/apps/meteor/tests/end-to-end/apps/installation.ts +++ b/apps/meteor/tests/end-to-end/apps/installation.ts @@ -52,7 +52,7 @@ describe('Apps - Installation', () => { expect(res.body).to.have.a.property('app'); expect(res.body.app).to.have.a.property('id'); expect(res.body.app).to.have.a.property('version'); - expect(res.body.app).to.have.a.property('status').and.to.be.equal('auto_enabled'); + expect(res.body.app).to.have.a.property('status').and.to.be.equal('manually_enabled'); app = res.body.app; }) From 1aefafe825ec3a01405b94bdde95ae53fc55ecba Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Tue, 28 Oct 2025 21:08:30 -0300 Subject: [PATCH 12/13] add changeset --- .changeset/real-pans-confess.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 .changeset/real-pans-confess.md diff --git a/.changeset/real-pans-confess.md b/.changeset/real-pans-confess.md new file mode 100644 index 0000000000000..5a6c5902a2f6d --- /dev/null +++ b/.changeset/real-pans-confess.md @@ -0,0 +1,8 @@ +--- +'@rocket.chat/apps-engine': minor +'@rocket.chat/meteor': minor +--- + +Changes a behavior that would store the result of every status transition that happened to apps + +This caused intermediate status to be saved to the database, which could prevent apps from being restored to the desired status when restarted or during server startup. From 4c145ca094234e036c9a2be701487c01a5c5f573 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 30 Oct 2025 15:46:26 -0300 Subject: [PATCH 13/13] refactor: remove unused parameter --- packages/apps-engine/src/server/AppManager.ts | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/packages/apps-engine/src/server/AppManager.ts b/packages/apps-engine/src/server/AppManager.ts index 649225c0aa4af..4c8348b726653 100644 --- a/packages/apps-engine/src/server/AppManager.ts +++ b/packages/apps-engine/src/server/AppManager.ts @@ -329,7 +329,7 @@ export class AppManager { for (const app of this.apps.values()) { const status = await app.getStatus(); if (!AppStatusUtils.isDisabled(status) && AppStatusUtils.isEnabled(app.getPreviousStatus())) { - await this.enableApp(app, app.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error); + await this.enableApp(app).catch(console.error); } else if (!AppStatusUtils.isError(status)) { this.listenerManager.lockEssentialEvents(app); this.uiActionButtonManager.clearAppActionButtons(app.getID()); @@ -457,7 +457,7 @@ export class AppManager { throw new Error(`Could not enable an App with the id of "${id}" as it doesn't exist.`); } - const isSetup = await this.runStartUpProcess(storageItem, rl, true, false); + const isSetup = await this.runStartUpProcess(storageItem, rl, false); return isSetup; } @@ -639,7 +639,7 @@ export class AppManager { // Otherwise, we only initialize it. if (enable) { // Start up the app - await this.runStartUpProcess(created, app, false, false); + await this.runStartUpProcess(created, app, false); } else { await this.initializeApp(app); } @@ -816,7 +816,7 @@ export class AppManager { public async updateAndStartupLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) { const app = await this.updateLocal(stored, appPackageOrInstance); - await this.runStartUpProcess(stored, app, false, true); + await this.runStartUpProcess(stored, app, true); } public async updateAndInitializeLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) { @@ -971,18 +971,13 @@ export class AppManager { } if (!AppStatusUtils.isDisabled(await rl.getStatus()) && AppStatusUtils.isEnabled(rl.getPreviousStatus())) { - await this.enableApp(rl, rl.getPreviousStatus() === AppStatus.MANUALLY_ENABLED, silenceStatus); + await this.enableApp(rl, silenceStatus); } return this.apps.get(item.id); } - private async runStartUpProcess( - storageItem: IAppStorageItem, - app: ProxiedApp, - isManual: boolean, - silenceStatus: boolean, - ): Promise { + private async runStartUpProcess(storageItem: IAppStorageItem, app: ProxiedApp, silenceStatus: boolean): Promise { if ((await app.getStatus()) !== AppStatus.INITIALIZED) { const isInitialized = await this.initializeApp(app, silenceStatus); if (!isInitialized) { @@ -995,7 +990,7 @@ export class AppManager { return false; } - return this.enableApp(app, isManual, silenceStatus); + return this.enableApp(app, silenceStatus); } private async installApp(app: ProxiedApp, user: IUser): Promise { @@ -1113,7 +1108,7 @@ export class AppManager { return result; } - private async enableApp(app: ProxiedApp, isManual: boolean, silenceStatus = false): Promise { + private async enableApp(app: ProxiedApp, silenceStatus = false): Promise { let enable: boolean; let status = AppStatus.ERROR_DISABLED; @@ -1124,7 +1119,7 @@ export class AppManager { enable = (await app.call(AppMethod.ONENABLE)) as boolean; if (enable) { - status = isManual ? AppStatus.MANUALLY_ENABLED : AppStatus.AUTO_ENABLED; + status = AppStatus.MANUALLY_ENABLED; } else { status = AppStatus.DISABLED; console.warn(`The App (${app.getID()}) disabled itself when being enabled. \nCheck the "onEnable" implementation for details.`);