fix compose feature image tagging#655
Merged
skevetter merged 9 commits intoskevetter:mainfrom Apr 6, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 18 minutes and 18 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughCompute a workspace-specific build image name for Compose services when features are applied, thread that name through compose build/service creation to avoid mutating the original compose Changes
Sequence Diagram(s)sequenceDiagram
participant DevPod as DevPod CLI
participant NameGen as composeBuildImageName
participant Builder as Docker Engine
participant ComposeSvc as Generated Compose Service
DevPod->>NameGen: Request build image name (composeService, hasFeatures)
NameGen-->>DevPod: return workspace-specific buildImageName
DevPod->>Builder: Build feature image FROM original-image tag=buildImageName
Builder-->>DevPod: build result (image ID)
DevPod->>ComposeSvc: set service.Image = buildImageName (leave original image tag unchanged)
ComposeSvc-->>DevPod: compose up uses derived image
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
YassineElbouchaibi
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes a Compose + Features bug where image-backed services would retag the original image: (a shared local tag), causing feature “leakage” across projects. The fix introduces a workspace-local derived image name for feature-extended builds while keeping the generated feature Dockerfile FROM pinned to the original Compose image.
Changes:
- Derive a workspace-scoped output image name for image-backed Compose services when Features are applied, instead of mutating the shared
image:tag. - Thread the computed build image name through the generated feature-build Compose service.
- Add unit and e2e regression coverage to ensure shared tags are not retagged across projects.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/devcontainer/compose.go | Computes a derived image name for image-backed + Features and uses it as the build output tag in the generated Compose override. |
| pkg/devcontainer/compose_test.go | Adds unit tests for image name selection and ensuring generated compose build service uses the provided build image name. |
| e2e/tests/up-docker-compose/up_docker_compose.go | Adds an end-to-end regression ensuring two projects sharing the same base tag don’t leak Features into each other. |
| e2e/tests/up-docker-compose/helper.go | Adds helpers to inspect/reset image tags and makes workspace testdata paths absolute-relative to the initial working directory. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.json | New e2e fixture: project A applies github-cli feature on a shared base image tag. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.yaml | New e2e fixture Compose file referencing the shared base image tag. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.override.yaml | New e2e fixture override env for project A. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.json | New e2e fixture: project B applies node feature on the same shared base image tag. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.yaml | New e2e fixture Compose file referencing the shared base image tag. |
| e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.override.yaml | New e2e fixture override env for project B. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
e2e/tests/up-docker-compose/up_docker_compose.go (1)
230-233: Make the negative checks prove tool absence, not just “some error”.These assertions pass on any
docker run/SSH failure, so the regression can go green even if the workspace is simply unhealthy. Prefer checkingcommand -v gh/command -v nodein a known-good shell, or assert that stderr matches the expected “not found” failure.Also applies to: 247-251, 258-262
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/up-docker-compose/up_docker_compose.go` around lines 230 - 233, Replace the broad error assertion that any docker/SSH failure counts as tool absence by running a known-good shell inside the container and checking for the tool explicitly: instead of exec.CommandContext(ctx, "docker", "run", "--rm", sharedImage, "gh", "--version") and asserting err != nil on hostGhCmd/hostGhOut, run docker run --rm sharedImage sh -lc "command -v gh" (and similarly for node in the other blocks referenced) and assert the command exits non‑zero or returns an empty stdout (or assert stderr contains the expected “not found” text) so the test proves gh/node are truly missing rather than any other docker failure; update the assertions at the other locations (lines ~247-251 and ~258-262) to follow the same pattern and reference their command variables accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/tests/up-docker-compose/up_docker_compose.go`:
- Around line 188-195: The test uses a moving tag for the baseline image
(constant sourceImage in up_docker_compose.go), causing non-deterministic
failures; update the sourceImage constant to a specific immutable image digest
(replace "mcr.microsoft.com/devcontainers/base:ubuntu-24.04" with the
corresponding "@sha256:..." digest) so the test always uses the exact image
expected by the negative assertions, leaving other constants like sharedImage
and projectAPath/projectBPath unchanged.
---
Nitpick comments:
In `@e2e/tests/up-docker-compose/up_docker_compose.go`:
- Around line 230-233: Replace the broad error assertion that any docker/SSH
failure counts as tool absence by running a known-good shell inside the
container and checking for the tool explicitly: instead of
exec.CommandContext(ctx, "docker", "run", "--rm", sharedImage, "gh",
"--version") and asserting err != nil on hostGhCmd/hostGhOut, run docker run
--rm sharedImage sh -lc "command -v gh" (and similarly for node in the other
blocks referenced) and assert the command exits non‑zero or returns an empty
stdout (or assert stderr contains the expected “not found” text) so the test
proves gh/node are truly missing rather than any other docker failure; update
the assertions at the other locations (lines ~247-251 and ~258-262) to follow
the same pattern and reference their command variables accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 92cdb157-47eb-4cec-b0ed-d78355662e58
📒 Files selected for processing (10)
e2e/tests/up-docker-compose/helper.goe2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.override.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.override.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/up_docker_compose.gopkg/devcontainer/compose.gopkg/devcontainer/compose_test.go
YassineElbouchaibi
force-pushed
the
codex/compose-feature-derived-image-654
branch
from
11ed196 to
05949b0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/tests/up-docker-compose/helper.go (1)
59-80: Consider guardingsourceImage == targetImagein reset helper.Defensive early-return avoids self-untagging when caller passes the same value for source and target.
Suggested hardening
func (btc *baseTestContext) resetTaggedImage( ctx context.Context, sourceImage, targetImage string, ) error { if err := btc.dockerHelper.Pull(ctx, sourceImage, nil, io.Discard, io.Discard); err != nil { return err } + if sourceImage == targetImage { + return nil + } _ = btc.dockerHelper.Run( ctx, []string{"image", "rm", "-f", targetImage}, nil, io.Discard,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/tests/up-docker-compose/helper.go` around lines 59 - 80, The resetTaggedImage helper should guard against sourceImage == targetImage to avoid removing the image when re-tagging to the same name: inside resetTaggedImage (method on baseTestContext) add an early-return (nil) when sourceImage equals targetImage before calling dockerHelper.Pull or running image rm/tag; this prevents the image removal Run call (dockerHelper.Run with ["image","rm","-f", targetImage]) from accidentally untagging/self-deleting the same image.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/tests/up-docker-compose/helper.go`:
- Around line 59-80: The resetTaggedImage helper should guard against
sourceImage == targetImage to avoid removing the image when re-tagging to the
same name: inside resetTaggedImage (method on baseTestContext) add an
early-return (nil) when sourceImage equals targetImage before calling
dockerHelper.Pull or running image rm/tag; this prevents the image removal Run
call (dockerHelper.Run with ["image","rm","-f", targetImage]) from accidentally
untagging/self-deleting the same image.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5cadcce5-69ba-4e4e-88d5-7b793ff7dfab
📒 Files selected for processing (10)
e2e/tests/up-docker-compose/helper.goe2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.override.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.override.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.yamle2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/up_docker_compose.gopkg/devcontainer/compose.gopkg/devcontainer/compose_test.go
✅ Files skipped from review due to trivial changes (8)
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.yaml
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.override.yaml
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.json
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/compose.yaml
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.json
- pkg/devcontainer/compose_test.go
- e2e/tests/up-docker-compose/up_docker_compose.go
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/compose.override.yaml
Move TestStripDigestFromImageRef, TestComposeBuildImageName, and TestCreateComposeServiceUsesBuildImageName into a ComposeSuite using stretchr/testify suite+assert, matching the PrepareBuildContextSuite pattern already in this file.
Pin github-cli to 1.1 and node to 1.7 in the shared-image e2e test fixtures to prevent flaky failures from floating major-version refs resolving to untested new minor versions.
- Call composeBuildImageName once after determining hasFeatures, instead of calling it twice (first with false, then overwriting with true). - Sanitize composeService.Image by stripping newlines before embedding it in the synthetic Dockerfile FROM line. - Return an explicit error when a compose service has neither image nor build configuration, preventing an invalid empty FROM. - Add test cases documenting image+build+features behavior.
Replace multi-value returns with result structs for four functions that exceed the revive 3-return-value limit: - dockerComposeProjectFiles -> composeProjectFiles - prepareComposeBuildInfo -> composeBuildInfo - buildAndExtendDockerCompose -> composeExtendResult - checkForPersistedFile -> persistedFileResult Also remove the unused error return from checkForPersistedFile (flagged by unparam, as it never returned a non-nil error).
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/devcontainer/build.go (1)
509-516:⚠️ Potential issue | 🟠 MajorPotential issue:
BuildInfo.ImageNamemay be empty whenbuildImageNameis empty.When
extendResult.buildImageNameis empty,currentImageNameis correctly set tooriginalImageName(lines 491-494), butBuildInfo.ImageNameon line 512 would still be empty. Based on the relevant code snippets,buildInfo.ImageNameis used inprebuild.gofor tagging and insingle.gofor run options, which could cause issues.🐛 Proposed fix: use currentImageName for BuildInfo.ImageName
return &config.BuildInfo{ ImageDetails: imageDetails, ImageMetadata: extendResult.imageMetadata, - ImageName: extendResult.buildImageName, + ImageName: currentImageName, PrebuildHash: imageTag, RegistryCache: options.RegistryCache, Tags: options.Tag, }, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/devcontainer/build.go` around lines 509 - 516, The returned config.BuildInfo currently uses extendResult.buildImageName for ImageName which can be empty; change it to use the resolved currentImageName (the value chosen between extendResult.buildImageName and originalImageName) so ImageName is always populated—update the BuildInfo creation in the function that returns &config.BuildInfo{...} to set ImageName: currentImageName instead of extendResult.buildImageName.
🧹 Nitpick comments (1)
pkg/devcontainer/compose.go (1)
672-677: Sanitization handles newline injection, but consider additional validation.The newline/carriage return stripping prevents basic Dockerfile injection. However, for defense in depth, consider validating that the image reference conforms to expected Docker image reference format (e.g., no shell metacharacters, spaces, or other unexpected characters).
💡 Optional: Use a regex to validate image reference format
- sanitizedImage := strings.ReplaceAll( - strings.ReplaceAll(composeService.Image, "\n", ""), - "\r", - "", - ) + // Validate and sanitize image reference + sanitizedImage := strings.TrimSpace(strings.ReplaceAll( + strings.ReplaceAll(composeService.Image, "\n", ""), + "\r", + "", + )) + if sanitizedImage == "" { + return composeExtendResult{}, fmt.Errorf( + "compose service %q has empty image after sanitization", + composeService.Name, + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/devcontainer/compose.go` around lines 672 - 677, The code currently strips newlines from composeService.Image and writes it into dockerfileContents using buildTarget, but it lacks stricter validation; add an image-reference validation step after computing sanitizedImage (before using it in dockerfileContents) that rejects or errors on values containing spaces, shell metacharacters (e.g., ; && | ` $ <>), control characters, or that do not match a Docker image reference pattern (e.g., optional domain[:port]/name[:tag|@digest]); update the function that constructs dockerfileContents to call this validator (referencing sanitizedImage, dockerfileContents, and buildTarget) and return an error if validation fails so only safe image references are embedded into the generated Dockerfile.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/devcontainer/compose.go`:
- Around line 1177-1195: In checkForPersistedFile(files []string, prefix string)
change the prefix comparison to operate on the filename, not the full path: use
filepath.Base(file) instead of file when calling strings.HasPrefix so checks
like strings.HasPrefix(filepath.Base(file), prefix) will correctly match entries
in the configFiles list; keep the rest of the logic (os.Stat, os.IsNotExist, and
returned persistedFileResult) unchanged.
---
Outside diff comments:
In `@pkg/devcontainer/build.go`:
- Around line 509-516: The returned config.BuildInfo currently uses
extendResult.buildImageName for ImageName which can be empty; change it to use
the resolved currentImageName (the value chosen between
extendResult.buildImageName and originalImageName) so ImageName is always
populated—update the BuildInfo creation in the function that returns
&config.BuildInfo{...} to set ImageName: currentImageName instead of
extendResult.buildImageName.
---
Nitpick comments:
In `@pkg/devcontainer/compose.go`:
- Around line 672-677: The code currently strips newlines from
composeService.Image and writes it into dockerfileContents using buildTarget,
but it lacks stricter validation; add an image-reference validation step after
computing sanitizedImage (before using it in dockerfileContents) that rejects or
errors on values containing spaces, shell metacharacters (e.g., ; && | ` $ <>),
control characters, or that do not match a Docker image reference pattern (e.g.,
optional domain[:port]/name[:tag|@digest]); update the function that constructs
dockerfileContents to call this validator (referencing sanitizedImage,
dockerfileContents, and buildTarget) and return an error if validation fails so
only safe image references are embedded into the generated Dockerfile.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3f87ab3-865f-446d-8a1e-cba2e3bf8770
📒 Files selected for processing (5)
e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.jsonpkg/devcontainer/build.gopkg/devcontainer/compose.gopkg/devcontainer/compose_test.go
✅ Files skipped from review due to trivial changes (1)
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.json
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.json
The config_files label stores absolute paths, but the prefix check compared against the full path, so it never matched the filename prefix (e.g. "docker-compose.devcontainer.build").
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/devcontainer/compose.go (2)
565-606:⚠️ Potential issue | 🔴 CriticalPreserve the original Dockerfile when
build.targetis already set.On the
originalTarget != ""path,dockerfileContentsstays empty. That sends feature builds into the fallback at Lines 666-677, which synthesizesFROM <composeService.Image> AS <target>. For a valid compose service that hasbuild.targetbut noimage, that becomesFROM AS ...and the feature build fails.Suggested fix
originalTarget := composeService.Build.Target if originalTarget != "" { buildTarget = originalTarget + dockerfileContents = string(originalDockerfile) } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/devcontainer/compose.go` around lines 565 - 606, When composeService.Build.Target (originalTarget) is non-empty we currently set buildTarget but leave dockerfileContents empty, which later causes a synthesized invalid FROM line; fix this by preserving the original Dockerfile in that branch: set dockerfileContents = string(originalDockerfile) when originalTarget != "" so the subsequent call to getImageBuildInfoFromDockerfile and the returned composeBuildInfo have the actual Dockerfile; ensure you still set buildTarget = originalTarget and keep the existing getImageBuildInfoFromDockerfile call parameters (subCtx, string(originalDockerfile), mappingToMap(composeService.Build.Args), originalTarget).
448-488:⚠️ Potential issue | 🟠 MajorNormalize digest-backed refs before inspect/up.
createComposeServicestrips digests at Line 913 before assigningservice.Image, but this block still carriesextendResult.buildImageNameforward verbatim. Withimage: repo/app@sha256:..., the feature build is tagged asrepo/appwhile Lines 453 and 487 still inspect/referencerepo/app@sha256:..., so the post-build metadata lookup and compose override can target the wrong image.Suggested fix
- currentImageName := extendResult.buildImageName + overrideImageName := extendResult.buildImageName + if extendResult.composeBuildFilePath != "" { + overrideImageName = stripDigestFromImageRef(overrideImageName) + } + currentImageName := overrideImageName if currentImageName == "" { currentImageName = originalImageName } @@ - extendResult.buildImageName, + overrideImageName,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/devcontainer/compose.go` around lines 448 - 488, The code is still using digest-backed refs (extendResult.buildImageName and originalImageName) when calling r.inspectImage and r.extendedDockerComposeUp which causes mismatched lookups; update this block to normalize/strip any digest from extendResult.buildImageName and originalImageName (the same normalization logic used in createComposeService for service.Image) before passing them to r.inspectImage and r.extendedDockerComposeUp so both the inspectImage call and the compose override use the digest-stripped image name; ensure you apply the normalization to the variables used in the imageDetails lookup and in the call to r.extendedDockerComposeUp (references: extendResult.buildImageName, originalImageName, r.inspectImage, r.extendedDockerComposeUp, createComposeService).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@pkg/devcontainer/compose.go`:
- Around line 565-606: When composeService.Build.Target (originalTarget) is
non-empty we currently set buildTarget but leave dockerfileContents empty, which
later causes a synthesized invalid FROM line; fix this by preserving the
original Dockerfile in that branch: set dockerfileContents =
string(originalDockerfile) when originalTarget != "" so the subsequent call to
getImageBuildInfoFromDockerfile and the returned composeBuildInfo have the
actual Dockerfile; ensure you still set buildTarget = originalTarget and keep
the existing getImageBuildInfoFromDockerfile call parameters (subCtx,
string(originalDockerfile), mappingToMap(composeService.Build.Args),
originalTarget).
- Around line 448-488: The code is still using digest-backed refs
(extendResult.buildImageName and originalImageName) when calling r.inspectImage
and r.extendedDockerComposeUp which causes mismatched lookups; update this block
to normalize/strip any digest from extendResult.buildImageName and
originalImageName (the same normalization logic used in createComposeService for
service.Image) before passing them to r.inspectImage and
r.extendedDockerComposeUp so both the inspectImage call and the compose override
use the digest-stripped image name; ensure you apply the normalization to the
variables used in the imageDetails lookup and in the call to
r.extendedDockerComposeUp (references: extendResult.buildImageName,
originalImageName, r.inspectImage, r.extendedDockerComposeUp,
createComposeService).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 763fb83b-e511-4677-82db-3f0d47ec3053
📒 Files selected for processing (5)
e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.jsone2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.jsonpkg/devcontainer/build.gopkg/devcontainer/compose.gopkg/devcontainer/compose_test.go
✅ Files skipped from review due to trivial changes (2)
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-b/.devcontainer/devcontainer.json
- pkg/devcontainer/build.go
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/tests/up-docker-compose/testdata/docker-compose-features-shared-image-a/.devcontainer/devcontainer.json
skevetter
approved these changes
Apr 6, 2026
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
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.
Summary
This isolates Compose feature builds from shared image tags when a service is image-backed.
Fixes #654
Public repro: https://github.com/YassineElbouchaibi/devpod-compose-image-stain-issue-654
What changed
FROMpointed at the original Composeimage:Why
The Compose feature extension path was reusing the original service
image:as the output image name. For image-backed services, that retagged the shared base image locally after Features were layered on top of it. Any other project using the same base image tag could then inherit the wrong tools and packages.This mirrors the image-backed Compose behavior used by
devcontainers/cli, which avoids retagging a previously pulled image and instead runs the workspace from a derived local image.Validation
go test ./pkg/devcontainer/...go run github.com/onsi/ginkgo/v2/ginkgo@latest --focus='does not retag shared image when applying features to image backed services' ./e2eMade with Codex + GPT-5.4.
Summary by CodeRabbit
Tests
Chores