fix(compose): reattach existing projects without fetching features

CHANGELOG.md

@@ -7,6 +7,60 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.5.2] - 2026-10-28
+
+### Fixed
+
+- **compose** — `Up` on a compose-source devcontainer whose containers
+  already exist no longer fetches every feature from its registry and
+  rebuilds the final image. The primary container now carries a
+  `dev.containers.compose-inputs-hash` label (`LabelComposeInputsHash`):
+  a digest of the container-shaping part of the resolved
+  `devcontainer.json` (features as requested, with options; local feature
+  content), the loaded compose project, the primary service's base image
+  ID and the engine options that reshape the project. When it matches,
+  `Up` reattaches as the image path's `attachExisting` does: it reads the
+  merged metadata from the existing image's `devcontainer.metadata` label
+  (entrypoint wrapper, mounts including the v0.5.1 volume/tmpfs fix, env,
+  privileged, capAdd, lifecycle commands) and starts the existing services
+  from their existing images — native under the caller's `AdoptExisting`,
+  shell-out with `up --no-recreate`. A restart therefore needs no
+  registry: before, a docker-in-docker compose workspace whose host had no
+  route to ghcr.io failed every start after a stop. Any mismatch (changed
+  config or compose file, new base image, a moved image tag, `Recreate`,
+  `PullAlways`) takes the full path as before. Upgrade note: containers
+  created by an earlier version have no label, so they take the full path.
+  The label is part of the primary service's config, so the native backend
+  without `AdoptExisting` recreates such a container on its first `Up`
+  after upgrading (and labels it); with `AdoptExisting`, or on the
+  shell-out backend, the container is kept unlabelled and keeps taking the
+  full path until it is recreated.
+- **features** — the `devcontainer.metadata` label written into feature
+  images kept `$` unescaped, so the Dockerfile parser expanded
+  `${devcontainerId}` to nothing: docker-in-docker's
+  `dind-var-lib-docker-${devcontainerId}` mount was recorded as
+  `dind-var-lib-docker-`. Any later read of the label (reattach, or the
+  image used as a base) lost the per-workspace volume name. The label is
+  now written literally, so a feature image whose metadata contains `$` gets
+  a new digest when it is next built.
+- **features** — OCI feature lookups are local-first where the answer
+  cannot change: a digest reference, or an exact `X.Y.Z` version tag
+  resolved before, is served from the disk cache without contacting the
+  registry. Floating tags (`2`, `2.17`, `latest`) are still resolved
+  online; when the registry is unreachable (no HTTP response), the last
+  recorded resolution is used if cached. A registry that answers with an
+  error is authoritative. Extraction is now atomic (temporary directory,
+  then rename). Policy: `design/features.md` §8.1.
+
+### Added
+
+- `LabelComposeInputsHash` (`dev.containers.compose-inputs-hash`), stamped
+  on the primary container of a compose-source devcontainer.
+- `runtime.ContainerDetails.ImageRef`: the image reference a container was
+  created from (docker's `Config.Image`).
+- `feature.DiskStoreOptions.OCITransport`: the HTTP transport for OCI
+  registry calls.
+
 ## [0.5.1] - 2026-10-27
 
 ### Fixed
@@ -613,7 +667,8 @@ shelling out to `@devcontainers/cli`.
 - `events` is doc-tagged **experimental** until v1.0.0 — type shapes may evolve
   without a SemVer-major bump.
 
-[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...HEAD
+[Unreleased]: https://github.com/crunchloop/devcontainer/compare/v0.5.2...HEAD
+[0.5.2]: https://github.com/crunchloop/devcontainer/compare/v0.5.1...v0.5.2
 [0.5.1]: https://github.com/crunchloop/devcontainer/compare/v0.5.0...v0.5.1
 [0.5.0]: https://github.com/crunchloop/devcontainer/compare/v0.4.3...v0.5.0
 [0.4.3]: https://github.com/crunchloop/devcontainer/compare/v0.4.2...v0.4.3

README.md

@@ -50,7 +50,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not
 | --- | --- | --- |
 | `image` | ✅ | Pull, run, exec |
 | `build` (`dockerfile`, `context`, `args`, `target`, `cacheFrom`) | ✅ | User Dockerfile + features layered atop |
-| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. |
+| `dockerComposeFile`, `service`, `runServices` | ✅ | compose-go parse + either shell-out to `docker compose` (default) or an in-process orchestrator (`EngineOptions.ComposeBackend = ComposeBackendNative`). The native orchestrator drives any `Runtime` implementing the compose primitives. An `Up` whose containers exist from unchanged inputs restarts them from their existing images, with the feature metadata read from the image label: no feature fetch, no rebuild. |
 
 **Container config**
 
@@ -69,7 +69,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not
 
 | Field | Status | Notes |
 | --- | --- | --- |
-| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache |
+| `features` (OCI / HTTPS / local) | ✅ | DAG ordering, options validation, content-addressed cache (local-first for digests and exact `X.Y.Z` versions; floating tags fall back to the last resolution when the registry is unreachable) |
 | `dependsOn` / `installsAfter` / `overrideFeatureInstallOrder` | ✅ | |
 | Pre-baked-image short-circuit | ✅ | `devcontainer.metadata` label read on Up |
 | `devcontainer-lock.json` | ❌ | [#26](https://github.com/crunchloop/devcontainer/issues/26) |

design/compose-native.md

@@ -399,6 +399,59 @@ Input: DownPlan { ProjectName string, RemoveImages bool, RemoveVolumes bool }
   service that didn't become healthy. The dependent services are not
   started.
 
+### 5.4 Engine-level reattach (both backends)
+
+The orchestrator's config-hash reuse (§5.1) decides per container, but
+it runs after the engine has already prepared the primary image: before
+v0.5.2 every compose `Up` fetched every feature from its registry and
+rebuilt `dc-go-final-<id>`, even when the project's containers already
+existed. A restart therefore needed the feature registry, and failed
+without it. The image path never had this cost: `attachExisting` reads
+the merged metadata from the existing container's image label.
+
+`Engine.Up` now does the same for compose sources (`reattachCompose`):
+
+1. The full path stamps `dev.containers.compose-inputs-hash` on the
+   primary container. The hash covers the inputs fixed before any
+   feature is fetched: the container-shaping part of the resolved
+   `devcontainer.json` (workspace and mounts, users,
+   `updateRemoteUserUID`, `containerEnv`, `runArgs`, init, privileged,
+   capAdd, securityOpt, overrideCommand, and the features as requested
+   with their options, plus a content digest of each local feature),
+   the loaded compose project (services, networks, volumes, secrets,
+   configs), the local image ID of the primary service's base image,
+   the engine options that reshape the project
+   (`ComposeBackend`, `DisableHostPortPublishing`,
+   `StrictFeatureVersionMatch`) and the workspace folder's owner.
+   Lifecycle commands, `remoteEnv`, ports attributes and
+   customizations are left out: `Up` re-reads them every call, and they
+   never reach the container or its image.
+2. On a later `Up` with an existing container (and neither `Recreate`,
+   `PullAlways` nor an `Engine.Build` override), the engine finds the
+   primary container by project, service and devcontainer id, and
+   reattaches when its label equals the freshly computed hash and the
+   image tag it was created from still names the image it runs.
+3. Reattach parses the image's `devcontainer.metadata` label, merges it
+   into the resolved config exactly as `attachExisting` does, rebuilds
+   the run override (entrypoint wrapper, every mount type, env,
+   privileged, capAdd, …) and brings the project up through the
+   selected backend against the existing images: native under the
+   caller's `AdoptExisting`, skipping sidecar builds whose image is
+   present; shell-out with `up -d --no-recreate`. Because the image is
+   the same, the orchestrator's per-service config hash matches too, so
+   a native `Up` without `AdoptExisting` reuses the containers instead
+   of recreating them.
+4. Any other case — no container, no label (containers created before
+   v0.5.2), a different hash, a moved tag — takes the full path,
+   unchanged. `AdoptExisting` keeps its meaning: it governs whether the
+   orchestrator may reuse a container whose config drifted; reattach
+   only removes the fetch and rebuild in front of it.
+
+Like the reference CLI starting an existing container, reattach does
+not re-resolve floating feature tags or look at the build context of a
+`build:` primary (only its definition and the base image ID); a caller
+that wants either picked up passes `Recreate`.
+
 ## 6. Mapping to `runtime.Runtime` methods
 
 Same shape as `runtime-applecontainer.md` §8.
@@ -1126,6 +1179,15 @@ Resolved during this design draft (2026-05-14):
     (upstream `#286`), the capability flag flips to true and the
     warning self-suppresses.
 
+14. **Compose `Up` reattaches without fetching features
+    (2026-10-28).** §5.4. An existing project whose stored inputs hash
+    matches is restarted from its existing images with the metadata
+    read from the primary image's label, as the image path's
+    `attachExisting` does. Motivation: a restart of a compose workspace
+    with features needed the feature registry (and paid a final-image
+    rebuild) on every start; a workspace without registry egress could
+    not restart at all.
+
 ---
 
 ## Appendix A: relationship to `compose.md`

design/features.md

@@ -314,14 +314,35 @@ features/
     sha256-<contenthash>/
       blob.tgz
       extracted/...
-  index.json                # ref → digest map with TTL hints
+    tags/
+      <sha256(ref)>.json    # {"ref": "<registry/repo:tag>", "digest": "sha256:…"}
 ```
 
 Cache key is **content-addressed**: OCI digest or HTTPS body sha256. A
-mutable tag (`:1`) resolves through `index.json` to the digest, then
-hits the disk cache. This is intentionally different from devpod's
-"hash-the-ref-string" scheme — content-addressed dedupes across mirror
-URLs and prevents stale-ref bugs.
+tag resolves to a digest, then hits the disk cache. This is
+intentionally different from devpod's "hash-the-ref-string" scheme —
+content-addressed dedupes across mirror URLs and prevents stale-ref
+bugs. Extraction goes to a temporary directory in the entry and is
+renamed into place, so a partial extraction is never taken for a hit.
+
+### 8.1 OCI resolution policy (v0.5.2)
+
+The cache is only useful offline if the registry is not asked first.
+`DiskStore` decides per reference form:
+
+| Reference | Policy |
+| --- | --- |
+| `name@sha256:…` | Local first: a cached extraction of the digest is used with no registry call. |
+| `name:X.Y.Z` (exact version) | Local first: the digest recorded under `tags/` the last time this tag was resolved is used with no registry call, if its extraction is cached. Published versions are immutable by the publishing convention (the reference CLI publishes `X.Y.Z` once and moves only `X`, `X.Y` and `latest`). |
+| `name:X`, `name:X.Y`, `name:latest`, any other tag | Online first, so a moved tag is picked up. If the registry is **unreachable** (no HTTP response: DNS, connect, TLS, timeout), the recorded resolution is used if its extraction is cached. |
+
+A registry that answers with an error (404, 401, 403, …) is
+authoritative: its error is returned even when a resolution is
+recorded. With nothing recorded or the content evicted, an unreachable
+registry fails the fetch as before. Every successful online fetch of a
+tag rewrites its record (write to a temporary file, then rename). HTTPS
+features are unchanged: their content hash is only known after the
+download.
 
 Concurrency: a small lockfile (`extracted/.lock`) per cache entry so
 parallel `Resolve` calls for the same ref don't race.

feature/disk_store.go

@@ -29,6 +29,12 @@ type DiskStoreOptions struct {
 	// fetches. Useful for Authorization on private servers.
 	HTTPSHeaders map[string]string
 
+	// OCITransport overrides the HTTP transport for OCI registry
+	// calls. Nil uses go-containerregistry's default (which honors the
+	// standard proxy environment variables). Tests use it to make the
+	// registry unreachable; callers may use it for a custom proxy.
+	OCITransport http.RoundTripper
+
 	// HTTPSClient overrides the default *http.Client used for HTTPS
 	// fetches. If nil, the package's default client (5-minute timeout)
 	// is used. Tests substitute this to drive httptest servers.
@@ -41,6 +47,7 @@ type DiskStoreOptions struct {
 type DiskStore struct {
 	cacheDir     string
 	ociKeychain  authn.Keychain
+	ociTransport http.RoundTripper
 	httpsHeaders map[string]string
 	httpsClient  *http.Client
 }
@@ -66,6 +73,7 @@ func NewDiskStore(opts DiskStoreOptions) (*DiskStore, error) {
 	return &DiskStore{
 		cacheDir:     dir,
 		ociKeychain:  kc,
+		ociTransport: opts.OCITransport,
 		httpsHeaders: opts.HTTPSHeaders,
 		httpsClient:  opts.HTTPSClient,
 	}, nil

feature/dockerfile.go

@@ -160,8 +160,7 @@ func generateDockerfile(plan BuildPlan) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("build metadata label: %w", err)
 	}
-	// Wrap the JSON in strconv-quoted form so embedded "/'/$ are unambiguous.
-	fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, strconv.Quote(string(label)))
+	fmt.Fprintf(&b, "LABEL %s=%s\n\n", MetadataLabel, dockerfileLiteral(string(label)))
 
 	user := "root"
 	if plan.RemoteUser != "" {
@@ -239,3 +238,16 @@ func copyFileContents(src, dst string, mode os.FileMode) error {
 	}
 	return out.Close()
 }
+
+// dockerfileLiteral quotes s as a double-quoted Dockerfile word that
+// evaluates to s exactly. strconv.Quote handles quotes and
+// backslashes, but the Dockerfile parser still expands $VAR and
+// ${VAR} inside double quotes, so every $ is escaped as well. Without
+// it a metadata label carrying a feature's
+// "dind-var-lib-docker-${devcontainerId}" mount was stored as
+// "dind-var-lib-docker-" (the build has no such variable), and any
+// later read of the label — a reattach, or this image as the base of
+// another build — lost the per-workspace volume name.
+func dockerfileLiteral(s string) string {
+	return strings.ReplaceAll(strconv.Quote(s), "$", `\$`)
+}

feature/dockerfile_test.go

@@ -301,3 +301,52 @@ func TestParseMetadataLabel_Empty(t *testing.T) {
 		t.Errorf("got %v, want nil", got)
 	}
 }
+
+// The metadata LABEL must carry ${...} literally: the Dockerfile parser
+// expands variables inside double quotes, so an unescaped
+// "dind-var-lib-docker-${devcontainerId}" mount source was stored as
+// "dind-var-lib-docker-" and every later read of the label lost the
+// per-workspace volume name.
+func TestGenerateBuildContext_LabelKeepsDollarLiteral(t *testing.T) {
+	f := fixtureFeature(t, "dind", "2.0")
+	f.Metadata.Mounts = []config.Mount{{
+		Type:   config.MountVolume,
+		Source: "dind-var-lib-docker-${devcontainerId}",
+		Target: "/var/lib/docker",
+	}}
+	dst := t.TempDir()
+	if err := GenerateBuildContext(BuildPlan{BaseImage: "alpine:3.20", Features: []config.ResolvedFeature{f}}, dst); err != nil {
+		t.Fatalf("GenerateBuildContext: %v", err)
+	}
+	df, err := os.ReadFile(filepath.Join(dst, "Dockerfile"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	var label string
+	for _, line := range strings.Split(string(df), "\n") {
+		if strings.HasPrefix(line, "LABEL "+MetadataLabel+"=") {
+			label = line
+		}
+	}
+	if label == "" {
+		t.Fatalf("no metadata LABEL line in:\n%s", df)
+	}
+	if strings.Count(label, "$") != strings.Count(label, `\$`) {
+		t.Errorf("LABEL has an unescaped $: %s", label)
+	}
+	if !strings.Contains(label, `dind-var-lib-docker-\${devcontainerId}`) {
+		t.Errorf("LABEL lost the mount source variable: %s", label)
+	}
+}
+
+func TestDockerfileLiteral(t *testing.T) {
+	for in, want := range map[string]string{
+		`plain`:            `"plain"`,
+		`a${b}c`:           `"a\${b}c"`,
+		`$HOME and "q" \x`: `"\$HOME and \"q\" \\x"`,
+	} {
+		if got := dockerfileLiteral(in); got != want {
+			t.Errorf("dockerfileLiteral(%q) = %s, want %s", in, got, want)
+		}
+	}
+}