Skip to content

Commit 7ecbebe

Browse files
pofallonclaude
andauthored
feat: upgrade — wire --feature / --target-version config pinning (PR-5b) (#40)
* feat: lockfile graduation — schema parity + CLI surface (PR-4a) 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> * feat: upgrade subcommand — regenerate-lockfile MVP (PR-5a) 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> * feat: upgrade — wire --feature / --target-version config pinning (PR-5b) 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> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6a56069 commit 7ecbebe

2 files changed

Lines changed: 311 additions & 27 deletions

File tree

crates/deacon/src/cli.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -529,11 +529,11 @@ pub enum Commands {
529529
docker_compose_path: String,
530530
/// HIDDEN: pin the version of a specific Feature in `devcontainer.json`
531531
/// before regenerating the lockfile. Used by Dependabot.
532-
/// Must be used with `--target-version`. **Pinning is deferred to PR-5b.**
532+
/// Must be used with `--target-version`.
533533
#[arg(long, short = 'f', hide = true)]
534534
feature: Option<String>,
535535
/// HIDDEN: target version for `--feature`. Must match
536-
/// `^\d+(\.\d+(\.\d+)?)?$`. **Pinning is deferred to PR-5b.**
536+
/// `^\d+(\.\d+(\.\d+)?)?$`.
537537
#[arg(long, short = 'v', hide = true)]
538538
target_version: Option<String>,
539539
},

0 commit comments

Comments
 (0)