fix(release): address PR #3407 review feedback round 2 (7 items)

Hook (.claude/hooks/main-ci-status.sh):
- write_cache_atomic prints from $1 instead of re-reading via cat
  (claude[bot], 2 inline duplicates + summary)
- jq slurp now dedupes reruns by max_by(id), mirroring the Ruby gate
  in release.rake — keeps session-time and release-time CI views in
  sync when a check has been re-run (cursor bugbot)
- Defensive ":=0" defaults for failed_count/in_progress_count guard
  the partial-output edge case (claude[bot])

rakelib/release.rake:
- validate_main_ci_status! now flags partial-missing required checks
  with a new :missing_required_checks violation kind. Branch protection
  would refuse this merge state, so a release shouldn't ship through it.
  The all-missing case keeps :no_required_checks for clarity
  (chatgpt-codex-connector P1)
- Comment in run_release_preflight_checks! explaining why the main-CI
  status check is wired separately in with_release_checkout
  (claude[bot] summary)

react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb:
- passing_run/failing_run/in_progress_run helpers now include "id"
  defaults to match real GitHub API responses (claude[bot])
- New test: aborts and lists missing required check names when some
  required checks are absent on a prerelease (chatgpt-codex-connector P1)
- New test: warns instead of aborting when git fetch fails with
  allow_override, matching the dry-run symmetric case (claude[bot] summary)

63 examples, 0 failures.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: justin808

.claude/hooks/main-ci-status.sh

@@ -39,15 +39,18 @@ fail_open() {
   exit 0
 }
 
-# Helper: atomically replace the cache file with the contents of $1, then cat
-# the new file to stdout. A direct `tee` would leave a partial file readable
+# Helper: atomically replace the cache file with the contents of $1, then
+# print $1 to stdout. A direct `tee` would leave a partial file readable
 # by a concurrent session-start if the script were interrupted mid-write.
+# We print from the parameter rather than re-reading the file, so a
+# concurrent delete between the `mv` and the print cannot trigger a
+# misleading "cache write failed" fail_open.
 write_cache_atomic() {
   local tmp
   tmp=$(mktemp "${CACHE_FILE}.XXXXXX") || return 1
   printf '%s' "$1" >"${tmp}" || { rm -f "${tmp}"; return 1; }
   mv -f "${tmp}" "${CACHE_FILE}" || { rm -f "${tmp}"; return 1; }
-  cat "${CACHE_FILE}"
+  printf '%s' "$1"
 }
 
 command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed"
@@ -78,8 +81,16 @@ checks_jsonl=$(gh api \
   --jq '.check_runs[]' \
   2>/dev/null) || fail_open "gh api check-runs failed"
 
-checks_json=$(echo "${checks_jsonl}" | jq -s '[.[] | {name, status, conclusion, html_url}]' 2>/dev/null) \
-  || fail_open "jq slurp failed"
+# Collapse multiple runs per check name to the most recent attempt (highest
+# check_run id). Without this, an old failed run that was later re-run and
+# passed would still be picked up by the `failed` filter below and show as
+# red here — while `validate_main_ci_status!` in `release.rake` (which does
+# the same dedup) would correctly report green. Keep the two in sync.
+checks_json=$(echo "${checks_jsonl}" | jq -s '
+  [.[] | {id, name, status, conclusion, html_url}]
+  | group_by(.name)
+  | map(max_by(.id))
+' 2>/dev/null) || fail_open "jq slurp failed"
 
 # Aggregate counts with jq. `success`, `skipped`, `neutral` are all "passing".
 # Anything completed with another conclusion is a failure. Anything not yet
@@ -105,6 +116,10 @@ total=$(echo "${summary}" | grep "^TOTAL=" | cut -d= -f2)
 passed=$(echo "${summary}" | grep "^PASSED=" | cut -d= -f2)
 failed_count=$(echo "${summary}" | grep "^FAILED_COUNT=" | cut -d= -f2)
 in_progress_count=$(echo "${summary}" | grep "^IN_PROGRESS_COUNT=" | cut -d= -f2)
+# Default to 0 when the parse step produced no line (partial-output edge case).
+# The `total=0` branch below already covers the all-empty case, but a defensive
+# default here keeps `[ "${failed_count}" -gt 0 ]` from silently failing.
+: "${failed_count:=0}" "${in_progress_count:=0}"
 
 # Build the output as a single string, then atomically swap it into the cache
 # so a concurrent reader never sees a half-written file.

rakelib/release.rake

@@ -148,6 +148,10 @@ def verify_gh_auth(monorepo_root:)
 end
 
 def run_release_preflight_checks!(monorepo_root:, dry_run:)
+  # The main-CI status check (`validate_main_ci_status!`) is intentionally
+  # NOT in this function — it runs inside `with_release_checkout` (after
+  # `git pull --rebase` but before tagging) so it still fires under
+  # `dry_run: true` and surfaces the warning operators need to see.
   return if dry_run
 
   puts "\n#{'=' * 80}"
@@ -385,6 +389,9 @@ def format_main_ci_status_violation(kind:, short_sha:, runs:) # rubocop:disable
              "CI may not have started yet, or the GitHub Checks API is unavailable."
            when :no_required_checks
              "❌ No required CI check runs found on origin/main (commit #{short_sha})."
+           when :missing_required_checks
+             "❌ Some required CI checks are missing on origin/main (commit #{short_sha}). " \
+             "Branch protection would refuse this merge."
            else
              "❌ CI on origin/main is not healthy (commit #{short_sha})."
            end
@@ -457,14 +464,32 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
   required_names = is_prerelease ? required_check_names_for_main(monorepo_root: monorepo_root) : nil
   evaluated = required_names.nil? ? check_runs : check_runs.select { |run| required_names.include?(run["name"]) }
 
-  if evaluated.empty? && !required_names.nil?
-    handle_main_ci_status_violation!(
-      message: format_main_ci_status_violation(kind: :no_required_checks, short_sha: short_sha, runs: nil) +
-               "\nRequired: #{required_names.join(', ')}",
-      allow_override: allow_override,
-      dry_run: dry_run
-    )
-    return
+  # When branch protection lists required checks, treat any missing required
+  # check as blocking. Branch protection would refuse the merge in this state,
+  # so a release that ignored the gap would ship against a commit GitHub
+  # itself considers unverified. `:no_required_checks` covers the all-missing
+  # case (typically: CI hasn't started yet); `:missing_required_checks` covers
+  # the partial case (some required workflows ran, others never registered —
+  # usually a renamed/deleted workflow that branch protection still requires).
+  unless required_names.nil?
+    missing_names = required_names - evaluated.map { |run| run["name"] }
+    if missing_names.length == required_names.length
+      handle_main_ci_status_violation!(
+        message: format_main_ci_status_violation(kind: :no_required_checks, short_sha: short_sha, runs: nil) +
+                 "\nRequired: #{required_names.join(', ')}",
+        allow_override: allow_override,
+        dry_run: dry_run
+      )
+      return
+    elsif missing_names.any?
+      handle_main_ci_status_violation!(
+        message: format_main_ci_status_violation(kind: :missing_required_checks, short_sha: short_sha, runs: nil) +
+                 "\nRequired: #{required_names.join(', ')}\nMissing: #{missing_names.join(', ')}",
+        allow_override: allow_override,
+        dry_run: dry_run
+      )
+      return
+    end
   end
 
   # Report failures before in-progress runs. If both are present, the operator

react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb

@@ -499,26 +499,29 @@
     let(:sha) { "abc1234def5678abcdef" }
     let(:short_sha) { "abc1234d" }
 
-    def passing_run(name)
+    def passing_run(name, id: rand(1_000_000))
       {
+        "id" => id,
         "name" => name,
         "status" => "completed",
         "conclusion" => "success",
         "html_url" => "https://github.com/shakacode/react_on_rails/runs/#{name.gsub(/\W/, '_')}"
       }
     end
 
-    def failing_run(name, conclusion: "failure")
+    def failing_run(name, conclusion: "failure", id: rand(1_000_000))
       {
+        "id" => id,
         "name" => name,
         "status" => "completed",
         "conclusion" => conclusion,
         "html_url" => "https://github.com/shakacode/react_on_rails/runs/#{name.gsub(/\W/, '_')}"
       }
     end
 
-    def in_progress_run(name)
+    def in_progress_run(name, id: rand(1_000_000))
       {
+        "id" => id,
         "name" => name,
         "status" => "in_progress",
         "conclusion" => nil,
@@ -756,6 +759,28 @@ def in_progress_run(name)
         end.to raise_error(SystemExit, /No required CI check runs found.*DoesNotExist/m)
       end
     end
+
+    context "when some required checks are present but others are missing on a prerelease" do
+      it "aborts and lists the missing required check names" do
+        allow(self).to receive(:fetch_main_ci_checks)
+          .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false)
+          .and_return(sha: sha, check_runs: [passing_run("Lint"), passing_run("Test")])
+        allow(self).to receive(:required_check_names_for_main)
+          .with(monorepo_root: monorepo_root).and_return(%w[Lint Test Build])
+
+        expect do
+          validate_main_ci_status!(
+            monorepo_root: monorepo_root,
+            is_prerelease: true,
+            allow_override: false,
+            dry_run: false
+          )
+        end.to raise_error(
+          SystemExit,
+          /Some required CI checks are missing.*Missing:\s*Build/m
+        )
+      end
+    end
   end
 
   describe "#fetch_main_ci_checks" do
@@ -773,6 +798,18 @@ def in_progress_run(name)
       end.to raise_error(SystemExit, %r{Unable to fetch origin/main})
     end
 
+    it "warns instead of aborting when `git fetch` fails with allow_override" do
+      allow(Open3).to receive(:capture2e)
+        .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet")
+        .and_return(["fetch failed: network down", failure_status])
+
+      result = nil
+      expect do
+        result = fetch_main_ci_checks(monorepo_root: monorepo_root, allow_override: true)
+      end.to output(%r{CI STATUS OVERRIDE enabled.*Unable to fetch origin/main}m).to_stdout
+      expect(result).to be_nil
+    end
+
     it "warns instead of aborting when `git fetch` fails in dry-run mode" do
       allow(Open3).to receive(:capture2e)
         .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet")