Refuse releases on red main and surface main CI status

[justin808]

Summary

This PR adds guardrails so releases and Claude Code sessions notice when main CI is unhealthy.

1. Release-time gate in rake release: checks CI on origin/main HEAD before publishing. Stable releases require every visible check to pass; prereleases gate on branch-protection-required checks, with fail-safe fallback to all checks when required checks cannot be read. Failed, in-progress, missing-required, unknown-status, and no-checks states block unless explicitly overridden.

2. Session-time signal for Claude Code: hooks print a compact, cached origin/main CI summary at session start and before PR creation or push-to-main commands. The hooks are fail-open, SHA-keyed, origin-derived, and surface missing required checks as well as failed/in-progress checks.

3. Agent guidance and design notes: AGENTS.md, CLAUDE.md, and the design spec document when to stop, wait, override, or proceed when main is red.

Review Follow-Up Included

• Rebased onto current main; the previous CI install failure came from the old merge base and is fixed upstream by the OpenTelemetry syntax fix now included here.
• Expanded push detection for HEAD:main, HEAD:refs/heads/main, fully qualified main refspecs, git -C ... push, --all, and --mirror.
• Changed the session hook to derive the repo slug from origin, matching the release gate.
• Added a warm-cache path that avoids a live ls-remote call when local origin/main already has a fresh SHA-keyed cache.
• Added missing-required-check reporting to the session hook.
• Made release override and dry-run output include the full diagnostic, including failing check names and URLs.
• Added explicit unknown-status diagnostics and explicit unknown-kind protection.
• Avoided the duplicate repo-slug lookup between check-run fetch and required-check discovery.
• Added/updated specs for the new diagnostic, required-check, and nil-check_suite behavior.

Test Plan

• bundle exec rspec react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb (74 examples, 0 failures)
• bundle exec rubocop rakelib/release.rake react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb
• (cd react_on_rails && bundle exec rubocop)
• bash -n .claude/hooks/main-ci-status.sh && bash -n .claude/hooks/main-ci-status-on-push.sh
• shellcheck .claude/hooks/main-ci-status.sh .claude/hooks/main-ci-status-on-push.sh
• pnpm install --frozen-lockfile (confirms the previous Pro node-renderer prepare failure is gone after rebase)
• Remote CI after force-with-lease push of the rebased branch

Follow-Ups

• fix(tanstack-router): loadRouteChunk called 2× under StrictMode hydration replay #3405 — fix(tanstack-router): loadRouteChunk called twice under StrictMode hydration replay
• fix(ci): Benchmark Workflow fails — bench.rb doesn't produce summary, port 3001 never frees #3406 — fix(ci): Benchmark Workflow fails because bench.rb does not produce a summary and port 3001 is not freed

---

Note

Medium Risk
Changes release publishing behavior (can block releases) and adds GitHub API-dependent hooks; mistakes in CI policy or hook matching could block legitimate releases or add noise, but dry-run/override paths and fail-open hooks limit blast radius.

Overview
Adds main-branch CI guardrails so releases and Claude Code sessions see when origin/main is unhealthy before shipping or opening PRs.

rake release now evaluates GitHub Checks on origin/main HEAD (after git pull --rebase in the release checkout, including dry runs). Stable releases require every visible check to pass; prereleases gate on branch-protection-required checks, with fail-safe “treat all as required” when protection cannot be read. Failed, in-progress, missing-required, unknown-status, and zero-check cases block unless RELEASE_CI_STATUS_OVERRIDE or the new 4th positional arg is set. Logic mirrors the hook (dedup by check_suite + name, combined contexts/checks for required names).

Claude Code hooks inject a compact, SHA-keyed 5-minute cache of main CI at SessionStart and before gh pr create / push-to-main (broad git push detection, fail-open). AGENTS.md, CLAUDE.md, .gitignore, and a design spec document the policy; release_rake_helpers_spec.rb covers the new release and fetch paths extensively.

