feat(containers): compose editor + compose tab improvements#2052
feat(containers): compose editor + compose tab improvements#2052kmendell merged 29 commits intogetarcaneapp:mainfrom
Conversation
…iners (getarcaneapp#1554) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ainerComposePanel component - Fix project lookup: search by name via getProjectsForEnvironment then fetch by ID, instead of incorrectly passing project name to getProject (which expects an ID) - Move project loading to +page.ts data loader (proper SvelteKit pattern) - Extract inline CodePanel usage into dedicated ContainerComposePanel.svelte component - Support editable compose file with save via projectService.updateProject - Show read-only alert for GitOps-managed projects - Use untrack() on initial state to satisfy Svelte 5 reactive reference rules - Link back to project page via ExternalLink button Closes getarcaneapp#1554
…-compose
When a project's compose.yml uses `include:` directives (a meta/wrapper compose),
the service definition lives in a sub-file — not the root compose. Showing and editing
the root compose from a container view is misleading and not useful.
Parse the project's composeContent with the yaml library to check whether
`services.{serviceName}` exists directly in the top-level services block.
If not, suppress the Compose tab entirely for that container.
Also tightens the info banner copy to drop the redundant
'Changes to this file affect all services' sentence (now always accurate since
we only show direct-service composes).
…ived not a stored function
$derived((): boolean => {...}) stores a function as the derived value; Svelte 5
doesn't track reactive reads (project, composeServiceName) when the stored function
is called later from inside tabItems. Use an IIFE inside $derived instead so the
boolean is computed directly and reactivity is properly tracked.
…including sub-files)
When a container belongs to a project with `include:` directives, find the
include file that directly defines the service and show/edit that instead of
the root wrapper compose.
- serviceComposeSource derived: scans root compose, then includeFiles[], to find
which file has `services.{serviceName}` defined
- Shows tab for both root-defined and include-file-defined services
- Hides tab only when the service isn't found in any file
- ContainerComposePanel now accepts optional `includeFile` prop
- Displays includeFile.content instead of project.composeContent
- Saves via projectService.updateProjectIncludeFile (relativePath + content)
- Falls back to updateProject for root compose
- fileTitle shows the actual file name (e.g. ollama/compose.yml)
- fileId scoped per include file so editor state doesn't collide
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
|
I'm not going to blindly approve those AI suggestions like last time. |
|
I would prefer to do most of this in the backend , vs all of it client side in the browser, that will just increase load time , go tends to be faster for loading this stuff. If the label exists just then incldue it in the reponse. Then if that is included the tab would show conditionally. As far as greptiles comments, they are all valid state being updated in an effect is aggainst sveltes reomendations. And there are a few missing i18n strings. |
|
Unfortunately, I don't think there's a good way to determine the necessary information in backend. I'll investigate further. |
|
This pull request has merge conflicts. Please resolve the conflicts so the PR can stay up-to-date and reviewed. |
- Add ComposeInfo to backend container Details response with Docker Compose metadata - Extract compose project/service info from labels in Go (backend-side processing) - Add i18n strings for all hardcoded text in ContainerComposePanel component - Simplify frontend logic to use composeInfo from API instead of label parsing - Fix Svelte reactivity pattern to properly track source content changes - Remove redundant Svelte effect antipattern per code review feedback Addresses code review feedback to move heavy lifting to backend and ensure proper internationalization coverage.
c19f751 to
678f8f2
Compare
Resolved conflict in types/container/container.go by keeping both: - ComposeInfo struct (from feature branch) - SummaryGroup struct (from upstream main) Both structs serve different purposes and can coexist without conflict.
|
@mkaltner You should be able to inspect the labels of teh container, i do that in other places |
1. Add dirty tracking to prevent unsaved edits from being lost on data refresh
- Track isDirty state based on composeContent vs sourceContent
- Only update editor when source changes AND no unsaved edits exist
- Reset isDirty flag after successful save
2. Add 'Viewing' vs 'Editing' label for read-only mode
- Added container_compose_viewing_info i18n string
- Conditionally display 'Viewing' when isReadOnly is true
- Maintains consistent i18n coverage
3. Remove unused saveError state variable
- Errors are already surfaced via toast notifications
- Simplifies component state management
4. Add explicit pagination limit to project lookup
- Pass pagination: { page: 1, limit: 100 } to getProjectsForEnvironment
- Ensures compose tab appears even with many projects (>20)
- Prevents silently missing projects beyond default page size
Addresses automated review feedback from greptile-apps[bot].
|
Please see the latest push, this should address your concerns as well as the code review comments. Thanks! |
🔧 Fix for gocognit linter failureThe CI is failing because Changes
Patchdiff --git a/types/container/container.go b/types/container/container.go
index 7890d9ce..6d54203b 100644
--- a/types/container/container.go
+++ b/types/container/container.go
@@ -832,33 +832,69 @@ func NewSummary(c container.Summary) Summary {
}
// NewDetails creates a Details from a docker container.InspectResponse.
-func NewDetails(c *container.InspectResponse) Details {
+func extractPorts(c *container.InspectResponse) []Port {
ports := make([]Port, 0)
- if c.NetworkSettings != nil && c.NetworkSettings.Ports != nil {
- for p, bindings := range c.NetworkSettings.Ports {
- privatePort := int(p.Num())
- typ := string(p.Proto())
-
- // When no host bindings exist, still include the private port
- if len(bindings) == 0 {
- ports = append(ports, Port{
- PrivatePort: privatePort,
- Type: typ,
- })
- continue
- }
- for _, b := range bindings {
- pub, _ := strconv.Atoi(b.HostPort)
- ports = append(ports, Port{
- IP: b.HostIP.String(),
- PrivatePort: privatePort,
- PublicPort: pub,
- Type: typ,
- })
- }
+ if c.NetworkSettings == nil || c.NetworkSettings.Ports == nil {
+ return ports
+ }
+
+ for p, bindings := range c.NetworkSettings.Ports {
+ privatePort := int(p.Num())
+ typ := string(p.Proto())
+
+ // When no host bindings exist, still include the private port
+ if len(bindings) == 0 {
+ ports = append(ports, Port{
+ PrivatePort: privatePort,
+ Type: typ,
+ })
+ continue
}
+ for _, b := range bindings {
+ pub, _ := strconv.Atoi(b.HostPort)
+ ports = append(ports, Port{
+ IP: b.HostIP.String(),
+ PrivatePort: privatePort,
+ PublicPort: pub,
+ Type: typ,
+ })
+ }
+ }
+ return ports
+}
+
+func extractComposeInfo(labels map[string]string) *ComposeInfo {
+ projectName, hasProject := labels["com.docker.compose.project"]
+ if !hasProject {
+ return nil
+ }
+
+ serviceName, hasService := labels["com.docker.compose.service"]
+ if !hasService {
+ return nil
+ }
+
+ info := &ComposeInfo{
+ ProjectName: projectName,
+ ServiceName: serviceName,
+ }
+
+ if configFile, ok := labels["com.docker.compose.config-files"]; ok {
+ info.ConfigFile = configFile
+ }
+ if workingDir, ok := labels["com.docker.compose.project.working_dir"]; ok {
+ info.WorkingDir = workingDir
+ }
+ if projectDir, ok := labels["com.docker.compose.project.config_files"]; ok {
+ info.ProjectDir = projectDir
+ }
+
+ return info
+}
+
+func NewDetails(c *container.InspectResponse) Details {
+ ports := extractPorts(c)
+
mounts := make([]Mount, 0, len(c.Mounts))
for _, m := range c.Mounts {
mounts = append(mounts, Mount{
@@ -922,24 +958,7 @@ func NewDetails(c *container.InspectResponse) Details {
}
// Extract Docker Compose information from labels if present
- var composeInfo *ComposeInfo
- if projectName, hasProject := labels["com.docker.compose.project"]; hasProject {
- if serviceName, hasService := labels["com.docker.compose.service"]; hasService {
- composeInfo = &ComposeInfo{
- ProjectName: projectName,
- ServiceName: serviceName,
- }
- if configFile, ok := labels["com.docker.compose.config-files"]; ok {
- composeInfo.ConfigFile = configFile
- }
- if workingDir, ok := labels["com.docker.compose.project.working_dir"]; ok {
- composeInfo.WorkingDir = workingDir
- }
- if projectDir, ok := labels["com.docker.compose.project.config_files"]; ok {
- composeInfo.ProjectDir = projectDir
- }
- }
- }
+ composeInfo := extractComposeInfo(labels)
return Details{
ID: c.ID,To apply:cd ~/Projects/arcane
git checkout feat/container-compose-editor
curl -o /tmp/fix.patch https://gist.github.com/... # or copy the diff above
git apply /tmp/fix.patch
git add types/container/container.go
git commit -m "refactor(types): reduce NewDetails cognitive complexity"
git pushVerified: builds successfully with |
|
@greptileai Please re-review — all four issues from the previous review were addressed in commit 1d802f1. |
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
…er-controlled values
- Replace $effect-based isDirty state with $derived for proper Svelte 5 reactivity
- Remove manual isDirty reset in handleSave (now auto-computed)
- Add escapeHtml() utility to prevent XSS vulnerabilities
- Escape all user-controlled values (project name, file paths, service names) before {@html} interpolation
Addresses Greptile security and code quality feedback.
- Fix XSS vulnerability by decomposing info banner to native Svelte (no {@html})
- Change isDirty to $derived instead of $state in ContainerComposePanel (Svelte rule violation)
- Remove dead isDirty = false reset in handleSave
- Rename ProjectDir to ConfigFiles and update doc comment for clarity
- Add tabs_compose i18n key and use it in Compose tab label
|
@greptileai All four issues addressed in 74864d8:
Please re-review. |
|
✅ Re: Greptile feedback All issues addressed in 74864d8: XSS vulnerability (ContainerComposePanel.svelte) - Replaced
Backend field mismatch (container.go) - Renamed Hardcoded i18n (+page.svelte) - Added Thank you for the thorough security review! |
- Remove {@html} interpolation with user-controlled values
(project.name, serviceName, includeFile.relativePath)
Plain text i18n messages don't need {@html}, prevents XSS
- Wrap $state mutations in untrack() to avoid reactivity cycle
Reads reactive sources outside, writes inside untrack()
Fixes Svelte 5 anti-pattern per codebase rules
Addresses core Greptile review concerns for PR getarcaneapp#2052
|
✅ Core security and reactivity issues fixed in commit 2521a72 Addressed the 3/5 confidence score blockers:
Previous commit (63bd939) fixed the two new findings:
@greptileai please re-review — all flagged issues should now be resolved |
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
frontend/src/routes/(app)/containers/[containerId]/+page.svelte
Outdated
Show resolved
Hide resolved
- Use {#key} remount strategy instead of $state mutations in $effect
Parent wraps ContainerComposePanel with {#key} on file path,
forcing clean remount on context switch (no manual tracking)
- Remove Map cache side effect from $derived block
YAML files are small, re-parsing is acceptable
- Change catch (err: any) to catch (err: unknown) with proper narrowing
All three Greptile-flagged issues resolved per project rules
|
✅ All Svelte 5 reactivity rule violations resolved in commit 78ddf74 Changes:
All three Greptile-flagged issues properly fixed per project rules. @greptileai please re-review |
frontend/src/routes/(app)/containers/[containerId]/+page.svelte
Outdated
Show resolved
Hide resolved
Two containers from different projects both using root compose
would share the same key ('root'), causing stale content and
potential cross-project save (data loss).
Key now includes project ID: `${project?.id}-${relativePath ?? 'root'}`
|
✅ Fixed cross-project key collision in commit ba62a3d Changed {#key serviceComposeSource?.includeFile?.relativePath ?? 'root'}To: {#key \`${project?.id}-${serviceComposeSource?.includeFile?.relativePath ?? 'root'}\`}Now two containers from different projects won't share the same key even if both use root compose. Frontend build verified ✅ @greptileai please re-review |
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
- Add disabled={!isDirty} to save button to prevent unnecessary API calls
- Route project list fetch through queryClient for cache benefits on repeat nav
|
✅ Minor UX/perf fixes in commit c844b33
Build verified ✅ @greptileai please re-review |
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
frontend/src/routes/(app)/containers/components/ContainerComposePanel.svelte
Outdated
Show resolved
Hide resolved
- Move success toast before invalidateAll() to avoid misleading error on cache refresh failure - Pass actual root compose filename instead of hardcoding compose.yml
|
Seems like a timeout error in the build. I think everything else is good now. |
|
The more i think about this the more im aggainst it. Id be fine with a lint in the tab bar to take you to the proejct but adding a editor here, just seems redundant honestly. and duplciated code. IF that makes sense, a link to eidt the project would make more sense to me. |
Extract duplicated compose editor UI (GitOps alert, info banner, save button, view project link, toast+invalidateAll logic) into a reusable ComposeEditorWrapper component in $lib/components/compose/. Update ContainerComposePanel to delegate to it via an onSave callback. Co-authored-by: Michael Kaltner <mkaltner@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
I pushed a small refactor. On the broader point: I don't see this as duplicated functionality. It's the same functionality accessible from the appropriate context, like how you can restart a container from the container view or the project view, as well as the recently merged redeploy. Multiple entry points, same underlying action. When I'm troubleshooting a specific container, I want to edit its config and redeploy without navigating away to find which project/file it lives in. That's the workflow this enables. Happy to discuss further if you want, but I think this is a UX improvement worth having. |
|
Okay i guess sure i get it. Ill look again tonight. |
|
Lgtm, Thanks |


Summary
Adds a Compose tab to the individual container detail view. When a container belongs to an Arcane-managed project, the tab loads the relevant compose file — whether defined directly in the root compose or in an included sub-file — and allows viewing and editing it without leaving the container view.
Fixes: #1554
Changes
+page.tsLoads the associated Arcane project during page load by matching the container's
com.docker.compose.projectlabel against project names viagetProjectsForEnvironment(search by name, then fetch full detail by ID). Returnsprojectas part of page data.+page.sveltecomposeServiceNamefrom thecom.docker.compose.servicecontainer labelserviceComposeSource: scans root compose first, thenproject.includeFiles[], to find which file directly definesservices.{serviceName}{ includeFile: null }when the service is in the root compose{ includeFile: <file> }when the service is in an included sub-filenull(tab hidden) when the service is not found anywhereserviceComposeSourceis non-nullContainerComposePanelwith the project and resolved include fileContainerComposePanel.svelte(new)project,serviceName, and optionalincludeFilepropsincludeFile.contentfor sub-files,project.composeContentfor rootupdateProjectIncludeFilefor sub-files,updateProjectfor rootfileIdscoped per file so editor state does not collide across servicesBehavior
updateProjectollama/compose.yml), saves viaupdateProjectIncludeFilePR Checklist
./scripts/development/dev.sh start)just lint frontend(currently 0 errors, 8 pre-existing CSS warnings unrelated to this PR)just format frontendManual Testing
include:-based project (e.g.ai-ollama-1→ollama/compose.yml)AI Disclosure
AI Tool: Claude Code (Anthropic)
Assistance Level: Significant — implementation was AI-generated with human direction, review, and testing oversight per AI_POLICY.md
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 a Compose tab to the container detail view, allowing users to view and edit the relevant compose file (root or include sub-file) directly from the container page. The implementation correctly extracts Docker Compose labels into a
ComposeInfostruct on the backend, looks up the matching Arcane project during page load, performs YAML service-name lookups to find which file owns the service, and uses{#key}remounting to safely handle container/file switches.Previous review issues have been thoroughly addressed —
isDirtyis correctly$derived, the{#key}now includesproject.id, the save button is disabled when content is unchanged,invalidateAll()is called after save, the info banner conditionally says "Viewing"/"Editing", and all{@html}usage has been eliminated.Remaining issues:
ContainerComposePanel.svelte,toast.successis called afterawait invalidateAll(). IfinvalidateAll()rejects (transient network error), the catch block shows an error toast even though the server-side save already succeeded, misleading the user into retrying a no-op.fileTitlehardcodes'compose.yml'for root compose files. The actual filename (available fromcomposeInfo.configFiles) could bedocker-compose.yml,docker-compose.yaml, etc. The banner text would show an inaccurate filename in those cases.Confidence Score: 4/5
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Container Page Load] --> B{composeInfo present?} B -->|No| C[No Compose Tab] B -->|Yes| D["Search projects by name\n(limit: 100)"] D --> E{Exact name match?} E -->|No| C E -->|Yes| F[Fetch full project detail] F --> G[Parse root composeContent YAML] G --> H{Service defined in root?} H -->|Yes| I["serviceComposeSource =\n{ includeFile: null }"] H -->|No| J[Iterate includeFiles] J --> K{Service in include file?} K -->|Yes| L["serviceComposeSource =\n{ includeFile: f }"] K -->|No - exhausted| C I --> M["Compose Tab shown\n{#key project.id + 'root'}"] L --> N["Compose Tab shown\n{#key project.id + relativePath}"] M --> O{GitOps managed?} N --> O O -->|Yes| P[Read-only editor\nViewing banner] O -->|No| Q[Editable editor\nEditing banner] Q --> R[User edits → isDirty=true\nSave button enabled] R --> S[handleSave] S --> T{includeFile?} T -->|Yes| U[updateProjectIncludeFile] T -->|No| V[updateProject] U --> W[invalidateAll → isDirty=false] V --> WPrompt To Fix All With AI
Last reviewed commit: c844b33