Rebase release-notes commits onto master before pushing - #1179
Merged
Conversation
The cloud release-notes job lost a push race on v1.0.219 and the page was silently discarded: run 30842563575 generated content/en/cloud/reference/releases/v1.0.219.md, committed it locally as bd9c2a5d, then git-auto-commit-action's push was rejected non-fast-forward because layer5io/meshery-cloud's chart publisher had landed b45c882 on master fifteen seconds earlier. The job failed, the commit went with the runner, and docs.layer5.io/cloud/reference/releases/v1.0.219 404'd. The window is wider than the race suggests. The checkout step is pinned to the SHA the run was dispatched at, so every commit landing on master between dispatch and push guarantees a stale base - rerunning the failed job reproduces the rejection deterministically rather than clearing it. git-auto-commit-action neither pulls nor retries, so a stale base is terminal. Replay the job's own commit on the current tip before the action pushes. Both jobs write only their version file and their own release-notes page, which nothing else writes, so the rebase cannot conflict in practice. The action passes its commit identity per-command and never configures it, so the hook has to be handed the same identity or the rebase commits under whatever ident the runner guesses. Applied to the Kanvas release-notes dispatcher too - identical step, identical gap. The counterpart pusher lives in layer5io/meshery-cloud and already fetches and retries around this same collision, so a concurrency: group here would not reach it and is deliberately not added. Signed-off-by: jamieplu <179417684+jamieplu@users.noreply.github.com>
📝 WalkthroughWalkthroughThe release documentation workflows now rebase generated release commits onto the latest target branch. Both workflows use autostash and the configured commit identity before pushing. ChangesRelease documentation synchronization
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the two release-notes workflows that auto-commit generated release pages to master, preventing non-fast-forward push failures when master advances after the workflow dispatch SHA is checked out (e.g., cross-repo chart publishing landing during the run).
Changes:
- Add a
before_push_hooktostefanzweifel/git-auto-commit-action@v7in the Cloud release notes workflow togit pull --rebase --autostashon the current remote tip before pushing. - Apply the same
before_push_hookfix to the Meshery Extensions (Kanvas) release notes workflow. - Ensure the rebase uses the same commit identity as the action’s commit step by passing
-c user.name/-c user.email.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/workflows/cloud-release-docs.yml | Adds a pre-push rebase hook so auto-committed Cloud release notes aren’t lost on non-fast-forward push rejections. |
| .github/workflows/meshery-extension-release-docs.yml | Adds the same pre-push rebase hook for Meshery Extensions/Kanvas release notes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/cloud-release-docs.yml:
- Around line 51-63: The release-note push hooks only rebase once and can still
fail when workflows run concurrently. Update before_push_hook in
.github/workflows/cloud-release-docs.yml at lines 51-63 and
.github/workflows/meshery-extension-release-docs.yml at lines 51-61 to use a
shared lock or bounded rebase/retry loop that handles concurrent
non-fast-forward pushes in both workflows.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e1a451d-beed-4587-a1d8-66d7a32bb291
📒 Files selected for processing (2)
.github/workflows/cloud-release-docs.yml.github/workflows/meshery-extension-release-docs.yml
Contributor
|
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.
Symptom
https://docs.layer5.io/cloud/reference/releases/v1.0.219was a 404. The release-notes job did its work and then threw it away.From run 30842563575, job "Release notes cloud with latest version", step Commit changes:
The page was generated correctly. The push was rejected, the job failed, and the commit died with the runner.
Root cause
Two things, and the second is the one that makes this a design defect rather than bad luck.
stefanzweifel/git-auto-commit-action@v7commits againstbranch: masterwith no pull, no rebase and no retry. Its only fetch is thegit fetch --depth=1inside_switch_to_branch, which runs before the commit and never re-bases anything. A remote tip that moved after that point is terminal.actions/checkoutstep is pinned to the SHA the run was dispatched at. The exposure is therefore not a millisecond push window - it is the entire span from dispatch to push. Rerunning the failed job reproduces the rejection deterministically, because the rerun checks out the same stale dispatch SHA: job 91788246307 failed identically, same1 and 1 different commits.v1.0.217 and v1.0.218 survived only because the notes job happened to push first.
Fix
Replay the job's own commit on the current tip before the action pushes, via the action's
before_push_hook:Applied to
cloud-release-docs.ymland to its twinmeshery-extension-release-docs.yml(identical step, identical gap).Rebase is the right shape here because the colliding jobs write disjoint paths - this job touches only
build/meshery-cloud.versionand its ownv1.0.219.md; the chart publisher touches onlycharts/. Nothing else writes those files, so the replay cannot conflict in practice.Two things worth calling out
pull:is not an input of this action. The obvious fix -pull: '--rebase --autostash'- would have been silently ignored and looked fixed.pullappears in no version ofaction.yml(checked v4, v5, v6, v7).before_push_hookis the real, documented extension point in v7:entrypoint.shruns_run_hook "before_push_hook"immediately before_push_to_github, after_local_commit, and the file runs underset -eu, so a failed rebase fails the job loudly instead of pushing from a stale base.The identity has to be passed explicitly.
_local_commitsupplies the identity per-invocation (git -c user.name=... -c user.email=... commit) and never writes it to config, so a baregit pull --rebasein the hook would rebase under whatever ident the runner guesses - or fail outright on a runner where git cannot guess one. Passing the action's own$INPUT_COMMIT_USER_NAME/$INPUT_COMMIT_USER_EMAILkeeps the rebased commit consistent withcommit_user_name/commit_user_email.Residual window
A rebase closes the gap between checkout and rebase, not the gap between rebase and push. Two publishers colliding inside those microseconds still leaves one rejected. That cannot be retried from
before_push_hook, which runs before a push it does not own - retrying would meanskip_push: trueplus a hand-rolled clone/rebase/push loop, i.e. the rewrite this is scoped away from, and which already exists on the counterpart side. What changes here is the magnitude: the window that cost us v1.0.219 was the whole dispatch-to-push span, and the surviving failure mode is a loud red job underset -eurather than a silent loss.The other side of the race
The competing pusher is the
Release Chartworkflow in layer5io/meshery-cloud, not in this repo. It has already been hardened - it now runs.github/build/publish-helm-chart.sh, which re-fetches the tip and retries with jitter on each attempt (verified against the live file on that repo'smaster). So:concurrency:group on this workflow cannot serialise it - the racer is cross-repo and GitHub concurrency groups do not span repositories. It is deliberately not added, rather than added as decoration.Verification
Reproduced and exercised, honestly scoped. I could not rehearse a real cross-repo race on Actions, so I rebuilt it locally with the same primitives - a bare origin, a shallow depth-1 clone matching
actions/checkout, and a competing commit landing between clone and push.! [rejected] HEAD -> master (non-fast-forward).charts/*.tgzandcontent/.../v1.0.219.md- no work lost on either side.--depth 1can leave a rebase with no reachable merge base. It does not here: verified with both 1-commit and 3-commit divergence against a shallow clone, both rebased and pushed cleanly. Nofetch-depth: 0needed - this repo is ~1.2 GB and a full clone per release would be a poor trade.set -eu; eval "$snippet") with no ambient git config: the rebased commit came out asl5io <ci@layer5.io>, matching the action's own inputs.actionlintreports no new findings on either file (the remaining shellcheck notes are pre-existing, on untouchedrun:steps).What this does not show: the fix is verified by construction and by a local rehearsal of the race, not by a reproduced cross-repo collision on GitHub Actions. That path only exercises on the next real release.
v1.0.219 is published
Handled separately from this PR, since rerunning could not work. A fresh
workflow_dispatch(run 30844765386) checked out current master and pushed cleanly. Verified at the source of truth, not at a green job:content/en/cloud/reference/releases/v1.0.219.mdis onorigin/masteras of 82c8887, andbuild/meshery-cloud.versionnow readsv1.0.219.Flagged, not fixed here
Four workflows in this repo push to
masterwith the same unguardedgit-auto-commit-actionstep, and all four are on the same0 0 * * *cron:feature-list.yml,generate-keys.yml,discussion-data-files-update.yml,generate-pricing-list.yml. They race each other nightly with the identical failure mode - a losing job drops its data update in silence. The same one-linebefore_push_hookfixes each. Left out of this PR to keep it to the reported defect; happy to fold them in here or file separately, whichever reviewers prefer.