ci(release): auto-patch-release only the 2 newest release lines#2856
Conversation
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
📝 WalkthroughWalkthroughThe auto-release workflow's branch processing script now enumerates all remote ChangesRelease branch maintenance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
NOT LGTM — the window logic itself is correct and verified against the live branch list, but the release-process documentation in this repo describes exactly the process whose behavior this PR changes, and it is now stale.
Business context: the nightly Auto Patch Release cron walked all 21 release-X.Y branches and cut patch tags on long-EOL lines, producing stray releases and blocked release PRs; this restricts it to the 2 newest minor lines with a self-sliding window.
Blockers
B1: docs/release.md does not reflect auto-patch-release behavior or the new support window
File: docs/release.md (section "Patch Releases")
Issue: the doc describes patch releases as a fully manual process (cherry-pick to the release branch, open a PR, tag manually). The Auto Patch Release workflow already cuts patch tags automatically, and this PR adds a support-window policy (only the 2 newest lines) that is recorded nowhere except a code comment inside the workflow YAML.
Evidence: docs/release.md:86-92 lists only manual steps; no mention of the nightly cron or any support window.
Impact: maintainers following the canonical release doc get a wrong model of what automation will and won't do — e.g. expecting an EOL line to auto-release, or being surprised when a supported line auto-tags.
Fix: add a short subsection to docs/release.md: the cron schedule, the trigger condition (new commits after the latest published release), the 2-newest-lines window, and the manual escape hatch for EOL lines (pushing a vX.Y.Z tag still triggers the release pipeline via tags.yaml).
Non-blocking follow-ups
- The window-selection logic (sort key
maj * 1000 + min+slice) is real logic living in untestable inline JS. Consider extracting branch selection into a small script underhack/with a unit test covering the numeric-vs-lexicographic trap (release-1.10>release-1.9). skipped.sort()sorts lexicographically, so the log line will orderrelease-0.9afterrelease-0.41; cosmetic, could reuselineNum.- Merge-order note: #2849 pins actions and tightens permissions in this same file — whichever lands second needs a trivial rebase.
Verified while reviewing: sort key handles release-0.10 > release-0.9 correctly; slice is safe with fewer than 2 branches; regex pre-filter rules out NaN; [...allBranches] copies before the mutating sort, so skipped is computed on the intact list; manual tag push on an EOL line still triggers the release pipeline (tags.yaml fires on any v*.*.* tag push), so the change removes only automatic tag cutting, as stated.
| const [maj, min] = b.replace('release-', '').split('.').map(Number); | ||
| return maj * 1000 + min; | ||
| }; | ||
| const branches = [...allBranches] |
There was a problem hiding this comment.
This selection logic (sort key + slice) is worth extracting into a small testable script under hack/ — a unit test feeding the real branch list would pin the numeric ordering (release-1.10 > release-1.9) against future edits to the lineNum multiplier.
There was a problem hiding this comment.
Good idea, but deferring out of this PR. The repo has no JS test toolchain (hack/ is shell + bats, no package.json) and every workflow keeps its logic inline in github-script, so extracting this into a tested module is a separate change rather than part of this CI tweak. The numeric trap you flagged (release-1.10 > release-1.9) is exercised by the existing lineNum sort, which I verified picks release-1.10/release-1.9 as the 2 newest. Happy to track the extraction as a follow-up.
| .sort((a, b) => lineNum(b) - lineNum(a)) | ||
| .slice(0, SUPPORTED_LINE_COUNT); | ||
|
|
||
| const skipped = allBranches.filter(b => !branches.includes(b)).sort(); |
There was a problem hiding this comment.
skipped.sort() is lexicographic (release-0.9 lands after release-0.41 in the log). Cosmetic only — could reuse lineNum for a numeric order.
There was a problem hiding this comment.
Fixed in b9eda36 — the skipped list now sorts with (a, b) => lineNum(a) - lineNum(b) instead of the default lexicographic .sort(), so the log lists EOL branches numerically (release-0.9 before release-0.41).
|
Aleksei Sviridkin (@lexfrei) Thanks for the thorough review — the blocker is addressed. B1 (docs/release.md): fixed in b9754d8. Added an "Automated Patch Releases" subsection under "Patch Releases" covering the nightly cron ( Non-blocking follow-ups: #2 ( |
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 `@docs/release.md`:
- Line 170: The sentence in the release documentation at line 170 overclaims the
automation scope by stating that the Auto Patch Release workflow "kicks off
steps 2–7" when it actually only automates patch tag creation and then hands off
to the existing release pipeline. Revise the wording to clarify that the
workflow automates the initial patch tag creation and initiates the release
process, but steps 4–7 still require manual maintainer action, making it clear
that automation is limited in scope rather than covering the entire steps 2–7
range.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3f42fc1d-c249-4e68-b23c-f49219c29736
📒 Files selected for processing (2)
.github/workflows/auto-release.yamldocs/release.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/auto-release.yaml
The Auto Patch Release cron walked every release-X.Y branch (21 of them,
back to release-0.9) and cut a vX.Y.(Z+1) tag whenever a branch had
commits after its latest published release. That fired patch releases on
long-EOL lines (e.g. v1.3.6), whose release PRs then sit blocked on
unrelated CI failures and accumulate stray draft releases, tags and
changelog PRs.
Restrict the cron to the SUPPORTED_LINE_COUNT (=2) newest minor lines.
The window is derived from the branch list, so it slides automatically as
new release-X.Y branches are cut — once release-1.5 exists the window
becomes {1.5, 1.4} and the trailing line retires with no further edits.
Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from lexfrei on docs/release.md: the Patch Releases section described only the manual process, so add an "Automated Patch Releases" subsection covering the nightly cron, the new-commits trigger, the 2-newest-lines support window, and the manual tag-push escape hatch for EOL lines. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Address review feedback from lexfrei on .github/workflows/auto-release.yaml:82: the skipped-branches log used a lexicographic sort (release-0.9 after release-0.41). Reuse the existing lineNum helper so the log lists EOL branches in numeric order. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
b9eda36 to
3bcab58
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
docs/release.md (3)
247-247: 💤 Low valueConsider demoting the section to a subsection under "Patch Releases".
The "Automated patch tag" section at line 247 is a top-level
##, but logically it belongs as a subsection under the "Patch Releases" section (line 99) since it documents the automated half of the patch-release workflow. Changing it to###would better reflect the document hierarchy.📋 Proposed fix
-## Automated patch tag (`auto-release.yaml`) +### Automated patch tag (`auto-release.yaml`)🤖 Prompt for 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. In `@docs/release.md` at line 247, The "Automated patch tag (auto-release.yaml)" section header is currently a top-level heading (##) but should be demoted to a subsection (###) since it logically belongs under "Patch Releases" to maintain proper document hierarchy. Change the section header from ## to ### to reflect that it is a subsection of the Patch Releases workflow documentation.
464-465: 💤 Low valueReduce repetition of "If" at sentence start.
Three successive sentences begin with "If", reducing readability. Consider restructuring one or two for variety.
✏️ Example refactor (keep tone & meaning)
If a PR-level entry already mentions the bump, enrich that entry with bullets — don't duplicate the entry. If 3+ bumps have no PR-level mention, introduce a `## Platform Components` section. -If you can't find upstream notes for a bumped component, list it with version-only info and no bullets. Hallucinated bullets are worse than missing detail. +When upstream notes are unavailable, list the bumped component with version-only info and no bullets. Hallucinated bullets are worse than missing detail.🤖 Prompt for 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. In `@docs/release.md` around lines 464 - 465, In the release.md document around lines 464-465, identify the three successive sentences that begin with "If" and restructure at least one or two of them to vary the sentence structure while maintaining the same tone and meaning. For example, convert conditional "If" statements into imperative statements, questions, or other grammatical structures that preserve the original intent but improve readability by reducing repetitive sentence openings.
469-503: 💤 Low valueUse consistent spelling: "pre-release" vs. "prerelease".
The document mixes both variants (e.g., "pre-release verification gaps" at line 469, "prerelease tags" in static context). Choose one form and apply it consistently. "Pre-release" (with hyphen) is more common for adjectival and noun forms in modern docs.
🤖 Prompt for 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. In `@docs/release.md` around lines 469 - 503, The document uses both "pre-release" and "prerelease" inconsistently throughout. Search and replace all instances of "prerelease" (without hyphen) with "pre-release" (with hyphen) to standardize on the hyphenated form used in "pre-release verification gaps" and "Pre-release verification checklist". Apply this change consistently across the entire document, including section headings and body text.
🤖 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.
Nitpick comments:
In `@docs/release.md`:
- Line 247: The "Automated patch tag (auto-release.yaml)" section header is
currently a top-level heading (##) but should be demoted to a subsection (###)
since it logically belongs under "Patch Releases" to maintain proper document
hierarchy. Change the section header from ## to ### to reflect that it is a
subsection of the Patch Releases workflow documentation.
- Around line 464-465: In the release.md document around lines 464-465, identify
the three successive sentences that begin with "If" and restructure at least one
or two of them to vary the sentence structure while maintaining the same tone
and meaning. For example, convert conditional "If" statements into imperative
statements, questions, or other grammatical structures that preserve the
original intent but improve readability by reducing repetitive sentence
openings.
- Around line 469-503: The document uses both "pre-release" and "prerelease"
inconsistently throughout. Search and replace all instances of "prerelease"
(without hyphen) with "pre-release" (with hyphen) to standardize on the
hyphenated form used in "pre-release verification gaps" and "Pre-release
verification checklist". Apply this change consistently across the entire
document, including section headings and body text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b95a74ba-7b81-4e99-9167-990d7e46c8d1
📒 Files selected for processing (2)
.github/workflows/auto-release.yamldocs/release.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/auto-release.yaml
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM — window logic is correct and verified against the live branch list, and the release-process docs now match the new behavior.
Business context: the nightly Auto Patch Release cron walked all 21 release-X.Y branches and cut patch tags on long-EOL lines, producing stray releases and blocked release PRs; this restricts it to the 2 newest minor lines via a self-sliding window.
Re-verified on the current head:
- Window selection (
lineNum = maj*1000 + min, descending sort,slice(0, SUPPORTED_LINE_COUNT)) correctly picks{release-1.4, release-1.3}out of the 21 live branches; the numeric key handlesrelease-1.10 > release-1.9andrelease-0.10 > release-0.9. [...allBranches]copies before the mutating sort, soskippedis computed on the intact list;sliceis safe with fewer than 2 branches; the regex pre-filter rules out NaN.skippednow sorts numerically vialineNum, so the EOL log listsrelease-0.9beforerelease-0.41.- Manual tag push on an EOL line still triggers the full pipeline (
tags.yamlfires on anyv*.*.*push), so only automatic tagging is restricted — as stated. docs/release.md"Automated patch tag" section now documents the 2-newest-lines window, the EOL skip, the self-sliding behavior, and the manual escape hatch.
The previous blocker (docs did not reflect the support window) is resolved. The docs section was reconciled against the rewritten main copy of docs/release.md to avoid a duplicate auto-release section.
Non-blocking follow-ups
- The branch-selection logic lives as inline
github-scriptJS with no unit test. Out of scope here (the repo has no JS test toolchain and every workflow keeps its logic inline), but worth tracking if this file grows — a test would pin the numeric-ordering trap against future edits to thelineNummultiplier. SUPPORTED_LINE_COUNT = 2is hardcoded in the workflow and referenced by name in the doc, so a future policy change is a one-line edit in one place. Fine as-is.
What this PR does
The Auto Patch Release cron (
.github/workflows/auto-release.yaml) walkedevery
release-X.Ybranch — 21 of them, back torelease-0.9— and cut avX.Y.(Z+1)tag whenever a branch had commits after its latest publishedrelease. This fires patch releases on long-EOL lines (e.g.
v1.3.6), whoserelease PRs then sit blocked on unrelated CI failures and accumulate stray
draft releases, tags and changelog PRs.
This restricts the cron to the 2 newest minor lines (
SUPPORTED_LINE_COUNT).The window is derived from the live branch list, so it slides automatically as
new
release-X.Ybranches are cut — oncerelease-1.5exists the windowbecomes
{1.5, 1.4}and the trailing line retires with no further edits.Currently active:
release-1.4,release-1.3. Skipped: 19 EOL lines.Release note
Summary by CodeRabbit
release-X.Ymaintenance lines.