Add Preview workflows and enforce versioning rules#1088
Open
dwcullop wants to merge 11 commits into
Open
Conversation
- release.yml triggers on push to both main and release/**
- ci-build.yml triggers on PRs to both main and release/**
- GitHub Releases conditionally marked as prerelease via NBGV.PrereleaseVersion
- Added RELEASING.md documenting the branching model and cutover sequence
This is part 1 of 2. version.json is unchanged; main continues to publish
9.4.N stable until the follow-up PR bumps it to 9.5-preview.{height}.
Order of operations:
1. Merge this PR.
2. Cut release/9.x from main HEAD (it now has correct workflows).
3. Merge the follow-up version.json bump PR to start main pre-releases.
This is part 2 of 2. After this commit, main publishes 9.5.0-preview.N on every PR merge. The release/<major>.x branches continue to publish stable packages. Do NOT merge this until release/9.x has been cut from main (with the workflows from part 1 in place).
Adds three workflow_dispatch actions that handle all version.json edits and release-branch creation, so maintainers never have to edit version.json by hand: - promote-minor.yml: promotes main to a new minor on release/<major>.x. Opens two PRs (promotion + main bump to next preview). - cut-major.yml: creates a new release/<major>.x branch at <major>.0 stable and opens a PR advancing main to the next preview. - bump-major-preview.yml: bumps main to next major preview ahead of breaking changes. Adds two passive guards: - pr-version-check.yml: blocks breaking-change PRs when main hasn't been bumped past the latest stable major. - release.yml: refuses to publish a prerelease for major.minor when a stable tag for that major.minor already exists (catches the 'main forgot to bump after promotion' regression). RELEASING.md is updated to document the automated flows and notes the GITHUB_TOKEN limitation (bot PRs don't trigger downstream CI by default).
Prevents accidentally skipping a major version (e.g., bumping from 9.x stable to 11.0-preview, leaving 10 unshipped). - pr-version-check.yml: a breaking-change PR now fails unless main's major is exactly latest_stable_major + 1. Previously allowed any major > latest_stable_major. - bump-major-preview.yml: rejects next_major inputs that aren't exactly latest_stable_major + 1. Workflow input validation makes this fail-fast at dispatch time. - RELEASING.md: notes the strict sequential-major rule. Skipping a major is still possible by editing version.json by hand (both checks reference workflow names, not version.json content).
main has shipped 9.4.37 since this branch was opened. The previously documented versionHeightOffset of 36 would produce 9.4.37 again (collision). Updated to 37 so the first build on release/9.x produces 9.4.38, continuing sequentially.
- Replace archived actions/create-release@v1.1.4 with softprops/action-gh-release@v2. The release_name input was renamed to name; GITHUB_TOKEN is now implicit via the contents:write permission on the job. - Add --skip-duplicate to dotnet nuget push so workflow re-runs are idempotent when a package version was already uploaded. - Standardize productNamespacePrefix to DynamicData (was inconsistently 'ReactiveMarbles' in release.yml vs 'DynamicData' in ci-build.yml). - Add explicit permissions: contents: write at the job level, required by softprops/action-gh-release to create the GitHub release and tag.
There was a problem hiding this comment.
Pull request overview
Adds NBGV-based preview/stable release automation, release-branch publishing, versioning guardrails, and documentation for maintainers.
Changes:
- Updates versioning to publish previews from
mainand stable packages fromrelease/**. - Adds workflow-dispatch automation for promoting minors, cutting majors, and bumping major previews.
- Adds PR/release guards plus releasing documentation.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
version.json |
Moves main to 9.5-preview.{height} and updates public release refs. |
RELEASING.md |
Documents branching, workflows, cutover, and daily release flows. |
.github/workflows/release.yml |
Publishes from main and release branches with prerelease handling and regression guard. |
.github/workflows/promote-minor.yml |
Adds automation to promote main into a stable minor and bump main. |
.github/workflows/pr-version-check.yml |
Adds breaking-change major-version validation for PRs. |
.github/workflows/cut-major.yml |
Adds automation to cut a new major release branch and bump main. |
.github/workflows/ci-build.yml |
Runs PR CI for release branch targets. |
.github/workflows/bump-major-preview.yml |
Adds automation to advance main to the next major preview. |
- version.json publicReleaseRefSpec: tighten 'release/.+' to 'release/\\d+\\.x' so accidental branches like 'release/test' don't get treated as public releases. - release.yml branch trigger: narrow from 'release/**' to 'release/*.x' and add an explicit branch-name validation step that refuses to publish from anything other than main or 'release/<digits>.x'. - ci-build.yml and pr-version-check.yml: same narrower trigger for consistency (no behavioural impact, just stops them firing on malformed release branch names). - promote-minor.yml: validate that stable_version matches main's current preview major.minor, so 'stable_version=9.6' while main is '9.5-preview' is rejected instead of silently mislabeling code. - promote-minor.yml: when setting the new stable version, also remove any existing versionHeightOffset so the new minor starts fresh at X.Y.1 instead of inheriting the previous minor's offset (which would otherwise produce 9.5.38 on the first 9.5 build given the cutover offset of 37). - cut-major.yml: tighten next_main_version validation. After cutting major X, the next main version must be either X.<minor>=1+ (same major continuation) or (X+1).0 (next sequential major). Skips like cutting 10.0 with next_main_version=12.0 are rejected. - cut-major.yml: also strip versionHeightOffset on the new release branch, defensively (main shouldn't have it, but in case it does). Verified the versionHeightOffset removal regex produces valid JSON in all three cases: offset in middle, offset at end, offset absent.
Reviewers: 6 (Claude Opus xhigh x2, Claude Sonnet x2, GPT-5.5 x2). ~99 raw findings consolidated; filtered to those with consensus or high-impact failure scenarios. Security and correctness: - All workflow_dispatch inputs now route through env: instead of being interpolated as inputs expressions into PowerShell single-quoted strings. Prevents shell injection via a quote-in-input. - Every pwsh block begins with $ErrorActionPreference='Stop' and $PSNativeCommandUseErrorActionPreference=$true so native command failures (gh, git) actually propagate. - 'git fetch ... 2>$null' replaced with explicit $LASTEXITCODE checks so a transient fetch failure no longer silently bypasses tag-based security guards (sequential-major check, prerelease regression guard). - 'gh pr create' output captured via Select-Object -Last 1 and trimmed; failure on empty or non-URL output. Previously a partial gh failure could leave PR_URL='' and the workflow would report success. - Input regex tightened from ^\d+$ to ^(0|[1-9]\d*)$ so leading zeros (e.g. '010') are rejected before being written to version.json. - Integer round-trip on parsed inputs to normalize string form. Workflow logic: - promote-minor uses 'git merge -X theirs origin/main', so the unavoidable conflict on the SECOND promotion (release branch's prior stable vs main's preview) resolves automatically. Verified empirically against the conflict case. - cut-major reordered: bump-main PR is opened FIRST, then the release branch is created and pushed. Previously the release branch was pushed before the bump PR, so any failure between them shipped stable but never advanced main. - cut-major now dispatches release.yml via 'gh workflow run' after pushing the new release branch. GITHUB_TOKEN-driven pushes do not trigger workflow_run events, so a bare push would never publish. - promote-minor and cut-major capture main's HEAD SHA at validation time and re-verify it before destructive ops, closing the TOCTOU window where main moves between validate and bump. - bot/* branch names include github.run_id so retries after a partial failure don't collide on the previous run's pushed branch. - All branch pushes use --force-with-lease for safer idempotency. - versionHeightOffset stripper regex now allows end-of-string in addition to \r?\n, so a missing trailing newline doesn't leave the offset in place after promotion. - bump-major-preview now also strips versionHeightOffset (consistency with promote-minor/cut-major). release.yml hardening: - Concurrency group per ref so two pushes to main don't race on tag creation. - NuGet push moved BEFORE GitHub Release creation. Previously a GitHub release/tag could exist for a version that never reached nuget.org. - New regression guard: release/*.x branches refuse to publish any prerelease (those branches must only ship stable). - workflow_dispatch trigger added so cut-major can manually invoke it. - Validate-publish-branch regex tightened to ^release/(0|[1-9]\d*)\.x$ - dotnet/nbgv pinned to v0.5.1 (was master, supply chain risk). - softprops/action-gh-release pinned to v2.6.2. - 'Build' step renamed to 'Pack' (it runs dotnet pack). - Dead 'productNamespacePrefix' env var removed. - Stable-tag regex escapes dots properly. - NuGet push glob narrowed from **/*.nupkg to src/**/bin/Release/*.nupkg. ci-build.yml: actions/upload-artifact pinned to v7 (was master). pr-version-check.yml: - Early exit when base ref is not 'main' (the check is main-only; PRs to release/*.x previously failed with a confusing error). - Validates the PR HEAD's version.json instead of the base branch, so a PR that reverts main's version is also caught. - Fail-closed when no stable tags exist for a breaking-change PR (previously silently passed). - 'semver:major' label support dropped; standardized on 'breaking-change'. - Error message now tells the maintainer to push an empty commit to re-trigger after the bump PR merges. RELEASING.md: - Corrected '9.5.0 stable' to 'first 9.5.x stable (9.5.1)' to match actual NBGV behavior (the version-bump commit is at height 1). - Documents that promotion PRs are NOT mechanical and need review. - Documents recommended branch protection for the version check. - Documents manual re-trigger requirement for breaking-change PRs. - Documents recovery paths for partial cut-major / promote-minor runs. - Shows the exact version.json shape after the initial cutover edit. Findings deliberately not addressed (false positives, low ROI, or out of scope): - ConvertFrom-Json on JSONC: verified working on PowerShell 7.5 (matches GH runners), no issue. - actions/checkout@v6 existence: verified, latest is v6.0.2. - NBGV height=0 vs 1 for cutover offset: verified, height=1 at the version-change commit, so offset of 37 produces 9.4.38 correctly. - Unicode digit injection in inputs: requires malicious maintainer with workflow_dispatch access; extremely improbable. - v-prefixed tag style: not used in this repo. - Fork PR label visibility: edge case, repo doesn't actively manage fork PRs through this label flow. - Per-step permissions scoping: requires major restructure for marginal benefit.
Round 2 had 4 reviewers (Opus codereview + general, Sonnet codereview, GPT-5 general). They found that round 1's fixes introduced new bugs and that round 1 missed several issues. Pattern fix: PowerShell error handling The round-1 pattern of $PSNativeCommandUseErrorActionPreference=$true made every subsequent explicit $LASTEXITCODE check unreachable: a non-zero native exit threw before the check could run. Reverted to just $ErrorActionPreference='Stop' and explicit $LASTEXITCODE checks after each native command. Curated error messages now actually fire. For 'gh pr create' specifically, the call is wrapped in try/catch so the curated message survives the strict-error throw path. Promote-minor merge strategy Replaced 'git merge -X theirs' with a plain merge that aborts on any conflict outside version.json. The -X theirs strategy would silently overwrite a release-branch-only hotfix that was never back-merged to main, dropping the fix without any conflict marker. The new strategy fails loudly on any non-version.json conflict and only auto-resolves the version.json overwrite (which the next step would overwrite anyway). cut-major step reordering Moved 'Verify main hasn't moved' BEFORE 'Open main bump PR'. Round 1 had the verify after the PR was already opened, leaving an orphan PR with stale content if main moved during the run. Cross-workflow concurrency All three main-mutating workflows (bump-major-preview, cut-major, promote-minor) now share a 'main-version-mutator' concurrency group. Two of these workflows can no longer race; the queue serializes them. Round 1 had per-workflow groups which only protected against self-races. TOCTOU coverage bump-major-preview now captures and re-verifies the main SHA the same way cut-major and promote-minor do. promote-minor now also captures and re-verifies the release-branch SHA (previously only main was checked). versionHeightOffset stripper Regex extended to tolerate trailing JSONC '//' comments on the offset line. Added a post-strip ConvertFrom-Json validation that throws if the field is somehow still present after the regex pass. Belt and suspenders: regex catches common cases, parse validation catches anything unusual. cut-major release.yml dispatch reliability Added a retry loop around 'gh workflow run release.yml' (up to 6 attempts, 5s apart) because GitHub's API can briefly fail to find a newly pushed branch. After dispatch, polls 'gh run list' to confirm a run actually queued and surfaces the run URL in the workflow summary so the maintainer can monitor. release.yml asymmetric guard fixed Added a symmetric check: stable versions can't be published from main. Round 1 only blocked prereleases from release branches. A manual edit that accidentally set main's version to a non-prerelease form would have published stable from main, bypassing the entire branching model. release.yml runs tests before pack Pack and push now follow 'dotnet test'. Previously the release pipeline ran restore -> pack -> push with no test coverage on the merge commit. Pack uses --no-build to avoid double-compilation. release.yml NuGet glob Replaced shell-expanded '**/*.nupkg' with explicit pwsh Get-ChildItem under src/**/bin/Release. Eliminates the risk of pushing 3rd-party packages restored into the workspace. Each package is pushed via an explicit $env: read of the API key (no expression interpolation inside the command string). Action pinning glennawatson/ChangeLog now pinned to commit SHA (was @v1 floating). Round 1 missed this third-party action while pinning the others. pr-version-check additions - Unconditional check: if a PR changes version.json's major, it MUST be labeled 'breaking-change'. Catches unlabeled major edits that would otherwise slip through. - Fail-closed when no stable tags exist now allows the legitimate first-breaking-change case (head major == base major + 1) on a brand-new repo without forcing the user to remove the label. - Error message tells the user to rebase onto main (not push an empty commit). The check reads version.json from the PR head; an empty commit on a stale fork point doesn't change version.json. Idempotency Bot branch names now include GITHUB_RUN_ATTEMPT in addition to GITHUB_RUN_ID, so a workflow rerun produces a fresh branch instead of failing to force-push over the prior attempt's branch. cut-major next_main_version safety When advancing main to a NEW major (next major from cut), now also verifies no stable tags already exist for that major. Prevents advancing main to a preview line that has already shipped stable. Documentation - 'push an empty commit' replaced with 'rebase onto main' (the check reads PR head; an empty commit doesn't help). - promote and cut docs emphasize merging the main-bump PR IMMEDIATELY after the stable promotion ships, because the prerelease regression guard fails between those two events. - cut-major step summary now links to the dispatched release run when available. Findings deliberately not addressed - actions/checkout@v6 and actions/setup-dotnet@v5.0.1 SHA pinning: pre-existing, scope creep, GitHub-owned (lower risk). - gh-release run for tag-already-exists recovery: rare; manual recovery is acceptable. - 'cut-major opens PR before TOCTOU': partially addressed by moving verify to between push and PR-create; the bump branch itself is still pushed before the verify, but the PR is not. Recovery is documented (delete the branch and re-run).
Round 3 (2 reviewers) found 4 real bugs introduced by round-2 fixes: pr-version-check: bot bump PRs trigger the unconditional check The new unconditional major-change check would block exactly the PRs that bump-major-preview and cut-major open: they legitimately change the major but aren't labeled 'breaking-change' (they're preparation for breaking, not breaking themselves). Added a bypass for PRs authored by github-actions[bot] with a head branch matching bot/bump-*. The bypass is narrow: it doesn't open a hole for arbitrary bot-authored PRs, just the specific automation pattern. pr-version-check: no-stable-tags branch off-by-one The fail-closed path required head_major == base_major + 1 even in the post-bump steady state where both are already advanced. This would block every breaking-change PR after bump-major-preview merged on any project with no prior stable tags. Now allows head == base (post-bump steady state) OR head == base + 1 (PR does its own bump). promote-minor merge: empty-conflicts case mishandled If 'git merge --no-commit' failed for a reason other than file conflicts (transport error, corrupted object), the code fell through to 'git checkout --theirs version.json' with nothing to resolve, producing a misleading error. Now explicitly detects the empty- conflicts case, aborts the merge cleanly, and rethrows with a diagnostic that points at the real cause. promote-minor verify step: missing $LASTEXITCODE checks The 'Verify branch and main still at validated SHAs' step had two 'git rev-parse' calls without exit-code checks. If either failed transiently, .Trim() returned '' and the comparison would fire the 'moved' error with empty SHAs, misdiagnosing the actual problem. Added exit-code checks consistent with the sibling workflows.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds pre-release publishing built on NBGV, plus three workflow_dispatch actions that handle every
version.jsonedit so we don't have to touch it by hand.How versions work
version.jsonmain"X.Y-preview.{height}"X.Y.0-preview.Non every PR mergerelease/<major>.x"X.Y"X.Y.Nstable on every PR merge (first published patch isX.Y.1, matching the existing tag history)NBGV's
{height}is the first-parent commit count since theversionfield inversion.jsonlast changed. Two PRs merged the same day get distinct heights. No NuGet querying, no race conditions, no manual patch increments.Automation workflows
All run from the Actions tab.
release/<major>.xand sets the stable version (this one has the full diff, review it carefully); (2) bumps main to the next previewrelease/<major>.x, dispatchesrelease.ymlagainst the new branchX.Y-previewto(X+1).0-preview. Rejects skipsPassive guards:
pr-version-check.ymlbreaking-changemust have main's major at exactlylatest_stable_major + 1. Any PR (labeled or not) that changes the major inversion.jsonmust carry thebreaking-changelabel (bot bump PRs are exempted by branch name and author)release.yml)release/*.x, refuses prerelease forX.Yif a stableX.Y.*tag already existsDay-to-day
release/<major>.x, mergemain, mergeOne-time cutover after this PR merges
release/9.xdoesn't exist yet, so it needs to be created once by hand. Full instructions inRELEASING.md. Short version:The next build on
release/9.xproduces9.4.<N+1>. After that, every release uses the automation.Process
The basic infra was straightforward. The automation went through three rounds of adversarial multi-agent review (six reviewers in round 1, then four in round 2, then two in round 3 to verify the round-2 fixes). The reviewers found ~50 unique issues across the rounds, most of which got fixed: PowerShell injection through inputs, the second-promotion always-conflicts merge,
cut-majorrace conditions and ordering bugs, error handling that swallowed git failures silently, a few subtle off-by-ones in the version checks, supply-chain risk from floating action versions, and the prerelease-regresses-past-stable case. The commit history walks through all of it if you want details.Known limitation
Bot PRs are authored by
GITHUB_TOKEN. GitHub deliberately suppresses workflows triggered by that token to prevent recursion, soci-build.ymlandpr-version-check.ymlwon't run on auto-generated PRs unless you close and reopen them (or wire up a PAT). The main-bump PRs are one-lineversion.jsonedits so this is usually fine. The promotion PRs frompromote-minorcarry the full diff of main and should be reviewed and have CI run before merging.