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..2d0f3cc6df --- /dev/null +++ b/.claude/hooks/main-ci-status-on-push.sh @@ -0,0 +1,59 @@ +#!/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) +# - 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. +# +# 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:]]+[^[: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 "") + if [ "${current_branch}" = "main" ]; then + matched=1 + fi +fi + +[ "${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..02842b0702 --- /dev/null +++ b/.claude/hooks/main-ci-status.sh @@ -0,0 +1,255 @@ +#!/usr/bin/env bash +# 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 +# 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_DIR="${REPO_ROOT}/.claude" +CACHE_PREFIX=".main-ci-status.cache" +CACHE_TTL_SECONDS=300 + +# Helper: print an "unavailable" message and exit 0 without writing the cache. +fail_open() { + echo "Main CI status unavailable: $1" + exit 0 +} + +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. +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}" +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. +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 +# 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 + # 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\n' "$1" +} + +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=$(github_slug_from_origin) || fail_open "unable to determine GitHub repo from origin" + +[ -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. +# +# `--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" \ + --jq '.check_runs[]' \ + 2>/dev/null) || fail_open "gh api check-runs failed" + +# 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 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]) + | 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 --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")], + 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 // "")), + (.missing_required[] | "MISSING_REQUIRED_LINE=" + .) +') || fail_open "jq summary failed" + +short_sha="${head_sha:0:8}" +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}" "${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. +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 | Required missing: %s\n' \ + "${total}" "${passed}" "${failed_count}" "${in_progress_count}" "${missing_required_count}" + + if [ "${failed_count}" -gt 0 ]; then + echo " Failures:" + echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /' + fi + + if [ "${in_progress_count}" -gt 0 ]; then + echo " In progress:" + echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /' + fi + + 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 + ) +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. 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 5b3ffee9c4..c0efdfd24c 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,5 +1,26 @@ { "hooks": { + "SessionStart": [ + { + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/main-ci-status.sh" + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "${CLAUDE_PROJECT_DIR}/.claude/hooks/main-ci-status-on-push.sh" + } + ] + } + ], "PostToolUse": [ { "matcher": "Edit|Write", diff --git a/.gitignore b/.gitignore index 75849aca0c..3b98422a11 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 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..6727113c12 --- /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**: Implemented (see PR #3407) +**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`: + +```ruby +task :release, %i[version dry_run override_version_policy override_ci_status] +``` + +```bash +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: + +```text +❌ 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 bundle exec rake release[...] + # or + 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"). + +### 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 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): + ✅ 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. diff --git a/rakelib/release.rake b/rakelib/release.rake index 76950c285b..e1a40db440 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}" @@ -289,6 +293,312 @@ 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 + +# 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" + ) + 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") + 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. 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}", + allow_override: allow_override, + dry_run: dry_run + ) + return nil + end + + begin + check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line| + JSON.parse(line) + end + rescue JSON::ParserError => e + 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, repo_slug: repo_slug, check_runs: check_runs } +end +# rubocop:enable Metrics/MethodLength + +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" + # 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 + # (fail-safe). + return nil unless status.success? + + begin + parsed = JSON.parse(output) + 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 +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})." + when :missing_required_checks + "❌ Some required CI checks are missing on origin/main (commit #{short_sha}). " \ + "Branch protection would refuse this merge." + 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? + + 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 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 — proceeding despite the following:" + puts message.lines.map { |line| " #{line}" }.join + return + end + + abort <<~ERROR + #{message} + + To override (use only if the failures are known-unrelated to this release): + RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake release[...] + # or pass override_ci_status as the 4th positional argument: + bundle exec 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, 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] + repo_slug = data[:repo_slug] + 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 + + # 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. + # 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["id"], run["name"]] } + .map { |_key, runs| runs.max_by { |run| run["id"].to_i } } + + # 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 (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 + check_runs + end + + # When branch protection lists required checks, treat any missing required + # 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? + 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) + + "\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 + # 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: :failed, short_sha: short_sha, runs: failed), + 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 + + # 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: :unknown_status, short_sha: short_sha, runs: unknown), + allow_override: allow_override, + dry_run: dry_run + ) + return + end + + # 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 +# 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 +1169,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 +1197,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 +1205,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 +1254,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..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 @@ -475,4 +475,662 @@ 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" } + + # `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 + + 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, + "status" => "completed", + "conclusion" => "success", + "html_url" => "https://github.com/shakacode/react_on_rails/runs/#{name.gsub(/\W/, '_')}" + } + end + + def failing_run(name, conclusion: "failure", id: next_check_run_id) + { + "id" => id, + "name" => name, + "status" => "completed", + "conclusion" => conclusion, + "html_url" => "https://github.com/shakacode/react_on_rails/runs/#{name.gsub(/\W/, '_')}" + } + end + + def in_progress_run(name, id: next_check_run_id) + { + "id" => id, + "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, allow_override: false, dry_run: false) + .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 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, allow_override: false, dry_run: false) + .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, 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"]) + + 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 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, 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"]) + + 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, allow_override: false, dry_run: false) + .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 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 + # 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]) + + 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 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 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( + "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) + .with(monorepo_root: monorepo_root, allow_override: false, dry_run: false) + .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, allow_override: true, dry_run: false) + .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(%r{CI STATUS OVERRIDE enabled.*Lint.*https://github.com}m).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, allow_override: false, dry_run: true) + .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.*DRY RUN:.*Lint}m).to_stdout + 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, 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) + + 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, 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"]) + + 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 + + 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 + + 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) + 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, /Check run\(s\) with unrecognized status.*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, /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 + 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 "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") + .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") + .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 "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 "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") + .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[: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 + 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) } + 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") + end + + 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") + .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 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, + "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