logicFunction sourceHandlerPath and builtHandlerPath manifest updates are not saved

[Weiko]

and delete legacy files

[greptile-apps]

Greptile Summary

This PR fixes a bug where updates to sourceHandlerPath and builtHandlerPath in logic function manifests were not being saved during workspace migrations. The fix involves two key changes: marking builtHandlerPath as a comparable property (setting toCompare: true), and adding file cleanup logic to delete old source and built files when their paths change.

Key Changes:

• Enabled change tracking for builtHandlerPath by setting toCompare: true in the flat entity configuration
• Added file deletion logic in the update handler to clean up old source and built files when paths change
• Updated test snapshot to reflect the new comparable property

Observations:

• The database update occurs before file deletion, which could leave orphaned files if deletion fails (though this is a minor concern)
• The fix properly handles both sourceHandlerPath and builtHandlerPath changes
• File cleanup uses the existing FileStorageService.delete method which handles both storage and database cleanup

Confidence Score: 4/5

• This PR is safe to merge with minor considerations around error handling
• The changes correctly fix the reported issue by enabling change tracking for builtHandlerPath and adding proper file cleanup. The implementation is straightforward and follows existing patterns. Score reduced by 1 due to the order of operations (DB update before file deletion) which could theoretically leave orphaned files if deletion fails, though this is unlikely to cause production issues.
• No files require special attention - all changes are straightforward bug fixes

Important Files Changed

Filename	Overview
packages/twenty-server/src/engine/metadata-modules/flat-entity/constant/tests/snapshots/all-universal-flat-entity-properties-to-compare-and-stringify.constant.spec.ts.snap	Test snapshot updated to include builtHandlerPath in the propertiesToCompare array for logicFunction entities
packages/twenty-server/src/engine/metadata-modules/flat-entity/constant/all-entity-properties-configuration-by-metadata-name.constant.ts	Changed builtHandlerPath.toCompare from false to true to enable tracking of changes to this property during workspace migrations
packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/logic-function/services/update-logic-function-action-handler.service.ts	Added file cleanup logic to delete old source and built files when their paths change, fixing issue where manifest updates were not persisted

_{Last reviewed commit: ef3db98}

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/logic-function/services/update-logic-function-action-handler.service.ts
Database update happens before file deletion - if file deletion fails, the database will have the new paths but old files will remain in storage. Consider performing file deletion first, or wrapping both operations in a try-catch to handle failures gracefully.

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (unresolved 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/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/logic-function/services/update-logic-function-action-handler.service.ts">

<violation number="1" location="packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/logic-function/services/update-logic-function-action-handler.service.ts:98">
P2: Deleting by subfolder when sourceHandlerPath changes wipes the entire logic-function source folder. If the new source handler path is in the same folder, this deletes the new file too. Use the previous sourceHandlerPath so only the old file is removed.</violation>
</file>

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

[cubic-dev-ai]

P2: Deleting by subfolder when sourceHandlerPath changes wipes the entire logic-function source folder. If the new source handler path is in the same folder, this deletes the new file too. Use the previous sourceHandlerPath so only the old file is removed.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-server/src/engine/workspace-manager/workspace-migration/workspace-migration-runner/action-handlers/logic-function/services/update-logic-function-action-handler.service.ts, line 98:

<comment>Deleting by subfolder when sourceHandlerPath changes wipes the entire logic-function source folder. If the new source handler path is in the same folder, this deletes the new file too. Use the previous sourceHandlerPath so only the old file is removed.</comment>

<file context>
@@ -58,5 +89,23 @@ export class UpdateLogicFunctionActionHandlerService extends WorkspaceMigrationR
+        workspaceId,
+        applicationUniversalIdentifier,
+        fileFolder: FileFolder.Source,
+        resourcePath: getLogicFunctionSubfolderForFromSource(entityId),
+      });
+    }
</file context>

Suggested change

	resourcePath: getLogicFunctionSubfolderForFromSource(entityId),
	resourcePath: existingLogicFunction.sourceHandlerPath,

[charlesBochet]

I would not touch source at all, it's not their responsibility

I could even argue that built source is not but as there is a 1 for 1 logicFunction / built files, why not

[martmull]

agree with Charles