From 67ed2689cef3397ccaa43e88128c9995f371a687 Mon Sep 17 00:00:00 2001 From: martmull Date: Wed, 15 Jul 2026 19:36:01 +0200 Subject: [PATCH] Unify manifest apply pipeline between application install and dev sync (#22921) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First step of unifying application behaviors across sources (NPM, TARBALL, LOCAL), following up on #22868. No storage layout changes. ## Problem Marketplace/tarball installs (`ApplicationInstallService`) and CLI dev sync (`ApplicationDevelopmentService`) each implemented the second half of application delivery — metadata sync, SDK client generation, registration refresh — with quietly diverging behavior: - Install decided SDK regeneration on `!isVersionUpgrade` (application row exists), dev sync on `!application.version`. The version-based check is the robust one: a failed first install leaves an application row without a version, and the row-based check then skipped SDK generation on retry unless the schema changed. - Install refreshed the registration manifest (`updateFromManifest`) without syncing its variable schemas, while catalog sync, tarball upload, and dev sync all do. An NPM install that bumped `latestAvailableVersion` with new `serverVariables` left the registration's variable schemas stale until the next catalog cron. - The guards protecting shared registrations from workspace writes (npm-sourced or not owned by the workspace) lived only inside dev sync's `syncRegistrationMetadata`. ## Change New `ApplicationManifestApplyService` (application-manifest module) owns those steps for both flows: - `applyManifestToWorkspace`: `synchronizeFromManifest` + SDK client generation when it's the first successful apply (no persisted version) or the schema changed. Used by install and dev sync. - `refreshRegistrationFromManifest`: guarded registration update + variable schema sync. Callers acting for a workspace (dev sync) pass `onlyIfOwnedByWorkspaceId`, moving the npm/ownership guard into the shared service; installs pass `latestAvailableVersion` + `preventVersionDowngrade` as before. Returns whether anything was written so dependent side effects (registration asset store in dev sync) skip together with it. - `ApplicationRegistrationService.updateFromManifest` now returns whether it wrote, so variable schemas are only synced from manifests that were actually applied — previously a downgraded install would still have synced variables from the older manifest if we had naively added the sync there. Install and dev services lose the duplicated logic (and their direct `SdkClientGenerationService` / variable-service dependencies); hooks and file writes stay where they were. ## Behavior deltas (all deliberate) - NPM/TARBALL installs now sync registration variable schemas when they refresh the registration. - Retrying a failed first install regenerates the SDK client even when the schema diff is empty. ## Follow-ups (agreed plan, separate PRs) Single semver version gate; one registration-metadata writer with lock coverage for dev-sync/tarball; unified upgrade incl. TARBALL; recoverable upgrades + stale file cleanup; tarball storage decoupled from the owner workspace; orphan cleanup cron; faster public-asset serving (ETag/304); parallel install file writes; PREBUILT execution mode for app logic functions. ## Validation - New spec `application-manifest-apply.service.spec.ts` (9 tests: SDK generation matrix, ownership/npm guards, downgrade-skip short-circuits variable sync) - All 26 application suites pass; typecheck, oxlint (type-aware) and oxfmt green on twenty-server --- _Generated by [Claude Code](https://claude.ai/code/session_015L4zvAL9bsk7azYkbhcZSg)_ Review in cubic --- .../application-development.module.ts | 2 - .../application-development.service.ts | 56 ++--- .../application-install.module.ts | 2 - .../application-install.service.ts | 54 ++--- ...application-manifest-apply.service.spec.ts | 226 ++++++++++++++++++ .../application-manifest-apply.service.ts | 102 ++++++++ .../application-manifest.module.ts | 12 +- ...plication-registration-variable.service.ts | 19 +- .../application-registration.service.ts | 122 ++++++---- 9 files changed, 458 insertions(+), 137 deletions(-) create mode 100644 packages/twenty-server/src/engine/core-modules/application/application-manifest/__tests__/application-manifest-apply.service.spec.ts create mode 100644 packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest-apply.service.ts diff --git a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.module.ts b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.module.ts index cfc554f579..0f6d078207 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.module.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.module.ts @@ -10,7 +10,6 @@ import { CacheLockModule } from 'src/engine/core-modules/cache-lock/cache-lock.m import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; import { FileStorageModule } from 'src/engine/core-modules/file-storage/file-storage.module'; import { ThrottlerModule } from 'src/engine/core-modules/throttler/throttler.module'; -import { SdkClientModule } from 'src/engine/core-modules/sdk-client/sdk-client.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-graphql-api-exception.interceptor'; @@ -22,7 +21,6 @@ import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/wor ApplicationRegistrationModule, CacheLockModule, FeatureFlagModule, - SdkClientModule, FileStorageModule, PermissionsModule, ThrottlerModule, diff --git a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts index 50ee74ff82..36ef2dac64 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts @@ -6,10 +6,10 @@ import { isDefined } from 'twenty-shared/utils'; import { type ApplicationInput } from 'src/engine/core-modules/application/application-development/dtos/application.input'; import { type DevelopmentApplicationDTO } from 'src/engine/core-modules/application/application-development/dtos/development-application.dto'; import { type WorkspaceMigrationDTO } from 'src/engine/core-modules/application/application-development/dtos/workspace-migration.dto'; +import { ApplicationManifestApplyService } from 'src/engine/core-modules/application/application-manifest/application-manifest-apply.service'; import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; import { ApplicationVersionValidationService } from 'src/engine/core-modules/application/application-package/application-version-validation.service'; import { VERSION_REASON_TO_APPLICATION_EXCEPTION_CODE } from 'src/engine/core-modules/application/application-package/constants/version-reason-to-exception-code.constant'; -import { ApplicationRegistrationVariableService } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.service'; import { ApplicationRegistrationAssetService } from 'src/engine/core-modules/application/application-registration/application-registration-asset.service'; import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service'; import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum'; @@ -22,7 +22,6 @@ import { CacheLockService } from 'src/engine/core-modules/cache-lock/cache-lock. import { FileStorageService } from 'src/engine/core-modules/file-storage/services/file-storage.service'; import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util'; import { type FileDTO } from 'src/engine/core-modules/file/dtos/file.dto'; -import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service'; import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; @@ -46,12 +45,11 @@ export class ApplicationDevelopmentService { constructor( private readonly applicationService: ApplicationService, private readonly applicationSyncService: ApplicationSyncService, + private readonly applicationManifestApplyService: ApplicationManifestApplyService, private readonly applicationRegistrationService: ApplicationRegistrationService, - private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService, private readonly applicationRegistrationAssetService: ApplicationRegistrationAssetService, private readonly applicationVersionValidationService: ApplicationVersionValidationService, private readonly fileStorageService: FileStorageService, - private readonly sdkClientGenerationService: SdkClientGenerationService, private readonly throttlerService: ThrottlerService, private readonly cacheLockService: CacheLockService, ) {} @@ -233,24 +231,14 @@ export class ApplicationDevelopmentService { ); } - const isFirstSync = !isDefined(application.version); - - const { workspaceMigration, hasSchemaMetadataChanged } = - await this.applicationSyncService.synchronizeFromManifest({ + const { workspaceMigration } = + await this.applicationManifestApplyService.applyManifestToWorkspace({ workspaceId, manifest, applicationRegistrationId, + application, }); - if (isFirstSync || hasSchemaMetadataChanged) { - await this.sdkClientGenerationService.generateSdkClientForApplication({ - workspaceId, - applicationId: application.id, - applicationUniversalIdentifier: - manifest.application.universalIdentifier, - }); - } - await this.syncRegistrationMetadata( applicationRegistrationId, manifest, @@ -299,29 +287,20 @@ export class ApplicationDevelopmentService { manifest: ApplicationInput['manifest'], workspaceId: string, ): Promise { - const registration = - await this.applicationRegistrationService.findOneByIdGlobal( - applicationRegistrationId, + const hasRefreshedRegistration = + await this.applicationManifestApplyService.refreshRegistrationFromManifest( + { + applicationRegistrationId, + manifest, + sourceType: ApplicationRegistrationSourceType.LOCAL, + onlyIfOwnedByWorkspaceId: workspaceId, + }, ); - // The registration is instance-global: for catalog-synced (npm) apps it is - // the marketplace entry and OAuth identity shared by every workspace, so - // dev-mode sync must not overwrite its manifest or flip its sourceType. - // Only registrations owned by the syncing workspace (and not npm-sourced) - // reflect local dev state. - if ( - registration.sourceType === ApplicationRegistrationSourceType.NPM || - registration.ownerWorkspaceId !== workspaceId - ) { + if (!hasRefreshedRegistration) { return; } - await this.applicationRegistrationService.updateFromManifest({ - applicationRegistrationId, - manifest, - sourceType: ApplicationRegistrationSourceType.LOCAL, - }); - // Public assets are uploaded to workspace storage before the sync, so the // logo and gallery images can be copied into the registration's // instance-global server files here. @@ -336,13 +315,6 @@ export class ApplicationDevelopmentService { path, }), }); - - if (manifest.application.serverVariables) { - await this.applicationRegistrationVariableService.syncVariableSchemas( - applicationRegistrationId, - manifest.application.serverVariables, - ); - } } private async readPublicAssetFromWorkspaceStorage({ diff --git a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.module.ts b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.module.ts index 8d8d818621..1a91add8f3 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.module.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.module.ts @@ -13,7 +13,6 @@ import { ApplicationInstallResolver } from 'src/engine/core-modules/application/ import { ApplicationInstallService } from 'src/engine/core-modules/application/application-install/application-install.service'; import { FileStorageModule } from 'src/engine/core-modules/file-storage/file-storage.module'; import { LogicFunctionModule } from 'src/engine/core-modules/logic-function/logic-function.module'; -import { SdkClientModule } from 'src/engine/core-modules/sdk-client/sdk-client.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache.module'; @@ -28,7 +27,6 @@ import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache CacheLockModule, FeatureFlagModule, LogicFunctionModule, - SdkClientModule, PermissionsModule, FileStorageModule, WorkspaceCacheModule, diff --git a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts index 67f07b5bc7..61c1cdfcb3 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.service.ts @@ -16,13 +16,13 @@ import { } from 'src/engine/core-modules/application/application.exception'; import { isImageFilePath } from 'src/engine/core-modules/application/application-registration/utils/is-image-file-path.util'; import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity'; -import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service'; import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum'; import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; import { ApplicationService } from 'src/engine/core-modules/application/application.service'; import { ApplicationPackageFetcherService } from 'src/engine/core-modules/application/application-package/application-package-fetcher.service'; import { ApplicationVersionValidationService } from 'src/engine/core-modules/application/application-package/application-version-validation.service'; import { VERSION_REASON_TO_APPLICATION_EXCEPTION_CODE } from 'src/engine/core-modules/application/application-package/constants/version-reason-to-exception-code.constant'; +import { ApplicationManifestApplyService } from 'src/engine/core-modules/application/application-manifest/application-manifest-apply.service'; import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; import { CacheLockService } from 'src/engine/core-modules/cache-lock/cache-lock.service'; import { FileStorageService } from 'src/engine/core-modules/file-storage/services/file-storage.service'; @@ -33,7 +33,6 @@ import { import { InjectMessageQueue } from 'src/engine/core-modules/message-queue/decorators/message-queue.decorator'; import { MessageQueue } from 'src/engine/core-modules/message-queue/message-queue.constants'; import { MessageQueueService } from 'src/engine/core-modules/message-queue/services/message-queue.service'; -import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service'; import { WorkspaceCacheService } from 'src/engine/workspace-cache/services/workspace-cache.service'; import { LogicFunctionExecutorService } from 'src/engine/core-modules/logic-function/logic-function-executor/logic-function-executor.service'; @@ -45,14 +44,13 @@ export class ApplicationInstallService { @InjectRepository(ApplicationRegistrationEntity) private readonly appRegistrationRepository: Repository, private readonly applicationService: ApplicationService, - private readonly applicationRegistrationService: ApplicationRegistrationService, private readonly applicationPackageFetcherService: ApplicationPackageFetcherService, private readonly applicationVersionValidationService: ApplicationVersionValidationService, private readonly applicationSyncService: ApplicationSyncService, + private readonly applicationManifestApplyService: ApplicationManifestApplyService, private readonly fileStorageService: FileStorageService, private readonly logicFunctionExecutorService: LogicFunctionExecutorService, private readonly cacheLockService: CacheLockService, - private readonly sdkClientGenerationService: SdkClientGenerationService, @InjectMessageQueue(MessageQueue.logicFunctionQueue) private readonly messageQueueService: MessageQueueService, private readonly workspaceCacheService: WorkspaceCacheService, @@ -238,20 +236,12 @@ export class ApplicationInstallService { universalIdentifier, }); - const { hasSchemaMetadataChanged } = - await this.applicationSyncService.synchronizeFromManifest({ - workspaceId: params.workspaceId, - manifest: resolvedPackage.manifest, - applicationRegistrationId: appRegistration.id, - }); - - if (!isVersionUpgrade || hasSchemaMetadataChanged) { - await this.sdkClientGenerationService.generateSdkClientForApplication({ - workspaceId: params.workspaceId, - applicationId: application.id, - applicationUniversalIdentifier: universalIdentifier, - }); - } + await this.applicationManifestApplyService.applyManifestToWorkspace({ + workspaceId: params.workspaceId, + manifest: resolvedPackage.manifest, + applicationRegistrationId: appRegistration.id, + application, + }); await this.runPostInstallHook({ manifest: resolvedPackage.manifest, @@ -262,11 +252,14 @@ export class ApplicationInstallService { universalIdentifier, }); - await this.refreshRegistrationFromInstall({ - appRegistration, - manifest: resolvedPackage.manifest, - installedVersion: newVersion, - }); + await this.applicationManifestApplyService.refreshRegistrationFromManifest( + { + applicationRegistrationId: appRegistration.id, + manifest: resolvedPackage.manifest, + latestAvailableVersion: newVersion, + preventVersionDowngrade: true, + }, + ); this.logger.log( `Successfully installed app ${universalIdentifier} v${resolvedPackage.packageJson.version ?? 'unknown'}`, @@ -295,21 +288,6 @@ export class ApplicationInstallService { } } - private async refreshRegistrationFromInstall(params: { - appRegistration: ApplicationRegistrationEntity; - manifest: Manifest; - installedVersion: string; - }): Promise { - const { appRegistration, manifest, installedVersion } = params; - - await this.applicationRegistrationService.updateFromManifest({ - applicationRegistrationId: appRegistration.id, - manifest, - latestAvailableVersion: installedVersion, - preventVersionDowngrade: true, - }); - } - private async runPreInstallHook(params: { manifest: Manifest; workspaceId: string; diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/__tests__/application-manifest-apply.service.spec.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/__tests__/application-manifest-apply.service.spec.ts new file mode 100644 index 0000000000..9b7143ad9b --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/__tests__/application-manifest-apply.service.spec.ts @@ -0,0 +1,226 @@ +import { Test, type TestingModule } from '@nestjs/testing'; + +import { type Manifest } from 'twenty-shared/application'; + +import { ApplicationManifestApplyService } from 'src/engine/core-modules/application/application-manifest/application-manifest-apply.service'; +import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; +import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service'; +import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum'; +import { type ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; +import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service'; + +const WORKSPACE_ID = '20202020-0000-4000-8000-000000000001'; +const REGISTRATION_ID = '20202020-0000-4000-8000-000000000002'; +const APPLICATION_ID = '20202020-0000-4000-8000-000000000003'; + +const buildManifest = (overrides: Record = {}): Manifest => + ({ + application: { + universalIdentifier: 'my-app', + displayName: 'My App', + ...overrides, + }, + }) as Manifest; + +const buildApplication = (version: string | null): ApplicationEntity => + ({ + id: APPLICATION_ID, + universalIdentifier: 'my-app', + version, + }) as ApplicationEntity; + +describe('ApplicationManifestApplyService', () => { + let service: ApplicationManifestApplyService; + + const mockApplicationSyncService = { + synchronizeFromManifest: jest.fn(), + }; + + const mockSdkClientGenerationService = { + generateSdkClientForApplication: jest.fn(), + }; + + const mockApplicationRegistrationService = { + findOneByIdGlobal: jest.fn(), + updateFromManifest: jest.fn(), + }; + + beforeEach(async () => { + jest.clearAllMocks(); + + mockApplicationSyncService.synchronizeFromManifest.mockResolvedValue({ + workspaceMigration: { actions: [] }, + hasSchemaMetadataChanged: false, + }); + mockApplicationRegistrationService.updateFromManifest.mockResolvedValue( + true, + ); + + const module: TestingModule = await Test.createTestingModule({ + providers: [ + ApplicationManifestApplyService, + { + provide: ApplicationSyncService, + useValue: mockApplicationSyncService, + }, + { + provide: SdkClientGenerationService, + useValue: mockSdkClientGenerationService, + }, + { + provide: ApplicationRegistrationService, + useValue: mockApplicationRegistrationService, + }, + ], + }).compile(); + + service = module.get(ApplicationManifestApplyService); + }); + + describe('applyManifestToWorkspace', () => { + it('should generate the SDK client when the application has no version yet', async () => { + await service.applyManifestToWorkspace({ + workspaceId: WORKSPACE_ID, + manifest: buildManifest(), + applicationRegistrationId: REGISTRATION_ID, + application: buildApplication(null), + }); + + expect( + mockSdkClientGenerationService.generateSdkClientForApplication, + ).toHaveBeenCalledWith({ + workspaceId: WORKSPACE_ID, + applicationId: APPLICATION_ID, + applicationUniversalIdentifier: 'my-app', + }); + }); + + it('should generate the SDK client when the schema changed', async () => { + mockApplicationSyncService.synchronizeFromManifest.mockResolvedValue({ + workspaceMigration: { actions: [] }, + hasSchemaMetadataChanged: true, + }); + + await service.applyManifestToWorkspace({ + workspaceId: WORKSPACE_ID, + manifest: buildManifest(), + applicationRegistrationId: REGISTRATION_ID, + application: buildApplication('1.0.0'), + }); + + expect( + mockSdkClientGenerationService.generateSdkClientForApplication, + ).toHaveBeenCalled(); + }); + + it('should not generate the SDK client on an unchanged re-apply', async () => { + await service.applyManifestToWorkspace({ + workspaceId: WORKSPACE_ID, + manifest: buildManifest(), + applicationRegistrationId: REGISTRATION_ID, + application: buildApplication('1.0.0'), + }); + + expect( + mockSdkClientGenerationService.generateSdkClientForApplication, + ).not.toHaveBeenCalled(); + }); + }); + + describe('refreshRegistrationFromManifest', () => { + it('should update the registration', async () => { + const manifest = buildManifest(); + + const result = await service.refreshRegistrationFromManifest({ + applicationRegistrationId: REGISTRATION_ID, + manifest, + latestAvailableVersion: '1.2.0', + preventVersionDowngrade: true, + }); + + expect(result).toBe(true); + expect( + mockApplicationRegistrationService.updateFromManifest, + ).toHaveBeenCalledWith({ + applicationRegistrationId: REGISTRATION_ID, + manifest, + sourceType: undefined, + latestAvailableVersion: '1.2.0', + preventVersionDowngrade: true, + }); + }); + + it('should report a registration update skipped as a downgrade', async () => { + mockApplicationRegistrationService.updateFromManifest.mockResolvedValue( + false, + ); + + const result = await service.refreshRegistrationFromManifest({ + applicationRegistrationId: REGISTRATION_ID, + manifest: buildManifest(), + latestAvailableVersion: '0.9.0', + preventVersionDowngrade: true, + }); + + expect(result).toBe(false); + }); + + it('should skip npm-sourced registrations when scoped to a workspace', async () => { + mockApplicationRegistrationService.findOneByIdGlobal.mockResolvedValue({ + id: REGISTRATION_ID, + sourceType: ApplicationRegistrationSourceType.NPM, + ownerWorkspaceId: WORKSPACE_ID, + }); + + const result = await service.refreshRegistrationFromManifest({ + applicationRegistrationId: REGISTRATION_ID, + manifest: buildManifest(), + onlyIfOwnedByWorkspaceId: WORKSPACE_ID, + }); + + expect(result).toBe(false); + expect( + mockApplicationRegistrationService.updateFromManifest, + ).not.toHaveBeenCalled(); + }); + + it('should skip registrations owned by another workspace when scoped to a workspace', async () => { + mockApplicationRegistrationService.findOneByIdGlobal.mockResolvedValue({ + id: REGISTRATION_ID, + sourceType: ApplicationRegistrationSourceType.LOCAL, + ownerWorkspaceId: 'another-workspace-id', + }); + + const result = await service.refreshRegistrationFromManifest({ + applicationRegistrationId: REGISTRATION_ID, + manifest: buildManifest(), + onlyIfOwnedByWorkspaceId: WORKSPACE_ID, + }); + + expect(result).toBe(false); + expect( + mockApplicationRegistrationService.updateFromManifest, + ).not.toHaveBeenCalled(); + }); + + it('should update a workspace-owned registration when scoped to that workspace', async () => { + mockApplicationRegistrationService.findOneByIdGlobal.mockResolvedValue({ + id: REGISTRATION_ID, + sourceType: ApplicationRegistrationSourceType.LOCAL, + ownerWorkspaceId: WORKSPACE_ID, + }); + + const result = await service.refreshRegistrationFromManifest({ + applicationRegistrationId: REGISTRATION_ID, + manifest: buildManifest(), + sourceType: ApplicationRegistrationSourceType.LOCAL, + onlyIfOwnedByWorkspaceId: WORKSPACE_ID, + }); + + expect(result).toBe(true); + expect( + mockApplicationRegistrationService.updateFromManifest, + ).toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest-apply.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest-apply.service.ts new file mode 100644 index 0000000000..de56d4814d --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest-apply.service.ts @@ -0,0 +1,102 @@ +import { Injectable } from '@nestjs/common'; + +import { type Manifest } from 'twenty-shared/application'; +import { isDefined } from 'twenty-shared/utils'; + +import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; +import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service'; +import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum'; +import { type ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; +import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service'; +import { type WorkspaceMigration } from 'src/engine/workspace-manager/workspace-migration/workspace-migration-builder/types/workspace-migration.type'; + +// Shared by marketplace/tarball installs and CLI dev sync so every source +// applies a manifest through the same steps with the same guards. +@Injectable() +export class ApplicationManifestApplyService { + constructor( + private readonly applicationSyncService: ApplicationSyncService, + private readonly sdkClientGenerationService: SdkClientGenerationService, + private readonly applicationRegistrationService: ApplicationRegistrationService, + ) {} + + async applyManifestToWorkspace({ + workspaceId, + manifest, + applicationRegistrationId, + application, + }: { + workspaceId: string; + manifest: Manifest; + applicationRegistrationId?: string; + application: ApplicationEntity; + }): Promise<{ + workspaceMigration: WorkspaceMigration; + hasSchemaMetadataChanged: boolean; + }> { + // The application version is only persisted by a successful sync: no + // version means no sync ever completed, so the SDK client must be + // generated regardless of schema changes. + const isFirstApply = !isDefined(application.version); + + const { workspaceMigration, hasSchemaMetadataChanged } = + await this.applicationSyncService.synchronizeFromManifest({ + workspaceId, + manifest, + applicationRegistrationId, + }); + + if (isFirstApply || hasSchemaMetadataChanged) { + await this.sdkClientGenerationService.generateSdkClientForApplication({ + workspaceId, + applicationId: application.id, + applicationUniversalIdentifier: application.universalIdentifier, + }); + } + + return { workspaceMigration, hasSchemaMetadataChanged }; + } + + async refreshRegistrationFromManifest({ + applicationRegistrationId, + manifest, + sourceType, + latestAvailableVersion, + preventVersionDowngrade, + onlyIfOwnedByWorkspaceId, + }: { + applicationRegistrationId: string; + manifest: Manifest; + sourceType?: ApplicationRegistrationSourceType; + latestAvailableVersion?: string; + preventVersionDowngrade?: boolean; + // The registration is instance-global: for catalog-synced (npm) apps it + // is the marketplace entry and OAuth identity shared by every workspace. + // Callers acting on behalf of a workspace (dev sync) pass their + // workspaceId so only registrations owned by that workspace (and not + // npm-sourced) are written. + onlyIfOwnedByWorkspaceId?: string; + }): Promise { + if (isDefined(onlyIfOwnedByWorkspaceId)) { + const registration = + await this.applicationRegistrationService.findOneByIdGlobal( + applicationRegistrationId, + ); + + if ( + registration.sourceType === ApplicationRegistrationSourceType.NPM || + registration.ownerWorkspaceId !== onlyIfOwnedByWorkspaceId + ) { + return false; + } + } + + return this.applicationRegistrationService.updateFromManifest({ + applicationRegistrationId, + manifest, + sourceType, + latestAvailableVersion, + preventVersionDowngrade, + }); + } +} diff --git a/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest.module.ts b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest.module.ts index d0919dc6d0..715a79799f 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest.module.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-manifest/application-manifest.module.ts @@ -1,14 +1,17 @@ import { Module } from '@nestjs/common'; import { ApplicationModule } from 'src/engine/core-modules/application/application.module'; +import { ApplicationManifestApplyService } from 'src/engine/core-modules/application/application-manifest/application-manifest-apply.service'; import { ApplicationManifestMigrationService } from 'src/engine/core-modules/application/application-manifest/application-manifest-migration.service'; import { ComputeApplicationManifestAllUniversalFlatEntityMapsService } from 'src/engine/core-modules/application/application-manifest/services/compute-application-manifest-all-universal-flat-entity-maps.service'; import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; +import { ApplicationRegistrationModule } from 'src/engine/core-modules/application/application-registration/application-registration.module'; import { ApplicationTranslationModule } from 'src/engine/core-modules/application/application-translation/application-translation.module'; import { ApplicationVariableEntityModule } from 'src/engine/core-modules/application/application-variable/application-variable.module'; import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module'; import { FileStorageModule } from 'src/engine/core-modules/file-storage/file-storage.module'; import { SecretEncryptionModule } from 'src/engine/core-modules/secret-encryption/secret-encryption.module'; +import { SdkClientModule } from 'src/engine/core-modules/sdk-client/sdk-client.module'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache.module'; import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace-migration/workspace-migration.module'; @@ -16,20 +19,27 @@ import { WorkspaceMigrationModule } from 'src/engine/workspace-manager/workspace @Module({ imports: [ ApplicationModule, + ApplicationRegistrationModule, ApplicationTranslationModule, ApplicationVariableEntityModule, FeatureFlagModule, FileStorageModule, PermissionsModule, SecretEncryptionModule, + SdkClientModule, WorkspaceCacheModule, WorkspaceMigrationModule, ], providers: [ + ApplicationManifestApplyService, ApplicationManifestMigrationService, ApplicationSyncService, ComputeApplicationManifestAllUniversalFlatEntityMapsService, ], - exports: [ApplicationManifestMigrationService, ApplicationSyncService], + exports: [ + ApplicationManifestApplyService, + ApplicationManifestMigrationService, + ApplicationSyncService, + ], }) export class ApplicationManifestModule {} diff --git a/packages/twenty-server/src/engine/core-modules/application/application-registration-variable/application-registration-variable.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-registration-variable/application-registration-variable.service.ts index c2cc87afc7..3a3b68027a 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-registration-variable/application-registration-variable.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-registration-variable/application-registration-variable.service.ts @@ -4,7 +4,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import { type ServerVariables } from 'twenty-shared/application'; import { FieldMetadataType } from 'twenty-shared/types'; import { isDefined } from 'twenty-shared/utils'; -import { In, Not, type Repository } from 'typeorm'; +import { In, Not, type EntityManager, type Repository } from 'typeorm'; import { ApplicationRegistrationVariableEntity } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.entity'; import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity'; @@ -118,10 +118,15 @@ export class ApplicationRegistrationVariableService { async syncVariableSchemas( applicationRegistrationId: string, serverVariables: ServerVariables, + entityManager?: EntityManager, ): Promise { + const variableRepository = isDefined(entityManager) + ? entityManager.getRepository(ApplicationRegistrationVariableEntity) + : this.variableRepository; + const declaredKeys = Object.keys(serverVariables); - const existingVariables = await this.variableRepository.find({ + const existingVariables = await variableRepository.find({ where: { applicationRegistrationId }, }); @@ -133,7 +138,7 @@ export class ApplicationRegistrationVariableService { const existing = existingByKey.get(key); if (existing) { - await this.variableRepository.update(existing.id, { + await variableRepository.update(existing.id, { description: schema.description ?? '', isSecret: schema.isSecret ?? true, isRequired: schema.isRequired ?? false, @@ -141,8 +146,8 @@ export class ApplicationRegistrationVariableService { options: schema.options ?? null, }); } else { - await this.variableRepository.save( - this.variableRepository.create({ + await variableRepository.save( + variableRepository.create({ applicationRegistrationId, key, encryptedValue: '', @@ -157,12 +162,12 @@ export class ApplicationRegistrationVariableService { } if (declaredKeys.length > 0) { - await this.variableRepository.delete({ + await variableRepository.delete({ applicationRegistrationId, key: Not(In(declaredKeys)), }); } else { - await this.variableRepository.delete({ applicationRegistrationId }); + await variableRepository.delete({ applicationRegistrationId }); } } diff --git a/packages/twenty-server/src/engine/core-modules/application/application-registration/application-registration.service.ts b/packages/twenty-server/src/engine/core-modules/application/application-registration/application-registration.service.ts index 2005feb9ab..9a82b5ce30 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-registration/application-registration.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-registration/application-registration.service.ts @@ -43,6 +43,14 @@ const BCRYPT_SALT_ROUNDS = 10; const MAX_APPLICATION_REGISTRATIONS_PAGE_SIZE = 100; +// Sized well above the manifest save + variable schema sync duration so the +// lease cannot expire mid-update and let a concurrent refresh interleave. +const APPLICATION_REGISTRATION_UPDATE_LOCK_OPTIONS = { + ttl: 60_000, + ms: 500, + maxRetries: 120, +}; + const APPLICATION_REGISTRATION_WITHOUT_MANIFEST_SELECT: (keyof ApplicationRegistrationEntity)[] = [ 'id', @@ -383,58 +391,82 @@ export class ApplicationRegistrationService { sourceType?: ApplicationRegistrationSourceType; latestAvailableVersion?: string; preventVersionDowngrade?: boolean; - }): Promise { - await this.cacheLockService.withLock(async () => { - const existing = - await this.applicationRegistrationRepository.findOneOrFail({ - where: { id: applicationRegistrationId }, - }); + }): Promise { + return this.cacheLockService.withLock( + async () => { + const existing = + await this.applicationRegistrationRepository.findOneOrFail({ + where: { id: applicationRegistrationId }, + }); - if ( - preventVersionDowngrade && - isDefined(latestAvailableVersion) && - !shouldRefreshApplicationRegistrationOnInstall({ - installedVersion: latestAvailableVersion, - latestAvailableVersion: existing.latestAvailableVersion, - }) - ) { - this.logger.log( - `Skipping registration update for ${existing.universalIdentifier}: version ${latestAvailableVersion} is older than latest available version ${existing.latestAvailableVersion}`, + if ( + preventVersionDowngrade && + isDefined(latestAvailableVersion) && + !shouldRefreshApplicationRegistrationOnInstall({ + installedVersion: latestAvailableVersion, + latestAvailableVersion: existing.latestAvailableVersion, + }) + ) { + this.logger.log( + `Skipping registration update for ${existing.universalIdentifier}: version ${latestAvailableVersion} is older than latest available version ${existing.latestAvailableVersion}`, + ); + + return false; + } + + const displayFields = fromManifestApplicationToDisplayFields( + manifest.application, ); - return; - } + // Gallery image files are stored by the source-specific flows (tarball + // upload, dev sync); keep their fileIds for paths that did not change. + const existingFileIdByPath = new Map( + (existing.galleryImages ?? []).map(({ path, fileId }) => [ + path, + fileId, + ]), + ); - const displayFields = fromManifestApplicationToDisplayFields( - manifest.application, - ); + // One transaction so the registration row and its variable schemas + // always come from the same manifest, even when the sync fails midway. + await this.applicationRegistrationRepository.manager.transaction( + async (entityManager) => { + await entityManager + .getRepository(ApplicationRegistrationEntity) + .save({ + ...existing, + name: manifest.application.displayName, + manifest, + ...displayFields, + galleryImages: displayFields.galleryImages.map( + (galleryImage) => ({ + ...galleryImage, + fileId: existingFileIdByPath.get(galleryImage.path) ?? null, + }), + ), + ...(sourceType !== undefined && { sourceType }), + ...(latestAvailableVersion !== undefined && { + latestAvailableVersion, + }), + }); - // Gallery image files are stored by the source-specific flows (tarball - // upload, dev sync); keep their fileIds for paths that did not change. - const existingFileIdByPath = new Map( - (existing.galleryImages ?? []).map(({ path, fileId }) => [ - path, - fileId, - ]), - ); + if (isDefined(manifest.application.serverVariables)) { + await this.applicationRegistrationVariableService.syncVariableSchemas( + applicationRegistrationId, + manifest.application.serverVariables, + entityManager, + ); + } + }, + ); - await this.applicationRegistrationRepository.save({ - ...existing, - name: manifest.application.displayName, - manifest, - ...displayFields, - galleryImages: displayFields.galleryImages.map((galleryImage) => ({ - ...galleryImage, - fileId: existingFileIdByPath.get(galleryImage.path) ?? null, - })), - ...(sourceType !== undefined && { sourceType }), - ...(latestAvailableVersion !== undefined && { - latestAvailableVersion, - }), - }); + await this.invalidateMarketplaceAppsCache(); - await this.invalidateMarketplaceAppsCache(); - }, `application-registration-update:${applicationRegistrationId}`); + return true; + }, + `application-registration-update:${applicationRegistrationId}`, + APPLICATION_REGISTRATION_UPDATE_LOCK_OPTIONS, + ); } async delete(id: string, ownerWorkspaceId: string): Promise {