config: substitute variables in feature-contributed metadata

[bilby91]

Summary

Fix #52: feature-contributed mounts (and env, lifecycle commands, …) now get host-context substitution before reaching Docker.

Previously ResolveBytes only substituted variables in the user's devcontainer.json. Fields folded in afterwards by MergeMetadata from feature / base-image metadata kept their literal \${devcontainerId}, \${localWorkspaceFolder}, \${localEnv:*} tokens and flowed straight into ContainerCreate. The docker-in-docker feature's dind-var-lib-docker-\${devcontainerId} volume mount tripped Docker's volume name validator.

MergeMetadata now takes a SubstitutionContext and resolves layer-contributed strings before folding them in. Substitution copies — input layers are not mutated, so cached feature metadata stays clean. Call sites (applyMetadataMerge in up.go, Attach) build the context from cfg + localEnv.

Test plan

• Unit: TestMergeMetadata_SubstitutesLayerStrings covers mount sources, env values, and lifecycle commands with \${devcontainerId} / \${localWorkspaceFolder} / \${localEnv:*}
• Unit: TestMergeMetadata_SubstitutionDoesNotMutateLayers pins the no-input-mutation invariant (feature metadata is often shared / cached)
• Integration (build tag integration, requires Docker): TestFeatureMountSubstitution reproduces the docker-in-docker mount shape with a local feature, asserts Up succeeds and the inspected container mount path embeds the substituted volume name

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Metadata substitution now consistently resolves placeholders like ${devcontainerId}, ${localEnv:...}, and ${localWorkspaceFolder} in mounts, env vars, and lifecycle commands so feature-provided values are correctly applied at workspace startup.

• Tests

  • Added/updated unit and integration tests to verify substitution behavior and prevent regressions.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 646581c9-7dfa-443a-80b8-a14e1bff46dc

📥 Commits

Reviewing files that changed from the base of the PR and between 9d4310e and e3afe20.

📒 Files selected for processing (3)

• config/parse_test.go
• config/resolve_test.go
• eventbus_test.go

✅ Files skipped from review due to trivial changes (3)

• eventbus_test.go
• config/resolve_test.go
• config/parse_test.go

---

📝 Walkthrough

Walkthrough

Adds placeholder substitution during MergeMetadata by accepting a SubstitutionContext, substituting layer string fields before merging, and threading localEnv through attach/up. Validated by updated unit tests and an integration test.

Changes

Feature mount variable substitution

Layer / File(s)	Summary
Substitution infrastructure in MergeMetadata config/merge_metadata.go	MergeMetadata signature updated to accept SubstitutionContext parameter; new substituteLayers, substituteLayer, and substituteLifecycleCommand helpers resolve placeholders in user/container names, env maps, mounts (source/target), capability/security slices, and lifecycle commands before merge logic proceeds.
Wiring localEnv through orchestration pipeline attach.go, up.go	localEnv is computed early from opts.LocalEnv or os.Environ() in attach.AttachWith; up.applyMetadataMerge signature updated to accept localEnv map and builds SubstitutionContext with LocalEnv field; all three call sites (attachExisting, createFresh, createFreshCompose) pass opts.LocalEnv to enable metadata merge substitution.
Unit tests for metadata merge and substitution config/merge_metadata_test.go	All existing MergeMetadata tests updated to pass empty SubstitutionContext{}; two new tests verify that ${devcontainerId} and ${localEnv:USER} placeholders are correctly substituted in mounts, environment variables, and lifecycle hook commands, and that input layer slices are not mutated.
Integration test for feature mount substitution test/integration/feature_mount_substitution_test.go	New test TestFeatureMountSubstitution reproduces issue #52: provisions a workspace with a local feature declaring a named volume using ${devcontainerId} in the mount source, starts the container, and verifies the placeholder is resolved to the expected dc-feat-vol-<workspaceID> substring with no remaining ${ markers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Feature-contributed mounts skip variable substitution — ${devcontainerId} reaches Docker literally #52: This PR implements the fix for feature mount source template substitution by adding SubstitutionContext parameter to MergeMetadata and threading LocalEnv through orchestration.

Possibly related PRs

• crunchloop/devcontainer#21: Both PRs modify the config.MergeMetadata pipeline; #21 introduced the core merge logic, and this PR extends it with placeholder substitution support via SubstitutionContext and LocalEnv threading.

Poem

🐰 A rabbit sees placeholders dance,
${devcontainerId} gets its chance,
SubstitutionContext hops through the map,
LocalEnv helps close the gap,
Mounts blossom — no ${ in sight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% 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 accurately and concisely summarizes the main change: adding variable substitution support for feature-contributed metadata fields.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/feature-mount-substitution

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

up.go (1)

263-277: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Normalize localEnv before metadata substitution in Up paths.

applyMetadataMerge currently uses opts.LocalEnv as-is. When nil, ${localEnv:*} from feature/base metadata will not resolve from host env, which conflicts with the UpOptions.LocalEnv contract (nil => use os.Environ()) and differs from AttachWith behavior.

Suggested fix

func applyMetadataMerge(cfg *config.ResolvedConfig, baseLayers []config.FeatureMetadata, localEnv map[string]string) {
+	if localEnv == nil {
+		localEnv = environAsMap(os.Environ())
+	}
 	chain := make([]config.FeatureMetadata, 0, len(baseLayers)+len(cfg.Features))
 	chain = append(chain, baseLayers...)
 	for _, f := range cfg.Features {
 		if f.AlreadyInstalled {
 			continue
 		}
 		chain = append(chain, f.Metadata)
 	}
 	subCtx := config.SubstitutionContext{
 		LocalWorkspaceFolder:     cfg.LocalWorkspaceFolder,
 		ContainerWorkspaceFolder: cfg.ContainerWorkspaceFolder,
 		DevcontainerID:           cfg.DevcontainerID,
 		LocalEnv:                 localEnv,
 	}
 	config.MergeMetadata(cfg, subCtx, chain)
 	cfg.Finalize()
}

Also applies to: 475-475, 863-879

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f31bd3dd-e0b5-473a-af10-4790707b6cba

📥 Commits

Reviewing files that changed from the base of the PR and between 6db715d and 9d4310e.

📒 Files selected for processing (5)

• attach.go
• config/merge_metadata.go
• config/merge_metadata_test.go
• test/integration/feature_mount_substitution_test.go
• up.go