fix(build): emit devcontainer.metadata label as JSON array (PR-2 of 7) - #27
Merged
Conversation
This was referenced May 25, 2026
Per devcontainers/cli#1199 (v0.86.0), the `devcontainer.metadata` image label is always a JSON array of partial-config entries, even when only a single entry is present. VS Code, Zed, and envbuilder iterate the array and merge entries in order. Writers that emit a single object break this contract. Writer changes (three sites): - crates/core/src/build/metadata.rs: DevcontainerMetadata::to_json now wraps in [...]. from_json returns Vec<Self> and accepts either form for backwards compatibility with images built by older Deacon versions. - crates/deacon/src/commands/build/mod.rs (image-reference path, ~L1272): the inline {name, image} label wrapped in [...]. - crates/deacon/src/commands/build/mod.rs (docker build path, ~L1546): the inline {configHash} label wrapped in [...]. Reader change: - crates/deacon/src/commands/read_configuration.rs (compute_merged_ configuration, ~L411): parses the label as Value, normalizes to a Vec of entries (array as-is; object wrapped in single-element vec), then deserializes each entry as a partial DevContainerConfig, applies variable substitution, and merges with the base config in declaration order. Behavior for upstream-built images (array form) is now correct; behavior for legacy Deacon-built images (object form) is preserved. Tests added (crates/core/src/build/metadata.rs): - test_to_json_emits_array: asserts label starts with '[' - test_from_json_handles_legacy_object_form: legacy single-object fixture - test_from_json_handles_array_form_with_multiple_entries: spec-form fixture - test_metadata_roundtrip: updated for new Vec return type - test_merge_labels_emits_array_label_value: end-to-end label assertion Verified locally: cargo fmt --check, cargo check, cargo clippy -D warnings, cargo nextest run --profile dev-fast --no-default-features (1930/1930 ok), cargo test --doc --workspace (130/130 ok), 38/38 merged-configuration adjacent tests pass. Known follow-up (not blocking 1.0; separate ticket): the DevcontainerMetadata struct wraps the config under a `config` subkey alongside features/ customizations/lockfile_hash. Upstream array entries are FLAT partial-config shapes. Deacon-emitted entries today are not directly merge-able as DevContainerConfig (latent issue pre-dating this PR). Flattening the struct to match upstream array-entry shape would be its own refactor. Refs: docs/ROADMAP_TO_1.0.md Tier 1 item "devcontainer.metadata label JSON array (CLI parity B.2)". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon
force-pushed
the
pr2-metadata-label-array
branch
from
May 25, 2026 18:45
8234df8 to
fd622b9
Compare
pofallon
added a commit
that referenced
this pull request
May 25, 2026
Implements the core value of `set-up` per `docs/subcommand-specs/set-up/SPEC.md`: convert an already-running container into a DevContainer by applying configuration + image metadata and executing lifecycle hooks, emitting a single-line JSON result on stdout. ## CLI surface `deacon set-up --container-id <id> [--config <path>] [--skip-post-create] [--skip-non-blocking-commands] [--remote-env NAME=VALUE]... [--include-configuration] [--include-merged-configuration] [--container-data-folder <path>]` ## What this PR includes - `--container-id` resolution + inspect validation. Missing container fails with the upstream-aligned summary `"Dev container not found."` - Optional `--config` load via `ConfigLoader::load_with_extends` (extends chain honored per CLAUDE.md). Missing path fails with `"Dev container config (<path>) not found."` - Image-metadata extraction from the container's `devcontainer.metadata` label. Tolerates BOTH the JSON-array form (PR-2 / #27) and the single-object form for older images. - Config merge: file config wins over image metadata on scalar fields (spec §4 `mergeConfiguration(config.config, imageMetadata)`). - Variable substitution for both `configuration` and `mergedConfiguration`. - Lifecycle hook execution via `ContainerLifecycle` (onCreate → updateContent → postCreate → postStart → postAttach), gated by `--skip-post-create` (skips ALL phases per spec §2) and `--skip-non-blocking-commands` (stops after the configured `waitFor`). - JSON output per spec §10: `{outcome: "success", configuration?, mergedConfiguration?}`. `containerId` is intentionally excluded (spec §16 design decision). ## Deferred to PR-6b - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` - Dotfiles installer with target-path marker (would reuse `crates/deacon/src/commands/up/dotfiles.rs`) - A second substitution pass against the live container environment (`${containerEnv:VAR}`) — current pass uses the configured `container_env`, not a live `docker exec` env probe These are spec §5 phases 3a and 3c; both marked "best-effort" with graceful fallback on failure. Splitting them out keeps PR-6a reviewable. ## Tests 14 new unit tests in `set_up::tests`: - `--remote-env` parsing (accepts `NAME=VALUE`, rejects malformed input) - `--config` loading (default when absent, error when missing path) - Image-metadata label parsing (missing → None, array form, single-object form, invalid JSON → error) - Config merging (file wins over metadata; file-only when no metadata) - Argument defaults and JSON-result shape (outcome field, optional fields) Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 227 pass - `make test-nextest-fast` → 1927 pass (no regression) Refs: issue #34 (Tier 1 progress tracker), plan `/home/vscode/.claude/plans/let-s-come-up-with-recursive-sutherland.md` (PR-6 sequencing rationale). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon
added a commit
that referenced
this pull request
May 25, 2026
Implements the core value of `set-up` per `docs/subcommand-specs/set-up/SPEC.md`: convert an already-running container into a DevContainer by applying configuration + image metadata and executing lifecycle hooks, emitting a single-line JSON result on stdout. ## CLI surface `deacon set-up --container-id <id> [--config <path>] [--skip-post-create] [--skip-non-blocking-commands] [--remote-env NAME=VALUE]... [--include-configuration] [--include-merged-configuration] [--container-data-folder <path>]` ## What this PR includes - `--container-id` resolution + inspect validation. Missing container fails with the upstream-aligned summary `"Dev container not found."` - Optional `--config` load via `ConfigLoader::load_with_extends` (extends chain honored per CLAUDE.md). Missing path fails with `"Dev container config (<path>) not found."` - Image-metadata extraction from the container's `devcontainer.metadata` label. Tolerates BOTH the JSON-array form (PR-2 / #27) and the single-object form for older images. - Config merge: file config wins over image metadata on scalar fields (spec §4 `mergeConfiguration(config.config, imageMetadata)`). - Variable substitution for both `configuration` and `mergedConfiguration`. - Lifecycle hook execution via `ContainerLifecycle` (onCreate → updateContent → postCreate → postStart → postAttach), gated by `--skip-post-create` (skips ALL phases per spec §2) and `--skip-non-blocking-commands` (stops after the configured `waitFor`). - JSON output per spec §10: `{outcome: "success", configuration?, mergedConfiguration?}`. `containerId` is intentionally excluded (spec §16 design decision). ## Deferred to PR-6b - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` - Dotfiles installer with target-path marker (would reuse `crates/deacon/src/commands/up/dotfiles.rs`) - A second substitution pass against the live container environment (`${containerEnv:VAR}`) — current pass uses the configured `container_env`, not a live `docker exec` env probe These are spec §5 phases 3a and 3c; both marked "best-effort" with graceful fallback on failure. Splitting them out keeps PR-6a reviewable. ## Tests 14 new unit tests in `set_up::tests`: - `--remote-env` parsing (accepts `NAME=VALUE`, rejects malformed input) - `--config` loading (default when absent, error when missing path) - Image-metadata label parsing (missing → None, array form, single-object form, invalid JSON → error) - Config merging (file wins over metadata; file-only when no metadata) - Argument defaults and JSON-result shape (outcome field, optional fields) Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 227 pass - `make test-nextest-fast` → 1927 pass (no regression) Refs: issue #34 (Tier 1 progress tracker), plan `/home/vscode/.claude/plans/let-s-come-up-with-recursive-sutherland.md` (PR-6 sequencing rationale). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon
added a commit
that referenced
this pull request
May 25, 2026
* feat: set-up subcommand — MVP scaffold + lifecycle hooks (PR-6a) Implements the core value of `set-up` per `docs/subcommand-specs/set-up/SPEC.md`: convert an already-running container into a DevContainer by applying configuration + image metadata and executing lifecycle hooks, emitting a single-line JSON result on stdout. ## CLI surface `deacon set-up --container-id <id> [--config <path>] [--skip-post-create] [--skip-non-blocking-commands] [--remote-env NAME=VALUE]... [--include-configuration] [--include-merged-configuration] [--container-data-folder <path>]` ## What this PR includes - `--container-id` resolution + inspect validation. Missing container fails with the upstream-aligned summary `"Dev container not found."` - Optional `--config` load via `ConfigLoader::load_with_extends` (extends chain honored per CLAUDE.md). Missing path fails with `"Dev container config (<path>) not found."` - Image-metadata extraction from the container's `devcontainer.metadata` label. Tolerates BOTH the JSON-array form (PR-2 / #27) and the single-object form for older images. - Config merge: file config wins over image metadata on scalar fields (spec §4 `mergeConfiguration(config.config, imageMetadata)`). - Variable substitution for both `configuration` and `mergedConfiguration`. - Lifecycle hook execution via `ContainerLifecycle` (onCreate → updateContent → postCreate → postStart → postAttach), gated by `--skip-post-create` (skips ALL phases per spec §2) and `--skip-non-blocking-commands` (stops after the configured `waitFor`). - JSON output per spec §10: `{outcome: "success", configuration?, mergedConfiguration?}`. `containerId` is intentionally excluded (spec §16 design decision). ## Deferred to PR-6b - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` - Dotfiles installer with target-path marker (would reuse `crates/deacon/src/commands/up/dotfiles.rs`) - A second substitution pass against the live container environment (`${containerEnv:VAR}`) — current pass uses the configured `container_env`, not a live `docker exec` env probe These are spec §5 phases 3a and 3c; both marked "best-effort" with graceful fallback on failure. Splitting them out keeps PR-6a reviewable. ## Tests 14 new unit tests in `set_up::tests`: - `--remote-env` parsing (accepts `NAME=VALUE`, rejects malformed input) - `--config` loading (default when absent, error when missing path) - Image-metadata label parsing (missing → None, array form, single-object form, invalid JSON → error) - Config merging (file wins over metadata; file-only when no metadata) - Argument defaults and JSON-result shape (outcome field, optional fields) Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 227 pass - `make test-nextest-fast` → 1927 pass (no regression) Refs: issue #34 (Tier 1 progress tracker), plan `/home/vscode/.claude/plans/let-s-come-up-with-recursive-sutherland.md` (PR-6 sequencing rationale). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: set-up — wire dotfiles installer (PR-6b) Follow-up to PR-6a (#36). Adds the three `--dotfiles-*` flags from spec §2 to the `set-up` CLI surface and threads them as a `DotfilesConfig` into `ContainerLifecycle`. The lifecycle helper already implements clone + auto-detected installer (`install.sh` / `bootstrap` / `setup` / `script/*`) + target-path marker for idempotency — set-up just needed to opt in. ## New flags - `--dotfiles-repository <URL or owner/repo>` - `--dotfiles-install-command <string>` (overrides auto-detection) - `--dotfiles-target-path <path>` (defaults to `~/dotfiles`) No repository → no clone. The lifecycle helper's marker-at-target-path guard provides idempotency across re-runs (spec §6, §16 design decision). ## Tests 3 new unit tests in `set_up::tests::build_dotfiles_config_*`: - `returns_none_when_no_repository` — no repo URL means no opt-in, even if the other flags are set - `forwards_all_three_fields` — all flags round-trip into `DotfilesConfig` - `leaves_defaults_to_lifecycle_helper` — `target_path` / `install_command` stay `None` so the helper computes its standard defaults Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib set_up` → 17 pass (14 from PR-6a + 3 new) ## Deferred to PR-6c - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` (spec §5 phase 3a; net-new code that needs root exec + heredoc plumbing) - Live-container `${containerEnv:VAR}` second-pass substitution Refs: issue #34 (Tier 1 progress tracker); PR-6a (#36). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon
added a commit
that referenced
this pull request
May 25, 2026
) * feat: set-up subcommand — MVP scaffold + lifecycle hooks (PR-6a) Implements the core value of `set-up` per `docs/subcommand-specs/set-up/SPEC.md`: convert an already-running container into a DevContainer by applying configuration + image metadata and executing lifecycle hooks, emitting a single-line JSON result on stdout. ## CLI surface `deacon set-up --container-id <id> [--config <path>] [--skip-post-create] [--skip-non-blocking-commands] [--remote-env NAME=VALUE]... [--include-configuration] [--include-merged-configuration] [--container-data-folder <path>]` ## What this PR includes - `--container-id` resolution + inspect validation. Missing container fails with the upstream-aligned summary `"Dev container not found."` - Optional `--config` load via `ConfigLoader::load_with_extends` (extends chain honored per CLAUDE.md). Missing path fails with `"Dev container config (<path>) not found."` - Image-metadata extraction from the container's `devcontainer.metadata` label. Tolerates BOTH the JSON-array form (PR-2 / #27) and the single-object form for older images. - Config merge: file config wins over image metadata on scalar fields (spec §4 `mergeConfiguration(config.config, imageMetadata)`). - Variable substitution for both `configuration` and `mergedConfiguration`. - Lifecycle hook execution via `ContainerLifecycle` (onCreate → updateContent → postCreate → postStart → postAttach), gated by `--skip-post-create` (skips ALL phases per spec §2) and `--skip-non-blocking-commands` (stops after the configured `waitFor`). - JSON output per spec §10: `{outcome: "success", configuration?, mergedConfiguration?}`. `containerId` is intentionally excluded (spec §16 design decision). ## Deferred to PR-6b - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` - Dotfiles installer with target-path marker (would reuse `crates/deacon/src/commands/up/dotfiles.rs`) - A second substitution pass against the live container environment (`${containerEnv:VAR}`) — current pass uses the configured `container_env`, not a live `docker exec` env probe These are spec §5 phases 3a and 3c; both marked "best-effort" with graceful fallback on failure. Splitting them out keeps PR-6a reviewable. ## Tests 14 new unit tests in `set_up::tests`: - `--remote-env` parsing (accepts `NAME=VALUE`, rejects malformed input) - `--config` loading (default when absent, error when missing path) - Image-metadata label parsing (missing → None, array form, single-object form, invalid JSON → error) - Config merging (file wins over metadata; file-only when no metadata) - Argument defaults and JSON-result shape (outcome field, optional fields) Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 227 pass - `make test-nextest-fast` → 1927 pass (no regression) Refs: issue #34 (Tier 1 progress tracker), plan `/home/vscode/.claude/plans/let-s-come-up-with-recursive-sutherland.md` (PR-6 sequencing rationale). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: set-up — wire dotfiles installer (PR-6b) Follow-up to PR-6a (#36). Adds the three `--dotfiles-*` flags from spec §2 to the `set-up` CLI surface and threads them as a `DotfilesConfig` into `ContainerLifecycle`. The lifecycle helper already implements clone + auto-detected installer (`install.sh` / `bootstrap` / `setup` / `script/*`) + target-path marker for idempotency — set-up just needed to opt in. ## New flags - `--dotfiles-repository <URL or owner/repo>` - `--dotfiles-install-command <string>` (overrides auto-detection) - `--dotfiles-target-path <path>` (defaults to `~/dotfiles`) No repository → no clone. The lifecycle helper's marker-at-target-path guard provides idempotency across re-runs (spec §6, §16 design decision). ## Tests 3 new unit tests in `set_up::tests::build_dotfiles_config_*`: - `returns_none_when_no_repository` — no repo URL means no opt-in, even if the other flags are set - `forwards_all_three_fields` — all flags round-trip into `DotfilesConfig` - `leaves_defaults_to_lifecycle_helper` — `target_path` / `install_command` stay `None` so the helper computes its standard defaults Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib set_up` → 17 pass (14 from PR-6a + 3 new) ## Deferred to PR-6c - `/etc/environment` + `/etc/profile` root-side patches with system markers under `/var/devcontainer/` (spec §5 phase 3a; net-new code that needs root exec + heredoc plumbing) - Live-container `${containerEnv:VAR}` second-pass substitution Refs: issue #34 (Tier 1 progress tracker); PR-6a (#36). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat: set-up — /etc/environment + /etc/profile root patches (PR-6c) Follow-up to PR-6b (#37). Implements spec §5 phase 3a: append-only root patches to `/etc/environment` and `/etc/profile` so login shells inherit the resolved container env / PATH. Both patches are guarded by marker files under `--container-system-data-folder` (default `/var/devcontainer/`) so re-running set-up against the same container is a no-op. ## What this PR adds - **New CLI flag**: `--container-system-data-folder <path>` (spec §6, default `/var/devcontainer`). - **`apply_etc_patches`** wires both patches before the lifecycle hook phase (spec §5 puts system patches in phase 3a). Skipped when `--skip-post-create` is set, mirroring upstream's "post-create is the umbrella for user customization" semantics. - **`build_etc_environment_patch_script`**: emits a shell script that appends a `# >>> deacon set-up >>>` ... `# <<< deacon set-up <<<` block of `KEY="VALUE"` lines to `/etc/environment`. Escapes `\` and `"` so the PAM parser doesn't choke. Touches the marker only after a successful append. Empty env → no-op (does NOT touch the marker, so a later run with a populated config still patches). - **`build_etc_profile_patch_script`**: emits a shell script that re-exports PATH from `/etc/environment` inside a delimited block. This is the spec-§16 design-decision recipe — without it the env appended in `/etc/environment` doesn't reach login shells. - **`collect_env_pairs`**: merges `containerEnv` + `remoteEnv` + CLI `--remote-env` overlays into a sorted, deterministic list. CLI wins on key conflicts. `None`-valued `remote_env` entries are dropped. - **Best-effort**: any non-zero exit from the patch shell (no root, read-only `/etc`, missing `sh`, etc.) emits a WARN and proceeds. Spec §9: system-level patches "do not abort set-up unless critical". ## Tests 10 new unit tests: - `build_etc_environment_patch_returns_noop_when_env_empty` - `build_etc_environment_patch_short_circuits_when_marker_present` - `build_etc_environment_patch_writes_sorted_env_block` - `build_etc_environment_patch_wraps_block_in_delimiters` - `build_etc_environment_patch_escapes_special_chars_in_values` - `build_etc_profile_patch_short_circuits_on_marker` - `build_etc_profile_patch_reexports_path_from_environment` - `build_etc_profile_patch_wraps_in_delimiters` - `collect_env_pairs_merges_config_remote_and_cli` - `collect_env_pairs_returns_sorted_output` Total set-up tests: 27 pass (14 from PR-6a + 3 from PR-6b + 10 new). Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 240 pass (no regression) ## With this PR, the set-up subcommand is functionally complete The only spec-§5 item still on the deferred list is the live-container `${containerEnv:VAR}` second-pass substitution — an enhancement, not a behavior gap; the configured `container_env` is already substituted. Refs: issue #34 (Tier 1 progress tracker); PR-6a (#36), PR-6b (#37). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- 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
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.
Per upstream PR #1199 (v0.86.0), the
devcontainer.metadataimage label is always a JSON array, even when only a single entry is present. VS Code, Zed, and envbuilder iterate the array and merge entries in order. Writers emitting a single object break this contract.Summary
Writer changes (three sites):
crates/core/src/build/metadata.rs—DevcontainerMetadata::to_jsonwraps in[...].from_jsonreturnsVec<Self>and accepts either array form or legacy single-object form.crates/deacon/src/commands/build/mod.rs— both inline metadata writers (image-reference path at ~L1272; docker build path at ~L1546) wrapped in[...].Reader change:
crates/deacon/src/commands/read_configuration.rs(compute_merged_configuration, ~L411) — parses the label asserde_json::Value, normalizes to aVecof entries (array as-is; object wrapped in single-element vec), then deserializes each entry as a partialDevContainerConfig, applies variable substitution, and merges with the base config in declaration order.Tests added
In
crates/core/src/build/metadata.rs:test_to_json_emits_array— asserts label starts with[test_from_json_handles_legacy_object_form— legacy single-object fixturetest_from_json_handles_array_form_with_multiple_entries— spec-form multi-entry fixturetest_metadata_roundtrip— updated forVecreturn typetest_merge_labels_emits_array_label_value— end-to-end label assertionTest plan
cargo fmt --all -- --checkcargo check --workspace --all-targetscargo clippy --all-targets -- -D warningscargo nextest run --profile dev-fast --no-default-features→ 1930/1930 pass, 30 skippedcargo test --doc --workspace→ 130/130 passdeacon buildagainstexamples/build/, thendocker inspect <image> | jq '.[0].Config.Labels["devcontainer.metadata"]'— should start with[Known follow-up (not in scope; file separate ticket)
The
DevcontainerMetadatastruct wraps the config under a"config"subkey alongsidefeatures/customizations/lockfile_hash. Upstream array entries are flat partial-config shapes. Deacon-emitted entries today are not directly merge-able asDevContainerConfig— a latent issue pre-dating this PR. Flattening the struct to match upstream's array-entry shape would be its own refactor.This PR ships the spec-mandated wire format change; the deeper shape alignment is post-1.0 cleanup.
Refs
docs/ROADMAP_TO_1.0.mdTier 1 item "devcontainer.metadata label JSON array (CLI parity B.2)".🤖 Generated with Claude Code