Skip to content

fix(release): gate the release train on CI, not a required perry/review #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LukasParke merged 1 commit into from
Aug 10, 2026
+17 −32
Merged

fix(release): gate the release train on CI, not a required perry/review #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 15 additions & 30 deletions .github/scripts/pr-gate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
#!/usr/bin/env bash
#
# pr-gate.sh — poll a bump PR until Perry + CI reach a terminal state, then
# pr-gate.sh — poll a bump PR until CI reaches a terminal state, then
# squash-merge it (when AUTO_MERGE=true) or alert and leave it red.
#
# The gate is CI: every check on the PR (lint, tests, etc.) must be green,
# none pending, and no CHANGES_REQUESTED review decision. A check posted by
# an AI reviewer (perry/review, Devin Review, ...) counts like any other
# check WHEN it appears — a red one blocks the merge — but no reviewer is
# *required* to show up: this repo's PRs are gated on CI, and a required
# reviewer that never runs (perry/review has never posted here) would stall
# every train run at "waiting for perry/review".
#
# This is the self-gating auto-merge: GitHub-native `gh pr merge --auto` cannot
# be relied on because the repo has no required status checks, so we poll the
# verdict ourselves. The verdict mirrors ~/.claude/skills/get-pr-reviewed's
Expand Down Expand Up @@ -66,7 +74,6 @@ TIMEOUT="${TIMEOUT:-1800}" # 30 min per vetted head (resets on head adoptio
# the failure mode turns confusing. Default 50 min leaves headroom to alert
# cleanly while the token still works.
MAX_WALL="${MAX_WALL:-3000}"
PERRY_TIMEOUT="${PERRY_TIMEOUT:-480}" # 8 min for perry/review to appear at all
SETTLE="${SETTLE:-45}"

AI_REVIEWERS='perry/review|Devin Review|Graphite / AI Reviews|codex|claude'
Expand Down Expand Up @@ -146,18 +153,13 @@ FAIL = {"FAILURE","ERROR","CANCELLED","TIMED_OUT","ACTION_REQUIRED","STARTUP_FAI
PENDING = {"PENDING","IN_PROGRESS","QUEUED","EXPECTED","WAITING"}
PASS_REVIEW = {"SUCCESS","NEUTRAL","SKIPPED"}

reasons = []
ci_pending = False
perry_present = False
perry_terminal = False

for c in checks:
name, state = c["name"], c["state"]
if ai.search(name):
if name == "perry/review":
perry_present = True
if state not in PENDING:
perry_terminal = True
# AI reviewer checks are not required to exist, but a red one that
# did run still blocks — it is a red check on the PR like any other.
if state not in PASS_REVIEW and state not in PENDING:
print(f"FAIL_REVIEWER", file=sys.stderr)
print(f"reviewer {name}={state}")
Expand All @@ -178,8 +180,6 @@ if meta.get("reviewDecision") == "CHANGES_REQUESTED":
# Not failing. Decide PASS vs PENDING.
if ci_pending:
print("PENDING", file=sys.stderr); print("CI still running"); sys.exit(0)
if not (perry_present and perry_terminal):
print("PENDING", file=sys.stderr); print("waiting for perry/review"); sys.exit(0)
if meta.get("mergeable") != "MERGEABLE":
print("PENDING", file=sys.stderr); print(f"mergeable={meta.get('mergeable')}"); sys.exit(0)
print("PASS", file=sys.stderr); print("all green")
Comment on lines 180 to 185

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Release PRs can be auto-merged when no test results exist yet

An empty list of check results is treated as "all green" (the pass decision at .github/scripts/pr-gate.sh:183-185 runs even when checks is []), so a release PR whose tests have not registered yet — or whose results could not be read — is squash-merged without any CI having passed.

Impact: A version-bump/release pull request can be merged and published to npm without a single test or lint job having actually run.

Mechanism: dropping the required-reviewer condition removed the only "a check must exist" guard

Before this PR, PASS required perry_present and perry_terminal, which implicitly guaranteed at least one check existed on the PR. That requirement is now gone and nothing replaced it: the loop over checks (.github/scripts/pr-gate.sh:158-173) simply does nothing when the list is empty, leaving ci_pending = False, so the verdict falls through to PASS.

Two ways the list can be empty:

  • gh pr checks prints nothing and exits non-zero when no checks are reported yet on the head commit; .github/scripts/pr-gate.sh:143-144 swallows that and substitutes []. Right after changesets/action force-pushes the Version PR head, or right after the bump PR is created in .github/workflows/bump-openrouter-sdk.yaml, workflows registered by .github/workflows/ci.yaml (on: pull_request) may not be attached yet.
  • A transient API/auth error on the same call also yields [] (stderr is discarded), again reading as green.

The 45s settle re-check (.github/scripts/pr-gate.sh:228-235) narrows but does not close the window, and it does not help at all for the persistent case where the PR genuinely never gets checks.

A safe formulation is to require at least one non-AI-reviewer check present and successful before returning PASS, and to distinguish "gh pr checks failed" from "no checks".

Prompt for agents 
In .github/scripts/pr-gate.sh, the verdict() python block previously could not return PASS unless a perry/review check existed (perry_present/perry_terminal). With that requirement removed, an empty checks array now falls straight through to PASS, meaning the gate treats 'no checks at all' as 'all green' and will squash-merge (and thereby publish) a PR that never ran CI. The empty array arises both when gh pr checks legitimately reports no checks yet (freshly created bump PR, or Version PR head just force-pushed by changesets/action, before ci.yaml's pull_request workflows attach) and when the gh call fails transiently — line 143 discards stderr and line 144 substitutes '[]' in both cases. Consider (a) tracking whether at least one non-AI-reviewer check was seen and returning PENDING (not PASS) when none were, so the timeout path alerts instead of merging, and (b) distinguishing a failed gh pr checks invocation from a genuinely empty result so an API failure is reported as PENDING/failure rather than green.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines 180 to 185

@devin-ai-integration devin-ai-integration Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟨 Auto-merge gate can pass with zero checks, weakening the unattended-publish control

Removing the required-reviewer condition leaves no requirement that any check exists before the gate declares PASS and squash-merges. When gh pr checks returns nothing (no checks attached yet, or a suppressed API/auth failure at .github/scripts/pr-gate.sh:143-144), the verdict falls through to PASS and the PR is merged into main, which triggers publish.yaml and an npm release. This weakens the control that unattended merges into the publish pipeline are validated, leaving only the diff-scope script (verify-version-pr-scope.sh) as a real gate.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Expand All @@ -189,15 +189,10 @@ PY
echo "Gating PR #${PR} on ${REPO} (timeout ${TIMEOUT}s, max wall ${MAX_WALL}s, interval ${INTERVAL}s)"
START=$(date +%s)
WALL_START=$START # never reset — see MAX_WALL above
# perry/review's "never appeared" clock. Reset whenever a new head is adopted
# mid-gate: the fresh head's checks (perry included) start from scratch, so
# measuring them against the run's original start time would misreport a
# routine changesets/action refresh late in the poll as a token misconfig.
PERRY_START=$START
LAST_REASON=""

while :; do
NOW=$(date +%s); ELAPSED=$((NOW - START)); PERRY_ELAPSED=$((NOW - PERRY_START))
NOW=$(date +%s); ELAPSED=$((NOW - START))

# Deadlines at the TOP of the loop, before any branch can `continue` past
# them: the settle re-check path loops back whenever the verdict flips away
Expand Down Expand Up @@ -272,14 +267,13 @@ while :; do
if [ -n "$NEW_VETTED" ]; then
echo "PR #${PR} head moved ${EXPECTED_HEAD:0:7} → ${NEW_VETTED:0:7}; new diff passes the scope check — adopting vetted head and re-polling."
EXPECTED_HEAD="$NEW_VETTED"
# Fresh head, fresh checks — restart both clocks. Leaving the
# Fresh head, fresh checks — restart the clock. Leaving the
# overall deadline on the run's original start would misreport a
# refresh late in the window as "did not settle" when the new
# head's CI never had a chance to finish. Adoption requires
# passing the scope re-vet, so this cannot extend a run
# unboundedly on hostile pushes — those exit 1 above instead.
PERRY_START=$(date +%s)
START=$PERRY_START
START=$(date +%s)
LAST_REASON=""
continue
fi
Expand All @@ -300,7 +294,7 @@ while :; do
# where nobody is watching the run and the PR would sit unmerged
# until the next scheduled attempt.
if gh pr merge "$PR" -R "$REPO" --squash --delete-branch "${MATCH_ARGS[@]}"; then
slack ":white_check_mark: ${GATE_LABEL} <${PR_URL}|PR #${PR}> passed Perry + CI and was auto-merged."
slack ":white_check_mark: ${GATE_LABEL} <${PR_URL}|PR #${PR}> passed CI and was auto-merged."
else
slack ":x: ${GATE_LABEL} <${PR_URL}|PR #${PR}>: checks passed but the merge itself failed (branch protection? conflict?). Left open for a human. <${RUN_URL:-$PR_URL}|run>"
echo "::error::gh pr merge failed for PR #${PR}"
Expand All @@ -312,15 +306,6 @@ while :; do
fi
exit 0
;;
PENDING)
# If perry/review never even shows up, the PR was likely opened with a
# token that doesn't trigger it — surface that rather than hang forever.
if [ "$REASON" = "waiting for perry/review" ] && [ "$PERRY_ELAPSED" -ge "$PERRY_TIMEOUT" ]; then
slack ":warning: ${GATE_LABEL} <${PR_URL}|PR #${PR}>: perry/review never appeared after ${PERRY_TIMEOUT}s (token/app misconfig?). Not merging. <${RUN_URL:-$PR_URL}|run>"
echo "::error::perry/review did not appear within ${PERRY_TIMEOUT}s"
exit 1
fi
;;
esac

sleep "$INTERVAL"
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/bump-openrouter-sdk.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ jobs:
fi
exit 1

- name: Wait for Perry + CI, then merge or alert
- name: Wait for CI, then merge or alert
env:
GH_TOKEN: ${{ steps.app-token.outputs.token }}
PR: ${{ needs.bump.outputs.pr_number }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/release-train.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ name: Release train
# changesets/action maintains (see publish.yaml). Before this existed, that PR
# waited for a human to merge it, so shipped features sat unreleased between
# manual releases. The train bounds that latency: twice a week it finds the
# Version Packages PR, gates it on Perry + CI via pr-gate.sh (the same gate the
# Version Packages PR, gates it on CI via pr-gate.sh (the same gate the
# SDK-bump flow uses), and squash-merges on green — which triggers publish.yaml
# on push to main and cuts the actual npm release.
#
Expand Down
Loading