Skip to content

feat(devcontainer): add devcontainer-lock.json lockfile support#724

Merged
skevetter merged 6 commits into
mainfrom
wealthy-bullfrog
Jul 24, 2026
Merged

feat(devcontainer): add devcontainer-lock.json lockfile support#724
skevetter merged 6 commits into
mainfrom
wealthy-bullfrog

Conversation

@skevetter

@skevetter skevetter commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds genuine devcontainer-lock.json lockfile support to devsy's native Go feature resolver, matching the reference devcontainer CLI behavior (previously devsy had only a per-feature local SHA-256 cache sidecar, no committed lockfile).

  • Format: { "features": { "<userFeatureId|tarballUri>": { "version", "resolved", "integrity", "dependsOn"? } } }. Filename is .devcontainer-lock.json when the config basename starts with a dot, otherwise devcontainer-lock.json, written in the config's directory.
  • Pinning: OCI features are pinned by manifest digest (registry/repo@sha256:…) and direct-tarball features by sha256: of the tarball. An existing lockfile is loaded to pin resolution and verify integrity; local (./) features are excluded.
  • Auto-write: the lockfile is generated/updated at the end of build & up resolution, only when the normalized content changed. Read-only paths (ResolveFeatureOrder) load/pin but never write.
  • --frozen-lockfile (on workspace build and workspace up): fails instead of writing — "lockfile does not exist" if missing, "lockfile does not match" if stale. Includes an early fail-fast precondition so CI doesn't download features before rejecting.

Implementation

  • New pkg/devcontainer/feature/lockfile.go (types, path resolution, read/write with frozen semantics, lockfileState for pin/record/commit).
  • features.go: OCI resolution now returns resolved digest + integrity (with an oci-digest sidecar for cache hits); tarball resolution returns/verifies integrity.
  • extend.go: threads a lockfile through feature resolution and records entries.
  • Frozen flag wired end-to-end via CLIOptions.FrozenLockfile → build/up (including compose) → GetExtendedBuildInfo.

Tests

  • Unit tests for lockfile path/read/write, frozen missing/mismatch/match, pin/record/commit, and the frozen precondition.
  • e2e specs under the up-features label: auto-writes a lockfile then reuses it under --frozen-lockfile, and fails --frozen-lockfile when no lockfile exists.

Independent code review completed; go build ./..., unit tests, and golangci-lint --new-from-rev origin/main are clean.

Summary by CodeRabbit

  • New Features
    • Added dev container feature lockfile support, including automatic generation and integrity pinning.
    • Added --frozen-lockfile to require an existing, matching devcontainer-lock.json.
    • Added --no-lockfile to disable lockfile generation and verification.
    • Made --frozen-lockfile and --no-lockfile mutually exclusive for relevant commands.
  • Bug Fixes
    • Builds now fail when --frozen-lockfile is used with a missing, mismatched, or invalidly pinned lockfile.

Adds genuine lockfile support to the native Go feature resolver, matching
the reference devcontainer CLI:

- Generates/updates .devcontainer-lock.json (or devcontainer-lock.json) next
  to the config during build/up, pinning OCI features by manifest digest and
  direct-tarball features by sha256, with a version/resolved/integrity entry
  per feature (dependencies included).
- Loads an existing lockfile to pin resolution and verify integrity.
- Adds --frozen-lockfile (build and up) which fails instead of writing when
  the lockfile is missing or does not match, with an early fail-fast check.

Includes unit tests for the lockfile logic and e2e coverage under the
up-features label (auto-write + frozen reuse, and frozen-missing failure).
@netlify

netlify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 05edc8a
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a624ba2f469a90008ce522c

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@skevetter, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e5162140-1464-4797-8e5c-48f53b271da4

📥 Commits

Reviewing files that changed from the base of the PR and between 745a07d and 05edc8a.

📒 Files selected for processing (1)
  • .github/workflows/pr-ci.yml
📝 Walkthrough

Walkthrough

Adds --frozen-lockfile and --no-lockfile controls, propagates them through build paths, implements lockfile persistence and integrity pinning for OCI and tarball features, and adds unit, command, and end-to-end coverage.

Changes

Feature lockfile support