^{Reviewed by Cursor Bugbot for commit 3076a51. Bugbot is set up for automated code reviews on this repo. Configure here.}

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Adds a release-time gate that validates origin/main GitHub check runs (with override/dry-run support) and session-time Bash hooks that display cached origin/main CI status before PR creation or pushes; includes Rake wiring, RSpec coverage, docs, and a 5-minute fail-open cache.

Changes

CI Status Guard: Release Gate and Monitoring

Layer / File(s)	Summary
Hook registration and gitignore .claude/settings.json, .gitignore	Registers SessionStart and PreToolUse hooks and ignores .claude/.main-ci-status.cache.
Command interceptor wrapper .claude/hooks/main-ci-status-on-push.sh	Wrapper parses Claude PreToolUse JSON to detect gh pr create and git push to origin main/HEAD, fail-opens on parse/tool errors, and execs the main status hook when matched.
Main CI status monitoring bash script .claude/hooks/main-ci-status.sh	Resolves origin/main SHA, uses a 5‑minute SHA-keyed cache with atomic writes, queries GitHub Checks API (paginated), deduplicates runs, aggregates pass/failed/in-progress counts via jq, formats a multi-line rollup, and fail-opens on tooling/API errors.
Release gate helper functions rakelib/release.rake	Adds CI override detection and constants; implements fetch_main_ci_checks to fetch and parse origin/main check runs and route errors to the violation handler.
Required-check discovery and violation formatting rakelib/release.rake	Implements required_check_names_for_main, format_main_ci_status_violation, and handle_main_ci_status_violation!.
Validate main CI status and rake integration rakelib/release.rake	Implements validate_main_ci_status! (latest-attempt collapse, prerelease required-check filtering, missing-check blocking, failure-before-in-progress rules), updates rake release signature/docs to accept override_ci_status, and wires validation into the release flow.
RSpec coverage for release gate helpers react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb	Adds extensive tests for override detection, fetch_main_ci_checks, required_check_names_for_main, and validate_main_ci_status! covering success, failure, in-progress, zero-run, reruns, override/dry-run, and branch-protection fallback cases.
Design doc, agent guidance, and CLAUDE integration docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md, AGENTS.md, CLAUDE.md	Design doc records release gate and session-monitoring behavior and follow-ups; AGENTS.md adds main-branch health guidance and override rules; CLAUDE.md adds session checks instruction.

Sequence Diagram(s)

sequenceDiagram
  participant ClaudeSession as Claude Session
  participant Wrapper as .claude/hooks/main-ci-status-on-push.sh
  participant Hook as .claude/hooks/main-ci-status.sh
  participant CacheFile as .claude/.main-ci-status.cache
  participant ghCLI as gh CLI
  participant ChecksAPI as GitHub Checks API

  ClaudeSession->>Wrapper: PreToolUse JSON (stdin)
  Wrapper->>Wrapper: extract .tool_input.command (jq)
  Wrapper->>Wrapper: match gh pr create / git push origin main/HEAD
  alt match
    Wrapper->>Hook: exec main-ci-status.sh
    Hook->>CacheFile: check TTL (stat)
    alt cache hit
      CacheFile-->>Hook: cached output
      Hook-->>ClaudeSession: print cached status
    else cache miss
      Hook->>ghCLI: resolve repo / git ls-remote origin main
      ghCLI-->>Hook: repo slug / head_sha
      Hook->>ChecksAPI: gh api --paginate /repos/:owner/:repo/commits/:sha/check-runs
      ChecksAPI-->>Hook: paged JSONL check-runs
      Hook->>Hook: dedupe & aggregate with jq
      Hook->>CacheFile: write atomically (tmp + mv)
      Hook-->>ClaudeSession: print status
    end
  else no match
    Wrapper-->>ClaudeSession: exit silently (0)
  end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

• Harden release gate against app_id collisions on required-check names #3413 — Related: both involve CI-status fetching/deduplication and may need coordinated check-run selection logic (e.g., app_id-aware selection).

