Release CI gate: evaluate last full-CI commit, not changelog/docs HEAD#4070
Conversation
The `rake release` CI gate evaluated origin/main HEAD verbatim and blocked until every check on that commit finished. Running `update-changelog` right before a release makes HEAD a changelog-only commit, on which CI path-skips the runtime suite but still queues detect-changes, CodeQL Analyze, and the markdown checks — so the release waited on checks that say nothing about whether the code is releasable. The gate now walks back from HEAD over commits that `script/ci-changes-detector` classifies as non-runtime-only (changelog, docs, source-comment changes — exactly the commits where CI skips the suite) and evaluates the first commit that actually ran the full suite. That commit's CI is already complete, so the release proceeds without waiting. The walk reuses the detector's own `non_runtime_only` output (via $GITHUB_OUTPUT) rather than re-deriving paths-ignore rules, is bounded by MAIN_CI_NONRUNTIME_WALK_LIMIT, and fail-safes to evaluating HEAD whenever a commit isn't provably non-runtime-only (missing detector, git/detector failure, unparseable output). Set RELEASE_CI_EVALUATE_HEAD=true to force strict HEAD evaluation. The pass/fail logic per evaluated commit (stable = all checks, prerelease = required checks) is unchanged — only commit selection changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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 (2)
WalkthroughThe release CI gate in ChangesRelease CI gate SHA selection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
+ci-run-hosted |
github-actions
Bot
added
the
ready-for-hosted-ci
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
Greptile SummaryThis PR fixes a release CI gate that blocked on changelog/docs-only commits where the full test suite was skipped by path rules. Rather than evaluating
Confidence Score: 4/5Safe to merge. The walk-back logic has solid fail-safes and degrades cleanly to HEAD evaluation in every error path. The new helpers are well-bounded (25-commit limit, escape hatch, root-commit guard, detector-unavailable guard) and backed by thorough spec coverage. Two minor log/error-reporting gaps: the walk-limit termination path emits a message that incorrectly claims full-suite confirmation, and unexpected exceptions in the detector invocation are swallowed without any diagnostic output. rakelib/release.rake — specifically the log message in log_main_ci_walkback and the bare rescue in commit_non_runtime_only? Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[fetch_main_ci_checks called] --> B[git fetch origin main]
B --> C[git rev-parse origin/main]
C --> D{RELEASE_CI_EVALUATE_HEAD=true?}
D -- yes --> H[evaluate head_sha]
D -- no --> E[main_ci_evaluation_sha]
E --> F{commit_non_runtime_only? current}
F -- false or error --> H
F -- true --> G[git_parent_sha current]
G -- nil --> H
G -- parent --> I[skipped << current, current = parent]
I --> J{walk count >= 25?}
J -- no --> F
J -- yes --> K[evaluate current, walk limit reached]
H --> L[query GitHub Checks API]
K --> L
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[fetch_main_ci_checks called] --> B[git fetch origin main]
B --> C[git rev-parse origin/main]
C --> D{RELEASE_CI_EVALUATE_HEAD=true?}
D -- yes --> H[evaluate head_sha]
D -- no --> E[main_ci_evaluation_sha]
E --> F{commit_non_runtime_only? current}
F -- false or error --> H
F -- true --> G[git_parent_sha current]
G -- nil --> H
G -- parent --> I[skipped << current, current = parent]
I --> J{walk count >= 25?}
J -- no --> F
J -- yes --> K[evaluate current, walk limit reached]
H --> L[query GitHub Checks API]
K --> L
Reviews (1): Last reviewed commit: "Release CI gate: evaluate last full-CI c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| puts "ℹ️ origin/main HEAD #{head_sha[0, 8]} is non-runtime-only " \ | ||
| "(changelog/docs/comments); CI path-skips the full suite on such commits." | ||
| puts " Skipped #{skipped.length} non-runtime commit(s): #{skipped.map { |s| s[0, 8] }.join(', ')}" | ||
| puts " Evaluating main CI on #{evaluated_sha[0, 8]} — the most recent commit that ran the full suite." |
There was a problem hiding this comment.
commit_non_runtime_only? returned false. When the loop terminates due to MAIN_CI_NONRUNTIME_WALK_LIMIT, the final current was never passed to commit_non_runtime_only? — it could itself be a non-runtime-only commit. The message would then misrepresent what CI ran on that commit.
| puts " Evaluating main CI on #{evaluated_sha[0, 8]} — the most recent commit that ran the full suite." | |
| walk_limit_reached = skipped.length >= MAIN_CI_NONRUNTIME_WALK_LIMIT | |
| suffix = walk_limit_reached ? " (walk limit reached; full-suite status unconfirmed)." : " — the most recent commit that ran the full suite." | |
| puts " Evaluating main CI on #{evaluated_sha[0, 8]}#{suffix}" |
| rescue StandardError | ||
| false | ||
| end |
There was a problem hiding this comment.
rescue StandardError at the end of commit_non_runtime_only? silently discards every unexpected failure (temp-dir creation errors, unexpected IO errors, etc.) and returns false. While the fail-safe direction is correct, a suppressed exception can make real operational problems (e.g. a permissions issue on the detector script, a broken git install) invisible during a release. Logging the exception message preserves the safe fallback while providing a diagnostic signal.
| rescue StandardError | |
| false | |
| end | |
| rescue StandardError => e | |
| warn "⚠️ commit_non_runtime_only? raised #{e.class}: #{e.message}; treating commit as runtime-bearing." | |
| false | |
| end |
![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: b708ec4e72
ℹ️ 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".
| _stdout, status = Open3.capture2e( | ||
| { "GITHUB_OUTPUT" => output_file }, detector, "#{sha}^", sha, chdir: monorepo_root | ||
| ) |
There was a problem hiding this comment.
When origin/main is updated by a push containing multiple commits, the workflows classify changes from github.event.before to the pushed HEAD (for example .github/workflows/gem-tests.yml:74-75 and .github/workflows/lint-js-and-ruby.yml:67-68), not from HEAD^ to HEAD. If that push contains a runtime commit followed by a changelog/docs-only commit, full CI actually runs on HEAD, but this per-commit detector invocation labels only the last commit as non_runtime_only and walks back to the parent. The release gate can then ignore the real HEAD check run (or block on a parent that never had a main-push run), so the selected commit no longer matches the commit whose CI GitHub just evaluated.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Code Review — Release CI gate: evaluate last full-CI commit, not changelog/docs HEADOverviewThis is a clean, well-motivated fix for a real release workflow friction: after The design choices are sound:
Test coverage is thorough: no-walk, single-hop, multi-hop, env bypass, detector-unavailable fail-safe, root-commit boundary, and walk-limit cap are all exercised. Issues1. Log message overclaims (minor but misleading)
The evaluated commit is actually the first one that isn't provably non-runtime-only — which is the correct and safe stopping condition, but not the same as "ran the full suite." If the commit sits at the walk-limit boundary, or its CI was skipped for an unrelated reason, the message is inaccurate. Suggest softening to something like "the most recent commit whose CI was not path-skipped" or "the most recent commit that likely ran the full suite." 2. Shallow clone degrades at the wrong boundary (niche, but worth noting)In That's the safe direction, but it means a shallow clone won't benefit from the walk-back at all. This is probably acceptable for release workflows (which typically fetch full history), but worth a comment near the 3. Missing test for the
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| puts "ℹ️ origin/main HEAD #{head_sha[0, 8]} is non-runtime-only " \ | ||
| "(changelog/docs/comments); CI path-skips the full suite on such commits." | ||
| puts " Skipped #{skipped.length} non-runtime commit(s): #{skipped.map { |s| s[0, 8] }.join(', ')}" | ||
| puts " Evaluating main CI on #{evaluated_sha[0, 8]} — the most recent commit that ran the full suite." |
There was a problem hiding this comment.
The phrase "the most recent commit that ran the full suite" overclaims what we actually know. main_ci_evaluation_sha returns the first commit that isn't provably non-runtime-only — which is the correct, safe stopping point — but that commit's CI could still have been skipped for an unrelated reason, or we might be at the walk-limit boundary without having verified it.
Suggest softening the wording:
| puts " Evaluating main CI on #{evaluated_sha[0, 8]} — the most recent commit that ran the full suite." | |
| puts " Evaluating main CI on #{evaluated_sha[0, 8]} — the most recent commit not classified as changelog/docs-only." |
| # instead of re-deriving paths-ignore rules here. `<sha>^ <sha>` diffs just | ||
| # that commit; a non-HEAD current ref means no uncommitted folding. | ||
| _stdout, status = Open3.capture2e( | ||
| { "GITHUB_OUTPUT" => output_file }, detector, "#{sha}^", sha, chdir: monorepo_root |
There was a problem hiding this comment.
In a shallow clone, sha^ may not be fetched, causing git_diff_base_resolve to exit non-zero (the detector uses set -euo pipefail). This returns false — the safe direction — but it silently prevents any walk-back. Worth a brief comment so the failure mode is obvious to a future reader:
| { "GITHUB_OUTPUT" => output_file }, detector, "#{sha}^", sha, chdir: monorepo_root | |
| # NOTE: `sha^` requires the parent commit to be present in the local clone. | |
| # In a shallow clone where the parent isn't fetched, the detector exits non-zero | |
| # and this method returns false (fail-safe: treat as runtime-bearing, stop walk). | |
| _stdout, status = Open3.capture2e( | |
| { "GITHUB_OUTPUT" => output_file }, detector, "#{sha}^", sha, chdir: monorepo_root | |
| ) |
|
|
||
| result = nil | ||
| expect { result = main_ci_evaluation_sha(monorepo_root:, head_sha: "c") } | ||
| .to output(/Skipped #{MAIN_CI_NONRUNTIME_WALK_LIMIT} non-runtime/o).to_stdout |
There was a problem hiding this comment.
The /o flag ("interpolate once") is a no-op in modern Ruby when the interpolated value is a constant — the regex is already compiled once at parse time. This is harmless but can confuse readers who expect it to mean something.
| .to output(/Skipped #{MAIN_CI_NONRUNTIME_WALK_LIMIT} non-runtime/o).to_stdout | |
| .to output(/Skipped #{MAIN_CI_NONRUNTIME_WALK_LIMIT} non-runtime/).to_stdout |
1 participant
Problem
When you run
update-changelogright before a release,origin/mainHEAD becomes a changelog-only commit. Therake releaseCI gate evaluated HEAD verbatim and blocked until every check on it finished — but on a changelog/docs commit CI path-skips the runtime suite and only queuesdetect-changes, CodeQLAnalyze, and the markdown checks. So the release waited on checks that say nothing about whether the code is releasable.You can't just hop to
HEAD~1either: the prior commit may itself be docs-only, where full CI also didn't run.Fix
The gate now walks back from HEAD over commits that
script/ci-changes-detectorclassifies as non-runtime-only (changelog, docs, source-comment changes — exactly the commits on which CI skips the runtime suite) and evaluates the first commit that actually ran the full suite. That commit's CI is already complete, so the release proceeds immediately.non_runtime_onlyoutput (written to a temp$GITHUB_OUTPUT), rather than re-deriving paths-ignore rules in Ruby.MAIN_CI_NONRUNTIME_WALK_LIMIT(25) and fail-safe: any commit not provably non-runtime-only (missing detector, git/detector failure, unparseable output) stops the walk, degrading to the original "evaluate HEAD" behavior.RELEASE_CI_EVALUATE_HEAD=trueforces strict HEAD evaluation.Example log when HEAD is changelog-only:
Tests
react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb— new coverage formain_ci_evaluation_sha(no-walk / single-hop / multi-hop / env bypass / detector-unavailable fail-safe / root-commit boundary / walk-limit cap),commit_non_runtime_only?(true/false/failure paths + the exact detector invocation), andgit_parent_sha. Full file: 130 examples, 0 failures; RuboCop clean.No CHANGELOG entry: this is release tooling (
rakelib/), not a user-visible gem change.🤖 Generated with Claude Code
Note
Medium Risk
Changes which commit gates publishing and depends on
ci-changes-detectorduring release; fail-safe defaults limit risk, but a misclassified commit could skip or over-walk history.Overview
Fixes false release blocks when
origin/mainHEAD is changelog/docs/comment-only and CI only runs lightweight checks there.fetch_main_ci_checksno longer always usesorigin/mainverbatim. It callsmain_ci_evaluation_sha, which walks parents (capMAIN_CI_NONRUNTIME_WALK_LIMIT= 25) whilecommit_non_runtime_only?invokesscript/ci-changes-detectorwith the samenon_runtime_onlyoutput CI uses. The gate then queries GitHub check-runs for the first commit that isn't provably non-runtime-only. Pass/fail rules are unchanged—only which SHA is evaluated.Fail-safe: missing detector, errors, or ambiguous output stop the walk and evaluate the current commit (same as old HEAD behavior).
RELEASE_CI_EVALUATE_HEAD=trueforces strict HEAD evaluation. Rake docs describe the policy and env var.Specs cover walk-back, detector invocation, parent resolution, and stub
main_ci_evaluation_shain existingfetch_main_ci_checksexamples.Reviewed by Cursor Bugbot for commit b708ec4. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
RELEASE_CI_EVALUATE_HEADenvironment variable option for controlling how commits are evaluated. Improved logic to better handle runtime-bearing commits during release checks. Expanded test coverage for commit evaluation behaviors to ensure reliable release processes.