Add manifest integration tests

[martmull]

• add integration test on sync manifest endpoint
• fix invalid standard uuid + migration command

[sentry]

Bug: The FixInvalidStandardUniversalIdentifiersCommand performs multiple database updates without wrapping them in a transaction, unlike other migration commands in the codebase.
_{Severity: HIGH}

Suggested Fix

Wrap the sequence of UPDATE operations within a transaction using queryRunner.startTransaction() and queryRunner.commitTransaction(). Implement a try...catch block to call queryRunner.rollbackTransaction() on error, ensuring atomicity and preventing partial updates.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location:
packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts#L280-L289

Potential issue: The `FixInvalidStandardUniversalIdentifiersCommand` in
`1-19-fix-invalid-standard-universal-identifiers.command.ts` executes multiple `UPDATE`
queries on critical metadata tables (e.g., `role`, `agent`, `objectMetadata`). However,
these operations are not wrapped in a database transaction. This violates the
established pattern seen in other migration commands within the framework. If the
migration script is interrupted or fails partway through, the database will be left in a
partially updated, inconsistent state. This can lead to data corruption and break
metadata integrity for the affected workspace, as there is no automatic rollback
mechanism.

_{Did we get this right? 👍 / 👎 to inform future reviews.}

[cubic-dev-ai]

1 issue found across 32 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts">

<violation number="1" location="packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts:269">
P2: Multiple UPDATE queries across different tables are executed without a transaction. If any query fails mid-way, the workspace will be left in a partially migrated state. Wrap the updates in a transaction using `queryRunner.startTransaction()`, `queryRunner.commitTransaction()`, and `queryRunner.rollbackTransaction()` in the catch/finally block.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: Multiple UPDATE queries across different tables are executed without a transaction. If any query fails mid-way, the workspace will be left in a partially migrated state. Wrap the updates in a transaction using queryRunner.startTransaction(), queryRunner.commitTransaction(), and queryRunner.rollbackTransaction() in the catch/finally block.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts, line 269:

<comment>Multiple UPDATE queries across different tables are executed without a transaction. If any query fails mid-way, the workspace will be left in a partially migrated state. Wrap the updates in a transaction using `queryRunner.startTransaction()`, `queryRunner.commitTransaction()`, and `queryRunner.rollbackTransaction()` in the catch/finally block.</comment>

<file context>
@@ -0,0 +1,359 @@
+      return;
+    }
+
+    const queryRunner = this.coreDataSource.createQueryRunner();
+
+    await queryRunner.connect();
</file context>

[prastoin]

Indeed with should have a query runner here just in case
Especially as we will perform the update on each command run

[cubic-dev-ai]

Thanks for confirming!

[greptile-apps]

Greptile Summary

This PR adds comprehensive integration tests for the manifest sync endpoint and fixes invalid standard UUIDs across the codebase. The changes include:

• Integration Tests: Added 4 new test files covering manifest update scenarios for fields, objects, roles, and skills. Tests verify create, update, and delete operations during manifest sync.
• UUID Fix: Replaced 138+ invalid UUID values in standard objects, fields, indexes, and views with proper UUID v4 format. Invalid UUIDs used patterns like 20202020-0001-0001-0001-000000000001 instead of proper UUID v4.
• Migration Command: Created upgrade command (1-19-fix-invalid-standard-universal-identifiers) to update existing workspaces with the new UUIDs by comparing old and new standard object definitions.
• DTO Updates: Added universalIdentifier fields to FieldMetadataDTO and ObjectMetadataDTO to support manifest sync functionality.
• Bug Fixes: Fixed typo in areFlatObjectMetadataNamesSyncedWithLabels (flatObjectdMetadata → flatObjectMetadata) and improved UUID validation error messages to include the invalid value.
• Test Infrastructure: Added buildBaseManifest utility to streamline integration test setup.

Confidence Score: 5/5

• This PR is safe to merge with comprehensive test coverage and proper migration handling
• The PR includes extensive integration tests for all manifest sync scenarios, a well-structured migration command with dry-run support, and systematic UUID fixes across the entire codebase. The changes are backward-compatible through the migration command that updates existing workspaces.
• No files require special attention

Important Files Changed

Filename	Overview
packages/twenty-server/src/database/commands/upgrade-version-command/1-19/1-19-fix-invalid-standard-universal-identifiers.command.ts	Migration command to fix invalid UUIDs by comparing old and new standard objects, updating mismatches in database
packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-field.integration-spec.ts	Integration tests for field manifest updates: create, update, delete, and rename operations
packages/twenty-server/test/integration/metadata/suites/application/successful-manifest-update-object.integration-spec.ts	Integration tests for object manifest updates: create, update, and delete operations
packages/twenty-shared/src/metadata/constants/standard-object.constant.ts	Updated 138 invalid UUIDs across standard objects, fields, indexes, and views to proper UUID v4 format
packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts	Added universalIdentifier field to FieldMetadataDTO for manifest sync support
packages/twenty-server/src/engine/metadata-modules/object-metadata/dtos/object-metadata.dto.ts	Added universalIdentifier field to ObjectMetadataDTO for manifest sync support

_{Last reviewed commit: 13fa995}

[greptile-apps]

_{32 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[prastoin]

Quick review will check test and command in a second one
Thanks for taking this !

In the end we won't be validating the universal identifier uuid integrity in the common builder/validators ?

[prastoin]

command review

[prastoin]

Indeed with should have a query runner here just in case
Especially as we will perform the update on each command run

[charlesBochet]

Left comments :)

[twenty-eng-sync]

Hey @martmull! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!