Skip to content

Commit ccb3a87

Browse files
justin808justin808
committed
fix(release): address PR #3407 review feedback (12 items)
Applies must-fix correctness improvements plus optional polish surfaced by cursor, codex, greptile, claude, and coderabbit bot reviews. None of these change the high-level shape of the release gate or session hooks documented in the design spec; they harden the edges. Release gate (rakelib/release.rake): - Combine .contexts + .checks[].context in the branch-protection query so newer `checks`-based protection rules don't return an empty array and trip the :no_required_checks path (cursor, greptile). - Treat an empty result from that query as nil so the caller falls back to "evaluate every check" rather than aborting (fail-safe). - Dedupe check_runs by name, keeping the highest id (= latest attempt), so an old failed run that was re-run and passed no longer blocks the release (codex P1, greptile). - Plumb dry_run/allow_override into fetch_main_ci_checks so dry-run on a machine without gh/network warns instead of aborting (codex P2). - Use capture_gh_output for all `gh api` calls so a missing gh binary surfaces the friendly install hint instead of crashing with Errno::ENOENT (coderabbit). - Check failed runs before in-progress runs. If both are present, the operator now sees the real blocker immediately instead of waiting for the in-progress run to finish before discovering the failure (claude). - Fix grammar of the success message ("(2 checks)" / "(1 required check)") and switch override instructions to `bundle exec rake` (claude, coderabbit). Session hooks (.claude/hooks/): - main-ci-status.sh: resolve origin/main HEAD via `git ls-remote origin main` instead of the latest push-workflow run's headSha so the SHA matches the release gate and doesn't lag right after a push or for paths-ignored pushes (cursor). - main-ci-status.sh: when no check runs are visible for the commit, print "no check runs visible yet" instead of `Total: 0 | ... | 0`, which an agent could misread as "all green" (cursor). - main-ci-status.sh: write the cache via mktemp + mv so a concurrent reader never sees a half-written file (claude). - main-ci-status-on-push.sh: replace substring case-globs with anchored bash regexes so `git push origin main-feature` / `maintenance` / `main-ci-fix` no longer match. Drop the redundant `--force` case (greptile P2, claude). Tests: - Add specs for the dedupe path, the failed-before-in-progress order, the dry-run fetch-failure path, the empty-array branch-protection fallback, and the missing-gh Errno::ENOENT path. - Update existing assertions for the new success-message grammar. Docs: - docs/superpowers/specs/2026-05-25-...: mark Implemented, add language identifiers to bare fenced code blocks (MD040), update override instructions to use `bundle exec rake` (coderabbit).
1 parent 666f5d3 commit ccb3a87

5 files changed

Lines changed: 267 additions & 102 deletions

File tree

‎.claude/hooks/main-ci-status-on-push.sh‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,16 @@ cmd=$(printf '%s' "${input}" | jq -r '.tool_input.command // empty' 2>/dev/null)
1414

1515
# Match:
1616
# - `gh pr create` (any args)
17-
# - `git push` to main / HEAD (with explicit origin main, or bare `git push`
18-
# while on main — we don't try to detect the current branch here, just
19-
# match the explicit-target forms).
17+
# - `git push ... origin main` or `git push ... origin HEAD` — must be
18+
# followed by whitespace or end-of-string. Without that anchor, a glob
19+
# substring match would also fire on `git push origin main-feature`,
20+
# `maintenance`, etc.
2021
matched=0
21-
case "${cmd}" in
22-
*gh\ pr\ create*) matched=1 ;;
23-
*git\ push*origin\ main*) matched=1 ;;
24-
*git\ push*origin\ HEAD*) matched=1 ;;
25-
*git\ push\ --force*main*) matched=1 ;;
26-
esac
22+
if [[ "${cmd}" =~ (^|[[:space:]])gh[[:space:]]+pr[[:space:]]+create([[:space:]]|$) ]]; then
23+
matched=1
24+
elif [[ "${cmd}" =~ (^|[[:space:]])git[[:space:]]+push([[:space:]]+[^[:space:]]+)*[[:space:]]+origin[[:space:]]+(main|HEAD)([[:space:]]|$) ]]; then
25+
matched=1
26+
fi
2727

2828
[ "${matched}" -eq 1 ] || exit 0
2929

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

Lines changed: 67 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env bash
2-
# Prints a compact CI-status block for origin/main's latest push commit.
2+
# Prints a compact CI-status block for origin/main's current HEAD commit.
33
#
44
# Designed to be wired into Claude Code's SessionStart and PreToolUse hooks
55
# (see .claude/settings.json) so the agent always sees whether main is green
@@ -39,32 +39,39 @@ fail_open() {
3939
exit 0
4040
}
4141

