Enforce source-atomic release backports#142
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughRelease backport guidance now requires source-atomic release PRs, serial processing, explicit exceptions for inseparable changes, preserved provenance, and authorization for aggregate PR closure. Contract tests verify the required wording across the batch skill, workflow, and guide. ChangesRelease backport policy
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2966a1d47b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| release-only divergence instead of replacing files with base-branch | ||
| post-images, and document every conflict resolution. | ||
|
|
||
| Before creating a release branch, search open release-targeted PRs for an |
There was a problem hiding this comment.
When backporting to an existing release line, agents normally create a short-lived backport branch/PR, not a new release branch; elsewhere in this section “release branch” means the target release line. Gating the aggregate-PR search on “creating a release branch” lets the intended duplicate/aggregate detection be skipped for normal backports, so please make the search happen before creating each backport branch/PR.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| ## Release Backport Granularity | ||
|
|
||
| When consumer policy selects merged base-branch work for a release branch, | ||
| default to **one merged source PR per release backport PR**. Give each source its | ||
| own target, lane, branch, validation, and release PR. Serialize work to the same | ||
| release line: merge one, refresh the release tip, then branch the next. Do not | ||
| combine independent sources because they share a destination, component, | ||
| milestone, or changelog. A shared changelog is a serialization reason, not a | ||
| bundling reason. | ||
|
|
||
| Combine only behaviorally inseparable sources that cannot be reviewed, tested, | ||
| or reverted safely alone; record the exact refs, dependency proof, and | ||
| maintainer-approved exception rationale first. | ||
| Use the consumer's cherry-pick/provenance method, preserve release-only | ||
| divergence, and stamp release notes after accepted backports. | ||
|
|
||
| If an aggregate backport PR already exists, do not keep extending it by default. | ||
| Recommend separate replacements; close the aggregate only with explicit write | ||
| authorization, and retain its branch unless branch deletion was also authorized. |
There was a problem hiding this comment.
This section fully restates the "Release Backport Granularity" policy from workflows/pr-processing.md instead of cross-referencing it, which breaks the pattern this file already uses two sections below for the analogous "Release Phase Gate" policy (skills/pr-batch/SKILL.md:98-99: "follow the Release Phase Gate in the resolved pr-processing.md").
The restatement has already drifted from the canonical version in workflows/pr-processing.md:234-262:
- It omits the source-eligibility checks from workflow.md step 1 (merged, belongs on the selected release line, not already present/superseded, independently validatable).
- It omits the proactive "search open release-targeted PRs for an existing aggregate ... before creating a release branch" instruction — this version only reacts "if an aggregate backport PR already exists."
- It says "close the aggregate only with explicit write authorization" without specifying who can grant it, whereas workflow.md and the guide both say "explicit user or maintainer authorization."
Since this is a safety policy meant to stop agents from re-creating the exact bundling mistake described in the PR description, an agent that only loads this SKILL.md (per the "entry point" note above) could miss the eligibility/search steps or misjudge who can authorize a close. Consider replacing this section with a short pointer to workflows/pr-processing.md#release-backport-granularity, matching the existing convention.
| ### Release backports | ||
|
|
||
| Use **one source PR -> one release PR** as the default backport shape. Process | ||
| multiple backports to the same release branch serially: merge one, refresh the | ||
| release tip, then branch the next. Do not bundle independent source PRs because | ||
| they share a release target or changelog file. Bundling requires an explicit | ||
| maintainer-approved exception with dependency proof showing the changes are | ||
| inseparable for review, testing, and rollback. | ||
|
|
||
| If a pending aggregate backport already combines independent sources, replace it | ||
| with separate lanes. Closing the aggregate is a GitHub write and still requires | ||
| explicit user or maintainer authorization; branch deletion is a separate action. | ||
|
|
There was a problem hiding this comment.
Minor: unlike neighboring sections in this guide (e.g. the pause/cancellation links a few lines up), this new "Release backports" section doesn't link to the canonical detail in workflows/pr-processing.md (### Release Backport Granularity). Adding [Release Backport Granularity](../workflows/pr-processing.md#release-backport-granularity) would keep this guide consistent with its own established cross-linking convention and reduce the chance of the three copies of this policy (guide, SKILL.md, workflow.md) drifting apart — see the related comment on skills/pr-batch/SKILL.md.
| def includes_words?(text, phrase) | ||
| text.gsub(/\s+/, " ").include?(phrase) | ||
| end |
There was a problem hiding this comment.
Nit: includes_words? doesn't check "words" — it does whitespace-normalized substring containment (so it also matches phrases that only appear contiguous after collapsing newlines/indentation across unrelated list items or paragraphs). A name like includes_normalized? or includes_phrase? would better describe the behavior. Not a functional issue given the current assertions, just a readability nit for future additions to this file.
Review SummaryWhat this PR does: Codifies a "source-atomic" release backport policy across Verification: I manually traced every new assertion in Findings (posted inline)
Other notes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
Closing after a seam-placement review. Release-train backport granularity is currently React on Rails-specific policy and now lives in shakacode/react_on_rails#4592. Keeping the portable workflow pack consumer-neutral; this branch is intentionally retained. |
1 participant
Why
An aggregate release PR in shakacode/react_on_rails#4590 combined two independently merged source PRs. That erased the review, validation, rollback, and failure-attribution boundary that each backport should retain.
What changed
Validation
bin/validateruby skills/pr-batch/bin/single_target_entrypoint_test.rbrubocop skills/pr-batch/bin/single_target_entrypoint_test.rbThe configured Codex review model was unavailable in the installed Codex version, so the required independent review fallback used Claude Opus 4.8 and every finding was resolved before this final clean pass.
Summary by CodeRabbit
Documentation
Tests