From 1c8b8970fdb7b0bdc8c8552f2a5f43d73da67df1 Mon Sep 17 00:00:00 2001 From: martmull Date: Fri, 10 Jul 2026 12:10:00 +0200 Subject: [PATCH] Allow CLI dev mode on catalog-synced apps without mutating the shared registration (#22756) --- .../twenty-sdk/src/cli/operations/dev-once.ts | 9 +- .../src/cli/utilities/api/api-service.ts | 6 + .../src/cli/utilities/api/application-api.ts | 55 +++ .../auth/build-app-token-pair-fetcher.ts | 22 ++ ...re-app-access-token-is-valid-or-refresh.ts | 36 +- .../utilities/auth/ensure-app-registration.ts | 16 +- .../dev/orchestrator/dev-mode-orchestrator.ts | 22 +- .../generate-api-client-orchestrator-step.ts | 5 +- .../steps/register-app-orchestrator-step.ts | 2 +- packages/twenty-sdk/vitest.e2e.config.ts | 8 +- .../application-development.module.ts | 2 + .../application-development.resolver.ts | 275 +------------- .../application-development.service.ts | 340 ++++++++++++++++++ ...og-synced-app-dev-mode.integration-spec.ts | 274 ++++++++++++++ 14 files changed, 780 insertions(+), 292 deletions(-) create mode 100644 packages/twenty-sdk/src/cli/utilities/auth/build-app-token-pair-fetcher.ts create mode 100644 packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts create mode 100644 packages/twenty-server/test/integration/metadata/suites/application/catalog-synced-app-dev-mode.integration-spec.ts diff --git a/packages/twenty-sdk/src/cli/operations/dev-once.ts b/packages/twenty-sdk/src/cli/operations/dev-once.ts index c37032196a..3ca7358a77 100644 --- a/packages/twenty-sdk/src/cli/operations/dev-once.ts +++ b/packages/twenty-sdk/src/cli/operations/dev-once.ts @@ -8,6 +8,7 @@ import { ensureAppAccessTokenIsValidOrRefresh, ensureAppRegistration, } from '@/cli/utilities/auth'; +import { buildAppTokenPairFetcher } from '@/cli/utilities/auth/build-app-token-pair-fetcher'; import { promptForReauthentication } from '@/cli/utilities/auth/reauth-helper'; import { buildApplication } from '@/cli/utilities/build/common/build-application'; import { runTypecheck } from '@/cli/utilities/build/common/typecheck-plugin'; @@ -354,7 +355,13 @@ const innerAppDevOnce = async ( try { const appAccessToken = await ensureAppAccessTokenIsValidOrRefresh( configService, - { clientId, clientSecret }, + { + credentials: clientSecret ? { clientId, clientSecret } : undefined, + fetchTokenPair: buildAppTokenPairFetcher( + apiService, + createDevAppResult.data.id, + ), + }, ); const clientService = new ClientService(); diff --git a/packages/twenty-sdk/src/cli/utilities/api/api-service.ts b/packages/twenty-sdk/src/cli/utilities/api/api-service.ts index e17075d03c..63c5861ceb 100644 --- a/packages/twenty-sdk/src/cli/utilities/api/api-service.ts +++ b/packages/twenty-sdk/src/cli/utilities/api/api-service.ts @@ -76,6 +76,12 @@ export class ApiService { return this.applicationApi.createDevelopmentApplication(...args); } + generateApplicationToken( + ...args: Parameters + ) { + return this.applicationApi.generateApplicationToken(...args); + } + syncApplication( manifest: Manifest, options?: { dryRun?: boolean }, diff --git a/packages/twenty-sdk/src/cli/utilities/api/application-api.ts b/packages/twenty-sdk/src/cli/utilities/api/application-api.ts index 21b2bac81d..ea9203f985 100644 --- a/packages/twenty-sdk/src/cli/utilities/api/application-api.ts +++ b/packages/twenty-sdk/src/cli/utilities/api/application-api.ts @@ -209,6 +209,61 @@ export class ApplicationApi { } } + async generateApplicationToken(applicationId: string): Promise< + ApiResponse<{ + applicationAccessToken: { token: string; expiresAt: string }; + applicationRefreshToken: { token: string; expiresAt: string }; + }> + > { + try { + const mutation = ` + mutation GenerateApplicationToken($applicationId: UUID!) { + generateApplicationToken(applicationId: $applicationId) { + applicationAccessToken { + token + expiresAt + } + applicationRefreshToken { + token + expiresAt + } + } + } + `; + + const response = await this.client.post( + '/metadata', + { + query: mutation, + variables: { applicationId }, + }, + { + headers: { + 'Content-Type': 'application/json', + Accept: '*/*', + }, + }, + ); + + if (response.data.errors) { + return { + success: false, + error: response.data.errors[0], + }; + } + + return { + success: true, + data: response.data.data.generateApplicationToken, + }; + } catch (error) { + return { + success: false, + error, + }; + } + } + async createDevelopmentApplication(input: { universalIdentifier: string; name: string; diff --git a/packages/twenty-sdk/src/cli/utilities/auth/build-app-token-pair-fetcher.ts b/packages/twenty-sdk/src/cli/utilities/auth/build-app-token-pair-fetcher.ts new file mode 100644 index 0000000000..90f62b2ad5 --- /dev/null +++ b/packages/twenty-sdk/src/cli/utilities/auth/build-app-token-pair-fetcher.ts @@ -0,0 +1,22 @@ +import { type ApiService } from '@/cli/utilities/api/api-service'; + +// Builds the fetchTokenPair token source for +// ensureAppAccessTokenIsValidOrRefresh: mints a workspace-scoped app token +// pair via the generateApplicationToken mutation. +export const buildAppTokenPairFetcher = + (apiService: ApiService, applicationId: string) => + async (): Promise< + { accessToken: string; refreshToken?: string } | undefined + > => { + const tokenResult = + await apiService.generateApplicationToken(applicationId); + + if (!tokenResult.success || !tokenResult.data) { + return undefined; + } + + return { + accessToken: tokenResult.data.applicationAccessToken.token, + refreshToken: tokenResult.data.applicationRefreshToken.token, + }; + }; diff --git a/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh.ts b/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh.ts index ece33a9a8c..5e15247b2f 100644 --- a/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh.ts +++ b/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh.ts @@ -17,9 +17,20 @@ const isTokenExpired = (token: string): boolean => { } }; +export type AppTokenSources = { + credentials?: { clientId: string; clientSecret: string }; + // Workspace-scoped token minting (generateApplicationToken mutation). Used + // when the registration is not owned by this workspace (e.g. an app synced + // from the marketplace catalog), where no client secret is available and + // rotating the shared one would break the published app. + fetchTokenPair?: () => Promise< + { accessToken: string; refreshToken?: string } | undefined + >; +}; + export const ensureAppAccessTokenIsValidOrRefresh = async ( configService: ConfigService, - credentials?: { clientId: string; clientSecret: string }, + tokenSources?: AppTokenSources, ): Promise => { const config = await configService.getConfig(); @@ -62,18 +73,31 @@ export const ensureAppAccessTokenIsValidOrRefresh = async ( appAccessToken: undefined, appRefreshToken: undefined, }); - - return undefined; } } catch { - // Non-JSON error response (e.g. proxy 502) — fall through to credential exchange + // Non-JSON error response (e.g. proxy 502) — fall through to the other token sources } } - if (credentials) { + if (tokenSources?.fetchTokenPair) { + const tokenPair = await tokenSources.fetchTokenPair(); + + if (tokenPair) { + await configService.setConfig({ + appAccessToken: tokenPair.accessToken, + ...(tokenPair.refreshToken + ? { appRefreshToken: tokenPair.refreshToken } + : {}), + }); + + return tokenPair.accessToken; + } + } + + if (tokenSources?.credentials) { const result = await exchangeCredentialsForTokens( configService, - credentials, + tokenSources.credentials, ); return result.accessToken; diff --git a/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-registration.ts b/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-registration.ts index 1ee53ef797..e2d57eb35c 100644 --- a/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-registration.ts +++ b/packages/twenty-sdk/src/cli/utilities/auth/ensure-app-registration.ts @@ -8,7 +8,7 @@ export const ensureAppRegistration = async ( app: { name: string; universalIdentifier: string }, ): Promise<{ clientId: string; - clientSecret: string; + clientSecret?: string; isNewRegistration: boolean; }> => { const createResult = await apiService.createApplicationRegistration({ @@ -68,18 +68,12 @@ export const ensureAppRegistration = async ( appRefreshToken: undefined, }); - const rotateResult = - await apiService.rotateApplicationRegistrationClientSecret(registration.id); - - if (!rotateResult.success || !rotateResult.data) { - throw new Error( - `Failed to rotate client secret for registration ${registration.id}`, - ); - } - + // The registration may be a catalog-synced npm app owned by another (or no) + // workspace, so rotating its shared client secret is neither allowed nor + // desirable. Dev mode mints workspace-scoped app tokens via the + // generateApplicationToken mutation instead. return { clientId: registration.oAuthClientId, - clientSecret: rotateResult.data.clientSecret, isNewRegistration: false, }; }; diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/dev-mode-orchestrator.ts b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/dev-mode-orchestrator.ts index 5ff8f048a7..1725d52dc6 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/dev-mode-orchestrator.ts +++ b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/dev-mode-orchestrator.ts @@ -1,4 +1,6 @@ import { ApiService } from '@/cli/utilities/api/api-service'; +import { buildAppTokenPairFetcher } from '@/cli/utilities/auth/build-app-token-pair-fetcher'; +import { type AppTokenSources } from '@/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh'; import { ClientService } from '@/cli/utilities/client/client-service'; import { ConfigService } from '@/cli/utilities/config/config-service'; import { type OrchestratorState } from '@/cli/utilities/dev/orchestrator/dev-mode-orchestrator-state'; @@ -227,13 +229,31 @@ export class DevModeOrchestrator { if (objectsOrFieldsChanged) { await this.generateApiClientStep.execute({ appPath: this.state.appPath, - credentials: this.registerAppStep.registrationCredentials, + tokenSources: this.buildAppTokenSources(), }); this.skipTypecheck = false; } } + private buildAppTokenSources(): AppTokenSources { + const credentials = this.registerAppStep.registrationCredentials; + const applicationId = + this.state.steps.resolveApplication.output.applicationId; + + return { + credentials: credentials?.clientSecret + ? { + clientId: credentials.clientId, + clientSecret: credentials.clientSecret, + } + : undefined, + fetchTokenPair: applicationId + ? buildAppTokenPairFetcher(this.apiService, applicationId) + : undefined, + }; + } + private async initializePipeline(manifest: Manifest): Promise { await this.registerAppStep.execute({ manifest }); diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/generate-api-client-orchestrator-step.ts b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/generate-api-client-orchestrator-step.ts index e08e0796af..95627f83ce 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/generate-api-client-orchestrator-step.ts +++ b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/generate-api-client-orchestrator-step.ts @@ -1,4 +1,5 @@ import { ensureAppAccessTokenIsValidOrRefresh } from '@/cli/utilities/auth'; +import { type AppTokenSources } from '@/cli/utilities/auth/ensure-app-access-token-is-valid-or-refresh'; import { type ClientService } from '@/cli/utilities/client/client-service'; import { type ConfigService } from '@/cli/utilities/config/config-service'; import { type OrchestratorState } from '@/cli/utilities/dev/orchestrator/dev-mode-orchestrator-state'; @@ -28,7 +29,7 @@ export class GenerateApiClientOrchestratorStep { async execute(input: { appPath: string; - credentials?: { clientId: string; clientSecret: string }; + tokenSources?: AppTokenSources; }): Promise { const step = this.state.steps.generateApiClient; @@ -38,7 +39,7 @@ export class GenerateApiClientOrchestratorStep { try { const appAccessToken = await ensureAppAccessTokenIsValidOrRefresh( this.configService, - input.credentials, + input.tokenSources, ); await this.clientService.generateCoreClient({ diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/register-app-orchestrator-step.ts b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/register-app-orchestrator-step.ts index a17e39e2c1..21a997949b 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/register-app-orchestrator-step.ts +++ b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/register-app-orchestrator-step.ts @@ -11,7 +11,7 @@ export class RegisterAppOrchestratorStep { private notify: () => void; registrationCredentials: - | { clientId: string; clientSecret: string } + | { clientId: string; clientSecret?: string } | undefined; constructor({ diff --git a/packages/twenty-sdk/vitest.e2e.config.ts b/packages/twenty-sdk/vitest.e2e.config.ts index dd5e7a98a5..a6baac2e38 100644 --- a/packages/twenty-sdk/vitest.e2e.config.ts +++ b/packages/twenty-sdk/vitest.e2e.config.ts @@ -12,11 +12,9 @@ export default defineConfig({ testTimeout: 60_000, hookTimeout: 60_000, pool: 'forks', - poolOptions: { - forks: { - singleFork: true, - }, - }, + // poolOptions.forks.singleFork was removed in Vitest 4; without serial + // file execution the e2e forks race on the shared ~/.twenty config file. + fileParallelism: false, sequence: { concurrent: false, }, 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 b562ac09df..cfc554f579 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 @@ -5,6 +5,7 @@ import { ApplicationManifestModule } from 'src/engine/core-modules/application/a import { ApplicationModule } from 'src/engine/core-modules/application/application.module'; import { ApplicationPackageModule } from 'src/engine/core-modules/application/application-package/application-package.module'; import { ApplicationDevelopmentResolver } from 'src/engine/core-modules/application/application-development/application-development.resolver'; +import { ApplicationDevelopmentService } from 'src/engine/core-modules/application/application-development/application-development.service'; import { CacheLockModule } from 'src/engine/core-modules/cache-lock/cache-lock.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'; @@ -28,6 +29,7 @@ import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/wor ], providers: [ ApplicationDevelopmentResolver, + ApplicationDevelopmentService, WorkspaceMigrationGraphqlApiExceptionInterceptor, ], }) diff --git a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts index 2aef49686a..be5c41bf26 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.resolver.ts @@ -8,38 +8,19 @@ import { Args, Mutation } from '@nestjs/graphql'; import GraphQLUpload from 'graphql-upload/GraphQLUpload.mjs'; import { PermissionFlagType } from 'twenty-shared/constants'; -import { FileFolder } from 'twenty-shared/types'; -import { isDefined } from 'twenty-shared/utils'; import type { FileUpload } from 'graphql-upload/processRequest.mjs'; import { MetadataResolver } from 'src/engine/api/graphql/graphql-config/decorators/metadata-resolver.decorator'; +import { ApplicationDevelopmentService } from 'src/engine/core-modules/application/application-development/application-development.service'; import { ApplicationInput } from 'src/engine/core-modules/application/application-development/dtos/application.input'; import { CreateDevelopmentApplicationInput } from 'src/engine/core-modules/application/application-development/dtos/create-development-application.input'; import { DevelopmentApplicationDTO } from 'src/engine/core-modules/application/application-development/dtos/development-application.dto'; import { UploadApplicationFileInput } from 'src/engine/core-modules/application/application-development/dtos/upload-application-file.input'; import { WorkspaceMigrationDTO } from 'src/engine/core-modules/application/application-development/dtos/workspace-migration.dto'; import { ApplicationExceptionFilter } from 'src/engine/core-modules/application/application-exception-filter'; -import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; -import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util'; -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 { 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 { - ApplicationException, - ApplicationExceptionCode, -} from 'src/engine/core-modules/application/application.exception'; -import { ApplicationService } from 'src/engine/core-modules/application/application.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'; -import { validateFilePath } from 'src/engine/core-modules/file-storage/utils/validate-file-path.util'; import { FileDTO } from 'src/engine/core-modules/file/dtos/file.dto'; import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe'; -import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service'; -import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service'; -import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard'; @@ -47,11 +28,6 @@ import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard'; import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-graphql-api-exception.interceptor'; import { streamToBuffer } from 'src/utils/stream-to-buffer'; -const APP_DEV_RATE_LIMIT_MAX = 30; -const APP_DEV_RATE_LIMIT_WINDOW_MS = 30_000; - -const APP_SYNC_LOCK_OPTIONS = { ttl: 60_000, ms: 500, maxRetries: 120 }; - @UsePipes(ResolverValidationPipe) @MetadataResolver() @UseInterceptors(WorkspaceMigrationGraphqlApiExceptionInterceptor) @@ -62,16 +38,7 @@ const APP_SYNC_LOCK_OPTIONS = { ttl: 60_000, ms: 500, maxRetries: 120 }; ) export class ApplicationDevelopmentResolver { constructor( - private readonly applicationService: ApplicationService, - private readonly applicationSyncService: ApplicationSyncService, - private readonly applicationRegistrationService: ApplicationRegistrationService, - private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService, - private readonly applicationVersionValidationService: ApplicationVersionValidationService, - private readonly fileStorageService: FileStorageService, - private readonly sdkClientGenerationService: SdkClientGenerationService, - private readonly twentyConfigService: TwentyConfigService, - private readonly throttlerService: ThrottlerService, - private readonly cacheLockService: CacheLockService, + private readonly applicationDevelopmentService: ApplicationDevelopmentService, ) {} @Mutation(() => DevelopmentApplicationDTO) @@ -79,36 +46,11 @@ export class ApplicationDevelopmentResolver { @Args() { universalIdentifier, name }: CreateDevelopmentApplicationInput, @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - await this.throttlePerApplication(universalIdentifier, workspaceId); - - const applicationRegistrationId = - await this.findApplicationRegistrationId(universalIdentifier); - - const existing = await this.applicationService.findByUniversalIdentifier({ - universalIdentifier, - workspaceId, - }); - - if (existing) { - return { - id: existing.id, - universalIdentifier: existing.universalIdentifier, - }; - } - - const application = await this.applicationService.create({ + return this.applicationDevelopmentService.createDevelopmentApplication({ universalIdentifier, name, - sourcePath: universalIdentifier, - sourceType: ApplicationRegistrationSourceType.LOCAL, - applicationRegistrationId, workspaceId, }); - - return { - id: application.id, - universalIdentifier: application.universalIdentifier, - }; } @Mutation(() => WorkspaceMigrationDTO) @@ -116,101 +58,11 @@ export class ApplicationDevelopmentResolver { @Args() { manifest, dryRun }: ApplicationInput, @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - await this.throttlePerApplication( - manifest.application.universalIdentifier, - workspaceId, - ); - - const versionValidation = - await this.applicationVersionValidationService.validateWorkspaceCompatibility( - { - requiredServerVersion: - manifest.application.requiredServerVersionRange ?? undefined, - workspaceId, - }, - ); - - if (!versionValidation.compatible) { - throw new ApplicationException( - versionValidation.message, - VERSION_REASON_TO_APPLICATION_EXCEPTION_CODE[versionValidation.reason], - ); - } - - if (dryRun === true) { - const { workspaceMigration } = - await this.applicationSyncService.synchronizeFromManifest({ - workspaceId, - manifest, - dryRun: true, - }); - - return { - applicationUniversalIdentifier: - workspaceMigration.applicationUniversalIdentifier, - actions: workspaceMigration.actions, - }; - } - - return this.cacheLockService.withLock( - () => this.applyManifestSync(manifest, workspaceId), - `app-sync:${workspaceId}`, - APP_SYNC_LOCK_OPTIONS, - ); - } - - private async applyManifestSync( - manifest: ApplicationInput['manifest'], - workspaceId: string, - ): Promise { - const applicationRegistrationId = await this.findApplicationRegistrationId( - manifest.application.universalIdentifier, - ); - - const application = await this.applicationService.findByUniversalIdentifier( - { - universalIdentifier: manifest.application.universalIdentifier, - workspaceId, - }, - ); - - if (!isDefined(application)) { - throw new ApplicationException( - `Application "${manifest.application.universalIdentifier}" not found in workspace "${workspaceId}". Run createDevelopmentApplication first.`, - ApplicationExceptionCode.APPLICATION_NOT_FOUND, - ); - } - - const isFirstSync = !isDefined(application.version); - - const { workspaceMigration, hasSchemaMetadataChanged } = - await this.applicationSyncService.synchronizeFromManifest({ - workspaceId, - manifest, - applicationRegistrationId, - }); - - if (isFirstSync || hasSchemaMetadataChanged) { - await this.sdkClientGenerationService.generateSdkClientForApplication({ - workspaceId, - applicationId: application.id, - applicationUniversalIdentifier: - manifest.application.universalIdentifier, - }); - } - - await this.syncRegistrationMetadata( - applicationRegistrationId, + return this.applicationDevelopmentService.syncApplication({ manifest, + dryRun, workspaceId, - application.id, - ); - - return { - applicationUniversalIdentifier: - workspaceMigration.applicationUniversalIdentifier, - actions: workspaceMigration.actions, - }; + }); } @Mutation(() => FileDTO) @@ -226,119 +78,12 @@ export class ApplicationDevelopmentResolver { filePath, }: UploadApplicationFileInput, ): Promise { - await this.throttlePerApplication( - applicationUniversalIdentifier, + return this.applicationDevelopmentService.uploadApplicationFile({ workspaceId, - ); - - const allowedApplicationFileFolders: FileFolder[] = [ - FileFolder.BuiltLogicFunction, - FileFolder.BuiltFrontComponent, - FileFolder.PublicAsset, - FileFolder.Source, - FileFolder.Dependencies, - ]; - - if (!allowedApplicationFileFolders.includes(fileFolder)) { - throw new ApplicationException( - `Invalid fileFolder for application file upload. Allowed values: ${allowedApplicationFileFolders.join(', ')}`, - ApplicationExceptionCode.INVALID_INPUT, - ); - } - - const pathValidationResult = validateFilePath({ - resourcePath: filePath, - fileFolder, - }); - - if (!pathValidationResult.isValid) { - throw new ApplicationException( - pathValidationResult.error, - ApplicationExceptionCode.INVALID_INPUT, - ); - } - - const application = await this.applicationService.findByUniversalIdentifier( - { - universalIdentifier: applicationUniversalIdentifier, - workspaceId, - }, - ); - - if (!isDefined(application)) { - throw new ApplicationException( - 'Application not found in workspace.', - ApplicationExceptionCode.APPLICATION_NOT_FOUND, - ); - } - - const buffer = await streamToBuffer(createReadStream()); - - return await this.fileStorageService.writeFile({ - sourceFile: buffer, - fileFolder, applicationUniversalIdentifier, - workspaceId, - resourcePath: filePath, - settings: { isTemporaryFile: false, toDelete: false }, + fileFolder, + filePath, + getFileBuffer: () => streamToBuffer(createReadStream()), }); } - - private async throttlePerApplication( - applicationIdentifier: string, - workspaceId: string, - ): Promise { - await this.throttlerService.tokenBucketThrottleOrThrow( - `app-dev:${workspaceId}:${applicationIdentifier}`, - 1, - APP_DEV_RATE_LIMIT_MAX, - APP_DEV_RATE_LIMIT_WINDOW_MS, - ); - } - - private async findApplicationRegistrationId( - universalIdentifier: string, - ): Promise { - const existingRegistration = - await this.applicationRegistrationService.findOneByUniversalIdentifier( - universalIdentifier, - ); - - if (!existingRegistration) { - throw new ApplicationException( - `No registration found for "${universalIdentifier}". Create one first with createApplicationRegistration.`, - ApplicationExceptionCode.APPLICATION_NOT_FOUND, - ); - } - - return existingRegistration.id; - } - - private async syncRegistrationMetadata( - applicationRegistrationId: string, - manifest: ApplicationInput['manifest'], - workspaceId: string, - applicationId: string, - ): Promise { - const serverUrl = this.twentyConfigService.get('SERVER_URL'); - - const manifestWithResolvedUrls = resolveManifestAssetUrls( - manifest, - (filePath) => - `${serverUrl}/public-assets/${workspaceId}/${applicationId}/${filePath}`, - ); - - await this.applicationRegistrationService.updateFromManifest({ - applicationRegistrationId, - manifest: manifestWithResolvedUrls, - sourceType: ApplicationRegistrationSourceType.LOCAL, - }); - - if (manifest.application.serverVariables) { - await this.applicationRegistrationVariableService.syncVariableSchemas( - applicationRegistrationId, - manifest.application.serverVariables, - ); - } - } } 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 new file mode 100644 index 0000000000..677c11bbc3 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application/application-development/application-development.service.ts @@ -0,0 +1,340 @@ +import { Injectable } from '@nestjs/common'; + +import { FileFolder } from 'twenty-shared/types'; +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 { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service'; +import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util'; +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 { 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 { + ApplicationException, + ApplicationExceptionCode, +} from 'src/engine/core-modules/application/application.exception'; +import { ApplicationService } from 'src/engine/core-modules/application/application.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'; +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 { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; + +const APP_DEV_RATE_LIMIT_MAX = 30; +const APP_DEV_RATE_LIMIT_WINDOW_MS = 30_000; + +const APP_SYNC_LOCK_OPTIONS = { ttl: 60_000, ms: 500, maxRetries: 120 }; + +const ALLOWED_APPLICATION_FILE_FOLDERS: FileFolder[] = [ + FileFolder.BuiltLogicFunction, + FileFolder.BuiltFrontComponent, + FileFolder.PublicAsset, + FileFolder.Source, + FileFolder.Dependencies, +]; + +@Injectable() +export class ApplicationDevelopmentService { + constructor( + private readonly applicationService: ApplicationService, + private readonly applicationSyncService: ApplicationSyncService, + private readonly applicationRegistrationService: ApplicationRegistrationService, + private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService, + private readonly applicationVersionValidationService: ApplicationVersionValidationService, + private readonly fileStorageService: FileStorageService, + private readonly sdkClientGenerationService: SdkClientGenerationService, + private readonly twentyConfigService: TwentyConfigService, + private readonly throttlerService: ThrottlerService, + private readonly cacheLockService: CacheLockService, + ) {} + + async createDevelopmentApplication({ + universalIdentifier, + name, + workspaceId, + }: { + universalIdentifier: string; + name: string; + workspaceId: string; + }): Promise { + await this.throttlePerApplication(universalIdentifier, workspaceId); + + const applicationRegistrationId = + await this.findApplicationRegistrationId(universalIdentifier); + + const existing = await this.applicationService.findByUniversalIdentifier({ + universalIdentifier, + workspaceId, + }); + + if (existing) { + return { + id: existing.id, + universalIdentifier: existing.universalIdentifier, + }; + } + + const application = await this.applicationService.create({ + universalIdentifier, + name, + sourcePath: universalIdentifier, + sourceType: ApplicationRegistrationSourceType.LOCAL, + applicationRegistrationId, + workspaceId, + }); + + return { + id: application.id, + universalIdentifier: application.universalIdentifier, + }; + } + + async syncApplication({ + manifest, + dryRun, + workspaceId, + }: { + manifest: ApplicationInput['manifest']; + dryRun?: boolean; + workspaceId: string; + }): Promise { + await this.throttlePerApplication( + manifest.application.universalIdentifier, + workspaceId, + ); + + const versionValidation = + await this.applicationVersionValidationService.validateWorkspaceCompatibility( + { + requiredServerVersion: + manifest.application.requiredServerVersionRange ?? undefined, + workspaceId, + }, + ); + + if (!versionValidation.compatible) { + throw new ApplicationException( + versionValidation.message, + VERSION_REASON_TO_APPLICATION_EXCEPTION_CODE[versionValidation.reason], + ); + } + + if (dryRun === true) { + const { workspaceMigration } = + await this.applicationSyncService.synchronizeFromManifest({ + workspaceId, + manifest, + dryRun: true, + }); + + return { + applicationUniversalIdentifier: + workspaceMigration.applicationUniversalIdentifier, + actions: workspaceMigration.actions, + }; + } + + return this.cacheLockService.withLock( + () => this.applyManifestSync(manifest, workspaceId), + `app-sync:${workspaceId}`, + APP_SYNC_LOCK_OPTIONS, + ); + } + + async uploadApplicationFile({ + workspaceId, + applicationUniversalIdentifier, + fileFolder, + filePath, + getFileBuffer, + }: { + workspaceId: string; + applicationUniversalIdentifier: string; + fileFolder: FileFolder; + filePath: string; + // Lazy so rejected or rate-limited uploads are not buffered into memory. + getFileBuffer: () => Promise; + }): Promise { + await this.throttlePerApplication( + applicationUniversalIdentifier, + workspaceId, + ); + + if (!ALLOWED_APPLICATION_FILE_FOLDERS.includes(fileFolder)) { + throw new ApplicationException( + `Invalid fileFolder for application file upload. Allowed values: ${ALLOWED_APPLICATION_FILE_FOLDERS.join(', ')}`, + ApplicationExceptionCode.INVALID_INPUT, + ); + } + + const pathValidationResult = validateFilePath({ + resourcePath: filePath, + fileFolder, + }); + + if (!pathValidationResult.isValid) { + throw new ApplicationException( + pathValidationResult.error, + ApplicationExceptionCode.INVALID_INPUT, + ); + } + + const application = await this.applicationService.findByUniversalIdentifier( + { + universalIdentifier: applicationUniversalIdentifier, + workspaceId, + }, + ); + + if (!isDefined(application)) { + throw new ApplicationException( + 'Application not found in workspace.', + ApplicationExceptionCode.APPLICATION_NOT_FOUND, + ); + } + + return await this.fileStorageService.writeFile({ + sourceFile: await getFileBuffer(), + fileFolder, + applicationUniversalIdentifier, + workspaceId, + resourcePath: filePath, + settings: { isTemporaryFile: false, toDelete: false }, + }); + } + + private async applyManifestSync( + manifest: ApplicationInput['manifest'], + workspaceId: string, + ): Promise { + const applicationRegistrationId = await this.findApplicationRegistrationId( + manifest.application.universalIdentifier, + ); + + const application = await this.applicationService.findByUniversalIdentifier( + { + universalIdentifier: manifest.application.universalIdentifier, + workspaceId, + }, + ); + + if (!isDefined(application)) { + throw new ApplicationException( + `Application "${manifest.application.universalIdentifier}" not found in workspace "${workspaceId}". Run createDevelopmentApplication first.`, + ApplicationExceptionCode.APPLICATION_NOT_FOUND, + ); + } + + const isFirstSync = !isDefined(application.version); + + const { workspaceMigration, hasSchemaMetadataChanged } = + await this.applicationSyncService.synchronizeFromManifest({ + workspaceId, + manifest, + applicationRegistrationId, + }); + + if (isFirstSync || hasSchemaMetadataChanged) { + await this.sdkClientGenerationService.generateSdkClientForApplication({ + workspaceId, + applicationId: application.id, + applicationUniversalIdentifier: + manifest.application.universalIdentifier, + }); + } + + await this.syncRegistrationMetadata( + applicationRegistrationId, + manifest, + workspaceId, + application.id, + ); + + return { + applicationUniversalIdentifier: + workspaceMigration.applicationUniversalIdentifier, + actions: workspaceMigration.actions, + }; + } + + private async throttlePerApplication( + applicationIdentifier: string, + workspaceId: string, + ): Promise { + await this.throttlerService.tokenBucketThrottleOrThrow( + `app-dev:${workspaceId}:${applicationIdentifier}`, + 1, + APP_DEV_RATE_LIMIT_MAX, + APP_DEV_RATE_LIMIT_WINDOW_MS, + ); + } + + private async findApplicationRegistrationId( + universalIdentifier: string, + ): Promise { + const existingRegistration = + await this.applicationRegistrationService.findOneByUniversalIdentifier( + universalIdentifier, + ); + + if (!existingRegistration) { + throw new ApplicationException( + `No registration found for "${universalIdentifier}". Create one first with createApplicationRegistration.`, + ApplicationExceptionCode.APPLICATION_NOT_FOUND, + ); + } + + return existingRegistration.id; + } + + private async syncRegistrationMetadata( + applicationRegistrationId: string, + manifest: ApplicationInput['manifest'], + workspaceId: string, + applicationId: string, + ): Promise { + const registration = + await this.applicationRegistrationService.findOneByIdGlobal( + applicationRegistrationId, + ); + + // 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 + ) { + return; + } + + const serverUrl = this.twentyConfigService.get('SERVER_URL'); + + const manifestWithResolvedUrls = resolveManifestAssetUrls( + manifest, + (filePath) => + `${serverUrl}/public-assets/${workspaceId}/${applicationId}/${filePath}`, + ); + + await this.applicationRegistrationService.updateFromManifest({ + applicationRegistrationId, + manifest: manifestWithResolvedUrls, + sourceType: ApplicationRegistrationSourceType.LOCAL, + }); + + if (manifest.application.serverVariables) { + await this.applicationRegistrationVariableService.syncVariableSchemas( + applicationRegistrationId, + manifest.application.serverVariables, + ); + } + } +} diff --git a/packages/twenty-server/test/integration/metadata/suites/application/catalog-synced-app-dev-mode.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/catalog-synced-app-dev-mode.integration-spec.ts new file mode 100644 index 0000000000..2994a30df1 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application/catalog-synced-app-dev-mode.integration-spec.ts @@ -0,0 +1,274 @@ +import crypto from 'crypto'; + +import request from 'supertest'; +import { buildBaseManifest } from 'test/integration/metadata/suites/application/utils/build-base-manifest.util'; +import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util'; +import { syncApplication } from 'test/integration/metadata/suites/application/utils/sync-application.util'; +import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util'; +import { type DataSource } from 'typeorm'; + +// A published npm app lands on an instance through the marketplace catalog +// sync as an instance-global, unowned registration (sourceType 'npm'). A dev +// iterating locally on that app with `twenty app dev` must be able to sync it +// into their own workspace without claiming ownership, without rotating the +// shared OAuth client secret, and without mutating the shared registration +// (which would delist the app from the marketplace for every workspace and +// fight with the catalog sync cron). +describe('CLI dev mode on a catalog-synced (npm) app', () => { + const baseUrl = `http://localhost:${APP_PORT}`; + + const universalIdentifier = crypto.randomUUID(); + const roleId = crypto.randomUUID(); + const registrationId = crypto.randomUUID(); + const oAuthClientId = crypto.randomUUID(); + + let ds: DataSource; + + const gqlRequest = (query: string, variables?: Record) => + request(baseUrl) + .post('/metadata') + .set('Authorization', `Bearer ${APPLE_JANE_ADMIN_ACCESS_TOKEN}`) + .send({ query, variables }); + + const fetchRegistrationRow = async () => { + const [row] = await ds.query( + `SELECT name, "sourceType", "workspaceId", "manifest", "isListed" + FROM core."applicationRegistration" WHERE id = $1`, + [registrationId], + ); + + return row; + }; + + beforeAll(async () => { + jest.useRealTimers(); + ds = global.testDataSource; + + // Simulate MarketplaceCatalogSyncService.upsertFromCatalog: unowned + // registration (workspaceId NULL), npm-sourced, no client secret. + await ds.query( + `INSERT INTO core."applicationRegistration" + (id, "universalIdentifier", name, "oAuthClientId", + "oAuthRedirectUris", "oAuthScopes", "workspaceId", "sourceType", + "sourcePackage", "latestAvailableVersion", "isListed", "manifest") + VALUES ($1, $2, $3, $4, $5, $6, NULL, 'npm', $7, '1.0.0', true, $8)`, + [ + registrationId, + universalIdentifier, + 'Published Catalog App', + oAuthClientId, + [], + [], + '@test/published-catalog-app', + JSON.stringify({ application: { universalIdentifier } }), + ], + ); + }); + + afterAll(async () => { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: universalIdentifier, + }); + jest.useFakeTimers(); + }); + + it('reports the universal identifier as already claimed on createApplicationRegistration', async () => { + const res = await gqlRequest( + `mutation CreateApplicationRegistration($input: CreateApplicationRegistrationInput!) { + createApplicationRegistration(input: $input) { + applicationRegistration { id } + clientSecret + } + }`, + { + input: { + name: 'Published Catalog App (dev)', + universalIdentifier, + }, + }, + ).expect(200); + + expect(res.body.errors).toBeDefined(); + expect(res.body.errors[0].message).toContain('already claimed'); + }); + + it('refuses to rotate the shared client secret for a non-owner workspace', async () => { + const res = await gqlRequest( + `mutation RotateSecret($id: String!) { + rotateApplicationRegistrationClientSecret(id: $id) { + clientSecret + } + }`, + { id: registrationId }, + ).expect(200); + + expect(res.body.errors).toBeDefined(); + expect(res.body.errors[0].message).toContain('not found'); + }); + + it('lets a workspace dev-sync the app and mint a dev token without claiming ownership or touching the shared registration', async () => { + const createDevAppRes = await gqlRequest( + `mutation CreateDevApp($universalIdentifier: String!, $name: String!) { + createDevelopmentApplication(universalIdentifier: $universalIdentifier, name: $name) { + id + } + }`, + { universalIdentifier, name: 'Published Catalog App (dev)' }, + ).expect(200); + + expect(createDevAppRes.body.errors).toBeUndefined(); + + const applicationId = createDevAppRes.body.data.createDevelopmentApplication + .id as string; + + // The CLI uploads the app's package.json before syncing the manifest. + await uploadApplicationFile({ + applicationUniversalIdentifier: universalIdentifier, + fileFolder: 'Dependencies', + filePath: 'package.json', + fileBuffer: Buffer.from( + JSON.stringify({ name: 'published-catalog-app', version: '1.0.1' }), + ), + filename: 'package.json', + expectToFail: false, + }); + + const syncRes = await syncApplication({ + manifest: buildBaseManifest({ + appId: universalIdentifier, + roleId, + overrides: { + application: { + universalIdentifier, + defaultRoleUniversalIdentifier: roleId, + displayName: 'Published Catalog App (dev)', + description: 'Local dev iteration of a published app', + applicationVariables: {}, + packageJsonChecksum: null, + yarnLockChecksum: null, + }, + }, + }), + expectToFail: false, + }); + + expect(syncRes.errors).toBeUndefined(); + + // The shared registration is untouched: still npm-sourced, unowned, + // listed in the marketplace catalog, and carrying the published name and + // manifest. + const registrationRow = await fetchRegistrationRow(); + + expect(registrationRow.sourceType).toBe('npm'); + expect(registrationRow.workspaceId).toBeNull(); + expect(registrationRow.name).toBe('Published Catalog App'); + expect(registrationRow.isListed).toBe(true); + expect(registrationRow.manifest).toEqual({ + application: { universalIdentifier }, + }); + + // Dev tooling gets a workspace-scoped app token via + // generateApplicationToken instead of rotating the shared client secret. + const tokenRes = await gqlRequest( + `mutation GenerateApplicationToken($applicationId: UUID!) { + generateApplicationToken(applicationId: $applicationId) { + applicationAccessToken { token expiresAt } + applicationRefreshToken { token expiresAt } + } + }`, + { applicationId }, + ).expect(200); + + expect(tokenRes.body.errors).toBeUndefined(); + expect( + tokenRes.body.data.generateApplicationToken.applicationAccessToken.token, + ).toBeDefined(); + }); + + it('still lets a workspace that owns a non-npm registration sync its metadata', async () => { + // Control: a local registration owned by the workspace keeps receiving + // manifest updates from dev sync (the guard only protects npm/unowned + // registrations). + const ownedUid = crypto.randomUUID(); + const ownedRoleId = crypto.randomUUID(); + + const createRes = await gqlRequest( + `mutation CreateApplicationRegistration($input: CreateApplicationRegistrationInput!) { + createApplicationRegistration(input: $input) { + applicationRegistration { id } + } + }`, + { input: { name: 'Owned Local App', universalIdentifier: ownedUid } }, + ).expect(200); + + expect(createRes.body.errors).toBeUndefined(); + + const ownedRegistrationId = + createRes.body.data.createApplicationRegistration.applicationRegistration + .id; + + try { + const createDevAppRes = await gqlRequest( + `mutation CreateDevApp($universalIdentifier: String!, $name: String!) { + createDevelopmentApplication(universalIdentifier: $universalIdentifier, name: $name) { + id + } + }`, + { universalIdentifier: ownedUid, name: 'Owned Local App' }, + ).expect(200); + + expect(createDevAppRes.body.errors).toBeUndefined(); + + await uploadApplicationFile({ + applicationUniversalIdentifier: ownedUid, + fileFolder: 'Dependencies', + filePath: 'package.json', + fileBuffer: Buffer.from( + JSON.stringify({ name: 'owned-local-app', version: '0.0.1' }), + ), + filename: 'package.json', + expectToFail: false, + }); + + const syncRes = await syncApplication({ + manifest: buildBaseManifest({ + appId: ownedUid, + roleId: ownedRoleId, + overrides: { + application: { + universalIdentifier: ownedUid, + defaultRoleUniversalIdentifier: ownedRoleId, + displayName: 'Owned Local App (renamed)', + description: 'Owned local app', + applicationVariables: {}, + packageJsonChecksum: null, + yarnLockChecksum: null, + }, + roles: [ + { + universalIdentifier: ownedRoleId, + label: 'Owned Local App Role', + description: 'A test role', + }, + ], + }, + }), + expectToFail: false, + }); + + expect(syncRes.errors).toBeUndefined(); + + const [row] = await ds.query( + `SELECT name, "sourceType" FROM core."applicationRegistration" WHERE id = $1`, + [ownedRegistrationId], + ); + + expect(row.name).toBe('Owned Local App (renamed)'); + expect(row.sourceType).toBe('local'); + } finally { + await cleanupApplicationAndAppRegistration({ + applicationUniversalIdentifier: ownedUid, + }); + } + }); +});