Repository navigation
Add release-merge: the release branch into the base branch, then the base branch into develop - #10
glaubersilva wants to merge 22 commits into
Conversation
release-prep's three-or-four-parts pattern moves to .github/lib/versions.js, where the other version rules live, so the release-merge action applies the same rule from the same place. The wrapper around the real @actions/exec moves from the release-prep test into tests/support, for the tests that run git for real.
Merges the release branch into the base branch and the base back into develop, then publishes the GitHub release for the version. Every step is safe to rerun: a branch that already holds the other merges as a no-op and an existing release is skipped. A conflict abandons the merge, prints a MERGE_CONFLICT line the release dashboard reads, and fails with the branch named. The token reaches git through the environment of the fetch and the pushes only, never the checkout's config, so the caller can keep persist-credentials off and run third-party actions after it. The tests run the merges with real git against a bare repository on disk.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Shared helpers and release-prep integration .github/lib/versions.js, .github/lib/exec.js, .github/lib/github.js, .github/lib/puprc.js, .github/actions/release-prep/release-prep.js, tests/support/actions.js, tests/actions/release-prep/release-prep.test.js |
Adds shared helpers for version validation, command execution, repository and tag lookups, and .puprc version data. The release-prep action and its test use shared helpers. |
Release validation and branch merges .github/actions/release-merge/action.yml, .github/actions/release-merge/release-merge.js, tests/actions/release-merge/release-merge.test.js |
Adds action inputs and outputs, release preflight and version-file checks, and merges from release to base and optionally from base to develop. Tests cover input checks, retries, conflicts, outputs, and failure paths. |
Workflow template and action documentation templates/workflows/release-merge.yml, .github/actions/release-merge/README.md, README.md |
Adds a manually dispatched workflow that invokes the action. Documents the action’s inputs, outputs, merge behavior, conflict handling, and caller requirements. Updates the Phase 2 checklist. |
Estimated code review effort: 4 (Complex) | ~45 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Workflow as release-merge workflow
participant Action as Release merge action
participant GitHub as GitHub API
participant Git as Git
Workflow->>Action: pass branch and version
Action->>GitHub: check tag and release
Action->>Git: fetch branches and merge release into base
Action->>Git: push base and optionally merge base into develop
Possibly related PRs
- stellarwp/plugin-toolbox#16: PR
#16addsrelease-publish, which uses thebase-shaoutput produced by this action. - stellarwp/plugin-toolbox#12: PR
#12checks for existing tags during release preparation. This action also checks tag and release state before merging. - stellarwp/plugin-toolbox#7: PR
#7creates and prepares the release branch that this action merges, including its.puprcversion files.
Suggested labels: enhancement
Suggested reviewers: dpanta94
Merge Risk: 
0891a
-
No actionable issue is established for this change. It is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and specifically summarizes the main change: adding the release-merge action that merges the release branch into the base branch and then into develop. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 19 functions across 9 files. (1 skipped: 1 unsupported.)
-
Finishing Touches
-
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A tag with a published release was read as a retry and the merges went on with the publish skipped. That is right when the release came from this branch, and wrong when the version went out earlier from another commit and the branch was never bumped past it: the new changes would land under a version that is already out. The tag is now fetched and the action goes on only when it contains the branch; otherwise it refuses before merging, naming the fix.
Two releases prepared with the same version and deployed at the same time would merge side by side, the second landing its changes under a version the first had just published. One concurrency group for the whole repository makes the second wait, and the action then refuses it on the published tag. The README says why the caller has to do this.
readRepository, isNotFound and tagExists move to .github/lib/github.js and runOrFail to .github/lib/exec.js, so release-prep and release-merge read GITHUB_REPOSITORY, a tag and a command's failure the same way. The docs name the base branch as main or master wherever it comes up, say which rule release-prep applies and when, and note the two step outputs the action does not expose.
The version becomes the tag and the title of the release, so it has to be the one release-prep wrote into the branch. The action now reads every file the branch's .puprc lists under paths.versions, with the regex pup uses for each, and refuses before merging when any holds a different version, when one holds none, or when the branch has no .puprc. The reader lives in .github/lib/puprc.js.
The release dashboard now sends the deploy step the branch, as it sends prepare, so the template declares branch and passes it to the action as it is.
…ex JavaScript rejects A publish value that is neither true nor false failed only in the publish step, after both merges were pushed; the merge step reads it now too, so the run stops before anything is merged. A .puprc regex pup accepts but JavaScript does not surfaced as a bare SyntaxError; the error now names the file and the regex. The README says where in the sequence the tag ancestry check runs.
| // A shallow checkout, the actions/checkout default, has no history for the merges to find a base | ||
| // in; `--unshallow` fetches it along with the branches. | ||
| const shallow = (await exec.getExecOutput('git', ['rev-parse', '--is-shallow-repository'], { cwd, silent: true })).stdout.trim() | ||
| const refspecs = [branch, base, develop].map((name) => `+refs/heads/${name}:refs/remotes/origin/${name}`) |
There was a problem hiding this comment.
Could the develop back-merge be optional? Not every repo has a develop branch. TEC's repos (the-events-calendar, event-tickets, tribe-common) only have main plus release branches. Right now the fetch fails on refs/heads/develop and the whole run stops. Maybe an empty develop-branch could skip that merge? The base === develop check would need to allow the empty value too.
There was a problem hiding this comment.
Resolved in 50b9cf8. develop-branch: '' skips the merge back: the release is merged into the base and pushed, nothing is fetched or merged for develop, and the develop-branch output is empty. One thing to write in the caller rather than pass through a dispatch input: a workflow_dispatch input left empty takes its default, so the empty value has to be literal in the workflow file (ff87d5d says so in the README).
| owner, | ||
| repo, | ||
| tag_name: version, | ||
| target_commitish: base, |
There was a problem hiding this comment.
target_commitish: base (here and on line 560) tags whatever main is when the publish step runs, not the commit we just merged. The concurrency group only covers this workflow. If someone merges a PR into main between the push and the publish, that commit gets tagged instead. Could runMerge output the merged SHA, and both generateReleaseNotes and createRelease use it?
There was a problem hiding this comment.
Resolved in 7ed3388. The merge step reads the commit it left the base on and exposes it as base-sha. Publishing moved to its own action in #16, release-publish, which takes that commit as target and tags it, so a pull request merged into the base in the meantime is not what gets tagged. It is also the commit the caller builds from.
| tag_name: version, | ||
| target_commitish: base, | ||
| }) | ||
| const { data: release } = await github.rest.repos.createRelease({ |
There was a problem hiding this comment.
createRelease doesn't pass make_latest, so GitHub defaults it to true. A hotfix on an older line (say 6.17.4 after 6.18.3 is out) would take "Latest" away from the newer release. We hit this in TEC and fixed it by comparing against the current latest. compareVersions in lib/versions.js should cover it.
There was a problem hiding this comment.
Resolved in 68a699a on #16, where the publish now lives. make_latest follows version order against the repository's current latest release (compareVersions from lib/versions.js), so a hotfix on an older line does not take "Latest". A latest whose tag is not a plain version is not compared. The dashboard refuses a version below the last released one, so through it the case cannot happen, but the action is shared.
| description: 'The branch the base is merged back into after the release.' | ||
| required: false | ||
| default: 'develop' | ||
| publish: |
There was a problem hiding this comment.
Could the publish step take a few optional inputs: title, notes-file and files? Some repos title releases differently ("Maintenance Release 6.18.3"), take notes from the readme.txt changelog, or attach the zip. Without them, those repos have to edit the release afterwards, and if immutable releases are on, that isn't possible. When files is set, we could create a draft, upload, then publish. A failed upload then leaves a draft to delete instead of a published release with nothing attached.
There was a problem hiding this comment.
Done, as its own action: #16 adds release-publish with title, notes-file and files, and release-merge keeps only the merges. The split is what makes files work: the zip is built after the merge, from the checkout release-merge leaves on the base, so a deploy workflow runs merge, build, publish, and the release is created as a draft, the files are uploaded, and the draft is published last. A failed upload deletes the draft, and a draft makes no tag, so nothing is left behind, which is also what immutable releases need. templates/workflows/release.yml is that deploy workflow. A repository that merges by hand runs release-publish alone, with the checked-out commit as target; its README has that workflow as a snippet. Does that cover the TEC publish?
| false to use the two merges on their own. | ||
| required: false | ||
| default: 'true' | ||
| token: |
There was a problem hiding this comment.
One token does both the pushes and the release. On repos with a tag ruleset, github.token can't create the tag, so the caller has to pass a bot token. If that bot can also bypass the "only PR pushes" rule on main, the action could push a merge nobody reviewed. Could the push token and the release token be separate inputs? Or could there be a merge: false mode for repos where the release PR is merged by a person and the action just checks the version and publishes?
There was a problem hiding this comment.
Right on both counts. I kept one token per action and wrote in each README what the repository has to allow it, instead of adding inputs. The reasoning:
The token is the job's own GITHUB_TOKEN, scoped by permissions: contents: write and gone when the run ends. On GiveWP it is enough: the master ruleset blocks deletion and non-fast-forward pushes, there is no tag ruleset, and a release published with it does not trigger any workflow that runs on the release event.
TEC needs tec-bot because its tag ruleset blocks tag creation, and GITHUB_TOKEN cannot be on a bypass list. A bot PAT that bypasses every rule is a larger key than the problem it solves. If the ruleset blocked deletion and updates but not creation, github.token would publish there too, with the environment's required reviewers as the gate on who creates tags.
On "a merge nobody reviewed": the action is built for the NX1 release dashboard, which is the only thing that dispatches the deploy workflow. In the dashboard a release goes through three steps per repository, and the deploy is the last one: first release-prep bumps the version and writes the changelog on the release branch and a reviewer approves that branch (on its pull request, or in the dashboard); then the RC zip is built and a QA person tests and approves it; only then can someone click "Release Now", which dispatches the workflow that runs release-merge. So by the time the action pushes the merge, the branch has been reviewed and the build tested, and the person who approves the live environment authorizes that specific run. That gate does not depend on which token pushes. Separate push and release tokens would only matter on a repository that trusts the token to rewrite main but not to create a tag, so I did not add them.
merge: false is running release-publish on its own (#16), with the branch as target, which is what your "Release: Publish" workflow does today. If TEC keeps the ruleset, a GitHub App installation token minted in the run (actions/create-github-app-token) passed as token is the safer form of the bypass; the README says it does trigger workflows on the release event. Does that cover the TEC case?
| `.git/config` and never on a command line in the log. Use `persist-credentials: false` on the | ||
| checkout; it matters when later steps of the same job run third-party actions. | ||
|
|
||
| A release published with the default `github.token` triggers no `release` workflow: GitHub does not |
There was a problem hiding this comment.
Small one: it might be worth saying that a PAT or GitHub App token, which some repos will need because of tag rulesets, does trigger on: release workflows. Then the "doesn't deploy twice" guarantee doesn't hold, and the concurrency note becomes more important.
There was a problem hiding this comment.
Resolved in 68a699a on #16: the paragraph is in release-publish's README now. It says that a personal access token or an App token does trigger workflows that run on the release event, so a repository that needs one deploys twice unless that workflow shares the concurrency group and ignores releases created by the bot, or is retired.
…the branch's tip createRelease took the base branch's name as target_commitish, so the tag was made at whatever the base pointed to when the publish step ran: a pull request merged into the base between the push and the publish would have been tagged instead of the release. The merge step now reads the commit it left the base on and hands it to the publish step, which tags that commit. It is also an output of the action, base-sha, since it is the commit the caller builds from. SOFT-5188
The merge back into develop was unconditional: the fetch asked for refs/heads/develop and a repository without that branch (the TEC plugins keep main and release branches only) failed before anything was merged. An empty develop-branch now skips the second merge: the release is merged into the base and pushed, nothing is merged back, and the develop-branch output is empty. SOFT-5188
…ent is the gate SOFT-5188
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
… through a dispatch input A workflow_dispatch input left empty takes its default, which a test run confirmed, so a repository without develop writes develop-branch: '' in its caller. SOFT-5188
glaubersilva
changed the title
The publish step, the publish input and the published / release-url outputs are gone. The action merges, pushes and leaves the commit to publish at in base-sha; the caller builds from it and publishes the GitHub release after the build, so the release can carry the build and is published only once it exists. validateVersion moves to the shared lib, for the publish action that follows. SOFT-5188
glaubersilva
changed the title
The default queue keeps one pending run and cancels it when another arrives, so a third release requested while one waited for approval dropped the second. queue: max keeps them all, in order. SOFT-5188
…onger sends it SOFT-5188
…: max The plugin repositories lint their workflows with actionlint 1.7, which refuses the key, and the template exists to be copied into them. The README says what the default does and that a cancelled deploy is started again. SOFT-5188
…lishes No repository merges a release without publishing it, so a template around release-merge alone had no consumer. The complete deploy workflow, release-merge then the build then release-publish, comes with the publish action. SOFT-5188
Related
Resolves SOFT-5188.
Summary
Every plugin repository merges a release the same way: the release branch into the base branch (
main, ormasterwhere that is its name), then the base branch back intodevelop.release-mergeis that part as a composite action, so a repository's deploy workflow runs it, builds from the result, and goes on to its own steps. Publishing the GitHub release is not part of it: #16 addsrelease-publishfor that, and the caller runs it after the build at the commit this action leaves on the base branch. #16 also brings the deploy workflow to copy,templates/workflows/release.yml, which runs the two actions with the build between them; this PR ships the action alone.The action follows the shape #3 and #7 set: a CommonJS module run by
actions/github-script, inputs read fromenv, unit tests withnode:testagainst the shared harness.Changes
branch,version,base-branch(defaultmain),develop-branch(defaultdevelop; empty for a repository without one, which skips the second merge),token(defaultgithub.token)..puprclists underpaths.versionsholds it, and its tag either does not exist or belongs to a release published from this branch (a retry). A tag without a release, or made from another commit, is refused with nothing merged.--no-ffmerge commits bygithub-actions[bot]. A branch that already holds the other merges as "Already up to date" and pushes nothing, so a failed run is rerun as it is.MERGE_CONFLICTon a line of its own (the marker the release dashboard reads from the log), names the branch in theconflictoutput and fails the step.version,branch,base-sha(the commit the base branch holds the release at, where the checkout is left and where the release is published) andconflict.GIT_CONFIG_*for the fetch and the pushes only, never.git/configor a command line. The README lists what the repository has to allow it: a direct push to the base branch anddevelop, since the merges are not pull requests. The caller template puts the job in an environment with required reviewers.concurrencygroup: two releases prepared with the same version would otherwise merge side by side..github/lib/github.js(readRepository,isNotFound,tagExists),.github/lib/exec.js(runOrFail),.github/lib/puprc.js(readVersionFiles,versionIn),validateVersionandSTELLARWP_VERSION_PATTERNin.github/lib/versions.js;recordedRealExecmoves totests/support/actions.js.How to test
mainanddevelop, write aworkflow_dispatchworkflow withbranchandversioninputs that checks out withpersist-credentials: falseand runs this action (the README's first snippet), withuses:pointing at this branch's commit.release/x.y.zfrommain, bump the version files.puprclists on it (release-prepdoes this) and push.gh workflow run <that workflow> -f branch=release/x.y.z -f version=x.y.z.maingetsMerge release/x.y.z into main,developgetsMerge main back into develop (release/x.y.z), both bygithub-actions[bot], and the checkout ends onmainat that merge commit.developand on a new release branch, then run step 3 for that branch: the job fails on the develop merge with aMERGE_CONFLICT:line,mainhas the release,developis unchanged.node --test(250 tests).The steps above were run on the two private test plugins with
release-mergeandrelease-publishpinned to879516b, this stack's head. The first two rows ran at that commit; the others ran ateab1115, whose action code is identical (only READMEs and the template changed since):github-actions[bot], the zip built between the actions attached to the release, tagged at the merge commitmainalready had the branch, so nothing merged there;developwas behindmainand received its newer commit; release skippeddevelop-branch: '':mainmerged and the release published,developneither fetched nor touchedreadme.txtnot bumped), a published version whose tag does not contain the branch: each refused with nothing mergeddevelopconflicts with the release:mainmerged,MERGE_CONFLICTon its own line, no tag, no release; then, after resolvingdevelopby hand, the rerun mergesdevelopand publishes 1.1.13Stack created with GitHub Stacks CLI • Give Feedback 💬
Summary by CodeRabbit