From 4b677915061ea90bb84e198e54f9237e890afcf2 Mon Sep 17 00:00:00 2001 From: BryanFRD Date: Tue, 25 Aug 2026 17:11:24 +0200 Subject: [PATCH] fix(release): make pr mode tag on merge instead of on the pre-bump commit --- README.md | 17 ++++ src/monorepo/run/execute.rs | 21 ++++- src/monorepo/run/execute_tests.rs | 136 ++++++++++++++++++++++++++++++ src/monorepo/run/finalize.rs | 135 +++++++++++++++++++++++++++++ src/monorepo/run/mod.rs | 44 +++++++++- 5 files changed, 348 insertions(+), 5 deletions(-) create mode 100644 src/monorepo/run/finalize.rs diff --git a/README.md b/README.md index a262eeb7..2806c47e 100644 --- a/README.md +++ b/README.md @@ -482,6 +482,23 @@ skip_ci = false # force CI to run on release commits In `pr` mode, `skip_ci` defaults to `false` since the PR merge triggers CI naturally. +### How `pr` mode releases + +`pr` mode runs in two phases, so nothing is published for a release that has not been accepted. + +The **proposing** run computes the bump, writes the version files and changelog onto the release +branch, and opens or updates the pull request. No tags are created and no releases are published. +Every later commit on the target branch regenerates the branch, so the open PR keeps tracking the +version that would ship now. + +The **finalising** run happens after the PR merges. FerrFlow sees a `chore(release):` commit on the +target branch whose versions carry no tag, and tags exactly those versions before publishing the +releases. The versions are read from the version files, never recomputed, so merging a release PR +cannot cascade into a further bump. Squash merges and merge commits both work. + +A package declared without `versionedFiles` has no version to read, so it is not finalised this way. +Use `commit` mode for tag-only packages. + ## Floating Tags Move abbreviated tags (e.g. `v1`, `v1.2`) to always point at the latest matching release: diff --git a/src/monorepo/run/execute.rs b/src/monorepo/run/execute.rs index 23c78026..487c56c7 100644 --- a/src/monorepo/run/execute.rs +++ b/src/monorepo/run/execute.rs @@ -30,6 +30,7 @@ pub(super) struct ReleasePlan<'a> { pub force: bool, pub draft: bool, pub tags_to_create: &'a [PlannedTag], + pub finalizing: bool, pub hook_contexts: &'a [(HookContext, usize)], pub files_to_commit: &'a mut Vec, pub files_per_package: &'a mut HashMap>, @@ -44,7 +45,11 @@ pub(super) fn execute_release(plan: &mut ReleasePlan<'_>) -> Result<()> { run_pre_commit_hooks(plan)?; let files_snapshot: Vec = plan.files_to_commit.clone(); - let mode = plan.config.workspace.release_commit_mode; + let mode = if plan.finalizing { + ReleaseCommitMode::None + } else { + plan.config.workspace.release_commit_mode + }; let scope = plan.config.workspace.release_commit_scope; let release_parts: Vec = plan @@ -85,7 +90,13 @@ pub(super) fn execute_release(plan: &mut ReleasePlan<'_>) -> Result<()> { } run_package_hooks(plan, HookPoint::PostCommit)?; run_package_hooks(plan, HookPoint::PreTag)?; - if !checkpoint_is_done(plan, Phase::TagsCreated) { + if mode == ReleaseCommitMode::Pr { + // The release is only proposed at this point. Tagging here would + // label the pre-bump commit and make later runs report "nothing to + // release", which is what froze the PR before #934. Tags and + // releases are produced by the finalising run, once the release + // commit has landed on the target branch. + } else if !checkpoint_is_done(plan, Phase::TagsCreated) { create_release_tags(plan)?; create_and_move_floating_tags(plan, &mut floating_tag_names)?; checkpoint_advance(plan, Phase::TagsCreated)?; @@ -104,7 +115,9 @@ pub(super) fn execute_release(plan: &mut ReleasePlan<'_>) -> Result<()> { } else if plan.verbose { tracing::info!(" ↻ Resumed: skipping push (already done)"); } - if !checkpoint_is_done(plan, Phase::ReleasesCreated) { + if mode == ReleaseCommitMode::Pr { + // Same reason as the tags above. + } else if !checkpoint_is_done(plan, Phase::ReleasesCreated) { publish_releases(plan)?; checkpoint_advance(plan, Phase::ReleasesCreated)?; } else if plan.verbose { @@ -493,7 +506,7 @@ fn push_refs( )); } - if !tag_refs.is_empty() { + if !tag_refs.is_empty() && mode != ReleaseCommitMode::Pr { push_tags(plan.repo, &plan.config.workspace.remote, &tag_refs)?; plan.shared_outputs.push("✓ Pushed tags".to_string()); } diff --git a/src/monorepo/run/execute_tests.rs b/src/monorepo/run/execute_tests.rs index 9b6834a6..cf071e6e 100644 --- a/src/monorepo/run/execute_tests.rs +++ b/src/monorepo/run/execute_tests.rs @@ -220,6 +220,7 @@ fn run_publish_phase( let result = { let mut plan = ReleasePlan { + finalizing: false, repo: &harness.repo, config: &harness.config, root: &harness.root, @@ -315,3 +316,138 @@ fn the_pushed_tag_keeps_its_annotation() { "the annotation must survive the push, got: {message}" ); } + +fn with_package(harness: &mut Harness, name: &str) { + let config: Config = serde_json::from_str(&format!( + r#"{{"package":[{{"name":"{name}","path":"."}}]}}"# + )) + .unwrap(); + harness.config.packages = config.packages; +} + +fn run_phase( + harness: &Harness, + tags: &[PlannedTag], + forge: &dyn Forge, + finalizing: bool, +) -> (Result<()>, Vec<(String, ReleaseResult)>) { + let hook_contexts: Vec<(HookContext, usize)> = Vec::new(); + let mut files_to_commit: Vec = vec!["README.md".to_string()]; + let mut files_per_package: HashMap> = HashMap::new(); + let mut pkg_outputs: Vec<(String, Vec)> = Vec::new(); + let mut shared_outputs: Vec = Vec::new(); + let mut forge_results: Vec<(String, ReleaseResult)> = Vec::new(); + + let result = { + let mut plan = ReleasePlan { + finalizing, + repo: &harness.repo, + config: &harness.config, + root: &harness.root, + target_branch: "main", + dry_run: false, + verbose: false, + force: false, + draft: false, + tags_to_create: tags, + hook_contexts: &hook_contexts, + files_to_commit: &mut files_to_commit, + files_per_package: &mut files_per_package, + pkg_outputs: &mut pkg_outputs, + shared_outputs: &mut shared_outputs, + forge_results: &mut forge_results, + checkpoint: None, + forge: Some(forge), + }; + execute_release(&mut plan) + }; + + (result, forge_results) +} + +#[test] +fn pr_mode_proposes_without_tagging_or_publishing() { + let mut harness = Harness::new(); + harness.config.workspace.release_commit_mode = crate::config::ReleaseCommitMode::Pr; + commit_file(&harness.root, "a.txt", "a", "feat: a feature"); + + let forge = RecordingForge::default(); + let tags = vec![tag_to_create("v1.1.0", "app", "1.1.0")]; + let (result, releases) = run_phase(&harness, &tags, &forge, false); + + assert!(result.is_ok(), "{:?}", result.err()); + assert!( + harness.git(&["tag", "-l"]).trim().is_empty(), + "pr mode must not create tags while the release is only proposed" + ); + assert!( + harness.remote_tags().is_empty(), + "pr mode must not push tags while the release is only proposed" + ); + assert!( + releases.is_empty(), + "pr mode must not publish releases before the PR merges" + ); +} + +#[test] +fn finalizing_tags_and_publishes_the_merged_release() { + let mut harness = Harness::new(); + with_package(&mut harness, "app"); + harness.config.workspace.release_commit_mode = crate::config::ReleaseCommitMode::Pr; + commit_file(&harness.root, "a.txt", "a", "chore(release): app v1.1.0"); + harness.git(&["push", "origin", "main:main"]); + + let forge = RecordingForge::default(); + let tags = vec![tag_to_create("v1.1.0", "app", "1.1.0")]; + let (result, releases) = run_phase(&harness, &tags, &forge, true); + + assert!(result.is_ok(), "{:?}", result.err()); + assert_eq!(harness.remote_tags(), vec!["v1.1.0".to_string()]); + assert_eq!( + releases.iter().map(|(t, _)| t.as_str()).collect::>(), + vec!["v1.1.0"] + ); +} + +#[test] +fn finalizing_tags_the_commit_that_carries_the_bump() { + let mut harness = Harness::new(); + with_package(&mut harness, "app"); + harness.config.workspace.release_commit_mode = crate::config::ReleaseCommitMode::Pr; + commit_file(&harness.root, "a.txt", "a", "feat: a feature"); + commit_file(&harness.root, "b.txt", "b", "chore(release): app v1.1.0"); + harness.git(&["push", "origin", "main:main"]); + let head = harness.git(&["rev-parse", "HEAD"]).trim().to_string(); + + let forge = RecordingForge::default(); + let tags = vec![tag_to_create("v1.1.0", "app", "1.1.0")]; + let (result, _) = run_phase(&harness, &tags, &forge, true); + + assert!(result.is_ok(), "{:?}", result.err()); + let tagged = harness + .git(&["rev-list", "-n1", "v1.1.0"]) + .trim() + .to_string(); + assert_eq!( + tagged, head, + "the tag must land on the release commit, not one behind it" + ); +} + +#[test] +fn commit_mode_still_tags_and_publishes_in_one_pass() { + let mut harness = Harness::new(); + with_package(&mut harness, "app"); + harness.config.workspace.release_commit_mode = crate::config::ReleaseCommitMode::Commit; + commit_file(&harness.root, "a.txt", "a", "feat: a feature"); + std::fs::write(harness.root.join("README.md"), "bumped").unwrap(); + + let forge = RecordingForge::default(); + let tags = vec![tag_to_create("v1.1.0", "app", "1.1.0")]; + let (result, releases) = run_phase(&harness, &tags, &forge, false); + + assert!(result.is_ok(), "{:?}", result.err()); + assert_eq!(harness.remote_tags(), vec!["v1.1.0".to_string()]); + assert_eq!(releases.len(), 1); +} diff --git a/src/monorepo/run/finalize.rs b/src/monorepo/run/finalize.rs new file mode 100644 index 00000000..64511413 --- /dev/null +++ b/src/monorepo/run/finalize.rs @@ -0,0 +1,135 @@ +use std::path::Path; + +use crate::changelog::{ChangelogRender, GitLog, build_section_with}; +use crate::config::{Config, PackageConfig}; +use crate::formats::read_version; +use crate::git::{Repository, find_last_tag_name, get_commits_since_last_tag}; + +use super::summary::PlannedTag; + +const RELEASE_SUBJECT_PREFIX: &str = "chore(release):"; + +pub(super) fn merged_release_tags( + repo: &Repository, + config: &Config, + root: &Path, + all_tags: &[String], + forge_base: Option, +) -> Vec { + let is_monorepo = config.is_monorepo(); + let mut pending = Vec::new(); + + for pkg in &config.packages { + let Some(version) = file_version(pkg, root) else { + continue; + }; + let tag = pkg.tag_for_version(&config.workspace, is_monorepo, &version); + if all_tags.iter().any(|t| t == &tag) { + continue; + } + + let prefix = pkg.tag_prefix(&config.workspace, is_monorepo); + let strategy = config.workspace.orphaned_tag_strategy; + let last_tag = find_last_tag_name(repo, &prefix, strategy).ok().flatten(); + let skip_markers = config.workspace.effective_commit_skip_markers(); + let Ok(commits) = get_commits_since_last_tag(repo, &prefix, strategy, &skip_markers, None) + else { + continue; + }; + if !carries_release_commit(&commits) { + continue; + } + + let render = ChangelogRender { + formats: Some(&config.workspace.commit_formats), + config: config.workspace.changelog.as_ref(), + forge_base: forge_base.clone(), + last_tag: last_tag.clone(), + new_tag: Some(tag.clone()), + }; + + let is_prerelease = version_is_prerelease(&version); + pending.push(PlannedTag { + message: format!("Release {tag}"), + body: build_section_with(&version, &commits, &render), + tag, + package: pkg.name.clone(), + version, + commit_count: commits.len() as i32, + is_prerelease, + }); + } + + pending +} + +fn version_is_prerelease(version: &str) -> bool { + semver::Version::parse(version.trim_start_matches('v')) + .map(|v| !v.pre.is_empty()) + .unwrap_or(false) +} + +fn file_version(pkg: &PackageConfig, root: &Path) -> Option { + let vf = pkg.versioned_files.first()?; + read_version(vf, root).ok() +} + +fn carries_release_commit(commits: &[GitLog]) -> bool { + commits.iter().any(|c| { + c.message + .lines() + .next() + .is_some_and(|subject| subject.trim_start().starts_with(RELEASE_SUBJECT_PREFIX)) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn log(message: &str) -> GitLog { + GitLog { + hash: String::new(), + message: message.to_string(), + } + } + + #[test] + fn a_semver_prerelease_version_is_flagged_as_one() { + assert!(version_is_prerelease("1.1.0-beta.1")); + assert!(version_is_prerelease("v2.0.0-rc.3")); + assert!(!version_is_prerelease("1.1.0")); + assert!(!version_is_prerelease("2026.8.25")); + } + + #[test] + fn a_release_commit_in_the_window_is_recognised() { + assert!(carries_release_commit(&[ + log("feat: something"), + log("chore(release): app v1.1.0"), + ])); + } + + #[test] + fn a_merged_release_commit_with_a_body_is_recognised() { + assert!(carries_release_commit(&[log( + "chore(release): app v1.1.0\n\n- app 1.1.0 (3 commits)" + )])); + } + + #[test] + fn ordinary_commits_alone_do_not_trigger_finalisation() { + assert!(!carries_release_commit(&[ + log("feat: something"), + log("fix: something else"), + log("chore: unrelated housekeeping"), + ])); + } + + #[test] + fn a_commit_merely_mentioning_a_release_does_not_count() { + assert!(!carries_release_commit(&[log( + "fix: do not break chore(release): parsing" + )])); + } +} diff --git a/src/monorepo/run/mod.rs b/src/monorepo/run/mod.rs index d6fd64be..b20b52ef 100644 --- a/src/monorepo/run/mod.rs +++ b/src/monorepo/run/mod.rs @@ -29,6 +29,7 @@ mod drafts; mod execute; #[cfg(test)] mod execute_tests; +mod finalize; mod forced; pub(crate) mod graph; mod groups; @@ -179,6 +180,15 @@ pub(super) fn run_release_logic( tracing::debug!(""); } + let finalize_tags = if dry_run + || config.workspace.release_commit_mode != crate::config::ReleaseCommitMode::Pr + { + Vec::new() + } else { + finalize::merged_release_tags(&repo, config, root, &all_tags, forge_base.clone()) + }; + let finalizing = !finalize_tags.is_empty(); + let mut any_bumped = false; let mut json_packages: Vec = Vec::new(); let mut released: Vec = Vec::new(); @@ -232,7 +242,37 @@ pub(super) fn run_release_logic( let all_packages = batch_package_snapshot(&release_order, &plans, &config.packages); - for &pkg_idx in &release_order { + if finalizing { + for tag in &finalize_tags { + pkg_outputs.push(( + tag.package.clone(), + vec![format!( + "{} {} {} ({})", + "●".green().bold(), + tag.package.bold(), + tag.version.green().bold(), + "release merged, tagging now".cyan() + )], + )); + released.push(ReleasedPackage { + package: tag.package.clone(), + previous_version: String::new(), + new_version: tag.version.clone(), + bump_type: "finalize".to_string(), + tag: tag.tag.clone(), + commit_count: tag.commit_count as usize, + prerelease: tag.is_prerelease, + version_source: None, + forge_release_url: None, + forge_release_id: None, + }); + } + tags_to_create = finalize_tags; + any_bumped = true; + } + let bump_order: &[usize] = if finalizing { &[] } else { &release_order }; + + for &pkg_idx in bump_order { let pkg = &config.packages[pkg_idx]; let plan = plans[pkg_idx] .take() @@ -706,6 +746,7 @@ pub(super) fn run_release_logic( force, draft, tags_to_create: &tags_to_create, + finalizing, hook_contexts: &hook_contexts, files_to_commit: &mut files_to_commit, files_per_package: &mut files_per_package, @@ -780,6 +821,7 @@ pub(super) fn run_release_logic( force, draft, tags_to_create: &tags_to_create, + finalizing, hook_contexts: &hook_contexts, files_to_commit: &mut files_to_commit, files_per_package: &mut files_per_package,