feat: upgrade — wire --feature / --target-version config pinning (PR-5b)#40
Merged
Conversation
Phase 1 of lockfile graduation per docs/ROADMAP_TO_1.0.md. Ships the schema parity fixes, the user-visible CLI flag surface, the writer helper, and upstream-aligned error messages. The actual writer integration (building a Lockfile from resolved features and writing it after `up`/`build`) is tracked as PR-4b at issue #32. ## Schema parity (deacon-core/lockfile.rs) - LockfileFeature.depends_on now serializes as `dependsOn` (camelCase) via #[serde(rename)], matching upstream `devcontainers/cli`'s generateLockfile in src/spec-configuration/lockfile.ts. Deserializer accepts the camelCase form on read. - write_lockfile() emits a trailing newline to match upstream `JSON.stringify(..., 2) + '\n'`. Byte-identical output keeps the --frozen-lockfile content comparison stable. - LockfileValidationResult::format_error() now emits upstream-aligned strings: "Lockfile does not exist." / "Lockfile does not match." as the leading summary line; trailing actionable guidance references the graduated --frozen-lockfile flag rather than the deprecated --experimental-frozen-lockfile. - New LockfileFeature::from_resolved() helper constructs entries in the upstream canonical form (resolved = "{registry}/{repo}@{digest}", integrity = "{digest}"). This is the writer entry point PR-4b uses. ## CLI surface graduation up + build both gain: - --no-lockfile (visible): skip lockfile generation and verification. - --frozen-lockfile (visible): require an up-to-date lockfile; fail if resolution would change it. Mutual exclusivity (--no-lockfile xor --frozen-lockfile) enforced in the CLI layer, mirroring upstream's pre-parse validation. Deprecation: - --experimental-lockfile and --experimental-frozen-lockfile remain accepted as hidden aliases through the 1.x line. The CLI emits a WARN on use directing users to the graduated flags. - effective_frozen = frozen_lockfile || experimental_frozen_lockfile matches upstream's effectiveFrozenLockfile coalescing. ## Downstream consumers - up/mod.rs frozen-validation path now reads args.frozen_lockfile (the effective value) instead of args.experimental_frozen_lockfile. No behavior change — only the variable name moves. - build/mod.rs args struct gains no_lockfile + frozen_lockfile fields (carrier-only for PR-4b; #[allow(dead_code)] until then). ## Tests Added in deacon-core/lockfile.rs: - test_depends_on_serializes_as_camel_case - test_write_lockfile_emits_trailing_newline - test_from_resolved_constructs_upstream_form Updated in deacon/tests/up_lockfile_frozen.rs (5 assertions): align with the new upstream-format error messages ("Lockfile does not exist." / "Lockfile does not match." / capital "F" on "Features ..." substrings). ## Verified - cargo fmt --all -- --check - cargo clippy --all-targets -- -D warnings - cargo nextest run --profile dev-fast --no-default-features: 1930/1930 pass - cargo test --doc --workspace: 130/130 pass ## Follow-up tracked - #32 — PR-4b: wire writer in up + build feature pipeline. - Build-command lockfile wiring is gated on the pre-existing TODO at build/mod.rs:1285 (build doesn't install features today). Will land with #32 or as a sibling PR. ## Refs - docs/ROADMAP_TO_1.0.md Tier 1 item "Lockfile graduation" - Upstream: devcontainers/cli#1212 (graduated lockfile in v0.87.0) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the regenerate path of `deacon upgrade` per
`docs/subcommand-specs/upgrade/SPEC.md`: re-resolve every Feature against
the OCI registry, build a fresh `Lockfile`, and either print it to stdout
(`--dry-run`) or write it to disk via `write_lockfile(force_init = true)`.
## CLI surface
`deacon upgrade [--dry-run] [--docker-path <path>]
[--docker-compose-path <path>] [-f <id>] [-v <ver>]`
Workspace + `--config` come from the global flags (parity with other
consumer subcommands). `-f`/`--feature` and `-v`/`--target-version` are
hidden (Dependabot-only).
## What this PR includes
- Validation (fail-fast per spec §2/§3):
- `--feature` and `--target-version` must both be set or both absent;
error message matches upstream: `"The '--target-version' and '--feature'
flag must be used together."`
- `--target-version` must match `^\d+(\.\d+(\.\d+)?)?$`; error matches
upstream: `"Invalid version 'X'. Must be in the form of 'x', 'x.y',
or 'x.y.z'"`
- Config load via the shared `load_config` helper (extends chain honored).
- Feature resolution via the existing OCI fetcher (`default_fetcher()` +
`fetch_feature`), which returns each feature's manifest digest + metadata
version.
- Lockfile assembly keyed by user-provided feature ID (matches upstream
`generateLockfile` + PR-4b's helper in `up`). Metadata-version missing →
falls back to the user-requested tag (e.g. `"1"`) with a WARN.
- `--dry-run`: pretty-printed canonical JSON to stdout (sorted keys,
trailing newline) — byte-identical to what `write_lockfile` would emit.
- Default: `write_lockfile(force_init = true)` per spec §5 phase 4.
## Tests
13 new unit tests in `upgrade::tests`:
- Pin-flag pairing: both-set, both-absent, only-feature, only-target
- Target-version regex: 5 valid forms, 8 invalid forms (incl. `latest`,
`v1`, `1.x.3`)
- Spec-exact error message on invalid `--target-version`
- Lockfile entry assembly: metadata-version vs tag fallback, sorted
`dependsOn`
- Empty-features short-circuit (two flavors — missing object, empty object)
- Argument defaults
Verification:
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets -- -D warnings`
- `cargo test -p deacon --lib` → 226 pass (no regression; baseline
includes PR-4a since this branch is based on `pr4-lockfile-graduation`)
## Deferred to PR-5b
- `--feature` / `--target-version` config-pin behavior (modifies
`devcontainer.json` in place). The flags are accepted today so the CLI
surface is stable, but using them returns
`"--feature/--target-version pinning is not yet implemented (PR-5b)"`
rather than silently doing nothing.
## Code-dedup follow-up
The lockfile-entry assembly logic is intentionally duplicated from PR-4b
(`crates/deacon/src/commands/up/features_build.rs::build_lockfile_from_features`)
so PR-5a doesn't depend on PR-4b's diff. Once both land on `main`, a
small follow-up PR can lift the shared logic into `deacon_core::lockfile`.
## Base branch
Based on `pr4-lockfile-graduation` (#33) since upgrade needs
`deacon_core::lockfile::{Lockfile, LockfileFeature, get_lockfile_path,
write_lockfile}` — all added in PR-4a. After #33 merges, this branch
auto-rebases onto `main`.
Closes part of the "Implement `upgrade`" Tier 1 blocker (issue #34).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up to PR-5a (#39). Implements spec §5 phase 2: when both `--feature` and `--target-version` are set, rewrite the matching feature key in `devcontainer.json` in place BEFORE the lockfile regeneration phase. ## Implementation **Text-level surgical edit** — we read the config file as a string, find the literal quoted JSON key for `--feature`, and replace its tag with `--target-version`. We never parse-and-re-emit the JSON, so: - Comments survive (`//` and `/* */` in JSONC/JSON5) - Whitespace and key order are preserved - No dependency on a JSON5-AST library Trade-offs (all acceptable given the edit is bounded to one key): - Registry-port colons are correctly distinguished from tag colons via the rule "tag separator is the last `:` after the last `/`" - The substring search is precise enough to ignore escaped-quote occurrences inside string values (the byte sequences differ: `\"feature:1\"` vs `"feature:1"`) - Multiple unescaped matches (key appears both as a JSON key AND as an unescaped string value) is detected and reported as ambiguous, with the user asked to resolve manually ## What this PR changes In `crates/deacon/src/commands/upgrade.rs`: - `pin_feature_in_config_file()` — reads, rewrites, writes; bubbles context on IO failures and errors on missing/ambiguous matches. - `pinned_feature_key()` — pure helper that handles tag replacement, no-tag append, and registry-port disambiguation. - `rewrite_feature_key()` — pure helper that surgically replaces the literal `"<feature>"` substring and reports the occurrence count. - The `execute_upgrade` flow now (a) edits the config when both flags are set, then (b) re-reads via the shared loader so the resolution phase sees the pinned form. Spec §5: "Re-read config for consistency after edit". - The previous "not yet implemented" surface-parity error is removed. In `crates/deacon/src/cli.rs`: - Doc strings on `--feature` / `--target-version` no longer flag them as deferred. ## Tests 10 new unit tests: - `pinned_feature_key_*` (3): tag replacement, tag append when absent, registry-port disambiguation - `rewrite_feature_key_*` (4): single-match replace, unrelated keys preserved, zero-match reports 0, ambiguous reports the actual count, escaped-quote substring is correctly skipped - `pin_feature_in_config_file_*` (2): round-trip through disk preserves comments + sibling keys; missing-feature surfaces a clear error Verification: - `cargo fmt --all -- --check` - `cargo clippy --all-targets -- -D warnings` - `cargo test -p deacon --lib` → 236 pass (no regression; 23 upgrade tests total — 13 from PR-5a + 10 new) ## Base branch Based on `pr5-upgrade-subcommand` (#39) so the diff shows only PR-5b's additions. Auto-rebases to `main` once #33 and #39 land in order. With this PR, the `upgrade` subcommand is functionally complete for spec §5. Refs: issue #34 (Tier 1 progress tracker); PR-5a (#39). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # crates/deacon/src/cli.rs
# Conflicts: # crates/deacon/src/cli.rs # crates/deacon/src/commands/upgrade.rs
pofallon
added a commit
that referenced
this pull request
May 25, 2026
…ng-build (#45) Follow-up to the Tier 1 merge cascade — closes out the [Unreleased] section with entries for the two tracks that landed after PR-7 (#31) was first drafted: - `upgrade` subcommand (PR-5a #44 + PR-5b #40 pinning) - features-during-build for Dockerfile configs (PR-4c #41) The lockfile and set-up entries were added earlier in commit e6b929c. This commit completes the Unreleased section. 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.
Follow-up to PR-5a (#39). Implements spec §5 phase 2: when both `--feature` and `--target-version` are set, rewrite the matching feature key in `devcontainer.json` in place BEFORE the lockfile regeneration phase.
With this PR, the `upgrade` subcommand is functionally complete for spec §5.
Implementation: text-level surgical edit
We read the config file as a string, find the literal quoted JSON key for `--feature`, and replace its tag with `--target-version`. We never parse-and-re-emit the JSON, so:
Trade-offs (all acceptable given the edit is bounded to one key):
What this PR changes
In `crates/deacon/src/commands/upgrade.rs`:
In `crates/deacon/src/cli.rs`:
Tests
10 new unit tests:
Verification:
Base branch
Based on `pr5-upgrade-subcommand` (#39) so the diff shows only PR-5b's additions. Auto-rebases to `main` once #33 and #39 land in order.
Refs
🤖 Generated with Claude Code