fix(ci): make auto-rollover dedup fail-closed and add merged-PR guard#2193
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (1){README,CLAUDE,*.md,docs/**/*.md}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (4)📚 Learning: 2026-05-16T18:36:31.446ZApplied to files:
📚 Learning: 2026-05-16T18:36:35.250ZApplied to files:
📚 Learning: 2026-05-16T18:36:35.250ZApplied to files:
📚 Learning: 2026-05-16T18:36:35.250ZApplied to files:
🪛 LanguageTooldocs/reference/claude-reference.md[uncategorized] ~99-~99: The official name of this software platform is spelled with a capital “H”. (GITHUB) 🔇 Additional comments (1)
WalkthroughThe auto-rollover workflow's decision logic is refactored to strengthen deduplication and safety. The next rollover version and version-specific branch name are now precomputed early, followed by a GitHub PR query to skip the rollover if a merged or open PR already exists for that branch. The Release-As trailer detection is reworked to explicitly handle git log failures rather than treating failure the same as no match. The later NEXT_VERSION reassignment is removed since the value is precomputed earlier. Reference documentation is updated to reflect the new skip condition. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the Auto Rollover workflow where duplicate release PRs were occasionally generated due to incomplete git history during workflow execution. By introducing a history-independent check against the GitHub API and hardening existing git-log logic to fail closed, the workflow is now more resilient against environment-specific checkout issues. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the documentation in docs/reference/claude-reference.md to accurately reflect the behavior of the auto-rollover.yml workflow, expanding the description of its skip guards from three to four. The updated documentation details new history-independent checks using gh pr list and fail-closed evaluations of the Release-As: trailer range. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
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/reference/claude-reference.md`:
- Line 99: The doc incorrectly states that auto-rollover appends the Release-As:
trailer to an empty commit and PATCHes refs/heads/main; update the wording in
the auto-rollover section (mentioning auto-rollover.yml and
__synthorg_rollover_at_patch) to reflect the actual merge path: the workflow
creates/updates a rollover branch ref (not directly PATCHing refs/heads/main),
places the Release-As: trailer in the PR body, and relies on a squash-merge to
land the trailer on main, and also remove or correct the claim about POST
/git/commits + PATCH /git/refs/heads/main so it matches the branch-ref + PR
flow.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 27a74b66-a15c-4327-801a-f6ff07aea085
📒 Files selected for processing (2)
.github/workflows/auto-rollover.ymldocs/reference/claude-reference.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Lighthouse Site
- GitHub Check: Build Web Assets (melange)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (1)
{README,CLAUDE,*.md,docs/**/*.md}
📄 CodeRabbit inference engine (CLAUDE.md)
Numerics in README and public docs must be sourced from data/runtime_stats.yaml via markers
Files:
docs/reference/claude-reference.md
🧠 Learnings (4)
📚 Learning: 2026-05-16T18:36:31.446Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/reference/conventions.md:787-789
Timestamp: 2026-05-16T18:36:31.446Z
Learning: In Aureliolo/synthorg, do not require adding `<!--RS:...-->` “Doc Numeric Claims (MANDATORY)” numeric macros for Python version numbers mentioned in documentation prose (e.g., “Python 3.14”, “Python 3.15”). The `scripts/check_doc_numeric_macros.py` gate only applies to `README.md`, `docs/index.md`, `docs/roadmap/index.md`, `docs/architecture/decisions.md`, and `docs/reference/convention-gates.md`, and it only flags digits adjacent to specific stat nouns (tests/providers/agents/stars/releases), not language version mentions like “Python 3.14”.
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo, account for the CI gate `check_doc_numeric_macros.py`: it skips fenced code blocks entirely, and it only flags digits that are adjacent to these stat nouns: `tests`, `providers`, `agents`, `stars`, `releases`. Therefore, numeric examples such as CLI flag values (e.g., `--num-workers=4` in fenced bash blocks) and prose version numbers (e.g., `3.14`/`3.15`) are not expected to trigger this check; prioritize changes only when digits appear next to one of the listed nouns (e.g., “5 tests”, “10 stars”, etc.).
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing markdown files for the "Doc Numeric Claims (MANDATORY)" RS-marker rule, only require/flag missing RS markers in the files that are actually in-scope for the rule. The scope is enforced via an identical _SCOPED_FILES allowlist in scripts/check_doc_numeric_macros.py and scripts/inject_runtime_stats.py, and currently includes: README.md; docs/index.md; docs/roadmap/index.md; docs/architecture/decisions.md; docs/reference/convention-gates.md. For any other markdown files (e.g., docs/getting_started.md, docs/guides/*), missing RS markers for numeric claims are no-ops and should NOT be flagged.
Applied to files:
docs/reference/claude-reference.md
📚 Learning: 2026-05-16T18:36:35.250Z
Learnt from: Aureliolo
Repo: Aureliolo/synthorg PR: 1944
File: docs/getting_started.md:109-109
Timestamp: 2026-05-16T18:36:35.250Z
Learning: When reviewing Markdown in the synthorg repo against the `check_doc_numeric_macros.py` gate, account for its documented behavior: it skips fenced code blocks entirely, and it only flags digits that are adjacent to specific stat nouns (`tests`, `providers`, `agents`, `stars`, `releases`). As a result, CLI-style numbers (e.g., `--num-workers=4`) inside fenced bash code blocks should never be treated as violations of this gate; only non-fenced text needs checking, and only around those specific nouns.
Applied to files:
docs/reference/claude-reference.md
🪛 LanguageTool
docs/reference/claude-reference.md
[uncategorized] ~99-~99: The official name of this software platform is spelled with a capital “H”.
Context: ...ynthorg_rollover_at_patchthreshold in.github/release-please-config.json` (default 9)...
(GITHUB)
🔇 Additional comments (1)
.github/workflows/auto-rollover.yml (1)
79-79: LGTM!Also applies to: 122-160
…ody trailer + squash-merge)
Summary
The Auto Rollover workflow opened a duplicate rollover PR (#2190, now closed) for v0.9.0 even though the rollover for that version had already merged (#2177) and release-please PR #2178 already targets 0.9.0.
Root cause
The workflow re-runs on every push to main and decides "rollover needed" from
last-stable-patch >= 9, which stays true for the whole window between the rollover merge and the 0.9.0 release (0.9.0 is not tagged yet, so last stable is stillv0.8.9). The only guard preventing a repeat rollover PR in that window was:This fails open. On the 01 Jun 10:26 run, that run's checkout did not materialise the
v0.8.9<->HEAD connection, so the range came up empty,grepmatched nothing, theifwas false, and the workflow fell through toneeded=true. Every other run since #2177 merged correctly detected the trailer and skipped; only this one slipped through. The two other safety nets did not cover the window: the release-please-body guard matches aRelease-As:trailer (the release PR body is a changelog, no trailer), and the create-step open-PR guard was moot because #2177 was already merged/closed.Fix
chore/auto-rollover-v<NEXT>; skip if one exists. This holds regardless of local git-history completeness, which is what failed before. The branch name carries the version, so a mergedv0.(X+1).0rollover never suppresses the next version's rollover; a CLOSED-not-merged PR (manual cancel) is intentionally not a skip.git loginto a variable and skip the run if it errors, instead of an inlinegit log | grepthat conflated agit logfailure with a genuine no-match underset -o pipefail. A missed rollover self-heals on the next push to main; a spurious PR does not.NEXT_VERSIONcomputation.docs/reference/claude-reference.md(was "three skip guards"; now four + fail-closed note) and tightened the new workflow comments to present-tense rationale.Test plan
actionlint,zizmor,yamllint: clean on the workflow.vale+lychee: clean on the doc.Release-As: 0.9.0trailer is an ancestor of the head that opened chore: auto-rollover to v0.9.0 #2190 and that the range-grep matches with full history present, confirming the bug is the checkout-history edge plus the fail-open guard, not a logic error in the trailer match.Review coverage
Pre-reviewed by 3 agents (infra-reviewer, docs-consistency, comment-quality-rot). All valid findings addressed: comment hygiene (dropped incident/migration framing and a stale issue ref), and the "three skip guards" doc drift. The infra reviewer confirmed the shell idioms (
if ! VAR=$(...),printf '%s'), thegh pr listjq filter, the least-privilege token/permissions, and theNEXT_VERSIONmove are correct, and that thegh pr listcall fails closed (a GitHub API outage aborts the step into the existing failure tracker rather than risking a duplicate).