From 288358e7ec5d3bbdd97d3ae0ce911c9c2eee381b Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:53:44 -0600 Subject: [PATCH 1/7] test: round-trip every supported format through the binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four of the ten supported layouts had no end-to-end coverage at all — pyproject.toml, uv.lock, Directory.Build.props and .targets were proven only by unit tests, so the formats added most recently were the ones being checked by hand. One table now drives two tests: a stable bump and a pre-release, each asserting that every declared file moved and that `check` agrees in both directions. Adding a format is a row. Breaking one writer turns both red, which is how the table was verified. The pre-release case is not redundant with the stable one. It writes a version that is not three numbers, which is where a format that cannot hold one shows up at all. Co-Authored-By: Claude Opus 5 --- tests/cli.rs | 209 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/tests/cli.rs b/tests/cli.rs index 4a8f178..670467e 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1821,3 +1821,212 @@ fn collect_tags(value: &serde_json::Value) -> Vec { _ => Vec::new(), } } + +// ─── Every supported format, through the real binary ──────────────────────── + +/// One supported layout: the files a project of that shape has, and what +/// `vump.toml` declares. +struct FormatCase { + /// Named in failure messages. + label: &'static str, + /// `files = [...]`, in configuration order. + declared: &'static [&'static str], + /// Files to write, each recording `1.0.0`. + contents: &'static [(&'static str, &'static str)], +} + +/// Every format vump recognizes, as a project that has it. +/// +/// Adding a format means adding a row. The alternative is what this replaced: +/// building one project per ecosystem by hand and reading the result, which is +/// neither repeatable nor run on three platforms. +const FORMAT_CASES: &[FormatCase] = &[ + FormatCase { + label: "plain text", + declared: &["VERSION"], + contents: &[("VERSION", "1.0.0\n")], + }, + FormatCase { + label: "npm manifest", + declared: &["package.json"], + contents: &[( + "package.json", + "{\n \"name\": \"demo\",\n \"version\": \"1.0.0\"\n}\n", + )], + }, + FormatCase { + label: "npm manifest and lock", + declared: &["package.json", "package-lock.json"], + contents: &[ + ( + "package.json", + "{\n \"name\": \"demo\",\n \"version\": \"1.0.0\"\n}\n", + ), + ( + "package-lock.json", + "{\n \"name\": \"demo\",\n \"version\": \"1.0.0\",\n \"lockfileVersion\": 3,\n \"packages\": {\n \"\": {\n \"name\": \"demo\",\n \"version\": \"1.0.0\"\n }\n }\n}\n", + ), + ], + }, + FormatCase { + label: "cargo manifest", + declared: &["Cargo.toml"], + contents: &[( + "Cargo.toml", + "[package]\nname = \"demo\"\nversion = \"1.0.0\"\n", + )], + }, + FormatCase { + label: "cargo manifest and lock", + declared: &["Cargo.toml", "Cargo.lock"], + contents: &[ + ( + "Cargo.toml", + "[package]\nname = \"demo\"\nversion = \"1.0.0\"\n", + ), + ( + "Cargo.lock", + "version = 3\n\n[[package]]\nname = \"demo\"\nversion = \"1.0.0\"\n", + ), + ], + }, + FormatCase { + label: "python manifest", + declared: &["pyproject.toml"], + contents: &[( + "pyproject.toml", + "[project]\nname = \"demo\"\nversion = \"1.0.0\"\n", + )], + }, + FormatCase { + label: "python manifest and uv lock", + declared: &["pyproject.toml", "uv.lock"], + contents: &[ + ( + "pyproject.toml", + "[project]\nname = \"demo\"\nversion = \"1.0.0\"\n", + ), + ( + "uv.lock", + "version = 1\nrevision = 3\n\n[[package]]\nname = \"demo\"\nversion = \"1.0.0\"\nsource = { editable = \".\" }\n", + ), + ], + }, + FormatCase { + label: "msbuild project", + declared: &["Demo.csproj"], + contents: &[( + "Demo.csproj", + "\n \n 1.0.0\n \n\n", + )], + }, + FormatCase { + label: "msbuild shared props", + declared: &["Directory.Build.props"], + contents: &[( + "Directory.Build.props", + "\n \n 1.0.0\n \n\n", + )], + }, + FormatCase { + label: "msbuild shared targets", + declared: &["Directory.Build.targets"], + contents: &[( + "Directory.Build.targets", + "\n \n 1.0.0\n \n\n", + )], + }, +]; + +#[test] +fn every_supported_format_round_trips_through_the_binary() { + for case in FORMAT_CASES { + let label = case.label; + let declared = case + .declared + .iter() + .map(|f| format!("\"{f}\"")) + .collect::>() + .join(", "); + + let mut fx = Fixture::new().write("vump.toml", &format!("files = [{declared}]\n")); + for (name, body) in case.contents { + fx = fx.write(name, body); + } + + let status = fx.run(&["status"]); + assert!(status.code == 0, "{label}: status: {}", status.output()); + assert!( + status.stdout.contains("1.0.0"), + "{label}: {}", + status.stdout + ); + + let check = fx.run(&["check", "1.0.0"]); + assert_eq!(check.code, 0, "{label}: check: {}", check.output()); + + let bump = fx.run(&["patch", "--through", "none"]); + assert_eq!(bump.code, 0, "{label}: patch: {}", bump.output()); + + // Every declared file moved, not just the first one read. + for (name, _) in case.contents { + assert!( + fx.read(name).contains("1.0.1"), + "{label}: {name} still holds the old version:\n{}", + fx.read(name), + ); + } + + // `check` is the contract CI depends on, so it is asserted in both + // directions: agreeing with the new version and refusing the old. The + // code is 4 and not 5 — the files agree with each other and disagree + // with the argument, which is a mismatch rather than being out of sync. + assert_eq!( + fx.run(&["check", "1.0.1"]).code, + 0, + "{label}: check after bump" + ); + assert_eq!( + fx.run(&["check", "1.0.0"]).code, + 4, + "{label}: a stale version is a mismatch", + ); + } +} + +#[test] +fn every_supported_format_reaches_a_pre_release_and_back() { + // The pre-release path writes a version that is not three numbers, which + // is where a format that cannot hold one shows up — an + // takes four numeric parts and would silently truncate. + for case in FORMAT_CASES { + let label = case.label; + let declared = case + .declared + .iter() + .map(|f| format!("\"{f}\"")) + .collect::>() + .join(", "); + + let mut fx = Fixture::new().write("vump.toml", &format!("files = [{declared}]\n")); + for (name, body) in case.contents { + fx = fx.write(name, body); + } + + let alpha = fx.run(&["alpha", "--toward", "minor", "--through", "none"]); + assert_eq!(alpha.code, 0, "{label}: alpha: {}", alpha.output()); + for (name, _) in case.contents { + assert!( + fx.read(name).contains("1.1.0-alpha.0"), + "{label}: {name}:\n{}", + fx.read(name), + ); + } + + let release = fx.run(&["release", "--through", "none"]); + assert_eq!(release.code, 0, "{label}: release: {}", release.output()); + for (name, _) in case.contents { + assert!(fx.read(name).contains("1.1.0"), "{label}: {name}"); + } + } +} From 65eac1eeb85d4b69bc9a98e5989a28014d696694 Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:56:48 -0600 Subject: [PATCH 2/7] fix: bump an npm workspace member without reading the root's version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A project declaring a member's package.json alongside the shared package-lock.json was told its files disagreed, naming the member's version against the root's. They did not disagree: the lock's entry for that member held exactly what the manifest held. vump was reading the root. The lock keys entries by path, which is why this looked like it needed a mapping from a declared manifest path to a `packages` key — the inference the backlog entry refused to guess at. It does not: each entry records the name inside it, so a member is selected by the name its manifest declares, as in every other lock. package.json joins the manifests read for that name. The root's two records, the top-level version and the "" entry, now belong to the project only when the lock's own name is one the project declares. That is what stops a member rewriting the root above it, and it leaves a lock with no `packages` map — or one declared without a manifest to name it — reading the root as before. Co-Authored-By: Claude Opus 5 --- BACKLOG.md | 12 -- DESIGN.md | 7 ++ src/app/mod.rs | 1 + src/domain/version_file.rs | 227 +++++++++++++++++++++++++++++++------ tests/cli.rs | 36 ++++++ 5 files changed, 234 insertions(+), 49 deletions(-) diff --git a/BACKLOG.md b/BACKLOG.md index b2fe11d..358ebdc 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -26,18 +26,6 @@ agreed, at which point it is ranked by value. Each says what would settle it. Nothing moves up until something does. -### npm workspaces - -A Cargo workspace's shared lock is now written per member, matched by the -package name each manifest declares. `package-lock.json` has the same shape and -not the same solution: its `packages` map is keyed by *path* -(`"packages/api"`), not by package name, and a workspace member's version -appears both there and in the member's own `package.json`. - -Undecided because no repository here needs it yet, and guessing at the mapping -between a declared manifest path and a `packages` key is exactly the kind of -inference that produced the original lock-file defect. - ### Declarative version-file formats Detection is by filename. Anything else — `gradle.properties`, a `*.gemspec`, a diff --git a/DESIGN.md b/DESIGN.md index d73cbb8..8dfdd2e 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -150,6 +150,13 @@ In `Cargo.lock` the entries that belong to the repository are the everything it fetched. A single-crate repository has one, and it needs no naming. +`package-lock.json` keys its entries by *path* rather than by name — the root +is `""` and a member is `"packages/api"` — but records the name inside each +entry, so a member is still selected by the name its manifest declares. The +root's two records, the top-level `version` and the `""` entry, belong to the +project only when the lock's own name is one the project declares; that is what +keeps a member from rewriting the root it sits under. + `uv.lock` is the same shape with the marker inverted: uv records a `source` for both, and the entries built from the tree are the ones marked `editable`. That distinction is load-bearing rather than cosmetic — a published release of diff --git a/src/app/mod.rs b/src/app/mod.rs index 3dc5ad3..527fa4f 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -185,6 +185,7 @@ pub(crate) fn local_package_names<'a>( .into_iter() .filter_map(|path| { let read = match file_name_of(path) { + "package.json" => version_file::package_json_name, "Cargo.toml" => version_file::cargo_package_name, "pyproject.toml" => version_file::pyproject_package_name, _ => return None, diff --git a/src/domain/version_file.rs b/src/domain/version_file.rs index af01d31..bec5357 100644 --- a/src/domain/version_file.rs +++ b/src/domain/version_file.rs @@ -346,9 +346,7 @@ impl LockFile { let raw = match self { Self::Cargo => read_cargo_lock_version(file, contents, packages, is_cargo_local)?, Self::Uv => read_cargo_lock_version(file, contents, packages, is_uv_local)?, - // An npm lock records only the root package until npm workspaces - // are supported, so there is nothing yet for `packages` to select. - Self::Npm => read_package_lock_version(file, contents)?, + Self::Npm => read_package_lock_version(file, contents, packages)?, }; parse_version(file, self.field_description(), &raw) @@ -373,7 +371,7 @@ impl LockFile { write_cargo_lock_version(file, contents, version, packages, is_cargo_local) } Self::Uv => write_cargo_lock_version(file, contents, version, packages, is_uv_local), - Self::Npm => write_package_lock_version(file, contents, version), + Self::Npm => write_package_lock_version(file, contents, version, packages), } } } @@ -649,6 +647,20 @@ fn write_pyproject_version( Ok(doc.to_string()) } +/// Reads the package name an npm manifest declares. +/// +/// This is what pairs a manifest with its entry in a workspace lock, exactly +/// as [`cargo_package_name`] does for Cargo. A lock keys its entries by path +/// and records the name inside each, so the name is still what matches. +#[must_use] +pub fn package_json_name(contents: &str) -> Option { + serde_json::from_str::(contents) + .ok()? + .get("name")? + .as_str() + .map(str::to_owned) +} + /// Reads the package name a Python manifest declares. /// /// This is what pairs a manifest with its entry in a shared uv workspace lock, @@ -712,29 +724,100 @@ fn write_cargo_version( /// The path to the root package's entry, from lockfile version 2 onwards. const NPM_ROOT_PACKAGE: &[&str] = &["packages", ""]; -/// Reads the version an npm lock file records for the project itself. -fn read_package_lock_version(file: &str, contents: &str) -> Result { - let root = find_json_version(contents, &[]).ok_or_else(|| VersionFileError::MissingField { - file: file.to_owned(), - field: "\"version\"".to_owned(), - })?; - let root = &contents[root]; - - // Version 1 has no `packages` map. Where one exists both records describe - // the same project, so a disagreement is a corrupt file rather than a - // question to answer by preferring one of them. - if let Some(nested) = find_json_version(contents, NPM_ROOT_PACKAGE) { - let nested = &contents[nested]; - if nested != root { - return Err(VersionFileError::InconsistentLock { - file: file.to_owned(), - root: root.to_owned(), - nested: nested.to_owned(), - }); +/// The spans an npm lock records this project's version in. +/// +/// A lock keys its `packages` map by path — `"packages/api"` — and records the +/// name inside each entry, so a member is selected by the name its manifest +/// declares, as in every other lock. The root is the entry keyed `""`, which +/// the top-level `version` restates; both belong to the project only when the +/// lock's own name is one the project declares, which is what keeps a +/// workspace member from rewriting the root it sits under. +fn npm_version_spans( + file: &str, + contents: &str, + packages: &[String], +) -> Result>, VersionFileError> { + let doc: serde_json::Value = + serde_json::from_str(contents).map_err(|e| VersionFileError::Malformed { + file: file.to_owned(), + format: "JSON".to_owned(), + detail: e.to_string(), + })?; + + let declares = |name: Option<&str>| name.is_some_and(|n| packages.contains(&n.to_owned())); + let entries = doc.get("packages").and_then(serde_json::Value::as_object); + + // Nothing to match against, and nothing to confuse it with: a lock with no + // `packages` map describes one project, and so does one declared without + // the manifest that would name it. + let root_only = entries.is_none() || packages.is_empty(); + let root = root_only + || declares(doc.get("name").and_then(serde_json::Value::as_str)) + || entries.is_some_and(|map| { + map.get("") + .is_some_and(|e| declares(e.get("name").and_then(serde_json::Value::as_str))) + }); + + let mut spans = Vec::new(); + if root { + if let Some(span) = find_json_version(contents, &[]) { + spans.push(span); + } + if let Some(span) = find_json_version(contents, NPM_ROOT_PACKAGE) { + spans.push(span); + } + } + + if !root_only { + for key in entries.into_iter().flatten().filter_map(|(key, entry)| { + let named = declares(entry.get("name").and_then(serde_json::Value::as_str)); + (!key.is_empty() && named && entry.get("version").is_some()).then_some(key) + }) { + if let Some(span) = find_json_version(contents, &["packages", key]) { + spans.push(span); + } } } - Ok(root.to_owned()) + if spans.is_empty() { + return Err(VersionFileError::MissingField { + file: file.to_owned(), + field: if packages.is_empty() { + "\"version\"".to_owned() + } else { + format!("an entry for {}", packages.join(" or ")) + }, + }); + } + + Ok(spans) +} + +/// Reads the version an npm lock file records for the project. +fn read_package_lock_version( + file: &str, + contents: &str, + packages: &[String], +) -> Result { + let spans = npm_version_spans(file, contents, packages)?; + + // Every place in scope describes the same project moving as a unit, so a + // disagreement is a corrupt file rather than a question to answer by + // preferring one of them. + let first = contents[spans[0].clone()].to_owned(); + if let Some(other) = spans + .iter() + .map(|span| &contents[span.clone()]) + .find(|v| *v != first) + { + return Err(VersionFileError::InconsistentLock { + file: file.to_owned(), + root: first, + nested: other.to_owned(), + }); + } + + Ok(first) } /// Rewrites every place an npm lock file records the project's own version. @@ -742,21 +825,13 @@ fn write_package_lock_version( file: &str, contents: &str, version: &Version, + packages: &[String], ) -> Result { - // Reading first rejects a lock with no version, or with two that already - // disagree, before any mutation is attempted. - read_package_lock_version(file, contents)?; + // Reading first rejects a lock with no version in scope, or with two that + // already disagree, before any mutation is attempted. + read_package_lock_version(file, contents, packages)?; - let mut spans = - vec![ - find_json_version(contents, &[]).ok_or_else(|| VersionFileError::MissingField { - file: file.to_owned(), - field: "\"version\"".to_owned(), - })?, - ]; - if let Some(nested) = find_json_version(contents, NPM_ROOT_PACKAGE) { - spans.push(nested); - } + let mut spans = npm_version_spans(file, contents, packages)?; // Splicing from the end keeps the earlier spans' offsets valid. spans.sort_by_key(|span| std::cmp::Reverse(span.start)); @@ -1323,6 +1398,84 @@ serde = \"1\" ); } + /// A root at 1.0.0 with one member at 2.5.0, as `npm install` writes it. + const NPM_WORKSPACE: &str = r#"{ + "name": "root", + "version": "1.0.0", + "lockfileVersion": 3, + "packages": { + "": { "name": "root", "version": "1.0.0" }, + "node_modules/@acme/api": { "resolved": "packages/api", "link": true }, + "packages/api": { "name": "@acme/api", "version": "2.5.0" } + } +} +"#; + + #[test] + fn an_npm_workspace_member_is_selected_by_name() { + // The lock keys entries by path, so the member is found by the name + // recorded inside its entry. Reading the root instead reported a + // disagreement between files that agree. + let api = names(&["@acme/api"]); + assert_eq!( + LockFile::Npm + .read("package-lock.json", NPM_WORKSPACE, &api) + .unwrap(), + v("2.5.0") + ); + + let out = LockFile::Npm + .write("package-lock.json", NPM_WORKSPACE, &v("2.5.1"), &api) + .unwrap(); + assert!( + out.contains(r#""name": "@acme/api", "version": "2.5.1""#), + "{out}" + ); + // The root sits above this member and is not its to move. + assert!( + out.contains(r#""name": "root", "version": "1.0.0""#), + "{out}" + ); + assert!(out.contains(r#""version": "1.0.0","#), "{out}"); + } + + #[test] + fn the_root_of_an_npm_workspace_leaves_its_members_alone() { + let root = names(&["root"]); + assert_eq!( + LockFile::Npm + .read("package-lock.json", NPM_WORKSPACE, &root) + .unwrap(), + v("1.0.0") + ); + + let out = LockFile::Npm + .write("package-lock.json", NPM_WORKSPACE, &v("1.1.0"), &root) + .unwrap(); + assert!( + out.contains(r#""name": "root", "version": "1.1.0""#), + "{out}" + ); + assert!(out.contains(r#""version": "1.1.0","#), "{out}"); + assert!( + out.contains(r#""name": "@acme/api", "version": "2.5.0""#), + "{out}" + ); + } + + #[test] + fn a_name_no_entry_declares_is_refused_rather_than_falling_back() { + // Falling back to the root here is the defect this replaced: it would + // answer about a different package than the one asked for. + let err = LockFile::Npm + .read("package-lock.json", NPM_WORKSPACE, &names(&["@acme/nope"])) + .unwrap_err(); + assert!( + matches!(err, VersionFileError::MissingField { .. }), + "{err:?}" + ); + } + #[test] fn reads_the_project_version_from_every_npm_lock_format() { for (lockfile_version, contents) in NPM_LOCKS { diff --git a/tests/cli.rs b/tests/cli.rs index 670467e..4165ccc 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1994,6 +1994,42 @@ fn every_supported_format_round_trips_through_the_binary() { } } +#[test] +fn an_npm_workspace_member_bumps_without_touching_the_root() { + // The lock keys its entries by path and the member's name lives inside the + // entry, so reading the root instead reported two files as disagreeing + // when they agreed. Asserted end to end because the pairing runs in the + // app layer, not in the format. + let fx = Fixture::new() + .write( + "vump.toml", + "[[project]]\nname = \"api\"\nfiles = [\"packages/api/package.json\", \"package-lock.json\"]\n", + ) + .write( + "packages/api/package.json", + "{\n \"name\": \"@acme/api\",\n \"version\": \"2.5.0\"\n}\n", + ) + .write( + "package-lock.json", + "{\n \"name\": \"root\",\n \"version\": \"1.0.0\",\n \"lockfileVersion\": 3,\n \"packages\": {\n \"\": { \"name\": \"root\", \"version\": \"1.0.0\" },\n \"packages/api\": { \"name\": \"@acme/api\", \"version\": \"2.5.0\" }\n }\n}\n", + ); + + let status = fx.run(&["status"]); + assert_eq!(status.code, 0, "{}", status.output()); + assert!(status.stdout.contains("2.5.0"), "{}", status.stdout); + + let bump = fx.run(&["patch", "--through", "none"]); + assert_eq!(bump.code, 0, "{}", bump.output()); + + let lock = fx.read("package-lock.json"); + assert!( + lock.contains("\"@acme/api\", \"version\": \"2.5.1\""), + "{lock}" + ); + assert!(lock.contains("\"root\", \"version\": \"1.0.0\""), "{lock}"); + assert_eq!(fx.run(&["check", "2.5.1"]).code, 0); +} + #[test] fn every_supported_format_reaches_a_pre_release_and_back() { // The pre-release path writes a version that is not three numbers, which From 779949e2bb621e288f34377f98218955995f308b Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:58:08 -0600 Subject: [PATCH 3/7] fix: let the check action name a project, and stop expanding inputs into bash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A repository declaring several projects and verifying a bare version rather than a tag could not use this action: vump refuses to guess which project, and there was no input to tell it. A tag needs nothing — it selects its own project through that project's tag_pattern — which is why this went unnoticed. There is deliberately no allow-nested input, and the description now says so. A vump.toml nested below another answers about the wrong project, and `check` is the command whose verdict gets believed; the action failing there is the refusal working. Adding the input would weigh the convenience of a layout [[project]] exists to prevent. The step also expanded ${{ inputs.* }} straight into a shell line, where an expression is substituted before bash parses it — a version input carrying shell syntax would have run as part of the step. The values now arrive through the environment, as the download step already does, and `set -euo pipefail` means a failure in the cd cannot leave the check running somewhere else. Co-Authored-By: Claude Opus 5 --- .github/actions/check/action.yml | 24 +++++++++++++++++++++--- BACKLOG.md | 12 ------------ 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/.github/actions/check/action.yml b/.github/actions/check/action.yml index eb3f873..1b296c9 100644 --- a/.github/actions/check/action.yml +++ b/.github/actions/check/action.yml @@ -9,6 +9,10 @@ inputs: description: 'Path to vump.toml, relative to the repository root. The action runs vump from the directory containing this file.' required: false default: 'vump.toml' + project: + description: 'Project to verify, in a repository declaring several. Only needed when `version` is a bare version: a tag selects its own project through that project''s tag_pattern. There is deliberately no allow-nested input — a vump.toml nested below another verifies the wrong project, and this action failing is that being caught rather than a limitation.' + required: false + default: '' vump-version: description: 'vump release tag to download (e.g. v0.2.0). Defaults to "latest".' required: false @@ -25,9 +29,23 @@ runs: - name: Run vump check shell: bash + # Inputs reach the script through the environment rather than being + # interpolated into it: an expression expanded into a shell line is + # expanded before bash sees it, so a value containing shell syntax would + # run as part of this step. + env: + VUMP_VERSION_TO_CHECK: ${{ inputs.version }} + VUMP_CONFIG: ${{ inputs.config }} + VUMP_PROJECT: ${{ inputs.project }} + VUMP_WORKSPACE: ${{ github.workspace }} run: | + set -euo pipefail + # cd to the directory containing vump.toml so it is found automatically. - CONFIG_DIR=$(dirname "${{ github.workspace }}/${{ inputs.config }}") - cd "$CONFIG_DIR" - "$VUMP_BIN" check "${{ inputs.version }}" + cd "$(dirname "$VUMP_WORKSPACE/$VUMP_CONFIG")" + if [ -n "$VUMP_PROJECT" ]; then + "$VUMP_BIN" check "$VUMP_VERSION_TO_CHECK" --project "$VUMP_PROJECT" + else + "$VUMP_BIN" check "$VUMP_VERSION_TO_CHECK" + fi diff --git a/BACKLOG.md b/BACKLOG.md index 358ebdc..deec085 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -55,18 +55,6 @@ The reason not to rush: a crate inside this repository's tree is not inert the way an npm or C# project is. It would need excluding from the workspace, and a mistake there breaks `cargo build` for the tool itself. -### Inputs on the check action - -The composite action takes `version`, `config` and `vump-version`. Passing a -tag now selects its own project, so a `project` input is only needed for a -repository that verifies bare versions rather than tags. - -There is no `allow-nested` input either, so a repository whose `config` input -points at a nested `vump.toml` now fails in CI. That is arguably the right -outcome — it surfaces the layout rather than verifying quietly against the -wrong project — but it is a consequence that was not chosen deliberately, and -the alternative is one input. - ### Pushing what a bump created, separately A bump that stops at `through = "tag"` leaves a commit and a tag to push by From 9c231a48cbed61217827b805d0118c35623cbc53 Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:04:20 -0600 Subject: [PATCH 4/7] feat: add `vump push` to finish a run that stopped at a tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finishing by hand meant `git push --follow-tags`, which carries every annotated tag reachable from HEAD. In a repository tagging several projects independently that publishes whichever other project's tag was sitting unpushed — the release workflow on the far end cannot tell the difference. `vump push` names one ref. It stores nothing: the tag comes from the version the files already record and the project's own pattern, which is the same derivation the bump used to create it, so it cannot name a tag other than the one that exists. Where that tag does not exist it refuses rather than pushing the branch alone, which would look like the job was done. The branch policy is checked here too. A bump stopping at `tag` was never checked against it, because nothing had left the machine; this is the step that would put the tag on the remote. The hint printed after tagging now names this command instead of the two git commands it replaces. Reading one agreed version from the tracked files was inline in `bump`, and is now shared rather than written twice. Co-Authored-By: Claude Opus 5 --- BACKLOG.md | 21 ---- DESIGN.md | 16 ++- README.md | 1 + src/adapters/git.rs | 8 ++ src/adapters/memory_vcs.rs | 17 +++ src/app/bump.rs | 21 +--- src/app/mod.rs | 27 +++++ src/app/push.rs | 206 +++++++++++++++++++++++++++++++++++++ src/cli/mod.rs | 50 +++++++++ src/cli/render.rs | 28 ++++- src/ports.rs | 7 ++ tests/cli.rs | 91 ++++++++++++++++ 12 files changed, 450 insertions(+), 43 deletions(-) create mode 100644 src/app/push.rs diff --git a/BACKLOG.md b/BACKLOG.md index deec085..3b04983 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -55,27 +55,6 @@ The reason not to rush: a crate inside this repository's tree is not inert the way an npm or C# project is. It would need excluding from the workspace, and a mistake there breaks `cargo build` for the tool itself. -### Pushing what a bump created, separately - -A bump that stops at `through = "tag"` leaves a commit and a tag to push by -hand. `git push --follow-tags` does it, but pushes *every* annotated tag -reachable — in a monorepo that can publish another project's tag that happened -to be sitting unpushed. vump's own push names the single ref it created, which -is strictly narrower. - -A `vump push` would close that gap: read the current version, render the -project's tag pattern, push `HEAD` and that one tag. It needs no stored state, -since both are derivable from the files and the pattern. - -Undecided because the repository that would feel it — a monorepo with several -projects tagged independently — does not exist here yet, and the single-project -case is served by `git push --follow-tags` today. - -Not to be confused with a prompt between tagging and pushing, which was -considered and rejected: a blocking prompt owns the terminal, so it cannot -deliver the inspection it appears to offer. Stopping at `tag` and looking -around with a free shell is strictly better. - ### Deciding the bump from commit messages semantic-release and release-please read Conventional Commits — `feat:`, `fix:`, diff --git a/DESIGN.md b/DESIGN.md index 8dfdd2e..ddcdee6 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -526,6 +526,7 @@ vump Interactive bump vump patch|minor|major Bump a stable version vump alpha|beta|rc Start or advance a pre-release (--toward required from stable) vump release Drop the pre-release suffix +vump push Push the commit and tag a bump already created vump set Write an exact version to every tracked file vump check Verify tracked files match the given version vump status Report current versions and sync state @@ -539,7 +540,7 @@ vump self list List published releases | Flag | Applies to | Description | | ------------------- | --------------- | ---------------------------------------------- | -| `--dry-run` | bump commands | Compute and report the plan, write nothing | +| `--dry-run` | bump commands, `push` | Compute and report the plan, write nothing | | `--toward ` | pre-release | Stable release the pre-release leads to | | `--project ` | all | Select a project in a multi-project repository | | `--through ` | bump commands | How far to carry the release: `none`, `commit`, `tag`, `push` | @@ -563,6 +564,19 @@ one invocation. `--to` takes a version when updating the binary; a bump command's `--toward` takes a bump name. Reading a name as reserved across the whole surface is a mistake — the "Applies to" column is what settles it. +**`vump push` finishes a run that stopped at `tag`.** It renders the tag from +the version the files already record and the project's own pattern — the same +derivation the bump used to create it — so it stores nothing and cannot name a +different ref than the one that exists. It refuses when that tag does not +exist rather than pushing the branch alone, which would look like the job was +done. The branch policy is checked here as well: a bump stopping at `tag` was +never checked against it, because nothing had left the machine yet. + +The alternative it replaces is `git push --follow-tags`, which pushes every +annotated tag reachable from `HEAD` — in a repository tagging several projects +independently, that publishes whichever other project's tag was sitting +unpushed. + Only files declared in configuration are staged. If the run reaches a commit, a dirty working tree is a hard failure — a version bump must not sweep unrelated changes into its commit. diff --git a/README.md b/README.md index 3a86f12..21e4d5f 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ needs, or it fails saying what is missing. | `vump patch\|minor\|major` | Bump a stable version | | `vump alpha\|beta\|rc` | Start or advance a pre-release | | `vump release` | Drop the pre-release suffix | +| `vump push` | Push the commit and the one tag a bump already created | | `vump set ` | Write an exact version to every tracked file | | `vump check ` | Verify tracked files record this version | | `vump status` | Report recorded versions and whether they agree | diff --git a/src/adapters/git.rs b/src/adapters/git.rs index 69c2ecd..fd3c0cf 100644 --- a/src/adapters/git.rs +++ b/src/adapters/git.rs @@ -94,6 +94,14 @@ impl Vcs for GitCli { Ok(()) } + fn has_tag(&self, name: &str) -> Result { + // `tag --list ` prints the name when it exists and nothing when + // it does not, succeeding either way — unlike `rev-parse`, which fails + // and would be indistinguishable from the repository being unreadable. + let out = self.run("tag", &["tag", "--list", name])?; + Ok(!out.trim().is_empty()) + } + fn push(&self, tag: Option<&str>) -> Result<(), VcsError> { self.run("push", &["push"])?; if let Some(tag) = tag { diff --git a/src/adapters/memory_vcs.rs b/src/adapters/memory_vcs.rs index 0da6b98..4964619 100644 --- a/src/adapters/memory_vcs.rs +++ b/src/adapters/memory_vcs.rs @@ -20,6 +20,8 @@ pub enum VcsCall { Commit(String), /// A tag was created with this name. Tag(String, Option), + /// A tag of this name was looked for. + HasTag(String), /// A push was performed, optionally including this tag. Push(Option), } @@ -31,6 +33,7 @@ pub struct MemoryVcs { calls: Mutex>, failing: Mutex>, branch: Mutex>, + tags: Mutex>, } impl Default for MemoryVcs { @@ -45,6 +48,7 @@ impl Default for MemoryVcs { calls: Mutex::default(), failing: Mutex::default(), branch: Mutex::new(Some("main".to_owned())), + tags: Mutex::default(), } } } @@ -56,6 +60,13 @@ impl MemoryVcs { Self::default() } + /// Gives the repository these tags, as though they had been created. + #[must_use] + pub fn with_tags(self, names: &[&str]) -> Self { + *lock(&self.tags) = names.iter().map(|n| (*n).to_owned()).collect(); + self + } + /// Places `HEAD` on `name`. #[must_use] pub fn on_branch(self, name: &str) -> Self { @@ -144,6 +155,12 @@ impl Vcs for MemoryVcs { Ok(()) } + fn has_tag(&self, name: &str) -> Result { + self.record(VcsCall::HasTag(name.to_owned())); + self.guard("tag")?; + Ok(lock(&self.tags).iter().any(|t| t == name)) + } + fn push(&self, tag: Option<&str>) -> Result<(), VcsError> { self.guard("push")?; self.record(VcsCall::Push(tag.map(ToOwned::to_owned))); diff --git a/src/app/bump.rs b/src/app/bump.rs index 588b5b3..5caf379 100644 --- a/src/app/bump.rs +++ b/src/app/bump.rs @@ -74,26 +74,7 @@ pub fn plan_from( let from = if let Some(base) = base { base } else { - let mut distinct: Vec<&Version> = Vec::new(); - for file in &files { - if !distinct.contains(&&file.version) { - distinct.push(&file.version); - } - } - - match distinct.as_slice() { - [only] => (*only).clone(), - // A source of truth contradicting itself is not something to guess - // about. `vump set` is the deliberate repair. - _ => { - return Err(ChangeError::OutOfSync { - found: files - .iter() - .map(|f| (f.path.clone(), f.version.clone())) - .collect(), - }); - } - } + crate::app::agreed_version(&files).map_err(|found| ChangeError::OutOfSync { found })? }; let target = transition_apply(&from, transition)?; diff --git a/src/app/mod.rs b/src/app/mod.rs index 527fa4f..23ab959 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -9,6 +9,7 @@ pub mod change; pub mod check; pub mod guided; pub mod init; +pub mod push; pub mod set; pub mod status; pub mod update; @@ -169,6 +170,32 @@ pub fn read_project_versions( Ok(versions) } +/// The one version every file records, or the disagreement that prevents it. +/// +/// Returns the evidence rather than an error type: the callers report a +/// disagreement differently — a bump refuses, a push refuses, `status` renders +/// it — and the shape of the finding is the only part they share. +/// +/// # Errors +/// +/// Returns every file and the version it records when they do not all agree. +pub fn agreed_version(files: &[FileVersion]) -> Result> { + let mut distinct: Vec<&Version> = Vec::new(); + for file in files { + if !distinct.contains(&&file.version) { + distinct.push(&file.version); + } + } + + match distinct.as_slice() { + [only] => Ok((*only).clone()), + _ => Err(files + .iter() + .map(|f| (f.path.clone(), f.version.clone())) + .collect()), + } +} + /// The package names this project's manifests declare. /// /// A shared workspace lock records every member it builds, so the entries a diff --git a/src/app/push.rs b/src/app/push.rs new file mode 100644 index 0000000..13860ff --- /dev/null +++ b/src/app/push.rs @@ -0,0 +1,206 @@ +//! Pushing what a bump already created. +//! +//! A bump stopping at `tag` leaves a commit and a tag behind, and finishing by +//! hand with `git push --follow-tags` pushes every annotated tag reachable +//! from `HEAD` — in a repository tagging several projects independently, that +//! publishes whichever other project's tag happened to be sitting unpushed. +//! Naming the one ref is the whole point of this command. +//! +//! It stores nothing. The tag is rendered from the version the tracked files +//! already record and the project's own pattern, which is the same derivation +//! the bump used to create it. + +use std::path::Path; + +use semver::Version; +use thiserror::Error; + +use crate::app::{self, AppError}; +use crate::config::{Config, ConfigError, Project}; +use crate::ports::{FileSystem, Vcs, VcsError}; + +/// What a push did, or would do. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Pushed { + /// The version the tracked files record. + pub version: Version, + /// The tag pushed alongside the branch. + pub tag: String, +} + +/// Why a push could not be performed. +#[derive(Debug, Error)] +pub enum PushError { + /// The configuration could not be read or the project not selected. + #[error("{0}")] + Config(#[from] ConfigError), + + /// The tracked files could not be read. + #[error("{0}")] + App(#[from] AppError), + + /// Tracked files record different versions, so no single tag follows. + #[error("tracked files disagree about the current version:\n{}", .found.iter().map(|(p, v)| format!(" {p} {v}")).collect::>().join("\n"))] + OutOfSync { + /// Each file and the version it records. + found: Vec<(String, Version)>, + }, + + /// The tag the files imply does not exist, so there is nothing to push. + #[error( + "no tag {tag} exists for version {version}; run a bump that reaches \ + `tag` first, or push the branch with git if that is all you meant" + )] + NoTag { + /// The tag that was looked for. + tag: String, + /// The version the tracked files record. + version: Version, + }, + + /// Git refused the push. + #[error("{0}")] + Vcs(#[from] VcsError), +} + +/// Renders the tag the tracked files imply, without pushing anything. +/// +/// # Errors +/// +/// Returns a [`PushError`] when the files cannot be read, disagree, or the tag +/// they imply does not exist locally. +pub fn plan( + fs: &dyn FileSystem, + vcs: &dyn Vcs, + root: &Path, + config: &Config, + project: &Project, +) -> Result { + let files = app::read_project_versions(fs, root, project)?; + let version = app::agreed_version(&files).map_err(|found| PushError::OutOfSync { found })?; + let tag = config.tag_pattern_for(project)?.render(&version); + + // Refusing here rather than pushing the branch alone: a caller asking for + // this command wants the tag moved, and a push that silently carried only + // half of that would look like it had done the job. + if !vcs.has_tag(&tag)? { + return Err(PushError::NoTag { tag, version }); + } + + Ok(Pushed { version, tag }) +} + +/// Pushes `HEAD` and the one tag the tracked files imply. +/// +/// # Errors +/// +/// Returns a [`PushError`] when the plan cannot be formed or the push fails. +pub fn push( + fs: &dyn FileSystem, + vcs: &dyn Vcs, + root: &Path, + config: &Config, + project: &Project, +) -> Result { + let pushed = plan(fs, vcs, root, config, project)?; + vcs.push(Some(&pushed.tag))?; + Ok(pushed) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::adapters::{MemoryFileSystem, MemoryVcs, VcsCall}; + + fn config(pattern: &str) -> Config { + crate::config::Config::parse( + Path::new("/repo/vump.toml"), + &format!("files = [\"VERSION\"]\n\n[git]\ntag_pattern = \"{pattern}\"\n"), + ) + .expect("fixture configuration parses") + } + + fn files(version: &str) -> MemoryFileSystem { + let fs = MemoryFileSystem::new(); + fs.insert("/repo/VERSION", format!("{version}\n")); + fs + } + + #[test] + fn pushes_only_the_tag_the_files_imply() { + // The point of the command: `git push --follow-tags` would also carry + // the second tag, which belongs to nothing this project released. + let fs = files("1.0.1"); + let vcs = MemoryVcs::new().with_tags(&["api-v1.0.1", "other-project-v9.9.9"]); + let config = config("api-v{new_version}"); + let project = config.select(None).expect("single project"); + + let pushed = push(&fs, &vcs, Path::new("/repo"), &config, project).expect("push succeeds"); + + assert_eq!(pushed.tag, "api-v1.0.1"); + assert!( + vcs.calls() + .contains(&VcsCall::Push(Some("api-v1.0.1".to_owned()))), + "{:?}", + vcs.calls() + ); + assert!( + !vcs.calls() + .iter() + .any(|c| matches!(c, VcsCall::Push(Some(t)) if t.contains("other-project"))), + "{:?}", + vcs.calls() + ); + } + + #[test] + fn a_tag_that_was_never_made_is_refused_before_pushing() { + // Pushing the branch alone would look like the job was done. + let fs = files("1.0.1"); + let vcs = MemoryVcs::new(); + let config = config("v{new_version}"); + let project = config.select(None).expect("single project"); + + let err = push(&fs, &vcs, Path::new("/repo"), &config, project).unwrap_err(); + assert!(matches!(err, PushError::NoTag { .. }), "{err:?}"); + assert!( + !vcs.calls().iter().any(|c| matches!(c, VcsCall::Push(_))), + "{:?}", + vcs.calls() + ); + } + + #[test] + fn planning_pushes_nothing() { + let fs = files("1.0.1"); + let vcs = MemoryVcs::new().with_tags(&["v1.0.1"]); + let config = config("v{new_version}"); + let project = config.select(None).expect("single project"); + + let planned = plan(&fs, &vcs, Path::new("/repo"), &config, project).expect("plan succeeds"); + + assert_eq!(planned.tag, "v1.0.1"); + assert!( + !vcs.calls().iter().any(|c| matches!(c, VcsCall::Push(_))), + "{:?}", + vcs.calls() + ); + } + + #[test] + fn files_that_disagree_imply_no_single_tag() { + let fs = MemoryFileSystem::new(); + fs.insert("/repo/VERSION", "1.0.1\n"); + fs.insert("/repo/sub/VERSION", "2.0.0\n"); + let config = crate::config::Config::parse( + Path::new("/repo/vump.toml"), + "files = [\"VERSION\", \"sub/VERSION\"]\n", + ) + .expect("fixture parses"); + let project = config.select(None).expect("single project"); + let vcs = MemoryVcs::new().with_tags(&["v1.0.1", "v2.0.0"]); + + let err = push(&fs, &vcs, Path::new("/repo"), &config, project).unwrap_err(); + assert!(matches!(err, PushError::OutOfSync { .. }), "{err:?}"); + } +} diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 6605ba1..9b6b122 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -124,6 +124,18 @@ enum Command { git: GitArgs, }, + /// Push the commit and the one tag a bump already created. + /// + /// For finishing a bump that stopped at `tag`. `git push --follow-tags` + /// does the same job but pushes every annotated tag reachable from `HEAD`, + /// which in a repository tagging several projects independently can + /// publish another project's release. This names one ref. + Push { + /// Report what would be pushed without pushing it. + #[arg(long)] + dry_run: bool, + }, + /// Report the versions currently recorded, and whether they agree. Status, @@ -448,6 +460,7 @@ fn execute(cli: &Cli) -> Result { dry_run, git, } => set(&ctx, version, *dry_run, git), + Command::Push { dry_run } => push(&ctx, *dry_run), Command::Status => status(&ctx), // Handled before configuration discovery. Command::Init { .. } | Command::SelfCmd(_) => Ok(Exit::Success), @@ -742,6 +755,31 @@ fn check(ctx: &Context, version: &str) -> Result { }) } +/// Pushes the commit and the one tag a bump already created. +fn push(ctx: &Context, dry_run: bool) -> Result { + let project = ctx.config.select(ctx.project.as_deref())?; + + let vcs = GitCli::new(&ctx.root); + let planned = app::push::plan(&ctx.fs, &vcs, &ctx.root, &ctx.config, project)?; + + // The branch policy governs this as it governs a bump. A bump that stopped + // at `tag` was never checked against it — nothing had left the machine — + // and this is the step that would put the tag on the remote, so the check + // belongs here too, against the version already recorded. + if let Some(off) = + app::change::check_branch(&vcs, &planned.version, true, &ctx.config.git, ctx.branch)? + { + render::off_branch(&off, ctx.branch); + } + + if !dry_run { + app::push::push(&ctx.fs, &vcs, &ctx.root, &ctx.config, project)?; + } + + render::pushed(&planned, dry_run, ctx.json); + Ok(Exit::Success) +} + fn status(ctx: &Context) -> Result { // Selecting a project narrows the report; without one, every declared // project is reported. @@ -811,6 +849,9 @@ enum CliError { #[error("{0}")] Change(#[from] ChangeError), + #[error("{0}")] + Push(#[from] app::push::PushError), + #[error("{0}")] Transition(#[from] TransitionError), @@ -849,6 +890,15 @@ impl CliError { | Self::UnrecognizedTag { .. } => Exit::Usage, Self::Config(_) => Exit::Config, Self::App(app) => app_exit(app), + Self::Push(push) => match push { + app::push::PushError::Config(_) => Exit::Config, + app::push::PushError::App(app) => app_exit(app), + app::push::PushError::OutOfSync { .. } => Exit::OutOfSync, + // Asking to push a tag that was never made is a mistake in the + // command, not a failure of the repository. + app::push::PushError::NoTag { .. } => Exit::Usage, + app::push::PushError::Vcs(_) => Exit::Git, + }, Self::Transition(_) => Exit::InvalidTransition, // Updating touches the installation rather than a project, so // nothing about it maps onto the project-shaped codes. Neither does diff --git a/src/cli/render.rs b/src/cli/render.rs index 831dcb0..2de5503 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -11,6 +11,7 @@ use serde_json::{Value, json}; use crate::app::bump::BumpPlan; use crate::app::change::{BranchCheck, ChangeSet, OffBranch, Outcome, TagPlan}; use crate::app::check::CheckReport; +use crate::app::push::Pushed; use crate::app::status::ProjectStatus; use crate::app::update::{Channel, Listing, Release, UpdateOutcome}; use crate::cli::exit::Exit; @@ -344,9 +345,15 @@ fn tag_note(tag: &TagPlan) -> &'static str { } } +/// The command that finishes a run which stopped short of pushing. +/// +/// `vump push` where a tag was made: it derives the same single ref from the +/// files and the pattern, so the suggestion cannot name the wrong one, and it +/// is shorter than the two git commands it replaces. Deliberately not +/// `--follow-tags`, which would carry every other reachable annotated tag. fn push_command(tag: Option<&str>) -> String { match tag { - Some(tag) => format!("git push && git push origin {tag}"), + Some(_) => "vump push".to_owned(), None => "git push".to_owned(), } } @@ -549,3 +556,22 @@ fn print(value: &Value) { Err(e) => eprintln!("error: cannot render JSON output: {e}"), } } + +/// Reports the commit and tag a push carried, or would carry. +pub fn pushed(pushed: &Pushed, dry_run: bool, json: bool) { + if json { + print(&serde_json::json!({ + "action": if dry_run { "push_plan" } else { "pushed" }, + "version": pushed.version.to_string(), + "tag": pushed.tag, + })); + return; + } + + if dry_run { + println!("DRY would push HEAD and {}", pushed.tag); + println!("\nNothing was pushed."); + } else { + println!("OK pushed HEAD and {}", pushed.tag); + } +} diff --git a/src/ports.rs b/src/ports.rs index 878e6d1..08f59ad 100644 --- a/src/ports.rs +++ b/src/ports.rs @@ -219,6 +219,13 @@ pub trait Vcs { /// of that name already exists. fn tag(&self, name: &str, annotation: Option<&Annotation>) -> Result<(), VcsError>; + /// Whether a tag of this name exists locally. + /// + /// # Errors + /// + /// Returns [`VcsError`] when the repository cannot be inspected. + fn has_tag(&self, name: &str) -> Result; + /// Pushes the current branch, and `tag` when one is given. /// /// # Errors diff --git a/tests/cli.rs b/tests/cli.rs index 4165ccc..19557a3 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -142,6 +142,46 @@ impl Fixture { self } + /// Creates a git repository with an initial commit and a bare remote. + /// + /// The remote is what makes a push assertable: the test can ask which refs + /// actually arrived, rather than trusting that vump named the right one. + /// It lives inside the work tree for want of a second directory, so it is + /// excluded — otherwise every bump would refuse on a dirty tree. + fn with_remote(self) -> Self { + let this = self.with_git(); + std::fs::write(this.dir.path().join(".git/info/exclude"), "remote.git/\n") + .expect("cannot write git exclude"); + + std::fs::create_dir_all(this.dir.path().join("remote.git")) + .expect("cannot create remote directory"); + let status = Command::new("git") + .args(["init", "-q", "--bare", "."]) + .current_dir(this.dir.path().join("remote.git")) + .status() + .expect("cannot run git"); + assert!(status.success(), "git init --bare failed"); + + this.git(&["remote", "add", "origin", "remote.git"]); + this.git(&["push", "-q", "-u", "origin", "HEAD"]); + this + } + + /// Tags present on the fixture's remote. + fn remote_tags(&self) -> Vec { + let out = Command::new("git") + .args(["--git-dir=remote.git", "tag"]) + .current_dir(self.dir.path()) + .output() + .expect("cannot list remote tags"); + String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::trim) + .filter(|line| !line.is_empty()) + .map(ToOwned::to_owned) + .collect() + } + /// Creates a git repository with an initial commit. fn with_git(self) -> Self { self.git(&["init", "-q", "."]) @@ -1822,6 +1862,57 @@ fn collect_tags(value: &serde_json::Value) -> Vec { } } +#[test] +fn push_sends_one_tag_and_leaves_the_others_local() { + // `git push --follow-tags` carries every annotated tag reachable from + // HEAD, which in a repository tagging several projects can publish + // someone else's release. End to end because the ref list is git's. + let fx = Fixture::new() + .write( + "vump.toml", + "files = [\"VERSION\"]\n\n[git]\nthrough = \"tag\"\ntag_pattern = \"api-v{new_version}\"\n", + ) + .write("VERSION", "1.0.0\n") + .with_remote(); + fx.git(&["tag", "-a", "other-project-v9.9.9", "-m", "another release"]); + + let bump = fx.run(&["patch"]); + assert_eq!(bump.code, 0, "{}", bump.output()); + assert!( + fx.tags().iter().any(|t| t == "api-v1.0.1"), + "{:?}", + fx.tags() + ); + + // The hint printed after tagging is the command being tested. + assert!(bump.stdout.contains("vump push"), "{}", bump.stdout); + + let dry = fx.run(&["push", "--dry-run"]); + assert_eq!(dry.code, 0, "{}", dry.output()); + assert!(dry.stdout.contains("api-v1.0.1"), "{}", dry.stdout); + assert!( + fx.remote_tags().is_empty(), + "a dry run pushed: {:?}", + fx.remote_tags() + ); + + let push = fx.run(&["push"]); + assert_eq!(push.code, 0, "{}", push.output()); + assert_eq!(fx.remote_tags(), vec!["api-v1.0.1".to_owned()]); +} + +#[test] +fn push_refuses_when_no_tag_was_made() { + let fx = Fixture::new() + .write("vump.toml", "files = [\"VERSION\"]\n") + .write("VERSION", "1.0.0\n") + .with_remote(); + + let run = fx.run(&["push"]); + assert_eq!(run.code, 2, "{}", run.output()); + assert!(run.stderr.contains("no tag"), "{}", run.stderr); +} + // ─── Every supported format, through the real binary ──────────────────────── /// One supported layout: the files a project of that shape has, and what From 6d07a6aa4a483194c27c85819de11cedb833fa22 Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:05:33 -0600 Subject: [PATCH 5/7] feat: add a Cargo workspace sandbox, and correct the entry that deferred it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The entry said a crate in this tree would need excluding from the workspace, and that a mistake there breaks `cargo build` for the tool itself. Neither holds: this repository's Cargo.toml declares no [workspace], so it is its own single member and a nested workspace is simply a separate one. `cargo metadata` still reports one member with the sandbox in place, and `publish = false` makes the packaged-contents question moot too. What it was right about is that the shape worth showing had no worked example. The repository demonstrates a single crate; a workspace — one lock entry per member, matched by the name each manifest declares — existed only in test fixtures. Two crates released together now show it, and `--locked` is the assertion that vump wrote what cargo expects rather than something cargo would rewrite. Co-Authored-By: Claude Opus 5 --- .gitignore | 1 + BACKLOG.md | 16 --------------- sandbox/README.md | 11 ++++++++++ sandbox/rs/workspace/Cargo.lock | 14 +++++++++++++ sandbox/rs/workspace/Cargo.toml | 5 +++++ sandbox/rs/workspace/crates/cli/Cargo.toml | 8 ++++++++ sandbox/rs/workspace/crates/cli/src/main.rs | 5 +++++ sandbox/rs/workspace/crates/engine/Cargo.toml | 5 +++++ sandbox/rs/workspace/crates/engine/src/lib.rs | 1 + sandbox/rs/workspace/vump.toml | 20 +++++++++++++++++++ tests/cli.rs | 2 ++ 11 files changed, 72 insertions(+), 16 deletions(-) create mode 100644 sandbox/rs/workspace/Cargo.lock create mode 100644 sandbox/rs/workspace/Cargo.toml create mode 100644 sandbox/rs/workspace/crates/cli/Cargo.toml create mode 100644 sandbox/rs/workspace/crates/cli/src/main.rs create mode 100644 sandbox/rs/workspace/crates/engine/Cargo.toml create mode 100644 sandbox/rs/workspace/crates/engine/src/lib.rs create mode 100644 sandbox/rs/workspace/vump.toml diff --git a/.gitignore b/.gitignore index 08ae3a7..b62be1e 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ sandbox/**/obj/ sandbox/**/node_modules/ sandbox/**/.venv/ sandbox/**/__pycache__/ +sandbox/**/target/ diff --git a/BACKLOG.md b/BACKLOG.md index 3b04983..887d66b 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -39,22 +39,6 @@ permanently to avoid work that keeps turning out to be small. What would settle it: a format someone needs that is not worth adding built-in. -### A Rust project in the sandbox - -The sandbox covers npm and C#. Rust is missing, and the argument at the time -was that this repository is itself the Cargo example — it tracks `Cargo.toml` -and `Cargo.lock` and is exercised on every release. - -That argument is weaker than it looked. The repository demonstrates a -single-crate project only, so the shape actually worth showing by hand — a -workspace, where the lock holds one entry per member and a project writes only -the entries its own manifests name — has no worked example anywhere outside the -test fixtures. - -The reason not to rush: a crate inside this repository's tree is not inert the -way an npm or C# project is. It would need excluding from the workspace, and a -mistake there breaks `cargo build` for the tool itself. - ### Deciding the bump from commit messages semantic-release and release-please read Conventional Commits — `feat:`, `fix:`, diff --git a/sandbox/README.md b/sandbox/README.md index f3d205a..84c79ec 100644 --- a/sandbox/README.md +++ b/sandbox/README.md @@ -17,6 +17,7 @@ This is not test coverage. The suite in `tests/` and the fixtures in | [`cs/multi-project`](cs/multi-project) | Two C# projects versioned independently | | [`cs/shared-version`](cs/shared-version) | An executable and its library on one `Directory.Build.props` | | [`py/single-project`](py/single-project) | One Python project, `pyproject.toml` and `uv.lock` together | +| [`rs/workspace`](rs/workspace) | Two crates sharing one `Cargo.lock`, released together | ## Git is off, deliberately @@ -100,6 +101,16 @@ vump minor --allow-nested # pyproject.toml and uv.lock uv run python -m demo # demo 1.1.0 ``` +A Cargo workspace shares one lock holding an entry per member, matched by the +name each manifest declares. Declaring both manifests is what moves them as a +unit — `--locked` is the check that vump wrote what cargo expects: + +```bash +cd sandbox/rs/workspace +vump minor --allow-nested # both manifests and the shared lock +cargo run --locked --bin sandbox-cli # sandbox-cli 1.1.0 +``` + ## Putting it back Experiments are meant to be thrown away: diff --git a/sandbox/rs/workspace/Cargo.lock b/sandbox/rs/workspace/Cargo.lock new file mode 100644 index 0000000..b5dd9b1 --- /dev/null +++ b/sandbox/rs/workspace/Cargo.lock @@ -0,0 +1,14 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "sandbox-cli" +version = "1.0.0" +dependencies = [ + "sandbox-engine", +] + +[[package]] +name = "sandbox-engine" +version = "1.0.0" diff --git a/sandbox/rs/workspace/Cargo.toml b/sandbox/rs/workspace/Cargo.toml new file mode 100644 index 0000000..bb2f867 --- /dev/null +++ b/sandbox/rs/workspace/Cargo.toml @@ -0,0 +1,5 @@ +# A virtual workspace manifest: it declares members and no package of its own, +# so there is no version here for vump to track. The members carry those. +[workspace] +resolver = "3" +members = ["crates/*"] diff --git a/sandbox/rs/workspace/crates/cli/Cargo.toml b/sandbox/rs/workspace/crates/cli/Cargo.toml new file mode 100644 index 0000000..96c5e84 --- /dev/null +++ b/sandbox/rs/workspace/crates/cli/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "sandbox-cli" +version = "1.0.0" +edition = "2024" +publish = false + +[dependencies] +sandbox-engine = { path = "../engine" } diff --git a/sandbox/rs/workspace/crates/cli/src/main.rs b/sandbox/rs/workspace/crates/cli/src/main.rs new file mode 100644 index 0000000..1a6be77 --- /dev/null +++ b/sandbox/rs/workspace/crates/cli/src/main.rs @@ -0,0 +1,5 @@ +// Both crates are released together, so one version describes the pair. +fn main() { + println!("sandbox-cli {}", env!("CARGO_PKG_VERSION")); + println!("{}", sandbox_engine::greeting()); +} diff --git a/sandbox/rs/workspace/crates/engine/Cargo.toml b/sandbox/rs/workspace/crates/engine/Cargo.toml new file mode 100644 index 0000000..d790526 --- /dev/null +++ b/sandbox/rs/workspace/crates/engine/Cargo.toml @@ -0,0 +1,5 @@ +[package] +name = "sandbox-engine" +version = "1.0.0" +edition = "2024" +publish = false diff --git a/sandbox/rs/workspace/crates/engine/src/lib.rs b/sandbox/rs/workspace/crates/engine/src/lib.rs new file mode 100644 index 0000000..6729bfb --- /dev/null +++ b/sandbox/rs/workspace/crates/engine/src/lib.rs @@ -0,0 +1 @@ +pub fn greeting() -> &'static str { "hello from the engine" } diff --git a/sandbox/rs/workspace/vump.toml b/sandbox/rs/workspace/vump.toml new file mode 100644 index 0000000..1574cfc --- /dev/null +++ b/sandbox/rs/workspace/vump.toml @@ -0,0 +1,20 @@ +# Two crates in one workspace, released together as a single version. +# +# The workspace shares one Cargo.lock holding an entry per member, matched by +# the name each manifest declares — so a project writes only the entries its +# own manifests name, and members nobody declared are left alone. Declaring +# every manifest here is what makes these two move as a unit. +# +# Git is off throughout the sandbox, and the tag pattern cannot be mistaken +# for vump's own — see ../../npm/single-project/vump.toml. + +files = [ + "crates/engine/Cargo.toml", + "crates/cli/Cargo.toml", + "Cargo.lock", +] + +[git] +through = "none" +tag_pattern = "sandbox-rs-v{new_version}" +commit_message = "chore(sandbox): bump rs/workspace to {new_version}" diff --git a/tests/cli.rs b/tests/cli.rs index 19557a3..efcb0ce 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1770,6 +1770,7 @@ fn the_sandbox_projects_stay_usable() { "cs/multi-project", "cs/shared-version", "py/single-project", + "rs/workspace", ] { let output = Command::new(env!("CARGO_BIN_EXE_vump")) .args(["status", "--allow-nested"]) @@ -1806,6 +1807,7 @@ fn no_sandbox_project_can_produce_a_release_shaped_tag() { ("cs/multi-project", Some("project-b")), ("cs/shared-version", None), ("py/single-project", None), + ("rs/workspace", None), ] { // --allow-nested is the point of this test: it asks for the tag a // nested configuration would create, precisely to inspect its shape. From 7a4614f3575a6f83f8f7582a4912750c837e011e Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:07:11 -0600 Subject: [PATCH 6/7] test: ask each ecosystem's own toolchain what version it builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite proves vump writes the bytes it meant to. Whether those bytes reach a running program is a question only dotnet, uv, npm and cargo can answer, and it was being answered by hand once per format. Deliberately a script rather than a test. Putting it in `cargo test` would put four toolchains and a network on three CI runners, making every job slower and flakier in order to test somebody else's tool — and the formats it exercises are already covered deterministically by the table in tests/cli.rs. A missing toolchain is skipped rather than failed, every project is reverted on the way out including after a failure, and a bump that fails is recorded as a failure rather than skipping the assertion that followed it — which the first version of this script did, reporting success while every step had failed. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 6 +++ sandbox/README.md | 15 +++++++ scripts/sandbox-smoke.sh | 86 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100755 scripts/sandbox-smoke.sh diff --git a/CLAUDE.md b/CLAUDE.md index a52ce25..02c93c1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,12 @@ cargo test CI runs exactly this on Linux, macOS and Windows. Running it locally first is much faster than waiting for a runner to report the same thing. +`./scripts/sandbox-smoke.sh` is the separate check: it bumps every sandbox and +asks each ecosystem's own toolchain what version it builds, which is the one +thing the suite cannot answer. It needs node, dotnet, uv and cargo, skips what +is missing, and is not part of CI for that reason — run it when a format's +reading or writing changed. + ## Working agreement - **Open a pull request when a coherent set of work is done, then pause for diff --git a/sandbox/README.md b/sandbox/README.md index 84c79ec..cbde30d 100644 --- a/sandbox/README.md +++ b/sandbox/README.md @@ -111,6 +111,21 @@ vump minor --allow-nested # both manifests and the shared lock cargo run --locked --bin sandbox-cli # sandbox-cli 1.1.0 ``` +## Checking every ecosystem at once + +The test suite proves vump writes the bytes it meant to. Only each ecosystem's +own toolchain can say whether those bytes reach a running program: + +```bash +./scripts/sandbox-smoke.sh +``` + +It bumps every project with the vump in this working tree, asks node, dotnet, +uv and cargo what version they build, and reverts afterwards. A toolchain that +is not installed is skipped rather than failed. It is deliberately not part of +`cargo test`, which would need all four on every CI runner to test somebody +else's tool. + ## Putting it back Experiments are meant to be thrown away: diff --git a/scripts/sandbox-smoke.sh b/scripts/sandbox-smoke.sh new file mode 100755 index 0000000..dd4038d --- /dev/null +++ b/scripts/sandbox-smoke.sh @@ -0,0 +1,86 @@ +#!/usr/bin/env bash +# +# Bumps every sandbox project with the vump in this working tree, then asks +# each ecosystem's own toolchain what version it ends up building. +# +# This is the one thing `cargo test` cannot answer. The test suite proves vump +# writes the bytes it meant to; only dotnet, uv, npm and cargo can say whether +# those bytes reach a running program. It is deliberately not part of `cargo +# test`: it needs four toolchains and a network, which would make every CI job +# slower and flakier in order to test somebody else's tool. +# +# Every project is reverted afterwards, including on failure. An ecosystem +# whose toolchain is missing is reported and skipped, not failed. +set -uo pipefail + +cd "$(dirname "$0")/.." +ROOT=$(pwd) +VUMP="$ROOT/target/debug/vump" +FAILED=() +SKIPPED=() + +cleanup() { + git checkout -- sandbox 2>/dev/null +} +trap cleanup EXIT + +echo "building vump" +cargo build -q || { echo "cannot build vump"; exit 1; } + +# check +# +# Skips when the toolchain is absent. Otherwise bumps the project and runs the +# command in it, failing when either step fails or the version does not appear. +check() { + local tool=$1 project=$2 wanted=$3 + shift 3 + + echo + echo "$project" + if ! command -v "$tool" >/dev/null 2>&1; then + echo " skip ($tool not installed)" + SKIPPED+=("$project") + return + fi + + local dir="$ROOT/sandbox/$project" + if ! ( cd "$dir" && "$VUMP" minor --allow-nested >/dev/null ); then + echo " FAIL the bump itself failed" + FAILED+=("$project (bump)") + return + fi + + local output + if ! output=$( cd "$dir" && "$@" 2>&1 ); then + echo " FAIL $tool could not build or run it:" + printf '%s\n' "$output" | sed 's/^/ /' + FAILED+=("$project ($tool)") + return + fi + + if printf '%s' "$output" | grep -qF "$wanted"; then + echo " ok $tool reports $wanted" + else + echo " FAIL $tool does not report $wanted:" + printf '%s\n' "$output" | sed 's/^/ /' + FAILED+=("$project (version)") + fi +} + +check node npm/single-project 1.1.0 node index.js +check dotnet cs/single-project 1.1.0 dotnet run --project Demo +check dotnet cs/shared-version 1.1.0 dotnet run --project App +check uv py/single-project 1.1.0 uv run --quiet python -m demo +# --locked is the assertion: cargo refuses a lock it would have rewritten, so a +# passing build proves vump's edit matches what cargo expects. +check cargo rs/workspace 1.1.0 cargo run -q --locked --bin sandbox-cli + +echo +if [ ${#SKIPPED[@]} -gt 0 ]; then + echo "skipped: ${SKIPPED[*]}" +fi +if [ ${#FAILED[@]} -gt 0 ]; then + echo "FAILED: ${FAILED[*]}" + exit 1 +fi +echo "every sandbox reports the version vump wrote" From 5e6b25e9bad8ca32593e6d6a33640f6f6b70c0e4 Mon Sep 17 00:00:00 2001 From: okcodes <80954089+okcodes@users.noreply.github.com> Date: Thu, 22 Oct 2026 00:16:59 -0600 Subject: [PATCH 7/7] fix: refuse to smoke-test over uncommitted sandbox work The script reverts sandbox/ when it finishes, which is a destructive git command in a real repository rather than in a temporary one. Anything sitting there unsaved went with it. Co-Authored-By: Claude Opus 5 --- scripts/sandbox-smoke.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/sandbox-smoke.sh b/scripts/sandbox-smoke.sh index dd4038d..fb78f5a 100755 --- a/scripts/sandbox-smoke.sh +++ b/scripts/sandbox-smoke.sh @@ -19,6 +19,16 @@ VUMP="$ROOT/target/debug/vump" FAILED=() SKIPPED=() +# Bumping the sandbox dirties it, so the script reverts what it did on the way +# out. That is a destructive git command in a real repository, so it refuses to +# start where there is anything there it might throw away. +if [ -n "$(git status --porcelain sandbox/)" ]; then + echo "sandbox/ has uncommitted changes, and this script reverts sandbox/ when" + echo "it finishes. Commit or stash them first:" + git status --short sandbox/ | sed 's/^/ /' + exit 1 +fi + cleanup() { git checkout -- sandbox 2>/dev/null }