ci(repo): post Major Version Check as a commit status on the PR head#8761
ci(repo): post Major Version Check as a commit status on the PR head#8761jacekradko wants to merge 1 commit into
Conversation
The major-version guard reported pass/fail only via the implicit github-script job check-run. On issue_comment runs (the !allow-major re-run) that check-run is tied to the default branch, not the PR head, so approving a major bump never updated the PR head's status. That made the check unsafe to require: a major PR would stay blocked even after approval. Post an explicit "Major Version Check" commit status to the PR head SHA on every run, so the required context flips red->green correctly regardless of trigger event. Also paginate listFiles/listComments so a changeset or approval comment past the first page isn't missed now that this gates merges.
🦋 Changeset detectedLatest commit: 84d3685 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR refactors the major version check workflow to post explicit commit statuses on the PR head SHA instead of using step output failures. It adds pagination for files and comments, detects major changesets from ChangesMajor Version Check Workflow Refactor
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested Labels
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 @.github/workflows/major-version-check.yml:
- Around line 101-112: The catch block around
github.rest.orgs.checkMembershipForUser currently swallows all errors; change it
to only ignore a 404 by checking error?.status === 404 and continuing in that
case, but rethrow any other errors so transient 403/5xx fail fast; update the
catch that wraps the call to github.rest.orgs.checkMembershipForUser (the block
referencing comment.user.login and approvalFound) to implement this conditional
rethrow.
🪄 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: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: ebe6bfe6-06fb-45cb-85b5-de8a05713ccb
📒 Files selected for processing (2)
.changeset/major-version-check-pr-head-status.md.github/workflows/major-version-check.yml
| try { | ||
| const { status } = await github.rest.orgs.checkMembershipForUser({ | ||
| org, | ||
| username, | ||
| org: context.repo.owner, | ||
| username: comment.user.login, | ||
| }); | ||
|
|
||
| if (status === 204) { | ||
| approvalFound = true; | ||
| break; | ||
| } | ||
| } catch (error) { | ||
| if (error.status !== 204) { | ||
| // User is not a public member | ||
| continue; | ||
| } | ||
| // Not a public org member; keep looking. | ||
| continue; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the referenced workflow section
sed -n '1,200p' .github/workflows/major-version-check.yml | nl -ba | sed -n '80,160p'Repository: clerk/javascript
Length of output: 103
🌐 Web query:
GitHub REST API checkMembershipForUser error status codes 404 403 5xx and octokit behavior
💡 Result:
The GitHub REST API endpoint GET /orgs/{org}/members/{username}, accessible via octokit.rest.orgs.checkMembershipForUser, uses specific HTTP status codes to communicate membership status and authorization [1][2][3]. Status Codes Explained: - 204 No Content: The authenticated user is an organization member and the specified user is also a member of the organization [1][3]. - 302 Found: The authenticated user is not a member of the organization [1][3]. The response will typically redirect to check public membership instead [4]. - 404 Not Found: The authenticated user is an organization member, but the specified user is not a member of the organization [1][3]. This code is also returned if the organization or the user does not exist [5][6]. - 403 Forbidden: Returned if the request is made by a GitHub App that is blocked by the organization, if the requester has insufficient OAuth scopes (e.g., missing user or read:org), or if rate limits are exceeded [1][5][2]. Octokit Behavior: - Method Renaming: In older versions of Octokit, this was named checkMembership. It was renamed to checkMembershipForUser in version 18.0.0 [7]. - Error Handling: Octokit typically throws an error (often a RequestError or ApiException) for non-success status codes that are not explicitly handled as expected outcomes [8][9]. For example, in some Octokit implementations, a 404 response is caught and translated into a boolean false return value, while other error codes are raised as exceptions that your application must catch [9]. Always verify your specific version's implementation, as it may wrap these status codes differently depending on the language (e.g., Octokit.js vs Octokit.net) [9]. - 5xx Errors: These represent server-side issues at GitHub and are not specific to this endpoint. Octokit will generally throw these as exceptions [5]. Recommendation: Always implement robust error handling (e.g., try-catch blocks) when calling this method, as it relies on specific status codes to distinguish between successful membership checks and various failure scenarios [5][9].
Citations:
- 1: https://docs.github.com/en/enterprise-server@3.21/rest/orgs/members?apiVersion=2026-03-10
- 2: https://actions-cool.github.io/octokit-rest/api/orgs/
- 3: https://docs.github.com/en/rest/orgs/members?apiVersion=2026-03-10
- 4: https://docs.github.com/enterprise/2.2/developer/v3/orgs/members/
- 5: https://docs.github.com/en/rest/using-the-rest-api/troubleshooting-the-rest-api?apiVersion=2026-03-10
- 6: Typo in docs for Enterprise Server 3.8- Check organization membership for a user github/docs#27266
- 7: github.orgs.checkMembership is not a function actions/github-script#101
- 8: https://github.com/octokit/octokit.net/blob/7fa5b0fe/Octokit.Tests/Clients/OrganizationMembersClientTests.cs
- 9: https://github.com/octokit/octokit.net/blob/7fa5b0fe/Octokit/Clients/OrganizationMembersClient.cs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the referenced range with line numbers using awk (nl isn't available here)
awk 'NR>=80 && NR<=150 { printf "%4d\t%s\n", NR, $0 }' .github/workflows/major-version-check.ymlRepository: clerk/javascript
Length of output: 2463
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find other usages of checkMembershipForUser
rg -n "checkMembershipForUser\\(" -S .github/workflows || true
# Find common error shape handling patterns in workflows
rg -n "error\\?\\.status|error\\.status|statusCode" .github/workflows || trueRepository: clerk/javascript
Length of output: 582
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' .github/workflows/e2e-staging.yml | nl -ba | sed -n '35,95p'Repository: clerk/javascript
Length of output: 103
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=30 && NR<=110 { printf "%4d\t%s\n", NR, $0 }' .github/workflows/e2e-staging.ymlRepository: clerk/javascript
Length of output: 3366
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show top of workflow + setStatus helper and surrounding control flow
awk 'NR<=220 { printf "%4d\t%s\n", NR, $0 }' .github/workflows/major-version-check.ymlRepository: clerk/javascript
Length of output: 6141
Only treat 404 as “not a member” in checkMembershipForUser
In .github/workflows/major-version-check.yml (lines ~101-113), the catch currently continues on any checkMembershipForUser failure, so transient 403/5xx can wrongly prevent recognizing a valid !allow-major and flip an approved PR to failed. Keep swallowing 404, but rethrow everything else (repo already uses the error?.status === 404 pattern in .github/workflows/e2e-staging.yml).
Suggested fix
try {
const { status } = await github.rest.orgs.checkMembershipForUser({
org: context.repo.owner,
username: comment.user.login,
});
if (status === 204) {
approvalFound = true;
break;
}
} catch (error) {
- // Not a public org member; keep looking.
- continue;
+ if (error?.status === 404) {
+ // Not a public org member; keep looking.
+ continue;
+ }
+ throw error;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const { status } = await github.rest.orgs.checkMembershipForUser({ | |
| org, | |
| username, | |
| org: context.repo.owner, | |
| username: comment.user.login, | |
| }); | |
| if (status === 204) { | |
| approvalFound = true; | |
| break; | |
| } | |
| } catch (error) { | |
| if (error.status !== 204) { | |
| // User is not a public member | |
| continue; | |
| } | |
| // Not a public org member; keep looking. | |
| continue; | |
| try { | |
| const { status } = await github.rest.orgs.checkMembershipForUser({ | |
| org: context.repo.owner, | |
| username: comment.user.login, | |
| }); | |
| if (status === 204) { | |
| approvalFound = true; | |
| break; | |
| } | |
| } catch (error) { | |
| if (error?.status === 404) { | |
| // Not a public org member; keep looking. | |
| continue; | |
| } | |
| throw error; | |
| } |
🤖 Prompt for 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.
In @.github/workflows/major-version-check.yml around lines 101 - 112, The catch
block around github.rest.orgs.checkMembershipForUser currently swallows all
errors; change it to only ignore a 404 by checking error?.status === 404 and
continuing in that case, but rethrow any other errors so transient 403/5xx fail
fast; update the catch that wraps the call to
github.rest.orgs.checkMembershipForUser (the block referencing
comment.user.login and approvalFound) to implement this conditional rethrow.
1 participant
The major-version guard signalled pass/fail only through the implicit github-script job check-run. On
issue_commentruns (the!allow-majorapproval re-run) that check-run is tied to the default branch, not the PR head, so approving a major bump never updated the PR head's status. That's what kept this check off the required list: requiring it as-is would leave a major PR blocked even after approval.Now it posts an explicit
Major Version Checkcommit status to the PR head SHA on every run, so the required context flips red to green correctly no matter which event triggered the run. The job intentionally stays green when it posts a red status, otherwise the approval re-run's green status would sit behind a stale red job check-run that never clears (the re-run lands onmain, not the PR head).listFiles/listCommentsare now paginated so a changeset or approval comment past the first page isn't missed, which matters once this gates merges.Follow-up after this lands: add
Major Version Checkto the required status checks onmain(and optionallyrelease/v4/release/core-2). It can't be added before merge or it would block every open PR waiting on a context that doesn't exist yet.Summary by CodeRabbit