Document serialized release backport policy#4592
Conversation
WalkthroughRelease-train evaluation rules and maintainer documentation now require serialized, lease-guarded one-source-PR backports, preserve cherry-pick provenance, define bundling exceptions, and add guarded promotion, recovery, and closeout procedures. ChangesRelease backport guidance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Coordinator
participant AgentCoord
participant OriginMain
participant ReleaseBranch
Coordinator->>AgentCoord: Claim release-line:X.Y.Z lease
AgentCoord-->>Coordinator: Confirm active lease
Coordinator->>OriginMain: Refresh and validate source
Coordinator->>ReleaseBranch: Cherry-pick source with -x
Coordinator->>AgentCoord: Renew lease before merge
ReleaseBranch-->>Coordinator: Record backport and changelog reconciliation
Possibly related PRs
Suggested labels: 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 |
Greptile SummaryThis PR documents the serialized release backport rule. The main changes are:
Confidence Score: 5/5This looks safe to merge after a small wording cleanup.
internal/contributor-info/release-train-runbook.md Important Files Changed
Reviews (1): Last reviewed commit: "Document serialized release backport pol..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review: Document serialized release backport policyDocs-only change ( Findings
No issues found with:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
…t-policy * origin/main: (33 commits) Release: add auditable accelerated RC async gates (#4669) Release: handle optional required-check metadata (#4685) Release: forward-port post-pull prerelease guard (#4680) Forward-port stable RSC 19.2.1 to main (#4672) Forward-port Pro license metadata to main (#4668) Docs: clarify local benchmark workflow (#4665) Fix Hacker News demo-fleet smoke path (#4664) Release: reuse verified ShakaPerf pre-run evidence (#4662) Release: require strict HEAD evidence before retry guidance (#4661) Fix: support React 18 non-RSC streaming (#4658) Docs: compare Next.js with Rails async-props streaming (#4651) Docs: add reusable demo fleet RC update prompt (#4649) CI: run Pro compatibility smokes in hosted tests (#4656) Fix ambiguous prerelease retries (#4654) Test: add packaged Pro smokes for React 16 and 17 (#4652) docs: clarify release version ownership and RSC promotion (#4650) [Pro] Keep react-on-rails-rsc out of non-RSC entry graphs (fixes React 18 builds on rc.9) (#4641) ci: make benchmark proof gates replayable (#4626) Test: keep React 19 coverage out of the React 18 lane (#4635) Redact prerender secrets and harden service checks (#4624) ... # Conflicts: # internal/contributor-info/release-train-runbook.md
Address-review summaryScan scope: full history via Mattered
Optional
Skipped
Stale-base race control — checklist+replay
Verification
Next default scan starts after this comment. Say |
Review: Document serialized release backport policyThis is a docs-only PR (no runtime code) adding a "one source PR per release backport PR, serialized" rule across three files: Overview
Issue found (posted inline)In the runbook, the "search open PRs for an existing aggregate before branching" instruction is placed as the last paragraph of the subsection, after the numbered 1–7 procedure — but step 2 of that procedure is itself the branch-creation step. An agent following the numbered steps top-to-bottom would branch before ever reaching the "check for an aggregate first" instruction, which undercuts the exact failure mode (#4590) this PR is meant to prevent. Recommend folding that check into the numbered list as step 1, ahead of "fetch the release branch and create a branch from its current tip." Left an inline comment with specifics. Minor / non-blocking
Correctness / security / performanceNot applicable — this PR only changes markdown process documentation and doesn't touch runtime code, tests, or CI configuration, so there's no correctness, security, or performance surface to review beyond the policy text itself. Test coverageN/A for a docs-only change; the PR description notes prettier, lychee link checks, and rubocop were run, which is reasonable diligence for this kind of change. |
![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: 1b97c5a2b5
ℹ️ 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".
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/contributor-info/release-train-runbook.md (2)
270-273: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRequire passing gates before merging each backport.
“Run” does not prevent merging after a failed validation, QA, or review gate. Require all applicable gates to be green, or require a documented maintainer-authorized waiver before merging.
Proposed wording
- Run the release-phase validation, QA, and review gates, then merge the PR. + Run the release-phase validation, QA, and review gates. Merge the PR only + after they are green or a documented maintainer-authorized waiver is recorded.🤖 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 `@internal/contributor-info/release-train-runbook.md` around lines 270 - 273, Update the release-train runbook step covering release-phase validation, QA, and review gates to require every applicable gate to pass before merging each backport. Permit merging only when a documented maintainer-authorized waiver exists for a failed or unavailable gate.
280-282: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire verified maintainer authorization for exceptions and destructive actions.
“Maintainer-approved” and “explicit write authorization” do not identify a trusted approver or approval record. Public GitHub text must not grant this authority without verified authorship or approval.
internal/contributor-info/release-train-runbook.md#L280-L282: require approval from a verified authorized maintainer for bundling exceptions.internal/contributor-info/release-train-runbook.md#L288-L291: require separately verified authorization before closing an aggregate PR or deleting its branch.🤖 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 `@internal/contributor-info/release-train-runbook.md` around lines 280 - 282, Update the bundling-exception guidance at internal/contributor-info/release-train-runbook.md lines 280-282 to require approval from a verified, authorized maintainer, including an identifiable approval record. Update the aggregate PR closure and branch deletion guidance at lines 288-291 to require separately verified authorization before either destructive action.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@internal/contributor-info/release-train-runbook.md`:
- Around line 270-273: Update the release-train runbook step covering
release-phase validation, QA, and review gates to require every applicable gate
to pass before merging each backport. Permit merging only when a documented
maintainer-authorized waiver exists for a failed or unavailable gate.
- Around line 280-282: Update the bundling-exception guidance at
internal/contributor-info/release-train-runbook.md lines 280-282 to require
approval from a verified, authorized maintainer, including an identifiable
approval record. Update the aggregate PR closure and branch deletion guidance at
lines 288-291 to require separately verified authorization before either
destructive action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c8085ba-ff08-46e6-8ca6-68e742ee9295
📒 Files selected for processing (2)
AGENTS.mdinternal/contributor-info/release-train-runbook.md
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
Address-review summaryScan scope: since previous summary at Addressed
Skipped
Verification
Churn
Next scan should use this summary's creation timestamp as the review cutoff and inspect only later feedback. |
| #### Backport merged `main` PRs one at a time | ||
|
|
||
| When maintainers select one or more merged `main` PRs for the release train, | ||
| use **one source PR -> one release PR**. When there are multiple selections, |
There was a problem hiding this comment.
Minor style nit: this doc otherwise uses the Unicode → for prose arrows (e.g. "1 → 2 → 4 → 5", "main → release/X.Y.Z"), reserving ASCII -> for code/CLI snippets. one source PR -> one release PR here is prose, so → would be more consistent.
| `main`, stop unless a maintainer renews the backport approval with that | ||
| current state. | ||
| 3. If step 1 found a valid backport PR, update its branch onto the current | ||
| release tip under the repository's branch-rewrite policy. Repeat the |
There was a problem hiding this comment.
"the repository's branch-rewrite policy" isn't a defined term anywhere else in the repo docs (checked AGENTS.md, CLAUDE.md, and the rest of this file) — it reads like it should link to the force-push/rebase rules in AGENTS.md's Git Safety section (e.g. "Force-push after a clean rebase ... is OK without asking"), but there's no explicit pointer. Worth linking to the actual section so this isn't a dangling reference for whoever executes this step.
| commits that landed from the PR on `main`, mapping the PR commits to landed | ||
| commits with merge metadata and stable patch IDs. Cherry-pick every landed | ||
| commit oldest-first with `git cherry-pick -x <source-sha>` only after the | ||
| maintainer-approved merge plan required by step 7 exists. Stop if the | ||
| complete mapping cannot be proven or that merge plan is absent. |
There was a problem hiding this comment.
Ordering issue: this step-4 bullet gates cherry-picking on "the maintainer-approved merge plan required by step 7" — i.e. the plan must already exist before step 4 runs. But step 7 (later in the doc) is where that requirement is actually spelled out ("Stop for a maintainer-approved merge plan until…"), and it's phrased as a merge-time gate, not something to front-load. A reader/agent following the steps in order will hit this at step 4 with no prior instruction on how/when to obtain that plan, then only learn the full requirement three steps later.
Consider either (a) adding an explicit early step that says "for a multi-commit rebase-merged source PR or aggregate, obtain maintainer approval for the merge plan before branching/cherry-picking," or (b) rewording this bullet to point forward more explicitly, e.g. "...only after obtaining the maintainer-approved merge plan described in step 7's stop condition."
ReviewDocumentation/policy-only PR: adds a serialized-backport rule (one merged Overview
Strengths
Issues found (posted as inline comments)
General observation (not inline-commented)The new prose is very dense — long, multi-clause sentences packing several conditionals into one paragraph (especially the single-paragraph No security, performance, or test-coverage concerns — this is process documentation with no executable surface. |
* origin/main: Fix Pro performance mark fallback accumulation (#4690)
…t-policy * origin/main: (30 commits) Fix SSR crash on unpaired UTF-16 surrogates from JS JSON output (#4710) (#4726) Remove false Doctor pack-tag pairing warnings (fixes #4619) (#4724) Fix Pro renderer artifact cache identity (#4701) [Pro] Skip stream caches for error-containing renders (fixes #4581) (#4722) docs: mark 16.2.0 immediate_hydration override note as historical (fixes #4639) (#4725) Redact RSC rendering error details from client DOM in production (#4631) Fix fragile envSpecific gsub silently leaving lazy compilation on (#4632) Docs: node-renderer default setup requires Node.js 20+ (Fastify 5) (#4733) [Pro] Harden loadable-stats success-cache test; route worker tests through formAutoContent shim (partially addresses #4506) (#4721) Add rsc-guardrails agent skill + payload-escaping regression test + advisory hook (#4599) Regenerate llms-full references (#4728) CI: make hosted dispatch exact-head idempotent (#4692) Docs: normalize version floors, release metadata, and branch links (#4709) Docs: fix node-renderer startup, versions, and missing config options (#4707) Docs: fix Shakapacker build & migration recipes (#4703) (#4708) Docs: document Pro ExecJS profiling prerequisites & missing cached helpers (#4706) docs: correct OSS helper & API-reference examples (broken/stale snippets) (#4637) Docs: correct immediate_hydration removal details (#4638) (#4640) CI: route gem generator specs by changed paths (#4691) Prevent oversized GitHub release failures (#4714) ...
…t-policy * origin/main: CI: keep ShakaPerf release workflows YAML-lint clean (#4741)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e51825acd7
ℹ️ 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".
| --branch "release/${RELEASE_VERSION}" \ | ||
| --phase release-write-serialization \ | ||
| --status in_progress \ | ||
| --ttl 900 |
There was a problem hiding this comment.
The current agent-coord CLI synopsis lists --ttl for claim but not for heartbeat, so copying this bootstrap makes agent-coord heartbeat fail with a usage error immediately after the 4-hour claim has already been written. In that state acquire_release_line_lease never succeeds and can leave the release line blocked until takeover or expiry; use the CLI's default heartbeat TTL or add heartbeat TTL support before documenting this flag.
Useful? React with 👍 / 👎.
| (.heartbeats | length == 1) and | ||
| (.heartbeats[0].agent_id == $holder) and | ||
| (.heartbeats[0].instance_id == $instance) and | ||
| (.heartbeats[0].target == $target) and |
There was a problem hiding this comment.
After the heartbeat write succeeds, this assertion still rejects the valid lease because the heartbeat was created with --repo shakacode/react_on_rails --target "${RELEASE_LINE_TARGET}", and the agent-coord heartbeat schema stores repo and raw target separately. Comparing .heartbeats[0].target to the synthetic shakacode/react_on_rails#release-line:... string therefore keeps require_live_release_line_lease false for the release coordinator; check .repo and raw .target separately or match the actual scoped-status schema.
Useful? React with 👍 / 👎.
| # If a PR is required, complete its exact-head gates, then in the merger's shell: | ||
| FORWARD_PORT_PR="${FORWARD_PORT_PR:?set the forward-port PR number or URL}" | ||
| require_live_release_line_lease || { return 1 2>/dev/null || exit 1; } | ||
| gh pr merge "${FORWARD_PORT_PR}" --rebase # stop if this would queue or defer the merge |
There was a problem hiding this comment.
When this runs against a branch with merge queue enabled, or when required checks are not complete, gh pr merge --rebase may schedule work instead of merging immediately: the GitHub CLI manual says pending checks enable auto-merge and passed checks add the PR to the queue. That means the merge can happen after this pre-command lease check expires or after the audited refs change, contradicting the surrounding “stop if this would queue or defer” requirement; add a precheck/API path that fails unless the merge completes synchronously, or avoid this command in queued contexts.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@internal/contributor-info/release-train-runbook.md`:
- Around line 736-737: Fence every closeout write before execution: make the
live forward-port helper preview-only or guard its individual writes with
require_live_release_line_lease, rather than checking only afterward. In the tag
closeout flow, perform the lease check before git tag -a creates the tag, and
retain the existing check immediately before the atomic git push.
In `@internal/contributor-info/releasing.md`:
- Around line 732-739: Update the NPM registry query in the release recovery
instructions to use pnpm instead of npm, specifically the command under step 2
alongside the existing RubyGems query. Preserve the package name, version
placeholder, and read-only verification intent.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e9069bb8-7250-42e9-a9cd-9d92f8e54e68
📒 Files selected for processing (4)
.agents/skills/react-on-rails-release-train-issue-evaluation/SKILL.mdAGENTS.mdinternal/contributor-info/release-train-runbook.mdinternal/contributor-info/releasing.md
| require_live_release_line_lease || { return 1 2>/dev/null || exit 1; } | ||
| git push # push main directly or push the exact forward-port PR branch |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fence every closeout write before it occurs.
Line 735 runs the live forward-port helper even though Lines 712-713 mark normal mode as blocked; the lease check at Line 736 is too late to fence that write. Likewise, git tag -a at Line 920 precedes the lease check at Line 929. Keep the helper preview-only or guard individual writes, and move the tag check before tag creation while retaining the check before the atomic push.
Also applies to: 920-933
🤖 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 `@internal/contributor-info/release-train-runbook.md` around lines 736 - 737,
Fence every closeout write before execution: make the live forward-port helper
preview-only or guard its individual writes with
require_live_release_line_lease, rather than checking only afterward. In the tag
closeout flow, perform the lease check before git tag -a creates the tag, and
retain the existing check immediately before the atomic git push.
| 1. Stop the compound helper and keep the release-line lease. Do not delete or move tags, rewrite the | ||
| release branch, rerun publication, or manually publish missing packages. | ||
| 2. Check what was published with read-only registry queries: | ||
| - NPM: `npm view react-on-rails@X.Y.Z` | ||
| - RubyGems: `gem list react_on_rails -r -a` | ||
|
|
||
| 2. If the git tag was created but packages weren't published: | ||
| - Delete the tag: `git tag -d vX.Y.Z && git push origin :vX.Y.Z` | ||
| - Revert the version commit: `git reset --hard HEAD~1 && git push -f` | ||
| - Start over with `bundle exec rake "release[X.Y.Z]"` | ||
|
|
||
| 3. If GitHub release creation fails after successful publishing: | ||
| - Fix GitHub auth (`gh auth login`) or permissions | ||
| - Ensure `CHANGELOG.md` has matching header `### [X.Y.Z]` | ||
| - Do not rerun registry publication; the failure message prints the GitHub-only recovery command | ||
| - Rerun only: `bundle exec rake "sync_github_release[X.Y.Z]"` | ||
|
|
||
| 4. If some packages were published but not others: | ||
| - You can manually publish the missing packages: | ||
| ```bash | ||
| cd packages/react-on-rails && pnpm version X.Y.Z && pnpm publish | ||
| cd ../react-on-rails-pro && pnpm version X.Y.Z && pnpm publish | ||
| gem release | ||
| ``` | ||
| `pnpm publish -r` will publish all packages where current version isn't published yet. | ||
| 3. Record the exact branch tip, local and remote tag identity, published artifact set, and helper output. | ||
| 4. Follow the [Release-Train Runbook](release-train-runbook.md#serialize-every-release-line-write) for | ||
| guarded live recovery. If lease state or any remote/artifact identity is `UNKNOWN`, remain stopped. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- internal/contributor-info/releasing.md (around lines 732-739) ---'
sed -n '728,742p' internal/contributor-info/releasing.md
echo
echo '--- search for registry-check commands ---'
rg -n '\b(npm|pnpm)\s+view\b|\b(npm|pnpm)\s+info\b' internal/contributor-info . -g '*.md' -g '*.mdx' || trueRepository: shakacode/react_on_rails
Length of output: 2209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- pnpm CLI help (view/info) if available ---'
if command -v pnpm >/dev/null 2>&1; then
pnpm --help | sed -n '1,120p' || true
echo
pnpm help view 2>/dev/null | sed -n '1,120p' || true
else
echo 'pnpm not installed in sandbox'
fiRepository: shakacode/react_on_rails
Length of output: 2769
🌐 Web query:
pnpm view command syntax package metadata equivalent to npm view version
💡 Result:
The command in pnpm that is equivalent to npm view is pnpm view [1][2]. As of recent versions, pnpm includes a native view command that functions similarly to npm view [2]. The syntax is [1]: pnpm view [field] For example, to view the version of a package, you can run: pnpm view version When no package name is provided, pnpm defaults to the package defined in the current project manifest (package.json) [1]. Like npm view, pnpm view also supports the --json flag to output metadata in JSON format [1][2]. Additionally, pnpm provides aliases for this command, including info, show, and v [2][3]. top_results: [1][2][4]
Citations:
- 1: https://pnpm.io/cli/view
- 2: feat: add native view/info/show/v command pnpm/pnpm#11064
- 3: Document
pnpm showandpnpm infocommands pnpm/pnpm#5935 - 4: https://docs.npmjs.com/cli/v10/commands/npm-view/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
pnpm help -a | sed -n '1,220p' | rg -n '^( *[a-z,/-]+ +)?(view|info|show)\b' || true
echo
echo '--- pnpm help info ---'
pnpm help info 2>/dev/null | sed -n '1,120p' || true
echo
echo '--- pnpm help show ---'
pnpm help show 2>/dev/null | sed -n '1,120p' || trueRepository: shakacode/react_on_rails
Length of output: 580
Use pnpm view for the registry check
Line 735 should use the pnpm equivalent of npm view so this stays consistent with the repo’s package-manager rules, e.g. pnpm view react-on-rails@X.Y.Z.
🤖 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 `@internal/contributor-info/releasing.md` around lines 732 - 739, Update the
NPM registry query in the release recovery instructions to use pnpm instead of
npm, specifically the command under step 2 alongside the existing RubyGems
query. Preserve the package name, version placeholder, and read-only
verification intent.
Source: Coding guidelines
| **Step 1b — cut rc.0 from the branch.** After at least one CI run finishes on the `release/17.0.0` tip, | ||
| ensure the rc changelog header is present (`$react-on-rails-update-changelog rc` | ||
| targeting the branch), then cut rc.0 with a | ||
| bare release — the version is read from CHANGELOG.md, so you do **not** pass `17.0.0.rc.0`: | ||
| targeting the branch). The bare release command reads the version from CHANGELOG.md, so you do **not** | ||
| pass `17.0.0.rc.0`. It is currently an unfenced compound writer: preview and prepare the cut, but stop | ||
| before live execution until the wrapper contract above is implemented. | ||
|
|
||
| ```bash | ||
| # On release/17.0.0, with CHANGELOG.md stamped ### [17.0.0.rc.0]: | ||
| bundle exec rake release # reads 17.0.0.rc.0 from CHANGELOG.md, bumps version.rb, tags v17.0.0.rc.0, publishes | ||
| require_live_release_line_lease || { return 1 2>/dev/null || exit 1; } | ||
| # BLOCKED: bundle exec rake release would bump, tag, and publish without per-write lease fences. |
There was a problem hiding this comment.
This PR's stated purpose (per the PR description) is "document serialized release backport policy" — but this hunk (and the parallel edits at promote and close-out) does something much bigger: it turns bundle exec rake release into a literal # BLOCKED: comment with no runnable command, pending a "repository-owned wrapper" that doesn't exist anywhere in this repo (grep -rn "wrapper contract" internal/ AGENTS.md only finds references added by this PR itself).
Concretely, as written, a maintainer following this runbook can no longer cut an RC, promote a final, or close out a release line using the documented/previously-working scripted paths (script/release-finish, bundle exec rake release, script/release-forward-port) — every one of them is now dry-run/preview-only. That's a much bigger behavioral change than "one source PR per release backport," isn't mentioned in the PR description, and leaves the release process undocumented/unusable until a wrapper that isn't part of this PR is built.
Is disabling live releases repo-wide actually intended scope here? If so it deserves its own PR/description (and ideally the wrapper itself, or at least a tracking issue) rather than being folded into the backport-policy PR.
| - **`main` never freezes.** It stays in the `beta` phase and keeps absorbing batch work the whole time. | ||
| - **RCs stabilize on an ephemeral `release/X.Y.Z` branch** (one branch per final target, deleted after the final ships; tags are the durable record). Only stabilizing fixes target `release/*`; new features keep targeting `main`. | ||
| - **Forward-port every `release/*` fix to `main` with `git cherry-pick -x <sha>`.** Never `git merge release/X.Y.Z` into `main` — that leaks the RC version-bump commits onto `main`. | ||
| - **Serialize every release-line write; backport one merged source PR per release PR.** Before creating, updating, tagging, promoting, merging, or deleting `release/X.Y.Z`—including release-line creation, every RC cut or re-spin, release-first stabilizers, `main` backports, changelog or metadata PRs, final promotion, and branch deletion—acquire and hold the canonical `release-line:X.Y.Z` coordination lease defined before Step 1 of the release-train runbook. Source-scoped claims do not serialize release writers. One dedicated release coordinator owns the lease and serial dispatch. Chain later batch lanes with `depends_on`, and do not launch them before the preceding merge is terminal. A writer that cannot participate in the canonical lease must stop; the repository's merge-group CI does not rerun release-specific source-liveness, provenance, attribution, manual QA, or review gates and is not an alternative. Refresh the dedicated heartbeat at the runbook cadence during long gates, and immediately before every write or merge require the canonical claim to be active, unexpired, and owned by the expected coordinator with a live matching heartbeat; stop if the guard is unavailable or its state is `UNKNOWN`. Search release-targeted PRs, targeted coordination, and owned remote branches so an existing valid lane is reused instead of duplicated. Give a new lane its own branch off the release tip, validation, QA, and PR. For a `main` backport, also require source-atomic `git cherry-pick -x` provenance: before updating or branching, fetch `origin/main` and confirm its source patch is still live there; a reverted or superseded source requires renewed maintainer approval. Merge it before updating a reused PR onto the refreshed release tip or branching the next backport; immediately before merge, refetch both `origin/main` and the release tip, and update plus rerun the gates if either relevant state changed. Each commit created by a `main`-to-release backport and landed on the release branch must contain exactly one direct `git cherry-pick -x` footer; record inherited provenance in the PR instead of copying another footer. A backport with exactly one source commit must be squash-merged with a final subject ending in `(#<backport-pr-number>)` and the direct footer in its body; a rebase merge is unsupported because an unattributed source subject can make the changelog sweep report `UNKNOWN`. For a multi-commit rebase-merged source PR or explicitly approved inseparable aggregate, stop for a maintainer-approved merge plan until the repository can preserve both one normalized release commit per source commit and changelog-sweep PR attribution; never produce a multi-footer commit. Do not combine independent source PRs because they share a release target, component, or `CHANGELOG.md`; shared metadata is a serialization reason. Combine only behaviorally inseparable fixes with an explicit maintainer-approved rationale covering review, testing, and rollback. Each backport retains its source PR's applicable changelog entry; after every backport retained in the final release set lands, reconcile those entries and stamp or regenerate the RC changelog. Before every RC cut or re-spin and final promotion, fetch `origin/main` and revalidate every retained main-origin backport; a source patch that is no longer live blocks the release until a maintainer explicitly reapproves retaining it. |
There was a problem hiding this comment.
This bullet is a single ~350-word run-on sentence-fragment paragraph packed into one bullet point (compare to the concise one-liner it replaced). It mixes at least 5 distinct rules (lease acquisition, lane reuse, cherry-pick provenance, merge-method requirements, changelog reconciliation) with no structure. As the canonical policy doc (per CLAUDE.md: "AGENTS.md is the canonical policy"), this is hard to scan, hard to keep consistent with the fuller runbook version, and easy to have drift between the two copies of essentially the same rules (this bullet vs. the ## Serialize every release-line write section + #### Backport merged main PRs one at a time section in release-train-runbook.md). Consider trimming this to a short pointer into the runbook section rather than restating the entire procedure inline.
| jq -e \ | ||
| --arg holder "${RELEASE_COORDINATOR_ID}" \ | ||
| --arg instance "${RELEASE_COORDINATOR_INSTANCE_ID}" \ | ||
| --arg target "shakacode/react_on_rails#${RELEASE_LINE_TARGET}" \ | ||
| '(.claims | length == 1) and | ||
| (.claims[0].status == "active") and | ||
| (.claims[0].agent_id == $holder) and | ||
| (.claims[0].instance_id == $instance) and | ||
| (.claims[0].expires_at != "UNKNOWN") and | ||
| ((.claims[0].expires_at | fromdateiso8601) > now) and | ||
| (.heartbeats | length == 1) and | ||
| (.heartbeats[0].agent_id == $holder) and | ||
| (.heartbeats[0].instance_id == $instance) and | ||
| (.heartbeats[0].target == $target) and | ||
| (.heartbeats[0].liveness == "live")' \ | ||
| >/dev/null <<<"${lease_json}" | ||
| } | ||
|
|
||
| acquire_release_line_lease() { | ||
| "${PR_BATCH_SKILL_DIR}/bin/agent-coord-bounded" --timeout 20 status \ | ||
| --repo shakacode/react_on_rails --target "${RELEASE_LINE_TARGET}" --json \ | ||
| >/dev/null || return 1 | ||
| "${PR_BATCH_SKILL_DIR}/bin/agent-coord-bounded" --timeout 20 claim \ | ||
| --agent-id "${RELEASE_COORDINATOR_ID}" \ | ||
| --instance-id "${RELEASE_COORDINATOR_INSTANCE_ID}" \ | ||
| --repo shakacode/react_on_rails \ | ||
| --target "${RELEASE_LINE_TARGET}" \ | ||
| --branch "release/${RELEASE_VERSION}" \ | ||
| --ttl 14400 || return 1 | ||
| heartbeat_release_line_lease || return 1 | ||
| require_live_release_line_lease | ||
| } | ||
|
|
||
| acquire_release_line_lease | ||
| lease_status=$? | ||
| if [ "${lease_status}" -ne 0 ]; then | ||
| echo "release-line lease unavailable or UNKNOWN; stop" >&2 | ||
| return "${lease_status}" 2>/dev/null || exit "${lease_status}" |
There was a problem hiding this comment.
require_live_release_line_lease hard-requires .claims | length == 1 and .heartbeats | length == 1 from agent-coord status --target ... --json. Elsewhere in this repo's docs (agent-coordination-backend.md), targeted agent-coord status is described as returning current lane state, but I don't see the backend's JSON schema confirming that heartbeats is always a singleton (vs. a history/log array) for a target that has been heartbeated many times over a multi-hour release. If the backend ever returns more than one heartbeat record (e.g. recent history) for the same target, this check fails closed every time — which is safe, but would make the whole release process permanently stuck rather than just failing on genuine staleness. Worth explicitly confirming against the backend schema/CLI docs (agent-coord config show --json) before this ships as the gating check for every release write.
Review summaryThis is a documentation-only PR (4 files: OverviewThe stated goal is to codify "one merged Main concern (see inline comment on release-train-runbook.md)Scope: the PR body only describes the backport-serialization rule, but the diff effectively disables the documented live-release workflow repo-wide (branch cut, RC cut, promotion, closeout all now say "stop until the wrapper contract above is implemented"). That's a significant, undisclosed behavioral change to maintainer-facing release tooling docs bundled into a PR framed as a narrower policy addition. Worth confirming this is intentional and, if so, splitting it out with its own description (and ideally landing the wrapper alongside it, or a tracking issue for it) so it's reviewable on its own merits. Other points
Nothing here is a security issue in the traditional sense (no app code changed), but the lease/lock design is effectively a distributed-coordination protocol now embedded in prose rather than code, which carries real correctness risk if the assumptions above don't hold. |
1 participant
Why
React on Rails release backports need a repo-specific operating rule that prevents independently merged
mainPRs from being combined into onerelease/X.Y.ZPR. The closed aggregate #4590 exposed that gap.This policy intentionally lives in React on Rails rather than the portable
agent-workflowspack: release trains and serialized backports are consumer-specific behavior, and React on Rails is currently the concrete consumer that needs it.What changed
Process-gap disposition
AGENTS.mdpolicy plus the repo-local release-train skill and release runbookValidation
.agents/bin/agent-workflow-seam-doctorThe 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 the final clean pass.
Summary by CodeRabbit
main→ activerelease/X.Y.Ztargets, including serialized one-source-at-a-time sequencing in a shared release-line order.release/X.Y.Ztargets are selected and apply the backport rules consistently.