Layer / File(s) Summary
CLI lockfile flags
pkg/flags/names/names.go, pkg/provider/workspace.go, cmd/workspace/build.go, cmd/workspace/up/*
Defines the lockfile options, registers them for build and up commands, and enforces mutual exclusion with command tests.
Build option propagation
pkg/devcontainer/build.go, pkg/devcontainer/compose.go, pkg/devcontainer/compose_build.go
Passes frozen and disabled lockfile settings through Dockerfile and Compose feature-extension paths.
Lockfile persistence and validation
pkg/devcontainer/feature/lockfile.go, pkg/devcontainer/feature/lockfile_test.go
Adds lockfile models, path resolution, stable read/write behavior, frozen validation, pinning, recording, commit logic, and unit tests.
Pinned feature resolution
pkg/devcontainer/feature/extend.go, pkg/devcontainer/feature/features.go, pkg/devcontainer/feature/features_oci_test.go
Tracks OCI and direct-tarball resolutions, persists integrity metadata, verifies pinned digests, and records lock entries with dependencies.
End-to-end lockfile coverage
e2e/tests/up-features/*
Tests lockfile generation, frozen reuse and failures, disabled mode, invalid committed pins, and HTTP tarball integrity recording.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant BuildPipeline
  participant FeatureProcessor
  participant Lockfile
  participant FeatureSource
  CLI->>BuildPipeline: pass frozen-lockfile or no-lockfile
  BuildPipeline->>FeatureProcessor: resolve features with lockfile mode
  FeatureProcessor->>Lockfile: read pinned entries
  FeatureProcessor->>FeatureSource: resolve OCI or tarball feature
  FeatureSource-->>FeatureProcessor: resolved reference and integrity
  FeatureProcessor->>Lockfile: record and commit entries
Loading

Possibly related PRs

  • devsy-org/devsy#89: Modifies direct HTTP(S) tar feature integrity handling used by this lockfile flow.
  • devsy-org/devsy#210: Modifies the feature resolution pipeline in pkg/devcontainer/feature/extend.go.
  • devsy-org/devsy#221: Changes the build parameter plumbing used by these lockfile options.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.91% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding devcontainer-lock.json lockfile support for devcontainer builds and feature resolution.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Jul 23, 2026

Copy link
Copy Markdown

Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 05edc8a
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6a624ba2e98d4b00082fe9a4

Parse the generated devcontainer-lock.json and assert structural
invariants (exactly the declared feature, sha256-formatted integrity,
resolved == repo@integrity, semver version) instead of loose substring
checks. Add a negative spec that a committed lockfile pinning a
non-existent digest is honored and fails resolution.
Add a spec that serves a tarball feature from the in-test http server and
asserts the lockfile records the exact integrity (sha256 of the served
bytes, computed with the same hash helper devsy uses) and the tarball URL
as resolved. Deterministic exact-value check with no upstream coupling.
@skevetter
skevetter marked this pull request as ready for review July 23, 2026 04:42
…frozen-lockfile

Match the official devcontainer CLI, which creates the lockfile by default but
lets you disable it. --no-lockfile turns the feature lockfile off entirely (no
pinning, no writing); it is mutually exclusive with --frozen-lockfile, enforced
via cobra's MarkFlagsMutuallyExclusive on both build and up.

Refactors the two positional lock bools threaded through fetchFeatures into a
lockfileMode{write,frozen,disabled} struct.

Adds unit tests (flag registration, mutual exclusivity, disabled no-op commit)
and an e2e spec asserting --no-lockfile writes no devcontainer-lock.json.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@cmd/workspace/build.go`:
- Around line 113-117: Update the FrozenLockfile and NoLockfile help text in
cmd/workspace/build.go, the corresponding CLIOptions comments in
pkg/provider/workspace.go, and the up-command help in
cmd/workspace/up/up_flags.go to refer to both supported lockfile names or use
neutral wording such as “the applicable devcontainer lockfile”; keep the
existing option behavior unchanged.
🪄 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 Plus

Run ID: f040eed9-079d-4412-8793-f80f22456310

📥 Commits

Reviewing files that changed from the base of the PR and between 386ba58 and 44faf50.

📒 Files selected for processing (19)
  • cmd/workspace/build.go
  • cmd/workspace/build_test.go
  • cmd/workspace/up/up.go
  • cmd/workspace/up/up_flags.go
  • cmd/workspace/up/up_test.go
  • e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer-lock.json
  • e2e/tests/up-features/testdata/docker-features-lockfile-bad/.devcontainer.json
  • e2e/tests/up-features/testdata/docker-features-lockfile/.devcontainer.json
  • e2e/tests/up-features/up_features.go
  • pkg/devcontainer/build.go
  • pkg/devcontainer/compose.go
  • pkg/devcontainer/compose_build.go
  • pkg/devcontainer/feature/extend.go
  • pkg/devcontainer/feature/features.go
  • pkg/devcontainer/feature/features_oci_test.go
  • pkg/devcontainer/feature/lockfile.go
  • pkg/devcontainer/feature/lockfile_test.go
  • pkg/flags/names/names.go
  • pkg/provider/workspace.go

Comment thread cmd/workspace/build.go
Comment on lines +113 to +117
cliflags.Bool(&cmd.FrozenLockfile, names.FrozenLockfile, false,
"Fail if devcontainer-lock.json is missing or does not match the resolved features "+
"instead of writing it (useful for CI)"),
cliflags.Bool(&cmd.NoLockfile, names.NoLockfile, false,
"Disable devcontainer-lock.json generation and verification"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Document both supported lockfile filenames consistently.

The lockfile is named .devcontainer-lock.json for .devcontainer.json configurations, so naming only devcontainer-lock.json can mislead users preparing frozen builds.

  • cmd/workspace/build.go#L113-L117: update build help to mention both names or say “the applicable devcontainer lockfile.”
  • pkg/provider/workspace.go#L294-L300: update the CLIOptions comments with the same neutral/both-name wording.
  • cmd/workspace/up/up_flags.go#L73-L77: update up-command help with the same wording.
📍 Affects 3 files
  • cmd/workspace/build.go#L113-L117 (this comment)
  • pkg/provider/workspace.go#L294-L300
  • cmd/workspace/up/up_flags.go#L73-L77
🤖 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 `@cmd/workspace/build.go` around lines 113 - 117, Update the FrozenLockfile and
NoLockfile help text in cmd/workspace/build.go, the corresponding CLIOptions
comments in pkg/provider/workspace.go, and the up-command help in
cmd/workspace/up/up_flags.go to refer to both supported lockfile names or use
neutral wording such as “the applicable devcontainer lockfile”; keep the
existing option behavior unchanged.

Signed-off-by: GitHub <noreply@github.com>
Signed-off-by: GitHub <noreply@github.com>
@skevetter
skevetter merged commit d73a28c into main Jul 24, 2026
109 of 111 checks passed
@skevetter
skevetter deleted the wealthy-bullfrog branch July 24, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant