Skip to content

Release: backport strict HEAD retry evidence to 17.0.0 - #4684

Merged
justin808 merged 6 commits into
release/17.0.0from
codex/backport-4661-release-17
Jul 15, 2026
Merged

Release: backport strict HEAD retry evidence to 17.0.0#4684
justin808 merged 6 commits into
release/17.0.0from
codex/backport-4661-release-17

Conversation

@justin808

@justin808 justin808 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Why

Backport #4661 to the active release/17.0.0 train so release retry guidance fails closed when GitHub CI evidence is incomplete, malformed, ambiguous, pending, or unhealthy. The next RC should not recommend an idempotent publish retry unless the evidence at the exact release commit is structurally valid and finalized.

This is release-process hardening only. It does not change runtime packages, public APIs, product versions, dependency ranges, or lockfiles.

What changed

  • adds authenticated JSONL pagination envelopes so valid empty API pages remain distinguishable from blank, truncated, or malformed output
  • validates exact-HEAD check runs and legacy statuses before recommending a retry
  • keeps non-required checks advisory during normal prerelease evaluation, while strict recovery fails closed on any unresolved evidence
  • rejects the CI override for stable/final promotion
  • documents the exact-HEAD recovery boundary for release operators
  • accepts GitHub's documented optional branch-protection checks and app_id fields while preserving malformed evidence for fail-closed validation
  • accepts the mixed LF/CRLF response framing emitted by current gh api --include without accepting arbitrary reverse-mixed framing

The backport ports the three source files from #4661. One main-only message hunk is omitted because the release branch already carries equivalent retry wording from #4676; the strict evidence behavior and source spec coverage are preserved, including the newer post-pull prerelease guard already on the release branch.

Validation

  • source release-helper spec hunks: 22/22 ported
  • source release-task hunks: 21/22 ported; one message-only hunk has equivalent release-branch wording
  • final branch is merged onto exact release tip 4ab97d6cfe5ea1af975643d743fb1b65f8439455 with no conflicts
  • combined release-helper and hosted-workflow suite — 338 examples, 0 failures
  • targeted RuboCop — 2 files, 0 offenses; Ruby syntax, Prettier, and git diff --check passed
  • all 22 source spec hunks and 21 applicable release-task hunks are present; the strict-HEAD documentation block is byte-identical to Release: require strict HEAD evidence before retry guidance #4661
  • Release: backport fail-closed prerelease retry to 17.0.0 #4676's post-pull prerelease guard and stale/matching boundaries remain present
  • product metadata remains 17.0.0.rc.11 / 17.0.0-rc.11
  • pre-push branch lint passed all 33 branch Ruby files; the Markdown-link hook could not parse the current .lychee.toml because this host has Lychee 0.24.2, but this backport adds no guide URLs and hosted link checks cover the PR
  • RED/GREEN regression coverage for the LF-status/CRLF-header framing emitted by gh api --include
  • live gh 2.89.0 probe against the unprotected PR branch now parses the 404 response and corroborates the branch as unprotected
  • real jq contract verifies documented optional checks / app_id shapes; malformed booleans, numbers, strings, objects, array entries, and missing contexts remain fail-closed
  • independent adversarial code/spec review of both corrective deltas found no findings

Review disposition

  • Claude requested a real gh api --include compatibility check. The probe exposed a genuine mixed-newline compatibility bug, fixed in c521db0e6 with narrow framing rules and regression coverage.
  • Greptile raised partial pagination. Official gh 2.89.0 source confirms any page request or response-processing failure returns a nonzero command status, and this release helper rejects all captured output on nonzero status; no code change was needed.
  • Codex identified that GitHub's documented legacy required-status response may omit checks, and modern entries may omit app_id. Fixed in 1d7f7a200 by normalizing only those optional omissions while preserving malformed evidence for strict validation.

Classification

Confidence note:

  • Validated: source semantic coverage, focused release behavior, preservation of the existing fail-closed prerelease guard, and both review-driven compatibility fixes
  • Pending: none; exact-head hosted CI and current-head review evidence are complete
  • Residual risk: moderate because this changes the release gate; the backport is confined to the release task, its focused specs, and operator documentation

