Fix release finish metadata-only promotion

[justin808]

Why

The release train intentionally commits the stable changelog after the accepted RC. The release rake task already permits non-runtime-only descendants of that RC, but script/release-finish and the runbook still required HEAD to equal the RC tag. That contradiction would either block the helper or encourage placing the tooling fix on the active release branch.

The helper also needs to reject a stale local release branch after the changelog PR merges, before the release task mutates the checkout and encounters a non-fast-forward push.

What changed

• allow the accepted RC to be an ancestor of the release tip when every intervening commit is positively classified as non-runtime by script/ci-changes-detector
• fail closed for runtime-bearing, empty, or unclassified commits
• require the local release branch to exactly match the freshly fetched remote release tip
• update the runbook and automation design for the dedicated post-RC changelog PR
• explicitly keep release-tooling changes on main; an already-cut release line uses the compatible direct rake promotion path

Why this is not backported to release/17.0.0

script/** is release-process code, not metadata-only content. Backporting this helper change after the accepted RC would itself invalidate RC-to-final promotion unless another RC were cut. For 17.0.0, merge the changelog PR into release/17.0.0, fast-forward the local release checkout, then run bundle exec rake "release[17.0.0]" directly.

Validation

• TDD red: stale local release branch fixture previously proceeded to the release command
• TDD green: bash script/release-finish-test.bash — 28 tests, 0 failures
• manual dry-run against the prospective 17.0.0 release state — resolved v17.0.0.rc.12, accepted the one metadata-only changelog commit, and printed the release command without mutation
• ruby -c script/release-finish
• bundle exec rubocop script/release-finish
• shellcheck script/release-finish-test.bash
• Prettier check for both changed Markdown files
• git diff --check
• pre-commit and pre-push hooks, including Markdown link checks
• independent Codex review: initial P1 stale-branch finding fixed with a regression test; follow-up review found no actionable regressions

.agents/bin/validate --changed completed dependency setup, docs sidebar, RuboCop, package build, ESLint, Prettier, and TypeScript checks successfully. Its expanded unrelated gem/generator suite was manually interrupted after roughly 15 minutes with no assertion failures; the focused release-helper checks above were rerun afterward and passed.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 73d03088-bd31-44aa-8981-43812f93ce64

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/fix-release-finish-metadata

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Duplicate/diverging classification logic vs. rakelib/release.rake.

non_runtime_only_commit? re-implements almost exactly what rakelib/release.rake's commit_non_runtime_only? (around line 5654) already does: spin up a temp GITHUB_OUTPUT file, shell out to script/ci-changes-detector "sha^" sha, and parse the trailing non_runtime_only= line. The two copies already differ in small ways — this version short-circuits to false when changed_paths is empty (an explicit pre-check before ever calling the detector), while the rake version has no such pre-check and instead relies on release_finalization_metadata_touched returning nil for an empty diff further up its call chain. That's a fragile way for the two gates to agree.

Since promote only exists to give the operator an early, friendlier version of a check the rake task performs authoritatively anyway (per the comment at the top of the file: "the rake guards enforce the dangerous tag/version safety; this script does not re-implement it"), consider extracting the shared "does this commit classify as non-runtime-only per script/ci-changes-detector" helper into script/lib/ so both rakelib/release.rake and script/release-finish call the same implementation instead of maintaining two hand-synced copies that can silently drift out of agreement (e.g. one script blocking something the rake task would actually accept, or vice versa).

[claude]

Minor UX nit: the remediation always says git pull --ff-only #{remote} #{release_branch}, but that's only the right fix when local is behind remote. If local is instead ahead of (or has diverged from) the fetched remote tip — e.g. an operator committed the changelog collapse locally before pushing/merging the PR — git pull --ff-only is effectively a no-op (or fails outright on divergence) and won't resolve the mismatch this check just reported, leaving the operator to re-discover that they actually need to git push instead. Not a new pattern (mirrors ensure_local_main_matches_remote! above), but since this check is new, it's worth branching the message on git merge-base --is-ancestor <local> <remote> vs. the reverse to give accurate guidance either way.

[claude]

Review summary

What this PR does

Closes a real gap in the release-train tooling: script/release-finish promote and the runbook previously required the release-branch tip to be the exact accepted RC commit, while rakelib/release.rake's own guard already tolerated non-runtime-only descendants (e.g. the dedicated post-RC changelog-collapse commit). That mismatch would either block the documented "commit the stable changelog after the RC" workflow through the helper, or push people to bypass the helper and put the fix on the release branch itself (which the PR correctly argues against, since script/** is release-process code, not metadata). The PR also adds a new guard requiring the local release branch to exactly match the freshly-fetched remote tip before promotion, closing a real "stale local branch after changelog PR merge" hazard.

Strengths

• Fail-closed design. non_runtime_only_commit? returns false (i.e. "treat as runtime-bearing, block promotion") on every ambiguous outcome: missing/non-executable detector, empty diff, detector failure, unparseable output, or any exception. That's the correct default for a release safety gate — "unknown" is never conflated with "safe."
• Good regression coverage. New tests cover the happy path (test_promote_accepts_changelog_commit_atop_rc), the new stale-local-branch guard (test_promote_aborts_when_local_release_branch_is_stale), and the existing empty-commit and runtime-drift tests were correctly updated to push to origin first (now required by the new sync check) and to make an actual Ruby source change (so RUBY_CORE_CHANGED still trips, keeping the fixture valid under the new classification logic).
• Identity check retained as defense-in-depth. The exact-SHA fast path (return if head_sha == tag_sha) is kept ahead of the ancestor/classification walk, preserving the original "empty/metadata commit with identical tree still fails" protection this file was built around.
• Docs updated consistently. Runbook and automation-design doc changes accurately reflect the new script behavior, and explicitly call out why this isn't backported to release/17.0.0 (release-branch process-code changes require another RC).

Issues raised inline

1. Duplicate/diverging classification logic (script/release-finish:240) — non_runtime_only_commit? re-implements rakelib/release.rake's commit_non_runtime_only? (temp GITHUB_OUTPUT file → shell out to ci-changes-detector "sha^" sha → parse non_runtime_only=), and the two copies already differ (this one short-circuits on an empty diff before calling the detector; the rake version relies on a different code path upstream to get the same result). Since this script's check is explicitly just an early/friendly version of what the rake task re-validates authoritatively, extracting a shared helper (e.g. under script/lib/) would prevent the two gates from silently drifting apart.
2. Imprecise remediation message (script/release-finish:416) — ensure_local_release_matches_remote! always suggests git pull --ff-only, which is a no-op/wrong fix if local is instead ahead of or diverged from the fetched remote (e.g. changelog commit made locally before pushing). Minor, and mirrors an existing pattern (ensure_local_main_matches_remote!), but worth tightening now that it's being extended.

Other observations (no action needed)

• non_runtime_only_commit?'s per-commit diff-tree "#{sha}^" sha assumes a linear (first-parent) commit history between the RC tag and the tip; fine for this repo's rebase/cherry-pick-based release workflow, just noting the assumption isn't documented.
• Running script/ci-changes-detector from the current checkout (rather than a version pinned to the commit being classified) is the right call here — it means the current definition of "non-runtime" is applied consistently to all walked commits.

Overall this is a solid, well-tested fix for a real process gap; the main suggestion is de-duplicating the classification helper so the script-level and rake-level gates can't drift out of sync.