engine: add Engine.Build for image-only builds#82
Merged
Merged
Conversation
Closes #78. Engine.Build resolves a workspace's devcontainer.json and produces the final container image (base image plus feature pipeline) without creating or running a container. Unblocks a future CLI \`build\` subcommand and matches the embedding-relevant subset of upstream \`devcontainer build\`. Scope decisions (agreed on the issue): - updateRemoteUserUID is deliberately skipped. The output is meant to be portable (push to a registry, reuse across hosts); UID reconcile bakes the calling host's UID into the image. - Compose sources are refused with a typed error — matching upstream. - Image source with no features short-circuits: pull and return the ref unchanged. ImageName is ignored in that case (the runtime has no TagImage primitive yet — tracked as a follow-up). - Single ImageName tag (not []string). Multi-tag, --push, --cache-to require BuildSpec extensions on both backends; first-pass ships what we already support. Implementation factors through the existing prepareBaseImage + layerFeatures helpers via a new unexported buildOverride threaded on UpOptions. Engine.Up sets it nil; Engine.Build populates FinalTag, BaseIsFinal (so BuildSource-no-features tags the base directly), NoCache, Platform, and ExtraCacheFrom. Integration tests cover: image-source short-circuit, build+features happy path with ImageName, build-no-features ImageName-on-base, compose refusal, and a no-container-left-behind assertion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughEngine.Build exposes a public API to build devcontainer images without creating containers: it resolves devcontainer configs, emits structured events, rejects compose sources, reuses Up’s base/feature build pipeline via an internal override, and returns the final image reference. ChangesEngine.Build for image-only builds
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/integration/build_test.go (1)
49-88: ⚡ Quick winAdd a regression test for “features requested but no feature build needed”.
Current coverage misses the path where requested features are already present in base metadata, causing
layerFeaturesto no-op. A test here should assertBuildOptions.ImageNameis still honored for build sources.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/integration/build_test.go` around lines 49 - 88, Add a regression test that covers the path where requested features are already present in the base metadata so no feature build occurs: in TestEngineBuild_BuildSource_WithFeature (or a new test named similarly) use writeBuildSourceWorkspace (or create a workspace whose devcontainer metadata lists the requested features) then call eng.Build(ctx, devcontainer.BuildOptions{LocalWorkspaceFolder: ws, ImageName: customTag}) and assert res.ImageID == customTag, then verify the image exists via rt.InspectImage(ctx, res.ImageID) and finally clean up with rt.RemoveImage; this ensures eng.Build honors BuildOptions.ImageName even when layerFeatures no-ops.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.go`:
- Around line 110-112: Replace the plain string error returned from Engine.Build
when cfg.Source is a *config.ComposeSource with a typed sentinel error (e.g.,
package-level var ErrComposeSourceNotSupported = errors.New("compose-source
devcontainers are not supported")) so callers can reliably match with errors.Is;
update the Engine.Build return to return that sentinel (and optionally wrap it
with fmt.Errorf only if adding context, using errors.Is-compatible wrapping) and
ensure the symbol names to change are Engine.Build, cfg.Source, and
config.ComposeSource.
In `@up.go`:
- Around line 895-897: The code currently overwrites a caller-provided ImageName
when layerFeatures short-circuits and returns baseImage because
opts.override.BaseIsFinal && opts.override.FinalTag unconditionally sets tag;
change the logic so FinalTag is applied only when we are not using a
short-circuited baseImage (i.e., when layerFeatures did real work) or when no
caller ImageName was supplied. Locate the layerFeatures call and the block that
sets tag using opts.override.BaseIsFinal and opts.override.FinalTag and add a
guard that preserves baseImage.ImageName / opts.override.ImageName (or only
applies FinalTag when ImageName is empty or plan.HasWork()==true) so the caller
ImageName is retained when the feature pipeline is a no-op.
---
Nitpick comments:
In `@test/integration/build_test.go`:
- Around line 49-88: Add a regression test that covers the path where requested
features are already present in the base metadata so no feature build occurs: in
TestEngineBuild_BuildSource_WithFeature (or a new test named similarly) use
writeBuildSourceWorkspace (or create a workspace whose devcontainer metadata
lists the requested features) then call eng.Build(ctx,
devcontainer.BuildOptions{LocalWorkspaceFolder: ws, ImageName: customTag}) and
assert res.ImageID == customTag, then verify the image exists via
rt.InspectImage(ctx, res.ImageID) and finally clean up with rt.RemoveImage; this
ensures eng.Build honors BuildOptions.ImageName even when layerFeatures no-ops.
🪄 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: 3f9be298-4a30-4966-8b7a-f9f77d730eb3
📒 Files selected for processing (3)
build.gotest/integration/build_test.goup.go
CodeRabbit caught two real issues: 1. Compose-source refusal used a plain string error. Replace with a package-level sentinel ErrComposeSourceUnsupported wrapped via %w, so callers can match with errors.Is. Update the integration test to assert via errors.Is. 2. ImageName was lost when the feature pipeline short-circuited — features declared but every one already pre-baked into the base image. layerFeatures returns baseImage unchanged in that case, but BaseIsFinal was false (because len(features) > 0), so the base build kept its auto tag. Same gap surfaced as the documented "ignored for pure image source" caveat. Fix: after layerFeatures, if finalImage == baseImage and the caller asked for a specific ImageName, run a thin `FROM <base>` Dockerfile build via a new retagImage helper. Covers both the pure-image-source case and the features-all-pre-baked case. The "ImageName ignored" caveat goes away. This is a workaround for the absent runtime.TagImage primitive (still tracked as a follow-up); a TagImage would let us avoid the extra build. Tests updated: - TestEngineBuild_ImageSource_NoFeatures_NoImageName — unchanged path - TestEngineBuild_ImageSource_RetagToImageName — new, exercises the retag step - TestEngineBuild_ComposeRefused — now asserts errors.Is rather than substring matching Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@build.go`:
- Around line 193-208: The short-circuit retag path calls e.runtime.BuildImage
with runtime.BuildSpec but omits propagating BuildOptions.NoCache, so builds may
reuse cache incorrectly; update the call site that constructs runtime.BuildSpec
(used in e.runtime.BuildImage) to set NoCache from the incoming BuildOptions
(e.g., pass override.NoCache or the BuildOptions instance) so the
runtime.BuildImage invocation honors NoCache; reference the BuildOptions.NoCache
flag and the runtime.BuildSpec passed into e.runtime.BuildImage to locate and
add the missing NoCache field.
🪄 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: 157042e9-efb9-4564-9bc3-b05a614f6f5e
📒 Files selected for processing (2)
build.gotest/integration/build_test.go
CodeRabbit catch on PR #82: retagImage read Platform and ExtraCacheFrom from buildOverride but dropped NoCache, so Engine.Build(..., NoCache: true) could still reuse a cached FROM-only layer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #78.
`Engine.Build` resolves a workspace's devcontainer.json and produces the final container image — base image plus the feature pipeline — without creating or running a container. Unblocks a future CLI `build` subcommand and matches the embedding-relevant subset of upstream `devcontainer build`.
API
```go
type BuildOptions struct {
LocalWorkspaceFolder string
ConfigPath string
LocalEnv map[string]string
ImageName string // single tag; ignored for pure image source
NoCache bool
PullPolicy PullPolicy
Platform string
CacheFrom []string // appended to devcontainer.json's build.cacheFrom
Events chan<- events.Event
}
type BuildResult struct {
ImageID string
}
func (e *Engine) Build(ctx context.Context, opts BuildOptions) (*BuildResult, error)
```
Scope decisions
Implementation
Factors through the existing `prepareBaseImage` + `layerFeatures` helpers via a new unexported `buildOverride` threaded on `UpOptions`. `Engine.Up` leaves it nil; `Engine.Build` populates:
Test plan
Follow-ups (won't block this PR)
build) #78)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes