Skip to content

feat(apps): stop persisting intermediate status transitions to database #37167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kodiakhq merged 13 commits into from
Nov 10, 2025
Merged

feat(apps): stop persisting intermediate status transitions to database #37167

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c61c4a2
feat: stop persisting status changes on all transitions
d-gubert Oct 8, 2025
16573fd
feat: persist manual status transitions
d-gubert Oct 8, 2025
10ad3c1
chore: remove unnecessary fields in update descriptor
d-gubert Oct 8, 2025
4f55907
chore: capture possible license validation error during disable
d-gubert Oct 8, 2025
35dfd01
feat: target status on install is manually_enabled
d-gubert Oct 9, 2025
a5e8587
refactor: initializeApp calls
d-gubert Oct 27, 2025
1fb56a5
refactor: installApp calls
d-gubert Oct 27, 2025
13e17fa
refactor: enableApp calls
d-gubert Oct 27, 2025
703d2e3
fix: descriptor instantiation
d-gubert Oct 27, 2025
3668e81
fix: make install more consistent
d-gubert Oct 27, 2025
3978c93
chore: adapt tests to new behavior
d-gubert Oct 27, 2025
1aefafe
add changeset
d-gubert Oct 29, 2025
4c145ca
refactor: remove unused parameter
d-gubert Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/real-pans-confess.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 2 additions & 2 deletions apps/meteor/ee/server/apps/communication/rest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/tests/end-to-end/apps/installation.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
})
Expand Down
96 changes: 27 additions & 69 deletions packages/apps-engine/src/server/AppManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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).catch(console.error);
} else if (!AppStatusUtils.isError(status)) {
this.listenerManager.lockEssentialEvents(app);
this.uiActionButtonManager.clearAppActionButtons(app.getID());
Expand Down Expand Up @@ -457,14 +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);

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(() => {});
}
const isSetup = await this.runStartUpProcess(storageItem, rl, false);

return isSetup;
}
Expand Down Expand Up @@ -495,12 +488,7 @@ export class AppManager {
const storageItem = await this.appMetadataStorage.retrieveOne(id);

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(() => {});
await app.validateLicense().catch(() => {});

return true;
}
Expand Down Expand Up @@ -570,7 +558,7 @@ export class AppManager {
const descriptor: IAppStorageItem = {
id: result.info.id,
info: result.info,
status: AppStatus.UNKNOWN,
status: enable ? AppStatus.MANUALLY_ENABLED : AppStatus.MANUALLY_DISABLED,
settings: {},
implemented: result.implemented.getValues(),
installationSource: marketplaceInfo ? AppInstallationSource.MARKETPLACE : AppInstallationSource.PRIVATE,
Expand Down Expand Up @@ -645,15 +633,15 @@ 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.
if (enable) {
// Start up the app
await this.runStartUpProcess(created, app, false, false);
await this.runStartUpProcess(created, app, false);
} else {
await this.initializeApp(created, app, true);
await this.initializeApp(app);
}

return aff;
Expand Down Expand Up @@ -727,15 +715,10 @@ export class AppManager {

const descriptor: IAppStorageItem = {
...old,
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(),
...(old.marketplaceInfo && { marketplaceInfo: old.marketplaceInfo }),
...(old.sourcePath && { sourcePath: old.sourcePath }),
};

if (!permissionsGranted) {
Expand Down Expand Up @@ -833,12 +816,12 @@ 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) {
const app = await this.updateLocal(stored, appPackageOrInstance);
await this.initializeApp(stored, app, true, true);
await this.initializeApp(app, true);
}

public getLanguageContent(): { [key: string]: object } {
Expand Down Expand Up @@ -870,19 +853,27 @@ 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())) {
throw new Error('Can not enable an App which is already enabled.');
}

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;
Expand Down Expand Up @@ -973,27 +964,22 @@ 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);
}

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, silenceStatus);
}

return this.apps.get(item.id);
}

private async runStartUpProcess(
storageItem: IAppStorageItem,
app: ProxiedApp,
isManual: boolean,
silenceStatus: boolean,
): Promise<boolean> {
private async runStartUpProcess(storageItem: IAppStorageItem, app: ProxiedApp, silenceStatus: boolean): Promise<boolean> {
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;
}
Expand All @@ -1004,10 +990,10 @@ export class AppManager {
return false;
}

return this.enableApp(storageItem, app, true, isManual, silenceStatus);
return this.enableApp(app, silenceStatus);
}

private async installApp(_storageItem: IAppStorageItem, app: ProxiedApp, user: IUser): Promise<boolean> {
private async installApp(app: ProxiedApp, user: IUser): Promise<boolean> {
let result: boolean;
const context = { user };

Expand Down Expand Up @@ -1044,7 +1030,7 @@ export class AppManager {
return result;
}

private async initializeApp(storageItem: IAppStorageItem, app: ProxiedApp, saveToDb = true, silenceStatus = false): Promise<boolean> {
private async initializeApp(app: ProxiedApp, silenceStatus = false): Promise<boolean> {
let result: boolean;

try {
Expand Down Expand Up @@ -1072,17 +1058,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;
Expand Down Expand Up @@ -1133,13 +1108,7 @@ export class AppManager {
return result;
}

private async enableApp(
storageItem: IAppStorageItem,
app: ProxiedApp,
saveToDb = true,
isManual: boolean,
silenceStatus = false,
): Promise<boolean> {
private async enableApp(app: ProxiedApp, silenceStatus = false): Promise<boolean> {
let enable: boolean;
let status = AppStatus.ERROR_DISABLED;

Expand All @@ -1150,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.`);
Expand All @@ -1167,10 +1136,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) {
Expand All @@ -1188,13 +1153,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;
Expand Down
Loading