Poem

🐰 I nibble logs and watch the main,
A five‑minute cache to ease the strain,
Before a push or PR takes flight,
I whisper if the checks are bright —
Fail open, gentle, keeping trust in chain.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the two main changes: refusing releases when main CI is red and surfacing main CI status to Claude sessions.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-conductor/main-ci-release-guard

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 666f5d32fe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[greptile-apps]

Greptile Summary

This PR adds two guardrails against releasing over a broken main: a rake release CI gate that fetches origin/main and aborts when checks are unhealthy, and a set of Claude Code hooks that inject main CI status at session start and before gh pr create / git push origin main. Documentation and tests are thorough.

• Release gate (rakelib/release.rake): fetches origin/main, queries the GitHub Checks API, applies stable-vs-pre-release policy (all checks vs. branch-protection-required only), and aborts with a structured error; override via env var or 4th positional arg mirrors the existing version-policy pattern.
• Session hooks (.claude/hooks/): fail-open bash scripts with a 5-minute cache; SessionStart prints CI status unconditionally, PreToolUse fires only before push/PR-create commands.
• Docs + tests: AGENTS.md gets a decision-framework section, CLAUDE.md a one-line pointer, and the spec file gains 16 examples covering all documented policy paths.

Confidence Score: 4/5

Safe to merge; the release gate and session hooks work correctly for the common case and all error paths are fail-open or have a documented override.

The core logic is well-tested and the fail-open design prevents hooks from breaking sessions. The two issues in release.rake — the deprecated contexts field potentially returning [] instead of nil for newer branch-protection setups, and the check-runs endpoint returning stale historical runs after a re-run — could cause false-positive blocks, but both have a clear documented escape hatch via RELEASE_CI_STATUS_OVERRIDE=true. The shell hook substring match on origin main is harmless extra output. None of these affect correctness of the shipped release artifacts.

rakelib/release.rake — the branch-protection query and check-run deduplication logic deserve a second look before the next release cycle.

Important Files Changed

Filename	Overview
rakelib/release.rake	Adds CI gate (fetch + check-runs API + policy logic) to the release task; logic is solid but queries the deprecated contexts field for branch protection and does not deduplicate historical check runs when a job has been re-run.
.claude/hooks/main-ci-status-on-push.sh	PreToolUse hook that fires before push/PR-create; the glob pattern *origin\ main* is a substring match and will also trigger on branches named main-*.
.claude/hooks/main-ci-status.sh	Well-structured fail-open script; caching, auth checks, jq pagination, and error paths all handled correctly.
react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb	16 new examples covering all documented policy paths (stable, pre-release, override, dry-run, branch-protection fallback, edge cases); no gaps found.
.claude/settings.json	Adds SessionStart and PreToolUse hooks correctly; existing PostToolUse hook preserved.
AGENTS.md	New 'Main branch health' section clearly documents the gate, override mechanism, and expected agent decision framework.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[rake release called] --> B[resolve version]
    B --> C{stable branch check}
    C -- fail --> ABORT1[abort: must run from main]
    C -- pass --> D[validate_main_ci_status!]
    D --> E[git fetch origin main]
    E --> F[gh api check-runs for HEAD SHA]
    F --> G{check_runs empty?}
    G -- yes --> H{override or dry-run?}
    G -- no --> I{is_prerelease?}
    I -- yes --> J[required_check_names_for_main]
    J -- nil fallback --> K[evaluate ALL checks]
    J -- names list --> L[filter to required checks only]
    I -- no --> K
    K --> M{any in_progress?}
    L --> M
    M -- yes --> H
    M -- no --> N{any failed?}
    N -- yes --> H
    N -- no --> SUCCESS[CI healthy - continue release]
    H -- dry-run --> WARN[DRY RUN warning, continue]
    H -- override=true --> WARN2[OVERRIDE warning, continue]
    H -- no override --> ABORT2[abort with failing check URLs]

Loading

