Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| stackSourcesDir := s.settingsService.GetStringSetting(ctx, "swarmStackSourcesDirectory", "/app/data/swarm/sources") | ||
| stackPath := filepath.Join(stackSourcesDir, sync.EnvironmentID, sync.ProjectName) | ||
|
|
||
| err := s.db.WithContext(ctx).Where("name = ?", sync.ProjectName).First(&proj).Error |
There was a problem hiding this comment.
Project names are not unique, and stack paths are scoped by environment, but this lookup uses only sync.ProjectName. If an ordinary project or a stack in another environment has the same name, this code overwrites that project’s path, target type, and GitOps owner, then binds the new sync to it. The listing reconciliation at backend/internal/project/project_listing.go:86-109 repeats the same name fallback, allowing one sync to take control of an unrelated project.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/gitops/gitops_sync.go
Line: 1193
Comment:
**Project ownership can be overwritten**
Project names are not unique, and stack paths are scoped by environment, but this lookup uses only `sync.ProjectName`. If an ordinary project or a stack in another environment has the same name, this code overwrites that project’s path, target type, and GitOps owner, then binds the new sync to it. The listing reconciliation at `backend/internal/project/project_listing.go:86-109` repeats the same name fallback, allowing one sync to take control of an unrelated project.
**Knowledge Base Used:**
- [GitOps and project automation](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/gitops-and-project-automation.md)
- [Projects, repositories, and buildables](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/projects-repositories-and-buildables.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| project, err := s.upsertSwarmStackProjectRecordInternal(ctx, sync) | ||
| if err != nil { | ||
| slog.WarnContext(ctx, "Failed to upsert Swarm stack project record", "stackName", sync.ProjectName, "error", err) | ||
| } | ||
|
|
||
| if len(syncedFiles) == 0 { | ||
| syncedFiles = singleFileSyncedFilesInternal(sync, source) | ||
| } | ||
| s.updateSyncStatusWithFiles(ctx, id, "success", "", source.commitHash, syncedFiles) | ||
| result.Success = true | ||
| result.Message = fmt.Sprintf("Successfully deployed swarm stack %s from %s", sync.ProjectName, sync.ComposePath) | ||
| s.logSyncSuccess(ctx, sync, project, actor) |
There was a problem hiding this comment.
If project creation fails, upsertSwarmStackProjectRecordInternal returns a nil project and an error. This path only logs the error and then passes that nil project to logSyncSuccess, which dereferences project.Name and panics after the stack was deployed. The helper also discards the project and sync binding update errors at lines 1215 and 1220, and listing reconciliation does the same at backend/internal/project/project_listing.go:95-109. This violates the repository directive to handle all errors explicitly and can report a successful sync with a broken two-way binding.
Rule Used: # Golang Pro Senior Go developer with deep expertise in Go 1.21+, concurrent programming, and cloud-native microservices. Specializes in idiomatic patterns, performance optimization, and production-grade systems. ## Role Definition You are a senio... (source)
Knowledge Base Used: GitOps and project automation
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/gitops/gitops_sync.go
Line: 1157-1168
Comment:
**Failed upsert can panic**
If project creation fails, `upsertSwarmStackProjectRecordInternal` returns a nil project and an error. This path only logs the error and then passes that nil project to `logSyncSuccess`, which dereferences `project.Name` and panics after the stack was deployed. The helper also discards the project and sync binding update errors at lines 1215 and 1220, and listing reconciliation does the same at `backend/internal/project/project_listing.go:95-109`. This violates the repository directive to handle all errors explicitly and can report a successful sync with a broken two-way binding.
**Rule Used:** # Golang Pro Senior Go developer with deep expertise in Go 1.21+, concurrent programming, and cloud-native microservices. Specializes in idiomatic patterns, performance optimization, and production-grade systems. ## Role Definition You are a senio... ([source](https://app.greptile.com/ofkm/-/custom-context?memory=214b40a8-9695-4738-986d-5949b5d65ff1))
**Knowledge Base Used:** [GitOps and project automation](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/gitops-and-project-automation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if syncRecord.TargetType == "swarm_stack" { | ||
| if _, err := s.upsertSwarmStackProjectRecordInternal(ctx, syncRecord); err != nil { | ||
| slog.WarnContext(ctx, "Failed to upsert Swarm stack project record during UpdateSync", "error", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
UpdateSync writes the new target type and project name through an update map but does not refresh syncRecord before this check. Changing a project sync to a Swarm stack therefore skips the upsert, while changing a stack to a project or renaming it upserts the old target and name. This leaves the synthetic project bound to obsolete configuration.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/gitops/gitops_sync.go
Line: 821-825
Comment:
**Upsert uses stale sync state**
`UpdateSync` writes the new target type and project name through an update map but does not refresh `syncRecord` before this check. Changing a project sync to a Swarm stack therefore skips the upsert, while changing a stack to a project or renaming it upserts the old target and name. This leaves the synthetic project bound to obsolete configuration.
**Knowledge Base Used:**
- [GitOps and project automation](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/gitops-and-project-automation.md)
- [Projects, repositories, and buildables](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/projects-repositories-and-buildables.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| Path: stackPath, | ||
| TargetType: "swarm_stack", | ||
| GitOpsManagedBy: &sync.ID, | ||
| Status: projectpkg.ProjectStatusRunning, |
There was a problem hiding this comment.
This initializes the synthetic project as Running before the first deployment. Deployment failures do not update that value, and the listing paths at backend/internal/project/project_listing.go:210-218 and backend/internal/project/project_listing.go:990-997 preserve it whenever no Compose containers are found instead of deriving status from Swarm services or tasks. An initial deployment failure or a subsequently removed or stopped stack will therefore remain displayed and counted as running indefinitely.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/gitops/gitops_sync.go
Line: 1201
Comment:
**Stack status remains falsely running**
This initializes the synthetic project as `Running` before the first deployment. Deployment failures do not update that value, and the listing paths at `backend/internal/project/project_listing.go:210-218` and `backend/internal/project/project_listing.go:990-997` preserve it whenever no Compose containers are found instead of deriving status from Swarm services or tasks. An initial deployment failure or a subsequently removed or stopped stack will therefore remain displayed and counted as running indefinitely.
**Knowledge Base Used:**
- [GitOps and project automation](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/gitops-and-project-automation.md)
- [Projects, repositories, and buildables](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/projects-repositories-and-buildables.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if p.TargetType == "swarm_stack" { | ||
| return true | ||
| } |
There was a problem hiding this comment.
This unconditional cleanup exemption also applies after a stack sync is deleted or converted to a regular project. Deleting a sync clears only gitops_managed_by, while reconciliation processes only current stack syncs. The orphaned project row therefore remains visible without an owner or backing stack and can never be removed by normal filesystem cleanup.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/project/project_sync.go
Line: 358-360
Comment:
**Orphaned stack projects persist**
This unconditional cleanup exemption also applies after a stack sync is deleted or converted to a regular project. Deleting a sync clears only `gitops_managed_by`, while reconciliation processes only current stack syncs. The orphaned project row therefore remains visible without an owner or backing stack and can never be removed by normal filesystem cleanup.
**Knowledge Base Used:**
- [GitOps and project automation](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/gitops-and-project-automation.md)
- [Projects, repositories, and buildables](https://app.greptile.com/ofkm/-/custom-context/knowledge-base/getarcaneapp/arcane/-/docs/projects-repositories-and-buildables.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| filterOptions: tagCatalog.map((tag) => ({ label: tag.name, value: tag.name })) | ||
| }, | ||
| { accessorKey: 'path', title: m.common_working_directory(), sortable: true, cell: DirectoryCell }, | ||
| { accessorKey: 'targetType', title: m.target_type(), sortable: true, cell: TargetTypeCell }, |
There was a problem hiding this comment.
The column is marked sortable, but neither backend listing path supports targetType: the database model lacks the required sortable tag, and the derived-filter configuration has no matching sort binding. Clicking this column therefore falls back to unrelated ordering instead of sorting projects by target type.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/projects-table.svelte
Line: 183
Comment:
**Target sorting is unsupported**
The column is marked sortable, but neither backend listing path supports `targetType`: the database model lacks the required sortable tag, and the derived-filter configuration has no matching sort binding. Clicking this column therefore falls back to unrelated ordering instead of sorting projects by target type.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
|
||
| {#snippet TargetTypeCell({ item }: { item: Project })} | ||
| <Badge variant="outline" class="font-normal capitalize"> | ||
| {item.targetType === 'swarm_stack' ? 'Swarm Stack' : 'Compose'} |
There was a problem hiding this comment.
Swarm Stack and Compose are new user-facing strings, but they are hard-coded instead of using Paraglide messages. Users in non-English locales will therefore see untranslated badge labels.
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/src/routes/(app)/projects/projects-table.svelte
Line: 291
Comment:
**Badge labels bypass localization**
`Swarm Stack` and `Compose` are new user-facing strings, but they are hard-coded instead of using Paraglide messages. Users in non-English locales will therefore see untranslated badge labels.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.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!
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
2 participants
Checklist
mainbranchm.*())What This PR Implements
Fixes: #3686
Changes Made
Frontend
[gitops-sync-dialog.svelte]
formDataderivation bug where changingselectedTargetTypecausedformData.composePathto re-derive and recreate the form state (resetting the selection back to "Project").composePathderivation to checknormalizeTargetType(targetType)instead ofselectedTargetType.TargetTypeonValueChangehandler inSelectWithLabelto updateselectedTargetTypeand set default compose file path (compose.ymlforswarm_stack,docker-compose.ymlforproject).[swarm.ts]
targetType?: string;to frontendProjectinterface.targetTypecolumn andTargetTypeCellbadge renderer inprojects-table.svelteto display deployment target type ("Swarm Stack" or "Compose").Backend & Types
[project.go]
TargetType stringfield toProjectdatabase model andDetailsDTO.[gitops_sync.go]
req.TargetTypeon sync creation ("stack", "swarm", "swarm_stack" -> "swarm_stack").upsertSwarmStackProjectRecordInternalto find or create theProjectrecord in the database withtarget_type = "swarm_stack",gitops_managed_by = &sync.ID, and linksync.ProjectID.upsertSwarmStackProjectRecordInternalduringCreateSync,UpdateSync, andperformSwarmStackSyncInternal.swarmService.DeployStackduringperformSwarmStackSyncInternal.[project_listing.go] [project_sync.go]
reconcileSwarmStackProjectsInternalto reconcile and list Swarm stack project records in project listings (ListAllProjects,ListProjects,GetProjectStatusCounts).swarm_stackprojects when active container count is 0.swarm_stackprojects from filesystem cleanup inskipProjectCleanupInternal.Testing Done
PATH=$PATH:/usr/local/go/bin:$HOME/go/bin go test ./backend/... > backend-test.datbackend-test.log
PATH=$PATH:/usr/local/go/bin:$HOME/go/bin go test ./backend/internal/gitops/... ./backend/internal/project/... > target-domain.logtarget-domain.log
PATH=$PATH:/usr/local/go/bin:$HOME/go/bin go test ./cli/...cli-test.log
PATH=$PATH:/usr/local/go/bin:$HOME/go/bin go test ./types/... > types-test.logtype-test.log
AI Tool Used (if applicable)
Used Google Antigravity with model Gemini 3.6
Additional Context
Also tested manually, bring it up using
Selected Environments > Git Syncs > Add Sync > Target Type > Stack
Stack created:
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 adds persisted and API-visible deployment target types, creates project records for GitOps-managed Swarm stacks, reconciles those records into project listings, and updates the GitOps dialog and project table.
Confidence Score: 0/5
This PR is not safe to merge because stack reconciliation can take over unrelated same-named projects, update and deletion transitions leave orphaned records, status is inaccurate, and database failures can panic the sync path.
The new stack-project lifecycle does not preserve unique two-way ownership: name-only lookup can repoint existing projects, updates operate on stale sync state, deleted bindings are never cleaned up, and failed upserts are either discarded or followed by a nil dereference. Stack status is also initialized and retained as running without consulting the Swarm runtime.
Files Needing Attention: backend/internal/gitops/gitops_sync.go, backend/internal/project/project_listing.go, backend/internal/project/project_sync.go, backend/internal/project/model.go, frontend/src/routes/(app)/projects/projects-table.svelte
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(3686): target type select when using..." | Re-trigger Greptile
Context used (4)