Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Contributor
Greptile OverviewGreptile SummaryThis PR implements automatic synchronization in development mode, allowing real-time manifest and file syncing with the server as developers make changes. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant AppDev as AppDevCommand
participant ManifestWatcher as ManifestWatcher
participant ManifestBuild as ManifestBuild
participant FunctionsWatcher as FunctionsWatcher
participant FrontComponentsWatcher as FrontComponentsWatcher
participant FileUploader as FileUploader
participant ApiService as ApiService
participant Server as Twenty Server
Dev->>AppDev: Execute app:dev command
AppDev->>ManifestBuild: runManifestBuild(appPath)
ManifestBuild->>ManifestBuild: Read package.json
ManifestBuild->>ManifestBuild: Read yarn.lock file
ManifestBuild->>ManifestBuild: Build all entities
ManifestBuild-->>AppDev: Return manifest + file paths
AppDev->>AppDev: Initialize FileUploader
AppDev->>ManifestWatcher: Start watching manifest changes
AppDev->>FunctionsWatcher: Start watching function files
AppDev->>FrontComponentsWatcher: Start watching front component files
Note over Dev,Server: Dev mode active - watching for changes
ManifestWatcher->>ManifestBuild: File change detected
ManifestBuild->>ManifestBuild: Rebuild manifest with yarn.lock
ManifestBuild-->>ManifestWatcher: Return updated manifest
ManifestWatcher->>AppDev: onBuildSuccess(manifest)
AppDev->>ApiService: syncApplication(manifest)
ApiService->>Server: POST /metadata (mutation SyncApplication)
Server-->>ApiService: Sync result
ApiService-->>AppDev: Success/Error response
FunctionsWatcher->>FunctionsWatcher: Function file changed
FunctionsWatcher->>AppDev: onFileBuilt(builtPath, checksum)
AppDev->>AppDev: updateFileStatus
AppDev->>FileUploader: uploadFile(builtPath, FileFolder.BuiltFunction)
FileUploader->>Server: Upload function file
FrontComponentsWatcher->>FrontComponentsWatcher: Component file changed
FrontComponentsWatcher->>AppDev: onFileBuilt(builtPath, checksum)
AppDev->>AppDev: updateFileStatus
AppDev->>FileUploader: uploadFile(builtPath, FileFolder.BuiltFrontComponent)
FileUploader->>Server: Upload component file
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+149
to
+152
| const yarnLock = await readFile( | ||
| await findPathFile(appPath, 'yarn.lock'), | ||
| 'utf8', | ||
| ); |
Contributor
There was a problem hiding this comment.
Missing error handling for yarn.lock file read. If yarn.lock doesn't exist, findPathFile will throw an error. Consider handling this gracefully by defaulting to an empty string when the file is missing.
Suggested change
| const yarnLock = await readFile( | |
| await findPathFile(appPath, 'yarn.lock'), | |
| 'utf8', | |
| ); | |
| const yarnLockPath = await findPathFile(appPath, 'yarn.lock').catch( | |
| () => null, | |
| ); | |
| const yarnLock = yarnLockPath | |
| ? await readFile(yarnLockPath, 'utf8') | |
| : ''; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-build.ts
Line: 149:152
Comment:
Missing error handling for yarn.lock file read. If `yarn.lock` doesn't exist, `findPathFile` will throw an error. Consider handling this gracefully by defaulting to an empty string when the file is missing.
```suggestion
const yarnLockPath = await findPathFile(appPath, 'yarn.lock').catch(
() => null,
);
const yarnLock = yarnLockPath
? await readFile(yarnLockPath, 'utf8')
: '';
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+165
to
+166
| private async syncApplication(manifest: ApplicationManifest): Promise<void> { | ||
| initLogger.log('Sync application'); |
Contributor
There was a problem hiding this comment.
Consider adding context to the log message to be more informative, e.g., "Syncing application manifest..."
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-sdk/src/cli/commands/app/app-dev.ts
Line: 165:166
Comment:
Consider adding context to the log message to be more informative, e.g., "Syncing application manifest..."
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+155
to
+157
| if (result.manifest) { | ||
| await this.syncApplication(result.manifest); | ||
| } |
Contributor
There was a problem hiding this comment.
Consider adding debouncing if rapid file changes could trigger multiple sync operations simultaneously.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-sdk/src/cli/commands/app/app-dev.ts
Line: 155:157
Comment:
Consider adding debouncing if rapid file changes could trigger multiple sync operations simultaneously.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
from
e1c82f2 to
bbce1b8
Compare
packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-extract-from-file-server.ts
Show resolved
Hide resolved
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 7 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-sdk/src/cli/utilities/dev/dev-mode-orchestrator.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/utilities/dev/dev-mode-orchestrator.ts:185">
P2: Changes detected during an in-flight sync can be dropped because performSync returns without scheduling a follow-up sync. Schedule another sync instead of returning silently so updates during a sync are eventually applied.</violation>
</file>
<file name="packages/twenty-sdk/src/cli/commands/app/app-dev.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/commands/app/app-dev.ts:73">
P2: Handle errors from startFileWatchers to avoid unhandled promise rejections when watcher startup fails.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| export const EXPECTED_MANIFEST: ApplicationManifest = { | ||
| sources: {}, | ||
| yarnLock: '', |
Member
There was a problem hiding this comment.
I think we should avoid storing files in JSON, this doesn't scale well (same for sources)
I would upload it too (to me it's actully part of the sourceCode of the app but as we will also need it for the functions layer, we can maybe store it somewhere else too.
(by the way, whenever we update the yarn.lock, we should rebuild the functionLayer, to me it's part of the application syncProcess and should generate a migrtion)
Contributor
Author
There was a problem hiding this comment.
I agree, i will improve behavior in another PR. I will use package.json and yarn.lock from tarball sources instead of core.serverlessFunctionLayer columns values
...s/twenty-sdk/src/cli/__tests__/apps/root-app/__integration__/app-dev/tests/manifest.tests.ts
Outdated
Show resolved
Hide resolved
packages/twenty-sdk/src/cli/utilities/build/common/restartable-watcher-interface.ts
Outdated
Show resolved
Hide resolved
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
from
2479ecc to
466ff0c
Compare
packages/twenty-sdk/src/cli/utilities/build/manifest/manifest-build.ts
Outdated
Show resolved
Hide resolved
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
3 times, most recently
from
a437c3b to
e36daa5
Compare
Contributor
There was a problem hiding this comment.
1 issue found across 15 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__e2e__/applications-install-delete-reinstall.e2e-spec.ts">
<violation number="1" location="packages/twenty-sdk/src/cli/__tests__/apps/rich-app/__e2e__/applications-install-delete-reinstall.e2e-spec.ts:32">
P2: The manifest is written to `.twenty/output/manifest.json`, but the test now checks for `manifest.json` at the app root. Update the assertion to point at the output directory so the test validates the generated manifest.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...dk/src/cli/__tests__/apps/rich-app/__e2e__/applications-install-delete-reinstall.e2e-spec.ts
Outdated
Show resolved
Hide resolved
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
from
4ef842e to
a314557
Compare
packages/twenty-sdk/src/cli/__tests__/apps/rich-app/src/utils/test-function-2.util.ts
Outdated
Show resolved
Hide resolved
packages/twenty-sdk/src/cli/utilities/dev/dev-mode-orchestrator.ts
Outdated
Show resolved
Hide resolved
Member
|
Tested it locally, awesome! Feedbacks from usage on my own app (can be treated in another PR)
|
charlesBochet
approved these changes
Jan 26, 2026
Member
charlesBochet
left a comment
charlesBochet
left a comment
There was a problem hiding this comment.
LGTM, awesome, I've left a few comments but non blocking
We just need to fix tests
Contributor
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:48094 This environment will automatically shut down when the PR is closed or after 5 hours. |
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
from
b4e3d5f to
eaec100
Compare
martmull
force-pushed
the
implement-sync-in-dev-mode
branch
from
eaec100 to
c7b400e
Compare
1 similar comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
implement orchestrator to sync application in dev mode
Log example when starting dev mode, delete and add back functions