42+
# Helper: atomically replace the cache file with the contents of $1, then cat
43+
# the new file to stdout. A direct `tee` would leave a partial file readable
44+
# by a concurrent session-start if the script were interrupted mid-write.
45+
write_cache_atomic() {
46+
local tmp
47+
tmp=$(mktemp "${CACHE_FILE}.XXXXXX") || return 1
48+
printf '%s' "$1" >"${tmp}" || { rm -f "${tmp}"; return 1; }
49+
mv -f "${tmp}" "${CACHE_FILE}" || { rm -f "${tmp}"; return 1; }
50+
cat "${CACHE_FILE}"
51+
}
52+
4253
command -v gh >/dev/null 2>&1 || fail_open "gh CLI not installed"
4354
gh auth status >/dev/null 2>&1 || fail_open "gh CLI not authenticated (run \`gh auth login\`)"
4455

45-
# Pull the latest push run's status check rollup. `--limit 1 --event push`
46-
# isolates main pushes from PR/comment-triggered runs that we don't care about.
47-
runs_json=$(gh run list \
48-
--branch main \
49-
--limit 1 \
50-
--event push \
51-
--json conclusion,headSha,workflowName,url,status,createdAt \
52-
2>/dev/null) || fail_open "gh run list failed"
53-
54-
[ -n "${runs_json}" ] || fail_open "no main push runs visible"
55-
56-
# Resolve the head SHA from the most recent push run, then pull every check
57-
# run on that commit. We use the Checks API (not `gh run list`) because a
58-
# single push commit triggers multiple workflows and we want them aggregated.
59-
head_sha=$(echo "${runs_json}" | jq -r '.[0].headSha // empty' 2>/dev/null) || fail_open "jq parse failed"
60-
[ -n "${head_sha}" ] || fail_open "no headSha on most recent push run"
61-
6256
repo_slug=$(gh repo view --json nameWithOwner --jq .nameWithOwner 2>/dev/null) || fail_open "gh repo view failed"
6357