Summary by CodeRabbit

  • Release Process

    • Added an optional CI-status waiver for prerelease releases.
    • Stable and final releases now require CI evidence without status waivers.
    • Improved release validation for exact commit CI results, including clearer guidance when evidence is missing, incomplete, or unhealthy.
    • Strengthened required-check discovery and reporting for protected branches.
  • Documentation

    • Updated release task arguments, environment variables, CI gates, and exact-commit evaluation guidance.
  • Quality

    • Expanded coverage for CI evidence validation, release overrides, malformed responses, branch protection scenarios, and dry-run messaging.

…ort-4661-release-17

* origin/release/17.0.0:
  CI: backport hosted Pro compatibility coverage to 17.0.0 (#4677)
…ort-4661-release-17

* origin/release/17.0.0:
  Fix: backport React 18 non-RSC streaming to 17.0.0 (#4679)
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The release task adds a prerelease-only CI override, standardizes GitHub CI evidence parsing and validation, strengthens required-check discovery, and gates strict exact-HEAD recovery on complete healthy evidence. Documentation and extensive helper specs reflect the updated behavior.

Changes

Release CI gating

Layer / File(s) Summary
Prerelease override contract and task wiring
internal/contributor-info/releasing.md, rakelib/release.rake, react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb
The release interface documents a fourth CI override argument, runtime enforcement limits overrides to prereleases, and validation receives branch and target-version context.
GitHub CI evidence acquisition and validation
rakelib/release.rake, react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb
Check runs and legacy statuses use framed JSONL queries with encoding, envelope, identity, state, and required-check validation.
Exact-HEAD evaluation and recovery guidance
rakelib/release.rake, react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb
CI runs are deduplicated and classified before exact-HEAD recovery guidance is offered only for complete healthy evidence; tests cover unavailable, malformed, pending, failed, and unknown evidence.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: backporting strict exact-HEAD retry evidence handling to the 17.0.0 release branch.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/backport-4661-release-17

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

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

@justin808

Copy link
Copy Markdown
Member Author

+ci-status

@justin808

Copy link
Copy Markdown
Member Author

+ci-force-full

@github-actions

Copy link
Copy Markdown
Contributor

CI Status

Head SHA: 9a29e0d796da
Changed files: 3
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: absent
force-full-hosted-ci label: absent
Current hosted-CI waiver: not present for this SHA

Only the required gate is active unless hosted CI is requested.

@github-actions github-actions Bot added force-full-hosted-ci Bypass optimized hosted CI selection and run all hosted suites ready-for-hosted-ci Run optimized hosted GitHub CI for this PR labels Jul 15, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Force-Full Hosted CI Requested

Triggered 9 workflow(s) for 9a29e0d796da.
Mode: force-full hosted CI (bypasses optimized change selection).
Added ready-for-hosted-ci and force-full-hosted-ci, so future commits will bypass optimized hosted CI selection until +ci-stop-full is used.

View progress in the Actions tab.

@greptile-apps

greptile-apps Bot commented Jul 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR hardens CI evidence checks for the 17.0.0 release process. The main changes are:

  • Adds framed validation for paginated GitHub check runs and statuses.
  • Distinguishes unknown branch protection from a valid empty configuration.
  • Restricts CI overrides to prereleases.
  • Adds strict exact-HEAD recovery guidance and operator documentation.

Confidence Score: 5/5

The release flow looks mergeable, with a pagination hardening case worth addressing.

  • Invalid and unknown CI evidence blocks the release.
  • Stable releases cannot use the CI override.
  • Partial multi-page output has no independent completeness check if the CLI does not return a failing final status.

rakelib/release.rake

Important Files Changed

Filename Overview
rakelib/release.rake Adds strict GitHub evidence parsing, required-check discovery, exact-HEAD recovery, and prerelease-only override enforcement; pagination completeness remains conditional on the CLI surfacing every page failure.
react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb Expands release-helper coverage for malformed evidence, branch-protection discovery, recovery guidance, and override policy.
internal/contributor-info/releasing.md Documents strict exact-HEAD evaluation and the prerelease-only CI override.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Fetch release branch] --> B[Choose walkback SHA or exact HEAD]
  B --> C[Fetch GitHub CI evidence]
  C --> D{Evidence valid?}
  D -- No --> X[Block release]
  D -- Yes --> E[Discover required checks]
  E --> F{Discovery known?}
  F -- No --> X
  F -- Yes --> G[Evaluate release policy]
  G --> H{Healthy?}
  H -- Yes --> I[Continue release]
  H -- No --> J{Exact HEAD complete and healthy?}
  J -- No --> X
  J -- Yes --> K[Offer strict exact-HEAD retry]
Loading
%%{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 release branch] --> B[Choose walkback SHA or exact HEAD]
  B --> C[Fetch GitHub CI evidence]
  C --> D{Evidence valid?}
  D -- No --> X[Block release]
  D -- Yes --> E[Discover required checks]
  E --> F{Discovery known?}
  F -- No --> X
  F -- Yes --> G[Evaluate release policy]
  G --> H{Healthy?}
  H -- Yes --> I[Continue release]
  H -- No --> J{Exact HEAD complete and healthy?}
  J -- No --> X
  J -- Yes --> K[Offer strict exact-HEAD retry]
Loading

Reviews (1): Last reviewed commit: "Docs: clarify CI override is prerelease-..." | Re-trigger Greptile

Comment thread rakelib/release.rake
Comment thread rakelib/release.rake
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review: Release CI-gate strict-HEAD backport (17.0.0)

Reviewed the diff (internal/contributor-info/releasing.md, rakelib/release.rake, react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb) and cross-checked the current state of rakelib/release.rake against the diff.

Overview

This backports #4661's release-gate hardening onto release/17.0.0: the CI-status gate now fails closed on incomplete/malformed/ambiguous GitHub API evidence instead of fail-safe, adds a JSONL envelope/item framing scheme so a genuinely-empty API page is distinguishable from truncated or malformed output, validates check-run/legacy-status shape before trusting it, restricts the "strict exact-HEAD" retry to only be offered after exact-HEAD evidence is confirmed healthy, and — notably — rejects override_ci_status/RELEASE_CI_STATUS_OVERRIDE outright for stable/final releases (ensure_ci_status_override_is_prerelease!), including at the ShakaPerf gate downstream since allow_ci_status_override is now guaranteed false for stable releases before it's threaded through.

Code quality

  • The refactor from monolithic fetch_main_ci_checks/fetch_main_commit_statuses bodies into small, named predicates (valid_ci_check_run?, valid_ci_status?, valid_required_checks_payload?, etc.) and a shared fetch_github_jsonl helper is a real improvement — the old code duplicated the fetch/parse/error-handle pattern twice, this collapses it to one place used by both check-runs and statuses (and now exact-HEAD recovery reuses the same fetchers).
  • Splitting validate_main_ci_status!'s decision logic into pure main_ci_status_evaluation (no I/O, returns a {kind:, message:}/{kind: :healthy, healthy_count:} struct) is a good separation — it's what makes exact_head_recovery_guidance able to re-run the same evaluation against exact-HEAD evidence without duplicating the failure/in-progress/missing-required logic.
  • REQUIRED_CHECK_DISCOVERY_UNKNOWN = Object.new.freeze compared via .equal? is the right pattern for a real three-state result (known-empty vs. known-populated vs. unknown) where nil was previously overloaded to mean "no required checks" for both the safe and unsafe cases — this is the crux of the fail-closed fix.
  • Test coverage is unusually thorough for a release script: dozens of new adversarial cases (malformed app ids, missing created_at, mismatched status-envelope SHA, mixed CRLF/LF, partial JSONL pages) all asserting fail-closed behavior. This gives good confidence the porting is behaviorally faithful to Release: require strict HEAD evidence before retry guidance #4661, which the PR description also claims (22/22 spec hunks, 21/22 rake hunks, one message-only hunk substituted with equivalent release-branch wording from Release: backport fail-closed prerelease retry to 17.0.0 #4676).

Potential issues / things worth double-checking

  • Left one inline comment on gh_included_response_status/known_branch_without_required_checks?: this is the one path in the PR that hand-parses raw gh api --include HTTP status-line/header text instead of consuming --jq-shaped JSON. It's fail-closed if the format doesn't match (blocks release rather than silently bypassing), but it's worth confirming against real gh CLI output rather than only the RSpec-mocked byte sequences, since a future CLI format change would degrade the "known unprotected branch" fast-path to always block.
  • ensure_ci_status_override_is_prerelease! is called both in the new ci_status_override_allowed_for_release! (task-level gate, before CI validation runs) and again at the top of validate_main_ci_status!. This looks intentional (defense-in-depth for direct/test callers of validate_main_ci_status! that skip the task-level gate) and is covered by a dedicated spec, but flagging in case the duplication was accidental.
  • Minor/non-blocking: in validate_main_ci_status!, recovery_ci_evidence_healthy? (and its statuses input) gets computed for every non-healthy evaluation[:kind], but the resulting guidance is only ever used when kind is :no_checks/:no_required_checks (line ~2374). For :failed/:in_progress/:unknown_status/:missing_required_checks this is wasted work, not a correctness bug (no extra network call beyond what's already been fetched).

Security / correctness

No security concerns — this only tightens an internal release-gate script; it doesn't touch runtime packages, public APIs, or shipped code. The design consistently favors failing closed (blocking a release) over failing open on ambiguous evidence, which is the correct posture for a publish gate. I was not able to execute the RSpec suite in this sandbox (tool execution was restricted), so I relied on static review of the code and the extensive existing/added spec coverage rather than a live run — the PR description states 335 examples pass locally, which lines up with what I read in the diff.

Overall: solid, well-tested hardening change with only minor nits.

@justin808

justin808 commented Jul 15, 2026

Copy link
Copy Markdown
Member Author

Review feedback addressed

Current head: c521db0e680e79a713e1b43f99c6c3013321c2fb

  • Greptile pagination concern: verified false positive for supported gh 2.89.0; any page failure makes the CLI exit nonzero, and the release helper rejects all captured output on nonzero status. Replied with official-source evidence and resolved.
  • Claude --include compatibility check: the live probe exposed LF status + CRLF headers, which the prior parser rejected. Fixed with a narrow framing rule plus a RED/GREEN regression test; reverse/arbitrary mixed framing remains fail-closed. Replied and resolved.
  • Current-head Claude wrapper-deduplication nit: auto-deferred as an optional, non-mechanical refactor; the wrappers intentionally preserve distinct Open3 return shapes. Replied and resolved.

Validation: 336 focused examples, 0 failures; targeted RuboCop 2 files/0 offenses; Ruby syntax and git diff --check clean; pre-push branch lint 33 files/0 offenses; independent adversarial delta review found no findings. Hosted CI and current-head automated reviews are now restarting for this head.

@justin808

Copy link
Copy Markdown
Member Author

+ci-status

@justin808

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Comment thread rakelib/release.rake
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review: Backport strict HEAD retry evidence to 17.0.0

This is a release-process-only backport of #4661 onto release/17.0.0. It touches no runtime/shipped code — only rakelib/release.rake, its spec, and internal/contributor-info/releasing.md. Reviewed the full diff plus the resulting file state.

Overall assessment

The change is a well-engineered hardening of the release CI gate: it makes every GitHub API evidence path (Checks API, Statuses/legacy API, branch-protection required-checks lookup) fail closed on anything ambiguous — malformed JSON, wrong SHA, non-positive IDs, missing created_at, empty/blank output, mismatched envelope framing, etc. — instead of the previous fail-open/"treat as unknown → skip" behavior in a few spots. The new JSONL envelope-framing scheme (CI_JSONL_FRAME_KEY/ENVELOPE_FRAME/ITEM_FRAME) is a nice solution to the classic "how do you tell a genuinely empty paginated response apart from a truncated/malformed one" problem, and it's defended by tests for adversarial cases (a real check-run payload that happens to contain a key that collides with the marker, mixed CRLF/LF framing from gh api --include, control bytes in headers, incompatible encodings, etc.).

Test coverage for this is unusually thorough (335 combined examples per the PR description), covering both the happy path and a long list of "what if the evidence is subtly wrong" cases for exact-HEAD recovery. RuboCop and Ruby-syntax checks are clean per the PR description.

Correctness

  • ci_status_override_allowed_for_release! / ensure_ci_status_override_is_prerelease! correctly reject RELEASE_CI_STATUS_OVERRIDE=true (or the positional arg) for stable/final releases before any CI evidence is fetched — this is the core safety property the backport is adding, and it's exercised by dedicated specs.
  • The "exact HEAD healthy but walked-back SHA has no usable runs" recovery path only ever prints a copy-pasteable retry command; it never auto-applies the strict evaluation itself, and it explicitly withholds that guidance whenever the other, already-visible walked-back or exact-HEAD evidence is still pending/failing/malformed. The negative-guidance tests (expect(...).not_to include("RELEASE_CI_EVALUATE_HEAD=true")) are a good way to pin that down.
  • required_check_names_for_branch now distinguishes "genuinely unprotected branch" (safe to treat as no required checks) from "ambiguous 404/error" via a second corroborating gh api --include + branch-info lookup (known_branch_without_required_checks?), rather than trusting the first error message string. That closes a real gap where a rate-limited or auth-scoped 404 could previously have been mistaken for "no branch protection."
  • Scoping of allow_ci_status_override (computed inside the with_release_checkout block, used later in the same block for both validate_main_ci_status! and run_shakaperf_release_gate!) is correct — no variable-scope issue from moving its computation later in the task body.

Minor code-quality nit (left inline)

  • capture_gh_stdout_and_stderr (rakelib/release.rake:210) is a near-duplicate of capture_gh_output (rakelib/release.rake:204) — same signature and same missing-binary abort message, differing only in Open3.capture3 vs capture2e. Worth collapsing into one helper parameterized by capture method, so the two don't drift.

Other observations (not blocking)

  • ensure_ci_status_override_is_prerelease! is invoked twice — once eagerly in ci_status_override_allowed_for_release!, and again defensively at the top of validate_main_ci_status!. This looks intentional (defense-in-depth against a caller that skips the first call) and is covered by a test, but it's worth a one-line comment at the second call site explaining why it's not redundant, for the next reader.
  • The hand-rolled HTTP response parsing in gh_included_response_parts/gh_included_response_status (splitting gh api --include output into status line + headers + body, handling mixed \n/\r\n framing) is inherently fiddly. It's well-tested for the cases exercised here, but it's the kind of code that's easy to regress if gh's --include output format ever changes; a short doc comment pointing at why this exists (rather than relying on a real HTTP parser) would help future maintainers.
  • This is release tooling, not shipped product code, so the security surface is limited to whoever runs bundle exec rake release; no injection or unsafe external input concerns found (all gh/git invocations use argument arrays, not shell interpolation).

No blocking issues found. The backport appears faithful to its stated scope (source hunks ported, one main-only message hunk correctly superseded by existing release-branch wording from #4676).

@github-actions

Copy link
Copy Markdown
Contributor

CI Status

Head SHA: c521db0e680e
Changed files: 3
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: present
force-full-hosted-ci label: present
Current hosted-CI waiver: not present for this SHA

Force-full hosted CI is enabled for this PR.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c521db0e68

ℹ️ 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".

Comment thread rakelib/release.rake Outdated
@justin808

Copy link
Copy Markdown
Member Author

+ci-status

@justin808

Copy link
Copy Markdown
Member Author

+ci-run-hosted

@justin808

Copy link
Copy Markdown
Member Author

@coderabbitai review

@justin808

Copy link
Copy Markdown
Member Author

Address-review refresh for exact head 1d7f7a200: the valid optional-field P1 is fixed, replied to, and resolved. The jq projection normalizes documented omissions while preserving malformed evidence for fail-closed Ruby validation. Local evidence: 338 combined examples, targeted RuboCop clean, syntax/diff clean, 33-file branch lint clean, and independent adversarial code/spec review with no findings. Current-head hosted CI and review agents have been re-requested; this is now the declared final candidate.

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

Copy link
Copy Markdown
Contributor

CI Status

Head SHA: 1d7f7a200835
Changed files: 3
Docs-only heuristic (matches ci-changes-detector metadata paths): no
ready-for-hosted-ci label: present
force-full-hosted-ci label: present
Current hosted-CI waiver: not present for this SHA

Force-full hosted CI is enabled for this PR.

@github-actions

Copy link
Copy Markdown
Contributor

Hosted CI Requested

Triggered 9 workflow(s) for 1d7f7a200835.
Mode: optimized hosted CI (path-selected by script/ci-changes-detector).
Added ready-for-hosted-ci, so future commits will keep running optimized hosted CI until +ci-stop-hosted is used.

View progress in the Actions tab.

Comment thread rakelib/release.rake
Comment thread rakelib/release.rake
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review: Backport strict HEAD retry evidence to release/17.0.0

Overview

This backports #4661's release-gate hardening onto the release/17.0.0 branch: the CI-status gate now fails closed (rather than fail-safe-open) whenever GitHub's CI evidence is empty, malformed, partially paginated, or ambiguous, and the RELEASE_CI_STATUS_OVERRIDE escape hatch is now rejected outright for stable/final promotions (prerelease-only from here on). It also adds a well-scoped "exact-HEAD recovery" diagnostic so operators get an explicit, evidence-gated retry command instead of a blanket suggestion to force strict HEAD evaluation.

This is release-tooling-only (rakelib/release.rake, its spec, and the operator doc) — no runtime/package/API changes, consistent with the PR's not_user_visible changelog classification.

Code quality

  • The refactor is well factored: JSONL fetch/parse/validate is centralized in fetch_github_jsonl / validated_github_jsonl_items, with check-runs and status fetches sharing one code path (fetch_ci_check_runs_for_sha / fetch_ci_statuses_for_sha) instead of duplicating the old inline Open3.capture2e + rescue blocks.
  • Validation predicates (valid_ci_check_run?, valid_ci_status?, valid_required_check_entry?, etc.) are small, single-purpose, and consistently used both by the normal gate and by the new exact-HEAD recovery path — good reuse rather than parallel logic that could drift.
  • main_ci_status_evaluation returning a {kind:, message:} struct (instead of the old method that directly called abort/handle_main_ci_status_violation! inline) is a nice decoupling — it lets exact_head_recovery_guidance reuse the same evaluation logic against head_sha evidence without duplicating the failure/in-progress/missing-checks branching.
  • The new internal/contributor-info/releasing.md section (exact-HEAD evidence table + examples) is clear and gives operators a decision table instead of just "set this env var."
  • Left two inline nits: an overly broad rubocop:disable span, and a fail-closed dependency on matching GitHub's exact English error-message text for the "branch is known-unprotected" fallback probe.

Correctness

I traced the core validate_main_ci_status! / main_ci_status_evaluation / exact_head_recovery_guidance control flow by hand, specifically hunting for any fail-open gap (the highest-impact bug class for a release gate):

  • The :no_checks / :no_required_checks → exact-HEAD recovery path only offers RELEASE_CI_EVALUATE_HEAD=true guidance when recovery_ci_evidence_healthy? confirms the exact-HEAD evidence is structurally valid and every check/status is completed+passing; any pending/failed/malformed evidence routes back through main_ci_status_evaluation and is reported as blocking (in-progress/failed/unknown), not silently accepted.
  • Stable-vs-prerelease override rejection (ensure_ci_status_override_is_prerelease!) is enforced both at task setup (ci_status_override_allowed_for_release!) and again inside validate_main_ci_status!, so there's no code path where a stable release can reach the gate with allow_override: true.
  • check_run_app_id / normalize_required_check_entries dropped the old &.to_i coercion, but since required inputs are now validated as real Integers up front (positive_github_id?) before reaching comparison, this isn't a behavior regression — it's tightening, not loosening.

I did not find a fail-open bug in this pass. I wasn't able to execute the Ruby spec suite in this environment (sandboxed shell blocked bundle exec rspec), so I'm relying on static reading rather than confirming the claimed "336 examples, 0 failures" myself — worth double-checking that run was against this exact backported branch tip if not already done in CI.

Security

  • No new untrusted input surface: all new parsing (gh_included_json_response, the JSONL envelope framing) operates on output from the trusted, locally-invoked gh CLI, not attacker-controlled network input directly.
  • The mixed-newline (\r/\r\n) framing parser for gh api --include fails closed to "unknown" on anything not cleanly LF or CRLF throughout, which is the right default for a security-relevant gate — reduces the risk of misparsing a response into an incorrect "unprotected branch" determination.
  • Rejecting RELEASE_CI_STATUS_OVERRIDE for stable/final promotions is a meaningful hardening: previously any release (including a final stable tag) could bypass CI validation entirely via one env var; now that escape hatch is scoped to prereleases only.

Test coverage

The spec file grew substantially (~2.3k lines) with dedicated coverage for: the new override/prerelease guard, JSONL envelope/frame validation, the mixed-newline HTTP framing regression, unprotected-branch/required-checks-disabled discovery, and strict exact-HEAD evaluation success/failure/pending paths. That's a good sign for a change this security-sensitive, though as noted above I couldn't independently execute the suite here to confirm green.

Risk assessment

Given this only affects release tooling (not shipped code), and the change is uniformly in the fail-closed direction, the blast radius of a residual bug would be "a valid release gets incorrectly blocked" (annoying, recoverable) rather than "an unhealthy release ships" — the more dangerous failure mode for a release gate. That asymmetry, combined with the extensive spec coverage, makes this a reasonably low-risk backport overall.

@justin808
justin808 merged commit 8601d96 into release/17.0.0 Jul 15, 2026
86 checks passed
@justin808
justin808 deleted the codex/backport-4661-release-17 branch July 15, 2026 22:24
justin808 added a commit that referenced this pull request Jul 15, 2026
## Why

Backport the diagnostic correction from #4680 to `release/17.0.0` after
the strict retry-evidence backport in #4684.

The prerelease safety guard correctly compares both the pre-pull
starting version and the post-pull checkout version, but its failure
message was still displaying and recommending the stale pre-pull
version. In cross-line or advanced-checkout cases, that guidance can
point the release operator at the wrong candidate even though downstream
policy would reject it.

## What changed

- pass the actual post-pull `current_version` to the retry-error
formatter
- update focused specs so messages and suggested commands identify the
checked-out version
- preserve the guard's comparisons against both starting and current
versions

This is the stable-patch-ID-equivalent of main PR #4680 commit
`0416c3d0aa40f98f905b3f424c3e3790d9089f09`, rebased as one commit onto
release tip `8601d96357b61b9818055a41ead46ed26d51bcbe`.

## Validation

- full release-helper spec: 337 examples, 0 failures
- targeted RuboCop: 2 files, 0 offenses
- pre-push branch RuboCop: 33 files, 0 offenses
- Ruby syntax and `git diff --check`: clean
- stable patch ID matches the reviewed main fix
- delta is exactly `rakelib/release.rake` and
`react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb`
- product metadata remains `17.0.0.rc.11` / `17.0.0-rc.11`

The local Markdown-link hook could not parse the repository's current
`.lychee.toml` with this host's Lychee 0.24.2. This PR changes no
Markdown; hosted link checks remain required.

## Classification

- Source: #4680 diagnostic correction
- Target: `release/17.0.0`, next RC
- Tracker: #3823 (`development`, RC phase)
- Changelog: `not_user_visible`

Confidence note:

- Validated: exact patch equivalence, focused release-helper behavior,
lint, syntax, diff scope, and version preservation
- Pending: none; exact-head hosted CI and configured current-head review
agents are complete
- Residual risk: low; this corrects operator-facing diagnostics without
changing the guard decision


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved prerelease release guidance to display the correct current
version after repository updates.
* Updated release error messages and commands to accurately reflect the
version available for release.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
justin808 added a commit that referenced this pull request Jul 31, 2026
…t-policy

* origin/main: (33 commits)
  Fix ci-required base ref for PR merge-ref checkouts (#4819)
  Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817)
  [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821)
  Forward-port the 17.0.1 changelog section to main (#4814)
  Handle selector metacharacters in renderComponent DOM IDs (#4808)
  [Pro] Prevent caching RSC renders with errors (#4804)
  Agents: trust Copilot review identities (#4807)
  Agents: bind fleet closeout to generated pack (#4805)
  Docs: ADR 0002 — Skills-in-package over MCP for agent-native DX (#4735)
  Scope GitHub release commands to the origin repository (#4803)
  Forward-port OSS npm license metadata fix (#4794)
  Add golden-output gate for the serverWebpackConfig generator template (#4790)
  Cover the rspack CSS SSR generator fixes and de-duplicate the loader path (#4788)
  Configure agent workflow repo policy (#4785)
  Forward-port gh include mixed framing from #4684 (#4784)
  Release: enforce one-change forward-port closeout (#4783)
  Forward-port multi-URL rolling-deploy seeding to main (#4782)
  Docs: clarify React 18 streaming without RSC (#4780)
  Docs: forward-port v17 upgrade and generator gate guidance (#4781)
  Record the final React on Rails 17.0.0 changelog (#4742)
  ...

# Conflicts:
#	AGENTS.md
#	internal/contributor-info/release-train-runbook.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

force-full-hosted-ci Bypass optimized hosted CI selection and run all hosted suites ready-for-hosted-ci Run optimized hosted GitHub CI for this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant