diff --git a/packages/twenty-sdk/src/cli/operations/dev-once.ts b/packages/twenty-sdk/src/cli/operations/dev-once.ts index add4dd9452d..b75e994b88c 100644 --- a/packages/twenty-sdk/src/cli/operations/dev-once.ts +++ b/packages/twenty-sdk/src/cli/operations/dev-once.ts @@ -13,7 +13,7 @@ import { manifestUpdateChecksums } from '@/cli/utilities/build/manifest/manifest import { writeManifestToOutput } from '@/cli/utilities/build/manifest/manifest-writer'; import { ClientService } from '@/cli/utilities/client/client-service'; import { ConfigService } from '@/cli/utilities/config/config-service'; -import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events'; +import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors'; import { serializeError } from '@/cli/utilities/error/serialize-error'; import { FileUploader } from '@/cli/utilities/file/file-uploader'; import { runSafe } from '@/cli/utilities/run-safe'; @@ -197,7 +197,7 @@ const innerAppDevOnce = async ( if (!syncResult.success) { const errorEvents = verbose ? null - : formatSyncErrorEvents(syncResult.error); + : formatManifestValidationErrors(syncResult.error); const message = errorEvents ? errorEvents.map((event) => event.message).join('\n') diff --git a/packages/twenty-sdk/src/cli/operations/install.ts b/packages/twenty-sdk/src/cli/operations/install.ts index 91994bb8ea4..7a5379839fe 100644 --- a/packages/twenty-sdk/src/cli/operations/install.ts +++ b/packages/twenty-sdk/src/cli/operations/install.ts @@ -1,6 +1,8 @@ import { ApiService } from '@/cli/utilities/api/api-service'; import { readManifestFromFile } from '@/cli/utilities/build/manifest/manifest-reader'; import { ConfigService } from '@/cli/utilities/config/config-service'; +import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors'; +import { serializeError } from '@/cli/utilities/error/serialize-error'; import { runSafe } from '@/cli/utilities/run-safe'; import { APP_ERROR_CODES, type CommandResult } from '@/cli/types'; @@ -34,16 +36,17 @@ const innerAppInstall = async ( }); if (!result.success) { - const errorMessage = - result.error instanceof Error - ? result.error.message - : String(result.error ?? 'Unknown error'); + const errorEvents = formatManifestValidationErrors(result.error); + + const message = errorEvents + ? errorEvents.map((event) => event.message).join('\n') + : `Install failed with error: ${serializeError(result.error)}`; return { success: false, error: { code: APP_ERROR_CODES.INSTALL_FAILED, - message: errorMessage, + message, }, }; } diff --git a/packages/twenty-sdk/src/cli/utilities/api/file-api.ts b/packages/twenty-sdk/src/cli/utilities/api/file-api.ts index 81bfe83c1ef..a2d069c86b4 100644 --- a/packages/twenty-sdk/src/cli/utilities/api/file-api.ts +++ b/packages/twenty-sdk/src/cli/utilities/api/file-api.ts @@ -161,8 +161,7 @@ export class FileApi { if (response.data.errors) { return { success: false, - error: - response.data.errors[0]?.message || 'Failed to install application', + error: response.data.errors[0] || 'Failed to install application', }; } diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/sync-application-orchestrator-step.ts b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/sync-application-orchestrator-step.ts index 16a08456239..e24362e20a4 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/sync-application-orchestrator-step.ts +++ b/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/sync-application-orchestrator-step.ts @@ -7,7 +7,7 @@ import { type OrchestratorStateStepEvent, type OrchestratorStateSyncStatus, } from '@/cli/utilities/dev/orchestrator/dev-mode-orchestrator-state'; -import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events'; +import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors'; import { serializeError } from '@/cli/utilities/error/serialize-error'; import { type Manifest } from 'twenty-shared/application'; @@ -81,7 +81,7 @@ export class SyncApplicationOrchestratorStep { const errorEvents = this.verbose ? null - : formatSyncErrorEvents(syncResult.error); + : formatManifestValidationErrors(syncResult.error); if (errorEvents) { events.push(...errorEvents); diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/__tests__/format-sync-error-events.spec.ts b/packages/twenty-sdk/src/cli/utilities/error/__tests__/format-manifest-validation-errors.spec.ts similarity index 88% rename from packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/__tests__/format-sync-error-events.spec.ts rename to packages/twenty-sdk/src/cli/utilities/error/__tests__/format-manifest-validation-errors.spec.ts index 27213768554..138270adbc1 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/__tests__/format-sync-error-events.spec.ts +++ b/packages/twenty-sdk/src/cli/utilities/error/__tests__/format-manifest-validation-errors.spec.ts @@ -1,26 +1,26 @@ -import { formatSyncErrorEvents } from '@/cli/utilities/dev/orchestrator/steps/format-sync-error-events'; +import { formatManifestValidationErrors } from '@/cli/utilities/error/format-manifest-validation-errors'; -describe('formatSyncErrorEvents', () => { +describe('formatManifestValidationErrors', () => { it('should return null for null input', () => { - expect(formatSyncErrorEvents(null)).toBeNull(); + expect(formatManifestValidationErrors(null)).toBeNull(); }); it('should return null for undefined input', () => { - expect(formatSyncErrorEvents(undefined)).toBeNull(); + expect(formatManifestValidationErrors(undefined)).toBeNull(); }); it('should return null for non-object input', () => { - expect(formatSyncErrorEvents('string error')).toBeNull(); - expect(formatSyncErrorEvents(42)).toBeNull(); + expect(formatManifestValidationErrors('string error')).toBeNull(); + expect(formatManifestValidationErrors(42)).toBeNull(); }); it('should return null when extensions is missing', () => { - expect(formatSyncErrorEvents({ message: 'error' })).toBeNull(); + expect(formatManifestValidationErrors({ message: 'error' })).toBeNull(); }); it('should return null when extensions.errors is missing', () => { expect( - formatSyncErrorEvents({ + formatManifestValidationErrors({ extensions: { summary: { totalErrors: 1 } }, }), ).toBeNull(); @@ -28,14 +28,14 @@ describe('formatSyncErrorEvents', () => { it('should return null when extensions.summary is missing', () => { expect( - formatSyncErrorEvents({ + formatManifestValidationErrors({ extensions: { errors: {} }, }), ).toBeNull(); }); it('should format a single error', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { fieldMetadata: [ @@ -72,7 +72,7 @@ describe('formatSyncErrorEvents', () => { }); it('should format multiple errors across metadata types', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { fieldMetadata: [ @@ -103,7 +103,7 @@ describe('formatSyncErrorEvents', () => { }); it('should format errors with details for both objectMetadata and fieldMetadata', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { objectMetadata: [ @@ -170,7 +170,7 @@ describe('formatSyncErrorEvents', () => { }); it('should include value in details when present', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { fieldMetadata: [ @@ -194,7 +194,7 @@ describe('formatSyncErrorEvents', () => { }); it('should omit details suffix when no value or universalIdentifier', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { fieldMetadata: [ @@ -212,7 +212,7 @@ describe('formatSyncErrorEvents', () => { }); it('should fall back to entries.length when summary count is missing for a metadata type', () => { - const events = formatSyncErrorEvents({ + const events = formatManifestValidationErrors({ extensions: { errors: { fieldMetadata: [ @@ -233,7 +233,7 @@ describe('formatSyncErrorEvents', () => { }); it('should pluralize correctly for singular and plural counts', () => { - const singleError = formatSyncErrorEvents({ + const singleError = formatManifestValidationErrors({ extensions: { errors: { objectMetadata: [ @@ -249,7 +249,7 @@ describe('formatSyncErrorEvents', () => { expect(singleError?.[0].message).toBe('Sync failed with 1 error'); expect(singleError?.[1].message).toBe('objectMetadata: 1 error'); - const multipleErrors = formatSyncErrorEvents({ + const multipleErrors = formatManifestValidationErrors({ extensions: { errors: { objectMetadata: [ diff --git a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/format-sync-error-events.ts b/packages/twenty-sdk/src/cli/utilities/error/format-manifest-validation-errors.ts similarity index 97% rename from packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/format-sync-error-events.ts rename to packages/twenty-sdk/src/cli/utilities/error/format-manifest-validation-errors.ts index 34fc9f94171..a7e042b1746 100644 --- a/packages/twenty-sdk/src/cli/utilities/dev/orchestrator/steps/format-sync-error-events.ts +++ b/packages/twenty-sdk/src/cli/utilities/error/format-manifest-validation-errors.ts @@ -15,7 +15,7 @@ type StructuredSyncError = { }; }; -export const formatSyncErrorEvents = ( +export const formatManifestValidationErrors = ( error: unknown, ): OrchestratorStateStepEvent[] | null => { if (!error || typeof error !== 'object') { diff --git a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.resolver.ts b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.resolver.ts index 49d2036a4f6..85d5d1d2ae0 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-install/application-install.resolver.ts @@ -1,4 +1,9 @@ -import { UseFilters, UseGuards, UsePipes } from '@nestjs/common'; +import { + UseFilters, + UseGuards, + UseInterceptors, + UsePipes, +} from '@nestjs/common'; import { Args, Mutation, Query } from '@nestjs/graphql'; import { PermissionFlagType } from 'twenty-shared/constants'; @@ -14,10 +19,12 @@ import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard'; 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'; @UsePipes(ResolverValidationPipe) @MetadataResolver() @UseFilters(ApplicationExceptionFilter, AuthGraphqlApiExceptionFilter) +@UseInterceptors(WorkspaceMigrationGraphqlApiExceptionInterceptor) @UseGuards(WorkspaceAuthGuard) export class ApplicationInstallResolver { constructor( diff --git a/packages/twenty-server/src/engine/core-modules/application/application-marketplace/marketplace.resolver.ts b/packages/twenty-server/src/engine/core-modules/application/application-marketplace/marketplace.resolver.ts index 75e94657041..e10e2f2fcee 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-marketplace/marketplace.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-marketplace/marketplace.resolver.ts @@ -1,4 +1,4 @@ -import { UseFilters, UseGuards } from '@nestjs/common'; +import { UseFilters, UseGuards, UseInterceptors } from '@nestjs/common'; import { Args, Mutation, Query } from '@nestjs/graphql'; import { PermissionFlagType } from 'twenty-shared/constants'; @@ -13,6 +13,7 @@ import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorat import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard'; 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 { MarketplaceCatalogSyncCronJob } from 'src/engine/core-modules/application/application-marketplace/crons/marketplace-catalog-sync.cron.job'; 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'; @@ -20,6 +21,7 @@ import { MessageQueueService } from 'src/engine/core-modules/message-queue/servi @MetadataResolver() @UseFilters(ApplicationRegistrationExceptionFilter) +@UseInterceptors(WorkspaceMigrationGraphqlApiExceptionInterceptor) @UseGuards(WorkspaceAuthGuard, NoPermissionGuard) export class MarketplaceResolver { constructor( diff --git a/packages/twenty-server/test/integration/metadata/suites/application/failing-install-application-validation-errors.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application/failing-install-application-validation-errors.integration-spec.ts new file mode 100644 index 00000000000..a08241ef9fe --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application/failing-install-application-validation-errors.integration-spec.ts @@ -0,0 +1,184 @@ +import crypto from 'crypto'; +import { promises as fs } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; + +import * as tar from 'tar'; +import { installApplication } from 'test/integration/metadata/suites/application/utils/install-application.util'; +import { uploadAppTarball } from 'test/integration/metadata/suites/application/utils/upload-app-tarball.util'; +import { STANDARD_OBJECTS } from 'twenty-shared/metadata'; +import { type DataSource } from 'typeorm'; + +const createTestTarball = async ( + files: Record, +): Promise => { + const tempId = crypto.randomUUID(); + const sourceDir = join(tmpdir(), `test-tarball-src-${tempId}`); + const tarballPath = join(tmpdir(), `test-tarball-${tempId}.tar.gz`); + + await fs.mkdir(sourceDir, { recursive: true }); + + for (const [name, content] of Object.entries(files)) { + const filePath = join(sourceDir, name); + const dir = filePath.substring(0, filePath.lastIndexOf('/')); + + if (dir !== sourceDir) { + await fs.mkdir(dir, { recursive: true }); + } + await fs.writeFile(filePath, content); + } + + await tar.create( + { + file: tarballPath, + gzip: true, + cwd: sourceDir, + }, + Object.keys(files), + ); + + const buffer = await fs.readFile(tarballPath); + + await fs.rm(sourceDir, { recursive: true, force: true }); + await fs.rm(tarballPath, { force: true }); + + return buffer; +}; + +const buildManifestWithCrossEntityIdentifierConflict = ( + universalIdentifier: string, + roleUniversalIdentifier: string, + duplicatedUniversalIdentifier: string, +) => + JSON.stringify({ + application: { + universalIdentifier, + displayName: 'Test App With Cross Entity Identifier Conflict', + description: + 'A test app whose manifest reuses a universalIdentifier across entity types', + icon: 'IconTestPipe', + defaultRoleUniversalIdentifier: roleUniversalIdentifier, + applicationVariables: {}, + packageJsonChecksum: null, + yarnLockChecksum: null, + }, + roles: [ + { + universalIdentifier: roleUniversalIdentifier, + label: 'First Role', + description: 'First role', + }, + { + universalIdentifier: duplicatedUniversalIdentifier, + label: 'Second Role', + description: 'Second role', + objectPermissions: [ + { + universalIdentifier: duplicatedUniversalIdentifier, + objectUniversalIdentifier: + STANDARD_OBJECTS.company.universalIdentifier, + canReadObjectRecords: true, + canUpdateObjectRecords: false, + canSoftDeleteObjectRecords: false, + canDestroyObjectRecords: false, + }, + ], + }, + ], + skills: [], + agents: [], + objects: [], + fields: [], + logicFunctions: [], + frontComponents: [], + publicAssets: [], + views: [], + navigationMenuItems: [], + pageLayouts: [], + }); + +describe('Install application should return structured validation errors', () => { + let ds: DataSource; + const createdRegistrationIds: string[] = []; + const createdApplicationUniversalIdentifiers: string[] = []; + + beforeAll(() => { + jest.useRealTimers(); + ds = globalThis.testDataSource; + }); + + afterAll(async () => { + for (const uid of createdApplicationUniversalIdentifiers) { + await ds.query( + `DELETE FROM core."file" WHERE "applicationId" IN ( + SELECT id FROM core."application" WHERE "universalIdentifier" = $1 + )`, + [uid], + ); + + await ds.query( + `DELETE FROM core."application" WHERE "universalIdentifier" = $1`, + [uid], + ); + } + + for (const id of createdRegistrationIds) { + await ds.query( + `DELETE FROM core."applicationRegistration" WHERE id = $1`, + [id], + ); + } + + jest.useFakeTimers(); + }); + + it('should return METADATA_VALIDATION_FAILED with structured errors when installing an app whose manifest has validation errors', async () => { + const universalIdentifier = crypto.randomUUID(); + const roleUniversalIdentifier = crypto.randomUUID(); + const duplicatedUniversalIdentifier = crypto.randomUUID(); + const manifest = buildManifestWithCrossEntityIdentifierConflict( + universalIdentifier, + roleUniversalIdentifier, + duplicatedUniversalIdentifier, + ); + + const tarball = await createTestTarball({ + 'manifest.json': manifest, + 'package.json': JSON.stringify({ + name: 'test-cross-entity-identifier-conflict-app', + version: '1.0.0', + }), + }); + + const uploadResult = await uploadAppTarball({ + tarballBuffer: tarball, + universalIdentifier, + }); + + expect(uploadResult.errors).toBeUndefined(); + expect(uploadResult.data?.uploadAppTarball.id).toBeDefined(); + + const registrationId = uploadResult.data!.uploadAppTarball.id; + + createdRegistrationIds.push(registrationId); + createdApplicationUniversalIdentifiers.push(universalIdentifier); + + const { errors } = await installApplication({ + input: { + appRegistrationId: registrationId, + }, + expectToFail: true, + }); + + expect(errors).toBeDefined(); + expect(errors.length).toBe(1); + + const [error] = errors; + + expect(error.extensions.code).toBe('METADATA_VALIDATION_FAILED'); + expect(error.extensions.errors).toBeDefined(); + expect(error.extensions.summary).toBeDefined(); + expect(error.extensions.summary.totalErrors).toBeGreaterThan(0); + expect(error.extensions.message).toMatch(/Validation failed for/); + }, 120000); +});