Comments Outside Diff (2)

1. rakelib/release.rake, line 743-744 (link)

   Deprecated contexts field may return an empty array with newer branch protection

   --jq ".contexts // []" falls back to [] only when .contexts is null or absent — not when it's an empty array. If the repository's branch protection was set up using the newer checks-based API (where contexts is [] and checks contains the real required names), this function returns [] (a non-nil value). Back in validate_main_ci_status!, required_names = [] causes evaluated to be empty, which triggers the :no_required_checks abort for every pre-release regardless of CI state. Consider also reading .checks[].context as a fallback: --jq "(.contexts // []) + (.checks // [] | map(.context)) | unique".

2. rakelib/release.rake, line 726-736 (link)

   Re-run scenario: stale failed check runs block release after a valid fix

   /commits/{sha}/check-runs returns all historical check runs for a commit, including superseded ones. If a check failed, was re-run (without a new push), and the re-run succeeded, both the old failed and new passed runs appear in the response. The failed filter below will still pick up the old failure and abort the release. Operators would need RELEASE_CI_STATUS_OVERRIDE=true even though CI is actually green. Filtering to only the latest run per check name (e.g. by id descending, or by deduplicating on name keeping the most recent) would eliminate this false positive.

_{Reviews (1): Last reviewed commit: "feat(claude): surface main CI status at ..." | Re-trigger Greptile}

[claude]

Code Review

This PR adds two well-motivated guardrails against releasing over a broken main: a rake release CI gate and a Claude Code session hook that surfaces main CI status at session start and before pushes/PRs. The design is solid and the implementation is generally clean.

What works well

• Fail-open everywhere: every tooling failure in the hook scripts exits 0 without blocking; the release gate propagates failures rather than swallowing them.
• 5-minute cache avoids hammering the GitHub API on rapid session restarts.
• override_ci_status / RELEASE_CI_STATUS_OVERRIDE mirror the existing override_version_policy pattern exactly — easy to discover and use.
• Prerelease fallback: when branch protection is not queryable, treating all checks as required is the correct fail-safe.
• 16 new test cases cover every meaningful branch including override/dry-run paths and the branch-protection fallback.

Issues

Medium: in_progress checked before failures may mislead the operator — see inline comment on rakelib/release.rake.

When origin/main has both already-failed and still-in-progress checks, the current ordering reports only "CI is in progress — wait or override." The operator waits, re-runs rake release, and only then sees the failures. Checking failed first (or reporting both together) gives the full picture in one shot.

Minor: redundant --force case in main-ci-status-on-push.sh — see inline comment.

The fourth case pattern is fully covered by the second pattern. It also widens the false-positive surface by omitting origin. Dropping it tightens the match without losing intended coverage.

Minor nits

• "(1 all checks)" in the success message is grammatically awkward; a simple length == 1 ternary fixes it.
• The design spec file still says "Status: Design - awaiting user review"; worth updating to Implemented.
• The tee cache write is not atomic. A mktemp + mv pattern would prevent a partial write from being served as a cached result, though this is very unlikely in practice.

Summary

Well-structured and ready to merge with small adjustments. The in_progress/failed ordering is the only issue worth fixing before merge; the rest are low-stakes polish. The fail-open design means the worst-case outcome of any future hook bug is a missing status line, not a blocked session.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md`:
- Around line 54-56: The fenced code blocks around the release task and examples
lack language identifiers; update the blocks shown (the one containing "task
:release, %i[version dry_run override_version_policy override_ci_status]" -> add
ruby, the block with "RELEASE_CI_STATUS_OVERRIDE=true   # bypass the CI status
check" -> add bash, and the example output blocks containing "❌ CI on main is
not healthy — refusing to release." and "Main CI status (3103496d, pushed 7h
ago):" -> add text) so each fenced block has an appropriate language tag (e.g.,
```ruby, ```bash, ```text) for lint MD040 and correct rendering.

In `@rakelib/release.rake`:
- Around line 386-389: Update the user-facing release command examples to use
bundle exec by replacing references to "rake" with "bundle exec rake" in the
override instructions; specifically change the examples that read
'RELEASE_CI_STATUS_OVERRIDE=true rake release[...]' and 'rake
"release[VERSION,false,false,true]"' so they become
'RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake ...' and 'bundle exec rake
"release[VERSION,false,false,true]"' to ensure consistency with the repo's Ruby
workflow.
- Around line 318-326: The gh API invocations in fetch_main_ci_checks /
required_check_names_for_main currently call Open3.capture2e directly and can
raise Errno::ENOENT when the gh CLI is missing; replace those Open3.capture2e
calls that run "gh api --paginate --jq '.check_runs[]' #{api_path}" with the
existing helper capture_gh_output so the friendly abort path is used (keep the
current parsing logic that maps lines to JSON and the rescue that aborts on
JSON::ParserError; do not change to --slurp/page flattening). Ensure you call
capture_gh_output with the same gh arguments and preserve the subsequent
status/output handling and abort messages.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bad39209-4302-4f7a-b7f6-43fe37ceebe5

📥 Commits

Reviewing files that changed from the base of the PR and between 3103496 and 666f5d3.

📒 Files selected for processing (9)

• .claude/hooks/main-ci-status-on-push.sh
• .claude/hooks/main-ci-status.sh
• .claude/settings.json
• .gitignore
• AGENTS.md
• CLAUDE.md
• docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md
• rakelib/release.rake
• react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb

[justin808]

Thanks for the review — addressed the inline items plus the three polish nits from your review summary in commit ccb3a87.

From the summary body:

• (N all checks) grammar — the success message now pluralizes correctly: (1 check) / (2 checks) / (1 required check) / (2 required checks). The word "all" was redundant once the counter was right, so dropped. Spec assertions updated.
• Design spec status — flipped from Design — awaiting user review to Implemented (see PR #3407) in docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md.
• Atomic cache write — .claude/hooks/main-ci-status.sh now writes through mktemp + mv rather than tee, so a concurrent session-start can never see a half-written file.

And the two inline must-fixes from your review: the failed-before-in_progress order swap and dropping the redundant 4th --force case in main-ci-status-on-push.sh.

Full per-comment replies are on the individual threads.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ccb3a87852

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Code Review

Overall: Well-designed and carefully implemented. Fail-open throughout, good test coverage, clear operator UX. A few minor issues worth addressing before merge.

---

What's Good

• Fail-open design is consistent — the shell hook, the cache write path, and the Ruby gate all degrade gracefully to "warn and continue" rather than breaking sessions or releases on tooling failures.
• 16 new tests cover all documented edge cases including the dedup/rerun path, dry-run, override, branch-protection fallback, and the no-checks case.
• Atomic cache writes via temp-file + mv prevent concurrent session starts from reading a half-written cache.
• Deduplication of rerun checks (latest attempt wins by id) is correct and tested.
• Differentiated policy (stable = all checks, pre-release = required only) with safe fallback when branch protection isn't queryable.

---

Issues

Minor: write_cache_atomic output path (shell hook)

The last line of write_cache_atomic is cat "${CACHE_FILE}", which re-reads the file after the mv. If a concurrent process deletes the file in that window, the cat fails, the function returns non-zero, and fail_open "cache write failed" fires — even though the cache was written successfully. The message is misleading and the next caller will just read the (now-fresh) cache anyway.

Simple fix: replace cat "${CACHE_FILE}" with printf '%s' "$1" since the content is already in the parameter.

Minor: Arithmetic comparisons with potentially-empty variables

Lines 171, 176, 181 in main-ci-status.sh do [ "${failed_count}" -gt 0 ] 2>/dev/null. The 2>/dev/null is intentional (fail-open) but if failed_count or in_progress_count is empty due to partial jq output, the comparison silently returns false and the failure/in-progress section is suppressed. The total=0 guard at line 156 covers the all-empty case, but a : "${failed_count:=0}" / : "${in_progress_count:=0}" assignment right after the parse block would eliminate the ambiguity without changing fail-open behavior.

Minor: Test helpers missing id field

passing_run, failing_run, and in_progress_run helpers don't include an id field. The deduplication code uses run["id"].to_i (nil to 0), which happens to work for tests with a single run per name, but the helpers being id-free is slightly inconsistent with real GitHub API responses. The dedup test correctly adds id fields explicitly — worth adding a sequential default to the base helpers for completeness.

Minor: Missing test — allow_override: true on git-fetch failure

There's a test for the dry-run path when git fetch fails (prints DRY RUN warning, returns nil). There's no symmetric test for allow_override: true when git fetch fails (should print override warning, return nil). The code path is the same handle_main_ci_status_violation! call but an explicit test would give complete coverage of the override branch in fetch_main_ci_checks.

Observation: Design spec diverges from implementation

docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md says validate_main_ci_status! runs inside run_release_preflight_checks!, but the actual implementation places it inside with_release_checkout (after git pull --rebase, once is_prerelease is known). The actual placement is more correct. The spec is already stale at merge time.

Also worth noting: run_release_preflight_checks! exits early on dry_run: true (line 151), so it skips npm/gh auth in dry runs. The CI check is NOT in that function, so it correctly runs in dry-run mode and prints the warning. This is right but non-obvious — a brief comment in run_release_preflight_checks! pointing to the separate CI check would help future readers.

---

Verdict

No blockers. The three minor code issues (cache output, empty-variable guards, test helper completeness) are low-risk. The missing override test is a nice-to-have. Mergeable after consideration.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rakelib/release.rake`:
- Around line 356-361: The current jq_query flattens required checks to bare
context strings losing app identity; change the query so it returns objects of
the form {context, app_id} (and convert legacy `contexts` strings to {context:
<string>, app_id: null}) when calling capture_gh_output, and then update the
later dedupe/matching (the code that currently collapses by run["name"] around
the dedupe block) to key and compare by both context and app_id (e.g. a
composite key like "#{context}|#{app_id}" or equivalent) so required checks are
matched and deduped only when both context and app_id match (preserving legacy
behavior when app_id is null).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ba505e48-4a0d-4f91-a6bf-87d2115f1c0f

📥 Commits

Reviewing files that changed from the base of the PR and between 666f5d3 and ccb3a87.

📒 Files selected for processing (5)

• .claude/hooks/main-ci-status-on-push.sh
• .claude/hooks/main-ci-status.sh
• docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md
• rakelib/release.rake
• react_on_rails/spec/react_on_rails/release_rake_helpers_spec.rb

✅ Files skipped from review due to trivial changes (1)

• docs/superpowers/specs/2026-05-25-main-ci-release-guard-design.md

[justin808]

Round-2 review feedback — addressed (commit d620c631f)

Addressed 7 of the items from this round and resolved the corresponding threads. Verified locally: 63 examples, 0 failures, RuboCop clean, ShellCheck clean.

Resolved inline

Source	Where	What changed
@chatgpt-codex-connector P1	rakelib/release.rake	validate_main_ci_status! now flags partial-missing required checks (:missing_required_checks) — branch protection would refuse the merge, so a release shouldn't ship through it. The all-missing case keeps :no_required_checks for clarity. New regression test included.
@cursor (Bugbot)	.claude/hooks/main-ci-status.sh	jq slurp now dedupes reruns by max_by(.id), mirroring the Ruby gate at release.rake:451-453. Session-time and release-time CI views now stay in sync when a check has been re-run.
@claude (×2 inline)	.claude/hooks/main-ci-status.sh:50	write_cache_atomic prints from $1 instead of re-reading via cat — closes a TOCTOU window that produced a misleading fail_open "cache write failed".
@claude	.claude/hooks/main-ci-status.sh	Defensive : "${failed_count:=0}" "${in_progress_count:=0}" guard before the -gt 0 comparisons.
@claude	release_rake_helpers_spec.rb	passing_run / failing_run / in_progress_run helpers now include id: defaults to match real GitHub API responses.

From @claude's review summary

• ✅ Missing test — allow_override: true on git fetch failure. Added a symmetric test to the existing dry-run+git-fetch-failure test.
• ✅ Design spec ↔ implementation divergence note. Added a brief comment in run_release_preflight_checks! explaining why the main-CI status check is wired separately inside with_release_checkout (so it still fires under dry_run: true). The spec itself is already labeled stale at merge time.

Acknowledged (no separate action)

• @chatgpt-codex-connector review summary: boilerplate-only review wrapper — no actionable body. Inline P1 + P2 addressed/deferred as noted.
• @coderabbitai review summary: "Actionable comments posted: 1" pointing at the inline app_id finding tracked under the deferred item below.

Deferred (pending review decision) — app_id matching for required checks

The Codex P2 / CodeRabbit duplicate finding is technically correct: required_status_checks.checks[].app_id lets branch protection pin a required check to a specific GitHub App, and the current code flattens that information away.

Why deferred rather than fixed in this PR:

• Practical risk in this repo is essentially zero. Required CI checks come from github-actions[bot]. Other bots in this repo (CodeRabbit, Claude, Codex, Cursor) emit reviews/checks with distinct names (CodeRabbit, Codex review, etc.). For a collision to fire the gap, an unrelated app would have to emit a check whose name matches a required CI workflow.
• Fix is non-trivial. It touches the branch-protection jq query (return [{context, app_id}] instead of [string]), the Checks API projection in fetch_main_ci_checks (carry app.id), the dedup key (compose [name, app_id]), and the matching predicate (with app_id == nil and app_id == -1 wildcards). Plus new tests for each branch.

Happy to fold it in here if you'd rather, or open a follow-up issue (Harden release gate against app_id collisions on required-check names) — let me know which you prefer and I'll close out the two threads with the chosen disposition.

Posted by I (Claude Code).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d620c631ff

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

@justin808 — all addressed items look great, thank you for the thorough round-up.

On the deferred app_id item: open a follow-up issue. Your reasoning is exactly right:

• The collision scenario requires a non-github-actions bot to emit a check whose name exactly matches a required workflow check — not a real risk in this repo today.
• The fix is genuinely multi-touch (branch-protection query, fetch_main_ci_checks projection, dedup key, matching predicate, test matrix), and doing it half-heartedly in a rush would be worse than the gap itself.
• A well-labelled issue keeps the contract visible without blocking this otherwise solid, well-tested PR.

If you'd like, I can open the issue now titled "Harden release gate against app_id collisions on required-check names" with a backlink to this PR and the two relevant threads (#3300616449 / #3300628005) as context. Just say the word and I'll run the script.

With that settled, this PR looks good to me — all the substantive concerns from both rounds are resolved. 🐇 ✨

✅ Actions performed

Comments resolved and changes approved.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.}

^{Reviewed by Cursor Bugbot for commit 3076a51. Configure here.}

[cursor]

Wrapped bash commands skip hook

Medium Severity

PreToolUse matching requires gh or git at the start of the command string or after whitespace. Invocations such as bash -c "git push origin main" or bash -c "gh pr create" place the tool name immediately after a quote, so the hook exits without emitting main CI status.

 

^{Reviewed by Cursor Bugbot for commit 3076a51. Configure here.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3076a5139a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Refresh remote SHA before trusting cached status

When a fresh cache exists for the local origin/main, the hook exits here before git ls-remote checks the actual remote tip. If another commit lands on main within the 5-minute TTL after this checkout last updated origin/main, session start / pre-PR / pre-push output can report the old cached green commit while current main is red, undermining the guardrail this hook adds. Consider resolving the remote SHA first, then using the SHA-keyed cache for that current tip.

Useful? React with 👍 / 👎.