64-
# Use `--paginate` with `--jq '.check_runs[]'` to emit JSONL (one check_run per
65-
# line). A separate `jq -s` then slurps the JSONL back into a single array.
66-
# This avoids the gotcha where `gh --paginate` with `--jq '[...]'` produces one
67-
# array per page (concatenated), breaking single-array aggregation.
58+
# Resolve the actual `origin/main` HEAD SHA. We use `git ls-remote` (not
59+
# `gh run list`) so the SHA reflects the current ref tip, not the latest
60+
# push-workflow run — which can lag right after a push (or never appear at
61+
# all for docs-only pushes when paths-ignore filters out every workflow).
62+
# Matching the release gate's SHA semantics (`git rev-parse origin/main`)
63+
# keeps session-time and release-time observations consistent.
64+
head_sha=$(git -C "${REPO_ROOT}" ls-remote origin main 2>/dev/null | awk 'NR==1 {print $1}') \
65+
|| fail_open "git ls-remote origin main failed"
66+
[ -n "${head_sha}" ] || fail_open "could not resolve origin/main HEAD"
67+
68+
# Pull every check run on the commit. We use the Checks API because a single
69+
# push commit triggers multiple workflows and we want them aggregated.
70+
#
71+
# `--paginate` with `--jq '.check_runs[]'` emits JSONL (one check_run per line).
72+
# A separate `jq -s` slurps the JSONL back into a single array. This avoids
73+
# the gotcha where `gh --paginate` with `--jq '[...]'` produces one array per
74+
# page (concatenated), breaking single-array aggregation.
6875
checks_jsonl=$(gh api \
6976
--paginate \
7077
"repos/${repo_slug}/commits/${head_sha}/check-runs" \
@@ -93,34 +100,45 @@ summary=$(echo "${checks_json}" | jq -r '
93100
(.in_progress[] | "INPROGRESS_LINE=" + .name + " — " + (.status // "in_progress") + " — " + (.html_url // ""))
94101
') || fail_open "jq summary failed"
95102

96-
# Pull short SHA + workflow run URL for the header. Both fields come from runs_json.
97103
short_sha="${head_sha:0:8}"
98-
created_at=$(echo "${runs_json}" | jq -r '.[0].createdAt // ""')
99-
100-
# Build the output as a single string, then write it to cache + stdout.
101-
{
102-
printf 'Main CI status (origin/main %s, pushed at %s):\n' "${short_sha}" "${created_at}"
103-
total=$(echo "${summary}" | grep "^TOTAL=" | cut -d= -f2)
104-
passed=$(echo "${summary}" | grep "^PASSED=" | cut -d= -f2)
105-
failed_count=$(echo "${summary}" | grep "^FAILED_COUNT=" | cut -d= -f2)
106-
in_progress_count=$(echo "${summary}" | grep "^IN_PROGRESS_COUNT=" | cut -d= -f2)
107-
108-
printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \
109-
"${total}" "${passed}" "${failed_count}" "${in_progress_count}"
110-
111-
if [ "${failed_count}" -gt 0 ] 2>/dev/null; then
112-
echo " Failures:"
113-
echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /'
114-
fi
115-
116-
if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then
117-
echo " In progress:"
118-
echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /'
119-
fi
104+
total=$(echo "${summary}" | grep "^TOTAL=" | cut -d= -f2)
105+
passed=$(echo "${summary}" | grep "^PASSED=" | cut -d= -f2)
106+
failed_count=$(echo "${summary}" | grep "^FAILED_COUNT=" | cut -d= -f2)
107+
in_progress_count=$(echo "${summary}" | grep "^IN_PROGRESS_COUNT=" | cut -d= -f2)
108+
109+
# Build the output as a single string, then atomically swap it into the cache
110+
# so a concurrent reader never sees a half-written file.
111+
if [ "${total:-0}" = "0" ]; then
112+
# No check runs visible for this commit. The Checks API may simply not have
113+
# registered any workflows yet (right after a push), or all workflows were
114+
# filtered out by paths-ignore. Either way, the agent should NOT read this
115+
# as "all green" — say so explicitly. The release gate treats the same case
116+
# as a blocking violation; aligning the wording here keeps the two signals
117+
# honest.
118+
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' \
119+
"${short_sha}" "${repo_slug}" "${head_sha}")
120+
else
121+
output=$(
122+
printf 'Main CI status (origin/main %s):\n' "${short_sha}"
123+
printf ' Total: %s | Passed: %s | Failed: %s | In progress: %s\n' \
124+
"${total}" "${passed}" "${failed_count}" "${in_progress_count}"
125+
126+
if [ "${failed_count}" -gt 0 ] 2>/dev/null; then
127+
echo " Failures:"
128+
echo "${summary}" | grep "^FAILED_LINE=" | sed 's/^FAILED_LINE=/ - /'
129+
fi
130+
131+
if [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then
132+
echo " In progress:"
133+
echo "${summary}" | grep "^INPROGRESS_LINE=" | sed 's/^INPROGRESS_LINE=/ - /'
134+
fi
135+
136+
if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then
137+
echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks"
138+
fi
139+
)
140+
fi
120141

121-
if [ "${failed_count}" -gt 0 ] 2>/dev/null || [ "${in_progress_count}" -gt 0 ] 2>/dev/null; then
122-
echo " See: https://github.com/${repo_slug}/commit/${head_sha}/checks"
123-
fi
124-
} | tee "${CACHE_FILE}"
142+
write_cache_atomic "${output}" || fail_open "cache write failed"
125143

126144
exit 0

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Main CI release guard + continuous monitoring
22

3-
**Status**: Design — awaiting user review
3+
**Status**: Implemented (see PR #3407)
44
**Date**: 2026-05-25
55
**Owner**: Justin Gordon (with Claude Code)
66

@@ -51,11 +51,11 @@ Note: `run_release_preflight_checks!` runs _before_ `with_release_checkout` (and
5151

5252
The `:release` task gets a new 4th positional argument and a paired env var, paralleling the existing `override_version_policy`:
5353

54-
```
54+
```ruby
5555
task :release, %i[version dry_run override_version_policy override_ci_status]
5656
```
5757

58-
```
58+
```bash
5959
RELEASE_CI_STATUS_OVERRIDE=true # bypass the CI status check
6060
```
6161

@@ -77,7 +77,7 @@ The target commit is **always `origin/main` HEAD**, regardless of where the rele
7777

7878
When blocked, the error names the failing checks, links to them, and tells the operator exactly how to proceed:
7979

80-
```
80+
```text
8181
❌ CI on main is not healthy — refusing to release.
8282
8383
Commit: 3103496d
@@ -87,9 +87,9 @@ Commit: 3103496d
8787
https://github.com/shakacode/react_on_rails/actions/runs/26404417325
8888
8989
To override (use only if the failures are known-unrelated to this release):
90-
RELEASE_CI_STATUS_OVERRIDE=true rake release[...]
90+
RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake release[...]
9191
# or
92-
rake "release[16.2.0,false,false,true]"
92+
bundle exec rake "release[16.2.0,false,false,true]"
9393
```
9494

9595
In-progress checks get a separate message ("CI in progress — wait for it to finish, or override").
@@ -162,7 +162,7 @@ end
162162

163163
A small bash script that runs `gh run list --branch main --limit 1 --event push --json conclusion,headSha,workflowName,databaseId,url,status` and emits a compact status block:
164164

165-
```
165+
```text
166166
Main CI status (3103496d, pushed 7h ago):
167167
✅ 11 success
168168
❌ 2 failure:

‎rakelib/release.rake‎

Lines changed: 72 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -300,25 +300,46 @@ CI_INCOMPLETE_STATUSES = %w[in_progress queued waiting requested pending].freeze
300300
# paths-ignore skips, or workflows that intentionally short-circuit).
301301
CI_PASSING_CONCLUSIONS = %w[success skipped neutral].freeze
302302

303-
def fetch_main_ci_checks(monorepo_root:)
303+
def fetch_main_ci_checks(monorepo_root:, allow_override: false, dry_run: false)
304304
fetch_output, fetch_status = Open3.capture2e(
305305
"git", "-C", monorepo_root, "fetch", "origin", "main", "--quiet"
306306
)
307-
abort "❌ Unable to fetch origin/main for CI status check.\n\n#{fetch_output}" unless fetch_status.success?
307+
unless fetch_status.success?
308+
handle_main_ci_status_violation!(
309+
message: "❌ Unable to fetch origin/main for CI status check.\n\n#{fetch_output}",
310+
allow_override: allow_override,
311+
dry_run: dry_run
312+
)
313+
return nil
314+
end
308315

309316
sha_output, sha_status = Open3.capture2e("git", "-C", monorepo_root, "rev-parse", "origin/main")
310-
abort "❌ Unable to resolve origin/main HEAD.\n\n#{sha_output}" unless sha_status.success?
317+
unless sha_status.success?
318+
handle_main_ci_status_violation!(
319+
message: "❌ Unable to resolve origin/main HEAD.\n\n#{sha_output}",
320+
allow_override: allow_override,
321+
dry_run: dry_run
322+
)
323+
return nil
324+
end
311325
sha = sha_output.strip
312326

313327
repo_slug = github_repo_slug(monorepo_root)
314328
api_path = "repos/#{repo_slug}/commits/#{sha}/check-runs"
315329

316330
# `--paginate --jq '.check_runs[]'` flattens paginated responses into JSONL.
317-
# Each non-empty line is one check_run object.
318-
output, status = Open3.capture2e(
319-
"gh", "api", "--paginate", "--jq", ".check_runs[]", api_path
320-
)
321-
abort "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}" unless status.success?
331+
# Each non-empty line is one check_run object. Using `capture_gh_output` so a
332+
# missing `gh` binary aborts with a friendly install hint instead of crashing
333+
# with Errno::ENOENT.
334+
output, status = capture_gh_output("api", "--paginate", "--jq", ".check_runs[]", api_path)
335+
unless status.success?
336+
handle_main_ci_status_violation!(
337+
message: "❌ Unable to query GitHub Checks API for #{sha}.\n\n#{output}",
338+
allow_override: allow_override,
339+
dry_run: dry_run
340+
)
341+
return nil
342+
end
322343

323344
check_runs = output.lines.reject { |line| line.strip.empty? }.map do |line|
324345
JSON.parse(line)
@@ -332,15 +353,24 @@ end
332353
def required_check_names_for_main(monorepo_root:)
333354
repo_slug = github_repo_slug(monorepo_root)
334355
api_path = "repos/#{repo_slug}/branches/main/protection/required_status_checks"
335-
output, status = Open3.capture2e("gh", "api", "--jq", ".contexts // []", api_path)
356+
# Combine the legacy `contexts` list (older protection rules) with the newer
357+
# `checks[].context` list. Branch protection set up via the `checks` API
358+
# leaves `contexts` as `[]`, so reading only `contexts` would yield an empty
359+
# array and trip the `:no_required_checks` abort path even when CI is green.
360+
jq_query = "(.contexts // []) + (.checks // [] | map(.context)) | unique"
361+
output, status = capture_gh_output("api", "--jq", jq_query, api_path)
336362
# If branch protection isn't configured, isn't queryable with current token scope, or the
337363
# endpoint returns 404, fall through to nil so the caller treats all checks as required
338364
# (fail-safe).
339365
return nil unless status.success?
340366

341367
begin
342368
parsed = JSON.parse(output)
343-
parsed.is_a?(Array) ? parsed : nil
369+
return nil unless parsed.is_a?(Array)
370+
371+
# Empty array (no required names parseable) is treated the same as "no
372+
# branch protection visible" — fail-safe to evaluating every check run.
373+
parsed.empty? ? nil : parsed
344374
rescue JSON::ParserError
345375
nil
346376
end
@@ -384,17 +414,22 @@ def handle_main_ci_status_violation!(message:, allow_override:, dry_run:)
384414
#{message}
385415
386416
To override (use only if the failures are known-unrelated to this release):
387-
RELEASE_CI_STATUS_OVERRIDE=true rake release[...]
417+
RELEASE_CI_STATUS_OVERRIDE=true bundle exec rake release[...]
388418
# or pass override_ci_status as the 4th positional argument:
389-
rake "release[VERSION,false,false,true]"
419+
bundle exec rake "release[VERSION,false,false,true]"
390420
ERROR
391421
end
392422

393423
# rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
394424
def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dry_run:)
395425
puts "\nChecking CI status on origin/main..."
396426

397-
data = fetch_main_ci_checks(monorepo_root: monorepo_root)
427+
data = fetch_main_ci_checks(monorepo_root: monorepo_root, allow_override: allow_override, dry_run: dry_run)
428+
# `fetch_main_ci_checks` returns nil when it surfaced a violation through
429+
# `handle_main_ci_status_violation!` (dry-run or override path). In that case
430+
# the warning has already been printed and we should not continue.
431+
return if data.nil?
432+
398433
sha = data[:sha]
399434
short_sha = sha[0, 8]
400435
check_runs = data[:check_runs]
@@ -408,6 +443,15 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
408443
return
409444
end
410445

446+
# Collapse multiple runs per check name to the most recent attempt (highest
447+
# check_run id). Without this, an old failed run that was later re-run and
448+
# passed would still be picked up by the `failed` filter below and block the
449+
# release. `id` is monotonically increasing per check run on a commit, so
450+
# `max_by { id }` reliably selects the latest attempt.
451+
check_runs = check_runs
452+
.group_by { |run| run["name"] }
453+
.map { |_name, runs| runs.max_by { |run| run["id"].to_i } }
454+
411455
# For prereleases, restrict the gate to GitHub-branch-protection-required checks.
412456
# For stable releases, evaluate every check run on the commit.
413457
required_names = is_prerelease ? required_check_names_for_main(monorepo_root: monorepo_root) : nil
@@ -423,30 +467,35 @@ def validate_main_ci_status!(monorepo_root:, is_prerelease:, allow_override:, dr
423467
return
424468
end
425469

426-
in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) }
427-
if in_progress.any?
470+
# Report failures before in-progress runs. If both are present, the operator
471+
# needs to know about the failure right away — telling them to "wait or
472+
# override" would just make them wait and re-run before seeing the real
473+
# blocker.
474+
failed = evaluated.select do |run|
475+
run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"])
476+
end
477+
if failed.any?
428478
handle_main_ci_status_violation!(
429-
message: format_main_ci_status_violation(kind: :in_progress, short_sha: short_sha, runs: in_progress),
479+
message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed),
430480
allow_override: allow_override,
431481
dry_run: dry_run
432482
)
433483
return
434484
end
435485

436-
failed = evaluated.select do |run|
437-
run["status"] == "completed" && !CI_PASSING_CONCLUSIONS.include?(run["conclusion"])
438-
end
439-
if failed.any?
486+
in_progress = evaluated.select { |run| CI_INCOMPLETE_STATUSES.include?(run["status"]) }
487+
if in_progress.any?
440488
handle_main_ci_status_violation!(
441-
message: format_main_ci_status_violation(kind: :failed, short_sha: short_sha, runs: failed),
489+
message: format_main_ci_status_violation(kind: :in_progress, short_sha: short_sha, runs: in_progress),
442490
allow_override: allow_override,
443491
dry_run: dry_run
444492
)
445493
return
446494
end
447495

448-
scope = required_names.nil? ? "all" : "required"
449-
puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{scope} checks)"
496+
qualifier = required_names.nil? ? "" : "required "
497+
noun = evaluated.length == 1 ? "check" : "checks"
498+
puts "✓ Main CI is healthy on #{short_sha} (#{evaluated.length} #{qualifier}#{noun})"
450499
end
451500
# rubocop:enable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/MethodLength, Metrics/PerceivedComplexity
452501

0 commit comments

Comments
 (0)