feat: Git Sync Folder Implementation#1629
Conversation
sasha-id
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Additional Comments (7)
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/fs/fs_writer.go
Line: 19:19
Comment:
unexported function missing "Internal" suffix
```suggestion
func detectExistingComposeFileInternal(dir string) string {
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/git/git.go
Line: 165:165
Comment:
unexported function missing "Internal" suffix
```suggestion
func getKnownHostsPathInternal() string {
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/git/git.go
Line: 181:181
Comment:
unexported function missing "Internal" suffix
```suggestion
func addHostKeyInternal(knownHostsPath, hostname string, key gossh.PublicKey) (err error) {
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/fs/fs_writer.go
Line: 57:57
Comment:
update function call to use new "Internal" suffix
```suggestion
if existingFile := detectExistingComposeFileInternal(dirPath); existingFile != "" {
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/git/git.go
Line: 97:97
Comment:
update function call to use new "Internal" suffix
```suggestion
return knownhosts.New(getKnownHostsPathInternal())
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/git/git.go
Line: 112:112
Comment:
update function call to use new "Internal" suffix
```suggestion
knownHostsPath := getKnownHostsPathInternal()
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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.
Context Used: Rule from Why: Clearly distinguishes private ... (source) 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 AIThis is a comment left during a code review.
Path: backend/internal/utils/git/git.go
Line: 154:154
Comment:
update function call to use new "Internal" suffix
```suggestion
if err := addHostKeyInternal(knownHostsPath, hostname, key); err != nil {
```
**Context Used:** Rule from `dashboard` - What: All unexported functions must have the "Internal" suffix.
Why: Clearly distinguishes private ... ([source](https://app.greptile.com/review/custom-context?memory=306fc233-4d2f-4ac4-bdf7-8059588e8a43))
<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. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
|
@sasha-id As anoteghr PR was merged to support .env files, do you wnat to restructure yours to allow this stuff including with the existsing support? |
Go isn't really my strongest skill — I was happy to make the initial contribution, but I think it would be better if someone more experienced with Go took this on to make sure it's done properly. |
Understood, Ill try to implement this based off your initial implementation. Thank you :) |
sasha-id
force-pushed
the
feat/git-sync-folder
branch
2 times, most recently
from
c4a54b6 to
64e9d14
Compare
…ility - Directory sync mode: sync entire git repo directories to projects, not just compose files - Configurable per-sync limits: maxSyncFiles, maxSyncTotalSize, maxSyncBinarySize (0 = unlimited) - File cleanup: removes files deleted from source repo on subsequent syncs - Project files UI: shows all project directory files in tree view, not just compose/env - Security: path traversal prevention, symlink skipping, binary detection - Fix: resolve http import collision in git client
sasha-id
force-pushed
the
feat/git-sync-folder
branch
from
64e9d14 to
5e24ede
Compare
|
Conflict resolved + few improvements, please review and merge |
|
Ill fix the rest of it tomorrow. Thanks for updating it :) |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
|
Thanks! |
|
Is this already deployed? I dont see the way to configure it |
3 participants
Fixes: #1619
I've implemented the directory sync feature for Git Sync. Here's what was built:
Directory sync capability that allows Git Sync to pull entire directories instead of just single compose files, supporting Docker Compose extends, include, and relative file references.
Disclaimer Greptiles Reviews use AI, make sure to check over its work.
To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike
To have Greptile Re-Review the changes, mention
greptileai.Greptile Summary
This PR implements directory sync for Git Sync, allowing Arcane to pull an entire compose-file directory (including subdirectories) rather than just a single compose file. This enables support for Docker Compose
extends,include, and relative file references that span multiple files. The feature is backward-compatible (syncDirectory=trueis the new default), adds configurable safety limits (file count, total size, binary-file size) enforced at both the per-sync and environment levels, and tracks synced file paths in the database for accurate cleanup on subsequent syncs.Key changes:
WalkDirectoryinbackend/pkg/gitutil/git.go: recursive directory traversal with binary detection vianet/http.DetectContentTypeand null-byte fallback.WriteSyncedDirectory/CleanupRemovedFilesinbackend/pkg/projects/fs_writer.go: secure multi-file writes withIsSafeSubdirectorypath-traversal guards and best-effort empty-directory cleanup.GitOpsSyncService: two-tier limit system (per-sync values capped by environment-wide settings) viaeffectiveIntLimit.enrichWithDirectoryFilesinproject_service.go: displays non-compose, non-env files from the project directory in the UI usingos.OpenRootfor sandboxed access.sync_directory,synced_files, and threemax_*columns with correct defaults.Issues found:
appendSyncFileingit.goreads the full file content into memory before applying the binary-size skip — large binary files cause a transient memory spike before being discarded.effectiveIntLimit/effectiveInt64Limittreat a sync-level0as "defer to environment cap", but all API documentation and model comments state "0 means unlimited" — this semantic mismatch can surprise API consumers who setmaxSyncFiles=0expecting to remove the per-sync cap.walkAndParseSyncDirectoryrelies on an implicit invariant (the walk root is always the direct parent of the compose file) that should be documented to prevent future regressions.Confidence Score: 3/5
Core logic is well-structured and tested, but two concrete issues — unexpected memory pressure on binary-heavy repos and a doc/behaviour mismatch that silently caps 'unlimited' syncs — should be resolved before merging.
The implementation is feature-complete with solid path-traversal guards, proper migration files for both databases, and good test coverage. However, the memory issue in appendSyncFile is a real reliability concern for users with large binaries in their repos, and the '0 means unlimited' semantic contradiction across multiple API types and model comments is a correctness/trust issue that will be hard to fix later without a breaking change.
backend/pkg/gitutil/git.go (appendSyncFile memory), backend/internal/services/gitops_sync_service.go and types/gitops/gitops.go (effectiveIntLimit semantics + documentation)
Important Files Changed
Sequence Diagram
sequenceDiagram participant S as GitOpsSyncService participant G as gitutil.Client participant FS as fs_writer participant DB as Database S->>S: getEffectiveSyncLimits(ctx, sync) note over S: min(sync limits, env limits) S->>G: WalkDirectory(repoPath, composePath, maxFiles, maxTotalSize, maxBinarySize) loop fs.WalkDir G->>G: walkSyncEntry(path, entry) G->>G: appendSyncFile(path) note over G: ReadFile → detect binary → check size limits end G-->>S: DirectoryWalkResult{Files, TotalFiles, TotalSize, SkippedBinaries} S->>S: find compose file in walkResult (by base name) S->>S: getOrCreateProjectInternal(composeContent, nil) S->>FS: CleanupRemovedFiles(projectsRoot, projectPath, oldFiles, newFiles) FS->>FS: delete removed files, cleanupEmptyDirs S->>FS: WriteSyncedDirectory(projectsRoot, projectPath, syncFiles) FS->>FS: validate paths (IsSafeSubdirectory) FS-->>S: writtenPaths S->>DB: updateSyncStatusWithFiles(id, success, commitHash, syncedFiles) DB-->>S: okComments Outside Diff (2)
frontend/src/routes/(app)/projects/[projectId]/+page.svelte, line 715-721 (link)The tablet layout's compose
CodePanellostbind:hasErrors,bind:validationReady,fileId,originalValue,enableDiff, andeditorContext. Withoutbind:hasErrors={composeHasErrors}andbind:validationReady={composeValidationReady}, the save-button guard that prevents submitting invalid YAML will stop working on tablet viewports — users could persist broken compose files without seeing any validation error. The diff view is also silently disabled.These same bindings are correctly kept in the non-tablet code path (around line 857). They should be restored in the tablet branch as well.
Prompt To Fix With AI
frontend/src/routes/(app)/projects/[projectId]/+page.svelte, line 734-743 (link)language="yaml"for all directory filesThe directory sync feature can pull any file type from the repository (shell scripts, JSON, Makefile,
.env, Nginx configs, etc.). Usinglanguage="yaml"for every directory file will produce incorrect syntax highlighting for non-YAML files.Consider deriving the language from the file extension:
{@const dirLang = dirFile.relativePath.endsWith('.json') ? 'json' : dirFile.relativePath.endsWith('.sh') ? 'shell' : dirFile.relativePath.endsWith('.env') ? 'env' : 'yaml'} <CodePanel open={true} title={dirFile.relativePath} language={dirLang} value={dirFile.content} readOnly={true} />This pattern is repeated in two places (tablet and desktop views).
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (3): Last reviewed commit: "fixes" | Re-trigger Greptile
Context used:
Senior Go developer with deep expert... (source)
What: Code should p... (source)