Skip to content

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

Merged
LukasParke merged 1 commit into
mainfrom
fix/pr-gate-ci-only
Aug 10, 2026
Merged

fix(release): gate the release train on CI, not a required perry/review#106
LukasParke merged 1 commit into
mainfrom
fix/pr-gate-ci-only

Conversation

@LukasParke

@LukasParke LukasParke commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Problem

pr-gate.sh refused to PASS until a check named exactly perry/review appeared and reached a terminal state. But perry has never posted a check in this repo — the AI reviewer that runs here is Devin Review. The requirement was inherited from the SDK bump flow the script was ported from (its default label is still @openrouter/sdk bump).

The first train run to ever reach the gate step (dry-run 31418471846, after the App-token fix) confirmed the stall: PENDING — waiting for perry/review until cancelled. Every scheduled train would have died the same way after 480s with "perry/review did not appear".

Fix

CI is the gate on PRs in this repo:

  • Dropped the perry_present/perry_terminal requirement, the PERRY_TIMEOUT machinery, and the never-appeared alert path.
  • Kept fail-closed behavior for anything that does post: a red AI-reviewer check still blocks (a red check is a red check), as does reviewDecision=CHANGES_REQUESTED.
  • PASS now means: no failing checks, none pending, mergeable.

Verification

Ran the modified script live against PR #88 in report-only mode (AUTO_MERGE=false): the verdict evaluates correctly end-to-end (it correctly reported mergeable=UNKNOWN after #88 was merged mid-poll — the merged-PR state, not a logic error).


Open in Devin Review

pr-gate.sh refused to PASS until a check named exactly perry/review
appeared and reached a terminal state — but perry has never posted a
check in this repo (the AI reviewer that runs here is Devin Review).
The requirement was inherited from the SDK bump flow the script was
ported from. The first train run to ever reach the gate step (dry-run
31418471846) confirmed the stall: PENDING 'waiting for perry/review'
until cancelled.

CI is the gate on PRs in this repo, so:
- drop the perry_present/perry_terminal requirement, the PERRY_TIMEOUT
  machinery, and the never-appeared alert path
- keep failing on any AI reviewer check that does run and goes red, and
  on reviewDecision=CHANGES_REQUESTED — a red check is a red check
- PASS now means: no failing checks, none pending, mergeable

Verified live against PR #88 in report-only mode: verdict evaluates
correctly end-to-end.

@devin-ai-integration devin-ai-integration Bot left a comment

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.

Devin Review found 2 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines 180 to 185
# 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")

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
# 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")

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.

@LukasParke
LukasParke merged commit a5b4d8c into main Aug 10, 2026
7 checks passed
@LukasParke
LukasParke deleted the fix/pr-gate-ci-only branch August 10, 2026 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant