From 53d2286630de2c37be689d8a094ea19ba0d32674 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 10:54:47 -1000 Subject: [PATCH 01/13] docs: add design spec for main CI release guard + monitoring Captures the design for a release-time CI gate (refuses release when main is red, with override) and continuous CI-status monitoring via hooks + prose updates. Also documents the two follow-up issues to file (tanstackRouter regression, Benchmark Workflow). Co-Authored-By: Claude Opus 4.7 (1M context) --- ...2026-05-25-main-ci-release-guard-design.md | 423 ++++++++++++++++++ 1 file changed, 423 insertions(+) create mode 100644 docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md diff --git a/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md new file mode 100644 index 0000000000..137e36cde4 --- /dev/null +++ b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md @@ -0,0 +1,423 @@ +# Main CI release guard + continuous monitoring + +**Status**: Design — awaiting user review +**Date**: 2026-05-25 +**Owner**: Justin Gordon (with Claude Code) + +## Problem + +CI on `main` has been failing for the last 8+ push commits, primarily on: + +- **JS unit tests for Renderer package** — `tests/tanstackRouter.test.ts:1669` (regression from #3213) +- **Benchmark Workflow** — port 3001 + missing summary file (#3403 attempted, insufficient) + +Today, nothing blocks a release from going out on top of a broken `main`. Nothing makes an AI coding agent (or human) notice that `main` is red before opening a PR on top of it. + +This design adds two guards: + +1. **Release-time gate**: `rake release` refuses to publish when CI on `main` isn't healthy, unless explicitly overridden. +2. **Session-time signal**: every Claude Code session gets `main`'s CI status injected at start (and again before pushing to `main` / opening a PR). + +The actual CI failures are tracked in separate issues (filed as part of this work) and will be fixed in separate PRs by parallel Conductor worktrees. + +## Scope + +This PR delivers: + +- `rakelib/release.rake` — new CI status check + override +- `release_rake_helpers_spec.rb` — tests for the new check +- `.claude/hooks/main-ci-status.sh` — hook script that prints `main` CI status +- `.claude/settings.json` — wire the hook into `SessionStart` and `PreToolUse` +- `AGENTS.md` — new "Main branch health" section +- `CLAUDE.md` — one-line pointer to the AGENTS.md section +- Two GitHub issues filed (tanstackRouter regression, Benchmark Workflow), each ready for a parallel worktree + +Out of scope: + +- Fixing the tanstackRouter test (separate worktree, separate PR) +- Fixing the Benchmark Workflow (separate worktree, separate PR) +- Branch-protection policy changes on GitHub itself +- Any change to the merge / PR-open flow (e.g., bot comments) + +## Design — Section 1: Release-time CI gate + +### Where it runs + +In `rakelib/release.rake`, inside `run_release_preflight_checks!`, after `verify_npm_auth` and `verify_gh_auth`. We already know which version we're shipping by this point and we have a verified `gh` session. + +Note: `run_release_preflight_checks!` runs _before_ `with_release_checkout` (and therefore before its `git pull --rebase`). The CI check must run `git fetch origin main --quiet` itself to ensure it's evaluating the latest `origin/main` SHA, not whatever the local refs happen to be. + +### Public API + +The `:release` task gets a new 4th positional argument and a paired env var, paralleling the existing `override_version_policy`: + +``` +task :release, %i[version dry_run override_version_policy override_ci_status] +``` + +``` +RELEASE_CI_STATUS_OVERRIDE=true # bypass the CI status check +``` + +The task description gets corresponding doc lines. + +### Policy + +The check first fetches `origin/main`, then evaluates the CI status of `origin/main` HEAD: + +- **Stable release** (no `.test.` / `.beta.` / `.alpha.` / `.rc.` / `.pre.` in target version): + Every check run on the commit must have `conclusion ∈ {success, skipped, neutral}`. Any other conclusion (failure, cancelled, timed_out, action_required, stale) → block. Any check still `status: in_progress` or `queued` → block. Zero check runs visible → block (we may be looking too early; the user can override). + +- **Pre-release** (RC / beta / alpha / pre / test): + Same rule, but restricted to the _required_ status checks as defined by GitHub branch protection (`gh api repos/.../branches/main/protection/required_status_checks`). Non-required checks are advisory and do not block. If branch protection isn't queryable (no protection configured, or insufficient token scope), fall back to the stable rule (treat all checks as required — fail safe). + +The target commit is **always `origin/main` HEAD**, regardless of where the release is being run from. Even pre-releases shouldn't ship when `main` is broken. + +### Failure UX + +When blocked, the error names the failing checks, links to them, and tells the operator exactly how to proceed: + +``` +❌ CI on main is not healthy — refusing to release. + +Commit: 3103496d + ❌ failure: JS unit tests for Renderer package + https://github.com/shakacode/react_on_rails/actions/runs/26404417346 + ❌ failure: Benchmark Workflow + https://github.com/shakacode/react_on_rails/actions/runs/26404417325 + +To override (use only if the failures are known-unrelated to this release): + RELEASE_CI_STATUS_OVERRIDE=true rake release[...] + # or + rake "release[16.2.0,false,false,true]" +``` + +In-progress checks get a separate message ("CI in progress — wait for it to finish, or override"). + +### Dry-run behavior + +Dry runs still run the check (so the operator sees the same diagnostic), but never abort. A red `main` in a dry-run prints a warning so the operator knows the real release would block. + +### Code shape + +Mirror the existing version-policy helpers: + +```ruby +def ci_status_override_enabled?(override_flag) + ReactOnRails::Utils.object_to_boolean(override_flag) || + ReactOnRails::Utils.object_to_boolean(ENV.fetch("RELEASE_CI_STATUS_OVERRIDE", nil)) +end + +def fetch_main_ci_checks(monorepo_root:) + # 1. `git fetch origin main --quiet` + # 2. resolve origin/main SHA (`git rev-parse origin/main`) + # 3. `gh api repos/{owner}/{repo}/commits/{sha}/check-runs --paginate` + # NB: this is the GitHub Checks API (used by Actions). We do NOT need + # the legacy Statuses API (`/commits/{sha}/status`) — all our CI runs + # through GitHub Actions, which reports via Check Runs. + # Returns: { sha:, check_runs: [{name, status, conclusion, html_url}, ...] } +end + +def required_check_names_for_main(monorepo_root:) + # `gh api repos/{owner}/{repo}/branches/main/protection/required_status_checks --jq '.contexts'` + # Returns nil if no branch protection (caller treats as "all required"). +end + +def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dry_run:) + # Apply policy, format error, abort or warn. +end +``` + +### Tests + +`react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb` gets new contexts that stub `Open3.capture2e`/`gh` calls: + +- all-success → passes +- one failure on a required check → blocks (raises) +- failure on a non-required check, prerelease → passes (advisory only) +- failure on a non-required check, stable → blocks +- check still in_progress → blocks with "in progress" message +- zero check runs visible → blocks with "no CI data" message +- override env var set → warns, returns +- override arg set → warns, returns +- branch protection query fails → falls back to "treat all as required" +- `gh` command failure → propagates the error verbatim (no silent override) + +### Edge cases and tradeoffs + +- **What if the operator wants to release from a commit other than `origin/main` HEAD?** + Today the release task already runs `git pull --rebase` (line 910) on the release worktree. By the time we check, HEAD will be `origin/main` HEAD. Releasing from elsewhere is an explicit override situation. +- **Why not check the _local_ commit being tagged?** + Stable releases create their own version-bump commit _on top of_ `origin/main`; that commit hasn't been pushed yet when we check. The question we're answering is "is the foundation we're building on healthy?", which is exactly what `origin/main` HEAD tells us. +- **Why not poll/wait for in-progress checks?** + Discussed and rejected: forces a deliberate "wait or override" decision instead of embedding a polling loop in the release script. Operators who want polling can use `gh run watch` themselves. +- **Why query `required_status_checks` instead of a hardcoded allowlist?** + Branch protection is already the source of truth for "what counts as mergeable." Reusing it avoids drift between two policy lists. If no branch protection exists, we degrade safely (treat everything as required). + +--- + +## Design — Section 2: Continuous CI monitoring + +### `.claude/hooks/main-ci-status.sh` + +A small bash script that runs `gh run list --branch main --limit 1 --event push --json conclusion,headSha,workflowName,databaseId,url,status` and emits a compact status block: + +``` +Main CI status (3103496d, pushed 7h ago): + ✅ 11 success + ❌ 2 failure: + - JS unit tests for Renderer package: https://github.com/shakacode/.../runs/... + - Benchmark Workflow: https://github.com/shakacode/.../runs/... + ⏳ 0 in_progress +``` + +Behavior: + +- Output goes to stdout (the harness injects it as additional context). +- Exits 0 always. Tooling failures (no `gh`, no auth, no network) print a one-line "main CI status unavailable: " and exit 0. We never block a session because the status check failed. +- Caches to `.claude/.main-ci-status.cache` for 5 minutes — repeated session starts in the same window reuse the cached output instead of hitting the GitHub API every time. + +### `.claude/settings.json` wiring + +- `hooks.SessionStart` — runs the script unconditionally. +- `hooks.PreToolUse` — runs the script when the next command matches: + - `gh pr create` (any args) + - `git push` to `origin main` or `origin HEAD` (regex-matched) + +The PreToolUse trigger uses a glob/regex on `tool_input.command` to avoid running on every Bash call. + +### Prose changes + +**`AGENTS.md` (canonical)** gets a new section after "Boundaries": + +```markdown +## Main branch health + +The `main` branch must stay green. CI failures on `main` block releases (see +`rakelib/release.rake` — the release task refuses to publish over a red `main` +without an explicit override). + +Every Claude Code session starts with a CI-status block for the latest `main` +push commit, and the same block is re-emitted before `gh pr create` or pushing +to `main`. Read it. + +If `main` is red: + +1. Decide whether the failure is related to your work. If yes, your job is to + fix it (or revert) before adding new commits. +2. If unrelated, decide whether your work is safe to merge on top. PRs that + add risk on top of a known-broken `main` should usually wait. +3. If you're the one merging a PR, check `main` post-merge within 30 minutes + (see `.claude/docs/main-health-monitoring.md`). + +**Never silently override the release CI gate.** If you `RELEASE_CI_STATUS_OVERRIDE=true`, +document in the PR / release notes why the red checks are unrelated to the release. +``` + +**`CLAUDE.md`** gets one new line under "Behavioral Defaults": + +```markdown +- Check main CI status at session start (the hook injects it) and again + before `gh pr create` or pushing to main. See `AGENTS.md` → Main branch + health. +``` + +### Tests / verification + +- Run the hook script manually after install: `.claude/hooks/main-ci-status.sh` should print a status block and exit 0. +- Disconnect from network, re-run: should print "unavailable" message and exit 0 (does not break). +- Start a new Claude Code session: status block appears in the session-start system reminder context. +- `gh pr create --dry-run`: PreToolUse hook fires (verify in transcript). + +--- + +## Issues to file (PR #2 and PR #3) + +The CI failures are out of scope for this PR but get full GitHub issues so parallel Conductor worktrees can pick them up cold. The bodies follow the "observations + leading hypotheses" pattern: definitive facts come first, then labeled hypotheses that future investigators can keep or discard. + +### Issue for PR #2 — tanstackRouter `loadRouteChunk` double-call under StrictMode + +**Title**: `fix(tanstack-router): loadRouteChunk called 2× under StrictMode hydration replay` + +**Body (sketch)**: + +```markdown +## Observed + +`packages/react-on-rails-pro/tests/tanstackRouter.test.ts:1669` fails on every +push to `main` as of #3213: + + ● tanstack-router integration (Pro) › + does not double-call loadRouteChunk when StrictMode replays hydration effects + + expect(jest.fn()).toHaveBeenCalledTimes(expected) + Expected: 1 + Received: 2 + +Implementation under test: `packages/react-on-rails-pro/src/tanstack-router/clientHydrate.ts`, +specifically the `routerRef.current === null` block (line ~186) which calls +`preloadMatchedRouteChunks` → `loadRouteChunk`. + +Introduced by [PR #3213](https://github.com/shakacode/react_on_rails/pull/3213) +("remove Suspense gate around RouterProvider during hydration"). The failing +test was added in that PR as a regression guard. + +## Repro + + cd packages/react-on-rails-pro + pnpm test -- tests/tanstackRouter.test.ts \ + -t "does not double-call loadRouteChunk when StrictMode" + +## Leading hypotheses (not verified) + +1. **StrictMode mount/unmount/mount cycle resets `routerRef`.** In React 18 dev + StrictMode, components are mounted, unmounted, and re-mounted. `useRef` + instances are per-component-instance, so the second mount sees + `routerRef.current === null` and re-enters the init block — calling + `preloadMatchedRouteChunks` a second time. If correct, either: + - the test's expectation is wrong (should be 2) and the comment at + `tanstackRouter.test.ts:1665-1668` describes a guard that doesn't actually + work across remount; or + - the implementation needs a guard that survives unmount (module-level + `WeakSet` keyed on the router instance? Or a guard on `options.createRouter` + identity?). + +2. **A render-phase double invocation within a single mount.** Less likely given + the `routerRef.current === null` guard, but possible if React discards a + render mid-init and the second render finds the ref still null because the + first render's assignment was discarded. + +Inspect the `routerRef.current = router` assignment (clientHydrate.ts:300) and +the surrounding "Safety invariant" comment (lines 192-196) — the comment +explicitly addresses discarded renders but may not address StrictMode remount. + +## Acceptance criteria + +- `tests/tanstackRouter.test.ts` test "does not double-call loadRouteChunk + when StrictMode replays hydration effects" passes. +- Other tanstackRouter tests still pass. +- Either the implementation enforces "one preload per (router instance, + hydration payload)" across mount cycles, _or_ the test expectation is + corrected with a code comment explaining why double-call is acceptable. +- CHANGELOG.md gets an entry (Fixed, Pro) if user-visible. + +## Out of scope + +- Broader hydration refactors. +- Changes to non-tanstackRouter integration paths. +``` + +### Issue for PR #3 — Benchmark Workflow port + summary failures + +**Title**: `fix(ci): Benchmark Workflow fails — bench.rb doesn't produce summary, port 3001 never frees` + +**Body (sketch)**: + +```markdown +## Observed + +`Benchmark Workflow` fails on every push to `main` as of at least 8 commits ago. + +Two error messages at the end of the failing run: + + ❌ ERROR: benchmark summary file not found (from "Validate Core benchmark results") + ❌ ERROR: Port 3001 is still in use after 10 seconds (from "Stop Core production server") + +K6 output earlier in the run shows `checks_failed: 75%` across many phases, +suggesting the production server isn't responding cleanly under load. + +Workflow file: `.github/workflows/benchmark.yml` (steps "Execute Core benchmark +suite" through "Stop Core production server"). + +## What's already been tried + +[PR #3403](https://github.com/shakacode/react_on_rails/pull/3403) +("Better specify `PORT` in `prod` scripts to fix the benchmark workflow") set +`PORT=3001` in both `react_on_rails/spec/dummy/bin/prod` and +`react_on_rails_pro/spec/dummy/bin/prod` so Foreman doesn't default to +`PORT=5000`. The workflow still fails after that change. + +Commits where the workflow has failed include 3103496d (post-#3403), +33bacf90, 4758078165, c611361e, fb174e5d, fcc817d2. (`gh run list --branch main +--event push --json conclusion,headSha,workflowName --jq '...'`.) + +## Repro + +Trigger the workflow via `gh workflow run benchmark.yml` against `main`, or +push any non-docs commit to `main`. + +For a local-ish repro: + + cd react_on_rails/spec/dummy + bin/prod-assets + PORT=3001 bin/prod & + # wait for http://localhost:3001 + bundle exec ruby benchmarks/bench.rb + ls bench_results/ + +## Leading hypotheses (not verified) + +1. **The Core server starts but doesn't survive the benchmark load.** The 75% + k6 check-failure rate suggests the server is failing to respond to most + requests. If it crashes, `bench.rb` may not write `bench_results/summary.txt` + on the way out. Investigate whether `bench.rb` writes the summary + incrementally or only at end, and whether the server logs show OOM / + worker crash patterns. + +2. **`pkill -9 -f "ruby|node|foreman|overmind|puma"` is too broad / too late.** + If the benchmark step exited via `set -e` because of the upstream failure, + the "Stop" step might be killing processes that have already crashed in + ways that leave port 3001 in `TIME_WAIT`. Worth checking what `lsof +-i:3001` actually shows. + +3. **Concurrent server processes.** The Core server starts, runs benchmarks, + then we expect to stop it before starting Pro on the same port. If Foreman + spawns child processes that survive `pkill`, the second `Start Pro +production server` step would find 3001 occupied even after the kill loop. + (#3403 may have helped or may have made this worse — the Pro server now + _also_ binds 3001 explicitly via `bin/prod`.) + +## Acceptance criteria + +- A push to `main` (non-docs) sees the Benchmark Workflow complete with + `conclusion: success` (or, if a regression is detected, with the warn-only + path on main per the existing step 7c — not a hard failure on operational + errors). +- `bench_results/summary.txt` is produced when `RUN_CORE` is true. +- Port 3001 reliably frees within the existing 10-second window between Core + and Pro server steps. + +## Out of scope + +- Tuning bencher thresholds (the t-test config is fine). +- Migrating off Foreman / overmind. +- Adding new benchmark scenarios. +``` + +--- + +## Risks and mitigation + +| Risk | Mitigation | +| ------------------------------------------------------------ | -------------------------------------------------------------------------------------------- | +| Hook adds latency to every session start | 5-min cache; gh CLI is fast (<1s); fail-open if slow | +| Release gate blocks a legitimate hotfix | Override env var is documented in the error message itself | +| GitHub branch protection API call requires extra token scope | Falls back to "treat all checks as required" if query fails | +| The hook script bug breaks every session | Fail-open: any script error → prints "unavailable" and exits 0 | +| `gh` not authenticated on a contributor's machine | Hook prints unauthenticated message; release gate aborts with clear error (existing pattern) | + +## Open questions + +None remaining at design time — the user has confirmed: + +- Required-checks (A) for rc; every check (C) for stable releases +- Override via env var + 4th positional arg, paralleling existing version-policy override +- In-progress checks block (no polling) +- Always check `origin/main` HEAD, regardless of where the release runs from +- Hook-driven signal _and_ prose docs +- Three separate PRs (this is PR #1; #2 and #3 file as issues for parallel worktrees) +- Issue format: observations first, then labeled hypotheses + +## Next step + +Invoke `writing-plans` skill to produce an executable implementation plan for this PR. From 5417c9356839caddf02bede76a24612b91e20fd4 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 11:33:19 -1000 Subject: [PATCH 02/13] feat(release): refuse to release over a red main, with override `rake release` now checks origin/main HEAD's CI status before any destructive operation: - Stable releases require every check run on the commit to have conclusion in {success, skipped, neutral}. - Pre-releases require only GitHub-branch-protection-required checks to pass; non-required checks are advisory. - In-progress checks block (no polling). - Override via env var RELEASE_CI_STATUS_OVERRIDE=true or the new 4th positional argument, mirroring the existing version-policy override. Failure messages name the failing checks and link to them, and explain exactly how to override. The check falls back safely: - If branch protection isn't queryable, treat all checks as required. - If gh fails (auth/network), abort with the underlying error rather than silently overriding. Tests cover all-pass, single failure (stable/prerelease), required vs non-required checks, in-progress, zero checks visible, override env/arg, branch protection fallback, and gh failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- rakelib/release.rake | 181 +++++++++- .../release_rake_helpers_spec.rb | 313 ++++++++++++++++++ 2 files changed, 493 insertions(+), 1 deletion(-) diff --git a/rakelib/release.rake b/rakelib/release.rake index 76950c285b..5fbd69f9e6 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -289,6 +289,167 @@ def version_policy_override_enabled?(override_flag) ReactOnRails::Utils.object_to_boolean(ENV.fetch("RELEASE_VERSION_POLICY_OVERRIDE", nil)) end +def ci_status_override_enabled?(override_flag) + ReactOnRails::Utils.object_to_boolean(override_flag) || + ReactOnRails::Utils.object_to_boolean(ENV.fetch("RELEASE_CI_STATUS_OVERRIDE", nil)) +end + +# Statuses considered "incomplete" — anything not yet a finalized conclusion. +CI_INCOMPLETE_STATUSES = %w[in_progress queued waiting requested pending].freeze +# Conclusions considered acceptable. `skipped`/`neutral` are not failures (e.g. docs-only +# paths-ignore skips, or workflows that intentionally short-circuit). +CI_PASSING_CONCLUSIONS = %w[success skipped neutral].freeze + +def fetch_main_ci_checks(monorepo_root:) + fetch_output, fetch_status = Open3.capture2e( + "git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet" + ) + abort "❌ Unable to fetch origin/main for CI status check.\n\n#{fetch_output}" unless fetch_status.success? + + sha_output, sha_status = Open3.capture2e("git", "-C", monorepo_root, "rev-parse", "origin/main") + abort "❌ Unable to resolve origin/main HEAD.\n\n#{sha_output}" unless sha_status.success? + sha = sha_output.strip + + repo_slug = github_repo_slug(monorepo_root) + api_path = "repos/#{repo_slug}/commits/#{sha}/check-runs" + + # `--paginate --jq '.check_runs[]'` flattens paginated responses into JSONL. + # Each non-empty line is one check_run object. + output, status = Open3.capture2e( + "gh", "api", "--paginate", "--jq", ".check_runs[]", api_path + ) + abort "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}" unless status.success? + + check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line| + JSON.parse(line) + rescue JSON::ParserError => e + abort "❌ Failed to parse check_runs response from gh: #{e.message}\n\nOutput:\n#{output}" + end + + { sha: sha, check_runs: check_runs } +end + +def required_check_names_for_main(monorepo_root:) + repo_slug = github_repo_slug(monorepo_root) + api_path = "repos/#{repo_slug}/branches/main/protection/required_status_checks" + output, status = Open3.capture2e("gh", "api", "--jq", ".contexts // []", api_path) + # If branch protection isn't configured, isn't queryable with current token scope, or the + # endpoint returns 404, fall through to nil so the caller treats all checks as required + # (fail-safe). + return nil unless status.success? + + begin + parsed = JSON.parse(output) + parsed.is_a?(Array) ? parsed : nil + rescue JSON::ParserError + nil + end +end + +def format_main_ci_status_violation(kind:, short_sha:, runs:) # rubocop:disable Metrics/CyclomaticComplexity + header = case kind + when :in_progress + "⏳ CI is still in progress on origin/main (commit #{short_sha})." + when :no_checks + "❌ No CI check runs visible on origin/main (commit #{short_sha}). " \ + "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})." + else + "❌ CI on origin/main is not healthy (commit #{short_sha})." + end + return header if runs.nil? || runs.empty? + + lines = runs.map do |run| + icon = kind == :in_progress ? "⏳" : "❌" + detail = kind == :in_progress ? (run["status"] || "in_progress") : (run["conclusion"] || "incomplete") + " #{icon} #{detail}: #{run['name']}\n #{run['html_url']}" + end + "#{header}\n\n#{lines.join("\n")}" +end + +def handle_main_ci_status_violation!(message:, allow_override:, dry_run:) + if dry_run + puts "⚠️ DRY RUN: #{message}" + puts "⚠️ DRY RUN: Real release would block. Use RELEASE_CI_STATUS_OVERRIDE=true to bypass." + return + end + + if allow_override + puts "⚠️ CI STATUS OVERRIDE enabled: #{message.lines.first.strip}" + return + end + + abort <<~ERROR + #{message} + + To override (use only if the failures are known-unrelated to this release): + RELEASE_CI_STATUS_OVERRIDE=true rake release[...] + # or pass override_ci_status as the 4th positional argument: + rake "release[VERSION,false,false,true]" + ERROR +end + +# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity +def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dry_run:) + puts "\nChecking CI status on origin/main..." + + data = fetch_main_ci_checks(monorepo_root: monorepo_root) + sha = data[:sha] + short_sha = sha[0, 8] + check_runs = data[:check_runs] + + if check_runs.empty? + handle_main_ci_status_violation!( + message: format_main_ci_status_violation(kind: :no_checks, short_sha: short_sha, runs: nil), + allow_override: allow_override, + dry_run: dry_run + ) + return + end + + # For prereleases, restrict the gate to GitHub-branch-protection-required checks. + # For stable releases, evaluate every check run on the commit. + 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 + end + + in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) } + if in_progress.any? + handle_main_ci_status_violation!( + message: format_main_ci_status_violation(kind: :in_progress, short_sha: short_sha, runs: in_progress), + allow_override: allow_override, + dry_run: dry_run + ) + return + end + + failed = evaluated.select do |run| + run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"]) + end + if failed.any? + handle_main_ci_status_violation!( + message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed), + allow_override: allow_override, + dry_run: dry_run + ) + return + end + + scope = required_names.nil? ? "all" : "required" + puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{scope} checks)" +end +# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity + def handle_version_policy_violation!(message:, allow_override:) if allow_override normalized = message.sub(/\A❌\s*/, "") @@ -859,12 +1020,22 @@ This will update and release: - empty (auto): use latest CHANGELOG.md version if newer, else patch bump 2nd argument: Dry run (true/false, default: false) 3rd argument: Override version policy checks (true/false, default: false) +4th argument: Override main-branch CI status check (true/false, default: false) + +Main-branch CI policy: + Before releasing, the script checks CI status on origin/main HEAD. + - Stable releases require every check run on the commit to have succeeded. + - Pre-releases require only the GitHub-branch-protection-required checks + to have succeeded. + In-progress checks block the release until they finish, or until you + explicitly override via the 4th argument or RELEASE_CI_STATUS_OVERRIDE=true. Environment variables: VERBOSE=1 # Enable verbose logging (shows all output) NPM_OTP= # Provide NPM one-time password (reused for all NPM publishes) RUBYGEMS_OTP= # Provide RubyGems one-time password (reused for both gems) RELEASE_VERSION_POLICY_OVERRIDE=true # Override release version policy checks + RELEASE_CI_STATUS_OVERRIDE=true # Override main-branch CI status check GEM_RELEASE_MAX_RETRIES= # Override max retry attempts (default: 3) Examples: @@ -877,7 +1048,7 @@ Examples: rake release[patch,true] # Dry run VERBOSE=1 rake release[patch] # Release with verbose logging NPM_OTP=123456 RUBYGEMS_OTP=789012 rake release[patch] # Skip OTP prompts") -task :release, %i[version dry_run override_version_policy] do |_t, args| +task :release, %i[version dry_run override_version_policy override_ci_status] do |_t, args| monorepo_root = current_monorepo_root args_hash = args.to_hash @@ -885,6 +1056,7 @@ task :release, %i[version dry_run override_version_policy] do |_t, args| is_dry_run = ReactOnRails::Utils.object_to_boolean(args_hash[:dry_run]) is_verbose = ENV["VERBOSE"] == "1" allow_version_policy_override = version_policy_override_enabled?(args_hash[:override_version_policy]) + allow_ci_status_override = ci_status_override_enabled?(args_hash[:override_ci_status]) npm_otp = ENV.fetch("NPM_OTP", nil) rubygems_otp = ENV.fetch("RUBYGEMS_OTP", nil) @@ -933,6 +1105,13 @@ task :release, %i[version dry_run override_version_policy] do |_t, args| ERROR end + validate_main_ci_status!( + monorepo_root: release_root, + is_prerelease: is_prerelease, + allow_override: allow_ci_status_override, + dry_run: is_dry_run + ) + validate_release_version_policy!( monorepo_root: release_root, target_gem_version: resolved_target_gem_version, diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index d986ce25cf..d5515579a7 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -475,4 +475,317 @@ end.to raise_error(SystemExit, /Version bump mismatch/) end end + + describe "#ci_status_override_enabled?" do + it "returns true when the override flag is truthy" do + expect(ci_status_override_enabled?("true")).to be true + expect(ci_status_override_enabled?(true)).to be true + end + + it "returns true when the env var is truthy" do + allow(ENV).to receive(:fetch).with("RELEASE_CI_STATUS_OVERRIDE", nil).and_return("true") + expect(ci_status_override_enabled?(nil)).to be true + end + + it "returns false when both are falsy" do + allow(ENV).to receive(:fetch).with("RELEASE_CI_STATUS_OVERRIDE", nil).and_return(nil) + expect(ci_status_override_enabled?(nil)).to be false + expect(ci_status_override_enabled?("false")).to be false + end + end + + describe "#validate_main_ci_status!" do + let(:monorepo_root) { "/tmp/repo" } + let(:sha) { "abc1234def5678abcdef" } + let(:short_sha) { "abc1234d" } + + def passing_run(name) + { + "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") + { + "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) + { + "name" => name, + "status" => "in_progress", + "conclusion" => nil, + "html_url" => "https://github.com/shakacode/react_on_rails/runs/#{name.gsub(/\W/, '_')}" + } + end + + context "when all check runs pass" do + it "logs success and returns without raising" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint"), passing_run("Test")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to output(/Main CI is healthy on #{short_sha} \(2 all checks\)/).to_stdout + end + end + + context "when a check has failed on a stable release" do + it "aborts with the failing check name and link" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("JS unit tests")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*JS unit tests}m) + end + end + + context "when a non-required check fails on a prerelease" do + it "passes because only required checks gate prereleases" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("Benchmark Workflow")]) + allow(self).to receive(:required_check_names_for_main) + .with(monorepo_root: monorepo_root).and_return(["Lint"]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: true, + allow_override: false, + dry_run: false + ) + end.to output(/Main CI is healthy on #{short_sha} \(1 required checks\)/).to_stdout + end + end + + context "when a required check fails on a prerelease" do + it "aborts because the required check gates prereleases" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [failing_run("Lint"), passing_run("Benchmark Workflow")]) + allow(self).to receive(:required_check_names_for_main) + .with(monorepo_root: monorepo_root).and_return(["Lint"]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: true, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Lint}m) + end + end + + context "when a check is still in progress" do + it "aborts with the in-progress message and link" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint"), in_progress_run("Slow test")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, /CI is still in progress.*Slow test/m) + end + end + + context "when there are zero check runs visible" do + it "aborts with a 'no CI data' message" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root).and_return(sha: sha, check_runs: []) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{No CI check runs visible on origin/main}) + end + end + + context "when override is set on a failing main" do + it "warns and returns instead of aborting" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [failing_run("Lint")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: true, + dry_run: false + ) + end.to output(/CI STATUS OVERRIDE enabled/).to_stdout + end + end + + context "when running in dry-run mode on a failing main" do + it "warns and returns instead of aborting" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [failing_run("Lint")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: true + ) + end.to output(%r{DRY RUN.*CI on origin/main is not healthy}m).to_stdout + end + end + + context "when branch protection is not queryable on a prerelease" do + it "falls back to evaluating all checks (fail-safe)" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("Optional Check")]) + allow(self).to receive(:required_check_names_for_main) + .with(monorepo_root: monorepo_root).and_return(nil) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: true, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Optional Check}m) + end + end + + context "when no check runs match the required names on a prerelease" do + it "aborts with a 'no required check runs' message" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root) + .and_return(sha: sha, check_runs: [passing_run("Lint")]) + allow(self).to receive(:required_check_names_for_main) + .with(monorepo_root: monorepo_root).and_return(["DoesNotExist"]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: true, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, /No required CI check runs found.*DoesNotExist/m) + end + end + end + + describe "#fetch_main_ci_checks" do + let(:monorepo_root) { "/tmp/repo" } + let(:success_status) { instance_double(Process::Status, success?: true) } + let(:failure_status) { instance_double(Process::Status, success?: false) } + + it "aborts if `git fetch origin main` fails" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["fetch failed: network down", failure_status]) + + expect do + fetch_main_ci_checks(monorepo_root: monorepo_root) + end.to raise_error(SystemExit, %r{Unable to fetch origin/main}) + end + + it "aborts if `gh api check-runs` fails (no silent override)" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["", success_status]) + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "rev-parse", "origin/main") + .and_return(["abc1234\n", success_status]) + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--paginate", "--jq", ".check_runs[]", + "repos/shakacode/react_on_rails/commits/abc1234/check-runs") + .and_return(["HTTP 401: unauthorized", failure_status]) + + expect do + fetch_main_ci_checks(monorepo_root: monorepo_root) + end.to raise_error(SystemExit, /Unable to query GitHub Checks API.*HTTP 401/m) + end + + it "parses paginated JSONL check_runs into an array of hashes" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["", success_status]) + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "rev-parse", "origin/main") + .and_return(["abc1234def\n", success_status]) + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + jsonl = [ + { "name" => "Lint", "status" => "completed", "conclusion" => "success" }, + { "name" => "Test", "status" => "completed", "conclusion" => "failure" } + ].map(&:to_json).join("\n") + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--paginate", "--jq", ".check_runs[]", + "repos/shakacode/react_on_rails/commits/abc1234def/check-runs") + .and_return([jsonl, success_status]) + + result = fetch_main_ci_checks(monorepo_root: monorepo_root) + expect(result[:sha]).to eq("abc1234def") + expect(result[:check_runs].length).to eq(2) + expect(result[:check_runs].first["name"]).to eq("Lint") + end + end + + describe "#required_check_names_for_main" do + let(:monorepo_root) { "/tmp/repo" } + let(:success_status) { instance_double(Process::Status, success?: true) } + let(:failure_status) { instance_double(Process::Status, success?: false) } + + before do + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + end + + it "returns the array of required context names when branch protection is configured" do + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--jq", ".contexts // []", + "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") + .and_return([%w[Lint Test].to_json, success_status]) + + expect(required_check_names_for_main(monorepo_root: monorepo_root)).to eq(%w[Lint Test]) + end + + it "returns nil when the branch protection endpoint returns an error" do + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--jq", ".contexts // []", + "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") + .and_return(["HTTP 404: Branch not protected", failure_status]) + + expect(required_check_names_for_main(monorepo_root: monorepo_root)).to be_nil + end + end end From 25c02696dbd992d5e36f67e2c5e30b4eb454690b Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 11:33:40 -1000 Subject: [PATCH 03/13] feat(claude): surface main CI status at session start and pre-push Adds two hook scripts under .claude/hooks/: - main-ci-status.sh: fetches the latest origin/main push commit's check runs via `gh api` and prints a compact status block. 5-minute cache in .claude/.main-ci-status.cache. Fail-open: any tooling failure (no gh, no auth, no network) prints a one-line "unavailable" message and exits 0. - main-ci-status-on-push.sh: PreToolUse:Bash wrapper that runs the status script only when the next Bash command matches `gh pr create` or `git push origin main|HEAD`. Silently exits for any other Bash call so we don't spam the agent context. Wired into .claude/settings.json on SessionStart (always) and PreToolUse:Bash (filtered by the wrapper). AGENTS.md gets a new "Main branch health" section describing the release gate, the hook signal, the decision framework when main is red, and the rule that the release CI override must be documented in the PR / release notes. CLAUDE.md gets one line under "Behavioral Defaults" pointing at the new section. .claude/.main-ci-status.cache is gitignored (per-machine state). Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status-on-push.sh | 33 +++++++ .claude/hooks/main-ci-status.sh | 126 ++++++++++++++++++++++++ .claude/settings.json | 21 ++++ .gitignore | 1 + AGENTS.md | 25 +++++ CLAUDE.md | 1 + 6 files changed, 207 insertions(+) create mode 100755 .claude/hooks/main-ci-status-on-push.sh create mode 100755 .claude/hooks/main-ci-status.sh diff --git a/.claude/hooks/main-ci-status-on-push.sh b/.claude/hooks/main-ci-status-on-push.sh new file mode 100755 index 0000000000..5f4c798dcd --- /dev/null +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# PreToolUse:Bash wrapper that surfaces main CI status only when the next Bash +# command is about to push to main or open a PR. Fail-open: any unexpected +# input or tooling error exits 0 without output so we never block a tool call. + +set -u + +# Claude Code passes tool input as JSON on stdin. Extract the command string. +# If jq is missing or the input is malformed, exit silently. +command -v jq >/dev/null 2>&1 || exit 0 +input=$(cat 2>/dev/null) || exit 0 +cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) || exit 0 +[ -n "${cmd}" ] || exit 0 + +# Match: +# - `gh pr create` (any args) +# - `git push` to main / HEAD (with explicit origin main, or bare `git push` +# while on main — we don't try to detect the current branch here, just +# match the explicit-target forms). +matched=0 +case "${cmd}" in + *gh\ pr\ create*) matched=1 ;; + *git\ push*origin\ main*) matched=1 ;; + *git\ push*origin\ HEAD*) matched=1 ;; + *git\ push\ --force*main*) matched=1 ;; +esac + +[ "${matched}" -eq 1 ] || exit 0 + +# Run the underlying status script. Its output goes to stdout, which Claude +# Code injects into context. +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec "${script_dir}/main-ci-status.sh" diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh new file mode 100755 index 0000000000..d6139ee726 --- /dev/null +++ b/.claude/hooks/main-ci-status.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# Prints a compact CI-status block for origin/main's latest push commit. +# +# Designed to be wired into Claude Code's SessionStart and PreToolUse hooks +# (see .claude/settings.json) so the agent always sees whether main is green +# before opening a PR or pushing. +# +# Fail-open by design: any tooling failure (gh missing, unauthenticated, no +# network) prints a one-line "unavailable" message and exits 0. We never +# block a session because the status check failed. +# +# Caches output for 5 minutes in .claude/.main-ci-status.cache to avoid +# pounding the GitHub API across rapid session starts. + +set -u # No `set -e` — we want to handle errors ourselves to stay fail-open. + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" +CACHE_FILE="${REPO_ROOT}/.claude/.main-ci-status.cache" +CACHE_TTL_SECONDS=300 + +# If the cache is fresh, print it and exit. Per-platform stat invocation differs. +if [ -f "${CACHE_FILE}" ]; then + if [ "$(uname)" = "Darwin" ]; then + cache_mtime=$(stat -f %m "${CACHE_FILE}" 2>/dev/null || echo 0) + else + cache_mtime=$(stat -c %Y "${CACHE_FILE}" 2>/dev/null || echo 0) + fi + now=$(date +%s) + age=$((now - cache_mtime)) + if [ "${age}" -lt "${CACHE_TTL_SECONDS}" ]; then + cat "${CACHE_FILE}" + exit 0 + fi +fi + +# Helper: print an "unavailable" message and exit 0 without writing the cache. +fail_open() { + echo "Main CI status unavailable: $1" + exit 0 +} + +command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed" +gh auth status >/dev/null 2>&1 || fail_open "gh CLI not authenticated (run \`gh auth login\`)" + +# Pull the latest push run's status check rollup. `--limit 1 --event push` +# isolates main pushes from PR/comment-triggered runs that we don't care about. +runs_json=$(gh run list \ + --branch main \ + --limit 1 \ + --event push \ + --json conclusion,headSha,workflowName,url,status,createdAt \ + 2>/dev/null) || fail_open "gh run list failed" + +[ -n "${runs_json}" ] || fail_open "no main push runs visible" + +# Resolve the head SHA from the most recent push run, then pull every check +# run on that commit. We use the Checks API (not `gh run list`) because a +# single push commit triggers multiple workflows and we want them aggregated. +head_sha=$(echo "${runs_json}" | jq -r '.[0].headSha // empty' 2>/dev/null) || fail_open "jq parse failed" +[ -n "${head_sha}" ] || fail_open "no headSha on most recent push run" + +repo_slug=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null) || fail_open "gh repo view failed" + +# Use `--paginate` with `--jq '.check_runs[]'` to emit JSONL (one check_run per +# line). A separate `jq -s` then slurps the JSONL back into a single array. +# This avoids the gotcha where `gh --paginate` with `--jq '[...]'` produces one +# array per page (concatenated), breaking single-array aggregation. +checks_jsonl=$(gh api \ + --paginate \ + "repos/${repo_slug}/commits/${head_sha}/check-runs" \ + --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" + +# Aggregate counts with jq. `success`, `skipped`, `neutral` are all "passing". +# Anything completed with another conclusion is a failure. Anything not yet +# completed is in_progress. +summary=$(echo "${checks_json}" | jq -r ' + . as $all + | { + total: length, + passed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral")))] | length, + failed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral") | not))], + in_progress: [.[] | select(.status != "completed")] + } + | "TOTAL=\(.total)", + "PASSED=\(.passed)", + "FAILED_COUNT=\(.failed | length)", + "IN_PROGRESS_COUNT=\(.in_progress | length)", + (.failed[] | "FAILED_LINE=" + .name + " — " + (.conclusion // "incomplete") + " — " + (.html_url // "")), + (.in_progress[] | "INPROGRESS_LINE=" + .name + " — " + (.status // "in_progress") + " — " + (.html_url // "")) +') || fail_open "jq summary failed" + +# Pull short SHA + workflow run URL for the header. Both fields come from runs_json. +short_sha="${head_sha:0:8}" +created_at=$(echo "${runs_json}" | jq -r '.[0].createdAt // ""') + +# Build the output as a single string, then write it to cache + stdout. +{ + printf 'Main CI status (origin/main %s, pushed at %s):\n' "${short_sha}" "${created_at}" + 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) + + printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \ + "${total}" "${passed}" "${failed_count}" "${in_progress_count}" + + if [ "${failed_count}" -gt 0 ] 2>/dev/null; then + echo " Failures:" + echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /' + fi + + if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + echo " In progress:" + echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /' + fi + + if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks" + fi +} | tee "${CACHE_FILE}" + +exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index 5b3ffee9c4..3db0cfc5e6 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,5 +1,26 @@ { "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": ".claude/hooks/main-ci-status.sh" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": ".claude/hooks/main-ci-status-on-push.sh" + } + ] + } + ], "PostToolUse": [ { "matcher": "Edit|Write", diff --git a/.gitignore b/.gitignore index 75849aca0c..c69db45d56 100644 --- a/.gitignore +++ b/.gitignore @@ -87,6 +87,7 @@ ssr-generated # Claude Code local settings .claude/settings.local.json .claude/napkin.md +.claude/.main-ci-status.cache .Codex/napkin.md # AI assistant internal files diff --git a/AGENTS.md b/AGENTS.md index 3218b1840f..07faf8b3ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -216,6 +216,31 @@ For small, focused PRs (roughly 5 files changed or fewer and one clear purpose): - Add files to the `docs/` root — OSS docs go in `docs/oss/` subdirectories (`getting-started/`, `core-concepts/`, `building-features/`, `configuration/`, `api-reference/`, `deployment/`, `migrating/`, `upgrading/`, `misc/`); Pro docs go in `docs/pro/` - Force push to `main` or `master` +## Main branch health + +The `main` branch must stay green. CI failures on `main` block releases: +`rake release` refuses to publish over a red `main` unless you explicitly +override (via `RELEASE_CI_STATUS_OVERRIDE=true` or the 4th positional arg). +Stable releases require every check to pass; pre-releases require only the +GitHub-branch-protection-required checks. + +Claude Code sessions get `main`'s CI status injected at session start (and +again before `gh pr create` / pushing to `main`) via +`.claude/hooks/main-ci-status.sh`. Read it. + +If `main` is red: + +1. **Decide whether the failure is related to your work.** If yes, your job + is to fix it (or revert) before adding new commits on top. +2. **If unrelated, decide whether your work is safe to merge on top.** PRs + that add risk on top of a known-broken `main` should usually wait. +3. **If you're the one merging a PR**, check `main` post-merge within 30 + minutes (see `.claude/docs/main-health-monitoring.md`). + +**Never silently override the release CI gate.** If you set +`RELEASE_CI_STATUS_OVERRIDE=true`, document in the PR / release notes why +the red checks are unrelated to the release. + ## Key Concept: File Suffixes vs. RSC Directive React on Rails has two **independent** systems that both use "client" and "server" terminology. Do not confuse them. diff --git a/CLAUDE.md b/CLAUDE.md index 8b5d783bee..ce39225223 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,6 +16,7 @@ If this file conflicts with `AGENTS.md`, follow `AGENTS.md`. ## Behavioral Defaults - When confident in your changes, **commit and push without asking for permission**. Always monitor CI after pushing. +- Check `main` CI status at session start (injected by `.claude/hooks/main-ci-status.sh`) and again before `gh pr create` or pushing to `main`. See `AGENTS.md` → "Main branch health" for the decision framework when `main` is red. ## Git Safety From a0cf43a535d5012f69db9715105f16cc026ccee1 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 15:14:40 -1000 Subject: [PATCH 04/13] fix(release): address PR #3407 review feedback (12 items) Applies must-fix correctness improvements plus optional polish surfaced by cursor, codex, greptile, claude, and coderabbit bot reviews. None of these change the high-level shape of the release gate or session hooks documented in the design spec; they harden the edges. Release gate (rakelib/release.rake): - Combine .contexts + .checks[].context in the branch-protection query so newer `checks`-based protection rules don't return an empty array and trip the :no_required_checks path (cursor, greptile). - Treat an empty result from that query as nil so the caller falls back to "evaluate every check" rather than aborting (fail-safe). - Dedupe check_runs by name, keeping the highest id (= latest attempt), so an old failed run that was re-run and passed no longer blocks the release (codex P1, greptile). - Plumb dry_run/allow_override into fetch_main_ci_checks so dry-run on a machine without gh/network warns instead of aborting (codex P2). - Use capture_gh_output for all `gh api` calls so a missing gh binary surfaces the friendly install hint instead of crashing with Errno::ENOENT (coderabbit). - Check failed runs before in-progress runs. If both are present, the operator now sees the real blocker immediately instead of waiting for the in-progress run to finish before discovering the failure (claude). - Fix grammar of the success message ("(2 checks)" / "(1 required check)") and switch override instructions to `bundle exec rake` (claude, coderabbit). Session hooks (.claude/hooks/): - main-ci-status.sh: resolve origin/main HEAD via `git ls-remote origin main` instead of the latest push-workflow run's headSha so the SHA matches the release gate and doesn't lag right after a push or for paths-ignored pushes (cursor). - main-ci-status.sh: when no check runs are visible for the commit, print "no check runs visible yet" instead of `Total: 0 | ... | 0`, which an agent could misread as "all green" (cursor). - main-ci-status.sh: write the cache via mktemp + mv so a concurrent reader never sees a half-written file (claude). - main-ci-status-on-push.sh: replace substring case-globs with anchored bash regexes so `git push origin main-feature` / `maintenance` / `main-ci-fix` no longer match. Drop the redundant `--force` case (greptile P2, claude). Tests: - Add specs for the dedupe path, the failed-before-in-progress order, the dry-run fetch-failure path, the empty-array branch-protection fallback, and the missing-gh Errno::ENOENT path. - Update existing assertions for the new success-message grammar. Docs: - docs/superpowers/specs/2026-05-25-...: mark Implemented, add language identifiers to bare fenced code blocks (MD040), update override instructions to use `bundle exec rake` (coderabbit). --- .claude/hooks/main-ci-status-on-push.sh | 18 +-- .claude/hooks/main-ci-status.sh | 116 +++++++++------- ...2026-05-25-main-ci-release-guard-design.md | 14 +- rakelib/release.rake | 95 +++++++++---- .../release_rake_helpers_spec.rb | 126 ++++++++++++++++-- 5 files changed, 267 insertions(+), 102 deletions(-) diff --git a/.claude/hooks/main-ci-status-on-push.sh b/.claude/hooks/main-ci-status-on-push.sh index 5f4c798dcd..151785decc 100755 --- a/.claude/hooks/main-ci-status-on-push.sh +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -14,16 +14,16 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) # Match: # - `gh pr create` (any args) -# - `git push` to main / HEAD (with explicit origin main, or bare `git push` -# while on main — we don't try to detect the current branch here, just -# match the explicit-target forms). +# - `git push ... origin main` or `git push ... origin HEAD` — must be +# followed by whitespace or end-of-string. Without that anchor, a glob +# substring match would also fire on `git push origin main-feature`, +# `maintenance`, etc. matched=0 -case "${cmd}" in - *gh\ pr\ create*) matched=1 ;; - *git\ push*origin\ main*) matched=1 ;; - *git\ push*origin\ HEAD*) matched=1 ;; - *git\ push\ --force*main*) matched=1 ;; -esac +if [[ "${cmd}" =~ (^|[[:space:]])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$) ]]; then + matched=1 +elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]+[^[:space:]]+)*[[:space:]]+origin[[:space:]]+(main|HEAD)([[:space:]]|$) ]]; then + matched=1 +fi [ "${matched}" -eq 1 ] || exit 0 diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index d6139ee726..e75381c3b5 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Prints a compact CI-status block for origin/main's latest push commit. +# Prints a compact CI-status block for origin/main's current HEAD commit. # # Designed to be wired into Claude Code's SessionStart and PreToolUse hooks # (see .claude/settings.json) so the agent always sees whether main is green @@ -39,32 +39,39 @@ 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 +# by a concurrent session-start if the script were interrupted mid-write. +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}" +} + command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed" gh auth status >/dev/null 2>&1 || fail_open "gh CLI not authenticated (run \`gh auth login\`)" -# Pull the latest push run's status check rollup. `--limit 1 --event push` -# isolates main pushes from PR/comment-triggered runs that we don't care about. -runs_json=$(gh run list \ - --branch main \ - --limit 1 \ - --event push \ - --json conclusion,headSha,workflowName,url,status,createdAt \ - 2>/dev/null) || fail_open "gh run list failed" - -[ -n "${runs_json}" ] || fail_open "no main push runs visible" - -# Resolve the head SHA from the most recent push run, then pull every check -# run on that commit. We use the Checks API (not `gh run list`) because a -# single push commit triggers multiple workflows and we want them aggregated. -head_sha=$(echo "${runs_json}" | jq -r '.[0].headSha // empty' 2>/dev/null) || fail_open "jq parse failed" -[ -n "${head_sha}" ] || fail_open "no headSha on most recent push run" - repo_slug=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null) || fail_open "gh repo view failed" -# Use `--paginate` with `--jq '.check_runs[]'` to emit JSONL (one check_run per -# line). A separate `jq -s` then slurps the JSONL back into a single array. -# This avoids the gotcha where `gh --paginate` with `--jq '[...]'` produces one -# array per page (concatenated), breaking single-array aggregation. +# Resolve the actual `origin/main` HEAD SHA. We use `git ls-remote` (not +# `gh run list`) so the SHA reflects the current ref tip, not the latest +# push-workflow run — which can lag right after a push (or never appear at +# all for docs-only pushes when paths-ignore filters out every workflow). +# Matching the release gate's SHA semantics (`git rev-parse origin/main`) +# keeps session-time and release-time observations consistent. +head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') \ + || fail_open "git ls-remote origin main failed" +[ -n "${head_sha}" ] || fail_open "could not resolve origin/main HEAD" + +# Pull every check run on the commit. We use the Checks API because a single +# push commit triggers multiple workflows and we want them aggregated. +# +# `--paginate` with `--jq '.check_runs[]'` emits JSONL (one check_run per line). +# A separate `jq -s` slurps the JSONL back into a single array. This avoids +# the gotcha where `gh --paginate` with `--jq '[...]'` produces one array per +# page (concatenated), breaking single-array aggregation. checks_jsonl=$(gh api \ --paginate \ "repos/${repo_slug}/commits/${head_sha}/check-runs" \ @@ -93,34 +100,45 @@ summary=$(echo "${checks_json}" | jq -r ' (.in_progress[] | "INPROGRESS_LINE=" + .name + " — " + (.status // "in_progress") + " — " + (.html_url // "")) ') || fail_open "jq summary failed" -# Pull short SHA + workflow run URL for the header. Both fields come from runs_json. short_sha="${head_sha:0:8}" -created_at=$(echo "${runs_json}" | jq -r '.[0].createdAt // ""') - -# Build the output as a single string, then write it to cache + stdout. -{ - printf 'Main CI status (origin/main %s, pushed at %s):\n' "${short_sha}" "${created_at}" - 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) - - printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \ - "${total}" "${passed}" "${failed_count}" "${in_progress_count}" - - if [ "${failed_count}" -gt 0 ] 2>/dev/null; then - echo " Failures:" - echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /' - fi - - if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then - echo " In progress:" - echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /' - fi +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) + +# Build the output as a single string, then atomically swap it into the cache +# so a concurrent reader never sees a half-written file. +if [ "${total:-0}" = "0" ]; then + # No check runs visible for this commit. The Checks API may simply not have + # registered any workflows yet (right after a push), or all workflows were + # filtered out by paths-ignore. Either way, the agent should NOT read this + # as "all green" — say so explicitly. The release gate treats the same case + # as a blocking violation; aligning the wording here keeps the two signals + # honest. + output=$(printf 'Main CI status (origin/main %s): no check runs visible yet.\n CI may not have started for this commit, or the Checks API is unavailable.\n See: https://github.com/%s/commit/%s/checks\n' \ + "${short_sha}" "${repo_slug}" "${head_sha}") +else + output=$( + printf 'Main CI status (origin/main %s):\n' "${short_sha}" + printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \ + "${total}" "${passed}" "${failed_count}" "${in_progress_count}" + + if [ "${failed_count}" -gt 0 ] 2>/dev/null; then + echo " Failures:" + echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /' + fi + + if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + echo " In progress:" + echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /' + fi + + if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks" + fi + ) +fi - if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then - echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks" - fi -} | tee "${CACHE_FILE}" +write_cache_atomic "${output}" || fail_open "cache write failed" exit 0 diff --git a/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md index 137e36cde4..834623b5c4 100644 --- a/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md +++ b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md @@ -1,6 +1,6 @@ # Main CI release guard + continuous monitoring -**Status**: Design — awaiting user review +**Status**: Implemented (see PR #3407) **Date**: 2026-05-25 **Owner**: Justin Gordon (with Claude Code) @@ -51,11 +51,11 @@ Note: `run_release_preflight_checks!` runs _before_ `with_release_checkout` (and The `:release` task gets a new 4th positional argument and a paired env var, paralleling the existing `override_version_policy`: -``` +```ruby task :release, %i[version dry_run override_version_policy override_ci_status] ``` -``` +```bash RELEASE_CI_STATUS_OVERRIDE=true # bypass the CI status check ``` @@ -77,7 +77,7 @@ The target commit is **always `origin/main` HEAD**, regardless of where the rele When blocked, the error names the failing checks, links to them, and tells the operator exactly how to proceed: -``` +```text ❌ CI on main is not healthy — refusing to release. Commit: 3103496d @@ -87,9 +87,9 @@ Commit: 3103496d https://github.com/shakacode/react_on_rails/actions/runs/26404417325 To override (use only if the failures are known-unrelated to this release): - RELEASE_CI_STATUS_OVERRIDE=true rake release[...] + RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake release[...] # or - rake "release[16.2.0,false,false,true]" + bundle exec rake "release[16.2.0,false,false,true]" ``` In-progress checks get a separate message ("CI in progress — wait for it to finish, or override"). @@ -162,7 +162,7 @@ end A small bash script that runs `gh run list --branch main --limit 1 --event push --json conclusion,headSha,workflowName,databaseId,url,status` and emits a compact status block: -``` +```text Main CI status (3103496d, pushed 7h ago): ✅ 11 success ❌ 2 failure: diff --git a/rakelib/release.rake b/rakelib/release.rake index 5fbd69f9e6..4b275b2d79 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -300,25 +300,46 @@ CI_INCOMPLETE_STATUSES = %w[in_progress queued waiting requested pending].freeze # paths-ignore skips, or workflows that intentionally short-circuit). CI_PASSING_CONCLUSIONS = %w[success skipped neutral].freeze -def fetch_main_ci_checks(monorepo_root:) +def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false) fetch_output, fetch_status = Open3.capture2e( "git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet" ) - abort "❌ Unable to fetch origin/main for CI status check.\n\n#{fetch_output}" unless fetch_status.success? + unless fetch_status.success? + handle_main_ci_status_violation!( + message: "❌ Unable to fetch origin/main for CI status check.\n\n#{fetch_output}", + allow_override: allow_override, + dry_run: dry_run + ) + return nil + end sha_output, sha_status = Open3.capture2e("git", "-C", monorepo_root, "rev-parse", "origin/main") - abort "❌ Unable to resolve origin/main HEAD.\n\n#{sha_output}" unless sha_status.success? + unless sha_status.success? + handle_main_ci_status_violation!( + message: "❌ Unable to resolve origin/main HEAD.\n\n#{sha_output}", + allow_override: allow_override, + dry_run: dry_run + ) + return nil + end sha = sha_output.strip repo_slug = github_repo_slug(monorepo_root) api_path = "repos/#{repo_slug}/commits/#{sha}/check-runs" # `--paginate --jq '.check_runs[]'` flattens paginated responses into JSONL. - # Each non-empty line is one check_run object. - output, status = Open3.capture2e( - "gh", "api", "--paginate", "--jq", ".check_runs[]", api_path - ) - abort "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}" unless status.success? + # Each non-empty line is one check_run object. Using `capture_gh_output` so a + # missing `gh` binary aborts with a friendly install hint instead of crashing + # with Errno::ENOENT. + output, status = capture_gh_output("api", "--paginate", "--jq", ".check_runs[]", api_path) + unless status.success? + handle_main_ci_status_violation!( + message: "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}", + allow_override: allow_override, + dry_run: dry_run + ) + return nil + end check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line| JSON.parse(line) @@ -332,7 +353,12 @@ end def required_check_names_for_main(monorepo_root:) repo_slug = github_repo_slug(monorepo_root) api_path = "repos/#{repo_slug}/branches/main/protection/required_status_checks" - output, status = Open3.capture2e("gh", "api", "--jq", ".contexts // []", api_path) + # Combine the legacy `contexts` list (older protection rules) with the newer + # `checks[].context` list. Branch protection set up via the `checks` API + # leaves `contexts` as `[]`, so reading only `contexts` would yield an empty + # array and trip the `:no_required_checks` abort path even when CI is green. + jq_query = "(.contexts // []) + (.checks // [] | map(.context)) | unique" + output, status = capture_gh_output("api", "--jq", jq_query, api_path) # If branch protection isn't configured, isn't queryable with current token scope, or the # endpoint returns 404, fall through to nil so the caller treats all checks as required # (fail-safe). @@ -340,7 +366,11 @@ def required_check_names_for_main(monorepo_root:) begin parsed = JSON.parse(output) - parsed.is_a?(Array) ? parsed : nil + return nil unless parsed.is_a?(Array) + + # Empty array (no required names parseable) is treated the same as "no + # branch protection visible" — fail-safe to evaluating every check run. + parsed.empty? ? nil : parsed rescue JSON::ParserError nil end @@ -384,9 +414,9 @@ def handle_main_ci_status_violation!(message:, allow_override:, dry_run:) #{message} To override (use only if the failures are known-unrelated to this release): - RELEASE_CI_STATUS_OVERRIDE=true rake release[...] + RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake release[...] # or pass override_ci_status as the 4th positional argument: - rake "release[VERSION,false,false,true]" + bundle exec rake "release[VERSION,false,false,true]" ERROR end @@ -394,7 +424,12 @@ end def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dry_run:) puts "\nChecking CI status on origin/main..." - data = fetch_main_ci_checks(monorepo_root: monorepo_root) + data = fetch_main_ci_checks(monorepo_root: monorepo_root, allow_override: allow_override, dry_run: dry_run) + # `fetch_main_ci_checks` returns nil when it surfaced a violation through + # `handle_main_ci_status_violation!` (dry-run or override path). In that case + # the warning has already been printed and we should not continue. + return if data.nil? + sha = data[:sha] short_sha = sha[0, 8] check_runs = data[:check_runs] @@ -408,6 +443,15 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr return end + # 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 block the + # release. `id` is monotonically increasing per check run on a commit, so + # `max_by { id }` reliably selects the latest attempt. + check_runs = check_runs + .group_by { |run| run["name"] } + .map { |_name, runs| runs.max_by { |run| run["id"].to_i } } + # For prereleases, restrict the gate to GitHub-branch-protection-required checks. # For stable releases, evaluate every check run on the commit. required_names = is_prerelease ? required_check_names_for_main(monorepo_root: monorepo_root) : nil @@ -423,30 +467,35 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr return end - in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) } - if in_progress.any? + # Report failures before in-progress runs. If both are present, the operator + # needs to know about the failure right away — telling them to "wait or + # override" would just make them wait and re-run before seeing the real + # blocker. + failed = evaluated.select do |run| + run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"]) + end + if failed.any? handle_main_ci_status_violation!( - message: format_main_ci_status_violation(kind: :in_progress, short_sha: short_sha, runs: in_progress), + message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed), allow_override: allow_override, dry_run: dry_run ) return end - failed = evaluated.select do |run| - run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"]) - end - if failed.any? + in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) } + if in_progress.any? handle_main_ci_status_violation!( - message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed), + message: format_main_ci_status_violation(kind: :in_progress, short_sha: short_sha, runs: in_progress), allow_override: allow_override, dry_run: dry_run ) return end - scope = required_names.nil? ? "all" : "required" - puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{scope} checks)" + qualifier = required_names.nil? ? "" : "required " + noun = evaluated.length == 1 ? "check" : "checks" + puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{qualifier}#{noun})" end # rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index d5515579a7..0c06b0fb22 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -529,7 +529,7 @@ def in_progress_run(name) context "when all check runs pass" do it "logs success and returns without raising" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint"), passing_run("Test")]) expect do @@ -539,14 +539,14 @@ def in_progress_run(name) allow_override: false, dry_run: false ) - end.to output(/Main CI is healthy on #{short_sha} \(2 all checks\)/).to_stdout + end.to output(/Main CI is healthy on #{short_sha} \(2 checks\)/).to_stdout end end context "when a check has failed on a stable release" do it "aborts with the failing check name and link" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("JS unit tests")]) expect do @@ -563,7 +563,7 @@ def in_progress_run(name) context "when a non-required check fails on a prerelease" do it "passes because only required checks gate prereleases" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("Benchmark Workflow")]) allow(self).to receive(:required_check_names_for_main) .with(monorepo_root: monorepo_root).and_return(["Lint"]) @@ -575,14 +575,14 @@ def in_progress_run(name) allow_override: false, dry_run: false ) - end.to output(/Main CI is healthy on #{short_sha} \(1 required checks\)/).to_stdout + end.to output(/Main CI is healthy on #{short_sha} \(1 required check\)/).to_stdout end end context "when a required check fails on a prerelease" do it "aborts because the required check gates prereleases" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [failing_run("Lint"), passing_run("Benchmark Workflow")]) allow(self).to receive(:required_check_names_for_main) .with(monorepo_root: monorepo_root).and_return(["Lint"]) @@ -601,7 +601,7 @@ def in_progress_run(name) context "when a check is still in progress" do it "aborts with the in-progress message and link" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint"), in_progress_run("Slow test")]) expect do @@ -615,10 +615,47 @@ def in_progress_run(name) end end + context "when there are both failed and in-progress checks" do + it "reports the failure first so the operator does not wait on an already-broken main" 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: [failing_run("JS unit tests"), in_progress_run("Slow test")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*JS unit tests}m) + end + end + + context "when a check has been rerun and the latest attempt passes" do + it "evaluates only the latest attempt per check name and passes" do + old_failed = failing_run("Lint").merge("id" => 1) + new_passed = passing_run("Lint").merge("id" => 2) + 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: [old_failed, new_passed]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to output(/Main CI is healthy/).to_stdout + end + end + context "when there are zero check runs visible" do it "aborts with a 'no CI data' message" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root).and_return(sha: sha, check_runs: []) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) + .and_return(sha: sha, check_runs: []) expect do validate_main_ci_status!( @@ -634,7 +671,7 @@ def in_progress_run(name) context "when override is set on a failing main" do it "warns and returns instead of aborting" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: true, dry_run: false) .and_return(sha: sha, check_runs: [failing_run("Lint")]) expect do @@ -651,7 +688,7 @@ def in_progress_run(name) context "when running in dry-run mode on a failing main" do it "warns and returns instead of aborting" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: true) .and_return(sha: sha, check_runs: [failing_run("Lint")]) expect do @@ -665,10 +702,27 @@ def in_progress_run(name) end end + context "when fetch surfaces a violation via override/dry-run (returns nil)" do + it "returns without raising and without trying to inspect the data" do + allow(self).to receive(:fetch_main_ci_checks) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: true) + .and_return(nil) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: true + ) + end.not_to raise_error + end + end + context "when branch protection is not queryable on a prerelease" do it "falls back to evaluating all checks (fail-safe)" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint"), failing_run("Optional Check")]) allow(self).to receive(:required_check_names_for_main) .with(monorepo_root: monorepo_root).and_return(nil) @@ -687,7 +741,7 @@ def in_progress_run(name) context "when no check runs match the required names on a prerelease" do it "aborts with a 'no required check runs' message" do allow(self).to receive(:fetch_main_ci_checks) - .with(monorepo_root: monorepo_root) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) .and_return(sha: sha, check_runs: [passing_run("Lint")]) allow(self).to receive(:required_check_names_for_main) .with(monorepo_root: monorepo_root).and_return(["DoesNotExist"]) @@ -719,6 +773,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 in dry-run mode" 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, dry_run: true) + end.to output(%r{DRY RUN.*Unable to fetch origin/main}m).to_stdout + expect(result).to be_nil + end + it "aborts if `gh api check-runs` fails (no silent override)" do allow(Open3).to receive(:capture2e) .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") @@ -737,6 +803,24 @@ def in_progress_run(name) end.to raise_error(SystemExit, /Unable to query GitHub Checks API.*HTTP 401/m) end + it "aborts with a friendly install hint when `gh` is not installed" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["", success_status]) + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "rev-parse", "origin/main") + .and_return(["abc1234\n", success_status]) + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--paginate", "--jq", ".check_runs[]", + "repos/shakacode/react_on_rails/commits/abc1234/check-runs") + .and_raise(Errno::ENOENT) + + expect do + fetch_main_ci_checks(monorepo_root: monorepo_root) + end.to raise_error(SystemExit, /GitHub CLI .* is not installed/) + end + it "parses paginated JSONL check_runs into an array of hashes" do allow(Open3).to receive(:capture2e) .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") @@ -765,6 +849,7 @@ def in_progress_run(name) let(:monorepo_root) { "/tmp/repo" } let(:success_status) { instance_double(Process::Status, success?: true) } let(:failure_status) { instance_double(Process::Status, success?: false) } + let(:expected_jq) { "(.contexts // []) + (.checks // [] | map(.context)) | unique" } before do allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") @@ -772,7 +857,7 @@ def in_progress_run(name) it "returns the array of required context names when branch protection is configured" do allow(Open3).to receive(:capture2e) - .with("gh", "api", "--jq", ".contexts // []", + .with("gh", "api", "--jq", expected_jq, "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") .and_return([%w[Lint Test].to_json, success_status]) @@ -781,11 +866,24 @@ def in_progress_run(name) it "returns nil when the branch protection endpoint returns an error" do allow(Open3).to receive(:capture2e) - .with("gh", "api", "--jq", ".contexts // []", + .with("gh", "api", "--jq", expected_jq, "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") .and_return(["HTTP 404: Branch not protected", failure_status]) expect(required_check_names_for_main(monorepo_root: monorepo_root)).to be_nil end + + it "returns nil when the protection response yields an empty array (fail-safe)" do + # Newer branch protection rules can return `contexts: []` with the real required + # names in `checks`. The combined jq query above returns `[]` only when neither + # field has names. Treat that as "no protection visible" and let the caller + # evaluate every check run rather than abort with :no_required_checks. + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--jq", expected_jq, + "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") + .and_return(["[]", success_status]) + + expect(required_check_names_for_main(monorepo_root: monorepo_root)).to be_nil + end end end From e2bf5da9defc3c956972dac2ebfc3a0ac621d936 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:13:48 -1000 Subject: [PATCH 05/13] fix(release): address PR #3407 review feedback round 2 (7 items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .claude/hooks/main-ci-status.sh | 25 ++++++++--- rakelib/release.rake | 41 ++++++++++++++---- .../release_rake_helpers_spec.rb | 43 +++++++++++++++++-- 3 files changed, 93 insertions(+), 16 deletions(-) diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index e75381c3b5..93bf5cefc4 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.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. diff --git a/rakelib/release.rake b/rakelib/release.rake index 4b275b2d79..ee4791eeaa 100644 --- a/rakelib/release.rake +++ b/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 diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index 0c06b0fb22..e6425c53bf 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -499,8 +499,9 @@ 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", @@ -508,8 +509,9 @@ def passing_run(name) } 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, @@ -517,8 +519,9 @@ def failing_run(name, conclusion: "failure") } 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") From b8ff2d48b2b71989cdee49a8789850b10dc1264d Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:26:16 -1000 Subject: [PATCH 06/13] fix(release): address PR #3407 round-3 cursor findings (2 items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hook (.claude/hooks/main-ci-status.sh): - SHA-key the CI status cache. The hook now resolves origin/main HEAD via git ls-remote BEFORE the cache lookup and keys the cache file by short SHA (.main-ci-status.cache.). The 5-minute mtime TTL falls back to its original role of rate-limiting API calls when the same SHA is checked many times; cross-SHA staleness can no longer happen because a new commit gets its own cache file. Older SHA-keyed files are pruned on a successful cache write, with a -mmin +1 guard to avoid racing concurrent sessions that may be writing for a different SHA. Legacy unkeyed cache name is retained as a last-resort fallback when ls-remote fails so a single failed call doesn't force a full live re-fetch (cursor bugbot, medium-severity). rakelib/release.rake: - fetch_main_ci_checks now routes the Errno::ENOENT path (missing `gh` binary) and JSON::ParserError path through handle_main_ci_status_violation! so they respect dry_run / allow_override the same way a git-fetch failure does. Previously both paths called abort unconditionally, letting a dry-run release exit hard before showing the intended CI diagnostics (cursor bugbot, medium-severity). - Added the rubocop:disable Metrics/MethodLength on fetch_main_ci_checks since the rescue blocks pushed the method over 41 lines; the method is intentionally linear (fetch → resolve sha → call gh → parse → return) and extracting helpers would obscure the flow. react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb: - 2 new tests for the dry-run paths through Errno::ENOENT and JSON::ParserError. .gitignore: - Widened .claude/.main-ci-status.cache → .main-ci-status.cache* to cover the new SHA-keyed cache files. 65 examples, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status.sh | 62 ++++++++++++++----- .gitignore | 2 +- rakelib/release.rake | 37 ++++++++--- .../release_rake_helpers_spec.rb | 40 ++++++++++++ 4 files changed, 116 insertions(+), 25 deletions(-) diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index 93bf5cefc4..ab87760745 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -15,10 +15,42 @@ set -u # No `set -e` — we want to handle errors ourselves to stay fail-open. REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" -CACHE_FILE="${REPO_ROOT}/.claude/.main-ci-status.cache" +CACHE_DIR="${REPO_ROOT}/.claude" +CACHE_PREFIX=".main-ci-status.cache" CACHE_TTL_SECONDS=300 -# If the cache is fresh, print it and exit. Per-platform stat invocation differs. +# Helper: print an "unavailable" message and exit 0 without writing the cache. +fail_open() { + echo "Main CI status unavailable: $1" + exit 0 +} + +# Resolve `origin/main` HEAD SHA up front so we can SHA-key the cache. +# We use `git ls-remote` (not `gh run list`) so the SHA reflects the +# current ref tip, not the latest push-workflow run — which can lag right +# after a push (or never appear at all for docs-only pushes when +# paths-ignore filters out every workflow). Matching the release gate's +# SHA semantics (`git rev-parse origin/main`) keeps session-time and +# release-time observations consistent. +# +# Resolving the SHA before the cache lookup also lets us key the cache by +# commit. With a mtime-only TTL, sessions could read the prior commit's +# summary as if it applied to today's tip when main advances inside the +# 5-minute window. A SHA-keyed file makes "no cache yet for this commit" +# the natural state, and the TTL falls back to its original role of +# rate-limiting API calls when the same SHA is checked many times. +head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') + +if [ -n "${head_sha}" ]; then + CACHE_FILE="${CACHE_DIR}/${CACHE_PREFIX}.${head_sha:0:12}" +else + # Network or git failure — fall back to the legacy un-keyed cache so a + # stale read is still possible, but a single failed `ls-remote` call + # doesn't force a full live re-fetch on every session start. + CACHE_FILE="${CACHE_DIR}/${CACHE_PREFIX}" +fi + +# If the cache for THIS SHA is fresh, print it and exit. Per-platform stat invocation differs. if [ -f "${CACHE_FILE}" ]; then if [ "$(uname)" = "Darwin" ]; then cache_mtime=$(stat -f %m "${CACHE_FILE}" 2>/dev/null || echo 0) @@ -33,23 +65,27 @@ if [ -f "${CACHE_FILE}" ]; then fi fi -# Helper: print an "unavailable" message and exit 0 without writing the cache. -fail_open() { - echo "Main CI status unavailable: $1" - exit 0 -} - # 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. +# +# After a successful swap, prune older SHA-keyed cache files so we don't +# accumulate one per main commit ever seen. The `-mmin +1` guard avoids +# racing a concurrent session that may have just written its own +# different-SHA cache. 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; } + find "${CACHE_DIR}" -maxdepth 1 \ + -name "${CACHE_PREFIX}*" \ + -not -name "$(basename "${CACHE_FILE}")" \ + -type f -mmin +1 \ + -delete 2>/dev/null || true printf '%s' "$1" } @@ -58,15 +94,7 @@ gh auth status >/dev/null 2>&1 || fail_open "gh CLI not authenticated (run \`gh repo_slug=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null) || fail_open "gh repo view failed" -# Resolve the actual `origin/main` HEAD SHA. We use `git ls-remote` (not -# `gh run list`) so the SHA reflects the current ref tip, not the latest -# push-workflow run — which can lag right after a push (or never appear at -# all for docs-only pushes when paths-ignore filters out every workflow). -# Matching the release gate's SHA semantics (`git rev-parse origin/main`) -# keeps session-time and release-time observations consistent. -head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') \ - || fail_open "git ls-remote origin main failed" -[ -n "${head_sha}" ] || fail_open "could not resolve origin/main HEAD" +[ -n "${head_sha}" ] || fail_open "git ls-remote origin main failed" # Pull every check run on the commit. We use the Checks API because a single # push commit triggers multiple workflows and we want them aggregated. diff --git a/.gitignore b/.gitignore index c69db45d56..3b98422a11 100644 --- a/.gitignore +++ b/.gitignore @@ -87,7 +87,7 @@ ssr-generated # Claude Code local settings .claude/settings.local.json .claude/napkin.md -.claude/.main-ci-status.cache +.claude/.main-ci-status.cache* .Codex/napkin.md # AI assistant internal files diff --git a/rakelib/release.rake b/rakelib/release.rake index ee4791eeaa..03caa1bacf 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -304,6 +304,7 @@ CI_INCOMPLETE_STATUSES = %w[in_progress queued waiting requested pending].freeze # paths-ignore skips, or workflows that intentionally short-circuit). CI_PASSING_CONCLUSIONS = %w[success skipped neutral].freeze +# rubocop:disable Metrics/MethodLength def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false) fetch_output, fetch_status = Open3.capture2e( "git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet" @@ -332,10 +333,24 @@ def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false) api_path = "repos/#{repo_slug}/commits/#{sha}/check-runs" # `--paginate --jq '.check_runs[]'` flattens paginated responses into JSONL. - # Each non-empty line is one check_run object. Using `capture_gh_output` so a - # missing `gh` binary aborts with a friendly install hint instead of crashing - # with Errno::ENOENT. - output, status = capture_gh_output("api", "--paginate", "--jq", ".check_runs[]", api_path) + # Each non-empty line is one check_run object. We invoke `gh` directly here + # (rather than via `capture_gh_output`) so that a missing binary routes + # through `handle_main_ci_status_violation!` — same as a git fetch failure — + # instead of aborting unconditionally. This keeps `dry_run` / `allow_override` + # symmetric across every fetch step. + begin + output, status = Open3.capture2e( + "gh", "api", "--paginate", "--jq", ".check_runs[]", api_path + ) + rescue Errno::ENOENT + handle_main_ci_status_violation!( + message: "❌ GitHub CLI (`gh`) is not installed. Install it from https://cli.github.com/ and retry.", + allow_override: allow_override, + dry_run: dry_run + ) + return nil + end + unless status.success? handle_main_ci_status_violation!( message: "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}", @@ -345,14 +360,22 @@ def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false) return nil end - check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line| - JSON.parse(line) + begin + check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line| + JSON.parse(line) + end rescue JSON::ParserError => e - abort "❌ Failed to parse check_runs response from gh: #{e.message}\n\nOutput:\n#{output}" + handle_main_ci_status_violation!( + message: "❌ Failed to parse check_runs response from gh: #{e.message}\n\nOutput:\n#{output}", + allow_override: allow_override, + dry_run: dry_run + ) + return nil end { sha: sha, check_runs: check_runs } end +# rubocop:enable Metrics/MethodLength def required_check_names_for_main(monorepo_root:) repo_slug = github_repo_slug(monorepo_root) diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index e6425c53bf..8d905d0c9a 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -858,6 +858,46 @@ def in_progress_run(name, id: rand(1_000_000)) end.to raise_error(SystemExit, /GitHub CLI .* is not installed/) end + it "warns instead of aborting when `gh` is missing in dry-run mode" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["", success_status]) + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "rev-parse", "origin/main") + .and_return(["abc1234\n", success_status]) + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--paginate", "--jq", ".check_runs[]", + "repos/shakacode/react_on_rails/commits/abc1234/check-runs") + .and_raise(Errno::ENOENT) + + result = nil + expect do + result = fetch_main_ci_checks(monorepo_root: monorepo_root, dry_run: true) + end.to output(/DRY RUN.*GitHub CLI .* is not installed/m).to_stdout + expect(result).to be_nil + end + + it "warns instead of aborting on unparseable check-runs JSON in dry-run mode" do + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") + .and_return(["", success_status]) + allow(Open3).to receive(:capture2e) + .with("git", "-C", monorepo_root, "rev-parse", "origin/main") + .and_return(["abc1234\n", success_status]) + allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--paginate", "--jq", ".check_runs[]", + "repos/shakacode/react_on_rails/commits/abc1234/check-runs") + .and_return(["this is not json", success_status]) + + result = nil + expect do + result = fetch_main_ci_checks(monorepo_root: monorepo_root, dry_run: true) + end.to output(/DRY RUN.*Failed to parse check_runs response/m).to_stdout + expect(result).to be_nil + end + it "parses paginated JSONL check_runs into an array of hashes" do allow(Open3).to receive(:capture2e) .with("git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet") From 34f55ab1d16aff75a1cfc17fe1a63c2500bd9a87 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:31:04 -1000 Subject: [PATCH 07/13] fix(release): treat unknown check_run statuses as a failure (1 item) rakelib/release.rake: - validate_main_ci_status! now catches check_runs whose `status` falls outside both "completed" and CI_INCOMPLETE_STATUSES (e.g. a future GitHub status value not yet in our allowlist, or a nil from a malformed response). Previously such runs slipped through silently and the gate would report main as healthy. Defensive: a last-line release gate should err toward blocking the ambiguous case (claude[bot] on commit f5a9476). react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb: - Two new tests for the unknown-status path: one for a literal "scheduled" status (a hypothetical future GitHub value) and one for nil. 67 examples, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) --- rakelib/release.rake | 17 ++++++++++ .../release_rake_helpers_spec.rb | 34 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/rakelib/release.rake b/rakelib/release.rake index 03caa1bacf..ff8d47352e 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -541,6 +541,23 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr return end + # Catch any run whose status falls outside both the "completed" and + # `CI_INCOMPLETE_STATUSES` buckets — e.g. a new GitHub status value we + # don't yet know about, or a `nil` from a malformed response. Treat the + # ambiguity as a failure rather than letting it slip through as green; + # the release gate is supposed to be the last-line check. + unknown = evaluated.reject do |run| + run["status"] == "completed" || CI_INCOMPLETE_STATUSES.include?(run["status"]) + end + if unknown.any? + handle_main_ci_status_violation!( + message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: unknown), + allow_override: allow_override, + dry_run: dry_run + ) + return + end + qualifier = required_names.nil? ? "" : "required " noun = evaluated.length == 1 ? "check" : "checks" puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{qualifier}#{noun})" diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index 8d905d0c9a..77189258a2 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -781,6 +781,40 @@ def in_progress_run(name, id: rand(1_000_000)) ) end end + + context "when a check has an unknown status (neither completed nor in CI_INCOMPLETE_STATUSES)" do + it "treats the ambiguity as a failure rather than silently passing through" do + weird_run = passing_run("Future Status").merge("status" => "scheduled", "conclusion" => nil) + 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"), weird_run]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Future Status}m) + end + + it "treats a nil status as a failure too" do + weird_run = passing_run("Malformed").merge("status" => nil, "conclusion" => nil) + 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"), weird_run]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Malformed}m) + end + end end describe "#fetch_main_ci_checks" do From a7be25d4c447f47dad6321e81d5ee71d8e59fc71 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:35:12 -1000 Subject: [PATCH 08/13] fix(release): dedup by (check_suite_id, name) + tighten push matcher (2 items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This pre-existing dedup was a real bug — both the original Ruby code AND my round-2 mirror-in-the-hook extended it: grouping by name alone collapses unrelated workflows that happen to share a job name. This repo has multiple workflows that each define a `detect-changes` job, so a passing run from one workflow was masking failing runs from others. Running the updated hook on the current origin/main shows 66 checks total, 11 failed — the prior dedup reported 26 / 1 (10 of the failures were silently dropped). rakelib/release.rake: - validate_main_ci_status! dedup key is now `[check_suite.id, name]`, not just `name`. True reruns (same suite, same name) still collapse to the latest attempt; cross-workflow same-name jobs stay distinct. .claude/hooks/main-ci-status.sh: - Matching jq aggregation now keys by `[suite_id, name]` so session-time output matches release-time output. The previous round-2 change brought the hook and Ruby code into sync — this round fixes the underlying shared bug in both. .claude/hooks/main-ci-status-on-push.sh: - Push matcher used to only fire on explicit `git push ... origin main`. Bare `git push` and `git push origin` (common shortcuts while on `main` with upstream tracking) were silently skipped. Added a third clause that catches any `git push` invocation when the current branch is `main`, preserving the stated guardrail without over-triggering on feature-branch work. react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb: - New test: two workflows emitting the same-named job on the same commit stay distinct (failing one is still reported). - New test: rerun-within-suite collapses while cross-suite same-name runs are preserved. Both findings from chatgpt-codex-connector on commit ffcb34474. 69 examples, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status-on-push.sh | 18 +++++-- .claude/hooks/main-ci-status.sh | 15 +++--- rakelib/release.rake | 18 ++++--- .../release_rake_helpers_spec.rb | 49 +++++++++++++++++++ 4 files changed, 82 insertions(+), 18 deletions(-) diff --git a/.claude/hooks/main-ci-status-on-push.sh b/.claude/hooks/main-ci-status-on-push.sh index 151785decc..a479f395ed 100755 --- a/.claude/hooks/main-ci-status-on-push.sh +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -14,15 +14,25 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) # Match: # - `gh pr create` (any args) -# - `git push ... origin main` or `git push ... origin HEAD` — must be -# followed by whitespace or end-of-string. Without that anchor, a glob -# substring match would also fire on `git push origin main-feature`, -# `maintenance`, etc. +# - explicit `git push ... origin main` or `git push ... origin HEAD` +# - ANY `git push` invocation while currently checked out on `main` — this +# covers the common shortcuts that would otherwise slip past us: +# `git push` (bare, with upstream tracking main) +# `git push origin` (no refspec — pushes matching branches) +# `git push -u origin main` (already covered by explicit match) +# The current-branch check keeps over-triggering minimal on feature work. matched=0 if [[ "${cmd}" =~ (^|[[:space:]])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$) ]]; then matched=1 elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]+[^[:space:]]+)*[[:space:]]+origin[[:space:]]+(main|HEAD)([[:space:]]|$) ]]; then matched=1 +elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]|$) ]]; then + # Any `git push` — check whether the current branch is main. + script_repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + current_branch=$(git -C "${script_repo_root}" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "") + if [ "${current_branch}" = "main" ]; then + matched=1 + fi fi [ "${matched}" -eq 1 ] || exit 0 diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index ab87760745..9a08a587c5 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -109,14 +109,15 @@ checks_jsonl=$(gh api \ --jq '.check_runs[]' \ 2>/dev/null) || fail_open "gh api check-runs 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. +# Collapse multiple runs per (check_suite_id, name) to the most recent +# attempt (highest check_run id). The key intentionally includes the +# suite id so we only collapse true reruns and not unrelated workflows +# that happen to share a job name (e.g. this repo has multiple workflows +# that each define a `detect-changes` job). Mirrors the Ruby dedup at +# release.rake:451-455. Keep the two in sync. checks_json=$(echo "${checks_jsonl}" | jq -s ' - [.[] | {id, name, status, conclusion, html_url}] - | group_by(.name) + [.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // null)}] + | group_by([.suite_id, .name]) | map(max_by(.id)) ' 2>/dev/null) || fail_open "jq slurp failed" diff --git a/rakelib/release.rake b/rakelib/release.rake index ff8d47352e..353770c375 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -473,14 +473,18 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr return end - # 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 block the - # release. `id` is monotonically increasing per check run on a commit, so - # `max_by { id }` reliably selects the latest attempt. + # Collapse multiple runs per (check_suite_id, name) to the most recent + # attempt (highest check_run id). The key intentionally includes + # check_suite_id so we only collapse *true* reruns (same workflow run, + # same job name) and not unrelated workflows that happen to share a job + # name. For example, this repo has multiple workflows that each define a + # `detect-changes` job; without the suite_id in the key, a passing run + # from one workflow could mask a failing run from another. `id` is + # monotonically increasing per check run, so `max_by { id }` reliably + # selects the latest attempt within a suite. check_runs = check_runs - .group_by { |run| run["name"] } - .map { |_name, runs| runs.max_by { |run| run["id"].to_i } } + .group_by { |run| [run.dig("check_suite", "id"), run["name"]] } + .map { |_key, runs| runs.max_by { |run| run["id"].to_i } } # For prereleases, restrict the gate to GitHub-branch-protection-required checks. # For stable releases, evaluate every check run on the commit. diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index 77189258a2..c0b6348954 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -654,6 +654,55 @@ def in_progress_run(name, id: rand(1_000_000)) end end + context "when two workflows emit jobs with the same name on the same commit" do + it "preserves both runs (different check_suite ids) instead of collapsing them" do + workflow_a_passing = passing_run("detect-changes").merge( + "id" => 1, "check_suite" => { "id" => 100 } + ) + workflow_b_failing = failing_run("detect-changes").merge( + "id" => 2, "check_suite" => { "id" => 200 } + ) + 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: [workflow_a_passing, workflow_b_failing]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*detect-changes}m) + end + end + + context "when a rerun and a same-named distinct-suite run exist together" do + it "collapses only within a suite (same-suite rerun) and keeps cross-suite runs distinct" do + suite_a_old_failed = failing_run("detect-changes").merge( + "id" => 1, "check_suite" => { "id" => 100 } + ) + suite_a_new_passed = passing_run("detect-changes").merge( + "id" => 3, "check_suite" => { "id" => 100 } + ) + suite_b_passing = passing_run("detect-changes").merge( + "id" => 2, "check_suite" => { "id" => 200 } + ) + 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: [suite_a_old_failed, suite_a_new_passed, suite_b_passing]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to output(/Main CI is healthy on #{short_sha} \(2 checks\)/).to_stdout + end + end + context "when there are zero check runs visible" do it "aborts with a 'no CI data' message" do allow(self).to receive(:fetch_main_ci_checks) From ceb5ea1c80ce6eebc59f2e07e3276a1f142f01c2 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:40:34 -1000 Subject: [PATCH 09/13] fix(release): round-6 review feedback (5 items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rakelib/release.rake: - validate_main_ci_status! now applies the missing-required-check gate to STABLE releases too. Branch protection would refuse a merge when required checks are missing, so a stable release shouldn't ship past that gap either. Prerelease mode still uses the strict required-only subset for the failed/in_progress gates; stable still evaluates all visible check_runs for those (cursor bugbot). .claude/hooks/main-ci-status.sh: - write_cache_atomic failure no longer discards the computed status. If the cache write fails (disk full, .claude/ read-only), we still print the output so the agent sees the useful information we already paid for the API round-trip to compute (claude[bot]). .claude/hooks/main-ci-status-on-push.sh: - Pre-push matcher now also fires on refspec-form pushes (`git push origin HEAD:main`, `git push origin feature:main`, etc.). Combined with the round-5 "on main + any push" fallback, all common forms of pushing to main now surface the CI status block (cursor bugbot). docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md: - Corrected the hook command description: the actual implementation uses `gh api repos/{slug}/commits/{sha}/check-runs` (the Checks API), not `gh run list`. The Checks API approach was the right call — it gives per-check-run deduplication that matches the release gate (claude[bot]). react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb: - Replaced `rand(1_000_000)` default `id` with a fixed sentinel 9999 so test data stays deterministic. Tests that rely on `max_by { id }` ordering supply explicit ids (claude[bot]). - Added a `before` block stubbing `required_check_names_for_main` to return nil by default, accommodating the new unconditional call from `validate_main_ci_status!`. - New test: stable releases also abort when a required check is missing. 70 examples, 0 failures. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status-on-push.sh | 22 ++++++---- .claude/hooks/main-ci-status.sh | 6 ++- ...2026-05-25-main-ci-release-guard-design.md | 2 +- rakelib/release.rake | 32 +++++++++----- .../release_rake_helpers_spec.rb | 42 +++++++++++++++++-- 5 files changed, 81 insertions(+), 23 deletions(-) diff --git a/.claude/hooks/main-ci-status-on-push.sh b/.claude/hooks/main-ci-status-on-push.sh index a479f395ed..ee77a6087f 100755 --- a/.claude/hooks/main-ci-status-on-push.sh +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -15,19 +15,27 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) # Match: # - `gh pr create` (any args) # - explicit `git push ... origin main` or `git push ... origin HEAD` -# - ANY `git push` invocation while currently checked out on `main` — this -# covers the common shortcuts that would otherwise slip past us: -# `git push` (bare, with upstream tracking main) -# `git push origin` (no refspec — pushes matching branches) -# `git push -u origin main` (already covered by explicit match) -# The current-branch check keeps over-triggering minimal on feature work. +# - any `git push` that contains a refspec ending in `:main` or `:HEAD` +# (e.g. `git push origin HEAD:main`, `git push origin feature:main`) +# - ANY `git push` invocation while currently checked out on `main` — +# covers the common shortcuts: `git push`, `git push origin`, +# `git push -u`, etc. +# +# We deliberately over-trigger rather than try to enumerate every form +# of `git push`. The 5-minute SHA-keyed cache makes the cost negligible, +# and a false negative ("agent silently pushed to main without seeing +# CI status") is much worse than a false positive ("status shown +# before a feature-branch push"). matched=0 if [[ "${cmd}" =~ (^|[[:space:]])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$) ]]; then matched=1 elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]+[^[:space:]]+)*[[:space:]]+origin[[:space:]]+(main|HEAD)([[:space:]]|$) ]]; then matched=1 +elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push.*:(main|HEAD)([[:space:]]|$) ]]; then + # Refspec form: `git push origin HEAD:main`, `git push origin feature:main`, etc. + matched=1 elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]|$) ]]; then - # Any `git push` — check whether the current branch is main. + # Any other `git push` — check whether the current branch is main. script_repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" current_branch=$(git -C "${script_repo_root}" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "") if [ "${current_branch}" = "main" ]; then diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index 9a08a587c5..a98cf63d65 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -183,6 +183,10 @@ else ) fi -write_cache_atomic "${output}" || fail_open "cache write failed" +# If the cache write itself fails (filesystem full, .claude/ read-only, +# interrupted mv), still emit the computed status — we already spent a +# full API round-trip on it and discarding the result just because the +# cache was unwritable is worse than not caching. +write_cache_atomic "${output}" || { printf '%s\n' "${output}"; exit 0; } exit 0 diff --git a/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md index 834623b5c4..6727113c12 100644 --- a/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md +++ b/docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md @@ -160,7 +160,7 @@ end ### `.claude/hooks/main-ci-status.sh` -A small bash script that runs `gh run list --branch main --limit 1 --event push --json conclusion,headSha,workflowName,databaseId,url,status` and emits a compact status block: +A small bash script that queries `gh api repos/{slug}/commits/{sha}/check-runs` (the GitHub Checks API, not `gh run list`) and emits a compact status block: ```text Main CI status (3103496d, pushed 7h ago): diff --git a/rakelib/release.rake b/rakelib/release.rake index 353770c375..4012bbae01 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -486,20 +486,30 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr .group_by { |run| [run.dig("check_suite", "id"), run["name"]] } .map { |_key, runs| runs.max_by { |run| run["id"].to_i } } - # For prereleases, restrict the gate to GitHub-branch-protection-required checks. - # For stable releases, evaluate every check run on the commit. - 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"]) } + # Always query branch-protection required checks (when configured) so the + # missing-required-check gate applies to both stable and prerelease. + # `evaluated` then differs by mode: + # - prerelease: only the required subset (strict gate) + # - stable: every check_run on the commit (lenient — non-required + # checks like benchmarks still block on failure) + required_names = required_check_names_for_main(monorepo_root: monorepo_root) + evaluated = if is_prerelease && required_names + check_runs.select { |run| required_names.include?(run["name"]) } + else + check_runs + end # 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). + # check as blocking — for stable AND prerelease. 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 or + # deleted workflow that branch protection still requires). unless required_names.nil? - missing_names = required_names - evaluated.map { |run| run["name"] } + observed_names = check_runs.map { |run| run["name"] } + missing_names = required_names - observed_names 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) + diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index c0b6348954..661addbf68 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -499,7 +499,21 @@ let(:sha) { "abc1234def5678abcdef" } let(:short_sha) { "abc1234d" } - def passing_run(name, id: rand(1_000_000)) + # `validate_main_ci_status!` now queries `required_check_names_for_main` + # unconditionally so the missing-required-check gate can apply to both + # stable and prerelease. The helper shells out to `git -C monorepo_root` + # which would abort in tests where `monorepo_root` is a stub path. Default + # to "no required checks configured" so tests that don't care about the + # gate behave as before; tests that exercise the gate override this stub. + before do + allow(self).to receive(:required_check_names_for_main) + .with(monorepo_root: monorepo_root).and_return(nil) + end + + # Fixed sentinel default keeps test data deterministic. Tests that rely + # on `max_by { id }` ordering (rerun-dedup, cross-suite dedup) supply + # explicit ids via `.merge("id" => N)`. + def passing_run(name, id: 9999) { "id" => id, "name" => name, @@ -509,7 +523,7 @@ def passing_run(name, id: rand(1_000_000)) } end - def failing_run(name, conclusion: "failure", id: rand(1_000_000)) + def failing_run(name, conclusion: "failure", id: 9999) { "id" => id, "name" => name, @@ -519,7 +533,7 @@ def failing_run(name, conclusion: "failure", id: rand(1_000_000)) } end - def in_progress_run(name, id: rand(1_000_000)) + def in_progress_run(name, id: 9999) { "id" => id, "name" => name, @@ -831,6 +845,28 @@ def in_progress_run(name, id: rand(1_000_000)) end end + context "when some required checks are missing on a stable release" do + it "aborts on stable too (branch protection would refuse the merge)" 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: false, + allow_override: false, + dry_run: false + ) + end.to raise_error( + SystemExit, + /Some required CI checks are missing.*Missing:\s*Build/m + ) + end + end + context "when a check has an unknown status (neither completed nor in CI_INCOMPLETE_STATUSES)" do it "treats the ambiguity as a failure rather than silently passing through" do weird_run = passing_run("Future Status").merge("status" => "scheduled", "conclusion" => nil) From f3e157181895211796717c4d8ac6bc51ebc34795 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:48:19 -1000 Subject: [PATCH 10/13] fix(release): round-7 polish (4 items) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .claude/hooks/main-ci-status.sh: - Add explicit `\n` to the printf in write_cache_atomic (and in the fail-soft fallback). The surrounding `output=$(...)` substitution strips trailing newlines, so without re-adding one here the cached file and stdout print both ended mid-line and glommed onto the next shell prompt (claude[bot]). - Add a comment above the `git ls-remote` call clarifying that it runs even on a cache hit (we need the SHA to key the cache correctly). The 5-min TTL only eliminates the more expensive `gh api` call (claude[bot]). rakelib/release.rake: - Add a comment in required_check_names_for_main explaining the deliberate use of capture_gh_output (fail-safe nil) vs the direct Open3.capture2e in fetch_main_ci_checks (routes through handle_main_ci_status_violation! for dry_run/override semantics). Same wrapper choice, different rationale — comment so future readers don't see it as inconsistency (claude[bot]). .claude/settings.json: - Reference the two new hook scripts via ${CLAUDE_PROJECT_DIR} so they resolve correctly when a session is launched from a subdirectory (e.g. `react_on_rails/spec/dummy`). The existing `bin/claude-hooks/autofix-file` entry is pre-existing and left as-is — out of scope for this PR (chatgpt-codex-connector P2). 70 examples, 0 failures. Shellcheck + RuboCop clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status.sh | 16 +++++++++++++--- .claude/settings.json | 4 ++-- rakelib/release.rake | 6 ++++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index a98cf63d65..c7b01024d6 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -39,6 +39,10 @@ fail_open() { # 5-minute window. A SHA-keyed file makes "no cache yet for this commit" # the natural state, and the TTL falls back to its original role of # rate-limiting API calls when the same SHA is checked many times. +# NOTE: `git ls-remote` always runs — even on a cache hit — because we need +# the current SHA to key the cache correctly. This is one lightweight network +# call per session start (~50-200ms). The 5-min TTL only eliminates the more +# expensive `gh api .../check-runs` call that follows on a cache miss. head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') if [ -n "${head_sha}" ]; then @@ -79,14 +83,18 @@ fi write_cache_atomic() { local tmp tmp=$(mktemp "${CACHE_FILE}.XXXXXX") || return 1 - printf '%s' "$1" >"${tmp}" || { rm -f "${tmp}"; return 1; } + # Trailing newline keeps the next shell prompt on its own line. The + # surrounding `output=$(...)` substitution strips trailing newlines, so + # without explicitly re-adding one here both the cached file and the + # stdout print would end mid-line. + printf '%s\n' "$1" >"${tmp}" || { rm -f "${tmp}"; return 1; } mv -f "${tmp}" "${CACHE_FILE}" || { rm -f "${tmp}"; return 1; } find "${CACHE_DIR}" -maxdepth 1 \ -name "${CACHE_PREFIX}*" \ -not -name "$(basename "${CACHE_FILE}")" \ -type f -mmin +1 \ -delete 2>/dev/null || true - printf '%s' "$1" + printf '%s\n' "$1" } command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed" @@ -186,7 +194,9 @@ fi # If the cache write itself fails (filesystem full, .claude/ read-only, # interrupted mv), still emit the computed status — we already spent a # full API round-trip on it and discarding the result just because the -# cache was unwritable is worse than not caching. +# cache was unwritable is worse than not caching. The `\n` here matches +# write_cache_atomic's normal output path so the printed status block +# never glues onto the next shell prompt. write_cache_atomic "${output}" || { printf '%s\n' "${output}"; exit 0; } exit 0 diff --git a/.claude/settings.json b/.claude/settings.json index 3db0cfc5e6..c0efdfd24c 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -5,7 +5,7 @@ "hooks": [ { "type": "command", - "command": ".claude/hooks/main-ci-status.sh" + "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/main-ci-status.sh" } ] } @@ -16,7 +16,7 @@ "hooks": [ { "type": "command", - "command": ".claude/hooks/main-ci-status-on-push.sh" + "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/main-ci-status-on-push.sh" } ] } diff --git a/rakelib/release.rake b/rakelib/release.rake index 4012bbae01..6e85f33c5b 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -385,6 +385,12 @@ def required_check_names_for_main(monorepo_root:) # leaves `contexts` as `[]`, so reading only `contexts` would yield an empty # array and trip the `:no_required_checks` abort path even when CI is green. jq_query = "(.contexts // []) + (.checks // [] | map(.context)) | unique" + # Intentional: use `capture_gh_output` (which aborts on missing `gh`) here, + # not `Open3.capture2e` directly like `fetch_main_ci_checks` does. The + # difference is deliberate — this helper's failure mode is "branch + # protection unknown" which we treat as nil/fail-safe (caller falls back + # to evaluating every check_run), so we don't need the + # `handle_main_ci_status_violation!` dry-run/override routing. output, status = capture_gh_output("api", "--jq", jq_query, api_path) # If branch protection isn't configured, isn't queryable with current token scope, or the # endpoint returns 404, fall through to nil so the caller treats all checks as required From ce0565e322000cbf2cb40dae7ad691aa2457fa80 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 16:55:44 -1000 Subject: [PATCH 11/13] =?UTF-8?q?fix(release):=20final=20cleanup=20?= =?UTF-8?q?=E2=80=94=20refs/heads/main=20+=20check=5Fsuite=20nil=20guard?= =?UTF-8?q?=20(2=20items)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit .claude/hooks/main-ci-status.sh: - `git ls-remote origin main` → `git ls-remote origin refs/heads/main`. The bare-name form is a glob that would match `refs/tags/main` too if such a tag existed on the remote, with `awk 'NR==1'` returning whichever ref the server listed first. `refs/heads/main` is unambiguous and always resolves to the branch tip (claude[bot]). - jq dedup falls back to the run's own `id` when `check_suite.id` is absent, so nil-suite runs from different workflows don't collapse: `suite_id: (.check_suite.id // .id)`. rakelib/release.rake: - Same nil-suite guard in `validate_main_ci_status!`: `[run.dig("check_suite", "id") || run["id"], run["name"]]`. GitHub Actions check_runs always populate `check_suite`, so this only affects external/third-party check integrations (claude[bot]). react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb: - Updated the existing rerun test to specify `check_suite => {"id" => 100}` on both attempts. Real reruns from the GitHub API preserve the suite across attempts; the previous test was simplified to omit it, which worked under the old name-only dedup but would now incorrectly treat the two attempts as distinct workflows under the suite-aware dedup with the new nil-fallback. The test now models reality. 70 examples, 0 failures. ShellCheck + RuboCop clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status.sh | 4 ++-- rakelib/release.rake | 7 ++++++- .../spec/react_on_rails/release_rake_helpers_spec.rb | 8 ++++++-- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index c7b01024d6..5ea88a567b 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -43,7 +43,7 @@ fail_open() { # the current SHA to key the cache correctly. This is one lightweight network # call per session start (~50-200ms). The 5-min TTL only eliminates the more # expensive `gh api .../check-runs` call that follows on a cache miss. -head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') +head_sha=$(git -C "${REPO_ROOT}" ls-remote origin refs/heads/main 2>/dev/null | awk 'NR==1 {print $1}') if [ -n "${head_sha}" ]; then CACHE_FILE="${CACHE_DIR}/${CACHE_PREFIX}.${head_sha:0:12}" @@ -124,7 +124,7 @@ checks_jsonl=$(gh api \ # that each define a `detect-changes` job). Mirrors the Ruby dedup at # release.rake:451-455. Keep the two in sync. checks_json=$(echo "${checks_jsonl}" | jq -s ' - [.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // null)}] + [.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // .id)}] | group_by([.suite_id, .name]) | map(max_by(.id)) ' 2>/dev/null) || fail_open "jq slurp failed" diff --git a/rakelib/release.rake b/rakelib/release.rake index 6e85f33c5b..52e47bad7e 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -488,8 +488,13 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr # from one workflow could mask a failing run from another. `id` is # monotonically increasing per check run, so `max_by { id }` reliably # selects the latest attempt within a suite. + # When `check_suite` is absent (rare — third-party integrations that don't + # attach to a suite), fall back to the run's own `id` for the group key so + # each nil-suite run sits in its own group and is never collapsed with + # another. The GitHub Actions Checks API always populates `check_suite`, + # so this only matters for external check integrations. check_runs = check_runs - .group_by { |run| [run.dig("check_suite", "id"), run["name"]] } + .group_by { |run| [run.dig("check_suite", "id") || run["id"], run["name"]] } .map { |_key, runs| runs.max_by { |run| run["id"].to_i } } # Always query branch-protection required checks (when configured) so the diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index 661addbf68..1364b6de86 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -651,8 +651,12 @@ def in_progress_run(name, id: 9999) context "when a check has been rerun and the latest attempt passes" do it "evaluates only the latest attempt per check name and passes" do - old_failed = failing_run("Lint").merge("id" => 1) - new_passed = passing_run("Lint").merge("id" => 2) + # Reruns from the GitHub API preserve `check_suite.id` across attempts. + # The dedup key includes the suite id so cross-workflow runs that share + # a name don't collapse; both attempts here belong to the same suite, + # which is what makes them a rerun rather than two distinct workflows. + old_failed = failing_run("Lint").merge("id" => 1, "check_suite" => { "id" => 100 }) + new_passed = passing_run("Lint").merge("id" => 2, "check_suite" => { "id" => 100 }) 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: [old_failed, new_passed]) From ad0a6396f8976400734ece3958e29062d1fa250a Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Mon, 25 May 2026 17:17:02 -1000 Subject: [PATCH 12/13] fix(release): review feedback round 8 (2 items) - release.rake: only label success-message count as "required" when `evaluated` was actually filtered to the required subset. On stable releases the filter is skipped, so reporting "(N required checks)" misrepresented the count when branch protection was visible. - main-ci-status.sh: replace stale `release.rake:451-455` line-number pointer (which now lands inside an error heredoc) with a stable reference to `validate_main_ci_status!` so the "keep in sync" note does not rot on the next edit. Co-Authored-By: Claude Opus 4.7 (1M context) --- .claude/hooks/main-ci-status.sh | 4 ++-- rakelib/release.rake | 7 ++++++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index 5ea88a567b..01424aabea 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -121,8 +121,8 @@ checks_jsonl=$(gh api \ # attempt (highest check_run id). The key intentionally includes the # suite id so we only collapse true reruns and not unrelated workflows # that happen to share a job name (e.g. this repo has multiple workflows -# that each define a `detect-changes` job). Mirrors the Ruby dedup at -# release.rake:451-455. Keep the two in sync. +# that each define a `detect-changes` job). Mirrors the Ruby dedup in +# `validate_main_ci_status!` (rakelib/release.rake). Keep the two in sync. checks_json=$(echo "${checks_jsonl}" | jq -s ' [.[] | {id, name, status, conclusion, html_url, suite_id: (.check_suite.id // .id)}] | group_by([.suite_id, .name]) diff --git a/rakelib/release.rake b/rakelib/release.rake index 52e47bad7e..660c60a449 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -583,7 +583,12 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr return end - qualifier = required_names.nil? ? "" : "required " + # Only label the count "required" when `evaluated` was actually filtered to + # the required subset (prerelease + branch protection visible). On stable + # releases we keep evaluating every check_run, so the count includes + # non-required runs and labelling them "required" would misrepresent the + # gate. + qualifier = is_prerelease && required_names ? "required " : "" noun = evaluated.length == 1 ? "check" : "checks" puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{qualifier}#{noun})" end From 3076a5139a7d657cae29e0d254e45095ccc214d5 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Tue, 26 May 2026 16:09:55 -1000 Subject: [PATCH 13/13] fix(release): address final CI guard review feedback --- .claude/hooks/main-ci-status-on-push.sh | 26 ++-- .claude/hooks/main-ci-status.sh | 135 ++++++++++++------ rakelib/release.rake | 39 ++--- .../release_rake_helpers_spec.rb | 69 +++++++-- 4 files changed, 191 insertions(+), 78 deletions(-) diff --git a/.claude/hooks/main-ci-status-on-push.sh b/.claude/hooks/main-ci-status-on-push.sh index ee77a6087f..2d0f3cc6df 100755 --- a/.claude/hooks/main-ci-status-on-push.sh +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -14,9 +14,12 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) # Match: # - `gh pr create` (any args) -# - explicit `git push ... origin main` or `git push ... origin HEAD` -# - any `git push` that contains a refspec ending in `:main` or `:HEAD` -# (e.g. `git push origin HEAD:main`, `git push origin feature:main`) +# - explicit `git push ... origin main`, `HEAD`, or `refs/heads/main` +# - refspecs that update main (e.g. `HEAD:main`, +# `HEAD:refs/heads/main`, `feature:refs/heads/main`) +# - multi-ref pushes that may update main (`--all`, `--mirror`) +# - `git -C push ...` forms, via intentionally broad `git ... push` +# detection # - ANY `git push` invocation while currently checked out on `main` — # covers the common shortcuts: `git push`, `git push origin`, # `git push -u`, etc. @@ -29,12 +32,17 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null) matched=0 if [[ "${cmd}" =~ (^|[[:space:]])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$) ]]; then matched=1 -elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]+[^[:space:]]+)*[[:space:]]+origin[[:space:]]+(main|HEAD)([[:space:]]|$) ]]; then - matched=1 -elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push.*:(main|HEAD)([[:space:]]|$) ]]; then - # Refspec form: `git push origin HEAD:main`, `git push origin feature:main`, etc. - matched=1 -elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]|$) ]]; then +elif [[ "${cmd}" =~ (^|[[:space:]])git([[:space:]]+[^[:space:]]+)*[[:space:]]+push([[:space:]]|$) ]]; then + if [[ "${cmd}" =~ (^|[[:space:]])--(all|mirror)([[:space:]]|$) ]]; then + matched=1 + elif [[ "${cmd}" =~ (^|[[:space:]:/])refs/heads/main([[:space:]]|$) ]]; then + matched=1 + elif [[ "${cmd}" =~ (^|[[:space:]:])main([[:space:]]|$) ]]; then + matched=1 + elif [[ "${cmd}" =~ (^|[[:space:]])HEAD(:refs/heads/main|:main)?([[:space:]]|$) ]]; then + matched=1 + fi + # Any other `git push` — check whether the current branch is main. script_repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" current_branch=$(git -C "${script_repo_root}" rev-parse --abbrev-ref HEAD 2>/dev/null || echo "") diff --git a/.claude/hooks/main-ci-status.sh b/.claude/hooks/main-ci-status.sh index 01424aabea..02842b0702 100755 --- a/.claude/hooks/main-ci-status.sh +++ b/.claude/hooks/main-ci-status.sh @@ -25,24 +25,70 @@ fail_open() { exit 0 } -# Resolve `origin/main` HEAD SHA up front so we can SHA-key the cache. -# We use `git ls-remote` (not `gh run list`) so the SHA reflects the -# current ref tip, not the latest push-workflow run — which can lag right -# after a push (or never appear at all for docs-only pushes when -# paths-ignore filters out every workflow). Matching the release gate's -# SHA semantics (`git rev-parse origin/main`) keeps session-time and +read_fresh_cache() { + local cache_file="$1" + local cache_mtime + local now + local age + + [ -f "${cache_file}" ] || return 1 + + if [ "$(uname)" = "Darwin" ]; then + cache_mtime=$(stat -f %m "${cache_file}" 2>/dev/null || echo 0) + else + cache_mtime=$(stat -c %Y "${cache_file}" 2>/dev/null || echo 0) + fi + + now=$(date +%s) + age=$((now - cache_mtime)) + if [ "${age}" -lt "${CACHE_TTL_SECONDS}" ]; then + cat "${cache_file}" + exit 0 + fi + + return 1 +} + +github_slug_from_origin() { + local origin_url + local slug + + origin_url=$(git -C "${REPO_ROOT}" remote get-url origin 2>/dev/null) || return 1 + case "${origin_url}" in + https://github.com/*) + slug=${origin_url#https://github.com/} + ;; + git@github.com:*) + slug=${origin_url#git@github.com:} + ;; + ssh://git@github.com/*) + slug=${origin_url#ssh://git@github.com/} + ;; + *) + return 1 + ;; + esac + + slug=${slug%.git} + [ -n "${slug}" ] || return 1 + printf '%s\n' "${slug}" +} + +# Fast path: if the local remote-tracking ref has a warm SHA-keyed cache, +# print it without making a live network call. On cache miss we still ask the +# remote for the authoritative current SHA before querying GitHub checks. +local_head_sha=$(git -C "${REPO_ROOT}" rev-parse --verify origin/main 2>/dev/null || echo "") +if [ -n "${local_head_sha}" ]; then + CACHE_FILE="${CACHE_DIR}/${CACHE_PREFIX}.${local_head_sha:0:12}" + read_fresh_cache "${CACHE_FILE}" +fi + +# Resolve `origin/main` HEAD SHA from the remote before querying checks. We use +# `git ls-remote` (not `gh run list`) so the SHA reflects the current ref tip, +# not the latest push-workflow run — which can lag right after a push (or never +# appear at all for docs-only pushes when paths-ignore filters every workflow). +# Matching the release gate's origin/main semantics keeps session-time and # release-time observations consistent. -# -# Resolving the SHA before the cache lookup also lets us key the cache by -# commit. With a mtime-only TTL, sessions could read the prior commit's -# summary as if it applied to today's tip when main advances inside the -# 5-minute window. A SHA-keyed file makes "no cache yet for this commit" -# the natural state, and the TTL falls back to its original role of -# rate-limiting API calls when the same SHA is checked many times. -# NOTE: `git ls-remote` always runs — even on a cache hit — because we need -# the current SHA to key the cache correctly. This is one lightweight network -# call per session start (~50-200ms). The 5-min TTL only eliminates the more -# expensive `gh api .../check-runs` call that follows on a cache miss. head_sha=$(git -C "${REPO_ROOT}" ls-remote origin refs/heads/main 2>/dev/null | awk 'NR==1 {print $1}') if [ -n "${head_sha}" ]; then @@ -54,20 +100,8 @@ else CACHE_FILE="${CACHE_DIR}/${CACHE_PREFIX}" fi -# If the cache for THIS SHA is fresh, print it and exit. Per-platform stat invocation differs. -if [ -f "${CACHE_FILE}" ]; then - if [ "$(uname)" = "Darwin" ]; then - cache_mtime=$(stat -f %m "${CACHE_FILE}" 2>/dev/null || echo 0) - else - cache_mtime=$(stat -c %Y "${CACHE_FILE}" 2>/dev/null || echo 0) - fi - now=$(date +%s) - age=$((now - cache_mtime)) - if [ "${age}" -lt "${CACHE_TTL_SECONDS}" ]; then - cat "${CACHE_FILE}" - exit 0 - fi -fi +# If the cache for THIS SHA is fresh, print it and exit. +read_fresh_cache "${CACHE_FILE}" # 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 @@ -100,7 +134,7 @@ write_cache_atomic() { command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed" gh auth status >/dev/null 2>&1 || fail_open "gh CLI not authenticated (run \`gh auth login\`)" -repo_slug=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null) || fail_open "gh repo view failed" +repo_slug=$(github_slug_from_origin) || fail_open "unable to determine GitHub repo from origin" [ -n "${head_sha}" ] || fail_open "git ls-remote origin main failed" @@ -129,23 +163,36 @@ checks_json=$(echo "${checks_jsonl}" | jq -s ' | map(max_by(.id)) ' 2>/dev/null) || fail_open "jq slurp failed" +required_json=$(gh api \ + "repos/${repo_slug}/branches/main/protection/required_status_checks" \ + --jq '(.contexts // []) + (.checks // [] | map(.context)) | unique' \ + 2>/dev/null || echo "null") +case "${required_json}" in + \[*\]) ;; + *) required_json="null" ;; +esac + # Aggregate counts with jq. `success`, `skipped`, `neutral` are all "passing". # Anything completed with another conclusion is a failure. Anything not yet # completed is in_progress. -summary=$(echo "${checks_json}" | jq -r ' +summary=$(echo "${checks_json}" | jq -r --argjson required_names "${required_json}" ' . as $all + | ($all | map(.name)) as $observed_names | { total: length, passed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral")))] | length, failed: [.[] | select(.status == "completed" and (.conclusion | IN("success", "skipped", "neutral") | not))], - in_progress: [.[] | select(.status != "completed")] + in_progress: [.[] | select(.status != "completed")], + missing_required: (if $required_names == null then [] else ($required_names - $observed_names) end) } | "TOTAL=\(.total)", "PASSED=\(.passed)", "FAILED_COUNT=\(.failed | length)", "IN_PROGRESS_COUNT=\(.in_progress | length)", + "MISSING_REQUIRED_COUNT=\(.missing_required | length)", (.failed[] | "FAILED_LINE=" + .name + " — " + (.conclusion // "incomplete") + " — " + (.html_url // "")), - (.in_progress[] | "INPROGRESS_LINE=" + .name + " — " + (.status // "in_progress") + " — " + (.html_url // "")) + (.in_progress[] | "INPROGRESS_LINE=" + .name + " — " + (.status // "in_progress") + " — " + (.html_url // "")), + (.missing_required[] | "MISSING_REQUIRED_LINE=" + .) ') || fail_open "jq summary failed" short_sha="${head_sha:0:8}" @@ -153,10 +200,11 @@ 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) +missing_required_count=$(echo "${summary}" | grep "^MISSING_REQUIRED_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}" +: "${failed_count:=0}" "${in_progress_count:=0}" "${missing_required_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. @@ -172,20 +220,25 @@ if [ "${total:-0}" = "0" ]; then else output=$( printf 'Main CI status (origin/main %s):\n' "${short_sha}" - printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \ - "${total}" "${passed}" "${failed_count}" "${in_progress_count}" + printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s | Required missing: %s\n' \ + "${total}" "${passed}" "${failed_count}" "${in_progress_count}" "${missing_required_count}" - if [ "${failed_count}" -gt 0 ] 2>/dev/null; then + if [ "${failed_count}" -gt 0 ]; then echo " Failures:" echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /' fi - if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + if [ "${in_progress_count}" -gt 0 ]; then echo " In progress:" echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /' fi - if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then + if [ "${missing_required_count}" -gt 0 ]; then + echo " Missing required checks:" + echo "${summary}" | grep "^MISSING_REQUIRED_LINE=" | sed 's/^MISSING_REQUIRED_LINE=/ - /' + fi + + if [ "${failed_count}" -gt 0 ] || [ "${in_progress_count}" -gt 0 ] || [ "${missing_required_count}" -gt 0 ]; then echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks" fi ) diff --git a/rakelib/release.rake b/rakelib/release.rake index 660c60a449..e1a40db440 100644 --- a/rakelib/release.rake +++ b/rakelib/release.rake @@ -373,24 +373,22 @@ def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false) return nil end - { sha: sha, check_runs: check_runs } + { sha: sha, repo_slug: repo_slug, check_runs: check_runs } end # rubocop:enable Metrics/MethodLength -def required_check_names_for_main(monorepo_root:) - repo_slug = github_repo_slug(monorepo_root) +def required_check_names_for_main(monorepo_root:, repo_slug: nil) + repo_slug ||= github_repo_slug(monorepo_root) api_path = "repos/#{repo_slug}/branches/main/protection/required_status_checks" # Combine the legacy `contexts` list (older protection rules) with the newer # `checks[].context` list. Branch protection set up via the `checks` API # leaves `contexts` as `[]`, so reading only `contexts` would yield an empty # array and trip the `:no_required_checks` abort path even when CI is green. jq_query = "(.contexts // []) + (.checks // [] | map(.context)) | unique" - # Intentional: use `capture_gh_output` (which aborts on missing `gh`) here, - # not `Open3.capture2e` directly like `fetch_main_ci_checks` does. The - # difference is deliberate — this helper's failure mode is "branch - # protection unknown" which we treat as nil/fail-safe (caller falls back - # to evaluating every check_run), so we don't need the - # `handle_main_ci_status_violation!` dry-run/override routing. + # Precondition: `fetch_main_ci_checks` already verified `gh` is installed + # before `validate_main_ci_status!` calls this helper. The remaining failure + # mode here is "branch protection unknown", which returns nil so the caller + # fail-safes to evaluating every visible check_run. output, status = capture_gh_output("api", "--jq", jq_query, api_path) # If branch protection isn't configured, isn't queryable with current token scope, or the # endpoint returns 404, fall through to nil so the caller treats all checks as required @@ -421,8 +419,12 @@ def format_main_ci_status_violation(kind:, short_sha:, runs:) # rubocop:disable when :missing_required_checks "❌ Some required CI checks are missing on origin/main (commit #{short_sha}). " \ "Branch protection would refuse this merge." - else + when :failed "❌ CI on origin/main is not healthy (commit #{short_sha})." + when :unknown_status + "❌ Check run(s) with unrecognized status on origin/main (commit #{short_sha})." + else + raise ArgumentError, "Unknown CI violation kind: #{kind.inspect}" end return header if runs.nil? || runs.empty? @@ -436,13 +438,14 @@ end def handle_main_ci_status_violation!(message:, allow_override:, dry_run:) if dry_run - puts "⚠️ DRY RUN: #{message}" + puts message.lines.map { |line| "⚠️ DRY RUN: #{line}" }.join puts "⚠️ DRY RUN: Real release would block. Use RELEASE_CI_STATUS_OVERRIDE=true to bypass." return end if allow_override - puts "⚠️ CI STATUS OVERRIDE enabled: #{message.lines.first.strip}" + puts "⚠️ CI STATUS OVERRIDE enabled — proceeding despite the following:" + puts message.lines.map { |line| " #{line}" }.join return end @@ -468,6 +471,7 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr sha = data[:sha] short_sha = sha[0, 8] + repo_slug = data[:repo_slug] check_runs = data[:check_runs] if check_runs.empty? @@ -500,10 +504,11 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr # Always query branch-protection required checks (when configured) so the # missing-required-check gate applies to both stable and prerelease. # `evaluated` then differs by mode: - # - prerelease: only the required subset (strict gate) - # - stable: every check_run on the commit (lenient — non-required - # checks like benchmarks still block on failure) - required_names = required_check_names_for_main(monorepo_root: monorepo_root) + # - prerelease: only the required subset (narrower filter; non-required failures are advisory) + # - stable: every check_run on the commit (broader filter; any failure blocks) + required_args = { monorepo_root: monorepo_root } + required_args[:repo_slug] = repo_slug if repo_slug + required_names = required_check_names_for_main(**required_args) evaluated = if is_prerelease && required_names check_runs.select { |run| required_names.include?(run["name"]) } else @@ -576,7 +581,7 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr end if unknown.any? handle_main_ci_status_violation!( - message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: unknown), + message: format_main_ci_status_violation(kind: :unknown_status, short_sha: short_sha, runs: unknown), allow_override: allow_override, dry_run: dry_run ) diff --git a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb index 1364b6de86..802720b406 100644 --- a/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb +++ b/react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb @@ -510,10 +510,15 @@ .with(monorepo_root: monorepo_root).and_return(nil) end - # Fixed sentinel default keeps test data deterministic. Tests that rely - # on `max_by { id }` ordering (rerun-dedup, cross-suite dedup) supply - # explicit ids via `.merge("id" => N)`. - def passing_run(name, id: 9999) + def next_check_run_id + @next_check_run_id ||= 999 + @next_check_run_id += 1 + end + + # Distinct defaults keep same-named helper-generated runs from collapsing + # in the nil-check_suite dedup path. Tests can still pin an id to model a + # specific GitHub payload. + def passing_run(name, id: next_check_run_id) { "id" => id, "name" => name, @@ -523,7 +528,7 @@ def passing_run(name, id: 9999) } end - def failing_run(name, conclusion: "failure", id: 9999) + def failing_run(name, conclusion: "failure", id: next_check_run_id) { "id" => id, "name" => name, @@ -533,7 +538,7 @@ def failing_run(name, conclusion: "failure", id: 9999) } end - def in_progress_run(name, id: 9999) + def in_progress_run(name, id: next_check_run_id) { "id" => id, "name" => name, @@ -695,6 +700,23 @@ def in_progress_run(name, id: 9999) end end + context "when same-named checks do not include check_suite data" do + it "keeps helper-generated runs distinct by default" 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"), failing_run("Lint")]) + + expect do + validate_main_ci_status!( + monorepo_root: monorepo_root, + is_prerelease: false, + allow_override: false, + dry_run: false + ) + end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Lint}m) + end + end + context "when a rerun and a same-named distinct-suite run exist together" do it "collapses only within a suite (same-suite rerun) and keeps cross-suite runs distinct" do suite_a_old_failed = failing_run("detect-changes").merge( @@ -751,7 +773,7 @@ def in_progress_run(name, id: 9999) allow_override: true, dry_run: false ) - end.to output(/CI STATUS OVERRIDE enabled/).to_stdout + end.to output(%r{CI STATUS OVERRIDE enabled.*Lint.*https://github.com}m).to_stdout end end @@ -768,7 +790,7 @@ def in_progress_run(name, id: 9999) allow_override: false, dry_run: true ) - end.to output(%r{DRY RUN.*CI on origin/main is not healthy}m).to_stdout + end.to output(%r{DRY RUN.*CI on origin/main is not healthy.*DRY RUN:.*Lint}m).to_stdout end end @@ -885,7 +907,7 @@ def in_progress_run(name, id: 9999) allow_override: false, dry_run: false ) - end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Future Status}m) + end.to raise_error(SystemExit, /Check run\(s\) with unrecognized status.*Future Status/m) end it "treats a nil status as a failure too" do @@ -901,9 +923,15 @@ def in_progress_run(name, id: 9999) allow_override: false, dry_run: false ) - end.to raise_error(SystemExit, %r{CI on origin/main is not healthy.*Malformed}m) + end.to raise_error(SystemExit, /Check run\(s\) with unrecognized status.*Malformed/m) end end + + it "raises when asked to format an unknown CI violation kind" do + expect do + format_main_ci_status_violation(kind: :typo, short_sha: short_sha, runs: []) + end.to raise_error(ArgumentError, /Unknown CI violation kind: :typo/) + end end describe "#fetch_main_ci_checks" do @@ -1040,6 +1068,7 @@ def in_progress_run(name, id: 9999) result = fetch_main_ci_checks(monorepo_root: monorepo_root) expect(result[:sha]).to eq("abc1234def") + expect(result[:repo_slug]).to eq("shakacode/react_on_rails") expect(result[:check_runs].length).to eq(2) expect(result[:check_runs].first["name"]).to eq("Lint") end @@ -1055,7 +1084,7 @@ def in_progress_run(name, id: 9999) allow(self).to receive(:github_repo_slug).with(monorepo_root).and_return("shakacode/react_on_rails") end - it "returns the array of required context names when branch protection is configured" do + it "returns legacy required status contexts when branch protection is configured" do allow(Open3).to receive(:capture2e) .with("gh", "api", "--jq", expected_jq, "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") @@ -1064,6 +1093,24 @@ def in_progress_run(name, id: 9999) expect(required_check_names_for_main(monorepo_root: monorepo_root)).to eq(%w[Lint Test]) end + it "returns modern required check contexts when branch protection uses checks" do + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--jq", expected_jq, + "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") + .and_return([%w[CodeQL Lint].to_json, success_status]) + + expect(required_check_names_for_main(monorepo_root: monorepo_root)).to eq(%w[CodeQL Lint]) + end + + it "returns the deduplicated union when branch protection has contexts and checks" do + allow(Open3).to receive(:capture2e) + .with("gh", "api", "--jq", expected_jq, + "repos/shakacode/react_on_rails/branches/main/protection/required_status_checks") + .and_return([%w[CodeQL Lint Test].to_json, success_status]) + + expect(required_check_names_for_main(monorepo_root: monorepo_root)).to eq(%w[CodeQL Lint Test]) + end + it "returns nil when the branch protection endpoint returns an error" do allow(Open3).to receive(:capture2e) .with("gh", "api", "--jq", expected_jq,