fix: resolve project status using effective compose project name#2198
fix: resolve project status using effective compose project name#2198GiulioSavini wants to merge 3 commits intogetarcaneapp:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if effectiveName == "" || effectiveName == normalizeComposeProjectName(dirName) { | ||
| return nil | ||
| } | ||
| return &effectiveName |
There was a problem hiding this comment.
The rest of this file creates *string values using new(value) (e.g., new(dirName), new(reason), new("healthy")), but this new function uses return &effectiveName. Per the project's pointer-initialisation convention, prefer the consistent form:
| return &effectiveName | |
| return new(effectiveName) |
Rule Used: What: Use the new built-in new function to initi... (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/services/project_service.go
Line: 3226
Comment:
**Pointer idiom inconsistency**
The rest of this file creates `*string` values using `new(value)` (e.g., `new(dirName)`, `new(reason)`, `new("healthy")`), but this new function uses `return &effectiveName`. Per the project's pointer-initialisation convention, prefer the consistent form:
```suggestion
return new(effectiveName)
```
**Rule Used:** What: Use the new built-in `new` function to initi... ([source](https://app.greptile.com/review/custom-context?memory=4da04955-3abb-4e9c-ae2f-26c1a1adeb57))
How can I resolve this? If you propose a fix, please make it concise.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!
| serviceCount, serviceCountErr := s.countServicesFromCompose(ctx, filesystemProject) | ||
| composeProjectName := s.resolveComposeProjectNameInternal(ctx, dirPath, dirName) |
There was a problem hiding this comment.
countServicesFromCompose (line 893) and resolveComposeProjectNameInternal (line 894) both independently call LoadComposeProjectFromDir, each also re-fetching the projects directory setting and path mapper from the settings service. For installations with many projects this doubles the compose-file parsing work on every sync cycle.
Consider extracting a shared loadedProject struct (or accepting the loaded *composetypes.Project as a parameter) so the file is parsed once and both pieces of information are derived from the same parse result.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/internal/services/project_service.go
Line: 893-894
Comment:
**Compose file loaded twice per project per sync**
`countServicesFromCompose` (line 893) and `resolveComposeProjectNameInternal` (line 894) both independently call `LoadComposeProjectFromDir`, each also re-fetching the projects directory setting and path mapper from the settings service. For installations with many projects this doubles the compose-file parsing work on every sync cycle.
Consider extracting a shared `loadedProject` struct (or accepting the loaded `*composetypes.Project` as a parameter) so the file is parsed once and both pieces of information are derived from the same parse result.
How can I resolve this? If you propose a fix, please make it concise.
|
I dont think this needs a whole new column in the table just to a "effective name" this is more over some validation logic failing somewhere or doing the wrong thing. Ill try to look tonight. |
|
The issue isn't really validation logic — the matching works correctly for what it knows. The problem is that normalizeComposeProjectName(directoryName) is the only key used to look up containers, but Docker labels them with |
|
right, so what im saying is if docker for some reason dowesnt allow dashes (which the do im pretty sure), then why is the normalize function removing it, this also bring up the other bug ive forgotten to fix is i dont think the "name" filed in a compose yaml is respected as the project name when it should be. |
|
The project name is set here technically speaking: arcane/backend/pkg/projects/load.go Line 140 in 4d1fabb |
|
Or rather that where the label comes from or should if its deployed with arcane. |
|
well...You're right about the name field — that's a real bug. Arcane always forces normalizeComposeProjectName(directoryName) as the project name when calling LoadComposeProjectFromDir (load.go L140), which overrides whatever compose-go would resolve from the name: field in the YAML. However, the normalize function itself doesn't strip dashes — loader.NormalizeProjectName("mailcow-dockerized") returns mailcow-dockerized unchanged. The mismatch in #1746 comes from the fact that mailcow's own .env sets COMPOSE_PROJECT_NAME=mailcowdockerized (no hyphen), so Docker labels the containers with mailcowdockerized while Arcane looks them up as mailcow-dockerized. Both cases (COMPOSE_PROJECT_NAME override and name: field) share the same root cause: Arcane forces the directory name instead of letting compose-go resolve theeffective project name. My resolveComposeProjectNameInternal handles both — it passes an empty project name so compose-go resolves from COMPOSE_PROJECT_NAME, name: field, or directory name in that order, matching what Docker actually does. That said, if you'd prefer to fix this at the LoadComposeProjectFromDir level (not forcing the name) rather than adding the fallback column, I'm happy to rework the approach. Just let me know which direction you'd prefer. Appreciate your work :) |
|
yes i dont feel we need a brand new column for this , id rather fix whats broken, the name field from the ui, mainly should just serve as the folder name, or project name if none if provided id think is what iwas going for. |
|
Right, I'll close this pr. Do you have something you would like or need me to work on? Bug features etc. Something that you prioritize. Thnks |
|
You can work on anything you would like, but no pressure on anything, if you want to try and do this one still you can, dont matter to me :) |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Projects with COMPOSE_PROJECT_NAME set in their .env file (e.g. mailcow-dockerized setting COMPOSE_PROJECT_NAME=mailcowdockerized) were shown as "stopped" on the Projects page even when all containers were running. The Containers page showed them correctly. Root cause: container-to-project matching used the normalized directory name, but Docker labels containers with the effective compose project name from COMPOSE_PROJECT_NAME. When these differ, no containers matched and the project appeared stopped. Fix: - Add compose_project_name column to projects table (migration 045) - Resolve the effective name by loading the compose file without forcing a project name, letting compose-go read COMPOSE_PROJECT_NAME - Store it during project discovery/sync - Use it as fallback when the normalized name finds no containers Fixes getarcaneapp#1746
- Use new() instead of & for pointer consistency with codebase convention - Load compose file once instead of twice per project during sync by combining countServicesFromCompose and resolveComposeProjectNameInternal into loadComposeMetadataForSyncInternal - Try loading without forcing project name first (to resolve COMPOSE_PROJECT_NAME), fall back to normalized name if that fails
Migration 045 now conflicts with 045_add_webhooks from main. Renaming to 046 restores unique ordering and unblocks CI (backend tests + E2E).
GiulioSavini
force-pushed
the
fix/project-status-compose-name-mismatch
branch
from
2f16a9b to
a72f390
Compare
2 participants
Summary
COMPOSE_PROJECT_NAMEin their.envwere shown as "stopped" on the Projects page while actually runningCOMPOSE_PROJECT_NAMEFixes #1746
Root cause
When Docker Compose starts containers, it labels them with
com.docker.compose.project= the effective project name. This name comes from (in priority order):COMPOSE_PROJECT_NAMEenvironment variable /.envfileArcane matched containers to projects using only
normalizeComposeProjectName(directoryName). For mailcow-dockerized, the directory ismailcow-dockerized(normalized:mailcow-dockerized), but the.envsetsCOMPOSE_PROJECT_NAME=mailcowdockerized(no hyphen). The names don't match → zero containers found → project shown as stopped.The Containers page was immune because it lists containers directly from Docker without project-name matching.
Changes
backend/internal/models/project.goComposeProjectName *stringfield toProjectmodelbackend/resources/migrations/{sqlite,postgres}/045_add_compose_project_name.{up,down}.sqlcompose_project_namecolumnbackend/internal/services/project_service.goresolveComposeProjectNameInternal: loads the compose file without forcing a project name, letting compose-go resolve it fromCOMPOSE_PROJECT_NAMEor directory name. Returns the name only if it differs from the normalized directory name.ptrStringEqualInternal: helper for nullable string comparisonupsertProjectForDir: populatesComposeProjectNameduring project discovery and syncmapProjectToDto: falls back toComposeProjectNamewhen normalized name finds no containersGetProjectStatusCounts: same fallbackTest plan
go test ./internal/services/...passesCOMPOSE_PROJECT_NAME=customnamein.envand a directory name that differsCOMPOSE_PROJECT_NAMEstill match correctlyDisclaimer 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 fixes a long-standing bug where projects using
COMPOSE_PROJECT_NAMEin their.envfile were always displayed as "Stopped" even when their containers were running. The root cause was that container-to-project matching relied solely on the normalised directory name, while Docker labels containers with the effective compose project name (whichCOMPOSE_PROJECT_NAMEcan override).Changes:
ComposeProjectName *stringfield to theProjectmodel and a corresponding migration (045) for both SQLite and Postgres.resolveComposeProjectNameInternal: loads the compose project with an empty name so compose-go resolves it from the.envfile; stores the result only when it differs from the normalised directory name.upsertProjectForDirto populate/syncComposeProjectNameon every discovery/sync pass.mapProjectToDtoandGetProjectStatusCountswith an identical two-step lookup: try the normalised directory name first, fall back toComposeProjectNameonly when no containers are found.ptrStringEqualInternalfor nullable-string comparison used in the sync diff check.The overall approach is sound and correctly follows the compose name-resolution priority order. The SQLite down migration properly reconstructs the full table (required because SQLite doesn't support
DROP COLUMNin older versions) and its column list matches the schema established by migration 044.Confidence Score: 5/5
mapProjectToDtoandGetProjectStatusCounts, the migration is sound for both SQLite and Postgres, and the fallback is gated behind a nil-check so existing projects withoutCOMPOSE_PROJECT_NAMEare unaffected.Important Files Changed
resolveComposeProjectNameInternalandptrStringEqualInternalhelpers; updatesupsertProjectForDir,mapProjectToDto, andGetProjectStatusCountsto store and use the effective compose project name as a fallback for container-matching. Logic is correct but causes a double compose-file load per project on every sync.ComposeProjectName *stringfield with correct GORM column tag and JSON omitempty; straightforward model change.compose_project_nameto theprojectstable; minimal and correct.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[upsertProjectForDir] --> B[countServicesFromCompose\nload with normalized dir name] A --> C[resolveComposeProjectNameInternal\nload with empty project name] C --> D{effectiveName equals\nnormalized dirName?} D -- Yes --> E[return nil - no override needed] D -- No --> F[return pointer to effectiveName] F --> G[ComposeProjectName persisted to DB] H[mapProjectToDto or GetProjectStatusCounts] --> I[Primary lookup\ncontainersByProject normalized-name] I --> J{containers found?} J -- Yes --> K[Use matched containers] J -- No --> L{ComposeProjectName set\nand differs from normName?} L -- Yes --> M[Fallback lookup\ncontainersByProject ComposeProjectName] L -- No --> N[No containers - Stopped or Unknown] M --> KPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: resolve project status using effect..." | Re-trigger Greptile